Skip to content

fix(tui): mirror MiniMax reasoning_details only on gateways - #145

Open
asto18089 wants to merge 1 commit into
pinvou3-cleanfrom
fix/minimax-reasoning-details-gateway-only
Open

asto18089 wants to merge 1 commit into
pinvou3-cleanfrom
fix/minimax-reasoning-details-gateway-only

Conversation

@asto18089

Copy link
Copy Markdown
Collaborator

Problem

The MiniMax mirror (introduced in e9f0c1c13 with the first-party MiniMax route) rewrites replayed assistant reasoning_content into a synthesized reasoning_details field on every MiniMax request. MiniMax's own API reference no longer documents that field anywhere in the Chat Completions request schema, so first-party endpoints receive an undocumented request field.

Document evidence

MiniMax API reference — https://platform.minimax.io/docs/api-reference/text-openai-api and https://platform.minimax.io/docs/api-reference/text-chat-openai, checked 2026-11-04 and re-verified 2026-11-05: no reasoning_details request field exists in the documented Chat Completions schema. The documented multi-turn contract is to append the full assistant message and preserve reasoning_content (M3.1-Flash) or the <think> content (M2.x) alongside role:tool pairing. The field now survives only in gateway/third-party dialects (APISIX, LiteLLM, OpenRouter's MiniMax M2 page).

Changes (head ba6f0d5b9)

  • The mirror narrows to gateways: minimax_route_mirrors_reasoning_details now fires only when the route is provider=minimax on a non-first-party base_url, judged with the existing exact-route predicate (is_exact_minimax_chat_route: https://api.minimax.io/v1, https://api.minimaxi.com/v1).
  • Wholesale deletion was weighed and rejected as not provably safe: MINIMAX_BASE_URL and [providers.minimax] base_url stay user-configurable, so gateway dialects remain reachable through the minimax provider slot (PROVIDERS.md already classifies custom/gateway endpoints on this slot), and OpenRouter's MiniMax M2 page still documents the reasoning_details round-trip the mirror feeds.
  • OpenRouter's own traffic keeps its independent capture/replay path on the openrouter provider branch; it never needed this mirror.
  • reasoning_split and max_completion_tokens are documented request parameters and are untouched, as is the OpenRouter-owned reasoning_details capture.

Test evidence (red-green)

  • minimax_official_route_replays_thinking_as_reasoning_content_only and minimax_wire_body_on_official_route_carries_no_reasoning_details fail against the old implementation (synthesized reasoning_details present on the official endpoint) and pass after the narrowing.
  • minimax_gateway_route_still_mirrors_thinking_as_reasoning_details pins the kept gateway behavior.
  • All 35 minimax-matching tui lib tests green.

Behavior changes

  • Official first-party MiniMax endpoints (api.minimax.io/v1, api.minimaxi.com/v1) no longer receive the synthesized reasoning_details request field; assistant thinking still replays as documented reasoning_content / <think> preservation.
  • Custom/gateway base_url values on the minimax provider keep the mirror unchanged.

Merge-conflict surface with #137

This branch edits crates/tui/src/client/chat.rs in the MiniMax reasoning region and a test-module use super::{...} line; it does not edit the top use crate::config::{...} block that #137 (fix/kimi-k31-replay-spellings) edits. Hunks are disjoint, but both PRs grow the same file, so a context-only rebase may be needed depending on merge order.

Probe declaration

No live probes were used in this change; the narrowing is grounded solely in the published MiniMax API reference cited above.

No-Issue: provider compatibility batch grounded in the published provider docs (fetched 2026-11-05); the repository has issues disabled.

The mirror (introduced in e9f0c1c with the first-party MiniMax route)
rewrites replayed assistant reasoning_content into reasoning_details on
every MiniMax request. MiniMax's own API reference, re-checked on
2026-11-04 (https://platform.minimax.io/docs/api-reference/text-openai-api
and https://platform.minimax.io/docs/api-reference/text-chat-openai), no
longer lists reasoning_details anywhere in the Chat Completions request
schema: their documented multi-turn contract is to append the full
assistant message and preserve reasoning_content (M3.1-Flash) or the
<think> content (M2.x) alongside role:tool pairing. The field now only
survives in gateway/third-party dialects (APISIX, LiteLLM, OpenRouter's
MiniMax M2 page).

Two options were weighed. Wholesale deletion is not provably safe:
MINIMAX_BASE_URL and [providers.minimax] base_url stay user-configurable,
so gateway dialects remain reachable through the minimax provider slot
(PROVIDERS.md already classifies custom/gateway endpoints on this slot),
and OpenRouter's MiniMax M2 page still documents the reasoning_details
round-trip the mirror feeds. So the mirror is narrowed instead: it now
fires only when the route is provider=minimax on a non-first-party
base_url, judged with the existing exact-route predicate
(is_exact_minimax_chat_route: https://api.minimax.io/v1 and
https://api.minimaxi.com/v1). OpenRouter's own traffic keeps its
independent capture/replay path on the openrouter provider branch and
never needed this mirror. reasoning_split and max_completion_tokens are
documented request parameters and are untouched, as is the OpenRouter-
owned reasoning_details capture.

Red-green evidence: minimax_official_route_replays_thinking_as_
reasoning_content_only and minimax_wire_body_on_official_route_carries_
no_reasoning_details fail against the old implementation (synthesized
reasoning_details present on the official endpoint) and pass after the
narrowing; minimax_gateway_route_still_mirrors_thinking_as_reasoning_
details pins the kept gateway behavior. All 35 minimax-matching tui lib
tests green.

Signed-off-by: asto <asto18089@126.com>

@asto18089 asto18089 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fresh 9-track review @ ba6f0d5b9 — recommend merge

Rebase check: branch parent == origin/pinvou3-clean head 11cba403d (re-fetched during review, unmoved) — the branch is already exactly on the latest base; no rebase needed or performed. Review was done from scratch on this head: call-site completeness, base_url plumbing, mirror-semantics/ordering audit, live provider-doc fact-check, mutation-tested red-green, local gate runs, byte-level diff purity, merge-tree against every open sibling PR, and an adversarial pass.

Key questions

1. Useful, complete, root-caused — yes. Root cause is exactly as described: e9f0c1c13 added the mirror gated on provider == Minimax alone. Post-PR, the full inventory of reasoning_details writers in the chat path is closed for first-party routes: the mirror's only two production call sites (PromptBuilder::build_for_provider_and_route, mirror_..._for_body at the end of the single production funnel build_chat_wire_body ← client.rs:2192) are both behind the predicate; OpenRouter-captured details are unconditionally stripped for non-OpenRouter providers before the gate, so even a restored OpenRouter-originated session cannot put the field on an official-route wire; MinimaxAnthropic/wire="anthropic" traffic structurally bypasses build_chat_wire_body. The documented reasoning_content replay contract stays intact (replay gate untouched; verified for MiniMax). Gateway behavior is unchanged and pinned by the new gateway test.

2. Elegant — yes. The predicate delegates exactness to the existing config-crate is_exact_minimax_chat_route (single source of truth) instead of hand-rolling host parsing like the Mistral/Google siblings, and its placement/naming/doc-comment match that family. One commit, no drive-by refactors.

3. No smuggled changes. git diff 11cba403d..ba6f0d5b9 = one file, +91/−11; every hunk classified as either the predicate/gating change or its minimax tests. No lockfile/doc/i18n/fingerprint edits.

4. Bug-free and mutation-verified. Red-green claims reproduced: reverting the narrowing (predicate → provider == Minimax) fails both official-route tests and passes the gateway test; un-gating only the body pass fails exactly the wire-body test (proving both tests are needed); flipping the predicate fails the gateway test. cargo test -p codewhale-tui --lib minimax_ → 35 passed / 0 failed (count claim exact). Full workspace nextest 15009/15010 (sole failure forced_idle_timeout_releases_the_worker_for_later_tasks passes in isolation — load flake, file untouched by PR). fmt, clippy on the PR's file, hermetic safety gate (361/361), 8 script gates, 6 npm suites, cargo check --workspace, eval harness: green. (Two clippy collapsible_if errors and one runtime_web_client vocabulary failure reproduce identically at base 11cba403d — pre-existing toolchain/env drift, not this PR.) Perf: predicate short-circuits on the enum and runs twice per request with no allocation. CI 13/13 relevant checks green, MERGEABLE, merge-tree clean against all open siblings (#137/#143/#144/#146/#148/#132/#130/#126/#127) in both orders — the body's "#137 disjoint hunks" claim is accurate, and merge-tree shows even the anticipated context-only rebase is unnecessary.

Doc claims — verified live (2026-11-05)

First-party absence is rock-solid: zero occurrences of reasoning_details in the raw markdown source of both cited pages, and the Chinese-hosted reference (platform.minimaxi.com → platform.minimax.cn) agrees. The multi-turn contract (append full assistant message; preserve reasoning_content for M3.1-Flash / <think> for M2.x; role:tool pairing) is documented verbatim as cited. OpenRouter's MiniMax M2 page still documents the reasoning_details round-trip today, so the gateway-keep decision is grounded. reasoning_split / max_completion_tokens are documented request parameters, as claimed.

Findings (none blocking)

  1. minor (wording) — MINIMAX_BASE_URL is cited in the new doc comment (chat.rs:1045) and commit message as what keeps gateway dialects reachable, but that env var never reaches the TUI chat route: its only reader is the config-crate CLI surface (crates/config/src/lib.rs:7305), and the TUI provider env-override list maps Minimax | MinimaxAnthropic => &[] (crates/tui/src/config.rs:8216). The operative knobs are [providers.minimax] base_url (which the commit message also names), CODEWHALE_BASE_URL/--base-url. Reachability itself holds — the gateway test pins it — but the comment justifies it via a variable this code path ignores. A one-line reword (and matching body tweak) in a follow-up would do.
  2. minor (body citations) — "the field survives in APISIX, LiteLLM, OpenRouter dialects": LiteLLM documents reasoning_details for MiniMax response-side only (no request round-trip anywhere in its MiniMax or general reasoning pages), and the round-trip claim actually lives in the API7 AI Gateway docs (docs.api7.ai/ai-gateway/providers/minimax), not Apache APISIX's own docs (zero mentions). Suggest correcting the body enumeration; the decision it supports is unaffected.
  3. minor (pre-existing, disclose or follow-up) — a bare official host (https://api.minimax.io without /v1) is dialed as the official endpoint (versioned_base_url re-appends /v1, client.rs:903) while the predicate classifies it as a gateway → mirror fires there. Same mismatch class exists for every is_exact_* predicate; requires an off-default config shape. Likewise the legacy api.minimax.chat host (grouped as MiniMax in pricing.rs:488) is outside the predicate. Both predate this PR; worst case is one ignored extra field, unchanged from base.
  4. nit — PR title ("only on gateways") vs commit subject ("only on gateway routes") differ slightly.

Verdict: recommend merge. The two wording minors can be fixed via a body edit / follow-up; finding 3 is a pre-existing pattern corner worth a follow-up note at most.

@JensenChen28 JensenChen28 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The route-scoped change and tests otherwise match the stated goal. I found one valid first-party URL form that bypasses the new guard; details are attached inline. I did not run the Rust test suite in this environment.

/// which still documents the field) keep the mirror.
fn minimax_route_mirrors_reasoning_details(provider: ApiProvider, base_url: &str) -> bool {
provider == ApiProvider::Minimax
&& !codewhale_config::provider::is_exact_minimax_chat_route(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard sees the configured base_url before transport normalization. For a valid MiniMax configuration such as https://api.minimax.io (or https://api.minimaxi.com), is_exact_minimax_chat_route returns false because there is no /v1, so this treats the route as a gateway and mirrors reasoning_details. The TUI transport later calls versioned_base_url, which appends /v1, and sends to the first-party /v1/chat/completions endpoint. Thus the documented field still reaches the official endpoint for this base URL form. Please classify against the effective normalized endpoint (or normalize the bare official hosts consistently) and add a regression case.

@zhuowp zhuowp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved for ba6f0d5b against pinvou3-clean at 11cba403. I reviewed all eight diff hunks and traced the configured provider and base URL through both prompt construction and final Chat Completions serialization. The standard MiniMax global and China /v1 routes retain reasoning text and omit synthesized reasoning_details. Custom MiniMax gateways keep the existing mirror, and OpenRouter's structured replay remains separate.

I reproduced the inline finding by tracing the source. An explicitly configured bare official host still gets the old mirrored field because URL construction adds /v1 after request shaping. I consider this a nonblocking compatibility gap for this patch. The documented /v1 forms receive the fix, the bare form behaves as it did on the base branch, and the available evidence does not establish a vendor rejection. The TUI source comment should also name the provider table override, which is the gateway entry point verified here.

The exact CI merge tree passed all three named MiniMax regressions. The workspace run reported 15,004 passes and 15 skips. No live MiniMax or gateway request was made as part of this review. This approval is for the reviewed source. The existing change request and repository merge requirements remain separate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants