fix: resolve state keys before builtins in Condition.safe_expr

[digit50]

closes #960

Condition.safe_expr resolved a name to an allowlisted builtin before looking at state, so a state key named max, sum or any evaluated to the builtin function. Condition.expr resolves state first, so the same expression gave a different answer: count < max raised TypeError, and sum == 0 silently returned False and sent a transition down the default edge.

Changes

• _SafeExprInterpreter._eval_Name now looks up the state names first and falls back to _SAFE_EXPR_BUILTINS, matching expr().
• The name collector for reads no longer drops every name found in builtins.__dict__. It skips the function name of a call (the validator only admits calls to allowlisted builtins, and the interpreter always resolves those to the builtin) and records every other name, so a state key named max or sum is reported as read and covered by the missing-key check.
• Calls are unchanged: max(a, b) still calls the builtin even when state has a max key.

How I tested this

Added test_safe_expr_state_key_shadows_builtin in tests/core/test_action.py. With state {"count": 3, "max": 5, "sum": 0, "any": False} it checks that count < max, sum == 0 and any give the same result under safe_expr as under expr and that reads contains the shadowed names, and that max(count, sum) == 3 still calls the builtin with reads == ["count", "sum"]. The test fails on main with TypeError: '<' not supported between instances of 'int' and 'builtin_function_or_method' and passes with the change.

pytest tests/core/test_action.py passes (155 tests). pytest tests/core gives the same result with and without the change apart from the new test; the only failures are the four test_visualize_dot_output cases, which need the Graphviz dot binary that my environment does not have.

Notes

Bare uses of an allowlisted builtin name that is not in state (for example any == 1) are now listed in reads, so they fail the missing-key check instead of comparing against the builtin function. I could not find a meaningful expression that relied on the old behaviour.

Checklist

• PR has an informative and human-readable title (this will be pulled into the release notes)
• Changes are limited to a single goal (no scope creep)
• Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
• Any change in functionality is tested
• New functions are documented (with a description, list of inputs, and expected output)
• Placeholder code is flagged / future TODOs are captured in comments
• Project documentation has been updated if adding/changing functionality.

[elijahbenizzy]

LGTM, this was a silent wrong-result. One docstring line to add in a follow-up (I'll fold it into #947 since that PR already rewrites the paragraph): bare names resolve state-first, calls always bind the allowlisted builtin even if state has that key -- intentionally different from expr(). Elijah