diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e810b6..bcc60d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,11 +10,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - [#8]: `plugin.json`, declaring this repository as an [Agent Plugin](https://agent-plugins.org/). +- `scripts/eval-routing.sh` and `evals/routing.tsv`, checking that a prompt reaches the + skill it should. Routing happens on descriptions alone, and with eighteen skills they + had started to compete. Works with any OpenAI-compatible endpoint, including a local + one, and calls Anthropic directly when none is configured. +- `validate-skills.sh` rejects a frontmatter value containing an unquoted `": "`, which + breaks YAML parsing and shows up as an unhelpful parse error. +- `validate-skills.sh` requires every skill to have a routing case, so the case file cannot + fall behind the skill set. - [#8]: `validate-skills.sh` checks that the `.claude/skills` compatibility path still resolves to `skills/`, and that `plugin.json` and `CHANGELOG.md` agree on the version. ### Changed +- Sharpened the descriptions of `clean-code`, `solid-principles` and + `spring-boot-patterns`, which overlapped enough to send prompts to the wrong skill. + Each now says where it stops and which skill takes over. - [#8]: Skills moved from `.claude/skills/` to `skills/`, which is where the Agent Plugins standard expects them. `.claude/skills` remains as a symlink, so existing links and the paths in our own documentation keep working. On a checkout without symlink support, diff --git a/README.md b/README.md index 4f91dba..38d1a41 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,22 @@ The skills follow the [Agent Skills specification](https://agentskills.io/specif **Who is this for?** Java developers who want consistent, high-quality AI assistance for common tasks like code reviews, testing, commits, and architecture decisions. +## Routing is tested + +An agent picks a skill from its name and description and nothing else. With eighteen of +them the descriptions compete, and the failure is quiet: the wrong skill loads and answers +plausibly anyway, so nobody notices. + +`scripts/eval-routing.sh` runs a set of prompts against the same list an agent receives and +reports where one lands somewhere other than expected. It found a real defect the first +time it ran: "this class does too much, split it" reached `clean-code` rather than +`solid-principles`. Sharpening three descriptions took the set from 17 of 18 prompts routed +correctly to 20 of 20. + +Every skill has at least one case, enforced by `scripts/validate-skills.sh` so the cases +cannot fall behind the skills. The check runs against any OpenAI-compatible endpoint, +including a local model. See [docs/SCRIPTS.md](docs/SCRIPTS.md#check-routing). + ## Purpose AI-powered development workflows with focus on: diff --git a/docs/SCRIPTS.md b/docs/SCRIPTS.md index d67455e..9ad97c9 100644 --- a/docs/SCRIPTS.md +++ b/docs/SCRIPTS.md @@ -12,6 +12,7 @@ | `configure-mcp.sh` | Generates MCP config and optionally adds servers | | `configure-settings.sh` | Copies Claude Code settings with pre-approved commands | | `validate-skills.sh` | Validates skills against the Agent Skills specification | +| `eval-routing.sh` | Checks that a prompt reaches the skill it should | | `test-all.sh` | Runs all tests to validate scripts work | ## Usage @@ -54,9 +55,15 @@ cd /path/to/claude-code-java Errors fail the run, recommendations are reported as warnings. -Beyond the spec, it checks two things specific to this repository: that the `.claude/skills` +Beyond the spec it checks three things specific to this repository: that the `.claude/skills` compatibility symlink still resolves to `skills/`, which a checkout without symlink support -silently breaks, and that `plugin.json` and `CHANGELOG.md` carry the same version. +silently breaks; that `plugin.json` and `CHANGELOG.md` carry the same version; and that every +skill has at least one case in `evals/routing.tsv`. + +That last one is a plain text check with no API call, so it runs on pull requests from forks. +It exists because a skill with no case is a skill nobody checks, and because a new skill +competes with the existing ones for the same prompts. Adding one can move traffic away from +another, which is why the checklist says to run the whole eval, not just the new case. #### Why two validators @@ -72,6 +79,45 @@ comma-separated; this repo's convention that every skill ships a `README.md`; an recommendations for the body and the description. It also runs with no Python, which matters in a repo that is otherwise bash and markdown. +### Check Routing + +```bash +./scripts/eval-routing.sh +``` + +An agent choosing between skills sees their names and descriptions, nothing else. With +eighteen of them the descriptions start competing, and the failure is quiet: the wrong +skill loads and answers plausibly. This runs the cases in `evals/routing.tsv` against the +same `` block an agent gets, and reports where a prompt lands somewhere +other than expected. + +Any model will do. Set `EVAL_BASE_URL` to use an OpenAI-compatible endpoint, which most +providers and local runtimes speak: + +```bash +EVAL_BASE_URL=https://api.openai.com/v1 EVAL_MODEL=gpt-4o-mini EVAL_API_KEY=... +EVAL_BASE_URL=http://localhost:11434/v1 EVAL_MODEL=llama3.1 EVAL_API_KEY=ollama +EVAL_BASE_URL=https://openrouter.ai/api/v1 EVAL_MODEL=... EVAL_API_KEY=... +``` + +With no `EVAL_BASE_URL` it calls Anthropic directly and reads the key from +`ANTHROPIC_API_KEY`, `ANTHROPIC_API_KEY_FILE`, or `~/.config/anthropic/api-key`. Prefer a +file, so the key stays out of shell history. + +The model is named in the summary line, because a result only means something next to the +model that produced it. + +Every skill should have at least one case. Cases marked `AMBIGUOUS` have no agreed answer +and are there to record prompts that are underspecified rather than to be fixed. + +This is deliberately 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. Run it before a release, or after changing a description. + +It does not measure output quality. For that the standard defines `evals/evals.json` per +skill, run with and without the skill to get a baseline. See +[the specification's guidance](https://agentskills.io/skill-creation/evaluating-skills). + ## Conventions All scripts follow the same structure for consistency and reliability. diff --git a/docs/SKILL_GUIDELINES.md b/docs/SKILL_GUIDELINES.md index 8edc28c..3ed6358 100644 --- a/docs/SKILL_GUIDELINES.md +++ b/docs/SKILL_GUIDELINES.md @@ -88,7 +88,33 @@ The skill should add something NEW: - "Other tools have this" (without validating the gap) - "I want to reorganize existing content" -### 5. Is It Focused Enough? +### 5. Will It Be Reached? + +The `description` is not a label. It is the whole of what an agent sees when it decides +which skill to load, alongside the name. Nothing else in `SKILL.md` takes part in that +decision, so a skill with excellent content and a vague description is a skill that never +runs, and a skill with a greedy description steals prompts from its neighbours. + +Two rules follow. + +**Say where you stop.** A description should name the boundary with the skills nearest to +it and point at them. `performance-smell-detection` does this: + +> ... Provides awareness, not absolutes - always measure before optimizing. For +> JPA/database performance, use jpa-patterns instead. + +Without that sentence, prompts about slow queries split unpredictably between two skills. + +**Describe symptoms, not the subject.** Users write what is wrong, not what the topic is +called. "This class does too much" reaches `solid-principles` because the description +mentions a class with too many responsibilities. It would not reach a description that only +listed the five principles by name. + +Adding a skill changes routing for every other skill, because they all compete for the same +prompts. Add a case to `evals/routing.tsv` and run `./scripts/eval-routing.sh` over the +whole set, not just the new case. + +### 6. Is It Focused Enough? A skill should be completable in one session. Signs it's too broad: - More than 10-15 checklist items @@ -399,6 +425,8 @@ Before submitting a new skill: - [ ] Identified clear level (micro/meso/macro/framework/cross) - [ ] Determined type (audit vs template) - [ ] Documented unique value added +- [ ] Description names its boundary with the nearest skills +- [ ] Case added to `evals/routing.tsv`, and the full routing check rerun - [ ] SKILL.md follows structure convention - [ ] README.md provides human-friendly docs - [ ] All examples are Java-specific diff --git a/evals/routing.tsv b/evals/routing.tsv new file mode 100644 index 0000000..3ffecdf --- /dev/null +++ b/evals/routing.tsv @@ -0,0 +1,24 @@ +# One line per case: prompt expected skill. +# Cases marked AMBIGUOUS have no single right answer today; they exist to prove it. +review this pull request before I merge java-code-review +my repository fires 200 queries for one page jpa-patterns +how should I structure a Spring Boot controller spring-boot-patterns +are my REST endpoints still backwards compatible api-contract-review +this class does too much, split it solid-principles +this method is 300 lines and unreadable clean-code +my @Async method is not running in parallel concurrency-review +we allocate in a hot loop and box everything performance-smell-detection +upgrade this project from Java 17 to 21 java-migration +add tests for this service test-quality +commit these changes git-commit +any known CVEs in our dependencies maven-dependency-audit +is this query open to SQL injection security-audit +add MDC so I can trace a request across services logging-patterns +which pattern fits pluggable strategies design-patterns +are my packages layered correctly architecture-review +triage the open issues issue-triage +what changed since the last release changelog-generator +my Spring Boot app throws LazyInitializationException jpa-patterns +these variable names are cryptic, rename them clean-code +refactor this class AMBIGUOUS +review this code AMBIGUOUS diff --git a/scripts/eval-routing.sh b/scripts/eval-routing.sh new file mode 100755 index 0000000..5e02f70 --- /dev/null +++ b/scripts/eval-routing.sh @@ -0,0 +1,142 @@ +#!/bin/bash +# eval-routing.sh - Check that a prompt reaches the skill it should +# Usage: ./eval-routing.sh [cases-file] +# +# Routing happens on the description alone: an agent sees name, description and path, +# nothing more. This asks a model to pick from exactly that, so a description change can +# be measured instead of argued about. +# +# Any model will do. Set EVAL_BASE_URL to use an OpenAI-compatible endpoint, which most +# providers and local runtimes speak: +# +# EVAL_BASE_URL=https://api.openai.com/v1 EVAL_MODEL=gpt-4o-mini EVAL_API_KEY=... +# EVAL_BASE_URL=http://localhost:11434/v1 EVAL_MODEL=llama3.1 EVAL_API_KEY=ollama +# EVAL_BASE_URL=https://openrouter.ai/api/v1 EVAL_MODEL=... EVAL_API_KEY=... +# +# With no EVAL_BASE_URL it calls Anthropic directly, reading the key from +# ANTHROPIC_API_KEY, or from ANTHROPIC_API_KEY_FILE, or from ~/.config/anthropic/api-key. +# Prefer a file, so the key stays out of shell history and out of any transcript. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +WORKSPACE_DIR="$(dirname "$SCRIPT_DIR")" +CASES="${1:-$WORKSPACE_DIR/evals/routing.tsv}" + +[ ! -f "$CASES" ] && echo "❌ Cases file not found: $CASES" && exit 1 + +if [ -n "${EVAL_BASE_URL:-}" ]; then + PROVIDER="openai" + EVAL_API_KEY="${EVAL_API_KEY:-}" + if [ -z "${EVAL_MODEL:-}" ]; then + echo "❌ EVAL_BASE_URL is set, so EVAL_MODEL is needed too. There is no sensible default." + exit 1 + fi +else + PROVIDER="anthropic" + KEY_FILE="${ANTHROPIC_API_KEY_FILE:-$HOME/.config/anthropic/api-key}" + if [ -z "${ANTHROPIC_API_KEY:-}" ] && [ -r "$KEY_FILE" ]; then + ANTHROPIC_API_KEY="$(tr -d '\r\n' < "$KEY_FILE")" + fi + if [ -z "${ANTHROPIC_API_KEY:-}" ]; then + echo "❌ No API key. Either set EVAL_BASE_URL for an OpenAI-compatible endpoint," + echo " or set ANTHROPIC_API_KEY, or put the key in $KEY_FILE" + echo " mkdir -p ~/.config/anthropic && chmod 700 ~/.config/anthropic" + echo " printf %s 'YOUR_KEY' > ~/.config/anthropic/api-key && chmod 600 ~/.config/anthropic/api-key" + exit 1 + fi + EVAL_API_KEY="$ANTHROPIC_API_KEY" + EVAL_MODEL="${EVAL_MODEL:-claude-haiku-4-5-20251001}" +fi +export PROVIDER EVAL_API_KEY EVAL_MODEL EVAL_BASE_URL="${EVAL_BASE_URL:-}" + +# skills-ref installs its command as agentskills. Use it if it is on PATH, otherwise run +# it on demand, so nobody has to install anything to run one eval. +if command -v agentskills >/dev/null 2>&1; then + AGENTSKILLS=(agentskills) +elif command -v uvx >/dev/null 2>&1; then + AGENTSKILLS=(uvx --from skills-ref agentskills) +elif command -v pipx >/dev/null 2>&1; then + AGENTSKILLS=(pipx run --spec skills-ref agentskills) +else + echo "❌ Need the reference validator. Install it with: pip install skills-ref" + exit 1 +fi + +# The same block an agent is given. +SKILLS_BLOCK="$("${AGENTSKILLS[@]}" to-prompt "$WORKSPACE_DIR"/skills/*/ 2>/dev/null)" +if [ -z "$SKILLS_BLOCK" ]; then + echo "❌ Could not build the skills block with: ${AGENTSKILLS[*]}" + exit 1 +fi + +SKILL_NAMES="$(basename -a "$WORKSPACE_DIR"/skills/*/ | tr '\n' ' ')" + +PASS=0; FAIL=0; AMBIGUOUS=0 + +while IFS=$'\t' read -r prompt expected; do + case "$prompt" in ''|'#'*) continue;; esac + + picked="$(SKILLS_BLOCK="$SKILLS_BLOCK" PROMPT="$prompt" SKILL_NAMES="$SKILL_NAMES" python3 - <<'PY' +import json, os, urllib.request + +SYSTEM = ("You route a user request to one skill. Reply with the skill name and nothing " + "else, or NONE if no skill fits. Decide from the descriptions alone.") +user = os.environ["SKILLS_BLOCK"] + "\n\nUser request: " + os.environ["PROMPT"] + "\n\nWhich skill?" + +if os.environ["PROVIDER"] == "openai": + url = os.environ["EVAL_BASE_URL"].rstrip("/") + "/chat/completions" + payload = {"model": os.environ["EVAL_MODEL"], "max_tokens": 24, "messages": [ + {"role": "system", "content": SYSTEM}, {"role": "user", "content": user}]} + headers = {"content-type": "application/json", + "authorization": "Bearer " + os.environ["EVAL_API_KEY"]} + pick = lambda d: d["choices"][0]["message"]["content"] +else: + url = "https://api.anthropic.com/v1/messages" + payload = {"model": os.environ["EVAL_MODEL"], "max_tokens": 24, "system": SYSTEM, + "messages": [{"role": "user", "content": user}]} + headers = {"content-type": "application/json", + "x-api-key": os.environ["EVAL_API_KEY"], + "anthropic-version": "2023-06-01"} + pick = lambda d: d["content"][0]["text"] + +req = urllib.request.Request(url, data=json.dumps(payload).encode(), headers=headers) +try: + with urllib.request.urlopen(req, timeout=60) as r: + text = pick(json.load(r)).strip() +except Exception as e: + print(f"ERROR:{e}") +else: + # Asking for a bare name does not guarantee one. An answer may open with prose, so + # take the first known skill name that appears rather than the first word. + names = os.environ["SKILL_NAMES"].split() + hit = min(((text.find(n), n) for n in names if n in text), default=None) + print(hit[1] if hit else "NONE") +PY +)" + + case "$picked" in + ERROR:*) + echo "❌ $picked" + echo " Stopping on the first failed call rather than repeating it for every case." + exit 1 + ;; + esac + + if [ "$expected" = "AMBIGUOUS" ]; then + echo "⚠️ $prompt" + echo " picked $picked, no agreed answer" + AMBIGUOUS=$((AMBIGUOUS + 1)) + elif [ "$picked" = "$expected" ]; then + echo "✓ $prompt" + PASS=$((PASS + 1)) + else + echo "✗ $prompt" + echo " expected $expected, got $picked" + FAIL=$((FAIL + 1)) + fi +done < "$CASES" + +echo "" +echo "$PASS routed correctly, $FAIL wrong, $AMBIGUOUS undecided (model: $EVAL_MODEL)" +[ "$FAIL" -eq 0 ] || exit 1 diff --git a/scripts/validate-skills.sh b/scripts/validate-skills.sh index 59169f8..681b464 100755 --- a/scripts/validate-skills.sh +++ b/scripts/validate-skills.sh @@ -99,6 +99,11 @@ for dir in "$SKILLS_DIR"/*/; do || fail "$name: name must be lowercase alphanumeric and single hyphens, no leading or trailing hyphen" fi + # An unquoted YAML scalar cannot contain ": ". The reference parser rejects the + # whole file, and the failure looks like a parse error rather than a typo. + frontmatter "$skill" | grep -qE '^[A-Za-z][A-Za-z0-9_-]*:[ \t]+[^"'"'"'].*: ' \ + && fail "$name: a frontmatter value contains \": \", which breaks YAML unless quoted" + desc="$(field "$skill" description)" if [ -z "$desc" ]; then fail "$name: 'description' is required and must be non-empty" @@ -137,6 +142,20 @@ for dir in "$SKILLS_DIR"/*/; do [ "$lines" -le 500 ] || warn "$name: SKILL.md is $lines lines, the spec recommends under 500" done +# A skill with no routing case is a skill nobody checks. Worse, a new skill competes +# with every existing one for the same prompts, so adding one without a case can move +# traffic away from another and go unnoticed. This is a plain text check, no API key, +# so it runs on pull requests from forks too. +CASES="$WORKSPACE_DIR/evals/routing.tsv" +if [ -f "$CASES" ]; then + for dir in "$SKILLS_DIR"/*/; do + [ -d "$dir" ] || continue + skill="$(basename "$dir")" + grep -qE " $skill\$" "$CASES" \ + || fail "$skill: no case in evals/routing.tsv, add one and rerun eval-routing.sh" + done +fi + # One version, two files that must agree. plugin.json is the anchor; the changelog's # top released heading has to match it, so a release cannot half-happen. MANIFEST="$WORKSPACE_DIR/plugin.json" diff --git a/skills/README.md b/skills/README.md index 5c5477a..25e83c3 100644 --- a/skills/README.md +++ b/skills/README.md @@ -69,7 +69,10 @@ Validate your skill idea against existing skills: 3. Create `README.md` with human documentation (use existing READMEs as template) 4. Update this table 5. Update main README.md -6. Run `./scripts/validate-skills.sh` +6. Add at least one case to `evals/routing.tsv` +7. Run `./scripts/validate-skills.sh` +8. Run `./scripts/eval-routing.sh`. A new skill competes with the existing ones for the + same prompts, so check the whole set, not only the new case. ## Usage diff --git a/skills/clean-code/SKILL.md b/skills/clean-code/SKILL.md index 286745e..317bead 100644 --- a/skills/clean-code/SKILL.md +++ b/skills/clean-code/SKILL.md @@ -1,6 +1,6 @@ --- name: clean-code -description: Clean Code principles (DRY, KISS, YAGNI), naming conventions, function design, and refactoring. Use when user says "clean this code", "refactor", "improve readability", or when reviewing code quality. +description: Clean Code principles (DRY, KISS, YAGNI), naming, function design and readability. Use when code is hard to read, with long methods, unclear names, duplication or deep nesting. For how responsibilities are split across classes and which way dependencies point, use solid-principles instead. license: MIT --- diff --git a/skills/solid-principles/SKILL.md b/skills/solid-principles/SKILL.md index 84fb770..dce44a4 100644 --- a/skills/solid-principles/SKILL.md +++ b/skills/solid-principles/SKILL.md @@ -1,6 +1,6 @@ --- name: solid-principles -description: SOLID principles checklist with Java examples. Use when reviewing classes, refactoring code, or when user asks about Single Responsibility, Open/Closed, Liskov, Interface Segregation, or Dependency Inversion. +description: SOLID principles checklist with Java examples. Use when a class has too many responsibilities, an abstraction leaks, or a dependency points the wrong way, and when the user asks about Single Responsibility, Open/Closed, Liskov, Interface Segregation or Dependency Inversion. For naming, duplication and method length, use clean-code instead. license: MIT --- diff --git a/skills/spring-boot-patterns/SKILL.md b/skills/spring-boot-patterns/SKILL.md index cba0128..8b6a701 100644 --- a/skills/spring-boot-patterns/SKILL.md +++ b/skills/spring-boot-patterns/SKILL.md @@ -1,6 +1,6 @@ --- name: spring-boot-patterns -description: Spring Boot best practices and patterns. Use when creating controllers, services, repositories, or when user asks about Spring Boot architecture, REST APIs, exception handling, or JPA patterns. +description: Spring Boot best practices and patterns. Use when creating controllers, services or repositories, or when the user asks about Spring Boot layering, wiring, configuration or exception handling. For JPA and Hibernate behaviour, use jpa-patterns instead. license: MIT ---