feat: routing info headers for streaming - #5495
Conversation
|
tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughSummary by CodeRabbit
WalkthroughStreaming routing snapshots are added to request context, updated during fallback and retries, and used to apply routed identity headers before provider headers or SSE chunks across inference and image streaming endpoints. ChangesStreaming routing header flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant handleStreamingRequest
participant BifrostCore
participant ApplyBifrostStreamResponseHeaders
Client->>handleStreamingRequest: request streaming response
handleStreamingRequest->>BifrostCore: start request or retry attempt
BifrostCore->>handleStreamingRequest: return stream and RoutingInfo snapshot
handleStreamingRequest->>ApplyBifrostStreamResponseHeaders: apply request-type and routing headers
ApplyBifrostStreamResponseHeaders->>Client: response headers before chunks
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@transports/bifrost-http/handlers/inference.go`:
- Around line 1936-1938: Move ApplyBifrostStreamResponseHeaders below the
provider-header forwarding block in both
transports/bifrost-http/handlers/inference.go (lines 1936-1938) and
transports/bifrost-http/integrations/router.go (lines 2609-2611), ensuring
routed-identity headers remain authoritative in both streaming paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a120ac19-7609-4752-aefc-8db3d19e46bc
📒 Files selected for processing (7)
core/bifrost.gocore/schemas/bifrost.gocore/schemas/context.gotransports/bifrost-http/handlers/inference.gotransports/bifrost-http/integrations/router.gotransports/bifrost-http/lib/responseheaders.gotransports/bifrost-http/lib/responseheaders_test.go
03a862a to
7fd4fce
Compare
0a42665 to
91e2e15
Compare
Merge activity
|
The base branch was changed.
91e2e15 to
82795cf
Compare
82795cf to
f551da7
Compare
## Summary Streaming responses in Bifrost write HTTP response headers before the first SSE chunk arrives, but routing identity (provider, model, key, fallback status) was only available on per-chunk `ExtraFields`. This meant routed-identity `x-bifrost-*` headers were missing entirely from streaming responses. This PR fixes that by stashing a `RoutingInfo` snapshot into the `BifrostContext` at stream setup time, then reading it in the transport layer to emit the correct headers before the first chunk is written. ## Changes - Added `BifrostContextKeyRoutingInfo` as a new reserved context key. Core writes a `RoutingInfo` snapshot into the context at each stream attempt (overwritten on retry, so the winning attempt's snapshot survives). On successful fallback, the snapshot is updated to reflect `IsFallback`, `PrimaryProvider`, and `PrimaryModel`, mirroring the existing `SetFallbackRoutingInfo` logic. - Added `RoutingInfo.ToExtraFields(requestType)` to build a `BifrostResponseExtraFields` from a finalized `RoutingInfo`, using the same deprecated-triplet sync rules as the non-streaming response path. - Added `ApplyBifrostStreamResponseHeaders` in the transport lib, which reads the context snapshot and calls the existing `ApplyBifrostResponseHeaders` before any SSE write. When no snapshot is present (e.g. a plugin short-circuited the stream), only the request-type header is emitted. - All streaming handler entry points (`handleStreamingTextCompletion`, `handleStreamingChatCompletion`, `handleStreamingResponses`, `handleStreamingSpeech`, `handleStreamingTranscriptionRequest`, `handleStreamingImageGeneration`, `handleStreamingImageEditRequest`) now pass their `RequestType` through to `handleStreamingResponse`, which calls `ApplyBifrostStreamResponseHeaders` after the stream channel is obtained but before any SSE headers are flushed. - The generic router's `handleStreamingRequest` follows the same pattern, tracking `requestType` per branch and calling `ApplyBifrostStreamResponseHeaders` at the same point. - Tests cover the normal identity case, the fallback-layered case (including deprecated header derivation), and the missing-snapshot fallback. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/... ./transports/bifrost-http/... ``` To validate end-to-end: send a streaming chat completion request through the HTTP transport and inspect the response headers. Expect `x-bifrost-routing-info-provider`, `x-bifrost-routing-info-model`, `x-bifrost-routing-info-key`, and `x-bifrost-request-type` to be present on the response before any SSE data is received. For a fallback scenario, also expect `x-bifrost-routing-info-is-fallback: true`, `x-bifrost-routing-info-primary-provider`, and `x-bifrost-routing-info-primary-model`. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations The `RoutingInfo` snapshot stored in context contains provider key identifiers (alias names, not secret values). This is consistent with what was already emitted on non-streaming responses via `ExtraFields`. ## 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 Streaming responses in Bifrost write HTTP response headers before the first SSE chunk arrives, but routing identity (provider, model, key, fallback status) was only available on per-chunk `ExtraFields`. This meant routed-identity `x-bifrost-*` headers were missing entirely from streaming responses. This PR fixes that by stashing a `RoutingInfo` snapshot into the `BifrostContext` at stream setup time, then reading it in the transport layer to emit the correct headers before the first chunk is written. ## Changes - Added `BifrostContextKeyRoutingInfo` as a new reserved context key. Core writes a `RoutingInfo` snapshot into the context at each stream attempt (overwritten on retry, so the winning attempt's snapshot survives). On successful fallback, the snapshot is updated to reflect `IsFallback`, `PrimaryProvider`, and `PrimaryModel`, mirroring the existing `SetFallbackRoutingInfo` logic. - Added `RoutingInfo.ToExtraFields(requestType)` to build a `BifrostResponseExtraFields` from a finalized `RoutingInfo`, using the same deprecated-triplet sync rules as the non-streaming response path. - Added `ApplyBifrostStreamResponseHeaders` in the transport lib, which reads the context snapshot and calls the existing `ApplyBifrostResponseHeaders` before any SSE write. When no snapshot is present (e.g. a plugin short-circuited the stream), only the request-type header is emitted. - All streaming handler entry points (`handleStreamingTextCompletion`, `handleStreamingChatCompletion`, `handleStreamingResponses`, `handleStreamingSpeech`, `handleStreamingTranscriptionRequest`, `handleStreamingImageGeneration`, `handleStreamingImageEditRequest`) now pass their `RequestType` through to `handleStreamingResponse`, which calls `ApplyBifrostStreamResponseHeaders` after the stream channel is obtained but before any SSE headers are flushed. - The generic router's `handleStreamingRequest` follows the same pattern, tracking `requestType` per branch and calling `ApplyBifrostStreamResponseHeaders` at the same point. - Tests cover the normal identity case, the fallback-layered case (including deprecated header derivation), and the missing-snapshot fallback. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/... ./transports/bifrost-http/... ``` To validate end-to-end: send a streaming chat completion request through the HTTP transport and inspect the response headers. Expect `x-bifrost-routing-info-provider`, `x-bifrost-routing-info-model`, `x-bifrost-routing-info-key`, and `x-bifrost-request-type` to be present on the response before any SSE data is received. For a fallback scenario, also expect `x-bifrost-routing-info-is-fallback: true`, `x-bifrost-routing-info-primary-provider`, and `x-bifrost-routing-info-primary-model`. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations The `RoutingInfo` snapshot stored in context contains provider key identifiers (alias names, not secret values). This is consistent with what was already emitted on non-streaming responses via `ExtraFields`. ## 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 Streaming responses in Bifrost write HTTP response headers before the first SSE chunk arrives, but routing identity (provider, model, key, fallback status) was only available on per-chunk `ExtraFields`. This meant routed-identity `x-bifrost-*` headers were missing entirely from streaming responses. This PR fixes that by stashing a `RoutingInfo` snapshot into the `BifrostContext` at stream setup time, then reading it in the transport layer to emit the correct headers before the first chunk is written. ## Changes - Added `BifrostContextKeyRoutingInfo` as a new reserved context key. Core writes a `RoutingInfo` snapshot into the context at each stream attempt (overwritten on retry, so the winning attempt's snapshot survives). On successful fallback, the snapshot is updated to reflect `IsFallback`, `PrimaryProvider`, and `PrimaryModel`, mirroring the existing `SetFallbackRoutingInfo` logic. - Added `RoutingInfo.ToExtraFields(requestType)` to build a `BifrostResponseExtraFields` from a finalized `RoutingInfo`, using the same deprecated-triplet sync rules as the non-streaming response path. - Added `ApplyBifrostStreamResponseHeaders` in the transport lib, which reads the context snapshot and calls the existing `ApplyBifrostResponseHeaders` before any SSE write. When no snapshot is present (e.g. a plugin short-circuited the stream), only the request-type header is emitted. - All streaming handler entry points (`handleStreamingTextCompletion`, `handleStreamingChatCompletion`, `handleStreamingResponses`, `handleStreamingSpeech`, `handleStreamingTranscriptionRequest`, `handleStreamingImageGeneration`, `handleStreamingImageEditRequest`) now pass their `RequestType` through to `handleStreamingResponse`, which calls `ApplyBifrostStreamResponseHeaders` after the stream channel is obtained but before any SSE headers are flushed. - The generic router's `handleStreamingRequest` follows the same pattern, tracking `requestType` per branch and calling `ApplyBifrostStreamResponseHeaders` at the same point. - Tests cover the normal identity case, the fallback-layered case (including deprecated header derivation), and the missing-snapshot fallback. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/... ./transports/bifrost-http/... ``` To validate end-to-end: send a streaming chat completion request through the HTTP transport and inspect the response headers. Expect `x-bifrost-routing-info-provider`, `x-bifrost-routing-info-model`, `x-bifrost-routing-info-key`, and `x-bifrost-request-type` to be present on the response before any SSE data is received. For a fallback scenario, also expect `x-bifrost-routing-info-is-fallback: true`, `x-bifrost-routing-info-primary-provider`, and `x-bifrost-routing-info-primary-model`. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations The `RoutingInfo` snapshot stored in context contains provider key identifiers (alias names, not secret values). This is consistent with what was already emitted on non-streaming responses via `ExtraFields`. ## 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
Streaming responses in Bifrost write HTTP response headers before the first SSE chunk arrives, but routing identity (provider, model, key, fallback status) was only available on per-chunk
ExtraFields. This meant routed-identityx-bifrost-*headers were missing entirely from streaming responses. This PR fixes that by stashing aRoutingInfosnapshot into theBifrostContextat stream setup time, then reading it in the transport layer to emit the correct headers before the first chunk is written.Changes
BifrostContextKeyRoutingInfoas a new reserved context key. Core writes aRoutingInfosnapshot into the context at each stream attempt (overwritten on retry, so the winning attempt's snapshot survives). On successful fallback, the snapshot is updated to reflectIsFallback,PrimaryProvider, andPrimaryModel, mirroring the existingSetFallbackRoutingInfologic.RoutingInfo.ToExtraFields(requestType)to build aBifrostResponseExtraFieldsfrom a finalizedRoutingInfo, using the same deprecated-triplet sync rules as the non-streaming response path.ApplyBifrostStreamResponseHeadersin the transport lib, which reads the context snapshot and calls the existingApplyBifrostResponseHeadersbefore any SSE write. When no snapshot is present (e.g. a plugin short-circuited the stream), only the request-type header is emitted.handleStreamingTextCompletion,handleStreamingChatCompletion,handleStreamingResponses,handleStreamingSpeech,handleStreamingTranscriptionRequest,handleStreamingImageGeneration,handleStreamingImageEditRequest) now pass theirRequestTypethrough tohandleStreamingResponse, which callsApplyBifrostStreamResponseHeadersafter the stream channel is obtained but before any SSE headers are flushed.handleStreamingRequestfollows the same pattern, trackingrequestTypeper branch and callingApplyBifrostStreamResponseHeadersat the same point.Type of change
Affected areas
How to test
go test ./core/... ./transports/bifrost-http/...To validate end-to-end: send a streaming chat completion request through the HTTP transport and inspect the response headers. Expect
x-bifrost-routing-info-provider,x-bifrost-routing-info-model,x-bifrost-routing-info-key, andx-bifrost-request-typeto be present on the response before any SSE data is received. For a fallback scenario, also expectx-bifrost-routing-info-is-fallback: true,x-bifrost-routing-info-primary-provider, andx-bifrost-routing-info-primary-model.Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
The
RoutingInfosnapshot stored in context contains provider key identifiers (alias names, not secret values). This is consistent with what was already emitted on non-streaming responses viaExtraFields.Checklist
docs/contributing/README.mdand followed the guidelines