feat(minimax): add MiniMax provider with M3 as default#3165
Conversation
- Add MiniMaxOpenAICompatibleProvider extending DefaultOpenAICompatibleProvider - Use OpenAI-compatible interface with https://api.minimax.io/v1 as default base URL - Enforce temperature in (0.0, 1.0] range; remove unsupported response_format - Register provider detection in determineProvider() factory - Add token limits for MiniMax-M2.7 (200K input, 64K output) - Add unit tests for all provider behaviors
| */ | ||
| override buildRequest( | ||
| request: OpenAI.Chat.ChatCompletionCreateParams, | ||
| userPromptId: string, |
There was a problem hiding this comment.
[Suggestion] buildRequest uses any type and delete instead of typed destructuring.
The method casts to any and uses delete to remove response_format. This disables type checking and conflicts with the project's strict/noImplicitAny conventions.
| userPromptId: string, | |
| const { response_format: _, ...rest } = baseRequest; | |
| const temperature = (rest.temperature === 0 || rest.temperature == null) | |
| ? 1.0 | |
| : rest.temperature; | |
| return { ...rest, temperature } as OpenAI.Chat.ChatCompletionCreateParams; |
— qwen3.6-plus via Qwen Code /review
| contentGeneratorConfig: ContentGeneratorConfig, | ||
| cliConfig: Config, | ||
| ) { | ||
| super(contentGeneratorConfig, cliConfig); |
There was a problem hiding this comment.
[Suggestion] Constructor mutates this.contentGeneratorConfig after the super() call.
Sibling providers (DashScope, DeepSeek) leave the config unchanged and apply defaults in buildClient() via destructuring fallback. If any code holds a reference to the original config object, it won't see the updated baseUrl.
Consider applying the default URL in buildClient() or via a protected getter, matching sibling patterns.
— qwen3.6-plus via Qwen Code /review
| const result: any = { ...baseRequest }; | ||
|
|
||
| // MiniMax does not support temperature = 0; default to 1.0 | ||
| if (result.temperature === 0 || result.temperature === undefined) { |
There was a problem hiding this comment.
[Suggestion] Temperature check doesn't guard against null.
The OpenAI SDK types temperature as number | null | undefined. If null is passed, it bypasses both guards and gets sent to the API.
| if (result.temperature === 0 || result.temperature === undefined) { | |
| if (result.temperature == null || result.temperature === 0) { |
— qwen3.6-plus via Qwen Code /review
| * Checks if the configuration targets the MiniMax API. | ||
| */ | ||
| static isMiniMaxProvider(config: ContentGeneratorConfig): boolean { | ||
| const baseUrl = config.baseUrl ?? ''; |
There was a problem hiding this comment.
[Nice to have] isMiniMaxProvider uses bare substring matching (baseUrl.includes(...)).
A URL like https://proxy.example.com?target=api.minimax.io.malicious.com would false-positive. Consider using new URL(baseUrl).hostname with a more precise check. Very low probability in practice, but worth noting for correctness.
— qwen3.6-plus via Qwen Code /review
- Fix URL detection to use hostname comparison (prevents substring false positives flagged by CodeQL) - Move default base URL to buildClient() override — avoids mutating contentGeneratorConfig in the constructor (matches sibling pattern) - Replace any cast + delete with typed destructuring for response_format - Guard temperature against null (number | null | undefined per SDK types) - Clarify token-limit comment: regex covers M2.7-highspeed too - Add MiniMax section to model-providers.md with config examples for both MiniMax-M2.7 and MiniMax-M2.7-highspeed
|
Thanks for the contribution! Overall the implementation is solid, follows the existing provider conventions, and has good test coverage. The strict hostname matching and typed destructuring for Mergeable, with minor suggestions1. Redundant input pattern in
|
…t pattern - buildRequest now clamps temperature > 1.0 down to 1.0 (was previously passed through to API and would have failed). Adds debug logs via the project DebugLogger when an explicit user value is rewritten so the adjustment is observable. - Removed redundant /^minimax-m2.7/ entry in tokenLimits.ts; the existing /^minimax-/ fallback already maps M2.7 / M2.7-highspeed to 200K. Addresses review suggestions on QwenLM#3165.
|
Thanks for the detailed review @wenshao! Addressed both points in 9417be7: 1. Redundant input pattern in Removed the explicit 2. Temperature Good catch — the previous code only normalized Added two tests covering the new clamping behavior. All 17 minimax tests + 54 tokenLimits tests pass locally. |
|
@octo-patch Hi, CI is failing on this PR — all 9 Test jobs (macOS/Ubuntu/Windows × Node 20/22/24) are broken. Failure: This was already fixed on main in #3415 (updated the |
- Add MiniMax-M3 to provider docs as the default (512K context, up to 128K output, supports image input). Keep M2.7 / M2.7-highspeed entries for users who want the previous generation. - Token limits: add /^minimax-m3/i input pattern at 512K and output pattern at 128K. M2.7 fallback unchanged at 200K input / 64K output. - Modality: M3 enables image input; everything else under the /^minimax-/ fallback stays text-only. - VSCode IDE companion token-limits table: same M3 entries for consistency. - Drop unused /^minimax-m2\.5/ pattern (no longer surfaced) along with its stale "MiniMax-M2.5 context window" comment on the shared 192k LIMITS slot. - Tests: replace the M2.5 / M2.1 cases with M3 (latest) and M2.7 / M2.7-highspeed (fallback) input/output assertions, plus a modality test verifying M3 returns image:true while M2.7 stays text-only.
| // MiniMax accepts temperature only in (0.0, 1.0]; rewrite invalid values. | ||
| const original = rest.temperature; | ||
| let temperature: number; | ||
| if (original == null || original === 0) { |
There was a problem hiding this comment.
[Critical] Temperature guard has a gap on the lower bound: negative values and NaN pass through all three checks (null/0/>1.0) and reach the else branch, which forwards them unchanged. The JSDoc and comment both state the valid range is (0.0, 1.0], but -0.5 and NaN bypass enforcement and will cause a MiniMax API 400 error.
| if (original == null || original === 0) { | |
| // MiniMax accepts temperature only in (0.0, 1.0]; rewrite invalid values. | |
| const original = rest.temperature; | |
| let temperature: number; | |
| if (original == null || !(original > 0)) { | |
| temperature = 1.0; | |
| if (original === 0) { | |
| // Only log when the user explicitly set 0; null/undefined is a default fill. | |
| debugLogger.debug( | |
| `temperature=0 is not supported; using 1.0 instead (request ${userPromptId})`, | |
| ); | |
| } | |
| } else if (original > 1.0) { | |
| temperature = 1.0; | |
| debugLogger.debug( | |
| `temperature=${original} exceeds the 1.0 max; clamping to 1.0 (request ${userPromptId})`, | |
| ); | |
| } else { | |
| temperature = original; | |
| } |
The !(original > 0) check catches both negative numbers and NaN in a single comparison (since NaN > 0 is false).
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Fixed in 92278c4. Replaced original == null || original === 0 with original == null || !(original > 0) per your suggestion, so negative values and NaN are also rewritten to 1.0. Added tests for temperature: -0.5 and temperature: Number.NaN that pin the behavior.
| // MiniMax | ||
| [/^minimax-m2\.5/i, LIMITS['64k']], | ||
| [/^minimax-m3/i, LIMITS['128k']], // MiniMax-M3: 128K output | ||
| [/^minimax-m2\.7/i, LIMITS['64k']], // MiniMax-M2.7 / M2.7-highspeed: 64K output |
There was a problem hiding this comment.
[Suggestion] OUTPUT_PATTERNS has no MiniMax fallback entry, while PATTERNS (input) retains [/^minimax-/i, LIMITS['200k']] as a fallback. This creates two issues:
- M2.5 regression: the old
[/^minimax-m2\.5/i, LIMITS['64k']]was removed; existing M2.5 users now fall through to the default 32K output. - Future variants: any new MiniMax model (M4, M3-turbo, etc.) added to
PATTERNSandMODALITY_PATTERNSbut notOUTPUT_PATTERNSwill silently get 32K instead of a reasonable default.
Additionally, hasExplicitOutputLimit() returns false for unmatched variants, which changes applyOutputTokenLimit behavior — user-configured max_tokens passes through uncapped, risking 400 errors when input + max_output > contextWindowSize.
| [/^minimax-m2\.7/i, LIMITS['64k']], // MiniMax-M2.7 / M2.7-highspeed: 64K output | |
| // MiniMax | |
| [/^minimax-m3/i, LIMITS['128k']], // MiniMax-M3: 128K output | |
| [/^minimax-m2\.7/i, LIMITS['64k']], // MiniMax-M2.7 / M2.7-highspeed: 64K output | |
| [/^minimax-/i, LIMITS['64k']], // MiniMax fallback: 64K output |
Apply the same fallback to packages/vscode-ide-companion/src/utils/tokenLimits.ts as well.
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Fixed in 92278c4. Added [/^minimax-m2\.5/i, 64K] and a [/^minimax-/i, 64K] fallback to both packages/core/src/core/tokenLimits.ts and packages/vscode-ide-companion/src/utils/tokenLimits.ts, so M2.5 keeps its 64K output limit and future MiniMax variants get a sensible default. New test covers the fallback path for unknown MiniMax variants.
| return false; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
[Suggestion] The catch {} block silently swallows URL parse errors. If a user configures a malformed MiniMax base URL (e.g., api.minimax.io/v1 without a scheme), isMiniMaxProvider returns false and the request falls through to DefaultOpenAICompatibleProvider, which sends parameters MiniMax doesn't accept (like temperature: 0 or response_format). The resulting API error gives no indication that provider mis-detection occurred.
| } catch (e) { | |
| debugLogger.debug(`isMiniMaxProvider: failed to parse baseUrl "${baseUrl}": ${e}`); | |
| return false; | |
| } |
A single debug log at the detection boundary would make misconfiguration immediately visible.
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Fixed in 92278c4. The catch now logs the parse failure via debugLogger.debug instead of swallowing silently, so misconfigured base URLs like api.minimax.io/v1 (no scheme) are visible at the detection boundary. Added a test that asserts the no-scheme case returns false.
…ug log) - Temperature guard: replace `original == null || original === 0` with `original == null || !(original > 0)` so negative values and NaN are also rewritten to 1.0 instead of being forwarded as-is. NaN > 0 is false, so the single comparison catches both. New tests cover temperature=-0.5 and temperature=NaN. - OUTPUT_PATTERNS: add `[/^minimax-m2\.5/i, 64K]` and a `[/^minimax-/i, 64K]` fallback in both packages/core and packages/vscode-ide-companion so M2.5 keeps its 64K output limit and any future MiniMax variant gets a sensible 64K default instead of the global 32K. Test covers the fallback path. - isMiniMaxProvider: log the parse failure inside `catch` instead of swallowing silently, so misconfigured base URLs (e.g. `api.minimax.io/v1` without a scheme) are visible at the detection boundary. New test covers the no-scheme case. Co-Authored-By: Octopus <liyuan851277048@icloud.com>
|
@wenshao Pushed 92278c4 addressing the three latest review points (temperature lower-bound gap, OUTPUT_PATTERNS fallback, debug log for malformed base URLs). All targeted tests pass locally. On the merge-conflict / older CI failure: |
DragonnZhang
left a comment
There was a problem hiding this comment.
LGTM. The MiniMax provider integration is well-implemented and follows the established patterns from sibling providers (DashScope, DeepSeek).
Key observations:
- Hostname-based detection via
new URL().hostname+Setlookup correctly addresses the CodeQL substring-matching warnings. - Typed destructuring for
response_formatremoval avoidsanycasts anddelete. - Temperature guard comprehensively handles
null,undefined,0, negative,NaN, and values > 1.0. - Token limits are consistent between
packages/coreandpackages/vscode-ide-companion, with proper fallback patterns for future MiniMax models. - Test coverage is thorough (15 unit tests) including edge cases for hostname spoofing, negative temperature, and NaN.
- All previously raised review comments have been addressed in the latest commit.
— qwen-code via Qwen Code /review
|
Thanks for the thorough work here, @octo-patch — the provider implementation and the write-up (URL hostname matching, temperature clamping, typed Closing this one as superseded: MiniMax-M3 support has since landed on If anything you covered isn't reflected in the shipped support — e.g. the |
danialzivehdar1992-hue
left a comment
There was a problem hiding this comment.
86458953action>: GetSearchAnalytics
provider: searchconsole_********************
siteUrl: 'autoro.io'
startDate: '2019-12-01'
endDate: '2019-12-08'
dimensions: ['query', 'page']
filters: [{"dimension":"query","operator':"contains","expression":"rpa"}]
rowLimit: 1000
Summary
This PR adds MiniMax as an OpenAI-compatible provider for Qwen Code, with MiniMax-M3 wired up as the default model.
Changes
MiniMaxOpenAICompatibleProviderextendingDefaultOpenAICompatibleProviderbuildClient()override (matches sibling provider pattern)api.minimaxi.comas an alternative (domestic mirror)new URL(baseUrl).hostnameto prevent substring false-positives (fixes CodeQL warnings)nullper SDK types (number | null | undefined); clamps values above1.0down to1.0with a debug logresponse_format: removed via typed destructuring — noanycast ordeletetokenLimits.tsandvscode-ide-companion/.../tokenLimits.ts):MiniMax-M3: 512K input, 128K outputMiniMax-M2.7/M2.7-highspeedfallback: 200K input, 64K outputMiniMax-M3enables image input; M2.7 stays text-onlydocs/users/configuration/model-providers.mdlists M3 as the default plus M2.7 / M2.7-highspeed for users who want the previous generationSupported Models
MiniMax-M3(default)MiniMax-M2.7MiniMax-M2.7-highspeedAPI References