Skip to content

loop-counter rule — bilateral narrowing, missing counter resets, ERROR promotion#4196

Merged
Trecek merged 3 commits into
developfrom
loop-counter-rule-bilateral-narrowing-missing-counter-resets/4194
Jul 7, 2026
Merged

loop-counter rule — bilateral narrowing, missing counter resets, ERROR promotion#4196
Trecek merged 3 commits into
developfrom
loop-counter-rule-bilateral-narrowing-missing-counter-resets/4194

Conversation

@Trecek

@Trecek Trecek commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fix three defects in the loop-counter-not-reset-on-outer-cycle semantic rule, add missing counter-reset steps to the three bundled audit recipes, promote the rule to ERROR severity, and add comprehensive test coverage.

Rule defects (in src/autoskillit/recipe/rules/rules_loop_counter.py):

  1. The forward-reachability BFS is unbounded — it reaches the entire post-audit terminal section (CI watch, merge queue, stall recovery), producing 7 classes of false-positive findings. Fix: add bilateral cycle-membership check requiring the inner guard to also be backward-reachable to the outer guard.
  2. No exemption for group_iteration_count, a run-wide safety ceiling that must not be reset per-cycle. Fix: add a documented exempt-counter constant.
  3. Severity is WARNING instead of the required ERROR. Fix: promote after eliminating false positives and recipe gaps.

Recipe gaps (in all three bundled audit recipes):

  1. merge_test_fix_loop_count is never reset between audit-remediation cycles — starvation risk identical to the test_fix_loop_count bug fixed in Run 4 of #3987: POSIX-ERE tests under Python re + no per-cycle fix budget #4192. Fix: add reset_merge_test_fix_counter step.
  2. ref_push_count is never reset — same starvation class. Fix: add reset_ref_push_counter step.

Test gaps: No TestLoopCounterNotResetOnOuterCycle test class exists. test_both_recipes_no_error_semantic_findings omits implementation-groups.

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/impl-20260706-185104-926367/.autoskillit/temp/make-plan/loop_counter_bilateral_narrowing_plan_2026-07-06_192500.md

Closes #4194

🤖 Generated with Claude Code via AutoSkillit

Token Usage Summary

Step Model count uncached output cache_read peak_ctx turns cache_write time
plan* opus[1m] 1 12.4k 39.3k 2.5M 154.8k 52 135.6k 31m 46s
review_approach* opus[1m] 1 24 4.7k 239.7k 57.4k 11 41.1k 7m 23s
verify* fable 1 12.2k 29.5k 1.2M 117.7k 37 117.9k 13m 24s
implement* MiniMax-M3 1 116.3k 13.7k 5.3M 0 102 0 6m 55s
fix* opus[1m] 1 40 5.9k 685.2k 75.2k 28 55.9k 5m 12s
audit_impl* opus[1m] 1 35 14.7k 393.0k 77.4k 20 71.5k 8m 8s
prepare_pr* MiniMax-M3 1 50.8k 1.7k 230.1k 0 13 0 1m 12s
compose_pr* MiniMax-M3 1 43.5k 1.2k 213.1k 0 8 0 41s
review_pr* opus[1m] 3 296 66.4k 1.6M 83.4k 86 187.9k 18m 52s
diagnose_ci* MiniMax-M3 1 51.7k 1.7k 443.3k 0 14 0 56s
resolve_ci* opus[1m] 1 60 8.2k 2.1M 77.2k 70 58.2k 5m 33s
Total 287.4k 186.9k 14.9M 154.8k 668.2k 1h 40m

* Step used a non-Anthropic provider; caching behavior may differ.

Token Efficiency

Step LoC Changed cache_read/LoC cache_write/LoC output/LoC
plan 0
review_approach 0
verify 0
implement 377 14180.8 0.0 36.3
fix 4 171310.0 13980.0 1474.0
audit_impl 0
prepare_pr 0
compose_pr 0
review_pr 0
diagnose_ci 0
resolve_ci 9 228689.8 6468.1 909.0
Total 390 38251.4 1713.3 479.3

Model Usage Breakdown

Model steps uncached output cache_read cache_write time
opus[1m] 6 12.8k 139.2k 7.5M 550.3k 1h 16m
fable 1 12.2k 29.5k 1.2M 117.9k 13m 24s
MiniMax-M3 4 262.4k 18.2k 6.2M 0 9m 44s

Trecek and others added 2 commits July 6, 2026 19:48
…-counter-not-reset-on-outer-cycle

- Add _WRAPPER_LOOP_EXEMPT_COUNTERS frozenset to exempt group_iteration_count
  (run-wide safety ceiling) from the cross-cycle reset requirement
- Promote severity from WARNING to ERROR (matches the rule's role as a
  blocking gate against starvation)
- Restructure rule body: swap loop order to (outer, inner), hoist
  forward_reachable BFS out of inner loop, add cycle_candidates precomputed
  via ctx.predecessors for bilateral cycle-membership check
- forward_reachable.add(non_exit_target) compensates for bfs_reachable's
  start-node exclusion so the direct-target case is not silently skipped
- Bilateral check structurally excludes post-audit terminal guards (CI
  watch, stall recovery) that lack a path back to the audit-remediation
  outer guard

Add reset_merge_test_fix_counter and reset_ref_push_counter steps to
remediation.yaml, implementation.yaml, and implementation-groups.yaml so
each audit-remediation cycle gets fresh merge-gate fix and ref-push
budgets. implementation-groups.yaml uses its existing double-quoted
style.

Update tests/recipe/test_rules_loop_counter.py with new
TestLoopCounterNotResetOnOuterCycle class covering: missing reset fires,
reset present suppresses, post-audit terminal excluded by bilateral
check, group_iteration_count exempt counter excluded, and parametrized
bundled-recipe zero-findings assertion across all three recipes.

Update tests/recipe/test_rules_integration_predicate.py:
_load_recipes now also loads implementation-groups.yaml; method renamed
from test_both_recipes_no_error_semantic_findings to
test_all_recipes_no_error_semantic_findings and iteration extended to
include the third recipe.

Regenerate pre-compiled JSON (.json siblings) via task compile-recipes
which also regenerates contract cards via regen-contracts dependency.
Update embedded SHA-256 hashes in recipes/diagrams/*.md to match the
new YAML content.
…_reset_present_does_not_fire

Route test step on_success back to outer_guard instead of done, forming the
cycle required by the bilateral narrowing check. Without this, the bilateral
check correctly excluded the inner guard (no path back to outer), producing
0 findings instead of the expected 1.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@Trecek Trecek left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AutoSkillit review passed. No blocking issues found.

…ne width

The new counter-reset chain (audit_impl → reset_test_fix_counter →
reset_merge_test_fix_counter → reset_ref_push_counter → remediate) exceeded
the 120-character line width limit enforced by
test_show_cook_preview_line_width_bounded_with_implementation_recipe.
Wrap the chain onto two lines in all three recipe diagrams.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Trecek Trecek added this pull request to the merge queue Jul 7, 2026
Merged via the queue into develop with commit 863ac1b Jul 7, 2026
3 checks passed
@Trecek Trecek deleted the loop-counter-rule-bilateral-narrowing-missing-counter-resets/4194 branch July 7, 2026 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant