From ef1569e0ebf6e5da2acd36be0a79d4a0cce1d530 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=AE=B7=E4=BA=AE=E8=BE=89?= Date: Thu, 13 Aug 2026 16:59:47 +0000 Subject: [PATCH] =?UTF-8?q?fix(fields):=20formatPercent=20renders=20points?= =?UTF-8?q?=20directly=20=E2=80=94=20ties=20round=20half-up=20and=20extrem?= =?UTF-8?q?es=20keep=20every=20digit=20(#4590)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .changeset/percent-tie-halfup-4590.md | 33 +++ .../percent-cell-vs-measure-4576.test.ts | 50 ++-- .../__tests__/percent-tie-halfup-4590.test.ts | 224 ++++++++++++++++++ packages/fields/src/index.tsx | 38 ++- 4 files changed, 319 insertions(+), 26 deletions(-) create mode 100644 .changeset/percent-tie-halfup-4590.md create mode 100644 packages/fields/src/__tests__/percent-tie-halfup-4590.test.ts diff --git a/.changeset/percent-tie-halfup-4590.md b/.changeset/percent-tie-halfup-4590.md new file mode 100644 index 000000000..5d764d7f7 --- /dev/null +++ b/.changeset/percent-tie-halfup-4590.md @@ -0,0 +1,33 @@ +--- +'@object-ui/fields': minor +--- + +fix(fields): `formatPercent` renders percentage points directly — ties round half-up and extremes keep every digit + +`formatPercent` rendered a value that is already in percentage POINTS through +`Intl`'s `style: 'percent'`, which expects a FRACTION, so the body 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. A stored +`1.005` at 2 decimals rendered `1.00%` where half-up on the authored decimal is +`1.01%`; `1.45` at 1 decimal rendered `1.4%` for `1.5%`. Every case was a +last-digit off-by-one — the failure mode least likely to be noticed and most +likely to be trusted. + +The body now renders through `style: 'percentPoints'` with no scaling round +trip. Measured on this repo's runner (node v22.22.2 / ICU 78.2), 27,577 of +1,200,003 ordinary en-US forms move (0.005-step grid to 2,000, precisions +0/1/2), and the same artefact at the top of the double range is gone too: +`Number.MAX_SAFE_INTEGER` percentage 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 a numeral move only. +`'percentPoints'` is `Intl`'s `style: 'unit'` / `unit: 'percent'`, re-measured on +this call shape across 720 combinations (10 locales x 18 values x 4 precisions): +0 convention differences, 130 numeral differences. The no-break space in +de/fr/ru/sv, Turkish's prefixed sign, Arabic's own percent sign and Bengali's +digits all render exactly as before. Percent SCALING (a stored fraction below 1 +scaling by 100) is upstream of the render and untouched. + +A percentage point now reads identically in a list cell and in a dashboard +measure, which `formatMeasure` already rendered this way. diff --git a/packages/fields/src/__tests__/percent-cell-vs-measure-4576.test.ts b/packages/fields/src/__tests__/percent-cell-vs-measure-4576.test.ts index 2fa8ad536..9b24437ee 100644 --- a/packages/fields/src/__tests__/percent-cell-vs-measure-4576.test.ts +++ b/packages/fields/src/__tests__/percent-cell-vs-measure-4576.test.ts @@ -16,10 +16,17 @@ * cannot import `formatPercent` — so the agreement is pinned here, the one * place both are in scope. * - * This file adds no implementation. `formatPercent` is untouched by #4576; it - * has rendered through `Intl` since #4553 / PR #4565 and is the SIDE the measure + * This file adds no implementation. `formatPercent` was untouched by #4576; it + * has rendered through `Intl` since #4553 / PR #4565 and was the SIDE the measure * formatter moved onto. * + * ── objectui#4590 ── + * The last case in this file was a NOT-a-defect pin recording that the two + * surfaces still disagreed at rounding TIES. #4590 closed that at the + * `formatPercent` end (it now renders percentage points directly, through the + * same `style: 'percentPoints'` the measure uses), so that pin is now an + * AGREEMENT pin. It is declared in place, at the case itself. + * * ── PREDICTIONS, written before the run ── * RED before the fix: every locale whose percent convention has a space or a * different sign — de, fr, es, ru, tr, ar. `formatMeasure` returned a literal @@ -79,23 +86,30 @@ describe('a percent renders under ONE convention in a cell and in a measure (#45 expect(formatMeasure(1234.5, '0.0%', undefined, 'whole', 'en-US')).toBe('1,234.5%'); }); - it('NOT a defect pin — the two still round ties differently, and that is #4565 not #4576', () => { - // Honest label rather than a quiet omission, and it records a real - // remaining gap. - // `formatPercent` divides by 100 so `Intl`'s `style: 'percent'` can multiply - // back, and that round trip is lossy at rounding ties; `formatMeasure` now - // formats the percentage points directly and keeps the authored decimal. - // Measured, 2,999 display magnitudes at or above 1 differ this way on a - // 0.005-step grid to 200 (and 27,581 of 1,200,013 forms overall). + it('AGREEMENT pin — the two now round ties the same way (was NOT-a-defect, closed by #4590)', () => { + // ── PIN MOVED (objectui#4590) ── + // This case was a NOT-a-defect pin: it asserted the two surfaces DISAGREED + // at rounding ties, recorded honestly rather than omitted, and said the gap + // was #4565's residue rather than #4576's business. #4590 closed it at the + // `formatPercent` end, so the pin flips from recording a divergence to + // asserting the agreement. + // + // Before: formatPercent(1.005, 2, 'en-US') -> '1.00%' (the artefact) + // formatMeasure(1.005, …) -> '1.01%' (the faithful one) + // After: both -> '1.01%'. // - // 1.005 percentage points to 2 decimals is half-up `1.01`, so the MEASURE is - // the faithful one here and the CELL is the artefact — the opposite of what - // "the cell is the reference" would suggest. It is a NARROWER divergence - // than the convention split this card closed, it predates this card at the - // `formatPercent` end, and closing it means changing `formatPercent`, which - // is outside #4576's surface. Filed separately; pinned here so the next - // reader finds it recorded rather than rediscovers it. - expect(formatPercent(1.005, 2, 'en-US')).toBe('1.00%'); + // The cause was `formatPercent` dividing by 100 so `Intl`'s + // `style: 'percent'` could multiply back — lossy at ties, because the + // quotient's shortest decimal representation is not the authored one. + // `formatMeasure` had already moved onto `style: 'percentPoints'` in #4576; + // the cell now renders through the same option, so there is one route and + // no second rounding behaviour to keep in step. + expect(formatPercent(1.005, 2, 'en-US')).toBe('1.01%'); expect(formatMeasure(1.005, '0.00%', undefined, 'whole', 'en-US')).toBe('1.01%'); + // …and asserted as an AGREEMENT rather than two coincidences, so a future + // divergence at either end fails here whatever the two happen to render. + expect(formatPercent(1.005, 2, 'en-US')).toBe( + formatMeasure(1.005, '0.00%', undefined, 'whole', 'en-US'), + ); }); }); diff --git a/packages/fields/src/__tests__/percent-tie-halfup-4590.test.ts b/packages/fields/src/__tests__/percent-tie-halfup-4590.test.ts new file mode 100644 index 000000000..8629406fa --- /dev/null +++ b/packages/fields/src/__tests__/percent-tie-halfup-4590.test.ts @@ -0,0 +1,224 @@ +/** + * 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#4590 — `formatPercent` rounds a tie the wrong way, because it renders + * a value that is ALREADY in percentage points through `Intl`'s + * `style: 'percent'`, which wants a FRACTION. + * + * The old body divided by 100 so `Intl` could multiply straight back: + * + * formatDisplayNumber(displayValue / 100, { style: 'percent', … }) + * + * That round trip is not value-preserving. `Intl` formats from the SHORTEST + * decimal representation of the double it is handed, and `displayValue / 100` + * has a different shortest representation from `displayValue`: `1.005` is + * `1.005`, but `1.005 / 100` is `0.010049999999999999`, which percent-scales to + * `1.0049999999999999` and rounds DOWN. The DIVISION loses the digit, not the + * rounding. It is therefore a numeral defect and not a convention one — it + * reproduces identically in every locale, in Arabic-Indic digits too. + * + * The fix is the option #4576 / PR #4589 put in `@object-ui/core` for exactly + * this: `style: 'percentPoints'` formats the points DIRECTLY (`Intl`'s + * `style: 'unit'` / `unit: 'percent'` / `unitDisplay: 'narrow'`), which was + * measured to give a byte-identical percent affix to `style: 'percent'` across + * all 171 locale tags tested. + * + * ── PREDICTIONS, written before the run ───────────────────────────────────── + * Runner measured: node v22.22.2, ICU 78.2, machine locale en-US. + * + * RED before the fix — the numerals: + * formatPercent(1.005, 2, 'en-US') `1.00%` → `1.01%` + * formatPercent(1.025, 2, 'en-US') `1.02%` → `1.03%` + * formatPercent(1.45, 1, 'en-US') `1.4%` → `1.5%` + * formatPercent(1.055, 2, 'en-US') `1.05%` → `1.06%` + * formatPercent(99999.995, 2, 'en') `99,999.99%` → `100,000.00%` + * MAX_SAFE_INTEGER p0 `9,007,199,254,740,990%` → `…991%` + * 1e23 p0 `99,999,999,999,999,990,000,000%` + * → `100,000,000,000,000,000,000,000%` + * the same four ties in de-DE / tr-TR / ar-EG — the locale-independence + * claim, red in every one of them + * + * GREEN ON BOTH SIDES — the must-not-change set, and the STOP condition if + * any of it moves (this card is NUMERAL-only; a convention move is a defect + * in the fix, not a pin to update): + * the percent CONVENTION of all ten locales at 1234.5 p1 — de/fr/ru/sv's + * no-break space, tr's PREFIX sign, ar's U+066A + U+061C, en/ja/zh's bare + * suffix, fr's U+202F group separator, bn's Bengali digits + * the sign position and glyph on negatives (sv-SE's U+2212 included) + * the affix parity check against `Intl`'s own `style: 'percent'` + * `percentDisplayValue`'s fraction/whole scaling — upstream of the render + * and deliberately untouched by this card + * + * MEASURED, this call shape, old route vs new: 10 locales x 18 values x 4 + * precisions = 720 combinations — 0 convention diffs, 130 numeral diffs (13 per + * locale, the SAME 13 in every locale). On the wide en-US grid the issue + * quantified (0.005 steps to 2000, precisions 0/1/2) 27,577 of 1,200,003 forms + * move. Every one is a last-digit-off-by-one. + * + * Every non-ASCII byte below is written as a `\u` escape, never pasted. + */ + +import { describe, it, expect } from 'vitest'; +import { formatPercent } from '../index'; + +/** U+00A0 NO-BREAK SPACE — de/fr/ru/sv put one before the percent sign. */ +const NBSP = '\u00a0'; +/** U+202F NARROW NO-BREAK SPACE — fr's group separator. */ +const NNBSP = '\u202f'; +/** U+066A ARABIC PERCENT SIGN, U+061C ARABIC LETTER MARK. */ +const ARABIC_PERCENT = '\u066a'; +const ALM = '\u061c'; + +describe('formatPercent rounds a tie half-up on the authored decimal (#4590)', () => { + /** + * MOVING PINS — the issue's measured table, verbatim. Each was the artefact + * of the `/ 100` round trip; the right-hand column is half-up on the decimal + * the author actually wrote. + */ + it.each([ + [1.005, 2, '1.01%', '1.00%'], + [1.025, 2, '1.03%', '1.02%'], + [1.45, 1, '1.5%', '1.4%'], + [1.055, 2, '1.06%', '1.05%'], + ])('formatPercent(%s, %s) is %s (was %s)', (value, precision, expected) => { + expect(formatPercent(value, precision, 'en-US')).toBe(expected); + }); + + /** + * A tie that carries into a new digit COLUMN rather than just flipping the + * last one — the round trip lost the carry as well as the digit. + */ + it('carries across the grouping boundary: 99999.995 to 2 decimals', () => { + // was `99,999.99%` + expect(formatPercent(99999.995, 2, 'en-US')).toBe('100,000.00%'); + }); + + /** + * The defect is in the VALUE, not the convention, so it must reproduce in + * every locale — including one that writes the sign in front and one that + * uses neither ASCII digits nor an ASCII sign. + */ + it('the same ties move in every locale — this is a numeral defect', () => { + expect(formatPercent(1.005, 2, 'de-DE')).toBe(`1,01${NBSP}%`); // was `1,00 %` + expect(formatPercent(1.45, 1, 'de-DE')).toBe(`1,5${NBSP}%`); // was `1,4 %` + expect(formatPercent(1.005, 2, 'tr-TR')).toBe('%1,01'); // was `%1,00` + // U+0661 U+066B U+0660 U+0661 — Arabic-Indic `1`, decimal separator, `0`, `1` + expect(formatPercent(1.005, 2, 'ar-EG')).toBe( + `\u0661\u066b\u0660\u0661${ARABIC_PERCENT}${ALM}`, + ); // was `\u0661\u066b\u0660\u0660` + the same affix + }); +}); + +describe('formatPercent keeps every digit at the top of the double range (#4590)', () => { + /** + * MOVING PINS, before/after verbatim. `Number.MAX_SAFE_INTEGER` percentage + * points used to render with its last digit replaced by a zero, and 1e23 used + * to render as a number that is not 1e23 at all. Both are the same artefact + * as the ties, at the other end of the range: divide by 100 and the shortest + * representation of the quotient no longer round-trips. + */ + it('MAX_SAFE_INTEGER keeps its last digit', () => { + // was `9,007,199,254,740,990%` — a 1 turned into a 0 + expect(formatPercent(Number.MAX_SAFE_INTEGER, 0, 'en-US')).toBe('9,007,199,254,740,991%'); + }); + + it('1e23 renders as 1e23', () => { + // was `99,999,999,999,999,990,000,000%` + expect(formatPercent(1e23, 0, 'en-US')).toBe('100,000,000,000,000,000,000,000%'); + }); +}); + +describe('MUST NOT CHANGE: the locale percent CONVENTION (#4590 is numeral-only)', () => { + /** + * The affix is what `style: 'percentPoints'` promises to preserve, so this is + * the half that proves the fix did not become a convention change. Byte-exact + * strings, chosen at a magnitude and precision where no tie is in play — they + * are GREEN on both sides of the fix, and a diff here is a STOP condition. + */ + it.each([ + ['en-US', '1,234.5%'], + ['de-DE', `1.234,5${NBSP}%`], + ['fr-FR', `1${NNBSP}234,5${NBSP}%`], + ['tr-TR', '%1.234,5'], // sign in FRONT + ['ar-EG', `\u0661\u066c\u0662\u0663\u0664\u066b\u0665${ARABIC_PERCENT}${ALM}`], + ['ja-JP', '1,234.5%'], + ['zh-CN', '1,234.5%'], + ['ru-RU', `1${NBSP}234,5${NBSP}%`], + ['sv-SE', `1${NBSP}234,5${NBSP}%`], + ['bn-IN', `\u09e7,\u09e8\u09e9\u09ea.\u09eb%`], + ])('%s renders its own percent convention, unchanged', (locale, expected) => { + expect(formatPercent(1234.5, 1, locale)).toBe(expected); + }); + + /** + * Sign POSITION and sign GLYPH are convention too — Turkish puts the minus + * outside its prefixed percent sign, Arabic leads with U+061C, and Swedish + * uses U+2212 MINUS SIGN rather than ASCII hyphen-minus. None of it moves. + */ + it.each([ + ['en-US', '-45.5%'], + ['de-DE', `-45,5${NBSP}%`], + ['tr-TR', '-%45,5'], + ['ar-EG', `${ALM}-\u0664\u0665\u066b\u0665${ARABIC_PERCENT}${ALM}`], + ['sv-SE', `\u221245,5${NBSP}%`], + ])('%s keeps its negative-sign convention', (locale, expected) => { + expect(formatPercent(-45.5, 1, locale)).toBe(expected); + }); + + /** + * The parity claim asserted rather than trusted: the affix `formatPercent` + * now produces must be the one `Intl`'s OWN `style: 'percent'` produces for + * the same locale. Comparing the affix alone (digits stripped) is deliberate + * — the numerals are exactly what this card moves, so a whole-string compare + * would re-assert the defect. + */ + it('the affix is byte-identical to Intl style:percent, for every locale here', () => { + const affix = (rendered: string) => rendered.replace(/[\p{Nd}]/gu, ''); + for (const locale of ['en-US', 'de-DE', 'fr-FR', 'tr-TR', 'ar-EG', 'ja-JP', + 'zh-CN', 'ru-RU', 'sv-SE', 'bn-IN']) { + const cell = formatPercent(1234.5, 1, locale); + const intl = new Intl.NumberFormat(locale, { + style: 'percent', + minimumFractionDigits: 1, + maximumFractionDigits: 1, + }).format(12.345); + expect(affix(cell), `percent affix for ${locale}`).toBe(affix(intl)); + } + }); +}); + +describe('MUST NOT CHANGE: percent SCALING is upstream of the render (#4590)', () => { + /** + * `percentDisplayValue` (a stored fraction below 1 scales by 100, a value at + * or above 1 passes through) decides WHICH number gets rendered; this card + * changes only HOW that number is rendered. Pinned unmoved so a future reader + * cannot mistake the two halves for one. + */ + it('a stored fraction still scales, a stored whole number still does not', () => { + expect(formatPercent(0.8, 0, 'en-US')).toBe('80%'); + expect(formatPercent(0.5, 0, 'en-US')).toBe('50%'); + expect(formatPercent(0.075, 1, 'en-US')).toBe('7.5%'); + expect(formatPercent(80, 0, 'en-US')).toBe('80%'); + expect(formatPercent(100, 0, 'en-US')).toBe('100%'); + // The boundary itself: 1 is NOT a fraction, so it stays 1%. + expect(formatPercent(1, 0, 'en-US')).toBe('1%'); + // …which is why the tie cases above are authored just above 1. + expect(formatPercent(0.9999, 0, 'en-US')).toBe('100%'); + }); + + it('a malformed locale tag still falls back instead of throwing', () => { + expect(() => formatPercent(80, 0, 'not a locale')).not.toThrow(); + expect(formatPercent(80, 0, 'not a locale')).toContain('80'); + }); + + it('is still callable with no locale and no precision', () => { + expect(formatPercent(80)).toBe('80%'); + expect(formatPercent(80, 0)).toBe('80%'); + }); +}); diff --git a/packages/fields/src/index.tsx b/packages/fields/src/index.tsx index a98c8de95..a38791383 100644 --- a/packages/fields/src/index.tsx +++ b/packages/fields/src/index.tsx @@ -478,15 +478,37 @@ export function formatNumber(value: number, decimals: number = 2, locale?: strin */ function formatPercentBody(displayValue: number, precision: number, locale?: string): string { try { - // `style: 'percent'` multiplies by 100, so the display magnitude is divided - // back out. Going through `Intl` rather than appending a literal '%' is what - // buys the locale's percent CONVENTION and not merely its separators: - // German writes `1.235 %` with a no-break space before the sign, English - // `1,235%` with none. Both bounds are set to `precision` so the width is - // exactly the one the caller asked for — the same contract `toFixed` gave. - return formatDisplayNumber(displayValue / 100, { + // `style: 'percentPoints'` renders a value that is ALREADY in percentage + // points, so there is no `/ 100` here. Going through `Intl` rather than + // appending a literal '%' is what buys the locale's percent CONVENTION and + // not merely its separators: German writes `1.235 %` with a no-break space + // before the sign, English `1,235%` with none, Turkish puts the sign in + // FRONT. Both bounds are set to `precision` so the width is exactly the one + // the caller asked for — the same contract `toFixed` gave. + // + // ⚠️ NOT `style: 'percent'` (objectui#4590). That style wants a FRACTION, so + // this used to divide by 100 for `Intl` to multiply straight back — and the + // 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 — so a stored 1.005 rendered `1.00%` where half-up is `1.01%`. + // The DIVISION lost the digit, not the rounding, which is why it reproduced + // in every locale and why 27,577 of 1,200,003 ordinary en-US forms moved + // (0.005-step grid to 2,000, precisions 0/1/2), every one a last-digit + // off-by-one. The same artefact reached the top of the double range: + // `MAX_SAFE_INTEGER` points rendered `…740,990%` for `…740,991%`. + // + // The affix is unchanged by the switch: `'percentPoints'` is `Intl`'s + // `style: 'unit'` / `unit: 'percent'` / `unitDisplay: 'narrow'`, measured + // byte-identical to `style: 'percent'` across all 171 locale tags in #4576 + // and re-measured on THIS call shape in #4590 — 720 combinations (10 locales + // x 18 values x 4 precisions), 0 convention diffs, 130 numeral diffs. + // `formatMeasure` renders through the same option, so a percentage point + // reads identically in a list cell and in a dashboard measure. + return formatDisplayNumber(displayValue, { locale, - style: 'percent', + style: 'percentPoints', minimumFractionDigits: precision, maximumFractionDigits: precision, });