diff --git a/.changeset/dedupe-reminder-survives-truncation.md b/.changeset/dedupe-reminder-survives-truncation.md new file mode 100644 index 0000000000..d9e631623b --- /dev/null +++ b/.changeset/dedupe-reminder-survives-truncation.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix repeat-tool-call reminders never reaching the model for oversized tool results. The dedupe reminder was appended at the end of the tool output, but the >50K offload path keeps only a 2K head preview, so the reminder was silently cut off exactly in the large-output scenarios where repeat loops are most likely. Reminders are now prepended to the result output so they survive truncation. Covers both the v1 (`agent-core`) and v2 (`agent-core-v2`) engines. diff --git a/packages/agent-core-v2/src/agent/toolDedupe/toolDedupeService.ts b/packages/agent-core-v2/src/agent/toolDedupe/toolDedupeService.ts index bb84242936..ad062dc6dd 100644 --- a/packages/agent-core-v2/src/agent/toolDedupe/toolDedupeService.ts +++ b/packages/agent-core-v2/src/agent/toolDedupe/toolDedupeService.ts @@ -80,18 +80,27 @@ interface CheckedToolCall { readonly syntheticResult: ToolDedupeResult | null; } +/** + * Prepends the reminder to the tool result output. + * + * The reminder MUST be at the head, not the tail: oversized text results are + * later replaced by a head-only preview (`toolResultTruncation` keeps only the + * first 2000 chars after `onDidExecuteTool` hooks run). A tail-appended + * reminder is silently cut off there, so the model never sees it exactly in + * the large-output scenarios where repeat loops are most likely. + */ function appendReminder(result: ToolDedupeResult, reminderText: string): ToolDedupeResult { const output = result.output; let newOutput: string | ContentPart[]; if (typeof output === 'string') { - newOutput = output + reminderText; + newOutput = reminderText + output; } else { - const arr: ContentPart[] = [...output]; - const last = arr.at(-1); - if (last !== undefined && last.type === 'text') { - arr[arr.length - 1] = { type: 'text', text: last.text + reminderText }; + const arr = [...output]; + const first = arr[0]; + if (first !== undefined && first.type === 'text') { + arr[0] = { type: 'text', text: reminderText + first.text }; } else { - arr.push({ type: 'text', text: reminderText }); + arr.unshift({ type: 'text', text: reminderText }); } newOutput = arr; } diff --git a/packages/agent-core-v2/test/agent/toolDedupe/toolDedupe.test.ts b/packages/agent-core-v2/test/agent/toolDedupe/toolDedupe.test.ts index aa6dd80bd5..17b2505d1b 100644 --- a/packages/agent-core-v2/test/agent/toolDedupe/toolDedupe.test.ts +++ b/packages/agent-core-v2/test/agent/toolDedupe/toolDedupe.test.ts @@ -13,6 +13,7 @@ import { IAgentLoopService } from '#/agent/loop/loop'; import type { ExecutableTool, ExecutableToolContext, ExecutableToolResult, ToolExecution, ToolResult } from '#/tool/toolContract'; import type { ToolDidExecuteContext, ResolvedToolExecutionHookContext, BeforeExecuteDecision } from '#/agent/toolExecutor/toolHooks'; import { IAgentToolDedupeService, type ToolDedupeResult } from '#/agent/toolDedupe/toolDedupe'; +import { ToolResultTruncationService } from '#/agent/toolResultTruncation/toolResultTruncationService'; import { AgentToolDedupeService, __testing as toolDedupeTesting } from '#/agent/toolDedupe/toolDedupeService'; import { IAgentToolExecutorService, type ToolExecutionResult } from '#/agent/toolExecutor/toolExecutor'; import { AgentToolExecutorService } from '#/agent/toolExecutor/toolExecutorService'; @@ -441,8 +442,60 @@ describe('AgentToolDedupeService', () => { }); }); + describe('reminder survives oversized-result truncation (regression)', () => { + // Oversized text results are later replaced by a head-only 2K preview + // (agent/toolResultTruncation). Reminders used to be appended at the tail + // and were silently cut off there — the model looped 12 times with no + // reminder ever visible. They must now sit at the head. + it('keeps the reminder inside the first 2K chars of an oversized result', async () => { + const h = createHarness(); + const tool = new EchoTool('Read', () => ({ output: 'x'.repeat(60_000) })); + h.registry.register(tool); + let last: ToolResult | undefined; + for (let i = 0; i < 3; i += 1) { + const [result] = await runStep(h, 1, i + 1, [toolCall(`c${String(i)}`, 'Read', { p: 1 })]); + last = result!.result; + } + const headPreview = (last!.output as string).slice(0, 2_000); + expect(headPreview).toContain(''); + expect(headPreview).toContain('what new information you expect'); + }); + + it('prepends the reminder ahead of the original output', async () => { + const h = createHarness(); + registerRead(h); + const last = await runStreak(h, 3); + expect((last.output as string).startsWith('\n\n')).toBe(true); + }); + + it('keeps the reminder visible through the real ToolResultTruncationService (composition)', async () => { + const h = createHarness(); + const tool = new EchoTool('Read', () => ({ output: 'x'.repeat(60_000) })); + h.registry.register(tool); + let last: ToolResult | undefined; + for (let i = 0; i < 3; i += 1) { + const [result] = await runStep(h, 1, i + 1, [toolCall(`c${String(i)}`, 'Read', { p: 1 })]); + last = result!.result; + } + // Compose with the actual truncation service — the exact chain that used + // to drop tail-appended reminders in production. + const truncation = new ToolResultTruncationService( + { homeDir: '/home/user' } as never, + { scope: (s: string) => s } as never, + { write: async () => undefined } as never, + ); + const modelResult = await truncation.truncateForModel({ + toolName: 'Read', + toolCallId: 'c2', + result: last!, + }); + expect(modelResult.output as string).toContain(''); + expect((modelResult.output as string).indexOf('')).toBeLessThan(2_000); + }); + }); + describe('reminder injection into ContentPart[] outputs', () => { - it('appends reminder1 to a trailing text part at streak 3', async () => { + it('prepends reminder1 to the leading text part at streak 3', async () => { const h = createHarness(); const tool = new EchoTool('X', () => ({ output: [{ type: 'text', text: 'hello' }] })); h.registry.register(tool); @@ -450,10 +503,10 @@ describe('AgentToolDedupeService', () => { await runStep(h, 1, i + 1, [toolCall(`p${String(i)}`, 'X', {})]); } const [final] = await runStep(h, 1, 3, [toolCall('final', 'X', {})]); - expect(final!.result.output).toBe('hello' + REMINDER_TEXT_1); + expect(final!.result.output).toBe(REMINDER_TEXT_1 + 'hello'); }); - it('appends reminder2 to a trailing text part at streak 5', async () => { + it('prepends reminder2 to the leading text part at streak 5', async () => { const h = createHarness(); const tool = new EchoTool('X', () => ({ output: [{ type: 'text', text: 'hello' }] })); h.registry.register(tool); @@ -461,10 +514,10 @@ describe('AgentToolDedupeService', () => { await runStep(h, 1, i + 1, [toolCall(`p${String(i)}`, 'X', { a: 1 })]); } const [final] = await runStep(h, 1, 5, [toolCall('final', 'X', { a: 1 })]); - expect(final!.result.output).toBe('hello' + makeReminderText2(5)); + expect(final!.result.output).toBe(makeReminderText2(5) + 'hello'); }); - it('pushes a new text part when trailing part is non-text', async () => { + it('prepends a new text part when leading part is non-text', async () => { const h = createHarness(); const tool = new EchoTool('X', () => ({ output: [{ type: 'image_url', imageUrl: { url: 'data:foo' } }], @@ -476,7 +529,7 @@ describe('AgentToolDedupeService', () => { const [final] = await runStep(h, 1, 3, [toolCall('final', 'X', {})]); const arr = final!.result.output as Array<{ type: string; text?: string }>; expect(arr.some((part) => part.type === 'image_url')).toBe(true); - expect(arr.at(-1)).toEqual({ type: 'text', text: REMINDER_TEXT_1 }); + expect(arr[0]).toEqual({ type: 'text', text: REMINDER_TEXT_1 }); }); it('preserves isError flag when injecting reminder', async () => { diff --git a/packages/agent-core/src/agent/turn/tool-dedup.ts b/packages/agent-core/src/agent/turn/tool-dedup.ts index f57679c8f7..7952c4f739 100644 --- a/packages/agent-core/src/agent/turn/tool-dedup.ts +++ b/packages/agent-core/src/agent/turn/tool-dedup.ts @@ -54,18 +54,27 @@ function makeKey(toolName: string, args: unknown): string { return `${toolName} ${canonicalTelemetryArgs(args)}`; } +/** + * Prepends the reminder to the tool result output. + * + * The reminder MUST be at the head, not the tail: oversized text results are + * later replaced by a head-only preview (see `budgetToolResultForModel` in + * `agent/turn/tool-result-budget.ts`, which keeps only the first 2000 chars). + * A tail-appended reminder is silently cut off there, so the model never sees + * it exactly in the large-output scenarios where repeat loops are most likely. + */ function appendReminder(result: ExecutableToolResult, reminderText: string): ExecutableToolResult { const output = result.output; let newOutput: string | ContentPart[]; if (typeof output === 'string') { - newOutput = output + reminderText; + newOutput = reminderText + output; } else { - const arr: ContentPart[] = [...output]; - const last = arr.at(-1); - if (last !== undefined && last.type === 'text') { - arr[arr.length - 1] = { type: 'text', text: last.text + reminderText }; + const arr = [...output]; + const first = arr[0]; + if (first !== undefined && first.type === 'text') { + arr[0] = { type: 'text', text: reminderText + first.text }; } else { - arr.push({ type: 'text', text: reminderText }); + arr.unshift({ type: 'text', text: reminderText }); } newOutput = arr; } diff --git a/packages/agent-core/test/agent/turn/tool-dedup.test.ts b/packages/agent-core/test/agent/turn/tool-dedup.test.ts index f2aa09f9aa..ee8a6b3eec 100644 --- a/packages/agent-core/test/agent/turn/tool-dedup.test.ts +++ b/packages/agent-core/test/agent/turn/tool-dedup.test.ts @@ -1,11 +1,16 @@ import { describe, expect, it } from 'vitest'; +import { mkdtemp } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + import type { ExecutableToolResult } from '../../../src/loop/types'; import type { TelemetryClient, TelemetryProperties, } from '../../../src/telemetry'; import { ToolCallDeduplicator, __testing } from '../../../src/agent/turn/tool-dedup'; +import { budgetToolResultForModel } from '../../../src/agent/turn/tool-result-budget'; const { REMINDER_TEXT_1, REMINDER_TEXT_3, makeReminderText2 } = __testing; @@ -235,8 +240,63 @@ describe('ToolCallDeduplicator', () => { }); }); + describe('reminder survives oversized-result truncation (regression)', () => { + // Oversized text results are later replaced by a head-only 2K preview + // (agent/turn/tool-result-budget.ts). Reminders used to be appended at the + // tail and were silently cut off there — the model looped 12 times with no + // reminder ever visible. They must now sit at the head. + it('keeps the reminder inside the first 2K chars of an oversized result', async () => { + const dedup = new ToolCallDeduplicator(); + const big = 'x'.repeat(60_000); + let last: ExecutableToolResult | undefined; + for (let i = 0; i < 3; i += 1) { + dedup.beginStep(); + last = await runOriginal(dedup, `c${String(i)}`, 'Read', { p: 1 }, okResult(big)); + dedup.endStep(); + } + const headPreview = (last!.output as string).slice(0, 2_000); + expect(headPreview).toContain(''); + expect(headPreview).toContain('what new information you expect'); + }); + + it('prepends the reminder ahead of the original output', async () => { + const dedup = new ToolCallDeduplicator(); + let last: ExecutableToolResult | undefined; + for (let i = 0; i < 3; i += 1) { + dedup.beginStep(); + last = await runOriginal(dedup, `c${String(i)}`, 'Read', { p: 1 }, okResult('R')); + dedup.endStep(); + } + expect((last!.output as string).startsWith('\n\n')).toBe(true); + expect((last!.output as string).endsWith('R')).toBe(true); + }); + + it('keeps the reminder visible through the real budgetToolResultForModel offload (composition)', async () => { + const dedup = new ToolCallDeduplicator(); + const big = 'x'.repeat(60_000); + let last: ExecutableToolResult | undefined; + for (let i = 0; i < 3; i += 1) { + dedup.beginStep(); + last = await runOriginal(dedup, `c${String(i)}`, 'Read', { p: 1 }, okResult(big)); + dedup.endStep(); + } + // Compose with the actual oversized-result offload — the exact chain that + // used to drop tail-appended reminders in production. + const homedir = await mkdtemp(join(tmpdir(), 'dedupe-budget-')); + const modelResult = await budgetToolResultForModel({ + homedir, + toolName: 'Read', + toolCallId: 'c2', + result: last!, + }); + expect(typeof modelResult.output).toBe('string'); + expect(modelResult.output as string).toContain(''); + expect((modelResult.output as string).indexOf('')).toBeLessThan(2_000); + }); + }); + describe('reminder injection into ContentPart[] outputs', () => { - it('appends reminder1 to a trailing text part at streak 3', async () => { + it('prepends reminder1 to the leading text part at streak 3', async () => { const dedup = new ToolCallDeduplicator(); const arrayResult: ExecutableToolResult = { output: [{ type: 'text', text: 'hello' }], @@ -253,10 +313,10 @@ describe('ToolCallDeduplicator', () => { const arr = final.output as Array<{ type: string; text: string }>; expect(arr).toHaveLength(1); expect(arr[0]!.type).toBe('text'); - expect(arr[0]!.text).toBe('hello' + REMINDER_TEXT_1); + expect(arr[0]!.text).toBe(REMINDER_TEXT_1 + 'hello'); }); - it('appends reminder2 to a trailing text part at streak 5', async () => { + it('prepends reminder2 to the leading text part at streak 5', async () => { const dedup = new ToolCallDeduplicator(); const arrayResult: ExecutableToolResult = { output: [{ type: 'text', text: 'hello' }], @@ -273,10 +333,10 @@ describe('ToolCallDeduplicator', () => { const arr = final.output as Array<{ type: string; text: string }>; expect(arr).toHaveLength(1); expect(arr[0]!.type).toBe('text'); - expect(arr[0]!.text).toBe('hello' + makeReminderText2(5)); + expect(arr[0]!.text).toBe(makeReminderText2(5) + 'hello'); }); - it('pushes a new text part when trailing part is non-text', async () => { + it('prepends a new text part when leading part is non-text', async () => { const dedup = new ToolCallDeduplicator(); const arrayResult: ExecutableToolResult = { output: [{ type: 'image_url', imageUrl: { url: 'data:foo' } }], @@ -292,9 +352,9 @@ describe('ToolCallDeduplicator', () => { dedup.endStep(); const arr = final.output as Array<{ type: string; text?: string }>; expect(arr).toHaveLength(2); - expect(arr[0]!.type).toBe('image_url'); - expect(arr[1]!.type).toBe('text'); - expect(arr[1]!.text).toBe(REMINDER_TEXT_1); + expect(arr[0]!.type).toBe('text'); + expect(arr[0]!.text).toBe(REMINDER_TEXT_1); + expect(arr[1]!.type).toBe('image_url'); }); it('preserves isError flag when injecting reminder', async () => {