feat(providers): split minimax zai into intl cn#398
Conversation
- Add minimaxi-cn (api.minimaxi.com) sharing model catalog with minimax - Add zai-cn (open.bigmodel.cn) splitting zai from china to intl - Decouple env vars: ZAI_API_KEY vs ZHIPU_API_KEY for zai/zai-cn, MINIMAX_API_KEY vs MINIMAXI_API_KEY for minimax/minimaxi-cn - Map zai-cn and minimaxi-cn to models.dev providers "zai" and "minimax" so the model catalog hydrates for both endpoints
WalkthroughThis PR adds provider catalog entries for MiniMax China (minimaxi-cn) and splits Z.ai into international (zai) and China (zai-cn) descriptors with distinct base URLs and API key env vars. Curated model lists are shared via new slices, provider ID mapping is updated, and tests cover resolver defaults, catalog ordering, and CN/international parity. ChangesMiniMax CN and Zai CN Provider Support
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
internal/config/resolver_test.go (2)
1146-1146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale comment: default model isn't glm-5.2.
This comment says
zainow resolves to "the glm-5.2 default," but the assertion two lines below (and the catalog descriptor) both useglm-4.5, with a separate comment clarifying glm-5.2 only comes via live discovery. The two comments contradict each other.📝 Suggested fix
- // "zai" now resolves to the international endpoint and the glm-5.2 default. + // "zai" now resolves to the international endpoint; the catalog default model + // stays glm-4.5, with live discovery upgrading to glm-5.2 at runtime.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/config/resolver_test.go` at line 1146, The comment in the resolver test is stale and contradicts the actual `zai` expectation used by the nearby assertion and catalog descriptor. Update the inline comment near the `zai` test case so it matches the current default model (`glm-4.5`) and reserve any mention of `glm-5.2` for the separate live-discovery comment, keeping the wording consistent with the test setup in `resolver_test.go`.
1145-1174: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
ProviderKind/APIKeyEnv/APIKeyassertions.Unlike its two sibling tests, this one never asserts
resolved.Provider.ProviderKind,APIKeyEnv, orAPIKey— despite the test being specifically aboutZAI_API_KEYrouting correctly, which is the core behavior this PR changes. As written, this test would still pass even ifZAI_API_KEYresolution silently broke.✅ Suggested additional assertions
if resolved.Provider.Model != "glm-4.5" { t.Fatalf("Model = %q, want glm-4.5 (preserved default; live discovery upgrades to glm-5.2 at runtime)", resolved.Provider.Model) } + if resolved.Provider.ProviderKind != ProviderKindOpenAICompatible { + t.Fatalf("ProviderKind = %q, want %q", resolved.Provider.ProviderKind, ProviderKindOpenAICompatible) + } + if resolved.Provider.APIKeyEnv != "ZAI_API_KEY" { + t.Fatalf("APIKeyEnv = %q, want ZAI_API_KEY", resolved.Provider.APIKeyEnv) + } + if resolved.Provider.APIKey != "sk-zai-intl" { + t.Fatalf("APIKey = %q, want env-resolved secret", resolved.Provider.APIKey) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/config/resolver_test.go` around lines 1145 - 1174, Add the missing assertions in TestResolveAppliesZaiInternationalCatalogDefaults so it also verifies the Z.ai auth routing, not just BaseURL and Model. After Resolve returns, assert resolved.Provider.ProviderKind, resolved.Provider.APIKeyEnv, and resolved.Provider.APIKey match the expected zai-intl values, using the existing Provider fields in the test alongside the current CatalogID checks to confirm ZAI_API_KEY is wired correctly.internal/providercatalog/catalog.go (1)
137-138: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winFlag the base URL flip for existing
zaiconfigs.Existing configs with
catalog_id: "zai"and no explicitbase_urlpreviously resolved to the China endpoint; after this change they'll silently resolve toapi.z.aiwith a different required env var (ZAI_API_KEYvs the old key). Anyone with a barezaiconfig and only a Zhipu key set will start getting auth failures with no clear signal as to why. Worth a migration note (README/CHANGELOG) or a one-time warning whenZHIPU_API_KEYis set butZAI_API_KEYisn't, givencatalog_id=zai.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/providercatalog/catalog.go` around lines 137 - 138, The default endpoint for the existing "zai" catalog_id is changing from the China base URL to api.z.ai, which can break bare "zai" configs that only have ZHIPU_API_KEY set. Update the catalog handling in internal/providercatalog/catalog.go around openAICompat for "zai" and "zai-cn" so the transition is explicit, and add either a migration note or a one-time warning when catalog_id is "zai" with ZHIPU_API_KEY present but ZAI_API_KEY missing. Make sure the warning points users to the new required env var and clarifies the endpoint selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/config/resolver_test.go`:
- Line 1146: The comment in the resolver test is stale and contradicts the
actual `zai` expectation used by the nearby assertion and catalog descriptor.
Update the inline comment near the `zai` test case so it matches the current
default model (`glm-4.5`) and reserve any mention of `glm-5.2` for the separate
live-discovery comment, keeping the wording consistent with the test setup in
`resolver_test.go`.
- Around line 1145-1174: Add the missing assertions in
TestResolveAppliesZaiInternationalCatalogDefaults so it also verifies the Z.ai
auth routing, not just BaseURL and Model. After Resolve returns, assert
resolved.Provider.ProviderKind, resolved.Provider.APIKeyEnv, and
resolved.Provider.APIKey match the expected zai-intl values, using the existing
Provider fields in the test alongside the current CatalogID checks to confirm
ZAI_API_KEY is wired correctly.
In `@internal/providercatalog/catalog.go`:
- Around line 137-138: The default endpoint for the existing "zai" catalog_id is
changing from the China base URL to api.z.ai, which can break bare "zai" configs
that only have ZHIPU_API_KEY set. Update the catalog handling in
internal/providercatalog/catalog.go around openAICompat for "zai" and "zai-cn"
so the transition is explicit, and add either a migration note or a one-time
warning when catalog_id is "zai" with ZHIPU_API_KEY present but ZAI_API_KEY
missing. Make sure the warning points users to the new required env var and
clarifies the endpoint selection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 07fbd2ae-56cd-4f06-a96a-0c9e3cd1de87
📒 Files selected for processing (7)
internal/config/resolver_test.gointernal/providercatalog/catalog.gointernal/providercatalog/catalog_test.gointernal/providermodelcatalog/catalog.gointernal/providermodelcatalog/catalog_test.gointernal/providermodelcatalog/remote.gointernal/providermodelcatalog/remote_test.go
kevincodex1
left a comment
kevincodex1
left a comment
There was a problem hiding this comment.
looks great to me. thnnks!
2 participants
Summary
minimaxi-cn(api.minimaxi.com) sharing the model catalog withminimaxzai-cn;zainow points at the international endpoint api.z.aiZAI_API_KEYvsZHIPU_API_KEYfor zai/zai-cn,MINIMAX_API_KEYvsMINIMAXI_API_KEYfor minimax/minimaxi-cnWhy
zaiwas hard-coded to the China endpointopen.bigmodel.cn. There was no built-in way to pick Z.ai's international host api.z.ai.ZAI_API_KEY/ZHIPU_API_KEYas their env vars, so a key set for one side was silently picked up by the other. If the user had separate accounts (e.g. personal overseas vs. company China), the wrong key would 401.minimax/minimaxi-cnwas already split this way for the same reasons; this PR bringszai/zai-cnto the same shape.Test
go test ./internal/providercatalog/... ./internal/providermodelcatalog/... ./internal/config/...All three packages pass. TUI smoke-tested: the picker shows the same model list (sourced from models.dev) for both endpoints, and the env vars stay scoped to their respective side.