Repository navigation
feat(provider): add MiniMax regional support with M3.1 Flash defaults - #366
octo-patch wants to merge 6 commits into
Conversation
Signed-off-by: octo-patch <266937838+octo-patch@users.noreply.github.com>
| class MiniMaxProvider: | ||
| """MiniMax credentials, regional routing, and bundled model metadata.""" | ||
|
|
||
| DEFAULT_MODEL = "MiniMax-M3" |
There was a problem hiding this comment.
Official MiniMax API docs currently list MiniMax-M2.7 / MiniMax-M2.7-highspeed as the supported OpenAI-compatible model IDs, and the /v1/models example does not include MiniMax-M3; this default would make every out-of-box request fail. Use a served model with documented limits (or provide authoritative endpoint evidence plus a live contract test). Source: https://platform.minimax.io/docs/api-reference/api-overview
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Requesting changes. The provider defaults to MiniMax-M3 with a 1M registry window, but current official MiniMax OpenAI-compatible docs list M2.7-family served IDs and a 204,800 combined context; the proposed default would fail out of the box. Use a documented served model/limits or provide authoritative endpoint evidence plus a live contract test. The branch also conflicts with main and DCO fails. See the inline source link.
|
@octo-patch - Please address review comment, resolve merge conflict and ping me. We can then merge the PR. |
|
Powered by Codex: PR council review result. This is a triage signal, not a maintainer approval.
|
Signed-off-by: octo-patch <266937838+octo-patch@users.noreply.github.com>
|
Thanks for the review. I updated the default to MiniMax-M2.7 with its 204,800-token context and merged the current main branch to resolve the conflicts. DCO, lint/format, and Docker smoke now pass; the unit-test job is still running. |
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Re-review of b97421a: the original default-model blocker is resolved—MiniMax-M2.7 is a documented OpenAI-compatible model with a 204,800-token combined context, and all hosted checks pass. One model-contract issue remains: src/skillspector/providers/minimax/model_registry.yaml still advertises MiniMax-M3 with an unverified 1,000,000-token context, and the test pins that claim, while MiniMax's current official supported-model list does not include M3. Remove that registry entry/test or provide authoritative endpoint evidence plus a live contract test. The branch is also behind main and must be updated before merge.
Official model list: https://platform.minimax.io/docs/api-reference/api-overview
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
@octo-patch thank you for the regional MiniMax provider, and for switching the default to M2.7 so quickly last time.
Value and readiness
The model-contract concerns are now resolved: MiniMax's own docs have caught up with the registry. What remains is the rebase. The branch conflicts with main in four files, and the rebase has to pick up one new main contract.
Material findings
Prior findings:
- Default
MiniMax-M3not served: resolved on Sep 4 (the default is nowMiniMax-M2.7). The outdated thread can be closed. - Unverified
MiniMax-M3entry with a 1M context: resolved. MiniMax's API overview and OpenAI-compatible reference now listMiniMax-M3with a 1,000,000-token combined context, matchingmodel_registry.yamland the test. - Blocker: branch behind and conflicting with
main. Still open, in.env.example,README.md,providers/__init__.pyandproviders/_agent_cli.py. For_agent_cli.py, drop your hunk: main replaced_SECRET_ENV_PREFIXESwith an allowlist (_RUNTIME_ENV_NAMES), soMINIMAX_API_KEYis already excluded.test_api_key_is_removed_from_cli_environmentstill holds through_scrub_env().
New:
4. Blocker (rebase): add MiniMaxProvider to inference_usage.provider_name() and to the adapter sets in llm_provenance.py (#556). Otherwise the report's LLM provenance labels the adapter minimaxprovider and reports no temperature, seed or reasoning-effort support, although the shared OpenAI builder forwards all three.
5. Optional: MiniMax's OpenAI-compatible reference documents tools but not response_format / json_schema. It also says reasoning can arrive inside <think> tags in content. Please run one live structured-output check against MiniMax-M2.7. If it fails, return function_calling from a structured_output_method() hook (#529, #806). A failure would surface as incomplete analysis (#796), not as a clean scan.
PIC tradeoffs
None identified beyond the general choice to accept named vendor providers. Main's openai_compatible can also reach MiniMax, but without region selection.
Verification and gaps
git merge-treeagainstmainat8b993d32reports 4 conflicting files.- Read main's
_scrub_env,create_openai_compatible_chat_model,provider_name()and the provenance adapter sets. - Fetched platform.minimax.io
api-overviewandtext-openai-apitoday (M3: 1,000,000; M2.7: 204,800). I could not confirm thereasoning_splitdefault. - CI was green on Sep 4, against an older base. Static review only.
Signed-off-by: octo-patch <266937838+octo-patch@users.noreply.github.com>
Signed-off-by: octo-patch <266937838+octo-patch@users.noreply.github.com>
Signed-off-by: octo-patch <266937838+octo-patch@users.noreply.github.com>
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Value and readiness
The regional provider is integrated with current main, now defaults to the documented MiniMax-M3.1-Flash, and uses function calling for scanner schemas. The earlier model, conflict and provenance blockers are resolved in code. Reviewed head e1a0a14582458eaba50f3638581ccbd6507aae74.
Material findings
No required code change found. Global/China routing and explicit base/model overrides remain predictable. inference_usage and provenance adapter sets include MiniMax. The shared subprocess allowlist still excludes its API key, and the YARA credential pattern recognizes it. The specialized chat model preserves reasoning_content alongside tool calls in sync/async streaming and nonstreaming histories. The official model/API docs now list Flash and M3 with 1M context, Flash effort values match the validation, and function tools are documented.
PIC tradeoffs
No live MiniMax inference was performed. The tests establish the scanner/LangChain/SDK wire contract against a mock endpoint; actual endpoint behavior remains vendor-dependent. Provenance distinguishes client forwarding from provider support, which remains unknown.
Verification and gaps
Read the complete 13-file diff and pinned source/tests, all earlier marked review bodies, author replies and the old model thread. Consulted the official MiniMax model and OpenAI-SDK docs (https://platform.minimax.io/docs/guides/models-intro and https://platform.minimax.io/docs/api-reference/text-openai-api). HTTP-boundary tests cover regional/custom URLs, auth, effort errors, structured schemas, saved registry overrides and reasoning/tool continuation. The hosted unit job has six binary-structure timing assertion failures outside the diff; other five checks pass. Those failures still block merging, and the outdated model conversation needs resolution. Static review only; no contributor code or live inference was run.
Reason: SkillSpector needs a regional MiniMax provider with current Flash-first defaults and request/provenance behavior compatible with the scanner.
Summary
Add
SKILLSPECTOR_PROVIDER=minimaxwithMiniMax-M3.1-Flashas the default and Flash /MiniMax-M3as the bundled model choices. Explicit legacy and customSKILLSPECTOR_MODELvalues remain unchanged. Global and China endpoints useMINIMAX_REGION;MINIMAX_BASE_URLstill overrides either endpoint.The provider validates Flash reasoning effort (
low,medium,high,xhigh,max), uses function calling for scanner schemas, and preserves MiniMax reasoning and tool-call history through synchronous, asynchronous, and streaming SDK paths. Register MiniMax in inference-usage and provenance metadata and retain API-key detection in the credential-exfiltration rule. Sync with current main resolves the previous merge conflicts and retains its subprocess credential allowlist.Official model and API references:
Validation
e1a0a14582458eaba50f3638581ccbd6507aae74: 317 passed, 12 skipped, covering MiniMax, provider selection, other OpenAI-compatible providers, provenance, inference usage, and model budgets. Pinned focused run checks out and asserts that source SHA; its workflow commit is separate from this PR.git diff --checkpass. Local heavy setup/build remains resource-paused, so pinned dependency-backed validation ran in CI. No live MiniMax API request was made.