Ver Fonte

Preserve model settings when selecting a specific OpenRouter provider (#3915)

Chris Estreich há 7 meses atrás
pai
commit
1366ba0115

+ 5 - 0
.changeset/twelve-pigs-reply.md

@@ -0,0 +1,5 @@
+---
+"roo-cline": patch
+---
+
+Preserve model settings when selecting a specific OpenRouter provider

+ 7 - 5
src/extension.ts

@@ -145,17 +145,19 @@ export async function activate(context: vscode.ExtensionContext) {
 	const socketPath = process.env.ROO_CODE_IPC_SOCKET_PATH
 	const enableLogging = typeof socketPath === "string"
 
-	// Watch the core files and automatically reload the extension host
-	const enableCoreAutoReload = process.env?.NODE_ENV === "development"
-	if (enableCoreAutoReload) {
-		console.log(`♻️♻️♻️ Core auto-reloading is ENABLED!`)
+	// Watch the core files and automatically reload the extension host.
+	if (process.env.NODE_ENV === "development") {
+		console.log(`♻️♻️♻️ Core auto-reloading is ENABLED! Watching for changes in ${context.extensionPath}/**/*.ts`)
+
 		const watcher = vscode.workspace.createFileSystemWatcher(
-			new vscode.RelativePattern(context.extensionPath, "src/**/*.ts"),
+			new vscode.RelativePattern(context.extensionPath, "**/*.ts"),
 		)
+
 		watcher.onDidChange((uri) => {
 			console.log(`♻️ File changed: ${uri.fsPath}. Reloading host…`)
 			vscode.commands.executeCommand("workbench.action.reloadWindow")
 		})
+
 		context.subscriptions.push(watcher)
 	}
 

+ 10 - 1
webview-ui/eslint.config.mjs

@@ -5,7 +5,16 @@ export default [
 	...reactConfig,
 	{
 		rules: {
-			"@typescript-eslint/no-unused-vars": "off",
+			"@typescript-eslint/no-unused-vars": [
+				"error",
+				{
+					args: "all",
+					ignoreRestSiblings: true,
+					varsIgnorePattern: "^_",
+					argsIgnorePattern: "^_",
+					caughtErrorsIgnorePattern: "^_",
+				},
+			],
 			"@typescript-eslint/no-explicit-any": "off",
 			"react/prop-types": "off",
 			"react/display-name": "off",

+ 1 - 1
webview-ui/src/components/common/__tests__/CodeBlock.test.tsx

@@ -33,7 +33,7 @@ jest.mock("lucide-react", () => {
 	return new Proxy(
 		{},
 		{
-			get: function (obj, prop) {
+			get: function (_obj, prop) {
 				// Return a component factory for any icon that's requested
 				if (prop === "__esModule") {
 					return true

+ 1 - 1
webview-ui/src/components/prompts/__tests__/PromptsView.test.tsx

@@ -17,7 +17,7 @@ jest.mock("lucide-react", () => {
 	return new Proxy(
 		{},
 		{
-			get: function (obj, prop) {
+			get: function (_obj, prop) {
 				// Return a component factory for any icon that's requested
 				if (prop === "__esModule") {
 					return true

+ 0 - 1
webview-ui/src/components/settings/CodeIndexSettings.tsx

@@ -17,7 +17,6 @@ import {
 } from "@/components/ui/alert-dialog"
 
 import { vscode } from "@/utils/vscode"
-import { ExtensionStateContextType } from "@/context/ExtensionStateContext"
 import { CodebaseIndexConfig, CodebaseIndexModels, ProviderSettings } from "../../../../src/schemas"
 import { EmbedderProvider } from "../../../../src/shared/embeddingModels"
 import { z } from "zod"

+ 1 - 1
webview-ui/src/components/settings/__tests__/SettingsView.test.tsx

@@ -15,7 +15,7 @@ jest.mock("lucide-react", () => {
 	return new Proxy(
 		{},
 		{
-			get: function (obj, prop) {
+			get: function (_obj, prop) {
 				// Return a component factory for any icon that's requested
 				if (prop === "__esModule") {
 					return true

+ 373 - 0
webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.test.ts

@@ -0,0 +1,373 @@
+// npx jest src/components/ui/hooks/__tests__/useSelectedModel.test.ts
+
+import React from "react"
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
+import { renderHook } from "@testing-library/react"
+
+import { ProviderSettings, ModelInfo } from "@roo/shared/api"
+
+import { useSelectedModel } from "../useSelectedModel"
+import { useRouterModels } from "../useRouterModels"
+import { useOpenRouterModelProviders } from "../useOpenRouterModelProviders"
+
+jest.mock("../useRouterModels")
+jest.mock("../useOpenRouterModelProviders")
+
+const mockUseRouterModels = useRouterModels as jest.MockedFunction<typeof useRouterModels>
+const mockUseOpenRouterModelProviders = useOpenRouterModelProviders as jest.MockedFunction<
+	typeof useOpenRouterModelProviders
+>
+
+const createWrapper = () => {
+	const queryClient = new QueryClient({
+		defaultOptions: {
+			queries: {
+				retry: false,
+			},
+		},
+	})
+	return ({ children }: { children: React.ReactNode }) =>
+		React.createElement(QueryClientProvider, { client: queryClient }, children)
+}
+
+describe("useSelectedModel", () => {
+	describe("OpenRouter provider merging", () => {
+		it("should merge base model info with specific provider info when both exist", () => {
+			const baseModelInfo: ModelInfo = {
+				maxTokens: 4096,
+				contextWindow: 8192,
+				supportsImages: false,
+				supportsPromptCache: false,
+			}
+
+			const specificProviderInfo: ModelInfo = {
+				maxTokens: 8192, // Different value that should override
+				contextWindow: 16384, // Different value that should override
+				supportsImages: true, // Different value that should override
+				supportsPromptCache: true, // Different value that should override
+				inputPrice: 0.001,
+				outputPrice: 0.002,
+				description: "Provider-specific description",
+			}
+
+			mockUseRouterModels.mockReturnValue({
+				data: {
+					openrouter: {
+						"test-model": baseModelInfo,
+					},
+					requesty: {},
+					glama: {},
+					unbound: {},
+					litellm: {},
+				},
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			mockUseOpenRouterModelProviders.mockReturnValue({
+				data: {
+					"test-provider": specificProviderInfo,
+				},
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			const apiConfiguration: ProviderSettings = {
+				apiProvider: "openrouter",
+				openRouterModelId: "test-model",
+				openRouterSpecificProvider: "test-provider",
+			}
+
+			const wrapper = createWrapper()
+			const { result } = renderHook(() => useSelectedModel(apiConfiguration), { wrapper })
+
+			expect(result.current.id).toBe("test-model")
+			expect(result.current.info).toEqual({
+				maxTokens: 8192, // From specific provider (overrides base)
+				contextWindow: 16384, // From specific provider (overrides base)
+				supportsImages: true, // From specific provider (overrides base)
+				supportsPromptCache: true, // From specific provider (overrides base)
+				inputPrice: 0.001,
+				outputPrice: 0.002,
+				description: "Provider-specific description",
+			})
+		})
+
+		it("should use only specific provider info when base model info is missing", () => {
+			const specificProviderInfo: ModelInfo = {
+				maxTokens: 8192,
+				contextWindow: 16384,
+				supportsImages: true,
+				supportsPromptCache: true,
+				inputPrice: 0.001,
+				outputPrice: 0.002,
+				description: "Provider-specific description",
+			}
+
+			mockUseRouterModels.mockReturnValue({
+				data: {
+					openrouter: {},
+					requesty: {},
+					glama: {},
+					unbound: {},
+					litellm: {},
+				},
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			mockUseOpenRouterModelProviders.mockReturnValue({
+				data: {
+					"test-provider": specificProviderInfo,
+				},
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			const apiConfiguration: ProviderSettings = {
+				apiProvider: "openrouter",
+				openRouterModelId: "test-model",
+				openRouterSpecificProvider: "test-provider",
+			}
+
+			const wrapper = createWrapper()
+			const { result } = renderHook(() => useSelectedModel(apiConfiguration), { wrapper })
+
+			expect(result.current.id).toBe("test-model")
+			expect(result.current.info).toEqual(specificProviderInfo)
+		})
+
+		it("should demonstrate the merging behavior validates the comment about missing fields", () => {
+			const baseModelInfo: ModelInfo = {
+				maxTokens: 4096,
+				contextWindow: 8192,
+				supportsImages: false,
+				supportsPromptCache: false,
+				supportsComputerUse: true,
+				cacheWritesPrice: 0.1,
+				cacheReadsPrice: 0.01,
+			}
+
+			const specificProviderInfo: Partial<ModelInfo> = {
+				inputPrice: 0.001,
+				outputPrice: 0.002,
+				description: "Provider-specific description",
+				maxTokens: 8192, // Override this one
+				supportsImages: true, // Override this one
+			}
+
+			mockUseRouterModels.mockReturnValue({
+				data: {
+					openrouter: {
+						"test-model": baseModelInfo,
+					},
+					requesty: {},
+					glama: {},
+					unbound: {},
+					litellm: {},
+				},
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			mockUseOpenRouterModelProviders.mockReturnValue({
+				data: { "test-provider": specificProviderInfo as ModelInfo },
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			const apiConfiguration: ProviderSettings = {
+				apiProvider: "openrouter",
+				openRouterModelId: "test-model",
+				openRouterSpecificProvider: "test-provider",
+			}
+
+			const wrapper = createWrapper()
+			const { result } = renderHook(() => useSelectedModel(apiConfiguration), { wrapper })
+
+			expect(result.current.id).toBe("test-model")
+			expect(result.current.info).toEqual({
+				// Fields from base model that provider doesn't have
+				contextWindow: 8192, // From base (provider doesn't override)
+				supportsPromptCache: false, // From base (provider doesn't override)
+				supportsComputerUse: true, // From base (provider doesn't have)
+				cacheWritesPrice: 0.1, // From base (provider doesn't have)
+				cacheReadsPrice: 0.01, // From base (provider doesn't have)
+
+				// Fields overridden by provider
+				maxTokens: 8192, // From provider (overrides base)
+				supportsImages: true, // From provider (overrides base)
+
+				// Fields only in provider
+				inputPrice: 0.001, // From provider (base doesn't have)
+				outputPrice: 0.002, // From provider (base doesn't have)
+				description: "Provider-specific description", // From provider (base doesn't have)
+			})
+		})
+
+		it("should use base model info when no specific provider is configured", () => {
+			const baseModelInfo: ModelInfo = {
+				maxTokens: 4096,
+				contextWindow: 8192,
+				supportsImages: false,
+				supportsPromptCache: false,
+			}
+
+			mockUseRouterModels.mockReturnValue({
+				data: {
+					openrouter: { "test-model": baseModelInfo },
+					requesty: {},
+					glama: {},
+					unbound: {},
+					litellm: {},
+				},
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			mockUseOpenRouterModelProviders.mockReturnValue({
+				data: {},
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			const apiConfiguration: ProviderSettings = {
+				apiProvider: "openrouter",
+				openRouterModelId: "test-model",
+			}
+
+			const wrapper = createWrapper()
+			const { result } = renderHook(() => useSelectedModel(apiConfiguration), { wrapper })
+
+			expect(result.current.id).toBe("test-model")
+			expect(result.current.info).toEqual(baseModelInfo)
+		})
+
+		it("should fall back to default when both base and specific provider info are missing", () => {
+			mockUseRouterModels.mockReturnValue({
+				data: {
+					openrouter: {
+						"anthropic/claude-3.7-sonnet": {
+							// Default model
+							maxTokens: 4096,
+							contextWindow: 8192,
+							supportsImages: false,
+							supportsPromptCache: false,
+						},
+					},
+					requesty: {},
+					glama: {},
+					unbound: {},
+					litellm: {},
+				},
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			mockUseOpenRouterModelProviders.mockReturnValue({
+				data: {},
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			const apiConfiguration: ProviderSettings = {
+				apiProvider: "openrouter",
+				openRouterModelId: "non-existent-model",
+				openRouterSpecificProvider: "non-existent-provider",
+			}
+
+			const wrapper = createWrapper()
+			const { result } = renderHook(() => useSelectedModel(apiConfiguration), { wrapper })
+
+			expect(result.current.id).toBe("anthropic/claude-3.7-sonnet")
+			expect(result.current.info).toEqual({
+				maxTokens: 4096,
+				contextWindow: 8192,
+				supportsImages: false,
+				supportsPromptCache: false,
+			})
+		})
+	})
+
+	describe("loading and error states", () => {
+		it("should return loading state when router models are loading", () => {
+			mockUseRouterModels.mockReturnValue({
+				data: undefined,
+				isLoading: true,
+				isError: false,
+			} as any)
+
+			mockUseOpenRouterModelProviders.mockReturnValue({
+				data: undefined,
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			const wrapper = createWrapper()
+			const { result } = renderHook(() => useSelectedModel(), { wrapper })
+
+			expect(result.current.isLoading).toBe(true)
+		})
+
+		it("should return loading state when open router model providers are loading", () => {
+			mockUseRouterModels.mockReturnValue({
+				data: { openrouter: {}, requesty: {}, glama: {}, unbound: {}, litellm: {} },
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			mockUseOpenRouterModelProviders.mockReturnValue({
+				data: undefined,
+				isLoading: true,
+				isError: false,
+			} as any)
+
+			const wrapper = createWrapper()
+			const { result } = renderHook(() => useSelectedModel(), { wrapper })
+
+			expect(result.current.isLoading).toBe(true)
+		})
+
+		it("should return error state when either hook has an error", () => {
+			mockUseRouterModels.mockReturnValue({
+				data: undefined,
+				isLoading: false,
+				isError: true,
+			} as any)
+
+			mockUseOpenRouterModelProviders.mockReturnValue({
+				data: {},
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			const wrapper = createWrapper()
+			const { result } = renderHook(() => useSelectedModel(), { wrapper })
+
+			expect(result.current.isError).toBe(true)
+		})
+	})
+
+	describe("default behavior", () => {
+		it("should return anthropic default when no configuration is provided", () => {
+			mockUseRouterModels.mockReturnValue({
+				data: undefined,
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			mockUseOpenRouterModelProviders.mockReturnValue({
+				data: undefined,
+				isLoading: false,
+				isError: false,
+			} as any)
+
+			const wrapper = createWrapper()
+			const { result } = renderHook(() => useSelectedModel(), { wrapper })
+
+			expect(result.current.provider).toBe("anthropic")
+			expect(result.current.id).toBe("claude-3-7-sonnet-20250219")
+			expect(result.current.info).toBeUndefined()
+		})
+	})
+})

+ 6 - 1
webview-ui/src/components/ui/hooks/useSelectedModel.ts

@@ -82,7 +82,12 @@ function getSelectedModel({
 			const specificProvider = apiConfiguration.openRouterSpecificProvider
 
 			if (specificProvider && openRouterModelProviders[specificProvider]) {
-				info = openRouterModelProviders[specificProvider]
+				// Overwrite the info with the specific provider info. Some
+				// fields are missing the model info for `openRouterModelProviders`
+				// so we need to merge the two.
+				info = info
+					? { ...info, ...openRouterModelProviders[specificProvider] }
+					: openRouterModelProviders[specificProvider]
 			}
 
 			return info

+ 1 - 1
webview-ui/src/components/ui/markdown/CodeBlock.tsx

@@ -44,7 +44,7 @@ export const CodeBlock: FC<CodeBlockProps> = memo(({ language, value, className,
 				})
 
 				setHighlightedCode(html)
-			} catch (e) {
+			} catch (_e) {
 				setHighlightedCode(value)
 			}
 		}

+ 2 - 2
webview-ui/src/utils/TelemetryClient.ts

@@ -35,8 +35,8 @@ class TelemetryClient {
 		if (TelemetryClient.telemetryEnabled) {
 			try {
 				posthog.capture(eventName, properties)
-			} catch (error) {
-				// Silently fail if there's an error capturing an event
+			} catch (_error) {
+				// Silently fail if there's an error capturing an event.
 			}
 		}
 	}