fix(claude): preserve output_config on OpenAI-compat path - #4318
Open
minimAluminiumalism wants to merge 3 commits into
Open
fix(claude): preserve output_config on OpenAI-compat path#4318minimAluminiumalism wants to merge 3 commits into
output_config on OpenAI-compat path#4318minimAluminiumalism wants to merge 3 commits into
Conversation
Contributor
WalkthroughAdded merging of an Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant RelayHandler as Relay Handler
participant DTOConverter as RequestOpenAI2ClaudeMessage
participant MergeHelper as MergeEffortIntoOutputConfig
participant Anthropic as Anthropic API
Client->>RelayHandler: POST /v1/chat/completions with model, messages, output_config
RelayHandler->>RelayHandler: Unmarshal GeneralOpenAIRequest (includes OutputConfig)
RelayHandler->>DTOConverter: RequestOpenAI2ClaudeMessage(textRequest)
alt model has effort suffix
DTOConverter->>MergeHelper: MergeEffortIntoOutputConfig(textRequest.OutputConfig, effort)
MergeHelper-->>DTOConverter: merged OutputConfig (effort set, existing keys preserved)
else plain model
DTOConverter-->>DTOConverter: propagate textRequest.OutputConfig as-is
end
DTOConverter-->>RelayHandler: claudeRequest with OutputConfig
RelayHandler->>RelayHandler: if OutputConfig contains "task_budget" then EnsureBetaHeader(...,"task-budgets-2026-03-13")
RelayHandler->>Anthropic: POST /v1/messages with claudeRequest (includes output_config)
Anthropic-->>RelayHandler: response
RelayHandler-->>Client: proxied response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
relay/channel/claude/relay_claude_output_config_test.go (1)
11-70: LGTM — covers the main passthrough/merge scenarios end-to-end.Tests validate: bare-model passthrough, suffix merge preserving
task_budget, suffix-only effort when no user config, and thinking-suffix merging. One optional extension: a case asserting that a user-suppliedeffortinOutputConfiggets overridden by the suffix-derived value (mirroring the_OverridesExistingEffortunit test at the DTO level) would lock in end-to-end override semantics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/claude/relay_claude_output_config_test.go` around lines 11 - 70, Add an end-to-end test that asserts a suffix-derived effort overrides a user-supplied effort: create a new test (e.g., TestOutputConfig_SuffixOverridesExistingEffort) that calls RequestOpenAI2ClaudeMessage with a model containing a suffix (like "claude-opus-4-7-xhigh") and an OutputConfig that already contains "effort":"low" plus another field such as task_budget; assert the returned cr.Model has the suffix stripped, cr.OutputConfig unmarshals to a map where "effort" equals the suffix-derived value ("xhigh") (not the user-provided "low"), and other fields like "task_budget" are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dto/claude.go`:
- Around line 424-432: The current MergeEffortIntoOutputConfig silently discards
non-object JSON by ignoring common.Unmarshal errors and resetting oc to an empty
map; change the logic in MergeEffortIntoOutputConfig to check the unmarshal
error and the runtime type: call common.Unmarshal into a temporary variable
(e.g., tmp any), if err != nil or tmp is not a map[string]any then return the
original existing payload unchanged (or propagate/log the error) so upstream
invalid/non-object JSON isn't masked; only when unmarshalling succeeds and tmp
asserts to map[string]any populate oc from that map, set oc["effort"]=effort,
then marshal and return.
---
Nitpick comments:
In `@relay/channel/claude/relay_claude_output_config_test.go`:
- Around line 11-70: Add an end-to-end test that asserts a suffix-derived effort
overrides a user-supplied effort: create a new test (e.g.,
TestOutputConfig_SuffixOverridesExistingEffort) that calls
RequestOpenAI2ClaudeMessage with a model containing a suffix (like
"claude-opus-4-7-xhigh") and an OutputConfig that already contains
"effort":"low" plus another field such as task_budget; assert the returned
cr.Model has the suffix stripped, cr.OutputConfig unmarshals to a map where
"effort" equals the suffix-derived value ("xhigh") (not the user-provided
"low"), and other fields like "task_budget" are preserved.
🪄 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: 1771b73f-d806-4639-a4c4-5ae84dcdd9df
📒 Files selected for processing (6)
dto/claude.godto/claude_output_config_test.godto/openai_request.gorelay/channel/claude/relay-claude.gorelay/channel/claude/relay_claude_output_config_test.gorelay/claude_handler.go
Comment on lines
+424
to
+432
| func MergeEffortIntoOutputConfig(existing json.RawMessage, effort string) json.RawMessage { | ||
| oc := map[string]any{} | ||
| if len(existing) > 0 { | ||
| _ = common.Unmarshal(existing, &oc) | ||
| } | ||
| oc["effort"] = effort | ||
| b, _ := common.Marshal(oc) | ||
| return b | ||
| } |
Contributor
There was a problem hiding this comment.
Silent fallback when existing is non-empty but not a JSON object.
If existing is non-empty but unmarshaling into map[string]any fails (e.g., it is a JSON array, number, or malformed), the error is discarded and oc silently resets to an empty map — the user-supplied payload is then dropped and replaced with only {"effort": ...}. For well-formed clients this is a non-issue, but it diverges from the PR's stated goal of surfacing upstream validation errors rather than silently dropping fields. Consider passing existing through unchanged (or logging) when it is non-nil but not a decodable object, so bad input is visible rather than masked.
🛡️ Optional defensive tweak
func MergeEffortIntoOutputConfig(existing json.RawMessage, effort string) json.RawMessage {
oc := map[string]any{}
if len(existing) > 0 {
- _ = common.Unmarshal(existing, &oc)
+ if err := common.Unmarshal(existing, &oc); err != nil {
+ common.SysLog("MergeEffortIntoOutputConfig: existing output_config is not a JSON object, ignoring: " + err.Error())
+ }
}
oc["effort"] = effort
b, _ := common.Marshal(oc)
return b
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dto/claude.go` around lines 424 - 432, The current
MergeEffortIntoOutputConfig silently discards non-object JSON by ignoring
common.Unmarshal errors and resetting oc to an empty map; change the logic in
MergeEffortIntoOutputConfig to check the unmarshal error and the runtime type:
call common.Unmarshal into a temporary variable (e.g., tmp any), if err != nil
or tmp is not a map[string]any then return the original existing payload
unchanged (or propagate/log the error) so upstream invalid/non-object JSON isn't
masked; only when unmarshalling succeeds and tmp asserts to map[string]any
populate oc from that map, set oc["effort"]=effort, then marshal and return.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/channel/claude/adaptor.go`:
- Around line 83-96: EnsureBetaHeader currently checks for c == nil but can
still panic if c.Request or c.Request.Header is nil; update EnsureBetaHeader to
return early if c.Request is nil, and if c.Request.Header is nil initialize it
(c.Request.Header = make(http.Header)) before calling Get/Set so Header.Get/Set
won't panic during tests or ungated call sites.
🪄 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: f4cc977b-e63c-495e-a33a-2b0f2faa3fc5
📒 Files selected for processing (5)
dto/claude.gorelay/channel/claude/adaptor.gorelay/channel/claude/relay-claude.gorelay/channel/claude/relay_claude_output_config_test.gorelay/claude_handler.go
🚧 Files skipped from review as they are similar to previous changes (1)
- relay/channel/claude/relay_claude_output_config_test.go
Comment on lines
+83
to
+96
| func EnsureBetaHeader(c *gin.Context, beta string) { | ||
| if c == nil { | ||
| return | ||
| } | ||
| existing := c.Request.Header.Get("anthropic-beta") | ||
| if strings.Contains(existing, beta) { | ||
| return | ||
| } | ||
| if existing != "" { | ||
| c.Request.Header.Set("anthropic-beta", existing+","+beta) | ||
| } else { | ||
| c.Request.Header.Set("anthropic-beta", beta) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Guard against nil c.Request to avoid a panic.
EnsureBetaHeader already guards c == nil, but if c.Request is nil (e.g., in some test setups or ungated call sites) the c.Request.Header.Get(...) call will panic. A small extra nil check keeps the helper as defensive as its counterpart guard.
🛡️ Proposed tweak
func EnsureBetaHeader(c *gin.Context, beta string) {
- if c == nil {
+ if c == nil || c.Request == nil {
return
}
existing := c.Request.Header.Get("anthropic-beta")📝 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 EnsureBetaHeader(c *gin.Context, beta string) { | |
| if c == nil { | |
| return | |
| } | |
| existing := c.Request.Header.Get("anthropic-beta") | |
| if strings.Contains(existing, beta) { | |
| return | |
| } | |
| if existing != "" { | |
| c.Request.Header.Set("anthropic-beta", existing+","+beta) | |
| } else { | |
| c.Request.Header.Set("anthropic-beta", beta) | |
| } | |
| } | |
| func EnsureBetaHeader(c *gin.Context, beta string) { | |
| if c == nil || c.Request == nil { | |
| return | |
| } | |
| existing := c.Request.Header.Get("anthropic-beta") | |
| if strings.Contains(existing, beta) { | |
| return | |
| } | |
| if existing != "" { | |
| c.Request.Header.Set("anthropic-beta", existing+","+beta) | |
| } else { | |
| c.Request.Header.Set("anthropic-beta", beta) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/claude/adaptor.go` around lines 83 - 96, EnsureBetaHeader
currently checks for c == nil but can still panic if c.Request or
c.Request.Header is nil; update EnsureBetaHeader to return early if c.Request is
nil, and if c.Request.Header is nil initialize it (c.Request.Header =
make(http.Header)) before calling Get/Set so Header.Get/Set won't panic during
tests or ungated call sites.
…ompat path - Add OutputConfig field to GeneralOpenAIRequest so it survives JSON deserialization on /v1/chat/completions - Copy user-supplied OutputConfig into ClaudeRequest before the effort-suffix / thinking-suffix branches run - Replace direct OutputConfig assignment with MergeEffortIntoOutputConfig so the effort key is injected without clobbering task_budget or other user-supplied fields - Add 8 unit / integration tests covering nil, merge, override, and passthrough scenarios Closes QuantumNous#4317
- Auto-inject anthropic-beta: task-budgets-2026-03-13 header when output_config contains task_budget, so OpenAI SDK users don't need to manually set the beta header - Add error logging in MergeEffortIntoOutputConfig when existing output_config is not a valid JSON object (CodeRabbit review feedback) - Add 3 tests for beta header injection (inject, skip, append)
The Claude native path already has client passthrough and admin-config mechanisms for anthropic-beta header. Auto-injection only belongs on the OpenAI-compat conversion path.
minimAluminiumalism
force-pushed
the
fix/output-config-passthrough
branch
from
April 17, 2026 20:15
3acbddf to
9384105
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
relay/channel/claude/relay-claude.go (1)
440-442: Substring match on"task_budget"is fragile.
bytes.Containswill also match when the literal string"task_budget"appears as a JSON value (or inside an escaped nested string) rather than as a top-level key. In practiceOutputConfigis small and user-controlled, so the blast radius is limited, but a structural check is more robust and keeps intent explicit.♻️ Proposed refactor
- if bytes.Contains(claudeRequest.OutputConfig, []byte(`"task_budget"`)) { - EnsureBetaHeader(c, "task-budgets-2026-03-13") - } + if len(claudeRequest.OutputConfig) > 0 { + var oc map[string]json.RawMessage + if err := common.Unmarshal(claudeRequest.OutputConfig, &oc); err == nil { + if _, ok := oc["task_budget"]; ok { + EnsureBetaHeader(c, "task-budgets-2026-03-13") + } + } + }With this change the
bytesimport on line 4 can also be dropped if no other usage remains.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/claude/relay-claude.go` around lines 440 - 442, The current bytes.Contains check on claudeRequest.OutputConfig is brittle; instead parse OutputConfig as JSON (e.g., unmarshal into map[string]json.RawMessage or map[string]interface{}) and test for the presence of the top-level key "task_budget" before calling EnsureBetaHeader(c, "task-budgets-2026-03-13"); also remove the bytes import if it becomes unused. Ensure you handle a non-JSON or empty OutputConfig gracefully (treat as absent) and reference claudeRequest.OutputConfig and EnsureBetaHeader in the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@relay/channel/claude/relay-claude.go`:
- Around line 440-442: The current bytes.Contains check on
claudeRequest.OutputConfig is brittle; instead parse OutputConfig as JSON (e.g.,
unmarshal into map[string]json.RawMessage or map[string]interface{}) and test
for the presence of the top-level key "task_budget" before calling
EnsureBetaHeader(c, "task-budgets-2026-03-13"); also remove the bytes import if
it becomes unused. Ensure you handle a non-JSON or empty OutputConfig gracefully
(treat as absent) and reference claudeRequest.OutputConfig and EnsureBetaHeader
in the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 350d24b2-dcad-45db-979c-3454de5dd266
📒 Files selected for processing (7)
dto/claude.godto/claude_output_config_test.godto/openai_request.gorelay/channel/claude/adaptor.gorelay/channel/claude/relay-claude.gorelay/channel/claude/relay_claude_output_config_test.gorelay/claude_handler.go
✅ Files skipped from review due to trivial changes (1)
- dto/openai_request.go
🚧 Files skipped from review as they are similar to previous changes (4)
- relay/channel/claude/adaptor.go
- dto/claude.go
- dto/claude_output_config_test.go
- relay/channel/claude/relay_claude_output_config_test.go
Calcium-Ion
force-pushed
the
main
branch
2 times, most recently
from
August 30, 2026 15:03
51fdfc5 to
2b6f1df
Compare
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
修复 OpenAI 兼容端点
/v1/chat/completions调用 Claude 模型时，output_config字段（task_budget）被静默丢弃的问题。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
{"id":"msg_01PhJUfX283EfZMik1Ei4GFb","model":"claude-opus-4-7","object":"chat.completion","created":1776451877,"choices":[{"index":0,"message":{"role":"assistant","content":"A "},"finish_reason":"stop"}],"usage":{"prompt_tokens":54,"completion_tokens":5,"total_tokens":59}} HTTP_STATUS: 200Summary by CodeRabbit
New Features
Bug Fixes
Tests