Skip to content

improvement(executor): subflows, hitl handling cleanup #4604

Open
icecrasher321 wants to merge 13 commits into
stagingfrom
improvement/subflow-orch-cleanup
Open

improvement(executor): subflows, hitl handling cleanup #4604
icecrasher321 wants to merge 13 commits into
stagingfrom
improvement/subflow-orch-cleanup

Conversation

@icecrasher321
Copy link
Copy Markdown
Collaborator

Summary

Refactors parallel batching onto explicit DAG control flow while hardening nested subflow execution, pause/resume, run-from-block, and output resolution edge cases.

Type of Change

  • Other: Code quality

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment May 26, 2026 7:33pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 14, 2026

PR Summary

High Risk
Touches core workflow execution (DAG construction, edge activation, nested loops/parallels, pause/resume, and variable resolution); regressions could stall runs or mis-route branches.

Overview
This PR refactors how loops and parallels are represented and executed: DAG nodes use unified subflowId / subflowType metadata (replacing separate loopId / parallelId fields), with shared sentinel construction in sentinels.ts.

DAG wiring gains explicit subflow control edges: start→end bypass routes for skip-at-start / early exit, parallel_continue back-edges (mirroring loop continue), and tighter handling when edges stay inside the same parallel or cross nested subflows. EdgeManager now keys deactivated edges with JSON (fixing prefix collisions), honors parallel_continue / subflow control handles, and can restore deactivated-edge state on resume.

Runtime behavior shifts parallel batching to sentinel-driven prepareCurrentBatch / prepareForBatchContinuation instead of ad-hoc dynamic queue expansion. Loops can be skippedAtStart (empty forEach, zero iterations, false while) and exit via loop_exit without running the body. Block output resolution is branch-aware, including stable __obranch-* aliases and stricter scoping for cloned outer branches.

Pause / resume / run-from-block: snapshots persist deactivated edges and activated join nodes; resumed edges mark targets as activated before readiness checks; pause completion is recorded before returning paused. Restored parallel batches re-register cloned nested subflows in the parent map. Large test coverage accompanies these paths.

Reviewed by Cursor Bugbot for commit 70d0777. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 14, 2026

Greptile Summary

This PR refactors the workflow executor's parallel batching and nested subflow handling: batch expansion is deferred from initializeParallelScope to a new prepareCurrentBatch call on sentinel-start, eliminating a race where in-flight branches could have their state reset mid-execution. It also unifies loopId/parallelId node metadata into a single subflowId/subflowType pair, adds handleParentSubflowCompletion to propagate nested subflow exit outputs to parent containers, and introduces updateResumeOutputInAggregationBuffers to patch HITL pause entries in aggregation buffers before snapshot serialization.

  • Parallel batch timing fix: prepareCurrentBatch (now called at sentinel-start) replaces the inline expandParallel call in initializeParallelScope and scheduleNextBatch; advanceToNextBatch only advances scope counters and the next sentinel-start cycle drives the actual DAG expansion.
  • Metadata unification: NodeMetadata.loopId/parallelId/isParallelSentinel collapsed into subflowId/subflowType; all orchestrators, resolvers, and the edge manager updated accordingly.
  • Nested subflow output propagation: handleParentSubflowCompletion fires on sentinel-end nodes and pushes { results } to the parent parallel's branch outputs or the parent loop's iteration outputs, replacing ad-hoc per-orchestrator logic.

Confidence Score: 3/5

The core batch-timing refactor and metadata unification look structurally sound, but the empty-parallel path loses its execution log because sentinel-end is never activated after emitEmptySubflowEvents was removed without a parallel-equivalent of the loop skippedAtStart sentinel-end bypass.

The redesign correctly fixes the concurrent batch-reset race by deferring expansion to sentinel-start. However, the empty-parallel case silently drops the block log: prepareCurrentBatch returns early, sentinel-start emits sentinelStart true with no selectedRoute, the PARALLEL_EXIT bypass edge is classified as a control edge and deactivated, and sentinel-end never fires. Empty loops handle this correctly with skippedAtStart plus selectedRoute LOOP_EXIT; the parallel path has no equivalent.

apps/sim/executor/orchestrators/parallel.ts (prepareCurrentBatch empty-scope early return and sentinel-start return value for empty parallels), apps/sim/executor/execution/state.ts (getScopedBlockOutput linear scan)

Important Files Changed

Filename Overview
apps/sim/executor/orchestrators/parallel.ts Major refactor: batch expansion moved from initializeParallelScope to new prepareCurrentBatch (called at sentinel-start time), advanceToNextBatch replaces scheduleNextBatch. Empty parallel case loses its block log because sentinel-end is never activated.
apps/sim/executor/orchestrators/node.ts Unifies loopId/parallelId metadata into subflowId/subflowType; adds handleParentSubflowCompletion to propagate nested subflow outputs to parent parallel/loop; adds isFinalSentinelOutput to correctly classify sentinel output finality.
apps/sim/executor/execution/engine.ts Adds handleNodeCompletion call for _pauseMetadata outputs and widens the stopAfterBlockId guard to include PARALLEL_CONTINUE.
apps/sim/executor/execution/state.ts Adds getScopedBlockOutput for outer-branch-aware lookups with full-scan O(n) fallback; early-returns undefined for outer-branch nodes not found in scope.
apps/sim/executor/execution/edge-manager.ts Switches edge key format to JSON arrays, adds restoreDeactivatedEdges for snapshot resume, and adds PARALLEL_CONTINUE edge activation logic.
apps/sim/executor/utils/parallel-expansion.ts Rewrites edge wiring and boundary detection to use resolveBranchChildBoundaryId; separates cloneDAGNode from edge remapping via remapClonedEdges.
apps/sim/executor/dag/construction/edges.ts Adds addSubflowStartExitBypass for bypass edges and edgeStaysWithinSameParallel to correctly route intra-parallel edges through sentinel boundaries.
apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts Adds updateResumeOutputInAggregationBuffers to patch _pauseMetadata entries in aggregation buffers with isPausedOutputForContext guard.
apps/sim/executor/utils/subflow-utils.ts Adds subflowContainsBlock and isSubflowNestedInside for recursive containment checks; adds extractInnermostOuterBranchIndex and buildOuterBranchScopedId helpers.
apps/sim/executor/execution/executor.ts Adds registerRestoredClonedSubflows to rebuild subflowParentMap for cloned subflows after snapshot restore; adds branchIndex to parentMap entries.

Sequence Diagram

sequenceDiagram
    participant E as ExecutionEngine
    participant SStart as ParallelSentinelStart
    participant PO as ParallelOrchestrator
    participant Branch as BranchNodes
    participant SEnd as ParallelSentinelEnd
    participant NO as NodeOrchestrator
    E->>SStart: execute
    SStart->>PO: initializeParallelScope
    SStart->>PO: prepareCurrentBatch
    PO->>PO: expandParallel, registerClonedSubflows, resetBatchExecutionState
    SStart-->>E: sentinelStart true
    E->>Branch: execute branches
    Branch-->>E: output per branch
    E->>NO: handleNodeCompletion
    NO->>PO: handleParallelBranchCompletion
    E->>SEnd: all branch edges satisfied
    SEnd->>PO: aggregateParallelResults
    alt more batches
        PO->>PO: advanceToNextBatch
        SEnd-->>E: PARALLEL_CONTINUE
        E->>SStart: re-execute
    else done
        PO->>PO: emitSubflowSuccessEvents
        SEnd-->>E: PARALLEL_EXIT
    end
Loading

Reviews (7): Last reviewed commit: "address comments" | Re-trigger Greptile

Comment thread apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts Outdated
Comment thread apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts Outdated
Comment thread apps/sim/executor/orchestrators/parallel.ts
@icecrasher321
Copy link
Copy Markdown
Collaborator Author

bugbot run

@icecrasher321
Copy link
Copy Markdown
Collaborator Author

@greptile

Comment thread apps/sim/executor/orchestrators/parallel.ts
Comment thread apps/sim/executor/orchestrators/node.ts
@icecrasher321
Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321
Copy link
Copy Markdown
Collaborator Author

bugbot run

Comment thread apps/sim/executor/execution/state.ts
@icecrasher321
Copy link
Copy Markdown
Collaborator Author

@greptile

Comment thread apps/sim/executor/orchestrators/parallel.ts
Comment thread apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts
Comment thread apps/sim/executor/execution/state.ts
@icecrasher321
Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321
Copy link
Copy Markdown
Collaborator Author

bugbot run

Comment thread apps/sim/executor/execution/state.ts
Comment thread apps/sim/executor/dag/construction/edges.ts
Comment thread apps/sim/executor/execution/executor.ts Outdated
Comment thread apps/sim/executor/orchestrators/parallel.ts
@icecrasher321
Copy link
Copy Markdown
Collaborator Author

bugbot run

@icecrasher321
Copy link
Copy Markdown
Collaborator Author

@greptile

Comment thread apps/sim/executor/execution/edge-manager.ts
@icecrasher321
Copy link
Copy Markdown
Collaborator Author

bugbot run

Comment thread apps/sim/executor/execution/engine.ts
@icecrasher321
Copy link
Copy Markdown
Collaborator Author

bugbot run

Comment thread apps/sim/executor/orchestrators/loop.ts
@icecrasher321
Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321
Copy link
Copy Markdown
Collaborator Author

bugbot run

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 70d0777. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant