[FIX] Remediate Loop Counter Shared Across Failure Paths#3354
Merged
Trecek merged 10 commits intoMay 31, 2026
Merged
Conversation
…er guard after verify Adds two new semantic rules (loop-counter-cross-path-sharing, loop-guard-before-verify) and fixes the recipes so each failure path uses its own counter and the guard fires after the test step. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…er rule Replace the local `_find_guard_loop_body` (bounded BFS, max 4 hops, early-return on first path) with `_find_cycle_members` from `_rule_helpers`, taking the union of all frozensets containing the guard step. This correctly captures multi-path cycles and avoids incomplete `full_cycle` sets. Remove unused `_build_yaml_successor_map`, `_find_guard_loop_body`, `_MAX_LOOP_HOPS`, and `from collections import deque`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…path coverage Add two test methods to TestLoopBudgetSeparation that were specified in the original plan but absent from the implementation: - test_test_fix_loop_not_reachable_from_merge_gate_path: BFS backward from check_test_fix_loop to assert it is not reachable from merge-gate steps (diagnose_merge_gate, merge_gate_assess, check_merge_test_fix_loop). - test_merge_gate_assess_routes_through_merge_fix_guard: asserts merge_gate_assess routes to check_merge_test_fix_loop and not to check_test_fix_loop (skipped when merge_gate_assess is absent). Also add _get_routing_targets helper and deque import used by the new tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…te_test step Two fixes: 1. Rule engine: exclude external predecessors whose own predecessors are all inside the cycle. These are satellite retry guards (e.g., commit_guard_pre_remediation), not independent entry paths. 2. Recipe routing: add merge_gate_test step to remediation, implementation, and implementation-groups recipes. check_merge_test_fix_loop now routes to merge_gate_test (not the shared test step), preventing merge-gate failures from entering the pre-merge fix loop via check_test_fix_loop. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tions Reorder merge-gate fix loop from fix→guard→test to fix→test→guard so the last fix attempt gets verified before the budget is exhausted. Flow is now: merge_gate_assess → merge_gate_test → check_merge_test_fix_loop. Update test assertions to match new routing: merge_gate_assess routes to merge_gate_test (not check_merge_test_fix_loop directly), and the reachability test now checks direct routing properties instead of whole-graph backward BFS. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…p-counter rule Replace _find_cycle_members DFS union (which misses cycles through already-visited nodes) with forward+backward BFS intersection. This correctly finds all nodes in cycles with the guard step. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ositives Skip the cross-path-sharing check when the cycle exceeds 10 members. Fix-retry cycles are always small (3-5 steps); large cycles (15+) indicate general processing loops where counter sharing is intentional (e.g., merge-prs PR iteration loop). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ounter rule Remove unreachable `if not full_cycle: continue` guard (frozenset always contains step_name). Rename inner loop variable `m` to `sn` in generator comprehension to avoid shadowing the outer regex match variable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…st isolation Replace for-loop iteration over recipe names with @pytest.mark.parametrize in test_bundled_recipes_no_counter_sharing and test_bundled_recipes_no_guard_before_verify for per-recipe failure attribution under xdist parallel execution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Trecek
force-pushed
the
recipe-test-fix-loop-count-shared-across-distinct-failure-pa/3328
branch
from
ca55c54 to
2b0eedb
Compare
The rules_loop_counter.py addition brought recipe/rules/ to 35 Python files, exceeding the previous exemption cap of 34. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Trecek
deleted the
recipe-test-fix-loop-count-shared-across-distinct-failure-pa/3328
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix two gaps in the current
recipe-test-fix-loop-count-shared-across-distinct-failure-pa/3328implementation identified by/audit-impl:Rule engine fix —
rules_loop_counter.pyuses a local bounded BFS (_find_guard_loop_body, max 4 hops) to find the cycle body containing a guard step. The plan required using_find_cycle_membersfrom_rule_helpers.pyand taking the union of all returned frozensets that contain the guard, ensuring multi-path cycles are fully captured. The local function is structurally incorrect: it early-returns on the first BFS path found and uses an arbitrary hop bound, producing incompletefull_cyclesets for guards reachable through multiple distinct cycle paths.Missing tests — Two test methods specified by the original implementation plan are absent from
TestLoopBudgetSeparationintests/recipe/test_rules_integration_predicate.py:test_test_fix_loop_not_reachable_from_merge_gate_pathandtest_merge_gate_assess_routes_through_merge_fix_guard.Closes #3328
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/remediation-20260530-130256-691280/.autoskillit/temp/make-plan/remediation_loop_counter_shared_plan_2026-05-30_150300.md🤖 Generated with Claude Code via AutoSkillit
Token Usage Summary
* Step used a non-Anthropic provider; caching behavior may differ.
Token Efficiency
Model Usage Breakdown