Visitar URL original
Python: fix(python): fix operator precedence and associativity in WorkflowState fallback evaluator by PratikWayase · Pull Request #10329 · microsoft/agent-framework · GitHub
Skip to content

Python: fix(python): fix operator precedence and associativity in WorkflowState fallback evaluator - #10329

Open
pratik wayase (PratikWayase) wants to merge 2 commits into
microsoft:mainfrom
PratikWayase:fix/10162-fallback-precedence
Open

pratik wayase (PratikWayase) wants to merge 2 commits into
microsoft:mainfrom
PratikWayase:fix/10162-fallback-precedence

Conversation

@PratikWayase

Copy link
Copy Markdown
Contributor

Motivation & Context

When PowerFx is not available, WorkflowState.eval() falls back to _eval_simple(), which evaluates formulas by splitting the string on operators. That approach gets And/Or precedence wrong, treats - and / as right-associative, and matches operators inside string literals, causing formulas to silently produce wrong values (repro cases in #10162). This change replaces the string-splitting logic with a small parser so the fallback agrees with PowerFx on the documented expression subset.

Description & Review Guide

  • What are the major changes?
    • Replaced string splitting in _eval_simple() with a tokenizer and precedence-climbing parser (Or < And < comparisons < + - < * / < unary !/Not()/negation), left-associative at every level, with (...) grouping while skipping string literals and comments.
    • Raised _UnsupportedExpressionError (a ValueError) for expressions outside the documented subset instead of returning incorrect results.
    • Preserved existing tolerant behaviors (None as 0 in arithmetic, + string concatenation fallback, division by zero returning None, and echoing the formula on runtime errors).
  • What is the impact of these changes?
    • Existing single-operator tests pass unchanged; two tests asserting the old echo behavior for & now assert the raise.
    • Added regression tests covering the four issue repro cases along with precedence, associativity, parentheses, and quoted strings.
  • What do you want reviewers to focus on?
    • The precedence ladder and left-associativity in the parser chain, and raising vs. echoing for out-of-subset expressions.

Related Issue

Fixes #10162

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: Used AI to help analyze the issue, the Copilot plan, and draft the PR description. The code changes and tests were written, reviewed, and verified locally.

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 links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • 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 any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI balanced review requested due to automatic review settings November 13, 2026 05:00
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Nov 13, 2026
@github-actions github-actions Bot changed the title fix(python): fix operator precedence and associativity in WorkflowState fallback evaluator Python: fix(python): fix operator precedence and associativity in WorkflowState fallback evaluator Nov 13, 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.

🟡 Changes recommended

Arithmetic failures are treated as successful operands, producing incorrect comparison results, while early returns leave supported operator chains partially parsed and trigger false syntax errors.

6 open issues
What changed in this PR

This PR replaces WorkflowState’s string-splitting fallback evaluator with a tokenizer and parser to address #10162 when PowerFx is unavailable.

Changes:

  • Adds operator precedence, left-associative arithmetic, parentheses, and quote-aware parsing.
  • Defines the supported expression subset and raises ValueError for unsupported syntax.
  • Adds regression tests for expression evaluation and malformed-input rejection.
File Description
python/​packages/​declarative/​tests/​test_workflow_state.py Adds precedence and rejection tests; updates unsupported-operator expectations.
python/​packages/​declarative/​agent_framework_declarative/​_workflows/​_state.py Introduces tokenization and parsing, replaces string splitting, and documents fallback behavior.

Review effort: Balanced

This branch was successfully deployed

1 active deployment
github-app-auth — df489c80 Deployed Nov 13, 2026 by PratikWayase via add_label #27784
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: WorkflowState fallback evaluator ignores And/Or precedence and evaluates chained - and / right-to-left

2 participants