Skip to content

fix(search): one search per leg, not one per leg per collection - #946

Open
shalom-t wants to merge 1 commit into
tobi:mainfrom
shalom-t:fix/structured-search-single-pass
Open

shalom-t wants to merge 1 commit into
tobi:mainfrom
shalom-t:fix/structured-search-single-pass

Conversation

@shalom-t

@shalom-t shalom-t commented Sep 8, 2026

Copy link
Copy Markdown

The bug

structuredSearch loops the collection list and pushes each collection's results as its own RRF ranked list, so a scope of N collections builds N ranked lists per search leg. RRF ranks with no relevance floor, so every collection's best-of-a-bad-lot enters fusion as a rank 1 next to the real answer.

The 2x weight on rankedLists[0] compounds it. That weight is documented as "assume caller ordered by importance" — meant for the caller's searches array — but the per-collection fan-out makes list 0 whichever collection happened to sort first, which the caller never chose deliberately.

The symptom is that ranking depends on argument order. Over a 21-collection scope and a 14-question gold set, reversing the collections array moved rank 1 for 13 of 14 queries. Rank 1 clustered in whichever collections sat at the front of the array.

The change

searchFTS and searchVec already take string | readonly string[], so this passes the scope straight through instead of looping it. One FTS query and one vector query per leg, one ranked list per leg — which is what the 2x weight assumes.

Measured on the same set:

unnarrowed scope, n=14 before after
P@1 0.000 0.500
recall@5 0.643 0.786
recall@10 0.714 0.786
MRR 0.279 0.607
rank 1 moves when array reversed 13/14 0/14
median query 0.499s 0.060s

Lexical-only ranking after the change matches qmd search over the same scope exactly (P@1 0.429, recall@10 0.714), which is the result you'd expect if the fan-out were the only difference between those two paths — it was.

The latency drop is arithmetic: 1 query instead of 21. searchVec was the worst of it, since it pulls the same global top-k from the vector index on every call and only filters by collection afterwards, so 20 of the 21 calls were redundant.

Compatibility

Single-collection scopes are byte-identical — the loop ran exactly once there already. The public signatures don't change.

vitest run on main before and after: same 5 failing files, same failing test set, no new failures. (The failures are pre-existing on this machine and look model/environment dependent.)

Note on the 2x weight

I left weights = rankedLists.map((_, i) => i === 0 ? 2.0 : 1.0) alone. With one list per leg it now does what its comment says. It seemed better to keep this change to the fan-out rather than also revisit the weighting in the same PR — happy to follow up if you'd rather it were derived from rankedListMeta explicitly.

structuredSearch loops the collection list and pushes each collection's
results as its own RRF ranked list, so a scope of N collections builds N
lists per search leg. RRF ranks with no relevance floor, so every
collection's best-of-a-bad-lot enters fusion as a rank 1 beside the real
answer. The 2x weight given to rankedLists[0] compounds it: it is meant for a
caller who ordered `searches` by importance, but the fan-out makes list 0
whichever collection happened to sort first.

The visible symptom is that ranking depends on argument order. Over a
21-collection scope and a 14-question gold set, reversing the collections
array moved rank 1 for 13 of 14 queries.

searchFTS and searchVec already accept `string | readonly string[]`, so this
just passes the scope straight through instead of looping it. On that same
set: P@1 0.000 -> 0.500, recall@5 0.643 -> 0.786, MRR 0.279 -> 0.607, and
reversing the array no longer changes anything. Median query time 0.499s ->
0.060s, since it is now one FTS and one vector query rather than 21 of each
-- searchVec in particular was pulling the same global top-k every time and
only then filtering by collection.

Single-collection scopes are unchanged, as they must be: the loop ran exactly
once there already.
@shalom-t

shalom-t commented Sep 8, 2026

Copy link
Copy Markdown
Author

One trade-off worth naming, since it isn't visible in the diff.

The loop guaranteed every collection its own top-20 regardless of how it scored globally. A single query can't do that: searchFTS overfetches limit * 10 from the FTS index and then filters, so a collection whose documents all fall outside that candidate window now contributes nothing.

I think that's the correct behaviour rather than a regression — a document outside the global top-200 by BM25 shouldn't outrank one inside it, and guaranteeing each collection a slot is what produced the rank-1 flooding in the first place. Recall went up, not down, on the set I measured. But the overfetch is a fixed multiple of limit and doesn't scale with collection count, so a scope with very many collections and a small limit would have a thinner margin than the 21-collection case I tested.

searchVec has no equivalent concern: it already drew limit * 3 neighbours from the global vector index on every call and only then filtered by collection, so the per-collection loop was slicing one global candidate set 21 ways rather than widening it. Coverage there is the same or better, with 1/21 of the work.

Happy to scale the FTS overfetch by collection count if you'd prefer the belt and braces.

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.

1 participant