fix(bedrock): read reasoningText on non-streaming Converse responses - #89671
Open
Jiaaqiliu wants to merge 1 commit into
Open
fix(bedrock): read reasoningText on non-streaming Converse responses#89671Jiaaqiliu wants to merge 1 commit into
Jiaaqiliu wants to merge 1 commit into
Conversation
AWS Converse has two different shapes for reasoning. The non-streaming
`ReasoningContentBlock` is a union whose text member is
`{"reasoningText": {"text", "signature"}}`; only the streaming
`ReasoningContentBlockDelta` puts `text` at the top level.
`normalize_converse_response` used the streaming shape, so
`reasoning.get("text", "")` was always empty on a non-streaming call and the
model's reasoning was dropped entirely: the reasoning box never rendered,
reasoning-token accounting saw nothing, and nothing was stored for replay.
`reasoningText` appeared nowhere in the tree. The streaming handler a few
hundred lines below is correct for its own shape — this is a mismatch between
the two functions, not a systematic omission.
Reproduces with any model that emits reasoningContent on a non-streaming
converse() call (for example us.deepseek.r1-v1:0, or a Claude model with
extended thinking enabled via additionalModelRequestFields).
The flat `{"text": ...}` form is still accepted as a fallback so a caller that
hands the function an already-flattened block keeps working. A block carrying
only `redactedContent` correctly yields no text.
Contributor
Correct shape fix with the AWS union documented inline where future readers need it: non-streaming — reviewer-b (automated review) No blocking issues found. |
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.
What does this PR do?
AWS Converse has two different shapes for reasoning content:
ReasoningContentBlock— a union whose text member is{"reasoningText": {"text", "signature"}}ReasoningContentBlockDelta— puts"text"at the top levelnormalize_converse_responseused the streaming shape on the non-streaming response:So
reasoning.get("text")was always empty andreasoning_contentalwaysNoneon non-streaming calls. The reasoning box never rendered, reasoning-token accounting saw nothing, and nothing was stored for replay.reasoningTextappears nowhere in the tree (grep -rn reasoningText→ 0 hits before this PR).The streaming handler in
stream_converse_responseis correct for its shape, so this is a mismatch between the two functions rather than a systematic omission.Reproduction
Hit by any model that emits
reasoningContenton a non-streamingconverse()call —us.deepseek.r1-v1:0emits it unconditionally, and Claude models do with extended thinking enabled viaadditionalModelRequestFields.Related Issue
No open issue found. #21202 (merged) added
reasoningContenthandling to normalized responses but used the delta shape; #36261 is about preserving the reasoning signature on interleaved-thinking replay, which is a separate concern and unaffected by this change.Fixes #
Type of Change
Changes Made
agent/bedrock_adapter.py—normalize_converse_responsereads thereasoningText.textunion member, falling back to the flattextform so a caller passing an already-flattened block keeps working. A block carrying onlyredactedContentcorrectly yields no text.tests/agent/test_bedrock_adapter.py— newTestNonStreamingReasoningContent: union member preserved, flat form still accepted, redacted-only yieldsNone, absent reasoning yieldsNone.How to Test
70 passed (66 existing + 4 new). Reverting only the source change makes
test_reasoning_text_union_member_is_preservedfail withNone != 'The user wants the file. I should call read_file.'.Checklist
Code
pytest tests/agent/test_bedrock_adapter.py -qand all tests pass