* fix(#4294): reserve a full progress bar for 100% and give the render half one owner Six call sites each carried `Math.round((percent / 100) * width)` inline, and every copy rounded to a full bar before the percent reached 100: from 95 up at width 10, from 98 up at width 20. A project at 19/20 plans drew the same bar as a shipped one beside a number that said otherwise, and an out-of-range percent threw `RangeError` from the unguarded `'░'.repeat`. ADR-3180 Decision 7 gave the completion-RATIO derivation one owner (`clampPercentFromFraction`). This gives the RENDER half the same: `progressBarFilledCells` / `renderProgressBar` in phase-lifecycle.cts, with the `progress` table and bar renderers, the stats renderer, the gsd2 import writer, and #4231's `formatProgressMachineSegment` (which now serves both STATE.md writers) all drawing through it. Contract: below 100 the fill is held one cell short of the width, so only the saturating percents move (95-99 at width 10, 98-99 at width 20) and every other value in 0-100 renders as before — pinned by an exhaustive comparison against the legacy formula at both widths. Null / non-finite renders an empty bar; out-of-range is clamped, never thrown. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012hbFn24VWaxJBw8DUWjAmU * chore(#4294): set changeset fragment pr to 4473 * docs(#4294): correct the pre-fix inline call-site count to five The kernel's doc comment said SIX call sites carried their own `Math.round((percent / 100) * width)`. The base tree has five: three in `commands.cts` plus one each in `gsd2-import.cts` and `formatProgressMachineSegment`, the latter two using `/ 10` with the width already substituted (`pct` and `clamped` respectively). The six is #4294's count of consumers -- it counts `cmdStateUpdateProgress` and `syncCore` separately, but #4231 had already routed both through `formatProgressMachineSegment` (as it does `applyPostSyncPreservation`), so by this branch's base they share one copy. The comment now states the tree's count and records where the six comes from, so neither number reads as an error later. Comment-only; no behaviour change, and no change to compiled output. --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
@@ -56,6 +56,14 @@ describe('formatProgressMachineSegment properties (#4213 review finding)', () =>
|
||||
assert.equal(formatProgressMachineSegment(105), '[██████████] 100%');
|
||||
assert.equal(formatProgressMachineSegment(-30), '[░░░░░░░░░░] 0%');
|
||||
});
|
||||
|
||||
// #4294: the bar is drawn by the render kernel, so a sub-100 percent can no
|
||||
// longer round up to a full bar beside a number that says otherwise.
|
||||
test('#4294 boundary literals: 95 and 99 hold one cell short of full; 94 is unchanged', () => {
|
||||
assert.equal(formatProgressMachineSegment(94), '[█████████░] 94%');
|
||||
assert.equal(formatProgressMachineSegment(95), '[█████████░] 95%');
|
||||
assert.equal(formatProgressMachineSegment(99), '[█████████░] 99%');
|
||||
});
|
||||
});
|
||||
|
||||
describe('stateReplaceProgressPercent properties (#4213 review finding)', () => {
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* Behavioral tests for phase-lifecycle.cjs
|
||||
*
|
||||
* Module: gsd-core/bin/lib/phase-lifecycle.cjs
|
||||
* Exports: deriveProgressFromRoadmap, clampPercent
|
||||
* Exports: deriveProgressFromRoadmap, clampPercent, progressBarFilledCells, renderProgressBar
|
||||
*
|
||||
* ADR-2143 (epic #2143) migrated deriveProgressFromRoadmap from position-based
|
||||
* regexes to the markdown-table schema registry (collectSection + parseMarkdownTable
|
||||
@@ -21,7 +21,13 @@
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
|
||||
const { deriveProgressFromRoadmap, clampPercent } = require('../gsd-core/bin/lib/phase-lifecycle.cjs');
|
||||
const {
|
||||
deriveProgressFromRoadmap,
|
||||
clampPercent,
|
||||
progressBarFilledCells,
|
||||
renderProgressBar,
|
||||
} = require('../gsd-core/bin/lib/phase-lifecycle.cjs');
|
||||
const fc = require('./helpers/fast-check-setup.cjs');
|
||||
|
||||
describe('deriveProgressFromRoadmap', () => {
|
||||
test('parses the 4-column flat Progress table (behaviour preserved)', () => {
|
||||
@@ -182,3 +188,91 @@ describe('clampPercent', () => {
|
||||
assert.equal(clampPercent(3, -1), 0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('renderProgressBar / progressBarFilledCells (#4294 — the render half has one owner)', () => {
|
||||
// The formula every call site used to carry inline. Kept here as the
|
||||
// reference curve the kernel is pinned against, NOT as an implementation.
|
||||
const legacyFilled = (percent, width) => Math.round((percent / 100) * width);
|
||||
const WIDTHS_IN_USE = [10, 20];
|
||||
|
||||
const count = (bar, glyph) => bar.split('').filter((ch) => ch === glyph).length;
|
||||
|
||||
test('a full bar is reserved for 100: the last sub-100 percents are held one cell short', () => {
|
||||
// width 10 — 94 already rounded to 9; 95-99 used to round to 10.
|
||||
assert.equal(renderProgressBar(94, 10), '█████████░');
|
||||
assert.equal(renderProgressBar(95, 10), '█████████░');
|
||||
assert.equal(renderProgressBar(99, 10), '█████████░');
|
||||
assert.equal(renderProgressBar(100, 10), '██████████');
|
||||
// width 20 — 97 already rounded to 19; 98-99 used to round to 20.
|
||||
assert.equal(renderProgressBar(97, 20), '███████████████████░');
|
||||
assert.equal(renderProgressBar(98, 20), '███████████████████░');
|
||||
assert.equal(renderProgressBar(99, 20), '███████████████████░');
|
||||
assert.equal(renderProgressBar(100, 20), '████████████████████');
|
||||
// The bottom of the range is untouched.
|
||||
assert.equal(renderProgressBar(0, 10), '░░░░░░░░░░');
|
||||
assert.equal(renderProgressBar(5, 10), '█░░░░░░░░░');
|
||||
assert.equal(renderProgressBar(50, 10), '█████░░░░░');
|
||||
});
|
||||
|
||||
test('exhaustive 0-100 curve at both widths: identical to the legacy formula except exactly the saturating percents', () => {
|
||||
const expectedMoved = { 10: [95, 96, 97, 98, 99], 20: [98, 99] };
|
||||
for (const width of WIDTHS_IN_USE) {
|
||||
const moved = [];
|
||||
for (let percent = 0; percent <= 100; percent++) {
|
||||
const legacy = legacyFilled(percent, width);
|
||||
const actual = progressBarFilledCells(percent, width);
|
||||
if (actual !== legacy) {
|
||||
moved.push(percent);
|
||||
// The only permitted departure: legacy saturated below 100, kernel holds one short.
|
||||
assert.equal(legacy, width, `width ${width}, ${percent}%: moved but legacy was not saturated`);
|
||||
assert.equal(actual, width - 1, `width ${width}, ${percent}%: moved to ${actual}, expected ${width - 1}`);
|
||||
}
|
||||
// The rendered glyph run agrees with the count and is always width glyphs long.
|
||||
const bar = renderProgressBar(percent, width);
|
||||
assert.equal(bar.length, width);
|
||||
assert.equal(count(bar, '█'), actual);
|
||||
assert.equal(count(bar, '░'), width - actual);
|
||||
}
|
||||
assert.deepEqual(moved, expectedMoved[width], `width ${width}: the set of moved percents`);
|
||||
}
|
||||
});
|
||||
|
||||
test('withheld / non-finite percent renders an empty bar (the `percent === null ? 0` guard, centralized)', () => {
|
||||
for (const value of [null, undefined, NaN, Infinity, -Infinity, '50']) {
|
||||
assert.equal(renderProgressBar(value, 10), '░░░░░░░░░░', `value ${String(value)}`);
|
||||
assert.equal(progressBarFilledCells(value, 10), 0, `value ${String(value)}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('out-of-range percent is clamped, never thrown: 120% is a full bar, -30% an empty one', () => {
|
||||
// The old inline form threw `RangeError: Invalid count value: -2` at 120%.
|
||||
assert.equal(renderProgressBar(120, 10), '██████████');
|
||||
assert.equal(renderProgressBar(-30, 10), '░░░░░░░░░░');
|
||||
assert.equal(renderProgressBar(100.4, 10), '██████████');
|
||||
assert.equal(renderProgressBar(99.9, 10), '█████████░');
|
||||
});
|
||||
|
||||
test('property: for any finite percent and width in use, exactly width glyphs, and full only at >= 100', () => {
|
||||
fc.assert(
|
||||
fc.property(
|
||||
fc.double({ noDefaultInfinity: true, noNaN: true }),
|
||||
fc.constantFrom(...WIDTHS_IN_USE),
|
||||
(percent, width) => {
|
||||
const bar = renderProgressBar(percent, width);
|
||||
assert.equal(bar.length, width);
|
||||
assert.match(bar, /^█*░*$/);
|
||||
const full = count(bar, '█') === width;
|
||||
assert.equal(full, percent >= 100, `${percent}% at width ${width}: full=${full}`);
|
||||
},
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('degenerate widths: non-positive yields nothing, width 1 fills only at 100', () => {
|
||||
assert.equal(renderProgressBar(50, 0), '');
|
||||
assert.equal(renderProgressBar(50, -3), '');
|
||||
assert.equal(progressBarFilledCells(50, 0), 0);
|
||||
assert.equal(renderProgressBar(99, 1), '░');
|
||||
assert.equal(renderProgressBar(100, 1), '█');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user