v0.0.135: fix #584 (Unit fn non-tail), #583 (Array<T> aliases), #568 (url_parse leading colon)#585
Conversation
Both bugs were in the same class - the type checker accepted code that the WASM codegen emitted invalid WAT for, with no Vera-native diagnostic in between. Found in the wild by a sandboxed Claude.ai instance writing Conway-style cellular automata. #584 - user-defined @Unit-returning fn in non-tail block-statement position emitted invalid WAT. _is_void_expr (vera/wasm/context.py) recognised IO.* qualified calls, UnitLit, effect-op FnCalls, and compound expressions, but missed FnCall to a user fn declared with @Unit return. The surrounding statement-sequencer fell through to "produces a value", emitted a stray drop, and WASM validation failed with "expected a type but nothing on stack". Fix: expand the codegen registry filter (_fn_ret_types in vera/codegen/functions.py) to include Unit-returning fns explicitly with None, then add a clause to _is_void_expr that recognises them. Recursive cases (Unit fn nested inside if/match arms in non-tail position) come for free via the existing recursion. #583 - type aliases over Array<T> broke codegen. _is_pair_type_name in vera/wasm/inference.py did string-pattern matching against unresolved alias names, so "type Row = Array<Bool>" left "Row" unrecognised as a pair type. Two failure modes: - SlotRef @Row.0 emitted only "local.get ptr" not (ptr, len); parameter use yielded invalid WAT at instantiate. - _slot_name_to_wasm_type("Row") returned None for a let binding, triggering silent E602 skip of the enclosing function. Fix: convert _is_pair_type_name from staticmethod to instance method that resolves aliases via _resolve_base_type_name first; complementary alias resolution added in _translate_array_lit (so [@Row.0, @Row.0] lays out elements at the correct stride) and _infer_index_element_type_expr (so @Row.0[1] resolves the element type correctly). Aliases work in parameter, let-binding, indexing, and array-literal-element positions. Conformance tests: tests/conformance/ch07_unit_fn_nontail.vera (#584; covers mid-block, leading, and if-arm-non-tail positions), tests/conformance/ch02_type_alias_array.vera (#583; covers all four alias-codegen positions). Conformance count: 82 -> 84. Doc / count sync: KNOWN_ISSUES.md drops the two closed bugs; SKILL.md drops their workaround entries (still-open #549 and #568 remain); CHANGELOG/version files bumped to 0.0.135. Co-Authored-By: Claude <noreply@anthropic.invalid>
|
Caution Review failedFailed to post review comments Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughFixes WASM codegen bugs for type-alias-over-Array and user ChangesWASM codegen fixes, tests, and release metadata
Sequence Diagram(s)sequenceDiagram
participant Codegen as Codegen
participant Ctx as WasmContext
participant Inf as Inference
participant Data as Data
participant Exec as Execute (runtime)
Codegen->>Codegen: build fn_ret_types map (include Unit → None)
Codegen->>Ctx: set_fn_ret_types(fn_ret_types)
Inf->>Inf: _alias_array_element() resolves Array<T> aliases
Inf->>Data: request resolved element type
Data->>Data: _translate_array_lit() resolve elem type and compute size
Ctx->>Ctx: _is_void_expr(FnCall)
alt FnCall in fn_ret_types mapped to None
Ctx->>Ctx: treat FnCall as void (no value)
else
Ctx->>Ctx: existing void checks (effects, UnitLit, QualifiedCall, etc.)
end
Codegen->>Exec: mark exported fns in fn_string_returns
Exec->>Exec: on execute(), decode (ptr,len) to Python str when fn listed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #585 +/- ##
==========================================
- Coverage 90.98% 90.97% -0.02%
==========================================
Files 59 59
Lines 22884 22930 +46
Branches 259 259
==========================================
+ Hits 20821 20860 +39
- Misses 2056 2063 +7
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:
|
Bundled into the same release because the fix is two lines of WAT plus
a comment. RFC 3986 sec 3.1 requires a non-empty scheme, so
url_parse(":foo") now returns Err("missing scheme") instead of Ok with
empty scheme that round-tripped through url_join as bare "foo".
The behaviour change is contained: any caller that relied on the old
Ok(...) had a malformed UrlParts (empty scheme_ptr/scheme_len) which
url_join would have emitted incorrectly anyway. Existing
ch09_url_parsing.vera test_parse_full / test_parse_no_query /
test_parse_err / test_join_full / test_roundtrip all still pass.
We don't yet enforce the full ALPHA / [ALPHA / DIGIT / "+" / "-" / "."]*
scheme grammar from RFC 3986 - that's a wider RFC-conformance follow-up.
Only the empty-scheme case lost its leading colon in the round-trip; the
full grammar enforcement isn't needed to close #568.
Conformance: extended ch09_url_parsing.vera with test_parse_empty_scheme
asserting Err return on ":foo". Conformance program count unchanged
(extension, not new file).
Doc updates: KNOWN_ISSUES.md drops #568 row; SKILL.md drops #568
from Known Bugs and Workarounds table; CHANGELOG.md adds #568 entry
to v0.0.135 Fixed section above the existing #584 + #583 entries.
Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@ROADMAP.md`:
- Line 253: Update the tagged-release summary string in ROADMAP.md from "134
tagged releases (as of v0.0.134)" to "135 tagged releases (as of v0.0.135)" to
reflect the new release; also update the corresponding release-count line in
HISTORY.md so both files remain consistent with the v0.0.135 bump.
In `@TESTING.md`:
- Line 9: Update the test-metrics row in TESTING.md so the numbers add up: the
current row reads "| **Tests** | 3,726 across 29 files (~34,800 lines of test
code; 3,702 passed, 14 skipped) |" which is inconsistent; change the passed
count to 3,712 (so 3,712 passed + 14 skipped = 3,726 total). Ensure the same
corrected string is used wherever that table row or exact phrase appears.
🪄 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: 75e98b10-eb7b-4931-8397-cd3ae2e94f0b
⛔ Files ignored due to path filters (7)
docs/SKILL.mdis excluded by!docs/**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/**tests/conformance/ch02_type_alias_array.verais excluded by!**/*.veratests/conformance/ch07_unit_fn_nontail.verais excluded by!**/*.vera
📒 Files selected for processing (17)
AGENTS.mdCHANGELOG.mdCLAUDE.mdFAQ.mdKNOWN_ISSUES.mdREADME.mdROADMAP.mdSKILL.mdTESTING.mdpyproject.tomlscripts/check_skill_examples.pytests/conformance/manifest.jsonvera/__init__.pyvera/codegen/functions.pyvera/wasm/context.pyvera/wasm/data.pyvera/wasm/inference.py
💤 Files with no reviewable changes (1)
- KNOWN_ISSUES.md
Three small fixes addressed in this commit: ROADMAP.md / HISTORY.md - tagged-release count was stale at 134 (reflecting v0.0.134) but this PR bumps to v0.0.135. Both files now report "135 tagged releases (as of v0.0.135)" / "135 tagged releases". TESTING.md - the test-metrics row was internally inconsistent: "3,726 across 29 files (3,702 passed, 14 skipped)". 3,702 + 14 = 3,716, not 3,726. Updated to 3,712 passed (3,712 + 14 = 3,726). The 3,702 figure was a stale leftover from before this PR added the two new conformance tests for #584 + #583. uv.lock - CI lint job ran "uv lock --check" which failed because pyproject.toml's version bumped 0.0.134 -> 0.0.135 but uv.lock still recorded vera at 0.0.134. Single-line change, regenerated via `uv lock`. Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
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)
272-273: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd Stage 11 table entry for v0.0.135.
The Stage 11 table shows individual version entries through v0.0.134 (line 272), but this PR releases v0.0.135 with three codegen bug fixes (
#584,#583,#568) and two new conformance tests. Following the established pattern where bug-fix releases like v0.0.128 and v0.0.129 received Stage 11 rows, this release warrants a table entry:| v0.0.134 | 6 May | **Active reclamation of host-store handles via heap-wrap-as-ADT — closes [`#573`](https://github.com/aallan/vera/issues/573), [`#575`](https://github.com/aallan/vera/issues/575), [`#576`](https://github.com/aallan/vera/issues/576), [`#579`](https://github.com/aallan/vera/issues/579)** (Map / Set / Decimal). | +| v0.0.135 | 6 May | **Codegen fixes: Unit-returning functions in non-tail positions, Array<T> type-alias resolution, url_parse empty-scheme rejection — closes [`#584`](https://github.com/aallan/vera/issues/584), [`#583`](https://github.com/aallan/vera/issues/583), [`#568`](https://github.com/aallan/vera/issues/568)**. Two new conformance tests (ch02_type_alias_array, ch07_unit_fn_nontail). |The footer release count (line 311) was correctly updated to 135. Based on learnings, per-release entries maintain the version ledger's completeness.
🤖 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` around lines 272 - 273, Add a new Stage 11 row for v0.0.135 in the Stage 11 table to mirror the existing per-release pattern (see the v0.0.134 and earlier entries such as v0.0.128/v0.0.129), noting the three codegen bug fixes (`#584`, `#583`, `#568`) and the two new conformance tests; ensure the entry format matches surrounding rows exactly (version, brief note of fixes/tests, and any links/PR numbers) so the release ledger remains continuous with the already-updated footer count of 135.
🤖 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.
Outside diff comments:
In `@HISTORY.md`:
- Around line 272-273: Add a new Stage 11 row for v0.0.135 in the Stage 11 table
to mirror the existing per-release pattern (see the v0.0.134 and earlier entries
such as v0.0.128/v0.0.129), noting the three codegen bug fixes (`#584`, `#583`,
`#568`) and the two new conformance tests; ensure the entry format matches
surrounding rows exactly (version, brief note of fixes/tests, and any links/PR
numbers) so the release ledger remains continuous with the already-updated
footer count of 135.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8d8ddebe-61ce-49a7-aae9-6b15c3f6b190
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (3)
HISTORY.mdROADMAP.mdTESTING.md
CodeRabbit finding (valid): the Stage 11 ledger ended at v0.0.134 but this PR bumps to v0.0.135 with three closed issues. Adding the row for ledger continuity, matching the format used by adjacent entries (one-liner with closes-N issue links plus a parenthetical summary). Co-Authored-By: Claude <noreply@anthropic.invalid>
|
@coderabbitai re: HISTORY.md Stage 11 ledger v0.0.135 row — valid finding, fixed in b9a1591. Added a row matching the format of the surrounding entries: terse one-liner with closes-N issue links plus a parenthetical summary of the three fixes (#584 user |
|
Tip For best results, initiate chat on the files or code changes.
|
Discovered while testing #583/#584 reproducers - vera run on a public main returning @string printed the heap pointer (e.g. "147492"), not the actual string. The (ptr, len) pair was being collapsed to its first half by the value extractor in execute(). Fix tracks which functions have a Vera return type that resolves to String (after alias resolution) via a new CompileResult.fn_string_returns set, populated in compile_program by walking each FnDecl's return type. execute() then reads memory[ptr:ptr+len] and UTF-8-decodes the bytes when fn_name is in that set, storing the decoded str in ExecuteResult.value. Array<T> returns deliberately keep the existing bare-pointer fallback - their bytes-at-ptr aren't UTF-8 and rendering them meaningfully would need element-type-aware formatting (separate scope). The asymmetry is locked in by the new test_array_return_unchanged regression. ExecuteResult.value type widens from "int | float | None" to "int | float | str | None". Only one existing test was checking for the pre-fix pointer value; updated to assert the decoded str. Tests added (test_codegen.py TestStringArraySignatures): - test_string_return_execution rewritten to assert == "hello" - test_string_alias_return_execution covering "type Greeting = String" - test_array_return_unchanged pinning the asymmetry Doc / count updates: - test_codegen.py: 1,065 -> 1,067 tests, 16,482 -> 16,521 lines - Total tests: 3,726 -> 3,728 (3,714 passed + 14 skipped) - TESTING.md and ROADMAP.md updated CHANGELOG entry added in [0.0.135] Fixed section above the existing #568 / #584 / #583 entries (no version bump - this rolls into the already-bumped 0.0.135). Co-Authored-By: Claude <noreply@anthropic.invalid>
Summary
Closes #584.
Closes #583.
Closes #568.
All three are codegen bugs in the same class — type checker accepts code that the WASM codegen handles incorrectly, with no Vera-native diagnostic in between. #584 and #583 surfaced via a sandboxed Claude.ai instance writing a cellular-automaton program; #568 was a pre-existing leading-colon round-trip bug. Bundled together because all three are small, surgical fixes (#568 is two lines of WAT plus a comment).
A fourth fix — a CLI display polish for String-returning
main— was added after testing the alias work surfaced it. Details in its own section below.#584 — user
@Unit-returning fn in non-tail positionBlock-statement sequencing in
vera/wasm/context.py::_is_void_exprrecognisedIO.*qualified calls,UnitLit, effect-opFnCalls, and compound expressions as void, but missedFnCallto a user-declared@Unitfn. The surrounding sequencer fell through to "produces a value", emitted a straydrop, and WASM validation failed withexpected a type but nothing on stack. The naturalrender(grid); IO.sleep(120); recurse(...)shape lands directly on this wheneverrenderis a user helper.Fix (5 lines logic + comment):
vera/codegen/functions.py— flip the_fn_ret_typesfilter fromif ret_wt and ret_wt != "unsupported"(which excluded Unit returns becauseNoneis falsy) toif ret_wt != "unsupported"so Unit-returning fns are registered withNonevalue. Already done this way invera/codegen/closures.py; this brings the two builders into agreement.vera/wasm/context.py— add aFnCallclause to_is_void_exprthat returns True when the call target is in_fn_ret_typeswith valueNone.#583 — type aliases over
Array<T>_is_pair_type_nameinvera/wasm/inference.pywas a@staticmethoddoing string-pattern matching against unresolved alias names, sotype Row = Array<Bool>left "Row" unrecognised as a pair type. Two failure modes from the issue body:@Row.0emitted onlylocal.get ptrnot(ptr, len)— parameter use yielded invalid WAT at instantiate._slot_name_to_wasm_type("Row")returnedNonefor a let binding, triggering silent E602 skip of the enclosing function.Fix:
vera/wasm/inference.py— convert_is_pair_type_namefrom@staticmethodto instance method that resolves aliases via_resolve_base_type_namefirst. All existing callers already passed names throughself._is_pair_type_name(...)so call sites need no change.vera/wasm/data.py::_translate_array_lit— resolve element-type aliases before the pair-layout helpers (so[@Row.0, @Row.0]lays out elements at the correct stride, not the 4-byte i32 path).vera/wasm/inference.py::_infer_index_element_type_expr— follow type aliases on the collection type so@Row.0[1]resolves the element type correctly. New helper_alias_array_elementhandles the common case of a non-generic alias pointing at a concreteArray<T>.Aliases now work in parameter, let-binding, indexing, and array-literal-element positions.
#568 —
url_parse/url_joinleading-colon round-tripurl_parse(":foo")returnedOkwith empty scheme;url_jointhen emitted bare"foo", losing the leading colon. Two viable fixes were noted in the issue body — RFC 3986-strict rejection vs. round-trip preservation flag. Going with RFC 3986 §3.1-aligned rejection (the cleaner option per maintainer call).Fix (2 lines WAT + comment):
vera/wasm/calls_encoding.py::_translate_url_parse— after the colon-scan loop, branch to$err_upifcolon_pos == 0. ReturnsErr("missing scheme")(the existing error message — empty scheme is also a missing scheme). No changes tourl_join; thes_len > 0gate stays since post-fix it can never see an empty scheme.We don't yet enforce the full ALPHA / [ALPHA / DIGIT / "+" / "-" / "."]* scheme grammar from RFC 3986 — that's a wider RFC-conformance follow-up — but the empty-scheme case alone is the only one that lost its leading colon in the round-trip; full grammar enforcement isn't needed to close #568.
CLI display polish —
vera runon String-returningmaindecodes the pairSpotted while testing the #583 alias fixes against host-handle types: a public
main(@Unit -> @String)returning"hello"printed something like147492instead ofhello. The (ptr, len) i32_pair return was being collapsed to its first half — the heap pointer — by the value extractor inexecute(). Distinct from the codegen bugs above (the WAT was already correct; only the CLI display was lying), but small enough to bundle.Fix:
vera/codegen/api.py— newCompileResult.fn_string_returns: set[str]populated bycompile_programfrom each FnDecl's return type. Alias-resolved via a new_return_type_is_stringhelper invera/codegen/core.pysotype Greeting = Stringparticipates (cooperates with Type aliases over Array<T> break WASM codegen (parameter case: invalid WAT; let-binding case: silent E602 skip) #583's alias work).vera/codegen/api.py::execute— whenraw_resultis a 2-tuple ANDfn_nameis infn_string_returns, readmemory[ptr:ptr+len]and decode as UTF-8. Stored inExecuteResult.value(type widened fromint | float | None→int | float | str | None). CLI'sprint(value)then renders the actual string.Array<T>returns deliberately keep the bare-pointer fallback — their bytes-at-ptr aren't UTF-8 and rendering them meaningfully would need element-type-aware formatting (separate scope). A newtest_array_return_unchangedtest pins this asymmetry.Test plan
vera check+vera runclean on the issue-body reproducers (all five variants for User-defined Unit-returning function call in non-tail position breaks WASM (block statement-sequencing emits drop with empty stack) #584, four for Type aliases over Array<T> break WASM codegen (parameter case: invalid WAT; let-binding case: silent E602 skip) #583, leading-colon for url_parse / url_join drops leading colon — ":foo" round-trips as "foo" #568, plus String-returning main now prints text not a pointer).tests/conformance/ch07_unit_fn_nontail.vera(User-defined Unit-returning function call in non-tail position breaks WASM (block statement-sequencing emits drop with empty stack) #584; mid-block, leading, and if-arm-non-tail positions)tests/conformance/ch02_type_alias_array.vera(Type aliases over Array<T> break WASM codegen (parameter case: invalid WAT; let-binding case: silent E602 skip) #583; param, let, index, nested-literal-element positions)tests/conformance/ch09_url_parsing.veraextended withtest_parse_empty_scheme(url_parse / url_join drops leading colon — ":foo" round-trips as "foo" #568)tests/test_codegen.py::TestStringArraySignatures:test_string_return_executionrewritten — assertsvalue == "hello"(was:isinstance(value, int))test_string_alias_return_execution— locks in alias cooperationtest_array_return_unchanged— pins the intentional Array-vs-String asymmetrycheck+verify.pytest tests/— 3,714 passed, 14 skipped (no new failures).mypy vera/clean.Doc updates
KNOWN_ISSUES.md— drop the Type aliases over Array<T> break WASM codegen (parameter case: invalid WAT; let-binding case: silent E602 skip) #583, User-defined Unit-returning function call in non-tail position breaks WASM (block statement-sequencing emits drop with empty stack) #584, and url_parse / url_join drops leading colon — ":foo" round-trips as "foo" #568 rows.SKILL.md— drop their workaround entries (the Type aliases over Array<T> break WASM codegen (parameter case: invalid WAT; let-binding case: silent E602 skip) #583 type-alias paragraph, the User-defined Unit-returning function call in non-tail position breaks WASM (block statement-sequencing emits drop with empty stack) #584 inline iteration-section footgun note, both rows from the Known Bugs and Workarounds table including url_parse / url_join drops leading colon — ":foo" round-trips as "foo" #568). Only GC-aware tail-call optimization for allocating functions #549 (TCO + GC) remains in the table.CHANGELOG.md— new[0.0.135]entry rolling up the [Unreleased] sandbox-install affordance from SKILL.md: tell sandbox-running agents they can install Vera #582 plus the four new fixes (CLI String display, url_parse / url_join drops leading colon — ":foo" round-trips as "foo" #568, User-defined Unit-returning function call in non-tail position breaks WASM (block statement-sequencing emits drop with empty stack) #584, Type aliases over Array<T> break WASM codegen (parameter case: invalid WAT; let-binding case: silent E602 skip) #583).vera/__init__.py,pyproject.toml,README.md,docs/index.html) bumped 0.0.134 → 0.0.135.TESTING.md,CLAUDE.md,SKILL.md,AGENTS.md,FAQ.md,ROADMAP.mdupdated 82 → 84 / 3,716 → 3,728.docs/llms.txt+docs/llms-full.txt+docs/SKILL.md+docs/index.md).uv.lockregenerated to recordvera 0.0.135(thelintCI job runsuv lock --checkwhich trips on a project version bump).HISTORY.mdStage 11 ledger row for v0.0.135.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features
Documentation
Tests
Chores