docs: accept several skill names in skills:eval and re-aim overlapping cases
Adds a multi-name filter to `npm run skills:eval` so a set of related skills can be iterated together, and makes an unknown name a hard error rather than an empty run — a typo would otherwise report a green 0/0 that reads exactly like a clean pass of the skill that was meant. Re-aims two `local-dev` cases and one `test-servers` case that partially overlapped `AGENTS.md`. Partial overlap oscillates across runs rather than sitting at zero, so it reads as eval noise instead of a defect in the case; `docs/skill-authoring.md` now records that failure mode and the observed scores. Widens the `test-servers` description to the "Use when …" shape and points the `testing` skill at it, since anything needing a real server to run against is a procedure `testing` does not carry. That growth moves the skill listing from 2,921 to 3,144 of 4,000 characters, so the figure recorded in AGENTS.md is updated to match. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
6dd43e78ba
commit
a344225e23
8 changed files with 100 additions and 12 deletions
|
|
@ -4,7 +4,7 @@
|
||||||
"expect": "local-dev"
|
"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"
|
"expect": "local-dev"
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
|
|
@ -16,7 +16,7 @@
|
||||||
"expect": "local-dev"
|
"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"
|
"expect": "local-dev"
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,6 @@
|
||||||
---
|
---
|
||||||
name: test-servers
|
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
|
disable-model-invocation: false
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -8,7 +8,7 @@
|
||||||
"expect": "test-servers"
|
"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"
|
"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
|
conventions live in [`AGENTS.md`](../../../AGENTS.md); this skill is where a
|
||||||
test goes, how to run it, and how to clear the gate.
|
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
|
## Where the test file goes
|
||||||
|
|
||||||
**Side-by-side by default; `src/test/` only for what can't be co-located; and
|
**Side-by-side by default; `src/test/` only for what can't be co-located; and
|
||||||
|
|
|
||||||
|
|
@ -233,7 +233,7 @@ that from happening:
|
||||||
overflows, and drops the least-invoked entries **first** — which are exactly the
|
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
|
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`
|
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
|
the budget deliberately, or tighten a description; each entry is capped at 1,536
|
||||||
characters regardless, so **put the key use case first**.
|
characters regardless, so **put the key use case first**.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -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. |
|
| "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%. |
|
| "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 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
|
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
|
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:
|
Then the suite:
|
||||||
|
|
||||||
```sh
|
```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
|
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
|
**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
|
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
|
of the same suite. `RUNS=5` is the smallest setting where a single flake does
|
||||||
|
|
|
||||||
|
|
@ -25,8 +25,9 @@
|
||||||
// Closing that would need a deterministic trigger oracle, which does not exist.
|
// Closing that would need a deterministic trigger oracle, which does not exist.
|
||||||
//
|
//
|
||||||
// Usage:
|
// Usage:
|
||||||
// npm run skills:eval # every model-invoked 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 # one skill's cases
|
||||||
|
// npm run skills:eval -- testing test-servers # several skills' cases
|
||||||
// RUNS=5 THRESHOLD=0.8 npm run skills:eval
|
// RUNS=5 THRESHOLD=0.8 npm run skills:eval
|
||||||
|
|
||||||
import { spawn } from "node:child_process";
|
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
|
* (Copilot). Every model-invoked skill is inspected; `only` is applied when
|
||||||
* enqueueing.
|
* 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]
|
* @param {string} [skillsDir]
|
||||||
* @returns {{ cases: object[], ours: Set<string> }}
|
* @returns {{ cases: object[], ours: Set<string> }}
|
||||||
*/
|
*/
|
||||||
export function collectCases(only, skillsDir = SKILLS_DIR) {
|
export function collectCases(only, skillsDir = SKILLS_DIR) {
|
||||||
|
const wanted =
|
||||||
|
only === undefined || only === null
|
||||||
|
? null
|
||||||
|
: new Set(Array.isArray(only) ? only : [only]);
|
||||||
const cases = [];
|
const cases = [];
|
||||||
const ours = new Set();
|
const ours = new Set();
|
||||||
for (const dir of readdirSync(skillsDir).sort()) {
|
for (const dir of readdirSync(skillsDir).sort()) {
|
||||||
|
|
@ -103,9 +112,18 @@ export function collectCases(only, skillsDir = SKILLS_DIR) {
|
||||||
if (invalid.length > 0) {
|
if (invalid.length > 0) {
|
||||||
throw new Error(`${dir}/evals/evals.json: ${invalid.join("; ")}`);
|
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 });
|
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 };
|
return { cases, ours };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -310,7 +328,9 @@ async function main() {
|
||||||
process.exit(1);
|
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) {
|
if (cases.length === 0) {
|
||||||
console.error("skills:eval — no cases found.");
|
console.error("skills:eval — no cases found.");
|
||||||
process.exit(1);
|
process.exit(1);
|
||||||
|
|
|
||||||
|
|
@ -322,6 +322,42 @@ test("focused mode narrows the cases but not the repo's own skill set", () => {
|
||||||
rmSync(root, { recursive: true, force: true });
|
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", () => {
|
test("a name-only skill is not part of the repo's model-invoked set", () => {
|
||||||
const root = skillsFixture();
|
const root = skillsFixture();
|
||||||
const { ours } = collectCases(undefined, root);
|
const { ours } = collectCases(undefined, root);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue