Tracked from the final whole-branch review of issue #431 (fork B, PR branch worktree-hybrid-bm25-rrf-431). All items are Minor — the branch was reviewed READY-to-merge; these are non-blocking polish/coverage follow-ups.
1. Hermetic test of the real LanceDB+FTS BM25 fetch happy path (test-coverage gap)
The _bm25_candidate_rows fetch (search_lancedb.py) is unit-tested with _FilterTable fakes; the integration test forces FTS-unavailable, so it exercises degradation, not the real fetch. The shopizer eval proved BM25 contributes (nonzero MRR delta), but that's non-hermetic and not CI-gated. A future LanceDB/LadybugDB change to the filter-only .search().where(...).to_list() semantics would break the fetch silently. Add a hermetic test using real LanceDB + a real sym_fts index on a tiny fixture.
2. _HYBRID_SCORE_MAX spec/code wording (doc consistency)
Spec says the RRF term is "derived from list count (num_lists/(k+1))", but _rrf_max is always called as _rrf_max(2) (static). This is harmless at runtime — _rrf_merge normalizes dynamically (len(lists)/(k+1)) and the MCP layer prefers _rrf_score; the static constant only governs the single-table hybrid display path (_hybrid_post_sort_normalization, a 2-signal fusion). Reconcile: either reword the spec to "_rrf_max helper introduced; static at 2 for the single-table-hybrid display path; the 3-list path normalizes dynamically inside _rrf_merge", or wire runtime list-count into the constant.
3. eval/runner.py — restore mutated JAVA_CODEBASE_RAG_INDEX_DIR env
run_eval sets os.environ["JAVA_CODEBASE_RAG_INDEX_DIR"] and never restores it. Benign for subprocess/CLI invocation (each call re-sets it), but a library-style caller in a long-lived process could see the leak. Wrap in try/finally restoring the prior value.
4. ConfigMetrics.num_queries dual semantics
num_queries counts only scored queries (empty-relevant entries are skipped from scoring but still counted in latencies_ms). Add a docstring distinguishing the two counts, or expose a separate num_latency_queries.
5. eval/runner.py import private _enclosing_type_fqn/_SYMBOL_RETURN despite public aliases
search_lexical exposes enclosing_type_fqn (public) for the cross-module caller, but the runner imports the underscored original. Use the public aliases for consistency.
6. build_tier_a materializes all queries before capping
On a very large repo (~100k symbols) it builds ~200k tiny dataclasses before slicing to max_queries. Cap at the symbol-enumeration step instead to avoid the transient spike.
7. (informational) Tier-A's two query forms hit different retrieval regimes
The identifier form (DistributionChunkService) triggers auto_hybrid (LanceDB single-table FTS) via looks_like_code_identifier; the lowercase form does not. Production-faithful, not a bug — but Tier-A results conflate two retrieval regimes. Note for eval interpretation.
🤖 Generated with Claude Code
Tracked from the final whole-branch review of issue #431 (fork B, PR branch
worktree-hybrid-bm25-rrf-431). All items are Minor — the branch was reviewed READY-to-merge; these are non-blocking polish/coverage follow-ups.1. Hermetic test of the real LanceDB+FTS BM25 fetch happy path (test-coverage gap)
The
_bm25_candidate_rowsfetch (search_lancedb.py) is unit-tested with_FilterTablefakes; the integration test forces FTS-unavailable, so it exercises degradation, not the real fetch. The shopizer eval proved BM25 contributes (nonzero MRR delta), but that's non-hermetic and not CI-gated. A future LanceDB/LadybugDB change to the filter-only.search().where(...).to_list()semantics would break the fetch silently. Add a hermetic test using real LanceDB + a realsym_ftsindex on a tiny fixture.2.
_HYBRID_SCORE_MAXspec/code wording (doc consistency)Spec says the RRF term is "derived from list count (
num_lists/(k+1))", but_rrf_maxis always called as_rrf_max(2)(static). This is harmless at runtime —_rrf_mergenormalizes dynamically (len(lists)/(k+1)) and the MCP layer prefers_rrf_score; the static constant only governs the single-table hybrid display path (_hybrid_post_sort_normalization, a 2-signal fusion). Reconcile: either reword the spec to "_rrf_max helper introduced; static at 2 for the single-table-hybrid display path; the 3-list path normalizes dynamically inside _rrf_merge", or wire runtime list-count into the constant.3.
eval/runner.py— restore mutatedJAVA_CODEBASE_RAG_INDEX_DIRenvrun_evalsetsos.environ["JAVA_CODEBASE_RAG_INDEX_DIR"]and never restores it. Benign for subprocess/CLI invocation (each call re-sets it), but a library-style caller in a long-lived process could see the leak. Wrap intry/finallyrestoring the prior value.4.
ConfigMetrics.num_queriesdual semanticsnum_queriescounts only scored queries (empty-relevant entries are skipped from scoring but still counted inlatencies_ms). Add a docstring distinguishing the two counts, or expose a separatenum_latency_queries.5.
eval/runner.pyimport private_enclosing_type_fqn/_SYMBOL_RETURNdespite public aliasessearch_lexicalexposesenclosing_type_fqn(public) for the cross-module caller, but the runner imports the underscored original. Use the public aliases for consistency.6.
build_tier_amaterializes all queries before cappingOn a very large repo (~100k symbols) it builds ~200k tiny dataclasses before slicing to
max_queries. Cap at the symbol-enumeration step instead to avoid the transient spike.7. (informational) Tier-A's two query forms hit different retrieval regimes
The identifier form (
DistributionChunkService) triggersauto_hybrid(LanceDB single-table FTS) vialooks_like_code_identifier; the lowercase form does not. Production-faithful, not a bug — but Tier-A results conflate two retrieval regimes. Note for eval interpretation.🤖 Generated with Claude Code