Skip to content

chore(index): remove dead eager FM-index queries#7880

Open
Xuanwo wants to merge 2 commits into
mainfrom
xuanwo/remove-dead-fmindex-query
Open

chore(index): remove dead eager FM-index queries#7880
Xuanwo wants to merge 2 commits into
mainfrom
xuanwo/remove-dead-fmindex-query

Conversation

@Xuanwo

@Xuanwo Xuanwo commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

The production query path has moved to LazyFMIndex, but the build-time FMIndex still retained an eager query implementation used only as a test oracle. Maintaining both paths increased maintenance cost and left dead code hidden behind #[allow(dead_code)].

This removes the eager RankBitVec, HuffmanWaveletTree, and FMIndex query methods and their oracle-only tests. The FM-index builder, suffix-array sampling, serialization, and lazy production query path remain unchanged.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: 89ee9a9c-6821-44be-a899-6a8865e9d168

📥 Commits

Reviewing files that changed from the base of the PR and between aea6ded and b91901b.

📒 Files selected for processing (1)
  • rust/lance-index/src/scalar/fmindex.rs
💤 Files with no reviewable changes (1)
  • rust/lance-index/src/scalar/fmindex.rs

📝 Walkthrough

Walkthrough

The change removes non-lazy rank, wavelet-tree query, and in-memory FMIndex search and locate operations. Related search and query-consistency tests are deleted or reduced, while index construction and serialization remain.

Changes

FMIndex query path removal

Layer / File(s) Summary
Remove rank and wavelet query primitives
rust/lance-index/src/scalar/fmindex.rs
RankBitVec no longer exposes bit/rank queries, and HuffmanWaveletTree no longer exposes access or rank operations.
Remove FMIndex search implementation and coverage
rust/lance-index/src/scalar/fmindex.rs
FMIndex substring search and locate helpers are removed, along with tests covering search, locate, sentinel handling, and wavelet query consistency.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: removal of dead eager FM-index query code.
Description check ✅ Passed The description matches the changeset and explains the removed query paths and tests while noting what remains unchanged.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch xuanwo/remove-dead-fmindex-query

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

@github-actions github-actions Bot added A-index Vector index, linalg, tokenizer chore labels Jul 21, 2026
@Xuanwo
Xuanwo marked this pull request as ready for review July 21, 2026 09:56
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Xuanwo
Xuanwo requested a review from jackye1995 July 21, 2026 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-index Vector index, linalg, tokenizer chore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant