From 1f527bd54a33357a69d15d23624be7725ff4eb26 Mon Sep 17 00:00:00 2001 From: mengw15 <125719918+mengw15@users.noreply.github.com> Date: Fri, 31 Jul 2026 04:50:23 -0700 Subject: [PATCH 1/2] test(agent-service): cover WorkflowState logical-plan, traversal, validation and content methods --- .../src/agent/workflow-state.spec.ts | 222 +++++++++++++++++- 1 file changed, 221 insertions(+), 1 deletion(-) diff --git a/agent-service/src/agent/workflow-state.spec.ts b/agent-service/src/agent/workflow-state.spec.ts index bffc3769eb7..f823dfca954 100644 --- a/agent-service/src/agent/workflow-state.spec.ts +++ b/agent-service/src/agent/workflow-state.spec.ts @@ -19,7 +19,7 @@ import { describe, expect, test } from "bun:test"; import { WorkflowState } from "./workflow-state"; -import type { OperatorPredicate, OperatorLink } from "../types/workflow"; +import type { OperatorPredicate, OperatorLink, ValidationError, WorkflowContent } from "../types/workflow"; function makeOperator(id: string, overrides: Partial = {}): OperatorPredicate { return { @@ -174,3 +174,223 @@ describe("WorkflowState - getSubDAG", () => { expect(subDag.operators.map(o => o.operatorID)).toEqual(["op2"]); }); }); + +describe("WorkflowState - toLogicalPlan", () => { + test("produces operators, port-indexed links, and an empty reuse list", () => { + const state = new WorkflowState(); + state.addOperator(makeOperator("op1")); + state.addOperator(makeOperator("op2")); + state.addLink(makeLink("l1", "op1", "op2")); + + const plan = state.toLogicalPlan(); + + expect(plan.operators.map(o => o.operatorID)).toEqual(["op1", "op2"]); + // operatorProperties are spread onto each logical operator; type + ports carry through + expect(plan.operators[0].operatorType).toBe("TestOp"); + expect(plan.links).toEqual([ + { fromOpId: "op1", fromPortId: { id: 0, internal: false }, toOpId: "op2", toPortId: { id: 0, internal: false } }, + ]); + expect(plan.opsToReuseResult).toEqual([]); + }); + + test("resolves the link port indices from the operators' port lists", () => { + const state = new WorkflowState(); + state.addOperator( + makeOperator("src", { + outputPorts: [ + { portID: "output-0", displayName: "Output 0" }, + { portID: "output-1", displayName: "Output 1" }, + ], + }) + ); + state.addOperator( + makeOperator("dst", { + inputPorts: [ + { portID: "input-0", displayName: "Input 0" }, + { portID: "input-1", displayName: "Input 1" }, + ], + }) + ); + state.addLink({ + linkID: "l1", + source: { operatorID: "src", portID: "output-1" }, + target: { operatorID: "dst", portID: "input-1" }, + }); + + expect(state.toLogicalPlan().links[0]).toEqual({ + fromOpId: "src", + fromPortId: { id: 1, internal: false }, + toOpId: "dst", + toPortId: { id: 1, internal: false }, + }); + }); + + test("the targetOperatorId argument is currently ignored — the whole-graph plan is produced either way", () => { + // The parameter is accepted but unused in the current implementation; assert the + // observed behavior so a future change that starts honoring it is caught here. + const state = new WorkflowState(); + state.addOperator(makeOperator("op1")); + state.addOperator(makeOperator("op2")); + state.addLink(makeLink("l1", "op1", "op2")); + + expect(state.toLogicalPlan("op1")).toEqual(state.toLogicalPlan()); + }); +}); + +describe("WorkflowState - traversal", () => { + // chain: op1 -> op2 -> op3 + function seedChain(): WorkflowState { + const state = new WorkflowState(); + state.addOperator(makeOperator("op1")); + state.addOperator(makeOperator("op2")); + state.addOperator(makeOperator("op3")); + state.addLink(makeLink("l1", "op1", "op2")); + state.addLink(makeLink("l2", "op2", "op3")); + return state; + } + + test("getFrontierOperators returns the leaf operators at depth 1", () => { + // op3 is the only operator that is not the source of a link + expect(seedChain().getFrontierOperators(1)).toEqual(["op3"]); + }); + + test("getFrontierOperators expands one hop upstream per depth, topologically ordered", () => { + const state = seedChain(); + expect(state.getFrontierOperators(2)).toEqual(["op2", "op3"]); + expect(state.getFrontierOperators(3)).toEqual(["op1", "op2", "op3"]); + }); + + test("getFrontierOperators returns an empty list for an empty workflow", () => { + expect(new WorkflowState().getFrontierOperators(3)).toEqual([]); + }); + + test("getSubDAG collects the target and every upstream operator and link", () => { + const sub = seedChain().getSubDAG("op3"); + expect(sub.operators.map(o => o.operatorID).sort()).toEqual(["op1", "op2", "op3"]); + expect(sub.links.map(l => l.linkID).sort()).toEqual(["l1", "l2"]); + }); +}); + +describe("WorkflowState - validation state", () => { + const err = (message: string): ValidationError => ({ isValid: false, messages: { general: message } }); + + test("setValidationError stores and getValidationOutput aggregates; clearValidationError removes", () => { + const state = new WorkflowState(); + + state.setValidationError("op1", err("bad")); + expect(state.getValidationOutput().errors).toEqual({ op1: err("bad") }); + + state.clearValidationError("op1"); + expect(state.getValidationOutput().errors).toEqual({}); + }); + + test("setAllValidationErrors replaces the map and recomputes the empty-state flag", () => { + const state = new WorkflowState(); + state.addOperator(makeOperator("op1")); + + state.setAllValidationErrors({ op1: err("x") }); + + const out = state.getValidationOutput(); + expect(out.errors).toEqual({ op1: err("x") }); + expect(out.workflowEmpty).toBe(false); // one enabled operator present + }); + + test("workflowEmpty is true with no operators and when every operator is disabled", () => { + const state = new WorkflowState(); + + state.setAllValidationErrors({}); + expect(state.getValidationOutput().workflowEmpty).toBe(true); + + state.addOperator(makeOperator("op1", { isDisabled: true })); + state.setAllValidationErrors({}); + expect(state.getValidationOutput().workflowEmpty).toBe(true); + }); + + test("getValidationChangedStream emits the aggregated output on every mutation", () => { + const state = new WorkflowState(); + const emitted: Array> = []; + state.getValidationChangedStream().subscribe(v => emitted.push(v)); + + state.setValidationError("op1", err("x")); + state.clearValidationError("op1"); + + expect(emitted).toHaveLength(2); + expect(emitted[0].errors).toEqual({ op1: err("x") }); + expect(emitted[1].errors).toEqual({}); + }); +}); + +describe("WorkflowState - updateOperatorInputPorts", () => { + test("rebuilds the input ports to the requested count, flagging the extras dynamic", () => { + const state = new WorkflowState(); + state.addOperator(makeOperator("op1")); + + expect(state.updateOperatorInputPorts("op1", 3)).toBe(true); + + const ports = state.getOperator("op1")!.inputPorts; + expect(ports.map(p => p.portID)).toEqual(["input-0", "input-1", "input-2"]); + expect(ports[0].isDynamicPort).toBe(false); // port 0 is static + expect(ports[1].isDynamicPort).toBe(true); // extra ports are dynamic + }); + + test("reducing the count drops the extra ports (links are left untouched)", () => { + const state = new WorkflowState(); + state.addOperator(makeOperator("src")); + state.addOperator( + makeOperator("op1", { + inputPorts: [ + { portID: "input-0", displayName: "Input 0" }, + { portID: "input-1", displayName: "Input 1" }, + ], + }) + ); + state.addLink({ + linkID: "l1", + source: { operatorID: "src", portID: "output-0" }, + target: { operatorID: "op1", portID: "input-1" }, + }); + + state.updateOperatorInputPorts("op1", 1); + + expect(state.getOperator("op1")!.inputPorts.map(p => p.portID)).toEqual(["input-0"]); + // Observed behavior: the link to the now-removed input-1 is NOT pruned by this method. + expect(state.getAllLinks().map(l => l.linkID)).toEqual(["l1"]); + }); + + test("returns false for a missing operator", () => { + expect(new WorkflowState().updateOperatorInputPorts("missing", 2)).toBe(false); + }); +}); + +describe("WorkflowState - workflow content round-trip", () => { + test("setWorkflowContent replaces the state and getWorkflowContent reflects it", () => { + const state = new WorkflowState(); + state.addOperator(makeOperator("stale")); // must be cleared by setWorkflowContent + + const content: WorkflowContent = { + operators: [makeOperator("op1"), makeOperator("op2")], + operatorPositions: { op1: { x: 1, y: 2 } }, + links: [makeLink("l1", "op1", "op2")], + commentBoxes: [], + settings: { dataTransferBatchSize: 123 }, + }; + state.setWorkflowContent(content); + + const out = state.getWorkflowContent(); + expect(out.operators.map(o => o.operatorID)).toEqual(["op1", "op2"]); + expect(out.links.map(l => l.linkID)).toEqual(["l1"]); + expect(out.operatorPositions).toEqual({ op1: { x: 1, y: 2 } }); + expect(out.settings).toEqual({ dataTransferBatchSize: 123 }); + expect(state.getOperator("stale")).toBeUndefined(); + }); + + test("setWorkflowContent falls back to defaults when settings/commentBoxes are absent", () => { + const state = new WorkflowState(); + // The runtime guards against missing optional fields even though the type requires them. + state.setWorkflowContent({ operators: [], operatorPositions: {}, links: [] } as unknown as WorkflowContent); + + const out = state.getWorkflowContent(); + expect(out.settings).toEqual({ dataTransferBatchSize: 400 }); // DEFAULT_WORKFLOW_SETTINGS + expect(out.commentBoxes).toEqual([]); + }); +}); From 2e440e7a6f9902ffe8da543dfa37f29a68e5edca Mon Sep 17 00:00:00 2001 From: mengw15 <125719918+mengw15@users.noreply.github.com> Date: Fri, 31 Jul 2026 05:43:06 -0700 Subject: [PATCH 2/2] test(agent-service): drop asserts pinning toLogicalPlan arg and input-port link behavior --- .../src/agent/workflow-state.spec.ts | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/agent-service/src/agent/workflow-state.spec.ts b/agent-service/src/agent/workflow-state.spec.ts index f823dfca954..2fd27af6527 100644 --- a/agent-service/src/agent/workflow-state.spec.ts +++ b/agent-service/src/agent/workflow-state.spec.ts @@ -224,17 +224,6 @@ describe("WorkflowState - toLogicalPlan", () => { toPortId: { id: 1, internal: false }, }); }); - - test("the targetOperatorId argument is currently ignored — the whole-graph plan is produced either way", () => { - // The parameter is accepted but unused in the current implementation; assert the - // observed behavior so a future change that starts honoring it is caught here. - const state = new WorkflowState(); - state.addOperator(makeOperator("op1")); - state.addOperator(makeOperator("op2")); - state.addLink(makeLink("l1", "op1", "op2")); - - expect(state.toLogicalPlan("op1")).toEqual(state.toLogicalPlan()); - }); }); describe("WorkflowState - traversal", () => { @@ -333,9 +322,8 @@ describe("WorkflowState - updateOperatorInputPorts", () => { expect(ports[1].isDynamicPort).toBe(true); // extra ports are dynamic }); - test("reducing the count drops the extra ports (links are left untouched)", () => { + test("reducing the count drops the extra ports", () => { const state = new WorkflowState(); - state.addOperator(makeOperator("src")); state.addOperator( makeOperator("op1", { inputPorts: [ @@ -344,17 +332,10 @@ describe("WorkflowState - updateOperatorInputPorts", () => { ], }) ); - state.addLink({ - linkID: "l1", - source: { operatorID: "src", portID: "output-0" }, - target: { operatorID: "op1", portID: "input-1" }, - }); state.updateOperatorInputPorts("op1", 1); expect(state.getOperator("op1")!.inputPorts.map(p => p.portID)).toEqual(["input-0"]); - // Observed behavior: the link to the now-removed input-1 is NOT pruned by this method. - expect(state.getAllLinks().map(l => l.linkID)).toEqual(["l1"]); }); test("returns false for a missing operator", () => {