diff --git a/.changeset/unify-delegation-strategy.md b/.changeset/unify-delegation-strategy.md new file mode 100644 index 00000000..8418cfa2 --- /dev/null +++ b/.changeset/unify-delegation-strategy.md @@ -0,0 +1,5 @@ +--- +"@perstack/runtime": patch +--- + +Unify delegation strategy into single DelegationExecutor class diff --git a/packages/runtime/src/helpers/checkpoint.test.ts b/packages/runtime/src/helpers/checkpoint.test.ts index 9f35958e..c5400fc5 100644 --- a/packages/runtime/src/helpers/checkpoint.test.ts +++ b/packages/runtime/src/helpers/checkpoint.test.ts @@ -1,7 +1,6 @@ import type { Checkpoint, RunSetting } from "@perstack/core" import { describe, expect, it, vi } from "vitest" import { - buildDelegateToState, buildDelegationReturnState, createInitialCheckpoint, createNextStepCheckpoint, @@ -218,86 +217,3 @@ describe("@perstack/runtime: buildDelegationReturnState", () => { warnSpy.mockRestore() }) }) - -describe("@perstack/runtime: buildDelegateToState", () => { - const baseSetting = { - jobId: "job-123", - runId: "run-123", - model: "claude-sonnet-4-20250514", - providerConfig: { providerName: "anthropic" as const, apiKey: "test-key" }, - expertKey: "parent-expert", - input: { text: "parent query" }, - experts: {}, - reasoningBudget: "low", - maxSteps: 100, - maxRetries: 3, - timeout: 30000, - startedAt: 1000, - updatedAt: 2000, - perstackApiBaseUrl: "https://api.perstack.dev", - env: {}, - } satisfies RunSetting - const resultCheckpoint: Checkpoint = { - id: "parent-checkpoint-id", - jobId: "job-123", - runId: "run-123", - expert: { key: "parent-expert", name: "parent", version: "1.0.0" }, - stepNumber: 3, - status: "stoppedByDelegate", - messages: [ - { id: "m1", type: "userMessage", contents: [{ id: "c1", type: "textPart", text: "hello" }] }, - ], - usage: { - inputTokens: 100, - outputTokens: 50, - reasoningTokens: 0, - totalTokens: 150, - cachedInputTokens: 0, - }, - delegateTo: [ - { - expert: { key: "child-expert", name: "child", version: "2.0.0" }, - toolCallId: "tool-call-456", - toolName: "delegateToChild", - query: "please do this", - }, - ], - } - const currentExpert = { key: "parent-expert", name: "parent", version: "1.0.0" } - - it("builds correct setting for delegate target", () => { - const result = buildDelegateToState(baseSetting, resultCheckpoint, currentExpert) - expect(result.setting.expertKey).toBe("child-expert") - expect(result.setting.input).toEqual({ text: "please do this" }) - }) - - it("builds checkpoint with delegate target expert and delegatedBy", () => { - const result = buildDelegateToState(baseSetting, resultCheckpoint, currentExpert) - expect(result.checkpoint.status).toBe("init") - expect(result.checkpoint.messages).toEqual([]) - expect(result.checkpoint.expert).toEqual({ - key: "child-expert", - name: "child", - version: "2.0.0", - }) - expect(result.checkpoint.delegatedBy).toEqual({ - expert: { key: "parent-expert", name: "parent", version: "1.0.0" }, - toolCallId: "tool-call-456", - toolName: "delegateToChild", - checkpointId: "parent-checkpoint-id", - runId: "run-123", - }) - }) - - it("preserves usage from result checkpoint", () => { - const result = buildDelegateToState(baseSetting, resultCheckpoint, currentExpert) - expect(result.checkpoint.usage).toEqual(resultCheckpoint.usage) - }) - - it("throws when delegateTo is missing", () => { - const checkpointWithoutDelegateTo = { ...resultCheckpoint, delegateTo: undefined } - expect(() => - buildDelegateToState(baseSetting, checkpointWithoutDelegateTo, currentExpert), - ).toThrow("delegateTo is required") - }) -}) diff --git a/packages/runtime/src/helpers/checkpoint.ts b/packages/runtime/src/helpers/checkpoint.ts index 81693ed1..1eb6b473 100644 --- a/packages/runtime/src/helpers/checkpoint.ts +++ b/packages/runtime/src/helpers/checkpoint.ts @@ -97,49 +97,3 @@ export function buildDelegationReturnState( }, } } - -export function buildDelegateToState( - currentSetting: RunSetting, - resultCheckpoint: Checkpoint, - currentExpert: Pick, -): DelegationStateResult { - const { delegateTo } = resultCheckpoint - if (!delegateTo || delegateTo.length === 0) { - throw new Error("delegateTo is required for buildDelegateToState") - } - const firstDelegation = delegateTo[0] - const { expert, toolCallId, toolName, query } = firstDelegation - return { - setting: { - ...currentSetting, - expertKey: expert.key, - input: { - text: query, - }, - }, - checkpoint: { - ...resultCheckpoint, - status: "init", - messages: [], - expert: { - key: expert.key, - name: expert.name, - version: expert.version, - }, - delegatedBy: { - expert: { - key: currentExpert.key, - name: currentExpert.name, - version: currentExpert.version, - }, - toolCallId, - toolName, - checkpointId: resultCheckpoint.id, - runId: currentSetting.runId, - }, - usage: resultCheckpoint.usage, - pendingToolCalls: undefined, - partialToolResults: undefined, - }, - } -} diff --git a/packages/runtime/src/orchestration/delegation-strategy.test.ts b/packages/runtime/src/orchestration/delegation-strategy.test.ts index a57ae5c1..28e8ee19 100644 --- a/packages/runtime/src/orchestration/delegation-strategy.test.ts +++ b/packages/runtime/src/orchestration/delegation-strategy.test.ts @@ -2,10 +2,8 @@ import type { Checkpoint, DelegationTarget, RunSetting, Usage } from "@perstack/ import { describe, expect, it, vi } from "vitest" import { type DelegationContext, + DelegationExecutor, extractDelegationContext, - ParallelDelegationStrategy, - SingleDelegationStrategy, - selectDelegationStrategy, } from "./delegation-strategy.js" const createMockUsage = (): Usage => ({ @@ -62,107 +60,54 @@ const createMockDelegation = (overrides?: Partial): Delegation }) describe("@perstack/runtime: delegation-strategy", () => { - describe("SingleDelegationStrategy", () => { - it("throws error when delegations.length !== 1", async () => { - const strategy = new SingleDelegationStrategy() + describe("DelegationExecutor", () => { + it("throws error when delegations is empty", async () => { + const executor = new DelegationExecutor() const setting = createMockSetting() const context = createMockContext() const parentExpert = { key: "parent", name: "Parent", version: "1.0" } const runFn = vi.fn() - await expect(strategy.execute([], setting, context, parentExpert, runFn)).rejects.toThrow( - "SingleDelegationStrategy requires exactly one delegation", + await expect(executor.execute([], setting, context, parentExpert, runFn)).rejects.toThrow( + "DelegationExecutor requires at least one delegation", ) - - await expect( - strategy.execute( - [createMockDelegation(), createMockDelegation()], - setting, - context, - parentExpert, - runFn, - ), - ).rejects.toThrow("SingleDelegationStrategy requires exactly one delegation") - }) - - it("builds delegation state for single delegation without executing runFn", async () => { - const strategy = new SingleDelegationStrategy() - const setting = createMockSetting() - const context = createMockContext() - const delegation = createMockDelegation() - const parentExpert = { key: "parent", name: "Parent", version: "1.0" } - const runFn = vi.fn() - - const result = await strategy.execute([delegation], setting, context, parentExpert, runFn) - - expect(runFn).not.toHaveBeenCalled() - expect(result.nextSetting).toBeDefined() - expect(result.nextCheckpoint).toBeDefined() - expect(result.nextSetting.expertKey).toBe("child-expert") - expect(result.nextSetting.input.text).toBe("child query") }) - it("uses delegations parameter directly, not checkpoint.delegateTo", async () => { - const strategy = new SingleDelegationStrategy() + it("executes single delegation and returns result", async () => { + const executor = new DelegationExecutor() const setting = createMockSetting() - const context = createMockContext() const delegation = createMockDelegation({ - expert: { key: "correct-expert", name: "Correct", version: "1.0" }, - query: "correct query", - }) - const parentExpert = { key: "parent", name: "Parent", version: "1.0" } - const runFn = vi.fn() - - const result = await strategy.execute([delegation], setting, context, parentExpert, runFn) - - // Should use the delegation parameter, not any other source - expect(result.nextSetting.expertKey).toBe("correct-expert") - expect(result.nextSetting.input.text).toBe("correct query") - }) - - it("sets delegatedBy with parent expert info", async () => { - const strategy = new SingleDelegationStrategy() - const setting = createMockSetting() - const context = createMockContext({ id: "parent-cp-id" }) - const delegation = createMockDelegation() - const parentExpert = { key: "parent-key", name: "Parent Name", version: "2.0" } - const runFn = vi.fn() - - const result = await strategy.execute([delegation], setting, context, parentExpert, runFn) - - expect(result.nextCheckpoint.delegatedBy).toEqual({ - expert: { - key: "parent-key", - name: "Parent Name", - version: "2.0", - }, toolCallId: "tc-1", - toolName: "delegateTo", - checkpointId: "parent-cp-id", - runId: "run-1", + expert: { key: "expert-a", name: "A", version: "1" }, }) - }) - }) - - describe("ParallelDelegationStrategy", () => { - it("throws error when delegations.length < 2", async () => { - const strategy = new ParallelDelegationStrategy() - const setting = createMockSetting() const context = createMockContext() const parentExpert = { key: "parent", name: "Parent", version: "1.0" } - const runFn = vi.fn() - await expect(strategy.execute([], setting, context, parentExpert, runFn)).rejects.toThrow( - "ParallelDelegationStrategy requires at least two delegations", - ) + const resultCheckpoint: Checkpoint = { + ...createMockCheckpoint(), + stepNumber: 3, + expert: { key: "expert-a", name: "expert-a", version: "1" }, + messages: [ + { + id: "msg-expert-a", + type: "expertMessage", + contents: [{ type: "textPart", id: "txt-1", text: "Result from expert-a" }], + }, + ], + } - await expect( - strategy.execute([createMockDelegation()], setting, context, parentExpert, runFn), - ).rejects.toThrow("ParallelDelegationStrategy requires at least two delegations") + const runFn = vi.fn().mockResolvedValueOnce(resultCheckpoint) + + const result = await executor.execute([delegation], setting, context, parentExpert, runFn) + + expect(runFn).toHaveBeenCalledTimes(1) + expect(result.nextSetting.input.interactiveToolCallResult?.toolCallId).toBe("tc-1") + expect(result.nextCheckpoint.stepNumber).toBe(3) + expect(result.nextCheckpoint.partialToolResults).toEqual([]) }) it("executes multiple delegations in parallel", async () => { - const strategy = new ParallelDelegationStrategy() + const executor = new DelegationExecutor() const setting = createMockSetting() const delegations = [ createMockDelegation({ @@ -195,7 +140,7 @@ describe("@perstack/runtime: delegation-strategy", () => { .mockResolvedValueOnce(createMockResultCheckpoint("expert-a", 3)) .mockResolvedValueOnce(createMockResultCheckpoint("expert-b", 5)) - const result = await strategy.execute(delegations, setting, context, parentExpert, runFn) + const result = await executor.execute(delegations, setting, context, parentExpert, runFn) expect(runFn).toHaveBeenCalledTimes(2) expect(result.nextSetting.input.interactiveToolCallResult?.toolCallId).toBe("tc-1") @@ -205,7 +150,7 @@ describe("@perstack/runtime: delegation-strategy", () => { }) it("preserves delegatedBy for nested delegations", async () => { - const strategy = new ParallelDelegationStrategy() + const executor = new DelegationExecutor() const setting = createMockSetting() const delegations = [ createMockDelegation({ @@ -243,14 +188,14 @@ describe("@perstack/runtime: delegation-strategy", () => { .mockResolvedValueOnce(createMockResultCheckpoint("expert-a")) .mockResolvedValueOnce(createMockResultCheckpoint("expert-b")) - const result = await strategy.execute(delegations, setting, context, parentExpert, runFn) + const result = await executor.execute(delegations, setting, context, parentExpert, runFn) // Should preserve the parent's delegatedBy reference expect(result.nextCheckpoint.delegatedBy).toEqual(parentDelegatedBy) }) it("restores parent messages after parallel delegation", async () => { - const strategy = new ParallelDelegationStrategy() + const executor = new DelegationExecutor() const setting = createMockSetting() const delegations = [ createMockDelegation({ @@ -289,14 +234,14 @@ describe("@perstack/runtime: delegation-strategy", () => { .mockResolvedValueOnce(createMockResultCheckpoint("expert-a")) .mockResolvedValueOnce(createMockResultCheckpoint("expert-b")) - const result = await strategy.execute(delegations, setting, context, parentExpert, runFn) + const result = await executor.execute(delegations, setting, context, parentExpert, runFn) // Should restore parent's conversation history, not empty or child messages expect(result.nextCheckpoint.messages).toEqual(parentMessages) }) it("aggregates usage from all delegations", async () => { - const strategy = new ParallelDelegationStrategy() + const executor = new DelegationExecutor() const setting = createMockSetting() const delegations = [ createMockDelegation({ @@ -352,7 +297,7 @@ describe("@perstack/runtime: delegation-strategy", () => { }), ) - const result = await strategy.execute(delegations, setting, context, parentExpert, runFn) + const result = await executor.execute(delegations, setting, context, parentExpert, runFn) // Original: 10+5+0+15+0, plus two delegations expect(result.nextCheckpoint.usage.inputTokens).toBe(310) // 10 + 100 + 200 @@ -361,7 +306,7 @@ describe("@perstack/runtime: delegation-strategy", () => { }) it("throws error if delegation result has no expertMessage", async () => { - const strategy = new ParallelDelegationStrategy() + const executor = new DelegationExecutor() const setting = createMockSetting() const delegations = [ createMockDelegation({ toolCallId: "tc-1" }), @@ -376,13 +321,13 @@ describe("@perstack/runtime: delegation-strategy", () => { }) await expect( - strategy.execute(delegations, setting, context, parentExpert, runFn), + executor.execute(delegations, setting, context, parentExpert, runFn), ).rejects.toThrow("Delegation error: delegation result message is incorrect") }) it("uses empty string and warns when delegation result has no text part", async () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) - const strategy = new ParallelDelegationStrategy() + const executor = new DelegationExecutor() const setting = createMockSetting() const delegations = [ createMockDelegation({ toolCallId: "tc-1" }), @@ -398,14 +343,14 @@ describe("@perstack/runtime: delegation-strategy", () => { ], }) - const result = await strategy.execute(delegations, setting, context, parentExpert, runFn) + const result = await executor.execute(delegations, setting, context, parentExpert, runFn) expect(result.nextSetting.input.interactiveToolCallResult?.text).toBe("") expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("has no text content")) warnSpy.mockRestore() }) it("passes parent options to child runs", async () => { - const strategy = new ParallelDelegationStrategy() + const executor = new DelegationExecutor() const setting = createMockSetting() const delegations = [ createMockDelegation({ @@ -443,7 +388,7 @@ describe("@perstack/runtime: delegation-strategy", () => { eventListener, } - await strategy.execute(delegations, setting, context, parentExpert, runFn, parentOptions) + await executor.execute(delegations, setting, context, parentExpert, runFn, parentOptions) // Verify runFn was called with merged options (parent options + returnOnDelegationComplete) expect(runFn).toHaveBeenCalledTimes(2) @@ -463,24 +408,6 @@ describe("@perstack/runtime: delegation-strategy", () => { }) }) - describe("selectDelegationStrategy()", () => { - it("returns SingleDelegationStrategy for count = 1", () => { - const strategy = selectDelegationStrategy(1) - expect(strategy).toBeInstanceOf(SingleDelegationStrategy) - }) - - it("returns ParallelDelegationStrategy for count > 1", () => { - expect(selectDelegationStrategy(2)).toBeInstanceOf(ParallelDelegationStrategy) - expect(selectDelegationStrategy(5)).toBeInstanceOf(ParallelDelegationStrategy) - expect(selectDelegationStrategy(100)).toBeInstanceOf(ParallelDelegationStrategy) - }) - - it("returns ParallelDelegationStrategy for count = 0 (edge case)", () => { - const strategy = selectDelegationStrategy(0) - expect(strategy).toBeInstanceOf(ParallelDelegationStrategy) - }) - }) - describe("extractDelegationContext()", () => { it("extracts only required fields from checkpoint", () => { const checkpoint = createMockCheckpoint({ diff --git a/packages/runtime/src/orchestration/delegation-strategy.ts b/packages/runtime/src/orchestration/delegation-strategy.ts index eae74531..1f8eb3c6 100644 --- a/packages/runtime/src/orchestration/delegation-strategy.ts +++ b/packages/runtime/src/orchestration/delegation-strategy.ts @@ -75,92 +75,10 @@ export type DelegationContext = { } /** - * Strategy interface for handling delegations. - * Implementations differ in how they execute delegations (single vs parallel). + * Executes delegations by running all child experts in parallel + * and aggregating results for the parent to resume. */ -export interface DelegationStrategy { - /** - * Execute delegations and return the next setting/checkpoint for the run loop. - * @param parentOptions - Options from the parent run to be inherited by child runs - */ - execute( - delegations: DelegationTarget[], - setting: RunSetting, - context: DelegationContext, - parentExpert: Pick, - runFn: (params: RunParamsInput, options?: DelegationRunOptions) => Promise, - parentOptions?: DelegationRunOptions, - ): Promise -} - -/** - * Strategy for single delegation - does not execute, just prepares next state. - * The actual execution happens in the next iteration of the run loop. - */ -export class SingleDelegationStrategy implements DelegationStrategy { - async execute( - delegations: DelegationTarget[], - setting: RunSetting, - context: DelegationContext, - parentExpert: Pick, - _runFn: (params: RunParamsInput, options?: DelegationRunOptions) => Promise, - _parentOptions?: DelegationRunOptions, - ): Promise { - if (delegations.length !== 1) { - throw new Error("SingleDelegationStrategy requires exactly one delegation") - } - - // Use the delegation parameter directly, not checkpoint.delegateTo - const delegation = delegations[0] - const { expert, toolCallId, toolName, query } = delegation - - // New runId for child expert - each delegation gets its own run - const childRunId = createId() - - const nextSetting: RunSetting = { - ...setting, - runId: childRunId, - expertKey: expert.key, - input: { text: query }, - } - - const nextCheckpoint: Checkpoint = { - id: context.id, - jobId: setting.jobId, - runId: childRunId, - status: "init", - stepNumber: context.stepNumber, - messages: [], // Child starts fresh - expert: { - key: expert.key, - name: expert.name, - version: expert.version, - }, - delegatedBy: { - expert: { - key: parentExpert.key, - name: parentExpert.name, - version: parentExpert.version, - }, - toolCallId, - toolName, - checkpointId: context.id, - runId: setting.runId, // Parent's runId for traceability - }, - usage: context.usage, - contextWindow: context.contextWindow, - pendingToolCalls: undefined, - partialToolResults: undefined, - } - - return { nextSetting, nextCheckpoint } - } -} - -/** - * Strategy for parallel delegation - executes all delegations in parallel. - */ -export class ParallelDelegationStrategy implements DelegationStrategy { +export class DelegationExecutor { async execute( delegations: DelegationTarget[], setting: RunSetting, @@ -169,8 +87,8 @@ export class ParallelDelegationStrategy implements DelegationStrategy { runFn: (params: RunParamsInput, options?: DelegationRunOptions) => Promise, parentOptions?: DelegationRunOptions, ): Promise { - if (delegations.length < 2) { - throw new Error("ParallelDelegationStrategy requires at least two delegations") + if (delegations.length < 1) { + throw new Error("DelegationExecutor requires at least one delegation") } const [firstDelegation, ...remainingDelegations] = delegations @@ -342,16 +260,6 @@ export class ParallelDelegationStrategy implements DelegationStrategy { } } -/** - * Factory to select the appropriate delegation strategy. - */ -export function selectDelegationStrategy(delegationCount: number): DelegationStrategy { - if (delegationCount === 1) { - return new SingleDelegationStrategy() - } - return new ParallelDelegationStrategy() -} - /** * Helper to build state when returning from a completed delegation to parent. */ diff --git a/packages/runtime/src/orchestration/index.ts b/packages/runtime/src/orchestration/index.ts index 662bc7e2..5d0e53c8 100644 --- a/packages/runtime/src/orchestration/index.ts +++ b/packages/runtime/src/orchestration/index.ts @@ -2,13 +2,10 @@ export { buildReturnFromDelegation, type DelegationContext, type DelegationExecutionResult, + DelegationExecutor, type DelegationResult, type DelegationRunOptions, - type DelegationStrategy, extractDelegationContext, - ParallelDelegationStrategy, - SingleDelegationStrategy, - selectDelegationStrategy, } from "./delegation-strategy.js" export { SingleRunExecutor, diff --git a/packages/runtime/src/run.ts b/packages/runtime/src/run.ts index 59c4366d..ff740a19 100755 --- a/packages/runtime/src/run.ts +++ b/packages/runtime/src/run.ts @@ -17,9 +17,9 @@ import { } from "./helpers/index.js" import { buildReturnFromDelegation, + DelegationExecutor, extractDelegationContext, SingleRunExecutor, - selectDelegationStrategy, } from "./orchestration/index.js" export type RunOptions = { @@ -134,12 +134,10 @@ export async function run(runInput: RunParamsInput, options?: RunOptions): Promi if (!delegateTo || delegateTo.length === 0) { throw new Error("No delegations found in checkpoint") } - const strategy = selectDelegationStrategy(delegateTo.length) + const executor = new DelegationExecutor() const context = extractDelegationContext(resultCheckpoint) - // All strategies now use the same interface - delegations parameter is used directly - // Pass parent options so child runs inherit callbacks for checkpoint persistence and events - const delegationResult = await strategy.execute( + const delegationResult = await executor.execute( delegateTo, setting, context,