v0.0.129 — WASM call translator major correctness fixes (#475 PR 2 of 2)#567
Conversation
Closes out #475 with the seven Major findings from CodeRabbit's review of PR #474's calls.py decomposition. PR 1 (v0.0.128) shipped the three Critical safety findings; this release ships the seven Major correctness fixes: 1. array_slice clamps in i64 before wrapping (same shape as the v0.0.128 string_slice fix; uses the cross-mixin _clamp_i64_to_range_then_wrap helper). 2. Map<K, Array<T>> rejected at codegen — _map_wasm_tag returns str | None, with 11 call sites guarding for None. 3. url_parse / url_join round-trip — flags packed at offset 44 with explicit-mode sentinel bit 3; url_join falls back to len > 0 heuristic when sentinel is clear (data-constructor path). 4. base64_decode rejects = outside the padding region. 5. parse_nat / parse_int reject embedded spaces while still trimming leading + trailing whitespace via a separate tail-validation loop. 6. int_to_string(INT64_MIN) — loop break uses unsigned i64.eqz after div_u/rem_u digit extraction. 7. float_to_string handles fraction-rounding carry — frac_val computed before integer write so 1.9999996 prints as "2.0" not "1.0". 22 new test cases under TestArraySliceClamp475, TestMapArrayValueRejected475, TestUrlParseJoinRoundTrip475, TestBase64DecodePadding475, TestParseEmbeddedSpaces475, TestToStringInt64Min475, TestFloatToStringCarry475. Test count: 3,658 → 3,680. All 82 conformance programs and 33 examples pass. mypy clean. Co-Authored-By: Claude <noreply@anthropic.invalid>
📝 WalkthroughWalkthroughRelease v0.0.129 fixes seven major WASM call-translator correctness bugs: array-slice i64 clamping before index narrowing, Map<K, Array> type rejection at codegen, URL delimiter-presence preservation across parse/join round-trips, strict Base64 padding validation, embedded-space rejection in numeric parsing, INT64_MIN string conversion, and float-to-string fraction rounding carry propagation. Includes 416 lines of regression tests and documentation updates. ChangesWASM Translator Bug Fixes & Regression Coverage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 #567 +/- ##
========================================
Coverage 90.94% 90.94%
========================================
Files 59 59
Lines 22418 22552 +134
Branches 259 259
========================================
+ Hits 20387 20511 +124
- Misses 2024 2034 +10
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:
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
HISTORY.md (1)
306-306:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTagged release count not incremented in footer.
The footer still shows 128 tagged releases but should be 129 for v0.0.129. Based on learnings, this total must be kept in sync with the Stage 11 table and the ROADMAP.md footer.
🔢 Proposed fix
-Total: **810+ commits, 128 tagged releases, 54 active development days.** +Total: **810+ commits, 129 tagged releases, 54 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 306, Update the footer count showing "128 tagged releases" to "129 tagged releases" in HISTORY.md so it reflects v0.0.129; search for the exact footer text "Total: **810+ commits, 128 tagged releases, 54 active development days.**" and replace the 128 with 129, and also verify the same tagged-release count is synchronized with the Stage 11 table and the ROADMAP.md footer entries to keep all references consistent.ROADMAP.md (1)
282-282:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTagged release count not incremented in footer.
The footer still shows 128 tagged releases (as of v0.0.128) but should be 129 (as of v0.0.129). Based on learnings, this total must be kept in sync with HISTORY.md.
🔢 Proposed fix
-**810+ commits, 128 tagged releases (as of v0.0.128), 3,658 tests, 96% coverage, 82 conformance programs, 33 examples, 13 spec chapters.** See [HISTORY.md](HISTORY.md) for the full narrative of how the compiler was built. +**810+ commits, 129 tagged releases (as of v0.0.129), 3,680 tests, 96% coverage, 82 conformance programs, 33 examples, 13 spec chapters.** See [HISTORY.md](HISTORY.md) for the full narrative of how the compiler was built.Note: The test count in this footer (3,658) also appears stale — it should match the 3,680 stated in "Where we are" at line 11.
🤖 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 `@ROADMAP.md` at line 282, Update the footer line that currently reads "**810+ commits, 128 tagged releases (as of v0.0.128), 3,658 tests, 96% coverage, 82 conformance programs, 33 examples, 13 spec chapters.**" so the tagged releases count and version are incremented to "129 (as of v0.0.129)" and the test count is updated to "3,680" to match the "Where we are" section; edit the exact footer string in ROADMAP.md (the bold summary line) to replace "128" with "129", "v0.0.128" with "v0.0.129", and "3,658" with "3,680".vera/wasm/calls_parsing.py (1)
80-154: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winCentralise the ASCII-space predicate.
This PR now encodes
byte == 32/ “remaining bytes must also be spaces” in multiple translators in this mixin (parse_nat,parse_int, plus the existingparse_boolandparse_float64). Given this fix is all about space semantics, keeping that byte-set duplicated is the same drift trap that previously caused translator predicates to diverge. Based on learnings: "If two or more builtin functions use the same predicate over input bytes ... the later implementation MUST delegate to the earlier helper instead of re-encoding duplicatedi32.constbyte-value literals."🤖 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 `@vera/wasm/calls_parsing.py` around lines 80 - 154, The code duplicates the ASCII-space predicate (byte == 32) in parse_nat (this diff), parse_int and other translators (parse_bool, parse_float64); add a single helper (e.g., is_space_byte or wasm_is_space_byte) that takes the byte/local and returns non-zero for ASCII space, then replace every direct i32.const 32 / i32.eq or i32.ne comparison in parse_nat (the space-check before br_if $brk_pn and the tail loop), parse_int, parse_bool and parse_float64 with calls to that helper so all translators delegate to the same predicate implementation.
🤖 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 `@CHANGELOG.md`:
- Line 9: Update the release date on the CHANGELOG header for "0.0.129" (the
line containing "## [0.0.129] - 2026-04-29") to the correct chronological date
after "0.0.128" (which is 2026-05-05); replace 2026-04-29 with the actual
release date (e.g., 2026-05-06) so the changelog ordering and compare links are
consistent.
In `@HISTORY.md`:
- Line 266: The HISTORY.md entry for release tag v0.0.129 has a chronologically
inconsistent date ("29 Apr") relative to the previous v0.0.128 entry ("5 May");
update the date on the v0.0.129 line to be on or after 5 May (e.g., change the
"29 Apr" date to the correct release date such as "5 May" or the actual date of
the v0.0.129 release) so the timeline in HISTORY.md stays monotonic and accurate
for the v0.0.129 entry.
In `@tests/test_codegen.py`:
- Around line 14526-14538: The test currently fails for unrelated syntax/type
errors (use of Map.empty(), `@Map.0`) instead of the intended Map<K, Array<T>
codegen guard; update the test source so it is otherwise well-typed except for
using a map-of-array value: declare a properly typed map variable (e.g., a
binding with an explicit Map<int, Array<int>> type), use the regular identifier
(not `@Map.0`) for map_get, and call map_set with matching key/value types so that
the only remaining failure comes from the nested Array in the map or the codegen
guard; keep the pytest.raises check but ensure the source uses map_set,
Map.empty(), map_get, and a typed map binding (e.g., m : Map<...>) so the test
verifies the intended codegen rejection.
In `@vera/wasm/calls_containers.py`:
- Around line 136-168: The function _map_wasm_tag currently falls through and
returns "b" when vera_type is None, which reintroduces the bad behavior; change
_map_wasm_tag so that a None vera_type is treated as a hard rejection (return
None) immediately (i.e., add an explicit if vera_type is None: return None) and
keep callers like _infer_map_value_from_map_arg and _infer_set_elem_from_set_arg
responsible for propagating that None so map_get/map_values/set_to_array skip
emitting host imports when the element type is unknown.
In `@vera/wasm/calls_encoding.py`:
- Around line 1970-1978: The code currently treats an initial colon like an
absent scheme because the branch controlled by s_len (local variable referenced
as s_len and total) only increments the scheme delimiter when s_len > 0; update
the logic so explicit-mode URLs that start with ':' are preserved: either change
_translate_url_parse to reject colon_pos == 0 (so url_parse never returns a
leading-colon case), or add and propagate a boolean bit from
_translate_url_parse (e.g., colon_present) into the encoding logic used by
url_join/url_parse and use that bit instead of relying solely on s_len when
deciding to emit the ':' delimiter; ensure the symbols involved are
_translate_url_parse, url_join, url_parse, colon_pos, s_len and the branch that
currently gates emission of the ':' is switched to check the new colon_present
flag or the non-zero colon_pos contract.
---
Outside diff comments:
In `@HISTORY.md`:
- Line 306: Update the footer count showing "128 tagged releases" to "129 tagged
releases" in HISTORY.md so it reflects v0.0.129; search for the exact footer
text "Total: **810+ commits, 128 tagged releases, 54 active development days.**"
and replace the 128 with 129, and also verify the same tagged-release count is
synchronized with the Stage 11 table and the ROADMAP.md footer entries to keep
all references consistent.
In `@ROADMAP.md`:
- Line 282: Update the footer line that currently reads "**810+ commits, 128
tagged releases (as of v0.0.128), 3,658 tests, 96% coverage, 82 conformance
programs, 33 examples, 13 spec chapters.**" so the tagged releases count and
version are incremented to "129 (as of v0.0.129)" and the test count is updated
to "3,680" to match the "Where we are" section; edit the exact footer string in
ROADMAP.md (the bold summary line) to replace "128" with "129", "v0.0.128" with
"v0.0.129", and "3,658" with "3,680".
In `@vera/wasm/calls_parsing.py`:
- Around line 80-154: The code duplicates the ASCII-space predicate (byte == 32)
in parse_nat (this diff), parse_int and other translators (parse_bool,
parse_float64); add a single helper (e.g., is_space_byte or wasm_is_space_byte)
that takes the byte/local and returns non-zero for ASCII space, then replace
every direct i32.const 32 / i32.eq or i32.ne comparison in parse_nat (the
space-check before br_if $brk_pn and the tail loop), parse_int, parse_bool and
parse_float64 with calls to that helper so all translators delegate to the same
predicate implementation.
🪄 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: f0de6f50-16f4-4a2f-93ec-9b91618c1b7c
⛔ Files ignored due to path filters (5)
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/**uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (14)
CHANGELOG.mdHISTORY.mdKNOWN_ISSUES.mdREADME.mdROADMAP.mdTESTING.mdpyproject.tomltests/test_codegen.pyvera/__init__.pyvera/wasm/calls_arrays.pyvera/wasm/calls_containers.pyvera/wasm/calls_encoding.pyvera/wasm/calls_parsing.pyvera/wasm/calls_strings.py
💤 Files with no reviewable changes (1)
- KNOWN_ISSUES.md
…with reason
Inline (5):
1. CHANGELOG.md line 9 — fixed date 2026-04-29 → 2026-05-05 (chronologically
after v0.0.128 2026-05-05 entry)
2. HISTORY.md line 266 — fixed date 29 Apr → 5 May for the v0.0.129 row
3. test_codegen.py 14519-14545 — TestMapArrayValueRejected475 now uses
valid Vera syntax (bare type names in <...>, map_new() not Map.empty(),
no spurious @Map.0); asserts the exact codegen-skip path (function
missing from exports + E602 unsupported warning) rather than relying
on a parse error to satisfy pytest.raises
4. calls_containers.py — kept None → "b" fall-through; the strict
None-rejection broke 3 pre-existing tests (test_set_empty_to_array,
test_map_keys_in_if_expr, test_set_to_array_in_if_expr) that depend on
it for empty-collection inference. Updated comment to document why
the fall-through is load-bearing
5. calls_encoding.py 1970-1978 (URL leading colon) — SKIPPED. Verified
pre-existing on main (url_parse plus url_join drops the leading colon
both before and after PR 2). Filed as #568, added to KNOWN_ISSUES.md
Outside-of-diff (3):
6. HISTORY.md line 306 — fixed footer 128 tagged releases → 129; also
updated the By the numbers table column from v0.0.128 → v0.0.129 with
test count 3,658 → 3,680
7. ROADMAP.md line 282 — fixed footer 128 tagged releases (as of v0.0.128),
3,658 tests → 129 (as of v0.0.129), 3,680 tests; also fixed the
Vera v0.0.128 delivers... opening line on line 3
8. calls_parsing.py 80-154 (is_space_byte helper refactor) — SKIPPED.
Code-quality refactor across parse_nat / parse_int / parse_bool /
parse_float64; valid suggestion but expands PR scope when the user
instruction is keep changes minimal. Defer to a follow-up
code-quality PR
Test count: 3,680 (1 test renamed but count stays).
mypy clean, all 82 conformance programs pass, all 33 examples pass.
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)
tests/test_codegen.py (1)
14435-14864: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winPlease align new tests with the repo’s required helper pattern
The newly added tests use
_run,_run_io, and_compile_okdirectly. For added tests intests/**/*.py, this repo requires_check_ok()/_check_err()/_verify_ok()/_verify_err()patterns. Please migrate these new cases to the required helpers for consistency with the enforced test conventions.As per coding guidelines: “
tests/**/*.py: When adding a test, use_check_ok()/_check_err()/_verify_ok()/_verify_err()helpers from existing test patterns”.🤖 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 `@tests/test_codegen.py` around lines 14435 - 14864, Tests directly call _run, _run_io, and _compile_ok which violates the repo test helper convention; replace these direct calls with the repository's test helpers (_check_ok / _check_err / _verify_ok / _verify_err) by mapping each test's expectation: use _check_ok or _verify_ok for cases that should succeed (return values or IO output) and _check_err/_verify_err for cases expecting an Err, update assertions to use the helper's returned/verified result shape (e.g., for compilation use the helper that returns the result with exports/diagnostics instead of _compile_ok), and adjust imports if needed so all new tests call the standard helpers rather than _run/_run_io/_compile_ok (refer to occurrences of _run, _run_io, and _compile_ok in the added Test* classes to locate changes).
♻️ Duplicate comments (1)
vera/wasm/calls_containers.py (1)
170-183:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Nonestill reopens the bad single-slot import path.Allowing
_map_wasm_tag(None)to fall through to"b"means unsupportedMap<_, Array<_>>/Set<Array<_>>values still compile whenever the element type is not recoverable from aSlotRefor the handful of recognised built-ins. For example, a helper returningMap<String, Array<Int>>will still letmap_get/map_valuesmonomorphise to the one-i32$..._vbshape, which is the same signature mismatch this PR is meant to block. If empty-collection round-trips must keep compiling, that needs a narrower caller-side special case for syntactically empty producers rather than treating every uninferred type as scalar here.🤖 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 `@vera/wasm/calls_containers.py` around lines 170 - 183, The function _map_wasm_tag should not map an uninferred vera_type (None) to the scalar tag "b" because that re-enables the single-slot i32 import path; change the final fall-through so that when vera_type is None the function returns None (preserving only the existing Array and primitive branches mapping to tags), and move any special-case handling for syntactically empty producers to the caller-side logic that can detect truly empty collection producers rather than treating every uninferred type as scalar; update the return in _map_wasm_tag (and its docstring/comment) accordingly.
🤖 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 `@tests/test_codegen.py`:
- Around line 14435-14864: Tests directly call _run, _run_io, and _compile_ok
which violates the repo test helper convention; replace these direct calls with
the repository's test helpers (_check_ok / _check_err / _verify_ok /
_verify_err) by mapping each test's expectation: use _check_ok or _verify_ok for
cases that should succeed (return values or IO output) and
_check_err/_verify_err for cases expecting an Err, update assertions to use the
helper's returned/verified result shape (e.g., for compilation use the helper
that returns the result with exports/diagnostics instead of _compile_ok), and
adjust imports if needed so all new tests call the standard helpers rather than
_run/_run_io/_compile_ok (refer to occurrences of _run, _run_io, and _compile_ok
in the added Test* classes to locate changes).
---
Duplicate comments:
In `@vera/wasm/calls_containers.py`:
- Around line 170-183: The function _map_wasm_tag should not map an uninferred
vera_type (None) to the scalar tag "b" because that re-enables the single-slot
i32 import path; change the final fall-through so that when vera_type is None
the function returns None (preserving only the existing Array and primitive
branches mapping to tags), and move any special-case handling for syntactically
empty producers to the caller-side logic that can detect truly empty collection
producers rather than treating every uninferred type as scalar; update the
return in _map_wasm_tag (and its docstring/comment) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2a4ed6dc-f47f-43ca-bec9-3aede52dc6bb
📒 Files selected for processing (7)
CHANGELOG.mdHISTORY.mdKNOWN_ISSUES.mdROADMAP.mdTESTING.mdtests/test_codegen.pyvera/wasm/calls_containers.py
|
@coderabbitai Closing out the outside-of-diff comments from both rounds: Round 1 — 3 outside-of-diff:
Round 2 — 1 outside-of-diff + 1 duplicate:
All fixes validated: mypy clean, 3,680 tests pass, 82 conformance programs pass, 33 examples pass. |
|
Tip For best results, initiate chat on the files or code changes.
[resolve] ✏️ Learnings added
|
Summary
calls.pydecomposition (PR 2 of 2 for #475). PR 1 (v0.0.128) shipped the three Critical safety fixes and prematurely auto-closed Pre-existing bugs in WASM call translators (surfaced by #474 review) #475 via its commit subject; the issue has been reopened so this PR can close it correctly.vera/wasm/calls_*.py. No grammar, parser, or type-checker changes.The seven fixes
array_sliceclamps in i64 before wrapping (calls_arrays.py) — same shape as the v0.0.128string_slicefix. Pre-fix, large i64 indices wrapped to small in-range-looking i32 values and copied past the array. Post-fix uses the cross-mixin_clamp_i64_to_range_then_wraphelper (shared via Python MRO withCallsStringsMixin).Map<K, Array<T>>rejected at codegen (calls_containers.py) —_map_wasm_tagreturn type changed fromstrtostr | None; 11 call sites guard forNone. Pre-fix the type system held but codegen produced compromised binaries treating Array values as opaque pointers.url_parse/url_joinround-trip preserves URL shape (calls_encoding.py) — flag bits packed at struct offset 44 with an explicit-mode sentinel (bit 3). When sentinel is set,url_joinuses bits 0-2 forhas_auth/has_query/has_frag; when clear (data-constructor case), falls back to the legacylen > 0heuristic so existing direct-construction tests still pass.base64_decoderejects=outside the padding region (calls_encoding.py) — RFC 4648-compliant position check (idx >= slen - pad). Pre-fixAB=Cdecoded silently with the embedded=treated as zero bits.parse_nat/parse_intreject embedded spaces (calls_parsing.py) — leading whitespace still trimmed; trailing whitespace still allowed via a separate post-loop tail-validation block; embedded space (between digits) breaks the digit loop and the tail-validator's "every remaining byte must be a space" check fires.int_to_string(INT64_MIN)correct (calls_strings.py) — loop break uses unsignedi64.eqzafteri64.div_u/i64.rem_udigit extraction. Pre-fix the signedi64.le_s 0break terminated immediately because-INT64_MINoverflows back toINT64_MIN.float_to_stringhandles fraction-rounding carry (calls_strings.py) —frac_val = round((f - floor(f)) * 1_000_000)is computed before the integer part is emitted; iffrac_val == 1_000_000we increment the integer and resetfrac_valto 0. Pre-fix1.9999996printed as1.0instead of2.0.Tests added
TestArraySliceClamp475TestMapArrayValueRejected475Map<Nat, Array<Nat>>codegen rejectionTestUrlParseJoinRoundTrip475http:path, full URLs, query-with-body, empty?, empty#TestBase64DecodePadding475=rejectedTestParseEmbeddedSpaces475TestToStringInt64Min475TestFloatToStringCarry475Test plan
mypy vera/cleanpython scripts/check_conformance.py— all 82 conformance programs passpython scripts/check_examples.py— all 33 examples pass check + verifypython scripts/check_doc_counts.pycleanpython scripts/check_version_sync.py— v0.0.129 consistent acrosspyproject.toml,vera/__init__.py,docs/index.html,README.mdCloses #475
Summary by CodeRabbit
Release Notes – v0.0.129
Bug Fixes
Tests