Skip to content

Feature/support deepseek reasoning content#3523

Open
berthojoris wants to merge 3 commits into
router-for-me:devfrom
berthojoris:feature/support-deepseek-reasoning-content
Open

Feature/support deepseek reasoning content#3523
berthojoris wants to merge 3 commits into
router-for-me:devfrom
berthojoris:feature/support-deepseek-reasoning-content

Conversation

@berthojoris
Copy link
Copy Markdown

Summary

 Adds support for preserving DeepSeek V4 reasoning content when OpenAI Responses requests are translated through
 OpenAI-compatible chat completions. This enables `deepseek-v4-pro` and `deepseek-v4-flash` to retain `reasoning_content`      
 across tool-call assistant messages, including model variants with suffixes like `(8192)`.

Changes

 - Detect DeepSeek V4 reasoning models: `deepseek-v4-pro` and `deepseek-v4-flash`.
 - Restore Responses `reasoning` items into OpenAI Chat Completions assistant messages as `reasoning_content`.
 - Apply preservation in OpenAI-compatible non-streaming, streaming, and token-count request paths.
 - Add unit tests covering:
   - supported DeepSeek V4 model detection
   - assistant ordinal mapping
   - tool-call reasoning restoration
   - skipping unsupported models
   - preserving existing `reasoning_content`
 - Add a DeepSeek OpenAI-compatible provider example to `config.example.yaml`.

Validation

 - `go test ./internal/config`
 - `go test ./internal/runtime/executor/helps`
 - `go test ./sdk/cliproxy`
 - `go build -o test-output ./cmd/server`

… processes

- Updated the Execute and ExecuteStream methods to include reasoning content preservation for original and translated payloads using the new helps.PreserveDeepSeekReasoningContent function.
- Added reasoning content preservation in the CountTokens method to ensure consistency across translation operations.
- Included an example configuration for the DeepSeek OpenAI-compatible provider in config.example.yaml.
- Added base URL and API key entries for the DeepSeek provider along with model definitions.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 362a7fe783

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +128 to +130
parts := collectReasoningTextParts(item.Get("summary"))
if len(parts) == 0 {
parts = collectReasoningTextParts(item.Get("content"))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Use full reasoning content when rebuilding DeepSeek history

The restoration logic reads reasoning.summary first and only falls back to reasoning.content when summary is absent, so requests that carry both fields will replay the abbreviated summary instead of the full reasoning trace. DeepSeek V4 tool-call turns require the prior reasoning_content to be fully round-tripped; sending a shortened summary can still trigger 400 errors on subsequent turns even though the helper ran. Prefer content when present (or merge with explicit precedence) to avoid lossy replay.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

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

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 reasoning content round-tripping for models such as deepseek-v4-pro and deepseek-v4-flash. The implementation includes logic to identify these models and map reasoning blocks from the source JSON back into the translated OpenAI-compatible message structure. Feedback was provided regarding a redundant bytes.Clone call in the PreserveDeepSeekReasoningContent function, which can be removed to avoid unnecessary allocations since sjson.SetBytes already returns a new byte slice.

return translatedChatCompletions
}

out := bytes.Clone(translatedChatCompletions)
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.

medium

The call to bytes.Clone is redundant here. sjson.SetBytes returns a new byte slice and does not modify the input slice in-place. Furthermore, the input translatedChatCompletions is already a fresh slice generated by the translator in the calling code. Removing this clone avoids an unnecessary allocation for every request involving these models.

Suggested change
out := bytes.Clone(translatedChatCompletions)
out := translatedChatCompletions

- Introduced a new test to verify that the PreserveDeepSeekReasoningContent function prefers full content over summary when processing reasoning data.
- Updated the PreserveDeepSeekReasoningContent function to streamline the handling of translated chat completions by removing unnecessary byte cloning.
- Adjusted the logic in responseReasoningItemText to prioritize content over summary when extracting reasoning text parts.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b0dc79cb2e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +105 to +107
if strings.TrimSpace(item.Get("role").String()) == "assistant" {
consumePendingReasoning()
assistantOrdinal++
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve reasoning on tool-call assistant in mixed turns

When an input turn contains reasoning followed by an assistant message and function_call items, this logic consumes pendingReasoning at the assistant message and increments the ordinal again at the subsequent function-call group, so only the content assistant gets reasoning_content and the tool-call assistant is left empty. That pattern is produced by this repo’s Responses translation path for mixed content + tool_calls assistant outputs (internal/translator/openai/openai/responses/openai_openai-responses_response.go emits both message and function_call items), so follow-up DeepSeek V4 tool-call requests can still hit 400 because the assistant message carrying tool_calls is missing the required reasoning payload.

Useful? React with 👍 / 👎.

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