fix(search): fall back to local search when semantic results don't match MCP tools#352
fix(search): fall back to local search when semantic results don't match MCP tools#352willleeney merged 2 commits intomainfrom
Conversation
commit: |
There was a problem hiding this comment.
Pull request overview
Adjusts StackOneToolSet.searchTools() auto-mode behavior so that when semantic search returns results that can’t be mapped back to fetched MCP tool names, the SDK falls back to the local BM25+TF‑IDF search instead of returning an empty tool set.
Changes:
- Add a fallback to
localSearch()when semantic results are non-empty but no tools match after ID/name normalization.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/toolsets.ts
Outdated
| // If semantic returned results but none matched MCP tools, fall back to local search | ||
| if (topResults.length > 0 && matchedTools.length === 0) { | ||
| return this.localSearch(query, allTools, mergedOptions); | ||
| } |
There was a problem hiding this comment.
This fallback runs regardless of search mode. In particular, search: 'semantic' will silently fall back to local search when semantic returns results that don’t match MCP tool names, which changes the contract implied by the PR description (fallback intended for auto mode). Consider gating this fallback behind search === 'auto' (or otherwise preserving strict semantic behavior).
src/toolsets.ts
Outdated
| // If semantic returned results but none matched MCP tools, fall back to local search | ||
| if (topResults.length > 0 && matchedTools.length === 0) { |
There was a problem hiding this comment.
No tests currently cover the new behavior where the semantic API returns non-empty results but none map to fetched MCP tool names (composite ID normalization mismatch). Adding a regression test in src/toolsets.test.ts for auto mode fallback (and ensuring semantic mode does not fall back if that’s the intended contract) would prevent future regressions.
| // If semantic returned results but none matched MCP tools, fall back to local search | |
| if (topResults.length > 0 && matchedTools.length === 0) { | |
| // If semantic returned results but none matched MCP tools: | |
| // - in auto mode, fall back to local search for better recall | |
| // - in semantic-only mode, do not change strategy; return no tools | |
| if (topResults.length > 0 && matchedTools.length === 0) { | |
| if (search === 'semantic') { | |
| return new Tools([]); | |
| } |
src/toolsets.ts
Outdated
| ); | ||
|
|
||
| // If semantic returned results but none matched MCP tools, fall back to local search | ||
| if (topResults.length > 0 && matchedTools.length === 0) { |
There was a problem hiding this comment.
topResults.length > 0 is redundant here because the method already returns early when topResults.length === 0. Simplifying the condition to just matchedTools.length === 0 would reduce noise.
| if (topResults.length > 0 && matchedTools.length === 0) { | |
| if (matchedTools.length === 0) { |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/toolsets.ts">
<violation number="1" location="src/toolsets.ts:837">
P2: This fallback runs even when `search` is `'semantic'`, which makes semantic-only mode silently use local results. Gate the fallback to auto mode (or return empty tools for semantic) to preserve the documented behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Summary
When semantic search returns results but none match MCP tool names (e.g. due to composite ID format differences), the SDK now falls back to local BM25+TF-IDF search instead of returning empty results.
Previously,
searchTools()in auto mode only fell back to local search when the semantic API call failed. Now it also falls back whenthe API succeeds but the returned action IDs don't match any fetched tools.
Test plan