Repository navigation
Conversation
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
left a comment
There was a problem hiding this comment.
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)
- minor (wording) —
MINIMAX_BASE_URLis 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 mapsMinimax | 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. - minor (body citations) — "the field survives in APISIX, LiteLLM, OpenRouter dialects": LiteLLM documents
reasoning_detailsfor 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. - minor (pre-existing, disclose or follow-up) — a bare official host (
https://api.minimax.iowithout/v1) is dialed as the official endpoint (versioned_base_urlre-appends/v1,client.rs:903) while the predicate classifies it as a gateway → mirror fires there. Same mismatch class exists for everyis_exact_*predicate; requires an off-default config shape. Likewise the legacyapi.minimax.chathost (grouped as MiniMax inpricing.rs:488) is outside the predicate. Both predate this PR; worst case is one ignored extra field, unchanged from base. - 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
left a comment
There was a problem hiding this comment.
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( |
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
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.
Problem
The MiniMax mirror (introduced in
e9f0c1c13with the first-party MiniMax route) rewrites replayed assistantreasoning_contentinto a synthesizedreasoning_detailsfield 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_detailsrequest field exists in the documented Chat Completions schema. The documented multi-turn contract is to append the full assistant message and preservereasoning_content(M3.1-Flash) or the<think>content (M2.x) alongsiderole:toolpairing. The field now survives only in gateway/third-party dialects (APISIX, LiteLLM, OpenRouter's MiniMax M2 page).Changes (head
ba6f0d5b9)minimax_route_mirrors_reasoning_detailsnow fires only when the route isprovider=minimaxon a non-first-partybase_url, judged with the existing exact-route predicate (is_exact_minimax_chat_route:https://api.minimax.io/v1,https://api.minimaxi.com/v1).MINIMAX_BASE_URLand[providers.minimax] base_urlstay 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 thereasoning_detailsround-trip the mirror feeds.reasoning_splitandmax_completion_tokensare documented request parameters and are untouched, as is the OpenRouter-ownedreasoning_detailscapture.Test evidence (red-green)
minimax_official_route_replays_thinking_as_reasoning_content_onlyandminimax_wire_body_on_official_route_carries_no_reasoning_detailsfail against the old implementation (synthesizedreasoning_detailspresent on the official endpoint) and pass after the narrowing.minimax_gateway_route_still_mirrors_thinking_as_reasoning_detailspins the kept gateway behavior.tuilib tests green.Behavior changes
api.minimax.io/v1,api.minimaxi.com/v1) no longer receive the synthesizedreasoning_detailsrequest field; assistant thinking still replays as documentedreasoning_content/<think>preservation.base_urlvalues on the minimax provider keep the mirror unchanged.Merge-conflict surface with #137
This branch edits
crates/tui/src/client/chat.rsin the MiniMax reasoning region and a test-moduleuse super::{...}line; it does not edit the topuse 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.