From 785cf14deb4979a153b5573973f90c8b13038e35 Mon Sep 17 00:00:00 2001 From: rsd-darshan Date: Wed, 20 May 2026 21:31:51 +0545 Subject: [PATCH 1/7] fix: prevent BYOK model from conflicting with same-named Copilot model in picker When a BYOK model is named identically to a Copilot-native model (e.g. GPT-4.1), both dropdown items received the same ID, causing the picker to highlight both as selected simultaneously. Use a provider-prefixed picker ID for BYOK models so each item has a unique identity, and update the selection and lookup logic to match. Fixes #167 --- .../eclipse/ui/chat/ModelPickerGroupsBuilder.java | 2 +- .../eclipse/ui/chat/services/ModelService.java | 4 ++-- .../copilot/eclipse/ui/utils/ModelUtils.java | 11 +++++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) 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 035a429f..364992c1 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 @@ -102,7 +102,7 @@ private static List buildModelDropdownItems(List mod List items = new ArrayList<>(); for (CopilotModel model : models) { String suffix = ModelUtils.getModelSuffix(model); - items.add(new DropdownItem.Builder().id(model.getModelName()).label(model.getModelName()).suffix(suffix) + items.add(new DropdownItem.Builder().id(ModelUtils.getPickerId(model)).label(model.getModelName()).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 5d1c74dc..f60de900 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 @@ -338,7 +338,7 @@ public void setActiveModel(String modelName) { CopilotModel foundModel = null; for (Map.Entry entry : currentModels.entrySet()) { - if (entry.getValue().getModelName().equals(modelName)) { + if (ModelUtils.getPickerId(entry.getValue()).equals(modelName)) { compositeKey = entry.getKey(); foundModel = entry.getValue(); break; @@ -444,7 +444,7 @@ public void bindModelPicker(final DropdownButton picker) { if (activeModel == null || picker.isDisposed()) { return; } - picker.setSelectedItemId(activeModel.getModelName()); + picker.setSelectedItemId(ModelUtils.getPickerId(activeModel)); 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/utils/ModelUtils.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java index d6e9b823..11a7df10 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java @@ -24,6 +24,17 @@ private ModelUtils() { // Private constructor to prevent instantiation } + /** + * Returns a unique picker ID for the given model. BYOK models include the provider name as a + * prefix to avoid collisions with Copilot-native models that share the same display name. + */ + public static String getPickerId(CopilotModel model) { + if (model.getProviderName() != null) { + return model.getProviderName() + "_" + model.getModelName(); + } + return model.getModelName(); + } + /** * Convert ByokModel to CopilotModel format for unified handling. */ From cf85ebe8ec12871a06e3ea32221030b94be86171 Mon Sep 17 00:00:00 2001 From: rsd-darshan Date: Wed, 20 May 2026 22:32:25 +0545 Subject: [PATCH 2/7] fix: support both picker ID and model name in setActiveModel lookup Existing internal callers pass plain model names (e.g. fallback model, custom mode event). Match by picker ID first, then fall back to model name so both call patterns keep working. --- .../copilot/eclipse/ui/chat/services/ModelService.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 f60de900..696aeda6 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 @@ -338,9 +338,10 @@ public void setActiveModel(String modelName) { CopilotModel foundModel = null; for (Map.Entry entry : currentModels.entrySet()) { - if (ModelUtils.getPickerId(entry.getValue()).equals(modelName)) { + CopilotModel candidate = entry.getValue(); + if (ModelUtils.getPickerId(candidate).equals(modelName) || candidate.getModelName().equals(modelName)) { compositeKey = entry.getKey(); - foundModel = entry.getValue(); + foundModel = candidate; break; } } @@ -392,7 +393,7 @@ public CopilotModel getFallbackModel() { */ public void setFallBackModelAsActiveModel() { if (fallbackModel != null) { - setActiveModel(fallbackModel.getModelName()); + setActiveModel(ModelUtils.getPickerId(fallbackModel)); } } From a8f7a38bc0fe07bab749d1d9a667896499ee4fee Mon Sep 17 00:00:00 2001 From: rsd-darshan Date: Thu, 21 May 2026 10:05:05 +0545 Subject: [PATCH 3/7] refactor: use existing getModelKey() for picker ID instead of custom concatenation Replaces the custom providerName + "_" + modelName concatenation with the model's existing getModelKey() method, which already produces a stable composite key used throughout the codebase. This avoids potential collisions from naive string concatenation and eliminates the blank-provider-name edge case. --- .../microsoft/copilot/eclipse/ui/utils/ModelUtils.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java index 425d18f7..f7ec3c38 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java @@ -29,14 +29,12 @@ private ModelUtils() { } /** - * Returns a unique picker ID for the given model. BYOK models include the provider name as a - * prefix to avoid collisions with Copilot-native models that share the same display name. + * Returns a unique picker ID for the given model. Uses the model's existing composite key + * (providerName_id for BYOK, id for native) to avoid collisions when a BYOK model shares + * the same display name as a Copilot-native model. */ public static String getPickerId(CopilotModel model) { - if (model.getProviderName() != null) { - return model.getProviderName() + "_" + model.getModelName(); - } - return model.getModelName(); + return model.getModelKey(); } /** From e5d875c584fd6fc2cf3712c940751c3da821a3a3 Mon Sep 17 00:00:00 2001 From: rsd-darshan Date: Thu, 21 May 2026 14:26:38 +0545 Subject: [PATCH 4/7] refactor: Remove unused message keys and properties across various modules (#208) Replace getPickerId() wrapper with direct model.getModelKey() calls at each use site, removing the unnecessary indirection. --- .../eclipse/ui/chat/ModelPickerGroupsBuilder.java | 2 +- .../copilot/eclipse/ui/chat/services/ModelService.java | 6 +++--- .../microsoft/copilot/eclipse/ui/utils/ModelUtils.java | 9 --------- 3 files changed, 4 insertions(+), 13 deletions(-) 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 96dc73a5..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(ModelUtils.getPickerId(model)).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 e9d1f490..20331df8 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 @@ -343,7 +343,7 @@ public void setActiveModel(String modelName) { for (Map.Entry entry : currentModels.entrySet()) { CopilotModel candidate = entry.getValue(); - if (ModelUtils.getPickerId(candidate).equals(modelName) || candidate.getModelName().equals(modelName)) { + if (candidate.getModelKey().equals(modelName) || candidate.getModelName().equals(modelName)) { compositeKey = entry.getKey(); foundModel = candidate; break; @@ -397,7 +397,7 @@ public CopilotModel getFallbackModel() { */ public void setFallBackModelAsActiveModel() { if (fallbackModel != null) { - setActiveModel(ModelUtils.getPickerId(fallbackModel)); + setActiveModel(fallbackModel.getModelKey()); } } @@ -552,7 +552,7 @@ public void bindModelPicker(final DropdownButton picker) { if (activeModel == null || picker.isDisposed()) { return; } - picker.setSelectedItemId(ModelUtils.getPickerId(activeModel)); + 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/utils/ModelUtils.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java index f7ec3c38..e7d18484 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java @@ -28,15 +28,6 @@ private ModelUtils() { // Private constructor to prevent instantiation } - /** - * Returns a unique picker ID for the given model. Uses the model's existing composite key - * (providerName_id for BYOK, id for native) to avoid collisions when a BYOK model shares - * the same display name as a Copilot-native model. - */ - public static String getPickerId(CopilotModel model) { - return model.getModelKey(); - } - /** * Convert ByokModel to CopilotModel format for unified handling. */ From b9366cd72df8895bf4c1d94b2cd2a1f362833c74 Mon Sep 17 00:00:00 2001 From: rsd-darshan Date: Fri, 22 May 2026 12:56:43 +0545 Subject: [PATCH 5/7] fix: make setActiveModel key-only and fix remaining name callers - setActiveModel now accepts only a composite model key (modelKey), matching the picker ID used throughout the codebase - Custom mode event handler resolves the model name to a key via findModelKeyByName() before calling setActiveModel - ModelHoverContentProvider uses model.getModelKey() directly instead of model.getModelName() --- .../ui/chat/services/ModelService.java | 34 ++++++++++++------- .../ui/swt/ModelHoverContentProvider.java | 2 +- 2 files changed, 22 insertions(+), 14 deletions(-) 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 20331df8..0cb4708b 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 @@ -146,8 +146,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); + } } }; } @@ -328,26 +331,31 @@ private void onDidCopilotStatusChange(CopilotStatusResult copilotStatusResult) { } } + private String findModelKeyByName(String modelName) { + Map currentModels = modelObservable.getValue(); + for (Map.Entry 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) { + public void setActiveModel(String modelKey) { 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()) { - CopilotModel candidate = entry.getValue(); - if (candidate.getModelKey().equals(modelName) || candidate.getModelName().equals(modelName)) { - compositeKey = entry.getKey(); - foundModel = candidate; - break; - } + if (currentModels.containsKey(modelKey)) { + compositeKey = modelKey; + foundModel = currentModels.get(modelKey); } model = foundModel; if (model != null && compositeKey != null) { 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. From 4e77bc4476b7e1c556a949642adf6e2450fb7187 Mon Sep 17 00:00:00 2001 From: rsd-darshan Date: Wed, 27 May 2026 09:37:58 +0545 Subject: [PATCH 6/7] fix: address review feedback on BYOK model picker fix - Simplify setActiveModel to direct O(1) map lookup, removing leftover loop-based boilerplate - Add TODO(#261) to findModelKeyByName documenting the same-name ambiguity in the custom-mode event path - Add ModelPickerGroupsBuilderTest asserting that two models sharing a modelName produce DropdownItems with distinct IDs (one per model key) --- .../ui/chat/ModelPickerGroupsBuilderTest.java | 81 +++++++++++++++++++ .../ui/chat/services/ModelService.java | 37 ++++----- 2 files changed, 95 insertions(+), 23 deletions(-) create mode 100644 com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilderTest.java 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/services/ModelService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelService.java index 0cb4708b..f1517a27 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 @@ -331,6 +331,9 @@ 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 currentModels = modelObservable.getValue(); for (Map.Entry entry : currentModels.entrySet()) { @@ -347,30 +350,18 @@ private String findModelKeyByName(String modelName) { * @param modelKey the composite key of the model */ public void setActiveModel(String modelKey) { - Map currentModels = modelObservable.getValue(); - - String compositeKey = null; - final CopilotModel model; - CopilotModel foundModel = null; - - if (currentModels.containsKey(modelKey)) { - compositeKey = modelKey; - foundModel = currentModels.get(modelKey); - } - 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); - - // Update observable - ensureRealm(() -> activeModelObservable.setValue(model)); + 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)); } /** From d539ade3cb846831113deb3acf5b9d1be1acfa58 Mon Sep 17 00:00:00 2001 From: rsd-darshan Date: Thu, 28 May 2026 12:31:20 +0545 Subject: [PATCH 7/7] fix: remove legacy TBB fallback model logic The fallback model path (getFallbackModel, setFallBackModelAsActiveModel, and the 402 replay logic in ChatContentViewer) was gated on !tokenBasedBillingEnabled(). Now that TBB has rolled out, this code is dead. Removing it as confirmed by @ethanyhou in PR #246. --- .../eclipse/ui/chat/ChatContentViewer.java | 51 ------------------- .../ui/chat/services/ModelService.java | 22 -------- 2 files changed, 73 deletions(-) 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/services/ModelService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ModelService.java index f1517a27..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; @@ -211,9 +210,6 @@ private void fetchCopilotModels() throws InterruptedException, ExecutionExceptio if (model.isChatDefault()) { defaultModel = model; } - if (model.isChatFallback()) { - fallbackModel = model; - } } copilotModels = newModels; @@ -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.getModelKey()); - } - } - /** * Check if the active model supports vision capabilities. */