Skip to content

Python: [BREAKING] Align workflow state attribute lookup - #8893

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 2 commits into
mainfrom
jpalvarezl-workflow-state-access
Sep 30, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 2 commits into
mainfrom
jpalvarezl-workflow-state-access

Conversation

@jpalvarezl

Copy link
Copy Markdown
Member

Motivation & Context

Make object-attribute lookup consistent between standalone and factory-managed workflow state while preserving dictionary-key lookup.

Description & Review Guide

  • What are the major changes? Share a full-string attribute-name validator between both state getters. Add focused lookup, expression-evaluation, and factory-routing regressions, and clarify the state API documentation.
  • What is the impact of these changes? Object-attribute names must match [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.
  • What do you want reviewers to focus on? The distinction between attribute access and dictionary data, validation before attribute access, and preservation of each evaluator's existing behavior.

Focused validation from python\packages\declarative (Python 3.11, PowerFx 0.0.34):

python -X utf8 -m pytest -o addopts='' -q -p no:cacheprovider tests\test_workflow_state.py tests\test_powerfx_safe.py tests\test_declarative_state_path_safety.py::TestStateMemberAccess tests\test_declarative_state_path_safety.py::TestGetAllowsValidPaths tests\test_graph_executors.py::TestFactoryStateMemberRoutes -m 'not integration'
# Passed: 168 tests.

$source = @('agent_framework_declarative\_workflows\_state.py', 'agent_framework_declarative\_workflows\_state_path.py', 'agent_framework_declarative\_workflows\_declarative_base.py')
$tests = @('tests\test_workflow_state.py', 'tests\test_declarative_state_path_safety.py', 'tests\test_graph_executors.py')
python -m ruff format --check @source @tests
# Passed: all six files formatted.
python -m ruff check --no-fix @source @tests
# Passed: no findings.
python -m pyright --project pyproject.toml --pythonpath (Get-Command python).Source @source
# Passed: zero errors or warnings.
python -m pyright --project ..\..\pyrightconfig.tests.json --pythonpath (Get-Command python).Source @tests
# Passed: zero errors or warnings.

The package source distribution and wheel also built successfully with uv build using 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

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after a language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI balanced review requested due to automatic review settings September 30, 2026 09:22
@agent-framework-automation agent-framework-automation Bot added documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python breaking change Usage: [PRs], Target: all PRs that introduce changes that are not backward compatible labels Sep 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-code-quality

github-code-quality Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: Python

Python / code-coverage/python

The overall line coverage in commit 254f8f9 in the jpalvarezl-workflow-... branch is 92%. Line coverage data for the main branch is not yet available.

Show a line coverage summary of the most covered files.
File main jpalvarezl-workflow-... 254f8f9 +/-
packages/core/a...ework/_tools.py — 96% —
packages/core/a...ework/_types.py — 95% —
packages/core/a...work/_skills.py — 95% —
packages/openai..._chat_client.py — 94% —
packages/core/a.../_compaction.py — 94% —
packages/core/a...ork/_vectors.py — 93% —
packages/core/a...amework/_mcp.py — 92% —
packages/ag-ui/...i/_agent_run.py — 90% —
packages/core/a...ork/security.py — 89% —
packages/foundr...g/_responses.py — 87% —

Updated September 30, 2026 10:08 UTC

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Merged via the queue into main with commit 3d39258 Sep 30, 2026
48 checks passed
@eavanvalkenburg
Eduard van Valkenburg (eavanvalkenburg) deleted the jpalvarezl-workflow-state-access branch September 30, 2026 11:37

This branch was successfully deployed

1 active deployment
github-app-auth — 254f8f99 Deployed Sep 30, 2026 by jpalvarezl via add_label #24103
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Usage: [PRs], Target: all PRs that introduce changes that are not backward compatible documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants