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..b195809abe 100644 --- a/packages/agent-core-v2/src/agent/toolDedupe/toolDedupeService.ts +++ b/packages/agent-core-v2/src/agent/toolDedupe/toolDedupeService.ts @@ -80,18 +80,19 @@ interface CheckedToolCall { readonly syntheticResult: ToolDedupeResult | null; } -function appendReminder(result: ToolDedupeResult, reminderText: string): ToolDedupeResult { +// Prepend, not append: oversized results are later replaced by a head-only preview that would silently drop a tail reminder. +function prependReminder(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; } @@ -101,7 +102,7 @@ function appendReminder(result: ToolDedupeResult, reminderText: string): ToolDed } function forceStopResult(result: ToolDedupeResult, reminderText: string): ToolDedupeResult { - const withReminder = appendReminder(result, reminderText); + const withReminder = prependReminder(result, reminderText); return { ...withReminder, stopTurn: true }; } @@ -277,13 +278,13 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu finalResult = forceStopResult(result, REMINDER_TEXT_3); action = 'stop'; } else if (streak >= REPEAT_REMINDER_3_START) { - finalResult = appendReminder(result, REMINDER_TEXT_3); + finalResult = prependReminder(result, REMINDER_TEXT_3); action = 'r3'; } else if (streak >= REPEAT_REMINDER_2_START) { - finalResult = appendReminder(result, makeReminderText2(streak)); + finalResult = prependReminder(result, makeReminderText2(streak)); action = 'r2'; } else if (streak >= REPEAT_REMINDER_1_START) { - finalResult = appendReminder(result, REMINDER_TEXT_1); + finalResult = prependReminder(result, REMINDER_TEXT_1); action = 'r1'; } 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..91277ae337 100644 --- a/packages/agent-core-v2/test/agent/toolDedupe/toolDedupe.test.ts +++ b/packages/agent-core-v2/test/agent/toolDedupe/toolDedupe.test.ts @@ -13,11 +13,15 @@ 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 { IAgentToolResultTruncationService } from '#/agent/toolResultTruncation/toolResultTruncation'; +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'; import { IAgentToolRegistryService } from '#/agent/toolRegistry/toolRegistry'; import { AgentToolRegistryService } from '#/agent/toolRegistry/toolRegistryService'; +import { InMemoryStorageService } from '#/persistence/backends/memory/inMemoryStorageService'; +import { IFileSystemStorageService } from '#/persistence/interface/storage'; import { registerLogServices } from '../../_base/log/stubs'; import { recordingTelemetry, type TelemetryRecord } from '../../app/telemetry/stubs'; import { stubLoopWithHooks } from '../loop/stubs'; @@ -56,7 +60,7 @@ interface Harness { function createHarness( telemetry: ITelemetryService = recordingTelemetry(telemetryEvents), - options: { readonly executorEvents?: boolean } = {}, + options: { readonly executorEvents?: boolean; readonly truncation?: 'real' } = {}, ): Harness { const loop = stubLoopWithHooks(); const events = options.executorEvents === true ? stubToolExecutorEvents() : undefined; @@ -91,7 +95,12 @@ function createHarness( } else { reg.defineInstance(IAgentToolExecutorService, events.executor); } - registerToolResultTruncationServices(reg); + if (options.truncation === 'real') { + reg.defineInstance(IFileSystemStorageService, new InMemoryStorageService()); + reg.define(IAgentToolResultTruncationService, ToolResultTruncationService); + } else { + registerToolResultTruncationServices(reg); + } reg.define(IAgentToolDedupeService, AgentToolDedupeService); registerLogServices(reg); }, @@ -441,8 +450,54 @@ describe('AgentToolDedupeService', () => { }); }); + describe('reminder survives oversized-result truncation (regression)', () => { + 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(); + h.registry.register(new EchoTool('Read')); + 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; + } + expect((last!.output as string).startsWith('\n\n')).toBe(true); + }); + + it('keeps the reminder visible through the real ToolResultTruncationService (composition)', async () => { + const h = createHarness(undefined, { truncation: 'real' }); + 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 truncation = h.ix.get(IAgentToolResultTruncationService); + 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 +505,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,22 +516,28 @@ 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 () => { - const h = createHarness(); - const tool = new EchoTool('X', () => ({ + it('prepends a new text part when leading part is non-text', async () => { + const h = createHarness(undefined, { executorEvents: true }); + // The executor pipeline prepends its own text part to non-text outputs, + // so drive the hooks directly to exercise the dedupe's unshift branch. + const imageOnly = (): ExecutableToolResult => ({ output: [{ type: 'image_url', imageUrl: { url: 'data:foo' } }], - })); - h.registry.register(tool); - for (let i = 0; i < 2; i += 1) { - await runStep(h, 1, i + 1, [toolCall(`p${String(i)}`, 'X', {})]); + }); + let final: ToolDidExecuteContext | undefined; + for (let i = 0; i < 3; i += 1) { + await beforeStep(h, 1, i + 1); + const id = i === 2 ? 'final' : `p${String(i)}`; + await h.fireBefore(willCtx(id, 'X', {})); + final = didCtx(id, 'X', {}, imageOnly()); + await h.executor.hooks.onDidExecuteTool.run(final); + await afterStep(h, 1, i + 1); } - 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..4098ee9ef6 100644 --- a/packages/agent-core/src/agent/turn/tool-dedup.ts +++ b/packages/agent-core/src/agent/turn/tool-dedup.ts @@ -54,18 +54,19 @@ function makeKey(toolName: string, args: unknown): string { return `${toolName} ${canonicalTelemetryArgs(args)}`; } -function appendReminder(result: ExecutableToolResult, reminderText: string): ExecutableToolResult { +// Prepend, not append: oversized results are later replaced by a head-only preview that would silently drop a tail reminder. +function prependReminder(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; } @@ -78,7 +79,7 @@ function forceStopResult( result: ExecutableToolResult, reminderText: string, ): ExecutableToolResult { - const withReminder = appendReminder(result, reminderText); + const withReminder = prependReminder(result, reminderText); return { ...withReminder, stopTurn: true }; } @@ -100,7 +101,7 @@ const DEDUP_PLACEHOLDER_RESULT: ExecutableToolResult = { output: '' }; * - Same-step dedup: a duplicate `(toolName, args)` issued in the same LLM step * reuses the original call's result instead of executing the tool twice. * - Cross-step dedup: when the exact same call is repeated consecutively - * across steps, the result returned to the model is suffixed with a system + * across steps, the result returned to the model is prefixed with a system * reminder once the streak hits 3. The reminder escalates as the streak * grows: r1 (expectation-setting nudge) from streak 3, r2 (forced decision * menu) from streak 5, r3 (final hand-off instruction) from streak 8. From streak 12 @@ -193,7 +194,7 @@ export class ToolCallDeduplicator { /** * Called from `finalizeToolResult`, in provider order. For first-occurrence * calls, projects the consecutive streak ending at this call and, if the - * threshold is reached, appends the system reminder, then resolves the + * threshold is reached, prepends the system reminder, then resolves the * deferred so subsequent same-step dups can fetch the real result. For * synthetic duplicates, awaits the original's deferred and returns its * value, discarding the placeholder. @@ -237,13 +238,13 @@ export class ToolCallDeduplicator { finalResult = forceStopResult(result, REMINDER_TEXT_3); action = 'stop'; } else if (streak >= REPEAT_REMINDER_3_START) { - finalResult = appendReminder(result, REMINDER_TEXT_3); + finalResult = prependReminder(result, REMINDER_TEXT_3); action = 'r3'; } else if (streak >= REPEAT_REMINDER_2_START) { - finalResult = appendReminder(result, makeReminderText2(streak)); + finalResult = prependReminder(result, makeReminderText2(streak)); action = 'r2'; } else if (streak >= REPEAT_REMINDER_1_START) { - finalResult = appendReminder(result, REMINDER_TEXT_1); + finalResult = prependReminder(result, REMINDER_TEXT_1); action = 'r1'; } 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..ac4470d7b5 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,57 @@ describe('ToolCallDeduplicator', () => { }); }); + describe('reminder survives oversized-result truncation (regression)', () => { + 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(); + } + 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 +307,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 +327,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 +346,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 () => {