feat: Search-based tool discovery with categorized call proxies - #727
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new, opt-in search-based tool discovery system designed to optimize LLM interactions. By replacing a large, static tool catalog with a dynamic search mechanism and categorized call proxies, the system drastically reduces the token footprint in the LLM's context. This enhancement improves efficiency and scalability without compromising the agent's access to the full range of functionalities, while also providing clear guidance for LLMs on how to discover and utilize tools based on their operational intent (read, write, delete). Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature for search-based tool discovery, which significantly reduces token usage. The implementation using a FastMCP transform is clean and well-integrated. The code is generally of high quality. I've identified a few areas for improvement related to performance, maintainability, correctness, and adherence to the project's style guide. These suggestions aim to make the new feature more robust and efficient.
…form), dev14 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@julienld @sergeykad I haven't done BAT testing yet but I Have tested it directly in claude.ai and it works pretty well., Since this is an opt-in feature I Feel that it can be merged. I did slightly modify ha_get_states to make the search function work better (if it searched for "lights status" it couldn't find this tool for example) but literally only added 5 words. I'm sure more little edge cases like that will pop up with this but overall this is going to save a TON of tokens and potentailly be a lot faster to use too. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces an excellent feature for search-based tool discovery, which will significantly reduce token usage for LLM clients. The implementation is well-structured, leveraging a transform to dynamically alter the tool catalog. My review focuses on improving maintainability by reducing code duplication, enhancing correctness by avoiding hardcoded values, and ensuring adherence to the project's error handling guidelines.
e365b7d to
40677dd
Compare
…homeassistant-ai#727, homeassistant-ai#732, dev35 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Change skill tool description from "IMPORTANT: Read this skill BEFORE" to "CALL THIS FIRST" for stronger LLM compliance - When enable_tool_search is active, append the full search workflow (proxy names, sequential calling, param structure) to the skill tool description — this is the bootstrap for clients like claude.ai that don't read server instructions - Uses getattr for forward-compatibility with homeassistant-ai#727's enable_tool_search setting (no-op when the setting doesn't exist yet) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…sistant-ai#732, bump to dev37 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…, sync addon source, bump to dev38 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Introduces an opt-in search transform (ENABLE_TOOL_SEARCH=true) that replaces the full 89+ tool catalog with a unified BM25 search tool and three categorized call proxies (read/write/delete), dramatically reducing idle context token usage for LLMs. Key design: - Unified search_tools searches ALL tools, results include annotations - call_read_tool (readOnlyHint) — clients can auto-approve - call_write_tool (destructiveHint) — requires confirmation - call_delete_tool (destructiveHint) — requires confirmation - Critical tools pinned via always_visible for individual permission gating: ha_restart, ha_reload_core, ha_backup_create/restore, ha_get_overview, ha_report_issue - ResourcesAsTools (list_resources/read_resource) auto-pinned when enable_skills_as_tools is enabled - Server instructions updated with tool discovery workflow - ha_get_overview includes tool discovery hint when active Subclasses FastMCP's BM25SearchTransform (~200 lines), leveraging native catalog bypass, auth filtering, and search infrastructure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix Ruff RUF012: annotate _PINNED_TOOLS with ClassVar[list[str]] - Fix runtime crash: move Context import out of TYPE_CHECKING (Pydantic needs it at runtime for Tool.from_function type resolution) - Fix critical bug: override get_tool() so categorized proxy tools (ha_call_read_tool, ha_call_write_tool, ha_call_delete_tool) are resolvable when the LLM calls them - Fix _categorize_tool: require destructiveHint AND name pattern for delete category (was matching on name pattern alone) - Fix search tool missing readOnlyHint annotation - Rename all tool references to ha_* prefix (ha_search_tools, ha_call_read_tool, etc.) per naming convention - Add DEFAULT_PINNED_TOOLS constant; use in tools_search.py (DRY) - Add lazy cache rebuild with _cache_built flag - Add enable_tool_search toggle to dev add-on config UI - Add 20 unit tests for CategorizedSearchTransform Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add domain keywords (lights, switches, sensors, etc.) and natural language terms (status, current) so BM25 can match common queries like "lights status" to this tool. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace ValueError with ToolError + create_error_response per style guide structured error requirements - Deduplicate _PINNED_TOOLS: use DEFAULT_PINNED_TOOLS as single source - Extract proxy descriptions to module constants shared between transform_tools() and get_tool() for consistency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The parameter default is "minimal" but the docstring said "Use 'standard' (default)" — causing LLMs to always pick standard and blow out context with the full entity list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Enrich ha_search_tools description with full bootstrap workflow, calling example, anti-nesting warning, and sequential call guidance - Add param structure and sequential call hints to all proxy descriptions - Add server-side double-wrap unwrapping in categorized_call() so LLMs that accidentally nest name/arguments still succeed - Update ha_get_overview tool_discovery hint with param and sequential info - Append skills hint to search tool description when skills_as_tools is on - Auto-enable enable_skills when enable_skills_as_tools is set (dependency) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace hard "your client supports resources" claim with soft "should be able to" and explicit fallback path to tools - Add catalog membership check to double-wrap detection so it only unwraps when the inner name is a known tool in the catalog - Auto-enable enable_skills when enable_skills_as_tools is set Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…assistant-ai#732 When skills-as-tools is enabled, pin any skill guidance tool names stored in _skill_tool_names (registered by homeassistant-ai#732) so they remain visible in list_tools() alongside other pinned tools. Uses getattr with empty default so this is a no-op until homeassistant-ai#732 merges. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
89b3e6e to
b439a3e
Compare
…queries Adds a lightweight FastMCP Transform that appends extra keywords to specific tool descriptions so BM25 ranks them higher for natural language queries. Only active when enable_tool_search is on — original tool docstrings are unchanged. Fixes three BM25 ranking issues identified during search quality testing: - "find entities" now ranks ha_search_entities above ha_deep_search - "get/read automation" now ranks ha_config_get_automation above set - "create helper" now ranks ha_config_set_helper above remove_helper To adjust rankings for additional tools, add an entry to the _SEARCH_KEYWORDS dict in server.py — no other files need changing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
UAT results — master vs #727 (
|
| Story | Title | master (fa925fc) |
#727 search (c283cfa) |
|---|---|---|---|
| s01 | Create sunset automation | ✅ 89s 39,025tok 2calls | ✅ 27s 5,493tok 1calls |
| s02 | Motion-activated light | ✅ 66s 39,062tok 3calls | ✅ 28s 5,530tok 2calls |
| s03 | Debug automation not firing | ✅ 64s 39,030tok 6calls | ❌ 72s 5,498tok 11calls |
| s04 | Create room dashboard | ✅ 70s 39,047tok 3calls | ❌ 6s — 0calls |
| s05 | Discover entities | ✅ 72s 39,027tok 7calls | ✅ 39s 5,495tok 1calls |
| s06 | Goodnight routine script | ✅ 68s 39,044tok 4calls | ❌ 116s 5,512tok 20calls |
| s07 | Update automation + condition | ❌ 133s 39,055tok 20calls | ✅ 34s 5,523tok 6calls |
| s08 | Complex multi-condition auto | ✅ 129s 39,092tok 12calls | ✅ 24s 5,560tok 2calls |
| s09 | Vacation mode toggle | ❌ 151s 39,057tok 20calls | ✅ 29s 5,525tok 5calls |
| s10 | Create areas + organize | ✅ 61s 39,041tok 4calls | ✅ 34s 5,509tok 6calls |
| s11 | History analysis | ❌ 102s 39,056tok 7calls | ❌ 43s 5,524tok 5calls |
| s12 | Create + assign labels | ✅ 65s 39,059tok 4calls | ✅ 31s 5,527tok 5calls |
| s13 | Find automations by entity | ✅ 86s 38,994tok 8calls | ✅ 13s 5,462tok 2calls |
| TOTAL (all 13) | 10/13 pass | 9/13 pass | |
| TOTAL (both passed, 7 stories) | s01 s02 s05 s08 s10 s12 s13 | 568s 39,042tok avg 40calls | 196s (−65%) 5,510tok avg (−86%) 19calls (−52%) |
Notes:
- Aggregate numbers are compared only on stories both branches passed (7/13), to avoid skew from stories where one branch hit the 20-call limit or a model crash.
- First-turn context is ~39K on master vs ~5.5K on the PR branch. Higher than the claimed 2–3K because this model generates more verbose tool schema output; the reduction ratio is consistent with the PR's claim.
- Pass rate is comparable (10/13 vs 9/13) and the failures are largely on different stories — the PR fixed s07 and s09 which master fails, but regressed s03 and s06.
- s03/s06 regressions appear to be the model struggling with the search-first workflow (s06 hit the 20-call limit rather than giving up), not functional gaps in the tools.
- s04: model crashed (exit 18) before executing any tool calls on the PR branch — not a bug in the PR, likely a schema/context issue specific to this model.
- s11 (history analysis) fails on both branches with this model.
The context reduction numbers check out and the pass rate is comparable for a 9B local model. The implementation looks good to me.
sergeykad
left a comment
There was a problem hiding this comment.
One issue cannot be placed inline because the relevant lines are not in a diff hunk:
ha_deep_search docstring says default 20, code says 5 (src/ha_mcp/tools/tools_search.py:658)
The docstring at line 658 says limit: Maximum total results to return (default: 20), but the function signature at line 632 has limit: int = 5. A caller reading the docstring will expect 20 results by default but receive 5. Fix the docstring to match the actual default.
- Generate instruction text from DEFAULT_PINNED_TOOLS instead of hardcoding - Log ERROR (not WARNING) when CategorizedSearchTransform import fails - Change logger.debug → logger.warning for notification fetch failures - Add direct-call option to tool_discovery hint for consistency - Rename _unused_call_tool → _base_call_proxy for clarity - Use Literal["read", "write", "delete"] type for category parameter - Add asyncio.Lock to prevent concurrent cache rebuilds - Fix ha_deep_search docstring: default is 5, not 20 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Re: |
…staleness - Move lazy imports to top of tools_search.py (no circular dependency exists) - Build proxy descriptions from configured search_tool_name instead of hardcoding - Replace boolean _cache_built with hash-based staleness detection matching BM25SearchTransform's pattern — category cache now rebuilds when catalog changes - Use local variable accumulation + atomic swap for cache thread safety Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…and SearchKeywordsTransform Covers the gaps identified in review: - categorized_call happy path for read/write/delete proxies - Wrong-category rejection with correct proxy suggestion in error - Unknown tool returns RESOURCE_NOT_FOUND - Double-unwrap detection and correct unwrapping - Double-unwrap wrong-category still rejected (security guarantee) - _rebuild_category_cache populates sets, updates on change, no-ops when unchanged - SearchKeywordsTransform: append, override, priority, no-match, get_tool Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t_tools await The hash-based cache always calls get_tool_catalog(ctx) to compute the current hash, even when the cache is already populated. Mock it on the transform so the hash comparison succeeds without hitting the real FastMCP context. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Error responses use {"error": {"code": ..., "message": ...}} not flat keys.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Documentation is incomplete for 1. Production add-on config missing the option options:
enable_tool_search: false
schema:
enable_tool_search: bool?2. Production add-on translation missing 3. 4. 5. No README/docs entry |
Address review feedback: - Production config.yaml: add enable_tool_search option and schema - Production translations/en.yaml: add UI description - .env.example: add commented ENABLE_TOOL_SEARCH entry - DOCS.md: document when to enable/disable with guidance Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Addressed 4 of 5 items in 7aceb08: 1. Production config.yaml — Added 2. Production translations — Added matching 3. 4. # Tool search transform — replaces the full tool catalog with a unified
# BM25 search tool and categorized call proxies (read/write/delete).
# Dramatically reduces idle context token usage for LLMs.
enable_tool_search: bool = Field(False, alias="ENABLE_TOOL_SEARCH")User-facing descriptions live in 5. DOCS.md — Added |
- Fix token reduction figure to ~5K (was ~2-3K) - Add guidance to toggle description: "Use this if using an LLM without deferred tools or with smaller context windows" - Clarify in DOCS.md that Claude Haiku does not use deferred tool loading, so Haiku users benefit from enabling tool search - Update both production and dev translations Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tool base class has no .fn attribute (only FunctionTool does). Use model_copy(update=...) to override description and annotations on the search tool, which preserves all fields including fn. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The module-level `mcp` object is _DeferredMCP (a transparent proxy), not FastMCP directly. The FastMCP type hint causes mypy to reject the call in start.py. Using Any since _DeferredMCP duck-types as FastMCP via __getattr__. This is a master-level issue (homeassistant-ai#777 + homeassistant-ai#793 interaction) affecting all open PRs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
upstream/master adopted `"FastMCP | _DeferredMCP"` (PR homeassistant-ai#800) over the `Any` approach this branch had. Keep master's version. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🧪 Your changes are now in the dev channel!Your PR has been merged to master and is available for testing in the dev channel. Test your changes before the next stable release (biweekly Wednesday): Quick start# Run dev version
uvx ha-mcp-dev
# Check version
uvx ha-mcp-dev --versionDocker: docker pull ghcr.io/homeassistant-ai/ha-mcp:dev
docker run --rm -i \
-e HOMEASSISTANT_URL=http://your-ha:8123 \
-e HOMEASSISTANT_TOKEN=your_token \
ghcr.io/homeassistant-ai/ha-mcp:devFound an issue? Please open a new bug report and mention this PR for context. |
What does this PR do?
Adds an opt-in
CategorizedSearchTransform(ENABLE_TOOL_SEARCH=true) that replaces the full 89+ tool catalog with a unified BM25 search tool and three categorized call proxies (read/write/delete). Reduces idle context from ~46K tokens to ~2-3K tokens while maintaining full functionality and client compatibility.How it works:
search_tools— unified BM25 search across all tools (results include full annotations so the LLM knows read vs write vs delete)call_read_tool—readOnlyHint: true, clients can auto-approvecall_write_tool—destructiveHint: true, requires confirmationcall_delete_tool—destructiveHint: true, requires confirmationPinned tools (always visible for individual permission gating):
ha_restart,ha_reload_core,ha_backup_create,ha_backup_restore,ha_get_overview,ha_report_issue, andlist_resources/read_resource(whenENABLE_SKILLS_AS_TOOLS=true)Bootstrap for LLM awareness:
instructionsupdated with tool discovery workflow when enabledha_get_overviewresponse includestool_discoverysection with hints and pinned tool listBM25 search quality tuning (
SearchKeywordsTransform):Transformsubclass that appends extra keywords to specific tool descriptions so BM25 ranks them correctly for common queriesENABLE_TOOL_SEARCH=true— original tool docstrings are unchangedha_search_entitiesranks aboveha_deep_searchha_config_get_automationranks aboveha_config_set_automationha_config_set_helperranks aboveha_config_remove_helper_SEARCH_KEYWORDSdict inserver.py— no other files need changingSubclasses FastMCP 3.1's
BM25SearchTransform(~200 lines). Tools auto-categorized by existing MCP annotations. Works with ANY MCP client. Pure opt-in — disable the flag and everything works exactly as before.Related: #637 (category gateways approach), #726 (CodeMode sandbox)
Type of change
Testing
uv run pytest)uv run ruff check)Checklist
🤖 Generated with Claude Code