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

@@ -12,7 +12,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.custom.ScrolledComposite;
@@ -24,24 +23,18 @@
 import org.eclipse.swt.layout.GridLayout;
 import org.eclipse.swt.widgets.Composite;
 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;
 
 /**
@@ -254,51 +247,7 @@ public void processTurnEvent(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);
-          }
-        }
       }
     }, this);
   }

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

@@ -130,7 +130,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

@@ -60,7 +60,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;
 
@@ -146,8 +145,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);
+        }
       }
     };
   }
@@ -208,9 +210,6 @@ private void fetchCopilotModels() throws InterruptedException, ExecutionExceptio
       if (model.isChatDefault()) {
         defaultModel = model;
       }
-      if (model.isChatFallback()) {
-        fallbackModel = model;
-      }
     }
 
     copilotModels = newModels;
@@ -328,40 +327,37 @@ private void onDidCopilotStatusChange(CopilotStatusResult copilotStatusResult) {
     }
   }
 
-  /**
-   * Set the active model by name.
-   *
-   * @param modelName the name of the model
-   */
-  public void setActiveModel(String modelName) {
+  // 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();
-
-    // Find model by model name and get its composite key
-    String compositeKey = null;
-    final CopilotModel model;
-    CopilotModel foundModel = null;
-
     for (Map.Entry<String, CopilotModel> entry : currentModels.entrySet()) {
       if (entry.getValue().getModelName().equals(modelName)) {
-        compositeKey = entry.getKey();
-        foundModel = entry.getValue();
-        break;
+        return entry.getKey();
       }
     }
-    model = foundModel;
-    if (model != null && compositeKey != null) {
-      // Persist using the composite key for proper identification
-      UserPreference preference = getUserPreference();
-      preference.setChatModel(compositeKey);
-      // 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.
-      CompletableFuture.runAsync(this::persistUserPreference);
+    return null;
+  }
 
-      // Update observable
-      ensureRealm(() -> activeModelObservable.setValue(model));
+  /**
+   * Set the active model by its composite key.
+   *
+   * @param modelKey the composite key of the model
+   */
+  public void setActiveModel(String modelKey) {
+    CopilotModel model = modelObservable.getValue().get(modelKey);
+    if (model == null) {
+      return;
     }
+    UserPreference preference = getUserPreference();
+    preference.setChatModel(modelKey);
+    // 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.
+    CompletableFuture.runAsync(this::persistUserPreference);
+    ensureRealm(() -> activeModelObservable.setValue(model));
   }
 
   /**
@@ -382,24 +378,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.
    */
@@ -551,7 +529,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

@@ -278,7 +278,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.