Fix inverse helper device linking (breaks in HA 2026.8) #1227
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds config-entry migration and a helper to resolve a source entity's device_id, updates inverse entities to accept Changes
Sequence DiagramsequenceDiagram
participant ConfigEntry
participant Migrator as async_migrate_entry
participant EntityReg as EntityRegistry
participant DeviceReg as DeviceRegistry
participant SourceDevice
ConfigEntry->>Migrator: run migration (version==1, minor < MIGRATION_MINOR_VERSION)
Migrator->>EntityReg: lookup entity (options[CONF_ENTITY_ID]) -> get device_id
EntityReg-->>Migrator: device_id or None
alt device_id found
Migrator->>DeviceReg: async_remove_helper_config_entry_from_source_device(entry_id, device_id)
DeviceReg->>SourceDevice: remove helper config entry
SourceDevice-->>DeviceReg: ack
DeviceReg-->>Migrator: removal complete
end
Migrator->>ConfigEntry: update options, set minor_version = MIGRATION_MINOR_VERSION
ConfigEntry-->>Migrator: persisted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
custom_components/spook/integrations/spook_inverse/__init__.py (1)
77-91: Consider addressing static analysis hints (optional).Ruff flagged two style issues:
- PLR2004: Magic value
2in the comparison- SIM102: Nested
ifstatements could be combinedThese are minor style improvements and don't affect correctness. The migration logic itself is correct.
Optional refactor addressing both hints
+MIGRATION_MINOR_VERSION = 2 + + async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: """Migrate old entry.""" - - if config_entry.version == 1: - options = {**config_entry.options} - if config_entry.minor_version < 2: - # Remove the spook_inverse config entry from the source device - if source_device_id := async_get_source_entity_device_id( + if config_entry.version == 1 and config_entry.minor_version < MIGRATION_MINOR_VERSION: + options = {**config_entry.options} + # Remove the spook_inverse config entry from the source device + if source_device_id := async_get_source_entity_device_id( + hass, options[CONF_ENTITY_ID] + ): + async_remove_helper_config_entry_from_source_device( + hass, + helper_config_entry_id=config_entry.entry_id, + source_device_id=source_device_id, + ) + hass.config_entries.async_update_entry( + config_entry, options=options, minor_version=MIGRATION_MINOR_VERSION + ) + elif config_entry.version == 1: + # Already at latest minor version, just ensure options are preserved + hass.config_entries.async_update_entry( + config_entry, options={**config_entry.options}, minor_version=MIGRATION_MINOR_VERSION + ) + + return TrueNote: The simpler fix is just addressing the docstring blank line (pipeline failure). The constant extraction and if-statement flattening are truly optional given the localized nature of this migration code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@custom_components/spook/integrations/spook_inverse/__init__.py` around lines 77 - 91, Extract the magic number 2 into a named constant (e.g., TARGET_MINOR_VERSION = 2) and simplify the nested conditionals by combining checks on config_entry.version and config_entry.minor_version into a single if (use config_entry.version and config_entry.minor_version together), then perform the existing migration steps: get options from config_entry.options, call async_get_source_entity_device_id(hass, options[CONF_ENTITY_ID]) and, if it returns a source_device_id, call async_remove_helper_config_entry_from_source_device(...) with helper_config_entry_id=config_entry.entry_id, and finally call hass.config_entries.async_update_entry(config_entry, options=options, minor_version=TARGET_MINOR_VERSION); reference symbols: config_entry.version, config_entry.minor_version, options, async_get_source_entity_device_id, async_remove_helper_config_entry_from_source_device, and hass.config_entries.async_update_entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@custom_components/spook/integrations/spook_inverse/__init__.py`:
- Around line 74-76: The function async_migrate_entry currently has a blank line
immediately after its docstring which triggers D202; remove the empty line so
the docstring is directly followed by the next statement or return in
async_migrate_entry to satisfy the Ruff docstring rule; ensure no other blank
lines exist between the triple-quoted docstring and following code in that
function.
---
Nitpick comments:
In `@custom_components/spook/integrations/spook_inverse/__init__.py`:
- Around line 77-91: Extract the magic number 2 into a named constant (e.g.,
TARGET_MINOR_VERSION = 2) and simplify the nested conditionals by combining
checks on config_entry.version and config_entry.minor_version into a single if
(use config_entry.version and config_entry.minor_version together), then perform
the existing migration steps: get options from config_entry.options, call
async_get_source_entity_device_id(hass, options[CONF_ENTITY_ID]) and, if it
returns a source_device_id, call
async_remove_helper_config_entry_from_source_device(...) with
helper_config_entry_id=config_entry.entry_id, and finally call
hass.config_entries.async_update_entry(config_entry, options=options,
minor_version=TARGET_MINOR_VERSION); reference symbols: config_entry.version,
config_entry.minor_version, options, async_get_source_entity_device_id,
async_remove_helper_config_entry_from_source_device, and
hass.config_entries.async_update_entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c5b07e00-e6ae-41c3-96f7-b74f0875fd72
📒 Files selected for processing (6)
custom_components/spook/integrations/spook_inverse/__init__.pycustom_components/spook/integrations/spook_inverse/binary_sensor.pycustom_components/spook/integrations/spook_inverse/config_flow.pycustom_components/spook/integrations/spook_inverse/entity.pycustom_components/spook/integrations/spook_inverse/switch.pyhacs.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@custom_components/spook/integrations/spook_inverse/__init__.py`:
- Around line 74-95: Run the Ruff formatter to fix failing style checks: format
the file containing the async_migrate_entry function (the migration logic using
async_get_source_entity_device_id and
async_remove_helper_config_entry_from_source_device) with ruff format, then
re-run ruff --check; this will normalize spacing/line breaks and ensure the
async_migrate_entry block and its calls (hass.config_entries.async_update_entry,
async_remove_helper_config_entry_from_source_device) meet the project's
formatter rules so CI passes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b62955b3-b7f9-41a9-949f-e1a5800332bc
📒 Files selected for processing (1)
custom_components/spook/integrations/spook_inverse/__init__.py
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
custom_components/spook/integrations/spook_inverse/__init__.py (1)
77-96: Consider avoiding redundant entry updates when already at target version.The SonarCloud warning about missing async features is a false positive—Home Assistant's framework expects
async_migrate_entryto be async even when the implementation doesn't await.However,
async_update_entryis called unconditionally for version 1 entries, even whenminor_versionalready equalsMIGRATION_MINOR_VERSION. This causes unnecessary config entry writes on each load for already-migrated entries.♻️ Proposed fix
async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: """Migrate old entry.""" - if config_entry.version == 1: + if config_entry.version == 1 and config_entry.minor_version < MIGRATION_MINOR_VERSION: options = {**config_entry.options} - if config_entry.minor_version < MIGRATION_MINOR_VERSION and ( - source_device_id := async_get_source_entity_device_id( - hass, options[CONF_ENTITY_ID] - ) - ): + if source_device_id := async_get_source_entity_device_id( + hass, options[CONF_ENTITY_ID] + ): # Remove the spook_inverse config entry from the source device async_remove_helper_config_entry_from_source_device( hass, helper_config_entry_id=config_entry.entry_id, source_device_id=source_device_id, ) hass.config_entries.async_update_entry( config_entry, options=options, minor_version=MIGRATION_MINOR_VERSION ) return True🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@custom_components/spook/integrations/spook_inverse/__init__.py` around lines 77 - 96, The migration currently always calls hass.config_entries.async_update_entry for entries with config_entry.version == 1, causing redundant writes even when config_entry.minor_version is already MIGRATION_MINOR_VERSION; modify async_migrate_entry so you only call hass.config_entries.async_update_entry when config_entry.minor_version < MIGRATION_MINOR_VERSION (i.e., after you may have removed the helper via async_remove_helper_config_entry_from_source_device using async_get_source_entity_device_id), otherwise return True without updating the entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@custom_components/spook/integrations/spook_inverse/__init__.py`:
- Around line 77-96: The migration currently always calls
hass.config_entries.async_update_entry for entries with config_entry.version ==
1, causing redundant writes even when config_entry.minor_version is already
MIGRATION_MINOR_VERSION; modify async_migrate_entry so you only call
hass.config_entries.async_update_entry when config_entry.minor_version <
MIGRATION_MINOR_VERSION (i.e., after you may have removed the helper via
async_remove_helper_config_entry_from_source_device using
async_get_source_entity_device_id), otherwise return True without updating the
entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1f1903a2-2335-4552-a72f-8eec9b9cba08
📒 Files selected for processing (1)
custom_components/spook/integrations/spook_inverse/__init__.py
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
custom_components/spook/integrations/spook_inverse/__init__.py (1)
77-97: Consider removing unnecessary options copy.The
optionsdictionary on line 83 is copied but never modified before being passed toasync_update_entry. You can passconfig_entry.optionsdirectly.♻️ Proposed simplification
async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: """Migrate old entry.""" if ( config_entry.version == 1 and config_entry.minor_version < MIGRATION_MINOR_VERSION ): - options = {**config_entry.options} if source_device_id := async_get_source_entity_device_id( - hass, options[CONF_ENTITY_ID] + hass, config_entry.options[CONF_ENTITY_ID] ): # Remove the spook_inverse config entry from the source device async_remove_helper_config_entry_from_source_device( hass, helper_config_entry_id=config_entry.entry_id, source_device_id=source_device_id, ) hass.config_entries.async_update_entry( - config_entry, options=options, minor_version=MIGRATION_MINOR_VERSION + config_entry, minor_version=MIGRATION_MINOR_VERSION ) return TrueNote on SonarCloud warning: The
asynckeyword is required here because Home Assistant's integration framework expectsasync_migrate_entryto have an async signature, even when the implementation doesn't need to await anything. This is a false positive from SonarCloud.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@custom_components/spook/integrations/spook_inverse/__init__.py` around lines 77 - 97, The options dict is copied into a new variable but never modified; inside async_migrate_entry replace the unnecessary shallow copy (options = {**config_entry.options}) by using config_entry.options directly when calling hass.config_entries.async_update_entry, keeping the same conditional logic around async_get_source_entity_device_id and async_remove_helper_config_entry_from_source_device and preserving the minor_version update via hass.config_entries.async_update_entry(config_entry, options=config_entry.options, minor_version=...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@custom_components/spook/integrations/spook_inverse/__init__.py`:
- Around line 77-97: The options dict is copied into a new variable but never
modified; inside async_migrate_entry replace the unnecessary shallow copy
(options = {**config_entry.options}) by using config_entry.options directly when
calling hass.config_entries.async_update_entry, keeping the same conditional
logic around async_get_source_entity_device_id and
async_remove_helper_config_entry_from_source_device and preserving the
minor_version update via hass.config_entries.async_update_entry(config_entry,
options=config_entry.options, minor_version=...).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 663274e2-91ab-4084-8ee6-6634de7cff7c
📒 Files selected for processing (1)
custom_components/spook/integrations/spook_inverse/__init__.py
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
custom_components/spook/integrations/spook_inverse/__init__.py (1)
79-80: UseMIGRATION_MINOR_VERSIONinstead of hardcoded2.Line 80 hardcodes the threshold, which can drift from Line 17 over time. Prefer comparing against the exported constant directly.
Proposed refactor
- if ( - config_entry.version == 1 and config_entry.minor_version < 2 # noqa: PLR2004 - ): + if ( + config_entry.version == 1 + and config_entry.minor_version < MIGRATION_MINOR_VERSION + ):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@custom_components/spook/integrations/spook_inverse/__init__.py` around lines 79 - 80, Replace the hardcoded threshold in the migration check with the exported constant: change the conditional that currently reads "if (config_entry.version == 1 and config_entry.minor_version < 2)" to compare against MIGRATION_MINOR_VERSION instead (use config_entry.minor_version < MIGRATION_MINOR_VERSION); update the import/namespace reference if needed so MIGRATION_MINOR_VERSION (the constant defined earlier) is used within the block that handles config_entry migration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@custom_components/spook/integrations/spook_inverse/__init__.py`:
- Around line 83-85: Guard against missing CONF_ENTITY_ID and unresolved entity
IDs by fetching the entity id via options.get(CONF_ENTITY_ID) into a local
variable (e.g., entity_id = options.get(CONF_ENTITY_ID)) before calling
async_get_source_entity_device_id, and only call
async_get_source_entity_device_id(hass, entity_id) if entity_id is truthy; if
async_get_source_entity_device_id returns None, log a warning (or debug)
indicating the entity could not be resolved and skip device cleanup instead of
silently doing nothing. Ensure you reference options, CONF_ENTITY_ID, and
async_get_source_entity_device_id in the change so the behavior is explicit and
safe during migration.
---
Nitpick comments:
In `@custom_components/spook/integrations/spook_inverse/__init__.py`:
- Around line 79-80: Replace the hardcoded threshold in the migration check with
the exported constant: change the conditional that currently reads "if
(config_entry.version == 1 and config_entry.minor_version < 2)" to compare
against MIGRATION_MINOR_VERSION instead (use config_entry.minor_version <
MIGRATION_MINOR_VERSION); update the import/namespace reference if needed so
MIGRATION_MINOR_VERSION (the constant defined earlier) is used within the block
that handles config_entry migration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0c505abe-e3e8-4645-8344-4a76467b0615
📒 Files selected for processing (1)
custom_components/spook/integrations/spook_inverse/__init__.py
frenckatron
added
the
bugfix
…nsor and switch setup
frenckatron
force-pushed
the
fix-inverse-helper
branch
from
f8db285 to
1b99383
Compare
Collaborator
|
I rebased this onto current |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1227 +/- ##
==========================================
+ Coverage 56.96% 60.29% +3.33%
==========================================
Files 119 119
Lines 3016 3035 +19
Branches 386 390 +4
==========================================
+ Hits 1718 1830 +112
+ Misses 1262 1166 -96
- Partials 36 39 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
frenck
requested changes
May 28, 2026
frenck
requested changes
May 28, 2026
|
frenck
approved these changes
May 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The Inverse helper uses the old method of attaching itself to the device, which will stop working in 2026.8.
Due to the helper method for migration being introduced in 2025.8 I've bumped the HACS min version to that.
Motivation and Context
There is an updated pattern to correctly do this
https://developers.home-assistant.io/blog/2025/07/18/updated-pattern-for-helpers-linking-to-devices/
I've done quite a few of these on other helper, just tidying up any I see as there's no deprecation warning.
How has this been tested?
Using HA 2026.3.4 I created an inverse switch helper against an existing switch.
I replaced the files for the inverse sub integration and checked the inverse helper config entry was migrated to 1.2.
Checked the inverse switch was still attached to the device.
Created a new inverse helper to check that future switches also worked correctly.
Screenshots (if appropriate):
Types of changes
Checklist