Check that prompts reach the right skill - #10
Merged
Merged
Conversation
An agent picks a skill from the descriptions alone, and three of them competed. solid-principles and clean-code both claimed refactoring and reviewing, and spring-boot-patterns claimed JPA patterns, which is another skill's subject. Measured before and after with scripts/eval-routing.sh: "this class does too much, split it" went to clean-code instead of solid-principles. Each description now states where it stops and which skill takes over, the way performance-smell-detection already did. Twenty of twenty cases route correctly after the change, against seventeen of eighteen before.
The standard's own eval guidance covers output quality: does a skill make the answer better than no skill. It assumes the right skill was already chosen. With eighteen skills that assumption is where the problem is, and the failure is quiet, because the wrong skill still answers plausibly. This runs prompts against the same <available_skills> block an agent receives and reports where one lands somewhere other than expected, so a description change can be measured rather than argued about. Not wired into CI. Fork pull requests do not get secrets, and a model's answer can vary between runs, so it belongs before a release rather than on a pull request.
An unquoted YAML scalar cannot contain ": ". The reference validator rejects the whole file and reports a parse error pointing at a line number, which does not say what is wrong. This caught it while writing a description in this very branch.
|
Verdict: APPROVE Files reviewed:
Findings: Overlap: PASS
Quality: PASS
Action required: None - ready to merge |
A skill with no case is a skill nobody checks. The larger risk is that a new skill competes with every existing one for the same prompts, so adding one can move traffic away from another and go unnoticed, which is why the checklist now says to rerun the whole eval rather than only the new case. The check reads the case file as text, with no API call, so it runs on pull requests from forks like everything else in validate-skills.sh.
|
Verdict: APPROVE Files reviewed:
Findings:
Action required: None — ready to merge |
The script spoke to one vendor, in a repository that had just stopped describing itself by one client. Setting EVAL_BASE_URL now sends it to any OpenAI-compatible endpoint, which covers the hosted providers, the aggregators and local runtimes like Ollama. With nothing configured it calls Anthropic as before. The summary line names the model, because a routing result only means something next to the model that produced it.
|
Verdict: APPROVE Files reviewed:
Findings:
Action required: None - ready to merge. The routing evaluation (evals/routing.tsv + eval-routing.sh) provides concrete validation that these descriptions reduce ambiguity. |
SKILL_GUIDELINES covered overlap, level and type, but treated the description as a label. It is the only thing an agent reads when choosing, so a skill with good content and a vague description never runs, and one with a greedy description takes prompts from its neighbours. States the two rules that follow, name your boundary and describe symptoms rather than the subject, and points at the routing check. The README gets three sentences, next to the existing note about automated review.
The summary at the end of SKILL_GUIDELINES lists what to check before submitting a skill, and it had not caught up with the routing rules above it.
|
Verdict: APPROVE Files reviewed:
Findings:
Action required: None — ready to merge |
It sat next to Contributing, where nobody scanning the first two screens would find it, which is a poor place for the one thing here that nothing else does. Now carries the measurement rather than the claim: a real defect found on the first run, and 17 of 18 to 20 of 20 after three descriptions were sharpened. Anyone who clones can check both.
|
Verdict: APPROVE Files reviewed: skills/clean-code/SKILL.md, skills/solid-principles/SKILL.md, skills/spring-boot-patterns/SKILL.md Findings:
Action required: None - ready to merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An agent choosing between our skills sees their names and descriptions and nothing else. With eighteen of them the descriptions had started to compete, and the failure is quiet: the wrong skill loads and still answers plausibly.
scripts/eval-routing.shruns prompts against the same<available_skills>block an agent receives and reports where one lands somewhere other than expected. It found a real defect on the first run: "this class does too much, split it" went toclean-coderather thansolid-principles, which is the textbook Single Responsibility case.The fix is the technique
performance-smell-detectionwas already using and nothing else was: say where you stop and who takes over.solid-principlesandclean-codenow divide along class responsibilities versus readability, andspring-boot-patternsno longer claims JPA, which belongs tojpa-patterns.Measured, not argued: seventeen of eighteen cases before, twenty of twenty after. Two cases stay marked
AMBIGUOUSon purpose. "refactor this class" does not say what is wrong with it, so no skill is the right answer and pretending otherwise would just hide an underspecified prompt.The validator also learned to reject a frontmatter value containing an unquoted
": ". That is not hypothetical: the first draft of theclean-codedescription contained one, the reference validator rejected the file with a line-number parse error, and our own checker passed it. Both validators earn their place.What this is not
It does not measure output quality, and it does not try to. The standard already covers that with
evals/evals.jsonper skill, run once with the skill and once without so there is a baseline, and there are tools that implement it. Building a second thing there would have been a waste. This covers the part nothing else does.It is also not a pull request check. Fork pull requests do not get repository secrets, so it would fail for every outside contributor, and a model's answer can vary between runs. It belongs before a release, or after changing a description.
Follow-ups, not in here
validate-skills.shcould require every skill to have at least one routing case, so the case file cannot quietly fall behind the skill set. And the output-quality side is worth doing on two or three skills, which would finally answer whether these skills help at all, a question that has been open in this project from the start.