docs: address Copilot review round 7 on #2203
Strip YAML properties (tags, anchors, in either order) before deciding whether a description scalar is quoted — testing the first character alone read `© "Counts # safely"` as unquoted and rejected a `#` the quotes make harmless. Count the positive-case floor by distinct prompt rather than by entry: the floor exists for breadth, so five copies of one prompt cleared it while exercising exactly the single trigger it is meant to look past. Compare on collapsed, case-folded text so trivial edits do not read as a new situation. Re-aim the two positive prompts that measured the wrong thing — the project-structure case now poses a concrete "which piece handles what" question, and the test-servers case names running a real MCP server. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SyBPs7caoMcyvnp9ArMBhV Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
5d3571597a
commit
0febaa0f6f
4 changed files with 97 additions and 8 deletions
|
|
@ -1,6 +1,6 @@
|
|||
[
|
||||
{
|
||||
"prompt": "This repo has three clients over a shared core. How should I orient myself in it before changing anything?",
|
||||
"prompt": "Something in the web client's Node backend needs changing but I do not know which piece handles what. How do I work that out?",
|
||||
"expect": "project-structure"
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@
|
|||
"expect": "test-servers"
|
||||
},
|
||||
{
|
||||
"prompt": "I need a real server to exercise this change against by hand. What do I run?",
|
||||
"prompt": "I need to run a real MCP server locally to exercise this change by hand. Which one do I run, and how do I start it?",
|
||||
"expect": "test-servers"
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -177,11 +177,19 @@ export function parseSkill(dirName, text) {
|
|||
);
|
||||
if (start === -1) continue;
|
||||
const head = lines[start].slice(field.length + 1).trim();
|
||||
// A scalar may carry YAML properties — a tag, an anchor, or both, in either
|
||||
// order — before its value. Testing the first character alone read
|
||||
// `© "Counts # safely"` as unquoted and then rejected the `#` that the
|
||||
// quotes make harmless, so strip the properties before deciding (Copilot).
|
||||
const value = head.replace(
|
||||
/^(?:(?:!(?:!?[\w:.-]*|<[^>]*>)|&[^\s]+)\s+)+/,
|
||||
"",
|
||||
);
|
||||
// Only an unquoted scalar can lose text to a comment; a quoted one is safe,
|
||||
// and a block scalar (`|`/`>`) is read literally rather than comment-stripped.
|
||||
if (/^["'|>]/.test(head)) continue;
|
||||
if (/^["'|>]/.test(value)) continue;
|
||||
// Continuation lines are indented and are not the next `key:` of the map.
|
||||
const parts = [head];
|
||||
const parts = [value];
|
||||
for (let i = start + 1; i < lines.length; i++) {
|
||||
const l = lines[i];
|
||||
if (!/^[ \t]/.test(l) || l.trim() === "") break;
|
||||
|
|
@ -307,18 +315,31 @@ export function validateEvalCases(skillName, cases) {
|
|||
);
|
||||
}
|
||||
});
|
||||
const positives = cases.filter((c) => c && c.expect === skillName).length;
|
||||
const positiveCases = cases.filter((c) => c && c.expect === skillName);
|
||||
const positives = positiveCases.length;
|
||||
// The floor exists for breadth, so it has to count *phrasings*, not entries:
|
||||
// five copies of one prompt clear an entry count while exercising exactly the
|
||||
// one trigger the floor is meant to look past (Copilot). Compare on collapsed,
|
||||
// case-folded text so trivial edits do not read as a new situation.
|
||||
const distinctPositives = new Set(
|
||||
positiveCases.map((c) =>
|
||||
String(c.prompt ?? "")
|
||||
.toLowerCase()
|
||||
.replace(/\s+/g, " ")
|
||||
.trim(),
|
||||
),
|
||||
).size;
|
||||
const negatives = cases.filter((c) => c && c.expect === null).length;
|
||||
if (positives === 0) {
|
||||
errors.push(`no positive case expects \`${skillName}\``);
|
||||
} else if (positives < MIN_POSITIVE_CASES) {
|
||||
} else if (distinctPositives < MIN_POSITIVE_CASES) {
|
||||
// A non-zero floor is not enough: one prompt measures one phrasing. Since
|
||||
// each prompt is scored on its own `passes / RUNS`, extra prompts do not
|
||||
// make any single rate steadier — they cover more of the ways someone might
|
||||
// reach the skill, so a description that only fires on one narrow shape is
|
||||
// visible instead of passing on its single lucky case (Copilot).
|
||||
errors.push(
|
||||
`only ${positives} positive case(s); a model-invoked skill needs at least ${MIN_POSITIVE_CASES} to cover the range of situations it should fire on`,
|
||||
`only ${distinctPositives} distinct positive prompt(s)${positives === distinctPositives ? "" : ` across ${positives} case(s)`}; a model-invoked skill needs at least ${MIN_POSITIVE_CASES} to cover the range of situations it should fire on`,
|
||||
);
|
||||
}
|
||||
if (negatives === 0) {
|
||||
|
|
|
|||
|
|
@ -426,7 +426,7 @@ test("a model-invoked skill needs at least MIN_POSITIVE_CASES positives", () =>
|
|||
|
||||
assert.match(
|
||||
validateEvalCases("alpha", withPositives(1)).join(" "),
|
||||
/only 1 positive case\(s\)/,
|
||||
/only 1 distinct positive prompt\(s\)/,
|
||||
);
|
||||
assert.match(
|
||||
validateEvalCases("alpha", withPositives(MIN_POSITIVE_CASES - 1)).join(" "),
|
||||
|
|
@ -533,3 +533,71 @@ test("the truncation guard ignores anchors and tags, which are not `#`", () => {
|
|||
/`description` is truncated/,
|
||||
);
|
||||
});
|
||||
|
||||
test("a quoted scalar stays safe behind a tag or an anchor", () => {
|
||||
// Properties may precede the value, in either order. Testing the first
|
||||
// character alone read these as unquoted and then rejected the `#` that the
|
||||
// quotes make harmless.
|
||||
const mk = (value) =>
|
||||
[
|
||||
"---",
|
||||
"name: alpha",
|
||||
`description: ${value}`,
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
|
||||
for (const value of [
|
||||
'© "Counts # safely"',
|
||||
'!!str "Counts # safely"',
|
||||
'!!str &a "Counts # safely"',
|
||||
]) {
|
||||
const parsed = parseSkill("alpha", mk(value));
|
||||
assert.deepEqual(parsed.errors, [], `rejected: ${value}`);
|
||||
assert.equal(parsed.description, "Counts # safely");
|
||||
}
|
||||
});
|
||||
|
||||
test("the positive floor counts distinct prompts, not entries", () => {
|
||||
// The floor exists for breadth, so repeating one prompt five times clears an
|
||||
// entry count while exercising exactly the single trigger it looks past.
|
||||
const duplicated = [
|
||||
...Array.from({ length: MIN_POSITIVE_CASES }, () => ({
|
||||
prompt: "the same prompt",
|
||||
expect: "alpha",
|
||||
})),
|
||||
{ prompt: "n", expect: null },
|
||||
];
|
||||
const errors = validateEvalCases("alpha", duplicated).join(" ");
|
||||
assert.match(
|
||||
errors,
|
||||
/only 1 distinct positive prompt\(s\) across 5 case\(s\)/,
|
||||
);
|
||||
|
||||
// Whitespace and casing are not a new situation either.
|
||||
const nearDuplicates = [
|
||||
{ prompt: "How do I do the thing?", expect: "alpha" },
|
||||
{ prompt: "how do i do the thing?", expect: "alpha" },
|
||||
{ prompt: "How do I do the thing? ", expect: "alpha" },
|
||||
{ prompt: "b", expect: "alpha" },
|
||||
{ prompt: "c", expect: "alpha" },
|
||||
{ prompt: "n", expect: null },
|
||||
];
|
||||
assert.match(
|
||||
validateEvalCases("alpha", nearDuplicates).join(" "),
|
||||
/only 3 distinct positive prompt\(s\)/,
|
||||
);
|
||||
|
||||
// Five genuinely different prompts pass.
|
||||
assert.deepEqual(
|
||||
validateEvalCases("alpha", [
|
||||
...Array.from({ length: MIN_POSITIVE_CASES }, (_, i) => ({
|
||||
prompt: `p${i}`,
|
||||
expect: "alpha",
|
||||
})),
|
||||
{ prompt: "n", expect: null },
|
||||
]),
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue