fix: correct gpt-5.6 completion ratio - #6063
Conversation
WalkthroughGPT-5.6 model identifiers now receive a locked completion ratio of 6, with tests covering three GPT-5.6 variants. ChangesGPT-5.6 ratio handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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.
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 `@setting/ratio_setting/model_ratio_test.go`:
- Around line 9-13: Update TestGetCompletionRatioGPT56UsesSpecialRatio so
gpt-5.6-sol, gpt-5.6-terra, and gpt-5.6-luna expect 8.0, and add a gpt-5.6
assertion expecting 6.0 to cover both pricing tiers.
In `@setting/ratio_setting/model_ratio.go`:
- Around line 507-509: Update the model-ratio logic around the gpt-5.6 prefix
check: add a more-specific branch for gpt-5.6-sol, gpt-5.6-terra, and
gpt-5.6-luna that returns ratio 8, while retaining ratio 6 for other gpt-5.6
models. Ensure the specific variants are evaluated before the general
HasPrefix("gpt-5.6") branch.
🪄 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: 8cc60e76-e8e9-4970-9df3-66e7f169a2e0
📒 Files selected for processing (2)
setting/ratio_setting/model_ratio.gosetting/ratio_setting/model_ratio_test.go
| func TestGetCompletionRatioGPT56UsesSpecialRatio(t *testing.T) { | ||
| assert.Equal(t, 6.0, GetCompletionRatio("gpt-5.6-sol")) | ||
| assert.Equal(t, 6.0, GetCompletionRatio("gpt-5.6-terra")) | ||
| assert.Equal(t, 6.0, GetCompletionRatio("gpt-5.6-luna")) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Correct the expected ratios for the $40/M variants.
Given the stated $5/M input and $40/M output pricing, gpt-5.6-sol, gpt-5.6-terra, and gpt-5.6-luna should expect 8.0. Add a base gpt-5.6 assertion expecting 6.0 so the test distinguishes both pricing tiers.
Proposed test correction
func TestGetCompletionRatioGPT56UsesSpecialRatio(t *testing.T) {
- assert.Equal(t, 6.0, GetCompletionRatio("gpt-5.6-sol"))
- assert.Equal(t, 6.0, GetCompletionRatio("gpt-5.6-terra"))
- assert.Equal(t, 6.0, GetCompletionRatio("gpt-5.6-luna"))
+ assert.Equal(t, 6.0, GetCompletionRatio("gpt-5.6"))
+ assert.Equal(t, 8.0, GetCompletionRatio("gpt-5.6-sol"))
+ assert.Equal(t, 8.0, GetCompletionRatio("gpt-5.6-terra"))
+ assert.Equal(t, 8.0, GetCompletionRatio("gpt-5.6-luna"))
}📝 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.
| func TestGetCompletionRatioGPT56UsesSpecialRatio(t *testing.T) { | |
| assert.Equal(t, 6.0, GetCompletionRatio("gpt-5.6-sol")) | |
| assert.Equal(t, 6.0, GetCompletionRatio("gpt-5.6-terra")) | |
| assert.Equal(t, 6.0, GetCompletionRatio("gpt-5.6-luna")) | |
| } | |
| func TestGetCompletionRatioGPT56UsesSpecialRatio(t *testing.T) { | |
| assert.Equal(t, 6.0, GetCompletionRatio("gpt-5.6")) | |
| assert.Equal(t, 8.0, GetCompletionRatio("gpt-5.6-sol")) | |
| assert.Equal(t, 8.0, GetCompletionRatio("gpt-5.6-terra")) | |
| assert.Equal(t, 8.0, GetCompletionRatio("gpt-5.6-luna")) | |
| } |
🤖 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 `@setting/ratio_setting/model_ratio_test.go` around lines 9 - 13, Update
TestGetCompletionRatioGPT56UsesSpecialRatio so gpt-5.6-sol, gpt-5.6-terra, and
gpt-5.6-luna expect 8.0, and add a gpt-5.6 assertion expecting 6.0 to cover both
pricing tiers.
| if strings.HasPrefix(name, "gpt-5.6") { | ||
| return 6, true | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle the $40/M GPT-5.6 variants separately.
This prefix matches gpt-5.6-sol, gpt-5.6-terra, and gpt-5.6-luna, but the stated pricing contract is $5/M input and $40/M output for those models, requiring a completion ratio of 8, not 6. Keep 6 for the $30/M-output GPT-5.6 models and add a more-specific branch for the $40/M variants; this value flows into relay pricing and quota calculations.
Proposed fix
if strings.HasPrefix(name, "gpt-5.6") {
+ if strings.HasPrefix(name, "gpt-5.6-sol") ||
+ strings.HasPrefix(name, "gpt-5.6-terra") ||
+ strings.HasPrefix(name, "gpt-5.6-luna") {
+ return 8, true
+ }
return 6, true
}📝 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 strings.HasPrefix(name, "gpt-5.6") { | |
| return 6, true | |
| } | |
| if strings.HasPrefix(name, "gpt-5.6") { | |
| if strings.HasPrefix(name, "gpt-5.6-sol") || | |
| strings.HasPrefix(name, "gpt-5.6-terra") || | |
| strings.HasPrefix(name, "gpt-5.6-luna") { | |
| return 8, true | |
| } | |
| return 6, true | |
| } |
🤖 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 `@setting/ratio_setting/model_ratio.go` around lines 507 - 509, Update the
model-ratio logic around the gpt-5.6 prefix check: add a more-specific branch
for gpt-5.6-sol, gpt-5.6-terra, and gpt-5.6-luna that returns ratio 8, while
retaining ratio 6 for other gpt-5.6 models. Ensure the specific variants are
evaluated before the general HasPrefix("gpt-5.6") branch.
51fdfc5 to
2b6f1df
Compare
?? ???? / PR Notice
Important
?? ???? / Description
?
gpt-5.6*?????????????????gpt-5.5*?gpt-5.4*???? 6 ?????????????gpt-5*? 8 ??????????? Token ????????
$5/M????????$30/M?????gpt-5.6-sol?gpt-5.6-terra?gpt-5.6-luna????$40/M??????????????????????? PR ? Codex ??????????????
gpt-5.6*?????????????? ???? / Type of change
?? ???? / Related Issue
? ?????? / Checklist
Bug fix?????????? Issue????????????????????????? bug??? ???? / Proof of Work
go test ./setting/ratio_setting -run TestGetCompletionRatioGPT56UsesSpecialRatio -count=1???
Summary by CodeRabbit
gpt-5.6models with the correct completion ratio.gpt-5.6variants consistently use a ratio of 6.