Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions src/agents/function_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from pydantic import BaseModel, Field, create_model
from pydantic.fields import FieldInfo

from .exceptions import UserError
from .exceptions import ModelBehaviorError, UserError
from .run_context import RunContextWrapper
from .strict_schema import ensure_strict_json_schema
from .tool_context import ToolContext
Expand Down Expand Up @@ -47,6 +47,11 @@ def to_call_args(self, data: BaseModel) -> tuple[list[Any], dict[str, Any]]:
"""
Converts validated data from the Pydantic model into (args, kwargs), suitable for calling
the original function.

Raises:
ModelBehaviorError: If the ``**kwargs`` payload carries a key that names one of the
function's own keyword-bindable parameters. The schema allows it, but no Python
call expresses it.
"""
positional_args: list[Any] = []
keyword_args: dict[str, Any] = {}
Expand All @@ -69,7 +74,9 @@ def to_call_args(self, data: BaseModel) -> tuple[list[Any], dict[str, Any]]:
seen_var_positional = True
elif param.kind == param.VAR_KEYWORD:
# e.g. **kwargs handling
keyword_args.update(value or {})
var_keyword_values = value or {}
self._raise_on_var_keyword_collisions(name, var_keyword_values)
keyword_args.update(var_keyword_values)
elif param.kind in (param.POSITIONAL_ONLY, param.POSITIONAL_OR_KEYWORD):
# Before *args, add to positional args. After *args, add to keyword args.
if not seen_var_positional:
Expand All @@ -81,6 +88,37 @@ def to_call_args(self, data: BaseModel) -> tuple[list[Any], dict[str, Any]]:
keyword_args[name] = value
return positional_args, keyword_args

def _raise_on_var_keyword_collisions(
self, var_keyword_name: str, var_keyword_values: dict[str, Any]
) -> None:
"""Reject ``**kwargs`` keys that name a parameter the call already binds by name.

``**kwargs`` is splatted last, so such a key either replaces the value the model
supplied for that parameter -- and Pydantic validated -- or makes the call fail with
"got multiple values for argument". Neither is what the schema promised, so treat it
as model misbehavior and say which keys clashed.

Positional-only parameters and ``*args`` are deliberately not reserved: for
``def f(a, /, **kw)``, the call ``f(1, a=2)`` is legal and routes ``a=2`` into ``kw``.
The names below only ever reveal the tool's own signature, which the model already
has, so they are safe to name even when tool data is redacted.
"""
reserved_names = {
name
for name, param in self.signature.parameters.items()
if param.kind in (param.POSITIONAL_OR_KEYWORD, param.KEYWORD_ONLY)
}
conflicts = sorted(reserved_names.intersection(var_keyword_values))
if not conflicts:
return

conflict_list = ", ".join(repr(conflict) for conflict in conflicts)
raise ModelBehaviorError(
f"Invalid arguments for tool {self.name}: {conflict_list} "
f"{'is' if len(conflicts) == 1 else 'are'} both a named parameter and a key in "
f"'{var_keyword_name}'. Pass each argument once, as a named parameter."
)


@dataclass
class FuncDocumentation:
Expand Down
93 changes: 92 additions & 1 deletion tests/test_function_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from typing_extensions import TypedDict

from agents import RunContextWrapper, function_tool
from agents.exceptions import UserError
from agents.exceptions import ModelBehaviorError, UserError
from agents.function_schema import function_schema, generate_func_documentation


