Reviewed and requested changes on PR #837
Public
Reviewed and requested changes on PR #837
Agent
Pi
Model
Claude Opus 4.5 (latest)
Started
Jan 19, 3:01 PM
Messages102
Downloads0
Read PR description, comments, commits, and changed files.
UnderstandIdentified and reviewed linked issues.
UnderstandAnalyzed PR diff and relevant source files.
UnderstandChecked changelog and documentation compliance.
VerifyPosted review comment, added label, and converted PR to draft.
Edit
You are given one or more GitHub PR URLs: https://github.com/badlogic/pi-mono/pull/837
For each PR URL, do the following in order:
packages/*/CHANGELOG.md files. Report whether an entry exists. If missing, state that a changelog entry is required before merge and that you will add it if the user decides to merge. Follow the changelog format rules in AGENTS.md. Verify:
### Breaking Changes, ### Added, ### Fixed, etc.)Fixed foo ([#123](https://github.com/badlogic/pi-mono/pull/123) by [@user](https://github.com/user))### Breaking Changes, not just ### FixedOutput format per PR: PR: Changelog:
If no issues are found, say so under Bad and Ugly.
The user wants me to review a GitHub PR. Let me start by fetching the PR details.
>gh pr view 837 --repo badlogic/pi-mono --json title,body,comments,commits,files,author,state,labels,headRefName,baseRefName{"author":{"id":"MDQ6VXNlcjUyNTg2Mzg=","is_bot":false,"login":"CarlosGtrz","name":"Carlos Gutierrez"},"baseRefName":"main","body":"Some improvements to `/scoped-models` vibecoded with gpt 5.2. Take it as feature requests if too much code is changed.\r\n\r\n- Add Ctrl+R reset to restore the initial model selection\r\n- Add Ctrl+T per-model thinking overrides, persisted via :level suffix\r\n- Include provider name in Ctrl+P model cycling status\r\n- Show ✓ for enabled models even when all models are enabled","comments":[],"commits":[{"authoredDate":"2026-01-19T03:57:45Z","authors":[{"email":"[REDACTED]","id":"MDQ6VXNlcjUyNTg2Mzg=","login":"CarlosGtrz","name":"Carlos Gutierrez"}],"committedDate":"2026-01-19T05:26:01Z","messageBody":"- Add Ctrl+R reset to restore the initial model selection\n- Add Ctrl+T per-model thinking overrides, persisted via :level suffix\n- Include provider name in Ctrl+P model cycling status\n- Show ✓ for enabled models even when all models are enabled","messageHeadline":"feat(coding-agent): improve /scoped-models cycling UX","oid":"4e37ea9b0229c25eda8f03474503e4258acf54a9"}],"files":[{"path":"packages/coding-agent/CHANGELOG.md","additions":9,"deletions":0},{"path":"packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts","additions":113,"deletions":33},{"path":"packages/coding-agent/src/modes/interactive/interactive-mode.ts","additions":59,"deletions":63},{"path":"packages/coding-agent/test/scoped-models-selector.test.ts","additions":119,"deletions":0}],"headRefName":"feature/scoped-models-improvements","labels":[],"state":"OPEN","title":"feat(coding-agent): improve /scoped-models cycling UX"}>gh pr diff 837 --repo badlogic/pi-monodiff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md
index c020d3bc1..65454fd31 100644
--- a/packages/coding-agent/CHANGELOG.md
+++ b/packages/coding-agent/CHANGELOG.md
@@ -2,6 +2,15 @@
## [Unreleased]
+### Added
+
+- `/scoped-models`: Ctrl+R resets to the initial model selection, and Ctrl+T cycles per-model thinking overrides (persisted via `:level` suffixes).
+
+### Changed
+
+- Ctrl+P model cycling status now includes the provider name.
+- `/scoped-models` now shows ✓ for enabled models even when all models are enabled.
+
## [0.49.1] - 2026-01-18
### Added
diff --git a/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts b/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts
index 3895ddee0..83df7fa97 100644
--- a/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts
+++ b/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts
@@ -1,4 +1,5 @@
-import type { Model } from "@mariozechner/pi-ai";
+import type { ThinkingLevel } from "@mariozechner/pi-agent-core";
+import { type Api, type Model, supportsXhigh } from "@mariozechner/pi-ai";
import {
Container,
type Focusable,
@@ -16,6 +17,9 @@ import { DynamicBorder } from "./dynamic-border.js";
// EnabledIds: null = all enabled (no filter), string[] = explicit ordered list
type EnabledIds = string[] | null;
+/** Stores only non-default overrides (no entry = off/default). */
+type ThinkingOverrides = Map<string, ThinkingLevel>;
+
function isEnabled(enabledIds: EnabledIds, id: string): boolean {
return enabledIds === null || enabledIds.includes(id);
}
@@ -34,7 +38,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;
}
function clearAll(enabledIds: EnabledIds, allIds: string[], targetIds?: string[]): EnabledIds {
@@ -62,30 +66,30 @@ function getSortedIds(enabledIds: EnabledIds, allIds: string[]): string[] {
return [...enabledIds, ...allIds.filter((id) => !enabledSet.has(id))];
}
+function getThinkingCycleLevels(model: Model<Api>): ThinkingLevel[] {
+ if (!model.reasoning) return [];
+ return supportsXhigh(model) ? ["minimal", "low", "medium", "high", "xhigh"] : ["minimal", "low", "medium", "high"];
+}
+
interface ModelItem {
fullId: string;
- model: Model<any>;
+ model: Model<Api>;
enabled: boolean;
+ thinkingOverride?: ThinkingLevel;
}
export interface ModelsConfig {
- allModels: Model<any>[];
- enabledModelIds: Set<string>;
- /** true if enabledModels setting is defined (empty = all enabled) */
- hasEnabledModelsFilter: boolean;
+ allModels: Model<Api>[];
+ enabledIds: EnabledIds;
+ /** Non-default overrides only (no entry = off/default). */
+ thinkingOverrides?: ThinkingOverrides;
}
export interface ModelsCallbacks {
- /** Called when a model is toggled (session-only, no persist) */
- onModelToggle: (modelId: string, enabled: boolean) => void;
- /** Called when user wants to persist current selection to settings */
- onPersist: (enabledModelIds: string[]) => void;
- /** Called when user enables all models. Returns list of all model IDs. */
- onEnableAll: (allModelIds: string[]) => void;
- /** Called when user clears all models */
- onClearAll: () => void;
- /** Called when user toggles all models for a provider. Returns affected model IDs. */
- onToggleProvider: (provider: string, modelIds: string[], enabled: boolean) => void;
+ /** Called whenever the in-memory selection changes (session-only). */
+ onChange: (patterns: string[] | null) => void;
+ /** Called when user wants to persist current selection to settings (Ctrl+S). */
+ onPersist: (patterns: string[] | null) => void;
onCancel: () => void;
}
@@ -94,13 +98,17 @@ export interface ModelsCallbacks {
* Changes are session-only until explicitly persisted with Ctrl+S.
*/
export class ScopedModelsSelectorComponent extends Container implements Focusable {
- private modelsById: Map<string, Model<any>> = new Map();
+ private modelsById: Map<string, Model<Api>> = new Map();
private allIds: string[] = [];
private enabledIds: EnabledIds = null;
+ private thinkingOverrides: ThinkingOverrides = new Map();
private filteredItems: ModelItem[] = [];
private selectedIndex = 0;
private searchInput: Input;
+ private readonly initialEnabledIds: EnabledIds;
+ private readonly initialThinkingOverrides: ThinkingOverrides;
+
// Focusable implementation - propagate to searchInput for IME cursor positioning
private _focused = false;
get focused(): boolean {
@@ -110,6 +118,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
this._focused = value;
this.searchInput.focused = value;
}
+
private listContainer: Container;
private footerText: Text;
private callbacks: ModelsCallbacks;
@@ -126,7 +135,12 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
this.allIds.push(fullId);
}
- this.enabledIds = config.hasEnabledModelsFilter ? [...config.enabledModelIds] : null;
+ this.enabledIds = config.enabledIds;
+ this.thinkingOverrides = new Map(config.thinkingOverrides ?? []);
+
+ this.initialEnabledIds = this.enabledIds === null ? null : [...this.enabledIds];
+ this.initialThinkingOverrides = new Map(this.thinkingOverrides);
+
this.filteredItems = this.buildItems();
// Header
@@ -154,11 +168,25 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
this.updateList();
}
+ private buildPatterns(): string[] | null {
+ // null = no filter (all models enabled). In this mode, thinking overrides are not active/persisted.
+ if (this.enabledIds === null) return null;
+ return this.enabledIds.map((id) => {
+ const override = this.thinkingOverrides.get(id);
+ return override ? `${id}:${override}` : id;
+ });
+ }
+
+ private emitChange(): void {
+ this.callbacks.onChange(this.buildPatterns());
+ }
+
private buildItems(): ModelItem[] {
return getSortedIds(this.enabledIds, this.allIds).map((id) => ({
fullId: id,
model: this.modelsById.get(id)!,
enabled: isEnabled(this.enabledIds, id),
+ thinkingOverride: this.thinkingOverrides.get(id),
}));
}
@@ -166,7 +194,17 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
const enabledCount = this.enabledIds?.length ?? this.allIds.length;
const allEnabled = this.enabledIds === null;
const countText = allEnabled ? "all enabled" : `${enabledCount}/${this.allIds.length} enabled`;
- const parts = ["Enter toggle", "^A all", "^X clear", "^P provider", "Alt+↑↓ reorder", "^S save", countText];
+ const parts = [
+ "Enter toggle",
+ "^A all",
+ "^X clear",
+ "^P provider",
+ "^T thinking",
+ "^R reset",
+ "Alt+↑↓ reorder",
+ "^S save",
+ countText,
+ ];
return this.isDirty
? theme.fg("dim", ` ${parts.join(" · ")} `) + theme.fg("warning", "(unsaved)")
: theme.fg("dim", ` ${parts.join(" · ")}`);
@@ -194,15 +232,18 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
Math.min(this.selectedIndex - Math.floor(this.maxVisible / 2), this.filteredItems.length - this.maxVisible),
);
const endIndex = Math.min(startIndex + this.maxVisible, this.filteredItems.length);
- const allEnabled = this.enabledIds === null;
for (let i = startIndex; i < endIndex; i++) {
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 thinkingSuffix = item.enabled && item.thinkingOverride ? `:${item.thinkingOverride}` : "";
+ const displayId = `${item.model.id}${thinkingSuffix}`;
+ const modelText = isSelected ? theme.fg("accent", displayId) : displayId;
const providerBadge = theme.fg("muted", ` [${item.model.provider}]`);
- const status = allEnabled ? "" : item.enabled ? theme.fg("success", " ✓") : theme.fg("dim", " ✗");
+ const status = item.enabled ? theme.fg("success", " ✓") : theme.fg("dim", " ✗");
+
this.listContainer.addChild(new Text(`${prefix}${modelText}${providerBadge}${status}`, 0, 0));
}
@@ -244,6 +285,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
this.enabledIds = move(this.enabledIds, this.allIds, item.fullId, delta);
this.isDirty = true;
this.selectedIndex += delta;
+ this.emitChange();
this.refresh();
}
}
@@ -254,22 +296,60 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
if (matchesKey(data, Key.enter)) {
const item = this.filteredItems[this.selectedIndex];
if (item) {
- const wasAllEnabled = this.enabledIds === null;
this.enabledIds = toggle(this.enabledIds, item.fullId);
this.isDirty = true;
- if (wasAllEnabled) this.callbacks.onClearAll();
- this.callbacks.onModelToggle(item.fullId, isEnabled(this.enabledIds, item.fullId));
+ this.emitChange();
this.refresh();
}
return;
}
- // Ctrl+A - Enable all (filtered if search active, otherwise all)
+ // Ctrl+T - Cycle per-model thinking override (selected models only)
+ if (matchesKey(data, Key.ctrl("t"))) {
+ const item = this.filteredItems[this.selectedIndex];
+ if (!item) return;
+ // Only for enabled models when a filter is active (not the default "all enabled" mode)
+ if (this.enabledIds === null || !item.enabled) return;
+
+ const levels = getThinkingCycleLevels(item.model);
+ if (levels.length === 0) return;
+
+ const current = this.thinkingOverrides.get(item.fullId);
+ let next: ThinkingLevel | undefined;
+ if (!current) {
+ next = levels[0];
+ } else {
+ const idx = levels.indexOf(current);
+ next = idx === -1 || idx === levels.length - 1 ? undefined : levels[idx + 1];
+ }
+
+ if (next) {
+ this.thinkingOverrides.set(item.fullId, next);
+ } else {
+ this.thinkingOverrides.delete(item.fullId);
+ }
+ this.isDirty = true;
+ this.emitChange();
+ this.refresh();
+ return;
+ }
+
+ // Ctrl+R - Reset to initial state
+ if (matchesKey(data, Key.ctrl("r"))) {
+ this.enabledIds = this.initialEnabledIds === null ? null : [...this.initialEnabledIds];
+ this.thinkingOverrides = new Map(this.initialThinkingOverrides);
+ this.isDirty = false;
+ this.emitChange();
+ this.refresh();
+ return;
+ }
+
+ // Ctrl+A - All enabled (clear filter)
if (matchesKey(data, Key.ctrl("a"))) {
- const targetIds = this.searchInput.getValue() ? this.filteredItems.map((i) => i.fullId) : undefined;
- this.enabledIds = enableAll(this.enabledIds, this.allIds, targetIds);
+ this.enabledIds = null;
+ this.thinkingOverrides.clear();
this.isDirty = true;
- this.callbacks.onEnableAll(targetIds ?? this.allIds);
+ this.emitChange();
this.refresh();
return;
}
@@ -279,7 +359,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
const targetIds = this.searchInput.getValue() ? this.filteredItems.map((i) => i.fullId) : undefined;
this.enabledIds = clearAll(this.enabledIds, this.allIds, targetIds);
this.isDirty = true;
- this.callbacks.onClearAll();
+ this.emitChange();
this.refresh();
return;
}
@@ -295,7 +375,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
? clearAll(this.enabledIds, this.allIds, providerIds)
: enableAll(this.enabledIds, this.allIds, providerIds);
this.isDirty = true;
- this.callbacks.onToggleProvider(provider, providerIds, !allEnabled);
+ this.emitChange();
this.refresh();
}
return;
@@ -303,7 +383,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
// Ctrl+S - Save/persist to settings
if (matchesKey(data, Key.ctrl("s"))) {
- this.callbacks.onPersist(this.enabledIds ?? [...this.allIds]);
+ this.callbacks.onPersist(this.buildPatterns());
this.isDirty = false;
this.footerText.setText(this.getFooterText());
return;
diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts
index e4e030fae..e72071ab3 100644
--- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts
+++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts
@@ -7,7 +7,7 @@ import * as crypto from "node:crypto";
import * as fs from "node:fs";
import * as os from "node:os";
import * as path from "node:path";
-import type { AgentMessage } from "@mariozechner/pi-agent-core";
+import type { AgentMessage, ThinkingLevel } from "@mariozechner/pi-agent-core";
import {
type AssistantMessage,
getOAuthProviders,
@@ -2244,7 +2244,9 @@ export class InteractiveMode {
this.updateEditorBorderColor();
const thinkingStr =
result.model.reasoning && result.thinkingLevel !== "off" ? ` (thinking: ${result.thinkingLevel})` : "";
- this.showStatus(`Switched to ${result.model.name || result.model.id}${thinkingStr}`);
+ this.showStatus(
+ `Switched to ${result.model.name || result.model.id}${thinkingStr} [${result.model.provider}]`,
+ );
}
} catch (error) {
this.showError(error instanceof Error ? error.message : String(error));
@@ -2746,46 +2748,75 @@ export class InteractiveMode {
const sessionScopedModels = this.session.scopedModels;
const hasSessionScope = sessionScopedModels.length > 0;
- // Build enabled model IDs from session state or settings
- const enabledModelIds = new Set<string>();
- let hasFilter = false;
+ let enabledIds: string[] | null = null;
+ const thinkingOverrides = new Map<string, ThinkingLevel>();
if (hasSessionScope) {
- // Use current session's scoped models
- for (const sm of sessionScopedModels) {
- enabledModelIds.add(`${sm.model.provider}/${sm.model.id}`);
+ // Prefer extracting explicit thinking overrides from settings patterns when the session scope
+ // matches the settings scope. This preserves explicit :level suffixes even if the level equals
+ // the current session default thinking level.
+ const settingsPatterns = this.settingsManager.getEnabledModels();
+ if (settingsPatterns !== undefined && settingsPatterns.length > 0) {
+ const scopedFromSettings = await resolveModelScope(settingsPatterns, this.session.modelRegistry);
+ const settingsIds = scopedFromSettings.map((sm) => `${sm.model.provider}/${sm.model.id}`);
+ const sessionIds = sessionScopedModels.map((sm) => `${sm.model.provider}/${sm.model.id}`);
+ const matchesSettingsScope =
+ sessionIds.length === settingsIds.length && sessionIds.every((id, i) => id === settingsIds[i]);
+
+ if (matchesSettingsScope) {
+ enabledIds = settingsIds;
+ for (const sm of scopedFromSettings) {
+ const fullId = `${sm.model.provider}/${sm.model.id}`;
+ if (sm.thinkingLevel && sm.thinkingLevel !== "off") {
+ thinkingOverrides.set(fullId, sm.thinkingLevel);
+ }
+ }
+ } else {
+ enabledIds = sessionIds;
+ for (const sm of sessionScopedModels) {
+ const fullId = `${sm.model.provider}/${sm.model.id}`;
+ if (sm.thinkingLevel !== "off") {
+ thinkingOverrides.set(fullId, sm.thinkingLevel);
+ }
+ }
+ }
+ } else {
+ enabledIds = sessionScopedModels.map((sm) => `${sm.model.provider}/${sm.model.id}`);
+ for (const sm of sessionScopedModels) {
+ const fullId = `${sm.model.provider}/${sm.model.id}`;
+ if (sm.thinkingLevel !== "off") {
+ thinkingOverrides.set(fullId, sm.thinkingLevel);
+ }
+ }
}
- hasFilter = true;
} else {
// Fall back to settings
const patterns = this.settingsManager.getEnabledModels();
if (patterns !== undefined && patterns.length > 0) {
- hasFilter = true;
const scopedModels = await resolveModelScope(patterns, this.session.modelRegistry);
+ enabledIds = scopedModels.map((sm) => `${sm.model.provider}/${sm.model.id}`);
for (const sm of scopedModels) {
- enabledModelIds.add(`${sm.model.provider}/${sm.model.id}`);
+ const fullId = `${sm.model.provider}/${sm.model.id}`;
+ if (sm.thinkingLevel && sm.thinkingLevel !== "off") {
+ thinkingOverrides.set(fullId, sm.thinkingLevel);
+ }
}
}
}
- // Track current enabled state (session-only until persisted)
- const currentEnabledIds = new Set(enabledModelIds);
- let currentHasFilter = hasFilter;
-
- // Helper to update session's scoped models (session-only, no persist)
- const updateSessionModels = async (enabledIds: Set<string>) => {
- if (enabledIds.size > 0 && enabledIds.size < allModels.length) {
- // Use current session thinking level, not settings default
+ const applySessionScope = async (patterns: string[] | null) => {
+ if (patterns && patterns.length > 0) {
+ // Use current session thinking level as default for models without explicit thinking suffix
const currentThinkingLevel = this.session.thinkingLevel;
- const newScopedModels = await resolveModelScope(Array.from(enabledIds), this.session.modelRegistry);
+ const scopedModels = await resolveModelScope(patterns, this.session.modelRegistry);
this.session.setScopedModels(
- newScopedModels.map((sm) => ({
+ scopedModels.map((sm) => ({
model: sm.model,
thinkingLevel: sm.thinkingLevel ?? currentThinkingLevel,
})),
);
} else {
- // All enabled or none enabled = no filter
+ // No models enabled = clear scope
this.session.setScopedModels([]);
}
};
@@ -2794,50 +2825,15 @@ export class InteractiveMode {
const selector = new ScopedModelsSelectorComponent(
{
allModels,
- enabledModelIds: currentEnabledIds,
- hasEnabledModelsFilter: currentHasFilter,
+ enabledIds,
+ thinkingOverrides,
},
{
- onModelToggle: async (modelId, enabled) => {
- if (enabled) {
- currentEnabledIds.add(modelId);
- } else {
- currentEnabledIds.delete(modelId);
- }
- currentHasFilter = true;
- await updateSessionModels(currentEnabledIds);
- },
- onEnableAll: async (allModelIds) => {
- currentEnabledIds.clear();
- for (const id of allModelIds) {
- currentEnabledIds.add(id);
- }
- currentHasFilter = false;
- await updateSessionModels(currentEnabledIds);
- },
- onClearAll: async () => {
- currentEnabledIds.clear();
- currentHasFilter = true;
- await updateSessionModels(currentEnabledIds);
- },
- onToggleProvider: async (_provider, modelIds, enabled) => {
- for (const id of modelIds) {
- if (enabled) {
- currentEnabledIds.add(id);
- } else {
- currentEnabledIds.delete(id);
- }
- }
- currentHasFilter = true;
- await updateSessionModels(currentEnabledIds);
+ onChange: async (patterns) => {
+ await applySessionScope(patterns);
},
- onPersist: (enabledIds) => {
- // Persist to settings
- const newPatterns =
- enabledIds.length === allModels.length
- ? undefined // All enabled = clear filter
- : enabledIds;
- this.settingsManager.setEnabledModels(newPatterns);
+ onPersist: (patterns) => {
+ this.settingsManager.setEnabledModels(patterns === null ? undefined : patterns);
this.showStatus("Model selection saved to settings");
},
onCancel: () => {
diff --git a/packages/coding-agent/test/scoped-models-selector.test.ts b/packages/coding-agent/test/scoped-models-selector.test.ts
new file mode 100644
index 000000000..0c6553b2d
--- /dev/null
+++ b/packages/coding-agent/test/scoped-models-selector.test.ts
@@ -0,0 +1,119 @@
+import type { Model } from "@mariozechner/pi-ai";
+import { beforeAll, describe, expect, test, vi } from "vitest";
+import { ScopedModelsSelectorComponent } from "../src/modes/interactive/components/scoped-models-selector.js";
+import { initTheme } from "../src/modes/interactive/theme/theme.js";
+
+const mockModels: Model<"anthropic-messages">[] = [
+ {
+ id: "claude-sonnet-4-5",
+ name: "Claude Sonnet 4.5",
+ api: "anthropic-messages",
+ provider: "anthropic",
+ baseUrl: "https://api.anthropic.com",
+ reasoning: true,
+ input: ["text"],
+ cost: { input: 3, output: 15, cacheRead: 0.3, cacheWrite: 3.75 },
+ contextWindow: 200000,
+ maxTokens: 8192,
+ },
+ {
+ id: "gpt-4o",
+ name: "GPT-4o",
+ api: "anthropic-messages",
+ provider: "openai",
+ baseUrl: "https://api.openai.com",
+ reasoning: true,
+ input: ["text"],
+ cost: { input: 5, output: 15, cacheRead: 0.5, cacheWrite: 5 },
+ contextWindow: 128000,
+ maxTokens: 4096,
+ },
+];
+
+function renderAll(selector: ScopedModelsSelectorComponent, width = 120): string {
+ return selector.render(width).join("\n");
+}
+
+describe("ScopedModelsSelectorComponent", () => {
+ beforeAll(() => {
+ initTheme("dark");
+ });
+
+ test("shows ✓ for enabled models even when all models are enabled", () => {
+ const selector = new ScopedModelsSelectorComponent(
+ { allModels: mockModels, enabledIds: null },
+ { onChange: vi.fn(), onPersist: vi.fn(), onCancel: vi.fn() },
+ );
+
+ const out = renderAll(selector);
+ expect(out).toContain("claude-sonnet-4-5");
+ expect(out).toContain("gpt-4o");
+ expect(out).toMatch(/✓/);
+ });
+
+ test("Ctrl+T cycles thinking overrides and shows :level suffix", () => {
+ const onChange = vi.fn();
+ const selector = new ScopedModelsSelectorComponent(
+ { allModels: mockModels, enabledIds: ["anthropic/claude-sonnet-4-5"] },
+ { onChange, onPersist: vi.fn(), onCancel: vi.fn() },
+ );
+
+ // Ctrl+T (\x14) should set first override to :minimal
+ selector.handleInput("\x14");
+ let out = renderAll(selector);
+ expect(out).toContain("claude-sonnet-4-5:minimal");
+
+ // Next cycle -> :low
+ selector.handleInput("\x14");
+ out = renderAll(selector);
+ expect(out).toContain("claude-sonnet-4-5:low");
+
+ // Ensure it emitted at least one change
+ expect(onChange).toHaveBeenCalled();
+ });
+
+ test("Ctrl+T is ignored when all models are enabled (no filter)", () => {
+ const selector = new ScopedModelsSelectorComponent(
+ { allModels: mockModels, enabledIds: null },
+ { onChange: vi.fn(), onPersist: vi.fn(), onCancel: vi.fn() },
+ );
+
+ selector.handleInput("\x14");
+ const out = renderAll(selector);
+ expect(out).not.toContain(":minimal");
+ });
+
+ test("Ctrl+T works when all models are explicitly selected", () => {
+ const selector = new ScopedModelsSelectorComponent(
+ { allModels: mockModels, enabledIds: ["anthropic/claude-sonnet-4-5", "openai/gpt-4o"] },
+ { onChange: vi.fn(), onPersist: vi.fn(), onCancel: vi.fn() },
+ );
+
+ selector.handleInput("\x14");
+ const out = renderAll(selector);
+ expect(out).toContain("claude-sonnet-4-5:minimal");
+ });
+
+ test("Ctrl+R resets selection and thinking overrides", () => {
+ const selector = new ScopedModelsSelectorComponent(
+ { allModels: mockModels, enabledIds: ["anthropic/claude-sonnet-4-5"] },
+ { onChange: vi.fn(), onPersist: vi.fn(), onCancel: vi.fn() },
+ );
+
+ // Set override
+ selector.handleInput("\x14"); // Ctrl+T
+ let out = renderAll(selector);
+ expect(out).toContain("claude-sonnet-4-5:minimal");
+
+ // Toggle off the model (Enter = \r)
+ selector.handleInput("\r");
+ out = renderAll(selector);
+ expect(out).toMatch(/claude-sonnet-4-5/);
+
+ // Reset (Ctrl+R = \x12)
+ selector.handleInput("\x12");
+ out = renderAll(selector);
+ expect(out).toContain("claude-sonnet-4-5");
+ expect(out).not.toContain("claude-sonnet-4-5:minimal");
+ });
+});Good, I have the PR details and diff. Let me read the relevant source files to understand the changes better. Let me also check if there are any linked issues.
From the PR, there are no linked issues mentioned. Let me read the source files to review the changes properly.
Now let me read the relevant source files to understand the context:
Let me read more of the interactive-mode.ts to understand the context better. I need to check the parts that were changed in the PR, specifically around the showModelsSelector function and the cycle model function.
Let me read more of the interactive-mode.ts file to see the cycle model functionality:
Let me check where supportsXhigh is exported from. The diff shows it's imported from @mariozechner/pi-ai. Let me search for it.
>cd /Users/badlogic/workspaces/pi-mono && grep -rn "supportsXhigh\|supports-xhigh\|supportsXhigh" packages/ai/src --include="*.ts" | head -20packages/ai/src/stream.ts:4:import { supportsXhigh } from "./models.js";
packages/ai/src/stream.ts:315: reasoningEffort: supportsXhigh(model) ? options?.reasoning : clampReasoning(options?.reasoning),
packages/ai/src/stream.ts:321: reasoningEffort: supportsXhigh(model) ? options?.reasoning : clampReasoning(options?.reasoning),
packages/ai/src/stream.ts:327: reasoningEffort: supportsXhigh(model) ? options?.reasoning : clampReasoning(options?.reasoning),
packages/ai/src/models.ts:55:export function supportsXhigh<TApi extends Api>(model: Model<TApi>): boolean {>cd /Users/badlogic/workspaces/pi-mono && grep -n "export.*supportsXhigh" packages/ai/src/**/*.ts 2>/dev/null || grep -rn "supportsXhigh" packages/ai/src --include="*.ts" | head -20packages/ai/src/stream.ts:4:import { supportsXhigh } from "./models.js";
packages/ai/src/stream.ts:315: reasoningEffort: supportsXhigh(model) ? options?.reasoning : clampReasoning(options?.reasoning),
packages/ai/src/stream.ts:321: reasoningEffort: supportsXhigh(model) ? options?.reasoning : clampReasoning(options?.reasoning),
packages/ai/src/stream.ts:327: reasoningEffort: supportsXhigh(model) ? options?.reasoning : clampReasoning(options?.reasoning),
packages/ai/src/models.ts:55:export function supportsXhigh<TApi extends Api>(model: Model<TApi>): boolean {Good, now let me check if supportsXhigh is exported from the index of @mariozechner/pi-ai.
>cd /Users/badlogic/workspaces/pi-mono && grep -n "supportsXhigh" packages/ai/src/index.ts(no output)
Command exited with code 1I see, supportsXhigh is not exported from the index, so the PR's imports may not work. Let me check the actual index exports:
>cd /Users/badlogic/workspaces/pi-mono && grep "supportsXhigh" packages/ai/src/index.ts || echo "Not in index.ts"Not in index.tsGood, export * from "./models.js"; means it will export everything from models.ts including supportsXhigh. So the import should work.
Now let me look at the test file added to understand what tests exist:
The test file doesn't exist on main yet, it's only in the PR. Let me check if the docs need to be updated.
>cd /Users/badlogic/workspaces/pi-mono && grep -rn "scoped-models\|Ctrl\+P\|Ctrl\+R\|Ctrl\+T" packages/coding-agent/README.md packages/coding-agent/docs/*.md 2>/dev/null | head -30packages/coding-agent/README.md:303:| `/scoped-models` | Enable/disable models for Ctrl+P cycling |Let me check the README and docs to see if Ctrl+T and Ctrl+R are documented anywhere for the /scoped-models command.
>cd /Users/badlogic/workspaces/pi-mono && grep -B2 -A5 "scoped-models" packages/coding-agent/README.md| `/settings` | Open settings menu (thinking, theme, message delivery modes, toggles) |
| `/model` | Switch models mid-session. Use `/model <search>` or `provider/model` to prefilter/disambiguate. |
| `/scoped-models` | Enable/disable models for Ctrl+P cycling |
| `/export [file]` | Export session to self-contained HTML |
| `/share` | Upload session as secret GitHub gist, get shareable URL (requires `gh` CLI) |
| `/session` | Show session info: path, message counts, token usage, cost |
| `/name <name>` | Set session display name (shown in session selector) |
| `/hotkeys` | Show all keyboard shortcuts |>cd /Users/badlogic/workspaces/pi-mono && ls packages/coding-agent/docs/compaction.md
extensions.md
rpc.md
sdk.md
session.md
skills.md
theme.md
tree.md
tui.mdLet me also verify if there are pkg labels expected for the PR:
>cd /Users/badlogic/workspaces/pi-mono && grep -rn "enabledModels\|scoped-models\|:level" packages/coding-agent/README.md packages/coding-agent/docs/*.md | head -20packages/coding-agent/README.md:303:| `/scoped-models` | Enable/disable models for Ctrl+P cycling |
packages/coding-agent/README.md:782: "enabledModels": ["anthropic/*", "*gpt*", "gemini-2.5-pro:high"],
packages/coding-agent/README.md:820:| `enabledModels` | Model patterns for cycling. Supports glob patterns (`github-copilot/*`, `*sonnet*`) and fuzzy matching. Same as `--models` CLI flag | - |The README already mentions the :level suffix format. Let me verify the tests would pass:
>cd /Users/badlogic/workspaces/pi-mono && npm run check 2>&1 | tail -50> pi-monorepo@0.0.3 check
> biome check --write . && tsgo --noEmit && npm run check -w @mariozechner/pi-web-ui
Checked 408 files in 249ms. No fixes applied.
packages/coding-agent/src/core/sdk.ts(644,48): error TS2304: Cannot find name 'currentModel'.
packages/coding-agent/src/core/sdk.ts(647,37): error TS2304: Cannot find name 'currentModel'.
packages/coding-agent/src/core/sdk.ts(649,23): error TS2304: Cannot find name 'currentModel'.
packages/coding-agent/src/core/sdk.ts(653,31): error TS2304: Cannot find name 'currentModel'.
packages/coding-agent/src/core/sdk.ts(654,61): error TS2304: Cannot find name 'currentModel'.There are type errors in the current main branch. But I should analyze the PR changes to determine if they pass the type check. Let me see if this PR's changes compile by checking the diff more carefully.
The key changes in the PR are:
scoped-models-selector.ts - new logic for Ctrl+R reset, Ctrl+T thinking overridesinteractive-mode.ts - simplified callbacks, includes provider name in Ctrl+P statusscoped-models-selector.test.tsLet me check if there are any obvious issues with the code:
>cd /Users/badlogic/workspaces/pi-mono && grep -n "ThinkingLevel" packages/agent-core/src/types.ts | head -10grep: packages/agent-core/src/types.ts: No such file or directoryLet me find the ThinkingLevel type definition:
>cd /Users/badlogic/workspaces/pi-mono && grep -rn "ThinkingLevel" packages/agent-core/src --include="*.ts" | head -5(no output)Let me search for where ThinkingLevel is defined:
>cd /Users/badlogic/workspaces/pi-mono && grep -rn "type ThinkingLevel\|ThinkingLevel =" packages --include="*.ts" | head -10packages/coding-agent/test/settings-manager.test.ts:97: currentSettings.defaultThinkingLevel = "low";
packages/coding-agent/test/session-manager/build-context.test.ts:9: type ThinkingLevelChangeEntry,
packages/coding-agent/test/compaction.test.ts:25: type ThinkingLevelChangeEntry,
packages/coding-agent/test/compaction-thinking-model.test.ts:14:import { Agent, type ThinkingLevel } from "@mariozechner/pi-agent-core";
packages/coding-agent/test/compaction-thinking-model.test.ts:55: thinkingLevel: ThinkingLevel = "high",
packages/coding-agent/test/compaction-thinking-model.test.ts:157: function createSession(model: Model<any>, thinkingLevel: ThinkingLevel = "high") {
packages/coding-agent/dist/core/sdk.d.ts:22:import { type ThinkingLevel } from "@mariozechner/pi-agent-core";
packages/coding-agent/dist/index.d.ts:12:export { type BranchSummaryEntry, buildSessionContext, type CompactionEntry, CURRENT_SESSION_VERSION, type CustomEntry, type CustomMessageEntry, type FileEntry, getLatestCompactionEntry, type ModelChangeEntry, migrateSessionEntries, type NewSessionOptions, parseSessionEntries, type SessionContext, type SessionEntry, type SessionEntryBase, type SessionHeader, type SessionInfo, type SessionInfoEntry, SessionManager, type SessionMessageEntry, type ThinkingLevelChangeEntry, } from "./core/session-manager.js";
packages/coding-agent/src/core/model-resolver.ts:275: let thinkingLevel: ThinkingLevel = "off";
packages/coding-agent/src/core/settings-manager.ts:277: this.globalSettings.defaultThinkingLevel = level;Let me check where ThinkingLevel is defined and imported from:
>cd /Users/badlogic/workspaces/pi-mono && grep -rn "export.*ThinkingLevel\|^export type ThinkingLevel" packages --include="*.ts" | head -10packages/coding-agent/dist/core/extensions/types.d.ts:618:export type GetThinkingLevelHandler = () => ThinkingLevel;
packages/coding-agent/dist/core/extensions/types.d.ts:619:export type SetThinkingLevelHandler = (level: ThinkingLevel) => void;
packages/coding-agent/dist/core/extensions/index.d.ts:7:export type { AgentEndEvent, AgentStartEvent, AgentToolResult, AgentToolUpdateCallback, AppAction, AppendEntryHandler, BashToolResultEvent, BeforeAgentStartEvent, BeforeAgentStartEventResult, CompactOptions, ContextEvent, ContextEventResult, ContextUsage, CustomToolResultEvent, EditToolResultEvent, ExecOptions, ExecResult, Extension, ExtensionActions, ExtensionAPI, ExtensionCommandContext, ExtensionCommandContextActions, ExtensionContext, ExtensionContextActions, ExtensionError, ExtensionEvent, ExtensionFactory, ExtensionFlag, ExtensionHandler, ExtensionRuntime, ExtensionShortcut, ExtensionUIContext, ExtensionUIDialogOptions, FindToolResultEvent, GetActiveToolsHandler, GetAllToolsHandler, GetThinkingLevelHandler, GrepToolResultEvent, InputEvent, InputEventResult, InputSource, KeybindingsManager, LoadExtensionsResult, LsToolResultEvent, MessageRenderer, MessageRenderOptions, ModelSelectEvent, ModelSelectSource, ReadToolResultEvent, RegisteredCommand, RegisteredTool, SendMessageHandler, SendUserMessageHandler, SessionBeforeCompactEvent, SessionBeforeCompactResult, SessionBeforeForkEvent, SessionBeforeForkResult, SessionBeforeSwitchEvent, SessionBeforeSwitchResult, SessionBeforeTreeEvent, SessionBeforeTreeResult, SessionCompactEvent, SessionEvent, SessionForkEvent, SessionShutdownEvent, SessionStartEvent, SessionSwitchEvent, SessionTreeEvent, SetActiveToolsHandler, SetLabelHandler, SetModelHandler, SetThinkingLevelHandler, ToolCallEvent, ToolCallEventResult, ToolDefinition, ToolInfo, ToolRenderResultOptions, ToolResultEvent, ToolResultEventResult, TreePreparation, TurnEndEvent, TurnStartEvent, UserBashEvent, UserBashEventResult, WriteToolResultEvent, } from "./types.js";
packages/coding-agent/dist/core/keybindings.d.ts:5:export type AppAction = "interrupt" | "clear" | "exit" | "suspend" | "cycleThinkingLevel" | "cycleModelForward" | "cycleModelBackward" | "selectModel" | "expandTools" | "toggleThinking" | "externalEditor" | "followUp" | "dequeue" | "pasteImage";
packages/coding-agent/dist/core/session-manager.d.ts:26:export interface ThinkingLevelChangeEntry extends SessionEntryBase {
packages/coding-agent/dist/core/session-manager.d.ts:100:export type SessionEntry = SessionMessageEntry | ThinkingLevelChangeEntry | ModelChangeEntry | CompactionEntry | BranchSummaryEntry | CustomEntry | CustomMessageEntry | LabelEntry | SessionInfoEntry;
packages/coding-agent/dist/cli/args.d.ts:37:export declare function isValidThinkingLevel(level: string): level is ThinkingLevel;
packages/coding-agent/dist/index.d.ts:12:export { type BranchSummaryEntry, buildSessionContext, type CompactionEntry, CURRENT_SESSION_VERSION, type CustomEntry, type CustomMessageEntry, type FileEntry, getLatestCompactionEntry, type ModelChangeEntry, migrateSessionEntries, type NewSessionOptions, parseSessionEntries, type SessionContext, type SessionEntry, type SessionEntryBase, type SessionHeader, type SessionInfo, type SessionInfoEntry, SessionManager, type SessionMessageEntry, type ThinkingLevelChangeEntry, } from "./core/session-manager.js";
packages/coding-agent/src/core/extensions/types.ts:917:export type GetThinkingLevelHandler = () => ThinkingLevel;
packages/coding-agent/src/core/extensions/types.ts:919:export type SetThinkingLevelHandler = (level: ThinkingLevel) => void;The PR imports ThinkingLevel from @mariozechner/pi-agent-core. Let me check if this is correct:
>cd /Users/badlogic/workspaces/pi-mono && grep -rn "ThinkingLevel" packages/agent-core --include="*.ts" | head -10(no output)Let me find the agent-core package:
>cd /Users/badlogic/workspaces/pi-mono && find packages -name "pi-agent-core" -type d 2>/dev/null || ls packages(no output)Let me list the packages:
>cd /Users/badlogic/workspaces/pi-mono && ls -la packages/total 0
drwxr-xr-x@ 9 badlogic staff 288 Jan 13 04:43 .
drwxr-xr-x@ 23 badlogic staff 736 Jan 19 00:23 ..
drwxr-xr-x@ 11 badlogic staff 352 Jan 19 00:23 agent
drwxr-xr-x@ 12 badlogic staff 384 Jan 19 15:55 ai
drwxr-xr-x@ 18 badlogic staff 576 Jan 19 15:55 coding-agent
drwxr-xr-x@ 15 badlogic staff 480 Jan 19 00:23 mom
drwxr-xr-x@ 9 badlogic staff 288 Jan 19 00:23 pods
drwxr-xr-x@ 11 badlogic staff 352 Jan 19 00:23 tui
drwxr-xr-x@ 11 badlogic staff 352 Jan 19 00:23 web-uiI see, the packages are agent, ai, etc., not agent-core. Let me check the package names:
>cd /Users/badlogic/workspaces/pi-mono && grep '"name"' packages/agent/package.json packages/ai/package.jsonpackages/agent/package.json: "name": "@mariozechner/pi-agent-core",
packages/ai/package.json: "name": "@mariozechner/pi-ai",Ah, packages/agent is @mariozechner/pi-agent-core. Let me check if ThinkingLevel is exported:
>cd /Users/badlogic/workspaces/pi-mono && grep -rn "ThinkingLevel" packages/agent/src --include="*.ts" | head -10packages/agent/src/agent.ts:24: ThinkingLevel,
packages/agent/src/agent.ts:169: setThinkingLevel(l: ThinkingLevel) {
packages/agent/src/types.ts:104:export type ThinkingLevel = "off" | "minimal" | "low" | "medium" | "high" | "xhigh";
packages/agent/src/types.ts:137: thinkingLevel: ThinkingLevel;Good, ThinkingLevel is defined in packages/agent/src/types.ts and should be exported. The import in the PR's code (import type { ThinkingLevel } from "@mariozechner/pi-agent-core";) looks correct.
Now let me analyze the PR changes thoroughly:
CHANGELOG.md changes - The PR adds an entry under ## [Unreleased] with:
### Added: Ctrl+R reset and Ctrl+T thinking overrides### Changed: Provider name in Ctrl+P status, ✓ for enabled modelsscoped-models-selector.ts changes:
ThinkingLevel, Api, supportsXhighThinkingOverrides typegetThinkingCycleLevels(model)ModelItem now includes thinkingOverride fieldModelsConfig interface changed from enabledModelIds: Set<string> + hasEnabledModelsFilter: boolean to enabledIds: EnabledIds + thinkingOverrides: ThinkingOverridesModelsCallbacks interface simplified from many callbacks to just onChange, onPersist, onCancelinitialEnabledIds and initialThinkingOverrides for reset functionalitybuildPatterns() to convert internal state to string patterns with :level suffixesemitChange() helper to notify parentenableAll function changed to not return null when all are enabled (always returns array)Ctrl+T (cycle thinking) and Ctrl+R (reset)Ctrl+A now just clears filter (sets null) instead of enabling all filteredinteractive-mode.ts file has been significantly refactored where the cycled model status now includes the provider name in brackets, and the showModelsSelector method contains complex logic to preserve thinking overrides from settings by matching session scope to settings scope to determine if explicit :level suffixes should be retained, with a new applySessionScope(patterns) function to update the session and a simplified callback structure using just onChange and onPersist.A new test file scoped-models-selector.test.ts covers showing ✓ when all enabled, cycling thinking overrides with Ctrl+T, ignoring Ctrl+T when no filter is active, handling Ctrl+T when all models are explicitly selected, and resetting with Ctrl+R. One potential issue stands out: the enableAll function now always returns an array instead of returning null when all models become enabled, which is a behavioral change since null previously indicated "all enabled, no filter" and thinking overrides only function when a filter is active.
The Ctrl+A handler sets enabledIds to null and clears thinking overrides, representing "all enabled without per-model thinking" rather than the previous behavior of keeping some models disabled. The matching logic for preserving :level suffixes from settings is intricate, comparing session and settings scopes by checking if IDs align in order. The test mocks use api: "anthropic-messages" for both models despite one having provider: "openai" with id gpt-4o, which works but could be clearer.
Looking closer at the Ctrl+A handler, there's a potential issue: it sets enabledIds to null and clears thinkingOverrides, but when persisting via Ctrl+S, this passes buildPatterns() which returns null when enabledIds is null. This could remove any filter and lose thinking level settings the user previously configured. The onPersist callback in interactive-mode.ts needs to handle this case properly.
Let me verify the type imports work correctly:
>cd /Users/badlogic/workspaces/pi-mono && grep -n "export" packages/agent/src/index.ts | head -202:export * from "./agent.js";
4:export * from "./agent-loop.js";
6:export * from "./proxy.js";
8:export * from "./types.js";export * from "./types.js" means ThinkingLevel is exported. Good.
Now let me check if there are any missing labels for the PR:
>cd /Users/badlogic/workspaces/pi-mono && gh label list --repo badlogic/pi-mono | grep pkgpkg:agent Agent runtime package #e99695
pkg:ai LLM API package #f9d0c4
pkg:coding-agent Coding agent CLI #0e8a16
pkg:mom Slack bot package #fbca04
pkg:pods vLLM deployment CLI #5319e7
pkg:proxy CORS proxy package #006b75
pkg:tui Terminal UI library #1d76db
pkg:web-ui Web UI components #d93f0bThe PR doesn't have a pkg:coding-agent label but it's modifying packages/coding-agent. Let me finalize my review.
Summary of the PR changes:
:level suffixes[provider] when cycling modelsThe CHANGELOG is properly formatted with ### Added and ### Changed sections. The PR includes tests for the new functionality.
Now let me create my review:
Now let me verify documentation requirements:
>cd /Users/badlogic/workspaces/pi-mono && grep -B2 -A10 "enabledModels" packages/coding-agent/README.md | head -40"defaultModel": "claude-sonnet-4-20250514",
"defaultThinkingLevel": "medium",
"enabledModels": ["anthropic/*", "*gpt*", "gemini-2.5-pro:high"],
"steeringMode": "one-at-a-time",
"followUpMode": "one-at-a-time",
"shellPath": "C:\\path\\to\\bash.exe",
"shellCommandPrefix": "shopt -s expand_aliases",
"hideThinkingBlock": false,
"collapseChangelog": false,
"compaction": {
"enabled": true,
"reserveTokens": 16384,
"keepRecentTokens": 20000
--
| `defaultModel` | Default model ID | - |
| `defaultThinkingLevel` | Thinking level: `off`, `minimal`, `low`, `medium`, `high`, `xhigh` | - |
| `enabledModels` | Model patterns for cycling. Supports glob patterns (`github-copilot/*`, `*sonnet*`) and fuzzy matching. Same as `--models` CLI flag | - |
| `steeringMode` | Steering message delivery: `all` or `one-at-a-time` | `one-at-a-time` |
| `followUpMode` | Follow-up message delivery: `all` or `one-at-a-time` | `one-at-a-time` |
| `shellPath` | Custom bash path (Windows) | auto-detected |
| `shellCommandPrefix` | Command prefix for bash (e.g., `shopt -s expand_aliases` for alias support) | - |
| `hideThinkingBlock` | Hide thinking blocks in output (Ctrl+T to toggle) | `false` |
| `collapseChangelog` | Show condensed changelog after update | `false` |
| `compaction.enabled` | Enable auto-compaction | `true` |
| `compaction.reserveTokens` | Tokens to reserve before compaction triggers | `16384` |
| `compaction.keepRecentTokens` | Recent tokens to keep after compaction | `20000` |
| `skills.enabled` | Enable skills discovery | `true` |The README already documents the enabledModels setting format with :level suffixes (e.g., gemini-2.5-pro:high). So there's no additional documentation needed for that.
However, the new /scoped-models UI keybindings (Ctrl+T for thinking, Ctrl+R for reset) are not documented in the README. The README only has a brief mention of /scoped-models in the commands table. We could argue this should be documented in the README since it's new interactive functionality.
Let me finalize my review.
PR: https://github.com/badlogic/pi-mono/pull/837
## [Unreleased] in packages/coding-agent/CHANGELOG.md### Added for new features and ### Changed for modifications- /scoped-models: Ctrl+R resets... ([#837](https://github.com/badlogic/pi-mono/pull/837) by [@CarlosGtrz](https://github.com/CarlosGtrz))onModelToggle, onEnableAll, onClearAll, onToggleProvider, onPersist) with 2 (onChange, onPersist):level suffix format (e.g., model-id:high)pkg:coding-agent labelenabledIds = null). This changes the semantics: pressing Ctrl+A with a search filter used to add those filtered models to the selection, now it clears all filteringshowModelsSelector code that tries to preserve explicit :level suffixes from settings by comparing session scope to settings scope is fragile. If the session scope gets modified (e.g., model order changes), the suffixes are lost and rebuilt from session stateenableAll() previously returned null when all models became enabled (indicating "no filter"). Now it always returns an array. This breaks the convention that null = "all enabled without explicit filter". Code comment says "null = all enabled (no filter)" but the function no longer produces that state/scoped-models?/scoped-models:level suffixes[provider] when cycling models via Ctrl+PonChange callback instead of multiple granular callbacksscoped-models-selector.test.ts with 5 test cases:
:level suffixpackages/coding-agent/README.md or docs (the :level suffix format is already documented)/scoped-models keybindings could be documented, but they're visible in the component's footer hint textpost that on the PR, ask the contributor to adjust their pr, turn the pr into a draft pr
The user wants me to:
Let me do all of these.
>gh pr comment 837 --repo badlogic/pi-mono --body '## PR Review
### Changelog
- **Present**: Yes, properly added under `## [Unreleased]` in `packages/coding-agent/CHANGELOG.md`
- **Format**: Correct. Uses `### Added` for new features and `### Changed` for modifications
- **Attribution**: External contribution, should include PR link and author. Currently missing:
- Should be: `- /scoped-models: Ctrl+R resets... ([#837](https://github.com/badlogic/pi-mono/pull/837) by [@CarlosGtrz](https://github.com/CarlosGtrz))`
### Good
- Clean API simplification: Replaces 5 callbacks (`onModelToggle`, `onEnableAll`, `onClearAll`, `onToggleProvider`, `onPersist`) with 2 (`onChange`, `onPersist`)
- Thinking overrides persist correctly using the existing `:level` suffix format (e.g., `model-id:high`)
- Tests cover key scenarios: checkmarks when all enabled, Ctrl+T cycling, Ctrl+T ignored without filter, Ctrl+R reset
- Footer hint text updated to show new shortcuts (^T thinking, ^R reset)
- Provider name in Ctrl+P status improves UX when multiple providers have similar model names
### Bad
- **Missing PR label**: Should have `pkg:coding-agent` label
- **Changelog attribution missing**: External contribution needs PR link and author per project rules
- **Ctrl+A behavior change**: Previous behavior enabled filtered models; new behavior clears the filter entirely (sets `enabledIds = null`). This changes the semantics: pressing Ctrl+A with a search filter used to add those filtered models to the selection, now it clears all filtering
- **Complex session/settings matching logic**: The `showModelsSelector` code that tries to preserve explicit `:level` suffixes from settings by comparing session scope to settings scope is fragile. If the session scope gets modified (e.g., model order changes), the suffixes are lost and rebuilt from session state
### Ugly
- **enableAll() function semantics changed silently**: The helper function `enableAll()` previously returned `null` when all models became enabled (indicating "no filter"). Now it always returns an array. This breaks the convention that `null` = "all enabled without explicit filter". Code comment says "null = all enabled (no filter)" but the function no longer produces that state
### Questions
- The Ctrl+A change from "enable filtered" to "clear filter entirely" seems intentional for the new thinking override feature (overrides only work with explicit filter). Is this the intended UX change?
- Should the README document the new Ctrl+T and Ctrl+R keybindings in `/scoped-models`?
### Change summary
1. **Ctrl+R reset**: Restores initial model selection and thinking overrides when entering `/scoped-models`
2. **Ctrl+T thinking cycling**: Cycles per-model thinking levels (minimal→low→medium→high→xhigh→off) for enabled models, persisted as `:level` suffixes
3. **Provider in Ctrl+P status**: Shows `[provider]` when cycling models via Ctrl+P
4. **Always show ✓**: Enabled models now always show ✓ checkmark, even in "all enabled" mode
5. **Simplified callbacks**: Component uses single `onChange` callback instead of multiple granular callbacks
### Tests
New test file `scoped-models-selector.test.ts` with 5 test cases covering the new functionality.
---
@CarlosGtrz Thanks for the contribution! Please address the following before we can merge:
1. **Add changelog attribution**: Update the changelog entries to include the PR link and your username (see format above)
2. **Clarify the Ctrl+A behavior change**: Is the change from "enable filtered models" to "clear filter entirely" intentional? If so, please add a note in the PR description explaining the rationale
3. **Consider the `enableAll()` semantics**: The function no longer returns `null` when all models are enabled, which breaks the documented convention. Either restore that behavior or update the code comments to reflect the new semantics
Converting this to draft until these items are addressed.'https://github.com/badlogic/pi-mono/pull/837#issuecomment-3768775371>gh pr edit 837 --repo badlogic/pi-mono --add-label "pkg:coding-agent"https://github.com/badlogic/pi-mono/pull/837>gh pr ready 837 --repo badlogic/pi-mono --undo✓ Pull request badlogic/pi-mono#837 is converted to "draft"Done. I've:
pkg:coding-agent labelThe contributor @CarlosGtrz has been asked to:
enableAll() semantics changeDone. I've:
pkg:coding-agentThe contributor has been asked to:
enableAll() function semantics issue