diff --git a/.changeset/lucky-mice-wake.md b/.changeset/lucky-mice-wake.md new file mode 100644 index 000000000..22426cc52 --- /dev/null +++ b/.changeset/lucky-mice-wake.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2389 +--- +**The decision-coverage gate no longer fails open on unrecognized decision-ID prefixes** — `check.decision-coverage-plan` classified a populated `` block as "no trackable decisions" (a clean pass) whenever its IDs used a prefix the parser couldn't read (e.g. `D5-01` instead of `D-01`), silently skipping the gate on real decisions. The gate now recognizes any bold-lead-in decision bullet as evidence and fails loud (`could-not-parse`) when it can't read a populated block, instead of passing. (#2347) diff --git a/src/decisions.cts b/src/decisions.cts index c6c19ff3f..1a88ae013 100644 --- a/src/decisions.cts +++ b/src/decisions.cts @@ -84,6 +84,27 @@ const bulletEmDashRe = /^\s*-\s+\*\*D-([A-Za-z0-9][A-Za-z0-9_-]*)(?:\s*\[([^\]]+ */ const bulletTitledColonRe = /^\s*-\s+\*\*D-([A-Za-z0-9][A-Za-z0-9_-]*)(?:\s*\[([^\]]+)\])?[^:*]*:[^:*]*\*\*\s*(.*)$/; +/** + * #2347: format-agnostic evidence that a block/section holds real decision + * ENTRIES the parser could not read — a bullet whose bold lead-in is an + * ID-SHAPED token (uppercase prefix, optional digits, hyphen, alnum), whatever + * the exact ID grammar. The three parser grammars above all require a `D-` + * prefix; #1365's fail-loud guard reused that same `\bD-` test as its "is this + * decision-shaped?" evidence, so any other prefix (e.g. `D5-01`) was invisible + * to BOTH parser and guard, collapsing `could-not-parse` into a clean + * `none-present` pass. + * + * The ID-shape requirement (not "any bold bullet") is deliberate: a decisions + * block or `### Claude's Discretion` sub-section legitimately contains prose + * bullets with bold labels (`- **Scope:** …`, `- **Why:** …`, `- **Note:** …`). + * Those are NOT decision entries and must stay `none-present` — a false + * `could-not-parse` hard-blocks the plan gate. `[A-Z]+[0-9]*-[A-Za-z0-9]` matches + * `D-01` / `D5-01` / `DEC-01` but not `Scope:` / `Why:` / `Follow-up:` (mixed + * case) / `TODO:` (no `-` id) — mirroring the parser's own `D-` + * shape without hardcoding the `D`. + */ +const boldLeadInBulletRe = /^\s*-\s+\*\*[A-Z]+[0-9]*-[A-Za-z0-9]/m; + interface ParseDecisionLinesResult { decisions: Decision[]; parseMisses: number; @@ -241,11 +262,13 @@ export function extractDecisions(content: unknown): DecisionExtraction { } // FIX A: Block present but 0 extracted and no parse-misses. // Only report could-not-parse when there is genuine evidence of real decisions - // that failed to parse: a \bD- token in the block text, or an unterminated fence. - // An empty scaffold () or an all-prose block has no such - // evidence — treat as none-present so the gate passes cleanly. + // that failed to parse: a bold-lead-in bullet (`- **…**`, any ID grammar — #2347), + // a \bD- token in the block text, or an unterminated fence. An empty scaffold + // () or an all-prose block has no such evidence — treat + // as none-present so the gate passes cleanly. const hasDecisionTokenInBlock = /\bD-[A-Za-z0-9]/m.test(combined); - if (hasDecisionTokenInBlock || unterminatedFence) { + const hasBoldLeadInBullet = boldLeadInBulletRe.test(combined); + if (hasDecisionTokenInBlock || hasBoldLeadInBullet || unterminatedFence) { return { decisions: [], outcome: 'could-not-parse' }; } return { decisions: [], outcome: 'none-present' }; @@ -271,11 +294,13 @@ export function extractDecisions(content: unknown): DecisionExtraction { return { decisions, outcome: 'could-not-parse' }; } // FIX A: Heading found but 0 extracted and no parse-misses. - // Only report could-not-parse when the section body contains a D- token. - // A heading with only prose, sub-headings, or all-discretion content - // (no trackable D- tokens) is a legitimate empty/discretion section → none-present. + // Report could-not-parse when the section body holds a decision-entry-shaped + // bold-lead-in bullet (`- **…**`, any ID grammar — #2347) or a D- token. A + // heading with only prose, sub-headings, or all-discretion content (no such + // evidence) is a legitimate empty/discretion section → none-present. const hasDecisionTokenInSection = /\bD-[A-Za-z0-9]/m.test(section.body); - if (hasDecisionTokenInSection) { + const hasBoldLeadInBulletInSection = boldLeadInBulletRe.test(section.body); + if (hasDecisionTokenInSection || hasBoldLeadInBulletInSection) { return { decisions: [], outcome: 'could-not-parse' }; } return { decisions: [], outcome: 'none-present' }; diff --git a/tests/decisions.test.cjs b/tests/decisions.test.cjs index e1909329e..8b626f42b 100644 --- a/tests/decisions.test.cjs +++ b/tests/decisions.test.cjs @@ -220,6 +220,100 @@ describe('extractDecisions — typed outcome (#1364 + #1365)', () => { }); }); +// ─── #2347: evidence test must not reuse the parser's own D- grammar ────────── +// #1365's fail-loud guard used `/\bD-[A-Za-z0-9]/` as its "is this decision- +// shaped?" evidence test — the SAME D- prefix the parser requires. So for any +// ID prefix the parser can't read (e.g. `D5-01`), BOTH the parser and the guard +// see nothing, the outcome collapses to none-present (clean pass) instead of +// could-not-parse, and the gate fails OPEN against a populated block of real +// decisions. Fix: a bold-lead-in bullet (`- **...**`, any ID) is format-agnostic +// evidence of a decision entry. Note none of these bullet texts contain a +// literal "D-" token, so they isolate the bold-bullet evidence from the old +// D--token path. +describe('extractDecisions — format-agnostic evidence test (#2347)', () => { + test('populated block with a non-D- ID prefix is could-not-parse, not none-present', () => { + const md = '\n' + + '- **D5-01:** choose the primary datastore\n' + + '- **D5-02:** pick the queue technology\n' + + '- **D5-03:** settle on the auth model\n' + + '\n'; + const r = extractDecisions(md); + assert.strictEqual(r.decisions.length, 0, 'parser cannot read the D5- prefix (0 extracted)'); + assert.strictEqual(r.outcome, 'could-not-parse', + 'a populated block the parser cannot read must FAIL LOUD, not pass as none-present'); + }); + + test('decisions HEADING section with a non-D- ID prefix is could-not-parse', () => { + const md = '## Decisions\n\n- **DEC-01: the chosen approach** rationale\n'; + const r = extractDecisions(md); + assert.strictEqual(r.decisions.length, 0); + assert.strictEqual(r.outcome, 'could-not-parse', + 'a decision-shaped heading section the parser cannot read must fail loud'); + }); + + test('em-dash bullet with a non-D- ID prefix is still evidence (could-not-parse)', () => { + const md = '\n- **DEC-02 — the chosen approach** body\n\n'; + assert.strictEqual(extractDecisions(md).outcome, 'could-not-parse'); + }); + + // ── Regression guards: the broadened evidence must NOT create false fail-loud ─ + test('empty scaffold stays none-present (no false fail-loud)', () => { + assert.strictEqual(extractDecisions('\n\n').outcome, 'none-present'); + assert.strictEqual( + extractDecisions('\n\n(no decisions this phase)\n\n\n').outcome, + 'none-present', + 'a prose-only scaffold with no bold-lead-in bullet must still pass cleanly'); + }); + + test('an all-prose decisions block with no bold bullet stays none-present', () => { + const md = '\n\nThis phase inherits every prior decision; nothing new.\n\n\n'; + assert.strictEqual(extractDecisions(md).outcome, 'none-present'); + }); + + test('canonical D- decisions still parse (no regression)', () => { + const md = '\n- **D-01:** a real decision\n\n'; + const r = extractDecisions(md); + assert.strictEqual(r.outcome, 'parsed'); + assert.strictEqual(r.decisions.length, 1); + }); + + // ── The evidence must be ID-SHAPED, not "any bold bullet" ──────────────────── + // A decisions block / discretion section legitimately uses bold LABELS on prose + // bullets (- **Why:** …, - **Scope:** …). Those are not decision entries; a + // false could-not-parse here hard-blocks the plan gate. Guards against the + // over-broad evidence regex an earlier iteration shipped. + test('bold prose-label bullets (Why/Note/Scope) are NOT evidence — stay none-present', () => { + const md = '\n' + + '- **Why:** rationale for inheriting prior decisions\n' + + '- **Scope:** everything from the previous phase carries over\n' + + '- **Note:** nothing new was decided here\n' + + '\n'; + assert.strictEqual(extractDecisions(md).outcome, 'none-present', + 'bold LABELS on prose bullets must not be mistaken for decision entries'); + }); + + test("a Claude's Discretion sub-section with bold-label bullets stays none-present", () => { + const md = '\n' + + "### Claude's Discretion\n" + + '- **Scope:** left to judgment, no specific preference\n' + + '- **Follow-up:** revisit if performance regresses\n' + + '\n'; + assert.strictEqual(extractDecisions(md).outcome, 'none-present', + 'a discretion section of bold-label prose bullets must pass the gate cleanly'); + }); + + test('a bold ALL-CAPS label with no id-shape (TODO/NOTE) is not evidence', () => { + const md = '\n- **TODO:** decide the datastore next phase\n- **NOTE:** blocked on infra\n\n'; + assert.strictEqual(extractDecisions(md).outcome, 'none-present', + 'a bold ALL-CAPS label with no - id structure is not a decision entry'); + }); + + test('heading path: bold prose-label bullets under a Decisions heading stay none-present', () => { + const md = '## Decisions\n\n- **Why:** we kept the prior stack\n- **Scope:** no new choices this phase\n'; + assert.strictEqual(extractDecisions(md).outcome, 'none-present'); + }); +}); + // ─── QA matrix for parser correctness ──────────────────────────────────────── describe('parseDecisions — parser QA matrix', () => { diff --git a/tests/fixtures/representative/decision-coverage-guard/MANIFEST.json b/tests/fixtures/representative/decision-coverage-guard/MANIFEST.json index f51db1228..10b5e2b48 100644 --- a/tests/fixtures/representative/decision-coverage-guard/MANIFEST.json +++ b/tests/fixtures/representative/decision-coverage-guard/MANIFEST.json @@ -6,13 +6,7 @@ "file": "d5-prefix-context.md", "expectedReason": "could-not-parse", "expectedPassed": false, - "currentBuggyOutput": { - "passed": true, - "skipped": true, - "reason": "no trackable decisions", - "total": 0 - }, - "note": "A block using the D5-01 ID-prefix shape from #2347's own reproduction ('- **D5-01:** some decision'), repeated twice so 'populated but 0 extracted' is unambiguous. The original report used 23 real decisions under a project-specific D5- prefix convention; this fixture preserves the exact grammar mismatch, not the count. The #1365 guard's evidence test (/\\bD-[A-Za-z0-9]/) shares the parser's own D- grammar, so it is blind to exactly the input class it exists to catch: both see nothing, and the gate reports passed:true, skipped:true, reason:'no trackable decisions' instead of failing loud." + "note": "A block using the D5-01 ID-prefix shape from #2347's own reproduction ('- **D5-01:** some decision'), repeated twice so 'populated but 0 extracted' is unambiguous. The original report used 23 real decisions under a project-specific D5- prefix convention; this fixture preserves the exact grammar mismatch, not the count. FIXED by #2347: the guard's evidence test no longer reuses the parser's D- grammar — a bold ID-shaped lead-in bullet (any prefix) is now evidence, so this populated block correctly fails loud (could-not-parse). currentBuggyOutput removed and the assertion graduated to expected* per this corpus's contract." } ] }