feat(provider): add API key providers - #1789
Conversation
📝 WalkthroughWalkthroughEight new built-in API-key providers (NVIDIA, Hugging Face, Moonshot AI, StepFun, Upstage, Alibaba Token Plan global/CN, MiniMax global) are registered across ChangesBasic API Key Providers Batch
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 1
🧹 Nitpick comments (2)
test/main/presenter/llmProviderPresenter/basicApiKeyProviders.test.ts (1)
99-102: ⚡ Quick winReset mock implementations between tests to avoid state leakage.
mockGetProvider.mockReturnValue(...)persists across tests withvi.clearAllMocks(). Use
vi.resetAllMocks()(or explicitly resetmockGetProvider) so each test starts from a clean implementation state.Suggested change
beforeEach(() => { - vi.clearAllMocks() + vi.resetAllMocks() mockRunAiSdkGenerateText.mockResolvedValue({ content: 'ok' }) + mockGetProvider.mockReturnValue(null) })Also applies to: 155-176
🤖 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 `@test/main/presenter/llmProviderPresenter/basicApiKeyProviders.test.ts` around lines 99 - 102, The beforeEach hook uses vi.clearAllMocks() which only clears mock call history but does not reset mock implementations like mockGetProvider.mockReturnValue(...), causing test state to leak between tests. Replace vi.clearAllMocks() with vi.resetAllMocks() in the beforeEach block to ensure both call history and mock implementations are reset between each test, preventing state leakage. This same issue applies to other test blocks in the file as well, so apply the change consistently wherever vi.clearAllMocks() is used in a beforeEach or similar setup block.test/renderer/components/ModelIcon.test.ts (1)
77-102: ⚡ Quick winAdd coverage for
alibaba-token-plan-cnicon mapping.This test covers
alibaba-token-planbut not the newly addedalibaba-token-plan-cnkey. Adding that case would protect the full new mapping surface.🤖 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 `@test/renderer/components/ModelIcon.test.ts` around lines 77 - 102, The test function 'resolves the basic API-key provider icons' is missing coverage for the newly added 'alibaba-token-plan-cn' icon mapping. Add a new mount call for ModelIcon with modelId set to 'alibaba-token-plan-cn' (similar to the existing alibabaTokenPlan mount), and add a corresponding expect assertion to verify that the resolved icon source matches the expected alibabaIcon URL. This ensures the new mapping is properly tested alongside the existing 'alibaba-token-plan' case.
🤖 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 `@docs/features/basic-api-key-providers/plan.md`:
- Around line 29-30: The documentation for the minimax-global provider in the
compatibility notes section contains an incorrect base URL. Update the URL from
`https://api.minimax.io/anthropic` to `https://api.minimax.io/anthropic/v1` to
match the actual implementation used in the config and tests. This will ensure
consistency between the documented and implemented behavior for the
minimax-global provider.
---
Nitpick comments:
In `@test/main/presenter/llmProviderPresenter/basicApiKeyProviders.test.ts`:
- Around line 99-102: The beforeEach hook uses vi.clearAllMocks() which only
clears mock call history but does not reset mock implementations like
mockGetProvider.mockReturnValue(...), causing test state to leak between tests.
Replace vi.clearAllMocks() with vi.resetAllMocks() in the beforeEach block to
ensure both call history and mock implementations are reset between each test,
preventing state leakage. This same issue applies to other test blocks in the
file as well, so apply the change consistently wherever vi.clearAllMocks() is
used in a beforeEach or similar setup block.
In `@test/renderer/components/ModelIcon.test.ts`:
- Around line 77-102: The test function 'resolves the basic API-key provider
icons' is missing coverage for the newly added 'alibaba-token-plan-cn' icon
mapping. Add a new mount call for ModelIcon with modelId set to
'alibaba-token-plan-cn' (similar to the existing alibabaTokenPlan mount), and
add a corresponding expect assertion to verify that the resolved icon source
matches the expected alibabaIcon URL. This ensures the new mapping is properly
tested alongside the existing 'alibaba-token-plan' case.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a32b2b0b-40f6-45e1-9ac7-dd31d38ee62f
📒 Files selected for processing (12)
docs/features/basic-api-key-providers/plan.mddocs/features/basic-api-key-providers/spec.mddocs/features/basic-api-key-providers/tasks.mdsrc/main/presenter/configPresenter/providerId.tssrc/main/presenter/configPresenter/providers.tssrc/main/presenter/llmProviderPresenter/providerRegistry.tssrc/renderer/src/components/icons/ModelIcon.vuesrc/shared/providerDbCatalog.tstest/main/presenter/configPresenter/defaultProviders.test.tstest/main/presenter/llmProviderPresenter/basicApiKeyProviders.test.tstest/main/shared/providerDbCatalog.test.tstest/renderer/components/ModelIcon.test.ts
Summary
UI Layout
BEFORE:
AFTER:
Tests
Summary by CodeRabbit
New Features
Documentation