fixes buffering flow for bedrock - #4578
Conversation
|
|
|
Warning Review limit reached
More reviews will be available in 16 minutes and 16 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesBedrock streaming gzip fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
2ab70fa to
f03a04a
Compare
Merge activity
|
## Summary Fixes a streaming buffering issue (Closes maximhq#4542) where Bedrock streaming responses arrived in a single burst at the end of generation rather than incrementally. Go's `net/http` transport automatically negotiates gzip encoding, which causes the eventstream to be buffered until the stream completes, collapsing time-to-first-byte (TTFB) to the total generation time. ## Changes - Added `Accept-Encoding: identity` header to all Bedrock streaming requests, preventing Go's `net/http` transport from auto-negotiating gzip compression on the eventstream connection - Removed a duplicate `Accept` header set that was only applied to the IAM auth path - Added `TestChatCompletionStream_StreamsIncrementally_NotBuffered`: a deterministic integration test using a channel-gated fake Bedrock server that verifies the first chunk arrives while the upstream is still held open (pre-fix this times out; post-fix it succeeds) - Added `TestMakeStreamingRequest_SendsIdentityAcceptEncoding`: a guard test that asserts the outbound request carries `Accept-Encoding: identity` rather than Go's default `gzip` ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/bedrock/... -v -run TestChatCompletionStream_StreamsIncrementally_NotBuffered go test ./core/providers/bedrock/... -v -run TestMakeStreamingRequest_SendsIdentityAcceptEncoding go test ./... ``` The buffering test is deterministic and channel-gated — no wall-clock sleeps. Pre-fix, `TestChatCompletionStream_StreamsIncrementally_NotBuffered` fails with a 2-second timeout; post-fix it passes immediately upon receiving the first flushed chunk. ## Screenshots/Recordings N/A ## Breaking changes - [x] No ## Related issues Closes maximhq#4542 ## Security considerations No security implications. The change only affects HTTP content-encoding negotiation for the Bedrock streaming endpoint. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Fixes a streaming buffering issue (Closes #4542) where Bedrock streaming responses arrived in a single burst at the end of generation rather than incrementally. Go's `net/http` transport automatically negotiates gzip encoding, which causes the eventstream to be buffered until the stream completes, collapsing time-to-first-byte (TTFB) to the total generation time. ## Changes - Added `Accept-Encoding: identity` header to all Bedrock streaming requests, preventing Go's `net/http` transport from auto-negotiating gzip compression on the eventstream connection - Removed a duplicate `Accept` header set that was only applied to the IAM auth path - Added `TestChatCompletionStream_StreamsIncrementally_NotBuffered`: a deterministic integration test using a channel-gated fake Bedrock server that verifies the first chunk arrives while the upstream is still held open (pre-fix this times out; post-fix it succeeds) - Added `TestMakeStreamingRequest_SendsIdentityAcceptEncoding`: a guard test that asserts the outbound request carries `Accept-Encoding: identity` rather than Go's default `gzip` ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/bedrock/... -v -run TestChatCompletionStream_StreamsIncrementally_NotBuffered go test ./core/providers/bedrock/... -v -run TestMakeStreamingRequest_SendsIdentityAcceptEncoding go test ./... ``` The buffering test is deterministic and channel-gated — no wall-clock sleeps. Pre-fix, `TestChatCompletionStream_StreamsIncrementally_NotBuffered` fails with a 2-second timeout; post-fix it passes immediately upon receiving the first flushed chunk. ## Screenshots/Recordings N/A ## Breaking changes - [x] No ## Related issues Closes #4542 ## Security considerations No security implications. The change only affects HTTP content-encoding negotiation for the Bedrock streaming endpoint. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Fixes a streaming buffering issue (Closes maximhq#4542) where Bedrock streaming responses arrived in a single burst at the end of generation rather than incrementally. Go's `net/http` transport automatically negotiates gzip encoding, which causes the eventstream to be buffered until the stream completes, collapsing time-to-first-byte (TTFB) to the total generation time. ## Changes - Added `Accept-Encoding: identity` header to all Bedrock streaming requests, preventing Go's `net/http` transport from auto-negotiating gzip compression on the eventstream connection - Removed a duplicate `Accept` header set that was only applied to the IAM auth path - Added `TestChatCompletionStream_StreamsIncrementally_NotBuffered`: a deterministic integration test using a channel-gated fake Bedrock server that verifies the first chunk arrives while the upstream is still held open (pre-fix this times out; post-fix it succeeds) - Added `TestMakeStreamingRequest_SendsIdentityAcceptEncoding`: a guard test that asserts the outbound request carries `Accept-Encoding: identity` rather than Go's default `gzip` ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/bedrock/... -v -run TestChatCompletionStream_StreamsIncrementally_NotBuffered go test ./core/providers/bedrock/... -v -run TestMakeStreamingRequest_SendsIdentityAcceptEncoding go test ./... ``` The buffering test is deterministic and channel-gated — no wall-clock sleeps. Pre-fix, `TestChatCompletionStream_StreamsIncrementally_NotBuffered` fails with a 2-second timeout; post-fix it passes immediately upon receiving the first flushed chunk. ## Screenshots/Recordings N/A ## Breaking changes - [x] No ## Related issues Closes maximhq#4542 ## Security considerations No security implications. The change only affects HTTP content-encoding negotiation for the Bedrock streaming endpoint. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
Fixes a streaming buffering issue (Closes #4542) where Bedrock streaming responses arrived in a single burst at the end of generation rather than incrementally. Go's
net/httptransport automatically negotiates gzip encoding, which causes the eventstream to be buffered until the stream completes, collapsing time-to-first-byte (TTFB) to the total generation time.Changes
Accept-Encoding: identityheader to all Bedrock streaming requests, preventing Go'snet/httptransport from auto-negotiating gzip compression on the eventstream connectionAcceptheader set that was only applied to the IAM auth pathTestChatCompletionStream_StreamsIncrementally_NotBuffered: a deterministic integration test using a channel-gated fake Bedrock server that verifies the first chunk arrives while the upstream is still held open (pre-fix this times out; post-fix it succeeds)TestMakeStreamingRequest_SendsIdentityAcceptEncoding: a guard test that asserts the outbound request carriesAccept-Encoding: identityrather than Go's defaultgzipType of change
Affected areas
How to test
The buffering test is deterministic and channel-gated — no wall-clock sleeps. Pre-fix,
TestChatCompletionStream_StreamsIncrementally_NotBufferedfails with a 2-second timeout; post-fix it passes immediately upon receiving the first flushed chunk.Screenshots/Recordings
N/A
Breaking changes
Related issues
Closes #4542
Security considerations
No security implications. The change only affects HTTP content-encoding negotiation for the Bedrock streaming endpoint.
Checklist
docs/contributing/README.mdand followed the guidelines