fix(proxy/transform): preserve thinking as reasoning_content on assistant tool calls#2370
fix(proxy/transform): preserve thinking as reasoning_content on assistant tool calls#237097wow wants to merge 1 commit intofarion1231:mainfrom
Conversation
…tant tool calls
Anthropic thinking blocks were dropped during the OpenAI conversion.
Moonshot AI rejects subsequent requests with HTTP 400 ("thinking is
enabled but reasoning_content is missing") when an assistant tool call
message lacks the field.
Collect thinking text into a reasoning_parts vec and inject it as
reasoning_content on assistant messages that carry tool_calls; fall
back to a "tool call" placeholder when no thinking was captured.
Plain assistant messages (no tool_calls) remain unchanged.
Closes farion1231#2365.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c92ff2591
ℹ️ 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 role == "assistant" && !tool_calls.is_empty() { | ||
| let reasoning_content = if reasoning_parts.is_empty() { | ||
| "tool call".to_string() |
There was a problem hiding this comment.
Restrict reasoning_content to compatible openai_chat providers
This change unconditionally adds reasoning_content on every assistant tool-call message, and even fabricates a placeholder when no thinking block exists. Because anthropic_to_openai is the shared transform for all openai_chat backends, strict Chat Completions implementations that validate message schemas can reject these requests with 400 on tool-call turns due to the extra message property. Please gate this field to providers/models that explicitly support it (or at least only emit it when real thinking content was present).
Useful? React with 👍 / 👎.
Closes #2365.
When
convert_message_to_openaiflattens an Anthropic assistant message that contains boththinkingandtool_useblocks, the thinking content is currently dropped. Some upstream providers ??Moonshot AI (kimi-k2.6) is the case in #2365 ??refuse the next request:This PR preserves the thinking text and re-emits it as
reasoning_contenton the converted assistant tool-call message.Changes
src-tauri/src/proxy/providers/transform.rs??convert_message_to_openai:thinkingblock text into areasoning_partsvec instead of skipping.role == "assistant"AND non-emptytool_calls, attachreasoning_content:\n"tool call"placeholder (Moonshot accepts non-empty; it only refuses missing)Design notes
reasoning_contentis additive; backends without thinking ignore it. Gating by something likeis_moonshot()would silently break the next provider that adopts the same constraint (DeepSeek's thinking mode is reportedly the same shape ??see cc switch接deepseek V4报“API Error: 400 {"error":{"message":"Thecontent[].thinkingin the thinking mode must be passed back to the API.","type":"invalid_request_error","param":null,"code":"invalid_request_error"}}” #2282).assistant+ tool_calls. Moonshot only validates this combination. Adding the field on other shapes is unnecessary.!content_parts.is_empty() || !tool_calls.is_empty()). Areasoning_parts-only message (no content, no tool_calls) would be invalid OpenAI Chat Completion shape, so the conservative behaviour is to keep dropping it. Happy to change this if a maintainer disagrees.Tests
Three new tests in the existing
#[cfg(test)]block:test_anthropic_to_openai_tool_use_preserves_reasoning_content??thinking + tool_use ??field carries the thinking text.test_anthropic_to_openai_tool_use_injects_placeholder_reasoning_content_when_missing??tool_use without thinking ??"tool call"placeholder.test_anthropic_to_openai_tool_use_assistant_text_only_does_not_inject_reasoning_content??guards against accidentally adding the field on plain assistant replies.Verification
rustfmt --checkclean on the file.cargo testdeferred to CI: the first dependency build in this WSL environment was pulling crates from upstream and would have taken ~20 min, so I opted to let CI be authoritative. Diff is small and rustfmt-clean.pnpm typecheck/pnpm format:checknot run (no.tstouched).