Python: fix(bedrock): don't send toolChoice when no tools are configured

[Bahtya]

Problem

When using BedrockChatClient with an Agent that has no tools configured, the client still sends toolConfig.toolChoice to the Bedrock API. AWS Bedrock requires toolConfig.tools to be present whenever toolChoice is specified, causing a 400 validation error.

Root Cause

In _prepare_run_options, when tool_choice is set but _prepare_tools returns None (no tools), the code creates an empty dict with tool_config or {} and adds toolChoice to it. This results in sending {"toolChoice": {"auto": {}}} without any tools key.

Fix

Only set toolChoice when tool_config is not None and has a "tools" key. When no tools are configured, toolChoice is silently skipped.

Fixes #5165

[eavanvalkenburg]

Could you add a test to verify the behavior?

[Bahtya]

@eavanvalkenburg Thanks for the review! I will add a test to verify that toolChoice is not sent when no tools are provided. Will push the update shortly.

[chetantoshniwal]

Automated Code Review

Reviewers: 4 | Confidence: 94%

✗ Correctness

The core fix in _chat_client.py is correct: it guards toolChoice assignment behind tool_config and "tools" in tool_config, preventing Bedrock ParamValidationError when tool_choice is set but no tools are provided. The old code used tool_config = tool_config or {} which would create an empty dict with only toolChoice and no tools, which Bedrock rejects. The new tests in test_bedrock_client.py properly cover both auto and required modes without tools. However, test_addition.py at the repo root is a stray duplicate test file with no imports — it would fail if discovered by a test runner and should not be committed.

✗ Security Reliability

The core fix in _chat_client.py is a valid reliability improvement: it prevents sending a toolChoice directive to Bedrock when no tools are defined, which would cause a ParamValidationError. The logic change is sound—it guards toolChoice assignment behind a check for tool_config and 'tools' in tool_config. However, a test-only file test_addition.py was added at the repository root that is non-functional (missing imports, no test infrastructure), appears to be an accidental leftover, and should be removed.

✗ Test Coverage

The production code change correctly guards against sending toolChoice without tools to Bedrock. The two new tests in test_bedrock_client.py properly cover the 'auto' and 'required' no-tools scenarios. However, there is a stray test_addition.py file at the repo root that duplicates these tests but is broken (missing imports and helpers), and there is a missing test for the 'required' mode with a specific required_function_name when no tools are provided.

✗ Design Approach

The core fix in _chat_client.py correctly prevents sending a toolChoice to Bedrock when no tools are registered, avoiding ParamValidationError. However, silently swallowing the constraint for the required mode (which semantically means 'the model MUST call a tool') masks a likely caller misconfiguration — a ValueError would be more appropriate there. More critically, test_addition.py was accidentally committed to the repo root: it is a broken duplicate of the tests already added to test_bedrock_client.py, missing all imports and the _make_client helper, and will fail immediately if pytest discovers it.

---

Automated review by chetantoshniwal's agents

[chetantoshniwal]

This file is a stray duplicate of the tests already added to python/packages/bedrock/tests/test_bedrock_client.py. It is missing all necessary imports (_make_client, Message, Content, Any) and will fail with NameError if any test runner discovers it. Please remove this file from the PR.

[chetantoshniwal]

Silently ignoring tool_choice='required' when no tools are present masks a caller misconfiguration. required means the model must invoke a tool; having no tools makes this a logical contradiction. Raising a ValueError here surfaces the bug to the caller rather than silently degrading behaviour.

Suggested change

	case "required":
	case "required":
	if not (tool_config and "tools" in tool_config):
	raise ValueError(
	"tool_choice='required' was specified but no tools were provided."
	)
	if required_name := tool_mode.get("required_function_name"):
	tool_config["toolChoice"] = {"tool": {"name": required_name}}
	else:
	tool_config["toolChoice"] = {"any": {}}

[Bahtya]

Thanks for the detailed review @chetantoshniwal! I have addressed both points of feedback:

1. Removed stray duplicate test file (test_addition.py)
This file was a duplicate of the tests already present in python/packages/bedrock/tests/test_bedrock_client.py. It was missing all necessary imports (_make_client, Message, Content, Any) and would have caused NameError failures if discovered by any test runner. The file has been removed entirely.

2. tool_choice='required' now raises ValueError when no tools are configured
Instead of silently skipping toolChoice when no tools are present, the required case now explicitly raises:

case "required":
    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."
        )

This surfaces the caller misconfiguration immediately rather than silently degrading behaviour. The corresponding test (test_prepare_options_tool_choice_required_without_tools_raises) has been updated to expect ValueError via pytest.raises.

[Bahtya]

Hi @eavanvalkenburg @chetantoshniwal, thanks for the review! I'll address all feedback: (1) remove the stray test_addition.py, (2) add proper tests for the fix, (3) consider raising ValueError instead of silently ignoring tool_choice. I'll also make sure the CLA is signed. Working on these updates now.

[Bahtya]

@microsoft-github-policy-service agree

[moonbox3]

@Bahtya please resolve open comments when they are indeed addressed/fixed. Thanks.

[Bahtya]

Hi @eavanvalkenburg, I've added the requested tests in test_bedrock_client.py that verify both the auto and required tool_choice modes when no tools are configured. The stray test_addition.py has also been removed and the required mode now raises a ValueError as suggested. Could you take another look?

[Bahtya]

@moonbox3 @eavanvalkenburg I've addressed the feedback in the latest commits: removed the stray test_addition.py, added ValueError for tool_choice='required' with no tools, and added proper tests. The CI workflows are currently stuck at action_required and need maintainer approval to run. Could someone approve the workflow runs so we can verify everything passes?