Merge pull request #2203 from modelcontextprotocol/v2/docs/2202-skill-eval-tuning
docs: tune skill eval cases and document how to write them
This commit is contained in:
commit
efb02650e7
21 changed files with 916 additions and 43 deletions
|
|
@ -1,6 +1,6 @@
|
|||
---
|
||||
name: board-ops
|
||||
description: gh recipes for the Inspector project boards — add a card, move its Status, set or re-score its Priority, delete a card, and recover from a deleted single-select option. Covers board #28 (v2) and board #11 (v1), their node/field/option IDs, and the option-deletion hazard.
|
||||
description: "gh recipes for the Inspector project boards — add a card, move its Status, set or re-score its Priority, delete a card, and recover from a deleted single-select option. Covers board #28 (v2) and board #11 (v1), their node/field/option IDs, and the option-deletion hazard."
|
||||
disable-model-invocation: false
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -1,10 +1,10 @@
|
|||
[
|
||||
{
|
||||
"prompt": "How do I get the web client running locally?",
|
||||
"prompt": "What is the difference between the launcher-driven run scripts and starting a client directly, and when should I use each?",
|
||||
"expect": "local-dev"
|
||||
},
|
||||
{
|
||||
"prompt": "I added a dependency and the TUI bundle broke at import time. Where should it have gone?",
|
||||
"prompt": "I added a package and now one of the client bundles fails at run time. What do I check?",
|
||||
"expect": "local-dev"
|
||||
},
|
||||
{
|
||||
|
|
@ -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": "Do I need to do anything special to set this repo up inside a git worktree?",
|
||||
"expect": "local-dev"
|
||||
},
|
||||
{
|
||||
"prompt": "Write a haiku about autumn.",
|
||||
"expect": null
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -1,14 +1,22 @@
|
|||
[
|
||||
{
|
||||
"prompt": "What lives in clients/web versus @inspector/core?",
|
||||
"prompt": "Something in the web client's Node backend needs changing but I do not know which piece handles what. How do I work that out?",
|
||||
"expect": "project-structure"
|
||||
},
|
||||
{
|
||||
"prompt": "I'm adding a helper both the CLI and the web client need. Which directory does it belong in?",
|
||||
"prompt": "I am adding a module that both the browser side and the Node side need to run. How do I decide where it goes in this repo?",
|
||||
"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": "I need to find where a feature is implemented but I do not know which client owns it. How do I work that out in this repo?",
|
||||
"expect": "project-structure"
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
---
|
||||
name: test-servers
|
||||
description: Run a composable MCP test server by hand — pick the showcase config that reproduces a given feature or bug, build test-servers, and connect with the right protocol era. Covers the stale-build hazard.
|
||||
description: Run a composable MCP test server by hand — pick the showcase config for a feature or bug, build it, and connect with the right protocol era. Use when a change, a PR or a smoke test needs a real server to exercise it; when reproducing a reported bug by hand; when choosing which fixture or protocol era to run; when a fixture keeps serving stale code after an edit; or when the config or preset you need does not exist yet.
|
||||
disable-model-invocation: false
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -8,7 +8,15 @@
|
|||
"expect": "test-servers"
|
||||
},
|
||||
{
|
||||
"prompt": "I renamed a file under test-servers/src and the fixture is still running the old code. Why?",
|
||||
"prompt": "I need to run a real MCP server locally to exercise this change by hand. Which one do I run, and how do I start it?",
|
||||
"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"
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -11,6 +11,12 @@ statements, functions, and branches.** That rule and the React/Mantine
|
|||
conventions live in [`AGENTS.md`](../../../AGENTS.md); this skill is where a
|
||||
test goes, how to run it, and how to clear the gate.
|
||||
|
||||
⚠️ **Anything that needs a real server to run against — an integration test, a
|
||||
smoke, reproducing a bug by hand — is `/test-servers`, and you have to load it.**
|
||||
Integration and smoke tests here drive a real server over a real transport
|
||||
rather than a mock, so picking, building and connecting to a fixture is a
|
||||
procedure of its own that this skill does not carry.
|
||||
|
||||
## Where the test file goes
|
||||
|
||||
**Side-by-side by default; `src/test/` only for what can't be co-located; and
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
24
AGENTS.md
24
AGENTS.md
|
|
@ -154,7 +154,14 @@ that from happening:
|
|||
Code does and fails on anything that would strip the metadata — most importantly
|
||||
**malformed YAML**, which loads the body with an *empty* description, so
|
||||
`/skill-name` still works and a manual spot check passes while the skill can
|
||||
never auto-fire again. An unquoted colon in a description is enough. It also
|
||||
never auto-fire again. An unquoted colon in a description is enough — and so
|
||||
is an unquoted **`#`**, which YAML reads as a comment and which truncates the
|
||||
description *silently* from that point on rather than emptying it. `board-ops`
|
||||
shipped that way: `Covers board #28 (v2) and board #11 (v1), their
|
||||
node/field/option IDs, and the option-deletion hazard` was cut at `#28`, so 90
|
||||
characters — including both board numbers — were absent from the listing while
|
||||
every check stayed green, because a truncated description is still a non-empty
|
||||
one. **Quote any description containing `#` or `:`.** It also
|
||||
runs `claude plugin validate` — the authoritative schema — when the installed
|
||||
CLI is **exactly** the pinned version, and otherwise says so and moves on.
|
||||
That best-effort hand-off keeps `validate` fast and offline, but it also
|
||||
|
|
@ -180,7 +187,7 @@ that from happening:
|
|||
skill at all: it is absent from the listing and the Skill tool refuses it. The
|
||||
costs are asymmetric — a spurious load costs ~250 characters, a missed one
|
||||
costs a wrong base branch or an unsigned commit — and the budget is not tight
|
||||
(nine of the ten are model-invoked today and total ~2.8k of 4k). Reserve
|
||||
(nine of the ten are model-invoked today and total ~3.2k of 4k). Reserve
|
||||
`true` for a procedure that is genuinely only ever started deliberately —
|
||||
`release` is the only one left, because nobody cuts a release by implication.
|
||||
⚠️ **A `true` skill cannot be reached by another skill either.** If a
|
||||
|
|
@ -191,10 +198,14 @@ that from happening:
|
|||
of the ones already there: `project-structure` fell from 100% to 0% on two
|
||||
cases (n=4) and `testing` from 3/5 to 2/5, while the six new skills all
|
||||
measured 100% and every negative case stayed clean. So the ceiling is
|
||||
attention, not characters — we were at 2.8k of a 4k budget throughout. Adding
|
||||
attention, not characters — we were at 2.8k of a 4k budget throughout *that
|
||||
experiment* (it is ~3.2k now; the point is that nothing was near the cap). Adding
|
||||
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
|
||||
|
|
@ -212,7 +223,10 @@ that from happening:
|
|||
`evals/evals.json` — positives **and negatives**. Seeing a skill fire once tells
|
||||
you Claude found it, not that it finds it reliably, and a skill that fires on
|
||||
everything is a context regression that nobody notices by hand. `verify:skills`
|
||||
requires both kinds; `npm run skills:eval` actually runs them (it needs the
|
||||
requires both kinds — and at least **five** positives, for breadth rather than
|
||||
variance: each prompt is scored on its own `passes / RUNS`, so more prompts
|
||||
steady nothing, they cover more of the ways someone might reach the skill and
|
||||
expose a description that only fires on one narrow phrasing; `npm run skills:eval` actually runs them (it needs the
|
||||
`claude` CLI and real model calls, so it is deliberately **not** in the gate —
|
||||
run it when adding a skill or editing a model-invoked description).
|
||||
Two things learned writing the first set, both of which make a case measure
|
||||
|
|
@ -233,7 +247,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
|
||||
(3,234/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**.
|
||||
|
||||
|
|
|
|||
|
|
@ -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 |
|
||||
|
|
|
|||
262
docs/skill-authoring.md
Normal file
262
docs/skill-authoring.md
Normal file
|
|
@ -0,0 +1,262 @@
|
|||
# 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 fire in the model's **first assistant
|
||||
turn**. Ordering _within_ that turn does not matter — `collectSkillInvocations`
|
||||
scans every `tool_use` block in the event, so a turn that calls `Read` and
|
||||
`Skill` together still scores a hit. What loses the sample is needing a tool
|
||||
*result* first: read a file, look at what came back, then decide — that
|
||||
decision lands in turn two, which never runs. So a prompt that invites the
|
||||
model to go look at something before answering is a miss even when its
|
||||
eventual answer would have been perfect. 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: <verb phrase saying what this does>. Use when <situation>;
|
||||
when <situation>; when <situation>; or when <situation>.
|
||||
```
|
||||
|
||||
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` ✅ |
|
||||
|
||||
⚠️ Those rows come from an **unrestricted** probe, run before the snippet below
|
||||
was aligned with the harness. The eval itself forbids `Bash` (and `Write`,
|
||||
`Edit`, `NotebookEdit`), so a scored case cannot actually reach for one — what
|
||||
the rows show is the *pull*: given a free choice, these prompts send the model
|
||||
to read the repo rather than to load a skill, and under the harness's flags that
|
||||
same pull simply produces no `Skill` call at all. Read them as a diagnosis of
|
||||
why a prompt scores badly, never as a claim about which tools the eval offers.
|
||||
|
||||
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.
|
||||
- **"\<Stage\> 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%. |
|
||||
|
||||
**Partial overlap reads as flakiness, not as a miss.** A case the rules file
|
||||
answers _outright_ sits at 0% and is easy to spot. A case it answers _half_ of
|
||||
oscillates — and looks like eval noise rather than a defect in the case. Two of
|
||||
`local-dev`'s cases behaved exactly this way before being retired:
|
||||
|
||||
| Case | Scores across runs | The overlap |
|
||||
| ---------------------------------------------------------------------------------------------- | ------------------------- | ---------------------------------------------------------------------------------------- |
|
||||
| "I added a dependency and the TUI bundle broke at import time. **Where should it have gone?**" | 33 / 67 / 100 / 100 / 60% | Dependency placement is stated in full in `AGENTS.md`; only the diagnosis is the skill's |
|
||||
| "I just pulled and a client's dependencies look out of sync. **What do I need to run?**" | 80 / 80 / 60% | `AGENTS.md` says a single root `npm install` is all you need |
|
||||
|
||||
⚠️ **A high score does not clear a case of overlap, and this is the eval's real
|
||||
blind spot.** The two rules above — outright overlap pins a case near 0%, partial
|
||||
overlap makes it oscillate — describe what the *scores* look like once you already
|
||||
suspect a case. They are not a detector. Across code review of this PR, three
|
||||
committed cases were identified as `AGENTS.md`-answerable by *reading* them, and
|
||||
all three were scoring **80–100%** at the time:
|
||||
|
||||
| Case | Score when flagged |
|
||||
| --- | --- |
|
||||
| `project-structure`: "Does a new Node-only backend piece for the web client belong in core…?" | 80 → 100% |
|
||||
| `project-structure`: "Does a helper that only the CLI and TUI use belong in core…?" | 80% |
|
||||
| `issue-create`: "What do I need to do beyond `gh issue create`…?" | 100 / 100 / 100% |
|
||||
|
||||
The first two were re-aimed; the third was kept, because a case that holds at
|
||||
100% across three independent suites is measuring *something* the rules file does
|
||||
not supply on its own. But the lesson stands either way: **the eval cannot tell
|
||||
you a passing case is well-aimed.** It can only tell you a case is failing. Read
|
||||
every new case against `AGENTS.md` by hand before committing it, and do not let a
|
||||
green suite substitute for that.
|
||||
|
||||
Both stabilised at 100% once re-aimed at what only the skill holds — "what do I
|
||||
**check**" instead of "where should it have **gone**", and the worktree install
|
||||
trap instead of the install command. If a case keeps moving between runs while
|
||||
its neighbours hold steady, suspect the case before you suspect the noise.
|
||||
|
||||
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 **model-invoked** skill's eval file needs at least one `"expect": null`
|
||||
case; a skill that fires on everything is a context regression nobody notices by
|
||||
hand. (A name-only skill has no eval file to put one in — see the checklist.) 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. The flags are the
|
||||
# harness's own (see `runPrompt` in `scripts/skill-eval.mjs`), so a probe and a
|
||||
# scored case see the same tool policy — a probe that may call a tool the eval
|
||||
# forbids predicts nothing. `printf` rather than zsh's `print -r`, so the
|
||||
# snippet also runs under bash.
|
||||
printf '%s' "<prompt>" \
|
||||
| claude -p --output-format stream-json --verbose --max-turns 1 \
|
||||
--disallowedTools Bash,Write,Edit,NotebookEdit \
|
||||
| jq -r 'select(.message.content?) | .message.content[]?
|
||||
| select(.type == "tool_use") | .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
|
||||
npm run skills:eval -- testing test-servers # a set of skills
|
||||
```
|
||||
|
||||
Narrowing the run never narrows what a **negative** case is scored against — a
|
||||
focused run still fails a negative that fires any model-invoked skill in the
|
||||
repo, because "no skill of ours fired" is the property being asserted. An
|
||||
unknown name is a hard error rather than an empty run, since a typo would
|
||||
otherwise report a green 0/0 that reads exactly like a clean pass.
|
||||
|
||||
⚠️ **A focused run cannot see displacement.** Adding or reshaping a description
|
||||
changes the trigger rate of skills you did not touch, and only the full suite
|
||||
observes that. Use the filter to iterate; take the full suite before you push.
|
||||
|
||||
**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, and any
|
||||
description containing `:` or `#` is **quoted**. An unquoted colon loads the
|
||||
body with an _empty_ description, so `/name` still works while the skill can
|
||||
never auto-fire again; an unquoted `#` is worse-behaved, truncating the
|
||||
description from that point on so it stays *non-empty* — which is why it went
|
||||
unnoticed until `board-ops` lost both its board numbers. `verify:skills` now
|
||||
rejects that truncation, so the gate catches it; quote the value rather than
|
||||
relying on the check to tell you.
|
||||
2. `disable-model-invocation` is declared explicitly; default it to `false`.
|
||||
3. Description is action-first, with `Use when …; when …; when …`.
|
||||
4. **If the skill is model-invoked**, `evals/evals.json` carries **at least
|
||||
five positives and at least one negative** — every positive shaped by the
|
||||
first-move rule, none answerable from `AGENTS.md`. A name-only skill
|
||||
(`disable-model-invocation: true`, as `release` is) has no eval file at all:
|
||||
nothing can measure a trigger that only a human types, so an eval there would
|
||||
be unrunnable rather than merely redundant. `verify:skills` enforces the
|
||||
floor — fewer than five positives fails the gate. The reason is **breadth,
|
||||
not variance**: each prompt is scored on its own `passes / RUNS`, so extra
|
||||
prompts do not steady any rate (`RUNS` is the knob for that). Five prompts
|
||||
cover five ways someone might arrive at the skill, which is what catches a
|
||||
description that fires on one narrow phrasing and nothing else.
|
||||
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.
|
||||
|
|
@ -11,7 +11,7 @@
|
|||
// So every check here is deny-by-default: a file we cannot read the way Claude
|
||||
// Code reads it is an error, not a skip.
|
||||
|
||||
import { parse as parseYaml } from "yaml";
|
||||
import { parseDocument } from "yaml";
|
||||
|
||||
/**
|
||||
* Per-entry cap Claude Code applies to a skill listing entry. It covers
|
||||
|
|
@ -84,9 +84,15 @@ export function parseSkill(dirName, text) {
|
|||
const { frontmatter, error } = splitFrontmatter(text);
|
||||
if (error) return { errors: [error] };
|
||||
|
||||
let doc;
|
||||
let meta;
|
||||
try {
|
||||
meta = parseYaml(frontmatter);
|
||||
// `parseDocument` rather than `parse`: the truncation guard below needs the
|
||||
// scalar NODES, not just their values, and parsing once keeps the two views
|
||||
// of the frontmatter from ever disagreeing.
|
||||
doc = parseDocument(frontmatter);
|
||||
if (doc.errors.length > 0) throw new Error(doc.errors[0].message);
|
||||
meta = doc.toJS();
|
||||
} catch (e) {
|
||||
// The whole point of the guard: this is what strips the description.
|
||||
return {
|
||||
|
|
@ -150,6 +156,46 @@ export function parseSkill(dirName, text) {
|
|||
errors.push("`paths` must be a list of glob strings");
|
||||
}
|
||||
}
|
||||
// An unquoted `#` is a YAML comment, so a description containing one is
|
||||
// truncated at that point — silently, and *not* to empty, which is why the
|
||||
// "malformed YAML" checks sail past it: what survives still parses as a
|
||||
// non-empty string. `board-ops` shipped for a while with its two board numbers
|
||||
// and the option-deletion hazard cut off this way.
|
||||
//
|
||||
// Ask the PARSER what it discarded rather than re-deriving it from the raw
|
||||
// text. `yaml` hangs the comment it stripped on the scalar node itself, so a
|
||||
// plain scalar with a `.comment` is exactly the truncation case, and a quoted
|
||||
// or block scalar never has one. Three rounds of regex here each fixed one
|
||||
// shape and missed the next — a tag or anchor before the value, a scalar
|
||||
// continued onto indented lines, then a continuation across a blank line, and
|
||||
// key forms (`"description":`, `description :`) that populate the mapping
|
||||
// while matching no `^description:` line at all (Copilot). The node carries
|
||||
// all of them for free, so none of that scanning survives.
|
||||
for (const field of ["description", "when_to_use"]) {
|
||||
const node = doc.get(field, true);
|
||||
// `typeof node.value !== "string"` also covers the non-scalar cases: a map
|
||||
// or list node has no `.value`, and its own checks above already report it.
|
||||
if (!node || typeof node.value !== "string") continue;
|
||||
// Only on a PLAIN scalar is a comment evidence of loss. A quoted or block
|
||||
// scalar can carry a perfectly legitimate trailing one — `description:
|
||||
// "Complete value" # note` keeps its whole value — and flagging that told
|
||||
// the author to quote a value they had already quoted (Copilot). The regex
|
||||
// this replaced tested the same thing by reading the first character; the
|
||||
// node states it outright.
|
||||
if (node.type !== "PLAIN") continue;
|
||||
const dropped = node.comment;
|
||||
if (typeof dropped !== "string") continue;
|
||||
errors.push(
|
||||
"`" +
|
||||
field +
|
||||
"` is truncated by an unquoted `#` — YAML kept `" +
|
||||
node.value +
|
||||
"` and dropped `#" +
|
||||
dropped +
|
||||
"`; quote the value",
|
||||
);
|
||||
}
|
||||
|
||||
// A skill nothing can reach is dead weight that still costs a directory.
|
||||
if (!modelInvoked && meta["user-invocable"] === false) {
|
||||
errors.push(
|
||||
|
|
@ -192,6 +238,18 @@ export function listingCost(skills) {
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* The number of positive cases a model-invoked skill's eval file must carry.
|
||||
*
|
||||
* This is a **breadth** floor, not a statistical one. The evaluator scores each
|
||||
* prompt independently — `passes / RUNS` for that prompt alone — so adding
|
||||
* prompts does not steady any other prompt's rate; `RUNS` is the only knob that
|
||||
* does. What five prompts buy is coverage of five different ways someone might
|
||||
* arrive at the skill, which is what exposes a description that fires on one
|
||||
* narrow phrasing and nothing else (Copilot).
|
||||
*/
|
||||
export const MIN_POSITIVE_CASES = 5;
|
||||
|
||||
/**
|
||||
* Validate an `evals/evals.json` payload for a model-invoked skill.
|
||||
*
|
||||
|
|
@ -232,10 +290,32 @@ export function validateEvalCases(skillName, cases) {
|
|||
);
|
||||
}
|
||||
});
|
||||
const positives = cases.filter((c) => c && c.expect === skillName).length;
|
||||
const positiveCases = cases.filter((c) => c && c.expect === skillName);
|
||||
const positives = positiveCases.length;
|
||||
// The floor exists for breadth, so it has to count *phrasings*, not entries:
|
||||
// five copies of one prompt clear an entry count while exercising exactly the
|
||||
// one trigger the floor is meant to look past (Copilot). Compare on collapsed,
|
||||
// case-folded text so trivial edits do not read as a new situation.
|
||||
const distinctPositives = new Set(
|
||||
positiveCases.map((c) =>
|
||||
String(c.prompt ?? "")
|
||||
.toLowerCase()
|
||||
.replace(/\s+/g, " ")
|
||||
.trim(),
|
||||
),
|
||||
).size;
|
||||
const negatives = cases.filter((c) => c && c.expect === null).length;
|
||||
if (positives === 0) {
|
||||
errors.push(`no positive case expects \`${skillName}\``);
|
||||
} else if (distinctPositives < MIN_POSITIVE_CASES) {
|
||||
// A non-zero floor is not enough: one prompt measures one phrasing. Since
|
||||
// each prompt is scored on its own `passes / RUNS`, extra prompts do not
|
||||
// make any single rate steadier — they cover more of the ways someone might
|
||||
// reach the skill, so a description that only fires on one narrow shape is
|
||||
// visible instead of passing on its single lucky case (Copilot).
|
||||
errors.push(
|
||||
`only ${distinctPositives} distinct positive prompt(s)${positives === distinctPositives ? "" : ` across ${positives} case(s)`}; a model-invoked skill needs at least ${MIN_POSITIVE_CASES} to cover the range of situations it should fire on`,
|
||||
);
|
||||
}
|
||||
if (negatives === 0) {
|
||||
errors.push(
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import assert from "node:assert/strict";
|
|||
import {
|
||||
splitFrontmatter,
|
||||
parseSkill,
|
||||
MIN_POSITIVE_CASES,
|
||||
validateEvalCases,
|
||||
listingCost,
|
||||
parseClaudeVersion,
|
||||
|
|
@ -175,15 +176,18 @@ test("listingCost counts only the skills that occupy the listing", () => {
|
|||
});
|
||||
|
||||
test("validateEvalCases requires a positive and a negative", () => {
|
||||
const positives = (n, expect = "testing") =>
|
||||
Array.from({ length: n }, (_, i) => ({ prompt: `p${i}`, expect }));
|
||||
|
||||
assert.deepEqual(
|
||||
validateEvalCases("testing", [
|
||||
{ prompt: "a", expect: "testing" },
|
||||
...positives(MIN_POSITIVE_CASES),
|
||||
{ prompt: "b", expect: null },
|
||||
]),
|
||||
[],
|
||||
);
|
||||
assert.match(
|
||||
validateEvalCases("testing", [{ prompt: "a", expect: "testing" }]).join(),
|
||||
validateEvalCases("testing", positives(MIN_POSITIVE_CASES)).join(),
|
||||
/no negative case/,
|
||||
);
|
||||
assert.match(
|
||||
|
|
@ -335,7 +339,10 @@ test("an eval file may only expect its own skill", () => {
|
|||
assert.match(
|
||||
validateEvalCases("testing", [
|
||||
{ prompt: "a", expect: "local-dev" },
|
||||
{ prompt: "b", expect: "testing" },
|
||||
...Array.from({ length: MIN_POSITIVE_CASES }, (_, i) => ({
|
||||
prompt: `p${i}`,
|
||||
expect: "testing",
|
||||
})),
|
||||
{ prompt: "c", expect: null },
|
||||
]).join(),
|
||||
/expects `local-dev`, but this file only measures `testing`/,
|
||||
|
|
@ -350,9 +357,351 @@ test("an eval file may only expect its own skill", () => {
|
|||
// Null and own-name cases are unaffected.
|
||||
assert.deepEqual(
|
||||
validateEvalCases("testing", [
|
||||
{ prompt: "a", expect: "testing" },
|
||||
...Array.from({ length: MIN_POSITIVE_CASES }, (_, i) => ({
|
||||
prompt: `p${i}`,
|
||||
expect: "testing",
|
||||
})),
|
||||
{ prompt: "b", expect: null },
|
||||
]),
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
test("an unquoted `#` truncates a description, and that is an error", () => {
|
||||
// The dangerous part is that YAML leaves a *non-empty* string behind, so the
|
||||
// "description is required and non-empty" check passes and the skill keeps
|
||||
// auto-firing — just with its most distinctive words missing from the
|
||||
// listing. `board-ops` shipped this way, losing both board numbers.
|
||||
const withHash = [
|
||||
"---",
|
||||
"name: board-ops",
|
||||
"description: Recipes. Covers board #28 (v2) and board #11 (v1).",
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
const parsed = parseSkill("board-ops", withHash);
|
||||
assert.equal(parsed.description, "Recipes. Covers board");
|
||||
assert.match(parsed.errors.join(" "), /truncated by an unquoted `#`/);
|
||||
|
||||
// Quoting is the fix, and it must come back clean.
|
||||
const quoted = withHash.replace(
|
||||
/^description: (.*)$/m,
|
||||
(_m, d) => `description: "${d}"`,
|
||||
);
|
||||
const ok = parseSkill("board-ops", quoted);
|
||||
assert.deepEqual(ok.errors, []);
|
||||
assert.equal(
|
||||
ok.description,
|
||||
"Recipes. Covers board #28 (v2) and board #11 (v1).",
|
||||
);
|
||||
});
|
||||
|
||||
test("a `#` inside a quoted description is not treated as truncation", () => {
|
||||
// Guard against the check firing on every legitimately quoted description.
|
||||
const quoted = [
|
||||
"---",
|
||||
"name: alpha",
|
||||
'description: "counts # and : safely"',
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
assert.deepEqual(parseSkill("alpha", quoted).errors, []);
|
||||
});
|
||||
|
||||
test("a model-invoked skill needs at least MIN_POSITIVE_CASES positives", () => {
|
||||
// `positives === 0` alone let a skill regress to a single prompt and still
|
||||
// pass the gate. The floor is about breadth, not variance: each prompt is
|
||||
// scored on its own `passes / RUNS`, so extra prompts steady nothing — they
|
||||
// cover more of the ways someone might reach the skill, so a description that
|
||||
// fires on one narrow phrasing cannot pass on that single case alone.
|
||||
const withPositives = (n) => [
|
||||
...Array.from({ length: n }, (_, i) => ({
|
||||
prompt: `p${i}`,
|
||||
expect: "alpha",
|
||||
})),
|
||||
{ prompt: "n", expect: null },
|
||||
];
|
||||
|
||||
assert.match(
|
||||
validateEvalCases("alpha", withPositives(1)).join(" "),
|
||||
/only 1 distinct positive prompt\(s\)/,
|
||||
);
|
||||
assert.match(
|
||||
validateEvalCases("alpha", withPositives(MIN_POSITIVE_CASES - 1)).join(" "),
|
||||
/needs at least 5 to cover the range/,
|
||||
);
|
||||
assert.deepEqual(
|
||||
validateEvalCases("alpha", withPositives(MIN_POSITIVE_CASES)),
|
||||
[],
|
||||
);
|
||||
|
||||
// Zero still reports the clearer "no positive case" message rather than the
|
||||
// count one, so the common mistake keeps its specific diagnosis.
|
||||
assert.match(
|
||||
validateEvalCases("alpha", [{ prompt: "n", expect: null }]).join(" "),
|
||||
/no positive case expects/,
|
||||
);
|
||||
});
|
||||
|
||||
test("the truncation guard covers `when_to_use`, not just `description`", () => {
|
||||
// The guard loops over both fields, so a regression that dropped `when_to_use`
|
||||
// from that list would be invisible to the description-only tests above.
|
||||
const withHash = [
|
||||
"---",
|
||||
"name: alpha",
|
||||
'description: "safe because quoted"',
|
||||
"when_to_use: Use when filing against board #28 or board #11.",
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
const parsed = parseSkill("alpha", withHash);
|
||||
assert.equal(parsed.whenToUse, "Use when filing against board");
|
||||
assert.match(parsed.errors.join(" "), /`when_to_use` is truncated/);
|
||||
// The description is quoted, so it must not be implicated.
|
||||
assert.doesNotMatch(parsed.errors.join(" "), /`description` is truncated/);
|
||||
|
||||
const quoted = withHash.replace(
|
||||
/^when_to_use: (.*)$/m,
|
||||
(_m, w) => `when_to_use: "${w}"`,
|
||||
);
|
||||
const ok = parseSkill("alpha", quoted);
|
||||
assert.deepEqual(ok.errors, []);
|
||||
assert.equal(ok.whenToUse, "Use when filing against board #28 or board #11.");
|
||||
});
|
||||
|
||||
test("the truncation guard spans a multiline plain scalar", () => {
|
||||
// A plain scalar continues onto more-indented lines, so a first-line-only
|
||||
// comparison sees `Covers boards` on both sides and reports no loss — passing
|
||||
// the exact truncation this guard exists to reject.
|
||||
const multiline = [
|
||||
"---",
|
||||
"name: alpha",
|
||||
"description: Covers boards",
|
||||
" #28 and #11 and the hazard.",
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
const parsed = parseSkill("alpha", multiline);
|
||||
assert.equal(parsed.description, "Covers boards");
|
||||
assert.match(parsed.errors.join(" "), /`description` is truncated/);
|
||||
|
||||
// A multiline plain scalar with no `#` folds to one line and must stay clean,
|
||||
// so the fix cannot be "reject anything multiline".
|
||||
const folded = [
|
||||
"---",
|
||||
"name: alpha",
|
||||
"description: Covers boards",
|
||||
" and the option-deletion hazard.",
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
const ok = parseSkill("alpha", folded);
|
||||
assert.deepEqual(ok.errors, []);
|
||||
assert.equal(ok.description, "Covers boards and the option-deletion hazard.");
|
||||
});
|
||||
|
||||
test("the truncation guard ignores anchors and tags, which are not `#`", () => {
|
||||
// YAML strips presentation syntax from the parsed value, so `&anchor` and
|
||||
// `!!tag` make the raw scalar legitimately longer than what it parses to.
|
||||
// A bare length comparison called those truncation and blamed a `#` that was
|
||||
// nowhere in the text.
|
||||
const mk = (value) =>
|
||||
[
|
||||
"---",
|
||||
"name: alpha",
|
||||
`description: ${value}`,
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
|
||||
const anchored = parseSkill("alpha", mk("© Some valid text."));
|
||||
assert.deepEqual(anchored.errors, []);
|
||||
assert.equal(anchored.description, "Some valid text.");
|
||||
|
||||
const tagged = parseSkill("alpha", mk("!!str Some tagged text."));
|
||||
assert.deepEqual(tagged.errors, []);
|
||||
|
||||
// The real case must still fail, so the gate is not simply disarmed.
|
||||
assert.match(
|
||||
parseSkill("alpha", mk("Covers board #28 (v2).")).errors.join(" "),
|
||||
/`description` is truncated/,
|
||||
);
|
||||
});
|
||||
|
||||
test("a quoted scalar stays safe behind a tag or an anchor", () => {
|
||||
// Properties may precede the value, in either order. Testing the first
|
||||
// character alone read these as unquoted and then rejected the `#` that the
|
||||
// quotes make harmless.
|
||||
const mk = (value) =>
|
||||
[
|
||||
"---",
|
||||
"name: alpha",
|
||||
`description: ${value}`,
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
|
||||
for (const value of [
|
||||
'© "Counts # safely"',
|
||||
'!!str "Counts # safely"',
|
||||
'!!str &a "Counts # safely"',
|
||||
]) {
|
||||
const parsed = parseSkill("alpha", mk(value));
|
||||
assert.deepEqual(parsed.errors, [], `rejected: ${value}`);
|
||||
assert.equal(parsed.description, "Counts # safely");
|
||||
}
|
||||
});
|
||||
|
||||
test("the positive floor counts distinct prompts, not entries", () => {
|
||||
// The floor exists for breadth, so repeating one prompt five times clears an
|
||||
// entry count while exercising exactly the single trigger it looks past.
|
||||
const duplicated = [
|
||||
...Array.from({ length: MIN_POSITIVE_CASES }, () => ({
|
||||
prompt: "the same prompt",
|
||||
expect: "alpha",
|
||||
})),
|
||||
{ prompt: "n", expect: null },
|
||||
];
|
||||
const errors = validateEvalCases("alpha", duplicated).join(" ");
|
||||
assert.match(
|
||||
errors,
|
||||
/only 1 distinct positive prompt\(s\) across 5 case\(s\)/,
|
||||
);
|
||||
|
||||
// Whitespace and casing are not a new situation either.
|
||||
const nearDuplicates = [
|
||||
{ prompt: "How do I do the thing?", expect: "alpha" },
|
||||
{ prompt: "how do i do the thing?", expect: "alpha" },
|
||||
{ prompt: "How do I do the thing? ", expect: "alpha" },
|
||||
{ prompt: "b", expect: "alpha" },
|
||||
{ prompt: "c", expect: "alpha" },
|
||||
{ prompt: "n", expect: null },
|
||||
];
|
||||
assert.match(
|
||||
validateEvalCases("alpha", nearDuplicates).join(" "),
|
||||
/only 3 distinct positive prompt\(s\)/,
|
||||
);
|
||||
|
||||
// Five genuinely different prompts pass.
|
||||
assert.deepEqual(
|
||||
validateEvalCases("alpha", [
|
||||
...Array.from({ length: MIN_POSITIVE_CASES }, (_, i) => ({
|
||||
prompt: `p${i}`,
|
||||
expect: "alpha",
|
||||
})),
|
||||
{ prompt: "n", expect: null },
|
||||
]),
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
test("a quoted or block scalar may carry a trailing comment", () => {
|
||||
// `yaml` attaches a legitimate trailing comment to these nodes too, so keying
|
||||
// on `.comment` alone rejected valid frontmatter and told the author to quote
|
||||
// a value that was already quoted. Only a PLAIN scalar can lose text to one.
|
||||
for (const value of ['"Complete value" # note', "'Complete value' # note"]) {
|
||||
const parsed = parseSkill(
|
||||
"alpha",
|
||||
[
|
||||
"---",
|
||||
"name: alpha",
|
||||
`description: ${value}`,
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n"),
|
||||
);
|
||||
assert.deepEqual(parsed.errors, [], value);
|
||||
assert.equal(parsed.description, "Complete value");
|
||||
}
|
||||
|
||||
const block = parseSkill(
|
||||
"alpha",
|
||||
[
|
||||
"---",
|
||||
"name: alpha",
|
||||
"description: | # note",
|
||||
" Complete value",
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n"),
|
||||
);
|
||||
assert.deepEqual(block.errors, []);
|
||||
});
|
||||
|
||||
test("the truncation guard covers noncanonical key forms", () => {
|
||||
// `"description":` and `description :` both populate the mapping, so a guard
|
||||
// that located the field by matching a `^description:` LINE skipped them
|
||||
// entirely and left `verify:skills` green on a truncated listing (Copilot).
|
||||
const mk = (key) =>
|
||||
[
|
||||
"---",
|
||||
"name: alpha",
|
||||
`${key} Covers board #28 and the hazard.`,
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
|
||||
for (const key of ['"description":', "'description':", "description :"]) {
|
||||
const parsed = parseSkill("alpha", mk(key));
|
||||
assert.equal(parsed.description, "Covers board", `key form: ${key}`);
|
||||
assert.match(
|
||||
parsed.errors.join(" "),
|
||||
/`description` is truncated/,
|
||||
`key form: ${key}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("the truncation guard spans a blank line inside a plain scalar", () => {
|
||||
// A plain scalar continues across empty lines too, so a scan that stopped at
|
||||
// the first blank one inspected only the opening paragraph and reported no
|
||||
// loss (Copilot).
|
||||
const parsed = parseSkill(
|
||||
"alpha",
|
||||
[
|
||||
"---",
|
||||
"name: alpha",
|
||||
"description: Covers boards",
|
||||
"",
|
||||
" #28 and #11 and the hazard.",
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n"),
|
||||
);
|
||||
assert.equal(parsed.description, "Covers boards");
|
||||
assert.match(parsed.errors.join(" "), /`description` is truncated/);
|
||||
});
|
||||
|
||||
test("a standalone comment line is not read as truncation", () => {
|
||||
// The guard keys on the comment the parser hung on the SCALAR. A comment on
|
||||
// its own line belongs to the document, not to the description, and flagging
|
||||
// it would fail a frontmatter that loses nothing.
|
||||
for (const frontmatter of [
|
||||
["# leading", "name: alpha", "description: Covers boards"],
|
||||
["name: alpha", "description: Covers boards", "# standalone"],
|
||||
["name: alpha", "description: Issue#28 needs no quoting"],
|
||||
]) {
|
||||
const parsed = parseSkill(
|
||||
"alpha",
|
||||
[
|
||||
"---",
|
||||
...frontmatter,
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n"),
|
||||
);
|
||||
assert.deepEqual(parsed.errors, [], frontmatter.join(" | "));
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -25,8 +25,9 @@
|
|||
// Closing that would need a deterministic trigger oracle, which does not exist.
|
||||
//
|
||||
// Usage:
|
||||
// npm run skills:eval # every model-invoked skill's cases
|
||||
// npm run skills:eval -- testing # one skill's cases
|
||||
// npm run skills:eval # every model-invoked skill's cases
|
||||
// npm run skills:eval -- testing # one skill's cases
|
||||
// npm run skills:eval -- testing test-servers # several skills' cases
|
||||
// RUNS=5 THRESHOLD=0.8 npm run skills:eval
|
||||
|
||||
import { spawn } from "node:child_process";
|
||||
|
|
@ -58,11 +59,19 @@ const CONCURRENCY = Number(process.env.CONCURRENCY ?? 4);
|
|||
* (Copilot). Every model-invoked skill is inspected; `only` is applied when
|
||||
* enqueueing.
|
||||
*
|
||||
* @param {string | undefined} only
|
||||
* A name that matches no model-invoked skill is a hard error rather than an
|
||||
* empty run: a typo would otherwise enqueue nothing and the eval would report a
|
||||
* green 0/0, which reads exactly like a clean pass of the skill you meant.
|
||||
*
|
||||
* @param {string | string[] | undefined} only One or more skill names.
|
||||
* @param {string} [skillsDir]
|
||||
* @returns {{ cases: object[], ours: Set<string> }}
|
||||
*/
|
||||
export function collectCases(only, skillsDir = SKILLS_DIR) {
|
||||
const wanted =
|
||||
only === undefined || only === null
|
||||
? null
|
||||
: new Set(Array.isArray(only) ? only : [only]);
|
||||
const cases = [];
|
||||
const ours = new Set();
|
||||
for (const dir of readdirSync(skillsDir).sort()) {
|
||||
|
|
@ -103,9 +112,18 @@ export function collectCases(only, skillsDir = SKILLS_DIR) {
|
|||
if (invalid.length > 0) {
|
||||
throw new Error(`${dir}/evals/evals.json: ${invalid.join("; ")}`);
|
||||
}
|
||||
if (only && dir !== only) continue;
|
||||
if (wanted && !wanted.has(dir)) continue;
|
||||
for (const c of parsed) cases.push({ ...c, from: dir });
|
||||
}
|
||||
if (wanted) {
|
||||
const unknown = [...wanted].filter((n) => !ours.has(n));
|
||||
if (unknown.length > 0) {
|
||||
throw new Error(
|
||||
`no model-invoked skill named ${unknown.map((n) => `\`${n}\``).join(", ")} — ` +
|
||||
`known: ${[...ours].sort().join(", ")}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
return { cases, ours };
|
||||
}
|
||||
|
||||
|
|
@ -310,7 +328,9 @@ async function main() {
|
|||
process.exit(1);
|
||||
}
|
||||
|
||||
const { cases, ours } = collectCases(process.argv[2]);
|
||||
const { cases, ours } = collectCases(
|
||||
process.argv.length > 2 ? process.argv.slice(2) : undefined,
|
||||
);
|
||||
if (cases.length === 0) {
|
||||
console.error("skills:eval — no cases found.");
|
||||
process.exit(1);
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ import {
|
|||
runPrompt,
|
||||
sampleHit,
|
||||
} from "./skill-eval.mjs";
|
||||
import { MIN_POSITIVE_CASES } from "./lib/skill-manifest.mjs";
|
||||
|
||||
const assistant = (...blocks) =>
|
||||
JSON.stringify({ type: "assistant", message: { content: blocks } });
|
||||
|
|
@ -285,14 +286,17 @@ function skillsFixture() {
|
|||
);
|
||||
}
|
||||
};
|
||||
write("alpha", "disable-model-invocation: false\n", [
|
||||
{ prompt: "a+", expect: "alpha" },
|
||||
{ prompt: "a-", expect: null },
|
||||
]);
|
||||
write("beta", "disable-model-invocation: false\n", [
|
||||
{ prompt: "b+", expect: "beta" },
|
||||
{ prompt: "b-", expect: null },
|
||||
]);
|
||||
// Enough positives to satisfy the same MIN_POSITIVE_CASES floor the real eval
|
||||
// files must clear; the prompts stay predictable so assertions can name them.
|
||||
const evalsFor = (letter, skill) => [
|
||||
...Array.from({ length: MIN_POSITIVE_CASES }, (_, i) => ({
|
||||
prompt: `${letter}+${i}`,
|
||||
expect: skill,
|
||||
})),
|
||||
{ prompt: `${letter}-`, expect: null },
|
||||
];
|
||||
write("alpha", "disable-model-invocation: false\n", evalsFor("a", "alpha"));
|
||||
write("beta", "disable-model-invocation: false\n", evalsFor("b", "beta"));
|
||||
write("gamma", "disable-model-invocation: true\n");
|
||||
return root;
|
||||
}
|
||||
|
|
@ -305,13 +309,13 @@ test("focused mode narrows the cases but not the repo's own skill set", () => {
|
|||
|
||||
const all = collectCases(undefined, root);
|
||||
assert.deepEqual([...all.ours].sort(), ["alpha", "beta"]);
|
||||
assert.equal(all.cases.length, 4);
|
||||
assert.equal(all.cases.length, (MIN_POSITIVE_CASES + 1) * 2);
|
||||
|
||||
const focused = collectCases("alpha", root);
|
||||
assert.deepEqual([...focused.ours].sort(), ["alpha", "beta"]);
|
||||
assert.deepEqual(
|
||||
focused.cases.map((c) => c.prompt),
|
||||
["a+", "a-"],
|
||||
["a+0", "a+1", "a+2", "a+3", "a+4", "a-"],
|
||||
);
|
||||
// The consequence, stated as the assertion that matters:
|
||||
assert.equal(
|
||||
|
|
@ -322,6 +326,53 @@ test("focused mode narrows the cases but not the repo's own skill set", () => {
|
|||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("focused mode accepts several skill names", () => {
|
||||
const root = skillsFixture();
|
||||
|
||||
const focused = collectCases(["alpha", "beta"], root);
|
||||
assert.deepEqual(
|
||||
focused.cases.map((c) => c.prompt).sort(),
|
||||
[
|
||||
"a+0",
|
||||
"a+1",
|
||||
"a+2",
|
||||
"a+3",
|
||||
"a+4",
|
||||
"a-",
|
||||
"b+0",
|
||||
"b+1",
|
||||
"b+2",
|
||||
"b+3",
|
||||
"b+4",
|
||||
"b-",
|
||||
].sort(),
|
||||
);
|
||||
// Narrowing to every skill is still not the same as no filter — `ours` is
|
||||
// unchanged either way, which is what keeps negative scoring honest.
|
||||
assert.deepEqual([...focused.ours].sort(), ["alpha", "beta"]);
|
||||
|
||||
// A single name keeps working, so the one-argument form is unaffected.
|
||||
assert.deepEqual(
|
||||
collectCases("beta", root).cases.map((c) => c.prompt),
|
||||
["b+0", "b+1", "b+2", "b+3", "b+4", "b-"],
|
||||
);
|
||||
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("an unknown skill name is an error, not an empty run", () => {
|
||||
// A typo would otherwise enqueue zero cases and report a green 0/0, which
|
||||
// reads exactly like a clean pass of the skill that was meant.
|
||||
const root = skillsFixture();
|
||||
assert.throws(
|
||||
() => collectCases(["alpha", "aplha"], root),
|
||||
/no model-invoked skill named `aplha`/,
|
||||
);
|
||||
// A name-only skill is not model-invoked, so asking for it is the same error.
|
||||
assert.throws(() => collectCases(["gamma"], root), /no model-invoked skill/);
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("a name-only skill is not part of the repo's model-invoked set", () => {
|
||||
const root = skillsFixture();
|
||||
const { ours } = collectCases(undefined, root);
|
||||
|
|
|
|||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Reference in a new issue