mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-08-25 16:46:17 +00:00
refactor(agent-core-v2): rename tool dedupe domain
This commit is contained in:
parent
898fa70901
commit
b6c9413baa
8 changed files with 53 additions and 53 deletions
|
|
@ -84,7 +84,7 @@ package "Agent scope (per agent)" #FDF5E6 {
|
|||
rectangle "<b>toolRegistry</b>\n<size:9><i>Agent</i></size>\n IAgentToolRegistryService" as toolRegistry #FDEBD0
|
||||
rectangle "<b>toolExecutor</b>\n<size:9><i>Agent</i></size>\n IAgentToolExecutorService" as toolExecutor #FDEBD0
|
||||
rectangle "<b>toolState</b>\n<size:9><i>Agent</i></size>\n IAgentToolState" as toolState #FDEBD0
|
||||
rectangle "<b>toolDedup</b>\n<size:9><i>Agent</i></size>\n IAgentToolDedupeService" as toolDedup #FDEBD0
|
||||
rectangle "<b>toolDedupe</b>\n<size:9><i>Agent</i></size>\n IAgentToolDedupeService" as toolDedupe #FDEBD0
|
||||
rectangle "<b>permissionGate</b>\n<size:9><i>Agent</i></size>\n IAgentPermissionGate" as permissionGate #FDEBD0
|
||||
rectangle "<b>permissionMode</b>\n<size:9><i>Agent</i></size>\n IAgentPermissionModeService" as permissionMode #FDEBD0
|
||||
rectangle "<b>permissionPolicy</b>\n<size:9><i>Agent</i></size>\n IAgentPermissionPolicyService" as permissionPolicy #FDEBD0
|
||||
|
|
@ -215,9 +215,9 @@ toolExecutor --> toolRegistry #34495E
|
|||
toolExecutor --> wire #34495E
|
||||
toolExecutor --> telemetry #34495E
|
||||
toolState --> wire #34495E
|
||||
toolDedup --> telemetry #34495E
|
||||
toolDedup --> loop #34495E
|
||||
toolDedup --> toolExecutor #34495E
|
||||
toolDedupe --> telemetry #34495E
|
||||
toolDedupe --> loop #34495E
|
||||
toolDedupe --> toolExecutor #34495E
|
||||
permissionGate --> permissionMode #34495E
|
||||
permissionGate --> permissionRules #34495E
|
||||
permissionGate --> permissionPolicy #34495E
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
Before Width: | Height: | Size: 260 KiB After Width: | Height: | Size: 260 KiB |
|
|
@ -5,7 +5,7 @@
|
|||
* `onWillExecuteTool` / `onDidExecuteTool` hooks and the decision results
|
||||
* handlers may return. Owned by `tool` because they describe tool execution,
|
||||
* not the turn lifecycle or the loop: participants such as `permission`,
|
||||
* `toolDedup`, and `externalHooks` consume them without reaching upward into
|
||||
* `toolDedupe`, and `externalHooks` consume them without reaching upward into
|
||||
* `loop` / `turn`. Pure contract (types only); no scoped service.
|
||||
*/
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
/**
|
||||
* `toolDedup` domain barrel — re-exports the tool-call deduplication
|
||||
* `toolDedupe` domain barrel — re-exports the tool-call deduplication
|
||||
* contract (`toolDedupe`) and its scoped service (`toolDedupeService`). Importing
|
||||
* this barrel registers the `IAgentToolDedupeService` binding into the scope registry.
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
/**
|
||||
* `toolDedup` domain (L4) — per-turn tool-call deduplication.
|
||||
* `toolDedupe` domain (L4) — per-turn tool-call deduplication.
|
||||
*
|
||||
* A self-wiring plugin: it participates in `turn` step boundaries and
|
||||
* `IAgentToolExecutorService`'s will/did hooks to suppress same-step duplicates and inject
|
||||
|
|
@ -12,25 +12,25 @@ import type { ContentPart } from '#/app/llmProtocol/message';
|
|||
|
||||
import { createDecorator, type ServiceIdentifier } from '#/_base/di/instantiation';
|
||||
|
||||
export type ToolDedupOutput = string | ContentPart[];
|
||||
export type ToolDedupeOutput = string | ContentPart[];
|
||||
|
||||
export interface ToolDedupSuccessResult {
|
||||
readonly output: ToolDedupOutput;
|
||||
export interface ToolDedupeSuccessResult {
|
||||
readonly output: ToolDedupeOutput;
|
||||
readonly isError?: false | undefined;
|
||||
readonly stopTurn?: boolean | undefined;
|
||||
readonly message?: string | undefined;
|
||||
readonly truncated?: boolean | undefined;
|
||||
}
|
||||
|
||||
export interface ToolDedupErrorResult {
|
||||
readonly output: ToolDedupOutput;
|
||||
export interface ToolDedupeErrorResult {
|
||||
readonly output: ToolDedupeOutput;
|
||||
readonly isError: true;
|
||||
readonly stopTurn?: boolean | undefined;
|
||||
readonly message?: string | undefined;
|
||||
readonly truncated?: boolean | undefined;
|
||||
}
|
||||
|
||||
export type ToolDedupResult = ToolDedupSuccessResult | ToolDedupErrorResult;
|
||||
export type ToolDedupeResult = ToolDedupeSuccessResult | ToolDedupeErrorResult;
|
||||
|
||||
export interface IAgentToolDedupeService {
|
||||
readonly _serviceBrand: undefined;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
/**
|
||||
* `toolDedup` domain (L4) — `IAgentToolDedupeService` implementation.
|
||||
* `toolDedupe` domain (L4) — `IAgentToolDedupeService` implementation.
|
||||
*
|
||||
* Self-wiring plugin: its constructor registers `loop` beforeStep/afterStep
|
||||
* hooks and `toolExecutor` onWillExecuteTool/onDidExecuteTool hooks to drive
|
||||
|
|
@ -18,7 +18,7 @@ import { ITelemetryService } from '#/app/telemetry/telemetry';
|
|||
import { IAgentLoopService } from '#/agent/loop';
|
||||
import { IAgentToolExecutorService } from '#/agent/toolExecutor';
|
||||
import type { ContentPart } from '#/app/llmProtocol/message';
|
||||
import { IAgentToolDedupeService, type ToolDedupResult } from './toolDedupe';
|
||||
import { IAgentToolDedupeService, type ToolDedupeResult } from './toolDedupe';
|
||||
|
||||
const REMINDER_TEXT_1 =
|
||||
'\n\n<system-reminder>\n' +
|
||||
|
|
@ -77,12 +77,12 @@ function argsHash(args: unknown): string {
|
|||
}
|
||||
|
||||
interface CheckedToolCall {
|
||||
readonly syntheticResult: ToolDedupResult | null;
|
||||
readonly syntheticResult: ToolDedupeResult | null;
|
||||
}
|
||||
|
||||
type ToolCallDupType = 'same_step' | 'cross_step';
|
||||
|
||||
function appendReminder(result: ToolDedupResult, reminderText: string): ToolDedupResult {
|
||||
function appendReminder(result: ToolDedupeResult, reminderText: string): ToolDedupeResult {
|
||||
const output = result.output;
|
||||
let newOutput: string | ContentPart[];
|
||||
if (typeof output === 'string') {
|
||||
|
|
@ -102,16 +102,16 @@ function appendReminder(result: ToolDedupResult, reminderText: string): ToolDedu
|
|||
: { ...result, output: newOutput };
|
||||
}
|
||||
|
||||
function forceStopResult(result: ToolDedupResult, reminderText: string): ToolDedupResult {
|
||||
function forceStopResult(result: ToolDedupeResult, reminderText: string): ToolDedupeResult {
|
||||
const withReminder = appendReminder(result, reminderText);
|
||||
return { ...withReminder, stopTurn: true };
|
||||
}
|
||||
|
||||
const DEDUP_PLACEHOLDER_RESULT: ToolDedupResult = { output: '' };
|
||||
const DEDUPE_PLACEHOLDER_RESULT: ToolDedupeResult = { output: '' };
|
||||
|
||||
export class AgentToolDedupeService extends Disposable implements IAgentToolDedupeService {
|
||||
declare readonly _serviceBrand: undefined;
|
||||
private readonly stepDeferreds = new Map<string, Deferred<ToolDedupResult>>();
|
||||
private readonly stepDeferreds = new Map<string, Deferred<ToolDedupeResult>>();
|
||||
private stepCalls: string[] = [];
|
||||
private readonly originalCallIndex = new Map<string, number>();
|
||||
private readonly syntheticCallIds = new Set<string>();
|
||||
|
|
@ -127,15 +127,15 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu
|
|||
@IAgentToolExecutorService toolExecutor: IAgentToolExecutorService,
|
||||
) {
|
||||
super();
|
||||
loop.hooks.beforeStep.register('toolDedup', async (ctx, next) => {
|
||||
loop.hooks.beforeStep.register('toolDedupe', async (ctx, next) => {
|
||||
this.beginStep(ctx.turnId, ctx.step);
|
||||
await next();
|
||||
});
|
||||
loop.hooks.afterStep.register('toolDedup', async (_ctx, next) => {
|
||||
loop.hooks.afterStep.register('toolDedupe', async (_ctx, next) => {
|
||||
this.endStep();
|
||||
await next();
|
||||
});
|
||||
toolExecutor.hooks.onWillExecuteTool.register('toolDedup', async (ctx, next) => {
|
||||
toolExecutor.hooks.onWillExecuteTool.register('toolDedupe', async (ctx, next) => {
|
||||
const checked = this.checkToolCall(ctx.toolCall.id, ctx.toolCall.name, ctx.args);
|
||||
if (checked.syntheticResult !== null) {
|
||||
ctx.decision = { syntheticResult: checked.syntheticResult };
|
||||
|
|
@ -143,7 +143,7 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu
|
|||
}
|
||||
await next();
|
||||
});
|
||||
toolExecutor.hooks.onDidExecuteTool.register('toolDedup', async (ctx, next) => {
|
||||
toolExecutor.hooks.onDidExecuteTool.register('toolDedupe', async (ctx, next) => {
|
||||
ctx.result = await this.finalizeResult(
|
||||
ctx.toolCall.id,
|
||||
ctx.toolCall.name,
|
||||
|
|
@ -201,9 +201,9 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu
|
|||
if (existing !== undefined) {
|
||||
this.syntheticCallIds.add(toolCallId);
|
||||
this.recordDupType(toolCallId, toolName, args, 'same_step');
|
||||
return { syntheticResult: DEDUP_PLACEHOLDER_RESULT };
|
||||
return { syntheticResult: DEDUPE_PLACEHOLDER_RESULT };
|
||||
}
|
||||
this.stepDeferreds.set(key, makeDeferred<ToolDedupResult>());
|
||||
this.stepDeferreds.set(key, makeDeferred<ToolDedupeResult>());
|
||||
this.originalCallIndex.set(toolCallId, index);
|
||||
if (this.consecutiveKey === key && this.consecutiveCount > 0) {
|
||||
this.recordDupType(toolCallId, toolName, args, 'cross_step');
|
||||
|
|
@ -218,7 +218,7 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu
|
|||
args: unknown,
|
||||
dupType: ToolCallDupType,
|
||||
): void {
|
||||
this.telemetry.track('tool_call_dedup_detected', {
|
||||
this.telemetry.track('tool_call_dedupe_detected', {
|
||||
turn_id: this.activeTurnId ?? 0,
|
||||
step_no: this.activeStep,
|
||||
tool_call_id: toolCallId,
|
||||
|
|
@ -232,8 +232,8 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu
|
|||
toolCallId: string,
|
||||
toolName: string,
|
||||
args: unknown,
|
||||
result: ToolDedupResult,
|
||||
): Promise<ToolDedupResult> {
|
||||
result: ToolDedupeResult,
|
||||
): Promise<ToolDedupeResult> {
|
||||
const key = this.callKeyByCallId.get(toolCallId);
|
||||
if (key === undefined) return result;
|
||||
this.callKeyByCallId.delete(toolCallId);
|
||||
|
|
@ -303,5 +303,5 @@ registerScopedService(
|
|||
IAgentToolDedupeService,
|
||||
AgentToolDedupeService,
|
||||
InstantiationType.Eager,
|
||||
'toolDedup',
|
||||
'toolDedupe',
|
||||
);
|
||||
|
|
|
|||
|
|
@ -19,8 +19,8 @@ import type {
|
|||
import {
|
||||
AgentToolDedupeService,
|
||||
IAgentToolDedupeService,
|
||||
__testing as toolDedupTesting,
|
||||
type ToolDedupResult,
|
||||
__testing as toolDedupeTesting,
|
||||
type ToolDedupeResult,
|
||||
} from '#/agent/toolDedupe';
|
||||
import {
|
||||
AgentToolExecutorService,
|
||||
|
|
@ -36,7 +36,7 @@ import { registerLogServices } from '../log/stubs';
|
|||
import { recordingTelemetry, type TelemetryRecord } from '../telemetry/stubs';
|
||||
import { stubLoopWithHooks, stubTurnWithHooks } from '../turn/stubs';
|
||||
|
||||
const { REMINDER_TEXT_1, REMINDER_TEXT_3, makeReminderText2 } = toolDedupTesting;
|
||||
const { REMINDER_TEXT_1, REMINDER_TEXT_3, makeReminderText2 } = toolDedupeTesting;
|
||||
const ZERO_USAGE = emptyUsage();
|
||||
|
||||
let disposables: DisposableStore;
|
||||
|
|
@ -64,7 +64,7 @@ interface Harness {
|
|||
|
||||
/**
|
||||
* Builds a container wired the same way the agent is: real executor + registry,
|
||||
* the dedup plugin registered (and realized so its constructor installs the
|
||||
* the dedupe plugin registered (and realized so its constructor installs the
|
||||
* loop / tool-executor hooks), recording telemetry, and stub loop / turn with
|
||||
* real hook slots. `ix.get(IAgentToolDedupeService)` is what forces the eager
|
||||
* plugin to construct and register its hooks.
|
||||
|
|
@ -82,7 +82,7 @@ function createHarness(telemetry: ITelemetryService = recordingTelemetry(telemet
|
|||
reg.defineInstance(IAgentWireRecordService, stubWireRecord());
|
||||
reg.defineInstance(
|
||||
IAgentWireService,
|
||||
disposables.add(new WireService({ logScope: 'wire', logKey: 'tool-dedup' })),
|
||||
disposables.add(new WireService({ logScope: 'wire', logKey: 'tool-dedupe' })),
|
||||
);
|
||||
reg.define(IAgentToolDedupeService, AgentToolDedupeService);
|
||||
registerLogServices(reg);
|
||||
|
|
@ -95,11 +95,11 @@ function createHarness(telemetry: ITelemetryService = recordingTelemetry(telemet
|
|||
return { ix, loop, executor, registry };
|
||||
}
|
||||
|
||||
function okResult(text: string): ToolDedupResult {
|
||||
function okResult(text: string): ToolDedupeResult {
|
||||
return { output: text };
|
||||
}
|
||||
|
||||
function errResult(text: string): ToolDedupResult {
|
||||
function errResult(text: string): ToolDedupeResult {
|
||||
return { output: text, isError: true };
|
||||
}
|
||||
|
||||
|
|
@ -191,7 +191,7 @@ function dummyExecution(): ToolWillExecuteContext['execution'] {
|
|||
return { approvalRule: 'x', execute: async () => ({ output: '' }) };
|
||||
}
|
||||
|
||||
/** Minimal `onWillExecuteTool` context — the dedup handler reads only id/name/args. */
|
||||
/** Minimal `onWillExecuteTool` context — the dedupe handler reads only id/name/args. */
|
||||
function willCtx(
|
||||
id: string,
|
||||
name: string,
|
||||
|
|
@ -210,7 +210,7 @@ function willCtx(
|
|||
};
|
||||
}
|
||||
|
||||
/** Minimal `onDidExecuteTool` context — the dedup handler reads only id/name/args/result. */
|
||||
/** Minimal `onDidExecuteTool` context — the dedupe handler reads only id/name/args/result. */
|
||||
function didCtx(
|
||||
id: string,
|
||||
name: string,
|
||||
|
|
@ -231,7 +231,7 @@ function didCtx(
|
|||
}
|
||||
|
||||
describe('AgentToolDedupeService', () => {
|
||||
describe('same-step dedup', () => {
|
||||
describe('same-step dedupe', () => {
|
||||
it('returns a placeholder synchronously and resolves to the real result on finalize', async () => {
|
||||
const h = createHarness();
|
||||
await beforeStep(h, 1, 1);
|
||||
|
|
@ -311,7 +311,7 @@ describe('AgentToolDedupeService', () => {
|
|||
expect(tool.calls).toHaveLength(1);
|
||||
expect(results.map((result) => result.output)).toEqual(['same', 'same']);
|
||||
expect(telemetryEvents).toContainEqual({
|
||||
event: 'tool_call_dedup_detected',
|
||||
event: 'tool_call_dedupe_detected',
|
||||
properties: expect.objectContaining({
|
||||
turn_id: 3,
|
||||
step_no: 1,
|
||||
|
|
@ -434,7 +434,7 @@ describe('AgentToolDedupeService', () => {
|
|||
registerRead(h);
|
||||
// 8 occurrences of the same call within a single step, but no prior
|
||||
// streak — the trigger is about sustained behaviour across steps, not
|
||||
// intra-step spam. Same-step dedup already short-circuits execution.
|
||||
// intra-step spam. Same-step dedupe already short-circuits execution.
|
||||
const calls = Array.from({ length: 8 }, (_, i) =>
|
||||
toolCall(i === 0 ? 'orig' : `dup${String(i)}`, 'Read', { p: 1 }),
|
||||
);
|
||||
|
|
@ -485,8 +485,8 @@ describe('AgentToolDedupeService', () => {
|
|||
const [final] = await runStep(h, 1, 3, [toolCall('final', 'X', {})]);
|
||||
const arr = final!.result.output as Array<{ type: string; text?: string }>;
|
||||
// The executor prepends a non-text companion to media-only output before
|
||||
// the dedup hook runs, so the array is [companion, image_url, reminder];
|
||||
// the dedup-specific behavior is the trailing reminder text part it pushed
|
||||
// the dedupe hook runs, so the array is [companion, image_url, reminder];
|
||||
// the dedupe-specific behavior is the trailing reminder text part it pushed
|
||||
// because the trailing part was non-text.
|
||||
expect(arr.some((part) => part.type === 'image_url')).toBe(true);
|
||||
expect(arr.at(-1)).toEqual({ type: 'text', text: REMINDER_TEXT_1 });
|
||||
|
|
@ -531,7 +531,7 @@ describe('AgentToolDedupeService', () => {
|
|||
it('resolves the dup deferred even when the original call args are rewritten before finalize', async () => {
|
||||
// Models the loop contract: prepareToolExecution may return
|
||||
// {updatedArgs}, in which case finalizeToolResult sees the rewritten
|
||||
// args. The dedup key is registered at onWillExecuteTool time under the
|
||||
// args. The dedupe key is registered at onWillExecuteTool time under the
|
||||
// LLM-issued args (keyed by call id), so the deferred is resolved under
|
||||
// that same key regardless of the rewritten args seen at finalize time.
|
||||
const h = createHarness();
|
||||
|
|
@ -686,7 +686,7 @@ describe('AgentToolDedupeService', () => {
|
|||
);
|
||||
|
||||
expect(telemetryEvents).toContainEqual({
|
||||
event: 'tool_call_dedup_detected',
|
||||
event: 'tool_call_dedupe_detected',
|
||||
properties: {
|
||||
turn_id: 7,
|
||||
step_no: 1,
|
||||
|
|
@ -709,7 +709,7 @@ describe('AgentToolDedupeService', () => {
|
|||
await executeAll(h, [toolCall('c2', 'Read', { path: '/a' })], 7, signal);
|
||||
|
||||
expect(telemetryEvents).toContainEqual({
|
||||
event: 'tool_call_dedup_detected',
|
||||
event: 'tool_call_dedupe_detected',
|
||||
properties: {
|
||||
turn_id: 7,
|
||||
step_no: 2,
|
||||
|
|
@ -731,7 +731,7 @@ describe('AgentToolDedupeService', () => {
|
|||
const [result] = await runStep(h, 7, 3, [toolCall('a2', 'Read', { path: '/a' })]);
|
||||
|
||||
expect(result!.result.output as string).not.toContain('<system-reminder>');
|
||||
expect(telemetryEvents.filter((e) => e.event === 'tool_call_dedup_detected')).toHaveLength(0);
|
||||
expect(telemetryEvents.filter((e) => e.event === 'tool_call_dedupe_detected')).toHaveLength(0);
|
||||
expect(telemetryEvents.filter((e) => e.event === 'tool_call_repeat')).toHaveLength(0);
|
||||
});
|
||||
|
||||
|
|
@ -819,7 +819,7 @@ describe('AgentToolDedupeService', () => {
|
|||
|
||||
expect(firstInNewTurn!.result.output as string).not.toContain('<system-reminder>');
|
||||
expect(telemetryEvents.filter((e) => e.event === 'tool_call_repeat')).toHaveLength(0);
|
||||
expect(telemetryEvents.filter((e) => e.event === 'tool_call_dedup_detected')).toHaveLength(0);
|
||||
expect(telemetryEvents.filter((e) => e.event === 'tool_call_dedupe_detected')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('runs with a no-op telemetry service', async () => {
|
||||
|
|
@ -199,7 +199,7 @@ describe('Agent turn flow', () => {
|
|||
await ctx.untilTurnEnd();
|
||||
|
||||
expect(records).toContainEqual({
|
||||
event: 'tool_call_dedup_detected',
|
||||
event: 'tool_call_dedupe_detected',
|
||||
properties: {
|
||||
turn_id: 0,
|
||||
step_no: 1,
|
||||
|
|
@ -237,7 +237,7 @@ describe('Agent turn flow', () => {
|
|||
await ctx.untilTurnEnd();
|
||||
|
||||
expect(records).toContainEqual({
|
||||
event: 'tool_call_dedup_detected',
|
||||
event: 'tool_call_dedupe_detected',
|
||||
properties: {
|
||||
turn_id: 0,
|
||||
step_no: 2,
|
||||
|
|
@ -259,7 +259,7 @@ describe('Agent turn flow', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('fires PostToolUse for same-step dups with the original real output, not the dedup placeholder', async () => {
|
||||
it('fires PostToolUse for same-step dups with the original real output, not the dedupe placeholder', async () => {
|
||||
// Hook command asserts the dup's PostToolUse payload carries the real
|
||||
// stdout ('dup'), not the placeholder ('').
|
||||
const assertScript = [
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue