Skip to content

feat(provider): add MiniMax regional support with M3.1 Flash defaults - #366

Open
octo-patch wants to merge 6 commits into
NVIDIA:mainfrom
octo-patch:octo/20260812-provider-add-recvs0kphjyPs8
Open

octo-patch wants to merge 6 commits into
NVIDIA:mainfrom
octo-patch:octo/20260812-provider-add-recvs0kphjyPs8

Conversation

@octo-patch

@octo-patch octo-patch commented Aug 12, 2026 •

Copy link
Copy Markdown

Reason: SkillSpector needs a regional MiniMax provider with current Flash-first defaults and request/provenance behavior compatible with the scanner.

Summary

Add SKILLSPECTOR_PROVIDER=minimax with MiniMax-M3.1-Flash as the default and Flash / MiniMax-M3 as the bundled model choices. Explicit legacy and custom SKILLSPECTOR_MODEL values remain unchanged. Global and China endpoints use MINIMAX_REGION; MINIMAX_BASE_URL still 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

  • Exact source 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.
  • Repository-wide lint and format checks pass with lockfile-pinned Ruff 0.15.19 (306 files formatted). Focused tests use Python 3.12, LangChain OpenAI 1.3.3, OpenAI 2.44.0, and pytest 9.1.1 from the frozen lockfile.
  • HTTP-boundary tests use the real scanner → LangChain → OpenAI SDK path for default / explicit models, saved registry overrides, regional and custom URLs, authorization, effort validation, tool schemas, multimodal payloads, and synchronous/asynchronous streaming and nonstreaming reasoning/tool continuation.
  • Local source syntax and git diff --check pass. Local heavy setup/build remains resource-paused, so pinned dependency-backed validation ran in CI. No live MiniMax API request was made.
  • Full upstream CI for this exact head is still running the full unit suite; lint, TypeScript tests and DCO have passed. The full suite is not yet claimed green.

octo-patch and others added 2 commits August 12, 2026 21:55
class MiniMaxProvider:
"""MiniMax credentials, regional routing, and bundled model metadata."""

DEFAULT_MODEL = "MiniMax-M3"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@rng1995

rng1995 commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

@octo-patch - Please address review comment, resolve merge conflict and ping me. We can then merge the PR.

@mohgupta-ship-it

mohgupta-ship-it commented Aug 25, 2026 •

Copy link
Copy Markdown
Member

Powered by Codex: PR council review result.

This is a triage signal, not a maintainer approval.

  • Rating: new feature
  • Confidence: medium
  • Status read: Changes requested, DCO fail, conflicting
  • Review method: fresh GitHub metadata/body/files/reviews/checks plus selected diffs; council lenses were spec fit, dead-code/reachability, YAGNI/scope, design/coupling, and code standards/tests.
  • Council assessment: MiniMax provider defaults/model limits appear unsupported by documented OpenAI-compatible behavior; provider could fail by default.
  • Recommended action: Verify current MiniMax served model IDs/limits, update defaults/tests, sign and rebase.

Signed-off-by: octo-patch <266937838+octo-patch@users.noreply.github.com>
@octo-patch

Copy link
Copy Markdown
Author

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 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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:

  1. Default MiniMax-M3 not served: resolved on Sep 4 (the default is now MiniMax-M2.7). The outdated thread can be closed.
  2. Unverified MiniMax-M3 entry with a 1M context: resolved. MiniMax's API overview and OpenAI-compatible reference now list MiniMax-M3 with a 1,000,000-token combined context, matching model_registry.yaml and the test.
  3. Blocker: branch behind and conflicting with main. Still open, in .env.example, README.md, providers/__init__.py and providers/_agent_cli.py. For _agent_cli.py, drop your hunk: main replaced _SECRET_ENV_PREFIXES with an allowlist (_RUNTIME_ENV_NAMES), so MINIMAX_API_KEY is already excluded. test_api_key_is_removed_from_cli_environment still 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-tree against main at 8b993d32 reports 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-overview and text-openai-api today (M3: 1,000,000; M2.7: 204,800). I could not confirm the reasoning_split default.
  • 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>
@octo-patch octo-patch changed the title feat(provider): add MiniMax regional support feat(provider): add MiniMax regional support with M3.1 Flash defaults Oct 20, 2026
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 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants