Merge PR #5972: fix: resolve configured model roles in --model (@paralin)

This commit is contained in:
can1357
2026-07-18 19:42:51 +02:00
10 changed files with 502 additions and 24 deletions
+3
View File
@@ -1,6 +1,9 @@
# Changelog
## [Unreleased]
### Fixed
- Fixed `--model <role>` resolving a bare configured `modelRoles` key.
### Fixed
+8 -2
View File
@@ -443,7 +443,7 @@ function resolveBenchModels(
const resolved: BenchTarget[] = [];
const errors: string[] = [];
for (const selector of selectors) {
const result = resolveCliModel({ cliModel: selector, modelRegistry, preferences });
const result = resolveCliModel({ cliModel: selector, modelRegistry, settings, preferences });
if (result.error) {
errors.push(`${selector}: ${result.error}`);
continue;
@@ -454,7 +454,13 @@ function resolveBenchModels(
}
if (result.warning) writeStderr(`${chalk.yellow(`Warning: ${result.warning}`)}\n`);
let model = result.model;
const authenticated = resolveAuthenticatedAlternative(selector, model, modelRegistry, preferences.providerOrder);
const authSelector = result.configuredPatterns?.[result.configuredPatternIndex ?? 0] ?? selector;
const authenticated = resolveAuthenticatedAlternative(
authSelector,
model,
modelRegistry,
preferences.providerOrder,
);
if (authenticated) {
writeStderr(
`${chalk.yellow(
@@ -555,6 +555,7 @@ async function resolveDryBalanceModel(
const resolved = resolveCliModel({
cliModel: modelSelector,
modelRegistry,
settings,
preferences,
});
if (resolved.error) throw new Error(resolved.error);
@@ -1138,6 +1138,8 @@ export function resolveAgentPrewalkPattern(options: AgentPrewalkResolutionOption
export interface ResolvedModelRoleValue {
model: Model<Api> | undefined;
thinkingLevel?: ConfiguredThinkingLevel;
/** matchedPatternIndex identifies the first configured pattern that matched an available model. */
matchedPatternIndex?: number;
explicitThinkingLevel: boolean;
warning: string | undefined;
}
@@ -1167,11 +1169,12 @@ export function resolveModelRoleValue(
// models) once and reuse it across every fallback pattern instead of
// rebuilding it per pattern inside parseModelPattern.
const preferenceContext = buildPreferenceContext(availableModels, matchPreferences);
for (const effectivePattern of effectivePatterns) {
for (const [patternIndex, effectivePattern] of effectivePatterns.entries()) {
const resolved = matchPatternWithContext(effectivePattern, availableModels, preferenceContext);
if (resolved.model) {
return {
model: resolved.model,
matchedPatternIndex: patternIndex,
thinkingLevel: resolved.explicitThinkingLevel
? resolved.thinkingLevel === AUTO_THINKING
? AUTO_THINKING
@@ -1598,6 +1601,10 @@ export function filterAvailableModelsByEnabledPatterns(
export interface ResolveCliModelResult {
model: Model<Api> | undefined;
/** configuredPatterns is the full configured fallback chain when the selector resolves through a role. */
configuredPatterns?: string[];
/** configuredPatternIndex identifies the configured role pattern that matched an available model. */
configuredPatternIndex?: number;
selector?: string;
thinkingLevel?: ConfiguredThinkingLevel;
warning: string | undefined;
@@ -1606,6 +1613,8 @@ export interface ResolveCliModelResult {
/**
* Resolve a single model from CLI flags.
*
* Exact model names take precedence over configured role names.
*/
export function resolveCliModel(options: {
cliProvider?: string;
@@ -1630,19 +1639,6 @@ export function resolveCliModel(options: {
};
}
if (!cliProvider && modelRoleAliasPrefixLength(cliModel) !== undefined) {
const resolved = resolveModelRoleValue(cliModel, availableModels, { settings, matchPreferences: preferences });
if (resolved.model) {
return {
model: resolved.model,
selector: formatModelString(resolved.model),
thinkingLevel: resolved.thinkingLevel,
warning: resolved.warning,
error: undefined,
};
}
}
const providerMap = new Map<string, string>();
for (const model of availableModels) {
providerMap.set(model.provider.toLowerCase(), model.provider);
@@ -1682,6 +1678,73 @@ export function resolveCliModel(options: {
error: undefined,
};
}
const { base: exactBase, level: exactThinkingLevel } = splitThinkingSuffix(
trimmedModel,
-1,
MAX_THINKING_SUFFIX_OPTIONS,
);
if (exactThinkingLevel) {
let exactSuffixed = findExactModelReferenceMatch(exactBase, availableModels);
if (!exactSuffixed) {
const lowerExactBase = exactBase.toLowerCase();
exactSuffixed = availableModels.find(
model =>
model.id.toLowerCase() === lowerExactBase ||
`${model.provider}/${model.id}`.toLowerCase() === lowerExactBase,
);
}
if (exactSuffixed) {
return {
model: exactSuffixed,
selector: formatModelString(exactSuffixed),
warning: undefined,
thinkingLevel: exactThinkingLevel,
error: undefined,
};
}
}
}
let configuredPatterns: string[] | undefined;
if (!cliProvider) {
const { base: bareRoleName, level: bareRoleThinkingLevel } = splitThinkingSuffix(
trimmedModel,
-1,
MAX_THINKING_SUFFIX_OPTIONS,
);
const roleSelector =
modelRoleAliasPrefixLength(trimmedModel) !== undefined
? trimmedModel
: settings?.getModelRole(bareRoleName) !== undefined
? `${formatModelRoleAlias(bareRoleName)}${bareRoleThinkingLevel ? `:${bareRoleThinkingLevel}` : ""}`
: undefined;
if (roleSelector) {
configuredPatterns = resolveConfiguredModelPatterns([roleSelector], settings);
const resolved = resolveModelRoleValue(roleSelector, availableModels, {
settings,
matchPreferences: preferences,
});
if (resolved.model) {
return {
model: resolved.model,
selector: formatModelString(resolved.model),
configuredPatterns,
configuredPatternIndex: resolved.matchedPatternIndex,
thinkingLevel: resolved.thinkingLevel,
warning: resolved.warning,
error: undefined,
};
}
if (configuredPatterns && configuredPatterns.length > 0) {
return {
model: undefined,
configuredPatterns,
selector: undefined,
thinkingLevel: undefined,
warning: resolved.warning,
error: `Model "${trimmedModel}" not found. Run "omp models" to see available models.`,
};
}
}
}
let pattern = trimmedModel;
@@ -1725,6 +1788,7 @@ export function resolveCliModel(options: {
const display = provider ? `${provider}/${pattern}` : cliModel;
return {
model: undefined,
configuredPatterns,
selector: undefined,
thinkingLevel: undefined,
warning,
+6 -2
View File
@@ -884,8 +884,12 @@ export async function buildSessionOptions(
if (resolved.warning) {
process.stderr.write(`${chalk.yellow(`Warning: ${resolved.warning}`)}\n`);
}
if (resolved.error) {
if (!parsed.provider && !parsed.model.includes(":")) {
const matchedAfterMissingRolePattern = (resolved.configuredPatternIndex ?? 0) > 0;
if (matchedAfterMissingRolePattern) {
// Extensions may register an earlier configured role candidate.
options.modelPattern = parsed.model;
} else if (resolved.error) {
if (!parsed.provider && ((resolved.configuredPatterns?.length ?? 0) > 0 || !parsed.model.includes(":"))) {
// Model not found in built-in registry — defer resolution to after extensions load
// (extensions may register additional providers/models via registerProvider)
options.modelPattern = parsed.model;
+27 -4
View File
@@ -50,6 +50,7 @@ import {
parseModelString,
pickDefaultAvailableModel,
resolveAllowedModels,
resolveCliModel,
resolveConfiguredModelPatterns,
resolveModelRoleValue,
} from "./config/model-resolver";
@@ -2066,13 +2067,35 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
}
}
// Resolve deferred --model/subagent patterns now that extension models are
// registered. Expand role aliases (`@smol`) and comma chains to concrete
// selectors first so deferred resolution accepts everything the immediate
// path (resolveModelOverride → resolveModelRoleValue) accepts.
// registered. Use the same CLI resolver as the immediate path so bare role
// names, exact model names, and provider selectors keep one precedence rule.
if (!model && deferredModelPatterns.length > 0) {
const expandedModelPatterns = resolveConfiguredModelPatterns(deferredModelPatterns, settings);
const availableModels = modelRegistry.getAll();
const matchPreferences = getModelMatchPreferences(settings);
const expandedModelPatterns = deferredModelPatterns.flatMap(pattern =>
pattern.split(",").flatMap(selector => {
const trimmedSelector = selector.trim();
if (!trimmedSelector) return [];
const resolved = resolveCliModel({
cliModel: trimmedSelector,
modelRegistry,
settings,
preferences: matchPreferences,
});
if (resolved.configuredPatterns && resolved.configuredPatterns.length > 0) {
return resolved.configuredPatterns;
}
if (resolved.model) {
return [
formatModelSelectorValue(
resolved.selector ?? formatModelStringWithRouting(resolved.model),
resolved.thinkingLevel,
),
];
}
return resolveConfiguredModelPatterns([trimmedSelector], settings);
}),
);
for (let patternIndex = 0; patternIndex < expandedModelPatterns.length; patternIndex += 1) {
const pattern = expandedModelPatterns[patternIndex];
const primary = parseModelPattern(pattern, availableModels, matchPreferences);
@@ -9,7 +9,7 @@ import type {
SimpleStreamOptions,
} from "@oh-my-pi/pi-ai";
import { type BenchModelRegistry, type BenchSummary, runBenchCommand } from "@oh-my-pi/pi-coding-agent/cli/bench-cli";
import type { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
function fakeModel(provider: string, id: string): Model<Api> {
return {
@@ -75,12 +75,13 @@ async function runBench(
selector: string,
registry: BenchModelRegistry,
streamFactory: () => AssistantMessageEventStream = fakeStream,
settings?: Settings,
) {
const stderr: string[] = [];
const summary = await runBenchCommand(
{ models: [selector], flags: { runs: 1, maxTokens: 64, json: false } },
{
createRuntime: async () => ({ modelRegistry: registry, settings: undefined, close: () => {} }),
createRuntime: async () => ({ modelRegistry: registry, settings, close: () => {} }),
randomSessionId: () => "sess-1",
writeStdout: () => {},
writeStderr: text => stderr.push(text),
@@ -141,6 +142,36 @@ describe("bench credential-aware provider selection", () => {
});
});
describe("bench configured role selection", () => {
it("resolves configured bare role names", async () => {
const model = fakeModel("acme", "bench-model");
const registry = fakeRegistry({ models: [model], authedProviders: ["acme"] });
const settings = Settings.isolated({ modelRoles: { task: "acme/bench-model" } });
const { summary } = await runBench("task", registry, fakeStream, settings);
expect(summary.models[0].model).toBe("acme/bench-model");
expect(summary.failures).toBe(0);
});
it("honors provider-pinned configured role targets", async () => {
const registry = fakeRegistry({
models: [fakeModel("groq", "openai/gpt-oss-20b"), fakeModel("openrouter", "openai/gpt-oss-20b")],
authedProviders: ["openrouter"],
});
const settings = Settings.isolated({
modelRoles: { task: "groq/openai/gpt-oss-20b" },
});
const { summary, stderr } = await runBench("task", registry, fakeStream, settings);
expect(summary.models[0].model).toBe("groq/openai/gpt-oss-20b");
expect(summary.failures).toBe(1);
expect(summary.models[0].results[0]).toMatchObject({ ok: false });
expect(stderr).not.toContain("benchmarking");
});
});
describe("bench empty-output guard", () => {
it("reports a run with no streamed content and no tokens as a failure", async () => {
const registry = fakeRegistry({ models: [fakeModel("acme", "model-x")], authedProviders: ["acme"] });
@@ -0,0 +1,46 @@
import { expect, test } from "bun:test";
import type { Api, Model, OAuthAccess } from "@oh-my-pi/pi-ai";
import { type DryBalanceModelRegistry, runDryBalanceCommand } from "@oh-my-pi/pi-coding-agent/cli/dry-balance-cli";
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
function fakeModel(provider: string, id: string): Model<Api> {
return {
provider,
id,
name: id,
api: "openai-completions",
baseUrl: "https://example.com/v1",
maxTokens: 4096,
contextWindow: 128_000,
} as unknown as Model<Api>;
}
test("dry-balance resolves configured bare role names", async () => {
const model = fakeModel("acme", "balance-model");
const registry: DryBalanceModelRegistry = {
authStorage: {
getOAuthAccess: async () =>
({ accessToken: "test-token", email: "test@example.com" }) as unknown as OAuthAccess,
},
getAll: () => [model],
getAvailable: () => [model],
getApiKey: async () => "test-token",
};
const settings = Settings.isolated({ modelRoles: { task: "acme/balance-model" } });
const summary = await runDryBalanceCommand(
{
flags: { model: "task", count: 1, concurrency: 1, json: true },
},
{
createRuntime: async () => ({ modelRegistry: registry, settings }),
randomSessionId: () => "session-1",
writeStdout: () => {},
writeStderr: () => {},
setExitCode: () => {},
},
);
expect(summary.model).toBe("acme/balance-model");
expect(summary.success.total).toBe(1);
});
@@ -1006,6 +1006,157 @@ describe("resolveCliModel", () => {
expect(result.model?.id).toBe("gpt-4o");
});
test("resolves bare configured role names from --model", () => {
const registry = {
getAll: () => allModels,
};
const settings = Settings.isolated({
modelRoles: { task: "openai/gpt-4o" },
});
const result = resolveCliModel({
cliModel: "task",
modelRegistry: registry,
settings,
});
expect(result.error).toBeUndefined();
expect(result.model?.provider).toBe("openai");
expect(result.model?.id).toBe("gpt-4o");
});
test("resolves bare configured role names with thinking suffixes", () => {
const registry = {
getAll: () => allModels,
};
const settings = Settings.isolated({
modelRoles: { task: "anthropic/claude-sonnet-4-5" },
});
const result = resolveCliModel({
cliModel: "task:high",
modelRegistry: registry,
settings,
});
expect(result.error).toBeUndefined();
expect(result.model?.id).toBe("claude-sonnet-4-5");
expect(result.thinkingLevel).toBe(Effort.High);
expect(result.configuredPatterns).toEqual(["anthropic/claude-sonnet-4-5:high"]);
});
test("preserves configured role fallback selectors for deferred resolution", () => {
const registry = {
getAll: () => allModels,
};
const settings = Settings.isolated({
modelRoles: {
task: "openrouter/z-ai/glm-4.7@cerebras,anthropic/claude-sonnet-4-5",
},
});
const result = resolveCliModel({
cliModel: "task",
modelRegistry: registry,
settings,
});
expect(result.configuredPatterns).toEqual(["openrouter/z-ai/glm-4.7@cerebras", "anthropic/claude-sonnet-4-5"]);
});
test("reports when a configured role matches after unresolved candidates", () => {
const registry = {
getAll: () => allModels,
};
const settings = Settings.isolated({
modelRoles: {
task: "runtime-provider/runtime-model,anthropic/claude-sonnet-4-5",
},
});
const result = resolveCliModel({
cliModel: "task",
modelRegistry: registry,
settings,
});
expect(result.model?.provider).toBe("anthropic");
expect(result.configuredPatternIndex).toBe(1);
expect(result.configuredPatterns).toEqual(["runtime-provider/runtime-model", "anthropic/claude-sonnet-4-5"]);
});
test("does not fuzzy-match unresolved configured roles", () => {
const registry = {
getAll: () => allModels,
};
const settings = Settings.isolated({
modelRoles: { sonnet: "runtime-provider/runtime-model" },
});
const result = resolveCliModel({
cliModel: "sonnet",
modelRegistry: registry,
settings,
});
expect(result.model).toBeUndefined();
expect(result.configuredPatterns).toEqual(["runtime-provider/runtime-model"]);
expect(result.error).toContain('Model "sonnet" not found');
});
test("keeps unknown --model names on the not-found path", () => {
const registry = {
getAll: () => allModels,
};
const result = resolveCliModel({
cliModel: "not-a-model",
modelRegistry: registry,
});
expect(result.model).toBeUndefined();
expect(result.error).toContain('Model "not-a-model" not found');
});
test("prefers an exact model name over a same-named configured role", () => {
const exactModel = buildModel({
id: "task",
name: "Task",
api: "anthropic-messages",
provider: "openai",
baseUrl: "https://api.openai.com",
reasoning: false,
input: ["text"],
cost: { input: 5, output: 15, cacheRead: 0.5, cacheWrite: 5 },
contextWindow: 128000,
maxTokens: 4096,
});
const registry = {
getAll: () => [...allModels, exactModel],
};
const settings = Settings.isolated({
modelRoles: { task: "anthropic/claude-sonnet-4-5" },
});
const result = resolveCliModel({
cliModel: "task",
modelRegistry: registry,
settings,
});
expect(result.error).toBeUndefined();
expect(result.model).toBe(exactModel);
const suffixed = resolveCliModel({
cliModel: "task:high",
modelRegistry: registry,
settings,
});
expect(suffixed.error).toBeUndefined();
expect(suffixed.model).toBe(exactModel);
expect(suffixed.thinkingLevel).toBe(Effort.High);
});
test("resolves configured custom, legacy, and default role aliases from --model", () => {
const registry = {
getAll: () => allModels,
@@ -6,8 +6,10 @@ import { Effort, type FetchImpl } from "@oh-my-pi/pi-ai";
import { buildModel } from "@oh-my-pi/pi-catalog/build";
import { writeModelCache } from "@oh-my-pi/pi-catalog/model-cache";
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
import { parseArgs } from "@oh-my-pi/pi-coding-agent/cli/args";
import { ModelRegistry, type ProviderConfigInput } from "@oh-my-pi/pi-coding-agent/config/model-registry";
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
import { buildSessionOptions as buildCliSessionOptions } from "@oh-my-pi/pi-coding-agent/main";
import { createAgentSession, type ExtensionFactory } from "@oh-my-pi/pi-coding-agent/sdk";
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
@@ -226,6 +228,153 @@ describe("createAgentSession deferred model pattern resolution", () => {
}
});
test("resolves deferred bare configured role names after extension providers register", async () => {
const settings = Settings.isolated();
settings.setModelRole("task", "runtime-provider/runtime-model");
const { session, modelFallbackMessage } = await createAgentSession({
...(await buildSessionOptions("task")),
settings,
});
try {
expect(session.model?.provider).toBe("runtime-provider");
expect(session.model?.id).toBe("runtime-model");
expect(modelFallbackMessage).toBeUndefined();
} finally {
await session.dispose();
}
});
test("resolves deferred suffixed bare configured roles after extension providers register", async () => {
const settings = Settings.isolated();
settings.setModelRole("task", "runtime-provider/runtime-reasoning-model");
const authStorage = await AuthStorage.create(path.join(tempDir, "cli-auth.db"));
authStoragesToClose.push(authStorage);
const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir, "cli-models.yml"));
const parsed = parseArgs(["--model", "task:high"]);
const exitSpy = vi.spyOn(process, "exit").mockImplementation((code?: number | string | null) => {
throw new Error(`buildSessionOptions unexpectedly exited with ${code}`);
});
try {
const cliOptions = await buildCliSessionOptions(
parsed,
[],
SessionManager.inMemory(),
modelRegistry,
settings,
);
expect(cliOptions.modelPattern).toBe("task:high");
const { session, modelFallbackMessage } = await createAgentSession({
...cliOptions,
cwd: tempDir,
agentDir: tempDir,
authStorage,
modelRegistry,
settings,
disableExtensionDiscovery: true,
extensions: [providerExtension],
skills: [],
contextFiles: [],
promptTemplates: [],
slashCommands: [],
enableMCP: false,
enableLsp: false,
skipPythonPreflight: true,
});
try {
expect(session.model?.provider).toBe("runtime-provider");
expect(session.model?.id).toBe("runtime-reasoning-model");
expect(session.thinkingLevel).toBe(Effort.High);
expect(modelFallbackMessage).toBeUndefined();
} finally {
await session.dispose();
}
} finally {
exitSpy.mockRestore();
}
});
test("defers bare role chains when an earlier candidate may be registered by extensions", async () => {
const fallbackModel = getBundledModel("anthropic", "claude-sonnet-4-5");
if (!fallbackModel) {
throw new Error("Expected bundled anthropic fallback model");
}
const settings = Settings.isolated();
settings.setModelRole("task", `runtime-provider/runtime-model,${fallbackModel.provider}/${fallbackModel.id}`);
const authStorage = await AuthStorage.create(path.join(tempDir, "role-chain-auth.db"));
authStoragesToClose.push(authStorage);
const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir, "role-chain-models.yml"));
const parsed = parseArgs(["--model", "task"]);
const exitSpy = vi.spyOn(process, "exit").mockImplementation((code?: number | string | null) => {
throw new Error(`buildSessionOptions unexpectedly exited with ${code}`);
});
try {
const cliOptions = await buildCliSessionOptions(
parsed,
[],
SessionManager.inMemory(),
modelRegistry,
settings,
);
expect(cliOptions.model).toBeUndefined();
expect(cliOptions.modelPattern).toBe("task");
const { session, modelFallbackMessage } = await createAgentSession({
...cliOptions,
cwd: tempDir,
agentDir: tempDir,
authStorage,
modelRegistry,
settings,
disableExtensionDiscovery: true,
extensions: [providerExtension],
skills: [],
contextFiles: [],
promptTemplates: [],
slashCommands: [],
enableMCP: false,
enableLsp: false,
skipPythonPreflight: true,
});
try {
expect(session.model?.provider).toBe("runtime-provider");
expect(session.model?.id).toBe("runtime-model");
expect(modelFallbackMessage).toBeUndefined();
} finally {
await session.dispose();
}
} finally {
exitSpy.mockRestore();
}
});
test("preserves deferred bare role fallback chains", async () => {
const settings = Settings.isolated();
settings.setModelRole("task", "runtime-provider/runtime-model,runtime-provider/runtime-reasoning-model");
const { session, modelFallbackMessage } = await createAgentSession({
...(await buildSessionOptions("task")),
modelPatternFallbackRole: "subagent:deferred",
settings,
});
try {
expect(session.model?.provider).toBe("runtime-provider");
expect(session.model?.id).toBe("runtime-model");
expect(session.settings.getModelRole("subagent:deferred")).toBe("runtime-provider/runtime-model");
expect(session.settings.get("retry.fallbackChains")["subagent:deferred"]).toEqual([
"runtime-provider/runtime-reasoning-model",
]);
expect(modelFallbackMessage).toBeUndefined();
} finally {
await session.dispose();
}
});
test("installs fallback chain for remaining deferred subagent modelPattern candidates", async () => {
const { session } = await createAgentSession({
...(await buildSessionOptions(["runtime-provider/runtime-model", "runtime-provider/runtime-reasoning-model"])),