Feature/support deepseek reasoning content - #3523
Conversation
… 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.
There was a problem hiding this comment.
💡 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".
| parts := collectReasoningTextParts(item.Get("summary")) | ||
| if len(parts) == 0 { | ||
| parts = collectReasoningTextParts(item.Get("content")) |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
| 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.
There was a problem hiding this comment.
💡 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".
| if strings.TrimSpace(item.Get("role").String()) == "assistant" { | ||
| consumePendingReasoning() | ||
| assistantOrdinal++ |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd08c7a343
ℹ️ 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".
| for _, choice := range gjson.GetBytes(body, "choices").Array() { | ||
| message := choice.Get("message") | ||
| if reasoning := message.Get("reasoning_content").String(); strings.TrimSpace(reasoning) != "" { | ||
| r.appendReasoning(reasoning) | ||
| } | ||
| r.recordToolCalls(message.Get("tool_calls")) | ||
| } |
There was a problem hiding this comment.
Keep DeepSeek reasoning per choice
When a chat completion request uses multiple choices (n > 1), DeepSeek can return separate reasoning_content and tool_calls for each choice, but this loop appends every choice's reasoning into one recorder and then records every choice's tool call ID against the combined text. A client that follows up with only one selected choice can therefore get another choice's reasoning injected from the cache, which can break DeepSeek's round-trip validation or expose unrelated reasoning; track the recorder state by choice.index instead of sharing one builder across all choices.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a7c0d7902
ℹ️ 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".
| if email != "" { | ||
| metadata["email"] = email |
There was a problem hiding this comment.
Avoid marking API-key auths as OAuth
When an OpenAI-compatible API key entry has email/label (or inherits one from an auth file), storing it in metadata["email"] makes Auth.AccountInfo() classify the auth as oauth because it checks metadata email before attributes["api_key"] (sdk/cliproxy/auth/types.go). That causes API-key-only paths such as resolveOpenAICompatUpstreamModelPool/applyAPIKeyModelAlias in sdk/cliproxy/auth/conductor.go to be skipped, so configured OpenAI-compatible model aliases or model pools stop resolving and the client-visible alias can be sent upstream directly. Keep the display email out of the generic email metadata key or set an explicit API-key auth kind.
Useful? React with 👍 / 👎.
…guration - Added new test cases for DeepSeek v4 models in deepseek_reasoning_test.go. - Registered DeepSeek provider in thinking_providers.go and applied it in apply.go. - Updated OpenAI compatibility configuration to include DeepSeek v4 models with additional thinking levels. - Introduced a helper function to check DeepSeek v4 model compatibility.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 823804b4e9
ℹ️ 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".
| if callID == "" { | ||
| continue | ||
| } | ||
| if reasoning := lookupDeepSeekReasoning(callID); strings.TrimSpace(reasoning) != "" { |
There was a problem hiding this comment.
Scope cached reasoning beyond the tool call ID
When two DeepSeek conversations or users reuse the same tool_call.id within the 30-minute cache TTL, this restore path looks up reasoning from the package-global cache by that ID alone and injects it into the current request. Tool call IDs are only conversation-local identifiers, so a collision or client-supplied repeated ID can corrupt the follow-up request with another turn's reasoning content, and may expose reasoning across requests. Include additional scope such as auth/request/model context in the cache key, or avoid global restoration on call ID alone.
Useful? React with 👍 / 👎.
Brings the 9 commits from upstream PR router-for-me#3523 plus the dev merge commit into main. Source: router-for-me#3523
Summary
Changes
Validation