fix(coding-agent): fixed model selector to handle multiple roles on same model

- Added formatRoleThinkingModeLabel helper to display 'inherit' for default thinking mode, preventing badge ambiguity when multiple roles share the same model. Enhanced role menu labels to include role tags for clarity. Fixed model resolver to avoid substring matching that could incorrectly resolve exact model IDs to similar variants.
This commit is contained in:
can1357
2026-03-04 23:21:10 +01:00
parent 1427e93183
commit 2e001bbf8d
4 changed files with 100 additions and 16 deletions
@@ -16,6 +16,11 @@ function makeInvertedBadge(label: string, color: ThemeColor): string {
return `${bgAnsi}\x1b[30m ${label} \x1b[39m\x1b[49m`;
}
function formatRoleThinkingModeLabel(thinkingMode: RoleThinkingMode): string {
if (thinkingMode === "default") return "inherit";
return formatThinkingEffortLabel(thinkingMode);
}
interface ModelItem {
provider: string;
id: string;
@@ -41,10 +46,14 @@ interface MenuRoleAction {
role: ModelRole;
}
const MENU_ROLE_ACTIONS: MenuRoleAction[] = MODEL_ROLE_IDS.map(role => ({
label: `Set as ${MODEL_ROLES[role].name}`,
role,
}));
const MENU_ROLE_ACTIONS: MenuRoleAction[] = MODEL_ROLE_IDS.map(role => {
const roleInfo = MODEL_ROLES[role];
const roleLabel = roleInfo.tag ? `${roleInfo.tag} (${roleInfo.name})` : roleInfo.name;
return {
label: `Set as ${roleLabel}`,
role,
};
});
const THINKING_MODE_OPTIONS: RoleThinkingMode[] = ["default", "off", "minimal", "low", "medium", "high"];
const ALL_TAB = "ALL";
@@ -403,12 +412,7 @@ export class ModelSelectorComponent extends Container {
if (!tag || !assigned || !modelsAreEqual(assigned.model, item.model)) continue;
const badge = makeInvertedBadge(tag, color ?? "success");
if (assigned.thinkingMode === "default") {
roleBadgeTokens.push(badge);
continue;
}
const thinkingLabel = formatThinkingEffortLabel(assigned.thinkingMode);
const thinkingLabel = formatRoleThinkingModeLabel(assigned.thinkingMode);
roleBadgeTokens.push(`${badge} ${theme.fg("dim", `(${thinkingLabel})`)}`);
}
const badgeText = roleBadgeTokens.length > 0 ? ` ${roleBadgeTokens.join(" ")}` : "";
@@ -503,7 +507,8 @@ export class ModelSelectorComponent extends Container {
const optionLines = showingThinking
? thinkingOptions.map((thinkingMode, index) => {
const prefix = index === this.#menuSelectedIndex ? ` ${theme.nav.cursor} ` : " ";
return `${prefix}${thinkingMode}`;
const label = formatRoleThinkingModeLabel(thinkingMode);
return `${prefix}${label}`;
})
: MENU_ROLE_ACTIONS.map((action, index) => {
const prefix = index === this.#menuSelectedIndex ? ` ${theme.nav.cursor} ` : " ";
@@ -124,6 +124,34 @@ describe("AgentSession role model thinking behavior", () => {
expect(session.thinkingLevel).toBe("minimal");
});
it("applies slow role thinking even when plan shares the same model", async () => {
const defaultModel = getAnthropicModelOrThrow("claude-sonnet-4-5");
const smolModel = getAnthropicModelOrThrow("claude-sonnet-4-6");
const slowPlanModel = getAnthropicModelOrThrow("claude-opus-4-5");
await createSession({
initialModelId: defaultModel.id,
initialThinkingLevel: "medium",
modelRoles: {
default: `${defaultModel.provider}/${defaultModel.id}`,
smol: `${smolModel.provider}/${smolModel.id}:low`,
slow: `${slowPlanModel.provider}/${slowPlanModel.id}:high`,
plan: `${slowPlanModel.provider}/${slowPlanModel.id}:off`,
},
});
const toSmol = await session.cycleRoleModels(["slow", "default", "smol"]);
expect(toSmol?.role).toBe("smol");
expect(toSmol?.thinkingLevel).toBe("low");
expect(session.thinkingLevel).toBe("low");
const toSlow = await session.cycleRoleModels(["slow", "default", "smol"]);
expect(toSlow?.role).toBe("slow");
expect(toSlow?.model.id).toBe(slowPlanModel.id);
expect(toSlow?.thinkingLevel).toBe("high");
expect(session.thinkingLevel).toBe("high");
});
it("preserves explicit role thinking when updating default model despite unresolved previous model", async () => {
const defaultModel = getAnthropicModelOrThrow("claude-sonnet-4-5");
const slowModel = getAnthropicModelOrThrow("claude-sonnet-4-6");
@@ -92,7 +92,34 @@ const mockProviderOverlapModels: Model<"anthropic-messages">[] = [
},
];
const allModels = [...mockModels, ...mockOpenRouterModels, ...mockProviderOverlapModels];
const mockCodexOverlapModels: Model<"anthropic-messages">[] = [
{
id: "gpt-5.3-codex",
name: "GPT-5.3 Codex",
api: "anthropic-messages",
provider: "openai-codex",
baseUrl: "https://api.openai.com",
reasoning: true,
input: ["text"],
cost: { input: 1.5, output: 6, cacheRead: 0.15, cacheWrite: 1.5 },
contextWindow: 200000,
maxTokens: 8192,
},
{
id: "gpt-5.3-codex-spark",
name: "GPT-5.3 Codex Spark",
api: "anthropic-messages",
provider: "openai-codex",
baseUrl: "https://api.openai.com",
reasoning: true,
input: ["text"],
cost: { input: 1, output: 4, cacheRead: 0.1, cacheWrite: 1 },
contextWindow: 200000,
maxTokens: 8192,
},
];
const allModels = [...mockModels, ...mockOpenRouterModels, ...mockProviderOverlapModels, ...mockCodexOverlapModels];
describe("parseModelPattern", () => {
describe("simple patterns without colons", () => {
@@ -298,6 +325,20 @@ describe("resolveModelRoleValue", () => {
expect(result.explicitThinkingLevel).toBe(false);
expect(result.warning).toBeUndefined();
});
test("does not resolve exact codex role values to codex-spark via substring matching", () => {
const providerQualified = resolveModelRoleValue("openai-codex/gpt-5.3-codex:xhigh", allModels);
expect(providerQualified.model?.provider).toBe("openai-codex");
expect(providerQualified.model?.id).toBe("gpt-5.3-codex");
expect(providerQualified.thinkingLevel).toBe("xhigh");
expect(providerQualified.explicitThinkingLevel).toBe(true);
const idOnly = resolveModelRoleValue("gpt-5.3-codex:xhigh", allModels);
expect(idOnly.model?.provider).toBe("openai-codex");
expect(idOnly.model?.id).toBe("gpt-5.3-codex");
expect(idOnly.thinkingLevel).toBe("xhigh");
expect(idOnly.explicitThinkingLevel).toBe(true);
});
});
describe("resolveModelFromString", () => {
test("falls back to pattern parsing for provider/model:thinking when strict provider+id miss", () => {
@@ -22,7 +22,7 @@ describe("ModelSelector role badge thinking display", () => {
initTheme();
});
test("renders explicit thinking next to role badges and removes details duplication", async () => {
test("renders per-role thinking labels with inherit mode to avoid badge ambiguity", async () => {
const model = getBundledModel("anthropic", "claude-sonnet-4-5");
if (!model) throw new Error("Expected bundled model anthropic/claude-sonnet-4-5");
@@ -30,7 +30,8 @@ describe("ModelSelector role badge thinking display", () => {
modelRoles: {
default: `${model.provider}/${model.id}`,
smol: `${model.provider}/${model.id}:minimal`,
slow: `${model.provider}/${model.id}:off`,
slow: `${model.provider}/${model.id}`,
plan: `${model.provider}/${model.id}:high`,
commit: `${model.provider}/${model.id}:medium`,
},
});
@@ -55,10 +56,19 @@ describe("ModelSelector role badge thinking display", () => {
await Bun.sleep(0);
const rendered = normalizeRenderedText(selector.render(220).join("\n"));
expect(rendered).toContain("DEFAULT (inherit)");
expect(rendered).toContain("SMOL (min)");
expect(rendered).toContain("SLOW (off)");
expect(rendered).toContain("SLOW (inherit)");
expect(rendered).toContain("PLAN (high)");
expect(rendered).toContain("COMMIT (medium)");
expect(rendered).not.toContain("DEFAULT (");
expect(rendered).not.toContain("Role Thinking:");
selector.handleInput("\n");
const menuRendered = normalizeRenderedText(selector.render(220).join("\n"));
expect(menuRendered).toContain("Set as DEFAULT (Default)");
expect(menuRendered).toContain("Set as SMOL (Fast)");
expect(menuRendered).toContain("Set as SLOW (Thinking)");
expect(menuRendered).toContain("Set as PLAN (Architect)");
expect(menuRendered).toContain("Set as COMMIT (Commit)");
});
});