fix(relay/openai): normalize stop field from string to array - #5930
fix(relay/openai): normalize stop field from string to array#5930AinzRimuru wants to merge 1 commit into
Conversation
Some strict OpenAI-compatible upstreams (Java/Jackson based, with stop declared as ArrayList and ACCEPT_SINGLE_VALUE_AS_ARRAY disabled) reject a single-string stop with HTTP 400. Convert a non-empty string stop to a string slice before forwarding. Mainstream upstreams (OpenAI, Azure, etc.) accept the array form equally well.
WalkthroughAdds normalization logic in ChangesStop field normalization
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
There was a problem hiding this comment.
Pull request overview
This PR updates the OpenAI relay adaptor to normalize the stop request field from a single string into a one-element array when needed, improving compatibility with stricter OpenAI-compatible upstream implementations (e.g., Jackson-based servers that reject single-value inputs for array-typed fields).
Changes:
- Normalize
request.Stopwhen it is a non-emptystringby converting it to[]string{stop}. - Add a warning log when this normalization occurs to aid troubleshooting.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 归一化 stop 字段：某些严格的 OpenAI 兼容上游（基于 Java/Jackson，将 stop 声明为 | ||
| // ArrayList 且未启用 ACCEPT_SINGLE_VALUE_AS_ARRAY）收到单字符串会返回 400。 | ||
| // 转成数组形式，OpenAI 官方/Azure 等主流上游同样接受。 | ||
| if s, ok := request.Stop.(string); ok && s != "" { | ||
| logger.LogWarn(c.Request.Context(), fmt.Sprintf( | ||
| "stop field normalized from string to array for channel %d (model: %s)", | ||
| info.ChannelId, info.UpstreamModelName)) | ||
| request.Stop = []string{s} | ||
| } |
| if s, ok := request.Stop.(string); ok && s != "" { | ||
| logger.LogWarn(c.Request.Context(), fmt.Sprintf( | ||
| "stop field normalized from string to array for channel %d (model: %s)", | ||
| info.ChannelId, info.UpstreamModelName)) | ||
| request.Stop = []string{s} | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
relay/channel/openai/adaptor.go (1)
238-240: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider downgrading log level for routine normalization.
LogWarnfires on every request with a stringstopvalue, across all channels. Once clients commonly send string-form stop, this becomes high-volume noise for expected/handled behavior rather than an actionable warning.🤖 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/adaptor.go` around lines 238 - 240, The normalization message in the OpenAI adaptor is too noisy for an expected request shape, so reduce the log level from warning to a lower severity in the code path that logs “stop field normalized from string to array” inside the channel adaptor handling. Update the logger call in the relevant request-processing flow so routine string-form stop normalization is logged as debug/info rather than warn, while keeping the same context fields like channel ID and upstream model name.
🤖 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/adaptor.go`:
- Around line 237-242: The stop normalization in adaptor.go skips empty strings
because of the s != "" guard, so a bare string stop can still be sent upstream
and trigger the same Jackson 400. Update the request.Stop handling in the
adaptor’s normalization block to convert any string value, including empty
string, into a []string, and keep the existing warning/logging around the
normalization path.
---
Nitpick comments:
In `@relay/channel/openai/adaptor.go`:
- Around line 238-240: The normalization message in the OpenAI adaptor is too
noisy for an expected request shape, so reduce the log level from warning to a
lower severity in the code path that logs “stop field normalized from string to
array” inside the channel adaptor handling. Update the logger call in the
relevant request-processing flow so routine string-form stop normalization is
logged as debug/info rather than warn, while keeping the same context fields
like channel ID and upstream model name.
🪄 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: 41209ba6-9db6-4f63-a4c7-c1b64668e66f
📒 Files selected for processing (1)
relay/channel/openai/adaptor.go
| if s, ok := request.Stop.(string); ok && s != "" { | ||
| logger.LogWarn(c.Request.Context(), fmt.Sprintf( | ||
| "stop field normalized from string to array for channel %d (model: %s)", | ||
| info.ChannelId, info.UpstreamModelName)) | ||
| request.Stop = []string{s} | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Empty-string stop is not normalized.
The guard s != "" skips normalization when request.Stop is an empty string. Strict Jackson-based upstreams that reject a bare string stop (the exact issue this PR fixes) would still receive a string type in that case and could return the same 400 error.
🐛 Proposed fix
- if s, ok := request.Stop.(string); ok && s != "" {
+ if s, ok := request.Stop.(string); ok {
logger.LogWarn(c.Request.Context(), fmt.Sprintf(
"stop field normalized from string to array for channel %d (model: %s)",
info.ChannelId, info.UpstreamModelName))
- request.Stop = []string{s}
+ if s == "" {
+ request.Stop = nil
+ } else {
+ request.Stop = []string{s}
+ }
}📝 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.
| if s, ok := request.Stop.(string); ok && s != "" { | |
| logger.LogWarn(c.Request.Context(), fmt.Sprintf( | |
| "stop field normalized from string to array for channel %d (model: %s)", | |
| info.ChannelId, info.UpstreamModelName)) | |
| request.Stop = []string{s} | |
| } | |
| if s, ok := request.Stop.(string); ok { | |
| logger.LogWarn(c.Request.Context(), fmt.Sprintf( | |
| "stop field normalized from string to array for channel %d (model: %s)", | |
| info.ChannelId, info.UpstreamModelName)) | |
| if s == "" { | |
| request.Stop = nil | |
| } else { | |
| request.Stop = []string{s} | |
| } | |
| } |
🤖 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/adaptor.go` around lines 237 - 242, The stop
normalization in adaptor.go skips empty strings because of the s != "" guard, so
a bare string stop can still be sent upstream and trigger the same Jackson 400.
Update the request.Stop handling in the adaptor’s normalization block to convert
any string value, including empty string, into a []string, and keep the existing
warning/logging around the normalization path.
Important
📝 变更描述 / Description
对OpenAI接口的请求体的Stop字段进行归一化处理。将String形式的Stop字段统一转换为Array，以解决部分平台不支持String输入的情况。
但考虑到当前操作涉及对请求体的重写，因此加入了Warn日志以方便排查可能存在的问题，目前本地测试一切正常。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
对应请求不再出现400报错。
Summary by CodeRabbit