feat(providers): add Z.ai provider#242
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 48 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (24)
📝 WalkthroughWalkthroughAdds Z.ai provider support: new internal provider implementation, provider registration in startup, config/example and Helm schema/templates updates, environment variable auto-discovery/tests, and documentation updates across README, docs, and OpenAPI metadata. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway as "GoModel Gateway"
participant Provider as "internal/providers/zai"
participant ZAI as "Z.ai API"
Client->>Gateway: HTTP request (Chat/Embeddings/Responses)
Gateway->>Provider: Adapt request to core.ChatRequest / EmbeddingRequest
Provider->>ZAI: HTTP POST to /chat/completions or /embeddings\nHeader: Authorization: Bearer <apiKey>
ZAI-->>Provider: 200 JSON response
Provider-->>Gateway: core formatted response
Gateway-->>Client: HTTP response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@helm/templates/_helpers.tpl`:
- Around line 112-126: Update the chart documentation to explicitly state that
when using providers.existingSecret the per-provider flag enabled: true is
required (in addition to Values.providers.existingSecret) to emit the
<NAME>_API_KEY environment variable; reference the template logic in
helm/templates/_helpers.tpl (symbols: providers.existingSecret, apiKey, enabled,
and the generated env var name pattern {{ upper $name }}_API_KEY) and add the
same note to the provider block in values.yaml so consumers understand the new
behavior contract.
In `@internal/providers/zai/zai.go`:
- Around line 45-53: NewWithHTTPClient currently ignores caller-supplied base
URL causing the Provider to temporarily use defaultBaseURL; update the function
signature to accept a baseURL string (e.g., NewWithHTTPClient(apiKey string,
baseURL string, httpClient *http.Client, hooks llmclient.Hooks) ) and inside use
baseURL if non-empty else defaultBaseURL, then pass that value into
openai.NewCompatibleProviderWithHTTPClient via CompatibleProviderConfig.BaseURL
so the created Provider (and its compatible field) is constructed with the
correct base URL; reference NewWithHTTPClient, Provider, compatible,
openai.NewCompatibleProviderWithHTTPClient, CompatibleProviderConfig,
defaultBaseURL and setHeaders when making the change.
- Around line 19-86: The literal "zai" is repeated; add a package-level const
providerName = "zai" and replace all direct uses of the string with that
constant: the registration Type value (currently Type: "zai"), both
CompatibleProviderConfig.ProviderName fields in New and NewWithHTTPClient, and
the "zai" argument passed to providers.StreamResponsesViaChat in
StreamResponses; ensure all references (ProviderName and the
StreamResponsesViaChat call) use providerName so the canonical provider name is
centralized.
- Around line 89-92: The Embeddings method on Provider is incorrectly
hard-disabled; update it to delegate to the compatibility layer like
ChatCompletion and ListModels by calling p.compatible.Embeddings(ctx, req) and
returning its result instead of returning core.NewInvalidRequestError, and
remove the corresponding TestEmbeddings_ReturnsUnsupportedError test; locate the
Embeddings method in internal/providers/zai/zai.go and the unit test named
TestEmbeddings_ReturnsUnsupportedError to apply these changes.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e74c3c27-a3df-4725-b014-e698654db7e7
📒 Files selected for processing (22)
.env.templateCLAUDE.mdGETTING_STARTED.mdREADME.mdcmd/gomodel/docs/docs.gocmd/gomodel/main.goconfig/config.example.yamlconfig/config_test.godocs/adr/0001-explicit-provider-registration.mddocs/advanced/configuration.mdxdocs/features/passthrough-api.mdxdocs/getting-started/quickstart.mdxdocs/openapi.jsonhelm/Chart.yamlhelm/README.mdhelm/templates/NOTES.txthelm/templates/_helpers.tplhelm/values.schema.jsonhelm/values.yamlinternal/providers/config_test.gointernal/providers/zai/zai.gointernal/providers/zai/zai_test.go
| Type: "zai", | ||
| New: New, | ||
| Discovery: providers.DiscoveryConfig{ | ||
| DefaultBaseURL: defaultBaseURL, | ||
| }, | ||
| } | ||
|
|
||
| // Provider implements the core.Provider interface for Z.ai. | ||
| type Provider struct { | ||
| compatible *openai.CompatibleProvider | ||
| } | ||
|
|
||
| // New creates a new Z.ai provider. | ||
| func New(cfg providers.ProviderConfig, opts providers.ProviderOptions) core.Provider { | ||
| baseURL := providers.ResolveBaseURL(cfg.BaseURL, defaultBaseURL) | ||
| return &Provider{ | ||
| compatible: openai.NewCompatibleProvider(cfg.APIKey, opts, openai.CompatibleProviderConfig{ | ||
| ProviderName: "zai", | ||
| BaseURL: baseURL, | ||
| SetHeaders: setHeaders, | ||
| }), | ||
| } | ||
| } | ||
|
|
||
| // NewWithHTTPClient creates a new Z.ai provider with a custom HTTP client. | ||
| // If httpClient is nil, http.DefaultClient is used. | ||
| func NewWithHTTPClient(apiKey string, httpClient *http.Client, hooks llmclient.Hooks) *Provider { | ||
| return &Provider{ | ||
| compatible: openai.NewCompatibleProviderWithHTTPClient(apiKey, httpClient, hooks, openai.CompatibleProviderConfig{ | ||
| ProviderName: "zai", | ||
| BaseURL: defaultBaseURL, | ||
| SetHeaders: setHeaders, | ||
| }), | ||
| } | ||
| } | ||
|
|
||
| // SetBaseURL allows configuring a custom base URL for the provider. | ||
| func (p *Provider) SetBaseURL(url string) { | ||
| p.compatible.SetBaseURL(url) | ||
| } | ||
|
|
||
| func setHeaders(req *http.Request, apiKey string) { | ||
| req.Header.Set("Authorization", "Bearer "+apiKey) | ||
| } | ||
|
|
||
| // ChatCompletion sends a chat completion request to Z.ai. | ||
| func (p *Provider) ChatCompletion(ctx context.Context, req *core.ChatRequest) (*core.ChatResponse, error) { | ||
| return p.compatible.ChatCompletion(ctx, req) | ||
| } | ||
|
|
||
| // StreamChatCompletion returns a raw response body for streaming. | ||
| func (p *Provider) StreamChatCompletion(ctx context.Context, req *core.ChatRequest) (io.ReadCloser, error) { | ||
| return p.compatible.StreamChatCompletion(ctx, req) | ||
| } | ||
|
|
||
| // ListModels retrieves the list of available models from Z.ai. | ||
| func (p *Provider) ListModels(ctx context.Context) (*core.ModelsResponse, error) { | ||
| return p.compatible.ListModels(ctx) | ||
| } | ||
|
|
||
| // Responses sends a Responses API request to Z.ai using chat-completions translation. | ||
| func (p *Provider) Responses(ctx context.Context, req *core.ResponsesRequest) (*core.ResponsesResponse, error) { | ||
| return providers.ResponsesViaChat(ctx, p, req) | ||
| } | ||
|
|
||
| // StreamResponses streams a Responses API request to Z.ai using chat-completions translation. | ||
| func (p *Provider) StreamResponses(ctx context.Context, req *core.ResponsesRequest) (io.ReadCloser, error) { | ||
| return providers.StreamResponsesViaChat(ctx, p, req, "zai") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Nit: extract "zai" provider-name literal into a package-level constant.
The string "zai" is repeated four times (Registration.Type, both CompatibleProviderConfig.ProviderName values, and the StreamResponsesViaChat argument). Extracting it into a const providerName = "zai" reduces the risk of drift if the canonical provider name ever changes (e.g., to match Registration.Type being rebranded).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/providers/zai/zai.go` around lines 19 - 86, The literal "zai" is
repeated; add a package-level const providerName = "zai" and replace all direct
uses of the string with that constant: the registration Type value (currently
Type: "zai"), both CompatibleProviderConfig.ProviderName fields in New and
NewWithHTTPClient, and the "zai" argument passed to
providers.StreamResponsesViaChat in StreamResponses; ensure all references
(ProviderName and the StreamResponsesViaChat call) use providerName so the
canonical provider name is centralized.
f16aeb7 to
60da1c2
Compare
d3c776e to
131e675
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Line 7: Update the CLAUDE.md provider inventory to include Azure alongside the
existing providers listed under the "GoModel" description and the provider/env
var sections; add Azure to the provider list (the same place OpenRouter and Z.ai
were added) and document the corresponding environment variables and
configuration keys used for Azure authentication and endpoints so the README
matches the actual supported providers and config. Ensure the Azure entry
mirrors the style/format of the other providers in the list and that any
examples or env var tables include Azure-specific variables.
In `@docs/adr/0001-explicit-provider-registration.md`:
- Line 5: The Consequences section still says "9 explicit registration calls"
but the provider list now includes OpenAI, Anthropic, Gemini, xAI, Groq,
OpenRouter, Z.ai, Azure OpenAI, Oracle, Ollama and custom OpenAI-compatible
endpoints (11 providers); update the numeric statement to "11 explicit
registration calls" (or adjust to match the actual count) so the ADR is
internally consistent, keeping the rest of the Consequences text unchanged.
In `@GETTING_STARTED.md`:
- Line 675: Update the duplicated/contradictory Embeddings statement so the
document is consistent: locate the list item starting "7. **Embeddings**: The
`/v1/embeddings` endpoint is supported by OpenAI, Gemini, Groq, Z.ai, xAI, and
Ollama. Anthropic does not offer embeddings natively." and reconcile it with the
earlier "Supported by" line by either adding Z.ai to that earlier supported-by
list or by removing Z.ai from this item so both statements match; ensure both
occurrences reference the exact same provider set and keep the phrasing
consistent.
In `@helm/templates/_helpers.tpl`:
- Around line 112-126: The base URL env var emission is currently nested under
the same conditional that requires either an API key ($hasAPIKey) or
existingSecret+enabled ($enabledWithExistingSecret), so BASE_URL (checked via
$config.baseUrl and emitted with name: {{ upper $name }}_BASE_URL) won't be
rendered for keyless providers; fix by changing the condition so the BASE_URL
block is emitted whenever $config.baseUrl is set (i.e., move the $config.baseUrl
check out of the $hasAPIKey/$enabledWithExistingSecret branch or OR the baseUrl
condition into that conditional), keeping the secret-related API_KEY block
untouched (references: $config, $hasAPIKey, $enabledWithExistingSecret,
$config.baseUrl, upper $name, $secretName).
In `@internal/providers/config_test.go`:
- Around line 363-381: Add a new test sibling to
TestApplyProviderEnvVars_DiscoversZAIFromAPIKey that sets both ZAI_API_KEY and
ZAI_BASE_URL environment variables, calls applyProviderEnvVars with
testDiscoveryConfigs, then asserts the discovered provider "zai" exists and that
p.APIKey equals the env key, p.Type equals "zai", and crucially that p.BaseURL
equals the explicit ZAI_BASE_URL override (not the default from
testDiscoveryConfigs["zai"].DefaultBaseURL) to guard the explicit-base-URL
precedence path.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5e960438-f6bf-4e7a-ac1b-f9a144c26636
📒 Files selected for processing (22)
.env.templateCLAUDE.mdGETTING_STARTED.mdREADME.mdcmd/gomodel/docs/docs.gocmd/gomodel/main.goconfig/config.example.yamlconfig/config_test.godocs/adr/0001-explicit-provider-registration.mddocs/advanced/configuration.mdxdocs/features/passthrough-api.mdxdocs/getting-started/quickstart.mdxdocs/openapi.jsonhelm/Chart.yamlhelm/README.mdhelm/templates/NOTES.txthelm/templates/_helpers.tplhelm/values.schema.jsonhelm/values.yamlinternal/providers/config_test.gointernal/providers/zai/zai.gointernal/providers/zai/zai_test.go
| {{- if kindIs "map" $config }} | ||
| {{- $hasAPIKey := and (hasKey $config "apiKey") $config.apiKey }} | ||
| {{- $enabledWithExistingSecret := and $.Values.providers.existingSecret (hasKey $config "enabled") $config.enabled }} | ||
| {{- if or $hasAPIKey $enabledWithExistingSecret }} | ||
| - name: {{ upper $name }}_API_KEY | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ $secretName }} | ||
| key: {{ upper $name }}_API_KEY | ||
| {{- if $config.baseUrl }} | ||
| {{- if $config.baseUrl }} | ||
| - name: {{ upper $name }}_BASE_URL | ||
| value: {{ $config.baseUrl | quote }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
Behavior note: <NAME>_BASE_URL now requires either apiKey or existingSecret + enabled.
Previously the base URL env var was emitted whenever baseUrl was set; now it's gated under the same _API_KEY emission condition. This is fine for all current providers (which all need an API key), but worth keeping in mind if a keyless provider like Ollama is ever added to values.yaml — it would need explicit enabled: true with existingSecret, or a dedicated code path, to surface its BASE_URL.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@helm/templates/_helpers.tpl` around lines 112 - 126, The base URL env var
emission is currently nested under the same conditional that requires either an
API key ($hasAPIKey) or existingSecret+enabled ($enabledWithExistingSecret), so
BASE_URL (checked via $config.baseUrl and emitted with name: {{ upper $name
}}_BASE_URL) won't be rendered for keyless providers; fix by changing the
condition so the BASE_URL block is emitted whenever $config.baseUrl is set
(i.e., move the $config.baseUrl check out of the
$hasAPIKey/$enabledWithExistingSecret branch or OR the baseUrl condition into
that conditional), keeping the secret-related API_KEY block untouched
(references: $config, $hasAPIKey, $enabledWithExistingSecret, $config.baseUrl,
upper $name, $secretName).
131e675 to
a952260
Compare
Summary
ZAI_API_KEYand optionalZAI_BASE_URL, including the GLM Coding Plan endpointValidation
go test -count=1 ./internal/providers/zai ./internal/providers ./config ./cmd/gomodeljq empty docs/openapi.jsonjq empty helm/values.schema.jsonhelm template gomodel ./helm --set providers.zai.apiKey=test-key --set providers.zai.baseUrl=https://api.z.ai/api/coding/paas/v4Summary by CodeRabbit
New Features
Documentation
Configuration