implement configuring language models from the editor #286616

sandy081 · 2026-01-08T20:33:10Z

No description provided.

Copilot

Pull request overview

This pull request implements configuration support for language models directly from the editor, allowing users to add and manage multiple language model provider groups with custom configurations.

Key Changes:

Adds configuration schema support to language model providers via extension point
Implements a new configuration service for persisting provider groups in a JSON file
Extends the UI to support configuring, adding, and deleting provider groups
Integrates secret storage for sensitive configuration values

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
vscode.proposed.chatProvider.d.ts	Adds `PrepareLanguageModelChatModelOptions` interface and `provideLanguageModelChatInformation` method to support configuration
languageModels.ts	Implements configuration handling, secret storage integration, and methods for managing provider groups
languageModelsConfiguration.ts	Defines service interface for managing language model provider groups
languageModelsConfigurationService.ts	Implements file-based configuration persistence with JSON parsing and range tracking
chatModelsWidget.ts	Updates UI to support configure/delete actions and status display for provider groups
chatModelsViewModel.ts	Refactors view model to support grouped models with status entries
languageModelsConfiguration.test.ts	Adds tests for JSON configuration parsing
chatModelsViewModel.test.ts	Updates tests for refactored view model structure
languageModels.test.ts	Updates mock service with new methods
chat.contribution.ts	Registers new configuration service and contribution
chatLanguageModelActions.ts	Adds action for configuring language model groups
jsonSchema.ts	Adds `secret` property to JSON schema interface

Comments suppressed due to low confidence (3)

src/vs/workbench/contrib/chat/test/browser/languageModelsConfiguration.test.ts:53

Test assertion is incorrect. On line 52-53, the result is being compared as a single object with assert.deepStrictEqual(config, { foo: 'bar' }), but result is an array of parsed provider groups, not a configuration object. This should likely access the first element's configuration property, or the test setup is incorrect.

		const config = result;
		assert.deepStrictEqual(config, { foo: 'bar' });

src/vs/workbench/contrib/chat/common/languageModels.ts:404

The SECRET_KEY format string at line 404 uses $ as a literal character, but this could conflict with variable substitution in some contexts. The format '${input:{0}}' might be interpreted as a template literal pattern. Consider using a more distinctive format or escaping the dollar sign to avoid potential parsing issues.

	private static SECRET_KEY = '${input:{0}}';

src/vs/workbench/contrib/chat/common/languageModels.ts:962

The decodeSecretKey method at lines 957-962 uses a simplistic substring extraction that assumes the format ${input:KEY}. However, it doesn't validate the expected prefix before extracting, which could lead to incorrect parsing if the string doesn't match the expected format. Add validation to ensure the string starts with the expected prefix before extracting.

	private decodeSecretKey(secretInput: unknown): string | undefined {
		if (!isString(secretInput)) {
			return undefined;
		}
		return secretInput.substring(secretInput.indexOf(':') + 1, secretInput.length - 1);
	}

Copilot · 2026-01-08T20:44:35Z

src/vs/workbench/contrib/chat/common/languageModelsConfiguration.ts

