fix: sync reasoning effort after parameter overrides - #6566
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughParameter overrides now audit reasoning fields and synchronize ChangesReasoning override handling
Estimated code review effort: 3 (Moderate) | ~15 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Fixes a metadata/auditing mismatch in the relay param-override pipeline by syncing RelayInfo.ReasoningEffort from the final outbound JSON after overrides are applied, and ensuring reasoning-related override operations are included in the sensitive audit path list.
Changes:
- Add
reasoning/reasoning_effortto the sensitive param-override audit path prefixes. - Sync
RelayInfo.ReasoningEffortfrom the overridden outbound JSON (reasoning.effortfor Responses;reasoning_effortfor Chat Completions). - Add regression tests covering Responses and Chat Completions reasoning-effort sync (and partially audit behavior).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| relay/common/override.go | Adds reasoning-sensitive audit prefixes and synchronizes RelayInfo.ReasoningEffort from the final overridden request JSON. |
| relay/common/override_test.go | Adds regression tests for reasoning-effort synchronization (and audit coverage for Responses). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func TestApplyParamOverrideWithRelayInfoSyncsChatReasoningEffort(t *testing.T) { | ||
| info := &RelayInfo{ | ||
| ReasoningEffort: "high", | ||
| ChannelMeta: &ChannelMeta{ | ||
| ParamOverride: map[string]interface{}{ | ||
| "operations": []interface{}{ | ||
| map[string]interface{}{ | ||
| "mode": "set", | ||
| "path": "reasoning_effort", | ||
| "value": "xhigh", | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } |
| if info.ReasoningEffort != "xhigh" { | ||
| t.Fatalf("expected final reasoning effort xhigh, got %q", info.ReasoningEffort) | ||
| } | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/common/override_test.go`:
- Around line 2182-2193: Update both test sites in relay/common/override_test.go
(lines 2182-2193 and 2217-2225): use require.NoError for
ApplyParamOverrideWithRelayInfo errors, and replace manual t.Fatalf checks for
ReasoningEffort and ParamOverrideAudit with assert.Equal while preserving the
existing expected values.
In `@relay/common/override.go`:
- Around line 208-228: Update syncReasoningEffort to clear info.ReasoningEffort
before iterating over the reasoning paths. Preserve the existing loop, but only
assign the field when the final JSON contains a string value, leaving it empty
when both paths are absent or non-string.
🪄 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 Plus
Run ID: d3c84571-e052-41c5-a91e-2c46b92c5baf
📒 Files selected for processing (2)
relay/common/override.gorelay/common/override_test.go
| // syncReasoningEffort keeps request metadata aligned with the final outbound | ||
| // body after parameter overrides have been applied. Adapters populate this | ||
| // field before overrides run, so reading the final JSON prevents logs from | ||
| // reporting the pre-override effort. | ||
| func syncReasoningEffort(info *RelayInfo, jsonData []byte) { | ||
| if info == nil { | ||
| return | ||
| } | ||
|
|
||
| for _, path := range []string{"reasoning.effort", "reasoning_effort"} { | ||
| value := gjson.GetBytes(jsonData, path) | ||
| if !value.Exists() { | ||
| continue | ||
| } | ||
| if value.Type == gjson.String { | ||
| info.ReasoningEffort = value.String() | ||
| } else { | ||
| info.ReasoningEffort = "" | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Clear metadata when the final body has no reasoning field.
Line 219 skips absent paths and Line 228 returns without changing info.ReasoningEffort. If an override removes reasoning.effort or reasoning_effort, logs retain the pre-override value.
Clear info.ReasoningEffort before this loop. Then set it only when the final body contains a string value.
Proposed fix
func syncReasoningEffort(info *RelayInfo, jsonData []byte) {
if info == nil {
return
}
+ info.ReasoningEffort = ""
for _, path := range []string{"reasoning.effort", "reasoning_effort"} {Based on PR objectives, metadata must align with the final outbound request body.
📝 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.
| // syncReasoningEffort keeps request metadata aligned with the final outbound | |
| // body after parameter overrides have been applied. Adapters populate this | |
| // field before overrides run, so reading the final JSON prevents logs from | |
| // reporting the pre-override effort. | |
| func syncReasoningEffort(info *RelayInfo, jsonData []byte) { | |
| if info == nil { | |
| return | |
| } | |
| for _, path := range []string{"reasoning.effort", "reasoning_effort"} { | |
| value := gjson.GetBytes(jsonData, path) | |
| if !value.Exists() { | |
| continue | |
| } | |
| if value.Type == gjson.String { | |
| info.ReasoningEffort = value.String() | |
| } else { | |
| info.ReasoningEffort = "" | |
| } | |
| return | |
| } | |
| // syncReasoningEffort keeps request metadata aligned with the final outbound | |
| // body after parameter overrides have been applied. Adapters populate this | |
| // field before overrides run, so reading the final JSON prevents logs from | |
| // reporting the pre-override effort. | |
| func syncReasoningEffort(info *RelayInfo, jsonData []byte) { | |
| if info == nil { | |
| return | |
| } | |
| info.ReasoningEffort = "" | |
| for _, path := range []string{"reasoning.effort", "reasoning_effort"} { | |
| value := gjson.GetBytes(jsonData, path) | |
| if !value.Exists() { | |
| continue | |
| } | |
| if value.Type == gjson.String { | |
| info.ReasoningEffort = value.String() | |
| } else { | |
| info.ReasoningEffort = "" | |
| } | |
| return | |
| } |
🤖 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/common/override.go` around lines 208 - 228, Update syncReasoningEffort
to clear info.ReasoningEffort before iterating over the reasoning paths.
Preserve the existing loop, but only assign the field when the final JSON
contains a string value, leaving it empty when both paths are absent or
non-string.
Problem
Parameter overrides can change the outbound reasoning setting after the adapter has populated RelayInfo.ReasoningEffort. Usage logs then report the stale pre-override value, and reasoning override operations are omitted from the parameter-override audit.
Fix
This keeps the displayed reasoning effort aligned with the request actually sent upstream without changing routing or persistence behavior.
Verification
Summary by CodeRabbit