[Bug][Backend] RedisVectorSearchService in-memory fallback caps the candidate pool at TopK keys BEFORE scoring — fallback returns arbitrary chunks, not the most similar ones #565

Closed
opened 2026-07-14 14:06:34 +00:00 by spikerj · 2 comments
Owner

QA Team — from the README audit's code verification, triaged with spikerj 2026-07-14 (decision: file).

The bug: when FT.SEARCH is unavailable/fails, RedisVectorSearchService falls back to an in-memory cosine-similarity scan — but it takes only the FIRST request.TopK keys matching BookPageChunk:* (server.Keys(...).Take(request.TopK), RedisVectorSearchService.cs:340-342) and scores just those. That caps the candidate pool at TopK arbitrary keys (SCAN order) instead of scoring all of the book's chunks and returning the top K by similarity. Net effect: the fallback path returns essentially random chunks with similarity scores attached — worse than useless as RAG context because it looks like it worked.

Why it matters now: the primary path is healthy (Redis 8.8's built-in Query Engine serves book_chunks_idx — verified live 2026-07-14), so this only bites when the index/query path degrades — i.e., exactly when nobody is looking, and AI:EnableRAG quiz context would silently degrade (the #553/#559/fail-open family's fourth member).

Fix shape: score all matching keys (optionally with BookId filter applied during the scan) then take top K; or make the fallback fail loudly (return empty + WRN) rather than pseudo-succeed.

**QA Team** — from the README audit's code verification, triaged with spikerj 2026-07-14 (decision: file). **The bug:** when `FT.SEARCH` is unavailable/fails, `RedisVectorSearchService` falls back to an in-memory cosine-similarity scan — but it takes only the FIRST `request.TopK` keys matching `BookPageChunk:*` (`server.Keys(...).Take(request.TopK)`, RedisVectorSearchService.cs:340-342) and scores just those. That caps the candidate pool at TopK **arbitrary** keys (SCAN order) instead of scoring all of the book's chunks and returning the top K by similarity. Net effect: the fallback path returns essentially random chunks with similarity scores attached — worse than useless as RAG context because it looks like it worked. **Why it matters now:** the primary path is healthy (Redis 8.8's built-in Query Engine serves `book_chunks_idx` — verified live 2026-07-14), so this only bites when the index/query path degrades — i.e., exactly when nobody is looking, and `AI:EnableRAG` quiz context would silently degrade (the #553/#559/fail-open family's fourth member). **Fix shape:** score all matching keys (optionally with BookId filter applied during the scan) then take top K; or make the fallback fail loudly (return empty + WRN) rather than pseudo-succeed.
Author
Owner

Fix in backend PR #279 — took your first fix shape (score all, then top K), not the fail-loudly one: the fallback exists to keep RAG alive through an index outage, and it can genuinely do that once it returns the right chunks.

What changed: ExecuteInMemorySearchAsync now scores every BookPageChunk:* key that passes filters and then takes the top TopK by cosine similarity — Offset/Limit paginate within that set and TotalCount reports its size, exactly mirroring the primary FT.SEARCH KNN semantics. The pre-fix .Take(request.TopK) on the key enumeration is gone.

The one new knob: an unbounded full scan + per-key HGETALL on a degraded path needed a ceiling, so VectorSearch:FallbackMaxScannedKeys (default 50,000) bounds it — and truncation logs a WRN naming the cap, because a capped scan can miss the true best matches and that must never be silent (your "worse than useless because it looks like it worked" point, applied to the cap itself).

5 new xUnit tests pin the contract by driving the internal fallback against mocked IServer/IDatabase with real embeddings — including the exact failure mode (best match last in SCAN order must still win, where the old code returned the first-scanned keys).

Leaving open for close after merge. Verification if you want it live: FT.DROPINDEX on a dev copy (or rename VectorSearch:IndexName) and confirm fallback results now match the index-backed results for the same query.

— macbook-claude-session

**Fix in backend PR #279** — took your first fix shape (score all, then top K), not the fail-loudly one: the fallback exists to keep RAG alive through an index outage, and it can genuinely do that once it returns the *right* chunks. **What changed:** `ExecuteInMemorySearchAsync` now scores every `BookPageChunk:*` key that passes filters and *then* takes the top `TopK` by cosine similarity — `Offset`/`Limit` paginate within that set and `TotalCount` reports its size, exactly mirroring the primary `FT.SEARCH KNN` semantics. The pre-fix `.Take(request.TopK)` on the key enumeration is gone. **The one new knob:** an unbounded full scan + per-key `HGETALL` on a degraded path needed a ceiling, so `VectorSearch:FallbackMaxScannedKeys` (default 50,000) bounds it — and truncation logs a WRN naming the cap, because a capped scan can miss the true best matches and that must never be silent (your "worse than useless because it looks like it worked" point, applied to the cap itself). 5 new xUnit tests pin the contract by driving the internal fallback against mocked `IServer`/`IDatabase` with real embeddings — including the exact failure mode (best match last in SCAN order must still win, where the old code returned the first-scanned keys). Leaving open for close after merge. Verification if you want it live: `FT.DROPINDEX` on a dev copy (or rename `VectorSearch:IndexName`) and confirm fallback results now match the index-backed results for the same query. — macbook-claude-session
Author
Owner

Resolved in spikersoft-backend PR #279 (merged to master 2026-07-14 as 81507f8). ExecuteInMemorySearchAsync now scores every matching BookPageChunk:* key and then takes the top TopK by cosine similarity — the .Take(request.TopK) on the key enumeration is gone — with VectorSearch:FallbackMaxScannedKeys (default 50,000) bounding the scan and logging a WRN when it truncates. Closing.

Resolved in spikersoft-backend PR #279 (merged to `master` 2026-07-14 as `81507f8`). `ExecuteInMemorySearchAsync` now scores every matching `BookPageChunk:*` key and *then* takes the top `TopK` by cosine similarity — the `.Take(request.TopK)` on the key enumeration is gone — with `VectorSearch:FallbackMaxScannedKeys` (default 50,000) bounding the scan and logging a WRN when it truncates. Closing.
Sign in to join this conversation.