improvement(executor): subflows, hitl handling cleanup #4604
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Runtime behavior is tightened end-to-end: Preview renders nested subflows with React Flow Reviewed by Cursor Bugbot for commit 1e5a746. Configure here. |
Greptile SummaryThis PR is a substantial executor refactor (~5130 insertions / ~837 deletions across 54 files) that replaces the
Confidence Score: 4/5Safe to merge; core correctness concerns around HITL double-execution, empty parallel bypass, and nested subflow routing are all addressed. The PR is a large, well-structured refactor with clear intent. HITL resume stub replacement, sentinel batching, and edge key migration all appear correct. The four flagged items are performance/maintainability concerns (O(n) scans, serial callback await, duplicated registration logic) rather than blocking bugs. Deducting one point for the volume of change and the O(n) hot-path scan in getScopedBlockOutput which could become a real issue in large workflows. apps/sim/executor/execution/state.ts (O(n) getScopedBlockOutput scan), apps/sim/executor/variables/resolvers/parallel.ts (O(n) resolveParentParallelBranchIndex), apps/sim/executor/execution/block-executor.ts (serial onBlockStart await latency) Important Files Changed
Sequence DiagramsequenceDiagram
participant E as Engine
participant N as NodeOrchestrator
participant P as ParallelOrchestrator
participant EM as EdgeManager
participant HITL as HITLManager
E->>N: handleNodeCompletion(ctx, sentinelStartId)
N->>P: handleParallelSentinel(start)
P->>P: initializeParallelScope()
P->>P: prepareCurrentBatch()
P->>P: expandParallel() [clone DAG branches]
P->>P: remapClonedEdges() [wire edges after full idMap]
P->>P: registerClonedSubflows() [populate subflowParentMap]
P->>EM: clearDeactivatedEdgesForNodes()
E->>N: handleNodeCompletion(ctx, branchNodeId)
N->>N: handleParentSubflowCompletion() [if nested sentinel-end]
N->>P: handleParallelBranchCompletion(branchIndex)
P->>P: aggregateParallelResults()
alt All branches complete
P->>N: route PARALLEL_EXIT
N->>E: emit parallel complete
else More batches
P->>P: advanceToNextBatch()
P->>P: prepareForBatchContinuation()
P->>N: route PARALLEL_CONTINUE
end
Note over E,HITL: HITL Pause/Resume Flow
E->>N: handleNodeCompletion(ctx, hitlNodeId)
N->>E: output contains _pauseMetadata
E->>EM: serializePauseSnapshot(edgeManager)
EM->>EM: persist deactivatedEdges + nodesWithActivatedEdge
E->>HITL: pause workflow
Note over HITL: User responds
HITL->>HITL: updateResumeOutputInAggregationBuffers()
HITL->>HITL: replace _pauseMetadata stub in branchOutputs/currentIterationOutputs
HITL->>E: resume with mergedOutput
E->>EM: restoreDeactivatedEdges(snapshot)
E->>N: handleNodeCompletion(ctx, nextNodeId)
Reviews (8): Last reviewed commit: "fix snapshot for nested subflows" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
bugbot run |
|
@greptile |
|
@greptile |
|
bugbot run |
|
@greptile |
|
@greptile |
|
bugbot run |
|
bugbot run |
|
@greptile |
|
bugbot run |
|
bugbot run |
|
@greptile |
|
bugbot run |
There was a problem hiding this comment.
✅ 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.
|
bugbot run |
|
@greptile |
There was a problem hiding this comment.
✅ 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 1e5a746. Configure here.
1 participant
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
Testing
Tested manually
Checklist