fix(openai): harden Chat-to-Responses compatibility - #5772
Merged
Conversation
Add a shared Responses-to-Chat stream state machine and use it from the OpenAI relay path. Preserve assistant text alongside tool calls, bind tool argument deltas by output_index, map incomplete finish reasons, support reasoning/custom tool events, and buffer upstream SSE for non-stream Chat clients. Add deterministic service tests and relay SSE tests for the conversion path. Related to #5745.
Contributor
WalkthroughThis PR updates Responses-to-chat compatibility across shared response mappings, streaming conversion, buffered handling, relay routing, and Claude/Gemini output shaping. It also adds tests for request/response conversion, stream events, buffered reconstruction, and local live-test artifact handling. ChangesResponses-to-chat compatibility
Sequence Diagram(s)sequenceDiagram
participant chatCompletionsViaResponses
participant OaiResponsesToChatStreamHandler
participant OaiResponsesToChatBufferedStreamHandler
participant ResponsesStreamEventToChatChunks
participant FinalizeResponsesToChatStream
participant ResponsesBufferedAccumulator
chatCompletionsViaResponses->>OaiResponsesToChatStreamHandler: clientStream && upstreamStream
chatCompletionsViaResponses->>OaiResponsesToChatBufferedStreamHandler: upstreamStream && !clientStream
OaiResponsesToChatStreamHandler->>ResponsesStreamEventToChatChunks: convert each stream event
ResponsesStreamEventToChatChunks-->>OaiResponsesToChatStreamHandler: chat chunks
OaiResponsesToChatStreamHandler->>FinalizeResponsesToChatStream: emit remaining chunks
OaiResponsesToChatBufferedStreamHandler->>ResponsesBufferedAccumulator: ProcessEvent(data lines)
ResponsesBufferedAccumulator-->>OaiResponsesToChatBufferedStreamHandler: rebuilt output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![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: 5
🧹 Nitpick comments (1)
relay/channel/openai/chat_via_responses_test.go (1)
10-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
assertfor non-fatal value checks.Keep
requirefor setup/fatal checks, but switch value/content assertions toassertso one mismatch does not hide the rest. As per coding guidelines, “New or substantially rewritten Go backend tests MUST userequirefor setup and fatal assertions, andassertfor non-fatal value checks.”Example adjustment
"github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) ... - require.Equal(t, 2, usage.PromptTokens) - require.Equal(t, 3, usage.CompletionTokens) - require.Equal(t, 5, usage.TotalTokens) + assert.Equal(t, 2, usage.PromptTokens) + assert.Equal(t, 3, usage.CompletionTokens) + assert.Equal(t, 5, usage.TotalTokens) ... - require.Contains(t, got, `"role":"assistant"`) + assert.Contains(t, got, `"role":"assistant"`)Also applies to: 63-86, 106-116
🤖 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 `@relay/channel/openai/chat_via_responses_test.go` around lines 10 - 16, The test file currently uses require for value/content checks in chat_via_responses_test.go, which makes later assertions stop running after the first mismatch. Keep require only for setup and fatal preconditions, and change the non-fatal checks in the affected test cases (including the blocks around the mentioned helper/test symbols) to assert so multiple failures can be reported in one run.Source: Coding guidelines
🤖 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 `@relay/channel/openai/chat_via_responses.go`:
- Around line 138-146: The terminal response handling in chat_via_responses.go
only backfills defaults when finalResponse is nil, so partial response objects
can still miss metadata like created and model. Update the response
normalization in the response.done/response.completed path to also fill any
missing fields on an existing finalResponse, using the same defaults currently
set in the nil case (helper.GetResponseID, time.Now().Unix, and
info.UpstreamModelName) before calling accumulator.SupplementResponseOutput.
In `@relay/chat_completions_via_responses.go`:
- Line 149: The upstream stream detection in the routing logic is too strict
because `HasPrefix` on `httpResp.Header.Get("Content-Type")` misses valid SSE
values with case differences or parameters. Update the `upstreamStream` check in
`chat_completions_via_responses.go` to parse the header as a media type (using
the existing or new `mime` import) and compare the media type against
`text/event-stream`, so SSE responses are routed correctly before the JSON
handler.
In `@service/openaicompat/responses_to_chat.go`:
- Around line 785-795: The buffered delta handling in responses_to_chat.go is
storing the same tool-argument fragments under both pendingByOutputIndex and
pendingByItemID, which can cause double replay when ensureTool() later resolves
both keys. Update the responsesEventFunctionArgsDelta and
responsesEventCustomToolInputDelta path to use one canonical pending key for
buffered mode, matching the streaming behavior, and keep the logic in a single
place around a.findToolIndex, pendingByOutputIndex, and pendingByItemID.
- Around line 806-841: BuildOutput currently serializes only a.tools, so
arg-only tool calls left in pendingByOutputIndex/pendingByItemID are dropped in
the non-stream SSE path. Update ResponsesBufferedAccumulator.BuildOutput to
synthesize any remaining buffered tools from those pending maps before
assembling the final dto.ResponsesOutput list, using the same recovery logic as
FinalizeResponsesToChatStream. Keep the existing reasoning/text handling intact
and ensure unmatched tool deltas are converted into function-call outputs even
when no output_item.added/done arrives.
- Around line 437-445: The unmatched tool-delta handling in
findToolForEvent/ensureToolForEvent is queuing the same delta twice when an
event has both OutputIndex and ItemID. Update the logic in responses_to_chat.go
so a delta is stored in only one pending bucket for a given event—prefer a
single canonical key or track whether one identifier was already used before
appending to the second map. Make the change in the tool lookup path around
findToolForEvent and ensureToolForEvent so draining pendingArgsByOutputIndex and
pendingArgsByItemID cannot emit duplicate arguments for the same delta.
---
Nitpick comments:
In `@relay/channel/openai/chat_via_responses_test.go`:
- Around line 10-16: The test file currently uses require for value/content
checks in chat_via_responses_test.go, which makes later assertions stop running
after the first mismatch. Keep require only for setup and fatal preconditions,
and change the non-fatal checks in the affected test cases (including the blocks
around the mentioned helper/test symbols) to assert so multiple failures can be
reported in one run.
🪄 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: de4d5289-dd6d-476c-a2fa-69e2ba17f6fa
📒 Files selected for processing (9)
.gitignoredto/openai_response.gorelay/channel/openai/chat_via_responses.gorelay/channel/openai/chat_via_responses_test.gorelay/chat_completions_via_responses.goservice/convert.goservice/openai_chat_responses_compat.goservice/openaicompat/chat_responses_compat_test.goservice/openaicompat/responses_to_chat.go
Comment on lines
+138
to
+146
| if finalResponse == nil { | ||
| finalResponse = &dto.OpenAIResponsesResponse{ | ||
| ID: helper.GetResponseID(c), | ||
| CreatedAt: int(time.Now().Unix()), | ||
| Model: info.UpstreamModelName, | ||
| Status: []byte(`"completed"`), | ||
| } | ||
| } | ||
| accumulator.SupplementResponseOutput(finalResponse) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Backfill missing metadata on partial terminal responses.
When response.done/response.completed includes a response object with only status/usage/output fields, this skips defaults and can emit created:0 or an empty model in the Chat response.
Proposed fix
if finalResponse == nil {
- finalResponse = &dto.OpenAIResponsesResponse{
- ID: helper.GetResponseID(c),
- CreatedAt: int(time.Now().Unix()),
- Model: info.UpstreamModelName,
- Status: []byte(`"completed"`),
- }
+ finalResponse = &dto.OpenAIResponsesResponse{}
+ }
+ if finalResponse.ID == "" {
+ finalResponse.ID = helper.GetResponseID(c)
+ }
+ if finalResponse.CreatedAt == 0 {
+ finalResponse.CreatedAt = int(time.Now().Unix())
+ }
+ if finalResponse.Model == "" {
+ finalResponse.Model = info.UpstreamModelName
+ }
+ if len(finalResponse.Status) == 0 {
+ finalResponse.Status = []byte(`"completed"`)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if finalResponse == nil { | |
| finalResponse = &dto.OpenAIResponsesResponse{ | |
| ID: helper.GetResponseID(c), | |
| CreatedAt: int(time.Now().Unix()), | |
| Model: info.UpstreamModelName, | |
| Status: []byte(`"completed"`), | |
| } | |
| } | |
| accumulator.SupplementResponseOutput(finalResponse) | |
| if finalResponse == nil { | |
| finalResponse = &dto.OpenAIResponsesResponse{} | |
| } | |
| if finalResponse.ID == "" { | |
| finalResponse.ID = helper.GetResponseID(c) | |
| } | |
| if finalResponse.CreatedAt == 0 { | |
| finalResponse.CreatedAt = int(time.Now().Unix()) | |
| } | |
| if finalResponse.Model == "" { | |
| finalResponse.Model = info.UpstreamModelName | |
| } | |
| if len(finalResponse.Status) == 0 { | |
| finalResponse.Status = []byte(`"completed"`) | |
| } | |
| accumulator.SupplementResponseOutput(finalResponse) |
🤖 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 `@relay/channel/openai/chat_via_responses.go` around lines 138 - 146, The
terminal response handling in chat_via_responses.go only backfills defaults when
finalResponse is nil, so partial response objects can still miss metadata like
created and model. Update the response normalization in the
response.done/response.completed path to also fill any missing fields on an
existing finalResponse, using the same defaults currently set in the nil case
(helper.GetResponseID, time.Now().Unix, and info.UpstreamModelName) before
calling accumulator.SupplementResponseOutput.
| httpResp = resp.(*http.Response) | ||
| info.IsStream = info.IsStream || strings.HasPrefix(httpResp.Header.Get("Content-Type"), "text/event-stream") | ||
| clientStream := info.IsStream | ||
| upstreamStream := strings.HasPrefix(httpResp.Header.Get("Content-Type"), "text/event-stream") |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Parse Content-Type as a media type before routing.
HasPrefix misses valid SSE headers like Text/Event-Stream; charset=utf-8, which would route an upstream SSE body into the non-stream JSON handler.
Proposed fix
- upstreamStream := strings.HasPrefix(httpResp.Header.Get("Content-Type"), "text/event-stream")
+ mediaType, _, parseErr := mime.ParseMediaType(httpResp.Header.Get("Content-Type"))
+ upstreamStream := parseErr == nil && strings.EqualFold(mediaType, "text/event-stream")Also add the mime import if it is not already present.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| upstreamStream := strings.HasPrefix(httpResp.Header.Get("Content-Type"), "text/event-stream") | |
| mediaType, _, parseErr := mime.ParseMediaType(httpResp.Header.Get("Content-Type")) | |
| upstreamStream := parseErr == nil && strings.EqualFold(mediaType, "text/event-stream") |
🤖 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 `@relay/chat_completions_via_responses.go` at line 149, The upstream stream
detection in the routing logic is too strict because `HasPrefix` on
`httpResp.Header.Get("Content-Type")` misses valid SSE values with case
differences or parameters. Update the `upstreamStream` check in
`chat_completions_via_responses.go` to parse the header as a media type (using
the existing or new `mime` import) and compare the media type against
`text/event-stream`, so SSE responses are routed correctly before the JSON
handler.
Comment on lines
+437
to
+445
| tool := s.findToolForEvent(event) | ||
| if tool == nil { | ||
| if event.OutputIndex != nil { | ||
| s.pendingArgsByOutputIndex[*event.OutputIndex] += event.Delta | ||
| } | ||
| if itemID := strings.TrimSpace(event.ItemID); itemID != "" { | ||
| s.pendingArgsByItemID[itemID] += event.Delta | ||
| } | ||
| return nil |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Don't queue the same tool delta twice.
Line 439-444 appends one unmatched delta to both pendingArgsByOutputIndex and pendingArgsByItemID when an event carries both identifiers. ensureToolForEvent() later drains both maps into the same tool, so the emitted arguments get duplicated.
Suggested fix
- if event.OutputIndex != nil {
- s.pendingArgsByOutputIndex[*event.OutputIndex] += event.Delta
- }
- if itemID := strings.TrimSpace(event.ItemID); itemID != "" {
- s.pendingArgsByItemID[itemID] += event.Delta
- }
+ if event.OutputIndex != nil {
+ s.pendingArgsByOutputIndex[*event.OutputIndex] += event.Delta
+ } else if itemID := strings.TrimSpace(event.ItemID); itemID != "" {
+ s.pendingArgsByItemID[itemID] += event.Delta
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| tool := s.findToolForEvent(event) | |
| if tool == nil { | |
| if event.OutputIndex != nil { | |
| s.pendingArgsByOutputIndex[*event.OutputIndex] += event.Delta | |
| } | |
| if itemID := strings.TrimSpace(event.ItemID); itemID != "" { | |
| s.pendingArgsByItemID[itemID] += event.Delta | |
| } | |
| return nil | |
| tool := s.findToolForEvent(event) | |
| if tool == nil { | |
| if event.OutputIndex != nil { | |
| s.pendingArgsByOutputIndex[*event.OutputIndex] += event.Delta | |
| } else if itemID := strings.TrimSpace(event.ItemID); itemID != "" { | |
| s.pendingArgsByItemID[itemID] += event.Delta | |
| } | |
| return nil |
🤖 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 `@service/openaicompat/responses_to_chat.go` around lines 437 - 445, The
unmatched tool-delta handling in findToolForEvent/ensureToolForEvent is queuing
the same delta twice when an event has both OutputIndex and ItemID. Update the
logic in responses_to_chat.go so a delta is stored in only one pending bucket
for a given event—prefer a single canonical key or track whether one identifier
was already used before appending to the second map. Make the change in the tool
lookup path around findToolForEvent and ensureToolForEvent so draining
pendingArgsByOutputIndex and pendingArgsByItemID cannot emit duplicate arguments
for the same delta.
Comment on lines
+785
to
+795
| case responsesEventFunctionArgsDelta, responsesEventCustomToolInputDelta: | ||
| if idx, ok := a.findToolIndex(event); ok { | ||
| a.tools[idx].Arguments.WriteString(event.Delta) | ||
| return | ||
| } | ||
| if event.OutputIndex != nil { | ||
| a.pendingByOutputIndex[*event.OutputIndex] += event.Delta | ||
| } | ||
| if itemID := strings.TrimSpace(event.ItemID); itemID != "" { | ||
| a.pendingByItemID[itemID] += event.Delta | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use one canonical pending key in buffered mode too.
Line 790-795 has the same double-buffer path as the streaming state. If a later response.output_item.added resolves both output_index and item_id, ensureTool() replays both pending entries and the buffered JSON path returns duplicated tool arguments.
Suggested fix
- if event.OutputIndex != nil {
- a.pendingByOutputIndex[*event.OutputIndex] += event.Delta
- }
- if itemID := strings.TrimSpace(event.ItemID); itemID != "" {
- a.pendingByItemID[itemID] += event.Delta
- }
+ if event.OutputIndex != nil {
+ a.pendingByOutputIndex[*event.OutputIndex] += event.Delta
+ } else if itemID := strings.TrimSpace(event.ItemID); itemID != "" {
+ a.pendingByItemID[itemID] += event.Delta
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case responsesEventFunctionArgsDelta, responsesEventCustomToolInputDelta: | |
| if idx, ok := a.findToolIndex(event); ok { | |
| a.tools[idx].Arguments.WriteString(event.Delta) | |
| return | |
| } | |
| if event.OutputIndex != nil { | |
| a.pendingByOutputIndex[*event.OutputIndex] += event.Delta | |
| } | |
| if itemID := strings.TrimSpace(event.ItemID); itemID != "" { | |
| a.pendingByItemID[itemID] += event.Delta | |
| } | |
| case responsesEventFunctionArgsDelta, responsesEventCustomToolInputDelta: | |
| if idx, ok := a.findToolIndex(event); ok { | |
| a.tools[idx].Arguments.WriteString(event.Delta) | |
| return | |
| } | |
| if event.OutputIndex != nil { | |
| a.pendingByOutputIndex[*event.OutputIndex] += event.Delta | |
| } else if itemID := strings.TrimSpace(event.ItemID); itemID != "" { | |
| a.pendingByItemID[itemID] += event.Delta | |
| } |
🤖 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 `@service/openaicompat/responses_to_chat.go` around lines 785 - 795, The
buffered delta handling in responses_to_chat.go is storing the same
tool-argument fragments under both pendingByOutputIndex and pendingByItemID,
which can cause double replay when ensureTool() later resolves both keys. Update
the responsesEventFunctionArgsDelta and responsesEventCustomToolInputDelta path
to use one canonical pending key for buffered mode, matching the streaming
behavior, and keep the logic in a single place around a.findToolIndex,
pendingByOutputIndex, and pendingByItemID.
Comment on lines
+806
to
+841
| func (a *ResponsesBufferedAccumulator) BuildOutput() []dto.ResponsesOutput { | ||
| if a == nil { | ||
| return nil | ||
| } | ||
| out := make([]dto.ResponsesOutput, 0, 2+len(a.tools)) | ||
| if a.reasoning.Len() > 0 { | ||
| out = append(out, dto.ResponsesOutput{ | ||
| Type: responsesOutputTypeReasoning, | ||
| Content: []dto.ResponsesOutputContent{ | ||
| {Type: "summary_text", Text: a.reasoning.String()}, | ||
| }, | ||
| }) | ||
| } | ||
| if a.text.Len() > 0 { | ||
| out = append(out, dto.ResponsesOutput{ | ||
| Type: responsesOutputTypeMessage, | ||
| Role: "assistant", | ||
| Content: []dto.ResponsesOutputContent{ | ||
| {Type: "output_text", Text: a.text.String()}, | ||
| }, | ||
| }) | ||
| } | ||
| for _, tool := range a.tools { | ||
| if tool == nil { | ||
| continue | ||
| } | ||
| argsRaw, _ := common.Marshal(tool.Arguments.String()) | ||
| out = append(out, dto.ResponsesOutput{ | ||
| Type: responsesOutputTypeFunctionCall, | ||
| ID: tool.ItemID, | ||
| CallId: tool.CallID, | ||
| Name: tool.Name, | ||
| Arguments: argsRaw, | ||
| }) | ||
| } | ||
| return out |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Buffered reconstruction still drops arg-only tools.
BuildOutput() only serializes a.tools. But ProcessEvent() can leave unmatched deltas in pendingByOutputIndex / pendingByItemID when the stream ends before any output_item.added/done arrives, so the non-stream SSE path loses that tool call entirely.
Suggested fix
func (a *ResponsesBufferedAccumulator) BuildOutput() []dto.ResponsesOutput {
if a == nil {
return nil
}
+ a.flushPendingTools()
out := make([]dto.ResponsesOutput, 0, 2+len(a.tools))That helper should synthesize buffered tools from the pending maps before the final output is built, mirroring FinalizeResponsesToChatStream().
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (a *ResponsesBufferedAccumulator) BuildOutput() []dto.ResponsesOutput { | |
| if a == nil { | |
| return nil | |
| } | |
| out := make([]dto.ResponsesOutput, 0, 2+len(a.tools)) | |
| if a.reasoning.Len() > 0 { | |
| out = append(out, dto.ResponsesOutput{ | |
| Type: responsesOutputTypeReasoning, | |
| Content: []dto.ResponsesOutputContent{ | |
| {Type: "summary_text", Text: a.reasoning.String()}, | |
| }, | |
| }) | |
| } | |
| if a.text.Len() > 0 { | |
| out = append(out, dto.ResponsesOutput{ | |
| Type: responsesOutputTypeMessage, | |
| Role: "assistant", | |
| Content: []dto.ResponsesOutputContent{ | |
| {Type: "output_text", Text: a.text.String()}, | |
| }, | |
| }) | |
| } | |
| for _, tool := range a.tools { | |
| if tool == nil { | |
| continue | |
| } | |
| argsRaw, _ := common.Marshal(tool.Arguments.String()) | |
| out = append(out, dto.ResponsesOutput{ | |
| Type: responsesOutputTypeFunctionCall, | |
| ID: tool.ItemID, | |
| CallId: tool.CallID, | |
| Name: tool.Name, | |
| Arguments: argsRaw, | |
| }) | |
| } | |
| return out | |
| func (a *ResponsesBufferedAccumulator) BuildOutput() []dto.ResponsesOutput { | |
| if a == nil { | |
| return nil | |
| } | |
| a.flushPendingTools() | |
| out := make([]dto.ResponsesOutput, 0, 2+len(a.tools)) | |
| if a.reasoning.Len() > 0 { | |
| out = append(out, dto.ResponsesOutput{ | |
| Type: responsesOutputTypeReasoning, | |
| Content: []dto.ResponsesOutputContent{ | |
| {Type: "summary_text", Text: a.reasoning.String()}, | |
| }, | |
| }) | |
| } | |
| if a.text.Len() > 0 { | |
| out = append(out, dto.ResponsesOutput{ | |
| Type: responsesOutputTypeMessage, | |
| Role: "assistant", | |
| Content: []dto.ResponsesOutputContent{ | |
| {Type: "output_text", Text: a.text.String()}, | |
| }, | |
| }) | |
| } | |
| for _, tool := range a.tools { | |
| if tool == nil { | |
| continue | |
| } | |
| argsRaw, _ := common.Marshal(tool.Arguments.String()) | |
| out = append(out, dto.ResponsesOutput{ | |
| Type: responsesOutputTypeFunctionCall, | |
| ID: tool.ItemID, | |
| CallId: tool.CallID, | |
| Name: tool.Name, | |
| Arguments: argsRaw, | |
| }) | |
| } | |
| return out | |
| } |
🤖 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 `@service/openaicompat/responses_to_chat.go` around lines 806 - 841,
BuildOutput currently serializes only a.tools, so arg-only tool calls left in
pendingByOutputIndex/pendingByItemID are dropped in the non-stream SSE path.
Update ResponsesBufferedAccumulator.BuildOutput to synthesize any remaining
buffered tools from those pending maps before assembling the final
dto.ResponsesOutput list, using the same recovery logic as
FinalizeResponsesToChatStream. Keep the existing reasoning/text handling intact
and ensure unmatched tool deltas are converted into function-call outputs even
when no output_item.added/done arrives.
11 tasks
This was referenced Jul 6, 2026
liulixin-lex
pushed a commit
to liulixin-lex/xy-api
that referenced
this pull request
Jul 7, 2026
Add a shared Responses-to-Chat stream state machine and use it from the OpenAI relay path. Preserve assistant text alongside tool calls, bind tool argument deltas by output_index, map incomplete finish reasons, support reasoning/custom tool events, and buffer upstream SSE for non-stream Chat clients. Add deterministic service tests and relay SSE tests for the conversion path. Related to QuantumNous#5745.
0401lucky
pushed a commit
to 0401lucky/new-api
that referenced
this pull request
Jul 7, 2026
Add a shared Responses-to-Chat stream state machine and use it from the OpenAI relay path. Preserve assistant text alongside tool calls, bind tool argument deltas by output_index, map incomplete finish reasons, support reasoning/custom tool events, and buffer upstream SSE for non-stream Chat clients. Add deterministic service tests and relay SSE tests for the conversion path. Related to QuantumNous#5745. (cherry picked from commit 3a506f5)
haojianggod
pushed a commit
to Hermerz/new-api
that referenced
this pull request
Jul 10, 2026
OpenAI's Responses API uses incomplete_details.reason, not .reasoning. The field had no readers yet; the hardened Responses->Chat conversion (next commits, upstream QuantumNous#5772) reads it. Upstream: QuantumNous/new-api 3a506f5 (dto portion) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
haojianggod
pushed a commit
to Hermerz/new-api
that referenced
this pull request
Jul 10, 2026
…uantumNous#5772) Replace the thin 133-line Responses->Chat response converter with upstream's hardened version: a streaming state machine (NewResponsesToChatStreamState, ResponsesStreamEventToChatChunks, FinalizeResponsesToChatStream), a buffered accumulator for non-stream clients, incomplete-status finish-reason mapping (ResponsesFinishReasonFromStatus + IncompleteDetails.Reason), reasoning and custom-tool events, and tool-argument deltas bound by output_index (so concurrent tool calls don't collide). This upgrades the Chat-client -> Responses-upstream path (chat completions served by a Responses-only upstream, e.g. the ChatGPT/Codex backend) from the old thin non-streaming-only converter. Adapted to the fork: Message.ReasoningContent is a plain string here (the pointerization wasn't synced), and NewStreamScanner is added to relay/helper (the fork already had InitialScannerBufferSize / getScannerBufferSize, just not the wrapper). The fork's raw-JSON tool-arguments handling (ArgumentsString) is already present in the upstream version. Ported upstream's 343-line state-machine test suite. Upstream: QuantumNous/new-api 3a506f5 (QuantumNous#5772) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
haojianggod
pushed a commit
to Hermerz/new-api
that referenced
this pull request
Jul 10, 2026
… handler
Wire chat_via_responses into the hardened state machine and fix the
client-vs-upstream stream mismatch. Previously info.IsStream was OR'd with the
upstream Content-Type, so a NON-streaming Chat client against an SSE-only
Responses upstream got its response streamed back — violating the client's
non-stream request. Now:
- client stream + upstream SSE -> OaiResponsesToChatStreamHandler
- client non-stream + upstream SSE -> OaiResponsesToChatBufferedStreamHandler
(buffer the SSE into one Chat response)
- non-stream upstream -> OaiResponsesToChatHandler
The handler port keeps the fork's StreamStatus callback
(func(data string, sr *helper.StreamResult) + sr.Stop/sr.Error), and the
request-building fork changes (applySystemPromptIfNeeded, RemoveDisabledFields,
ApplyParamOverrideWithRelayInfo) are untouched.
convert.go's ResponseOpenAI2Claude tweak from the same PR is deliberately
skipped: it's unrelated to Chat<->Responses and touches the fork's heavily
diverged Claude/Gemini conversion.
Upstream: QuantumNous/new-api 3a506f5 (QuantumNous#5772)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
haojianggod
pushed a commit
to Hermerz/new-api
that referenced
this pull request
Jul 10, 2026
…ous#5772) 让 /v1/chat/completions 的 Chat 客户端能落到 Responses-only 上游(ChatGPT/Codex 后端）： Chat 请求 → 转 Responses → 上游 → Responses 响应 → 转回合法 Chat。 - port 上游 QuantumNous#5772 加固版状态机（responses_to_chat.go 850 行 + 上游 471 行测试） - 二开保留：StreamStatus 中断记录、param override、raw-json args、usage→计费 - Q4 修复：乱序参数 delta 不再双缓冲重复 - e2e 已在 staging 验证：Chat 请求回来是合法 Chat 流（resp_ id 证明走了 Responses 后端） WSS 死代码正交独立，未动。
11 tasks
ruanhangjian
pushed a commit
to ruanhangjian/new-api
that referenced
this pull request
Jul 11, 2026
Add a shared Responses-to-Chat stream state machine and use it from the OpenAI relay path. Preserve assistant text alongside tool calls, bind tool argument deltas by output_index, map incomplete finish reasons, support reasoning/custom tool events, and buffer upstream SSE for non-stream Chat clients. Add deterministic service tests and relay SSE tests for the conversion path. Related to QuantumNous#5745.
This was referenced Jul 15, 2026
2 tasks
noah-wung
pushed a commit
to noah-wung/new-api
that referenced
this pull request
Jul 17, 2026
Add a shared Responses-to-Chat stream state machine and use it from the OpenAI relay path. Preserve assistant text alongside tool calls, bind tool argument deltas by output_index, map incomplete finish reasons, support reasoning/custom tool events, and buffer upstream SSE for non-stream Chat clients. Add deterministic service tests and relay SSE tests for the conversion path. Related to QuantumNous#5745.
11 tasks
zhaodechao2008
pushed a commit
to zhaodechao2008/new-api
that referenced
this pull request
Jul 27, 2026
Add a shared Responses-to-Chat stream state machine and use it from the OpenAI relay path. Preserve assistant text alongside tool calls, bind tool argument deltas by output_index, map incomplete finish reasons, support reasoning/custom tool events, and buffer upstream SSE for non-stream Chat clients. Add deterministic service tests and relay SSE tests for the conversion path. Related to QuantumNous#5745.
isboyjc
added a commit
to isboyjc/amux-api
that referenced
this pull request
Aug 14, 2026
模型在调工具前先说一句话时，工具调用会被整个静默丢弃，客户端只收到一段文字 和 finish_reason=stop，agent 无事可执行就停止对话。gpt-5.x 几乎每次调工具前 都有前言，所以这是必现的。 Responses API 的事件顺序固定为先 output_text.delta 后 function_call，因此 sendToolCallDelta 里那道 `outputText.Len() > 0 就直接返回` 的闸必定成立： - 工具调用及其参数增量全部丢弃 - sawToolCall 留在 false（赋值在被跳过的分支之后），finish_reason 判成 stop 非流式路径同病：要求 text == "" 才提取工具调用，且有工具调用时把 Content 抹成空，二者永远不能共存。而 Chat Completions 的 assistant 消息本就允许 content 与 tool_calls 并存。 改动： - 删掉流式那道闸；finish_reason 改为只看 sawToolCall，不看有没有文本 - 非流式去掉 text == "" 前置条件，并保留 Content 对应上游 3a506f5 (QuantumNous#5772) "Preserve assistant text alongside tool calls"， 该提交在我们同步点之后。此处只做等效的最小修复，未移植其整套状态机（output_index 绑定、custom tool 事件、非流式 SSE 缓冲仍与上游有差异）。 新增回归测试，已验证在修复前均失败： - 流式：前言文本 + 工具调用共存、纯工具调用、并行工具调用各自 index、 工具调用后被截断 - 非流式：文本与工具调用共存、纯工具调用 Claude 格式客户端同样受益：该路径此前也丢工具调用，转换器的 stopOpenBlocksAndAdvance 本就能处理文本块到工具块的切换。
330079598
pushed a commit
to 330079598/new-api
that referenced
this pull request
Aug 19, 2026
Add a shared Responses-to-Chat stream state machine and use it from the OpenAI relay path. Preserve assistant text alongside tool calls, bind tool argument deltas by output_index, map incomplete finish reasons, support reasoning/custom tool events, and buffer upstream SSE for non-stream Chat clients. Add deterministic service tests and relay SSE tests for the conversion path. Related to QuantumNous#5745.
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
1 participant
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.
Important
📝 变更描述 / Description
完善 ChatCompletions -> Responses -> ChatCompletions 兼容链路，将 Responses 流式回转逻辑收敛到
service/openaicompat的状态机中，relay 层只负责 SSE 读写、格式回转和 usage 回传。主要修复：
tool_calls并存，不再因为工具调用清空正文。output_index归位，并兼容item_id/call_id。call_id、EOF finalize 等场景。response.incomplete的结束原因：max_output_tokens->length，content_filter->content_filter。function_call与custom_tool_call，以及对应参数 delta。Related to #5745. That PR surfaced overlapping Responses streaming tool-call compatibility gaps; this PR implements the fix independently around a shared state machine and adds broader deterministic/relay coverage.
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
已运行并通过：
go test ./service/openaicompat ./relay/channel/openai ./relay/...关键覆盖：
n>1拒绝。output_index工具参数归位、delta 先到 item 后到、custom tool、reasoning delta、terminal done/completed/incomplete/failed、EOF finalize。[DONE]。Summary by CodeRabbit
New Features
Bug Fixes