diff --git a/python/packages/core/agent_framework/_types.py b/python/packages/core/agent_framework/_types.py index 4b8572e884a..de30bf6d67d 100644 --- a/python/packages/core/agent_framework/_types.py +++ b/python/packages/core/agent_framework/_types.py @@ -4215,11 +4215,19 @@ def merge_chat_options( # Merge tools lists base_tools = result.get("tools") if base_tools and value: - # Add tools that aren't already present - merged_tools = list(base_tools) - for tool in value if isinstance(value, Iterable) else [value]: # type: ignore[reportUnknownVariableType] - if tool not in merged_tools: - merged_tools.append(tool) + # Add tools that aren't already present, matching by name so the same + # plain callable wrapped on both sides is not added twice. + from ._tools import _get_tool_name # pyright: ignore[reportPrivateUsage] + + merged_tools = normalize_tools(base_tools) + names = {name for t in merged_tools if (name := _get_tool_name(t))} + for tool in normalize_tools(value): + name = _get_tool_name(tool) + if tool in merged_tools or (name is not None and name in names): + continue + merged_tools.append(tool) + if name is not None: + names.add(name) result["tools"] = merged_tools elif value: result["tools"] = value if isinstance(value, list) else [value] diff --git a/python/packages/core/tests/core/test_types.py b/python/packages/core/tests/core/test_types.py index 585a654a3df..495069db620 100644 --- a/python/packages/core/tests/core/test_types.py +++ b/python/packages/core/tests/core/test_types.py @@ -5327,3 +5327,56 @@ def test_agent_response_update_serialization_includes_finish_reason() -> None: # endregion + + +def test_merge_chat_options_single_mapping_tool_is_not_spread_into_keys(): + """A single tool given as a mapping is one tool, on either side of the merge.""" + + @tool + def my_tool() -> None: + pass + + hosted = {"type": "web_search", "name": "ws"} + + merged = merge_chat_options({"tools": [my_tool]}, {"tools": hosted}) # type: ignore[arg-type] # pyrefly: ignore[bad-argument-type] # ty: ignore[invalid-argument-type] + assert merged["tools"] == [my_tool, hosted] + + merged = merge_chat_options({"tools": hosted}, {"tools": [my_tool]}) # type: ignore[arg-type] # pyrefly: ignore[bad-argument-type] # ty: ignore[invalid-argument-type] + assert merged["tools"] == [hosted, my_tool] + + +def test_merge_chat_options_same_plain_callable_on_both_sides(): + """The same undecorated function on both sides is merged into one tool.""" + + def my_func() -> None: + pass + + merged = merge_chat_options({"tools": [my_func]}, {"tools": [my_func]}) # type: ignore[arg-type] # pyrefly: ignore[bad-argument-type] # ty: ignore[invalid-argument-type] + assert [t.name for t in merged["tools"]] == ["my_func"] + + +def test_merge_chat_options_keeps_pydantic_provider_tool_whole(): + """A Pydantic provider-native tool spec is one tool, not spread into its fields.""" + + class ProviderTool(BaseModel): + type: str = "code_interpreter" + container: str = "auto" + + @tool + def my_tool() -> None: + pass + + provider_tool = ProviderTool() + + merged = merge_chat_options({"tools": [my_tool]}, {"tools": [provider_tool]}) # type: ignore[arg-type] # pyrefly: ignore[bad-argument-type] # ty: ignore[invalid-argument-type] + assert merged["tools"] == [my_tool, provider_tool] + + merged = merge_chat_options({"tools": [provider_tool]}, {"tools": [my_tool]}) # type: ignore[arg-type] # pyrefly: ignore[bad-argument-type] # ty: ignore[invalid-argument-type] + assert merged["tools"] == [provider_tool, my_tool] + + # A single, unwrapped provider tool on either side. + merged = merge_chat_options({"tools": [my_tool]}, {"tools": provider_tool}) # type: ignore[arg-type] # pyrefly: ignore[bad-argument-type] # ty: ignore[invalid-argument-type] + assert merged["tools"] == [my_tool, provider_tool] + + merged = merge_chat_options({"tools": provider_tool}, {"tools": [my_tool]}) # type: ignore[arg-type] # pyrefly: ignore[bad-argument-type] # ty: ignore[invalid-argument-type] + assert merged["tools"] == [provider_tool, my_tool]