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 👍 / 👎.
Summary
Changes
Validation