feat(evaluator): PR-E E5 loop cardinality execution

[briansrls]

Summary

• wire Behavior::Loop through the evaluator dispatch shell
• implement eager LoopBound::Cardinality execution with LoopNode.source accumulator binding in a fresh per-iteration frame
• keep LoopBound::Descent as a typed fail-closed residual
• add loop count diagnostics for non-integer and negative cardinality witnesses
• add focused evaluator tests for zero iteration, accumulator threading, count failures, descent residual, and stack restoration

Scope

• no Transform/Branch/Bind expansion beyond existing landed evaluator behavior
• no strategy/value carrier widening
• no substrate, runner, witness, or Bool bridge work

Validation

• cargo fmt --all --check
• RUSTC_WRAPPER= cargo test -p v3-compiler --lib evaluator::tests
• RUSTC_WRAPPER= cargo clippy -p v3-compiler --lib -- -D warnings
• RUSTC_WRAPPER= cargo test -p v3-compiler --lib
• pre-push: cargo fmt --all --check

[briansrls]

Manager pass: E5 scope looks correct. This stays inside loop cardinality execution, uses LoopNode.source as the per-iteration accumulator binding per the readiness audit, keeps Descent fail-closed, and adds focused diagnostics/tests without absorbing Bind, E6/E7, runner, substrate, Bool bridge, or strategy work.

I also checked the key frame discipline shape: per-iteration frame is pushed before binding the accumulator and popped on both success and diagnostic paths; zero iterations return the evaluated init without touching the body. Hold for CI/review, but no manager-blocking issue from this pass.

— sent from snappy-moth-795

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: de453a3c · Trigger: schedule
• Comparison: origin/main @ 8c745cae ... review/pr-1476-de453a3c @ de453a3c
• Thinking: 26s wall

---

Review complete. The diff is confined to src/v3/compiler/src/lib.rs (evaluator module). Rubric files were read from the pinned context paths.

Findings: None. The change aligns with fail-closed behavior (P3): typed EvalError variants for descent deferral, non-integer count, and negative count; UnsupportedBehavior for Loop is removed in favor of real loop handling. Descent is explicitly deferred with an error that carries cluster and measure (lib.rs comments and LoopBoundDescentResidual), matching tracked-deferral style (P5: named residual / dissolution direction). Implementation matches CODING.md: free functions, explicit (dag, state, strategy) dependencies, no new hidden panic paths in the added loop path—decode_loop_cardinality_count uses match and returns Result. Tests follow TESTING.md: minimal Dag construction, behavior-named tests, one scenario per test, hermetic setup.

Verdict: APPROVE — Scoped evaluator work; fail-closed catalog and frame push/pop balance match existing eval_branch patterns; tests cover zero iterations, threading, missing port, non-integer/negative count, descent residual, and stack cleanup on body failure.

Exploratory observations (optional): On 32-bit targets, i64 as usize for large positive counts could truncate iteration bounds; if CI ever builds for 32-bit, consider an explicit range check against usize::MAX. Not cited as a rubric violation without a project guarantee about targets.

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: d99cec17 · Trigger: schedule
• Thinking: 237s wall

---

✅ No blocking concerns; the code PR keeps LoopBound::Descent fail-closed, validates cardinality counts explicitly, and restores iteration frames after body diagnostics.

[briansrls]

Review metadata

• Provider / model: codex / codex-default
• Commit: ea8fa06d · Trigger: schedule
• Comparison: origin/main @ abc95a8a ... review/pr-1476-ea8fa06d @ ea8fa06d
• Thinking: 74s wall

---

Findings:

• src/v3/compiler/src/lib.rs:404 — n as usize is not fail-closed on 32-bit targets: a non-negative i64 larger than usize::MAX truncates, so a huge cardinality witness can become a smaller loop count instead of a typed diagnostic. That violates INVARIANTS.md P3 / the E5 readiness rule that count decoding must not wrap into an unsigned iteration count. Use usize::try_from(n) and return a typed error on overflow.

Verdict: REQUEST_CHANGES. The loop execution shape otherwise matches the E5 audit: cardinality is typed, descent is a named residual, and frames are restored on success/error. The count conversion needs to fail closed before this lands.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: e869890f · Trigger: schedule
• Comparison: origin/main @ abc95a8a ... review/pr-1476-e869890f @ e869890f
• Thinking: 31s wall

---

Findings

None. The diff adds implementation-only evaluator behavior: typed EvalError arms for E5 (lib.rs around the new enum variants and decode_loop_cardinality_count), cardinality execution with per-iteration frame push/pop and correct ordering of body vs pop_frame (see the match (body_result, pop_result) at the lines introduced in the diff), and defers LoopBound::Descent via LoopBoundDescentResidual with rustdoc that names the gap—consistent with P3 / DB-1 (fail-closed, no fabricated progress) and the readiness framing in the module comment. Tests stay hermetic, behavior-named, minimal-Dag, which matches TESTING.md for this layer.

Verdict

APPROVE — Small, scoped E5 cardinality execution with explicit typed failures and regression tests; nothing in the diff clearly violates the pinned INVARIANTS / modeling-discipline / CODING / TESTING bar.

Exploratory observations (optional)

eval_loop_cardinality_count_too_large_for_usize_fails_closed is #[cfg(target_pointer_width = "32")], so 64-bit CI never exercises LoopCardinalityTooLarge; that is a deliberate platform split, not a rubric breach—only worth a thought if you ever want a synthetic try_from failure on 64-bit without lying about i64 range.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: 325ff151 · Trigger: schedule
• Comparison: origin/main @ 5722bcd0 ... review/pr-1476-325ff151 @ 325ff151
• Thinking: 28s wall

---

Findings

None. The diff only touches src/v3/compiler/src/lib.rs: new EvalError variants are typed carriers (fail-closed, not string probing), LoopBound::Descent is explicitly rejected with LoopBoundDescentResidual instead of guessing behavior, cardinality uses decode_loop_cardinality_count with distinct errors for non-integer, negative, and usize overflow, and loop frames follow the same push / body / pop pattern as eval_branch (including the branch comment about body vs frame errors). Tests replace a single “unsupported loop” expectation with focused, minimal-Dag cases that match TESTING.md’s unit-first, behavior-named style.

Verdict

APPROVE — Scoped evaluator implementation with explicit residuals and regression tests; nothing in the diff clearly violates INVARIANTS.md, docs/modeling-discipline.md, CODING.md, or TESTING.md as pinned from context.

Exploratory observations (optional)

On 64-bit targets, LoopCardinalityTooLarge is effectively unused because positive i64 values always convert to usize; the 32-bit-only test reflects that. Astronomically large positive counts could still imply impractical work on 64-bit without a separate policy cap—that is outside what this diff asserts and is not framed as a rubric violation here.

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 325ff151 · Trigger: schedule
• Thinking: 206s wall

---

Non-blocking — Strengths

• src/v3/compiler/src/lib.rs Code-only evaluator slice: the new Loop behavior path stays within the existing evaluator implementation surface and does not introduce substrate shape changes or parallel authorities.

✅ No blocking concerns.