fix: hybridRank stack overflow on large candidate sets#62
Open
mags-sully wants to merge 1 commit into
Open
Conversation
Math.min(...arr)/Math.max(...arr) spreads every candidate as a call argument; past V8's argument limit (~100k items) search() crashes with 'RangeError: Maximum call stack size exceeded'. search() ranks the entire embedding corpus, so any store past ~100k observations hits this on every query. Compute extrema with a loop instead — identical behavior, no argument-count ceiling. Repro: 480MB real-world store (138k observations) crashed on every 'cavemem search'; regression test with 150k candidates fails with RangeError before this change and passes after.
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
1 participant
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.
Problem
hybridRankcomputes score extrema withMath.min(...bm25s)/Math.max(...bm25s). Spread passes every element as a call argument, and past V8's argument limit (~100k) that throwsRangeError: Maximum call stack size exceeded.Because
MemoryStore.search()feeds the entire embedding corpus intohybridRank(all rows fromallEmbeddings()merged with keyword hits), any store past ~100k observations crashes on every search — CLI and MCP alike. Hit in the wild on a real store: 480MB / 138k observations after ~3 weeks of heavy Claude Code use.Fix
Compute extrema with a single loop over
items. Identical behavior (including the empty-input case: ranges default via|| 1as before), no argument-count ceiling, and one pass instead of two intermediate arrays.Evidence
New regression test with 150k candidates:
RangeError: Maximum call stack size exceededpnpm vitest run test/ranker.test.ts)Possible follow-ups (not in this PR)
search()is O(corpus) per query — cosine-scoring every embedding then ranking the full merged set. A top-k cap beforehybridRankwould bound both latency and memory as stores grow.data.dbgrows without bound (~7k observations/day under heavy agent use). A retention window setting would pair well with the cap.Happy to take a swing at either if you're interested.