fix(fields): formatPercent renders points directly — ties round half-up and extremes keep every digit (#4590) - #4595
Merged
Conversation
…up and extremes keep every digit (#4590) `formatPercentBody` rendered a value already in percentage POINTS through `Intl`'s `style: 'percent'`, which expects a FRACTION, so it divided by 100 for `Intl` to multiply straight back. That round trip is not value-preserving: `Intl` formats from the shortest decimal representation of the double it is handed, and the quotient's is not the authored one — `1.005` is `1.005`, but `1.005 / 100` is `0.010049999999999999`, which percent-scales to `1.0049999999999999` and rounds DOWN. A stored 1.005 at 2 decimals rendered `1.00%` where half-up is `1.01%`. The body now renders through `style: 'percentPoints'` (the option #4576 / PR #4589 added for exactly this) with no scaling round trip. Measured on this call shape, old route vs new: 720 combinations (10 locales x 18 values x 4 precisions) — 0 convention differences, 130 numeral differences, the same 13 in every locale. On the wide en-US grid 27,577 of 1,200,003 forms move. The extremes are digit-exact again: MAX_SAFE_INTEGER points rendered `9,007,199,254,740,990%` and now render `9,007,199,254,740,991%`. The locale percent CONVENTION is unchanged — this is numeral-only. Percent SCALING (`percentDisplayValue`) is upstream of the render and untouched; both are pinned unmoved. The #4576 cross-surface pin flips from NOT-a-defect to an AGREEMENT pin, declared in place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 #4590
Mechanism
formatPercentBodyrendered a value that is ALREADY in percentage POINTS throughIntl'sstyle: 'percent', which expects a FRACTION — so it divided by 100 forIntlto multiply straight back:That round trip is not value-preserving.
Intlformats from the SHORTEST decimal representation of the double it is handed, and the quotient's is not the authored one:1.005is1.005, but1.005 / 100is0.010049999999999999, which percent-scales to1.0049999999999999and rounds DOWN. The division loses the digit, not the rounding — which is why it reproduced identically in every locale, Arabic-Indic digits included.The body now renders the points directly, through the option #4576 / PR #4589 added for exactly this:
Affix-parity re-verification, on THIS call shape
PR #4589 measured
style: 'unit'/unit: 'percent'affix parity forformatMeasure's path across 171 locale tags. Re-measured here forformatPercent's own call shape, old route vs new, with the affix isolated from the numerals viaIntl.formatToParts— leading and trailing non-digit parts, the group separator glyph, the decimal separator glyph, and whether grouping happened:130 = 13 per locale, the SAME 13 in every one of the ten — which is the locale-independence claim, measured rather than asserted. Locale set: en-US, de-DE, fr-FR, tr-TR, ar-EG, ja-JP, zh-CN, ru-RU, sv-SE, bn-IN.
One methodology note, because it initially read as 720 convention diffs:
style: 'percent'labels the sign partpercentSignwhilestyle: 'unit'labels the SAME glyphunit. That is anIntlAPI label, not a rendered convention —formatPercentreturns a string and no caller ever sees the part type — so the two are normalized to one name in the skeleton. Every rendered byte is identical; only the part'stypefield differs.Convention pins at 1234.5 p1, old route vs new route. Written in
U+XXXXnotation rather than pasted, because three of the characters involved (U+00A0, U+202F, U+061C) are INVISIBLE — a pasted table would look like it had ordinary spaces, and one of them would be undetectable entirely:Negatives, where sign position and sign glyph are also convention — all identical, including Turkish's minus outside the prefixed sign, Arabic's leading U+061C, and Swedish's U+2212 MINUS SIGN (not an ASCII hyphen):
Red-first
Predictions were written into the test file headers before the run. The red run had the new expectations present and the fix absent:
Verbatim assertion errors, matching the issue's measured table:
The de-DE case in that run is quoted separately rather than pasted, because the runner emits a raw U+00A0 inside both strings and pasting it here would silently turn into an ordinary space:
The 40 that passed on the RED side are the must-not-change half — every convention pin, every negative-sign pin, the affix-parity check and the scaling pins were green before the fix, which is what makes them evidence rather than decoration.
Green after the fix:
Test Files 5 passed (5) / Tests 64 passed (64).Reverse verification took the fix out with
git checkout origin/main -- packages/fields/src/index.tsx(nevergit stash— the stash stack is shared across worktrees), re-ran to the same 9 red, then restored from a patch file and confirmed the restore with sha256:Moving pins, each declared
percent-cell-vs-measure-4576.test.ts— the NOT-a-defect pin becomes an AGREEMENT pin, declared in place with before/after. Before:formatPercent(1.005, 2, 'en-US')was'1.00%'against the measure's'1.01%'. After: both'1.01%'. It also now asserts the two AGAINST EACH OTHER, not just against two literals, so a future divergence at either end fails whatever they happen to render.9,007,199,254,740,990%, now9,007,199,254,740,991%(a 1 that had turned into a 0).99,999,999,999,999,990,000,000%, now100,000,000,000,000,000,000,000%.#4565's
formatPercentsuite (percent-formatter-locale-4553.test.ts) has ZERO moving pins — it passed untouched on both sides of the fix, and is visible as the1 passedfile in the red run above. Every one of its 25 assertions was measured against both routes beforehand; none differ.Must-not-change proofs
formatPercent's affix (digits stripped) againstIntl's ownstyle: 'percent'affix for each of the ten locales, so a future convention drift fails here.percentDisplayValue(a stored fraction below 1 scales by 100, a value at or above 1 passes through) decides WHICH number is rendered; this card changes only HOW. Pinned unmoved, including the boundary at exactly 1.packages/core/src/utils/not touched — this PR consumespercentPoints, it does not edit it. Diff is 4 files, all inpackages/fieldsplus the changeset..d.ts measurement and grading
dist/deleted and notsconfig.tsbuildinfopresent (this package generates declarations via unplugin-dts, not tsc project references), then rebuilt:All 76 declaration files hash-identical — expected, since no signature changed. Graded
'@object-ui/fields': minoranyway, on the published-behaviour-move precedent (#4495 / #4271 / #4479): the rendered output of an exported formatter moves for 27,577 of 1,200,003 measured en-US forms. Both facts are reported plainly rather than letting the identical.d.tsargue for a patch. Never major.Verification
'@object-ui/fields^...'+ the package itself)vitest run— fields, plugin-grid, plugin-gantt, plugin-dashboard, plugin-report, core, i18nvitest run— app-shell, components, react, plugin-detail, plugin-list, plugin-form, plugin-view, mobileturbo run type-check --filter='...@object-ui/fields'(PREFIX = downstream consumers)Consumer census for
formatPercent(grepped, then every consuming package's suite swept):packages/plugin-grid/src/ObjectGrid.tsx,packages/plugin-gantt/src/ObjectGantt.tsx,packages/plugin-dashboard/src/recordFields.tsx, andPercentCellRendererinsidepackages/fieldsitself.Byte discipline
Every non-ASCII character in a test expectation is a backslash-u escape in the source, never a pasted byte — three of them (U+00A0, U+202F, U+061C) are invisible on screen, so a raw byte would leave the file holding a byte where a reader greps for the escape text.
node scripts/check-control-bytes.mjspasses, and a codepoint-exact self-scan over all four touched files (including the untracked ones) finds no control bytes and no raw invisible characters.The same hazard is why the two convention tables above are written in
U+XXXXnotation. The first revision of this body pasted them literally, and the ar-EG rows lost their trailing U+061C outright between composing and storing — the exact failure the notation prevents, reproduced live in this PR's own description.Generated by Claude Code