Skip to content

Release v0.0.121: nested closures + ADT capture work (#514) + #527 CVE cleanup#536

Merged
aallan merged 8 commits into
mainfrom
v0.0.121-closures-and-cve-cleanup
Apr 27, 2026
Merged

Release v0.0.121: nested closures + ADT capture work (#514) + #527 CVE cleanup#536
aallan merged 8 commits into
mainfrom
v0.0.121-closures-and-cve-cleanup

Conversation

@aallan

@aallan aallan commented Apr 27, 2026

Copy link
Copy Markdown
Owner

Second from the bug-killing campaign. Closes the headline #514 closure-codegen bug and folds in the queued #527 CVE-ignore cleanup that was waiting for pip 26.1 to ship.

Closes #514
Closes #527

#514 — nested closures work end-to-end

Pre-fix: _lift_pending_closures iterated only the outer WasmContext's _pending_closures list. _compile_lifted_closure created a fresh inner ctx to translate the body, and any fn { ... } discovered during that translation registered on the inner ctx — never bubbled back. Result: only the outermost closure was lifted, only $anon_0 ended up in the function table, and inner call_indirects targeted a missing entry. Surface symptoms varied by inner return type:

  • pair-typed return → type mismatch: expected i64, found i32 at WASM validation
  • primitive return → unreachable at runtime

Fix: convert _lift_pending_closures to a worklist.

  • _compile_lifted_closure gains a collect_pending parameter that bubbles the inner ctx's _pending_closures back to the worklist.
  • Inner ctx's _closure_sigs and _next_closure_id are now shared by reference with the module-level state to avoid $closure_sig_0 / $anon_0 name collisions across contexts.
  • Lifter handles arbitrary nesting depth (verified at three levels).
  • _walk_free_vars now recurses into nested AnonFn (was missing the case entirely; latent because nested closures didn't make it through lifting in the first place).

Investigation surfaced a residual — the historical "all heap captures broken" framing was inaccurate. ADT captures actually work (single-pointer representation). Only pair types (String, Array<T>) silently drop the len field during closure-struct serialisation. Filed as #535 with a clear pointer-only fix path.

#527 — CI cleanup

pip 26.1 shipped on 2026-04-26 with the pypa/pip#13870 fix for CVE-2026-3219. Removed the bridge: --ignore-vuln CVE-2026-3219 from .github/workflows/ci.yml, the row from KNOWN_ISSUES.md's CI ignores table, and the per-flag annotation from TESTING.md. Verified locally that pip-audit --skip-editable against pip 26.1 returns "No known vulnerabilities found" without the ignore.

Doc sweep (per the user's request to look for code that worked around the limitation)

No examples/*.vera had a #514 workaround to remove (the SKILL reference to examples/life.vera was aspirational — that example doesn't exist).

New example + conformance

  • examples/nested_closures.vera: 2D array_map (build_grid), two-layer array_fold (grid_sum = 60), 3D nesting (three_d_count). All 6 contracts verified at Tier 1.
  • tests/conformance/ch05_nested_closures.vera (level: run): 2D no-capture, 2D with outer-param capture, 3D nesting all in one program.

Test plan

  • 5 new TestNestedClosures cases in tests/test_codegen_closures.py — primitive return, pair return (the original Closures capturing heap-allocated values produce invalid WASM (nested-closure case is a narrow instance) #514 reproducer), outer-param capture, three-level nesting, white-box check that $anon_0 AND $anon_1 are both in the lifted function table.
  • Full suite: 3,553 passed, 14 skipped (pre-fix: 3,543 + 14 — +5 nested closure tests, +5 from conformance parametrisation, zero regressions).
  • All 33 examples pass vera check + vera verify (Tier 1 baseline 213 → 219, captured in test_overall_tier_counts).
  • All 81 conformance programs pass their declared level.
  • mypy clean.
  • All 12 doc validators green (version sync, doc counts, limitations sync, etc.).

Files changed (24 files, +532/-317)

Area Files
Code vera/codegen/closures.py, vera/wasm/closures.py
CI .github/workflows/ci.yml
Tests tests/test_codegen_closures.py, tests/test_verifier.py
New examples/nested_closures.vera, tests/conformance/ch05_nested_closures.vera
Docs SKILL.md, KNOWN_ISSUES.md, ROADMAP.md, HISTORY.md, CHANGELOG.md, TESTING.md, README.md, AGENTS.md, CLAUDE.md, FAQ.md, docs/index.html
Build pyproject.toml, vera/__init__.py, uv.lock, tests/conformance/manifest.json, scripts/check_skill_examples.py
Generated docs/SKILL.md, docs/index.md, docs/llms.txt, docs/llms-full.txt

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Full end-to-end support for nested closures
  • Improvements

    • Closure capture now works for primitives and single‑pointer ADTs; pair‑type (pointer+length) captures remain a known caveat
    • CI dependency-audit now upgrades pip before auditing and no longer suppresses the pip-related CVE (other ignore retained)
  • Bug Fixes

    • Added regression tests and example programs exercising nested-closure scenarios
  • Documentation

    • Updated docs, changelog and guidance to reflect fixes, new release and remaining pair‑type caveat

aallan and others added 2 commits April 27, 2026 11:27
pip 26.1 shipped on 2026-04-26 carrying the pypa/pip#13870 fix for
the concatenated-tar+ZIP archive-handling bug (CVE-2026-3219,
GHSA-58qw-9mgm-455v). Verified locally:

  $ pip install pip==26.1 pip-audit
  $ pip-audit --skip-editable
  No known vulnerabilities found

Removes the bridge from three places:

- .github/workflows/ci.yml: drop --ignore-vuln CVE-2026-3219 flag
  and its dated comment block (the pygments CVE-2026-4539 ignore
  stays — that one still has no upstream fix release).
- KNOWN_ISSUES.md: drop the row from the 'CI ignores' table.
  (The section header stays, with the pygments row still present.)
- TESTING.md: drop the per-flag annotation from the dependency-audit
  command example so it matches the workflow.

Closes #527 — the action item there was specifically 'remove this
ignore once pip 26.1 is on PyPI' and the trigger has fired.

Bundled with the v0.0.121 #514 closure-codegen work as the first
commit on the campaign branch — small, mechanical, ships before
the bigger codegen change so the cleanup isn't blocked by review
iterations on the closure fix.

Co-Authored-By: Claude <noreply@anthropic.invalid>
Second from the bug-killing campaign. Two related fixes that share the
same surface (vera/codegen/closures.py and vera/wasm/closures.py)
ship together as v0.0.121, alongside the #527 CVE-ignore cleanup that
had been queued since pip 26.1 shipped on 2026-04-26.

## Closes

  #514  - Closures capturing heap-allocated values produce invalid WASM
          (nested-closure case is a narrow instance)
  #527  - CI: ignore CVE-2026-3219 (pip 26.0.1 archive-handling)
          until pip 26.1 ships

## Splits

  #535  - Pair-type captures (String, Array<T>) silently drop the
          len field. Investigation during #514 showed the historical
          'all heap captures broken' framing was inaccurate: ADT captures
          actually work, only pair types are affected. Filed as a
          residual follow-up with a clear pointer-only fix path.

## Fix shape: closure-lifting worklist

Pre-fix: _lift_pending_closures iterated only the outer WasmContext's
_pending_closures list. _compile_lifted_closure created a fresh inner
ctx to translate the body, and any fn { ... } discovered during that
translation registered on the inner ctx -- never bubbled back. Result:
only the outermost closure was lifted, only $anon_0 ended up in the
function table, and inner call_indirects targeted a missing entry,
surfacing as 'type mismatch: expected i64, found i32' (validation)
when the inner returned a pair type, or 'unreachable' (runtime)
otherwise.

Fix: convert _lift_pending_closures to a worklist.

  - _compile_lifted_closure gains a collect_pending parameter that
    bubbles the inner ctx's _pending_closures back to the worklist
  - Inner ctx's _closure_sigs and _next_closure_id are now shared by
    reference with the module-level state to avoid $closure_sig_0 /
    $anon_0 name collisions across contexts
  - Lifter handles arbitrary nesting depth (verified at three levels)
  - _walk_free_vars now recurses into nested AnonFn (was missing the
    case entirely; latent because nested closures didn't make it
    through lifting in the first place)

## Files changed

  vera/codegen/closures.py   - worklist + collect_pending + share state
  vera/wasm/closures.py      - AnonFn case in _walk_free_vars
  tests/test_codegen_closures.py - 5 new TestNestedClosures cases
  tests/test_verifier.py     - bumped tier1 baseline (213 -> 219)

## New example + conformance

  examples/nested_closures.vera         - 2D array_map, 2-layer fold,
                                          3D nesting, all working
  tests/conformance/ch05_nested_closures.vera (level: run)

## Doc sweep

  - SKILL.md 'Capturing outer bindings' section rewritten -- the old
    'primitives only' framing was inaccurate (ADTs work too); pair
    types are the residual
  - SKILL.md 'Known limitation: nested closures' subsection removed
    entirely -- no longer broken
  - SKILL.md Known Bugs table: replaced #514 row with #535 row;
    removed #522 row (closed in v0.0.120); updated #516 row to
    reflect Stage 1 having shipped
  - KNOWN_ISSUES.md, ROADMAP.md, HISTORY.md, CHANGELOG.md parallel
    updates; ROADMAP queue intro now reads 'ten remain'

## CI cleanup folded in (#527)

  - Drop --ignore-vuln CVE-2026-3219 from .github/workflows/ci.yml
  - Drop CVE-2026-3219 row from KNOWN_ISSUES CI ignores table
  - Update TESTING dependency-audit command example

## Verification

  - 3553 passed, 14 skipped (pre-fix: 3543 + 14 -- +5 nested closure
    tests, +5 conformance via parametrisation, no regressions)
  - mypy clean
  - All 33 examples pass check + verify (Tier 1: 213 -> 219)
  - All 81 conformance programs pass their declared level
  - All 12 doc validators green

Co-Authored-By: Claude <noreply@anthropic.invalid>
@codecov

codecov Bot commented Apr 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.02%. Comparing base (5b7bd95) to head (f382a8a).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #536   +/-   ##
=======================================
  Coverage   91.01%   91.02%           
=======================================
  Files          58       58           
  Lines       22014    22033   +19     
  Branches      259      259           
=======================================
+ Hits        20036    20055   +19     
  Misses       1971     1971           
  Partials        7        7           
Flag Coverage Δ
javascript 56.83% <ø> (ø)
python 94.97% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai

coderabbitai Bot commented Apr 27, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Implements FIFO worklist-driven nested-closure lifting with shared closure signature/ID propagation and extends free-variable walking to recurse into anonymous nested functions. Adds tests, manifest entry, documentation and version bumps, updates CI pip-audit to upgrade pip and removes the CVE-2026-3219 ignore.

Changes

Cohort / File(s) Summary
Nested closure compiler implementation
vera/codegen/closures.py, vera/wasm/closures.py
Introduces deque-based FIFO worklist lifting and shared _closure_sigs/_next_closure_id propagation; _walk_free_vars now treats ast.AnonFn as nested scope so nested bodies contribute to outer capture lists.
Nested closure tests & manifest
tests/test_codegen_closures.py, tests/conformance/manifest.json
Adds regression tests verifying multi-level nested closures (runtime checks and WAT inspection) and a ch05_nested_closures.vera manifest entry.
Closure compilation internals
vera/codegen/closures.py
_lift_pending_closures now processes a worklist seeded from ctx._pending_closures; _compile_lifted_closure accepts collect_pending and returns inner pending closures while propagating signature/counter state.
Free-var walker
vera/wasm/closures.py
Extends the free-variable walker to clone/increment param_counts for AnonFn and recurse, ensuring inner anonymous functions contribute to outer captures.
Tests & verifier expectations
tests/test_verifier.py
Updates expected verifier totals: Tier 1 213→219, total 239→245.
Docs & metrics
TESTING.md, SKILL.md, KNOWN_ISSUES.md, ROADMAP.md, FAQ.md, AGENTS.md, CLAUDE.md, README.md, CHANGELOG.md, HISTORY.md
Bumps counts (tests, conformance 80→81, examples 32→33), documents nested-closure fixes and pair-type capture caveat, replaces per-CVE ignore with pip upgrade + retaining ignore for CVE-2026-4539, adds changelog/history entries.
Allowlist maintenance
scripts/check_skill_examples.py
Updates ALLOWLIST line-number mappings to match shifted SKILL.md fences; removes obsolete nested-closures workaround entry and relocates many entries.
Packaging & version
pyproject.toml, vera/__init__.py
Bumps package version 0.0.120 → 0.0.121.
CI & dependency audit
.github/workflows/ci.yml, TESTING.md
Runs pip install --upgrade pip before dependency audit and removes --ignore-vuln CVE-2026-3219 from pip-audit; keeps --ignore-vuln CVE-2026-4539.

Sequence Diagram

sequenceDiagram
    participant Parser
    participant Worklist as ClosureLiftingWorklist
    participant FreeVar as FreeVarWalker
    participant InnerCtx as WasmContext (inner)
    participant Module as ModuleState

    Parser->>Worklist: discover pending closures
    Worklist->>Worklist: seed deque from outer pending

    loop for each closure in worklist
        Worklist->>InnerCtx: create inner WasmContext (fresh)
        InnerCtx->>Module: share `_closure_sigs` & `_next_closure_id` (by reference)
        InnerCtx->>FreeVar: walk closure body for free variables

        FreeVar->>FreeVar: clone param_counts, recurse into nested `AnonFn`
        FreeVar->>Module: record captures discovered via recursion

        InnerCtx->>InnerCtx: compile lifted closure body
        InnerCtx->>Worklist: append any inner pending closures found
        InnerCtx->>Module: propagate updated `_next_closure_id`
    end

    Worklist->>Module: finalize lifted closures & signatures
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested labels

compiler, tests, ci, docs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main changes: nested closures support (issue #514), ADT capture improvements, and CVE cleanup (issue #527).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch v0.0.121-closures-and-cve-cleanup

Comment @coderabbitai help to get the list of available commands and usage tips.

actions/setup-python@v6 bakes pip 26.0.1 into its Python 3.12 toolchain;
the runner doesn't pick up newly-released pips from PyPI until GitHub
refreshes the image. Without an explicit upgrade step, pip-audit scans
the runner's own pip, finds 26.0.1 in the environment, and flags it
for CVE-2026-3219 (the very issue v0.0.121 cleared via #527).

Local verification (which used the latest PyPI pip) returned clean,
so the audit logic itself is correct -- the runner pip just hasn't
caught up. Add 'pip install --upgrade pip' before installing the
vera package and pip-audit. Comment notes the upgrade can be dropped
once the runner image ships pip 26.1.

Co-Authored-By: Claude <noreply@anthropic.invalid>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@FAQ.md`:
- Line 206: The prose in FAQ.md is inconsistent: the conformance suite count was
changed to "81-program conformance suite" but an adjacent example bullet still
says "30"; update the example bullet so counts match the release (use "81"
instead of "30") or run/consult scripts/check_doc_counts.py to derive the
canonical example counts and replace the outdated "30" with the authoritative
number from that script to keep FAQ.md consistent with the release docs.

In `@HISTORY.md`:
- Line 258: Condense the Stage 11 HISTORY.md row into one or two sentences: keep
the core accomplishments (nested closures and ADT capture now work end-to-end,
closure-lifting and by-reference sig/ID sharing to avoid name collisions) and
mention the key fix (_walk_free_vars now recurses into AnonFn) plus removal of
the CVE-2026-3219 ignore, but remove detailed examples and multi-clause
explanations so the entry matches the concise style used elsewhere.

In `@README.md`:
- Line 184: Update the inconsistent example count in the README by running the
canonical check script scripts/check_doc_counts.py to determine the correct
number of examples, then make the prose consistent (replace the "33 examples" or
the "32" example occurrence so both match the script's reported value); ensure
both occurrences of the example count in the README (the sentence containing
"Vera is in **active development** at v0.0.121 — ... 33 examples" and the
project structure line that currently says "32") are changed to the single
canonical value and re-run scripts/check_doc_counts.py to verify no other doc
counts need adjustment.

In `@ROADMAP.md`:
- Line 272: Update the aggregate summary string that currently reads "120 tagged
releases (as of v0.0.120)" to reflect the new release by bumping it to "121
tagged releases (as of v0.0.121)"; locate the text in ROADMAP.md containing the
phrase "120 tagged releases (as of v0.0.120)" and change both the numeric count
and the referenced release tag to "121" and "v0.0.121" respectively so the
summary matches the newly added release row.
- Line 25: The summary sentence claiming "ten remain in the campaign" is
inconsistent with the listed issue IDs (`#515`, `#516`, `#517`, `#520`, `#475`,
`#535`, `#487`, `#348`, `#346`, `#347`, `#490`) which total eleven; update the
sentence in the paragraph that mentions v0.0.120/v0.0.121 and the remaining bugs
to reflect the correct count (change "ten" to "eleven") or remove/adjust the
count so it matches the listed issue IDs exactly, ensuring the phrase "ten
remain in the campaign" or its surrounding clause is corrected wherever it
appears.

In `@tests/test_codegen_closures.py`:
- Around line 625-630: Replace the fragile exact-name assertions that look for
"(func $anon_0" and "(func $anon_1" with a count-based check that the WAT
contains at least two lifted anon closures; for example, search the generated
WAT string for the substring "$anon_" (or a regex r"\$anon_\d+") and assert the
number of matches is >= 2 while keeping the existing table-size check below;
update the assertions in tests/test_codegen_closures.py (the block currently
asserting "(func $anon_0" and "(func $anon_1") to use this occurrence-count
approach so ordering/ordinal changes won't break the test.

In `@tests/test_verifier.py`:
- Around line 1609-1611: The explanatory narrative above the assertions for t1,
t3, and total is out of date; locate the explanatory comment or docstring that
describes the historical counts (near the assertions that check t1, t3, and
total) and update the numbers and wording to match the new expected totals 219
(T1), 26 (T3), and 245 (total) so the test message and narrative align with the
assertions.

In `@vera/codegen/closures.py`:
- Around line 43-44: The loop uses an ordinary list and worklist.pop(0) which is
O(n) per removal; replace worklist with a collections.deque and use popleft()
for O(1) removals: import collections (or from collections import deque),
initialize worklist = deque(...) where it is created, and change worklist.pop(0)
to worklist.popleft() in the while loop that destructures anon_fn, captures,
closure_id.
🪄 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: ed3c8ec9-e887-4854-93a9-41d2da0ca81d

📥 Commits

Reviewing files that changed from the base of the PR and between 5b7bd95 and d64e20d.

⛔ Files ignored due to path filters (8)
  • docs/SKILL.md is excluded by !docs/**
  • docs/index.html is excluded by !docs/**
  • docs/index.md is excluded by !docs/**
  • docs/llms-full.txt is excluded by !docs/**
  • docs/llms.txt is excluded by !docs/**
  • examples/nested_closures.vera is excluded by !**/*.vera
  • tests/conformance/ch05_nested_closures.vera is excluded by !**/*.vera
  • uv.lock is excluded by !**/*.lock, !uv.lock
📒 Files selected for processing (19)
  • .github/workflows/ci.yml
  • AGENTS.md
  • CHANGELOG.md
  • CLAUDE.md
  • FAQ.md
  • HISTORY.md
  • KNOWN_ISSUES.md
  • README.md
  • ROADMAP.md
  • SKILL.md
  • TESTING.md
  • pyproject.toml
  • scripts/check_skill_examples.py
  • tests/conformance/manifest.json
  • tests/test_codegen_closures.py
  • tests/test_verifier.py
  • vera/__init__.py
  • vera/codegen/closures.py
  • vera/wasm/closures.py

Comment thread FAQ.md
Comment thread HISTORY.md Outdated
Comment thread README.md Outdated
Comment thread ROADMAP.md Outdated
Comment thread ROADMAP.md Outdated
Comment thread tests/test_codegen_closures.py Outdated
Comment thread tests/test_verifier.py
Comment thread vera/codegen/closures.py Outdated
aallan and others added 2 commits April 27, 2026 12:56
All eight rabbit findings verified against current code; all real,
all fixed:

1. (real) FAQ.md line 207: '30 working example programs' -> '33'
   (my earlier sed missed this exact phrasing).

2. (real) HISTORY.md row v0.0.121: condensed from a 5-clause
   paragraph to one sentence matching the file's older Stage 1
   convention. Per memory feedback_coderabbit.md: check the FILE'S
   OLDEST examples for the format target, not the nearest rows.

3. (real) README.md line 228: '32 example Vera programs' -> '33'
   (project structure ascii art, missed by my earlier sed).

4. (real) ROADMAP line 272: '120 tagged releases (as of v0.0.120)'
   -> '121 tagged releases (as of v0.0.121)'. Forgot this one in
   the version-bump pass.

5. (real) ROADMAP line 25: 'ten remain' -> 'eleven remain' with the
   issue list spelled out (#515, #516, #517, #520, #475, #535, #487,
   #348, #346, #347, #490). v0.0.121 closed #514 and added #535,
   net zero, so still 11. I had miscounted.

6. (real) test_codegen_closures.py: replaced 'assert "(func $anon_0"
   in wat' / 'assert "(func $anon_1" in wat' with a re.findall
   count check. Closure-ID assignment order is an implementation
   detail; counting >= 2 lifted functions is the real invariant.

7. (real) test_verifier.py docstring: 213/26/239 narrative was
   stale after I bumped the assertions to 219/26/245. Updated the
   docstring with the new trajectory: 184 -> 213 -> 219 with the
   nested_closures.vera contribution noted.

8. (real, perf) vera/codegen/closures.py: worklist now uses
   collections.deque + popleft() instead of list.pop(0). O(1) vs
   O(n) per removal. Worklists are typically tiny so the saving
   is theoretical at present, but it's the right idiom and
   removes the need to reason about pop-cost as nesting grows.

Plus #537 wiring (your point that the runner-pip-upgrade workaround
needs to be tracked, not just commented):

  - File #537 'CI: drop pip --upgrade once setup-python ships pip
    26.1+' with explicit removal trigger
  - Renamed KNOWN_ISSUES section 'CI ignores' -> 'CI workarounds'
    (broader umbrella covering both --ignore-vuln flags and forced
    upgrades, both share the bridge-with-removal-trigger pattern)
  - Added a row for the pip --upgrade workaround pointing at #537
  - Updated the workflow comment to cross-reference #537 and the
    KNOWN_ISSUES section

Co-Authored-By: Claude <noreply@anthropic.invalid>
Per user reinforcement: 'HISTORY file is single line summaries.
Detail is for the CHANGELOG.' Drift into multi-clause implementation
paragraphs has been the recurring CR finding since v0.0.118; my
v0.0.121 row still had two sentences with implementation detail
(worklist + sig sharing + walk_free_vars). Trimmed to one sentence
+ residual + CI cleanup, matching the v0.0.114-117 canonical form.

Memory feedback_coderabbit.md updated with the third-occurrence
reinforcement so the rule sticks.

Co-Authored-By: Claude <noreply@anthropic.invalid>
Comment thread README.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@FAQ.md`:
- Around line 206-207: Update the stale bullet that reads "153 built-in
functions" to match the release docs by changing the text to "164 built-in
functions"; locate the list items in FAQ.md where the adjacent bullets mention
built-in functions (the line currently showing "153 built-in functions") and
replace that number so the status snapshot matches the other release
documentation.

In `@HISTORY.md`:
- Line 258: The release footer still reads "120 tagged releases" but you added a
new entry "v0.0.121"; update the footer count to "121 tagged releases" so the
tagged-release total matches the new v0.0.121 entry (look for the footer text
containing "tagged releases" in HISTORY.md and increment the numeric value).

In `@README.md`:
- Line 184: Update the stale test total in the README headline: replace the
"3,548 tests" figure in the string that begins "Vera is in **active
development** at v0.0.121 — 810+ commits, 121 releases, 3,548 tests, ..." with
the correct pytest collected total (3,567) to match the release totals (3,553
passed / 14 skipped). Regenerate or verify the number against the canonical
source scripts/check_doc_counts.py or TESTING.md before committing so the README
and release metrics remain consistent.

In `@tests/test_codegen_closures.py`:
- Around line 547-572: The test test_nested_closure_with_outer_param_capture
currently only asserts array_length(`@Array`<Array<Int>>.0) == 3 which doesn't
verify that the inner closure captures `@Int.1`; change the test body so the
nested array contents are reduced to a capture-dependent scalar (for example,
flatten or sum the inner arrays produced by array_map over array_range) and
assert the numeric total (e.g. 18) instead of length; update the invocation that
checks the result via _run("test")/assert to expect the scalar sum so that the
inner closure's use of `@Int.1` (and functions array_map, array_range, nat_to_int,
array_length) is actually exercised and validated.
🪄 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: 628dca22-3772-4957-8fec-b176584e5052

📥 Commits

Reviewing files that changed from the base of the PR and between 05ede31 and f414110.

⛔ Files ignored due to path filters (1)
  • docs/llms-full.txt is excluded by !docs/**
📒 Files selected for processing (10)
  • .github/workflows/ci.yml
  • FAQ.md
  • HISTORY.md
  • KNOWN_ISSUES.md
  • README.md
  • ROADMAP.md
  • TESTING.md
  • tests/test_codegen_closures.py
  • tests/test_verifier.py
  • vera/codegen/closures.py

Comment thread FAQ.md
Comment thread HISTORY.md
Comment thread README.md Outdated
Comment thread tests/test_codegen_closures.py Outdated
All four real, all fixed:

1. (real) FAQ.md line 208: '153 built-in functions' -> '164'
   (number was last touched pre-Stage 11 stdlib expansion).

2. (real) HISTORY.md line 295: '120 tagged releases' -> '121'
   in the bottom roll-up footer (was a separate string from the
   top roll-up I'd already updated). Same drift class as the
   ROADMAP line-272 fix from the previous round.

3. (real) README.md line 184: '3,548 tests' -> '3,567'.
   I had guessed 3,548 = 3,534 baseline + 14 in the version-bump
   pass without running pytest; the actual collection is 3,567
   (3,553 passed + 14 skipped) per check_doc_counts.py.

4. (real) test_nested_closure_with_outer_param_capture: replaced
   the trivial length-only assertion (== 3) with a sum-via-fold
   that actually depends on the captured @Int.1 flowing into the
   inner closure body. Cells are (col + row); summing the 3x3
   grid gives 18. A length-only check would pass even if the
   capture silently returned 0 inside the inner closure -- the
   sum forces the captured value through into the result.

Co-Authored-By: Claude <noreply@anthropic.invalid>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@TESTING.md`:
- Line 76: Update the inconsistent test count in TESTING.md so the "Running the
conformance suite" example matches the actual parametrized test total for
test_conformance.py: either change the text "(parametrized — 400 tests)" to
"(parametrized — 405 tests)" or remove the hard-coded numeric count entirely and
use a generic phrase (e.g., "(parametrized — X tests)" or "parametrized tests")
to avoid future drift; edit the "Running the conformance suite" example and any
other occurrences referencing test_conformance.py to keep them consistent.
- Around line 11-12: Update the mismatched conformance count so the "Conformance
Suite" intro text that currently reads "a collection of 80" is changed to "a
collection of 81" to match the table entry (**Conformance programs** | 81
programs...) and ensure any other occurrences of "80" referring to the
conformance suite in TESTING.md are updated for consistency.

In `@tests/test_codegen_closures.py`:
- Around line 520-545: The test test_nested_closure_inner_returns_array only
asserts the outer length and can miss malformed inner Array<Int> payloads;
update the test body (the src string used by _run) so the nested array_map
produces a scalar derived from inner elements (e.g., sum or a specific
transformed element) and assert that _run("test") returns that expected scalar
value, thereby validating the inner Array<Int> payload produced by the inner
closure (look for the array_map calls and the nat_to_int(array_length(...))
return expression and replace it with a scalar computation based on inner
values).
🪄 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: c7b72ffe-0622-4c1d-8770-8ada458e97fe

📥 Commits

Reviewing files that changed from the base of the PR and between f414110 and 7de39ff.

⛔ Files ignored due to path filters (1)
  • docs/llms-full.txt is excluded by !docs/**
📒 Files selected for processing (5)
  • FAQ.md
  • HISTORY.md
  • README.md
  • TESTING.md
  • tests/test_codegen_closures.py

Comment thread TESTING.md
Comment thread TESTING.md
Comment thread tests/test_codegen_closures.py Outdated
All three real, all fixed:

1. (real) TESTING.md line 86: 'a collection of 80' -> '81'.
   The previous round's similar 80->81 fix landed elsewhere in the
   same file but missed this specific phrasing. Worth noting for
   #528 -- check_doc_counts.py is gated on 80 in 4 known places
   in TESTING.md but doesn't currently catch the 'a collection of'
   phrasing.

2. (real) TESTING.md line 187: '(parametrized -- 400 tests)' ->
   '405 tests' to match the live test_conformance.py count of 405
   (one new conformance program added 5 parametrised checks).

3. (real) test_nested_closure_inner_returns_array: same length-only
   weakness as the previous round's outer-param-capture test.
   Replaced length assertion with a sum-via-nested-fold over every
   cell. Each row is [0, 2, 4]; 3 rows gives 18. A length-only
   check would have passed even if the inner closure returned a
   malformed Array<Int> (wrong elements, zeros from a broken
   pair-write, etc.). The sum forces the inner values through.

Co-Authored-By: Claude <noreply@anthropic.invalid>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/test_codegen_closures.py`:
- Around line 610-679: Add a new unit test that builds two separate outer
closures each defining inner closures (similar structure to
test_nested_closure_emits_anon_for_inner) to exercise shared `_next_closure_id`
allocation; use _compile_ok(src) to get result.wat, use re.findall(r"\(func
\$anon_\d+") to collect anon function names and assert len(anon_funcs) >= 4 and
len(set(anon_funcs)) == len(anon_funcs) to ensure uniqueness, then
re.search(r"\(table\s+(\d+)\s+funcref\)", wat) to extract table_size and assert
table_size >= 4 so all lifted closures are placed in the table and invokable.
🪄 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: 26daffdd-4bf9-4cf1-a97b-97b4ac144d0d

📥 Commits

Reviewing files that changed from the base of the PR and between 7de39ff and 7296b59.

📒 Files selected for processing (2)
  • TESTING.md
  • tests/test_codegen_closures.py

Comment thread tests/test_codegen_closures.py
Real coverage gap: TestNestedClosures::test_nested_closure_emits_anon_for_inner
exercises one top-level function with one outer + one inner closure,
but doesn't catch the class of regression where module-level state
(self._closure_sigs, self._next_closure_id) gets re-initialised between
top-level functions instead of staying shared by-reference.

If a future refactor moved that state from module-level to per-function-call,
the existing tests would all still pass (each only exercises one top-level
fn) but compilation would fail at module link time when two functions both
tried to register $anon_0 or $closure_sig_0.

New test_two_top_level_fns_with_nested_closures: two separate public
functions, each with one outer + one inner closure (4 lifted total).
Asserts:

  - >= 4 distinct $anon_N functions in the WAT (no ID-counter reset)
  - all $anon_N names are unique (set length == list length)
  - function table size >= 4 (all four are call_indirect-invokable)
  - both functions actually run (catches WASM validation failures)

Test count: 3567 -> 3568. TESTING.md per-file row updated 24/679 -> 25/759.

Co-Authored-By: Claude <noreply@anthropic.invalid>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant