From 6dd43e78ba1bf773a930994159369c44dc334918 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 31 Aug 2026 23:22:25 -0400 Subject: [PATCH] docs: tune skill eval cases and document how to write them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `skills:eval` was at 38/46 with several skills measuring 0-33%. The low scores were mostly bad eval cases rather than bad descriptions, for two reasons that were nowhere written down: - The harness runs each case with `--max-turns 1`, so the skill has to be the model's FIRST tool call. A prompt asking "how does X work here", or naming a concrete file, sends the model to Read/Grep first — a better answer to the question, scored as a miss. Every 0% case was one of these. - `AGENTS.md` is resident on every turn, so a prompt it already answers is answered without any skill and the case can never pass. Replace the cases that measured the wrong thing, expand every model-invoked skill from 3 to 5 positives (negatives kept) so one flake cannot cross the threshold, and reshape `local-dev`'s description to the action-first form. Add `docs/skill-authoring.md` with the case shapes that fire, the ones that cannot, and the probe-then-measure loop. RUNS=5 npm run skills:eval: 63/63 at or above threshold, 60 at 100%, all 18 negatives clean. Closes #2202 Signed-off-by: cliffhall --- .claude/skills/board-ops/evals/evals.json | 8 + .claude/skills/issue-create/evals/evals.json | 8 + .claude/skills/issue-triage/evals/evals.json | 10 +- .claude/skills/local-dev/SKILL.md | 2 +- .claude/skills/local-dev/evals/evals.json | 4 + .claude/skills/pr-flow/evals/evals.json | 8 + .claude/skills/pre-push-gate/evals/evals.json | 10 +- .../skills/project-structure/evals/evals.json | 12 +- .claude/skills/test-servers/evals/evals.json | 8 + .claude/skills/testing/evals/evals.json | 24 ++- AGENTS.md | 5 +- README.md | 1 + docs/skill-authoring.md | 181 ++++++++++++++++++ 13 files changed, 267 insertions(+), 14 deletions(-) create mode 100644 docs/skill-authoring.md diff --git a/.claude/skills/board-ops/evals/evals.json b/.claude/skills/board-ops/evals/evals.json index 8bd8dcd9..ac476e90 100644 --- a/.claude/skills/board-ops/evals/evals.json +++ b/.claude/skills/board-ops/evals/evals.json @@ -11,6 +11,14 @@ "prompt": "The project board rejected my Status option ID. How do I get the current ones?", "expect": "board-ops" }, + { + "prompt": "I'm about to edit the options on one of the board's single-select fields. Should I do anything first?", + "expect": "board-ops" + }, + { + "prompt": "Several cards lost their Priority value after the field was edited. How do I put it back?", + "expect": "board-ops" + }, { "prompt": "Explain the difference between TCP and UDP.", "expect": null diff --git a/.claude/skills/issue-create/evals/evals.json b/.claude/skills/issue-create/evals/evals.json index b701c81d..d9aa0502 100644 --- a/.claude/skills/issue-create/evals/evals.json +++ b/.claude/skills/issue-create/evals/evals.json @@ -11,6 +11,14 @@ "prompt": "Before I file this, how do I check nobody has reported it already?", "expect": "issue-create" }, + { + "prompt": "I found a bug while working on something else. How do I get it tracked in this repo?", + "expect": "issue-create" + }, + { + "prompt": "What do I need to do beyond `gh issue create` for a new issue to be properly tracked here?", + "expect": "issue-create" + }, { "prompt": "What is the chemical symbol for potassium?", "expect": null diff --git a/.claude/skills/issue-triage/evals/evals.json b/.claude/skills/issue-triage/evals/evals.json index 067302f1..3329e2cf 100644 --- a/.claude/skills/issue-triage/evals/evals.json +++ b/.claude/skills/issue-triage/evals/evals.json @@ -4,13 +4,21 @@ "expect": "issue-triage" }, { - "prompt": "What Priority should issue 2189 be given?", + "prompt": "How do I decide what Priority an incoming issue deserves?", "expect": "issue-triage" }, { "prompt": "Audit the project board for anything that has drifted.", "expect": "issue-triage" }, + { + "prompt": "How do I find the issues that aren't on either board yet?", + "expect": "issue-triage" + }, + { + "prompt": "After scoring an issue against the rubric, what do I do with the score?", + "expect": "issue-triage" + }, { "prompt": "Who wrote the novel Moby-Dick?", "expect": null diff --git a/.claude/skills/local-dev/SKILL.md b/.claude/skills/local-dev/SKILL.md index 168a1b2c..dfc0930d 100644 --- a/.claude/skills/local-dev/SKILL.md +++ b/.claude/skills/local-dev/SKILL.md @@ -1,6 +1,6 @@ --- name: local-dev -description: Getting the Inspector running locally, and the reasoning behind its dependency rules. Which install to run and which script starts each client (web, cli, tui, launcher); why v2 is not an npm workspace; how the @inspector/core alias works; and why the placement rules in AGENTS.md are what they are — what each one is defending against, what it looked like when it was violated, and how to tell you have hit one. +description: Install, build and run the Inspector locally, and place a dependency correctly. Use when starting the web, cli, tui or launcher client; when asking whether a build or rebuild is needed before a run script works; when an install, an @inspector/core import or a client bundle fails at run time; when adding or bumping a dependency and choosing which package.json declares it; or when a bundler external, a version skew or a git worktree is the suspect. disable-model-invocation: false --- diff --git a/.claude/skills/local-dev/evals/evals.json b/.claude/skills/local-dev/evals/evals.json index f3c7d909..a95355ce 100644 --- a/.claude/skills/local-dev/evals/evals.json +++ b/.claude/skills/local-dev/evals/evals.json @@ -15,6 +15,10 @@ "prompt": "How do I run the CLI from the built launcher in this repo instead of via npx?", "expect": "local-dev" }, + { + "prompt": "I just pulled and a client's dependencies look out of sync. What do I need to run?", + "expect": "local-dev" + }, { "prompt": "Write a haiku about autumn.", "expect": null diff --git a/.claude/skills/pr-flow/evals/evals.json b/.claude/skills/pr-flow/evals/evals.json index 0b83c32e..d8d87f3a 100644 --- a/.claude/skills/pr-flow/evals/evals.json +++ b/.claude/skills/pr-flow/evals/evals.json @@ -11,6 +11,14 @@ "prompt": "The DCO check is red on my branch. How do I fix the commits I already pushed?", "expect": "pr-flow" }, + { + "prompt": "Copilot left a pile of review comments on my PR. How do I answer them so nothing gets lost?", + "expect": "pr-flow" + }, + { + "prompt": "My PR is open and green. Walk me through getting it merged and closed out here.", + "expect": "pr-flow" + }, { "prompt": "What does this regex match? /^[a-z]+$/", "expect": null diff --git a/.claude/skills/pre-push-gate/evals/evals.json b/.claude/skills/pre-push-gate/evals/evals.json index 6655b2fe..63b4309f 100644 --- a/.claude/skills/pre-push-gate/evals/evals.json +++ b/.claude/skills/pre-push-gate/evals/evals.json @@ -4,13 +4,21 @@ "expect": "pre-push-gate" }, { - "prompt": "The Storybook stage of the gate fails in my worktree even though the tests pass. What is wrong?", + "prompt": "The gate's Storybook stage is failing for me. How do I diagnose it?", "expect": "pre-push-gate" }, { "prompt": "Everything in the gate started timing out at once. What causes that?", "expect": "pre-push-gate" }, + { + "prompt": "How do I check whether a stage of the gate really passed?", + "expect": "pre-push-gate" + }, + { + "prompt": "The gate failed at the lint stage. How do I work out what is wrong?", + "expect": "pre-push-gate" + }, { "prompt": "What is 17 multiplied by 23?", "expect": null diff --git a/.claude/skills/project-structure/evals/evals.json b/.claude/skills/project-structure/evals/evals.json index c690949a..e8bfc42b 100644 --- a/.claude/skills/project-structure/evals/evals.json +++ b/.claude/skills/project-structure/evals/evals.json @@ -4,11 +4,19 @@ "expect": "project-structure" }, { - "prompt": "I'm adding a helper both the CLI and the web client need. Which directory does it belong in?", + "prompt": "Does a new Node-only backend piece for the web client belong in core or in the web client itself?", "expect": "project-structure" }, { - "prompt": "Which package owns the OAuth secret storage backends?", + "prompt": "Does browser-only transport code belong in core or in the web client?", + "expect": "project-structure" + }, + { + "prompt": "I have a new file and I'm not sure where it goes. How do I decide in this repo?", + "expect": "project-structure" + }, + { + "prompt": "Does a helper that only the CLI and TUI use belong in core or in one of the clients?", "expect": "project-structure" }, { diff --git a/.claude/skills/test-servers/evals/evals.json b/.claude/skills/test-servers/evals/evals.json index 25ec237f..ba117347 100644 --- a/.claude/skills/test-servers/evals/evals.json +++ b/.claude/skills/test-servers/evals/evals.json @@ -11,6 +11,14 @@ "prompt": "I renamed a file under test-servers/src and the fixture is still running the old code. Why?", "expect": "test-servers" }, + { + "prompt": "Which protocol era should I connect with when I run one of these fixtures by hand?", + "expect": "test-servers" + }, + { + "prompt": "I need a fixture combination that doesn't exist yet. How do I add one?", + "expect": "test-servers" + }, { "prompt": "Sort this list alphabetically: banana, apple, cherry.", "expect": null diff --git a/.claude/skills/testing/evals/evals.json b/.claude/skills/testing/evals/evals.json index 5d7e1216..2271ebfb 100644 --- a/.claude/skills/testing/evals/evals.json +++ b/.claude/skills/testing/evals/evals.json @@ -1,16 +1,24 @@ [ - { - "prompt": "I just added clients/tui/__tests__/serverList.test.ts. What command runs only the TUI suite?", - "expect": "testing" - }, - { - "prompt": "In clients/web/src/components/Foo.test.tsx I need to assert on a Mantine transition mid-flight. How?", - "expect": "testing" - }, { "prompt": "Which test tier in this repo spawns the built binary rather than importing it?", "expect": "testing" }, + { + "prompt": "How do I run just the TUI suite, and is there one command that runs everything?", + "expect": "testing" + }, + { + "prompt": "How do I write a test for a TUI component in this repo?", + "expect": "testing" + }, + { + "prompt": "How am I supposed to invoke the CLI from a new test in this repo?", + "expect": "testing" + }, + { + "prompt": "How do I run the web integration tests, and do they run as part of the normal test command?", + "expect": "testing" + }, { "prompt": "What does this regex do? /^\\d{3}-\\d{4}$/", "expect": null diff --git a/AGENTS.md b/AGENTS.md index 1c6bf976..252c1e39 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -192,6 +192,9 @@ that from happening: a skill therefore has a cost paid by the *existing* ones, which only `skills:eval` can see. **Re-run the full eval after any flip _or description edit_**, not just the changed skill's own cases. + **How to write a description that fires, and cases that measure it, is + [`docs/skill-authoring.md`](./docs/skill-authoring.md)** — the case shapes + that work, the ones that can never pass, and the probe-then-measure loop. The lever that works is the description's *shape*. Leading with the actions and then enumerating concrete situations ("Use when … ; when … ; when …") beats a noun-phrase list of contents: it took `pre-push-gate` from 3/5 to 5/5 @@ -230,7 +233,7 @@ that from happening: overflows, and drops the least-invoked entries **first** — which are exactly the model-invoked skills that must fire on their own. `verify:skills` prints the current cost against the budget recorded in `scripts/lib/skill-manifest.mjs` - (2,834/4,000 characters as of this writing) and fails when it is exceeded. Raise + (2,921/4,000 characters as of this writing) and fails when it is exceeded. Raise the budget deliberately, or tighten a description; each entry is capped at 1,536 characters regardless, so **put the key use case first**. diff --git a/README.md b/README.md index aaaffc97..4593d217 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,7 @@ Each client has its own README with client-specific detail: | --- | --- | | [Architecture](./docs/architecture.md) | The `@inspector/core` shared package, and the web client's "dumb components" + Storybook approach | | [Testing and the quality gate](./docs/quality-gate.md) | What each `validate` / `coverage` / `smoke` / `verify:*` script covers, the GitHub-CI-vs-local-gate split, and the supported browsers | +| [Writing a skill](./docs/skill-authoring.md) | How to write a skill description that actually fires, and eval cases that measure it — the case shapes that work, and the tuning loop | | [Test servers](./docs/test-servers.md) | The composable test servers and the showcase config for every feature — what to run, what to click, and what the broken build did | | [Publishing](./docs/publishing.md) | What ships in the tarball, the packaging invariants, and `pack:verify` | | [Docker](./docs/docker.md) | Running the container image — ports, volumes, and where secrets go | diff --git a/docs/skill-authoring.md b/docs/skill-authoring.md new file mode 100644 index 00000000..113ebdc0 --- /dev/null +++ b/docs/skill-authoring.md @@ -0,0 +1,181 @@ +# Writing a skill and its eval cases + +The **rules** for skill maintenance — the four numbered requirements, the +invocation-mode policy, the listing budget — are in +[`AGENTS.md`](../AGENTS.md#maintaining-the-skills). This page is the _how_: what +makes a description fire, what makes an eval case measure the right thing, and +the loop for tuning both. It exists because `verify:skills` can prove a skill is +well-formed but cannot prove it is ever _reachable_, and the tool that measures +reachability — `npm run skills:eval` — reports a hit rate rather than a verdict. + +## How the eval actually measures + +`scripts/skill-eval.mjs` runs every committed case in a **fresh headless +session** (`claude -p`), `RUNS` times, and scores the fraction of runs in which +the `Skill` tool fired with the expected name. + +Four properties of that harness drive everything below: + +- **`--max-turns 1`.** The skill must be the model's **first** tool call. If it + opens a file first — even on its way to a perfect answer — the sample is a + miss. This is the single most important fact about writing cases. +- **The session is fresh, but not empty.** `CLAUDE.md` → `AGENTS.md` is loaded + in full on every turn, so anything the rules file already answers gets + answered without a skill. +- **Negative cases are scored against every model-invoked skill in this repo**, + not just the one whose file they live in. A negative passes only if _none_ of + ours fired. +- **Positive cases may only expect their own skill.** `verify:skills` rejects a + foreign name, because a case that passes when a _different_ skill fires + reports a measurement of something it does not describe. + +## Writing a description + +Lead with the **actions**, then enumerate concrete **situations**: + +``` +description: . Use when ; +when ; when ; or when . +``` + +This shape is what the measurements keep rewarding. Reshaping `pre-push-gate` +into it took it from 3/5 to 5/5, and `testing`'s three cases from 25/50/25% to +100% each. A noun-phrase list of contents ("Where things live, what is in core/, +and what each directory is for") consistently underperforms it. + +Three further rules, each learned the expensive way: + +- **Put the key use case first.** Each listing entry is capped at 1,536 + characters and the whole listing at 4,000; entries are dropped least-invoked + first when it overflows. +- **Adding a skill costs the skills already there.** Going from three + model-invoked skills to nine measurably lowered the trigger rate of the + existing ones — the ceiling is attention, not characters. So **re-run the full + suite after any flip or description edit**, never just the changed skill's + cases. +- **`paths` is not a free win.** It halved `testing`'s conversational trigger + rate, and a prompt-only eval can never exercise a path trigger, so shipping it + means shipping an untestable claim. + +## Writing an eval case + +### The first-move rule + +A case measures the description only if invoking a skill is the model's natural +**first** move. Ask **"how do I …"**, not **"how does this work"** — the second +invites a `Read` or a `Grep`, which spends the only turn the harness allows. + +Measured, one probe each, on this repo: + +| Prompt | First tool call | +| ---------------------------------------------------------------------------------------- | --------------- | +| "How do TUI components get mounted and keypresses driven in this repo's tests?" | `Bash` | +| "How do I write a test for a TUI component in this repo?" | `Skill` ✅ | +| "Which part of this repo owns the browser's HTTP transport and the backend it talks to?" | `Bash` | +| "Does browser-only transport code belong in core or in the web client?" | `Skill` ✅ | +| "Which parts of the gate only ever run locally and never in CI?" | `Read` | +| "The gate failed at the lint stage. How do I work out what is wrong?" | `Skill` ✅ | + +Shapes that reliably fire: + +- **"How do I …"** — a procedure, not an explanation. +- **"Does X belong in A or B?"** — a decision the repo has a convention for. +- **"\ failed. How do I diagnose it?"** — a symptom with a documented cause. +- **"Which command runs …?"** — when the answer is a repo-specific script. +- **A bare imperative** — "Triage the new issues.", "Move the card for issue 2189 + to In Review." + +Shapes that reliably do not: + +- **"How does X work here?"** / **"Where is X implemented?"** — an invitation to + read the source, which is the _better_ answer and scores as a miss. +- **Anything naming a concrete file, symbol, or helper.** Naming the mechanism + hands the model a grep target and removes the reason to load a procedure. + +### Do not write a case `AGENTS.md` already answers + +`AGENTS.md` is resident on every turn, so a prompt it answers is answered +without the skill and the case reads as a permanent miss. Two that had to be +retired here: + +| Retired case | Why it could never pass | +| ------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | +| "Which package owns the OAuth secret storage backends?" | The directory tree in `AGENTS.md` says `auth/ — OAuth end to end + the per-server SecretStore backends`. Scored 0% across two full runs. | +| "In `…/Foo.test.tsx` I need to assert on a Mantine transition mid-flight. How?" | `AGENTS.md` names `renderWithMantineTransitions` and states the rule outright. Also named a concrete file — two faults at once. | +| "What should I run before I push?" | `AGENTS.md` states `npm run local:gate` is the mandatory pre-push command. Scored 40%. | + +The test to apply before committing a case: **could a reader answer this from +the rules file alone?** If yes, it measures `AGENTS.md`, not the skill. Aim the +case at what only the skill body holds — a live ID, a command sequence, a +recovery recipe, a diagnosis. + +### When a neighbouring skill fires, the case is wrong + +A positive case may only expect its own skill, so a prompt that a _different_ +skill legitimately owns can never pass — and rewriting the description to win it +would be the wrong fix, because it would broaden a skill onto another's ground. +"How do I get an issue I already filed onto the project board with the right +fields?" fired `board-ops` 3/3 as an `issue-create` case. `board-ops` is the +right answer: adding a card and setting its fields is a board operation. The +case was replaced, not the description. + +Before blaming a description for a miss, check **which** skill fired. If it is a +neighbour and the neighbour is right, the case is aimed at the wrong skill. + +### Negative cases + +Every skill file needs at least one `"expect": null` case; a skill that fires on +everything is a context regression nobody notices by hand. Keep them plainly +unrelated to the repo (arithmetic, trivia, a one-line refactor). All 18 in this +repo have held at 100% through every reshaping so far — if one starts firing, a +description has grown too broad. + +## The tuning loop + +**Probe first, then measure.** A full suite run is ~63 cases × `RUNS` sessions +of metered model calls; a single probe is one. Iterate with probes until a +prompt fires at all, and only then spend a full run on its rate: + +```sh +# One sample, printing which tool actually fired first. +print -r -- "" \ + | claude -p --output-format stream-json --verbose --max-turns 1 --allowedTools "Skill" \ + | grep -o '"name":"[^"]*"' | head -3 +``` + +**Probe a marginal case more than once.** A prompt that fires on a single probe +can still measure 60% over five runs — one sample cannot distinguish "reliable" +from "coin flip". Three probes is enough to tell a solid replacement from a +lucky one, and it is still far cheaper than a full suite run. + +Then the suite: + +```sh +npm run skills:eval # every model-invoked skill, RUNS=3 +RUNS=5 CONCURRENCY=6 npm run skills:eval +npm run skills:eval -- testing # one skill's cases (still scores negatives against all) +``` + +**Read the noise before reading the result.** At `RUNS=3` one sample is worth 33 +points, and an untouched skill was observed swinging 100 points across two runs +of the same suite. `RUNS=5` is the smallest setting where a single flake does +not cross the 0.8 threshold on its own. A one-case move is not evidence; a skill +whose cases move _together_ is. + +This is why the eval is **not** in `validate`, `local:gate`, or CI: it spends +metered calls, it is non-deterministic by construction, and it goes red on a +rate limit. A case below threshold is a signal to investigate, not a build +break. + +## Checklist for a new or edited skill + +1. Frontmatter opens on line 1 (no BOM, no blank line), YAML is valid — an + unquoted colon in a description loads the body with an _empty_ description, + so `/name` still works while the skill can never auto-fire again. +2. `disable-model-invocation` is declared explicitly; default it to `false`. +3. Description is action-first, with `Use when …; when …; when …`. +4. `evals/evals.json` has both positives and at least one negative, every + positive shaped by the first-move rule and none answerable from `AGENTS.md`. +5. `npm run verify:skills` passes and the listing is under budget. +6. `RUNS=5 npm run skills:eval` — the **whole** suite — is ≥80% on every case, + including the skills you did not touch.