diff --git a/apps/app/.ladle/model-picker-query-provider.tsx b/apps/app/.ladle/model-picker-query-provider.tsx index 852466ffb9..19a4c73959 100644 --- a/apps/app/.ladle/model-picker-query-provider.tsx +++ b/apps/app/.ladle/model-picker-query-provider.tsx @@ -101,9 +101,7 @@ function makeAvailableModels({ id: model.value, model: model.value, displayName: model.label, - ...(model.routeProviderId - ? { routeProviderId: model.routeProviderId } - : {}), + ...(model.qualifier ? { routeProviderId: model.qualifier } : {}), description: "", supportedReasoningEfforts, defaultReasoningEffort, diff --git a/apps/app/.ladle/story-fixtures.ts b/apps/app/.ladle/story-fixtures.ts index 1ee7dc5316..7e2e5eb54e 100644 --- a/apps/app/.ladle/story-fixtures.ts +++ b/apps/app/.ladle/story-fixtures.ts @@ -145,47 +145,47 @@ export const STORY_PI_MODELS: readonly ModelPickerOption[] = [ { value: "openai-codex/gpt-5.5", label: "GPT-5.5", - routeProviderId: "openai-codex", + qualifier: "openai-codex", }, { value: "openai-codex/gpt-5.4", label: "GPT-5.4", - routeProviderId: "openai-codex", + qualifier: "openai-codex", }, { value: "openai-codex/gpt-5.4-mini", label: "GPT-5.4 Mini", - routeProviderId: "openai-codex", + qualifier: "openai-codex", }, { value: "openai-codex/gpt-5.3-codex", label: "GPT-5.3 Codex", - routeProviderId: "openai-codex", + qualifier: "openai-codex", }, { value: "openai/gpt-5.3-codex-spark", label: "GPT-5.3 Codex Spark", - routeProviderId: "openai", + qualifier: "openai", }, { value: "openai-codex/gpt-5.3-codex-spark", label: "GPT-5.3 Codex Spark", - routeProviderId: "openai-codex", + qualifier: "openai-codex", }, { value: "anthropic/claude-haiku-4-5", label: "Claude Haiku 4.5", - routeProviderId: "anthropic", + qualifier: "anthropic", }, { value: "anthropic/claude-opus-4-8", label: "Claude Opus 4.8", - routeProviderId: "anthropic", + qualifier: "anthropic", }, { value: "anthropic/claude-opus-4-7", label: "Claude Opus 4.7", - routeProviderId: "anthropic", + qualifier: "anthropic", }, ]; diff --git a/apps/app/src/components/pickers/ModelReasoningPicker.test.tsx b/apps/app/src/components/pickers/ModelReasoningPicker.test.tsx index 3ed5b7ce46..38d2ab146f 100644 --- a/apps/app/src/components/pickers/ModelReasoningPicker.test.tsx +++ b/apps/app/src/components/pickers/ModelReasoningPicker.test.tsx @@ -106,17 +106,19 @@ const splitPaneContext: PaneContextValue = { function availableModel({ value, label, + description = "", isDefault = false, }: { value: string; label: string; + description?: string; isDefault?: boolean; }): AvailableModel { return { id: value, model: value, displayName: label, - description: "", + description, supportedReasoningEfforts: [ { reasoningEffort: "medium", description: "Medium" }, ], @@ -520,11 +522,11 @@ describe("ModelReasoningPicker", () => { const modelLabel = "GPT-5.3 Codex Spark"; const { onModelChange } = renderPicker({ modelOptions: [ - { value: apiModel, label: modelLabel, routeProviderId: "openai" }, + { value: apiModel, label: modelLabel, qualifier: "openai" }, { value: subscriptionModel, label: modelLabel, - routeProviderId: "openai-codex", + qualifier: "openai-codex", }, ], modelValue: subscriptionModel, @@ -549,6 +551,55 @@ describe("ModelReasoningPicker", () => { expect(onModelChange).toHaveBeenCalledWith(apiModel); }); + it("tells apart previewed models that share a display name (#2062)", async () => { + // omp exposes the same display name under several route prefixes; the + // agent's short description (its "provider/model" id) is the only thing + // that distinguishes them, so it fills the inline qualifier slot. + const { onModelChange } = renderPicker({ + alternateProviderModels: [ + availableModel({ + value: "github-copilot/gpt-5.1", + label: "GPT-5.1", + description: "github-copilot/gpt-5.1", + isDefault: true, + }), + availableModel({ + value: "openai-codex/gpt-5.1", + label: "GPT-5.1", + description: "openai-codex/gpt-5.1", + }), + availableModel({ + value: "github-copilot/gpt-5.2", + label: "GPT-5.2", + description: "github-copilot/gpt-5.2", + }), + ], + }); + fireEvent.click( + screen.getByRole("button", { name: "Provider, model and reasoning" }), + ); + fireEvent.click(screen.getByTitle("Claude Code")); + + const copilotRow = ( + await screen.findByText("github-copilot/gpt-5.1") + ).closest("button"); + const codexRow = screen.getByText("openai-codex/gpt-5.1").closest("button"); + expect(copilotRow).not.toBeNull(); + expect(codexRow).not.toBeNull(); + expect(copilotRow).not.toBe(codexRow); + expect(copilotRow?.textContent).toBe("GPT-5.1github-copilot/gpt-5.1"); + expect(codexRow?.textContent).toBe("GPT-5.1openai-codex/gpt-5.1"); + // A unique label keeps its plain single-segment row. + expect(screen.getByText("GPT-5.2").closest("button")?.textContent).toBe( + "GPT-5.2", + ); + expect(screen.queryByText("github-copilot/gpt-5.2")).toBeNull(); + + fireEvent.click(screen.getByText("openai-codex/gpt-5.1")); + + expect(onModelChange).toHaveBeenCalledWith("openai-codex/gpt-5.1"); + }); + it("fuzzy-filters a long model list and selects the match by keyboard", () => { const { onModelChange } = renderPicker({ modelOptions: manyCodexModels }); diff --git a/apps/app/src/components/pickers/ModelReasoningPicker.tsx b/apps/app/src/components/pickers/ModelReasoningPicker.tsx index 322e6ba592..ff7462b53a 100644 --- a/apps/app/src/components/pickers/ModelReasoningPicker.tsx +++ b/apps/app/src/components/pickers/ModelReasoningPicker.tsx @@ -53,7 +53,10 @@ import { OPTION_TRIGGER_CONTENT_CLASS_NAME, } from "@bb/shared-ui/option-display"; import { type PickerOption } from "./OptionPicker"; -import type { ModelPickerOption } from "./model-picker-option"; +import { + toModelPickerOptions, + type ModelPickerOption, +} from "./model-picker-option"; import { formatModelLoadErrorText, ModelLoadErrorMessage, @@ -148,9 +151,11 @@ function modelSearchText( option: ModelPickerOption, brandPrefix: string | undefined, ): string { - return `${stripModelBrandPrefix(option.label, brandPrefix)} ${option.routeProviderId ?? ""} ${option.value}`; + return `${stripModelBrandPrefix(option.label, brandPrefix)} ${option.qualifier ?? ""} ${option.value}`; } +const identityLabel = (displayName: string): string => displayName; + /** * A keyboard-navigable row in the model list. Each entry maps 1:1 to a rendered, * highlightable row and drives arrow movement, Enter handling, and the active @@ -384,29 +389,13 @@ export function ModelReasoningPicker({ if (!isPreviewing) return modelOptions; const models = previewQuery.data?.models; if (!models || models.length === 0) return []; - return models.map((model) => ({ - value: model.model, - label: formatModelLabel - ? formatModelLabel(model.displayName || model.model) - : model.displayName || model.model, - ...(model.routeProviderId - ? { routeProviderId: model.routeProviderId } - : {}), - })); + return toModelPickerOptions(models, formatModelLabel ?? identityLabel); }, [isPreviewing, modelOptions, previewQuery.data?.models, formatModelLabel]); const previewMoreModelOptions = useMemo((): readonly ModelPickerOption[] => { if (!isPreviewing) return moreModelOptions; const models = previewQuery.data?.selectedOnlyModels; if (!models || models.length === 0) return []; - return models.map((model) => ({ - value: model.model, - label: formatModelLabel - ? formatModelLabel(model.displayName || model.model) - : model.displayName || model.model, - ...(model.routeProviderId - ? { routeProviderId: model.routeProviderId } - : {}), - })); + return toModelPickerOptions(models, formatModelLabel ?? identityLabel); }, [ isPreviewing, moreModelOptions, @@ -1045,7 +1034,7 @@ export function ModelReasoningPicker({ option.label, activeBrandPrefix, )} - qualifier={option.routeProviderId} + qualifier={option.qualifier} selected={!isPreviewing && option.value === modelValue} onClick={() => handleModelSelect(option.value)} /> @@ -1362,7 +1351,7 @@ function MoreModelsSubmenu({ onSelect(option.value)} /> diff --git a/apps/app/src/components/pickers/model-picker-option.test.ts b/apps/app/src/components/pickers/model-picker-option.test.ts new file mode 100644 index 0000000000..9e7d3fc61e --- /dev/null +++ b/apps/app/src/components/pickers/model-picker-option.test.ts @@ -0,0 +1,117 @@ +import type { AvailableModel } from "@bb/domain"; +import { describe, expect, it } from "vitest"; +import { toModelPickerOptions } from "./model-picker-option"; + +function model( + id: string, + displayName: string, + description = "", + extra: Partial = {}, +): AvailableModel { + return { + id, + model: id, + displayName, + description, + supportedReasoningEfforts: [], + defaultReasoningEffort: "medium", + isDefault: false, + ...extra, + }; +} + +const identity = (label: string) => label; + +describe("toModelPickerOptions", () => { + it("leaves rows with unique labels exactly as before, whatever their description", () => { + expect( + toModelPickerOptions( + [ + model( + "claude-opus-4-7", + "Claude Opus 4.7", + "Most capable model for complex work", + ), + model("claude-sonnet-4-6", "Claude Sonnet 4.6", "Fast and smart"), + ], + identity, + ), + ).toEqual([ + { value: "claude-opus-4-7", label: "Claude Opus 4.7" }, + { value: "claude-sonnet-4-6", label: "Claude Sonnet 4.6" }, + ]); + }); + + it("qualifies only the rows whose formatted labels collide", () => { + expect( + toModelPickerOptions( + [ + model("github-copilot/gpt-5.1", "GPT-5.1", "github-copilot/gpt-5.1"), + model("openai-codex/gpt-5.1", "GPT-5.1", "openai-codex/gpt-5.1"), + model("openai-codex/gpt-5.2", "GPT-5.2", "openai-codex/gpt-5.2"), + ], + identity, + ), + ).toEqual([ + { + value: "github-copilot/gpt-5.1", + label: "GPT-5.1", + qualifier: "github-copilot/gpt-5.1", + }, + { + value: "openai-codex/gpt-5.1", + label: "GPT-5.1", + qualifier: "openai-codex/gpt-5.1", + }, + { value: "openai-codex/gpt-5.2", label: "GPT-5.2" }, + ]); + }); + + it("falls back to the raw model id when the description is missing or too long to be an identifier", () => { + expect( + toModelPickerOptions( + [ + model("vendor-a/glm-4.7", "GLM 4.7"), + model( + "vendor-b/glm-4.7", + "GLM 4.7", + "A long-form marketing sentence describing the model at length.", + ), + ], + identity, + ), + ).toEqual([ + { + value: "vendor-a/glm-4.7", + label: "GLM 4.7", + qualifier: "vendor-a/glm-4.7", + }, + { + value: "vendor-b/glm-4.7", + label: "GLM 4.7", + qualifier: "vendor-b/glm-4.7", + }, + ]); + }); + + it("detects collisions on the formatted label and keeps a route provider as the qualifier", () => { + expect( + toModelPickerOptions( + [ + model("openai/gpt-5", "gpt-5", "api", { routeProviderId: "openai" }), + model("openai-codex/gpt-5", "GPT-5", "subscription", { + routeProviderId: "openai-codex", + }), + ], + (label) => label.toUpperCase(), + ), + ).toEqual([ + { value: "openai/gpt-5", label: "GPT-5", qualifier: "openai" }, + { + value: "openai-codex/gpt-5", + label: "GPT-5", + qualifier: "openai-codex", + }, + ]); + }); +}); diff --git a/apps/app/src/components/pickers/model-picker-option.ts b/apps/app/src/components/pickers/model-picker-option.ts index c7f746f1f2..e83674c2bd 100644 --- a/apps/app/src/components/pickers/model-picker-option.ts +++ b/apps/app/src/components/pickers/model-picker-option.ts @@ -1,6 +1,56 @@ +import type { AvailableModel } from "@bb/domain"; import type { PickerOption } from "./OptionPicker"; -/** A model option can expose a distinct runtime route beside its friendly name. */ +/** + * A model row in the picker. `qualifier` is short inline text, rendered after + * the label and in its tooltip, that tells apart rows whose labels collide: a + * Pi model's nested route provider, or (for any provider) the agent's short + * description or raw model id when two models share a display name. + */ export interface ModelPickerOption extends PickerOption { - routeProviderId?: string; + qualifier?: string; +} + +// Longer descriptions are marketing copy, not an identifier; the raw model id +// is the shorter and more useful disambiguator then. +const MAX_DESCRIPTION_QUALIFIER_LENGTH = 40; + +function collidingModelQualifier(model: AvailableModel): string { + const description = model.description.trim(); + return description.length > 0 && + description.length <= MAX_DESCRIPTION_QUALIFIER_LENGTH + ? description + : model.model; +} + +/** + * Maps a provider's model catalog to picker rows. Only rows whose formatted + * label is shared with another row in the same list get a qualifier (unless + * the model already carries a route provider), so a catalog of unique names + * renders exactly as before. + */ +export function toModelPickerOptions( + models: readonly AvailableModel[], + formatLabel: (displayName: string) => string, +): ModelPickerOption[] { + const labeled = models.map((model) => ({ + model, + label: formatLabel(model.displayName || model.model), + })); + const labelCounts = new Map(); + for (const { label } of labeled) { + labelCounts.set(label, (labelCounts.get(label) ?? 0) + 1); + } + return labeled.map(({ model, label }): ModelPickerOption => { + const qualifier = + model.routeProviderId ?? + ((labelCounts.get(label) ?? 0) > 1 + ? collidingModelQualifier(model) + : undefined); + return { + value: model.model, + label, + ...(qualifier ? { qualifier } : {}), + }; + }); } diff --git a/apps/app/src/hooks/useThreadCreationOptions.test.tsx b/apps/app/src/hooks/useThreadCreationOptions.test.tsx index 9b2b17360c..6d043cef80 100644 --- a/apps/app/src/hooks/useThreadCreationOptions.test.tsx +++ b/apps/app/src/hooks/useThreadCreationOptions.test.tsx @@ -597,7 +597,7 @@ describe("useThreadCreationOptions", () => { expect(result.current.modelOptions[0]).toEqual({ value: "global-model", label: "Global Model", - routeProviderId: "openai-codex", + qualifier: "openai-codex", }); }); }); diff --git a/apps/app/src/hooks/useThreadCreationOptions.ts b/apps/app/src/hooks/useThreadCreationOptions.ts index 1c565f2f41..894d1c0fee 100644 --- a/apps/app/src/hooks/useThreadCreationOptions.ts +++ b/apps/app/src/hooks/useThreadCreationOptions.ts @@ -22,7 +22,10 @@ import type { SystemProvidersQuery, } from "@bb/server-contract"; import type { PickerOption } from "@/components/pickers/OptionPicker"; -import type { ModelPickerOption } from "@/components/pickers/model-picker-option"; +import { + toModelPickerOptions, + type ModelPickerOption, +} from "@/components/pickers/model-picker-option"; import type { ProviderPickerOption } from "@/components/pickers/model-brand-prefix"; import { parseEnvironmentValue } from "@/components/pickers/environment-picker-value"; import { PERMISSION_MODE_OPTIONS } from "@/lib/permission-mode-options"; @@ -603,13 +606,7 @@ export function useThreadCreationOptions( const modelOptions = useMemo( (): ModelPickerOption[] => - availableModels.map((model) => ({ - value: model.model, - label: formatModelLabel(model.displayName || model.model), - ...(model.routeProviderId - ? { routeProviderId: model.routeProviderId } - : {}), - })), + toModelPickerOptions(availableModels, formatModelLabel), [availableModels], ); @@ -618,18 +615,13 @@ export function useThreadCreationOptions( // here rather than listed twice. const moreModelOptions = useMemo( (): ModelPickerOption[] => - (executionOptionsQuery.data?.selectedOnlyModels ?? []) - .filter( + toModelPickerOptions( + (executionOptionsQuery.data?.selectedOnlyModels ?? []).filter( (model) => !availableModels.some((active) => active.model === model.model), - ) - .map((model) => ({ - value: model.model, - label: formatModelLabel(model.displayName || model.model), - ...(model.routeProviderId - ? { routeProviderId: model.routeProviderId } - : {}), - })), + ), + formatModelLabel, + ), [executionOptionsQuery.data?.selectedOnlyModels, availableModels], ); diff --git a/plugins/provider-acp/src/bridge/model-catalog.test.ts b/plugins/provider-acp/src/bridge/model-catalog.test.ts index 64085c1197..ed1e11ba90 100644 --- a/plugins/provider-acp/src/bridge/model-catalog.test.ts +++ b/plugins/provider-acp/src/bridge/model-catalog.test.ts @@ -438,6 +438,55 @@ describe("acp configOptions model catalog", () => { ]); }); + it("keeps the per-option description the agent sends (#2062)", () => { + // omp sends description: "provider/modelId" on every model select option, + // and two providers can expose the same display name; the description is + // the only field that tells them apart downstream. + const models = buildModelCatalogFromConfigOptions({ + id: "model", + category: "model", + type: "select", + currentValue: "github-copilot/gpt-5.1", + options: [ + { + value: "github-copilot/gpt-5.1", + name: "GPT-5.1", + description: "github-copilot/gpt-5.1", + }, + { + value: "openai-codex/gpt-5.1", + name: "GPT-5.1", + description: "openai-codex/gpt-5.1", + }, + { value: "opencode/big-pickle", name: "Big Pickle" }, + ], + }); + + expect( + models.map((model) => ({ + id: model.id, + displayName: model.displayName, + description: model.description, + })), + ).toEqual([ + { + id: "github-copilot/gpt-5.1", + displayName: "GPT-5.1", + description: "github-copilot/gpt-5.1", + }, + { + id: "openai-codex/gpt-5.1", + displayName: "GPT-5.1", + description: "openai-codex/gpt-5.1", + }, + { + id: "opencode/big-pickle", + displayName: "Big Pickle", + description: "", + }, + ]); + }); + it("finds and maps ACP thought_level config options", () => { const thoughtLevel = { id: "effort", diff --git a/plugins/provider-acp/src/bridge/model-catalog.ts b/plugins/provider-acp/src/bridge/model-catalog.ts index 8f3a11ce65..a23b6f9c57 100644 --- a/plugins/provider-acp/src/bridge/model-catalog.ts +++ b/plugins/provider-acp/src/bridge/model-catalog.ts @@ -280,7 +280,7 @@ export function buildModelCatalogFromConfigOptions( id: option.value, model: option.value, displayName: option.name ?? option.value, - description: "", + description: option.description ?? "", supportedReasoningEfforts: reasoning.supportedReasoningEfforts, defaultReasoningEffort: reasoning.defaultReasoningEffort, isDefault, diff --git a/plugins/provider-acp/src/wire.test.ts b/plugins/provider-acp/src/wire.test.ts index beff3c8cf7..f1f6fc59e0 100644 --- a/plugins/provider-acp/src/wire.test.ts +++ b/plugins/provider-acp/src/wire.test.ts @@ -47,6 +47,11 @@ describe("acpSessionNewResultSchema", () => { { value: "openai-codex/gpt-5.5", name: "openai-codex/GPT-5.5", + description: "openai-codex/gpt-5.5", + }, + { + value: "github-copilot/gpt-5.5", + name: "GPT-5.5", description: null, }, ], @@ -72,6 +77,12 @@ describe("acpSessionNewResultSchema", () => { expect(parsed.data.configOptions?.[0].options?.[0].name).toBe( "openai-codex/GPT-5.5", ); + expect(parsed.data.configOptions?.[0].options?.[0].description).toBe( + "openai-codex/gpt-5.5", + ); + expect( + parsed.data.configOptions?.[0].options?.[1].description, + ).toBeUndefined(); expect(parsed.data.configOptions?.[1].category).toBeUndefined(); expect(parsed.data.configOptions?.[1].options?.[0].name).toBeUndefined(); }); diff --git a/plugins/provider-acp/src/wire.ts b/plugins/provider-acp/src/wire.ts index 97234a07f8..3e96cbea0d 100644 --- a/plugins/provider-acp/src/wire.ts +++ b/plugins/provider-acp/src/wire.ts @@ -234,6 +234,7 @@ const acpConfigOptionSelectOptionSchema = z .object({ value: z.string(), name: acpOptionalString, + description: acpOptionalString, }) .passthrough();