From 4e98151ea87a9fc0ef5d9359d8472789d001fd04 Mon Sep 17 00:00:00 2001 From: ObjectStack Agent Date: Fri, 21 Aug 2026 21:05:37 +0000 Subject: [PATCH 1/2] fix(plugin-dashboard): let core own percent scaling in the record-field branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `renderFieldValue`'s `%`-format branch normalised the value itself before calling `formatPercent`: const normalized = value > 1 ? value / 100 : value; return formatPercent(normalized * 100, decimals, displayLocale); `formatPercent` already applies `percentDisplayValue`, which `@object-ui/core` documents as the single source of truth for percent display scaling, so this was a second, drifted copy of one decision. Measured on the current tip, core already handles both arms — `percentDisplayValue(0.75) === 75` and `percentDisplayValue(1.605) === 1.605` — so the branch is deleted rather than patched, and the raw stored value now goes to `formatPercent`: the identical call the list-view percent cell already makes for an ordinary percent column. Three measured divergences, all one defect: - `(value / 100) * 100` is not value-preserving in binary floating point, re-introducing one call frame upstream the round trip that was removed from inside `formatPercent`. On the 0.001-step grid to 200, 19,978 of 199,000 values change bit pattern and 1,108 rendered strings move, every one a last-digit off-by-one: a stored 1.605 rendered 1.60% where half-up is 1.61%. - A stored fraction below 0.01 was scaled twice, because the local `* 100` put it back under 1 and core's fraction arm scaled it again: 0.005 rendered 50.00%. - The local test was `value > 1` rather than core's symmetric `|value| < 1`, so a negative already in percentage points took the fraction arm: -5 rendered -500.00%. Reverse-verified: the round-trip form restored on disk turns 8 of the 13 cases red, including `expected '1.60%' to be '1.61%'`. The 4 control cases and the precision pin stay green on both legs. --- .changeset/olive-pugs-sing.md | 33 ++++ .../recordFields.percentScaling-5607.test.ts | 144 ++++++++++++++++++ .../plugin-dashboard/src/recordFields.tsx | 32 +++- 3 files changed, 207 insertions(+), 2 deletions(-) create mode 100644 .changeset/olive-pugs-sing.md create mode 100644 packages/plugin-dashboard/src/__tests__/recordFields.percentScaling-5607.test.ts diff --git a/.changeset/olive-pugs-sing.md b/.changeset/olive-pugs-sing.md new file mode 100644 index 0000000000..3530361041 --- /dev/null +++ b/.changeset/olive-pugs-sing.md @@ -0,0 +1,33 @@ +--- +'@object-ui/plugin-dashboard': patch +--- + +Dashboard record fields: percent columns now render through the one percent +scaling decision instead of a second, drifted copy of it. + +`renderFieldValue`'s `%`-format branch normalised the value itself before +calling `formatPercent` (`const normalized = value > 1 ? value / 100 : value`, +then `normalized * 100`). `formatPercent` already applies `percentDisplayValue`, +which `@object-ui/core` documents as the single source of truth for percent +display scaling, so the branch was re-deciding what core owns — and its copy had +drifted from it in three measured ways: + +- `(value / 100) * 100` is not value-preserving in binary floating point, + re-introducing one call frame upstream the round trip that was removed from + inside `formatPercent`. On the 0.001-step grid to 200, 19,978 of 199,000 + values change bit pattern and 1,108 rendered strings move, every one a + last-digit off-by-one: a stored `1.605` rendered `1.60%` where half-up is + `1.61%`. +- A stored fraction below `0.01` was scaled twice — the local `* 100` put it + back under 1, so core's fraction arm scaled it again. `0.005` (0.5%) rendered + `50.00%`. +- The local test was `value > 1` rather than core's symmetric `|value| < 1`, so + a negative already in percentage points took the fraction arm: `-5` rendered + `-500.00%`. + +The branch now hands the raw stored value to `formatPercent` — the identical +call the list-view percent cell already makes for an ordinary percent column — +so a percent reads the same as a record field, as a grid cell and as a dashboard +measure. Output moves where it was wrong: values at or above 1 whose round trip +lost a digit, fractions below `0.01`, negatives at or below `-1`, and exactly +`1`, which is percentage points by core's convention and now renders `1%`. diff --git a/packages/plugin-dashboard/src/__tests__/recordFields.percentScaling-5607.test.ts b/packages/plugin-dashboard/src/__tests__/recordFields.percentScaling-5607.test.ts new file mode 100644 index 0000000000..aa6958ea68 --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/recordFields.percentScaling-5607.test.ts @@ -0,0 +1,144 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#5607 — the record-field percent branch stops re-deciding what + * `@object-ui/core` owns. + * + * `renderFieldValue`'s `%`-format branch used to normalise the value itself + * before calling `formatPercent`: + * + * const normalized = value > 1 ? value / 100 : value; + * return formatPercent(normalized * 100, decimals, displayLocale); + * + * `formatPercent` already applies `percentDisplayValue`, which its own doc + * comment names as the SINGLE source of truth for percent display scaling. So + * this was a second, drifted copy of one decision, and it is now deleted: the + * raw stored value goes to `formatPercent` — the identical call the list-view + * percent cell already makes for an ordinary percent column + * (`PercentCellRenderer`, `@object-ui/fields`). + * + * ── Measured on the current tip BEFORE the repair ──────────────────────── + * Core already handles BOTH arms, which is why the local branch goes rather + * than gets patched: `percentDisplayValue(0.75) === 75` (fraction) and + * `percentDisplayValue(1.605) === 1.605` (already points, passed through), so + * `formatPercent(1.605, 2, 'en-US')` is `1.61%` with no help from the caller. + * + * ── Directions, predicted in writing BEFORE the run ────────────────────── + * Runner machine locale is irrelevant: every case threads an explicit + * `'en-US'`, so these are locale-pinned, not locale-dependent. + * the five movers RED on the round-trip form, green repaired + * double-scaled fraction RED on the round-trip form (`50.00%` for 0.005) + * negative already-points RED on the round-trip form (`-500.00%` for -5) + * the exactly-1 boundary RED on the round-trip form (`100.00%` for 1) + * CONTROL GREEN on BOTH forms — see its own note + */ + +import { describe, it, expect } from 'vitest'; +import { renderFieldValue, type FieldMeta } from '../recordFields'; + +/** + * One percent-formatted record field, rendered exactly as the dashboard table + * and the record-detail drawer render it. `'0.00%'` is what drives the branch's + * own `decimals` extraction to 2 — the precision every value below is quoted at. + */ +const pct = (value: number, format = '0.00%'): string => + renderFieldValue( + value, + { name: 'rate', label: 'Rate', type: 'number', format } satisfies FieldMeta, + undefined, + 'en-US', + ) as string; + +describe('renderFieldValue percent branch — core owns the scaling (objectui#5607)', () => { + /** + * The five movers named in the card, each a last-digit off-by-one that the + * `(value / 100) * 100` round trip produced. These are the falsification + * criterion: on the unrepaired branch every one of them renders the value in + * the second column and this case is RED. + * + * 1.605 -> repaired 1.61% round trip 1.60% + * 1.655 -> repaired 1.66% round trip 1.65% + * 1.705 -> repaired 1.71% round trip 1.70% + * 1.785 -> repaired 1.79% round trip 1.78% + * 1.835 -> repaired 1.84% round trip 1.83% + */ + it.each([ + [1.605, '1.61%'], + [1.655, '1.66%'], + [1.705, '1.71%'], + [1.785, '1.79%'], + [1.835, '1.84%'], + ])('renders %p half-up at 2 decimals as %p, not the round trip\'s last-digit-down', (value, expected) => { + expect(pct(value as number)).toBe(expected); + }); + + /** + * CONTROL — same function, same format, same locale, same precision as the + * cases above; only the VALUES differ, chosen because the round trip and the + * repair were measured to agree on them. + * + * It is green on the unrepaired code as well as the repaired code, and that + * is its whole job: it shows the movers above are a targeted red and not a + * branch that stopped rendering percents at all. It can still fail — a wrong + * precision, a dropped locale, a lost `%` affix or a broken branch takes it + * down with everything else, which is what makes it a control rather than a + * tautology. + */ + it.each([ + [0.75, '75.00%'], + [2.5, '2.50%'], + [12.25, '12.25%'], + [57, '57.00%'], + ])('CONTROL: %p renders %p under both the round trip and the repair', (value, expected) => { + expect(pct(value as number)).toBe(expected); + }); + + /** + * The same defect's other two faces, both fixed by the same deletion. + * + * A stored fraction below 0.01 was scaled TWICE — the local `* 100` put it + * back under 1, so `percentDisplayValue`'s fraction arm scaled it again. That + * is a factor of 100, not a last digit. + */ + it('does not double-scale a stored fraction below 0.01', () => { + expect(pct(0.005)).toBe('0.50%'); + expect(pct(0.0075)).toBe('0.75%'); + }); + + /** + * The local test was `value > 1`; core's is the symmetric `|value| < 1`. A + * negative already in percentage points therefore took the fraction arm. + */ + it('treats a negative already in percentage points as points, like core does', () => { + expect(pct(-5)).toBe('-5.00%'); + expect(pct(-0.5)).toBe('-50.00%'); + }); + + /** + * The boundary itself. `percentDisplayValue` is `value > -1 && value < 1`, so + * exactly 1 is percentage points and renders `1%` — the convention + * `PercentScale` states in those words (`whole` is `1 => "1%"`). The local + * branch's `value > 1` put exactly 1 on the fraction side and rendered + * `100.00%`, which is the drift this card removes. + */ + it('puts exactly 1 on core\'s side of the boundary', () => { + expect(pct(1)).toBe('1.00%'); + expect(pct(0.999)).toBe('99.90%'); + }); + + /** + * The branch's own `decimals` extraction is untouched by the repair, and this + * pins that: `'0.0%'` is 1 decimal, a bare `'%'` matches no `0.(0+)%` group + * and is 0. + */ + it('still reads the precision out of the format string', () => { + expect(pct(1.605, '0.0%')).toBe('1.6%'); + expect(pct(1.605, '%')).toBe('2%'); + }); +}); diff --git a/packages/plugin-dashboard/src/recordFields.tsx b/packages/plugin-dashboard/src/recordFields.tsx index 0a959f5467..37cfe2ede1 100644 --- a/packages/plugin-dashboard/src/recordFields.tsx +++ b/packages/plugin-dashboard/src/recordFields.tsx @@ -187,8 +187,36 @@ export function renderFieldValue( } if (typeof fmt === 'string' && /%/.test(fmt) && typeof value === 'number') { const decimals = (fmt.match(/0\.(0+)%/) || [undefined, ''] as any)[1].length; - const normalized = value > 1 ? value / 100 : value; - return formatPercent(normalized * 100, decimals, displayLocale); + // The RAW stored value goes to `formatPercent`, which applies + // `percentDisplayValue` — the single source of truth for percent display + // scaling (`@object-ui/core`), whose doc comment says so in those words. + // This is the same call the list-view percent cell makes for an ordinary + // percent column (`PercentCellRenderer` in `@object-ui/fields`), so a + // percent now reads identically as a record field, as a grid cell and as a + // dashboard measure. + // + // ⚠️ This call site used to make the fraction/points decision AGAIN, with a + // local copy that had drifted from the one it duplicated (objectui#5607): + // + // const normalized = value > 1 ? value / 100 : value; + // return formatPercent(normalized * 100, decimals, displayLocale); + // + // Three measured divergences, all of them the one defect — a caller + // re-deciding what core owns: + // - `(value / 100) * 100` is NOT value-preserving in binary floating + // point. It re-introduced, one call frame upstream, exactly the round + // trip objectui#4590 removed from inside `formatPercent`: 19,978 of + // 199,000 values on the 0.001-step grid to 200 change bit pattern and + // 1,108 rendered strings move, every one a last-digit off-by-one — + // a stored `1.605` rendered `1.60%` where half-up is `1.61%`. + // - A stored fraction below 0.01 was scaled TWICE: the local `* 100` put + // it below 1, so core's fraction arm scaled it again — `0.005` (0.5%) + // rendered `50.00%`, a factor of 100. + // - The local test was `value > 1`, not core's symmetric `|value| < 1`, + // so a negative already in points was treated as a fraction: `-5` + // rendered `-500.00%`. + // Deleting the branch fixes all three, because they were never three bugs. + return formatPercent(value, decimals, displayLocale); } if (typeof fmt === 'string' && /[YMDHms]/.test(fmt)) { return formatDate(value, fmt); From 348c1330fe41bdabfbf5cf5462da8d226837c3a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 21:32:33 +0000 Subject: [PATCH 2/2] docs(changeset): quote the exactly-1 boundary at the precision the rest of the note uses The note quoted every other value at two decimals but rendered the boundary case as `1%`, which is the 0-decimal form. At the `0.00%` precision the rest of the note uses it is `1.00%`, against `100.00%` from the local `value > 1` test. Same fact, stated at one precision. --- .changeset/olive-pugs-sing.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.changeset/olive-pugs-sing.md b/.changeset/olive-pugs-sing.md index 3530361041..0ebe368860 100644 --- a/.changeset/olive-pugs-sing.md +++ b/.changeset/olive-pugs-sing.md @@ -30,4 +30,6 @@ call the list-view percent cell already makes for an ordinary percent column — so a percent reads the same as a record field, as a grid cell and as a dashboard measure. Output moves where it was wrong: values at or above 1 whose round trip lost a digit, fractions below `0.01`, negatives at or below `-1`, and exactly -`1`, which is percentage points by core's convention and now renders `1%`. +`1`, which is one percentage point by core's convention and now renders +`1.00%` at two decimals, where the local `value > 1` test had made it +`100.00%`.