[robobun]

Problem

Angle::eql in angle.zig compares via toDegrees(), and Zig's css.implementEql recursively calls .eql() on nested fields, so any container holding an Angle (Transform, Rotate, LineDirection, ConicGradient, FontStyle, ...) treats Deg(180) and Turn(0.5) as equal.

The Rust port kept that semantics in impl CssEql for Angle, but the enum also #[derive(PartialEq)], which is structural (Deg(180.0) != Turn(0.5)). The containers use #[derive(PartialEq)] on themselves and then css_eql_partialeq!(...) to bridge CssEql to ==, so a nested Angle is compared structurally. The rule merger / declaration deduper compares declaration values through these paths, so adjacent rules whose values contain semantically equal angles in different units stopped merging under --minify. Output is still correct CSS, just larger than the Zig build produced.

.a{transform:rotateZ(180deg)}.b{transform:rotateZ(0.5turn)}
  before: .a{transform:rotate(180deg)}.b{transform:rotate(.5turn)}
  after:  .a,.b{transform:rotate(180deg)}

Fix

Replace the derived PartialEq on Angle with a manual impl that compares to_degrees(), matching angle.zig:200. This propagates through every container's derived PartialEq (and therefore their CssEql) without touching any of them. The inherent Angle::eql and the CssEql impl now forward to == so there is a single source of truth.

Tests

Added minify_test cases in test/js/bun/css/css.test.ts covering transform (rotateZ, skew), the rotate property, linear-gradient direction, conic-gradient from-angle, and font-style: oblique, plus a negative case (same unit, different value stays unmerged). All fail on the unfixed build and pass with this change; the existing 1093 css tests continue to pass.

[coderabbitai]

Warning

Review limit reached

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

More reviews will be available in 3 minutes and 22 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

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.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 926843c2-28ed-413b-b2c6-60873b2f0275

📥 Commits

Reviewing files that changed from the base of the PR and between c3b4720 and f31c0ee.

📒 Files selected for processing (1)

• src/css/values/angle.rs

Walkthrough

PartialEq is removed from Angle's derive list and replaced with a custom impl PartialEq for Angle that compares angles by their degree value, making cross-unit equality (e.g., 90deg == 0.25turn) structural. Angle::eql and CssEql::eql are updated to delegate to this new PartialEq. Tests are added to verify CSS rule merging for semantically equivalent angle units in transform, rotate, skew, and gradient properties.

Changes

Angle Semantic Equality and CSS Rule Merging

Layer / File(s)	Summary
Custom PartialEq and CssEql for Angle src/css/values/angle.rs	Removes derived PartialEq from Angle; adds a manual impl PartialEq for Angle that compares via to_degrees(); updates Angle::eql and CssEql::eql to delegate to *self == *other.
Minify tests for cross-unit angle merging test/js/bun/css/css.test.ts	Adds minify_test cases asserting that adjacent CSS rules with transform/rotate/skew/gradient angle values in different but semantically equal units are merged, and that rules with equal units but different values are not.

Possibly related PRs

• oven-sh/bun#31920: Directly related — that PR implements the CSS minifier's selector/rule merging and compatibility logic that this PR's Angle semantic equality and cross-unit merge tests exercise.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: implementing semantic equality comparison for Angle by degrees in PartialEq instead of structural comparison.
Description check	✅ Passed	The description comprehensively covers the problem (structural vs semantic equality mismatch), the fix (custom PartialEq implementation), and testing (multiple minify_test cases), fully addressing the template requirements.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[robobun]

^{Updated 10:09 AM PT - Jun 15th, 2026}

❌ @robobun, your commit f31c0ee has 2 failures in Build #62552 (All Failures):

• test/cli/install/bun-install-lifecycle-scripts.test.ts - code 1 on 🍎 14 aarch64
• test/cli/install/GHSA-pfwx-36v6-832x.test.ts - code 1 on 🍎 14 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 32307

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

bun-32307 --bun

[robobun]

This also covers the font-style: oblique default-angle comparison reported separately: FontStyle::to_css/is_compatible in src/css/properties/font.rs use angle != default_oblique_angle(), and the @font-face variant in src/css/rules/font_face.rs goes through Size2D::<Angle>::eql/val_eql which bottom out in ==. A few extra cases that exercise those paths directly (fail on main, pass with this change):

minify_test(".foo { font-style: oblique 15.5555556grad }", ".foo{font-style:oblique}");
minify_test("@font-face { font-style: oblique 15.5555556grad }", "@font-face{font-style:oblique}");
minify_test("@font-face { font-style: oblique 18deg 20grad }", "@font-face{font-style:oblique 18deg}");
prefix_test(
  ".foo { font-style: oblique; font-style: oblique 15.5555556grad; }",
  indoc`.foo { font-style: oblique; }`,
  { firefox: 50 << 16 },
);

Happy to push these onto this branch if useful.

[claude]

LGTM — the CssHash divergence is now documented, thanks.

Extended reasoning...

Overview

