feat(providers): add Zhipu AI GLM provider (open.bigmodel.cn) - #536
feat(providers): add Zhipu AI GLM provider (open.bigmodel.cn)#536Lucinegogo wants to merge 2 commits into
Conversation
|
✅ Target branch corrected This pull request now targets The |
📝 WalkthroughWalkthroughAdds the ChangesGLM provider
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@tests/provider-registry-parity.test.ts`:
- Line 35: Extend the provider parity test beyond checking that "glm" is listed:
retrieve the GLM entry from the expected provider registry and assert its
baseUrl, adapter/auth configuration, default model, liveModels, and thinking
mappings match the contract defined in the GLM registry entry. Keep the
assertions focused near the existing provider-list coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ae8abb9b-a3b6-4171-a18e-c743096d4e8b
📒 Files selected for processing (2)
src/providers/registry.tstests/provider-registry-parity.test.ts
| "anthropic-apikey", "openai-apikey", "umans", "opencode-go", "neuralwatt", "openrouter", "orcarouter", "bizrouter", "groq", "google", "google-vertex", "azure-openai", | ||
| "deepseek", "cerebras", "together", "fireworks", "firepass", "moonshot", | ||
| "huggingface", "nvidia", "venice", "zai", "nanogpt", "synthetic", "siliconflow", "qwen-cloud", "tencent-coding-plan", | ||
| "huggingface", "nvidia", "venice", "zai", "glm", "nanogpt", "synthetic", "siliconflow", "qwen-cloud", "tencent-coding-plan", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Cover the GLM provider contract, not only its ID.
This test only proves that "glm" appears in the expected key-provider list. It will not catch an incorrect baseUrl, adapter/auth configuration, default model, liveModels, or thinking mappings in src/providers/registry.ts Lines 795-808. Add a focused assertion that retrieves the glm entry and verifies those fields.
As per path instructions, a behavior change in src/ should come with a focused regression test near the existing subsystem tests.
🤖 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 `@tests/provider-registry-parity.test.ts` at line 35, Extend the provider
parity test beyond checking that "glm" is listed: retrieve the GLM entry from
the expected provider registry and assert its baseUrl, adapter/auth
configuration, default model, liveModels, and thinking mappings match the
contract defined in the GLM registry entry. Keep the assertions focused near the
existing provider-list coverage.
Source: Path instructions
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26e518407f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| authKind: "key", | ||
| dashboardUrl: "https://bigmodel.cn/console/usercenter/apikeys", | ||
| defaultModel: "glm-4.6", | ||
| models: ["glm-4.6", "glm-4.5", "glm-4-plus", "glm-4-air", "glm-4-airx", "glm-4-flash", "glm-4-flashx", "glm-4-long", "glm-4v"], |
There was a problem hiding this comment.
When glm-4v is selected, the catalog publishes it as text-only because this provider supplies no modelInputModalities entry and the generic catalog fallback is ["text"]; live /models parsing does not infer image capability either. Codex therefore blocks image attachments before they can reach this vision model. Add explicit glm-4v: ["text", "image"] capability metadata.
Useful? React with 👍 / 👎.
| thinkingToggleModels: ["glm-4.6"], | ||
| modelReasoningEfforts: { "glm-4.6": THINKING_TOGGLE_EFFORTS }, | ||
| modelReasoningEffortMap: { "glm-4.6": THINKING_TOGGLE_MAP }, | ||
| preserveReasoningContentModels: ["glm-4.6"], |
There was a problem hiding this comment.
The test change only adds glm to the expected provider-id list, so it never exercises the newly declared thinking-toggle or reasoning-history behavior. Add a focused test that routes glm/glm-4.6 and verifies effort mapping produces thinking: { type: "enabled" | "disabled" } rather than reasoning_effort, plus the intended reasoning_content replay; otherwise the central behavior introduced here can regress while the parity test remains green.
AGENTS.md reference: AGENTS.md:L75-L77
Useful? React with 👍 / 👎.
| // coding-plan subscription. Live model discovery is enabled so newly published GLM ids | ||
| // surface automatically without a registry edit. Evidence: docs.bigmodel.cn/api-reference. | ||
| { | ||
| id: "glm", |
There was a problem hiding this comment.
glm provider name
When upgrading an existing config with a user-defined provider named glm, routedProviderConfig now recognizes this registry id and, because allowBaseUrlOverride is not enabled, silently replaces the saved endpoint with open.bigmodel.cn. The repository already assigns glm in FREE_PROVIDER_DIRECTORY to Z.AI at https://api.z.ai/api/coding/paas/v4, so a config created from that identity will suddenly send requests and its API key to a different host. Use an unused id such as glm-cn/zhipu-bigmodel, or add an explicit migration that does not retarget existing custom entries.
AGENTS.md reference: AGENTS.md:L65-L70
Useful? React with 👍 / 👎.
| adapter: "openai-chat", | ||
| authKind: "key", | ||
| dashboardUrl: "https://bigmodel.cn/console/usercenter/apikeys", | ||
| defaultModel: "glm-4.6", |
There was a problem hiding this comment.
The new default model has neither a modelContextWindows entry nor a metadata bundle alias, so whenever live discovery fails or returns the ordinary OpenAI model-list shape without context_length, catalog normalization advertises only the generic 128,000-token fallback. The repository's generated GLM metadata records glm-4.6 at 204,800 tokens, meaning Codex compacts or truncates roughly 76,800 tokens early for this preset. Add the explicit context window or connect this provider to the appropriate GLM metadata bundle.
Useful? React with 👍 / 👎.
| id: "glm", | ||
| label: "Zhipu AI — GLM", | ||
| baseUrl: "https://open.bigmodel.cn/api/paas/v4", |
There was a problem hiding this comment.
This adds a user-selectable provider with a distinct endpoint and billing product, but the public provider catalog still documents only the Z.AI coding-plan endpoint and never explains the new glm route. Update the English provider guide and its translated tables so users can distinguish the domestic pay-as-you-go preset from zai and configure the intended credential and route.
AGENTS.md reference: AGENTS.md:L78-L79
Useful? React with 👍 / 👎.
|
Closing the current revision. The domestic BigModel preset can be useful, but this head is not safe to merge.
Please retarget to |
|
Reopening this, and I've retargeted it to To be concrete about what I changed on the PR itself: GitHub refuses a base change on a closed PR, so the order was reopen first, then retarget Why I'm folding a maintainer contribution on top
The one release blocker, with the exact mechanismThe const baseUrl = (registryBaseUrlIsTemplate || registryEntry.allowBaseUrlOverride) && userBaseUrlIsResolved
? userBaseUrl
: registryEntry.baseUrl;Your entry sets neither
Worth saying plainly: our tests would not have caught this. The isolation test at The rest of the corrections
What I can't do without youVerifying Thanks for this. The routing analysis in your description was accurate and the thinking-toggle citation saved me a lookup; the collision was a repository sharp edge, and it's getting sanded down because you hit it. |
|
Landed on
Two things worth reporting back: The new guard fails on the original shape. I mutated the id back to
Verification: I'm leaving this PR open rather than closing it — it's the record of where the design came from, and I'd rather it read as your contribution with maintainer follow-through than as a closed PR with a separate commit somewhere else. |
Adds focused coverage for zhipu-bigmodel: the provider contract, the thinking-toggle request shaping (both the enabled and disabled halves, built through the real adapter), and the derived key-login/preset/bundle surfaces. Also generalizes the directory isolation rule. The existing guard only checked directory rows with supportLevel "reference", so re-registering a CONNECTABLE id against a different host stayed green — which is why #536 could propose `glm` for open.bigmodel.cn while `glm` was already bound to api.z.ai. The new case fails on exactly that shape. Mutation-verified: reverting the id to `glm` fails all four new tests plus the generalized guard, and removing thinkingToggleModels fails only the request-shaping test. Co-authored-by: Lucinegogo <103441383+Lucinegogo@users.noreply.github.com>
|
Heads up, and thanks for the contribution — the BigModel provider has landed on Two things came out of that review and both changed the shape of the final version: The The model list needed refreshing. This PR pins Closing this one as superseded rather than merging it, since the two would now conflict in the registry. The idea and the endpoint research were yours and they carried straight through — if anything in the landed version looks wrong to you, say so and I will fix it. |
|
Shipped — this is now on
I'm closing this PR as superseded, not rejected. The branch here still carries the original One thing I'd genuinely like your help with: I left Thanks for pushing on this. The |
Summary
Adds Zhipu AI — GLM (
open.bigmodel.cn/api/paas/v4) as a built-in key-auth provider, so OpenAI Codex CLI / Claude Code can route through GLM via opencodex.This complements the existing
zaientry, which targets the z.ai international coding-plan subscription. The newglmentry targets Zhipu's domestic OpenAI-compatible pay-as-you-go endpoint — same GLM model family, different host and billing.Changes
src/providers/registry.ts: append aglmentry toPROVIDER_REGISTRY(afterzai).baseUrl:https://open.bigmodel.cn/api/paas/v4adapter:openai-chat,authKind: "key"(Bearer JWT)defaultModel:glm-4.6liveModels: trueso newly published GLM ids surface in the picker without a registry editthinkingToggleModels: ["glm-4.6"]+ reuse of the sharedTHINKING_TOGGLE_EFFORTS/THINKING_TOGGLE_MAP, since GLM-4.6 exposes the binarythinking: {type: "enabled" | "disabled"}knobtests/provider-registry-parity.test.ts: append"glm"toEXPECTED_KEY_PROVIDER_IDSso thekey-login export is derived from the registryparity test stays green.No other surfaces need changes — CLI, GUI, docs-site provider pickers are all derived from
PROVIDER_REGISTRY.Evidence
thinking: {type: "enabled" | "disabled"}): https://docs.bigmodel.cn/cn/guide/models/text/glm-4.6 and https://docs.z.ai/guides/capabilities/thinking-modeTest plan
bun test tests/provider-registry-parity.test.ts— passes withglminEXPECTED_KEY_PROVIDER_IDSocx provider add glm --api-key <zhipu-jwt>registers the providerocx startthen selecting Zhipu AI — GLM in Codex CLI routes a chat completion tohttps://open.bigmodel.cn/api/paas/v4/chat/completionssuccessfullyreasoning_effort: "high"selected in Codex, the upstream request body carriesthinking: {type: "enabled"}and returns reasoning content🤖 Generated with Claude Code
Summary by CodeRabbit