fix: key instance states by state.value, not state.id (#624)

[AndreyNenashev]

Summary

Restores the 3.0.0 active-state resolution path that PR #592 inadvertently changed. Configuration._instance_states is re-keyed by state.value (the canonical identifier already used by states_map) instead of state.id (the Python attribute name, which collides across sibling Compounds with same-named children).

Closes #624.

Change

• statemachine/statemachine.py:211 — _build_configuration writes instance_states[state.value] = ist.
• statemachine/configuration.py:87 — Configuration.states resolves directly: self._instance_states[v] (drops the redundant _states_map[v].id hop).
• statemachine/configuration.py:36 — type hint loosened to Mapping[Any, State] to match the established dict[Any, State] convention used by states_map.

The vars(self)[state.id] = ist user-facing attribute shortcut at statemachine.py:213 is untouched — orthogonal concern.

Tests

New TestSiblingCompoundIdCollision class in tests/test_configuration.py:

• test_dispatch_to_g1_inner_state_both_engines — the exact Sibling Compound states with same-named inner attributes break in 3.1.0+ (regression bisected to #592) #624 repro, parametrized sync/async via sm_runner.
• test_dispatch_to_g2_inner_state_both_engines — same shape but in the second compound.
• test_instance_states_keyed_by_value_no_collision — white-box guard: len(_instance_states) == len(states_map) and key sets match.

Full suite: 1472 passed, 144 skipped, 44 xfailed in ~23s. Branch coverage on statemachine.configuration and statemachine.statemachine preserved.

Out of scope — deeper issue surfaced during review

Charts that rely on default value= and reuse Python attribute names across sibling Compounds were never supported, in any version:

class g1(State.Compound):
    a = State(initial=True)   # value defaults to "a"
class g2(State.Compound):
    a = State(initial=True)   # value defaults to "a" — overwrites g1.a in states_map

factory.py:341 (cls.states_map[state.value] = state) silently overwrites. The chart constructs successfully but states_map has fewer entries than declared states. This predates #592 and is not a regression — it cannot be reached or repaired at the _instance_states layer.

Tracking as separate follow-ups:

• Validate uniqueness at class-build time — raise InvalidDefinition in factory.add_state when state.value collides. Surfaces the constraint instead of silently overwriting. Backward-incompatible (charts that "worked by accident" will refuse to construct); should land in a minor release with notes.
• Auto-qualify nested state.id (SCXML-style globally-unique ids) — would also make the default-value case work, but touches __repr__, diagrams, and callback name resolution. Larger design discussion.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (34d51cf) to head (88a1e26).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop      #625   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           42        42           
  Lines         4976      4976           
  Branches       812       812           
=========================================
  Hits          4976      4976           

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.

[fgmacedo]

Hi @AndreyNenashev, thanks a lot for the exceptionally detailed report and the fix! 🙏

The write-up in #624 was a model bug report: clean repro, correct bisect to #592, and a precise root-cause analysis. I reproduced it on develop and confirmed the fix resolves it on both engines, with the full suite green and no regressions.

The approach is spot on: re-keying _instance_states by state.value aligns it with the canonical identifier already used by states_map, restoring the 3.0.0 resolution path with a minimal, well-targeted change. I also really appreciate you surfacing the deeper default-value= collision as a separate, well-scoped follow-up instead of folding it into this bugfix.

Validating value uniqueness at class-build time (raising InvalidDefinition) is the natural next step, and your offer to put up a PR for it would be very welcome. Thanks again!