feat(moonshot): implement provider with model picker, API fetching, streaming, and model metadata#857
Open
grizmin wants to merge 4 commits into
Open
feat(moonshot): implement provider with model picker, API fetching, streaming, and model metadata#857grizmin wants to merge 4 commits into
grizmin wants to merge 4 commits into
Conversation
…reaming - Fix duplicate ModelPicker by adding moonshot to PROVIDERS_WITH_CUSTOM_MODEL_UI - Fix HTTP 404 on Refresh Models by preserving /v1 in fetcher URL - Fix empty model list (consequence of 404 fix) - Fix 'No output generated' error by switching MoonshotHandler from OpenAICompatibleHandler (Vercel AI SDK) to OpenAiHandler (OpenAI Node SDK) which is what Moonshot's own API docs recommend - Add dedicated Moonshot fetcher with proper model ID prefixing - Update tests to match new handler implementation All 6,660 backend and 1,432 webview-ui tests pass.
grizmin
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds Moonshot as a dynamic provider, including model typing, model fetching and caching, handler behavior updates, router-model aggregation, and settings UI support for refreshing and selecting fetched models. ChangesMoonshot dynamic provider integration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
webview-ui/src/components/settings/providers/Moonshot.tsx (1)
1-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMissing local Vitest coverage for new stateful component logic.
This component adds a non-trivial state machine (refresh status, message listener, error/success reconciliation,
ModelPickerwiring) with no accompanying test file in this PR.Do you want me to draft a
Moonshot.spec.tsxcovering the refresh flow, the message-listener success/error paths, and the missing-API-key guard?As per path instructions, "Prefer local
webview-uitests for React/webview behavior such as component rendering, local state, hooks, form dirty-state, validation, or prop wiring. Add or update Vitest coverage underwebview-ui/src/**/__tests__."🤖 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 `@webview-ui/src/components/settings/providers/Moonshot.tsx` around lines 1 - 172, Add local Vitest coverage for Moonshot’s new stateful behavior by creating a test near Moonshot that exercises the refresh flow, the window message listener paths, and the missing API key guard. Cover the `handleRefreshModels` logic, the `useEffect` message handling for both `singleRouterModelFetchResponse` error and `routerModels` success reconciliation, and verify the `ModelPicker`/button wiring in `Moonshot`. Ensure the tests validate that `refreshStatus`, `refreshError`, and the disabled/loading states change as expected.Source: Path instructions
src/core/webview/webviewMessageHandler.ts (1)
1139-1153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the repeated credential-resolution/flush/candidate pattern.
This block duplicates the exact same shape used for
poeanddeepseekjust above it (resolve frommessage.valuesvsapiConfiguration, conditionally flush on explicit override, push a candidate). A small helper would reduce copy/paste risk as more single-apiKey providers are added.♻️ Example helper extraction
+function resolveApiKeyGatedCandidate( + key: RouterName, + apiKeyField: string, + baseUrlField: string, + message: WebviewMessage, + apiConfiguration: ProviderSettings, +) { + const apiKey = (message?.values as any)?.[apiKeyField] ?? (apiConfiguration as any)[apiKeyField] + const baseUrl = (message?.values as any)?.[baseUrlField] ?? (apiConfiguration as any)[baseUrlField] + return { apiKey, baseUrl, hasExplicitOverride: !!(message?.values as any)?.[apiKeyField] || !!(message?.values as any)?.[baseUrlField] } +}🤖 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 `@src/core/webview/webviewMessageHandler.ts` around lines 1139 - 1153, The Moonshot handling in webviewMessageHandler repeats the same credential resolution, optional flush, and candidate push pattern already used for poe and deepseek. Extract that shared logic into a small helper so the provider-specific code only supplies the provider name and credential keys, while the helper handles resolving from message.values vs apiConfiguration, calling flushModels on explicit overrides, and adding the candidate. Keep the Moonshot call site aligned with the existing provider blocks to reduce copy/paste drift as more single-apiKey providers are added.
🤖 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 `@src/api/providers/fetchers/moonshot.ts`:
- Around line 66-87: The fallback in moonshot model fetching is assigning
guessed pricing for unknown IDs, which conflicts with
`MoonshotHandler.resolveModelInfo` treating those same models as unpriced.
Update the unknown-model branch in `src/api/providers/fetchers/moonshot.ts` so
it matches `resolveModelInfo`’s behavior for IDs not in `moonshotModels`,
leaving pricing fields unset/undefined instead of hardcoding values. Keep the
known-model path unchanged and ensure the fallback description still identifies
the model.
In `@webview-ui/src/components/settings/providers/Moonshot.tsx`:
- Around line 35-59: The Moonshot message handler in useEffect is setting error
state too broadly, which can surface errors from unrelated fetches. Update the
singleRouterModelFetchResponse branch to mirror the routerModels success guard
by only calling setRefreshStatus("error") and setRefreshError when refreshStatus
is "loading" and the message is for the moonshot provider. Keep the existing
moonshot providerName check in handleMessage so only refresh-triggered failures
affect the UI.
---
Nitpick comments:
In `@src/core/webview/webviewMessageHandler.ts`:
- Around line 1139-1153: The Moonshot handling in webviewMessageHandler repeats
the same credential resolution, optional flush, and candidate push pattern
already used for poe and deepseek. Extract that shared logic into a small helper
so the provider-specific code only supplies the provider name and credential
keys, while the helper handles resolving from message.values vs
apiConfiguration, calling flushModels on explicit overrides, and adding the
candidate. Keep the Moonshot call site aligned with the existing provider blocks
to reduce copy/paste drift as more single-apiKey providers are added.
In `@webview-ui/src/components/settings/providers/Moonshot.tsx`:
- Around line 1-172: Add local Vitest coverage for Moonshot’s new stateful
behavior by creating a test near Moonshot that exercises the refresh flow, the
window message listener paths, and the missing API key guard. Cover the
`handleRefreshModels` logic, the `useEffect` message handling for both
`singleRouterModelFetchResponse` error and `routerModels` success
reconciliation, and verify the `ModelPicker`/button wiring in `Moonshot`. Ensure
the tests validate that `refreshStatus`, `refreshError`, and the
disabled/loading states change as expected.
🪄 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 Plus
Run ID: 040c25c1-8e98-4429-ac98-112116194e26
📒 Files selected for processing (15)
packages/types/src/provider-settings.tssrc/api/providers/__tests__/moonshot.spec.tssrc/api/providers/fetchers/__tests__/moonshot.spec.tssrc/api/providers/fetchers/modelCache.tssrc/api/providers/fetchers/moonshot.tssrc/api/providers/moonshot.tssrc/core/webview/__tests__/ClineProvider.spec.tssrc/core/webview/__tests__/webviewMessageHandler.routerModels.spec.tssrc/core/webview/__tests__/webviewMessageHandler.spec.tssrc/core/webview/webviewMessageHandler.tssrc/shared/api.tswebview-ui/src/components/settings/providers/Moonshot.tsxwebview-ui/src/components/settings/utils/providerModelConfig.tswebview-ui/src/components/ui/hooks/useSelectedModel.tswebview-ui/src/utils/__tests__/validate.spec.ts
… docs Add kimi-k2.7-code, kimi-k2.7-code-highspeed, and kimi-k2.6 to static models map with accurate pricing from Moonshot documentation. Also fix kimi-k2.5 to correctly mark supportsImages: true.
grizmin
changed the title
github-actions
Bot
added
the
awaiting-review
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement the Moonshot provider from scratch, enabling full model selection, API-based model fetching, and working streaming responses.
What was missing
The previous Moonshot implementation was non-functional:
MoonshotHandlerextendedOpenAICompatibleHandler(Vercel AI SDK) which has compatibility issues with the Moonshot APIChanges
Handler refactor (
src/api/providers/moonshot.ts)MoonshotHandlerfromOpenAICompatibleHandler(Vercel AI SDK) toOpenAiHandler(OpenAI Node SDK)getModel(),processUsageMetrics(),addMaxTokensIfNeeded()Model fetching (
src/api/providers/fetchers/moonshot.ts)getMoonshotModels()fetcher that callsGET /v1/modelsModel picker UI (
webview-ui/src/components/settings/providers/Moonshot.tsx)Model metadata (
packages/types/src/providers/moonshot.ts)kimi-k2.7-code,kimi-k2.7-code-highspeed,kimi-k2.6with correct pricing from official docskimi-k2.5to correctly marksupportsImages: trueInfrastructure
moonshottodynamicProviderslist (enables model fetching pipeline)modelCache.ts)requestRouterModelsTesting
Summary by CodeRabbit