diff --git a/webview-ui/src/components/settings/ModelPicker.tsx b/webview-ui/src/components/settings/ModelPicker.tsx index 3bb356f5f2..ae0ee01752 100644 --- a/webview-ui/src/components/settings/ModelPicker.tsx +++ b/webview-ui/src/components/settings/ModelPicker.tsx @@ -38,6 +38,7 @@ interface ModelPickerProps { apiConfiguration: ProviderSettings setApiConfigurationField: (field: K, value: ProviderSettings[K]) => void organizationAllowList: OrganizationAllowList + onOpenRefetch?: () => void } export const ModelPicker = ({ @@ -49,6 +50,7 @@ export const ModelPicker = ({ apiConfiguration, setApiConfigurationField, organizationAllowList, + onOpenRefetch, }: ModelPickerProps) => { const { t } = useAppTranslation() @@ -57,64 +59,78 @@ export const ModelPicker = ({ const searchInputRef = useRef(null) const currentConfiguredModelId = apiConfiguration[modelIdKey] + const [searchValue, setSearchValue] = useState(currentConfiguredModelId || "") const modelIdsForDropdown = useMemo(() => { const filteredModels = filterModels(models, apiConfiguration.apiProvider, organizationAllowList) return Object.keys(filteredModels ?? {}).sort((a, b) => a.localeCompare(b)) }, [models, apiConfiguration.apiProvider, organizationAllowList]) - const { id: selectedModelIdForInfo, info: selectedModelInfo } = useSelectedModel(apiConfiguration) - - const [searchValue, setSearchValue] = useState(currentConfiguredModelId || "") - + // Synchronize apiConfiguration and searchValue when models/selection changes + const currentIdInSettings = apiConfiguration[modelIdKey] useEffect(() => { - const currentIdInSettings = apiConfiguration[modelIdKey] - - if (!models || Object.keys(models).length === 0) { + if (!models || modelIdsForDropdown.length === 0) { + // Use modelIdsForDropdown for check after filtering if (currentIdInSettings !== undefined) { setApiConfigurationField(modelIdKey, undefined) } - if (searchValue !== "") setSearchValue("") } else { - const availableIds = Object.keys(models) let newIdToSet: string | undefined = undefined - - if (currentIdInSettings && availableIds.includes(currentIdInSettings)) { + if (currentIdInSettings && modelIdsForDropdown.includes(currentIdInSettings)) { newIdToSet = currentIdInSettings - } else if (availableIds.includes(defaultModelId)) { + } else if (modelIdsForDropdown.includes(defaultModelId)) { newIdToSet = defaultModelId - } else if (availableIds.length > 0) { - newIdToSet = availableIds[0] + } else { + newIdToSet = modelIdsForDropdown[0] // Fallback to the first available model } if (currentIdInSettings !== newIdToSet) { setApiConfigurationField(modelIdKey, newIdToSet) } - const targetSearchValue = newIdToSet || "" - if (searchValue !== targetSearchValue) setSearchValue(targetSearchValue) } - }, [models, apiConfiguration, searchValue, defaultModelId, modelIdKey, setApiConfigurationField]) + // This effect primarily ensures the configured ID is valid against the available models. + // SearchValue will be synced by another effect or callbacks. + }, [models, modelIdsForDropdown, currentIdInSettings, defaultModelId, modelIdKey, setApiConfigurationField]) + + // Effect to sync searchValue with currentConfiguredModelId. + // Primarily handles changes when the popover is closed. + // When open, user input and specific actions (onSelect, onClearSearch) manage searchValue. + // onOpenChange handles resetting searchValue when the popover closes. + useEffect(() => { + if (!open) { + // Only act if the popover is closed + // If currentConfiguredModelId has changed and searchValue is out of sync, update it. + // Also handles if searchValue somehow changed while closed. + if (searchValue !== (currentConfiguredModelId || "")) { + setSearchValue(currentConfiguredModelId || "") + } + } + // When 'open' is true, do nothing here to allow user input to control searchValue. + }, [currentConfiguredModelId, open, searchValue]) // Rerun if currentConfiguredModelId changes or popover opens/closes + + const { id: selectedModelIdForInfo, info: selectedModelInfo } = useSelectedModel(apiConfiguration) const onSelect = useCallback( (modelId: string) => { - if (!modelId) { - return - } + if (!modelId) return + setApiConfigurationField(modelIdKey, modelId) // This will trigger currentConfiguredModelId update + setSearchValue(modelId) // Directly set search for immediate feedback in closed popover setOpen(false) - setApiConfigurationField(modelIdKey, modelId) - setSearchValue(modelId) }, [modelIdKey, setApiConfigurationField], ) const onOpenChange = useCallback( - (open: boolean) => { - setOpen(open) - if (!open) { - setSearchValue(apiConfiguration[modelIdKey] || "") + (newOpenState: boolean) => { + setOpen(newOpenState) + if (newOpenState && onOpenRefetch) { + onOpenRefetch() + } + if (!newOpenState) { + setSearchValue(currentConfiguredModelId || "") } }, - [apiConfiguration, modelIdKey], + [currentConfiguredModelId, onOpenRefetch], ) const onClearSearch = useCallback(() => { @@ -132,8 +148,13 @@ export const ModelPicker = ({ variant="combobox" role="combobox" aria-expanded={open} - className="w-full justify-between"> -
{currentConfiguredModelId ?? t("settings:common.select")}
+ className="w-full justify-between" + disabled={modelIdsForDropdown.length === 0}> +
+ {modelIdsForDropdown.length === 0 + ? "" + : (currentConfiguredModelId ?? t("settings:common.select"))} +
@@ -142,8 +163,8 @@ export const ModelPicker = ({
{modelIdsForDropdown.map((model) => ( - + onSelect(model)}> {model} {searchValue && !modelIdsForDropdown.includes(searchValue) && (
- + onSelect(searchValue)}> {t("settings:modelPicker.useCustomModel", { modelId: searchValue })}
diff --git a/webview-ui/src/components/settings/providers/LiteLLM.tsx b/webview-ui/src/components/settings/providers/LiteLLM.tsx index 65a3cb7575..d84792e76e 100644 --- a/webview-ui/src/components/settings/providers/LiteLLM.tsx +++ b/webview-ui/src/components/settings/providers/LiteLLM.tsx @@ -33,6 +33,7 @@ export const LiteLLM = ({ apiConfiguration, setApiConfigurationField, organizati models: litellmModelsData, isLoading: isLoadingModels, error: modelsError, + refetch: refetchLiteLLMModels, } = useProviderModels("litellm", providerModelsOptions) console.log("litellmModelsData1212", litellmModelsData, isLoadingModels, modelsError) @@ -87,6 +88,7 @@ export const LiteLLM = ({ apiConfiguration, setApiConfigurationField, organizati serviceUrl="https://docs.litellm.ai/" setApiConfigurationField={setApiConfigurationField} organizationAllowList={organizationAllowList} + onOpenRefetch={refetchLiteLLMModels} /> ) diff --git a/webview-ui/src/utils/validate.ts b/webview-ui/src/utils/validate.ts index 5122ca58d4..552cae8381 100644 --- a/webview-ui/src/utils/validate.ts +++ b/webview-ui/src/utils/validate.ts @@ -2,11 +2,10 @@ import i18next from "i18next" import type { ProviderSettings, OrganizationAllowList } from "@roo-code/types" -import { isRouterName, RouterModels } from "@roo/api" +// import { isRouterName } from "@roo/api" // Removed as it's no longer used export function validateApiConfiguration( apiConfiguration: ProviderSettings, - routerModels?: RouterModels, organizationAllowList?: OrganizationAllowList, ): string | undefined { const keysAndIdsPresentErrorMessage = validateModelsAndKeysProvided(apiConfiguration) @@ -22,88 +21,67 @@ export function validateApiConfiguration( return organizationAllowListErrorMessage } - return validateModelId(apiConfiguration, routerModels) + return undefined } function validateModelsAndKeysProvided(apiConfiguration: ProviderSettings): string | undefined { - switch (apiConfiguration.apiProvider) { + const { apiProvider } = apiConfiguration + + switch (apiProvider) { case "openrouter": - if (!apiConfiguration.openRouterApiKey) { - return i18next.t("settings:validation.apiKey") - } + if (!apiConfiguration.openRouterApiKey) return i18next.t("settings:validation.apiKey") + if (!apiConfiguration.openRouterModelId) return i18next.t("settings:validation.modelId") break case "glama": - if (!apiConfiguration.glamaApiKey) { - return i18next.t("settings:validation.apiKey") - } + if (!apiConfiguration.glamaApiKey) return i18next.t("settings:validation.apiKey") + if (!apiConfiguration.glamaModelId) return i18next.t("settings:validation.modelId") break case "unbound": - if (!apiConfiguration.unboundApiKey) { - return i18next.t("settings:validation.apiKey") - } + if (!apiConfiguration.unboundApiKey) return i18next.t("settings:validation.apiKey") + if (!apiConfiguration.unboundModelId) return i18next.t("settings:validation.modelId") break case "requesty": - if (!apiConfiguration.requestyApiKey) { - return i18next.t("settings:validation.apiKey") - } + if (!apiConfiguration.requestyApiKey) return i18next.t("settings:validation.apiKey") + if (!apiConfiguration.requestyModelId) return i18next.t("settings:validation.modelId") break case "litellm": - if (!apiConfiguration.litellmApiKey) { - return i18next.t("settings:validation.apiKey") - } + if (!apiConfiguration.litellmApiKey) return i18next.t("settings:validation.apiKey") + if (!apiConfiguration.litellmModelId) return i18next.t("settings:validation.modelId") break - case "anthropic": - if (!apiConfiguration.apiKey) { - return i18next.t("settings:validation.apiKey") - } - break - case "bedrock": - if (!apiConfiguration.awsRegion) { - return i18next.t("settings:validation.awsRegion") - } - break - case "vertex": - if (!apiConfiguration.vertexProjectId || !apiConfiguration.vertexRegion) { - return i18next.t("settings:validation.googleCloud") - } - break - case "gemini": - if (!apiConfiguration.geminiApiKey) { - return i18next.t("settings:validation.apiKey") - } - break - case "openai-native": - if (!apiConfiguration.openAiNativeApiKey) { - return i18next.t("settings:validation.apiKey") - } - break - case "mistral": - if (!apiConfiguration.mistralApiKey) { - return i18next.t("settings:validation.apiKey") - } - break - case "openai": + case "openai": // This is openai-compatible router if (!apiConfiguration.openAiBaseUrl || !apiConfiguration.openAiApiKey || !apiConfiguration.openAiModelId) { return i18next.t("settings:validation.openAi") } break case "ollama": - if (!apiConfiguration.ollamaModelId) { - return i18next.t("settings:validation.modelId") - } + if (!apiConfiguration.ollamaModelId) return i18next.t("settings:validation.modelId") break case "lmstudio": - if (!apiConfiguration.lmStudioModelId) { - return i18next.t("settings:validation.modelId") - } + if (!apiConfiguration.lmStudioModelId) return i18next.t("settings:validation.modelId") break case "vscode-lm": - if (!apiConfiguration.vsCodeLmModelSelector) { - return i18next.t("settings:validation.modelSelector") - } + if (!apiConfiguration.vsCodeLmModelSelector) return i18next.t("settings:validation.modelSelector") + break + case "anthropic": + if (!apiConfiguration.apiKey) return i18next.t("settings:validation.apiKey") + break + case "bedrock": + if (!apiConfiguration.awsRegion) return i18next.t("settings:validation.awsRegion") + break + case "vertex": + if (!apiConfiguration.vertexProjectId || !apiConfiguration.vertexRegion) + return i18next.t("settings:validation.googleCloud") + break + case "gemini": + if (!apiConfiguration.geminiApiKey) return i18next.t("settings:validation.apiKey") + break + case "openai-native": + if (!apiConfiguration.openAiNativeApiKey) return i18next.t("settings:validation.apiKey") + break + case "mistral": + if (!apiConfiguration.mistralApiKey) return i18next.t("settings:validation.apiKey") break } - return undefined } @@ -132,6 +110,7 @@ function validateProviderAgainstOrganizationSettings( } } } + return undefined } function getModelIdForProvider(apiConfiguration: ProviderSettings, provider: string): string | undefined { @@ -147,26 +126,19 @@ function getModelIdForProvider(apiConfiguration: ProviderSettings, provider: str case "litellm": return apiConfiguration.litellmModelId case "openai": - return apiConfiguration.openAiModelId + return apiConfiguration.openAiModelId // openai-compatible case "ollama": return apiConfiguration.ollamaModelId case "lmstudio": return apiConfiguration.lmStudioModelId case "vscode-lm": - // vsCodeLmModelSelector is an object, not a string return apiConfiguration.vsCodeLmModelSelector?.id default: return apiConfiguration.apiModelId } } -/** - * Validates an Amazon Bedrock ARN format and optionally checks if the region in the ARN matches the provided region - * @param arn The ARN string to validate - * @param region Optional region to check against the ARN's region - * @returns An object with validation results: { isValid, arnRegion, errorMessage } - */ + export function validateBedrockArn(arn: string, region?: string) { - // Validate ARN format const arnRegex = /^arn:aws:(?:bedrock|sagemaker):([^:]+):([^:]*):(?:([^/]+)\/([\w.\-:]+)|([^/]+))$/ const match = arn.match(arnRegex) @@ -178,10 +150,8 @@ export function validateBedrockArn(arn: string, region?: string) { } } - // Extract region from ARN const arnRegion = match[1] - // Check if region in ARN matches provided region (if specified) if (region && arnRegion !== region) { return { isValid: true, @@ -190,46 +160,5 @@ export function validateBedrockArn(arn: string, region?: string) { } } - // ARN is valid and region matches (or no region was provided to check against) return { isValid: true, arnRegion, errorMessage: undefined } } - -export function validateModelId(apiConfiguration: ProviderSettings, routerModels?: RouterModels): string | undefined { - const provider = apiConfiguration.apiProvider ?? "" - - if (!isRouterName(provider)) { - return undefined - } - - let modelId: string | undefined - - switch (provider) { - case "openrouter": - modelId = apiConfiguration.openRouterModelId - break - case "glama": - modelId = apiConfiguration.glamaModelId - break - case "unbound": - modelId = apiConfiguration.unboundModelId - break - case "requesty": - modelId = apiConfiguration.requestyModelId - break - case "litellm": - modelId = apiConfiguration.litellmModelId - break - } - - if (!modelId) { - return i18next.t("settings:validation.modelId") - } - - const models = routerModels?.[provider] - - if (models && Object.keys(models).length > 1 && !Object.keys(models).includes(modelId)) { - return i18next.t("settings:validation.modelAvailability", { modelId }) - } - - return undefined -}