Skip to content

Python: resolve postponed @response_handler annotations - #8328

Open
CoralGarden52 wants to merge 3 commits into
microsoft:mainfrom
CoralGarden52:fix/python-response-handler-postponed-annotations
Open

Python: resolve postponed @response_handler annotations#8328
CoralGarden52 wants to merge 3 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 on lines +355 to +358
try:
type_hints = typing.get_type_hints(func)
except (NameError, AttributeError, RecursionError):
type_hints = {p.name: p.annotation for p in params}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Implemented in commit cf3e412. Added regression coverage for the unresolved-annotation fallback path and verified that it produces the existing ValueError diagnostic. The postponed-annotation test now uses WorkflowContext[str, bool] and asserts both output_types and workflow_output_types. The targeted executor tests and full workflow test suite pass.

Comment thread python/packages/core/tests/workflow/test_executor_future.py Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@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.

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

3 participants