Fix fast model toggle not being persisted

src/vs/platform/actionWidget/browser/tabbedActionListWidget.ts

@@ -96,8 +96,8 @@ export interface ITabbedActionListShowOptions<T> {
 	readonly tabs: readonly ITabDescriptor[];
 	/** Initially active tab id. Must match an entry in {@link tabs}. */
 	readonly initialTab: string;
-	/** Computes the list items and per-tab options shown when the given tab is active. */
-	createActionList(activeTab: string): ITabbedActionListBuildResult<T>;
+	/** Computes a tab's list, or its resting contents when `forSizing` is true. */
+	createActionList(activeTab: string, forSizing?: boolean): ITabbedActionListBuildResult<T>;
 	/** Item delegate (selection, hide, focus). */
 	readonly delegate: IActionListDelegate<T>;
 	/** Optional accessibility provider passed to the underlying list. */
@@ -158,7 +158,7 @@ export class TabbedActionListWidget extends Disposable {
 	/** Boxes and labels from the last render, so the next one can animate from them. */
 	private _previousTabBoxes: Map<string, ITabBox> | undefined;
 	private _previousTabTexts: ReadonlyMap<string, string> | undefined;
-	/** Initial list height from {@link ITabbedActionListShowOptions.sizingTab}. */
+	/** List height from {@link ITabbedActionListShowOptions.sizingTab}, recomputed when items change. */
 	private _fixedListHeight: number | undefined;
 	private _fixedPopupHeight: number | undefined;
 	private _hasMeasuredSizingTab = false;
@@ -304,11 +304,10 @@ export class TabbedActionListWidget extends Disposable {
 
 				// Built before the active tab's list because a consumer may hold per-tab state
 				// while building, and the active tab has to be the one that keeps it.
-				const needsSizing = !this._hasMeasuredSizingTab
-					&& options.sizingTab !== undefined
-					&& options.tabs.some(tab => tab.id === options.sizingTab);
-				const sizingBuild = needsSizing && options.sizingTab !== activeTab
-					? options.createActionList(options.sizingTab!)
+				const sizingTab = options.tabs.find(tab => tab.id === options.sizingTab)?.id;
+				const needsSizing = !this._hasMeasuredSizingTab && sizingTab !== undefined;
+				const sizingBuild = needsSizing
+					? options.createActionList(sizingTab, true)
 					: undefined;
 
 				const { items, listOptions } = options.createActionList(activeTab);
@@ -329,10 +328,26 @@ export class TabbedActionListWidget extends Disposable {
 				this._refreshActiveList = refreshOptions => {
 					const hadFocus = dom.isAncestorOfActiveElement(widget);
 					applyWidgetClassNames();
-					list.updateItems(options.createActionList(activeTab).items, refreshOptions?.focusItemId, {
+					const sizing = sizingTab !== undefined
+						? options.createActionList(sizingTab, true)
+						: undefined;
+					const refreshed = options.createActionList(activeTab);
+					const sizingHeight = sizing
+						? list.computeHeightForItems(sizing.items, sizing.listOptions?.collapsedByDefault, sizing.listOptions) || undefined
+						: undefined;
+					const sizingChanged = sizingHeight !== this._fixedListHeight;
+					if (sizingChanged) {
+						this._fixedListHeight = sizingHeight;
+						this._fixedPopupHeight = undefined;
+					}
+					list.updateItems(refreshed.items, refreshOptions?.focusItemId, {
 						preserveHover: refreshOptions?.preserveHover,
 						animateItemMove: refreshOptions?.animateItemMove && !this._accessibilityService.isMotionReduced(),
 					});
+					if (sizingChanged) {
+						layout();
+						this._contextViewService.layout();
+					}
 					if (hadFocus && !dom.isAncestorOfActiveElement(widget)) {
 						if (emptyBody) {
 							radio.focusActiveItem();
@@ -574,9 +589,8 @@ export class TabbedActionListWidget extends Disposable {
 	}
 
 	/**
-	 * Rebuilds the active tab's items and the popup's class names in place, keeping its
-	 * position and whatever currently has focus. Use when an action inside the popup
-	 * changes what it shows but should not dismiss it; `preserveHover` retains its live detail panel.
+	 * Rebuilds the active tab and remeasures its resting sizing contents without dismissing the popup.
+	 * Focus is retained, and `preserveHover` keeps the live detail panel.
 	 */
 	refreshActiveList(options?: ITabbedActionListRefreshOptions): void {
 		this._refreshActiveList?.(options);

src/vs/platform/actionWidget/test/browser/tabbedActionListWidget.test.ts

@@ -648,7 +648,7 @@ suite('TabbedActionListWidget', () => {
 
 		// Opens on the short tab on purpose: the height has to come from the sizing tab
 		// regardless of which tab the popup happens to open on.
-		const heightsAcrossTabs = (sizingTab: string | undefined) => {
+		const heightsAcrossTabs = (sizingTab: string | undefined, sizingItemIds = ['a', 'b', 'c', 'd', 'e', 'f']) => {
 			const { widget } = createWidget(disposables);
 			widget.show<ITestItem>({
 				user: 'test',
@@ -658,29 +658,37 @@ suite('TabbedActionListWidget', () => {
 				sizingTab,
 				createActionList: tab => ({
 					items: tab === 'Copilot'
-						? ['a', 'b', 'c', 'd', 'e', 'f'].map(action)
+						? sizingItemIds.map(action)
 						: [action('only')],
 				}),
 				delegate: { onSelect: () => { }, onHide: () => { } },
 			});
 			const onShortTab = listHeight();
+			widget.refreshActiveList();
+			const afterRefresh = listHeight();
 			document.querySelectorAll<HTMLElement>('.tabbed-action-list-tabstrip .monaco-button')[0].click();
 			const onSizingTab = listHeight();
 			widget.hide();
-			return { onShortTab, onSizingTab };
+			return { onShortTab, afterRefresh, onSizingTab };
 		};
 
 		const unsized = heightsAcrossTabs(undefined);
 		const sized = heightsAcrossTabs('Copilot');
+		const empty = heightsAcrossTabs('Copilot', []);
 
 		// Clamping depends on the room around the anchor, which differs between the two
 		// renders here, so compare how each tab is sized rather than the pixels.
 		assert.deepStrictEqual(
 			{
 				resizesWithoutASizingTab: unsized.onShortTab < unsized.onSizingTab,
 				shortTabTakesTheSizingTabsHeight: sized.onShortTab > unsized.onShortTab,
+				emptySizingTab: [empty.onShortTab, empty.afterRefresh],
+			},
+			{
+				resizesWithoutASizingTab: true,
+				shortTabTakesTheSizingTabsHeight: true,
+				emptySizingTab: [unsized.onShortTab, unsized.onShortTab],
 			},
-			{ resizesWithoutASizingTab: true, shortTabTakesTheSizingTabsHeight: true },
 		);
 	});
 
@@ -724,6 +732,80 @@ suite('TabbedActionListWidget', () => {
 		);
 	});
 
+	test('refreshing pins preserves collapsed sizing across tabs and search', async () => {
+		const { widget, contextView } = createWidget(disposables);
+		const anchor = document.createElement('div');
+		anchor.style.cssText = 'position: fixed; top: 400px; width: 120px; height: 20px;';
+		document.body.appendChild(anchor);
+		disposables.add({ dispose: () => anchor.remove() });
+		const content = document.createElement('button');
+		content.textContent = 'Unpin Model';
+		let pinned = true;
+		let searching = false;
+
+		widget.show<ITestItem>({
+			user: 'test',
+			anchor,
+			tabs: [{ id: 'Copilot' }, { id: 'Other' }],
+			initialTab: 'Copilot',
+			sizingTab: 'Copilot',
+			showCheckedItemHover: true,
+			createActionList: (tab, forSizing) => {
+				const model = {
+					...action('model'),
+					item: { id: 'model', checked: true },
+					section: pinned ? undefined : 'other',
+					hover: { content, expandable: true },
+				};
+				return {
+					items: searching && !forSizing ? ['one', 'two', 'three', 'four', 'five', 'six'].map(action) : tab === 'Copilot' ? [
+						...(pinned ? [{ kind: ActionListItemKind.Separator, label: 'Pinned' }, model] : []),
+						action('suggested'),
+						{ ...action('other-models'), section: 'other', isSectionToggle: true },
+						...(pinned ? [] : [model]),
+						...['one', 'two', 'three'].map(id => ({ ...action(id), section: 'other' })),
+					] : [action('provider-model')],
+					listOptions: { collapsedByDefault: new Set(['other']), anchorPosition: AnchorPosition.ABOVE, persistentHover: true },
+				};
+			},
+			delegate: { onSelect: () => { }, onHide: () => { } },
+		});
+
+		const listHeight = () => contextView.getContextViewElement().querySelector<HTMLElement>('.actionList')!.offsetHeight;
+		const initialHeight = listHeight();
+		const toggleOther = () => {
+			const row = Array.from(contextView.getContextViewElement().querySelectorAll<HTMLElement>('.monaco-list-row'))
+				.find(row => row.textContent === 'other-models');
+			assert.ok(row);
+			row.click();
+		};
+		content.focus();
+		pinned = false;
+		widget.refreshActiveList({ focusItemId: 'model', preserveHover: true });
+		toggleOther();
+		await Promise.resolve();
+		const afterUnpin = listHeight();
+		const tabHeights = [];
+		for (const index of [1, 0]) {
+			contextView.getContextViewElement().querySelectorAll<HTMLElement>('.tabbed-action-list-tabstrip .monaco-button')[index].click();
+			tabHeights.push(listHeight());
+		}
+		searching = true;
+		widget.refreshActiveList();
+		const searchHeight = listHeight();
+		pinned = true;
+		widget.refreshActiveList();
+		const afterPin = listHeight();
+		widget.hide();
+
+		assert.deepStrictEqual({ afterUnpin, tabHeights, searchHeight, afterPin }, {
+			afterUnpin: initialHeight / 2,
+			tabHeights: [initialHeight / 2, initialHeight / 2],
+			searchHeight: initialHeight / 2,
+			afterPin: initialHeight,
+		});
+	});
+
 	for (const initialFooterHeight of [20, 80]) {
 		test(`resizing the footer preserves the fixed popup height when opened with a ${initialFooterHeight}px footer`, async () => {
 			const { widget, contextView } = createWidget(disposables);

src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerTabbedWidget.ts

@@ -23,7 +23,7 @@ import { withChatInputPickerMotion } from '../chatInputPickerActionItem.js';
 import { IModelConfigurationAccess } from './modelPickerModelConfig.js';
 import { ModelPickerAutoRow } from './modelPickerAutoRow.js';
 import { IModelCardOptions, IPricingDisclosure, ModelCard } from './modelPickerCard.js';
-import { buildSpeedVariants, collapseSpeedVariants, IModelSpeedVariants } from './modelPickerVariants.js';
+import { getPreferredSpeedVariant, IModelSpeedVariants } from './modelPickerVariants.js';
 import { getModelBadge } from './modelPickerBadges.js';
 import { createModelAction, createUnavailableModelItem, getUnavailableReason } from './modelPickerItemPrimitives.js';
 import { getModelPickerAccessibilityProvider } from './modelPickerItems.js';
@@ -98,7 +98,8 @@ export class TabbedModelPicker extends Disposable {
 	private _anchor: HTMLElement | undefined;
 	private _activeDestination: string | undefined;
 	private _searchVisible = false;
-	private _speedVariants: ReadonlyMap<string, IModelSpeedVariants> = new Map();
+	private readonly _speedVariants = new Map<string, IModelSpeedVariants>();
+	private readonly _preferredSpeedVariants = new Map<string, string>();
 	private _selectionVersion = 0;
 	/** The model to fall back to when Auto is switched off. */
 	private _lastExplicitModelId: string | undefined;
@@ -149,7 +150,7 @@ export class TabbedModelPicker extends Disposable {
 			return;
 		}
 
-		this._speedVariants = buildSpeedVariants(context.models);
+		this._speedVariants.clear();
 		this._cards.clearAndDisposeAll();
 		const destinations = this._buildDestinations(context);
 		if (!destinations.length) {
@@ -179,23 +180,24 @@ export class TabbedModelPicker extends Disposable {
 			tabLabels: 'active',
 			filterInTabBar: true,
 			width: PICKER_WIDTH,
-			createActionList: activeTab => {
+			createActionList: (activeTab, forSizing) => {
 				const current = this._context ?? context;
 				const currentDestinations = this._buildDestinations(current);
 				const destination = currentDestinations.find(candidate => candidate.id === activeTab) ?? currentDestinations[0];
 				const sections = this._buildSections(destination, current);
 				// Search spans every destination at once, so each model names its provider.
-				const items = this._searchVisible
-					? currentDestinations.flatMap(candidate => this._buildSearchItems(candidate, current))
+				const searching = this._searchVisible && !forSizing;
+				const items = searching
+					? currentDestinations.flatMap(candidate => this._buildSearchItems(candidate, candidate === destination ? sections : this._buildSections(candidate, current), current))
 					: this._buildItems(destination, sections, current);
 				return {
 					items,
 					listOptions: withChatInputPickerMotion({
 						className: 'chat-model-picker-dropdown chat-model-picker-tabbed',
 						persistentHover: true,
-						showFilter: this._searchVisible,
+						showFilter: searching,
 						filterPlaceholder: localize('chat.modelPicker.search', "Search models"),
-						focusFilterOnOpen: this._searchVisible,
+						focusFilterOnOpen: searching,
 						initialFilterValue,
 						filterAsCombobox: true,
 						onType: text => {
@@ -239,11 +241,25 @@ export class TabbedModelPicker extends Disposable {
 			},
 			accessibilityProvider: getModelPickerAccessibilityProvider(this._searchVisible),
 		});
+		if (this._context?.selectedModelId) {
+			this._rememberSpeedVariant(this._context.selectedModelId);
+		}
 	}
 
 	private _buildDestinations(context: ITabbedModelPickerContext): IModelPickerDestination[] {
-		const models = collapseSpeedVariants(context.models, this._speedVariants, context.selectedModelId);
-		return buildModelPickerDestinations(models, this._languageModelsService, context.providerPlaceholders);
+		return buildModelPickerDestinations(context.models, this._languageModelsService, context.providerPlaceholders);
+	}
+
+	private _rememberSpeedVariant(modelIdentifier: string): void {
+		const pair = this._speedVariants.get(modelIdentifier);
+		if (pair) {
+			this._preferredSpeedVariants.set(pair.standard.identifier, modelIdentifier);
+		}
+	}
+
+	private _pinnedVariantIds(modelIdentifier: string, context: ITabbedModelPickerContext): string[] {
+		const pair = this._speedVariants.get(modelIdentifier);
+		return context.pinnedModelIds.filter(id => id === modelIdentifier || id === pair?.standard.identifier || id === pair?.fast.identifier);
 	}
 
 	private _isAutoSelected(context: ITabbedModelPickerContext): boolean {
@@ -257,19 +273,24 @@ export class TabbedModelPicker extends Disposable {
 
 	private _buildSections(destination: IModelPickerDestination, context: ITabbedModelPickerContext): IModelPickerSections {
 		const isBuiltIn = destination.id === MODEL_PICKER_BUILT_IN_DESTINATION;
-		return buildModelPickerSections({
+		const sections = buildModelPickerSections({
 			models: destination.models,
 			selectedModelId: context.selectedModelId,
 			recentModelIds: context.recentModelIds,
 			pinnedModelIds: context.pinnedModelIds,
 			controlModels: context.controlModels,
+			preferredSpeedVariants: this._preferredSpeedVariants,
 			// Only the built-in provider curates a shortlist. A provider the user added
 			// gets a tab of its own, which is already the whole of what it offers.
 			showSuggested: isBuiltIn,
 			// Only the built-in provider has a curated catalogue to compare against.
 			showUnavailable: isBuiltIn && context.unavailableContext.show,
 			currentVSCodeVersion: context.unavailableContext.currentVSCodeVersion,
 		});
+		for (const [id, pair] of sections.speedVariants) {
+			this._speedVariants.set(id, pair);
+		}
+		return sections;
 	}
 
 	private _buildTabBarActions(context: ITabbedModelPickerContext): ITabBarAction[] {
@@ -370,9 +391,8 @@ export class TabbedModelPicker extends Disposable {
 	 * Every model in one destination as flat rows, for searching. Sections would only
 	 * get in the way of a result list, but each row still names its provider.
 	 */
-	private _buildSearchItems(destination: IModelPickerDestination, context: ITabbedModelPickerContext): IActionListItem<IActionWidgetDropdownAction>[] {
-		return destination.models
-			.slice()
+	private _buildSearchItems(destination: IModelPickerDestination, sections: IModelPickerSections, context: ITabbedModelPickerContext): IActionListItem<IActionWidgetDropdownAction>[] {
+		return [...sections.pinned, ...sections.suggested, ...sections.other]
 			.sort((left, right) => left.metadata.name.localeCompare(right.metadata.name))
 			.map(model => this._createModelItem(model, context, undefined, getModelProviderLabel(model, this._languageModelsService)));
 	}
@@ -383,7 +403,13 @@ export class TabbedModelPicker extends Disposable {
 		section?: string,
 		providerLabel?: string,
 	): IActionListItem<IActionWidgetDropdownAction> {
-		const { action, ariaDescription } = createModelAction(model, context.selectedModelId, context.onSelect, section, true);
+		const { action, ariaDescription } = createModelAction(model, context.selectedModelId, next => {
+			this._selectionVersion++;
+			const pair = this._speedVariants.get(next.identifier);
+			context.onSelect(pair
+				? getPreferredSpeedVariant(pair, this._context?.selectedModelId, this._preferredSpeedVariants.get(pair.standard.identifier))
+				: next);
+		}, section, true);
 		const badge = getModelBadge(model, { configurationAccess: context.configurationAccess, providerLabel });
 		// While Auto is choosing, a model's settings do not apply, so the card that edits
 		// them stays shut. The row is still selectable, which is what turns Auto off.
@@ -394,12 +420,13 @@ export class TabbedModelPicker extends Disposable {
 			configurationAccess: context.configurationAccess,
 			isUBB: context.isUBB,
 			openerService: this._openerService,
-			isPinned: context.pinnedModelIds.includes(model.identifier),
+			isPinned: this._pinnedVariantIds(model.identifier, context).length > 0,
 			pricingDisclosure: this._pricingDisclosure,
 			speedVariants: this._speedVariants.get(model.identifier),
 			onWillSelect: () => { selectionVersion = ++this._selectionVersion; },
 			onSelect: next => {
 				if (selectionVersion === this._selectionVersion && this._widget.isVisible && next.identifier !== (this._context ?? context).selectedModelId) {
+					this._rememberSpeedVariant(next.identifier);
 					context.onSelect(next);
 					this._context = { ...(this._context ?? context), selectedModelId: next.identifier };
 					this._lastExplicitModelId = next.identifier;
@@ -450,12 +477,19 @@ export class TabbedModelPicker extends Disposable {
 		if (!context?.onTogglePin) {
 			return;
 		}
-		context.onTogglePin(modelIdentifier, pinned);
+		const pinnedVariantIds = this._pinnedVariantIds(modelIdentifier, context);
+		if (pinned) {
+			context.onTogglePin(modelIdentifier, true);
+		} else {
+			for (const id of pinnedVariantIds) {
+				context.onTogglePin(id, false);
+			}
+		}
 		this._context = {
 			...context,
 			pinnedModelIds: pinned
 				? [...context.pinnedModelIds, modelIdentifier]
-				: context.pinnedModelIds.filter(id => id !== modelIdentifier),
+				: context.pinnedModelIds.filter(id => !pinnedVariantIds.includes(id)),
 		};
 		this._widget.refreshActiveList({
 			focusItemId: modelIdentifier,