Fix service actions being identified as entities #1076
Conversation
frenck
added
the
bugfix
📝 WalkthroughWalkthroughPropagates a hass: HomeAssistant parameter through automation and script unknown-entity repairs and shared util functions. Updates template/config extraction signatures and call sites to be hass-aware, and adds regex-based filtering of known Home Assistant services during template parsing to avoid misclassifying service names as entities. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant AutomationRepair as Automation Repair
participant ScriptRepair as Script Repair
participant Util as util.py
participant HA as hass (HomeAssistant)
User->>AutomationRepair: async_inspect()
AutomationRepair->>Util: async_extract_entities_from_config(hass, config)
Util->>Util: parse config, collect template strings
Util->>Util: async_extract_entities_from_template_string(hass, str)
Util->>Util: extract_entities_from_template_regex(hass, str)
Util->>HA: get known services
Util->>Util: subtract services from found tokens
Util-->>AutomationRepair: entity_ids (services filtered)
AutomationRepair-->>User: unknown entity references
User->>ScriptRepair: async_inspect()
ScriptRepair->>Util: async_extract_entities_from_config(hass, config)
Util-->>ScriptRepair: entity_ids (services filtered)
ScriptRepair-->>User: unknown entity references
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
custom_components/spook/util.py (2)
83-91: Restricting entity domain regex to a fixed list risks missing custom domains (false negatives)Using KNOWN_DOMAINS in _DOMAIN means entity IDs from custom integrations (and possibly core domains not listed) won’t be detected in templates. That undermines the repair’s ability to catch unknown entities outside the curated list.
Prefer HA’s canonical entity-id domain pattern (same as _OBJECT_ID) or derive domains dynamically from current entity_registry/states:
-# Modified _DOMAIN pattern to only match known domains -_DOMAIN = r"(?:" + "|".join(KNOWN_DOMAINS) + r")" +# Domain: same pattern as HA core uses for object IDs (allow any valid domain) +_DOMAIN = _OBJECT_IDThis still pairs well with your service filtering and avoids missing real entity references from custom domains.
400-413: Bug: floors filter uses label IDs instead of floor IDsknown_floor_ids defaults to async_get_all_label_ids, which is incorrect and will misclassify floors.
Apply:
- if known_floor_ids is None: - known_floor_ids = async_get_all_label_ids(hass) + if known_floor_ids is None: + known_floor_ids = async_get_all_floor_ids(hass)custom_components/spook/ectoplasms/automation/repairs/unknown_entity_references.py (1)
26-43: New hass-aware automation template extractorApproach is sound. Same docstring note: it mentions RenderInfo but returns regex-only results via util.
Align docstring or add RenderInfo extraction for parity with the description.
🧹 Nitpick comments (3)
custom_components/spook/util.py (2)
499-533: Filtering out known services from regex matches fixes the false positivesThis directly addresses #1068 by subtracting hass-known services. LGTM.
Optional: normalize to lowercase for both sets to be extra safe against any case variance (entities are lowercase by convention, but templates may vary):
- known_services = async_get_all_services(hass) - return entities - known_services + known_services = {s.lower() for s in async_get_all_services(hass)} + return {e.lower() for e in entities} - known_services
670-697: hass-aware config extractionGood change and correct forwarding to the hass-aware template extractor. Consider minor optimization: pass a precomputed known_services set into regex extraction if you end up calling it in tight loops (micro).
custom_components/spook/ectoplasms/automation/repairs/unknown_entity_references.py (1)
223-258: Template value extraction now hass-awareCorrectly routes template strings through hass-aware extractor; fallback for dotted strings preserved. Consider filtering out known services here too as a belt-and-braces (rare, but harmless).
- elif "." in value and not value.startswith("!"): + elif "." in value and not value.startswith("!"): # Check if it looks like an entity ID - entities.add(value) + entities.add(value)(If you adopt this, subtract known services via util before adding.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
custom_components/spook/ectoplasms/automation/repairs/unknown_entity_references.py(4 hunks)custom_components/spook/ectoplasms/script/repairs/unknown_entity_references.py(5 hunks)custom_components/spook/util.py(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
custom_components/spook/ectoplasms/automation/repairs/unknown_entity_references.py (1)
custom_components/spook/util.py (5)
async_extract_entities_from_config(670-696)async_extract_entities_from_template_string(471-496)async_filter_known_entity_ids_with_templates(583-625)async_get_all_entity_ids(190-223)is_template_string(464-468)
custom_components/spook/ectoplasms/script/repairs/unknown_entity_references.py (1)
custom_components/spook/util.py (3)
async_extract_entities_from_config(670-696)async_filter_known_entity_ids_with_templates(583-625)async_get_all_entity_ids(190-223)
🔇 Additional comments (9)
custom_components/spook/util.py (2)
562-571: hass propagated into template-string processingGood propagation; consistent with the new signatures.
472-496: async_extract_entities_from_template_string: all call sites updated All invocations now pass bothhassandtemplate_str.custom_components/spook/ectoplasms/script/repairs/unknown_entity_references.py (4)
14-17: Updated util import: hass-aware call pathImporting the hass-aware extractor is correct for the new flow.
19-21: TYPE_CHECKING HomeAssistant importNice to keep runtime surface clean while retaining type hints.
76-76: Forwarding hass into config extractorCorrect fix; leverages service filtering from util.
145-147: Call site updated to pass hassConsistent with new signature. LGTM.
custom_components/spook/ectoplasms/automation/repairs/unknown_entity_references.py (3)
15-20: Imports updated to hass-aware utilitiesGood; is_template_string reuse keeps logic centralized.
45-73: Config-level extraction flowBreakdown across trigger/condition/action is clear and avoids scanning “service” fields, preventing the original false positives.
305-307: Call site updated to pass hassMatches new signature. LGTM.
| async def extract_template_entities_from_script_entity( | ||
| hass: HomeAssistant, entity: Any | ||
| ) -> set[str]: | ||
| """Extract entities from script configuration using Template analysis. |
There was a problem hiding this comment.
hass added to extractor signature; docstring overpromises RenderInfo
Function returns regex-based results via util, not RenderInfo. Either update the docstring or implement RenderInfo-based extraction here.
I can provide a RenderInfo-based implementation if desired.
🤖 Prompt for AI Agents
In
custom_components/spook/ectoplasms/script/repairs/unknown_entity_references.py
around lines 54 to 57, the docstring falsely claims the function returns
RenderInfo while the implementation actually returns a set of entity IDs
extracted via the regex-based util; update the docstring to accurately state
that the function returns a set[str] of entity ids extracted using the util
(e.g., util.extract_entities_from_template or the specific helper used), include
expected input (hass, entity) and format of output, or alternatively replace the
current extraction logic with a RenderInfo-based implementation by calling Home
Assistant's async render-info helpers (async_get_render_info /
template.async_render) and deriving entities from the RenderInfo object — pick
one approach and make the docstring consistent with the chosen implementation.
| hass: HomeAssistant, | ||
| template: Template, | ||
| known_entity_ids: set[str], | ||
| unknown_entities: set[str], | ||
| ) -> None: | ||
| """Process a Template object and add unknown entities to the set.""" | ||
| template_entities = set() | ||
|
|
||
| # Use regex patterns on the template string | ||
| try: | ||
| if hasattr(template, "template") and template.template: | ||
| regex_entities = extract_entities_from_template_regex(template.template) | ||
| regex_entities = extract_entities_from_template_regex( | ||
| hass, template.template | ||
| ) | ||
| template_entities.update(regex_entities) |
There was a problem hiding this comment.
Template processing only uses regex; docstring mentions RenderInfo
Docstring implies RenderInfo/Template.async_render_to_info(), but only regex extraction is used here. Align docs or add RenderInfo-based extraction if intended.
Happy to draft a RenderInfo-based pass to complement regex for higher fidelity.
🤖 Prompt for AI Agents
In custom_components/spook/util.py around lines 536 to 550, the function
docstring promises RenderInfo-based extraction but the code only uses regex; add
a RenderInfo pass to complement regex by attempting to call
template.async_render_to_info(...) (awaiting it), extract entities from the
returned RenderInfo (e.g., render_info.entities or render_info.extract_entities)
and update template_entities with those, catching and ignoring any exceptions
and falling back to the existing regex extraction; ensure the call is guarded
with hasattr(template, "async_render_to_info") and that you await it properly,
then keep the regex step as a fallback so both sources are combined.
Description
fixes #1068
Motivation and Context
How has this been tested?
Screenshots (if appropriate):
Types of changes
Checklist