docs(planning): #38 research — discovery scan strategy (own the flat scan, then note-centroid two-stage) - #39
Merged
Conversation
…scan, then note-centroid two-stage) Deep analysis of the remaining O(vault) note-open cost behind b2 similar: localizes the 4.4 s to the vec0 per-row shadow-probe read path + arithmetic shape (microbenched: ~530 ms current shape vs ~75 ms scratch+unrolled at 38.6k chunks), weighs five options (plain-table flat scan, note-centroid prefilter, quantization, ANN, materialized results), and recommends: plain embeddings table + in-process top-k + mmap/cache pragmas now (exact-identical results, drops sqlite-vec, also closes #36), note-centroid two-stage for 10x headroom, chunker upgrade #19 as the multiplier. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K8W2sYMr6PhVzjdA3PvYR8
… drop sqlite-vec (#38) Implements planning/research/discovery-scan-strategy.md (schema v3): - Vectors move from sqlite-vec's chunks_vec vec0 vtab into a plain embeddings(chunk_id, vector) table with an ON DELETE CASCADE FK; every distance is computed in-process. The vec0 scan charged a shadow-table probe per row (~38.6k internal statements per note-open, #38 — same mechanism as #36's reindex cost, also fixed here: the missing-vector anti-join is now a plain indexed join). The sqlite-vec dependency is gone. - Discovery is two-stage: an O(notes) coarse scan over per-note centroids (note_centroids, refreshed by the embed pass per completed note; dropped on re-chunk) shortlists max(200, 20x limit) notes, then the exact max-sim rescores only their chunks. Shortlist >= candidates (any test-scale vault) reproduces the exhaustive scan bit-for-bit, locked by a new parity test. - Arithmetic shape: unrolled 8-accumulator l2_sq (autovectorizable; measured ~7x over the iterator shape at 38.6k x 768) and a reusable decode buffer (unpack_f32_into) instead of a Vec per row. - db::open gains PRAGMA mmap_size=1GiB + cache_size=32MiB so whole-space scans stream through the OS page cache instead of pread-per-page. - graph_filtered_search resolves chunk->note via one bulk map (the #37 N+1 shape, removed from its last remaining path). - Schema v3: migrate() drops the new derived tables; a pre-3 index's vec0 entry is left inert (module no longer linked) — delete .b2/b2.sqlite for a clean slate, then reindex. Verified on a synthetic 1000-note / 40,144-chunk vault (fake embedder, release build): b2 similar ~28-31 ms wall including process startup (was ~4.4 s at the same chunk count in #38), and one open logs 259 SQLite statements instead of ~38.6k. Two-stage output checked equal to an independently computed exhaustive ground truth where the shortlist covers the candidate set. cargo test -p b2-core: 26/26 targets green; clippy clean. Closes #36. Closes #38. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K8W2sYMr6PhVzjdA3PvYR8
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR removes ChangesEmbedding storage and scoring
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Sequence Diagram(s)sequenceDiagram
participant Embedder
participant EmbeddingPass
participant SQLite
participant Discovery
Embedder->>EmbeddingPass: generate chunk vectors
EmbeddingPass->>SQLite: store embeddings
EmbeddingPass->>SQLite: refresh note_centroids
Discovery->>SQLite: read note centroids
SQLite-->>Discovery: shortlist candidates
Discovery->>SQLite: read shortlisted chunk vectors
SQLite-->>Discovery: exact vector data
Discovery-->>Discovery: compute max-sim scores
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@crates/b2-core/src/vault.rs`:
- Line 260: Update the documentation near Vault::embed to state that
vector-table shaping and re-embedding occur only during explicit indexing
operations, including both reindex and the standalone embed operation; remove
any wording that implies this happens only during reindex.
In `@planning/index-engine.md`:
- Around line 217-226: Update the exact-results claim in the discovery strategy
documentation to qualify that parity is guaranteed only when the centroid
shortlist includes the full candidate set. Do not state unqualified exactness,
and align the wording with discover::candidates truncating normal shortlists and
the documented possibility of recall loss.
In `@planning/research/discovery-scan-strategy.md`:
- Around line 108-115: Revise the paragraph around the embeddings table
description to distinguish the three vector consumers—“for_each_stored_vector”,
“vector_search”, and “vector_search_all”—as sequential B-tree scans from the
separate “chunks_missing_vectors” LEFT JOIN anti-join. Preserve the existing
storage, lifecycle, mmap, page/cache, and vector-shape requirements without
implying the anti-join uses the same scan shape.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d02dcab-1675-446c-b9ce-e5f5fc94fb97
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
CLAUDE.mdcrates/b2-core/Cargo.tomlcrates/b2-core/src/db.rscrates/b2-core/src/discover.rscrates/b2-core/src/embed.rscrates/b2-core/src/ingest.rscrates/b2-core/src/lib.rscrates/b2-core/src/search.rscrates/b2-core/src/vault.rscrates/b2-core/tests/cancel.rscrates/b2-core/tests/discover.rscrates/b2-core/tests/discover_query_count.rscrates/b2-core/tests/embed.rscrates/b2-core/tests/project_embed.rscrates/b2-core/tests/substrate.rscrates/b2-core/tests/vector_pool_scale.rscrates/b2-core/tests/write.rscrates/b2-embed/src/model.rsplanning/index-engine.mdplanning/research/discovery-scan-strategy.md
💤 Files with no reviewable changes (1)
- crates/b2-core/Cargo.toml
| /// | ||
| /// `open` **never mutates the embedding space** (the `open()`-time-drop fix, | ||
| /// tasks.md / index-engine.md §8): shaping `chunks_vec` and any re-embed happen | ||
| /// tasks.md / index-engine.md §8): shaping the vector tables and any re-embed happen |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Mention the standalone embed operation here.
Vault::embed is separately invokable and creates/fills the vector tables, so saying this happens “only on reindex” is stale and can mislead callers about the projection/embedding split. Clarify that vector shaping occurs only during explicit indexing operations such as reindex or embed.
🤖 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 `@crates/b2-core/src/vault.rs` at line 260, Update the documentation near
Vault::embed to state that vector-table shaping and re-embedding occur only
during explicit indexing operations, including both reindex and the standalone
embed operation; remove any wording that implies this happens only during
reindex.
Comment on lines
+217
to
+226
| **Update (2026-07-12, [#38](https://github.com/AlteredCraft/B2/issues/38)).** The brute-force *math* | ||
| scaled as predicted; `sqlite-vec`'s **read path** did not — every `vec0` scan probes a shadow table | ||
| per row (~38.6k internal statements per `b2 similar` on the primary vault, ~4.4 s per note-open). | ||
| Since its only shipped search was brute force we already compute, the dependency was **removed**: | ||
| vectors now live in plain tables (`embeddings`, plus per-note `note_centroids`) scored in-process, | ||
| and discovery is two-stage (O(notes) centroid shortlist → exact max-sim rescore). Same SQLite store, | ||
| same single-store property, same exact results at test scale. Full analysis + options: | ||
| [research/discovery-scan-strategy.md](research/discovery-scan-strategy.md). Quantization and ANN keep | ||
| their standby order, now behind the centroid stage. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify the exactness claim.
The supplied parity test covers the full candidate set by calling candidates(..., NOTES), so it does not prove exact results at normal shortlist sizes. Since discover::candidates truncates the centroid shortlist and the research document acknowledges possible recall loss, state that parity holds when the shortlist covers the candidate set.
🧰 Tools
🪛 LanguageTool
[style] ~223-~223: This phrase is redundant. Consider writing “same”.
Context: ...Lite store, same single-store property, same exact results at test scale. Full analysis + ...
(SAME_EXACT)
🤖 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 `@planning/index-engine.md` around lines 217 - 226, Update the exact-results
claim in the discovery strategy documentation to qualify that parity is
guaranteed only when the centroid shortlist includes the full candidate set. Do
not state unqualified exactness, and align the wording with discover::candidates
truncating normal shortlists and the documented possibility of recall loss.
Comment on lines
+108
to
+115
| Store embeddings in an ordinary table (`embeddings(chunk_id INTEGER PRIMARY KEY, vector BLOB)`, | ||
| same packed-LE-f32 blob), keyed/lifecycled exactly as `chunks_vec` is today (created at embed | ||
| time at the model's dim; dropped on model swap; `meta` discipline unchanged). All four vector | ||
| consumers — `for_each_stored_vector`, `vector_search`, `vector_search_all`, and #36's | ||
| missing-vector anti-join — become one sequential B-tree scan (plus, for search, an in-process | ||
| top-k heap). Add `PRAGMA mmap_size` (≥ DB size, e.g. 1–2 GB — it's a cap, not an allocation) | ||
| and a bigger `page_size`/`cache_size` so the scan streams from the OS page cache — the one | ||
| cache we're happy to use because nobody manages it. Fix `l2_sq`/`unpack_f32` shape as measured. |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- planning/research/discovery-scan-strategy.md around lines 90-130 ---'
sed -n '90,130p' planning/research/discovery-scan-strategy.md | cat -n
echo
echo '--- search for vector consumer symbols ---'
rg -n "for_each_stored_vector|vector_search_all|vector_search|chunks_missing_vectors|missing-vector|anti-join|chunks_vec|embeddings" -S .
echo
echo '--- candidate planning docs mentioning the same terms ---'
rg -n "for_each_stored_vector|vector_search_all|vector_search|chunks_missing_vectors|chunks_vec|embeddings" planning -SRepository: AlteredCraft/B2
Length of output: 36515
Separate the missing-vector anti-join from the flat table scans.
for_each_stored_vector, vector_search, and vector_search_all can be described as sequential scans of embeddings, but chunks_missing_vectors is a chunks LEFT JOIN embeddings ... WHERE v.chunk_id IS NULL anti-join. Reword this paragraph so it doesn’t imply all four consumers share the same scan shape.
🤖 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 `@planning/research/discovery-scan-strategy.md` around lines 108 - 115, Revise
the paragraph around the embeddings table description to distinguish the three
vector consumers—“for_each_stored_vector”, “vector_search”, and
“vector_search_all”—as sequential B-tree scans from the separate
“chunks_missing_vectors” LEFT JOIN anti-join. Preserve the existing storage,
lifecycle, mmap, page/cache, and vector-shape requirements without implying the
anti-join uses the same scan shape.
…flight scan The side pane fetched Connections (fast `explain` graph read) and Similar & unlinked (slower whole-vault `similar` scan) with one Promise.all, so the fast section couldn't paint until the slow one finished. Now the two reads run as independent chains, each repainting as it settles, with per-section loading flags — a note with no connections no longer shows a stuck "Loading…" until the similar scan returns. refreshDiscovery owns the flags (stale-guarded against a superseding open), so a fast open-A-then-B can't clear B's loading state. Similar's in-flight state is now a subtle, theme-aware, reduced-motion-aware spinner (role=status) instead of static "Finding similar notes…" text. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 22, 2026
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.
Deep analysis of the remaining O(vault) note-open cost behind b2 similar:
localizes the 4.4 s to the vec0 per-row shadow-probe read path + arithmetic
shape (microbenched: ~530 ms current shape vs ~75 ms scratch+unrolled at
38.6k chunks), weighs five options (plain-table flat scan, note-centroid
prefilter, quantization, ANN, materialized results), and recommends:
plain embeddings table + in-process top-k + mmap/cache pragmas now
(exact-identical results, drops sqlite-vec, also closes #36), note-centroid
two-stage for 10x headroom, chunker upgrade #19 as the multiplier.
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01K8W2sYMr6PhVzjdA3PvYR8
Summary by CodeRabbit