docs: address Copilot review round 5 on #2203
The truncation guard read only the first physical line of the scalar, so
a multiline plain scalar slipped past it:
description: Covers boards
#28 and #11
YAML parses that as `Covers boards`, and the first-line-only comparison
saw `Covers boards` on both sides and reported no loss — passing the
exact silent truncation the guard exists to reject.
It now walks the whole plain scalar: the value line plus every indented
continuation, whitespace-collapsed to match YAML's folding of newlines to
spaces. Quoted and block scalars are still skipped, since neither loses
text to a comment.
Two regression cases: the multiline truncation, and a multiline scalar
with no `#`, which must stay clean — the fix cannot be "reject anything
multiline".
No skill description or eval case changed, so skills:eval is unchanged
from round 4's 63/63. local:gate passes, 34/34. 390/390 script tests.
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
4acab97e63
commit
ec2ef5656d
2 changed files with 66 additions and 10 deletions
|
|
@ -156,27 +156,50 @@ export function parseSkill(dirName, text) {
|
|||
// non-empty string. `board-ops` shipped for a while with its two board numbers
|
||||
// and the option-deletion hazard cut off this way. Compare the raw scalar
|
||||
// against what YAML actually kept and make any loss an error.
|
||||
//
|
||||
// The comparison has to span the WHOLE plain scalar, not its first physical
|
||||
// line. A plain scalar continues onto more-indented lines, so
|
||||
//
|
||||
// description: Covers boards
|
||||
// #28 and #11
|
||||
//
|
||||
// parses to `Covers boards` — and a first-line-only check compares that
|
||||
// against `Covers boards` and sees no loss, passing the exact truncation this
|
||||
// guard exists to reject (Copilot).
|
||||
const lines = frontmatter.split("\n");
|
||||
for (const [field, parsed] of [
|
||||
["description", description],
|
||||
["when_to_use", whenToUse],
|
||||
]) {
|
||||
if (typeof parsed !== "string") continue;
|
||||
const raw = new RegExp("^" + field + ":[ \\t]*(.*)$", "m").exec(
|
||||
frontmatter,
|
||||
)?.[1];
|
||||
if (raw === undefined) continue;
|
||||
const trimmed = raw.trim();
|
||||
const start = lines.findIndex((l) =>
|
||||
new RegExp("^" + field + ":([ \\t]|$)").test(l),
|
||||
);
|
||||
if (start === -1) continue;
|
||||
const head = lines[start].slice(field.length + 1).trim();
|
||||
// Only an unquoted scalar can lose text to a comment; a quoted one is safe,
|
||||
// and a block scalar (`|`/`>`) puts nothing on this line to compare.
|
||||
if (/^["'|>]/.test(trimmed)) continue;
|
||||
if (trimmed.length > parsed.trim().length) {
|
||||
// and a block scalar (`|`/`>`) is read literally rather than comment-stripped.
|
||||
if (/^["'|>]/.test(head)) continue;
|
||||
// Continuation lines are indented and are not the next `key:` of the map.
|
||||
const parts = [head];
|
||||
for (let i = start + 1; i < lines.length; i++) {
|
||||
const l = lines[i];
|
||||
if (!/^[ \t]/.test(l) || l.trim() === "") break;
|
||||
parts.push(l.trim());
|
||||
}
|
||||
// YAML folds a plain scalar's newlines to single spaces, so compare on
|
||||
// whitespace-collapsed text rather than on the raw characters.
|
||||
const collapse = (s) => s.replace(/\s+/g, " ").trim();
|
||||
const rawText = collapse(parts.join(" "));
|
||||
const keptText = collapse(parsed);
|
||||
if (rawText.length > keptText.length) {
|
||||
errors.push(
|
||||
"`" +
|
||||
field +
|
||||
"` is truncated by an unquoted `#` — YAML kept " +
|
||||
parsed.trim().length +
|
||||
keptText.length +
|
||||
" of " +
|
||||
trimmed.length +
|
||||
rawText.length +
|
||||
" characters; quote the value",
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -471,3 +471,36 @@ test("the truncation guard covers `when_to_use`, not just `description`", () =>
|
|||
assert.deepEqual(ok.errors, []);
|
||||
assert.equal(ok.whenToUse, "Use when filing against board #28 or board #11.");
|
||||
});
|
||||
|
||||
test("the truncation guard spans a multiline plain scalar", () => {
|
||||
// A plain scalar continues onto more-indented lines, so a first-line-only
|
||||
// comparison sees `Covers boards` on both sides and reports no loss — passing
|
||||
// the exact truncation this guard exists to reject.
|
||||
const multiline = [
|
||||
"---",
|
||||
"name: alpha",
|
||||
"description: Covers boards",
|
||||
" #28 and #11 and the hazard.",
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
const parsed = parseSkill("alpha", multiline);
|
||||
assert.equal(parsed.description, "Covers boards");
|
||||
assert.match(parsed.errors.join(" "), /`description` is truncated/);
|
||||
|
||||
// A multiline plain scalar with no `#` folds to one line and must stay clean,
|
||||
// so the fix cannot be "reject anything multiline".
|
||||
const folded = [
|
||||
"---",
|
||||
"name: alpha",
|
||||
"description: Covers boards",
|
||||
" and the option-deletion hazard.",
|
||||
"disable-model-invocation: false",
|
||||
"---",
|
||||
"body",
|
||||
].join("\n");
|
||||
const ok = parseSkill("alpha", folded);
|
||||
assert.deepEqual(ok.errors, []);
|
||||
assert.equal(ok.description, "Covers boards and the option-deletion hazard.");
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue