Skip to content

Commit f92e4b9

Browse files
committed
Enhance LiteLLM provider configuration and default handling
- Updated ApiOptions to ensure model-specific IDs are set correctly when switching to the LiteLLM provider. - Added useEffect in SettingsView to set default values for LiteLLM configuration fields if they are missing. - Improved error handling in LiteLLM component for missing API key and base URL during model refresh. - Added missing configuration error messages in multiple languages for better user feedback.
1 parent 9546dc7 commit f92e4b9

File tree

22 files changed

+202
-41
lines changed

22 files changed

+202
-41
lines changed

webview-ui/src/components/settings/ApiOptions.tsx

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -263,16 +263,29 @@ const ApiOptions = ({
263263
setApiConfigurationField("requestyModelId", requestyDefaultModelId)
264264
}
265265
break
266-
case "litellm":
267-
if (!apiConfiguration.litellmModelId) {
266+
case "litellm": {
267+
let currentLitellmModelId = apiConfiguration.litellmModelId
268+
if (!currentLitellmModelId) {
268269
setApiConfigurationField("litellmModelId", litellmDefaultModelId)
270+
currentLitellmModelId = litellmDefaultModelId // Use the default for the next step
271+
}
272+
// Ensure apiModelId is also set to the specific litellm model id
273+
if (apiConfiguration.apiModelId !== currentLitellmModelId) {
274+
setApiConfigurationField("apiModelId", currentLitellmModelId)
269275
}
270276
break
277+
}
271278
}
272279

273-
setApiConfigurationField("apiProvider", value)
280+
// Only update the apiProvider if it's actually changing.
281+
// This should be called after model-specific IDs are handled for the new provider.
282+
if (apiConfiguration.apiProvider !== value) {
283+
setApiConfigurationField("apiProvider", value)
284+
}
274285
},
275286
[
287+
apiConfiguration.apiProvider,
288+
apiConfiguration.apiModelId, // Add apiModelId as it's read and potentially set
276289
setApiConfigurationField,
277290
apiConfiguration.openRouterModelId,
278291
apiConfiguration.glamaModelId,

webview-ui/src/components/settings/SettingsView.tsx

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727

2828
import { ExperimentId } from "@roo/shared/experiments"
2929
import { TelemetrySetting } from "@roo/shared/TelemetrySetting"
30-
import { ProviderSettings } from "@roo/shared/api"
30+
import { ProviderSettings, litellmDefaultModelId } from "@roo/shared/api"
3131

3232
import { vscode } from "@/utils/vscode"
3333
import { ExtensionStateContextType, useExtensionState } from "@/context/ExtensionStateContext"
@@ -219,6 +219,51 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
219219
})
220220
}, [])
221221

222+
useEffect(() => {
223+
// This effect ensures that if LiteLLM is the provider,
224+
// its specific configuration fields have default values in the cached state.
225+
const config = cachedState.apiConfiguration
226+
if (config && config.apiProvider === "litellm") {
227+
const baseUrlMissing = config.litellmBaseUrl === undefined
228+
const apiKeyMissing = config.litellmApiKey === undefined
229+
230+
if (baseUrlMissing || apiKeyMissing) {
231+
setCachedState((prevState) => {
232+
// Ensure we are working with the latest apiConfiguration from prevState
233+
const currentApiConfig = prevState.apiConfiguration ?? {}
234+
let updatedApiConfig = { ...currentApiConfig } // Clone to modify
235+
let madeChanges = false
236+
237+
// Check and apply defaults based on the fresh currentApiConfig
238+
if (currentApiConfig.litellmBaseUrl === undefined) {
239+
updatedApiConfig.litellmBaseUrl = "http://localhost:4000"
240+
madeChanges = true
241+
}
242+
if (currentApiConfig.litellmApiKey === undefined) {
243+
updatedApiConfig.litellmApiKey = "sk-1234"
244+
madeChanges = true
245+
}
246+
if (currentApiConfig.litellmModelId === undefined) {
247+
updatedApiConfig.litellmModelId = litellmDefaultModelId
248+
madeChanges = true
249+
}
250+
251+
if (madeChanges) {
252+
// setChangeDetected is not directly available here unless passed to setCachedState's scope or handled differently
253+
// For now, we focus on updating apiConfiguration correctly.
254+
// The parent component (SettingsView) already calls setChangeDetected(true) when setApiConfigurationField is used.
255+
// If we bypass setApiConfigurationField, we need to call setChangeDetected here.
256+
setChangeDetected(true) // Call setChangeDetected as we are modifying the state that tracks changes.
257+
return { ...prevState, apiConfiguration: updatedApiConfig }
258+
}
259+
return prevState // No changes were actually needed based on the fresh check
260+
})
261+
}
262+
}
263+
// Adding setCachedState and setChangeDetected to dependencies as they are used directly or indirectly.
264+
// setApiConfigurationField is removed as we are not calling it from here for these specific defaults.
265+
}, [cachedState.apiConfiguration, setCachedState, setChangeDetected])
266+
222267
const setTelemetrySetting = useCallback((setting: TelemetrySetting) => {
223268
setCachedState((prevState) => {
224269
if (prevState.telemetrySetting === setting) {

webview-ui/src/components/settings/providers/LiteLLM.tsx

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useState } from "react"
1+
import { useCallback, useState, useEffect, useRef } from "react"
22
import { VSCodeTextField } from "@vscode/webview-ui-toolkit/react"
33
import { useEvent } from "react-use"
44

@@ -24,6 +24,7 @@ export const LiteLLM = ({ apiConfiguration, setApiConfigurationField, routerMode
2424
const { t } = useAppTranslation()
2525
const [refreshStatus, setRefreshStatus] = useState<"idle" | "loading" | "success" | "error">("idle")
2626
const [refreshError, setRefreshError] = useState<string | undefined>()
27+
const initialRefreshPerformedRef = useRef(false)
2728

2829
const handleInputChange = useCallback(
2930
<K extends keyof ProviderSettings, E>(
@@ -36,14 +37,18 @@ export const LiteLLM = ({ apiConfiguration, setApiConfigurationField, routerMode
3637
[setApiConfigurationField],
3738
)
3839

39-
const handleRefreshModels = () => {
40+
const handleRefreshModels = useCallback(() => {
4041
setRefreshStatus("loading")
4142
setRefreshError(undefined)
4243

43-
// Due to the button's disabled state logic, litellmApiKey and litellmBaseUrl are guaranteed to be non-empty strings here.
44-
// We use non-null assertions (!) to reflect this guarantee for type safety.
45-
const key = apiConfiguration.litellmApiKey!
46-
const url = apiConfiguration.litellmBaseUrl!
44+
const key = apiConfiguration.litellmApiKey
45+
const url = apiConfiguration.litellmBaseUrl
46+
47+
if (!key || !url) {
48+
setRefreshStatus("error")
49+
setRefreshError(t("settings:providers.refreshModels.missingConfig"))
50+
return
51+
}
4752

4853
const message: WebviewMessage = {
4954
type: "requestProviderModels",
@@ -54,9 +59,34 @@ export const LiteLLM = ({ apiConfiguration, setApiConfigurationField, routerMode
5459
},
5560
}
5661
vscode.postMessage(message)
57-
}
62+
}, [apiConfiguration.litellmApiKey, apiConfiguration.litellmBaseUrl, setRefreshStatus, setRefreshError, t])
63+
64+
// Effect to trigger initial model refresh, once per component instance when conditions are met
65+
useEffect(() => {
66+
// Only proceed if the initial refresh for this component instance hasn't been done
67+
if (initialRefreshPerformedRef.current) {
68+
return
69+
}
70+
71+
// Check if the necessary configuration is available
72+
if (apiConfiguration.litellmApiKey && apiConfiguration.litellmBaseUrl) {
73+
// Mark that we are performing the refresh for this instance
74+
initialRefreshPerformedRef.current = true
75+
// Directly execute refresh logic
76+
setRefreshStatus("loading")
77+
setRefreshError(undefined)
78+
const message: WebviewMessage = {
79+
type: "requestProviderModels",
80+
payload: {
81+
provider: "litellm",
82+
apiKey: apiConfiguration.litellmApiKey,
83+
baseUrl: apiConfiguration.litellmBaseUrl,
84+
},
85+
}
86+
vscode.postMessage(message)
87+
}
88+
}, [apiConfiguration.litellmApiKey, apiConfiguration.litellmBaseUrl])
5889

59-
// Listen for model refresh responses using useEvent
6090
useEvent("message", (event: MessageEvent<ExtensionMessage>) => {
6191
const message = event.data
6292
if (message.type === "providerModelsResponse") {
@@ -67,7 +97,6 @@ export const LiteLLM = ({ apiConfiguration, setApiConfigurationField, routerMode
6797
setRefreshError(message.payload.error)
6898
} else {
6999
setRefreshStatus("success")
70-
// Parent (ApiOptions.tsx) will handle updating the routerModels prop for ModelPicker
71100
}
72101
} else {
73102
console.log(
@@ -77,7 +106,6 @@ export const LiteLLM = ({ apiConfiguration, setApiConfigurationField, routerMode
77106
}
78107
}
79108
})
80-
console.log("apiconfig1212", apiConfiguration)
81109

82110
return (
83111
<>

webview-ui/src/components/welcome/WelcomeView.tsx

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useState } from "react"
1+
import { useCallback, useState, useEffect } from "react"
22
import { VSCodeButton, VSCodeLink } from "@vscode/webview-ui-toolkit/react"
33
import { useExtensionState } from "@src/context/ExtensionStateContext"
44
import { validateApiConfiguration } from "@src/utils/validate"
@@ -10,12 +10,67 @@ import { useAppTranslation } from "@src/i18n/TranslationContext"
1010
import { getRequestyAuthUrl, getOpenRouterAuthUrl } from "@src/oauth/urls"
1111
import RooHero from "./RooHero"
1212
import knuthShuffle from "knuth-shuffle-seeded"
13+
import { ProviderSettings, litellmDefaultModelId } from "@roo/shared/api"
1314

1415
const WelcomeView = () => {
1516
const { apiConfiguration, currentApiConfigName, setApiConfiguration, uriScheme, machineId } = useExtensionState()
1617
const { t } = useAppTranslation()
1718
const [errorMessage, setErrorMessage] = useState<string | undefined>(undefined)
1819

20+
// Memoize the setApiConfigurationField function to pass to ApiOptions
21+
const setApiConfigurationFieldForApiOptions = useCallback(
22+
<K extends keyof ProviderSettings>(field: K, value: ProviderSettings[K]) => {
23+
setApiConfiguration({ [field]: value })
24+
},
25+
[setApiConfiguration], // setApiConfiguration from context is stable
26+
)
27+
28+
useEffect(() => {
29+
if (!apiConfiguration) {
30+
// If no apiConfig at all, nothing to default yet.
31+
// This can happen before the initial state hydration from the extension.
32+
return
33+
}
34+
35+
const currentProvider = apiConfiguration.apiProvider
36+
// Needs provider default if apiProvider is undefined, null, or an empty string.
37+
const needsProviderDefault = !currentProvider
38+
const isLiteLLMSelectedOrShouldBeDefault = currentProvider === "litellm" || needsProviderDefault
39+
40+
if (isLiteLLMSelectedOrShouldBeDefault) {
41+
const updates: Partial<ProviderSettings> = {}
42+
let madeChanges = false
43+
44+
if (apiConfiguration.litellmBaseUrl === undefined) {
45+
updates.litellmBaseUrl = "http://localhost:4000"
46+
madeChanges = true
47+
}
48+
if (apiConfiguration.litellmApiKey === undefined) {
49+
updates.litellmApiKey = "sk-1234"
50+
madeChanges = true
51+
}
52+
if (apiConfiguration.litellmModelId === undefined) {
53+
updates.litellmModelId = litellmDefaultModelId
54+
madeChanges = true
55+
}
56+
57+
// If apiProvider was initially missing or falsy, set it to "litellm".
58+
if (needsProviderDefault) {
59+
updates.apiProvider = "litellm"
60+
madeChanges = true
61+
}
62+
63+
if (madeChanges) {
64+
// This log helps confirm if we are about to call setApiConfiguration
65+
setApiConfiguration(updates)
66+
} else {
67+
// This log helps confirm that on subsequent runs, no changes are deemed necessary.
68+
}
69+
}
70+
}, [apiConfiguration, setApiConfiguration])
71+
72+
useEffect(() => {}, [apiConfiguration])
73+
1974
const handleSubmit = useCallback(() => {
2075
const error = apiConfiguration ? validateApiConfiguration(apiConfiguration) : undefined
2176

@@ -106,7 +161,7 @@ const WelcomeView = () => {
106161
fromWelcomeView
107162
apiConfiguration={apiConfiguration || {}}
108163
uriScheme={uriScheme}
109-
setApiConfigurationField={(field, value) => setApiConfiguration({ [field]: value })}
164+
setApiConfigurationField={setApiConfigurationFieldForApiOptions}
110165
errorMessage={errorMessage}
111166
setErrorMessage={setErrorMessage}
112167
/>

webview-ui/src/context/ExtensionStateContext.tsx

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,16 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
188188
[],
189189
)
190190

191+
const setApiConfiguration = useCallback((value: ProviderSettings) => {
192+
setState((prevState) => ({
193+
...prevState,
194+
apiConfiguration: {
195+
...prevState.apiConfiguration,
196+
...value,
197+
},
198+
}))
199+
}, [])
200+
191201
const handleMessage = useCallback(
192202
(event: MessageEvent) => {
193203
const message: ExtensionMessage = event.data
@@ -266,14 +276,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
266276
screenshotQuality: state.screenshotQuality,
267277
setExperimentEnabled: (id, enabled) =>
268278
setState((prevState) => ({ ...prevState, experiments: { ...prevState.experiments, [id]: enabled } })),
269-
setApiConfiguration: (value) =>
270-
setState((prevState) => ({
271-
...prevState,
272-
apiConfiguration: {
273-
...prevState.apiConfiguration,
274-
...value,
275-
},
276-
})),
279+
setApiConfiguration,
277280
setCustomInstructions: (value) => setState((prevState) => ({ ...prevState, customInstructions: value })),
278281
setAlwaysAllowReadOnly: (value) => setState((prevState) => ({ ...prevState, alwaysAllowReadOnly: value })),
279282
setAlwaysAllowReadOnlyOutsideWorkspace: (value) =>

webview-ui/src/i18n/locales/ca/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@
121121
"hint": "Si us plau, torneu a obrir la configuració per veure els models més recents.",
122122
"loading": "Actualitzant models...",
123123
"success": "Models actualitzats correctament.",
124-
"error": "No s'han pogut actualitzar els models. Si us plau, comproveu la vostra configuració i torneu-ho a provar."
124+
"error": "No s'han pogut actualitzar els models. Si us plau, comproveu la vostra configuració i torneu-ho a provar.",
125+
"missingConfig": "Falta la clau API o l'URL base. Si us plau, proporcioneu ambdós per actualitzar els models."
125126
},
126127
"getRequestyApiKey": "Obtenir clau API de Requesty",
127128
"openRouterTransformsText": "Comprimir prompts i cadenes de missatges a la mida del context (<a>Transformacions d'OpenRouter</a>)",

webview-ui/src/i18n/locales/de/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@
121121
"hint": "Bitte öffne die Einstellungen erneut, um die neuesten Modelle zu sehen.",
122122
"loading": "Modelle werden aktualisiert...",
123123
"success": "Modelle erfolgreich aktualisiert.",
124-
"error": "Fehler beim Aktualisieren der Modelle. Bitte überprüfe deine Konfiguration und versuche es erneut."
124+
"error": "Fehler beim Aktualisieren der Modelle. Bitte überprüfe deine Konfiguration und versuche es erneut.",
125+
"missingConfig": "API-Schlüssel oder Basis-URL fehlt. Bitte gib beides an, um Modelle zu aktualisieren."
125126
},
126127
"getRequestyApiKey": "Requesty API-Schlüssel erhalten",
127128
"openRouterTransformsText": "Prompts und Nachrichtenketten auf Kontextgröße komprimieren (<a>OpenRouter Transformationen</a>)",

webview-ui/src/i18n/locales/en/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@
121121
"hint": "Please reopen the settings to see the latest models.",
122122
"loading": "Refreshing models...",
123123
"success": "Models refreshed successfully.",
124-
"error": "Failed to refresh models. Please check your configuration and try again."
124+
"error": "Failed to refresh models. Please check your configuration and try again.",
125+
"missingConfig": "API key or base URL missing. Please provide both to refresh models."
125126
},
126127
"getRequestyApiKey": "Get Requesty API Key",
127128
"openRouterTransformsText": "Compress prompts and message chains to the context size (<a>OpenRouter Transforms</a>)",

webview-ui/src/i18n/locales/es/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@
121121
"hint": "Por favor, vuelve a abrir la configuración para ver los modelos más recientes.",
122122
"loading": "Actualizando modelos...",
123123
"success": "Modelos actualizados correctamente.",
124-
"error": "Error al actualizar los modelos. Por favor, verifica tu configuración e inténtalo de nuevo."
124+
"error": "Error al actualizar los modelos. Por favor, verifica tu configuración e inténtalo de nuevo.",
125+
"missingConfig": "Falta la clave API o la URL base. Por favor, proporciona ambos para actualizar los modelos."
125126
},
126127
"getRequestyApiKey": "Obtener clave API de Requesty",
127128
"openRouterTransformsText": "Comprimir prompts y cadenas de mensajes al tamaño del contexto (<a>Transformaciones de OpenRouter</a>)",

webview-ui/src/i18n/locales/fr/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@
121121
"hint": "Veuillez rouvrir les paramètres pour voir les modèles les plus récents.",
122122
"loading": "Actualisation des modèles...",
123123
"success": "Modèles actualisés avec succès.",
124-
"error": "Échec de l'actualisation des modèles. Veuillez vérifier votre configuration et réessayer."
124+
"error": "Échec de l'actualisation des modèles. Veuillez vérifier votre configuration et réessayer.",
125+
"missingConfig": "Clé API ou URL de base manquante. Veuillez fournir les deux pour actualiser les modèles."
125126
},
126127
"getRequestyApiKey": "Obtenir la clé API Requesty",
127128
"openRouterTransformsText": "Compresser les prompts et chaînes de messages à la taille du contexte (<a>Transformations OpenRouter</a>)",

0 commit comments

Comments
 (0)