chat: refactor model picker delegate and improve picker UX#300436
Merged
chat: refactor model picker delegate and improve picker UX#300436
Conversation
…y useGroupedModelPicker logic
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the chat model picker delegate API to decouple “grouped layout” from “manage models” affordances, and improves the picker UX by supporting explicit hover positioning and allowing taller action list dropdowns.
Changes:
- Split
IModelPickerDelegate.canManageModels()intouseGroupedModelPicker()andshowManageModelsAction(), updating callers in workbench chat input and Sessions chat. - Thread
hoverPositionthrough the enhanced model picker widget so item hovers can be positioned consistently. - Increase the platform
ActionListdropdown max height ratio to better accommodate longer grouped pickers.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/browser/widget/input/modelPickerActionItem2.ts | Passes hoverPosition into ModelPickerWidget during instantiation. |
| src/vs/workbench/contrib/chat/browser/widget/input/modelPickerActionItem.ts | Refactors IModelPickerDelegate to split grouped layout vs manage action visibility. |
| src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts | Adds hover-position support, updates delegate usage, and restructures grouped vs flat item building. |
| src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts | Updates delegate implementation to always use grouped picker and gate “Manage Models...” by session type. |
| src/vs/sessions/contrib/chat/browser/newChatViewPane.ts | Updates Sessions’ delegate to use grouped picker but disable the manage action; sets hover position above. |
| src/vs/platform/actionWidget/browser/actionList.ts | Raises dropdown viewport max height cap from 40% to 60%. |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts:377
- The flat-list branch (
useGroupedModelPicker === false) ignores themanageModelsActionparameter entirely. With the new delegate split (useGroupedModelPicker()vsshowManageModelsAction()), this means callers cannot request a flat list and a “Manage Models...” entry even though the API suggests these are independent. Either rendermanageModelsActionin the flat-list path as well, or document/enforce that it only applies to the grouped picker.
} else {
// Flat list: auto first, then all models sorted alphabetically
const autoModel = models.find(m => m.metadata.id === 'auto' && m.metadata.vendor === 'copilot');
if (autoModel) {
items.push(createModelItem(createModelAction(autoModel, selectedModelId, onSelect), autoModel, hoverPosition));
}
const sortedModels = models
.filter(m => m !== autoModel)
.sort((a, b) => {
const vendorCmp = a.metadata.vendor.localeCompare(b.metadata.vendor);
return vendorCmp !== 0 ? vendorCmp : a.metadata.name.localeCompare(b.metadata.name);
});
for (const model of sortedModels) {
items.push(createModelItem(createModelAction(model, selectedModelId, onSelect), model, hoverPosition));
}
}
Fixes test failure where empty models list didn't include the manage models entry because it was nested inside the if (models.length) block. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lramos15
approved these changes
Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors
IModelPickerDelegateto splitcanManageModels()into two independent flags and improves the model picker UX with hover position support and increased max height.Changes
IModelPickerDelegaterefactoringuseGroupedModelPicker()— controls whether the picker uses grouped layout (promoted/featured/other sections) vs flat listshowManageModelsAction()— controls whether the "Manage Models..." link appearsThese were previously conflated in a single
canManageModels()method. Now they can be independently controlled per caller:chatInputPart.ts: always uses grouped picker (true), manage action gated by session typenewChatViewPane.ts: always uses grouped picker (true), manage action disabled (false)buildModelPickerItemsrestructuringcanManageModelsparameter withuseGroupedModelPickerHover position support
ModelPickerWidgetnow acceptshoverPositionand threads it through tobuildModelPickerItems,createModelItem, andcreateUnavailableModelItemEnhancedModelPickerActionItempassespickerOptions.hoverPositionto the widgetAction list max height