From 94758a589908273e2aab68e041c00b333fde3264 Mon Sep 17 00:00:00 2001 From: Anish Mehta Date: Thu, 24 Sep 2026 22:43:16 +0530 Subject: [PATCH 1/3] Python: fix(core): treat a single mapping tool as one tool in merge_chat_options --- python/packages/core/agent_framework/_types.py | 11 +++++++++-- python/packages/core/tests/core/test_types.py | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/python/packages/core/agent_framework/_types.py b/python/packages/core/agent_framework/_types.py index 4b8572e884a..d0c56b05885 100644 --- a/python/packages/core/agent_framework/_types.py +++ b/python/packages/core/agent_framework/_types.py @@ -4160,6 +4160,13 @@ def _append_instructions( return combined +def _as_tool_list(tools: Any) -> list[Any]: + """Tools as a list, treating a single mapping (e.g. {"type": "web_search"}) as one tool.""" + if isinstance(tools, Mapping) or not isinstance(tools, Iterable): + return [tools] + return list(tools) # type: ignore[reportUnknownArgumentType] + + def merge_chat_options( base: dict[str, Any] | None, override: dict[str, Any] | None, @@ -4216,8 +4223,8 @@ def merge_chat_options( 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] + merged_tools = _as_tool_list(base_tools) + for tool in _as_tool_list(value): if tool not in merged_tools: merged_tools.append(tool) result["tools"] = merged_tools diff --git a/python/packages/core/tests/core/test_types.py b/python/packages/core/tests/core/test_types.py index 585a654a3df..2785057b05a 100644 --- a/python/packages/core/tests/core/test_types.py +++ b/python/packages/core/tests/core/test_types.py @@ -5327,3 +5327,18 @@ 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.""" + + 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] From 810bed732febb71ccd0688a5bd312a2820f48bd2 Mon Sep 17 00:00:00 2001 From: Anish Mehta Date: Tue, 29 Sep 2026 10:08:55 +0530 Subject: [PATCH 2/3] Merge tools through normalize_tools instead of a separate parser --- .../packages/core/agent_framework/_types.py | 11 ++------ python/packages/core/tests/core/test_types.py | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/python/packages/core/agent_framework/_types.py b/python/packages/core/agent_framework/_types.py index d0c56b05885..e5b2b734762 100644 --- a/python/packages/core/agent_framework/_types.py +++ b/python/packages/core/agent_framework/_types.py @@ -4160,13 +4160,6 @@ def _append_instructions( return combined -def _as_tool_list(tools: Any) -> list[Any]: - """Tools as a list, treating a single mapping (e.g. {"type": "web_search"}) as one tool.""" - if isinstance(tools, Mapping) or not isinstance(tools, Iterable): - return [tools] - return list(tools) # type: ignore[reportUnknownArgumentType] - - def merge_chat_options( base: dict[str, Any] | None, override: dict[str, Any] | None, @@ -4223,8 +4216,8 @@ def merge_chat_options( base_tools = result.get("tools") if base_tools and value: # Add tools that aren't already present - merged_tools = _as_tool_list(base_tools) - for tool in _as_tool_list(value): + merged_tools = normalize_tools(base_tools) + for tool in normalize_tools(value): if tool not in merged_tools: merged_tools.append(tool) result["tools"] = merged_tools diff --git a/python/packages/core/tests/core/test_types.py b/python/packages/core/tests/core/test_types.py index 2785057b05a..a6f8a9d6f82 100644 --- a/python/packages/core/tests/core/test_types.py +++ b/python/packages/core/tests/core/test_types.py @@ -5332,6 +5332,7 @@ def test_agent_response_update_serialization_includes_finish_reason() -> None: 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 @@ -5342,3 +5343,30 @@ def my_tool() -> None: 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_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] From 57e9025a6214443b36d312b1ff8ef7b9204b4690 Mon Sep 17 00:00:00 2001 From: Anish Mehta Date: Tue, 29 Sep 2026 17:42:28 +0530 Subject: [PATCH 3/3] Dedupe merged tools by name so the same plain callable is added once --- python/packages/core/agent_framework/_types.py | 14 +++++++++++--- python/packages/core/tests/core/test_types.py | 10 ++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/python/packages/core/agent_framework/_types.py b/python/packages/core/agent_framework/_types.py index e5b2b734762..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 + # 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): - if tool not in merged_tools: - merged_tools.append(tool) + 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 a6f8a9d6f82..495069db620 100644 --- a/python/packages/core/tests/core/test_types.py +++ b/python/packages/core/tests/core/test_types.py @@ -5345,6 +5345,16 @@ def my_tool() -> None: 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."""