fix codex(openai/responses): universal reasoning_content round-trip support：eq：deepseak - #3159
fix codex(openai/responses): universal reasoning_content round-trip support：eq：deepseak#3159OmMaBaMiHong wants to merge 5 commits into
Conversation
… DeepSeek compat
Two fixes for OpenAI Responses → Chat Completions translation when used
with DeepSeek models (deepseek-v4-flash):
1. Tool call grouping: consecutive function_call items in the input array
are now grouped into a single assistant message with multiple tool_calls,
fixing "insufficient tool messages following tool_calls message" errors.
2. Thinking mode disabled: DeepSeek's deepseek-v4-flash defaults to
thinking mode, which returns reasoning_content and requires it to be
echoed back. Send thinking: {type: "disabled"} instead of
reasoning_effort to avoid this requirement entirely.
Also strips reasoning_content from streaming and non-streaming responses
since the proxy does not support echoing it back.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace DeepSeek-specific thinking mode handling with a universal
approach that works across all models (DeepSeek, MiMo, etc.):
Response direction (Chat Completions → Responses):
- reasoning_content → type: "reasoning" output item with summary text
- reasoning_content → reasoning_text content part in message content
for round-trip echo-back
- Both streaming and non-streaming paths supported
Request direction (Responses → Chat Completions):
- reasoning_text in message content → reasoning_content on assistant msg
- type: "reasoning" input item summary → reasoning_content on assistant msg
Reasoning parameter handling:
- reasoning: {effort: "..."} → reasoning_effort: "..."
- reasoning: null or absent → thinking: {type: "disabled"}
(prevents DeepSeek default thinking mode which would require echo-back)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a "Reasoning Content Round-Trip" mechanism to ensure that reasoning and thinking content are correctly preserved when translating between the OpenAI Responses API and Chat Completions. Key changes include a new group-buffering approach for tool calls to satisfy strict ordering requirements, the extraction of reasoning text from input items to inject into upstream requests, and the inclusion of reasoning content in both streaming and non-streaming responses. Feedback from the review indicates a potential issue where the non-standard 'thinking' parameter might cause 400 errors on providers like OpenAI, suggesting it should be restricted to DeepSeek models. Additionally, there is a logic error in the response handler where reasoning content is not correctly matched to specific message choices when multiple outputs are generated.
| } else { | ||
| // reasoning not present — disable thinking to prevent echo-back requirement. | ||
| out, _ = sjson.SetBytes(out, "thinking", map[string]interface{}{"type": "disabled"}) |
There was a problem hiding this comment.
Unconditionally adding the thinking parameter when reasoning is absent will cause 400 Bad Request errors on providers that do not support this non-standard field (e.g., the official OpenAI API). This field is specific to DeepSeek and should only be injected when the target model or provider is known to support it.
| } else { | |
| // reasoning not present — disable thinking to prevent echo-back requirement. | |
| out, _ = sjson.SetBytes(out, "thinking", map[string]interface{}{"type": "disabled"}) | |
| } else if strings.Contains(strings.ToLower(modelName), "deepseek") { | |
| // reasoning not present — disable thinking to prevent echo-back requirement for DeepSeek. | |
| out, _ = sjson.SetBytes(out, "thinking", map[string]interface{}{"type": "disabled"}) | |
| } |
| if len(st.Reasonings) > 0 { | ||
| rp := []byte(`{"type":"reasoning_text","text":""}`) | ||
| rp, _ = sjson.SetBytes(rp, "text", st.Reasonings[len(st.Reasonings)-1].ReasoningData) | ||
| item, _ = sjson.SetRawBytes(item, "content.-1", rp) | ||
| } |
There was a problem hiding this comment.
Using st.Reasonings[len(st.Reasonings)-1] always picks the last reasoning item for every message in the response. If the response contains multiple choices (n > 1), all assistant messages will incorrectly receive the reasoning content of the last choice. The reasoning should be matched to the specific message by its choice index.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5939974123
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if len(pendingFunctionCalls) == 0 { | ||
| return |
There was a problem hiding this comment.
The new buffering path can drop function_call_output items in follow-up requests that send only tool results (for example with previous_response_id). function_call_output entries are buffered, but flushToolGroup returns early when there are no pending function_call items, so no role:"tool" message is emitted and upstream never receives the tool output.
Useful? React with 👍 / 👎.
| part, _ = sjson.SetBytes(part, "content_index", nextContentIdx) | ||
| out = append(out, emitRespEvent("response.content_part.added", part)) |
There was a problem hiding this comment.
After reasoning text is inserted, this branch can publish the output_text content part at index 1, but downstream response.output_text.delta / response.output_text.done events in the same function still use content_index: 0. In streams that include reasoning, that index mismatch can cause clients to associate text deltas with the wrong content part.
Useful? React with 👍 / 👎.
| // reasoning not present — disable thinking to prevent echo-back requirement. | ||
| out, _ = sjson.SetBytes(out, "thinking", map[string]interface{}{"type": "disabled"}) |
There was a problem hiding this comment.
This now injects thinking: {"type":"disabled"} for every request where reasoning is absent, regardless of upstream model/provider. Because this translator handles generic OpenAI-chat-completions targets, strict providers can reject requests with an unknown thinking field, turning normal non-reasoning calls into 400s.
Useful? React with 👍 / 👎.
- Gate the non-standard "thinking" parameter to DeepSeek only (prevents 400 errors on OpenAI and other providers) - Fix reasoning content matched to correct choice index (not always last) - Track output_text content_index per choice after reasoning_text insertion - Flush tool outputs even when no pending function_calls exist (tool-only follow-up requests) - Add findReasoningByChoiceIndex helper for correct lookups Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
已根据 review 反馈修复，具体改动：
commit: 0b750cf5 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cd94d2dae
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if len(pendingFunctionCalls) == 0 && len(pendingToolOutputs) == 0 { | ||
| return | ||
| } | ||
| // 1. Emit one assistant message with all accumulated tool_calls (only if there are function calls to emit) | ||
| assistantMessage := []byte(`{"role":"assistant","tool_calls":[]}`) |
There was a problem hiding this comment.
When a request contains only function_call_output items (for example a follow-up turn using previous_response_id), this branch still builds an assistant message with tool_calls: [] before emitting tool messages. That produces an invalid chat-completions sequence because the tool messages no longer follow a real tool call, and strict upstreams can reject the request; in this case the converter should emit only role:"tool" messages when no function calls are buffered.
Useful? React with 👍 / 👎.
| if st.ReasoningID != "" { | ||
| stopReasoning(st.ReasoningBuf.String()) | ||
| reasoningText = st.ReasoningBuf.String() | ||
| stopReasoning(st.ReasoningBuf.String(), idx) | ||
| st.ReasoningBuf.Reset() |
There was a problem hiding this comment.
Reasoning state is global, but stopReasoning is keyed by the current idx, so in multi-choice streams reasoning from one choice can be attached to another choice if their chunks are interleaved (e.g., choice 0 sends reasoning_content and choice 1 sends content next). That mis-association makes reasoning_text and completed output items reference the wrong assistant message for n>1 responses.
Useful? React with 👍 / 👎.
…sent
Remove the model-name gating on `thinking: {type: "disabled"}` since
the model name in the request may be an alias (e.g. gpt-5.4) that
does not contain "deepseek", yet still routes to DeepSeek upstream.
Without this, DeepSeek enters default thinking mode, returns
reasoning_content, and requires echo-back on follow-up requests.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdd64837eb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| rcText := gjson.GetBytes(rawJSON, "choices.0.message.reasoning_content").String() | ||
| includeReasoning := rcText != "" |
There was a problem hiding this comment.
In ConvertOpenAIChatCompletionsResponseToOpenAIResponsesNonStream, reasoning text is read once from choices.0.message.reasoning_content and then reused while iterating all choices, so n>1 responses can mis-attach choice 0's reasoning to other choices or drop reasoning returned only on non-zero choices. This produces incorrect reasoning/reasoning_text output associations for multi-choice non-stream calls.
Useful? React with 👍 / 👎.
luispater
left a comment
luispater
left a comment
There was a problem hiding this comment.
Summary
This PR improves OpenAI Responses → Chat Completions conversion (for clients sending Responses-style payloads to /v1/chat/completions) by:
- Buffering
function_call/function_call_outputso Chat Completions ordering stays valid even with interleaved messages. - Adding reasoning content round-trip plumbing (
reasoning_text↔reasoning_content) and Responses-side reasoning output emission (stream + non-stream).
Blocking
- Unconditional
thinking: {"type":"disabled"}injection + incorrect “retry layer” assumption
- When
reasoningis absent (or present withouteffort), the converter injectsthinking: {"type":"disabled"}. - This is a non-standard Chat Completions parameter and will likely produce HTTP 400 on providers that do not recognize it.
- The comment says OpenAI “will return a 400, which is caught and handled by the retry layer”, but
sdk/cliproxy/auth/conductor.go:isRequestInvalidErrortreats many 400invalid_request_errorcases as non-retryable request-shape failures. - Please either gate
thinkinginjection by upstream/provider capability, or implement a safe one-shot fallback that retries withoutthinkingonly when the error indicates unsupported parameters.
- Tool-only follow-up handling can emit invalid Chat Completions history
flushToolGroup()can emit an assistant message with emptytool_calls: []and then emitrole:"tool"messages when onlyfunction_call_outputitems are present.- For strict OpenAI-compatible Chat Completions, tool messages must correspond to a preceding assistant tool_calls message with matching IDs; an empty tool_calls array is typically rejected.
- If tool-only follow-ups are a real supported scenario, please add a strategy to reconstruct missing tool_calls (or avoid emitting invalid sequences) plus a regression test.
- Docs language mismatch
docs/reasoning-content-roundtrip.mdis newly added but mostly Chinese. Repo guidance indicates new Markdown docs should be English.
Non-blocking
- Non-streaming reasoning extraction uses
choices.0.message.reasoning_contentonly; consider guarding/documenting the single-choice assumption. - Consider adding focused tests for streaming
reasoning_contentinterleaved withcontentandtool_calls.
Verification (per PR notes)
go test ./internal/translator/openai/openai/responses -count=1go build -o test-output ./cmd/server && rm -f test-output
2 participants
Summary
fix codex新版user+response强制转换不兼容deepseak等三方模型问题
reasoning_content统一转换为type: "reasoning"output item + message content 中的type: "reasoning_text"内容片段reasoning_text和type: "reasoning"summary 中提取推理文本，注入 assistant message 的reasoning_content字段reasoning.effort时 →thinking: disabled（防止 DeepSeek 默认思考模式产生 echo-back 要求）；传了 → 映射reasoning_effort验证
reasoning_textreasoning_textcontent partreasoning: null: 不返回 reasoning_content，无 echo-back 要求reasoning: {effort: "high"}: 正确启用思考模式go vet/go build错误