fix(fts): slice fuzzy query prefix on a char boundary#7892
Conversation
📝 WalkthroughWalkthroughChangesFuzzy prefix UTF-8 handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
rust/lance-index/src/scalar/inverted/index.rs-10807-10858 (1)
10807-10858: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExtend the regression test to validate prefix semantics, not only panic avoidance.
Because the test indexes and queries only
"über", it would pass even if prefix filtering were ignored. Add a near-edit-distance token such as"uber"and assert it is excluded withprefix_length = 1; also cover lengths exceeding the token’s character count and the stated"café"/CJK cases. As per coding guidelines, userstestfor the resulting parameterized Rust cases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance-index/src/scalar/inverted/index.rs` around lines 10807 - 10858, The regression coverage in test_fuzzy_prefix_length_handles_multibyte_query_token only verifies that “über” matches; parameterize the test with rstest to validate prefix filtering as well. Add a near-edit-distance “uber” candidate and assert it is excluded for prefix_length = 1, and include cases where the prefix length exceeds token character counts plus the documented “café” and CJK multibyte scenarios, preserving the expected match/exclusion results for each case.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.
Other comments:
In `@rust/lance-index/src/scalar/inverted/index.rs`:
- Around line 10807-10858: The regression coverage in
test_fuzzy_prefix_length_handles_multibyte_query_token only verifies that “über”
matches; parameterize the test with rstest to validate prefix filtering as well.
Add a near-edit-distance “uber” candidate and assert it is excluded for
prefix_length = 1, and include cases where the prefix length exceeds token
character counts plus the documented “café” and CJK multibyte scenarios,
preserving the expected match/exclusion results for each case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: 07f90237-f730-494c-8d23-fd7b38bd2606
📒 Files selected for processing (1)
rust/lance-index/src/scalar/inverted/index.rs
`collect_fuzzy_candidates` cut the query token with
`&token[..prefix_length]`, treating the character-count `prefix_length`
as a byte offset. When the cut landed inside a multibyte UTF-8 character
the slice panicked ("byte index N is not a char boundary"), so a fuzzy
full-text search with `prefix_length > 0` crashed on any query term
beginning with a non-ASCII character (e.g. "über", "café").
Advance `prefix_length` characters via `char_indices` and slice on the
resulting boundary, clamping to the whole token when it is shorter. Adds
a regression test covering a multibyte query token.
49b8971 to
bad1b1f
Compare
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
rust/lance-index/src/scalar/inverted/index.rs-10807-10858 (1)
10807-10858: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExtend the regression test to cover all promised UTF-8 cases.
This verifies the original
überpanic, but not oversizedprefix_lengthclamping or other multibyte layouts. Add cases such ascaféwith an oversized prefix and CJK text, asserting each expected match. As per coding guidelines, userstestfor Rust parameterized tests.🤖 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/index.rs` around lines 10807 - 10858, Extend test_fuzzy_prefix_length_handles_multibyte_query_token into an rstest-parameterized regression test covering the existing über case, café with a prefix_length larger than the token, and a CJK token with a multibyte prefix boundary. Parameterize the indexed token, query token, prefix length, and expected row ID, then assert each bm25_search call returns the corresponding match while preserving the existing setup and regression coverage.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.
Other comments:
In `@rust/lance-index/src/scalar/inverted/index.rs`:
- Around line 10807-10858: Extend
test_fuzzy_prefix_length_handles_multibyte_query_token into an
rstest-parameterized regression test covering the existing über case, café with
a prefix_length larger than the token, and a CJK token with a multibyte prefix
boundary. Parameterize the indexed token, query token, prefix length, and
expected row ID, then assert each bm25_search call returns the corresponding
match while preserving the existing setup and regression coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: 75f8dafb-649a-483a-9f26-9c7ec909d96c
📒 Files selected for processing (1)
rust/lance-index/src/scalar/inverted/index.rs
What
Fixes a panic in fuzzy full-text search: a
MatchQuerywithfuzziness > 0andprefix_length > 0crashes when the query term starts with a multibyte UTF-8 character (e.g.über,café).Closes #7891.
Why
InvertedPartition::collect_fuzzy_candidatestook the leading prefix of the query token with a raw byte slice, butprefix_lengthis a character count. When the byte offset falls inside a multibyte character the slice panics, aborting the query while expanding the term.How
Advance
prefix_lengthcharacters withchar_indices()and slice on the resulting boundary, clamping to the whole token when it is shorter.Testing
cargo test -p lance-index --lib fuzzy,clippy -D warnings, andrustfmtall clean.