Open
Conversation
🦋 Changeset detectedLatest commit: 47e2bb4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
A-CLI
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
ematipico
force-pushed
the
perf/code-actions-filter
branch
from
0cb0634 to
d421def
Compare
ematipico
force-pushed
the
perf/code-actions-filter
branch
from
d117401 to
bc18a2e
Compare
ematipico
force-pushed
the
perf/code-actions-filter
branch
from
bc18a2e to
2c76147
Compare
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReworks analyzer actions and diagnostics flow: adds ActionFilter and ActionMetadata and changes AnalyzerSignal::actions to accept a filter plus actions_metadata for cheap enumeration; threads diagnostic controls (max_diagnostics, diagnostic_level, enforce_assist) through crawler/handler/process_file/workspace and surfaces aggregated errors/warnings/infos; makes code-action computation conditional ( Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_service/src/workspace/server.rs (1)
1757-1839:⚠️ Potential issue | 🟠 Major
max_diagnosticsis applied per pass, not per request.Right now each embedded lint run can consume the full cap, and the parse-only path does not apply the cap at all. That means a low
max_diagnosticscan still return a large result set. Please apply one shared remaining budget across root + embedded passes, and enforce it in the parse-only branch too.Also applies to: 1841-1865
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_service/src/workspace/server.rs` around lines 1757 - 1839, The LSP currently applies max_diagnostics separately per lint pass (root and each embedded snippet) and ignores it in the parse-only path, allowing the total returned diagnostics to exceed the requested cap; update the logic around the lint invocation (the lint function and LintParams usage in this block and the parse-only branch) to allocate a single shared remaining_budget integer (starting from max_diagnostics) that is decremented by the number of diagnostics emitted after each call, pass the remaining_budget into each LintParams (replace max_diagnostics with remaining_budget) and stop calling lint or accumulating diagnostics once remaining_budget reaches zero; ensure the parse-only branch also consumes and respects remaining_budget the same way so root+embedded runs share one cap.
🧹 Nitpick comments (4)
crates/biome_js_analyze/tests/spec_tests.rs (1)
224-225: Use the narrowest filter the harness needs.
check_action_typealready tells this test whether it wants fixes or suppressions, soActionFilter::all()is asking for the whole buffet: slower, and it does not really validate the caller-side filtering this PR is introducing.As per coding guidelines, "All code changes MUST include appropriate tests: lint rules require snapshot tests ... and bug fixes require tests that reproduce and validate the fix."
Also applies to: 254-255
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/tests/spec_tests.rs` around lines 224 - 225, The test iterates using event.actions(ActionFilter::all()) which requests every action and undermines the new caller-side filtering; update the loop to use the narrowest ActionFilter based on check_action_type (e.g., use ActionFilter::suppressions() when check_action_type.is_suppression() is true, otherwise use ActionFilter::fixes()), and make the same change at the other occurrence referenced (around lines 254-255) so the harness only fetches the needed actions and validates the new filtering behavior.xtask/rules_check/src/lib.rs (1)
403-407: Consider removing redundant suppression check.Since
ActionFilter::rule_fix()should already exclude suppression actions, the!action.is_suppression()check on line 404 appears redundant. The JS handler at lines 356-358 doesn't include this check after the same refactor.Suggested simplification
if let Some(mut diag) = signal.diagnostic() { for action in signal.actions(ActionFilter::rule_fix()) { - if !action.is_suppression() { - diag = diag.add_code_suggestion(action.into()); - } + diag = diag.add_code_suggestion(action.into()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xtask/rules_check/src/lib.rs` around lines 403 - 407, The loop filters actions using ActionFilter::rule_fix() but then redundantly checks !action.is_suppression() before calling diag.add_code_suggestion; remove the suppression check to simplify the logic. Update the block iterating signal.actions(ActionFilter::rule_fix()) so it unconditionally calls diag.add_code_suggestion(action.into()) for each action, relying on ActionFilter::rule_fix() to already exclude suppression actions (references: signal.actions, ActionFilter::rule_fix(), action.is_suppression(), diag.add_code_suggestion).crates/biome_cli/src/runner/impls/process_file/format.rs (1)
119-119: Consider addressing or clarifying the "probably to revisit" note.The comment
// NOTE: probably to revisitonpull_code_actions: falsesuggests uncertainty. If this is intentional for format-only mode, a brief clarification would help future maintainers. If it genuinely needs revisiting, consider opening an issue to track it.Would you like me to open an issue to track this for later review?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_cli/src/runner/impls/process_file/format.rs` at line 119, Clarify the ambiguous comment on the pull_code_actions flag: locate the configuration/struct field named pull_code_actions (in format.rs) and either replace `// NOTE: probably to revisit` with a concise rationale (e.g., `// intentionally disabled in format-only mode to avoid network calls`) or change it to a TODO linking an issue/issue number (e.g., `// TODO: revisit behavior - see issue `#NNN``) and if this change represents future work, open an issue and reference it in the TODO comment so maintainers can track it.crates/biome_analyze/src/signals.rs (1)
39-55: Minor: filter constructors can be simplified.The mutable pattern is verbose when a single flag is inserted. Consider a more concise form:
♻️ Suggested simplification
pub fn rule_fix() -> Self { - let mut filter = ActionKind::empty(); - filter.insert(ActionKind::RuleFix); - Self(filter) + Self(ActionKind::RuleFix.into()) } pub fn inline_suppression() -> Self { - let mut filter = ActionKind::empty(); - filter.insert(ActionKind::InlineSuppression); - Self(filter) + Self(ActionKind::InlineSuppression.into()) } pub fn toplevel_suppression() -> Self { - let mut filter = ActionKind::empty(); - filter.insert(ActionKind::ToplevelSuppression); - Self(filter) + Self(ActionKind::ToplevelSuppression.into()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_analyze/src/signals.rs` around lines 39 - 55, The three constructor functions (rule_fix, inline_suppression, toplevel_suppression) use a verbose mutable pattern creating ActionKind::empty() and then insert(...); simplify each by returning Self(ActionKind::RuleFix), Self(ActionKind::InlineSuppression), and Self(ActionKind::ToplevelSuppression) respectively (i.e., drop the mutable filter and insert calls and construct the bitflag directly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/gentle-comics-win.md:
- Line 5: The changeset line currently reads "Fixed
[`#191`](https://github.com/biomejs/biome-zed/issues/191). Improved the
performance..." and needs the required bug-fix prefix format; update that line
so it uses the exact format "Fixed
[`#191`](https://github.com/biomejs/biome-zed/issues/191): Improved the
performance of how the Biome Language Server pulls code actions and diagnostics"
(replace the period after the issue link with a colon and keep the user-facing
description), ensuring the single-line changeset description matches the "Fixed
[`#NUMBER`](issue link): ..." guideline.
In @.changeset/smart-radios-end.md:
- Line 5: The release note line contains typos: replace the misspelled command
name `linet` with `lint` and correct `caled` to `called` so the sentence reads
something like "Improved the performance of the commands `lint` and `check` when
they are called with `--write`"; update the string in the .changeset entry that
contains `linet`, `check`, and `--write` accordingly.
In `@crates/biome_cli/src/runner/impls/collectors/default.rs`:
- Around line 223-229: The aggregate counters
(self.errors/self.warnings/self.infos) in ProcessLint::process_signal are being
incremented for diagnostics that DefaultCollector::should_skip_diagnostic would
later filter out due to the verbose flag; update ProcessLint::process_signal to
apply the same verbose-filtering logic used by
DefaultCollector::should_skip_diagnostic before adding to the counters (or defer
incrementing until after should_skip_diagnostic is evaluated) so that only
retained diagnostics contribute to the workspace totals.
In `@crates/biome_lsp/src/handlers/analysis.rs`:
- Around line 479-489: The error uses WorkspaceError::not_found("") with empty
messages, which provides no context; update the two occurrences to include
descriptive messages: when mapping action.suggestion to `suggestion` (in the
action.suggestion.ok_or_else(...) call) use a message like "suggestion missing
for action {action.id or action.name}" or similar identifying info, and when
mapping errors from `utils::text_edit(...).map_err(...)` include context such as
"failed to compute text edits for suggestion on doc {doc.uri or doc.id} at
position {offset or position_encoding}" so the LspError created from
WorkspaceError::not_found(...) contains meaningful debugging details.
- Around line 428-433: The current use of Box::leak in the AnalyzerSelector
construction (Box::leak(resolve_data.group.into_boxed_str()),
Box::leak(resolve_data.rule.into_boxed_str())) leaks memory on every
codeAction/resolve; replace this by avoiding leaking owned strings — either
change RuleSelector to accept owned String (or Cow<'static, str>) and construct
AnalyzerSelector::from(RuleSelector::Rule(resolve_data.group,
resolve_data.rule)), or implement a reuse/cache for selectors (e.g., a global
interner or HashMap keyed by (group,rule) that returns &'static str or a shared
reference) and remove Box::leak usage; update all call sites of
RuleSelector/AnalyzerSelector accordingly (look for enum RuleSelector and
AnalyzerSelector::from) so no per-request leaks occur.
- Around line 366-375: CodeActionResolveData derives serde traits but its range
field uses TextRange which lacks Serialize/Deserialize; add serde support by
updating the TextRange definition (in biome_text_size::range.rs) to derive
serde::Serialize and serde::Deserialize (or implement a small serde wrapper type
around TextRange and replace the field type in CodeActionResolveData with that
wrapper) so serde_json::from_value() can successfully (de)serialize
CodeActionResolveData; ensure the symbol names are TextRange and
CodeActionResolveData when making the change.
In `@crates/biome_lsp/src/session.rs`:
- Around line 413-429: The current early-return when diagnostics_in_flight
already contains biome_path can drop the newest diagnostics; instead implement
coalescing by adding a pending/dirty marker when insert fails and arranging a
single re-run after the in-flight task completes. Concretely, introduce a
diagnostics_pending (or mark) keyed by biome_path; when the insert into
diagnostics_in_flight in this block fails, set
diagnostics_pending.insert(biome_path.clone()) and return Ok(()). In the
completion path (after calling update_diagnostics_for_document_inner and
before/after diagnostics_in_flight.remove(&biome_path)), check
diagnostics_pending for that biome_path: if present, clear the pending marker
and loop/invoke update_diagnostics_for_document_inner again (or re-queue a
single follow-up run) so the latest version is processed. Ensure all accesses
use the same locking pattern as diagnostics_in_flight to avoid races.
- Around line 636-644: The supports_code_action_resolve check currently only
verifies resolveSupport exists; change it to ensure the client explicitly
supports resolving the "edit" property by using is_some_and to inspect
resolve_support.properties for a "edit" string. Update the function
supports_code_action_resolve to traverse initialize_params ->
client_capabilities.text_document -> code_action -> resolve_support and return
true only if resolve_support.properties contains "edit" (use is_some_and with an
iterator/any check on the properties list) so deferred edit computation is only
used when the client advertises "edit".
In `@crates/biome_lsp/src/utils.rs`:
- Around line 167-223: The constructed CodeAction currently uses synthetic
title/kind/is_preferred and loses real metadata for unresolved actions; update
the title/kind/is_preferred computation inside the block that builds the
CodeAction (the match on action.rule_name and the title generation logic) to
derive the same metadata you would on resolve rather than relying on LSP
resolve: (1) determine suppression vs rule assists by checking the
SUPPRESSION_INLINE_ACTION_CATEGORY and SUPPRESSION_TOP_LEVEL_ACTION_CATEGORY
matches first (so suppression actions with Other(InlineSuppression) get the
correct "assist" vs "lint" prefix) and compute message_kind and rule_category
from action.rule_name and those SUPPRESSION_* categories; (2) set is_preferred
on the returned lsp::CodeAction from action.is_preferred (or a sensible default)
and ensure kind is produced from the real kind variable used for resolved
actions; and (3) ensure any applicability-based labels (Apply safe/unsafe fix)
are computed here exactly as in the resolve path so unresolved actions carry
full metadata when returned in lsp::CodeAction (update the title, kind, and
is_preferred fields in the lsp::CodeAction construction accordingly).
In `@crates/biome_rowan/src/ast/batch.rs`:
- Around line 112-121: Add unit tests for Batch::merge: create batches via the
same tree root and assert non-overlapping changes combine (apply Batch::merge
then Batch::commit yields both edits), test conflicting edits on the same
parent/slot to verify last-write-wins behavior (apply two batches with different
values for the same slot, merge and commit, assert final value equals the later
change), and add an invariant-breach test (e.g., attempt merging batches that
reference different tree roots or invalid node IDs) to assert the expected
error/panic behavior. Use the Batch type and its changes field to construct the
cases and call Batch::merge and Batch::commit to validate outcomes. Ensure tests
cover happy path, conflict resolution, and the invariant breach.
- Around line 119-121: The merge method currently extends self.changes with
other.changes without enforcing the same-root invariant; update the merge
function to first verify that both batches originate from the same tree (e.g.,
compare whatever root/tree identifier is stored on the batch struct or derive it
from changes) and panic or return an error if they differ; only when the root
identifiers match should you call self.changes.extend(other.changes) so you
never create an invalid merged state (refer to the merge method and the changes
field to locate where to add the guard and the comparison).
In `@crates/biome_service/src/file_handlers/mod.rs`:
- Around line 764-780: The aggregate counts (errors/warnings/infos) in the
LintResults are missing contributions from analyzer_diagnostics: compute counts
for analyzer_diagnostics (by mapping into biome_diagnostics::serde::Diagnostic
or by inspecting their severity) and add those counts to
parse_errors/parse_warnings/parse_infos (or directly to the totals) before
constructing LintResults; update the values used for errors, warnings, and infos
so they include both the parse_* totals and the analyzer-derived counts
(referencing analyzer_diagnostics, biome_diagnostics::serde::Diagnostic::new,
diagnostic_level, and the LintResults construction).
- Around line 724-726: The loop that populates pull_diagnostics currently
requests only rule fixes (signal.actions(ActionFilter::rule_fix())), which drops
suppression suggestions; update the action query used inside the
pull_code_actions branch so it requests both fixes and suppression suggestions
(e.g., use a filter that includes suppressions such as ActionFilter::all() or
the combined fix+suppression variant your API provides) and continue adding each
action to diagnostic via diagnostic.add_code_suggestion(action.into()); ensure
you update the call to signal.actions(...) rather than changing how suggestions
are appended.
- Around line 720-723: The cap/filter leaks because the per-signal guard uses <=
against diagnostic_count that already includes parse diagnostics and then
appends parse_diagnostics and analyzer_diagnostics without applying the same
budget or diagnostic_level filtering; change the per-signal check in the block
that references max_diagnostics and diagnostic_count to use < (strictly less) so
you don't allow an off-by-one, and when serializing/appending parse_diagnostics
and analyzer_diagnostics (the code that pushes into the signal diagnostics
array) apply the same remaining-budget logic derived from max_diagnostics minus
diagnostic_count (slice or take only up to remaining) and honor diagnostic_level
(e.g., skip parse warnings when diagnostic_level == Error) so all diagnostics
are uniformly capped and filtered (update the logic in the code sections
handling parse_diagnostics, analyzer_diagnostics and the block that checks
max_diagnostics/diagnostic_count).
In `@crates/biome_service/src/workspace/server.rs`:
- Line 1864: The tuple returned for parse-only mode currently hardcodes zero for
warning/info/hint counts (the line returning (parse_diagnostics, errors, 0, 0,
0)), which causes aggregate counts to diverge from the actual diagnostics;
update the return to compute the counts from parse_diagnostics (or derive
warning/info/hint counts the same way the non-parse branch does) so the returned
tuple reflects actual diagnostics counts—locate the return that uses
parse_diagnostics and replace the zero literals with the computed
warning/info/hint totals.
---
Outside diff comments:
In `@crates/biome_service/src/workspace/server.rs`:
- Around line 1757-1839: The LSP currently applies max_diagnostics separately
per lint pass (root and each embedded snippet) and ignores it in the parse-only
path, allowing the total returned diagnostics to exceed the requested cap;
update the logic around the lint invocation (the lint function and LintParams
usage in this block and the parse-only branch) to allocate a single shared
remaining_budget integer (starting from max_diagnostics) that is decremented by
the number of diagnostics emitted after each call, pass the remaining_budget
into each LintParams (replace max_diagnostics with remaining_budget) and stop
calling lint or accumulating diagnostics once remaining_budget reaches zero;
ensure the parse-only branch also consumes and respects remaining_budget the
same way so root+embedded runs share one cap.
---
Nitpick comments:
In `@crates/biome_analyze/src/signals.rs`:
- Around line 39-55: The three constructor functions (rule_fix,
inline_suppression, toplevel_suppression) use a verbose mutable pattern creating
ActionKind::empty() and then insert(...); simplify each by returning
Self(ActionKind::RuleFix), Self(ActionKind::InlineSuppression), and
Self(ActionKind::ToplevelSuppression) respectively (i.e., drop the mutable
filter and insert calls and construct the bitflag directly).
In `@crates/biome_cli/src/runner/impls/process_file/format.rs`:
- Line 119: Clarify the ambiguous comment on the pull_code_actions flag: locate
the configuration/struct field named pull_code_actions (in format.rs) and either
replace `// NOTE: probably to revisit` with a concise rationale (e.g., `//
intentionally disabled in format-only mode to avoid network calls`) or change it
to a TODO linking an issue/issue number (e.g., `// TODO: revisit behavior - see
issue `#NNN``) and if this change represents future work, open an issue and
reference it in the TODO comment so maintainers can track it.
In `@crates/biome_js_analyze/tests/spec_tests.rs`:
- Around line 224-225: The test iterates using
event.actions(ActionFilter::all()) which requests every action and undermines
the new caller-side filtering; update the loop to use the narrowest ActionFilter
based on check_action_type (e.g., use ActionFilter::suppressions() when
check_action_type.is_suppression() is true, otherwise use
ActionFilter::fixes()), and make the same change at the other occurrence
referenced (around lines 254-255) so the harness only fetches the needed actions
and validates the new filtering behavior.
In `@xtask/rules_check/src/lib.rs`:
- Around line 403-407: The loop filters actions using ActionFilter::rule_fix()
but then redundantly checks !action.is_suppression() before calling
diag.add_code_suggestion; remove the suppression check to simplify the logic.
Update the block iterating signal.actions(ActionFilter::rule_fix()) so it
unconditionally calls diag.add_code_suggestion(action.into()) for each action,
relying on ActionFilter::rule_fix() to already exclude suppression actions
(references: signal.actions, ActionFilter::rule_fix(), action.is_suppression(),
diag.add_code_suggestion).
🪄 Autofix (Beta)
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
Run ID: 503ba152-d141-4f0b-ab53-0e61228c9c91
⛔ Files ignored due to path filters (26)
crates/biome_cli/tests/snapshots/main_cases_css_parsing/check_combined_format_with_errors_and_css_modules.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_css_parsing/check_css_parse_css_modules_false.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_css_parsing/ci_css_parse_css_modules_false.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_format_with_errors/check_format_with_errors_false.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_format_with_errors/check_format_with_errors_overrides_config.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_format_with_errors/check_format_with_errors_respects_config_false.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_format_with_errors/check_format_with_errors_respects_config_true.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_format_with_errors/check_format_with_errors_true.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_format_with_errors/ci_format_with_errors_false.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_format_with_errors/ci_format_with_errors_true.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_json_parsing/check_json_parse_allow_comments_false.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_json_parsing/check_json_parse_allow_trailing_commas_false.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_overrides_formatter/disallow_comments_on_well_known_files.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_reporter_summary/reports_diagnostics_summary_check_command.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_reporter_summary/reports_diagnostics_summary_check_verbose_command.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_reporter_summary/reports_diagnostics_summary_check_verbose_command_destination.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_reporter_summary/reports_diagnostics_summary_ci_command.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_reporter_terminal/reports_diagnostics_check_command_verbose.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_reporter_terminal/reports_diagnostics_check_write_command_file.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_reporter_terminal/reports_diagnostics_check_write_command_verbose.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_check/apply_bogus_argument.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_check/check_format_with_syntax_errors_when_flag_enabled.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_check/parse_error.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_ci/ci_parse_error.snapis excluded by!**/*.snapand included by**crates/biome_service/src/workspace/snapshots/biome_service__workspace__server__tests__pull_diagnostics_and_actions_for_js_file.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**
📒 Files selected for processing (54)
.changeset/gentle-comics-win.md.changeset/mighty-pillows-count.md.changeset/silly-islands-relate.md.changeset/smart-radios-end.md.changeset/thirty-breads-beg.mdcrates/biome_analyze/src/lib.rscrates/biome_analyze/src/signals.rscrates/biome_cli/src/commands/search.rscrates/biome_cli/src/execute/migrate.rscrates/biome_cli/src/runner/crawler.rscrates/biome_cli/src/runner/handler.rscrates/biome_cli/src/runner/impls/collectors/default.rscrates/biome_cli/src/runner/impls/process_file/check.rscrates/biome_cli/src/runner/impls/process_file/format.rscrates/biome_cli/src/runner/impls/process_file/lint_and_assist.rscrates/biome_cli/src/runner/mod.rscrates/biome_cli/src/runner/process_file.rscrates/biome_css_analyze/benches/css_analyzer.rscrates/biome_css_analyze/src/lib.rscrates/biome_css_analyze/tests/quick_test.rscrates/biome_css_analyze/tests/spec_tests.rscrates/biome_graphql_analyze/src/lib.rscrates/biome_graphql_analyze/tests/spec_tests.rscrates/biome_html_analyze/benches/html_analyzer.rscrates/biome_html_analyze/src/lib.rscrates/biome_html_analyze/tests/spec_tests.rscrates/biome_js_analyze/benches/js_analyzer.rscrates/biome_js_analyze/src/suppressions.tests.rscrates/biome_js_analyze/tests/quick_test.rscrates/biome_js_analyze/tests/spec_tests.rscrates/biome_json_analyze/benches/json_analyzer.rscrates/biome_json_analyze/src/lib.rscrates/biome_json_analyze/tests/spec_tests.rscrates/biome_lsp/src/capabilities.rscrates/biome_lsp/src/handlers/analysis.rscrates/biome_lsp/src/server.rscrates/biome_lsp/src/server.tests.rscrates/biome_lsp/src/session.rscrates/biome_lsp/src/utils.rscrates/biome_migrate/src/lib.rscrates/biome_migrate/tests/spec_tests.rscrates/biome_rowan/src/ast/batch.rscrates/biome_service/src/file_handlers/css.rscrates/biome_service/src/file_handlers/graphql.rscrates/biome_service/src/file_handlers/grit.rscrates/biome_service/src/file_handlers/html.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_service/src/file_handlers/json.rscrates/biome_service/src/file_handlers/mod.rscrates/biome_service/src/workspace.rscrates/biome_service/src/workspace.tests.rscrates/biome_service/src/workspace/server.rscrates/biome_test_utils/src/lib.rsxtask/rules_check/src/lib.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_configuration/src/analyzer/mod.rs`:
- Around line 718-726: Add unit tests for the from_group_and_rule function
inside this file's #[cfg(test)] module that cover the three paths: a valid
group+rule returns Some(Self::Rule(...)), an invalid rule (valid group but
unknown rule) returns None, and an invalid group (linter::RuleGroup::from_str
fails) returns None; use concrete known group and rule strings (and a couple of
clearly invalid strings) and call from_group_and_rule directly, asserting the
Option result (and if Some, that the inner Rule variant contains the expected
group and rule via Rule::as_str or pattern matching); reference
from_group_and_rule, linter::RuleGroup::from_str, and Rules::has_rule when
locating where to add the tests.
In `@crates/biome_lsp/src/handlers/analysis.rs`:
- Around line 203-232: The deferred FixAll path fails to preserve the client's
organize-imports preference; update the CodeActionResolveData struct to include
a boolean like include_organize_imports (or reuse an existing field name such as
has_organize_imports), set that field when creating the CodeActionResolveData in
the supports_resolve branch (where CodeActionResolveKind::FixAll is used), and
then use that field in the resolve handler that currently hardcodes true before
calling fix_all (the same fix_all function used in the non-deferred branch).
Ensure the field is serialized/deserialized with serde so
serde_json::to_value(data).ok() continues to work.
🪄 Autofix (Beta)
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
Run ID: 37596087-853a-4ca5-be3b-57281f32e711
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lockand included by**
📒 Files selected for processing (8)
.changeset/gentle-comics-win.md.changeset/smart-radios-end.mdcrates/biome_configuration/src/analyzer/mod.rscrates/biome_lsp/Cargo.tomlcrates/biome_lsp/src/handlers/analysis.rscrates/biome_lsp/src/session.rscrates/biome_lsp/src/utils.rscrates/biome_service/src/file_handlers/mod.rs
✅ Files skipped from review due to trivial changes (2)
- .changeset/smart-radios-end.md
- crates/biome_service/src/file_handlers/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_lsp/src/session.rs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_lsp/src/server.tests.rs (1)
1530-1533: Make the resolve test less order-sensitive.Picking
res[0]assumes action ordering stays fixed. Finding the quick-fix by kind/title would keep this test stable if ordering changes later.♻️ Suggested tweak
- let first_action = match &res[0] { - CodeActionOrCommand::CodeAction(action) => action.clone(), - _ => panic!("expected CodeAction"), - }; + let first_action = res + .iter() + .find_map(|item| match item { + CodeActionOrCommand::CodeAction(action) + if action.kind + == Some(CodeActionKind::new( + "quickfix.biome.suspicious.noCompareNegZero", + )) => + { + Some(action.clone()) + } + _ => None, + }) + .context("expected quick-fix code action for noCompareNegZero")?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_lsp/src/server.tests.rs` around lines 1530 - 1533, The test currently assumes a fixed ordering by taking res[0]; instead, search the result vector (res) for the CodeAction matching the quick-fix by inspecting each CodeActionOrCommand and checking action.title and/or action.kind (match on CodeActionOrCommand::CodeAction and examine action.title or action.kind) and use the found action as first_action; update the code that panics on non-matches to fail if no matching quick-fix is found so the test is order-insensitive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_lsp/src/server.tests.rs`:
- Around line 1530-1533: The test currently assumes a fixed ordering by taking
res[0]; instead, search the result vector (res) for the CodeAction matching the
quick-fix by inspecting each CodeActionOrCommand and checking action.title
and/or action.kind (match on CodeActionOrCommand::CodeAction and examine
action.title or action.kind) and use the found action as first_action; update
the code that panics on non-matches to fail if no matching quick-fix is found so
the test is order-insensitive.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b749101-6a12-48a3-b515-bca87645f49f
📒 Files selected for processing (10)
crates/biome_cli/src/runner/impls/collectors/default.rscrates/biome_configuration/src/analyzer/mod.rscrates/biome_lsp/src/server.tests.rscrates/biome_lsp/src/session.rscrates/biome_rowan/src/ast/batch.rscrates/biome_service/src/file_handlers/mod.rscrates/biome_service/src/workspace.rscrates/biome_service/src/workspace.tests.rscrates/biome_service/src/workspace/server.rscrates/biome_test_utils/src/lib.rs
✅ Files skipped from review due to trivial changes (1)
- crates/biome_service/src/file_handlers/mod.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- crates/biome_test_utils/src/lib.rs
- crates/biome_cli/src/runner/impls/collectors/default.rs
- crates/biome_service/src/workspace.tests.rs
- crates/biome_rowan/src/ast/batch.rs
- crates/biome_lsp/src/session.rs
- crates/biome_service/src/workspace/server.rs
- crates/biome_service/src/workspace.rs
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_cli/src/runner/impls/collectors/default.rs (1)
223-265:⚠️ Potential issue | 🟠 MajorCollector-side verbose filtering still loses diagnostics after the workspace cap.
errors/warnings/infosandskipped_diagnosticsarrive frompull_diagnostics_resultbeforeshould_skip_diagnosticruns. If one of the firstmax_diagnosticsentries is later dropped here as verbose, you cannot backfill the next non-verbose diagnostic from the skipped tail, and the totals are still wrong whenever that tail also contains verbose entries. Please apply the verbose filter beforemax_diagnosticsis enforced, or carry post-filtered counts/skipped totals inMessage::Diagnostics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_cli/src/runner/impls/collectors/default.rs` around lines 223 - 265, The current logic updates workspace counters (errors/warnings/infos and not_printed_diagnostics) before applying the per-diagnostic verbose filter in should_skip_diagnostic, causing counts to be wrong when early diagnostics are dropped; fix by moving the verbose filtering step to happen before enforcing the max_diagnostics cap or by propagating post-filtered counts from Message::Diagnostics (i.e., have pull_diagnostics_result include counts after verbose filtering). Concretely, ensure diagnostics is pre-filtered via should_skip_diagnostic(severity, diag.tags()) (used by the loop here) before you compute/assign errors, warnings, infos, and skipped_diagnostics, or modify the producer that constructs Message::Diagnostics to carry adjusted actual_errors/actual_warnings/actual_infos and skipped_diagnostics so that this collector can safely fetch_add those final totals (affecting symbols: should_skip_diagnostic, pull_diagnostics_result/Message::Diagnostics, errors/warnings/infos, not_printed_diagnostics, push_diagnostic).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@crates/biome_cli/src/runner/impls/collectors/default.rs`:
- Around line 223-265: The current logic updates workspace counters
(errors/warnings/infos and not_printed_diagnostics) before applying the
per-diagnostic verbose filter in should_skip_diagnostic, causing counts to be
wrong when early diagnostics are dropped; fix by moving the verbose filtering
step to happen before enforcing the max_diagnostics cap or by propagating
post-filtered counts from Message::Diagnostics (i.e., have
pull_diagnostics_result include counts after verbose filtering). Concretely,
ensure diagnostics is pre-filtered via should_skip_diagnostic(severity,
diag.tags()) (used by the loop here) before you compute/assign errors, warnings,
infos, and skipped_diagnostics, or modify the producer that constructs
Message::Diagnostics to carry adjusted
actual_errors/actual_warnings/actual_infos and skipped_diagnostics so that this
collector can safely fetch_add those final totals (affecting symbols:
should_skip_diagnostic, pull_diagnostics_result/Message::Diagnostics,
errors/warnings/infos, not_printed_diagnostics, push_diagnostic).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 675e6e18-a533-450c-8d5f-c60810645d63
⛔ Files ignored due to path filters (1)
packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**
📒 Files selected for processing (1)
crates/biome_cli/src/runner/impls/collectors/default.rs
dyc3
reviewed
Mar 27, 2026
.changeset/smart-radios-end.md
Outdated
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Improved the performance of the commands `linet` and `check` when they are caled with `--write`. |
Contributor
There was a problem hiding this comment.
nit: typos
Suggested change
| Improved the performance of the commands `linet` and `check` when they are caled with `--write`. | |
| Improved the performance of the commands `lint` and `check` when they are called with `--write`. |
Comment on lines
32
to
+47
| @@ -57,5 +44,5 @@ file.module.css format ━━━━━━━━━━━━━━━━━━━ | |||
|
|
|||
| ```block | |||
| Checked 1 file in <TIME>. No fixes applied. | |||
| Found 3 errors. | |||
| Found 2 errors. | |||
Contributor
There was a problem hiding this comment.
is this a regression? I'm seeing similar losses of diagnostics all over the place in the cli tests
Member
Author
There was a problem hiding this comment.
Many losses were caused by the fix. Before, in check and ci, parse diagnostics were pulled twice (one from format, one form lint), inflating the numbers.
However, if you see a possible problem in some particular case, let me know and I'll check
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Note
Summary created via agent. Some of the code was done via coding agent, some other parts of the code were done by me. Essentially, I changed the important parts and let the agent fill the void.
Five areas of change: code action filtering, LSP resolve support, per-rule batch fix-all, diagnostics refactoring, and bug fixes discovered along the way.
Code action filtering
ActionFilter controls which action categories (rule_fix, inline_suppression, top_level_suppression) are computed by AnalyzerSignal::actions(). Each category may create a BatchMutation that clones the syntax tree root.
Callers that only need rule fixes (e.g. fix-all) or only suppressions now skip the unneeded categories, avoiding expensive allocations on large files. All signal.actions() call sites across all language handlers updated to
pass the appropriate filter.
LSP codeAction/resolve
When the editor supports it, code actions are returned without text edits. Edits are computed lazily on codeAction/resolve when the user selects an action. fix_all is also deferred. PullActionsParams gains compute_actions:
bool; CodeAction makes suggestion optional and adds applicability.
Per-rule batch fix-all
BatchMutation::merge() combines mutations. ProcessFixAll collects ALL fix actions per analyzer pass, then merges actions from the same rule into one commit. Cross-rule batching is avoided (mutations can conflict). For 644
useConst fixes: ~10 iterations instead of ~700.
Diagnostics refactoring
PullDiagnosticsParams gains max_diagnostics, diagnostic_level, and enforce_assist. The workspace now handles severity filtering, assist enforcement, and accurate counting — the CLI collector uses workspace counts directly.
In check mode, FormatProcessFile::format_only() skips pull_diagnostics since lint already handled it.
Bug fixes
Test Plan
Snapshot changes
Docs
N/A