Experimental model picker redesign - #334332
Conversation
Redesigns the chat model picker behind `chat.experimentalModelPicker` (off by default) so the new and old designs can be compared side by side. The separate model, context, and thinking effort pickers become one surface: a tabbed list with a detail card that shows thinking effort, context, and pricing that updates as those change. Destinations are capped at two, built-in and user-provided, since almost no one has more than one BYOK provider. Sections are Pinned, a short suggested list, and the rest folded away. Also adds a shared Switch widget and a segmented control, both reused by the picker and the customizations UI, and support in the action list for tab bar actions, icon-only tabs, footers, welcome bodies, and in-place item updates. Includes an unrelated fix for the DOM sanitizer, which now replaces its Trusted Types policy when the realm that created it goes away.
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: Matt Bierner (@mjbvz)Matched files:
|
|
have some merge conflicts already 🪨 |
Resolves the model picker name-chip minimum width conflict. Main added a fixed 50px floor for the "Auto" label; this branch measures the chip's intrinsic width and clamps it at the standard floor, which covers "Auto" and every other short label, so the branch's rule is kept and the now-dead constant removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
It's because I fixed the model label being too wide after complaining about it for a week, so now I have a conflcit because you finally fixed it. :p |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Two critical picker failures and multiple unresolved accessibility, focus, telemetry, styling, state-refresh, and test issues must be addressed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 2
New issues introduced by this change (15)
| Severity | Finding |
|---|---|
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerTabs.ts — Keying destinations only by their display label merges distinct providers that happen to have the… |
|
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerTabs.ts — A promoted model can also be gated by minVSCodeVersion. Such a model is removed from… |
|
src/vs/base/browser/ui/toggle/switch.css — The workbench uses .hc-black/.hc-light, so this webview-only selector does not apply to the new… |
|
src/vs/base/test/browser/domSanitize.test.ts — This test does not install the throwing policy: the many sanitizeHtml calls earlier in the suite… |
|
src/vs/platform/actionWidget/browser/actionList.ts — The expandable row gains a visual chevron and keyboard-open behavior, but the trigger receives no… |
|
src/vs/platform/actionWidget/browser/tabbedActionListWidget.ts — aria-pressed="false" makes every momentary tab-bar action (for example “Add Models…”) announce as… |
|
src/vs/platform/actionWidget/browser/tabbedActionListWidget.ts — When renderEmpty handles the tab, the list DOM is not attached, but the code still focuses that… |
|
src/vs/platform/actionWidget/browser/tabbedActionListWidget.ts — The empty body may contain focusable actions such as the model picker's “Sign in” button, but it is… |
|
src/vs/platform/actionWidget/browser/tabbedActionListWidget.ts — listNavigation is also true inside the new expandable hover panel. Native card controls such as… |
|
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/media/modelPicker.css — This removes every visible focus treatment from the search field without providing an alternative.… |
|
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerAutoRow.ts — Changing Auto's routing tier writes the configuration directly but never calls the picker's… |
|
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerCard.ts — This fallback can never render a tooltip because _renderHeader() has already added the first… |
|
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerTabbedWidget.ts — The new picker is a rich interactive surface, but no focused context key or accessibility-help… |
|
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerTabbedWidget.ts — Refreshing only the current list leaves the newly selected fallback invisible when it belongs to… |
|
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/media/modelPicker.css — This welcome glyph falls outside the shared codicon size roles. Codicons are designed for either… |
What changed in this PR
Redesigns the experimental chat model picker with provider tabs, model details, configuration controls, and supporting shared UI infrastructure.
Changes:
- Adds provider tabs, search, model cards, badges, speed variants, pricing, and Auto controls.
- Extends shared action-list, radio, and switch widgets.
- Adds fixtures, tests, and Trusted Types sanitizer recovery.
| File | Reviewed change | Review comments |
|---|---|---|
src/vs/workbench/test/browser/componentFixtures/chat/tabbedModelPicker.fixture.ts |
Adds visual picker fixtures. | None. |
src/vs/workbench/contrib/chat/test/browser/widget/input/sessionTargetPickerActionItem.test.ts |
Supports lazy hover content. | None. |
src/vs/workbench/contrib/chat/test/browser/widget/input/modelPicker/modelPickerConfiguration.test.ts |
Updates configuration imports. | None. |
src/vs/workbench/contrib/chat/test/browser/widget/input/modelPicker/modelPickerAutoRow.test.ts |
Tests Auto-row interactions. | None. |
src/vs/workbench/contrib/chat/common/languageModels.ts |
Adds model demotion metadata. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelProviderIcons.ts |
Generalizes provider icon resolution. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerWidget.ts |
Integrates the experimental picker. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerWelcome.ts |
Adds empty-provider content. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerVariants.ts |
Groups speed variants. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerTelemetry.ts |
Centralizes configuration telemetry. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerTabs.ts |
Builds provider tabs and sections. | Critical: Guard failed promoted-model take() results to prevent crashes for version-gated models.Critical: Key destinations by stable provider identity as well as display label to prevent provider merging and duplicate tab IDs. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerTabbedWidget.ts |
Implements the tabbed picker. | Moderate: Add picker-specific accessibility help and focus restoration. Moderate: After disabling Auto, recompute destinations and activate the selected model’s destination while preserving switch focus. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerPresentation.ts |
Adds category presentation helpers. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerModelConfig.ts |
Extracts model configuration helpers. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerLineage.ts |
Derives model lineages. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerItems.ts |
Preserves demoted model metadata. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerItemPrimitives.ts |
Styles linked unavailable models. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerHover.ts |
Reuses model detail helpers. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerDetails.ts |
Extracts pricing and detail rendering. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerConfiguration.ts |
Reuses configuration and telemetry helpers. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerCard.ts |
Adds interactive model details. | Moderate: The description fallback checks the wrong child count and never renders; detect when the header is the sole child. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerBadges.ts |
Adds model status badges. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerAutoRow.ts |
Adds the Auto control. | Moderate: Route tier changes through onConfigurationChanged so thinking-effort telemetry is emitted. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/modelPickerActionItem.ts |
Moves the configuration interface. | None. |
src/vs/workbench/contrib/chat/browser/widget/input/modelPicker/media/modelPicker.css |
Styles the redesigned picker. | Moderate: Restore a visible keyboard-focus indicator on the search field. Nit: Use the base codicon size for the welcome glyph. |
src/vs/workbench/contrib/chat/browser/widget/input/chatModelConfigurationStore.ts |
Updates configuration imports. | None. |
src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts |
Registers the experimental setting. | None. |
src/vs/workbench/contrib/chat/browser/aiCustomization/pluginListWidget.ts |
Adopts the shared switch. | None. |
src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css |
Removes obsolete switch styling. | None. |
src/vs/sessions/test/browser/permissionPickerList.fixture.ts |
Updates the switch selector. | None. |
src/vs/platform/actionWidget/test/browser/tabbedActionListWidget.test.ts |
Tests dynamic popup classes. | None. |
src/vs/platform/actionWidget/test/browser/actionList.test.ts |
Tests switch and refresh behavior. | None. |
src/vs/platform/actionWidget/browser/tabbedActionListWidget.ts |
Adds tab actions, footers, and empty states. | Moderate: Exclude empty-state actions from list keyboard handling so Enter activates them. Moderate: Only set aria-pressed for actions that provide checked.Moderate: Focus the active tab rather than a detached list when rendering empty content. Moderate: Exclude submenu panels from list-navigation handling. |
src/vs/platform/actionWidget/browser/tabbedActionListWidget.css |
Styles tab-bar actions and footers. | None. |
src/vs/platform/actionWidget/browser/actionWidget.css |
Removes obsolete toggle styling. | None. |
src/vs/platform/actionWidget/browser/actionList.ts |
Adds expandable lazy hover panels. | Moderate: Add expanded-popup semantics to the trigger and a named role to the focused detail panel, updating state on close. Nit: Split the joined declaration and conditional into separate statements. |
src/vs/base/test/browser/domSanitize.test.ts |
Adds stale-policy coverage. | Moderate: The cached policy prevents the throwing-policy path from being tested; use a fresh sanitizer or replace the cached policy through a test hook. |
src/vs/base/browser/ui/toggle/switch.ts |
Introduces a shared switch widget. | None. |
src/vs/base/browser/ui/toggle/switch.css |
Styles the shared switch. | Moderate: Cover workbench .hc-black and .hc-light themes while retaining the webview selector so switches keep their contrast border. |
src/vs/base/browser/ui/radio/radio.ts |
Adds ARIA and keyboard behavior. | None. |
src/vs/base/browser/ui/radio/radio.css |
Adds segmented-radio styling. | None. |
src/vs/base/browser/domSanitize.ts |
Recovers from stale Trusted Types policies. | None. |
Suppressed comments (1)
src/vs/platform/actionWidget/browser/actionList.ts:864
- The declaration and conditional were accidentally joined onto one line, making this accessibility-label branch easy to miss and violating the file's statement layout. Split them into separate statements.
const hoverText = typeof hoverContent === 'string' ? hoverContent : isMarkdownString(hoverContent) ? hoverContent.value : dom.isHTMLElement(hoverContent) ? hoverContent.textContent ?? undefined : undefined; if (hoverText && (!element.detail || stripNewlines(element.detail) !== stripNewlines(hoverText))) {
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Screenshot ChangesBase: Changed (52)4 insignificant change(s) omitted (≤20 px, Δ≤2). See CI logs for details. Added (48) |
The stale-policy replacement was cast to the global `TrustedTypePolicy`, but dompurify's config types that property with the `TrustedTypePolicy` imported from `trusted-types/lib/index.js`. The two normally unify, so the regular build accepts it. The editor tree shaker compiles against a synthetic root where `trusted-types` resolves through both a relative and an absolute path, leaving two declarations whose private `brand` makes them nominally incompatible, and `editor-distro` failed to compile. Name the property's own type instead, so only one declaration is involved however `trusted-types` resolves. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`_renderLabel` runs from a resize-driven autorun. Measuring the chip there cleared `minWidth`, read `scrollWidth`, then wrote `minWidth` again, so every ResizeObserver pass dirtied layout twice and never settled. The NewSessionCompactAutoModel fixture failed to render with "ResizeObserver loop completed with undelivered notifications". Take the fixed widths from main instead, including the narrower floor for the Auto label that #334128 added alongside that fixture. This resolves the merge conflict in this hunk the other way; the measured floor read better in the abstract, but it cannot be computed from inside the resize callback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The plugin list moved to the shared `Switch` widget and the hand-rolled `.plugin-enable-switch` CSS was deleted with it, but the MCP server list still built that markup by hand. With no rules left to match, its switches rendered as bare unstyled buttons: the McpServersTab fixture lost the filled pill and its thumb entirely. Move the MCP list onto `Switch` too, so both lists share one control. This is what the screenshot diff on this PR was reporting; accepting those hashes would have pinned the unstyled rendering as the baseline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>



Redesigns the model picker from the ground up behind an experimental setting so we can A/B test it.
Implements #333609