Skip to content

fix(agent): handle deepseek empty reasoning#1571

Merged
zerob13 merged 1 commit into
devfrom
fix/deepseek-interleved
Apr 30, 2026
Merged

fix(agent): handle deepseek empty reasoning#1571
zerob13 merged 1 commit into
devfrom
fix/deepseek-interleved

Conversation

@zerob13
Copy link
Copy Markdown
Collaborator

@zerob13 zerob13 commented Apr 30, 2026

Summary by CodeRabbit

  • Improvements

    • Enhanced reasoning content handling for DeepSeek models with refined preservation logic and clearer separation of concerns.
    • Improved model detection and configuration for more reliable compatibility behavior.
  • Tests

    • Added test coverage for DeepSeek reasoning preservation with compatibility settings disabled.
    • Added test coverage validating empty reasoning content handling.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 30, 2026

📝 Walkthrough

Walkthrough

This change decouples the handling of non-empty and empty interleaved reasoning preservation in the context builder, introducing a new isDeepSeekSeriesModelId predicate to classify DeepSeek models, and refining how preserveReasoningContent and preserveEmptyReasoningContent flags are resolved during configuration initialization.

Changes

Cohort / File(s) Summary
DeepSeek Model Classification
src/shared/model.ts
Introduces new exported predicate isDeepSeekSeriesModelId that normalizes and checks for DeepSeek substring in model IDs.
Interleaved Reasoning Configuration
src/main/presenter/agentRuntimePresenter/index.ts
Refines resolveInterleavedReasoningConfig to use the DeepSeek series predicate for both preserveReasoningContent (now model-driven) and preserveEmptyReasoningContent (now solely driven by DeepSeek classification).
Context Building Logic
src/main/presenter/agentRuntimePresenter/contextBuilder.ts
Decouples "preserve non-empty reasoning" from "preserve empty reasoning" by introducing shouldPreserveEmptyReasoning flag, extending applyReasoningContent with allowEmptyReasoning parameter, and adding explicit early returns for no-tool-call and empty-tool-calls paths.
Test Coverage
test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts, test/main/presenter/agentRuntimePresenter/contextBuilder.test.ts
Validates that DeepSeek v4 preserves both reasoning flags when interleaved thinking compatibility is disabled (unlike GPT-4 behavior), and confirms empty reasoning content is not added when the prior assistant message lacks reasoning blocks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 With whiskers twitching, we've untangled the knot,
DeepSeek now shines with reasoning thoughts,
Empty or full, each gets its own path—
No more mixed signals, just logic precise as math! 🎯

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: fixing handling of DeepSeek empty reasoning in the agent, which is the core focus of the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/deepseek-interleved

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.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/presenter/agentRuntimePresenter/contextBuilder.ts`:
- Around line 331-333: The empty-reasoning fallback is currently guarded only by
preserveEmptyInterleavedReasoning, causing inconsistent behavior; change the
guards in the contexts where the fallback is returned (the block using
assistantContent and the similar block later) to require both
preserveInterleavedReasoning && preserveEmptyInterleavedReasoning before
returning [{ role: 'assistant', content: assistantContent }], so the
empty-preservation path is only taken when interleaved reasoning preservation is
enabled too.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: aaa19f72-66f6-42f0-ad76-7a498f10f28c

📥 Commits

Reviewing files that changed from the base of the PR and between c2e8699 and 02c9376.

📒 Files selected for processing (5)
  • src/main/presenter/agentRuntimePresenter/contextBuilder.ts
  • src/main/presenter/agentRuntimePresenter/index.ts
  • src/shared/model.ts
  • test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts
  • test/main/presenter/agentRuntimePresenter/contextBuilder.test.ts

Comment on lines +331 to +333
if (preserveEmptyInterleavedReasoning) {
return [{ role: 'assistant', content: assistantContent }]
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard empty-reasoning fallback with the same composite condition.

Line 331 and Line 357 check preserveEmptyInterleavedReasoning directly, but empty-preservation elsewhere is gated by preserveInterleavedReasoning && preserveEmptyInterleavedReasoning. This creates inconsistent behavior when only the empty flag is true.

Proposed fix
-    if (preserveEmptyInterleavedReasoning) {
+    if (shouldPreserveEmptyReasoning) {
       return [{ role: 'assistant', content: assistantContent }]
     }
@@
-    if (preserveEmptyInterleavedReasoning) {
+    if (shouldPreserveEmptyReasoning) {
       return [{ role: 'assistant', content: assistantContent }]
     }

Also applies to: 357-359

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/presenter/agentRuntimePresenter/contextBuilder.ts` around lines 331
- 333, The empty-reasoning fallback is currently guarded only by
preserveEmptyInterleavedReasoning, causing inconsistent behavior; change the
guards in the contexts where the fallback is returned (the block using
assistantContent and the similar block later) to require both
preserveInterleavedReasoning && preserveEmptyInterleavedReasoning before
returning [{ role: 'assistant', content: assistantContent }], so the
empty-preservation path is only taken when interleaved reasoning preservation is
enabled too.

@zerob13 zerob13 merged commit bc789fe into dev Apr 30, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant