Skip to content

fix: support event declarations inside State.Compound bodies - #645

Open
fgmacedo wants to merge 3 commits into
developfrom
fix/643-event-inside-compound
Open

fix: support event declarations inside State.Compound bodies#645
fgmacedo wants to merge 3 commits into
developfrom
fix/643-event-inside-compound

Conversation

@fgmacedo

@fgmacedo fgmacedo commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Closes #643. Closes MAC-21 (personal board)

The bug

Inside a State.Compound / State.Parallel class body, only the assignment form of an event
declaration was understood:

visit_pub = bag_end.to(green_dragon)   # works

The nested class body scanner in NestedStateFactory.__new__ matched States, HistoryState,
State and TransitionList, then fell through to a generic callable(value) branch. An Event
is callable, so visit_pub = Event(bag_end.to(green_dragon)) landed there: the name was bound to
a detached placeholder Event (still carrying its generated __event__<uuid> id) and the
transition it wrapped never received an event, becoming eventless. It then fired as soon as its
source state became active, which is why the reporter's machine started already in green_dragon.

While fixing it I found the @<source>.to(<target>) decorator fails the same way in the same
place, for the same reason: add_from_attributes has an attr_name branch that the nested
scanner lacks. Inside a compound body it registered no event and never ran the decorated body
(sm.send("coin") raised AttributeError: 'function' object has no attribute 'name'). Both are
the same defect, so both are fixed here.

The fix

statemachine/state.py: the scanner is extracted from NestedStateFactory.__new__ into
_collect_nested_members and gains two branches, both placed before callable(value):

  • Event_bind_declared_event rebuilds the event with the id derived from the attribute
    name (or the explicit id=, when given), keeps name as the display name, and attaches it to
    the declared transitions. The rebuilt event is returned through the existing _callbacks
    unpacking so the Python attribute name keeps resolving to it even when id= differs.
  • decorated callbacks → _bind_decorated_event mirrors
    StateMachineMetaclass._add_unbounded_callback: the callback is stored under its mangled
    attr_name and, when it is an event, the attribute name is added to its transitions.

Nested declarations now follow the top-level rules: the attribute name becomes the event id,
an explicit id takes precedence, name is kept, and the error_ / done_state_ /
done_invoke_ prefixes expand to their dotted form.

Tests

TestEventClassInsideCompound and TestDecoratorEventInsideCompound in
tests/test_statechart_compound.py (14 of the 16 new test items fail on develop; the 2 that
pass are the @<event>.on non-regression guard for the new attr_name branch). They cover the
reporter's exact machine, display name, explicit id, combined transitions via |, the
error_ prefix, a parallel region, a transition-less Event, and the decorator form.

Each test asserts the configuration before sending, so an eventless regression cannot pass by
letting the machine auto-advance to the expected end state.

Full suite green with 100% branch coverage; ruff, mypy and pyright clean.

Known remaining divergence

An Event declared inside a nested body with no transitions at all (knock = Event()) is
reachable as a class attribute but is not added to the machine's event list, unlike the
top-level form which registers it. The nested factory has no channel to the metaclass's event
registry for a transition-less event. This is asserted by a test and stated in the release
notes rather than left implicit.

Adversarial review

An independent sub-agent review attacked the fix and surfaced two unrelated pre-existing bugs
that are not addressed here (both reproduce on develop, neither is a regression):

  1. Subclassing any StateChart whose compound body has callbacks crashes:
    AttributeError: 'State' object has no attribute '_callbacks'. factory.py
    _unpack_builders_callbacks does del state._callbacks, and the nested State object is
    shared with the subclass via add_inherited, so the subclass re-reads a deleted attribute.
  2. delay= / internal= passed to Event(...) are silently dropped at both levels:
    Event(dark.to(lit), delay=500) yields SM.light.delay == 0 and fires immediately.
    tests/test_statechart_delayed.py::test_delayed_event_on_event_definition does not catch it
    because it builds its own BoundEvent(id="light", delay=50) to trigger instead of using the
    declared event.

A nested state class body only understood the assignment form of an event
declaration. The `Event` class and the `@<source>.to(<target>)` decorator both
fell through to the generic callable branch, so the name was bound to a
detached object and the transition it wrapped stayed eventless, firing as soon
as its source state became active.

Handle both forms in the nested class body scanner, which is extracted from
`NestedStateFactory.__new__` into `_collect_nested_members`.

Closes #643

Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (78e2b4c) to head (b8cd61e).

Additional details and impacted files
@@            Coverage Diff            @@
##           develop      #645   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           52        53    +1     
  Lines         5572      5608   +36     
  Branches       879       876    -3     
=========================================
+ Hits          5572      5608   +36     
Flag Coverage Δ
unittests 100.00% <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.

The two kinds of statechart class body, a StateChart subclass and a nested
State.Compound / State.Parallel, accept the same declaration forms, but each
had its own copy of the recognition table. That is how #643 happened: the
nested copy never learned about `Event`, so an event declared there silently
became an eventless transition.

Recognition now lives once in `class_body.read`, which dispatches to a reader
supplying only what each side does with a form. The reader interface is a
Protocol, so a form added to one side and forgotten on the other is a type
error rather than a silent gap.

Drop the `error_` prefix expansion the previous commit gave to nested
decorated events: it is not what the top-level path does, and the two must
agree.

Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
The docs describe the current behavior. The previous behavior was a bug, not a
documented contract, and the release notes already carry the history.

Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
@fgmacedo

Copy link
Copy Markdown
Owner Author

Two commits added after a design review of the fix.

refactor: read statechart class bodies through one shared reader

The review's central objection: add_from_attributes and the nested class-body scanner both
encoded the same recognition table, and that duplication is why #643 happened. It was not a
hypothetical drift either. The two already disagreed on @a.to(b) def error_foo, where the
first commit had given the nested path an error. expansion the top level does not do.

Recognition now lives once in statemachine/class_body.py::read, dispatching to a reader that
supplies only what each side does with a form: factory._StateChartBody and state._NestedBody.
The reader interface is a Protocol, so a form implemented on one side and forgotten on the
other is a type error:

statemachine/state.py:124: error: Argument 2 to "read" has incompatible type "_NestedBody"; expected "ClassBodyReader"
note: "_NestedBody" is missing following "ClassBodyReader" protocol member:  on_decorated

Also in this commit: both # noqa: C901 gone, the two single-use binding helpers removed,
_add_unbounded_callback inlined into its only caller, and the nested error_ expansion
reverted so both paths agree.

Straightening the recognition chain exposed something worth noting. add_from_attributes had an
if where the chain otherwise used elif, so a States value fell all the way through to
getattr(value, "attr_name", None), which called States.__getattr__ and raised. That accident
was the only coverage of states.py:57. Rather than preserve the quirk, States.__getattr__ now
has a direct test.

The new TestEventClassInsideCompound::test_event_id_expansion_conventions was a near-copy of
the existing test_error_execution_inside_compound, down to the class name, so the two are
collapsed into one parametrize over the declaration form.

docs: drop the versionchanged note for the compound Event fix

The docs describe current behavior; the release notes carry the history.

Follow-ups filed

The two pre-existing bugs found while reviewing are now #646 (subclassing a StateChart with
callbacks in a compound body raises AttributeError) and #647 (Event(delay=…, internal=…)
silently dropped). Neither is touched here.

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The Event class is not fully supported in State.Compound

1 participant