fix: preserve reasoning_content for DeepSeek thinking mode on multi-turn tool calls - #10366
Conversation
…urn tool calls Three fixes for the 'reasoning_content in thinking mode must be passed back' 400 error with DeepSeek/Kimi thinking models: 1. RedactedThinking was silently dropped in format_messages_with_options, causing reasoning_content to be missing for signed/redacted thinking. 2. When reasoning arrived across multiple streaming chunks, only the last thinking-only message was used (rposition). Now accumulates ALL prior thinking, even when direct_thinking is non-empty (reasoning on same chunk as tool_calls). 3. When thinking + text arrived in the same streaming chunk, the resulting mixed message (Thinking+Text) was not recognized as a thinking source by the prior-message filter (which required ALL content to be Thinking). Now extracts thinking from ANY prior assistant message.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36305599df
ℹ️ 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".
…te signed/unsigned reasoning
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db83395e5f
ℹ️ 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".
… opaque encrypted data, not reasoning text
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 561d0b7498
ℹ️ 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".
| } else if has_thinking { | ||
| // Strip thinking blocks from mixed messages so the same | ||
| // signed/unsigned thinking is not duplicated when it is | ||
| // carried onto the tool-call request messages below. | ||
| m.content.retain(|c| { |
There was a problem hiding this comment.
Keep thinking on earlier streamed tool calls
When a provider streams multiple tool-use blocks as separate messages, the first request_msg is already in messages_to_add as a mixed Thinking + ToolRequest by the time the second block reaches this loop; Anthropic's stream parser does yield each tool request on content_block_stop (crates/goose-provider-types/src/formats/anthropic.rs). This branch treats that earlier tool-call message like a prior mixed text chunk, strips its signed thinking, and reattaches it only to the later request, leaving the persisted split history with the thinking after the first tool result instead of on the first tool-call message (the conversation fixer’s signed-thinking dedupe expects the first split message to keep it). Skip messages that already contain tool requests when stripping prior mixed chunks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch! When a provider yields multiple tool-use blocks as separate stream events (e.g. Anthropic content_block_stop), the thinking accumulation code runs for each block. On the second pass, the already-split request_msg from the first tool call sits in messages_to_add with its thinking properly attached — but the else if has_thinking branch treated it like a mixed text chunk and stripped the thinking. Fixed in c688b6d: the retain branch now skips messages that already contain ToolRequest content, since those are prior-split request_msg items that must keep their thinking intact.
…prior thinking chunks
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c688b6d725
ℹ️ 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".
…ly when stripping
|
thanks! |
- feat: allow askai bot to read issues and PRs (aaif-goose#10398) - fix: Fix Gemini OAuth onboarding (aaif-goose#10363) - chore: remove unavailable models from cerebras catalog (aaif-goose#10407) - feat(cli): Fix output.rs merge conflicts - chore(deps): bump actions/stale from 10.3.0 to 10.4.0 (aaif-goose#10371) - chore(deps): bump dorny/paths-filter from 4.0.1 to 4.0.2 (aaif-goose#10370) - chore(deps): bump actions/cache from 6.0.0 to 6.1.0 (aaif-goose#10374) - fix: treat empty-string finish_reason as non-terminal in streaming (aaif-goose#10258) - chore(deps): bump step-security/harden-runner from 2.19.4 to 2.20.0 (aaif-goose#10372) - chore(deps): bump docker/login-action from 4.2.0 to 4.4.0 (aaif-goose#10373) - test: drop unavailable and preview Gemini smoke models (aaif-goose#10355) - fix(ui): restore pnpm 11 project configuration (aaif-goose#10395) - Fix/compact disabled during approval (aaif-goose#10089) - fix(prompt): make prompt timestamps timezone-explicit (aaif-goose#10209) - fix: preserve reasoning_content for DeepSeek thinking mode on multi-turn tool calls (aaif-goose#10366) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Picks up the DeepSeek/Kimi thinking-mode fix (aaif-goose/goose#10366): on multi-turn tool calls the agent now accumulates all prior thinking blocks, keeps RedactedThinking, and extracts reasoning from mixed thinking+text messages, so providers stop rejecting turns with "reasoning_content in thinking mode must be passed back" 400s. Contract surfaces the ACP bridge depends on were checked against the v1.41.0..v1.43.0 diff and are unchanged: the `--with-builtin` flag, GOOSE_PATH_ROOT, the `YYYYMMDD_<n>` session id format, the `_goose/unstable/session/update` usage_update payload with its accumulated token counters, `_meta.goose.toolCall.toolName`, and every GOOSE_*/OPENAI_* env var applyProviderEnv sets. Two behaviour changes worth knowing about: - Session usage totals are now derived as max(sessions.accumulated_*, SUM(usage_ledger)) and roll subagent sessions up into their parent, so accumulated counters grow to include delegated work that used to go unbilled. The added `message_usage` notification is ignored by the bridge, which only reads usage_update. - The developer shell tool falls back to DEFAULT_EXTENSION_TIMEOUT (300s) when a call omits timeout_secs; it used to run unbounded. Left at the upstream default. The session store migrates v15 -> v16 (adds the usage_ledger table and sessions.parent_session_id); both steps are additive, so a rollback to v1.41.0 still reads the schema. Verified in a linux/amd64 container: the pinned asset downloads and reports 1.43.0, `goose acp --help` still offers --with-builtin, and a real initialize + session/new handshake returns session id 20260721_1, loads the developer builtin, creates the store under GOOSE_PATH_ROOT, and emits _goose/unstable/session/update with accumulatedInputTokens / accumulatedOutputTokens. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNkGCVRVYMSziXZFbCTG68
Summary
Fixes four issues in thinking/reasoning preservation for DeepSeek/Kimi thinking models on multi-turn tool calls:
RedactedThinking was silently dropped in
format_messages_with_options—reasoning_contentwas missing for signed/redacted thinking blocks.Only the last thinking-only message was used when reasoning arrived across multiple streaming chunks. Now accumulates ALL prior thinking, even when
direct_thinkingis non-empty (reasoning on same chunk as tool_calls).Mixed thinking+text messages were not recognized as a thinking source. The prior-message filter required ALL content to be Thinking. Now extracts thinking from ANY prior assistant message.
Thinking from mixed messages created duplicate signed blocks (review feedback). When thinking was cloned from a mixed (thinking+text) message onto a tool-call message, the original mixed message kept its thinking blocks. Since
fix_conversation/dedupe_signed_thinkingdoes not run between turns in the multi-turn loop, the duplicate signed blocks reached the provider on the next API call — causing a 400 rejection from Anthropic-style providers. Now thinking blocks are stripped from mixed messages after cloning.Changes
crates/goose-provider-types/src/formats/openai.rsRedactedThinking.datais now appended toreasoning_textinstead of being skippedcrates/goose/src/agents/agent.rsdirect_thinkingTesting
format_messageswithpreserve_thinking_contextRelated Issues
Closes #9397
Closes #9434
Closes #9675
Closes #9891
Closes #10012