test(e2e): cover Bedrock provider response headers on streaming /v1/messages - #35861
Conversation
…essages Streaming /v1/messages over native Bedrock invoke must forward the upstream provider response headers so a customer can pull x-amzn-requestid off the response to open an AWS support case. The streaming path previously dropped them because a bare async generator could not carry the header context; only the non-streaming path forwarded them (LIT-3724, fix BerriAI#32160). The test registers a bedrock/invoke Claude deployment, streams a /v1/messages call, and asserts the response carries a non-empty llm_provider-x-amzn-requestid header. Adds the matching coverage_registry cell llm.messages.bedrock_invoke.basic.stream.provider_headers.
Greptile SummaryThis PR adds live end-to-end coverage for forwarding the Bedrock request ID header on streaming
Confidence Score: 4/5The PR appears safe to merge after aligning the new coverage ID with the registry's documented assertion vocabulary The live test follows the existing e2e lifecycle and streaming-response contracts, while the only accepted concern is a non-blocking inconsistency in coverage taxonomy Files Needing Attention: tests/e2e/coverage_registry/llm_conversational.yaml
|
| Filename | Overview |
|---|---|
| tests/e2e/coverage_registry/llm_conversational.yaml | Adds the intended coverage cell, but its provider_headers assertion falls outside the documented LLM registry vocabulary |
| tests/e2e/llm_translation/test_messages_e2e.py | Adds a focused live Bedrock streaming regression test using the shared model lifecycle and streaming transport helpers |
Reviews (1): Last reviewed commit: "test(e2e): cover Bedrock provider respon..." | Re-trigger Greptile
| - {id: llm.messages.anthropic.vision.nonstream.works, module: llm, tier: P0, subject_endpoint: messages, route: anthropic, capability: vision, streaming: nonstream, assertions: [works], source: "model_prices json", rationale: "Vision via Messages API"} | ||
| - {id: llm.messages.anthropic.prompt_cache_5m.nonstream.works, module: llm, tier: P0, subject_endpoint: messages, route: anthropic, capability: prompt_cache_5m, streaming: nonstream, assertions: [works], source: "model_prices json", rationale: "Prompt caching via Messages API"} | ||
| - {id: llm.messages.anthropic.thinking.nonstream.works, module: llm, tier: P1, subject_endpoint: messages, route: anthropic, capability: thinking, streaming: nonstream, assertions: [works], source: "model_prices json", rationale: "Extended thinking via Messages API"} | ||
| - {id: llm.messages.bedrock_invoke.basic.stream.provider_headers, module: llm, tier: P1, subject_endpoint: messages, route: bedrock_invoke, capability: basic, streaming: stream, assertions: [works, provider_headers], source: "llms/anthropic/experimental_pass_through/messages/streaming_iterator.py", rationale: "Streaming /v1/messages over native Bedrock invoke must forward provider response headers as llm_provider-* so a customer can pull x-amzn-requestid for an AWS support case; the streaming path previously dropped them (LIT-3724 / #32160)", fail_before_fix: unproven} |
There was a problem hiding this comment.
Nonstandard coverage assertion taxonomy
The new coverage ID uses provider_headers, but the documented LLM registry grammar limits assertions to works, cost_logged, or cache_hit. This creates a nonstandard category that downstream reporting and future registry maintenance cannot interpret consistently; update both this row and its matching covers marker to use the documented taxonomy.
Context Used: CLAUDE.md (source)
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!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
TLDR
Problem this solves:
How it solves it:
Relevant issues
Adds e2e coverage for the fix in #32160 (streaming header forwarding). Internal context: LIT-3724
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
This PR adds an e2e regression test only, no product code changes, so there is no before/after product behavior to curl. The behavior it guards was fixed in #32160 and is already on staging
Honest disclosure on verification. I could not run this test against a live Bedrock-backed gateway in my environment, so the live assertion will run in the e2e stage pipeline rather than here. What I verified instead is that the assertion targets a header that actually reaches the client on the streaming path, by tracing the source rather than trusting a docstring:
litellm/llms/anthropic/experimental_pass_through/messages/streaming_iterator.pywraps the SSE stream inAnthropicMessagesStreamingResponse, carrying_hidden_params["additional_headers"]with the provider headers prefixedllm_provider-*(this is exactly what fix(anthropic_messages): forward provider response headers on streaming /v1/messages responses #32160 added; a bare async generator could not carry them, which is why streaming dropped them before)litellm/proxy/common_request_processing.py:1799-1838reads those hidden params on the streaming branch and merges**additional_headersinto the streaming responsecustom_headers, which become the SSE HTTP response headersStreamingResponse.headers(lowercased), soresult.headers["llm_provider-x-amzn-requestid"]is the right seamThe registry cell is marked
fail_before_fix: unprovenbecause I have not run the red-before/green-after inversion against a live gateway. A reviewer running the QA runbook below on a live proxy can confirm both the pass and, by reverting #32160, the regressionType
✅ Test
Changes
tests/e2e/llm_translation/test_messages_e2e.py: newTestBedrockMessagesProviderHeadersstreaming /v1/messages test over abedrock/invokeClaude deployment, asserting a non-emptyllm_provider-x-amzn-requestidresponse headertests/e2e/coverage_registry/llm_conversational.yaml: new cellllm.messages.bedrock_invoke.basic.stream.provider_headersQA runbook
Final Attestation