Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/types/src/provider-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ const sambaNovaSchema = apiModelIdProviderModelSchema.extend({
const zaiSchema = apiModelIdProviderModelSchema.extend({
zaiApiKey: z.string().optional(),
zaiApiLine: z.union([z.literal("china"), z.literal("international")]).optional(),
zaiUseGlmCodingPlan: z.boolean().optional(),
})

const fireworksSchema = apiModelIdProviderModelSchema.extend({
Expand Down
24 changes: 24 additions & 0 deletions src/api/providers/__tests__/zai.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ describe("ZAiHandler", () => {
expect(OpenAI).toHaveBeenCalledWith(expect.objectContaining({ baseURL: "https://api.z.ai/api/paas/v4" }))
})

it("should use the international GLM Coding Plan URL when toggle is enabled", () => {
new ZAiHandler({ zaiApiKey: "test-zai-api-key", zaiApiLine: "international", zaiUseGlmCodingPlan: true })
expect(OpenAI).toHaveBeenCalledWith(
expect.objectContaining({ baseURL: "https://api.z.ai/api/coding/paas/v4" }),
)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good test coverage for the basic functionality! Consider adding edge case tests:

  • What happens when switching regions while GLM Coding Plan is enabled?
  • How does it behave with missing API key but toggle enabled?
  • Could we test the actual API error responses when plan limits are hit?

})

it("should use the provided API key for international", () => {
const zaiApiKey = "test-zai-api-key"
new ZAiHandler({ zaiApiKey, zaiApiLine: "international" })
Expand Down Expand Up @@ -81,6 +88,13 @@ describe("ZAiHandler", () => {
)
})

it("should use the China GLM Coding Plan URL when toggle is enabled", () => {
new ZAiHandler({ zaiApiKey: "test-zai-api-key", zaiApiLine: "china", zaiUseGlmCodingPlan: true })
expect(OpenAI).toHaveBeenCalledWith(
expect.objectContaining({ baseURL: "https://open.bigmodel.cn/api/coding/paas/v4" }),
)
})

it("should use the provided API key for China", () => {
const zaiApiKey = "test-zai-api-key"
new ZAiHandler({ zaiApiKey, zaiApiLine: "china" })
Expand Down Expand Up @@ -120,6 +134,16 @@ describe("ZAiHandler", () => {
new ZAiHandler({ zaiApiLine: "international" })
expect(OpenAI).toHaveBeenCalledWith(expect.objectContaining({ apiKey: "not-provided" }))
})

it("should use standard URL when GLM Coding Plan toggle is false", () => {
new ZAiHandler({ zaiApiKey: "test-zai-api-key", zaiApiLine: "international", zaiUseGlmCodingPlan: false })
expect(OpenAI).toHaveBeenCalledWith(expect.objectContaining({ baseURL: "https://api.z.ai/api/paas/v4" }))
})

it("should use standard URL when GLM Coding Plan toggle is undefined", () => {
new ZAiHandler({ zaiApiKey: "test-zai-api-key", zaiApiLine: "international" })
expect(OpenAI).toHaveBeenCalledWith(expect.objectContaining({ baseURL: "https://api.z.ai/api/paas/v4" }))
})
})

describe("API Methods", () => {
Expand Down
12 changes: 11 additions & 1 deletion src/api/providers/zai.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,20 @@ export class ZAiHandler extends BaseOpenAiCompatibleProvider<InternationalZAiMod
const models = isChina ? mainlandZAiModels : internationalZAiModels
const defaultModelId = isChina ? mainlandZAiDefaultModelId : internationalZAiDefaultModelId

// Determine the base URL based on region and GLM Coding Plan toggle
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a comment here explaining the endpoint routing logic for future maintainability:

let baseURL: string
if (options.zaiUseGlmCodingPlan) {
// Use coding plan endpoints
baseURL = isChina ? "https://open.bigmodel.cn/api/coding/paas/v4" : "https://api.z.ai/api/coding/paas/v4"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation looks clean, but I notice there's no error handling specific to GLM Coding Plan failures. The issue mentions showing "a brief notice explaining the situation" when plan limits are reached.

Is this intentional? Should we consider adding a try-catch that could detect plan limit errors and provide user feedback?

} else {
// Use standard endpoints
baseURL = isChina ? "https://open.bigmodel.cn/api/paas/v4" : "https://api.z.ai/api/paas/v4"
}

super({
...options,
providerName: "Z AI",
baseURL: isChina ? "https://open.bigmodel.cn/api/paas/v4" : "https://api.z.ai/api/paas/v4",
baseURL,
apiKey: options.zaiApiKey ?? "not-provided",
defaultProviderModelId: defaultModelId,
providerModels: models,
Expand Down
12 changes: 11 additions & 1 deletion webview-ui/src/components/settings/providers/ZAi.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useCallback } from "react"
import { VSCodeTextField, VSCodeDropdown, VSCodeOption } from "@vscode/webview-ui-toolkit/react"
import { VSCodeTextField, VSCodeDropdown, VSCodeOption, VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react"

import type { ProviderSettings } from "@roo-code/types"

Expand Down Expand Up @@ -71,6 +71,16 @@ export const ZAi = ({ apiConfiguration, setApiConfigurationField }: ZAiProps) =>
</VSCodeButtonLink>
)}
</div>
<div>
<VSCodeCheckbox
checked={apiConfiguration?.zaiUseGlmCodingPlan || false}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we validate that the API key is configured before allowing the GLM Coding Plan toggle to be enabled? Users might enable this without having set up their API key first.

Consider disabling the checkbox when or showing a warning message.

onChange={(e: any) => setApiConfigurationField("zaiUseGlmCodingPlan", e.target.checked)}>
{t("settings:providers.zaiUseGlmCodingPlan")}
</VSCodeCheckbox>
<div className="text-xs text-vscode-descriptionForeground mt-1">
{t("settings:providers.zaiUseGlmCodingPlanDescription")}
</div>
</div>
</>
)
}
2 changes: 2 additions & 0 deletions webview-ui/src/i18n/locales/en/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,8 @@
"getZaiApiKey": "Get Z AI API Key",
"zaiEntrypoint": "Z AI Entrypoint",
"zaiEntrypointDescription": "Please select the appropriate API entrypoint based on your location. If you are in China, choose open.bigmodel.cn. Otherwise, choose api.z.ai.",
"zaiUseGlmCodingPlan": "Use GLM Coding Plan",
"zaiUseGlmCodingPlanDescription": "Route requests through GLM Coding Plan endpoints to use your plan instead of API credits. When plan limits are reached, disable this toggle to continue with API credits.",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical issue: You've only added the new translation keys to the English locale. All other 17 supported languages are missing and , which will cause the UI to display missing translation keys for non-English users.

Could we add placeholder translations or at least copy the English text to other locales to prevent broken UI?

"geminiApiKey": "Gemini API Key",
"getGroqApiKey": "Get Groq API Key",
"groqApiKey": "Groq API Key",
Expand Down
Loading