Skip to content

Keep ACP model descriptions and disambiguate same-name picker rows #2226

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.

Sign up for GitHub

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

Open
SawyerHood wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Keep ACP model descriptions and disambiguate same-name picker rows #2226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 1 addition & 3 deletions apps/app/.ladle/model-picker-query-provider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 9 additions & 9 deletions apps/app/.ladle/story-fixtures.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
];

Expand Down
57 changes: 54 additions & 3 deletions apps/app/src/components/pickers/ModelReasoningPicker.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
],
Expand Down Expand Up @@ -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,
Expand All @@ -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 });

Expand Down
33 changes: 11 additions & 22 deletions apps/app/src/components/pickers/ModelReasoningPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)}
/>
Expand Down Expand Up @@ -1362,7 +1351,7 @@ function MoreModelsSubmenu({
<MenuRowButton
key={option.value}
label={stripModelBrandPrefix(option.label, activeBrandPrefix)}
qualifier={option.routeProviderId}
qualifier={option.qualifier}
selected={!isPreviewing && option.value === modelValue}
onClick={() => onSelect(option.value)}
/>
Expand Down
117 changes: 117 additions & 0 deletions apps/app/src/components/pickers/model-picker-option.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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> = {},
): 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",
},
]);
});
});
Loading
Loading