Skip to content

fix(memory): build the RaBitQ index on demand and use it on the vector path - #4318

Merged
ruvnet merged 2 commits into
ruvnet:mainfrom
shuishui313:fix/4203-rabitq-auto-build
Oct 20, 2026
Merged

ruvnet merged 2 commits into
ruvnet:mainfrom
shuishui313:fix/4203-rabitq-auto-build

Conversation

@shuishui313

Copy link
Copy Markdown
Contributor

Summary

  • Add ensureRabitqIndex(): builds/rebuilds on demand using the existing
    shouldRebuildRabitq() thresholds (10-vector floor, 20% drift), with a 60s
    throttle, a process-level disable when the WASM backend is unavailable, and
    binding to the source dbPath and dimensions.
  • Wire it into all three search entry points: the sql.js pre-filter, the bridge
    vector path, and embeddings_rabitq_search, which no longer needs a manual
    build first.
  • bridgeSearchBruteForceCosine() reranks RaBitQ's Hamming candidates with the
    same exact cosine, reading only the candidate rows by primary key; any failure
    falls back to the unchanged exhaustive scan.

Closes #4203.

Behavior

The exhaustive path pre-ranks LIMIT 10000, so on corpora larger than 10k the
indexed path no longer drops rows outside that window. This is an intended
improvement, not a ranking change. The bridge hybrid path (bridgeSearchEntries,
1000-row BM25+cosine) is untouched, so describeBackend() continues to report
brute-force cosine there.

Test plan

  • __tests__/issue-4203-rabitq-auto-build.test.ts — 6 tests against a real
    sqlite store: build on demand, exact rerank, no rebuild per search,
    below-floor negative control, indexed-vs-exhaustive parity within the
    candidate window, recall + exact rerank on a 60-row corpus above the
    window (>=80% top-5 recall).
  • Regression: memory-bridge suite, issue-3311, memory-active-row-visibility,
    issue-2922-hnsw-status-honesty — 9 files / 32 tests pass.
  • tsc --noEmit on @claude-flow/cli: no new errors (14 pre-existing, none
    in touched files).

Risks

  • Corpora >10k rows: the indexed path no longer hides rows the pre-rank window
    dropped. A rowCount > 10000 fallback can be added if strict parity is
    required.
  • Index freshness remains bounded by the 20% drift policy; rows written inside
    the window are not candidates until a rebuild fires.
  • rabitq.meta.json is written but never read, so a new process rebuilds on its
    first search. Restoring from meta needs version/dims/sourcePath/count
    validation and is out of scope.
  • memory-bridge.ts already exceeds 500 lines; the change is additive rather
    than a refactor.

