css: keep the lower bound when simplifying clamp()

[robobun]

Replaces #32290. #39515 (folding min(), max() and clamp() over plain numbers) is stacked on this PR and adds the number case to the helper introduced here.

Problem

• The CSS minifier drops the lower bound of a clamp() whose center is not above its maximum. width: clamp(10px, 5px, 20px) prints width:min(20px,5px), which is 5px instead of 10px. clamp(10%, 5%, 20%) prints min(20%,5%), and clamp(1em, 2px, 3px) prints min(3px,2px), dropping the 1em bound that may be the larger one. Same on bun 1.4.0 and main. Reported in css: fix clamp() simplification to preserve min argument #32290.
• When the center is above the maximum, the result is left as max(10px,20px) instead of 20px, because the minimum is never compared.
• Cause: the clamp() arm of Calc::parse_with in src/css/values/calc.rs compares the center with the maximum and, when the center is not above it, sets min = None (calc.rs:370 on main) where it means max = None. The (None, Some(max)) arm of the match below (calc.rs:379) then prints min(max, center). lightningcss does not have this bug and also compares the center with the minimum afterwards, this came in with the port.

Fix

• The arm is restructured around the definition clamp(min, center, max) = max(min, min(center, max)). If the center and the maximum are comparable, the maximum is applied (the center becomes the maximum when it is above it), then the minimum: a comparable minimum folds the result to one value, an incomparable one is kept as max(min, center). If the center and the maximum are not comparable, the clamp() is kept unchanged, as before and as upstream.
• The two comparisons go through a new Calc::partial_cmp_args, which orders two Calc::Value arguments through the existing PartialCmp and returns None for anything else, so a Calc::Number, a sum or a nested function is never compared, exactly as before. The Option bookkeeping is gone, and with it the min(max, center) output, which only the bug produced.
• Every asserted output is the one lightningcss 1.33.0 prints for the same input, including the eleven rows taken from its own calc tests.
• Tests: test/js/bun/css/css.test.ts, new clamp() simplification block: lightningcss's rows (<length> through border-width, sums as arguments, unit conversion, an incomparable center, an incomparable minimum, a sum as a center or a bound), the center equal to either bound, the minimum winning over a smaller maximum with the center between the bounds and, in the two rows of the clamp() WPT, above both (these only pass when the minimum is compared with the center after the maximum has replaced it), css: fix clamp() simplification to preserve min argument #32290's incomparable lower bound with the center below and above the maximum, an incomparable maximum with a comparable minimum, and %, deg, s and a clamp() inside calc(). 18 of the 24 rows fail on the released binary, all 24 pass with the fix.
• Also run: the rest of css.test.ts and test/bundler/css/ (1348 pass), cargo clippy -p bun_css clean.

Background

• Calc<V> is the parsed form of a math expression for a value type V (Length, Percentage, Angle, ...). Calc::Value holds a V and two of them can be ordered at parse time when their units convert into each other (px and pt, not px and em). Calc::Number holds a bare number. Comparing numbers is css: fold min(), max() and clamp() over plain numbers #39515.
• Unparsed fallback: when a property's value parser rejects a value, bun keeps the declaration as the tokens it was written as. rotate: clamp(0deg, 45deg, 30deg) took that path on main (the max() the bug produced is not a value an <angle> accepts), which is why it printed unchanged rather than wrong. width accepts any math function, which is why it printed the wrong min().

Notes

