fix(ai): add MiniMax regional endpoint metadata - #498
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous Review Summary (commit 99e99f0)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 99e99f0)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Reviewed by ling-3.0-flash:free · Input: 336.6K · Output: 27.4K · Cached: 2.2M |
debba
left a comment
There was a problem hiding this comment.
Thanks for this. I checked out the branch, ran the tests and probed the endpoints directly, and the model-list part works as advertised: both /v1/models URLs are live and return 401 on a wrong-region key, so the fallback loop does what it should. A CN key that previously got nothing will now populate the model dropdown. ai::tests passes on the branch and cargo check is clean.
The problem is that the fix stops halfway. generate_minimax (line ~988) still posts to a hardcoded https://api.minimax.io/v1/chat/completions, so a mainland-China key now sees its models listed but every actual generation fails with a 401. That's arguably worse than before, because the feature looks configured and then breaks the moment you use it. Could you route the chat call through the same regional endpoints? Ideally remember which region the models call succeeded on rather than retrying both on every request.
Two smaller things, both of which probably resolve themselves once the above is done:
regionandanthropic_base_urlare never read at runtime. The only warning keeping them "alive" is suppressed byderive(Debug). If the chat call starts using the endpoint table, fine; otherwise they should go.test_minimax_endpoint_metadatajust re-asserts the constants' own values, so it can't catch any regression that matters. Once the endpoints are actually exercised bygenerate_minimax, a test around the fallback selection would pull more weight.
Requesting changes for the generate_minimax gap, since "mainland China API keys are supported" is the stated goal of the PR and generation is the part users actually notice.
|
@octo-patch are you able to check them? |
|
Thanks for the review. I routed generate_minimax through the regional endpoint table, made model discovery and generation remember the last successful region while retaining fallback, and replaced the unused metadata assertion with endpoint-order coverage. The repository test job and automated code review both pass on the new commit. |
Summary
cn_zhregionsTesting
git diff --checkcargois not installed