From ab14029f224cc2a118629a5e55b774e887cecf49 Mon Sep 17 00:00:00 2001 From: Felipe Lalanne <1822826+pipex@users.noreply.github.com> Date: Thu, 10 Aug 2023 19:12:40 -0400 Subject: [PATCH] Convert Plan from a Linked List to a DAG Plan nodes can now be of three types, a regular action node (same as before), a fork node, creating branching on the plan, and an empty node, to indicate joining of previously created branches. This commit also updates all existing functions to deal with this new plan format, even though the planner still cannot generate these types of plans Change-type: minor --- lib/agent/runtime.ts | 35 ++++++++++++---- lib/planner/findPlan.ts | 21 ++++++---- lib/planner/index.ts | 30 +++++++++----- lib/planner/node.ts | 45 ++++++++++++++++++-- lib/testing/builder.spec.ts | 47 +++++++++++++++++++++ lib/testing/builder.ts | 83 +++++++++++++++++++++++++++++++++++-- lib/testing/mermaid.ts | 60 +++++++++++++++++---------- lib/testing/simplify.ts | 41 ++++++++++++++++-- lib/testing/types.ts | 8 +++- 9 files changed, 309 insertions(+), 61 deletions(-) diff --git a/lib/agent/runtime.ts b/lib/agent/runtime.ts index f570b82..cb31600 100644 --- a/lib/agent/runtime.ts +++ b/lib/agent/runtime.ts @@ -120,9 +120,14 @@ export class Runtime { } } - private async runPlan(node: Node | null) { + private async runPlan(node: Node | null): Promise { const { logger } = this.opts; - while (node != null) { + + if (node == null) { + return; + } + + if (Node.isAction(node)) { const { action } = node; if (this.stopped) { @@ -146,9 +151,15 @@ export class Runtime { } logger.info(`${action.description}: success`); - // Advance the list - node = node.next; + return await this.runPlan(node.next); + } + + if (Node.isFork(node)) { + // Run children in parallel + await Promise.all(node.next.map(this.runPlan)); } + + // Nothing to do if the node is empty } start() { @@ -158,12 +169,20 @@ export class Runtime { const { logger } = this.opts; - const toArray = (n: Node | null): string[] => { - if (n == null) { + const flatten = (node: Node | null): string[] => { + if (node == null) { return []; } - return [n.action.description, ...toArray(n.next)]; + if (Node.isAction(node)) { + return [node.action.description, ...flatten(node.next)]; + } + + if (Node.isFork(node)) { + node.next.flatMap((n) => flatten(n)); + } + + return []; }; this.promise = new Promise>(async (resolve) => { @@ -190,7 +209,7 @@ export class Runtime { logger.debug( 'plan found, will execute the following actions', - toArray(start), + flatten(start), ); // If we got here, we have found a suitable plan diff --git a/lib/planner/findPlan.ts b/lib/planner/findPlan.ts index 35020de..a93ecd3 100644 --- a/lib/planner/findPlan.ts +++ b/lib/planner/findPlan.ts @@ -28,13 +28,20 @@ interface PlanningState { callStack?: Array>; } -function planHasId(id: string, node: Node | null): boolean { - while (node != null) { - if (node.id === id) { - return true; - } - node = node.next; +function findLoop(id: string, node: Node | null): boolean { + if (node == null) { + return false; + } + + if (Node.isAction(node)) { + return node.id === id; } + + if (Node.isFork(node)) { + return node.next.some((n) => findLoop(id, n)); + } + + // If the node is empty, ignore it return false; } @@ -51,7 +58,7 @@ function tryAction( const id = node.id; // Detect loops in the plan - if (planHasId(id, initialPlan.start)) { + if (findLoop(id, initialPlan.start)) { return { success: false, stats: initialPlan.stats, error: LoopDetected }; } const state = action.effect(initialPlan.state); diff --git a/lib/planner/index.ts b/lib/planner/index.ts index 7560af0..36fe677 100644 --- a/lib/planner/index.ts +++ b/lib/planner/index.ts @@ -19,18 +19,26 @@ export interface Planner { findPlan(current: TState, target: Target): Plan; } -function reverse(head: Node | null): Node | null { - let curr = head; - let prev: Node | null = null; - - while (curr != null) { - const next = curr.next; - curr.next = prev; - prev = curr; - curr = next; +function reversePlan( + curr: Node | null, + prev: Node | null = null, +): Node | null { + if (curr == null) { + return prev; + } + if (Node.isFork(curr)) { + // When reversing a fork node, we are turning the node + // into an empty node. For this reason, we create the empty node + // first that we pass as the `prev` argument to the recursive call to + // reversePlan for each of the children + const empty = { next: prev }; + curr.next.map((n) => reversePlan(n, empty)); + return empty; } - return prev; + const next = curr.next; + curr.next = prev; + return reversePlan(next, curr); } function of({ @@ -74,7 +82,7 @@ function of({ }); res.stats = { ...res.stats, time: performance.now() - time }; if (res.success) { - res.start = reverse(res.start); + res.start = reversePlan(res.start); trace({ event: 'success', start: res.start }); } else { trace({ event: 'failed' }); diff --git a/lib/planner/node.ts b/lib/planner/node.ts index a372c67..616ff72 100644 --- a/lib/planner/node.ts +++ b/lib/planner/node.ts @@ -4,9 +4,9 @@ import { Action } from '../task'; import { Pointer } from '../pointer'; /** - * A node defines a specific step in a plan. + * An action node defines an executable step of a plan */ -export interface Node { +export interface ActionNode { /** * Unique id for the node. This is calculated from the * action metadata and the current runtime state expected @@ -20,13 +20,48 @@ export interface Node { readonly action: Action; /** - * The next step in the plan + * The next step in the plan. */ next: Node | null; } +/** + * A fork node defines a branching in the plan created + * by the existence of a parallel task. A fork node can + * have zero or more next nodes. + */ +export interface ForkNode { + next: Array>; +} + +/** + * An empty node is a node that doesn't specify a specific action but can be + * use to indicate an empty step at the start of the plan, or a joining of the branches + * created by the split node. + */ +export interface EmptyNode { + next: Node | null; +} + +export type Node = + | ActionNode + | ForkNode + | EmptyNode; + +function isActionNode(n: Node): n is ActionNode { + return (n as ActionNode).action !== undefined; +} + +function isForkNode(n: Node): n is ForkNode { + return ( + (n as any).action === undefined && + (n as any).next !== undefined && + Array.isArray((n as any).next) + ); +} + export const Node = { - of: (s: TState, a: Action): Node => { + of: (s: TState, a: Action): ActionNode => { // We don't use the full state to calculate the // id as there may be changes in the state that have nothing // to do with the action. We just use the part of the state @@ -52,4 +87,6 @@ export const Node = { next: null, }; }, + isAction: isActionNode, + isFork: isForkNode, }; diff --git a/lib/testing/builder.spec.ts b/lib/testing/builder.spec.ts index 9ede73e..bb7a3f1 100644 --- a/lib/testing/builder.spec.ts +++ b/lib/testing/builder.spec.ts @@ -11,4 +11,51 @@ describe('testing/builder', () => { 'c', ]); }); + + it('builds a plan with parallel branches', () => { + expect( + plan() + .action('a') + .fork() + .branch('b', 'c') + .branch('d') + .join() + .action('f') + .end(), + ).to.deep.equal(['a', [['b', 'c'], ['d']], 'f']); + + expect( + plan() + .fork() + .action('a') + // A fork within a fork + .fork() + .branch('c') + .branch('d') + .join() + .branch('b') + .join() + .action('f') + .end(), + ).to.deep.equal([[['a', [['c'], ['d']]], ['b']], 'f']); + }); + + it('collapses empty branches', () => { + // Add an empty fork to the plan + expect(plan().action('a').fork().join().action('f').end()).to.deep.equal([ + 'a', + 'f', + ]); + expect( + plan() + .action('a') + .fork() + .branch('b', 'c') + // This branch is empty + .branch() + .join() + .action('f') + .end(), + ).to.deep.equal(['a', [['b', 'c']], 'f']); + }); }); diff --git a/lib/testing/builder.ts b/lib/testing/builder.ts index 55277c9..4299454 100644 --- a/lib/testing/builder.ts +++ b/lib/testing/builder.ts @@ -1,10 +1,16 @@ import { SimplePlan } from './types'; -export interface Builder { +interface PlanBuilder { /** * Adds a next node to the plan */ - action(description: string): Builder; + action(description: string): PlanBuilder; + + /** + * Adds a sub-plan to the simple plan + * to represent a fork in the current plan + */ + fork(): ForkBuilder; /** * Builds the test plan @@ -12,10 +18,69 @@ export interface Builder { end(): SimplePlan; } +interface ForkBuilder

= PlanBuilder> { + /** + * Adds a branch to the fork + */ + branch(...actions: string[]): ForkBuilder

; + + /** + * Adds an action to the current branch + */ + action(description: string): ForkBuilder

; + + /** + * Creates a fork within a fork + */ + fork(): ForkBuilder>; + + /** + * Joins the forked branches and returns + * the original builder + */ + join(): P; +} + +function createFork

= PlanBuilder>( + parent: P, + p: SimplePlan = [], +): ForkBuilder

{ + const repr: SimplePlan = []; + let br: SimplePlan = []; + const f: ForkBuilder

= { + branch(...actions: string[]) { + if (br.length > 0) { + repr.push(br); + br = []; + } + br.push(...actions); + return f; + }, + action(description: string) { + br.push(description); + return f; + }, + fork() { + return createFork(f, br); + }, + join() { + if (br.length > 0) { + repr.push(br); + } + if (repr.length > 0) { + p.push(repr); + } + return parent; + }, + }; + + return f; +} + /** * Start building a plan */ -export function plan(): Builder { +export function plan(): PlanBuilder { const repr: SimplePlan = []; const builder = { @@ -25,6 +90,10 @@ export function plan(): Builder { return builder; }, + fork() { + return createFork(builder, repr); + }, + end() { return repr; }, @@ -32,3 +101,11 @@ export function plan(): Builder { return builder; } + +export function branch(...values: string[]) { + let b = plan(); + for (const a of values) { + b = b.action(a); + } + return b.end(); +} diff --git a/lib/testing/mermaid.ts b/lib/testing/mermaid.ts index 6ee0a0f..1fe8c39 100644 --- a/lib/testing/mermaid.ts +++ b/lib/testing/mermaid.ts @@ -11,13 +11,14 @@ type GraphState = { depth?: number; }; +/** + * Mermaid classses for nodes + */ +const ERROR_NODE = 'stroke:#f00'; +const SELECTED_NODE = 'stroke:#0f0'; + export type MermaidOpts = { meta: boolean; - /** - * Mermaid class for errors - */ - error: string; - selected: string; }; function htmlEncode(s: string) { @@ -42,17 +43,35 @@ function instructionId(s: any, i: Method | Action | Parallel): string { .digest('hex'); } +function expandPlan(node: Node | null, prev: string): string[] { + if (node == null) { + // If we reached the end of the plan, add a stop node + return [` ${prev} --> stop(( ))`, ` stop:::finish`]; + } + + if (Node.isAction(node)) { + return [ + ` ${prev} --> ${node.id}`, + ` ${node.id}:::selected`, + ...expandPlan(node.next, node.id), + ]; + } + + if (Node.isFork(node)) { + return node.next.flatMap((n) => expandPlan(n, prev)); + } + + // If the node is an empty node, ignore it + return []; +} + /** * Return a trace function that generates * a mermaid graph */ export function mermaid( title: string, - { - meta = false, - error = 'stroke:#f00', - selected = 'stroke:#0f0', - }: Partial = {}, + { meta = false }: Partial = {}, ) { // The graph description, for now we store it // in memory @@ -114,25 +133,20 @@ export function mermaid( case 'success': graph.push(` start:::selected`); - let n = e.start; - let p = 'start'; - while (n != null) { - graph.push(` ${p} --> ${n.id}`); - graph.push(` ${n.id}:::selected`); - p = n.id; - n = n.next; - } + const n = e.start; + + // Expand the plan + graph.push(...expandPlan(n, 'start')); - graph.push(` ${p} --> stop(( ))`); - graph.push(` stop:::finish`); - graph.push(` classDef error ${error}`); - graph.push(` classDef selected ${selected}`); + // Add the style data + graph.push(` classDef error ${ERROR_NODE}`); + graph.push(` classDef selected ${SELECTED_NODE}`); graph.push(` classDef finish stroke:#000,fill:#000`); return; case 'failed': graph.push(` start:::error`); - graph.push(` classDef error ${error}`); + graph.push(` classDef error ${ERROR_NODE}`); return; case 'error': diff --git a/lib/testing/simplify.ts b/lib/testing/simplify.ts index bd78b77..9d1af37 100644 --- a/lib/testing/simplify.ts +++ b/lib/testing/simplify.ts @@ -1,13 +1,40 @@ import { Node, Plan } from '../planner'; +import { assert } from '../assert'; import { SimplePlan } from './types'; -function toArray(n: Node | null): SimplePlan { +function expand(n: Node | null): [...SimplePlan, Node | null] { if (n == null) { - return []; + return [null]; } - return [n.action.description, ...toArray(n.next)]; + if (Node.isAction(n)) { + return [n.action.description, ...expand(n.next)]; + } + + if (Node.isFork(n)) { + // We expand each branch + const branches = n.next.map(expand); + const res: SimplePlan = []; + let next: Node | null = null; + for (const branch of branches) { + const last = branch.pop(); + + // Since the fork will always end with an empty node, + // the last element of the branch should always be a node + assert( + last !== undefined && typeof last !== 'string' && !Array.isArray(last), + ); + next = last; + res.push(branch as SimplePlan); + } + + return [res, ...expand(next)]; + } + + // If empty we want the fork to handle + // the next node + return [n.next]; } /** @@ -20,5 +47,11 @@ export function simplified(p: Plan): SimplePlan { throw new Error('Plan not found'); } - return toArray(p.start); + const plan = expand(p.start); + + // The last element of the simplified plan will always be null + const last = plan.pop(); + assert(last === null); + + return plan as SimplePlan; } diff --git a/lib/testing/types.ts b/lib/testing/types.ts index c5436a0..7686b9b 100644 --- a/lib/testing/types.ts +++ b/lib/testing/types.ts @@ -1,7 +1,13 @@ +/** + * Our very simple representation of a DAG + * for testing + */ +type DAG = Array>; + /** * A "simplified" plan. * * It is an object representation * of a plan that's easier to print and compare. */ -export type SimplePlan = string[]; +export type SimplePlan = DAG;