Repository navigation
Python: [BREAKING] Align workflow state attribute lookup - #8893
Eduard van Valkenburg (eavanvalkenburg) merged 2 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation consistently applies the documented rule and includes focused regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Aligns attribute lookup rules across standalone and checkpoint-backed Python workflow state while preserving dictionary-key behavior.
Changes:
- Adds a shared full-string attribute validator.
- Applies validation to standalone state lookup.
- Adds regression tests and clarifies documentation.
| File | Description |
|---|---|
AGENTS.md |
Clarifies state implementations and lookup rules. |
_state.py |
Validates standalone object attributes. |
_state_path.py |
Defines the shared validator. |
_declarative_base.py |
Uses the shared validator. |
test_workflow_state.py |
Tests standalone lookup and fallback behavior. |
test_declarative_state_path_safety.py |
Tests shared path-safety semantics. |
test_graph_executors.py |
Verifies factory state routing. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (1 commit(s)): d215d2736376
Model: gpt-5.6-sol
Overview
The PR centralizes the object-attribute segment validator and applies it consistently to both workflow state getters while preserving unrestricted dictionary-key lookup. Validation occurs before attribute access, and focused tests cover invalid members, mixed dictionary/object traversal, evaluator routing, factory-managed state, and unchanged state-budget behavior. No publishable Critical, High, or Medium issue was established.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Code Coverage OverviewLanguages: Python Python / code-coverage/pythonThe overall line coverage in commit 254f8f9 in the Show a line coverage summary of the most covered files.
Updated |
There was a problem hiding this comment.
MAF Automated Review — Iteration 2
Result: No findings
Scope: 1 net-new commit(s): 254f8f9979c6
Model: gpt-5.6-sol
Overview
This incremental commit replaces the standard-library regex compiler with the package's available regex dependency while preserving the fixed ASCII identifier pattern and fullmatch behavior. Both state implementations validate before attribute access, retain unrestricted dictionary-key lookup, and are covered by shared boundary tests. No publishable defect was introduced in the authoritative incremental range.
Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Motivation & Context
Make object-attribute lookup consistent between standalone and factory-managed workflow state while preserving dictionary-key lookup.
Description & Review Guide
[A-Za-z][A-Za-z0-9_]*; rejected lookups return the supplied default. This changes standalone lookup behavior and rejects trailing newlines in modern attribute names. Dictionary keys, record conversion, evaluator fallback, and state-budget errors retain their existing behavior.Focused validation from
python\packages\declarative(Python 3.11, PowerFx 0.0.34):The package source distribution and wheel also built successfully with
uv buildusing a temporary output directory. The full unit and integration suites were not run.Related Issue
None. No overlapping open PR was found for this change.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after a language prefix) — a workflow keeps the label and title prefix in sync automatically.