Skip to content

Python: resolve postponed @response_handler annotations - #8328

Merged
Evan Mattson (moonbox3) merged 7 commits into
microsoft:mainfrom
CoralGarden52:fix/python-response-handler-postponed-annotations
Sep 15, 2026
Merged

Evan Mattson (moonbox3) merged 7 commits into
microsoft:mainfrom
CoralGarden52:fix/python-response-handler-postponed-annotations

Conversation

@CoralGarden52

Copy link
Copy Markdown
Contributor

Motivation & Context

The introspection path of @response_handler reads raw values from inspect.signature(func). With from __future__ import annotations, valid request, response, and WorkflowContext[...] annotations are strings, so a valid executor class currently fails during decoration with ValueError and cannot be imported or instantiated. The existing @handler path already resolves these annotations; issue #8327 tracks the missing response-handler counterpart.

The bug was reproduced on clean upstream commit 3c670707766a8455da6491a9049cc9d575e019f0 with a four-parameter response handler using original_request: str, response: int, and ctx: WorkflowContext[str].

Description & Review Guide

  • What are the major changes?
    • Resolve response-handler annotations with typing.get_type_hints(func) during signature validation, matching the existing @handler behavior.
    • Fall back to raw annotations when resolution fails, preserving the existing diagnostic path for unresolved references.
    • Add a future-annotation regression test that verifies request type, response type, and workflow output type registration.
  • What is the impact of these changes?
    • Executors using postponed annotations can define and register @response_handler methods normally.
    • Existing handlers with evaluated annotations and explicit decorator type parameters retain their behavior.
    • Before the fix, the executor definition raised ValueError: Response handler parameter 'ctx' must be annotated ... got WorkflowContext[str]; after the fix it registers (str, int) with workflow output type str.
    • Verification evidence: the full core suite passed with 5044 passed, 131 skipped, 2 xfailed; relevant request-info, executor, and future-annotation tests passed; Ruff format/lint passed; Pyright reported 0 errors, 0 warnings for both changed files.
  • What do you want reviewers to focus on?
    • Whether the response-handler resolution and fallback behavior correctly mirror _validate_handler_signature.
    • Whether the regression test covers the public decorator/discovery path without changing explicit-parameter behavior.

Related Issue

Fixes #8327

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 any language prefix) — a workflow keeps the label and title prefix in sync automatically.

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

Add fallback-path coverage and verify workflow_output_types registration for two-argument contexts.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes postponed-annotation handling for Python @response_handler methods.

Changes:

  • Resolve annotations with typing.get_type_hints, with raw-annotation fallback.
  • Add regression coverage for future annotations.
File summaries
File Summary
python/packages/core/tests/workflow/test_executor_future.py Tests response-handler registration with postponed annotations.
python/packages/core/agent_framework/_workflows/_request_info_mixin.py Resolves response-handler parameter annotations.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.


💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/packages/core/agent_framework/_workflows/_request_info_mixin.py Outdated
Comment thread python/packages/core/tests/workflow/test_executor_future.py Outdated
@CoralGarden52
CoralGarden52 deployed to github-app-auth September 12, 2026 04:59 — with GitHub Actions Active
@CoralGarden52
CoralGarden52 deployed to github-app-auth September 12, 2026 05:08 — with GitHub Actions Active

@Ricky-7-Yan Ricky-7-Yan 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.

Reviewed current head cf3e412. Resolving annotations with typing.get_type_hints() and using the same (NameError, AttributeError, RecursionError) raw-annotation fallback as the existing @handler and function-executor validation paths keeps the decorator behaviors aligned. The regression exercises the public class-decoration/discovery path, verifies request/response plus both WorkflowContext type arguments, and the added unresolved-reference case covers the fallback diagnostic. I found no blocking issue in the scoped diff.

The full upstream Python workflows have not run on this external branch; this approval relies on the current code/tests and the author's reported full core suite, Ruff, and Pyright results rather than treating the governance checks as full CI.

Comment thread python/packages/core/agent_framework/_workflows/_request_info_mixin.py Outdated
@eavanvalkenburg

Copy link
Copy Markdown
Member

Thanks for the update. Before this is ready, could you please:

Once those are addressed, please re-request review. Thanks!

@CoralGarden52

Copy link
Copy Markdown
Contributor Author

Hi Evan Mattson (@moonbox3) , I’ve resolved the remaining outdated review discussions at _request_info_mixin.py:387 and _request_info_mixin.py:358, and confirmed that there are no unresolved review conversations left. I’ve re-requested your review—please take another look when you have a chance. Thank you for your time!

@moonbox3

Copy link
Copy Markdown
Contributor

Please fix failing CI/CD checks.

@CoralGarden52
CoralGarden52 deployed to github-app-auth September 15, 2026 06:26 — with GitHub Actions Active
@CoralGarden52
CoralGarden52 deployed to github-app-auth September 15, 2026 06:31 — with GitHub Actions Active
@CoralGarden52

Copy link
Copy Markdown
Contributor Author

Hi Evan Mattson (@moonbox3) , I’ve addressed the failing CI/CD checks and pushed the fixes in commits 7108dc7 and 48d8156. The relevant tests, Ruff, and Pyrefly checks pass locally. The PR is ready for another review when you have a chance. Thank you for pointing this out and for your time.

@moonbox3
Evan Mattson (moonbox3) added this pull request to the merge queue Sep 15, 2026
Merged via the queue into microsoft:main with commit 5409193 Sep 15, 2026
40 checks passed

This branch was successfully deployed

1 active deployment
github-app-auth — 48d81566 Deployed Sep 15, 2026 by CoralGarden52 via add_label #22855
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]: @response_handler fails with postponed annotations

5 participants