feat(search): add hybrid search with RRF fusion#224
Conversation
Combine FTS5 keyword search with sqlite-vec vector similarity via Reciprocal Rank Fusion (RRF). vault_search transparently upgrades to hybrid ranking when embeddings are available, falling back to FTS-only when they are not. New functions: - computeRrfScores: pure RRF with k=60 + top-rank bonuses - vectorSearch: embed query → KNN → best chunk per note - hybridSearch: orchestrates FTS + vector + RRF fusion Spike result: 8/9 previously-failed queries fixed, 0 regressions, ~8ms. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Tighten assertion quality across computeRrfScores and hybridSearch tests: decomposed assertions collapsed to toEqual, position-agnostic .find() replaced with positional checks, loose matchers (toBeTruthy, toBeDefined) replaced with exact fixture values, conditional assertion on leading_callout made unconditional. Add coverage for notePassesFilters related/properties branches and snippet_tokens truncation behavior. Use onTestFinished for spy cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Defensive fix: Buffer.from(float32.buffer) ignores byteOffset and byteLength when the Float32Array is a subarray view. Use the three-arg form so vector blobs are always the correct slice, even if a future embedder returns a view instead of a fresh copy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
|
@CodeRabbit review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughAdds a hybrid search path to ChangesHybrid FTS+Vector Search (RRF)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/vault-mcp/search/search-index.ts (1)
871-878: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftReplace changed
asassertions with runtime row guards.These new DB/JSON paths assert row shapes instead of narrowing them, which violates the TypeScript guideline and can crash hybrid search if a row or persisted JSON value is malformed. Add small validators for vector rows, note rows, and parsed string arrays before using them.
As per coding guidelines, “Do not use
any,as, or non-null assertions (!); use runtime guards or schema validation to narrow types instead.”Also applies to: 1303-1324, 1896-1916
🤖 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 `@src/vault-mcp/search/search-index.ts` around lines 871 - 878, The hybrid search and persisted JSON handling currently rely on `as` assertions instead of validating the data shape at runtime, which can break on malformed rows or JSON. Replace the assertions in the search flow (including the `knnSearchStmt.all` result and the related DB/JSON paths around the other affected blocks) with small runtime guards or schema checks that narrow rows to the expected vector row, note row, and string-array shapes before use. Keep the existing search logic in place, but make the row parsing explicit and safe by validating the fields first and only proceeding with typed values after the guard passes.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 `@src/vault-mcp/search/__tests__/search-index.test.ts`:
- Around line 3328-3366: The vector-only search test is too weak because it does
not prove the snippet comes from the intended note. Update the mocked embedText
behavior in search-index.test.ts so each embed call maps deterministically to
the intended note/query embedding, then assert the returned result includes the
expected note path and the exact snippet value from that note’s chunk text. Use
the existing hybridSearch, embedNote, and createSearchIndex flow to keep the
test focused on the intended vector-only behavior.
- Around line 3748-3761: The truncated snippet assertion in the hybrid search
test is too loose even though the output is deterministic for this fixture. In
the `hybridIndex.hybridSearch` test case that inspects `verboseResult`, replace
the token-count/contains checks with a whole-value assertion against the exact
snippet produced by `buildSnippetFromChunkText`, so the test verifies the
precise truncated string for `verbose.md` rather than any five-word output with
ellipsis.
In `@src/vault-mcp/search/search-index.ts`:
- Around line 1271-1284: The RRF merge in search-index.ts is combining
unfiltered vectorHits from vectorSearch with ftsResults before applying
folder/tag/property constraints, so matching semantic results can be missed and
non-matching vectors can skew scores. Update the vector path in the search flow
around vectorSearch, computeRrfScores, and the later vector-only filtering to
apply the same filters to vector candidates before ranking, or pass the filters
into vectorSearch so only eligible notes contribute to RRF.
---
Nitpick comments:
In `@src/vault-mcp/search/search-index.ts`:
- Around line 871-878: The hybrid search and persisted JSON handling currently
rely on `as` assertions instead of validating the data shape at runtime, which
can break on malformed rows or JSON. Replace the assertions in the search flow
(including the `knnSearchStmt.all` result and the related DB/JSON paths around
the other affected blocks) with small runtime guards or schema checks that
narrow rows to the expected vector row, note row, and string-array shapes before
use. Keep the existing search logic in place, but make the row parsing explicit
and safe by validating the fields first and only proceeding with typed values
after the guard passes.
🪄 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 Plus
Run ID: e50beed6-d73a-4c02-8e62-79c731690a67
📒 Files selected for processing (3)
src/vault-mcp/mcp-core/tools/search-tools.tssrc/vault-mcp/search/__tests__/search-index.test.tssrc/vault-mcp/search/search-index.ts
Assert specific note path and snippet content for the vector-only snippet test. Use exact whole-value assertion for truncated snippet. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
…NABLED The vault_search tool description now reflects the actual search mode: hybrid (RRF + semantic) when EMBEDDING_ENABLED=true, keyword-only (FTS5 BM25) when false. Prevents misleading clients about semantic matching capabilities when embeddings are disabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Assert hybrid search terms (RRF, semantic, career aspirations example) appear when EMBEDDING_ENABLED=true, and are absent when false. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Returns "hybrid" when RRF fusion with vector similarity was used, "fts" when only keyword matching was available. Lets clients interpret scores correctly (BM25 vs RRF) and know whether semantic search is active. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Replace the mild "career aspirations" example with "dealing with burnout recovery" — a clearer vocabulary gap that teaches LLM clients what hybrid search can do (finds notes about mental health without the word "burnout"). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Replace personal burnout example with a technical one — "how the server watches for file changes" bridging to chokidar/file watchers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
…per reference Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Log searchMode ("hybrid" or "fts") in the structured info log for
both the RRF fusion path and the FTS-only fallback path. The fallback
previously returned silently without logging.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Both fullTextSearch and hybridSearch mapped NoteRow fields to SearchResult with identical conditional-spread logic for created, bytes, and leading_callout. Shared helper eliminates the duplication. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Summary
vault_searchtoolcomputeRrfScores(pure RRF),vectorSearch(KNN),hybridSearch(orchestrator)vault_searchtool description to reflect hybrid search capabilitiesSpike result
8/9 previously-failed queries fixed, 0 regressions, ~8ms latency (RRF-only mode).
Test plan
computeRrfScoresunit tests (table-driven: single/both systems, bonuses, custom k, sort order)hybridSearchintegration tests (fallback, hybrid ranking, filters, limits, dedup, callouts)🤖 Generated with Claude Code
https://claude.ai/code/session_018ZbaHwP28ASYTpQnZk6MqM
Summary by CodeRabbit