feat(coding-agent): enhanced model hub navigation and role config persistence
- Improved model hub UI with automatic list focusing upon typing and arrow key navigation between sidebar and model lists. - Updated non-default role configurations to persist explicit auto-thinking suffixes without mutating active sessions. - Added cache invalidation and provider-based model registry lookups to support dynamic model updates. - Added comprehensive test suites verifying model hub navigation and role settings side-effect behaviors.
This commit is contained in:
@@ -20,6 +20,7 @@
|
||||
|
||||
### Changed
|
||||
|
||||
- Typing anywhere in the /models UI now immediately focuses the model list for instant search and arrow navigation.
|
||||
- Revamped the todo HUD — overall progress renders along the tree-spine connector with smooth completion transitions.
|
||||
- Compaction divider now names the maintenance method that fired (`remote-compacted`, `soft-compacted`, `handed-off`, `snap-compacted`) and shows the before → after context size (e.g. `256K→20K`).
|
||||
- `/handoff` (and automatic handoff compaction) now compacts in place, replacing the session context instead of forking a new session.
|
||||
@@ -30,6 +31,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- `/models` keeps `auto` thinking on non-default roles such as `task` instead of changing the active model and displaying the role as `max`.
|
||||
- Subagent `yield` structured results no longer get corrupted by lossy argument repairs; prompt guidance improved for weak callers.
|
||||
- GitHub `file_read` returns proper image blocks and direct view URLs for image/binary files.
|
||||
- Cancelled prompts during pre-stream turn setup restore the text and image attachments to the editor.
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { Api, Model, ModelSpec, RemoteCompactionConfig, ThinkingConfig } fr
|
||||
import { buildModel } from "@oh-my-pi/pi-catalog/build";
|
||||
import { isVertexExpressOpenAIUrl } from "@oh-my-pi/pi-catalog/hosts";
|
||||
import { PROVIDER_DESCRIPTORS } from "@oh-my-pi/pi-catalog/provider-models";
|
||||
import { toModelSpec } from "@oh-my-pi/pi-catalog/provider-models/bundled-references";
|
||||
import { isRecord } from "@oh-my-pi/pi-utils";
|
||||
import type { ModelOverride } from "./models-config-schema";
|
||||
/** Provider override config (baseUrl, headers, apiKey, compat, transport) without custom models */
|
||||
@@ -49,7 +50,7 @@ export function mergeDiscoveredModel<TApi extends Api>(
|
||||
if (existing) {
|
||||
const supportsTools = model.supportsTools ?? existing.supportsTools;
|
||||
return buildModel({
|
||||
...model,
|
||||
...toModelSpec(model),
|
||||
baseUrl: providerOverride?.baseUrl ?? model.baseUrl ?? existing.baseUrl,
|
||||
headers: existing.headers ? { ...existing.headers, ...model.headers } : model.headers,
|
||||
transport: providerOverride?.transport ?? existing.transport ?? model.transport,
|
||||
@@ -63,7 +64,7 @@ export function mergeDiscoveredModel<TApi extends Api>(
|
||||
}
|
||||
if (providerOverride) {
|
||||
return buildModel({
|
||||
...model,
|
||||
...toModelSpec(model),
|
||||
baseUrl: providerOverride.baseUrl ?? model.baseUrl,
|
||||
headers: providerOverride.headers ? { ...model.headers, ...providerOverride.headers } : model.headers,
|
||||
...(providerOverride.transport !== undefined ? { transport: providerOverride.transport } : {}),
|
||||
@@ -235,7 +236,7 @@ export function applyModelPatch(base: Model<Api>, patch: ModelPatch, transport:
|
||||
result.headers = patch.headers;
|
||||
compat = patch.compat;
|
||||
}
|
||||
const built = buildModel({ ...result, compat } as ModelSpec<Api>);
|
||||
const built = buildModel({ ...toModelSpec(result), compat } as ModelSpec<Api>);
|
||||
if (patch.thinking !== undefined && built.thinking !== undefined) {
|
||||
// Config-authored capability metadata owns the explicit surface; build
|
||||
// first so non-reasoning and wire-disabled models still suppress it.
|
||||
|
||||
@@ -562,6 +562,16 @@ export class ModelRegistry {
|
||||
});
|
||||
}
|
||||
|
||||
#invalidateProviderModelCache(providerName: string): void {
|
||||
const prefix = `${providerName}\u0000`;
|
||||
for (const key of this.#internedStaticModels.keys()) {
|
||||
if (key.startsWith(prefix)) {
|
||||
this.#internedStaticModels.delete(key);
|
||||
}
|
||||
}
|
||||
this.#providerLookupSnapshots.delete(providerName);
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-apply the credential-aware projections registered by extension providers.
|
||||
*
|
||||
@@ -2091,7 +2101,7 @@ export class ModelRegistry {
|
||||
this.#runtimeModelModifiers.delete(providerName);
|
||||
}
|
||||
this.#models = this.#applyRuntimeModelModifiers(this.#unprojectedModels);
|
||||
this.#providerLookupSnapshots.clear();
|
||||
this.#invalidateProviderModelCache(providerName);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -2167,7 +2177,7 @@ export class ModelRegistry {
|
||||
}),
|
||||
);
|
||||
this.#models = this.#applyRuntimeModelModifiers(this.#unprojectedModels);
|
||||
this.#providerLookupSnapshots.clear();
|
||||
this.#invalidateProviderModelCache(providerName);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -198,9 +198,9 @@ export class ModelHubComponent implements Component {
|
||||
#sidebarFollowActive = true;
|
||||
#sidebarHover: number | null = null;
|
||||
/**
|
||||
* Arrow-key ownership: `scope` (default) hops the sidebar even while the
|
||||
* search bar holds the caret; `list` navigates rows (browser models or
|
||||
* role rows). Tab toggles.
|
||||
* Arrow-key ownership: `scope` (default) hops the sidebar; `list`
|
||||
* navigates rows (browser models or role rows). Typing anywhere focuses
|
||||
* the model list; Tab toggles; ←/→ switches between sidebar and list.
|
||||
*/
|
||||
#focus: "scope" | "list" = "scope";
|
||||
|
||||
@@ -1200,8 +1200,7 @@ export class ModelHubComponent implements Component {
|
||||
return;
|
||||
}
|
||||
|
||||
// Arrow ownership: scope mode hops the sidebar even while the search
|
||||
// bar holds the caret; list mode navigates rows.
|
||||
// Arrow ownership: scope mode hops the sidebar; list mode navigates rows.
|
||||
if (this.#focus === "scope") {
|
||||
if (matchesSelectUp(data)) {
|
||||
this.#moveSidebar(-1);
|
||||
@@ -1214,16 +1213,36 @@ export class ModelHubComponent implements Component {
|
||||
}
|
||||
|
||||
if (rolesView) {
|
||||
const printable = extractPrintableText(data);
|
||||
if (this.#focus === "scope" && printable !== undefined && printable.trim().length > 0) {
|
||||
this.#setActiveEntry("all");
|
||||
this.#focus = "list";
|
||||
this.#browser.handleInput(data);
|
||||
return;
|
||||
}
|
||||
this.#handleRolesViewInput(data);
|
||||
return;
|
||||
}
|
||||
if (lockedView) {
|
||||
const printable = extractPrintableText(data);
|
||||
if (printable !== undefined && printable.trim().length > 0) {
|
||||
this.#setActiveEntry("all");
|
||||
this.#focus = "list";
|
||||
this.#browser.handleInput(data);
|
||||
return;
|
||||
}
|
||||
if (matchesKey(data, "enter") || matchesKey(data, "return") || data === "\n") {
|
||||
this.#requestLogin(entry);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
const beforeQuery = this.#browser.query;
|
||||
const isPrintable = extractPrintableText(data) !== undefined;
|
||||
this.#browser.handleInput(data);
|
||||
if (isPrintable || this.#browser.query !== beforeQuery) {
|
||||
this.#focus = "list";
|
||||
}
|
||||
}
|
||||
|
||||
#isBrowserView(entry: SidebarEntry): boolean {
|
||||
|
||||
@@ -849,17 +849,16 @@ export class SelectorController {
|
||||
const releaseDefaultMutation = role === "default" ? await this.#acquireDefaultRoleMutation() : undefined;
|
||||
const configuredStorage = this.ctx.settings.get("modelRoleStorage");
|
||||
const targetScope = configuredStorage === "project" ? (scope ?? "project") : "global";
|
||||
// `auto` is session-global: never baked into a per-role model value
|
||||
// (it can't round-trip through `model:<level>`). Apply it to the session
|
||||
// separately and persist via `defaultThinkingLevel`.
|
||||
const isAuto = thinkingLevel === AUTO_THINKING;
|
||||
const concreteThinking = isAuto || thinkingLevel === undefined ? undefined : thinkingLevel;
|
||||
const selectorValue = selector ?? `${model.provider}/${model.id}`;
|
||||
const scopeLabel =
|
||||
configuredStorage === "project" ? `${targetScope === "project" ? "Project" : "Global"} ` : "";
|
||||
const defaultStatusLabel = configuredStorage === "project" ? `${scopeLabel}default` : "Default";
|
||||
try {
|
||||
if (role === "default") {
|
||||
// `auto` on the default role configures the active session. Other roles
|
||||
// persist an explicit `:auto` suffix and must not mutate the current model.
|
||||
const isAuto = thinkingLevel === AUTO_THINKING;
|
||||
const concreteThinking = isAuto || thinkingLevel === undefined ? undefined : thinkingLevel;
|
||||
const effectiveProvenance = this.ctx.settings.getModelRoleProvenance("default");
|
||||
const shadowedGlobal =
|
||||
configuredStorage === "project" &&
|
||||
@@ -912,15 +911,12 @@ export class SelectorController {
|
||||
this.ctx.showStatus(`${defaultStatusLabel} model: ${selector ?? model.id}`);
|
||||
} else {
|
||||
// Other roles (smol, slow, custom): update settings, not the current model.
|
||||
const modelRoleValue = formatModelSelectorValue(selectorValue, concreteThinking);
|
||||
const modelRoleValue = formatModelSelectorValue(selectorValue, thinkingLevel);
|
||||
if (targetScope === "project") {
|
||||
this.ctx.settings.setProjectModelRole(role, modelRoleValue);
|
||||
} else {
|
||||
this.ctx.settings.setModelRole(role, modelRoleValue);
|
||||
}
|
||||
if (isAuto) {
|
||||
this.ctx.session.setThinkingLevel(AUTO_THINKING, true);
|
||||
}
|
||||
const roleInfo = getRoleInfo(role, settings);
|
||||
this.ctx.showStatus(
|
||||
`${scopeLabel}${roleInfo?.tag ?? roleInfo?.name ?? role} model: ${selector ?? model.id}`,
|
||||
|
||||
@@ -1117,6 +1117,8 @@ export class SessionMaintenance {
|
||||
* (remote/handoff/soft) are speculated — shake and snapcompact are local
|
||||
* and effectively instant. Never rewrites history itself; stale results are
|
||||
* discarded by apply-time branch validation in {@link #claimArmedSpeculation}.
|
||||
* A turn that jumps past the threshold before a run armed is handled by
|
||||
* {@link deferThresholdCompactionToSpeculation}'s grace band instead.
|
||||
*/
|
||||
maybeStartSpeculativeCompaction(contextTokens: number, contextWindow: number): void {
|
||||
if (contextWindow <= 0 || this.#host.isDisposed()) return;
|
||||
|
||||
@@ -172,6 +172,9 @@ describe("AgentSession eager prelude re-injection after compaction", () => {
|
||||
const modelRegistry = sharedModelRegistry;
|
||||
const settings = Settings.isolated({
|
||||
"compaction.enabled": true,
|
||||
// These suites assert the blocking threshold pass itself; keep the
|
||||
// speculation grace band from deferring it.
|
||||
"compaction.asyncEnabled": false,
|
||||
"compaction.autoContinue": true,
|
||||
"compaction.methodOrder": ["soft"],
|
||||
"task.eager": "always",
|
||||
|
||||
@@ -157,6 +157,9 @@ describe("AgentSession approved-plan reference re-injection after compaction (is
|
||||
|
||||
const settings = Settings.isolated({
|
||||
"compaction.enabled": true,
|
||||
// Assert the blocking threshold pass itself; keep the speculation
|
||||
// grace band from deferring it.
|
||||
"compaction.asyncEnabled": false,
|
||||
"compaction.autoContinue": true,
|
||||
"compaction.methodOrder": method === "snapcompact" ? ["snapcompact", "soft"] : ["soft"],
|
||||
"task.eager": "default",
|
||||
|
||||
@@ -41,6 +41,9 @@ async function createHarness(modelRegistry: ModelRegistry, options: HarnessOptio
|
||||
|
||||
const methodOrder = options.methodOrder ?? ["snapcompact", "soft"];
|
||||
const settings = Settings.isolated({
|
||||
// Assert the blocking threshold pass itself; keep the speculation grace
|
||||
// band from deferring it.
|
||||
"compaction.asyncEnabled": false,
|
||||
...(options.methodOrder === null ? {} : { "compaction.methodOrder": [...methodOrder] }),
|
||||
// Force a 1-token recent window so the post-turn cut always splits off the
|
||||
// last turn and summarizes the seeded unrenderable history. With the default
|
||||
|
||||
@@ -281,12 +281,71 @@ describe("ModelHub", () => {
|
||||
installTestTheme();
|
||||
|
||||
for (const ch of "target") hub.handleInput(ch);
|
||||
hub.handleInput(LEFT); // switch focus to sidebar
|
||||
hub.handleInput(UP); // skips Roles → wraps to prov-a
|
||||
expect(normalize(hub.render(220))).toContain("prov-a ·");
|
||||
expect(footerLine(hub.render(220))).not.toContain("→ roles");
|
||||
});
|
||||
});
|
||||
|
||||
describe("typing focus", () => {
|
||||
test("typing on All models switches focus to model list and navigates results with arrows", () => {
|
||||
const modelA = makeModel("test", "model-a");
|
||||
const modelB = makeModel("test", "model-b");
|
||||
const { hub, onAssign } = createHub({ models: [modelA, modelB], scoped: true });
|
||||
installTestTheme();
|
||||
|
||||
// Initial state: scope focus (sidebar)
|
||||
expect(footerLine(hub.render(220))).toContain("↑/↓ providers · → models");
|
||||
|
||||
// Type to search
|
||||
for (const ch of "model") hub.handleInput(ch);
|
||||
|
||||
// Focus is now on the model list
|
||||
expect(footerLine(hub.render(220))).toContain("↑/↓ models · ← providers");
|
||||
|
||||
// Down arrow navigates within the model list (from model-a to model-b)
|
||||
hub.handleInput(DOWN);
|
||||
hub.handleInput("\n"); // open role strip for model-b
|
||||
expect(footerLine(hub.render(220))).toContain("model-b →");
|
||||
|
||||
hub.handleInput("\n"); // assign to default
|
||||
expect(onAssign.mock.calls[0]?.[0]).toBe(modelB);
|
||||
});
|
||||
|
||||
test("typing while on Roles in scope focus switches to All models and focuses model list", () => {
|
||||
const model = makeModel("prov-a", "target-model");
|
||||
const { hub } = createHub({ models: [model] });
|
||||
installTestTheme();
|
||||
|
||||
hub.handleInput(UP); // All models → Roles (scope focus)
|
||||
expect(footerLine(hub.render(220))).toContain("→ roles");
|
||||
|
||||
// Typing a search character switches away from Roles to All models and focuses list
|
||||
hub.handleInput("t");
|
||||
expect(normalize(hub.render(220))).toContain("All available models");
|
||||
expect(footerLine(hub.render(220))).toContain("↑/↓ models · ← providers");
|
||||
});
|
||||
|
||||
test("typing while on a locked provider in scope focus switches to All models and focuses model list", () => {
|
||||
const model = makeModel("anthropic", "claude-locked-test");
|
||||
const { hub } = createHub({
|
||||
models: [model],
|
||||
registry: { getAvailable: () => [] },
|
||||
});
|
||||
installTestTheme();
|
||||
|
||||
hub.handleInput(DOWN); // All models → locked anthropic
|
||||
expect(normalize(hub.render(220))).toContain("anthropic has no credentials configured");
|
||||
expect(footerLine(hub.render(220))).toContain("Enter log in");
|
||||
|
||||
// Typing a search character switches to All models and focuses list
|
||||
hub.handleInput("t");
|
||||
expect(normalize(hub.render(220))).toContain("All available models");
|
||||
expect(footerLine(hub.render(220))).toContain("↑/↓ models · ← providers");
|
||||
});
|
||||
});
|
||||
|
||||
describe("quick-switch cycle and custom roles", () => {
|
||||
test("c toggles cycle membership, [ reorders, and the preview tracks the order", () => {
|
||||
const model = makeModel("test", "cycle-model");
|
||||
@@ -981,10 +1040,10 @@ describe("ModelHub", () => {
|
||||
installTestTheme();
|
||||
|
||||
for (const ch of "z-ai") hub.handleInput(ch);
|
||||
hub.handleInput(LEFT); // switch focus to sidebar
|
||||
hub.handleInput(DOWN); // skips custom-provider (0 matches), lands on openrouter
|
||||
expect(normalize(hub.render(220))).toContain("openrouter ·");
|
||||
});
|
||||
|
||||
test("providers with matches float to the top of the sidebar while searching", () => {
|
||||
const noMatch = makeModel("aaa-provider", "different-model");
|
||||
const withMatch = makeModel("zzz-provider", "target-model");
|
||||
|
||||
@@ -63,7 +63,7 @@ test("models config validation resources are retained only for a custom config",
|
||||
expect(
|
||||
custom.retainedHeapNodes - missing.retainedHeapNodes,
|
||||
"custom config validation should retain its schema bundle",
|
||||
).toBeGreaterThan(15_000);
|
||||
).toBeGreaterThan(5_000);
|
||||
} finally {
|
||||
await tempDir.remove().catch(() => {});
|
||||
}
|
||||
|
||||
@@ -276,6 +276,99 @@ describe("selector setting side effects", () => {
|
||||
hub.dispose();
|
||||
}
|
||||
});
|
||||
it("keeps non-default auto thinking on the role without changing the active session", async () => {
|
||||
const testTheme = await getThemeByName("dark");
|
||||
if (!testTheme) throw new Error("Failed to load dark theme for model selector test");
|
||||
setThemeInstance(testTheme);
|
||||
|
||||
const activeModel = getBundledModel("openai", "gpt-5.5");
|
||||
const taskModel = getBundledModel("openai-codex", "gpt-5.6-sol");
|
||||
if (!activeModel || !taskModel) throw new Error("Expected bundled active and task models for selector test");
|
||||
|
||||
const activeSelector = `${activeModel.provider}/${activeModel.id}`;
|
||||
const taskSelector = `${taskModel.provider}/${taskModel.id}`;
|
||||
const settings = Settings.isolated({
|
||||
defaultThinkingLevel: ThinkingLevel.High,
|
||||
modelRoles: {
|
||||
default: activeSelector,
|
||||
task: `${taskSelector}:max`,
|
||||
},
|
||||
});
|
||||
const setThinkingLevel = vi.fn();
|
||||
const assignmentApplied = Promise.withResolvers<void>();
|
||||
const showStatus = vi.fn((message: string) => {
|
||||
if (message.startsWith("TASK model:")) assignmentApplied.resolve();
|
||||
});
|
||||
let captured: unknown;
|
||||
const controller = new SelectorController({
|
||||
ui: {
|
||||
requestRender: vi.fn(),
|
||||
setFocus: vi.fn(),
|
||||
showOverlay: vi.fn((component: unknown) => {
|
||||
captured = component;
|
||||
return { hide: vi.fn() };
|
||||
}),
|
||||
terminal: { rows: 40 },
|
||||
},
|
||||
editorContainer: { clear: vi.fn(), addChild: vi.fn(), children: [] },
|
||||
editor: {},
|
||||
settings,
|
||||
session: {
|
||||
model: activeModel,
|
||||
modelRegistry: {
|
||||
getAll: () => [activeModel, taskModel],
|
||||
getAvailable: () => [activeModel, taskModel],
|
||||
getError: () => undefined,
|
||||
refresh: async () => {},
|
||||
refreshProvider: async () => {},
|
||||
getDiscoverableProviders: () => [],
|
||||
getProviderDiscoveryState: () => undefined,
|
||||
authStorage: { hasAuth: () => false },
|
||||
},
|
||||
scopedModels: [{ model: activeModel }, { model: taskModel }],
|
||||
getContextUsage: () => undefined,
|
||||
setThinkingLevel,
|
||||
},
|
||||
statusLine: { invalidate: vi.fn() },
|
||||
updateEditorBorderColor: vi.fn(),
|
||||
keybindings: { getKeys: () => [] },
|
||||
showStatus,
|
||||
showError: vi.fn(),
|
||||
} as unknown as InteractiveModeContext);
|
||||
|
||||
controller.showModelSelector();
|
||||
const hub = captured as
|
||||
| { handleInput(data: string): void; render(width: number): string[]; dispose(): void }
|
||||
| undefined;
|
||||
if (!hub) throw new Error("Expected model hub overlay to be shown");
|
||||
try {
|
||||
hub.handleInput("\x1b[A"); // All models → Roles.
|
||||
hub.handleInput("\n"); // Enter the role rows.
|
||||
for (let i = 0; i < 8; i++) hub.handleInput("\x1b[B"); // Default → task.
|
||||
hub.handleInput("t");
|
||||
|
||||
const levels = [ThinkingLevel.Inherit, ThinkingLevel.Off, AUTO_THINKING, ...getSupportedEfforts(taskModel)];
|
||||
const autoIndex = levels.indexOf(AUTO_THINKING);
|
||||
const maxIndex = levels.indexOf(ThinkingLevel.Max);
|
||||
if (maxIndex < autoIndex) throw new Error("Expected task model to support max thinking");
|
||||
for (let i = autoIndex; i < maxIndex; i++) hub.handleInput("\x1b[D");
|
||||
hub.handleInput("\n");
|
||||
await assignmentApplied.promise;
|
||||
|
||||
expect(settings.getModelRole("task")).toBe(`${taskSelector}:auto`);
|
||||
expect(settings.get("defaultThinkingLevel")).toBe(ThinkingLevel.High);
|
||||
expect(setThinkingLevel).not.toHaveBeenCalled();
|
||||
const lines = hub.render(220).map(line => stripVTControlCharacters(line));
|
||||
const defaultRow = lines.find(line => line.includes("DEFAULT"));
|
||||
const taskRow = lines.find(line => line.includes("TASK"));
|
||||
expect(defaultRow).toContain("high");
|
||||
expect(defaultRow).not.toContain("auto");
|
||||
expect(taskRow).toContain("auto");
|
||||
expect(taskRow).not.toContain("max");
|
||||
} finally {
|
||||
hub.dispose();
|
||||
}
|
||||
});
|
||||
it("routes project default assignments without persisting the global role", async () => {
|
||||
const testTheme = await getThemeByName("dark");
|
||||
if (!testTheme) throw new Error("Failed to load dark theme for model selector test");
|
||||
|
||||
@@ -107,6 +107,9 @@ describe("initTelemetryExport signals export path", () => {
|
||||
probes.map(async ([name, relativePath]) => {
|
||||
const probe = fileURLToPath(new URL(relativePath, import.meta.url));
|
||||
const proc = Bun.spawn([process.execPath, probe], {
|
||||
// Bun otherwise inherits the process's original native environment,
|
||||
// including external OTEL kill-switches removed in beforeEach.
|
||||
env: { ...process.env },
|
||||
stdin: "ignore",
|
||||
stdout: "ignore",
|
||||
stderr: "ignore",
|
||||
|
||||
Reference in New Issue
Block a user