mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: fix(bedrock): don't send toolChoice when no tools are configured (#5172)
* fix(bedrock): don't send toolChoice when no tools are configured BedrockChatClient was sending toolConfig.toolChoice even when no tools were configured (tools=None). AWS Bedrock requires toolConfig.tools to be present whenever toolChoice is specified, causing a 400 validation error. Only set toolChoice when tool_config has a 'tools' key present. Fixes #5165 Signed-off-by: bahtya <bahtyar153@qq.com> * test: add tests for toolChoice without tools - test_prepare_options_tool_choice_auto_without_tools_omits_tool_config - test_prepare_options_tool_choice_required_without_tools_omits_tool_config Verifies that toolConfig is omitted when tool_choice is set but no tools are provided, preventing ParamValidationError from Bedrock. * fix: address maintainer feedback — remove stray test file, raise ValueError for required without tools 1. Remove test_addition.py — stray duplicate of tests already in python/packages/bedrock/tests/test_bedrock_client.py, missing all necessary imports and would fail with NameError. 2. Change tool_choice='required' handling to raise ValueError when no tools are configured instead of silently falling through. Using 'required' without tools is a logical contradiction — the model must invoke a tool but none exist — so surfacing this as a ValueError helps callers catch the misconfiguration early. 3. Update the corresponding test to expect ValueError instead of silently omitted toolConfig. --------- Signed-off-by: bahtya <bahtyar153@qq.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
ddfbdf5c7a
commit
f3f71f0fe8
@@ -413,10 +413,14 @@ class BedrockChatClient(
|
||||
# Omit toolConfig entirely so the model won't attempt tool calls.
|
||||
tool_config = None
|
||||
case "auto":
|
||||
tool_config = tool_config or {}
|
||||
tool_config["toolChoice"] = {"auto": {}}
|
||||
if tool_config and "tools" in tool_config:
|
||||
tool_config["toolChoice"] = {"auto": {}}
|
||||
case "required":
|
||||
tool_config = tool_config or {}
|
||||
if not (tool_config and "tools" in tool_config):
|
||||
raise ValueError(
|
||||
"tool_choice='required' requires at least one tool to be configured, "
|
||||
"but no tools were provided."
|
||||
)
|
||||
if required_name := tool_mode.get("required_function_name"):
|
||||
tool_config["toolChoice"] = {"tool": {"name": required_name}}
|
||||
else:
|
||||
|
||||
@@ -137,3 +137,36 @@ def test_prepare_options_tool_choice_required_includes_any() -> None:
|
||||
|
||||
assert "toolConfig" in request
|
||||
assert request["toolConfig"]["toolChoice"] == {"any": {}}
|
||||
|
||||
|
||||
|
||||
def test_prepare_options_tool_choice_auto_without_tools_omits_tool_config() -> None:
|
||||
"""When tool_choice='auto' but no tools are provided, toolConfig must be omitted.
|
||||
|
||||
Without tools, setting toolChoice would cause a ParamValidationError from Bedrock.
|
||||
"""
|
||||
client = _make_client()
|
||||
messages = [Message(role="user", contents=[Content.from_text(text="hello")])]
|
||||
|
||||
options: dict[str, Any] = {
|
||||
"tool_choice": "auto",
|
||||
}
|
||||
|
||||
request = client._prepare_options(messages, options)
|
||||
|
||||
assert "toolConfig" not in request, (
|
||||
f"toolConfig should be omitted when no tools are provided, got: {request.get('toolConfig')}"
|
||||
)
|
||||
|
||||
|
||||
def test_prepare_options_tool_choice_required_without_tools_raises() -> None:
|
||||
"""When tool_choice='required' but no tools are provided, a ValueError must be raised."""
|
||||
client = _make_client()
|
||||
messages = [Message(role="user", contents=[Content.from_text(text="hello")])]
|
||||
|
||||
options: dict[str, Any] = {
|
||||
"tool_choice": "required",
|
||||
}
|
||||
|
||||
with pytest.raises(ValueError, match="tool_choice='required' requires at least one tool"):
|
||||
client._prepare_options(messages, options)
|
||||
|
||||
Reference in New Issue
Block a user