Skip to content

fix(deepseek): preserve reasoning_content through full pipeline for DeepSeek V4 models - #2231

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.0from
kang-heewon:reasoning-replay-deepseek-v4
May 14, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.0from
kang-heewon:reasoning-replay-deepseek-v4

Conversation

@kang-heewon

@kang-heewon kang-heewon commented May 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Rebased against upstream release/v3.8.0 (bbf3ba0). The 4 core DeepSeek V4 reasoning replay commits from this PR were already cherry-picked by upstream, so only the PR review feedback changes remain.

Remaining Changes (2 files, 4 insertions, 2 deletions)

1. responseSanitizer.ts — Extract inline regex to constant

  • Extracted /deepseek[-/]v4/i inline regex to DEEPSEEK_V4_SANITIZER_MODEL_PATTERN constant
  • Improves readability and makes the detection intent explicit

2. translator/index.ts — Boundary-aware DeepSeek detection

  • Changed normalizedModel.includes("deepseek")/(^|\/)deepseek/i.test(normalizedModel)
  • Prevents false matches on model strings that happen to contain "deepseek" as a substring

Verification

  • npm run typecheck:core — clean (1 pre-existing ioredis error unrelated)
  • lsp_diagnostics — clean on both changed files
  • 1 commit (e8268db) on top of upstream/release/v3.8.0

@kang-heewon
kang-heewon requested a review from diegosouzapw as a code owner May 13, 2026 16:42

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces support for DeepSeek V4 models, specifically focusing on their reasoning capabilities. Key changes include updating the provider registry, enhancing the response sanitizer to preserve reasoning content for DeepSeek V4, and extending the reasoning cache to support request-based keys when tool calls are absent. Feedback from the review suggests centralizing the model detection regex patterns to avoid inconsistencies and refining the model identification logic in the translator to prevent overly broad matching that could lead to false positives.

Comment thread open-sse/handlers/responseSanitizer.ts
Comment thread open-sse/translator/index.ts
@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.0 May 14, 2026 02:53
@diegosouzapw

Copy link
Copy Markdown
Owner

Hey @kang-heewon! Thanks for the deep work on DeepSeek V4 reasoning preservation.

This PR overlaps significantly with work already merged in release/v3.8.0 (FASE 5-9 consolidation, plus upstream cherry-picks):

  • bc73fa24 — xiaomi-mimo reasoning replay
  • 9e7cb90a — sanitizer preserves reasoning_content on tool_calls
  • 28801557 — responses delta.reasoning_content

The new release/v3.8.0 HEAD has reconciliable changes around sanitizer + reasoning cache that probably overlap with parts of your diff (requireReasoningReplay, reasoning cache key change to requestId+messageIndex).

Could you rebase against the current release/v3.8.0 and:

  1. Reconcile with the new cache-key signature
  2. Verify requiresReasoningReplay callers are all updated (it changed shape — provider was kept positional in upstream, your PR changes it to an options object)
  3. Keep only the delta that's still missing (e.g., the DeepSeek-V4-specific supportsReasoning: true for 14 models)

Happy to review the rebased version. Leaving open.

diegosouzapw added a commit that referenced this pull request May 14, 2026
…rry-pick from PR #2231)

Cherry-picks non-overlapping changes from @kang-heewon's PR #2231:
- isDeepSeekV4Model() check in responseSanitizer
- providerRegistry V4 model entries with supportsReasoning
- schemaCoercion model-param for injectEmptyReasoningContentForToolCalls
- reasoningCache request-ID-based stable keys
- translator reasoning-only message replay for DeepSeek
- Comprehensive test coverage (81 tests across 5 providers)

Co-authored-by: kang-heewon <owen@kangheewon.dev>
diegosouzapw added a commit that referenced this pull request May 14, 2026
…ct signature

After cherry-picking PR #2231, the function signature changed from
positional (provider, model) to object ({ provider, model }). Fixes the
2 pre-existing tests that still used the old positional style.
diegosouzapw added a commit that referenced this pull request May 14, 2026
Deep audit of all 320 commits since v3.7.9 found:
- 18 merged PRs not documented in CHANGELOG (4 features, 10 bug fixes, 1 security, 2 chores, 1 debug improvement)
- 3 contributors entirely missing from credits table (@NomenAK with 12 PRs, @kang-heewon, @one-vs)
- 4 existing contributors with inaccurate PR counts (@oyi77 8→12, @ddarkr 2→3, @andrewmunsell 2→3, @nickwizard 2→3)

New entries added:
- feat: #2135 (1proxy settings), #2227 (antigravity project ID), #2238 (Z.AI Search), #2240 (CLI Suite)
- fix: #2217, #2218, #2219, #2221, #2222, #2223, #2224, #2231, #2233, #2236, #2242, #2243
- security: #2209 (stack trace exposure)
- chore: #2228, #2234

Total contributors updated from 50+ to 55+.
This was referenced May 14, 2026
@kang-heewon
kang-heewon force-pushed the reasoning-replay-deepseek-v4 branch from 2ad82d0 to e8268db Compare May 14, 2026 17:04
@kang-heewon

Copy link
Copy Markdown
Contributor Author

Rebase complete. Here is what was done:

Rebased on upstream/release/v3.8.0 (HEAD: bbf3ba0). The 4 overlapping commits were already cherry-picked upstream, so they were dropped during rebase:

  • preserve reasoning_content = upstream 18ef28e
  • widen sanitizer regex + requestId = upstream 72dd7c9
  • messageIndex 0 = upstream 4726dea
  • fix cache key = upstream c2bf5c7

Regarding your 3 requests:

  1. Cache-key signature — upstream and our PR now agree: buildAssistantMessageCacheKey(requestId, messageIndex)request:${requestId}:message:${messageIndex}. No conflict.
  2. requiresReasoningReplay callers — upstream already uses the object parameter signature ({ provider, model, thinkingEnabled?, supportsReasoning? }). Verified: reasoningCache.ts definition + translator/index.ts caller both match. No adjustment needed.
  3. supportsReasoning for DeepSeek V4 models — already registered in upstream's providerRegistry.ts with supportsReasoning: true for 18 entries. No delta to add.

Remaining delta (2 files, 4 insertions, 2 deletions):

  • responseSanitizer.ts: Extracted inline regex to DEEPSEEK_V4_SANITIZER_MODEL_PATTERN constant
  • translator/index.ts: Changed includes("deepseek") to /(^|\/)deepseek/i boundary regex

Both changes are PR review feedback from gemini-code-assist. The PR description has been updated to reflect the current scope.

@diegosouzapw
diegosouzapw merged commit 67f713d into diegosouzapw:release/v3.8.0 May 14, 2026
3 checks passed
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…rry-pick from PR diegosouzapw#2231)

Cherry-picks non-overlapping changes from @kang-heewon's PR diegosouzapw#2231:
- isDeepSeekV4Model() check in responseSanitizer
- providerRegistry V4 model entries with supportsReasoning
- schemaCoercion model-param for injectEmptyReasoningContentForToolCalls
- reasoningCache request-ID-based stable keys
- translator reasoning-only message replay for DeepSeek
- Comprehensive test coverage (81 tests across 5 providers)

Co-authored-by: kang-heewon <owen@kangheewon.dev>
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…ct signature

After cherry-picking PR diegosouzapw#2231, the function signature changed from
positional (provider, model) to object ({ provider, model }). Fixes the
2 pre-existing tests that still used the old positional style.
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
Deep audit of all 320 commits since v3.7.9 found:
- 18 merged PRs not documented in CHANGELOG (4 features, 10 bug fixes, 1 security, 2 chores, 1 debug improvement)
- 3 contributors entirely missing from credits table (@NomenAK with 12 PRs, @kang-heewon, @one-vs)
- 4 existing contributors with inaccurate PR counts (@oyi77 8→12, @ddarkr 2→3, @andrewmunsell 2→3, @nickwizard 2→3)

New entries added:
- feat: diegosouzapw#2135 (1proxy settings), diegosouzapw#2227 (antigravity project ID), diegosouzapw#2238 (Z.AI Search), diegosouzapw#2240 (CLI Suite)
- fix: diegosouzapw#2217, diegosouzapw#2218, diegosouzapw#2219, diegosouzapw#2221, diegosouzapw#2222, diegosouzapw#2223, diegosouzapw#2224, diegosouzapw#2231, diegosouzapw#2233, diegosouzapw#2236, diegosouzapw#2242, diegosouzapw#2243
- security: diegosouzapw#2209 (stack trace exposure)
- chore: diegosouzapw#2228, diegosouzapw#2234

Total contributors updated from 50+ to 55+.
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
… through full pipeline for DeepSeek V4 models (thanks @kang-heewon)

Squash merge as part of the v3.8.0 release housekeeping. Closes diegosouzapw#2231.
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…rry-pick from PR diegosouzapw#2231)

Cherry-picks non-overlapping changes from @kang-heewon's PR diegosouzapw#2231:
- isDeepSeekV4Model() check in responseSanitizer
- providerRegistry V4 model entries with supportsReasoning
- schemaCoercion model-param for injectEmptyReasoningContentForToolCalls
- reasoningCache request-ID-based stable keys
- translator reasoning-only message replay for DeepSeek
- Comprehensive test coverage (81 tests across 5 providers)

Co-authored-by: kang-heewon <owen@kangheewon.dev>
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…ct signature

After cherry-picking PR diegosouzapw#2231, the function signature changed from
positional (provider, model) to object ({ provider, model }). Fixes the
2 pre-existing tests that still used the old positional style.
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
Deep audit of all 320 commits since v3.7.9 found:
- 18 merged PRs not documented in CHANGELOG (4 features, 10 bug fixes, 1 security, 2 chores, 1 debug improvement)
- 3 contributors entirely missing from credits table (@NomenAK with 12 PRs, @kang-heewon, @one-vs)
- 4 existing contributors with inaccurate PR counts (@oyi77 8→12, @ddarkr 2→3, @andrewmunsell 2→3, @nickwizard 2→3)

New entries added:
- feat: diegosouzapw#2135 (1proxy settings), diegosouzapw#2227 (antigravity project ID), diegosouzapw#2238 (Z.AI Search), diegosouzapw#2240 (CLI Suite)
- fix: diegosouzapw#2217, diegosouzapw#2218, diegosouzapw#2219, diegosouzapw#2221, diegosouzapw#2222, diegosouzapw#2223, diegosouzapw#2224, diegosouzapw#2231, diegosouzapw#2233, diegosouzapw#2236, diegosouzapw#2242, diegosouzapw#2243
- security: diegosouzapw#2209 (stack trace exposure)
- chore: diegosouzapw#2228, diegosouzapw#2234

Total contributors updated from 50+ to 55+.
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
… through full pipeline for DeepSeek V4 models (thanks @kang-heewon)

Squash merge as part of the v3.8.0 release housekeeping. Closes diegosouzapw#2231.
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.

2 participants