feat: add ModelScope API support #2773
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds comprehensive support for a new ModelScope provider across the application stack, including icon rendering, provider type registration, utility classes for model discovery, default model configurations, factory integration, and data migration logic. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Areas of focus:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/renderer/components/icons/ProviderIcon.tsx (1)
109-115: ModelScope icon wiring looks good; consider camelCaseclipRuleThe new
ModelScopebranch is correctly wired and will render forprovider === ModelProviderEnum.ModelScope. To stay consistent with the rest of this file and React’s SVG props, you might want to useclipRuleinstead ofclip-rule:- <path clip-rule="evenodd" d="M21.333 13.3v-2.667h-2.666V7.967H16V5.3h5.333v2.667H24V13.3h-2.667zm0-2.667H23.5V8.467h-2.167v2.166z"></path> + <path clipRule="evenodd" d="M21.333 13.3v-2.667h-2.666V7.967H16V5.3h5.333v2.667H24V13.3h-2.667zm0-2.667H23.5V8.467h-2.167v2.166z"></path>src/shared/models/index.ts (1)
32-33: ModelScope model wiring is consistent with other OpenAI‑style providers
- The new
ModelProviderEnum.ModelScopecase ingetModelmirrors other OpenAI‑compatible providers and forwards the expected knobs (model,temperature,topP,maxOutputTokens,stream) intoModelScope, which itself supplies the fixed host.aiProviderNameHashandAIModelProviderMenuOptionListentries ensure ModelScope appears correctly in the provider selector UI.If you ever decide to make the ModelScope host configurable, you’ll only need to thread
formattedApiHostinto theModelScopeconstructor; the surrounding plumbing is already in place.Also applies to: 81-99, 116-318, 401-421, 423-515
src/shared/defaults.ts (1)
824-906: SystemProviders entry for ModelScope matches existing provider patternsThe new ModelScope provider definition (id/type/urls/defaultSettings/apiHost/models) is structurally aligned with the other OpenAI‑compatible providers, and its host matches the one used in
ModelScopeitself, so consumers ofSystemProvidersshould just work. If the host ever needs to change, consider centralizing it in a shared constant to avoid drift.src/renderer/packages/model-setting-utils/modelscope-setting-util.ts (1)
17-21: Avoid non-null assertion onsettings.apiKeyand validate explicitlyRelying on
settings.apiKey!here assumes upstream always provides a non-empty key. If configuration is ever incomplete (e.g., user hasn’t entered a key yet), this can result in confusing runtime behavior at the model client layer.Consider validating the API key up front and removing the non-null assertion:
- protected async listProviderModels(settings: ProviderSettings) { - const dependencies = await createModelDependencies() - const modelscope = new ModelScope({ apiKey: settings.apiKey!, model: { modelId: '', capabilities: [] } }, dependencies) - return modelscope.listModels() - } + protected async listProviderModels(settings: ProviderSettings) { + if (!settings.apiKey) { + throw new Error('ModelScope API key is required to list models') + } + + const dependencies = await createModelDependencies() + const modelscope = new ModelScope( + { apiKey: settings.apiKey, model: { modelId: '', capabilities: [] } }, + dependencies, + ) + return modelscope.listModels() + }This makes failure modes clearer and keeps the type-safety aligned with runtime behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/renderer/static/icons/providers/modelscope.pngis excluded by!**/*.png
📒 Files selected for processing (9)
src/renderer/components/icons/ProviderIcon.tsx(1 hunks)src/renderer/packages/model-setting-utils/index.ts(2 hunks)src/renderer/packages/model-setting-utils/modelscope-setting-util.ts(1 hunks)src/renderer/stores/migration.ts(3 hunks)src/shared/defaults.ts(1 hunks)src/shared/models/index.ts(4 hunks)src/shared/models/modelscope.ts(1 hunks)src/shared/types/provider.ts(1 hunks)src/shared/utils/llm_utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/shared/utils/llm_utils.ts (1)
src/shared/types/session.ts (1)
ModelProviderEnum(7-7)
src/shared/models/modelscope.ts (2)
src/shared/models/openai-compatible.ts (1)
OpenAICompatibleSettings(10-19)src/shared/types/adapters.ts (1)
ModelDependencies(27-32)
🔇 Additional comments (6)
src/shared/utils/llm_utils.ts (1)
123-140: ModelScope correctly marked as OpenAI‑compatibleIncluding
ModelProviderEnum.ModelScopein the allowlist keeps ModelScope on the shared OpenAI‑compatible path alongside the other proxy-style providers; this is aligned with the newModelScopemodel class and looks good.src/shared/types/provider.ts (1)
4-24: ModelProviderEnum extension for ModelScope is consistentThe new
ModelScope = 'modelscope'enum member fits cleanly with existing usage (defaults, models, migrations) and doesn’t alter any existing behavior.src/renderer/packages/model-setting-utils/index.ts (1)
31-32: ModelScopeSettingUtil registration is correctRegistering
ModelProviderEnum.ModelScope→ModelScopeSettingUtilin the hash cleanly plugs the new provider into the existing settings resolution flow without affecting customs or other providers.Also applies to: 37-56
src/shared/models/modelscope.ts (1)
1-28: ModelScope OpenAI‑compatible wrapper looks soundThe
ModelScopeclass correctly extendsOpenAICompatible, injects the fixed ModelScope host, and forwards all the relevant settings (apiKey,model,temperature,topP,maxOutputTokens,stream) to the base class while keeping a typedoptionssnapshot. This should behave like the other OpenAI‑compatible providers with a fixed endpoint.src/renderer/stores/migration.ts (1)
400-482: ModelScope settings and session migration logic is aligned with existing providersThe added handling in
migrate_9_to_10formodelscopeApiKey/modelscopeModelfollows the same pattern as other providers: it seedsproviders[ModelScope]from legacy settings and maps sessionmodelIdvia the newModelProviderEnum.ModelScope: 'modelscopeModel'entry. This should preserve existing ModelScope usage across upgrades without affecting other providers.Also applies to: 488-593, 639-666
src/renderer/packages/model-setting-utils/modelscope-setting-util.ts (1)
7-15: Display name logic is clear and robustThe display name composition is straightforward and safely falls back to the raw
modelwhen there is no configured nickname. This is a good fit for UI display and handles missingproviderSettingsgracefully.
1 participant
Description
Hello,
I have added the ModelScope provider, as well as some well-known models.
Added svg to ProviderIcon.tsx
I also added a png image for the provider, with a white background in modelscope.png
Created modelscope-setting-util.ts
Added models to default.ts
Additional Notes
This PR primarily adds support for the ModelScope API. Users can now utilize models supported by ModelScope API inference by setting an API key.
How to obtain an API key
Note: Please bind your Alibaba Cloud account before using this feature.
Screenshots
Contributor Agreement
By submitting this Pull Request, I confirm that I have read and agree to the following terms:
Please check the box below to confirm:
[x] I have read and agree with the above statement.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.