Python: Fix agent option merge to support dict-defined tools

[moonbox3]

Motivation and Context

When tools were defined as plain dicts (the {"type": "function", "function": {"name": ...}} format accepted by OpenAI-compatible APIs), _merge_options used getattr(t, "name", None) to extract tool names for deduplication, which always returned None for dicts — causing all override tools to be silently dropped as false-positive duplicates.

Fixes #4303

Description

The root cause was that _merge_options assumed tools were always objects with a .name attribute. For dict-defined tools, the name lives at tool["function"]["name"], so getattr returned None for every tool, and the None-populated existing_names set incorrectly matched all incoming tools. The fix introduces a _get_tool_name helper that handles both object and dict tool formats, and discards None from the existing-names set so that unnamed tools never cause spurious deduplication.

Contribution Checklist

• The code builds clean without any errors or warnings
• The PR follows the Contribution Guidelines
• All unit tests pass, and I have added new tests where possible
• Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

---

Note: PR autogenerated by moonbox3's agent

[moonbox3]

Automated Code Review

Reviewers: 3 | Confidence: 90%

✓ Correctness

The diff introduces a _get_tool_name helper to support both dict-style and object-style tool definitions in _merge_options, and correctly discards None from the existing names set so that unnamed tools are not erroneously deduplicated. The logic is sound, the tests cover dict tools, object tools, deduplication, and the no-name case. No correctness bugs found.

✓ Security Reliability

The diff introduces a helper _get_tool_name and updates _merge_options to handle dict-format tools. The changes are generally clean. One minor reliability concern: if a dict tool has a "function" key mapped to a non-dict value (e.g., None), the chained .get("name") call will raise an AttributeError. This is unlikely in practice but violates the defensive style the rest of the function aims for. No security issues found.

✗ Test Coverage

The new tests cover the core dict-tool scenarios well (_get_tool_name extraction, dict tool merging, deduplication). However, there is a meaningful gap: the existing_names.discard(None) on line 108 of _agents.py is a deliberate behavior change (nameless tools in the override are no longer silently dropped when a nameless tool exists in base), yet no test exercises this path. Additionally, there is no test for mixed tool types (object tools in base + dict tools in override), which is the cross-format scenario this change is specifically enabling.

Blocking Issues

• No test covers the existing_names.discard(None) behavior (line 108). Without this, a regression that removes the discard call would silently drop nameless override tools, and the test suite would not catch it.

Suggestions

• In _get_tool_name, if a dict has a "function" key whose value is not itself a dict (e.g., a string), the chained .get("name") call will raise an AttributeError. Consider guarding with an isinstance check if malformed tool dicts are a realistic input.
• In _get_tool_name, guard against tool["function"] being a non-dict value (e.g., None) which would cause an AttributeError on the chained .get("name") call. Using (tool.get("function") or {}).get("name") or an explicit isinstance check would make it more robust.
• Add a test for mixed tool types (e.g., object-based tool in base, dict-based tool in override) to verify cross-format deduplication works correctly.
• Consider testing _get_tool_name with a dict that has a function key but no name key (e.g., {"function": {}}) to explicitly cover that branch.

---

Automated review by moonbox3's agents

[Copilot]

Pull request overview

This PR fixes a critical bug where dict-defined tools (OpenAI-compatible {"type": "function", "function": {"name": ...}} format) were being silently dropped during agent option merging. The root cause was that _merge_options used getattr(t, "name", None) to extract tool names, which always returned None for dict tools, causing all runtime dict tools to be incorrectly treated as duplicates.

Changes:

• Introduced _get_tool_name() helper function that extracts names from both object-based tools (via .name attribute) and dict-based tools (via ["function"]["name"] path)
• Updated _merge_options() to use the new helper and discard None from the existing-names set to prevent spurious deduplication
• Added comprehensive test coverage for dict tools, object tools, and edge cases where tools have no name

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
python/packages/core/agent_framework/_agents.py	Added _get_tool_name() helper and updated _merge_options() to properly handle dict-defined tools
python/packages/core/tests/core/test_agents.py	Added tests for dict tool merging, deduplication, and _get_tool_name() helper with both dict and object formats

[markwallace-microsoft]

Python Test Coverage Report •

File	Stmts	Miss	Cover	Missing
packages/core/agent_framework
_agents.py	337	43	87%	434, 438, 490, 855, 891, 907, 983–986, 1047–1049, 1170, 1186, 1188, 1201, 1207, 1243, 1245, 1254–1259, 1264, 1266, 1272–1273, 1280, 1282–1283, 1291–1292, 1295–1297, 1305–1306, 1308, 1313, 1315
TOTAL	22180	2762	87%

Python Unit Test Overview

Tests	Skipped	Failures	Errors	Time
4677	247 💤	0 ❌	0 🔥	1m 20s ⏱️

[moonbox3]

Closing due to agent review errors.