diff --git a/.changeset/kind-deer-caper.md b/.changeset/kind-deer-caper.md new file mode 100644 index 000000000..9dd568925 --- /dev/null +++ b/.changeset/kind-deer-caper.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3599 +--- +**`roadmap` tools recognize table-style phase listings** — a ROADMAP whose current-milestone phases are declared as markdown table rows (`| 20 | … |`) reported phase_count: 0 and found: false across roadmap.analyze, roadmap.get-phase, init.phase-op, and the milestone filter; all four surfaces now resolve table-declared phases (progress tables and fenced examples excluded). (#3577) diff --git a/scripts/lint-phase-enumeration-drift.cjs b/scripts/lint-phase-enumeration-drift.cjs index b8acc8d6c..924d1dfa8 100644 --- a/scripts/lint-phase-enumeration-drift.cjs +++ b/scripts/lint-phase-enumeration-drift.cjs @@ -119,6 +119,10 @@ * read the physical set. Its ENUMERATION path routes through the owner. * - `src/roadmap-parser.cts` `getMilestonePhaseFilter` and its #3262-extracted * set-building owner `scanMilestonePhaseIds` (the same two heading/ + * #3577 `collectTablePhaseRows` — the table-scan sibling feeding the same + * membership set; its local 999-only exclusion mirrors the owner's + * deliberate NOT-isSentinelPhaseId choice (a leading 0 is a real decimal + * phase, #2554), so it cannot route through the sentinel owner either). * bullet scans, lifted verbatim so the `roadmap milestone-scope` probe * reads the identical derivation): both deliberately use the local * `999`-only literal, NOT `isSentinelPhaseId`. That canonical predicate @@ -286,7 +290,7 @@ const FUNCTION_SCOPED_EXEMPTIONS = new Map([ [path.join('src', 'state.cts'), new Set(['cmdStateValidate', 'cmdStateSync', 'cmdStateRebuild'])], [path.join('src', 'roadmap-upgrade.cts'), new Set(['computeMigrationPlan'])], [path.join('src', 'smart-entry.cts'), new Set(['detectVerifyFailed'])], - [path.join('src', 'roadmap-parser.cts'), new Set(['getMilestonePhaseFilter', 'scanMilestonePhaseIds'])], + [path.join('src', 'roadmap-parser.cts'), new Set(['getMilestonePhaseFilter', 'scanMilestonePhaseIds', 'collectTablePhaseRows'])], [path.join('src', 'planning-snapshot.cts'), new Set(['buildAllPhaseDirNamesField'])], ]); diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index d2966b654..d28c3c1c5 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -42,7 +42,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 } from './markdown-table.cjs'; +import { findTableWithColumns, matchTableSchema, 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'); @@ -424,7 +424,48 @@ function hasPhaseEntries(markdown: string): boolean { // doc showing the convention) counts as a real phase entry. Strip fences // through the canonical seam before testing, matching tokenizeHeadings' // fence-awareness above. - return BULLET_PHASE_LINE_PATTERN.test(stripFencedCode(markdown).text); + if (BULLET_PHASE_LINE_PATTERN.test(stripFencedCode(markdown).text)) return true; + // #3577: a markdown-table phase listing also declares phases. + return collectTablePhaseRows(markdown).length > 0; +} + +// ─── #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. +const PHASE_LISTING_HEADER_RE = /^\|?\s*phase(?:\s*(?:#|no\.?|number))?\s*\|/i; +const TABLE_PHASE_ID_RE = /^[A-Za-z]?\d[\w.-]*$/; + +function collectTablePhaseRows(window: string): Array<{ id: string; name: string | null; row: string }> { + const unfenced = stripFencedCode(window).text; + const lines = unfenced.split(/\r?\n/); + const rows: Array<{ id: string; name: string | null; row: 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 + 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 + // row. Review finding: breaking only on blank lines let subsequent prose + // (e.g. a bare `2026-01-01` date line) be harvested as a phase id. + if (!/^\s*\|/.test(lines[j])) break; + const cells = splitTableRow(lines[j]); + if (cells.length === 0 || cells.every((c) => c === '')) break; // defensive: blank row + 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] }); + } + } + } + return rows; } /** @@ -465,6 +506,9 @@ function scanMilestonePhaseIds(window: string): Set { while ((bm = scanner.exec(unfenced)) !== null) { if (!/^999\b/.test(bm[1])) ids.add(bm[1]); } + // #3577: table-declared ids join the same membership set — the milestone + // filter must not collapse a table-house-style window to zero-count. + for (const tr of collectTablePhaseRows(window)) ids.add(tr.id); return ids; } @@ -884,6 +928,25 @@ function findRoadmapPhaseInContent(content: string, phaseNum: unknown, phaseSour }; } +// #3577: markdown-table row fallback. Mirrors the #2199 bullet fallback's +// tier — used only AFTER heading and bullet lookups fail on scoped + full +// content, so a heading with a Requirements/Goal section always wins. The row +// itself is the section (single line), the name comes from column 2. +function findRoadmapTablePhaseInContent(content: string, phaseNum: unknown): RoadmapPhaseResult | null { + const wanted = String(phaseNum).replace(/^0+(?=.)/, ''); + for (const tr of collectTablePhaseRows(content)) { + if (tr.id.replace(/^0+(?=.)/, '') !== wanted) continue; + return { + found: true, + phase_number: String(phaseNum), + phase_name: tr.name ?? `Phase ${tr.id}`, + goal: null, + section: tr.row.trim(), + }; + } + return null; +} + function findRoadmapBulletPhaseInContent(content: string, phaseNum: unknown, phaseSource?: string): RoadmapPhaseResult | null { // #2199: bullet/checkbox entry fallback (`- [ ] **Phase N — name**`). Returns // the single bullet line as the section (no multi-line body) — used only as a @@ -941,6 +1004,12 @@ function getRoadmapPhaseInternal(cwd: string, phaseNum: unknown): RoadmapPhaseRe if (fullBullet) return fullBullet; } + // #3577: last tier — a markdown-table row declaration. + const scopedTable = findRoadmapTablePhaseInContent(content, phaseNum); + if (scopedTable) return scopedTable; + const fullTable = findRoadmapTablePhaseInContent(fullContent, phaseNum); + if (fullTable) return fullTable; + return null; } catch (err) { // Absence already returned above via existsSync; anything caught here is a read fault @@ -1591,5 +1660,6 @@ export = { // CLI probe) and the free-text predicate the phase add/add-batch/insert // guards and the edit-phase workflow's pre/post capture are built on. scanMilestonePhaseIds, + collectTablePhaseRows, findMilestoneScopeHeadingLines, }; diff --git a/src/roadmap.cts b/src/roadmap.cts index 1971b851c..a41a03b0a 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -26,7 +26,7 @@ const { SCOPE } = planningScopeMod; type Scope = planningScopeMod.Scope; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserModule = require('./roadmap-parser.cjs'); -const { stripShippedMilestones, extractCurrentMilestone, extractCurrentMilestoneScoped, replaceInCurrentMilestone, listMilestoneHeadings, scanMilestonePhaseIds } = roadmapParserModule; +const { stripShippedMilestones, extractCurrentMilestone, extractCurrentMilestoneScoped, replaceInCurrentMilestone, listMilestoneHeadings, scanMilestonePhaseIds, collectTablePhaseRows } = roadmapParserModule; import { tokenizeHeadings } from './markdown-sectionizer.cjs'; import { updateTableCell } from './markdown-table.cjs'; import { clampPercent } from './phase-lifecycle.cjs'; @@ -311,6 +311,22 @@ function cmdRoadmapGetPhase(cwd: string, phaseNum: string, raw: boolean): void { if (!malformed) malformed = (milestoneResult?.error ? milestoneResult : (fullResult?.error ? fullResult : null)); } + // #3577: no heading or checklist entry matched — fall back to a + // markdown-table row declaration (the same last-resort tier + // getRoadmapPhaseInternal gained). Zero-pad-tolerant id compare (#3572 + // lesson: the declared form may be padded). + const stripPad = (s: string) => s.replace(/^0+(?=.)/, ''); + const tableHit = collectTablePhaseRows(milestoneContent).find((tr) => stripPad(tr.id) === stripPad(phaseNum)) + ?? collectTablePhaseRows(fullContent).find((tr) => stripPad(tr.id) === stripPad(phaseNum)); + if (tableHit) { + output( + { found: true, phase_number: phaseNum, phase_name: tableHit.name ?? `Phase ${tableHit.id}`, goal: null, section: tableHit.row.trim() }, + raw, + tableHit.row.trim(), + ); + return; + } + if (malformed) { output(malformed, raw, ''); return; @@ -459,6 +475,41 @@ function collectAnalyzePhases(content: string, phasesDir: string, phaseDirNames: roadmap_complete: roadmapComplete, }); } + + // #3577: markdown-table row declarations join the enumeration — same + // enrichment contract as headings (disk counts when the directory exists), + // zero-pad-tolerant duplicate guard so an id declared in BOTH a heading and + // a table counts once. + const stripPadA = (s: string) => s.replace(/^0+(?=.)/, ''); + const seen = new Set(phases.map((ph) => stripPadA(ph.number))); + for (const tr of collectTablePhaseRows(content)) { + if (seen.has(stripPadA(tr.id))) continue; + const dirMatchA = matchPhaseDirs(phaseDirNames, normalizePhaseName(tr.id)).matches[0]; + let tPlanCount = 0; + let tSummaryCount = 0; + let tHasContext = false; + let tHasResearch = false; + if (dirMatchA) { + const counts = countPhasePlansAndSummaries(path.join(phasesDir, dirMatchA)); + tPlanCount = counts.planCount; + tSummaryCount = counts.summaryCount; + tHasContext = fs.existsSync(path.join(phasesDir, dirMatchA, 'CONTEXT.md')); + tHasResearch = fs.existsSync(path.join(phasesDir, dirMatchA, 'RESEARCH.md')); + } + phases.push({ + number: tr.id, + name: tr.name ?? `Phase ${tr.id}`, + goal: null, + mode: null, + depends_on: null, + plan_count: tPlanCount, + summary_count: tSummaryCount, + has_context: tHasContext, + has_research: tHasResearch, + disk_status: dirMatchA ? 'ok' : 'no_directory', + roadmap_complete: false, + }); + } return phases; } diff --git a/tests/roadmap-parser.test.cjs b/tests/roadmap-parser.test.cjs index 73e93e6af..eae4bb804 100644 --- a/tests/roadmap-parser.test.cjs +++ b/tests/roadmap-parser.test.cjs @@ -3325,3 +3325,123 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { assert.strictEqual(UNUSABLE_REASON.ROADMAP_UNREADABLE, 'roadmap_unreadable'); }); }); + +// ─── #3577: markdown-table phase listings ──────────────────────────────────── +// Self-contained block: a ROADMAP whose current-milestone phase listing is a +// GFM table (`| Phase | ... |` header, id in the first data cell) declared real +// phases that every enumeration surface reported as absent (phase_count: 0, +// found: false) — the #2199 bullet blind spot's table sibling. Surfaces: the +// lookup chain (get-phase/phase-op), scanMilestonePhaseIds (milestone filter + +// scope probe), hasPhaseEntries (window classification), and roadmap analyze's +// enumerator. The canonical RoadmapProgress table also leads with `Phase` — +// schema discrimination is load-bearing. +{ + const { describe: d3, test: t3, beforeEach: be3, afterEach: ae3 } = require('node:test'); + const a3 = require('node:assert/strict'); + const fs3 = require('node:fs'); + const path3 = require('node:path'); + const { createTempProject: ctp3, cleanup: cu3, runGsdTools: rgt3 } = require('./helpers.cjs'); + const rp3 = require('../gsd-core/bin/lib/roadmap-parser.cjs'); + const writeRoadmap3 = (d, c) => fs3.writeFileSync(path3.join(d, '.planning', 'ROADMAP.md'), c); + + const TABLE_ROADMAP = [ + '# Roadmap: Table Repro', '', + '## Milestone v2.0', '', + '| Phase | Focus | Requirements | Success criteria (preview) |', + '| --- | --- | --- | --- |', + '| 20 | Alpha focus | R1 | Works |', + '| 21 | Beta focus | R2 | Works too |', + '', + ].join('\n'); + + d3('#3577 markdown-table phase listings resolve across surfaces', () => { + let tmpDir; + be3(() => { tmpDir = ctp3('fix-3577-'); }); + ae3(() => { cu3(tmpDir); }); + + t3('#3577: roadmap get-phase finds a table-declared phase', () => { + writeRoadmap3(tmpDir, TABLE_ROADMAP); + const p20 = rp3.getRoadmapPhaseInternal(tmpDir, '20'); + a3.ok(p20 && p20.found, 'phase 20 must resolve from its table row'); + a3.match(p20.phase_name, /Alpha focus/); + const absent = rp3.getRoadmapPhaseInternal(tmpDir, '99'); + a3.ok(!absent || !absent.found, 'an absent phase must not resolve'); + }); + + t3('#3577: init phase-op resolves a table-declared phase (third named tool)', () => { + writeRoadmap3(tmpDir, TABLE_ROADMAP); + const r = rgt3(['init', 'phase-op', '20'], tmpDir); + a3.ok(r.success, `init phase-op failed: ${r.error}`); + const out = JSON.parse(r.output); + a3.ok(out.found !== false, `phase-op must resolve the table-declared phase; got ${r.output.slice(0, 200)}`); + }); + + t3('#3577: scanMilestonePhaseIds sees table-declared ids (milestone filter)', () => { + writeRoadmap3(tmpDir, TABLE_ROADMAP); + const scoped = rp3.extractCurrentMilestoneScoped( + fs3.readFileSync(path3.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf8'), + tmpDir, + ); + const ids = rp3.scanMilestonePhaseIds(scoped.value); + a3.ok(ids.has('20') || [...ids].some((i) => i.replace(/^0+/, '') === '20'), `ids must contain 20; got ${[...ids]}`); + a3.ok(ids.has('21') || [...ids].some((i) => i.replace(/^0+/, '') === '21'), `ids must contain 21; got ${[...ids]}`); + }); + + t3('#3577: roadmap analyze counts table-declared phases', () => { + writeRoadmap3(tmpDir, TABLE_ROADMAP); + const r = rgt3(['roadmap', 'analyze', 'json'], tmpDir); + a3.ok(r.success, `roadmap analyze json failed: ${r.error}`); + const out = JSON.parse(r.output); + a3.ok(Array.isArray(out.phases), `expected phases array; got ${r.output.slice(0, 200)}`); + a3.strictEqual(out.phases.length, 2, `both table phases counted; got ${out.phases.length}`); + a3.ok(out.phases.some((p) => /Alpha focus/.test(p.phase_name || p.name || '')), 'phase 20 named from column 2'); + }); + + t3('#3577: the canonical progress table is not a phase listing; fenced examples excluded', () => { + writeRoadmap3(tmpDir, [ + '# Roadmap', '', '## v1.0', '', + '## Progress', '', + '| Phase | Plans Complete | Status | Completed |', + '| --- | --- | --- | --- |', + '| 3 | 1/2 | In Progress | |', + '', + '```md', + '| Phase | Focus |', + '| --- | --- |', + '| 77 | fenced example |', + '```', + '', + ].join('\n')); + const scoped = rp3.extractCurrentMilestoneScoped( + fs3.readFileSync(path3.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf8'), + tmpDir, + ); + const ids = [...rp3.scanMilestonePhaseIds(scoped.value)].map((i) => i.replace(/^0+/, '')); + a3.ok(!ids.includes('3'), `a RoadmapProgress row is a progress marker, not a declaration; got ${ids}`); + a3.ok(!ids.includes('77'), `a fenced table example must not count; got ${ids}`); + const p77 = rp3.getRoadmapPhaseInternal(tmpDir, '77'); + a3.ok(!p77 || !p77.found, 'a fenced table row must not resolve'); + }); + + t3('#3577: heading and table declarations union without duplicates; decimals count, 999 excluded', () => { + writeRoadmap3(tmpDir, [ + '# Roadmap', '', '## v1.0', '', + '### Phase 1: Heading Form', '**Goal:** g', '', + '| Phase | Focus |', + '| --- | --- |', + '| 1 | heading dup guard |', + '| 2.5 | decimal row |', + '| 999 | icebox row |', + '', + ].join('\n')); + const r = rgt3(['roadmap', 'analyze', 'json'], tmpDir); + a3.ok(r.success, `analyze failed: ${r.error}`); + const out = JSON.parse(r.output); + const nums = out.phases.map((p) => p.phase_number || p.number).sort(); + a3.ok(nums.includes('1'), 'heading phase present'); + a3.strictEqual(nums.filter((n) => n === '1' || n === '01').length, 1, `id declared in BOTH heading and table counts once; got ${nums}`); + a3.ok(nums.some((n) => n.replace(/^0+/, '') === '2.5'), `decimal table id counted; got ${nums}`); + a3.ok(!nums.some((n) => /^0*999/.test(n)), `icebox 999 excluded; got ${nums}`); + }); + }); +}