* test(#3577): pin table-declared phase resolution across all four surfaces Failing-first regression for #3577: a GFM table phase listing (Phase header, id in the first data cell) declared real phases that roadmap.analyze, roadmap.get-phase, init.phase-op, and the milestone filter all reported as absent (phase_count: 0 / found: false). Rows pin the lookup, the scope probe, the analyzer, schema discrimination against the canonical RoadmapProgress table, fenced-example exclusion, heading+table union without double-count, decimal ids, and the 999 icebox exclusion. * fix(#3577): recognize markdown-table phase rows across the enumeration family A GFM table whose header leads with Phase and whose data rows carry the id in the first cell is a phase listing — the #2199 bullet blind spot's table sibling. collectTablePhaseRows (schema-discriminated against the canonical RoadmapProgress table via matchTableSchema, fence-aware via stripFencedCode, digit-bearing id shape, 999 icebox excluded) now feeds: the milestone filter's sole owner scanMilestonePhaseIds, window classification hasPhaseEntries, both roadmap lookup chains (getRoadmapPhaseInternal + cmdRoadmapGetPhase, as last-resort tiers after heading and bullet), and roadmap analyze's enumerator (with the same disk enrichment contract as headings and a zero-pad-tolerant duplicate guard). init.phase-op resolves through its existing getRoadmapPhaseInternal fallback. * fix(#3577): GFM table termination + icebox word boundary in the table scan Review findings: the row harvest broke only on blank lines, so prose after a table (a bare date line) could be harvested as a phase id — rows now stop at the first non-row line per GFM semantics; the 999 icebox exclusion gains the heading scan's word boundary so 9991 is kept. * fix(#3577): sanction collectTablePhaseRows in the enumeration drift scanner The scan's local 999-only exclusion mirrors its parent owner scanMilestonePhaseIds' deliberate NOT-isSentinelPhaseId choice (a leading 0 is a real decimal phase, #2554), so it cannot route through the sentinel owner — function-scoped exemption with the documented reason, same entry shape as the #3262 owner's. * chore(#3577): add changeset fragment * chore(#3577): backfill changeset pr number --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
5
.changeset/kind-deer-caper.md
Normal file
5
.changeset/kind-deer-caper.md
Normal file
@@ -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)
|
||||
@@ -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'])],
|
||||
]);
|
||||
|
||||
|
||||
@@ -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<string> {
|
||||
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,
|
||||
};
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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}`);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user