feat(search): redesign jrag search — honest scores, dedup, hybrid, guidance (Phase 0)#388
Merged
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
…CH-2) Collapses multiple chunks of the same primary_type_fqn into one row (best chunk wins by rank), so a single type can't flood the page. Default ON; --chunks / chunks=true opt back to chunk-level output. Changes: - search_lancedb.py: Add DEDUP_OVERFETCH=4, _dedup_by_fqn(rows, dedup_by_fqn) helper, dedup_by_fqn parameter to run_search with over-fetch logic (need = max((limit+offset)*4, limit+offset+1)) to absorb chunk multiplicity while preserving +1 truncation sentinel - mcp_v2.py: Add chunks: int|None to SearchHit, dedup: bool=True to search_v2 (default ON), copy _chunks_collapsed into chunks in _row_to_search_hit - jrag.py: Add --chunks flag to search subparser, pass dedup=not args.chunks - server.py: Add chunks: bool=False to MCP search tool, pass dedup=not chunks - tests: 4 tests in test_search_lancedb.py, 2 in test_jrag_orientation.py, 3 in test_mcp_v2.py (TDD: red→green, all pass) - docs: Document breaking change in AGENT-GUIDE.md and JAVA-CODEBASE-RAG-CLI.md Non-dedup path (--chunks) reproduces prior output exactly (test guard). sql/yaml rows (no primary_type_fqn) pass through unchanged. Truncation detection (+1 fetch) preserved via over-fetch multiplier. Co-Authored-By: Claude <noreply@anthropic.com>
…(PR-SEARCH-4) - --limit 0 short-circuits to a clean empty page (truncated not true); skips the embedding-model load. mark_truncated untouched (test pins it). - _zero_result_guidance: when a --role/--service/--module filter yields 0, run one cheap unfiltered probe and point at where matches live. Probe failure is non-fatal. - search_v2: fail-fast guard for table=all + hybrid (clean envelope before the model load); was already caught by the broad except, now deterministic. - #4 (Lance _score/_distance stderr deprecation) deferred: couldn't reproduce after the test index was erased; needs a live-index follow-up to verify. Co-Authored-By: Claude <noreply@anthropic.com>
PR-SEARCH-4's zero-result guidance probe calls search_v2 a second time with filter=None when an empty result has a filter set. The auto_scope test captured the filter on every call, so the probe's None overwrote the real search's filter. Capture the first (real search) call only — the test's intent (verify auto_scope reaches the search NodeFilter) is unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
HumanBean17
force-pushed
the
feat/search-enhance
branch
from
68c6d54 to
eed8848
Compare
PR-SEARCH-3 set results[name]="ok (fts skipped: ...)" when create_index raised, which broke test_lance_optimize (the _FakeTable mock has no create_index -> the optimize status string changed from "ok"). FTS at index time is best-effort (the lazy ensure_text_fts_index is the runtime fallback), so it must never alter the "ok" optimize status — only log the skip when verbose. CI caught this (local env lacked pytest-asyncio, masking the real assertion). Co-Authored-By: Claude <noreply@anthropic.com>
Code-review finding: the FTS-at-index-time path (PR-SEARCH-3) had zero unit
coverage — _FakeTable lacked create_index and lancedb.index was unmocked, so
the broad except swallowed the failure and every optimize test stayed green
without ever reaching create_index. Add a tracked create_index to _FakeTable
+ a test that mocks lancedb.index.FTS and asserts create_index('text',
replace=True) is invoked once after a successful optimize. Closes the
silent-failure gap.
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
jrag searchredesign — Phase 0Reviewed redesign of
jrag search(the tool's foundational feature). The semantic core was already strong; this ships the trust/UX foundation as 5 reviewable PRs, each gated by a per-task review + a final whole-branch review. Driven byplans/active/PLAN-SEARCH.md.What ships
score=is now monotonic with rank and on a common scale across vector + hybrid modes, so--min-scoremeans the same thing in both. Ranking itself is unchanged (sort keys read raw signals).--explain(1b) — surfacesdist / role / symbol / import / rrfper hit (CLI token + MCPscore_components); revives previously-deadexplain_score_components.primary_type_fqn(best chunk wins,chunks=Nshown);--chunks/chunks=trueopts back to chunk-level.--limit 0+ cleanall+hybrid(4) — filtered empties point at where matches live;--limit 0is a clean empty page;table=all + hybridfails fast.--hybridnow works on yaml/sql (no more crash); old indexes fall back to vector-only with an advisory.searchnow returns one row per symbol/type by default (was: every chunk). Use--chunks(CLI) orchunks=true(MCP) to restore chunk-level output. Documented indocs/AGENT-GUIDE.mdanddocs/JAVA-CODEBASE-RAG-CLI.md.Migration
Existing indexes should be
reprocessed once to (a) pick up per-symbol dedup re-tagging and (b) build the index-time FTS index that makes--hybridwork on yaml/sql. Seedocs/CONFIGURATION.md.Tests
All new/changed search tests pass. The branch introduces zero new failures vs master. 17 pre-existing failures exist on master itself (lance_optimize, mcp_tools, cli_progress_stdout_invariant, test_installer::TestPR4IndexProgress, + 3 async
pytest-asyncioconfig) — confirmed identical at the branch point; not caused by this PR.Deferred (see
plans/active/PLAN-SEARCH.md+ ledger)REPOSITORYrole for JPA interfaces (name-based role recognition) — descoped, will be done separately._score/_distancestderr deprecation noise — needs a live-index follow-up to verify suppression.🤖 Generated with Claude Code