Add rank-stability probe to measure retrieval pool sensitivity - #143
Conversation
The eval corpus is 26 chunks; retrieval reaches `vault::candidate_pool(10) = 150` candidates per signal, so both halves of the hybrid return the whole index and the two ranked lists RRF fuses are identical for any pool at least that wide. Every score the eval prints is therefore invariant under pool/fusion width — #140's 3x widening of `search_chunks`' pool moved 5 of 7 probe top-10s on fixtures/test-vault while printing bit-identical eval numbers. Options (2) and (3) from the issue: - `--example stability` (`just stability`) — a rank probe on fixtures/test-vault (~780 chunks, where the pool binds). It reports pool sensitivity (the same query asked at widening pools, since candidate depth is a function of the ask) and drift of the shipped top-10 against a committed snapshot; `--bless` accepts a new ranking. Runs the fake embedder, so it is deterministic, needs no model, and its baseline is reproducible on any machine; `--model` scores the same probes on real bge vectors when the magnitude rather than the mechanism is the question. - The eval now states its blindness out loud: a corpus smaller than the pool prints a warning, and every results.jsonl row carries `pool` / `pool_blind`, so the caveat travels with the numbers instead of living in an issue. `vault::candidate_pool` is the one new public seam — the composition of `hit_pool` and `search::pool_size` that a measurement has to know. The suite pins that composition and the small-corpus invariance property it explains. Measured with the probe (fake embedder, fixtures/test-vault): the shipped 3x pool (150 candidates/signal) against the conservative `limit + 2` reading (60) drifts 10/10 probes with ~50% of each top-10 turning over — the evidence #142 was waiting on, with the caveat that uncorrelated fake vectors exaggerate what correlated bge rankings would show. The same probe on the eval corpus moves nothing at any pool, which is exactly the gap this issue names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014jpJ649GKkMnAh8Zrcw6MC
📝 WalkthroughWalkthroughThe change exposes effective retrieval-pool sizing, detects pool-blind evaluation runs, and adds a deterministic rank-stability probe with configurable queries, depth comparisons, baseline drift reporting, and blessing commands. ChangesRetrieval Pool Measurement
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Justfile
participant StabilityExample
participant VaultCopy
participant Embedder
participant Retrieval
participant Baseline
Justfile->>StabilityExample: Run stability probe with flags
StabilityExample->>VaultCopy: Copy selected vault
StabilityExample->>Embedder: Index copied vault
StabilityExample->>Retrieval: Query configured probes at multiple depths
Retrieval-->>StabilityExample: Return ranked notes and chunks
StabilityExample->>Baseline: Compare or write ranking snapshot
Baseline-->>StabilityExample: Report drift or save baseline
Possibly related issues
Possibly related PRs
Suggested reviewers: 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
crates/b2-core/tests/search.rs (1)
54-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert chunk-level rank stability for the depth invariant.
search_chunksreturnsChunkSearchResult, whilechunk_idlives onHit; comparingpathcan pass across different chunks in the same note and can miss swapped chunks. Comparehit.chunk_idwith an internalHit-level API, or use a stable result identity fromsearch_chunks, and limit the test to its observed depths unless it actually checks the entire depth range.🤖 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/tests/search.rs` around lines 54 - 75, Update a_corpus_smaller_than_the_pool_ranks_the_same_at_any_depth to compare chunk-level identities rather than paths, using hit.chunk_id or another stable search_chunks result identity. Ensure the assertion detects swapped chunks within the same note, and either validate every intended depth or limit the test wording and inputs to the depths it actually observes.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 `@crates/b2-core/src/search.rs`:
- Around line 91-98: Update the candidate-depth calculation in pool_size to use
saturating multiplication for limit * 5, preventing overflow while preserving
the existing minimum depth of 30.
In `@crates/b2-embed/examples/eval.rs`:
- Around line 452-460: Correct pool-blind handling across all listed sites: in
crates/b2-embed/examples/eval.rs lines 452-460 and 564-568, use <= for the pool
comparison and limit the warning and metadata to candidate-pool width; in
crates/b2-core/src/vault.rs lines 86-92, remove claims that signal lists are
identical or RRF_K is invisible; in crates/b2-core/tests/search.rs lines 47-76,
cover the exact-pool boundary and candidate-width stability; in
docs/design/index-engine.md lines 321-329 and crates/b2-embed/examples/eval.rs
lines 29-35, update guidance to state that complete candidate lists are stable
while RRF_K may change fused ordering and BM25 need not return the whole corpus.
In `@crates/b2-embed/examples/stability.rs`:
- Around line 353-364: Update prefix_cell so a zero-length comparison is
reported as unmeasured with an “n/a” label and does not count as stable;
preserve the existing =N and N/M output for non-empty comparisons, ensuring
empty answers are excluded from moved and pool-invariant counts.
In `@justfile`:
- Around line 243-253: Update the stability usage examples in the recipe
comment, its [doc] description, and CLAUDE.md lines 89-90 to include the `--`
separator before forwarded flags, using forms such as `just stability --
--verbose` and `just stability -- --vault <path>`. Keep the existing stability
recipe and argument forwarding unchanged.
---
Nitpick comments:
In `@crates/b2-core/tests/search.rs`:
- Around line 54-75: Update
a_corpus_smaller_than_the_pool_ranks_the_same_at_any_depth to compare
chunk-level identities rather than paths, using hit.chunk_id or another stable
search_chunks result identity. Ensure the assertion detects swapped chunks
within the same note, and either validate every intended depth or limit the test
wording and inputs to the depths it actually observes.
🪄 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: e7134638-ede1-4d51-b606-94acff4bdff7
📒 Files selected for processing (11)
CLAUDE.mdcrates/b2-core/src/search.rscrates/b2-core/src/vault.rscrates/b2-core/tests/search.rscrates/b2-embed/evals/stability-baseline.jsoncrates/b2-embed/evals/stability.jsoncrates/b2-embed/examples/eval.rscrates/b2-embed/examples/stability.rsdocs/design/index-engine.mdfixtures/README.mdjustfile
… pool (#141) Review of the #141 instrument, three corrections. Two are factual, and both were measured rather than argued. `search::pool_size` now saturates. `limit` is user input and the two widenings compose (`pool_size(hit_pool(limit))`), so a large `--limit` reached a product that overflows: `b2 search foo --limit 18446744073709551615` panicked a debug build at `(limit * 5)`, and a release build would have *wrapped* it into a tiny pool — an absurd-but-harmless ask silently returning wrong results. Saturating matches `hit_pool`'s own widening; a test pins that an absurd limit still answers. The blindness claim was too broad. It said a corpus smaller than the pool hides a change to `hit_pool`, `pool_size` **or `RRF_K`**. The first two are right; `RRF_K` is not — it re-weights the *same* two lists, so it reorders any corpus. Measured on the 26-chunk eval corpus: k = 60 → 10 moves note ranks across the query set, so the eval sees a k change fine. Everything now says *candidate* width, and the docs state the mechanism precisely: it is not that both signals "return the whole index" (BM25 returns every *matching* chunk), it is that neither list is truncated, so widening cannot add a candidate. The boundary is `<=` for the same reason — a pool exactly the size of the corpus truncates nothing either — so the warning and the `pool_blind` field now use it. The probe no longer counts an empty comparison as stability. A probe that returns nothing gave `span = 0`, printed `=0`, and was folded in as pool-invariant, which let a run that measured nothing announce that everything was invariant. Cells are now stable / moved / unmeasured, the denominators count only what was measured, and an all-empty run says so. Also: the probe drops a bare `--`. `just stability --verbose` forwards the flag directly (measured on just 1.58), but `just stability -- --verbose` forwards the separator into the recipe verbatim, and the example would have rejected it as an unknown flag. Rankings are unchanged — the blessed baseline still matches exactly; only its explanatory note moved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014jpJ649GKkMnAh8Zrcw6MC
Summary
Adds the rank-stability probe (
--example stability), the second half of the eval harness (GH #141). While the scored eval measures retrieval quality on a hand-labelled 26-chunk corpus, that corpus is too small to see fusion-width changes — both signals return the entire index, making the ranking invariant under pool widening. This probe measures the property the eval cannot: how much the top-K ranking shifts when retrieval pool width changes on a vault large enough for the pool to bind.fixes #141
Key Changes
New example:
crates/b2-embed/examples/stability.rs(668 lines)evals/stability.jsonat three retrieval depths (4, 10, 30), each widening the candidate poolevals/stability-baseline.json--modelfor real bge vectors (skips baseline comparison),--vaultfor custom vaults,--blessto accept current ranking as new baseline,--verboseto inspect diverging listsCommitted baseline:
crates/b2-embed/evals/stability-baseline.jsonfixtures/test-vaultunder the fake embedderProbe set:
crates/b2-embed/evals/stability.jsonfixtures/test-vaulttopicsPublic API:
vault::candidate_pool(limit)functioncandidate_pool(10) = 150Integration:
eval.rsto warn when corpus is smaller than pool (the blindness condition)search.rsto exposepool_sizeaspub(crate)forvault::candidate_pooljustfilewithstabilityrecipe and documentationindex-engine.md,CLAUDE.md,fixtures/README.md) to explain the blindness and the probeNotable Details
evals/corpus(the eval corpus) shows every prefix holds at every depth — that is the blindness (evals: the corpus is smaller than the retrieval pool, so the harness is blind to fusion-width changes #141)https://claude.ai/code/session_014jpJ649GKkMnAh8Zrcw6MC
Summary by CodeRabbit
New Features
Documentation
Tests