From 4ee10f875fa9bf3e379304fd830ae7fd076fa738 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 31 May 2026 22:05:59 -0400 Subject: [PATCH] fix(#549): State progress writer over-counts total_phases by 1 when a decimal (inserted) phase exists (#561) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Import `extractCurrentMilestone` from `./core.cjs` into `state.cjs` - Replace `getMilestonePhaseFilter.phaseCount` usage in `buildStateFrontmatter` with a direct ROADMAP parse using the same digit-anchored pattern as `roadmap.analyze` — single source of truth for `total_phases` (#549) - Apply the same replacement in `cmdStateSync` for consistency - Add regression test: bug-549-total-phases-overcounts-with-phase-section-heading.test.cjs - Add changeset fragment: .changeset/549-total-phases-decimal-overcounting.md Co-authored-by: Claude Sonnet 4.6 --- .../549-total-phases-decimal-overcounting.md | 5 + get-shit-done/bin/lib/state.cjs | 47 +++- ...counts-with-phase-section-heading.test.cjs | 233 ++++++++++++++++++ 3 files changed, 279 insertions(+), 6 deletions(-) create mode 100644 .changeset/549-total-phases-decimal-overcounting.md create mode 100644 tests/bug-549-total-phases-overcounts-with-phase-section-heading.test.cjs diff --git a/.changeset/549-total-phases-decimal-overcounting.md b/.changeset/549-total-phases-decimal-overcounting.md new file mode 100644 index 000000000..ad1eb9ffc --- /dev/null +++ b/.changeset/549-total-phases-decimal-overcounting.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 549 +--- +State progress writer no longer over-counts `total_phases` by 1 when the ROADMAP contains a non-phase section heading (e.g. `## Phase Overview:`) that matched the looser regex in `getMilestonePhaseFilter`. Both `buildStateFrontmatter` and `cmdStateSync` now source `total_phases` from the same digit-anchored phase-heading pattern used by `roadmap.analyze` — single source of truth. diff --git a/get-shit-done/bin/lib/state.cjs b/get-shit-done/bin/lib/state.cjs index 941096850..4557b0df1 100644 --- a/get-shit-done/bin/lib/state.cjs +++ b/get-shit-done/bin/lib/state.cjs @@ -4,7 +4,7 @@ const fs = require('fs'); const path = require('path'); -const { escapeRegex, loadConfig, getMilestoneInfo, getMilestonePhaseFilter, output, error } = require('./core.cjs'); +const { escapeRegex, loadConfig, getMilestoneInfo, getMilestonePhaseFilter, extractCurrentMilestone, output, error } = require('./core.cjs'); const { platformWriteSync, platformReadSync, platformEnsureDir } = require('./shell-command-projection.cjs'); const { planningDir, planningPaths } = require('./planning-workspace.cjs'); const { realClock } = require('./clock.cjs'); @@ -886,9 +886,31 @@ function buildStateFrontmatter(bodyContent, cwd) { diskTotalSummaries += summaryCount; if (completed) diskCompletedPhases++; } + // Count phase headings from ROADMAP using a digit-containing pattern + // that matches both numeric phases (01, 05.1) and project-code phases + // (PROJ-42, CK-05) but excludes pure-word section headers like + // `## Phase Overview:` or `## Phase Details:` — single source of + // truth for total_phases (#549). + let roadmapPhaseCount = 0; + try { + const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); + const roadmapRaw = platformReadSync(roadmapPath); + if (roadmapRaw !== null) { + const roadmapScope = extractCurrentMilestone(roadmapRaw, cwd); + const phaseHeadingPattern = /#{2,4}\s*Phase\s+([\w][\w.-]*)\s*:/gi; + let m; + while ((m = phaseHeadingPattern.exec(roadmapScope)) !== null) { + // Only count tokens that contain at least one digit — excludes + // pure-word section headings (Overview, Details) while keeping + // numeric phases (01, 05.1) and project-code IDs (PROJ-42). + if (/\d/.test(m[1])) roadmapPhaseCount++; + } + } + } catch { /* fall through: phaseDirs.length used as sole count */ } + cached = { - totalPhases: isDirInMilestone.phaseCount > 0 - ? Math.max(phaseDirs.length, isDirInMilestone.phaseCount) + totalPhases: roadmapPhaseCount > 0 + ? Math.max(phaseDirs.length, roadmapPhaseCount) : phaseDirs.length, completedPhases: diskCompletedPhases, totalPlans: diskTotalPlans, @@ -1651,9 +1673,22 @@ function cmdStateSync(cwd, options, raw) { // Mirrors the logic in buildStateFrontmatter so both report consistent percents (#3242 Bug B). let syncTotalPhases = null; try { - const isDirInMilestone = getMilestonePhaseFilter(cwd); - if (isDirInMilestone.phaseCount > 0) { - syncTotalPhases = Math.max(entries.length, isDirInMilestone.phaseCount); + let roadmapPhaseCount = 0; + const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); + const roadmapRaw = platformReadSync(roadmapPath); + if (roadmapRaw !== null) { + const roadmapScope = extractCurrentMilestone(roadmapRaw, cwd); + const phaseHeadingPattern = /#{2,4}\s*Phase\s+([\w][\w.-]*)\s*:/gi; + let m; + while ((m = phaseHeadingPattern.exec(roadmapScope)) !== null) { + // Only count tokens that contain at least one digit — excludes + // pure-word section headings (Overview, Details) while keeping + // numeric phases (01, 05.1) and project-code IDs (PROJ-42). + if (/\d/.test(m[1])) roadmapPhaseCount++; + } + } + if (roadmapPhaseCount > 0) { + syncTotalPhases = Math.max(entries.length, roadmapPhaseCount); } else { syncTotalPhases = entries.length; } diff --git a/tests/bug-549-total-phases-overcounts-with-phase-section-heading.test.cjs b/tests/bug-549-total-phases-overcounts-with-phase-section-heading.test.cjs new file mode 100644 index 000000000..41abb1916 --- /dev/null +++ b/tests/bug-549-total-phases-overcounts-with-phase-section-heading.test.cjs @@ -0,0 +1,233 @@ +'use strict'; + +/** + * Regression test for bug #549: + * STATE.md progress.total_phases is over-counted by 1 when the ROADMAP contains + * a non-phase section heading that happens to match the broader pattern used by + * getMilestonePhaseFilter (e.g. `## Phase Overview:`, `## Phase Details:`). + * + * Root cause: + * buildStateFrontmatter sources total_phases from getMilestonePhaseFilter.phaseCount, + * which uses the looser regex `#{2,4}\s*Phase\s+([\w][\w.-]*)\s*:` to build its + * milestonePhaseNums set. That pattern matches section headings like + * `## Phase Overview:` and `## Phase Details:`, adding non-numeric tokens + * ("Overview", "Details") to the set and inflating phaseCount by 1 per heading. + * + * roadmap.analyze uses the stricter pattern + * `#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:\s*([^\n]+)` which requires a + * leading digit, so it only counts real phase headings. + * + * Fix: buildStateFrontmatter (and cmdStateSync) must source total_phases from + * the same digit-anchored phase-heading parser as roadmap.analyze — single + * source of truth. + * + * Scenario under test: + * ROADMAP with 6 integer phases (01-06) + 1 inserted decimal phase (05.1) = 7 + * phases, plus a `## Phase Overview:` section header. + * + * BEFORE fix: state json / state sync report total_phases: 8 (7 + 1 spurious + * "Overview" token from the getMilestonePhaseFilter pattern). + * AFTER fix: state json / state sync report total_phases: 7, matching + * roadmap.analyze.phase_count. + */ + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +// ROADMAP: 6 integer phases + 1 inserted decimal + `## Phase Overview:` section header. +// The section header must include a trailing `:` so it matches the getMilestonePhaseFilter +// broader pattern (the bug trigger). +const ROADMAP = `## Milestone v1.0: Test Milestone + +## Phase Overview: + +High-level narrative about the phases. + +### Phase 01: Alpha +**Goal:** alpha + +### Phase 02: Beta +**Goal:** beta + +### Phase 03: Gamma +**Goal:** gamma + +### Phase 04: Delta +**Goal:** delta + +### Phase 05: Epsilon +**Goal:** epsilon + +### Phase 05.1: Inserted Hotfix (INSERTED) +**Goal:** inserted hotfix + +### Phase 06: Zeta +**Goal:** zeta +`; + +describe('bug #549 — total_phases over-counted by non-phase section headings', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject('bug-549-'); + const planning = path.join(tmpDir, '.planning'); + + fs.writeFileSync(path.join(planning, 'ROADMAP.md'), ROADMAP, 'utf-8'); + fs.writeFileSync( + path.join(planning, 'STATE.md'), + [ + '---', + 'gsd_state_version: 1.0', + 'milestone: v1.0', + 'milestone_name: Test Milestone', + 'status: executing', + '---', + '', + '# Project State', + '', + '## Configuration', + 'Current Phase: 1', + 'Status: Executing Phase 1', + 'Last Activity: 2026-01-01', + '', + ].join('\n'), + 'utf-8', + ); + fs.writeFileSync(path.join(planning, 'config.json'), '{}', 'utf-8'); + + // Create 7 phase dirs (6 integer + 1 decimal). + const phaseDirs = [ + '01-alpha', + '02-beta', + '03-gamma', + '04-delta', + '05-epsilon', + '05.1-inserted-hotfix', + '06-zeta', + ]; + for (const d of phaseDirs) { + const dir = path.join(planning, 'phases', d); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'PLAN.md'), '# Plan\n', 'utf-8'); + } + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('state json total_phases matches roadmap.analyze phase_count (7, not 8)', () => { + // Authoritative count from roadmap.analyze — uses the digit-anchored pattern. + const analyzeResult = runGsdTools(['roadmap', 'analyze'], tmpDir); + assert.ok(analyzeResult.success, `roadmap analyze failed: ${analyzeResult.error}`); + const analyzed = JSON.parse(analyzeResult.output); + + assert.equal( + analyzed.phase_count, + 7, + `roadmap.analyze should count 7 phases (01-06 + 05.1), got ${analyzed.phase_count}`, + ); + + // State frontmatter must equal the authoritative count. + const stateResult = runGsdTools(['state', 'json'], tmpDir); + assert.ok(stateResult.success, `state json failed: ${stateResult.error}`); + const state = JSON.parse(stateResult.output); + + assert.ok(state.progress, 'state json must return a progress block'); + assert.equal( + state.progress.total_phases, + 7, + `progress.total_phases must be 7 (not 8) — ## Phase Overview: section must not be counted as a phase. Got ${state.progress.total_phases}`, + ); + assert.equal( + state.progress.total_phases, + analyzed.phase_count, + `progress.total_phases (${state.progress.total_phases}) must equal roadmap.analyze.phase_count (${analyzed.phase_count})`, + ); + }); + + test('state sync total_phases matches roadmap.analyze phase_count', () => { + // Add a Progress field to the body so cmdStateSync has something to update. + const statePath = path.join(tmpDir, '.planning', 'STATE.md'); + const before = fs.readFileSync(statePath, 'utf-8'); + fs.writeFileSync(statePath, before.replace('Last Activity: 2026-01-01', 'Last Activity: 2026-01-01\nProgress: [░░░░░░░░░░] 0%'), 'utf-8'); + + const syncResult = runGsdTools(['state', 'sync'], tmpDir); + assert.ok(syncResult.success, `state sync failed: ${syncResult.error}`); + + // Read frontmatter via state json (authoritative JSON path). + const stateResult = runGsdTools(['state', 'json'], tmpDir); + assert.ok(stateResult.success, `state json after sync failed: ${stateResult.error}`); + const state = JSON.parse(stateResult.output); + + assert.ok(state.progress, 'state json must return a progress block after sync'); + assert.equal( + state.progress.total_phases, + 7, + `state sync must write total_phases: 7, not 8. ## Phase Overview: must not inflate the count. Got ${state.progress.total_phases}`, + ); + }); + + test('integer-only project without decimal phase also counts correctly', () => { + // Regression guard: the fix must not break projects with no decimal phases. + const tmpDir2 = createTempProject('bug-549-integer-'); + try { + const planning2 = path.join(tmpDir2, '.planning'); + + // ROADMAP: 4 integer phases only + non-phase section heading. + fs.writeFileSync( + path.join(planning2, 'ROADMAP.md'), + [ + '## Milestone v1.0: Simple', + '', + '## Phase Overview:', + '', + '### Phase 01: One', + '**Goal:** one', + '', + '### Phase 02: Two', + '**Goal:** two', + '', + '### Phase 03: Three', + '**Goal:** three', + '', + '### Phase 04: Four', + '**Goal:** four', + ].join('\n'), + 'utf-8', + ); + fs.writeFileSync( + path.join(planning2, 'STATE.md'), + '---\ngsd_state_version: 1.0\nmilestone: v1.0\nstatus: executing\n---\n\n# State\n\nStatus: Executing Phase 1\nLast Activity: 2026-01-01\n', + 'utf-8', + ); + fs.writeFileSync(path.join(planning2, 'config.json'), '{}', 'utf-8'); + + for (const d of ['01-one', '02-two', '03-three', '04-four']) { + const dir = path.join(planning2, '.planning', 'phases', d); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'PLAN.md'), '# Plan\n', 'utf-8'); + } + + const analyzeResult = runGsdTools(['roadmap', 'analyze'], tmpDir2); + assert.ok(analyzeResult.success, `roadmap analyze failed: ${analyzeResult.error}`); + const analyzed = JSON.parse(analyzeResult.output); + assert.equal(analyzed.phase_count, 4, `expected 4 phases, got ${analyzed.phase_count}`); + + const stateResult = runGsdTools(['state', 'json'], tmpDir2); + assert.ok(stateResult.success, `state json failed: ${stateResult.error}`); + const state = JSON.parse(stateResult.output); + assert.ok(state.progress, 'state json must return a progress block'); + assert.equal( + state.progress.total_phases, + 4, + `integer-only project: total_phases must be 4, not 5. Got ${state.progress.total_phases}`, + ); + } finally { + cleanup(tmpDir2); + } + }); +});