Skip to content

Fix model picker to handle metadata.id collisions between vendors#317170

Draft
vritant24 wants to merge 1 commit into
mainfrom
agents/coastal-cicada
Draft

Fix model picker to handle metadata.id collisions between vendors#317170
vritant24 wants to merge 1 commit into
mainfrom
agents/coastal-cicada

Conversation

@vritant24
Copy link
Copy Markdown
Member

No description provided.

Copilot AI review requested due to automatic review settings May 18, 2026 21:06
@vritant24 vritant24 added this to the 1.122.0 milestone May 18, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the chat model picker collision handling so models from different vendors that share the same metadata.id are not hidden from the grouped picker.

Changes:

  • Separates placement tracking for globally unique model identifiers vs control-manifest ids.
  • Preserves the first metadata-id lookup entry for legacy/id-based resolution.
  • Adds regression tests for BYOK/Copilot metadata-id collisions.
Show a summary per file
File Description
src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts Adjusts grouped picker placement and Other Models filtering for metadata-id collisions.
src/vs/workbench/contrib/chat/test/browser/widget/input/chatModelPicker.test.ts Adds regression coverage for BYOK models sharing metadata ids with Copilot models.

Copilot's findings

Comments suppressed due to low confidence (2)

src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts:623

  • Surfacing colliding BYOK models in Other Models is incomplete because the rendering path below still looks up controlModels[model.metadata.id]. A Copilot control entry with a minVSCodeVersion for this id will disable the BYOK model as unavailable even though that manifest entry only describes copilot/<id>; Other Models should avoid applying Copilot-only control entries to non-Copilot models with the same metadata id.
			// Filter only by identifier — metadata.id is not unique across
			// vendors, so a BYOK model with the same metadata.id as an
			// already-placed Copilot model must still be surfaced here.
			otherModels = models.filter(m => !placedIdentifiers.has(m.identifier));

src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts:533

  • This promoted-model path still applies controlModels[model.metadata.id] after resolving by a full model identifier. If the selected or recent model is a BYOK model whose metadata id collides with a Copilot entry, the Copilot entry's version gate/unavailable metadata can disable or relabel the BYOK model; only apply metadata-id control entries to the Copilot model they describe.
				if (model && !placedIdentifiers.has(model.identifier)) {
					markModelPlaced(model);
					const entry = controlModels[model.metadata.id];
  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants