Wire tool_keywords into the BM25 search arm

[aponcedeleonch]

Summary

• find_tool advertises a tool_keywords input to the model, but the field was decoded off the wire, logged once, and dropped. ToolStore.Search took a single query string, so tool_description fed both the semantic arm (Embed) and the lexical arm (sanitizeFTS5Query), and there was nowhere to put keywords. The schema's claim that keywords are "combined with tool_description for hybrid search" was false.
• Two costs: models paid tokens for a field that did nothing, and a model trusting the schema could write a short, vague tool_description on the assumption that tool_keywords carried the lexical precision. That degrades the only arm running in FTS5-only mode (--optimizer without embeddings).
• ToolStore.Search now takes a types.SearchQuery{Description, Keywords}. Keywords build the FTS5 expression, the description is embedded for semantic matching, and the lexical arm falls back to the description when no usable keyword remains.
• Collapsed the two FTS5 sanitizers into one sanitizeFTS5Terms and dropped the phrase-search fallback. See "Special notes" below.
• Updated the tool_keywords schema description so the model knows semantic matching uses tool_description only, and keeps writing a complete one.

Type of change

• Bug fix

Test plan

• Unit tests (task test)
• Linting (task lint-fix)

Full suite passes. Two failures remain in pkg/plugins/pluginsvc and pkg/desktop; both are pre-existing on main and neither package has the changed packages anywhere in its dependency graph, including test deps. Lint is clean apart from a pre-existing G115 in cmd/thv/app/upgrade.go.

Each new test was validated as a real guard by reintroducing the bug and confirming a failure:

Mutation	Caught by
Keywords ignored (the original bug)	TestSQLiteToolStore_Search/keywords_surface_a_tool_the_description_alone_misses, TestSQLiteToolStore_KeywordsOnly/keywords_alone_drive_the_lexical_arm
Semantic arm embeds keywords instead of the description	TestSQLiteToolStore_SemanticArmEmbedsDescriptionOnly, TestSQLiteToolStore_KeywordsOnly/hybrid_tolerates_an_empty_description

That exercise changed the tests. A first attempt asserted Contains(results, "run_query") in hybrid mode and survived both mutations, because the semantic arm returns that tool on its own. The lexical assertions now run against FTS5-only stores where the semantic arm cannot mask the result.

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Changes

File	Change
internal/types/types.go	Add SearchQuery; widen ToolStore.Search
internal/toolstore/sqlite_store.go	Replace both sanitizers with sanitizeFTS5Terms; rework Search to split the arms
optimizer.go	Pass keywords into Search; update the schema tag
internal/types/mocks/mock_types.go	Regenerated via task gen

Does this introduce a user-facing change?

Yes. tool_keywords on find_tool now affects results instead of being ignored, and its schema description changed. Separately, a tool_description containing a common word (name, data, table, ...) now returns results where it previously returned none.

Special notes for reviewers

The sanitizer collapse is a second logical change. It could be split out if you'd prefer. sanitizeFTS5Query used to fall back to a phrase search whenever the query contained a word from problematicWords. A phrase search demands exact token adjacency, so it reliably matched nothing: the new "do something with data" test case returns zero results from the description arm for exactly this reason. Dropping the noise word and OR-joining the rest avoids the result flood the fallback was guarding against without going empty-handed. Four TestSanitizeFTS5Query expectations flip as a result, all toward returning results instead of none.

Fallback is keyed on the sanitized result, not on len(Keywords). All-noise keywords like ["table","schema"] fall through to the description rather than blanking the lexical arm.

fakeEmbeddingClient now records texts passed to Embed, behind a mutex since Search calls it from an errgroup goroutine. EmbedBatch, the write path, is deliberately not recorded so embeddedTexts() reflects queries only.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.48%. Comparing base (ea5ae5c) to head (c32c2f2).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6124      +/-   ##
==========================================
+ Coverage   72.43%   72.48%   +0.04%     
==========================================
  Files         733      733              
  Lines       75856    75860       +4     
==========================================
+ Hits        54945    54985      +40     
+ Misses      17016    16967      -49     
- Partials     3895     3908      +13     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jerm-dro]

Approve. Correct, minimal, well-tested fix for a real bug.

The bug: FindTool decoded ToolKeywords off the wire, advertised it in the tool's JSON schema, logged it, and then dropped it — Search was only ever called with ToolDescription. So tool_keywords was inert since introduction.

The fix is the right shape: the new types.SearchQuery{Description, Keywords} draws a clean seam so the lexical (FTS5/BM25) and semantic (embedding) arms get distinct inputs — lexical prefers Keywords and falls back to the description's words when keywords are empty or all dropped; semantic always embeds Description.

One thing worth noting in the PR description: this also changes the description-only lexical path, not just adds keywords. Old behavior phrase-searched the whole string whenever a problematic word was present (e.g. "read tool write" → phrase, matches nothing); new behavior drops problematic words and OR-joins the rest ("read" OR "write"). Strict improvement, and the problematicWords comment explains it — but it's a bug fix + a search-quality behavior change in one commit, so the title undersells the second half. Not blocking.

Tests are genuinely good:

• TestOptimizer_FindToolPassesKeywords is the direct regression guard for the dropped-keywords bug.
• TestSQLiteToolStore_SemanticArmEmbedsDescriptionOnly + the embeddedTexts() spy pin that the semantic arm embeds Description verbatim and never the keywords — a property result-set assertions can't catch.
• The "negative twin" (same description, no keywords → finds nothing) guards against a false positive where the description alone starts matching.

Security posture unchanged and sound: every term is still double-quoted with embedded quotes doubled and ftsExpr is bound via ?, so no FTS5-operator or SQL injection.

Verified locally on the PR branch: go test ./pkg/vmcp/optimizer/... ./pkg/vmcp/schema/... and golangci-lint/go vet on those packages all pass.