From 1482dc5ce0a45eed412aa8553c55a4e57097dba4 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 23 Jul 2026 07:38:00 -0400 Subject: [PATCH] fix(#2366): scope parseCoverageMatrix to recognized coverage tables (#2551) * test(#2366): regression tests for parseCoverageMatrix scoping bugs Bug 1: summary table outside matrix not parsed as data Bug 2: multi-section matrix with repeated headers parses correctly Bug 3: markdown emphasis on decision cell is stripped * fix(#2366): scope parseCoverageMatrix to recognized coverage tables Replace latching sawHeader with contextual inMatrix tracking that resets on non-pipe lines, preventing summary tables from being parsed as data (bug 1). Allow multiple headers for multi-section matrices (bug 2). Strip markdown emphasis from decision cells before validation (bug 3). Closes #2366 * fix(#2366): update representative-corpus test to expect correct behavior The test previously documented the known-buggy parseCoverageMatrix behavior. Now that the fix is in place, test against the expected correct output (expectedBlock, expectedCounts, expectedErrorCount) instead of the currentBuggyOutput snapshot. * docs(#2366): backfill changeset PR number (2551) --- .../2366-parse-coverage-matrix-scope.md | 5 ++ src/api-coverage.cts | 29 ++++++++--- tests/api-coverage.test.cjs | 52 +++++++++++++++++++ tests/representative-corpus.test.cjs | 20 ++----- 4 files changed, 83 insertions(+), 23 deletions(-) create mode 100644 .changeset/2366-parse-coverage-matrix-scope.md diff --git a/.changeset/2366-parse-coverage-matrix-scope.md b/.changeset/2366-parse-coverage-matrix-scope.md new file mode 100644 index 000000000..4abaa8183 --- /dev/null +++ b/.changeset/2366-parse-coverage-matrix-scope.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2551 +--- +**`parseCoverageMatrix` now scopes table parsing to recognized coverage matrices** — pipe-tables outside the matrix (e.g., summary tables) are ignored instead of being silently parsed as data rows, multi-section matrices with repeated headers are supported, and inline markdown emphasis (`**OPT-OUT**`) on decision cells is stripped before validation. Previously, the parser scanned every `|`-prefixed line file-wide with a latching header flag, causing silent phantom-capability corruption from unrelated tables, false rejection of multi-section matrices, and rejection of bold-emphasized decisions. (#2366) diff --git a/src/api-coverage.cts b/src/api-coverage.cts index 5952aaae1..f2c3f1826 100644 --- a/src/api-coverage.cts +++ b/src/api-coverage.cts @@ -657,25 +657,38 @@ export function parseCoverageMatrix(text: unknown): CoverageParseResult { return out; } - // (2) markdown table — collect table rows whose decision column parses. + // (2) markdown table — collect rows from coverage matrix tables only (#2366). + // Track whether we are inside a recognized coverage matrix (after a header + // row, before a non-pipe line ends the table). This prevents summary tables + // elsewhere in the file from being parsed as data (#2366 bug 1) and allows + // multi-section matrices with repeated headers (#2366 bug 2). const lines = src.split('\n'); - let sawHeader = false; + let inMatrix = false; for (const line of lines) { const trimmed = line.trim(); - if (!trimmed.startsWith('|')) continue; + if (!trimmed.startsWith('|')) { + inMatrix = false; + continue; + } const cells = trimmed.slice(1, trimmed.endsWith('|') ? -1 : trimmed.length).split('|'); if (cells.length < 2) continue; const cleaned = cells.map((c) => c.trim()); // skip separator rows (|---|---|); require ≥3 dashes so a literal "-" cell // is not mistaken for a separator. if (cleaned.every((c) => /^:?-{3,}:?$/.test(c))) continue; - const decisionCell = (cleaned[1] || '').toUpperCase(); - // header detection - if (!sawHeader && cleaned[0].toLowerCase() === 'capability') { - sawHeader = true; - out.format = 'table'; + // Strip markdown emphasis (**, *, __, _, `) from the decision cell before + // comparison so **OPT-OUT** parses correctly (#2366 bug 3). + const decisionCell = (cleaned[1] || '').replace(/[*_`]/g, '').trim().toUpperCase(); + // header detection — recognized by 'capability' in column 0; allows multiple + // headers for multi-section matrices (#2366 bug 2). + if (cleaned[0].toLowerCase() === 'capability') { + inMatrix = true; + if (out.format === 'none') out.format = 'table'; continue; } + // Only parse data rows from inside a recognized coverage matrix table. + // A pipe-table outside the matrix (e.g., a summary table) is ignored (#2366 bug 1). + if (!inMatrix) continue; if (!VALID_DECISIONS.has(decisionCell as CoverageDecision)) { // A row that otherwise looks like data (≥3 cells, non-empty capability) // but carries a malformed decision is a real error, not a row to skip diff --git a/tests/api-coverage.test.cjs b/tests/api-coverage.test.cjs index 21793d117..b96e53e9f 100644 --- a/tests/api-coverage.test.cjs +++ b/tests/api-coverage.test.cjs @@ -562,6 +562,58 @@ describe('coverage matrix — parse / validate (#1562 acceptance #2)', () => { assert.strictEqual(p.rows.length, 0); }); + test('#2366 bug 1: summary table outside the matrix is not parsed as data', () => { + const md = [ + '# API Coverage', + '', + '| capability | decision | reason |', + '|---|---|---|', + '| search | INTEGRATE | |', + '', + '## Coverage summary', + '', + '| tier | INTEGRATE | OPT-OUT |', + '|---|---|---|', + '| phase 8 | 12 | 6 |', + ].join('\n'); + const p = parseCoverageMatrix(md); + assert.strictEqual(p.rows.length, 1, 'only the matrix row, not the summary table'); + assert.strictEqual(p.rows[0].capability, 'search'); + assert.strictEqual(p.errors.length, 0); + }); + + test('#2366 bug 2: multi-section matrix with repeated headers is parsed', () => { + const md = [ + '| capability | decision | reason |', + '|---|---|---|', + '| search | INTEGRATE | |', + '', + '## Transferred', + '', + '| capability | decision | reason |', + '|---|---|---|', + '| widget | OPT-OUT | deferred to 9 |', + ].join('\n'); + const p = parseCoverageMatrix(md); + assert.strictEqual(p.rows.length, 2, 'both sections should parse'); + assert.strictEqual(p.rows[0].capability, 'search'); + assert.strictEqual(p.rows[1].capability, 'widget'); + assert.strictEqual(p.errors.length, 0); + }); + + test('#2366 bug 3: markdown emphasis on decision is stripped', () => { + const md = [ + '| capability | decision | reason |', + '|---|---|---|', + '| search | INTEGRATE | |', + '| skip | **OPT-OUT** | not needed yet |', + ].join('\n'); + const p = parseCoverageMatrix(md); + assert.strictEqual(p.rows.length, 2); + assert.strictEqual(p.rows[1].decision, 'OPT-OUT', '**OPT-OUT** should parse as OPT-OUT'); + assert.strictEqual(p.errors.length, 0); + }); + // ── validate: boundaries 0 / 1 / 2 rows (limit-1, limit, limit+1) ──────── test('validate — empty matrix is invalid (acceptance #1: surface must be enumerated)', () => { const v = validateCoverageMatrix('| capability | decision | reason |\n|---|---|---|'); diff --git a/tests/representative-corpus.test.cjs b/tests/representative-corpus.test.cjs index cc86c1073..9f4f2971f 100644 --- a/tests/representative-corpus.test.cjs +++ b/tests/representative-corpus.test.cjs @@ -153,9 +153,7 @@ describe('representative corpus — api-coverage matrix (#2366)', () => { const manifest = readManifest('api-coverage-matrix'); for (const fx of manifest.fixtures) { - const label = fx.currentBuggyOutput - ? `${fx.file} → currently silently-corrupted + spurious errors (#2366)` - : `${fx.file} → exactly the canonical rows, 0 errors`; + const label = `${fx.file} → exactly the canonical rows, 0 errors`; test(label, () => { tmpDir = makeProject(); const phaseDir = makePhaseDir(tmpDir, '01-repcorpus'); @@ -166,18 +164,10 @@ describe('representative corpus — api-coverage matrix (#2366)', () => { assert.ok(r.success, `gate should succeed (JSON). stderr: ${r.error}`); const j = JSON.parse(r.output); - if (fx.currentBuggyOutput) { - assert.strictEqual(j.block, fx.currentBuggyOutput.block, - `${fx.file}: expected today's known-buggy block:${fx.currentBuggyOutput.block}, got ${JSON.stringify(j)}. ` + - `If this now differs, #2366 may be fixed — check against expectedBlock:${fx.expectedBlock} instead.`); - assert.strictEqual(j.error_count, fx.currentBuggyOutput.error_count, `${fx.file}: error_count`); - assert.deepStrictEqual(j.errors, fx.currentBuggyOutput.errors, `${fx.file}: errors`); - } else { - assert.strictEqual(j.block, fx.expectedBlock, `${fx.file}: block. Got ${JSON.stringify(j)}`); - assert.deepStrictEqual(j.counts, fx.expectedCounts, `${fx.file}: counts. Got ${JSON.stringify(j)}`); - assert.strictEqual((j.errors || []).length, fx.expectedErrorCount, - `${fx.file}: errors. Got ${JSON.stringify(j.errors)}`); - } + assert.strictEqual(j.block, fx.expectedBlock, `${fx.file}: block. Got ${JSON.stringify(j)}`); + assert.deepStrictEqual(j.counts, fx.expectedCounts, `${fx.file}: counts. Got ${JSON.stringify(j)}`); + assert.strictEqual((j.errors || []).length, fx.expectedErrorCount, + `${fx.file}: errors. Got ${JSON.stringify(j.errors)}`); }); } });