-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Deprecate free grok 4 fast #8481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||||||
| }) | ||||||
| .map(([modelId]) => ({ | ||||||
| value: modelId, | ||||||
| label: modelId, | ||||||
| })) | ||||||
| : [] | ||||||
|
|
||||||
| return modelOptions | ||||||
| }, [selectedProvider, organizationAllowList]) | ||||||
| return availableModels | ||||||
| }, [selectedProvider, organizationAllowList, selectedModelId]) | ||||||
|
|
||||||
| const onProviderChange = useCallback( | ||||||
| (value: ProviderName) => { | ||||||
|
|
@@ -716,20 +725,28 @@ const ApiOptions = ({ | |||||
| </Select> | ||||||
| </div> | ||||||
|
|
||||||
| {/* Show error if a deprecated model is selected */} | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: The deprecated-model warning is gated by
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P3] Potential duplicate alerts: a general |
||||||
| {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} | ||||||
| /> | ||||||
| )} | ||||||
| </> | ||||||
| )} | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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]) => { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||
| // 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)) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||||||
| }, [models, apiConfiguration.apiProvider, organizationAllowList, selectedModelId]) | ||||||
|
|
||||||
| const [searchValue, setSearchValue] = useState("") | ||||||
|
|
||||||
|
|
@@ -225,7 +242,10 @@ export const ModelPicker = ({ | |||||
| </Popover> | ||||||
| </div> | ||||||
| {errorMessage && <ApiErrorMessage errorMessage={errorMessage} />} | ||||||
| {selectedModelId && selectedModelInfo && ( | ||||||
| {selectedModelInfo?.deprecated && ( | ||||||
| <ApiErrorMessage errorMessage={t("settings:validation.modelDeprecated")} /> | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P3] Possible duplicate alerts: both a general |
||||||
| )} | ||||||
| {selectedModelId && selectedModelInfo && !selectedModelInfo.deprecated && ( | ||||||
| <ModelInfoView | ||||||
| apiProvider={apiConfiguration.apiProvider} | ||||||
| selectedModelId={selectedModelId} | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,191 @@ | ||
| // 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) => { | ||
| 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() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| }) | ||
| }) | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
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.