Skip to content

Release v0.0.119: JSON typed accessors (#366)#511

Merged
aallan merged 6 commits into
mainfrom
json-typed-accessors
Apr 23, 2026
Merged

Release v0.0.119: JSON typed accessors (#366)#511
aallan merged 6 commits into
mainfrom
json-typed-accessors

Conversation

@aallan

@aallan aallan commented Apr 23, 2026

Copy link
Copy Markdown
Owner

Summary

Eleven new JSON accessor functions for the built-in Json ADT — the ergonomic polish layer that eliminates the two-level pattern-match boilerplate every JSON API consumer would otherwise write. First non-WASM-translator stdlib addition of Stage 11: these are pure-Vera prelude functions, not WASM translators.

Layer 1 — type-coercion accessors (Json → Option<T>)

  • json_as_string, json_as_number, json_as_bool, json_as_int, json_as_array, json_as_object
  • Some when the Json's constructor matches, None otherwise. Disjoint: at most one returns Some for any given Json.
  • json_as_int guards float_to_int against its trap on NaN and infinity via float_is_nan / float_is_infinite, returning None for those inputs. Overflow (|f| ≥ 2^63) still traps — a deliberate choice to keep the body simple.

Layer 2 — compound field accessors (Json, String → Option<T>)

  • json_get_string, json_get_number, json_get_bool, json_get_int, json_get_array
  • Each collapses json_get(obj, key) and the matching json_as_* coercion into one call. Missing field AND wrong-typed field both return None — exactly what 90% of real API-consuming code wants.
  • No json_get_object: chained field access is handled by json_get returning Option<Json>, then recursing.

Before / after

-- Before (the pattern this PR kills):
match json_parse(response) {
  Err(@String) -> Err(@String.0),
  Ok(@Json) -> match json_get(@Json.0, "x") {
    None -> Err("missing x"),
    Some(@Json) -> match @Json.0 {
      JNumber(@Float64) -> Ok(float_to_int(@Float64.0)),
      _ -> Err("x is not a number")
    }
  }
}

-- After:
match json_parse(response) {
  Err(@String) -> Err(@String.0),
  Ok(@Json) -> match json_get_int(@Json.0, "x") {
    Some(@Int) -> Ok(@Int.0),
    None -> Err("x missing or not a number")
  }
}

Implementation

Pure-Vera prelude functions, not WASM emitters:

  • vera/prelude.py _JSON_COMBINATORS extended with all 11 bodies as match expressions over Json. The compiler already injects the prelude into every module that references Json values — no detection changes needed beyond adding the 11 new names to the detection set and the injection-site shadow-check set.
  • vera/environment.py registers the 11 new FunctionInfo entries. Layer-1 accessors come from a tight Python loop (six identical Json → Option<T> shapes).
  • No WASM translator code added. No new host imports. Compiled modules that don't reference Json stay byte-identical.

One oddity worth calling out: my first pass modelled json_as_int as match float_to_int(...) { Ok(@Int) -> Some(...), Err(_) -> None } — but float_to_int actually returns Int and traps on NaN/Inf, not Result<Int, String>. Caught the WASM type-mismatch at smoke-test time (expected i32, found i64) and rewrote to use the float_is_nan || float_is_infinite guard. The spec entry flags this asymmetry.

Tests

  • 19 new unit tests (TestJsonTypedAccessors in tests/test_codegen.py): every Layer-1 accessor (matching + mismatched constructors), every Layer-2 accessor (hit + missing + wrong type), edge cases (json_as_int NaN, infinity, toward-zero truncation for negatives), disjointness invariant.
  • 2 new browser parity tests (TestBrowserJsonAccessors): exhaustive Layer-1 + Layer-2 sweeps.
  • New conformance program tests/conformance/ch09_json_accessors.vera (level: run) with 15 test functions.
  • examples/json.vera rewritten to use the accessors — the weather-API scenario that previously needed a helper-per-field pattern now uses Layer-2 accessors directly.

Docs swept

  • spec/09-standard-library.md — new §9.6.23 with full signatures, Layer-1 / Layer-2 distinction, NaN-guard note, and verification tier classification.
  • SKILL.md — new "Typed accessors (Layer 1 and Layer 2)" subsection after the existing Json section, plus the naming-convention paragraph extended with json_as_ / json_get_ prefixes.
  • CHANGELOG.md — v0.0.119 entry with separate Added / Implementation / Example rewrite / Tests sections.
  • HISTORY.md — Stage 11 row added; tagged-release total bumped 118 → 119.
  • ROADMAP.mdJSON typed accessors: json_as_string, json_get_number, etc. #366 removed from short-term queue and Phase 4c; replaced with Pre-existing bugs in WASM call translators (surfaced by #474 review) #475 (WASM call translator bug cleanup) as the new top priority for the next PR.
  • Count bumps across TESTING / CLAUDE / AGENTS / FAQ / README / vera-README: 3,527 tests, 80 conformance programs, 164 built-in functions.

Release prep

  • Version bumped 0.0.118 → 0.0.119 across vera/__init__.py, pyproject.toml, docs/index.html, README.md, uv.lock.
  • Tag v0.0.119 placed on the feature branch; will be force-moved to main HEAD after merge.

Test plan

  • 19 new unit tests pass (pytest tests/test_codegen.py::TestJsonTypedAccessors)
  • 2 new browser parity tests pass
  • All 80 conformance programs pass
  • All 32 examples pass check + verify
  • Full pytest suite green (3,513 passed)
  • mypy clean
  • All 14 doc validation scripts green

Closes #366

Summary by CodeRabbit

  • New Features

    • Added 11 JSON typed accessors: six json_as_* coercions and five json_get_* field accessors. json_as_int returns None for NaN, ±infinity and out‑of‑range floats.
  • Documentation

    • Spec, changelog, README, roadmap and guides updated; conformance/test counts bumped to v0.0.119.
  • Tests

    • New unit, browser‑parity and conformance tests covering the JSON accessors; conformance suite now 80 programs.
  • Chores

    • Package/library version bumped to v0.0.119.

Eleven new accessor functions for the built-in Json ADT, shipped as
pure-Vera prelude functions rather than WASM translators.  Closes #366.

Layer 1 - type-coercion accessors (Json -> Option<T>):
- json_as_string, json_as_number, json_as_bool, json_as_int,
  json_as_array, json_as_object
- Some when the Json's constructor matches the target type, None
  otherwise.  Accessors are disjoint: at most one returns Some.

Layer 2 - compound field accessors (Json, String -> Option<T>):
- json_get_string, json_get_number, json_get_bool, json_get_int,
  json_get_array
- Each collapses json_get(obj, key) + json_as_X into one call;
  missing field OR wrong-typed field both return None.  The pattern
  ~90% of real API-consuming code wants.

json_as_int specifically:
- Calls float_to_int, which truncates toward zero.
- Guards against the float_to_int trap on NaN and infinity via
  float_is_nan / float_is_infinite, returning None for those inputs.

Implementation - first non-WASM-translator stdlib addition this
stage:
- All 11 bodies live in vera/prelude.py _JSON_COMBINATORS as match
  expressions over Json; the compiler already injects the prelude
  into every module that references Json values.  No new host
  imports, no new WASM emit.
- vera/environment.py registers the 11 new FunctionInfo entries
  alongside existing json_get / json_type registrations.
- vera/prelude.py extended in three places: combinator bodies,
  _source_mentions_json detection set, and the injection-site
  shadow-check set.

Tests and example:
- 19 new unit tests (TestJsonTypedAccessors in test_codegen.py) -
  every accessor, matching and mismatched constructors, hit and
  missing field, wrong type, NaN/infinity guards, disjointness
  invariant.
- 2 new browser parity tests (TestBrowserJsonAccessors) - exhaustive
  Layer-1 + Layer-2 sweeps under wasmtime and Node.js.
- New conformance ch09_json_accessors.vera with 15 test functions
  (level: run).
- examples/json.vera rewritten to use the accessors; eliminates the
  per-field two-level match boilerplate.

Docs swept:
- spec/09-standard-library.md: new section 9.6.23.
- SKILL.md: new 'Typed accessors (Layer 1 and Layer 2)' subsection
  in the Json section plus naming-convention bump.
- CHANGELOG, HISTORY (Stage 11 table row), ROADMAP (#366 removed
  from short-term queue and Phase 4c, replaced with #475 bug
  cleanup as the new top priority).
- Count bumps across TESTING/CLAUDE/AGENTS/FAQ/README/vera-README.

Co-Authored-By: Claude <noreply@anthropic.invalid>
@codecov

codecov Bot commented Apr 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.03%. Comparing base (e5d97e2) to head (0db5ce2).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #511      +/-   ##
==========================================
+ Coverage   90.59%   91.03%   +0.44%     
==========================================
  Files          58       58              
  Lines       21958    21964       +6     
  Branches      237      259      +22     
==========================================
+ Hits        19892    19995     +103     
+ Misses       2062     1962     -100     
- Partials        4        7       +3     
Flag Coverage Δ
javascript 56.83% <ø> (+4.20%) ⬆️
python 95.00% <100.00%> (+<0.01%) ⬆️

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.

@coderabbitai

coderabbitai Bot commented Apr 23, 2026

Copy link
Copy Markdown

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds eleven JSON typed-accessor prelude functions (json_as_*, json_get_*), updates spec (§9.6.23), registers new builtins in the type environment, injects prelude combinators, adds tests and a conformance program, and bumps package/version metadata and documentation for v0.0.119.

Changes

Cohort / File(s) Summary
Version & Package
vera/__init__.py, pyproject.toml
Bumped package version from 0.0.1180.0.119.
Specification
spec/09-standard-library.md
Added §9.6.23 describing six json_as_* coercions and five json_get_* field accessors; documents json_as_int float-to-int guards and verification-tier notes.
Prelude & Injection
vera/prelude.py
Implemented json_as_* and json_get_* as pure-Vera prelude functions and updated JSON detection/injection lists (_JSON_COMBINATORS, json_fn_names, _source_mentions_json).
Type Environment
vera/environment.py
Registered eleven new builtin function signatures in the type environment as pure effect functions.
Stdlib Spec → Type Env Sync
spec/09-standard-library.md, vera/environment.py, vera/prelude.py
Spec additions mirrored by builtin registrations and prelude injection for the new accessors.
Tests — Codegen & Browser
tests/test_codegen.py, tests/test_browser.py
Added TestJsonTypedAccessors suites covering Layer‑1 coercions and Layer‑2 field accessors, including json_as_int edge cases and purity/import assertions.
Conformance & Manifests
tests/conformance/manifest.json, tests/conformance/*, TESTING.md
Added conformance manifest entry for ch09_json_accessors.vera; incremented conformance program count (79→80) and updated testing metrics.
Spec Examples & Allowlist Scripts
scripts/check_spec_examples.py, scripts/check_skill_examples.py
Updated internal ALLOWLIST mappings and line-number entries to accommodate new spec/skill fragments for the typed accessors.
Documentation & Release Notes
CHANGELOG.md, HISTORY.md, README.md, ROADMAP.md, SKILL.md, AGENTS.md, CLAUDE.md, FAQ.md
Documented v0.0.119 release and JSON accessor feature; bumped counts (builtins 153→164, tests/conformance, tagged releases to 119) and added SKILL naming/docs for typed accessors.
Test Manifests (duplicate entry)
tests/conformance/manifest.json
Added manifest entry referencing ch09_json_accessors.vera.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested labels

compiler, tests, spec, ci, 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 Title accurately describes the primary changeset: a release v0.0.119 adding JSON typed accessors and closes issue #366.
Linked Issues check ✅ Passed All stated objectives from #366 are met: Layer 1 coercions (json_as_), Layer 2 field accessors (json_get_), json_as_int float_to_int guards, pure-Vera prelude implementation, and naming conventions.
Out of Scope Changes check ✅ Passed All changes align with issue #366 objectives; the PR adds JSON accessors, updates documentation, version metadata, and test coverage without introducing unrelated refactoring or feature scope creep.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch json-typed-accessors

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

@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: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@HISTORY.md`:
- Around line 255-256: The release rows are out of order: locate the table
entries for versions "v0.0.119" and "v0.0.118" and swap their entire rows so
that v0.0.118 appears before v0.0.119 (preserving their full cells including
dates, descriptions and links), then verify the table remains valid Markdown and
the links/descriptions are unchanged.

In `@scripts/check_skill_examples.py`:
- Around line 121-138: The ALLOWLIST dict in scripts/check_skill_examples.py
contains duplicate numeric keys that silently overwrite earlier entries; remove
the stale entries keyed at 1845 and 1893 so they no longer shadow the correct,
shifted entries (keep the corrected entries at 1878 and 1926 which are the
proper references after the JSON insertion). Locate the ALLOWLIST definition and
delete the two duplicate lines that assign 1845: ("FRAGMENT", "Wrong: import
aliasing not supported") and 1893: ("FRAGMENT", "Correct: map_new/set_new with
type context"), ensuring only the intended entries (e.g., 1878 and 1926 with the
"(shifted by `#366`)" annotations) remain. Also run the script’s quick sanity
check to confirm no other duplicate numeric keys exist in ALLOWLIST.

In `@tests/test_browser.py`:
- Around line 1272-1370: The tests claim to cover every json_as_* and json_get_*
accessor but miss array/object coercions and int edge-cases and don't test
json_get_* on non-object inputs; update test_layer1_coercions to include
json_as_array(JArray(...)) and json_as_object(JObject(...)) checks and add
json_as_int(JNumber(inf)) and json_as_int(JNumber(-inf)) cases (expect None for
infinities or appropriate behavior), and update test_layer2_compound_accessors
to also exercise
json_get_string/json_get_int/json_get_bool/json_get_number/json_get_array when
the parsed Json is a non-object (e.g., parse "[]" or "42") and assert they
return None; locate these changes around the test functions
test_layer1_coercions and test_layer2_compound_accessors and the json_* symbols
mentioned to insert the missing assertions.

In `@tests/test_codegen.py`:
- Around line 12373-12433: Add a new unit test in tests/test_codegen.py that
exercises the finite-overflow path for json_as_int: create a test (e.g.
test_json_as_int_finite_overflow_returns_none) that calls json_as_int with a
JNumber whose magnitude is >= 2^63 (e.g. 9223372036854775808.0) and asserts the
accessor returns None (match Some -> 0, None -> 1, expect 1). This ensures
json_as_int’s guard around float_to_int (and i64.trunc_f64_s) handles finite
overflow without trapping.
- Around line 12294-12646: Add import-gating tests around the new typed
accessors to assert they do not force vera.json_parse or vera.json_stringify
imports when only json_as_* and json_get_* are used: in the
TestJsonTypedAccessors test class add two small tests that compile/run snippets
invoking json_as_string/json_as_int/json_get_string (and another invoking
json_get_int/json_get_array) and then assert the generated module's imports list
does not contain "vera.json_parse" or "vera.json_stringify"; also add a
complementary test that calls json_parse/json_stringify directly to assert those
imports do appear. Locate the tests near existing Layer-1/Layer-2 tests and
reference the accessor names (json_as_string, json_as_int, json_get_string,
json_get_int, json_get_array) so the assertions check import emission rather
than runtime behavior.
🪄 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: 5d334cf4-4cfc-4ed2-aeed-32d054c6ace1

📥 Commits

Reviewing files that changed from the base of the PR and between e5d97e2 and 1f7fbb9.

⛔ Files ignored due to path filters (8)
  • 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/**
  • examples/json.vera is excluded by !**/*.vera
  • tests/conformance/ch09_json_accessors.vera is excluded by !**/*.vera
  • uv.lock is excluded by !**/*.lock, !uv.lock
📒 Files selected for processing (19)
  • AGENTS.md
  • CHANGELOG.md
  • CLAUDE.md
  • FAQ.md
  • HISTORY.md
  • README.md
  • ROADMAP.md
  • SKILL.md
  • TESTING.md
  • pyproject.toml
  • scripts/check_skill_examples.py
  • scripts/check_spec_examples.py
  • spec/09-standard-library.md
  • tests/conformance/manifest.json
  • tests/test_browser.py
  • tests/test_codegen.py
  • vera/__init__.py
  • vera/environment.py
  • vera/prelude.py

Comment thread HISTORY.md Outdated
Comment thread scripts/check_skill_examples.py Outdated
Comment thread tests/test_browser.py Outdated
Comment thread tests/test_codegen.py
Comment thread tests/test_codegen.py
Verified each finding against the current code; fixed all five.

Real fixes:
- HISTORY.md: the v0.0.119 row was listed before v0.0.118 in the
  Stage 11 release table.  Convention is oldest-to-newest; swapped
  them.  Also fixed a spurious duplicate of line 255 introduced by
  the awk swap.
- scripts/check_skill_examples.py: the ALLOWLIST dict had two
  duplicate numeric keys (1845 at lines 121 and 124; 1893 at
  lines 126 and 138) that silently shadowed two of the four real
  entries via last-write-wins.  Four code blocks at four distinct
  line numbers (1845, 1878, 1893, 1926) now each have their own
  explicit entry; my earlier #366 additions used 1878 and 1926 as
  separate entries, so folded those into the main Import / Map-Set
  sections.  Added an AST-based duplicate-key check during CR-fix
  verification (AnnAssign-aware because the ALLOWLIST is type-
  annotated now).
- vera/prelude.py json_as_int: extended the float-to-int trap guard
  to cover finite overflow (|f| >= 2^63), not just NaN/infinity.
  Now returns None for all four WASM trap paths.  Previously
  returned `None` for NaN/infinity but would still trap on finite
  out-of-range values — documented but arguably surprising for
  an accessor.  One extra `||` pair in the guard expression.

Test coverage additions:
- tests/test_browser.py test_layer1_coercions: extended with +inf,
  -inf, json_as_array matching JArray, and json_as_object matching
  a parsed JObject (via json_parse to get a real Map).
- tests/test_browser.py test_layer2_compound_accessors: extended
  with a non-object (JArray) Json across all five Layer-2
  accessors to assert they return None (underlying json_get returns
  None for non-JObject; the accessor chain preserves that).
- tests/test_codegen.py: added test_json_as_int_finite_overflow_
  returns_none (|f| = 2^63) and
  test_json_as_int_finite_negative_overflow_returns_none (|f| = 2^64).
  Both required the finite-overflow guard above.
- tests/test_codegen.py: new "Import gating" section with three
  tests — two pin the invariant that json_as_* / json_get_*
  accessors do NOT force `(import "vera" "json_parse")` or
  `(import "vera" "json_stringify")` emissions (grep the WAT for
  the import-table entry); one complementary test uses json_parse
  and asserts the import DOES appear, pinning the check's
  direction.

Doc updates reflecting the finite-overflow guard:
- CHANGELOG, HISTORY, spec/09 §9.6.23 sentence, SKILL.md
  one-liner comment all updated to "every float_to_int trap path"
  or equivalent phrasing; the old wording ("NaN and infinity")
  was now incomplete.

All 3,518 tests pass; mypy clean; all 14 doc validation scripts
green.

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

@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: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@HISTORY.md`:
- Line 256: The Markdown table row describing json_as_int contains the raw pipe
characters in the expression |f| >= 2^63 which breaks the table; update that
cell to either backtick-code-format the expression (e.g. `|f| >= 2^63`) or
escape the pipes (e.g. \|f\| >= 2^63) so the table renders correctly; touch the
HISTORY.md table row that mentions json_as_int and the surrounding sentence
about float_to_int traps to make this change.

In `@scripts/check_skill_examples.py`:
- Around line 123-128: Update the contradictory allowlist comment block that
begins "Import syntax — intentionally unsupported." to remove or correct the
misleading parenthetical that currently lists only two line numbers; either list
all four actual fence line numbers or delete the parenthetical so the sentence
simply states "four distinct line numbers" to avoid future confusion during
allowlist maintenance.

In `@spec/09-standard-library.md`:
- Around line 1815-1816: Clarify the sentence about WASM: update the line that
currently reads "No new host imports; no new WASM emit." to state that while
there are no new host imports and no dedicated WASM translator paths, the
pure-Vera prelude accessors (the functions injected for Json and the existing
json_get / json_keys / json_type combinators) still compile into ordinary WASM
functions when referenced; i.e., emphasize "no new host imports or dedicated
translator paths, but these accessors are emitted as regular WASM functions when
used."

In `@tests/test_browser.py`:
- Around line 1272-1346: The test test_layer1_coercions currently checks NaN and
±infinity for json_as_int but misses finite overflow; update the
test_layer1_coercions body to include additional match cases that call
json_as_int with JNumber(9223372036854775808.0) and JNumber(0.0 -
9223372036854775808.0) (i.e. ±2^63) and assert they take the None branch
(printing "none") just like the NaN/∞ cases so that json_as_int rejects finite
values with |f| >= 2^63.

In `@tests/test_codegen.py`:
- Around line 12455-12468: The test
test_json_as_int_finite_negative_overflow_returns_none currently checks -2^64
but misses the exact -2^63 boundary; add an explicit case invoking
json_as_int(JNumber(-9223372036854775808.0)) (or a value just below that like
-9223372036854775809.0 if representable) and assert it returns None (match to
None -> 1) to ensure the implementation returns None at the |f| >= 2^63
threshold; update the test body in
test_json_as_int_finite_negative_overflow_returns_none to include this
boundary-check alongside the existing JNumber(...) check using the same
Some(`@Int`)/None match form.
- Around line 12295-12301: The docstring header for the new Json accessors
currently only mentions NaN/Infinity guards for json_as_int; update the
class/module docstring text around the "#366 — 11 new Json typed accessors"
header to state that json_as_int also guards against finite overflow (in
addition to NaN/Infinity) by checking float_is_nan / float_is_infinite and
overflow conditions before converting; edit the sentence that mentions
json_as_int to explicitly include "finite overflow" (or similar phrasing) and
reference the guard behavior so future maintainers see the full protection
semantics.
🪄 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: d6209382-c9da-4e4f-beb0-828bfc02860a

📥 Commits

Reviewing files that changed from the base of the PR and between 1f7fbb9 and 6da9f21.

⛔ Files ignored due to path filters (2)
  • docs/SKILL.md is excluded by !docs/**
  • docs/llms-full.txt is excluded by !docs/**
📒 Files selected for processing (11)
  • CHANGELOG.md
  • HISTORY.md
  • README.md
  • ROADMAP.md
  • SKILL.md
  • TESTING.md
  • scripts/check_skill_examples.py
  • spec/09-standard-library.md
  • tests/test_browser.py
  • tests/test_codegen.py
  • vera/prelude.py

Comment thread HISTORY.md Outdated
Comment thread scripts/check_skill_examples.py Outdated
Comment thread spec/09-standard-library.md Outdated
Comment thread tests/test_browser.py
Comment thread tests/test_codegen.py Outdated
Comment thread tests/test_codegen.py
Verified each finding; fixed all six.  One uncovered a pre-existing
bug (already tracked in #475 bug 9) during browser-parity testing
and required a workaround.

Real fixes:
- HISTORY.md: the literal `|f|` in the v0.0.119 row was being
  parsed as extra table separators (same bug as the `||` issue on
  the v0.0.118 row last week).  Wrapped `|f| >= 2^63` in backticks.
- scripts/check_skill_examples.py: the comment block above the
  Import allowlist entries had a misleading parenthetical
  referring to only two line numbers.  Rewrote to state the four
  distinct numbers explicitly and reference the #511 fix.
- spec/09-standard-library.md §9.6.23: "No new host imports; no
  new WASM emit" was too terse and technically wrong — the
  accessors ARE compiled to ordinary WASM functions when
  referenced.  Rewrote to clarify "no new host imports and no
  dedicated WASM translator paths" while noting the accessors
  still compile as regular WASM when used; modules that never
  call any of them pay zero WASM cost.
- tests/test_browser.py test_layer1_coercions: extended the
  json_as_int sweep to cover the finite-overflow cases (|f| =
  2^63 upper, -2^63 lower boundary, below -2^63), with an
  explicit note that the upper bound is exclusive but the lower
  bound is inclusive (matching WASM's i64 range).
- tests/test_codegen.py: added test_json_as_int_boundary_minus_2_63
  _is_representable (pins Some(INT64_MIN) at the inclusive lower
  bound — a strict-vs-non-strict comparison off-by-one would
  silently corrupt valid inputs) and test_json_as_int_boundary_
  just_below_minus_2_63 (pins None at the next representable
  Float64 below -2^63).  The first failing attempt used
  -2^63 - 1024, which is exactly halfway between representable
  doubles and rounds back to -2^63 — now using -2^63 - 2048,
  the true ulp-below value.  Docstring of TestJsonTypedAccessors
  updated to document all four guarded trap paths.

Pre-existing bug surfaced:
- The browser -2^63 test initially printed the value via
  int_to_string(@Int.0).  Browser runtime returned `-` instead of
  `-9223372036854775808` — this is #475 bug 9 (_translate_to_string
  INT64_MIN negation overflow), which predates this PR.  Worked
  around with `bool_to_string(@Int.0 < 0)` which probes the value
  without triggering the bug.  Note added to the test body.  The
  bug itself is on the queue as part of the upcoming v0.0.120
  #475 cleanup.

All 3,520 tests pass; mypy clean; all 14 doc validation scripts
green.

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

@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 the current code and only fix it if needed.

Inline comments:
In `@HISTORY.md`:
- Line 256: Replace the long feature description in the v0.0.119 HISTORY.md row
with a single concise closeout line summarizing the addition; e.g., one sentence
noting the new JSON typed accessors (json_as_string, json_as_number,
json_as_bool, json_as_int, json_as_array, json_as_object) and the five
convenience getters (json_get_string, json_get_number, json_get_bool,
json_get_int, json_get_array), keeping only the brief one-line entry required by
the project closeout format.

In `@spec/09-standard-library.md`:
- Around line 1841-1842: The spec text for json_as_int is wrong about the finite
overflow boundary; update the wording for json_as_int (and any related
explanation of float_to_int / i64.trunc_f64_s) to state that finite overflow
occurs when f < -2^63 or f >= 2^63 (i.e., -2^63 is representable), and that
json_as_int should return Some(INT64_MIN) for exactly -2^63 while returning None
for values below -2^63 or >= 2^63; ensure this aligns with the existing guards
(float_is_nan, float_is_infinite and the explicit range checks) and the behavior
validated by test_json_as_int_boundary_minus_2_63_is_representable and the
prelude implementation.

In `@TESTING.md`:
- Line 11: The documentation has an inconsistent conformance count: update the
stale phrase "collection of 79" to "collection of 80" so it matches the earlier
line that reads "80 programs across 9 spec chapters"; search TESTING.md for any
other occurrences of "79" or "collection of 79" and replace them with "80"
(verify phrases like "Conformance programs" and nearby sentences remain
grammatically correct after the change).

In `@tests/test_codegen.py`:
- Around line 12473-12495: The test
test_json_as_int_boundary_minus_2_63_is_representable should assert a concrete
observable property of the returned Int instead of only checking Some(_); change
the match on json_as_int(JNumber(...)) to, using the existing `#475` workaround
path, convert the matched Int value to a boolean/string probe (e.g. via
bool_to_string(Int.0 < 0) or equivalent) so you verify the value is negative (or
otherwise uniquely identifies INT64_MIN) when Some(`@Int`) is returned; apply the
same pattern to the analogous test in tests/test_browser.py and keep the inline
`#475` comment explaining why the indirect probe is used.
🪄 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: c1f23a66-a449-4b21-8649-b49037d25ab8

📥 Commits

Reviewing files that changed from the base of the PR and between 6da9f21 and d5192e3.

📒 Files selected for processing (8)
  • HISTORY.md
  • README.md
  • ROADMAP.md
  • TESTING.md
  • scripts/check_skill_examples.py
  • spec/09-standard-library.md
  • tests/test_browser.py
  • tests/test_codegen.py

Comment thread HISTORY.md Outdated
Comment thread spec/09-standard-library.md Outdated
Comment thread TESTING.md
Comment thread tests/test_codegen.py
aallan and others added 2 commits April 23, 2026 14:48
Verified each finding; fixed three; dismissed one as contradicting
established table format.

Real fixes:
- TESTING.md line 85: "collection of 79 small" was stale; bumped
  to 80 to match the header "80 programs across 9 spec chapters"
  two lines above.  One of the conformance-count references that
  slipped through the earlier count-sync sweep.
- spec/09-standard-library.md §9.6.23: the boundary wording for
  json_as_int was imprecise.  Rewrote to state the exact i64 range
  ([-2^63, 2^63) — closed-open), explain the asymmetry (two's
  complement can represent INT64_MIN = -2^63 but not +2^63), and
  add an explicit note that json_as_int(JNumber(-2^63)) correctly
  returns Some(INT64_MIN).  The "|f| >= 2^63" phrasing was
  technically wrong — it conflated f = -2^63 (representable) with
  f = +2^63 (not).
- tests/test_codegen.py test_json_as_int_boundary_minus_2_63_is_
  representable: previously only asserted Some(_), which failed to
  pin the observed value at INT64_MIN.  Added two indirect probes
  that sidestep #475 bug 9 (int_to_string(INT64_MIN) negation
  overflow):
    (a) @Int.0 < 0 holds (bool_to_string -> "true");
    (b) @Int.0 + 1 = INT64_MIN + 1 = -9223372036854775807, which IS
        serialisable without hitting the bug.
  Assertion now: _run_io(src) == "true,-9223372036854775807".  Any
  silent truncation or off-by-one guard would fail at least one
  probe.  Mirror probe applied to the browser test in the same
  class.

Dismissed:
- HISTORY.md line 256: CR claimed the row should be a "single
  concise closeout line" per "the project closeout format".  That
  claim doesn't match the actual format — every Stage 11 entry
  (v0.0.112 through v0.0.118) is a detailed paragraph describing
  what shipped, what was learned, and what's deferred.  My v0.0.119
  entry matches this established convention.  Shortening it would
  break consistency, not restore it.  No change.

All 3,520 tests pass; mypy clean; all 14 doc validation scripts
green.

Co-Authored-By: Claude <noreply@anthropic.invalid>
Stage 1 through ~Stage 8 entries are one or two sentences per row.
Stage 11 entries had drifted into essay-length paragraphs — CR
was right (twice now) that this is wrong; I was wrong to defend
it as 'established format'.  The established format is short.

Reducing the v0.0.119 row from ~240 words to ~70 restores
consistency with the file's actual convention.  A pass over the
Stage 11 rows as a whole is a separate cleanup — out of scope
for this PR but worth tracking.

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

@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 the current code and only fix it if needed.

Inline comments:
In `@tests/test_codegen.py`:
- Around line 12299-12301: Update the class contract comment that describes
json_as_int/float_to_int overflow handling: replace the incorrect inequality
"`|f| >= 2^63`" with the inclusive/exclusive bounds phrasing such as "`f >= 2^63
or f < -2^63`" (or “outside `[-2^63, 2^63)`”) to match the tests like
test_json_as_int_boundary_minus_2_63_is_representable and
test_json_as_int_boundary_just_below_minus_2_63 and the behavior in
json_as_int/float_to_int.
- Around line 12423-12435: The test test_json_as_int_infinity_returns_none only
checks +infinity; add a negative-infinity case to cover -infinity for
json_as_int by extending or creating a test (e.g., in
test_json_as_int_infinity_returns_none) that matches
json_as_int(JNumber(-infinity())) and asserts the function returns the None
branch (expect value 1), referencing the existing
test_json_as_int_infinity_returns_none and the json_as_int/JNumber(infinity())
pattern so both +infinity and -infinity are covered.
🪄 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: c017044e-30a8-404e-a123-adf3263d1bfd

📥 Commits

Reviewing files that changed from the base of the PR and between d5192e3 and 3cf5a13.

📒 Files selected for processing (4)
  • TESTING.md
  • spec/09-standard-library.md
  • tests/test_browser.py
  • tests/test_codegen.py

Comment thread tests/test_codegen.py Outdated
Comment thread tests/test_codegen.py Outdated
Trim Stage 10 + 11 HISTORY rows to match early-stage format:
- The table format established in Stage 1 is one to two sentences
  per row (headline + key details).  Stage 10 and 11 entries had
  drifted into essay-length paragraphs (some 240+ words).  Shortened
  every Stage 10 and Stage 11 row to the early-stage shape —
  headline (bolded feature / issue link), one sentence of detail,
  optional reference to tracking issues.  Detailed narrative stays
  in the stage-intro paragraphs, CHANGELOG, and the GitHub release.
- Net diff across the two stages: ~2,000 words → ~600 words.
- v0.0.119 row stays at its earlier shortening.
- #512 filed to track the convention going forward (brief
  summary in HISTORY; the essay in CHANGELOG).

Two new CR findings addressed:
- tests/test_codegen.py TestJsonTypedAccessors docstring: the
  "finite overflow (|f| >= 2^63)" phrasing conflated -2^63
  (representable as INT64_MIN) with +2^63 (not representable).
  Rewrote to state the exact closed-open i64 range ([-2^63, 2^63))
  and the precise trap condition (f >= 2^63 or f < -2^63).
- tests/test_codegen.py test_json_as_int_infinity_returns_none:
  previously only exercised +infinity.  Extended to cover both
  signs — both are distinct trap inputs from finite-overflow and
  both are caught by float_is_infinite (sign-agnostic).  Browser
  parity test already covered both ±inf.

All 3,520 tests pass; mypy clean; all 14 doc validation scripts
green.

Co-Authored-By: Claude <noreply@anthropic.invalid>
@aallan aallan merged commit eef844b into main Apr 23, 2026
20 checks passed
@aallan aallan deleted the json-typed-accessors branch April 23, 2026 14:58
aallan added a commit that referenced this pull request Apr 23, 2026
Two new issues surfaced by the same agent run:

- #515: $gc_collect itself faults with out-of-bounds memory access
  under sustained allocation pressure.  40×20 Conway's Game of Life
  over 200 generations reliably reproduces.  The collector walks
  past $heap_ptr to the linear-memory bound and traps — gc_collect
  is the top frame of the crashing stack, not the program.  This
  is distinct from #487 (alloc path) and #484 (sweeper 16-bit
  truncation).
- #516: Runtime traps bubble up as raw wasmtime stack traces; the
  CLI mis-labels every Trap/WasmtimeError as "Runtime contract
  violation" even when the actual cause is out-of-bounds memory
  access, integer overflow, unreachable, etc.  No source-line
  mapping, no actionable guidance — the exact opposite of the
  carefully crafted compile-time diagnostics.  Three-stage
  proposed scope: categorise trap reason, source-map the Vera
  function that trapped, specialise help for common trap classes.

KNOWN_ISSUES.md updated with rows for both.

ROADMAP.md implementation-order table re-sorted:
  1. #514 (nested closures + captured-scalar indirection)
  2. #515 (GC collect faults)
  3. #516 (runtime trap diagnostics)
  4. #475 (WASM translator bug cleanup — previously #1)
  5. #507 (Eq/Ord-dispatched array ops)

The top three are all surfaced by the single Game of Life agent
run — the empirical signal is the clearest prioritisation we've
had.  #475 was promoted to #1 after PR #511 merged; pushed to #4
as the new issues outrank it on agent-adoption impact.

Agent-noted quote, worth capturing as the motivation for #516
specifically:

  "the gap between 'the type system is happy' and 'the compiled
   artefact actually runs' is wider than you'd expect from a
   language with SMT-verified contracts. The verifier can prove
   your termination argument is sound while the codegen silently
   miscompiles your closure environment out from under you."

Also posted comment on #514 documenting the third shape the
agent is now isolating (closure body with i32/i64 type mismatch,
distinct from direct-nesting and captured-scalar-indirection).

No code changes.

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.

JSON typed accessors: json_as_string, json_get_number, etc.

1 participant