fix(relay): sanitize Opus 4.7 params for bare model and reasoning_effort paths - #4337
fix(relay): sanitize Opus 4.7 params for bare model and reasoning_effort paths#4337forgottener wants to merge 3 commits into
Conversation
…ort paths Claude Opus 4.7 rejects non-default temperature/top_p/top_k with HTTP 400 "Improperly formed request". The existing effort-suffix and -thinking branches already handle this, but two code paths were missed: 1. Bare `claude-opus-4-7` (no suffix) — client-supplied temperature/top_p/top_k are forwarded as-is, causing 400. Fix: unconditionally clear these params after the effort/thinking branches for any claude-opus-4-7 prefix model. 2. `reasoning_effort` / `reasoning` parameters — these set `thinking.type="enabled"` which Opus 4.7 also rejects. Fix: convert thinking.type to "adaptive" (with display="summarized") for Opus 4.7 after the reasoning params are applied. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 (1)
WalkthroughAdded model-specific normalization for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
relay/channel/claude/relay-claude.go (2)
249-256: Effort granularity is dropped when convertingenabled→adaptive.The conversion is correct for satisfying the Opus 4.7 schema, but a user-supplied
reasoning_effortoflow/medium/high(or areasoning.max_tokens) is effectively discarded here: theBudgetTokensmapping set at lines 217/222/227 (and 242-245) is cleared without a correspondingOutputConfig={"effort":"<level>"}translation. Compare the effort-suffix branch (line 162) which does preserve effort viaOutputConfig. Consider mappingreasoning_effort→OutputConfigfor Opus 4.7 so the user's requested effort level is still signaled to the upstream.♻️ Proposed mapping
// Opus 4.7 rejects thinking.type="enabled"; convert to adaptive if strings.HasPrefix(claudeRequest.Model, "claude-opus-4-7") && claudeRequest.Thinking != nil && claudeRequest.Thinking.Type == "enabled" { claudeRequest.Thinking.Type = "adaptive" claudeRequest.Thinking.BudgetTokens = nil if claudeRequest.Thinking.Display == "" { claudeRequest.Thinking.Display = "summarized" } + if len(claudeRequest.OutputConfig) == 0 && textRequest.ReasoningEffort != "" { + claudeRequest.OutputConfig = json.RawMessage(fmt.Sprintf(`{"effort":"%s"}`, textRequest.ReasoningEffort)) + } }Please confirm with the Anthropic Opus 4.7 docs whether
thinking.type="adaptive"withoutoutput_config.effortbehaves equivalently to an explicit effort level for the low/medium/high tiers, to decide whether this mapping is worth adding.🤖 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 249 - 256, When converting claudeRequest.Thinking.Type from "enabled" to "adaptive" in the Opus 4.7 branch, preserve user effort and token-budget settings by translating Thinking.BudgetTokens and any reasoning_effort into the corresponding OutputConfig fields instead of clearing BudgetTokens; specifically, populate claudeRequest.OutputConfig.effort from claudeRequest.Thinking.reasing_effort/effort-level (low/medium/high) and map reasoning.max_tokens (or Thinking.BudgetTokens) into OutputConfig.max_tokens (or the schema-appropriate field) before setting Thinking.Type="adaptive" and nulling Thinking.BudgetTokens, and ensure the code that handles the effort-suffix branch (where OutputConfig is used) is reused for this branch so effort is signaled upstream (also add a TODO to verify with Opus 4.7 docs whether adaptive without output_config.effort is equivalent).
206-211: Opus 4.7 bare-model sanitization — correct, with minor duplication.The fix correctly closes the bare-path gap. Note that this block now overlaps with the same nil-assignments already performed in the effort-suffix branch (lines 167-169) and the
-thinkingbranch (lines 182-184), making those earlier clears redundant. Harmless, but you could collapse them into this single unconditional post-block clear for Opus 4.7 to keep the normalization in one place.♻️ Optional consolidation
if strings.HasPrefix(baseModel, "claude-opus-4-7") { - // Opus 4.7 rejects non-default temperature/top_p/top_k with 400 - // and defaults display to "omitted"; restore the 4.6 visible summary. + // Opus 4.7 defaults display to "omitted"; restore the 4.6 visible summary. + // temperature/top_p/top_k are cleared unconditionally below. claudeRequest.Thinking.Display = "summarized" - claudeRequest.Temperature = nil - claudeRequest.TopP = nil - claudeRequest.TopK = nil } else { claudeRequest.TopP = nil claudeRequest.Temperature = common.GetPointer[float64](1.0) }(and similarly drop lines 182-184 in the
-thinkingbranch)🤖 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 206 - 211, The niling of temperature/top_p/top_k for Opus 4.7 is duplicated across branches; consolidate by removing the earlier clears in the effort-suffix and -thinking branches and keep a single unconditional clear for any model matching strings.HasPrefix(claudeRequest.Model, "claude-opus-4-7") where you set claudeRequest.Temperature = nil, claudeRequest.TopP = nil, claudeRequest.TopK = nil so normalization lives only in that one post-check block.
🤖 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/claude_handler.go`:
- Around line 106-111: The Claude-format handler (ClaudeHelper) currently clears
temperature/top_p/top_k for models with prefix "claude-opus-4-7" but does not
perform the companion translation of thinking.type="enabled" to
thinking.type="adaptive", which causes Anthropic to reject such requests; after
the existing if block that nils request.Temperature/TopP/TopK, detect if
request.Model has prefix "claude-opus-4-7" and if request.Thinking != nil and
request.Thinking.Type == "enabled" then set request.Thinking.Type = "adaptive"
(mirroring the conversion in relay/channel/claude/relay-claude.go) so
pass-through Claude-format clients are accepted.
---
Nitpick comments:
In `@relay/channel/claude/relay-claude.go`:
- Around line 249-256: When converting claudeRequest.Thinking.Type from
"enabled" to "adaptive" in the Opus 4.7 branch, preserve user effort and
token-budget settings by translating Thinking.BudgetTokens and any
reasoning_effort into the corresponding OutputConfig fields instead of clearing
BudgetTokens; specifically, populate claudeRequest.OutputConfig.effort from
claudeRequest.Thinking.reasing_effort/effort-level (low/medium/high) and map
reasoning.max_tokens (or Thinking.BudgetTokens) into OutputConfig.max_tokens (or
the schema-appropriate field) before setting Thinking.Type="adaptive" and
nulling Thinking.BudgetTokens, and ensure the code that handles the
effort-suffix branch (where OutputConfig is used) is reused for this branch so
effort is signaled upstream (also add a TODO to verify with Opus 4.7 docs
whether adaptive without output_config.effort is equivalent).
- Around line 206-211: The niling of temperature/top_p/top_k for Opus 4.7 is
duplicated across branches; consolidate by removing the earlier clears in the
effort-suffix and -thinking branches and keep a single unconditional clear for
any model matching strings.HasPrefix(claudeRequest.Model, "claude-opus-4-7")
where you set claudeRequest.Temperature = nil, claudeRequest.TopP = nil,
claudeRequest.TopK = nil so normalization lives only in that one post-check
block.
🪄 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: ec7f0c64-bc0a-4609-9ef5-8683ab881c56
📒 Files selected for processing (2)
relay/channel/claude/relay-claude.gorelay/claude_handler.go
…deHelper The native Claude API path (ClaudeHelper) was missing the thinking.type conversion that relay-claude.go already performs. A client sending thinking.type="enabled" directly on a claude-opus-4-7 request would still get HTTP 400 from Anthropic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Combine temperature/top_p/top_k clearing and thinking.type="enabled" → "adaptive" conversion into one if-block for clarity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Summary
Follow-up to 47d7bca — two code paths were missed for Claude Opus 4.7 compatibility:
Bare
claude-opus-4-7(no effort/thinking suffix): client-suppliedtemperature/top_p/top_kare forwarded as-is to the Anthropic API, causing HTTP 400 "Improperly formed request". The existing effort-suffix and-thinkingbranches handle this correctly, but the bare model name falls through without sanitization.reasoning_effort/reasoningparameters: these setthinking.type="enabled"which Opus 4.7 also rejects (only"adaptive"is accepted). This affects the OpenAI-compat relay path.Changes
relay/channel/claude/relay-claude.go:temperature/top_p/top_kfor anyclaude-opus-4-7prefix modelReasoningEffort/Reasoningbranches, convertthinking.type="enabled"→"adaptive"(withdisplay="summarized") for Opus 4.7relay/claude_handler.go:temperature/top_p/top_kfor anyclaude-opus-4-7prefix modelTest plan
claude-opus-4-7withtemperature: 0.7— should succeed (param stripped)claude-opus-4-7withreasoning_effort: "high"— thinking type should beadaptive, notenabledclaude-opus-4-7-high— should still work as before (no regression)claude-opus-4-6— should still work as before (no regression)Summary by CodeRabbit