fix(minimax): preserve M3 adaptive thinking on Anthropic routes - #66694
fix(minimax): preserve M3 adaptive thinking on Anthropic routes#66694bbasketballer75 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Hermes codebase to ensure MiniMax-M3 “thinking” controls are emitted on the Anthropic-compatible MiniMax route (not just the OpenAI-compatible /v1 route), so configured reasoning settings actually take effect for default MiniMax providers. It also includes additional changes to local command execution and file write error reporting.
Changes:
- Extend the MiniMax provider profile to emit
thinkingcontrols forhttps://api.minimax.io/anthropic(and keepreasoning_splitonly for/v1). - Add a direct-exec path for standalone
powershell/pwshinvocations to avoidbash -cmangling. - Improve
write_fileerror messages by detecting MSYS/Git-Bash spawn failures and appending targeted remediation guidance.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
plugins/model-providers/minimax/__init__.py |
Emit MiniMax-M3 thinking controls on both /v1 and /anthropic MiniMax routes, keeping reasoning_split only for /v1. |
tools/environments/local.py |
Add _direct_interpreter_argv() and route standalone PowerShell/pwsh calls around bash -c parsing. |
tools/file_operations.py |
Enhance write_file failure handling to surface MSYS/Git-Bash ASLR remediation when detected. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Interpreters that, when they are the ENTIRE command (not mixed with any | ||
| # other bash construct), get invoked directly instead of through | ||
| # `bash -c`. See _direct_interpreter_argv() for why. | ||
| _DIRECT_INTERPRETER_EXES = { | ||
| "powershell": "powershell.exe", | ||
| "powershell.exe": "powershell.exe", | ||
| "pwsh": "pwsh.exe", | ||
| "pwsh.exe": "pwsh.exe", | ||
| } |
There was a problem hiding this comment.
No longer applies to the current head. The replacement implementation changes only the Anthropic adapter, auxiliary URL propagation, and their focused tests; tools/environments/local.py is not in this PR.
| is_m3 = _is_minimax_m3(model) | ||
| is_oai = _is_minimax_global_openai_base_url(base_url) | ||
| is_ant = _is_minimax_global_anthropic_base_url(base_url) | ||
|
|
||
| if not is_m3 or (not is_oai and not is_ant): | ||
| return {}, {} | ||
|
|
||
| extra_body: dict[str, Any] = {"reasoning_split": True} | ||
| extra_body: dict[str, Any] = {} | ||
|
|
||
| if is_oai: | ||
| extra_body["reasoning_split"] = True | ||
|
|
||
| if isinstance(reasoning_config, dict) and reasoning_config.get("enabled") is False: | ||
| extra_body["thinking"] = {"type": "disabled"} | ||
| return extra_body, {} | ||
|
|
||
| if reasoning_config is not None: | ||
| elif reasoning_config is not None: | ||
| extra_body["thinking"] = {"type": "adaptive"} | ||
|
|
There was a problem hiding this comment.
Superseded by the current head. The standard anthropic_messages transport is handled in build_anthropic_kwargs rather than MiniMaxProfile.extra_body, with explicit adaptive, disabled, and unset coverage in tests/agent/test_minimax_provider.py.
| """Emit M3 reasoning controls for api.minimax.io (both /v1 and /anthropic). | ||
|
|
||
| MiniMax-M3's /v1 endpoint keeps thinking inline unless ``reasoning_split`` | ||
| is sent, so always request the split format on that route. The /anthropic | ||
| endpoint returns thinking as native ``thinking`` content blocks already, so | ||
| no split flag is needed there. ``thinking`` controls the M3 mode; Hermes' | ||
| effort levels are not a MiniMax depth knob here — they only select | ||
| adaptive vs disabled. On /anthropic, omitting ``thinking`` causes M3 to | ||
| default to OFF (per MiniMax docs), which is the bug this branch fixes. | ||
| """ |
There was a problem hiding this comment.
Resolved by replacing the earlier branch contents. The current PR no longer contains the unrelated terminal or file-operation changes; its four files are limited to the Anthropic adapter, auxiliary base-URL propagation, and tests.
450c44c to
5ba1429
Compare
bbasketballer75
changed the title
alt-glitch
added
type/bug
Related to open #42560: this branch includes its default-on agent behavior and adds a separate MiniMaxProfile change for explicitly configured reasoning on |
450c44c to
8267f85
Compare
|
Responding to the triage bot's earlier ask to split this PR: Keeping both commits in one PR is intentional, not accidental. A short rationale: What
|
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the request-shaping paths. The adapter premise is still present on current main: agent/anthropic_adapter.py:2652-2669 routes non-Claude models, including M3, to manual thinking.type=enabled.
Problems
- The added
_supports_adaptive_thinking()M3 branch is model-only. It applies adaptive thinking to an M3-named model on any Anthropic-compatible host, despite the PR’s stated wrong-host pass-through requirement. agent/transports/anthropic.py:63-78directly invokesbuild_anthropic_kwargs; it does not callMiniMaxProfile.build_api_kwargs_extras. The provider/anthropichunk therefore does not affect the standard route.- The diff has no tests, and current
tests/plugins/model_providers/test_minimax_profile.py:190-212asserts the opposite profile behavior for/anthropic.
Suggested changes
- Endpoint-gate the adapter behavior with the existing MiniMax Anthropic endpoint recognition, remove the ineffective profile
/anthropicbranch, and add adapter-level regression coverage for endpoint/model/disable cases.
Automated hermes-sweeper review.
| models (minimax, qwen3, …) return False so they keep the manual path. | ||
| models (qwen3, GLM, …) return False so they keep the manual path; | ||
| MiniMax M3 is the documented exception because the M3 Anthropic endpoint | ||
| accepts ``thinking.type=adaptive`` (and silently ignores the manual |
There was a problem hiding this comment.
_supports_adaptive_thinking() has no base_url, so this makes every Anthropic-compatible endpoint using an M3-named model receive MiniMax-specific adaptive fields. Please gate this behavior at the build_anthropic_kwargs() call site with the recognized MiniMax Anthropic endpoint; the PR body’s wrong-host case otherwise cannot pass.
There was a problem hiding this comment.
Addressed in 0bf25c750. Adaptive M3 controls now require both a canonical M3 slug and a recognized MiniMax global/China Anthropic endpoint. Wrong-host and wrong-model negative cases are included in the focused matrix.
teknium1
added
sweeper:risk-compatibility
|
I have an independently tested implementation that addresses the sweeper feedback above and the stateful replay path that becomes relevant once M3 returns structured thinking blocks. Comparison branch: main...Silronin:fix/minimax-m3-anthropic-thinking Commit: The branch follows the sweeper's requested shape:
Regression coverage includes:
Validation on current main: Live MiniMax CN A live two-turn tool probe also returned Happy for you to cherry-pick/adapt any part of this into #66694, or for a maintainer to indicate that a separate replacement PR would be preferable. I wanted to coordinate first rather than open a competing PR without notice. |
8267f85 to
3c680ea
Compare
alt-glitch
removed
comp/plugins
Correction: the prior overlap concern was corrected on the current head, which now carries the endpoint/model-gated Anthropic contract and replay coverage. CI and maintainer confirmation are still pending. |
3c680ea to
dc63a08
Compare
bbasketballer75
changed the title
dc63a08 to
8dd00ee
Compare
Comment for #66694@teknium1 @alt-glitch — re-review requested. Significant updates since the last look:
PR is now MERGEABLE, +664/-1278 (the deletions are upstream code that this PR replaces for the MiniMax routes specifically). Thanks for the previous round of feedback. |
…3 reasoning controls Extends the `MiniMaxProfile.build_api_kwargs_extras` provider plugin to recognize MiniMax's Anthropic-compatible `/anthropic` endpoint at `api.minimax.io/anthropic`, in addition to the existing `api.minimax.io/v1` OpenAI-compatible route. ## Why this is separate from NousResearch#66694 NousResearch#66694 (`fix(minimax): preserve M3 adaptive thinking on Anthropic routes`) covers the Anthropic adapter layer — the request kwargs that go out as Anthropic `thinking: {"type": "adaptive"}` on the `auxiliary_client` adapter path. That's correct and stays open. This PR covers the provider plugin layer — the `extra_body` that goes out on the OpenAI-compatible request shape that MiniMax's `/anthropic` endpoint also accepts. Different code path, different kwargs structure. ## The bug On the `/anthropic` endpoint, omitting `thinking` causes MiniMax-M3 to default to OFF (per MiniMax docs). Previously, the provider plugin only matched `/v1`, so requests to `/anthropic` got an empty extra_body and M3 returned thinking as disabled. ## The fix Add `_is_minimax_global_anthropic_base_url()` helper that matches `api.minimax.io/anthropic`. Update `build_api_kwargs_extras` to: - Accept both `/v1` and `/anthropic` (the existing `/v1` condition becomes an OR with the new `/anthropic` condition). - Send `reasoning_split=True` only on `/v1` (the `/anthropic` endpoint already returns thinking as native content blocks, no split flag needed). - Send `thinking: {type: adaptive}` (or `disabled` if reasoning is explicitly disabled) on both routes. ## Out of scope The `/anthropic` route also requires stateful replay preservation across multi-turn tool calls, which is the focus of the Anthropic adapter work in NousResearch#66694. The provider plugin here only handles the request-shape kwargs.
Enable MiniMax-M3 on https://api.minimaxi.com/anthropic (and the global MiniMax endpoint) to use the official adaptive thinking contract (thinking={"type":"adaptive"} or {"type":"disabled"}) instead of the Anthropic-style enabled + budget_tokens form. M2.x and any non-canonical M3 slug fall through to the existing manual thinking branch. Fixes the regression where MiniMax-M3 thinking and the model response were indistinguishable on the /anthropic route (the upstream MiniMax profile only adapted the /v1 path). - Endpoint + canonical-slug gate: _is_minimax_anthropic_endpoint() AND _is_minimax_m3() prevent over-matching (vendor/minimax-m3-preview, minimax-m3-128k) and false-positive matches on non-MiniMax Anthropic-compatible endpoints (Azure, Foundry, Palantir). - AnthropicCompletionsAdapter forwards the underlying SDK clients base_url so the auxiliary/MoA call path reaches the same gate. - _manage_thinking_signatures preserves the full thinking/text/tool_use sequence for intact turns and drops the now-invalid thinking blocks on orphan-mutated turns without leaking the internal _thinking_signature_invalidated flag onto the wire. - New regression tests cover: canonical-slug exact match (positive + negative), raw SDK response -> normalize_response -> persist -> round-trip, Codex->MiniMax fallback with reasoning_content history, redacted_thinking preservation, M3 orphan-then-text tool_use fixture, orphan flag cleanup across merged assistant turns, M3 on non-MiniMax endpoints (manual), M2.x on MiniMax endpoints (manual), and the auxiliary adapters _client.base_url=None fallback. Validated on latest origin/main (f4df260) with 604 focused tests passing and a live MiniMax CN /anthropic smoke returning distinct thinking and text blocks under the adaptive contract.
8dd00ee to
7ad7e49
Compare
|
Rebased onto current The prior rebase base in my last comment ( Re-review still welcome. No new behavioral changes in this push. |
alt-glitch
added
needs-decision
…herry-pick The cherry-picked commit's own diff (2bd9721) showed it deleting large unrelated chunks of auxiliary_client.py -- the entire forward-progress-hook mechanism for streamed auxiliary calls, among other things. That's not an intentional part of the MiniMax fix; it means the upstream contributor's branch was based on a much older auxiliary_client.py than current main, and the 3-way cherry-pick merge silently reverted real, unrelated upstream progress instead of just adding the MiniMax-specific base_url threading. Restored auxiliary_client.py and its test file to the pre-cherry-pick (current-main) state. The anthropic_adapter.py half of the fix is untouched -- it's clean, fully tested, and covers the actual bug (M3 thinking not working on the /anthropic route). The auxiliary/MoA call path not sharing the same endpoint gate is a narrower, secondary gap; bringing that in properly needs the PR itself rebased on GitHub against current upstream, not a local hand-reconciliation of a stale branch.
Summary
Fix MiniMax-M3 reasoning on MiniMax's Anthropic-compatible global and China endpoints without changing Claude, Kimi, M2.x, or non-MiniMax behavior.
The adapter now uses MiniMax's documented M3 wire contract only when both the endpoint and canonical model slug match:
lowthroughultra):thinking={"type":"adaptive"}thinking={"type":"disabled"}thinkingIt never sends M3
budget_tokens,output_config.effort,display, or a forced temperature.Contract reference: MiniMax Anthropic SDK documentation documents M3 as default-off,
adaptiveas enabled,disabledas explicitly off, and requires preserving complete thinking/text/tool-use responses across tool turns.Additional fixes
maxandultraas the same provider-level adaptive mode, since MiniMax exposes no depth tiers.Verification
Tested candidate base:
2ebeede00Coverage includes endpoint/model gates, unset/disabled/enabled effort matrices,
max/ultra, response normalization and replay, auxiliary URL propagation, orphan recovery, Kimi, Claude, M2.x, and non-MiniMax endpoints.Scope
This replaces the earlier default-on/model-only proposal. It does not add an ineffective
/anthropicbranch toMiniMaxProfile, and it preserves MiniMax's default-off behavior when Hermes reasoning is unset.