claude provider: properly handle "withThinking"#11
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
OrriMandarin
left a comment
There was a problem hiding this comment.
LGTM
A minor remark, but it can be treated independently.
| } else if r.Thinking != nil && *r.Thinking { | ||
| params.Thinking = lo.ToPtr(DefaultThinkingBudgetTokens) |
There was a problem hiding this comment.
We can avoid defining a "default" budget by utilizing the ThinkingConfigAdaptiveParam option to control the thinking feature. (instead of ThinkingConfigParamOfEnabled).
The AdaptiveParam feature allows Claude to determine the budget allocation based on the complexity of the request. I believe this feature is a perfect match.
This can be implemented in another PR.
See thread in checkmarble/marble-backend#1277.
The implementation was not properly handling thinking without a budget.