Skip to content

test(agent-service): cover WorkflowState logical-plan, traversal, validation and content methods - #7159

Open
mengw15 wants to merge 2 commits into
apache:mainfrom
mengw15:chore/7158-workflow-state-test
Open

test(agent-service): cover WorkflowState logical-plan, traversal, validation and content methods#7159
mengw15 wants to merge 2 commits into
apache:mainfrom
mengw15:chore/7158-workflow-state-test

Conversation

@mengw15

@mengw15 mengw15 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Extends the existing WorkflowState spec to cover the remaining pure,
in-memory methods (agent-service/src/agent/workflow-state.ts). All are
deterministic; no external services are mocked. No production code was changed.

15 tests, grouped by area:

  • toLogicalPlan — the produced operators, port-indexed links (indices
    resolved from each operator's port list), and the empty reuse list.
  • TraversalgetFrontierOperators(depth) returning the leaf set and
    expanding one hop upstream per depth (topologically ordered), and getSubDAG
    collecting a target's upstream operators and links.
  • Validation statesetValidationError / clearValidationError /
    setAllValidationErrors / getValidationOutput, the workflowEmpty flag
    (no operators, or all disabled), and getValidationChangedStream emitting on
    each mutation.
  • updateOperatorInputPorts — rebuilding the input-port list to a requested
    count (extras flagged dynamic), reducing the count, and the missing-operator
    guard.
  • Content round-tripsetWorkflowContent replacing the state and
    getWorkflowContent reflecting it, plus the default-settings fallback.

Any related issues, documentation, discussions?

Closes #7158

How was this PR tested?

Extended unit tests, run locally in agent-service/ (all green; the failure path
was verified by breaking an assertion to confirm the suite goes red):

bun test src/agent/workflow-state.spec.ts   # 28 pass, 0 fail
bun run typecheck                           # tsc --noEmit, clean
prettier --check src/agent/workflow-state.spec.ts   # clean

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.8 [1M context])

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @bobbai00
    You can notify them by mentioning @bobbai00 in a comment.

@codecov-commenter

codecov-commenter commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.96%. Comparing base (45b7b20) to head (2e440e7).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7159      +/-   ##
============================================
+ Coverage     79.62%   79.96%   +0.33%     
  Complexity     3836     3836              
============================================
  Files          1160     1160              
  Lines         46188    46166      -22     
  Branches       5145     5145              
============================================
+ Hits          36777    36916     +139     
+ Misses         7778     7617     -161     
  Partials       1633     1633              
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from 1f527bd
agent-service 82.56% <ø> (+5.14%) ⬆️
amber 73.09% <ø> (ø) Carriedforward from 1f527bd
computing-unit-managing-service 20.49% <ø> (ø) Carriedforward from 1f527bd
config-service 65.97% <ø> (ø) Carriedforward from 1f527bd
file-service 66.80% <ø> (ø) Carriedforward from 1f527bd
frontend 83.16% <ø> (ø) Carriedforward from 1f527bd
notebook-migration-service 78.89% <ø> (ø) Carriedforward from 1f527bd
pyamber 97.36% <ø> (ø) Carriedforward from 1f527bd
workflow-compiling-service 26.31% <ø> (ø) Carriedforward from 1f527bd

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mengw15
mengw15 requested a review from aglinxinyuan July 31, 2026 12:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the agent-service unit test suite for WorkflowState’s pure in-memory behaviors (logical plan conversion, traversal helpers, validation state, port updates, and content round-tripping) without changing production code.

Changes:

  • Added toLogicalPlan assertions for operator/link shaping and port-index resolution.
  • Added traversal tests for getFrontierOperators(depth) and getSubDAG(target).
  • Added tests for validation state mutation/stream emission, updateOperatorInputPorts, and workflow content round-trip/defaulting.
Suppressed comments (1)

agent-service/src/agent/workflow-state.spec.ts:358

  • This test asserts that updateOperatorInputPorts does not prune links that target removed ports. The linked issue #7158 explicitly lists link-pruning as the behavior to cover, and keeping this assertion will make a future correctness fix (pruning invalid links) break the test suite.

If pruning links is the desired behavior, please either update updateOperatorInputPorts to remove links targeting ports that were dropped and then update this test to expect pruning, or avoid closing #7158 and track the behavior gap separately.

  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"]);
  });

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread agent-service/src/agent/workflow-state.spec.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add unit test coverage for the remaining WorkflowState methods (agent-service)

3 participants