Skip to content

fix(rag): make BM25 indexing incremental and idempotent - #81

Open
Agnik47 wants to merge 1 commit into
supermemoryai:mainfrom
Agnik47:fix/bm25-incremental-idempotent-index
Open

fix(rag): make BM25 indexing incremental and idempotent#81
Agnik47 wants to merge 1 commit into
supermemoryai:mainfrom
Agnik47:fix/bm25-incremental-idempotent-index

Conversation

@Agnik47

@Agnik47 Agnik47 commented Aug 14, 2026

Copy link
Copy Markdown

Fixes #74.

Problem

Two defects in addToBM25Index (src/providers/rag/search.ts).

1. Quadratic ingest. avgDocLength was recomputed by walking the entire docLengths map on every single insert, so indexing n chunks scanned the map n times.

2. Re-ingest corrupted the index. addChunks overwrites container.chunks (idempotent) but addToBM25Index unconditionally did docCount++. Chunk IDs are deterministic (${containerTag}_${sessionId}_${chunkIndex}), so a forced or resumed ingest re-adds the same IDs — docCount drifted above docLengths.size, which skewed:

  • avgDocLength = totalLength / docCount → understated, distorting length normalisation for every document
  • idf = log((docCount - df + 0.5) / (df + 0.5) + 1) → inflated for every term

Postings from the replaced content were also left behind, so a chunk kept matching terms that were no longer in its text. Since BM25 carries 30% of the hybrid score, rag retrieval quality depended on run history rather than on the algorithm.

Fix

  • Track a running totalLength on the index instead of rescanning.
  • Track docTerms (distinct terms per document) so removeFromBM25Index can drop an entry's postings, length and count in O(terms) before it is re-added.
  • addToBM25Index now replaces rather than appends, keeping docCount === docLengths.size and the inverted index consistent with current content.

No public API change; HybridSearchEngine behaves identically for a fresh ingest.

Tests

New src/providers/rag/search.test.ts (3 tests, run with bun test):

  • re-ingesting the same chunks yields byte-identical search results to a fresh index
  • re-indexing a chunk with new content drops its old terms (stale posting scores 0)
  • an index whose content was replaced matches a freshly built one

All three fail on main and pass with this change.

Ingest timing

addChunks with 30-token chunks, before → after:

chunks before after
5,000 219 ms 157 ms
10,000 478 ms 264 ms
20,000 1,426 ms 456 ms
40,000 6,140 ms 1,481 ms

Before scales ~4x per doubling; after, ~2x.

bunx tsc --noEmit is clean and the new file is Prettier-clean. I left the pre-existing Prettier warnings elsewhere in search.ts alone to keep the diff scoped — bun run format:check already fails on 69 files on main.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JqGuPVWVYRJEJQp2RXchf1

`addToBM25Index` rescanned the whole `docLengths` map on every insert to
recompute `avgDocLength`, making ingest O(n^2) in the number of chunks.
It also incremented `docCount` unconditionally, so re-adding a chunk ID —
which happens on a forced or resumed ingest, since chunk IDs are
deterministic — pushed `docCount` above `docLengths.size` and corrupted
both `avgDocLength` and idf for every term, while leaving stale postings
for the replaced content.

Track a running `totalLength`, and replace an existing entry (dropping its
postings and length) before re-adding it. Ingest is now linear and BM25
scores no longer depend on run history.

Fixes supermemoryai#74

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JqGuPVWVYRJEJQp2RXchf1
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.

BM25 index rebuilds its average document length on every chunk (O(n²) ingest) and double-counts on re-ingest

1 participant