Merge branch 'v2/main' into v2/feat/2234-skills-extension
This commit is contained in:
Cliff Hall
GitHub
commit
18724b2338
9 changed files with 1196 additions and 76 deletions
|
|
@ -19,6 +19,20 @@
|
|||
"prompt": "I need a fixture combination that doesn't exist yet. How do I add one?",
|
||||
"expect": "test-servers"
|
||||
},
|
||||
{
|
||||
"prompt": "Write an integration test that exercises tool listing end to end.",
|
||||
"chain": [
|
||||
"testing",
|
||||
"test-servers"
|
||||
]
|
||||
},
|
||||
{
|
||||
"prompt": "Add end-to-end coverage for the tool-list pagination path.",
|
||||
"chain": [
|
||||
"testing",
|
||||
"test-servers"
|
||||
]
|
||||
},
|
||||
{
|
||||
"prompt": "Sort this list alphabetically: banana, apple, cherry.",
|
||||
"expect": null
|
||||
|
|
|
|||
14
AGENTS.md
14
AGENTS.md
|
|
@ -262,6 +262,20 @@ that from happening:
|
|||
("how does the `@inspector/core` alias resolve?") invites a `Read`, which is
|
||||
a *better* answer than a skill. Good cases are "how do I / where does this go"
|
||||
questions whose answer is a procedure.
|
||||
**A pointer from one skill's body to another is measured by a `chain` case,
|
||||
not an `expect` one.** A first-move case can only observe the model's opening
|
||||
tool call, so a skill reached only *through* another scores a clean 100% on
|
||||
its direct cases while the hand-off silently never fires (#2204). A chained
|
||||
case names the ordered skills one run should load, **ending with the skill
|
||||
whose file it lives in** — so the file that goes red is the one belonging to
|
||||
the skill that stopped being reached. It runs on a wider turn budget, is
|
||||
scored against its own `CHAIN_THRESHOLD`, and is **reported in its own
|
||||
column**: a hand-off rate and a first-move rate are not comparable, and
|
||||
folding them together would move a headline everyone reads as trigger
|
||||
reliability. It counts toward neither the five-positive floor nor the
|
||||
negative requirement, and it is only worth writing where the first link's
|
||||
body actually points at the target — a chain through a skill that says
|
||||
nothing about it is a permanent 0% with no lever.
|
||||
⚠️ **The gate cannot catch a description that never matches.** `verify:skills`
|
||||
checks that a skill is well-formed and that its cases exist; only
|
||||
`skills:eval` observes whether it actually fires, and that cannot be gated —
|
||||
|
|
|
|||
|
|
@ -14,6 +14,17 @@ reachability — `npm run skills:eval` — reports a hit rate rather than a verd
|
|||
session** (`claude -p`), `RUNS` times, and scores the fraction of runs in which
|
||||
the `Skill` tool fired with the expected name.
|
||||
|
||||
There are **two kinds of case**, measured against different turn budgets and
|
||||
reported in separate columns:
|
||||
|
||||
| | asserts | budget | column |
|
||||
| --- | --- | --- | --- |
|
||||
| `"expect": "<skill>"` / `null` | the skill is (or is not) the model's **first move** | 1 turn | first-move |
|
||||
| `"chain": ["a", …, "<skill>"]` | loading `a` **leads to** loading this skill | `CHAIN_MAX_TURNS` (14) | hand-off, `CHAIN_THRESHOLD` 0.5 |
|
||||
|
||||
Almost every case is the first kind, and the four properties below are about
|
||||
that kind. The hand-off case has its own section further down.
|
||||
|
||||
Four properties of that harness drive everything below:
|
||||
|
||||
- **`--max-turns 1`.** The skill must fire in the model's **first assistant
|
||||
|
|
@ -180,6 +191,125 @@ 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.
|
||||
|
||||
### Chained cases: measuring a hand-off
|
||||
|
||||
A skill body may point at another skill — `testing` opens by telling the model
|
||||
that picking a fixture is `/test-servers` and that it has to load it, and
|
||||
because `test-servers` is model-invocable that pointer is live rather than a
|
||||
dead end. **Nothing in a first-move case can observe whether that pointer is
|
||||
ever taken.** `test-servers` scores 5/5 on its own cases and every one of them
|
||||
asks for it by name; a skill only ever reached _through_ another would score a
|
||||
clean 100% while the hand-off silently never fired (#2204).
|
||||
|
||||
A chained case names the ordered skills one run should load:
|
||||
|
||||
```json
|
||||
{
|
||||
"prompt": "Write an integration test that exercises tool listing against a real server.",
|
||||
"chain": ["testing", "test-servers"]
|
||||
}
|
||||
```
|
||||
|
||||
**Write a chained case when the prompt names nothing about the target skill and
|
||||
the path to it runs through another skill.** Write an ordinary first-move case
|
||||
for everything else — a prompt someone would actually type to reach this skill
|
||||
directly is a first-move case even when a hand-off could also get there, and it
|
||||
is the cheaper measurement by an order of magnitude.
|
||||
|
||||
Six rules the shape enforces, each for a reason worth knowing:
|
||||
|
||||
- **The chain ends with the skill whose file it lives in.** The case exists to
|
||||
measure whether _this_ skill is reachable, so the file that must go red when
|
||||
the hand-off stops working is the one belonging to the skill that stops being
|
||||
reached. Anchoring on the first link would file the `testing → test-servers`
|
||||
measurement under `testing`, where a `test-servers` description edit would
|
||||
never be seen.
|
||||
- **A chained case satisfies neither floor.** It is not one of the five
|
||||
positives and it is not the negative. It measures a different thing, so
|
||||
letting it stand in would let a skill ship with no measurement of the way
|
||||
users actually reach it.
|
||||
- **The match is an ordered _subsequence_, not a prefix and not a contiguous
|
||||
run.** The model may load something before the chain starts and something
|
||||
unrelated in between; neither changes the claim that A led to B. What does not
|
||||
score is the reverse order.
|
||||
- **Every link after the first must land in a later assistant turn.** Position
|
||||
in the stream is not causation: the model can emit several `tool_use` blocks
|
||||
in one message, and it has not seen the first skill's body when it does — so
|
||||
two `Skill` calls in the same turn are parallel guesses, not a hand-off, and
|
||||
a flat index would score them as one (Copilot). This is the difference
|
||||
between "B was loaded after A" and "A led to B", and it is the second way a
|
||||
chained case can false-pass — the first being a prompt that carries the
|
||||
target's own trigger, below. Only the chain's *first* link is unconstrained.
|
||||
- **Repeats and unknown links are rejected.** A repeated link cannot be
|
||||
observed, and a link naming a skill the model cannot invoke can never fire —
|
||||
it would score a permanent 0% that reads as a description problem.
|
||||
- **The two numbers never share a column.** A hand-off rate is a second-hop load
|
||||
over many turns; a first-move rate is the model's opening move. Summing them
|
||||
would produce a figure describing neither, and a handful of hand-off cases
|
||||
would quietly move a headline everyone reads as trigger reliability.
|
||||
|
||||
⚠️ **The prompt must not carry the TARGET skill's own trigger.** This is the
|
||||
subtle way a chained case false-passes. `test-servers` claims the situation "a
|
||||
change needs a real server to exercise it", so a prompt saying "…against a real
|
||||
server" matches it directly: the model can pick `testing` first and then pick
|
||||
`test-servers` from the *original prompt*, in that order, and the case scores a
|
||||
hit that would survive deleting the pointer from `testing` entirely (Copilot).
|
||||
Both committed cases said "against a real/live server" and were rewritten to
|
||||
"end to end" for exactly this reason — and the measured rate **fell from 100%
|
||||
and 67% to 33% and 33%**, which is the size of the effect this trap hides.
|
||||
**Write the prompt so only the loaded first skill can introduce the second**,
|
||||
and sanity-check it by asking whether the case would still pass if the pointer
|
||||
were removed.
|
||||
|
||||
⚠️ **A chained case only measures a pointer that exists.** `pr-flow` says
|
||||
nothing about test fixtures, so a `["pr-flow", "test-servers"]` case measured 0%
|
||||
— correctly, and with no lever to fix it short of broadening a description onto
|
||||
another skill's ground. Before writing one, confirm the first link's body
|
||||
actually points at the target; otherwise the case is a permanent zero that reads
|
||||
as a description problem.
|
||||
|
||||
⚠️ **A hand-off case is a measurement under the harness's tool policy, not a
|
||||
prediction about an unrestricted session.** `--max-turns 1` was doing much of
|
||||
the read-only containment on its own; a 14-turn budget removes that, so the deny
|
||||
list covers the agentic and network tools too (`Task` in particular, whose
|
||||
subagent the flag does not reach). Denying `Bash` also changes the path a run
|
||||
can take toward the second skill, since investigating a repo by hand often
|
||||
starts there. `Read`/`Glob`/`Grep` remain, which is enough to reach a hand-off.
|
||||
|
||||
**A hand-off is far less reliable than a first move, and the threshold says so.**
|
||||
`CHAIN_THRESHOLD` defaults to **0.5**, not 0.8 — the weakest claim worth
|
||||
asserting is that the pointer is taken more often than not — and it is compared
|
||||
**strictly**. "More often than not" is `> 0.5`, and an inclusive compare would
|
||||
pass 2/4 whenever `RUNS` is even, reporting a result the criterion does not
|
||||
license (Copilot). A strict bound of `1.0` is therefore unreachable and the
|
||||
harness rejects it up front rather than failing every case.
|
||||
|
||||
At 0.8 a hand-off case would be red no matter how strongly the first skill
|
||||
pointed at the second, and the column would stop carrying signal. Read a
|
||||
hand-off number as a description-strength measurement, not a verdict — and read
|
||||
it at `RUNS=5`, since at `RUNS=3` one sample is worth 33 points.
|
||||
|
||||
**The committed cases have measured 33% / 33% on one `RUNS=3` run and 100% /
|
||||
33% on another, and at least one of them being red is the intended state rather
|
||||
than an oversight.** `skills:eval` is not a gate (see below), and the number is
|
||||
the finding: `testing` points at `test-servers` in its first paragraph and the
|
||||
model follows that pointer *sometimes*. Strengthening it is its own change
|
||||
against its own issue (#2247); lowering the bar to turn the column green would
|
||||
throw away the only signal this feature adds.
|
||||
|
||||
⚠️ **Do not read a rise between two `RUNS=3` runs as an improvement.** One
|
||||
sample is 33 points there, and the two runs above straddle a 67-point swing on
|
||||
the same prompt with no change to the pointer. Note in particular that the
|
||||
turn-boundary rule added later can only ever *lower* a chained score — it
|
||||
rejects matches a flatter reading accepted — so a higher number after it is
|
||||
noise by construction, not an effect. `RUNS=5` is the smallest honest setting
|
||||
for a hand-off, and the cost is real: each sample is up to 14 turns.
|
||||
|
||||
⚠️ **Expect a hand-off to cost far more than a first move.** Each sample is up
|
||||
to 14 turns rather than one, so a chained case is the most expensive line in the
|
||||
suite by a wide margin — the two above take longer between them than all seven
|
||||
first-move cases.
|
||||
|
||||
## The tuning loop
|
||||
|
||||
**Probe first, then measure.** A full suite run is ~63 cases × `RUNS` sessions
|
||||
|
|
@ -194,11 +324,35 @@ prompt fires at all, and only then spend a full run on its rate:
|
|||
# snippet also runs under bash.
|
||||
printf '%s' "<prompt>" \
|
||||
| claude -p --output-format stream-json --verbose --max-turns 1 \
|
||||
--disallowedTools Bash,Write,Edit,NotebookEdit \
|
||||
--tools Read,Glob,Grep,Skill --allowedTools Read,Glob,Grep,Skill \
|
||||
--disallowedTools Bash,Write,Edit,NotebookEdit,Task,Agent,SlashCommand,WebFetch,WebSearch,KillShell \
|
||||
--strict-mcp-config \
|
||||
| jq -r 'select(.message.content?) | .message.content[]?
|
||||
| select(.type == "tool_use") | .name' | head -3
|
||||
```
|
||||
|
||||
⚠️ **`--tools` is the restriction; `--allowedTools` only pre-approves.**
|
||||
Dropping the first leaves the bound resting on the deny list alone, so a tool a
|
||||
user's or a plugin's settings already permit stays reachable for all 14 turns
|
||||
(Copilot). Keep both.
|
||||
|
||||
⚠️ **These flags are a copy of the harness's, so they go stale.** Whenever
|
||||
`runPrompt` in `scripts/skill-eval.mjs` changes its tool policy, change this
|
||||
snippet in the same edit — a probe that may call a tool the eval forbids
|
||||
predicts nothing, which is the whole reason the two are meant to match
|
||||
(Copilot). To probe a **hand-off** instead, raise `--max-turns` to
|
||||
`CHAIN_MAX_TURNS` and drop the `head -3`:
|
||||
|
||||
```sh
|
||||
printf '%s' "<prompt>" \
|
||||
| claude -p --output-format stream-json --verbose --max-turns 14 \
|
||||
--tools Read,Glob,Grep,Skill --allowedTools Read,Glob,Grep,Skill \
|
||||
--disallowedTools Bash,Write,Edit,NotebookEdit,Task,Agent,SlashCommand,WebFetch,WebSearch,KillShell \
|
||||
--strict-mcp-config \
|
||||
| jq -r 'select(.message.content?) | .message.content[]?
|
||||
| select(.type == "tool_use" and .name == "Skill") | .input.skill'
|
||||
```
|
||||
|
||||
**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
|
||||
|
|
@ -211,6 +365,14 @@ npm run skills:eval # every model-invoked skill, RU
|
|||
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
|
||||
CHAIN_THRESHOLD=0.4 CHAIN_MAX_TURNS=20 npm run skills:eval -- test-servers
|
||||
```
|
||||
|
||||
The summary is two lines, never one:
|
||||
|
||||
```
|
||||
7/7 first-move cases at or above 80%.
|
||||
1/2 hand-off cases above 50%.
|
||||
```
|
||||
|
||||
Narrowing the run never narrows what a **negative** case is scored against — a
|
||||
|
|
@ -257,6 +419,12 @@ break.
|
|||
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.
|
||||
5. **If this skill is meant to be reachable from another skill's body**, that
|
||||
hand-off has a `chain` case — a pointer between skills is otherwise measured
|
||||
by nothing at all, and a skill reached only that way scores a clean 100% on
|
||||
direct cases while the hand-off never fires. It does not count toward the
|
||||
floor in 4.
|
||||
6. `npm run verify:skills` passes and the listing is under budget.
|
||||
7. `RUNS=5 npm run skills:eval` — the **whole** suite — is ≥80% on every
|
||||
first-move case, including the skills you did not touch, and the hand-off
|
||||
column is read on its own rather than against that number.
|
||||
|
|
|
|||
|
|
@ -250,6 +250,31 @@ export function listingCost(skills) {
|
|||
*/
|
||||
export const MIN_POSITIVE_CASES = 5;
|
||||
|
||||
/**
|
||||
* The fewest links a chained case may name.
|
||||
*
|
||||
* A one-link "chain" is a first-move case written the long way — it asserts
|
||||
* nothing about a hand-off, and accepting it would let a skill satisfy the
|
||||
* hand-off column without ever measuring one.
|
||||
*/
|
||||
export const MIN_CHAIN_LENGTH = 2;
|
||||
|
||||
/**
|
||||
* Whether a case measures a hand-off (`chain`) rather than a first move
|
||||
* (`expect`).
|
||||
*
|
||||
* The two are scored against different turn budgets and reported in different
|
||||
* columns, so every consumer has to tell them apart; doing it by field
|
||||
* presence in one place keeps that decision from drifting between the
|
||||
* validator and the runner.
|
||||
*
|
||||
* @param {unknown} c
|
||||
* @returns {boolean}
|
||||
*/
|
||||
export function isChainCase(c) {
|
||||
return c !== null && typeof c === "object" && Array.isArray(c.chain);
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate an `evals/evals.json` payload for a model-invoked skill.
|
||||
*
|
||||
|
|
@ -257,11 +282,30 @@ export const MIN_POSITIVE_CASES = 5;
|
|||
* is the failure nobody notices by hand — so negatives are required, not
|
||||
* optional.
|
||||
*
|
||||
* A case is one of two shapes, and it must be exactly one: a **first-move**
|
||||
* case names `expect` (the skill the model should reach with its first tool
|
||||
* call, or `null`), and a **hand-off** case names `chain` — the ordered skills
|
||||
* a single run should load, ending with this one. Requiring exactly one of the
|
||||
* two rather than letting `chain` shadow `expect` means a case that carries
|
||||
* both is a typo caught here, not a silently half-scored measurement.
|
||||
*
|
||||
* A hand-off case counts toward neither the positive floor nor the negative
|
||||
* requirement. It measures a different thing (a second-hop load, over many
|
||||
* turns) and it is scored in its own column, so letting one stand in for a
|
||||
* first-move positive would let a skill ship with no measurement of the way
|
||||
* users actually reach it.
|
||||
*
|
||||
* @param {string} skillName
|
||||
* @param {unknown} cases Parsed JSON.
|
||||
* @param {Set<string> | null} [known] Every model-invoked skill in the repo.
|
||||
* When supplied, each link of a `chain` is checked against it — a link naming
|
||||
* a skill that does not exist, or one the model cannot invoke, can never fire
|
||||
* and would score the case a permanent 0% that reads as a description
|
||||
* problem. Omitted (null), the link names are left unchecked, so a caller
|
||||
* that has not yet parsed the whole directory can still validate shape.
|
||||
* @returns {string[]} errors
|
||||
*/
|
||||
export function validateEvalCases(skillName, cases) {
|
||||
export function validateEvalCases(skillName, cases, known = null) {
|
||||
if (!Array.isArray(cases) || cases.length === 0) {
|
||||
return ["evals.json must be a non-empty array of cases"];
|
||||
}
|
||||
|
|
@ -274,11 +318,23 @@ export function validateEvalCases(skillName, cases) {
|
|||
if (typeof c.prompt !== "string" || c.prompt.trim() === "") {
|
||||
errors.push(`case ${i}: \`prompt\` must be a non-empty string`);
|
||||
}
|
||||
if ("expect" in c && "chain" in c) {
|
||||
errors.push(
|
||||
`case ${i}: carries both \`expect\` and \`chain\` — a case is either a first-move case or a hand-off case`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
if ("chain" in c) {
|
||||
errors.push(...validateChain(skillName, i, c.chain, known));
|
||||
return;
|
||||
}
|
||||
if (
|
||||
!("expect" in c) ||
|
||||
(c.expect !== null && typeof c.expect !== "string")
|
||||
) {
|
||||
errors.push(`case ${i}: \`expect\` must be a skill name or null`);
|
||||
errors.push(
|
||||
`case ${i}: needs \`expect\` (a skill name, or null for a negative case) or \`chain\``,
|
||||
);
|
||||
} else if (c.expect !== null && c.expect !== skillName) {
|
||||
// A case living in this skill's evals may only expect THIS skill. A
|
||||
// foreign name passes the eval whenever that other skill fires, so the
|
||||
|
|
@ -325,6 +381,57 @@ export function validateEvalCases(skillName, cases) {
|
|||
return errors;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate one hand-off case's `chain`.
|
||||
*
|
||||
* The last link must be the owning skill, not the first. A hand-off case exists
|
||||
* to measure whether **this** skill is reachable at all when nothing about the
|
||||
* prompt names it, so the file that has to go red when the hand-off stops
|
||||
* working is the one belonging to the skill that stops being reached. Anchoring
|
||||
* on the first link instead would file the case under whichever skill happened
|
||||
* to start the run, and a `test-servers` description edit would then be
|
||||
* measured only inside `testing`.
|
||||
*
|
||||
* @param {string} skillName
|
||||
* @param {number} i Case index, for the message.
|
||||
* @param {unknown} chain
|
||||
* @param {Set<string> | null} known
|
||||
* @returns {string[]}
|
||||
*/
|
||||
function validateChain(skillName, i, chain, known) {
|
||||
if (!Array.isArray(chain) || chain.length < MIN_CHAIN_LENGTH) {
|
||||
return [
|
||||
`case ${i}: \`chain\` must be an ordered array of at least ${MIN_CHAIN_LENGTH} skill names`,
|
||||
];
|
||||
}
|
||||
const errors = [];
|
||||
if (chain.some((n) => typeof n !== "string" || n.trim() === "")) {
|
||||
errors.push(`case ${i}: every \`chain\` link must be a non-empty string`);
|
||||
return errors;
|
||||
}
|
||||
if (new Set(chain).size !== chain.length) {
|
||||
// A repeated link cannot be observed: the run records which skills fired
|
||||
// in what order, and a second load of one already recorded is
|
||||
// indistinguishable from the first.
|
||||
errors.push(`case ${i}: \`chain\` repeats a skill name`);
|
||||
}
|
||||
if (chain[chain.length - 1] !== skillName) {
|
||||
errors.push(
|
||||
`case ${i}: \`chain\` ends with \`${chain[chain.length - 1]}\`, but this file measures whether \`${skillName}\` is reached`,
|
||||
);
|
||||
}
|
||||
if (known) {
|
||||
for (const link of chain) {
|
||||
if (!known.has(link)) {
|
||||
errors.push(
|
||||
`case ${i}: \`chain\` names \`${link}\`, which is not a model-invoked skill — it can never fire`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
return errors;
|
||||
}
|
||||
|
||||
/**
|
||||
* Claude Code version the authoritative validator is pinned to when it has to
|
||||
* be fetched. Pinned rather than @latest: a validator that moves on its own can
|
||||
|
|
|
|||
|
|
@ -9,6 +9,8 @@ import assert from "node:assert/strict";
|
|||
import {
|
||||
splitFrontmatter,
|
||||
parseSkill,
|
||||
isChainCase,
|
||||
MIN_CHAIN_LENGTH,
|
||||
MIN_POSITIVE_CASES,
|
||||
validateEvalCases,
|
||||
listingCost,
|
||||
|
|
@ -204,8 +206,116 @@ test("validateEvalCases rejects malformed cases", () => {
|
|||
);
|
||||
assert.match(
|
||||
validateEvalCases("x", [{ prompt: "a", expect: 7 }]).join(),
|
||||
/expect. must be a skill name or null/,
|
||||
/needs .expect./,
|
||||
);
|
||||
assert.match(
|
||||
validateEvalCases("x", [{ prompt: "a" }]).join(),
|
||||
/needs .expect. .* or .chain./,
|
||||
);
|
||||
});
|
||||
|
||||
test("a hand-off case names an ordered chain ending in this skill", () => {
|
||||
const base = [
|
||||
...Array.from({ length: MIN_POSITIVE_CASES }, (_, i) => ({
|
||||
prompt: `p${i}`,
|
||||
expect: "test-servers",
|
||||
})),
|
||||
{ prompt: "n", expect: null },
|
||||
];
|
||||
const withChain = (chain) => [...base, { prompt: "c", chain }];
|
||||
const known = new Set(["testing", "test-servers", "pr-flow"]);
|
||||
|
||||
assert.deepEqual(
|
||||
validateEvalCases(
|
||||
"test-servers",
|
||||
withChain(["testing", "test-servers"]),
|
||||
known,
|
||||
),
|
||||
[],
|
||||
);
|
||||
// The case belongs to the skill that must be REACHED, so a chain anchored on
|
||||
// its first link would file a `test-servers` measurement under `testing` and
|
||||
// leave a `test-servers` description edit unmeasured by its own file.
|
||||
assert.match(
|
||||
validateEvalCases(
|
||||
"test-servers",
|
||||
withChain(["test-servers", "testing"]),
|
||||
known,
|
||||
).join(),
|
||||
/ends with .testing., but this file measures .* .test-servers./,
|
||||
);
|
||||
assert.match(
|
||||
validateEvalCases(
|
||||
"test-servers",
|
||||
withChain(["test-servers"]),
|
||||
known,
|
||||
).join(),
|
||||
new RegExp(`at least ${MIN_CHAIN_LENGTH} skill names`),
|
||||
);
|
||||
assert.match(
|
||||
validateEvalCases(
|
||||
"test-servers",
|
||||
withChain(["testing", "testing", "test-servers"]),
|
||||
known,
|
||||
).join(),
|
||||
/repeats a skill name/,
|
||||
);
|
||||
assert.match(
|
||||
validateEvalCases(
|
||||
"test-servers",
|
||||
withChain(["nope", "test-servers"]),
|
||||
known,
|
||||
).join(),
|
||||
/.nope., which is not a model-invoked skill/,
|
||||
);
|
||||
assert.match(
|
||||
validateEvalCases(
|
||||
"test-servers",
|
||||
withChain(["", "test-servers"]),
|
||||
known,
|
||||
).join(),
|
||||
/non-empty string/,
|
||||
);
|
||||
// Without the known set the shape is still checked; only the link names go
|
||||
// unverified, so a caller that has not parsed the directory can still run.
|
||||
assert.deepEqual(
|
||||
validateEvalCases("test-servers", withChain(["nope", "test-servers"])),
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
test("a hand-off case satisfies neither floor and never doubles as a first move", () => {
|
||||
// A hand-off is a second-hop load over many turns and is scored in its own
|
||||
// column. Letting one stand in for a first-move positive would let a skill
|
||||
// ship with no measurement of the way users actually reach it.
|
||||
const chained = Array.from({ length: MIN_POSITIVE_CASES + 1 }, (_, i) => ({
|
||||
prompt: `c${i}`,
|
||||
chain: ["testing", "test-servers"],
|
||||
}));
|
||||
const errors = validateEvalCases("test-servers", chained).join(" ");
|
||||
assert.match(errors, /no positive case/);
|
||||
assert.match(errors, /no negative case/);
|
||||
});
|
||||
|
||||
test("a case carrying both shapes is a typo, not a half-scored measurement", () => {
|
||||
assert.match(
|
||||
validateEvalCases("test-servers", [
|
||||
{
|
||||
prompt: "c",
|
||||
expect: "test-servers",
|
||||
chain: ["testing", "test-servers"],
|
||||
},
|
||||
]).join(),
|
||||
/carries both .expect. and .chain./,
|
||||
);
|
||||
});
|
||||
|
||||
test("isChainCase tells the two shapes apart", () => {
|
||||
assert.equal(isChainCase({ chain: ["a", "b"] }), true);
|
||||
assert.equal(isChainCase({ expect: "a" }), false);
|
||||
assert.equal(isChainCase({ expect: null }), false);
|
||||
assert.equal(isChainCase(null), false);
|
||||
assert.equal(isChainCase("chain"), false);
|
||||
});
|
||||
|
||||
test("parseClaudeVersion reads the CLI's version banner", () => {
|
||||
|
|
|
|||
|
|
@ -29,6 +29,13 @@
|
|||
// 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
|
||||
//
|
||||
// Two kinds of case, measured and reported separately (#2204). A `expect` case
|
||||
// is a FIRST-MOVE measurement: one turn, does the model reach for the skill
|
||||
// before anything else. A `chain` case is a HAND-OFF measurement: many turns,
|
||||
// does loading skill A actually lead the model to load skill B. The two numbers
|
||||
// are not comparable — a hand-off is a second-hop load that only happens once
|
||||
// the run has established it needs one — so they never share a column.
|
||||
|
||||
import { spawn } from "node:child_process";
|
||||
import { readFileSync, existsSync, readdirSync, statSync } from "node:fs";
|
||||
|
|
@ -36,6 +43,7 @@ import path from "node:path";
|
|||
import { fileURLToPath } from "node:url";
|
||||
import { claudeSpawnArgs, probeClaudeVersion } from "./lib/claude-cli.mjs";
|
||||
import {
|
||||
isChainCase,
|
||||
parseClaudeVersion,
|
||||
parseSkill,
|
||||
validateEvalCases,
|
||||
|
|
@ -45,9 +53,35 @@ const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..");
|
|||
const SKILLS_DIR = path.join(ROOT, ".claude", "skills");
|
||||
|
||||
const THRESHOLD = Number(process.env.THRESHOLD ?? 0.8);
|
||||
// A hand-off is a harder thing to hit than a first move, and what counts as
|
||||
// acceptable is a separate judgement rather than one inherited from a number
|
||||
// tuned for the other measurement. 0.5 is the weakest claim worth asserting —
|
||||
// the pointer is taken more often than not. It is deliberately not 0.8: the
|
||||
// committed `testing` -> `test-servers` cases measure 33% (RUNS=3) against a
|
||||
// pointer that is live and stated in the first paragraph of `testing`'s body,
|
||||
// so an 0.8 bar would mark every hand-off red regardless of how strongly the
|
||||
// first skill points at the second, and the column would stop carrying signal.
|
||||
//
|
||||
// It is compared STRICTLY, unlike the first-move threshold. "More often than
|
||||
// not" is `> 0.5`, and an inclusive compare passes exactly half the samples
|
||||
// whenever RUNS is even — 2/4 would report a pass the stated criterion does not
|
||||
// license (Copilot). A consequence worth knowing: a strict bound of 1.0 can
|
||||
// never be met, so it is rejected below rather than silently failing every case.
|
||||
const CHAIN_THRESHOLD = Number(process.env.CHAIN_THRESHOLD ?? 0.5);
|
||||
const RUNS = Number(process.env.RUNS ?? 3);
|
||||
const CONCURRENCY = Number(process.env.CONCURRENCY ?? 4);
|
||||
|
||||
/**
|
||||
* Turns a hand-off case gets.
|
||||
*
|
||||
* `--max-turns 1` is what makes a first-move case a first-move case, so a
|
||||
* chained case needs a budget wide enough for the run to establish that it
|
||||
* needs the second skill. #2204 measured the `testing` -> `test-servers`
|
||||
* hand-off going 9-12 tool calls without reaching it; a budget under that
|
||||
* cannot distinguish "the hand-off does not fire" from "the run was cut short".
|
||||
*/
|
||||
const CHAIN_MAX_TURNS = Number(process.env.CHAIN_MAX_TURNS ?? 14);
|
||||
|
||||
/** Collect the committed cases for every model-invoked skill (optionally one). */
|
||||
/**
|
||||
* Collect the committed cases, and the set of skill names that are OURS.
|
||||
|
|
@ -63,6 +97,13 @@ const CONCURRENCY = Number(process.env.CONCURRENCY ?? 4);
|
|||
* 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.
|
||||
*
|
||||
* Collection is two passes. A hand-off case names other skills, and its links
|
||||
* are checked against the repo's model-invoked set — which is only complete
|
||||
* once every directory has been parsed. Validating inside the first pass would
|
||||
* make the check depend on directory order: `test-servers` sorts before
|
||||
* `testing`, so a chain through `testing` would be rejected as unknown purely
|
||||
* because of where the alphabet put it.
|
||||
*
|
||||
* @param {string | string[] | undefined} only One or more skill names.
|
||||
* @param {string} [skillsDir]
|
||||
* @returns {{ cases: object[], ours: Set<string> }}
|
||||
|
|
@ -74,6 +115,7 @@ export function collectCases(only, skillsDir = SKILLS_DIR) {
|
|||
: new Set(Array.isArray(only) ? only : [only]);
|
||||
const cases = [];
|
||||
const ours = new Set();
|
||||
const files = [];
|
||||
for (const dir of readdirSync(skillsDir).sort()) {
|
||||
const skillFile = path.join(skillsDir, dir, "SKILL.md");
|
||||
if (
|
||||
|
|
@ -108,7 +150,10 @@ export function collectCases(only, skillsDir = SKILLS_DIR) {
|
|||
`${dir}/evals/evals.json is not valid JSON — ${e.message}`,
|
||||
);
|
||||
}
|
||||
const invalid = validateEvalCases(dir, parsed);
|
||||
files.push({ dir, parsed });
|
||||
}
|
||||
for (const { dir, parsed } of files) {
|
||||
const invalid = validateEvalCases(dir, parsed, ours);
|
||||
if (invalid.length > 0) {
|
||||
throw new Error(`${dir}/evals/evals.json: ${invalid.join("; ")}`);
|
||||
}
|
||||
|
|
@ -136,15 +181,33 @@ export function collectCases(only, skillsDir = SKILLS_DIR) {
|
|||
* of an eval run and would otherwise only ever be exercised by the thing they
|
||||
* are supposed to measure.
|
||||
*
|
||||
* The invocations come back as an ORDERED array, repeats included, rather than
|
||||
* a set. A hand-off case asserts that one skill was loaded *after* another, so
|
||||
* occurrence order is the observation — and collapsing repeats would make a
|
||||
* run that loaded B, then A, then B again indistinguishable from one that never
|
||||
* reached B from A (#2204).
|
||||
*
|
||||
* Each entry also carries the **assistant event** it came from, and that
|
||||
* boundary is what makes the order mean something. The model may emit several
|
||||
* `tool_use` blocks in one message, and it cannot see the first skill's body
|
||||
* until the message after — so two `Skill` calls in the SAME event are
|
||||
* concurrent guesses, not a hand-off, however they happen to be ordered inside
|
||||
* the array. Flattening the stream and reading position alone would score that
|
||||
* as `A` leading to `B` (Copilot); `chainHit` requires a later turn instead.
|
||||
*
|
||||
* @param {string} text One or more newline-delimited JSON events. A trailing
|
||||
* partial line is ignored, so this can be fed incrementally.
|
||||
* @returns {{ invoked: Set<string>, rest: string, result: string | null }}
|
||||
* @param {number} [turnOffset] Assistant events already seen, so a stream fed
|
||||
* in chunks keeps one monotonic turn count rather than restarting per chunk.
|
||||
* @returns {{ invoked: {payload: string, turn: number}[], rest: string,
|
||||
* result: string | null, nextTurn: number }}
|
||||
*/
|
||||
export function collectSkillInvocations(text) {
|
||||
export function collectSkillInvocations(text, turnOffset = 0) {
|
||||
const lines = text.split("\n");
|
||||
const rest = lines.pop() ?? "";
|
||||
const invoked = new Set();
|
||||
const invoked = [];
|
||||
let result = null;
|
||||
let turn = turnOffset;
|
||||
for (const line of lines) {
|
||||
if (!line.trim()) continue;
|
||||
let evt;
|
||||
|
|
@ -156,13 +219,26 @@ export function collectSkillInvocations(text) {
|
|||
}
|
||||
if (evt?.type === "result") result = evt.subtype ?? null;
|
||||
if (evt?.type !== "assistant") continue;
|
||||
// One assistant event is one turn: everything inside it was decided at
|
||||
// once, before any of its results came back.
|
||||
turn++;
|
||||
for (const block of evt.message?.content ?? []) {
|
||||
if (block?.type !== "tool_use" || block.name !== "Skill") continue;
|
||||
// Don't assume the input field's name — match on the whole payload.
|
||||
invoked.add(JSON.stringify(block.input ?? {}));
|
||||
invoked.push({ payload: JSON.stringify(block.input ?? {}), turn });
|
||||
}
|
||||
}
|
||||
return { invoked, rest, result };
|
||||
return { invoked, rest, result, nextTurn: turn };
|
||||
}
|
||||
|
||||
/**
|
||||
* The skill names one recorded invocation asked for.
|
||||
*
|
||||
* @param {{payload: string} | string} entry
|
||||
* @returns {string[]}
|
||||
*/
|
||||
function entryNames(entry) {
|
||||
return invokedSkillNames(typeof entry === "string" ? entry : entry.payload);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -230,18 +306,135 @@ export function invokedSkillNames(payload) {
|
|||
* false failure about someone else's environment rather than about these
|
||||
* skills (Copilot).
|
||||
*
|
||||
* Turn boundaries are irrelevant here — a first-move case asks only whether a
|
||||
* skill fired at all — so this reads the names and ignores the rest.
|
||||
*
|
||||
* @param {string | null} expect Skill name, or null for a negative case.
|
||||
* @param {Set<string>} invoked
|
||||
* @param {Iterable<{payload: string} | string>} invoked
|
||||
* @param {Set<string> | null} [ours] Repo skill names. Null counts any skill.
|
||||
*/
|
||||
export function sampleHit(expect, invoked, ours = null) {
|
||||
const names = [...invoked].flatMap(invokedSkillNames);
|
||||
const names = [...invoked].flatMap(entryNames);
|
||||
if (expect === null) {
|
||||
return ours === null ? names.length === 0 : !names.some((n) => ours.has(n));
|
||||
}
|
||||
return names.includes(expect);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether one sample satisfies a hand-off case.
|
||||
*
|
||||
* The chain has to appear as an ordered SUBSEQUENCE of what fired, not as a
|
||||
* prefix and not as a contiguous run. Two reasons, both of which a stricter
|
||||
* match gets wrong: the model is free to load an unrelated skill in between,
|
||||
* and it may well load something before the chain's first link — neither
|
||||
* changes the fact that A led to B, which is the only claim the case makes.
|
||||
*
|
||||
* Every link after the first must land in a **strictly later assistant turn**
|
||||
* than the one before. Position in the stream is not causation: the model can
|
||||
* emit several `tool_use` blocks in one message, and it has not seen the first
|
||||
* skill's body when it does, so two `Skill` calls in the same turn are parallel
|
||||
* guesses that a flat index would happily score as a hand-off (Copilot). This
|
||||
* is the whole difference between "B was loaded after A" and "A led to B".
|
||||
*
|
||||
* The scan stays greedy, which is still correct under that constraint: taking
|
||||
* the EARLIEST occurrence of a link can only leave more room for the rest, so
|
||||
* no later starting point could succeed where the greedy one fails.
|
||||
*
|
||||
* Nothing is asserted about foreign skills here, unlike a negative case. A
|
||||
* hand-off case names exactly what it wants and a contributor's own
|
||||
* `~/.claude/skills` entry firing alongside it says nothing either way.
|
||||
*
|
||||
* @param {string[]} chain Ordered skill names, ending with the owning skill.
|
||||
* @param {Iterable<{payload: string, turn: number}>} invoked
|
||||
* @returns {boolean}
|
||||
*/
|
||||
export function chainHit(chain, invoked) {
|
||||
let want = 0;
|
||||
let prevTurn = -Infinity;
|
||||
for (const entry of invoked) {
|
||||
if (!entryNames(entry).includes(chain[want])) continue;
|
||||
// A link in the same turn as the previous one cannot have been caused by
|
||||
// it — the model had not seen that skill's body yet.
|
||||
if (want > 0 && !(entry.turn > prevTurn)) continue;
|
||||
prevTurn = entry.turn;
|
||||
want++;
|
||||
if (want === chain.length) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Score one sample against whichever kind of case it belongs to.
|
||||
*
|
||||
* @param {{ expect?: string | null, chain?: string[] }} c
|
||||
* @param {Iterable<string>} invoked
|
||||
* @param {Set<string> | null} ours
|
||||
*/
|
||||
export function caseHit(c, invoked, ours) {
|
||||
return isChainCase(c)
|
||||
? chainHit(c.chain, invoked)
|
||||
: sampleHit(c.expect, invoked, ours);
|
||||
}
|
||||
|
||||
/**
|
||||
* The only tools an eval run needs: read the repo, and load a skill.
|
||||
*
|
||||
* Enumerating what is AVAILABLE rather than only what is denied is the load-
|
||||
* bearing half. A deny list cannot bound a 14-turn run, because it only names
|
||||
* the tools known when it was written: this checkout configures an HTTP
|
||||
* `mcp-docs` server in `.mcp.json`, and a contributor's own MCP servers and
|
||||
* plugins add more tools that no list here has ever seen (Copilot). Naming the
|
||||
* four the harness actually needs closes that by construction.
|
||||
*
|
||||
* The list goes to `--tools`, which selects from the built-in set, AND to
|
||||
* `--allowedTools`, which pre-approves. The distinction matters and cost us a
|
||||
* round: `--allowedTools` grants permission, it does not filter availability,
|
||||
* so a tool a user's or a plugin's settings already permit would still have
|
||||
* been reachable across those 14 turns (Copilot). `--tools` is the restriction;
|
||||
* `--allowedTools` keeps the four from needing a prompt no headless run can
|
||||
* answer.
|
||||
*/
|
||||
const ALLOWED_TOOLS = ["Read", "Glob", "Grep", "Skill"];
|
||||
|
||||
/**
|
||||
* Tools no eval run may use, first-move or hand-off.
|
||||
*
|
||||
* Kept alongside the allow list rather than replaced by it: a deny is
|
||||
* unconditional, while an allow list governs which tools are pre-approved, so
|
||||
* the two together are stricter than either. A skill may inject `!`-prefixed
|
||||
* shell commands on load, and those run BEFORE its content reaches the model —
|
||||
* so this is what keeps a measurement from having side effects. It matters
|
||||
* more for a hand-off case than a first-move one: `--max-turns 1` was doing
|
||||
* much of the containment by itself, and a 14-turn budget removes that
|
||||
* (#2204). Hence the agentic and network tools too — `Task` would spawn a
|
||||
* subagent whose own tool policy neither flag reaches.
|
||||
*
|
||||
* MCP servers are dropped outright with `--strict-mcp-config` (and no
|
||||
* `--mcp-config`) rather than named here, since their tool names are not
|
||||
* knowable from this file. What remains outside all three mechanisms is a
|
||||
* contributor's own plugin tools; `--bare` would remove those and skills with
|
||||
* them, which would measure nothing.
|
||||
*
|
||||
* The cost is stated rather than hidden: denying `Bash` also changes the path
|
||||
* a run can take toward the second skill, since investigating a repo by hand
|
||||
* often starts there. `Read`/`Glob`/`Grep` remain, which is enough to reach a
|
||||
* hand-off, but a chained rate is a measurement under this policy and not a
|
||||
* prediction of an unrestricted session.
|
||||
*/
|
||||
const DISALLOWED_TOOLS = [
|
||||
"Bash",
|
||||
"Write",
|
||||
"Edit",
|
||||
"NotebookEdit",
|
||||
"Task",
|
||||
"Agent",
|
||||
"SlashCommand",
|
||||
"WebFetch",
|
||||
"WebSearch",
|
||||
"KillShell",
|
||||
];
|
||||
|
||||
/**
|
||||
* Drive one fresh session and return the payloads the `Skill` tool was called
|
||||
* with.
|
||||
|
|
@ -252,12 +445,18 @@ export function sampleHit(expect, invoked, ours = null) {
|
|||
* silently reported a plausible hit rate for runs that never happened (Copilot).
|
||||
*
|
||||
* @param {string} prompt
|
||||
* @param {{ spawnFn?: typeof spawn, cwd?: string }} [opts]
|
||||
* @returns {Promise<Set<string>>}
|
||||
* @param {{ spawnFn?: typeof spawn, cwd?: string, maxTurns?: number }} [opts]
|
||||
* @returns {Promise<{payload: string, turn: number}[]>} Skill invocations, in
|
||||
* the order they fired, each tagged with the assistant turn it came from.
|
||||
*/
|
||||
export function runPrompt(
|
||||
prompt,
|
||||
{ spawnFn = spawn, cwd = ROOT, platform = process.platform } = {},
|
||||
{
|
||||
spawnFn = spawn,
|
||||
cwd = ROOT,
|
||||
platform = process.platform,
|
||||
maxTurns = 1,
|
||||
} = {},
|
||||
) {
|
||||
return new Promise((resolve, reject) => {
|
||||
// The prompt goes in on STDIN, not in argv. `claude -p` with piped stdin
|
||||
|
|
@ -273,11 +472,17 @@ export function runPrompt(
|
|||
"stream-json",
|
||||
"--verbose",
|
||||
"--max-turns",
|
||||
"1",
|
||||
// Keep the run read-only. A skill may inject `!`-prefixed shell commands
|
||||
// on load, and those run BEFORE its content reaches the model.
|
||||
String(maxTurns),
|
||||
// Keep the run read-only, across every turn it is given: what the
|
||||
// harness needs, minus what it must never do, minus every MCP server
|
||||
// this checkout or the contributor happens to configure.
|
||||
"--tools",
|
||||
ALLOWED_TOOLS.join(","),
|
||||
"--allowedTools",
|
||||
ALLOWED_TOOLS.join(","),
|
||||
"--disallowedTools",
|
||||
"Bash,Write,Edit,NotebookEdit",
|
||||
DISALLOWED_TOOLS.join(","),
|
||||
"--strict-mcp-config",
|
||||
],
|
||||
{ cwd, stdio: ["pipe", "pipe", "inherit"] },
|
||||
platform,
|
||||
|
|
@ -285,12 +490,19 @@ export function runPrompt(
|
|||
const p = spawnFn(command, args, options);
|
||||
|
||||
let buf = "";
|
||||
const invoked = new Set();
|
||||
const invoked = [];
|
||||
let result = null;
|
||||
// Carried across chunks so the turn count is monotonic over the whole
|
||||
// stream rather than restarting at each read.
|
||||
let turnOffset = 0;
|
||||
p.stdout.on("data", (chunk) => {
|
||||
const parsed = collectSkillInvocations(buf + chunk.toString());
|
||||
const parsed = collectSkillInvocations(
|
||||
buf + chunk.toString(),
|
||||
turnOffset,
|
||||
);
|
||||
buf = parsed.rest;
|
||||
for (const payload of parsed.invoked) invoked.add(payload);
|
||||
turnOffset = parsed.nextTurn;
|
||||
for (const entry of parsed.invoked) invoked.push(entry);
|
||||
if (parsed.result !== null) result = parsed.result;
|
||||
});
|
||||
p.on("error", reject);
|
||||
|
|
@ -320,7 +532,122 @@ async function pool(items, n, fn) {
|
|||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a measured rate clears its bar.
|
||||
*
|
||||
* The comparison differs by case kind, and the difference is the point. A
|
||||
* first-move threshold is a floor to reach (`>=` 0.8 means four of five). A
|
||||
* chain threshold states "the pointer is taken more often than not", which is
|
||||
* strictly `> 0.5` — an inclusive compare would pass 2/4 whenever RUNS is even
|
||||
* and report a result the stated criterion does not license (Copilot).
|
||||
*
|
||||
* @param {number} rate
|
||||
* @param {number} threshold
|
||||
* @param {boolean} strict
|
||||
*/
|
||||
export function passesThreshold(rate, threshold, strict) {
|
||||
return strict ? rate > threshold : rate >= threshold;
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the whole report, and say how many cases fell short.
|
||||
*
|
||||
* Extracted and exported so the SEPARATION itself is testable. The acceptance
|
||||
* criterion of #2204 is that a hand-off rate is never folded into the
|
||||
* first-move headline, and until this was a function that claim had no
|
||||
* automated coverage at all: the tests exercised the scorers and the turn
|
||||
* budget while the reporting — the thing that could silently merge the two
|
||||
* measurements — lived inside `main` where nothing could reach it (Copilot).
|
||||
*
|
||||
* @param {object[]} cases
|
||||
* @param {{c: object, invoked: Iterable<string>}[]} results One per sample.
|
||||
* @param {Set<string> | null} ours
|
||||
* @param {{threshold: number, chainThreshold: number, chainMaxTurns: number}} opts
|
||||
* @returns {{ lines: string[], failed: number }}
|
||||
*/
|
||||
export function formatReport(cases, results, ours, opts) {
|
||||
const lines = [];
|
||||
let failed = 0;
|
||||
|
||||
const group = (members, heading, threshold, strict) => {
|
||||
if (members.length === 0) return 0;
|
||||
lines.push("", heading);
|
||||
let short = 0;
|
||||
for (const c of members) {
|
||||
const mine = results.filter((r) => r.c === c);
|
||||
const passes = mine.filter((r) => caseHit(c, r.invoked, ours)).length;
|
||||
const rate = mine.length === 0 ? 0 : passes / mine.length;
|
||||
const ok = mine.length > 0 && passesThreshold(rate, threshold, strict);
|
||||
if (!ok) short++;
|
||||
const label = isChainCase(c)
|
||||
? c.chain.join(" → ")
|
||||
: (c.expect ?? "(no skill)");
|
||||
lines.push(
|
||||
`${ok ? "PASS" : "FAIL"} ${(rate * 100).toFixed(0).padStart(3)}% ${label.padEnd(26)} ${c.prompt}`,
|
||||
);
|
||||
}
|
||||
return short;
|
||||
};
|
||||
|
||||
const direct = cases.filter((c) => !isChainCase(c));
|
||||
const chained = cases.filter(isChainCase);
|
||||
const directShort = group(
|
||||
direct,
|
||||
"First move (1 turn)",
|
||||
opts.threshold,
|
||||
false,
|
||||
);
|
||||
const chainedShort = group(
|
||||
chained,
|
||||
`Hand-off (${opts.chainMaxTurns} turns)`,
|
||||
opts.chainThreshold,
|
||||
true,
|
||||
);
|
||||
failed = directShort + chainedShort;
|
||||
|
||||
// Two numbers, never one. A hand-off is a second-hop load over many turns and
|
||||
// a first-move rate is the model's opening move; summing them would produce a
|
||||
// figure that describes neither, and a handful of hand-off cases would
|
||||
// quietly move a headline everyone reads as trigger reliability (#2204).
|
||||
lines.push("");
|
||||
if (direct.length > 0) {
|
||||
lines.push(
|
||||
`${direct.length - directShort}/${direct.length} first-move cases at or above ${opts.threshold * 100}%.`,
|
||||
);
|
||||
}
|
||||
lines.push(
|
||||
chained.length === 0
|
||||
? "No hand-off cases in this selection."
|
||||
: `${chained.length - chainedShort}/${chained.length} hand-off cases above ${opts.chainThreshold * 100}%.`,
|
||||
);
|
||||
return { lines, failed };
|
||||
}
|
||||
|
||||
async function main() {
|
||||
// The chain bar is strict, so 1.0 cannot be cleared by any run and would fail
|
||||
// every hand-off case while looking like a trigger problem. NaN and negative
|
||||
// values are rejected for the same reason from the other side: `Number("abc")`
|
||||
// is NaN, which fails every comparison and prints an `above NaN%` summary,
|
||||
// and a negative bar passes every chain unconditionally — both turn an
|
||||
// advertised knob into a measurement that quietly means nothing (Copilot).
|
||||
if (
|
||||
!Number.isFinite(CHAIN_THRESHOLD) ||
|
||||
CHAIN_THRESHOLD < 0 ||
|
||||
CHAIN_THRESHOLD >= 1
|
||||
) {
|
||||
console.error(
|
||||
`skills:eval — CHAIN_THRESHOLD must be a number in [0, 1) (got ${process.env.CHAIN_THRESHOLD ?? CHAIN_THRESHOLD}); it is a strict lower bound.`,
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
if (!Number.isFinite(THRESHOLD) || THRESHOLD < 0 || THRESHOLD > 1) {
|
||||
// The first-move bar is inclusive, so 1.0 is meetable and allowed; the
|
||||
// non-finite and negative cases fail the same way as above.
|
||||
console.error(
|
||||
`skills:eval — THRESHOLD must be a number in [0, 1] (got ${process.env.THRESHOLD ?? THRESHOLD}).`,
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
if (probeClaudeVersion(parseClaudeVersion) === null) {
|
||||
console.error(
|
||||
"skills:eval — no usable `claude` CLI on PATH. This eval needs one.",
|
||||
|
|
@ -339,27 +666,17 @@ async function main() {
|
|||
const jobs = cases.flatMap((c) => Array.from({ length: RUNS }, () => c));
|
||||
const results = await pool(jobs, CONCURRENCY, async (c) => ({
|
||||
c,
|
||||
invoked: await runPrompt(c.prompt),
|
||||
invoked: await runPrompt(c.prompt, {
|
||||
maxTurns: isChainCase(c) ? CHAIN_MAX_TURNS : 1,
|
||||
}),
|
||||
}));
|
||||
|
||||
let failed = 0;
|
||||
for (const c of cases) {
|
||||
const mine = results.filter((r) => r.c === c);
|
||||
const passes = mine.filter((r) =>
|
||||
sampleHit(c.expect, r.invoked, ours),
|
||||
).length;
|
||||
const rate = passes / mine.length;
|
||||
const ok = rate >= THRESHOLD;
|
||||
if (!ok) failed++;
|
||||
const label = c.expect ?? "(no skill)";
|
||||
console.log(
|
||||
`${ok ? "PASS" : "FAIL"} ${(rate * 100).toFixed(0).padStart(3)}% ${label.padEnd(20)} ${c.prompt}`,
|
||||
);
|
||||
}
|
||||
|
||||
console.log(
|
||||
`\n${cases.length - failed}/${cases.length} cases at or above ${THRESHOLD * 100}%.`,
|
||||
);
|
||||
const { lines, failed } = formatReport(cases, results, ours, {
|
||||
threshold: THRESHOLD,
|
||||
chainThreshold: CHAIN_THRESHOLD,
|
||||
chainMaxTurns: CHAIN_MAX_TURNS,
|
||||
});
|
||||
for (const line of lines) console.log(line);
|
||||
process.exit(failed > 0 ? 1 : 0);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -11,10 +11,16 @@
|
|||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import {
|
||||
caseHit,
|
||||
chainHit,
|
||||
formatReport,
|
||||
passesThreshold,
|
||||
collectCases,
|
||||
collectSkillInvocations,
|
||||
runRejection,
|
||||
|
|
@ -24,6 +30,20 @@ import {
|
|||
} from "./skill-eval.mjs";
|
||||
import { MIN_POSITIVE_CASES } from "./lib/skill-manifest.mjs";
|
||||
|
||||
/**
|
||||
* What a run records, one skill per assistant turn — the shape a genuine
|
||||
* hand-off has.
|
||||
*/
|
||||
const fired = (...names) =>
|
||||
names.map((n, i) => ({ payload: JSON.stringify({ skill: n }), turn: i + 1 }));
|
||||
|
||||
/** Several skills emitted together in ONE assistant turn. */
|
||||
const firedTogether = (turn, ...names) =>
|
||||
names.map((n) => ({ payload: JSON.stringify({ skill: n }), turn }));
|
||||
|
||||
/** The eval script itself, for the cases that must exercise `main`'s guards. */
|
||||
const SCRIPT_PATH = fileURLToPath(new URL("./skill-eval.mjs", import.meta.url));
|
||||
|
||||
const assistant = (...blocks) =>
|
||||
JSON.stringify({ type: "assistant", message: { content: blocks } });
|
||||
const skillUse = (name) => ({
|
||||
|
|
@ -36,8 +56,9 @@ test("collectSkillInvocations finds Skill tool_use payloads", () => {
|
|||
const { invoked } = collectSkillInvocations(
|
||||
assistant(skillUse("testing")) + "\n",
|
||||
);
|
||||
assert.equal(invoked.size, 1);
|
||||
assert.ok([...invoked][0].includes("testing"));
|
||||
assert.equal(invoked.length, 1);
|
||||
assert.ok(invoked[0].payload.includes("testing"));
|
||||
assert.equal(invoked[0].turn, 1);
|
||||
});
|
||||
|
||||
test("collectSkillInvocations ignores other tools and other event types", () => {
|
||||
|
|
@ -46,40 +67,194 @@ test("collectSkillInvocations ignores other tools and other event types", () =>
|
|||
"\n" +
|
||||
JSON.stringify({ type: "result", result: "Skill" }) +
|
||||
"\n";
|
||||
assert.equal(collectSkillInvocations(text).invoked.size, 0);
|
||||
assert.equal(collectSkillInvocations(text).invoked.length, 0);
|
||||
});
|
||||
|
||||
test("collectSkillInvocations survives malformed and blank lines", () => {
|
||||
const text = "not json\n\n" + assistant(skillUse("local-dev")) + "\n";
|
||||
const { invoked } = collectSkillInvocations(text);
|
||||
assert.equal(invoked.size, 1);
|
||||
assert.equal(invoked.length, 1);
|
||||
});
|
||||
|
||||
test("collectSkillInvocations holds back a trailing partial line", () => {
|
||||
const whole = assistant(skillUse("local-dev"));
|
||||
const first = collectSkillInvocations(whole.slice(0, 20));
|
||||
assert.equal(first.invoked.size, 0);
|
||||
assert.equal(first.invoked.length, 0);
|
||||
assert.equal(first.rest, whole.slice(0, 20));
|
||||
// Feeding the remainder back with the held-over prefix completes the event.
|
||||
const second = collectSkillInvocations(first.rest + whole.slice(20) + "\n");
|
||||
assert.equal(second.invoked.size, 1);
|
||||
assert.equal(second.invoked.length, 1);
|
||||
});
|
||||
|
||||
test("collectSkillInvocations tolerates a tool_use with no input", () => {
|
||||
const text = assistant({ type: "tool_use", name: "Skill" }) + "\n";
|
||||
assert.deepEqual([...collectSkillInvocations(text).invoked], ["{}"]);
|
||||
assert.deepEqual(collectSkillInvocations(text).invoked, [
|
||||
{ payload: "{}", turn: 1 },
|
||||
]);
|
||||
});
|
||||
|
||||
test("sampleHit scores positive and negative cases", () => {
|
||||
const fired = new Set(['{"skill":"testing"}']);
|
||||
const none = new Set();
|
||||
assert.equal(sampleHit("testing", fired), true);
|
||||
assert.equal(sampleHit("local-dev", fired), false);
|
||||
const hit = fired("testing");
|
||||
const none = [];
|
||||
assert.equal(sampleHit("testing", hit), true);
|
||||
assert.equal(sampleHit("local-dev", hit), false);
|
||||
assert.equal(sampleHit(null, none), true);
|
||||
assert.equal(sampleHit(null, fired), false);
|
||||
assert.equal(sampleHit(null, hit), false);
|
||||
assert.equal(sampleHit("testing", none), false);
|
||||
});
|
||||
|
||||
test("collectSkillInvocations preserves order and repeats", () => {
|
||||
// A hand-off case asserts one skill was loaded AFTER another, so occurrence
|
||||
// order is the observation. Deduplicating into a Set would make the B, A, B
|
||||
// run below indistinguishable from one that never reached B from A.
|
||||
const text =
|
||||
[
|
||||
assistant(skillUse("test-servers")),
|
||||
assistant(skillUse("testing")),
|
||||
assistant(skillUse("test-servers")),
|
||||
].join("\n") + "\n";
|
||||
const { invoked } = collectSkillInvocations(text);
|
||||
assert.deepEqual(
|
||||
invoked.map((e) => [JSON.parse(e.payload).skill, e.turn]),
|
||||
[
|
||||
["test-servers", 1],
|
||||
["testing", 2],
|
||||
["test-servers", 3],
|
||||
],
|
||||
);
|
||||
});
|
||||
|
||||
test("collectSkillInvocations tags each invocation with its assistant turn", () => {
|
||||
// Two `Skill` blocks in ONE message share a turn: the model chose both before
|
||||
// seeing either result, so nothing in that message can have caused anything
|
||||
// else in it. The turn is what lets `chainHit` tell that apart from a
|
||||
// hand-off; a flat index cannot.
|
||||
const text =
|
||||
[
|
||||
assistant(skillUse("testing"), skillUse("test-servers")),
|
||||
assistant(skillUse("board-ops")),
|
||||
].join("\n") + "\n";
|
||||
const { invoked, nextTurn } = collectSkillInvocations(text);
|
||||
assert.deepEqual(
|
||||
invoked.map((e) => [JSON.parse(e.payload).skill, e.turn]),
|
||||
[
|
||||
["testing", 1],
|
||||
["test-servers", 1],
|
||||
["board-ops", 2],
|
||||
],
|
||||
);
|
||||
// The count carries across chunks, so a stream read in pieces stays monotonic.
|
||||
assert.equal(nextTurn, 2);
|
||||
const more = collectSkillInvocations(
|
||||
assistant(skillUse("local-dev")) + "\n",
|
||||
nextTurn,
|
||||
);
|
||||
assert.equal(more.invoked[0].turn, 3);
|
||||
});
|
||||
|
||||
test("chainHit refuses two skills loaded in the same assistant turn", () => {
|
||||
// The finding this pins: the model can emit several `tool_use` blocks in one
|
||||
// message, and it has NOT seen the first skill's body when it does. So an
|
||||
// [A, B] pair from one message is two parallel guesses, and scoring it as a
|
||||
// hand-off would report a causal link that cannot exist — a case that would
|
||||
// keep passing after the pointer in `testing` was deleted.
|
||||
assert.equal(
|
||||
chainHit(
|
||||
["testing", "test-servers"],
|
||||
firedTogether(1, "testing", "test-servers"),
|
||||
),
|
||||
false,
|
||||
);
|
||||
// The same two skills one turn apart is the real thing.
|
||||
assert.equal(
|
||||
chainHit(
|
||||
["testing", "test-servers"],
|
||||
[...firedTogether(1, "testing"), ...firedTogether(2, "test-servers")],
|
||||
),
|
||||
true,
|
||||
);
|
||||
// A same-turn pair does not poison a later genuine hand-off.
|
||||
assert.equal(
|
||||
chainHit(
|
||||
["testing", "test-servers"],
|
||||
[
|
||||
...firedTogether(1, "testing", "test-servers"),
|
||||
...firedTogether(2, "test-servers"),
|
||||
],
|
||||
),
|
||||
true,
|
||||
);
|
||||
// Only the FIRST link is unconstrained; every later one needs a new turn.
|
||||
assert.equal(
|
||||
chainHit(
|
||||
["local-dev", "testing", "test-servers"],
|
||||
[
|
||||
...firedTogether(1, "local-dev"),
|
||||
...firedTogether(2, "testing", "test-servers"),
|
||||
],
|
||||
),
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test("chainHit wants the links in order", () => {
|
||||
assert.equal(chainHit(["testing", "test-servers"], fired("testing")), false);
|
||||
assert.equal(
|
||||
chainHit(["testing", "test-servers"], fired("testing", "test-servers")),
|
||||
true,
|
||||
);
|
||||
// The reverse hand-off is a different claim and must not score.
|
||||
assert.equal(
|
||||
chainHit(["testing", "test-servers"], fired("test-servers", "testing")),
|
||||
false,
|
||||
);
|
||||
assert.equal(chainHit(["testing", "test-servers"], []), false);
|
||||
});
|
||||
|
||||
test("chainHit matches a subsequence, not a prefix or a contiguous run", () => {
|
||||
// The model is free to load something before the chain starts, and something
|
||||
// unrelated in between — neither changes the fact that A led to B.
|
||||
assert.equal(
|
||||
chainHit(
|
||||
["testing", "test-servers"],
|
||||
fired("local-dev", "testing", "board-ops", "test-servers"),
|
||||
),
|
||||
true,
|
||||
);
|
||||
// And a repeat of the first link before it does not consume the match.
|
||||
assert.equal(
|
||||
chainHit(
|
||||
["testing", "test-servers"],
|
||||
fired("test-servers", "testing", "test-servers"),
|
||||
),
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test("caseHit routes each case shape to its own scorer", () => {
|
||||
const ours = new Set(["testing", "test-servers"]);
|
||||
const run = fired("testing", "test-servers");
|
||||
assert.equal(
|
||||
caseHit({ chain: ["testing", "test-servers"] }, run, ours),
|
||||
true,
|
||||
);
|
||||
assert.equal(
|
||||
caseHit({ chain: ["test-servers", "testing"] }, run, ours),
|
||||
false,
|
||||
);
|
||||
assert.equal(caseHit({ expect: "testing" }, run, ours), true);
|
||||
assert.equal(caseHit({ expect: null }, run, ours), false);
|
||||
// A chained case says nothing about foreign skills, unlike a negative one.
|
||||
assert.equal(
|
||||
caseHit(
|
||||
{ chain: ["testing", "test-servers"] },
|
||||
fired("testing", "my-personal-notes", "test-servers"),
|
||||
ours,
|
||||
),
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test("invokedSkillNames matches structurally, not by substring", () => {
|
||||
// `{"skill":"not-testing"}` contains "testing" and must NOT count — a
|
||||
// substring match inflates the measured hit rate with invocations of a
|
||||
|
|
@ -87,16 +262,10 @@ test("invokedSkillNames matches structurally, not by substring", () => {
|
|||
assert.deepEqual(invokedSkillNames('{"skill":"not-testing"}'), [
|
||||
"not-testing",
|
||||
]);
|
||||
assert.equal(
|
||||
sampleHit("testing", new Set(['{"skill":"not-testing"}'])),
|
||||
false,
|
||||
);
|
||||
assert.equal(sampleHit("testing", new Set(['{"skill":"testing"}'])), true);
|
||||
assert.equal(sampleHit("testing", fired("not-testing")), false);
|
||||
assert.equal(sampleHit("testing", fired("testing")), true);
|
||||
// The field name is not assumed, so any string value is a candidate.
|
||||
assert.equal(
|
||||
sampleHit("testing", new Set(['{"name":"testing","args":""}'])),
|
||||
true,
|
||||
);
|
||||
assert.equal(sampleHit("testing", ['{"name":"testing","args":""}']), true);
|
||||
});
|
||||
|
||||
test("invokedSkillNames tolerates payloads it cannot read", () => {
|
||||
|
|
@ -180,6 +349,185 @@ test("runPrompt collects invocations across chunk boundaries", async () => {
|
|||
assert.equal(sampleHit("testing", invoked), true);
|
||||
});
|
||||
|
||||
test("runPrompt gives a hand-off case a wider turn budget", () => {
|
||||
// `--max-turns 1` is what makes a first-move case a first-move case; a chain
|
||||
// that only ever gets one turn can never observe a second-hop load.
|
||||
const seen = [];
|
||||
for (const opts of [{}, { maxTurns: 14 }]) {
|
||||
runPrompt("p", {
|
||||
...opts,
|
||||
spawnFn: (_c, args) => {
|
||||
seen.push(args[args.indexOf("--max-turns") + 1]);
|
||||
const c = new EventEmitter();
|
||||
c.stdout = new EventEmitter();
|
||||
c.stdin = { end: () => {} };
|
||||
queueMicrotask(() => c.emit("close", 0));
|
||||
return c;
|
||||
},
|
||||
}).catch(() => {});
|
||||
}
|
||||
assert.deepEqual(seen, ["1", "14"]);
|
||||
});
|
||||
|
||||
test("runPrompt keeps the run read-only across every turn", () => {
|
||||
// A wider budget removes the containment `--max-turns 1` was doing on its
|
||||
// own, so the deny list has to cover the agentic and network tools too —
|
||||
// `Task` in particular, whose subagent this flag does not reach.
|
||||
let denied;
|
||||
runPrompt("p", {
|
||||
spawnFn: (_c, args) => {
|
||||
denied = args[args.indexOf("--disallowedTools") + 1].split(",");
|
||||
const c = new EventEmitter();
|
||||
c.stdout = new EventEmitter();
|
||||
c.stdin = { end: () => {} };
|
||||
queueMicrotask(() => c.emit("close", 0));
|
||||
return c;
|
||||
},
|
||||
}).catch(() => {});
|
||||
for (const tool of ["Bash", "Write", "Edit", "NotebookEdit", "Task"]) {
|
||||
assert.ok(denied.includes(tool), `${tool} must be denied`);
|
||||
}
|
||||
});
|
||||
|
||||
test("runPrompt bounds the run by what it needs, not only by what it forbids", () => {
|
||||
// A deny list only names the tools known when it was written. This checkout
|
||||
// configures an HTTP `mcp-docs` server in `.mcp.json`, and a contributor's
|
||||
// own MCP servers and plugins add more that no list here has seen — over 14
|
||||
// turns those can reach the network or mutate state.
|
||||
let args;
|
||||
runPrompt("p", {
|
||||
spawnFn: (_c, a) => {
|
||||
args = a;
|
||||
const c = new EventEmitter();
|
||||
c.stdout = new EventEmitter();
|
||||
c.stdin = { end: () => {} };
|
||||
queueMicrotask(() => c.emit("close", 0));
|
||||
return c;
|
||||
},
|
||||
}).catch(() => {});
|
||||
// `--tools` is the availability filter and is what actually bounds the run.
|
||||
// `--allowedTools` only pre-approves: a tool a user's or a plugin's settings
|
||||
// already permit would still be reachable across 14 turns without this.
|
||||
for (const flag of ["--tools", "--allowedTools"]) {
|
||||
assert.deepEqual(args[args.indexOf(flag) + 1].split(","), [
|
||||
"Read",
|
||||
"Glob",
|
||||
"Grep",
|
||||
"Skill",
|
||||
]);
|
||||
}
|
||||
// No `--mcp-config` accompanies it, so this drops every configured server.
|
||||
assert.ok(args.includes("--strict-mcp-config"));
|
||||
assert.ok(!args.includes("--mcp-config"));
|
||||
});
|
||||
|
||||
test("a malformed threshold is rejected rather than silently measured", () => {
|
||||
// `Number("abc")` is NaN, which fails every comparison and would print an
|
||||
// `above NaN%` summary; a negative bar passes every chain unconditionally.
|
||||
// Either turns an advertised env knob into a measurement that means nothing.
|
||||
const run = (env) =>
|
||||
spawnSync(process.execPath, [SCRIPT_PATH], {
|
||||
encoding: "utf8",
|
||||
env: { ...process.env, ...env },
|
||||
});
|
||||
|
||||
for (const bad of ["abc", "-0.5", "1", "1.5"]) {
|
||||
const { status, stderr } = run({ CHAIN_THRESHOLD: bad });
|
||||
assert.equal(status, 1, `CHAIN_THRESHOLD=${bad} must be rejected`);
|
||||
assert.match(stderr, /CHAIN_THRESHOLD must be a number in \[0, 1\)/);
|
||||
assert.ok(stderr.includes(bad), "the message names the offending value");
|
||||
}
|
||||
// The first-move bar is inclusive, so 1 is legitimate there and only the
|
||||
// nonsensical values are refused.
|
||||
for (const bad of ["abc", "-1", "1.5"]) {
|
||||
const { status, stderr } = run({ THRESHOLD: bad });
|
||||
assert.equal(status, 1, `THRESHOLD=${bad} must be rejected`);
|
||||
assert.match(stderr, /THRESHOLD must be a number in \[0, 1\]/);
|
||||
}
|
||||
});
|
||||
|
||||
test("passesThreshold is a floor for a first move and strictly above for a chain", () => {
|
||||
// "More often than not" is `> 0.5`. An inclusive compare passes 2/4 whenever
|
||||
// RUNS is even, reporting a result the stated criterion does not license.
|
||||
assert.equal(passesThreshold(0.5, 0.5, true), false);
|
||||
assert.equal(passesThreshold(2 / 3, 0.5, true), true);
|
||||
// A first-move threshold is a floor to REACH: 4/5 clears 0.8 exactly.
|
||||
assert.equal(passesThreshold(0.8, 0.8, false), true);
|
||||
assert.equal(passesThreshold(0.6, 0.8, false), false);
|
||||
});
|
||||
|
||||
const OPTS = { threshold: 0.8, chainThreshold: 0.5, chainMaxTurns: 14 };
|
||||
/** `RUNS` samples of one case, `hits` of which fired the whole chain/skill. */
|
||||
const samples = (c, hits, runs) =>
|
||||
Array.from({ length: runs }, (_, i) => ({
|
||||
c,
|
||||
invoked: i < hits ? fired(...(c.chain ?? [c.expect])) : [],
|
||||
}));
|
||||
|
||||
test("the report keeps the two measurements in separate columns", () => {
|
||||
// The acceptance criterion of #2204: a hand-off rate is never folded into
|
||||
// the first-move headline. Nothing covered this while it lived in `main`.
|
||||
const direct = { prompt: "d", expect: "test-servers" };
|
||||
const chain = { prompt: "c", chain: ["testing", "test-servers"] };
|
||||
const { lines, failed } = formatReport(
|
||||
[direct, chain],
|
||||
[...samples(direct, 3, 3), ...samples(chain, 1, 3)],
|
||||
new Set(["testing", "test-servers"]),
|
||||
OPTS,
|
||||
);
|
||||
const text = lines.join("\n");
|
||||
assert.match(text, /First move \(1 turn\)/);
|
||||
assert.match(text, /Hand-off \(14 turns\)/);
|
||||
assert.match(text, /1\/1 first-move cases at or above 80%\./);
|
||||
assert.match(text, /0\/1 hand-off cases above 50%\./);
|
||||
// One summary line per kind, and no line that merges them.
|
||||
assert.equal(text.match(/cases (at or above|above)/g).length, 2);
|
||||
assert.equal(failed, 1, "the chained case is short, the direct one is not");
|
||||
});
|
||||
|
||||
test("each group is scored against its own threshold", () => {
|
||||
// 2/3 clears the chain bar strictly but would fail the first-move bar, so a
|
||||
// single shared threshold would misreport whichever kind it was not tuned for.
|
||||
const direct = { prompt: "d", expect: "test-servers" };
|
||||
const chain = { prompt: "c", chain: ["testing", "test-servers"] };
|
||||
const { lines, failed } = formatReport(
|
||||
[direct, chain],
|
||||
[...samples(direct, 2, 3), ...samples(chain, 2, 3)],
|
||||
new Set(["testing", "test-servers"]),
|
||||
OPTS,
|
||||
);
|
||||
const text = lines.join("\n");
|
||||
assert.match(text, /FAIL\s+67%\s+test-servers/);
|
||||
assert.match(text, /PASS\s+67%\s+testing → test-servers/);
|
||||
assert.equal(failed, 1);
|
||||
});
|
||||
|
||||
test("a single-kind selection reports only that kind, and says so", () => {
|
||||
const direct = { prompt: "d", expect: "test-servers" };
|
||||
const only = formatReport(
|
||||
[direct],
|
||||
samples(direct, 3, 3),
|
||||
new Set(["test-servers"]),
|
||||
OPTS,
|
||||
);
|
||||
assert.match(only.lines.join("\n"), /No hand-off cases in this selection\./);
|
||||
assert.doesNotMatch(only.lines.join("\n"), /Hand-off \(14 turns\)/);
|
||||
assert.equal(only.failed, 0);
|
||||
|
||||
// And a chain-only selection prints no first-move headline or summary.
|
||||
const chain = { prompt: "c", chain: ["testing", "test-servers"] };
|
||||
const chainOnly = formatReport(
|
||||
[chain],
|
||||
samples(chain, 3, 3),
|
||||
new Set(["testing", "test-servers"]),
|
||||
OPTS,
|
||||
);
|
||||
const text = chainOnly.lines.join("\n");
|
||||
assert.doesNotMatch(text, /first-move cases/);
|
||||
assert.match(text, /1\/1 hand-off cases above 50%\./);
|
||||
assert.equal(chainOnly.failed, 0);
|
||||
});
|
||||
|
||||
test("runPrompt rejects a run that produced no terminal result", async () => {
|
||||
await assert.rejects(
|
||||
runPrompt("p", { spawnFn: fakeSpawn({ code: 1 }) }),
|
||||
|
|
@ -211,12 +559,12 @@ test("a negative case ignores skills that are not this repo's", () => {
|
|||
// a negative prompt says nothing about these skills — failing on it would be
|
||||
// a false failure about someone else's environment.
|
||||
const ours = new Set(["testing", "local-dev"]);
|
||||
const foreign = new Set(['{"skill":"my-personal-notes"}']);
|
||||
const mine = new Set(['{"skill":"testing"}']);
|
||||
const foreign = fired("my-personal-notes");
|
||||
const mine = fired("testing");
|
||||
|
||||
assert.equal(sampleHit(null, foreign, ours), true);
|
||||
assert.equal(sampleHit(null, mine, ours), false);
|
||||
assert.equal(sampleHit(null, new Set(), ours), true);
|
||||
assert.equal(sampleHit(null, [], ours), true);
|
||||
// A positive case is unaffected: it names the skill it wants.
|
||||
assert.equal(sampleHit("testing", mine, ours), true);
|
||||
assert.equal(sampleHit("testing", foreign, ours), false);
|
||||
|
|
@ -318,10 +666,7 @@ test("focused mode narrows the cases but not the repo's own skill set", () => {
|
|||
["a+0", "a+1", "a+2", "a+3", "a+4", "a-"],
|
||||
);
|
||||
// The consequence, stated as the assertion that matters:
|
||||
assert.equal(
|
||||
sampleHit(null, new Set(['{"skill":"beta"}']), focused.ours),
|
||||
false,
|
||||
);
|
||||
assert.equal(sampleHit(null, fired("beta"), focused.ours), false);
|
||||
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
|
@ -378,7 +723,7 @@ test("a name-only skill is not part of the repo's model-invoked set", () => {
|
|||
const { ours } = collectCases(undefined, root);
|
||||
assert.equal(ours.has("gamma"), false);
|
||||
// So its firing does not fail a negative case — it cannot fire on its own.
|
||||
assert.equal(sampleHit(null, new Set(['{"skill":"gamma"}']), ours), true);
|
||||
assert.equal(sampleHit(null, fired("gamma"), ours), true);
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -130,6 +130,36 @@ test("fails a skill that does not declare its invocation mode", () => {
|
|||
rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("a hand-off case is checked against the whole model-invoked set", () => {
|
||||
// The ordering trap this pins: `zeta` sorts AFTER `beta`, so validating each
|
||||
// file as it is read would reject a chain through `zeta` as unknown purely
|
||||
// because of where the alphabet put it.
|
||||
const withChain = (chain) =>
|
||||
JSON.stringify([
|
||||
...JSON.parse(goodEvals("beta")),
|
||||
{ prompt: "reached the long way", chain },
|
||||
]);
|
||||
|
||||
const ok = fixture({
|
||||
beta: { skill: modelInvoked("beta"), evals: withChain(["zeta", "beta"]) },
|
||||
zeta: { skill: modelInvoked("zeta"), evals: goodEvals("zeta") },
|
||||
});
|
||||
const passed = run(ok);
|
||||
assert.equal(passed.code, 0, passed.out);
|
||||
rmSync(ok, { recursive: true, force: true });
|
||||
|
||||
// A link the model cannot invoke can never fire, so it would score a
|
||||
// permanent 0% that reads as a description problem rather than a typo.
|
||||
const bad = fixture({
|
||||
alpha: { skill: byName("alpha") },
|
||||
beta: { skill: modelInvoked("beta"), evals: withChain(["alpha", "beta"]) },
|
||||
});
|
||||
const failed = run(bad);
|
||||
assert.equal(failed.code, 1);
|
||||
assert.match(failed.out, /`alpha`, which is not a model-invoked skill/);
|
||||
rmSync(bad, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("fails a model-invoked skill with no eval cases", () => {
|
||||
const dir = fixture({ beta: { skill: modelInvoked("beta") } });
|
||||
const { code, out } = run(dir);
|
||||
|
|
|
|||
|
|
@ -254,6 +254,8 @@ function main(argv = process.argv.slice(2)) {
|
|||
}
|
||||
|
||||
const parsed = [];
|
||||
const evalFiles = [];
|
||||
const modelInvokedDirs = new Set();
|
||||
for (const dir of dirs) {
|
||||
const file = path.join(SKILLS_DIR, dir, "SKILL.md");
|
||||
if (!existsSync(file)) {
|
||||
|
|
@ -266,6 +268,7 @@ function main(argv = process.argv.slice(2)) {
|
|||
parsed.push(skill);
|
||||
|
||||
if (skill.modelInvoked) {
|
||||
modelInvokedDirs.add(dir);
|
||||
const evalsFile = path.join(SKILLS_DIR, dir, "evals", "evals.json");
|
||||
if (!existsSync(evalsFile)) {
|
||||
failures.push(
|
||||
|
|
@ -280,9 +283,21 @@ function main(argv = process.argv.slice(2)) {
|
|||
failures.push(`${dir}/evals/evals.json: not valid JSON — ${e.message}`);
|
||||
continue;
|
||||
}
|
||||
for (const e of validateEvalCases(dir, cases)) {
|
||||
failures.push(`${dir}/evals/evals.json: ${e}`);
|
||||
}
|
||||
evalFiles.push({ dir, cases });
|
||||
}
|
||||
}
|
||||
|
||||
// Validated after the loop, not inside it: a hand-off case names other
|
||||
// skills, and checking those links needs the model-invoked set complete.
|
||||
// Inside the loop the check would depend on directory order — `test-servers`
|
||||
// sorts before `testing`, so a chain through `testing` would be rejected as
|
||||
// unknown purely because of where the alphabet put it.
|
||||
// Keyed on the DIRECTORY name, which is what a chain link names and what a
|
||||
// skill is addressed by; the frontmatter `name` can be absent or disagree,
|
||||
// and either would silently shrink the set a chain is checked against.
|
||||
for (const { dir, cases } of evalFiles) {
|
||||
for (const e of validateEvalCases(dir, cases, modelInvokedDirs)) {
|
||||
failures.push(`${dir}/evals/evals.json: ${e}`);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue