feat(minimax): expose MiniMax-M2.7 in Token Plan model picker - #9429
feat(minimax): expose MiniMax-M2.7 in Token Plan model picker#9429octo-patch wants to merge 1 commit into
Conversation
The MiniMax Token Plan provider fetches its model list dynamically from /v1/models, which returns an empty list when no API key is configured or the request fails. This left the current MiniMax-M2.7 model unlisted in the model picker. Return a small fallback list of current model IDs (MiniMax-M3, MiniMax-M2.7) when the dynamic fetch is unavailable; the dynamic list remains authoritative whenever it succeeds.
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
get_models, consider catchinghttpx.RequestError(or a similarly scoped exception) instead of a bareExceptionto avoid masking unrelated errors while still handling network/API issues. - The fallback model list is duplicated as string literals in the tests; importing and reusing
_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS(or a public alias) would reduce the risk of drift if the fallback list changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `get_models`, consider catching `httpx.RequestError` (or a similarly scoped exception) instead of a bare `Exception` to avoid masking unrelated errors while still handling network/API issues.
- The fallback model list is duplicated as string literals in the tests; importing and reusing `_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS` (or a public alias) would reduce the risk of drift if the fallback list changes.
## Individual Comments
### Comment 1
<location path="astrbot/core/provider/sources/minimax_token_plan_source.py" line_range="12" />
<code_context>
+# dynamic /v1/models fetch is unavailable (no API key configured yet or a
+# transient network error) so the model picker can still surface the current
+# models. The dynamic list remains authoritative whenever it succeeds.
+_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS = ["MiniMax-M3", "MiniMax-M2.7"]
+
</code_context>
<issue_to_address>
**suggestion:** Consider freezing the fallback model IDs to avoid accidental runtime mutation.
Defining this as a tuple (or another immutable sequence) would better signal that it’s a constant and remove the need to return `.copy()` elsewhere, e.g.:
```python
_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS = ("MiniMax-M3", "MiniMax-M2.7")
```
Callers can then construct a list if needed: `return list(_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS)` (or use the tuple directly if a generic sequence is sufficient).
Suggested implementation:
```python
# Current MiniMax Token Plan model IDs. Returned as a fallback when the
# dynamic /v1/models fetch is unavailable (no API key configured yet or a
# transient network error) so the model picker can still surface the current
# models. The dynamic list remains authoritative whenever it succeeds.
_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS = ("MiniMax-M3", "MiniMax-M2.7")
```
Search for all usages of `_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS` in the codebase and:
1. If any code currently calls `_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS.copy()`, replace it with `list(_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS)`.
2. If callers mutate the sequence (e.g. `.append`, `.extend`, item assignment), change them to operate on a list copy instead, for example:
- Before: `models = _MINIMAX_TOKEN_PLAN_FALLBACK_MODELS.copy(); models.append("...")`
- After: `models = list(_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS); models.append("...")`
3. If callers only iterate or treat it as a generic sequence, no change is needed; they can use the tuple directly.
</issue_to_address>
### Comment 2
<location path="astrbot/core/provider/sources/minimax_token_plan_source.py" line_range="62-64" />
<code_context>
- return []
+ return _MINIMAX_TOKEN_PLAN_FALLBACK_MODELS.copy()
try:
async with httpx.AsyncClient() as client:
resp = await client.get(
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Lack of explicit timeout on the HTTP client can lead to hanging calls.
Using `httpx.AsyncClient()` without an explicit timeout risks tasks hanging if the MiniMax endpoint is slow or unresponsive. Please set a sensible timeout (or reuse a shared client with a central timeout config) so these calls can fail fast instead of blocking indefinitely.
```suggestion
try:
# Explicit timeout to avoid hanging calls if MiniMax endpoint is slow/unresponsive.
# Adjust the timeout value if you need stricter or more lenient behavior.
async with httpx.AsyncClient(timeout=10.0) as client:
resp = await client.get(
)
resp.raise_for_status()
data = resp.json()
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # dynamic /v1/models fetch is unavailable (no API key configured yet or a | ||
| # transient network error) so the model picker can still surface the current | ||
| # models. The dynamic list remains authoritative whenever it succeeds. | ||
| _MINIMAX_TOKEN_PLAN_FALLBACK_MODELS = ["MiniMax-M3", "MiniMax-M2.7"] |
There was a problem hiding this comment.
suggestion: Consider freezing the fallback model IDs to avoid accidental runtime mutation.
Defining this as a tuple (or another immutable sequence) would better signal that it’s a constant and remove the need to return .copy() elsewhere, e.g.:
_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS = ("MiniMax-M3", "MiniMax-M2.7")Callers can then construct a list if needed: return list(_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS) (or use the tuple directly if a generic sequence is sufficient).
Suggested implementation:
# Current MiniMax Token Plan model IDs. Returned as a fallback when the
# dynamic /v1/models fetch is unavailable (no API key configured yet or a
# transient network error) so the model picker can still surface the current
# models. The dynamic list remains authoritative whenever it succeeds.
_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS = ("MiniMax-M3", "MiniMax-M2.7")Search for all usages of _MINIMAX_TOKEN_PLAN_FALLBACK_MODELS in the codebase and:
- If any code currently calls
_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS.copy(), replace it withlist(_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS). - If callers mutate the sequence (e.g.
.append,.extend, item assignment), change them to operate on a list copy instead, for example:- Before:
models = _MINIMAX_TOKEN_PLAN_FALLBACK_MODELS.copy(); models.append("...") - After:
models = list(_MINIMAX_TOKEN_PLAN_FALLBACK_MODELS); models.append("...")
- Before:
- If callers only iterate or treat it as a generic sequence, no change is needed; they can use the tuple directly.
| try: | ||
| async with httpx.AsyncClient() as client: | ||
| resp = await client.get( |
There was a problem hiding this comment.
suggestion (bug_risk): Lack of explicit timeout on the HTTP client can lead to hanging calls.
Using httpx.AsyncClient() without an explicit timeout risks tasks hanging if the MiniMax endpoint is slow or unresponsive. Please set a sensible timeout (or reuse a shared client with a central timeout config) so these calls can fail fast instead of blocking indefinitely.
| try: | |
| async with httpx.AsyncClient() as client: | |
| resp = await client.get( | |
| try: | |
| # Explicit timeout to avoid hanging calls if MiniMax endpoint is slow/unresponsive. | |
| # Adjust the timeout value if you need stricter or more lenient behavior. | |
| async with httpx.AsyncClient(timeout=10.0) as client: | |
| resp = await client.get( | |
| ) | |
| resp.raise_for_status() | |
| data = resp.json() |
Reason: The MiniMax Token Plan provider did not surface MiniMax-M2.7 because its model list is fetched dynamically and is empty when no API key is configured or the request fails.
Changes:
MiniMax-M3,MiniMax-M2.7) returned byget_models()only when the dynamic/v1/modelsfetch is unavailable (no API key, an empty response, or a network/API error)./v1/modelsrequest succeeds, so newly released models are still discovered without a code change.Checks:
ruff formatandruff checkon the changed files.pytest tests/test_minimax_token_plan_source.py(4 passed).pytest tests/test_openai_source.py tests/test_anthropic_kimi_code_provider.py(90 passed, no regression).Summary by Sourcery
Ensure MiniMax Token Plan model picker always surfaces current models by falling back to a small static list when dynamic model discovery fails or is unavailable.
New Features:
Tests: