Skip to content

Commit 311940b

Browse files
authored
fix: preserve tool_use blocks in summary message during condensing with native tools (#9582)
1 parent 774b492 commit 311940b

File tree

5 files changed

+399
-4
lines changed

5 files changed

+399
-4
lines changed

src/core/condense/__tests__/index.spec.ts

Lines changed: 294 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@ import { TelemetryService } from "@roo-code/telemetry"
77
import { ApiHandler } from "../../../api"
88
import { ApiMessage } from "../../task-persistence/apiMessages"
99
import { maybeRemoveImageBlocks } from "../../../api/transform/image-cleaning"
10-
import { summarizeConversation, getMessagesSinceLastSummary, N_MESSAGES_TO_KEEP } from "../index"
10+
import {
11+
summarizeConversation,
12+
getMessagesSinceLastSummary,
13+
getKeepMessagesWithToolBlocks,
14+
N_MESSAGES_TO_KEEP,
15+
} from "../index"
1116

1217
vi.mock("../../../api/transform/image-cleaning", () => ({
1318
maybeRemoveImageBlocks: vi.fn((messages: ApiMessage[], _apiHandler: ApiHandler) => [...messages]),
@@ -24,6 +29,222 @@ vi.mock("@roo-code/telemetry", () => ({
2429
const taskId = "test-task-id"
2530
const DEFAULT_PREV_CONTEXT_TOKENS = 1000
2631

32+
describe("getKeepMessagesWithToolBlocks", () => {
33+
it("should return keepMessages without tool blocks when no tool_result blocks in first kept message", () => {
34+
const messages: ApiMessage[] = [
35+
{ role: "user", content: "Hello", ts: 1 },
36+
{ role: "assistant", content: "Hi there", ts: 2 },
37+
{ role: "user", content: "How are you?", ts: 3 },
38+
{ role: "assistant", content: "I'm good", ts: 4 },
39+
{ role: "user", content: "What's new?", ts: 5 },
40+
]
41+
42+
const result = getKeepMessagesWithToolBlocks(messages, 3)
43+
44+
expect(result.keepMessages).toHaveLength(3)
45+
expect(result.toolUseBlocksToPreserve).toHaveLength(0)
46+
})
47+
48+
it("should return all messages when messages.length <= keepCount", () => {
49+
const messages: ApiMessage[] = [
50+
{ role: "user", content: "Hello", ts: 1 },
51+
{ role: "assistant", content: "Hi there", ts: 2 },
52+
]
53+
54+
const result = getKeepMessagesWithToolBlocks(messages, 3)
55+
56+
expect(result.keepMessages).toEqual(messages)
57+
expect(result.toolUseBlocksToPreserve).toHaveLength(0)
58+
})
59+
60+
it("should preserve tool_use blocks when first kept message has tool_result blocks", () => {
61+
const toolUseBlock = {
62+
type: "tool_use" as const,
63+
id: "toolu_123",
64+
name: "read_file",
65+
input: { path: "test.txt" },
66+
}
67+
const toolResultBlock = {
68+
type: "tool_result" as const,
69+
tool_use_id: "toolu_123",
70+
content: "file contents",
71+
}
72+
73+
const messages: ApiMessage[] = [
74+
{ role: "user", content: "Hello", ts: 1 },
75+
{ role: "assistant", content: "Let me read that file", ts: 2 },
76+
{ role: "user", content: "Please continue", ts: 3 },
77+
{
78+
role: "assistant",
79+
content: [{ type: "text" as const, text: "Reading file..." }, toolUseBlock],
80+
ts: 4,
81+
},
82+
{
83+
role: "user",
84+
content: [toolResultBlock, { type: "text" as const, text: "Continue" }],
85+
ts: 5,
86+
},
87+
{ role: "assistant", content: "Got it, the file says...", ts: 6 },
88+
{ role: "user", content: "Thanks", ts: 7 },
89+
]
90+
91+
const result = getKeepMessagesWithToolBlocks(messages, 3)
92+
93+
// keepMessages should be the last 3 messages
94+
expect(result.keepMessages).toHaveLength(3)
95+
expect(result.keepMessages[0].ts).toBe(5)
96+
expect(result.keepMessages[1].ts).toBe(6)
97+
expect(result.keepMessages[2].ts).toBe(7)
98+
99+
// Should preserve the tool_use block from the preceding assistant message
100+
expect(result.toolUseBlocksToPreserve).toHaveLength(1)
101+
expect(result.toolUseBlocksToPreserve[0]).toEqual(toolUseBlock)
102+
})
103+
104+
it("should not preserve tool_use blocks when first kept message is assistant role", () => {
105+
const toolUseBlock = {
106+
type: "tool_use" as const,
107+
id: "toolu_123",
108+
name: "read_file",
109+
input: { path: "test.txt" },
110+
}
111+
112+
const messages: ApiMessage[] = [
113+
{ role: "user", content: "Hello", ts: 1 },
114+
{ role: "assistant", content: "Hi there", ts: 2 },
115+
{ role: "user", content: "Please read", ts: 3 },
116+
{
117+
role: "assistant",
118+
content: [{ type: "text" as const, text: "Reading..." }, toolUseBlock],
119+
ts: 4,
120+
},
121+
{ role: "user", content: "Continue", ts: 5 },
122+
{ role: "assistant", content: "Done", ts: 6 },
123+
]
124+
125+
const result = getKeepMessagesWithToolBlocks(messages, 3)
126+
127+
// First kept message is assistant, not user with tool_result
128+
expect(result.keepMessages).toHaveLength(3)
129+
expect(result.keepMessages[0].role).toBe("assistant")
130+
expect(result.toolUseBlocksToPreserve).toHaveLength(0)
131+
})
132+
133+
it("should not preserve tool_use blocks when first kept user message has string content", () => {
134+
const messages: ApiMessage[] = [
135+
{ role: "user", content: "Hello", ts: 1 },
136+
{ role: "assistant", content: "Hi there", ts: 2 },
137+
{ role: "user", content: "How are you?", ts: 3 },
138+
{ role: "assistant", content: "Good", ts: 4 },
139+
{ role: "user", content: "Simple text message", ts: 5 }, // String content, not array
140+
{ role: "assistant", content: "Response", ts: 6 },
141+
{ role: "user", content: "More text", ts: 7 },
142+
]
143+
144+
const result = getKeepMessagesWithToolBlocks(messages, 3)
145+
146+
expect(result.keepMessages).toHaveLength(3)
147+
expect(result.toolUseBlocksToPreserve).toHaveLength(0)
148+
})
149+
150+
it("should handle multiple tool_use blocks that need to be preserved", () => {
151+
const toolUseBlock1 = {
152+
type: "tool_use" as const,
153+
id: "toolu_123",
154+
name: "read_file",
155+
input: { path: "file1.txt" },
156+
}
157+
const toolUseBlock2 = {
158+
type: "tool_use" as const,
159+
id: "toolu_456",
160+
name: "read_file",
161+
input: { path: "file2.txt" },
162+
}
163+
const toolResultBlock1 = {
164+
type: "tool_result" as const,
165+
tool_use_id: "toolu_123",
166+
content: "contents 1",
167+
}
168+
const toolResultBlock2 = {
169+
type: "tool_result" as const,
170+
tool_use_id: "toolu_456",
171+
content: "contents 2",
172+
}
173+
174+
const messages: ApiMessage[] = [
175+
{ role: "user", content: "Hello", ts: 1 },
176+
{
177+
role: "assistant",
178+
content: [{ type: "text" as const, text: "Reading files..." }, toolUseBlock1, toolUseBlock2],
179+
ts: 2,
180+
},
181+
{
182+
role: "user",
183+
content: [toolResultBlock1, toolResultBlock2],
184+
ts: 3,
185+
},
186+
{ role: "assistant", content: "Got both files", ts: 4 },
187+
{ role: "user", content: "Thanks", ts: 5 },
188+
]
189+
190+
const result = getKeepMessagesWithToolBlocks(messages, 3)
191+
192+
// Should preserve both tool_use blocks
193+
expect(result.toolUseBlocksToPreserve).toHaveLength(2)
194+
expect(result.toolUseBlocksToPreserve).toContainEqual(toolUseBlock1)
195+
expect(result.toolUseBlocksToPreserve).toContainEqual(toolUseBlock2)
196+
})
197+
198+
it("should not preserve tool_use blocks when preceding message has no tool_use blocks", () => {
199+
const toolResultBlock = {
200+
type: "tool_result" as const,
201+
tool_use_id: "toolu_123",
202+
content: "file contents",
203+
}
204+
205+
const messages: ApiMessage[] = [
206+
{ role: "user", content: "Hello", ts: 1 },
207+
{ role: "assistant", content: "Plain text response", ts: 2 }, // No tool_use blocks
208+
{
209+
role: "user",
210+
content: [toolResultBlock], // Has tool_result but preceding message has no tool_use
211+
ts: 3,
212+
},
213+
{ role: "assistant", content: "Response", ts: 4 },
214+
{ role: "user", content: "Thanks", ts: 5 },
215+
]
216+
217+
const result = getKeepMessagesWithToolBlocks(messages, 3)
218+
219+
expect(result.keepMessages).toHaveLength(3)
220+
expect(result.toolUseBlocksToPreserve).toHaveLength(0)
221+
})
222+
223+
it("should handle edge case when startIndex - 1 is negative", () => {
224+
const toolResultBlock = {
225+
type: "tool_result" as const,
226+
tool_use_id: "toolu_123",
227+
content: "file contents",
228+
}
229+
230+
// Only 3 messages total, so startIndex = 0 and precedingIndex would be -1
231+
const messages: ApiMessage[] = [
232+
{
233+
role: "user",
234+
content: [toolResultBlock],
235+
ts: 1,
236+
},
237+
{ role: "assistant", content: "Response", ts: 2 },
238+
{ role: "user", content: "Thanks", ts: 3 },
239+
]
240+
241+
const result = getKeepMessagesWithToolBlocks(messages, 3)
242+
243+
expect(result.keepMessages).toEqual(messages)
244+
expect(result.toolUseBlocksToPreserve).toHaveLength(0)
245+
})
246+
})
247+
27248
describe("getMessagesSinceLastSummary", () => {
28249
it("should return all messages when there is no summary", () => {
29250
const messages: ApiMessage[] = [
@@ -535,6 +756,78 @@ describe("summarizeConversation", () => {
535756
// Restore console.error
536757
console.error = originalError
537758
})
759+
760+
it("should append tool_use blocks to summary message when first kept message has tool_result blocks", async () => {
761+
const toolUseBlock = {
762+
type: "tool_use" as const,
763+
id: "toolu_123",
764+
name: "read_file",
765+
input: { path: "test.txt" },
766+
}
767+
const toolResultBlock = {
768+
type: "tool_result" as const,
769+
tool_use_id: "toolu_123",
770+
content: "file contents",
771+
}
772+
773+
const messages: ApiMessage[] = [
774+
{ role: "user", content: "Hello", ts: 1 },
775+
{ role: "assistant", content: "Let me read that file", ts: 2 },
776+
{ role: "user", content: "Please continue", ts: 3 },
777+
{
778+
role: "assistant",
779+
content: [{ type: "text" as const, text: "Reading file..." }, toolUseBlock],
780+
ts: 4,
781+
},
782+
{
783+
role: "user",
784+
content: [toolResultBlock, { type: "text" as const, text: "Continue" }],
785+
ts: 5,
786+
},
787+
{ role: "assistant", content: "Got it, the file says...", ts: 6 },
788+
{ role: "user", content: "Thanks", ts: 7 },
789+
]
790+
791+
// Create a stream with usage information
792+
const streamWithUsage = (async function* () {
793+
yield { type: "text" as const, text: "Summary of conversation" }
794+
yield { type: "usage" as const, totalCost: 0.05, outputTokens: 100 }
795+
})()
796+
797+
mockApiHandler.createMessage = vi.fn().mockReturnValue(streamWithUsage) as any
798+
mockApiHandler.countTokens = vi.fn().mockImplementation(() => Promise.resolve(50)) as any
799+
800+
const result = await summarizeConversation(
801+
messages,
802+
mockApiHandler,
803+
defaultSystemPrompt,
804+
taskId,
805+
DEFAULT_PREV_CONTEXT_TOKENS,
806+
false, // isAutomaticTrigger
807+
undefined, // customCondensingPrompt
808+
undefined, // condensingApiHandler
809+
true, // useNativeTools - required for tool_use block preservation
810+
)
811+
812+
// Verify the summary message has content array with text and tool_use blocks
813+
const summaryMessage = result.messages[1]
814+
expect(summaryMessage.role).toBe("assistant")
815+
expect(summaryMessage.isSummary).toBe(true)
816+
expect(Array.isArray(summaryMessage.content)).toBe(true)
817+
818+
// Content should be [text block, tool_use block]
819+
const content = summaryMessage.content as any[]
820+
expect(content).toHaveLength(2)
821+
expect(content[0].type).toBe("text")
822+
expect(content[0].text).toBe("Summary of conversation")
823+
expect(content[1].type).toBe("tool_use")
824+
expect(content[1].id).toBe("toolu_123")
825+
expect(content[1].name).toBe("read_file")
826+
827+
// Verify the keepMessages are preserved correctly
828+
expect(result.messages).toHaveLength(1 + 1 + N_MESSAGES_TO_KEEP) // first + summary + last 3
829+
expect(result.error).toBeUndefined()
830+
})
538831
})
539832

540833
describe("summarizeConversation with custom settings", () => {

0 commit comments

Comments
 (0)