diff --git a/.claude/skills/local-dev/evals/evals.json b/.claude/skills/local-dev/evals/evals.json index a95355ce..d07254b1 100644 --- a/.claude/skills/local-dev/evals/evals.json +++ b/.claude/skills/local-dev/evals/evals.json @@ -4,7 +4,7 @@ "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" }, { @@ -16,7 +16,7 @@ "expect": "local-dev" }, { - "prompt": "I just pulled and a client's dependencies look out of sync. What do I need to run?", + "prompt": "Do I need to do anything special to set this repo up inside a git worktree?", "expect": "local-dev" }, { diff --git a/.claude/skills/test-servers/SKILL.md b/.claude/skills/test-servers/SKILL.md index 2e8b91b6..16556b18 100644 --- a/.claude/skills/test-servers/SKILL.md +++ b/.claude/skills/test-servers/SKILL.md @@ -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 --- diff --git a/.claude/skills/test-servers/evals/evals.json b/.claude/skills/test-servers/evals/evals.json index ba117347..237fece3 100644 --- a/.claude/skills/test-servers/evals/evals.json +++ b/.claude/skills/test-servers/evals/evals.json @@ -8,7 +8,7 @@ "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 a real server to exercise this change against by hand. What do I run?", "expect": "test-servers" }, { diff --git a/.claude/skills/testing/SKILL.md b/.claude/skills/testing/SKILL.md index dbbb4438..7a597e4c 100644 --- a/.claude/skills/testing/SKILL.md +++ b/.claude/skills/testing/SKILL.md @@ -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 diff --git a/AGENTS.md b/AGENTS.md index 252c1e39..3455f38e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -233,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,921/4,000 characters as of this writing) and fails when it is exceeded. Raise + (3,144/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/docs/skill-authoring.md b/docs/skill-authoring.md index 113ebdc0..34947428 100644 --- a/docs/skill-authoring.md +++ b/docs/skill-authoring.md @@ -104,6 +104,21 @@ retired here: | "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 | + +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 @@ -151,11 +166,22 @@ 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 +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) +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 diff --git a/scripts/skill-eval.mjs b/scripts/skill-eval.mjs index 2c8689a9..5325c1b3 100755 --- a/scripts/skill-eval.mjs +++ b/scripts/skill-eval.mjs @@ -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 }} */ 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); diff --git a/scripts/skill-eval.test.mjs b/scripts/skill-eval.test.mjs index 7c8536f8..a6b3ae99 100644 --- a/scripts/skill-eval.test.mjs +++ b/scripts/skill-eval.test.mjs @@ -322,6 +322,42 @@ 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+", + "a-", + "b+", + "b-", + ]); + // 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+", "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);