Chat window model picker enhancements for multiple providers#314865
Draft
Chat window model picker enhancements for multiple providers#314865
Conversation
… for group separationj
Contributor
There was a problem hiding this comment.
Pull request overview
Enhances the chat model picker UX to better support multiple language model providers by grouping “Other Models” by vendor, optionally showing vendor labels inline for promoted items, and preserving provider grouping context during search/filtering.
Changes:
- Add vendor display-name resolution and use it to group “Other Models” with titled vendor separators.
- Show an inline vendor label for promoted items when multiple vendors exist, and suppress duplicate vendor info in the detail/description.
- Update the generic ActionListWidget filtering behavior to keep labeled separators visible above matching results; add CSS + tests.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts | Adds vendor display-name lookup, vendor-grouped “Other Models” separators, and inline vendor labels for promoted items. |
| src/vs/platform/actionWidget/browser/actionList.ts | Adjusts filtering to preserve labeled separators above matching results; updates separator row height handling. |
| src/vs/platform/actionWidget/browser/actionWidget.css | Styles titled separators (separators with non-empty labels). |
| src/vs/workbench/contrib/chat/browser/widget/media/chat.css | Tweaks flex behavior so inline vendor labels fit beside promoted model titles. |
| src/vs/workbench/contrib/chat/test/browser/widget/input/chatModelPicker.test.ts | Updates/extends tests for vendor grouping, header omission for single vendor, and inline vendor label behavior. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/chat/test/browser/widget/input/chatModelPicker.test.ts:468
- The comment says vendors are "sorted alphabetically", but the implementation sorts
copilotfirst and then alphabetically by display name for the rest. Updating the comment to match the actual ordering will prevent confusion (and makes it clearer why 'Copilot' is expected first here).
// Vendors sorted alphabetically by display name
assert.strictEqual(vendorSeparators[0].label, 'Copilot');
assert.strictEqual(vendorSeparators[1].label, 'Other-vendor');
// Models within each vendor group are sorted alphabetically
- Files reviewed: 5/5 changed files
- Comments generated: 5
Comment on lines
822
to
827
| if (item.kind === ActionListItemKind.Separator) { | ||
| if (isFiltering) { | ||
| if (labeledSeparatorsToShow.has(item)) { | ||
| visible.push(item); | ||
| } | ||
| continue; |
Comment on lines
+783
to
+801
| for (let i = 0; i < this._allMenuItems.length; i++) { | ||
| const item = this._allMenuItems[i]; | ||
| if (item.kind !== ActionListItemKind.Separator || !item.label) { | ||
| continue; | ||
| } | ||
| for (let j = i + 1; j < this._allMenuItems.length; j++) { | ||
| const next = this._allMenuItems[j]; | ||
| if (next.kind === ActionListItemKind.Separator) { | ||
| break; | ||
| } | ||
| if (next.kind === ActionListItemKind.Action && !next.isSectionToggle && !next.showAlways) { | ||
| const label = (next.label ?? '').toLowerCase(); | ||
| const descValue = typeof next.description === 'string' ? next.description : next.description?.value ?? ''; | ||
| if (label.includes(filterLower) || descValue.toLowerCase().includes(filterLower)) { | ||
| labeledSeparatorsToShow.add(item); | ||
| break; | ||
| } | ||
| } | ||
| } |
Comment on lines
+142
to
147
| description, | ||
| group: { title: '', icon: action.icon ?? ThemeIcon.fromId(action.checked ? Codicon.check.id : Codicon.blank.id) }, | ||
| hideIcon: false, | ||
| section: action.section, | ||
| className: vendorLabel ? 'chat-model-recently-used' : undefined, | ||
| hover: hoverContent ? { content: hoverContent } : undefined, |
Comment on lines
+2077
to
+2082
| /* Recently used model items: show vendor label inline next to the model name. */ | ||
| .action-widget .monaco-list-row.chat-model-recently-used .title { | ||
| flex: 0 1 auto; | ||
| } | ||
|
|
||
| .action-widget .monaco-list-row.chat-model-recently-used .description { |
| }; | ||
|
|
||
| const stubLanguageModelsService = { getModelConfigurationActions: () => [], getModelConfiguration: () => undefined } as unknown as ILanguageModelsService; | ||
| const stubLanguageModelsService = { getModelConfigurationActions: () => [], getModelConfiguration: () => undefined, getVendors: () => [] } as unknown as ILanguageModelsService; |
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.
Groups models in the "Other Models" section by providers using titled separator headers.
Changes