Conversation
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Code Review
This pull request updates the DeepSeek outbound transformer to default the 'thinking' parameter to 'enabled' unless explicitly disabled and ensures that assistant messages include a 'reasoning_content' field. It also updates the test suite to reflect these changes and adds new test cases for reasoning content population. Feedback was provided regarding the 'thinking' parameter, noting that it is only supported by the 'deepseek-reasoner' model and should not be sent to other models to avoid potential API errors.
| // DeepSeek defaults thinking to enabled unless explicitly disabled. | ||
| thinkingDisabled := llmReq.ReasoningEffort == "none" | ||
| dsReq.Thinking = &Thinking{ | ||
| Type: "enabled", | ||
| } | ||
| if thinkingDisabled { | ||
| dsReq.Thinking.Type = "disabled" | ||
| } | ||
|
|
||
| // Unless thinking is explicitly disabled, DeepSeek assistant messages | ||
| // should carry the reasoning_content field; fill in an empty string if missing. | ||
| if !thinkingDisabled { | ||
| for i := range dsReq.Messages { | ||
| if dsReq.Messages[i].Role == "assistant" && dsReq.Messages[i].ReasoningContent == nil { | ||
| dsReq.Messages[i].ReasoningContent = lo.ToPtr("") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation defaults thinking to enabled for all DeepSeek models when ReasoningEffort is not explicitly set to "none". However, the thinking parameter is only supported by the deepseek-reasoner model. Sending this parameter to other models like deepseek-chat will likely result in an API error (e.g., Unknown parameter 'thinking'). It is recommended to only enable this by default if the model is identified as a reasoning model, or if the user explicitly requested a reasoning effort via ReasoningEffort.
// DeepSeek defaults thinking to enabled for reasoner models unless explicitly disabled.
// Note: The 'thinking' parameter is only supported by deepseek-reasoner.
isReasoner := llmReq.Model == "deepseek-reasoner"
thinkingDisabled := llmReq.ReasoningEffort == "none"
if isReasoner || (llmReq.ReasoningEffort != "" && !thinkingDisabled) {
dsReq.Thinking = &Thinking{
Type: "enabled",
}
if thinkingDisabled {
dsReq.Thinking.Type = "disabled"
}
}
// Unless thinking is explicitly disabled, DeepSeek assistant messages
// should carry the reasoning_content field; fill in an empty string if missing.
if dsReq.Thinking != nil && dsReq.Thinking.Type == "enabled" {
for i := range dsReq.Messages {
if dsReq.Messages[i].Role == "assistant" && dsReq.Messages[i].ReasoningContent == nil {
dsReq.Messages[i].ReasoningContent = lo.ToPtr("")
}
}
}
Uh oh!
There was an error while loading. Please reload this page.