+export interface ILanguageModelsProviderGroup extends IStringDictionary<unknown> {
+	readonly name: string;
+	readonly vendor: string;
+	readonly range?: IRange;


The interface ILanguageModelsProviderGroup at line 29 extends IStringDictionary<unknown> which allows any string keys, making it difficult to enforce type safety for the required name, vendor, and optional range properties. This design could lead to naming conflicts if configuration properties happen to be named 'name', 'vendor', or 'range'. Consider using a more explicit structure with a separate configuration property to hold the dynamic configuration values.

Suggested change

export interface ILanguageModelsProviderGroup extends IStringDictionary<unknown> {

readonly name: string;

readonly vendor: string;

readonly range?: IRange;

export interface ILanguageModelsProviderGroup {

readonly name: string;

readonly vendor: string;

readonly range?: IRange;

readonly configuration: IStringDictionary<unknown>;

Copilot · 2026-01-08T20:44:35Z

src/vs/workbench/contrib/chat/browser/chatManagement/media/chatModelsWidget.css

+.models-widget .models-table-container .monaco-table-tr.models-status-row .monaco-table-td .model-name-container .model-name .monaco-highlighted-label.error-status {
+	color: var(--vscode-errorForeground);
+}
+
+.models-widget .models-table-container .monaco-table-tr.models-status-row .monaco-table-td .model-name-container .model-name .monaco-highlighted-label.warning-status {


The CSS classes at lines 124-130 use compound selectors that are very specific (.models-widget .models-table-container .monaco-table-tr.models-status-row .monaco-table-td .model-name-container .model-name .monaco-highlighted-label.error-status). These deeply nested selectors make the CSS fragile and hard to maintain. Consider using more semantic class names or reducing specificity to improve maintainability.

Suggested change

.models-widget .models-table-container .monaco-table-tr.models-status-row .monaco-table-td .model-name-container .model-name .monaco-highlighted-label.error-status {

color: var(--vscode-errorForeground);

}

.models-widget .models-table-container .monaco-table-tr.models-status-row .monaco-table-td .model-name-container .model-name .monaco-highlighted-label.warning-status {

.models-widget .models-table-container .monaco-table-tr.models-status-row .monaco-highlighted-label.error-status {

color: var(--vscode-errorForeground);

}

.models-widget .models-table-container .monaco-table-tr.models-status-row .monaco-highlighted-label.warning-status {

Copilot · 2026-01-08T20:44:36Z

src/vs/workbench/contrib/chat/test/browser/languageModelsConfiguration.test.ts

+		const config = result[0];
+		assert.strictEqual(config.str, 'value');
+		assert.strictEqual(config.num, 123);
+		assert.strictEqual(config.bool, true);
+		assert.strictEqual(config.null, null);
+		assert.deepStrictEqual(config.arr, [1, 2]);
+		assert.deepStrictEqual(config.obj, { nested: 'val' });


Test assertions are accessing properties directly on config object (lines 96-102), but config appears to be the first element of the result array based on line 96. The property access should be from the correct nested structure (e.g., accessing configuration properties from the first group's configuration object).

This issue also appears in the following locations of the same file:

line 52

Copilot · 2026-01-08T20:44:36Z

src/vs/workbench/contrib/chat/test/browser/chatManagement/chatModelsViewModel.test.ts

+		assert.strictEqual(visibleModels[0].model.metadata.name, 'GPT-4');
+		assert.strictEqual(visibleModels[0].model.provider, 'copilot');

-		assert.strictEqual(visibleModels[1].modelEntry.metadata.name, 'GPT-4o');
-		assert.strictEqual(visibleModels[1].modelEntry.vendor, 'copilot');
+		assert.strictEqual(visibleModels[1].model.metadata.name, 'GPT-4o');
+		assert.strictEqual(visibleModels[1].model.provider, 'copilot');

-		assert.strictEqual(visibleModels[2].modelEntry.metadata.name, 'Claude 3');
-		assert.strictEqual(visibleModels[2].modelEntry.vendor, 'anthropic');
+		assert.strictEqual(visibleModels[2].model.metadata.name, 'Claude 3');
+		assert.strictEqual(visibleModels[2].model.provider, 'anthropic');

-		assert.strictEqual(visibleModels[3].modelEntry.metadata.name, 'GPT-3.5 Turbo');
-		assert.strictEqual(visibleModels[3].modelEntry.vendor, 'openai');
+		assert.strictEqual(visibleModels[3].model.metadata.name, 'GPT-3.5 Turbo');
+		assert.strictEqual(visibleModels[3].model.provider, 'openai');


Test assertions at lines 852-862 are accessing model.provider directly as a string (e.g., assert.strictEqual(visibleModels[0].model.provider, 'copilot')), but based on the type definitions, model.provider should be an ILanguageModelProvider object, not a string. These should likely be model.provider.vendor.vendor to access the vendor ID string.

See below for a potential fix:

assert.strictEqual(visibleModels[0].model.provider.vendor.vendor, 'copilot'); assert.strictEqual(visibleModels[1].model.metadata.name, 'GPT-4o'); assert.strictEqual(visibleModels[1].model.provider.vendor.vendor, 'copilot'); assert.strictEqual(visibleModels[2].model.metadata.name, 'Claude 3'); assert.strictEqual(visibleModels[2].model.provider.vendor.vendor, 'anthropic'); assert.strictEqual(visibleModels[3].model.metadata.name, 'GPT-3.5 Turbo'); assert.strictEqual(visibleModels[3].model.provider.vendor.vendor, 'openai');

Copilot · 2026-01-08T20:44:36Z

src/vs/workbench/contrib/chat/common/languageModels.ts

+				if (existing?.[property]) {
+					inputBox.value = String(existing?.[property]);


Potential null reference error: at lines 896-897, the code accesses existing?.[property] which could be undefined, but then calls String(existing?.[property]) on line 897. If existing[property] is null or undefined, this will convert it to the string "null" or "undefined" rather than an empty string, which is likely not the intended behavior for displaying the previous value in an input box.

Copilot · 2026-01-08T20:44:37Z

src/vs/workbench/contrib/chat/browser/chatManagement/chatModelsViewModel.ts

+				if (a.models[0]?.provider.vendor.vendor === 'copilot') { return -1; }
+				if (b.models[0]?.provider.vendor.vendor === 'copilot') { return 1; }


The condition at line 412 checks for copilot vendor and sorts it first with if (a.models[0]?.provider.vendor.vendor === 'copilot'), but if a.models is an empty array (which can happen when there's only a status entry), this will cause a.models[0] to be undefined, potentially leading to incorrect sorting behavior. Add a check to ensure the models array has elements before accessing the first element.

Suggested change

if (a.models[0]?.provider.vendor.vendor === 'copilot') { return -1; }

if (b.models[0]?.provider.vendor.vendor === 'copilot') { return 1; }

if (a.models.length > 0 && a.models[0]?.provider.vendor.vendor === 'copilot') { return -1; }

if (b.models.length > 0 && b.models[0]?.provider.vendor.vendor === 'copilot') { return 1; }

Copilot · 2026-01-08T20:44:38Z

src/vs/workbench/contrib/chat/common/languageModels.ts

+	async addLanguageModelsProviderGroup(name: string, vendorId: string, configuration: IStringDictionary<unknown> | undefined): Promise<void> {
+		const vendor = this.getVendors().find(({ vendor }) => vendor === vendorId);
+		if (!vendor) {
+			throw new Error(`Vendor ${vendorId} not found.`);
+		}
+
+		const languageModelProviderGroup = await this._resolveLanguageModelProviderGroup(name, vendorId, configuration, vendor.configuration);
+		await this._languageModelsConfigurationService.addLanguageModelsProviderGroup(languageModelProviderGroup);
+	}


Incomplete JSDoc comment: The method addLanguageModelsProviderGroup at lines 740-748 is a public API method but lacks JSDoc documentation explaining its parameters, purpose, and behavior. Given that this is part of the public service interface, proper documentation is essential for maintainability and API consumers.

Copilot · 2026-01-08T20:44:38Z

src/vs/workbench/contrib/chat/browser/languageModelsConfigurationService.ts

+		return result;
+	}
+
+	async updateLanguageModelsProviderGroup(toUpdate: ILanguageModelsProviderGroup): Promise<ILanguageModelsProviderGroup> {


The method signature in updateLanguageModelsProviderGroup only takes one parameter toUpdate, but the documentation comment and method call in languageModels.ts at line 732 suggest it should take two parameters (from and to). The signature should match the interface definition in languageModelsConfiguration.ts line 22 which declares updateLanguageModelsProviderGroup(from: ILanguageModelsProviderGroup, to: ILanguageModelsProviderGroup).

Copilot · 2026-01-08T20:44:38Z

src/vs/workbench/contrib/chat/common/languageModels.ts

-			description: localize('vscode.extension.contributes.languageModels.managementCommand', "A command to manage the language model chat provider, e.g. 'Manage Copilot models'. This is used in the chat model picker. If not provided, a gear icon is not rendered during vendor selection.")
+			description: localize('vscode.extension.contributes.languageModels.managementCommand', "A command to manage the language model chat provider, e.g. 'Manage Copilot models'. This is used in the chat model picker. If not provided, a gear icon is not rendered during vendor selection."),
+			deprecated: true,
+			deprecationMessage: localize('vscode.extension.contributes.languageModels.managementCommand.deprecated', "The managementCommand property is deprecated and will be removed in a future release. Use the new configuration property instead.")


The deprecation message at line 372 states "The managementCommand property is deprecated and will be removed in a future release." However, the code still relies heavily on checking managementCommand in multiple places (e.g., lines 711-714, 768). If this property is truly deprecated, there should be a clear migration path documented, and the deprecation timeline should be specified to help extension authors plan their updates.

Suggested change

deprecationMessage: localize('vscode.extension.contributes.languageModels.managementCommand.deprecated', "The managementCommand property is deprecated and will be removed in a future release. Use the new configuration property instead.")

deprecationMessage: localize('vscode.extension.contributes.languageModels.managementCommand.deprecated', "The managementCommand property is deprecated and will be removed in a future major release. Extension authors should instead describe all management options in the configuration schema and surface them through the configuration property (for example, using settings-based toggles or options) rather than relying on a dedicated management command.")

src/vs/workbench/contrib/chat/common/languageModels.ts

sandy081 added 10 commits December 1, 2025 18:58

improvements to manage models

d8fd83f

Merge branch 'main' into sandy081/llm-management

306e419

fix errors

7e3ea99

Merge branch 'main' into sandy081/llm-management

5b19ac5

Merge branch 'main' into sandy081/llm-management

9156db3

remove quality check

e8eba72

fix compilation

9f442d1

handle errors and display in table

61a2226

Merge branch 'main' into sandy081/llm-management

06a3e01

remove commented code

1946957

Copilot AI review requested due to automatic review settings January 8, 2026 20:33

sandy081 enabled auto-merge (squash) January 8, 2026 20:33

sandy081 self-assigned this Jan 8, 2026

Copilot started reviewing on behalf of sandy081 January 8, 2026 20:33 View session

vs-code-engineering bot added this to the January 2026 milestone Jan 8, 2026

Copilot AI reviewed Jan 8, 2026

View reviewed changes

lszomoru approved these changes Jan 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

implement configuring language models from the editor #286616

implement configuring language models from the editor #286616

sandy081 commented Jan 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		if (existing?.[property]) {
		inputBox.value = String(existing?.[property]);

		if (a.models[0]?.provider.vendor.vendor === 'copilot') { return -1; }
		if (b.models[0]?.provider.vendor.vendor === 'copilot') { return 1; }

implement configuring language models from the editor #286616

Are you sure you want to change the base?

implement configuring language models from the editor #286616

Conversation

sandy081 commented Jan 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants