fix(coding-agent): preserve scoped model order closes #3217

packages/coding-agent/CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Fixed
 
+- Fixed `/scoped-models` reordering to propagate into the `/model` scoped tab, preserving the user-defined scoped model order instead of re-sorting it ([#3217](https://github.com/badlogic/pi-mono/issues/3217))
 - Fixed `session_shutdown` to fire on `SIGHUP` and `SIGTERM` in interactive, print, and RPC modes so extensions can run shutdown cleanup on those signal-driven exits ([#3212](https://github.com/badlogic/pi-mono/issues/3212))
 - Fixed screenshot path parsing to handle lower case am/pm in macOS screenshot filenames ([#3194](https://github.com/badlogic/pi-mono/pull/3194) by [@jay-aye-see-kay](https://github.com/jay-aye-see-kay))
 - Fixed interactive auto-retry status updates to show a live countdown during backoff instead of a static retry delay message ([#3187](https://github.com/badlogic/pi-mono/issues/3187))

packages/coding-agent/src/modes/interactive/components/model-selector.ts

@@ -168,16 +168,16 @@ export class ModelSelectorComponent extends Container implements Focusable {
 			const refreshed = this.modelRegistry.find(scoped.model.provider, scoped.model.id);
 			return refreshed ? { ...scoped, model: refreshed } : scoped;
 		});
-		this.scopedModelItems = this.sortModels(
+		this.scopedModelItems = this.scopedModels.map((scoped) => ({
-			this.scopedModels.map((scoped) => ({
+			provider: scoped.model.provider,
-				provider: scoped.model.provider,
+			id: scoped.model.id,
-				id: scoped.model.id,
+			model: scoped.model,
-				model: scoped.model,
+		}));
-			})),
-		);
 		this.activeModels = this.scope === "scoped" ? this.scopedModelItems : this.allModels;
 		this.filteredModels = this.activeModels;
-		this.selectedIndex = Math.min(this.selectedIndex, Math.max(0, this.filteredModels.length - 1));
+		const currentIndex = this.filteredModels.findIndex((item) => modelsAreEqual(this.currentModel, item.model));
+		this.selectedIndex =
+			currentIndex >= 0 ? currentIndex : Math.min(this.selectedIndex, Math.max(0, this.filteredModels.length - 1));
 	}
 
 	private sortModels(models: ModelItem[]): ModelItem[] {
@@ -207,7 +207,8 @@ export class ModelSelectorComponent extends Container implements Focusable {
 		if (this.scope === scope) return;
 		this.scope = scope;
 		this.activeModels = this.scope === "scoped" ? this.scopedModelItems : this.allModels;
-		this.selectedIndex = 0;
+		const currentIndex = this.activeModels.findIndex((item) => modelsAreEqual(this.currentModel, item.model));
+		this.selectedIndex = currentIndex >= 0 ? currentIndex : 0;
 		this.filterModels(this.searchInput.getValue());
 		if (this.scopeText) {
 			this.scopeText.setText(this.getScopeText());

packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts

@@ -70,22 +70,14 @@ interface ModelItem {
 
 export interface ModelsConfig {
 	allModels: Model<any>[];
-	enabledModelIds: Set<string>;
+	enabledModelIds: string[] | null;
-	/** true if enabledModels setting is defined (empty = all enabled) */
-	hasEnabledModelsFilter: boolean;
 }
 
 export interface ModelsCallbacks {
-	/** Called when a model is toggled (session-only, no persist) */
+	/** Called whenever the enabled model set or order changes (session-only, no persist) */
-	onModelToggle: (modelId: string, enabled: boolean) => void;
+	onChange: (enabledModelIds: string[] | null) => void | Promise<void>;
 	/** Called when user wants to persist current selection to settings */
-	onPersist: (enabledModelIds: string[]) => void;
+	onPersist: (enabledModelIds: string[] | null) => void | Promise<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;
 	onCancel: () => void;
 }
 
@@ -126,7 +118,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
 			this.allIds.push(fullId);
 		}
 
-		this.enabledIds = config.hasEnabledModelsFilter ? [...config.enabledModelIds] : null;
+		this.enabledIds = config.enabledModelIds === null ? null : [...config.enabledModelIds];
 		this.filteredItems = this.buildItems();
 
 		// Header
@@ -184,6 +176,10 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
 		this.footerText.setText(this.getFooterText());
 	}
 
+	private notifyChange(): void {
+		this.callbacks.onChange(this.enabledIds === null ? null : [...this.enabledIds]);
+	}
+
 	private updateList(): void {
 		this.listContainer.clear();
 
@@ -254,6 +250,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
 					this.isDirty = true;
 					this.selectedIndex += delta;
 					this.refresh();
+					this.notifyChange();
 				}
 			}
 			return;
@@ -263,12 +260,10 @@ 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.refresh();
+				this.notifyChange();
 			}
 			return;
 		}
@@ -278,8 +273,8 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl
 			const targetIds = this.searchInput.getValue() ? this.filteredItems.map((i) => i.fullId) : undefined;
 			this.enabledIds = enableAll(this.enabledIds, this.allIds, targetIds);
 			this.isDirty = true;
-			this.callbacks.onEnableAll(targetIds ?? this.allIds);
 			this.refresh();
+			this.notifyChange();
 			return;
 		}
 
@@ -288,8 +283,8 @@ 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.refresh();
+			this.notifyChange();
 			return;
 		}
 
@@ -304,15 +299,15 @@ 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.refresh();
+				this.notifyChange();
 			}
 			return;
 		}
 
 		// Ctrl+S - Save/persist to settings
 		if (matchesKey(data, Key.ctrl("s"))) {
-			this.callbacks.onPersist(this.enabledIds ?? [...this.allIds]);
+			this.callbacks.onPersist(this.enabledIds === null ? null : [...this.enabledIds]);
 			this.isDirty = false;
 			this.footerText.setText(this.getFooterText());
 			return;

packages/coding-agent/src/modes/interactive/interactive-mode.ts

@@ -3673,35 +3673,25 @@ export class InteractiveMode {
 		const hasSessionScope = sessionScopedModels.length > 0;
 
 		// Build enabled model IDs from session state or settings
-		const enabledModelIds = new Set<string>();
+		let currentEnabledIds: string[] | null = null;
-		let hasFilter = false;
 
 		if (hasSessionScope) {
 			// Use current session's scoped models
-			for (const sm of sessionScopedModels) {
+			currentEnabledIds = sessionScopedModels.map((scoped) => `${scoped.model.provider}/${scoped.model.id}`);
-				enabledModelIds.add(`${sm.model.provider}/${sm.model.id}`);
-			}
-			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);
-				for (const sm of scopedModels) {
+				currentEnabledIds = scopedModels.map((scoped) => `${scoped.model.provider}/${scoped.model.id}`);
-					enabledModelIds.add(`${sm.model.provider}/${sm.model.id}`);
-				}
 			}
 		}
 
-		// 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>) => {
+		const updateSessionModels = async (enabledIds: string[] | null) => {
-			if (enabledIds.size > 0 && enabledIds.size < allModels.length) {
+			currentEnabledIds = enabledIds === null ? null : [...enabledIds];
-				const newScopedModels = await resolveModelScope(Array.from(enabledIds), this.session.modelRegistry);
+			if (enabledIds && enabledIds.length > 0 && enabledIds.length < allModels.length) {
+				const newScopedModels = await resolveModelScope(enabledIds, this.session.modelRegistry);
 				this.session.setScopedModels(
 					newScopedModels.map((sm) => ({
 						model: sm.model,
@@ -3721,49 +3711,18 @@ export class InteractiveMode {
 				{
 					allModels,
 					enabledModelIds: currentEnabledIds,
-					hasEnabledModelsFilter: currentHasFilter,
 				},
 				{
-					onModelToggle: async (modelId, enabled) => {
+					onChange: async (enabledIds) => {
-						if (enabled) {
+						await updateSessionModels(enabledIds);
-							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);
 					},
 					onPersist: (enabledIds) => {
 						// Persist to settings
 						const newPatterns =
-							enabledIds.length === allModels.length
+							enabledIds === null || enabledIds.length === allModels.length
 								? undefined // All enabled = clear filter
 								: enabledIds;
-						this.settingsManager.setEnabledModels(newPatterns);
+						this.settingsManager.setEnabledModels(newPatterns ? [...newPatterns] : undefined);
 						this.showStatus("Model selection saved to settings");
 					},
 					onCancel: () => {

packages/coding-agent/test/suite/regressions/3217-scoped-model-order.test.ts

@@ -0,0 +1,98 @@
+import type { TUI } from "@mariozechner/pi-tui";
+import stripAnsi from "strip-ansi";
+import { afterEach, beforeAll, describe, expect, it } from "vitest";
+import { ModelSelectorComponent } from "../../../src/modes/interactive/components/model-selector.js";
+import { ScopedModelsSelectorComponent } from "../../../src/modes/interactive/components/scoped-models-selector.js";
+import { initTheme } from "../../../src/modes/interactive/theme/theme.js";
+import { createHarness, type Harness } from "../harness.js";
+
+function createFakeTui(): TUI {
+	return {
+		requestRender: () => {},
+	} as unknown as TUI;
+}
+
+async function waitForAsyncRender(): Promise<void> {
+	await new Promise((resolve) => setTimeout(resolve, 0));
+}
+
+describe("issue #3217 scoped model ordering", () => {
+	const harnesses: Harness[] = [];
+
+	beforeAll(() => {
+		initTheme("dark");
+	});
+
+	afterEach(() => {
+		while (harnesses.length > 0) {
+			harnesses.pop()?.cleanup();
+		}
+	});
+
+	it("propagates reordered scoped models back to the session state", async () => {
+		const harness = await createHarness({
+			models: [
+				{ id: "faux-1", name: "One", reasoning: true },
+				{ id: "faux-2", name: "Two", reasoning: true },
+				{ id: "faux-3", name: "Three", reasoning: true },
+			],
+		});
+		harnesses.push(harness);
+
+		const orderedIds = harness.models.map((model) => `${model.provider}/${model.id}`);
+		const changes: Array<string[] | null> = [];
+		const selector = new ScopedModelsSelectorComponent(
+			{
+				allModels: [...harness.models],
+				enabledModelIds: orderedIds,
+			},
+			{
+				onChange: (enabledModelIds) => {
+					changes.push(enabledModelIds);
+				},
+				onPersist: () => {},
+				onCancel: () => {},
+			},
+		);
+
+		selector.handleInput("\x1b[1;3B");
+
+		expect(changes).toEqual([[orderedIds[1], orderedIds[0], orderedIds[2]]]);
+	});
+
+	it("preserves scoped model order in the /model scoped tab", async () => {
+		const harness = await createHarness({
+			models: [
+				{ id: "faux-1", name: "One", reasoning: true },
+				{ id: "faux-2", name: "Two", reasoning: true },
+				{ id: "faux-3", name: "Three", reasoning: true },
+			],
+		});
+		harnesses.push(harness);
+
+		const modelOne = harness.getModel("faux-1")!;
+		const modelTwo = harness.getModel("faux-2")!;
+		const modelThree = harness.getModel("faux-3")!;
+		const selector = new ModelSelectorComponent(
+			createFakeTui(),
+			modelOne,
+			harness.settingsManager,
+			harness.session.modelRegistry,
+			[{ model: modelTwo }, { model: modelOne }, { model: modelThree }],
+			() => {},
+			() => {},
+		);
+
+		await waitForAsyncRender();
+
+		const renderedLines = stripAnsi(selector.render(120).join("\n"))
+			.split("\n")
+			.filter((line) => line.includes(`[${modelOne.provider}]`));
+		const orderedIds = renderedLines.slice(0, 3).map((line) => {
+			const [modelId] = line.trim().replace(/^→\s*/, "").split(" [");
+			return modelId?.trim() ?? "";
+		});
+
+		expect(orderedIds).toEqual([modelTwo.id, modelOne.id, modelThree.id]);
+	});
+});