docs: address Copilot review round 3 on #2203
Replaces the last `project-structure` case that `AGENTS.md` answers — a helper used by the CLI and TUI belongs in `core/` per its own directory tree. This was also the only case below 100% in the previous suite, so the static reading and the score agreed. The replacement targets the skill's first stated purpose, locating code, which `AGENTS.md` gives no method for; it probed 3/3 and now measures 100%. Extends the truncation guard's tests to `when_to_use`. The guard loops over both fields but only `description` was pinned, so dropping the second from that list would have gone unnoticed. The new case pairs a quoted description with an unquoted `when_to_use` and asserts the description is not implicated. Scopes the negative-case requirement in the prose to model-invoked skills; round 2 scoped the checklist but left this sentence saying "every skill file". Refreshes the stale `~2.8k of 4k` listing figure. The other instance is past-tense about the three-to-nine experiment and stays, now said explicitly so two numbers do not look like a disagreement. Records the eval's real blind spot: all three cases review identified as `AGENTS.md`-answerable were scoring 80-100% when flagged, so a high score does not clear a case of overlap. The eval can show a case failing; it cannot show a passing case is well-aimed. RUNS=5 skills:eval: 63/63. local:gate passes, 34/34. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
880be3acdd
commit
104a05dd7f
4 changed files with 54 additions and 5 deletions
|
|
@ -16,7 +16,7 @@
|
|||
"expect": "project-structure"
|
||||
},
|
||||
{
|
||||
"prompt": "Does a helper that only the CLI and TUI use belong in core or in one of the clients?",
|
||||
"prompt": "I need to find where a feature is implemented but I do not know which client owns it. How do I work that out in this repo?",
|
||||
"expect": "project-structure"
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -187,7 +187,7 @@ that from happening:
|
|||
skill at all: it is absent from the listing and the Skill tool refuses it. The
|
||||
costs are asymmetric — a spurious load costs ~250 characters, a missed one
|
||||
costs a wrong base branch or an unsigned commit — and the budget is not tight
|
||||
(nine of the ten are model-invoked today and total ~2.8k of 4k). Reserve
|
||||
(nine of the ten are model-invoked today and total ~3.2k of 4k). Reserve
|
||||
`true` for a procedure that is genuinely only ever started deliberately —
|
||||
`release` is the only one left, because nobody cuts a release by implication.
|
||||
⚠️ **A `true` skill cannot be reached by another skill either.** If a
|
||||
|
|
@ -198,7 +198,8 @@ that from happening:
|
|||
of the ones already there: `project-structure` fell from 100% to 0% on two
|
||||
cases (n=4) and `testing` from 3/5 to 2/5, while the six new skills all
|
||||
measured 100% and every negative case stayed clean. So the ceiling is
|
||||
attention, not characters — we were at 2.8k of a 4k budget throughout. Adding
|
||||
attention, not characters — we were at 2.8k of a 4k budget throughout *that
|
||||
experiment* (it is ~3.2k now; the point is that nothing was near the cap). Adding
|
||||
a skill therefore has a cost paid by the *existing* ones, which only
|
||||
`skills:eval` can see. **Re-run the full eval after any flip _or description
|
||||
edit_**, not just the changed skill's own cases.
|
||||
|
|
|
|||
|
|
@ -122,6 +122,26 @@ oscillates — and looks like eval noise rather than a defect in the case. Two o
|
|||
| "I added a dependency and the TUI bundle broke at import time. **Where should it have gone?**" | 33 / 67 / 100 / 100 / 60% | Dependency placement is stated in full in `AGENTS.md`; only the diagnosis is the skill's |
|
||||
| "I just pulled and a client's dependencies look out of sync. **What do I need to run?**" | 80 / 80 / 60% | `AGENTS.md` says a single root `npm install` is all you need |
|
||||
|
||||
⚠️ **A high score does not clear a case of overlap, and this is the eval's real
|
||||
blind spot.** The two rules above — outright overlap pins a case near 0%, partial
|
||||
overlap makes it oscillate — describe what the *scores* look like once you already
|
||||
suspect a case. They are not a detector. Across code review of this PR, three
|
||||
committed cases were identified as `AGENTS.md`-answerable by *reading* them, and
|
||||
all three were scoring **80–100%** at the time:
|
||||
|
||||
| Case | Score when flagged |
|
||||
| --- | --- |
|
||||
| `project-structure`: "Does a new Node-only backend piece for the web client belong in core…?" | 80 → 100% |
|
||||
| `project-structure`: "Does a helper that only the CLI and TUI use belong in core…?" | 80% |
|
||||
| `issue-create`: "What do I need to do beyond `gh issue create`…?" | 100 / 100 / 100% |
|
||||
|
||||
The first two were re-aimed; the third was kept, because a case that holds at
|
||||
100% across three independent suites is measuring *something* the rules file does
|
||||
not supply on its own. But the lesson stands either way: **the eval cannot tell
|
||||
you a passing case is well-aimed.** It can only tell you a case is failing. Read
|
||||
every new case against `AGENTS.md` by hand before committing it, and do not let a
|
||||
green suite substitute for that.
|
||||
|
||||
Both stabilised at 100% once re-aimed at what only the skill holds — "what do I
|
||||
**check**" instead of "where should it have **gone**", and the worktree install
|
||||
trap instead of the install command. If a case keeps moving between runs while
|
||||
|
|
@ -147,8 +167,9 @@ neighbour and the neighbour is right, the case is aimed at the wrong skill.
|
|||
|
||||
### Negative cases
|
||||
|
||||
Every skill file needs at least one `"expect": null` case; a skill that fires on
|
||||
everything is a context regression nobody notices by hand. Keep them plainly
|
||||
Every **model-invoked** skill's eval file needs at least one `"expect": null`
|
||||
case; a skill that fires on everything is a context regression nobody notices by
|
||||
hand. (A name-only skill has no eval file to put one in — see the checklist.) Keep them plainly
|
||||
unrelated to the repo (arithmetic, trivia, a one-line refactor). All 18 in this
|
||||
repo have held at 100% through every reshaping so far — if one starts firing, a
|
||||
description has grown too broad.
|
||||
|
|
|
|||
|
|
@ -443,3 +443,30 @@ test("a model-invoked skill needs at least MIN_POSITIVE_CASES positives", () =>
|
|||
/no positive case expects/,
|
||||
);
|
||||
});
|
||||
|
||||
test("the truncation guard covers `when_to_use`, not just `description`", () => {
|
||||
// The guard loops over both fields, so a regression that dropped `when_to_use`
|
||||
// from that list would be invisible to the description-only tests above.
|
||||
const withHash = [
|
||||
"---",
|
||||
"name: alpha",
|
||||
'description: "safe because quoted"',
|
||||
"when_to_use: Use when filing against board #28 or board #11.",
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
const parsed = parseSkill("alpha", withHash);
|
||||
assert.equal(parsed.whenToUse, "Use when filing against board");
|
||||
assert.match(parsed.errors.join(" "), /`when_to_use` is truncated/);
|
||||
// The description is quoted, so it must not be implicated.
|
||||
assert.doesNotMatch(parsed.errors.join(" "), /`description` is truncated/);
|
||||
|
||||
const quoted = withHash.replace(
|
||||
/^when_to_use: (.*)$/m,
|
||||
(_m, w) => `when_to_use: "${w}"`,
|
||||
);
|
||||
const ok = parseSkill("alpha", quoted);
|
||||
assert.deepEqual(ok.errors, []);
|
||||
assert.equal(ok.whenToUse, "Use when filing against board #28 or board #11.");
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue