Skip to content

Commit fef46c9

Browse files
committed
fix: show model id error below model selector
1 parent e2516eb commit fef46c9

File tree

9 files changed

+113
-35
lines changed

9 files changed

+113
-35
lines changed

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

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
} from "@roo-code/types"
1515

1616
import { vscode } from "@src/utils/vscode"
17-
import { validateApiConfiguration } from "@src/utils/validate"
17+
import { validateApiConfigurationExcludingModelErrors, getModelValidationError } from "@src/utils/validate"
1818
import { useAppTranslation } from "@src/i18n/TranslationContext"
1919
import { useRouterModels } from "@src/components/ui/hooks/useRouterModels"
2020
import { useSelectedModel } from "@src/components/ui/hooks/useSelectedModel"
@@ -176,8 +176,11 @@ const ApiOptions = ({
176176
)
177177

178178
useEffect(() => {
179-
const apiValidationResult = validateApiConfiguration(apiConfiguration, routerModels, organizationAllowList)
180-
179+
const apiValidationResult = validateApiConfigurationExcludingModelErrors(
180+
apiConfiguration,
181+
routerModels,
182+
organizationAllowList,
183+
)
181184
setErrorMessage(apiValidationResult)
182185
}, [apiConfiguration, routerModels, organizationAllowList, setErrorMessage])
183186

@@ -197,51 +200,58 @@ const ApiOptions = ({
197200

198201
const onProviderChange = useCallback(
199202
(value: ProviderName) => {
200-
// It would be much easier to have a single attribute that stores
201-
// the modelId, but we have a separate attribute for each of
202-
// OpenRouter, Glama, Unbound, and Requesty.
203-
// If you switch to one of these providers and the corresponding
204-
// modelId is not set then you immediately end up in an error state.
205-
// To address that we set the modelId to the default value for th
206-
// provider if it's not already set.
203+
setApiConfigurationField("apiProvider", value)
204+
205+
// Helper function to validate and reset model if invalid
206+
const validateAndResetModel = (
207+
modelId: string | undefined,
208+
field: keyof ProviderSettings,
209+
defaultValue?: string,
210+
) => {
211+
if (modelId) {
212+
const tempConfig = { ...apiConfiguration, apiProvider: value, apiModelId: modelId }
213+
const modelError = getModelValidationError(tempConfig, routerModels, organizationAllowList)
214+
if (modelError) {
215+
setApiConfigurationField(field, "")
216+
}
217+
} else if (defaultValue) {
218+
setApiConfigurationField(field, defaultValue)
219+
}
220+
}
221+
222+
// Validate current model for the new provider
207223
switch (value) {
208224
case "openrouter":
209-
if (!apiConfiguration.openRouterModelId) {
210-
setApiConfigurationField("openRouterModelId", openRouterDefaultModelId)
211-
}
225+
validateAndResetModel(
226+
apiConfiguration.openRouterModelId,
227+
"openRouterModelId",
228+
openRouterDefaultModelId,
229+
)
212230
break
213231
case "glama":
214-
if (!apiConfiguration.glamaModelId) {
215-
setApiConfigurationField("glamaModelId", glamaDefaultModelId)
216-
}
232+
validateAndResetModel(apiConfiguration.glamaModelId, "glamaModelId", glamaDefaultModelId)
217233
break
218234
case "unbound":
219-
if (!apiConfiguration.unboundModelId) {
220-
setApiConfigurationField("unboundModelId", unboundDefaultModelId)
221-
}
235+
validateAndResetModel(apiConfiguration.unboundModelId, "unboundModelId", unboundDefaultModelId)
222236
break
223237
case "requesty":
224-
if (!apiConfiguration.requestyModelId) {
225-
setApiConfigurationField("requestyModelId", requestyDefaultModelId)
226-
}
238+
validateAndResetModel(apiConfiguration.requestyModelId, "requestyModelId", requestyDefaultModelId)
227239
break
228240
case "litellm":
229-
if (!apiConfiguration.litellmModelId) {
230-
setApiConfigurationField("litellmModelId", litellmDefaultModelId)
231-
}
241+
validateAndResetModel(apiConfiguration.litellmModelId, "litellmModelId", litellmDefaultModelId)
242+
break
243+
case "openai":
244+
validateAndResetModel(apiConfiguration.openAiModelId, "openAiModelId")
245+
break
246+
case "ollama":
247+
validateAndResetModel(apiConfiguration.ollamaModelId, "ollamaModelId")
248+
break
249+
case "lmstudio":
250+
validateAndResetModel(apiConfiguration.lmStudioModelId, "lmStudioModelId")
232251
break
233252
}
234-
235-
setApiConfigurationField("apiProvider", value)
236253
},
237-
[
238-
setApiConfigurationField,
239-
apiConfiguration.openRouterModelId,
240-
apiConfiguration.glamaModelId,
241-
apiConfiguration.unboundModelId,
242-
apiConfiguration.requestyModelId,
243-
apiConfiguration.litellmModelId,
244-
],
254+
[setApiConfigurationField, apiConfiguration, routerModels, organizationAllowList],
245255
)
246256

247257
const docs = useMemo(() => {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
} from "@src/components/ui"
2424

2525
import { ModelInfoView } from "./ModelInfoView"
26+
import { ApiErrorMessage } from "./ApiErrorMessage"
2627

2728
type ModelIdKey = keyof Pick<
2829
ProviderSettings,
@@ -38,6 +39,7 @@ interface ModelPickerProps {
3839
apiConfiguration: ProviderSettings
3940
setApiConfigurationField: <K extends keyof ProviderSettings>(field: K, value: ProviderSettings[K]) => void
4041
organizationAllowList: OrganizationAllowList
42+
errorMessage?: string
4143
}
4244

4345
export const ModelPicker = ({
@@ -49,6 +51,7 @@ export const ModelPicker = ({
4951
apiConfiguration,
5052
setApiConfigurationField,
5153
organizationAllowList,
54+
errorMessage,
5255
}: ModelPickerProps) => {
5356
const { t } = useAppTranslation()
5457

@@ -177,6 +180,7 @@ export const ModelPicker = ({
177180
</PopoverContent>
178181
</Popover>
179182
</div>
183+
{errorMessage && <ApiErrorMessage errorMessage={errorMessage} />}
180184
{selectedModelId && selectedModelInfo && (
181185
<ModelInfoView
182186
apiProvider={apiConfiguration.apiProvider}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { getGlamaAuthUrl } from "@src/oauth/urls"
1010
import { VSCodeButtonLink } from "@src/components/common/VSCodeButtonLink"
1111

1212
import { inputEventTransform } from "../transforms"
13+
import { getModelValidationError } from "@src/utils/validate"
1314
import { ModelPicker } from "../ModelPicker"
1415

1516
type GlamaProps = {
@@ -67,6 +68,7 @@ export const Glama = ({
6768
serviceName="Glama"
6869
serviceUrl="https://glama.ai/models"
6970
organizationAllowList={organizationAllowList}
71+
errorMessage={getModelValidationError(apiConfiguration, routerModels, organizationAllowList)}
7072
/>
7173
</>
7274
)

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { useAppTranslation } from "@src/i18n/TranslationContext"
1212
import { Button } from "@src/components/ui"
1313

1414
import { inputEventTransform } from "../transforms"
15+
import { getModelValidationError } from "@src/utils/validate"
1516
import { ModelPicker } from "../ModelPicker"
1617

1718
type LiteLLMProps = {
@@ -143,6 +144,7 @@ export const LiteLLM = ({ apiConfiguration, setApiConfigurationField, organizati
143144
serviceUrl="https://docs.litellm.ai/"
144145
setApiConfigurationField={setApiConfigurationField}
145146
organizationAllowList={organizationAllowList}
147+
errorMessage={getModelValidationError(apiConfiguration, routerModels, organizationAllowList)}
146148
/>
147149
</>
148150
)

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { Button } from "@src/components/ui"
1919

2020
import { convertHeadersToObject } from "../utils/headers"
2121
import { inputEventTransform, noTransform } from "../transforms"
22+
import { getModelValidationError } from "@src/utils/validate"
2223
import { ModelPicker } from "../ModelPicker"
2324
import { R1FormatSetting } from "../R1FormatSetting"
2425
import { ThinkingBudget } from "../ThinkingBudget"
@@ -144,6 +145,7 @@ export const OpenAICompatible = ({
144145
serviceName="OpenAI"
145146
serviceUrl="https://platform.openai.com"
146147
organizationAllowList={organizationAllowList}
148+
errorMessage={getModelValidationError(apiConfiguration, undefined, organizationAllowList)}
147149
/>
148150
<R1FormatSetting
149151
onChange={handleInputChange("openAiR1FormatEnabled", noTransform)}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { VSCodeButtonLink } from "@src/components/common/VSCodeButtonLink"
1818
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@src/components/ui"
1919

2020
import { inputEventTransform, noTransform } from "../transforms"
21+
import { getModelValidationError } from "@src/utils/validate"
2122

2223
import { ModelPicker } from "../ModelPicker"
2324
import { OpenRouterBalanceDisplay } from "./OpenRouterBalanceDisplay"
@@ -135,6 +136,7 @@ export const OpenRouter = ({
135136
serviceName="OpenRouter"
136137
serviceUrl="https://openrouter.ai/models"
137138
organizationAllowList={organizationAllowList}
139+
errorMessage={getModelValidationError(apiConfiguration, routerModels, organizationAllowList)}
138140
/>
139141
{openRouterModelProviders && Object.keys(openRouterModelProviders).length > 0 && (
140142
<div>

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { VSCodeButtonLink } from "@src/components/common/VSCodeButtonLink"
1111
import { Button } from "@src/components/ui"
1212

1313
import { inputEventTransform } from "../transforms"
14+
import { getModelValidationError } from "@src/utils/validate"
1415
import { ModelPicker } from "../ModelPicker"
1516
import { RequestyBalanceDisplay } from "./RequestyBalanceDisplay"
1617

@@ -96,6 +97,7 @@ export const Requesty = ({
9697
serviceName="Requesty"
9798
serviceUrl="https://requesty.ai"
9899
organizationAllowList={organizationAllowList}
100+
errorMessage={getModelValidationError(apiConfiguration, routerModels, organizationAllowList)}
99101
/>
100102
</>
101103
)

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { vscode } from "@src/utils/vscode"
1212
import { Button } from "@src/components/ui"
1313

1414
import { inputEventTransform } from "../transforms"
15+
import { getModelValidationError } from "@src/utils/validate"
1516
import { ModelPicker } from "../ModelPicker"
1617

1718
type UnboundProps = {
@@ -176,6 +177,7 @@ export const Unbound = ({
176177
serviceUrl="https://api.getunbound.ai/models"
177178
setApiConfigurationField={setApiConfigurationField}
178179
organizationAllowList={organizationAllowList}
180+
errorMessage={getModelValidationError(apiConfiguration, routerModels, organizationAllowList)}
179181
/>
180182
</>
181183
)

webview-ui/src/utils/validate.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,3 +233,55 @@ export function validateModelId(apiConfiguration: ProviderSettings, routerModels
233233

234234
return undefined
235235
}
236+
237+
/**
238+
* Extracts model-specific validation errors from the API configuration
239+
* This is used to show model errors specifically in the model selector components
240+
*/
241+
export function getModelValidationError(
242+
apiConfiguration: ProviderSettings,
243+
routerModels?: RouterModels,
244+
organizationAllowList?: OrganizationAllowList,
245+
): string | undefined {
246+
const modelId = getModelIdForProvider(apiConfiguration, apiConfiguration.apiProvider || "")
247+
const configWithModelId = {
248+
...apiConfiguration,
249+
apiModelId: modelId || "",
250+
}
251+
252+
const orgError = validateProviderAgainstOrganizationSettings(configWithModelId, organizationAllowList)
253+
if (orgError && orgError.includes("model")) {
254+
return orgError
255+
}
256+
257+
return validateModelId(configWithModelId, routerModels)
258+
}
259+
260+
/**
261+
* Validates API configuration but excludes model-specific errors
262+
* This is used for the general API error display to prevent duplication
263+
* when model errors are shown in the model selector
264+
*/
265+
export function validateApiConfigurationExcludingModelErrors(
266+
apiConfiguration: ProviderSettings,
267+
_routerModels?: RouterModels, // keeping this for compatibility with the old function
268+
organizationAllowList?: OrganizationAllowList,
269+
): string | undefined {
270+
const keysAndIdsPresentErrorMessage = validateModelsAndKeysProvided(apiConfiguration)
271+
if (keysAndIdsPresentErrorMessage) {
272+
return keysAndIdsPresentErrorMessage
273+
}
274+
275+
const organizationAllowListErrorMessage = validateProviderAgainstOrganizationSettings(
276+
apiConfiguration,
277+
organizationAllowList,
278+
)
279+
280+
// only return organization errors if they're not model-specific
281+
if (organizationAllowListErrorMessage && !organizationAllowListErrorMessage.includes("model")) {
282+
return organizationAllowListErrorMessage
283+
}
284+
285+
// skip model validation errors as they'll be shown in the model selector
286+
return undefined
287+
}

0 commit comments

Comments
 (0)