Skip to content

Commit e2cd2f7

Browse files
ethanyhouethanyhou
authored
feat: Show thinking block in chat view (#157)
1 parent 3452fa5 commit e2cd2f7

14 files changed

Lines changed: 836 additions & 24 deletions

File tree

‎com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlockParseTest.java‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
package com.microsoft.copilot.eclipse.ui.chat;
5+
6+
import static org.junit.jupiter.api.Assertions.assertEquals;
7+
import static org.junit.jupiter.api.Assertions.assertNull;
8+
9+
import java.lang.reflect.Method;
10+
import java.util.List;
11+
12+
import org.junit.jupiter.api.Test;
13+
14+
/**
15+
* Unit tests for the private static parsing helpers in {@link ThinkingBlock}: {@code stripTrailingNewlines} and
16+
* {@code parseSections}. Exercises the helpers via reflection so the production visibility stays untouched.
17+
*
18+
* <p>Primary goal: guard CRLF handling so a {@code \r\n}-terminated body or title boundary does not leak a stray
19+
* {@code \r} into the rendered text.
20+
*/
21+
class ThinkingBlockParseTest {
22+
23+
@Test
24+
void stripTrailingNewlines_stripsLfCrAndCrlf() throws Exception {
25+
assertEquals("body", invokeStripTrailingNewlines("body\n"));
26+
assertEquals("body", invokeStripTrailingNewlines("body\r"));
27+
assertEquals("body", invokeStripTrailingNewlines("body\r\n"));
28+
assertEquals("body", invokeStripTrailingNewlines("body\r\n\r\n"));
29+
assertEquals("body", invokeStripTrailingNewlines("body\n\r\n"));
30+
assertEquals("body", invokeStripTrailingNewlines("body"));
31+
assertEquals("", invokeStripTrailingNewlines(""));
32+
assertEquals("", invokeStripTrailingNewlines("\r\n\r\n"));
33+
// Internal newlines and leading whitespace must survive untouched.
34+
assertEquals(" code\n more", invokeStripTrailingNewlines(" code\n more\r\n"));
35+
}
36+
37+
@Test
38+
void parseSections_handlesCrlfTitleBoundary() throws Exception {
39+
String raw = "intro body\r\n**Plan**\r\nplan body\r\n**Next**\r\nnext body\r\n";
40+
List<?> sections = invokeParseSections(raw);
41+
assertEquals(3, sections.size());
42+
43+
assertNull(title(sections.get(0)));
44+
assertEquals("intro body", body(sections.get(0)));
45+
46+
assertEquals("Plan", title(sections.get(1)));
47+
assertEquals("plan body", body(sections.get(1)));
48+
49+
assertEquals("Next", title(sections.get(2)));
50+
assertEquals("next body", body(sections.get(2)));
51+
}
52+
53+
@Test
54+
void parseSections_handlesLfOnlyTitleBoundary() throws Exception {
55+
// Existing LF behavior must remain unchanged.
56+
String raw = "intro\n**Plan**\nplan body";
57+
List<?> sections = invokeParseSections(raw);
58+
assertEquals(2, sections.size());
59+
assertNull(title(sections.get(0)));
60+
assertEquals("intro", body(sections.get(0)));
61+
assertEquals("Plan", title(sections.get(1)));
62+
assertEquals("plan body", body(sections.get(1)));
63+
}
64+
65+
private static String invokeStripTrailingNewlines(String input) throws Exception {
66+
Method m = ThinkingBlock.class.getDeclaredMethod("stripTrailingNewlines", String.class);
67+
m.setAccessible(true);
68+
return (String) m.invoke(null, input);
69+
}
70+
71+
private static List<?> invokeParseSections(String raw) throws Exception {
72+
Method m = ThinkingBlock.class.getDeclaredMethod("parseSections", String.class);
73+
m.setAccessible(true);
74+
return (List<?>) m.invoke(null, raw);
75+
}
76+
77+
private static String title(Object parsedSection) throws Exception {
78+
Method m = parsedSection.getClass().getDeclaredMethod("title");
79+
m.setAccessible(true);
80+
return (String) m.invoke(parsedSection);
81+
}
82+
83+
private static String body(Object parsedSection) throws Exception {
84+
Method m = parsedSection.getClass().getDeclaredMethod("body");
85+
m.setAccessible(true);
86+
return (String) m.invoke(parsedSection);
87+
}
88+
}

‎com.microsoft.copilot.eclipse.ui/css/dark.css‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,10 @@
115115
background-color: #2F3030;
116116
}
117117

118-
#chat-content-viewer > Composite > CopilotTurnWidget > AgentStatusLabel > .text-secondary {
119-
color: #A4A4A4;
120-
}
121-
122-
#chat-content-viewer > Composite > CopilotTurnWidget > AgentToolCancelLabel > Label.text-secondary {
123-
color: #A4A4A4;
124-
}
125-
118+
#chat-content-viewer > Composite > CopilotTurnWidget > AgentStatusLabel > .text-secondary,
119+
#chat-content-viewer > Composite > CopilotTurnWidget > ThinkingBlock .text-secondary,
120+
#chat-content-viewer > Composite > CopilotTurnWidget > .subagent-message-block ThinkingBlock .text-secondary,
121+
#chat-content-viewer > Composite > CopilotTurnWidget > AgentToolCancelLabel > Label.text-secondary,
126122
#chat-content-viewer > Composite > CopilotTurnWidget > Composite > Label.model-info-label {
127123
color: #A4A4A4;
128124
}

‎com.microsoft.copilot.eclipse.ui/css/light.css‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,10 @@
115115
background-color: #F1F1F2;
116116
}
117117

118-
#chat-content-viewer > Composite > CopilotTurnWidget > AgentStatusLabel > .text-secondary {
119-
color: #808080;
120-
}
121-
122-
#chat-content-viewer > Composite > CopilotTurnWidget > AgentToolCancelLabel > Label.text-secondary {
123-
color: #808080;
124-
}
125-
118+
#chat-content-viewer > Composite > CopilotTurnWidget > AgentStatusLabel > .text-secondary,
119+
#chat-content-viewer > Composite > CopilotTurnWidget > ThinkingBlock .text-secondary,
120+
#chat-content-viewer > Composite > CopilotTurnWidget > .subagent-message-block ThinkingBlock .text-secondary,
121+
#chat-content-viewer > Composite > CopilotTurnWidget > AgentToolCancelLabel > Label.text-secondary,
126122
#chat-content-viewer > Composite > CopilotTurnWidget > Composite > Label.model-info-label {
127123
color: #808080;
128124
}

‎com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,16 @@ protected BaseTurnWidget(Composite parent, int style, ChatServiceManager service
113113
IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class);
114114
this.cancelMsgEventHandler = event -> {
115115
cancelToolConfirmation();
116+
onChatMessageCancelled();
116117
};
117118
eventBroker.subscribe(CopilotEventConstants.TOPIC_CHAT_MESSAGE_CANCELLED, cancelMsgEventHandler);
118119
}
119120

121+
/** Hook invoked when the chat message cancel event is broadcast. Default no-op; subclasses may override. */
122+
protected void onChatMessageCancelled() {
123+
// no-op
124+
}
125+
120126
public String getTurnId() {
121127
return turnId;
122128
}
@@ -523,7 +529,8 @@ private void reset() {
523529
this.currentTextBlock = null;
524530
this.inCodeBlock = false;
525531

526-
// Don't reset subagent block state here - it's managed by tool call status
532+
// Subagent and thinking blocks have their own lifecycle (tool call status / ThinkingTurnWidget)
533+
// and are intentionally not reset here.
527534
}
528535

529536
/**

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,14 @@ public void processTurnEvent(ChatProgressValue value) {
181181
ChatServiceManager chatServiceManager = CopilotUi.getPlugin().getChatServiceManager();
182182

183183
if (value.getKind() == WorkDoneProgressKind.report) {
184+
if (turnWidget instanceof ThinkingTurnWidget thinkingTurn) {
185+
thinkingTurn.appendThinking(value.getThinking());
186+
if (hasRenderableOutput(value)) {
187+
// Seal before appending the reply so the spinner stops and the title is fetched.
188+
thinkingTurn.sealThinking();
189+
}
190+
}
191+
184192
if (value.getAgentRounds() != null && !value.getAgentRounds().isEmpty()) {
185193
// Handle agent mode responses
186194
AgentRound agentRound = value.getAgentRounds().get(0);
@@ -201,6 +209,10 @@ public void processTurnEvent(ChatProgressValue value) {
201209
turnWidget.appendMessage(value.getReply());
202210
}
203211
} else if (value.getKind() == WorkDoneProgressKind.end) {
212+
// Seal any in-progress thinking block before the turn ends.
213+
if (turnWidget instanceof ThinkingTurnWidget thinkingTurn) {
214+
thinkingTurn.sealThinking();
215+
}
204216
turnWidget.notifyTurnEnd();
205217
}
206218
refreshScrollerLayout();
@@ -260,6 +272,29 @@ public void appendMessageToTheLatestTurn(String message) {
260272
}
261273
}
262274

275+
/**
276+
* Whether {@code value} carries reply text or an agent round with rendered content; thinking-only reports return
277+
* {@code false} so the banner keeps streaming.
278+
*/
279+
private static boolean hasRenderableOutput(ChatProgressValue value) {
280+
return StringUtils.isNotBlank(value.getReply()) || hasRenderableAgentRound(value);
281+
}
282+
283+
private static boolean hasRenderableAgentRound(ChatProgressValue value) {
284+
if (value.getAgentRounds() == null || value.getAgentRounds().isEmpty()) {
285+
return false;
286+
}
287+
for (AgentRound round : value.getAgentRounds()) {
288+
if (StringUtils.isNotBlank(round.getReply())) {
289+
return true;
290+
}
291+
if (round.getToolCalls() != null && !round.getToolCalls().isEmpty()) {
292+
return true;
293+
}
294+
}
295+
return false;
296+
}
297+
263298
/**
264299
* Process todo list from tool call result. Extracts todo list data from the tool-specific data
265300
* and updates the TodoListService.

‎com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -844,7 +844,8 @@ public void onChatProgress(ChatProgressValue value) {
844844
}
845845
}
846846
if ((value.getAgentRounds() == null || value.getAgentRounds().isEmpty())
847-
&& (value.getReply() == null || value.getReply().isEmpty())) {
847+
&& (value.getReply() == null || value.getReply().isEmpty())
848+
&& (value.getThinking() == null || StringUtils.isBlank(value.getThinking().text()))) {
848849
return;
849850
}
850851
if (this.chatContentViewer != null) {

‎com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/CopilotTurnWidget.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@
2424
/**
2525
* A custom widget that displays a turn for the copilot.
2626
*/
27-
public class CopilotTurnWidget extends BaseTurnWidget {
27+
public class CopilotTurnWidget extends ThinkingTurnWidget {
2828
/**
2929
* Create the widget.
3030
*/
3131
public CopilotTurnWidget(Composite parent, int style, ChatServiceManager serviceManager, String turnId) {
32-
super(parent, style, serviceManager, turnId, true, null);
32+
super(parent, style, serviceManager, turnId, null);
3333
setData("org.eclipse.swtbot.widget.key", "copilot-turn");
3434
}
3535

‎com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ public final class Messages extends NLS {
3939
public static String todoList_clearButtonDisabled;
4040
public static String todoList_expandTooltip;
4141
public static String todoList_collapseTooltip;
42+
public static String thinking_inProgressTitle;
43+
public static String thinking_completedTitle;
44+
public static String thinking_cancelledTitle;
45+
public static String thinking_expandTooltip;
46+
public static String thinking_collapseTooltip;
4247

4348
static {
4449
// initialize resource bundle

‎com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/SubagentMessageBlock.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class SubagentMessageBlock extends Composite {
2424
private AgentToolCall toolCall;
2525

2626
// Track the current content widget for message processing
27-
private BaseTurnWidget currentSubagentTurnWidget;
27+
private ThinkingTurnWidget currentSubagentTurnWidget;
2828

2929
/**
3030
* Create the subagent message block.
@@ -102,7 +102,7 @@ public void notifyTurnEnd() {
102102
*
103103
* @return the subagent turn widget
104104
*/
105-
public BaseTurnWidget getSubagentTurnWidget() {
105+
public ThinkingTurnWidget getSubagentTurnWidget() {
106106
return currentSubagentTurnWidget;
107107
}
108108
}

‎com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/SubagentTurnWidget.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@
1919
* A turn widget for displaying subagent messages within a SubagentMessageBlock.
2020
* This widget doesn't show an avatar or role name, only the message content.
2121
*/
22-
public class SubagentTurnWidget extends BaseTurnWidget {
22+
public class SubagentTurnWidget extends ThinkingTurnWidget {
2323

2424
/**
2525
* Create the widget.
2626
*/
2727
public SubagentTurnWidget(Composite parent, int style, ChatServiceManager serviceManager, String turnId,
2828
AgentToolCall toolCall) {
29-
super(parent, style, serviceManager, turnId + "_subagent", true,
29+
super(parent, style, serviceManager, turnId + "_subagent",
3030
getToolCallRoleName(toolCall));
3131
}
3232

0 commit comments

Comments
 (0)