Skip to content

Commit ad7baf6

Browse files
committed
Fix context pruner
1 parent 89a023f commit ad7baf6

File tree

2 files changed

+385
-18
lines changed

2 files changed

+385
-18
lines changed

.agents/__tests__/context-pruner.test.ts

Lines changed: 329 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, test, expect, beforeEach } from 'bun:test'
22

33
import contextPruner from '../context-pruner'
44

5-
import type { Message, ToolMessage } from '../types/util-types'
5+
import type { JSONValue, Message, ToolMessage } from '../types/util-types'
66
const createMessage = (
77
role: 'user' | 'assistant',
88
content: string,
@@ -216,11 +216,15 @@ describe('context-pruner handleSteps', () => {
216216
// Should have fewer messages due to pruning
217217
expect(resultMessages.length).toBeLessThan(messages.length)
218218

219-
// Should contain replacement messages
219+
// Should contain replacement messages (content is an array of parts)
220220
const hasReplacementMessage = resultMessages.some(
221221
(m: any) =>
222-
typeof m.content === 'string' &&
223-
m.content.includes('Previous message(s) omitted due to length'),
222+
Array.isArray(m.content) &&
223+
m.content.some(
224+
(part: any) =>
225+
part.type === 'text' &&
226+
part.text.includes('Previous message(s) omitted due to length'),
227+
),
224228
)
225229
expect(hasReplacementMessage).toBe(true)
226230
})
@@ -242,11 +246,14 @@ describe('context-pruner handleSteps', () => {
242246
expect(results).toHaveLength(1)
243247
const resultMessages = results[0].input.messages
244248

245-
// Important message should be preserved
249+
// Important message should be preserved (content is an array of parts)
246250
const importantMessage = resultMessages.find(
247251
(m: any) =>
248-
typeof m.content === 'string' &&
249-
m.content.includes('Important message'),
252+
Array.isArray(m.content) &&
253+
m.content.some(
254+
(part: any) =>
255+
part.type === 'text' && part.text.includes('Important message'),
256+
),
250257
)
251258
expect(importantMessage).toBeDefined()
252259
})
@@ -291,6 +298,321 @@ describe('context-pruner handleSteps', () => {
291298
})
292299
})
293300

301+
describe('context-pruner tool-call/tool-result pair preservation', () => {
302+
let mockAgentState: any
303+
304+
beforeEach(() => {
305+
mockAgentState = {
306+
messageHistory: [] as Message[],
307+
}
308+
})
309+
310+
const createToolCallMessage = (
311+
toolCallId: string,
312+
toolName: string,
313+
input: Record<string, unknown>,
314+
): Message => ({
315+
role: 'assistant',
316+
content: [
317+
{
318+
type: 'tool-call',
319+
toolCallId,
320+
toolName,
321+
input,
322+
},
323+
],
324+
})
325+
326+
const createToolResultMessage = (
327+
toolCallId: string,
328+
toolName: string,
329+
value: unknown,
330+
): ToolMessage => ({
331+
role: 'tool',
332+
toolCallId,
333+
toolName,
334+
content: [
335+
{
336+
type: 'json',
337+
value: value as JSONValue,
338+
},
339+
],
340+
})
341+
342+
const runHandleSteps = (messages: Message[]) => {
343+
mockAgentState.messageHistory = messages
344+
const mockLogger = {
345+
debug: () => {},
346+
info: () => {},
347+
warn: () => {},
348+
error: () => {},
349+
}
350+
const generator = contextPruner.handleSteps!({
351+
agentState: mockAgentState,
352+
logger: mockLogger,
353+
})
354+
const results: any[] = []
355+
let result = generator.next()
356+
while (!result.done) {
357+
if (typeof result.value === 'object') {
358+
results.push(result.value)
359+
}
360+
result = generator.next()
361+
}
362+
return results
363+
}
364+
365+
test('preserves tool-call and tool-result pairs together during message pruning', () => {
366+
const largeContent = 'x'.repeat(50000)
367+
368+
// Create messages with tool-call/tool-result pairs interspersed with regular messages
369+
const messages: Message[] = [
370+
createMessage('user', `First: ${largeContent}`),
371+
createMessage('assistant', `Response 1: ${largeContent}`),
372+
createMessage('user', `Second: ${largeContent}`),
373+
// Tool call pair that should be kept together
374+
createToolCallMessage('call-1', 'read_files', { paths: ['test.ts'] }),
375+
createToolResultMessage('call-1', 'read_files', { content: 'small' }),
376+
createMessage('user', `Third: ${largeContent}`),
377+
createMessage('assistant', `Response 2: ${largeContent}`),
378+
createMessage('user', `Fourth: ${largeContent}`),
379+
]
380+
381+
const results = runHandleSteps(messages)
382+
383+
expect(results).toHaveLength(1)
384+
const resultMessages = results[0].input.messages
385+
386+
// Find the tool call and result
387+
const toolCall = resultMessages.find(
388+
(m: any) =>
389+
m.role === 'assistant' &&
390+
Array.isArray(m.content) &&
391+
m.content.some(
392+
(c: any) => c.type === 'tool-call' && c.toolCallId === 'call-1',
393+
),
394+
)
395+
const toolResult = resultMessages.find(
396+
(m: any) => m.role === 'tool' && m.toolCallId === 'call-1',
397+
)
398+
399+
// Both should be present (kept together) or both absent
400+
if (toolCall) {
401+
expect(toolResult).toBeDefined()
402+
}
403+
if (toolResult) {
404+
expect(toolCall).toBeDefined()
405+
}
406+
})
407+
408+
test('never removes tool-call message while keeping its tool-result', () => {
409+
const largeContent = 'x'.repeat(60000)
410+
411+
const messages: Message[] = [
412+
createMessage('user', `Start: ${largeContent}`),
413+
createMessage('assistant', `Middle: ${largeContent}`),
414+
createToolCallMessage('call-abc', 'code_search', { pattern: 'test' }),
415+
createToolResultMessage('call-abc', 'code_search', { results: [] }),
416+
createMessage('user', `End: ${largeContent}`),
417+
createMessage('assistant', `Final: ${largeContent}`),
418+
]
419+
420+
const results = runHandleSteps(messages)
421+
const resultMessages = results[0].input.messages
422+
423+
// Check for orphaned tool results (tool result without matching tool call)
424+
const toolResults = resultMessages.filter((m: any) => m.role === 'tool')
425+
for (const toolResult of toolResults) {
426+
const matchingCall = resultMessages.find(
427+
(m: any) =>
428+
m.role === 'assistant' &&
429+
Array.isArray(m.content) &&
430+
m.content.some(
431+
(c: any) =>
432+
c.type === 'tool-call' && c.toolCallId === toolResult.toolCallId,
433+
),
434+
)
435+
expect(matchingCall).toBeDefined()
436+
}
437+
})
438+
439+
test('never removes tool-result message while keeping its tool-call', () => {
440+
const largeContent = 'x'.repeat(60000)
441+
442+
const messages: Message[] = [
443+
createMessage('user', `A: ${largeContent}`),
444+
createToolCallMessage('call-xyz', 'find_files', { pattern: '*.ts' }),
445+
createToolResultMessage('call-xyz', 'find_files', { files: ['a.ts'] }),
446+
createMessage('assistant', `B: ${largeContent}`),
447+
createMessage('user', `C: ${largeContent}`),
448+
]
449+
450+
const results = runHandleSteps(messages)
451+
const resultMessages = results[0].input.messages
452+
453+
// Check for orphaned tool calls (tool call without matching tool result)
454+
const toolCalls = resultMessages.filter(
455+
(m: any) =>
456+
m.role === 'assistant' &&
457+
Array.isArray(m.content) &&
458+
m.content.some((c: any) => c.type === 'tool-call'),
459+
)
460+
461+
for (const toolCallMsg of toolCalls) {
462+
for (const part of toolCallMsg.content) {
463+
if (part.type === 'tool-call') {
464+
const matchingResult = resultMessages.find(
465+
(m: any) => m.role === 'tool' && m.toolCallId === part.toolCallId,
466+
)
467+
expect(matchingResult).toBeDefined()
468+
}
469+
}
470+
}
471+
})
472+
473+
test('preserves multiple tool-call/tool-result pairs in same context', () => {
474+
const largeContent = 'x'.repeat(40000)
475+
476+
const messages: Message[] = [
477+
createMessage('user', `Request: ${largeContent}`),
478+
// First tool call pair
479+
createToolCallMessage('call-1', 'read_files', { paths: ['a.ts'] }),
480+
createToolResultMessage('call-1', 'read_files', { content: 'file a' }),
481+
// Second tool call pair
482+
createToolCallMessage('call-2', 'read_files', { paths: ['b.ts'] }),
483+
createToolResultMessage('call-2', 'read_files', { content: 'file b' }),
484+
// Third tool call pair
485+
createToolCallMessage('call-3', 'code_search', { pattern: 'foo' }),
486+
createToolResultMessage('call-3', 'code_search', { matches: [] }),
487+
createMessage('assistant', `Response: ${largeContent}`),
488+
createMessage('user', `Follow up: ${largeContent}`),
489+
]
490+
491+
const results = runHandleSteps(messages)
492+
const resultMessages = results[0].input.messages
493+
494+
// Verify each tool call has its corresponding result
495+
const toolCallIds = ['call-1', 'call-2', 'call-3']
496+
for (const callId of toolCallIds) {
497+
const hasToolCall = resultMessages.some(
498+
(m: any) =>
499+
m.role === 'assistant' &&
500+
Array.isArray(m.content) &&
501+
m.content.some(
502+
(c: any) => c.type === 'tool-call' && c.toolCallId === callId,
503+
),
504+
)
505+
const hasToolResult = resultMessages.some(
506+
(m: any) => m.role === 'tool' && m.toolCallId === callId,
507+
)
508+
509+
// Either both exist or neither exists
510+
expect(hasToolCall).toBe(hasToolResult)
511+
}
512+
})
513+
514+
test('abridges tool result content while preserving the pair structure', () => {
515+
const largeContent = 'x'.repeat(150000)
516+
const largeToolResult = 'y'.repeat(2000) // > 1000 chars, triggers abridging
517+
518+
const messages: Message[] = [
519+
createMessage('user', largeContent),
520+
createMessage('assistant', largeContent),
521+
createMessage('user', largeContent),
522+
createMessage('assistant', largeContent),
523+
createToolCallMessage('call-large', 'read_files', { paths: ['big.ts'] }),
524+
createToolResultMessage('call-large', 'read_files', {
525+
content: largeToolResult,
526+
}),
527+
]
528+
529+
const results = runHandleSteps(messages)
530+
const resultMessages = results[0].input.messages
531+
532+
// Tool call should be unchanged
533+
const toolCall = resultMessages.find(
534+
(m: any) =>
535+
m.role === 'assistant' &&
536+
Array.isArray(m.content) &&
537+
m.content.some((c: any) => c.toolCallId === 'call-large'),
538+
)
539+
expect(toolCall).toBeDefined()
540+
expect(toolCall.content[0].input).toEqual({ paths: ['big.ts'] })
541+
542+
// Tool result should be abridged but still present with same toolCallId
543+
const toolResult = resultMessages.find(
544+
(m: any) => m.role === 'tool' && m.toolCallId === 'call-large',
545+
)
546+
expect(toolResult).toBeDefined()
547+
expect(toolResult.content[0].value.message).toBe(
548+
'[LARGE_TOOL_RESULT_OMITTED]',
549+
)
550+
})
551+
552+
test('handles assistant message with multiple tool calls', () => {
553+
const largeContent = 'x'.repeat(50000)
554+
555+
// Assistant message with multiple tool calls in one message
556+
const multiToolCallMessage: Message = {
557+
role: 'assistant',
558+
content: [
559+
{
560+
type: 'tool-call',
561+
toolCallId: 'multi-1',
562+
toolName: 'read_files',
563+
input: { paths: ['file1.ts'] },
564+
},
565+
{
566+
type: 'tool-call',
567+
toolCallId: 'multi-2',
568+
toolName: 'read_files',
569+
input: { paths: ['file2.ts'] },
570+
},
571+
],
572+
}
573+
574+
const messages: Message[] = [
575+
createMessage('user', `Request: ${largeContent}`),
576+
multiToolCallMessage,
577+
createToolResultMessage('multi-1', 'read_files', { content: 'file1' }),
578+
createToolResultMessage('multi-2', 'read_files', { content: 'file2' }),
579+
createMessage('user', `More: ${largeContent}`),
580+
createMessage('assistant', `Done: ${largeContent}`),
581+
]
582+
583+
const results = runHandleSteps(messages)
584+
const resultMessages = results[0].input.messages
585+
586+
// Both tool results should have their corresponding tool calls
587+
const result1 = resultMessages.find(
588+
(m: any) => m.role === 'tool' && m.toolCallId === 'multi-1',
589+
)
590+
const result2 = resultMessages.find(
591+
(m: any) => m.role === 'tool' && m.toolCallId === 'multi-2',
592+
)
593+
594+
if (result1) {
595+
const hasCall1 = resultMessages.some(
596+
(m: any) =>
597+
m.role === 'assistant' &&
598+
Array.isArray(m.content) &&
599+
m.content.some((c: any) => c.toolCallId === 'multi-1'),
600+
)
601+
expect(hasCall1).toBe(true)
602+
}
603+
604+
if (result2) {
605+
const hasCall2 = resultMessages.some(
606+
(m: any) =>
607+
m.role === 'assistant' &&
608+
Array.isArray(m.content) &&
609+
m.content.some((c: any) => c.toolCallId === 'multi-2'),
610+
)
611+
expect(hasCall2).toBe(true)
612+
}
613+
})
614+
})
615+
294616
describe('context-pruner edge cases', () => {
295617
let mockAgentState: any
296618

0 commit comments

Comments
 (0)