Feature/UI parity list_filtered_entities #1048
Conversation
…rations selector options; UI-parity filtering/search; truthy-only status
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a new Home Assistant service Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor HA as Home Assistant Core
participant Spook as SpookService
participant Yaml as services.yaml
participant Reg as Registries (Entity/Device/Area/Label)
participant States as State Machine
rect rgba(200,220,255,0.25)
note over HA,Spook: Startup → dynamic schema registration
HA->>Spook: Initialize component
Spook->>Yaml: Load base schema block (homeassistant_list_filtered_entities)
alt schema found
Spook->>Reg: Collect live domains/integration platforms
Spook->>States: Collect domains from active states
Spook->>HA: async_set_service_schema(updated selectors with runtime options)
else not found
Spook-->>HA: Skip schema registration
end
end
sequenceDiagram
autonumber
actor Caller as Service Caller
participant HA as Home Assistant
participant Spook as SpookService
participant Reg as Registries (Entity/Device/Area/Label)
participant States as State Machine
rect rgba(220,255,220,0.25)
note over Caller,Spook: Service invocation and filtering flow
Caller->>HA: call homeassistant.list_filtered_entities(payload)
HA->>Spook: async_handle_service(call)
Spook->>Spook: Validate/normalize inputs (cap limit)
Spook->>Reg: Fetch registry entities (+ metadata)
Spook->>States: Include state-only entities
Spook->>Spook: Apply search/status/area/device/domain/integration/label filters
Spook->>Reg: Resolve area names, labels, integration titles
Spook-->>HA: { count, entities[IDs or enriched dicts] } (sorted, limited)
HA-->>Caller: Service response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
custom_components/spook/services.yaml (1)
1373-1383: Correct limit description to match default behavior and docs.Description says “Defaults to 50,000” but
default: 500is set and docs claim 500 by default. Align the description.- description: >- - Maximum number of entities to return. Defaults to 50,000 if not specified. - Maximum allowed is 50,000 for safety. + description: >- + Maximum number of entities to return. Defaults to 500. + Maximum allowed is 50,000 for safety.
🧹 Nitpick comments (4)
documentation/entities.md (1)
298-341: Document the action response shape.You state the action has a response, but the “Action response data” table is missing here (unlike the orphaned DB entities action). Add a short table for
countandentities.* - {term}`Action response` - Yes @@ ```{list-table} +:header-rows: 2 +* - Action response data +* - Attribute + - Type +* - `count` + - {term}`integer <integer>` +* - `entities` + - {term}`list <list>` +```custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (3)
161-169: Normalize single values to lists to harden API usage.Service schema enforces lists, but programmatic calls may send single strings. Normalize to lists defensively.
search = call.data.get("search", "") - areas = call.data.get("areas", []) - devices = call.data.get("devices", []) - domains = call.data.get("domains", []) - integrations = call.data.get("integrations", []) - status = call.data.get("status", []) - label_id = call.data.get("label_id", []) - values = call.data.get("values", []) + def as_list(v): + if v is None: + return [] + if isinstance(v, (list, tuple, set)): + return list(v) + return [v] + areas = as_list(call.data.get("areas")) + devices = as_list(call.data.get("devices")) + domains = as_list(call.data.get("domains")) + integrations = as_list(call.data.get("integrations")) + status = as_list(call.data.get("status")) + label_id = as_list(call.data.get("label_id")) + values = as_list(call.data.get("values"))
247-258: Avoid repeated registry lookups inside the loop.
dr.async_get(self.hass)is called multiple times per entity. Fetch registries once and reuse to cut overhead on large installations.- device_registry = dr.async_get(self.hass) + device_registry = dr.async_get(self.hass) @@ - area_registry = ar.async_get(self.hass) + area_registry = ar.async_get(self.hass) @@ - device_registry = dr.async_get(self.hass) if device := device_registry.async_get(entity_entry.device_id): area_id = device.area_id
77-85: Use public API to load services schema if available; guard private import.
_load_services_fileis private. It’s fine if Spook already uses it, but consider guarding for API changes or wrapping in a helper to reduce churn.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
documentation/images/entities/list_filtered_entities.pngis excluded by!**/*.png
📒 Files selected for processing (3)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py(1 hunks)custom_components/spook/services.yaml(1 hunks)documentation/entities.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (1)
custom_components/spook/services.py (1)
AbstractSpookService(78-114)
🪛 LanguageTool
documentation/entities.md
[grammar] ~358-~358: There might be a mistake here.
Context: ...sponse: - name: Entity's display name - device: Device name and ID (if applicable) - `...
(QB_NEW_EN)
[grammar] ~359-~359: There might be a mistake here.
Context: ...ice: Device name and ID (if applicable) - area: Area name and ID (if applicable) - in...
(QB_NEW_EN)
[grammar] ~360-~360: There might be a mistake here.
Context: ...area: Area name and ID (if applicable) - integration: Integration name - status: Current s...
(QB_NEW_EN)
[grammar] ~361-~361: There might be a mistake here.
Context: ...cable) - integration: Integration name - status: Current status (enabled/disabled, visi...
(QB_NEW_EN)
[grammar] ~362-~362: There might be a mistake here.
Context: ..., visible/hidden, available/unavailable) - icon: Entity's icon - created: Entity crea...
(QB_NEW_EN)
[grammar] ~363-~363: There might be a mistake here.
Context: ...ble/unavailable) - icon: Entity's icon - created: Entity creation timestamp - modified...
(QB_NEW_EN)
[grammar] ~364-~364: There might be a mistake here.
Context: ...n - created: Entity creation timestamp - modified: Entity last modified timestamp - `labe...
(QB_NEW_EN)
[grammar] ~365-~365: There might be a mistake here.
Context: ...odified: Entity last modified timestamp - labels`: Associated label names and IDs When n...
(QB_NEW_EN)
[grammar] ~372-~372: There might be a mistake here.
Context: ...on in {term}YAML` :class: dropdown Simple entity ID list:...
(QB_NEW_EN)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py
Outdated
Show resolved
Hide resolved
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py
Outdated
Show resolved
Hide resolved
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…_filtered_entities.py Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
documentation/entities.md (1)
421-432: Fix example: label_id expects IDs, not names.Using names under
label_idwill not match; provide label IDs (UUID/slug-like), as the schema requires.action: homeassistant.list_filtered_entities data: - label_id: - - security - - automation_controlled + label_id: + - 2f4f1d7b9a3a4b7c9d3e1a2b3c4d5e6f + - 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d values: - name - labels - area
🧹 Nitpick comments (3)
documentation/entities.md (3)
298-341: Add “Action response data” table for parity and clarity.Other sections (e.g., orphaned DB entities) show response shape. Add the table here too.
- `500`+```{list-table}
+:header-rows: 2
+* - Action response data
+* - Attribute
- Type
- Description
+* -count
- {term}
integer <integer>
- Number of entities returned (after filtering and limit)
+* -entities
- {term}
list <list>
- Either a list of
entity_idstrings, or a list of objects whenvaluesare requested
+```--- `347-355`: **Clarify “not_provided” status wording.** Align with HA terminology to reduce ambiguity. ```diff -- `not_provided`: Entity state is restored (no live provider) +- `not_provided`: State is restored (no live provider/state not currently supplied)
338-341: Document the explicit maximum forlimit
Thelimitparameter is capped at 50,000 in the code (seeMAX_ENTITIES_LIMIT = 50000incustom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py:26). To avoid confusion, surface this cap alongside the default in the docs:• File:
documentation/entities.md
Lines: 338-341* - `limit` - {term}`integer <integer>` - No - - `500` + - `500 (max 50,000)`This is a minor clarity improvement but will help users understand both the default and the hard cap.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py(1 hunks)custom_components/spook/services.yaml(1 hunks)documentation/entities.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- custom_components/spook/services.yaml
- custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py
🧰 Additional context used
🪛 LanguageTool
documentation/entities.md
[grammar] ~358-~358: There might be a mistake here.
Context: ...sponse: - name: Entity's display name - device: Device name and ID (if applicable) - `...
(QB_NEW_EN)
[grammar] ~359-~359: There might be a mistake here.
Context: ...ice: Device name and ID (if applicable) - area: Area name and ID (if applicable) - in...
(QB_NEW_EN)
[grammar] ~360-~360: There might be a mistake here.
Context: ...area: Area name and ID (if applicable) - integration: Integration name - status: Current s...
(QB_NEW_EN)
[grammar] ~361-~361: There might be a mistake here.
Context: ...cable) - integration: Integration name - status: Current status (enabled/disabled, visi...
(QB_NEW_EN)
[grammar] ~362-~362: There might be a mistake here.
Context: ..., visible/hidden, available/unavailable) - icon: Entity's icon - created: Entity crea...
(QB_NEW_EN)
[grammar] ~363-~363: There might be a mistake here.
Context: ...ble/unavailable) - icon: Entity's icon - created: Entity creation timestamp - modified...
(QB_NEW_EN)
[grammar] ~364-~364: There might be a mistake here.
Context: ...n - created: Entity creation timestamp - modified: Entity last modified timestamp - `labe...
(QB_NEW_EN)
[grammar] ~365-~365: There might be a mistake here.
Context: ...odified: Entity last modified timestamp - labels`: Associated label names and IDs When n...
(QB_NEW_EN)
[grammar] ~372-~372: There might be a mistake here.
Context: ...on in {term}YAML` :class: dropdown Simple entity ID list:...
(QB_NEW_EN)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (6)
339-348: Don’t rely on private attribute fallback for config entries.Accessing config_entries._entries is fragile across HA releases; prefer returning None when async_get_entry isn’t available.
- config_entries = self.hass.config_entries - if hasattr(config_entries, "async_get_entry"): - config_entry = config_entries.async_get_entry(entity_entry.config_entry_id) - else: - # Fallback to private access if public method not available - config_entry = getattr(config_entries, "_entries", {}).get( - entity_entry.config_entry_id - ) + config_entries = self.hass.config_entries + config_entry = None + if hasattr(config_entries, "async_get_entry"): + config_entry = config_entries.async_get_entry(entity_entry.config_entry_id)
95-99: Good: iterate states without copying.Using self.hass.states.async_all() directly avoids unnecessary memory.
205-223: Remove early-return limiting; enforce limit after sort for deterministic results.Early returns make the result order depend on registry/state iteration and can diverge from UI parity. Gather all matches, sort, then slice.
@@ - item = self._append_if_match(entity_entry, entity_data, filters, values) - if item is not None: - matching_entities.append(item) - if len(matching_entities) >= limit: - return matching_entities + item = self._append_if_match(entity_entry, entity_data, filters, values) + if item is not None: + matching_entities.append(item) @@ - item = self._append_if_match(entry, entity_data, filters, values) - if item is not None: - matching_entities.append(item) - if len(matching_entities) >= limit: - return matching_entities + item = self._append_if_match(entry, entity_data, filters, values) + if item is not None: + matching_entities.append(item) @@ - return matching_entities + return matching_entitiesApply with the related changes in async_handle_service and _format_response (see separate comments).
152-156: Pass limit into formatter and slice post-sort.- matching_entities = self._find_matching_entities(filters, values, limit) - return self._format_response(matching_entities) + matching_entities = self._find_matching_entities(filters, values, limit) + return self._format_response(matching_entities, limit)
542-558: Apply limit after sorting to guarantee deterministic selection.- def _format_response( - self, matching_entities: list[str | dict[str, Any]] - ) -> ServiceResponse: + def _format_response( + self, matching_entities: list[str | dict[str, Any]], limit: int + ) -> ServiceResponse: @@ - matching_entities[:] = dict_entities + matching_entities[:] = dict_entities else: @@ - matching_entities[:] = string_entities + matching_entities[:] = string_entities + + # Enforce limit after sort for determinism + if limit is not None: + matching_entities = matching_entities[:limit] @@ - return { + return { "count": len(matching_entities), "entities": matching_entities, }
317-326: Fix “unavailable” vs “unknown” semantics; add explicit unavailable flag.Currently “unavailable” == not available, which incorrectly matches “unknown”. Set an explicit unavailable status and use it in filters/search terms.
@@ def _get_state_only_entity_data(self, entity_id: str) -> tuple[SimpleNamespace, dict[str, Any]]: - if state is not None and state.state not in ("unavailable", "unknown"): + if state is not None and state.state not in ("unavailable", "unknown"): status["available"] = True if state is not None and state.state == "unknown": status["unknown"] = True + if state is not None and state.state == "unavailable": + status["unavailable"] = True @@ def _get_status_info(self, entity_entry: er.RegistryEntry) -> dict[str, Any]: - if state is not None and state.state not in ("unavailable", "unknown"): + if state is not None and state.state not in ("unavailable", "unknown"): status["available"] = True - - if state is not None and state.state == "unknown": + if state is not None and state.state == "unknown": status["unknown"] = True + if state is not None and state.state == "unavailable": + status["unavailable"] = True @@ def _status_terms(self, status: dict[str, Any] | None) -> list[str]: - if status.get("available"): - terms.append("available") - else: - terms.append("unavailable") + if status.get("available"): + terms.append("available") + if status.get("unknown"): + terms.append("unknown") + if status.get("unavailable"): + terms.append("unavailable") @@ def _matches_status_filter(self, entity_data: dict[str, Any], status_filters: list[str]) -> bool: status_checks = { "available": lambda s: s.get("available", False), - "unavailable": lambda s: not s.get("available", False), + "unavailable": lambda s: s.get("unavailable", False), "enabled": lambda s: not s.get("disabled_by"), "disabled": lambda s: bool(s.get("disabled_by")), "visible": lambda s: not s.get("hidden_by"), "hidden": lambda s: bool(s.get("hidden_by")), "not_provided": lambda s: s.get("not_provided", False), "unmanageable": lambda s: s.get("unmanageable", False), }Also applies to: 360-372, 416-420, 481-491
🧹 Nitpick comments (1)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (1)
250-261: Avoid redundant registry fetch.dr.async_get(self.hass) is called twice; reuse the first result.
- device_registry = dr.async_get(self.hass) + device_registry = dr.async_get(self.hass) @@ - device_registry = dr.async_get(self.hass) - if device := device_registry.async_get(entity_entry.device_id): + if device := device_registry.async_get(entity_entry.device_id): area_id = device.area_id
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (1)
custom_components/spook/services.py (1)
AbstractSpookService(78-114)
🔇 Additional comments (4)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (4)
26-27: Defaults/caps defined correctly.MAX_ENTITIES_LIMIT=50_000 and DEFAULT_ENTITIES_LIMIT=500 look good.
172-177: Correct default (500) and safety cap (50k) handling.This resolves earlier feedback about the default limit mismatch.
44-77: Runtime schema injection looks solid.Gracefully skips when base schema block is missing and injects live options on HA start. Nice.
26-27: Double-check docs/services.yaml align with 500 default and 50k cap.Ensure end-user docs and the services.yaml selector default reflect the code.
Also applies to: 172-177
…emantics like HA UI
…fig entry missing
…ion regression in async_handle_service
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
documentation/entities.md (1)
421-432: Fix example: provide label IDs, not names.This was already flagged earlier; the example still shows names.
action: homeassistant.list_filtered_entities data: label_id: - - security - - automation_controlled + - 2f4f1d7b9a3a4b7c9d3e1a2b3c4d5e6f + - 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d values: - name - labels - area
🧹 Nitpick comments (2)
documentation/entities.md (2)
342-342: Add response schema table for parity with other actions.Document the response shape (
count,entities) like other sections.+```{list-table}
+:header-rows: 2
+* - Action response data
+* - Attribute
- Type
+* -count
- {term}
integer <integer>
+* -entities
- {term}
list <list>
+```The
searchparameter searches across entity IDs, domains, integration names, display names, device names, area names, and label names. Matching is case-insensitive and based on substring containment.--- `356-367`: **Tighten phrasing in value descriptions (minor style).** Use parallel, possessive-free phrasing to avoid LanguageTool nits. ```diff -- `name`: Entity's display name -`device`: Device name and ID (if applicable) -`area`: Area name and ID (if applicable) -`integration`: Integration name -`status`: Current status (enabled/disabled, visible/hidden, available/unavailable) -`icon`: Entity's icon -`created`: Entity creation timestamp -`modified`: Entity last modified timestamp -`labels`: Associated label names and IDs +- `name`: Display name +- `device`: Device name and ID (if applicable) +- `area`: Area name and ID (if applicable) +- `integration`: Integration name +- `status`: Current status (enabled/disabled, visible/hidden, available/unavailable) +- `icon`: Icon +- `created`: Creation timestamp +- `modified`: Last modified timestamp +- `labels`: Associated label names and IDs
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
documentation/entities.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
documentation/entities.md
[grammar] ~358-~358: There might be a mistake here.
Context: ...sponse: - name: Entity's display name - device: Device name and ID (if applicable) - `...
(QB_NEW_EN)
[grammar] ~359-~359: There might be a mistake here.
Context: ...ice: Device name and ID (if applicable) - area: Area name and ID (if applicable) - in...
(QB_NEW_EN)
[grammar] ~360-~360: There might be a mistake here.
Context: ...area: Area name and ID (if applicable) - integration: Integration name - status: Current s...
(QB_NEW_EN)
[grammar] ~361-~361: There might be a mistake here.
Context: ...cable) - integration: Integration name - status: Current status (enabled/disabled, visi...
(QB_NEW_EN)
[grammar] ~362-~362: There might be a mistake here.
Context: ..., visible/hidden, available/unavailable) - icon: Entity's icon - created: Entity crea...
(QB_NEW_EN)
[grammar] ~363-~363: There might be a mistake here.
Context: ...ble/unavailable) - icon: Entity's icon - created: Entity creation timestamp - modified...
(QB_NEW_EN)
[grammar] ~364-~364: There might be a mistake here.
Context: ...n - created: Entity creation timestamp - modified: Entity last modified timestamp - `labe...
(QB_NEW_EN)
[grammar] ~365-~365: There might be a mistake here.
Context: ...odified: Entity last modified timestamp - labels`: Associated label names and IDs When n...
(QB_NEW_EN)
[grammar] ~372-~372: There might be a mistake here.
Context: ...on in {term}YAML` :class: dropdown Simple entity ID list:...
(QB_NEW_EN)
🔇 Additional comments (1)
documentation/entities.md (1)
370-371: Confirmed default (500) and maximum (50,000) limits are correctly documented and enforcedI’ve verified that:
- In
custom_components/spook/services.yaml(lines 1372–1382), thelimitparameter is documented with a default of 500, a maximum allowed value of 50,000, and the selector enforcesmin: 1andmax: 50000.- In
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py, the constantsDEFAULT_ENTITIES_LIMIT = 500andMAX_ENTITIES_LIMIT = 50000are defined, and the_parse_service_callmethod applies these values as the default and cap respectively.No discrepancies were found—no changes are needed here.
… -> labels; update schema and docs; add docs preview README
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
documentation/entities.md (2)
329-333: Parameter name should belabel_id, notlabels(expects IDs).Matches the service schema/implementation and prior review. Use IDs, not human-readable names.
-* - `labels` +* - `label_id` - {term}`string <string>` | {term}`list of strings <list>` - No - - `["security", "automation_controlled"]` + - `["2f4f1d7b9a3a4b7c9d3e1a2b3c4d5e6f", "1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d"]`
421-432: Fix example: uselabel_idwith actual IDs.The action consumes label IDs. Names won’t match.
action: homeassistant.list_filtered_entities data: - labels: - - security - - automation_controlled + label_id: + - 2f4f1d7b9a3a4b7c9d3e1a2b3c4d5e6f + - 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d values: - name - - labels + - labels - area
🧹 Nitpick comments (4)
documentation/README.md (1)
18-18: Fix markdownlint: wrap bare URL.- - Then open: http://localhost:3000 + - Then open: <http://localhost:3000>documentation/entities.md (2)
269-433: Deduplicate: section “List filtered entities” appears twice.Keep a single authoritative section to avoid drift between copies.
370-371: Add response schema table for consistency.Mirror “List all orphaned database entities” by documenting
countandentitiesfields.Would you like a patch adding a small “Action response data” list-table here?
custom_components/spook/services.yaml (1)
1249-1251: Tighten search description (remove redundancy).- Search term to filter entities. Searches across entity ID, domain, integration, - display names, device names, area names, integration names, and label names. + Search term to filter entities. Searches entity ID, domain, integration (domain/title), + display names, device names, area names, and label names.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py(1 hunks)custom_components/spook/services.yaml(1 hunks)documentation/README.md(1 hunks)documentation/entities.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py
🧰 Additional context used
🪛 LanguageTool
documentation/entities.md
[grammar] ~358-~358: There might be a mistake here.
Context: ...sponse: - name: Entity's display name - device: Device name and ID (if applicable) - `...
(QB_NEW_EN)
[grammar] ~359-~359: There might be a mistake here.
Context: ...ice: Device name and ID (if applicable) - area: Area name and ID (if applicable) - in...
(QB_NEW_EN)
[grammar] ~360-~360: There might be a mistake here.
Context: ...area: Area name and ID (if applicable) - integration: Integration name - status: Current s...
(QB_NEW_EN)
[grammar] ~361-~361: There might be a mistake here.
Context: ...cable) - integration: Integration name - status: Current status (enabled/disabled, visi...
(QB_NEW_EN)
[grammar] ~362-~362: There might be a mistake here.
Context: ..., visible/hidden, available/unavailable) - icon: Entity's icon - created: Entity crea...
(QB_NEW_EN)
[grammar] ~363-~363: There might be a mistake here.
Context: ...ble/unavailable) - icon: Entity's icon - created: Entity creation timestamp - modified...
(QB_NEW_EN)
[grammar] ~364-~364: There might be a mistake here.
Context: ...n - created: Entity creation timestamp - modified: Entity last modified timestamp - `labe...
(QB_NEW_EN)
[grammar] ~365-~365: There might be a mistake here.
Context: ...odified: Entity last modified timestamp - labels`: Associated label names and IDs When n...
(QB_NEW_EN)
[grammar] ~372-~372: There might be a mistake here.
Context: ...on in {term}YAML` :class: dropdown Simple entity ID list:...
(QB_NEW_EN)
documentation/README.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...al > Run Task… > "Preview documentation" - It runs: `cd documentation && myst start...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
documentation/README.md
18-18: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (1)
custom_components/spook/services.yaml (1)
1373-1382: Limit defaults align with docs and code — LGTM.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (6)
10-10: Use HA state constants instead of string literals ("unknown"/"unavailable")Improves correctness and forward-compat with core constants.
-from homeassistant.const import EVENT_HOMEASSISTANT_STARTED +from homeassistant.const import ( + EVENT_HOMEASSISTANT_STARTED, + STATE_UNKNOWN, + STATE_UNAVAILABLE, +) @@ - if state is not None and state.state not in ("unavailable", "unknown"): + if state is not None and state.state not in (STATE_UNAVAILABLE, STATE_UNKNOWN): status["available"] = True - if state is not None and state.state == "unknown": + if state is not None and state.state == STATE_UNKNOWN: status["unknown"] = True - if state is not None and state.state == "unavailable": + if state is not None and state.state == STATE_UNAVAILABLE: status["unavailable"] = True @@ - if state is not None and state.state not in ("unavailable", "unknown"): + if state is not None and state.state not in (STATE_UNAVAILABLE, STATE_UNKNOWN): status["available"] = True - if state is not None and state.state == "unknown": + if state is not None and state.state == STATE_UNKNOWN: status["unknown"] = True - if state is not None and state.state == "unavailable": + if state is not None and state.state == STATE_UNAVAILABLE: status["unavailable"] = TrueAlso applies to: 309-315, 349-356
238-257: Prefer user-provided name and friendly_name; fall back to original_nameAligns search/display with HA UI naming semantics.
- data: dict[str, Any] = {} - if hasattr(entity_entry, "original_name") and entity_entry.original_name: - data["name"] = entity_entry.original_name + data: dict[str, Any] = {} + state = self.hass.states.get(entity_entry.entity_id) + # Prefer user-defined name, then original name, then state friendly_name + name_by_user = getattr(entity_entry, "name", None) + if name_by_user: + data["name"] = name_by_user + elif getattr(entity_entry, "original_name", None): + data["name"] = entity_entry.original_name + elif state and isinstance(state.attributes, dict): + friendly = state.attributes.get("friendly_name") + if isinstance(friendly, str) and friendly: + data["name"] = friendly
242-252: Avoid redundant registry fetchReuse device_registry instead of calling dr.async_get twice.
device_registry = dr.async_get(self.hass) if ( device := device_registry.async_get(entity_entry.device_id) ) and device.name: data["device_name"] = device.name @@ - if not area_id and entity_entry.device_id: - device_registry = dr.async_get(self.hass) - if device := device_registry.async_get(entity_entry.device_id): + if not area_id and entity_entry.device_id: + if device := device_registry.async_get(entity_entry.device_id): area_id = device.area_id
306-323: Include friendly_name for state-only entitiesImproves parity with UI search/display for entities without registry entries.
- data: dict[str, Any] = {} + data: dict[str, Any] = {} + if state is not None and isinstance(state.attributes, dict): + fn = state.attributes.get("friendly_name") + if isinstance(fn, str) and fn: + data["name"] = fn status: dict[str, Any] = {}
399-411: Add “unknown”, “enabled”, and “visible” to searchable status termsUsers often search these status words in the UI; adding them improves parity.
if status.get("available"): terms.append("available") if status.get("unavailable"): terms.append("unavailable") + if status.get("unknown"): + terms.append("unknown") + if not status.get("disabled_by"): + terms.append("enabled") + if not status.get("hidden_by"): + terms.append("visible") return terms
512-515: Expose integration_domain alongside integration_name in enriched resultsProvides both the slug (platform) and the human label—useful for automation logic.
- "integration": lambda: { - "integration_name": entity_data.get("integration_name") - }, + "integration": lambda: { + "integration_domain": getattr(entity_entry, "platform", None), + "integration_name": entity_data.get("integration_name"), + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (1)
custom_components/spook/services.py (1)
AbstractSpookService(78-114)
🔇 Additional comments (3)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (3)
44-76: Schema injection on HA start: solid approachDynamic options built from live domains/integrations and applied via async_set_service_schema look correct and robust.
533-548: Deterministic sort then apply limit: LGTMSorting by entity_id and slicing after sort ensures stable, reproducible results across runs.
149-186: Ignore label key mismatch suggestionThe
homeassistant_list_filtered_entitiesservice incustom_components/spook/services.yamlindeed defines the filter parameter aslabels:(notlabel_id:), and the parser inlist_filtered_entities.pycorrectly callscall.data.get("labels", []). There is no mismatch between schema and code for this service—other services uselabel_id, but this one is intentionallylabels. You can disregard the original comment.Likely an incorrect or invalid review comment.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…_filtered_entities.py Co-authored-by: Copilot <[email protected]>
…_filtered_entities.py Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (1)
78-86: Harden schema loading with error logging (repeat of prior feedback).Wrap _load_base_schema with try/except to avoid noisy tracebacks on loader issues and add a targeted error log.
@@ - async def _load_base_schema(self) -> dict[str, Any] | None: - """Load base services.yaml and return our service schema block.""" - integration = await async_get_integration(self.hass, "spook") - services = await self.hass.async_add_executor_job( - _load_services_file, self.hass, integration - ) - block = services.get("homeassistant_list_filtered_entities") - return block if isinstance(block, dict) else None + async def _load_base_schema(self) -> dict[str, Any] | None: + """Load base services.yaml and return our service schema block.""" + try: + integration = await async_get_integration(self.hass, "spook") + services = await self.hass.async_add_executor_job( + _load_services_file, self.hass, integration + ) + block = services.get("homeassistant_list_filtered_entities") + return block if isinstance(block, dict) else None + except Exception as err: # pragma: no cover + logging.getLogger(__name__).error( + "Failed to load schema for homeassistant_list_filtered_entities: %s", + err, + ) + return NoneAdditional import needed:
@@ -from datetime import datetime +from datetime import datetime +import logging
🧹 Nitpick comments (7)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (7)
155-175: Make limit parsing more robust (coerce, clamp negatives).Service schema should protect types, but direct API calls can pass bad values. Coerce to int and clamp to [0, MAX_ENTITIES_LIMIT].
@@ - limit = call.data.get("limit") - - # Default and cap - if limit is None: - limit = DEFAULT_ENTITIES_LIMIT - elif limit > MAX_ENTITIES_LIMIT: - limit = MAX_ENTITIES_LIMIT + limit = call.data.get("limit") + + # Default and cap + if limit is None: + limit = DEFAULT_ENTITIES_LIMIT + else: + try: + limit = int(limit) + except (TypeError, ValueError): + limit = DEFAULT_ENTITIES_LIMIT + if limit < 0: + limit = 0 + elif limit > MAX_ENTITIES_LIMIT: + limit = MAX_ENTITIES_LIMIT
236-257: Provide name fallback and avoid repeated device_registry lookup.Match HA UI behavior: prefer original_name, then state.friendly_name, else entity_id. Also reuse device_registry instead of re-fetching it.
@@ - if hasattr(entity_entry, "original_name") and entity_entry.original_name: - data["name"] = entity_entry.original_name + if hasattr(entity_entry, "original_name") and entity_entry.original_name: + data["name"] = entity_entry.original_name + else: + # Fallback: friendly_name from state, then entity_id + state = self.hass.states.get(entity_entry.entity_id) + if state: + attrs = getattr(state, "attributes", None) + if isinstance(attrs, dict) and attrs.get("friendly_name"): + data["name"] = attrs["friendly_name"] + if "name" not in data: + data["name"] = entity_entry.entity_id @@ - area_registry = ar.async_get(self.hass) + area_registry = ar.async_get(self.hass) area_id = entity_entry.area_id if not area_id and entity_entry.device_id: - device_registry = dr.async_get(self.hass) - if device := device_registry.async_get(entity_entry.device_id): + if device := device_registry.async_get(entity_entry.device_id): area_id = device.area_id
295-320: Confirm “not_provided” semantics for state-only entities.Entities without a registry entry are typically “Not provided” in the UI regardless of the restored flag. Consider setting not_provided unconditionally for state-only entries.
@@ - if state is not None: - attrs = getattr(state, "attributes", None) - if isinstance(attrs, dict) and attrs.get("restored", False): - status["not_provided"] = True + # State-only entities are not provided by a config entry/integration + status["not_provided"] = TrueWould you like me to align registry-backed entities similarly (e.g., mark not_provided when config_entry_id is None)?
394-412: Add “unknown” as a searchable status term.You already track unknown in status; exposing it to search improves parity and diagnostics.
@@ - if status.get("unavailable"): - terms.append("unavailable") + if status.get("unavailable"): + terms.append("unavailable") + if status.get("unknown"): + terms.append("unknown")
425-435: Broaden typing to cover state-only entries passed via SimpleNamespace.Signatures reference er.RegistryEntry but callers also pass SimpleNamespace. Use Any (or a Protocol) to silence type-checker noise.
@@ - def _entity_matches_filters( - self, - entity_entry: er.RegistryEntry, + def _entity_matches_filters( + self, + entity_entry: Any, @@ - def _matches_basic_filters( - self, - entity_entry: er.RegistryEntry, + def _matches_basic_filters( + self, + entity_entry: Any,Alternative: define a Protocol with entity_id: str and platform: str | None and use that instead of Any.
Also applies to: 436-445
533-548: Minor: limit is always int; drop the None guard.limit is guaranteed int after _parse_service_call; slice directly.
@@ - # Apply limit post-sort for deterministic results - if limit is not None: - matching_entities = matching_entities[:limit] + # Apply limit post-sort for determinism + matching_entities = matching_entities[:limit]
149-154: Consider adding basic tests for determinism and filters.Recommend unit/integration tests covering: default/cap on limit, deterministic sorting, OR status filter behavior, label_id filtering, and merged registry+state results with de-duplication.
I can scaffold pytest tests using Home Assistant’s core test helpers; want me to open a follow-up PR with a minimal suite?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (1)
custom_components/spook/services.py (1)
AbstractSpookService(78-114)
🔇 Additional comments (1)
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py (1)
44-77: Dynamic schema injection and options wiring look solid.Startup hook, live domain/integration discovery, and async_set_service_schema usage are clean. Nice touch on stable labeling via integration.name.
…_filtered_entities.py Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new homeassistant.list_filtered_entities action that provides UI parity with Home Assistant's entities page filtering capabilities. It enables dynamic entity querying with comprehensive filtering options for automations, scripts, and inspection workflows.
- Implements complete UI filtering parity including search, areas, devices, domains, integrations, status, and labels
- Adds runtime schema injection for domains and integrations dropdowns using live system data
- Supports both minimal (entity IDs only) and detailed entity information responses
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py |
Core service implementation with comprehensive filtering logic and dynamic schema management |
custom_components/spook/services.yaml |
Service definition with dynamic schema placeholders for domains/integrations |
documentation/entities.md |
Complete documentation with examples and parameter descriptions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py
Show resolved
Hide resolved
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py
Show resolved
Hide resolved
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py
Show resolved
Hide resolved
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py
Show resolved
Hide resolved
custom_components/spook/ectoplasms/homeassistant/services/list_filtered_entities.py
Show resolved
Hide resolved
|
@frenck Could we get a new-feature label applied to this PR to satisfy the PR Labels check? This adds a new action (homeassistant.list_filtered_entities) with UI-parity filtering, so new-feature seems appropriate. All other workflow review bots have passed, and pro-commit checks pass. Thanks! |
…sabled_by/hidden_by to strings) and keep status nested under single key; remove non-UI 'unknown' status handling
|
Description
homeassistant.list_filtered_entitiesThis action allows you to list entities with advanced filtering options, replicating the Home Assistant entities UI filtering capabilities. This is perfect for inspection workflows, scripts, and automations that need to query entities dynamically based on various criteria.
The action supports comprehensive filtering by search terms, areas, devices, domains, integrations, status, and labels. It can return either simple entity ID lists or detailed entity information including names, devices, areas, integrations, status, icons, timestamps, and labels.
It has dynamically populated domain and integration lists in the developer tools action UI.
live system (dropdown, multi-select, custom_value supported where applicable).
Actions UI dropdowns remain accurate without static YAML lists.
Motivation and Context
The entity list in the HA UI is pretty slick - you can quickly find most anything. Unfortunately you can't use it from an automation or script, or even export the results - so I built an action with the same features.
I started a discussion #1046 with this idea, and decided to take a shot at implementing it since I wanted it.
How has this been tested?
Screenshots (if appropriate):
Types of changes
Checklist