Python: Fix PydanticSchemaGenerationError when using from __future__ import annotations with @tool#4822
Python: Fix PydanticSchemaGenerationError when using from __future__ import annotations with @tool#4822giles17 wants to merge 6 commits intomicrosoft:mainfrom
from __future__ import annotations with @tool#4822Conversation
_resolve_input_model used raw param.annotation from inspect.signature(), which returns string annotations when 'from __future__ import annotations' is active (PEP 563). This caused Pydantic's create_model to fail for complex types like Optional[int] or FunctionInvocationContext. Use typing.get_type_hints() to resolve annotations to actual types before passing them to create_model, matching the approach already used by _discover_injected_parameters. Fixes microsoft#4809 Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
giles17
left a comment
giles17
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 93%
✓ Correctness
This PR correctly removes unused imports (Optional, pytest, FunctionTool) from test_tools_future_annotations.py and deletes the temporary REPRODUCTION_REPORT.md file. All remaining imports are used in the test file. The test file uses
int | Nonesyntax rather thanOptional[int], and none of the test functions referencepytestorFunctionTooldirectly. No correctness issues found.
✓ Security Reliability
This diff is pure cleanup: it removes a temporary investigation artifact (REPRODUCTION_REPORT.md) and three unused imports (Optional, pytest, FunctionTool) from the test file. The test file uses
int | Nonesyntax instead ofOptional, has no pytest markers/fixtures/assertions, and doesn't referenceFunctionTool. The actual fix to_resolve_input_modelusingtyping.get_type_hints()is already present in_tools.pylines 469-472 and is not part of this diff. No security or reliability concerns.
✗ Test Coverage
The test file covers the key PEP 563 scenarios (context parameter exclusion, optional params, invocation) but has weak assertions that don't verify type correctness—the core of the bug. Tests only check parameter presence (e.g.,
'limit' in params['properties']) rather than verifying the type was correctly resolved (e.g., checking thatlimit's schema includesinteger/nulltypes). A test that just checks key existence would pass even if types were still unresolved strings. Additionally, there is no test covering theget_type_hints()failure fallback path (theexcept Exceptionbranch added in _tools.py:471-472), and no test usingOptional[X]syntax fromtypingwhich was the exact failure in the original issue report.
✓ Design Approach
This diff removes an investigation artifact (REPRODUCTION_REPORT.md) that was committed by mistake and cleans up unused imports in the test file. The test coverage itself is solid: it exercises the four scenarios relevant to the PEP 563 fix (context-param exclusion in two orderings, Optional resolution, and async invocation). The removal of
Optionalin favour ofint | Noneis consistent with thefrom __future__ import annotationscontext and modern Python style. No design issues are present.
Flagged Issues
- Tests for
test_tool_with_optional_paramandtest_tool_with_optional_param_and_contextonly assert that parameter keys exist in the schema properties, but do not verify that the types were correctly resolved. This is the core of the bug—types becoming unresolved strings under PEP 563—so the assertions should verify type information (e.g., thatlimit's schema containsanyOfwithintegerandnulltypes). Without this, the tests would pass even if the fix were reverted.
Suggestions
- Add a test that verifies the
get_type_hints()failure fallback (theexcept Exceptionbranch at _tools.py line 471-472), e.g., by using a function with an unresolvable forward reference annotation and confirming it still produces a schema (falling back to raw annotations). - Consider adding a test using
typing.Optional[SomeCustomClass]syntax in addition toint | None, sinceOptional[CustomType]was the exact pattern that triggered the original PydanticUserError in the bug report.
Automated review by giles17's agents
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
Fixes a Python tooling bug where @tool input-model generation fails under from __future__ import annotations because stringified annotations were being forwarded into Pydantic model creation.
Changes:
- Resolve tool function annotations via
typing.get_type_hints(..., include_extras=True)before building the Pydanticcreate_modelfield definitions. - Add regression tests covering
FunctionInvocationContextexclusion from schema andOptional/union types under PEP 563 string annotations. - Add an end-to-end invocation test ensuring
FunctionInvocationContextinjection still works with future annotations enabled.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
python/packages/core/agent_framework/_tools.py |
Updates _resolve_input_model to use resolved type hints (instead of raw inspect.signature annotations) when constructing the input model for @tool. |
python/packages/core/tests/core/test_tools_future_annotations.py |
Adds regression tests validating schema generation and invocation behavior with from __future__ import annotations. |
python/packages/core/tests/core/test_tools_future_annotations.py
Outdated
Show resolved
Hide resolved
…icrosoft#4809) - Verify type correctness in schema assertions (not just key presence) - Fix ctx annotation to FunctionInvocationContext | None for type consistency - Add test for Optional[CustomType] pattern (original bug trigger) - Add test for get_type_hints() fallback with unresolvable forward refs Co-authored-by: Copilot <[email protected]>
…hemaGenerationError in FunctionInvocationContext
giles17
left a comment
giles17
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 96% | Result: All clear
Reviewed: Correctness, Security Reliability, Test Coverage, Design Approach
Automated review by giles17's agents
Motivation and Context
When
from __future__ import annotations(PEP 563) is active, all annotations become strings._resolve_input_modelpassed these raw string annotations directly to Pydantic'screate_model, causingPydanticSchemaGenerationErrorfor complex types likeOptional[int]andFunctionInvocationContext.Fixes #4809
Description
The root cause was that
_resolve_input_modelin_tools.pyusedparam.annotationdirectly frominspect.signature(), which under PEP 563 returns string representations instead of actual types. The fix mirrors the approach already used in_discover_injected_parametersby callingtyping.get_type_hints()to resolve stringified annotations back to real types before passing them to Pydantic'screate_model. Regression tests verify that@toolworks correctly with PEP 563 forFunctionInvocationContextparameters,Optionaltypes, and full invocation scenarios.Contribution Checklist