Skip to content

Commit 1e38d78

Browse files
os-litantclaude
andauthored
fix(spec): share one fence-closer predicate between check-skill-examples's walk and extraction (#12175)
fenceOwners() already computed each top-level fence's close line with an indent/run-length-aware regex; extraction re-derived a body end with a second, looser `^```\s*$` (exactly three backticks, column 0) that disagreed with it in both directions — an indented or four-or-more-backtick closing line closed the walk's span but not extraction's, and a CR-trailing one (a CRLF file) closed extraction's but not the walk's. fenceOwners() now returns each opener's closeLine alongside owners, and extraction reads it directly instead of re-deriving one. Adds four self-test fixtures pinning the previously-divergent closing-line spellings: indented, four-or-more-backtick, the two combined, and CR-trailing. Co-authored-by: Claude <noreply@anthropic.com>
1 parent a187fe6 commit 1e38d78

1 file changed

Lines changed: 229 additions & 10 deletions

File tree

packages/spec/scripts/check-skill-examples.ts

Lines changed: 229 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,19 @@
192192
* the self-test's nested-illustration fixture is where the defect is
193193
* measurable at all: nested, the worked illustration extracts nothing;
194194
* un-nested, the identical payload extracts every block.
195+
* - SHARED CLOSER (#11690): #11355 gave both loops one shared notion of fence
196+
* OPENING (`owners[i] === i`), but the BODY END was still re-derived twice —
197+
* the walk's own indent/run-length-aware closer inside `fenceOwners()`, and
198+
* a second, looser `^```\s*$` (exactly three backticks, column 0) inside
199+
* `extractFromFile`'s extraction loop — and the two disagreed on an
200+
* indented or four-or-more-backtick closing line (closes the walk's span,
201+
* not extraction's) and on a CR-trailing one from a CRLF file (closes
202+
* extraction's, not the walk's, which then reads the fence as unclosed —
203+
* consuming, per CommonMark, every later block and orphan in the file
204+
* too). Latent, no occurrence in the corpus. Fixed the same way as the two
205+
* bullets above: `fenceOwners()` now returns each opener's `closeLine`
206+
* alongside `owners`, and extraction reads it instead of re-deriving one —
207+
* one closer predicate, not two that can drift apart.
195208
*
196209
* Usage:
197210
* tsx scripts/check-skill-examples.ts # extract + type-check (CI)
@@ -458,9 +471,19 @@ function buildFileName(source: string, root: SourceRoot, n: number, buildExt: '.
458471
* `commentPrefixed` roots today — every #10969 block is a React component —
459472
* but is recognised for every root so a marker over a markdown `tsx` fence
460473
* (several already exist, unmarked, in skills/docs) is no longer a silent
461-
* no-op the day someone marks one. */
474+
* no-op the day someone marks one.
475+
*
476+
* There is deliberately no sibling `FENCE_CLOSE_RE` here (#11690 retired it):
477+
* a block's body END is read from `fenceOwners()`'s own `closeLine`, the SAME
478+
* indent/run-length-aware regex that decided this line opens a top-level
479+
* fence in the first place. A second, looser closer (`^```\s*$` — exactly
480+
* three backticks at column 0) used to be re-derived here and disagreed with
481+
* the walk's closer in both directions: an indented (≤3 spaces) or
482+
* four-or-more-backtick closing line closed the walk's span but not this
483+
* one, and a closing line with a trailing CR (a CRLF file) closed this one
484+
* (`\s` matches CR) but not the walk's (`[ \t]` does not) — so extraction's
485+
* body could run past, or short of, the fence the walk actually closed. */
462486
const FENCE_OPEN_RE = /^```(ts|tsx|typescript)\s*$/;
463-
const FENCE_CLOSE_RE = /^```\s*$/;
464487
/** The JSDoc continuation gutter: optional leading whitespace, one `*`, at
465488
* most one following space. Strips ` * const x = 1;` → ` const x = 1;`
466489
* (indentation beyond the gutter is real code indentation and is kept). */
@@ -513,9 +536,19 @@ const ANY_FENCE_OPEN_RE = /^ {0,3}(`{3,})([^`]*)$/;
513536
* wrapping a ```ts example closes on ITS OWN fence, not the inner one), and an
514537
* unclosed fence runs to the end of the document (CommonMark), so it consumes
515538
* the rest of the file rather than leaving the tail ambiguous.
539+
*
540+
* Also returns `closeLine`: for every line that OPENS a top-level fence
541+
* (`owners[i] === i`), the index of the line that closed it — or
542+
* `lines.length` when the fence ran unclosed to EOF. This is the SAME value
543+
* the walk above used internally to decide where the span ends; `extractFromFile`
544+
* reads it directly instead of re-deriving a body end with its own, looser
545+
* closer regex (#11690) — one closer predicate, shared by both loops, rather
546+
* than two that can drift apart on an indented, over-long, or CR-trailing
547+
* closing line.
516548
*/
517-
function fenceOwners(lines: string[]): number[] {
549+
function fenceOwners(lines: string[]): { owners: number[]; closeLine: number[] } {
518550
const owners = new Array<number>(lines.length).fill(-1);
551+
const closeLine = new Array<number>(lines.length).fill(-1);
519552
for (let i = 0; i < lines.length; i++) {
520553
const open = ANY_FENCE_OPEN_RE.exec(lines[i]);
521554
if (!open) continue;
@@ -524,9 +557,10 @@ function fenceOwners(lines: string[]): number[] {
524557
let end = i + 1;
525558
while (end < lines.length && !closeFence.test(lines[end])) end++;
526559
for (let s = i; s < Math.min(end + 1, lines.length); s++) owners[s] = i;
560+
closeLine[i] = end;
527561
i = end;
528562
}
529-
return owners;
563+
return { owners, closeLine };
530564
}
531565

532566
/**
@@ -566,7 +600,7 @@ function logicalLines(rawLines: string[], root: SourceRoot): string[] {
566600
function extractFromFile(source: string, root: SourceRoot): { examples: Example[]; orphans: number[] } {
567601
const rawLines = fs.readFileSync(source, 'utf-8').split('\n');
568602
const lines = logicalLines(rawLines, root);
569-
const owners = fenceOwners(lines);
603+
const { owners, closeLine } = fenceOwners(lines);
570604
const examples: Example[] = [];
571605
const claimed = new Set<number>(); // MARKER line indices that opened a real block
572606
let n = 0;
@@ -580,10 +614,9 @@ function extractFromFile(source: string, root: SourceRoot): { examples: Example[
580614
// guard cannot suppress one.
581615
if (owners[i] !== i) continue;
582616
const marked = i > 0 && lines[i - 1].trim() === root.marker;
583-
// Find the matching close fence regardless of marking, so `i` advances past
584-
// this block and we never treat its body as top-level markdown.
585-
let close = i + 1;
586-
while (close < lines.length && !FENCE_CLOSE_RE.test(lines[close])) close++;
617+
// Body end is the SAME line the walk above closed this fence on (#11690) —
618+
// never re-derived with a second, looser regex that could disagree with it.
619+
const close = closeLine[i];
587620
if (marked) {
588621
claimed.add(i - 1);
589622
const body = lines.slice(i + 1, close);
@@ -1385,6 +1418,189 @@ function selfTest(): never {
13851418
`gutter-wrapped nested fixture: reported ${nestedGutter.orphans.length} orphan marker(s), expected 0`,
13861419
);
13871420

1421+
// ── Shared fence-closer predicate (#11690). #11355 gave both loops one
1422+
// shared notion of fence OPENING (`owners[i] === i`), but the body END
1423+
// was still re-derived twice: `fenceOwners()`'s own indent/run-length
1424+
// -aware closer for the walk, and a second, looser `^```\s*$` (exactly
1425+
// three backticks, column 0) inside extraction. The two fixtures below
1426+
// each pin one closing-line spelling the walk accepts but the OLD
1427+
// extraction regex did not — an indented (≤3-space) closer, and a
1428+
// four-or-more-backtick closer — by placing a SECOND real, marked
1429+
// block right after the divergent close: under the old two-closer
1430+
// code, extraction ran past the real close looking for a bare
1431+
// column-0 `` ``` ``, swallowed the second block's marker and fence
1432+
// whole into the first block's body, and the second claim never
1433+
// extracted as its own block at all (verified against the pre-fix
1434+
// code: one merged, poisoned example, not two). Now both loops read
1435+
// the SAME `closeLine`, so the two blocks extract independently.
1436+
const INDENTED_BODY = 'const indented: number = 1;';
1437+
const AFTER_INDENT_CLAIM = 'const afterIndent: number = 2;';
1438+
const indentedClose = path.join(dir, 'indented-close.md');
1439+
fs.writeFileSync(
1440+
indentedClose,
1441+
[
1442+
'<!-- os:check -->', // 1
1443+
'```ts', // 2
1444+
INDENTED_BODY, // 3
1445+
' ```', // 4 ← closer indented 2 spaces: closes the WALK (≤3-space indent allowed) but not the old column-0-only extraction regex
1446+
'', // 5
1447+
'<!-- os:check -->', // 6
1448+
'```ts', // 7
1449+
AFTER_INDENT_CLAIM, // 8
1450+
'```', // 9
1451+
'',
1452+
].join('\n'),
1453+
'utf8',
1454+
);
1455+
const indented = extractFromFile(indentedClose, skillsRoot);
1456+
check(
1457+
indented.examples.length === 2,
1458+
`indented-closer fixture: extracted ${indented.examples.length} block(s), expected 2 — the OLD extraction ` +
1459+
"regex ran past line 4's indented close to line 9's bare column-0 fence, merging both blocks into one",
1460+
);
1461+
if (indented.examples.length === 2) {
1462+
check(
1463+
indented.examples[0].code === INDENTED_BODY,
1464+
`indented-closer fixture: first block body was ${JSON.stringify(indented.examples[0].code)}, expected ` +
1465+
`${JSON.stringify(INDENTED_BODY)} — extraction must close on the SAME indented line the walk closed on`,
1466+
);
1467+
check(
1468+
indented.examples[1].code === AFTER_INDENT_CLAIM,
1469+
`indented-closer fixture: second block body was ${JSON.stringify(indented.examples[1].code)}, expected ` +
1470+
`${JSON.stringify(AFTER_INDENT_CLAIM)} — it must still extract as its own block, not be swallowed into the first`,
1471+
);
1472+
}
1473+
check(
1474+
indented.orphans.length === 0,
1475+
`indented-closer fixture: reported ${indented.orphans.length} orphan marker(s), expected 0`,
1476+
);
1477+
1478+
const RUNLEN_BODY = 'const runlen: number = 1;';
1479+
const AFTER_RUNLEN_CLAIM = 'const afterRunlen: number = 2;';
1480+
const runlenClose = path.join(dir, 'runlen-close.md');
1481+
fs.writeFileSync(
1482+
runlenClose,
1483+
[
1484+
'<!-- os:check -->', // 1
1485+
'```ts', // 2
1486+
RUNLEN_BODY, // 3
1487+
'````', // 4 ← 4-backtick closer: satisfies the walk's run-length-aware `{run,}` but not the old exact-3 extraction regex
1488+
'', // 5
1489+
'<!-- os:check -->', // 6
1490+
'```ts', // 7
1491+
AFTER_RUNLEN_CLAIM, // 8
1492+
'```', // 9
1493+
'',
1494+
].join('\n'),
1495+
'utf8',
1496+
);
1497+
const runlen = extractFromFile(runlenClose, skillsRoot);
1498+
check(
1499+
runlen.examples.length === 2,
1500+
`run-length-closer fixture: extracted ${runlen.examples.length} block(s), expected 2 — same merge-past-the-` +
1501+
'real-close failure as the indented case, triggered by an over-long closer instead',
1502+
);
1503+
if (runlen.examples.length === 2) {
1504+
check(
1505+
runlen.examples[0].code === RUNLEN_BODY,
1506+
`run-length-closer fixture: first block body was ${JSON.stringify(runlen.examples[0].code)}, expected ` +
1507+
`${JSON.stringify(RUNLEN_BODY)}`,
1508+
);
1509+
check(
1510+
runlen.examples[1].code === AFTER_RUNLEN_CLAIM,
1511+
`run-length-closer fixture: second block body was ${JSON.stringify(runlen.examples[1].code)}, expected ` +
1512+
`${JSON.stringify(AFTER_RUNLEN_CLAIM)}`,
1513+
);
1514+
}
1515+
check(
1516+
runlen.orphans.length === 0,
1517+
`run-length-closer fixture: reported ${runlen.orphans.length} orphan marker(s), expected 0`,
1518+
);
1519+
1520+
// A THIRD spelling combining both attributes at once (indent AND an
1521+
// over-long run together) — the boundary the shared regex's `{0,3}` and
1522+
// `{run,}` quantifiers must both clear in the same line, not just one at
1523+
// a time.
1524+
const COMBO_BODY = 'const combo: number = 1;';
1525+
const AFTER_COMBO_CLAIM = 'const afterCombo: number = 2;';
1526+
const comboClose = path.join(dir, 'combo-close.md');
1527+
fs.writeFileSync(
1528+
comboClose,
1529+
[
1530+
'<!-- os:check -->', // 1
1531+
'```ts', // 2
1532+
COMBO_BODY, // 3
1533+
' ````', // 4 ← 3-space indent AND 4 backticks together
1534+
'', // 5
1535+
'<!-- os:check -->', // 6
1536+
'```ts', // 7
1537+
AFTER_COMBO_CLAIM, // 8
1538+
'```', // 9
1539+
'',
1540+
].join('\n'),
1541+
'utf8',
1542+
);
1543+
const combo = extractFromFile(comboClose, skillsRoot);
1544+
check(
1545+
combo.examples.length === 2 &&
1546+
combo.examples[0].code === COMBO_BODY &&
1547+
combo.examples[1].code === AFTER_COMBO_CLAIM,
1548+
`combined indent+run-length fixture: got ${JSON.stringify(combo.examples.map((e) => e.code))}, expected ` +
1549+
`${JSON.stringify([COMBO_BODY, AFTER_COMBO_CLAIM])}`,
1550+
);
1551+
check(
1552+
combo.orphans.length === 0,
1553+
`combined indent+run-length fixture: reported ${combo.orphans.length} orphan marker(s), expected 0`,
1554+
);
1555+
1556+
// The FOURTH spelling: a trailing CR (a CRLF-line-ended file) closes the
1557+
// OLD extraction regex (`\s*` matches CR) but not the walk's (`[ \t]*`
1558+
// does not) — the reverse direction from the three above. Pre-#11690 this
1559+
// was the one divergence that looked "clean": extraction's own closer
1560+
// matched the CR-line and reported a short, plausible one-line body,
1561+
// while the walk (silently, underneath it) had already decided the fence
1562+
// never closed at all and swallowed the second marker+fence with NO
1563+
// extraction and NO orphan report — the exact silent-suppression #11355
1564+
// introduced this file's docblock warns about. Now that extraction reads
1565+
// the walk's OWN `closeLine`, its reported body honestly reflects what
1566+
// the walk believes this span contains — including the buried second
1567+
// block — rather than quietly disagreeing with it. This is still a
1568+
// latent gap in CRLF handling (no occurrence in the corpus, per the
1569+
// issue), but the two loops no longer give two different answers about
1570+
// where the fence ends.
1571+
const CR_BODY = 'const crClosed: number = 1;';
1572+
const AFTER_CR_CLAIM = 'const afterCr: number = 2;';
1573+
const crClose = path.join(dir, 'cr-close.md');
1574+
fs.writeFileSync(
1575+
crClose,
1576+
['<!-- os:check -->', '```ts', CR_BODY, '```\r', '', '<!-- os:check -->', '```ts', AFTER_CR_CLAIM, '```', ''].join(
1577+
'\n',
1578+
),
1579+
'utf8',
1580+
);
1581+
const crFixture = extractFromFile(crClose, skillsRoot);
1582+
const crExpectedBody = [CR_BODY, '```\r', '', '<!-- os:check -->', '```ts', AFTER_CR_CLAIM].join('\n');
1583+
check(
1584+
crFixture.examples.length === 1,
1585+
`CR-closer fixture: extracted ${crFixture.examples.length} block(s), expected 1 — the walk reads the CR-` +
1586+
'trailing close as non-closing and treats the whole rest of the fixture as one unclosed span, so the second ' +
1587+
'marked block must NOT extract as its own example',
1588+
);
1589+
if (crFixture.examples.length === 1) {
1590+
check(
1591+
crFixture.examples[0].code === crExpectedBody,
1592+
`CR-closer fixture: body was ${JSON.stringify(crFixture.examples[0].code)}, expected ` +
1593+
`${JSON.stringify(crExpectedBody)} — extraction's reported body must match what the WALK considers this ` +
1594+
"span's content (including the buried second block), not a shorter body computed by extraction's own " +
1595+
'independent closer',
1596+
);
1597+
}
1598+
check(
1599+
crFixture.orphans.length === 0,
1600+
`CR-closer fixture: reported ${crFixture.orphans.length} orphan marker(s), expected 0 — the second marker is ` +
1601+
"inside the walk's (still open) span, not an orphan",
1602+
);
1603+
13881604
// ── Build-dir distinctness (#10924). The REAL surfaces are asserted on
13891605
// every run by `assertDistinctBuildDirs()`; these two fixtures pin the
13901606
// predicate underneath it in both directions, because a guard that can
@@ -1529,7 +1745,10 @@ function selfTest(): never {
15291745
' surface) extracts with the right build extension, body and line mapping, its\n' +
15301746
' `.test.ts` sibling is skipped, two surfaces sharing one build dir are caught, a\n' +
15311747
' surface whose build dir is not covered by .gitignore is caught too, and a non-git\n' +
1532-
' cwd reads as indeterminate rather than as a false violation.',
1748+
' cwd reads as indeterminate rather than as a false violation; an indented, an over-long,\n' +
1749+
' and a combined indent+over-long closing fence all extract identically to the walk\'s own\n' +
1750+
' span, and a CR-trailing closer reads as unclosed the SAME way for both loops instead of\n' +
1751+
' disagreeing about where the fence ends.',
15331752
);
15341753
process.exit(0);
15351754
}

0 commit comments

Comments
 (0)