[fix]: openai provider - add usage to completed event in responses to chat completions fallback - #3519
Merged
akshaydeo merged 4 commits intoMay 15, 2026
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds per-request stream idle-timeout propagation to OpenAI streaming handlers and buffers the final Responses terminal event in the Responses→Chat fallback to aggregate and attach usage before emitting the final event. ChangesOpenAI streaming and fallback updates
Sequence DiagramsequenceDiagram
participant Client
participant HandleOpenAIResponsesStreaming
participant SSEReader
participant FallbackAggregator
participant StreamEmitter
Client ->> HandleOpenAIResponsesStreaming: open stream (includes streamIdleTimeoutInSeconds)
HandleOpenAIResponsesStreaming ->> SSEReader: start SSE read (timeout set)
SSEReader ->> FallbackAggregator: chunked Responses events (partial usage)
FallbackAggregator ->> FallbackAggregator: aggregate usage and buffer terminal event
SSEReader ->> HandleOpenAIResponsesStreaming: end of stream
HandleOpenAIResponsesStreaming ->> FallbackAggregator: finalize usage and attach metadata
FallbackAggregator ->> StreamEmitter: emit finalized completed/incomplete event (with usage)
StreamEmitter ->> Client: deliver terminal event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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" Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Confidence Score: 4/5Safe to merge for the common case; a narrow regression exists where a non-EOF read error after the terminal chunk causes the terminal event to be dropped entirely. The happy path (normal EOF, usage in trailing chunk) is correctly handled and the usageSeen guard prevents phantom zero-usage objects. The one concrete regression is in the non-EOF read-error path: when that error fires after pendingFinalEvent is set but before EOF, the terminal event is silently discarded — a behavior change that wasn't possible before because the old code sent and returned at the terminal event. core/providers/openai/openai.go — the non-EOF read-error return at line 1140 should flush pendingFinalEvent before returning. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@core/providers/openai/openai.go`:
- Around line 1122-1123: The final aggregated usage should only be attached when
a usage chunk was actually observed: add a boolean flag (e.g., sawUsageChunk) in
the streaming/fallback flow where usage is accumulated (the variable usage of
type *schemas.BifrostLLMUsage) and set it true whenever a usage chunk arrives;
when building pendingFinalEvent or assigning usage to any
schemas.BifrostResponsesStreamResponse, guard the assignment with sawUsageChunk
so you only call usage.ToResponsesResponseUsage() and set the usage field if
sawUsageChunk is true (leave it nil otherwise). Update all similar blocks that
assign usage in the fallback stream handling (mentions: pendingFinalEvent logic
and the other spots around the BifrostLLMUsage accumulation and response
construction) to use this flag.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73ea8ded-d1f4-40a7-8886-03aa14b4baf4
📒 Files selected for processing (2)
core/changelog.mdcore/providers/openai/openai.go
|
|
… chat completions fallback
When a Responses streaming request falls back to chat completions, Bifrost streams from the upstream provider (OpenAI, Groq, Mistral, etc.) and translates each chunk into a Responses event for the caller.
The provider emits finish_reason first and usage one chunk later, but Bifrost was forwarding response.completed/response.incomplete as soon as finish_reason arrived — so the caller's terminal event had usage unset.
Upstream chunk order:
...content deltas...
{ ..., "finish_reason": "stop", "usage": null } <- we sent terminal here (bug)
{ ..., "usage": {...} } <- arrives too late
Fix: accumulate usage from every chunk, hold the terminal event until the upstream stream ends, then attach usage before sending. Native chat-completion and Responses streaming paths are unchanged.
Modified files:
- core/providers/openai/openai.go: attach usage information to completed event
Update changelogs:
- core/changelog.md:
[fix]: openai provider - add usage to completed event in responses to chat completions fallback [@kevinpdev](https://github.com/kevinpdev)
…s usage chunks Follow-up to 9581cb6. If a provider in the fallback path never sends usage data, the previous fix would still attach a zero-valued usage object to the final event instead of leaving it null. Only attach usage when we actually saw a usage chunk from upstream. Affected packages: - core/providers/openai: gate fallback terminal usage attach on observed usage chunk Update changelogs: - core/changelog.md: [fix]: openai provider - preserve nil usage when fallback stream omits usage chunks [@kevinpdev](https://github.com/kevinpdev)
kevinpdev
force-pushed
the
fix/responses-to-chat-fallback-usage-streaming
branch
from
May 15, 2026 06:13
e214102 to
194b3c5
Compare
Contributor
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 `@core/providers/openai/openai.go`:
- Around line 1350-1352: When usageSeen is true always overwrite the buffered
terminal event usage with the accumulated usage instead of only when
pendingFinalEvent.Response.Usage == nil; in the block referencing usageSeen and
pendingFinalEvent.Response (the symbols usageSeen, pendingFinalEvent.Response,
and usage.ToResponsesResponseUsage()), remove the nil-guard and assign
pendingFinalEvent.Response.Usage = usage.ToResponsesResponseUsage()
unconditionally so the terminal response.completed/response.incomplete reflects
the aggregated usage (including cost/token fields) every time.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1fcf89f-df31-44af-a4c7-7fa6182fcedb
📒 Files selected for processing (1)
core/providers/openai/openai.go
… fallback Drop the nil guard so the merged usage accumulator always overwrites the terminal event's usage, even if a partial usage struct was already attached. Modified files: - core/providers/openai/openai.go: drop nil guard on terminal usage attach
Pratham-Mishra04
previously approved these changes
May 15, 2026
Contributor
|
❤️ for the PR @kevinpdev |
d3lm
dismissed stale reviews from Pratham-Mishra04 and coderabbitai[bot]
via
May 15, 2026 13:47
2c7c4f6
akshaydeo
pushed a commit
that referenced
this pull request
May 15, 2026
… chat completions fallback (#3519) * [fix]: openai provider - add usage to completed event in responses to chat completions fallback When a Responses streaming request falls back to chat completions, Bifrost streams from the upstream provider (OpenAI, Groq, Mistral, etc.) and translates each chunk into a Responses event for the caller. The provider emits finish_reason first and usage one chunk later, but Bifrost was forwarding response.completed/response.incomplete as soon as finish_reason arrived — so the caller's terminal event had usage unset. Upstream chunk order: ...content deltas... { ..., "finish_reason": "stop", "usage": null } <- we sent terminal here (bug) { ..., "usage": {...} } <- arrives too late Fix: accumulate usage from every chunk, hold the terminal event until the upstream stream ends, then attach usage before sending. Native chat-completion and Responses streaming paths are unchanged. Modified files: - core/providers/openai/openai.go: attach usage information to completed event Update changelogs: - core/changelog.md: [fix]: openai provider - add usage to completed event in responses to chat completions fallback [@kevinpdev](https://github.com/kevinpdev) * [fix]: openai provider - preserve nil usage when fallback stream omits usage chunks Follow-up to 9581cb6. If a provider in the fallback path never sends usage data, the previous fix would still attach a zero-valued usage object to the final event instead of leaving it null. Only attach usage when we actually saw a usage chunk from upstream. Affected packages: - core/providers/openai: gate fallback terminal usage attach on observed usage chunk Update changelogs: - core/changelog.md: [fix]: openai provider - preserve nil usage when fallback stream omits usage chunks [@kevinpdev](https://github.com/kevinpdev) * [fix]: openai provider - always use merged usage on terminal event in fallback Drop the nil guard so the merged usage accumulator always overwrites the terminal event's usage, even if a partial usage struct was already attached. Modified files: - core/providers/openai/openai.go: drop nil guard on terminal usage attach --------- Co-authored-by: Dominic Elm <elmdominic@gmx.net>
akshaydeo
pushed a commit
that referenced
this pull request
May 20, 2026
… chat completions fallback (#3519) * [fix]: openai provider - add usage to completed event in responses to chat completions fallback When a Responses streaming request falls back to chat completions, Bifrost streams from the upstream provider (OpenAI, Groq, Mistral, etc.) and translates each chunk into a Responses event for the caller. The provider emits finish_reason first and usage one chunk later, but Bifrost was forwarding response.completed/response.incomplete as soon as finish_reason arrived — so the caller's terminal event had usage unset. Upstream chunk order: ...content deltas... { ..., "finish_reason": "stop", "usage": null } <- we sent terminal here (bug) { ..., "usage": {...} } <- arrives too late Fix: accumulate usage from every chunk, hold the terminal event until the upstream stream ends, then attach usage before sending. Native chat-completion and Responses streaming paths are unchanged. Modified files: - core/providers/openai/openai.go: attach usage information to completed event Update changelogs: - core/changelog.md: [fix]: openai provider - add usage to completed event in responses to chat completions fallback [@kevinpdev](https://github.com/kevinpdev) * [fix]: openai provider - preserve nil usage when fallback stream omits usage chunks Follow-up to 9581cb6. If a provider in the fallback path never sends usage data, the previous fix would still attach a zero-valued usage object to the final event instead of leaving it null. Only attach usage when we actually saw a usage chunk from upstream. Affected packages: - core/providers/openai: gate fallback terminal usage attach on observed usage chunk Update changelogs: - core/changelog.md: [fix]: openai provider - preserve nil usage when fallback stream omits usage chunks [@kevinpdev](https://github.com/kevinpdev) * [fix]: openai provider - always use merged usage on terminal event in fallback Drop the nil guard so the merged usage accumulator always overwrites the terminal event's usage, even if a partial usage struct was already attached. Modified files: - core/providers/openai/openai.go: drop nil guard on terminal usage attach --------- Co-authored-by: Dominic Elm <elmdominic@gmx.net>
Merged
This was referenced May 26, 2026
This was referenced Jun 5, 2026
This was referenced Jul 2, 2026
Closed
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
5 participants
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.
Summary
When a responses streaming request to bifrost falls back to chat completions, bifrost streams from the upstream provider (OpenAI, Azure Openai, etc.) and translates each chunk into a responses event for the caller.
While many providers emit usage data in the same chunk as the finish_reason, some providers emit a finish_reason first and then usage data one chunk later. Bifrost currenly forwards response.completed/response.incomplete as soon as finish_reason chunk arrives. This makes the caller's terminal event have usage unset if the usage chunk comes after.
This applies to azure foundry and also openai provider (when a model supports chat completions but not responses). This could also fix the bug in other providers if they have the same multiple-chunk behavior.
Upstream chunk order:
...content deltas...
{ ..., "finish_reason": "stop", "usage": null } <- we sent terminal here (bug)
{ ..., "usage": {...} } <- arrives too late
Fix: accumulate usage from every chunk, hold the terminal event until the upstream stream ends, then attach usage before sending. Native chat-completion and Responses streaming paths are unchanged.
The fix is done on openai provider because this will fix any provider that relies on it, including any custom providers built on top of it.
Changes
core/providers/openai/openai.go: in the Responses->Chat Completions fallback path, defer the terminalresponse.completed/response.incompleteevent until the upstream stream ends and attach accumulated usagebefore sending.
core/changelog.md: changelog entry.Type of change
Affected areas
How to test
The easiest reproduction for this is using an OSS model on azure foundry, and making your own custom provider for it. To reproduce you need to be on top of #3505 or you will run into other errors first.
Below is an example of using kimi-k2.6 on azure foundry. Azure foundry sends usage in a separate chunk after the response_completed = true chunk.
Request:
Provider:
Before (see usage field):
After (see usage field):
Screenshots/Recordings
See above
Breaking changes
Yes
No
The change only affects the Responses→Chat Completions fallback streaming path.
Callers previously received a terminal event with usage: null; now they receive the same terminal event with usage populated (when the upstream provides it).
Related issues
Security considerations
No security considerations
Checklist
docs/contributing/README.mdand followed the guidelines