Deprecate free grok 4 fast #8481
Conversation
dosubot
bot
added
size:L
hannesrudolph
added
the
Issue/PR - Triage
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| // 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]) => { |
There was a problem hiding this comment.
[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 |
There was a problem hiding this comment.
[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.
| {errorMessage && <ApiErrorMessage errorMessage={errorMessage} />} | ||
| {selectedModelId && selectedModelInfo && ( | ||
| {selectedModelInfo?.deprecated && ( | ||
| <ApiErrorMessage errorMessage={t("settings:validation.modelDeprecated")} /> |
There was a problem hiding this comment.
[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.
| ) | ||
|
|
||
| // Model info should not be displayed for deprecated models | ||
| expect(screen.queryByText("This is a deprecated model")).not.toBeInTheDocument() |
There was a problem hiding this comment.
[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.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
| </Select> | ||
| </div> | ||
|
|
||
| {/* Show error if a deprecated model is selected */} |
There was a problem hiding this comment.
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.
| ) | ||
|
|
||
| const { id: selectedModelId, info: selectedModelInfo } = useSelectedModel(apiConfiguration) | ||
| return Object.keys(availableModels).sort((a, b) => a.localeCompare(b)) |
There was a problem hiding this comment.
P3: Improve sort stability/readability by using localeCompare with numeric, case-insensitive options for natural ordering.
| 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" })) |
| // Always include the currently selected model | ||
| if (modelId === selectedModelId) return true | ||
| // Filter out deprecated models that aren't currently selected | ||
| return !modelInfo.deprecated |
There was a problem hiding this comment.
[P3] Ensure stable, natural ordering of the model list across locales/renders.
| return !modelInfo.deprecated | |
| return availableModels.sort((a, b) => a.label.localeCompare(b.label, undefined, { numeric: true, sensitivity: "base" })) |
| </Select> | ||
| </div> | ||
|
|
||
| {/* Show error if a deprecated model is selected */} |
There was a problem hiding this comment.
[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.
Free Grok 4 Fast is going away, so we should remove it from the Roo Code Cloud provider. In general it seems like we should have a pattern for deprecating models that don't work anymore. I suggest that we show a warning when it's selected and then hide it when not selected.
Warning when selected:
No longer in the list when not selected:
Important
Deprecates
Grok 4 Fastmodel, updates UI to handle deprecated models, and adds tests for deprecated model behavior.Grok 4 Fastas deprecated inproviders/roo.ts.ApiOptions.tsxto filter out deprecated models unless currently selected.ModelPicker.tsxto exclude deprecated models from selection unless currently selected.ModelPicker.tsx.ModelPicker.deprecated.spec.tsxto test deprecated model behavior.modelDeprecatedmessage in multiple locale files for warning display.This description was created by
for 51cd953. You can customize this summary. It will automatically update as commits are pushed.