No actionable comments were generated in the recent review. 🎉

_{Comment @coderabbitai help to get the list of available commands.}

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.06%. Comparing base (25a86df) to head (61d697b).

@@            Coverage Diff             @@
##             main     #833      +/-   ##
==========================================
+ Coverage   92.05%   92.06%   +0.01%     
==========================================
  Files          90       90              
  Lines       27307    27321      +14     
  Branches      324      324              
==========================================
+ Hits        25137    25154      +17     
+ Misses       2162     2159       -3     
  Partials        8        8              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@coderabbitai full review

Pushed 3235391 addressing all three findings: (1) the E699 test now also asserts the path is not [E602] + a second test pins the closure-lift→[E699] propagation; (2) a closure-body CodegenInvariantError now propagates to _compile_fn (new handler around _lift_pending_closures) for a single [E699] instead of [E699]+spurious[E602] — the local handler in _compile_lifted_closure is removed; (3) the quantifier-predicate guard is now if len(pred.params) != 1.

#!/bin/bash
git log --oneline -10
echo "---"
git show 3235391 --stat

#!/bin/bash
gh api repos/aallan/vera/commits/3235391 --jq '.files[].filename'
echo "---"
gh api repos/aallan/vera/commits/3235391 --jq '.files[] | select(.filename | test("closures.py|operators.py|functions.py|test_codegen_invariant_e699.py")) | .patch'

#!/bin/bash
gh api repos/aallan/vera/contents/vera/codegen/closures.py?ref=3235391 --jq '.content' | base64 -d > closures.py
gh api repos/aallan/vera/contents/vera/codegen/functions.py?ref=3235391 --jq '.content' | base64 -d > functions.py
gh api repos/aallan/vera/contents/vera/wasm/operators.py?ref=3235391 --jq '.content' | base64 -d > operators.py
echo "=== closures.py: _compile_lifted_closure full ==="
awk '/def _compile_lifted_closure/,/^    def [a-zA-Z_]+\(/{print NR": "$0}' closures.py | sed -n '1,140p'
echo "=== _lift_pending_closures ==="
grep -n "_lift_pending_closures" closures.py functions.py

#!/bin/bash
echo "=== _lift_pending_closures (closures.py) ==="
sed -n '1,70p' closures.py
echo "=== _compile_lifted_closure full body ==="
sed -n '177,420p' closures.py

#!/bin/bash
sed -n '70,177p' closures.py

#!/bin/bash
sed -n '380,460p' functions.py
echo "=== quantifier fix context ==="
grep -n "quantifier predicate" operators.py
sed -n '675,715p' operators.py

@aallan Verified all three fixes in 3235391 — they land correctly:

1. Test coverage: test_codegen_invariant_e699.py now explicitly asserts not any(d.error_code == "E602" ...) alongside E699, and the new test_closure_lifting_invariant_error_surfaces_as_e699 pins the closure-lift propagation via monkeypatch.setattr(ClosureLiftingMixin, "_lift_pending_closures", _boom). Both assertions are meaningful and correctly target the regression this PR fixes.

2. Single-signal propagation: Confirmed _lift_pending_closures (vera/codegen/closures.py) calls _compile_lifted_closure with no intervening try/except CodegenInvariantError — the removed local handler means the exception now propagates cleanly out to vera/codegen/functions.py::_compile_fn's new try/except around self._lift_pending_closures(ctx). decl is correctly in scope for the error node fallback and message. This structurally guarantees a single [E699] rather than [E699] + spurious [E602], since the only catch point left is at the function boundary.

3. Quantifier guard: if len(pred.params) != 1 in _translate_quantifier is the correct invariant — the old if not pred.params only rejected the empty case, silently allowing a hypothetical multi-param predicate to fall through to pred.params[0] without complaint. Now any predicate arity other than exactly one raises CodegenInvariantError before indexing.

No further issues found — the reachable-None [E615]/[E630] forwarding paths in the same functions (domain/body translation in the quantifier, etc.) remain untouched and correctly documented.

@coderabbitai full review

Pushed e715ab5 for the two review-round findings: (1) _lift_pending_closures now rolls back _next_closure_id (mirroring its any_failed path) when a closure-body CodegenInvariantError aborts the worklist, then re-raises to _compile_fn — no module-state desync; (2) the closure test now patches _compile_lifted_closure to drive the real worklist+rollback+handler path.

#!/bin/bash
git log --oneline -5
echo "---"
git show e715ab5 --stat
echo "=== patches ==="
gh api repos/aallan/vera/commits/e715ab5 --jq '.files[] | select(.filename | test("closures.py|test_codegen_invariant_e699.py")) | .patch'

#!/bin/bash
gh api repos/aallan/vera/commits/e715ab5 --jq '.files[].filename'
echo "---"
gh api repos/aallan/vera/commits/e715ab5 --jq '.files[] | select(.filename | test("closures.py|test_codegen_invariant_e699.py")) | .patch'

#!/bin/bash
gh api repos/aallan/vera/contents/vera/codegen/closures.py?ref=e715ab5 --jq '.content' | base64 -d > closures_new.py
sed -n '55,140p' closures_new.py

#!/bin/bash
sed -n '140,190p' closures_new.py

Verified. Both fixes in e715ab5 hold up:

1. Rollback correctness: prev_next_closure_id is snapshotted before the worklist starts, and the new except CodegenInvariantError block restores self._next_closure_id = prev_next_closure_id before re-raising — an exact mirror of the existing any_failed rollback at the bottom of the function. Since local buffers (new_closure_fns_wat, new_closure_table, new_source_map, new_sigs) are only committed on the all-success path (and are discarded with the stack frame on the exception path), there's nothing else that needs unwinding. The closure_id ↔ table_index correspondence for subsequent top-level functions is preserved correctly.

2. Test now exercises the real path: test_closure_body_invariant_error_surfaces_as_e699 patches _compile_lifted_closure (not _lift_pending_closures), so the actual worklist loop, the new rollback, and the try/except in _compile_fn are all genuinely driven. Good catch on documenting why the no E602 assertion was dropped here — the rationale (incidental [E602]/[E604] warnings from unrelated prelude paths even in the unpatched compile) is sound and correctly scoped; the single-signal property is instead pinned by the earlier test_codegen_invariant_error_surfaces_as_e699 test, which is the right division of labour.

No further issues — this closes out the #657 review thread cleanly.

@coderabbitai full review

Pushed 61d697b: the closure E699 test now injects the failure at a callee (translate_block, AST-identity-keyed to the closure body) so the real _compile_lifted_closure runs and must propagate — mutation-validated that a re-added local swallow turns the test RED.

#!/bin/bash
git log --oneline -5
echo "---"
git show 61d697b --stat
echo "=== patch ==="
gh api repos/aallan/vera/commits/61d697b --jq '.files[] | select(.filename | test("test_codegen_invariant_e699.py|closures.py")) | .patch'

#!/bin/bash
gh api repos/aallan/vera/commits/61d697b --jq '.files[].filename'
echo "---"
gh api repos/aallan/vera/commits/61d697b --jq '.files[] | select(.filename | test("test_codegen_invariant_e699.py|closures.py")) | .patch'

#!/bin/bash
git fetch origin pull/833/head:pr833 2>&1 | tail -5
git checkout pr833 2>&1 | tail -5
ast-grep run --pattern 'def translate_block($$$ARGS):' --lang python vera/wasm | head -30
grep -n "def translate_block" -r vera/wasm | head

#!/bin/bash
sed -n '540,570p' vera/wasm/context.py

#!/bin/bash
grep -n "class AnonFn" -A 15 vera/ast.py
echo "---"
sed -n '1,180p' tests/test_codegen_invariant_e699.py

#!/bin/bash
grep -n "except CodegenInvariantError" vera/codegen/closures.py vera/codegen/functions.py

This confirms the only two catch points are the rollback-and-reraise in _lift_pending_closures (Line 99) and the two _compile_fn boundary handlers — no local swallow remains in _compile_lifted_closure itself.

@aallan Verified 61d697b — this is a solid strengthening of the test.

• _find_anon_fn's generic dataclass/list/tuple walk correctly locates anon.body: Block, and the monkeypatch signature _boom(self, block, env) matches WasmContext.translate_block(self, block: ast.Block, env: WasmSlotEnv) (vera/wasm/context.py, Line 548) exactly.
• The block is closure_body check is an identity comparison, so it's unaffected by AnonFn/Block being frozen dataclasses with structural __eq__ — it fires only for the specific closure body node produced by this one parse_file/transform run, and delegates to original_translate_block for everything else, so the rest of the compile (prelude, other blocks) proceeds normally.
• Re-confirmed the only except CodegenInvariantError sites left in the codebase are the rollback-and-reraise in _lift_pending_closures (vera/codegen/closures.py, Line 99) and the two _compile_fn boundary handlers (vera/codegen/functions.py, Lines 334 and 423) — no local swallow remains in _compile_lifted_closure. This matches the mutation-testing claim: reintroducing a local catch there would intercept the exception before it ever reaches _compile_fn, turning this test RED as described.
• This test now genuinely exercises the full real path (_compile_lifted_closure → _lift_pending_closures rollback → _compile_fn handler) rather than a patched stand-in.

No further issues — this closes out the #657 review thread.

[resolve]