feat: add code analyzer for FTS#7681
Conversation
github-actions
Bot
added
A-python
|
Important This PR touches the Lance format specification. Substantive changes to the format specification — the If this is a meaningful format change:
|
BubbleCal
left a comment
BubbleCal
left a comment
There was a problem hiding this comment.
Two concise inline comments on the correctness issues found.
| }) | ||
| } | ||
|
|
||
| fn canonicalize_inverted_index_details( |
There was a problem hiding this comment.
This roundtrip still preserves a missing lance_tokenizer instead of canonicalizing it to "text". A text segment with None can be compared against a later segment with Some("text"), causing multi-segment FTS to fail as inconsistent.
| @@ -1895,12 +1858,26 @@ | |||
There was a problem hiding this comment.
This grouped-query path unions posting lists without preserving positions. Code analyzer phrase queries can hit it when a full identifier and subword share a position, and WAND later expects positions to exist.
|
I'd think we should disable |
Makes sense to me |
|
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:
📝 WalkthroughWalkthroughThis PR adds code-aware full-text indexing and search support across schema, tokenizer construction, query execution, mem-WAL search, and legacy segment-detail handling. ChangesCode Analyzer FTS Pipeline
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant PythonDataset
participant InvertedIndexParams
participant CodeLexTokenizer
participant WordDelimiterFilter
PythonDataset->>InvertedIndexParams: parse analyzer, base_tokenizer, and code flags
InvertedIndexParams->>CodeLexTokenizer: build code tokenizer for indexed text
InvertedIndexParams->>WordDelimiterFilter: wrap identifier splitting when enabled
InvertedIndexParams-->>PythonDataset: validated index parameters
sequenceDiagram
participant MemWALFTS
participant Tokens
participant GroupedSearch
participant PhraseSearch
MemWALFTS->>Tokens: analyze query text with positions
Tokens->>GroupedSearch: grouped AND token positions
Tokens->>PhraseSearch: grouped phrase alternatives
GroupedSearch-->>MemWALFTS: intersected row matches
PhraseSearch-->>MemWALTS: phrase matches by group
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
rust/lance-tokenizer/src/word_delimiter_filter.rs (2)
8-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the public filter API with an example.
WordDelimiterFilteris publicly re-exported, and the wrapper/stream types are public; add example docs that show how it composes with [CodeLexTokenizer] / [TextAnalyzer].As per coding guidelines, "All public APIs must have documentation with examples, and documentation should link to relevant structs and methods."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance-tokenizer/src/word_delimiter_filter.rs` around lines 8 - 61, Add documentation comments with a runnable example for the public WordDelimiterFilter API, since WordDelimiterFilter, WordDelimiterFilterWrapper, and WordDelimiterTokenStream are publicly exposed. Update the docs on WordDelimiterFilter and its constructor/transform path to show how it composes with CodeLexTokenizer and TextAnalyzer, and include proper intra-doc links to the relevant types and methods so the public API is fully documented.Source: Coding guidelines
200-203: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid
expectin library token-stream code.
refill()establishes the invariant, but this is still a library panic path. Prefer a non-panicking invariant check.Suggested refactor
- self.current = self - .pending - .pop_front() - .expect("pending token should be available after refill"); + let Some(token) = self.pending.pop_front() else { + debug_assert!(false, "pending token should be available after refill"); + return false; + }; + self.current = token;As per coding guidelines, "Never use
.unwrap(),.expect(),panic!(), orassert!()in library code for fallible operations."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance-tokenizer/src/word_delimiter_filter.rs` around lines 200 - 203, Replace the `expect` in `WordDelimiterFilter::refill`/token advancement with a non-panicking invariant check so library code never panics on fallible state. Use the existing `pending` queue and `self.current` assignment path to handle the empty case gracefully, returning an appropriate error/option-like flow instead of `expect("pending token should be available after refill")`. Keep the fix localized to `word_delimiter_filter.rs` and preserve the refill invariant without introducing `panic!`, `assert!`, or `unwrap`-style behavior.Source: Coding guidelines
rust/lance-index/src/scalar/inverted/tokenizer.rs (2)
510-545: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRemove
expectfrom public builder helpers.These calls are currently invariant-based, but public library code should avoid panic paths. A small private
apply_code_defaults()helper can set the fields directly.Suggested direction
+ fn apply_code_defaults(&mut self) { + self.analyzer = "code".to_string(); + self.base_tokenizer = "code".to_string(); + self.split_identifiers = false; + self.split_on_numerics = true; + self.preserve_original = true; + self.lower_case = true; + self.ascii_folding = true; + self.stem = false; + self.remove_stop_words = false; + self.index_operators = false; + } + /// Create parameters for the code analyzer profile. pub fn code() -> Self { - Self::new("code".to_string(), Language::English) - .analyzer("code") - .expect("code analyzer should be valid") + let mut params = Self::new("code".to_string(), Language::English); + params.apply_code_defaults(); + params }As per coding guidelines, "Never use
.unwrap(),.expect(),panic!(), orassert!()in library code for fallible operations."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance-index/src/scalar/inverted/tokenizer.rs` around lines 510 - 545, Remove the panic-based `expect` calls from the public builder helpers in `TokenizerOptions::code()` and `TokenizerOptions::base_tokenizer()`. Replace the `analyzer("code")` round-trip with a private helper such as `apply_code_defaults()` that sets the code profile fields directly, and call that helper from both places so these library APIs remain fallible-free.Source: Coding guidelines
510-619: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd examples for the new analyzer builder API.
The new public
code(),analyzer(...), and code-flag setters should document valid combinations and include examples for default code analysis and identifier splitting.As per coding guidelines, "All public APIs must have documentation with examples, and documentation should link to relevant structs and methods."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance-index/src/scalar/inverted/tokenizer.rs` around lines 510 - 619, The public builder API in TokenizerConfig is missing the required docs/examples for the new analyzer flow. Add documentation examples to the relevant public methods in tokenizer.rs, especially code(), analyzer(...), and the code-related setters like base_tokenizer(), split_identifiers(), split_on_numerics(), preserve_original(), and index_operators(), showing valid combinations for default code analysis and identifier splitting. Make sure the doc comments link to the TokenizerConfig type and the related builder methods so the public API docs are complete.Source: Coding guidelines
rust/lance-tokenizer/src/code_tokenizer.rs (1)
8-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd example docs for the exported tokenizer API.
CodeLexTokenizeris publicly re-exported, andCodeLexTokenStreamis public, but the public docs do not include an example or cross-links.Suggested doc shape
/// Tokenizer for code-like text. /// /// Identifiers are Unicode alphanumeric characters plus `_`. Other characters /// are lexical boundaries. When operator indexing is enabled, contiguous /// operator characters are emitted as tokens too. +/// +/// # Examples +/// +/// ``` +/// use lance_tokenizer::{CodeLexTokenizer, TextAnalyzer, TokenStream}; +/// +/// let mut analyzer = TextAnalyzer::builder(CodeLexTokenizer::new(true)).build(); +/// let mut stream = analyzer.token_stream("a::b"); +/// +/// assert!(stream.advance()); +/// assert_eq!(stream.token().text, "a"); +/// ``` #[derive(Clone, Default)] pub struct CodeLexTokenizer { @@ -pub struct CodeLexTokenStream<'a> { +/// Token stream produced by [`CodeLexTokenizer`]. +pub struct CodeLexTokenStream<'a> {As per coding guidelines, "All public APIs must have documentation with examples, and documentation should link to relevant structs and methods."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance-tokenizer/src/code_tokenizer.rs` around lines 8 - 28, Add documentation for the exported tokenizer API by expanding the doc comment on CodeLexTokenizer with a runnable usage example and relevant cross-links. Update the public docs so CodeLexTokenizer references TextAnalyzer and TokenStream, and add a short summary doc comment on CodeLexTokenStream that links back to CodeLexTokenizer. Keep the example aligned with the existing constructor and token stream flow so the public API is fully documented.Source: Coding guidelines
python/python/tests/test_scalar_index.py (1)
995-1052: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winParameterize the repeated search cases.
These assertions differ only by
column,query, andexpected_path; converting them to@pytest.mark.parametrizewill give each analyzer scenario an independent failure. While doing that, consider asserting the expected result set instead of only membership so an AND-to-OR regression cannot still pass. As per coding guidelines, “Use@pytest.mark.parametrizefor Python tests when cases differ only by inputs.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/python/tests/test_scalar_index.py` around lines 995 - 1052, The repeated search assertions in the test around assert_search should be converted into a single `@pytest.mark.parametrize-driven` test so each column/query/expected_path case runs independently. Refactor the helper or inline the scanner call under a parametrized test using the existing assert_search logic, and strengthen the check to verify the full expected path set rather than only membership so an AND-to-OR regression cannot slip through.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@protos/index_old.proto`:
- Around line 59-66: The code analyzer flags in the proto need to be
presence-aware because bare proto3 bools treat omitted legacy values as false,
which can override the defaults used by InvertedIndexParams::code(). Update
split_on_numerics and preserve_original in the InvertedIndexDetails schema to
optional bool, then adjust the Rust conversion logic for
InvertedIndexParams::code() so None maps back to the code analyzer defaults
instead of false.
In `@python/src/dataset.rs`:
- Around line 2451-2464: The current analyzer/base_tokenizer validation in the
dataset setup still allows code-only flags to be enabled when the effective
analyzer is not code. Update the validation around the analyzer resolution path
and the setters for split_identifiers, split_on_numerics, preserve_original, and
index_operators so they reject any of these options unless analyzer resolves to
code, matching the upstream contract and the existing PyValueError style used in
this block. Use the existing analyzer/base_tokenizer checks in dataset
configuration to centralize the guard before applying those code analyzer flags.
In `@rust/lance-index/src/scalar/inverted/index.rs`:
- Around line 6030-6041: The new public API
flat_bm25_search_stream_with_metrics_and_operator has documentation but no usage
example, so add a minimal doc example to its Rustdoc showing how to call it with
a stream, tokenizer, scorer, and Operator, and link to the relevant
types/methods used. If this function is not meant to be public, reduce its
visibility instead of leaving an undocumented public entrypoint.
- Around line 1960-1963: The union-docs lookup in the grouped-query path is
using an expect that can panic in library code if the invariant is broken.
Update the code in the inverted index query flow around docs_for_union to return
a proper Result error instead of calling expect, and propagate it with ? through
the surrounding function that builds the grouped query terms. Use the existing
query-building symbols in index.rs so the failure becomes a fallible error path
rather than a process panic.
---
Nitpick comments:
In `@python/python/tests/test_scalar_index.py`:
- Around line 995-1052: The repeated search assertions in the test around
assert_search should be converted into a single `@pytest.mark.parametrize-driven`
test so each column/query/expected_path case runs independently. Refactor the
helper or inline the scanner call under a parametrized test using the existing
assert_search logic, and strengthen the check to verify the full expected path
set rather than only membership so an AND-to-OR regression cannot slip through.
In `@rust/lance-index/src/scalar/inverted/tokenizer.rs`:
- Around line 510-545: Remove the panic-based `expect` calls from the public
builder helpers in `TokenizerOptions::code()` and
`TokenizerOptions::base_tokenizer()`. Replace the `analyzer("code")` round-trip
with a private helper such as `apply_code_defaults()` that sets the code profile
fields directly, and call that helper from both places so these library APIs
remain fallible-free.
- Around line 510-619: The public builder API in TokenizerConfig is missing the
required docs/examples for the new analyzer flow. Add documentation examples to
the relevant public methods in tokenizer.rs, especially code(), analyzer(...),
and the code-related setters like base_tokenizer(), split_identifiers(),
split_on_numerics(), preserve_original(), and index_operators(), showing valid
combinations for default code analysis and identifier splitting. Make sure the
doc comments link to the TokenizerConfig type and the related builder methods so
the public API docs are complete.
In `@rust/lance-tokenizer/src/code_tokenizer.rs`:
- Around line 8-28: Add documentation for the exported tokenizer API by
expanding the doc comment on CodeLexTokenizer with a runnable usage example and
relevant cross-links. Update the public docs so CodeLexTokenizer references
TextAnalyzer and TokenStream, and add a short summary doc comment on
CodeLexTokenStream that links back to CodeLexTokenizer. Keep the example aligned
with the existing constructor and token stream flow so the public API is fully
documented.
In `@rust/lance-tokenizer/src/word_delimiter_filter.rs`:
- Around line 8-61: Add documentation comments with a runnable example for the
public WordDelimiterFilter API, since WordDelimiterFilter,
WordDelimiterFilterWrapper, and WordDelimiterTokenStream are publicly exposed.
Update the docs on WordDelimiterFilter and its constructor/transform path to
show how it composes with CodeLexTokenizer and TextAnalyzer, and include proper
intra-doc links to the relevant types and methods so the public API is fully
documented.
- Around line 200-203: Replace the `expect` in
`WordDelimiterFilter::refill`/token advancement with a non-panicking invariant
check so library code never panics on fallible state. Use the existing `pending`
queue and `self.current` assignment path to handle the empty case gracefully,
returning an appropriate error/option-like flow instead of `expect("pending
token should be available after refill")`. Keep the fix localized to
`word_delimiter_filter.rs` and preserve the refill invariant without introducing
`panic!`, `assert!`, or `unwrap`-style 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7095279a-fa1f-4d0e-be47-9cd5f7bb0d7a
📒 Files selected for processing (12)
protos/index_old.protopython/python/tests/test_scalar_index.pypython/src/dataset.rsrust/lance-index/src/scalar/inverted/index.rsrust/lance-index/src/scalar/inverted/tokenizer.rsrust/lance-tokenizer/src/code_tokenizer.rsrust/lance-tokenizer/src/lib.rsrust/lance-tokenizer/src/word_delimiter_filter.rsrust/lance/src/dataset/mem_wal/index.rsrust/lance/src/dataset/mem_wal/index/fts.rsrust/lance/src/index/scalar/inverted.rsrust/lance/src/io/exec/fts.rs
| match (analyzer.as_deref(), base_tokenizer.as_deref()) { | ||
| (Some("text"), Some("code")) => { | ||
| return Err(PyValueError::new_err( | ||
| "base_tokenizer='code' requires analyzer='code'", | ||
| )); | ||
| } | ||
| (Some("code"), Some(base_tokenizer)) if base_tokenizer != "code" => { | ||
| return Err(PyValueError::new_err(format!( | ||
| "analyzer='code' requires base_tokenizer='code', got '{}'", | ||
| base_tokenizer | ||
| ))); | ||
| } | ||
| _ => {} | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject code-only flags unless the effective analyzer is code.
The new validation handles analyzer/base_tokenizer conflicts, but split_identifiers, split_on_numerics, preserve_original, and index_operators can still be set while using the default/text analyzer. The upstream resolver rejects this combination as "code analyzer flags require analyzer='code'", so mirror that contract here before applying these setters.
Suggested fix
match (analyzer.as_deref(), base_tokenizer.as_deref()) {
(Some("text"), Some("code")) => {
return Err(PyValueError::new_err(
"base_tokenizer='code' requires analyzer='code'",
));
@@
}
_ => {}
}
+
+ let uses_code_analyzer =
+ analyzer.as_deref() == Some("code")
+ || base_tokenizer.as_deref() == Some("code");
+ if !uses_code_analyzer {
+ for flag in [
+ "split_identifiers",
+ "split_on_numerics",
+ "preserve_original",
+ "index_operators",
+ ] {
+ if kwargs
+ .get_item(flag)?
+ .map(|value| value.extract::<bool>())
+ .transpose()?
+ .unwrap_or(false)
+ {
+ return Err(PyValueError::new_err(
+ "code analyzer flags require analyzer='code'",
+ ));
+ }
+ }
+ }
if let Some(analyzer) = analyzer {
params = params
.analyzer(&analyzer)Also applies to: 2514-2525
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/src/dataset.rs` around lines 2451 - 2464, The current
analyzer/base_tokenizer validation in the dataset setup still allows code-only
flags to be enabled when the effective analyzer is not code. Update the
validation around the analyzer resolution path and the setters for
split_identifiers, split_on_numerics, preserve_original, and index_operators so
they reject any of these options unless analyzer resolves to code, matching the
upstream contract and the existing PyValueError style used in this block. Use
the existing analyzer/base_tokenizer checks in dataset configuration to
centralize the guard before applying those code analyzer flags.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| // e.g. "text" for a plain string or "json" for JSON content. The extracted | ||
| // text is then passed to base_tokenizer. Absent means text / auto-inferred | ||
| // legacy behavior. | ||
| optional string lance_tokenizer = 13; |
There was a problem hiding this comment.
can we just reuse lance_tokenizer?
There was a problem hiding this comment.
My current understanding:
lance_tokenizer: document-level extractor, text, jsonbase_tokenizer: lexical tokenizer, like simple, icuanalyzer: the high-level profile
For example, if we support code in json, we will have lance_tokenizer=json + analyzer=code. Do you think this makes sense?
There was a problem hiding this comment.
should we use token.position？
There was a problem hiding this comment.
🧹 Nitpick comments (2)
rust/lance/src/dataset/mem_wal/index/fts.rs (2)
2490-2528: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueSilent guards against conditions that cannot occur on this path.
Grouped phrase search is only reached when
self.params.has_positions()is true (guarded insearch_phrase_tokens, Line 1531), and any chunk withbatch_position < snap.visible_countis by construction backed by a visible batch. So bothlet Some(meta) = snap.batch_for(...) else { continue; }andlet Some(positions) = &chunk.positions else { continue; }silently skip states that should be impossible here, which can mask a real regression by dropping a doc instead of surfacing it. Prefer adebug_assert!/explicit error, or a comment stating when the branch legitimately triggers.As per coding guidelines: "Do not silently guard against impossible conditions; use
debug_assert!, return an explicit error, or remove the check." and "Comment fallback or guard code paths with when they trigger and why they exist."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance/src/dataset/mem_wal/index/fts.rs` around lines 2490 - 2528, The `tail_phrase_group_docs` path is silently continuing on states that should be impossible after `search_phrase_tokens` and the visible-batch check, which can hide regressions. Update the `snap.batch_for(...)` and `chunk.positions` branches to use a `debug_assert!`, return an explicit error, or add a clear comment explaining the legitimate fallback condition, rather than silently skipping the document.Source: Coding guidelines
1879-1889: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
analyze_for_searchtruncatest.positionwith an uncheckedas u32, unlike the insert path.The writer path now validates the
usize → u32position conversion viachecked_token_position(Line 1180), but the search path silently truncates withpositions.push(t.position as u32). In practice a search string can't realistically exceedu32::MAXtokens, so this is a consistency concern rather than a live bug. Consider reusingchecked_token_positionhere (or documenting why the search side is exempt) so both position pipelines behave identically.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance/src/dataset/mem_wal/index/fts.rs` around lines 1879 - 1889, analyze_for_search currently converts token positions with an unchecked usize-to-u32 cast, unlike the insert path. Update the search token collection in analyze_for_search to use the same checked conversion helper as the writer path (checked_token_position) or otherwise handle overflow consistently, so token position handling matches across both pipelines. Reference analyze_for_search, token_stream_for_search, and checked_token_position while making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@rust/lance/src/dataset/mem_wal/index/fts.rs`:
- Around line 2490-2528: The `tail_phrase_group_docs` path is silently
continuing on states that should be impossible after `search_phrase_tokens` and
the visible-batch check, which can hide regressions. Update the
`snap.batch_for(...)` and `chunk.positions` branches to use a `debug_assert!`,
return an explicit error, or add a clear comment explaining the legitimate
fallback condition, rather than silently skipping the document.
- Around line 1879-1889: analyze_for_search currently converts token positions
with an unchecked usize-to-u32 cast, unlike the insert path. Update the search
token collection in analyze_for_search to use the same checked conversion helper
as the writer path (checked_token_position) or otherwise handle overflow
consistently, so token position handling matches across both pipelines.
Reference analyze_for_search, token_stream_for_search, and
checked_token_position while making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ab0ee6a2-1de4-49bb-bcc4-34d7cc765e5d
📒 Files selected for processing (1)
rust/lance/src/dataset/mem_wal/index/fts.rs
2 participants
This PR adds a first-slice code analyzer profile for FTS so code-like text can be searched by identifiers and subwords instead of natural-language defaults.
The current text analyzer treats code poorly: camelCase/acronyms/numeric boundaries are not split as code identifiers, and stemming / stop-word removal can rewrite code tokens. The new
analyzer="code"profile resolves to a persisted tokenizer configuration, adds code lexical tokenization and identifier splitting, and wires the same analyzer through indexed, flat, MemWAL, and Python paths.The implementation also canonicalizes FTS segment details before comparing them, so existing text FTS segments that omit newly added default fields remain compatible with newly written text FTS segments.
Validation covered targeted Rust tokenizer/index/MemWAL/details tests, targeted Python FTS tests,
cargo fmt --all --check,cargo clippy --all --tests --benches -- -D warnings, and Python ruff checks.uv run make lintstill fails at pyright because this local environment does not provide optionaltensorflow, which affects existingpython/lance/arrow.pyandpython/tests/test_arrow.pyimports outside this change.Summary by CodeRabbit