feat(coding-agent): adjust TUI selections in thinking-mode, models and scoped models (#8900)

* fix(coding-agent): keep selector status markers visible

- place status markers before labels
- strike unavailable scoped models and update tests

* fix(coding-agent): align scoped model toggle behavior

- show checks for the all-enabled state
- toggle individual models consistently and add coverage

* docs(coding-agent): add changelog entry

* fix(coding-agent): keep trust marker visible while browsing

- move the saved marker before the trust option
- update tests and changelog

* fix(coding-agent): mark per-model thinking overrides

- add the leading level marker
- update tests and changelog
This commit is contained in:
Ramiz Wachtler 2026-09-01 11:02:53 +02:00 • committed by GitHub
parent afda4d6202
commit f2a6227899
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 286 additions and 50 deletions

View file

@ -22,6 +22,10 @@
- Added environment variables and advanced settings for overriding auto-detected terminal hyperlink, image, and truecolor capabilities ([#8665](https://github.com/earendil-works/pi/issues/8665)).
- Added `fullscreenCopyOnSelect` to disable automatic fullscreen selection copy; when disabled, `Ctrl+X` copies the active text selection before falling back to the last assistant message, while `/tree` still copies the selected message ([#7720](https://github.com/earendil-works/pi/issues/7720)).
### Changed
- Changed selectors in `/thinking`, `/model`, `/scoped-models`, `/trust`, and per-model thinking settings to keep active options marked while browsing. `/scoped-models` now uses consistent per-item toggles and strikes through unavailable models ([#8900](https://github.com/earendil-works/pi/pull/8900)).
### Fixed
- Fixed toggling thinking visibility clearing partial output from running Bash tools ([#8611](https://github.com/earendil-works/pi/issues/8611)).

View file

@ -316,19 +316,11 @@ export class ModelSelectorComponent extends Container implements Focusable {
const isDefault = this.isDefaultModel(item.model);
const defaultBadge = isDefault ? theme.fg("muted", " · default") : "";
let line = "";
if (isSelected) {
const prefix = theme.fg("accent", "→ ");
const modelText = `${item.id}`;
const providerBadge = theme.fg("muted", `[${item.provider}]`);
const checkmark = isCurrent ? theme.fg("success", " ✓") : "";
line = `${prefix + theme.fg("accent", modelText)} ${providerBadge}${defaultBadge}${checkmark}`;
} else {
const modelText = ` ${item.id}`;
const providerBadge = theme.fg("muted", `[${item.provider}]`);
const checkmark = isCurrent ? theme.fg("success", " ✓") : "";
line = `${modelText} ${providerBadge}${defaultBadge}${checkmark}`;
}
const cursor = isSelected ? theme.fg("accent", "→ ") : " ";
const currentMarker = isCurrent ? theme.fg("accent", "✓ ") : " ";
const modelText = isSelected ? theme.fg("accent", item.id) : item.id;
const providerBadge = theme.fg("muted", `[${item.provider}]`);
const line = `${cursor}${currentMarker}${modelText} ${providerBadge}${defaultBadge}`;
this.listContainer.addChild(new Text(line, 0, 0));
}

View file

@ -22,11 +22,16 @@ function isEnabled(enabledIds: EnabledIds, id: string): boolean {
return enabledIds === null || enabledIds.includes(id);
}
function toggle(enabledIds: EnabledIds, id: string): EnabledIds {
if (enabledIds === null) return [id]; // First toggle: start with only this one
/** Collapse an explicit list back to null (= all enabled) when it covers every available model. */
function normalizeEnabled(result: string[], allIds: string[]): EnabledIds {
return result.length === allIds.length && result.every((id) => allIds.includes(id)) ? null : result;
}
function toggle(enabledIds: EnabledIds, allIds: string[], id: string): EnabledIds {
if (enabledIds === null) return allIds.filter((modelId) => modelId !== id);
const index = enabledIds.indexOf(id);
if (index >= 0) return [...enabledIds.slice(0, index), ...enabledIds.slice(index + 1)];
return [...enabledIds, id];
return normalizeEnabled([...enabledIds, id], allIds);
}
function enableAll(enabledIds: EnabledIds, allIds: string[], targetIds?: string[]): EnabledIds {
@ -36,7 +41,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 && result.every((id) => allIds.includes(id)) ? null : result;
return normalizeEnabled(result, allIds);
}
function clearAll(enabledIds: EnabledIds, allIds: string[], targetIds?: string[]): EnabledIds {
@ -240,23 +245,16 @@ 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 id = item.model?.id ?? item.fullId;
const modelText = isSelected ? theme.fg("accent", id) : id;
const styledId = item.model ? id : theme.strikethrough(id);
const modelText = isSelected ? theme.fg("accent", styledId) : styledId;
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));
const status = item.model && item.enabled ? theme.fg("accent", "✓ ") : " ";
this.listContainer.addChild(new Text(`${prefix}${status}${modelText}${providerBadge}`, 0, 0));
}
// Add scroll indicator if needed
@ -322,7 +320,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
if (kb.matches(data, "tui.select.confirm")) {
const item = this.filteredItems[this.selectedIndex];
if (item) {
this.enabledIds = toggle(this.enabledIds, item.fullId);
this.enabledIds = toggle(this.enabledIds, this.allIds, item.fullId);
this.isDirty = true;
this.refresh();
this.notifyChange();

View file

@ -626,15 +626,16 @@ export class SettingsSelectorComponent extends Container {
const levels = (
model.reasoning ? getSupportedThinkingLevels(model) : ["off"]
) as ThinkingLevel[];
const activeLevel = currentModelThinkingLevels[ctx.model];
const items: SelectItem[] = levels.map((level) => ({
value: level,
label: level,
label: `${level === activeLevel ? "✓ " : " "}${level}`,
description: THINKING_DESCRIPTIONS[level],
}));
if (currentModelThinkingLevels[ctx.model] !== undefined) {
items.push({
value: CLEAR_OVERRIDE_VALUE,
label: "(clear override)",
label: " (clear override)",
description: `Revert to global default (${config.thinkingLevel})`,
});
}

View file

@ -68,7 +68,7 @@ export class ThinkingSelectorComponent extends Container implements Focusable {
this.allItems = availableLevels.map((level) => ({
value: level,
label: level,
label: `${level === currentLevel ? "✓ " : " "}${level}`,
description:
level === defaultThinkingLevel ? `${LEVEL_DESCRIPTIONS[level]} · default` : LEVEL_DESCRIPTIONS[level],
}));
@ -110,7 +110,7 @@ export class ThinkingSelectorComponent extends Container implements Focusable {
private applyFilter(query: string): void {
const filtered = query
? fuzzyFilter(this.allItems, query, (item) => `${item.label} ${item.description ?? ""}`)
? fuzzyFilter(this.allItems, query, (item) => `${item.value} ${item.description ?? ""}`)
: this.allItems;
const selectedValue = this.selectList.getSelectedItem()?.value as ThinkingLevel | undefined;
const newList = this.buildSelectList(filtered, selectedValue);

View file

@ -107,10 +107,10 @@ export class TrustSelectorComponent extends Container {
const isSelected = i === this.selectedIndex;
const isCurrent = this.isSavedOption(option);
const checkmark = isCurrent ? theme.fg("success", " ✓") : "";
const currentMarker = isCurrent ? theme.fg("accent", "✓ ") : " ";
const prefix = isSelected ? theme.fg("accent", "→ ") : " ";
const label = isSelected ? theme.fg("accent", option.label) : theme.fg("text", option.label);
this.listContainer.addChild(new Text(`${prefix}${label}${checkmark}`, 1, 0));
this.listContainer.addChild(new Text(`${prefix}${currentMarker}${label}`, 1, 0));
}
}

View file

@ -1,5 +1,6 @@
import type { TUI } from "@earendil-works/pi-tui";
import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import { setKeybindings, type TUI } from "@earendil-works/pi-tui";
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import { KeybindingsManager } from "../src/core/keybindings.ts";
import { ModelSelectorComponent } from "../src/modes/interactive/components/model-selector.ts";
import { initTheme } from "../src/modes/interactive/theme/theme.ts";
import { stripAnsi } from "../src/utils/ansi.ts";
@ -16,11 +17,45 @@ describe("model selector", () => {
initTheme("dark");
});
beforeEach(() => {
setKeybindings(new KeybindingsManager());
});
afterEach(() => {
harness?.cleanup();
harness = undefined;
});
it("keeps the current model marked while browsing", async () => {
harness = await createHarness({
models: [
{ id: "current-model", name: "Current Model", reasoning: true },
{ id: "browsed-model", name: "Browsed Model", reasoning: true },
],
});
const currentModel = harness.getModel("current-model")!;
const selector = new ModelSelectorComponent(
createFakeTui(),
currentModel,
harness.session.modelRuntime,
[],
() => {},
() => {},
);
const getModelRow = (id: string): string | undefined =>
stripAnsi(selector.render(120).join("\n"))
.split("\n")
.find((line) => line.includes(`${id} [`))
?.trimEnd();
expect(getModelRow("current-model")).toBe(`→ ✓ current-model [${currentModel.provider}]`);
selector.handleInput("\x1b[B");
expect(getModelRow("current-model")).toBe(` ✓ current-model [${currentModel.provider}]`);
expect(getModelRow("browsed-model")).toBe(`→ browsed-model [${currentModel.provider}]`);
selector.dispose();
});
it("lists every catalog that failed to refresh", async () => {
harness = await createHarness();
vi.spyOn(harness.session.modelRuntime, "refresh").mockResolvedValue({

View file

@ -0,0 +1,126 @@
import { setKeybindings } from "@earendil-works/pi-tui";
import { afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest";
import { KeybindingsManager } from "../src/core/keybindings.ts";
import { ScopedModelsSelectorComponent } from "../src/modes/interactive/components/scoped-models-selector.ts";
import { initTheme } from "../src/modes/interactive/theme/theme.ts";
import { stripAnsi } from "../src/utils/ansi.ts";
import { createHarness, type Harness } from "./suite/harness.ts";
interface ModelState {
id: string;
name: string;
enabled: boolean;
}
function getMarkerStates(selector: ScopedModelsSelectorComponent, models: ModelState[]): boolean[] {
const lines = stripAnsi(selector.render(120).join("\n")).split("\n");
return models.map((model) => {
const line = lines.find((candidate) => candidate.includes(`${model.id} [`));
if (!line) throw new Error(`Expected rendered row for ${model.id}`);
return line.slice(2).startsWith("✓ ");
});
}
describe("scoped models selector", () => {
let harness: Harness | undefined;
beforeAll(() => {
initTheme("dark");
});
beforeEach(() => {
setKeybindings(new KeybindingsManager());
});
afterEach(() => {
harness?.cleanup();
harness = undefined;
});
async function createSelector(models: ModelState[]) {
harness = await createHarness({ models });
const provider = harness.models[0].provider;
const selector = new ScopedModelsSelectorComponent(
{
allModels: [...harness.models],
enabledModelIds: models.filter((model) => model.enabled).map((model) => `${provider}/${model.id}`),
},
{
onChange: (enabledModelIds) => {
for (const model of models) {
model.enabled = enabledModelIds === null || enabledModelIds.includes(`${provider}/${model.id}`);
}
},
onPersist: () => {},
onCancel: () => {},
},
);
return selector;
}
it("marks every model after enabling all", async () => {
const models = [
{ id: "model-a", name: "Model A", enabled: true },
{ id: "model-b", name: "Model B", enabled: false },
{ id: "model-c", name: "Model C", enabled: false },
];
const selector = await createSelector(models);
selector.handleInput("\x01");
expect(models.map((model) => model.enabled)).toEqual([true, true, true]);
expect(getMarkerStates(selector, models)).toEqual([true, true, true]);
expect(stripAnsi(selector.render(120).join("\n"))).toContain("all enabled");
});
it("disables only the selected model after enabling all", async () => {
const models = [
{ id: "model-a", name: "Model A", enabled: true },
{ id: "model-b", name: "Model B", enabled: false },
{ id: "model-c", name: "Model C", enabled: false },
];
const selector = await createSelector(models);
selector.handleInput("\x01");
selector.handleInput("\r");
expect(models.map((model) => model.enabled)).toEqual([false, true, true]);
expect(getMarkerStates(selector, models)).toEqual([false, true, true]);
});
it("enables only the selected model after clearing all", async () => {
const models = [
{ id: "model-a", name: "Model A", enabled: true },
{ id: "model-b", name: "Model B", enabled: true },
{ id: "model-c", name: "Model C", enabled: true },
];
const selector = await createSelector(models);
selector.handleInput("\x18");
expect(models.map((model) => model.enabled)).toEqual([false, false, false]);
expect(getMarkerStates(selector, models)).toEqual([false, false, false]);
selector.handleInput("\r");
expect(models.map((model) => model.enabled)).toEqual([true, false, false]);
expect(getMarkerStates(selector, models)).toEqual([true, false, false]);
});
it("restores the all-enabled state after re-enabling the last disabled model", async () => {
const models = [
{ id: "model-a", name: "Model A", enabled: true },
{ id: "model-b", name: "Model B", enabled: false },
{ id: "model-c", name: "Model C", enabled: false },
];
const selector = await createSelector(models);
selector.handleInput("\x01"); // enable all -> null
selector.handleInput("\r"); // disable model-a; enabled models re-sort first: [b, c, a]
selector.handleInput("\x1b[B");
selector.handleInput("\x1b[B"); // move selection back to model-a
selector.handleInput("\r"); // re-enable model-a
expect(models.map((model) => model.enabled)).toEqual([true, true, true]);
expect(getMarkerStates(selector, models)).toEqual([true, true, true]);
expect(stripAnsi(selector.render(120).join("\n"))).toContain("all enabled");
});
});

View file

@ -1,5 +1,5 @@
import { setKeybindings } from "@earendil-works/pi-tui";
import { beforeAll, describe, expect, it, vi } from "vitest";
import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import { KeybindingsManager } from "../src/core/keybindings.ts";
import {
type SettingsCallbacks,
@ -7,13 +7,21 @@ import {
SettingsSelectorComponent,
} from "../src/modes/interactive/components/settings-selector.ts";
import { initTheme } from "../src/modes/interactive/theme/theme.ts";
import { stripAnsi } from "../src/utils/ansi.ts";
import { createHarness, type Harness } from "./suite/harness.ts";
describe("SettingsSelectorComponent", () => {
let harness: Harness | undefined;
beforeAll(() => {
initTheme("dark");
setKeybindings(new KeybindingsManager());
});
afterEach(() => {
harness?.cleanup();
harness = undefined;
});
it("cycles through fullscreen settings", () => {
const onExitOutputChange = vi.fn();
const onScrollbarChange = vi.fn();
@ -48,4 +56,33 @@ describe("SettingsSelectorComponent", () => {
cycle("Fullscreen copy on select", 2);
expect(onCopyOnSelectChange.mock.calls.flat()).toEqual([false, true]);
});
it("keeps the configured per-model thinking level marked while browsing", async () => {
harness = await createHarness({
models: [{ id: "thinking-model", reasoning: true }],
});
const model = harness.getModel("thinking-model")!;
const modelKey = `${model.provider}/${model.id}`;
const config = {
defaultModel: modelKey,
availableDefaultModels: [model],
thinkingLevel: "high",
modelThinkingLevels: { [modelKey]: "medium" },
} as unknown as SettingsConfig;
const callbacks = { onCancel: () => {} } as unknown as SettingsCallbacks;
const list = new SettingsSelectorComponent(config, callbacks).getSettingsList();
list.selectItem("model-thinking");
list.handleInput("\r");
list.handleInput("\r");
let output = stripAnsi(list.render(120).join("\n"));
expect(output).toContain("→ ✓ medium");
expect(output).toContain(" (clear override)");
list.handleInput("\x1b[B");
output = stripAnsi(list.render(120).join("\n"));
expect(output).toContain(" ✓ medium");
expect(output).toContain("→ high");
});
});

View file

@ -95,7 +95,7 @@ describe("issue #3217 scoped model ordering", () => {
.filter((line) => line.includes(`[${modelOne.provider}]`));
const orderedIds = renderedLines.slice(0, 3).map((line) => {
const [modelId] = line.trim().replace(/^→\s*/, "").split(" [");
return modelId?.trim() ?? "";
return modelId?.replace(/^✓\s*/, "").trim() ?? "";
});
expect(orderedIds).toEqual([modelTwo.id, modelOne.id, modelThree.id]);

View file

@ -4,7 +4,7 @@ import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vite
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 { initTheme, theme } from "../../../src/modes/interactive/theme/theme.ts";
import { stripAnsi } from "../../../src/utils/ansi.ts";
import { createHarness, type Harness } from "../harness.ts";
@ -82,7 +82,9 @@ describe("issue #6949 unavailable scoped models", () => {
},
);
expect(stripAnsi(selector.render(100).join("\n"))).toContain(`${unavailableId} [unavailable] ✗`);
const rendered = selector.render(100).join("\n");
expect(stripAnsi(rendered)).toContain(`${unavailableId} [unavailable]`);
expect(rendered).toContain(theme.strikethrough(unavailableId));
selector.handleInput("\r");
expect(changes).toEqual([[availableId]]);
selector.handleInput("\x13");
@ -104,7 +106,7 @@ describe("issue #6949 unavailable scoped models", () => {
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(rendered).toContain(`${unavailableId} [unavailable]`);
}
expect(getAvailableSnapshot).toHaveBeenCalled();
});
@ -124,7 +126,7 @@ describe("issue #6949 unavailable scoped models", () => {
const selector = getSelector();
if (!selector) throw new Error("Expected scoped-model selector to open");
expect(stripAnsi(selector.render(100).join("\n"))).toContain(`${fullId} [unavailable] ✗`);
expect(stripAnsi(selector.render(100).join("\n"))).toContain(`${fullId} [unavailable]`);
});
it("does not clear a partial scope when an enabled model is unavailable", async () => {

View file

@ -15,7 +15,7 @@ function selectedModelId(rendered: string): string | undefined {
const line = rendered.split("\n").find((l) => l.startsWith("→ "));
if (!line) return undefined;
const rest = line.replace(/^→\s*/, "");
const id = rest.split(" [")[0];
const id = rest.split(" [")[0]?.replace(/^✓\s*/, "");
return id?.trim() || undefined;
}

View file

@ -0,0 +1,37 @@
import { setKeybindings } from "@earendil-works/pi-tui";
import { beforeAll, beforeEach, describe, expect, it } from "vitest";
import { KeybindingsManager } from "../src/core/keybindings.ts";
import { ThinkingSelectorComponent } from "../src/modes/interactive/components/thinking-selector.ts";
import { initTheme } from "../src/modes/interactive/theme/theme.ts";
import { stripAnsi } from "../src/utils/ansi.ts";
describe("thinking selector", () => {
beforeAll(() => {
initTheme("dark");
});
beforeEach(() => {
setKeybindings(new KeybindingsManager());
});
it("keeps the current thinking level marked while browsing", () => {
const selector = new ThinkingSelectorComponent(
"medium",
["medium", "high"],
() => {},
() => {},
);
const getLevelRow = (level: string): string | undefined =>
selector
.getSelectList()
.render(80)
.map((line) => stripAnsi(line))
.find((line) => line.includes(level));
expect(selector.getSelectList().getSelectedItem()?.label).toBe("✓ medium");
expect(getLevelRow("medium")?.startsWith("→ ✓ medium")).toBe(true);
selector.handleInput("\x1b[B");
expect(getLevelRow("medium")?.startsWith(" ✓ medium")).toBe(true);
expect(getLevelRow("high")?.startsWith("→ high")).toBe(true);
});
});

View file

@ -14,7 +14,7 @@ describe("TrustSelectorComponent", () => {
setKeybindings(new KeybindingsManager());
});
it("marks the saved trusted decision", () => {
it("keeps the saved trusted decision marked while browsing", () => {
const selector = new TrustSelectorComponent({
cwd: "/project",
savedDecision: { path: "/project", decision: true },
@ -23,12 +23,16 @@ describe("TrustSelectorComponent", () => {
onCancel: () => {},
});
const output = stripAnsi(selector.render(120).join("\n"));
let output = stripAnsi(selector.render(120).join("\n"));
expect(output).toContain("Saved decision: trusted (/project)");
expect(output).toContain("Current session: trusted");
expect(output).toContain("Trust ✓");
expect(output).not.toContain("Do not trust ✓");
expect(output).toContain("→ ✓ Trust");
selector.handleInput("\x1b[B");
output = stripAnsi(selector.render(120).join("\n"));
expect(output).toContain("✓ Trust");
expect(output).toContain("→ Trust parent folder (/)");
expect(output).not.toContain("✓ Do not trust");
});
it("selects a trust decision", () => {
@ -72,7 +76,7 @@ describe("TrustSelectorComponent", () => {
const output = stripAnsi(selector.render(120).join("\n"));
expect(output).toContain("Saved decision: trusted (inherited from /parent)");
expect(output).toContain("Trust parent folder (/parent) ✓");
expect(output).toContain("✓ Trust parent folder (/parent)");
selector.handleInput("\n");