Expand Down Expand Up @@ -1183,3 +1183,94 @@ def test_default_equality_is_not_used_for_sentinel_comparison(
parsed = fs.params_pydantic_model(x=1)
args, kwargs = fs.to_call_args(parsed)
assert isinstance((args + list(kwargs.values()))[-1], default_type)


def _kwargs_keyword_only(*, opt: int = 1, **kw: Any) -> tuple[int, dict[str, Any]]:
return opt, kw


def _kwargs_positional_or_keyword(x: int, *rest: int, **kw: Any) -> tuple[int, tuple[int, ...]]:
return x, rest


def _kwargs_after_var_positional(*rest: int, y: int = 0, **kw: Any) -> tuple[int, int]:
return y, len(rest)


def _kwargs_with_context(ctx: RunContextWrapper[str], n: int, **kw: Any) -> int:
return n


@pytest.mark.parametrize(
("func", "payload", "conflict"),
[
pytest.param(
_kwargs_keyword_only,
{"opt": 5, "kw": {"opt": 9}},
"'opt'",
id="keyword-only",
),
pytest.param(
_kwargs_positional_or_keyword,
{"x": 1, "rest": [2, 3], "kw": {"x": 99}},
"'x'",
id="positional-or-keyword",
),
pytest.param(
_kwargs_after_var_positional,
{"rest": [1], "y": 2, "kw": {"y": 3}},
"'y'",
id="keyword-only-after-var-positional",
),
pytest.param(
_kwargs_with_context,
{"n": 1, "kw": {"ctx": 9}},
"'ctx'",
id="context-parameter",
),
],
)
def test_to_call_args_rejects_kwargs_keys_that_collide_with_named_params(
func: Callable[..., Any], payload: dict[str, Any], conflict: str
) -> None:
"""A **kwargs key naming a keyword-bindable parameter is not a callable combination.

Splatting it would either replace the validated value for that parameter or make the
call fail with "got multiple values for argument", so it is reported to the model.
"""
fs = function_schema(func, strict_json_schema=False)
parsed = fs.params_pydantic_model(**payload)

with pytest.raises(ModelBehaviorError) as exc_info:
fs.to_call_args(parsed)

assert conflict in str(exc_info.value)
assert fs.name in str(exc_info.value)


def _kwargs_positional_only(a: int, /, **kw: Any) -> tuple[int, dict[str, Any]]:
return a, kw


def _kwargs_var_positional_name(*rest: int, **kw: Any) -> tuple[tuple[int, ...], dict[str, Any]]:
return rest, kw


def test_to_call_args_allows_kwargs_key_matching_positional_only_param() -> None:
"""``f(1, a=2)`` is legal for ``def f(a, /, **kw)``: the key belongs to ``**kw``."""
fs = function_schema(_kwargs_positional_only, strict_json_schema=False)
parsed = fs.params_pydantic_model(**{"a": 1, "kw": {"a": 2}})

args, kwargs_dict = fs.to_call_args(parsed)

assert _kwargs_positional_only(*args, **kwargs_dict) == (1, {"a": 2})


def test_to_call_args_allows_kwargs_key_matching_var_positional_param() -> None:
"""``*args`` binds no name, so a key of the same name belongs to ``**kw``."""
fs = function_schema(_kwargs_var_positional_name, strict_json_schema=False)
parsed = fs.params_pydantic_model(**{"rest": [1], "kw": {"rest": 5}})

args, kwargs_dict = fs.to_call_args(parsed)

assert _kwargs_var_positional_name(*args, **kwargs_dict) == ((1,), {"rest": 5})
23 changes: 23 additions & 0 deletions tests/test_function_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -1435,3 +1435,26 @@ def test_function_tool_timeout_error_function_must_be_callable() -> None:
on_invoke_tool=_noop_on_invoke_tool,
timeout_error_function=cast(Any, "not-callable"),
)


def kwargs_collision_function(x: int, *rest: int, **kw: Any) -> str:
return f"x={x} rest={rest} kw={kw}"


@pytest.mark.asyncio
async def test_kwargs_key_colliding_with_param_is_reported_as_model_behavior_error():
"""The collision reaches the model as feedback, not as an unhandled TypeError.

``kw={"x": 99}`` used to splat into the call as ``f(1, 2, 3, x=99)``, which raised
``TypeError: got multiple values for argument 'x'`` from inside the tool call.
"""
tool = function_tool(kwargs_collision_function, strict_mode=False, failure_error_function=None)
arguments = '{"x": 1, "rest": [2, 3], "kw": {"x": 99}}'

with pytest.raises(ModelBehaviorError) as exc_info:
await tool.on_invoke_tool(
ToolContext(None, tool_name=tool.name, tool_call_id="1", tool_arguments=arguments),
arguments,
)

assert "'x'" in str(exc_info.value)
Loading