Skip to content

fix(parallel): remove broken node-counting completion + resolver claim cross-block - #4045

Merged
icecrasher321 merged 2 commits into
stagingfrom
waleedlatif1/parallel-error-handling
Apr 8, 2026
Merged

fix(parallel): remove broken node-counting completion + resolver claim cross-block#4045
icecrasher321 merged 2 commits into
stagingfrom
waleedlatif1/parallel-error-handling

Conversation

@waleedlatif1

@waleedlatif1waleedlatif1 commented Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Parallel blocks used totalExpectedNodes = branchCount * allNodesInParallel to detect completion, which is wrong when branches have conditional paths (error edges, conditions)
  • Success branches complete fewer nodes than error branches, so the count never matches and the counting-based aggregation misfires
  • Removed the counting mechanism entirely — sentinel-end (triggered by edge processing) is the correct and sole mechanism for detecting parallel completion
  • Wrong resolver claiming known properties to fix issue Parallel block fails #4043

Type of Change

  • Bug fix

Testing

All 670 executor tests pass. Tested with parallel blocks containing error edges. Added tests for cross-block case.

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

vercelBot commented Apr 8, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
docsSkippedSkippedApr 8, 2026 5:42pm

Request Review

@cursor

cursorBot commented Apr 8, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes parallel completion/aggregation behavior by removing node-count-based completion detection and relying solely on sentinel-end aggregation, which could affect workflows with complex branching if any code still expected early aggregation. Also extends variable resolution to read parallel outputs across blocks, touching execution state access patterns.

Overview
Fixes parallel execution completion by removing the per-node counting mechanism (dropping completedCount/totalExpectedNodes from ParallelScope/ExecutionContext) and making aggregation happen only when the parallel sentinel-end runs, avoiding mis-detection with conditional/error paths.

Updates node orchestration and parallel scope initialization to no longer compute/pass an expected terminal-node count, and adjusts ParallelResolver to support resolving a parallel’s stored results output via <parallel.result(s)> (including named parallel references) with nested path access; adds/updates tests to cover the cross-block named-reference cases.

Reviewed by Cursor Bugbot for commit c4ee839. Configure here.

@greptile-apps

greptile-appsBot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes the broken node-counting completion detection for parallel blocks (where completedCount/totalExpectedNodes misfired on conditional paths) and makes the sentinel-end the sole aggregation trigger. It also fixes cross-block variable resolution: named parallel references like <parallelName.result> now correctly route to resolveOutput instead of falling through to the iteration-context path that requires the caller to be inside the parallel.

Confidence Score: 5/5

Safe to merge — the counting-based completion removal is architecturally correct and all 670 executor tests pass.

Both fixes are well-scoped: removing the broken node-count aggregation path eliminates a class of misfires on conditional branches, and the OUTPUT_PROPERTIES early-return in the resolver correctly separates result-access from iteration-context resolution. No P0/P1 findings. New tests cover named-reference resolution from outside a parallel, cross-block paths, nested paths, and InvalidFieldError cases.

No files require special attention.

Vulnerabilities

No security concerns identified. Changes are confined to executor internals (parallel completion detection and variable resolution) with no auth, user-input handling, or data persistence surface.

Important Files Changed

FilenameOverview
apps/sim/executor/orchestrators/parallel.tsRemoves completedCount/totalExpectedNodes from ParallelScope initialization; handleParallelBranchCompletion now returns void and only stores output — aggregation exclusively driven by sentinel-end.
apps/sim/executor/orchestrators/node.tsDrops the allComplete guard around aggregateParallelResults in handleParallelNodeCompletion; initializeParallelScope calls no longer pass nodesInParallel.
apps/sim/executor/variables/resolvers/parallel.tsAdds OUTPUT_PROPERTIES set {'result','results'} checked before KNOWN_PROPERTIES; resolveOutput reads block output stored by aggregateParallelResults; availableFields in InvalidFieldError updated to include 'result'.
apps/sim/executor/variables/resolvers/parallel.test.tsAdds a full describe block for named parallel references covering result/results resolution from outside, nested paths, cross-block access, and InvalidFieldError for unknown properties.
apps/sim/executor/execution/state.tsRemoves completedCount and totalExpectedNodes from ParallelScope interface, consistent with the orchestrator changes.
apps/sim/executor/types.tsMinor type cleanup related to the parallel scope changes.
apps/sim/executor/orchestrators/parallel.test.tsUpdates initializeParallelScope call to drop the nodesInParallel argument, keeping test consistent with the new signature.
apps/sim/executor/utils/iteration-context.test.tsAdds cross-block test cases for the iteration context utilities.

Sequence Diagram

sequenceDiagram
participant NE as NodeExecutionOrchestrator
participant PO as ParallelOrchestrator
participant PR as ParallelResolver
participant State as BlockState
Note over NE,State: Branch node completes
NE->>PO: handleParallelBranchCompletion(ctx, parallelId, nodeId, output)
PO->>PO: extractBranchIndex(nodeId)
PO->>PO: scope.branchOutputs.set(branchIndex, output)
Note right of PO: Returns void — no counting logic
Note over NE,State: Sentinel-end fires (sole aggregation trigger)
NE->>NE: handleParallelSentinel(ctx, node, 'end', parallelId)
NE->>PO: aggregateParallelResults(ctx, parallelId)
PO->>State: setBlockOutput(parallelId, { results })
PO-->>NE: { allBranchesComplete: true, results, totalBranches }
Note over PR,State: Cross-block resolution (new fix)
PR->>PR: OUTPUT_PROPERTIES.has('result'|'results') = true
PR->>PR: resolveOutput(targetParallelId, pathParts, ctx)
PR->>State: getBlockOutput(targetParallelId)
State-->>PR: { results: [...] }
PR-->>PR: return output.results
Loading

Reviews (2): Last reviewed commit: "fix resolver claim" | Re-trigger Greptile

@icecrasher321icecrasher321 changed the title fix(parallel): remove broken node-counting completion in parallel blocksfix(parallel): remove broken node-counting completion + resolver claim cross-bloclApr 8, 2026
@icecrasher321icecrasher321 changed the title fix(parallel): remove broken node-counting completion + resolver claim cross-bloclfix(parallel): remove broken node-counting completion + resolver claim cross-blockApr 8, 2026
@icecrasher321

Copy link
Copy Markdown
Collaborator

bugbot run

@icecrasher321

Copy link
Copy Markdown
Collaborator

@greptile

@cursorcursorBot left a comment

Copy link
Copy Markdown

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 c4ee839. Configure here.

@icecrasher321
icecrasher321 merged commit 579d240 into stagingApr 8, 2026
12 checks passed
@icecrasher321
icecrasher321 deleted the waleedlatif1/parallel-error-handling branch April 8, 2026 18:05
Sign up for freeto 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.

2 participants

@waleedlatif1@icecrasher321