…r path (ruvnet#4203)

shouldRebuildRabitq() had no callers and the index was only ever populated by the explicit embeddings_rabitq_build tool, so searchRabitq() always returned null and the sql.js pre-filter was dead.

ensureRabitqIndex() drives the existing thresholds from the search entry points (60s throttle, 10-vector floor, 20% drift, disabled after a missing-WASM failure, bound to the source dbPath and dimensions).

The bridge cosine path now scans RaBitQ candidates and reranks them with the same exact cosine, falling back to the unchanged exhaustive scan on any error.
…uvnet#4203)

Six tests against a real sqlite store: build on demand, bridge uses the index and reranks with exact cosine, no rebuild per search, below-floor negative control, exhaustive-vs-indexed parity when the corpus fits the candidate window, and recall plus exact-rerank on a 60-row corpus larger than the window (>=80% top-5 recall).
@shuishui313
shuishui313 requested a review from ruvnet as a code owner October 20, 2026 09:15
@shuishui313

Copy link
Copy Markdown
Contributor Author

CI is waiting for approval as a first-time contribution. Could a maintainer approve the workflow run? thanks.
The results:
Local results: 6 new tests plus the memory regression suite (9 files / 32 tests) pass;
tsc --noEmit reports no new errors (14 pre-existing, none in touched files).

@ruvnet

ruvnet commented Oct 20, 2026

Copy link
Copy Markdown
Owner

Reviewed the diff for correctness against the PR description's own claims (not just taking them at face value):

  • Throttle/staleness interaction (rabitq-index.ts, AUTO_BUILD_RETRY_MS): lastAutoBuildAttempt is set unconditionally before the build attempt, so a failed build (non-WASM-unavailable error) does delay the next retry by the full 60s. Matches documented behavior, not a hidden bug.
  • Process-scoped disable: autoBuildDisabled is a module-level let — resets on restart as expected.
  • Concurrency: traced the throttle's check-then-set and the build's rabitqInitializing guard — both synchronous with no await in between, so no real race under Node's single-threaded model. searchRabitq() is async but contains no actual await, so a concurrent rebuild can't interleave mid-search.
  • shouldRebuildRabitq() wiring: correctly invoked with a fresh indexable-entry count each window against the last real build's vectorCount; 10-vector floor and 20%-drift logic both exercised as documented.
  • LIMIT 10000 claim: verified accurate. It pre-exists unchanged in the exhaustive scan; the new RaBitQ path chunks candidate lookups by 500 instead (no 10k cap), so the claimed behavior difference is real. bridgeSearchEntries's hybrid path is untouched — confirmed no diff there.
  • Mergeability: a raw two-dot diff against main looks noisy (shows feat(memory,providers): tag filter for memory list/search, explicit-provider precedence, Ollama default model #3526's already-merged tag-filter code as "removed" since this branch forked before that landed), but a true three-way merge is clean with zero conflicts — only 4 commits behind / 2 ahead, not stale the way a couple of other open PRs are right now.

Minor gap: no test exercises the WASM-unavailable permanent-disable path or genuinely concurrent ensureRabitqIndex() calls — correctness there is established analytically above, but a follow-up test would be good.

No blocking issues. CI is running now (just approved the pending first-contribution workflow runs) — will merge once green.

@ruvnet
ruvnet merged commit d7f2286 into ruvnet:main Oct 20, 2026
17 checks passed
ruvnet pushed a commit that referenced this pull request Oct 20, 2026
…-build (#4203) (#4319)

Review follow-up to #4318, which merged without coverage for two branches that
ensureRabitqIndex() relies on.

A missing @ruvector/rabitq-wasm now has a test: the first attempt fails and
sets autoBuildDisabled, and a second call ten minutes later (past the 60s
throttle) performs no further import, so the flag - not the throttle - is what
stops the retries.

Four concurrent ensureRabitqIndex() calls on a cold index: exactly one starts
the build, the index ends up with the right vector count, and it serves
searches afterwards.
@ruvnet ruvnet mentioned this pull request Oct 20, 2026
meefs pushed a commit to meefs/claude-code-flow that referenced this pull request Oct 20, 2026
Minor release (new backward-compatible flags: Grok Build CLI host
surface ruvnet#3512, memory list/search tag filter ruvnet#3526) bundled with a
round of correctness fixes merged since 3.58.1: memory delete-sync
(ruvnet#4138), real AgentDB HNSWIndex replacing the dead getController('hnsw')
path with safe exact-search fallback (ruvnet#4249/ruvnet#4322), harness live-process
reporting (ruvnet#3521), RaBitQ on-demand index build (ruvnet#4318/ruvnet#4319),
metaharness ADR-152 cost-dimension and verdict-band fix (ruvnet#3520), swarm
idle-agent heartbeat false-positive (ruvnet#4320), ephemeral-agent termination
idempotency (ruvnet#4323), status/task_list field-mapping and total-count
fixes (ruvnet#4321/ruvnet#4324).

Also fixes the actual cause of broken images/links on
npmjs.com/package/ruflo: `ruflo/package.json`'s `repository.directory:
"ruflo"` field added an extra path layer on top of the README content
(the root README.md, copied in verbatim by `prepublishOnly` at publish
time, which is already written with repo-root-relative paths) — every
relative image/link resolved to e.g. `raw/main/ruflo/ruflo/assets/...`
instead of `raw/main/ruflo/assets/...`. Removed the `directory` field to
match claude-flow's own (directory-less) repository entry. A prior PR
(ruvnet#4325) fixed `ruflo/README.md`'s own relative paths for anyone browsing
that file directly on GitHub, but that file is overwritten by the root
README at publish time and was never the actual npm-rendered content.

Published to npm: @claude-flow/cli@3.59.0, claude-flow@3.59.0,
ruflo@3.59.0, and standalone @claude-flow/memory@3.0.8 (the ruvnet#4249 fix —
not part of the 3-package train, published separately per its own
leaf's npm presence). All three umbrella packages' latest/alpha/v3alpha
dist-tags verified pointing at 3.59.0.

Co-Authored-By: RuFlo <ruv@ruv.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

the RaBitQ index is never built automatically and is unused on the bridge search path

2 participants