Repository navigation
fix(graph): keep every parallel success when a sibling branch fails - #173
Conversation
A parallel step stopped folding at the lowest-index failure, so a higher-index branch that had already succeeded was dropped from the failure checkpoint and run a second time on retry. Fold every branch, record the lowest-index failure as the step's error, and on failure route the completed branches and re-pend every branch that did not complete, using the same partition as the interrupt path.
|
Warning Review limit reached
This review includes 3 billable files and costs up to $0.75. Or wait 57 minutes for your next included review. View limit details
Comment |
Tiny Sweeper reviewReviewed head: Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): fix(graph): keep every parallel success when a sibling branch fails. This pull request fixes a parallel-execution bug where a failure at a lower index caused already-succeeded higher-index sibling branches to be dropped from the failure checkpoint, losing their reducer updates and re-running them (and their side effects) on retry. The fold in `run_active_parallel` now continues past a failure, keeping every success and recording only the lowest-index failure as the step error; the run loop's failure boundary now partitions completed branches, routes only those, and re-pends every branch that did not complete. The latest-push-delta section is unavailable. Review snapshot
Completeness: Complete What changedParallel result folding no longer breaks at the lowest-index failing branch: every successful branch in `run_active_parallel` contributes its updates regardless of its position relative to a failure or interrupt, and only the first (lowest-index) failure is recorded as the step error. The failure boundary in the run loop now uses `partition_completed` to split completed from incomplete branches, routes successors only for completed branches with their re-keyed goto targets, re-pends all branches that did not complete (failed, interrupted, or never run), and persists with the completed set. The `run_active_parallel` doc comment was updated to describe the new fold. No public API change: `StepFailure` and `persist_failure_checkpoint` keep their shape, and sequential mode behavior is unchanged. Features
TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Before mergeNone. Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
Summary
What was wrong. In a parallel step every branch runs before the results are folded. The fold stopped at the lowest-index branch that failed (
break). The failure boundary then routed onlyactive[..failed_index]and re-pendedactive[failed_index..]. So when a lower-index branch failed, a higher-index branch that had already succeeded was dropped from the failure checkpoint. Its update was lost and it ran a second time onretry/resume, firing any side effect twice.A related problem: an interrupted branch below the failure was routed as if it had completed.
The fix.
run_active_parallelkeeps folding after a failure. It records only the first (lowest-index) failure as the step's error and pushes every success tocompleted.partition_completed, routes the completed branches with their re-keyedgototargets, re-pends every branch that did not complete (failed, interrupted or never run), and persists with the completed set.completedis exactly the prefix before the failure, so the result is identical.API Or Behavior Changes
StepFailureandpersist_failure_checkpointkeep their shape.Tests
Regression tests (in
compiled_tests_part_06_tests.rs):parallel_lower_index_failure_preserves_higher_index_success: the lower-index branch fails once, and the higher-index success must not re-run on retry.parallel_failures_on_both_sides_of_a_success_retry_only_the_failures: two failures around one success. The success is kept, and both failures re-run.cargo test -p tinyflows --lib -- <tests>:mainsource): 0 passed, 2 failed, on the "kept in the snapshot" / "not re-run" assertions.parallel_partial_progress_is_preserved_on_failure.Only those tests were run locally, per our process; CI runs the full matrix.
cargo fmt --check. I ranrustfmton the touched files; CI runs the check.cargo clippy --all-targets -- -D warnings. Left to CI.cargo clippy --all-targets --all-features -- -D warnings. Left to CI.cargo build --all-targets. Left to CI.cargo build --all-targets --all-features. Left to CI.cargo test. Only the targeted tests above were run; CI runs the full suite.cargo test --all-features. Left to CI.Documentation
Updated the
run_active_paralleldoc comment to describe the new fold. No other docs were affected.