Skip to content

fix(validate): a loop on a side path no longer bounds a cycle that bypasses it - #175

Merged
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/validate-loop-side-path-bypass
Oct 23, 2026
Merged

M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/validate-loop-side-path-bypass

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

What was wrong. The unbounded-cycle check treats a cycle as bounded when any loop node lies on it. That check looks at every node that is both reachable from the cycle's head and able to reach its tail. A loop on a side path between the two (for example b -> l -> c alongside a direct b -> c, with the back-edge c -> b) therefore counted as bounding the cycle. But the direct b -> c -> b cycle 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:

  • its back-edge starts or ends at a loop node, or
  • there is no path from the cycle's head back to its tail that avoids every loop node.

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:

  • Before, on unmodified main: FAILED, and validate_all returned [] (the bypass cycle was accepted).
  • After: passes. The rest of the validate:: tests also pass (112 passed).

API Or Behavior Changes

Validation now refuses, with IllegalCycle, a cycle whose only loop node sits on a side path. Graphs whose cycles all pass through a loop node, or that set a run-level bound, are unaffected. No public API change.

Tests

  • cargo fmt --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 -p tinyflows --lib loop_on_side_path_does_not_bound (before: failed; after: passed), plus cargo test -p tinyflows --lib validate:: (112 passed)
  • cargo test --all-features: left to CI

Documentation

Not needed. This tightens an existing validation rule, and its doc comment explains the rule.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed cycle validation so a cycle is no longer treated as bounded when it can be traversed without passing through a loop. Workflows with such unbounded cycles are now reported as illegal.

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

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ 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: 18794c6b-8030-49ce-ade4-a50f29ec385f
📥 Commits

Reviewing files that changed from the base of the PR and between 2fa539b and bb1e1b3.

📒 Files selected for processing: 2
  • crates/tinyflows/src/loop_tests/loop_tests_part_01_tests.rs
  • crates/tinyflows/src/validate/loops.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Cycle validation now checks whether a back-edge’s return paths pass through a loop node. A new test verifies that a bounded loop on a side path does not bound a separate cycle that bypasses it.

Changes: Loop-aware cycle validation

Layer / File(s) Summary
Back-edge validation and bypass test
crates/tinyflows/src/validate/loops.rs, crates/tinyflows/src/loop_tests/loop_tests_part_01_tests.rs
Validation accepts a back-edge when either endpoint is a loop head or every path from target to source passes through a loop node. The test checks that a direct cycle bypassing a loop is reported as an IllegalCycle.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: senamakel

Merge Risk: ⚪ Minimal · up to bb1e1

The change has no confirmed issue requiring a fix before merge. The back-edge contract remains unverified.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main validation fix: a loop on a side path no longer incorrectly bounds a bypassing cycle.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the looping trail,
A side path cannot hide a tail.
The bypass cycle gets its name,
IllegalCycle marks the claim.
Then hops away beneath the moon.

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

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 2 of 2 files.
Pull request purpose (title): fix(validate): a loo…

Diagnostics and supporting details

Full summary

Pull 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

Change surface Files Review signal Count
Production 1 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.

Features

  • Modified — Cycle-bounding rule requires the loop on every route around the cycle: A cycle is now considered bounded only when its back-edge starts or ends at a `loop` node, or when no path from the cycle head to the cycle tail avoids all loop nodes. A cycle whose only `loop` node sits on a bypassing side path is now reported as `IllegalCycle` instead of being accepted. Graphs whose cycles all pass through a `loop`, or which set a run-level bound, are unaffected. No public API change. (crates/tinyflows/src/validate/loops.rs pub(super) fn validate_loops(graph: &WorkflowGraph, errors: &mut Vec<ValidationE)

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
  • Positive: The critique lane (answered) reviewed both changed files and reported zero findings.
  • Lane summary: Reviewed 2 files; 0 findings. _The code index is behind this pull request (indexed at `7cd439cda606`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The security lane (answered) reviewed both changed files and reported zero findings.
  • Lane summary: Reviewed 2 files; 0 findings. _The code index is behind this pull request (indexed at `7cd439cda606`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The tests lane (answered) reported no reportable defect and noted the new regression test would fail under the prior boundedness check.
  • Lane summary: The change tightens unbounded-cycle detection so a `loop` node merely on a side path no longer counts as bounding a direct cycle, and adds `loop_on_side_path_does_not_bound_a_bypass_cycle` in the existing `_tests.rs` sibling, which asserts `IllegalCycle` is reported for the bypass graph — that test fails if the old `nodes_on_cycle(...).any(is_loop)` check returns. No reportable defect found. _The code index is behind this pull request (indexed at `7cd439cda606`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The commits lane (answered) found no sensitive content in the committed files across the changed paths.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `7cd439cda606`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The description matches the diff: it explains the old false-negative (a `loop` on a side path counted as bounding), the new rule (loop must be on every way round), the new helper's role, and the regression test. No code defects found. _The code index is behind this pull request (indexed at `7cd439cda606`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.030528
  • Tokens: 170211 input · 4229 output · 42728 cached · 197 embedding
  • Updated: 2026-10-22T23:40:10Z
Head State Pass summary
bb1e1b3d18b0 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-22T23:40:10Z)

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.0305

@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 1ab36e3 into tinyhumansai:main Oct 23, 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