From ddedab101b02c5ba7a78f92b79579a40f2e558b9 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Tue, 25 Aug 2026 19:32:39 +0800 Subject: [PATCH 1/3] fix(settings): preserve configured LiteLLM model ID in model picker The LiteLLM case in useSelectedModel validated the configured model ID against the fetched /models list and silently substituted the hardcoded default (claude-3-7-sonnet-20250219) whenever the configured ID was absent. LiteLLM is a proxy that fronts arbitrary models and aliases, so a configured ID is the user's explicit selection even when it is not in the fetched list (custom aliases, incomplete or stale listings, renamed deployments). On the settings screen the picker reverted to the default after every selection, making the model ID appear unchangeable; the saved custom ID was also not displayed after reopening settings. Only fall back to the default when nothing is configured and a populated list exists; keep the empty-ID behavior for the empty-list case. Adds a hook-level regression test and a ModelPicker component test covering the full user flow (open picker, use-custom-model, re-render with updated config). --- .../settings/__tests__/ModelPicker.spec.tsx | 102 +++++++++++++++++- .../hooks/__tests__/useSelectedModel.spec.ts | 42 +++++++- .../components/ui/hooks/useSelectedModel.ts | 30 +++--- 3 files changed, 154 insertions(+), 20 deletions(-) diff --git a/webview-ui/src/components/settings/__tests__/ModelPicker.spec.tsx b/webview-ui/src/components/settings/__tests__/ModelPicker.spec.tsx index 06d149b20c..d2d81ad49b 100644 --- a/webview-ui/src/components/settings/__tests__/ModelPicker.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/ModelPicker.spec.tsx @@ -3,16 +3,22 @@ import { screen, fireEvent, renderWithExtensionState } from "@/utils/test-utils" import { act } from "react" import { QueryClient } from "@tanstack/react-query" +import { type Mock } from "vitest" -import { ModelInfo, providerIdentifiers } from "@roo-code/types" +import { litellmDefaultModelId, ModelInfo, providerIdentifiers } from "@roo-code/types" import { ModelPicker } from "../ModelPicker" +import { useRouterModels } from "@src/components/ui/hooks/useRouterModels" vi.mock("@src/context/ExtensionStateContext", () => ({ ExtensionStateContextProvider: ({ children }: any) => children, useExtensionState: vi.fn(), })) +vi.mock("@src/components/ui/hooks/useRouterModels") + +const mockUseRouterModels = useRouterModels as Mock + Element.prototype.scrollIntoView = vi.fn() describe("ModelPicker", () => { @@ -55,6 +61,8 @@ describe("ModelPicker", () => { beforeEach(() => { vi.clearAllMocks() vi.useFakeTimers() + // Default: no router models available. Provider-specific tests override per test. + mockUseRouterModels.mockReturnValue({ data: {}, isLoading: false, isError: false } as any) }) afterEach(() => { @@ -254,4 +262,96 @@ describe("ModelPicker", () => { expect(screen.getByTestId("automatic-fetch-hint")).toBeInTheDocument() }) }) + + describe("LiteLLM custom model selection", () => { + const litellmModels: Record = { + "gpt-4o-mini": { description: "LiteLLM proxy model", ...modelInfo }, + } + + const renderLiteLLMPicker = ( + apiConfiguration: Record, + setField: (field: string, value: unknown) => void, + ) => + renderWithExtensionState( + , + { queryClient }, + ) + + beforeEach(() => { + mockUseRouterModels.mockReturnValue({ + data: { litellm: litellmModels }, + isLoading: false, + isError: false, + } as any) + }) + + it("keeps a custom model ID in the picker instead of reverting to the default", async () => { + // Regression: on the LiteLLM settings screen the user could not change the + // model ID to a value absent from the fetched /models list -- the picker + // silently reverted to the hardcoded default model after the selection. + const customModelId = "my-litellm-alias" + let apiConfiguration: Record = { apiProvider: providerIdentifiers.litellm } + const setField = vi.fn((field: string, value: unknown) => { + apiConfiguration = { ...apiConfiguration, [field]: value } + }) + + const { rerender } = await act(async () => { + return renderLiteLLMPicker(apiConfiguration, setField) + }) + + // Before any selection the picker shows the provider default. + expect(screen.getByTestId("model-picker-button")).toHaveTextContent(litellmDefaultModelId) + + // Open the popover and type a model ID that is not in the fetched list. + await act(async () => { + fireEvent.click(screen.getByTestId("model-picker-button")) + }) + await act(async () => { + vi.advanceTimersByTime(100) + }) + await act(async () => { + fireEvent.input(screen.getByTestId("model-input"), { target: { value: customModelId } }) + }) + await act(async () => { + vi.advanceTimersByTime(100) + }) + await act(async () => { + fireEvent.click(screen.getByTestId("use-custom-model")) + }) + await act(async () => { + vi.advanceTimersByTime(100) + }) + + expect(setField).toHaveBeenCalledWith("litellmModelId", customModelId) + + // Re-render with the updated configuration (as SettingsView does after the + // setter runs) and assert the selection is kept, not reset to the default. + await act(async () => { + rerender( + , + ) + }) + + expect(screen.getByTestId("model-picker-button")).toHaveTextContent(customModelId) + expect(screen.getByTestId("model-picker-button")).not.toHaveTextContent(litellmDefaultModelId) + }) + }) }) diff --git a/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts b/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts index 3558d47e38..04b26345a2 100644 --- a/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts +++ b/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts @@ -899,7 +899,12 @@ describe("useSelectedModel", () => { expect(result.current.id).toBe("my-custom-model") }) - it("should use litellmDefaultModelInfo when selected model not found in routerModels", () => { + it("preserves a configured model ID that is absent from the populated list", () => { + // Regression: LiteLLM is a proxy whose users may configure aliases or models + // that the fetched /models list does not include (custom aliases, incomplete or + // stale listings). The configured ID is the user's explicit selection and must + // not be replaced by a hardcoded default, which made the settings screen appear + // to ignore model ID changes. mockUseRouterModels.mockReturnValue({ data: { openrouter: {}, @@ -926,9 +931,40 @@ describe("useSelectedModel", () => { const { result } = renderHook(() => useSelectedModel(apiConfiguration), { wrapper }) expect(result.current.provider).toBe(providerIdentifiers.litellm) - // Falls back to default model ID + // The configured ID is preserved even though it is absent from the fetched list + expect(result.current.id).toBe("non-existing-model") + // Model info falls back to litellmDefaultModelInfo since the model is not in router models + expect(result.current.info).toEqual(litellmDefaultModelInfo) + }) + + it("falls back to the default model ID only when nothing is configured but a list exists", () => { + mockUseRouterModels.mockReturnValue({ + data: { + openrouter: {}, + requesty: {}, + litellm: { + "existing-model": { + maxTokens: 4096, + contextWindow: 8192, + supportsImages: false, + supportsPromptCache: false, + }, + }, + }, + isLoading: false, + isError: false, + } as any) + + const apiConfiguration: ProviderSettings = { + apiProvider: providerIdentifiers.litellm, + // litellmModelId intentionally omitted + } + + const wrapper = createWrapper() + const { result } = renderHook(() => useSelectedModel(apiConfiguration), { wrapper }) + + // Nothing configured: fall back to the provider default so the picker shows a selection expect(result.current.id).toBe("claude-3-7-sonnet-20250219") - // Should use litellmDefaultModelInfo as fallback since default model also not in router models expect(result.current.info).toEqual(litellmDefaultModelInfo) }) diff --git a/webview-ui/src/components/ui/hooks/useSelectedModel.ts b/webview-ui/src/components/ui/hooks/useSelectedModel.ts index b7ad9e87e5..b8a85c9ad1 100644 --- a/webview-ui/src/components/ui/hooks/useSelectedModel.ts +++ b/webview-ui/src/components/ui/hooks/useSelectedModel.ts @@ -191,22 +191,20 @@ function getSelectedModel({ return { id, info: routerInfo } } case providerIdentifiers.litellm: { - // When the model list is empty (not yet loaded or still loading), - // preserve the configured model ID. LiteLLM is a proxy with no inherent - // default model, so we never substitute a hardcoded default here -- when - // nothing is configured we return an empty ID so the picker shows "no - // selection" rather than a phantom model that does not exist on the server. - const hasModels = - routerModels[providerIdentifiers.litellm] && - Object.keys(routerModels[providerIdentifiers.litellm]).length > 0 - const id = hasModels - ? getValidatedModelId( - apiConfiguration.litellmModelId, - routerModels[providerIdentifiers.litellm], - defaultModelId, - ) - : (apiConfiguration.litellmModelId ?? "") - const routerInfo = routerModels[providerIdentifiers.litellm]?.[id] + // LiteLLM is a proxy that fronts arbitrary models and aliases, so a + // configured model ID is the user's explicit selection even when it is + // absent from the fetched list (custom aliases, incomplete or stale + // listings, renamed deployments). Never substitute a hardcoded default + // over a configured ID -- doing so silently discards the user's model + // choice on the settings screen. Only fall back to the default when + // nothing is configured and a populated list exists; when the list is + // empty we return an empty ID so the picker shows "no selection" rather + // than a phantom model that does not exist on the server. + const litellmModels = routerModels[providerIdentifiers.litellm] + const id = + apiConfiguration.litellmModelId ?? + (litellmModels && Object.keys(litellmModels).length > 0 ? defaultModelId : "") + const routerInfo = litellmModels?.[id] return { id, info: routerInfo ?? litellmDefaultModelInfo } } case providerIdentifiers.xai: { From 000361e3d793a3a074110bf00622c9776e80a469 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Wed, 2 Sep 2026 17:44:29 +0800 Subject: [PATCH 2/3] fix(settings): resolve LiteLLM selection once router fetch settles When the router-models payload lacks a litellm provider entry (partial listing, failed fetch, renamed deployment), hasValidRouterData stayed false and useSelectedModel substituted the provider default, silently replacing the user-configured litellmModelId. LiteLLM now only needs the fetch to settle, since a configured ID is an explicit selection; other dynamic providers still require a populated provider entry. Add a hook-level regression test for a payload without the litellm entry and a configured custom ID, and convert the affected test doubles to the typed createRouterModelsResult helper, removing the as any / as never casts. --- .../settings/__tests__/ModelPicker.spec.tsx | 50 +++--- .../hooks/__tests__/useSelectedModel.spec.ts | 155 ++++++------------ .../components/ui/hooks/useSelectedModel.ts | 26 ++- 3 files changed, 99 insertions(+), 132 deletions(-) diff --git a/webview-ui/src/components/settings/__tests__/ModelPicker.spec.tsx b/webview-ui/src/components/settings/__tests__/ModelPicker.spec.tsx index d2d81ad49b..9a21ff678f 100644 --- a/webview-ui/src/components/settings/__tests__/ModelPicker.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/ModelPicker.spec.tsx @@ -1,17 +1,33 @@ // npx vitest src/components/settings/__tests__/ModelPicker.spec.tsx import { screen, fireEvent, renderWithExtensionState } from "@/utils/test-utils" -import { act } from "react" +import { act, type ReactNode } from "react" import { QueryClient } from "@tanstack/react-query" import { type Mock } from "vitest" -import { litellmDefaultModelId, ModelInfo, providerIdentifiers } from "@roo-code/types" +import { + litellmDefaultModelId, + type ModelInfo, + type ProviderSettings, + type RouterModels, + providerIdentifiers, +} from "@roo-code/types" import { ModelPicker } from "../ModelPicker" import { useRouterModels } from "@src/components/ui/hooks/useRouterModels" +type SetApiConfigurationField = ( + field: K, + value: ProviderSettings[K], + isUserAction?: boolean, +) => void + +// useRouterModels returns a react-query observable result; these tests only need the stable state fields. +const createRouterModelsResult = (data: Partial): ReturnType => + ({ data, isLoading: false, isError: false }) as ReturnType + vi.mock("@src/context/ExtensionStateContext", () => ({ - ExtensionStateContextProvider: ({ children }: any) => children, + ExtensionStateContextProvider: ({ children }: { children: ReactNode }) => children, useExtensionState: vi.fn(), })) @@ -40,8 +56,9 @@ describe("ModelPicker", () => { model2: { name: "Model 2", description: "Test model 2", ...modelInfo }, } + const apiConfiguration: ProviderSettings = {} const defaultProps = { - apiConfiguration: {}, + apiConfiguration, defaultModelId: "model1", modelIdKey: "openRouterModelId" as const, serviceName: "Test Service", @@ -62,7 +79,7 @@ describe("ModelPicker", () => { vi.clearAllMocks() vi.useFakeTimers() // Default: no router models available. Provider-specific tests override per test. - mockUseRouterModels.mockReturnValue({ data: {}, isLoading: false, isError: false } as any) + mockUseRouterModels.mockReturnValue(createRouterModelsResult({})) }) afterEach(() => { @@ -268,30 +285,23 @@ describe("ModelPicker", () => { "gpt-4o-mini": { description: "LiteLLM proxy model", ...modelInfo }, } - const renderLiteLLMPicker = ( - apiConfiguration: Record, - setField: (field: string, value: unknown) => void, - ) => + const renderLiteLLMPicker = (apiConfiguration: ProviderSettings, setField: SetApiConfigurationField) => renderWithExtensionState( , { queryClient }, ) beforeEach(() => { - mockUseRouterModels.mockReturnValue({ - data: { litellm: litellmModels }, - isLoading: false, - isError: false, - } as any) + mockUseRouterModels.mockReturnValue(createRouterModelsResult({ litellm: litellmModels })) }) it("keeps a custom model ID in the picker instead of reverting to the default", async () => { @@ -299,8 +309,8 @@ describe("ModelPicker", () => { // model ID to a value absent from the fetched /models list -- the picker // silently reverted to the hardcoded default model after the selection. const customModelId = "my-litellm-alias" - let apiConfiguration: Record = { apiProvider: providerIdentifiers.litellm } - const setField = vi.fn((field: string, value: unknown) => { + let apiConfiguration: ProviderSettings = { apiProvider: providerIdentifiers.litellm } + const setField = vi.fn(function (field: K, value: ProviderSettings[K]) { apiConfiguration = { ...apiConfiguration, [field]: value } }) @@ -338,13 +348,13 @@ describe("ModelPicker", () => { await act(async () => { rerender( , ) diff --git a/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts b/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts index fc68dfa370..56cc396ce7 100644 --- a/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts +++ b/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts @@ -685,15 +685,7 @@ describe("useSelectedModel", () => { describe("bedrock provider with 1M context", () => { beforeEach(() => { - mockUseRouterModels.mockReturnValue({ - data: { - openrouter: {}, - requesty: {}, - litellm: {}, - }, - isLoading: false, - isError: false, - } as any) + mockUseRouterModels.mockReturnValue(createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: {} })) mockUseOpenRouterModelProviders.mockReturnValue({ data: {}, @@ -747,15 +739,7 @@ describe("useSelectedModel", () => { describe("bedrock provider with custom ARN", () => { beforeEach(() => { - mockUseRouterModels.mockReturnValue({ - data: { - openrouter: {}, - requesty: {}, - litellm: {}, - }, - isLoading: false, - isError: false, - } as any) + mockUseRouterModels.mockReturnValue(createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: {} })) mockUseOpenRouterModelProviders.mockReturnValue({ data: {}, @@ -801,15 +785,7 @@ describe("useSelectedModel", () => { }) it("should use litellmDefaultModelInfo as fallback when routerModels.litellm is empty", () => { - mockUseRouterModels.mockReturnValue({ - data: { - openrouter: {}, - requesty: {}, - litellm: {}, - }, - isLoading: false, - isError: false, - } as any) + mockUseRouterModels.mockReturnValue(createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: {} })) const apiConfiguration: ProviderSettings = { apiProvider: providerIdentifiers.litellm, @@ -827,15 +803,7 @@ describe("useSelectedModel", () => { }) it("should return an empty model ID when the list is empty and no model is configured", () => { - mockUseRouterModels.mockReturnValue({ - data: { - openrouter: {}, - requesty: {}, - litellm: {}, - }, - isLoading: false, - isError: false, - } as any) + mockUseRouterModels.mockReturnValue(createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: {} })) const apiConfiguration: ProviderSettings = { apiProvider: providerIdentifiers.litellm, @@ -855,8 +823,8 @@ describe("useSelectedModel", () => { // Primary user-visible scenario: a "Sync Models" click momentarily empties the // router-models list before the refreshed list arrives. The selection must be held // across that transition rather than reset. - mockUseRouterModels.mockReturnValue({ - data: { + mockUseRouterModels.mockReturnValue( + createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: { @@ -867,10 +835,8 @@ describe("useSelectedModel", () => { supportsPromptCache: false, }, }, - }, - isLoading: false, - isError: false, - } as any) + }), + ) const apiConfiguration: ProviderSettings = { apiProvider: providerIdentifiers.litellm, @@ -884,15 +850,7 @@ describe("useSelectedModel", () => { expect(result.current.id).toBe("my-custom-model") // Simulate the list emptying mid-sync. - mockUseRouterModels.mockReturnValue({ - data: { - openrouter: {}, - requesty: {}, - litellm: {}, - }, - isLoading: false, - isError: false, - } as any) + mockUseRouterModels.mockReturnValue(createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: {} })) rerender() // Selection is preserved through the empty window. @@ -905,8 +863,8 @@ describe("useSelectedModel", () => { // stale listings). The configured ID is the user's explicit selection and must // not be replaced by a hardcoded default, which made the settings screen appear // to ignore model ID changes. - mockUseRouterModels.mockReturnValue({ - data: { + mockUseRouterModels.mockReturnValue( + createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: { @@ -917,10 +875,8 @@ describe("useSelectedModel", () => { supportsPromptCache: false, }, }, - }, - isLoading: false, - isError: false, - } as any) + }), + ) const apiConfiguration: ProviderSettings = { apiProvider: providerIdentifiers.litellm, @@ -938,8 +894,8 @@ describe("useSelectedModel", () => { }) it("falls back to the default model ID only when nothing is configured but a list exists", () => { - mockUseRouterModels.mockReturnValue({ - data: { + mockUseRouterModels.mockReturnValue( + createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: { @@ -950,10 +906,8 @@ describe("useSelectedModel", () => { supportsPromptCache: false, }, }, - }, - isLoading: false, - isError: false, - } as any) + }), + ) const apiConfiguration: ProviderSettings = { apiProvider: providerIdentifiers.litellm, @@ -968,6 +922,27 @@ describe("useSelectedModel", () => { expect(result.current.info).toEqual(litellmDefaultModelInfo) }) + it("preserves a configured model ID when the router payload has no litellm entry", () => { + // Regression: when the router-models payload lacks the litellm provider entry + // (partial or failed response), the hook must not substitute the provider + // default, which would silently replace the user's configured ID. + mockUseRouterModels.mockReturnValue(createRouterModelsResult({})) + + const apiConfiguration: ProviderSettings = { + apiProvider: providerIdentifiers.litellm, + litellmModelId: "my-litellm-alias", + } + + const wrapper = createWrapper() + const { result } = renderHook(() => useSelectedModel(apiConfiguration), { wrapper }) + + expect(result.current.provider).toBe(providerIdentifiers.litellm) + // The configured ID survives even though the payload has no litellm entry at all + expect(result.current.id).toBe("my-litellm-alias") + // No router info is available for the configured ID, so the fallback info applies + expect(result.current.info).toEqual(litellmDefaultModelInfo) + }) + it("should return routerModels info when model exists", () => { const customModelInfo: ModelInfo = { maxTokens: 16384, @@ -977,17 +952,13 @@ describe("useSelectedModel", () => { description: "Custom LiteLLM model", } - mockUseRouterModels.mockReturnValue({ - data: { + mockUseRouterModels.mockReturnValue( + createRouterModelsResult({ openrouter: {}, requesty: {}, - litellm: { - "custom-model": customModelInfo, - }, - }, - isLoading: false, - isError: false, - } as any) + litellm: { "custom-model": customModelInfo }, + }), + ) const apiConfiguration: ProviderSettings = { apiProvider: providerIdentifiers.litellm, @@ -1119,15 +1090,7 @@ describe("useSelectedModel", () => { describe("openai provider", () => { beforeEach(() => { - mockUseRouterModels.mockReturnValue({ - data: { - openrouter: {}, - requesty: {}, - litellm: {}, - }, - isLoading: false, - isError: false, - } as any) + mockUseRouterModels.mockReturnValue(createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: {} })) mockUseOpenRouterModelProviders.mockReturnValue({ data: {}, @@ -1200,15 +1163,7 @@ describe("useSelectedModel", () => { describe("minimax provider", () => { beforeEach(() => { - mockUseRouterModels.mockReturnValue({ - data: { - openrouter: {}, - requesty: {}, - litellm: {}, - }, - isLoading: false, - isError: false, - } as any) + mockUseRouterModels.mockReturnValue(createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: {} })) mockUseOpenRouterModelProviders.mockReturnValue({ data: {}, @@ -1247,15 +1202,7 @@ describe("useSelectedModel", () => { describe("vscode-lm provider", () => { beforeEach(() => { - mockUseRouterModels.mockReturnValue({ - data: { - openrouter: {}, - requesty: {}, - litellm: {}, - }, - isLoading: false, - isError: false, - } as any) + mockUseRouterModels.mockReturnValue(createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: {} })) mockUseOpenRouterModelProviders.mockReturnValue({ data: {}, @@ -1318,15 +1265,7 @@ describe("useSelectedModel", () => { describe("friendli provider", () => { beforeEach(() => { - mockUseRouterModels.mockReturnValue({ - data: { - openrouter: {}, - requesty: {}, - litellm: {}, - }, - isLoading: false, - isError: false, - } as any) + mockUseRouterModels.mockReturnValue(createRouterModelsResult({ openrouter: {}, requesty: {}, litellm: {} })) mockUseOpenRouterModelProviders.mockReturnValue({ data: {}, diff --git a/webview-ui/src/components/ui/hooks/useSelectedModel.ts b/webview-ui/src/components/ui/hooks/useSelectedModel.ts index b8a85c9ad1..8d4b70ad4a 100644 --- a/webview-ui/src/components/ui/hooks/useSelectedModel.ts +++ b/webview-ui/src/components/ui/hooks/useSelectedModel.ts @@ -57,6 +57,17 @@ function getValidatedModelId( return configuredId && availableModels?.[configuredId] ? configuredId : defaultModelId } +/** + * Resolves the model currently selected for the active API provider. + * + * Dynamic providers validate the configured model ID against the fetched + * router-model list and only resolve the selection once that list is + * available. LiteLLM is the exception: it fronts arbitrary models and + * aliases, so a configured `litellmModelId` is the user's explicit + * selection and is preserved as soon as the fetch settles, even when the + * router payload has no LiteLLM entry (partial listing, failed fetch, or + * renamed deployment). + */ export const useSelectedModel = (apiConfiguration?: ProviderSettings) => { const provider = apiConfiguration?.apiProvider || providerIdentifiers.openrouter const activeProvider: ProviderName | undefined = isRetiredProvider(provider) ? undefined : provider @@ -84,12 +95,19 @@ export const useSelectedModel = (apiConfiguration?: ProviderSettings) => { const needLmStudio = typeof lmStudioModelId !== "undefined" const needOllama = typeof ollamaModelId !== "undefined" + // LiteLLM may legitimately have no entry in the router payload (partial + // listing, failed fetch, renamed deployment) even though the configured + // ID is a valid selection, so it only needs the fetch to settle. Other + // dynamic providers require a populated provider entry before the + // selection is resolved. const hasValidRouterData = needRouterModels && dynamicProvider - ? routerModels.data && - routerModels.data[dynamicProvider] !== undefined && - typeof routerModels.data[dynamicProvider] === "object" && - !routerModels.isLoading + ? dynamicProvider === providerIdentifiers.litellm + ? !routerModels.isLoading + : routerModels.data && + routerModels.data[dynamicProvider] !== undefined && + typeof routerModels.data[dynamicProvider] === "object" && + !routerModels.isLoading : true const isReady = From e24510dcd0ec4038c8302d50a6a330d222c6939e Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sat, 12 Sep 2026 03:06:09 +0000 Subject: [PATCH 3/3] test(useSelectedModel): cover LiteLLM isError path after hasValidRouterData change --- .../hooks/__tests__/useSelectedModel.spec.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts b/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts index 56cc396ce7..cf29cbe6fa 100644 --- a/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts +++ b/webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts @@ -972,6 +972,30 @@ describe("useSelectedModel", () => { expect(result.current.id).toBe("custom-model") expect(result.current.info).toEqual(customModelInfo) }) + + it("resolves the configured model ID even when the router fetch errors", () => { + // Regression guard: hasValidRouterData for LiteLLM now only requires + // !isLoading, so a failed fetch (isError=true, isLoading=false) must + // still resolve the hook and preserve the configured ID rather than + // resetting to the provider default. + mockUseRouterModels.mockReturnValue( + createRouterModelsResult(undefined, { isLoading: false, isError: true }), + ) + + const apiConfiguration: ProviderSettings = { + apiProvider: providerIdentifiers.litellm, + litellmModelId: "my-litellm-alias", + } + + const wrapper = createWrapper() + const { result } = renderHook(() => useSelectedModel(apiConfiguration), { wrapper }) + + // Configured ID preserved — a fetch failure must not reset the user's selection. + expect(result.current.id).toBe("my-litellm-alias") + expect(result.current.info).toEqual(litellmDefaultModelInfo) + // Callers that surface error banners still see the flag. + expect(result.current.isError).toBe(true) + }) }) describe("kenari provider", () => {