Skip to content

v0.0.140 — String-returning FnCall in interpolation (#602)#627

Merged
aallan merged 5 commits intomainfrom
claude/string-interp-fncall-602
May 8, 2026
Merged

v0.0.140 — String-returning FnCall in interpolation (#602)#627
aallan merged 5 commits intomainfrom
claude/string-interp-fncall-602

Conversation

@aallan
Copy link
Copy Markdown
Owner

@aallan aallan commented May 8, 2026

Summary

Closes #602IO.print("\(make())") where make returns String produced expected i64, found i32 at WASM validation.

Same inference-gap shape as #614 (closed yesterday): #614 was the element-type of an indexed FnCall result; #602 is the return-type inference half. Both gaps are now closed via the _fn_ret_type_exprs registry that #614 introduced.

Bumps version v0.0.139 → v0.0.140.

Also files #626 as a cross-cutting tracking issue for the "translate returns None → silent skip" failure-shape pattern — observed across #583, #604, #614, and #615 (closure case). Adds it to the ROADMAP stabilisation tier alongside the renumbered remaining items.

Root cause

_infer_fncall_vera_type in vera/wasm/inference.py:774-782 maps user-fn WAT return types back to Vera-type names:

ret_wt = self._fn_ret_types.get(call.name)
if ret_wt == "i64":   return "Int"
if ret_wt == "i32":   return "Bool"
if ret_wt == "f64":   return "Float64"
return None  # ← i32_pair (String / Array<T>) fell through here

For a fn returning String (WAT type i32_pair), this returns None. Then _translate_interpolated_string consults the result:

vera_type = self._infer_vera_type(p)
if vera_type == "String":
    parts.append(p)
elif vera_type in self._INTERP_TO_STRING:
    parts.append(ast.FnCall(name=self._INTERP_TO_STRING[vera_type], args=(p,)))
else:  # pragma: no cover  ← here
    parts.append(ast.FnCall(name="to_string", args=(p,)))

The # pragma: no cover was lying — #602 reaches it. to_string(make_fncall) desugars to a call that reads its arg as i64, but the FnCall pushed i32_pair — hence the type mismatch.

Fix

Extend the WAT-type → Vera-type fallback to consult _fn_ret_type_exprs (the registry added by #614) when the WAT type is i32_pair:

if ret_wt == "i32_pair":
    ret_te = self._fn_ret_type_exprs.get(call.name)
    if isinstance(ret_te, ast.NamedType):
        return ret_te.name

Same registry, same pattern, same load-bearing infrastructure paying off twice.

Regression tests

Two new tests in TestStringInterpolation:

ROADMAP changes

Test plan

  • Minimum String-returning function call in string interpolation produces invalid WASM (i64/i32 mismatch) #602 repro — was expected i64, found i32, now prints hello
  • Sibling Array<T> interpolation case — runs and prints 10
  • pytest tests/ -q -n auto — 3,761 passed, 14 skipped (was 3,759; +2 new)
  • mypy vera/ — clean
  • python scripts/check_doc_counts.py — consistent (3,775 / 34 / 86)
  • python scripts/check_examples.py — 34 examples pass check + verify
  • python scripts/check_conformance.py — 86 programs pass
  • python scripts/check_version_sync.py — v0.0.140 across 6 files
  • python scripts/check_site_assets.py — up to date

Closes #602

Summary by CodeRabbit

  • Bug Fixes

    • Fixed type inference for function-call results used in string interpolation, preventing invalid WASM validation for string/array returns.
  • Tests

    • Added E2E regression tests for interpolated function calls (string, array, alias) and a structural test to ensure no incorrect conversion is inserted.
  • Chores

    • Bumped release to v0.0.140 and updated package version metadata.
  • Documentation

    • Updated changelog, history, README, ROADMAP, TESTING and KNOWN_ISSUES for the new release.

`IO.print("\(make())")` where `make` returns `String` produced
`expected i64, found i32` at WASM validation.  Root cause:
`_infer_fncall_vera_type` mapped user-fn WAT return types back to
Vera-type names for the i64 / i32 / f64 cases but had no `i32_pair`
branch — a fn returning String mapped to None.
`_translate_interpolated_string` then fell through to the
`to_string(...)` Int-conversion fallback wrapper, which reads its
arg as i64 — but the FnCall pushed i32_pair.  Same inference-gap
shape as #614 (which was the *element-type* of an indexed FnCall
result; this is the *return-type* inference half).

Fix: extend the WAT-type → Vera-type fallback to consult
`_fn_ret_type_exprs` (the registry added by #614) when the WAT
type is `i32_pair`, so String and Array<T> returns are
disambiguated.  Same registry, same pattern.

Two new tests in TestStringInterpolation cover the String- and
Array-returning FnCall cases (the Array case also exercises
the IndexExpr-of-FnCall path that #614 fixed — both halves of the
inference path together).

Also files #626 as a tracking issue for the cross-cutting
"translate returns None → silent skip" failure-shape pattern
observed across #583, #604, #614, #615.  Promotes #626 onto the
ROADMAP stabilisation tier alongside the renumbered remaining
items.

Test count 3,773 → 3,775 (+2).  Closes #602.

Closes #602

Co-Authored-By: Claude <noreply@anthropic.invalid>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR releases Vera v0.0.140. It fixes issue #602 by resolving i32_pair FnCall return inference via the _fn_ret_type_exprs registry, adds runtime and structural regression tests ensuring interpolated FnCall expressions no longer trigger incorrect to_string wrapping, and updates version and release documentation/metrics.

Changes

Vera v0.0.140 Release

Layer / File(s) Summary
Type Inference Fix
vera/wasm/inference.py
_infer_fncall_vera_type now handles ret_wt == "i32_pair" by consulting _fn_ret_type_exprs (monomorphized and non-generic paths) and returning a NamedType name when present.
Operators fallback comment
vera/wasm/operators.py
Expanded comment in the interpolated-string fallback branch clarifying why the branch is reachable; no logic change.
Regression Tests (runtime)
tests/test_codegen.py
Adds runtime tests: interpolating a String-returning FnCall, interpolating an Array<Int>-returning FnCall with indexing, and alias-resolved String-returning function case.
Regression Test (structural)
tests/test_codegen.py
Adds a structural test that compiles an interpolation example and asserts call $to_string is not present in main's WAT.
Version Updates
pyproject.toml, vera/__init__.py
Package version incremented from 0.0.139 to 0.0.140.
Release Notes
CHANGELOG.md, HISTORY.md
New 0.0.140 changelog entry and compare links; HISTORY.md records v0.0.140 stage release and increments total tagged releases.
Project Metrics & Roadmap
README.md, ROADMAP.md, TESTING.md, KNOWN_ISSUES.md
Project test counts and metrics updated (tests total → 3,777); ROADMAP stabilisation tier reordered (remove #602, reorder #604/#626); KNOWN_ISSUES removed #602 and added #628.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • aallan/vera#625: Related propagation of _fn_ret_type_exprs into wasm inference and WasmContext.
  • aallan/vera#454: Earlier changes touching i32_pair/NamedType return handling in wasm inference.

Suggested labels

compiler, tests, docs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main fix: addressing String-returning FnCall in interpolation for v0.0.140, directly mapping to issue #602.
Linked Issues check ✅ Passed All coding requirements from #602 are fully addressed: _infer_fncall_vera_type now consults _fn_ret_type_exprs for i32_pair, resolves type aliases, and prevents incorrect to_string wrapping; comprehensive regression tests validate the fix.
Out of Scope Changes check ✅ Passed All changes are scoped to #602: version bumps (pyproject.toml, vera/init.py), documentation updates reflecting the fix and test-count increments, and the core inference/interpolation fix with regression coverage.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/string-interp-fncall-602

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

@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

❌ Patch coverage is 60.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.93%. Comparing base (7624b09) to head (f7fd287).

Files with missing lines Patch % Lines
vera/wasm/inference.py 50.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #627      +/-   ##
==========================================
- Coverage   90.94%   90.93%   -0.01%     
==========================================
  Files          59       59              
  Lines       23024    23033       +9     
  Branches      259      259              
==========================================
+ Hits        20940    20946       +6     
- Misses       2077     2080       +3     
  Partials        7        7              
Flag Coverage Δ
javascript 57.36% <ø> (ø)
python 94.74% <60.00%> (-0.02%) ⬇️

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

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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)

328-328: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update tagged release count from 139 to 140.

The footer still shows "139 tagged releases" but v0.0.140 is now documented in the Stage 12 table. Per project conventions, this count must stay consistent across HISTORY.md (table + footer) and ROADMAP.md. Based on learnings from prior reviews, all three locations should be incremented together.

Proposed fix
-Total: **810+ commits, 139 tagged releases, 56 active development days.**
+Total: **810+ commits, 140 tagged releases, 56 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 328, Update the footer text "Total: **810+ commits, 139
tagged releases, 56 active development days.**" to reflect the new release count
(change 139 to 140) and ensure the same numeric change is made consistently in
the Stage 12 table entry and in ROADMAP.md where the tagged-release count is
shown; search for the literal "139 tagged releases" and update all occurrences
to "140 tagged releases" so the table, footer, and roadmap remain consistent.
🤖 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`:
- Line 328: Update the footer text "Total: **810+ commits, 139 tagged releases,
56 active development days.**" to reflect the new release count (change 139 to
140) and ensure the same numeric change is made consistently in the Stage 12
table entry and in ROADMAP.md where the tagged-release count is shown; search
for the literal "139 tagged releases" and update all occurrences to "140 tagged
releases" so the table, footer, and roadmap remain consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5a6c9e94-d9e0-4741-8f36-2392468d8aa1

📥 Commits

Reviewing files that changed from the base of the PR and between 7624b09 and f76302c.

⛔ Files ignored due to path filters (5)
  • docs/index.html is excluded by !docs/**
  • docs/index.md is excluded by !docs/**
  • docs/llms-full.txt is excluded by !docs/**
  • docs/llms.txt is excluded by !docs/**
  • uv.lock is excluded by !**/*.lock, !uv.lock
📒 Files selected for processing (9)
  • CHANGELOG.md
  • HISTORY.md
  • README.md
  • ROADMAP.md
  • TESTING.md
  • pyproject.toml
  • tests/test_codegen.py
  • vera/__init__.py
  • vera/wasm/inference.py

@aallan
Copy link
Copy Markdown
Owner Author

aallan commented May 8, 2026

@coderabbitai Fixed in b9eb3e5.

Verified the only occurrence of "139 tagged releases" was the HISTORY.md footer at line 328 — grep -rn "139 tagged releases" . returned a single hit. Updated to 140.

The "Stage 12 table entry" and "ROADMAP.md" mirroring requested in the comment didn't have parallel content:

  • The Stage 12 table in HISTORY.md tracks per-release entries (v0.0.139, v0.0.140) as line items, not an aggregate tagged-release count. Nothing to update.
  • ROADMAP.md has only 3,775 tests, 86 conformance programs, 34 examples, 13 spec chapters in its "Where we are" line — the aggregate tagged-release count was removed by the previous ROADMAP trim (PR ROADMAP refactor + Tetris experiment: 2 codegen bugs + design-reviewed friction items #613) and isn't currently in that file. Nothing to update.

Same shape as your earlier "make the corresponding change in ROADMAP" finding on PR #625's HISTORY footer fix — concrete observation correct, mirroring suggestion overbroad.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Tip

For best results, initiate chat on the files or code changes.

@aallan Thanks for verifying and fixing — confirmed that the sole occurrence was the HISTORY.md footer and the mirroring suggestions to the Stage 12 table and ROADMAP.md were overbroad. Noted on the pattern: the Stage 12 table is a per-release item list (not an aggregate count), and the aggregate tagged-release count was already removed from ROADMAP.md in PR #613, so there's nothing to mirror there. I'll stop making those suggestions.


✏️ Learnings added
Learnt from: aallan
URL: https://github.com/aallan/vera/pull/627

Timestamp: 2026-05-08T17:18:57.730Z
Learning: In the `aallan/vera` repository (Python → WebAssembly compiler), HISTORY.md has a Stage 12 table that tracks per-release line items (e.g., `v0.0.139`, `v0.0.140`) — it does NOT contain an aggregate tagged-release count. Do not suggest updating it when a tagged-release count is incremented elsewhere.

Learnt from: aallan
URL: https://github.com/aallan/vera/pull/627

Timestamp: 2026-05-08T17:18:57.730Z
Learning: In the `aallan/vera` repository, ROADMAP.md no longer contains an aggregate tagged-release count (it was removed in PR `#613`). Its "Where we are" line only contains `tests`, `conformance programs`, `examples`, and `spec chapters` counts. Do not suggest mirroring tagged-release count changes there.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

…test + pragma fix

Three convergent findings from the four review agents on PR #627:

1. Generic-branch i32_pair gap (flagged by code-reviewer,
   pr-test-analyzer, silent-failure-hunter).  The non-generic
   branch in `_infer_fncall_vera_type` got the new `i32_pair` case;
   the *generic* branch above (lines 750-773) didn't — same
   inference gap for monomorphised generic fns returning
   `String`/`Array<T>`.  Mirrored the fix one block above.

2. `# pragma: no cover` on `operators.py:482-486` (flagged by
   pr-test-analyzer, comment-analyzer, silent-failure-hunter).
   The pragma asserted unreachability that #602 disproved.
   Removed pragma + replaced misleading "Int-compatible types"
   comment with an honest explanation tying the branch to #626's
   broader silent-skip tracking.

3. Structural assertion test (flagged by pr-test-analyzer,
   silent-failure-hunter).  Added
   `test_no_to_string_wrap_on_string_returning_fncall_602` —
   asserts the WAT for `main` doesn't contain `call $to_string`
   when interpolating a String-returning FnCall.  Locks the fix
   at the codegen layer rather than only at runtime output.

Skipped: cross-module imports gap (filed as #628 — same shape
exists in #614's shipped fix, deserves its own scoped issue
rather than scope creep here); CHANGELOG/comment-style trims
(taste, no functional issue).

Test count 3,775 → 3,776 (+1).

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

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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 `@tests/test_codegen.py`:
- Around line 8190-8238: Add a regression test that exercises the monomorphised
generic i32_pair path by creating a small generic identity function (id<T>) and
using it in a string interpolation where T is instantiated as `@String`; mirror
the existing style of test_string_returning_fncall_inside_interpolation_602 and
test_array_returning_fncall_indexed_inside_interpolation by defining private
generic fn id<T>(...) and a public main that IO.prints the interpolation of
id(make(())) (or equivalent instantiation) and assert the output is "hello\n" so
the generic branch producing i32_pair is exercised.

In `@vera/wasm/inference.py`:
- Around line 773-784: The code returns ret_te.name directly for i32_pair
handling (when ret_wt == "i32_pair" using self._fn_ret_type_exprs.get(mangled)
and ast.NamedType), which leaves type aliases (e.g., type Str = String)
unresolved; change the logic to canonicalise/resolve NamedType aliases before
returning by looking up the named type in your type-alias/typedef table or
calling the project’s type-normalisation helper (e.g., a _resolve_named_type or
similar utility) so you return the underlying canonical Vera name (not the
alias) whenever ret_te is an ast.NamedType; apply the same fix to the other
identical branch that inspects _fn_ret_type_exprs for monomorphised names.
🪄 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: e1e6f4f4-2180-43b5-ac18-53c3cfb8eacd

📥 Commits

Reviewing files that changed from the base of the PR and between b9eb3e5 and d78b4dc.

📒 Files selected for processing (6)
  • README.md
  • ROADMAP.md
  • TESTING.md
  • tests/test_codegen.py
  • vera/wasm/inference.py
  • vera/wasm/operators.py

Comment thread tests/test_codegen.py
Comment thread vera/wasm/inference.py Outdated
@aallan
Copy link
Copy Markdown
Owner Author

aallan commented May 8, 2026

Self-review via /pr-review-toolkit:review-pr — findings and actions

Ran the four-agent review toolkit (code-reviewer, pr-test-analyzer, comment-analyzer, silent-failure-hunter) before requesting human/CodeRabbit review. Three convergent findings (flagged independently by ≥2 agents) were addressed in commit d78b4dc; one deferred to a follow-up issue; two skipped on taste.

Critical issues fixed (commit d78b4dc)

# Finding Flagged by Action
1 Generic-branch i32_pair gap in _infer_fncall_vera_type (vera/wasm/inference.py:759-773). The non-generic branch got the new i32_pair case; the parallel generic-fn branch above didn't — same inference gap for monomorphised generic fns returning String / Array<T> (e.g. forall<T> fn id(@T -> @T) instantiated at String). code-reviewer, pr-test-analyzer, silent-failure-hunter (3 of 4) Mirrored the i32_pair branch one block above; comment-block links the symmetry to #602's failure mode.
2 # pragma: no cover on vera/wasm/operators.py:482-486 claimed unreachability that #602 proved false. The comment "Fallback: try to_string (Int-compatible types)" was also factually wrong — the branch fires for unknown types, not Int-compatible ones. pr-test-analyzer, comment-analyzer (critical), silent-failure-hunter (critical) Removed pragma; replaced misleading comment with honest documentation tying the branch to #626 (the broader silent-skip pattern).
3 No structural assertion on WAT shape — both new tests asserted only stdout. A regression that produced different bad WAT but coincidentally correct output would pass. pr-test-analyzer, silent-failure-hunter (high) Added test_no_to_string_wrap_on_string_returning_fncall_602 — asserts main's WAT doesn't contain call $to_string when interpolating a String-returning FnCall. Locks the fix at the codegen layer rather than only at runtime.

Important — deferred to follow-up

Skipped (taste, not functional)

  • CHANGELOG "Same registry, same pattern, same load-bearing infrastructure paying off twice" reads as marketing-speak (comment-analyzer). Functional content above it is solid; the line stays.
  • inference.py parenthetical (see #614 — same registry, same pattern) could be plainer (comment-analyzer). Minor wording, not worth a churn commit.

Strengths confirmed by the review

  • _register_fn is called for monomorphised clones at core.py:333, so the generic-call-site path with non-i32_pair type args was already correct.
  • Built-ins returning i32_pair (string_concat, int_to_string, etc.) are handled by name in earlier branches of _infer_fncall_vera_type (lines 232-690), so the new line-794 fallback never sees them.
  • All issue/PR cross-references valid. CHANGELOG / HISTORY / ROADMAP / version sync all consistent.

Test count

3,773 → 3,776 (+3 new in this PR: original behavioural test + Array-via-IndexExpr + the new structural assertion).

aallan and others added 2 commits May 8, 2026 18:48
…test

CodeRabbit raised two findings on PR #627 post-review-toolkit:

VALID: type aliases over `String` (e.g. `type Str = String;
fn make(-> @str)`) reproduce the original #602 bug.  My initial
i32_pair fix returned `ret_te.name` from `_fn_ret_type_exprs` —
which stores the *declared* TypeExpr `NamedType("Str")`, not the
resolved `NamedType("String")`.  Downstream consumer checks
`vera_type == "String"` and the conversion-map; both miss "Str";
falls through to `to_string(...)` wrapper → original i64/i32 trap
with a different trigger.

Verified reproducer:
  type Str = String;
  private fn make(-> @str) requires(true) ensures(true) effects(pure)
  { "hello" }
  public fn main(-> @Unit) requires(true) ensures(true) effects(<IO>)
  { IO.print("\(make(()))\n") }

Pre-fix: same trap as #602.  Post-fix: prints "hello".

Fix: resolve aliases via the existing `_resolve_base_type_name`
helper (vera/wasm/inference.py:1167) before returning from both
`i32_pair` branches (non-generic and generic).  New regression
test `test_type_alias_string_in_interpolation` documents the
shape and ties it to the fix.

SKIP: generic-id i32_pair regression test.  Verified the shape is
unreachable today — `private forall<T> fn id(@t -> @t)`
type-checks but `vera run` reports "Function 'id' has unsupported
parameter type — skipped" (same backend gap as #604's `_unwrap_or`
combinators: bare type-var params can't be lowered).  My
generic-branch i32_pair fix in commit d78b4dc is latent
infrastructure for when that prior gap closes.  Updated the
in-source comment to flag this explicitly.

Test count 3,776 → 3,777 (+1).

Co-Authored-By: Claude <noreply@anthropic.invalid>
Two missed updates from PR #627 (caught when prepping rabbit reply):

1. #602 entry should have been deleted in this PR per the
   release-workflow rule ("when a PR closes an issue, the same PR
   removes its row from KNOWN_ISSUES").  ROADMAP cleanup was done
   correctly; KNOWN_ISSUES was not.

2. #628 (cross-module gap, filed during agent-review pass) was
   missed entirely from KNOWN_ISSUES per the new-issue-tracking
   rule ("PR-generated new issues must be added to ROADMAP.md or
   KNOWN_ISSUES.md in the same PR").  Now added to the Bugs
   section with a brief description and a pointer to the issue
   body's fix sketch.

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

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
vera/wasm/inference.py (1)

785-793: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle RefinementType in both new i32_pair branches.

At Line 786 and Line 815, the new lookup only accepts ast.NamedType. If _fn_ret_type_exprs carries ast.RefinementType (e.g., refined String return), this still falls through to None and can hit the same downstream interpolation misclassification path.

🔧 Proposed fix
             if ret_wt == "i32_pair":
                 ret_te = self._fn_ret_type_exprs.get(mangled)
+                if isinstance(ret_te, ast.RefinementType):
+                    ret_te = ret_te.base_type
                 if isinstance(ret_te, ast.NamedType):
                     # Resolve type aliases (`type Str = String`) so
                     # the downstream consumer sees the canonical name
                     # — see the matching branch below for the bug
                     # this prevents.
                     return self._resolve_base_type_name(ret_te.name)
@@
         if ret_wt == "i32_pair":
             ret_te = self._fn_ret_type_exprs.get(call.name)
+            if isinstance(ret_te, ast.RefinementType):
+                ret_te = ret_te.base_type
             if isinstance(ret_te, ast.NamedType):
                 # Resolve type aliases through `_resolve_base_type_name`
                 # so that `type Str = String; fn make() -> `@Str`` returns
                 # `"String"` here, not `"Str"`.  Without this, a fn
                 # whose declared return type is an alias would still

Also applies to: 815-830

🤖 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/inference.py` around lines 785 - 793, The branch handling ret_wt ==
"i32_pair" only treats ret_te as ast.NamedType and returns None for
ast.RefinementType; update the logic in the ret_wt == "i32_pair" branches (where
ret_te is fetched from self._fn_ret_type_exprs via mangled) to also accept
ast.RefinementType by unwrapping its base/inner named type (or resolving its
.type / .base name) and then calling self._resolve_base_type_name on that name,
so both ast.NamedType and ast.RefinementType return the canonical base type
instead of falling through to None (adjust the same change in the second branch
around the other i32_pair handling).
🤖 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.

Duplicate comments:
In `@vera/wasm/inference.py`:
- Around line 785-793: The branch handling ret_wt == "i32_pair" only treats
ret_te as ast.NamedType and returns None for ast.RefinementType; update the
logic in the ret_wt == "i32_pair" branches (where ret_te is fetched from
self._fn_ret_type_exprs via mangled) to also accept ast.RefinementType by
unwrapping its base/inner named type (or resolving its .type / .base name) and
then calling self._resolve_base_type_name on that name, so both ast.NamedType
and ast.RefinementType return the canonical base type instead of falling through
to None (adjust the same change in the second branch around the other i32_pair
handling).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 953368fc-017a-44f5-9ef6-3c1f1c610ecf

📥 Commits

Reviewing files that changed from the base of the PR and between d78b4dc and f7fd287.

📒 Files selected for processing (6)
  • KNOWN_ISSUES.md
  • README.md
  • ROADMAP.md
  • TESTING.md
  • tests/test_codegen.py
  • vera/wasm/inference.py

@aallan aallan merged commit 13f8d58 into main May 8, 2026
20 checks passed
@aallan aallan deleted the claude/string-interp-fncall-602 branch May 8, 2026 18:02
aallan added a commit that referenced this pull request May 8, 2026
…test

Four agents reviewed PR #629; the silent-failure-hunter agent
identified a CRITICAL gap that the other three missed: nested
RefinementType (`@{ @{ @string | p1 } | p2 }`) is reachable per
the grammar AND reproduces the original #602 trap.  Empirically
verified — `vera check` passes, `vera run` traps at WASM
validation with the same `expected i64, found i32` signature.

This PR's initial fix used `if isinstance(ret_te, ast.RefinementType):
base = ret_te.base_type` — only one level of unwrap.  Nested
refinements fell through to None, downstream wraps with
`to_string(...)`, same #602 trap.  Fifth trigger of the same bug
class.

Fix: replaced the one-level unwrap with a `while isinstance(ret_te,
ast.RefinementType): ret_te = ret_te.base_type` loop in both call
sites (`_resolve_i32_pair_ret_te` and the parallel IndexExpr-of-
FnCall path).  Handles arbitrary nesting depth.

Also addressed:
- comment-analyzer: `core.py:333` ambiguity → `codegen/core.py:333`
- comment-analyzer + silent-failure-hunter: added #626 reference
  to the helper's docstring tying its None-return paths to the
  broader silent-skip tracking issue
- code-reviewer + comment-analyzer: justified the helper-vs-inline
  choice in the IndexExpr branch (helper returns `str`, the branch
  needs the `NamedType` AST node for `_alias_array_element`)

Two new regression tests in TestStringInterpolation:
- test_nested_refinement_string_in_interpolation
- test_refinement_over_type_alias_in_interpolation (sibling case
  that already worked but was previously untested — pins the
  alias-recursion behaviour against future regressions)

Skipped (with reason in the PR thread):
- Cross-module gap reproducing the same trap — that's #628,
  explicitly deferred when filed during PR #627's review
- Structural-WAT test for RefinementType — duplicates the
  existing PR #627 structural test which would catch any
  inference regression; nice-to-have but redundant
- Logging at None-return sites — broader change that lives in
  #626's Layer 1/2/3 work
- Parameterised cross-product test enumerating all shapes ×
  all targets — over-scoped for this PR

Test count 3,779 → 3,781 (+2).

Co-Authored-By: Claude <noreply@anthropic.invalid>
aallan added a commit that referenced this pull request May 8, 2026
…ents + CodeRabbit

Four review agents (code-reviewer, pr-test-analyzer, comment-analyzer,
silent-failure-hunter) plus three CodeRabbit comments produced a
batch of cross-cutting findings.  This commit addresses them all
in one pass to avoid the iterative round-trip cost the bug class
itself was filed to eliminate.

**Critical fixes:**

1. **Closure body E615 harvest gap (silent-failure-hunter C1).**
   `_compile_lifted_closure` constructed its own WasmContext but
   never harvested `_interp_inference_failures` — closure bodies
   with E615-triggering interpolation segments were silently
   dropped, producing call_indirect→missing-table-entry WASM
   validation traps with no source-located diagnostic.  Extracted
   the harvest into `CodeGenerator._harvest_interp_inference_failures`
   on the codegen base and called it from both functions.py and
   closures.py.  Removed the now-disproved `# pragma: no cover —
   defensive` claim on closures.py's body-instrs-None path.  New
   regression test `test_e615_in_closure_body_emits_diagnostic`.

2. **`_canonical_named_type` type_args from terminal NamedType
   (CodeRabbit #3 + code-reviewer I1).**  The walker's
   `outer_type_args` capture rule (always read from first NamedType)
   lost type_args when alias_map substitution bound a generic
   param to a parameterised type.  Empirically reachable today via
   `type Mapper<A, B> = fn(A -> B); Mapper<Int, Array<Int>>` etc.
   Fix: drop the "outermost" rule, return type_args from the
   terminal NamedType reached.  All 21 existing #602-class
   regression tests still pass.  New regression test
   `test_canonical_named_type_terminal_args_propagation`.

3. **HISTORY.md footer (CodeRabbit #1).**  141 tagged releases →
   142 to match the v0.0.142 row added in PR #631.

**UX fix:**

4. **Multiple E615 per function (silent-failure-hunter H2).**
   Pre-this-fix `_translate_interpolated_string` returned None on
   the first failing segment, requiring N round-trips for N bad
   segments in one interpolation.  Now collects every failing
   segment via `had_failure` flag, then bails at the end.  New
   regression test `test_multiple_e615_in_one_interpolation`.

**Doc-quality fixes (comment-analyzer):**

5. **Trigger-count drift** in `operators.py:491-494` —
   "every canonicalisation gap (#602 and ten subsequent triggers)"
   implied 11; rewrote to "the ten triggers of the #602 bug class
   accumulated across PRs #627 + #629".

6. **`_format_named_type_canonical` docstring drift** — claim
   "matches the pre-#630 fallback shape" was empirically wrong
   (pre-#630 resolved through aliases, post-#630 fallback doesn't).
   Rewrote to acknowledge the deliberate behavioural change.

7. **"Future closure-arg shapes plug in here without further
   dispatch ladder"** — overstatement.  Adding a `FnCall`
   returning a closure still needs an `elif`; what's saved is
   canonicalisation logic, not dispatch logic.  `IfExpr` between
   closures has no single `return_type` field — example was
   misleading.  Rewrote in both `_infer_fncall_vera_type` and
   `_infer_apply_fn_return_type`.

8. **`_canonical_wasm_type` docstring** — added explicit `Unit →
   None` documentation; clarified that `None` (no WASM type) and
   `"i64"` (unreachable-NamedType default) are distinct return
   shapes that callers must not conflate.

**Tests added (besides the regressions tied to fixes):**

9. **`test_per_function_isolation_of_failures_list`** — pins
   that a clean function and a dirty function in the same source
   produce independent `_interp_inference_failures` lists.

10. **`test_e615_fires_on_result_in_interpolation`** — adjacent
    ADT shape (Result vs Option) so a future Option-handling
    broadening doesn't accidentally regress the parallel Result
    path.

11. **`test_array_map_refinement_returning_closure`** — pins the
    previously-unaudited `_infer_closure_return_vera_type` path
    in `calls_arrays.py`, which the #630 migration broadened to
    handle refinements.

**Test enhancements:**

12. **E615 ordering before matching E602** in the existing test
    (CodeRabbit #2) — pinned via `warnings.index(e615[0])` <
    `warnings.index(matching_e602)`.  Filtered to the E602 that
    mentions the offending function so prelude-skip E602s don't
    confound the assertion.

13. **E615 source-location attached** in the existing test —
    softened to "line > 0" (any location) because SlotRef-inside-
    InterpolatedString spans are unreliable today (#634, separate
    follow-up).  Tightening to "line points at the segment" is
    the natural acceptance test for that follow-up.

**Skipped with reasons:**

- **Cycle-guard regression on `_resolve_base_type_name`** —
  test-analyzer C1 / code-reviewer S4.  Verified pre-existing
  and dead-code-safe today (cycles crash an upstream resolver
  first).  Filed as #633.

- **`_canonical_wasm_type`'s `i64` default for unhandled
  apply_fn / call_indirect shapes** — silent-failure-hunter H1.
  Bigger refactor parallel to Tier 2 but for the WASM-side
  dispatch.  Filed as #632.

- **SlotRef-inside-InterpolatedString span fidelity** —
  surfaced when writing the E615 source-location assertion.
  Diagnostic-quality concern, separate from #630's
  canonicalisation scope.  Filed as #634.

- **`_canonical_named_type` worked-example docstring**
  (comment-analyzer I6) — superseded by fix #2 above (semantics
  changed; docstring rewritten with concrete examples).

- **Per-context isolation prose-vs-test** (comment-analyzer I4)
  — addressed via fix #9 (test added).

- **`_resolve_i32_pair_ret_te` 30-line upper docstring** —
  comment-analyzer style nit.  The prose is load-bearing
  context (#628 cross-module narrative); trimming would lose
  history without obvious gain.  Defer.

**Follow-ups:** #632 (apply_fn / call_indirect E616), #633
(_resolve_base_type_name cycle guard), #634 (interpolation
SlotRef source spans).  Added to KNOWN_ISSUES.md alongside #628.

Counts: 3,792 tests (3,778 passed, 14 skipped) — six new tests
under `TestE615LoudInterpolationFallthrough630`.  CHANGELOG +
KNOWN_ISSUES + ROADMAP + TESTING.md + HISTORY footer all
synced.  mypy clean.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

String-returning function call in string interpolation produces invalid WASM (i64/i32 mismatch)

1 participant