feat(cli): add watch mode (--watch) to the CLI for check/format/lint commands#8731
Merged
Conversation
🦋 Changeset detectedLatest commit: 53a3127 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
Merging this PR will not alter performance
Comparing Footnotes
|
Contributor
WalkthroughAdds a 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @crates/biome_cli/src/commands/lint.rs:
- Around line 268-288: The watch-mode incompatibility check in
check_incompatible_arguments currently only checks self.fix and self.write; add
a similar branch for self.suppress so that if self.is_watch_mode() and
self.suppress is true you return
Err(CliDiagnostic::incompatible_arguments("--watch", "--suppress", "Modifying
files by adding suppression comments is not available in watch mode.")); place
this new check next to the existing checks for self.fix and self.write inside
the same if self.is_watch_mode() block and use the same
CliDiagnostic::incompatible_arguments helper.
In @crates/biome_cli/src/runner/impls/watchers/default.rs:
- Around line 35-57: The poll method uses self.rx.iter() which blocks
indefinitely and prevents graceful shutdown; replace the blocking iterator with
a non-blocking/timeout-driven receive (e.g., use
self.rx.recv_timeout(Duration::from_millis(...)) in a loop or try_recv() with a
sleep) so poll can return None or propagate a shutdown signal when the watcher
is dropped or on Ctrl+C; update the logic that filters events (the matches!
block) to operate on the received Event and construct the WatcherEvent paths the
same way, and ensure any recv timeout error is handled (returning None or
breaking) to allow clean shutdown.
In @crates/biome_cli/src/runner/watcher.rs:
- Around line 7-17: The trait method Watcher::poll currently takes &self but the
implementation calls iter() on a Receiver which requires &mut self; change the
trait signature to fn poll(&mut self) or alternatively wrap the internal
Receiver in interior mutability (e.g., Mutex/RefCell) so iter() can be called
from an immutable reference; update all impls of Watcher (and any callers) to
match the new fn poll(&mut self) signature if you choose the signature change,
or refactor the watcher struct to hold the Receiver inside a Mutex/RefCell and
adjust the impl of poll to lock/borrow and call iter() accordingly.
🧹 Nitpick comments (2)
crates/biome_cli/src/commands/format.rs (1)
232-252: Consider extracting shared validation logic.This validation is now duplicated across
check.rs,lint.rs, andformat.rs. A shared helper function would reduce repetition.♻️ Optional refactor suggestion
// In commands/mod.rs or a shared module pub(crate) fn check_watch_incompatible_arguments( is_watch: bool, fix: bool, write: bool, ) -> Result<(), CliDiagnostic> { if is_watch { if fix { return Err(CliDiagnostic::incompatible_arguments( "--watch", "--fix", "Applying code fixes is not available in watch mode.", )); } if write { return Err(CliDiagnostic::incompatible_arguments( "--watch", "--write", "Applying code fixes is not available in watch mode.", )); } } Ok(()) }crates/biome_cli/src/runner/mod.rs (1)
345-388: Two observations on the watch loop.
Reusing
scan_duration: The original scan duration is passed to everyfinalizecall, but it doesn't reflect the actual time for re-runs. Consider measuring per-iteration or passingNone.Discarded result:
_ = Self::Finalizer::finalize(...)silently discards errors. If finalization fails (e.g., console issues), the loop continues without feedback.♻️ Suggested improvement
while let Some(event) = watcher.poll() { + let iteration_start = std::time::Instant::now(); let collector = self.collector(fs, execution.as_ref(), cli_options); // ... crawl and before_finalize ... - _ = Self::Finalizer::finalize(FinalizePayload { + if let Err(e) = Self::Finalizer::finalize(FinalizePayload { cli_options, project_key, execution: execution.as_ref(), fs, console, workspace, - scan_duration: duration, + scan_duration: Some(iteration_start.elapsed()), crawler_output: output, paths: event .paths .into_iter() .map(|path| path.into_string()) .collect(), - }); + }) { + console.error(markup! { + {PrintDiagnostic::simple(&e)} + }); + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
Cargo.lockis excluded by!**/*.lockand included by**crates/biome_cli/tests/snapshots/main_commands_check/check_help.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_format/format_help.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_lint/lint_help.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (17)
.changeset/fresh-otters-peel.mdCargo.tomlcrates/biome_cli/Cargo.tomlcrates/biome_cli/src/commands/check.rscrates/biome_cli/src/commands/format.rscrates/biome_cli/src/commands/lint.rscrates/biome_cli/src/commands/mod.rscrates/biome_cli/src/lib.rscrates/biome_cli/src/runner/crawler.rscrates/biome_cli/src/runner/impls/commands/custom_execution.rscrates/biome_cli/src/runner/impls/commands/traversal.rscrates/biome_cli/src/runner/impls/mod.rscrates/biome_cli/src/runner/impls/watchers/default.rscrates/biome_cli/src/runner/impls/watchers/mod.rscrates/biome_cli/src/runner/mod.rscrates/biome_cli/src/runner/watcher.rscrates/biome_service/Cargo.toml
🧰 Additional context used
📓 Path-based instructions (2)
**/Cargo.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/Cargo.toml: Use workspace dependencies withworkspace = truefor internal crates in Cargo.toml
Use path dependencies for dev-dependencies in crates to avoid requiring published versions
Files:
crates/biome_cli/Cargo.tomlcrates/biome_service/Cargo.tomlCargo.toml
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_cli/src/runner/impls/mod.rscrates/biome_cli/src/runner/impls/watchers/mod.rscrates/biome_cli/src/commands/format.rscrates/biome_cli/src/lib.rscrates/biome_cli/src/commands/lint.rscrates/biome_cli/src/runner/crawler.rscrates/biome_cli/src/runner/impls/watchers/default.rscrates/biome_cli/src/runner/impls/commands/traversal.rscrates/biome_cli/src/runner/watcher.rscrates/biome_cli/src/commands/check.rscrates/biome_cli/src/commands/mod.rscrates/biome_cli/src/runner/impls/commands/custom_execution.rscrates/biome_cli/src/runner/mod.rs
🧠 Learnings (34)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Implement watcher tests for workspace methods in watcher.tests.rs and end-to-end tests in LSP tests
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace_watcher.rs : Use WorkspaceWatcher to keep workspace state in sync with the filesystem, and only activate it in daemon mode
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Debug the WorkspaceWatcher by starting the daemon with cargo run --bin=biome -- start and running commands such as cargo run --bin=biome -- lint --use-server <path>
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/Cargo.toml : Include development dependencies in `Cargo.toml` for formatter tests: `biome_formatter_test`, `biome_<language>_factory`, `biome_<language>_parser`, `biome_parser`, `biome_service`, `countme`, `iai`, `quickcheck`, `quickcheck_macros`, and `tests_macros`
Applied to files:
crates/biome_cli/Cargo.tomlcrates/biome_cli/src/runner/impls/mod.rscrates/biome_service/Cargo.toml
📚 Learning: 2025-12-21T21:15:03.796Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-21T21:15:03.796Z
Learning: Applies to **/Cargo.toml : Use workspace dependencies with `workspace = true` for internal crates in Cargo.toml
Applied to files:
crates/biome_cli/Cargo.tomlcrates/biome_service/Cargo.tomlCargo.toml
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Create two new crates `biome_{language}_syntax` and `biome_{language}_factory` using `cargo new --lib` for new language parsers
Applied to files:
crates/biome_cli/Cargo.toml
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace*.rs : Implement the Workspace trait in the Biome Service to manage internal state of projects, including open documents, project layout instances, and module graph instances
Applied to files:
crates/biome_cli/Cargo.tomlcrates/biome_cli/src/runner/impls/mod.rscrates/biome_service/Cargo.toml
📚 Learning: 2025-12-21T21:15:03.796Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-21T21:15:03.796Z
Learning: Build debug binaries using `cargo build --bin biome` for development and triaging
Applied to files:
crates/biome_cli/Cargo.toml
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Implement watcher tests for workspace methods in watcher.tests.rs and end-to-end tests in LSP tests
Applied to files:
crates/biome_cli/Cargo.tomlcrates/biome_cli/src/runner/impls/mod.rscrates/biome_cli/src/runner/impls/watchers/mod.rscrates/biome_cli/src/commands/format.rscrates/biome_cli/src/lib.rscrates/biome_cli/src/commands/lint.rscrates/biome_cli/src/runner/impls/watchers/default.rscrates/biome_cli/src/runner/impls/commands/traversal.rscrates/biome_cli/src/runner/watcher.rscrates/biome_cli/src/commands/check.rscrates/biome_cli/src/commands/mod.rscrates/biome_service/Cargo.tomlcrates/biome_cli/src/runner/impls/commands/custom_execution.rscrates/biome_cli/src/runner/mod.rs
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace_watcher.rs : Use WorkspaceWatcher to keep workspace state in sync with the filesystem, and only activate it in daemon mode
Applied to files:
crates/biome_cli/Cargo.tomlcrates/biome_cli/src/runner/impls/mod.rscrates/biome_cli/src/runner/impls/watchers/mod.rscrates/biome_cli/src/commands/format.rscrates/biome_cli/src/runner/impls/watchers/default.rscrates/biome_cli/src/runner/impls/commands/traversal.rscrates/biome_cli/src/runner/watcher.rscrates/biome_cli/src/commands/check.rscrates/biome_cli/src/commands/mod.rscrates/biome_cli/src/runner/impls/commands/custom_execution.rscrates/biome_cli/src/runner/mod.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/js_module_info/collector.rs : Implement module-level (thin) inference to resolve `TypeReference::Qualifier` variants by looking up declarations in module scopes and handling import statements
Applied to files:
crates/biome_cli/src/runner/impls/mod.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper types from the biome_diagnostics::v2 module (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) or implement the Advices trait yourself for custom advice handling
Applied to files:
crates/biome_cli/src/runner/impls/mod.rscrates/biome_cli/src/lib.rscrates/biome_cli/src/commands/check.rscrates/biome_cli/src/runner/mod.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/local_inference.rs : Implement local inference in dedicated modules to derive type definitions from expressions without context of surrounding scopes
Applied to files:
crates/biome_cli/src/runner/impls/mod.rs
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Debug the WorkspaceWatcher by starting the daemon with cargo run --bin=biome -- start and running commands such as cargo run --bin=biome -- lint --use-server <path>
Applied to files:
.changeset/fresh-otters-peel.mdcrates/biome_cli/src/lib.rscrates/biome_cli/src/runner/impls/commands/traversal.rscrates/biome_cli/src/commands/mod.rscrates/biome_cli/src/runner/mod.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/src/lib.rs : Expose a public `format_node` function that accepts formatting options and a root syntax node, returning a `FormatResult<Formatted<Context>>` with appropriate documentation
Applied to files:
crates/biome_cli/src/commands/format.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/src/context.rs : Define `<Language>FormatContext` struct in a `context.rs` file containing `comments` and `source_map` fields, implementing `FormatContext` and `CstFormatContext` traits
Applied to files:
crates/biome_cli/src/commands/format.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Applied to files:
crates/biome_cli/src/commands/format.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use helper functions like `map`, `filter`, and `and_then` to avoid excessive nested `if let` statements
Applied to files:
crates/biome_cli/src/lib.rscrates/biome_cli/src/commands/lint.rscrates/biome_cli/src/runner/crawler.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Deprecated rules must include a `deprecated` field in the `declare_lint_rule!` macro with an explanation of what rule to use instead
Applied to files:
crates/biome_cli/src/commands/lint.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `declare_lint_rule!` macro with a `version` field set to `next` for new rules
Applied to files:
crates/biome_cli/src/commands/lint.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Implement the `diagnostic` function to provide error messages explaining what the error is, why it is triggered, and what the user should do
Applied to files:
crates/biome_cli/src/commands/lint.rs
📚 Learning: 2025-12-22T09:27:13.161Z
Learnt from: ematipico
Repo: biomejs/biome PR: 8537
File: crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs:167-210
Timestamp: 2025-12-22T09:27:13.161Z
Learning: In crates/biome_analyze/**/*analyze/src/**/*.rs, the `fix_kind` field in `declare_lint_rule!` should only be specified when the rule implements the `action` function. Rules that only emit diagnostics without providing code fixes should not include `fix_kind` in their metadata.
Applied to files:
crates/biome_cli/src/commands/lint.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Implement the `action` function and add `fix_kind` metadata to the rule macro if the rule provides code actions
Applied to files:
crates/biome_cli/src/commands/lint.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `declare_node_union!` macro to query multiple node types together to avoid redundant traversal passes
Applied to files:
crates/biome_cli/src/commands/lint.rscrates/biome_cli/src/runner/crawler.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `Markup!` macro for diagnostic messages and code action descriptions to ensure proper formatting
Applied to files:
crates/biome_cli/src/commands/lint.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Mark rules with `issue_number` in the `declare_lint_rule!` macro if they are work-in-progress to indicate missing features
Applied to files:
crates/biome_cli/src/commands/lint.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ParseSeparatedList` and `ParseNodeList` for parsing lists with error recovery to avoid infinite loops
Applied to files:
crates/biome_cli/src/runner/crawler.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Use `Box<[Box<str>]>` instead of `Vec<String>` for collections of strings in rule options to save memory
Applied to files:
crates/biome_cli/src/runner/crawler.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Store type data in linear vectors instead of using recursive data structures with `Arc` for improved data locality and performance
Applied to files:
crates/biome_cli/src/runner/crawler.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Implement custom `Queryable` and `Visitor` types for rules that require deep inspection of child nodes to avoid inefficient traversals
Applied to files:
crates/biome_cli/src/runner/crawler.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Avoid string allocations by using `&str` or `TokenText` instead of `to_string()`
Applied to files:
crates/biome_cli/src/runner/crawler.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_cli/src/runner/crawler.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/language_kind.rs : Add a new variant to `LanguageKind` enum in `language_kind.rs` file and implement all methods for the new language variant
Applied to files:
crates/biome_cli/src/runner/crawler.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use the `try operator (?)` to transform `Result` types into `Option` when the `run` function returns `Option`
Applied to files:
crates/biome_cli/src/runner/crawler.rs
📚 Learning: 2025-12-21T21:15:03.796Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-21T21:15:03.796Z
Learning: Applies to **/Cargo.toml : Use path dependencies for dev-dependencies in crates to avoid requiring published versions
Applied to files:
crates/biome_service/Cargo.toml
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace/server.rs : Use WorkspaceServer implementation for maintaining workspace state in daemon mode and CLI daemonless mode
Applied to files:
crates/biome_cli/src/runner/mod.rs
🧬 Code graph analysis (8)
crates/biome_cli/src/commands/format.rs (8)
crates/biome_cli/src/commands/check.rs (2)
is_watch_mode(165-167)check_incompatible_arguments(285-305)crates/biome_cli/src/commands/lint.rs (2)
is_watch_mode(156-158)check_incompatible_arguments(268-288)crates/biome_cli/src/runner/impls/commands/traversal.rs (4)
is_watch_mode(101-103)is_watch_mode(161-163)check_incompatible_arguments(134-136)check_incompatible_arguments(216-218)crates/biome_cli/src/runner/mod.rs (2)
is_watch_mode(210-210)check_incompatible_arguments(593-595)crates/biome_cli/src/commands/ci.rs (1)
check_incompatible_arguments(249-266)crates/biome_cli/src/commands/migrate.rs (1)
check_incompatible_arguments(111-119)crates/biome_cli/src/commands/mod.rs (1)
incompatible_arguments(947-958)crates/biome_cli/src/diagnostics.rs (1)
incompatible_arguments(297-307)
crates/biome_cli/src/lib.rs (2)
crates/biome_cli/src/runner/impls/watchers/default.rs (1)
watch(23-33)crates/biome_cli/src/runner/watcher.rs (2)
watch(12-12)watch(22-22)
crates/biome_cli/src/commands/lint.rs (8)
crates/biome_cli/src/commands/check.rs (2)
is_watch_mode(165-167)check_incompatible_arguments(285-305)crates/biome_cli/src/runner/impls/commands/custom_execution.rs (3)
is_watch_mode(100-102)check_incompatible_arguments(57-57)check_incompatible_arguments(144-146)crates/biome_cli/src/runner/impls/commands/traversal.rs (4)
is_watch_mode(101-103)is_watch_mode(161-163)check_incompatible_arguments(134-136)check_incompatible_arguments(216-218)crates/biome_cli/src/runner/mod.rs (2)
is_watch_mode(210-210)check_incompatible_arguments(593-595)crates/biome_cli/src/commands/ci.rs (1)
check_incompatible_arguments(249-266)crates/biome_cli/src/commands/migrate.rs (1)
check_incompatible_arguments(111-119)crates/biome_cli/src/commands/mod.rs (1)
incompatible_arguments(947-958)crates/biome_cli/src/diagnostics.rs (1)
incompatible_arguments(297-307)
crates/biome_cli/src/runner/impls/watchers/default.rs (1)
crates/biome_cli/src/runner/watcher.rs (6)
new(9-9)new(20-20)watch(12-12)watch(22-22)poll(16-16)poll(24-26)
crates/biome_cli/src/runner/impls/commands/traversal.rs (5)
crates/biome_cli/src/commands/check.rs (1)
is_watch_mode(165-167)crates/biome_cli/src/commands/format.rs (1)
is_watch_mode(123-125)crates/biome_cli/src/commands/lint.rs (1)
is_watch_mode(156-158)crates/biome_cli/src/runner/impls/commands/custom_execution.rs (1)
is_watch_mode(100-102)crates/biome_cli/src/runner/mod.rs (1)
is_watch_mode(210-210)
crates/biome_cli/src/runner/watcher.rs (2)
crates/biome_cli/src/runner/impls/watchers/default.rs (3)
new(15-21)watch(23-33)poll(35-57)crates/biome_cli/src/runner/mod.rs (1)
paths(230-233)
crates/biome_cli/src/runner/impls/commands/custom_execution.rs (5)
crates/biome_cli/src/commands/check.rs (1)
is_watch_mode(165-167)crates/biome_cli/src/commands/format.rs (1)
is_watch_mode(123-125)crates/biome_cli/src/commands/lint.rs (1)
is_watch_mode(156-158)crates/biome_cli/src/runner/impls/commands/traversal.rs (2)
is_watch_mode(101-103)is_watch_mode(161-163)crates/biome_cli/src/runner/mod.rs (1)
is_watch_mode(210-210)
crates/biome_cli/src/runner/mod.rs (4)
crates/biome_cli/src/commands/check.rs (1)
is_watch_mode(165-167)crates/biome_cli/src/commands/format.rs (1)
is_watch_mode(123-125)crates/biome_cli/src/commands/lint.rs (1)
is_watch_mode(156-158)crates/biome_cli/src/runner/watcher.rs (2)
new(9-9)new(20-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (26)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: End-to-end tests
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_html_analyze)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_tailwind_parser)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: autofix
🔇 Additional comments (30)
crates/biome_service/Cargo.toml (1)
71-71: LGTM!Correctly migrated to workspace dependency whilst preserving the required feature.
crates/biome_cli/src/runner/crawler.rs (2)
20-23: LGTM!The
CrawlPathenum provides a clean abstraction for handling paths from different sources (CLI args vs. watch events).
84-87: LGTM!Both variants are correctly converted to
Utf8PathBuffor traversal.crates/biome_cli/Cargo.toml (1)
58-58: LGTM!Correctly declared as a workspace dependency for watch mode support.
crates/biome_cli/src/runner/impls/watchers/mod.rs (1)
1-1: LGTM! Consider adding tests for the watcher.The module declaration is correct. Based on learnings, consider implementing watcher tests (similar to the workspace watcher tests) to verify the watch-mode behaviour, even if end-to-end tests are challenging.
Cargo.toml (1)
130-130: No action required—notify 8.2.0 is the latest version and has no known security advisories.Version 8.2.0 (released August 2025) is current and secure.
crates/biome_cli/src/runner/impls/mod.rs (1)
8-8: LGTM!Clean module addition that follows the existing pattern.
.changeset/fresh-otters-peel.md (1)
1-5: LGTM!The changeset accurately documents the new watch mode feature with appropriate versioning.
crates/biome_cli/src/runner/impls/commands/custom_execution.rs (1)
87-87: LGTM!Correctly signals that custom execution commands don't participate in watch mode by using the unit type for
Watcherand returningfalsefromis_watch_mode().Also applies to: 100-102
crates/biome_cli/src/commands/mod.rs (1)
207-209: LGTM!The watch flag is consistently added across Check, Lint, and Format commands with clear documentation. Appropriately excluded from CI command since it's read-only.
Also applies to: 325-327, 410-412
crates/biome_cli/src/commands/check.rs (2)
47-47: LGTM!The watch field and
is_watch_mode()accessor are correctly implemented, matching the pattern used in Format and Lint commands.Also applies to: 165-167
285-305: Critical validation logic looks sound.The incompatible arguments check correctly prevents combining
--watchwith--fixor--write, which would create an infinite loop (watch detects change → applies fix → file changes → watch detects change...).However, as you noted in the PR description, test coverage for watch mode behaviour is missing. Consider adding tests to verify this validation and the watch mode lifecycle.
Based on learnings, implement watcher tests in
watcher.tests.rsfor workspace methods and end-to-end tests in LSP tests. Would you like me to help generate test cases for the incompatible arguments validation?crates/biome_cli/src/commands/lint.rs (2)
52-52: LGTM!The watch flag is correctly added to the payload.
156-158: LGTM!Clean delegation to the watch field.
crates/biome_cli/src/commands/format.rs (2)
45-45: LGTM!Watch field correctly added.
123-125: LGTM!Clean implementation.
crates/biome_cli/src/lib.rs (3)
102-127: LGTM!Watch flag correctly propagated to the Check command payload.
153-181: LGTM!Watch flag correctly propagated to the Lint command payload.
240-264: LGTM!Watch flag correctly propagated to the Format command payload.
crates/biome_cli/src/runner/impls/commands/traversal.rs (4)
8-8: LGTM!DefaultWatcher import correctly added.
100-103: LGTM!Sensible default - commands opt-in to watch mode rather than opt-out.
147-147: LGTM!DefaultWatcher correctly wired as the concrete watcher type.
161-163: LGTM!Clean delegation to the inner command's is_watch_mode().
crates/biome_cli/src/runner/watcher.rs (2)
3-5: LGTM!Clean struct definition for watch events.
19-27: LGTM!Useful no-op implementation for commands that don't support watch mode.
crates/biome_cli/src/runner/mod.rs (4)
133-133: LGTM!Watcher module correctly added.
192-192: LGTM!Watcher associated type correctly added to the trait.
209-210: LGTM!Clear documentation and method signature.
266-272: LGTM!Good UX decision - non-default reporters (JSON, GitHub Actions) wouldn't work well with continuous watch output.
crates/biome_cli/src/runner/impls/watchers/default.rs (1)
23-33: The code is correct —paths_mut()andcommit()are standard notify crate APIs.The
notifycrate (v8.2.0) provides exactly this API on theWatchertrait for batch operations. ThePathsMuthelper (returned bypaths_mut()) allows adding or removing multiple paths efficiently before callingcommit()to apply changes. This is the documented, standard approach and is properly implemented here.Likely an incorrect or invalid review comment.
Comment on lines
+35
to
+57
| fn poll(&self) -> Option<WatcherEvent> { | ||
| let event = self.rx.iter().filter_map(Result::ok).find(|event| { | ||
| // Modifying folder or metadata is ignored as it can unlikely affect the results. | ||
| // Any event types are necessary for some platforms to catch events. | ||
| matches!( | ||
| event.kind, | ||
| EventKind::Create(CreateKind::File | CreateKind::Any) | ||
| | EventKind::Modify( | ||
| ModifyKind::Name(_) | ModifyKind::Data(_) | ModifyKind::Any | ||
| ) | ||
| | EventKind::Remove(RemoveKind::File | RemoveKind::Any) | ||
| | EventKind::Any | ||
| ) | ||
| })?; | ||
|
|
||
| Some(WatcherEvent { | ||
| paths: event | ||
| .paths | ||
| .into_iter() | ||
| .filter_map(|path| Utf8PathBuf::from_path_buf(path).ok()) | ||
| .collect(), | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Blocking iterator with no shutdown mechanism.
self.rx.iter() blocks indefinitely. If the watcher is dropped or an error occurs, this could hang. Consider using try_recv() with a timeout or recv_timeout() to allow graceful shutdown on Ctrl+C.
🤖 Prompt for AI Agents
In @crates/biome_cli/src/runner/impls/watchers/default.rs around lines 35 - 57,
The poll method uses self.rx.iter() which blocks indefinitely and prevents
graceful shutdown; replace the blocking iterator with a
non-blocking/timeout-driven receive (e.g., use
self.rx.recv_timeout(Duration::from_millis(...)) in a loop or try_recv() with a
sleep) so poll can return None or propagate a shutdown signal when the watcher
is dropped or on Ctrl+C; update the logic that filters events (the matches!
block) to operate on the received Event and construct the WatcherEvent paths the
same way, and ensure any recv timeout error is handled (returning None or
breaking) to allow clean shutdown.
Member
Author
There was a problem hiding this comment.
As the watching process is run synchronously, Ctrl+C still works (at least on my environment)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @crates/biome_cli/src/runner/impls/watchers/default.rs:
- Around line 27-32: The code currently swallows errors from
watched_paths.add(path.as_std_path(), RecursiveMode::Recursive).ok() and
watched_paths.commit().ok(), which can hide failures to watch paths; update the
calls to handle the Result instead of calling .ok(): for each
watched_paths.add(...) match the Result and log a warning or error including the
path display and the error (e.g., using the crate's logger/tracing) so users
know which paths failed to be added, and likewise check watched_paths.commit()
and log or propagate the error with context if commit fails.
- Around line 50-56: The current construction of WatcherEvent drops non-UTF-8
paths silently by calling Utf8PathBuf::from_path_buf(path).ok() inside the
filter_map; update this to detect conversion failures and log a warning when a
path is dropped. Replace the filter_map call in the WatcherEvent creation (the
block creating Some(WatcherEvent { paths: ... })) with code that attempts
Utf8PathBuf::from_path_buf for each path, collects successful conversions into
the paths vector, and emits a concise warning (e.g., tracing::warn! or
log::warn!) including the original PathBuf or its debug representation for any
conversion that fails so users are notified when non-UTF-8 paths are ignored.
🧹 Nitpick comments (3)
crates/biome_cli/src/runner/impls/watchers/default.rs (3)
15-21: Improve the panic message for better debugging.The watcher creation can fail for several reasons (e.g., OS resource limits, permissions, unsupported platform features). Consider a more descriptive error message to help users diagnose failures.
📝 Suggested improvement
- watcher: Box::new(recommended_watcher(tx).expect("watcher created")), + watcher: Box::new(recommended_watcher(tx).expect("failed to create file system watcher: check OS resources and permissions")),
35-48: Document the blocking behaviour and consider surfacing watcher errors.Line 36:
self.rx.iter()blocks the current thread indefinitely until a file system event arrives. This behaviour should be documented, aspoll()typically implies non-blocking. Additionally,filter_map(Result::ok)silently discards watcher errors that could indicate issues (e.g., buffer overflow, dropped events).Consider adding a rustdoc comment explaining the blocking nature:
/// Polls for the next file system event, blocking until one arrives. /// Returns `None` if the event is filtered out as irrelevant. fn poll(&mut self) -> Option<WatcherEvent> {For errors, consider logging them:
- let event = self.rx.iter().filter_map(Result::ok).find(|event| { + let event = self.rx.iter().find_map(|result| { + match result { + Ok(event) => Some(event), + Err(e) => { + eprintln!("Watch error: {}", e); + None + } + } + }).find(|event| {
9-12: Add rustdoc documentation for the struct and implementation.As per coding guidelines, the struct and its methods should have rustdoc comments explaining their purpose and behaviour, particularly the blocking semantics of
poll().Based on coding guidelines.
📚 Example documentation
+/// Default file system watcher implementation using the notify crate. +/// +/// Watches paths recursively and reports file creation, modification, and removal events. pub(crate) struct DefaultWatcher { rx: Receiver<Result<Event>>, watcher: Box<dyn notify::Watcher>, }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/biome_cli/src/commands/lint.rscrates/biome_cli/src/runner/impls/watchers/default.rscrates/biome_cli/src/runner/watcher.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_cli/src/runner/watcher.rs
- crates/biome_cli/src/commands/lint.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_cli/src/runner/impls/watchers/default.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Implement watcher tests for workspace methods in watcher.tests.rs and end-to-end tests in LSP tests
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace_watcher.rs : Use WorkspaceWatcher to keep workspace state in sync with the filesystem, and only activate it in daemon mode
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Debug the WorkspaceWatcher by starting the daemon with cargo run --bin=biome -- start and running commands such as cargo run --bin=biome -- lint --use-server <path>
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace*.rs : Implement the Workspace trait in the Biome Service to manage internal state of projects, including open documents, project layout instances, and module graph instances
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Implement watcher tests for workspace methods in watcher.tests.rs and end-to-end tests in LSP tests
Applied to files:
crates/biome_cli/src/runner/impls/watchers/default.rs
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace_watcher.rs : Use WorkspaceWatcher to keep workspace state in sync with the filesystem, and only activate it in daemon mode
Applied to files:
crates/biome_cli/src/runner/impls/watchers/default.rs
🧬 Code graph analysis (1)
crates/biome_cli/src/runner/impls/watchers/default.rs (1)
crates/biome_cli/src/runner/watcher.rs (6)
new(9-9)new(20-20)watch(12-12)watch(22-22)poll(16-16)poll(24-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (26)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: End-to-end tests
- GitHub Check: Documentation
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_tailwind_parser)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_html_analyze)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: autofix
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
Comment on lines
+50
to
+56
| Some(WatcherEvent { | ||
| paths: event | ||
| .paths | ||
| .into_iter() | ||
| .filter_map(|path| Utf8PathBuf::from_path_buf(path).ok()) | ||
| .collect(), | ||
| }) |
Contributor
There was a problem hiding this comment.
Non-UTF-8 paths are silently ignored.
Line 54: Paths that cannot be converted to UTF-8 are silently dropped with .ok(). Whilst uncommon, this could cause file changes to be missed without any indication to the user.
Consider logging when paths are dropped:
.paths
.into_iter()
- .filter_map(|path| Utf8PathBuf::from_path_buf(path).ok())
+ .filter_map(|path| {
+ Utf8PathBuf::from_path_buf(path.clone()).or_else(|_| {
+ eprintln!("Warning: Ignoring non-UTF-8 path: {:?}", path);
+ Err(path)
+ }).ok()
+ })
.collect(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Some(WatcherEvent { | |
| paths: event | |
| .paths | |
| .into_iter() | |
| .filter_map(|path| Utf8PathBuf::from_path_buf(path).ok()) | |
| .collect(), | |
| }) | |
| Some(WatcherEvent { | |
| paths: event | |
| .paths | |
| .into_iter() | |
| .filter_map(|path| { | |
| Utf8PathBuf::from_path_buf(path.clone()).or_else(|_| { | |
| eprintln!("Warning: Ignoring non-UTF-8 path: {:?}", path); | |
| Err(path) | |
| }).ok() | |
| }) | |
| .collect(), | |
| }) |
🤖 Prompt for AI Agents
In @crates/biome_cli/src/runner/impls/watchers/default.rs around lines 50 - 56,
The current construction of WatcherEvent drops non-UTF-8 paths silently by
calling Utf8PathBuf::from_path_buf(path).ok() inside the filter_map; update this
to detect conversion failures and log a warning when a path is dropped. Replace
the filter_map call in the WatcherEvent creation (the block creating
Some(WatcherEvent { paths: ... })) with code that attempts
Utf8PathBuf::from_path_buf for each path, collects successful conversions into
the paths vector, and emits a concise warning (e.g., tracing::warn! or
log::warn!) including the original PathBuf or its debug representation for any
conversion that fails so users are notified when non-UTF-8 paths are ignored.
Member
There was a problem hiding this comment.
@coderabbitai we work on the assumptions that all paths are UTF-8
Contributor
There was a problem hiding this comment.
@ematipico Understood. If UTF-8 paths are a repo invariant, then the current conversion is fine and my warning suggestion isn’t needed. If you want to make the invariant explicit for future readers, a short comment or an expect with a clear message would do; otherwise I’m happy to drop this.
✏️ Learnings added
Learnt from: ematipico
Repo: biomejs/biome PR: 8731
File: crates/biome_cli/src/runner/impls/watchers/default.rs:50-56
Timestamp: 2026-04-08T06:48:10.750Z
Learning: Repo biomejs/biome: Paths are assumed to be UTF-8 across the CLI and watcher code. Do not flag missing logs for non‑UTF‑8 paths in crates/biome_cli/src/runner/impls/watchers/default.rs; conversion failures are considered out of scope unless the policy changes.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace_watcher.rs : Use WorkspaceWatcher to keep workspace state in sync with the filesystem, and only activate it in daemon mode
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Implement watcher tests for workspace methods in watcher.tests.rs and end-to-end tests in LSP tests
Learnt from: ematipico
Repo: biomejs/biome PR: 9391
File: crates/biome_cli/src/runner/crawler.rs:86-87
Timestamp: 2026-03-08T16:05:37.629Z
Learning: In biomejs/biome, the crawler in `crates/biome_cli/src/runner/crawler.rs` uses a two-phase traversal: (1) evaluation phase — paths including directories and symlinks are traversed to discover files, with `PathKind` metadata used for routing; (2) handle phase — by this point, all paths in `evaluated_paths` are guaranteed to be files, so reconstructing `BiomePath` from a bare `PathBuf` (defaulting `path_kind` to `File`) in `TraversalScope::handle` is intentional and correct. Do not flag the loss of `path_kind` in the handle phase as a bug.
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-04-07T12:58:59.307Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/lint/**/*.rs : Avoid `unwrap()` and `expect()` in rule code that could panic; use combinators like `?`, `map`, or `let else` instead
Learnt from: dyc3
Repo: biomejs/biome PR: 8901
File: crates/biome_analyze_macros/src/group_macro.rs:9-9
Timestamp: 2026-01-29T22:10:30.888Z
Learning: Do not import xtask inside biome crates. In the biomeps repository, ensure that biome crates (e.g., crates/biome_*/...) do not have imports like use xtask_glue::... or other xtask-related crates. The correct dependency direction is for xtask to depend on and import biome crates, not the reverse, since xtask contains heavier utilities (e.g., git2) that can inflate build times when included in biome crates. When reviewing, verify dependency declarations (Cargo.toml) and any use statements to confirm xtask crates are not pulled into biome crates; prefer having xtask depend on biome crates and export needed functionality through stable interfaces.
Learnt from: jfmcdowell
Repo: biomejs/biome PR: 9783
File: crates/biome_markdown_parser/src/syntax/inline/html.rs:261-269
Timestamp: 2026-04-03T17:11:44.346Z
Learning: In the biomejs/biome codebase, when handling whitespace and character classification, use the whitespace/character classification dispatch tables from `biome_unicode_table::lookup_byte` (e.g., `Dispatch::WHS`) rather than ad-hoc raw byte literal comparisons (e.g., `b <= b' '` or explicit `b == b'\t'`). When reviewing this repo, do not treat `biome_unicode_table` dispatch variants as a spec-alignment issue—dispatch-table usage is the requested/accepted approach.
ematipico
reviewed
Mar 3, 2026
ematipico
left a comment
Member
There was a problem hiding this comment.
Sorry it took a long time, I completely lost it
ematipico
approved these changes
Apr 8, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/biome_cli/src/runner/mod.rs (2)
353-366: Consider adding debouncing to the watch loop.The current implementation processes each file event immediately. With auto-saving editors or bulk file operations, this could trigger many rapid re-runs. A short debounce window (e.g., 100–300ms) would coalesce related events and reduce unnecessary work.
This isn't blocking—the current behaviour is correct, just potentially chatty.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_cli/src/runner/mod.rs` around lines 353 - 366, The watch loop currently calls Self::Crawler::crawl for every poll() event which can cause rapid re-runs; introduce a short debounce window (e.g., 100–300ms) to coalesce successive watcher.poll() events into a single crawl invocation: buffer incoming event.paths (convert to CrawlPath::Path as currently done) into a temporary collection, wait the debounce duration (using a timer or sleep in the same task/loop) while collecting additional polls, then call self.collector(...) and Self::Crawler::crawl once with the merged paths to produce the CrawlerOutput; update the loop around watcher.poll(), event.paths handling, and the call site of Self::Crawler::crawl to use the merged path list so rapid events are coalesced.
133-133: Tests would be valuable for the watch mode feature.As per coding guidelines, code changes should include appropriate tests. For watch mode, consider:
- Unit tests for the watcher abstraction
- Integration tests verifying the watch loop correctly re-processes changed files
The PR summary mentions uncertainty about testing—the learnings suggest
watcher.tests.rsand end-to-end tests in LSP tests as potential homes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_cli/src/runner/mod.rs` at line 133, Add tests for the newly introduced watch functionality: create unit tests for the watcher abstraction (e.g., in a watcher.tests.rs module) that exercise the Watcher trait implementation(s) and verify that the watch loop (watcher::watch_loop / any start_watch function) detects file changes and enqueues/re-processes work; additionally add an end-to-end/integration test in the LSP test suite that simulates file edits and asserts the watch mode re-runs processing and updates outputs. Mock file system events where possible, assert expected calls/state transitions on the watcher implementation, and ensure the tests cover success and error paths for change handling and debounce behavior.
🤖 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_cli/src/runner/mod.rs`:
- Around line 353-366: The watch loop currently calls Self::Crawler::crawl for
every poll() event which can cause rapid re-runs; introduce a short debounce
window (e.g., 100–300ms) to coalesce successive watcher.poll() events into a
single crawl invocation: buffer incoming event.paths (convert to CrawlPath::Path
as currently done) into a temporary collection, wait the debounce duration
(using a timer or sleep in the same task/loop) while collecting additional
polls, then call self.collector(...) and Self::Crawler::crawl once with the
merged paths to produce the CrawlerOutput; update the loop around
watcher.poll(), event.paths handling, and the call site of Self::Crawler::crawl
to use the merged path list so rapid events are coalesced.
- Line 133: Add tests for the newly introduced watch functionality: create unit
tests for the watcher abstraction (e.g., in a watcher.tests.rs module) that
exercise the Watcher trait implementation(s) and verify that the watch loop
(watcher::watch_loop / any start_watch function) detects file changes and
enqueues/re-processes work; additionally add an end-to-end/integration test in
the LSP test suite that simulates file edits and asserts the watch mode re-runs
processing and updates outputs. Mock file system events where possible, assert
expected calls/state transitions on the watcher implementation, and ensure the
tests cover success and error paths for change handling and debounce behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 824390b6-9d9e-47dc-a859-8c8d772b97b9
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lockand included by**
📒 Files selected for processing (10)
Cargo.tomlcrates/biome_cli/Cargo.tomlcrates/biome_cli/src/commands/check.rscrates/biome_cli/src/commands/format.rscrates/biome_cli/src/commands/lint.rscrates/biome_cli/src/commands/mod.rscrates/biome_cli/src/lib.rscrates/biome_cli/src/runner/crawler.rscrates/biome_cli/src/runner/impls/commands/traversal.rscrates/biome_cli/src/runner/mod.rs
✅ Files skipped from review due to trivial changes (2)
- crates/biome_cli/Cargo.toml
- Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/biome_cli/src/commands/lint.rs
- crates/biome_cli/src/commands/format.rs
- crates/biome_cli/src/commands/check.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_cli/src/runner/impls/watchers/default.rs`:
- Around line 1-60: Add unit tests that exercise DefaultWatcher behavior: create
tests for poll() filtering and error handling by instantiating DefaultWatcher,
sending synthetic notify::Event values through the underlying channel (use the
rx/tx created in new()), and asserting poll() returns Some(WatcherEvent) only
for accepted EventKind variants (Create(File/Any), Modify(Name/Data/Any),
Remove(File/Any), Any) and filters out ignored kinds; also add tests that
simulate path conversion failures and watcher path add/commit errors by invoking
watch() with invalid paths or mocking the watcher to return Err, asserting
appropriate warnings are logged and that poll() handles Result::Err from the
channel (i.e., ignores Result::Err and continues). Reference DefaultWatcher,
new(), watch(), poll(), and WatcherEvent in the tests.
- Around line 38-50: The current use of self.rx.iter().filter_map(Result::ok)
silently drops notify errors; change the iterator logic to inspect each Result
from self.rx.iter(), explicitly log Err cases with warn!(...) including the
error value (so backend failures/overflow/I/O/watch-limit events are visible),
and only evaluate the Ok(event) for matching the EventKind predicate (the same
matches! expression currently used). Specifically replace filter_map(Result::ok)
with iteration/closure that matches on Result::Err(e) => { warn!("watcher error:
{:?}", e); None } and Result::Ok(ev) =>
Some(ev).then_apply_the_existing_find/match against event.kind so errors are
logged and valid events still trigger the existing behavior.
🪄 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: 187a97a9-1d66-401c-b87e-5ce9558f8566
📒 Files selected for processing (1)
crates/biome_cli/src/runner/impls/watchers/default.rs
Comment on lines
+1
to
+60
| use std::sync::mpsc::{Receiver, channel}; | ||
|
|
||
| use camino::Utf8PathBuf; | ||
| use notify::event::{CreateKind, ModifyKind, RemoveKind}; | ||
| use notify::{Event, EventKind, RecursiveMode, Result, recommended_watcher}; | ||
| use tracing::warn; | ||
|
|
||
| use crate::runner::watcher::{Watcher, WatcherEvent}; | ||
|
|
||
| pub(crate) struct DefaultWatcher { | ||
| rx: Receiver<Result<Event>>, | ||
| watcher: Box<dyn notify::Watcher>, | ||
| } | ||
|
|
||
| impl Watcher for DefaultWatcher { | ||
| fn new() -> Self { | ||
| let (tx, rx) = channel(); | ||
| Self { | ||
| rx, | ||
| watcher: Box::new(recommended_watcher(tx).expect("watcher created")), | ||
| } | ||
| } | ||
|
|
||
| fn watch(&mut self, paths: impl IntoIterator<Item = Utf8PathBuf>) { | ||
| let mut watched_paths = self.watcher.paths_mut(); | ||
|
|
||
| for path in paths { | ||
| if let Err(e) = watched_paths.add(path.as_std_path(), RecursiveMode::Recursive) { | ||
| warn!("Failed to watch path {}: {}", path, e); | ||
| } | ||
| } | ||
| if let Err(e) = watched_paths.commit() { | ||
| warn!("Failed to commit watched paths: {}", e); | ||
| } | ||
| } | ||
|
|
||
| fn poll(&mut self) -> Option<WatcherEvent> { | ||
| let event = self.rx.iter().filter_map(Result::ok).find(|event| { | ||
| // Modifying folder or metadata is ignored as it can unlikely affect the results. | ||
| // Any event types are necessary for some platforms to catch events. | ||
| matches!( | ||
| event.kind, | ||
| EventKind::Create(CreateKind::File | CreateKind::Any) | ||
| | EventKind::Modify( | ||
| ModifyKind::Name(_) | ModifyKind::Data(_) | ModifyKind::Any | ||
| ) | ||
| | EventKind::Remove(RemoveKind::File | RemoveKind::Any) | ||
| | EventKind::Any | ||
| ) | ||
| })?; | ||
|
|
||
| Some(WatcherEvent { | ||
| paths: event | ||
| .paths | ||
| .into_iter() | ||
| .filter_map(|path| Utf8PathBuf::from_path_buf(path).ok()) | ||
| .collect(), | ||
| }) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Please add automated tests for the new watcher behaviour.
This introduces a new execution path for CLI watch mode, but no tests are present in the provided changes. At minimum, add coverage for event-kind filtering and error-path handling in poll().
As per coding guidelines: “All code changes MUST include appropriate tests: … and bug fixes require tests that reproduce and validate the fix.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_cli/src/runner/impls/watchers/default.rs` around lines 1 - 60,
Add unit tests that exercise DefaultWatcher behavior: create tests for poll()
filtering and error handling by instantiating DefaultWatcher, sending synthetic
notify::Event values through the underlying channel (use the rx/tx created in
new()), and asserting poll() returns Some(WatcherEvent) only for accepted
EventKind variants (Create(File/Any), Modify(Name/Data/Any), Remove(File/Any),
Any) and filters out ignored kinds; also add tests that simulate path conversion
failures and watcher path add/commit errors by invoking watch() with invalid
paths or mocking the watcher to return Err, asserting appropriate warnings are
logged and that poll() handles Result::Err from the channel (i.e., ignores
Result::Err and continues). Reference DefaultWatcher, new(), watch(), poll(),
and WatcherEvent in the tests.
Comment on lines
+38
to
+50
| let event = self.rx.iter().filter_map(Result::ok).find(|event| { | ||
| // Modifying folder or metadata is ignored as it can unlikely affect the results. | ||
| // Any event types are necessary for some platforms to catch events. | ||
| matches!( | ||
| event.kind, | ||
| EventKind::Create(CreateKind::File | CreateKind::Any) | ||
| | EventKind::Modify( | ||
| ModifyKind::Name(_) | ModifyKind::Data(_) | ModifyKind::Any | ||
| ) | ||
| | EventKind::Remove(RemoveKind::File | RemoveKind::Any) | ||
| | EventKind::Any | ||
| ) | ||
| })?; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the latest notify crate docs, which watcher failures are delivered as Err events (e.g. overflow/backend errors), and are these expected to be surfaced to users rather than ignored?
💡 Result:
In the latest notify docs (v8.2.0 on docs.rs), watcher “failures” that come through the callback/channel as Err(...) events are delivered as a notify::Error (i.e. the EventHandler is invoked with Result<Event>, so error notifications are first-class items in the event stream). (docs.rs)
What gets delivered as Err events?
notify::Error in the event stream can have these ErrorKind variants: (docs.rs)
ErrorKind::Io(std::io::Error)— backend / OS / filesystem I/O failures. (docs.rs)ErrorKind::Generic(String)— “opaque internal errors” and cases where a platform-specific error is mapped into a generic one. (This is where backend-specific conditions such as queue overflow / dropped-events-style conditions would typically end up if they can’t be represented more specifically.) (docs.rs)ErrorKind::MaxFilesWatch— inotify watch-limit reached (can’t watch more files). (docs.rs)ErrorKind::PathNotFound— watched path doesn’t exist. (docs.rs)ErrorKind::WatchNotFound— attempted to remove a watch that doesn’t exist. (docs.rs)ErrorKind::InvalidConfig(Config)— invalid runtime configuration. (docs.rs)
Also, the docs explicitly state that errors are emitted not only at watcher creation, but also “during the event stream”, and may be associated with specific paths. (docs.rs)
Are these expected to be surfaced to users (vs ignored)?
Yes: the crate docs’ examples show consumers matching on Result<Event> and printing/logging Err(e) as a “watch error”, which indicates these are intended to be observable, user-handled conditions rather than silently ignored. (docs.rs)
Citations:
- 1: https://docs.rs/notify/latest/notify/trait.EventHandler.html
- 2: https://docs.rs/notify/latest/notify/enum.ErrorKind.html
- 3: https://docs.rs/notify/latest/notify/enum.ErrorKind.html
- 4: https://docs.rs/notify/latest/notify/enum.ErrorKind.html
- 5: https://docs.rs/notify/latest/notify/enum.ErrorKind.html
- 6: https://docs.rs/notify/latest/notify/enum.ErrorKind.html
- 7: https://docs.rs/notify/latest/notify/enum.ErrorKind.html
- 8: https://docs.rs/notify/latest/notify/enum.ErrorKind.html
- 9: https://docs.rs/notify/latest/notify/struct.Error.html
- 10: https://docs.rs/notify/latest/notify/
🏁 Script executed:
cat -n crates/biome_cli/src/runner/impls/watchers/default.rs | head -60Repository: biomejs/biome
Length of output: 2479
🏁 Script executed:
# Check if warn!() macro is imported and used in this file
head -20 crates/biome_cli/src/runner/impls/watchers/default.rsRepository: biomejs/biome
Length of output: 645
🏁 Script executed:
# Search for logging/warn! patterns in similar watcher code or nearby files
rg "warn!" crates/biome_cli/src/runner/impls/watchers/ -A 1 -B 1Repository: biomejs/biome
Length of output: 673
🏁 Script executed:
# Check what logging infrastructure is used in the biome_cli crate
rg "use.*log|use.*warn|use.*tracing" crates/biome_cli/src/ -m 5Repository: biomejs/biome
Length of output: 2177
Handle filesystem watcher errors instead of silently dropping them in line 38.
filter_map(Result::ok) discards all notify errors—backend failures, overflow, I/O issues, and watch-limit conditions get swallowed without a trace. The notify crate explicitly delivers these as first-class Err events in the stream for a reason; users deserve to know when their watcher is choking.
Replace with explicit error handling and logging (using warn!(), which is already imported on line 6 and used consistently on lines 29–33):
Suggested fix
fn poll(&mut self) -> Option<WatcherEvent> {
- let event = self.rx.iter().filter_map(Result::ok).find(|event| {
- // Modifying folder or metadata is ignored as it can unlikely affect the results.
- // Any event types are necessary for some platforms to catch events.
- matches!(
- event.kind,
- EventKind::Create(CreateKind::File | CreateKind::Any)
- | EventKind::Modify(
- ModifyKind::Name(_) | ModifyKind::Data(_) | ModifyKind::Any
- )
- | EventKind::Remove(RemoveKind::File | RemoveKind::Any)
- | EventKind::Any
- )
- })?;
+ let event = self.rx.iter().find_map(|message| {
+ let event = match message {
+ Ok(event) => event,
+ Err(error) => {
+ warn!("Filesystem watcher error: {error}");
+ return None;
+ }
+ };
+
+ // Modifying folder or metadata is ignored as it can unlikely affect the results.
+ // Any event types are necessary for some platforms to catch events.
+ matches!(
+ event.kind,
+ EventKind::Create(CreateKind::File | CreateKind::Any)
+ | EventKind::Modify(
+ ModifyKind::Name(_) | ModifyKind::Data(_) | ModifyKind::Any
+ )
+ | EventKind::Remove(RemoveKind::File | RemoveKind::Any)
+ | EventKind::Any
+ )
+ .then_some(event)
+ })?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_cli/src/runner/impls/watchers/default.rs` around lines 38 - 50,
The current use of self.rx.iter().filter_map(Result::ok) silently drops notify
errors; change the iterator logic to inspect each Result from self.rx.iter(),
explicitly log Err cases with warn!(...) including the error value (so backend
failures/overflow/I/O/watch-limit events are visible), and only evaluate the
Ok(event) for matching the EventKind predicate (the same matches! expression
currently used). Specifically replace filter_map(Result::ok) with
iteration/closure that matches on Result::Err(e) => { warn!("watcher error:
{:?}", e); None } and Result::Ok(ev) =>
Some(ev).then_apply_the_existing_find/match against event.kind so errors are
logged and valid events still trigger the existing behavior.
Member
Author
|
@ematipico Thank you so much for your follow-up. Even I don't remember this pull request... |
Member
|
Oh no! I was still working on it! No problem, I will send another PR. The current implementation isn't testable |
Member
Author
|
@ematipico My bad, happy to review your pull request once it's ready! |
Merged
Merged
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
Resolves #135
Added the new option
--watchto enable watch mode that watches file changes and re-run the traversal automatically.Test Plan
Manually tested. I'm not sure if this can be tested in a test code.
Docs
I suppose it will be automatically added by the codegen