From 325fc25c0142a1c88b605971b1538fd8f5a7ba46 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 17 Aug 2026 11:39:01 -0400 Subject: [PATCH] fix(#3569): require a digit-bearing phase id in the stats heading scan (#3591) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#3569): pin stats phase-id shape — inline-code mentions produce no phantom row Failing-first regression for #3569: cmdStats' heading scan accepted any word as a phase id, so prose mentioning ### Phase N: inside inline code inflated phases_total and disagreed with roadmap analyze. New adversarial fixture phase-heading-inside-inline-code.md (blockquote + bare mention), parity assertion against roadmap analyze, and over-narrowing guards for decimal / milestone-prefixed / letter-prefixed ids. * fix(#3569): require a digit-bearing phase id in the stats heading scan cmdStats' hand-rolled heading pattern captured any word as a phase id, so a ### Phase N: token inside an inline code span (the issue's blockquote) produced a phantom Not-Started row that could never complete, inflating phases_total and deflating percent forever. The id capture is now the canonical #3036 shape roadmap.cts uses (digit required; letter-prefixed, decimal, and milestone-prefixed ids keep counting), so stats and roadmap analyze agree. * fix(#3569): sanction the stats id-shape literal; correct zero-padded expectation Review findings: the phase-id drift guard requires the // phase-id-owner: comment directly above the regex (same form as roadmap.cts); the milestone-prefixed over-narrowing guard must expect normalizePhaseName's zero-padded 02-01 form, not the raw 2-01 token. * chore(#3569): add changeset fragment * chore(#3569): backfill changeset pr number --------- Co-authored-by: sim --- .changeset/gallant-jaguars-forage.md | 5 + src/commands.cts | 8 +- .../phase-heading-inside-inline-code.md | 17 +++ tests/stats-phase-id-shape.test.cjs | 102 ++++++++++++++++++ 4 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 .changeset/gallant-jaguars-forage.md create mode 100644 tests/fixtures/adversarial/roadmap/phase-heading-inside-inline-code.md create mode 100644 tests/stats-phase-id-shape.test.cjs diff --git a/.changeset/gallant-jaguars-forage.md b/.changeset/gallant-jaguars-forage.md new file mode 100644 index 000000000..8db047888 --- /dev/null +++ b/.changeset/gallant-jaguars-forage.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3591 +--- +**`gsd-tools stats` no longer counts phantom phases from inline code** — prose mentioning `### Phase N:` inside an inline code span (e.g. a roadmap explaining its own numbering) inflated phases_total with a never-completing Not-Started row and deflated completion percent; stats now requires the same digit-bearing phase id shape roadmap analyze uses, so the two agree. (#3569) diff --git a/src/commands.cts b/src/commands.cts index 3136c12ed..18da38a91 100644 --- a/src/commands.cts +++ b/src/commands.cts @@ -2178,7 +2178,13 @@ function cmdStats(cwd: string, format: string | undefined, raw: boolean): void { // Matches both plain numeric (Phase 1:) and milestone-prefixed (Phase 2-01:) headings. // Also tolerates optional [bracket-token] scope prefix on phase headings. // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const headingPattern = /#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:\s*([^\n]+)/gi; + // #3569: the id capture is the canonical #3036 shape (digit REQUIRED — incl. + // letter-prefixed B7, decimals, milestone 2-01), the same group roadmap.cts's + // collectAnalyzePhases uses. The former `([\w][\w.-]*)` matched ANY word, so + // prose mentioning `### Phase N:` inside an inline code span produced a phantom + // Not-Started row and made phases_total disagree with roadmap analyze. + // phase-id-owner: uses the [.-] (dot-or-dash) separator variant, not the canonical dot-only token; a swap to PHASE_NUMBER_TOKEN_SOURCE would drop hyphenated phase-id matches. + const headingPattern = /#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+([A-Za-z]?\d+[A-Z]?(?:[.-]\d+)*)(?:\s*\([^)\n]{0,200}\))?\s*:\s*([^\n]+)/gi; let match: RegExpExecArray | null; while ((match = headingPattern.exec(roadmapContent)) !== null) { // #3185: the heading seed carried no sentinel filter, so a diff --git a/tests/fixtures/adversarial/roadmap/phase-heading-inside-inline-code.md b/tests/fixtures/adversarial/roadmap/phase-heading-inside-inline-code.md new file mode 100644 index 000000000..a52aeab1b --- /dev/null +++ b/tests/fixtures/adversarial/roadmap/phase-heading-inside-inline-code.md @@ -0,0 +1,17 @@ +# Roadmap: Repro + +Some intro prose mentioning `### Phase N:` mid-paragraph, outside any blockquote. + +## Phases + +- [x] **Phase 1: Real Phase** - The only real phase (completed 2026-01-01) + +> Phase numbering resets on a major-version bump. Archived numbers live in +> `milestones/` and never collide with the active `### Phase N:` headers. + +## Phase Details + +### Phase 1: Real Phase + +**Goal**: Prove the parser counts one phase, not two (#3569 repro — heading token +inside an inline code span, both bare and inside a blockquote). diff --git a/tests/stats-phase-id-shape.test.cjs b/tests/stats-phase-id-shape.test.cjs new file mode 100644 index 000000000..9c72ab043 --- /dev/null +++ b/tests/stats-phase-id-shape.test.cjs @@ -0,0 +1,102 @@ +'use strict'; + +/** + * stats — phase-id shape contract (#3569) + * + * `gsd-tools stats` hand-rolls a phase-heading scan whose id capture accepted ANY + * word (`[\w][\w.-]*`), so prose mentioning `` `### Phase N:` `` inside an inline + * code span produced a phantom Not-Started row that could never complete — and + * `phases_total` disagreed with `roadmap analyze`, whose canonical scan requires + * a digit-bearing id (#3036 shape). These tests pin the agreement and the id shape. + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { createTempProject, cleanup, runGsdTools } = require('./helpers.cjs'); + +const ADVERSARIAL_ROADMAP_DIR = path.join(__dirname, 'fixtures', 'adversarial', 'roadmap'); + +function projectWithRoadmap(t, markdown) { + const tmpDir = createTempProject('gsd-bug-3569-'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), markdown); + t.after(() => cleanup(tmpDir)); + return tmpDir; +} + +function statsJson(tmpDir) { + const result = runGsdTools('stats json', tmpDir); + assert.ok(result.success, `stats json failed: ${result.error}`); + return JSON.parse(result.output); +} + +function analyzeJson(tmpDir) { + const result = runGsdTools('roadmap analyze json', tmpDir); + assert.ok(result.success, `roadmap analyze json failed: ${result.error}`); + return JSON.parse(result.output); +} + +describe('bug #3569: stats phantom phase from ### Phase N: inside inline code', () => { + test('#3569: stats ignores ### Phase N: inside inline code — exactly one phase row', (t) => { + // The issue's exact repro: a blockquote explaining the roadmap's own numbering + // mentions `### Phase N:` in an inline code span (plus a bare mid-paragraph + // mention). Pre-fix, cmdStats' heading scan accepted the digit-free id "N", + // producing a phantom Not-Started row that could never complete. + const markdown = fs.readFileSync(path.join(ADVERSARIAL_ROADMAP_DIR, 'phase-heading-inside-inline-code.md'), 'utf-8'); + const tmpDir = projectWithRoadmap(t, markdown); + const output = statsJson(tmpDir); + assert.ok(Array.isArray(output.phases), `expected phases array, got: ${typeof output.phases}`); + assert.deepEqual( + output.phases.map((p) => p.number).sort(), + ['01'], + 'exactly one phase — the inline-code "N" mention must not produce a row', + ); + assert.equal(output.phases_total, 1, 'phases_total must agree with the row count'); + }); + + test('#3569: stats and roadmap analyze agree on the inline-code fixture', (t) => { + const markdown = fs.readFileSync(path.join(ADVERSARIAL_ROADMAP_DIR, 'phase-heading-inside-inline-code.md'), 'utf-8'); + const tmpDir = projectWithRoadmap(t, markdown); + const stats = statsJson(tmpDir); + const analyze = analyzeJson(tmpDir); + const analyzeCount = Array.isArray(analyze.phases) ? analyze.phases.length : 0; + assert.equal(analyzeCount, 1, 'parity control: roadmap analyze itself counts one phase'); + assert.equal( + stats.phases.length, + analyzeCount, + 'stats and roadmap analyze must agree on phase count (the disagreement IS the bug)', + ); + }); + + test('#3569: digit-required id shape still counts decimal phases (decimal-phase-mixed fixture)', (t) => { + const markdown = fs.readFileSync(path.join(ADVERSARIAL_ROADMAP_DIR, 'decimal-phase-mixed.md'), 'utf-8'); + const tmpDir = projectWithRoadmap(t, markdown); + const output = statsJson(tmpDir); + const numbers = output.phases.map((p) => p.number); + assert.ok(numbers.length >= 1, 'fixture carries at least one decimal phase'); + for (const num of numbers) { + assert.match(num, /\d/, `phase id "${num}" is digit-bearing — decimals must keep counting (over-narrowing guard)`); + } + }); + + test('#3569: milestone-prefixed and letter-prefixed phase ids still count', (t) => { + const markdown = [ + '# Roadmap', + '', + '### Phase 2-01: Milestone Scoped', + '**Goal:** G', + '', + '### Phase B7: Letter Prefixed', + '**Goal:** G', + '', + ].join('\n'); + const tmpDir = projectWithRoadmap(t, markdown); + const output = statsJson(tmpDir); + assert.deepEqual( + output.phases.map((p) => p.number).sort(), + ['02-01', 'B7'], + 'canonical id shapes (milestone-prefixed — normalizePhaseName zero-pads the segment — and letter-prefixed #3036) must survive the digit requirement', + ); + }); +});