Skip to content

Commit d34aa9c

Browse files
committed
fix(aggregator): emit 'thinking' only on first reasoning part
1 parent 03e9e69 commit d34aa9c

2 files changed

Lines changed: 87 additions & 17 deletions

File tree

src/summary/aggregator.ts

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ class SummaryAggregator {
117117
private onFileChangeCallback: FileChangeCallback | null = null;
118118
private onClearedCallback: ClearedCallback | null = null;
119119
private processedToolStates: Set<string> = new Set();
120+
private thinkingFiredForMessages: Set<string> = new Set();
120121
private bot: Bot | null = null;
121122
private chatId: number | null = null;
122123
private typingTimer: ReturnType<typeof setInterval> | null = null;
@@ -271,6 +272,7 @@ class SummaryAggregator {
271272
this.messages.clear();
272273
this.partHashes.clear();
273274
this.processedToolStates.clear();
275+
this.thinkingFiredForMessages.clear();
274276
this.messageCount = 0;
275277
this.lastUpdated = 0;
276278

@@ -303,18 +305,6 @@ class SummaryAggregator {
303305
this.currentMessageParts.set(messageID, []);
304306
this.messageCount++;
305307
this.startTypingIndicator();
306-
307-
const isSummaryMessage = (info as { summary?: boolean }).summary === true;
308-
309-
// Notify that agent started thinking
310-
if (!isSummaryMessage && this.onThinkingCallback) {
311-
const callback = this.onThinkingCallback;
312-
setImmediate(() => {
313-
if (typeof callback === "function") {
314-
callback(info.sessionID);
315-
}
316-
});
317-
}
318308
}
319309

320310
const pending = this.pendingParts.get(messageID) || [];
@@ -394,7 +384,20 @@ class SummaryAggregator {
394384
const messageID = part.messageID;
395385
const messageInfo = this.messages.get(messageID);
396386

397-
if (part.type === "text" && "text" in part && part.text) {
387+
if (part.type === "reasoning") {
388+
// Fire the thinking callback once per message on the first reasoning part.
389+
// This is the signal that the model is actually doing extended thinking.
390+
if (!this.thinkingFiredForMessages.has(messageID) && this.onThinkingCallback) {
391+
this.thinkingFiredForMessages.add(messageID);
392+
const callback = this.onThinkingCallback;
393+
const sessionID = part.sessionID;
394+
setImmediate(() => {
395+
if (typeof callback === "function") {
396+
callback(sessionID);
397+
}
398+
});
399+
}
400+
} else if (part.type === "text" && "text" in part && part.text) {
398401
const partHash = this.hashString(part.text);
399402

400403
if (!this.partHashes.has(messageID)) {

tests/summary/aggregator.test.ts

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ describe("summary/aggregator", () => {
137137
expect(onToolFile).not.toHaveBeenCalled();
138138
});
139139

140-
it("passes sessionId to thinking callback", async () => {
140+
it("passes sessionId to thinking callback when reasoning part arrives", async () => {
141141
const onThinking = vi.fn();
142142
summaryAggregator.setOnThinking(onThinking);
143143
summaryAggregator.setSession("session-1");
@@ -154,34 +154,101 @@ describe("summary/aggregator", () => {
154154
},
155155
} as unknown as Event);
156156

157+
summaryAggregator.processEvent({
158+
type: "message.part.updated",
159+
properties: {
160+
part: {
161+
id: "part-reasoning-1",
162+
sessionID: "session-1",
163+
messageID: "message-1",
164+
type: "reasoning",
165+
text: "Let me think about this...",
166+
time: { start: Date.now() },
167+
},
168+
},
169+
} as unknown as Event);
170+
157171
await new Promise<void>((resolve) => setImmediate(resolve));
158172

159173
expect(onThinking).toHaveBeenCalledWith("session-1");
160174
});
161175

162-
it("does not send thinking callback for summary assistant messages", async () => {
176+
it("does not send thinking callback when no reasoning part arrives", async () => {
163177
const onThinking = vi.fn();
164178
summaryAggregator.setOnThinking(onThinking);
165179
summaryAggregator.setSession("session-1");
166180

181+
// Only a message.updated event without any reasoning part — should NOT trigger thinking
167182
summaryAggregator.processEvent({
168183
type: "message.updated",
169184
properties: {
170185
info: {
171-
id: "message-summary",
186+
id: "message-no-reasoning",
172187
sessionID: "session-1",
173188
role: "assistant",
174-
summary: true,
175189
time: { created: Date.now() },
176190
},
177191
},
178192
} as unknown as Event);
179193

194+
summaryAggregator.processEvent({
195+
type: "message.part.updated",
196+
properties: {
197+
part: {
198+
id: "part-text-1",
199+
sessionID: "session-1",
200+
messageID: "message-no-reasoning",
201+
type: "text",
202+
text: "Here is my answer.",
203+
time: { start: Date.now() },
204+
},
205+
},
206+
} as unknown as Event);
207+
180208
await new Promise<void>((resolve) => setImmediate(resolve));
181209

182210
expect(onThinking).not.toHaveBeenCalled();
183211
});
184212

213+
it("fires thinking callback only once per message even with multiple reasoning parts", async () => {
214+
const onThinking = vi.fn();
215+
summaryAggregator.setOnThinking(onThinking);
216+
summaryAggregator.setSession("session-1");
217+
218+
summaryAggregator.processEvent({
219+
type: "message.updated",
220+
properties: {
221+
info: {
222+
id: "message-multi-reasoning",
223+
sessionID: "session-1",
224+
role: "assistant",
225+
time: { created: Date.now() },
226+
},
227+
},
228+
} as unknown as Event);
229+
230+
for (let i = 0; i < 3; i++) {
231+
summaryAggregator.processEvent({
232+
type: "message.part.updated",
233+
properties: {
234+
part: {
235+
id: `part-reasoning-${i}`,
236+
sessionID: "session-1",
237+
messageID: "message-multi-reasoning",
238+
type: "reasoning",
239+
text: `Thinking step ${i}`,
240+
time: { start: Date.now() },
241+
},
242+
},
243+
} as unknown as Event);
244+
}
245+
246+
await new Promise<void>((resolve) => setImmediate(resolve));
247+
248+
expect(onThinking).toHaveBeenCalledTimes(1);
249+
expect(onThinking).toHaveBeenCalledWith("session-1");
250+
});
251+
185252
it("sends apply_patch payload as tool file", () => {
186253
const onToolFile = vi.fn();
187254
summaryAggregator.setOnToolFile(onToolFile);

0 commit comments

Comments
 (0)