fix(oaistream): replay reasoning content with opt-out

[MysticalMount]

Fixes #4363

Supersedes #4365 (rebased onto current main with signed commits).

Root cause

The shared OpenAI-compatible message converter never copied chat.Message.ReasoningContent into replayed assistant messages. After a reasoning model emitted a tool call, the next Chat Completions request therefore omitted reasoning the model had already produced and forced it to derive that reasoning again.

Fix

• Serialize stored assistant reasoning as the non-standard Chat Completions reasoning_content field using SetExtraFields.
• Preserve valid wire shape by emitting content: "" when a surviving assistant turn has reasoning but neither content nor tool calls.
• Keep replay copy-on-write so session-owned history is never mutated.
• Default Chat Completions reasoning replay ON.
• Add provider_opts.replay_reasoning: false as the per-model/provider compatibility opt-out requested during review. Invalid non-boolean values fail closed and also disable replay.
• Apply the option before every conversion path, including prompt-cache breakpoint conversion.

There is deliberately no endpoint classifier, provider allowlist, or provenance state in the generic provider factory. DeepSeek, DMR, llama.cpp/Qwen, first-party endpoints, Azure, and other OpenAI-compatible Chat Completions destinations all use the same default and the same explicit escape hatch.

Scope

This fixes replay for reasoning attached to a history turn the session retains, including the reasoning-plus-tool-call scenario from #4363.

Reasoning-only assistant turns are still removed by pre-existing session-history filters before conversion. Changing retention, compaction, and context accounting for those turns remains out of scope; the converter nevertheless produces a valid message if such a turn reaches it.

The option is specific to plain Chat Completions reasoning_content. It is separate from Responses API preserve_reasoning, which replays encrypted first-party reasoning state.

A model switch or fallback can place reasoning captured from one vendor in front of another Chat Completions destination. Configure replay_reasoning: false on a destination where that is incompatible or undesirable; automatic cross-vendor provenance is intentionally outside this focused fix.

Tests

• End-to-end history-to-wire regression for a reasoning plus tool-call turn.
• Exact serialized message-shape coverage for reasoning with text, tools, filtered multi-content, whitespace-only input, and reasoning-only turns.
• Default-on coverage across custom, first-party OpenAI, Azure, hosted alias, DeepSeek, and DMR configurations.
• Explicit opt-out and opt-in coverage, including invalid values failing closed.
• Copy-on-write and prompt-cache-path coverage.

Validation on the latest branch:

• task build
• focused OpenAI, DMR, shared converter, and session tests
• schema tests
• golangci-lint: 0 issues
• repository custom lint: no offenses
• git diff --check

The full suite remains covered by CI.

llama.cpp validation

Using ghcr.io/ggml-org/llama.cpp:server build b11561 with the converter's serialized output:

Request shape	Result
reasoning plus content: ""	200
reasoning without content or tool_calls (old shape)	400
reasoning plus text	200
reasoning plus tool calls and tool result	200
content-only control	200

This validates request acceptance and the required assistant-message shape. It does not claim that the SmolLM fixture consumes DeepSeek/Qwen reasoning; that behavior is the subject of the originating Qwen report.

Documentation

• Documents the default-on behavior and per-model/provider opt-out.
• Separates Chat Completions replay_reasoning from Responses API preserve_reasoning.
• Documents that reasoning-only history retention and automatic cross-vendor provenance are out of scope.
• Updates agent-schema.json and the replay example.

[aheritier]

Moving forward with this PR rather than #4365 to consolidate the implementation and discussion. Before merge, please fix the reasoning-only assistant wire format and add serialized-JSON regression coverage, as detailed inline. Please also resolve the compatibility policy for replaying the nonstandard reasoning_content field through the shared converter.

The correctness finding is based on SDK and llama.cpp source inspection, not a live backend reproduction. Please validate against the target backend and rerun required checks after the fixes. Blocking CI was green during the earlier review, but the latest check-runs/status lookups returned HTTP 500, so current CI was not reverified; that is not evidence of failing tests.

Optional cleanup: fold the repro tests into messages_test.go and shorten the explanatory comment to the non-obvious reason for using SDK extra fields. Compaction accounting and overflow recovery remain outside this focused change.

[aheritier]

[blocking] Preserve reasoning-only turns with a valid wire-format assistant message.

A chat.Message with assistant role and ReasoningContent: "thinking", but no content or tool calls, now survives this guard. The conversion leaves Content unset, and the pinned openai-go SDK omits both content and tool_calls, yielding a message with only role and reasoning_content. llama.cpp rejects that shape: https://github.com/ggml-org/llama.cpp/blob/448147d42a71d0147c6dc844f99f9ffe7419183a/tools/server/server-common.cpp#L1334-L1337. This is based on source inspection, not a live reproduction.

For a retained reasoning-only turn, explicitly set empty-string content, for example assistantParam.Content.OfString = param.NewOpt(""). Add marshaled-JSON tests asserting content: "" is present. Also cover reasoning alongside text/tool calls, absence of the extension when no reasoning is stored, and empty/whitespace edge cases. The current ExtraFields/message-count assertions do not catch this invalid wire shape. Validate subsequent requests using the retained session history against the target backend.

[MysticalMount]

Fixed in 3c0843d — you were right, and the shape was invalid on the wire.

convertMessagesWithCaps now tracks whether any content was set and, when neither content nor tool calls are present, sets assistantParam.Content.OfString = param.NewOpt(""). param.NewOpt("") has status: included, so the SDK's omitzero keeps the key and the request carries "content": "".

The guard is !hasContent && len(assistantParam.ToolCalls) == 0, which also covers a pre-existing case unrelated to reasoning: an assistant turn whose only MultiContent part was filtered out used to serialize with neither key too.

Tests, added to messages_test.go and asserting on the serialized body rather than the SDK struct (a field the SDK omits is invisible to struct-level assertions):

• TestConvertMessages_ReasoningContentWireFormat — table-driven; marshals each converted message and compares the whole map[string]any, pinning the exact key set. Covers reasoning-only → content: "", reasoning + text, reasoning + tool calls (content absent, tool_calls present), no reasoning → no extension, whitespace-only reasoning → no extension, reasoning + MultiContent.
• TestConvertMessages_AssistantAlwaysMarshalable — asserts every surviving assistant turn serializes with content or tool_calls, mirroring the rule llama.cpp enforces on assistant turns.
• Two rows added to the TestConvertMessages table for the skip-guard side (reasoning-only kept, whitespace-only reasoning dropped).

Both new tests fail without the fix, and the failure message carries the exact invalid JSON.

Validated against the backend. llama.cpp enforces the requirement in two places: oaicompat_chat_params_parse (tools/server/server-common.cpp:1336, commit 448147d), which produces the message quoted below, and common_chat_msgs_parse_oaicompat (common/chat.cpp:449). Against a live ghcr.io/ggml-org/llama.cpp:server (build b11561), posting the converter's own output as the request body:

request	result
{"content":"","role":"assistant","reasoning_content":"…"} (fixed)	200
{"role":"assistant","reasoning_content":"…"} (pre-fix)	400 Assistant message must contain either 'content' or 'tool_calls'!
reasoning + text	200
reasoning + tool calls, followed by the tool result	200
control: content only	200

One caveat on that evidence: I used SmolLM2-135M-Instruct-Q2_K, so it validates the server's request handling, not a specific reasoning template. Both throws happen before any template is consulted, so the validation is model-independent — but I have not exercised a DeepSeek/Qwen-style reasoning template end to end.

[aheritier]

[should-fix] Resolve the destination-specific replay policy explicitly.

This condition checks stored reasoning, not the destination provider/model or a replay setting. The shared converter feeds OpenAI-compatible Chat Completions and DMR, so the nonstandard reasoning_content extension is sent across those paths. Issue #4363 raises configurability; the existing attachment capability overrides do not provide a reasoning-replay escape hatch.

Please either provide a dedicated per-model opt-out/capability with tests, or record the maintainer decision to accept universal default-on replay and the compatibility validation supporting it. This is a compatibility-policy risk, not a demonstrated rejection by another provider.

[MysticalMount]

No code change for this one yet — it needs a maintainer call, so I'd rather flag it than pick unilaterally.

The argument for keeping it unconditional: the gate is not really destination-blind in practice. ReasoningContent is only ever populated from a response that carried the extension in the first place — oaistream/adapter.go reads reasoning_content / reasoning off the streamed chunk — so a request only ever carries the field back to a server that already spoke it to us. A model that never emits reasoning produces no field at all, which TestConvertMessages_ReasoningContentWireFormat pins.

The honest gap: model switching is a supported runtime operation (App.SetCurrentAgentModel, and pkg/acp/configuration.go offers it in-session), so reasoning captured from model A can be replayed to model B. What a foreign backend does with the key is untested here; llama.cpp reads it into its own message struct (common/chat.cpp:453) and simply has no use for it if the template doesn't reference it, which suggests a small blast radius but doesn't prove it for every server.

So the question back to you: record default-on as the policy, or add an opt-out? If opt-out, the minimal shape looks like a ModelCapabilities flag (default true) plus a matching CapsOverride, which touches pkg/modelinfo, pkg/config/latest, agent-schema.json, the DMR and openai providers, and the docs — noticeably larger than this PR, so I'd want it as a follow-up rather than folded in here.

[aheritier]

🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Re-reviewed 3c0843dc02d6cd673e286364f1b25b78eb3764cb. The original SDK wire-format issue is fixed: serialized reasoning-only assistant messages now include content: "", and the new exact-key JSON tests pass. A full pinned-SDK loopback request also preserves that field.

One remaining acceptance gap: normal retained session history still drops reasoning-only turns before this converter. isUsageOnlyMessage ignores ReasoningContent, normalizeMessageContent independently drops content-empty turns, and runtime recording also skips a no-Usage reasoning-only response. A public persisted-session/history-to-converter probe yields zero replayed assistants for reasoning-only cases, while text/tool controls replay correctly. These filters predate this PR (not an introduced regression), but conflict with its reasoning-only preservation promise. Please reconcile that path and add a history-to-request regression, or explicitly narrow the PR's promise and defer this support.

The replay policy is also still open. ReasoningContent can come from Anthropic/Bedrock/Gemini/Responses, not only the compatible stream adapter; model switching and automatic fallback can forward it to another Chat Completions destination. Replay remains unconditional. No foreign-backend rejection has been demonstrated. Please record the maintainer decision to accept universal replay, including those cases, or provide a destination/model opt-out with tests and clarify the Responses-versus-Chat-Completions docs.

In the completed re-review, local converter/surrounding package tests, application build and vet passed. Current CI gate passes at this head; test-race was skipped. Local race did not run because CGO/compiler support is unavailable. The author's SmolLM2 backend handling results are not an independent reasoning-template end-to-end validation. I recommend holding approval until the history acceptance gap and policy choice are resolved.

[MysticalMount]

Both review concerns addressed.

History path: you're right that reasoning-only assistant turns are dropped before they reach oaistream. I narrowed the PR rather than changing those existing session/runtime invariants, since doing so also affects context accounting and compaction behaviour.

The original tool-call case is covered end-to-end: I added a regression test that drives a real session through GetMessages → ConvertMessages and asserts that reasoning_content is present alongside tool_calls in the serialized request. It fails without this converter fix. The PR title, description, and code comments now explicitly state that reasoning-only turns are not retained by session history.

Replay policy: my earlier premise about ReasoningContent provenance was incorrect. The field can be populated by Anthropic, Bedrock, Gemini, OpenAI Responses, and OpenAI-compatible adapters, so unconditional replay can cross provider boundaries.

I've therefore added provider_opts.replay_reasoning, with destination-based defaults and an explicit override in either direction:

Destination	Default
custom OpenAI-compatible provider, open-model host, openai with custom base_url, DMR	on
first-party openai / chatgpt	off

The gate is applied in the provider clients before conversion, including the cache-breakpoint path, and tests cover destination defaults, explicit overrides, and non-mutation of retained messages.

The docs now also distinguish the two mechanisms explicitly: replay_reasoning controls reasoning_content replay on Chat Completions requests, while the existing preserve_reasoning controls Responses API reasoning state.

Agreed on the SmolLM2 point as well: it demonstrates request acceptance and the content/tool_calls wire-format requirement, not that a DeepSeek/Qwen template consumes replayed reasoning. The PR description now says that explicitly.

[MysticalMount]

Small correction to the default-policy table above after tightening the destination classification.

replay_reasoning now defaults on only for destinations Docker Agent already classifies as generic OpenAI-compatible/open-model endpoints (including custom providers and openai with a custom base_url). It defaults off for hosted vendor APIs, not only first-party OpenAI/ChatGPT. Any model can still opt in or out explicitly with provider_opts.replay_reasoning.

This also fixes an issue in my previous implementation where I had incorrectly treated “not first-party OpenAI” as equivalent to “generic OpenAI-compatible”; tests now cover the hosted vendor cases as default-off.

[MysticalMount]

Tightened the default after another pass — it is now more conservative and simpler than what I described earlier, so the earlier comments no longer describe this PR.

Replay defaults on only where the operator configured the server: a provider from the providers: section, the openai provider pointed at another endpoint (base_url or OPENAI_BASE_URL), and dmr. Every built-in provider defaults off, including the open-model hosts. The default now asks "did the user deliberately point us at their own compatible endpoint?" instead of "which vendors probably understand reasoning replay?", which removes any vendor compatibility taxonomy from the change.

Two things made the previous version wrong:

1. It rode on the consecutive-message-merge classification. Those two policies answer different questions — whether a chat template tolerates several system messages, versus whether a server reads a reasoning_content request field back. main adding atlascloud and daoxe to openModelHostProviders during this PR's lifetime silently made them replay-default-on with no reasoning decision behind it. shouldMergeConsecutiveMessages is untouched by this PR now.
2. The classifier read the config field, not the resolved endpoint. NewClient resolves base_url, then OPENAI_BASE_URL, then the official URL, so provider: openai with OPENAI_BASE_URL=http://localhost:8080/v1 was classified as first-party OpenAI and defaulted off. TestNewClient_ReplayReasoningFollowsEffectiveEndpoint covers it through NewClient.

Corrected alongside:

• The custom-provider docs said the destination default stopped an Anthropic → local fallback from forwarding thinking blocks. It does not: a local compatible endpoint is replay-by-default, so that forwarding happens. What the default prevents is Anthropic → hosted vendor API. The docs now say which is which and name replay_reasoning: false as the opt-out.
• chat.Message.ReasoningContent's comment claimed DeepSeek is the only endpoint needing the field and that others ignore unknown fields. Neither is something this change can guarantee, and the linked DeepSeek page documents the field as a response field, so the claim is gone in favour of "compatibility varies, which is what the gate is for".
• A provider_opts.replay_reasoning value that is present but not a boolean now turns replay off instead of inheriting an on default.

While tracing effective base URLs I noticed the existing Responses preserve_reasoning path makes its first-party determination from the configured BaseURL rather than the resolved OPENAI_BASE_URL. That is an unrelated pre-existing path and I have deliberately not changed it here; filed separately as #4594.

[MysticalMount]

Final policy clarification after simplifying the implementation: the destination-classification approach discussed in my previous comments has been removed.

Chat Completions reasoning replay now defaults on, with provider_opts.replay_reasoning: false as the dedicated per-model/provider opt-out requested in the review. There is no vendor allowlist, endpoint classifier, or provenance inference in this PR.

The earlier comments describing destination-gated defaults are therefore superseded by the current PR description and implementation. The history/reasoning-only scope remains unchanged: reasoning-only messages are handled correctly if they reach the converter, but changing session-history retention is deliberately outside this PR.