diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilderTest.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilderTest.java new file mode 100644 index 00000000..4affa502 --- /dev/null +++ b/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 modelMap = new HashMap<>(); + modelMap.put(nativeModel.getModelKey(), nativeModel); + modelMap.put(byokModel.getModelKey(), byokModel); + + List groups = ModelPickerGroupsBuilder.build(modelMap, false, false, null); + + List 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 modelMap = new HashMap<>(); + modelMap.put(model.getModelKey(), model); + + List groups = ModelPickerGroupsBuilder.build(modelMap, false, false, null); + + List 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()); + } +} diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java index 7ef32077..94cadfda 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java +++ b/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 properties = Map.of("previousInput", previousInput, "needCreateUserTurn", false); - eventBroker.post(CopilotEventConstants.TOPIC_CHAT_ON_SEND, properties); - } - } } }, this); } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilder.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilder.java index fb838528..705ec253 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilder.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilder.java @@ -130,7 +130,7 @@ private static List buildModelDropdownItems(List 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; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelService.java index c76cd6b1..1d250571 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelService.java +++ b/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 copilotModels = new HashMap<>(); private Map 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 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 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 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)); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java index a1698ffe..e0690211 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java +++ b/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.