• Related but not fixed here: clamp(1px , 2px , 3px) (a space before a comma) is not simplified at all, because the clamp() arm reads each argument with parse_sum, which stops at whitespace that is not followed by + or -, while min()/max() read theirs through parse_comma_separated. The declaration is kept as written, so the output is correct, only longer. lightningcss 1.33.0 behaves the same. It is a parsing change with its own tests and belongs in its own PR.
• Eleven of the rows are lightningcss's own test_calc clamp rows. bun has none of lightningcss's test_calc, test_math_fn or test_trig tables in css.test.ts; porting them as one block would give the open calc.rs PRs (css: fix rem() sign semantics to match CSS Values 4 #32286, css: use the right-hand operand when folding math functions over mixed length units #38489, css: fold hypot() with three or more arguments pairwise #38501, css: fold one-argument hypot() to the absolute value of its argument #38639, css: wrap a one-argument min()/max() sum or product in calc() #38654, css: fold min(), max() and clamp() over plain numbers #39515) a shared oracle instead of a describe block each. That is a test-only change and is not part of this PR.

---

[review] gate passed · iteration 0 · 2 files touched

fails on main (without fix)

ASAN without fix: 18 failed, 67 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/css/css.test.ts
bun test v1.4.0 (8326d1bd3)

test/js/bun/css/css.test.ts:
Output .flexrow {
  flex-direction: row;
}

.flexcol {
  flex-direction: column;
}

.hello {
  flex-wrap: wrap;
}

.world {
  flex-wrap: nowrap;
}

(pass) css tests > .flexrow {
	flex-direction: row;
}

.flexcol {
	flex-direction: column;
}

.hello {
	flex-wrap: wrap;
}

.world {
	flex-wrap: nowrap;
} [3.18ms]
Output .foo {
  content: "+";
}

(pass) css tests > .foo {
  content: "\2b";
} [1.04ms]
Output div {
  --foo: 1 1 1 #0101011a, 2 2 2 #02020233;
  --bar: 1 1 1 #01010116, 2 2 2 #02020233;
}

(pass) css tests > custom property cases > div {
        --foo: 1 1 1 rgba(1, 1, 1, 0.1), 2 2 2 rgba(2, 2, 2, 0.2);
        --bar: 1 1 1 #01010116, 2 2 2 #02020233;
      } [1.02ms]
Output :root {
  --my-color: red;
  --my-bg: white;
  --my-font-size: 16px;
}

.element {
  color: var(--my-color);
  background-color: var(--my-bg);
  font-size: var(--my-font-size);
}

(pass) css tests > custom property cases > :root {
        --my-color: red;
        --my-bg: w
... (truncated)

release without fix: 67 skipped
bun test v1.4.0-canary.1 (9c303217a)

test/js/bun/css/css.test.ts:
Output .flexrow {
  flex-direction: row;
}

.flexcol {
  flex-direction: column;
}

.hello {
  flex-wrap: wrap;
}

.world {
  flex-wrap: nowrap;
}

(pass) css tests > .flexrow {
	flex-direction: row;
}

.flexcol {
	flex-direction: column;
}

.hello {
	flex-wrap: wrap;
}

.world {
	flex-wrap: nowrap;
} [0.11ms]
Output .foo {
  content: "+";
}

(pass) css tests > .foo {
  content: "\2b";
} [0.02ms]
Output div {
  --foo: 1 1 1 #0101011a, 2 2 2 #02020233;
  --bar: 1 1 1 #01010116, 2 2 2 #02020233;
}

(pass) css tests > custom property cases > div {
        --foo: 1 1 1 rgba(1, 1, 1, 0.1), 2 2 2 rgba(2, 2, 2, 0.2);
        --bar: 1 1 1 #01010116, 2 2 2 #02020233;
      } [0.03ms]
Output :root {
  --my-color: red;
  --my-bg: white;
  --my-font-size: 16px;
}

.element {
  color: var(--my-color);
  background-color: var(--my-bg);
  font-size: var(--my-font-size);
}

(pass) css tests > custom property cases > :root {
        --my-color: red;
        --my-bg: white;
        --my-font-size: 16px;
      }
      .element {
        color: var(--my-color);
        background-color: var(--my-bg);
        font-size: 
... (truncated)

passes on PR (with fix)

ASAN with fix: 67 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/css/css.test.ts
bun test v1.4.0 (8326d1bd3)

test/js/bun/css/css.test.ts:
Output .flexrow {
  flex-direction: row;
}

.flexcol {
  flex-direction: column;
}

.hello {
  flex-wrap: wrap;
}

.world {
  flex-wrap: nowrap;
}

(pass) css tests > .flexrow {
	flex-direction: row;
}

.flexcol {
	flex-direction: column;
}

.hello {
	flex-wrap: wrap;
}

.world {
	flex-wrap: nowrap;
} [4.46ms]
Output .foo {
  content: "+";
}

(pass) css tests > .foo {
  content: "\2b";
} [1.13ms]
Output div {
  --foo: 1 1 1 #0101011a, 2 2 2 #02020233;
  --bar: 1 1 1 #01010116, 2 2 2 #02020233;
}

(pass) css tests > custom property cases > div {
        --foo: 1 1 1 rgba(1, 1, 1, 0.1), 2 2 2 rgba(2, 2, 2, 0.2);
        --bar: 1 1 1 #01010116, 2 2 2 #02020233;
      } [1.08ms]
Output :root {
  --my-color: red;
  --my-bg: white;
  --my-font-size: 16px;
}

.element {
  color: var(--my-color);
  background-color: var(--my-bg);
  font-size: var(--my-font-size);
}

(pass) css tests > custom property cases > :root {
        --my-color: red;
        --my-bg: w
... (truncated)

release with fix: 67 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 643ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[0/5] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu)

  nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19)

�[1m�[92m   Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core)
�[1m�[92m   Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno)
�[1m�[92m   Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr)
�[1m�[92m   Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys)
�[1m�[92m   Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety)
�[1m�[92m   Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys)
�[1m�[92m   Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys)
�[1m�[92m   Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd)
�[1m�[92m   Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp)
�[1m�[92m   Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli)
�[1m�[92m   Compiling�[0m bun_output v0.0.0 (/workspace/bun/src/output)
�[1m�[92m   Compiling�[0m bun_clap 
... (truncated)

diff hotspot

src/css/values/calc.rs      | 56 +++++++++++++++++++--------------------------
 test/js/bun/css/css.test.ts | 35 ++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 33 deletions(-)

gate history · 2 passed · 0 rejected · iteration 0

evidence per changed file

file                         reads  edits  tests
src/css/values/calc.rs          12     10      0
test/js/bun/css/css.test.ts      3      7      0

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0e651d49-badf-4195-bd7e-9818be382c81

📥 Commits

Reviewing files that changed from the base of the PR and between 3a68d6d and 14b55f1.

📒 Files selected for processing (1)

• test/js/bun/css/css.test.ts

Included review availability: Your plan provides up to 5 included reviews per hour; 0 remain after this review.

---

Walkthrough

clamp() parsing now retains all arguments, compares compatible values, simplifies resolved expressions, and preserves unresolved expressions. CSS minification tests cover numeric, dimensional, percentage, angle, time, incompatible, and nested calc() cases.

Changes

CSS clamp simplification

Layer / File(s)	Summary
Clamp comparison and validation src/css/values/calc.rs, test/js/bun/css/css.test.ts	clamp() compares compatible values, applies the maximum before the minimum, and reduces expressions to a value or max(). Incomparable expressions remain as clamp(). Tests cover supported value types and nested expressions.

Possibly related PRs

• oven-sh/bun#39515: Both PRs update calc.rs to compare and simplify clamp() arguments with partial_cmp_args.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the main change: preserving the lower bound during CSS clamp() simplification.
Description check	✅ Passed	The description explains the problem, fix, scope, and verification results in substantial detail, although it does not use the exact template headings.

---

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

[claude]

LGTM — focused correctness fix for clamp() simplification, restructured around the definition max(min, min(center, max)).

What was reviewed:

• Traced the new CalcUnit::Clamp arm against every test row; each output matches the semantic definition and the asserted lightningcss 1.33.0 output.
• Confirmed partial_cmp_args preserves the old "only Calc::Value is comparable" behavior, so sums/numbers/nested functions still fall through to clamp()/max() unchanged.
• Checked the incomparable-min path emits max(min, center) with args in an order that serializes as valid CSS (matches the existing Min/Max single-arg return shape).
• No memory-safety or security surface: safe Rust, values moved by ownership, no new allocations beyond what the old code did.

Extended reasoning...

Overview

The PR rewrites the CalcUnit::Clamp arm of Calc::parse_with in src/css/values/calc.rs (~30 lines) and adds a small partial_cmp_args helper plus 22 minify_test rows in test/js/bun/css/css.test.ts. The old code had a typo-level bug (min = None where max = None was intended) that dropped the lower bound whenever center <= max, producing min(max, center) instead of applying the minimum. The new code applies the max first, then the min, folding to a single value when both bounds are comparable and to max(min, center) when only the max was, and leaves the clamp() intact when the max is incomparable — exactly what lightningcss does.

Security risks

None. This is pure CSS value simplification in safe Rust with no unsafe, no I/O, no external input beyond already-tokenized CSS. Worst case for a logic error here is a mis-minified declaration, not a vulnerability.

Level of scrutiny

Medium-low. It is a self-contained logic fix in one match arm with a clear mathematical spec (clamp(a,b,c) = max(a, min(b,c))) and a reference implementation (lightningcss) to diff against. I hand-traced each of the 22 test inputs through the new control flow and all produce the asserted output. The helper partial_cmp_args matches on (Calc::Value, Calc::Value) only, identical to the inlined check it replaces, so Calc::Number/Sum/Function arguments remain incomparable as before (relevant because #39515 will extend this to numbers separately). The removed (None, Some(max)) → Min(max, center) arm was reachable only via the bug, so nothing correct was lost.

Other factors

The test block covers the variant matrix well: lightningcss's own 11 rows, boundary equality, min-wins-over-smaller-max, incomparable min with center on either side of max, incomparable max with comparable min, and %/deg/s/nested-in-calc() value types. The PR states 16/22 fail on the released binary and all pass with the fix, and that the rest of css.test.ts plus test/bundler/css/ still pass. The change is strictly more correct and never less simplified than before (the one case where center > max now folds further to a single value when min is comparable). No outstanding review comments on the PR.

[claude]

LGTM — the restructured clamp() arm correctly implements max(min, min(center, max)) and every test row matches lightningcss.

What was reviewed:

• Traced the new logic against the spec definition for each comparability combination (both bounds comparable, only min incomparable, max incomparable) — all fold to the right shape.
• Confirmed partial_cmp_args preserves the old "only Calc::Value pairs compare" contract, so sums/numbers/nested functions still fall through to clamp()/max().
• Checked that the removed (None, Some(max)) → Min(max, center) arm was reachable only via the min = None bug, so no valid simplification is lost.
• Verified ownership: max is consumed into center and min/center into the result — no leaked boxes.

Extended reasoning...

Overview

Rewrites the CalcUnit::Clamp arm of Calc::parse_with in src/css/values/calc.rs (~30 lines net -18) and adds a 22-row clamp() simplification block to test/js/bun/css/css.test.ts. The old code compared center against max and, when center ≤ max, set min = None instead of max = None, dropping the lower bound and emitting min(max, center). The new code applies the max first (center becomes max when center > max), then compares against min: comparable → single value, incomparable → max(min, center), and keeps the full clamp() when center and max are incomparable. A small partial_cmp_args helper factors the two comparisons.

Security risks

None. Pure CSS constant-folding over already-parsed values; no untrusted-length arithmetic, no allocation sizing, no I/O.

Level of scrutiny

Medium — a correctness fix in the CSS minifier. I walked every branch of the new match against the definition clamp(a,b,c) = max(a, min(b,c)): when center vs max is incomparable the clamp is preserved (same as before and upstream); when center > max, center is replaced and then compared with min; when center ≤ max, max is safely dropped because min(center,max)=center. The None arm on the second comparison correctly emits max(min, center) since center is now the resolved inner min(). The removed Min(max, center) output was only reachable through the bug. partial_cmp_args matches only (Calc::Value, Calc::Value), exactly the guard the old inline code used, so Calc::Number, sums, and nested functions remain incomparable as before (the follow-up #39515 extends this).

Other factors

Tests are strong: eleven rows lifted from lightningcss's own suite plus targeted cases for equal bounds, min-wins-over-smaller-max (WPT rows), incomparable min with center below/above max, incomparable max, and %/deg/s/nested-in-calc(). The PR states 16/22 fail on the released binary and all pass with the fix, and that css.test.ts + test/bundler/css/ (1348 tests) still pass. No CODEOWNERS on this path, no outstanding reviewer comments, and the diff deletes more than it adds while removing the Option bookkeeping entirely.

[robobun]

^{Updated 11:54 PM PT - Aug 18th, 2026}

❌ @robobun, your commit 7e15aeb has 1 failures in Build #101088 (All Failures):

• test/js/node/tls/node-tls-server.test.ts - timeout on 🍎 any aarch64

---

🧪   To try this PR locally:

bunx bun-pr 39577

That installs a local version of the PR into your bun-39577 executable, so you can run:

bun-39577 --bun

[claude]

LGTM — the restructured clamp() arm correctly implements max(min, min(center, max)) and every test row matches lightningcss 1.33.0.

Checked: traced all 24 test rows through the new two-step comparison (max applied first, then min) — each produces the asserted output, including the min-wins-over-smaller-max WPT rows and the incomparable-bound cases that fall back to max() or the original clamp(). partial_cmp_args returning None for Calc::Number/Sum/Function preserves the prior non-comparison behavior for those variants. Confirmed arr2 is still live and returning a bare Calc::Value matches how the single-arg min()/max() arms already exit.

Extended reasoning...

Overview

The PR rewrites the CalcUnit::Clamp arm of Calc::parse_with in src/css/values/calc.rs (~30 lines net-negative) and adds a small partial_cmp_args helper plus 24 minify_test rows in test/js/bun/css/css.test.ts. The old code had a variable-swap bug (min = None where max = None was meant) that dropped the lower bound of clamp() and produced semantically wrong CSS output.

Correctness

The new logic follows the spec definition clamp(MIN, VAL, MAX) = max(MIN, min(VAL, MAX)) directly: apply the max cap first (replace center with max if center > max), then compare the result against min. I traced every test row through the three-way match on partial_cmp_args(&center, &min) — None yields max(min, center) (incomparable min preserved), Less yields min (min wins), Equal/Greater yield center. The early return when center and max are incomparable keeps the full clamp(), matching lightningcss and the pre-existing behavior. The Option<Calc<V>> bookkeeping is gone, which eliminates the (None, Some(max)) arm the bug depended on. partial_cmp_args deliberately only compares Calc::Value pairs, so Number/Sum/Function arguments stay unsimplified exactly as before (#39515 extends this to numbers).

Security risks

None. Pure CSS-value simplification over already-parsed Calc<V> trees; no I/O, no unsafe, no allocation-size arithmetic on untrusted input, and Rust ownership handles the moved min/center/max values.

Level of scrutiny

Medium — a minifier correctness bug that can silently change layout in user CSS, so the semantics matter, but the change is small, self-contained to one match arm, and every asserted output was cross-checked against lightningcss 1.33.0 (the reference implementation this code was ported from). The 24 tests cover the variant matrix well: comparable/incomparable bounds in each position, equal-to-bound edges, min > max ordering, mixed units (px/pt/in), and %/deg/s/nested-calc() value types.

Other factors

The PR description states 18/24 rows fail on the released binary and all 24 pass with the fix, and that test/bundler/css/ (1348 tests) and clippy are clean. No prior reviewer comments to address. The helper is placed alongside apply_op/apply_map following file conventions, and arr2 remains live via the Max fallback.

[claude]

LGTM — the restructured clamp() arm correctly implements max(min, min(center, max)) and every test row matches lightningcss 1.33.0.

What was reviewed:

• Traced the new center_vs_max / center_vs_min logic against each test row (center in-range, center above max, min-wins-over-smaller-max, equal-to-bound, incomparable min → max(), incomparable max → keep clamp()).
• Confirmed partial_cmp_args preserves the old comparison scope (only Calc::Value, so sums/numbers/nested functions stay incomparable) and that the removed MathFunction::Min output arm was only reachable via the bug.
• Checked that max is moved (not dropped) on the Greater branch and that the incomparable-max early return still carries all three arguments.

Extended reasoning...

Overview

This PR fixes a correctness bug in the CSS minifier's clamp() simplification (src/css/values/calc.rs, CalcUnit::Clamp arm of Calc::parse_with). On main, when the center is not above the maximum the code sets min = None where it means max = None, so clamp(10px, 5px, 20px) prints as min(20px,5px) (5px) instead of 10px. The fix drops the Option bookkeeping entirely and applies the definition clamp(a,b,c) = max(a, min(b,c)) directly: fold the max into the center first, then compare with the min. A new partial_cmp_args helper factors out the two-Calc::Value comparison. 24 new minify_test rows in css.test.ts (11 lifted from lightningcss's own suite, plus WPT rows and the #32290 repro) assert lightningcss 1.33.0's exact output; 18 of them fail on main and all pass with the fix, verified in the PR's mechgate on both ASAN-debug and release builds.

Security risks

None. This is a pure-computation transform in the CSS parser over already-parsed Calc<V> values. No untrusted length/index arithmetic, no allocation sizing, no I/O, no unsafe Rust. The worst a bug here can do is emit semantically-different CSS, which is exactly what's being fixed.

Level of scrutiny

Low-to-medium. The change is confined to ~25 lines in one match arm plus an 8-line private helper, in safe Rust with owned values (moves, no borrows outliving anything). The logic is directly verifiable against the CSS spec's definition of clamp(), and every asserted output is oracle'd against the upstream reference (lightningcss). The test matrix covers the boundary I'd ask about: center equal to a bound, min > max, incomparable min with center on either side of max, incomparable max, mixed units, and other value types (%, deg, s).

Other factors

• The removed (None, Some(max)) => MathFunction::Min(...) arm was only reachable via the bug (the PR description calls this out); grep confirms no other producer relied on that shape.
• partial_cmp_args matches only (Calc::Value, Calc::Value), identical to the old inline check, so Calc::Number and sums remain incomparable — no behavior widening (that's #39515's job, stacked on this).
• No CODEOWNERS entry for src/css/. No prior human review comments to address. CI build was triggered on the head commit.

[robobun]

Status: ready for review. The diff is 23 lines in the clamp() arm of src/css/values/calc.rs plus tests; 18 of the 24 new rows fail on the released binary and all pass with the fix.

CI: in both runs on this head state (builds 101070 and 101088) the only failing lane is macOS aarch64 on test/js/node/tls/node-tls-server.test.ts, a 90 s timeout in the "peer resets the connection behind unread data" block (a different case each time). It does not involve CSS and is reported separately. Build 101071 for #39515, whose binary contains this exact change, passed all 179 jobs, including that lane. Not pushing again for it.