Skip to content

fix(graph): keep every parallel success when a sibling branch fails - #173

Merged
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/parallel-failure-keeps-sibling-successes
Oct 22, 2026
Merged

M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/parallel-failure-keeps-sibling-successes

Conversation

@M3gA-Mind

Copy link
Copy Markdown
Collaborator

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 only active[..failed_index] and re-pended active[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 on retry/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_parallel keeps folding after a failure. It records only the first (lowest-index) failure as the step's error and pushes every success to completed.
  • The failure branch in the run loop now does what the interrupt path already does. It partitions with partition_completed, routes the completed branches with their re-keyed goto targets, re-pends every branch that did not complete (failed, interrupted or never run), and persists with the completed set.
  • Sequential mode is unchanged: there completed is exactly the prefix before the failure, so the result is identical.

API Or Behavior Changes

  • No public API change. StepFailure and persist_failure_checkpoint keep their shape.
  • Behaviour:
    • A parallel failure checkpoint now includes the updates of every successful sibling, not only the lower-index ones.
    • Retry re-runs only the branches that did not complete.
    • When several branches fail, all of them are retried; the error reported is still the lowest-index one.

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>:

  • Before (fix stubbed out, main source): 0 passed, 2 failed, on the "kept in the snapshot" / "not re-run" assertions.
  • After: 3 passed, including the existing parallel_partial_progress_is_preserved_on_failure.

Only those tests were run locally, per our process; CI runs the full matrix.

  • N/A: cargo fmt --check. I ran rustfmt on the touched files; CI runs the check.
  • N/A: cargo clippy --all-targets -- -D warnings. Left to CI.
  • N/A: cargo clippy --all-targets --all-features -- -D warnings. Left to CI.
  • N/A: cargo build --all-targets. Left to CI.
  • N/A: cargo build --all-targets --all-features. Left to CI.
  • N/A: cargo test. Only the targeted tests above were run; CI runs the full suite.
  • N/A: cargo test --all-features. Left to CI.

Documentation

Updated the run_active_parallel doc comment to describe the new fold. No other docs were affected.

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.
@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 3 billable files and costs up to $0.75.

Or wait 57 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 995c04ab-5353-43a4-8ac4-e319a93819d9
📥 Commits

Reviewing files that changed from the base of the PR and between 99fd391 and 6f2effd.

📒 Files selected for processing: 3
  • crates/tinyflows/src/graph/compiled/compiled_tests/compiled_tests_part_06_tests.rs
  • crates/tinyflows/src/graph/compiled/executor/node_execution.rs
  • crates/tinyflows/src/graph/compiled/executor/run_loop.rs

Comment @coderabbitai help to get the list of available commands.

@tinysweeper

tinysweeper Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: 6f2effdf6c40f8447314c2429abfbc2b26e8fedc

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 3 of 3 files.
Pull request purpose (title): fix(graph): keep eve…

Diagnostics and supporting details

Full summary

Pull 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

Change surface Files Review signal Count
Production 2 Active findings 0
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

Parallel 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

  • Modified — Parallel fold keeps successes on both sides of a failure: A parallel step's failure checkpoint now includes the updates of every successful sibling, not just lower-index ones, so their reducer contributions are preserved and their side effects do not fire twice on retry/resume. (crates/tinyflows/src/graph/compiled/executor/node_execution.rs where)
  • Modified — Failure boundary re-pends only incomplete branches: On a parallel failure, routing is limited to completed branches while failed, interrupted, and never-run branches are re-pended for retry; the error reported remains the lowest-index failure. (crates/tinyflows/src/graph/compiled/executor/run_loop.rs where)
  • Modified — Updated run_active_parallel doc comment: Documents that every successful branch contributes updates whichever side of a failure or interrupt it sits on, and that the lowest-index failure names the step's error. (crates/tinyflows/src/graph/compiled/executor/node_execution.rs where)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 3 files; 0 findings. _The code index is behind this pull request (indexed at `104f6da9a367`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 3 files; 0 findings. _The code index is behind this pull request (indexed at `104f6da9a367`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Tests lane reported no defect, noting the new tests assert on call counts and concrete state values that would fail if the fold reverted to the early break.
  • Lane summary: The change makes the parallel fold keep every successful branch regardless of its position relative to a failure, and reschedules only the branches that did not complete, with routing re-keyed through partition_completed. Two new tests pin the behaviour (lower-index failure preserving a higher-index success, and failures on both sides of a success), plus the existing parallel_partial_progress test. The new tests assert on call counts and concrete state values that would fail if the fold reverted to the early break; no defect found. _The code index is behind this pull request (indexed at `104f6da9a367`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Commits lane reported no sensitive content in committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `104f6da9a367`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Description lane found the description matches the diff accurately and the regression tests plus doc-comment update cover the claimed behavior; no defects found.
  • Lane summary: The change makes the parallel fold continue past a failure so every successful sibling's update is kept, and the failure boundary now partitions completed branches for routing while re-pending only the ones that did not complete. The description matches the diff accurately, and the two new regression tests plus the doc-comment update cover the behaviour the body claims. No defects found. _The code index is behind this pull request (indexed at `104f6da9a367`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.000660
  • Tokens: 297737 input · 5375 output · 41472 cached · 315 embedding
  • Updated: 2026-10-22T21:14:54Z
Head State Pass summary
6f2effdf6c40 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-22T21:14:54Z)

tinysweeper 0.1.2

@tinysweeper tinysweeper Bot 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.

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0007

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 22, 2026
@M3gA-Mind
M3gA-Mind merged commit 3cdf97c into tinyhumansai:main Oct 22, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant