fix(#4480): require a name column in roadmap phase tables (#4511)

* test(#4480): reproduce unnamed roadmap table phases

* fix(#4480): require named roadmap phase tables

* docs(#4480): add changeset for #4511

* test(#4480): cover phase name columns generatively

---------

Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
Michel Moreira
2026-09-10 19:32:55 -03:00
committed by GitHub
parent 5e2055ab90
commit 411199d08b
3 changed files with 85 additions and 15 deletions

View File

@@ -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.

View File

@@ -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] });
}
}
}

View File

@@ -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 |',