fix(gemini): handle minimal reasoning effort budget - #2460
Conversation
- Add minimal case to clampThinkingBudgetByEffort to avoid defaulting to full thinking budget
WalkthroughThe changes consolidate reasoning effort suffix handling by replacing a local Gemini adapter function with a centralized Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–12 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
relay/channel/gemini/relay-gemini.go (1)
106-114: Latent logic bug:is25FlashLitebranch gets overwritten.The
if is25FlashLiteblock on lines 107-108 setsmaxBudget, but then the subsequentif isNew25Pro ... elseblock on lines 110-114 unconditionally overwrites it (unlessisNew25Prois true). Currently this works by coincidence becauseflash25LiteMaxBudget == flash25MaxBudget, but if these constants ever diverge, the flash-lite budget will be incorrect.Apply this diff to use proper
else ifchaining:- maxBudget := 0 - if is25FlashLite { - maxBudget = flash25LiteMaxBudget - } - if isNew25Pro { - maxBudget = pro25MaxBudget - } else { - maxBudget = flash25MaxBudget - } + var maxBudget int + if is25FlashLite { + maxBudget = flash25LiteMaxBudget + } else if isNew25Pro { + maxBudget = pro25MaxBudget + } else { + maxBudget = flash25MaxBudget + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
relay/channel/gemini/adaptor.go(2 hunks)relay/channel/gemini/relay-gemini.go(3 hunks)setting/reasoning/suffix.go(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: 9Ninety
Repo: QuantumNous/new-api PR: 1273
File: relay/channel/gemini/relay-gemini.go:97-116
Timestamp: 2025-06-21T03:37:41.726Z
Learning: In relay/channel/gemini/relay-gemini.go, the thinking budget calculation logic (including the MaxOutputTokens multiplication) was introduced in PR #1247. PR #1273 focused specifically on decoupling the thoughts summary feature from thinking budget settings and did not modify the existing thinking budget behavior.
📚 Learning: 2025-06-21T03:37:41.726Z
Learnt from: 9Ninety
Repo: QuantumNous/new-api PR: 1273
File: relay/channel/gemini/relay-gemini.go:97-116
Timestamp: 2025-06-21T03:37:41.726Z
Learning: In relay/channel/gemini/relay-gemini.go, the thinking budget calculation logic (including the MaxOutputTokens multiplication) was introduced in PR #1247. PR #1273 focused specifically on decoupling the thoughts summary feature from thinking budget settings and did not modify the existing thinking budget behavior.
Applied to files:
relay/channel/gemini/relay-gemini.gorelay/channel/gemini/adaptor.go
📚 Learning: 2025-08-05T17:14:17.246Z
Learnt from: neotf
Repo: QuantumNous/new-api PR: 1511
File: setting/ratio_setting/model_ratio.go:118-123
Timestamp: 2025-08-05T17:14:17.246Z
Learning: Claude models handle "-thinking" variants differently from Gemini models. For Claude models, only the base model (without "-thinking") gets an entry in defaultModelRatio map. The "-thinking" variants rely on the Claude relay handler stripping the suffix using strings.TrimSuffix(textRequest.Model, "-thinking") before looking up the ratio, so they automatically use the base model's ratio.
Applied to files:
relay/channel/gemini/relay-gemini.gorelay/channel/gemini/adaptor.go
📚 Learning: 2025-08-21T06:31:11.073Z
Learnt from: jiajunly
Repo: QuantumNous/new-api PR: 1629
File: relay/channel/openai/relay-openai.go:170-174
Timestamp: 2025-08-21T06:31:11.073Z
Learning: In relay/channel/openai/relay-openai.go, the streaming logic for the AddThinkFirst feature is designed so that only the first chunk of a stream gets the "<think>\n" prefix. The final flush in the streaming handler intentionally uses addThink=false because the last chunk should never receive the prefix, even in single-chunk streams where the prefix would have been applied during normal processing.
Applied to files:
relay/channel/gemini/relay-gemini.go
🧬 Code graph analysis (2)
relay/channel/gemini/relay-gemini.go (1)
setting/reasoning/suffix.go (1)
TrimEffortSuffix(12-20)
relay/channel/gemini/adaptor.go (1)
setting/reasoning/suffix.go (1)
TrimEffortSuffix(12-20)
🔇 Additional comments (5)
setting/reasoning/suffix.go (1)
9-9: LGTM!The addition of "-minimal" to
EffortSuffixesis straightforward and aligns with the new minimal effort level support.relay/channel/gemini/adaptor.go (2)
16-16: LGTM!Correctly imports the centralized reasoning package for suffix handling.
141-143: LGTM!The refactor to use
reasoning.TrimEffortSuffixconsolidates the suffix parsing logic. The condition properly checks theokreturn value before usingbaseModel.relay/channel/gemini/relay-gemini.go (2)
101-101: LGTM!The "minimal" effort level at 5% of max tokens is appropriately documented and implemented. The resulting budget values (e.g., ~1228 tokens for flash models) will still be valid after clamping.
Also applies to: 122-123
184-190: LGTM!The refactor to use
reasoning.TrimEffortSuffixcorrectly extracts the effort level and sets bothThinkingConfig.ThinkingLevelandinfo.ReasoningEffort. This now supports the new "-minimal" suffix via the centralized suffix handling.
…h-minial fix(gemini): handle minimal reasoning effort budget
2 participants
#2525
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.