Skip to content

Ignore notify targets in automation entity checks#1283

Open
frenckatron wants to merge 1 commit into
mainfrom
fix/issue-1253-stale-actions
Open

Ignore notify targets in automation entity checks#1283
frenckatron wants to merge 1 commit into
mainfrom
fix/issue-1253-stale-actions

Conversation

@frenckatron
Copy link
Copy Markdown
Collaborator

Description

Ignore data.target values on notify actions when extracting entity references from automation action data.

Notify targets are service-specific notification targets. They are not Home Assistant entity references, even when they look like notify.*. Other action data values are still scanned, including templates, and non-notify target values keep the existing behavior.

Motivation and Context

Fixes #1253.

Spook could keep reporting an old notify target as an unknown entity after an automation had already been updated. The scanner was treating the notify action's data.target as an entity ID, which made the repair noisy for mobile app notification targets.

How has this been tested?

  • ruff check custom_components/spook/ectoplasms/automation/repairs/unknown_entity_references.py tests/ectoplasms/automation/repairs/test_unknown_entity_references.py
  • ruff format --check custom_components/spook/ectoplasms/automation/repairs/unknown_entity_references.py tests/ectoplasms/automation/repairs/test_unknown_entity_references.py
  • pylint custom_components/spook/ectoplasms/automation/repairs/unknown_entity_references.py tests/ectoplasms/automation/repairs/test_unknown_entity_references.py
  • pytest -q tests/ectoplasms/automation/repairs/test_unknown_entity_references.py
  • pytest -q

Screenshots (if appropriate):

Not applicable.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@frenckatron frenckatron added the bugfix Inconsistencies or issues which will cause a problem for users or implementors. label May 29, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 29, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 24a89511-4f19-4649-9c7e-cbfc1d5c4f21

📥 Commits

Reviewing files that changed from the base of the PR and between b51ff68 and 68e35cf.

📒 Files selected for processing (2)
  • custom_components/spook/ectoplasms/automation/repairs/unknown_entity_references.py
  • tests/ectoplasms/automation/repairs/test_unknown_entity_references.py

📝 Walkthrough

Walkthrough

Automation entity-reference scanning now excludes data.target values from notify.* services, preventing false positives when notify actions reference notification targets (device names) instead of entity IDs. Helper functions determine service names and apply skip logic; tests validate the new special-casing for both action: and legacy service: notify forms.

Changes

Notify service target exclusion

Layer / File(s) Summary
Helper functions and entity extraction logic
custom_components/spook/ectoplasms/automation/repairs/unknown_entity_references.py
Two helpers extract the service/action name and decide whether to skip data.target values for notify.* services. _extract_entities_from_service_data now iterates key/value pairs and skips target scanning for notify services, instead of scanning all dict values unconditionally.
Test coverage for notify-service special-casing
tests/ectoplasms/automation/repairs/test_unknown_entity_references.py
Updated test_action_data_dict to use a non-notify service (light.turn_on). Added three new tests: action: notify.* with data.target is not extracted; legacy service: notify.* with data.target (including list form) is not extracted; non-notify services continue to extract data.target as entity references.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • frenck/spook#1275: Introduces the automation entity-reference extractor test suite that validates extraction behavior directly tied to this PR's implementation changes.
  • frenck/spook#977: Prior overhaul of _extract_entities_from_service_data that expanded extraction for templates and nested configs; this PR adds notify-service special-casing to that same function.
  • frenck/spook#1077: Earlier modification of entity-ID extraction behavior via ENTITY_ID_PATTERN tightening to avoid false entity matches.

Poem

🐰 A notify was crying "false alarm!" so loud,
The targets got tangled in entities shroud,
Now helpers decide with a skip-or-extract dance,
And notify targets get their second chance! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: ignoring notify targets in automation entity checks, which matches the core objective.
Description check ✅ Passed The description directly explains the change (ignoring data.target on notify actions), the problem it solves (false entity references), and testing approach.
Linked Issues check ✅ Passed The PR directly addresses issue #1253 by preventing Spook from treating notify action data.target as entity references, eliminating false positives for stale notify targets.
Out of Scope Changes check ✅ Passed All changes are scoped to the automation unknown entity reference repair module with helper functions and conditional skipping logic for notify services, directly supporting the fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-1253-stale-actions

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.82%. Comparing base (b51ff68) to head (68e35cf).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1283      +/-   ##
==========================================
+ Coverage   62.72%   62.82%   +0.09%     
==========================================
  Files         124      124              
  Lines        3147     3155       +8     
  Branches      403      404       +1     
==========================================
+ Hits         1974     1982       +8     
  Misses       1126     1126              
  Partials       47       47              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Inconsistencies or issues which will cause a problem for users or implementors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False positive on unknown action after automation is fixed

2 participants