From 88775b58076c64dade8914e212c5da4dd645be5b Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Thu, 25 Jun 2026 17:42:52 +0800 Subject: [PATCH 1/6] fix render issue --- .../eclipse/ui/chat/BaseTurnWidget.java | 2 +- .../eclipse/ui/chat/ChatContentViewer.java | 404 ++++++++++++------ .../eclipse/ui/chat/ThinkingBlock.java | 1 - 3 files changed, 284 insertions(+), 123 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java index 45557687..f0dafdfd 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java @@ -655,7 +655,7 @@ public CompletableFuture requestToolExecuti Composite ancestor = this.getParent(); while (ancestor != null && !ancestor.isDisposed()) { if (ancestor instanceof ChatContentViewer) { - ((ChatContentViewer) ancestor).requestRefreshScrollerLayout(); + ((ChatContentViewer) ancestor).scheduleRefresh(); break; } ancestor = ancestor.getParent(); 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 86a0bbf7..b84b7544 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 @@ -7,7 +7,10 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Queue; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.regex.Pattern; @@ -23,6 +26,7 @@ import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.ScrollBar; import org.eclipse.ui.PlatformUI; @@ -70,9 +74,27 @@ public class ChatContentViewer extends ScrolledComposite { private BaseTurnWidget latestUserTurn; private CopilotTurnWidget latestCopilotTurn; private BaseTurnWidget latestTurnWidget; - // Auto-scroll state management private boolean autoScrollEnabled; + /** Streaming events queued by LSP threads and drained on the UI thread in batches. */ + private final Queue pendingEvents = new ConcurrentLinkedQueue<>(); + private final AtomicBoolean drainScheduled = new AtomicBoolean(false); + + /** Client-area width of the last layout pass; a change forces a full re-measure (text re-wraps). */ + private int lastLayoutWidth = -1; + + /** + * Guards {@link #doRefreshScrollerLayout(boolean)} against synchronous re-entrancy: setting the + * scroller min size can toggle the scrollbar, changing the client width and re-firing + * {@code controlResized}. The running pass records any nested request here and drains it after + * unwinding (a full re-measure wins over an incremental one). + */ + private enum LayoutState { + IDLE, RUNNING, RERUN_INCREMENTAL, RERUN_FULL + } + + private LayoutState layoutState = LayoutState.IDLE; + /** * Create the composite. * @@ -186,123 +208,151 @@ public void setConversationId(String conversationId) { } /** - * Process turn event. + * Process turn event. Events are queued and drained in batches on the UI thread so the LSP thread + * is never blocked and multiple in-flight events coalesce into a single layout pass. */ public void processTurnEvent(ChatProgressValue value) { - SwtUtils.invokeOnDisplayThread(() -> { - if (!turns.containsKey(value.getTurnId())) { - CopilotCore.LOGGER.error(new IllegalStateException("turnId not found: " + value.getTurnId())); - return; - } - BaseTurnWidget turnWidget = turns.get(value.getTurnId()); - if (turnWidget == null) { - appendMessageToTheLatestTurn(value.getReply()); - } - - ChatServiceManager chatServiceManager = CopilotUi.getPlugin().getChatServiceManager(); - - if (value.getKind() == WorkDoneProgressKind.report) { - if (turnWidget instanceof ThinkingTurnWidget thinkingTurn) { - thinkingTurn.setConversationContext(conversationId, value.getTurnId()); - thinkingTurn.appendThinking(value.getThinking()); - updateActiveThinkingBlockId(value.getTurnId(), thinkingTurn); - if (hasRenderableOutput(value)) { - // Seal before appending the reply so the spinner stops and the title is fetched. - thinkingTurn.sealThinking(); - } - } + pendingEvents.offer(value); + if (drainScheduled.compareAndSet(false, true)) { + SwtUtils.invokeOnDisplayThreadAsync(this::drainPendingEvents, this); + } + } - if (value.getAgentRounds() != null && !value.getAgentRounds().isEmpty()) { - // Handle agent mode responses - AgentRound agentRound = value.getAgentRounds().get(0); + private void drainPendingEvents() { + drainScheduled.set(false); + if (isDisposed()) { + pendingEvents.clear(); + return; + } + boolean sawTurnEnd = false; + ChatProgressValue event; + while ((event = pendingEvents.poll()) != null) { + final ChatProgressValue e = event; + if (e.getKind() == WorkDoneProgressKind.end) { + sawTurnEnd = true; + } + doProcessTurnEvent(e); + } + // Use a full re-measure on the turn-end batch so minHeight is accurate before scrollToBottom(); + // the incremental path can accumulate a stale containerSize that leaves phantom space below the + // real content. All streaming batches keep the O(1) incremental path. + doRefreshScrollerLayout(sawTurnEnd); + if (shouldAutoScrollToBottom()) { + scrollToBottom(); + } + // Events may have arrived while draining; schedule a follow-up drain if so. + if (!pendingEvents.isEmpty() && drainScheduled.compareAndSet(false, true)) { + SwtUtils.invokeOnDisplayThreadAsync(this::drainPendingEvents, this); + } + } - if (agentRound.getReply() != null) { - turnWidget.appendMessage(agentRound.getReply()); - } + private void doProcessTurnEvent(ChatProgressValue value) { + if (!turns.containsKey(value.getTurnId())) { + CopilotCore.LOGGER.error(new IllegalStateException("turnId not found: " + value.getTurnId())); + return; + } + BaseTurnWidget turnWidget = turns.get(value.getTurnId()); + if (turnWidget == null) { + appendMessageToTheLatestTurn(value.getReply()); + } - if (agentRound.getToolCalls() != null && !agentRound.getToolCalls().isEmpty()) { - AgentToolCall toolCall = agentRound.getToolCalls().get(0); - turnWidget.appendToolCallStatus(toolCall); + ChatServiceManager chatServiceManager = CopilotUi.getPlugin().getChatServiceManager(); - // Extract and process todo list from tool result details - processTodoListFromToolCall(chatServiceManager, value.getConversationId(), toolCall); - } - } else { - // Handle chat mode responses - turnWidget.appendMessage(value.getReply()); - } - } else if (value.getKind() == WorkDoneProgressKind.end) { - // Seal any in-progress thinking block before the turn ends. - if (turnWidget instanceof ThinkingTurnWidget thinkingTurn) { + if (value.getKind() == WorkDoneProgressKind.report) { + if (turnWidget instanceof ThinkingTurnWidget thinkingTurn) { + thinkingTurn.setConversationContext(conversationId, value.getTurnId()); + thinkingTurn.appendThinking(value.getThinking()); + updateActiveThinkingBlockId(value.getTurnId(), thinkingTurn); + if (hasRenderableOutput(value)) { + // Seal before appending the reply so the spinner stops and the title is fetched. thinkingTurn.sealThinking(); - updateActiveThinkingBlockId(value.getTurnId(), thinkingTurn); } - turnWidget.flushMessageBuffer(); } - refreshScrollerLayout(); - // Auto-scroll to bottom if enabled - if (shouldAutoScrollToBottom()) { - scrollToBottom(); - } + if (value.getAgentRounds() != null && !value.getAgentRounds().isEmpty()) { + // Handle agent mode responses + AgentRound agentRound = value.getAgentRounds().get(0); + + if (agentRound.getReply() != null) { + turnWidget.appendMessage(agentRound.getReply()); + } - String errMsg = value.getErrorMessage(); - if (StringUtils.isNotEmpty(errMsg)) { - errMsg = REQUEST_ID_SUFFIX.matcher(errMsg).replaceAll(StringUtils.EMPTY).trim(); + if (agentRound.getToolCalls() != null && !agentRound.getToolCalls().isEmpty()) { + AgentToolCall toolCall = agentRound.getToolCalls().get(0); + turnWidget.appendToolCallStatus(toolCall); + + // Extract and process todo list from tool result details + processTodoListFromToolCall(chatServiceManager, value.getConversationId(), toolCall); + } + } else { + // Handle chat mode responses + turnWidget.appendMessage(value.getReply()); } - String reason = value.getErrorReason(); - if (StringUtils.isNotEmpty(reason) && reason.equals("model_not_supported")) { - // TODO: add enable button for better UX. - errMsg = Messages.chat_model_unsupported_message; + } else if (value.getKind() == WorkDoneProgressKind.end) { + // Seal any in-progress thinking block before the turn ends. + if (turnWidget instanceof ThinkingTurnWidget thinkingTurn) { + thinkingTurn.sealThinking(); + updateActiveThinkingBlockId(value.getTurnId(), thinkingTurn); } - 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); - } + turnWidget.flushMessageBuffer(); + } + + String errMsg = value.getErrorMessage(); + if (StringUtils.isNotEmpty(errMsg)) { + errMsg = REQUEST_ID_SUFFIX.matcher(errMsg).replaceAll(StringUtils.EMPTY).trim(); + } + String reason = value.getErrorReason(); + if (StringUtils.isNotEmpty(reason) && reason.equals("model_not_supported")) { + // TODO: add enable button for better UX. + 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); - } + 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); + } } /** Returns the active thinking block ID last observed while processing this turn's progress. */ @@ -437,11 +487,19 @@ public void renderErrorMessage(String errorMessage) { } /** - * Schedules a single async {@link #refreshScrollerLayout()} call so that multiple dispose/layout - * events that arrive in the same event-loop tick are coalesced into one pass. + * Schedules a coalesced asynchronous full layout pass; calls in the same UI tick collapse into one + * {@code asyncExec}. Used by structural changes that can resize a non-trailing turn (e.g. a + * tool-confirmation dialog disposing) outside the streaming event drain. */ - public void requestRefreshScrollerLayout() { - SwtUtils.invokeOnDisplayThreadAsync(() -> refreshScrollerLayout(), this); + public void scheduleRefresh() { + if (this.isDisposed()) { + return; + } + SwtUtils.invokeOnDisplayThreadAsync(() -> { + if (!this.isDisposed()) { + doRefreshScrollerLayout(true); + } + }, this); } /** @@ -451,9 +509,61 @@ public void refreshScrollerLayout() { if (this.isDisposed()) { return; } + // Public entry point for structural changes (turn start, error/warn widgets, expand/collapse of + // historical thinking blocks). These can resize a non-trailing turn, so force a full re-measure. + // The streaming path instead goes through the event drain, which stays O(1). + doRefreshScrollerLayout(true); + } + + private void doRefreshScrollerLayout(boolean forceFullMeasure) { + // Re-entrancy guard: setting the scroller min size can re-fire controlResized -> + // refreshScrollerLayout() while we are still inside this method. Recursing there reruns the + // expensive full re-measure and froze the UI. Instead record the nested request and run a single + // follow-up pass after the current one unwinds. + if (layoutState != LayoutState.IDLE) { + if (forceFullMeasure) { + layoutState = LayoutState.RERUN_FULL; + } else if (layoutState == LayoutState.RUNNING) { + layoutState = LayoutState.RERUN_INCREMENTAL; + } + return; + } + + layoutState = LayoutState.RUNNING; + try { + doRefreshScrollerLayoutDecide(forceFullMeasure); + // Drain follow-up requests queued while we were laying out. The idempotent setMin* calls + // guarantee a fixed point, so this loop terminates. + while (layoutState != LayoutState.RUNNING) { + boolean rerunFull = layoutState == LayoutState.RERUN_FULL; + layoutState = LayoutState.RUNNING; + doRefreshScrollerLayoutDecide(rerunFull); + } + } finally { + layoutState = LayoutState.IDLE; + } + } + private void doRefreshScrollerLayoutDecide(boolean forceFullMeasure) { Rectangle clientArea = this.getClientArea(); - Point containerSize = cmpContent.computeSize(clientArea.width, SWT.DEFAULT); + int width = clientArea.width; + + // Width change re-wraps text and forces every turn to be re-measured. Otherwise (the common + // streaming case) only trailing turns are flushed and sealed turns keep cached sizes, so + // computeSize stays O(1) in the number of turns. + boolean fullMeasure = forceFullMeasure || width != lastLayoutWidth; + lastLayoutWidth = width; + + doRefreshScrollerLayoutApply(clientArea, width, fullMeasure); + } + + private void doRefreshScrollerLayoutApply(Rectangle clientArea, int width, boolean fullMeasure) { + if (!fullMeasure) { + // Only the trailing turns can grow/change during streaming. + flushTrailingTurnCaches(); + } + + Point containerSize = cmpContent.computeSize(width, SWT.DEFAULT, fullMeasure); // Use the default size as a fallback if (latestUserTurn == null) { @@ -461,22 +571,57 @@ public void refreshScrollerLayout() { return; } - Point userTurnSize = latestUserTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT); - Point copilotTurnSize = latestCopilotTurn == null ? new Point(0, 0) - : latestCopilotTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT); + // Measure at the actual column width, not SWT.DEFAULT: unconstrained width collapses + // soft-wrapped text to a single line and under-estimates the height. roundedHeight must match + // the laid-out height shouldAutoScrollToBottom() reads via getBounds(), or the padding branch + // below reserves phantom whitespace while auto-scroll fires into it (issue #259 flicker). + int userTurnHeight = latestUserTurn.computeSize(width, SWT.DEFAULT).y; + int copilotTurnHeight = latestCopilotTurn == null || latestCopilotTurn.isDisposed() ? 0 + : latestCopilotTurn.computeSize(width, SWT.DEFAULT).y; // Calculate the content height, so that the latest user turn is able to be put at the top of the client area. int contentHeight = 0; - int roundedHeight = userTurnSize.y + copilotTurnSize.y; + int roundedHeight = userTurnHeight + copilotTurnHeight; if (roundedHeight < clientArea.height) { contentHeight = clientArea.height + containerSize.y - roundedHeight; } else { contentHeight = containerSize.y; } - this.setMinHeight(contentHeight); - this.setMinWidth(containerSize.x); - this.layout(true, true); + // Only write min size when it changes: setMin* re-fires controlResized, so skipping no-op writes + // lets the re-entrancy guard converge to a fixed point. + if (this.getMinHeight() != contentHeight) { + this.setMinHeight(contentHeight); + } + if (this.getMinWidth() != containerSize.x) { + this.setMinWidth(containerSize.x); + } + // Incremental layout: only re-position the latest (growing) copilot turn instead of recursing + // into all past turns as conversations grow longer. + if (latestCopilotTurn != null && !latestCopilotTurn.isDisposed()) { + cmpContent.layout(new Control[] {latestCopilotTurn}); + } else { + cmpContent.layout(true, false); + } + this.layout(true, false); + } + + /** + * Flushes the cached layout sizes of the trailing (mutating) turns so the next + * {@code computeSize(width, DEFAULT, false)} re-measures them while sealed historical turns stay + * cached, keeping the layout pass O(1) in the number of historical turns. + */ + private void flushTrailingTurnCaches() { + List dirty = new ArrayList<>(2); + if (latestUserTurn != null && !latestUserTurn.isDisposed()) { + dirty.add(latestUserTurn); + } + if (latestCopilotTurn != null && !latestCopilotTurn.isDisposed()) { + dirty.add(latestCopilotTurn); + } + if (!dirty.isEmpty()) { + cmpContent.layout(dirty.toArray(new Control[0])); + } } /** @@ -493,16 +638,34 @@ private boolean shouldAutoScrollToBottom() { } Rectangle clientArea = this.getClientArea(); - Point userTurnSize = latestUserTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT); - Point copilotTurnSize = latestCopilotTurn == null ? new Point(0, 0) - : latestCopilotTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT); - - int roundedHeight = userTurnSize.y + copilotTurnSize.y; + // Use the freshly laid-out bounds rather than computeSize(): the incremental streaming pass + // repositions the trailing turns but does not flush their computeSize cache, so computeSize + // would return a stale height and the auto-scroll trigger would fire seconds too late. + // getBounds() reflects the just-applied layout. + int roundedHeight = currentTurnLaidOutHeight(); // Only auto-scroll when content height exceeds the visible area return roundedHeight >= clientArea.height; } + /** + * Returns the height of the latest (streaming) turn from its applied layout bounds. Falls back to + * {@code computeSize} only when bounds are not yet available (before the first layout pass). + */ + private int currentTurnLaidOutHeight() { + int height = 0; + if (latestUserTurn != null && !latestUserTurn.isDisposed()) { + int userHeight = latestUserTurn.getBounds().height; + height += userHeight > 0 ? userHeight : latestUserTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT).y; + } + if (latestCopilotTurn != null && !latestCopilotTurn.isDisposed()) { + int copilotHeight = latestCopilotTurn.getBounds().height; + height += copilotHeight > 0 ? copilotHeight + : latestCopilotTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT).y; + } + return height; + } + /** * Scroll to the bottom. */ @@ -523,9 +686,7 @@ private void scrollToLatestUserTurn() { return; } - // Use async execution to ensure layout is computed before reading positions. - // Using sync execution would read positions before the layout is complete, - // resulting in incorrect scroll position (always scrolling to 0). + // Async so layout is computed before reading positions; reading synchronously scrolls to 0. SwtUtils.invokeOnDisplayThreadAsync(() -> { if (this.isDisposed() || latestUserTurn.isDisposed()) { return; @@ -537,6 +698,7 @@ private void scrollToLatestUserTurn() { @Override public void dispose() { + pendingEvents.clear(); super.dispose(); for (BaseTurnWidget turn : turns.values()) { turn.dispose(); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java index 8e974b9e..bebcf018 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java @@ -315,7 +315,6 @@ private void refreshBody() { body.requestLayout(); updateScrollerDuringStreaming(); - refreshEnclosingScroller(); } /** Resize the scroller to fit content (up to max height) and auto-scroll to bottom if enabled. */ From 3614d29ebf54586893a342a55f73ebc4228091d0 Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Fri, 26 Jun 2026 10:28:57 +0800 Subject: [PATCH 2/6] resolve copilot comments --- .../copilot/eclipse/ui/chat/ChatContentViewer.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 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 b84b7544..7aac87cf 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 @@ -253,7 +253,9 @@ private void doProcessTurnEvent(ChatProgressValue value) { } BaseTurnWidget turnWidget = turns.get(value.getTurnId()); if (turnWidget == null) { + CopilotCore.LOGGER.error(new IllegalStateException("turnWidget not found: " + value.getTurnId())); appendMessageToTheLatestTurn(value.getReply()); + return; } ChatServiceManager chatServiceManager = CopilotUi.getPlugin().getChatServiceManager(); @@ -487,9 +489,8 @@ public void renderErrorMessage(String errorMessage) { } /** - * Schedules a coalesced asynchronous full layout pass; calls in the same UI tick collapse into one - * {@code asyncExec}. Used by structural changes that can resize a non-trailing turn (e.g. a - * tool-confirmation dialog disposing) outside the streaming event drain. + * Schedules an asynchronous full layout pass for structural changes that can resize a non-trailing + * turn outside the streaming event drain. */ public void scheduleRefresh() { if (this.isDisposed()) { From f08ea2eca0539c286e5f5b64f01669ed20e92c8b Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Fri, 26 Jun 2026 17:15:03 +0800 Subject: [PATCH 3/6] remove unnecessary state machine and duplicated methods --- .../eclipse/ui/chat/BaseTurnWidget.java | 4 +- .../eclipse/ui/chat/ChatContentViewer.java | 120 +++++++----------- 2 files changed, 47 insertions(+), 77 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java index f0dafdfd..1db7082b 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java @@ -654,8 +654,8 @@ public CompletableFuture requestToolExecuti this.confirmDialog.addDisposeListener(e -> { Composite ancestor = this.getParent(); while (ancestor != null && !ancestor.isDisposed()) { - if (ancestor instanceof ChatContentViewer) { - ((ChatContentViewer) ancestor).scheduleRefresh(); + if (ancestor instanceof ChatContentViewer viewer) { + SwtUtils.invokeOnDisplayThreadAsync(viewer::refreshScrollerLayout, viewer); break; } ancestor = ancestor.getParent(); 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 7aac87cf..03431e00 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 @@ -83,17 +83,8 @@ public class ChatContentViewer extends ScrolledComposite { /** Client-area width of the last layout pass; a change forces a full re-measure (text re-wraps). */ private int lastLayoutWidth = -1; - /** - * Guards {@link #doRefreshScrollerLayout(boolean)} against synchronous re-entrancy: setting the - * scroller min size can toggle the scrollbar, changing the client width and re-firing - * {@code controlResized}. The running pass records any nested request here and drains it after - * unwinding (a full re-measure wins over an incremental one). - */ - private enum LayoutState { - IDLE, RUNNING, RERUN_INCREMENTAL, RERUN_FULL - } - - private LayoutState layoutState = LayoutState.IDLE; + /** Guards against scheduling more than one pending async refresh from a burst of resize events. */ + private final AtomicBoolean refreshScheduled = new AtomicBoolean(false); /** * Create the composite. @@ -120,7 +111,7 @@ public ChatContentViewer(Composite parent, int style, ChatServiceManager service this.addControlListener(new ControlAdapter() { @Override public void controlResized(ControlEvent e) { - refreshScrollerLayout(); + scheduleCoalescedRefresh(); } }); @@ -224,22 +215,32 @@ private void drainPendingEvents() { pendingEvents.clear(); return; } - boolean sawTurnEnd = false; ChatProgressValue event; + boolean sawTurnEnd = false; while ((event = pendingEvents.poll()) != null) { - final ChatProgressValue e = event; - if (e.getKind() == WorkDoneProgressKind.end) { + doProcessTurnEvent(event); + if (event.getKind() == WorkDoneProgressKind.end) { sawTurnEnd = true; } - doProcessTurnEvent(e); } - // Use a full re-measure on the turn-end batch so minHeight is accurate before scrollToBottom(); - // the incremental path can accumulate a stale containerSize that leaves phantom space below the - // real content. All streaming batches keep the O(1) incremental path. - doRefreshScrollerLayout(sawTurnEnd); + refreshScrollerLayout(false); if (shouldAutoScrollToBottom()) { scrollToBottom(); } + if (sawTurnEnd) { + // A turn's height settles one frame after its content changes. Mid-stream the next chunk's drain + // re-measures and re-scrolls into that settled height; the final chunk has no follow-up, so run + // one on the next frame. + SwtUtils.invokeOnDisplayThreadAsync(() -> { + if (isDisposed()) { + return; + } + refreshScrollerLayout(false); + if (shouldAutoScrollToBottom()) { + scrollToBottom(); + } + }, this); + } // Events may have arrived while draining; schedule a follow-up drain if so. if (!pendingEvents.isEmpty() && drainScheduled.compareAndSet(false, true)) { SwtUtils.invokeOnDisplayThreadAsync(this::drainPendingEvents, this); @@ -489,76 +490,45 @@ public void renderErrorMessage(String errorMessage) { } /** - * Schedules an asynchronous full layout pass for structural changes that can resize a non-trailing - * turn outside the streaming event drain. + * Coalesces resize-triggered refreshes into a single async incremental pass. Writing the scroller + * min size can re-fire {@code controlResized}; deferring instead of recursing avoids a synchronous + * re-entrancy guard while the idempotent min-size writes still converge. */ - public void scheduleRefresh() { - if (this.isDisposed()) { - return; + private void scheduleCoalescedRefresh() { + if (refreshScheduled.compareAndSet(false, true)) { + SwtUtils.invokeOnDisplayThreadAsync(() -> { + refreshScheduled.set(false); + refreshScrollerLayout(false); + }, this); } - SwtUtils.invokeOnDisplayThreadAsync(() -> { - if (!this.isDisposed()) { - doRefreshScrollerLayout(true); - } - }, this); } /** - * Update the size of scrolled composite when there are content updates. + * Full re-measure entry point for structural changes (turn start, error/warn widgets, + * expand/collapse of historical thinking blocks) that can resize a non-trailing turn. The streaming + * path instead calls {@link #refreshScrollerLayout(boolean)} with an incremental measure, which + * stays O(1) in the number of turns. */ public void refreshScrollerLayout() { - if (this.isDisposed()) { - return; - } - // Public entry point for structural changes (turn start, error/warn widgets, expand/collapse of - // historical thinking blocks). These can resize a non-trailing turn, so force a full re-measure. - // The streaming path instead goes through the event drain, which stays O(1). - doRefreshScrollerLayout(true); + refreshScrollerLayout(true); } - private void doRefreshScrollerLayout(boolean forceFullMeasure) { - // Re-entrancy guard: setting the scroller min size can re-fire controlResized -> - // refreshScrollerLayout() while we are still inside this method. Recursing there reruns the - // expensive full re-measure and froze the UI. Instead record the nested request and run a single - // follow-up pass after the current one unwinds. - if (layoutState != LayoutState.IDLE) { - if (forceFullMeasure) { - layoutState = LayoutState.RERUN_FULL; - } else if (layoutState == LayoutState.RUNNING) { - layoutState = LayoutState.RERUN_INCREMENTAL; - } + /** + * Re-measure the scroller and update its min size. + * + * @param forceFullMeasure when {@code true}, recursively re-measures every turn; when {@code false} + * only the trailing (mutating) turns are flushed while sealed turns keep cached sizes, keeping + * the pass O(1). A width change always upgrades to a full measure because text re-wraps. + */ + private void refreshScrollerLayout(boolean forceFullMeasure) { + if (this.isDisposed()) { return; } - - layoutState = LayoutState.RUNNING; - try { - doRefreshScrollerLayoutDecide(forceFullMeasure); - // Drain follow-up requests queued while we were laying out. The idempotent setMin* calls - // guarantee a fixed point, so this loop terminates. - while (layoutState != LayoutState.RUNNING) { - boolean rerunFull = layoutState == LayoutState.RERUN_FULL; - layoutState = LayoutState.RUNNING; - doRefreshScrollerLayoutDecide(rerunFull); - } - } finally { - layoutState = LayoutState.IDLE; - } - } - - private void doRefreshScrollerLayoutDecide(boolean forceFullMeasure) { Rectangle clientArea = this.getClientArea(); int width = clientArea.width; - - // Width change re-wraps text and forces every turn to be re-measured. Otherwise (the common - // streaming case) only trailing turns are flushed and sealed turns keep cached sizes, so - // computeSize stays O(1) in the number of turns. boolean fullMeasure = forceFullMeasure || width != lastLayoutWidth; lastLayoutWidth = width; - doRefreshScrollerLayoutApply(clientArea, width, fullMeasure); - } - - private void doRefreshScrollerLayoutApply(Rectangle clientArea, int width, boolean fullMeasure) { if (!fullMeasure) { // Only the trailing turns can grow/change during streaming. flushTrailingTurnCaches(); @@ -590,7 +560,7 @@ private void doRefreshScrollerLayoutApply(Rectangle clientArea, int width, boole } // Only write min size when it changes: setMin* re-fires controlResized, so skipping no-op writes - // lets the re-entrancy guard converge to a fixed point. + // lets the coalesced async refresh converge to a fixed point. if (this.getMinHeight() != contentHeight) { this.setMinHeight(contentHeight); } From 3ab08260c344e48da1954dedc1050c6a7985718e Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Mon, 29 Jun 2026 16:40:06 +0800 Subject: [PATCH 4/6] Preserve auto-scroll during active turn refreshes --- .../eclipse/ui/chat/BaseTurnWidget.java | 4 +- .../eclipse/ui/chat/ChatContentViewer.java | 54 +++++++------------ .../copilot/eclipse/ui/chat/ChatView.java | 17 ++---- .../eclipse/ui/chat/ThinkingBlock.java | 2 +- .../ui/chat/services/AgentToolService.java | 2 +- 5 files changed, 28 insertions(+), 51 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java index 1db7082b..a14873f8 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java @@ -651,11 +651,13 @@ public CompletableFuture requestToolExecuti reset(); this.confirmDialog = new InvokeToolConfirmationDialog(this, content, input); + ensureFooterAtBottom(); + requestLayout(); this.confirmDialog.addDisposeListener(e -> { Composite ancestor = this.getParent(); while (ancestor != null && !ancestor.isDisposed()) { if (ancestor instanceof ChatContentViewer viewer) { - SwtUtils.invokeOnDisplayThreadAsync(viewer::refreshScrollerLayout, viewer); + SwtUtils.invokeOnDisplayThreadAsync(() -> viewer.refreshScrollerLayout(false), viewer); break; } ancestor = ancestor.getParent(); 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 03431e00..7c0118be 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 @@ -148,7 +148,7 @@ public void startNewTurn(String workDoneToken, String message) { turnWidget.appendMessage(message); turnWidget.flushMessageBuffer(); - refreshScrollerLayout(); + refreshScrollerLayout(false); scrollToLatestUserTurn(); // Reset auto-scroll for new conversation turn autoScrollEnabled = true; @@ -216,31 +216,10 @@ private void drainPendingEvents() { return; } ChatProgressValue event; - boolean sawTurnEnd = false; while ((event = pendingEvents.poll()) != null) { doProcessTurnEvent(event); - if (event.getKind() == WorkDoneProgressKind.end) { - sawTurnEnd = true; - } - } - refreshScrollerLayout(false); - if (shouldAutoScrollToBottom()) { - scrollToBottom(); - } - if (sawTurnEnd) { - // A turn's height settles one frame after its content changes. Mid-stream the next chunk's drain - // re-measures and re-scrolls into that settled height; the final chunk has no follow-up, so run - // one on the next frame. - SwtUtils.invokeOnDisplayThreadAsync(() -> { - if (isDisposed()) { - return; - } - refreshScrollerLayout(false); - if (shouldAutoScrollToBottom()) { - scrollToBottom(); - } - }, this); } + refreshScrollerLayoutInternal(false, true); // Events may have arrived while draining; schedule a follow-up drain if so. if (!pendingEvents.isEmpty() && drainScheduled.compareAndSet(false, true)) { SwtUtils.invokeOnDisplayThreadAsync(this::drainPendingEvents, this); @@ -446,7 +425,7 @@ public void showCompactingStatusOnLatestCopilotTurn() { // the next round's reply and produce a single garbled line. latestCopilotTurn.flushMessageBuffer(); latestCopilotTurn.showCompactingStatus(); - refreshScrollerLayout(); + refreshScrollerLayoutInternal(true, true); } /** @@ -461,7 +440,7 @@ public void hideCompactingStatusOnLatestCopilotTurn() { // in case a cancel path did not receive an end progress event to flush it. latestCopilotTurn.flushMessageBuffer(); latestCopilotTurn.hideCompactingStatus(); - refreshScrollerLayout(); + refreshScrollerLayoutInternal(true, true); } /** @@ -473,7 +452,7 @@ public BaseTurnWidget getTurnWidget(String turnId) { private void renderWarnMessageWithUpgradePlanButton(String errorMessage, int code, String modelProviderName) { latestTurnWidget.createWarnDialog(errorMessage, code, modelProviderName); - refreshScrollerLayout(); + refreshScrollerLayout(false); scrollToLatestUserTurn(); } @@ -485,7 +464,7 @@ public void renderErrorMessage(String errorMessage) { this.errorWidget.dispose(); } this.errorWidget = new ErrorWidget(cmpContent, SWT.BOTTOM, errorMessage); - refreshScrollerLayout(); + refreshScrollerLayout(false); scrollToLatestUserTurn(); } @@ -498,19 +477,19 @@ private void scheduleCoalescedRefresh() { if (refreshScheduled.compareAndSet(false, true)) { SwtUtils.invokeOnDisplayThreadAsync(() -> { refreshScheduled.set(false); - refreshScrollerLayout(false); + refreshScrollerLayoutInternal(false, false); }, this); } } /** - * Full re-measure entry point for structural changes (turn start, error/warn widgets, - * expand/collapse of historical thinking blocks) that can resize a non-trailing turn. The streaming - * path instead calls {@link #refreshScrollerLayout(boolean)} with an incremental measure, which - * stays O(1) in the number of turns. + * Full re-measure entry point that optionally preserves the current auto-scroll position. + * + * @param preserveAutoScroll when {@code true}, keeps the viewport at the bottom after this refresh + * if auto-scroll is currently enabled */ - public void refreshScrollerLayout() { - refreshScrollerLayout(true); + public void refreshScrollerLayout(boolean preserveAutoScroll) { + refreshScrollerLayoutInternal(true, preserveAutoScroll); } /** @@ -519,8 +498,10 @@ public void refreshScrollerLayout() { * @param forceFullMeasure when {@code true}, recursively re-measures every turn; when {@code false} * only the trailing (mutating) turns are flushed while sealed turns keep cached sizes, keeping * the pass O(1). A width change always upgrades to a full measure because text re-wraps. + * @param preserveAutoScroll when {@code true}, keeps the viewport at the bottom after this refresh + * if auto-scroll is currently enabled */ - private void refreshScrollerLayout(boolean forceFullMeasure) { + private void refreshScrollerLayoutInternal(boolean forceFullMeasure, boolean preserveAutoScroll) { if (this.isDisposed()) { return; } @@ -575,6 +556,9 @@ private void refreshScrollerLayout(boolean forceFullMeasure) { cmpContent.layout(true, false); } this.layout(true, false); + if (preserveAutoScroll && shouldAutoScrollToBottom()) { + scrollToBottom(); + } } /** diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java index 89b350df..bbf71caa 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java @@ -32,7 +32,6 @@ import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Display; -import org.eclipse.swt.widgets.ScrollBar; import org.eclipse.ui.IPartListener2; import org.eclipse.ui.IWorkbenchPartReference; import org.eclipse.ui.PlatformUI; @@ -1819,16 +1818,7 @@ public void scrollContentToBottom() { return; } - chatContentViewer.getDisplay().asyncExec(() -> { - if (chatContentViewer.isDisposed()) { - return; - } - chatContentViewer.refreshScrollerLayout(); - ScrollBar verticalBar = chatContentViewer.getVerticalBar(); - if (verticalBar != null && !verticalBar.isDisposed()) { - chatContentViewer.setOrigin(0, verticalBar.getMaximum()); - } - }); + SwtUtils.invokeOnDisplayThreadAsync(() -> chatContentViewer.refreshScrollerLayout(true), chatContentViewer); } /** @@ -1847,8 +1837,9 @@ private void renderModelInfoInTurnWidget(String turnId, String modelName, double if (turnWidget instanceof CopilotTurnWidget copilotWidget) { copilotWidget.renderModelInfo(modelName, billingMultiplier, reasoningEffort); - // Refresh the scroller layout to ensure the footer is visible - SwtUtils.invokeOnDisplayThreadAsync(() -> this.chatContentViewer.refreshScrollerLayout(), this.chatContentViewer); + // Refresh the scroller layout to ensure the footer is visible. + SwtUtils.invokeOnDisplayThreadAsync(() -> this.chatContentViewer.refreshScrollerLayout(true), + this.chatContentViewer); } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java index bebcf018..04b8a210 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java @@ -498,7 +498,7 @@ private void refreshEnclosingScroller() { Composite p = getParent(); while (p != null && !p.isDisposed()) { if (p instanceof ChatContentViewer) { - ((ChatContentViewer) p).refreshScrollerLayout(); + ((ChatContentViewer) p).refreshScrollerLayout(false); return; } p = p.getParent(); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java index be236d9c..d5b9ff17 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java @@ -294,7 +294,7 @@ public CompletableFuture onToolConfirmation SwtUtils.invokeOnDisplayThread(() -> { ref.set(activeTurnWidget.requestToolExecutionConfirmation( content, params.getInput())); - boundChatView.getChatContentViewer().refreshScrollerLayout(); + boundChatView.getChatContentViewer().refreshScrollerLayout(false); }); CompletableFuture future = ref.get(); From 0f3fdcc8a4e893fb226ba4f059d95039eeca9b68 Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Mon, 29 Jun 2026 17:39:31 +0800 Subject: [PATCH 5/6] decouple refresh and scrollToBottom --- .../eclipse/ui/chat/BaseTurnWidget.java | 2 +- .../eclipse/ui/chat/ChatContentViewer.java | 41 +++++++++++-------- .../copilot/eclipse/ui/chat/ChatView.java | 11 +++-- .../eclipse/ui/chat/ThinkingBlock.java | 2 +- .../ui/chat/services/AgentToolService.java | 2 +- 5 files changed, 35 insertions(+), 23 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java index a14873f8..029b49f0 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java @@ -657,7 +657,7 @@ public CompletableFuture requestToolExecuti Composite ancestor = this.getParent(); while (ancestor != null && !ancestor.isDisposed()) { if (ancestor instanceof ChatContentViewer viewer) { - SwtUtils.invokeOnDisplayThreadAsync(() -> viewer.refreshScrollerLayout(false), viewer); + SwtUtils.invokeOnDisplayThreadAsync(() -> viewer.refreshScrollerLayout(), viewer); break; } ancestor = ancestor.getParent(); 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 7c0118be..bfe04bd0 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 @@ -148,7 +148,7 @@ public void startNewTurn(String workDoneToken, String message) { turnWidget.appendMessage(message); turnWidget.flushMessageBuffer(); - refreshScrollerLayout(false); + refreshScrollerLayout(); scrollToLatestUserTurn(); // Reset auto-scroll for new conversation turn autoScrollEnabled = true; @@ -219,7 +219,8 @@ private void drainPendingEvents() { while ((event = pendingEvents.poll()) != null) { doProcessTurnEvent(event); } - refreshScrollerLayoutInternal(false, true); + refreshScrollerLayoutInternal(false); + scrollToBottomIfAutoScroll(); // Events may have arrived while draining; schedule a follow-up drain if so. if (!pendingEvents.isEmpty() && drainScheduled.compareAndSet(false, true)) { SwtUtils.invokeOnDisplayThreadAsync(this::drainPendingEvents, this); @@ -425,7 +426,8 @@ public void showCompactingStatusOnLatestCopilotTurn() { // the next round's reply and produce a single garbled line. latestCopilotTurn.flushMessageBuffer(); latestCopilotTurn.showCompactingStatus(); - refreshScrollerLayoutInternal(true, true); + refreshScrollerLayout(); + scrollToBottomIfAutoScroll(); } /** @@ -440,7 +442,8 @@ public void hideCompactingStatusOnLatestCopilotTurn() { // in case a cancel path did not receive an end progress event to flush it. latestCopilotTurn.flushMessageBuffer(); latestCopilotTurn.hideCompactingStatus(); - refreshScrollerLayoutInternal(true, true); + refreshScrollerLayout(); + scrollToBottomIfAutoScroll(); } /** @@ -452,7 +455,7 @@ public BaseTurnWidget getTurnWidget(String turnId) { private void renderWarnMessageWithUpgradePlanButton(String errorMessage, int code, String modelProviderName) { latestTurnWidget.createWarnDialog(errorMessage, code, modelProviderName); - refreshScrollerLayout(false); + refreshScrollerLayout(); scrollToLatestUserTurn(); } @@ -464,7 +467,7 @@ public void renderErrorMessage(String errorMessage) { this.errorWidget.dispose(); } this.errorWidget = new ErrorWidget(cmpContent, SWT.BOTTOM, errorMessage); - refreshScrollerLayout(false); + refreshScrollerLayout(); scrollToLatestUserTurn(); } @@ -477,19 +480,17 @@ private void scheduleCoalescedRefresh() { if (refreshScheduled.compareAndSet(false, true)) { SwtUtils.invokeOnDisplayThreadAsync(() -> { refreshScheduled.set(false); - refreshScrollerLayoutInternal(false, false); + refreshScrollerLayoutInternal(false); }, this); } } /** - * Full re-measure entry point that optionally preserves the current auto-scroll position. - * - * @param preserveAutoScroll when {@code true}, keeps the viewport at the bottom after this refresh - * if auto-scroll is currently enabled + * Full re-measure entry point. Layout only; scrolling is a separate concern handled by callers via + * {@link #scrollToBottomIfAutoScroll()}. */ - public void refreshScrollerLayout(boolean preserveAutoScroll) { - refreshScrollerLayoutInternal(true, preserveAutoScroll); + public void refreshScrollerLayout() { + refreshScrollerLayoutInternal(true); } /** @@ -498,10 +499,8 @@ public void refreshScrollerLayout(boolean preserveAutoScroll) { * @param forceFullMeasure when {@code true}, recursively re-measures every turn; when {@code false} * only the trailing (mutating) turns are flushed while sealed turns keep cached sizes, keeping * the pass O(1). A width change always upgrades to a full measure because text re-wraps. - * @param preserveAutoScroll when {@code true}, keeps the viewport at the bottom after this refresh - * if auto-scroll is currently enabled */ - private void refreshScrollerLayoutInternal(boolean forceFullMeasure, boolean preserveAutoScroll) { + private void refreshScrollerLayoutInternal(boolean forceFullMeasure) { if (this.isDisposed()) { return; } @@ -556,7 +555,15 @@ private void refreshScrollerLayoutInternal(boolean forceFullMeasure, boolean pre cmpContent.layout(true, false); } this.layout(true, false); - if (preserveAutoScroll && shouldAutoScrollToBottom()) { + } + + /** + * Scrolls the viewport to the bottom when auto-scroll is currently enabled. Kept separate from the + * layout pass so refresh and scroll stay independent concerns; callers invoke this after a refresh + * when they want the latest content to stay in view. + */ + public void scrollToBottomIfAutoScroll() { + if (shouldAutoScrollToBottom()) { scrollToBottom(); } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java index bbf71caa..23c18b5d 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java @@ -1818,7 +1818,10 @@ public void scrollContentToBottom() { return; } - SwtUtils.invokeOnDisplayThreadAsync(() -> chatContentViewer.refreshScrollerLayout(true), chatContentViewer); + SwtUtils.invokeOnDisplayThreadAsync(() -> { + chatContentViewer.refreshScrollerLayout(); + chatContentViewer.scrollToBottomIfAutoScroll(); + }, chatContentViewer); } /** @@ -1838,8 +1841,10 @@ private void renderModelInfoInTurnWidget(String turnId, String modelName, double copilotWidget.renderModelInfo(modelName, billingMultiplier, reasoningEffort); // Refresh the scroller layout to ensure the footer is visible. - SwtUtils.invokeOnDisplayThreadAsync(() -> this.chatContentViewer.refreshScrollerLayout(true), - this.chatContentViewer); + SwtUtils.invokeOnDisplayThreadAsync(() -> { + this.chatContentViewer.refreshScrollerLayout(); + this.chatContentViewer.scrollToBottomIfAutoScroll(); + }, this.chatContentViewer); } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java index 04b8a210..bebcf018 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java @@ -498,7 +498,7 @@ private void refreshEnclosingScroller() { Composite p = getParent(); while (p != null && !p.isDisposed()) { if (p instanceof ChatContentViewer) { - ((ChatContentViewer) p).refreshScrollerLayout(false); + ((ChatContentViewer) p).refreshScrollerLayout(); return; } p = p.getParent(); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java index d5b9ff17..be236d9c 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java @@ -294,7 +294,7 @@ public CompletableFuture onToolConfirmation SwtUtils.invokeOnDisplayThread(() -> { ref.set(activeTurnWidget.requestToolExecutionConfirmation( content, params.getInput())); - boundChatView.getChatContentViewer().refreshScrollerLayout(false); + boundChatView.getChatContentViewer().refreshScrollerLayout(); }); CompletableFuture future = ref.get(); From 7debe99dc7d3c9ca0868d012775c0abb8a4fc6fb Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Mon, 29 Jun 2026 17:43:55 +0800 Subject: [PATCH 6/6] Remove layout calls before confirmation dialog --- .../com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java index 029b49f0..fb799d9c 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java @@ -651,8 +651,6 @@ public CompletableFuture requestToolExecuti reset(); this.confirmDialog = new InvokeToolConfirmationDialog(this, content, input); - ensureFooterAtBottom(); - requestLayout(); this.confirmDialog.addDisposeListener(e -> { Composite ancestor = this.getParent(); while (ancestor != null && !ancestor.isDisposed()) {