fix(model-picker): list each provider/model combo separately (#970)
* fix(model-picker): list each provider/model combo separately Previously, /model grouped models by name and showed all providers that offer the same model (e.g. 'claude-sonnet-4-5 — anthropic, google-antigravity'). This was confusing because: 1. Users couldn't tell which provider would be used when selecting by number 2. The display implied choice between providers but selection was automatic Now each provider/model combination is listed separately so users can explicitly select the exact provider they want. - Remove model grouping in buildModelPickerItems - Display format changed from 'model — providers' to 'provider/model' - pickProviderForModel now returns the single provider directly - Updated tests to reflect new behavior * fix: simplify model picker entries (#970) (thanks @mcinteerj) --------- Co-authored-by: Keith the Silly Goose <keith@42bolton.macnet.nz> Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
## 2026.1.15 (unreleased)
|
## 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.
|
- 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.
|
- Docs: clarify multi-gateway rescue bot guidance. (#969) — thanks @bjesuiter.
|
||||||
- Fix: guard model fallback against undefined provider/model values. (#954) — thanks @roshanasingh4.
|
- Fix: guard model fallback against undefined provider/model values. (#954) — thanks @roshanasingh4.
|
||||||
|
|||||||
@@ -85,8 +85,8 @@ describe("directive behavior", () => {
|
|||||||
|
|
||||||
const text = Array.isArray(res) ? res[0]?.text : res?.text;
|
const text = Array.isArray(res) ? res[0]?.text : res?.text;
|
||||||
expect(text).toContain("Pick: /model <#> or /model <provider/model>");
|
expect(text).toContain("Pick: /model <#> or /model <provider/model>");
|
||||||
expect(text).toContain("claude-opus-4-5 — anthropic");
|
expect(text).toContain("anthropic/claude-opus-4-5");
|
||||||
expect(text).toContain("gpt-4.1-mini — openai");
|
expect(text).toContain("openai/gpt-4.1-mini");
|
||||||
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
|
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -116,8 +116,8 @@ describe("directive behavior", () => {
|
|||||||
|
|
||||||
const text = Array.isArray(res) ? res[0]?.text : res?.text;
|
const text = Array.isArray(res) ? res[0]?.text : res?.text;
|
||||||
expect(text).toContain("Pick: /model <#> or /model <provider/model>");
|
expect(text).toContain("Pick: /model <#> or /model <provider/model>");
|
||||||
expect(text).toContain("claude-opus-4-5 — anthropic");
|
expect(text).toContain("anthropic/claude-opus-4-5");
|
||||||
expect(text).toContain("gpt-4.1-mini — openai");
|
expect(text).toContain("openai/gpt-4.1-mini");
|
||||||
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
|
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -166,9 +166,9 @@ describe("directive behavior", () => {
|
|||||||
);
|
);
|
||||||
|
|
||||||
const text = Array.isArray(res) ? res[0]?.text : res?.text;
|
const text = Array.isArray(res) ? res[0]?.text : res?.text;
|
||||||
expect(text).toContain("claude-opus-4-5 — anthropic");
|
expect(text).toContain("anthropic/claude-opus-4-5");
|
||||||
expect(text).toContain("gpt-4.1-mini — openai");
|
expect(text).toContain("openai/gpt-4.1-mini");
|
||||||
expect(text).toContain("MiniMax-M2.1 — minimax");
|
expect(text).toContain("minimax/MiniMax-M2.1");
|
||||||
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
|
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -213,7 +213,7 @@ describe("directive behavior", () => {
|
|||||||
const text = Array.isArray(res) ? res[0]?.text : res?.text;
|
const text = Array.isArray(res) ? res[0]?.text : res?.text;
|
||||||
expect(text).toContain("anthropic/claude-opus-4-5");
|
expect(text).toContain("anthropic/claude-opus-4-5");
|
||||||
expect(text).toContain("Pick: /model <#> or /model <provider/model>");
|
expect(text).toContain("Pick: /model <#> or /model <provider/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(text).not.toContain("claude-sonnet-4-1");
|
||||||
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
|
expect(runEmbeddedPiAgent).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,4 +1,3 @@
|
|||||||
import fs from "node:fs/promises";
|
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||||
import { normalizeTestText } from "../../test/helpers/normalize-text.js";
|
import { normalizeTestText } from "../../test/helpers/normalize-text.js";
|
||||||
@@ -96,7 +95,7 @@ afterEach(() => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe("trigger handling", () => {
|
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) => {
|
await withTempHome(async (home) => {
|
||||||
const cfg = makeCfg(home);
|
const cfg = makeCfg(home);
|
||||||
const res = await getReplyFromConfig(
|
const res = await getReplyFromConfig(
|
||||||
@@ -116,8 +115,11 @@ describe("trigger handling", () => {
|
|||||||
const text = Array.isArray(res) ? res[0]?.text : res?.text;
|
const text = Array.isArray(res) ? res[0]?.text : res?.text;
|
||||||
const normalized = normalizeTestText(text ?? "");
|
const normalized = normalizeTestText(text ?? "");
|
||||||
expect(normalized).toContain("Pick: /model <#> or /model <provider/model>");
|
expect(normalized).toContain("Pick: /model <#> or /model <provider/model>");
|
||||||
expect(normalized).toContain("1) claude-opus-4-5 — anthropic, openrouter");
|
// Each provider/model combo is listed separately for clear selection
|
||||||
expect(normalized).toContain("3) gpt-5.2 — openai, openai-codex");
|
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).toContain("More: /model status");
|
||||||
expect(normalized).not.toContain("reasoning");
|
expect(normalized).not.toContain("reasoning");
|
||||||
expect(normalized).not.toContain("image");
|
expect(normalized).not.toContain("image");
|
||||||
@@ -152,27 +154,12 @@ describe("trigger handling", () => {
|
|||||||
expect(store[sessionKey]?.modelOverride).toBeUndefined();
|
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) => {
|
await withTempHome(async (home) => {
|
||||||
const cfg = makeCfg(home);
|
const cfg = makeCfg(home);
|
||||||
const sessionKey = "telegram:slash:111";
|
const sessionKey = "telegram:slash:111";
|
||||||
|
|
||||||
await fs.writeFile(
|
// /model 1 should select the first item (anthropic/claude-opus-4-5)
|
||||||
cfg.session.store,
|
|
||||||
JSON.stringify(
|
|
||||||
{
|
|
||||||
[sessionKey]: {
|
|
||||||
sessionId: "session-openrouter",
|
|
||||||
updatedAt: Date.now(),
|
|
||||||
providerOverride: "openrouter",
|
|
||||||
modelOverride: "anthropic/claude-opus-4-5",
|
|
||||||
},
|
|
||||||
},
|
|
||||||
null,
|
|
||||||
2,
|
|
||||||
),
|
|
||||||
);
|
|
||||||
|
|
||||||
const res = await getReplyFromConfig(
|
const res = await getReplyFromConfig(
|
||||||
{
|
{
|
||||||
Body: "/model 1",
|
Body: "/model 1",
|
||||||
@@ -188,13 +175,15 @@ describe("trigger handling", () => {
|
|||||||
);
|
);
|
||||||
|
|
||||||
const text = Array.isArray(res) ? res[0]?.text : res?.text;
|
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(
|
expect(normalizeTestText(text ?? "")).toContain(
|
||||||
"Model set to openrouter/anthropic/claude-opus-4-5",
|
"anthropic/claude-opus-4-5",
|
||||||
);
|
);
|
||||||
|
|
||||||
const store = loadSessionStore(cfg.session.store);
|
const store = loadSessionStore(cfg.session.store);
|
||||||
expect(store[sessionKey]?.providerOverride).toBe("openrouter");
|
// When selecting the default, overrides are cleared
|
||||||
expect(store[sessionKey]?.modelOverride).toBe("anthropic/claude-opus-4-5");
|
expect(store[sessionKey]?.providerOverride).toBeUndefined();
|
||||||
|
expect(store[sessionKey]?.modelOverride).toBeUndefined();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
it("selects a model by index via /model <#>", async () => {
|
it("selects a model by index via /model <#>", async () => {
|
||||||
|
|||||||
@@ -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";
|
import type { ClawdbotConfig } from "../../config/config.js";
|
||||||
|
|
||||||
export type ModelPickerCatalogEntry = {
|
export type ModelPickerCatalogEntry = {
|
||||||
@@ -7,11 +7,7 @@ export type ModelPickerCatalogEntry = {
|
|||||||
name?: string;
|
name?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
export type ModelPickerItem = {
|
export type ModelPickerItem = ModelRef;
|
||||||
model: string;
|
|
||||||
providers: string[];
|
|
||||||
providerModels: Record<string, string>;
|
|
||||||
};
|
|
||||||
|
|
||||||
const MODEL_PICK_PROVIDER_PREFERENCE = [
|
const MODEL_PICK_PROVIDER_PREFERENCE = [
|
||||||
"anthropic",
|
"anthropic",
|
||||||
@@ -31,68 +27,43 @@ const MODEL_PICK_PROVIDER_PREFERENCE = [
|
|||||||
"lmstudio",
|
"lmstudio",
|
||||||
] as const;
|
] as const;
|
||||||
|
|
||||||
function normalizeModelFamilyId(id: string): string {
|
const PROVIDER_RANK = new Map<string, number>(
|
||||||
const trimmed = id.trim();
|
MODEL_PICK_PROVIDER_PREFERENCE.map((provider, idx) => [provider, idx]),
|
||||||
if (!trimmed) return trimmed;
|
);
|
||||||
const parts = trimmed.split("/").filter(Boolean);
|
|
||||||
return parts.length > 0 ? (parts[parts.length - 1] ?? trimmed) : trimmed;
|
|
||||||
}
|
|
||||||
|
|
||||||
function sortProvidersForPicker(providers: string[]): string[] {
|
function compareProvidersForPicker(a: string, b: string): number {
|
||||||
const pref = new Map<string, number>(
|
const pa = PROVIDER_RANK.get(a);
|
||||||
MODEL_PICK_PROVIDER_PREFERENCE.map((provider, idx) => [provider, idx]),
|
const pb = PROVIDER_RANK.get(b);
|
||||||
);
|
if (pa !== undefined && pb !== undefined) return pa - pb;
|
||||||
return providers.sort((a, b) => {
|
if (pa !== undefined) return -1;
|
||||||
const pa = pref.get(a);
|
if (pb !== undefined) return 1;
|
||||||
const pb = pref.get(b);
|
return a.localeCompare(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[] {
|
export function buildModelPickerItems(catalog: ModelPickerCatalogEntry[]): ModelPickerItem[] {
|
||||||
const byModel = new Map<string, { providerModels: Record<string, string> }>();
|
const seen = new Set<string>();
|
||||||
|
const out: ModelPickerItem[] = [];
|
||||||
|
|
||||||
for (const entry of catalog) {
|
for (const entry of catalog) {
|
||||||
const provider = normalizeProviderId(entry.provider);
|
const provider = normalizeProviderId(entry.provider);
|
||||||
const model = normalizeModelFamilyId(entry.id);
|
const model = entry.id?.trim();
|
||||||
if (!provider || !model) continue;
|
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: {
|
const key = `${provider}/${model}`;
|
||||||
item: ModelPickerItem;
|
if (seen.has(key)) continue;
|
||||||
preferredProvider?: string;
|
seen.add(key);
|
||||||
}): { provider: string; model: string } | null {
|
|
||||||
const preferred = params.preferredProvider
|
out.push({ model, provider });
|
||||||
? normalizeProviderId(params.preferredProvider)
|
|
||||||
: undefined;
|
|
||||||
if (preferred && params.item.providerModels[preferred]) {
|
|
||||||
return {
|
|
||||||
provider: preferred,
|
|
||||||
model: params.item.providerModels[preferred],
|
|
||||||
};
|
|
||||||
}
|
}
|
||||||
const first = params.item.providers[0];
|
|
||||||
if (!first) return null;
|
// Sort by provider preference first, then by model name
|
||||||
return {
|
out.sort((a, b) => {
|
||||||
provider: first,
|
const providerOrder = compareProvidersForPicker(a.provider, b.provider);
|
||||||
model: params.item.providerModels[first] ?? params.item.model,
|
if (providerOrder !== 0) return providerOrder;
|
||||||
};
|
return a.model.toLowerCase().localeCompare(b.model.toLowerCase());
|
||||||
|
});
|
||||||
|
|
||||||
|
return out;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function resolveProviderEndpointLabel(
|
export function resolveProviderEndpointLabel(
|
||||||
|
|||||||
@@ -18,7 +18,6 @@ import {
|
|||||||
import {
|
import {
|
||||||
buildModelPickerItems,
|
buildModelPickerItems,
|
||||||
type ModelPickerCatalogEntry,
|
type ModelPickerCatalogEntry,
|
||||||
pickProviderForModel,
|
|
||||||
resolveProviderEndpointLabel,
|
resolveProviderEndpointLabel,
|
||||||
} from "./directive-handling.model-picker.js";
|
} from "./directive-handling.model-picker.js";
|
||||||
import type { InlineDirectives } from "./directive-handling.parse.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 current = `${params.provider}/${params.model}`;
|
||||||
const lines: string[] = [`Current: ${current}`, "Pick: /model <#> or /model <provider/model>"];
|
const lines: string[] = [`Current: ${current}`, "Pick: /model <#> or /model <provider/model>"];
|
||||||
for (const [idx, item] of items.entries()) {
|
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");
|
lines.push("", "More: /model status");
|
||||||
return { text: lines.join("\n") };
|
return { text: lines.join("\n") };
|
||||||
@@ -236,22 +235,13 @@ export function resolveModelSelectionFromDirective(params: {
|
|||||||
errorText: `Invalid model selection "${raw}". Use /model to list.`,
|
errorText: `Invalid model selection "${raw}". Use /model to list.`,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
const picked = pickProviderForModel({
|
const key = `${item.provider}/${item.model}`;
|
||||||
item,
|
|
||||||
preferredProvider: params.provider,
|
|
||||||
});
|
|
||||||
if (!picked) {
|
|
||||||
return {
|
|
||||||
errorText: `Invalid model selection "${raw}". Use /model to list.`,
|
|
||||||
};
|
|
||||||
}
|
|
||||||
const key = `${picked.provider}/${picked.model}`;
|
|
||||||
const aliases = params.aliasIndex.byKey.get(key);
|
const aliases = params.aliasIndex.byKey.get(key);
|
||||||
const alias = aliases && aliases.length > 0 ? aliases[0] : undefined;
|
const alias = aliases && aliases.length > 0 ? aliases[0] : undefined;
|
||||||
modelSelection = {
|
modelSelection = {
|
||||||
provider: picked.provider,
|
provider: item.provider,
|
||||||
model: picked.model,
|
model: item.model,
|
||||||
isDefault: picked.provider === params.defaultProvider && picked.model === params.defaultModel,
|
isDefault: item.provider === params.defaultProvider && item.model === params.defaultModel,
|
||||||
...(alias ? { alias } : {}),
|
...(alias ? { alias } : {}),
|
||||||
};
|
};
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
Reference in New Issue
Block a user