From 172f0c199ea611ba0150132ad731eb5fb5ab52c8 Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Thu, 6 Aug 2026 09:24:48 +0100 Subject: [PATCH 1/3] fix(backends): resolve postponed annotations in ollama tool schema convert_function_to_ollama_tool read raw inspect.signature() annotations, which are unresolved strings under `from __future__ import annotations` (PEP 563). Pydantic could not build a schema for any non-builtin parameter type, raising PydanticUserError. Resolve with eval_str=True, matching the fix already applied to genstub.py for the same root cause. Assisted-by: Claude Code Signed-off-by: Nigel Jones --- mellea/backends/tools.py | 9 ++++-- test/backends/_pep563_samples.py | 28 +++++++++++++++++++ .../test_discriminated_union_tools.py | 23 +++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 test/backends/_pep563_samples.py diff --git a/mellea/backends/tools.py b/mellea/backends/tools.py index 12b02493c..b29c31a38 100644 --- a/mellea/backends/tools.py +++ b/mellea/backends/tools.py @@ -1344,15 +1344,20 @@ def convert_function_to_ollama_tool( """ doc_string_hash = str(hash(inspect.getdoc(func))) parsed_docstring = _parse_docstring(inspect.getdoc(func)) + # eval_str=True resolves PEP 563 postponed (string) annotations back to + # real type objects; without it, `from __future__ import annotations` in + # the tool's module leaves Pydantic unable to build the schema for + # non-builtin parameter types. + sig = inspect.signature(func, eval_str=True) schema = type( func.__name__, (BaseModel,), { "__annotations__": { k: v.annotation if v.annotation != inspect._empty else str - for k, v in inspect.signature(func).parameters.items() + for k, v in sig.parameters.items() }, - "__signature__": inspect.signature(func), + "__signature__": sig, "__doc__": parsed_docstring[doc_string_hash], }, ).model_json_schema() # type: ignore diff --git a/test/backends/_pep563_samples.py b/test/backends/_pep563_samples.py new file mode 100644 index 000000000..8c4dba1a8 --- /dev/null +++ b/test/backends/_pep563_samples.py @@ -0,0 +1,28 @@ +# Copyright IBM Corp. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Sample functions with postponed annotations (PEP 563), for regression tests. + +Isolated in its own module because `from __future__ import annotations` is a +module-level directive — it cannot be scoped to a single test function. +""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass +class Address: + """A custom, non-builtin parameter type.""" + + city: str + + +def send_letter(to: Address) -> str: + """Send a letter to the given address. + + Args: + to: the destination address + """ + return "sent" diff --git a/test/backends/test_discriminated_union_tools.py b/test/backends/test_discriminated_union_tools.py index 88fd28abe..c469db59c 100644 --- a/test/backends/test_discriminated_union_tools.py +++ b/test/backends/test_discriminated_union_tools.py @@ -24,6 +24,7 @@ convert_function_to_ollama_tool, validate_tool_arguments, ) +from test.backends._pep563_samples import Address, send_letter class Cat(BaseModel): @@ -708,3 +709,25 @@ def deeply_nested( if __name__ == "__main__": pytest.main([__file__, "-v"]) + + +def test_convert_function_to_ollama_tool_resolves_postponed_annotations(): + # Regression test: under `from __future__ import annotations`, a + # non-builtin parameter type's annotation is a string rather than the + # real type object, which Pydantic cannot resolve when building the + # dynamic schema model - raising PydanticUserError instead of producing + # a tool schema. + + # Guard the precondition: if the sample module ever drops its + # `from __future__ import annotations`, this test would otherwise keep + # passing without exercising postponed annotations at all. + assert send_letter.__annotations__["to"] == "Address" + + tool = convert_function_to_ollama_tool(send_letter) + assert tool.function is not None + assert tool.function.parameters is not None + + props = tool.function.parameters.model_dump(exclude_none=True)["properties"] + assert props["to"]["type"] == "object" + assert props["to"]["title"] == Address.__name__ + assert props["to"]["properties"]["city"]["type"] == "string" From 2a3db415c51331a7634a0c02a0da9fbbbf644cec Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Fri, 7 Aug 2026 13:02:54 +0100 Subject: [PATCH 2/3] fix(backends): resolve postponed annotations per-parameter to avoid return annotation regression Assisted-by: opencode Signed-off-by: Nigel Jones --- mellea/backends/tools.py | 31 +++++-- test/backends/_pep563_samples.py | 38 ++++++++ .../test_discriminated_union_tools.py | 23 ----- test/backends/test_pep563_tool_annotations.py | 86 +++++++++++++++++++ 4 files changed, 150 insertions(+), 28 deletions(-) create mode 100644 test/backends/test_pep563_tool_annotations.py diff --git a/mellea/backends/tools.py b/mellea/backends/tools.py index b29c31a38..48890ddd8 100644 --- a/mellea/backends/tools.py +++ b/mellea/backends/tools.py @@ -1344,11 +1344,32 @@ def convert_function_to_ollama_tool( """ doc_string_hash = str(hash(inspect.getdoc(func))) parsed_docstring = _parse_docstring(inspect.getdoc(func)) - # eval_str=True resolves PEP 563 postponed (string) annotations back to - # real type objects; without it, `from __future__ import annotations` in - # the tool's module leaves Pydantic unable to build the schema for - # non-builtin parameter types. - sig = inspect.signature(func, eval_str=True) + # Resolve postponed (string) parameter annotations back to real type + # objects so Pydantic can build schemas for non-builtin parameter types + # under `from __future__ import annotations` (PEP 563). Evaluate + # parameter annotations individually rather than using + # `eval_str=True` because the return annotation is never consumed by + # this schema — resolving it would fail for TYPE_CHECKING-only or + # forward-referenced return types where the pre-existing path + # succeeded. `func.__globals__` already carries `__builtins__`, so no + # defensive copy is needed. + try: + sig = inspect.signature(func, eval_str=True) + except Exception: + sig = inspect.signature(func) + g = getattr(func, "__globals__", {}) + params = [] + for p in sig.parameters.values(): + if isinstance(p.annotation, str): + try: + # ast.literal_eval cannot evaluate type expressions + # (e.g. `Decimal`, `Foo | None`); this mirrors what + # `inspect.signature(..., eval_str=True)` does internally. + p = p.replace(annotation=eval(p.annotation, g)) # noqa: S307 + except Exception: + pass # leave as string; Pydantic will report it + params.append(p) + sig = sig.replace(parameters=params) schema = type( func.__name__, (BaseModel,), diff --git a/test/backends/_pep563_samples.py b/test/backends/_pep563_samples.py index 8c4dba1a8..431e998af 100644 --- a/test/backends/_pep563_samples.py +++ b/test/backends/_pep563_samples.py @@ -10,6 +10,10 @@ from __future__ import annotations from dataclasses import dataclass +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from decimal import Decimal @dataclass @@ -19,6 +23,13 @@ class Address: city: str +@dataclass +class Period: + """A custom parameter type for testing.""" + + name: str + + def send_letter(to: Address) -> str: """Send a letter to the given address. @@ -26,3 +37,30 @@ def send_letter(to: Address) -> str: to: the destination address """ return "sent" + + +def tc_only_return_builtin_param(query: str) -> Decimal: + """TYPE_CHECKING-only return with builtin params. + + Args: + query: query string + """ + return Decimal("0") + + +def tc_return_custom_param(period: Period) -> Decimal: + """TYPE_CHECKING-only return with custom param. + + Args: + period: the period + """ + return Decimal("0") + + +def unresolvable_param(query: NonExistentType) -> str: # type: ignore[name-defined] # noqa: F821 + """Unresolvable parameter annotation. + + Args: + query: query string + """ + return "ok" diff --git a/test/backends/test_discriminated_union_tools.py b/test/backends/test_discriminated_union_tools.py index c469db59c..88fd28abe 100644 --- a/test/backends/test_discriminated_union_tools.py +++ b/test/backends/test_discriminated_union_tools.py @@ -24,7 +24,6 @@ convert_function_to_ollama_tool, validate_tool_arguments, ) -from test.backends._pep563_samples import Address, send_letter class Cat(BaseModel): @@ -709,25 +708,3 @@ def deeply_nested( if __name__ == "__main__": pytest.main([__file__, "-v"]) - - -def test_convert_function_to_ollama_tool_resolves_postponed_annotations(): - # Regression test: under `from __future__ import annotations`, a - # non-builtin parameter type's annotation is a string rather than the - # real type object, which Pydantic cannot resolve when building the - # dynamic schema model - raising PydanticUserError instead of producing - # a tool schema. - - # Guard the precondition: if the sample module ever drops its - # `from __future__ import annotations`, this test would otherwise keep - # passing without exercising postponed annotations at all. - assert send_letter.__annotations__["to"] == "Address" - - tool = convert_function_to_ollama_tool(send_letter) - assert tool.function is not None - assert tool.function.parameters is not None - - props = tool.function.parameters.model_dump(exclude_none=True)["properties"] - assert props["to"]["type"] == "object" - assert props["to"]["title"] == Address.__name__ - assert props["to"]["properties"]["city"]["type"] == "string" diff --git a/test/backends/test_pep563_tool_annotations.py b/test/backends/test_pep563_tool_annotations.py new file mode 100644 index 000000000..e3e1053bd --- /dev/null +++ b/test/backends/test_pep563_tool_annotations.py @@ -0,0 +1,86 @@ +# Copyright IBM Corp. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for postponed annotation (PEP 563) resolution in tool schemas. + +Covers `convert_function_to_ollama_tool`'s handling of `from __future__ +import annotations`: non-builtin parameter types must resolve to real type +objects for Pydantic schema building, while a return annotation that is +unresolvable at call time (e.g. `TYPE_CHECKING`-only imports) must not break +the conversion, since the return annotation is never consumed by the +produced schema. +""" + +import pydantic +import pytest + +from mellea.backends.tools import convert_function_to_ollama_tool +from test.backends._pep563_samples import ( + Address, + Period, + send_letter, + tc_only_return_builtin_param, + tc_return_custom_param, + unresolvable_param, +) + + +def test_convert_function_to_ollama_tool_resolves_postponed_annotations(): + # Regression test: under `from __future__ import annotations`, a + # non-builtin parameter type's annotation is a string rather than the + # real type object, which Pydantic cannot resolve when building the + # dynamic schema model - raising PydanticUserError instead of producing + # a tool schema. + + # Guard the precondition: if the sample module ever drops its + # `from __future__ import annotations`, this test would otherwise keep + # passing without exercising postponed annotations at all. + assert send_letter.__annotations__["to"] == "Address" + + tool = convert_function_to_ollama_tool(send_letter) + assert tool.function is not None + assert tool.function.parameters is not None + + props = tool.function.parameters.model_dump(exclude_none=True)["properties"] + assert props["to"]["type"] == "object" + assert props["to"]["title"] == Address.__name__ + assert props["to"]["properties"]["city"]["type"] == "string" + + +def test_convert_function_to_ollama_tool_tc_only_return(): + """TYPE_CHECKING-only return + builtin params must produce schema.""" + tool = convert_function_to_ollama_tool(tc_only_return_builtin_param) + assert tool.function is not None + assert tool.function.parameters is not None + props = tool.function.parameters.model_dump(exclude_none=True)["properties"] + assert "query" in props + + +def test_convert_function_to_ollama_tool_tc_return_custom_param(): + """TYPE_CHECKING return + custom param must resolve param. + + This is the case that separates the try/except-around-`eval_str=True` + fallback (which discards parameter resolution entirely on any + failure) from per-parameter resolution: the return annotation is + unresolvable, but the custom parameter type still must resolve. + """ + tool = convert_function_to_ollama_tool(tc_return_custom_param) + assert tool.function is not None + assert tool.function.parameters is not None + props = tool.function.parameters.model_dump(exclude_none=True)["properties"] + assert props["period"]["type"] == "object" + assert props["period"]["title"] == Period.__name__ + + +def test_convert_function_to_ollama_tool_unresolvable_param(): + """Genuinely unresolvable parameter must still raise PydanticUserError. + + Confirms the documented degradation: unresolvable param annotations + surface as `PydanticUserError`, not `NameError`. + """ + with pytest.raises(pydantic.PydanticUserError): + convert_function_to_ollama_tool(unresolvable_param) + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) From 14fbeff50a50bef474a2007c90c7fc06f7b55b50 Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Fri, 7 Aug 2026 16:26:18 +0100 Subject: [PATCH 3/3] test(backends): fold postponed-annotation tests into existing tool test module Addresses review feedback on #1509. The PEP 563 regression tests were moved into test_discriminated_union_tools.py, already the only module exercising convert_function_to_ollama_tool, rather than living in a second module for the same function. The previous change only renamed the standalone file instead of moving the tests, which did not address the review. Also renames the samples module to _postponed_annotation_samples.py, dropping the PEP number from the filename as requested. The module-level 'from __future__ import annotations' still requires the sample code to sit in its own module; only the tests move. Verified the moved tests are not vacuous: against the pre-fix baseline 2 of 4 fail, and against parameter-blind eval_str=True resolution 3 of 4 fail. Assisted-by: Claude Code Signed-off-by: Nigel Jones --- ...es.py => _postponed_annotation_samples.py} | 0 .../test_discriminated_union_tools.py | 74 +++++++++++++++- test/backends/test_pep563_tool_annotations.py | 86 ------------------- 3 files changed, 73 insertions(+), 87 deletions(-) rename test/backends/{_pep563_samples.py => _postponed_annotation_samples.py} (100%) delete mode 100644 test/backends/test_pep563_tool_annotations.py diff --git a/test/backends/_pep563_samples.py b/test/backends/_postponed_annotation_samples.py similarity index 100% rename from test/backends/_pep563_samples.py rename to test/backends/_postponed_annotation_samples.py diff --git a/test/backends/test_discriminated_union_tools.py b/test/backends/test_discriminated_union_tools.py index 88fd28abe..3f3a5a83e 100644 --- a/test/backends/test_discriminated_union_tools.py +++ b/test/backends/test_discriminated_union_tools.py @@ -11,19 +11,34 @@ be preserved and the OAS-3 `discriminator` keyword must be stripped from the output (the JSON Schema subset accepted by tool-calling APIs does not include it; the `Literal` tag fields carry the discriminator signal). + +Also covers `convert_function_to_ollama_tool`'s handling of postponed +annotations (`from __future__ import annotations`, PEP 563): non-builtin +parameter types must resolve to real type objects for Pydantic schema +building, while a return annotation that is unresolvable at call time (e.g. +`TYPE_CHECKING`-only imports) must not break the conversion, since the +produced schema never consumes it. """ import json from typing import Annotated, Literal import pytest -from pydantic import BaseModel, Field, ValidationError +from pydantic import BaseModel, Field, PydanticUserError, ValidationError from mellea.backends.tools import ( MelleaTool, convert_function_to_ollama_tool, validate_tool_arguments, ) +from test.backends._postponed_annotation_samples import ( + Address, + Period, + send_letter, + tc_only_return_builtin_param, + tc_return_custom_param, + unresolvable_param, +) class Cat(BaseModel): @@ -706,5 +721,62 @@ def deeply_nested( ) +class TestPostponedAnnotations: + """Postponed annotation (PEP 563) resolution in generated tool schemas.""" + + def test_resolves_postponed_parameter_annotation(self): + # Regression test: under `from __future__ import annotations`, a + # non-builtin parameter type's annotation is a string rather than the + # real type object, which Pydantic cannot resolve when building the + # dynamic schema model - raising PydanticUserError instead of producing + # a tool schema. + + # Guard the precondition: if the sample module ever drops its + # `from __future__ import annotations`, this test would otherwise keep + # passing without exercising postponed annotations at all. + assert send_letter.__annotations__["to"] == "Address" + + tool = convert_function_to_ollama_tool(send_letter) + assert tool.function is not None + assert tool.function.parameters is not None + + props = tool.function.parameters.model_dump(exclude_none=True)["properties"] + assert props["to"]["type"] == "object" + assert props["to"]["title"] == Address.__name__ + assert props["to"]["properties"]["city"]["type"] == "string" + + def test_type_checking_only_return_with_builtin_params(self): + """TYPE_CHECKING-only return + builtin params must produce schema.""" + tool = convert_function_to_ollama_tool(tc_only_return_builtin_param) + assert tool.function is not None + assert tool.function.parameters is not None + props = tool.function.parameters.model_dump(exclude_none=True)["properties"] + assert "query" in props + + def test_type_checking_only_return_with_custom_param(self): + """TYPE_CHECKING return + custom param must resolve param. + + This is the case that separates the try/except-around-`eval_str=True` + fallback (which discards parameter resolution entirely on any + failure) from per-parameter resolution: the return annotation is + unresolvable, but the custom parameter type still must resolve. + """ + tool = convert_function_to_ollama_tool(tc_return_custom_param) + assert tool.function is not None + assert tool.function.parameters is not None + props = tool.function.parameters.model_dump(exclude_none=True)["properties"] + assert props["period"]["type"] == "object" + assert props["period"]["title"] == Period.__name__ + + def test_unresolvable_parameter_annotation_raises(self): + """Genuinely unresolvable parameter must still raise PydanticUserError. + + Confirms the documented degradation: unresolvable param annotations + surface as `PydanticUserError`, not `NameError`. + """ + with pytest.raises(PydanticUserError): + convert_function_to_ollama_tool(unresolvable_param) + + if __name__ == "__main__": pytest.main([__file__, "-v"]) diff --git a/test/backends/test_pep563_tool_annotations.py b/test/backends/test_pep563_tool_annotations.py deleted file mode 100644 index e3e1053bd..000000000 --- a/test/backends/test_pep563_tool_annotations.py +++ /dev/null @@ -1,86 +0,0 @@ -# Copyright IBM Corp. All Rights Reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Tests for postponed annotation (PEP 563) resolution in tool schemas. - -Covers `convert_function_to_ollama_tool`'s handling of `from __future__ -import annotations`: non-builtin parameter types must resolve to real type -objects for Pydantic schema building, while a return annotation that is -unresolvable at call time (e.g. `TYPE_CHECKING`-only imports) must not break -the conversion, since the return annotation is never consumed by the -produced schema. -""" - -import pydantic -import pytest - -from mellea.backends.tools import convert_function_to_ollama_tool -from test.backends._pep563_samples import ( - Address, - Period, - send_letter, - tc_only_return_builtin_param, - tc_return_custom_param, - unresolvable_param, -) - - -def test_convert_function_to_ollama_tool_resolves_postponed_annotations(): - # Regression test: under `from __future__ import annotations`, a - # non-builtin parameter type's annotation is a string rather than the - # real type object, which Pydantic cannot resolve when building the - # dynamic schema model - raising PydanticUserError instead of producing - # a tool schema. - - # Guard the precondition: if the sample module ever drops its - # `from __future__ import annotations`, this test would otherwise keep - # passing without exercising postponed annotations at all. - assert send_letter.__annotations__["to"] == "Address" - - tool = convert_function_to_ollama_tool(send_letter) - assert tool.function is not None - assert tool.function.parameters is not None - - props = tool.function.parameters.model_dump(exclude_none=True)["properties"] - assert props["to"]["type"] == "object" - assert props["to"]["title"] == Address.__name__ - assert props["to"]["properties"]["city"]["type"] == "string" - - -def test_convert_function_to_ollama_tool_tc_only_return(): - """TYPE_CHECKING-only return + builtin params must produce schema.""" - tool = convert_function_to_ollama_tool(tc_only_return_builtin_param) - assert tool.function is not None - assert tool.function.parameters is not None - props = tool.function.parameters.model_dump(exclude_none=True)["properties"] - assert "query" in props - - -def test_convert_function_to_ollama_tool_tc_return_custom_param(): - """TYPE_CHECKING return + custom param must resolve param. - - This is the case that separates the try/except-around-`eval_str=True` - fallback (which discards parameter resolution entirely on any - failure) from per-parameter resolution: the return annotation is - unresolvable, but the custom parameter type still must resolve. - """ - tool = convert_function_to_ollama_tool(tc_return_custom_param) - assert tool.function is not None - assert tool.function.parameters is not None - props = tool.function.parameters.model_dump(exclude_none=True)["properties"] - assert props["period"]["type"] == "object" - assert props["period"]["title"] == Period.__name__ - - -def test_convert_function_to_ollama_tool_unresolvable_param(): - """Genuinely unresolvable parameter must still raise PydanticUserError. - - Confirms the documented degradation: unresolvable param annotations - surface as `PydanticUserError`, not `NameError`. - """ - with pytest.raises(pydantic.PydanticUserError): - convert_function_to_ollama_tool(unresolvable_param) - - -if __name__ == "__main__": - pytest.main([__file__, "-v"])