mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
fix: distinguish model options with duplicate labels (#1286)
Fixes #1246. When `availableModels` includes aliases and pinned IDs that inherit the same SDK display name, name-only model pickers show indistinguishable rows. Append the selectable model ID to names that remain duplicated after the existing version-label normalization. For example, the four `Opus 5.5` rows become `Opus 5.5 (opus)`, `Opus 5.5 (opus[1m])`, and their explicit-ID equivalents. Keep every option and its ID, description, selection, and recommendation behavior. Existing distinct labels stay unchanged. Regression tests exercise the real allowlist transformation and cover duplicate custom labels too. Validation: - Linux, locked dependencies and the repository's `lts/*` Node configuration: `npm run check`, `npm run build`, and the full test suite passed atba37dd8: **2,491 passed, 30 skipped** ([fix job](https://github.com/jipeng6036-del/claude-agent-acp/actions/runs/37923655565/job/113797353655)). - Exact baseline908c994: **2,489 passed, 30 skipped**; both new regression tests fail against the original implementation. All existing test outcomes match ([baseline verification](https://github.com/jipeng6036-del/claude-agent-acp/actions/runs/37923859076)). - Windows/Node 24: build and 105 focused tests passed. The full suite has the same 48 failures on baseline and fix; only the two new tests add passes. Authenticated SDK and JetBrains UI integration were not run. Signed-off-by: Jasper <241105014+jipeng6036-del@users.noreply.github.com> Co-authored-by: Jasper <241105014+jipeng6036-del@users.noreply.github.com>
This commit is contained in:
@@ -595,12 +595,20 @@ export async function getAvailableModels(
|
|||||||
for (const name of displayNames) {
|
for (const name of displayNames) {
|
||||||
displayNameCounts.set(name, (displayNameCounts.get(name) ?? 0) + 1);
|
displayNameCounts.set(name, (displayNameCounts.get(name) ?? 0) + 1);
|
||||||
}
|
}
|
||||||
|
const names = models.map((model, index) =>
|
||||||
|
displayNameCounts.get(displayNames[index]) === 1 ? displayNames[index] : model.displayName,
|
||||||
|
);
|
||||||
|
const nameCounts = new Map<string, number>();
|
||||||
|
for (const name of names) {
|
||||||
|
nameCounts.set(name, (nameCounts.get(name) ?? 0) + 1);
|
||||||
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
availableModels: models.map((model, index) => ({
|
availableModels: models.map((model, index) => ({
|
||||||
modelId: model.value,
|
modelId: model.value,
|
||||||
name:
|
// Allowlisted aliases can inherit the same SDK label. Keep every ID
|
||||||
displayNameCounts.get(displayNames[index]) === 1 ? displayNames[index] : model.displayName,
|
// selectable, but distinguish the rows for clients that show only names.
|
||||||
|
name: nameCounts.get(names[index]) === 1 ? names[index] : `${names[index]} (${model.value})`,
|
||||||
description: model.description,
|
description: model.description,
|
||||||
})),
|
})),
|
||||||
currentModelId: currentModel.value,
|
currentModelId: currentModel.value,
|
||||||
|
|||||||
@@ -1,6 +1,10 @@
|
|||||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
import { query, type ModelInfo, type Query } from "@anthropic-ai/claude-agent-sdk";
|
import { query, type ModelInfo, type Query } from "@anthropic-ai/claude-agent-sdk";
|
||||||
import { buildModelConfigOption, getAvailableModels } from "../session-model.js";
|
import {
|
||||||
|
applyAvailableModelsAllowlist,
|
||||||
|
buildModelConfigOption,
|
||||||
|
getAvailableModels,
|
||||||
|
} from "../session-model.js";
|
||||||
|
|
||||||
const OPUS: ModelInfo = {
|
const OPUS: ModelInfo = {
|
||||||
value: "opus[1m]",
|
value: "opus[1m]",
|
||||||
@@ -151,6 +155,68 @@ describe("versioned model display names", () => {
|
|||||||
);
|
);
|
||||||
expect(state.availableModels.map((model) => model.name)).toEqual(["Opus (1M context)", "Opus"]);
|
expect(state.availableModels.map((model) => model.name)).toEqual(["Opus (1M context)", "Opus"]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("distinguishes allowlisted aliases and pins that inherit the same SDK label", async () => {
|
||||||
|
const sdkModels: ModelInfo[] = [
|
||||||
|
{ value: "default", displayName: "Default (recommended)", description: "" },
|
||||||
|
{
|
||||||
|
value: "opus",
|
||||||
|
resolvedModel: "claude-opus-5-5",
|
||||||
|
displayName: "Opus 5.5",
|
||||||
|
description: "For complex work and everyday tasks",
|
||||||
|
},
|
||||||
|
{ value: "haiku", displayName: "Haiku", description: "Fast" },
|
||||||
|
];
|
||||||
|
const ids = ["claude-opus-5-5[1m]", "claude-opus-5-5", "opus[1m]", "opus", "haiku"];
|
||||||
|
const models = applyAvailableModelsAllowlist(sdkModels, ids);
|
||||||
|
const setModel = vi.fn();
|
||||||
|
const state = await getAvailableModels(
|
||||||
|
{ setModel } as unknown as Query,
|
||||||
|
models,
|
||||||
|
sdkModels,
|
||||||
|
{ getSettings: () => ({}) },
|
||||||
|
{ log: vi.fn(), error: vi.fn() },
|
||||||
|
false,
|
||||||
|
"model-presentation-test",
|
||||||
|
);
|
||||||
|
|
||||||
|
const expected = [
|
||||||
|
"Default (recommended)",
|
||||||
|
...ids.slice(0, 4).map((id) => `Opus 5.5 (${id})`),
|
||||||
|
"Haiku",
|
||||||
|
];
|
||||||
|
expect(state.availableModels.map((model) => model.name)).toEqual(expected);
|
||||||
|
expect(state.availableModels.map((model) => model.modelId)).toEqual(["default", ...ids]);
|
||||||
|
expect(state.currentModelId).toBe("default");
|
||||||
|
expect(state.availableModels.slice(1, 5).map((model) => model.description)).toEqual(
|
||||||
|
Array(4).fill(sdkModels[1].description),
|
||||||
|
);
|
||||||
|
const option = buildModelConfigOption(state, models, false);
|
||||||
|
expect(option).toMatchObject({ currentValue: "default" });
|
||||||
|
expect(option.type === "select" && option.options.map((model) => model.name)).toEqual(expected);
|
||||||
|
expect(sdkModels[1].displayName).toBe("Opus 5.5");
|
||||||
|
expect(setModel).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("distinguishes identical custom labels without changing their model IDs", async () => {
|
||||||
|
const models: ModelInfo[] = [
|
||||||
|
{ value: "provider-a", displayName: "My model", description: "First endpoint" },
|
||||||
|
{ value: "provider-b", displayName: "My model", description: "Second endpoint" },
|
||||||
|
];
|
||||||
|
const state = await getAvailableModels(
|
||||||
|
{} as Query,
|
||||||
|
models,
|
||||||
|
models,
|
||||||
|
{ getSettings: () => ({}) },
|
||||||
|
{ log: vi.fn(), error: vi.fn() },
|
||||||
|
false,
|
||||||
|
"model-presentation-test",
|
||||||
|
);
|
||||||
|
expect(state.availableModels).toEqual([
|
||||||
|
{ modelId: "provider-a", name: "My model (provider-a)", description: "First endpoint" },
|
||||||
|
{ modelId: "provider-b", name: "My model (provider-b)", description: "Second endpoint" },
|
||||||
|
]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
// Account-dependent model availability requires the same authenticated environment
|
// Account-dependent model availability requires the same authenticated environment
|
||||||
|
|||||||
Reference in New Issue
Block a user