fix(bedrock): preserve stream param and decode SSE for bedrock mantle streaming - #32141
Conversation
Greptile SummaryThis PR fixes two compounding bugs that caused streaming requests to
Confidence Score: 4/5Safe to merge — both changes are narrowly scoped to the mantle routing path and do not touch any shared Bedrock or Anthropic code paths. The fix is correct and well-tested with mock-only unit tests covering sync/async and both surfaces. The one minor concern is in litellm/llms/bedrock/messages/mantle_transformation.py — the
|
| Filename | Overview |
|---|---|
| litellm/llms/bedrock/chat/mantle/transformation.py | Adds stream restoration in body, opts out of the binary Bedrock stream wrapper, and delegates to Anthropic's SSE iterator — all logically correct. |
| litellm/llms/bedrock/messages/mantle_transformation.py | Correctly restores stream in the request body and delegates streaming to AnthropicMessagesConfig; uses an unconventional self=self keyword-argument pattern that is currently harmless but fragile if the target method ever starts using self. |
| tests/test_litellm/llms/bedrock/test_mantle.py | New mock-only tests cover all surfaces (sync/async chat, messages API) for both the stream body injection and the SSE decode path; no real network calls and no weakening of existing assertions. |
Reviews (1): Last reviewed commit: "fix(bedrock): preserve stream param and ..." | Re-trigger Greptile
| def get_async_streaming_response_iterator( | ||
| self, | ||
| model: str, | ||
| httpx_response: httpx.Response, | ||
| request_body: dict, | ||
| litellm_logging_obj: LiteLLMLoggingObj, | ||
| ) -> AsyncIterator: | ||
| return AnthropicMessagesConfig.get_async_streaming_response_iterator( | ||
| self=self, | ||
| model=model, | ||
| httpx_response=httpx_response, | ||
| request_body=request_body, | ||
| litellm_logging_obj=litellm_logging_obj, | ||
| ) |
There was a problem hiding this comment.
Calling
AnthropicMessagesConfig.get_async_streaming_response_iterator(self=self, ...) with self passed as a keyword argument is unconventional. More importantly, because AmazonMantleMessagesConfig does not inherit from AnthropicMessagesConfig, any future change to that method that calls self.some_attr will dispatch through AmazonMantleMessagesConfig's MRO rather than AnthropicMessagesConfig's — silently producing unexpected behavior. The standard way to call an unbound method from a non-parent class is to pass the instance positionally.
| def get_async_streaming_response_iterator( | |
| self, | |
| model: str, | |
| httpx_response: httpx.Response, | |
| request_body: dict, | |
| litellm_logging_obj: LiteLLMLoggingObj, | |
| ) -> AsyncIterator: | |
| return AnthropicMessagesConfig.get_async_streaming_response_iterator( | |
| self=self, | |
| model=model, | |
| httpx_response=httpx_response, | |
| request_body=request_body, | |
| litellm_logging_obj=litellm_logging_obj, | |
| ) | |
| def get_async_streaming_response_iterator( | |
| self, | |
| model: str, | |
| httpx_response: httpx.Response, | |
| request_body: dict, | |
| litellm_logging_obj: LiteLLMLoggingObj, | |
| ) -> AsyncIterator: | |
| return AnthropicMessagesConfig.get_async_streaming_response_iterator( | |
| self, | |
| model=model, | |
| httpx_response=httpx_response, | |
| request_body=request_body, | |
| litellm_logging_obj=litellm_logging_obj, | |
| ) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Greptile SummaryThis PR fixes two compounding bugs that caused HTTP 500
Confidence Score: 4/5Safe to merge — the change is narrowly scoped to the mantle routing path, non-mantle Bedrock callers are unaffected, and the new tests exercise both the body-transform and SSE-decode paths end-to-end with mocked HTTP. The two-part fix is logically correct and well-tested. The only roughness is in litellm/llms/bedrock/messages/mantle_transformation.py — the unbound MRO-bypass call warrants a second look.
|
| Filename | Overview |
|---|---|
| litellm/llms/bedrock/chat/mantle/transformation.py | Restores stream in the request body via _restore_mantle_body_fields and opts out of the Bedrock binary event-stream wrapper by returning False from has_custom_stream_wrapper and delegating to the Anthropic SSE ModelResponseIterator. |
| litellm/llms/bedrock/messages/mantle_transformation.py | Restores stream in the messages request body and bypasses the binary AmazonAnthropicClaudeMessagesStreamDecoder by calling AnthropicMessagesConfig.get_async_streaming_response_iterator directly via an unbound call; the unbound pattern works but is stylistically unusual. |
| tests/test_litellm/llms/bedrock/test_mantle.py | Adds new mock-based tests for stream body preservation (sync/async, chat and messages paths) and end-to-end SSE decoding for completion, acompletion, and anthropic_messages; no real network calls, consistent with CI requirements. |
Reviews (2): Last reviewed commit: "fix(bedrock): preserve stream param and ..." | Re-trigger Greptile
| def get_async_streaming_response_iterator( | ||
| self, | ||
| model: str, | ||
| httpx_response: httpx.Response, | ||
| request_body: dict, | ||
| litellm_logging_obj: LiteLLMLoggingObj, | ||
| ) -> AsyncIterator: | ||
| return AnthropicMessagesConfig.get_async_streaming_response_iterator( | ||
| self=self, | ||
| model=model, | ||
| httpx_response=httpx_response, | ||
| request_body=request_body, | ||
| litellm_logging_obj=litellm_logging_obj, | ||
| ) |
There was a problem hiding this comment.
The unbound call
AnthropicMessagesConfig.get_async_streaming_response_iterator(self=self, ...) works but is unusual — it silently skips the AmazonAnthropicClaudeMessagesConfig MRO level without stating intent. The idiomatic Python equivalent is super(AmazonAnthropicClaudeMessagesConfig, self).get_async_streaming_response_iterator(...), which makes the MRO-bypass explicit and will continue to work correctly if the class hierarchy is refactored.
| def get_async_streaming_response_iterator( | |
| self, | |
| model: str, | |
| httpx_response: httpx.Response, | |
| request_body: dict, | |
| litellm_logging_obj: LiteLLMLoggingObj, | |
| ) -> AsyncIterator: | |
| return AnthropicMessagesConfig.get_async_streaming_response_iterator( | |
| self=self, | |
| model=model, | |
| httpx_response=httpx_response, | |
| request_body=request_body, | |
| litellm_logging_obj=litellm_logging_obj, | |
| ) | |
| def get_async_streaming_response_iterator( | |
| self, | |
| model: str, | |
| httpx_response: httpx.Response, | |
| request_body: dict, | |
| litellm_logging_obj: LiteLLMLoggingObj, | |
| ) -> AsyncIterator: | |
| return super(AmazonAnthropicClaudeMessagesConfig, self).get_async_streaming_response_iterator( | |
| model=model, | |
| httpx_response=httpx_response, | |
| request_body=request_body, | |
| litellm_logging_obj=litellm_logging_obj, | |
| ) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
AmazonMantleMessagesConfig actually inherit AnthropicMessagesConfig (w/ AmazonAnthropicClaudeMessagesConfig), and passing self keyword vs positionally never changes dispatch, so your concern is factually wrong. But ig it's more conventional to do self so I'll take the suggestion
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit be402cc. Configure here.
📝 WalkthroughWalkthroughBedrock Mantle chat and messages transformation code now centralizes request body restoration (model and conditional stream flag) via a shared helper, and Mantle messages streaming delegates to AnthropicMessagesConfig's async streaming iterator for native SSE handling instead of AWS binary event-stream parsing. Tests were expanded accordingly. ChangesMantle Streaming Fix
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AmazonMantleMessagesConfig
participant AnthropicMessagesConfig
participant BedrockAPI
Client->>AmazonMantleMessagesConfig: transform_anthropic_messages_request(stream=True)
AmazonMantleMessagesConfig-->>Client: request body with stream=True
Client->>BedrockAPI: POST request
BedrockAPI-->>Client: httpx_response (native SSE)
Client->>AmazonMantleMessagesConfig: get_async_streaming_response_iterator(httpx_response)
AmazonMantleMessagesConfig->>AnthropicMessagesConfig: get_async_streaming_response_iterator(model, httpx_response, request_body)
AnthropicMessagesConfig-->>Client: AsyncIterator of decoded SSE chunks
Related issues: Suggested labels: bug, bedrock, streaming Suggested reviewers: krrishdholakia, ishaan-jaff 🐰 A mantle wrapped in SSE, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_litellm/llms/bedrock/test_mantle.py (1)
496-522: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMonkeypatching
HTTPHandler.post/AsyncHTTPHandler.postclass attributes.These three new tests patch the
postmethod directly on theHTTPHandler/AsyncHTTPHandlerclasses rather than injecting a mocked/custom client instance intolitellm.completion/acompletion/anthropic_messages(which accept aclient=param in most call paths). As per coding guidelines,**/*.{py,pyi}: "Prefer dependency injection over monkeypatching class attributes in tests; pass mocked dependencies into classes instead."#!/bin/bash # Verify whether completion/acompletion/anthropic_messages accept an injectable client param # for the bedrock provider path, to confirm DI is a viable alternative here. rg -n -A5 'def completion\(' litellm/llms/anthropic/chat/handler.py rg -n 'client=' tests/test_litellm/llms/bedrock/test_mantle.pyAlso applies to: 524-557, 559-592
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_litellm/llms/bedrock/test_mantle.py` around lines 496 - 522, The new Mantle streaming tests are monkeypatching HTTPHandler.post and AsyncHTTPHandler.post directly, which should be replaced with dependency injection. Update test_mantle.py so test_mantle_completion_streaming_sends_stream_and_decodes_sse and the related async/message tests pass a mocked client or transport into litellm.completion, litellm.acompletion, or anthropic_messages instead of patching class attributes. Use the existing client= injection path where available, and keep the request/assertion logic the same.Source: Coding guidelines
litellm/llms/bedrock/chat/mantle/transformation.py (1)
128-149: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winExtract the shared Mantle request-restoration helper.
_restore_mantle_body_fieldsrepeats the samestream/modelrestoration logic used inlitellm/llms/bedrock/messages/mantle_transformation.py; a small helper would keep the chat and messages transforms aligned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@litellm/llms/bedrock/chat/mantle/transformation.py` around lines 128 - 149, The Mantle request-restoration logic in _restore_mantle_body_fields is duplicated from the messages transform and should be centralized. Extract the shared stream/model restoration into a common helper used by both Mantle transformation classes, then have _restore_mantle_body_fields delegate to it so chat and messages stay aligned. Keep the existing behavior in get_model_response_iterator and the Mantle transformation flow unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@litellm/llms/bedrock/chat/mantle/transformation.py`:
- Around line 128-149: The Mantle request-restoration logic in
_restore_mantle_body_fields is duplicated from the messages transform and should
be centralized. Extract the shared stream/model restoration into a common helper
used by both Mantle transformation classes, then have
_restore_mantle_body_fields delegate to it so chat and messages stay aligned.
Keep the existing behavior in get_model_response_iterator and the Mantle
transformation flow unchanged.
In `@tests/test_litellm/llms/bedrock/test_mantle.py`:
- Around line 496-522: The new Mantle streaming tests are monkeypatching
HTTPHandler.post and AsyncHTTPHandler.post directly, which should be replaced
with dependency injection. Update test_mantle.py so
test_mantle_completion_streaming_sends_stream_and_decodes_sse and the related
async/message tests pass a mocked client or transport into litellm.completion,
litellm.acompletion, or anthropic_messages instead of patching class attributes.
Use the existing client= injection path where available, and keep the
request/assertion logic the same.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ea251a21-ad4b-4335-bcb7-8ebcb19f301c
📒 Files selected for processing (3)
litellm/llms/bedrock/chat/mantle/transformation.pylitellm/llms/bedrock/messages/mantle_transformation.pytests/test_litellm/llms/bedrock/test_mantle.py
|
This is a fairly important patch, at least for my use case, because the only way to use fable-5 on bedrock is via |
@lzy7071 thanks for letting us know. Merged. Thanks for the reminder! |
Relevant issues
Resolves LIT-4330
Fixes #31845
Linear ticket
Resolves LIT-4190
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Verified end to end against the real Bedrock Mantle us-east-1 endpoint with zero mocks: the litellm proxy was run from source in a fresh venv, once detached at the base commit 4bd579c and once at the PR head be402cc, with the same config, same venv, and same port (61632) for both runs. The config points
mantle-claudeatbedrock/mantle/anthropic.claude-opus-4-7viaaws_profile_name, so requests are SigV4-signed with real AWS credentials and cost real moneyBefore (proxy at base commit 4bd579c)
Non-streaming control, proving config, auth, and model access are all fine
Streaming on /v1/chat/completions fails with HTTP 500 ChecksumMismatch
Streaming on /v1/messages fails the same way
After (proxy at PR head be402cc, same port, same venv, same commands)
Streaming on /v1/chat/completions now returns real SSE chunks ending in [DONE]
Streaming on /v1/messages now returns native Anthropic SSE events from message_start through message_stop
Non-streaming control still works after the fix
Type
Bug Fix
Changes
Streaming requests to
bedrock/mantle/...models failed with an HTTP 500botocore.eventstream.ChecksumMismatcherror on both the/v1/chat/completionsand/v1/messagessurfaces, while non-streaming requests worked fine. Two problems compoundedFirst, the outbound request body never carried
stream. The mantle transforms build the body via the shared Bedrock Invoke Anthropic base, which strips bothmodelandstreambecause Bedrock Invoke puts the model in the URL and streams via the dedicatedinvoke-with-response-streamendpoint. The mantle configs restoredmodelbut notstream, so the bedrock-mantle endpoint (which speaks the native Anthropic Messages API and needs"stream": truein the body) returned a plain JSON response instead of SSE. Both mantle transforms (chat sync/async and the/v1/messagestransform) now restorestreaminto the body when the caller streams, before SigV4 signing so the signature stays validSecond, even with
streamrestored, the response was decoded with botocore's binary AWS event-stream parser, which crashes withChecksumMismatchwhen fed thetext/event-streambytes that the mantle endpoint actually returns. The chat config now opts out of the Bedrock custom stream wrapper and reuses the existing Anthropic SSE iterator (litellm/llms/anthropic/chat/handler.py'sModelResponseIterator), and the/v1/messagesconfig reuses the nativeAnthropicMessagesConfigstreaming iterator that passes the SSE bytes through, instead of theAmazonAnthropicClaudeMessagesStreamDecoderbinary pathRegression tests cover
streambeing present in the transformed body for both surfaces (sync and async), its absence when not streaming, the SSE iterator selection for the chat path, and full mocked-SSE streaming flows forcompletion,acompletion, andanthropic_messagesthat fail with the old binary decoderSummary by CodeRabbit
New Features
Bug Fixes
Note
Medium Risk
Scoped to mantle Bedrock routing and streaming paths with regression tests; wrong decoder or body fields could still break mantle-only streaming without affecting standard Bedrock Invoke.
Overview
Fixes streaming for
bedrock/mantle/...on chat completions and Anthropic/messages, which previously failed withbotocore.eventstream.ChecksumMismatchbecause the client treated mantle responses like Bedrock Invoke binary event streams.Request bodies now re-add
stream: true(along withmodel) when the caller streams, via shared_restore_mantle_body_fieldson the chat config and the same pattern onAmazonMantleMessagesConfig, since the Invoke base strips both fields.Response handling switches mantle chat streaming to Anthropic
ModelResponseIterator(has_custom_stream_wrapper→False) and mantle messages streaming toAnthropicMessagesConfig’s async SSE pass-through instead of the Bedrock binary stream decoder.Tests cover body shaping, iterator choice, and end-to-end mocked SSE for
completion,acompletion, andanthropic_messages.Reviewed by Cursor Bugbot for commit be402cc. Bugbot is set up for automated code reviews on this repo. Configure here.