fix(providers): minimax connection issue, and print out api_base while configuring - #378
fix(providers): minimax connection issue, and print out api_base while configuring#378KevinHuSh wants to merge 7 commits into
Conversation
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: MiniMax endpoint selection is inconsistent between regions and between preflight and real requests; see the inline note.
I reviewed the complete three-file diff, all test_provider and _verify_provider callers, the runtime provider-construction path, relevant blame/history, and backward compatibility for both MiniMax regions. I also checked AGENTS.md/CLAUDE.md, CONTEXT-MAP.md and its Provider Endpoint terminology, architecture boundaries, and whether tests were weakened (no tests were changed). Lint-enforced metadata issues were left out of scope as instructed.
Verification: uv run pytest tests/test_config_update_providers.py tests/test_cli_onboard_commands.py tests/test_provider_catalog.py tests/test_provider_auth_method.py -x passed with 447 passed and 3 skipped. The full uv run pytest reached 6,889 passed, 43 skipped, and 13 deselected, with 4 failures and 2 errors before exiting 139; the failures were in unchanged theme/TUI paths, including file-descriptor exhaustion. Rerunning those six cases in isolation gave 5 passed and the unchanged test_bold_accent_renders_styled_not_bare assertion failure, again followed by exit 139.
| # Needed by `provider test` and the wizard preflight, which probe | ||
| # /v1/models before any LiteLLM call resolves an endpoint. | ||
| default_api_base="https://api.minimax.io/v1", | ||
| default_api_base="https://api.minimaxi.com/v1", |
There was a problem hiding this comment.
Blocker: this changes only the /models preflight default. The runtime construction path calls Config.get_api_base, which intentionally does not return defaults for direct providers, and this spec has no MINIMAX_API_BASE env extra, so LiteLLMProvider.api_base remains None. LiteLLM then sends the subsequent test message and real chats to its international default, https://api.minimax.io/v1; a China key can pass this probe and still fail the real request. Conversely, minimax is the sole unqualified API-key entry, and MiniMax's regional setup documentation says international users use api.minimax.io while China users use api.minimaxi.com, so replacing the shared default sends international preflights to the wrong region. Please make the API-key region selection explicit or account-aware and propagate the selected base into the actual provider, with tests for both regional flows.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: this revision still has region-routing errors and adds a debug option that cannot produce its advertised output; see the inline notes.
I re-reviewed the delta from 73069a35c555 and the complete PR diff. I checked the previous endpoint blocker, registry and wire identities, environment setup, ProviderPool, runtime make_provider/onboarding/doctor callers, the benchmark consumer, relevant history, and backward compatibility across both MiniMax regions. I also checked AGENTS.md/CLAUDE.md, CONTEXT-MAP.md and its Provider Endpoint contract, architecture boundaries, and test changes; no tests were weakened, but no tests were added or updated for this revision. Lint-enforced metadata remains out of scope.
Verification: uv run pytest tests/test_config_update_providers.py tests/test_cli_onboard_commands.py tests/test_provider_catalog.py tests/test_provider_auth_method.py tests/test_provider_resolution_invariants.py tests/test_provider_wire_model.py tests/test_cli_doctor_commands.py tests/test_cli_helpers.py finished with 819 passed, 35 skipped, and 5 failed. The failures are the curated MiniMax ordering assertion, both pinned provider-inventory assertions, and both wire-baseline coverage assertions for the new minimaxi provider.
| display_name="MiniMax CN", | ||
| via_driver="minimax", | ||
| skip_prefixes=("minimax/", "minimaxi/", "openrouter/"), | ||
| env_extras=(("MINIMAX_API_BASE", "{api_base}"),), |
There was a problem hiding this comment.
Blocker: this carries the CN address through process-global state, not through the provider instance. _setup_env applies env_extras with os.environ.setdefault, while LiteLLMProvider.api_base remains None. In a process with both bindings (which ProviderPool supports), constructing CN first makes a later international MiniMax provider read the CN base; conversely, a pre-existing international MINIMAX_API_BASE prevents the CN binding from selecting its own base. I reproduced both cases with MinimaxChatConfig.get_api_base. Please pass the selected regional base per provider/request rather than using shared ambient state.
| "--skip-test", | ||
| help="Skip the one-shot test message (avoids a billed call; connectivity is still checked)", | ||
| ), | ||
| debug_http: bool = typer.Option( |
There was a problem hiding this comment.
Blocker: this option only sets RAVEN_DEBUG_HTTP, but neither Raven nor the installed LiteLLM code reads that variable. Consequently --debug-http follows the ordinary probe path and never prints the advertised redacted request; the new failure hint recommending RAVEN_DEBUG_HTTP=1 raven doctor --probe is equally ineffective. Please connect this flag to an actual redacted HTTP diagnostic or remove the option and hint.
| via_driver="anthropic", | ||
| skip_prefixes=("anthropic/",), | ||
| default_api_base="https://api.minimax.io/anthropic/v1", | ||
| default_api_base="https://api.minimaxi.com/anthropic/v1", |
There was a problem hiding this comment.
Blocker: this is the minimax_global OAuth spec, but the change replaces its international .io default with the mainland-China .com host. The dedicated OAuth runtime normally gets a resource URL from its token, but benchmarks/pinchbench/direct/raven_executor.py::_make_benchmark_provider consumes spec.default_api_base directly; it now builds minimax_global with https://api.minimaxi.com/anthropic/v1 and routes that Global request to the wrong region. Please restore the international default here.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the existing process-global MiniMax endpoint blocker remains unresolved, and the relevant contract tests still have five failures.
The delta since the immediately preceding revision only removes the unused os import, so it does not change the substantive verdict. The earlier no-op debug-option and Global OAuth endpoint findings are resolved; I found no new independent issue worth raising and am not duplicating the surviving inline finding. I rechecked the current full diff, endpoint propagation through LiteLLMProvider and ProviderPool, onboarding/provider callers, registry and wire contracts, relevant history, backward compatibility, AGENTS.md/CLAUDE.md, CONTEXT-MAP.md's Provider Endpoint terminology, architecture boundaries, and whether tests were weakened (none were changed). Lint-enforced issues remain out of scope.
Verification on this head: uv run pytest tests/test_cli_onboard_commands.py tests/test_provider_catalog.py tests/test_provider_wire_model.py finished with 404 passed, 4 skipped, and 5 failed. The failures are the curated MiniMax ordering assertion, both pinned provider-inventory assertions, and both wire-baseline coverage assertions for minimaxi.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the existing process-global MiniMax endpoint issue remains unresolved, and this revision introduces a consistently failing help test; see the inline note.
The new inventory, ordering, endpoint-display, and wire-baseline updates resolve the five contract failures from the previous revision. However, the new regional test deletes MINIMAX_API_BASE between provider constructions, so it does not exercise or refute the existing multi-binding and pre-existing-environment failure modes. I rechecked the delta and full diff, LiteLLMProvider/ProviderPool endpoint propagation, onboarding/provider callers, registry and wire contracts, relevant history, backward compatibility, AGENTS.md/CLAUDE.md, CONTEXT-MAP.md's Provider Endpoint terminology, architecture boundaries, and whether existing tests were weakened. Lint-enforced issues remain out of scope.
Verification: uv run pytest tests/test_config_update_providers.py tests/test_cli_onboard_commands.py tests/test_provider_catalog.py tests/test_provider_auth_method.py tests/test_provider_resolution_invariants.py tests/test_provider_wire_model.py tests/test_cli_doctor_commands.py tests/test_cli_helpers.py finished with 826 passed, 35 skipped, and 1 failed. The sole failure is test_onboard_help_lists_all_flags, which still expects the removed --debug-http flag.
| "--non-interactive", | ||
| "--yes", | ||
| "--reset", | ||
| "--debug-http", |
There was a problem hiding this comment.
Blocker: --debug-http was removed from the command in the preceding revision, but this new assertion still requires it. test_onboard_help_lists_all_flags therefore fails on every run (missing flag in help: --debug-http). Please remove this stale expectation rather than restoring the nonfunctional option.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the existing process-global MiniMax endpoint defect remains unresolved, although the affected contract suite is now green.
This one-line delta correctly removes the stale --debug-http help expectation, resolving the only new finding from the previous revision. I found no new independent issue and am not duplicating the surviving endpoint-locality finding. The added regional test still deletes MINIMAX_API_BASE between provider constructions, so it does not exercise the multi-binding or pre-existing-environment cases described in that thread. I rechecked the current full diff, LiteLLMProvider/ProviderPool endpoint propagation, onboarding/provider callers, registry and wire contracts, relevant history, backward compatibility, AGENTS.md/CLAUDE.md, CONTEXT-MAP.md's Provider Endpoint terminology, architecture boundaries, and whether tests were weakened. Lint-enforced issues remain out of scope.
Verification: uv run pytest tests/test_config_update_providers.py tests/test_cli_onboard_commands.py tests/test_provider_catalog.py tests/test_provider_auth_method.py tests/test_provider_resolution_invariants.py tests/test_provider_wire_model.py tests/test_cli_doctor_commands.py tests/test_cli_helpers.py passed with 827 passed and 35 skipped.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the existing process-global MiniMax endpoint-locality defect remains unresolved; this formatting-only delta does not affect it.
I reviewed the full main diff and the delta from the previously reviewed head, relevant construction/configuration callers and history, backward compatibility for the two MiniMax regions, whether tests were weakened, and the repository rules in AGENTS.md, CLAUDE.md, CONTEXT-MAP.md, and CONTEXT.md. The stale --debug-http expectation is gone, and I found no new finding in this revision. A direct reproduction still leaves both provider instances with api_base=None while the CN URL persists in shared MINIMAX_API_BASE, so the existing inline blocker still applies.
Verification: uv run pytest tests/test_config_update_providers.py tests/test_cli_onboard_commands.py tests/test_provider_catalog.py tests/test_provider_auth_method.py tests/test_provider_resolution_invariants.py tests/test_provider_wire_model.py tests/test_cli_doctor_commands.py tests/test_cli_helpers.py (827 passed, 35 skipped). git diff --check github/main...HEAD also passed.
e4ccfe4 to
2af3bef
Compare
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: this history-only rewrite has the exact same tree as the previously blocked revision, so the existing MiniMax endpoint-locality blocker still applies.
I rechecked the full main diff, the rewritten history, relevant construction/configuration callers, regional backward compatibility, test changes, and the repository rules in AGENTS.md, CLAUDE.md, CONTEXT-MAP.md, and CONTEXT.md. The prior debug-option, OAuth-host, and stale-test issues remain fixed, and there is no new inline finding. The provider-locality reproduction is unchanged.
Verification: uv run pytest tests/test_config_update_providers.py tests/test_cli_onboard_commands.py tests/test_provider_catalog.py tests/test_provider_auth_method.py tests/test_provider_resolution_invariants.py tests/test_provider_wire_model.py tests/test_cli_doctor_commands.py tests/test_cli_helpers.py (827 passed, 35 skipped); git diff --check github/main...HEAD passed.
Summary
I get the API key of Minimax from page: https://platform.minimaxi.com/console/plan
But I've got errors while adding it.
After fixing, it shows no error.
Before:

After:

Type
Verification
Risk