Add MiniMax official Token Plan video provider (direct API, not FAL)#297
Add MiniMax official Token Plan video provider (direct API, not FAL)#297yiyabo wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new first-class MiniMax “Token Plan” video provider tool that calls the official MiniMax API directly (CN/global endpoints) so the cost route is explicit and distinct from the existing FAL-backed minimax_video.
Changes:
- Introduces
minimax_tokenplan_video(submit → poll → retrieve → download) usingMINIMAX_API_KEY/MINIMAX_TOKEN_PLAN_API_KEYandMINIMAX_REGION. - Adds contract tests covering BaseTool compliance, idempotency keys, and key status behavior.
- Updates provider documentation and
.env.exampleto expose setup for the new provider.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| tools/video/minimax_tokenplan_video.py | New MiniMax Token Plan provider tool (official API flow, polling, download, output probing). |
| tests/contracts/test_minimax_tokenplan_video.py | Contract + tool-specific tests ensuring registry discovery, idempotency, and schema behavior. |
| docs/PROVIDERS.md | Documents the new MiniMax Token Plan provider and setup. |
| .env.example | Adds MINIMAX_API_KEY (and should reflect the supported alias env var). |
| def _api_key(self) -> str | None: | ||
| key = os.environ.get("MINIMAX_API_KEY") or os.environ.get("MINIMAX_TOKEN_PLAN_API_KEY") | ||
| if key and not key.strip().startswith("#"): | ||
| return key.strip() | ||
| return None |
| api_key = self._api_key() | ||
| if not api_key: | ||
| return ToolResult( | ||
| success=False, | ||
| error="MINIMAX_API_KEY not set. " + self.install_instructions, | ||
| ) |
| "aigc_watermark": { | ||
| "type": "boolean", | ||
| "default": False, | ||
| "description": "Embed an AI-generated content watermark.", | ||
| }, | ||
| "output_path": {"type": "string"}, | ||
| "poll_interval_seconds": { | ||
| "type": "number", | ||
| "minimum": 2, | ||
| "default": 5.0, | ||
| }, | ||
| "timeout_seconds": { | ||
| "type": "integer", | ||
| "minimum": 60, | ||
| "default": 600, | ||
| }, | ||
| }, |
| retrieve_resp = requests.get( | ||
| f"{base}/v1/files/retrieve", | ||
| headers=headers, | ||
| params={"file_id": file_id}, | ||
| timeout=30, | ||
| ) | ||
| retrieve_resp.raise_for_status() | ||
| retrieve_data = retrieve_resp.json() | ||
| download_url = ( | ||
| retrieve_data.get("file", {}).get("download_url") | ||
| or retrieve_data.get("download_url") | ||
| ) |
| @staticmethod | ||
| def _safe_error(exc: Exception) -> str: | ||
| return str(exc).replace( | ||
| os.environ.get("MINIMAX_API_KEY", ""), "[redacted]" | ||
| ) |
| **Tools unlocked:** `minimax_tokenplan_video` | ||
| **Env var:** `MINIMAX_API_KEY` (optionally `MINIMAX_REGION=global` for the international endpoint) | ||
|
|
| MINIMAX_API_KEY= # MiniMax official Token Plan (direct Hailuo API, not FAL). Set MINIMAX_REGION=global for api.minimax.io | ||
| # Get one at https://platform.minimax.io/user-center/basic-information/interface-key |
c7e6ef0 to
aaaa979
Compare
calesthio
left a comment
There was a problem hiding this comment.
Thanks for the focused provider PR. I reviewed this against docs/PR_REVIEW_GUIDE.md, including Copilot's earlier comments and the latest force-pushed head. The provider direction is useful and aligned with issue #216, and the PR's focused test file passes locally, but a few contract/setup blockers remain.
Findings:
-
The Token Plan key alias is still lower priority than the generic key.
_api_key()currently returnsMINIMAX_API_KEYbeforeMINIMAX_TOKEN_PLAN_API_KEY. For this tool, the route is the product: if a user has both a generic MiniMax API key and a Token Plan subscription key configured, this provider should not silently choose the generic key. Please either preferMINIMAX_TOKEN_PLAN_API_KEYfirst, or remove the alias entirely and makeMINIMAX_API_KEYthe only supported contract. Given the PR already advertises Token Plan behavior and tests the alias, preferring the Token Plan key is the cleaner fix. -
The alias is still not reflected in setup-facing docs/errors.
.env.example,docs/PROVIDERS.md,install_instructions, and the missing-key error mention onlyMINIMAX_API_KEY, while the implementation and tests supportMINIMAX_TOKEN_PLAN_API_KEY. That leaves a hidden supported config path and makes setup/debug output misleading. Please align docs and runtime messages with whichever env-var contract you choose. -
callback_urlis still an undocumented public input._build_payload()accepts and forwardscallback_url, butinput_schemadoes not declare it. Tool schemas feed registry/docs/agent planning, so accepted provider inputs need to be part of the contract or removed from the implementation. -
_safe_error()still has the empty-string replacement bug and does not redact the token-plan alias. I reproduced this on the current PR head:
safe-empty: [redacted]a[redacted]b[redacted]c[redacted]
safe-token: [redacted]b[redacted]a[redacted]d[redacted] [redacted]t[redacted]o[redacted]k[redacted]e[redacted]n[redacted]-[redacted]s[redacted]e[redacted]c[redacted]r[redacted]e[redacted]t[redacted]
This happens because os.environ.get("MINIMAX_API_KEY", "") can be empty, and the function never checks MINIMAX_TOKEN_PLAN_API_KEY. Please redact only non-empty configured secret values, including both supported env vars.
Copilot's /v1/files/retrieve error-handling comment does appear addressed: the current code uses _json_or_raise() and _check_base_resp() after retrieving the file metadata. The remaining comments above are still valid on the latest head.
Validation run locally:
python -m py_compile tools\\video\\minimax_tokenplan_video.py tests\\contracts\\test_minimax_tokenplan_video.py-> passedpython -m pytest tests\\contracts\\test_minimax_tokenplan_video.py -q-> 49 passedpython -c "from tools.tool_registry import registry; registry.discover(); ..."->minimax_tokenplan_videois discoverable asprovider=minimax,capability=video_generationgit merge-tree --write-tree HEAD origin/main-> clean merge tree
I did not find an actionable security issue beyond the secret-redaction bug above, and I did not see supply-chain risk in this diff. Requesting changes because the provider's public setup contract and secret handling need to be corrected before merge.
aaaa979 to
bd52eaa
Compare
|
Thanks for the thorough review — all four findings are fixed in bd52eaa. 1. Token Plan key priority — 2. Alias in docs/errors — 3. callback_url — Added to 4. _safe_error() empty-string bug — Rewritten to iterate both env vars ( Added 4 regression tests covering each fix:
Happy to adjust if anything still looks off. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
docs/PROVIDERS.md:141
- The DashScope provider section lost its
### Alibaba DashScope — Qwen Image + TTS + ASRheading. After the MiniMax section separator (---), the DashScope section starts directly with a blockquote, which makes the docs harder to scan and breaks the table-of-contents style structure used elsewhere in this file.
---
> **Best for Chinese-language production.** One key unlocks Qwen-Image generation, Qwen-TTS Mandarin narration, and Qwen-ASR with word-level timestamps — the only DashScope path that provides word-level granularity for subtitle alignment.
**Tools unlocked:** `dashscope_image`, `dashscope_tts`, `dashscope_asr`
| idempotency_key_fields = [ | ||
| "prompt", | ||
| "model", | ||
| "operation", | ||
| "duration", | ||
| "resolution", | ||
| "first_frame_image", | ||
| "prompt_optimizer", | ||
| "fast_pretreatment", | ||
| "aigc_watermark", | ||
| ] |
| MINIMAX_API_KEY= # MiniMax official Token Plan (direct Hailuo API, not FAL). Set MINIMAX_REGION=global for api.minimax.io | ||
| # Preferred: MINIMAX_TOKEN_PLAN_API_KEY (same key, explicit Token Plan route) | ||
| # Get one at https://platform.minimax.io/user-center/basic-information/interface-key |
| def test_lazy_imports_requests(self): | ||
| import importlib | ||
| import sys | ||
| mod_name = "tools.video.minimax_tokenplan_video" | ||
| if "requests" in sys.modules: | ||
| del sys.modules["requests"] | ||
| importlib.reload(sys.modules[mod_name]) | ||
|
|
calesthio
left a comment
There was a problem hiding this comment.
Thanks for the follow-up fixes on the Token Plan provider. I re-reviewed the PR description, comments, previous requested-changes review, latest diff, and CI against docs/PR_REVIEW_GUIDE.md. The provider implementation direction is useful and the prior setup/redaction/schema blockers look addressed: Token Plan key priority, both env vars in errors/docs, callback_url in the schema, and safe secret redaction all have regression tests.\n\nValidation run locally on the PR head:\n- python -m pytest tests/contracts/test_minimax_tokenplan_video.py -q -> 53 passed\n- python -m py_compile tools/video/minimax_tokenplan_video.py tests/contracts/test_minimax_tokenplan_video.py -> passed\n\nOne docs regression still needs cleanup before merge: docs/PROVIDERS.md lost the ### Alibaba DashScope — Qwen Image + TTS + ASR section heading. The DashScope body now starts immediately after the new MiniMax section separator with the blockquote, so the providers doc no longer has a proper DashScope heading/section anchor. Provider docs are user-facing setup contract, so please restore that heading after the MiniMax section.\n\nI did not find remaining code, security, or supply-chain blockers in the provider itself; this should be ready once the docs section structure is repaired.
Implements Part 1 of issue calesthio#216: a first-class MiniMax Token Plan video provider that calls the official MiniMax API directly (api.minimaxi.com / api.minimax.io), distinct from the FAL-backed minimax_video tool. API flow: POST /v1/video_generation -> poll GET /v1/query/video_generation -> GET /v1/files/retrieve -> download. Features: - Text-to-video and image-to-video (Hailuo 2.3 / 2.3-Fast) - MiniMax-Hailuo-2.3-Fast correctly rejected for text-to-video - Watermark control (aigc_watermark) - Prompt optimizer toggle - callback_url webhook support (declared in input_schema) - CN endpoint (api.minimaxi.com) by default, global (api.minimax.io) via MINIMAX_REGION=global - Full error handling with MiniMax base_resp status codes - API key redaction in error messages (both env vars, no empty-string bug) - Token Plan key preferred over generic key when both are set Env vars: MINIMAX_TOKEN_PLAN_API_KEY (preferred) or MINIMAX_API_KEY. All docs, install_instructions, and error messages mention both. Idempotency keys include all output-affecting fields (prompt, model, operation, duration, resolution, first_frame_image, prompt_optimizer, fast_pretreatment, aigc_watermark). Files: - tools/video/minimax_tokenplan_video.py — new tool - tests/contracts/test_minimax_tokenplan_video.py — 53 contract tests - .env.example — MINIMAX_API_KEY + MINIMAX_TOKEN_PLAN_API_KEY - docs/PROVIDERS.md — MiniMax Token Plan provider section Test results: python -m pytest tests/contracts/test_minimax_tokenplan_video.py -q # 53 passed Full suite: 526 passed, 6 skipped (3 unrelated variation_checker leftovers)
bd52eaa to
9e0a820
Compare
|
Restored the This was the only remaining blocker from your latest review. Happy to merge once you confirm the docs structure looks right. |
Implements Part 1 of issue #216: a first-class MiniMax Token Plan video provider that calls the official MiniMax API directly, distinct from the FAL-backed
minimax_video.What this adds
A new tool
minimax_tokenplan_video(tools/video/minimax_tokenplan_video.py) that routes through the official MiniMax API (api.minimaxi.com/api.minimax.io) usingMINIMAX_API_KEY, so users with a Token Plan subscription consume their own quota instead of FAL credits.API flow:
POST /v1/video_generation→ pollGET /v1/query/video_generation→GET /v1/files/retrieve→ download.Why it's a separate tool (not a modification of
minimax_video)The core point of #216 is cost-route transparency.
minimax_videoroutes throughFAL_KEY;minimax_tokenplan_videoroutes throughMINIMAX_API_KEY. Both shareprovider="minimax"but have different tool names, making the cost route explicit in the registry and decision trail.Features
MiniMax-Hailuo-2.3/MiniMax-Hailuo-2.3-Fast)MiniMax-Hailuo-2.3-Fastcorrectly rejected for text-to-video (the API does not support it)aigc_watermarkparameterapi.minimaxi.com) by default; global (api.minimax.io) viaMINIMAX_REGION=globalbase_respstatus codes (1002 rate limit, 1004 auth, 1008 insufficient balance, 1026 sensitive content, etc.)Idempotency
All output-affecting fields are in
idempotency_key_fields(prompt, model, operation, duration, resolution, first_frame_image, prompt_optimizer, fast_pretreatment, aigc_watermark). Execution-only fields (output_path, poll_interval_seconds, timeout_seconds) are correctly excluded. Lesson applied from PR #240 review.Scope note
Issue #216 asks for three things:
make doctorenhancement) — broader scope, overlaps with Out-of-box animated-explainer run: Remotion URI bug, unenforced checkpoints, and silent text-card downgrade #210 and Google service-account auth is advertised but non-functional (TTS + Imagen) + false-availability bugs #131Parts 2 and 3 are left for separate PRs to keep this one focused and reviewable.
Files
tools/video/minimax_tokenplan_video.pytests/contracts/test_minimax_tokenplan_video.py.env.exampleMINIMAX_API_KEYentrydocs/PROVIDERS.mdTest results
Closes #216 (Part 1).