Skip to content

Commit 172afff

Browse files
committed
feat: Stream reasoning events as agent thoughts
We weren't showing any reasoning/thinking before, but this also allows for streaming in the reasoning deltas if the backend supports it.
1 parent 7e18de9 commit 172afff

7 files changed

Lines changed: 243 additions & 27 deletions

src/CodexAcpClient.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ export class CodexAcpClient {
459459
input: input,
460460
approvalPolicy: agentMode.approvalPolicy,
461461
sandboxPolicy: agentMode.sandboxPolicy,
462-
summary: disableSummary ? "none" : null,
462+
summary: disableSummary ? "none" : "auto",
463463
effort: effort,
464464
model: modelId.model,
465465
serviceTier: serviceTier,

src/CodexEventHandler.ts

Lines changed: 73 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ import type {
1818
ItemCompletedNotification,
1919
ItemStartedNotification, ThreadItem,
2020
ModelReroutedNotification,
21+
ReasoningSummaryPartAddedNotification,
22+
ReasoningSummaryTextDeltaNotification,
23+
ReasoningTextDeltaNotification,
2124
ThreadTokenUsageUpdatedNotification,
2225
TurnPlanUpdatedNotification,
2326
WarningNotification
@@ -45,6 +48,7 @@ export class CodexEventHandler {
4548
private readonly sessionState: SessionState;
4649
private failure: RequestError | null = null;
4750
private readonly activeFuzzyFileSearchSessions = new Set<string>();
51+
private readonly seenReasoningDeltaItemIds = new Set<string>();
4852

4953
constructor(connection: acp.AgentSideConnection, sessionState: SessionState) {
5054
this.connection = connection;
@@ -125,13 +129,13 @@ export class CodexEventHandler {
125129
case "guardianWarning":
126130
return this.createGuardianWarningEvent(notification.params);
127131
case "thread/compacted":
128-
return {
129-
sessionUpdate: "agent_message_chunk",
130-
content: {
131-
type: "text",
132-
text: "*Context compacted to fit the model's context window.*\n\n"
133-
}
134-
};
132+
return this.createContextCompactedEvent();
133+
case "item/reasoning/summaryTextDelta":
134+
return this.createReasoningDeltaEvent(notification.params);
135+
case "item/reasoning/textDelta":
136+
return this.createReasoningDeltaEvent(notification.params);
137+
case "item/reasoning/summaryPartAdded":
138+
return this.createReasoningSectionBreakEvent(notification.params);
135139
case "model/rerouted":
136140
return this.createModelReroutedEvent(notification.params);
137141
case "fuzzyFileSearch/sessionUpdated":
@@ -144,9 +148,6 @@ export class CodexEventHandler {
144148
case "item/autoApprovalReview/completed":
145149
case "hook/started":
146150
case "hook/completed":
147-
case "item/reasoning/summaryTextDelta":
148-
case "item/reasoning/summaryPartAdded":
149-
case "item/reasoning/textDelta":
150151
case "turn/diff/updated":
151152
case "item/commandExecution/terminalInteraction":
152153
case "item/fileChange/outputDelta":
@@ -242,6 +243,28 @@ export class CodexEventHandler {
242243
};
243244
}
244245

246+
private createReasoningDeltaEvent(
247+
event: ReasoningSummaryTextDeltaNotification | ReasoningTextDeltaNotification
248+
): UpdateSessionEvent {
249+
this.seenReasoningDeltaItemIds.add(event.itemId);
250+
return this.createAgentThoughtEvent(event.delta);
251+
}
252+
253+
private createReasoningSectionBreakEvent(event: ReasoningSummaryPartAddedNotification): UpdateSessionEvent {
254+
this.seenReasoningDeltaItemIds.add(event.itemId);
255+
return this.createAgentThoughtEvent("\n\n");
256+
}
257+
258+
private createAgentThoughtEvent(text: string): UpdateSessionEvent {
259+
return {
260+
sessionUpdate: "agent_thought_chunk",
261+
content: {
262+
type: "text",
263+
text,
264+
}
265+
};
266+
}
267+
245268
private async createItemEvent(event: ItemStartedNotification): Promise<UpdateSessionEvent | null> {
246269
switch (event.item.type) {
247270
case "fileChange":
@@ -288,15 +311,10 @@ export class CodexEventHandler {
288311
case "commandExecution":
289312
return this.completeCommandExecutionEvent(event.item);
290313
case "reasoning":
291-
const summary = event.item.summary[0];
292-
if (!summary) return null;
293-
return {
294-
sessionUpdate: "agent_thought_chunk",
295-
content: {
296-
type: "text",
297-
text: summary
298-
}
314+
if (this.seenReasoningDeltaItemIds.delete(event.item.id)) {
315+
return null;
299316
}
317+
return this.createCompletedReasoningEvent(event.item);
300318
case "collabAgentToolCall":
301319
case "userMessage":
302320
case "hookPrompt":
@@ -305,11 +323,46 @@ export class CodexEventHandler {
305323
case "imageView":
306324
case "imageGeneration":
307325
case "enteredReviewMode":
308-
case "exitedReviewMode":
309-
case "contextCompaction":
310326
case "plan":
311327
return null;
328+
case "exitedReviewMode":
329+
return this.createExitedReviewModeEvent(event.item);
330+
case "contextCompaction":
331+
return this.createContextCompactedEvent();
332+
}
333+
}
334+
335+
private createCompletedReasoningEvent(item: ThreadItem & { type: "reasoning" }): UpdateSessionEvent | null {
336+
const parts = item.summary.length > 0 ? item.summary : item.content;
337+
const text = parts.filter(part => part.length > 0).join("\n\n");
338+
if (text.length === 0) {
339+
return null;
312340
}
341+
return this.createAgentThoughtEvent(text);
342+
}
343+
344+
private createExitedReviewModeEvent(item: ThreadItem & { type: "exitedReviewMode" }): UpdateSessionEvent | null {
345+
const text = item.review.trim();
346+
if (text.length === 0) {
347+
return null;
348+
}
349+
return {
350+
sessionUpdate: "agent_message_chunk",
351+
content: {
352+
type: "text",
353+
text,
354+
}
355+
};
356+
}
357+
358+
private createContextCompactedEvent(): UpdateSessionEvent {
359+
return {
360+
sessionUpdate: "agent_message_chunk",
361+
content: {
362+
type: "text",
363+
text: "*Context compacted to fit the model's context window.*\n\n"
364+
}
365+
};
313366
}
314367

315368
private createCommandOutputDeltaEvent(event: CommandExecutionOutputDeltaNotification): UpdateSessionEvent {

src/__tests__/CodexACPAgent/CodexAcpClient.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -998,22 +998,22 @@ describe('ACP server test', { timeout: 40_000 }, () => {
998998
return { mockFixture, sessionState, turnStartSpy };
999999
}
10001000

1001-
it ('should disable resasoning.summary if key authorization is used', async () => {
1001+
it ('should disable reasoning.summary if key authorization is used', async () => {
10021002
const { mockFixture, turnStartSpy } = setupPromptFixture({ account: { type: "apiKey" } });
10031003

10041004
await mockFixture.getCodexAcpAgent().prompt({ sessionId: "id", prompt: [{ type: "text", text: "test" }] });
10051005

10061006
expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: "none" }));
10071007
});
10081008

1009-
it ('should not disable resasoning.summary by default', async () => {
1009+
it ('should enable reasoning.summary by default', async () => {
10101010
const { mockFixture, turnStartSpy } = setupPromptFixture({
10111011
account: { type: "chatgpt", email: "test@example.com", planType: "pro" },
10121012
});
10131013

10141014
await mockFixture.getCodexAcpAgent().prompt({ sessionId: "id", prompt: [{ type: "text", text: "test" }] });
10151015

1016-
expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: null }));
1016+
expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: "auto" }));
10171017
});
10181018

10191019
it ('should disable reasoning.summary when model lacks reasoning', async () => {
@@ -1027,7 +1027,7 @@ describe('ACP server test', { timeout: 40_000 }, () => {
10271027
expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: "none" }));
10281028
});
10291029

1030-
it ('should not disable reasoning.summary when model supports reasoning', async () => {
1030+
it ('should enable reasoning.summary when model supports reasoning', async () => {
10311031
const { mockFixture, turnStartSpy } = setupPromptFixture({
10321032
account: { type: "chatgpt", email: "test@example.com", planType: "pro" },
10331033
supportedReasoningEfforts: [
@@ -1038,7 +1038,7 @@ describe('ACP server test', { timeout: 40_000 }, () => {
10381038

10391039
await mockFixture.getCodexAcpAgent().prompt({ sessionId: "id", prompt: [{ type: "text", text: "test" }] });
10401040

1041-
expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: null }));
1041+
expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ summary: "auto" }));
10421042
});
10431043

10441044
it ('should reject prompt with images when model does not support image input', async () => {
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"method": "sessionUpdate",
3+
"args": [
4+
{
5+
"sessionId": "test-session-id",
6+
"update": {
7+
"sessionUpdate": "agent_thought_chunk",
8+
"content": {
9+
"type": "text",
10+
"text": "First summary\n\nSecond summary"
11+
}
12+
}
13+
}
14+
]
15+
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
{
2+
"method": "sessionUpdate",
3+
"args": [
4+
{
5+
"sessionId": "test-session-id",
6+
"update": {
7+
"sessionUpdate": "agent_thought_chunk",
8+
"content": {
9+
"type": "text",
10+
"text": "First thought"
11+
}
12+
}
13+
}
14+
]
15+
}
16+
{
17+
"method": "sessionUpdate",
18+
"args": [
19+
{
20+
"sessionId": "test-session-id",
21+
"update": {
22+
"sessionUpdate": "agent_thought_chunk",
23+
"content": {
24+
"type": "text",
25+
"text": "\n\n"
26+
}
27+
}
28+
}
29+
]
30+
}
31+
{
32+
"method": "sessionUpdate",
33+
"args": [
34+
{
35+
"sessionId": "test-session-id",
36+
"update": {
37+
"sessionUpdate": "agent_thought_chunk",
38+
"content": {
39+
"type": "text",
40+
"text": "Raw reasoning detail"
41+
}
42+
}
43+
}
44+
]
45+
}

src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
"excludeTmpdirEnvVar": false,
5252
"excludeSlashTmp": false
5353
},
54-
"summary": null,
54+
"summary": "auto",
5555
"effort": "effort",
5656
"model": "model",
5757
"serviceTier": null
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
import type { ServerNotification } from "../../app-server";
3+
import type { SessionState } from "../../CodexAcpServer";
4+
import { AgentMode } from "../../AgentMode";
5+
import {
6+
createCodexMockTestFixture,
7+
createTestSessionState,
8+
setupPromptAndSendNotifications,
9+
type CodexMockTestFixture
10+
} from "../acp-test-utils";
11+
12+
describe("CodexEventHandler - reasoning events", () => {
13+
let mockFixture: CodexMockTestFixture;
14+
const sessionId = "test-session-id";
15+
16+
beforeEach(() => {
17+
mockFixture = createCodexMockTestFixture();
18+
vi.clearAllMocks();
19+
});
20+
21+
const sessionState: SessionState = createTestSessionState({
22+
sessionId,
23+
currentModelId: "model-id[effort]",
24+
agentMode: AgentMode.DEFAULT_AGENT_MODE
25+
});
26+
27+
it("streams reasoning deltas and section breaks without duplicating the completed item", async () => {
28+
const notifications: ServerNotification[] = [
29+
{
30+
method: "item/reasoning/summaryTextDelta",
31+
params: {
32+
threadId: sessionId,
33+
turnId: "turn-1",
34+
itemId: "reasoning-1",
35+
summaryIndex: 0,
36+
delta: "First thought",
37+
},
38+
},
39+
{
40+
method: "item/reasoning/summaryPartAdded",
41+
params: {
42+
threadId: sessionId,
43+
turnId: "turn-1",
44+
itemId: "reasoning-1",
45+
summaryIndex: 1,
46+
},
47+
},
48+
{
49+
method: "item/reasoning/textDelta",
50+
params: {
51+
threadId: sessionId,
52+
turnId: "turn-1",
53+
itemId: "reasoning-1",
54+
contentIndex: 0,
55+
delta: "Raw reasoning detail",
56+
},
57+
},
58+
{
59+
method: "item/completed",
60+
params: {
61+
threadId: sessionId,
62+
turnId: "turn-1",
63+
item: {
64+
type: "reasoning",
65+
id: "reasoning-1",
66+
summary: ["Completed summary should not duplicate"],
67+
content: ["Completed content should not duplicate"],
68+
},
69+
},
70+
},
71+
];
72+
73+
await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, notifications);
74+
75+
await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot(
76+
"data/reasoning-deltas-and-section-break.json"
77+
);
78+
});
79+
80+
it("emits all completed reasoning parts when no deltas streamed", async () => {
81+
const notifications: ServerNotification[] = [
82+
{
83+
method: "item/completed",
84+
params: {
85+
threadId: sessionId,
86+
turnId: "turn-1",
87+
item: {
88+
type: "reasoning",
89+
id: "reasoning-2",
90+
summary: ["First summary", "Second summary"],
91+
content: ["Raw content fallback"],
92+
},
93+
},
94+
},
95+
];
96+
97+
await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, notifications);
98+
99+
await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot(
100+
"data/reasoning-completed-parts.json"
101+
);
102+
});
103+
});

0 commit comments

Comments
 (0)