Skip to content

fix(thinking): replay MiniMax reasoning inline as <think> tags in content #264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ltmoerdani merged 1 commit into from
Oct 20, 2026
Merged

fix(thinking): replay MiniMax reasoning inline as <think> tags in content #264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 20 additions & 20 deletions ARCHITECTURE-MAP.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<think>` 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 |

---

Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<think>...</think>` 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 `<think>` prefix on `content` instead. Receive-side `<think>` 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=<unknown>` 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.
Expand Down
57 changes: 57 additions & 0 deletions docs/features/19-20261020-minimax-interleaved-thinking-replay.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 `<think>...</think>` inside `content`, in its original format** — "Do not remove the `<think>...</think>` 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 `"<think>…</think>\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 `<think>...</think>` inside `content`** (this fix) | Interleaved thinking model; upstream drops the field, degrading tool-loop reasoning |

Receive-side note: incoming `<think>` 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 `<think>` 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**.
Loading
Loading