Properly tear down group entities on membership changes and group removal

[TheJulianJES]

Group entity teardown was skipped in several places, leaking member entity subscriptions: member entities kept references to dead group entities and kept invoking their debounced updates.

What was broken

• discover_group_entities() cleared group_entities with a bare .clear() when a group dropped below 2 members, without calling on_remove() or unregister_group_entity() — so the Debouncer and every member subscription stayed alive.
• It also never removed a platform's group entity when that platform alone dropped below 2 eligible members (e.g. a mixed light+switch group losing one of its two switches). The stale entity stayed registered and functional, backed by a single member.
• Gateway.group_removed() popped the group without calling Group.on_remove(), unlike shutdown() and device_removed().
• Group.on_remove() only removed the group entities themselves, leaving the member entity subscriptions in _entity_unsubs dangling.

What this changes

• discover_group_entities() now tears stale group entities down properly — unregister_group_entity() plus a scheduled on_remove() — both when the whole group drops below 2 members and when a single platform loses eligibility. Coordinator members are already excluded from the per-platform counts, so a "coordinator + one light" group now correctly loses its group entity too.
• Gateway.group_removed() schedules Group.on_remove(), so removing a group tears down its entities even when the members were not removed first.
• Group.on_remove() releases any remaining member entity subscriptions, and unregisters a group entity even if that entity's own on_remove() raised — otherwise _group_entities and _entity_unsubs could desync and a later unregister_group_entity() would raise KeyError.
• unregister_group_entity() verifies the exact entity instance is registered, so a delayed removal cannot unregister a recreated group entity that has the same unique id.

Notes

• Removing a member device (rather than a member endpoint) still leaves a stale group entity behind: zigpy's _remove_device() does not remove the device's endpoints from groups, so no group_member_removed event fires and Group.members keeps its cached value. That is pre-existing and a different code path (Gateway.device_removed), so it is deliberately out of scope here.
• This is not a prerequisite for Create device registry entries for ZHA group entities home-assistant/core#162307 ("Create device registry entries for ZHA group entities"). Home Assistant rebuilds its group entity set from the GroupInfo.entities snapshot on every group event, and that snapshot is correct either way. The two are complementary: without this PR, a platform that drops below two eligible members keeps a stale group entity that Home Assistant faithfully re-adds.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.20%. Comparing base (f6273ae) to head (f450763).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #849      +/-   ##
==========================================
+ Coverage   97.19%   97.20%   +0.01%     
==========================================
  Files          57       57              
  Lines       10543    10558      +15     
==========================================
+ Hits        10247    10263      +16     
+ Misses        296      295       -1     

☔ 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.

[TheJulianJES]

@zigpy-review-bot Review and check if anything else is needed, or if this should be merged as is. Push any changes needed. Consider the linked HA Core PR home-assistant/core#162307 and this branch: home-assistant/core@dev...TheJulianJES:core:zigpy-bot/zha-group-devices.

Check if that is complete as well. Do review rounds on both this PR and the HA Core PR, but with Opus agent (not Fable) and also with Copilot: 5.6 Sol. You can also push more changes to the linked HA Core branch, but do not force-push that one. If needed, create another branch based off that one. You can force-push this ZHA PR if needed though (or create other issues or PRs for other things discovered along the way). Do verify everything with review rounds before pushing. You can also update this PR description (but again: verify before).

Also, is this PR required for the Core PR?

[zigpy-review-bot]

Reviewed both, pushed changes to both, and ran review rounds until clean — an Opus agent (claude-opus-5[1m]) and Copilot GPT-5.6 Sol on each side, two rounds for this PR and four for the HA Core branch.

Your three questions

Is this PR required for the Core PR? No. Home Assistant rebuilds its group entity set from the GroupInfo.entities snapshot on every group event, and that snapshot is correct with or without this PR. I checked it rather than argued it: HA Core's ZHA suite is green against both the manifest-pinned zha==2.1.0 and this branch. They are complementary — without this PR, a platform that drops below two eligible members keeps a stale group entity, and HA faithfully re-adds it onto the new group device.

Should this be merged as is? It needed three fixes first, which are now pushed. With those, both reviewers call it ready.

Is the HA Core branch complete? It wasn't — it had two hard breaks against current dev and a lifecycle gap. All fixed and pushed to zigpy-bot/zha-group-devices (fast-forward, no force-push).

#849 — rebased onto dev and force-pushed

The branch was 21 commits behind. What I added on top:

• A real bug in Group.on_remove(): if a group entity's own on_remove() raised, the entity stayed in _group_entities while the new while self._entity_unsubs: popitem() loop popped its unsubscribe callback anyway. The two mappings desynced and the next unregister_group_entity() raised KeyError on the unguarded .pop(). It now unregisters the entity in that case, and the pop tolerates a missing callback.
• Deterministic ordering: eligible_platforms was a set, so group entity creation order — and therefore GroupInfo.entities order — was hash-randomised per process. Now a list; confirmed stable across four PYTHONHASHSEED values.
• Coverage: three of the four production hunks passed with the code reverted. Notably Gateway.group_removed, because zigpy.group.Groups.pop() removes every member first, so the existing test reached the teardown through the member-removal path and never through the new code — its comment claimed the opposite. Four tests added, each verified to fail when its hunk is reverted.

CI is green and I updated the PR description.

Not fixed here, filed as #875 instead: removing a member device (rather than its group membership) still leaves a fully stale group entity — zigpy's _remove_device() doesn't remove the device's endpoints from groups, so no group_member_removed fires, Group.members keeps its cached value, and the group keeps a subscription to the removed device's entity. Different entry point (Gateway.device_removed), pre-existing, and a fix needs its own test surface.

The HA Core branch

It was 2551 commits behind dev, and that is where it broke. I merged current dev (conflicts in zha/entity.py and test_entity.py resolved by hand) and fixed what that exposed:

• Two hard breaks on current dev. device_registry.async_get_device and device_registry.deleted_devices now report_usage(..., core_integration_behavior=ERROR), which raises for a core integration. Both sat on live paths — the first is hit for every group at setup, the second on every group removal. DeviceInfo["via_device"] is also gone in favour of via_device_id.
• Dropped both registry-internals pokes. There is no public purge API, and this is exactly what the still-open Copilot thread on #162307 asks for. The trade-off is documented in a code comment: a new group reusing a freed zigpy group id can inherit the deleted group's name/area/labels. Both reviewers agreed a core-side async_purge_deleted_device should not block this.
• A group that shrank below two members left an orphaned device forever — created when entities appeared, never removed, and HA never reaps it since it's referenced by both a config entry and an entity entry. Users were left with a group device card holding one permanently unavailable entity. Now cleaned up, and regrowing the group recreates it.
• associated_entities was always empty and raised TypeError for a member with no entities. The new diagnostics endpoint exposed it.
• Device diagnostics for a stale group device was a 500.
• Identifier parsing is anchored to the config entry with a strict four-hex-digit id, so a legacy zha_group_0xNNNN unique id or another entry's group device can't be misread.

One thing worth flagging because I caused it: my first version of the shrink cleanup removed every entity registry entry on the group device, not just ZHA's. A "Switch as X" helper built on a group entity attaches to the group device, so adding a member to a group silently deleted the helper. Round 2 caught it, I reproduced it, and it's fixed — entries are matched on platform, config entry and the group entity unique id shape, with a regression test. Round 4 then showed my follow-up guard was itself wrong: HA detaches other integrations' entities when a device is removed rather than deleting them, so the guard only created a device nothing could reap. Removed.

For the PR description, when you update #162307

The upgrade effects, all verified by test: group entities move from the coordinator device to a new SERVICE group device; the friendly name changes from "<Coordinator> <Group>" to "<Group>"; existing users keep their coordinator-derived entity_id while new installs get the short one; user name/icon/area survive the move. That last point settles the thread with dmulcahey and piitaya from February — entity-id automations are unaffected, and device-scoped automations are the part that genuinely breaks.

One coordination point

The per-platform half of the new cleanup is inert against the pinned zha==2.1.0 — 2.1.0 only clears group_entities when the whole group drops below two members, so the mixed-group case (a light+switch group losing one switch) still leaves a dead switch.<group> there. It only works once #849 lands and the pin moves, so that half can't be regression-tested yet. The docstring says so; a pin bump is the follow-up.

Verification

• Properly tear down group entities on membership changes and group removal #849: 1380 passed, ruff clean, mypy clean, full CI green on the pushed head.
• HA Core branch: 368 passed / 4 skipped, ruff 0.16.3 clean, format clean, mypy clean — against the manifest-pinned zha==2.1.0.
• Cross-repo: HA Core's ZHA suite also run against the Properly tear down group entities on membership changes and group removal #849 branch. The one failure there (test_number_quirks_v2_metadata) reproduces against plain zha dev too, so it's dev-vs-2.1.0 drift, not Properly tear down group entities on membership changes and group removal #849.
• Round 4 had one disagreement: Copilot argued that removing the group device could delete a foreign-platform entity sharing ZHA's config entry. That state isn't reachable through the entity platform, and the group-removal path has removed the device unconditionally since before this work, so it isn't something these changes introduce.

Two things I deliberately left to you: opening this as a PR against #162307's branch, and the core-side purge API.