fix: preserve reasoning_content for DeepSeek chat completions#3290
fix: preserve reasoning_content for DeepSeek chat completions#3290lizzielizzielizzie wants to merge 2 commits intowavetermdev:mainfrom
Conversation
WalkthroughThe diff adds separate tracking for streaming "reasoning" deltas: ChatRequestMessage and ContentDelta gain a ReasoningContent field (with JSON wire mapping). processChatStream accumulates reasoning, emits SSE lifecycle events (AiMsgReasoningStart/AiMsgReasoningDelta/AiMsgReasoningEnd), ensures reasoning is ended if necessary, and includes reasoning in the final assistant message. extractPartialTextMessage now accepts both text and reasoning and returns nil only when both are empty. Tests were added to validate JSON round-trips, streaming chunk parsing, clean() behavior, and partial-message extraction involving reasoning. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 1
🤖 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 `@pkg/aiusechat/openaichat/openaichat-backend.go`:
- Around line 264-269: Currently the code waits until after the generation loop
to call sseHandler.AiMsgReasoningEnd(msgID), causing reasoning-end to be emitted
after AiMsgTextEnd; modify the loop that switches from reasoning deltas to text
deltas to detect the reasoning→text transition, call
sseHandler.AiMsgReasoningEnd(msgID) immediately at that point and set
reasoningStarted = false (so the post-loop block won't emit it again), ensuring
the existing post-loop branch still covers the reasoning-only case; update
references: reasoningStarted, textStarted, sseHandler.AiMsgReasoningEnd(msgID),
sseHandler.AiMsgTextEnd(textID).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b1e3572d-e024-4956-9e24-e0184e214682
📒 Files selected for processing (3)
pkg/aiusechat/openaichat/openaichat-backend.gopkg/aiusechat/openaichat/openaichat-backend_test.gopkg/aiusechat/openaichat/openaichat-types.go
DeepSeek V4 requires reasoning_content to be passed back unchanged in assistant messages on multi-turn conversations. Previously it was dropped at every stage: not captured from the stream delta, not stored, and not re-serialized in follow-up requests, causing HTTP 400 on the second API call.
Close AiMsgReasoningEnd inline on first content delta instead of deferring to post-loop cleanup. This ensures SSE event order matches the Anthropic backend: reasoning-start → delta×N → reasoning-end → text-start → delta×M → text-end. Fallback reasoning-end kept for the edge case where the stream ends during reasoning with no content ever arriving (e.g. max_tokens).
5289304 to
d8fed0d
Compare
DeepSeek V4 enables thinking mode by default and requires that the
reasoning_contentfield be passed back unchanged in assistant messages during multi-turn conversations. The openai-chat backend, however, does not support this. This PR adds that support, roughly following the pattern that is employed by the anthropic backend.Fixes #3266
Flow
reasoning-start, delta, and end SSE events are captured, sent to the frontend, and included in subsequent API calls
When a stream chunk contains reasoning_content in its delta, we now:
Non-reasoning providers are unaffected — the stream field is absent, the Go field defaults to "", and the omitempty JSON tag keeps it off the wire.
Automated Testing
Manual Testing
Is there anything else I should cover?
Screenshots
Before screenshot, demonstrating issue reproduction
Afterscreenshot, demonstrating issue resolution