Repository navigation
Python: fix(workflows): support keyword-only RunContext in functional workflows - #9397
Nithin (nithin42) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The focused binding fix preserves positional behavior, includes regression coverage, and has no identified blocking issues.
0 open findings
What changed in this PR
Fixes #9324 by respecting keyword-only parameters when invoking functional workflows, while preserving positional argument handling.
Changes:
- Pass keyword-only context and input parameters by name.
- Add regression tests for annotated and unannotated context parameters, keyword-only inputs, and streaming.
| File | Description |
|---|---|
python/packages/core/tests/workflow/test_functional_workflow.py |
Adds keyword-only parameter regression tests. |
python/packages/core/agent_framework/_workflows/_functional.py |
Injects keyword-only arguments through keyword bindings. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Bicheng (Kenneth) (BichengWang)
left a comment
There was a problem hiding this comment.
Reviewed 8d8806dab43bbc71b13537c59533aab25a177131. I found no correctness blocker in the argument-binding change: keyword-only values are passed by name, while positional-only and ordinary positional parameters keep their existing ordering. Both streaming and non-streaming runs reach the same _execute path.
Offline validation on macOS arm64 / CPython 3.14.7:
- All 159 tests in
python/packages/core/tests/workflow/test_functional_workflow.pypassed. - Replacing only
_executein memory with the parent commit's implementation made all four new regression tests fail with the expected positional-argumentTypeError; all four pass with this head. - An additional 20-case harness passed: eight input/context signature arrangements in both streaming modes, plus four HITL resume cases covering response-only continuation and checkpoint restore into a newly built workflow. The resume cases used an explicit replay key and verified that a completed step ran only once.
Non-blocking coverage suggestion: keep a test for an annotated context named something other than ctx, such as async def wf(text: str, *, context: RunContext), and one keyword-only-context HITL checkpoint-resume test. These worked in the additional harness, but the committed tests currently exercise the ctx name fallback and state access only.
I checked the existing review and inline comments before this review. Validation used cached dependencies and local source; I did not run the full workspace suite, type checks, other Python versions, or service integration tests. The issue-first maintainer agreement for #9324 remains a separate prerequisite for the maintainers to resolve.
8d8806d to
d134b43
Compare
|
Thanks for testing this so thoroughly Bicheng (Kenneth) (@BichengWang)! Really appreciate the feedback. I'll wait to hear what the maintainers think and will happily add those extra tests if they'd like them. |
Summary
Fixes #9324.
Resolves an issue where defining a
@workflowwith a keyword-onlyRunContextparameter (e.g.async def wf(text: str, *, ctx: RunContext) -> str:) succeeded at decoration time but failed during execution with: