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.
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