From dbc38b8b02770f46fec0a756ef4eaf09a4170b65 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sun, 13 Sep 2026 21:39:05 +0200 Subject: [PATCH 1/3] fix: resolve nested child states to per-instance proxies for is_active A compound state nested inside a parallel region was reported as inactive when reached through an attribute chain (e.g. `sm.door.shut`), even though it was present in the configuration. `sm.shut` (accessed directly) worked because it resolves to an `InstanceState` proxy, but `sm.door.shut` returned the raw class-level `State`, whose `is_active` is a stale attribute fixed to `False`. Root cause: `InstanceState.__init__` ran `_init_states()`, which set the raw child `State` objects directly on the proxy's `__dict__` (bypassing the delegating `__getattr__`) and mutated the shared raw children's `parent`. Now children are resolved lazily: `__getattr__` maps a declared child (or history) state to the machine's per-instance proxy via the new `Configuration.instance_state()`, so `is_active` is consistent regardless of access path. Fixes #651 Signed-off-by: Martin Bernstorff --- statemachine/configuration.py | 10 ++++++++++ statemachine/state.py | 14 +++++++++++++- tests/test_statechart_parallel.py | 26 ++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/statemachine/configuration.py b/statemachine/configuration.py index 0a293658..3b02ddfc 100644 --- a/statemachine/configuration.py +++ b/statemachine/configuration.py @@ -93,6 +93,16 @@ def states(self) -> "OrderedSet[State]": def states(self, new_configuration: "OrderedSet[State]"): self._write_to_model(OrderedSet(s.value for s in new_configuration)) + def instance_state(self, value: Any) -> "State | None": + """Return the per-instance proxy registered for *value*, if any. + + Used to resolve a nested child :class:`State` (reached through an + attribute chain like ``sm.door.shut``) to the same proxy the machine + exposes directly, so instance-scoped attributes (e.g. ``is_active``) + stay consistent regardless of access path. + """ + return self._instance_states.get(value) + # -- Incremental mutation (used by the engine) ----------------------------- def add(self, state: "State"): diff --git a/statemachine/state.py b/statemachine/state.py index 065cc52a..cb87d233 100644 --- a/statemachine/state.py +++ b/statemachine/state.py @@ -370,10 +370,22 @@ def __init__( self._state = state self._machine = ref(machine) self._hash = hash(state) - self._init_states() + # Children are resolved lazily by __getattr__ to their per-instance + # proxies. Running _init_states() here would instead cache the raw + # child States and mutate their shared ``parent`` to point at this + # proxy — so it is deliberately skipped. def __getattr__(self, name: str): value = getattr(self._state, name) + if isinstance(value, State) and ( + value in self._state.states or value in self._state.history + ): + # A nested child state resolves to its per-instance proxy, so + # instance-scoped attributes (e.g. is_active) stay consistent + # whether accessed directly or through an attribute chain. + machine = self._machine() + assert machine is not None + value = machine._config.instance_state(value.value) or value self.__dict__[name] = value return value diff --git a/tests/test_statechart_parallel.py b/tests/test_statechart_parallel.py index 297589e3..3a237e29 100644 --- a/tests/test_statechart_parallel.py +++ b/tests/test_statechart_parallel.py @@ -199,3 +199,29 @@ async def test_top_level_parallel_not_terminated_when_one_region_pending(self, s assert sm.is_terminated is False assert "closed" in sm.configuration_values assert "running" in sm.configuration_values + + +@pytest.mark.timeout(5) +class TestNestedIsActive: + async def test_is_active_on_nested_child_via_attribute_chain(self, sm_runner): + """``is_active`` is correct for a leaf reached through an attribute chain. + + A compound child nested inside a parallel region must report ``is_active`` + consistently whether accessed directly (``sm.shut``) or through its parent + proxy (``sm.door.shut``). Both resolve to the same per-instance proxy. + """ + + class Microwave(StateChart): + class microwave(State.Parallel): + my_state = State(initial=True) + + class door(State.Compound): + shut = State(initial=True) + + sm = await sm_runner.start(Microwave) + + assert sm.my_state.is_active + assert sm.door.is_active + assert "shut" in sm.configuration_values + assert sm.shut.is_active + assert sm.door.shut.is_active From 5d2285efaa5de03406eed20733fa2e7e01101837 Mon Sep 17 00:00:00 2001 From: Fernando Macedo Date: Sun, 11 Oct 2026 19:10:44 -0300 Subject: [PATCH 2/3] fix: resolve any State reached through a proxy, including parent Review follow-up for #652: - resolve every State-valued attribute of an InstanceState to its per-instance proxy, so `parent` no longer returns the raw State - hand the instance_states mapping to the proxy at construction instead of reaching it through `machine._config` - pin proxy identity and cross-instance isolation in tests - unify assertion argument order (Sonar S3415) Signed-off-by: Fernando Macedo --- statemachine/configuration.py | 10 ------ statemachine/state.py | 19 ++++------- statemachine/statemachine.py | 2 +- tests/test_statechart_parallel.py | 54 +++++++++++++++++++------------ 4 files changed, 41 insertions(+), 44 deletions(-) diff --git a/statemachine/configuration.py b/statemachine/configuration.py index 3b02ddfc..0a293658 100644 --- a/statemachine/configuration.py +++ b/statemachine/configuration.py @@ -93,16 +93,6 @@ def states(self) -> "OrderedSet[State]": def states(self, new_configuration: "OrderedSet[State]"): self._write_to_model(OrderedSet(s.value for s in new_configuration)) - def instance_state(self, value: Any) -> "State | None": - """Return the per-instance proxy registered for *value*, if any. - - Used to resolve a nested child :class:`State` (reached through an - attribute chain like ``sm.door.shut``) to the same proxy the machine - exposes directly, so instance-scoped attributes (e.g. ``is_active``) - stay consistent regardless of access path. - """ - return self._instance_states.get(value) - # -- Incremental mutation (used by the engine) ----------------------------- def add(self, state: "State"): diff --git a/statemachine/state.py b/statemachine/state.py index cb87d233..2b01a501 100644 --- a/statemachine/state.py +++ b/statemachine/state.py @@ -1,4 +1,5 @@ from collections.abc import Generator +from collections.abc import Mapping from enum import Enum from typing import TYPE_CHECKING from typing import Any @@ -366,26 +367,18 @@ def __init__( self, state: State, machine: "StateChart", + instance_states: "Mapping[Any, State]", ): self._state = state self._machine = ref(machine) + self._instance_states = instance_states self._hash = hash(state) - # Children are resolved lazily by __getattr__ to their per-instance - # proxies. Running _init_states() here would instead cache the raw - # child States and mutate their shared ``parent`` to point at this - # proxy — so it is deliberately skipped. + # No _init_states(): it would rebind the shared children's ``parent`` to this proxy. def __getattr__(self, name: str): value = getattr(self._state, name) - if isinstance(value, State) and ( - value in self._state.states or value in self._state.history - ): - # A nested child state resolves to its per-instance proxy, so - # instance-scoped attributes (e.g. is_active) stay consistent - # whether accessed directly or through an attribute chain. - machine = self._machine() - assert machine is not None - value = machine._config.instance_state(value.value) or value + if isinstance(value, State): + value = self._instance_states.get(value.value, value) self.__dict__[name] = value return value diff --git a/statemachine/statemachine.py b/statemachine/statemachine.py index 5277c4fc..dffe3f8e 100644 --- a/statemachine/statemachine.py +++ b/statemachine/statemachine.py @@ -207,7 +207,7 @@ def _build_configuration(self) -> Configuration: instance_states: dict[Any, Any] = {} events = self.__class__._events for state in self.states_map.values(): - ist = InstanceState(state, self) + ist = InstanceState(state, self, instance_states) instance_states[state.value] = ist if state.id not in events: vars(self)[state.id] = ist diff --git a/tests/test_statechart_parallel.py b/tests/test_statechart_parallel.py index 3a237e29..4fd97e78 100644 --- a/tests/test_statechart_parallel.py +++ b/tests/test_statechart_parallel.py @@ -64,7 +64,7 @@ async def test_exit_parallel_exits_all_regions(self, sm_runner): sm = await sm_runner.start(WarWithExit) assert "war" in sm.configuration_values await sm_runner.send(sm, "truce") - assert {"peace"} == set(sm.configuration_values) + assert set(sm.configuration_values) == {"peace"} async def test_event_in_one_region_no_effect_on_others(self, sm_runner): """Region isolation: events affect only the targeted region.""" @@ -106,7 +106,7 @@ async def test_parallel_done_when_all_regions_final(self, sm_runner): await sm_runner.send(sm, "flood") # Both regions are final -> done.state.battle fires - assert {"aftermath"} == set(sm.configuration_values) + assert set(sm.configuration_values) == {"aftermath"} async def test_parallel_not_done_when_one_region_final(self, sm_runner): """Parallel not done when only one region reaches final.""" @@ -142,7 +142,7 @@ async def test_top_level_parallel_done_state_fires_before_termination(self, sm_r await sm_runner.send(sm, "close_ui") await sm_runner.send(sm, "stop_backend") # done.state.session fires, transitions to finished, then terminates - assert {"finished"} == set(sm.configuration_values) + assert set(sm.configuration_values) == {"finished"} assert sm.is_terminated is True async def test_from_enum_inside_parallel(self, sm_runner): @@ -173,7 +173,7 @@ class region_b(State.Compound): finish = work.to(done) sm = await sm_runner.start(SC) - assert {"start"} == set(sm.configuration_values) + assert set(sm.configuration_values) == {"start"} await sm_runner.send(sm, "begin") vals = set(sm.configuration_values) @@ -188,9 +188,8 @@ class region_b(State.Compound): await sm_runner.send(sm, "go_b") # Both regions final -> done.state.work fires - assert {RegionA.ACTIVE, RegionB.ON} <= set(sm.configuration_values) or {"done"} == set( - sm.configuration_values - ) + vals = set(sm.configuration_values) + assert {RegionA.ACTIVE, RegionB.ON} <= vals or vals == {"done"} async def test_top_level_parallel_not_terminated_when_one_region_pending(self, sm_runner): """Machine keeps running when only one region reaches final.""" @@ -203,25 +202,40 @@ async def test_top_level_parallel_not_terminated_when_one_region_pending(self, s @pytest.mark.timeout(5) class TestNestedIsActive: - async def test_is_active_on_nested_child_via_attribute_chain(self, sm_runner): - """``is_active`` is correct for a leaf reached through an attribute chain. - - A compound child nested inside a parallel region must report ``is_active`` - consistently whether accessed directly (``sm.shut``) or through its parent - proxy (``sm.door.shut``). Both resolve to the same per-instance proxy. - """ + class Microwave(StateChart): + class microwave(State.Parallel): + my_state = State(initial=True) - class Microwave(StateChart): - class microwave(State.Parallel): - my_state = State(initial=True) + class door(State.Compound): + shut = State(initial=True) + opened = State() - class door(State.Compound): - shut = State(initial=True) + toggle = shut.to(opened) | opened.to(shut) - sm = await sm_runner.start(Microwave) + async def test_nested_child_resolves_to_the_same_proxy_on_every_path(self, sm_runner): + """A compound child nested inside a parallel region reports ``is_active`` + consistently whether reached directly (``sm.shut``), through its parent + (``sm.door.shut``) or back through ``parent``.""" + sm = await sm_runner.start(self.Microwave) assert sm.my_state.is_active assert sm.door.is_active assert "shut" in sm.configuration_values assert sm.shut.is_active assert sm.door.shut.is_active + assert sm.door.shut is sm.shut + assert sm.door.shut.parent.is_active + assert sm.shut.parent is sm.door + + async def test_nested_child_is_isolated_between_instances(self, sm_runner): + """Each machine resolves nested children to its own proxies, so one + instance toggling a region does not leak into another.""" + sm = await sm_runner.start(self.Microwave) + other = await sm_runner.start(self.Microwave) + + await sm_runner.send(other, "toggle") + + assert other.door.opened.is_active + assert not other.door.shut.is_active + assert sm.door.shut.is_active + assert not sm.door.opened.is_active From f1ec945c872ae97a18389641c7f27990bb4c2fb8 Mon Sep 17 00:00:00 2001 From: Fernando Macedo Date: Sun, 11 Oct 2026 20:25:58 -0300 Subject: [PATCH 3/3] docs: add nested parallel is_active fix to the 3.2.2 release notes Signed-off-by: Fernando Macedo --- docs/releases/3.2.2.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/docs/releases/3.2.2.md b/docs/releases/3.2.2.md index 970d292a..47b25b89 100644 --- a/docs/releases/3.2.2.md +++ b/docs/releases/3.2.2.md @@ -131,3 +131,39 @@ effect at trigger time: 50 ``` + +### `is_active` on a state nested in a parallel region + +Reaching a child state through its parent (`sm.door.shut`) returned the class-level `State`, +whose `is_active` is always `False`, while `sm.shut.is_active` on the same machine was `True`. +The same happened to `parent`. A state reached through any attribute path now resolves to the +machine's per-instance proxy: + +```py +>>> class Microwave(StateChart): +... class microwave(State.Parallel): +... my_state = State(initial=True) +... +... class door(State.Compound): +... shut = State(initial=True) + +>>> sm = Microwave() +>>> sm.door.shut.is_active +True + +>>> sm.door.shut is sm.shut +True + +>>> sm.shut.parent.is_active +True + +``` + +Creating a machine also rebound the shared `parent` of every nested state to that machine's +proxy, so with two instances alive, `parent` on the first answered for the second. Each +instance now keeps its own proxies and the class-level states are never mutated. + +Reported by [@MartinBernstorff](https://github.com/MartinBernstorff). + +[#651](https://github.com/fgmacedo/python-statemachine/issues/651), +[#652](https://github.com/fgmacedo/python-statemachine/pull/652).