addyosmani / addyosmani/agent-skills

Routing ambiguity: code-reviewer persona and code-review-and-quality skill compete for the same natural-language intent

Aperta
#173 1 commento 1 reazione 0 assegnatari Vedi su GitHub
Lingua principale
JavaScript
Stelle
93.8k
Fork
10k
Merge medio
3g 15h
PR unite (30g)
23

Descrizione

## Summary

When a user makes a natural-language request like "review this PR" or
"review this change", there is no deterministic way for the LLM to decide
whether to route to the `code-reviewer` persona (`agents/code-reviewer.md`)
or to invoke the `code-review-and-quality` skill
(`skills/code-review-and-quality/SKILL.md`).

Their descriptions overlap, and the project's disambiguation rules live in
files that Claude Code does not auto-load into the agent's context at
routing time. The decision is therefore left to the LLM's reading of two
near-identical descriptions.

## The two competing entry points

**Persona** — `agents/code-reviewer.md:3`:
> Senior code reviewer that evaluates changes across five dimensions —
> correctness, readability, architecture, security, and performance.
> **Use for thorough code review before merge.**

**Skill** — `skills/code-review-and-quality/SKILL.md:3`:
> Conducts multi-axis code review. **Use before merging any change.**
> **Use when reviewing code written by yourself, another agent, or a human.**
> Use when you need to assess code quality across multiple dimensions
> before it enters the main branch.

Both descriptions match the same natural-language intent. The skill's
description in particular reads like a user-facing entry point.

## Where the routing rules actually live (and why the LLM can't see them)

The repo does document the intended split, but in three different files
that contradict each other:

- `AGENTS.md` Intent → Skill Mapping says "Code review → `code-review-and-quality`"
- `agents/README.md` Decision matrix says "Review this PR" → invoke
`code-reviewer` directly
- `references/orchestration-patterns.md` documents fan-out for `/ship`

The bigger problem: at routing time, the LLM only sees

- skill names + descriptions (injected via system-reminder)
- subagent type names + descriptions
- `CLAUDE.md` (auto-loaded)
- the user's private global rules

`AGENTS.md`, `agents/README.md`, and `references/orchestration-patterns.md`
are **not** in that list — Claude Code does not auto-load them. So at the
exact moment the LLM has to pick between persona and skill, the repo's
three-layer composition rule is invisible to it.

(`AGENTS.md` is the OpenCode / Codex convention; Claude Code does not read
it. The plugin manifest at `.claude-plugin/plugin.json` doesn't ship it
either.)

## Proposed fixes (sketch)

Two cheap interventions; doing both is best:

**A. Rewrite descriptions so they are disjoint.** Push the routing
contract into the only place that is always loaded — the descriptions
themselves.

~~~yaml
# skills/code-review-and-quality/SKILL.md
description: |
Code review workflow library — defines the 5-axis framework, severity
rubric, process steps, and exit criteria. Invoked BY the code-reviewer
persona or by /review and /ship slash commands. NOT a user-facing entry
point; invoking this directly skips voice, verdict, and output formatting.

# agents/code-reviewer.md
description: |
User-facing entry point for code review. Senior Staff Engineer voice with
a structured Verdict report. Use when the user asks "review this PR /
change" in natural language. Internally follows the
code-review-and-quality skill.
~~~

**B. Move the routing table into `CLAUDE.md`** (which IS auto-loaded by
Claude Code):

~~~markdown
## Routing: code review
- Natural language ("review this", "审查") → code-reviewer persona
- /review → code-review-and-quality skill
- /ship → fan-out (3 personas in parallel)
- Programmatic batch review (CI / scripts) → invoke skill directly
~~~

## Same pattern repeats elsewhere

The same routing ambiguity exists between

- `security-auditor` ↔ `security-and-hardening`
- `test-engineer` ↔ `test-driven-development`

Whatever fix is chosen should be applied consistently across all three pairs.

## Related

Companion issue #172 covers the structural reason this routing problem
exists: `code-reviewer.md` duplicates the skill's framework rather than
delegating to it, which forces the two descriptions to compete for the
same intent.

Happy to send a PR implementing A + B if the direction sounds right.

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.