fix(openai): tolerate object response arguments - #4445
Closed
gaoren002 wants to merge 1 commit into
Closed
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 (8)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughIntroduces a new dto.ResponseArguments type with custom JSON (un)marshalling and replaces plain string usage for function/tool-call arguments across DTOs, relay channels (OpenAI, Claude, Gemini, Ollama), and service utilities to ensure consistent argument handling. Changes
Sequence Diagram(s)(Skipped — changes are DTO/type replacements and small control-flow adaptations; no new multi-actor sequential feature introduced.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
🧹 Nitpick comments (1)
dto/openai_response_test.go (1)
8-28: Consider extending coverage tonull/empty and array forms.The two existing cases cover the main regression, but since
UnmarshalJSONalso handlesnull/empty input (should become"") and valid non-object JSON (e.g. arrays), a couple of additional sub-tests would lock in that contract and prevent accidental regressions to these branches.🧪 Suggested additional cases
func TestResponseArgumentsNullAndEmpty(t *testing.T) { var fn FunctionResponse if err := json.Unmarshal([]byte(`{"name":"x","arguments":null}`), &fn); err != nil { t.Fatalf("null arguments should unmarshal: %v", err) } if got := fn.Arguments.String(); got != "" { t.Fatalf("expected empty string for null, got %q", got) } } func TestResponsesOutputArgumentsAcceptsArray(t *testing.T) { var output ResponsesOutput if err := json.Unmarshal([]byte(`{"type":"function_call","arguments":[1,2,3]}`), &output); err != nil { t.Fatalf("Unmarshal ResponsesOutput failed: %v", err) } if got := output.Arguments.String(); got != `[1,2,3]` { t.Fatalf("unexpected arguments: %s", got) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dto/openai_response_test.go` around lines 8 - 28, Add two unit tests to cover the null/empty and array forms: create TestResponseArgumentsNullAndEmpty which unmarshals {"name":"x","arguments":null} into FunctionResponse and asserts fn.Arguments.String() == "" and that unmarshaling returns no error, and create TestResponsesOutputArgumentsAcceptsArray which unmarshals {"type":"function_call","arguments":[1,2,3]} into ResponsesOutput and asserts output.Arguments.String() == `[1,2,3]`; reference the FunctionResponse and ResponsesOutput types and their Arguments.String()/UnmarshalJSON behavior to ensure these branches remain covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@dto/openai_response_test.go`:
- Around line 8-28: Add two unit tests to cover the null/empty and array forms:
create TestResponseArgumentsNullAndEmpty which unmarshals
{"name":"x","arguments":null} into FunctionResponse and asserts
fn.Arguments.String() == "" and that unmarshaling returns no error, and create
TestResponsesOutputArgumentsAcceptsArray which unmarshals
{"type":"function_call","arguments":[1,2,3]} into ResponsesOutput and asserts
output.Arguments.String() == `[1,2,3]`; reference the FunctionResponse and
ResponsesOutput types and their Arguments.String()/UnmarshalJSON behavior to
ensure these branches remain covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f57efd22-930f-4ee3-8299-aa47d5cedff2
📒 Files selected for processing (8)
dto/openai_response.godto/openai_response_test.gorelay/channel/claude/relay-claude.gorelay/channel/gemini/relay-gemini.gorelay/channel/ollama/stream.gorelay/channel/openai/chat_via_responses.gorelay/channel/openai/helper.goservice/convert.go
gaoren002
force-pushed
the
fix/response-arguments-object
branch
from
April 24, 2026 16:36
e90a0b2 to
e9f245e
Compare
Contributor
Author
|
我正在确认该PR是否必要，可能是不正常使用导致的 |
Contributor
Author
|
此PR并无必要 |
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.
变更描述 / Description
本 PR 修复 OpenAI Responses 流式/转换链路中
arguments字段只能按字符串解析的问题。目前代码里
FunctionResponse.Arguments和ResponsesOutput.Arguments都按string建模。但在 Responses API 的部分工具调用事件里，上游可能返回对象格式的arguments，例如：{ "type": "function_call", "arguments": { "query": "hello", "limit": 3 } }这种 payload 在 Go 里反序列化到
string会失败，进而导致响应转换中断。实际表现可能是流式响应提前断开、上游响应无法正常转换，或者下游客户端看到解码/连接中断类错误。本 PR 新增
dto.ResponseArguments类型，使arguments同时兼容两种输入："{\"query\":\"hello\"}"{"query":"hello","limit":3}内部仍统一保存为 JSON 字符串，并且
MarshalJSON继续输出字符串，避免影响现有 OpenAI Chat Completions / Claude / Gemini / Ollama 转换链路对工具参数字符串的预期。同时，本 PR 把相关转换代码改为显式使用
ResponseArguments(...)或.String()，避免类型变化后在 Claude、Gemini、Ollama、OpenAI Responses 转换路径中出现不一致。为什么之前一直没发现？
这个问题比较隐蔽，主要原因是历史测试和常见请求都覆盖在
arguments为字符串的路径上：tool_calls[].function.arguments通常就是字符串。arguments。ResponsesOutput.arguments为 JSON object 的反序列化场景。因此正常聊天、普通工具调用、以及大部分 provider 转换都不会触发该错误。只有当 Responses 流式事件中出现对象型
arguments时，才会暴露出结构体字段类型过窄的问题，需要足够多的请求来发现此问题。本 PR 增加了对象型
arguments的回归测试，避免之后再把该字段收窄回纯string。变更类型 / Type of change
关联任务 / Related Issue
提交前检查项 / Checklist
arguments入站解析，出站仍保持字符串格式，降低兼容风险。运行证明 / Proof of Work
本地验证命令：
go test ./dto ./relay/channel/openai ./service结果：
Summary by CodeRabbit
Refactoring
Tests