Fix mono-suffix bug for FnType-alias SlotRef args + release v0.0.145#659
Merged
Conversation
Closes #604, #655 (Shape A). ## Root cause (#604 + #655 Shape A) `_unify_param_arg` in `vera/codegen/monomorphize.py` had an `AnonFn`-specific alias-resolution path; when a generic call's closure argument was a `SlotRef` typed as an FnType alias (e.g. `@Doubler.0` where `type Doubler = fn(Int -> Int)`), that path was skipped. The fall-through `_get_arg_type_info` couldn't bind the alias's type variables to the param type's positional args (name mismatch — `"Doubler" != "OptionMapFn"`). The unbound type variable then fell to the `"Bool"` phantom-var fallback at result-building time (line 300), producing mono suffixes like `option_map$Int_Bool` instead of `option_map$Int_Int`. At runtime the call_indirect inside the wrong-suffix mono clone expected an `i32` (Bool) return; the user's closure returned `i64` (Int) → `wasm trap: indirect call type mismatch`. ## Fix Factored the AnonFn / SlotRef-alias dichotomy into a shape-resolver helper `_resolve_arg_fn_shape` that returns `(param_types, return_type)` for either kind of "callable arg": - `AnonFn` literal → its declared params + return type - `SlotRef` typed with an FnType alias → the alias's resolved params + return type `_infer_fn_alias_type_args` now operates on this shape uniformly. Purely additive: existing AnonFn behavior unchanged, SlotRef-alias case starts binding correctly. Same parallel fix applied at the WASM call-site rewriting layer in `vera/wasm/calls.py::_infer_fn_alias_type_args_wasm` (new `_resolve_arg_fn_shape_wasm` helper). ## Template-warning suppression (audit recommendation 2) The 5 prelude generic templates (`option_unwrap_or`, `option_map`, `option_and_then`, `result_unwrap_or`, `result_map`) emit `[E602]` / `[E604]` warnings even though their mono clones work end-to-end. The warning fires on the *template* — which can never be compiled since its type vars have no monomorphic WASM representation — and was the source of "5 spurious warnings per compile" noise. Post-compile suppression pass in `vera/codegen/core.py::compile_program` drops `[E602]`/`[E604]`/`[E605]` template-only warnings on forall decls whose mono clones successfully compile (appear in `_fn_sigs` with a `$<types>` suffix). Preserves the "this generic can never compile and you're never using a mono clone of it" signal for genuinely-broken or unused user generics — warning still fires for forall decls with zero successful mono clones. This is audit recommendation 2 from the #604 investigation comment. Pre-fix the warnings were the only signal that wrong-mono-suffix bugs existed; post-fix (the mono-suffix bug is closed) they're pure noise for the catalogued cases. ## Allowlist cleanup `scripts/check_e602_clean.py` allowlist shrinks from 11 → 6 entries: - Removed: 5 user-code generics from #655 Shape A (`identity`, `const`, `is_some`, `are_equal`, `cmp_sign`) — these no longer fire because their mono clones compile in every test program that uses them, triggering the new suppression. - Kept: 5 prelude generics (still fire in test files that don't call them — intentional per audit) + `head` (genuine codegen gap for refinement-of-Array, unrelated to this fix). ## Regression test New `TestGenericMonoSuffixFromSlotRef604` class with two tests: 1. `test_mono_suffix_correct_for_slotref_fn_alias_arg` — asserts the WAT output contains `$option_map$Int_Int` (and not `$option_map$Int_Bool`) when `option_map` is called with a SlotRef typed as an FnType alias. 2. `test_template_warning_suppressed_when_mono_clone_compiles` — asserts no `[E602]`/`[E604]` diagnostics fire for `option_map` / `option_unwrap_or` in a program that calls them successfully. Both fail against the pre-fix codepath, locking the regression- detection contract. ## Not closed by this fix - **#559** (nested type aliases `type Grid = Array<Array<Foo>>`): separate codegen-side alias-resolution gap, not the monomorphizer type-arg inference. Reproducer still trips a different error (malformed WAT instead of unknown-func). KNOWN_ISSUES.md row added for tracking (was missing pre-#658). - **#655 Shape B** (`head` over refinement-of-Array): genuine codegen gap, not a monomorphization issue. Allowlist entry retained. ## Release prep (v0.0.145) Bundled into this PR per the agreed "no separate cleanup PRs" policy: - `vera/__init__.py` + `pyproject.toml` + `uv.lock`: 0.0.144 → 0.0.145 - `CHANGELOG.md`: date-stamped `[0.0.145] - 2026-05-11` with Fixed entries; new `[Unreleased]` heading; updated `[Unreleased]` and `[0.0.145]` compare-URL link refs - `HISTORY.md` Stage 12 table: new v0.0.145 row (one-sentence format per HISTORY-entries-are-NOT-mini-CHANGELOGs rule) - `docs/index.html`: version tag manual update - `README.md` / `TESTING.md` / `ROADMAP.md`: count refreshes (3,807 tests, 145 releases, etc.) - `CLAUDE.md`: "Stage 9 table" reference replaced with stage-agnostic instruction pointing at the most-recent Stage table in HISTORY.md (caught during 2026-05-11 review cycle — stale since the project moved through Stages 10, 11, 12) - `KNOWN_ISSUES.md`: new Bugs row for #559 ## Verification - mypy: clean (59 source files) - pytest: 3,793 passed, 14 skipped (was 3,791 + 2 new regression tests) - conformance: all 86 programs pass - examples: all 34 pass `vera check` + `vera verify` - e602 gate: 116 files scanned clean, 6 allowlist entries matched (down from 11), 0 stale - version-sync: 0.0.145 consistent across 6 files - doc-counts: consistent (3807 tests, 29 files, 86 conformance, 34 examples) - limitations-sync: consistent (36 in KNOWN_ISSUES, 18 in vera/ README, 4 across spec chapters) Co-Authored-By: Claude <noreply@anthropic.invalid>
|
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:
📝 WalkthroughWalkthroughThis release (v0.0.145) extends monomorphization to infer FnType-alias bindings from SlotRef callables, suppresses template-only diagnostics when a mono clone compiles, adds regression tests, updates the allowlist and substitution semantics, and bumps version/docs. ChangesGeneric SlotRef FnType alias type-arg inference and warning suppression
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 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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #659 +/- ##
==========================================
- Coverage 91.03% 91.03% -0.01%
==========================================
Files 60 60
Lines 23134 23197 +63
Branches 259 259
==========================================
+ Hits 21060 21117 +57
- Misses 2067 2073 +6
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)
333-333:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the tagged releases count from 144 to 145.
This release is v0.0.145, so the footer total should reflect 145 tagged releases, not 144. Based on learnings, the tagged-release count should be kept consistent across all designated locations in HISTORY.md and ROADMAP.md.
📊 Proposed fix
-Total: **810+ commits, 144 tagged releases, 58 active development days.** +Total: **810+ commits, 145 tagged releases, 58 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 333, Update the footer string "Total: **810+ commits, 144 tagged releases, 58 active development days.**" to reflect the new tagged release count (change 144 to 145) for release v0.0.145, and then search HISTORY.md and ROADMAP.md for any other occurrences of "144 tagged releases" or similar footer lines and update them to "145 tagged releases" so the tagged-release count is consistent across files.vera/codegen/monomorphize.py (1)
679-718:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve nested type arguments when binding alias vars.
These branches bind
arg_param_te.name/arg_return.name, which collapses concrete shapes likeArray<Int>toArray. That will mis-monomorphise higher-order aliases whenever the callable’s parameter or return type is itself parameterised. Please record the full formatted type name here, not just the outer constructor.🤖 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/codegen/monomorphize.py` around lines 679 - 718, The code currently binds alias_mapping entries using only the outer .name (e.g., arg_param_te.name, arg_return.name, art.name), which loses nested type arguments; change those bindings so they record the full formatted type string of the matching AST type node (not just the constructor name). Concretely, in the blocks that set alias_mapping for parameter and return matches (the loops using alias_te.params/arg_params and the return-handling code using ret and arg_return), replace use of .name with a full type serialization of the corresponding arg node (for param types, use a formatter on arg_param_te; for return types, use a formatter on arg_return or its type_args element art). Use the existing type-formatting helper if one exists (or add a small helper like format_type(type_node) that includes nested type_args) and store that formatted string in alias_mapping under the alias parameter name.vera/wasm/calls.py (1)
688-722:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the Wasm alias inference mirror in lock-step with monomorphisation.
This mirror still strips nested type args via
.name, and it no longer mirrors the monomorphiser’sarg_return: ast.FnType -> "Fn"case. A function-returning or parameterised callable can therefore monomorphise to one suffix and be rewritten here to another, which brings back the sameunknown func/ indirect-call mismatch class of failure.🤖 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.py` around lines 688 - 722, The alias inference currently collapses nested type args to .name and omits the monomorphiser's special-case where arg_return can be an ast.FnType (normalized to name "Fn"), causing mismatches; update the logic in the block that builds alias_mapping (referencing alias_te, alias_params, arg_params, arg_return, alias_mapping, ast.NamedType) to (1) handle arg_return being an ast.FnType by treating it as if it were a NamedType with name "Fn" (same normalization the monomorphiser uses), and (2) when matching type arguments do not strip nested type expressions to .name blindly — instead compare/propagate the full type expression or explicitly handle nested NamedType vs other ast nodes so that nested parameterised types are mapped consistently with the monomorphiser.
🤖 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 `@scripts/check_e602_clean.py`:
- Around line 85-93: The _extract_skips() function filters only ("E602", "E604")
but the comment and behavior expect E605 to be treated as a template-warning
too; update the filter/tuple inside _extract_skips() (the place that currently
checks for "E602" and "E604") to include "E605" so the gate watches E605 as
well.
In `@tests/test_codegen.py`:
- Around line 15880-15903: The test
test_mono_suffix_correct_for_slotref_fn_alias_arg only checks WAT suffixes but
doesn't execute the compiled module, so add a runtime assertion that actually
runs the module's entrypoint (invoke the compiled result's main or run method
returned by _compile_ok(self._SLOT_FN_SRC)) and assert it completes without
trapping; keep the existing WAT checks but after obtaining result (result =
_compile_ok(...)) call the runtime execution helper on result (e.g.,
result.run_main()/result.invoke_main()/result.run()) and assert no
trap/exception is raised and the return value matches expected, to pin the
indirect-call signature regression.
In `@vera/codegen/core.py`:
- Around line 545-574: The current suppression builds mono_compiled from
self._fn_sigs (which is populated before actual mono bodies are compiled),
hiding real errors; instead, collect base function names only for clones that
were actually compiled (i.e., where the compile step produced a non-None body).
Concretely: replace the logic that derives mono_compiled from self._fn_sigs with
a set populated at the point clones are compiled (the branches that call
_compile_fn or where fn_wat is assigned) by adding base =
fn_name.split("$",1)[0] to mono_compiled only when fn_wat is not None (or
_compile_fn returned a non-None result); then use that mono_compiled set
(alongside suppressible_codes and diagnostics) to filter diagnostics as shown.
In `@vera/codegen/monomorphize.py`:
- Around line 625-631: The current branch in monomorphize.py returns an alias_fn
type (alias_te) without applying the SlotRef's type_args, causing generic
parameters (like T) to remain uninstantiated; update the logic in the
isinstance(arg, ast.SlotRef) branch to instantiate the alias before returning by
substituting arg.type_args for the alias's type parameters inside alias_te (so
alias_te.params and alias_te.return_type are rewritten to concrete types), e.g.
by invoking or reusing the existing _infer_fn_alias_type_args() / a small
substitution helper to map alias type variables to arg.type_args and then return
the substituted (params_tuple, return_type).
In `@vera/wasm/calls.py`:
- Around line 651-654: When encountering a SlotRef in the resolver (the if block
checking isinstance(arg, ast.SlotRef) and fetching alias_te from _type_aliases),
you must apply the SlotRef's type arguments to the alias body before returning
it so the resolver sees the instantiated fn type (not the raw generic body).
Modify the branch that handles alias_te being an ast.FnType to substitute alias
type parameters with arg's type arguments (e.g., map alias type params ->
arg.type_args and apply that mapping to alias_te.params and
alias_te.return_type) and then return the instantiated (params, return_type);
this will keep _resolve_generic_call's inferred mangled names consistent with
Pass 1.5/monomorphiser.
---
Outside diff comments:
In `@HISTORY.md`:
- Line 333: Update the footer string "Total: **810+ commits, 144 tagged
releases, 58 active development days.**" to reflect the new tagged release count
(change 144 to 145) for release v0.0.145, and then search HISTORY.md and
ROADMAP.md for any other occurrences of "144 tagged releases" or similar footer
lines and update them to "145 tagged releases" so the tagged-release count is
consistent across files.
In `@vera/codegen/monomorphize.py`:
- Around line 679-718: The code currently binds alias_mapping entries using only
the outer .name (e.g., arg_param_te.name, arg_return.name, art.name), which
loses nested type arguments; change those bindings so they record the full
formatted type string of the matching AST type node (not just the constructor
name). Concretely, in the blocks that set alias_mapping for parameter and return
matches (the loops using alias_te.params/arg_params and the return-handling code
using ret and arg_return), replace use of .name with a full type serialization
of the corresponding arg node (for param types, use a formatter on arg_param_te;
for return types, use a formatter on arg_return or its type_args element art).
Use the existing type-formatting helper if one exists (or add a small helper
like format_type(type_node) that includes nested type_args) and store that
formatted string in alias_mapping under the alias parameter name.
In `@vera/wasm/calls.py`:
- Around line 688-722: The alias inference currently collapses nested type args
to .name and omits the monomorphiser's special-case where arg_return can be an
ast.FnType (normalized to name "Fn"), causing mismatches; update the logic in
the block that builds alias_mapping (referencing alias_te, alias_params,
arg_params, arg_return, alias_mapping, ast.NamedType) to (1) handle arg_return
being an ast.FnType by treating it as if it were a NamedType with name "Fn"
(same normalization the monomorphiser uses), and (2) when matching type
arguments do not strip nested type expressions to .name blindly — instead
compare/propagate the full type expression or explicitly handle nested NamedType
vs other ast nodes so that nested parameterised types are mapped consistently
with the monomorphiser.
🪄 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: e9f209e8-393d-411e-af1e-e00ca5c7700f
⛔ 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.mdCLAUDE.mdHISTORY.mdKNOWN_ISSUES.mdREADME.mdROADMAP.mdTESTING.mdpyproject.tomlscripts/check_e602_clean.pytests/test_codegen.pyvera/__init__.pyvera/codegen/core.pyvera/codegen/monomorphize.pyvera/wasm/calls.py
Five findings from CodeRabbit's review of #659 — three 🟠 Major correctness issues and two 🟡 Minor quality-of-life improvements. All five verified valid against current code and addressed minimally. CR-1 (`scripts/check_e602_clean.py:174`) — 🟠 `_extract_skips()` filtered only `("E602", "E604")` but the CHANGELOG entry and the suppression pass in `vera/codegen/core.py` both treat `[E605]` as part of the same template-warning surface. A future E605 silent skip would have slipped past the gate. Added `"E605"` to the filter tuple + inline comment cross-referencing the suppression pass. CR-2 (`tests/test_codegen.py:15880`) — 🟡 `test_mono_suffix_correct_for_slotref_fn_alias_arg` asserted WAT suffix strings but didn't execute the compiled module — a regression that produced correct WAT but broken indirect-call wiring would slip past. Added `execute(result, fn_name="main")` + `assert exec_result.value == 20` to pin the runtime contract. Also added a sibling test `test_parameterised_alias_substitutes_type_args` exercising the CR-4/CR-5 fix with the same execute+assert shape (21 for `Some(7) * 3`). CR-3 (`vera/codegen/core.py:552`) — 🟠 Pre-fix the template-warning suppression checked `_fn_sigs` for the existence of mono clones. But `_fn_sigs` is populated in Pass 1.5 (registration) *before* any mono body is compiled — so a clone that `_compile_fn(mdecl)` later rejects with `None` would still be counted as "compiled" and wrongly suppress its template's diagnostic. That hides the only pre-runtime signal for a genuinely-broken generic. Introduced `compiled_mono_bases: set[str]` populated only inside the `if fn_wat is not None:` branch of the mono-clone compile loop. Suppression now uses `compiled_mono_bases & forall_decl_names`. CR-4 (`vera/codegen/monomorphize.py::_resolve_arg_fn_shape`) — 🟠 For a *parameterised* FnType alias like `type Mapper<T> = fn(T -> T)`, `@Mapper<Int>.0` resolved to the raw alias body `fn(T -> T)` without substituting `T → Int`. Downstream `_infer_fn_alias_type_args` then bound alias-local names (`A → T`, `B → T`) instead of concrete ones, producing mono suffixes like `option_map$T_T` rather than `option_map$Int_Int`. Fixed by substituting the SlotRef's `type_args` into the alias body before returning. CR-5 (`vera/wasm/calls.py::_resolve_arg_fn_shape_wasm`) — 🟠 Same gap on the WASM call-site rewriting side. Same fix applied. Shared substitution helper Rather than duplicate the type-var substitution logic in two codegen modules, extended the existing module-level `substitute_type_vars` in `vera/wasm/inference.py` (used by `InferenceMixin` canonicaliser + `CodeGenerator._type_expr_to_wasm_type`) to also handle `FnType` (it previously handled only `NamedType` and `RefinementType`). Both `_resolve_arg_fn_shape` helpers now import + use the canonical implementation — same semantics across all four callers, no drift risk. Validation - mypy: clean (59 source files) - pytest: 3,794 passed, 14 skipped (was 3,793 + 1 new test) - e602 gate: 116 files clean, 6 allowlist entries matched, 0 stale - All 86 conformance programs pass; all 34 examples pass `vera check` + `vera verify` Refs #604 #655 #659 Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@TESTING.md`:
- Line 9: The README-table entry at line 9 under the "Tests" row incorrectly
lists the aggregate test code lines as "~35,939"; update that cell to show the
corrected total "~50,638" (so the row reads something like "| **Tests** | 3,808
across 29 files (~50,638 lines of test code; 3,794 passed, 14 skipped) |") to
reflect the actual per-file aggregate.
In `@tests/test_codegen.py`:
- Around line 15962-15998: Add a sibling negative-control test to
tests/test_codegen.py (next to
test_template_warning_suppressed_when_mono_clone_compiles) that compiles a
program which defines/imports the same generic template(s) but never calls them
(use _compile_ok to get result), then collect warnings (result.diagnostics with
d.severity == "warning") and assert that there are diagnostics whose error_code
is in {"E602","E604","E605"} and whose description starts with the target
function name (e.g., "option_map" or a generic that is unused); this ensures the
suppression only applies when a mono clone actually compiled rather than
globally suppressing template-only warnings.
In `@vera/wasm/calls.py`:
- Around line 723-727: In _resolve_generic_call(), the return-type binding only
handles ast.NamedType so alias type vars that should bind to a function get left
unresolved; update the block that inspects alias_te.return_type (and uses
alias_params, arg_return, alias_mapping) to mirror the monomorphiser by also
detecting when the argument return represents a function and binding the alias
name to "Fn" (i.e., if ret is ast.NamedType and ret.name in alias_params and
arg_return indicates a function type, set alias_mapping[ret.name] = "Fn" instead
of skipping it) so the WASM matcher and monomorphiser agree.
🪄 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: 8eb86b86-06ef-44a1-ab41-532e67dd7533
📒 Files selected for processing (8)
ROADMAP.mdTESTING.mdscripts/check_e602_clean.pytests/test_codegen.pyvera/codegen/core.pyvera/codegen/monomorphize.pyvera/wasm/calls.pyvera/wasm/inference.py
Three further findings from CodeRabbit's review of `067817b` — one
🟠 Major correctness mirror, two 🟡 Minor quality fixes.
CR-6 (`TESTING.md:9`) — 🟡
The "lines of test code" prose count in the overview table read
~35,939; the actual `find tests -name "test_*.py" -type f -exec
wc -l {} +` aggregate is 50,638. My running deltas correctly
tracked incremental changes but preserved an already-incorrect
baseline. `scripts/check_doc_counts.py` only validates the
total-tests + file-count fields against live counts (regex over
prose at lines 121-130), not the per-file line-count tail of the
"Tests" row — so the drift wasn't caught locally. Updated to
~50,638.
CR-7 (`tests/test_codegen.py::TestGenericMonoSuffixFromSlotRef604`)
— 🟡
The existing `test_template_warning_suppressed_when_mono_clone_compiles`
covered only the positive path. An over-broad suppressor that
dropped *all* forall-decl warnings would pass that test silently.
Added a sibling negative-control test
`test_template_warning_NOT_suppressed_when_generic_never_called`:
defines `private forall<T> fn unused_generic(@t -> @t)` that's
never called; asserts an `[E604]` template warning DOES fire for
it (no mono clone exists for an uncalled generic, so the
suppression's `compiled_mono_bases & forall_decl_names`
intersection is empty for this fn). Pin protects the targeted-
not-blanket suppression semantics.
CR-8 (`vera/wasm/calls.py::_infer_fn_alias_type_args_wasm`) — 🟠
The monomorphiser's `_infer_fn_alias_type_args` (in
`vera/codegen/monomorphize.py`) has an `elif isinstance(
arg_return, ast.FnType): alias_mapping[ret.name] = "Fn"` branch
that handles higher-order aliases (callable arg returns another
function). The WASM-side mirror was missing that branch. In
that case the return-type var stayed unbound, fell back to
`"Bool"` at result-building time, and `_resolve_generic_call`
rewrote the call to a mangled name that did NOT match the mono
clone Pass 1.5 registered. Added the matching branch with a
comment cross-referencing the monomorphiser-side binding and
explaining the failure mode without it (e.g. a `type Lifter<F> =
fn(Int -> F) effects(pure)` called with a fn-returning AnonFn).
Validation
- mypy: clean (59 source files)
- pytest: 3,795 passed, 14 skipped (was 3,794 + 1 new test)
- e602 gate: 116 files clean, 6 allowlist matched, 0 stale
- doc-counts: consistent
Refs #604 #655 #659
Co-Authored-By: Claude <noreply@anthropic.invalid>
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 `@tests/test_codegen.py`:
- Around line 15921-15960: The test
test_parameterised_alias_substitutes_type_args currently only asserts the
presence of the concrete clone "$option_map$Int_Int" but doesn't fail if an
unsubstituted "$option_map$T_T" clone is also emitted; add a negative assertion
to ensure "$option_map$T_T" is absent from wat (e.g. assert "$option_map$T_T"
not in wat) placed after the existing assert that checks for
"$option_map$Int_Int" and before executing the runtime check, so the test pins
the substitution fix rather than allowing a dead generic clone.
🪄 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: 9f88c755-2887-4dd7-82c1-920977c24a73
📒 Files selected for processing (4)
ROADMAP.mdTESTING.mdtests/test_codegen.pyvera/wasm/calls.py
…lone `test_parameterised_alias_substitutes_type_args` previously only asserted the presence of `$option_map$Int_Int` after the `T → Int` substitution. A regression that emitted BOTH the substituted clone AND a stray `$option_map$T_T` would still pass the test silently (partial-substitution bug class). Added `assert "$option_map$T_T" not in wat` right after the positive assertion, with an inline comment noting the bug class it would catch. Refs #604 #655 #659 Co-Authored-By: Claude <noreply@anthropic.invalid>
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 `@tests/test_codegen.py`:
- Around line 15901-15909: The assertions in tests/test_codegen.py use simple
"in"/"not in" checks on the WAT string (variable wat) which allow substring
false positives for similarly-prefixed symbols like "$option_map$Int_Int" and
"$option_map$Int_Bool"; update those checks to use boundary-safe regex matching
instead (e.g., replace the direct membership checks for the symbols
"$option_map$Int_Int" and "$option_map$Int_Bool" with re.search using
re.escape(symbol) plus lookaround anchors such as (?<![\w$]) and (?![\w$]) so
the test matches the exact emitted clone name token rather than substrings).
🪄 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: 6fc375ca-6338-4753-9c1c-4a0c593323d1
📒 Files selected for processing (2)
TESTING.mdtests/test_codegen.py
…ecks The four `in`/`not in` substring assertions on mono-suffix strings in `TestGenericMonoSuffixFromSlotRef604` could pass on longer symbol variants (e.g. `$option_map$Int_Int_X` would satisfy `"$option_map$Int_Int" in wat`). No current code path produces longer variants, but the assertions weren't precise about what they were pinning. Replaced each substring check with a boundary-safe regex using a negative lookahead: `re.search(r"\$option_map\$<suffix>(?![A-Za-z0-9_])", wat)`. The pattern rejects any continuation character that would indicate a longer variant. `re` was already imported at top of file. Two test methods updated: - `test_mono_suffix_correct_for_slotref_fn_alias_arg` — positive `$option_map$Int_Int` + negative `$option_map$Int_Bool` - `test_parameterised_alias_substitutes_type_args` — positive `$option_map$Int_Int` + negative `$option_map$T_T` Validation: all 4 tests in `TestGenericMonoSuffixFromSlotRef604` pass against current compiler output. Refs #604 #655 #659 Co-Authored-By: Claude <noreply@anthropic.invalid>
….145 Now that v0.0.145 closes #604 + #655 Shape A in `da65a52`-`20a7498`, do a sweep across SKILL.md, KNOWN_ISSUES.md, the example workaround in `examples/closures.vera`, and the conformance suite to make sure no surface still describes the bug or works around it. SKILL.md (`§Limitations` table) Removed the "Five prelude combinators silently skipped from every WASM compile" row. All five combinators now work end-to-end: `option_map`, `option_and_then`, `result_map` had real runtime trap fixed by the SlotRef-FnType-alias mono inference; the two `_unwrap_or` variants were already working via mono and only emitted misleading template warnings, now suppressed by the post-compile pass in `compile_program`. The recommended workaround (`match` directly) is no longer needed. KNOWN_ISSUES.md Removed the `#604` Bugs-table row (closed by v0.0.145). Trimmed the `#655` row to just the `head` over `NonEmptyArray` codegen gap (Shape B); the 7 spurious user-code generic warnings (Shape A — `identity`, `const`, `is_some`, `are_equal`, `cmp_sign`) are no longer flagged by the gate post-suppression. Limitation count drops 36 → 35; cross-checked with `check_limitations_sync.py`. examples/closures.vera — workaround removed The example used to define a private non-generic `option_map(@option<Int>, @intmapper -> @option<Int>)` that *shadowed* the prelude's generic version specifically because the prelude version trapped pre-fix. The shadow was a workaround; removing it makes the example demonstrate the actual fix shape — a `SlotRef` typed with an `FnType` alias passed to the prelude generic. `test_option_map` now calls the prelude `option_map` directly with `@IntToInt.0` as the closure arg. Result is unchanged (returns 105 = 5+100). Conformance test: SlotRef-FnType-alias call shape (#604) Added a new `type IntMap = fn(Int -> Int)` alias plus a helper `map_with_slot_closure(@option<Int>, @intmap -> @option<Int>)` that calls `option_map(@option<Int>.0, @IntMap.0)` — exactly the call shape that pre-fix produced `option_map$Int_Bool` and trapped at runtime. New `test_map_some_via_slot` function added to the `main` integration sum. Conformance level stays at `run`; file runs cleanly returning 260 (= 42+11+10+77+99+21). tests/test_verifier.py — tier-count update `test_overall_tier_counts` trajectory comment + assertion bumped from `254 T1 / 26 T3 / 280 total` to `252 T1 / 26 T3 / 278 total`. The removed private `option_map` shadow in `examples/closures.vera` had a `requires(true) ensures(true)` pair contributing 2 T1 + 2 total that no longer appear. Validation - mypy clean - pytest: 3,795 passed, 14 skipped - conformance: all 86 programs pass - examples: all 34 pass `vera check` + `vera verify` - e602 gate: 116 files clean, 6 allowlist matched, 0 stale - limitations-sync, doc-counts, skill-examples, examples-readme: all consistent Spec chapters checked (`05-functions.md`, `07-effects.md`, `09-standard-library.md`): the prelude-combinator references all match current behaviour. The `option_map` example in §5.9.1 + §7.6 is an illustrative effect-polymorphism sketch, not a workaround. Refs #604 #655 #659 Co-Authored-By: Claude <noreply@anthropic.invalid>
Multi-agent review (code-reviewer + comment-analyzer + silent-failure-hunter + type-design-analyzer + pr-test-analyzer) of PR #659. No critical issues. Five important findings addressed here; one (F6) investigated and discarded as type-checker-rejected; F4 noted for follow-up. F1 (`scripts/check_e602_clean.py:93`) — code-reviewer Replaced the literal `PR #<TBD>` placeholder I left when removing the 5 user-code generic allowlist entries with the correct `PR #659`. Caught at the top of code-reviewer's findings. F2 (`vera/codegen/core.py:574`) — comment-analyzer The suppression-pass block comment used the shorthand "_compile_fn ([E602])" — `_compile_fn` lives in `functions.py`, not in `core.py` where this comment sits. Expanded the attribution to name both `compilability.py` (for `_is_compilable` emitting E604/E605) and `functions.py` (for the E602 emit sites reached via `_compile_fn`). F3 (`_resolve_arg_fn_shape` length-mismatch) — code-reviewer + silent-failure-hunter Reproduced a same-class bug as #604 via a different entry point: `type Pair<A, B> = fn(A -> B)` plus `@Pair<Int>.0` (1 type-arg supplied where 2 expected). The type checker accepts this silently; codegen falls through to the legacy path, the generic's type-vars stay unbound, and the mono mangler emits a literal alias-local name in the suffix (`option_map$Int_B`) producing a `call_indirect` to a non-existent function-table entry at runtime. Expanded inline comments in both `monomorphize.py::_resolve_arg_fn_shape` and `wasm/calls.py::_resolve_arg_fn_shape_wasm` documenting the latent gap; the actual fix belongs upstream in the type checker (separate follow-up will be filed). No behaviour change in this PR for that path — pre-fix it produced wrong mono suffixes; we preserve that behaviour rather than emit a misleading E699 "internal compiler error" since this is a type-checker bug, not a compiler invariant violation. F5 (`substitute_type_vars` FnType branch) — type-design + silent-failure-hunter The new `FnType` branch in `vera/wasm/inference.py::substitute_type_vars` passes the `effect` field through unchanged. Fine today (all parameterised FnType aliases use monomorphic effects like `effects(pure)`), but type-vars inside effects (e.g. `fn(A -> A) effects(<State<T>>)`) would NOT be substituted. Type-design-analyzer flagged this as a latent gap. Pinned the contract two ways: * Inline comment in the FnType branch documenting the deliberate non-substitution. * New `TestSubstituteTypeVarsFnType` class in `tests/test_wasm.py` with two unit tests: one positive (params + return ARE substituted), one contract-pin (effect with type-var is preserved verbatim). A future refactor that "fixes" the gap by walking effects must update the test (and the comment), making the contract change visible rather than silent. F8 (`tests/test_codegen.py::test_mono_suffix_correct_for_slotref_fn_alias_arg`) — pr-test-analyzer The existing test asserts the mono-clone function *definition* `(func $option_map$Int_Int ...)` appears in WAT. But the call-site rewriter in `vera/wasm/calls.py::_infer_fn_alias_type_args_wasm` is a parallel implementation that runs later and could independently mangle to a different name — a regression where Pass 1.5 produces the right clone but the rewriter mangles the call to a different name would pass the function-definition assertion and fail at WASM validation with `unknown function $option_map$<wrong>`. Added a third assertion using a boundary-safe regex matching `(?:return_)?call \$option_map\$Int_Int` to pin that the call-site rewriter and the monomorphizer agree on the mangled name. F6 (RefinementType wrapping FnType) — silent-failure-hunter Investigated: type checker rejects `option_map(opt, @SafeMap.0)` where `SafeMap = { @inner | pred }` with error "Argument 1 of 'option_map' has type {@fn(Int -> Int) effects(pure) | ...}, expected fn(Int -> B) effects(pure)". Refinement-wrapped FnType aliases cannot flow into generic combinator args. Not reachable; no fix needed. F4 (`compiled_mono_bases` cross-module name collision) — code-reviewer Pre-existing concern flagged for follow-up: if two modules both declare a `forall<T> fn map(...)`, the suppression set keys on the bare base name and could suppress both. Today's user-vs-prelude namespace separation makes this mostly theoretical; will file as a separate issue. Validation - mypy clean - pytest 3,797 passed, 14 skipped (was 3,795 + 2 new F5 tests) - e602 gate: 116 files clean, 6 allowlist matched, 0 stale - conformance + examples all pass - doc-counts consistent Refs #604 #655 #659 Co-Authored-By: Claude <noreply@anthropic.invalid>
This was referenced May 12, 2026
Owner
Author
Multi-agent review summary (round 2)Ran the TL;DR
Findings addressed in this PR (
|
| # | Finding | Source | What changed |
|---|---|---|---|
| F1 | scripts/check_e602_clean.py:93 had a literal PR #<TBD> placeholder |
code-reviewer (flagged as important) | Replaced with PR #659 |
| F2 | vera/codegen/core.py:574 comment said _compile_fn ([E602]) but _compile_fn lives in functions.py, not core.py |
comment-analyzer | Expanded attribution to name both compilability.py (E604/E605) and functions.py (E602) explicitly |
| F3 | _resolve_arg_fn_shape length-mismatch fall-through reproduces the same bug class as #604 via a different entry point |
code-reviewer + silent-failure-hunter | Added defensive inline comments in both monomorphize.py + wasm/calls.py documenting the latent gap. Real fix belongs in the type checker — filed as #660 |
| F5 | substitute_type_vars FnType branch preserves effect field unchanged — latent gap for effects carrying type-vars |
type-design + silent-failure-hunter | Inline comment + 2 new unit tests TestSubstituteTypeVarsFnType in tests/test_wasm.py pinning the deliberate non-substitution contract |
| F8 | The mono-suffix test verified the function definition in WAT but not the call-site call instruction; the WASM-side rewriter is a parallel implementation that could mangle differently |
pr-test-analyzer | Added a third boundary-safe regex assertion (?:return_)?call \$option_map\$Int_Int to pin the call-site rewriter agrees with the monomorphizer |
Findings filed as follow-ups
| # | Issue | Severity | Disposition |
|---|---|---|---|
| F3 (root cause) | #660 — Type checker accepts wrong-arity SlotRef of parameterised FnType alias | Important | Type-checker-side fix; codegen workaround documented |
| F4 | #661 — compiled_mono_bases cross-module name collision in template-warning suppression |
Defensive | Failure mode is benign (silenced warning, no runtime bug); user-vs-prelude separation today makes this mostly theoretical |
Finding investigated + discarded
F6 (silent-failure-hunter) — Refinement-wrapped FnType aliases passed to generic combinators. Tested: type checker rejects option_map(opt, @SafeMap.0) where SafeMap = { @Inner | pred } with Argument 1 of 'option_map' has type {@fn(Int -> Int) effects(pure) | ...}, expected fn(Int -> B) effects(pure). Not reachable from user code; no fix needed.
Type-design suggestions deferred to polish follow-up
- Refactor
_resolve_arg_fn_shapeto a free function shared between mono.py and wasm/calls.py (eliminates duplication +getattrvs direct-attribute drift) - Introduce
@dataclass(frozen=True) class CallableShapeinstead of the ad-hoctuple[tuple[ast.TypeExpr, ...], ast.TypeExpr] | Nonereturn type - Browser-parity coverage for
examples/closures.vera::test_option_map
Validation
- mypy clean (59 source files)
- pytest 3,797 passed, 14 skipped (was 3,795 + 2 new F5 unit tests)
- conformance: all 86 programs pass
- examples: all 34 pass
vera check+vera verify - e602 gate: 116 files clean, 6 allowlist matched, 0 stale
- doc-counts, limitations-sync: consistent
Ready for merge once CI on 3f0b5b9 lands green.
This was referenced May 12, 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
SlotRefargs typed as FnType aliases — the root cause behind Five prelude combinators silently skipped from every WASM compile (option_map / option_and_then / result_map + two _unwrap_or variants) #604 (5 prelude combinators emitting spurious warnings, 3 of them trapping at runtime) and Layer-1 #626 surfaced 8 silent-skip cases: 7 spurious generic-decl warnings + 1 real codegen gap (head over refinement-of-Array) #655 Shape A (5 user-code generics emitting spurious warnings).[E602]/[E604]/[E605]warnings onforall<T>decls whose mono clones successfully compile — audit recommendation 2 from the Five prelude combinators silently skipped from every WASM compile (option_map / option_and_then / result_map + two _unwrap_or variants) #604 investigation comment.docs/index.htmltag,CLAUDE.mdStage-agnostic phrasing fix, KNOWN_ISSUES Nested type aliases (Array<Array<Foo>>) cause silent codegen skip; WAT error misleadingly points at callers #559 row.Closes #604, #655.
Root cause (one paragraph)
_unify_param_arginvera/codegen/monomorphize.pyhad anAnonFn-specific alias-resolution path at line 335. When a generic call's closure argument was aSlotReftyped as an FnType alias (e.g.@Doubler.0wheretype Doubler = fn(Int -> Int)), that path was skipped. The fall-through_get_arg_type_infocouldn't bind the alias's type variables to the param type's positional args because of a name mismatch ("Doubler" != "OptionMapFn"). The unbound type var then hit the"Bool"phantom-var fallback at result-building time (line 300), producing mono suffixes likeoption_map$Int_Boolinstead ofoption_map$Int_Int. At runtime thecall_indirectinside the wrong-suffix mono clone expected ani32(Bool) return; the user's closure returnedi64(Int) →wasm trap: indirect call type mismatch.Fix shape
Factored the AnonFn / SlotRef-alias dichotomy into a shape-resolver helper
_resolve_arg_fn_shapethat returns(param_types, return_type)for either kind of "callable arg"._infer_fn_alias_type_argsoperates on that shape uniformly. Purely additive: existing AnonFn behavior unchanged, SlotRef-alias case starts binding correctly. Same parallel fix invera/wasm/calls.py::_infer_fn_alias_type_args_wasm.Template-warning suppression
Post-compile pass in
vera/codegen/core.py::compile_programdrops[E602]/[E604]/[E605]template-only warnings on forall decls whose mono clones successfully compile (appear in_fn_sigswith a$<types>suffix). Preserves observability for genuinely-broken or unused user generics — warning still fires for forall decls with zero successful mono clones.Allowlist cleanup
scripts/check_e602_clean.pyallowlist shrinks 11 → 6 entries:identity,const,is_some,are_equal,cmp_sign) — their mono clones compile in every test program that uses them, triggering the new suppression.head(genuine codegen gap for refinement-of-Array, unrelated to this fix; tracked under Layer-1 #626 surfaced 8 silent-skip cases: 7 spurious generic-decl warnings + 1 real codegen gap (head over refinement-of-Array) #655 Shape B).Regression tests
TestGenericMonoSuffixFromSlotRef604class with two tests:test_mono_suffix_correct_for_slotref_fn_alias_arg— asserts WAT contains$option_map$Int_Int(not$option_map$Int_Bool) when called with a SlotRef typed as an FnType alias.test_template_warning_suppressed_when_mono_clone_compiles— asserts no[E602]/[E604]foroption_map/option_unwrap_orin a program that calls them successfully.Both fail against the pre-fix codepath.
Not closed by this fix
type Grid = Array<Array<Foo>>): separate codegen-side alias-resolution gap. Reproducer still trips a different error (malformed WAT instead of unknown-func).KNOWN_ISSUES.mdrow added for tracking (was missing pre-Layer 3 of #626: CodegenSkip infrastructure + convert 104 SILENT_SKIPs #658).headover refinement-of-Array): genuine codegen gap, not a monomorphization issue. Allowlist entry retained.Release prep included in this PR
Per agreed policy (no separate cleanup-only PRs after the v0.0.144 → #651 recovery):
vera/__init__.py+pyproject.toml+uv.lock: 0.0.144 → 0.0.145CHANGELOG.md: date-stamped[0.0.145] - 2026-05-11; new[Unreleased]; updated link refsHISTORY.mdStage 12 table: v0.0.145 rowdocs/index.html: version tag manual updateREADME.md/TESTING.md/ROADMAP.md: count refreshesCLAUDE.md: Stage-9 → stage-agnostic phrasing fixKNOWN_ISSUES.md: new Bugs row for Nested type aliases (Array<Array<Foo>>) cause silent codegen skip; WAT error misleadingly points at callers #559Test plan
vera check+vera verify🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Improvements
Testing
Documentation
Chores