fix: ensure assistant messages have reasoningContent when thinking is enabled#1889
Open
giulio-leone wants to merge 1 commit intostrands-agents:mainfrom
Open
fix: ensure assistant messages have reasoningContent when thinking is enabled#1889giulio-leone wants to merge 1 commit intostrands-agents:mainfrom
giulio-leone wants to merge 1 commit intostrands-agents:mainfrom
Conversation
… enabled When extended thinking is enabled, Bedrock requires every assistant message to start with a reasoningContent block. Custom session managers (e.g., AgentCoreMemorySessionManager) or external history providers may strip these blocks during serialization, causing ValidationException on subsequent API calls. This fix adds defensive handling in _format_bedrock_messages(): - Injects a minimal redactedContent placeholder when reasoningContent is missing from assistant messages with thinking enabled - Reorders content blocks to ensure reasoningContent comes first when it exists but is not at position 0 Closes strands-agents#1698
6b1094e to
853244c
Compare
Contributor
Author
|
Friendly ping — ensures assistant messages include |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Closes #1698
Problem
When extended thinking is enabled, Bedrock requires every assistant message to start with a
reasoningContentblock. Custom session managers (e.g.,AgentCoreMemorySessionManager) or external history providers may strip these blocks during serialization/deserialization, causing aValidationExceptionon subsequent API calls:Root Cause
The SDK correctly constructs
reasoningContentblocks during streaming and preserves them through the event loop. However, custom session managers that serialize/deserialize conversation history may stripreasoningContentblocks, breaking Bedrock's multi-turn thinking constraint.Solution
Added defensive handling in
_format_bedrock_messages()to ensure assistant messages conform to Bedrock's thinking requirements:reasoningContentblock, injects a minimalredactedContentplaceholder at position 0reasoningContentexists but is not the first content block, reorders blocks to put reasoning firstThis is a defensive measure — it handles misbehaving session managers without breaking correct flows where
reasoningContentis already properly preserved.Testing
Added 6 new tests covering:
test_format_request_injects_reasoning_when_thinking_enabled_and_missing— placeholder injection when thinking blocks are strippedtest_format_request_no_injection_when_thinking_disabled— no modification without thinkingtest_format_request_preserves_existing_reasoning_when_thinking_enabled— existing blocks preservedtest_format_request_reorders_reasoning_to_first_when_thinking_enabled— block reorderingtest_format_request_multi_turn_thinking_session_reload— multi-turn session reload scenariotest_format_request_does_not_inject_reasoning_for_user_messages— user messages untouchedAll 131 tests pass (125 existing + 6 new).