mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): expose unavailable scoped models (#7032)
* fix(coding-agent): expose unavailable scoped models Keep missing configured models editable in the scoped-model selector. This fixes #6949. * fix(coding-agent): reuse model scope diagnostics * fix(coding-agent): classify model scope diagnostics
This commit is contained in:
parent
8eef62ed3e
commit
a3ee1d286e
6 changed files with 244 additions and 37 deletions
|
|
@ -5,6 +5,7 @@
|
|||
### Fixed
|
||||
|
||||
- Fixed compaction and branch summaries for providers whose authentication resolves entirely to request headers ([#5871](https://github.com/earendil-works/pi/issues/5871))
|
||||
- Fixed unavailable scoped models being hidden from `/models`, allowing them to be removed without editing settings manually ([#6949](https://github.com/earendil-works/pi/issues/6949)).
|
||||
|
||||
## [0.82.0] - 2026-07-24
|
||||
|
||||
|
|
|
|||
|
|
@ -260,6 +260,7 @@ export function parseModelPattern(
|
|||
*/
|
||||
export interface ModelScopeDiagnostic {
|
||||
type: "warning";
|
||||
code: "no-match" | "invalid-thinking-level";
|
||||
message: string;
|
||||
pattern: string;
|
||||
}
|
||||
|
|
@ -309,7 +310,12 @@ export async function resolveModelScopeWithDiagnostics(
|
|||
});
|
||||
|
||||
if (matchingModels.length === 0) {
|
||||
diagnostics.push({ type: "warning", message: `No models match pattern "${pattern}"`, pattern });
|
||||
diagnostics.push({
|
||||
type: "warning",
|
||||
code: "no-match",
|
||||
message: `No models match pattern "${pattern}"`,
|
||||
pattern,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -324,11 +330,16 @@ export async function resolveModelScopeWithDiagnostics(
|
|||
const { model, thinkingLevel, warning } = parseModelPattern(pattern, availableModels);
|
||||
|
||||
if (warning) {
|
||||
diagnostics.push({ type: "warning", message: warning, pattern });
|
||||
diagnostics.push({ type: "warning", code: "invalid-thinking-level", message: warning, pattern });
|
||||
}
|
||||
|
||||
if (!model) {
|
||||
diagnostics.push({ type: "warning", message: `No models match pattern "${pattern}"`, pattern });
|
||||
diagnostics.push({
|
||||
type: "warning",
|
||||
code: "no-match",
|
||||
message: `No models match pattern "${pattern}"`,
|
||||
pattern,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -36,7 +36,7 @@ function enableAll(enabledIds: EnabledIds, allIds: string[], targetIds?: string[
|
|||
for (const id of targets) {
|
||||
if (!result.includes(id)) result.push(id);
|
||||
}
|
||||
return result.length === allIds.length ? null : result;
|
||||
return result.length === allIds.length && result.every((id) => allIds.includes(id)) ? null : result;
|
||||
}
|
||||
|
||||
function clearAll(enabledIds: EnabledIds, allIds: string[], targetIds?: string[]): EnabledIds {
|
||||
|
|
@ -67,7 +67,7 @@ function getSortedIds(enabledIds: EnabledIds, allIds: string[]): string[] {
|
|||
|
||||
interface ModelItem {
|
||||
fullId: string;
|
||||
model: Model<any>;
|
||||
model: Model<any> | undefined;
|
||||
enabled: boolean;
|
||||
}
|
||||
|
||||
|
|
@ -152,20 +152,20 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
|||
}
|
||||
|
||||
private buildItems(): ModelItem[] {
|
||||
// Filter out IDs that no longer have a corresponding model (e.g., after logout)
|
||||
return getSortedIds(this.enabledIds, this.allIds)
|
||||
.filter((id) => this.modelsById.has(id))
|
||||
.map((id) => ({
|
||||
fullId: id,
|
||||
model: this.modelsById.get(id)!,
|
||||
enabled: isEnabled(this.enabledIds, id),
|
||||
}));
|
||||
return getSortedIds(this.enabledIds, this.allIds).map((id) => ({
|
||||
fullId: id,
|
||||
model: this.modelsById.get(id),
|
||||
enabled: isEnabled(this.enabledIds, id),
|
||||
}));
|
||||
}
|
||||
|
||||
private getFooterText(): string {
|
||||
const enabledCount = this.enabledIds?.length ?? this.allIds.length;
|
||||
const enabledCount = this.enabledIds?.filter((id) => this.modelsById.has(id)).length ?? this.allIds.length;
|
||||
const unavailableCount = this.enabledIds?.filter((id) => !this.modelsById.has(id)).length ?? 0;
|
||||
const allEnabled = this.enabledIds === null;
|
||||
const countText = allEnabled ? "all enabled" : `${enabledCount}/${this.allIds.length} enabled`;
|
||||
const countText = allEnabled
|
||||
? "all enabled"
|
||||
: `${enabledCount}/${this.allIds.length} enabled${unavailableCount ? ` · ${unavailableCount} unavailable` : ""}`;
|
||||
const parts = [
|
||||
`${keyText("tui.select.confirm")} toggle`,
|
||||
`${keyText("app.models.enableAll")} all`,
|
||||
|
|
@ -184,8 +184,10 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
|||
const query = this.searchInput.getValue();
|
||||
const items = this.buildItems();
|
||||
this.filteredItems = query
|
||||
? fuzzyFilter(items, query, (i) =>
|
||||
getModelSearchText({ id: i.model.id, provider: i.model.provider, name: i.model.name }),
|
||||
? fuzzyFilter(items, query, (item) =>
|
||||
item.model
|
||||
? getModelSearchText({ id: item.model.id, provider: item.model.provider, name: item.model.name })
|
||||
: item.fullId,
|
||||
)
|
||||
: items;
|
||||
this.selectedIndex = Math.min(this.selectedIndex, Math.max(0, this.filteredItems.length - 1));
|
||||
|
|
@ -216,9 +218,16 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
|||
const item = this.filteredItems[i]!;
|
||||
const isSelected = i === this.selectedIndex;
|
||||
const prefix = isSelected ? theme.fg("accent", "→ ") : " ";
|
||||
const modelText = isSelected ? theme.fg("accent", item.model.id) : item.model.id;
|
||||
const providerBadge = theme.fg("muted", ` [${item.model.provider}]`);
|
||||
const status = allEnabled ? "" : item.enabled ? theme.fg("success", " ✓") : theme.fg("dim", " ✗");
|
||||
const id = item.model?.id ?? item.fullId;
|
||||
const modelText = isSelected ? theme.fg("accent", id) : id;
|
||||
const providerBadge = theme.fg("muted", item.model ? ` [${item.model.provider}]` : " [unavailable]");
|
||||
const status = item.model
|
||||
? allEnabled
|
||||
? ""
|
||||
: item.enabled
|
||||
? theme.fg("success", " ✓")
|
||||
: theme.fg("dim", " ✗")
|
||||
: theme.fg("dim", " ✗");
|
||||
this.listContainer.addChild(new Text(`${prefix}${modelText}${providerBadge}${status}`, 0, 0));
|
||||
}
|
||||
|
||||
|
|
@ -232,7 +241,13 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
|||
if (this.filteredItems.length > 0) {
|
||||
const selected = this.filteredItems[this.selectedIndex];
|
||||
this.listContainer.addChild(new Spacer(1));
|
||||
this.listContainer.addChild(new Text(theme.fg("muted", ` Model Name: ${selected.model.name}`), 0, 0));
|
||||
this.listContainer.addChild(
|
||||
new Text(
|
||||
theme.fg("muted", ` ${selected.model ? `Model Name: ${selected.model.name}` : "Model unavailable"}`),
|
||||
0,
|
||||
0,
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -310,7 +325,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
|
|||
// Toggle provider of current item
|
||||
if (kb.matches(data, "app.models.toggleProvider")) {
|
||||
const item = this.filteredItems[this.selectedIndex];
|
||||
if (item) {
|
||||
if (item?.model) {
|
||||
const provider = item.model.provider;
|
||||
const providerIds = this.allIds.filter((id) => this.modelsById.get(id)!.provider === provider);
|
||||
const allEnabled = providerIds.every((id) => isEnabled(this.enabledIds, id));
|
||||
|
|
|
|||
|
|
@ -76,7 +76,12 @@ import { FooterDataProvider, type ReadonlyFooterDataProvider } from "../../core/
|
|||
import { configureHttpDispatcher, formatHttpIdleTimeoutMs } from "../../core/http-dispatcher.ts";
|
||||
import { type AppKeybinding, KeybindingsManager } from "../../core/keybindings.ts";
|
||||
import { createCompactionSummaryMessage } from "../../core/messages.ts";
|
||||
import { defaultModelPerProvider, findExactModelReferenceMatch, resolveModelScope } from "../../core/model-resolver.ts";
|
||||
import {
|
||||
defaultModelPerProvider,
|
||||
findExactModelReferenceMatch,
|
||||
resolveModelScope,
|
||||
resolveModelScopeWithDiagnostics,
|
||||
} from "../../core/model-resolver.ts";
|
||||
import { DefaultPackageManager } from "../../core/package-manager.ts";
|
||||
import type { ResourceDiagnostic } from "../../core/resource-loader.ts";
|
||||
import { formatMissingSessionCwdPrompt, MissingSessionCwdError } from "../../core/session-cwd.ts";
|
||||
|
|
@ -4463,14 +4468,20 @@ export class InteractiveMode {
|
|||
// Get all available models
|
||||
await this.session.modelRuntime.refresh();
|
||||
const allModels = [...(await this.session.modelRuntime.getAvailable())];
|
||||
const allModelIds = new Set(allModels.map((model) => `${model.provider}/${model.id}`));
|
||||
const configuredPatterns = this.settingsManager.getEnabledModels();
|
||||
const sessionScopedModels = this.session.scopedModels;
|
||||
|
||||
if (allModels.length === 0) {
|
||||
if (allModels.length === 0 && !configuredPatterns?.length && sessionScopedModels.length === 0) {
|
||||
this.showStatus("No models available");
|
||||
return;
|
||||
}
|
||||
|
||||
const configuredScope = configuredPatterns?.length
|
||||
? await resolveModelScopeWithDiagnostics(configuredPatterns, this.session.modelRuntime)
|
||||
: undefined;
|
||||
|
||||
// Check if session has scoped models (from previous session-only changes or CLI --models)
|
||||
const sessionScopedModels = this.session.scopedModels;
|
||||
const hasSessionScope = sessionScopedModels.length > 0;
|
||||
|
||||
// Build enabled model IDs from session state or settings
|
||||
|
|
@ -4479,19 +4490,25 @@ export class InteractiveMode {
|
|||
if (hasSessionScope) {
|
||||
// Use current session's scoped models
|
||||
currentEnabledIds = sessionScopedModels.map((scoped) => `${scoped.model.provider}/${scoped.model.id}`);
|
||||
} else {
|
||||
// Fall back to settings
|
||||
const patterns = this.settingsManager.getEnabledModels();
|
||||
if (patterns !== undefined && patterns.length > 0) {
|
||||
const scopedModels = await resolveModelScope(patterns, this.session.modelRuntime);
|
||||
currentEnabledIds = scopedModels.map((scoped) => `${scoped.model.provider}/${scoped.model.id}`);
|
||||
}
|
||||
} else if (configuredScope) {
|
||||
currentEnabledIds = configuredScope.scopedModels.map(
|
||||
(scoped) => `${scoped.model.provider}/${scoped.model.id}`,
|
||||
);
|
||||
}
|
||||
|
||||
for (const diagnostic of configuredScope?.diagnostics ?? []) {
|
||||
if (diagnostic.code !== "no-match") continue;
|
||||
currentEnabledIds ??= [];
|
||||
if (!currentEnabledIds.includes(diagnostic.pattern)) currentEnabledIds.push(diagnostic.pattern);
|
||||
}
|
||||
|
||||
// Helper to update session's scoped models (session-only, no persist)
|
||||
const updateSessionModels = async (enabledIds: string[] | null) => {
|
||||
currentEnabledIds = enabledIds === null ? null : [...enabledIds];
|
||||
if (enabledIds && enabledIds.length > 0 && enabledIds.length < allModels.length) {
|
||||
const hasEnabledAvailableModel = enabledIds?.some((id) => allModelIds.has(id)) ?? false;
|
||||
const allAvailableModelsEnabled =
|
||||
enabledIds !== null && [...allModelIds].every((id) => enabledIds.includes(id));
|
||||
if (enabledIds && hasEnabledAvailableModel && !allAvailableModelsEnabled) {
|
||||
const newScopedModels = await resolveModelScope(enabledIds, this.session.modelRuntime);
|
||||
this.session.setScopedModels(
|
||||
newScopedModels.map((sm) => ({
|
||||
|
|
@ -4519,10 +4536,11 @@ export class InteractiveMode {
|
|||
},
|
||||
onPersist: (enabledIds) => {
|
||||
// Persist to settings
|
||||
const newPatterns =
|
||||
enabledIds === null || enabledIds.length === allModels.length
|
||||
? undefined // All enabled = clear filter
|
||||
: enabledIds;
|
||||
const allEnabled =
|
||||
enabledIds !== null &&
|
||||
enabledIds.length === allModels.length &&
|
||||
enabledIds.every((id) => allModelIds.has(id));
|
||||
const newPatterns = enabledIds === null || allEnabled ? undefined : enabledIds;
|
||||
this.settingsManager.setEnabledModels(newPatterns ? [...newPatterns] : undefined);
|
||||
this.showStatus("Model selection saved to settings");
|
||||
},
|
||||
|
|
|
|||
|
|
@ -225,11 +225,13 @@ describe("resolveModelScopeWithDiagnostics", () => {
|
|||
{
|
||||
type: "warning",
|
||||
message: 'Invalid thinking level "invalid" in pattern "gpt-4o:invalid". Using default instead.',
|
||||
code: "invalid-thinking-level",
|
||||
pattern: "gpt-4o:invalid",
|
||||
},
|
||||
{
|
||||
type: "warning",
|
||||
message: 'No models match pattern "missing"',
|
||||
code: "no-match",
|
||||
pattern: "missing",
|
||||
},
|
||||
]);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,160 @@
|
|||
import type { Api, Model } from "@earendil-works/pi-ai";
|
||||
import { setKeybindings } from "@earendil-works/pi-tui";
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { KeybindingsManager } from "../../../src/core/keybindings.ts";
|
||||
import { ScopedModelsSelectorComponent } from "../../../src/modes/interactive/components/scoped-models-selector.ts";
|
||||
import { InteractiveMode } from "../../../src/modes/interactive/interactive-mode.ts";
|
||||
import { initTheme } from "../../../src/modes/interactive/theme/theme.ts";
|
||||
import { stripAnsi } from "../../../src/utils/ansi.ts";
|
||||
import { createHarness, type Harness } from "../harness.ts";
|
||||
|
||||
function createInteractiveContext(options: {
|
||||
allModels: Model<Api>[];
|
||||
enabledModelIds: string[];
|
||||
scopedModels?: Array<{ model: Model<Api> }>;
|
||||
}) {
|
||||
let selector: ScopedModelsSelectorComponent | undefined;
|
||||
const setScopedModels = vi.fn();
|
||||
const getAvailable = vi.fn().mockResolvedValue(options.allModels);
|
||||
const context = {
|
||||
session: {
|
||||
modelRuntime: {
|
||||
refresh: vi.fn(),
|
||||
getAvailable,
|
||||
},
|
||||
scopedModels: options.scopedModels ?? [],
|
||||
setScopedModels,
|
||||
},
|
||||
settingsManager: {
|
||||
getEnabledModels: () => options.enabledModelIds,
|
||||
setEnabledModels: vi.fn(),
|
||||
},
|
||||
showStatus: vi.fn(),
|
||||
showSelector: (factory: (done: () => void) => { component: ScopedModelsSelectorComponent }) => {
|
||||
selector = factory(() => {}).component;
|
||||
},
|
||||
updateAvailableProviderCount: vi.fn(),
|
||||
ui: { requestRender: vi.fn() },
|
||||
};
|
||||
return { context, getAvailable, getSelector: () => selector, setScopedModels };
|
||||
}
|
||||
|
||||
async function showModelsSelector(context: object): Promise<void> {
|
||||
const show = Reflect.get(InteractiveMode.prototype, "showModelsSelector") as (this: object) => Promise<void>;
|
||||
await show.call(context);
|
||||
}
|
||||
|
||||
describe("issue #6949 unavailable scoped models", () => {
|
||||
const harnesses: Harness[] = [];
|
||||
|
||||
beforeAll(() => {
|
||||
initTheme("dark");
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
setKeybindings(new KeybindingsManager());
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
while (harnesses.length > 0) harnesses.pop()?.cleanup();
|
||||
});
|
||||
|
||||
it("shows and removes an enabled model without a catalog entry", async () => {
|
||||
const harness = await createHarness({ models: [{ id: "available", name: "Available" }] });
|
||||
harnesses.push(harness);
|
||||
const availableId = `${harness.models[0].provider}/${harness.models[0].id}`;
|
||||
const unavailableId = `${harness.models[0].provider}/unavailable`;
|
||||
const changes: Array<string[] | null> = [];
|
||||
const persisted: Array<string[] | null> = [];
|
||||
const selector = new ScopedModelsSelectorComponent(
|
||||
{
|
||||
allModels: [...harness.models],
|
||||
enabledModelIds: [unavailableId, availableId],
|
||||
},
|
||||
{
|
||||
onChange: (enabledIds) => {
|
||||
changes.push(enabledIds);
|
||||
},
|
||||
onPersist: (enabledIds) => {
|
||||
persisted.push(enabledIds);
|
||||
},
|
||||
onCancel: () => {},
|
||||
},
|
||||
);
|
||||
|
||||
expect(stripAnsi(selector.render(100).join("\n"))).toContain(`${unavailableId} [unavailable] ✗`);
|
||||
selector.handleInput("\r");
|
||||
expect(changes).toEqual([[availableId]]);
|
||||
selector.handleInput("\x13");
|
||||
expect(persisted).toEqual([[availableId]]);
|
||||
});
|
||||
|
||||
it("passes unmatched settings patterns to the selector with one combined resolution", async () => {
|
||||
const harness = await createHarness({ models: [{ id: "available", name: "Available" }] });
|
||||
harnesses.push(harness);
|
||||
const unavailableIds = ["unavailable-one", "unavailable-two"].map((id) => `${harness.models[0].provider}/${id}`);
|
||||
const { context, getAvailable, getSelector } = createInteractiveContext({
|
||||
allModels: [],
|
||||
enabledModelIds: unavailableIds,
|
||||
});
|
||||
|
||||
await showModelsSelector(context);
|
||||
|
||||
const selector = getSelector();
|
||||
if (!selector) throw new Error("Expected scoped-model selector to open");
|
||||
const rendered = stripAnsi(selector.render(100).join("\n"));
|
||||
for (const unavailableId of unavailableIds) {
|
||||
expect(rendered).toContain(`${unavailableId} [unavailable] ✗`);
|
||||
}
|
||||
expect(getAvailable).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("opens when only a session-scoped model is unavailable", async () => {
|
||||
const harness = await createHarness({ models: [{ id: "unavailable", name: "Unavailable" }] });
|
||||
harnesses.push(harness);
|
||||
const model = harness.models[0];
|
||||
const fullId = `${model.provider}/${model.id}`;
|
||||
const { context, getSelector } = createInteractiveContext({
|
||||
allModels: [],
|
||||
enabledModelIds: [],
|
||||
scopedModels: [{ model }],
|
||||
});
|
||||
|
||||
await showModelsSelector(context);
|
||||
|
||||
const selector = getSelector();
|
||||
if (!selector) throw new Error("Expected scoped-model selector to open");
|
||||
expect(stripAnsi(selector.render(100).join("\n"))).toContain(`${fullId} [unavailable] ✗`);
|
||||
});
|
||||
|
||||
it("does not clear a partial scope when an enabled model is unavailable", async () => {
|
||||
const harness = await createHarness({
|
||||
models: [
|
||||
{ id: "one", name: "One" },
|
||||
{ id: "two", name: "Two" },
|
||||
{ id: "three", name: "Three" },
|
||||
],
|
||||
});
|
||||
harnesses.push(harness);
|
||||
const [one, two] = harness.models;
|
||||
const enabledIds = [one, two].map((model) => `${model.provider}/${model.id}`);
|
||||
const unavailableId = `${one.provider}/unavailable`;
|
||||
const { context, getSelector, setScopedModels } = createInteractiveContext({
|
||||
allModels: [...harness.models],
|
||||
enabledModelIds: [...enabledIds, unavailableId],
|
||||
scopedModels: [{ model: one }, { model: two }],
|
||||
});
|
||||
|
||||
await showModelsSelector(context);
|
||||
const selector = getSelector();
|
||||
if (!selector) throw new Error("Expected scoped-model selector to open");
|
||||
selector.handleInput("\x1b[1;3B");
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(setScopedModels).toHaveBeenLastCalledWith([
|
||||
{ model: two, thinkingLevel: undefined },
|
||||
{ model: one, thinkingLevel: undefined },
|
||||
]);
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue