Skip to content

feat(async): concurrent async(Http.get/post) on host worker threads — release v0.0.192#842

Merged
aallan merged 6 commits into
mainfrom
feat/841-concurrent-async
Jul 2, 2026
Merged

feat(async): concurrent async(Http.get/post) on host worker threads — release v0.0.192#842
aallan merged 6 commits into
mainfrom
feat/841-concurrent-async

Conversation

@aallan

@aallan aallan commented Jul 2, 2026

Copy link
Copy Markdown
Owner

What

Delivers the concurrency half deferred when #59 shipped the <Async> language surface, and cuts release v0.0.192 — Stage A of the server-effects sprint (WASI.md records the Stage-0 groundwork this builds on).

async(Http.get(url)) / async(Http.post(url, body)) — with call-free argument expressions — now fuse into a single vera.async_http_get / vera.async_http_post host import that issues the request on a host ThreadPoolExecutor at the async(...) point (request issuance keeps program order) and returns the Future as a #578 bit-31-tagged handle wrapper (new wrap kind 4). await probes the value's first word for the wrapper tag: fused handles unwrap and block on vera.async_await; eager values pass through unchanged. Every other async shape keeps the eager identity lowering.

Two overlapping async(Http.get)s are pinned by a server-side request-ordering test (local ThreadingHTTPServer, the /a handler holds until /b arrives — no wall-clock assertions).

Design invariants

  • One fusion predicate, two consumers. Import emission is decided by both the _scan_io_ops pre-scan and the WasmContext translation; if they disagree, a fused call site references an import the pre-scan suppressed (WAT compile error) or a sync import is declared that translation never calls. The predicates live in one new module — vera/wasm/async_fusion.py — imported by both, and mutation M5 (pre-scan intercept removed) proves the pin.
  • Fusion ⊂ W002 whitelist, so the warning is never false. The fuse condition (direct Http.get/post call, call-free args) is strictly narrower than the checker's W002 commutative whitelist — a warned async can never execute concurrently, and an unwarned-but-eager one is spec-conformant (§9.5.4's MAY).
  • Type-directed await discrimination. The handle-check keys on the literal type Future<Result<String, String>> — the only type fused futures inhabit, always a heap pointer, so the tag probe (0xFEEDC004 at offset 0) is memory-safe. Value-typed futures (Future<Int> is an i64) keep the identity lowering; a pure program emits zero task imports. Alias-typed slots fall back loudly (the wrapper tag matches no constructor → trap), never silently.
  • Workers never touch guest memory. The pure fetch halves (fetch_get/fetch_post, split out of the Http host callbacks in vera/runtime/http.py) run on workers and return Python strings; the Result ADT is built at await time on the guest thread. The GC shadow-stack overflow in array_map (and sibling iterative builders) at ~4000 heap-allocating elements #570/html_parse and json_parse trap with Out-of-bounds memory access on inputs that pressure GC during host-side tree walk #692 UAF class cannot reach a worker.
  • Full opaque-handle GC contract (Decimal pattern, kind 4): wrapper shadow-rooted at the async site — including the operand-stack window (both(async(A), async(B)), get/post-distinguished Err text making an aliased wrapper observable) — and Phase 2c fires host_decref_handle(4, handle) for unawaited futures (cancel + evict, observable via ExecuteResult.host_store_sizes["future"]).
  • Ctrl-C + teardown. A KeyboardInterrupt during a blocked await rides the wasmtime>=45 BaseException trampoline to the centralized exit-130 handler (new e2e sibling of the IO.sleep/IO.read_char pins); the executor is torn down (shutdown(cancel_futures=True)) on every exit path via the invocation finally.
  • Browser stays eager (spec-conformant): the request fires synchronously (XHR) at the async point; the (isOk, payload) strings are buffered host-side (a guest pointer held only in a JS map would be invisible to the conservative GC scan) and the ADT is built at await. New Node e2e test pins the fused-shape Err path.
  • Future<T> return types now compile. Previously a function declaring a Future<T> return was silently [E605]-skipped; Future is now transparent in the type mapper (RED-first via the GC-registration test).

Mutation validation (8/8 kill)

# Mutation Killed by
M1 fusion predicate disabled 4 tests (pure-shape test stays green)
M2 untag i32.and dropped at await behavioral overlap test
M3 wrapper registered as kind 3 kind-4 register_wrapper pin
M4 await check disabled import pin + behavioral
M5 pre-scan intercept removed sync-import-absent pin
M6 kind-4 decref eviction disabled store-reclamation test
M7 wrap-site shadow push dropped operand-stack window test
W002 (prior commit) 4 rule mutations checker battery

(M7's first fixture survived the mutation — let-bound wrappers are rooted elsewhere — so the operand-stack fixture was added and confirmed RED under the mutation before keeping both.)

Docs

Spec §9.5.4 rewritten (implementation MAY evaluate concurrently when the effect row is commutative; the reference implementation's exact shape documented, alias limitation noted), §7.4 effect-ordering note, §12.9.3 browser-divergence row; SKILL.md async section rewritten with a fan-out example; vera/README.md module map (new async_fusion.py + async_http.py rows); TESTING.md tables + battery descriptions; #591 test anchors moved to the fetch halves.

Release v0.0.192

Version sync across 6 files; CHANGELOG section (the queued #420/#419/#839/#835 [Unreleased] split entries ride this release); HISTORY opens Stage 16: the server-effects sprint; doc counts 5,577.

Verification

  • pytest tests/ — 5,529 passed, 22 skipped (incl. browser parity 108)
  • pytest -m stress — 26 passed
  • mypy vera/ clean; ruff check clean without --fix
  • all scripts/check_*.py green (conformance 103, examples 35, doc-counts, version-sync, site-assets, encoding, diagnostic-fields, skill/spec/readme/faq/html examples, licenses, wheel gate, limitations sync, changelog gate)
  • 8 mutations each kill their discriminating tests; restore green
  • behavioral concurrency via server-side request ordering, never wall-clock

Closes #841

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • async(Http.get/post(...)) now lowers to a fused concurrent path on the native runtime; await blocks for results, including browser support.
  • Bug Fixes
    • Added W002 warning when async(...) must evaluate eagerly/serially outside the commutativity whitelist.
    • Improved interrupt and async error-path handling; hardened UTF-8 decoding/propagation for HTTP responses.
  • Documentation
    • Updated spec and guidance to reflect v0.0.192 semantics; added WASI sprint record and refreshed project/test metrics.
  • Tests
    • Expanded checker, codegen, GC-rooting, and cross-module fused-await coverage (including new browser parity).

T and others added 3 commits July 2, 2026 08:00
Nine spike checks executed against wasmtime-py 45.0.0 + wasmtime CLI
46.0.1 -- all pass (one PARTIAL: structured trap frames degrade to
message text through a component boundary). No sprint stage
invalidated. WASI.md records the results, the invariants (the
set_wasi-or-SIGABRT rule, canonical-ABI text landmines, the 4096-byte
write cap, the sync-only #406 gate, the wasm-tools supply-chain trap),
and where each loose end lands. Referenced from TOOLCHAIN.md.

Refs #841 #305 #237 #406

Skip-changelog: design doc only; the sprint stages carry their own CHANGELOG entries

Co-Authored-By: Claude <[email protected]>
First slice of host-threaded concurrent <Async>: the checker rule that
keeps the verifier's sequential-semantics claims honest. async(e) will
only be made concurrent when e's effect row is within the commutative
whitelist ({Http} v1; the Async marker has no operations); any other
row now gets W002 "evaluates eagerly" with rationale/fix/spec_ref --
never a silent implication of concurrency.

The effect collector resolves effect-op calls to their parent effect
and fn calls to declared rows, recursively; unresolvable callees are
conservatively non-commutative. Registered W002 in ERROR_CODES;
diagnostic-fields gate green (spec_ref resolves to §9.5.4 "Async").

Test-first: TestAsyncConcurrencyWhitelist841 was RED (expected W002,
got []) before the rule; mutation-validated four ways -- hook deleted,
whitelist over-permissive (+IO), whitelist over-firing (-Http), and
fn-call rows ignored each flip exactly their discriminating tests.

Refs #841 (the lowering/runtime half follows in this PR)

Co-Authored-By: Claude <[email protected]>
…841)

The concurrency half deferred when #59 shipped the Async surface.
async(Http.get(url)) / async(Http.post(url, body)) -- with call-free
argument expressions -- fuse into a single vera.async_http_* host
import that submits the request to a host ThreadPoolExecutor at the
async(...) point and returns the Future as a #578 bit-31-tagged handle
wrapper (new wrap kind 4).  await probes the value's first word for
the wrapper tag (0xFEEDC004): fused handles unwrap and block on
vera.async_await; eager values pass through unchanged.  Two
overlapping gets are pinned by a server-side request-ordering test
(ThreadingHTTPServer, no wall-clock).

Design invariants:

- The fusion predicate lives in ONE module (vera/wasm/async_fusion.py)
  consumed by both import-emission passes (_scan_io_ops pre-scan and
  the WasmContext translation), so they cannot desync.  It is strictly
  narrower than W002's whitelist, so an "evaluates eagerly" warning
  can never coexist with concurrent execution.
- The await handle-check keys on the literal type
  Future<Result<String, String>> -- the only type fused futures
  inhabit, always a heap pointer, making the tag probe memory-safe.
  Value-typed futures (Future<Int> is an i64) keep the identity
  lowering; alias-typed slots fall back loudly (wrapper tag matches no
  constructor -> trap), never silently.
- Workers run only the pure fetch halves (fetch_get/fetch_post, split
  out of the Http host callbacks in vera/runtime/http.py) and never
  touch guest memory; the Result ADT is built at await time on the
  guest thread under the usual rooting discipline.
- GC follows the full opaque-handle contract: wrapper shadow-rooted at
  the async site (operand-stack window), Phase 2c fires
  host_decref_handle(4, handle) for unawaited futures (cancel + evict,
  observable via ExecuteResult.host_store_sizes["future"]).
- Ctrl-C during a blocked await rides the wasmtime>=45 BaseException
  trampoline to the exit-130 handler; the executor is torn down
  (shutdown(cancel_futures=True)) on every exit path via the
  invocation finally.
- Browser runtime stays eager (spec-conformant MAY): the request fires
  synchronously at the async point, the (isOk, payload) strings are
  buffered host-side (a guest ptr in a JS map would be GC-invisible),
  and the ADT is built at await.
- Functions may now declare a Future<T> return type (previously
  E605-skipped; Future is transparent in the type mapper).

Spec: 9.5.4 rewritten (implementation MAY evaluate concurrently when
the row is commutative; reference-implementation shape documented),
7.4 effect-ordering note, 12.9.3 browser divergence row.  SKILL.md
async section rewritten with a fan-out example.

Tests (all RED-first; 8 mutations each kill their discriminating
tests): TestConcurrentAsync841 (5), TestFutureHandleGCRooting841 (4),
Ctrl-C-during-await e2e, browser fused-shape e2e, updated #591
fetch-half anchors.

Release v0.0.192; the queued #420/#419/#839/#835 [Unreleased] split
entries ride this release.  HISTORY opens Stage 16 (the server-effects
sprint).

Closes #841

Co-Authored-By: Claude <[email protected]>
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.45897% with 61 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.94%. Comparing base (ef8802b) to head (26371ce).

Files with missing lines Patch % Lines
vera/browser/runtime.mjs 53.33% 35 Missing ⚠️
vera/wasm/async_fusion.py 76.56% 15 Missing ⚠️
vera/checker/calls.py 85.71% 5 Missing ⚠️
vera/runtime/async_http.py 94.28% 2 Missing ⚠️
vera/runtime/http.py 92.85% 2 Missing ⚠️
vera/wasm/calls_markup.py 93.10% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #842      +/-   ##
==========================================
- Coverage   92.06%   91.94%   -0.13%     
==========================================
  Files          90       92       +2     
  Lines       27321    27626     +305     
  Branches      324      332       +8     
==========================================
+ Hits        25154    25400     +246     
- Misses       2159     2218      +59     
  Partials        8        8              
Flag Coverage Δ
javascript 65.23% <53.33%> (-0.36%) ⬇️
python 95.02% <89.76%> (-0.05%) ⬇️

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

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

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

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bf53373a-e146-4571-bdfb-b41b4ea461ec

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds host-threaded concurrent execution for whitelisted async(Http.get/post(...)), with a new W002 warning for non-commutative async arguments, fused Future handle plumbing, browser parity, and updated specs, docs, and tests for release 0.0.192.

Changes

Concurrent Async feature

Layer / File(s) Summary
Async fusion predicates
vera/wasm/async_fusion.py, vera/codegen/modules.py, vera/codegen/functions.py, vera/codegen/closures.py, vera/wasm/context.py
Adds shared predicates for fused async target detection, future-return function classification, and await runtime-check eligibility, and threads the metadata through module, function, and closure compilation contexts.
Checker W002 warning
vera/errors.py, vera/checker/calls.py, spec/07-effects.md, tests/test_checker_effects.py, ROADMAP.md, scripts/check_skill_examples.py, scripts/check_spec_examples.py
Adds W002, effect-collection logic, spec wording for effect ordering, and checker coverage for commutative and non-commutative async arguments.
Wasm handle wrapping and async lowering
vera/wasm/calls_containers.py, vera/wasm/calls_markup.py, vera/codegen/assembly.py
Adds the Future wrapper kind and tag, and rewrites async/await lowering to emit fused host imports or identity paths.
Codegen tracking and module assembly
vera/codegen/core.py, vera/codegen/assembly.py, vera/codegen/api.py, vera/codegen/compilability.py
Propagates async host-import usage and future-return metadata through code generation, adjusts Future<T> typing, and emits the needed fused-async imports, alloc support, and wrap-table wiring.
Native and browser async runtimes
vera/runtime/http.py, vera/runtime/async_http.py, vera/browser/runtime.mjs, vera/codegen/api.py
Extracts pure fetch helpers, adds worker-thread async host bindings, stores futures by handle, and wires executor lifecycle and future decref into execute(), with browser-side fused async parity and future-store eviction.
Codegen, GC-rooting, and runtime tests
tests/test_codegen_effects.py, tests/test_codegen_gc_rooting.py, tests/test_browser.py, tests/test_runtime_traps.py, tests/test_codegen_modules.py
Adds checker, codegen, GC-rooting, browser parity, cross-module await, and interrupt regression coverage for fused async behaviour, plus runtime trap anchor updates.
Spec and documentation updates
CHANGELOG.md, HISTORY.md, README.md, ROADMAP.md, SKILL.md, TESTING.md, TOOLCHAIN.md, WASI.md, vera/README.md, pyproject.toml, vera/__init__.py, KNOWN_ISSUES.md, spec/09-standard-library.md
Updates the effect, standard-library, and runtime specifications, release notes, history, README, roadmap, testing notes, toolchain docs, module maps, allowlists, and version fields for the new release.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GuestCode
  participant CallsMarkup
  participant AsyncHTTP
  participant ThreadPoolExecutor
  participant FutureStore

  GuestCode->>CallsMarkup: async(Http.get(url))
  CallsMarkup->>AsyncHTTP: vera.async_http_get(ptr,len)
  AsyncHTTP->>ThreadPoolExecutor: submit(fetch_get(url))
  ThreadPoolExecutor-->>FutureStore: store Future under handle
  AsyncHTTP-->>GuestCode: return wrapped Future handle

  GuestCode->>CallsMarkup: await(future)
  CallsMarkup->>AsyncHTTP: vera.async_await(handle)
  AsyncHTTP->>FutureStore: lookup(handle)
  FutureStore-->>AsyncHTTP: Future
  AsyncHTTP->>ThreadPoolExecutor: Future.result()
  AsyncHTTP-->>GuestCode: Result<String,String>
Loading

Possibly related issues

Possibly related PRs

  • aallan/vera#454: Both PRs modify vera/codegen/closures.py’s lifted-closure path to propagate metadata from the closure WasmContext back to module-level tracking.
  • aallan/vera#707: Both PRs modify the WASM wrap-handle and wrapper-bucket GC plumbing that this PR extends with a new Future wrapper kind.
  • aallan/vera#789: Both PRs touch the HTTP host runtime layer in vera/runtime/http.py, including the shared request helpers reused by the fused async path.

Suggested labels: compiler, tests, spec, ci, docs

🚥 Pre-merge checks | ✅ 8
✅ Passed checks (8 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarises the main change: concurrent fused async HTTP on host worker threads for release v0.0.192.
Linked Issues check ✅ Passed The PR matches #841: fused async HTTP lowering, W002 diagnostics, GC/reclamation, browser parity, Ctrl-C handling, and spec/tests are all covered.
Out of Scope Changes check ✅ Passed The changes are broadly tied to the async-concurrency release and its supporting docs, tests, tooling, and version bumps.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Changelog Covers Public-Surface Changes ✅ Passed CHANGELOG.md’s v0.0.192 entry covers the async handle ABI, W002 diagnostic, and §9.5.4 spec changes; no public-surface change appears omitted.
Spec And Implementation Move Together ✅ Passed PASS: async fusion, W002, Future handle semantics, and browser eager behaviour are implemented under vera/ and reflected in spec/07, 09, and 12.
Diagnostics Carry An Error Code ✅ Passed PASS: the new async warning is tagged W002 and registered in ERROR_CODES; I found no changed diagnostic site missing an E/W### code.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/841-concurrent-async

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

…ions (#841)

Review round (pr-review-toolkit code-reviewer on PR #842) found one
confirmed critical bug and two documentation-accuracy issues:

- CRITICAL: await of a cross-module call returning
  Future<Result<String, String>> lowered to identity -- neither the
  unqualified imported-call shape (compute_future_ret_fns scanned only
  local declarations) nor the qualified m::f shape (no ModuleCall arm
  in await_needs_check) classified, so the kind-4 wrapper flowed into
  the match's unconditional last arm and the future was silently never
  awaited.  Fixed RED-first (TestCrossModuleFusedAwait841, both
  shapes): compute_future_ret_fns now derives from the codegen
  return-type registry (_fn_ret_type_exprs -- local Pass-1
  registrations + the Pass-0 #628 cross-module harvest) and
  await_needs_check gains a ModuleCall arm.

- The "a smuggled wrapper traps loudly on match" claim was false: the
  match lowering's final arm is an unconditional catch-all, so a
  wrapper reaching an identity await is read as the ADT (zero-length
  Err string), not trapped.  Corrected in the async_fusion docstring,
  spec 9.5.4, and the CHANGELOG entry.  The residual unclassifiable
  shape (indirectly-called closure returning this future type) is now
  a documented limitation: KNOWN_ISSUES.md row + #843.

- Alias-typed futures: the accurate story is that an alias-typed let
  has no WASM representation today, so the function is E602-skipped
  before any await could mis-lower -- reworded in spec/CHANGELOG.

Also from review: aligned the CompileResult async_ops_used indentation
at the two early-return sites, and updated the _emit_wrap_handle inner
bucket comment that still read Decimal-only.

Doc counts follow (5,579 tests).

Refs #841

Co-Authored-By: Claude <[email protected]>
@aallan

aallan commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

Review round 1 (pr-review-toolkit code-reviewer) — recorded.

Findings and dispositions, all addressed in 7b304f1:

  1. Critical — cross-module fused await miscompiled (confirmed with a two-module repro): await of an imported or module-qualified call returning Future<Result<String, String>> lowered to identity — compute_future_ret_fns scanned only local declarations and await_needs_check had no ModuleCall arm — so the kind-4 wrapper flowed into the match's unconditional last arm and the future was never awaited. Fixed RED-first (TestCrossModuleFusedAwait841, both the unqualified-import and m::f shapes): the fn set now derives from the codegen return-type registry (local Pass-1 registrations + the Pass-0 Cross-module imports don't propagate _fn_ret_type_exprs (#614 / #602 cross-module gap) #628 cross-module harvest) and await_needs_check gains a ModuleCall arm.
  2. Important — the "smuggled wrapper traps loudly on match" claim was false (the match lowering's final arm is an unconditional catch-all). Corrected in the async_fusion docstring, spec §9.5.4, and the CHANGELOG entry; the residual unclassifiable shape (indirectly-called closure returning this future type) is now a documented limitation — KNOWN_ISSUES row + Fused-async await classification misses indirectly-called closure results #843.
  3. Important — alias-typed futures: accurate story documented (an alias-typed let has no WASM representation today → [E602] skip before any await could mis-lower), replacing the wrapper-tag-trap wording.
  4. Suggestion — CompileResult indentation at the two early-return sites: aligned.
  5. Suggestion — stale Decimal-only comment in _emit_wrap_handle: updated for kind 4.

Full gates re-run green after the fix (suite, mypy, ruff, doc-counts, site assets, limitations sync, spec/SKILL example checkers).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against 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 test-count summary is inconsistent because the total in the Tests
row does not match the passed, stress, and skipped breakdown. Update the
TESTING.md counts so the aggregate and sub-counts reconcile, using the Tests
table entry and the reported passed/stress/skipped figures as the source of
truth.

In `@vera/checker/calls.py`:
- Around line 211-283: W002 currently only inspects effect rows in
_warn_non_commutative_async and _collect_expr_effects, so it misses async
arguments that are non-fusable by shape even when they look whitelisted by
effects. Update the checker to also validate the async argument’s call shape
(for example, only direct Http.get/Http.post QualifiedCall with call-free
arguments), and warn when the expression contains a helper FnCall or other
non-fusable form. Use _warn_non_commutative_async, _collect_expr_effects, and
the async fusion shape rules from fused_async_arg_target / _expr_is_call_free as
the reference points when tightening the condition.

In `@vera/codegen/api.py`:
- Around line 1069-1081: Add a test that exercises Ctrl-C while a real async
HTTP request is in flight, since the current coverage only mocks Future.result()
and uses ftp:// and never hits the shutdown wait path. Update or extend
test_async_await_keyboard_interrupt_end_to_end to use a live HTTP fetch and
trigger KeyboardInterrupt during the request, so the
_async_executor.shutdown(wait=True, cancel_futures=True) behavior is covered
when an already-started urlopen is still running. Reference the async execution
path in vera.codegen.api around func(store, *call_args) and the _async_executor
shutdown handling to keep the test aligned with the real exit-130 flow.

In `@vera/codegen/compilability.py`:
- Around line 312-319: The narrowing assert in compilability.py should be marked
to satisfy Ruff’s S101 check. Update the `assert isinstance(inner,
ast.QualifiedCall)` inside the `fused_async_target` handling in
`CompilabilityAnalyzer._scan_io_ops` to include `# noqa: S101`, matching the
existing mypy-only narrowing asserts so the lint gate stays green.
🪄 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 Plus

Run ID: 42848729-e368-4018-ad6b-b46694dd88cf

📥 Commits

Reviewing files that changed from the base of the PR and between ef8802b and a81c39d.

⛔ Files ignored due to path filters (6)
  • docs/SKILL.md is excluded by !docs/**
  • docs/index.html is excluded by !docs/**
  • docs/index.md is excluded by !docs/**
  • docs/llms-full.txt is excluded by !docs/**
  • docs/llms.txt is excluded by !docs/**
  • uv.lock is excluded by !**/*.lock, !uv.lock
📒 Files selected for processing (36)
  • CHANGELOG.md
  • HISTORY.md
  • README.md
  • ROADMAP.md
  • SKILL.md
  • TESTING.md
  • TOOLCHAIN.md
  • WASI.md
  • pyproject.toml
  • scripts/check_skill_examples.py
  • scripts/check_spec_examples.py
  • spec/07-effects.md
  • spec/09-standard-library.md
  • spec/12-runtime.md
  • tests/test_browser.py
  • tests/test_checker_effects.py
  • tests/test_codegen_effects.py
  • tests/test_codegen_gc_rooting.py
  • tests/test_runtime_traps.py
  • vera/README.md
  • vera/__init__.py
  • vera/browser/runtime.mjs
  • vera/checker/calls.py
  • vera/codegen/api.py
  • vera/codegen/assembly.py
  • vera/codegen/closures.py
  • vera/codegen/compilability.py
  • vera/codegen/core.py
  • vera/codegen/functions.py
  • vera/errors.py
  • vera/runtime/async_http.py
  • vera/runtime/http.py
  • vera/wasm/async_fusion.py
  • vera/wasm/calls_containers.py
  • vera/wasm/calls_markup.py
  • vera/wasm/context.py
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • aallan/vera-bench (manual)

Comment thread TESTING.md Outdated
Comment thread vera/checker/calls.py
Comment thread vera/codegen/api.py
Comment thread vera/codegen/compilability.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 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 `@vera/codegen/core.py`:
- Around line 453-462: _Recompute the future-return function registry after
monomorphized registrations._ `_future_ret_fns` is currently built too early in
`Core.compile`, before the `mono_decls` loop registers clone names like
`grab$Int`, so `await_needs_check` can miss rewritten awaited calls. Move or
repeat the `compute_future_ret_fns(self._fn_ret_type_exprs)` step after `for
mdecl in mono_decls: self._register_fn(mdecl)` and before any `_compile_fn`
calls, so monomorphized names are included in the await-check classification.

In `@vera/wasm/async_fusion.py`:
- Around line 117-143: Module-qualified future detection is using only the bare
function name, so `ModuleCall` can apply the fused await path to the wrong
target when different modules share a name. Update the resolution in
`compute_future_ret_fns`/the `ModuleCall` await-lowering path to key off the
same resolved `(path, name)` WAT target used for module-qualified calls, and
compare that resolved target against the future-return registry instead of just
`"grab"`. Make sure both the pre-scan and `WasmContext` await handling use the
same resolved target lookup so imported and local functions are classified
consistently.
🪄 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 Plus

Run ID: 385b8183-1378-4ec5-b1ce-d31f4e77fc82

📥 Commits

Reviewing files that changed from the base of the PR and between a81c39d and 7b304f1.

📒 Files selected for processing (10)
  • CHANGELOG.md
  • KNOWN_ISSUES.md
  • README.md
  • ROADMAP.md
  • TESTING.md
  • spec/09-standard-library.md
  • tests/test_codegen_modules.py
  • vera/codegen/core.py
  • vera/wasm/async_fusion.py
  • vera/wasm/calls_containers.py
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • aallan/vera-bench (manual)

Comment thread vera/codegen/core.py
Comment thread vera/wasm/async_fusion.py
… 2 (#841)

CodeRabbit round on PR #842 (6 findings, each verified empirically):

- CONFIRMED (repro: name collision, value 0, async_await import
  missing): ModuleCall awaits classified by BARE name, so a local
  `grab` returning a different Future shape misclassified
  await(fetcher::grab(...)) in both directions (identity -> silent
  wrapper smuggle; or a probe on an i64 future).  Fixed RED-first: the
  modules.py #628 harvest now also keys (module path, name) ->
  return-type, compute_future_ret_module_fns derives a qualified set,
  and the ModuleCall arm of await_needs_check resolves by
  (path, name).  New collision regression test.

- NOT REPRODUCIBLE (mono recompute claim): a generic fn with a
  concrete Future<Result<String, String>> return classifies via its
  template name -- classification runs on the pre-monomorphization
  AST; the call-site mangling to wrap$Int happens later, during
  translation.  Pinned with a new test so a future move to AST-level
  mono rewriting fails loudly here.

- Ctrl-C coverage gap: added an event-gated live-request test --
  interrupt_main() fires only after the server confirms the request
  is in flight, the handler is then released so the invocation
  finally's shutdown(wait=True) has a real worker to wait out; pins
  exit-130 + no-hang on the un-mocked path.

- Two S101 narrowing asserts now carry noqa (the CI lint job runs
  ruff --select S on vera/); TESTING.md summary breakdown reconciled
  (5,534 passed + 26 stress + 22 skipped = 5,582).

- The effect-eligible-but-shape-unfusable advisory (W003 proposal) is
  a design extension beyond the #841 scope (W002's contract is
  commutativity, and the no-warning helper-wrap shape is deliberately
  pinned) -- filed as #844 with a ROADMAP row.

Refs #841

Co-Authored-By: Claude <[email protected]>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against 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 90: Update the TESTING.md coverage row for test_codegen_effects.py to
include the newly added generic-fn future-return case. In the existing summary
near the other pinned effect tests, add a reference to
test_await_of_generic_fn_with_concrete_future_return and describe it as the
concrete Future<Result<String, String>> await-site classification check so the
documented coverage matches the current test suite.
- Line 145: The TestHostSleepKeyboardInterrupt summary undercounts the
fused-await coverage: it now includes four end-to-end tests, not three. Update
the `TESTING.md` description around `TestHostSleepKeyboardInterrupt` to mention
the additional live, unmocked interrupted fetch test
(`test_async_await_keyboard_interrupt_live_request`) alongside the existing
`IO.sleep`, `IO.read_char`, and mocked `Future.result()` fused-await coverage.
Keep the wording aligned with the test names and the `execute()`
keyboard-interrupt handling described in this section.

In `@vera/wasm/async_fusion.py`:
- Around line 147-166: The docstring in compute_future_ret_module_fns uses
inconsistent triple-single-quote style compared with the rest of the file.
Update that function’s docstring to the standard triple-double-quote form used
elsewhere so it matches Black’s formatting conventions and the surrounding
documentation style.
🪄 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 Plus

Run ID: db9cfca7-1e82-4a9f-a298-e38f391653e3

📥 Commits

Reviewing files that changed from the base of the PR and between 7b304f1 and 8fe42b9.

📒 Files selected for processing (15)
  • CHANGELOG.md
  • README.md
  • ROADMAP.md
  • TESTING.md
  • tests/test_codegen_effects.py
  • tests/test_codegen_modules.py
  • tests/test_runtime_traps.py
  • vera/codegen/closures.py
  • vera/codegen/compilability.py
  • vera/codegen/core.py
  • vera/codegen/functions.py
  • vera/codegen/modules.py
  • vera/wasm/async_fusion.py
  • vera/wasm/calls_markup.py
  • vera/wasm/context.py
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • aallan/vera-bench (manual)

Comment thread TESTING.md Outdated
Comment thread TESTING.md Outdated
Comment thread vera/wasm/async_fusion.py
…uotes (#841)

Two TESTING.md description rows catch up with the round-2 tests (the
generic-fn future-return pin; three -> four Ctrl-C e2e tests incl. the
live in-flight variant), and compute_future_ret_module_fns's docstring
quote style joins the rest of the file.

Refs #841

Co-Authored-By: Claude <[email protected]>
@aallan

aallan commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

Host-threaded concurrent <Async> for commutative effect rows

1 participant