-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: add SiliconCloud provider support #8552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add SiliconCloud provider implementation based on OpenAI-compatible API - Support multiple API endpoints (China, China-overseas, International) - Add model definitions with region-specific pricing - Integrate with UI settings and validation - Add i18n translations for SiliconCloud settings Closes #8550
- Extend BaseOpenAiCompatibleProvider for consistency - Fix type safety issues - Add proper imports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conducting self-review: the robot critiques the robot, and both nod in deterministic disappointment.
| const temperature = this.options.modelTemperature ?? SILICONCLOUD_DEFAULT_TEMPERATURE | ||
|
|
||
| // Build the request parameters | ||
| const params: OpenAI.Chat.Completions.ChatCompletionCreateParamsStreaming = { | ||
| model, | ||
| max_tokens, | ||
| temperature, | ||
| messages: [{ role: "system", content: systemPrompt }, ...convertToOpenAiMessages(messages)], | ||
| stream: true, | ||
| stream_options: { include_usage: true }, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Max tokens override from settings is ignored. Respect modelMaxTokens when present so the ThinkingBudget UI actually applies.
suggestion
| const temperature = this.options.modelTemperature ?? SILICONCLOUD_DEFAULT_TEMPERATURE | |
| // Build the request parameters | |
| const params: OpenAI.Chat.Completions.ChatCompletionCreateParamsStreaming = { | |
| model, | |
| max_tokens, | |
| temperature, | |
| messages: [{ role: "system", content: systemPrompt }, ...convertToOpenAiMessages(messages)], | |
| stream: true, | |
| stream_options: { include_usage: true }, | |
| } | |
| const temperature = this.options.modelTemperature ?? SILICONCLOUD_DEFAULT_TEMPERATURE | |
| const resolved_max_tokens = this.options.modelMaxTokens ?? max_tokens | |
| // Build the request parameters | |
| const params: OpenAI.Chat.Completions.ChatCompletionCreateParamsStreaming = { | |
| model, | |
| max_tokens: resolved_max_tokens, | |
| temperature, | |
| messages: [{ role: "system", content: systemPrompt }, ...convertToOpenAiMessages(messages)], | |
| stream: true, | |
| stream_options: { include_usage: true }, | |
| } |
| if (supportsReasoningBudget) { | ||
| // SiliconCloud uses different parameter names than OpenAI | ||
| ;(params as any).enable_thinking = true | ||
| // Default thinking budget could be added here if needed | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] SiliconCloud reasoning params: you set enable_thinking but never send thinking_budget and ignore enableReasoningEffort. This breaks budget control for reasoning-capable models.
suggestion
| if (supportsReasoningBudget) { | |
| // SiliconCloud uses different parameter names than OpenAI | |
| ;(params as any).enable_thinking = true | |
| // Default thinking budget could be added here if needed | |
| } | |
| if (supportsReasoningBudget && (this.options.enableReasoningEffort ?? true)) { | |
| // SiliconCloud uses different parameter names than OpenAI | |
| ;(params as any).enable_thinking = true | |
| const budget = this.options.modelMaxThinkingTokens ?? Math.floor(0.8 * ((params as any).max_tokens || 8192)) | |
| ;(params as any).thinking_budget = budget | |
| } |
| // Export all models for the default list | ||
| export const siliconCloudModels = siliconCloudChinaModels | ||
|
|
||
| export type SiliconCloudModelId = keyof typeof siliconCloudModels |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Type-safety/UX: SiliconCloudModelId only includes China models, but you expose different model sets per apiLine. Unify the default list so static selectors and types cover all regions.
suggestion
| // Export all models for the default list | |
| export const siliconCloudModels = siliconCloudChinaModels | |
| export type SiliconCloudModelId = keyof typeof siliconCloudModels | |
| // Export all models for the default list (union across regions) | |
| export const siliconCloudModels = { | |
| ...siliconCloudChinaModels, | |
| ...siliconCloudChinaOverseasModels, | |
| ...siliconCloudInternationalModels, | |
| } | |
| export type SiliconCloudModelId = keyof typeof siliconCloudModels |
| export { Featherless } from "./Featherless" | ||
| export { VercelAiGateway } from "./VercelAiGateway" | ||
| export { DeepInfra } from "./DeepInfra" | ||
| export { SiliconCloud } from "./SiliconCloud" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P0] UI integration gap: SiliconCloud is exported here but not wired into the Settings UI. Without adding it to PROVIDERS and MODELS_BY_PROVIDER (webview-ui/src/components/settings/constants.ts) and rendering in ApiOptions (webview-ui/src/components/settings/ApiOptions.tsx), users cannot select or configure this provider. Please add:
- constants.ts: include { value: "siliconcloud", label: "SiliconCloud" } in PROVIDERS and map siliconcloud -> siliconCloudModels in MODELS_BY_PROVIDER
- ApiOptions.tsx: render when selectedProvider === "siliconcloud"
Related GitHub Issue
Closes: #8550
Description
This PR implements a new SiliconCloud provider based on the OpenAI compatible provider, addressing the issues mentioned in #8550.
Key features include:
enable_thinkingandthinking_budget)Implementation Details
Test Procedure
Manual testing steps:
Notes
Pre-Submission Checklist
Important
Adds support for SiliconCloud provider with API handler, model definitions, UI components, and validation logic.
SiliconCloudHandlerinsrc/api/providers/siliconcloud.tsto handle SiliconCloud API interactions.enable_thinking,thinking_budget).packages/types/src/providers/siliconcloud.tswith region-specific pricing and context window information.webview-ui/src/components/ui/hooks/useSelectedModel.ts.SiliconCloudcomponent inwebview-ui/src/components/settings/providers/SiliconCloud.tsxfor UI settings.webview-ui/src/i18n/locales/en/settings.jsonfor SiliconCloud settings.validateApiConfigurationinwebview-ui/src/utils/validate.tsto include SiliconCloud-specific validation.ProfileValidatorinsrc/shared/ProfileValidator.ts.provider-settings.tsto include SiliconCloud in provider schemas and settings.This description was created by
for fc8f478. You can customize this summary. It will automatically update as commits are pushed.