From e5a33e3fc71e86a6a2866a6e512a745c3b21ffb4 Mon Sep 17 00:00:00 2001 From: Yasser-Ameur Date: Sat, 26 Sep 2026 15:56:55 +0200 Subject: [PATCH 1/5] Python: Fix as_agent_framework_tool dropping parameter defaults The field-building loop in KernelFunction.as_agent_framework_tool assigned a Field with a default for parameters that have one, then unconditionally overwrote that same dict entry with a no-default Field on the next line. Every optional kernel function parameter therefore became required in the generated agent-framework input model. Branch on param.is_required instead of param.default_value is not None: a parameter such as `tag: str | None = None` is not required but has an actual default of None, which the None check could not tell apart from having no default at all. When a parameter is not required and its default is None, the annotation is widened to accept None too, otherwise pydantic rejects both the None default and an explicit None passed by the caller. Fixes #14481 --- .../functions/kernel_function.py | 14 +++- .../test_kernel_function_from_method.py | 72 +++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/python/semantic_kernel/functions/kernel_function.py b/python/semantic_kernel/functions/kernel_function.py index 15f2ab34c6b7..e75422f6c2e1 100644 --- a/python/semantic_kernel/functions/kernel_function.py +++ b/python/semantic_kernel/functions/kernel_function.py @@ -447,12 +447,20 @@ def as_agent_framework_tool( fields = {} for param in self.parameters: if param.include_in_function_choices: - if param.default_value is not None: + if param.is_required: + fields[param.name] = (param.type_, Field(description=param.description)) + else: + # A parameter that isn't required may still carry an actual default of None + # (e.g. `tag: str | None = None`), which is different from having no default + # at all. The annotation needs to accept None in that case, or pydantic + # rejects the None default and any explicit None passed by the caller. + annotation = param.type_ + if param.default_value is None: + annotation = f"{param.type_} | None" fields[param.name] = ( - param.type_, + annotation, Field(description=param.description, default=param.default_value), ) - fields[param.name] = (param.type_, Field(description=param.description)) input_model = create_model("InputModel", **fields) # type: ignore async def wrapper(*args: Any, **kwargs: Any) -> Any: diff --git a/python/tests/unit/functions/test_kernel_function_from_method.py b/python/tests/unit/functions/test_kernel_function_from_method.py index 3b868625d87c..9461fcbf0788 100644 --- a/python/tests/unit/functions/test_kernel_function_from_method.py +++ b/python/tests/unit/functions/test_kernel_function_from_method.py @@ -1,4 +1,6 @@ # Copyright (c) Microsoft. All rights reserved. +import sys +import types from collections.abc import AsyncGenerator, Iterable from typing import Annotated, Any from unittest.mock import Mock @@ -566,3 +568,73 @@ def test_function_model_dump_json(get_custom_type_function_pydantic): model_dump = func.model_dump_json() assert isinstance(model_dump, str) assert "metadata" in model_dump + + +@pytest.fixture +def stub_agent_framework(monkeypatch): + """Stub the optional `agent_framework` dependency so as_agent_framework_tool can be exercised.""" + + class AIFunction: + def __init__(self, *, name, description, input_model, func): + self.name = name + self.description = description + self.input_model = input_model + self.func = func + + module = types.ModuleType("agent_framework") + module.AIFunction = AIFunction + monkeypatch.setitem(sys.modules, "agent_framework", module) + + +def test_as_agent_framework_tool_keeps_defaults_and_required(stub_agent_framework): + @kernel_function(name="search", description="search things") + def search( + query: Annotated[str, "search query"], + top_k: Annotated[int, "number of results"] = 5, + include_meta: Annotated[bool, "include metadata"] = False, + offset: Annotated[int, "results to skip"] = 0, + label: Annotated[str, "label"] = "", + ) -> str: + return f"{query}:{top_k}:{include_meta}:{offset}:{label}" + + func = KernelFunction.from_method(search, "test") + tool = func.as_agent_framework_tool() + schema = tool.input_model.model_json_schema() + + # Only the parameter without a default should be required. + assert schema.get("required") == ["query"] + # Falsy defaults (0, False, "") must survive, not be dropped as if they were None. + assert schema["properties"]["top_k"]["default"] == 5 + assert schema["properties"]["include_meta"]["default"] is False + assert schema["properties"]["offset"]["default"] == 0 + assert schema["properties"]["label"]["default"] == "" + + +def test_as_agent_framework_tool_none_defaults_accept_none(stub_agent_framework): + @kernel_function(name="configure", description="configure things") + def configure( + q: Annotated[str, "required query"], + tag: Annotated[str | None, "tag"] = None, + n: Annotated[int | None, "n"] = None, + ) -> str: + return f"{q}:{tag}:{n}" + + func = KernelFunction.from_method(configure, "test") + tool = func.as_agent_framework_tool() + schema = tool.input_model.model_json_schema() + + # Only the parameter without any default is required; a None default still + # makes a parameter optional, it must not be conflated with "no default". + assert schema.get("required") == ["q"] + assert schema["properties"]["tag"]["default"] is None + assert schema["properties"]["n"]["default"] is None + + # The model must accept both an omitted value and an explicit None for the + # None-defaulted parameters. + instance = tool.input_model(q="x") + assert instance.tag is None + assert instance.n is None + + instance_explicit = tool.input_model(q="x", tag=None, n=None) + assert instance_explicit.tag is None + assert instance_explicit.n is None From 64f3daa6ef9da5b0f8c9e9452c79ddb8fe619a0a Mon Sep 17 00:00:00 2001 From: Yasser-Ameur Date: Sat, 26 Sep 2026 18:07:39 +0200 Subject: [PATCH 2/5] Python: Address Copilot review on as_agent_framework_tool defaults Build the annotation from param.type_object when it is available, instead of always going through the type_ string. type_ collapses a multi-type union such as str | int | None into the comma-joined string str, int, and appending None to that produces an invalid forward reference. Falling back to the type_ string only when type_object is None, and skipping the None suffix when that string contains a comma, fixes list[str] | None and a dataclass or BaseModel | None while leaving the pre-existing str | int | None gap exactly as broken as before, which a new test pins. Also add a test pinning a separate, pre-existing gap noticed during review: a bare x: str | None with no explicit default is marked is_required=False purely because its annotation contains NoneType, but invoking the function with that argument omitted still raises a raw TypeError from the underlying Python call, since the method itself has no default for x. Fixing that needs a way to tell no default in the signature apart from annotation includes None, which is a larger change than this PR's scope. --- .../functions/kernel_function.py | 19 ++++- .../test_kernel_function_from_method.py | 78 +++++++++++++++++++ 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/python/semantic_kernel/functions/kernel_function.py b/python/semantic_kernel/functions/kernel_function.py index e75422f6c2e1..06dfa0c0c03a 100644 --- a/python/semantic_kernel/functions/kernel_function.py +++ b/python/semantic_kernel/functions/kernel_function.py @@ -454,9 +454,22 @@ def as_agent_framework_tool( # (e.g. `tag: str | None = None`), which is different from having no default # at all. The annotation needs to accept None in that case, or pydantic # rejects the None default and any explicit None passed by the caller. - annotation = param.type_ - if param.default_value is None: - annotation = f"{param.type_} | None" + if param.type_object is not None: + # Prefer the real type object when we have one: it is unambiguous, + # unlike the type_ string, which collapses a multi-type union such as + # `str | int | None` into "str, int" (no way to tell it apart from a + # literal two-argument type). + annotation: Any = param.type_object + if param.default_value is None: + annotation = param.type_object | None + else: + annotation = param.type_ + # type_ is a comma-joined string for a multi-type union (e.g. "str, int") + # when no single non-None type could be resolved; appending "| None" to + # that would produce an invalid forward reference, so it is left alone, + # same as it already behaves without this fix. + if param.default_value is None and param.type_ and "," not in param.type_: + annotation = f"{param.type_} | None" fields[param.name] = ( annotation, Field(description=param.description, default=param.default_value), diff --git a/python/tests/unit/functions/test_kernel_function_from_method.py b/python/tests/unit/functions/test_kernel_function_from_method.py index 9461fcbf0788..34fc901cb804 100644 --- a/python/tests/unit/functions/test_kernel_function_from_method.py +++ b/python/tests/unit/functions/test_kernel_function_from_method.py @@ -2,6 +2,7 @@ import sys import types from collections.abc import AsyncGenerator, Iterable +from dataclasses import dataclass from typing import Annotated, Any from unittest.mock import Mock @@ -259,6 +260,30 @@ def my_function(input: str) -> str: await func.invoke(kernel=kernel, arguments=KernelArguments()) +async def test_optional_annotation_without_default_raises_typeerror_on_invoke(kernel: Kernel): + """Pins a pre-existing gap, unrelated to as_agent_framework_tool, found while reviewing it. + + `_parse_parameter` marks `is_required=False` for any annotation containing NoneType, + including a bare `x: str | None` with no `= None` in the signature. `gather_function_parameters` + then treats a missing argument for a non-required parameter as fine to just skip, rather than + supplying `None`. But the underlying Python function still has no default for `x`, so calling + it without `x` raises a raw TypeError, not the FunctionExecutionException `is_required` implies. + Fixing this needs a way to tell "no default in the signature" apart from "annotation includes + None", which _process_signature does not currently retain; that is a bigger change than this + PR's scope, so this only pins the current behaviour. + """ + + @kernel_function() + def my_function(x: str | None) -> str: + return str(x) + + func = KernelFunction.from_method(my_function, "test") + assert func.parameters[0].is_required is False + + with pytest.raises(TypeError, match="missing 1 required positional argument"): + await func.invoke(kernel=kernel, arguments=KernelArguments()) + + async def test_service_execution_with_complex_object(kernel: Kernel): class InputObject(KernelBaseModel): arg1: str @@ -638,3 +663,56 @@ def configure( instance_explicit = tool.input_model(q="x", tag=None, n=None) assert instance_explicit.tag is None assert instance_explicit.n is None + + +@dataclass +class _ProbeCustomType: + id: str = "" + + +def test_as_agent_framework_tool_none_default_generic_and_model_types(stub_agent_framework): + """A None-defaulted `list[str]` or dataclass/BaseModel parameter has a real + `type_object`, unlike a plain string, so the annotation can be built from it + directly (`param.type_object | None`) instead of the type_ string. + """ + + @kernel_function(name="configure", description="configure things") + def configure( + q: Annotated[str, "required query"], + b: Annotated[list[str] | None, "b"] = None, + c: Annotated[_ProbeCustomType | None, "c"] = None, + ) -> str: + return f"{q}:{b}:{c}" + + func = KernelFunction.from_method(configure, "test") + tool = func.as_agent_framework_tool() + schema = tool.input_model.model_json_schema() + + assert schema.get("required") == ["q"] + assert schema["properties"]["b"]["default"] is None + assert schema["properties"]["c"]["default"] is None + + instance = tool.input_model(q="x") + assert instance.b is None + assert instance.c is None + + instance_explicit = tool.input_model(q="x", b=["y"], c=_ProbeCustomType(id="z")) + assert instance_explicit.b == ["y"] + assert instance_explicit.c == _ProbeCustomType(id="z") + + +def test_as_agent_framework_tool_multi_type_union_stays_broken(stub_agent_framework): + """`str | int | None` has no single non-None type, so `type_object` is None and the + loop falls back to the comma-joined type_ string "str, int". Appending "| None" to + that would only make it worse (an invalid forward reference), so this case is left + exactly as broken as it already was before this fix: this pins that it is a + pre-existing gap, not something this change regresses. + """ + + @kernel_function(name="probe", description="probe things") + def probe(q: Annotated[str, "required"], a: Annotated[str | int | None, "a"] = None) -> str: + return f"{q}:{a}" + + func = KernelFunction.from_method(probe, "test") + with pytest.raises(TypeError, match="Forward references must evaluate to types"): + func.as_agent_framework_tool() From 3b54a67937c4bd9e3e4d9183d785839e9d3d3762 Mon Sep 17 00:00:00 2001 From: Yasser-Ameur Date: Sat, 26 Sep 2026 18:30:32 +0200 Subject: [PATCH 3/5] Python: Fix as_agent_framework_tool regression on Optional without a default Before this branch, every parameter of as_agent_framework_tool's input model was required, so the caller always had to supply x for a signature like x: str | None. This branch made non-required parameters optional based on param.is_required alone, which the decorator sets to False for any annotation containing None, whether or not the signature actually has a default. That turned a bare x: str | None into an optional field, and gather_function_parameters then omits an argument nobody supplied for a non-required parameter, so the underlying call raised a raw TypeError instead of failing input validation the way a missing required field should. That is a regression this branch introduced for the bridge, not a pre-existing one worth only pinning. Recover the missing-default distinction from the real Python signature when one is available: as_agent_framework_tool now reads inspect.signature(self.method) once (guarded with getattr, since a prompt-based function has no method and keeps the existing is_required path), and treats a parameter as required when param.is_required is True or the corresponding signature parameter's default is inspect.Parameter.empty. Replace the invoke-time pinning test with one asserting the bridge schema directly: x: str | None without a default stays in the schema's required list and fails validation when omitted, while y: str | None = None does not. --- .../functions/kernel_function.py | 18 +++++- .../test_kernel_function_from_method.py | 55 +++++++++++-------- 2 files changed, 48 insertions(+), 25 deletions(-) diff --git a/python/semantic_kernel/functions/kernel_function.py b/python/semantic_kernel/functions/kernel_function.py index 06dfa0c0c03a..561a72018d44 100644 --- a/python/semantic_kernel/functions/kernel_function.py +++ b/python/semantic_kernel/functions/kernel_function.py @@ -426,6 +426,7 @@ def as_agent_framework_tool( Returns: AIFunction: The agent framework tool. """ + import inspect import json from pydantic import Field, create_model @@ -444,10 +445,25 @@ def as_agent_framework_tool( kernel = Kernel() name = name or self.name description = description or self.description + + # `param.is_required` is False for any annotation that contains None (e.g. both + # `x: str | None` and `x: str | None = None`), even though only the second one + # actually has a default. Without correcting for that, a parameter that Python + # itself still requires would be built as optional here, and invoking the tool + # with it omitted would raise a raw TypeError from the underlying call instead of + # failing input validation the way a missing required field should. When the + # function has a real Python callable, recover the distinction from its signature. + method = getattr(self, "method", None) + signature = inspect.signature(method) if method is not None else None + fields = {} for param in self.parameters: if param.include_in_function_choices: - if param.is_required: + sig_param = signature.parameters.get(param.name) if signature is not None and param.name else None + is_required = param.is_required or ( + sig_param is not None and sig_param.default is inspect.Parameter.empty + ) + if is_required: fields[param.name] = (param.type_, Field(description=param.description)) else: # A parameter that isn't required may still carry an actual default of None diff --git a/python/tests/unit/functions/test_kernel_function_from_method.py b/python/tests/unit/functions/test_kernel_function_from_method.py index 34fc901cb804..0eaec5161edd 100644 --- a/python/tests/unit/functions/test_kernel_function_from_method.py +++ b/python/tests/unit/functions/test_kernel_function_from_method.py @@ -7,6 +7,7 @@ from unittest.mock import Mock import pytest +from pydantic import ValidationError from semantic_kernel.connectors.ai.open_ai.services.open_ai_chat_completion import OpenAIChatCompletion from semantic_kernel.exceptions import FunctionExecutionException, FunctionInitializationError @@ -260,30 +261,6 @@ def my_function(input: str) -> str: await func.invoke(kernel=kernel, arguments=KernelArguments()) -async def test_optional_annotation_without_default_raises_typeerror_on_invoke(kernel: Kernel): - """Pins a pre-existing gap, unrelated to as_agent_framework_tool, found while reviewing it. - - `_parse_parameter` marks `is_required=False` for any annotation containing NoneType, - including a bare `x: str | None` with no `= None` in the signature. `gather_function_parameters` - then treats a missing argument for a non-required parameter as fine to just skip, rather than - supplying `None`. But the underlying Python function still has no default for `x`, so calling - it without `x` raises a raw TypeError, not the FunctionExecutionException `is_required` implies. - Fixing this needs a way to tell "no default in the signature" apart from "annotation includes - None", which _process_signature does not currently retain; that is a bigger change than this - PR's scope, so this only pins the current behaviour. - """ - - @kernel_function() - def my_function(x: str | None) -> str: - return str(x) - - func = KernelFunction.from_method(my_function, "test") - assert func.parameters[0].is_required is False - - with pytest.raises(TypeError, match="missing 1 required positional argument"): - await func.invoke(kernel=kernel, arguments=KernelArguments()) - - async def test_service_execution_with_complex_object(kernel: Kernel): class InputObject(KernelBaseModel): arg1: str @@ -716,3 +693,33 @@ def probe(q: Annotated[str, "required"], a: Annotated[str | int | None, "a"] = N func = KernelFunction.from_method(probe, "test") with pytest.raises(TypeError, match="Forward references must evaluate to types"): func.as_agent_framework_tool() + + +def test_as_agent_framework_tool_optional_annotation_without_default_stays_required(stub_agent_framework): + """`x: str | None` (no default) still makes `is_required=False` in the decorator's own + metadata, because that only checks whether the annotation contains None, not whether the + Python signature has a default. Python itself still requires `x`, so the bridge recovers + that from the real callable's signature and keeps `x` required, while `y: str | None = None`, + which genuinely has a default, stays optional. + """ + + @kernel_function(name="probe", description="probe things") + def probe(x: Annotated[str | None, "x, no default"], y: Annotated[str | None, "y, defaulted"] = None) -> str: + return f"{x}:{y}" + + func = KernelFunction.from_method(probe, "test") + assert func.parameters[0].is_required is False # the decorator's own metadata, unchanged + + tool = func.as_agent_framework_tool() + schema = tool.input_model.model_json_schema() + + assert schema.get("required") == ["x"] + assert "y" not in (schema.get("required") or []) + assert schema["properties"]["y"]["default"] is None + + with pytest.raises(ValidationError): + tool.input_model() + + instance = tool.input_model(x="hi") + assert instance.x == "hi" + assert instance.y is None From df5158d3d048f4d853d3ce9b434b9504689cf2de Mon Sep 17 00:00:00 2001 From: Yasser-Ameur Date: Sat, 26 Sep 2026 19:16:02 +0200 Subject: [PATCH 4/5] Python: Fix two more as_agent_framework_tool gaps from review Delete test_as_agent_framework_tool_multi_type_union_stays_broken. On Python 3.10, the declared floor, pydantic raises a different message for the same comma-joined type_ string than on 3.12, so the test's regex is not portable, and pinning a known bug through a third-party error message is fragile regardless. The source comment on the comma form already documents the gap. Fix a crash for any unannotated parameter with a default, such as def f(x=None, y=3): _process_signature stores inspect.Parameter.empty as type_object in that case, and pydantic cannot build a schema from that value. Treat type_object is inspect.Parameter.empty the same as type_object is None, falling back to the type_ string ("Any"). Added y=3 to the first test to cover it: its default and non-required status are now asserted. Also: when a parameter is required only because the signature has no default while its annotation includes None (x: str | None with no default), annotate it with type_object | None instead of the bare type_object, so an explicit None still validates and the schema advertises it. Asserted in the optional-annotation test that input_model(x=None) validates. --- .../functions/kernel_function.py | 24 +++++++++++---- .../test_kernel_function_from_method.py | 30 ++++++++----------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/python/semantic_kernel/functions/kernel_function.py b/python/semantic_kernel/functions/kernel_function.py index 561a72018d44..88286d910fd6 100644 --- a/python/semantic_kernel/functions/kernel_function.py +++ b/python/semantic_kernel/functions/kernel_function.py @@ -463,27 +463,39 @@ def as_agent_framework_tool( is_required = param.is_required or ( sig_param is not None and sig_param.default is inspect.Parameter.empty ) + # _process_signature stores inspect.Parameter.empty as type_object for an + # unannotated parameter, and pydantic cannot build a schema from that, so it + # counts the same as "no usable type object" alongside None. + type_object = param.type_object + if type_object is inspect.Parameter.empty: + type_object = None + annotation: Any = param.type_ if is_required: - fields[param.name] = (param.type_, Field(description=param.description)) + # This parameter is only required because the signature has no default; + # the decorator's own is_required is False, meaning the annotation itself + # includes None (e.g. `x: str | None` with no default). An explicit None + # should still validate, and the schema should say so. + if not param.is_required and type_object is not None: + annotation = type_object | None + fields[param.name] = (annotation, Field(description=param.description)) else: # A parameter that isn't required may still carry an actual default of None # (e.g. `tag: str | None = None`), which is different from having no default # at all. The annotation needs to accept None in that case, or pydantic # rejects the None default and any explicit None passed by the caller. - if param.type_object is not None: + if type_object is not None: # Prefer the real type object when we have one: it is unambiguous, # unlike the type_ string, which collapses a multi-type union such as # `str | int | None` into "str, int" (no way to tell it apart from a # literal two-argument type). - annotation: Any = param.type_object + annotation = type_object if param.default_value is None: - annotation = param.type_object | None + annotation = type_object | None else: annotation = param.type_ # type_ is a comma-joined string for a multi-type union (e.g. "str, int") # when no single non-None type could be resolved; appending "| None" to - # that would produce an invalid forward reference, so it is left alone, - # same as it already behaves without this fix. + # that would produce an invalid forward reference, so it is left alone. if param.default_value is None and param.type_ and "," not in param.type_: annotation = f"{param.type_} | None" fields[param.name] = ( diff --git a/python/tests/unit/functions/test_kernel_function_from_method.py b/python/tests/unit/functions/test_kernel_function_from_method.py index 0eaec5161edd..3883a7dc2bab 100644 --- a/python/tests/unit/functions/test_kernel_function_from_method.py +++ b/python/tests/unit/functions/test_kernel_function_from_method.py @@ -596,8 +596,9 @@ def search( include_meta: Annotated[bool, "include metadata"] = False, offset: Annotated[int, "results to skip"] = 0, label: Annotated[str, "label"] = "", + y=3, # unannotated parameter with a default: type_object is inspect.Parameter.empty ) -> str: - return f"{query}:{top_k}:{include_meta}:{offset}:{label}" + return f"{query}:{top_k}:{include_meta}:{offset}:{label}:{y}" func = KernelFunction.from_method(search, "test") tool = func.as_agent_framework_tool() @@ -610,6 +611,11 @@ def search( assert schema["properties"]["include_meta"]["default"] is False assert schema["properties"]["offset"]["default"] == 0 assert schema["properties"]["label"]["default"] == "" + # An unannotated parameter with a default must not crash tool creation (its + # type_object is inspect.Parameter.empty, which pydantic cannot build a schema + # from) and must fall back to being optional with its default preserved. + assert schema["properties"]["y"]["default"] == 3 + assert "y" not in schema.get("required", []) def test_as_agent_framework_tool_none_defaults_accept_none(stub_agent_framework): @@ -678,23 +684,6 @@ def configure( assert instance_explicit.c == _ProbeCustomType(id="z") -def test_as_agent_framework_tool_multi_type_union_stays_broken(stub_agent_framework): - """`str | int | None` has no single non-None type, so `type_object` is None and the - loop falls back to the comma-joined type_ string "str, int". Appending "| None" to - that would only make it worse (an invalid forward reference), so this case is left - exactly as broken as it already was before this fix: this pins that it is a - pre-existing gap, not something this change regresses. - """ - - @kernel_function(name="probe", description="probe things") - def probe(q: Annotated[str, "required"], a: Annotated[str | int | None, "a"] = None) -> str: - return f"{q}:{a}" - - func = KernelFunction.from_method(probe, "test") - with pytest.raises(TypeError, match="Forward references must evaluate to types"): - func.as_agent_framework_tool() - - def test_as_agent_framework_tool_optional_annotation_without_default_stays_required(stub_agent_framework): """`x: str | None` (no default) still makes `is_required=False` in the decorator's own metadata, because that only checks whether the annotation contains None, not whether the @@ -723,3 +712,8 @@ def probe(x: Annotated[str | None, "x, no default"], y: Annotated[str | None, "y instance = tool.input_model(x="hi") assert instance.x == "hi" assert instance.y is None + + # x is required (no default in the signature), but its annotation still includes + # None, so an explicit null must validate rather than be rejected as the wrong type. + instance_explicit_none = tool.input_model(x=None) + assert instance_explicit_none.x is None From e696c1a2f429556e9049573bae0a5f63aaa38ed7 Mon Sep 17 00:00:00 2001 From: Yasser-Ameur Date: Sat, 26 Sep 2026 19:39:09 +0200 Subject: [PATCH 5/5] Python: tighten comments and test naming in as_agent_framework_tool --- python/semantic_kernel/functions/kernel_function.py | 13 +++++-------- .../functions/test_kernel_function_from_method.py | 10 +++++----- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/python/semantic_kernel/functions/kernel_function.py b/python/semantic_kernel/functions/kernel_function.py index 88286d910fd6..a6cef14a06f6 100644 --- a/python/semantic_kernel/functions/kernel_function.py +++ b/python/semantic_kernel/functions/kernel_function.py @@ -471,11 +471,10 @@ def as_agent_framework_tool( type_object = None annotation: Any = param.type_ if is_required: - # This parameter is only required because the signature has no default; - # the decorator's own is_required is False, meaning the annotation itself - # includes None (e.g. `x: str | None` with no default). An explicit None - # should still validate, and the schema should say so. if not param.is_required and type_object is not None: + # Only the signature makes it required: the annotation itself includes + # None (e.g. `x: str | None` with no default), so an explicit None + # should still validate, and the schema should say so. annotation = type_object | None fields[param.name] = (annotation, Field(description=param.description)) else: @@ -485,14 +484,12 @@ def as_agent_framework_tool( # rejects the None default and any explicit None passed by the caller. if type_object is not None: # Prefer the real type object when we have one: it is unambiguous, - # unlike the type_ string, which collapses a multi-type union such as - # `str | int | None` into "str, int" (no way to tell it apart from a - # literal two-argument type). + # unlike the type_ string, which turns a multi-type union such as + # `str | int | None` into "str, int", a string pydantic cannot evaluate. annotation = type_object if param.default_value is None: annotation = type_object | None else: - annotation = param.type_ # type_ is a comma-joined string for a multi-type union (e.g. "str, int") # when no single non-None type could be resolved; appending "| None" to # that would produce an invalid forward reference, so it is left alone. diff --git a/python/tests/unit/functions/test_kernel_function_from_method.py b/python/tests/unit/functions/test_kernel_function_from_method.py index 3883a7dc2bab..8b2f2af2f2a9 100644 --- a/python/tests/unit/functions/test_kernel_function_from_method.py +++ b/python/tests/unit/functions/test_kernel_function_from_method.py @@ -649,12 +649,12 @@ def configure( @dataclass -class _ProbeCustomType: +class _CustomType: id: str = "" def test_as_agent_framework_tool_none_default_generic_and_model_types(stub_agent_framework): - """A None-defaulted `list[str]` or dataclass/BaseModel parameter has a real + """A None-defaulted `list[str]` or dataclass parameter has a real `type_object`, unlike a plain string, so the annotation can be built from it directly (`param.type_object | None`) instead of the type_ string. """ @@ -663,7 +663,7 @@ def test_as_agent_framework_tool_none_default_generic_and_model_types(stub_agent def configure( q: Annotated[str, "required query"], b: Annotated[list[str] | None, "b"] = None, - c: Annotated[_ProbeCustomType | None, "c"] = None, + c: Annotated[_CustomType | None, "c"] = None, ) -> str: return f"{q}:{b}:{c}" @@ -679,9 +679,9 @@ def configure( assert instance.b is None assert instance.c is None - instance_explicit = tool.input_model(q="x", b=["y"], c=_ProbeCustomType(id="z")) + instance_explicit = tool.input_model(q="x", b=["y"], c=_CustomType(id="z")) assert instance_explicit.b == ["y"] - assert instance_explicit.c == _ProbeCustomType(id="z") + assert instance_explicit.c == _CustomType(id="z") def test_as_agent_framework_tool_optional_annotation_without_default_stays_required(stub_agent_framework):