mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Raise clear handler registration error for unresolved TypeVar annotations (#4944)
* Raise clear handler registration error for unresolved TypeVar (#4943) Detect unresolved TypeVar in message parameter annotations during handler registration in both _validate_handler_signature (Executor) and _validate_function_signature (FunctionExecutor). Raises a ValueError with an actionable message recommending @handler(input=..., output=...) or @executor(input=..., output=...) instead of letting TypeVar leak through to a confusing TypeCompatibilityError during workflow edge validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review feedback for #4943: reorder checks and harden function executor - Move TypeVar check before validate_workflow_context_annotation in _executor.py so users see the more actionable error first - Wrap get_type_hints in try/except in _function_executor.py matching the defensive pattern in _executor.py - Repurpose duplicate test to cover bounded TypeVar rejection - Add test_function_executor_allows_concrete_types for test symmetry Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Narrow get_type_hints except clause and add missing tests (#4943) - Narrow `except Exception` to `except (NameError, AttributeError, RecursionError)` in both _executor.py and _function_executor.py so unexpected failures in get_type_hints are not silently swallowed. - Add test_handler_unresolvable_annotation_raises to test_function_executor_future.py exercising the except branch of get_type_hints in the function executor path. - Add test_function_executor_rejects_bounded_typevar_in_message_annotation to test_function_executor.py for parity with the Executor bounded TypeVar test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add error ordering test for TypeVar vs WorkflowContext priority (#4943) Add test_handler_typevar_error_takes_priority_over_context_error to verify that when a handler has both a TypeVar message and an unannotated ctx, the TypeVar error is raised first (the more actionable issue). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Python: Fix image content serialization sending null file_id to Foundry API Omit file_id from input_image dict when not present instead of including it as null, which Azure AI Foundry's stricter schema validation rejects. * Python: Fix Foundry API rejecting rich content in function_call_output Azure AI Foundry does not support list-format output in function_call_output items. Add SUPPORTS_RICH_FUNCTION_OUTPUT flag (default True) to RawOpenAIChatClient, set to False in RawFoundryChatClient so Foundry falls back to string output for tool results with images/files. Also omit file_id from input_image dicts when not set, since Foundry rejects explicit nulls. * Python: Surface rich tool content as user message when Foundry lacks support When SUPPORTS_RICH_FUNCTION_OUTPUT is False, image/file items from tool results are injected as a follow-up user message so the model can still process the visual content via Foundry's supported user message format. * Xfail Foundry image integration test for the meantime --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
942cb04ccb
commit
36cafe4e5a
@@ -728,9 +728,22 @@ def _validate_handler_signature(
|
||||
# AttributeError, or RecursionError), so registration failures are easier to diagnose.
|
||||
try:
|
||||
type_hints = typing.get_type_hints(func)
|
||||
except Exception:
|
||||
except (NameError, AttributeError, RecursionError):
|
||||
type_hints = {p.name: p.annotation for p in params}
|
||||
|
||||
message_type = type_hints.get(message_param.name, message_param.annotation)
|
||||
if message_type == inspect.Parameter.empty:
|
||||
message_type = None
|
||||
|
||||
# Reject unresolved TypeVar in message annotation -- these are not supported
|
||||
# for workflow type validation and must be replaced with concrete types.
|
||||
if not skip_message_annotation and isinstance(message_type, TypeVar):
|
||||
raise ValueError(
|
||||
f"Handler {func.__name__} has an unresolved TypeVar '{message_type}' as its message type annotation. "
|
||||
"Generic TypeVar annotations are not supported for workflow type validation. "
|
||||
"Use @handler(input=<concrete_type>, output=<concrete_type>) to specify explicit types."
|
||||
)
|
||||
|
||||
# Validate ctx parameter is WorkflowContext and extract type args
|
||||
ctx_param = params[2]
|
||||
ctx_annotation = type_hints.get(ctx_param.name, ctx_param.annotation)
|
||||
@@ -744,10 +757,6 @@ def _validate_handler_signature(
|
||||
ctx_annotation, f"parameter '{ctx_param.name}'", "Handler"
|
||||
)
|
||||
|
||||
message_type = type_hints.get(message_param.name, message_param.annotation)
|
||||
if message_type == inspect.Parameter.empty:
|
||||
message_type = None
|
||||
|
||||
return message_type, ctx_annotation, output_types, workflow_output_types
|
||||
|
||||
|
||||
|
||||
@@ -325,11 +325,26 @@ def _validate_function_signature(
|
||||
if not skip_message_annotation and message_param.annotation == inspect.Parameter.empty:
|
||||
raise ValueError(f"Function instance {func.__name__} must have a type annotation for the message parameter")
|
||||
|
||||
type_hints = typing.get_type_hints(func)
|
||||
# Resolve string annotations from `from __future__ import annotations`.
|
||||
# Fall back to raw annotations if resolution fails (e.g. unresolvable forward refs,
|
||||
# AttributeError, or RecursionError), so registration failures are easier to diagnose.
|
||||
try:
|
||||
type_hints = typing.get_type_hints(func)
|
||||
except (NameError, AttributeError, RecursionError):
|
||||
type_hints = {p.name: p.annotation for p in params}
|
||||
message_type = type_hints.get(message_param.name, message_param.annotation)
|
||||
if message_type == inspect.Parameter.empty:
|
||||
message_type = None
|
||||
|
||||
# Reject unresolved TypeVar in message annotation -- these are not supported
|
||||
# for workflow type validation and must be replaced with concrete types.
|
||||
if not skip_message_annotation and isinstance(message_type, typing.TypeVar):
|
||||
raise ValueError(
|
||||
f"Function instance {func.__name__} has an unresolved TypeVar '{message_type}' as its message type "
|
||||
"annotation. Generic TypeVar annotations are not supported for workflow type validation. "
|
||||
"Use @executor(input=<concrete_type>, output=<concrete_type>) to specify explicit types."
|
||||
)
|
||||
|
||||
# Check if there's a context parameter
|
||||
if len(params) == 2:
|
||||
ctx_param = params[1]
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
# Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
from dataclasses import dataclass
|
||||
from typing import Generic, TypeVar
|
||||
|
||||
import pytest
|
||||
from typing_extensions import Never
|
||||
@@ -919,3 +920,87 @@ class TestHandlerExplicitTypes:
|
||||
|
||||
|
||||
# endregion: Tests for @handler decorator with explicit input_type and output_type
|
||||
|
||||
|
||||
# region Tests for unresolved TypeVar rejection in handler registration
|
||||
|
||||
_T = TypeVar("_T")
|
||||
|
||||
|
||||
def test_handler_rejects_unresolved_typevar_in_message_annotation():
|
||||
"""Test that @handler raises ValueError when the message parameter is an unresolved TypeVar."""
|
||||
|
||||
with pytest.raises(ValueError, match="unresolved TypeVar"):
|
||||
|
||||
class GenericEcho(Executor, Generic[_T]):
|
||||
@handler
|
||||
async def echo(self, message: _T, ctx: WorkflowContext) -> None:
|
||||
pass
|
||||
|
||||
|
||||
_BT = TypeVar("_BT", bound=str)
|
||||
|
||||
|
||||
def test_handler_rejects_bounded_typevar_in_message_annotation():
|
||||
"""Test that @handler raises ValueError for a bounded TypeVar in message annotation."""
|
||||
|
||||
with pytest.raises(ValueError, match="unresolved TypeVar"):
|
||||
|
||||
class BoundedGenericExecutor(Executor, Generic[_BT]):
|
||||
@handler
|
||||
async def process(self, message: _BT, ctx: WorkflowContext) -> None:
|
||||
await ctx.send_message(message)
|
||||
|
||||
|
||||
def test_handler_allows_concrete_types():
|
||||
"""Test that @handler works normally with concrete type annotations."""
|
||||
|
||||
class ConcreteExecutor(Executor):
|
||||
@handler
|
||||
async def handle(self, message: str, ctx: WorkflowContext[str]) -> None:
|
||||
pass
|
||||
|
||||
exec_instance = ConcreteExecutor(id="concrete")
|
||||
assert str in exec_instance.input_types
|
||||
|
||||
|
||||
def test_handler_explicit_input_bypasses_typevar_check():
|
||||
"""Test that @handler(input=...) bypasses TypeVar check since explicit types take precedence."""
|
||||
|
||||
class GenericWithExplicit(Executor, Generic[_T]):
|
||||
@handler(input=str, output=str)
|
||||
async def echo(self, message, ctx: WorkflowContext) -> None:
|
||||
pass
|
||||
|
||||
exec_instance = GenericWithExplicit(id="explicit")
|
||||
assert str in exec_instance.input_types
|
||||
|
||||
|
||||
def test_handler_error_message_recommends_explicit_types():
|
||||
"""Test that the TypeVar error message recommends @handler(input=..., output=...)."""
|
||||
|
||||
with pytest.raises(ValueError, match=r"@handler\(input=<concrete_type>, output=<concrete_type>\)"):
|
||||
|
||||
class GenericBad(Executor, Generic[_T]):
|
||||
@handler
|
||||
async def echo(self, message: _T, ctx: WorkflowContext) -> None:
|
||||
pass
|
||||
|
||||
|
||||
# endregion: Tests for unresolved TypeVar rejection in handler registration
|
||||
|
||||
|
||||
def test_handler_typevar_error_takes_priority_over_context_error():
|
||||
"""Test that TypeVar message error is raised before WorkflowContext validation.
|
||||
|
||||
When a handler has both a TypeVar message annotation and an unannotated ctx
|
||||
parameter, the TypeVar error should be reported first since it is the more
|
||||
actionable issue.
|
||||
"""
|
||||
|
||||
with pytest.raises(ValueError, match="unresolved TypeVar"):
|
||||
|
||||
class DualBad(Executor, Generic[_T]):
|
||||
@handler
|
||||
async def process(self, message: _T, ctx) -> None: # type: ignore[no-untyped-def]
|
||||
pass
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
# Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
from dataclasses import dataclass
|
||||
from typing import Any
|
||||
from typing import Any, TypeVar
|
||||
|
||||
import pytest
|
||||
from typing_extensions import Never
|
||||
@@ -895,3 +895,73 @@ class TestExecutorExplicitTypes:
|
||||
assert str in exec_instance._handlers # pyright: ignore[reportPrivateUsage]
|
||||
assert int in exec_instance.output_types
|
||||
assert bool in exec_instance.workflow_output_types
|
||||
|
||||
|
||||
# region Tests for unresolved TypeVar rejection in function executor registration
|
||||
|
||||
_FT = TypeVar("_FT")
|
||||
|
||||
|
||||
class TestFunctionExecutorTypeVarRejection:
|
||||
"""Tests that FunctionExecutor rejects unresolved TypeVar in message annotations."""
|
||||
|
||||
def test_function_executor_rejects_unresolved_typevar(self):
|
||||
"""Test that FunctionExecutor raises ValueError for unresolved TypeVar message annotation."""
|
||||
|
||||
def echo(message: _FT) -> _FT:
|
||||
return message
|
||||
|
||||
with pytest.raises(ValueError, match="unresolved TypeVar"):
|
||||
FunctionExecutor(echo, id="echo")
|
||||
|
||||
def test_function_executor_rejects_typevar_with_context(self):
|
||||
"""Test that FunctionExecutor raises ValueError for TypeVar even with WorkflowContext."""
|
||||
|
||||
async def echo(message: _FT, ctx: WorkflowContext) -> None:
|
||||
pass
|
||||
|
||||
with pytest.raises(ValueError, match="unresolved TypeVar"):
|
||||
FunctionExecutor(echo, id="echo")
|
||||
|
||||
def test_function_executor_explicit_input_bypasses_typevar_check(self):
|
||||
"""Test that explicit input= parameter bypasses TypeVar detection."""
|
||||
|
||||
async def echo(message: _FT, ctx: WorkflowContext) -> None:
|
||||
pass
|
||||
|
||||
exec_instance = FunctionExecutor(echo, id="echo", input=str, output=str)
|
||||
assert str in exec_instance.input_types
|
||||
|
||||
def test_function_executor_allows_concrete_types(self):
|
||||
"""Test that FunctionExecutor works normally with concrete type annotations."""
|
||||
|
||||
async def handle(message: str, ctx: WorkflowContext[str]) -> None:
|
||||
pass
|
||||
|
||||
exec_instance = FunctionExecutor(handle, id="concrete")
|
||||
assert str in exec_instance.input_types
|
||||
|
||||
def test_function_executor_error_recommends_explicit_types(self):
|
||||
"""Test that error message recommends @executor(input=..., output=...)."""
|
||||
|
||||
def echo(message: _FT) -> _FT:
|
||||
return message
|
||||
|
||||
with pytest.raises(ValueError, match=r"@executor\(input=<concrete_type>, output=<concrete_type>\)"):
|
||||
FunctionExecutor(echo, id="echo")
|
||||
|
||||
|
||||
# endregion: Tests for unresolved TypeVar rejection in function executor registration
|
||||
|
||||
|
||||
_FBT = TypeVar("_FBT", bound=str)
|
||||
|
||||
|
||||
def test_function_executor_rejects_bounded_typevar_in_message_annotation():
|
||||
"""Test that FunctionExecutor raises ValueError for a bounded TypeVar in message annotation."""
|
||||
|
||||
async def process(message: _FBT, ctx: WorkflowContext) -> None:
|
||||
await ctx.send_message(message)
|
||||
|
||||
with pytest.raises(ValueError, match="unresolved TypeVar"):
|
||||
FunctionExecutor(process, id="bounded")
|
||||
|
||||
@@ -4,6 +4,8 @@ from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from agent_framework import FunctionExecutor, WorkflowContext, executor
|
||||
|
||||
|
||||
@@ -37,3 +39,20 @@ class TestFunctionExecutorFutureAnnotations:
|
||||
spec = process_complex._handler_specs[0] # pyright: ignore[reportPrivateUsage]
|
||||
assert spec["message_type"] == dict[str, Any]
|
||||
assert spec["output_types"] == [list[str]]
|
||||
|
||||
def test_handler_unresolvable_annotation_raises(self):
|
||||
"""Test that an unresolvable forward-reference annotation raises ValueError.
|
||||
|
||||
When get_type_hints fails (e.g. NameError for NonExistentType), the code falls back
|
||||
to raw string annotations. The ctx parameter's raw string annotation is then not
|
||||
recognised as a valid WorkflowContext type, so a ValueError is still raised.
|
||||
"""
|
||||
with pytest.raises(ValueError):
|
||||
FunctionExecutor(
|
||||
_func_with_bad_annotation, # pyright: ignore[reportUnknownArgumentType]
|
||||
id="bad",
|
||||
)
|
||||
|
||||
|
||||
async def _func_with_bad_annotation(message: NonExistentType, ctx: WorkflowContext[int]) -> None: # noqa: F821 # type: ignore[name-defined]
|
||||
pass
|
||||
|
||||
@@ -124,6 +124,7 @@ class RawFoundryChatClient( # type: ignore[misc]
|
||||
"""
|
||||
|
||||
OTEL_PROVIDER_NAME: ClassVar[str] = "azure.ai.foundry" # type: ignore[reportIncompatibleVariableOverride, misc]
|
||||
SUPPORTS_RICH_FUNCTION_OUTPUT: ClassVar[bool] = False # type: ignore[reportIncompatibleVariableOverride, misc]
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
|
||||
@@ -715,6 +715,7 @@ async def test_integration_web_search() -> None:
|
||||
|
||||
@pytest.mark.flaky
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.xfail(reason="Azure AI Foundry stopped accepting array-format output in function_call_output ~2026-04-03")
|
||||
@skip_if_foundry_integration_tests_disabled
|
||||
@_with_foundry_debug()
|
||||
async def test_integration_tool_rich_content_image() -> None:
|
||||
|
||||
@@ -265,6 +265,7 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
|
||||
INJECTABLE: ClassVar[set[str]] = {"client"}
|
||||
STORES_BY_DEFAULT: ClassVar[bool] = True # type: ignore[reportIncompatibleVariableOverride, misc]
|
||||
SUPPORTS_RICH_FUNCTION_OUTPUT: ClassVar[bool] = True
|
||||
|
||||
FILE_SEARCH_MAX_RESULTS: int = 50
|
||||
|
||||
@@ -1353,16 +1354,17 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
return ret
|
||||
case "data" | "uri":
|
||||
if content.has_top_level_media_type("image"):
|
||||
return {
|
||||
result: dict[str, Any] = {
|
||||
"type": "input_image",
|
||||
"image_url": content.uri,
|
||||
"detail": content.additional_properties.get("detail", "auto")
|
||||
if content.additional_properties
|
||||
else "auto",
|
||||
"file_id": content.additional_properties.get("file_id", None)
|
||||
if content.additional_properties
|
||||
else None,
|
||||
}
|
||||
file_id = content.additional_properties.get("file_id") if content.additional_properties else None
|
||||
if file_id:
|
||||
result["file_id"] = file_id
|
||||
return result
|
||||
if content.has_top_level_media_type("audio"):
|
||||
if content.media_type and "wav" in content.media_type:
|
||||
format = "wav"
|
||||
@@ -1444,7 +1446,11 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
}
|
||||
# call_id for the result needs to be the same as the call_id for the function call
|
||||
output: str | list[dict[str, Any]] = content.result or ""
|
||||
if content.items and any(item.type in ("data", "uri") for item in content.items):
|
||||
if (
|
||||
self.SUPPORTS_RICH_FUNCTION_OUTPUT
|
||||
and content.items
|
||||
and any(item.type in ("data", "uri") for item in content.items)
|
||||
):
|
||||
output_parts: list[dict[str, Any]] = []
|
||||
for item in content.items:
|
||||
if item.type == "text":
|
||||
|
||||
@@ -2577,7 +2577,7 @@ def test_prepare_content_for_openai_image_content() -> None:
|
||||
result = client._prepare_content_for_openai("user", image_content_basic)
|
||||
assert result["type"] == "input_image"
|
||||
assert result["detail"] == "auto"
|
||||
assert result["file_id"] is None
|
||||
assert "file_id" not in result
|
||||
|
||||
|
||||
def test_prepare_content_for_openai_audio_content() -> None:
|
||||
|
||||
Reference in New Issue
Block a user