Skip to content

rag: no test drives the legacy RAGBackend::search route this PR fixes #1139

Open

Description

@sanchitmonga22

Follow-up from #1137 by @Adityakk9031 — thanks again for that PR!

What

#1137 threads config_.bm25_k1 / bm25_b into BM25Index::search (core/src/features/rag/rag_backend.cpp:415) and config_.rrf_k into fuse_results (:418), but nothing in the tree executes that route: its only caller, core/tests/rag_backend_thread_safety_test.cpp:52 (backend.search("hello", 1)), is excluded from the build (core/tests/CMakeLists.txt:1588-1591 — it targets the removed IEmbeddingProvider / set_embedding_provider API), and every test the PR adds drives the primitive layers instead — core/tests/test_rag_fusion.cpp:76-104 calls BM25Index::search and fuse_rankings directly, and core/tests/test_advanced_modality_proto_abi.cpp:1985 onward goes through rac_rag_search_proto → retrieve → run_rag_retrieve. Reverting the three headline lines would leave every test green.

Why it matters

Issue #1128's second acceptance box — "core/tests/rag_backend_thread_safety_test.cpp still passes against whichever shape lands" — cannot be checked by anyone, CI included, and the silent divergence this work removed can return unnoticed the next time someone wires a caller to RAGBackend::search.

Suggested approach

Either add a focused test that constructs a RAGBackend with a non-default RAGBackendConfig and asserts the fused scores move with bm25_k1 / bm25_b / rrf_k (the mock ops at core/tests/test_advanced_modality_proto_abi.cpp:582 and :404 are reusable), or rewrite rag_backend_thread_safety_test.cpp against the current constructor and re-add it to core/tests/CMakeLists.txt. Deleting the legacy route instead and repointing its caller at retrieve would also close it, and is what #1128's alternative option contemplated.

Done when

  • Some test fails if the legacy route stops honouring bm25_k1 / bm25_b / rrf_k, or the route is deleted.
  • rag_backend_thread_safety_test.cpp is either built and passing or removed with its exclusion note.

Not blocking #1137. @Adityakk9031, you know this code well now — you're welcome to take this one if you're interested.

Opened with help from Claude Code and Codex.

Reviewed with help from Claude Code and Codex.

Activity

  1. added
    p3Low priority, when there's time
    bugSomething is broken
    coreC++ commons
    on Oct 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething is brokencoreC++ commonsp3Low priority, when there's time

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions