feat(minimax): auto-detect CN/global endpoint and region label#230
feat(minimax): auto-detect CN/global endpoint and region label#230robinebers merged 13 commits intorobinebers:mainfrom
Conversation
FrankieeW
requested review from
davidarny,
robinebers and
validatedev
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba1c2be45a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
plugins/minimax/plugin.js
Outdated
| } | ||
| lastError = "Could not parse usage data." | ||
| } catch (e) { | ||
| lastError = String(e) |
There was a problem hiding this comment.
The fallback loop always overwrites lastError on each failed region attempt, so a definitive auth failure from the first endpoint can be replaced by a later unrelated failure from the fallback endpoint (for example: GLOBAL returns 401/403, then CN returns 5xx/network), and users get a misleading "Request failed" message instead of a key-expired message. This regression makes credential issues harder to diagnose compared with the previous behavior where auth failures surfaced directly.
Useful? React with 👍 / 👎.
|
Addressed review feedback in commit 4759164:\n\n1. Restored stable MiniMax line label to (no dynamic region suffix), so overview/tray matching stays intact.\n2. Updated AUTO retry error handling to preserve the first meaningful failure instead of always overwriting with later retry errors.\n3. Added regression tests for both precedence directions:\n - first non-auth error preserved when later retry is auth\n - first auth error preserved when later retry is non-auth\n\nValidation: ✓ plugins/minimax/plugin.test.js (38 tests) 15ms Test Files 1 passed (1) |
|
Correction (clean summary):\n\nAddressed feedback in commit 4759164.\n- Restored stable MiniMax line label to 'Session' (no region suffix), preserving overview/tray matching.\n- AUTO retry now preserves the first meaningful error instead of overwriting with later retry errors.\n- Added regression tests for both precedence directions (non-auth then auth, auth then non-auth).\n\nValidation: bun run test -- plugins/minimax/plugin.test.js (38/38 passing). |
|
Initially I considered keeping endpoint selection configurable. After revisiting the use case, there are only two valid regions (CN/GLOBAL), so I simplified the implementation to automatic detection/fallback instead. This has now been updated in the latest commit. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…RL retry logic - Add (CN)/(GLOBAL) suffix to plan name based on successful endpoint - Extract tryUrls() helper to reduce duplicate URL retry logic - Simplify fetchUsagePayload() to use shared tryUrls() - Update tests to verify region labels in plan names Benefits: - Users can now see which region their plan is using - Single source of truth for URL retry logic - Easier to maintain when adding/removing fallback URLs
- Separate GLOBAL and CN plan tier mappings - GLOBAL: 100/300/1000/2000 model calls for Starter/Plus/Max/Ultra - CN: 600/1500/4500 model calls (40/100/300 prompts × 15) for Starter/Plus/Max - Update parsePayloadShape to accept endpointSelection parameter - Add tests for CN plan inference (Starter/Plus/Max)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/providers/minimax.md">
<violation number="1" location="docs/providers/minimax.md:29">
P3: Documentation is internally inconsistent: the Authentication section still lists only `MINIMAX_API_KEY` and `MINIMAX_API_TOKEN` as the lookup order, but the newly added region-specific lookup says CN checks `MINIMAX_CN_API_KEY` first. Update the initial key-order description to reflect the CN key precedence.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Remove inconsistent description that only listed MINIMAX_API_KEY and MINIMAX_API_TOKEN - Consolidate region auto-selection and key lookup into one clear section - Emphasize that CN region checks MINIMAX_CN_API_KEY first - Fix internal inconsistency in documentation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1c3d6502a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // CN plan tiers (based on model call counts = prompts × 15) | ||
| // Starter: 40 prompts = 600, Plus: 100 prompts = 1500, Max: 300 prompts = 4500 | ||
| const CN_PROMPT_LIMIT_TO_PLAN = { | ||
| 600: "Starter", |
There was a problem hiding this comment.
In the CN path, limits are explicitly treated as model-call counts (600/1500/4500), but probe still returns used/limit under format.suffix: "prompts"; when CN is selected this reports values ~15x larger than actual prompt usage (e.g., 600 shown as prompts instead of 40), so quota tracking becomes materially inaccurate for users. Please normalize CN counts before emitting the line or emit a unit that matches model calls.
Useful? React with 👍 / 👎.
P1: Fix incorrect unit display for CN region Problem: - CN API returns model call counts (e.g., 4500 for Max plan) - Plugin displayed these as "prompts" without conversion - Users saw 15x inflated values (4500 instead of 300 prompts) - Quota tracking was materially inaccurate Solution: - Detect CN endpoint in probe() - Convert CN values: model_calls / 15 = prompts - GLOBAL values remain unchanged (already in prompts) Examples: - CN Max: 4500 model_calls → 300 prompts ✓ - CN Plus: 1500 model_calls → 100 prompts ✓ - CN Starter: 600 model_calls → 40 prompts ✓ Tests updated to reflect correct conversion.
There was a problem hiding this comment.
Pull request overview
Adds automatic MiniMax CN vs GLOBAL endpoint selection to improve usability (especially for CN users) by probing region-specific endpoints and annotating output with the chosen region.
Changes:
- Add CN/GLOBAL region auto-detection with region-specific endpoint + API-key precedence and cross-region fallback.
- Adjust usage/plan parsing to support CN model-call counts (÷15) and append region to the displayed plan name.
- Update Tauri env allowlist, docs, and plugin tests for the new CN variable and behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src-tauri/src/plugin_engine/host_api.rs |
Allowlists MINIMAX_CN_API_KEY so the plugin can read it in the Tauri runtime. |
plugins/minimax/plugin.js |
Implements region probing, CN endpoints, CN plan inference, and CN count conversion + region plan labels. |
plugins/minimax/plugin.test.js |
Adds/updates tests for auto-selection, fallback behavior, CN plan inference, and auth error handling. |
docs/providers/minimax.md |
Documents region auto-selection, key lookup order, and CN endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
plugins/minimax/plugin.js
Outdated
| // GLOBAL plan tiers (based on model call counts) | ||
| const GLOBAL_PROMPT_LIMIT_TO_PLAN = { | ||
| 100: "Starter", | ||
| 300: "Plus", | ||
| 1000: "Max", | ||
| 2000: "Ultra", | ||
| } |
There was a problem hiding this comment.
The comment says the GLOBAL plan tiers are “based on model call counts”, but the mapping keys (100/300/1000/2000) are prompt limits (the GLOBAL API is treated as prompts elsewhere in this plugin). Please correct the comment to avoid confusion when maintaining the tier inference logic.
plugins/minimax/plugin.js
Outdated
| throw "Session expired. Check your MiniMax API key." | ||
| if (!parsed) { | ||
| if (lastError) throw lastError | ||
| throw "MiniMax API key missing. Set MINIMAX_API_KEY." |
There was a problem hiding this comment.
The missing-credentials error message only tells users to set MINIMAX_API_KEY, but this plugin now also supports MINIMAX_CN_API_KEY (and will attempt both regions). Update the message to mention the valid env var options (at least MINIMAX_API_KEY and MINIMAX_CN_API_KEY) so CN users aren’t misled.
| throw "MiniMax API key missing. Set MINIMAX_API_KEY." | |
| throw "MiniMax API key missing. Set MINIMAX_API_KEY or MINIMAX_CN_API_KEY." |
| If no key is found after attempting both regions, it throws: | ||
|
|
||
| - `MiniMax API key missing. Set MINIMAX_API_KEY.` | ||
|
|
There was a problem hiding this comment.
Docs say the plugin throws MiniMax API key missing. Set MINIMAX_API_KEY., but the plugin now supports MINIMAX_CN_API_KEY as well. Once the runtime error message is updated, please update this doc snippet to list the supported env vars (e.g., MINIMAX_API_KEY / MINIMAX_CN_API_KEY) so it matches actual configuration options.
| const line = { | ||
| label: "Session", | ||
| used: parsed.used, | ||
| limit: parsed.total, | ||
| used: Math.round(parsed.used * displayMultiplier), | ||
| limit: Math.round(parsed.total * displayMultiplier), | ||
| format: { kind: "count", suffix: "prompts" }, | ||
| } | ||
| if (parsed.resetsAt) line.resetsAt = parsed.resetsAt | ||
| if (parsed.periodDurationMs !== null) line.periodDurationMs = parsed.periodDurationMs | ||
|
|
||
| const result = { lines: [ctx.line.progress(line)] } | ||
| if (parsed.planName) result.plan = parsed.planName | ||
| if (parsed.planName) { | ||
| const regionLabel = successfulEndpoint === "CN" ? " (CN)" : " (GLOBAL)" | ||
| result.plan = parsed.planName + regionLabel | ||
| } |
There was a problem hiding this comment.
PR description states the selected region will be shown in the Session label (e.g., Session (CN) / Session (GLOBAL)), but the implementation keeps label: "Session" and instead appends the region to result.plan. Please either update the PR description to match the implemented behavior or change the output label to include the region as described.
|
Am fixing the remaining issues now and then preparing it for release. Thanks all! |
…rence - Updated error message to indicate both MINIMAX_API_KEY and MINIMAX_CN_API_KEY are required. - Clarified plan tier inference logic for CN mode to only use exact known limits. - Adjusted documentation to reflect changes in API key requirements and plan tier inference. - Added tests to ensure correct error handling and plan inference behavior.
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.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - Treat `current_interval_usage_count` as remaining prompts (MiniMax remains API behavior). | ||
| - If only remaining aliases are provided, compute `used = total - remaining`. | ||
| - If explicit used-count fields are provided, prefer them. | ||
| - Plan name is taken from explicit plan/title fields when available. | ||
| - If plan fields are missing, infer plan tier from known limits (`100/300/1000/2000` prompts or `1500/4500/15000/30000` model-call equivalents). | ||
| - If plan fields are missing in GLOBAL mode, infer plan tier from known limits (`100/300/1000/2000` prompts or `1500/4500/15000/30000` model-call equivalents). | ||
| - If plan fields are missing in CN mode, infer only exact known CN limits (`600/1500/4500` model-call counts). |
There was a problem hiding this comment.
In CN mode, current_interval_total_count/current_interval_usage_count are treated as model-call counts and later converted to prompts (÷15). This bullet currently says current_interval_usage_count is remaining prompts, which is misleading for the CN endpoint—consider rewording to “remaining count (same unit as total), converted to prompts for display” or calling out the CN model-call unit explicitly.
| { | ||
| model_name: "MiniMax-M2", | ||
| current_interval_total_count: 600, // 40 prompts × 15 | ||
| current_interval_usage_count: 500, // Remaining (not used!) |
There was a problem hiding this comment.
The inline comment // Remaining (not used!) is inaccurate here: current_interval_usage_count is used (as the inferred remaining count) to compute used = total - remaining for this test. Please update or remove the comment to avoid confusion.
| current_interval_usage_count: 500, // Remaining (not used!) | |
| current_interval_usage_count: 500, // Remaining |
| const line = { | ||
| label: "Session", | ||
| used: parsed.used, | ||
| limit: parsed.total, | ||
| used: Math.round(parsed.used * displayMultiplier), | ||
| limit: Math.round(parsed.total * displayMultiplier), | ||
| format: { kind: "count", suffix: "prompts" }, |
There was a problem hiding this comment.
PR description mentions a visible session label like Session (CN) / Session (GLOBAL), but this code keeps the progress line label as Session and instead adds the region suffix to result.plan. Either update the session line label to include the region, or adjust the PR description to match the implemented behavior.
Description
Adds MiniMax region auto-detection and improves CN usability without manual endpoint config.
MINIMAX_CN_API_KEY: triesCN -> GLOBALMINIMAX_CN_API_KEY: triesGLOBAL -> CNMINIMAX_CN_API_KEYfirst).Session (CN)/Session (GLOBAL).Related Issue
N/A
Type of Change
Testing
bun run buildand it succeededbun run testand all tests passbun tauri devAdditional validation:
bun run test -- plugins/minimax/plugin.test.js(pass)bun run test:coveragewas executed; suite passed, but repo-level coverage thresholds currently fail in pre-existing unrelated files.Screenshots
Verified locally with screenshot showing MiniMax
Session (CN)and live usage values.Checklist
mainbranchSummary by cubic
Auto-detects MiniMax CN vs GLOBAL and removes manual endpoint config. Converts CN model-call counts to prompts, adds a region suffix to the plan, and keeps the Session label stable.
New Features
Bug Fixes
Written for commit 8805c34. Summary will update on new commits.