Skip to content

feat(model_apis): add MiniMax Cloud API as text generation provider - #339

Open
octo-patch wants to merge 2 commits into
stochasticai:mainfrom
octo-patch:feature/add-minimax-cloud-api
Open

feat(model_apis): add MiniMax Cloud API as text generation provider#339
octo-patch wants to merge 2 commits into
stochasticai:mainfrom
octo-patch:feature/add-minimax-cloud-api

Conversation

@octo-patch

Copy link
Copy Markdown

Summary

Add MiniMax as the 4th cloud text-generation API provider (alongside OpenAI, Cohere, and Claude).

xTuring already has excellent MiniMax M2 local fine-tuning support (PR #307) — this PR complements it by adding cloud API access to MiniMax latest models (M2.7 / M2.7-highspeed) for self-instruct data generation and text generation workflows.

Changes

File Description
src/xturing/model_apis/minimax.py MiniMaxTextGenerationAPI — OpenAI-compat client at api.minimax.io/v1, temperature clamping, retry with exponential backoff. Convenience classes MiniMaxM27 and MiniMaxM27HighSpeed.
src/xturing/model_apis/init.py Register minimax, minimax_m2_7, minimax_m2_7_highspeed in BaseApi registry
tests/xturing/model_apis/test_minimax_api.py 18 unit tests + 3 integration tests (skip when MINIMAX_API_KEY not set)
README.md Cloud API table with all 4 providers, usage example, roadmap update

Usage

from xturing.model_apis import MiniMaxM27

api = MiniMaxM27(api_key="your-minimax-api-key")
results = api.generate_text(
    prompts=["Explain quantum computing for beginners."],
    max_tokens=256,
    temperature=0.7,
)
print(results[0]["response"]["choices"][0]["text"])

Test plan

  • 18 unit tests pass
  • 3 integration tests pass with live MINIMAX_API_KEY
  • Existing Claude API tests still pass (pre-existing failures unrelated)
  • No new dependencies — reuses existing openai SDK

PR Bot and others added 2 commits March 28, 2026 19:31
Add MiniMax as a 4th cloud text-generation API alongside OpenAI, Cohere,
and Claude. Uses the OpenAI-compatible endpoint at api.minimax.io/v1,
with temperature clamping (MiniMax requires >0), retry logic, and
convenience wrappers for MiniMax-M2.7 and MiniMax-M2.7-highspeed.

- src/xturing/model_apis/minimax.py: MiniMaxTextGenerationAPI + presets
- src/xturing/model_apis/__init__.py: register minimax/m2_7/m2_7_highspeed
- tests/xturing/model_apis/test_minimax_api.py: 18 unit + 3 integration tests
- README.md: Cloud API table and usage example
`OpenAI` is only exported by openai>=1.0.0. On a 0.x install -- which
pyproject still permits via `openai >= 0.27.0` -- the package imports fine
but the symbol is missing, raising ImportError. ModuleNotFoundError does not
catch that, so the guard was bypassed and `import xturing.model_apis` failed
outright.

Verified against a simulated 0.x layout: the module now imports and
construction raises the intended actionable error.
@glennko

glennko commented Aug 29, 2026

Copy link
Copy Markdown
Member

Pushed a one-line fix to this branch (thanks for enabling maintainer edits).

except ModuleNotFoundErrorexcept ImportError in minimax.py.

OpenAI is only exported by openai>=1.0.0. On a 0.x install — which pyproject.toml still permits via openai >= 0.27.0 — the package imports fine but the symbol is missing, so from openai import OpenAI raises ImportError, which ModuleNotFoundError does not catch. The guard was bypassed and import xturing.model_apis failed outright. Since xturing.datasets imports that package transitively, it would have broken the README quickstart on a 0.x install.

Verified against a simulated 0.x layout: the module now imports and construction raises the intended actionable error.

The overall shape of the PR is good — the deferred-dependency pattern matches ClaudeTextGenerationAPI, and the tests use strict patch and cover the missing-dependency path.

Three things left for you, none blocking:

  1. Exhausted retries return {"response": None}. A caller doing results[0]["response"]["choices"] gets a TypeError with no indication the API failed. cohere.py has the same shape, so it is consistent with existing code — but worth considering raising instead.
  2. _clamp_temperature silently turns temperature=0 into 0.01, converting greedy decoding into sampling. Reasonable if the API rejects 0, but worth a docstring note.
  3. frequency_penalty, presence_penalty, logprobs, n, best_of are accepted and silently ignored.

Heads-up on ordering: this conflicts with #318 on src/xturing/model_apis/__init__.py (both add imports and registry lines). Trivial both-sides-add resolution, but whichever merges second needs a rebase.

@glennko

glennko commented Sep 3, 2026

Copy link
Copy Markdown
Member

Review findings

Thanks for this — the structure is genuinely good. minimax.py follows claude.py closely (same optional-dependency shim, same _make_request/_render_response/generate_text split, same retry/backoff, same base-class + convenience-subclass + registry pattern), and the output envelope matches what self_instruct/bootstrap_instructions.py:69-75 consumes. I ran the new tests locally: 18 passed, 3 skipped.

A few things need resolving before this can merge.

Blocking

1. Could you confirm the model IDs against a live endpoint? The PR ships MiniMax-M2.7 and MiniMax-M2.7-highspeed (lines 145, 156). I wasn't able to corroborate either against MiniMax's published lineup (M1 / M2 / M2.1 / MiniMax-Text-01), and -highspeed doesn't appear as a MiniMax suffix anywhere I could find. This repo's own local engine uses MiniMaxAI/MiniMax-M2 (src/xturing/engines/minimax_m2_engine.py:11). The base URL itself looks right.

I'm flagging this rather than asserting it's wrong — you may have access to models I can't see. But because every unit test is mock-based, CI would pass identically with non-existent IDs, so nothing here can catch it. A transcript from a real call with MINIMAX_API_KEY set would settle it.

2. openai 0.x vs 1.x conflict, currently undeclared. minimax.py:22 needs the 1.x OpenAI client class, but pyproject.toml pins openai >= 0.27.0 and the existing src/xturing/model_apis/openai.py is written against 0.x (openai.Completion.create, except openai.error.OpenAIError). Both can't work under one installed version, and this PR doesn't bump the pin. Under 0.x, MiniMax degrades to ModuleNotFoundError at construction; under 1.x the OpenAI provider is broken (that half is pre-existing, and #341 is addressing it from the other side).

Either bump/declare the dependency, or drop the SDK and call the endpoint with httpx/requests directly — the latter also decouples this provider from the 0.x wrapper entirely, which may be the cleaner option.

3. pre-commit will fail. black (pinned 25.1.0) reformats src/xturing/model_apis/__init__.py — the add_to_registry(MiniMaxTextGenerationAPI...) call fits in 88 chars and gets collapsed — plus ~6 sites in the test file. .github/workflows/ci.yml runs pre-commit run -a. A pre-commit run -a pass locally should clear it.

Note that no workflow has ever run on this branch — as an outside contributor's PR it needs a maintainer to approve the run, so pre-commit and semgrep have never actually reported. @glennko worth approving that so this gets real signal.

Worth addressing

  • top_p=0 isn't clamped. Lines 50-53 clamp temperature but lines 62-63 forward top_p verbatim. Every self-instruct call site passes top_p=0 (identify_if_classification.py:81, generate_instances.py:120), and OpenAI-compatible endpoints generally want top_p in (0, 1]. If temperature needed clamping, top_p very likely does too — otherwise the headline use case may 400 on every request.
  • Silent failure after retry exhaustion (lines 105-137): response stays None and the result gets "response": None with no raise or warning. Combined with the uncapped 30s→45s→67.5s→101s backoff, that's ~4 minutes of blocking sleep per prompt before quietly yielding nothing. It also sleeps after the final failed attempt. Inherited from claude.py, so fixing it here is optional, but worth knowing.
  • Silently dropped params: frequency_penalty, presence_penalty, logprobs, n, best_of are in the signature (86-100) but never forwarded, and request_batch_size is stored but unused (prompts go serially) even though self-instruct slices batches by it. Either forward them or document the omission.
  • _clamp_temperature rewrites negative temperatures to 0.01 rather than erroring.
  • Minor: import importlib is function-local at line 37 where claude.py uses a module-level from importlib import import_module; and _render_response (69-84) reaches into response.choices[0].message.content directly where claude.py uses defensive getattr.
  • No docs/ page was added, only README. Not required, but the other providers are documented there.

Tests

They pass locally, but they will never run in CI as things stand. The lightweight-tests job hardcodes only tests/xturing/cli/test_api_server.py and tests/xturing/evaluation/test_runner.py and installs only pytest fastapi uvicorn httpx — and model_apis/__init__.py eagerly imports cohere, anthropic, and openai, so the file couldn't be added there as-is. This is a pre-existing gap (test_claude_api.py is un-run for the same reason), and #343 adds a model-api-tests job that would cover tests/xturing/model_apis/. Once that lands, these should execute.

Security

Clean. No hardcoded credentials (test keys are literal "test-key"), no key logging, the key only reaches the SDK constructor, and integration tests read os.environ["MINIMAX_API_KEY"] and skip when absent. No new third-party dependency. The print(f"MiniMaxError: {e}.") at line 124 could echo a provider error body to stdout, but that matches the existing OpenAI/Cohere/Claude providers.

@glennko

glennko commented Sep 3, 2026

Copy link
Copy Markdown
Member

Correction to my earlier comment. I said no workflow had ever run on this branch — that was wrong. gh pr checks returns nothing here, which misled me, but the check suites on a08047d show CI did run:

check result
semgrep/ci success
docs-build success
lightweight-tests success
pre-commit failure

So no maintainer approval is needed, and my point 3 is confirmed by CI rather than just predicted: the pre-commit job failed. A local pre-commit run -a should clear it — black 25.1.0 wants src/xturing/model_apis/__init__.py and ~6 sites in the test file reformatted.

One thing that's changed since: #343 has now merged, adding a model-api-tests job that runs pytest -q tests/xturing/model_apis/. Once you rebase on current main, your MiniMax tests will actually execute in CI instead of being collected by nothing — so the suite will be doing real work on the next push.

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.

2 participants