From 68de733888cf497be14c40c48974853527075f60 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 14 Aug 2025 18:04:15 -0500 Subject: [PATCH 1/3] fix: enable save button for provider dropdown and checkbox changes The save button wasn't enabling when users changed provider settings like dropdowns and checkboxes because setApiConfigurationField was treating all changes from undefined to a defined value as 'initial sync' and not marking the form as dirty. Added an optional isUserAction parameter (defaults to true) to distinguish: - User actions (should enable save button) - the default - Automatic initialization (shouldn't enable save button) - pass false This fixes the issue where changing provider dropdowns, checkboxes with default values, and other settings wouldn't enable the save button. --- webview-ui/src/components/settings/ApiOptions.tsx | 10 +++++++--- webview-ui/src/components/settings/ModelPicker.tsx | 8 ++++++-- webview-ui/src/components/settings/SettingsView.tsx | 8 ++++---- webview-ui/src/components/settings/ThinkingBudget.tsx | 10 +++++++--- .../src/components/settings/providers/HuggingFace.tsx | 8 ++++++-- .../src/components/settings/providers/Unbound.tsx | 8 ++++++-- 6 files changed, 36 insertions(+), 16 deletions(-) diff --git a/webview-ui/src/components/settings/ApiOptions.tsx b/webview-ui/src/components/settings/ApiOptions.tsx index dcdf072a11c..56ed40ebecf 100644 --- a/webview-ui/src/components/settings/ApiOptions.tsx +++ b/webview-ui/src/components/settings/ApiOptions.tsx @@ -105,7 +105,11 @@ import { buildDocLink } from "@src/utils/docLinks" export interface ApiOptionsProps { uriScheme: string | undefined apiConfiguration: ProviderSettings - setApiConfigurationField: (field: K, value: ProviderSettings[K]) => void + setApiConfigurationField: ( + field: K, + value: ProviderSettings[K], + isUserAction?: boolean, + ) => void fromWelcomeView?: boolean errorMessage: string | undefined setErrorMessage: React.Dispatch> @@ -186,7 +190,7 @@ const ApiOptions = ({ // Update `apiModelId` whenever `selectedModelId` changes. useEffect(() => { if (selectedModelId && apiConfiguration.apiModelId !== selectedModelId) { - setApiConfigurationField("apiModelId", selectedModelId) + setApiConfigurationField("apiModelId", selectedModelId, false) } }, [selectedModelId, setApiConfigurationField, apiConfiguration.apiModelId]) @@ -280,7 +284,7 @@ const ApiOptions = ({ const shouldSetDefault = !modelId if (shouldSetDefault) { - setApiConfigurationField(field, defaultValue) + setApiConfigurationField(field, defaultValue, false) } } diff --git a/webview-ui/src/components/settings/ModelPicker.tsx b/webview-ui/src/components/settings/ModelPicker.tsx index 2160d4b3c55..0753f9fc2bd 100644 --- a/webview-ui/src/components/settings/ModelPicker.tsx +++ b/webview-ui/src/components/settings/ModelPicker.tsx @@ -46,7 +46,11 @@ interface ModelPickerProps { serviceName: string serviceUrl: string apiConfiguration: ProviderSettings - setApiConfigurationField: (field: K, value: ProviderSettings[K]) => void + setApiConfigurationField: ( + field: K, + value: ProviderSettings[K], + isUserAction?: boolean, + ) => void organizationAllowList: OrganizationAllowList errorMessage?: string } @@ -124,7 +128,7 @@ export const ModelPicker = ({ useEffect(() => { if (!selectedModelId && !isInitialized.current) { const initialValue = modelIds.includes(selectedModelId) ? selectedModelId : defaultModelId - setApiConfigurationField(modelIdKey, initialValue) + setApiConfigurationField(modelIdKey, initialValue, false) // false = automatic initialization } isInitialized.current = true diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 987d2451059..9866c1e235a 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -219,7 +219,7 @@ const SettingsView = forwardRef(({ onDone, t }, []) const setApiConfigurationField = useCallback( - (field: K, value: ProviderSettings[K]) => { + (field: K, value: ProviderSettings[K], isUserAction: boolean = true) => { setCachedState((prevState) => { if (prevState.apiConfiguration?.[field] === value) { return prevState @@ -227,9 +227,9 @@ const SettingsView = forwardRef(({ onDone, t const previousValue = prevState.apiConfiguration?.[field] - // Don't treat initial sync from undefined to a defined value as a user change - // This prevents the dirty state when the component initializes and auto-syncs the model ID - const isInitialSync = previousValue === undefined && value !== undefined + // Only skip change detection for automatic initialization (not user actions) + // This prevents the dirty state when the component initializes and auto-syncs values + const isInitialSync = !isUserAction && previousValue === undefined && value !== undefined if (!isInitialSync) { setChangeDetected(true) diff --git a/webview-ui/src/components/settings/ThinkingBudget.tsx b/webview-ui/src/components/settings/ThinkingBudget.tsx index c2aa2349830..7a85e61e7a5 100644 --- a/webview-ui/src/components/settings/ThinkingBudget.tsx +++ b/webview-ui/src/components/settings/ThinkingBudget.tsx @@ -20,7 +20,11 @@ import { useSelectedModel } from "@src/components/ui/hooks/useSelectedModel" interface ThinkingBudgetProps { apiConfiguration: ProviderSettings - setApiConfigurationField: (field: K, value: ProviderSettings[K]) => void + setApiConfigurationField: ( + field: K, + value: ProviderSettings[K], + isUserAction?: boolean, + ) => void modelInfo?: ModelInfo } @@ -71,7 +75,7 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod // Set default reasoning effort when model supports it and no value is set useEffect(() => { if (isReasoningEffortSupported && !apiConfiguration.reasoningEffort && defaultReasoningEffort) { - setApiConfigurationField("reasoningEffort", defaultReasoningEffort) + setApiConfigurationField("reasoningEffort", defaultReasoningEffort, false) } }, [isReasoningEffortSupported, apiConfiguration.reasoningEffort, defaultReasoningEffort, setApiConfigurationField]) @@ -91,7 +95,7 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod // appropriately. useEffect(() => { if (isReasoningBudgetSupported && customMaxThinkingTokens > modelMaxThinkingTokens) { - setApiConfigurationField("modelMaxThinkingTokens", modelMaxThinkingTokens) + setApiConfigurationField("modelMaxThinkingTokens", modelMaxThinkingTokens, false) } }, [isReasoningBudgetSupported, customMaxThinkingTokens, modelMaxThinkingTokens, setApiConfigurationField]) diff --git a/webview-ui/src/components/settings/providers/HuggingFace.tsx b/webview-ui/src/components/settings/providers/HuggingFace.tsx index 8716739d80b..94f60cc4c82 100644 --- a/webview-ui/src/components/settings/providers/HuggingFace.tsx +++ b/webview-ui/src/components/settings/providers/HuggingFace.tsx @@ -34,7 +34,11 @@ type HuggingFaceModel = { type HuggingFaceProps = { apiConfiguration: ProviderSettings - setApiConfigurationField: (field: keyof ProviderSettings, value: ProviderSettings[keyof ProviderSettings]) => void + setApiConfigurationField: ( + field: keyof ProviderSettings, + value: ProviderSettings[keyof ProviderSettings], + isUserAction?: boolean, + ) => void } export const HuggingFace = ({ apiConfiguration, setApiConfigurationField }: HuggingFaceProps) => { @@ -93,7 +97,7 @@ export const HuggingFace = ({ apiConfiguration, setApiConfigurationField }: Hugg // Set to "auto" as default const defaultProvider = "auto" setSelectedProvider(defaultProvider) - setApiConfigurationField("huggingFaceInferenceProvider", defaultProvider) + setApiConfigurationField("huggingFaceInferenceProvider", defaultProvider, false) // false = automatic default } } } diff --git a/webview-ui/src/components/settings/providers/Unbound.tsx b/webview-ui/src/components/settings/providers/Unbound.tsx index a242ee4b3f1..2dd105a6637 100644 --- a/webview-ui/src/components/settings/providers/Unbound.tsx +++ b/webview-ui/src/components/settings/providers/Unbound.tsx @@ -17,7 +17,11 @@ import { ModelPicker } from "../ModelPicker" type UnboundProps = { apiConfiguration: ProviderSettings - setApiConfigurationField: (field: keyof ProviderSettings, value: ProviderSettings[keyof ProviderSettings]) => void + setApiConfigurationField: ( + field: keyof ProviderSettings, + value: ProviderSettings[keyof ProviderSettings], + isUserAction?: boolean, + ) => void routerModels?: RouterModels organizationAllowList: OrganizationAllowList modelValidationError?: string @@ -110,7 +114,7 @@ export const Unbound = ({ if (!currentModelId || !modelExists) { const firstAvailableModelId = Object.keys(updatedModels)[0] - setApiConfigurationField("unboundModelId", firstAvailableModelId) + setApiConfigurationField("unboundModelId", firstAvailableModelId, false) // false = automatic model selection } } From a07ac9ae5b030d7a5e63e90a33916ea9bdb2004c Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 14 Aug 2025 18:14:26 -0500 Subject: [PATCH 2/3] fix: remove incorrect isUserAction=false from apiModelId sync The useEffect that syncs apiModelId with selectedModelId was incorrectly passing isUserAction=false, which prevented the save button from enabling when users selected a different model. Since this effect responds to all selectedModelId changes (including user selections), it should use the default isUserAction=true behavior. --- webview-ui/src/components/settings/ApiOptions.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webview-ui/src/components/settings/ApiOptions.tsx b/webview-ui/src/components/settings/ApiOptions.tsx index 56ed40ebecf..b51b1713543 100644 --- a/webview-ui/src/components/settings/ApiOptions.tsx +++ b/webview-ui/src/components/settings/ApiOptions.tsx @@ -190,7 +190,7 @@ const ApiOptions = ({ // Update `apiModelId` whenever `selectedModelId` changes. useEffect(() => { if (selectedModelId && apiConfiguration.apiModelId !== selectedModelId) { - setApiConfigurationField("apiModelId", selectedModelId, false) + setApiConfigurationField("apiModelId", selectedModelId) } }, [selectedModelId, setApiConfigurationField, apiConfiguration.apiModelId]) From 55f7792f04fd0f6363d3ed1bedcb9df5fdaac063 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 14 Aug 2025 18:22:03 -0500 Subject: [PATCH 3/3] test: fix ThinkingBudget test to expect isUserAction parameter The test now correctly expects setApiConfigurationField to be called with three arguments including the isUserAction=false parameter for automatic thinking token adjustments. --- .../src/components/settings/__tests__/ThinkingBudget.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx b/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx index fa7493edc66..9bc235e5f00 100644 --- a/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/ThinkingBudget.spec.tsx @@ -112,7 +112,7 @@ describe("ThinkingBudget", () => { ) // Effect should trigger and cap the value - expect(setApiConfigurationField).toHaveBeenCalledWith("modelMaxThinkingTokens", 8000) // 80% of 10000 + expect(setApiConfigurationField).toHaveBeenCalledWith("modelMaxThinkingTokens", 8000, false) // 80% of 10000 }) it("should use default thinking tokens if not provided", () => {