[FEAT] Add MiniMax LLM adapter#2166
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds MiniMax LLM adapter support with provider-aware model routing, thinking and service-tier validation, adapter metadata, configuration schema, exports, and branded adapter tests. ChangesMiniMax adapter integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AdapterConfiguration
participant MiniMaxLLMAdapter
participant MiniMaxLLMParameters
AdapterConfiguration->>MiniMaxLLMAdapter: provide api_key, model, and options
MiniMaxLLMAdapter->>MiniMaxLLMParameters: validate configuration
MiniMaxLLMParameters-->>MiniMaxLLMAdapter: return routed model and validated parameters
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| unstract/sdk1/src/unstract/sdk1/adapters/base1.py | Adds MiniMax routing, request validation, cost-model selection, and context-window metadata. |
| unstract/sdk1/src/unstract/sdk1/adapters/llm1/init.py | Registers and exports the MiniMax adapter. |
| unstract/sdk1/src/unstract/sdk1/adapters/llm1/minimax.py | Defines the branded MiniMax adapter and its metadata. |
| unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/minimax.json | Defines MiniMax credentials, endpoint, model, thinking, and service-tier settings. |
| unstract/sdk1/src/unstract/sdk1/llm.py | Uses validated adapter metadata for context sizing and removes internal metadata before requests. |
| unstract/sdk1/tests/test_branded_openai_adapters.py | Covers MiniMax registration, routing, validation, pricing, schema, and context windows. |
Reviews (11): Last reviewed commit: "Merge branch 'main' into octo/20260712-p..." | Re-trigger Greptile
for more information, see https://pre-commit.ci
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@unstract/sdk1/src/unstract/sdk1/adapters/base1.py`:
- Line 539: Update the MiniMax subclass’s temperature declaration to retain the
base class’s Pydantic Field validation, including default 0.1 and ge=0/le=2
constraints, rather than redeclaring it as an unconstrained scalar. Preserve the
existing nullable float type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 288ec4d5-fe33-44a6-b15c-b218f17ee909
⛔ Files ignored due to path filters (1)
frontend/public/icons/adapter-icons/MiniMax.pngis excluded by!**/*.png
📒 Files selected for processing (5)
unstract/sdk1/src/unstract/sdk1/adapters/base1.pyunstract/sdk1/src/unstract/sdk1/adapters/llm1/__init__.pyunstract/sdk1/src/unstract/sdk1/adapters/llm1/minimax.pyunstract/sdk1/src/unstract/sdk1/adapters/llm1/static/minimax.jsonunstract/sdk1/tests/test_branded_openai_adapters.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
unstract/sdk1/src/unstract/sdk1/adapters/base1.py (1)
564-583: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win"Thinking always enabled for MiniMax-M2.7" is only partially enforced.
The validation blocks explicitly disabling thinking for
minimax-m2*models (line 579-583), but if neitherenable_thinkingnorthinkingis present inadapter_metadataat all,thinkingstays at the class default (None) and nothinkingparam is sent — relying entirely on the JSON-schema UI default (enable_thinking: true) and/or the provider's own default to actually enable it. Any caller that invokesMiniMaxLLMParameters.validate()/LLM.__init__directly (bypassing the schema-populated defaults — e.g. persisted adapter configs created before this default existed, or non-UI programmatic callers) would silently omitthinkingfor M2.7, which doesn't guarantee "always enabled" as stated in the PR objectives.Consider enforcing the default at the validation layer itself, e.g.:
💡 Proposed fix to force adaptive thinking as the default for the M2 family
thinking = adapter_metadata.get("thinking") + if thinking is None and model_id.lower().startswith("minimax-m2"): + thinking = {"type": "adaptive"} + adapter_metadata["thinking"] = thinking if thinking is not None:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@unstract/sdk1/src/unstract/sdk1/adapters/base1.py` around lines 564 - 583, Update the adapter metadata validation around the existing thinking handling so minimax-m2* models default to {"type": "adaptive"} when neither enable_thinking nor thinking is provided. Preserve explicit disabled-thinking rejection and existing validation for supplied values, ensuring direct validation and LLM initialization always send adaptive thinking for MiniMax-M2 models.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@unstract/sdk1/src/unstract/sdk1/adapters/base1.py`:
- Around line 564-583: Update the adapter metadata validation around the
existing thinking handling so minimax-m2* models default to {"type": "adaptive"}
when neither enable_thinking nor thinking is provided. Preserve explicit
disabled-thinking rejection and existing validation for supplied values,
ensuring direct validation and LLM initialization always send adaptive thinking
for MiniMax-M2 models.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 476807d6-7add-498f-a968-25dda55fe2c3
📒 Files selected for processing (6)
unstract/sdk1/src/unstract/sdk1/adapters/base1.pyunstract/sdk1/src/unstract/sdk1/adapters/llm1/minimax.pyunstract/sdk1/src/unstract/sdk1/adapters/llm1/static/minimax.jsonunstract/sdk1/src/unstract/sdk1/llm.pyunstract/sdk1/tests/test_branded_openai_adapters.pyunstract/sdk1/tests/test_openai_compatible_adapter.py
🚧 Files skipped from review as they are similar to previous changes (1)
- unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/minimax.json
|
Thanks for the rework — routing through One structural request on the cost side: could we drop the bespoke pricing engine ( The only reason that engine is needed today is that our pinned Once #2169 lands, the standard path covers M3 with zero local pricing code — which avoids the drift/maintenance of a MiniMax-only cost table and keeps this adapter consistent with the rest. Two follow-ups:
Happy to help coordinate sequencing with #2169. |
|
@octo-patch TLDR from this comment
|
…r-add-recvozNE1fKYRA-clean
for more information, see https://pre-commit.ci
|
Thanks for the review. I removed the adapter-specific model registration, pricing calculator, and shared billing hook; the adapter now uses the standard LiteLLM cost path from the merged dependency update, while keeping the model field free-text. I also added protocol-specific temperature validation and MiniMax-M2.x thinking coverage, and ran 25 focused tests plus four endpoint request captures. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
unstract/sdk1/src/unstract/sdk1/adapters/base1.py (1)
622-624: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix literal
"None"coercion for missing models.If a caller explicitly sets
{"model": None}inadapter_metadata,adapter_metadata.get("model", "")evaluates toNone. The subsequentstr(None)call coerces it into the literal string"None". Because"None"is truthy, it bypasses thenot modelcheck and sends a malformed model ID to the upstream API.Update the extraction to safely handle
Nonevalues, mirroring the robust pattern used in the Mistral adapter below.🐛 Proposed fix
`@staticmethod` def validate_model(adapter_metadata: dict[str, "Any"]) -> str: - model = str(adapter_metadata.get("model", "")).strip() + raw_model = adapter_metadata.get("model") + model = str(raw_model).strip() if raw_model is not None else "" if not model: raise ValueError("model is required for the MiniMax adapter.")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@unstract/sdk1/src/unstract/sdk1/adapters/base1.py` around lines 622 - 624, Update the model extraction in the MiniMax adapter to handle an explicit None as missing before string conversion, matching the robust handling used by the Mistral adapter. Ensure None, empty, and whitespace-only values still trigger the existing “model is required” ValueError, while valid model identifiers remain stripped and accepted.
🧹 Nitpick comments (1)
unstract/sdk1/src/unstract/sdk1/adapters/base1.py (1)
591-604: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFuture-proof the
MiniMax-M2prefix check.The
model_id.lower().startswith("minimax-m2")check correctly identifies current models likeMiniMax-M2.7, but it will also unintentionally match future models likeMiniMax-M20orMiniMax-M200.Consider tightening the check (e.g., matching
"minimax-m2.","minimax-m2-", or using a regex boundary) to ensure future model families don't accidentally inherit M2-specific thinking constraints.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@unstract/sdk1/src/unstract/sdk1/adapters/base1.py` around lines 591 - 604, Update both MiniMax-M2 checks in the thinking logic to require a model-family boundary after “minimax-m2” (such as a dot, hyphen, or equivalent regex boundary), so models like MiniMax-M20 and MiniMax-M200 do not match. Preserve automatic adaptive thinking and the disabled-thinking validation for valid MiniMax-M2 variants.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@unstract/sdk1/src/unstract/sdk1/adapters/base1.py`:
- Around line 622-624: Update the model extraction in the MiniMax adapter to
handle an explicit None as missing before string conversion, matching the robust
handling used by the Mistral adapter. Ensure None, empty, and whitespace-only
values still trigger the existing “model is required” ValueError, while valid
model identifiers remain stripped and accepted.
---
Nitpick comments:
In `@unstract/sdk1/src/unstract/sdk1/adapters/base1.py`:
- Around line 591-604: Update both MiniMax-M2 checks in the thinking logic to
require a model-family boundary after “minimax-m2” (such as a dot, hyphen, or
equivalent regex boundary), so models like MiniMax-M20 and MiniMax-M200 do not
match. Preserve automatic adaptive thinking and the disabled-thinking validation
for valid MiniMax-M2 variants.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77326b44-0569-40ac-8939-f863ea0f799e
📒 Files selected for processing (4)
unstract/sdk1/src/unstract/sdk1/adapters/base1.pyunstract/sdk1/src/unstract/sdk1/adapters/llm1/minimax.pyunstract/sdk1/src/unstract/sdk1/adapters/llm1/static/minimax.jsonunstract/sdk1/tests/test_branded_openai_adapters.py
💤 Files with no reviewable changes (1)
- unstract/sdk1/src/unstract/sdk1/adapters/llm1/minimax.py
🚧 Files skipped from review as they are similar to previous changes (1)
- unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/minimax.json
|
Thanks for the review. I pushed a follow-up that rejects null or blank model values, tightens the MiniMax-M2 family boundary, and aligns both compatible protocols with the official 0-2 temperature range. I ran 29 focused tests, the repository pre-commit hooks, and four regional and protocol request captures. |
|
Updated the branch to follow the standard LiteLLM cost path by removing local model registration, priority cost multiplication, and shared cache billing changes. The adapter retains protocol-native thinking defaults and uses validated internal context-window metadata that is stripped before requests. I ran 58 focused tests, 147 shared LLM tests, repository pre-commit hooks, Python compilation, and four regional and protocol request captures. |
…icing The MiniMax adapter's JSON schema descriptions embedded per-token prices, per-model context windows and the four regional endpoint URLs. Every other LLM adapter keeps these one line long and links to the provider's docs, and the adapter-ops skill template does the same. Provider-owned facts go stale: MiniMax's pricing page currently advertises a "permanent 50% discount", the model description named 2 of the 8 documented chat models, and the model field is free text. Keep the facts the adapter owns and links out for the rest: - model: link to MiniMax's model list and pricing, keep the examples. - api_base: state the routing rule (a path ending in /anthropic selects the Anthropic-compatible protocol), which is not discoverable from MiniMax's docs. Drop the URL list. - service_tier: drop the 1.5x multiplier and note that cost tracking records standard-tier rates, since LiteLLM has no priority pricing for minimax. - enable_thinking: M2.x, not just M2.7, keeps thinking enabled -- matches _is_minimax_m2_model(). - max_tokens: match the wording used by the other adapters. The schema test asserted on the description prose, which pinned the copy in place; the four endpoints it checked are already covered by the protocol routing tests. Replace it with a guard that fails if a price is ever pasted back into a description.
_MINIMAX_CONTEXT_WINDOWS hardcoded MiniMax-M3 and MiniMax-M2.7 and let every other model fall through to LiteLLM. That is backwards: LiteLLM already has the right window for M3, while it reports 1,000,000 for MiniMax-M2.1 and MiniMax-M2.5 against a documented 204,800. The model field is free text, so selecting M2.1 or M2.5 sized prompts around a ~5x overstated window and the request would fail upstream. MiniMax documents 204,800 for the whole M2.x family, so derive it from the existing _is_minimax_m2_model() boundary instead of listing each model. This also covers the -highspeed and -lightning variants. Unknown models still fall through to LiteLLM, so future models keep working without a change here.
for more information, see https://pre-commit.ci
chandrasekharan-zipstack
changed the title
|
2 participants
What
Why
How
minimax/routing for OpenAI-compatible bases andanthropic/routing for bases ending in/anthropic.service_tier.Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
litellm==1.90.3.Notes on Testing
/v1/chat/completionspaths and both Anthropic-compatible/anthropic/v1/messagespaths.Screenshots
Checklist