From e265c20d580a34173a7f711cc195f00487ac28c8 Mon Sep 17 00:00:00 2001 From: Leo P Date: Tue, 17 Mar 2026 12:41:29 -0400 Subject: [PATCH 1/5] WIP --- .../coding-agent/src/config/model-registry.ts | 31 ++++++++ .../src/config/settings-schema.ts | 27 +++++++ .../src/modes/components/model-selector.ts | 73 +++++++++++++------ .../src/modes/controllers/input-controller.ts | 6 +- .../modes/controllers/selector-controller.ts | 4 +- .../coding-agent/src/session/agent-session.ts | 10 +-- .../test/cycle-order-custom-roles.test.ts | 32 ++++++++ packages/coding-agent/test/role-info.test.ts | 67 +++++++++++++++++ 8 files changed, 217 insertions(+), 33 deletions(-) create mode 100644 packages/coding-agent/test/cycle-order-custom-roles.test.ts create mode 100644 packages/coding-agent/test/role-info.test.ts diff --git a/packages/coding-agent/src/config/model-registry.ts b/packages/coding-agent/src/config/model-registry.ts index 29a49a7d8..23c82059b 100644 --- a/packages/coding-agent/src/config/model-registry.ts +++ b/packages/coding-agent/src/config/model-registry.ts @@ -30,6 +30,7 @@ import { type Static, Type } from "@sinclair/typebox"; import { type ConfigError, ConfigFile } from "../config"; import type { ThemeColor } from "../modes/theme/theme"; import type { AuthStorage, OAuthCredential } from "../session/auth-storage"; +import type { Settings } from "./settings"; export const kNoAuth = "N/A"; @@ -57,6 +58,36 @@ export const MODEL_ROLES: Record = { export const MODEL_ROLE_IDS: ModelRole[] = ["default", "smol", "slow", "vision", "plan", "commit", "task"]; +/** Alias for ModelRoleInfo - used for both built-in and custom roles */ +export type RoleInfo = ModelRoleInfo; + +/** + * Get role info for a role name (built-in or custom). + * Returns built-in role info if the role is predefined, + * otherwise looks up custom role from settings, or returns a fallback. + */ +export function getRoleInfo(role: string, settings: Settings): RoleInfo { + // Check if it's a built-in role + if (role in MODEL_ROLES) { + return MODEL_ROLES[role as ModelRole]; + } + + // Check if it's a custom role in settings + const customTags = settings.get("modelTags"); + if (customTags && typeof customTags === "object" && role in customTags) { + const tagDef = (customTags as Record)[role]; + if (tagDef && typeof tagDef === "object") { + return { + name: tagDef.name || role, + color: tagDef.color as ThemeColor | undefined, + }; + } + } + + // Fallback for undefined roles + return { name: role, color: "muted" }; +} + const OpenRouterRoutingSchema = Type.Object({ only: Type.Optional(Type.Array(Type.String())), order: Type.Optional(Type.Array(Type.String())), diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 260855051..8311dbec4 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -135,10 +135,21 @@ type SettingDef = // Schema Definition // ═══════════════════════════════════════════════════════════════════════════ +export interface ModelTagDef { + name: string; + color?: string; +} + +export interface ModelTagsSettings { + [key: string]: ModelTagDef; +} + // Typed defaults for array/record settings — named constants avoid `as` casts // under `as const` while still letting SettingValue infer the correct element type. const EMPTY_STRING_ARRAY: string[] = []; const EMPTY_STRING_RECORD: Record = {}; +const DEFAULT_CYCLE_ORDER: string[] = ["smol", "default", "slow"]; +const EMPTY_MODEL_TAGS_RECORD: ModelTagsSettings = {}; export const DEFAULT_BASH_INTERCEPTOR_RULES: BashInterceptorRule[] = [ { pattern: "^\\s*(cat|head|tail|less|more)\\s+", @@ -195,6 +206,10 @@ export const SETTINGS_SCHEMA = { modelRoles: { type: "record", default: EMPTY_STRING_RECORD }, + modelTags: { type: "record", default: EMPTY_MODEL_TAGS_RECORD }, + + cycleOrder: { type: "array", default: DEFAULT_CYCLE_ORDER }, + // ──────────────────────────────────────────────────────────────────────── // Appearance // ──────────────────────────────────────────────────────────────────────── @@ -980,6 +995,16 @@ export const SETTINGS_SCHEMA = { default: false, ui: { tab: "editing", label: "Bash Interceptor", description: "Block shell commands that have dedicated tools" }, }, + + "bashInterceptor.simpleLs": { + type: "boolean", + default: true, + ui: { + tab: "editing", + label: "Intercept `ls`", + description: "Intercept bare ls commands (when interceptor is enabled)", + }, + }, "bashInterceptor.patterns": { type: "array", default: DEFAULT_BASH_INTERCEPTOR_RULES }, // Python @@ -1767,6 +1792,8 @@ export interface GroupTypeMap { thinkingBudgets: ThinkingBudgetsSettings; stt: SttSettings; modelRoles: Record; + modelTags: ModelTagsSettings; + cycleOrder: string[]; } export type GroupPrefix = keyof GroupTypeMap; diff --git a/packages/coding-agent/src/modes/components/model-selector.ts b/packages/coding-agent/src/modes/components/model-selector.ts index 2108ce48d..3c91db537 100644 --- a/packages/coding-agent/src/modes/components/model-selector.ts +++ b/packages/coding-agent/src/modes/components/model-selector.ts @@ -12,7 +12,8 @@ import { type TUI, visibleWidth, } from "@oh-my-pi/pi-tui"; -import { MODEL_ROLE_IDS, MODEL_ROLES, type ModelRegistry, type ModelRole } from "../../config/model-registry"; +import type { ModelRegistry } from "../../config/model-registry"; +import { getRoleInfo, MODEL_ROLE_IDS, MODEL_ROLES } from "../../config/model-registry"; import { resolveModelRoleValue } from "../../config/model-resolver"; import type { Settings } from "../../config/settings"; import { type ThemeColor, theme } from "../../modes/theme/theme"; @@ -43,22 +44,13 @@ interface RoleAssignment { thinkingLevel: ThinkingLevel; } -type RoleSelectCallback = (model: Model, role: ModelRole | null, thinkingLevel?: ThinkingLevel) => void; +type RoleSelectCallback = (model: Model, role: string | null, thinkingLevel?: ThinkingLevel) => void; type CancelCallback = () => void; interface MenuRoleAction { label: string; - role: ModelRole; + role: string; // now accepts custom role strings } -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 ALL_TAB = "ALL"; /** @@ -77,7 +69,7 @@ export class ModelSelectorComponent extends Container { #allModels: ModelItem[] = []; #filteredModels: ModelItem[] = []; #selectedIndex: number = 0; - #roles = {} as Record; + #roles = {} as Record; #settings = null as unknown as Settings; #modelRegistry = null as unknown as ModelRegistry; #onSelectCallback = (() => {}) as RoleSelectCallback; @@ -87,6 +79,8 @@ export class ModelSelectorComponent extends Container { #scopedModels: ReadonlyArray; #temporaryOnly: boolean; + #menuRoleActions: MenuRoleAction[] = []; + // Tab state #providers: string[] = [ALL_TAB]; #activeTabIndex: number = 0; @@ -95,7 +89,7 @@ export class ModelSelectorComponent extends Container { #isMenuOpen: boolean = false; #menuSelectedIndex: number = 0; #menuStep: "role" | "thinking" = "role"; - #menuSelectedRole: ModelRole | null = null; + #menuSelectedRole: string | null = null; constructor( tui: TUI, @@ -103,7 +97,7 @@ export class ModelSelectorComponent extends Container { settings: Settings, modelRegistry: ModelRegistry, scopedModels: ReadonlyArray, - onSelect: (model: Model, role: ModelRole | null, thinkingLevel?: ThinkingLevel) => void, + onSelect: (model: Model, role: string | null, thinkingLevel?: ThinkingLevel) => void, onCancel: () => void, options?: { temporaryOnly?: boolean; initialSearchInput?: string }, ) { @@ -118,6 +112,9 @@ export class ModelSelectorComponent extends Container { this.#temporaryOnly = options?.temporaryOnly ?? false; const initialSearchInput = options?.initialSearchInput; + // Initialize menu role actions (built-in + custom from settings) + this.#buildMenuRoleActions(); + // Load current role assignments from settings this.#loadRoleModels(); @@ -184,6 +181,36 @@ export class ModelSelectorComponent extends Container { }); } + #buildMenuRoleActions(): void { + const actions: MenuRoleAction[] = []; + + // Add built-in roles + for (const role of MODEL_ROLE_IDS) { + const roleInfo = MODEL_ROLES[role]; + const roleLabel = roleInfo.tag ? `${roleInfo.tag} (${roleInfo.name})` : roleInfo.name; + actions.push({ + label: `Set as ${roleLabel}`, + role: role as string, + }); + } + + // Add custom roles from settings + const customTags = this.#settings.get("modelTags"); + if (customTags && typeof customTags === "object") { + for (const [role, tagDef] of Object.entries(customTags as Record)) { + if (tagDef && typeof tagDef === "object" && "name" in tagDef) { + const roleLabel = tagDef.name; + actions.push({ + label: `Set as ${roleLabel}`, + role, + }); + } + } + } + + this.#menuRoleActions = actions; + } + #loadRoleModels(): void { const allModels = this.#modelRegistry.getAll(); const matchPreferences = { usageOrder: this.#settings.getStorage()?.getModelUsageOrder() }; @@ -527,11 +554,11 @@ export class ModelSelectorComponent extends Container { return [ThinkingLevel.Inherit, ThinkingLevel.Off, ...getSupportedEfforts(model)]; } - #getCurrentRoleThinkingLevel(role: ModelRole): ThinkingLevel { + #getCurrentRoleThinkingLevel(role: string): ThinkingLevel { return this.#roles[role]?.thinkingLevel ?? ThinkingLevel.Inherit; } - #getThinkingPreselectIndex(role: ModelRole, model: Model): number { + #getThinkingPreselectIndex(role: string, model: Model): number { const options = this.#getThinkingLevelsForModel(model); const currentLevel = this.#getCurrentRoleThinkingLevel(role); const foundIndex = options.indexOf(currentLevel); @@ -569,12 +596,12 @@ export class ModelSelectorComponent extends Container { const label = getThinkingLevelMetadata(thinkingLevel).label; return `${prefix}${label}`; }) - : MENU_ROLE_ACTIONS.map((action, index) => { + : this.#menuRoleActions.map((action, index) => { const prefix = index === this.#menuSelectedIndex ? ` ${theme.nav.cursor} ` : " "; return `${prefix}${action.label}`; }); - const selectedRoleName = this.#menuSelectedRole ? MODEL_ROLES[this.#menuSelectedRole].name : ""; + const selectedRoleName = this.#menuSelectedRole ? getRoleInfo(this.#menuSelectedRole, this.#settings).name : ""; const headerText = showingThinking && this.#menuSelectedRole ? ` Thinking for: ${selectedRoleName} (${selectedModel.id})` @@ -674,7 +701,7 @@ export class ModelSelectorComponent extends Container { const optionCount = this.#menuStep === "thinking" && this.#menuSelectedRole !== null ? this.#getThinkingLevelsForModel(selectedModel.model).length - : MENU_ROLE_ACTIONS.length; + : this.#menuRoleActions.length; if (optionCount === 0) return; if (matchesKey(keyData, "up")) { @@ -691,7 +718,7 @@ export class ModelSelectorComponent extends Container { if (matchesKey(keyData, "enter") || matchesKey(keyData, "return") || keyData === "\n") { if (this.#menuStep === "role") { - const action = MENU_ROLE_ACTIONS[this.#menuSelectedIndex]; + const action = this.#menuRoleActions[this.#menuSelectedIndex]; if (!action) return; this.#menuSelectedRole = action.role; this.#menuStep = "thinking"; @@ -712,7 +739,7 @@ export class ModelSelectorComponent extends Container { if (getKeybindings().matches(keyData, "tui.select.cancel")) { if (this.#menuStep === "thinking" && this.#menuSelectedRole !== null) { this.#menuStep = "role"; - const roleIndex = MENU_ROLE_ACTIONS.findIndex(action => action.role === this.#menuSelectedRole); + const roleIndex = this.#menuRoleActions.findIndex(action => action.role === this.#menuSelectedRole); this.#menuSelectedRole = null; this.#menuSelectedIndex = roleIndex >= 0 ? roleIndex : 0; this.#updateMenu(); @@ -728,7 +755,7 @@ export class ModelSelectorComponent extends Container { if (thinkingLevel === ThinkingLevel.Inherit) return modelKey; return `${modelKey}:${thinkingLevel}`; } - #handleSelect(model: Model, role: ModelRole | null, thinkingLevel?: ThinkingLevel): void { + #handleSelect(model: Model, role: string | null, thinkingLevel?: ThinkingLevel): void { // For temporary role, don't save to settings - just notify caller if (role === null) { this.#onSelectCallback(model, null); diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts index 9af32c5d3..be0d5494a 100644 --- a/packages/coding-agent/src/modes/controllers/input-controller.ts +++ b/packages/coding-agent/src/modes/controllers/input-controller.ts @@ -595,8 +595,8 @@ export class InputController { async cycleRoleModel(options?: { temporary?: boolean }): Promise { try { - const roleOrder = ["smol", "default", "slow"] as const; - const result = await this.ctx.session.cycleRoleModels(roleOrder, options); + const cycleOrder = settings.get("cycleOrder") || ["smol", "default", "slow"]; + const result = await this.ctx.session.cycleRoleModels(cycleOrder, options); if (!result) { this.ctx.showStatus("Only one role model available"); return; @@ -612,7 +612,7 @@ export class InputController { : ""; const tempLabel = options?.temporary ? " (temporary)" : ""; const cycleSeparator = theme.fg("dim", " > "); - const cycleLabel = roleOrder + const cycleLabel = cycleOrder .map(role => { if (role === result.role) { return theme.bold(theme.fg("accent", role)); diff --git a/packages/coding-agent/src/modes/controllers/selector-controller.ts b/packages/coding-agent/src/modes/controllers/selector-controller.ts index b30850872..aa00d207c 100644 --- a/packages/coding-agent/src/modes/controllers/selector-controller.ts +++ b/packages/coding-agent/src/modes/controllers/selector-controller.ts @@ -3,7 +3,7 @@ import { getOAuthProviders, type OAuthProvider } from "@oh-my-pi/pi-ai"; import type { Component } from "@oh-my-pi/pi-tui"; import { Input, Loader, Spacer, Text } from "@oh-my-pi/pi-tui"; import { getAgentDbPath, getProjectDir } from "@oh-my-pi/pi-utils"; -import { MODEL_ROLES } from "../../config/model-registry"; +import { getRoleInfo } from "../../config/model-registry"; import { settings } from "../../config/settings"; import { DebugSelectorComponent } from "../../debug"; import { disableProvider, enableProvider } from "../../discovery"; @@ -406,7 +406,7 @@ export class SelectorController { // Don't call done() - selector stays open for role assignment } else { // Other roles (smol, slow): just update settings, not current model - const roleInfo = MODEL_ROLES[role]; + const roleInfo = getRoleInfo(role, settings); const roleLabel = roleInfo?.name ?? role; this.ctx.showStatus(`${roleLabel} model: ${model.id}`); // Don't call done() - selector stays open diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index ffe123444..a334aa87c 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -267,7 +267,7 @@ export interface ModelCycleResult { export interface RoleModelCycleResult { model: Model; thinkingLevel: ThinkingLevel | undefined; - role: ModelRole; + role: string; } /** Session statistics for /session command */ @@ -3077,7 +3077,7 @@ export class AgentSession { * Validates API key, saves to session and settings. * @throws Error if no API key available for the model */ - async setModel(model: Model, role: ModelRole = "default"): Promise { + async setModel(model: Model, role: string = "default"): Promise { const apiKey = await this.#modelRegistry.getApiKey(model, this.sessionId); if (!apiKey) { throw new Error(`No API key for ${model.provider}/${model.id}`); @@ -3131,7 +3131,7 @@ export class AgentSession { * @param options - Optional settings: `temporary` to not persist to settings */ async cycleRoleModels( - roleOrder: readonly ModelRole[], + roleOrder: readonly string[], options?: { temporary?: boolean }, ): Promise { const availableModels = this.#modelRegistry.getAvailable(); @@ -3141,7 +3141,7 @@ export class AgentSession { if (!currentModel) return undefined; const matchPreferences = { usageOrder: this.settings.getStorage()?.getModelUsageOrder() }; const roleModels: Array<{ - role: ModelRole; + role: string; model: Model; thinkingLevel?: ThinkingLevel; explicitThinkingLevel: boolean; @@ -4102,7 +4102,7 @@ export class AgentSession { return `${model.provider}/${model.id}`; } - #formatRoleModelValue(role: ModelRole, model: Model): string { + #formatRoleModelValue(role: string, model: Model): string { const modelKey = `${model.provider}/${model.id}`; const existingRoleValue = this.settings.getModelRole(role); if (!existingRoleValue) return modelKey; diff --git a/packages/coding-agent/test/cycle-order-custom-roles.test.ts b/packages/coding-agent/test/cycle-order-custom-roles.test.ts new file mode 100644 index 000000000..f28416e16 --- /dev/null +++ b/packages/coding-agent/test/cycle-order-custom-roles.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, test } from "bun:test"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; + +describe("cycleOrder with custom roles", () => { + test("cycleOrder setting accepts custom role names", () => { + const settings = Settings.isolated({ + cycleOrder: ["smol", "custom-fast", "default"], + }); + expect(settings.get("cycleOrder")).toEqual(["smol", "custom-fast", "default"]); + }); + + test("cycleOrder falls back to default when not set", () => { + const settings = Settings.isolated({}); + expect(settings.get("cycleOrder")).toEqual(["smol", "default", "slow"]); + }); + + test("modelTags can define custom role display info", () => { + const settings = Settings.isolated({ + modelTags: { + "custom-fast": { + name: "Fast Custom", + color: "warning", + }, + }, + }); + const modelTags = settings.get("modelTags") as Record; + expect(modelTags["custom-fast"]).toEqual({ + name: "Fast Custom", + color: "warning", + }); + }); +}); diff --git a/packages/coding-agent/test/role-info.test.ts b/packages/coding-agent/test/role-info.test.ts new file mode 100644 index 000000000..2bfac776e --- /dev/null +++ b/packages/coding-agent/test/role-info.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, test } from "bun:test"; +import { getRoleInfo } from "@oh-my-pi/pi-coding-agent/config/model-registry"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; + +describe("getRoleInfo", () => { + test("returns built-in role info", () => { + const settings = Settings.isolated({}); + + expect(getRoleInfo("default", settings)).toEqual({ + name: "Default", + color: "success", + tag: "DEFAULT", + }); + expect(getRoleInfo("smol", settings)).toEqual({ + name: "Fast", + color: "warning", + tag: "SMOL", + }); + expect(getRoleInfo("slow", settings)).toEqual({ + name: "Thinking", + color: "accent", + tag: "SLOW", + }); + }); + + test("returns custom role info from modelTags", () => { + const settings = Settings.isolated({ + modelTags: { + custom: { name: "My Custom Tag", color: "error" }, + another: { name: "Another Tag" }, + }, + }); + + expect(getRoleInfo("custom", settings)).toEqual({ + name: "My Custom Tag", + color: "error", + }); + expect(getRoleInfo("another", settings)).toEqual({ + name: "Another Tag", + color: undefined, + }); + }); + + test("returns fallback for unknown roles", () => { + const settings = Settings.isolated({}); + + expect(getRoleInfo("unknown-role", settings)).toEqual({ + name: "unknown-role", + color: "muted", + }); + }); + + test("custom role does not override built-in roles", () => { + const settings = Settings.isolated({ + modelTags: { + smol: { name: "My Smol", color: "success" }, + }, + }); + + // Built-in 'smol' always returns built-in info, ignoring custom tag + expect(getRoleInfo("smol", settings)).toEqual({ + name: "Fast", + color: "warning", + tag: "SMOL", + }); + }); +}); From 2416249b7f79ecee72829c1ac09edc1edf969bf2 Mon Sep 17 00:00:00 2001 From: Leo P Date: Tue, 17 Mar 2026 12:41:29 -0400 Subject: [PATCH 2/5] WIP --- packages/coding-agent/src/config/settings-schema.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 8311dbec4..e44961c4c 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -995,16 +995,6 @@ export const SETTINGS_SCHEMA = { default: false, ui: { tab: "editing", label: "Bash Interceptor", description: "Block shell commands that have dedicated tools" }, }, - - "bashInterceptor.simpleLs": { - type: "boolean", - default: true, - ui: { - tab: "editing", - label: "Intercept `ls`", - description: "Intercept bare ls commands (when interceptor is enabled)", - }, - }, "bashInterceptor.patterns": { type: "array", default: DEFAULT_BASH_INTERCEPTOR_RULES }, // Python From afdfe24ea3445045948791d8dfc12a7f578cadd9 Mon Sep 17 00:00:00 2001 From: Leo P Date: Wed, 18 Mar 2026 15:45:50 -0400 Subject: [PATCH 3/5] cleanup --- .../coding-agent/src/config/model-registry.ts | 10 +-- .../src/modes/components/model-selector.ts | 48 ++++++++++--- .../src/modes/controllers/input-controller.ts | 2 +- .../coding-agent/src/modes/theme/theme.ts | 68 +++++++++++++++++++ .../coding-agent/src/session/agent-session.ts | 13 ++-- 5 files changed, 118 insertions(+), 23 deletions(-) diff --git a/packages/coding-agent/src/config/model-registry.ts b/packages/coding-agent/src/config/model-registry.ts index 23c82059b..ddfb58168 100644 --- a/packages/coding-agent/src/config/model-registry.ts +++ b/packages/coding-agent/src/config/model-registry.ts @@ -28,7 +28,7 @@ import { import { isRecord, logger } from "@oh-my-pi/pi-utils"; import { type Static, Type } from "@sinclair/typebox"; import { type ConfigError, ConfigFile } from "../config"; -import type { ThemeColor } from "../modes/theme/theme"; +import { isValidThemeColor, type ThemeColor } from "../modes/theme/theme"; import type { AuthStorage, OAuthCredential } from "../session/auth-storage"; import type { Settings } from "./settings"; @@ -74,12 +74,12 @@ export function getRoleInfo(role: string, settings: Settings): RoleInfo { // Check if it's a custom role in settings const customTags = settings.get("modelTags"); - if (customTags && typeof customTags === "object" && role in customTags) { - const tagDef = (customTags as Record)[role]; - if (tagDef && typeof tagDef === "object") { + if (customTags && role in customTags) { + const tagDef = customTags[role]; + if (tagDef) { return { name: tagDef.name || role, - color: tagDef.color as ThemeColor | undefined, + color: tagDef.color && isValidThemeColor(tagDef.color) ? tagDef.color : undefined, }; } } diff --git a/packages/coding-agent/src/modes/components/model-selector.ts b/packages/coding-agent/src/modes/components/model-selector.ts index 3c91db537..2c8d2db73 100644 --- a/packages/coding-agent/src/modes/components/model-selector.ts +++ b/packages/coding-agent/src/modes/components/model-selector.ts @@ -190,21 +190,17 @@ export class ModelSelectorComponent extends Container { const roleLabel = roleInfo.tag ? `${roleInfo.tag} (${roleInfo.name})` : roleInfo.name; actions.push({ label: `Set as ${roleLabel}`, - role: role as string, + role, }); } // Add custom roles from settings - const customTags = this.#settings.get("modelTags"); - if (customTags && typeof customTags === "object") { - for (const [role, tagDef] of Object.entries(customTags as Record)) { - if (tagDef && typeof tagDef === "object" && "name" in tagDef) { - const roleLabel = tagDef.name; - actions.push({ - label: `Set as ${roleLabel}`, - role, - }); - } + for (const [role, tagDef] of Object.entries(this.#settings.get("modelTags"))) { + if (tagDef.name) { + actions.push({ + label: `Set as ${tagDef.name}`, + role, + }); } } @@ -230,6 +226,27 @@ export class ModelSelectorComponent extends Container { }; } } + + // Load custom roles from modelTags settings + for (const [role] of Object.entries(this.#settings.get("modelTags"))) { + if (role in MODEL_ROLES) continue; + const roleValue = this.#settings.getModelRole(role); + if (!roleValue) continue; + + const resolved = resolveModelRoleValue(roleValue, allModels, { + settings: this.#settings, + matchPreferences, + }); + if (resolved.model) { + this.#roles[role] = { + model: resolved.model, + thinkingLevel: + resolved.explicitThinkingLevel && resolved.thinkingLevel !== undefined + ? resolved.thinkingLevel + : ThinkingLevel.Inherit, + }; + } + } } #sortModels(models: ModelItem[]): void { @@ -505,6 +522,15 @@ export class ModelSelectorComponent extends Container { const thinkingLabel = getThinkingLevelMetadata(assigned.thinkingLevel).label; roleBadgeTokens.push(`${badge} ${theme.fg("dim", `(${thinkingLabel})`)}`); } + // Custom role badges + for (const [role, assigned] of Object.entries(this.#roles)) { + if (role in MODEL_ROLES || !assigned || !modelsAreEqual(assigned.model, item.model)) continue; + const roleInfo = getRoleInfo(role, this.#settings); + const badgeLabel = roleInfo.tag ?? roleInfo.name; + const badge = makeInvertedBadge(badgeLabel, roleInfo.color ?? "muted"); + const thinkingLabel = getThinkingLevelMetadata(assigned.thinkingLevel).label; + roleBadgeTokens.push(`${badge} ${theme.fg("dim", `(${thinkingLabel})`)}`); + } const badgeText = roleBadgeTokens.length > 0 ? ` ${roleBadgeTokens.join(" ")}` : ""; let line = ""; diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts index be0d5494a..d386d0fd6 100644 --- a/packages/coding-agent/src/modes/controllers/input-controller.ts +++ b/packages/coding-agent/src/modes/controllers/input-controller.ts @@ -595,7 +595,7 @@ export class InputController { async cycleRoleModel(options?: { temporary?: boolean }): Promise { try { - const cycleOrder = settings.get("cycleOrder") || ["smol", "default", "slow"]; + const cycleOrder = settings.get("cycleOrder"); const result = await this.ctx.session.cycleRoleModels(cycleOrder, options); if (!result) { this.ctx.showStatus("Only one role model available"); diff --git a/packages/coding-agent/src/modes/theme/theme.ts b/packages/coding-agent/src/modes/theme/theme.ts index eef4aa9f3..03157f4ab 100644 --- a/packages/coding-agent/src/modes/theme/theme.ts +++ b/packages/coding-agent/src/modes/theme/theme.ts @@ -955,6 +955,74 @@ export type ThemeColor = | "statusLineCost" | "statusLineSubagents"; +/** Set of all valid ThemeColor string values for runtime validation */ +const VALID_THEME_COLORS: ReadonlySet = new Set([ + "accent", + "border", + "borderAccent", + "borderMuted", + "success", + "error", + "warning", + "muted", + "dim", + "text", + "thinkingText", + "userMessageText", + "customMessageText", + "customMessageLabel", + "toolTitle", + "toolOutput", + "mdHeading", + "mdLink", + "mdLinkUrl", + "mdCode", + "mdCodeBlock", + "mdCodeBlockBorder", + "mdQuote", + "mdQuoteBorder", + "mdHr", + "mdListBullet", + "toolDiffAdded", + "toolDiffRemoved", + "toolDiffContext", + "syntaxComment", + "syntaxKeyword", + "syntaxFunction", + "syntaxVariable", + "syntaxString", + "syntaxNumber", + "syntaxType", + "syntaxOperator", + "syntaxPunctuation", + "thinkingOff", + "thinkingMinimal", + "thinkingLow", + "thinkingMedium", + "thinkingHigh", + "thinkingXhigh", + "bashMode", + "pythonMode", + "statusLineSep", + "statusLineModel", + "statusLinePath", + "statusLineGitClean", + "statusLineGitDirty", + "statusLineContext", + "statusLineSpend", + "statusLineStaged", + "statusLineDirty", + "statusLineUntracked", + "statusLineOutput", + "statusLineCost", + "statusLineSubagents", +]); + +/** Check if a string is a valid ThemeColor value */ +export function isValidThemeColor(color: string): color is ThemeColor { + return VALID_THEME_COLORS.has(color); +} + export type ThemeBg = | "selectedBg" | "userMessageBg" diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index a334aa87c..7c85d691d 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -53,7 +53,7 @@ import { import { abortableSleep, getAgentDbPath, isEnoent, logger } from "@oh-my-pi/pi-utils"; import type { AsyncJob, AsyncJobManager } from "../async"; import type { Rule } from "../capability/rule"; -import { MODEL_ROLE_IDS, type ModelRegistry, type ModelRole } from "../config/model-registry"; +import { MODEL_ROLE_IDS, type ModelRegistry } from "../config/model-registry"; import { extractExplicitThinkingSelector, parseModelString, resolveModelRoleValue } from "../config/model-resolver"; import { expandPromptTemplate, type PromptTemplate, renderPromptTemplate } from "../config/prompt-templates"; import type { Settings, SkillsSettings } from "../config/settings"; @@ -2021,7 +2021,7 @@ export class AgentSession { ); } - resolveRoleModel(role: ModelRole): Model | undefined { + resolveRoleModel(role: string): Model | undefined { return this.#resolveRoleModel(role, this.#modelRegistry.getAvailable(), this.model); } @@ -3171,9 +3171,10 @@ export class AgentSession { if (roleModels.length <= 1) return undefined; const lastRole = this.sessionManager.getLastModelChangeRole(); - let currentIndex = lastRole - ? roleModels.findIndex(entry => entry.role === lastRole) - : roleModels.findIndex(entry => modelsAreEqual(entry.model, currentModel)); + let currentIndex = lastRole ? roleModels.findIndex(entry => entry.role === lastRole) : -1; + if (currentIndex === -1) { + currentIndex = roleModels.findIndex(entry => modelsAreEqual(entry.model, currentModel)); + } if (currentIndex === -1) currentIndex = 0; const nextIndex = (currentIndex + 1) % roleModels.length; @@ -4124,7 +4125,7 @@ export class AgentSession { return availableModels.find(m => m.provider === currentModel.provider && m.id === configuredTarget); } - #resolveRoleModel(role: ModelRole, availableModels: Model[], currentModel: Model | undefined): Model | undefined { + #resolveRoleModel(role: string, availableModels: Model[], currentModel: Model | undefined): Model | undefined { const roleModelStr = role === "default" ? (this.settings.getModelRole("default") ?? From bf9d58b8ac3d75075056937231270b72b3fd02ae Mon Sep 17 00:00:00 2001 From: Leo P Date: Thu, 19 Mar 2026 15:31:20 -0400 Subject: [PATCH 4/5] docs(coding-agent): update changelog for custom model tags and cycle order --- packages/coding-agent/CHANGELOG.md | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 43a9b92d2..580f69105 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,7 +2,6 @@ ## [Unreleased] -## [13.15.0] - 2026-03-23 ### Breaking Changes - Changed hashline edit schema from flat `op`/`pos`/`end`/`lines` fields to structured `loc`/`content` format with location-specific objects @@ -15,6 +14,8 @@ ### Added +- Added custom model roles/tags via config YAML +- Added ability to reorder model role/tag cycling via config YAML - Added prompt for tradeoff metrics during autoresearch setup to collect secondary metrics alongside primary metric - Added validation of contract path specifications to reject absolute paths and parent directory references - Added stricter benchmark command validation in `isAutoresearchShCommand()` to reject chained commands, pipes, and redirects @@ -71,19 +72,6 @@ ### Changed -- Changed `isAutoresearchShCommand()` to use proper command-line argument parsing instead of regex, improving accuracy for complex shell invocations -- Changed autoresearch initialization prompt to display collected tradeoff metrics in the setup summary -- Changed `command-initialize.md` template to include guidance on preflight requirements, comparability invariants, and marking measurement-critical files as off-limits -- Changed `command-initialize.md` to instruct users to write or update `autoresearch.program.md` with durable heuristics and repo-specific strategy -- Changed autoresearch resume guidance to emphasize continuing on the current protected branch rather than switching branches -- Changed autoresearch prompt to clarify that `autoresearch.md` holds durable conclusions while `autoresearch.ideas.md` is the scratch backlog -- Changed autoresearch prompt guidance to require stable measurement harness and fixed benchmark inputs unless intentionally starting a new segment -- Changed autoresearch prompt to recommend keeping equal or near-equal results when they materially simplify implementation -- Changed `init_experiment` to reset pending run state (checks, duration, ASI, artifact directory) when initializing a new segment -- Changed `log_experiment` to set `autoResumeArmed` flag after successfully logging a run to enable auto-resume on next agent turn -- Changed `run_experiment` to set `autoResumeArmed` flag and update dashboard after completing a run -- Changed auto-resume logic to only prompt when a new pending run exists or when `autoResumeArmed` is explicitly set, preventing duplicate prompts -- Changed path normalization in contract validation to use `path.posix.normalize()` for consistent path handling - Changed autoresearch initialization to collect and validate benchmark command, metric definition, scope paths, off-limits list, and constraints before `init_experiment` - Changed `init_experiment` to require exact benchmark command, metric definition, scope, off-limits, and constraints matching collected contract - Changed `log_experiment` to record run number, benchmark command, scope paths, off-limits list, constraints, and segment fingerprint with each result @@ -133,9 +121,6 @@ ### Fixed -- Fixed boundary duplication warnings to always display when replacement lines match the next surviving line, even when auto-correction is disabled -- Fixed secondary metrics validation to properly reject missing configured metrics and new metrics without force flag -- Fixed ASI data cloning to prevent prototype pollution attacks by filtering reserved property names - Fixed autoresearch resume to detect and recover pending run artifacts that were left unlogged from previous sessions - Fixed dashboard overlay to display when running experiment even with zero completed results - Fixed tab character rendering in dashboard command display and tool output summaries @@ -6333,4 +6318,4 @@ Initial public release. - Git branch display in footer - Message queueing during streaming responses - OAuth integration for Gmail and Google Calendar access -- HTML export with syntax highlighting and collapsible sections \ No newline at end of file +- HTML export with syntax highlighting and collapsible sections From 1f288d97335981d7b0ed176e16e22f23cb2210ad Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 26 Mar 2026 19:12:51 +0100 Subject: [PATCH 5/5] fix(models): unify selector role sources --- .../coding-agent/src/config/model-registry.ts | 53 ++++++++---- .../src/modes/components/model-selector.ts | 50 ++--------- ...model-selector-role-badge-thinking.test.ts | 82 ++++++++++++++----- packages/coding-agent/test/role-info.test.ts | 7 +- 4 files changed, 109 insertions(+), 83 deletions(-) diff --git a/packages/coding-agent/src/config/model-registry.ts b/packages/coding-agent/src/config/model-registry.ts index ddfb58168..817f460e0 100644 --- a/packages/coding-agent/src/config/model-registry.ts +++ b/packages/coding-agent/src/config/model-registry.ts @@ -61,30 +61,49 @@ export const MODEL_ROLE_IDS: ModelRole[] = ["default", "smol", "slow", "vision", /** Alias for ModelRoleInfo - used for both built-in and custom roles */ export type RoleInfo = ModelRoleInfo; +/** + * Return the canonical set of known roles for selector/carousel UI. + * + * Built-ins always come first. Configured cycle order, model assignments, and + * tag metadata can introduce additional custom roles without requiring duplicate + * entries across settings. + */ +export function getKnownRoleIds(settings: Settings): string[] { + // Avoid MODEL_ROLE_IDS here: this helper is reached during selector initialization, + // and model-registry participates in import cycles while the module is still evaluating. + const roles = ["default", "smol", "slow", "vision", "plan", "commit", "task"]; + const seen = new Set(roles); + const addRole = (role: string) => { + if (seen.has(role)) return; + seen.add(role); + roles.push(role); + }; + + for (const role of settings.get("cycleOrder")) addRole(role); + for (const role of Object.keys(settings.getModelRoles())) addRole(role); + for (const role of Object.keys(settings.get("modelTags"))) addRole(role); + + return roles; +} + /** * Get role info for a role name (built-in or custom). - * Returns built-in role info if the role is predefined, - * otherwise looks up custom role from settings, or returns a fallback. + * Configured metadata overrides built-in defaults when present. */ export function getRoleInfo(role: string, settings: Settings): RoleInfo { - // Check if it's a built-in role - if (role in MODEL_ROLES) { - return MODEL_ROLES[role as ModelRole]; + const builtIn = role in MODEL_ROLES ? MODEL_ROLES[role as ModelRole] : undefined; + const configured = settings.get("modelTags")[role]; + + if (configured) { + return { + tag: builtIn?.tag, + name: configured.name || builtIn?.name || role, + color: configured.color && isValidThemeColor(configured.color) ? configured.color : builtIn?.color, + }; } - // Check if it's a custom role in settings - const customTags = settings.get("modelTags"); - if (customTags && role in customTags) { - const tagDef = customTags[role]; - if (tagDef) { - return { - name: tagDef.name || role, - color: tagDef.color && isValidThemeColor(tagDef.color) ? tagDef.color : undefined, - }; - } - } + if (builtIn) return builtIn; - // Fallback for undefined roles return { name: role, color: "muted" }; } diff --git a/packages/coding-agent/src/modes/components/model-selector.ts b/packages/coding-agent/src/modes/components/model-selector.ts index 2c8d2db73..8fca03a3b 100644 --- a/packages/coding-agent/src/modes/components/model-selector.ts +++ b/packages/coding-agent/src/modes/components/model-selector.ts @@ -13,7 +13,7 @@ import { visibleWidth, } from "@oh-my-pi/pi-tui"; import type { ModelRegistry } from "../../config/model-registry"; -import { getRoleInfo, MODEL_ROLE_IDS, MODEL_ROLES } from "../../config/model-registry"; +import { getKnownRoleIds, getRoleInfo, MODEL_ROLE_IDS, MODEL_ROLES } from "../../config/model-registry"; import { resolveModelRoleValue } from "../../config/model-resolver"; import type { Settings } from "../../config/settings"; import { type ThemeColor, theme } from "../../modes/theme/theme"; @@ -182,54 +182,20 @@ export class ModelSelectorComponent extends Container { } #buildMenuRoleActions(): void { - const actions: MenuRoleAction[] = []; - - // Add built-in roles - for (const role of MODEL_ROLE_IDS) { - const roleInfo = MODEL_ROLES[role]; + this.#menuRoleActions = getKnownRoleIds(this.#settings).map(role => { + const roleInfo = getRoleInfo(role, this.#settings); const roleLabel = roleInfo.tag ? `${roleInfo.tag} (${roleInfo.name})` : roleInfo.name; - actions.push({ + return { label: `Set as ${roleLabel}`, role, - }); - } - - // Add custom roles from settings - for (const [role, tagDef] of Object.entries(this.#settings.get("modelTags"))) { - if (tagDef.name) { - actions.push({ - label: `Set as ${tagDef.name}`, - role, - }); - } - } - - this.#menuRoleActions = actions; + }; + }); } #loadRoleModels(): void { const allModels = this.#modelRegistry.getAll(); const matchPreferences = { usageOrder: this.#settings.getStorage()?.getModelUsageOrder() }; - for (const role of MODEL_ROLE_IDS) { - const roleValue = this.#settings.getModelRole(role); - if (!roleValue) continue; - - const { model, thinkingLevel, explicitThinkingLevel } = resolveModelRoleValue(roleValue, allModels, { - settings: this.#settings, - matchPreferences, - }); - if (model) { - this.#roles[role] = { - model, - thinkingLevel: - explicitThinkingLevel && thinkingLevel !== undefined ? thinkingLevel : ThinkingLevel.Inherit, - }; - } - } - - // Load custom roles from modelTags settings - for (const [role] of Object.entries(this.#settings.get("modelTags"))) { - if (role in MODEL_ROLES) continue; + for (const role of getKnownRoleIds(this.#settings)) { const roleValue = this.#settings.getModelRole(role); if (!roleValue) continue; @@ -514,7 +480,7 @@ export class ModelSelectorComponent extends Container { // Build role badges (inverted: color as background, black text) const roleBadgeTokens: string[] = []; for (const role of MODEL_ROLE_IDS) { - const { tag, color } = MODEL_ROLES[role]; + const { tag, color } = getRoleInfo(role, this.#settings); const assigned = this.#roles[role]; if (!tag || !assigned || !modelsAreEqual(assigned.model, item.model)) continue; diff --git a/packages/coding-agent/test/model-selector-role-badge-thinking.test.ts b/packages/coding-agent/test/model-selector-role-badge-thinking.test.ts index 124e0b7c1..6f3d69b37 100644 --- a/packages/coding-agent/test/model-selector-role-badge-thinking.test.ts +++ b/packages/coding-agent/test/model-selector-role-badge-thinking.test.ts @@ -1,9 +1,9 @@ import { beforeAll, describe, expect, test, vi } from "bun:test"; -import { getBundledModel } from "@oh-my-pi/pi-ai"; +import { getBundledModel, type Model } from "@oh-my-pi/pi-ai"; import type { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { ModelSelectorComponent } from "@oh-my-pi/pi-coding-agent/modes/components/model-selector"; -import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import { setThemeInstance } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import type { TUI } from "@oh-my-pi/pi-tui"; function normalizeRenderedText(text: string): string { @@ -17,9 +17,38 @@ function normalizeRenderedText(text: string): string { ); } +function createSelector(model: Model, settings: Settings): ModelSelectorComponent { + const modelRegistry = { + getAll: () => [model], + getDiscoverableProviders: () => [], + } as unknown as ModelRegistry; + const ui = { + requestRender: vi.fn(), + } as unknown as TUI; + + return new ModelSelectorComponent( + ui, + model, + settings, + modelRegistry, + [{ model, thinkingLevel: "off" }], + () => {}, + () => {}, + ); +} + describe("ModelSelector role badge thinking display", () => { beforeAll(() => { - initTheme(); + setThemeInstance( + { + fg: (_color: string, text: string) => text, + bg: (_color: string, text: string) => text, + bold: (text: string) => text, + getFgAnsi: () => "\x1b[38;5;1m", + nav: { cursor: ">" }, + boxSharp: { horizontal: "-" }, + } as never, + ); }); test("renders per-role thinking labels with inherit mode to avoid badge ambiguity", async () => { @@ -36,23 +65,7 @@ describe("ModelSelector role badge thinking display", () => { }, }); - const modelRegistry = { - getAll: () => [model], - getDiscoverableProviders: () => [], - } as unknown as ModelRegistry; - const ui = { - requestRender: vi.fn(), - } as unknown as TUI; - - const selector = new ModelSelectorComponent( - ui, - model, - settings, - modelRegistry, - [{ model, thinkingLevel: "off" }], - () => {}, - () => {}, - ); + const selector = createSelector(model, settings); await Bun.sleep(0); @@ -72,4 +85,33 @@ describe("ModelSelector role badge thinking display", () => { expect(menuRendered).toContain("Set as PLAN (Architect)"); expect(menuRendered).toContain("Set as COMMIT (Commit)"); }); + + test("shows custom roles from cycleOrder/modelRoles and honors built-in metadata overrides", async () => { + const model = getBundledModel("anthropic", "claude-sonnet-4-5"); + if (!model) throw new Error("Expected bundled model anthropic/claude-sonnet-4-5"); + + const settings = Settings.isolated({ + cycleOrder: ["smol", "custom-fast", "default"], + modelRoles: { + default: `${model.provider}/${model.id}`, + "custom-fast": `${model.provider}/${model.id}:low`, + smol: `${model.provider}/${model.id}`, + }, + modelTags: { + smol: { name: "Quick", color: "error" }, + }, + }); + + const selector = createSelector(model, settings); + await Bun.sleep(0); + + const rendered = normalizeRenderedText(selector.render(220).join("\n")); + expect(rendered).toContain("custom-fast (low)"); + expect(rendered).toContain("SMOL (inherit)"); + + selector.handleInput("\n"); + const menuRendered = normalizeRenderedText(selector.render(220).join("\n")); + expect(menuRendered).toContain("Set as custom-fast"); + expect(menuRendered).toContain("Set as SMOL (Quick)"); + }); }); diff --git a/packages/coding-agent/test/role-info.test.ts b/packages/coding-agent/test/role-info.test.ts index 2bfac776e..5f3a5d116 100644 --- a/packages/coding-agent/test/role-info.test.ts +++ b/packages/coding-agent/test/role-info.test.ts @@ -50,18 +50,17 @@ describe("getRoleInfo", () => { }); }); - test("custom role does not override built-in roles", () => { + test("configured metadata overrides built-in role info while keeping built-in defaults", () => { const settings = Settings.isolated({ modelTags: { smol: { name: "My Smol", color: "success" }, }, }); - // Built-in 'smol' always returns built-in info, ignoring custom tag expect(getRoleInfo("smol", settings)).toEqual({ - name: "Fast", - color: "warning", tag: "SMOL", + name: "My Smol", + color: "success", }); }); });