fix: prevent BYOK model from conflicting with same-named Copilot model in picker

com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilderTest.java

@@ -0,0 +1,81 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+package com.microsoft.copilot.eclipse.ui.chat;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import org.junit.jupiter.api.Test;
+
+import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel;
+import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotScope;
+import com.microsoft.copilot.eclipse.ui.swt.DropdownItem;
+import com.microsoft.copilot.eclipse.ui.swt.DropdownItemGroup;
+
+class ModelPickerGroupsBuilderTest {
+
+  private static CopilotModel buildNativeModel(String id, String name) {
+    CopilotModel model = new CopilotModel();
+    model.setId(id);
+    model.setModelName(name);
+    model.setScopes(List.of(CopilotScope.CHAT_PANEL, CopilotScope.AGENT_PANEL));
+    return model;
+  }
+
+  private static CopilotModel buildByokModel(String id, String name, String provider) {
+    CopilotModel model = new CopilotModel();
+    model.setId(id);
+    model.setModelName(name);
+    model.setProviderName(provider);
+    model.setScopes(List.of(CopilotScope.CHAT_PANEL, CopilotScope.AGENT_PANEL));
+    return model;
+  }
+
+  @Test
+  void twoModelsWithSameNameProduceDistinctDropdownItemIds() {
+    CopilotModel nativeModel = buildNativeModel("gpt-4", "GPT-4");
+    CopilotModel byokModel = buildByokModel("gpt-4", "GPT-4", "Azure");
+
+    Map<String, CopilotModel> modelMap = new HashMap<>();
+    modelMap.put(nativeModel.getModelKey(), nativeModel);
+    modelMap.put(byokModel.getModelKey(), byokModel);
+
+    List<DropdownItemGroup> groups = ModelPickerGroupsBuilder.build(modelMap, false, false, null);
+
+    List<String> ids = groups.stream()
+        .flatMap(g -> g.getItems().stream())
+        .map(DropdownItem::getId)
+        .collect(Collectors.toList());
+
+    assertEquals(2, ids.size(), "Expected two dropdown items for two models");
+    assertNotEquals(ids.get(0), ids.get(1), "Items sharing a model name must have distinct IDs");
+    assertEquals(nativeModel.getModelKey(), ids.stream()
+        .filter(id -> id.equals(nativeModel.getModelKey())).findFirst().orElse(null));
+    assertEquals(byokModel.getModelKey(), ids.stream()
+        .filter(id -> id.equals(byokModel.getModelKey())).findFirst().orElse(null));
+  }
+
+  @Test
+  void dropdownItemIdIsModelKeyNotModelName() {
+    CopilotModel model = buildNativeModel("claude-3-5-sonnet", "Claude 3.5 Sonnet");
+
+    Map<String, CopilotModel> modelMap = new HashMap<>();
+    modelMap.put(model.getModelKey(), model);
+
+    List<DropdownItemGroup> groups = ModelPickerGroupsBuilder.build(modelMap, false, false, null);
+
+    List<DropdownItem> items = groups.stream()
+        .flatMap(g -> g.getItems().stream())
+        .collect(Collectors.toList());
+
+    assertEquals(1, items.size());
+    assertEquals(model.getModelKey(), items.get(0).getId());
+    assertEquals(model.getModelName(), items.get(0).getLabel());
+  }
+}

com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java

@@ -16,7 +16,6 @@
 import java.util.regex.Pattern;
 
 import org.apache.commons.lang3.StringUtils;
-import org.eclipse.e4.core.services.events.IEventBroker;
 import org.eclipse.lsp4j.WorkDoneProgressKind;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.graphics.Font;
@@ -26,24 +25,18 @@
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
 import org.eclipse.swt.widgets.ScrollBar;
-import org.eclipse.ui.PlatformUI;
-
 import com.microsoft.copilot.eclipse.core.CopilotCore;
 import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants;
 import com.microsoft.copilot.eclipse.core.lsp.protocol.AgentRound;
 import com.microsoft.copilot.eclipse.core.lsp.protocol.AgentToolCall;
 import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatProgressValue;
-import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel;
 import com.microsoft.copilot.eclipse.core.lsp.protocol.TodoItem;
 import com.microsoft.copilot.eclipse.core.lsp.protocol.ToolSpecificData;
-import com.microsoft.copilot.eclipse.core.lsp.protocol.quota.CheckQuotaResult;
-import com.microsoft.copilot.eclipse.core.lsp.protocol.quota.CopilotPlan;
 import com.microsoft.copilot.eclipse.ui.CopilotUi;
 import com.microsoft.copilot.eclipse.ui.chat.services.ChatServiceManager;
 import com.microsoft.copilot.eclipse.ui.chat.services.TodoListService;
 import com.microsoft.copilot.eclipse.ui.i18n.Messages;
 import com.microsoft.copilot.eclipse.ui.swt.CssConstants;
