diff --git a/docs/superpowers/specs/active/2026-07-12-hybrid-bm25-rrf-design.md b/docs/superpowers/specs/active/2026-07-12-hybrid-bm25-rrf-design.md index b14c712..7c67e5a 100644 --- a/docs/superpowers/specs/active/2026-07-12-hybrid-bm25-rrf-design.md +++ b/docs/superpowers/specs/active/2026-07-12-hybrid-bm25-rrf-design.md @@ -48,7 +48,17 @@ search_v2 → search_lancedb.run_search → _rrf_merge([vector_rows, graph_rows], k=60) ``` -**Proposed:** +> **Correction (post-merge, 2026-07-12):** the `graph-expand` arrow above was +> never true on the live path. `search_v2` did not pass `graph_expand=True` to +> `run_search` (nor did `jrag search` / the MCP `search` tool), so +> `_graph_expand_merge` — the 2-list fusion that pre-existed this change *and* +> the BM25 3-list this change adds — was **dormant** for every user-facing +> search. Tests and the eval stayed green only because they invoke +> `run_search(graph_expand=True)` directly, never through `search_v2`. Wiring +> landed as `graph_expand=(table == "java")` at both `run_search` call sites in +> `search_v2`, guarded by an integration test through `search_v2` +> (`test_search_v2_enables_graph_expand_on_java`). The "Proposed" flow below is +> now the actual flow. ``` search_v2 → search_lancedb.run_search → vector query (LanceDB) → vector_rows diff --git a/src/java_codebase_rag/mcp/mcp_v2.py b/src/java_codebase_rag/mcp/mcp_v2.py index 07df593..0d8f933 100644 --- a/src/java_codebase_rag/mcp/mcp_v2.py +++ b/src/java_codebase_rag/mcp/mcp_v2.py @@ -1043,6 +1043,13 @@ def search_v2( exclude_generated=nf.exclude_generated if nf else None, generated_only=nf.generated_only if nf else None, dedup_by_fqn=dedup, + # Always-on 3-list RRF fusion (vector + graph + BM25) on the java + # path — design spec, issue #431. run_search guards this to the + # java single-table path and degrades silently to pure-vector when + # the graph/FTS index is unavailable. Omitting this kwarg left the + # fusion dormant for every user-facing search (jrag search / MCP + # search); see the search_v2 graph_expand integration test. + graph_expand=(table == "java"), ) except Exception as exc: # Check if this is a missing-FTS error (old index built before PR-SEARCH-3) @@ -1069,6 +1076,7 @@ def search_v2( exclude_generated=nf.exclude_generated if nf else None, generated_only=nf.generated_only if nf else None, dedup_by_fqn=dedup, + graph_expand=(table == "java"), # 3-list fusion survives the FTS fallback ) advisories.append( f"hybrid unavailable on table '{table}' (FTS index missing on this index built before " diff --git a/tests/mcp/test_mcp_v2.py b/tests/mcp/test_mcp_v2.py index 6f4f5b9..c745d65 100644 --- a/tests/mcp/test_mcp_v2.py +++ b/tests/mcp/test_mcp_v2.py @@ -160,6 +160,61 @@ def test_search_path_contains_filter(monkeypatch, ladybug_graph) -> None: assert len(out.results) == 1 +@needs_vectors +def test_search_v2_enables_graph_expand_on_java(monkeypatch, ladybug_graph) -> None: + """search_v2 must pass graph_expand=True on the java table so the always-on + 3-list RRF fusion (vector + graph + BM25; design spec, issue #431) actually + runs on the user-facing path. This is the wiring contract that let PR #443 + ship dormant: search_v2 never asked for graph_expand, so _graph_expand_merge + (and its BM25 third list) never executed for `jrag search` / MCP `search`.""" + seen: dict[str, Any] = {} + + def fake_run_search(query, **kwargs): + seen["graph_expand"] = kwargs.get("graph_expand") + seen["table_keys"] = kwargs.get("table_keys") + return _fake_search_rows() + + monkeypatch.setattr("java_codebase_rag.mcp.mcp_v2.run_search", fake_run_search) + out = search_v2("ChatService", table="java", graph=ladybug_graph) + + assert out.success is True + assert seen.get("table_keys") == ["java"] + assert seen.get("graph_expand") is True, ( + "search_v2 must enable graph_expand on the java table; without it the " + "vector+graph+BM25 fusion is dormant (the PR #443 wiring bug)" + ) + + +@needs_vectors +def test_search_v2_enables_graph_expand_through_hybrid_fts_fallback( + monkeypatch, ladybug_graph +) -> None: + """The hybrid→vector-only FTS-missing retry (PR-SEARCH-3) must also carry + graph_expand=True on java, so the 3-list fusion still runs after the fallback. + Both run_search call sites in search_v2 are wiring points for #443 — the + LadybugDB sym_fts BM25 index is independent of LanceDB's native FTS, so the + fusion can still contribute even when hybrid falls back.""" + calls: list[dict[str, Any]] = [] + + def fake_run_search(query, *, hybrid, **kwargs): + calls.append({"hybrid": hybrid, "graph_expand": kwargs.get("graph_expand")}) + if hybrid: + raise ValueError( + "Cannot perform full text search unless an INVERTED index has been created" + ) + return _fake_search_rows() + + monkeypatch.setattr("java_codebase_rag.mcp.mcp_v2.run_search", fake_run_search) + out = search_v2("server port", table="java", hybrid=True, graph=ladybug_graph) + + assert out.success is True + assert len(calls) == 2 # initial hybrid attempt + vector-only retry + assert calls[1]["hybrid"] is False # retry dropped LanceDB-native FTS + assert calls[1]["graph_expand"] is True, ( + "the FTS-missing retry must still enable graph_expand on java" + ) + + def test_find_symbol_by_role(ladybug_graph) -> None: out = find_v2("symbol", {"role": "CONTROLLER"}, graph=ladybug_graph) assert out.success is True