Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/types/src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ export const modelInfoSchema = z.object({
minTokensPerCachePoint: z.number().optional(),
maxCachePoints: z.number().optional(),
cachableFields: z.array(z.string()).optional(),
// Flag to indicate if the model is deprecated and should not be used
deprecated: z.boolean().optional(),
/**
* Service tiers with pricing information.
* Each tier can have a name (for OpenAI service tiers) and pricing overrides.
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/providers/roo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const rooModels = {
outputPrice: 0,
description:
"Grok 4 Fast is xAI's latest multimodal model with SOTA cost-efficiency and a 2M token context window. (Note: prompts and completions are logged by xAI and used to improve the model.)",
deprecated: true,
},
"deepseek/deepseek-chat-v3.1": {
maxTokens: 16_384,
Expand Down
45 changes: 31 additions & 14 deletions webview-ui/src/components/settings/ApiOptions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -263,15 +263,24 @@ const ApiOptions = ({

const filteredModels = filterModels(models, selectedProvider, organizationAllowList)

const modelOptions = filteredModels
? Object.keys(filteredModels).map((modelId) => ({
value: modelId,
label: modelId,
}))
// Include the currently selected model even if deprecated (so users can see what they have selected)
// But filter out other deprecated models from being newly selectable
const availableModels = filteredModels
? Object.entries(filteredModels)
.filter(([modelId, modelInfo]) => {
// Always include the currently selected model
if (modelId === selectedModelId) return true
// Filter out deprecated models that aren't currently selected
return !modelInfo.deprecated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Same nuance as in ModelPicker: the selected model is only included if it survives filterModels(...). If org policy removes it, the dropdown options won’t include the current selection even though the control value still points to it. Consider explicitly reinserting the selected model into the options (with a warning) so users can see what’s currently set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Ensure stable, natural ordering of the model list across locales/renders.

Suggested change
return !modelInfo.deprecated
return availableModels.sort((a, b) => a.label.localeCompare(b.label, undefined, { numeric: true, sensitivity: "base" }))

})
.map(([modelId]) => ({
value: modelId,
label: modelId,
}))
: []

return modelOptions
}, [selectedProvider, organizationAllowList])
return availableModels
}, [selectedProvider, organizationAllowList, selectedModelId])

const onProviderChange = useCallback(
(value: ProviderName) => {
Expand Down Expand Up @@ -716,20 +725,28 @@ const ApiOptions = ({
</Select>
</div>

{/* Show error if a deprecated model is selected */}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The deprecated-model warning is gated by selectedProviderModels.length > 0, so if no selectable models remain (e.g., org policy filters everything out) the warning never shows even if the current selection is deprecated. Consider rendering this banner outside the model-options block so it always appears when selectedModelInfo?.deprecated is true.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Potential duplicate alerts: a general errorMessage may render above while the deprecated-model warning renders here, leading to stacked banners. Consider de-duplicating (e.g., suppress the general error when selectedModelInfo?.deprecated is true, or merge messages) to reduce noise.

{selectedModelInfo?.deprecated && (
<ApiErrorMessage errorMessage={t("settings:validation.modelDeprecated")} />
)}

{selectedProvider === "bedrock" && selectedModelId === "custom-arn" && (
<BedrockCustomArn
apiConfiguration={apiConfiguration}
setApiConfigurationField={setApiConfigurationField}
/>
)}

<ModelInfoView
apiProvider={selectedProvider}
selectedModelId={selectedModelId}
modelInfo={selectedModelInfo}
isDescriptionExpanded={isDescriptionExpanded}
setIsDescriptionExpanded={setIsDescriptionExpanded}
/>
{/* Only show model info if not deprecated */}
{!selectedModelInfo?.deprecated && (
<ModelInfoView
apiProvider={selectedProvider}
selectedModelId={selectedModelId}
modelInfo={selectedModelInfo}
isDescriptionExpanded={isDescriptionExpanded}
setIsDescriptionExpanded={setIsDescriptionExpanded}
/>
)}
</>
)}

Expand Down
28 changes: 24 additions & 4 deletions webview-ui/src/components/settings/ModelPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,30 @@ export const ModelPicker = ({
const selectTimeoutRef = useRef<NodeJS.Timeout | null>(null)
const closeTimeoutRef = useRef<NodeJS.Timeout | null>(null)

const { id: selectedModelId, info: selectedModelInfo } = useSelectedModel(apiConfiguration)

const modelIds = useMemo(() => {
const filteredModels = filterModels(models, apiConfiguration.apiProvider, organizationAllowList)

return Object.keys(filteredModels ?? {}).sort((a, b) => a.localeCompare(b))
}, [models, apiConfiguration.apiProvider, organizationAllowList])
// Include the currently selected model even if deprecated (so users can see what they have selected)
// But filter out other deprecated models from being newly selectable
const availableModels = Object.entries(filteredModels ?? {})
.filter(([modelId, modelInfo]) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] The intention is to always include the currently selected model, but this starts from filterModels(...) so the selected model will still be dropped if org policy filters it out. This makes the comment misleading and can hide the current selection. Consider merging the selected model back into availableModels after org filtering (with a soft warning) to keep UX consistent.

// Always include the currently selected model
if (modelId === selectedModelId) return true
// Filter out deprecated models that aren't currently selected
return !modelInfo.deprecated
})
.reduce(
(acc, [modelId, modelInfo]) => {
acc[modelId] = modelInfo
return acc
},
{} as Record<string, ModelInfo>,
)

const { id: selectedModelId, info: selectedModelInfo } = useSelectedModel(apiConfiguration)
return Object.keys(availableModels).sort((a, b) => a.localeCompare(b))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Improve sort stability/readability by using localeCompare with numeric, case-insensitive options for natural ordering.

Suggested change
return Object.keys(availableModels).sort((a, b) => a.localeCompare(b))
return Object.keys(availableModels).sort((a, b) => a.localeCompare(b, undefined, { numeric: true, sensitivity: "base" }))

}, [models, apiConfiguration.apiProvider, organizationAllowList, selectedModelId])

const [searchValue, setSearchValue] = useState("")

Expand Down Expand Up @@ -225,7 +242,10 @@ export const ModelPicker = ({
</Popover>
</div>
{errorMessage && <ApiErrorMessage errorMessage={errorMessage} />}
{selectedModelId && selectedModelInfo && (
{selectedModelInfo?.deprecated && (
<ApiErrorMessage errorMessage={t("settings:validation.modelDeprecated")} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Possible duplicate alerts: both a general errorMessage and the modelDeprecated warning can render, resulting in stacked banners. Consider deduplicating (e.g., prioritize modelDeprecated or merge messages) to reduce noise.

)}
{selectedModelId && selectedModelInfo && !selectedModelInfo.deprecated && (
<ModelInfoView
apiProvider={apiConfiguration.apiProvider}
selectedModelId={selectedModelId}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
// npx vitest src/components/settings/__tests__/ModelPicker.deprecated.spec.tsx

import { render, screen } from "@testing-library/react"
import userEvent from "@testing-library/user-event"
import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
import { describe, it, expect, vi, beforeEach } from "vitest"

import { ModelPicker } from "../ModelPicker"
import type { ModelInfo } from "@roo-code/types"

// Mock the i18n module
vi.mock("@src/i18n/TranslationContext", () => ({
useAppTranslation: () => ({
t: (key: string, options?: any) => {
// Handle specific translation keys
if (key === "settings:validation.modelDeprecated") {
return "This model is no longer available. Please select a different model."
}
if (options) return `${key} ${JSON.stringify(options)}`
return key
},
}),
}))

// Mock the useSelectedModel hook
vi.mock("@/components/ui/hooks/useSelectedModel", () => ({
useSelectedModel: (apiConfiguration: any) => {
const modelId = apiConfiguration?.openRouterModelId || "model-1"
const models: Record<string, ModelInfo> = {
"model-1": {
maxTokens: 1000,
contextWindow: 4000,
supportsPromptCache: true,
},
"model-2": {
maxTokens: 2000,
contextWindow: 8000,
supportsPromptCache: false,
},
"deprecated-model": {
maxTokens: 1500,
contextWindow: 6000,
supportsPromptCache: true,
deprecated: true,
},
}
return {
id: modelId,
info: models[modelId],
provider: "openrouter",
isLoading: false,
isError: false,
}
},
}))

describe("ModelPicker - Deprecated Models", () => {
const mockSetApiConfigurationField = vi.fn()
const queryClient = new QueryClient({
defaultOptions: {
queries: { retry: false },
},
})

const regularModels: Record<string, ModelInfo> = {
"model-1": {
maxTokens: 1000,
contextWindow: 4000,
supportsPromptCache: true,
},
"model-2": {
maxTokens: 2000,
contextWindow: 8000,
supportsPromptCache: false,
},
"deprecated-model": {
maxTokens: 1500,
contextWindow: 6000,
supportsPromptCache: true,
deprecated: true,
},
}

beforeEach(() => {
vi.clearAllMocks()
})

it("should filter out deprecated models from the dropdown", async () => {
const user = userEvent.setup()

render(
<QueryClientProvider client={queryClient}>
<ModelPicker
defaultModelId="model-1"
models={regularModels}
modelIdKey="openRouterModelId"
serviceName="Test Service"
serviceUrl="https://test.com"
apiConfiguration={{ apiProvider: "openrouter" }}
setApiConfigurationField={mockSetApiConfigurationField}
organizationAllowList={{ allowAll: true, providers: {} }}
/>
</QueryClientProvider>,
)

// Open the dropdown
const button = screen.getByTestId("model-picker-button")
await user.click(button)

// Check that non-deprecated models are shown
expect(screen.getByTestId("model-option-model-1")).toBeInTheDocument()
expect(screen.getByTestId("model-option-model-2")).toBeInTheDocument()

// Check that deprecated model is NOT shown
expect(screen.queryByTestId("model-option-deprecated-model")).not.toBeInTheDocument()
})

it("should show error when a deprecated model is currently selected", () => {
render(
<QueryClientProvider client={queryClient}>
<ModelPicker
defaultModelId="deprecated-model"
models={regularModels}
modelIdKey="openRouterModelId"
serviceName="Test Service"
serviceUrl="https://test.com"
apiConfiguration={{
apiProvider: "openrouter",
openRouterModelId: "deprecated-model",
}}
setApiConfigurationField={mockSetApiConfigurationField}
organizationAllowList={{ allowAll: true, providers: {} }}
/>
</QueryClientProvider>,
)

// Check that the error message is displayed
expect(
screen.getByText("This model is no longer available. Please select a different model."),
).toBeInTheDocument()
})

it("should allow selecting non-deprecated models", async () => {
const user = userEvent.setup()

render(
<QueryClientProvider client={queryClient}>
<ModelPicker
defaultModelId="model-1"
models={regularModels}
modelIdKey="openRouterModelId"
serviceName="Test Service"
serviceUrl="https://test.com"
apiConfiguration={{ apiProvider: "openrouter" }}
setApiConfigurationField={mockSetApiConfigurationField}
organizationAllowList={{ allowAll: true, providers: {} }}
/>
</QueryClientProvider>,
)

// Open the dropdown
const button = screen.getByTestId("model-picker-button")
await user.click(button)

// Select a non-deprecated model
const model2Option = screen.getByTestId("model-option-model-2")
await user.click(model2Option)

// Verify the selection was made
expect(mockSetApiConfigurationField).toHaveBeenCalledWith("openRouterModelId", "model-2")
})

it("should not display model info for deprecated models", () => {
render(
<QueryClientProvider client={queryClient}>
<ModelPicker
defaultModelId="deprecated-model"
models={regularModels}
modelIdKey="openRouterModelId"
serviceName="Test Service"
serviceUrl="https://test.com"
apiConfiguration={{
apiProvider: "openrouter",
openRouterModelId: "deprecated-model",
}}
setApiConfigurationField={mockSetApiConfigurationField}
organizationAllowList={{ allowAll: true, providers: {} }}
/>
</QueryClientProvider>,
)

// Model info should not be displayed for deprecated models
expect(screen.queryByText("This is a deprecated model")).not.toBeInTheDocument()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Brittle assertion: checks for 'This is a deprecated model', which is not part of the component copy. Prefer asserting on the absence of ModelInfoView (via a test id) or another stable UI signal to avoid false negatives from text changes.

})
})
1 change: 1 addition & 0 deletions webview-ui/src/i18n/locales/ca/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions webview-ui/src/i18n/locales/de/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions webview-ui/src/i18n/locales/en/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,7 @@
"regionMismatch": "Warning: The region in your ARN ({{arnRegion}}) does not match your selected region ({{region}}). This may cause access issues. The provider will use the region from the ARN."
},
"modelAvailability": "The model ID ({{modelId}}) you provided is not available. Please choose a different model.",
"modelDeprecated": "This model is no longer available. Please select a different model.",
"providerNotAllowed": "Provider '{{provider}}' is not allowed by your organization",
"modelNotAllowed": "Model '{{model}}' is not allowed for provider '{{provider}}' by your organization",
"profileInvalid": "This profile contains a provider or model that is not allowed by your organization",
Expand Down
1 change: 1 addition & 0 deletions webview-ui/src/i18n/locales/es/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions webview-ui/src/i18n/locales/fr/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions webview-ui/src/i18n/locales/hi/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions webview-ui/src/i18n/locales/id/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions webview-ui/src/i18n/locales/it/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions webview-ui/src/i18n/locales/ja/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading