golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(#2137): parse the ROADMAP Progress table by header, not fixed column count

Browse Source 
deriveProgressFromRoadmap read the `## Progress` table with two
4-column-only regexes, so every project past its v1.0 milestone — which
uses the 5-column milestone-grouped shape the same template ships
(| Phase | Milestone | Plans Complete | Status | Completed |) — parsed to
{completedPhases: null, totalPhases: null, totalPlans: null}. Because the
body is wrapped in a bare try/catch the failure was silent, and
phase.complete flipped the ROADMAP row to Complete but never advanced
STATE's progress percent / completed-phase count.

Parse the table by header instead: locate the `## Progress` section
(line-anchored h2, mirroring the writer's #2012 scoping so an earlier
Phase/Status/Completed-shaped table can't hijack it), read the header row
to find the Status, Completed, and plans columns by name, then index each
data row positionally. One code path now handles both the 4-column
greenfield and 5-column milestone-grouped shapes — the same understanding
the writer (cmdPhaseComplete, with its `cells.length === 5` branch)
already had. Preserves the 999.x backlog exclusion, M/N plan summing, and
the prior contract that a count of 0 is reported as null.

Regression tests fold into tests/state.test.cjs: the template's own
5-column Progress block parses non-null, both shapes derive identically,
999.x stays excluded in the 5-column shape, and decoy tables (an earlier
## Retrospective table and an ### Progress h3 subheading) do not hijack
the scope.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Behruz Nassre Esfahani
2026-07-10 23:57:15 -07:00
parent e3a8c063b8
commit 3f6be47db9
2 changed files with 208 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

114
src/phase-lifecycle.cts
View File

@@ -37,43 +37,93 @@ export function deriveProgressFromRoadmap(roadmapContent: string): RoadmapProgre
let totalPlans: number | null = null;
try {
// Count Complete rows in the progress table (Status column = "Complete").
// Pattern: row where the phase cell starts with a digit (data row, not header),
// followed by any cell content, then a "Complete" status cell.
// Handles both short form ("| 4. |") and long form ("| 01. Foundation |").
// See phase-lifecycle.ts ~line 1655 for the original SDK pattern.
const tableCompletePattern = /\|\s*\d+[^|]*\|\s*[^|]*\|\s*Complete\s*\|/gi;
const completeMatches = roadmapContent.match(tableCompletePattern);
completedPhases = completeMatches ? completeMatches.length : null;
// Parse the Progress table by HEADER, not by a fixed column count. The
// writer (cmdPhaseComplete) already branches on `cells.length === 5`, so it
// understands both the 4-column greenfield table
// | Phase | Plans Complete | Status | Completed |
// and the 5-column milestone-grouped table the same template ships
// | Phase | Milestone | Plans Complete | Status | Completed |
// The reader used two 4-column-only regexes, so every project past its v1.0
// milestone (5-column shape) parsed to all-null and phase.complete silently
// skipped the STATE progress write. Reading column indices by NAME keeps the
// reader and writer in agreement across both shapes and any future column
// (#2137). The table is located by its header row rather than a `## Progress`
// heading because some callers pass a milestone slice with no heading (#1445).
//
// When a `## Progress` heading IS present, scope the search to that section
// (mirroring the writer's #2012 scoping in cmdPhaseComplete) so the reader
// cannot bind to an earlier Phase/Status/Completed-shaped table elsewhere in
// the roadmap. Callers that pass a headingless milestone slice fall back to
// scanning the whole input.
// Line-anchored h2 match — `indexOf('## Progress')` would also match inside
// an h3 `### Progress` (the `## Progress` substring starts at the 2nd hash),
// letting a decoy subheading hijack the slice.
// Case-insensitive to match the case-insensitive header-cell comparison below.
const progressMatch = roadmapContent.match(/^##[ \t]+Progress\b/im);
let scoped = roadmapContent;
if (progressMatch && progressMatch.index !== undefined) {
// Slice from `## Progress` to the next h1/h2 heading (or end); h3+ headings
// inside the section do not terminate it. The heading sits at index 0 of
// this slice with no leading newline, so the `\n#` search cannot match it.
const afterHeading = roadmapContent.slice(progressMatch.index);
const nextHeading = afterHeading.search(/\n#{1,2}[ \t]/);
scoped = nextHeading >= 0 ? afterHeading.slice(0, nextHeading) : afterHeading;
}
const lines = scoped.split('\n');
// Count total phase rows in the progress table.
// Identify the table by looking for Phase|...|Status|...|Completed header.
const progressTableMatch = roadmapContent.match(
// allow-adhoc-markdown: table-scoped regex with heading lookahead as stop; table parsing, out of seam scope; pending #1372
/\|\s*Phase\s*\|[^|]*\|[^|]*Status[^|]*\|[^|]*Completed[^|]*\|[\s\S]*?(?=\n\n|\n##|$)/i,
);
if (progressTableMatch) {
const tableText = progressTableMatch[0];
// Count data rows (rows starting with pipe then a phase number),
// excluding 999.x backlog phases. Mirrors init.cts /^999(?:\.|$)/ filter.
const dataRowPattern = /^\|\s*(\d+[^|]*)\|/gm;
let dataRowCount = 0;
let drm: RegExpExecArray | null;
while ((drm = dataRowPattern.exec(tableText)) !== null) {
if (/^999\b/.test(drm[1].trim())) continue;
dataRowCount++;
// Split a markdown table row into trimmed cells — the same
// `split('|').slice(1, -1)` boundary the writer uses (phase.cts).
const rowCells = (line: string): string[] =>
line.split('|').slice(1, -1).map((c) => c.trim());
const isTableRow = (line: string): boolean => line.trim().startsWith('|');
const isSeparatorRow = (cells: string[]): boolean =>
cells.length > 0 && cells.every((c) => /^:?-+:?$/.test(c));
let headerLine = -1;
let phaseIdx = -1;
let statusIdx = -1;
let plansIdx = -1;
for (let i = 0; i < lines.length; i++) {
if (!isTableRow(lines[i])) continue;
const lc = rowCells(lines[i]).map((c) => c.toLowerCase());
const p = lc.indexOf('phase');
const s = lc.indexOf('status');
const c = lc.indexOf('completed');
if (p >= 0 && s >= 0 && c >= 0) {
headerLine = i;
phaseIdx = p;
statusIdx = s;
plansIdx = lc.findIndex((h) => h.includes('plans'));
break;
}
totalPhases = dataRowCount > 0 ? dataRowCount : null;
}
// Sum plan counts from M/N columns in progress table
let totalPlansSum = 0;
const planCellPattern = /\|\s*\d+[^|]*\|\s*(\d+)\/(\d+)\s*\|/gi;
let pm: RegExpExecArray | null;
while ((pm = planCellPattern.exec(roadmapContent)) !== null) {
totalPlansSum += parseInt(pm[2], 10);
if (headerLine >= 0) {
let phaseCount = 0;
let completedCount = 0;
let plansSum = 0;
// Walk the contiguous rows after the header; a markdown table ends at the
// first non-`|` line.
for (let i = headerLine + 1; i < lines.length; i++) {
if (!isTableRow(lines[i])) break;
const cells = rowCells(lines[i]);
if (isSeparatorRow(cells)) continue;
const phaseToken = (cells[phaseIdx] ?? '').trim();
if (!/^\d/.test(phaseToken)) continue; // not a data row
if (/^999\b/.test(phaseToken)) continue; // 999.x backlog sentinel (#1445)
phaseCount++;
if ((cells[statusIdx] ?? '').toLowerCase() === 'complete') completedCount++;
if (plansIdx >= 0) {
const mn = (cells[plansIdx] ?? '').match(/^(\d+)\/(\d+)$/);
if (mn) plansSum += parseInt(mn[2], 10);
}
}
// Preserve the prior contract: a count of 0 is reported as null (absent),
// so the consumer leaves the existing STATE value untouched.
completedPhases = completedCount > 0 ? completedCount : null;
totalPhases = phaseCount > 0 ? phaseCount : null;
totalPlans = plansSum > 0 ? plansSum : null;
}
if (totalPlansSum > 0) totalPlans = totalPlansSum;
} catch { /* intentionally empty — fall through to existing values */ }
return { completedPhases, totalPhases, totalPlans };

126
tests/state.test.cjs
View File

@@ -9447,6 +9447,132 @@ describe('bug #1445 — deriveProgressFromRoadmap excludes 999.x rows', () => {
});
});
// ─── #2137: header-driven parse handles the milestone-grouped (5-col) table ──
//
// Regression for #2137: deriveProgressFromRoadmap read the `## Progress` table
// with two 4-column-only regexes. Every project past its v1.0 milestone uses the
// 5-column milestone-grouped shape the same template ships, so the reader (which
// only understood 4 columns) returned { null, null, null } while the writer
// (cmdPhaseComplete, with its explicit `cells.length === 5` branch) happily wrote
// it — and phase.complete then silently skipped the STATE progress update. The
// fix reads columns by NAME, so both shapes parse identically. These tests would
// fail against the pre-fix 4-column regexes (which returned all-null for 5-col).
describe('#2137 regression: deriveProgressFromRoadmap parses the milestone-grouped 5-column table', () => {
test("the template's own 5-column milestone-grouped Progress block parses non-null", () => {
// Byte-identical to gsd-core/templates/roadmap.md's "Milestone-Grouped
// Roadmap" Progress block — the exact shape that silently returned all-null.
const roadmap = [
'## Progress',
'',
'| Phase | Milestone | Plans Complete | Status | Completed |',
'|-------|-----------|----------------|--------|-----------|',
'| 1. Foundation | v1.0 | 3/3 | Complete | YYYY-MM-DD |',
'| 2. Features | v1.0 | 2/2 | Complete | YYYY-MM-DD |',
'| 5. Security | v1.1 | 0/2 | Not started | - |',
].join('\n');
const result = deriveProgressFromRoadmap(roadmap);
assert.equal(result.totalPhases, 3, `totalPhases must be 3 (5-col table must parse). Got ${result.totalPhases}`);
assert.equal(result.completedPhases, 2, `completedPhases must be 2 (Status is column 4 in the 5-col shape). Got ${result.completedPhases}`);
assert.equal(result.totalPlans, 7, `totalPlans must be 3+2+2=7 (Plans is column 3 in the 5-col shape). Got ${result.totalPlans}`);
});
test('the 4-column greenfield and 5-column milestone-grouped shapes derive the same progress', () => {
// The reader must agree with the writer on both shapes the template ships.
const fiveCol = [
'## Progress',
'| Phase | Milestone | Plans Complete | Status | Completed |',
'| --- | --- | --- | --- | --- |',
'| 1. Foundation | v1.0 | 3/3 | Complete | 2026-01-01 |',
'| 2. Features | v1.0 | 2/2 | Complete | 2026-01-02 |',
].join('\n');
const fourCol = [
'## Progress',
'| Phase | Plans Complete | Status | Completed |',
'| --- | --- | --- | --- |',
'| 1. Foundation | 3/3 | Complete | 2026-01-01 |',
'| 2. Features | 2/2 | Complete | 2026-01-02 |',
].join('\n');
assert.deepEqual(
deriveProgressFromRoadmap(fiveCol),
deriveProgressFromRoadmap(fourCol),
'the milestone-grouped and greenfield shapes must derive identical progress',
);
assert.deepEqual(deriveProgressFromRoadmap(fiveCol), {
completedPhases: 2,
totalPhases: 2,
totalPlans: 5,
});
});
test('999.x backlog rows stay excluded in the 5-column shape', () => {
const roadmap = [
'## Progress',
'| Phase | Milestone | Plans Complete | Status | Completed |',
'| --- | --- | --- | --- | --- |',
'| 1. Alpha | v1.0 | 2/2 | Complete | 2026-01-01 |',
'| 2. Beta | v1.0 | 1/1 | Complete | 2026-01-02 |',
'| 999.1 Future | v2.0 | 0/0 | Backlog | - |',
].join('\n');
const result = deriveProgressFromRoadmap(roadmap);
assert.equal(result.totalPhases, 2, `999.1 backlog row must be excluded in the 5-col shape too. Got ${result.totalPhases}`);
assert.equal(result.completedPhases, 2, `completedPhases must be 2. Got ${result.completedPhases}`);
});
test('binds to the ## Progress table, not an earlier Phase/Status/Completed-shaped table', () => {
// A decoy table under a different heading shares the Phase/Status/Completed
// header shape. The reader must scope to ## Progress (mirroring the writer's
// #2012 scoping) rather than binding to the first matching table it sees.
const roadmap = [
'## Retrospective',
'',
'| Phase | Owner | Status | Completed |',
'| --- | --- | --- | --- |',
'| 1. Old | jo | Complete | 2025-01-01 |',
'',
'## Progress',
'',
'| Phase | Milestone | Plans Complete | Status | Completed |',
'| --- | --- | --- | --- | --- |',
'| 1. Foundation | v1.0 | 3/3 | Complete | 2026-01-01 |',
'| 2. Features | v1.0 | 2/2 | Complete | 2026-01-02 |',
'| 3. Security | v1.1 | 0/2 | Not started | - |',
'',
'## Next',
].join('\n');
const result = deriveProgressFromRoadmap(roadmap);
assert.equal(result.totalPhases, 3, `must count the 3 rows of the ## Progress table, not the 1-row decoy. Got ${result.totalPhases}`);
assert.equal(result.completedPhases, 2, `must count Complete rows in ## Progress (2), not the decoy's 1. Got ${result.completedPhases}`);
assert.equal(result.totalPlans, 7, `must sum the ## Progress plans (3+2+2=7). Got ${result.totalPlans}`);
});
test('an h3 ### Progress decoy does not hijack the h2 ## Progress scope', () => {
// Heading detection must be line-anchored to h2: "### Progress".indexOf("## Progress")
// is 1, so a substring scan would start the slice inside the h3 subheading and
// miss the real table below.
const roadmap = [
'### Progress notes',
'',
'Some prose about progress, no table here.',
'',
'## Progress',
'',
'| Phase | Milestone | Plans Complete | Status | Completed |',
'| --- | --- | --- | --- | --- |',
'| 1. Foundation | v1.0 | 3/3 | Complete | 2026-01-01 |',
'| 2. Features | v1.0 | 2/2 | Complete | 2026-01-02 |',
].join('\n');
const result = deriveProgressFromRoadmap(roadmap);
assert.equal(result.totalPhases, 2, `h2 ## Progress table must be found past the h3 decoy. Got ${result.totalPhases}`);
assert.equal(result.completedPhases, 2, `completedPhases must be 2. Got ${result.completedPhases}`);
});
});
// ─── Scenario B: state json total_phases via roadmapPhaseCount ───────────────
describe('bug #1445 — state json excludes 999.x phase headings from total_phases', () => {