This PR replaces #[derive(PartialEq)] on the Angle enum in src/css/values/angle.rs with a manual impl that compares via to_degrees(), so Deg(180) == Turn(0.5) == Grad(200). The inherent Angle::eql and CssEql for Angle now both forward to ==, giving a single source of truth. Seven minify_test cases are added in test/js/bun/css/css.test.ts covering rule-merging across transform/rotate/skew/gradient/font-style plus a negative case. A follow-up commit (2a22263) added a comment documenting that CssHash remains intentionally structural to match angle.zig.

Security risks

None. This is a pure value-equality change inside the CSS minifier; no auth, crypto, filesystem, network, or untrusted-input parsing surface is touched. Worst-case failure mode is suboptimal or over-merged CSS output, not a security issue.

Level of scrutiny

Low-to-moderate. The change is small (~15 lines of logic + comments + tests), restores documented parity with the Zig reference (angle.zig:200), and the previous CssEql impl already had exactly these semantics — this just propagates them to containers via derived PartialEq. The blast radius is confined to CSS minification quality, and the existing 1093 css tests reportedly continue to pass.

Other factors

• My prior review raised one minor nit (structural CssHash vs semantic PartialEq); the author addressed it with the documenting comment I suggested, and the thread is resolved.
• The bug-hunting system found no issues.
• The CI failures on the build bot are infrastructure-related (Windows agent creation failures, musl LTO data-layout linking) and unrelated to this diff; the latest commit only adds comments.
• No CODEOWNERS entry covers src/css.
• Angle does not implement Eq or core::hash::Hash, so no std Hash/Eq contract is at risk.

[claude]

LGTM — the comment addressing the CssHash divergence resolves my earlier nit.

Extended reasoning...

Overview

This PR replaces the derived (structural) PartialEq on Angle in src/css/values/angle.rs with a manual impl that compares via to_degrees(), restoring parity with angle.zig. The inherent Angle::eql and CssEql impl now both forward to == so there is a single source of truth. Seven minify_test cases are added to test/js/bun/css/css.test.ts covering rule-merging across transform, rotate, linear-gradient, conic-gradient, and font-style: oblique, plus a negative case.

Security risks

None. This is a pure value-equality change in the CSS minifier. There is no parsing, I/O, auth, or memory-safety surface here; the only behavioral effect is that semantically-equal angles in different units now compare equal, enabling additional (correct) rule merging. Worst-case failure mode is suboptimal minification, not incorrect output.

Level of scrutiny

Low–medium. The diff is ~15 lines of Rust plus tests, mechanically aligns the Rust port with the Zig reference (angle.zig:200), and is well-covered by new tests that fail before / pass after. I verified there is no Eq impl on Angle (so no std Hash/Eq contract is at stake), and a grep for Angle::Variant(...) usages shows only pattern-matches and constructors — nothing that depended on the old structural ==.

Other factors

In my prior review I flagged that #[derive(CssHash)] remains structural while PartialEq/CssEql are now degree-based. The author addressed this in 2a22263 by adding a comment documenting that the divergence is intentional (matches angle.zig) and that no consumer keys a CssHash map on an Angle-containing value with CssEql as equality. That thread is resolved. The robobun comment suggesting additional font-style: oblique / @font-face test cases is non-blocking — the existing tests already exercise the fix adequately. CI was retriggered in f31c0ee; the code change itself is correct independent of any infra flakes.

[robobun]

All 280 test jobs that ran passed. The remaining red is the darwin test lanes (darwin-14-aarch64, darwin-14-x64, darwin-26-aarch64) expiring without an agent on both build 62452 and 62552, plus a handful of known flakes annotated as flaky (serve-body-leak on asan, spawn-stdin EPIPE on windows, http-emit-close timeout on windows, 30205 on asan), none touching CSS. The diff is ready; needs a maintainer to merge past the darwin agent-pool outage.

[robobun]

Stale PR review: keep open, rework.

The change is still wanted. Main still derives structural PartialEq on Angle (src/css/values/angle.rs:21) while impl CssEql for Angle at :218-223 compares to_degrees(), so the minifier is inconsistent today: bun build --minify merges .c{--x:180deg} .d{--x:0.5turn} and .e{filter:hue-rotate(180deg)} .f{filter:hue-rotate(0.5turn)} but leaves .a{transform:rotateZ(180deg)} .b{transform:rotateZ(0.5turn)} as two rules. Upstream lightningcss hand-implements impl PartialEq for Angle as self.to_degrees() == other.to_degrees() (src/values/angle.rs:224-228), which is what this PR restores. The CssHash concern does not apply: StyleRule::hash_key (src/css/rules/style.rs:38-52) hashes selectors and property-id tags only, not values.

The diff cannot merge as posted because one hunk edits the inherent Angle::eql(self, rhs: Angle), which #39420 (merged 2026-08-17) removed as dead code. Wanted shape: the same change rebased onto main with that hunk dropped. Keep the manual PartialEq, forward CssEql to ==, leave the derived CssHash alone, keep the seven minify_test cases, and trim the multi-line hash comment to one line. Upstream's .zero-rad / .zero-turn linear-gradient merge case from lightningcss test_gradients would be a good parity anchor to add.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-06-15, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.