Skip to content

feat(useSortedClasses): order variants in sort_v4 - #11249

Open
johncarmack1984 wants to merge 8 commits into
biomejs:mainfrom
johncarmack1984:feat/use-sorted-classes-variants
Open

feat(useSortedClasses): order variants in sort_v4#11249
johncarmack1984 wants to merge 8 commits into
biomejs:mainfrom
johncarmack1984:feat/use-sorted-classes-variants

Conversation

@johncarmack1984

@johncarmack1984 johncarmack1984 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Finishing touches to @jiwon79 's branch codex/use-sorted-classes-v4-variants from June.

This PR adds variant-prefixed classes (hover:, sm:/md:, dark:, data-[...]:, [&:hover]:, has-[...]:, compounds like md:hover:) to sort_v4. Previously, any variant made the candidate Unknown and floated it to the front in input order. Variant weight is the outermost sort key, places variantless utilities first; variants group in Tailwind's canonical order. This necessitates a two-phase sort: classify, then gather every variant in the list into the VariantGroups, then assign each a VariantBits, then sort. A variant's rank is relative to the whole list.

Followup to #10880, #11016, #11041, #11076, and #11120. As with those, AI tools were used to identify the next step in useSortedClasses nursery promotion and brainstorm idiomatic solutions based on the previous work from jiwon. This implementation was chosen for its performance, integration of previous feedback, adherence to repo conventions, and inclusion of new snapshots to test updated functionality.

No user-facing behavior change, therefore no changeset; sort_v4 is (for now) unwired.

Test Plan

  • 13 new variant fixtures
  • fixture expectations cross-checked against prettier-plugin-tailwindcss 0.8.0
  • 136 of 139 fixtures byte-identical with plugin (was 122/126 on main, the shadcn example now sorts correctly)

Docs

No rule, rustdoc, or website change (no user-facing behavior as sort_v4 is as-yet unwired).

Planned follow-up PRs:

  1. implement fixes for the final 4 divergences from prettier-plugin-tailwindcss
  2. wire sort_v4 into user-facing behavior (and the doc/changesets to accompany)

jiwon79 and others added 2 commits August 5, 2026 16:13
Wires jiwon79's variant engine into the current sort_v4 sort key: variant_bits sort outermost, resolved in a two-phase classify pass. Regenerates the v4 preset (variants + breakpoint/container value maps) and adds variant fixtures.
@changeset-bot

changeset-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 4fac033

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

✅ Organic activity

No automation signals detected in the analyzed events.

View full analysis →

This is an automated analysis by AgentScan

@github-actions github-actions Bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Aug 5, 2026
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Tailwind v4 variant metadata is extracted and generated as Rust maps for variants, breakpoints, and containers. The sorter parses static, functional, compound, and arbitrary variants. It groups recognised variants across the complete candidate list and compares variant weights before existing sort keys. Tests cover variant, responsive, compound, important, and arbitrary breakpoint ordering.

Possibly related PRs

Suggested labels: A-Parser, A-Tooling

Suggested reviewers: dyc3

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the variant-sorting changes, test coverage, current scope, and planned follow-up work.
Title check ✅ Passed The title clearly and concisely identifies the main change: adding variant ordering to sort_v4.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (8)
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs (2)

74-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider documenting the bits_for None contract.

bits_for returns None when a variant is missing from groups, and into_sort_key turns that into SortKey::Unknown. In the production path this cannot happen, because VariantGroups::new receives exactly the variants of every Known pending key. The fallback is therefore defensive, which is fine, but a reader has to reconstruct that reasoning.

