Skip to content

fix(mcp): rank every BM25 candidate when post-window filters would hide them - #2819

Merged
DeusData merged 6 commits into
mainfrom
fix/2386-bm25-post-window-filters
Oct 31, 2026
Merged

DeusData merged 6 commits into
mainfrom
fix/2386-bm25-post-window-filters

Conversation

@DeusData

Copy link
Copy Markdown
Owner

Follow-up to #2423 (merged as b956413). Refs #2386, which #2423 closed.

What was wrong

In query (BM25) mode, search_graph ranks the top 2000 FTS5 candidates and applies file_pattern, kind and namespace afterwards. Since #2423 the label is part of the FTS5 MATCH, so a labelled window holds only rows of that label; these three filters cannot be expressed in the MATCH that way. With a common query token and a narrow filter, the window held none of the filtered rows, and the call returned 0 rows -- or a few rows of an under-counted total -- although matching nodes exist. An unlabelled query whose window held only File/Folder/Variable/Project rows (which unlabelled queries do not rank) had the same failure.

What this changes

  • When the candidate window is saturated and one of these filters is set, or the window held no row passing the filters, the query is ranked over every candidate (inner LIMIT -1). The FTS-first subquery plan stays, the extra work is linear in the token's match count (the saturation probe already orders every match), and total is exact (total_relation: "eq").
  • The decision depends only on the query, never on the page, so offset pages stay consistent.
  • Unfiltered and label-only queries that find rows keep the window and return the same output as before.
  • The filter binds move into one binder shared by the ranked and the count query (the bind blocks were duplicated); the count query and the saturation probe move into their own helpers.

Measured 2026-10-20 (indexes of Django, meilisearch and kubernetes)

Measured on 2026-10-20 with this change as b95b403 against its base 8af6833 (#2423's head that day, on main a222715). Not re-run after merging current main (b956413); no change on main since then touches the BM25 search code or the FTS index.

corpus filtered queries zero results rows returned exact totals
Django (67,914 nodes) 9 (file_pattern) 6 -> 0 52 -> 156 0 -> 9
meilisearch (18,417 nodes) 9 (file_pattern) 6 -> 0 3 -> 303 0 -> 9
kubernetes (348,201 nodes) 24 (file_pattern, kind, namespace) 23 -> 0 19 -> 807 0 -> 24

Byte-identity (2026-10-20): 52 unlabelled queries (two pages, json and tree) were identical to main a222715, and 36 label-filtered queries were identical to #2423's head 8af6833, on Django and kubernetes. The graph is unchanged (node and edge digests identical after a reindex; search-only change).

Latency, end to end over one warm stdio session (2026-10-20, min of 5 calls, informational only). The worst case is a token present in every qualified name, combined with a filter:

query before after
Django users + file_pattern 63 ms (0 rows) 118 ms (exact)
meilisearch users + file_pattern 29 ms (0 rows) 39 ms (exact)
kubernetes users + file_pattern 278 ms (total 63, gte) 582 ms (total 11,838, exact)
kubernetes users + kind 588 ms (0 rows) 890 ms (exact)
kubernetes unfiltered users 386 ms 391 ms

Tests

  • tool_search_graph_bm25_post_window_filters_rank_every_candidate: 2001 short-path Functions fill the window ahead of 3 that are the only ones passing file_pattern, kind or namespace; each filter must return all 3 with an exact total, offset pages must be disjoint and complete, and an unfiltered query must still report the saturated window. Fails without the change (the filtered rows are not returned).
  • tool_search_graph_bm25_empty_window_ranks_every_candidate: 2001 Folder rows fill an unlabelled window ahead of one Function; the Function must be returned with total:1. Fails without the change (the Function is not returned).
  • Re-checked after merging current main: with the src/mcp/mcp.c change reverted, exactly these two of the 392 mcp tests fail; restored, all pass.
  • mcp, incremental, integration, index_resilience on the merged tree: 587 passed, 4 skipped (Windows-only platform skips).

Checklist

  • Every commit is signed off
  • Tests that fail without the change
  • clang-format (touched hunks), cppcheck 2.20, memory-core lint clean

…de them

file_pattern, kind and namespace are applied after the 2000-row BM25
candidate window: unlike the label they cannot go into the FTS5 MATCH. With a
common query token and a narrow filter the window held none of the filtered
rows, and search_graph answered 0 rows -- or a few rows of an under-counted
total -- although matching nodes exist. An unlabelled query whose window held
only File/Folder/Variable/Project rows had the same failure.

When the window is saturated and such a filter is set, or the window held no
row passing the filters, the query is now ranked over every candidate
(inner LIMIT -1): the FTS-first subquery plan stays, the extra work is linear
in the token's match count (the saturation probe already orders every match),
and the total is exact. The decision depends only on the query, never on the
page, so offset pages stay consistent. Unfiltered and label-only queries that
find rows keep the window and are byte-identical.

The filter binds, the count query and the saturation probe move into helpers
shared by the ranked and the count query.

Measured on Django, meilisearch and kubernetes indexes (9 / 9 / 24 filtered
queries): zero results 6 -> 0, 6 -> 0, 23 -> 0; rows returned 52 -> 156,
3 -> 303, 19 -> 807; exact totals 0 -> 9, 0 -> 9, 0 -> 24. The graph is
unchanged (search-only change).

Refs #2386

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData
DeusData merged commit e082387 into main Oct 31, 2026
27 checks passed
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