refactor: Picking copilot model restarts via configuration change hook

See discussion in #1053

Author: dustinbyrne

src/commands/pickCopilotModel.ts

@@ -26,7 +26,6 @@ export default class PickCopilotModelCommand {
     );
     if (!model) return;
 
-    await ExtensionSettings.setPreferredCopilotModel(model.details);
-    return vscode.commands.executeCommand('appmap.rpc.restart');
+    return ExtensionSettings.setPreferredCopilotModel(model.details);
   }
 }

src/services/chatCompletion.ts

@@ -120,12 +120,16 @@ export default class ChatCompletion implements Disposable {
     return this.models[0];
   }
 
+  // This is used to determine whether the preferred model has changed upon refreshing the models.
+  private static previousModelId: string | undefined;
+
   static async refreshModels(): Promise<boolean> {
-    const previousBest = this.preferredModel?.id;
     this._models = (await vscode.lm.selectChatModels()).sort(
       (a, b) => b.maxInputTokens - a.maxInputTokens + b.family.localeCompare(a.family)
     );
-    return this.preferredModel?.id !== previousBest;
+    const hasChanged = this.previousModelId !== this.preferredModel?.id;
+    this.previousModelId = this.preferredModel?.id;
+    return hasChanged;
   }
 
   static get instance(): Promise<ChatCompletion> | undefined {
@@ -238,7 +242,8 @@ export default class ChatCompletion implements Disposable {
     context.subscriptions.push(
       vscode.workspace.onDidChangeConfiguration(
         (e) =>
-          e.affectsConfiguration('appMap.navie.useVSCodeLM') &&
+          (e.affectsConfiguration('appMap.navie.useVSCodeLM') ||
+            e.affectsConfiguration('appMap.copilot.preferredModel')) &&
           this.checkConfiguration(context, true)
       )
     );

test/unit/commands/pickCopilotModel.test.ts

@@ -2,34 +2,43 @@ import vscode from '../mock/vscode';
 import sinon from 'sinon';
 import { expect } from 'chai';
 import PickCopilotModelCommand from '../../../src/commands/pickCopilotModel';
+import { addMockChatModel, resetModelMocks } from '../mock/vscode/lm';
+import { LanguageModelChat } from 'vscode';
 
 describe('pickCopilotModel', () => {
   describe('execute', () => {
-    let models: Record<string, unknown>[];
+    let models: LanguageModelChat[];
     let executeCommandStub: sinon.SinonStub;
     let showQuickPickStub: sinon.SinonStub;
     let showErrorMessageStub: sinon.SinonStub;
     const chosenModel = 'claude-3.5-sonnet';
     beforeEach(() => {
       models = [
-        { id: 'gpt-4o', name: 'GPT-4o', maxInputTokens: 325, family: 'copilot' },
+        {
+          id: 'gpt-4o',
+          name: 'GPT-4o',
+          maxInputTokens: 325,
+          family: 'copilot',
+        } as LanguageModelChat,
         {
           id: 'claude-3.5-sonnet',
           name: 'Claude 3.5 Sonnet',
           maxInputTokens: 325,
           family: 'copilot',
-        },
+        } as LanguageModelChat,
       ];
       showQuickPickStub = sinon.stub(vscode.window, 'showQuickPick').callsFake(() => ({
         details: chosenModel,
       }));
       showErrorMessageStub = sinon.stub(vscode.window, 'showErrorMessage').resolves();
       executeCommandStub = sinon.stub(vscode.commands, 'executeCommand').resolves();
       sinon.stub(vscode.lm, 'selectChatModels').callsFake(() => models as any); // eslint-disable-line @typescript-eslint/no-explicit-any
+      models.forEach((m) => addMockChatModel(m));
     });
 
     afterEach(() => {
       sinon.restore();
+      resetModelMocks();
     });
 
     const setPreferredModel = (model: string | undefined) =>
@@ -51,11 +60,6 @@ describe('pickCopilotModel', () => {
       expect(executeCommandStub.called).to.be.false;
     });
 
-    it('restarts the RPC server', async () => {
-      await PickCopilotModelCommand.execute();
-      expect(executeCommandStub.calledWith('appmap.rpc.restart')).to.be.true;
-    });
-
     it('does nothing if the user cancels the quick pick', async () => {
       showQuickPickStub.resolves(undefined);
       setPreferredModel(chosenModel);