One line of rustdoc on bits_for stating "returns None if a variant was not part of the list the groups were built from" would save that effort.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs`
around lines 74 - 110, Document the `VariantGroups::bits_for` contract with one
line of rustdoc stating that it returns `None` when any requested variant was
not included in the variants used to construct the groups.

208-216: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The Option<Option<VariantValue>> return can collapse to Option<VariantValue>.

The outer Option means "this shape is acceptable" and the inner one means "a value is present". Every arm returns either None or Some(Some(_)), so the inner None is unreachable. The double wrapping makes the call site at Line 170 harder to read than the logic warrants.

Proposed simplification
-fn variant_value_from_segments(segments: &[VariantSegment]) -> Option<Option<VariantValue>> {
+fn variant_value_from_segments(segments: &[VariantSegment]) -> Option<VariantValue> {
     match segments {
-        [] => None,
-        [VariantSegment::Named(value)] => Some(Some(VariantValue::Named(value.clone()))),
-        [VariantSegment::Arbitrary(value)] => Some(Some(VariantValue::Arbitrary(value.clone()))),
-        [VariantSegment::CssVariable] => None,
+        [VariantSegment::Named(value)] => Some(VariantValue::Named(value.clone())),
+        [VariantSegment::Arbitrary(value)] => Some(VariantValue::Arbitrary(value.clone())),
         _ => None,
     }
 }

The call site then becomes:

                 VariantKind::Functional => Some(VariantKey::Functional {
                     root,
-                    value: variant_value_from_segments(value_segments)?,
+                    value: Some(variant_value_from_segments(value_segments)?),
                 }),

VariantKey::Functional::value stays Option<VariantValue>, so the shape at rest is unchanged.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs`
around lines 208 - 216, Change variant_value_from_segments to return
Option<VariantValue>, using None for unsupported or valueless segment shapes and
Some for named or arbitrary values. Update its call site around
VariantKey::Functional value construction to use the simplified result directly,
while preserving the existing Option<VariantValue> field shape.
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/tailwind_preset_v4_types.rs (1)

118-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the compounds and compounds_with bitflag semantics.

kind, order, and compare explain themselves. compounds and compounds_with do not. They carry Tailwind's Compounds bitflag values (StyleRules = 1, AtRules = 2), so a bare 2 in the generated map is unreadable. The sibling UtilityEntry and FunctionalEntry in this file already document their non-obvious fields, so this would match.

Proposed rustdoc
 pub struct VariantEntry {
     pub kind: VariantKind,
     pub order: u16,
     pub compare: VariantCompare,
+    /// Tailwind's `Compounds` bitflags for what this variant produces:
+    /// `1` = style rules, `2` = at-rules, `0` = neither.
     pub compounds: u8,
+    /// Tailwind's `Compounds` bitflags for what a compound variant
+    /// accepts as its nested variant.
     pub compounds_with: u8,
 }

As per coding guidelines: "Use rustdoc documentation for documenting new features, rule changes, and rule/assist options in Rust code".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/tailwind_preset_v4_types.rs`
around lines 118 - 125, Update the Rustdoc for the VariantEntry fields compounds
and compounds_with to describe their Tailwind Compounds bitflag semantics,
including StyleRules = 1 and AtRules = 2, matching the documentation style used
by UtilityEntry and FunctionalEntry. Leave the other fields unchanged.

Source: Coding guidelines

packages/tailwindcss-config-analyzer/src/v4/render-rust.ts (1)

342-361: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Two small nits in the new renderers.

First, visibility. These two renderers emit pub static, but renderThemeKeys just above emits pub(super) static. The only consumer is sort_v4_variants, which is a descendant of use_sorted_classes, so pub(super) reaches it exactly as it reaches the theme-key sets. Matching the existing convention keeps the generated surface tight.

Second, shadowing. Inside renderThemeValueMap, the destructured { name, value } shadows the name parameter. The template happens to use the outer name outside the callback, so it works today. A future edit that needs the map name inside the callback would silently pick up the entry name instead.

Proposed fix
-	return `pub static VARIANTS: phf::Map<&'static str, VariantEntry> = phf_map! {
+	return `pub(super) static VARIANTS: phf::Map<&'static str, VariantEntry> = phf_map! {
 ${lines.join("\n")}
 };
 `;
 }
 
-function renderThemeValueMap(name: string, values: ThemeValue[]): string {
+function renderThemeValueMap(mapName: string, values: ThemeValue[]): string {
 	const lines = values.map(
 		({ name, value }) => `    ${rustString(name)} => ${rustString(value)},`,
 	);
-	return `pub static ${name}: phf::Map<&'static str, &'static str> = phf_map! {
+	return `pub(super) static ${mapName}: phf::Map<&'static str, &'static str> = phf_map! {
 ${lines.join("\n")}
 };
 `;
 }

The generated tailwind_preset_v4.rs needs regenerating to match.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/tailwindcss-config-analyzer/src/v4/render-rust.ts` around lines 342
- 361, Update renderVariants and renderThemeValueMap to emit pub(super) static,
matching renderThemeKeys and keeping the generated visibility limited to the
required consumer. In renderThemeValueMap, rename the destructured theme entry
name to avoid shadowing the name parameter while preserving the generated map
name and values. Regenerate tailwind_preset_v4.rs so it reflects the updated
visibility.
crates/biome_js_analyze/tests/sort_v4/cases.jsonc (2)

219-236: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add container-query fixtures. ContainerAsc and ContainerDesc currently have no coverage.

The 13 new fixtures cover a good spread: pseudo-class, arbitrary, data, aria, nth, peer, group, has, not, responsive, and compound. Nice work on the breadth.

One whole comparison mode is missing though. CONTAINER_VALUES and the ContainerAsc and ContainerDesc arms of compare_variant_values are new in this PR, and no fixture exercises them. Line 231 covers the breakpoint equivalents thoroughly, so the container side is the asymmetry.

Suggested additional fixtures
 	"md:flex sm:flex flex max-lg:flex max-sm:flex min-lg:flex",
+	"`@lg`:flex `@sm`:flex flex `@max-lg`:flex `@max-sm`:flex `@min-lg`:flex",
+	"`@min-`[30rem]:flex `@lg`:flex flex",
+	"dark:flex hover:flex flex",
 	"md:hover:flex hover:flex flex",

The dark: case is a smaller point, since the PR description lists dark mode as covered but no fixture names it.

As per coding guidelines: "parser, formatter, and lint-rule changes must cover their required valid, invalid, or snapshot cases".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/biome_js_analyze/tests/sort_v4/cases.jsonc` around lines 219 - 236,
Add container-query sorting fixtures to the variants section in cases.jsonc so
both ContainerAsc and ContainerDesc paths in compare_variant_values are
exercised, matching the existing responsive breakpoint coverage pattern. Include
representative container variants, including a dark: case if applicable, while
preserving the expected ordering format used by the surrounding fixtures.

Source: Coding guidelines


219-222: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The section header does not match its contents.

The header at Line 219 says "variants + important suffix". Lines 224 to 232 carry no ! at all, and Lines 233, 235, and 236 carry no variant. Only Line 234 is genuinely both.

Two headers would map cleanly onto the two groups, which helps the next person adding a case pick the right spot.

Also applies to: 233-236

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/biome_js_analyze/tests/sort_v4/cases.jsonc` around lines 219 - 222,
Split the misleading section header around the relevant test cases: use one
header for the variant-ordering cases and another for the important-suffix
cases, with a distinct header for the single case combining both if needed.
Ensure the headers accurately describe the `!` and variant usage in cases
224–236.
packages/tailwindcss-config-analyzer/src/v4/extract-variants.ts (1)

70-70: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Use a locale-independent comparator for the committed artefact.

localeCompare without an explicit locale follows the host ICU collation. This generator writes tailwind_preset_v4.rs, which is committed and diffed byte-wise. Two contributors on different locales can therefore produce two different files from the same Tailwind version.

The variant names include punctuation (*, **, @, @max, @min), and ICU treats punctuation quite unlike code-point order, so the tie-break at Line 70 is the most exposed.

Proposed fix
-		.sort((a, b) => a.order - b.order || a.name.localeCompare(b.name));
+		.sort((a, b) => a.order - b.order || (a.name < b.name ? -1 : a.name > b.name ? 1 : 0));
 	return [...ds.theme.namespace(namespace)]
 		.map(([name, value]) => ({ name, value }))
-		.sort((a, b) => a.name.localeCompare(b.name));
+		.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0));

Also applies to: 79-82

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/tailwindcss-config-analyzer/src/v4/extract-variants.ts` at line 70,
Replace the locale-dependent tie-break comparator in the variant sorting logic
with a deterministic locale-independent comparison, using code-point or
equivalent stable ordering for variant names. Apply the same change to the
additional sorting block around the related variant ordering logic, while
preserving the primary order comparison.
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4.rs (1)

615-627: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The classify helper cannot express variant-against-variant ordering.

classify builds VariantGroups from one candidate, so each call gets its own group map. Any single variant lands in group 0 and produces bits [1].

The consequence: compare(&classify("sm:flex"), &classify("md:flex")) compares equal bits, then falls through to signature and name. The assertion would pass or fail for reasons unrelated to variant ordering. The test at Line 966 works only because flex has no variants at all, so the length check in cmp_numeric decides it.

So the variant ordering that this PR is about is currently covered only by the cases.jsonc snapshots. A helper that shares one group map across several inputs would make the comparator directly testable, including the breakpoint ordering.

Sketch of a list-wide helper
/// Classify a whole class list against one shared `VariantGroups`,
/// the way `sort_class_list` does.
fn classify_all(input: &str) -> Vec<SortKey> {
    let parsed = parse_tailwind(input);
    let pending: Vec<PendingSortKey> = parsed
        .tree()
        .candidates()
        .iter()
        .map(|candidate| PendingSortKey::from_candidate(&candidate))
        .collect();
    let groups = VariantGroups::new(pending.iter().flat_map(|key| match key {
        PendingSortKey::Known { variants, .. } => variants.as_slice(),
        PendingSortKey::Unknown => &[],
    }));
    pending
        .into_iter()
        .map(|key| key.into_sort_key(&groups))
        .collect()
}

A test could then assert directly that sm:flex precedes md:flex, and that max-lg:flex precedes max-sm:flex.

As per coding guidelines: "All code changes must include appropriate tests".

Also applies to: 958-969

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4.rs`
around lines 615 - 627, Update the test helpers around classify and the affected
assertions so variant ordering is evaluated with one shared VariantGroups across
the entire class list, rather than constructing groups per candidate. Add a
classify_all helper that parses all candidates, collects their variants, creates
shared groups, and converts each PendingSortKey; use it to directly test
breakpoint ordering such as sm:flex before md:flex and max-lg:flex before
max-sm:flex, while preserving existing plain-utility comparisons.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs`:
- Around line 267-314: Update variant_compare_value, used by
compare_variant_values, to return a fixed extreme numeric rank instead of None
when a breakpoint or container value cannot be parsed, while preserving normal
parsed-value ordering and the existing Default behavior. Ensure the resulting
comparator is total and cannot create input-order-dependent cycles, and add a
regression case covering mixed known, parseable arbitrary, and unparseable
breakpoint/container variants such as sm, min-[50rem], and min-[foo].

In `@packages/tailwindcss-config-analyzer/src/v4/extract-variants.ts`:
- Around line 52-55: Add a runtime shape guard around the design system loaded
by __unstable__loadDesignSystem before exporting variants, verifying
variants.variants, sort order, and compoundsWith are present with the expected
structure. If validation fails, use the existing fallback path instead of
accessing the private internals; avoid relying on the DesignSystemWithVariants
cast to validate renamed or missing fields.

---

Nitpick comments:
In
`@crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs`:
- Around line 74-110: Document the `VariantGroups::bits_for` contract with one
line of rustdoc stating that it returns `None` when any requested variant was
not included in the variants used to construct the groups.
- Around line 208-216: Change variant_value_from_segments to return
Option<VariantValue>, using None for unsupported or valueless segment shapes and
Some for named or arbitrary values. Update its call site around
VariantKey::Functional value construction to use the simplified result directly,
while preserving the existing Option<VariantValue> field shape.

In `@crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4.rs`:
- Around line 615-627: Update the test helpers around classify and the affected
assertions so variant ordering is evaluated with one shared VariantGroups across
the entire class list, rather than constructing groups per candidate. Add a
classify_all helper that parses all candidates, collects their variants, creates
shared groups, and converts each PendingSortKey; use it to directly test
breakpoint ordering such as sm:flex before md:flex and max-lg:flex before
max-sm:flex, while preserving existing plain-utility comparisons.

In
`@crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/tailwind_preset_v4_types.rs`:
- Around line 118-125: Update the Rustdoc for the VariantEntry fields compounds
and compounds_with to describe their Tailwind Compounds bitflag semantics,
including StyleRules = 1 and AtRules = 2, matching the documentation style used
by UtilityEntry and FunctionalEntry. Leave the other fields unchanged.

In `@crates/biome_js_analyze/tests/sort_v4/cases.jsonc`:
- Around line 219-236: Add container-query sorting fixtures to the variants
section in cases.jsonc so both ContainerAsc and ContainerDesc paths in
compare_variant_values are exercised, matching the existing responsive
breakpoint coverage pattern. Include representative container variants,
including a dark: case if applicable, while preserving the expected ordering
format used by the surrounding fixtures.
- Around line 219-222: Split the misleading section header around the relevant
test cases: use one header for the variant-ordering cases and another for the
important-suffix cases, with a distinct header for the single case combining
both if needed. Ensure the headers accurately describe the `!` and variant usage
in cases 224–236.

In `@packages/tailwindcss-config-analyzer/src/v4/extract-variants.ts`:
- Line 70: Replace the locale-dependent tie-break comparator in the variant
sorting logic with a deterministic locale-independent comparison, using
code-point or equivalent stable ordering for variant names. Apply the same
change to the additional sorting block around the related variant ordering
logic, while preserving the primary order comparison.

In `@packages/tailwindcss-config-analyzer/src/v4/render-rust.ts`:
- Around line 342-361: Update renderVariants and renderThemeValueMap to emit
pub(super) static, matching renderThemeKeys and keeping the generated visibility
limited to the required consumer. In renderThemeValueMap, rename the
destructured theme entry name to avoid shadowing the name parameter while
preserving the generated map name and values. Regenerate tailwind_preset_v4.rs
so it reflects the updated visibility.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 134adbc0-c6b5-4f9a-8394-8d25699f4091

📥 Commits

Reviewing files that changed from the base of the PR and between bd0b68d and fff42ba.

⛔ Files ignored due to path filters (1)
  • crates/biome_js_analyze/tests/sort_v4/cases.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (9)
  • crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs
  • crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4.rs
  • crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs
  • crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/tailwind_preset_v4.rs
  • crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/tailwind_preset_v4_types.rs
  • crates/biome_js_analyze/tests/sort_v4/cases.jsonc
  • packages/tailwindcss-config-analyzer/src/v4/extract-variants.ts
  • packages/tailwindcss-config-analyzer/src/v4/generate-tailwind-preset-v4.ts
  • packages/tailwindcss-config-analyzer/src/v4/render-rust.ts

Comment thread packages/tailwindcss-config-analyzer/src/v4/extract-variants.ts
@codspeed-hq

codspeed-hq Bot commented Aug 5, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 15.08%

❌ 1 regressed benchmark
✅ 61 untouched benchmarks
⏩ 222 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
js_analyzer[index_3894593175024091846.js] 73.4 ms 86.5 ms -15.08%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing johncarmack1984:feat/use-sorted-classes-variants (a9f8c9d) with main (bd0b68d)

Open in CodSpeed

Footnotes

  1. 222 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

- make the breakpoint/container value comparator total, so an unparseable arbitrary value ranks last instead of comparing equal to everything (which was non-transitive)
- sort the variant codegen by code point rather than locale, keeping the generated preset deterministic across machines
- limit the generated variant tables to pub(super)
- collapse a redundant Option and document the Compounds bitflags
- add dark-mode and variant-vs-variant test coverage
@johncarmack1984

Copy link
Copy Markdown
Contributor Author

@codspeedbot this seems to be a layout artifact, no executed code was changed in this PR

@johncarmack1984

Copy link
Copy Markdown
Contributor Author

@codspeedbot done

@dyc3 dyc3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand the sorting logic going on here. can you explain it?

Comment thread crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs Outdated
…strings

- Rename VariantBits to VariantWeight; replace cmp_numeric with Ord/PartialOrd
- Store registered variant roots as &'static str and named values as TokenText
- Keep the variant-key grouping comparator a free function, documented
- Add module and type docs explaining the two-pass variant weighting

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs (1)

193-198: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the change-history wording from this comment.

The phrase “kept for now to preserve behavior” describes history, not an invariant. Remove it, or replace it with the semantic reason that requires the early lookup.

As per coding guidelines, “For developer-facing comments, explain behavior, invariants, panics, module rationale, or non-obvious rationale; do not narrate change history or address reviewers.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs`
around lines 193 - 198, Update the comment above raw in the expression lookup to
remove the change-history wording about being “kept for now to preserve
behavior”; retain only the semantic explanation for why the early check remains,
or remove that sentence if no invariant or non-obvious rationale applies.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs`:
- Around line 193-198: Update the comment above raw in the expression lookup to
remove the change-history wording about being “kept for now to preserve
behavior”; retain only the semantic explanation for why the early check remains,
or remove that sentence if no invariant or non-obvious rationale applies.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 68389785-7cb6-4d76-b3f4-76fe3774e697

📥 Commits

Reviewing files that changed from the base of the PR and between 80d652b and 9718125.

📒 Files selected for processing (2)
  • crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4.rs
  • crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4_variants.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/biome_js_analyze/src/lint/nursery/use_sorted_classes/sort_v4.rs

Replace the compare_variant_keys free function with an Ord/PartialOrd
impl on VariantKey, and simplify VariantGroups::new to variants.sort().
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@johncarmack1984
johncarmack1984 requested a review from dyc3 August 6, 2026 15:42
@johncarmack1984

Copy link
Copy Markdown
Contributor Author

Fine time for GitHub Actions to have an outage ^_^ afaik everything but codspeed should pass, an 'acknowledge' on codspeed and a re-run post-incident should turn the checks green, meanwhile happy to address any other feedback on the changes :)

…ions

Store the variant weight inline in a SmallVec so the common case does
not heap-allocate, and rank variants with a sorted Vec + binary search
instead of building a HashMap per class list.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Linter Area: linter L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants