fix(events): re-emit member changes as group-addressed events (pyisy-3.x parity)

[shbatm]

Closes #173

Problem

pyisyox.runtime.Group is pull-only — group_all_on / group_any_on computed on access, with no event-subscription plumbing. Unlike pyisy 3.x (whose Group subscribed to every member's status_events and re-published its own), nothing notifies subscribers when a member changes. Consumers subscribe to the group's own address — which the controller never emits — so scene/group switches freeze at load-time value. Regression from pyisy 3.x, not a design choice.

Fix

EventDispatcher takes the group registry, builds a member → (group, …) reverse index, and after a member property update fans out a synthetic ST event addressed to each containing group. Notification only — not fed back through feed() (no re-parse / _apply_property_update; groups aren't in the node registry) and cannot recurse. Per-address subscribers re-render and re-read the computed-on-access group_any_on. No groups → legacy behaviour. Controller wires groups=self._loaded.groups. Consumer-transparent: hacs-udi-iox's existing (group_addr, None) wildcard listener starts firing with zero changes.

Tests

tests/test_runtime/test_events.py — re-emit on member change; node in multiple scenes → one per scene; non-member → none; no-groups legacy no-op; system events never re-emit. Full suite: 844 passed · mypy clean · pylint 10/10 · ruff clean.

🤖 Generated with Claude Code

[github-actions]

Claude finished @shbatm's task in 2m 42s —— View job

---

Code Review

• Gather context and read changed files
• Review pyisyox/runtime/events.py
• Review pyisyox/controller.py
• Review tests/test_runtime/test_events.py
• Post final review

---

The design is clean and the core logic is correct. One medium-severity bug to fix before merging, plus a few smaller observations.

---

Bug: stale _group_members_index after Controller.refresh()

Controller.refresh() replaces the groups dict in place:

# controller.py:632
loaded.groups = fresh.groups   # new dict object

But the dispatcher's reverse index was built once in __init__ from the original dict reference and is never rebuilt. After a _3 lifecycle event (scene membership changed → requires_reload=True) the consumer calls refresh(), which delivers a fresh LoadResult.groups, but the dispatcher keeps fanning out on the old membership. New scene members won't trigger group events; removed members still will.

Contrast with nodes — that's handled correctly via in-place mutation:

loaded.nodes.clear()
loaded.nodes.update(fresh.nodes)

The comment in EventDispatcher.__init__ says "Built once; group membership only changes on a lifecycle event, after which the consumer reloads anyway" — but "reload" here goes through refresh(), not connect() (which would create a new dispatcher). The comment and the wiring are inconsistent.

Suggested fix (minimal diff):

Add a method to EventDispatcher:

def update_groups(self, groups: dict[str, GroupRecord]) -> None:
    """Rebuild the member→groups reverse index from a fresh registry.

    Call after Controller.refresh() so membership changes from lifecycle
    events are reflected in the re-emit fan-out.
    """
    self._groups = groups
    members_index: dict[str, list[str]] = {}
    for group_address, group_record in self._groups.items():
        for member_address in group_record.member_addresses:
            members_index.setdefault(member_address, []).append(group_address)
    self._group_members_index = {
        member: tuple(addrs) for member, addrs in members_index.items()
    }

And call it from refresh() right after loaded.groups = fresh.groups:

loaded.groups = fresh.groups
if self._dispatcher is not None:
    self._dispatcher.update_groups(fresh.groups)

Fix this →

---

_groups slot is write-once dead state

self._groups is assigned in __init__ but never read after that (only _group_members_index is used in _reemit_group_status). If you add update_groups() above, _groups becomes useful there. Without it, it's unused and the slot can be dropped. Either way, worth resolving.

---

Synthetic group event carries the member's action value under control="ST"

_reemit_group_status constructs:

group_event = Event(
    control=PROP_STATUS,   # "ST"
    action=event.action,   # e.g. "42" from a GV1 change
    node_address=group_address,
    ...
)

If the triggering member event was GV1=42, the synthetic group event says ST=42 — which is misleading if a downstream listener reads event.action and treats it as the group's actual status. The docstring ("the carried action/uom echo the member only as provenance") is clear, but the wire shape is subtly surprising. For the current consumer (hacs-udi-iox re-reads group_any_on on receipt and doesn't use action directly), this is harmless. Worth a brief note in the docstring that action is the triggering member's value, not a group status value, to prevent future consumers from misusing it.

---

Missing test: non-ST property change on a member

All five new tests use control="ST" for the triggering member event. Since is_node_property is true for any non-system control with a node address (GV1, OL, RR, etc.), non-ST changes also fan out to group listeners — but this isn't tested. Adding one case (e.g., GV1 on a member) would document the intentional behaviour.

---

Cosmetic: test string reformatting

The test file collapses several multi-line XML strings to single lines (e.g., lines 645–648, 769–772, 842, etc.). This is a ruff-format change unrelated to the feature. Fine to include, but it makes the diff wider than necessary.

---

Everything else looks good

• Reverse-index built once at construction — O(1) per fan-out during the event loop. ✓
• tuple(self._listeners) snapshot in _reemit_group_status is consistent with the existing pattern in feed(). ✓
• Re-emit is gated on event.is_node_property (already checked in feed() before calling _reemit_group_status), so system events, heartbeats, and lifecycle frames never fan out. ✓
• _reemit_group_status does NOT call feed() recursively — no re-parse, no double property update, no infinite recursion. ✓
• controller.py change is correct — passes the same LoadResult.groups dict the rest of the controller uses. ✓
• Test coverage for happy-path, multi-scene, non-member, legacy-no-groups, and system-event cases is solid. ✓

[shbatm]

Thanks for the review — addressed in 4c601cf.

Bug: stale _group_members_index after refresh() — Fixed. Added EventDispatcher.update_groups(groups) which rebuilds _groups + _group_members_index; __init__ now delegates to it (DRY, no duplicated index-build). Controller.refresh() calls self._dispatcher.update_groups(fresh.groups) right after loaded.groups = fresh.groups, with a comment explaining the replace-vs-mutate asymmetry vs nodes. Regression test added: after a refresh-style replace with swapped membership, the new member re-emits and the removed one no longer does.

_groups write-once dead state — Resolved by the above; _groups is now read/rewritten by update_groups(). Slot kept.

Synthetic event carried member action under control="ST" — Changed to a pure notification: action="", uom/formatted_* left default. A group has no single status value, so fabricating ST=<member value> was misleading; control stays "ST" only for routing on the group's normal per-address channel, seqnum/timestamp kept for ordering/provenance. Docstring updated to say so explicitly, and a test now asserts the neutral action/uom.

Missing non-ST test — Added test_dispatcher_reemits_for_non_st_member_property (a GV1 member change still pokes the group), documenting that re-emit is intentionally gated on is_node_property, not ST specifically.

Cosmetic test reformatting — These collapses are ruff format output and the pre-commit ruff-format hook enforces them on every commit touching the file, so reverting would just bounce on the next commit. Left as-is for hook consistency.

846 passed · mypy clean · pylint 10/10 · ruff clean.

[github-actions]

Claude finished @shbatm's task in 1m 58s —— View job

---

Code Review (Follow-up — 4c601cf)

• Gather context from PR and comments
• Read changed files from the updated commits
• Review pyisyox/runtime/events.py changes
• Review pyisyox/controller.py changes
• Review tests/test_runtime/test_events.py changes
• Post final review

---

All four issues from the first review pass cleanly. One very minor residual observation, then an overall verdict.

---

Bug fix: update_groups() — correct and DRY

__init__ now delegates to update_groups() rather than duplicating the index-build:

self._groups: dict[str, GroupRecord] = {}
self._group_members_index: dict[str, tuple[str, ...]] = {}
self.update_groups(groups if groups is not None else {})

update_groups() rebuilds both _groups and _group_members_index atomically, and Controller.refresh() calls it right after loaded.groups = fresh.groups with a clear comment explaining why. The guard if self._dispatcher is not None is correct — refresh() is documented as a post-connect operation but the guard is defensive and harmless.

---

Neutral re-emit action — correct

group_event = Event(
    seqnum=event.seqnum,
    timestamp=event.timestamp,
    control=PROP_STATUS,
    action="",          # neutral: no fabricated group status value
    node_address=group_address,
)

seqnum/timestamp carried for ordering/provenance; action="" avoids the misleading ST=<member value> shape. The docstring accurately describes this. The test assertion on action == "" / uom == "" documents the contract.

---

Test coverage — solid

Seven new tests cover: happy-path ST re-emit, multi-scene fan-out, non-member no-op, legacy-no-groups no-op, system-event skip, non-ST property (GV1) re-emit, and refresh-style membership swap. The regression case (test_update_groups_rebuilds_index_after_refresh_style_replace) is exactly the right way to pin the original bug.

---

Minor: self._groups is still write-only in update_groups()

The method assigns self._groups = groups then iterates over groups (the local parameter), not self._groups:

def update_groups(self, groups: dict[str, GroupRecord]) -> None:
    self._groups = groups                              # stored…
    for group_address, group_record in groups.items(): # …but loop reads local param

After the method returns, self._groups is never referenced again in the class. The slot keeps the dict alive by reference (useful for debugging / future extension) but is otherwise dead state. Not a bug — just a residual from the first review's comment that the author resolved structurally without quite closing the read-path. Lowest severity, fine to leave as-is or drop the slot.

---

Everything else holds from the first review ✓

• Reverse-index is O(1) per fan-out during the event loop.
• tuple(self._listeners) snapshot in _reemit_group_status matches the existing pattern in feed().
• is_node_property gates re-emit correctly (empty node_address on system frames evaluates False).
• No recursion: synthetic group events go directly to listeners, not back through feed().
• controller.py wiring passes the same LoadResult.groups dict the rest of the controller uses.

Ready to merge. The one minor observation above (write-only self._groups) doesn't block.

[shbatm]

Residual nit fixed in 98001ba: dropped the write-only _groups slot/assignments — update_groups() builds _group_members_index directly from its argument; that index is the only state read at dispatch. 127 events/groups tests pass, mypy/pylint/ruff clean.

[github-actions]

Claude finished @shbatm's task in 49s —— View job

---

Code Review (Follow-up — 98001ba)

• Gather context from PR and comments
• Read changed files from the latest commit
• Review pyisyox/runtime/events.py changes
• Review pyisyox/controller.py changes
• Review tests/test_runtime/test_events.py changes
• Post final review

---

All issues from both prior rounds are fully resolved. This is a clean finish.

---

Residual nit — _groups write-only slot: fixed ✓

__slots__ now contains only _group_members_index; _groups is gone. update_groups() builds and atomically replaces _group_members_index directly from its argument without retaining the groups dict:

def update_groups(self, groups: dict[str, GroupRecord]) -> None:
    members_index: dict[str, list[str]] = {}
    for group_address, group_record in groups.items():
        for member_address in group_record.member_addresses:
            members_index.setdefault(member_address, []).append(group_address)
    self._group_members_index = {
        member: tuple(group_addresses) for member, group_addresses in members_index.items()
    }

No dead state, no unnecessary reference held. Exactly right.

---

All prior issues confirmed resolved

Issue	Status
Stale _group_members_index after refresh()	Fixed — update_groups() called in Controller.refresh()
_groups write-only dead state	Fixed — slot removed entirely
Synthetic event carries member action under ST	Fixed — action="", neutral notification
Missing non-ST property test	Fixed — test_dispatcher_reemits_for_non_st_member_property added

Seven new tests pin the contract cleanly. The refresh-regression test (test_update_groups_rebuilds_index_after_refresh_style_replace) is the right shape: it exercises the exact failure mode (old member fires, new one doesn't) rather than just checking index state.

Ready to merge.