fix(transformer): support DeepSeek thinking parameter passthrough (#88) - #99
Conversation
Add support for DeepSeek's thinking parameter to fix issue where
thinking mode could not be disabled from client applications.
Root cause: The thinking parameter ({"thinking": {"type": "disabled"}})
sent by clients like Cherry Studio was silently dropped during JSON
deserialization because InternalLLMRequest lacked this field. This
caused DeepSeek API to use its default value (thinking.type = "enabled"),
making the model think even when users explicitly disabled it.
Changes:
- Add ThinkingConfig type to model package
- Add Thinking field to InternalLLMRequest
- Pass through thinking parameter in OpenAI chat outbound transformer
- Add unit tests and end-to-end regression test
Design:
- Protocol-neutral: only passes through the parameter without transformation
- Backward compatible: thinking is optional (omitempty)
- Isolated: does not affect other providers (Anthropic, Gemini, etc.)
Fixes: #88
📝 WalkthroughWalkthroughAdds a ChangesThinking parameter passthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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.
🧹 Nitpick comments (1)
internal/transformer/outbound/openai/chat_issue88_test.go (1)
45-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
io.ReadAlland handle read errors explicitly.The manual read loop breaks on any error and can hide unexpected read failures. Use
io.ReadAll(httpReq.Body)for clearer and safer test logic.♻️ Proposed refactor
import ( "context" "encoding/json" + "io" "testing" @@ - // Read the request body that would be sent to DeepSeek - body := make([]byte, 0) - buf := make([]byte, 1024) - for { - n, err := httpReq.Body.Read(buf) - if n > 0 { - body = append(body, buf[:n]...) - } - if err != nil { - break - } - } + // Read the request body that would be sent to DeepSeek + body, err := io.ReadAll(httpReq.Body) + if err != nil { + t.Fatalf("failed to read outgoing request body: %v", err) + }🤖 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 `@internal/transformer/outbound/openai/chat_issue88_test.go` around lines 45 - 56, The request-body capture in the chat_issue88 test uses a manual read loop that can mask unexpected failures; update the test to use io.ReadAll on httpReq.Body instead, and handle any returned read error explicitly before asserting on the collected body. Keep the change scoped to the test helper logic around httpReq.Body so the intent remains clear and safer.
🤖 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.
Nitpick comments:
In `@internal/transformer/outbound/openai/chat_issue88_test.go`:
- Around line 45-56: The request-body capture in the chat_issue88 test uses a
manual read loop that can mask unexpected failures; update the test to use
io.ReadAll on httpReq.Body instead, and handle any returned read error
explicitly before asserting on the collected body. Keep the change scoped to the
test helper logic around httpReq.Body so the intent remains clear and safer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e4051ef-53ae-44cf-a1fd-e9dc79df7bd1
📒 Files selected for processing (4)
internal/transformer/model/model.gointernal/transformer/outbound/openai/chat.gointernal/transformer/outbound/openai/chat_issue88_test.gointernal/transformer/outbound/openai/chat_thinking_test.go
Add support for DeepSeek's thinking parameter to fix issue where thinking mode could not be disabled from client applications.
Root cause: The thinking parameter ({"thinking": {"type": "disabled"}}) sent by clients like Cherry Studio was silently dropped during JSON deserialization because InternalLLMRequest lacked this field. This caused DeepSeek API to use its default value (thinking.type = "enabled"), making the model think even when users explicitly disabled it.
Changes:
Design:
Fixes: #88
Summary by CodeRabbit
New Features
Tests