refactor(webview): complete provider identifier migration - #1141
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change replaces hardcoded provider strings across webview settings, provider configuration, model selection, router synchronization, and tests with shared ChangesProvider identifier centralization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/LiteLLM.tsx`:
- Around line 63-66: The LiteLLM refresh currently invalidates only the
provider-scoped router-model query, leaving the unscoped entry used by
ApiOptions stale. In the LiteLLM success path near
queryClient.invalidateQueries, also invalidate or refresh the ["routerModels",
"all"] entry so ApiOptions validates against current router models.
🪄 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: bf22512c-a28e-490e-82a3-e9439cf38edd
📒 Files selected for processing (18)
webview-ui/src/components/settings/ApiOptions.tsxwebview-ui/src/components/settings/ModelInfoView.tsxwebview-ui/src/components/settings/ModelPicker.tsxwebview-ui/src/components/settings/constants.tswebview-ui/src/components/settings/providers/Kenari.tsxwebview-ui/src/components/settings/providers/KimiCode.tsxwebview-ui/src/components/settings/providers/LiteLLM.tsxwebview-ui/src/components/settings/providers/Moonshot.tsxwebview-ui/src/components/settings/providers/OpenCodeGo.tsxwebview-ui/src/components/settings/providers/Poe.tsxwebview-ui/src/components/settings/providers/Requesty.tsxwebview-ui/src/components/settings/providers/Unbound.tsxwebview-ui/src/components/settings/providers/VercelAiGateway.tsxwebview-ui/src/components/settings/providers/ZooGateway.tsxwebview-ui/src/components/settings/utils/__tests__/providerModelConfig.spec.tswebview-ui/src/components/settings/utils/providerModelConfig.tswebview-ui/src/components/ui/hooks/useSelectedModel.tswebview-ui/src/components/ui/hooks/useZooGatewayRouterModelsSync.ts
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts (1)
67-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the
as anycasts with typed test doubles.These casts bypass the return contracts of
useRouterModelsanduseOpenRouterModelProviders. Define typed fixtures or helpers from the hook return types. This keeps the tests able to detect router-model shape changes.After editing, run the required ESLint command and confirm that the suppression count did not increase.
As per coding guidelines, TypeScript files must avoid
as anyand use typed APIs or precise test doubles.Also applies to: 89-90, 106-107
🤖 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/ui/hooks/__tests__/useSelectedModel.spec.ts` around lines 67 - 68, Replace the as any casts in the useSelectedModel tests, including the useRouterModels and useOpenRouterModelProviders mock returns, with typed fixtures or helpers derived from their hook return types. Ensure the test doubles satisfy the hooks’ contracts so model-shape changes are detected, then run the required ESLint command and verify the suppression count has not increased.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.
Nitpick comments:
In `@webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts`:
- Around line 67-68: Replace the as any casts in the useSelectedModel tests,
including the useRouterModels and useOpenRouterModelProviders mock returns, with
typed fixtures or helpers derived from their hook return types. Ensure the test
doubles satisfy the hooks’ contracts so model-shape changes are detected, then
run the required ESLint command and verify the suppression count has not
increased.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d80a1d1b-326a-4e36-a390-30ef049a5700
📒 Files selected for processing (1)
webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts
github-actions
Bot
added
the
awaiting-review
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
webview-ui/src/components/settings/__tests__/ApiOptions.interactions.spec.tsx (3)
188-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the debounce boundary and one refresh.
These assertions pass if the refresh occurs immediately or occurs more than once. Advance to 249 ms and assert zero matching refresh calls. Then advance one more millisecond and assert exactly one matching call. Apply this check to both the
vscode.postMessageandrequestLmStudioModelspaths.🤖 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/__tests__/ApiOptions.interactions.spec.tsx` around lines 188 - 206, The debounce tests around the parameterized vscode.postMessage case and the LM Studio request case only verify eventual calls. Update both tests to advance fake timers by 249 ms and assert zero matching calls, then advance one additional millisecond and assert exactly one matching call, preserving each test’s existing expected message or configured base URL.
224-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert model preservation independently of the optional flag.
The negative matcher only rejects an
apiModelIdupdate whose third argument isfalse. A call that clears the model with another flag, or without that argument, passes this test. Inspect allsetApiConfigurationField.mock.callsforapiModelId, or assert that any write retains"custom-arn".🤖 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/__tests__/ApiOptions.interactions.spec.tsx` around lines 224 - 229, Strengthen the provider-change assertion in the test around providerSelect and setApiConfigurationField so it verifies that no apiModelId write occurs regardless of the optional third argument, while preserving the existing apiProvider assertion and the expected "custom-arn" model value.
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse canonical provider identifiers in the fixtures.
The test defines a second list of provider values with raw strings. This conflicts with the provider identifier migration. Import
providerIdentifiersfrom@roo-code/typesand use its values forapiProvider, provider-select values, and expected provider updates. Keep request-messagetypevalues unchanged because they are a separate protocol contract. Verify the exact member names from the shared export.Also applies to: 150-187, 201-201, 212-212, 220-227, 245-245
🤖 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/__tests__/ApiOptions.interactions.spec.tsx` at line 2, Update the fixtures and assertions in the ApiOptions interaction tests to import and use the canonical providerIdentifiers members for apiProvider, provider-select values, and expected provider updates. Remove the locally defined raw provider list, verify the exact member names from the shared export, and leave request-message type values unchanged.
🤖 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/__tests__/ApiOptions.interactions.spec.tsx`:
- Around line 98-126: Replace the broad any types in the mocked VSCodeTextField
and SearchableSelect components with narrow local prop and event types,
including typed children, value, placeholder, onInput, onValueChange, options,
and select change events. Apply similarly narrow types to the remaining mocked
components where practical, then run the required ESLint command without adding
suppressions.
---
Nitpick comments:
In
`@webview-ui/src/components/settings/__tests__/ApiOptions.interactions.spec.tsx`:
- Around line 188-206: The debounce tests around the parameterized
vscode.postMessage case and the LM Studio request case only verify eventual
calls. Update both tests to advance fake timers by 249 ms and assert zero
matching calls, then advance one additional millisecond and assert exactly one
matching call, preserving each test’s existing expected message or configured
base URL.
- Around line 224-229: Strengthen the provider-change assertion in the test
around providerSelect and setApiConfigurationField so it verifies that no
apiModelId write occurs regardless of the optional third argument, while
preserving the existing apiProvider assertion and the expected "custom-arn"
model value.
- Line 2: Update the fixtures and assertions in the ApiOptions interaction tests
to import and use the canonical providerIdentifiers members for apiProvider,
provider-select values, and expected provider updates. Remove the locally
defined raw provider list, verify the exact member names from the shared export,
and leave request-message type values unchanged.
🪄 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: 9e5338af-bb88-471d-b6c8-81f2975bfa2d
📒 Files selected for processing (1)
webview-ui/src/components/settings/__tests__/ApiOptions.interactions.spec.tsx
| VSCodeTextField: ({ children, value, onInput, placeholder }: any) => ( | ||
| <label> | ||
| {children} | ||
| <input value={value} placeholder={placeholder} onChange={onInput} /> | ||
| </label> | ||
| ), | ||
| VSCodeLink: ({ children }: any) => <span>{children}</span>, | ||
| })) | ||
|
|
||
| vi.mock("@/components/ui", () => ({ | ||
| SearchableSelect: ({ value, onValueChange, options, "data-testid": testId }: any) => ( | ||
| <div data-testid={testId}> | ||
| <select value={value} onChange={(event) => onValueChange(event.target.value)}> | ||
| {options.map((option: { value: string; label: string }) => ( | ||
| <option key={option.value} value={option.value}> | ||
| {option.label} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| ), | ||
| Collapsible: ({ children }: any) => <div>{children}</div>, | ||
| CollapsibleTrigger: ({ children }: any) => <div>{children}</div>, | ||
| CollapsibleContent: ({ children }: any) => <div>{children}</div>, | ||
| Select: ({ children }: any) => <div>{children}</div>, | ||
| SelectTrigger: ({ children }: any) => <div>{children}</div>, | ||
| SelectValue: () => null, | ||
| SelectContent: ({ children }: any) => <div>{children}</div>, | ||
| SelectItem: ({ children }: any) => <div>{children}</div>, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant test file around the cited lines.
file="webview-ui/src/components/settings/__tests__/ApiOptions.interactions.spec.tsx"
if [ ! -f "$file" ]; then
echo "Missing file: $file"
git ls-files | rg 'ApiOptions\.interactions\.spec\.tsx$|ApiOptions' || true
exit 0
fi
echo "== file size =="
wc -l "$file"
echo "== lines 1-180 =="
sed -n '1,180p' "$file" | cat -n
echo "== any occurrences and broad casts in file =="
rg -n '\bany\b|as\s+any' "$file" || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 8045
Replace broad any types in the test doubles.
Define narrow local prop types for the mocked components so onInput, onValueChange, options, and event props are checked. Keep the rest of the mocks narrow when possible, and run the required ESLint command without increasing suppressions.
🤖 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/__tests__/ApiOptions.interactions.spec.tsx`
around lines 98 - 126, Replace the broad any types in the mocked VSCodeTextField
and SearchableSelect components with narrow local prop and event types,
including typed children, value, placeholder, onInput, onValueChange, options,
and select change events. Apply similarly narrow types to the remaining mocked
components where practical, then run the required ESLint command without adding
suppressions.
Source: Coding guidelines
4222e9b to
30e9bfe
Compare
github-actions
Bot
removed
the
awaiting-review
1 participant
Summary
Completes item 3 from the provider identifier migration plan in #944:
MODELS_BY_PROVIDERandPROVIDERSApiOptionsrouterModelskeys inuseSelectedModelSerialized provider values and runtime behavior remain unchanged.
Validation
npx vitest runfor 8 focused webview test files: 128 tests passedpnpm check-typesinwebview-ui--prune-suppressions --max-warnings=0for all changed filesgit diff --checkRelated to #944.
Summary by CodeRabbit
Refactor
Bug Fixes
Tests