Skip to content

fix: search_mode reflects actual search path, not result count#538

Open
dstier-git wants to merge 3 commits into
tirth8205:mainfrom
dstier-git:fix/search-mode-accuracy
Open

fix: search_mode reflects actual search path, not result count#538
dstier-git wants to merge 3 commits into
tirth8205:mainfrom
dstier-git:fix/search-mode-accuracy

Conversation

@dstier-git

Copy link
Copy Markdown

Fixes #537

Problem

search_mode in semantic_search_nodes is set based on whether results were returned, not which search path actually ran:

search_mode = "hybrid"
if not results:
    search_mode = "keyword"

This means a query where only embeddings matched reports "hybrid", and a query where embeddings silently failed (caught exception, returned empty) reports "keyword", giving no signal that semantic search never ran. Agents and users have no reliable way to tell whether the embedding path contributed.

Fix

Added an optional _out_mode: list | None = None output parameter to hybrid_search in search.py. It gets set to "hybrid", "fts", "semantic", or "keyword" based on which paths actually contributed results. All existing callers are unaffected since the parameter defaults to None.

semantic_search_nodes in tools/query.py passes _out_mode and reads from it instead of inferring from result count.

Test results

Verified by having Claude Code test queries that it would actually ask + nonsense:

image

"App" correctly stays "hybrid" (both FTS and embeddings matched). Multi-word natural language queries correctly report "semantic" since FTS5 phrase matching on identifier names returns nothing for those.

@dstier-git dstier-git changed the title Fix: search_mode reflects actual search path, not result count fix: search_mode reflects actual search path, not result count Jun 8, 2026
@tirth8205

Copy link
Copy Markdown
Owner

Thanks @dstier-git — the approach is right. The _out_mode plumbing correctly distinguishes hybrid/fts/semantic/keyword; I verified "fts" is now reported when only FTS contributes (the exact misreport in #537), and existing callers are untouched.

Two blockers:

  1. Syntax error: tools/query.py line 1 has the file path pasted before the docstring, so the whole tools package fails to import — pytest tests/test_tools.py dies at collection. Looks like an accidental paste after your screenshot run; please remove it and re-push.
  2. Tests: nothing in the repo asserts on search_mode and this PR adds none. Please add: in tests/test_search.py, _out_mode cases for "fts" (FTS-only), "keyword" (LIKE fallback), and "hybrid"/"semantic" (stub embeddings); in tests/test_tools.py, one semantic_search_nodes case asserting search_mode == "fts".

Nits: type the param Optional[list[str]] and document it in hybrid_search's docstring; the empty-query early return (search.py:333) leaves the mode unset, so the tool still reports "keyword".

With line 1 repaired locally, both suites pass (101 tests) and ruff/mypy are clean — this is close.

@dstier-git dstier-git left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 major issues to fix:

  • Accidental file path paste in tools/query.py
  • Lack of test coverage

@dstier-git

Copy link
Copy Markdown
Author

Thanks for the review @tirth8205. Here's what I changed with #538:

Issue 1 - syntax error in tools/query.py:
Removed accidental file path string on line 1, now compiles correctly.

Issue 2 - test coverage:
Added 7 tests across two files:

  • tests/test_search.py — 6 new cases in TestHybridSearch:
    • test_out_mode_fts_only — FTS table present, no embeddings → "fts"
    • test_out_mode_keyword — FTS table absent, keyword fallback finds results → "keyword"
    • test_out_mode_keyword_no_results — FTS absent, keyword finds nothing → "none" (see P1 fix below)
    • test_out_mode_semantic — stub embeddings only, no FTS → "semantic"
    • test_out_mode_hybrid — both FTS and stub embeddings contribute → "hybrid"
    • test_out_mode_empty_query — empty string → "none"
  • tests/test_tools.py — test_search_mode_fts: end-to-end test that semantic_search_nodes returns search_mode == "fts"

Nits:

  • _out_mode typed as Optional[list[str]] as requested
  • _out_mode documented in the hybrid_search docstring with all 5 possible values as requested
  • Empty-query early return now appends "none" to _out_mode instead of leaving it unset as requested

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.

search_mode field in semantic_search_nodes reflects result count, not actual search used

2 participants