From 4025b582cdcde9be6e7444501b438a96beb0695f Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Wed, 25 Mar 2026 20:46:57 +0100 Subject: [PATCH] refactor(site): show one model picker option per config (#23533) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `/agents` model picker collapsed distinct configured model variants into fewer entries because options were built from the deduplicated catalog (`ChatModelsResponse`). Two configs with the same provider/model but different display names or settings appeared as a single option. Switch option building from `getModelOptionsFromCatalog()` to a new `getModelOptionsFromConfigs()` that emits one `ModelSelectorOption` per enabled `ChatModelConfig` row. The option ID is the config UUID directly, eliminating the catalog-ID ↔ config-ID mapping layer (`buildModelConfigIDByModelID`, `buildModelIDByConfigID`). Provider availability is still gated by the catalog response, and status messaging ("no models configured" vs "models unavailable") is unchanged. The sidebar now resolves model labels by config ID first, and the /agents Storybook fixtures were updated so the stories seed matching config IDs and model-config query data after the picker contract change. --- site/src/pages/AgentsPage/AgentCreatePage.tsx | 15 +- .../pages/AgentsPage/AgentDetail.stories.tsx | 20 +- site/src/pages/AgentsPage/AgentDetail.tsx | 56 ++- site/src/pages/AgentsPage/AgentsPage.tsx | 6 +- .../AgentsPage/AgentsPageView.stories.tsx | 11 +- .../components/AgentChatInput.stories.tsx | 4 +- .../components/AgentCreateForm.stories.tsx | 4 +- .../AgentsPage/components/AgentCreateForm.tsx | 45 +-- .../components/AgentDetailView.stories.tsx | 16 +- .../AgentsPage/components/MCPServerPicker.tsx | 6 +- .../components/Sidebar/AgentsSidebar.test.tsx | 92 +++++ .../components/Sidebar/AgentsSidebar.tsx | 48 ++- .../AgentsPage/utils/modelOptions.test.ts | 365 +++++++++++++++--- .../pages/AgentsPage/utils/modelOptions.ts | 176 ++++----- 14 files changed, 600 insertions(+), 264 deletions(-) diff --git a/site/src/pages/AgentsPage/AgentCreatePage.tsx b/site/src/pages/AgentsPage/AgentCreatePage.tsx index ed9e452840..c540f47490 100644 --- a/site/src/pages/AgentsPage/AgentCreatePage.tsx +++ b/site/src/pages/AgentsPage/AgentCreatePage.tsx @@ -15,10 +15,7 @@ import { import { AgentPageHeader } from "./components/AgentPageHeader"; import { ChimeButton } from "./components/ChimeButton"; import { WebPushButton } from "./components/WebPushButton"; -import { - buildModelConfigIDByModelID, - getModelOptionsFromCatalog, -} from "./utils/modelOptions"; +import { getModelOptionsFromConfigs } from "./utils/modelOptions"; const lastModelConfigIDStorageKey = "agents.last-model-config-id"; const nilUUID = "00000000-0000-0000-0000-000000000000"; @@ -32,12 +29,9 @@ const AgentCreatePage: FC = () => { const mcpServersQuery = useQuery(mcpServerConfigs()); const createMutation = useMutation(createChat(queryClient)); - const catalogModelOptions = getModelOptionsFromCatalog( + const catalogModelOptions = getModelOptionsFromConfigs( + chatModelConfigsQuery.data, chatModelsQuery.data, - chatModelConfigsQuery.data, - ); - const modelConfigIDByModelID = buildModelConfigIDByModelID( - chatModelConfigsQuery.data, ); const handleCreateChat = async ({ @@ -47,8 +41,7 @@ const AgentCreatePage: FC = () => { model, mcpServerIds, }: CreateChatOptions) => { - const modelConfigID = - (model && modelConfigIDByModelID.get(model)) || nilUUID; + const modelConfigID = model || nilUUID; const content: TypesGen.ChatInputPart[] = []; if (message.trim()) { content.push({ type: "text", text: message }); diff --git a/site/src/pages/AgentsPage/AgentDetail.stories.tsx b/site/src/pages/AgentsPage/AgentDetail.stories.tsx index 52eb84f617..44b9e01047 100644 --- a/site/src/pages/AgentsPage/AgentDetail.stories.tsx +++ b/site/src/pages/AgentsPage/AgentDetail.stories.tsx @@ -15,6 +15,7 @@ import { chatDiffContentsKey, chatKey, chatMessagesKey, + chatModelConfigs, chatModelsKey, chatsKey, mcpServerConfigsKey, @@ -65,6 +66,7 @@ const AgentDetailLayout: FC = () => { // Shared mock data // --------------------------------------------------------------------------- const CHAT_ID = "chat-1"; +const MODEL_CONFIG_ID = "model-config-1"; const mockWorkspaceAgent: TypesGen.WorkspaceAgent = { ...MockWorkspaceAgent, @@ -107,10 +109,25 @@ const mockModelCatalog: TypesGen.ChatModelsResponse = { ], }; +const mockModelConfigs: TypesGen.ChatModelConfig[] = [ + { + id: MODEL_CONFIG_ID, + provider: "openai", + model: "gpt-4o", + display_name: "GPT-4o", + enabled: true, + is_default: true, + context_limit: 200000, + compression_threshold: 70, + created_at: "2026-02-18T00:00:00.000Z", + updated_at: "2026-02-18T00:00:00.000Z", + }, +]; + const baseChatFields = { owner_id: "owner-id", workspace_id: mockWorkspace.id, - last_model_config_id: "model-config-1", + last_model_config_id: MODEL_CONFIG_ID, mcp_server_ids: [], labels: {}, created_at: "2026-02-18T00:00:00.000Z", @@ -178,6 +195,7 @@ const buildQueries = ( data: mockWorkspace, }, { key: chatModelsKey, data: mockModelCatalog }, + { key: chatModelConfigs().queryKey, data: mockModelConfigs }, { key: mcpServerConfigsKey, data: [] }, ]; }; diff --git a/site/src/pages/AgentsPage/AgentDetail.tsx b/site/src/pages/AgentsPage/AgentDetail.tsx index 90ae69547b..d45bb36992 100644 --- a/site/src/pages/AgentsPage/AgentDetail.tsx +++ b/site/src/pages/AgentsPage/AgentDetail.tsx @@ -62,11 +62,10 @@ import { } from "./components/MCPServerPicker"; import { useGitWatcher } from "./hooks/useGitWatcher"; import { - buildModelConfigIDByModelID, - buildModelIDByConfigID, - getModelOptionsFromCatalog, + getModelOptionsFromConfigs, getModelSelectorPlaceholder, hasConfiguredModelsInCatalog, + resolveModelOptionId, } from "./utils/modelOptions"; import { parsePullRequestUrl } from "./utils/pullRequest"; import { @@ -271,17 +270,11 @@ const getPersistedDetailError = ({ function resolveCompactionThreshold( modelConfigID: string | undefined, userThresholds: readonly TypesGen.UserChatCompactionThreshold[] | undefined, - modelConfigs: readonly TypesGen.ChatModelConfig[], + modelConfigs: readonly TypesGen.ChatModelConfig[] | null | undefined, ): number | undefined { - if (!modelConfigID) { - return undefined; - } - const config = modelConfigs.find( - (modelConfig) => modelConfig.id === modelConfigID, - ); - if (!config) { - return undefined; - } + if (!modelConfigID || !Array.isArray(modelConfigs)) return undefined; + const config = modelConfigs.find((c) => c.id === modelConfigID); + if (!config) return undefined; const userOverride = userThresholds?.find( (threshold) => threshold.model_config_id === modelConfigID, ); @@ -373,14 +366,10 @@ const AgentDetail: FC = () => { void mcpServersQuery.refetch(); }; - const modelOptions = getModelOptionsFromCatalog( + const modelOptions = getModelOptionsFromConfigs( + chatModelConfigsQuery.data, chatModelsQuery.data, - chatModelConfigsQuery.data, ); - const modelConfigIDByModelID = buildModelConfigIDByModelID( - chatModelConfigsQuery.data, - ); - const modelIDByConfigID = buildModelIDByConfigID(modelConfigIDByModelID); const modelConfigs = chatModelConfigsQuery.data ?? []; const modelCatalog = chatModelsQuery.data; const isModelCatalogLoading = chatModelsQuery.isLoading; @@ -560,18 +549,22 @@ const AgentDetail: FC = () => { // explicit choice against the current model options, falling // back to the chat's last model or the first available option. const effectiveSelectedModel = (() => { - if ( - selectedModel && - modelOptions.some((model) => model.id === selectedModel) - ) { - return selectedModel; + const resolvedSelectedModel = resolveModelOptionId( + selectedModel, + modelOptions, + ); + if (resolvedSelectedModel) { + return resolvedSelectedModel; } - if (chatLastModelConfigID) { - const fromChat = modelIDByConfigID.get(chatLastModelConfigID); - if (fromChat && modelOptions.some((model) => model.id === fromChat)) { - return fromChat; - } + + const resolvedChatModel = resolveModelOptionId( + chatLastModelConfigID, + modelOptions, + ); + if (resolvedChatModel) { + return resolvedChatModel; } + return modelOptions[0]?.id ?? ""; })(); @@ -692,10 +685,7 @@ const AgentDetail: FC = () => { } return; } - const selectedModelConfigID = - (effectiveSelectedModel && - modelConfigIDByModelID.get(effectiveSelectedModel)) || - undefined; + const selectedModelConfigID = effectiveSelectedModel || undefined; const request: TypesGen.CreateChatMessageRequest = { content, model_config_id: selectedModelConfigID, diff --git a/site/src/pages/AgentsPage/AgentsPage.tsx b/site/src/pages/AgentsPage/AgentsPage.tsx index b755038d76..58de22f7f9 100644 --- a/site/src/pages/AgentsPage/AgentsPage.tsx +++ b/site/src/pages/AgentsPage/AgentsPage.tsx @@ -38,7 +38,7 @@ import { resolveArchiveAndDeleteAction, shouldNavigateAfterArchive, } from "./utils/agentWorkspaceUtils"; -import { getModelOptionsFromCatalog } from "./utils/modelOptions"; +import { getModelOptionsFromConfigs } from "./utils/modelOptions"; import { type ChatDetailError, chatDetailErrorsEqual, @@ -199,9 +199,9 @@ const AgentsPage: FC = () => { const [chatErrorReasons, setChatErrorReasons] = useState< Record >({}); - const catalogModelOptions = getModelOptionsFromCatalog( - chatModelsQuery.data, + const catalogModelOptions = getModelOptionsFromConfigs( chatModelConfigsQuery.data, + chatModelsQuery.data, ); const setChatErrorReason = (chatId: string, reason: ChatDetailError) => { const trimmedMessage = reason.message.trim(); diff --git a/site/src/pages/AgentsPage/AgentsPageView.stories.tsx b/site/src/pages/AgentsPage/AgentsPageView.stories.tsx index cc5a3401da..e454535b09 100644 --- a/site/src/pages/AgentsPage/AgentsPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentsPageView.stories.tsx @@ -27,9 +27,11 @@ import AgentCreatePage from "./AgentCreatePage"; import AgentSettingsPage from "./AgentSettingsPage"; import { AgentsPageView } from "./AgentsPageView"; +const defaultModelConfigID = "model-config-1"; + const defaultModelOptions: ModelSelectorOption[] = [ { - id: "openai:gpt-4o", + id: defaultModelConfigID, provider: "openai", model: "gpt-4o", displayName: "GPT-4o", @@ -38,7 +40,7 @@ const defaultModelOptions: ModelSelectorOption[] = [ const defaultModelConfigs: TypesGen.ChatModelConfig[] = [ { - id: "config-openai-gpt-4o", + id: defaultModelConfigID, provider: "openai", model: "gpt-4o", display_name: "GPT-4o", @@ -63,7 +65,7 @@ const mockAnalyticsSummary: TypesGen.ChatCostSummary = { total_cache_creation_tokens: 5_432, by_model: [ { - model_config_id: "model-config-1", + model_config_id: defaultModelConfigID, display_name: "GPT-4.1", provider: "OpenAI", model: "gpt-4.1", @@ -227,7 +229,7 @@ const meta: Meta = { }); spyOn(API.experimental, "getChatModelConfigs").mockResolvedValue([ { - id: "config-openai-gpt-4o", + id: defaultModelConfigID, provider: "openai", model: "gpt-4o", display_name: "GPT-4o", @@ -239,6 +241,7 @@ const meta: Meta = { updated_at: "2026-02-18T00:00:00.000Z", }, ]); + spyOn(API.experimental, "getMCPServerConfigs").mockResolvedValue([]); spyOn(API.experimental, "getChatDesktopEnabled").mockResolvedValue({ enable_desktop: false, }); diff --git a/site/src/pages/AgentsPage/components/AgentChatInput.stories.tsx b/site/src/pages/AgentsPage/components/AgentChatInput.stories.tsx index b459bfbaa4..c334043bbc 100644 --- a/site/src/pages/AgentsPage/components/AgentChatInput.stories.tsx +++ b/site/src/pages/AgentsPage/components/AgentChatInput.stories.tsx @@ -5,9 +5,11 @@ import type * as TypesGen from "#/api/typesGenerated"; import type { ChatMessageInputRef } from "#/components/ChatMessageInput/ChatMessageInput"; import { AgentChatInput, type UploadState } from "./AgentChatInput"; +const defaultModelConfigID = "model-config-1"; + const defaultModelOptions = [ { - id: "openai:gpt-4o", + id: defaultModelConfigID, provider: "openai", model: "gpt-4o", displayName: "GPT-4o", diff --git a/site/src/pages/AgentsPage/components/AgentCreateForm.stories.tsx b/site/src/pages/AgentsPage/components/AgentCreateForm.stories.tsx index 52ed1bf56a..00c3a6cb15 100644 --- a/site/src/pages/AgentsPage/components/AgentCreateForm.stories.tsx +++ b/site/src/pages/AgentsPage/components/AgentCreateForm.stories.tsx @@ -5,9 +5,11 @@ import { API } from "#/api/api"; import { MockWorkspace } from "#/testHelpers/entities"; import { AgentCreateForm } from "./AgentCreateForm"; +const modelConfigID = "model-config-1"; + const modelOptions = [ { - id: "openai:gpt-4o", + id: modelConfigID, provider: "openai", model: "gpt-4o", displayName: "GPT-4o", diff --git a/site/src/pages/AgentsPage/components/AgentCreateForm.tsx b/site/src/pages/AgentsPage/components/AgentCreateForm.tsx index 32c0fad085..1736de8942 100644 --- a/site/src/pages/AgentsPage/components/AgentCreateForm.tsx +++ b/site/src/pages/AgentsPage/components/AgentCreateForm.tsx @@ -13,7 +13,6 @@ import { useDashboard } from "#/modules/dashboard/useDashboard"; import { useFileAttachments } from "../hooks/useFileAttachments"; import { getModelSelectorPlaceholder, - getNormalizedModelRef, hasConfiguredModelsInCatalog, } from "../utils/modelOptions"; import { @@ -124,43 +123,21 @@ export const AgentCreateForm: FC = ({ const [initialLastModelConfigID] = useState(() => { return localStorage.getItem(lastModelConfigIDStorageKey) ?? ""; }); - const modelIDByConfigID = (() => { - const optionIDByRef = new Map(); - for (const option of modelOptions) { - const provider = option.provider.trim().toLowerCase(); - const model = option.model.trim(); - if (!provider || !model) { - continue; - } - const key = `${provider}:${model}`; - if (!optionIDByRef.has(key)) { - optionIDByRef.set(key, option.id); - } - } - - const byConfigID = new Map(); - for (const config of modelConfigs) { - const { provider, model } = getNormalizedModelRef(config); - if (!provider || !model) { - continue; - } - const modelID = optionIDByRef.get(`${provider}:${model}`); - if (!modelID || byConfigID.has(config.id)) { - continue; - } - byConfigID.set(config.id, modelID); - } - return byConfigID; - })(); - const lastUsedModelID = initialLastModelConfigID - ? (modelIDByConfigID.get(initialLastModelConfigID) ?? "") - : ""; + const lastUsedModelID = + initialLastModelConfigID && + modelOptions.some((option) => option.id === initialLastModelConfigID) + ? initialLastModelConfigID + : ""; const defaultModelID = (() => { - const defaultModelConfig = modelConfigs.find((config) => config.is_default); + const defaultModelConfig = Array.isArray(modelConfigs) + ? modelConfigs.find((config) => config.is_default) + : undefined; if (!defaultModelConfig) { return ""; } - return modelIDByConfigID.get(defaultModelConfig.id) ?? ""; + return modelOptions.some((option) => option.id === defaultModelConfig.id) + ? defaultModelConfig.id + : ""; })(); const preferredModelID = lastUsedModelID || defaultModelID || (modelOptions[0]?.id ?? ""); diff --git a/site/src/pages/AgentsPage/components/AgentDetailView.stories.tsx b/site/src/pages/AgentsPage/components/AgentDetailView.stories.tsx index 5fc9f28aa4..0681c6023a 100644 --- a/site/src/pages/AgentsPage/components/AgentDetailView.stories.tsx +++ b/site/src/pages/AgentsPage/components/AgentDetailView.stories.tsx @@ -21,9 +21,11 @@ import { // --------------------------------------------------------------------------- const AGENT_ID = "agent-detail-view-1"; +const defaultModelConfigID = "model-config-1"; + const defaultModelOptions: ModelSelectorOption[] = [ { - id: "openai:gpt-4o", + id: defaultModelConfigID, provider: "openai", model: "gpt-4o", displayName: "GPT-4o", @@ -37,7 +39,7 @@ const buildChat = (overrides: Partial = {}): TypesGen.Chat => ({ owner_id: "owner-1", title: "Help me refactor", status: "completed", - last_model_config_id: "model-config-1", + last_model_config_id: defaultModelConfigID, mcp_server_ids: [], labels: {}, created_at: oneWeekAgo, @@ -105,7 +107,7 @@ const StoryAgentDetailView: FC = ({ editing, ...overrides }) => { hasWorkspace: true, store: createChatStore(), pendingEditMessageId: null as number | null, - effectiveSelectedModel: "openai:gpt-4o", + effectiveSelectedModel: defaultModelConfigID, setSelectedModel: fn(), modelOptions: defaultModelOptions, modelSelectorPlaceholder: "Select a model", @@ -297,7 +299,7 @@ export const Loading: Story = { Loading — Agents} isInputDisabled - effectiveSelectedModel="openai:gpt-4o" + effectiveSelectedModel={defaultModelConfigID} setSelectedModel={fn()} modelOptions={defaultModelOptions} modelSelectorPlaceholder="Select a model" @@ -315,7 +317,7 @@ export const LoadingWithModelOptions: Story = { Loading — Agents} isInputDisabled={false} - effectiveSelectedModel="openai:gpt-4o" + effectiveSelectedModel={defaultModelConfigID} setSelectedModel={fn()} modelOptions={defaultModelOptions} modelSelectorPlaceholder="Select a model" @@ -332,7 +334,7 @@ export const LoadingWithRightPanel: Story = { Loading — Agents} isInputDisabled - effectiveSelectedModel="openai:gpt-4o" + effectiveSelectedModel={defaultModelConfigID} setSelectedModel={fn()} modelOptions={defaultModelOptions} modelSelectorPlaceholder="Select a model" @@ -350,7 +352,7 @@ export const LoadingSidebarCollapsed: Story = { Loading — Agents} isInputDisabled - effectiveSelectedModel="openai:gpt-4o" + effectiveSelectedModel={defaultModelConfigID} setSelectedModel={fn()} modelOptions={defaultModelOptions} modelSelectorPlaceholder="Select a model" diff --git a/site/src/pages/AgentsPage/components/MCPServerPicker.tsx b/site/src/pages/AgentsPage/components/MCPServerPicker.tsx index 24c3883a41..222a8cb774 100644 --- a/site/src/pages/AgentsPage/components/MCPServerPicker.tsx +++ b/site/src/pages/AgentsPage/components/MCPServerPicker.tsx @@ -94,8 +94,7 @@ export const mcpSelectionStorageKey = "agents.selected-mcp-server-ids"; * Read the persisted MCP selection from localStorage, filtered to only * include IDs that still exist in the current server list. * Returns `null` when nothing is stored (caller should fall back to defaults). - */ -export const getSavedMCPSelection = ( + */ export const getSavedMCPSelection = ( servers: readonly TypesGen.MCPServerConfig[], ): string[] | null => { const raw = localStorage.getItem(mcpSelectionStorageKey); @@ -138,8 +137,7 @@ export const getSavedMCPSelection = ( /** * Persist the current MCP selection to localStorage. - */ -export const saveMCPSelection = (ids: readonly string[]): void => { + */ export const saveMCPSelection = (ids: readonly string[]): void => { localStorage.setItem(mcpSelectionStorageKey, JSON.stringify(ids)); }; diff --git a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx index 809d72096b..8e1503839a 100644 --- a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx +++ b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx @@ -320,3 +320,95 @@ describe("AgentsSidebar load-more behavior", () => { expect(observeCount).toBe(0); }); }); + +describe("AgentsSidebar model display names", () => { + it("uses the chat model config ID to pick the correct duplicate model label", () => { + const modelOptions = [ + { + id: "config-fast", + provider: "openai", + model: "gpt-4o", + displayName: "GPT-4o (Fast)", + }, + { + id: "config-quality", + provider: "openai", + model: "gpt-4o", + displayName: "GPT-4o (Quality)", + }, + ]; + const modelConfigs: TypesGen.ChatModelConfig[] = [ + { + id: "config-fast", + provider: "openai", + model: "gpt-4o", + display_name: "GPT-4o (Fast)", + enabled: true, + is_default: false, + context_limit: 128_000, + compression_threshold: 70, + created_at: oneWeekAgo, + updated_at: oneWeekAgo, + }, + { + id: "config-quality", + provider: "openai", + model: "gpt-4o", + display_name: "GPT-4o (Quality)", + enabled: true, + is_default: false, + context_limit: 128_000, + compression_threshold: 70, + created_at: oneWeekAgo, + updated_at: oneWeekAgo, + }, + ]; + + const { getByText, queryByText } = render( + + + , + ); + + expect(getByText("GPT-4o (Quality)")).toBeInTheDocument(); + expect(queryByText("GPT-4o (Fast)")).not.toBeInTheDocument(); + }); + + it("falls back to legacy provider/model matching when no config ID match exists", () => { + const { getByText } = render( + + + , + ); + + expect(getByText("GPT-4o (Quality)")).toBeInTheDocument(); + }); +}); diff --git a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx index 6841ac9d12..aae941f6c2 100644 --- a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx +++ b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx @@ -179,35 +179,49 @@ const getModelDisplayName = ( modelConfigs: readonly ChatModelConfig[], modelOptions: readonly ModelSelectorOption[], ) => { - if (!lastModelConfigID) { + const normalizedModelConfigID = asString(lastModelConfigID).trim(); + if (!normalizedModelConfigID) { return "Default model"; } + + const modelOption = modelOptions.find( + (option) => option.id === normalizedModelConfigID, + ); + if (modelOption?.displayName) { + return modelOption.displayName; + } + const modelConfig = modelConfigs.find( - (config) => config.id === lastModelConfigID, + (config) => config.id === normalizedModelConfigID, ); if (!modelConfig) { + const legacyModelOption = modelOptions.find( + (option) => + `${option.provider}:${option.model}` === normalizedModelConfigID, + ); + if (legacyModelOption?.displayName) { + return legacyModelOption.displayName; + } return "Default model"; } - const { provider, model } = getNormalizedModelRef(modelConfig); + const displayName = asString(modelConfig.display_name).trim(); - if (!provider || !model) { - return displayName || "Default model"; - } - - // Try to find a matching option with a display name. - const match = modelOptions.find( - (opt) => - opt.id === `${provider}:${model}` || - (opt.provider === provider && opt.model === model), - ); - if (match?.displayName) { - return match.displayName; - } - if (displayName) { return displayName; } + const { provider, model } = getNormalizedModelRef(modelConfig); + if (!provider || !model) { + return "Default model"; + } + + const fallbackModelOption = modelOptions.find( + (option) => option.provider === provider && option.model === model, + ); + if (fallbackModelOption?.displayName) { + return fallbackModelOption.displayName; + } + return model; }; diff --git a/site/src/pages/AgentsPage/utils/modelOptions.test.ts b/site/src/pages/AgentsPage/utils/modelOptions.test.ts index 78f7ecb190..cef7e1e4a0 100644 --- a/site/src/pages/AgentsPage/utils/modelOptions.test.ts +++ b/site/src/pages/AgentsPage/utils/modelOptions.test.ts @@ -1,9 +1,50 @@ +import type { ChatModelConfig, ChatModelsResponse } from "api/typesGenerated"; import { describe, expect, it } from "vitest"; import { - getModelOptionsFromCatalog, + getModelOptionsFromConfigs, getNormalizedModelRef, + resolveModelOptionId, } from "./modelOptions"; +const createConfig = ( + overrides: Partial & + Pick, +): ChatModelConfig => { + const { + id, + provider, + model, + display_name, + enabled = true, + is_default = false, + context_limit = 0, + compression_threshold = 0, + model_config, + created_at = "", + updated_at = "", + } = overrides; + + return { + id, + provider, + model, + display_name: display_name ?? model, + enabled, + is_default, + context_limit, + compression_threshold, + model_config, + created_at, + updated_at, + }; +}; + +const createCatalog = ( + providers: ChatModelsResponse["providers"], +): ChatModelsResponse => ({ + providers, +}); + describe("getNormalizedModelRef", () => { it("returns empty strings for malformed values", () => { expect(getNormalizedModelRef({ provider: undefined, model: null })).toEqual( @@ -18,70 +59,280 @@ describe("getNormalizedModelRef", () => { }); }); -describe("getModelOptionsFromCatalog", () => { - it("skips malformed configs and catalog models without crashing", () => { - const catalog = { - providers: [ - { - provider: "openai", - available: true, - models: [ - { - id: " valid-model ", - provider: " OpenAI ", - model: " gpt-4o ", - display_name: " GPT‑4o ", - }, - { - id: "broken-model", - provider: undefined, - model: " gpt-4.1 ", - display_name: "Broken", - }, - { - id: " fallback-model ", - provider: " OpenAI ", - model: " zz-model ", - display_name: undefined, - }, - ], - }, - ], - } satisfies NonNullable[0]>; +describe("resolveModelOptionId", () => { + const modelOptions = [ + { + id: "config-1", + provider: "openai", + model: "gpt-4o", + displayName: "GPT-4o", + }, + { + id: "config-2", + provider: "anthropic", + model: "claude-sonnet-4-20250514", + displayName: "Claude Sonnet", + }, + ] as const; - const configs = [ - { - provider: undefined, - model: " gpt-4o ", - context_limit: 123, - }, - { - provider: " openai ", - model: " gpt-4o ", - context_limit: 456, - }, - { - provider: " openai ", - model: " zz-model ", - context_limit: 789, - }, - ] satisfies NonNullable[1]>; + it("returns an empty string for nullish and blank input", () => { + expect(resolveModelOptionId(undefined, modelOptions)).toBe(""); + expect(resolveModelOptionId(null, modelOptions)).toBe(""); + expect(resolveModelOptionId(" ", modelOptions)).toBe(""); + }); - expect(() => getModelOptionsFromCatalog(catalog, configs)).not.toThrow(); - expect(getModelOptionsFromCatalog(catalog, configs)).toEqual([ + it("returns the config ID for a direct match", () => { + expect(resolveModelOptionId("config-2", modelOptions)).toBe("config-2"); + }); + + it("returns the config ID for a legacy provider:model match", () => { + expect(resolveModelOptionId("openai:gpt-4o", modelOptions)).toBe( + "config-1", + ); + }); + + it("returns an empty string when no option matches", () => { + expect(resolveModelOptionId("openai:gpt-5", modelOptions)).toBe(""); + }); + + it("returns the first duplicate legacy match deterministically", () => { + const duplicateModelOptions = [ + ...modelOptions, { - id: "valid-model", + id: "config-3", provider: "openai", model: "gpt-4o", - displayName: "GPT‑4o", - contextLimit: 456, + displayName: "GPT-4o duplicate", + }, + ] as const; + + expect(resolveModelOptionId("openai:gpt-4o", duplicateModelOptions)).toBe( + "config-1", + ); + }); +}); + +describe("getModelOptionsFromConfigs", () => { + it("returns distinct options for configs with the same provider and model", () => { + const configs = [ + createConfig({ + id: "config-1", + provider: "openai", + model: "gpt-4o", + display_name: "GPT-4o (Fast)", + context_limit: 128_000, + }), + createConfig({ + id: "config-2", + provider: "openai", + model: "gpt-4o", + display_name: "GPT-4o (Quality)", + context_limit: 128_000, + }), + ]; + const catalog = createCatalog([ + { + provider: "openai", + available: true, + models: [], + }, + ]); + + expect(getModelOptionsFromConfigs(configs, catalog)).toEqual([ + { + id: "config-1", + provider: "openai", + model: "gpt-4o", + displayName: "GPT-4o (Fast)", + contextLimit: 128_000, }, { - id: "fallback-model", + id: "config-2", provider: "openai", - model: "zz-model", - displayName: "zz-model", - contextLimit: 789, + model: "gpt-4o", + displayName: "GPT-4o (Quality)", + contextLimit: 128_000, + }, + ]); + }); + + it("excludes configs whose providers are unavailable", () => { + const configs = [ + createConfig({ + id: "config-1", + provider: "anthropic", + model: "claude-sonnet-4-20250514", + display_name: "Claude Sonnet", + context_limit: 200_000, + }), + ]; + const catalog = createCatalog([ + { + provider: "anthropic", + available: false, + models: [], + }, + ]); + + expect(getModelOptionsFromConfigs(configs, catalog)).toEqual([]); + }); + + it("excludes disabled configs", () => { + const configs = [ + createConfig({ + id: "config-1", + provider: "openai", + model: "gpt-4o", + display_name: "GPT-4o", + enabled: false, + context_limit: 128_000, + }), + createConfig({ + id: "config-2", + provider: "openai", + model: "gpt-4.1", + display_name: "GPT-4.1", + context_limit: 128_000, + }), + ]; + const catalog = createCatalog([ + { + provider: "openai", + available: true, + models: [], + }, + ]); + + expect(getModelOptionsFromConfigs(configs, catalog)).toEqual([ + { + id: "config-2", + provider: "openai", + model: "gpt-4.1", + displayName: "GPT-4.1", + contextLimit: 128_000, + }, + ]); + }); + + it("falls back to the model name when display_name is blank", () => { + const configs = [ + createConfig({ + id: "config-1", + provider: " openai ", + model: " gpt-4o ", + display_name: " ", + context_limit: 0, + }), + ]; + const catalog = createCatalog([ + { + provider: "openai", + available: true, + models: [], + }, + ]); + + expect(getModelOptionsFromConfigs(configs, catalog)).toEqual([ + { + id: "config-1", + provider: "openai", + model: "gpt-4o", + displayName: "gpt-4o", + contextLimit: 0, + }, + ]); + }); + + it("returns an empty array for null and undefined inputs", () => { + expect(getModelOptionsFromConfigs(null, null)).toEqual([]); + expect(getModelOptionsFromConfigs(undefined, undefined)).toEqual([]); + }); + + it("sorts options by provider and display name", () => { + const configs = [ + createConfig({ + id: "config-openai-zeta", + provider: "openai", + model: "gpt-z", + display_name: "Zeta", + context_limit: 32_000, + }), + createConfig({ + id: "config-anthropic", + provider: "anthropic", + model: "claude-sonnet-4-20250514", + display_name: "Claude Sonnet", + context_limit: 200_000, + }), + createConfig({ + id: "config-openai-alpha", + provider: "openai", + model: "gpt-a", + display_name: "Alpha", + context_limit: 32_000, + }), + ]; + const catalog = createCatalog([ + { + provider: "openai", + available: true, + models: [], + }, + { + provider: "anthropic", + available: true, + models: [], + }, + ]); + + expect( + getModelOptionsFromConfigs(configs, catalog).map((option) => option.id), + ).toEqual([ + "config-anthropic", + "config-openai-alpha", + "config-openai-zeta", + ]); + }); + + it("keeps canonical wrapper-provider model strings distinct", () => { + const configs = [ + createConfig({ + id: "config-1", + provider: "openrouter", + model: "openai/gpt-4o", + display_name: "GPT-4o via OpenRouter", + context_limit: 128_000, + }), + createConfig({ + id: "config-2", + provider: "openrouter", + model: "anthropic/claude-sonnet-4-20250514", + display_name: "Claude via OpenRouter", + context_limit: 200_000, + }), + ]; + const catalog = createCatalog([ + { + provider: "openrouter", + available: true, + models: [], + }, + ]); + + expect(getModelOptionsFromConfigs(configs, catalog)).toEqual([ + { + id: "config-2", + provider: "openrouter", + model: "anthropic/claude-sonnet-4-20250514", + displayName: "Claude via OpenRouter", + contextLimit: 200_000, + }, + { + id: "config-1", + provider: "openrouter", + model: "openai/gpt-4o", + displayName: "GPT-4o via OpenRouter", + contextLimit: 128_000, }, ]); }); diff --git a/site/src/pages/AgentsPage/utils/modelOptions.ts b/site/src/pages/AgentsPage/utils/modelOptions.ts index d4f71b4404..98509fd44c 100644 --- a/site/src/pages/AgentsPage/utils/modelOptions.ts +++ b/site/src/pages/AgentsPage/utils/modelOptions.ts @@ -27,9 +27,14 @@ type ModelCatalogLike = { readonly providers?: readonly CatalogProviderLike[]; }; -type ChatModelConfigLike = - | Pick - | (RuntimeModelRef & Pick); +type ModelOptionConfigLike = + | TypesGen.ChatModelConfig + | (RuntimeModelRef & { + readonly id?: unknown; + readonly display_name?: unknown; + readonly enabled?: unknown; + readonly context_limit?: unknown; + }); export const getNormalizedModelRef = ( value: ModelRefLike, @@ -41,41 +46,6 @@ export const getNormalizedModelRef = ( }; }; -/** - * Build a lookup from model reference strings (both "provider:model" and - * "provider/model" forms) to model config IDs. - */ -export const buildModelConfigIDByModelID = ( - configs: - | readonly Pick[] - | undefined, -): ReadonlyMap => { - const byModelID = new Map(); - for (const config of configs ?? []) { - const { provider, model } = getNormalizedModelRef(config); - if (!provider || !model) continue; - const colonRef = `${provider}:${model}`; - if (!byModelID.has(colonRef)) byModelID.set(colonRef, config.id); - const slashRef = `${provider}/${model}`; - if (!byModelID.has(slashRef)) byModelID.set(slashRef, config.id); - } - return byModelID; -}; - -/** - * Build a reverse lookup from model config IDs back to model reference - * strings. Uses the first matching reference for each config ID. - */ -export const buildModelIDByConfigID = ( - modelConfigIDByModelID: ReadonlyMap, -): ReadonlyMap => { - const byConfigID = new Map(); - for (const [modelID, configID] of modelConfigIDByModelID.entries()) { - if (!byConfigID.has(configID)) byConfigID.set(configID, modelID); - } - return byConfigID; -}; - const getCatalogProviders = ( catalog: ModelCatalogLike | null | undefined, ): readonly CatalogProviderLike[] => { @@ -109,65 +79,89 @@ export const hasConfiguredModelsInCatalog = ( return getCatalogProviders(catalog).some(isProviderConfiguredInCatalog); }; -export const getModelOptionsFromCatalog = ( - catalog: ModelCatalogLike | null | undefined, - configs?: readonly ChatModelConfigLike[], -): readonly ModelSelectorOption[] => { - const optionsByID = new Map(); - - // Build a lookup of context limits from admin model configs so - // we can surface this in the model selector tooltip. - const contextLimitByKey = new Map(); - if (configs) { - for (const config of configs) { - const contextLimit = asNumber(config.context_limit); - if (contextLimit === undefined || contextLimit <= 0) { - continue; - } - const { provider, model } = getNormalizedModelRef(config); - if (!provider || !model) { - continue; - } - const key = `${provider}:${model}`; - if (!contextLimitByKey.has(key)) { - contextLimitByKey.set(key, contextLimit); - } - } - } - +const getAvailableProviders = ( + catalog: TypesGen.ChatModelsResponse | null | undefined, +): ReadonlySet => { + const availableProviders = new Set(); for (const provider of getCatalogProviders(catalog)) { - const models = getProviderModels(provider); - if (provider.available !== true || models.length === 0) { + if (provider.available !== true) { continue; } - for (const model of models) { - if (!model) { - continue; - } - - const modelID = asString(model.id).trim(); - const { provider: modelProvider, model: modelRef } = - getNormalizedModelRef(model); - if (!modelID || !modelProvider || !modelRef) { - continue; - } - if (optionsByID.has(modelID)) { - continue; - } - - const configKey = `${modelProvider.toLowerCase()}:${modelRef}`; - - optionsByID.set(modelID, { - id: modelID, - provider: modelProvider, - model: modelRef, - displayName: asString(model.display_name).trim() || modelRef, - contextLimit: contextLimitByKey.get(configKey), - }); + const providerName = asString(provider.provider).trim().toLowerCase(); + if (providerName) { + availableProviders.add(providerName); } } + return availableProviders; +}; - return Array.from(optionsByID.values()).sort((a, b) => { +/** + * Resolves a stored model reference (config ID or legacy + * "provider:model" string) to the ID of a matching model option. + * Returns the matched option ID, or an empty string if no match is + * found. + */ +export const resolveModelOptionId = ( + storedRef: string | null | undefined, + modelOptions: readonly ModelSelectorOption[], +): string => { + const normalized = asString(storedRef).trim(); + if (!normalized) { + return ""; + } + + const directMatch = modelOptions.find((option) => option.id === normalized); + if (directMatch) { + return directMatch.id; + } + + const legacyMatch = modelOptions.find( + (option) => `${option.provider}:${option.model}` === normalized, + ); + if (legacyMatch) { + return legacyMatch.id; + } + + return ""; +}; + +export const getModelOptionsFromConfigs = ( + configs: readonly TypesGen.ChatModelConfig[] | null | undefined, + catalog: TypesGen.ChatModelsResponse | null | undefined, +): readonly ModelSelectorOption[] => { + if (!configs || !catalog) { + return []; + } + + const availableProviders = getAvailableProviders(catalog); + const options: ModelSelectorOption[] = []; + + for (const config of configs as readonly ModelOptionConfigLike[]) { + if (config.enabled !== true) { + continue; + } + + const configID = asString(config.id).trim(); + const { provider, model } = getNormalizedModelRef(config); + if (!configID || !provider || !model) { + continue; + } + if (!availableProviders.has(provider)) { + continue; + } + + const displayName = asString(config.display_name).trim() || model; + const contextLimit = asNumber(config.context_limit); + options.push({ + id: configID, + provider, + model, + displayName, + ...(contextLimit !== undefined ? { contextLimit } : {}), + }); + } + + return options.sort((a, b) => { const providerCompare = a.provider.localeCompare(b.provider); if (providerCompare !== 0) { return providerCompare;