Python: fix(bedrock): don't send toolChoice when no tools are configured by Bahtya · Pull Request #5172 · microsoft/agent-framework

Bahtya · 2026-04-08T17:35:32Z

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

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 <[email protected]>

eavanvalkenburg

Could you add a test to verify the behavior?

Bahtya · 2026-04-13T11:56:38Z

@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.

- 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.

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

…eError 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.

Bahtya · 2026-04-17T06:13:22Z

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 · 2026-04-17T16:20:23Z

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 · 2026-04-18T13:12:44Z

@microsoft-github-policy-service agree

moonbox3 · 2026-04-21T02:40:59Z

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

Bahtya · 2026-04-27T03:35:53Z

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 · 2026-04-27T04:33:07Z

@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?

moonbox3 · 2026-05-05T07:53:38Z

Python Test Coverage Report •

File	Stmts	Miss	Cover	Missing
packages/bedrock/agent_framework_bedrock
_chat_client.py	381	100	73%	298–299, 315–324, 329, 345–351, 354–355, 363, 380, 389, 400, 402, 404, 426–427, 448, 461, 473, 476, 484–485, 488–489, 491–492, 497–499, 501, 511–512, 534, 541, 550–551, 553–554, 556–558, 560, 562–563, 569–571, 574–575, 581–584, 590–600, 603, 622, 627, 669–670, 683, 709, 721, 726, 735, 739, 747–748, 752, 754–761
TOTAL	28326	3299	88%

Python Unit Test Overview

Tests	Skipped	Failures	Errors	Time
5653	30 💤	0 ❌	0 🔥	1m 34s ⏱️

moonbox3 added the python Usage: [Issues, PRs], Target: Python label Apr 8, 2026

github-actions Bot changed the title ~~fix(bedrock): don't send toolChoice when no tools are configured~~ Python: fix(bedrock): don't send toolChoice when no tools are configured Apr 8, 2026

eavanvalkenburg reviewed Apr 10, 2026

View reviewed changes

chetantoshniwal reviewed Apr 14, 2026

View reviewed changes

Comment thread test_addition.py Outdated

Comment thread python/packages/bedrock/agent_framework_bedrock/_chat_client.py

eavanvalkenburg approved these changes May 5, 2026

View reviewed changes

eavanvalkenburg enabled auto-merge May 5, 2026 07:51

chetantoshniwal approved these changes May 5, 2026

View reviewed changes

eavanvalkenburg added this pull request to the merge queue May 5, 2026

Merged via the queue into microsoft:main with commit f3f71f0 May 5, 2026
31 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

Python: fix(bedrock): don't send toolChoice when no tools are configured#5172
eavanvalkenburg merged 3 commits into
microsoft:mainfrom
Bahtya:fix/bedrock-toolconfig

Bahtya commented Apr 8, 2026

Uh oh!

eavanvalkenburg left a comment

Uh oh!

Bahtya commented Apr 13, 2026

Uh oh!

chetantoshniwal left a comment

Uh oh!

Uh oh!

Uh oh!

Bahtya commented Apr 17, 2026

Uh oh!

Bahtya commented Apr 17, 2026

Uh oh!

Bahtya commented Apr 18, 2026

Uh oh!

moonbox3 commented Apr 21, 2026

Uh oh!

Bahtya commented Apr 27, 2026

Uh oh!

Bahtya commented Apr 27, 2026

Uh oh!

moonbox3 commented May 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

Bahtya commented Apr 8, 2026

Problem

Root Cause

Fix

Uh oh!

eavanvalkenburg left a comment

Choose a reason for hiding this comment

Uh oh!

Bahtya commented Apr 13, 2026

Uh oh!

chetantoshniwal left a comment

Choose a reason for hiding this comment

Automated Code Review

✗ Correctness

✗ Security Reliability

✗ Test Coverage

✗ Design Approach

Uh oh!

Uh oh!

Uh oh!

Bahtya commented Apr 17, 2026

Uh oh!

Bahtya commented Apr 17, 2026

Uh oh!

Bahtya commented Apr 18, 2026

Uh oh!

moonbox3 commented Apr 21, 2026

Uh oh!

Bahtya commented Apr 27, 2026

Uh oh!

Bahtya commented Apr 27, 2026

Uh oh!

moonbox3 commented May 5, 2026

Python Unit Test Overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants