Refactor: Pipeline Keymaster refreshes through dirty lock scopes

[tykeal]

Summary

Issue #670 exposed an event-loop storm in large Keymaster deployments: one
global coordinator fanned every refresh and mutation out to roughly
12k-49k entities, tripping Home Assistant 2026.7's
_MAX_QUEUED_EVENT_DISPATCHES = 10_000 guard.

This PR completes the issue #682 refresh pipeline refactor by routing refresh
completion and mutation notifications through dirty per-lock scopes. It builds
on merged #680, which introduced per-lock coordinators, and #681, which rebound
entities to those coordinators.

Details

• Detects changed lock data from sanitized before/after snapshots and notifies
  only the per-lock coordinators whose entry data changed.
• Splits _async_update_data() into async_refresh_all_locks() and
  async_refresh_lock(), returning dirty config entry IDs from refreshes.
• Removes the manager's global listener notification leg and collapses the two
  pending notification handles into one scoped handle.
• Scopes all mutation call sites to the affected config entry IDs.
• Keeps refreshes sequential and keeps coordinator.data as a dict mirror of
  kmlocks.

Two correctness paths are deliberately not scoped:

• Refresh health transitions still fan out to all lock coordinators because
  availability is global, not per-lock data.
• Locally-mutated entries are marked externally dirty so a later debounced
  refresh cannot overwrite a local entity-setter change.

Validation

• ruff check custom_components/ tests/
• ruff format custom_components/ tests/
• mypy custom_components/keymaster/
• pytest tests/ — 1029 tests passing

Closes #682

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.96%. Comparing base (cdb4922) to head (5c94c52).
⚠️ Report is 206 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #695      +/-   ##
==========================================
+ Coverage   84.14%   93.96%   +9.82%     
==========================================
  Files          10       42      +32     
  Lines         801     5403    +4602     
  Branches        0       30      +30     
==========================================
+ Hits          674     5077    +4403     
- Misses        127      326     +199     

Flag	Coverage Δ
python	93.86% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[tykeal]

CRITICAL 1: the premise does not hold. _push_lock_coordinator_update() is only called for the already-scoped target set: custom_components/keymaster/coordinator.py:390-393:

targets = list(self._lock_coordinators) if all_entry_ids else list(entry_ids)
for entry_id in targets:
    self._push_lock_coordinator_update(

That target set is built from async_schedule_keymaster_notifications() filtering/unioning the dirty IDs at custom_components/keymaster/coordinator.py:356-360:

valid = {entry_id for entry_id in entry_ids if entry_id in self.kmlocks}
self._pending_notify_entry_ids |= valid
self._pending_notify_all_entry_ids |= all_entry_ids

The suggested identity check would suppress real notifications because locks are mutated in place, e.g. custom_components/keymaster/coordinator.py:1613:

kmlock.lock_state = lock_state

CRITICAL 2: the premise does not hold. The raw arm intentionally bypasses the keymaster override at tests/test_coordinator_fanout_guard.py:382-387:

dispatching_snapshots.append(hass.bus._dispatching)
try:
    DataUpdateCoordinator.async_update_listeners(coordinator)

That arm asserts the old nested fan-out path trips the guard at tests/test_coordinator_fanout_guard.py:454-457. The deferred arm separately schedules [entries[0].entry_id], asserts fan-out happens after _dispatching is false, and proves entries[1] stays clean at tests/test_coordinator_fanout_guard.py:490-498.

CRITICAL 3: valid. Fixed in 469e3ef2e3e2a16cafd9f6b0d2c2d8c56f9187fc by tracking active refreshes, recording dirty IDs additively, and consuming the batch once notification work is scheduled while preserving the unknown/all-lock fallback (custom_components/keymaster/coordinator.py:282-298, 404-424, 2340-2343). Added regressions in tests/test_coordinator_lifecycle.py:512-582 for overlapping refresh union and unknown-sentinel all-lock fallback.

WARNING 1: verified the removed fields are gone as expected; no action needed.

WARNING 2: verified _async_save_data(kmlocks=None) still falls back to self.kmlocks, and an empty mapping iterates zero times (custom_components/keymaster/coordinator.py:549-558).

WARNING 3: verified _build_coordinator_tree() creates entries[0] as the parent and children from entries[1:] (tests/test_coordinator_fanout_guard.py:175-205); the deferred test registers fan-out listeners on entries[0] and the clean listener on entries[1] (tests/test_coordinator_fanout_guard.py:473-486).

[tykeal]

Suppressed comment A (entity.py:76) is incorrect. Home Assistant dev homeassistant/helpers/update_coordinator.py has BaseCoordinatorEntity.async_added_to_hass() only register the listener:

async def async_added_to_hass(self) -> None:
    await super().async_added_to_hass()
    self.async_on_remove(
        self.coordinator.async_add_listener(
            self._handle_coordinator_update, self.coordinator_context
        )
    )

It does not call _handle_coordinator_update(). Latest CI resolved homeassistant==2026.8.0b5; the local 2026.2.3 source has the same behavior. test_multi_entry_startup_entities_initialize_available covers the production regression: without Keymaster's initial coordinator update, entities constructed unavailable can stay unavailable.

Suppressed comment B (coordinator.py:289) was valid. Commit 97f1ad350956c712bdf1db305add12c5a2cfe97d moves shutdown cleanup into finally: timer cancellation, notification cancellation, lock-coordinator clearing, base coordinator shutdown, and _shutdown_complete now run even if pending-save flush fails. Non-cancellation flush failures are logged and pending save IDs remain queued; CancelledError propagates after cleanup. Covered by test_shutdown_cleanup_runs_when_pending_save_flush_fails and test_shutdown_cleanup_runs_before_flush_cancellation_propagates.

[tykeal]

Follow-up on shutdown re-entrancy: the window was reachable. test_shutdown_unregisters_stop_listener_before_flush_await failed against 97f1ad35 with Expected mock to have been awaited once. Awaited 2 times. after firing EVENT_HOMEASSISTANT_STOP while async_shutdown() was suspended in the pending-save flush. Commit eed0c82e82d35b40ddebf59c8592987979f07028 restores the pre-await stop-listener unsubscribe while keeping the cleanup-in-finally behavior.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

[tykeal]

Suppressed review follow-up:

1. lock.py:257 was valid. Legacy stored locks can omit child_config_entry_ids; _dict_to_kmlocks() then passes None, overriding the dataclass default, and reload inheritance previously raised TypeError: 'NoneType' object is not iterable. Commit 5c94c526e34fa8c61acd1a0bf44cfb4098f18dee copies list(old.child_config_entry_ids or []) and adds test_reload_inherits_missing_child_config_entry_ids_as_empty_list.

2. coordinator.py:2416 is invalid. HA dev DataUpdateCoordinator._async_refresh() success branch is:

else:
    if not self.last_update_success:
        self.last_update_success = True
        self.logger.info("Fetching %s data recovered", self.name)

HA does not clear last_exception on success; Keymaster mirrors upstream.

3/4. coordinator.py:2506/2513 are valid but pre-existing/out of scope. The all-lock refresh loops already iterate self.kmlocks across awaits on the base branch. Tracked separately in #705.

5. coordinator.py:665: dropping SETUP_IN_PROGRESS alone would strand the final setup flush because the caller is still in progress in its own finally. Empirically, the concern was real under concurrent setup: before the follow-up, a 40-lock concurrent in-progress run flushed/saved 40 times and measured 81.4 ms. Commit 5c94c526e34fa8c61acd1a0bf44cfb4098f18dee now tracks completed setup entry IDs explicitly; the same run flushes/saves once. Standard restart benchmark remains linear: 5/10/20/40 = 9.7 / 14.0 / 24.0 / 41.5 ms, with 1 full refresh, N scoped refreshes, and 2N lock updates.

[tykeal]

@firstof9 my agent has made more changes based upon real-world testing. I'm going to deploy this again soon to a production system, I just have to wait for the right window.

[firstof9]

Real-world testing is great. Rock on man!

[tykeal]

Ok, just deployed to the system that caused me to pull this into draft state. (14 locks. 1 parent, 13 children) much better now! Ready for any re-review you may have

[tykeal]

@firstof9 one thing to note, my agent did flag this refactor and some of the open PRs you've got as being in partial conflict. I'm leaning towards getting this merged first given the work that it is doing but if you want me to re-review your recent changes on #700 #701 and #702 and get those merged first and then do the extra work here let me know

[firstof9]

Nah go ahead and get your big fixes in and we can go back over my 3 part fix for that user's issue after.

[secondof9]

Code Review: FutureTense/keymaster PR #695

Summary

This PR completes the Issue #682 refresh pipeline refactor by routing refresh completion and mutation notifications through per-lock dirty scopes. It builds on merged PRs #680 and #681, which introduced per-lock coordinators and rebound entities to them.

Critical Findings

1. preserve_failed_refresh State Clobber in _schedule_refresh_global_notification and _flush_pending_keymaster_notifications

File: custom_components/keymaster/coordinator.py
Lines: 294-304, 417-437

The preserve_failed_refresh flag is a boolean that indicates whether a failed refresh should be preserved so that entity listeners see the failure state. However, there's a subtle bug where this state can be overwritten by independent notification paths:

• In _schedule_refresh_global_notification (line 294-304), when a global notification is scheduled, preserve_failed_refresh is set to False unconditionally (line 303), even if a per-lock notification path had previously set it to True. This means that if a per-lock refresh fails (setting preserve_failed_refresh = True), a subsequent global schedule would overwrite this state, causing the failed-refresh state to be lost.

• Similarly, in _flush_pending_keymaster_notifications (line 417-437), the same issue applies. A per-lock path could set preserve_failed_refresh = True, but the global path's _flush_global_leg would reset it.

Suggestion:

@callback
def _schedule_refresh_global_notification(self) -> None:
    """Schedule a deferred refresh-completion listener notification."""
    if self._deferred_notifications_shutting_down:
        return

    self._pending_global_notification = True
    if not getattr(self, "last_update_success", True):
        self._pending_global_failed_refresh = True
    else:
        self._pending_global_failed_refresh = False
    # CRITICAL: Do NOT overwrite preserve_failed_refresh if it was
    # already set by a per-lock path. Only set it if we haven't
    # already.
    if not self._pending_global_failed_refresh:
        self._pending_global_failed_refresh = False
    self._schedule_pending_global_notification()

2. _async_update_data Dead Code in KeymasterLockCoordinator

File: custom_components/keymaster/coordinator.py
Lines: 122-124

The KeymasterLockCoordinator._async_update_data method is now dead code in the normal path. It's only called if someone explicitly calls async_refresh() on the per-lock coordinator, which doesn't happen in the normal flow. The actual refresh logic lives in the manager's _async_refresh, which defers listener fan-out. The per-lock coordinator receives updates via _push_lock_coordinator_update, not via its own _async_update_data.

Suggestion: Consider adding a deprecation warning or removing the method to avoid confusion.

3. Defensive except Exception in _flush_pending_save_after_setup

File: custom_components/keymaster/__init__.py
Lines: 76-93

The _flush_pending_save_after_setup function catches a broad except Exception to log and attempt a flush even on setup failure. While this is defensive, it introduces a slight risk: a generic Exception swallow inside the flush could mask a real bug if the flush itself fails. However, it's intentional — the goal is to surface the flush failure for debugging without breaking the setup path.

Suggestion: Consider logging the specific exception type instead of a generic Exception:

async def _flush_pending_save_after_setup(
    coordinator: KeymasterCoordinator,
    entry_id: str,
    *,
    setup_failed: bool,
) -> None:
    """Flush deferred setup save work without masking setup failures."""
    if not setup_failed:
        await coordinator.async_flush_pending_save_data_if_setup_complete(entry_id)
        return

    try:
        await coordinator.async_flush_pending_save_data_if_setup_complete(entry_id)
    except Exception as e:
        _LOGGER.exception("Failed to flush pending keymaster save data after setup error: %s", e)

Warnings

4. _get_schema filter_func=is_platform_supported Runtime Check

File: custom_components/keymaster/config_flow.py
Lines: 284-289

The _get_schema function applies filter_func=is_platform_supported to lock entity selection. This is a runtime check, not a data validation issue. However, it's worth noting that this filter is applied during the config flow, which could be confusing if the filter changes between flow steps.

Suggestion: Consider caching the filtered entities or making the filter more explicit in the UI.

5. _available_parent_locks Function Scope

File: custom_components/keymaster/config_flow.py
Lines: 179-194

The _available_parent_locks function filters out entries that already have a parent. This is correct, but it could be more efficient by using a set lookup instead of a linear scan.

Suggestion: Pre-compute a set of parented entries for repeated calls.

6. _get_locks_in_use Function Scope

File: custom_components/keymaster/config_flow.py
Lines: 246-255

The _get_locks_in_use function is called during the config flow to exclude locks that are already in use. This is correct, but it could be more efficient by using a set lookup instead of a linear scan.

Suggestion: Pre-compute a set of in-use locks for repeated calls.

Positive Findings

7. async_setup_entry Entry Is In coordinator.kmlocks Check

File: custom_components/keymaster/__init__.py
Lines: 223-227

The needs_update flag is set based on whether the entry is in coordinator.kmlocks. This is correct for the new path where add_lock is called once.

8. async_remove_entry Properly Cleans Up Coordinator and Lock Coordinator

File: custom_components/keymaster/__init__.py
Lines: 337-341

The removal path properly cleans up the coordinator and lock coordinator.

9. _get_schema _get_default Helper Correctly Falls Back Through Multiple Levels

File: custom_components/keymaster/config_flow.py
Lines: 274-276

The _get_default helper correctly falls back through user_input → default_dict → fallback_default. This is a defensive pattern that prevents KeyError.

10. _get_schema _available_parent_locks Function Filters Out Entries That Already Have a Parent

File: custom_components/keymaster/config_flow.py
Lines: 179-194

The _available_parent_locks function filters out entries that already have a parent. This is correct.

Conclusion

This PR is a solid refactor that addresses the Issue #682 event-loop storm in large Keymaster deployments. The core changes (dirty per-lock scopes, scoped notification paths) are well-implemented and address the original problem.

The two critical findings (points 1 and 2) are worth addressing:

1. The preserve_failed_refresh state clobber issue could cause failed-refresh states to be lost in certain edge cases.
2. The _async_update_data dead code in KeymasterLockCoordinator is harmless but could be cleaned up.

The warnings are mostly about potential inefficiencies in the config flow, which are not blocking.

Overall, this PR is ready for merge with the critical findings addressed.