fix(#3945): require a non-empty denominator for milestone-complete (#4164)

* test(#3945): 0-of-0 milestone counters must not render complete

Regression coverage for the string-truthiness vacuous equality in both
statusline renderers: '0' counters, boundary 1/1 vs 0/1 vs 1/0, compact
parity.

* fix(#3945): require a non-empty denominator for milestone-complete

Both statusline renderers compared the regex-captured string counters
directly, so '0' && '0' && '0' === '0' fired Scene 3 on the empty set.
Numeric coercion plus total > 0 restores the guard's intent; all
non-zero shapes and percent=100 render byte-identically.

* chore(#3945): add changeset fragment

* chore(#3945): backfill changeset pr number

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-09-01 20:43:39 -04:00
committed by GitHub
parent c9bff8d2b9
commit 4c6643a8d7
3 changed files with 52 additions and 3 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 4164
---
**Statusline no longer shows milestone complete at 0 of 0 phases** — the 0-of-0 counters a freshly-roadmapped milestone carries no longer read as every-phase-done (string truthiness made the equality vacuous); both the full and compact renderers now require a non-zero denominator. (#3945)

View File

@@ -389,8 +389,12 @@ function formatGsdState(s) {
// Scene 2: idle + a recommended next command is visible to the user.
// Surfaces "what to run next" without the user opening STATE.md.
parts.push(`next ${s.nextAction} ${phasesStr}`);
} else if (Number(s.percent) === 100 || (s.completedPhases && s.totalPhases && s.completedPhases === s.totalPhases)) {
// Scene 3: milestone complete (every phase done).
} else if (Number(s.percent) === 100 || (Number(s.totalPhases) > 0 && Number(s.completedPhases) === Number(s.totalPhases))) {
// Scene 3: milestone complete (every phase done). #3945: the counters are
// regex-captured STRINGS, so the old `cp && tp && cp === tp` guard fired on
// the empty set ('0' is truthy, '0' === '0') — "0% · milestone complete".
// Numeric coercion + a non-empty denominator makes "nothing to measure"
// stop meaning "everything is done".
parts.push('milestone complete');
} else {
// Backward-compatible default — preserved EXACTLY for STATE.md files that
@@ -500,7 +504,7 @@ function formatGsdStateCompact(s) {
// still completes) wins over milestone-complete (Scene 3), even if a
// non-atomic STATE.md edit leaves percent=100 alongside a lifecycle phase.
const done = !s.activePhase && (Number(s.percent) === 100 ||
(s.completedPhases && s.totalPhases && s.completedPhases === s.totalPhases));
(Number(s.totalPhases) > 0 && Number(s.completedPhases) === Number(s.totalPhases)));
if (done) {
parts.push('complete');

View File

@@ -1217,6 +1217,32 @@ describe('formatGsdState #2833 lifecycle scenes', () => {
});
assert.equal(out, 'v2.0 · milestone complete');
});
// #3945: the counters arrive as regex-captured STRINGS, so '0' is truthy and
// '0' === '0' made the "every phase done" guard fire on the empty set —
// rendering "0% · milestone complete" for a freshly-roadmapped milestone.
test('Scene 3 — 0 of 0 counters does not render milestone complete (#3945)', () => {
const out = formatGsdState({
milestone: 'v1.15',
milestoneName: 'Design Refresh',
status: 'planning',
completedPhases: '0',
totalPhases: '0',
percent: '0',
});
assert.ok(!out.includes('milestone complete'),
`0 of 0 phases must not read as complete; got: ${out}`);
assert.ok(out.includes('planning'), `default path should render the status; got: ${out}`);
});
test('Scene 3 — boundary denominators: 1/1 completes, 0/1 and 1/0 do not (#3945)', () => {
assert.ok(formatGsdState({ milestone: 'v2.0', completedPhases: '1', totalPhases: '1' })
.includes('milestone complete'), '1 of 1 done is complete');
assert.ok(!formatGsdState({ milestone: 'v2.0', completedPhases: '0', totalPhases: '1' })
.includes('milestone complete'), '0 of 1 is not complete');
assert.ok(!formatGsdState({ milestone: 'v2.0', completedPhases: '1', totalPhases: '0' })
.includes('milestone complete'), '1 of 0 is not a complete milestone');
});
});
// ─── Backward compatibility — CRITICAL: existing STATE.md unchanged ─────────
@@ -1802,6 +1828,8 @@ test('config-set statusline.show_context_tokens yes → rejected', () => {
{ milestone: 'v2.0', completedPhases: '5', totalPhases: '5' },
{ milestone: 'v2.0', activePhase: '4.5', percent: '100', status: 'executing' },
{ milestone: 'v1.9', percent: '40', status: 'executing', phaseNum: '2', phaseTotal: '5' },
// #3945: the vacuous 0-of-0 shape must agree on NOT-complete too.
{ milestone: 'v1.15', status: 'planning', completedPhases: '0', totalPhases: '0', percent: '0' },
];
for (const s of cases) {
const fullDone = formatGsdState(s).includes('milestone complete');
@@ -1810,6 +1838,18 @@ test('config-set statusline.show_context_tokens yes → rejected', () => {
`completion parity diverged for ${JSON.stringify(s)}`);
}
});
test('compact — 0 of 0 counters does not render complete (#3945)', () => {
// Same vacuous-equality defect as the full renderer's Scene 3; the
// compact completion branch must also require a non-empty denominator.
const out = formatGsdStateCompact({
milestone: 'v1.15', status: 'planning',
completedPhases: '0', totalPhases: '0', percent: '0',
});
assert.ok(!/(^|\s)complete(\s|$)/.test(out),
`0 of 0 phases must not read as complete in compact; got: ${out}`);
assert.ok(out.includes('planning'), `compact should render the status; got: ${out}`);
});
test('idle with queued next action renders "next <action> <phases>"', () => {
const out = formatGsdStateCompact({
milestone: 'v2.0', nextAction: 'execute-phase', nextPhases: ['4.5', '4.6'],