Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-dashboard): let @object-ui/core own percent scaling in the record-field branch - #5639
Merged
Merged
Conversation
…ld branch `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.
…st 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.
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
marked this pull request as ready for review
August 21, 2026 21:45
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#5607
renderFieldValue's%-format branch normalised the value itself before callingformatPercent.formatPercentalready appliespercentDisplayValue, which@object-ui/coredocuments as the SINGLE source of truth for percent displayscaling — so the call site was re-deciding what core owns, with a copy that had
drifted from it.
The branch is deleted rather than repaired, and the measurement below is why.
Verified line numbers (re-derived by code on
origin/main)The card cites
packages/plugin-dashboard/src/recordFields.tsx:189-192. Derivedon
cdda37ae3withgit show origin/main:… | grep -n:No drift. The two defect lines are 190-191; the branch opens at 188. The file
is byte-identical between
f52d36c96(when the card was filed) and currentorigin/main—git diff --stat f52d36c96 origin/main -- <file>is empty.What core already does (the measurement that chose the shape)
Triage asked for this to be measured before choosing between repairing the local
normalisation and deleting it. Executed against the built
dist:percentDisplayValueisvalue > -1 && value < 1 ? value * 100 : value—both arms, and a symmetric test. Core needs no help from the caller, so the
condition triage named for "the branch should go rather than be fixed" is met by
measurement. The raw stored value now goes to
formatPercent: the identical callthe list-view percent cell already makes for an ordinary percent column.
Note this also rules out the shape the issue body floated
(
formatPercent(value > 1 ? value : value * 100, …)): it keeps the asymmetric> 1test and double-scales small fractions, i.e. it preserves two of the threedivergences below.
Intloption spellings, re-read on the current tipTriage's second note. The body's measurement was taken through
style: 'unit'/unit: 'percent'/unitDisplay: 'narrow'quoted fromformatPercent. Those spellings have moved. On the current tipformatPercentrenders throughformatDisplayNumber(displayValue, { locale, style: 'percentPoints', … })— the rawIntltriple is now an internal detailof the
'percentPoints'style token. This PR therefore pins noIntloptions: every assertion goes through
renderFieldValue, so nothing here canbecome a second drift when that token's internals move again.
The three divergences, all one defect
1.6051.60%1.61%0.00550.00%0.50%-5-500.00%-5.00%1100.00%1.00%(value / 100) * 100is not value-preserving in binary floating point. Gridv = i/1000,iin1001..200000, re-measured through the current callshape: 19,978 of 199,000 values change bit pattern and 1,108 rendered
strings move — reproducing the card's numbers and its five first movers
exactly.
0.01was scaled twice — the local* 100put itback under 1, so core's fraction arm scaled it again.
value > 1, not core's symmetric|value| < 1, so anegative already in percentage points took the fraction arm.
Evidence — the repair is what turns the test green
Ablation by mutating the fix away on disk. Each leg proves the mutation on disk
with anchored whole-line counts in both directions before measuring, and a
trap … EXIT INT TERMrestores the file. No rebuild is needed for the mutationto take effect: the test reaches the subject by a relative source import
(
../recordFields), not through the package'sexports→dist.Leg A — restore the round-trip form.
8 failed | 5 passed (13), and all fivenamed movers are red with exactly the card's values:
The 5 that stay green are the 4 controls plus the precision pin — so the red is
targeted, not a branch that stopped rendering percents.
Leg B — the controls' own mutation leg. A control that cannot fail is a
tautology, so this measures rather than argues it: same branch, same call, same
locale, only
decimals→decimals + 1. Result13 failed (13)— all fourcontrols red, e.g.
expected '1.000%' to be '1.00%'.An earlier revision of the ablation used
grep -E, where(is a groupingmetacharacter, so the literal parens matched nothing and every anchor read 0. The
pristine assertion aborted with
ablation NOT RUNrather than reporting ano-op as a result; anchors are now
grep -cxF. Recorded because the guardfiring is the reason the reading is trustworthy.
Runs
All at
348c1330f, from the repo root (package-cwdvitestis refused by therepo's own guard, objectui#3378).
Test Files 1 passed (1)·Tests 13 passed (13)plugin-dashboardsuiteTest Files 74 passed (74)·Tests 670 passed (670)type-checkVERDICT command-exit 0(tsc --noEmit && tsc -p tsconfig.test.jsonechoed)lint✖ 365 problems (0 errors, 365 warnings)— exit 0check-control-bytes✅ OK (scanned 4684 tracked text file(s); skipped 85 binary)The heavier runs were measured at
4e98151ea; the only commit since touches.changeset/*.md, so the code tree is byte-identical and they apply unchanged.No existing fixture pinned the old behaviour — notably
ObjectDataTable.percentLocale.test.tsx, which drives this same branch with1234.5('0%') and33.33('0.00%'), is green unchanged.Lint narrowing — declared, and measured
The repo-wide
eslint .is CI's run. This narrowing is a measurement, not a skip:isPathIgnored: 3,506lintable tracked files, of which 104 under
packages/plugin-dashboard/.--format json: 104 — the wholechanged package, 0 errors.
projectService/parserOptions/project:ineslint.config.js), so no untouched file's verdict can dependon this diff. The 3,402 excluded files are provably unmovable by it.
src/recordFields.tsxcarries 14no-explicit-anywarnings, all at lines 47-126— upstream of the percent branch and untouched here.
Changeset
check-changeset-presence.mjsis the authority:Dist deltas, measured at the real
dist/path withdist/(which containstsconfig.tsbuildinfo) removed on every leg, compared by hash:dist/index.js77df7ba4d50ceb43(131,405 B)a2db53c8a5fae48a(131,436 B)*.d.ts061342ba4c1f1e5b061342ba4c1f1e5bThe behaviour-card signature exactly: JS moves,
.d.tsdoes not — which iswhy a zero
.d.tsdelta would have been the wrong reason to skip a changeset.dist/was rebuilt from the restored source afterwards and re-hashed to77df7ba4d50ceb43, with the mutation marker absent, so no mutated build was leftbehind. (
plugin-dashboardemits noindex.cjs; ESM only.)Generated by Claude Code