feat(friendli): fetch model list dynamically from /v1/models - #1219
feat(friendli): fetch model list dynamically from /v1/models#1219Lee-Si-Yoon wants to merge 8 commits into
Conversation
|
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)
📝 WalkthroughWalkthroughFriendli now discovers model metadata through the Friendli API, caches it, and uses it for provider requests, routing, and settings model selection. Static metadata remains as a fallback. Reasoning and model-fetching tests cover the new behavior. ChangesFriendli dynamic model discovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsUI
participant WebviewMessageHandler
participant FriendliProvider
participant modelCache
participant FriendliAPI
SettingsUI->>WebviewMessageHandler: request router models
WebviewMessageHandler->>FriendliProvider: request Friendli models
FriendliProvider->>modelCache: load dynamic metadata
modelCache->>FriendliAPI: request model list
FriendliAPI-->>modelCache: model response
modelCache-->>FriendliProvider: ModelInfo records
FriendliProvider-->>WebviewMessageHandler: Friendli models
WebviewMessageHandler-->>SettingsUI: router model data
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/api/providers/fetchers/__tests__/friendli.spec.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. 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: 7
🧹 Nitpick comments (1)
src/api/providers/__tests__/friendli.spec.ts (1)
422-423: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove private-state double assertions from dynamic-load tests.
Wait for observable
getModel()results after resolving or rejecting a controlledmockGetModelspromise. Do not inspectdynamicModelsLoadedthroughas unknown as Record<string, unknown>.
src/api/providers/__tests__/friendli.spec.ts#L422-L423: wait until the dynamic model is selected.src/api/providers/__tests__/friendli.spec.ts#L525-L526: wait until the unknown model falls back to the default.src/api/providers/__tests__/friendli.spec.ts#L551-L552: wait until dynamic metadata appears ingetModel().src/api/providers/__tests__/friendli.spec.ts#L573-L575: configure an unknown requested ID, then wait until rejection causes fallback.As per coding guidelines, “Use double assertions only as a last resort and explain them with a comment.”
🤖 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/api/providers/__tests__/friendli.spec.ts` around lines 422 - 423, Replace the private dynamicModelsLoaded double assertions in src/api/providers/__tests__/friendli.spec.ts at lines 422-423, 525-526, 551-552, and 573-575 with waits for observable getModel() outcomes: confirm the dynamic model is selected, the unknown model falls back to the default, dynamic metadata appears, and rejection after configuring an unknown requested ID causes fallback. Do not inspect handler private state or use the double assertion.Source: Coding guidelines
🤖 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/friendli.ts`:
- Around line 235-237: Update the supportsMaxTokens assignment in the model
metadata construction to depend only on model.max_completion_tokens, removing
the model.reasoning requirement so non-reasoning chat models expose max-token
controls.
- Around line 106-122: Update the reasoning-effort handling in the Friendli
fetcher so that when the API provides effort values but filtering leaves no
known values, it returns binary reasoning support instead of an empty effort
list that later defaults to unsupported "high". Preserve the existing
filtered-effort behavior when known values remain, and update the Friendli
regression test to expect binary support for this case.
In `@src/api/providers/friendli.ts`:
- Around line 136-144: Update the dynamic-only branch in the provider
model-selection flow, near thisProviderModelId handling, so it does not assign
defaultProviderModelId metadata to the requested model. Ensure createStream does
not derive max_tokens or reasoning parameters from fallback metadata: wait for
the initial dynamic model load or omit those parameters until the selected
model’s metadata is available. Add a regression test covering a request made
while the dynamic fetch is pending.
In `@webview-ui/src/components/settings/ApiOptions.tsx`:
- Around line 701-704: Update the Friendli model binding around routerModels and
requestRouterModels so an absent or empty Friendli record falls back to the
existing static friendliModels list, while populated records remain active. Add
focused UI tests covering missing, empty, and populated Friendli router models.
In `@webview-ui/src/components/settings/providers/__tests__/Friendli.spec.tsx`:
- Around line 30-32: Update the ModelPicker mock and the Friendli test to
capture its props, then assert the dynamic models, static fallback,
defaultModelId, and errorMessage bindings. Add a model-selection interaction
assertion verifying the callback writes through setApiConfigurationField,
covering both UI binding and save behavior.
In `@webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts`:
- Around line 1082-1091: Replace the as any cast in the mockUseRouterModels
fixture with a fully typed React Query result matching useRouterModels,
including a complete RouterModels provider map and all required query-result
fields. Use the hook’s return type or a typed fixture helper so TypeScript
validates both the provider data and query state.
In `@webview-ui/src/components/ui/hooks/useSelectedModel.ts`:
- Around line 366-372: Preserve the saved Friendli model selection while router
data is loading by updating the Friendli branch in useSelectedModel to avoid
returning a default ID that ApiOptions can persist over
apiConfiguration.apiModelId; retain the saved ID until dynamic data is
available. In webview-ui/src/components/ui/hooks/Friendli.tsx lines 76-76, use
friendliModels whenever routerModels.friendli is absent or empty so ModelPicker
receives the static fallback.
---
Nitpick comments:
In `@src/api/providers/__tests__/friendli.spec.ts`:
- Around line 422-423: Replace the private dynamicModelsLoaded double assertions
in src/api/providers/__tests__/friendli.spec.ts at lines 422-423, 525-526,
551-552, and 573-575 with waits for observable getModel() outcomes: confirm the
dynamic model is selected, the unknown model falls back to the default, dynamic
metadata appears, and rejection after configuring an unknown requested ID causes
fallback. Do not inspect handler private state or use the double assertion.
🪄 Autofix
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: 7e06fa35-05a8-4f1d-8a96-78d71bd14073
📒 Files selected for processing (20)
packages/types/src/__tests__/provider-identifiers.test.tspackages/types/src/provider-settings.tspackages/types/src/providers/friendli.tssrc/api/providers/__tests__/friendli.spec.tssrc/api/providers/fetchers/__tests__/friendli.spec.tssrc/api/providers/fetchers/friendli.tssrc/api/providers/fetchers/modelCache.tssrc/api/providers/friendli.tssrc/core/webview/__tests__/ClineProvider.spec.tssrc/core/webview/__tests__/webviewMessageHandler.spec.tssrc/core/webview/webviewMessageHandler.tssrc/shared/api.tswebview-ui/src/components/settings/ApiOptions.tsxwebview-ui/src/components/settings/constants.tswebview-ui/src/components/settings/providers/Friendli.tsxwebview-ui/src/components/settings/providers/__tests__/Friendli.spec.tsxwebview-ui/src/components/settings/utils/providerModelConfig.tswebview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.tswebview-ui/src/components/ui/hooks/useSelectedModel.tswebview-ui/src/utils/__tests__/validate.spec.ts
💤 Files with no reviewable changes (1)
- webview-ui/src/components/settings/constants.ts
| routerModels={routerModels} | ||
| organizationAllowList={organizationAllowList} | ||
| modelValidationError={modelValidationError} | ||
| simplifySettings={fromWelcomeView} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the static fallback when Friendli returns no models.
The new routerModels prop activates routerModels?.["friendli"] ?? friendliModels in Friendli. requestRouterModels returns {} for an empty or failed Friendli fetch. Since {} is non-nullish, the picker receives no models instead of the static GLM-5.2 fallback.
Normalize an empty Friendli record to undefined, or make Friendli fall back when the record has zero keys. Add a UI test for missing, empty, and populated Friendli router models.
As per coding guidelines, add focused tests for UI binding behavior.
🤖 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/ApiOptions.tsx` around lines 701 - 704,
Update the Friendli model binding around routerModels and requestRouterModels so
an absent or empty Friendli record falls back to the existing static
friendliModels list, while populated records remain active. Add focused UI tests
covering missing, empty, and populated Friendli router models.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@webview-ui/src/components/settings/providers/__tests__/Friendli.spec.tsx`:
- Around line 30-35: Update the ModelPicker test double in the vi.mock factory
to replace props: any and the untyped mockModelPickerProps with
React.ComponentProps<typeof ModelPicker> or the exported ModelPickerProps type,
keeping the mock aligned with the component contract.
🪄 Autofix
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: 67e8c45e-7201-4938-818c-a316235e8393
📒 Files selected for processing (8)
src/api/providers/__tests__/friendli.spec.tssrc/api/providers/fetchers/__tests__/friendli.spec.tssrc/api/providers/fetchers/friendli.tssrc/api/providers/friendli.tswebview-ui/src/components/settings/providers/Friendli.tsxwebview-ui/src/components/settings/providers/__tests__/Friendli.spec.tsxwebview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.tswebview-ui/src/components/ui/hooks/useSelectedModel.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- webview-ui/src/components/ui/hooks/useSelectedModel.ts
- webview-ui/src/components/ui/hooks/tests/useSelectedModel.spec.ts
- src/api/providers/fetchers/tests/friendli.spec.ts
- webview-ui/src/components/settings/providers/Friendli.tsx
- src/api/providers/fetchers/friendli.ts
- src/api/providers/tests/friendli.spec.ts
| let mockModelPickerProps: Record<string, unknown> = {} | ||
|
|
||
| vi.mock("../../ModelPicker", () => ({ | ||
| ModelPicker: (props: any) => { | ||
| mockModelPickerProps = props | ||
| return <div data-testid="friendli-model-picker-mock" /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate test structure ---'
ast-grep outline webview-ui/src/components/settings/providers/__tests__/Friendli.spec.tsx
printf '%s\n' '--- relevant test lines ---'
cat -n webview-ui/src/components/settings/providers/__tests__/Friendli.spec.tsx | sed -n '1,180p'
printf '%s\n' '--- ModelPicker declarations and usages ---'
rg -n --glob '*.{ts,tsx}' 'interface .*ModelPicker|type .*ModelPicker|function ModelPicker|const ModelPicker|ModelPicker\(' webview-ui/src | head -120
printf '%s\n' '--- package and lint scripts ---'
fd -i 'package.json' -a . | xargs -r rg -n '"(test|lint|typecheck|vitest|eslint)[^"]*"'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 11691
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- ModelPicker props and implementation ---'
cat -n webview-ui/src/components/settings/ModelPicker.tsx | sed -n '35,125p'
printf '%s\n' '--- Friendli implementation ---'
cat -n webview-ui/src/components/settings/providers/Friendli.tsx | sed -n '1,180p'
printf '%s\n' '--- TypeScript config and available dependencies ---'
for f in webview-ui/tsconfig.json webview-ui/tsconfig.*.json webview-ui/package.json; do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f" | sed -n '1,180p'
fi
done
printf '%s\n' '--- all ModelPicker mock prop typing patterns ---'
rg -n -C 4 'mockModelPickerProps|vi\.mock\(".*ModelPicker|ModelPicker:.*props' webview-ui/src/components/settings/providers webview-ui/src/components/settings --glob '*.{spec,test}.{ts,tsx}'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 34202
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
model_picker = Path("webview-ui/src/components/settings/ModelPicker.tsx").read_text()
friendli_test = Path("webview-ui/src/components/settings/providers/__tests__/Friendli.spec.tsx").read_text()
props_decl = re.search(r"(?m)^interface\s+ModelPickerProps\s*\{", model_picker)
props_exported = bool(re.search(r"(?m)^export\s+interface\s+ModelPickerProps\s*\{", model_picker))
mock_any = bool(re.search(r"ModelPicker:\s*\(props:\s*any\)", friendli_test))
print(f"ModelPickerProps_declared={bool(props_decl)}")
print(f"ModelPickerProps_exported={props_exported}")
print(f"Friendli_ModelPicker_mock_uses_any={mock_any}")
print("Friendli_ModelPicker_mock_capture_type=Record<string, unknown>")
PYRepository: Zoo-Code-Org/Zoo-Code
Length of output: 325
Type the ModelPicker test double.
Use React.ComponentProps<typeof ModelPicker> or export and import ModelPickerProps instead of props: any. This keeps the mock aligned with the component contract.
🤖 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/__tests__/Friendli.spec.tsx`
around lines 30 - 35, Update the ModelPicker test double in the vi.mock factory
to replace props: any and the untyped mockModelPickerProps with
React.ComponentProps<typeof ModelPicker> or the exported ModelPickerProps type,
keeping the mock aligned with the component contract.
Source: Coding guidelines
Convert Friendli from a static provider (4 hardcoded models) to a dynamic provider that fetches the live model list from the public https://api.friendli.ai/serverless/v1/models endpoint at runtime. - Add getFriendliModels() fetcher with zod schema validation - Wire friendli into modelCache, webviewMessageHandler, and dynamicProviders - FriendliHandler loads dynamic models in constructor, falls back to static friendliModels for cold-start and API lag - UI model picker uses routerModels.friendli instead of static list - Add fetcher spec (14 tests) and update Friendli.spec.tsx with ModelPicker mock
- fetcher: return empty list when safeParse fails instead of consuming unvalidated response data - fetcher: add 10s timeout to /models axios request - fetcher: preserve API-provided reasoning effort values verbatim, dropping only "default" and unknown values like "ultracode" instead of merging hardcoded extra efforts - handler: track dynamicModelsLoaded and preserve a dynamic-only requestedId during the initial load window so the first request after construction doesn't silently fall back to the default model - shared/api.ts: use object type instead of eslint-disable suppression - Friendli.tsx: fall back to static friendliModels when routerModels is unavailable so the picker always has selectable models
Add friendli to routerModels expectations and mock sequences — it was added to the handler's provider list but the existing tests weren't updated, causing the mock call order to shift and expectations to miss.
Same issue as webviewMessageHandler spec — routerModels expectations and mock sequences needed friendli added to match the handler's provider list.
…oading - fetcher: test ultracode/unknown effort value dropping + de-duplication - handler: mock getModels, test dynamicModelsLoaded branches — pending load preserves dynamic-only id, completed load falls back to default, dynamic info used when available, rejection sets loaded flag
…enum Friendli API returns reasoning: true for all models, but only GLM-5.2 has a discrete effort enum (["high","max"]). Other models (DeepSeek-V3.2, MiniMax-M2.5, GLM-5.1, gemma, K-EXAONE) only support on/off thinking toggle via chat_template_kwargs.enable_thinking. Previously these models got supportsReasoningEffort: true (boolean), which made the UI show a full effort dropdown (low/medium/high/...) even though the API ignores reasoning_effort for them. Now they get supportsReasoningBinary: true, which shows a simple on/off checkbox. Also fixes max tokens: all Friendli reasoning models with max_completion_tokens now get supportsMaxTokens: true (the fetcher already did this, but the static fallback also needs it — it already has it, so dynamic + static are now consistent). Handler updated to send enable_thinking + parse_reasoning for binary reasoning models when reasoning is enabled, and nothing when disabled.
The /v1/models endpoint has been updated. Static seed now only contains GLM-5.2 (default model) with corrected maxTokens (1_048_576) and context window (1_048_576) matching the live API. All other models (GLM-5.1, DeepSeek-V3.2, MiniMax-M2.5) are removed from the static seed and are served exclusively from the dynamic /v1/models fetcher at runtime. - packages/types: FriendliModelId narrowed to "zai-org/GLM-5.2"; friendliModels reduced to single entry with live API values - src/api/providers/friendli.ts: updated comment about binary reasoning models to reference the live /v1/models list - Tests updated: removed static model assertions for GLM-5.1/DeepSeek-V3.2/ MiniMax-M2.5; DeepSeek-V3.2 binary reasoning test now provides a dynamic model mock; useSelectedModel test uses routerModels for dynamic models; clamp tests updated for new GLM-5.2 maxTokens
081d9f6 to
70cfac6
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
7 actionable comments + 1 nitpick resolved:
1. (Major) fetchers/friendli.ts: empty effort array now falls back to
binary reasoning instead of returning [] that defaults to unsupported
reasoning_effort="high"
2. (Minor) fetchers/friendli.ts: supportsMaxTokens no longer requires
model.reasoning — all chat models with max_completion_tokens get the
max-token slider
3. (Major) friendli.ts: dynamic-loading fallback now uses
openAiModelInfoSaneDefaults instead of GLM-5.2-specific metadata,
preventing wrong reasoning params and max_tokens during cold start;
createStream and completePrompt call getModel() once and pass the
result to buildFriendliReasoningParams
4. (Major) Friendli.tsx: empty {} routerModels.friendli now falls back
to static friendliModels (checks Object.keys length, not truthiness)
5. (Minor) Friendli.spec.tsx: ModelPicker mock now captures props and
asserts bindings (defaultModelId, models, errorMessage)
6. (Minor) useSelectedModel.spec.ts: replaced `as any` with typed
`as unknown as ReturnType<typeof useRouterModels>` cast
7. (Major) useSelectedModel.ts: Friendli branch preserves saved model
selection during dynamic catalog loading instead of substituting
the default ID
8. (Nitpick) friendli.spec.ts: removed all dynamicModelsLoaded double
assertions — tests now wait for observable getModel() outcomes
70cfac6 to
16a4270
Compare
Related GitHub Issue
Closes: #
Description
The Friendli
/v1/modelsendpoint has been updated and is now the source of truth for the available model list. This PR reduces the static seed to only the default model (GLM-5.2) and lets all other models come from the live /v1/models fetcher at runtime.Static seed reduction:
FriendliModelIdnarrowed to"zai-org/GLM-5.2"onlyfriendliModelsreduced to a single entry with corrected values from the live API (maxTokens: 1_048_576, contextWindow: 1_048_576)Why only GLM-5.2 in the seed: the static map is a cold-start fallback for when the dynamic fetch hasn't completed or has failed. Only the default model needs to be available in that window; all other models are populated from /v1/models within seconds of handler construction.
Test updates:
it.eachstatic model assertions for GLM-5.1, DeepSeek-V3.2, MiniMax-M2.5vi.waitForuseSelectedModeltest usesrouterModels.friendlifor dynamic-only modelsTest Procedure
pnpm vitest run api/providers/__tests__/friendli.spec.ts api/providers/fetchers/__tests__/friendli.spec.ts— 44 tests passpnpm vitest run src/components/ui/hooks/__tests__/useSelectedModel.spec.ts— 42 tests passpnpm vitest run src/components/settings/providers/__tests__/Friendli.spec.tsx— 3 tests passpnpm vitest run core/webview/__tests__/ClineProvider.spec.ts core/webview/__tests__/webviewMessageHandler.spec.ts— 184 tests passpnpm check-types— passpnpm build— passpnpm lint— passPre-Submission Checklist
Documentation Updates
Additional Notes
Supersedes #1152 (closed) and #1028 (closed). The /v1/models response was updated since #1152 was opened; this PR follows the latest response.
Summary by CodeRabbit
New Features
Bug Fixes