diff --git a/CHANGELOG.md b/CHANGELOG.md index 72e73398d..19decdf18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 2026.1.15 (unreleased) +- Fix: list model picker entries as provider/model pairs for explicit selection. (#970) — thanks @mcinteerj. - Daemon: fix profile-aware service label resolution (env-driven) and add coverage for launchd/systemd/schtasks. (#969) — thanks @bjesuiter. - Docs: clarify multi-gateway rescue bot guidance. (#969) — thanks @bjesuiter. - Fix: guard model fallback against undefined provider/model values. (#954) — thanks @roshanasingh4. diff --git a/src/auto-reply/reply.directive.directive-behavior.lists-allowlisted-models-model-list.test.ts b/src/auto-reply/reply.directive.directive-behavior.lists-allowlisted-models-model-list.test.ts index c959ce362..fd77248f1 100644 --- a/src/auto-reply/reply.directive.directive-behavior.lists-allowlisted-models-model-list.test.ts +++ b/src/auto-reply/reply.directive.directive-behavior.lists-allowlisted-models-model-list.test.ts @@ -85,8 +85,8 @@ describe("directive behavior", () => { const text = Array.isArray(res) ? res[0]?.text : res?.text; expect(text).toContain("Pick: /model <#> or /model "); - expect(text).toContain("claude-opus-4-5 — anthropic"); - expect(text).toContain("gpt-4.1-mini — openai"); + expect(text).toContain("anthropic/claude-opus-4-5"); + expect(text).toContain("openai/gpt-4.1-mini"); expect(runEmbeddedPiAgent).not.toHaveBeenCalled(); }); }); @@ -116,8 +116,8 @@ describe("directive behavior", () => { const text = Array.isArray(res) ? res[0]?.text : res?.text; expect(text).toContain("Pick: /model <#> or /model "); - expect(text).toContain("claude-opus-4-5 — anthropic"); - expect(text).toContain("gpt-4.1-mini — openai"); + expect(text).toContain("anthropic/claude-opus-4-5"); + expect(text).toContain("openai/gpt-4.1-mini"); expect(runEmbeddedPiAgent).not.toHaveBeenCalled(); }); }); @@ -166,9 +166,9 @@ describe("directive behavior", () => { ); const text = Array.isArray(res) ? res[0]?.text : res?.text; - expect(text).toContain("claude-opus-4-5 — anthropic"); - expect(text).toContain("gpt-4.1-mini — openai"); - expect(text).toContain("MiniMax-M2.1 — minimax"); + expect(text).toContain("anthropic/claude-opus-4-5"); + expect(text).toContain("openai/gpt-4.1-mini"); + expect(text).toContain("minimax/MiniMax-M2.1"); expect(runEmbeddedPiAgent).not.toHaveBeenCalled(); }); }); diff --git a/src/auto-reply/reply.directive.directive-behavior.updates-tool-verbose-during-flight-run-toggle.test.ts b/src/auto-reply/reply.directive.directive-behavior.updates-tool-verbose-during-flight-run-toggle.test.ts index 8844bf846..c97a07434 100644 --- a/src/auto-reply/reply.directive.directive-behavior.updates-tool-verbose-during-flight-run-toggle.test.ts +++ b/src/auto-reply/reply.directive.directive-behavior.updates-tool-verbose-during-flight-run-toggle.test.ts @@ -213,7 +213,7 @@ describe("directive behavior", () => { const text = Array.isArray(res) ? res[0]?.text : res?.text; expect(text).toContain("anthropic/claude-opus-4-5"); expect(text).toContain("Pick: /model <#> or /model "); - expect(text).toContain("gpt-4.1-mini — openai"); + expect(text).toContain("openai/gpt-4.1-mini"); expect(text).not.toContain("claude-sonnet-4-1"); expect(runEmbeddedPiAgent).not.toHaveBeenCalled(); }); diff --git a/src/auto-reply/reply.triggers.trigger-handling.shows-quick-model-picker-grouped-by-model.test.ts b/src/auto-reply/reply.triggers.trigger-handling.shows-quick-model-picker-grouped-by-model.test.ts index 248ff9e42..3a8569800 100644 --- a/src/auto-reply/reply.triggers.trigger-handling.shows-quick-model-picker-grouped-by-model.test.ts +++ b/src/auto-reply/reply.triggers.trigger-handling.shows-quick-model-picker-grouped-by-model.test.ts @@ -1,4 +1,3 @@ -import fs from "node:fs/promises"; import { join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { normalizeTestText } from "../../test/helpers/normalize-text.js"; @@ -96,7 +95,7 @@ afterEach(() => { }); describe("trigger handling", () => { - it("shows a quick /model picker grouped by model with providers", async () => { + it("shows a quick /model picker listing provider/model pairs", async () => { await withTempHome(async (home) => { const cfg = makeCfg(home); const res = await getReplyFromConfig( @@ -116,8 +115,11 @@ describe("trigger handling", () => { const text = Array.isArray(res) ? res[0]?.text : res?.text; const normalized = normalizeTestText(text ?? ""); expect(normalized).toContain("Pick: /model <#> or /model "); - expect(normalized).toContain("1) claude-opus-4-5 — anthropic, openrouter"); - expect(normalized).toContain("3) gpt-5.2 — openai, openai-codex"); + // Each provider/model combo is listed separately for clear selection + expect(normalized).toContain("anthropic/claude-opus-4-5"); + expect(normalized).toContain("openrouter/anthropic/claude-opus-4-5"); + expect(normalized).toContain("openai/gpt-5.2"); + expect(normalized).toContain("openai-codex/gpt-5.2"); expect(normalized).toContain("More: /model status"); expect(normalized).not.toContain("reasoning"); expect(normalized).not.toContain("image"); @@ -152,27 +154,12 @@ describe("trigger handling", () => { expect(store[sessionKey]?.modelOverride).toBeUndefined(); }); }); - it("prefers the current provider when selecting /model <#>", async () => { + it("selects exact provider/model combo by index via /model <#>", async () => { await withTempHome(async (home) => { const cfg = makeCfg(home); const sessionKey = "telegram:slash:111"; - await fs.writeFile( - cfg.session.store, - JSON.stringify( - { - [sessionKey]: { - sessionId: "session-openrouter", - updatedAt: Date.now(), - providerOverride: "openrouter", - modelOverride: "anthropic/claude-opus-4-5", - }, - }, - null, - 2, - ), - ); - + // /model 1 should select the first item (anthropic/claude-opus-4-5) const res = await getReplyFromConfig( { Body: "/model 1", @@ -188,13 +175,15 @@ describe("trigger handling", () => { ); const text = Array.isArray(res) ? res[0]?.text : res?.text; + // Selecting the default model shows "reset to default" instead of "set to" expect(normalizeTestText(text ?? "")).toContain( - "Model set to openrouter/anthropic/claude-opus-4-5", + "anthropic/claude-opus-4-5", ); const store = loadSessionStore(cfg.session.store); - expect(store[sessionKey]?.providerOverride).toBe("openrouter"); - expect(store[sessionKey]?.modelOverride).toBe("anthropic/claude-opus-4-5"); + // When selecting the default, overrides are cleared + expect(store[sessionKey]?.providerOverride).toBeUndefined(); + expect(store[sessionKey]?.modelOverride).toBeUndefined(); }); }); it("selects a model by index via /model <#>", async () => { diff --git a/src/auto-reply/reply/directive-handling.model-picker.ts b/src/auto-reply/reply/directive-handling.model-picker.ts index 98c917752..0fd2af367 100644 --- a/src/auto-reply/reply/directive-handling.model-picker.ts +++ b/src/auto-reply/reply/directive-handling.model-picker.ts @@ -1,4 +1,4 @@ -import { normalizeProviderId } from "../../agents/model-selection.js"; +import { type ModelRef, normalizeProviderId } from "../../agents/model-selection.js"; import type { ClawdbotConfig } from "../../config/config.js"; export type ModelPickerCatalogEntry = { @@ -7,11 +7,7 @@ export type ModelPickerCatalogEntry = { name?: string; }; -export type ModelPickerItem = { - model: string; - providers: string[]; - providerModels: Record; -}; +export type ModelPickerItem = ModelRef; const MODEL_PICK_PROVIDER_PREFERENCE = [ "anthropic", @@ -31,68 +27,43 @@ const MODEL_PICK_PROVIDER_PREFERENCE = [ "lmstudio", ] as const; -function normalizeModelFamilyId(id: string): string { - const trimmed = id.trim(); - if (!trimmed) return trimmed; - const parts = trimmed.split("/").filter(Boolean); - return parts.length > 0 ? (parts[parts.length - 1] ?? trimmed) : trimmed; -} +const PROVIDER_RANK = new Map( + MODEL_PICK_PROVIDER_PREFERENCE.map((provider, idx) => [provider, idx]), +); -function sortProvidersForPicker(providers: string[]): string[] { - const pref = new Map( - MODEL_PICK_PROVIDER_PREFERENCE.map((provider, idx) => [provider, idx]), - ); - return providers.sort((a, b) => { - const pa = pref.get(a); - const pb = pref.get(b); - if (pa !== undefined && pb !== undefined) return pa - pb; - if (pa !== undefined) return -1; - if (pb !== undefined) return 1; - return a.localeCompare(b); - }); +function compareProvidersForPicker(a: string, b: string): number { + const pa = PROVIDER_RANK.get(a); + const pb = PROVIDER_RANK.get(b); + if (pa !== undefined && pb !== undefined) return pa - pb; + if (pa !== undefined) return -1; + if (pb !== undefined) return 1; + return a.localeCompare(b); } export function buildModelPickerItems(catalog: ModelPickerCatalogEntry[]): ModelPickerItem[] { - const byModel = new Map }>(); + const seen = new Set(); + const out: ModelPickerItem[] = []; + for (const entry of catalog) { const provider = normalizeProviderId(entry.provider); - const model = normalizeModelFamilyId(entry.id); + const model = entry.id?.trim(); if (!provider || !model) continue; - const existing = byModel.get(model); - if (existing) { - existing.providerModels[provider] = entry.id; - continue; - } - byModel.set(model, { providerModels: { [provider]: entry.id } }); - } - const out: ModelPickerItem[] = []; - for (const [model, data] of byModel.entries()) { - const providers = sortProvidersForPicker(Object.keys(data.providerModels)); - out.push({ model, providers, providerModels: data.providerModels }); - } - out.sort((a, b) => a.model.toLowerCase().localeCompare(b.model.toLowerCase())); - return out; -} -export function pickProviderForModel(params: { - item: ModelPickerItem; - preferredProvider?: string; -}): { provider: string; model: string } | null { - const preferred = params.preferredProvider - ? normalizeProviderId(params.preferredProvider) - : undefined; - if (preferred && params.item.providerModels[preferred]) { - return { - provider: preferred, - model: params.item.providerModels[preferred], - }; + const key = `${provider}/${model}`; + if (seen.has(key)) continue; + seen.add(key); + + out.push({ model, provider }); } - const first = params.item.providers[0]; - if (!first) return null; - return { - provider: first, - model: params.item.providerModels[first] ?? params.item.model, - }; + + // Sort by provider preference first, then by model name + out.sort((a, b) => { + const providerOrder = compareProvidersForPicker(a.provider, b.provider); + if (providerOrder !== 0) return providerOrder; + return a.model.toLowerCase().localeCompare(b.model.toLowerCase()); + }); + + return out; } export function resolveProviderEndpointLabel( diff --git a/src/auto-reply/reply/directive-handling.model.ts b/src/auto-reply/reply/directive-handling.model.ts index b63f2c6d1..61a693798 100644 --- a/src/auto-reply/reply/directive-handling.model.ts +++ b/src/auto-reply/reply/directive-handling.model.ts @@ -18,7 +18,6 @@ import { import { buildModelPickerItems, type ModelPickerCatalogEntry, - pickProviderForModel, resolveProviderEndpointLabel, } from "./directive-handling.model-picker.js"; import type { InlineDirectives } from "./directive-handling.parse.js"; @@ -126,7 +125,7 @@ export async function maybeHandleModelDirectiveInfo(params: { const current = `${params.provider}/${params.model}`; const lines: string[] = [`Current: ${current}`, "Pick: /model <#> or /model "]; for (const [idx, item] of items.entries()) { - lines.push(`${idx + 1}) ${item.model} — ${item.providers.join(", ")}`); + lines.push(`${idx + 1}) ${item.provider}/${item.model}`); } lines.push("", "More: /model status"); return { text: lines.join("\n") }; @@ -236,22 +235,13 @@ export function resolveModelSelectionFromDirective(params: { errorText: `Invalid model selection "${raw}". Use /model to list.`, }; } - const picked = pickProviderForModel({ - item, - preferredProvider: params.provider, - }); - if (!picked) { - return { - errorText: `Invalid model selection "${raw}". Use /model to list.`, - }; - } - const key = `${picked.provider}/${picked.model}`; + const key = `${item.provider}/${item.model}`; const aliases = params.aliasIndex.byKey.get(key); const alias = aliases && aliases.length > 0 ? aliases[0] : undefined; modelSelection = { - provider: picked.provider, - model: picked.model, - isDefault: picked.provider === params.defaultProvider && picked.model === params.defaultModel, + provider: item.provider, + model: item.model, + isDefault: item.provider === params.defaultProvider && item.model === params.defaultModel, ...(alias ? { alias } : {}), }; } else {