-import com.microsoft.copilot.eclipse.ui.utils.MenuUtils;
 import com.microsoft.copilot.eclipse.ui.utils.SwtUtils;
 
 /**
@@ -304,51 +297,7 @@ private void doProcessTurnEvent(ChatProgressValue value) {
       errMsg = Messages.chat_model_unsupported_message;
     }
     if (StringUtils.isNotEmpty(errMsg)) {
-      // TODO: Remove this legacy fallback after TBB is officially released.
-      // When the language server has not enabled token-based billing yet, fall back to the
-      // original main-branch 402 behavior: replace the message with a plan-driven fallback
-      // notice, switch to the fallback model, refresh quota, and replay the previous input.
-      CheckQuotaResult quotaStatus = this.serviceManager.getAuthStatusManager().getQuotaStatus();
-      CopilotModel fallbackModel = null;
-      if (!quotaStatus.tokenBasedBillingEnabled() && value.getCode() == 402) {
-        CopilotPlan userPlan = quotaStatus.copilotPlan();
-        fallbackModel = this.serviceManager.getModelService().getFallbackModel();
-        String fallbackModelName = fallbackModel != null ? fallbackModel.getModelName()
-            : Messages.chat_noQuotaView_fallbackModel;
-
-        if (MenuUtils.isCfiPlan(userPlan)) {
-          // Pro, Pro+ and Max message
-          errMsg = String.format(Messages.chat_noQuotaView_proProplusWarnMsg, fallbackModelName);
-        } else if (userPlan == CopilotPlan.business || userPlan == CopilotPlan.enterprise) {
-          // CE and CB message
-          errMsg = String.format(Messages.chat_noQuotaView_cbCeWarnMsg, fallbackModelName);
-        }
-      }
-
       renderWarnMessageWithUpgradePlanButton(errMsg, value.getCode(), value.getErrorModelProviderName());
-
-      // TODO: Remove this legacy fallback after TBB is officially released.
-      // Only replay the previous input when a fallback model is actually available; otherwise
-      // setFallBackModelAsActiveModel() is a no-op and re-posting the input with the same
-      // active model would just trigger the same 402 again.
-      if (!quotaStatus.tokenBasedBillingEnabled() && value.getCode() == 402
-          && quotaStatus.copilotPlan() != CopilotPlan.free
-          && fallbackModel != null) {
-        // Detach the failed turn so the replayed response creates a new Copilot turn below the
-        // warning, instead of streaming into the same turn that just rendered the warn widget.
-        this.latestTurnWidget = null;
-        this.latestCopilotTurn = null;
-
-        this.serviceManager.getModelService().setFallBackModelAsActiveModel();
-        this.serviceManager.getAuthStatusManager().checkQuota();
-
-        String previousInput = this.serviceManager.getUserPreferenceService().getPreviousInput(StringUtils.EMPTY);
-        if (StringUtils.isNotEmpty(previousInput)) {
-          IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class);
-          Map<String, Object> properties = Map.of("previousInput", previousInput, "needCreateUserTurn", false);
-          eventBroker.post(CopilotEventConstants.TOPIC_CHAT_ON_SEND, properties);
-        }
-      }
     }
   }
 

com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilder.java

@@ -128,7 +128,7 @@ private static List<DropdownItem> buildModelDropdownItems(List<CopilotModel> mod
       String selectedLabel = StringUtils.isNotBlank(effortLevel) && StringUtils.isNotBlank(name)
           ? name + " - " + effortLevel : null;
 
-      items.add(new DropdownItem.Builder().id(rawName).label(name).selectedLabel(selectedLabel).suffix(suffix)
+      items.add(new DropdownItem.Builder().id(model.getModelKey()).label(name).selectedLabel(selectedLabel).suffix(suffix)
           .icon(resolveModelIcon(model)).hoverProvider(new ModelHoverContentProvider(model)).build());
     }
     return items;

com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelService.java

@@ -58,7 +58,6 @@ public class ModelService extends ChatBaseService {
   private Map<String, CopilotModel> copilotModels = new HashMap<>();
   private Map<String, CopilotModel> registeredByokModels = new HashMap<>();
   private CopilotModel defaultModel;
-  private CopilotModel fallbackModel;
 
   private ChatMode currentChatMode = ChatMode.Agent;
 
@@ -152,8 +151,11 @@ private void initializeEventHandlers() {
         currentChatMode = ChatMode.Agent;
         updateModelsForChatMode(ChatMode.Agent);
 
-        // Then switch to the specified model (setActiveModel will be called after models are loaded)
-        setActiveModel(modelName);
+        // Resolve name to key, then activate
+        String modelKey = findModelKeyByName(modelName);
+        if (modelKey != null) {
+          setActiveModel(modelKey);
+        }
       }
     };
   }
@@ -202,7 +204,6 @@ private void fetchCopilotModels() throws InterruptedException, ExecutionExceptio
     CopilotModel[] modelArray = lsConnection.listModels().get();
     Map<String, CopilotModel> newModels = new HashMap<>();
     CopilotModel newDefaultModel = null;
-    CopilotModel newFallbackModel = null;
 
     for (CopilotModel model : modelArray) {
       boolean supportsChat = model.getScopes().contains(CopilotScope.CHAT_PANEL);
@@ -213,14 +214,10 @@ private void fetchCopilotModels() throws InterruptedException, ExecutionExceptio
       if (model.isChatDefault()) {
         newDefaultModel = model;
       }
-      if (model.isChatFallback()) {
-        newFallbackModel = model;
-      }
     }
 
     copilotModels = newModels;
     defaultModel = newDefaultModel;
-    fallbackModel = newFallbackModel;
   }
 
   private void fetchByokModels() throws InterruptedException, ExecutionException {
@@ -357,28 +354,35 @@ private void onDidCopilotStatusChange(CopilotStatusResult copilotStatusResult) {
     }
   }
 
+  // TODO(#261): if a BYOK model and a native model share the same modelName this returns the first
+  // match regardless of provider. Fix by extending the custom-mode event protocol to carry the
+  // composite key so the lookup can be unambiguous.
+  private String findModelKeyByName(String modelName) {
+    Map<String, CopilotModel> currentModels = modelObservable.getValue();
+    for (Map.Entry<String, CopilotModel> entry : currentModels.entrySet()) {
+      if (entry.getValue().getModelName().equals(modelName)) {
+        return entry.getKey();
+      }
+    }
+    return null;
+  }
+
   /**
-   * Set the active model by name.
+   * Set the active model by its composite key.
    *
-   * @param modelName the name of the model
+   * @param modelKey the composite key of the model
    */
-  public void setActiveModel(String modelName) {
-    Map<String, CopilotModel> currentModels = modelObservable.getValue();
-
-    final CopilotModel model = currentModels.values().stream()
-        .filter(candidateModel -> candidateModel.getModelName().equals(modelName))
-        .findFirst()
-        .orElse(null);
-    if (model != null) {
-      // Persist asynchronously to avoid deadlock: persistUserPreference() calls
-      // persistence().get() which blocks waiting for the LSP listener thread.
-      // If called on the UI thread while the listener is in syncExec, both threads
-      // deadlock.
-      persistModelSelection(model);
-
-      // Update observable
-      ensureRealm(() -> activeModelObservable.setValue(model));
+  public void setActiveModel(String modelKey) {
+    CopilotModel model = modelObservable.getValue().get(modelKey);
+    if (model == null) {
+      return;
     }
+    // Persist asynchronously to avoid deadlock: persistUserPreference() calls
+    // persistence().get() which blocks waiting for the LSP listener thread.
+    // If called on the UI thread while the listener is in syncExec, both threads
+    // deadlock.
+    persistModelSelection(model);
+    ensureRealm(() -> activeModelObservable.setValue(model));
   }
 
   /**
@@ -399,24 +403,6 @@ public Map<String, CopilotModel> getModels() {
     return modelObservable.getValue();
   }
 
-  /**
-   * Get the fallback model.
-   *
-   * @return the fallback model
-   */
-  public CopilotModel getFallbackModel() {
-    return fallbackModel;
-  }
-
-  /**
-   * Set the fallback model as the active model.
-   */
-  public void setFallBackModelAsActiveModel() {
-    if (fallbackModel != null) {
-      setActiveModel(fallbackModel.getModelName());
-    }
-  }
-
   /**
    * Check if the active model supports vision capabilities.
    */
@@ -568,7 +554,7 @@ public void bindModelPicker(final DropdownButton picker) {
             if (activeModel == null || picker.isDisposed()) {
               return;
             }
-            picker.setSelectedItemId(activeModel.getModelName());
+            picker.setSelectedItemId(activeModel.getModelKey());
             String suffix = StringUtils.isNotBlank(activeModel.getDegradationReason())
                 ? " - " + activeModel.getDegradationReason() : "";
             picker.setToolTipText(NLS.bind(Messages.chat_actionBar_modelPicker_Tooltip, suffix));

com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java

@@ -268,7 +268,7 @@ public void mouseDown(MouseEvent e) {
         // to update its label/suffix so the dropdown control reflects the (model, effort) pair the user just
         // chose -- even when they clicked an effort on a non-active model.
         modelService.setSelectedReasoningEffort(model, effort);
-        modelService.setActiveModel(model.getModelName());
+        modelService.setActiveModel(model.getModelKey());
         // Close the entire dropdown (hover + main popup) via the host-provided callback so the user sees an
         // immediate dismiss. Next time the dropdown opens, refreshBoundModelPickers (invoked from
         // setSelectedReasoningEffort) has updated the model row's suffix to reflect the newly selected effort.