From 0d757065be100da0010e2035b8590218021daaaa Mon Sep 17 00:00:00 2001 From: ltmoerdani Date: Tue, 20 Oct 2026 10:56:26 +0700 Subject: [PATCH] fix(thinking): replay MiniMax reasoning inline as tags in content MiniMax is an interleaved thinking model: its upstream silently drops a separate reasoning_content field on replay, so prior thinking vanished from context and the model re-derived (and visibly repeated) the same reasoning preamble before every tool call in agent loops. shouldEchoThinkingHistory() now returns false for /^minimax-/i and convertMessage() replays thinking (plain + tool-call branches, per-tool reasoning included) as a prefix on content. Receive-side think-tag filtering unchanged. Other families keep their contracts. Docs: docs/features/19-20261020-minimax-interleaved-thinking-replay.md, ARCHITECTURE-MAP.md, CHANGELOG.md (0.7.10), historical note in doc 55. --- ARCHITECTURE-MAP.md | 40 ++++++------- CHANGELOG.md | 2 + ...020-minimax-interleaved-thinking-replay.md | 57 +++++++++++++++++++ ...1-pr123-deepseek-reasoning-content-echo.md | 2 + src/provider/messages.ts | 32 ++++++++--- src/reasoningHistory.ts | 26 +++++++++ src/test/reasoningHistory.test.ts | 20 ++++++- 7 files changed, 149 insertions(+), 30 deletions(-) create mode 100644 docs/features/19-20261020-minimax-interleaved-thinking-replay.md diff --git a/ARCHITECTURE-MAP.md b/ARCHITECTURE-MAP.md index bf766f3e4..5100911d1 100644 --- a/ARCHITECTURE-MAP.md +++ b/ARCHITECTURE-MAP.md @@ -347,26 +347,26 @@ Not a plain API hook — it **monkey-patches Copilot Chat internals** to inject Reuse these before writing new logic (all under `src/` root unless noted): -| Lib | Purpose | Import from | -| ------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------- | -| `config.ts` | **All** tunables: URLs, timeouts, limits, storage keys, setting keys, defaults. Dependency-free. | anywhere | -| `utils.ts` | `isRecord`, `firstString`, `getErrorMessage`, `formatUsd`, `formatTokenCount`, `formatCount`, `formatRelativeTime`, `escapeHtml`, `sleep*` | anywhere | -| `errors.ts` | `OpenCodeRequestError` taxonomy + `buildOpenCodeRequestError` (rate-limit / Router.Unavailable hints) | all request paths | -| `retry.ts` | `analyzeHttp400ForRetry`, `isTransientServerError`, transient-5xx constants | engine + fetch | -| `chatParts.ts` | `createUsageDataParts`, reasoning marker parts | engine / extractors | -| `tokenEstimate.ts` | `estimateTokenCount`, `estimatePromptTokenCount` | provider | -| `imageNormalizer.ts` | normalize image data-URLs to provider-safe dims/encoding | messages + vision | -| `openCodeAuth.ts` | `buildOpenCodeGatewayAuthHeaders` (Bearer / x-api-key / x-goog-api-key) | provider | -| `apiKeyResolution.ts` | `resolveResponseApiKey` cold-start fallback | provider | -| `providerEnablement.ts` | `providerEnabledSetting()` — reads root config (never section-scoped) | extension + commands | -| `reasoningHistory.ts` | `thinkingTextFromValue`, `shouldEchoThinkingHistory` (family-gated echo) | messages | -| `toolCallAccumulator.ts` | `ToolCallAccumulator` (flush only on `tool_calls` finish reason; `argumentsDone` delta REPLACES pending arguments — done-event repair, #244) | extractors | -| `contextWindowHook*.ts` | context-window usage injection — **monkey-patches** Copilot's internal `$handleProgressChunk` + `Set.prototype`; bridge = lazy-load seam; silent no-op if capture fails | engine/provider | -| `visionProxyCache.ts` | SHA-256 image-hash → vision-proxy description cache (FIFO, cap 200) | provider | -| `runtimeDiagnostics.ts` | `runtimeDiagnosticsLines()` — version/host/platform/integrity lines for the Diagnostics command | provider | -| `request/headers.ts` | `buildOpenCodeRequestHeaders` (sticky session/request id) | provider | -| `request/builders.ts` | public barrel for body builders per endpoint | provider | -| `request/shared.ts` | `messagesHaveImages`, `withDeferredToolImageMessages` (tool-image defer, #233) — pure, no `vscode` import | provider | +| Lib | Purpose | Import from | +| ------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------- | +| `config.ts` | **All** tunables: URLs, timeouts, limits, storage keys, setting keys, defaults. Dependency-free. | anywhere | +| `utils.ts` | `isRecord`, `firstString`, `getErrorMessage`, `formatUsd`, `formatTokenCount`, `formatCount`, `formatRelativeTime`, `escapeHtml`, `sleep*` | anywhere | +| `errors.ts` | `OpenCodeRequestError` taxonomy + `buildOpenCodeRequestError` (rate-limit / Router.Unavailable hints) | all request paths | +| `retry.ts` | `analyzeHttp400ForRetry`, `isTransientServerError`, transient-5xx constants | engine + fetch | +| `chatParts.ts` | `createUsageDataParts`, reasoning marker parts | engine / extractors | +| `tokenEstimate.ts` | `estimateTokenCount`, `estimatePromptTokenCount` | provider | +| `imageNormalizer.ts` | normalize image data-URLs to provider-safe dims/encoding | messages + vision | +| `openCodeAuth.ts` | `buildOpenCodeGatewayAuthHeaders` (Bearer / x-api-key / x-goog-api-key) | provider | +| `apiKeyResolution.ts` | `resolveResponseApiKey` cold-start fallback | provider | +| `providerEnablement.ts` | `providerEnabledSetting()` — reads root config (never section-scoped) | extension + commands | +| `reasoningHistory.ts` | `thinkingTextFromValue`, `shouldEchoThinkingHistory` (family-gated echo), `isMiniMaxModel` + `inlineThinkingForHistory` (MiniMax replays thinking inline as `` in `content` — see `docs/features/19-20261020-minimax-interleaved-thinking-replay.md`) | messages | +| `toolCallAccumulator.ts` | `ToolCallAccumulator` (flush only on `tool_calls` finish reason; `argumentsDone` delta REPLACES pending arguments — done-event repair, #244) | extractors | +| `contextWindowHook*.ts` | context-window usage injection — **monkey-patches** Copilot's internal `$handleProgressChunk` + `Set.prototype`; bridge = lazy-load seam; silent no-op if capture fails | engine/provider | +| `visionProxyCache.ts` | SHA-256 image-hash → vision-proxy description cache (FIFO, cap 200) | provider | +| `runtimeDiagnostics.ts` | `runtimeDiagnosticsLines()` — version/host/platform/integrity lines for the Diagnostics command | provider | +| `request/headers.ts` | `buildOpenCodeRequestHeaders` (sticky session/request id) | provider | +| `request/builders.ts` | public barrel for body builders per endpoint | provider | +| `request/shared.ts` | `messagesHaveImages`, `withDeferredToolImageMessages` (tool-image defer, #233) — pure, no `vscode` import | provider | --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f0876aba..466032ab8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ All notable changes to the **OpenCode Go BYOK Provider** extension are documente ### Fixed +- **`[Thinking]` MiniMax no longer repeats an identical reasoning preamble before every tool call.** MiniMax is an interleaved thinking model: its official contract (MiniMax-AI/MiniMax-M2 README, "Inference Parameters") requires assistant thinking to be replayed **inline as `...` inside `content`** — the upstream silently drops a separate `reasoning_content` field. The extension replayed MiniMax thinking history as `reasoning_content` (family-gated "tolerated" since PR #123), so prior thinking effectively vanished from context on every follow-up request; in long agent loops the model re-derived the same conclusion at each tool call, producing the same reasoning sentence 15+ times in a row. `shouldEchoThinkingHistory()` now returns `false` for `/^minimax-/i` and `convertMessage()` replays the thinking (plain and tool-call branches, per-tool reasoning included) as a `` prefix on `content` instead. Receive-side `` filtering is untouched. All other families (DeepSeek required-echo, Kimi/GLM/Qwen/Gemini, MiMo omitted, GPT/Claude n/a) keep their existing contracts — the new helpers are gated by `/^minimax-/i` alone. Live E2E against the gateway still pending. Documented in `docs/features/19-20261020-minimax-interleaved-thinking-replay.md`. + - **`[Models]` Gemini models served by OpenCode Go now route to the Google API instead of chat-completions.** Every request to `gemini-4-argon-contributor` failed with `400 Model does not support this protocol.` because the `gemini` row in `core/registry.ts` was restricted to `opencodezen`, so on Go the lookup fell through to the catch-all row and the model was sent to `chat/completions` — while the gateway serves it Google-native (models.dev marks it `@ai-sdk/google`, and OpenCode's own Go endpoint table lists it under `@ai-sdk/google` too). The row now applies to every vendor. Fixing that exposed the second half of the bug: the Google route is built as `{base}/{modelId}:streamGenerateContent?alt=sse`, and Go's built-in `/zen/go/v1` base does not serve that path at all (it answers with the opencode.ai website 404 page), so provider routing gained an optional `googleModelsUrl` — Go's built-in default points at `https://opencode.ai/inference/go/v1/models` (the base OpenCode's own endpoint table documents for the model), while Zen and any configured `apiBaseUrl` gateway keep deriving the route from `modelsUrl` exactly as before (issue #247 contract intact). Every other route (chat/completions, messages, responses, models, usage) is untouched. Routing tests now cover Go, Zen, and both agent-host variants. Registry drift found while investigating (Go/Zen `minimax-m3`, `qwen3.8-max`, `qwen3.8-flash`, `qwen3.7-plus` documented as Anthropic `messages`) is deliberately not changed here — each needs live verification first. Documented in `docs/features/17-20260814-data-driven-model-registry.md`. - **`[Usage]` Google-route responses now report usage and finish reasons.** `updateRequestUsageSummary` understood OpenAI, Responses, and Anthropic payload shapes but not Google's `usageMetadata` / `candidates[].finishReason`, so every Gemini request logged `promptTokens=n/a … finishReason=` and the Go usage meter recorded `promptTokens=0` — silently losing cost tracking for the whole family. The Google normalizer's own usage mapping is now reused for the summary. With usage visible again, the empty-response diagnostic (`[diag-empty-response]`, the raw-SSE dump built for exactly the "gateway sent bytes but nothing parsed" case) no longer requires completion tokens to fire — that precondition made it unreachable on Google responses and left an empty `200 OK` stream of 5 unparsed events undiagnosable. diff --git a/docs/features/19-20261020-minimax-interleaved-thinking-replay.md b/docs/features/19-20261020-minimax-interleaved-thinking-replay.md new file mode 100644 index 000000000..978f84405 --- /dev/null +++ b/docs/features/19-20261020-minimax-interleaved-thinking-replay.md @@ -0,0 +1,57 @@ +**Status:** 🟢 Active + +# MiniMax Interleaved Thinking Replay — Reasoning Preamble Repetition in Agent Loops + +**Topic:** provider / thinking / models +**Updated:** 2026-10-20 +**Tags:** #streaming #models #provider #minimax #thinking #tool-calling + +**Related:** [55 — PR123 DeepSeek reasoning_content echo](../issues/55-20260811-pr123-deepseek-reasoning-content-echo.md), [85 — DeepSeek reasoning leak regression analysis](../issues/85-20260826-discussion118-deepseek-reasoning-leak-regression-analysis.md), [21 — MiniMax M3 think-tag leak](../issues/21-20260613-minimax-m3-think-tag-leak-reimplementation.md), [36 — MiMo thinking infinite loop](../issues/36-20260723-mimo-thinking-infinite-loop.md) + +--- + +## Overview + +MiniMax models (`minimax-m2.*`, `minimax-m3.x`) with thinking enabled repeated an identical reasoning preamble before almost every tool call in long agent loops (15+ consecutive terminal commands, each preceded by the same "definitive evidence secured…" sentence). This document records the root cause, the fix, and the per-family reasoning-replay contract. + +## Problem + +MiniMax is an **interleaved thinking model**. Its official contract (MiniMax-AI/MiniMax-M2 README, "Inference Parameters") requires assistant thinking to be replayed **inline as `...` inside `content`, in its original format** — "Do not remove the `...` part, otherwise, the model's performance will be negatively affected." + +The extension previously replayed MiniMax reasoning history as a separate `reasoning_content` field (`shouldEchoThinkingHistory()` returned `true` for the family). The upstream **silently ignores that field** on replay, so prior thinking effectively vanished from context. As a result the model re-derived the same conclusion from scratch at every tool call, producing the visibly identical reasoning preamble each time. + +This is **not** an echo-loop (the class of issues #38 / #244): the model neither rejects the field nor traps in one response — it loses thinking continuity across the tool loop. + +## Solution + +Per-family gates keep the blast radius isolated — no code path changes for DeepSeek / Kimi / GLM / Qwen / Gemini / MiMo / GPT / Claude: + +1. `src/reasoningHistory.ts` + - `shouldEchoThinkingHistory()` now returns `false` for `/^minimax-/i`. + - New helpers `isMiniMaxModel()` and `inlineThinkingForHistory()` — wrap thinking text as a `"…\n"` prefix. +2. `src/provider/messages.ts` (`convertMessage`) + - Plain assistant messages: `content = inlinePrefix + textContent` for MiniMax; `reasoning_content` omitted. + - Assistant messages with `tool_calls`: per-tool reasoning (`reasoningForToolCalls`) is wrapped inline at the front of `content`; `reasoning_content` omitted. +3. `src/test/reasoningHistory.test.ts` — MiniMax moved from the "requires the echo" group to its own; two new tests for `inlineThinkingForHistory`. + +## Contracts (reasoning replay per family) + +| Family | Replay format | Reason | +| ------------- | ----------------------------------------------------------- | ----------------------------------------------------------------------------------- | +| DeepSeek | `reasoning_content` field (required) | Upstream 400s without it (upstream issue #36354, PR #123) | +| Kimi/GLM/Qwen | `reasoning_content` field | Tolerate and benefit from cross-turn continuity | +| Gemini | `reasoning_content` field (maps to `thought: true` parts) | Needs the echo for thought parts | +| MiMo | none (omitted) | Strict validator rejects the field with tool_calls (issue #38) | +| GPT / Claude | n/a | Wire formats carry no `reasoning_content` | +| MiniMax | **inline `...` inside `content`** (this fix) | Interleaved thinking model; upstream drops the field, degrading tool-loop reasoning | + +Receive-side note: incoming `` tags in streams are already filtered by `src/transports/thinkTags.ts` (no UI leak) — that path is unchanged. + +## Open Risk + +Not yet verified empirically whether the OpenCode gateway relays assistant `content` containing `` tags without distortion. Live validation (a multi-tool-call chat with `minimax-m3.1`, thinking on, confirming the preamble no longer repeats) is required before this moves to ✅ Solved. + +## Verification + +- `npm run lint` (editorconfig, eslint, markdown, prettier, shellcheck, typecheck, unit tests): **Passed** on branch `fix/minimax-interleaved-thinking-replay`. +- Live E2E: **pending**. diff --git a/docs/issues/55-20260811-pr123-deepseek-reasoning-content-echo.md b/docs/issues/55-20260811-pr123-deepseek-reasoning-content-echo.md index c0c50a39f..6063c2047 100644 --- a/docs/issues/55-20260811-pr123-deepseek-reasoning-content-echo.md +++ b/docs/issues/55-20260811-pr123-deepseek-reasoning-content-echo.md @@ -15,6 +15,8 @@ Full review cycle for community contributor PR #123 by [@Fahad090NP](https://git This is the mirror image of the MiMo carve-out in issue #38: DeepSeek's upstream validator **requires** the previously emitted `reasoning_content` to be echoed back unchanged, while MiMo's validator **rejects** it. The extension must therefore gate the echo by model family. +> **Historical note (2026-10-20):** The "MiniMax tolerated" classification established here was later found incomplete — MiniMax _tolerates_ the field but its upstream **silently ignores** it on replay, breaking interleaved-thinking continuity in agent loops. MiniMax now replays thinking inline as `` tags inside `content` instead; see [feature doc 19](../features/19-20261020-minimax-interleaved-thinking-replay.md). + --- ## Problem diff --git a/src/provider/messages.ts b/src/provider/messages.ts index 72378dc06..1af7c4946 100644 --- a/src/provider/messages.ts +++ b/src/provider/messages.ts @@ -3,7 +3,7 @@ import { isInternalDataPart, isReasoningMarkerPart, readReasoningMarker } from " import { requiresStringToolContent } from "../models/modelCapabilities"; import { MAX_HISTORY_IMAGES_KEPT, MAX_TOOL_RESULT_IMAGE_BYTES } from "../config"; import { getImageDataUrlBase64Bytes, MAX_IMAGE_BASE64_BYTES, normalizeImageDataUrl } from "../imageNormalizer"; -import { shouldEchoThinkingHistory, thinkingTextFromValue } from "../reasoningHistory"; +import { inlineThinkingForHistory, isMiniMaxModel, shouldEchoThinkingHistory, thinkingTextFromValue } from "../reasoningHistory"; import { withDeferredToolImageMessages } from "../request/shared"; import type { ApiMessage, OpenAiContentPart, OpenAiToolCall } from "../request/types"; import { partToText } from "./tokens"; @@ -239,16 +239,29 @@ export async function convertMessage( // For MiMo we omit reasoning_content in the echoed assistant tool_call // history. The current live response still surfaces reasoning_content to // the user via the thinking panel — only the *history echo* is dropped. - // Other families (DeepSeek, Kimi, GLM, Qwen, MiniMax) tolerate the echo - // and keep it for cross-turn reasoning continuity. + // Other families (DeepSeek, Kimi, GLM, Qwen) tolerate the echo and keep + // it for cross-turn reasoning continuity. + // + // MiniMax is an interleaved thinking model: the upstream IGNORES the + // `reasoning_content` field on replay and requires the thinking to stay + // inline as `...` at the front of assistant `content` + // (MiniMax-AI/MiniMax-M2 README, "Inference Parameters"). Dropping or + // mis-formatting it severs thinking continuity across the tool loop and + // makes the model re-derive — and visibly repeat — the same reasoning + // preamble before every tool call. const shouldOmitReasoningEcho = rawModelId !== undefined && /^mimo-/i.test(rawModelId); + const toolCallReasoning = reasoningForToolCalls(toolCalls, reasoningContentByToolCallId) ?? thinkingText; + const inlinePrefix = isMiniMaxModel(rawModelId) ? inlineThinkingForHistory(toolCallReasoning) : ""; return finish([ { role, - content: typeof content === "string" ? content || null : content, - reasoning_content: shouldOmitReasoningEcho - ? undefined - : (reasoningForToolCalls(toolCalls, reasoningContentByToolCallId) ?? thinkingText), + content: + inlinePrefix && typeof content === "string" + ? inlinePrefix + (content || "") + : typeof content === "string" + ? content || null + : content, + reasoning_content: shouldOmitReasoningEcho || isMiniMaxModel(rawModelId) ? undefined : toolCallReasoning, tool_calls: toolCalls, }, ]); @@ -259,10 +272,13 @@ export async function convertMessage( } if (role === "assistant") { + // MiniMax replays thinking inline as `` tags inside `content` + // (see the tool_call branch above); everyone else uses `reasoning_content`. + const plainPrefix = isMiniMaxModel(rawModelId) ? inlineThinkingForHistory(thinkingText) : ""; return finish([ { role, - content: typeof content === "string" ? content || null : content, + content: plainPrefix ? plainPrefix + (textContent || "") : typeof content === "string" ? content || null : content, reasoning_content: shouldEchoThinkingHistory(rawModelId) ? thinkingText : undefined, }, ]); diff --git a/src/reasoningHistory.ts b/src/reasoningHistory.ts index c8a64d573..6b85b5a15 100644 --- a/src/reasoningHistory.ts +++ b/src/reasoningHistory.ts @@ -32,6 +32,16 @@ export function thinkingTextFromValue(value: string | string[]): string { * - MiMo: strict Pydantic-style validator rejects `reasoning_content` (issue #38). * - GPT: OpenAI Responses API — messages carry no `reasoning_content` field. * - Claude: Anthropic Messages API — no `reasoning_content` field. + * - MiniMax: interleaved thinking model — reasoning must be replayed as + * `...` inline in assistant `content` (see + * {@link inlineThinkingForHistory}), NOT as a separate field. Sending the + * `reasoning_content` field is silently dropped by the upstream, which + * severs thinking continuity across tool loops and makes the model + * re-derive (and repeat) the same reasoning preamble on every tool call. + * Source: MiniMax-AI/MiniMax-M2 README, "Inference Parameters" — "it is + * important to retain the thinking content from the assistant's turns + * within the historical messages... Do not remove the `...` + * part, otherwise, the model's performance will be negatively affected." * - Unknown families are left untouched (no echo). */ export function shouldEchoThinkingHistory(rawModelId: string | undefined): boolean { @@ -39,5 +49,21 @@ export function shouldEchoThinkingHistory(rawModelId: string | undefined): boole if (/^mimo-/i.test(rawModelId)) return false; if (/^gpt-/i.test(rawModelId)) return false; if (/^claude-/i.test(rawModelId)) return false; + if (isMiniMaxModel(rawModelId)) return false; return thinkingFamily(rawModelId) !== null || /^gemini-/i.test(rawModelId); } + +/** Whether the model replays assistant thinking as inline `` tags (MiniMax). */ +export function isMiniMaxModel(rawModelId: string | undefined): boolean { + return rawModelId !== undefined && /^minimax-/i.test(rawModelId); +} + +/** + * Wrap thinking text in `` tags for history replay on interleaved + * thinking models (MiniMax). Returns the `"...\n"` prefix to prepend to + * the assistant `content`, or an empty string when there is nothing to replay. + */ +export function inlineThinkingForHistory(thinkingText: string | undefined): string { + const trimmed = thinkingText?.trim(); + return trimmed ? `${trimmed}\n` : ""; +} diff --git a/src/test/reasoningHistory.test.ts b/src/test/reasoningHistory.test.ts index b39cdd079..8b93a4e43 100644 --- a/src/test/reasoningHistory.test.ts +++ b/src/test/reasoningHistory.test.ts @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { shouldEchoThinkingHistory, thinkingTextFromValue } from "../reasoningHistory"; +import { shouldEchoThinkingHistory, inlineThinkingForHistory, thinkingTextFromValue } from "../reasoningHistory"; test("thinkingTextFromValue — passes plain strings through", () => { assert.equal(thinkingTextFromValue("hello"), "hello"); @@ -30,7 +30,23 @@ test("shouldEchoThinkingHistory — OpenAI-compatible reasoning families require assert.equal(shouldEchoThinkingHistory("kimi-k2.6"), true); assert.equal(shouldEchoThinkingHistory("glm-5.2"), true); assert.equal(shouldEchoThinkingHistory("qwen3.6-plus"), true); - assert.equal(shouldEchoThinkingHistory("minimax-m2.7"), true); +}); + +test("shouldEchoThinkingHistory — MiniMax replays thinking inline, not as a field", () => { + // MiniMax is an interleaved thinking model: reasoning must be replayed as + // tags inside content (see inlineThinkingForHistory), never as + // reasoning_content — the upstream drops the field (MiniMax-M2 README). + assert.equal(shouldEchoThinkingHistory("minimax-m2.7"), false); + assert.equal(shouldEchoThinkingHistory("minimax-m3.1"), false); +}); + +test("inlineThinkingForHistory — wraps trimmed thinking in tags", () => { + assert.equal(inlineThinkingForHistory(" plan "), "plan\n"); +}); + +test("inlineThinkingForHistory — empty or missing thinking produces no prefix", () => { + assert.equal(inlineThinkingForHistory(undefined), ""); + assert.equal(inlineThinkingForHistory(" "), ""); }); test("shouldEchoThinkingHistory — Gemini needs the echo for thought parts", () => {