diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index fb3ec1fd21..1e900db45d 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -582,8 +582,8 @@ export interface WebviewMessage { // eslint-disable-next-line @typescript-eslint/no-explicit-any settings?: any url?: string // For openExternal - ollamaBaseUrl?: string // For testOllamaConnection - allows passing current value from UI - ollamaApiKey?: string // For testOllamaConnection - allows passing current value from UI + ollamaBaseUrl?: string // For testOllamaConnection and refreshOllamaModels - allows passing current value from UI + ollamaApiKey?: string // For testOllamaConnection and refreshOllamaModels - allows passing current value from UI mpItem?: MarketplaceItem mpInstallOptions?: InstallMarketplaceItemOptions // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 52dc8304fd..cd06c90494 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -1059,11 +1059,16 @@ export const webviewMessageHandler = async ( const { apiConfiguration: ollamaApiConfig } = await provider.getState() const startTime = Date.now() + // Use the baseUrl and apiKey from the message if provided (current UI values), + // otherwise fall back to saved state + const baseUrl = message.ollamaBaseUrl ?? ollamaApiConfig.ollamaBaseUrl + const apiKey = message.ollamaApiKey ?? ollamaApiConfig.ollamaApiKey + try { const ollamaOptions = { provider: "ollama" as const, - baseUrl: ollamaApiConfig.ollamaBaseUrl, - apiKey: ollamaApiConfig.ollamaApiKey, + baseUrl: baseUrl, + apiKey: apiKey, ollamaModelDiscoveryTimeout: ollamaApiConfig.ollamaModelDiscoveryTimeout, ollamaMaxRetries: ollamaApiConfig.ollamaMaxRetries, ollamaRetryDelay: ollamaApiConfig.ollamaRetryDelay, @@ -1072,16 +1077,12 @@ export const webviewMessageHandler = async ( await flushModels(ollamaOptions, true) - const result = await discoverOllamaModelsWithSorting( - ollamaApiConfig.ollamaBaseUrl, - ollamaApiConfig.ollamaApiKey, - { - modelDiscoveryTimeout: ollamaApiConfig.ollamaModelDiscoveryTimeout, - maxRetries: ollamaApiConfig.ollamaMaxRetries, - retryDelay: ollamaApiConfig.ollamaRetryDelay, - enableLogging: ollamaApiConfig.ollamaEnableLogging, - }, - ) + const result = await discoverOllamaModelsWithSorting(baseUrl, apiKey, { + modelDiscoveryTimeout: ollamaApiConfig.ollamaModelDiscoveryTimeout, + maxRetries: ollamaApiConfig.ollamaMaxRetries, + retryDelay: ollamaApiConfig.ollamaRetryDelay, + enableLogging: ollamaApiConfig.ollamaEnableLogging, + }) const durationMs = Date.now() - startTime @@ -1093,7 +1094,7 @@ export const webviewMessageHandler = async ( if (ollamaApiConfig.ollamaEnableLogging) { console.debug("[Ollama Model Refresh]", { - baseUrl: ollamaApiConfig.ollamaBaseUrl, + baseUrl: baseUrl, modelsWithTools: result.modelsWithTools.length, modelsWithoutTools: result.modelsWithoutTools.length, totalCount: result.totalCount, diff --git a/webview-ui/src/components/settings/providers/Ollama.tsx b/webview-ui/src/components/settings/providers/Ollama.tsx index ca2dfc328a..5db3d7e31b 100644 --- a/webview-ui/src/components/settings/providers/Ollama.tsx +++ b/webview-ui/src/components/settings/providers/Ollama.tsx @@ -127,8 +127,12 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro const handleRefreshModels = useCallback(() => { setRefreshingModels(true) setRefreshResult(null) - vscode.postMessage({ type: "refreshOllamaModels" }) - }, []) + vscode.postMessage({ + type: "refreshOllamaModels", + ollamaBaseUrl: apiConfiguration?.ollamaBaseUrl || "", + ollamaApiKey: apiConfiguration?.ollamaApiKey || "", + }) + }, [apiConfiguration?.ollamaBaseUrl, apiConfiguration?.ollamaApiKey]) useEffect(() => { return () => { @@ -170,6 +174,11 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro return false }, [apiConfiguration?.ollamaModelId, routerModels.data, ollamaModels]) + // Sort models with tools by name for consistent ordering + const sortedModelsWithTools = useMemo(() => { + return [...modelsWithTools].sort((a, b) => a.name.localeCompare(b.name)) + }, [modelsWithTools]) + return ( <>
@@ -249,7 +258,7 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro
{t("settings:providers.ollama.toolsSupport")} ({modelsWithTools.length}{" "} - {t("settings:providers.ollama.models")}) + {t("settings:providers.ollama.models", { count: modelsWithTools.length })})
- {modelsWithTools.map((model) => { + {sortedModelsWithTools.map((model) => { const formatSize = (bytes?: number): string => { if (!bytes) return "-" const gb = bytes / (1024 * 1024 * 1024) @@ -333,7 +342,7 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro
{t("settings:providers.ollama.noToolsSupport")} ({modelsWithoutTools.length}{" "} - {t("settings:providers.ollama.models")}) + {t("settings:providers.ollama.models", { count: modelsWithoutTools.length })})
{t("settings:providers.ollama.noToolsSupportHelp")} diff --git a/webview-ui/src/i18n/__tests__/ollama-coverage.spec.ts b/webview-ui/src/i18n/__tests__/ollama-coverage.spec.ts new file mode 100644 index 0000000000..b26c194ad5 --- /dev/null +++ b/webview-ui/src/i18n/__tests__/ollama-coverage.spec.ts @@ -0,0 +1,190 @@ +import { describe, it, expect } from "vitest" +import fs from "fs" +import path from "path" +import { fileURLToPath } from "url" + +// Get directory path for ES modules +const __filename = fileURLToPath(import.meta.url) +const __dirname = path.dirname(__filename) + +// Import English as the reference (source of truth) +import enSettings from "../locales/en/settings.json" + +// All target locales (17 non-English locales) +const locales = [ + "ca", // Catalan + "de", // German + "es", // Spanish + "fr", // French + "hi", // Hindi + "id", // Indonesian + "it", // Italian + "ja", // Japanese + "ko", // Korean + "nl", // Dutch + "pl", // Polish + "pt-BR", // Portuguese (Brazil) + "ru", // Russian + "tr", // Turkish + "vi", // Vietnamese + "zh-CN", // Chinese (Simplified) + "zh-TW", // Chinese (Traditional) +] + +describe("Ollama i18n Coverage", () => { + // Get all keys from English (source of truth) + const enOllamaKeys = Object.keys(enSettings.providers.ollama) + + locales.forEach((locale) => { + it(`should have all Ollama keys for ${locale}`, () => { + const filePath = path.join(__dirname, `../locales/${locale}/settings.json`) + const fileContent = fs.readFileSync(filePath, "utf-8") + const localeSettings = JSON.parse(fileContent) + + // Verify providers.ollama exists + expect(localeSettings.providers).toBeDefined() + expect(localeSettings.providers.ollama).toBeDefined() + + const localeOllamaKeys = Object.keys(localeSettings.providers.ollama) + + // Check that all English keys exist in the locale + enOllamaKeys.forEach((key) => { + expect(localeOllamaKeys, `Missing key "${key}" in ${locale}/settings.json`).toContain(key) + }) + }) + + it(`should have valid JSON structure for ${locale}`, () => { + const filePath = path.join(__dirname, `../locales/${locale}/settings.json`) + const fileContent = fs.readFileSync(filePath, "utf-8") + + // Should parse without errors + expect(() => JSON.parse(fileContent)).not.toThrow() + + const localeSettings = JSON.parse(fileContent) + + // Verify structure + expect(localeSettings).toBeDefined() + expect(localeSettings.providers).toBeDefined() + expect(localeSettings.providers.ollama).toBeDefined() + expect(typeof localeSettings.providers.ollama).toBe("object") + }) + + it(`should have "models" key as plural object for ${locale}`, () => { + const filePath = path.join(__dirname, `../locales/${locale}/settings.json`) + const fileContent = fs.readFileSync(filePath, "utf-8") + const localeSettings = JSON.parse(fileContent) + + const modelsKey = localeSettings.providers.ollama.models + + // Should be an object (plural form), not a string + expect(typeof modelsKey).toBe("object") + expect(modelsKey).not.toBeNull() + + // Should have at least "one" and "other" keys + expect(modelsKey).toHaveProperty("one") + expect(modelsKey).toHaveProperty("other") + + // Russian and Polish should have additional plural forms + if (locale === "ru" || locale === "pl") { + expect(modelsKey).toHaveProperty("few") + expect(modelsKey).toHaveProperty("many") + } + + // All values should be strings + Object.values(modelsKey).forEach((value) => { + expect(typeof value).toBe("string") + expect(value).not.toBe("") + }) + }) + }) + + it("should have consistent key count across all locales", () => { + const keyCounts = locales.map((locale) => { + const filePath = path.join(__dirname, `../locales/${locale}/settings.json`) + const fileContent = fs.readFileSync(filePath, "utf-8") + const localeSettings = JSON.parse(fileContent) + return Object.keys(localeSettings.providers.ollama).length + }) + + const enKeyCount = enOllamaKeys.length + + // All locales should have the same number of keys as English + keyCounts.forEach((count, index) => { + expect(count, `${locales[index]} has ${count} keys, but English has ${enKeyCount} keys`).toBe(enKeyCount) + }) + }) + + // Test common terms translations (Provider, Base, Model) + describe("Common Terms Translation Coverage", () => { + // English words that should not appear in non-English locales + const englishWords = ["Provider", "Providers", "Base", "Model", "URL", "ID"] + + locales.forEach((locale) => { + it(`should have translated common terms for ${locale}`, () => { + const filePath = path.join(__dirname, `../locales/${locale}/settings.json`) + const fileContent = fs.readFileSync(filePath, "utf-8") + const localeSettings = JSON.parse(fileContent) + + // Check sections.providers + expect(localeSettings.sections).toBeDefined() + expect(localeSettings.sections.providers).toBeDefined() + expect(typeof localeSettings.sections.providers).toBe("string") + expect(localeSettings.sections.providers).not.toBe("") + expect(localeSettings.sections.providers).not.toBe("Provider") + expect(localeSettings.sections.providers).not.toBe("Providers") + + // Check providers.ollama.baseUrl + expect(localeSettings.providers).toBeDefined() + expect(localeSettings.providers.ollama).toBeDefined() + expect(localeSettings.providers.ollama.baseUrl).toBeDefined() + expect(typeof localeSettings.providers.ollama.baseUrl).toBe("string") + expect(localeSettings.providers.ollama.baseUrl).not.toBe("") + // Should not be exactly "Base URL (optional)" or "Base URL (opsional)" + expect(localeSettings.providers.ollama.baseUrl).not.toMatch(/^Base URL/i) + + // Check providers.ollama.modelId + expect(localeSettings.providers.ollama.modelId).toBeDefined() + expect(typeof localeSettings.providers.ollama.modelId).toBe("string") + expect(localeSettings.providers.ollama.modelId).not.toBe("") + // Should not be exactly "Model ID" + expect(localeSettings.providers.ollama.modelId).not.toMatch(/^Model ID$/i) + }) + + it(`should not contain English words in common terms for ${locale}`, () => { + const filePath = path.join(__dirname, `../locales/${locale}/settings.json`) + const fileContent = fs.readFileSync(filePath, "utf-8") + const localeSettings = JSON.parse(fileContent) + + const providerValue = localeSettings.sections.providers + const baseUrlValue = localeSettings.providers.ollama.baseUrl + const modelIdValue = localeSettings.providers.ollama.modelId + + // Check that common terms don't contain standalone English words + // (Allow "URL" and "ID" as they are technical acronyms commonly used) + const providerHasEnglish = englishWords + .filter((word) => word !== "URL" && word !== "ID") + .some((word) => providerValue === word || providerValue.startsWith(word + " ")) + + const baseUrlHasEnglish = englishWords + .filter((word) => word !== "URL" && word !== "ID") + .some((word) => baseUrlValue.includes(word) && !baseUrlValue.includes("URL")) + + const modelIdHasEnglish = englishWords + .filter((word) => word !== "URL" && word !== "ID") + .some((word) => modelIdValue.includes(word) && !modelIdValue.includes("ID")) + + // Note: This is a soft check - some languages may legitimately use English technical terms + // The main check is that the values are not exactly the English defaults + if (providerHasEnglish) { + console.warn(`${locale}: "sections.providers" may contain English word: "${providerValue}"`) + } + if (baseUrlHasEnglish) { + console.warn(`${locale}: "providers.ollama.baseUrl" may contain English word: "${baseUrlValue}"`) + } + if (modelIdHasEnglish) { + console.warn(`${locale}: "providers.ollama.modelId" may contain English word: "${modelIdValue}"`) + } + }) + }) + }) +})