fix(#4213): keep STATE.md progress surfaces synchronized (#4231)

* fix(#4213): keep STATE.md progress surfaces synchronized

* fix(#4213): clamp the shared progress bar and keep bold-first priority, changeset + property tests

- formatProgressMachineSegment clamps through clampPercentFromFraction
  (ADR-3180 Decision 7 kernel) with a 0 floor, so a hand-edited
  out-of-range persisted percent renders a clamped bar instead of
  throwing RangeError on repeat() inside the write seam
- stateReplaceProgressPercent restores the #2177 bold-first priority:
  **Progress:** anywhere in the body wins; a plain ^Progress: line is
  the fallback, so free text starting with Progress: cannot capture
  the rewrite ahead of the real status line
- cross-reference comment names the three consumers and the
  cmdStateSync sanctioned exception (ADR-3408 §8.3)
- CONTEXT.md: applyPostSyncPreservation reconciliation documented in
  the STATE.md Transition Module entry
- property tests (never-throws/well-formed, idempotency, round-trip,
  bold-first) + two regression rows through the CLI

---------

Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
Atirna
2026-09-05 17:26:06 +05:30
committed by GitHub
parent dad16b6ef9
commit 70f22e4643
6 changed files with 261 additions and 42 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 4231
---
**`state` verbs keep the STATE.md body Progress bar in sync with frontmatter `progress.percent`** — 13 of 15 verbs rewrote the frontmatter percent while the body bar stayed stale (issue #4213: frontmatter 75, body bar still 50), so the two surfaces silently diverged on every record-session, add-decision and milestone switch. The bar is now rewritten through one shared helper on the write seam, keeping the bold `**Progress:**` status line the target even when a free-text line above it starts with `Progress:`, and an out-of-range persisted percent renders a clamped 0-100 bar instead of crashing the write. (#4213)

File diff suppressed because one or more lines are too long

View File

@@ -20,7 +20,7 @@ import { stateReplaceField, stateExtractField, stateReplaceFieldIfTemplate, stat
import { KNOWN_TEMPLATE_DEFAULTS, toFiniteNumber } from './state-document.cjs';
import { tokenizeHeadings } from './markdown-sectionizer.cjs';
import type { HeadingToken } from './markdown-sectionizer.cjs';
import { deriveProgressFromRoadmap, clampPercent } from './phase-lifecycle.cjs';
import { deriveProgressFromRoadmap, clampPercent, clampPercentFromFraction } from './phase-lifecycle.cjs';
import { escapeRegex } from './pattern.cjs';
// eslint-disable-next-line @typescript-eslint/no-require-imports
import stateMdSchemaMod = require('./state-md-schema.cjs');
@@ -29,6 +29,47 @@ type StateFieldSchema = stateMdSchemaMod.StateFieldSchema;
const { extractFrontmatter, reconstructFrontmatter, stripFrontmatter, FRONTMATTER_UNPARSEABLE } = frontmatter;
export function formatProgressMachineSegment(percent: number): string {
// ADR-3180 Decision 7: rounding and the 100 ceiling belong to the
// completion-ratio kernel. The floor is added here because this helper is
// also fed persisted frontmatter values (hand-editable, unlike the
// count-shaped entries into that kernel), and `'░'.repeat` throws on a
// negative count. Bar and printed percent use the clamped value so the two
// halves of the segment can never disagree.
const clamped = Math.max(0, clampPercentFromFraction(percent / 100));
const filled = Math.round(clamped / 10);
return `[${'█'.repeat(filled)}${'░'.repeat(10 - filled)}] ${clamped}%`;
}
// Consumers (a future STATE.md writer that bypasses all three reintroduces the
// #4213 divergence class): `cmdStateUpdateProgress` and `syncCore`'s progress
// intent (both in this module) plus the post-sync body reconciliation in
// `applyPostSyncPreservation` (src/state.cts). `cmdStateSync` never reaches
// that reconciliation — ADR-3408 §8.3: `state sync` lets the body win, so
// preservation must NOT run — which is why its correctness comes from
// `syncCore`'s call here.
export function stateReplaceProgressPercent(content: string, percent: number): string | null {
const body = stripFrontmatter(content);
// #2177: bold `**Progress:**` anywhere in the body wins outright; the plain
// `^Progress:` form is the fallback only when no bold line exists, so an
// earlier free-text line starting with `Progress:` cannot capture the
// rewrite ahead of the real status line.
const boldProgressPattern = /(\*\*Progress:\*\*[ \t]*)([^\r\n]*)/i;
const plainProgressPattern = /^(Progress:[ \t]*)([^\r\n]*)/im;
const pattern = boldProgressPattern.test(body)
? boldProgressPattern
: plainProgressPattern.test(body)
? plainProgressPattern
: null;
if (!pattern) return null;
const machineSegment = /(?:\[[^\]\r\n]*\][ \t]*)?\d{1,3}%/;
const progress = formatProgressMachineSegment(percent);
const updatedBody = body.replace(pattern, (_match: string, prefix: string, value: string) => (
`${prefix}${machineSegment.test(value) ? value.replace(machineSegment, progress) : progress}`
));
return content.slice(0, content.length - body.length) + updatedBody;
}
/**
* ADR-3473 §8.1 (#3881, consequence 2 wiring): does `existingFm` carry the
* `FRONTMATTER_UNPARSEABLE` marker `extractFrontmatter` sets when a
@@ -2771,13 +2812,12 @@ function syncCore(
if (currentProgress) {
const currentPercent = parseInt(currentProgress.replace(/[^\d]/g, ''), 10);
if (currentPercent !== intent.percent) {
const barWidth = 10;
const filled = Math.round((intent.percent / 100) * barWidth);
const bar = '█'.repeat(filled) + '░'.repeat(barWidth - filled);
const progressStr = `[${bar}] ${intent.percent}%`;
changes.push(`Progress: ${currentProgress} -> ${progressStr}`);
const result = stateReplaceField(modified, 'Progress', progressStr);
if (result) { modified = result; updated.push('Progress'); }
const result = stateReplaceProgressPercent(modified, intent.percent);
if (result) {
const progressStr = formatProgressMachineSegment(intent.percent);
changes.push(`Progress: ${currentProgress} -> ${progressStr}`);
modified = result; updated.push('Progress');
}
}
}
}

View File

@@ -91,7 +91,7 @@ import { findProjectRoot } from './project-root.cjs';
// it introduces no cycle on this path.
// eslint-disable-next-line @typescript-eslint/no-require-imports
import milestoneLockMod = require('./milestone-lock.cjs');
const { transitionCore, applyStatePreservation, sliceCurrentPositionSection } = stateTransitionMod;
const { transitionCore, applyStatePreservation, sliceCurrentPositionSection, stateReplaceProgressPercent, formatProgressMachineSegment } = stateTransitionMod;
// #3699: the frontmatter-key <-> body-field routing behind `state update`'s
// failure explanation, and the classification table it falls back to.
const { getFieldClassification, getFrontmatterBodySource, frontmatterKeyForBodyField } = stateTransitionMod;
@@ -112,6 +112,7 @@ import {
shouldPreserveExistingProgress,
stateExtractField,
stateFieldValue,
toFiniteNumber,
// #3696: the `last_activity` invariant that `state validate` (S008/S009) now
// asserts. Both live in the field-semantics owner, not here, so `smart-entry`
// and `state validate` cannot drift apart about the same field.
@@ -1445,41 +1446,15 @@ function cmdStateUpdateProgress(cwd: string, raw: boolean): void {
return;
}
const { percent, completedPlans: fmCompletedPlans, totalPlans: fmTotalPlans } = preview;
const barWidth = 10;
const filled = Math.round(percent / 100 * barWidth);
const bar = '█'.repeat(filled) + '░'.repeat(barWidth - filled);
const progressStr = `[${bar}] ${percent}%`;
const progressStr = formatProgressMachineSegment(percent);
let updated = false;
readModifyWriteStateMd(statePath, (content) => {
// #2177: match against the BODY only. With /i the patterns below would
// otherwise hit the YAML frontmatter `progress:` key first (and `\s*` would
// eat its newline, mangling the nested block), while the body Progress: line
// — which frontmatter `percent` is re-derived from on every write — stays
// stale and silently reverts the update.
const body = stripFrontmatter(content);
const fmPrefix = content.slice(0, content.length - body.length);
// Swap only the machine segment ("[bar] NN%" or bare "NN%"), preserving any
// descriptive suffix an agent authored, e.g. "(2/4 plans done; blocked on…)".
const machineSegment = /(?:\[[^\]\r\n]*\][ \t]*)?\d{1,3}%/;
const replaceValue = (value: string) => machineSegment.test(value)
? value.replace(machineSegment, progressStr)
: progressStr;
// Try **Progress:** bold format first, then plain Progress: format.
const boldProgressPattern = /(\*\*Progress:\*\*[ \t]*)([^\r\n]*)/i;
const plainProgressPattern = /^(Progress:[ \t]*)([^\r\n]*)/im;
const pattern = boldProgressPattern.test(body)
? boldProgressPattern
: plainProgressPattern.test(body)
? plainProgressPattern
: null;
if (!pattern) return content;
const result = stateReplaceProgressPercent(content, percent);
if (result === null) return content;
updated = true;
return fmPrefix + body.replace(pattern, (_match, prefix: string, value: string) => `${prefix}${replaceValue(value)}`);
return result;
}, cwd);
if (updated) {
@@ -4090,6 +4065,8 @@ function applyPostSyncPreservation(
}
}
let finalContent = syncedContent;
if (preservation.mutated || authoritativeReasserted) {
// #3742: preservation RESTORES frontmatter keys the body-derived rebuild
// could not produce (e.g. `current_phase` on a layout with no body
@@ -4110,9 +4087,16 @@ function applyPostSyncPreservation(
}
const yamlStr = reconstructFrontmatter(preservation.postFm as unknown as Frontmatter);
const body = stripFrontmatter(syncedContent);
return `---\n${yamlStr}\n---\n\n${body}`;
finalContent = `---\n${yamlStr}\n---\n\n${body}`;
}
return syncedContent;
const persistedPercent = toFiniteNumber(
preservation.postFm['progress'] && (preservation.postFm['progress'] as Record<string, unknown>)['percent'],
);
if (persistedPercent !== null) {
const reconciled = stateReplaceProgressPercent(finalContent, persistedPercent);
if (reconciled !== null) finalContent = reconciled;
}
return finalContent;
}
/**

View File

@@ -0,0 +1,90 @@
'use strict';
/**
* Property-based tests for the #4213 progress-surface helpers
*
* Module: gsd-core/bin/lib/state-transition.cjs
* Exported: formatProgressMachineSegment(percent),
* stateReplaceProgressPercent(content, percent)
*
* Properties tested:
* (a) formatProgressMachineSegment: never throws on any finite input
* (the review's RangeError case — persisted frontmatter values are
* hand-editable and only finiteness-checked upstream)
* (b) formatProgressMachineSegment: always returns a well-formed
* `[bar] NN%` segment with an exactly-10-glyph bar, 0-100 percent
* (c) stateReplaceProgressPercent: idempotent — a second application with
* the same percent is a no-op
* (d) stateReplaceProgressPercent: round-trip — the written bar re-parses
* to the same (clamped) percent for any 0-100 input
*/
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fc = require('./helpers/fast-check-setup.cjs');
const {
formatProgressMachineSegment,
stateReplaceProgressPercent,
} = require('../gsd-core/bin/lib/state-transition.cjs');
const SEGMENT_RE = /^\[(█{0,10}░{0,10})\] (\d{1,3})%$/;
function clampToPercentRange(n) {
return Math.max(0, Math.min(100, Math.round(n)));
}
describe('formatProgressMachineSegment properties (#4213 review finding)', () => {
// (a) + (b) never throws, always well-formed, for ANY finite number.
test('property: never throws and always yields a well-formed segment for any finite percent', () => {
fc.assert(
fc.property(fc.double({ noDefaultInfinity: true, noNaN: true }), (percent) => {
const segment = formatProgressMachineSegment(percent);
const match = SEGMENT_RE.exec(segment);
assert.ok(match, `segment must match [bar] NN%, got ${segment}`);
assert.equal(match[1].length, 10, 'bar must be exactly 10 glyphs');
const printed = Number(match[2]);
assert.ok(printed >= 0 && printed <= 100, `printed percent must be clamped, got ${printed}`);
}),
);
});
// Bar fill agrees with the printed percent at the 0-100 boundaries reviewers read.
test('boundary literals: 0, 100, 105, -30', () => {
assert.equal(formatProgressMachineSegment(0), '[░░░░░░░░░░] 0%');
assert.equal(formatProgressMachineSegment(100), '[██████████] 100%');
assert.equal(formatProgressMachineSegment(105), '[██████████] 100%');
assert.equal(formatProgressMachineSegment(-30), '[░░░░░░░░░░] 0%');
});
});
describe('stateReplaceProgressPercent properties (#4213 review finding)', () => {
// Any percent the writer can be handed, applied to a representative body.
const anyPercent = fc.integer({ min: -1000, max: 1000 });
// (c) idempotency: applying twice with the same percent changes nothing more.
test('property: idempotent on a second application with the same percent', () => {
fc.assert(
fc.property(anyPercent, (percent) => {
const content = '# Project State\n\n**Progress:** [█████░░░░░] 50% (2/4 plans done)\n';
const once = stateReplaceProgressPercent(content, percent);
const twice = stateReplaceProgressPercent(once, percent);
assert.equal(twice, once);
}),
);
});
// (d) round-trip: the segment the helper writes re-parses to the same
// clamped percent it printed for any in-range input.
test('property: written segment re-parses to the clamped percent (0-100)', () => {
fc.assert(
fc.property(fc.integer({ min: 0, max: 100 }), (percent) => {
const content = 'Progress: [██░░░░░░░░] 20%\n';
const updated = stateReplaceProgressPercent(content, percent);
const match = /\[(█{0,10}░{0,10})\] (\d{1,3})%/.exec(updated);
assert.ok(match, `rewritten body must carry a machine segment, got ${updated}`);
assert.equal(Number(match[2]), clampToPercentRange(percent));
}),
);
});
});

View File

@@ -2857,6 +2857,106 @@ describe('cmdStateUpdateProgress (state update-progress)', () => {
});
});
describe('#4213: resyncing state verbs keep body Progress bar equal to frontmatter progress.percent', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createFixture();
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), '# Roadmap\n');
for (const num of ['01', '02', '03', '04']) {
const dir = path.join(tmpDir, '.planning', 'phases', num);
fs.mkdirSync(dir, { recursive: true });
fs.writeFileSync(path.join(dir, `${num}-PLAN.md`), '# Plan\n');
}
});
afterEach(() => cleanup(tmpDir));
function seedState(seededPercent = 50, withProgress = true) {
const progressLine = withProgress
? `Progress: [█████░░░░░] ${seededPercent}% (2/4 plans done)`
: '**Status:** Executing';
fs.writeFileSync(
path.join(tmpDir, '.planning', 'STATE.md'),
`---\ngsd_state_version: "1.0"\nstatus: executing\nprogress:\n total_phases: 4\n completed_phases: ${seededPercent / 25}\n total_plans: 4\n completed_plans: ${seededPercent / 25}\n percent: ${seededPercent}\n---\n\n# Project State\n\n${progressLine}\n`
);
}
function completePhasesOnDisk(count) {
for (const num of ['01', '02', '03', '04'].slice(0, count)) {
const dir = path.join(tmpDir, '.planning', 'phases', num);
fs.writeFileSync(path.join(dir, `${num}-PLAN-SUMMARY.md`), '# Summary\n');
writePassedVerification(tmpDir, num, num);
}
}
function assertProgress(expected, suffix = false) {
const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
assert.strictEqual(bodyProgressPercent(state), expected);
assert.strictEqual(Number(JSON.parse(runGsdTools('state json', tmpDir).output).progress.percent), expected);
if (suffix) assert.match(stateDocument.stateExtractField(state, 'Progress'), /\(2\/4 plans done\)/);
}
test('resyncing verbs repair drift-up and preserve the body suffix', () => {
for (const command of [['state', 'record-session', '--stopped-at', '2.3'], 'state sync']) {
seedState();
completePhasesOnDisk(3);
assert.ok(runGsdTools(command, tmpDir).success, `${command} failed`);
assertProgress(75, true);
}
});
test('a no-drift write keeps both surfaces at the existing percent', () => {
seedState(); completePhasesOnDisk(2);
assert.ok(runGsdTools(['state', 'record-session', '--stopped-at', '2.3'], tmpDir).success);
assertProgress(50);
});
test('resyncing verbs repair drift-down without inserting a missing bar', () => {
seedState();
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'),
'# Roadmap\n\n### Phase 01: A\n### Phase 02: B\n### Phase 03: C\n### Phase 04: D\n### Phase 05: E\n### Phase 06: F\n');
completePhasesOnDisk(2);
assert.ok(runGsdTools(['state', 'add-decision', '--phase', '3', '--summary', 's'], tmpDir).success);
assertProgress(33);
seedState(50, false);
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), '# Roadmap\n');
completePhasesOnDisk(3);
assert.ok(runGsdTools(['state', 'record-session', '--stopped-at', '2.3'], tmpDir).success);
const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
assert.strictEqual(bodyProgressPercent(state), null);
assert.strictEqual(Number(JSON.parse(runGsdTools('state json', tmpDir).output).progress.percent), 75);
});
test('a free-text plain Progress: line above the status line cannot capture the rewrite, and an out-of-range percent clamps', () => {
// The #2177 bold-first priority restated for the shared helper (an earlier
// free-text line starting with `Progress:` must stay byte-identical while
// the bold status line is rewritten — a leftmost-match alternation got
// this wrong), and the clamp case: a hand-edited body percent (105%) with
// an unmeasured scan (no plans on disk, so the curated block stands)
// reaches the helper through applyPostSyncPreservation and must render the
// clamped 100% bar instead of throwing RangeError on repeat(-1).
const freeText = 'Progress: tracked in the weekly thread, do not edit this line by hand';
seedState(50);
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'),
`---\ngsd_state_version: "1.0"\nstatus: executing\nprogress:\n total_phases: 4\n completed_phases: 2\n total_plans: 4\n completed_plans: 2\n percent: 50\n---\n\n# Project State\n\n${freeText}\n\n**Progress:** [█████░░░░░] 50%\n`);
completePhasesOnDisk(3);
assert.ok(runGsdTools(['state', 'record-session', '--stopped-at', '2.3'], tmpDir).success);
let state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
assert.ok(state.includes(freeText), 'the free-text plain line must stay byte-identical');
assert.match(stateDocument.stateExtractField(state, 'Progress'), /^\[████████░░\] 75%$/, 'the bold status line is the one rewritten (extractor returns its value)');
assert.strictEqual(bodyProgressPercent(state), 75);
assert.strictEqual(Number(JSON.parse(runGsdTools('state json', tmpDir).output).progress.percent), 75);
seedState(50);
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'),
'---\ngsd_state_version: "1.0"\nstatus: executing\nprogress:\n total_phases: 0\n completed_phases: 0\n total_plans: 0\n completed_plans: 0\n percent: 105\n---\n\n# Project State\n\nProgress: [██████████░] 105% (2/4 plans done)\n');
for (let n = 1; n <= 4; n++) {
// eslint-disable-next-line local/no-raw-rmsync-in-tests -- removing fixture phase dirs beforeEach created; helpers.cleanup owns the tmp root itself
fs.rmSync(path.join(tmpDir, '.planning', 'phases', String(n).padStart(2, '0')), { recursive: true, force: true });
}
assert.ok(runGsdTools(['state', 'add-decision', '--phase', '3', '--summary', 's'], tmpDir).success);
state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
assert.strictEqual(bodyProgressPercent(state), 100, 'bar renders the clamped 100%');
assert.match(stateDocument.stateExtractField(state, 'Progress'), /\(2\/4 plans done\)/, 'suffix survives');
});
});
// ─────────────────────────────────────────────────────────────────────────────
// cmdStateResolveBlocker, cmdStateRecordSession
// ─────────────────────────────────────────────────────────────────────────────