fix: parallelize deep_search Tier 3 config fetches (closes #879) - #882
Conversation
…ore prioritization When bulk config fetches fail (Tier 1/2), Tier 3 now fetches individual automation/script configs in parallel batches of 10 instead of sequentially. Crucially, fetch order is no longer prioritized by name-match score — all configs are fetched regardless of whether the automation name matches the query. This ensures entities referenced only inside conditions/actions (not in the automation name) are found by deep_search. Changes: - Replace sequential Tier 3 fetch with parallel asyncio.gather batches - Remove name-score prioritization for fetch order (score used only for result ranking after configs are fetched) - Make time budgets configurable via HAMCP_AUTOMATION_CONFIG_TIME_BUDGET and HAMCP_SCRIPT_CONFIG_TIME_BUDGET env vars - Increase default budgets (15s→30s automation, 10s→20s script) since parallel fetching covers more ground per second - Log warning when budget is exhausted with fetch/skip counts Closes homeassistant-ai#879 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 addresses an issue where automations and scripts were missing from deep search results when bulk configuration fetching failed. By switching to parallel batch processing and removing restrictive name-based prioritization, the system can now retrieve more configurations within the allotted time budget. Additionally, the inclusion of configurable time budgets and improved logging provides better reliability and diagnostic capabilities for users. Highlights
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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the deep_search functionality to fetch automation and script configurations in parallel batches, ensuring that matches within conditions and actions are identified even if the entity name does not match the query. It also allows the configuration of fetch time budgets via environment variables. Feedback includes a correction for a unit test where the mocked time budget is too high to verify budget exhaustion and a recommendation to add error handling when parsing environment variables to prevent potential crashes from invalid input.
…dict The asyncio.gather call in Tier 3 was assigned to `results`, overwriting the outer `results` dict that holds search results by category. Renamed to `batch_results` to avoid the collision. Also fixed test to use _make_tools pattern for proper settings mocking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…timing - Wrap env var float parsing in try/except to prevent startup crash on invalid values (Gemini review feedback) - Fix test_tier3_respects_time_budget: increase per-fetch sleep from 0.5s to 1.5s so parallel batches actually exceed the 2s budget (batches of 10 run concurrently, so 0.5s per fetch = 0.5s per batch < 2s budget) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
kingpanther13
left a comment
There was a problem hiding this comment.
✅ Fixed both in 19f4a1e: test sleep increased to 1.5s so batch execution exceeds budget, and env var parsing wrapped in try/except with fallback to defaults.
|
Assert call_count >= 10 (one full batch) instead of > 0. The comment describes expecting at least one complete batch, so the assertion should match. Thanks @Patch76. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@Patch76 Good catch — you're absolutely right. The lower bound Fixed in c5e7874: changed to |
|
LGTM 🚀 |
1. Remove return_exceptions=True — inner functions already handle errors 2. _env_float: log warning on invalid values, fix os.environ.get pattern 3. Add script-path test (TestAttemptCScriptParallelFetch) 4. Fix misleading "Fetch ALL" comment — now says "subject to time budget" 5. Rename Tier 3 → Attempt C for consistency with inline code comments 6. Track failed_count separately in budget warning messages 7. Replace fragile 1.5s sleep with 0.01s + proportional budget (0.005s) 8. Fix imprecise assertion comments — remove hedging language Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sergeykad
left a comment
There was a problem hiding this comment.
All 8 review items addressed. Code changes verified, CI green (Performance Tests failure is unrelated Docker rate-limiting).
🧪 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?
Fixes
ha_deep_searchmissing automations/scripts when bulk config fetch fails (Tier 1/2) and the Tier 3 fallback times out before fetching all configs.Root cause: Tier 3 fetched configs sequentially, prioritized by name-match score. Automations whose names didn't match the query were deprioritized and skipped when the time budget expired — even if the entity was referenced inside their conditions/actions.
Fix (3 changes):
Parallel batch fetching — Replace sequential
forloop withasyncio.gatherbatches of 10. This fetches 10x more configs in the same time window.Remove name-score prioritization for fetch order — All configs are now fetched without priority ordering. Name score is still used for result ranking after configs are retrieved. Deep search's purpose is to find matches inside configs, so deprioritizing by name defeats the purpose.
Configurable time budgets — Budgets are now configurable via
HAMCP_AUTOMATION_CONFIG_TIME_BUDGET(default: 30s, was 15s) andHAMCP_SCRIPT_CONFIG_TIME_BUDGET(default: 20s, was 10s). Higher defaults are safe since parallel fetching completes faster.Additional: Logs a warning with fetch/skip counts when the budget is exhausted, so users can diagnose incomplete results.
Closes #879
Type of change
Testing
uv run pytest)uv run ruff check)Checklist