diff --git a/.changeset/noble-deer-zip.md b/.changeset/noble-deer-zip.md new file mode 100644 index 000000000..041f308e0 --- /dev/null +++ b/.changeset/noble-deer-zip.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4511 +--- +**Roadmap phase tables now require an explicit name column** — ordinary status tables can no longer mint bogus names or hide missing phase details. diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index d4ae05690..899ffb006 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -63,7 +63,7 @@ import unusableInputMod = require('./unusable-input.cjs'); const { UNUSABLE_REASON, warnUnusableInput } = unusableInputMod; import { tokenizeHeadings, stripTaggedBlocks, withSection, stripFencedCode, collectSection } from './markdown-sectionizer.cjs'; import type { HeadingToken } from './markdown-sectionizer.cjs'; -import { findTableWithColumns, matchTableSchema, isDelimiterRow, splitTableRow } from './markdown-table.cjs'; +import { findTableWithColumns, isDelimiterRow, splitTableRow } from './markdown-table.cjs'; import type { MarkdownTable } from './markdown-table.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import planningScopeMod = require('./planning-scope.cjs'); @@ -645,16 +645,17 @@ function hasPhaseEntries(markdown: string, phaseIdConvention?: string | null): b } // ─── #3577: markdown-table phase listings ───────────────────────────────────── -// #3577: a GFM table declares phases when its header's FIRST cell is the literal -// `Phase` (optionally `Phase #` / `Phase No.` / `Phase number`) and the header does -// NOT match a known non-listing schema — the canonical RoadmapProgress table -// (`| Phase | Plans Complete | Status | Completed |`) leads with `Phase` too, and -// its rows are progress markers, not declarations. Data rows carry the phase id in -// their first cell (digit-bearing canonical shape — `Phase`-word header cells and -// `---` delimiter rows are digit-free and excluded by construction). Fence-aware -// via stripFencedCode, matching the #3184 lesson: a fenced EXAMPLE of the table -// form is not a declared phase. +// #3577/#4480: a GFM table declares phases only when its header's FIRST cell is +// the literal `Phase` (optionally `Phase #` / `Phase No.` / `Phase number`) AND +// it positively identifies a `Name` or `Phase Name` column. This fails closed: +// ordinary progress/summary tables such as `| Phase | Status |` cannot mint a +// phase whose name is whichever value happens to occupy column two. Data rows +// carry the phase id in their first cell (digit-bearing canonical shape — +// `Phase`-word header cells and `---` delimiter rows are digit-free and excluded +// by construction). Fence-aware via stripFencedCode, matching the #3184 lesson: +// a fenced EXAMPLE of the table form is not a declared phase. const PHASE_LISTING_HEADER_RE = /^\|?\s*phase(?:\s*(?:#|no\.?|number))?\s*\|/i; +const PHASE_NAME_HEADER_RE = /^(?:phase\s+)?name$/i; const TABLE_PHASE_ID_RE = /^[A-Za-z]?\d[\w.-]*$/; function collectTablePhaseRows(window: string): Array<{ id: string; name: string | null; row: string }> { @@ -664,7 +665,8 @@ function collectTablePhaseRows(window: string): Array<{ id: string; name: string for (let i = 0; i + 1 < lines.length; i++) { if (!PHASE_LISTING_HEADER_RE.test(lines[i])) continue; const headerCells = splitTableRow(lines[i]); - if (matchTableSchema(headerCells) !== null) continue; // canonical non-listing schema + const nameColumn = headerCells.findIndex((cell) => PHASE_NAME_HEADER_RE.test(cell)); + if (nameColumn === -1) continue; if (!isDelimiterRow(splitTableRow(lines[i + 1]))) continue; for (let j = i + 2; j < lines.length; j++) { // GFM semantics: the table ENDS at the first line that is not a table @@ -676,7 +678,8 @@ function collectTablePhaseRows(window: string): Array<{ id: string; name: string const first = cells[0] ?? ''; if (!TABLE_PHASE_ID_RE.test(first)) continue; if (!/^999\b/.test(first)) { - rows.push({ id: first, name: cells[1] && cells[1] !== '' ? cells[1] : null, row: lines[j] }); + const name = cells[nameColumn]; + rows.push({ id: first, name: name && name !== '' ? name : null, row: lines[j] }); } } } diff --git a/tests/roadmap-parser.test.cjs b/tests/roadmap-parser.test.cjs index 6712ce9f5..b7a638304 100644 --- a/tests/roadmap-parser.test.cjs +++ b/tests/roadmap-parser.test.cjs @@ -3580,7 +3580,7 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { const TABLE_ROADMAP = [ '# Roadmap: Table Repro', '', '## Milestone v2.0', '', - '| Phase | Focus | Requirements | Success criteria (preview) |', + '| Phase | Name | Requirements | Success criteria (preview) |', '| --- | --- | --- | --- |', '| 20 | Alpha focus | R1 | Works |', '| 21 | Beta focus | R2 | Works too |', @@ -3633,6 +3633,68 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { a3.ok(out.phases.some((p) => /Alpha focus/.test(p.phase_name || p.name || '')), 'phase 20 named from column 2'); }); + t3('#4480: table phase names are resolved by a recognized header', () => { + const nameRows = rp3.collectTablePhaseRows([ + '| Phase | Status | Name |', + '| --- | --- | --- |', + '| 1 | done | First Thing |', + ].join('\n')); + const phaseNameRows = rp3.collectTablePhaseRows([ + '| Phase | Phase Name | Status |', + '| --- | --- | --- |', + '| 2 | Second Thing | pending |', + ].join('\n')); + a3.deepStrictEqual(nameRows.map(({ id, name }) => ({ id, name })), [ + { id: '1', name: 'First Thing' }, + ]); + a3.deepStrictEqual(phaseNameRows.map(({ id, name }) => ({ id, name })), [ + { id: '2', name: 'Second Thing' }, + ]); + }); + + t3('#4480 property: the declared name column wins at every column position', () => { + const otherHeader = fc.constantFrom('Status', 'Goal', 'Plans', 'Owner'); + fc.assert(fc.property( + fc.array(otherHeader, { maxLength: 4 }), + fc.array(otherHeader, { maxLength: 4 }), + fc.constantFrom('Name', 'Phase Name'), + fc.integer({ min: 1, max: 998 }), + fc.stringMatching(/^[A-Za-z0-9][A-Za-z0-9 ]{0,30}$/), + (before, after, nameHeader, phase, name) => { + const headers = ['Phase', ...before, nameHeader, ...after]; + const values = [String(phase), ...before.map(() => 'x'), name, ...after.map(() => 'y')]; + const delimiter = headers.map(() => '---'); + const rows = rp3.collectTablePhaseRows([ + `| ${headers.join(' | ')} |`, + `| ${delimiter.join(' | ')} |`, + `| ${values.join(' | ')} |`, + ].join('\n')); + a3.deepStrictEqual(rows.map(({ id, name: parsedName }) => ({ id, name: parsedName })), [ + { id: String(phase), name: name.trim() }, + ]); + }, + ), { numRuns: 200 }); + }); + + t3('#4480: a status table cannot hide missing phase details', () => { + writeRoadmap3(tmpDir, [ + '# Roadmap', '', + '## Phases', '', + '- [x] **Phase 1: First Thing** — shipped', + '- [ ] **Phase 2: Second Thing** — not started', '', + '## Progress', '', + '| Phase | Status |', + '| --- | --- |', + '| 1 | done |', + '', + ].join('\n')); + const r = rgt3(['roadmap', 'analyze', 'json'], tmpDir); + a3.ok(r.success, `analyze failed: ${r.error}`); + const out = JSON.parse(r.output); + a3.strictEqual(out.phase_count, 0, `status rows are not declarations; got ${r.output}`); + a3.deepStrictEqual(out.missing_phase_details, ['1', '2']); + }); + t3('#3577: the canonical progress table is not a phase listing; fenced examples excluded', () => { writeRoadmap3(tmpDir, [ '# Roadmap', '', '## v1.0', '', @@ -3642,7 +3704,7 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { '| 3 | 1/2 | In Progress | |', '', '```md', - '| Phase | Focus |', + '| Phase | Name |', '| --- | --- |', '| 77 | fenced example |', '```', @@ -3664,7 +3726,7 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { writeRoadmap3(tmpDir, [ '# Roadmap', '', '## v1.0', '', '### Phase 1: Heading Form', '**Goal:** g', '', - '| Phase | Focus |', + '| Phase | Name |', '| --- | --- |', '| 1 | heading dup guard |', '| 2.5 | decimal row |',