Repository navigation
fix(validate): a loop on a side path no longer bounds a cycle that bypasses it - #175
Conversation
…passes it The unbounded-cycle check accepted a cycle as bounded when any loop node lay anywhere on it. A loop hanging off a side path (b -> l -> c) left the direct b -> c -> b cycle with no pass count, and validation accepted it. A cycle is now bounded only when its back-edge touches a loop node, or no path from its head back to its tail avoids every loop node.
Tiny Sweeper reviewReviewed head: Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): fix(validate): a loop on a side path no longer bounds a cycle that bypasses it. This pull request tightens the unbounded-cycle validation in tinyflows. Previously a cycle counted as bounded if any `loop` node lay on a node that was both reachable from the cycle head and able to reach the cycle tail, which allowed a `loop` sitting on a side detour to falsely bound a direct cycle that never passes through it. The fix in `crates/tinyflows/src/validate/loops.rs#pub(super) fn validate_loops(graph: &WorkflowGraph, errors: &mut Vec<ValidationE` requires the loop to be on every way round the cycle, and a regression test `loop_on_side_path_does_not_bound_a_bypass_cycle` in `crates/tinyflows/src/loop_tests/loop_tests_part_01_tests.rs` covers the bypass scenario. The latest-push-delta evidence is unavailable, so no push-delta comparison is described. Review snapshot
Completeness: Complete 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. The unbounded-cycle check treats a cycle as bounded when any
loopnode lies on it. That check looks at every node that is both reachable from the cycle's head and able to reach its tail. Aloopon a side path between the two (for exampleb -> l -> calongside a directb -> c, with the back-edgec -> b) therefore counted as bounding the cycle. But the directb -> c -> bcycle never passes through the loop, so nothing counts its passes, and validation accepted the graph.The fix. A cycle is bounded only if one of these holds:
loopnode, orloopnode.In other words, the loop must sit on every way round. This is a small reachability walk in
validate/loops.rs, next to the existing check. The run-level bounds (recursion_limit/max_node_visits) still exempt the graph as before.Regression test.
loop_on_side_path_does_not_bound_a_bypass_cycle, in the loop tests:main: FAILED, andvalidate_allreturned[](the bypass cycle was accepted).validate::tests also pass (112 passed).API Or Behavior Changes
Validation now refuses, with
IllegalCycle, a cycle whose onlyloopnode sits on a side path. Graphs whose cycles all pass through aloopnode, or that set a run-level bound, are unaffected. No public API change.Tests
cargo fmt --checkcargo clippy --all-targets -- -D warnings: left to CIcargo clippy --all-targets --all-features -- -D warnings: left to CIcargo build --all-targets: left to CIcargo build --all-targets --all-features: left to CIcargo test -p tinyflows --lib loop_on_side_path_does_not_bound(before: failed; after: passed), pluscargo test -p tinyflows --lib validate::(112 passed)cargo test --all-features: left to CIDocumentation
Not needed. This tightens an existing validation rule, and its doc comment explains the rule.
Summary by CodeRabbit