Fix SCXML parallel transition domains - #26
Conversation
There was a problem hiding this comment.
Code Review
This pull request corrects the SCXML transition-domain resolution by ensuring that the least common compound ancestor (find_lcca) is a compound state. This change resolves the remaining failures in the more-parallel SCXML conformance suite, allowing all 54 configured SCXML test cases to pass. The documentation, changelog, and test suites have been updated to reflect this milestone. I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 254148e2e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """Return the least common compound ancestor for SCXML transition domains.""" | ||
| for anc in get_proper_ancestors(state_list[0], state2=None): | ||
| if all(is_descendent(s, state2=anc) for s in state_list[1:]): | ||
| if is_compound_state(anc) and all( |
There was a problem hiding this comment.
Preserve root-parallel transition domains
When the machine root is itself type: "parallel", this filter skips that root parallel ancestor, so find_lcca returns None for an external transition declared on a region that targets a descendant such as a.on.RESET -> #a1. The entry-set code then uses ancestor=None and re-enters the machine root, causing root entry actions to run again on every regional reset; through the public Machine.transition path there is no matching root exit because configurations are reconstructed from state.value without the root node. Before this change the root parallel node was the transition domain, so this root-level side effect did not occur.
Useful? React with 👍 / 👎.
Summary
more-parallelfailuresmore-parallelcases into the required SCXML CI subsetRoot Cause
find_lcca()accepted any common ancestor, including a parallel state. For external transitions sourced inside one parallel region, that made the parallel parent the transition domain. The algorithm exited sibling regions but did not re-enter the parallel parent, so those siblings disappeared from the resulting configuration.SCXML requires the least common compound ancestor. Restricting the domain candidate restores parallel entry fan-out and also gives conflict resolution the correct exit sets.
Behavioral Impact
External transitions inside a parallel state may now correctly exit and re-enter the enclosing parallel branch. Affected sibling regions return to their initial states and their exit/entry actions run once. Competing transitions continue to resolve by source depth and document order.
There are no public API or type changes.
Machine(config, ...), snapshot immutability, handler adaptation, and the safe SCXML condition subset are unchanged.Validation
poetry run python -m pytest tests/ --ignore=tests/test_scxml.py— 392 passedpoetry run python -m pytest tests/test_parallel.py tests/test_scxml.py -q— 63 passedpoetry run mypy src/xstate/poetry run ruff format --check src/ tests/poetry run ruff check src/ tests/git diff --checkConformance Scope
The configured SCXML subset is now green, including all enabled
more-parallelcases. This PR does not claim broad W3C datamodel or executable-content conformance; disabled assignment-oriented cases and general JavaScript evaluation remain out of scope.