ICU: backport ICU-23110 (percent formatRange scaled twice on approximately path)

[robobun]

Problem

With style: "percent", Intl.NumberFormat.prototype.formatRange (and formatRangeToParts) applies the scale/100 multiplier twice whenever the range collapses to a single approximate value (equal endpoints, or endpoints that round to the same formatted number):

const nf = new Intl.NumberFormat("en", { style: "percent" });
nf.format(0.5);             // "50%"
nf.formatRange(0.5, 0.5);   // bun:  "~5,000%"   node: "~50%"
nf.formatRange(0.5, 0.6);   // bun:  "50% – 60%" (correct)

Reproduces in every locale and through every input path (double, BigInt, string). Distinct ranges, style: "unit"/"currency"/"decimal" collapses, and macOS are all unaffected.

Cause

ICU-23110. NumberRangeFormatterImpl::format calls formatterImpl1.preProcess(data.quantity1, ...) (which applies the skeleton's scale/100), then on the UNUM_IDENTITY_FALLBACK_APPROXIMATELY path hands the already-scaled data.quantity1 to formatApproximately, which calls fApproximatelyFormatter.preProcess(data.quantity1, ...) again. The resetExponent() call in between only clears compact/scientific exponent state, not the multiplier, so 0.5 → 50 → 5000.

JSC builds the range formatter from the same skeleton as format() (" percent scale/100") and passes raw inputs to unumrf_formatDoubleRange / unumrf_formatDecimalRange, which is also what V8 does; Node is correct only because it ships ICU 78.

Fix

Backport unicode-org/icu@757be359 (first released in ICU 78.1) as a build-time patch and apply it in every Dockerfile that builds ICU from source:

• Dockerfile / Dockerfile.musl / Dockerfile.windows: applied alongside the existing udata-decompress-hook.patch.
• Dockerfile.android / Dockerfile.freebsd: first ICU patch for these images; added a COPY for the patch file and applied it in the target build after extraction.

Dockerfile.macos links Apple's libicucore and is already correct, so no change there.

The patch takes a copy of data.quantity1 before the first preProcess and passes that copy to formatApproximately, so the approximately formatter starts from the original quantity.

Verification

• Patch applies cleanly to ICU 75.1 (linux/musl/android/freebsd) and 73.2 (windows, one-line offset).
• Built ICU 75.1 with and without the patch and drove unumrf_formatDoubleRange directly:

                        unpatched       patched
formatRange(0.5, 0.5)   ~5,000%         ~50%
formatRange(0.5, 0.6)   50% – 60%       50% – 60%
formatRange(15, 15)     ~150,000%       ~1,500%

Test coverage lands in oven-sh/bun alongside the WEBKIT_VERSION bump.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: aa1a6af3-245a-4dd8-a16c-9115cfbe93d6

📥 Commits

Reviewing files that changed from the base of the PR and between 34c01d1 and 46c558e.

📒 Files selected for processing (6)

• Dockerfile
• Dockerfile.android
• Dockerfile.freebsd
• Dockerfile.musl
• Dockerfile.windows
• icu/icu-23110-numrange-percent.patch

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

LGTM — mechanical backport of an upstream ICU fix, applied via the same patch -p1 pattern already used for udata-decompress-hook.patch. The inline note about build-icu.ps1 is a pre-existing gap on a debug-only artifact and not blocking.

Also checked:

• android/freebsd correctly add the patch package and COPY only the single patch file (those images don't use the decompress hook); patch is applied to the target build, host build is tools-only so leaving it unpatched is fine.
• Dropping resetExponent() is correct since quantityBackup is copied before the first preProcess and was never exponent-adjusted.
• patch -p1 runs from the extracted ICU root and the hunk paths are source/i18n/..., matching all five images.

Extended reasoning...

Overview

Adds icu/icu-23110-numrange-percent.patch (backport of unicode-org/icu@757be359, fixed upstream in ICU 78.1) and applies it in the five Dockerfiles that build ICU from source: Dockerfile, Dockerfile.musl, Dockerfile.windows (one new line each, next to the existing udata-decompress-hook.patch step), and Dockerfile.android / Dockerfile.freebsd (first ICU patch for those images — adds patch to apt, a targeted COPY of the one patch file, and the patch -p1 invocation between tar extraction and cd source). The patch itself takes a copy of data.quantity1 before formatterImpl1.preProcess and hands that unscaled copy to formatApproximately, so the scale/100 multiplier is applied once instead of twice on the identity/approximately path.

Security risks

None. This is a build-time source patch to a vendored dependency; no new network fetches, no credential handling, no auth/crypto surface. The ICU tarball checksums and download URLs are unchanged.

Level of scrutiny

Low–moderate. The change is build-infrastructure only and follows an established pattern in this repo (udata-decompress-hook.patch is applied the same way in the same places). The C++ change is a verbatim upstream backport with a linked ICU ticket, upstream commit hash, and an explicit drop condition (ICU >= 78) in the patch header. If the patch failed to apply on either 75.1 or 73.2 the Docker build would fail loudly, and the PR description reports it was verified against both.

Other factors

• The one finding (native build-icu.ps1 path unpatched) is a pre-existing infrastructure gap that also skips the other ICU patch, affects only bun-webkit-windows-arm64-debug, and is correctly scoped as a follow-up nit rather than a defect introduced here.
• Dockerfile.macos is intentionally untouched (links Apple's system libicucore, already correct per the description).
• Android/FreeBSD host ICU builds are left unpatched, which is fine — they only produce icupkg/pkgdata tools; the target-arch libicuin that ships is patched.
• Test coverage is stated to land in oven-sh/bun with the WEBKIT_VERSION bump, which is the normal flow for this repo.

[claude]

🟡 Note that the native Windows build path (windows-release.ps1 → build-icu.ps1) — still used by CI for the bun-webkit-windows-arm64-debug artifact — downloads icu4c-73_2-src.tgz and builds via MSBuild without applying any .patch to the ICU C++ source, so that artifact will still show formatRange(0.5, 0.5) → "~5,000%". This is a pre-existing gap (udata-decompress-hook.patch is also skipped there) and only affects a debug-only artifact, so probably fine as a follow-up rather than blocking this PR.

Extended reasoning...

What this is

The PR applies icu/icu-23110-numrange-percent.patch in every Dockerfile that builds ICU from source — Dockerfile, Dockerfile.musl, Dockerfile.android, Dockerfile.freebsd, and Dockerfile.windows — exactly as the description says. However, there is one more ICU-from-source build path in the repo that CI still exercises: the native Windows runner job, which goes windows-release.ps1 → build-icu.ps1. That path is not touched by this PR, so the artifact it produces will still contain unpatched ICU 73.2 with the double-scaling bug.

Code path

In .github/workflows/build-reusable.yml:

• Lines 151–153 note that most Windows variants moved to Dockerfile.windows, but the native windows job is kept for arm64-debug, and windows-release.ps1 "remains for building on a real Windows machine".
• Lines 247–254: the windows job matrix has a single entry — runner: windows-11-arm, build-type: Debug, label: bun-webkit-windows-arm64-debug.
• Line 298: that job runs ./windows-release.ps1 -Platform ARM64.
• Lines 712/806/894: the resulting bun-webkit-windows-arm64-debug artifact is downloaded and published to the autobuild-{sha} release alongside all the Docker-built artifacts.

In build-icu.ps1:

• Line 32: downloads icu4c-73_2-src.tgz from the upstream release.
• Line 85 onward: the only "patching" is text replacement on .vcxproj / .props / makedata.mak build-config files. No .patch from icu/ is ever applied to source/i18n/numrange_impl.cpp or numrange_impl.h.

So sicuin.lib in bun-webkit-windows-arm64-debug is compiled from unmodified ICU 73.2 numrange_impl.cpp.

Why nothing existing prevents it

Dockerfile.windows (lines 209–210) does apply the patch, but that file only produces the cross-compiled x64 / arm64 release/LTO/asan artifacts. The native windows job doesn't use Docker at all — it invokes build-icu.ps1 directly on the runner, and that script has never had a step that reads icu/*.patch.

Step-by-step proof

1. CI runs the windows job on windows-11-arm → ./windows-release.ps1 -Platform ARM64.
2. windows-release.ps1 calls build-icu.ps1, which fetches icu4c-73_2-src.tgz and extracts it.
3. build-icu.ps1 edits vcxproj/props for /MT static, then runs MSBuild. source/i18n/numrange_impl.cpp is untouched, so it still contains data.quantity1.resetExponent(); fApproximatelyFormatter.preProcess(data.quantity1, ...) on the approximately path.
4. The resulting sicuin.lib is packed into bun-webkit-windows-arm64-debug.tar.gz and published.
5. A Bun built against that artifact evaluates new Intl.NumberFormat("en", {style:"percent"}).formatRange(0.5, 0.5): JSC calls unumrf_formatDoubleRange(0.5, 0.5), ICU's formatterImpl1.preProcess scales 0.5 → 50, the identity check picks UNUM_IDENTITY_FALLBACK_APPROXIMATELY, formatApproximately calls fApproximatelyFormatter.preProcess on the same already-scaled quantity → 5000, and the output is "~5,000%" instead of "~50%".

Impact

Low. Only the bun-webkit-windows-arm64-debug artifact is affected — a Debug build, not something end users install. All release/LTO Windows artifacts come from Dockerfile.windows and are correctly patched. Also, this is a pre-existing infrastructure gap: build-icu.ps1 already skips udata-decompress-hook.patch, so "ICU source patches are not applied on the native Windows path" predates this PR rather than being introduced by it. The PR description scoped itself to "every Dockerfile that builds ICU from source", and it delivers exactly that.

Fix

As a follow-up (not blocking): have build-icu.ps1 apply icu/*.patch after extraction — e.g. git apply (git is on the runner), or patch.exe from Git for Windows, or a targeted PowerShell -replace on numrange_impl.cpp/.h. Alternatively, note the limitation in the PR / patch header so the arm64-debug divergence is documented.

[github-actions]

Preview Builds

Commit	Release	Date
46c558e6	autobuild-preview-pr-374-46c558e6	2026-07-29 12:47:51 UTC