smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(coding-agent): refactored model role system and extracted model resolution utilities

Browse Source 
- Added 'commit' model role for dedicated commit message generation.
- Exported 'resolveModelOverride' function from model-resolver module for public use.
- Refactored model role system to centralize role configuration in MODEL_ROLES registry.
- Extracted model resolution utilities into dedicated model-resolver module for improved code organization.
- Made 'tag' and 'color' properties optional in ModelRoleInfo interface to support roles without visual indicators.
- Enhanced model selector with defensive null/undefined checks for optional role properties.
This commit is contained in:
can1357
2026-02-05 00:22:31 +01:00
parent d5790b5414
commit ba0b0f6d86
10 changed files with 58 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+10
View File
@@ -1,8 +1,18 @@
# Changelog
## [Unreleased]
### Added
- Added `commit` model role for dedicated commit message generation
- Exported `resolveModelOverride` function from model resolver for external use
### Changed
- Updated model role resolution to accept optional `roleOrder` parameter for custom role priority
- Made `tag` and `color` properties optional in `ModelRoleInfo` interface
- Updated model selector to safely handle roles without tag or color definitions
- Refactored role label display to use centralized `MODEL_ROLES` registry instead of hardcoded strings
- Refactored model role system to use centralized `MODEL_ROLES` registry with consistent tag, name, and color definitions
- Simplified model role resolution to use `MODEL_ROLE_IDS` array instead of hardcoded role checks
- Updated model selector to dynamically generate menu actions from `MODEL_ROLES` registry
packages/coding-agent/src/commit/model-selection.ts
+18 -39
View File
@@ -1,5 +1,12 @@
import type { Api, Model } from "@oh-my-pi/pi-ai";
import { parseModelPattern, parseModelString, SMOL_MODEL_PRIORITY } from "../config/model-resolver";
import { MODEL_ROLE_IDS } from "../config/model-registry";
import {
expandRoleAlias,
parseModelPattern,
resolveModelFromSettings,
resolveModelFromString,
SMOL_MODEL_PRIORITY,
} from "../config/model-resolver";
import type { Settings } from "../config/settings";
export async function resolvePrimaryModel(
@@ -12,9 +19,15 @@ export async function resolvePrimaryModel(
): Promise<{ model: Model<Api>; apiKey: string }> {
const available = modelRegistry.getAvailable();
const matchPreferences = { usageOrder: settings.getStorage()?.getModelUsageOrder() };
const roleOrder = ["commit", "smol", ...MODEL_ROLE_IDS] as const;
const model = override
? resolveModelFromString(expandRoleAlias(override, settings), available, matchPreferences)
: resolveModelFromSettings(settings, available, matchPreferences);
: resolveModelFromSettings({
settings,
availableModels: available,
matchPreferences,
roleOrder,
});
if (!model) {
throw new Error("No model available for commit generation");
}
@@ -37,7 +50,9 @@ export async function resolveSmolModel(
const available = modelRegistry.getAvailable();
const matchPreferences = { usageOrder: settings.getStorage()?.getModelUsageOrder() };
const role = settings.getModelRole("smol");
const roleModel = role ? resolveModelFromString(role, available, matchPreferences) : undefined;
const roleModel = role
? resolveModelFromString(expandRoleAlias(role, settings), available, matchPreferences)
: undefined;
if (roleModel) {
const apiKey = await modelRegistry.getApiKey(roleModel);
if (apiKey) return { model: roleModel, apiKey };
@@ -52,39 +67,3 @@ export async function resolveSmolModel(
return { model: fallbackModel, apiKey: fallbackApiKey };
}
function resolveModelFromSettings(
settings: Settings,
available: Model<Api>[],
matchPreferences: { usageOrder?: string[] },
): Model<Api> | undefined {
const roles = ["commit", "smol", "default"];
for (const role of roles) {
const configured = settings.getModelRole(role);
if (!configured) continue;
const resolved = resolveModelFromString(expandRoleAlias(configured, settings), available, matchPreferences);
if (resolved) return resolved;
}
return available[0];
}
function resolveModelFromString(
value: string,
available: Model<Api>[],
matchPreferences: { usageOrder?: string[] },
): Model<Api> | undefined {
const parsed = parseModelString(value);
if (parsed) {
return available.find(model => model.provider === parsed.provider && model.id === parsed.id);
}
return parseModelPattern(value, available, matchPreferences).model;
}
function expandRoleAlias(value: string, settings: Settings): string {
const lower = value.toLowerCase();
if (lower.startsWith("pi/")) {
const role = value.slice(3);
return settings.getModelRole(role) ?? value;
}
return value;
}
packages/coding-agent/src/config/model-registry.ts
+5 -4
View File
@@ -18,12 +18,12 @@ import { YAML } from "bun";
import type { ThemeColor } from "../modes/theme/theme";
import type { AuthStorage } from "../session/auth-storage";
export type ModelRole = "default" | "smol" | "slow" | "plan";
export type ModelRole = "default" | "smol" | "slow" | "plan" | "commit";
export interface ModelRoleInfo {
tag: string;
tag?: string;
name: string;
color: ThemeColor;
color?: ThemeColor;
}
export const MODEL_ROLES: Record<ModelRole, ModelRoleInfo> = {
@@ -31,9 +31,10 @@ export const MODEL_ROLES: Record<ModelRole, ModelRoleInfo> = {
smol: { tag: "SMOL", name: "Fast", color: "warning" },
slow: { tag: "SLOW", name: "Thinking", color: "accent" },
plan: { tag: "PLAN", name: "Architect", color: "muted" },
commit: { name: "Commit" },
};
export const MODEL_ROLE_IDS: ModelRole[] = ["default", "smol", "slow", "plan"];
export const MODEL_ROLE_IDS: ModelRole[] = ["default", "smol", "slow", "plan", "commit"];
const Ajv = (AjvModule as any).default || AjvModule;
packages/coding-agent/src/config/model-resolver.ts
+15 -13
View File
@@ -6,7 +6,7 @@ import { type Api, type KnownProvider, type Model, modelsAreEqual } from "@oh-my
import chalk from "chalk";
import { isValidThinkingLevel } from "../cli/args";
import { fuzzyMatch } from "../utils/fuzzy";
import { MODEL_ROLE_IDS, type ModelRegistry } from "./model-registry";
import { MODEL_ROLE_IDS, type ModelRegistry, type ModelRole } from "./model-registry";
import type { Settings } from "./settings";
/** Default model IDs for each known provider */
@@ -311,28 +311,30 @@ export function parseModelPattern(
return parseModelPatternWithContext(pattern, availableModels, context);
}
const MODEL_ROLE_ALIAS_PREFIX = "pi/";
const PREFIX_MODEL_ROLE = "pi/";
const DEFAULT_MODEL_ROLE = "default";
const DEFAULT_MODEL_ALIASES = new Set([DEFAULT_MODEL_ROLE, `${MODEL_ROLE_ALIAS_PREFIX}${DEFAULT_MODEL_ROLE}`]);
/**
* Check if a model override value is effectively the default role.
*/
export function isDefaultModelAlias(value: string | string[] | undefined): boolean {
if (!value) return true;
const values = Array.isArray(value) ? value : [value];
if (values.length === 0) return true;
return values.every(entry => DEFAULT_MODEL_ALIASES.has(entry.trim().toLowerCase()));
if (Array.isArray(value)) return value.every(entry => isDefaultModelAlias(entry));
if (value.startsWith(PREFIX_MODEL_ROLE)) {
value = value.slice(PREFIX_MODEL_ROLE.length);
}
return value === DEFAULT_MODEL_ROLE;
}
/**
* Expand a role alias like "pi/smol" to the configured model string.
*/
export function expandRoleAlias(value: string, settings?: Settings): string {
const normalized = value.trim().toLowerCase();
if (!normalized.startsWith(MODEL_ROLE_ALIAS_PREFIX)) return value;
const role = normalized.slice(MODEL_ROLE_ALIAS_PREFIX.length);
if (!MODEL_ROLE_IDS.includes(role as (typeof MODEL_ROLE_IDS)[number])) return value;
const normalized = value.trim();
if (normalized === "default") return settings?.getModelRole("default") ?? value;
if (!normalized.startsWith(PREFIX_MODEL_ROLE)) return value;
const role = normalized.slice(PREFIX_MODEL_ROLE.length) as ModelRole;
if (!MODEL_ROLE_IDS.includes(role)) return value;
return settings?.getModelRole(role) ?? value;
}
@@ -358,7 +360,7 @@ export function resolveModelFromSettings(options: {
settings: Settings;
availableModels: Model<Api>[];
matchPreferences?: ModelMatchPreferences;
roleOrder?: string[];
roleOrder?: readonly ModelRole[];
}): Model<Api> | undefined {
const { settings, availableModels, matchPreferences, roleOrder } = options;
const roles = roleOrder ?? MODEL_ROLE_IDS;
@@ -382,8 +384,8 @@ export function resolveModelOverride(
if (modelPatterns.length === 0) return {};
const matchPreferences = { usageOrder: settings?.getStorage()?.getModelUsageOrder() };
for (const pattern of modelPatterns) {
const normalized = pattern.trim().toLowerCase();
if (!normalized || DEFAULT_MODEL_ALIASES.has(normalized)) {
const normalized = pattern.trim();
if (!normalized || isDefaultModelAlias(normalized)) {
continue;
}
const effectivePattern = expandRoleAlias(pattern, settings);
packages/coding-agent/src/modes/components/model-selector.ts
+2 -2
View File
@@ -356,8 +356,8 @@ export class ModelSelectorComponent extends Container {
const badges: string[] = [];
for (const role of MODEL_ROLE_IDS) {
const { tag, color } = MODEL_ROLES[role];
if (modelsAreEqual(this.roles[role], item.model)) {
badges.push(makeInvertedBadge(tag, color));
if (tag && modelsAreEqual(this.roles[role], item.model)) {
badges.push(makeInvertedBadge(tag, color ?? "success"));
}
}
const badgeText = badges.length > 0 ? ` ${badges.join(" ")}` : "";
packages/coding-agent/src/modes/controllers/input-controller.ts
+1 -1
View File
@@ -649,7 +649,7 @@ export class InputController {
async cycleRoleModel(options?: { temporary?: boolean }): Promise<void> {
try {
const roleOrder = ["slow", "default", "smol"];
const roleOrder = ["slow", "default", "smol"] as const;
const result = await this.ctx.session.cycleRoleModels(roleOrder, options);
if (!result) {
this.ctx.showStatus("Only one role model available");
packages/coding-agent/src/modes/controllers/selector-controller.ts
+4 -2
View File
@@ -3,6 +3,7 @@ import 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 } from "../../config";
import { MODEL_ROLES } from "../../config/model-registry";
import { settings } from "../../config/settings";
import { DebugSelectorComponent } from "../../debug";
import { disableProvider, enableProvider } from "../../discovery";
@@ -277,7 +278,7 @@ export class SelectorController {
this.ctx.session.scopedModels,
async (model, role) => {
try {
if (role === "temporary") {
if (role === null) {
// Temporary: update agent state but don't persist to settings
await this.ctx.session.setModelTemporary(model);
this.ctx.statusLine.invalidate();
@@ -294,7 +295,8 @@ 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 roleLabel = role === "smol" ? "Smol" : role;
const roleInfo = MODEL_ROLES[role];
const roleLabel = roleInfo?.name ?? role;
this.ctx.showStatus(`${roleLabel} model: ${model.id}`);
// Don't call done() - selector stays open
}
packages/coding-agent/src/session/agent-session.ts
+1 -1
View File
@@ -1854,7 +1854,7 @@ export class AgentSession {
* @param options - Optional settings: `temporary` to not persist to settings
*/
async cycleRoleModels(
roleOrder: ModelRole[],
roleOrder: readonly ModelRole[],
options?: { temporary?: boolean },
): Promise<RoleModelCycleResult | undefined> {
const availableModels = this._modelRegistry.getAvailable();
packages/coding-agent/src/task/executor.ts
+1 -33
View File
@@ -10,7 +10,7 @@ import { logger, untilAborted } from "@oh-my-pi/pi-utils";
import type { TSchema } from "@sinclair/typebox";
import Ajv, { type ValidateFunction } from "ajv";
import type { ModelRegistry } from "../config/model-registry";
import { parseModelPattern } from "../config/model-resolver";
import { resolveModelOverride } from "../config/model-resolver";
import { type PromptTemplate, renderPromptTemplate } from "../config/prompt-templates";
import { Settings } from "../config/settings";
import type { CustomTool } from "../extensibility/custom-tools/types";
@@ -128,38 +128,6 @@ function getReportFindingKey(value: unknown): string | null {
return `${filePath}:${lineStart}:${lineEnd}:${priority ?? ""}:${title}`;
}
function resolveModelOverride(
modelPatterns: string[],
modelRegistry: ModelRegistry,
settings?: Settings,
): { model?: Model<Api>; thinkingLevel?: ThinkingLevel } {
if (modelPatterns.length === 0) return {};
const matchPreferences = { usageOrder: settings?.getStorage()?.getModelUsageOrder() };
for (const pattern of modelPatterns) {
const normalized = pattern.trim().toLowerCase();
if (!normalized || normalized === "default" || normalized === "pi/default") {
continue;
}
let effectivePattern = pattern;
if (normalized.startsWith("pi/")) {
const role = pattern.slice(3);
const configured = settings?.getModelRole(role);
if (configured) {
effectivePattern = configured;
}
}
const { model, thinkingLevel } = parseModelPattern(
effectivePattern,
modelRegistry.getAvailable(),
matchPreferences,
);
if (model) {
return { model, thinkingLevel: thinkingLevel !== "off" ? thinkingLevel : undefined };
}
}
return {};
}
function buildSubmitResultToolChoice(model?: Model<Api>): ToolChoice | undefined {
if (!model) return undefined;
if (
packages/coding-agent/src/task/index.ts
+1 -10
View File
@@ -20,6 +20,7 @@ import type { Usage } from "@oh-my-pi/pi-ai";
import { $ } from "bun";
import { nanoid } from "nanoid";
import type { ToolSession } from "..";
import { isDefaultModelAlias } from "../config/model-resolver";
import { renderPromptTemplate } from "../config/prompt-templates";
import type { Theme } from "../modes/theme/theme";
import planModeSubagentPrompt from "../prompts/system/plan-mode-subagent.md" with { type: "text" };
@@ -165,16 +166,6 @@ export class TaskTool implements AgentTool<typeof taskSchema, TaskToolDetails, T
const { agent: agentName, context, schema: outputSchema, isolated } = params;
const isIsolated = isolated === true;
const isDefaultModelAlias = (value: string | string[] | undefined): boolean => {
if (!value) return true;
const values = Array.isArray(value) ? value : [value];
if (values.length === 0) return true;
return values.every(entry => {
const normalized = entry.trim().toLowerCase();
return normalized === "default" || normalized === "pi/default";
});
};
// Validate agent exists
const agent = getAgent(agents, agentName);
if (!agent) {