From dea9cd8f2eff6c65f1ecd9bcedcfab206ea80347 Mon Sep 17 00:00:00 2001 From: Jay <12496124+MysticalMount@users.noreply.github.com> Date: Wed, 14 Oct 2026 16:18:17 +0100 Subject: [PATCH] fix(oaistream): replay reasoning content with opt-out Fixes #4363 Signed-off-by: Jay <12496124+MysticalMount@users.noreply.github.com> --- agent-schema.json | 2 +- docs/providers/custom/index.md | 40 +++++ docs/providers/openai/index.md | 12 +- examples/reasoning_replay.yaml | 52 ++++++ pkg/chat/chat.go | 15 +- pkg/model/provider/dmr/client.go | 5 + .../provider/dmr/reasoning_replay_test.go | 73 ++++++++ pkg/model/provider/oaistream/messages.go | 35 +++- pkg/model/provider/oaistream/messages_test.go | 164 +++++++++++++++++ .../provider/oaistream/reasoning_replay.go | 75 ++++++++ .../oaistream/reasoning_replay_test.go | 103 +++++++++++ pkg/model/provider/openai/client.go | 13 ++ .../provider/openai/reasoning_replay_test.go | 167 ++++++++++++++++++ pkg/session/history_reasoning_replay_test.go | 100 +++++++++++ 14 files changed, 847 insertions(+), 9 deletions(-) create mode 100644 examples/reasoning_replay.yaml create mode 100644 pkg/model/provider/dmr/reasoning_replay_test.go create mode 100644 pkg/model/provider/oaistream/reasoning_replay.go create mode 100644 pkg/model/provider/oaistream/reasoning_replay_test.go create mode 100644 pkg/model/provider/openai/reasoning_replay_test.go create mode 100644 pkg/session/history_reasoning_replay_test.go diff --git a/agent-schema.json b/agent-schema.json index 9fe60c4b3d..741109e15e 100644 --- a/agent-schema.json +++ b/agent-schema.json @@ -1756,7 +1756,7 @@ "additionalProperties": true } }, - "description": "Provider-specific options. Anthropic: thinking_prefix_mismatch (error or drop_block; defaults to drop_block on Fable/Mythos 5.1), strict_tools (true for eligible schemas or a tool-name array requiring strict mode), cache_diagnostics (boolean; Claude API only), native_compaction (boolean; synchronous whole-history signed compaction on Claude API), cache_preserving_updates (boolean; native system/tool/effort changes with turn-scoped reminders on supported models), thinking_display: updates (progress updates beta on Fable 5+ and Mythos 5.1). Sampling parameters: top_k (integer, supported by anthropic, google, amazon-bedrock, and custom OpenAI-compatible providers like vLLM/Ollama), repetition_penalty (float, forwarded to custom OpenAI-compatible providers), min_p (float, forwarded to custom providers), seed (integer, forwarded to OpenAI). Lifecycle: unload_api (string) overrides the unload endpoint inherited from the provider config (relative path resolved against base_url's scheme+host, or an absolute URL); used by the runtime's `unload` on_agent_switch builtin hook to release model resources between agent switches. Infrastructure options: http_headers (map of string to string, adds custom HTTP headers to every request; used for OpenAI-compatible providers like github-copilot which requires Copilot-Integration-Id). dmr: runtime_flags; supports_images (boolean) and supports_pdf (boolean) declare which document attachment types the local model accepts. DMR-hosted models are not in the models.dev catalog, so attachment capabilities cannot be detected automatically and default to text-only; set supports_images: true for a vision model so image attachments are forwarded instead of silently dropped. anthropic/amazon-bedrock (Claude): interleaved_thinking (boolean, default true). anthropic: thinking_display ('summarized', 'omitted', 'updates', or 'display') controls whether thinking blocks are returned in responses when thinking is enabled. Claude Opus 4.7 hides thinking by default ('omitted'); set thinking_display: summarized to receive thinking blocks. 'display' (full thinking blocks) is only accepted by pre-4.6 token-thinking models (e.g. Sonnet 4.5, Haiku 4.5); models from the adaptive-thinking generation onward (Opus/Sonnet 4.6+, Sonnet 5, Fable 5) reject it, and docker-agent fails fast with a configuration error (fallbacks models are validated too); fallbacks (array of model ID strings, in priority order; enables the server-side-fallback beta so requests refused by safety classifiers, e.g. on Claude Fable 5, are retried with the listed models in a single round trip). OpenAI-compatible APIs: service_tier (string, forwarded unchanged to Chat Completions and Responses, including WebSocket and reranking; 'fast' or 'priority' enables Fast mode at premium pricing on supported models; omitted or empty leaves the API default, non-string values are ignored). openai/chatgpt: preserve_reasoning (boolean, default false) preserves ordered Responses output and encrypted reasoning for stateless continuation; native_tool_search also enables output replay. openai: cache_diagnostics (boolean, default false) logs Responses prompt-cache comparison results with --debug on GPT-5.6+; comparisons are scoped to the current session and resolved model; ignored for chatgpt and custom base_url endpoints. transport ('sse' or 'websocket') to choose between SSE and WebSocket streaming for the Responses API; native_tool_search (boolean, default false; also enables ordered output replay) declares the agent's deferred tools with defer_loading and a server-executed tool_search tool so OpenAI's hosted tool search discovers them (only openai models supporting deferred tools, gpt-5.4+; ignored on chatgpt, OpenAI-compatible endpoints including a custom base_url, and Chat Completions requests). openai/anthropic/google: rerank_prompt (string) to fully override the system prompt used for RAG reranking (advanced - prefer using results.reranking.criteria for domain-specific guidance). Google: google_search (boolean) enables Google Search grounding, google_maps (boolean) enables Google Maps grounding, code_execution (boolean) enables server-side code execution, url_context (boolean) enables reading public URLs during generation, output_dimensionality (positive integer) truncates Gemini embedding vectors to that size when the model is used as a RAG embedding_model; service_tier (string, forwarded unchanged as the top-level serviceTier of every generateContent request, including reranking, title generation, and compaction, on the Gemini API, Vertex AI, and the Docker models gateway; 'flex' for discounted, queued processing that may wait up to 15 minutes before the first byte, 'standard' for the default tier, 'priority' for premium processing; omitted or empty leaves the API default, non-string values are ignored; cost estimates are not tier-adjusted, set the model's cost override).", + "description": "Provider-specific options. Anthropic: thinking_prefix_mismatch (error or drop_block; defaults to drop_block on Fable/Mythos 5.1), strict_tools (true for eligible schemas or a tool-name array requiring strict mode), cache_diagnostics (boolean; Claude API only), native_compaction (boolean; synchronous whole-history signed compaction on Claude API), cache_preserving_updates (boolean; native system/tool/effort changes with turn-scoped reminders on supported models), thinking_display: updates (progress updates beta on Fable 5+ and Mythos 5.1). Sampling parameters: top_k (integer, supported by anthropic, google, amazon-bedrock, and custom OpenAI-compatible providers like vLLM/Ollama), repetition_penalty (float, forwarded to custom OpenAI-compatible providers), min_p (float, forwarded to custom providers), seed (integer, forwarded to OpenAI). Lifecycle: unload_api (string) overrides the unload endpoint inherited from the provider config (relative path resolved against base_url's scheme+host, or an absolute URL); used by the runtime's `unload` on_agent_switch builtin hook to release model resources between agent switches. Infrastructure options: http_headers (map of string to string, adds custom HTTP headers to every request; used for OpenAI-compatible providers like github-copilot which requires Copilot-Integration-Id). dmr: runtime_flags; supports_images (boolean) and supports_pdf (boolean) declare which document attachment types the local model accepts. DMR-hosted models are not in the models.dev catalog, so attachment capabilities cannot be detected automatically and default to text-only; set supports_images: true for a vision model so image attachments are forwarded instead of silently dropped. anthropic/amazon-bedrock (Claude): interleaved_thinking (boolean, default true). anthropic: thinking_display ('summarized', 'omitted', 'updates', or 'display') controls whether thinking blocks are returned in responses when thinking is enabled. Claude Opus 4.7 hides thinking by default ('omitted'); set thinking_display: summarized to receive thinking blocks. 'display' (full thinking blocks) is only accepted by pre-4.6 token-thinking models (e.g. Sonnet 4.5, Haiku 4.5); models from the adaptive-thinking generation onward (Opus/Sonnet 4.6+, Sonnet 5, Fable 5) reject it, and docker-agent fails fast with a configuration error (fallbacks models are validated too); fallbacks (array of model ID strings, in priority order; enables the server-side-fallback beta so requests refused by safety classifiers, e.g. on Claude Fable 5, are retried with the listed models in a single round trip). OpenAI-compatible APIs: service_tier (string, forwarded unchanged to Chat Completions and Responses, including WebSocket and reranking; 'fast' or 'priority' enables Fast mode at premium pricing on supported models; omitted or empty leaves the API default, non-string values are ignored). openai/chatgpt: preserve_reasoning (boolean, default false) preserves ordered Responses output and encrypted reasoning for stateless continuation; native_tool_search also enables output replay. Any OpenAI-compatible destination: replay_reasoning (boolean) replays the reasoning stored on assistant turns back as a reasoning_content field on later Chat Completions requests, so a reasoning model sees its own earlier reasoning instead of re-deriving it after each tool call (distinct from preserve_reasoning, which replays encrypted reasoning items on the Responses API and does not apply to Chat Completions); default true for Chat Completions; set replay_reasoning to false for an endpoint that rejects or must not receive the non-standard field; a non-boolean value also disables replay. openai: cache_diagnostics (boolean, default false) logs Responses prompt-cache comparison results with --debug on GPT-5.6+; comparisons are scoped to the current session and resolved model; ignored for chatgpt and custom base_url endpoints. transport ('sse' or 'websocket') to choose between SSE and WebSocket streaming for the Responses API; native_tool_search (boolean, default false; also enables ordered output replay) declares the agent's deferred tools with defer_loading and a server-executed tool_search tool so OpenAI's hosted tool search discovers them (only openai models supporting deferred tools, gpt-5.4+; ignored on chatgpt, OpenAI-compatible endpoints including a custom base_url, and Chat Completions requests). openai/anthropic/google: rerank_prompt (string) to fully override the system prompt used for RAG reranking (advanced - prefer using results.reranking.criteria for domain-specific guidance). Google: google_search (boolean) enables Google Search grounding, google_maps (boolean) enables Google Maps grounding, code_execution (boolean) enables server-side code execution, url_context (boolean) enables reading public URLs during generation, output_dimensionality (positive integer) truncates Gemini embedding vectors to that size when the model is used as a RAG embedding_model; service_tier (string, forwarded unchanged as the top-level serviceTier of every generateContent request, including reranking, title generation, and compaction, on the Gemini API, Vertex AI, and the Docker models gateway; 'flex' for discounted, queued processing that may wait up to 15 minutes before the first byte, 'standard' for the default tier, 'priority' for premium processing; omitted or empty leaves the API default, non-string values are ignored; cost estimates are not tier-adjusted, set the model's cost override).", "additionalProperties": true }, "track_usage": { diff --git a/docs/providers/custom/index.md b/docs/providers/custom/index.md index 54ec3b0e32..3be5e236ae 100644 --- a/docs/providers/custom/index.md +++ b/docs/providers/custom/index.md @@ -159,6 +159,46 @@ agents: > > Models that stream reasoning under `delta.reasoning` (e.g. Qwen3 served via OVHcloud AI Endpoints, OpenRouter, or a self-hosted vLLM / SGLang deployment) are fully supported. Docker Agent reads both the `delta.reasoning_content` and `delta.reasoning` fields from the stream, so thinking blocks are captured and shown in the TUI regardless of which field the server uses. +### Replaying reasoning on later turns + +Captured reasoning is sent back to the model on subsequent requests as a `reasoning_content` field on the assistant message. Servers that understand the field read it back, so the model sees its own earlier reasoning instead of re-deriving it after every tool call. Reasoning that is only useful to the model is not free: replaying it can multiply the size of a long request, so it is limited to turns the session keeps anyway (those with content or a tool call). + +Chat Completions reasoning replay defaults on. Because `reasoning_content` is a +non-standard extension, set `provider_opts.replay_reasoning: false` for a model +or provider whose endpoint rejects or must not receive it. An invalid, +non-boolean value also fails closed and disables replay. + +Set the opt-out on a model, or once on a provider and let its models inherit it: + +```yaml +providers: + local_llm: + base_url: http://localhost:8000/v1 + provider_opts: + replay_reasoning: false # opt out every local_llm model + +models: + qwen_default: + provider: local_llm + model: qwen3 + provider_opts: + replay_reasoning: true # optional: replay is already the default + +agents: + root: + model: qwen_default +``` + +This option is specific to the Chat Completions API and its `reasoning_content` +field. It is not the same as `preserve_reasoning`, which replays the encrypted +reasoning items of first-party OpenAI Responses sessions; neither option +applies to the other API. See [Reasoning state preservation](../openai/index.md#reasoning-state-preservation). + +The field is worth a look if you switch models mid-session or use fallbacks: +Docker Agent stores reasoning from every provider in one place, so a later Chat +Completions model may otherwise receive reasoning captured from another vendor. +Set `replay_reasoning: false` on that destination when this is undesirable. + ### API Router (Requesty, LiteLLM) ```yaml diff --git a/docs/providers/openai/index.md b/docs/providers/openai/index.md index 6924edd686..04f2011ede 100644 --- a/docs/providers/openai/index.md +++ b/docs/providers/openai/index.md @@ -218,9 +218,9 @@ generated values. This option does not apply to the Responses API. ## Reasoning State Preservation -Reasoning replay is opt-in, so existing configurations keep their request and -session-storage behavior. Enable it for first-party OpenAI Responses sessions, -including the `chatgpt` provider: +Responses reasoning replay is opt-in, so existing configurations keep their +request and session-storage behavior. Enable it for first-party OpenAI +Responses sessions, including the `chatgpt` provider: ```yaml provider_opts: @@ -247,3 +247,9 @@ WebSocket requests replay the full history without also chaining it through `pre See [OpenAI's reasoning guide](https://developers.openai.com/api/docs/guides/reasoning) and [cache diagnostics guide](https://developers.openai.com/api/docs/guides/prompt-caching/diagnostics). + +`preserve_reasoning` is unrelated to `provider_opts.replay_reasoning`, which +replays plain reasoning text as a `reasoning_content` field on Chat +Completions requests to OpenAI-compatible servers. It does not apply to the +Responses API, and `preserve_reasoning` does not apply to Chat Completions. +See [Replaying reasoning on later turns](../custom/index.md#replaying-reasoning-on-later-turns). diff --git a/examples/reasoning_replay.yaml b/examples/reasoning_replay.yaml new file mode 100644 index 0000000000..fc38fc4fe3 --- /dev/null +++ b/examples/reasoning_replay.yaml @@ -0,0 +1,52 @@ +# Example: Replaying reasoning to an OpenAI-compatible model +# +# Docker Agent captures reasoning from reasoning models (delta.reasoning_content +# or delta.reasoning on OpenAI-compatible streams) and replays it on later turns, +# so the model sees its own earlier reasoning instead of re-deriving it after +# every tool call. The replay is sent as a reasoning_content field on the +# assistant message. +# +# Replay defaults on for Chat Completions. Set +# provider_opts.replay_reasoning: false when an endpoint rejects or must not +# receive the non-standard field. Replayed reasoning counts against the context +# window, so a model that reasons at length is also the case where opting out +# saves the most tokens. +# +# This option is Chat Completions only. The Responses API has a separate +# preserve_reasoning option for its encrypted reasoning items; neither option +# applies to the other API. + +providers: + # A local server fronting an open-weight reasoning model. Replay is already + # on for this destination; the explicit setting is shown for contrast. + local_vllm: + provider: openai + api_type: openai_chatcompletions + base_url: http://localhost:8000/v1 + +models: + qwen_replay: + provider: local_vllm + model: qwen3-30b-a3b + provider_opts: + replay_reasoning: true + + # Same server, but this model should never receive stored reasoning. + qwen_no_replay: + provider: local_vllm + model: qwen3-30b-a3b + provider_opts: + replay_reasoning: false + +agents: + root: + model: qwen_replay + description: Main assistant, replaying its own reasoning + instruction: | + You are a helpful AI assistant. Be concise and helpful in your responses. + + analyst: + model: qwen_no_replay + description: Sub-agent that starts each turn from scratch + instruction: | + You are a specialized assistant for specific tasks. diff --git a/pkg/chat/chat.go b/pkg/chat/chat.go index c7aa8a8d63..e2a49677e1 100644 --- a/pkg/chat/chat.go +++ b/pkg/chat/chat.go @@ -56,10 +56,17 @@ type Message struct { Content string `json:"content"` MultiContent []MessagePart `json:"multi_content,omitempty"` - // This property is used for the "reasoning" feature supported by deepseek-reasoner - // which is not in the official documentation. - // the doc from deepseek: - // - https://api-docs.deepseek.com/api/create-chat-completion#responses + // ReasoningContent carries the reasoning text of a reasoning model. It is a + // generic internal field, not one wire field: it is filled from + // delta.reasoning_content / delta.reasoning on OpenAI-compatible streams + // (DeepSeek, Qwen, vLLM, llama.cpp, ...), from Anthropic and Bedrock thinking + // blocks, from Gemini reasoning parts, and from the OpenAI Responses API. + // Providers that keep reasoning signed or encrypted of their own store that + // alongside in ThinkingSignature or ThoughtSignature. + // + // Replaying it on later turns stops a reasoning model from re-deriving work it + // already produced. Chat Completions replay defaults on and can be disabled + // with provider_opts.replay_reasoning for incompatible endpoints. ReasoningContent string `json:"reasoning_content,omitempty"` // ThinkingSignature is used for Anthropic's extended thinking feature diff --git a/pkg/model/provider/dmr/client.go b/pkg/model/provider/dmr/client.go index de003fe01d..f294056d76 100644 --- a/pkg/model/provider/dmr/client.go +++ b/pkg/model/provider/dmr/client.go @@ -195,6 +195,11 @@ func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) [ if override := c.CapsOverride(); override != nil { caps = modelinfo.CapsWith(override.Image, override.PDF, override.Audio, override.Video) } + // Chat Completions reasoning replay defaults on; + // provider_opts.replay_reasoning: false opts out. + if !oaistream.ReplayReasoning(c.ModelConfig.ProviderOpts) { + messages = oaistream.WithoutReplayedReasoning(messages) + } openaiMessages := oaistream.ConvertMessagesWithCaps(ctx, messages, caps) return oaistream.MergeConsecutiveMessages(openaiMessages) } diff --git a/pkg/model/provider/dmr/reasoning_replay_test.go b/pkg/model/provider/dmr/reasoning_replay_test.go new file mode 100644 index 0000000000..495ccb3ba2 --- /dev/null +++ b/pkg/model/provider/dmr/reasoning_replay_test.go @@ -0,0 +1,73 @@ +package dmr + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/chat" + "github.com/docker/docker-agent/pkg/config/latest" + "github.com/docker/docker-agent/pkg/model/provider/base" + "github.com/docker/docker-agent/pkg/tools" +) + +// Chat Completions reasoning replay defaults on and +// provider_opts.replay_reasoning: false turns it off. +func TestConvertMessages_ReasoningReplay(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + opts map[string]any + wantSent bool + }{ + {name: "on by default", wantSent: true}, + {name: "explicit opt-out", opts: map[string]any{"replay_reasoning": false}, wantSent: false}, + {name: "explicit true keeps replay enabled", opts: map[string]any{"replay_reasoning": true}, wantSent: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + c := &Client{Config: base.Config{ModelConfig: latest.ModelConfig{ + Provider: "dmr", + Model: "ai/qwen3", + ProviderOpts: tt.opts, + }}} + + converted := c.convertMessages(t.Context(), []chat.Message{ + {Role: chat.MessageRoleUser, Content: "weather?"}, + { + Role: chat.MessageRoleAssistant, + Content: "checking", + ReasoningContent: "I should call the weather tool", + ToolCalls: []tools.ToolCall{{ID: "c1", Function: tools.FunctionCall{Name: "weather", Arguments: `{}`}}}, + }, + }) + + raw, err := json.Marshal(converted) + require.NoError(t, err) + + var msgs []map[string]any + require.NoError(t, json.Unmarshal(raw, &msgs)) + + var assistant map[string]any + for _, m := range msgs { + if m["role"] == "assistant" { + assistant = m + } + } + require.NotNil(t, assistant, "assistant turn must survive conversion: %s", raw) + + if tt.wantSent { + assert.Equal(t, "I should call the weather tool", assistant["reasoning_content"]) + } else { + assert.NotContains(t, assistant, "reasoning_content") + } + assert.Len(t, assistant["tool_calls"], 1) + }) + } +} diff --git a/pkg/model/provider/oaistream/messages.go b/pkg/model/provider/oaistream/messages.go index 0f724ce5df..5ef502ea38 100644 --- a/pkg/model/provider/oaistream/messages.go +++ b/pkg/model/provider/oaistream/messages.go @@ -85,7 +85,7 @@ func convertMessagesWithCaps(ctx context.Context, messages []chat.Message, mc mo msg := &messages[i] // Skip invalid assistant messages upfront. This can happen if the model is out of tokens (max_tokens reached) - if msg.Role == chat.MessageRoleAssistant && len(msg.ToolCalls) == 0 && len(msg.MultiContent) == 0 && strings.TrimSpace(msg.Content) == "" { + if msg.Role == chat.MessageRoleAssistant && len(msg.ToolCalls) == 0 && len(msg.MultiContent) == 0 && strings.TrimSpace(msg.Content) == "" && strings.TrimSpace(msg.ReasoningContent) == "" { continue } @@ -117,10 +117,12 @@ func convertMessagesWithCaps(ctx context.Context, messages []chat.Message, mc mo case chat.MessageRoleAssistant: assistantParam := openai.ChatCompletionAssistantMessageParam{} + hasContent := false if len(msg.MultiContent) == 0 { if msg.Content != "" { assistantParam.Content.OfString = param.NewOpt(msg.Content) + hasContent = true } } else { // Convert multi-content for assistant messages @@ -136,9 +138,33 @@ func convertMessagesWithCaps(ctx context.Context, messages []chat.Message, mc mo } if len(contentParts) > 0 { assistantParam.Content.OfArrayOfContentParts = contentParts + hasContent = true } } + // Replay stored reasoning so a reasoning model can see its own previous + // reasoning instead of re-deriving it (#4363). openai-go has no + // reasoning_content field on the assistant param -- it is a non-standard + // extension (openai/openai-go#558) -- so it can only go through + // SetExtraFields. + // + // Scope: this replays reasoning carried by a turn that is retained in + // session history, i.e. one that also has content or tool calls. A turn + // whose only payload is reasoning is dropped earlier, by the history + // filters (session.isUsageOnlyMessage and session.normalizeMessageContent + // both key on content and tool calls), so it never reaches this + // converter. Retaining such turns is a separate change with context-size + // consequences. + // + // Whether reasoning reaches this converter at all is the caller's policy; + // provider clients clear it when replay_reasoning is disabled. See + // [ReplayReasoning]. + if strings.TrimSpace(msg.ReasoningContent) != "" { + assistantParam.SetExtraFields(map[string]any{ + "reasoning_content": msg.ReasoningContent, + }) + } + if msg.FunctionCall != nil { assistantParam.FunctionCall.Name = msg.FunctionCall.Name assistantParam.FunctionCall.Arguments = msg.FunctionCall.Arguments @@ -160,6 +186,13 @@ func convertMessagesWithCaps(ctx context.Context, messages []chat.Message, mc mo assistantParam.ToolCalls = toolCalls } + // content is only optional when tool_calls are present, and llama.cpp rejects + // an assistant message carrying neither, so a reasoning-only turn must still + // send content: "". See issue #4363. + if !hasContent && len(assistantParam.ToolCalls) == 0 { + assistantParam.Content.OfString = param.NewOpt("") + } + openaiMessage.OfAssistant = &assistantParam case chat.MessageRoleTool: diff --git a/pkg/model/provider/oaistream/messages_test.go b/pkg/model/provider/oaistream/messages_test.go index 91e40368a3..72c1920c89 100644 --- a/pkg/model/provider/oaistream/messages_test.go +++ b/pkg/model/provider/oaistream/messages_test.go @@ -1,6 +1,7 @@ package oaistream import ( + "encoding/json" "testing" "github.com/openai/openai-go/v3" @@ -117,6 +118,16 @@ func TestConvertMessages(t *testing.T) { }, want: 1, }, + { + name: "reasoning-only assistant message not skipped", + messages: []chat.Message{{Role: chat.MessageRoleAssistant, ReasoningContent: "thinking"}}, + want: 1, + }, + { + name: "whitespace-only reasoning assistant message skipped", + messages: []chat.Message{{Role: chat.MessageRoleAssistant, ReasoningContent: " "}}, + want: 0, + }, { name: "tool message", messages: []chat.Message{ @@ -225,3 +236,156 @@ func TestJSONSchema_MarshalJSON(t *testing.T) { assert.Contains(t, string(data), `"type":"object"`) assert.Contains(t, string(data), `"properties"`) } + +// TestConvertMessages_ReasoningContentWireFormat covers issue #4363. Assertions are on +// the serialized request body, not on the SDK struct: a field that openai-go omits is +// invisible to struct-level assertions, and llama.cpp rejects an assistant message that +// carries neither content nor tool_calls. +func TestConvertMessages_ReasoningContentWireFormat(t *testing.T) { + t.Parallel() + + store := modelsdev.NewDatabaseStore(&modelsdev.Database{}) + + tests := []struct { + name string + msg chat.Message + want map[string]any + }{ + { + // A reasoning model that exhausts its output budget mid-reasoning leaves an + // assistant turn with no content and no tool calls. Dropping it loses the + // reasoning; keeping it with no content is rejected by the backend. + name: "reasoning-only turn replays reasoning with empty content", + msg: chat.Message{ + Role: chat.MessageRoleAssistant, + ReasoningContent: "the answer requires two steps", + }, + want: map[string]any{ + "role": "assistant", + "content": "", + "reasoning_content": "the answer requires two steps", + }, + }, + { + name: "reasoning alongside text", + msg: chat.Message{ + Role: chat.MessageRoleAssistant, + Content: "Paris.", + ReasoningContent: "France's capital, from memory", + }, + want: map[string]any{ + "role": "assistant", + "content": "Paris.", + "reasoning_content": "France's capital, from memory", + }, + }, + { + // content stays absent here: tool_calls satisfies the backend's requirement. + name: "reasoning alongside tool calls", + msg: chat.Message{ + Role: chat.MessageRoleAssistant, + ReasoningContent: "I should call the weather tool", + ToolCalls: []tools.ToolCall{{ + ID: "call_1", + Function: tools.FunctionCall{ + Name: "weather", + Arguments: `{"city":"Paris"}`, + }, + }}, + }, + want: map[string]any{ + "role": "assistant", + "reasoning_content": "I should call the weather tool", + "tool_calls": []any{map[string]any{ + "id": "call_1", + "type": "function", + "function": map[string]any{ + "name": "weather", + "arguments": `{"city":"Paris"}`, + }, + }}, + }, + }, + { + name: "no stored reasoning adds no extension", + msg: chat.Message{Role: chat.MessageRoleAssistant, Content: "Hi there!"}, + want: map[string]any{ + "role": "assistant", + "content": "Hi there!", + }, + }, + { + name: "whitespace-only reasoning is not replayed", + msg: chat.Message{ + Role: chat.MessageRoleAssistant, + Content: "Hi there!", + ReasoningContent: " ", + }, + want: map[string]any{ + "role": "assistant", + "content": "Hi there!", + }, + }, + { + name: "reasoning is replayed on a multi-content turn", + msg: chat.Message{ + Role: chat.MessageRoleAssistant, + MultiContent: []chat.MessagePart{{Type: chat.MessagePartTypeText, Text: "answer"}}, + ReasoningContent: "reasoning", + }, + want: map[string]any{ + "role": "assistant", + "content": []any{map[string]any{"type": "text", "text": "answer"}}, + "reasoning_content": "reasoning", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + result := ConvertMessages(t.Context(), []chat.Message{tt.msg}, modelsdev.ID{}, store, nil) + require.Len(t, result, 1) + require.NotNil(t, result[0].OfAssistant) + + data, err := json.Marshal(result[0]) + require.NoError(t, err) + + var got map[string]any + require.NoError(t, json.Unmarshal(data, &got)) + assert.Equal(t, tt.want, got, "serialized assistant message: %s", data) + }) + } +} + +// TestConvertMessages_AssistantAlwaysMarshalable asserts every assistant turn that +// survives conversion serializes to a shape llama.cpp accepts: either content or +// tool_calls must be present. +func TestConvertMessages_AssistantAlwaysMarshalable(t *testing.T) { + t.Parallel() + + store := modelsdev.NewDatabaseStore(&modelsdev.Database{}) + + messages := []chat.Message{ + {Role: chat.MessageRoleAssistant, ReasoningContent: "thinking"}, + {Role: chat.MessageRoleAssistant, Content: "text"}, + {Role: chat.MessageRoleAssistant, MultiContent: []chat.MessagePart{{Type: chat.MessagePartTypeImageURL, ImageURL: &chat.MessageImageURL{URL: "http://example.com/i.png"}}}}, + {Role: chat.MessageRoleAssistant, ToolCalls: []tools.ToolCall{{ID: "call_1", Function: tools.FunctionCall{Name: "f", Arguments: "{}"}}}}, + } + + result := ConvertMessages(t.Context(), messages, modelsdev.ID{}, store, nil) + require.Len(t, result, len(messages)) + + for i, converted := range result { + data, err := json.Marshal(converted) + require.NoError(t, err) + + var got map[string]any + require.NoError(t, json.Unmarshal(data, &got)) + _, hasContent := got["content"] + _, hasToolCalls := got["tool_calls"] + assert.True(t, hasContent || hasToolCalls, + "message %d rejected by llama.cpp: assistant message must contain either 'content' or 'tool_calls', got %s", i, data) + } +} diff --git a/pkg/model/provider/oaistream/reasoning_replay.go b/pkg/model/provider/oaistream/reasoning_replay.go new file mode 100644 index 0000000000..5c2d9dd942 --- /dev/null +++ b/pkg/model/provider/oaistream/reasoning_replay.go @@ -0,0 +1,75 @@ +package oaistream + +/* +This file holds the policy for replaying stored assistant reasoning to an +OpenAI-compatible endpoint, and the message rewrite that policy requires. +*/ + +import ( + "log/slog" + "slices" + "sync" + + "github.com/docker/docker-agent/pkg/chat" +) + +// ProviderOptReplayReasoning is the provider_opts key that overrides, per +// model, whether stored assistant reasoning is replayed as a reasoning_content +// field on Chat Completions requests. A boolean; when unset, replay defaults +// on. It does not apply to the Responses API, where +// provider_opts.preserve_reasoning governs the replay of encrypted reasoning +// items instead. +const ProviderOptReplayReasoning = "replay_reasoning" + +// ReplayReasoning reports whether assistant reasoning stored in the session may +// be replayed to a model's endpoint. Replay defaults on. An explicit +// provider_opts.replay_reasoning overrides the default. A +// value that is present but not a boolean turns replay off instead of falling +// back to the default: the option is how an operator keeps reasoning away from +// an endpoint, so a typo must not leave replay enabled. +// +// Replaying [chat.Message.ReasoningContent] lets a reasoning model see its own +// previous reasoning after a tool call (issue #4363). Because reasoning_content +// is a non-standard extension, the explicit false value is the compatibility +// escape hatch for endpoints that reject or must not receive it. +func ReplayReasoning(providerOpts map[string]any) bool { + value, present := providerOpts[ProviderOptReplayReasoning] + if !present { + return true + } + if enabled, ok := value.(bool); ok { + return enabled + } + warnInvalidReplayReasoning.Do(func() { + slog.Warn("Rejecting provider_opts.replay_reasoning: value is not a boolean; assistant reasoning will not be replayed", "value", value) + }) + return false +} + +// warnInvalidReplayReasoning keeps the diagnostic above to once per process: +// the option is read on every request. +var warnInvalidReplayReasoning sync.Once + +// WithoutReplayedReasoning returns messages with assistant reasoning cleared, +// for a model that opted out with [ProviderOptReplayReasoning]. Only the +// reasoning field is +// affected: content, tool calls, and media are left alone, so an assistant turn +// that had both reasoning and content still converts to a valid message. +// +// The input is never mutated -- the slice is copied only when something has to +// change -- because the messages belong to the session, not to the provider. +func WithoutReplayedReasoning(messages []chat.Message) []chat.Message { + out := messages + copied := false + for i := range messages { + if messages[i].Role != chat.MessageRoleAssistant || messages[i].ReasoningContent == "" { + continue + } + if !copied { + out = slices.Clone(messages) + copied = true + } + out[i].ReasoningContent = "" + } + return out +} diff --git a/pkg/model/provider/oaistream/reasoning_replay_test.go b/pkg/model/provider/oaistream/reasoning_replay_test.go new file mode 100644 index 0000000000..5985803308 --- /dev/null +++ b/pkg/model/provider/oaistream/reasoning_replay_test.go @@ -0,0 +1,103 @@ +package oaistream + +import ( + "slices" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/docker/docker-agent/pkg/chat" + "github.com/docker/docker-agent/pkg/tools" +) + +func TestReplayReasoning(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + opts map[string]any + want bool + }{ + {name: "defaults on", want: true}, + { + name: "explicit true", + opts: map[string]any{ProviderOptReplayReasoning: true}, + want: true, + }, + { + name: "explicit false", + opts: map[string]any{ProviderOptReplayReasoning: false}, + want: false, + }, + { + // A quoted value is a typo, and the option exists to keep reasoning + // away from an endpoint, so it must fail closed. + name: "non-bool value fails closed", + opts: map[string]any{ProviderOptReplayReasoning: "yes"}, + want: false, + }, + { + name: "unrelated provider opts ignored", + opts: map[string]any{"context_size": float64(8192)}, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, ReplayReasoning(tt.opts)) + }) + } +} + +func TestWithoutReplayedReasoning(t *testing.T) { + t.Parallel() + + in := []chat.Message{ + {Role: chat.MessageRoleSystem, Content: "be helpful"}, + {Role: chat.MessageRoleUser, Content: "weather?"}, + { + Role: chat.MessageRoleAssistant, + ReasoningContent: "call the weather tool", + ToolCalls: []tools.ToolCall{{ID: "c1", Function: tools.FunctionCall{Name: "weather", Arguments: `{}`}}}, + }, + {Role: chat.MessageRoleTool, ToolCallID: "c1", Content: "sunny"}, + {Role: chat.MessageRoleAssistant, Content: "it is sunny", ReasoningContent: "answer"}, + } + // Snapshot so the no-mutation contract is checked against the original. + original := slices.Clone(in) + + got := WithoutReplayedReasoning(in) + + assert.Equal(t, "be helpful", got[0].Content) + assert.Equal(t, "weather?", got[1].Content) + + // Reasoning is gone, but the turn itself stays convertible. + assert.Empty(t, got[2].ReasoningContent) + assert.Len(t, got[2].ToolCalls, 1) + assert.Equal(t, "c1", got[2].ToolCalls[0].ID) + + assert.Equal(t, "sunny", got[3].Content) + assert.Empty(t, got[4].ReasoningContent) + assert.Equal(t, "it is sunny", got[4].Content) + + assert.Equal(t, original, in, "input messages must not be mutated") +} + +func TestWithoutReplayedReasoning_NothingToStrip(t *testing.T) { + t.Parallel() + + in := []chat.Message{ + {Role: chat.MessageRoleUser, Content: "hi"}, + {Role: chat.MessageRoleAssistant, Content: "hello"}, + // Reasoning on a non-assistant role is never converted, so it must not + // force a copy of the whole history. + {Role: chat.MessageRoleUser, Content: "again", ReasoningContent: "stray"}, + } + + got := WithoutReplayedReasoning(in) + + assert.Equal(t, in, got) + assert.Equal(t, "stray", got[2].ReasoningContent) +} diff --git a/pkg/model/provider/openai/client.go b/pkg/model/provider/openai/client.go index 89233da32d..e89607dba5 100644 --- a/pkg/model/provider/openai/client.go +++ b/pkg/model/provider/openai/client.go @@ -219,9 +219,22 @@ func (c *Client) Close() { } } +// replaysReasoning reports whether stored assistant reasoning is replayed as +// reasoning_content on Chat Completions requests. Replay defaults on; an +// explicit provider_opts.replay_reasoning value can opt out. +func (c *Client) replaysReasoning() bool { + return oaistream.ReplayReasoning(c.ModelConfig.ProviderOpts) +} + // convertMessages converts chat.Message to openai.ChatCompletionMessageParamUnion // using the shared oaistream implementation. func (c *Client) convertMessages(ctx context.Context, messages []chat.Message) []openai.ChatCompletionMessageParamUnion { + // Gated here rather than inside the converter so that every path below -- + // including the prompt-cache breakpoint path, which must keep computing the + // same body it will send -- sees the same messages. + if !c.replaysReasoning() { + messages = oaistream.WithoutReplayedReasoning(messages) + } // Explicit prompt-cache breakpoints take priority over the consecutive- // message merge below. [sendsExplicitCacheBreakpoints] only admits genuine // OpenAI vendor endpoints — including a trusted named custom OpenAI diff --git a/pkg/model/provider/openai/reasoning_replay_test.go b/pkg/model/provider/openai/reasoning_replay_test.go new file mode 100644 index 0000000000..8c2f132f0f --- /dev/null +++ b/pkg/model/provider/openai/reasoning_replay_test.go @@ -0,0 +1,167 @@ +package openai + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/chat" + "github.com/docker/docker-agent/pkg/config/latest" + "github.com/docker/docker-agent/pkg/model/provider/base" + "github.com/docker/docker-agent/pkg/tools" +) + +// assistantWire marshals the converted request and returns the first assistant +// message as a raw map, so the test asserts on the wire shape the endpoint +// actually receives -- reasoning_content is a non-standard extension that only +// survives marshalling, never the typed param. +func assistantWire(t *testing.T, c *Client, messages []chat.Message) map[string]any { + t.Helper() + + raw, err := json.Marshal(c.convertMessages(t.Context(), messages)) + require.NoError(t, err) + + var msgs []map[string]any + require.NoError(t, json.Unmarshal(raw, &msgs)) + for _, m := range msgs { + if m["role"] == "assistant" { + return m + } + } + t.Fatalf("no assistant message in request: %s", raw) + return nil +} + +func reasoningHistory() []chat.Message { + return []chat.Message{ + {Role: chat.MessageRoleUser, Content: "weather?"}, + { + Role: chat.MessageRoleAssistant, + Content: "checking", + ReasoningContent: "I should call the weather tool", + ToolCalls: []tools.ToolCall{{ID: "c1", Function: tools.FunctionCall{Name: "weather", Arguments: `{}`}}}, + }, + } +} + +// Reasoning replay is a Chat Completions capability, not an endpoint +// classification. Every destination defaults on and may opt out explicitly. +func TestConvertMessages_ReasoningReplayDefaultsOn(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + provider string + baseURL string + opts map[string]any + }{ + {name: "custom provider", provider: "local_llm", baseURL: "http://127.0.0.1:8080/v1", opts: map[string]any{"api_type": "openai_chatcompletions"}}, + {name: "first-party OpenAI", provider: "openai"}, + {name: "Azure", provider: "azure", baseURL: "https://team.openai.azure.com/openai/deployments/gpt-5.6"}, + {name: "hosted alias", provider: "xai"}, + {name: "DeepSeek", provider: "deepseek"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + c := &Client{Config: base.Config{ModelConfig: latest.ModelConfig{ + Provider: tt.provider, Model: "qwen3", BaseURL: tt.baseURL, ProviderOpts: tt.opts, + }}} + msg := assistantWire(t, c, reasoningHistory()) + assert.Equal(t, "I should call the weather tool", msg["reasoning_content"]) + assert.Len(t, msg["tool_calls"], 1) + }) + } +} + +func TestConvertMessages_ReasoningReplayOverride(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + provider string + baseURL string + opts map[string]any + wantSent bool + }{ + { + name: "explicit true keeps replay enabled on OpenAI", + provider: "openai", + opts: map[string]any{"replay_reasoning": true}, + wantSent: true, + }, + { + name: "explicit true keeps replay enabled on a hosted alias", + provider: "xai", + opts: map[string]any{"replay_reasoning": true}, + wantSent: true, + }, + { + name: "custom provider opts out", + provider: "openai", + baseURL: "http://127.0.0.1:8080/v1", + opts: map[string]any{"api_type": "openai_chatcompletions", "replay_reasoning": false}, + wantSent: false, + }, + { + name: "explicit true keeps replay enabled on ChatGPT", + provider: "chatgpt", + opts: map[string]any{"replay_reasoning": true}, + wantSent: true, + }, + { + // A value that is present but not a boolean must not inherit the + // on default: the option is how an operator keeps reasoning away + // from an endpoint, so a quoted "false" has to mean off. + name: "non-boolean value fails closed on a custom provider", + provider: "local_llm", + baseURL: "http://127.0.0.1:8080/v1", + opts: map[string]any{"api_type": "openai_chatcompletions", "replay_reasoning": "false"}, + }, + { + name: "non-boolean value fails closed on a hosted alias", + provider: "xai", + opts: map[string]any{"replay_reasoning": "true"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + c := &Client{Config: base.Config{ModelConfig: latest.ModelConfig{ + Provider: tt.provider, + Model: "qwen3", + BaseURL: tt.baseURL, + ProviderOpts: tt.opts, + }}} + + msg := assistantWire(t, c, reasoningHistory()) + if tt.wantSent { + assert.Equal(t, "I should call the weather tool", msg["reasoning_content"]) + } else { + assert.NotContains(t, msg, "reasoning_content") + } + }) + } +} + +// Replay also applies before the prompt-cache breakpoint path is chosen, so the +// body the cache key is computed from is the body that gets sent. +func TestConvertMessages_ReasoningReplayAppliesToBreakpointPath(t *testing.T) { + t.Parallel() + + c := &Client{Config: base.Config{ModelConfig: latest.ModelConfig{ + Provider: "openai", + Model: "gpt-5.6", + }}} + require.True(t, sendsExplicitCacheBreakpoints(&c.ModelConfig, false), + "test is only meaningful on the explicit-breakpoint path") + + msg := assistantWire(t, c, reasoningHistory()) + assert.Equal(t, "I should call the weather tool", msg["reasoning_content"]) +} diff --git a/pkg/session/history_reasoning_replay_test.go b/pkg/session/history_reasoning_replay_test.go new file mode 100644 index 0000000000..bc3ecc50c3 --- /dev/null +++ b/pkg/session/history_reasoning_replay_test.go @@ -0,0 +1,100 @@ +package session + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/agent" + "github.com/docker/docker-agent/pkg/chat" + "github.com/docker/docker-agent/pkg/model/provider/oaistream" + "github.com/docker/docker-agent/pkg/modelsdev" + "github.com/docker/docker-agent/pkg/tools" +) + +// assistantFromWire marshals a converted request and returns its first +// assistant message as a raw map: reasoning_content is a non-standard +// extension that only exists on the wire. +func assistantFromWire(t *testing.T, msgs any) map[string]any { + t.Helper() + + raw, err := json.Marshal(msgs) + require.NoError(t, err) + + var out []map[string]any + require.NoError(t, json.Unmarshal(raw, &out)) + for _, m := range out { + if m["role"] == "assistant" { + return m + } + } + t.Fatalf("no assistant message in request: %s", raw) + return nil +} + +// TestGetMessages_ReasoningReplayedToRequest is the end-to-end regression for +// issue #4363: reasoning captured on an assistant turn that also carries a tool +// call has to survive session history and then reach the next Chat Completions +// request, so the model does not re-derive reasoning it already produced. +// +// Both halves matter. The history filters (isUsageOnlyMessage, +// normalizeMessageContent) key on content and tool calls, so this turn is +// retained; the converter is what used to drop the reasoning on the floor. +func TestGetMessages_ReasoningReplayedToRequest(t *testing.T) { + t.Parallel() + + s := New() + s.AddMessage(UserMessage("what is the weather?")) + s.AddMessage(NewAgentMessage("test-agent", &chat.Message{ + Role: chat.MessageRoleAssistant, + ReasoningContent: "I should call the weather tool", + ToolCalls: []tools.ToolCall{{ID: "call_1", Function: tools.FunctionCall{Name: "weather", Arguments: `{"city":"Paris"}`}}}, + Usage: &chat.Usage{InputTokens: 10, OutputTokens: 5}, + })) + s.AddMessage(&Message{Message: chat.Message{Role: chat.MessageRoleTool, ToolCallID: "call_1", Content: "sunny, 24C"}}) + + history := s.GetMessages(agent.New("test-agent", "test instruction")) + + var stored *chat.Message + for i := range history { + if history[i].Role == chat.MessageRoleAssistant { + stored = &history[i] + } + } + require.NotNil(t, stored, "a reasoning turn that carries a tool call must stay in history") + assert.Equal(t, "I should call the weather tool", stored.ReasoningContent) + + converted := oaistream.ConvertMessages(t.Context(), history, modelsdev.NewID("custom", "qwen3"), nil, nil) + assistant := assistantFromWire(t, converted) + + assert.Equal(t, "I should call the weather tool", assistant["reasoning_content"]) + require.Len(t, assistant["tool_calls"], 1) + calls, ok := assistant["tool_calls"].([]any) + require.True(t, ok) + assert.Equal(t, "call_1", calls[0].(map[string]any)["id"]) +} + +// TestGetMessages_ReasoningOnlyTurnNotReplayed pins the boundary of what issue +// #4363 asks for: a turn whose only payload is reasoning is dropped by the +// history filters before any converter runs, so replaying it is out of scope +// here. Retaining such turns is a separate change with context-size +// consequences. +func TestGetMessages_ReasoningOnlyTurnNotReplayed(t *testing.T) { + t.Parallel() + + s := New() + s.AddMessage(UserMessage("think about it")) + s.AddMessage(NewAgentMessage("test-agent", &chat.Message{ + Role: chat.MessageRoleAssistant, + ReasoningContent: "pondering", + Usage: &chat.Usage{InputTokens: 10, OutputTokens: 5}, + })) + + history := s.GetMessages(agent.New("test-agent", "test instruction")) + for _, m := range history { + assert.NotEqual(t, "pondering", m.ReasoningContent, + "reasoning-only turns are not retained by session history") + } +}