v0.0.139 — closure codegen pair (#614 + #615)#625
Merged
Conversation
Two distinct root causes that share a failure path: closure dropped from the function table -> `unknown table 0` at WASM validation, or silent miscompute when later `let` pushes mask the bad index. #614: `f()[i]` where `f` returns `Array<T>` was silently dropped. `_infer_index_element_type_expr` only handled SlotRef and nested- IndexExpr collections; FnCall fell through to None, propagating up until either `_compile_fn` skipped the function with [E602] (top- level case, same shape as #604) or `_compile_lifted_closure` returned None (closure case -- silent: closure_id was registered at the call site but the lifted function never made it to the table). Fix: register each FnDecl's full Vera return-type expression in a new `_fn_ret_type_exprs` dict on `CodeGenerator`, propagate to per- function and closure WasmContexts, and extend `_infer_index_element_type_expr` to look up the called fn's return type and extract the `Array<T>` element when applicable. #615: closure capture order miscompile, two failure shapes from one root cause: 1. Non-contiguous outer slot. Body refs `@Int.k` while skipping `@Int.j` (j<k) -> lift-side env had no entry for j -> resolve returned None -> closure dropped -> WASM validation trap. 2. Ascending walker-order silent miscompute. Even contiguous captures: walker visited lower outer_idx first (e.g. body `@Int.1 - @Int.2`) -> ascending push order -> wrong stack layout under `WasmSlotEnv.resolve` -> body's slot refs resolved to WRONG captured locals. No trap, just wrong output. Fix: in `_collect_free_vars`, group captures by type, fill prefix [0, max] per type with synthetic entries (their wasm_type matches the type's other captures since type_name deterministically maps to a single WAT type), and sort each group descending by outer_idx so the lift-side push lands the highest outer_idx at the deepest stack position. Regression tests: new `TestIndexExprOfFnCall614` (3 tests) and `TestNonContiguousCapture615` (4 tests) classes cover all five concrete failure shapes plus a baseline that prevents regression of the case that was previously coincidentally working. Test count 3,766 -> 3,773 (added 7). Closes #614 and #615 from the ROADMAP stabilisation tier; HISTORY Stage 12 gains the v0.0.139 entry; KNOWN_ISSUES bug entries for #614 / #615 removed. Co-Authored-By: Claude <noreply@anthropic.invalid>
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughVersion 0.0.139 fixes two closure-codegen correctness bugs: function-call indexing loses return-type information when inferring indexed-access element types ( ChangesClosure Codegen Pair Fixes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #625 +/- ##
==========================================
+ Coverage 90.92% 90.94% +0.02%
==========================================
Files 59 59
Lines 22997 23024 +27
Branches 259 259
==========================================
+ Hits 20910 20940 +30
+ Misses 2080 2077 -3
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
HISTORY.md (1)
327-327:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTagged-release count needs incrementing.
The footer shows "138 tagged releases" but this PR ships v0.0.139. Based on learnings, the tagged-release total in HISTORY.md (and the corresponding count in ROADMAP.md) should be incremented together on each release.
Proposed fix
-Total: **810+ commits, 138 tagged releases, 55 active development days.** +Total: **810+ commits, 139 tagged releases, 55 active development days.**🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@HISTORY.md` at line 327, The footer in HISTORY.md still reads "138 tagged releases" but this PR adds v0.0.139; update the tagged-release count string to "139 tagged releases" in HISTORY.md (replace the "138 tagged releases" token) and make the same corresponding change in ROADMAP.md so both documents stay in sync for each release.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 184: The README has an inconsistent example count: the summary line (the
sentence containing "Vera is in **active development** at v0.0.139 — ... 34
examples ...") and the later sentence that says "33 example Vera programs"
disagree; run python scripts/check_doc_counts.py to get the canonical examples
count and update both occurrences to that single authoritative number (update
the sentence containing "34 examples" and the sentence that currently reads "33
example Vera programs") so the README is internally consistent.
---
Outside diff comments:
In `@HISTORY.md`:
- Line 327: The footer in HISTORY.md still reads "138 tagged releases" but this
PR adds v0.0.139; update the tagged-release count string to "139 tagged
releases" in HISTORY.md (replace the "138 tagged releases" token) and make the
same corresponding change in ROADMAP.md so both documents stay in sync for each
release.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 32102452-72e3-48a5-abb1-4bd2c685f5b6
⛔ Files ignored due to path filters (6)
docs/index.htmlis excluded by!docs/**docs/index.mdis excluded by!docs/**docs/llms-full.txtis excluded by!docs/**docs/llms.txtis excluded by!docs/**docs/sitemap.xmlis excluded by!docs/**uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (16)
CHANGELOG.mdHISTORY.mdKNOWN_ISSUES.mdREADME.mdROADMAP.mdTESTING.mdpyproject.tomltests/test_codegen_closures.pyvera/__init__.pyvera/codegen/closures.pyvera/codegen/core.pyvera/codegen/functions.pyvera/codegen/registration.pyvera/wasm/closures.pyvera/wasm/context.pyvera/wasm/inference.py
💤 Files with no reviewable changes (1)
- KNOWN_ISSUES.md
- README.md line 228: "33 example Vera programs" -> 34 (line 184 already says 34; fixes the internal inconsistency). - HISTORY.md footer line 327: "138 tagged releases, 55 active development days" -> "139 tagged releases, 56 active development days" to match this PR's release. Skipped: ROADMAP.md doesn't have a corresponding "tagged releases" counter — the previous trim removed it, so there's no stale string to update. Also fixed the PR description out-of-band so both #614 and #615 auto-close on merge: the original "Closes #614 and #615" form only triggered the link for #614 (GitHub's parser requires a close keyword before each issue number. Added explicit Closes
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@HISTORY.md`:
- Line 327: The HISTORY.md document has inconsistent "active development days"
values: the header says "55" while the summary line reads "56 active development
days"; pick the correct number and make both occurrences identical by updating
the header value or the summary line (the exact text to locate: the header "55"
near the top and the summary line "Total: **810+ commits, 139 tagged releases,
56 active development days.**") so the release history metadata is consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: faa33e89-64a9-42bf-9f21-4c0d22809352
📒 Files selected for processing (2)
HISTORY.mdREADME.md
Line 3 said "through Stage 11, across 55 active development days" while line 327 footer (just bumped to 56 in commit 431584d) said "56 active development days". Ground truth: `git log --pretty=format:'%ad' --date=short | sort -u | wc -l` returns 56 — so 56 is correct. Line 3 also had a stale "through Stage 11" scope qualifier — Stage 12 has been open since v0.0.138 (PR #612) but that PR's bump didn't touch line 3. Fixed both at once: "through Stage 12, across 56 active development days". Co-Authored-By: Claude <noreply@anthropic.invalid>
9 tasks
This was referenced May 8, 2026
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
Closes #614 and #615 — two distinct closure-codegen root causes that share a failure path. Both bugs were filed during the Tetris-experiment design review on PR #613; the original hypothesis (one fix would close both) turned out to be wrong, but the two fixes together close the user-visible failure mode for every shape we've reproduced.
Bumps version
v0.0.138 → v0.0.139per the release-on-fix convention.Root causes (independent)
#614 —
f()[i]element-type inference gap._infer_index_element_type_exprinvera/wasm/inference.pyonly handledSlotRefand nested-IndexExprcollections. When the collection was aFnCallreturningArray<T>, inference fell through toreturn None,_translate_index_exprreturned None too, and the enclosing function got dropped from the WAT output.f(x)[i]f(x)[i]_compile_lifted_closurereturns None → closure dropped →unknown table 0: table index out of boundsat WASM validationSame gap, two surfaces. Closing the inference path closes both manifestations.
#615 — closure capture order miscompile.
_collect_free_varsreturned captures in walker (DFS) order without filling missing prefix indices or sorting per-type. Two failure shapes from one root cause:@Int.kwhile skipping@Int.j(j<k) → lift-side env had no entry for j →env.resolve("Int", k)returned None → closure dropped → WASM validation trap.@Int.1 - @Int.2), the walker added(Int, 0)before(Int, 1)→ ascending push order → wrong stack layout underWasmSlotEnv.resolve(which usespos = len-1-index) → body's slot refs resolved to the WRONG captured locals. No trap, just wrong output.Discovered during the diagnosis: silent miscompute applied even to programs that were "supposedly" working — the sum tests in
test_closure_with_capturehappened to pass only because of commutative addition.Fixes
vera/codegen/registration.py_fn_ret_type_exprsdict alongside the WAT-type_fn_sigs.vera/codegen/core.py_fn_ret_type_exprs: dict[str, ast.TypeExpr]field.vera/codegen/functions.pyWasmContext.vera/codegen/closures.pyWasmContext.vera/wasm/context.py_fn_ret_type_exprsfield +set_fn_ret_type_exprs()setter.vera/wasm/inference.py_infer_index_element_type_exprto handle FnCall collection — look up return TypeExpr, extract Array element.vera/wasm/closures.py_collect_free_varsto group captures by type, fill prefix[0, max], sort descending per type.Regression tests
New test classes in
tests/test_codegen_closures.py:TestIndexExprOfFnCall614(3 tests) — top-level + closure + chained-double-index manifestations of Closure capturing data ADT and passing it to a function call inside body emits malformed WASM #614.TestNonContiguousCapture615(4 tests) — non-contiguous (tail-shape and silent-miscompute) + ascending-walker-order silent-miscompute + descending-walker-order baseline (prevents regression of the case that was coincidentally working pre-fix).Test count: 3,766 → 3,773 (+7).
Test plan
/tmp/tetris-repros/confirmed:unknown table 0trap → now returns20✓unknown table 0trap → now returns[13, 24, 35, 46]✓fits = false→ nowfits = true✓90, now-90(the correct value) ✓pytest tests/ -q -n auto— 3,759 passed, 14 skipped (was 3,752; +7 new regression tests)mypy vera/— clean (58 source files)python scripts/check_conformance.py— 86 programs passpython scripts/check_examples.py— 34 examples pass check + verifypython scripts/check_doc_counts.py— consistentpython scripts/check_version_sync.py— v0.0.139 across 6 filespython scripts/check_site_assets.py— up to datepython scripts/check_limitations_sync.py— consistentWhat this doesn't do
_compile_lifted_closurereturns None, the closure_id was already registered at the call site, so the call_indirect references a missing table entry → WASM validation trap. A safer pattern is to propagate the lift failure back to the parent function (so it gets the [E602] "skipped" warning rather than producing malformed WAT), or to track unrealised closure_ids and emit a runtime trap with a Vera-level diagnostic. Worth a follow-up issue; out of scope for this fix because closing the two underlying causes makes the failure unreachable for known programs.type Row = Array<Bool>, the alias path in_alias_array_elementalready handles it, but the FnCall branch could miss aliases that resolve only after monomorphisation. Not exercised by current tests; can extend if a real program hits it.Summary by CodeRabbit
Bug Fixes
Tests
Documentation
Chore
Closes #614
Closes #615