Skip to content

Add Decimal type with 14 built-in operations#340

Merged
aallan merged 9 commits into
mainfrom
feat/decimal-type
Mar 24, 2026
Merged

Add Decimal type with 14 built-in operations#340
aallan merged 9 commits into
mainfrom
feat/decimal-type

Conversation

@aallan

@aallan aallan commented Mar 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • Decimal type (#333) — 14 host-import operations for exact decimal arithmetic, following the Map/Set opaque handle pattern
  • Operations: decimal_from_int, decimal_from_float, decimal_from_string, decimal_to_string, decimal_to_float, decimal_add, decimal_sub, decimal_mul, decimal_div, decimal_neg, decimal_compare, decimal_eq, decimal_round, decimal_abs
  • Runtime: Python uses decimal.Decimal; browser uses string-based arithmetic
  • Tests: 17 new unit tests (7 checker + 10 codegen), conformance test ch09_decimal (59 programs)
  • Spec: §9.7.2 updated with full documentation of all operations
  • Docs: SKILL.md, ROADMAP, CHANGELOG, TESTING.md all updated
  • Version: Bumped to v0.0.97
  • ROADMAP: Ticked off Standard library collections (Set, Map, Decimal) #62 Map in the ASCII diagram

Known limitations

  • Option<Decimal> from decimal_div/decimal_from_string cannot be used with match or option_unwrap_or due to monomorphization gaps (Decimal is not recognized as a monomorphizable type parameter)
  • Ordering from decimal_compare cannot be used with match for the same reason
  • These limitations affect all new opaque types and are tracked as a broader codegen issue

Closes #333

Test plan

  • All 21 pre-commit hooks pass
  • 2,853 tests pass
  • 59 conformance programs pass
  • 26 examples pass
  • mypy clean
  • CI passes

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added a Decimal type for exact decimal arithmetic with 14 built-in operations and browser runtime support.
  • Documentation

    • Spec, guides and READMEs updated with Decimal docs, examples and a limitation about using Option/Ordering with generic match/unwrapping.
  • Tests

    • Conformance suite increased to 59 programs; new unit and end-to-end tests cover Decimal behaviour and edge cases.
  • Chores

    • Project version bumped to 0.0.97.

14 host-import operations for exact decimal arithmetic:
decimal_from_int, decimal_from_float, decimal_from_string,
decimal_to_string, decimal_to_float, decimal_add, decimal_sub,
decimal_mul, decimal_div, decimal_neg, decimal_compare, decimal_eq,
decimal_round, decimal_abs.

Same opaque i32 handle pattern as Map/Set. Python runtime uses
decimal.Decimal; browser runtime uses string-based arithmetic.
New conformance test ch09_decimal (59 programs). 17 unit tests.
Spec §9.7.2 updated with full documentation. Tick #62 Map in
ROADMAP diagram. Bump version to 0.0.97.

Closes #333

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

coderabbitai Bot commented Mar 24, 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 an opaque Decimal type with 14 host-imported built-ins for exact decimal arithmetic. Changes touch the type environment, inference, WASM lowering, codegen/import tracking, browser runtime host functions, tests, spec/docs, adds a conformance test ch09_decimal, and bumps release to 0.0.97.

Changes

Cohort / File(s) Summary
Documentation & Version
CHANGELOG.md, ROADMAP.md, SKILL.md, TESTING.md, CLAUDE.md, README.md, vera/README.md, pyproject.toml, vera/__init__.py
Bump release to v0.0.97; document the new Decimal type and its 14 built-ins; update conformance/test counts (58→59) and roadmap/spec references; add limitation notes about opaque-handle monomorphization.
Specification
spec/09-standard-library.md
Promote Decimal from “Future” to a full built-in: list constructors, conversions, arithmetic, comparison, rounding, examples, and known limitations (Option/Ordering monomorphization).
Type Environment & Checker
vera/environment.py, vera/checker/resolution.py
Register Decimal and Option<Decimal>/Ordering builtins; make Decimal non‑parameterised and emit an error for type arguments.
Type Inference & WASM Slot Mapping
vera/wasm/inference.py, vera/wasm/context.py
Map Vera Decimal → WASM i32 handles; infer wasm/vera return types for decimal builtins; add _decimal_imports and _decimal_ops_used tracking in WasmContext.
WASM Lowering / Translation
vera/wasm/calls.py
Translate decimal_* calls to WASM calls, register host-import signatures, desugar show(Decimal)decimal_to_string, and mark allocation/memory needs for handle results.
Codegen Tracking & Assembly
vera/codegen/core.py, vera/codegen/compilability.py, vera/codegen/functions.py, vera/codegen/modules.py, vera/codegen/assembly.py
Propagate decimal import/op usage through codegen; treat Decimal as opaque i32; include decimal imports in emitted WAT and export $alloc when decimal ops require allocation.
WASM Host API & Runtime
vera/codegen/api.py, vera/browser/runtime.mjs
Add host-side decimal handle store and allocation helpers; register host functions for constructors, conversions, arithmetic, compare/eq, round, abs/neg, and Option/Ordering allocation; JS runtime implements runtime semantics (string-backed decimal representations, Number for arithmetic in browser MVP).
Tests & Conformance
tests/test_checker.py, tests/test_codegen.py, tests/conformance/manifest.json
Add extensive checker and codegen/runtime tests for decimal ops (happy and error paths); add ch09_decimal conformance entry and update test manifests.
Tooling Allowlists & Examples
scripts/check_skill_examples.py, scripts/check_spec_examples.py, scripts/check_readme_examples.py, tests/test_readme.py
Update allowlist (filename, line) mappings and add new allowlisted fragments for Decimal examples to match docs reorder.
Browser & Package Metadata
vera/browser/runtime.mjs, pyproject.toml, vera/__init__.py
Browser runtime host-import wiring for decimal ops; package version bump to 0.0.97 and vera.__version__ update.

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer (compile)
  participant Compiler as Vera Compiler
  participant WASM as Generated WASM Module
  participant Host as Host Runtime (browser JS)

  Dev->>Compiler: compile Vera program using Decimal ops
  Compiler->>Compiler: infer types, mark decimal_ops_used & register imports
  Compiler->>WASM: emit module with import "vera".decimal_* entries
  WASM->>Host: call host `decimal_*` function at runtime
  Host->>Host: map handle ↔ value, perform arithmetic/compare, allocate Option/Ordering ADTs
  Host-->>WASM: return i32 handles / pair results / ADT pointers
  WASM-->>Dev: program runs with Decimal semantics
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

compiler, tests, spec, ci, docs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 78.30% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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 change: adding a Decimal type with 14 built-in operations (constructors, arithmetic, comparisons, and utilities). It is concise and specific.
Linked Issues check ✅ Passed The PR fully addresses issue #333: implements the Decimal type [#333], documents 14 host-import operations in the spec [#333], implements runtime support across targets using the opaque-handle pattern [#333], and maintains independence from Map/Set work [#333].
Out of Scope Changes check ✅ Passed All changes are directly scoped to Decimal type support: spec updates (§9.7.2), runtime implementations (Python and browser), type checker/codegen integration, conformance tests, documentation, and version bumps. No unrelated changes are present.

✏️ 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 feat/decimal-type

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

@aallan

aallan commented Mar 24, 2026

Copy link
Copy Markdown
Owner Author

Decimal completes the standard library collections from issue #62, Map (v0.0.94), Set (v0.0.95), and Decimal (v0.0.97). All three use the same host-import pattern: Python/JS runtimes maintain native data structures behind opaque i32 handles. The WASM module never touches the actual values; all operations are delegated to the host via function calls.

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

codecov Bot commented Mar 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.11858% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.37%. Comparing base (9161a87) to head (45fb4c4).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
vera/wasm/inference.py 51.85% 13 Missing ⚠️
vera/wasm/calls.py 89.58% 10 Missing ⚠️
vera/codegen/api.py 97.84% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #340      +/-   ##
==========================================
- Coverage   95.43%   95.37%   -0.06%     
==========================================
  Files          45       45              
  Lines       15892    16141     +249     
==========================================
+ Hits        15167    15395     +228     
- Misses        725      746      +21     

☔ 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.

aallan and others added 2 commits March 24, 2026 16:22
Co-Authored-By: Claude <noreply@anthropic.invalid>
…, compare branches)

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: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
vera/checker/resolution.py (1)

80-85: ⚠️ Potential issue | 🟠 Major

Reject type arguments on Decimal to preserve type-system correctness.

This branch currently accepts parameterised Decimal forms by constructing AdtType("Decimal", args). For an opaque non-generic Decimal type, Decimal<...> should be rejected with a diagnostic.

🔧 Proposed fix
-        # Array, Tuple, Map, Set, Decimal (built-in non-primitive types)
-        if name in ("Array", "Tuple", "Map", "Set", "Decimal"):
+        # Decimal (built-in opaque, non-parameterised type)
+        if name == "Decimal":
+            if te.type_args:
+                self._error(
+                    te,
+                    "Decimal does not take type arguments.",
+                    rationale="Decimal is an opaque built-in type.",
+                    fix="Use 'Decimal' without type parameters.",
+                )
+                return UnknownType()
+            return AdtType(name, ())
+
+        # Array, Tuple, Map, Set (built-in non-primitive types)
+        if name in ("Array", "Tuple", "Map", "Set"):
             if te.type_args:
                 args = tuple(self._resolve_type(a) for a in te.type_args)
                 return AdtType(name, args)
             return AdtType(name, ())
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vera/checker/resolution.py` around lines 80 - 85, In _resolve_type, treat
Decimal as an opaque non-generic ADT: when name == "Decimal" and te.type_args is
non-empty, emit a diagnostic using the checker’s existing reporting mechanism
(e.g. self.diagnostic / self.report_error) indicating that Decimal does not
accept type arguments, and return the non-parameterised form (AdtType("Decimal",
())) instead of constructing AdtType with args; otherwise continue returning
AdtType("Decimal", ()) as before.
SKILL.md (1)

1576-1576: ⚠️ Potential issue | 🟡 Minor

Update conformance suite count in documentation.

Line 1576 states the conformance directory contains 39 programmes, but it currently contains 59. Update to reflect the actual count.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@SKILL.md` at line 1576, Update the documentation sentence in SKILL.md that
currently reads "The `tests/conformance/` directory contains 39 small,
self-contained programs..." to reflect the current count by replacing "39" with
"59" so it reads "contains 59 small, self-contained programs..."; locate the
exact sentence mentioning `tests/conformance/` and change only the numeric
count.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ROADMAP.md`:
- Line 78: The roadmap line about Decimal’s monomorphization gaps is incomplete:
update the sentence referencing Option<Decimal> to state that Option<Decimal>
returned from decimal_div/decimal_from_string and Ordering from decimal_compare
cannot currently be used with match or option_unwrap_or due to monomorphization
gaps; specifically mention Option<Decimal>, decimal_div, decimal_from_string,
decimal_compare, Ordering, match, and option_unwrap_or so readers know both
match and option_unwrap_or are affected.

In `@scripts/check_skill_examples.py`:
- Around line 46-47: The ALLOWLIST dict contains duplicate numeric keys (e.g.,
974, 1274, 1364 and the groups around 99/119-127) so later entries silently
override earlier reasons; fix by collapsing duplicates into a single entry per
key (merge the reasons into one list/string) or rename keys so each line number
is unique, and update any references that expect the original entries (search
for the ALLOWLIST definition and entries named 974, 1274, 1364 and the
99/119-127 groups to perform the merge/rename).

In `@SKILL.md`:
- Around line 556-561: Update SKILL.md examples to document the codegen
limitation for decimal_compare: after the existing line that shows
"decimal_compare(`@Decimal.0`, `@Decimal.1`) -- returns Ordering", add a brief note
that pattern matching (match) on the Ordering result is not yet supported by
codegen (reference Ordering and match) and advise users to avoid match on
decimal_compare outputs; also add explicit example signatures for
decimal_from_string and decimal_div (showing their parameter lists and that they
return Option<Decimal>) alongside the existing warning about Option<Decimal>
usage and mention that Option<Decimal> cannot be used with option_unwrap_or or
match due to monomorphization limitations (reference decimal_from_string,
decimal_div, and option_unwrap_or) so readers see both the functions and their
current restrictions.

In `@spec/09-standard-library.md`:
- Line 1561: Update the note in §9.7.2 to also mention that `Ordering` values
produced by `decimal_compare` suffer the same monomorphization gap as
`Option<Decimal>` (i.e., cannot be used with `option_unwrap_or`/`match` because
the monomorphizer does not produce `option_unwrap_or$Decimal` or the equivalent
for `Ordering`); specifically add `Ordering`/`decimal_compare` to the known
limitation sentence alongside `Option<Decimal>` and the functions `decimal_div`
and `decimal_from_string` so the limitation is complete and accurate.

In `@TESTING.md`:
- Line 36: Update the outdated conformance program counts in the README: change
the phrase "58 small, focused programs" found at the sentence referenced by line
80 to "59 small, focused programs" and similarly update the other two
occurrences (the references currently at lines 307 and 349) to read 59 so all
counts match the actual number of `.vera` files in tests/conformance; ensure any
adjacent text that references the count (e.g., summaries, totals or parentheses)
is updated to use 59 as well.

In `@tests/test_checker.py`:
- Around line 1710-1767: The TestDecimalChecker currently covers only some
Decimal built-ins; add tests in tests/test_checker.py under the
TestDecimalChecker class for the remaining functions (create a pair of tests for
each symbol): decimal_from_float, decimal_from_string, decimal_sub, decimal_mul,
decimal_div, decimal_neg, decimal_compare, decimal_round, and decimal_abs. For
each built-in add one _check_ok case exercising correct types (e.g., calling
decimal_sub(decimal_from_int(2), decimal_from_int(1)) or
decimal_from_float(1.23)) and one _check_err case passing a wrong arg type
(e.g., plain Int or String) asserting the checker returns a "type" (or "expects"
for arity errors) message; name tests descriptively like test_decimal_sub and
test_decimal_sub_wrong_type to match the existing pattern and use the same
_check_ok/_check_err helpers and docstring style so they integrate with the
existing TestDecimalChecker.

In `@tests/test_codegen.py`:
- Around line 8177-8187: The test only checks negation of 42 and misses the -0
path which the browser runtime produces as "-0" string; update tests to include
the zero edge-case by adding an assertion that decimal_neg(decimal_from_int(0))
compares equal to decimal_from_int(0) (via decimal_eq) or add a separate test
function (e.g., test_decimal_neg_zero) that calls
decimal_neg(decimal_from_int(0)) and asserts equality using decimal_eq so the
browser runtime's "-0" is exercised and fails if not normalized.
- Around line 8214-8221: The test test_decimal_to_float is too weak (uses floor)
and should assert exact conversion results: change the assertion to compare
decimal_to_float(decimal_from_int(42)) exactly to 42.0 (or the expected float
representation) and add an exact-string assertion for
decimal_to_string(decimal_from_int(42)) == "42"; also add extra cases for zero
and a negative value (e.g., decimal_from_int(0) -> 0.0 and decimal_from_int(-7)
-> -7.0) to cover edge behavior and composition with decimal_to_string so broken
implementations like 42.9 or "24" fail; locate and edit the test methods
test_decimal_to_float and the related test around decimal_to_string to implement
these exact equality checks.

In `@vera/browser/runtime.mjs`:
- Around line 1373-1463: The decimal helpers currently convert decimal strings
to JS Number in functions like decimal_add, decimal_sub, decimal_mul,
decimal_div, decimal_round, decimal_to_float and decimal_compare which
reintroduces IEEE-754 errors and can produce unnormalized "-0"; instead
implement all arithmetic and rounding using string-based or BigInt-safe routines
(use or expand decStrAdd, decStrSub, decStrMul, decStrDiv and a string-based
round) so operations on values stored in decimalStore never pass through Number;
also normalize results (e.g., convert "-0" -> "0", trim leading zeros, and a
canonical sign/scale) and update decimal_eq and decimal_to_string to
compare/return normalized strings; keep decimal_div returning allocOptionNone()
on exact division-by-zero as before.

In `@vera/codegen/api.py`:
- Around line 1347-1368: The host function host_decimal_from_string duplicates
raw memory access; replace the manual memory reads (the bytes/mem.data_ptr and
raw.decode steps) with the existing _read_wasm_string helper to get the UTF-8
string. In other words, inside host_decimal_from_string (the
wasmtime.Caller-based function handling ptr and length) call s =
_read_wasm_string(caller, ptr, length) and keep the try/except InvalidOperation,
_decimal_alloc(d), and the _alloc_option_some_i32/_alloc_option_none behavior
unchanged; remove the direct mem access lines so the function uses the helper
for consistent WASM memory reads.
- Around line 1497-1511: The host_decimal_round handler can raise
decimal.InvalidOperation when places is extreme; update the host_decimal_round
implementation (registered via linker.define_func for "vera","decimal_round") to
validate or clamp the places parameter and/or wrap the quantize call in a
try/except: fetch d from _decimal_store[h], compute a safe clamped_places (e.g.
limit to a reasonable min/max), build q from that clamped value, call
d.quantize(q) inside try/except catching decimal.InvalidOperation and return a
safe fallback (for example the original _decimal_alloc(d) or an error sentinel)
using the existing _decimal_alloc logic so WASM traps are prevented.

In `@vera/wasm/calls.py`:
- Around line 7305-7314: The Decimal host-import translators that return
heap-backed data (e.g., _translate_decimal_from_string and the other translators
that call _register_decimal_import for Decimal functions returning
Option/Ordering pointers or String pairs) must set self.needs_alloc = True so
the allocator is enabled; after calling _register_decimal_import (the call that
produces wasm_name) and before returning the instruction list, set
self.needs_alloc = True and then return arg_instrs + [f"call {wasm_name}"].
Ensure this change is applied to each translator that registers Decimal imports
which return heap-backed values.

In `@vera/wasm/inference.py`:
- Around line 857-858: _infer_fncall_wasm_type currently doesn't return types
for Decimal builtins; add explicit cases mapping builtin names to the correct
WASM return types: return "i32" for decimal_from_int, decimal_add, decimal_sub,
decimal_mul, decimal_neg, decimal_round, decimal_abs, decimal_from_string,
decimal_div; return "f64" for decimal_to_float; return "i32" for decimal_eq and
decimal_compare; and return "i32_pair" for decimal_to_string. Place these checks
alongside the other builtin handlers inside _infer_fncall_wasm_type (use the
builtin name strings to locate the switch/if chain). Also simplify the
host-import handle check that now does multiple startswith calls (the one
checking Map<, Set<, Decimal) by using a single startswith with a tuple or an
any(...) pattern to make it clearer and consistent.

---

Outside diff comments:
In `@SKILL.md`:
- Line 1576: Update the documentation sentence in SKILL.md that currently reads
"The `tests/conformance/` directory contains 39 small, self-contained
programs..." to reflect the current count by replacing "39" with "59" so it
reads "contains 59 small, self-contained programs..."; locate the exact sentence
mentioning `tests/conformance/` and change only the numeric count.

In `@vera/checker/resolution.py`:
- Around line 80-85: In _resolve_type, treat Decimal as an opaque non-generic
ADT: when name == "Decimal" and te.type_args is non-empty, emit a diagnostic
using the checker’s existing reporting mechanism (e.g. self.diagnostic /
self.report_error) indicating that Decimal does not accept type arguments, and
return the non-parameterised form (AdtType("Decimal", ())) instead of
constructing AdtType with args; otherwise continue returning AdtType("Decimal",
()) as before.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e5889a9f-f312-4289-9f8e-c18686d159fb

📥 Commits

Reviewing files that changed from the base of the PR and between 9161a87 and cc091d1.

⛔ 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/**
  • tests/conformance/ch09_decimal.vera is excluded by !**/*.vera
📒 Files selected for processing (25)
  • CHANGELOG.md
  • CLAUDE.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_checker.py
  • tests/test_codegen.py
  • vera/__init__.py
  • vera/browser/runtime.mjs
  • vera/checker/resolution.py
  • vera/codegen/api.py
  • vera/codegen/assembly.py
  • vera/codegen/compilability.py
  • vera/codegen/core.py
  • vera/codegen/functions.py
  • vera/codegen/modules.py
  • vera/environment.py
  • vera/wasm/calls.py
  • vera/wasm/context.py
  • vera/wasm/inference.py

Comment thread ROADMAP.md Outdated
Comment thread scripts/check_skill_examples.py
Comment thread SKILL.md Outdated
Comment thread spec/09-standard-library.md Outdated
Comment thread TESTING.md
Comment thread vera/browser/runtime.mjs
Comment thread vera/codegen/api.py
Comment thread vera/codegen/api.py
Comment thread vera/wasm/calls.py
Comment thread vera/wasm/inference.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: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
README.md (1)

515-515: ⚠️ Potential issue | 🟡 Minor

Update version number to v0.0.97.

The PR summary explicitly states "Version bumped to v0.0.97", but this line still shows v0.0.96. This creates a factual inconsistency between the documented version and the actual release version.

📝 Proposed fix
-Vera is in **active development** at v0.0.96. The reference compiler — parser, AST, type checker, contract verifier (Z3), WASM code generator, module system, browser runtime, and runtime contract insertion — is working. Programs compile to WebAssembly and execute via wasmtime or in the browser.
+Vera is in **active development** at v0.0.97. The reference compiler — parser, AST, type checker, contract verifier (Z3), WASM code generator, module system, browser runtime, and runtime contract insertion — is working. Programs compile to WebAssembly and execute via wasmtime or in the browser.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 515, Update the documented release version string from
"v0.0.96" to "v0.0.97" in the README entry that currently reads "Vera is in
**active development** at v0.0.96." so the text matches the PR summary; locate
and replace the exact token v0.0.96 with v0.0.97 in README.md (the
human-readable release/version line).
♻️ Duplicate comments (3)
TESTING.md (1)

11-11: ⚠️ Potential issue | 🟡 Minor

Conformance count is still inconsistent in this document.

These updated lines use 59, but Line 80, Line 307, and Line 349 still say 58. Please align all references to 59 to keep TESTING.md internally consistent.

As per coding guidelines, "**/*.md: Review Markdown files for factual accuracy against the codebase, broken links, and outdated information."

Also applies to: 36-36, 117-125

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@TESTING.md` at line 11, Update the inconsistent conformance program count in
TESTING.md so all mentions read "59" instead of "58": search for occurrences of
the phrase "Conformance programs" and the numeric "58" (notably the lines
referenced around Line 80, Line 307, Line 349 and the ranges around 36 and
117-125) and replace them with "59" so the table row "| **Conformance programs**
| 59 programs across 9 spec chapters, validating every language feature |" and
all standalone counts are consistent across the file.
tests/test_codegen.py (2)

8177-8187: ⚠️ Potential issue | 🟠 Major

Add the zero-edge regression for decimal_neg.

Line 8177 only validates neg(42), but it misses the zero path (-0) where host/runtime normalisation bugs usually surface. Please add an explicit decimal_neg(decimal_from_int(0)) equality assertion against zero.

As per coding guidelines, "Add codegen/runtime tests in tests/test_codegen.py when adding or modifying built-in functions — cover normal cases, edge cases (empty inputs, zero values), and composition with other built-ins".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_codegen.py` around lines 8177 - 8187, The test test_decimal_neg
only checks neg(42) but omits the zero edge-case where -0 can surface
normalization bugs; update the test_decimal_neg source to also compute
decimal_neg(decimal_from_int(0)) and assert via decimal_eq that it equals
decimal_from_int(0) (e.g., include a separate equality check or combine into the
existing if) so the call to decimal_neg and decimal_eq covers both 42 and 0
cases when invoking _run.

8214-8221: ⚠️ Potential issue | 🟠 Major

Strengthen conversion assertions to exact-value checks.

Line 8219 and Line 8237 currently allow incorrect implementations through (floor(...) and length-only checks). These should assert exact conversion values/strings, including zero/negative edge cases.

Suggested tightening
-    def test_decimal_to_float(self) -> None:
-        """Convert decimal 42 to float and check via floor."""
+    def test_decimal_to_float(self) -> None:
+        """decimal_to_float preserves exact numeric value for integer decimals."""
         source = """
-public fn main(-> `@Int`)
+public fn main(-> `@Float64`)
   requires(true) ensures(true) effects(pure)
-{ floor(decimal_to_float(decimal_from_int(42))) }
+{ decimal_to_float(decimal_from_int(42)) }
 """
-        assert _run(source) == 42
+        assert _run_float(source) == 42.0

-    def test_decimal_to_string_length(self) -> None:
-        """string_length(decimal_to_string(decimal_from_int(42))) > 0."""
-        source = """
-public fn main(-> `@Int`)
-  requires(true) ensures(true) effects(pure)
-{ string_length(decimal_to_string(decimal_from_int(42))) }
-"""
-        assert _run(source) == 2
+    def test_decimal_to_string(self) -> None:
+        """decimal_to_string renders exact decimal text."""
+        source = _IO_PRELUDE + """
+public fn main(`@Unit` -> `@Unit`)
+  requires(true) ensures(true) effects(<IO>)
+{ IO.print(decimal_to_string(decimal_from_int(42))) }
+"""
+        assert _run_io(source, fn="main") == "42"

As per coding guidelines, "DO flag: Tests that don't assert anything meaningful" and "Add codegen/runtime tests in tests/test_codegen.py when adding or modifying built-in functions — cover normal cases, edge cases (empty inputs, zero values), and composition with other built-ins".

Also applies to: 8232-8239, 8241-8249

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_codegen.py` around lines 8214 - 8221, The test_decimal_to_float
currently asserts via floor(decimal_to_float(decimal_from_int(42))) which allows
incorrect implementations; update test_decimal_to_float to assert the exact
output value from _run (use direct equality to 42.0 or appropriate language
literal) instead of wrapping with floor, and add sibling tests that call
decimal_to_float/decimal_from_int for zero and negative cases (e.g.,
decimal_from_int(0) -> 0.0, decimal_from_int(-1) -> -1.0) to cover edge cases;
locate and modify the test function named test_decimal_to_float and any similar
tests around lines referencing decimal_to_float, decimal_from_int, floor, and
_run to replace length/floor-style checks with exact-value assertions for
positive, zero, and negative inputs.
🤖 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 8277-8289: The test test_decimal_compare_less currently only calls
decimal_eq and doesn't exercise decimal_compare; update the test source so it
constructs two decimals via decimal_from_int (as already done) but calls
decimal_compare(`@Decimal.0`, `@Decimal.1`) and checks the returned ordering
(expecting a negative/“less than” result) instead of using decimal_eq; ensure
the assertion uses _run(...) to verify the decimal_compare return value (e.g.,
expecting -1 or whatever the host import returns for less-than) so the test
actually validates ordering and host import wiring for decimal_compare.
- Around line 8263-8275: The test function test_decimal_div_nonzero currently
performs decimal_add/decimal_round and never calls decimal_div; change the test
body so it actually invokes decimal_div (e.g., create `@Decimal` from 10 and 2,
call decimal_div(`@Decimal.0`, `@Decimal.1`) and then check decimal_round(..., 0) ==
decimal_from_int(5)) so the test calls decimal_div and still returns 1 on
success (update the comparison literal from 12 to 5); reference the test
function name test_decimal_div_nonzero, the helper _run, and the functions
decimal_from_int, decimal_div, decimal_round in your edit.
- Around line 8250-8262: The test test_decimal_from_string_valid is wrong
because it never calls decimal_from_string; update the test to invoke
decimal_from_string (not decimal_from_int) with at least one valid string (e.g.,
"0" and "42" or a single valid decimal string) and assert the result is the
expected Some/tag=1 value; use the existing decimal_eq helper (or compare .1/.0
as the pattern in the test) to verify equality and keep the test name and intent
unchanged so it actually exercises the decimal_from_string API.

---

Outside diff comments:
In `@README.md`:
- Line 515: Update the documented release version string from "v0.0.96" to
"v0.0.97" in the README entry that currently reads "Vera is in **active
development** at v0.0.96." so the text matches the PR summary; locate and
replace the exact token v0.0.96 with v0.0.97 in README.md (the human-readable
release/version line).

---

Duplicate comments:
In `@TESTING.md`:
- Line 11: Update the inconsistent conformance program count in TESTING.md so
all mentions read "59" instead of "58": search for occurrences of the phrase
"Conformance programs" and the numeric "58" (notably the lines referenced around
Line 80, Line 307, Line 349 and the ranges around 36 and 117-125) and replace
them with "59" so the table row "| **Conformance programs** | 59 programs across
9 spec chapters, validating every language feature |" and all standalone counts
are consistent across the file.

In `@tests/test_codegen.py`:
- Around line 8177-8187: The test test_decimal_neg only checks neg(42) but omits
the zero edge-case where -0 can surface normalization bugs; update the
test_decimal_neg source to also compute decimal_neg(decimal_from_int(0)) and
assert via decimal_eq that it equals decimal_from_int(0) (e.g., include a
separate equality check or combine into the existing if) so the call to
decimal_neg and decimal_eq covers both 42 and 0 cases when invoking _run.
- Around line 8214-8221: The test_decimal_to_float currently asserts via
floor(decimal_to_float(decimal_from_int(42))) which allows incorrect
implementations; update test_decimal_to_float to assert the exact output value
from _run (use direct equality to 42.0 or appropriate language literal) instead
of wrapping with floor, and add sibling tests that call
decimal_to_float/decimal_from_int for zero and negative cases (e.g.,
decimal_from_int(0) -> 0.0, decimal_from_int(-1) -> -1.0) to cover edge cases;
locate and modify the test function named test_decimal_to_float and any similar
tests around lines referencing decimal_to_float, decimal_from_int, floor, and
_run to replace length/floor-style checks with exact-value assertions for
positive, zero, and negative inputs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1a949c16-ac9f-4931-9727-0b9a8317d8c2

📥 Commits

Reviewing files that changed from the base of the PR and between cc091d1 and ebb2f9d.

📒 Files selected for processing (6)
  • README.md
  • TESTING.md
  • scripts/check_readme_examples.py
  • tests/test_codegen.py
  • tests/test_readme.py
  • vera/README.md

Comment thread tests/test_codegen.py Outdated
Comment thread tests/test_codegen.py Outdated
Comment thread tests/test_codegen.py Outdated
@aallan

aallan commented Mar 24, 2026

Copy link
Copy Markdown
Owner Author

Critical (🔴):

  1. Browser runtime JS Number precision (ID 2982857754) — Valid concern: JS Number is IEEE 754, so 0.1 + 0.2 !== 0.3. The browser runtime uses Number() for arithmetic. This is a known accuracy limitation documented as "string-based decimal" but actually using float arithmetic.
  2. needs_alloc not set for Decimal (ID 2982857773) — Need to check if this is actually an issue.

Major (🟠):
3. Duplicate ALLOWLIST keys (ID 2982857696) — Must fix.
4. SKILL.md decimal_compare limitation (ID 2982857703) — Should document.
5. Checker coverage incomplete (ID 2982857731) — Should add more tests.
6. decimal_neg zero-value (ID 2982857738) — Good edge case.
7. Assert exact conversion (ID 2982857743) — Fair point.
8. inference.py (ID 2982857780) — Need to check.

Minor (🟡):
9. ROADMAP limitation wording (ID 2982857677) — Quick fix.
10. Spec limitation note (ID 2982857713) — Quick fix.
11. TESTING.md conformance count (ID 2982857720) — Check.

Trivial (🔵):
12. Use _read_wasm_string (ID 2982857758) — Quick refactor.
13. quantize InvalidOperation (ID 2982857763) — Edge case for extreme exponents.

Outside diff:
14. resolver: reject Decimal — Good, should reject type args.
15. SKILL.md conformance count — Fix "39" → "59".

- Fix 3 duplicate ALLOWLIST keys in check_skill_examples.py
- Set needs_alloc for Decimal ops returning heap values (calls.py)
- Add precision-loss comment to browser runtime decimal arithmetic
- Fix decimal_neg to handle "-0" canonically
- Add option_unwrap_or to ROADMAP/spec/SKILL limitation notes
- Add Ordering/decimal_compare to limitation documentation
- Separate Decimal from parameterised types in resolver
- Use _read_wasm_string helper in host_decimal_from_string
- Wrap decimal_round quantize in try/except for edge cases
- Add 6 new checker tests + 2 codegen tests
- Add Decimal entries to _infer_fncall_wasm_type
- Update SKILL.md conformance count 39→59
- Update TESTING.md counts (2863→2871)
- Regenerate site assets

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

aallan commented Mar 24, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Addressed both outside-diff items:

  1. vera/checker/resolution.py: Separated Decimal from the parameterised types (Array/Tuple/Map/Set). It now has its own check that always returns AdtType("Decimal", ()) regardless of whether type args are provided.

  2. SKILL.md conformance count: Updated from '39' to '59'.

@coderabbitai

coderabbitai Bot commented Mar 24, 2026

Copy link
Copy Markdown

Tip

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

@aallan — both fixes look exactly right:

  1. vera/checker/resolution.py — Giving Decimal its own branch that unconditionally returns AdtType("Decimal", ()) is the correct approach. It means Decimal<Int> (or any other spurious type argument) will be caught cleanly rather than silently accepted via the parameterised-type path shared with Map/Set. Consistent with how Bool, Ordering, etc. are handled as 0-arity ADTs.

  2. SKILL.md conformance count39 → 59 corrected. ✓


🧠 Learnings used
Learnt from: CR
Repo: aallan/vera PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-24T11:56:27.112Z
Learning: Applies to vera/**/*.py : Type-check the compiler codebase with mypy and ensure no type errors remain

…rtions

- 12 new checker tests (happy-path + wrong-type for all Decimal ops)
- Replace test_decimal_to_float with exact 3.5 round-trip via _run_float
- Replace test_decimal_to_string_length with exact IO assertion
- Rename misleading tests that did not call their claimed function
- Update doc counts (2883 tests)

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

♻️ Duplicate comments (8)
tests/test_codegen.py (4)

8276-8288: ⚠️ Potential issue | 🟠 Major

test_decimal_from_string_valid never exercises decimal_from_string.

Line 8282-Line 8284 only compares two decimal_from_int values, so parser/import regressions for decimal_from_string will not be caught.

💡 Proposed fix
     def test_decimal_from_string_valid(self) -> None:
-        """decimal_from_string with valid input returns Some (tag=1)."""
+        """decimal_from_string host call succeeds for valid input."""
         source = """
 public fn main(-> `@Int`)
   requires(true) ensures(true) effects(pure)
 {
-  let `@Decimal` = decimal_from_int(0);
-  let `@Decimal` = decimal_from_int(42);
-  if decimal_eq(`@Decimal.1`, `@Decimal.0`) then { 0 } else { 1 }
+  let `@Option`<Decimal> = decimal_from_string("42");
+  1
 }
 """
         assert _run(source) == 1

As per coding guidelines, "Add codegen/runtime tests in tests/test_codegen.py when adding or modifying built-in functions — cover normal cases, edge cases (empty inputs, zero values), and composition with other built-ins".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_codegen.py` around lines 8276 - 8288, The test named
test_decimal_from_string_valid currently never calls decimal_from_string (it
only uses decimal_from_int), so add a call to decimal_from_string in the test
source and assert the expected result to exercise parsing/import paths; update
the in-test source string inside test_decimal_from_string_valid to invoke
decimal_from_string with a valid string (e.g., "0" and "42") instead of/in
addition to decimal_from_int, then run via _run and adjust the final assertion
to verify the returned tag/value from decimal_from_string (keep function name
test_decimal_from_string_valid, helper _run, and comparisons to decimal_eq as
appropriate).

8303-8315: ⚠️ Potential issue | 🟠 Major

test_decimal_compare_less never calls decimal_compare.

Line 8311 only checks decimal_eq on two different values, so ordering behaviour is untested.

💡 Proposed fix
     def test_decimal_compare_less(self) -> None:
-        """decimal_compare(1, 2) — test indirectly via decimal_eq."""
+        """decimal_compare host call executes for less-than operands."""
         source = """
 public fn main(-> `@Int`)
   requires(true) ensures(true) effects(pure)
 {
-  let `@Decimal` = decimal_from_int(1);
-  let `@Decimal` = decimal_from_int(2);
-  if decimal_eq(`@Decimal.1`, `@Decimal.0`) then { 0 } else { 1 }
+  let `@Ordering` = decimal_compare(decimal_from_int(1), decimal_from_int(2));
+  1
 }
 """
         assert _run(source) == 1

As per coding guidelines, "Add codegen/runtime tests in tests/test_codegen.py when adding or modifying built-in functions — cover normal cases, edge cases (empty inputs, zero values), and composition with other built-ins".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_codegen.py` around lines 8303 - 8315, The test
test_decimal_compare_less never invokes decimal_compare but instead only calls
decimal_eq; update the test source so it constructs two decimals with
decimal_from_int (as currently done) and calls decimal_compare on them (e.g.,
decimal_compare(`@Decimal.0`, `@Decimal.1`)) and assert the runtime result equals
the expected ordering value (e.g., -1 for 1 < 2). Ensure the test uses the
decimal_compare call in the function body and that the final assert in
test_decimal_compare_less checks the returned comparison integer.

8289-8301: ⚠️ Potential issue | 🟠 Major

test_decimal_div_nonzero does not call decimal_div.

Line 8297-Line 8298 performs decimal_add + decimal_round; division logic is not under test.

💡 Proposed fix
     def test_decimal_div_nonzero(self) -> None:
-        """decimal_div by nonzero works (test via decimal_eq on result)."""
+        """decimal_div host call executes for non-zero divisor."""
         source = """
 public fn main(-> `@Int`)
   requires(true) ensures(true) effects(pure)
 {
-  let `@Decimal` = decimal_from_int(10);
-  let `@Decimal` = decimal_from_int(2);
-  let `@Decimal` = decimal_add(`@Decimal.1`, `@Decimal.0`);
-  if decimal_eq(decimal_round(`@Decimal.0`, 0), decimal_from_int(12)) then { 1 } else { 0 }
+  let `@Option`<Decimal> = decimal_div(decimal_from_int(10), decimal_from_int(2));
+  1
 }
 """
         assert _run(source) == 1

As per coding guidelines, "Add codegen/runtime tests in tests/test_codegen.py when adding or modifying built-in functions — cover normal cases, edge cases (empty inputs, zero values), and composition with other built-ins".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_codegen.py` around lines 8289 - 8301, The test
test_decimal_div_nonzero is supposed to exercise decimal_div but currently uses
decimal_add/decimal_round; update the inline Wig source in
test_decimal_div_nonzero to call decimal_div on two nonzero decimals (use
decimal_from_int(10) and decimal_from_int(2)), then assert the result via
decimal_eq (or decimal_round + decimal_eq) to the expected decimal (e.g.,
decimal_from_int(5)). Locate the test by name and replace the decimal_add line
with a call to decimal_div and adjust the final decimal_eq check to compare
against the correct expected value.

8226-8247: ⚠️ Potential issue | 🟠 Major

Strengthen decimal conversion assertions so broken implementations cannot pass.

Line 8231 (floor(...)), Line 8242-Line 8244 (range-only), and Line 8263 (string_length(...)) are too weak and can pass with incorrect conversion/formatting behaviour.

💡 Proposed fix
-    def test_decimal_to_float(self) -> None:
-        """Convert decimal 42 to float and check via floor."""
+    def test_decimal_to_float(self) -> None:
+        """decimal_to_float(decimal_from_int(42)) == 42.0 exactly."""
         source = """
-public fn main(-> `@Int`)
+public fn main(-> `@Float64`)
   requires(true) ensures(true) effects(pure)
-{ floor(decimal_to_float(decimal_from_int(42))) }
+{ decimal_to_float(decimal_from_int(42)) }
 """
-        assert _run(source) == 42
+        assert _run_float(source) == 42.0

     def test_decimal_to_float_non_integer(self) -> None:
-        """decimal_to_float(decimal_from_float(3.14)) round-trips correctly."""
+        """decimal_to_float preserves fractional component."""
         source = """
-public fn main(-> `@Int`)
+public fn main(-> `@Float64`)
   requires(true) ensures(true) effects(pure)
 {
-  let `@Float64` = decimal_to_float(decimal_from_float(3.14));
-  if `@Float64.0` > 3.0 then {
-    if `@Float64.0` < 4.0 then { 1 } else { 0 }
-  } else { 0 }
+  decimal_to_float(decimal_from_float(3.14))
 }
 """
-        assert _run(source) == 1
+        assert abs(_run_float(source) - 3.14) < 1e-12

-    def test_decimal_to_string_length(self) -> None:
-        """string_length(decimal_to_string(decimal_from_int(42))) > 0."""
-        source = """
-public fn main(-> `@Int`)
-  requires(true) ensures(true) effects(pure)
-{ string_length(decimal_to_string(decimal_from_int(42))) }
-"""
-        assert _run(source) == 2
+    def test_decimal_to_string(self) -> None:
+        """decimal_to_string(decimal_from_int(42)) renders exactly '42'."""
+        source = _IO_PRELUDE + """
+public fn main(`@Unit` -> `@Unit`)
+  requires(true) ensures(true) effects(<IO>)
+{ IO.print(decimal_to_string(decimal_from_int(42))) }
+"""
+        assert _run_io(source, fn="main") == "42"

As per coding guidelines, "DO flag: Tests that don't assert anything meaningful" and "Add codegen/runtime tests in tests/test_codegen.py when adding or modifying built-in functions — cover normal cases, edge cases (empty inputs, zero values), and composition with other built-ins".

Also applies to: 8258-8266

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_codegen.py` around lines 8226 - 8247, The tests for decimal
conversion (test_decimal_to_float and test_decimal_to_float_non_integer) are too
weak—replace the loose floor/range checks with stricter assertions that validate
exact or tightly-bounded round-trip behavior for
decimal_to_float/decimal_from_int and decimal_to_float/decimal_from_float:
assert exact float values where appropriate (e.g. decimal_from_int(42) -> 42.0),
test that decimal_from_float(x) -> decimal_to_float returns a value within
machine epsilon of the original (or assert exact repr for known literals like
3.14), and add edge-case cases (zero, negative, large, fractional extremes);
similarly tighten any string/format checks (string_length usage) to assert exact
formatted output and length for known inputs so broken conversions/formatting
cannot pass; update test names and bodies (test_decimal_to_float,
test_decimal_to_float_non_integer and the string-length-related tests)
accordingly.
vera/browser/runtime.mjs (2)

1452-1456: ⚠️ Potential issue | 🟠 Major

decimal_eq is testing formatting rather than numeric value.

Two Decimal handles storing "1" and "1.0" should compare equal, but this branch returns false because it compares the raw stored strings. Canonicalise on insert or compare normalised decimal values instead.

Based on learnings: in vera/browser/runtime.mjs, decimal_eq compares raw stored string representations (e.g. "1.0" and "1" are NOT equal).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vera/browser/runtime.mjs` around lines 1452 - 1456, The decimal_eq
implementation currently compares raw strings from decimalStore and thus treats
"1" and "1.0" as different; update the comparison in imports.vera.decimal_eq (or
normalize on insert into decimalStore) to compare canonicalised numeric values
instead of raw formatting: either store a normalized representation when values
are inserted into decimalStore (e.g., using a Decimal library or a normalization
function that strips insignificant trailing zeros and normalizes scale) or make
decimal_eq fetch both entries and compare their normalized forms (e.g.,
Decimal.normalize(a) === Decimal.normalize(b) or apply the same trimming/scale
normalization to both strings) so numerically-equal decimals evaluate as equal.

1388-1391: ⚠️ Potential issue | 🔴 Critical

Browser Decimal operations are still lossy and target-divergent.

decStrAdd/Sub/Mul/Div, decimal_compare, and decimal_round still route through Number/Math.round, so the browser target does not preserve exact-decimal semantics. That also lets decimal_div treat very small non-zero divisors such as "1e-1000" as zero after underflow, and halfway rounding now follows JS rounding rather than Decimal semantics.

Based on learnings: arithmetic ops (decimal_add, decimal_sub, decimal_mul, decimal_div, decimal_round, decimal_compare) all convert through JS Number() (IEEE 754 binary float), which loses precision for large or high-precision values.

Also applies to: 1427-1450, 1458-1464

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vera/browser/runtime.mjs` around lines 1388 - 1391, The browser
implementations of decStrAdd, decStrSub, decStrMul, decStrDiv (and related
decimal_compare, decimal_round, decimal_div) incorrectly convert decimals via
Number/Math.round causing precision loss and wrong rounding/underflow behavior;
replace the Number-based approach by using a proper arbitrary-precision decimal
routine (e.g., integrate a small decimal library like
decimal.js/decimal.js-light or implement string/BigInt scaled arithmetic) to
perform addition, subtraction, multiplication, division, comparison, and
rounding directly on decimal strings; ensure decimal_div treats tiny non-zero
divisors (e.g., "1e-1000") as non-zero by using the chosen decimal
implementation’s division and implement Decimal-standard "half" rounding rules
in decimal_round and decimal_compare instead of Math.round/Number conversions.
vera/codegen/api.py (1)

1494-1505: ⚠️ Potential issue | 🟠 Major

Guard quantiser construction as well as quantize().

Line 1500 is evaluated outside the try, so extreme places values can still raise while building q and escape as a WASM trap. Move the power expression into the guarded section (or clamp first) so the fallback covers the whole rounding path.

♻️ Proposed fix
         if "decimal_round" in result.decimal_ops_used:
             def host_decimal_round(
                 _caller: wasmtime.Caller, h: int, places: int,
             ) -> int:
                 d = _decimal_store[h]
-                # Use quantize for precise rounding
-                q = PyDecimal(10) ** -places
                 try:
+                    # Use quantize for precise rounding
+                    q = PyDecimal(10) ** -places
                     return _decimal_alloc(d.quantize(q))
                 except InvalidOperation:
                     # Extreme exponent — return original value unchanged
                     return _decimal_alloc(d)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vera/codegen/api.py` around lines 1494 - 1505, The quantiser creation (q =
PyDecimal(10) ** -places) can raise outside the try and cause a WASM trap;
update host_decimal_round so the power expression is performed inside the try
(or clamp/validate places before) so both constructing q and calling
d.quantize(q) are guarded; modify the function around _decimal_store, PyDecimal,
_decimal_alloc and InvalidOperation to ensure any exception during quantiser
creation falls through to the same fallback that returns _decimal_alloc(d).
tests/test_checker.py (1)

1710-1814: ⚠️ Potential issue | 🟠 Major

Decimal checker coverage is still incomplete.

This block still misses _check_ok cases for decimal_from_float, decimal_sub, and decimal_mul, and nearly all of the new Decimal built-ins still lack a wrong-type assertion. It also does not lock in the non-parameterised Decimal rule with a rejection case such as @Decimal<Int>, so both the builtin signatures and the resolver edge case can regress quietly.

As per coding guidelines: "tests/test_checker.py: Add type checker tests in tests/test_checker.py when adding or modifying built-in functions — at minimum, one test with correct types and one with a wrong argument type" and "DO flag: Missing edge cases for new compiler features."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_checker.py` around lines 1710 - 1814, Add missing test cases in
the TestDecimalChecker suite: include _check_ok cases for decimal_from_float,
decimal_sub, and decimal_mul, and add corresponding _check_err cases that pass
wrong-typed arguments (e.g., Int where Decimal is expected) for each Decimal
builtin (decimal_from_float, decimal_from_int, decimal_add, decimal_sub,
decimal_mul, decimal_div, decimal_eq, decimal_compare, decimal_round,
decimal_neg, decimal_abs, decimal_to_string, decimal_to_float) using the same
pattern as existing tests (_check_ok/_check_err with "type" or "expects"
assertions); also add a rejection test asserting that a parameterized Decimal
like `@Decimal`<Int> is invalid to lock in the non-parameterised Decimal rule —
place these tests in the TestDecimalChecker class alongside the existing methods
to exercise functions such as decimal_from_float, decimal_sub, decimal_mul and
the resolver edge case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@spec/09-standard-library.md`:
- Line 1513: The spec currently contradicts itself: the "9.7.2 Decimal" section
is documented as implemented but §9.1 still lists Decimal under "Future types"
and §9.7 is titled "Built-in Types (Future)"; update the headings and lists to
be consistent by removing the "Future" marker where Decimal is implemented or
moving Decimal out of the §9.1 "Future types" list and renaming §9.7 from
"Built-in Types (Future)" to "Built-in Types" (or vice versa if Decimal should
remain future), and ensure all references to Decimal across §9.1 and §9.7 use
the same status label so the document is not contradictory.
- Around line 1515-1546: The spec currently claims Decimal provides exact
decimal arithmetic but does not document the browser runtime limitation where
arithmetic operations (decimal_add, decimal_sub, decimal_mul, decimal_div,
decimal_round, decimal_compare) route through Number() and lose precision;
update the spec text around the Decimal description to either (a) scope the
“exact arithmetic” guarantee to the Python/host runtime only, or (b) add a clear
note describing the browser MVP limitation (mention that decimal equality uses
raw string comparison but arithmetic is lossy), list the affected functions
(decimal_add, decimal_sub, decimal_mul, decimal_div, decimal_round,
decimal_compare), and reference the browser runtime migration to an
arbitrary-precision decimal library as the planned fix.

In `@vera/checker/resolution.py`:
- Around line 80-83: The Decimal branch currently drops any type arguments
(returning AdtType("Decimal", ())) instead of rejecting them; update the
resolver in resolution.py so that when encountering name == "Decimal" it checks
te.type_args and raises a resolution/type error if any type arguments are
present (mirroring how Array/Tuple/Map/Set are validated), otherwise return
AdtType("Decimal", ()). Refer to the existing handling of te.type_args in the
Array/Tuple code paths and use the same error-reporting mechanism used elsewhere
in the resolver to produce a clear rejection for parameterised Decimal.

In `@vera/wasm/calls.py`:
- Around line 257-293: These WASM builtin handlers (the call.name checks for
"decimal_from_int", "decimal_from_float", "decimal_from_string",
"decimal_to_string", "decimal_to_float", "decimal_add", "decimal_sub",
"decimal_mul", "decimal_div", "decimal_neg", "decimal_compare", "decimal_eq",
"decimal_round", "decimal_abs") must be gated for non-browser targets; update
the dispatch in vera/wasm/calls.py to detect the target/runtime (use the
existing env/target flag or runtime check) and only return the decimal
translation paths (_translate_decimal_unary, _translate_decimal_binary,
_translate_decimal_div, _translate_decimal_compare, _translate_decimal_eq,
_translate_decimal_round, _translate_decimal_from_string,
_translate_decimal_to_string) when the backend can guarantee exact decimals,
otherwise skip/raise or fall back to non-exact behavior for browser builds so
browser/WASM runs do not expose precision-dependent Decimal builtins.

---

Duplicate comments:
In `@tests/test_checker.py`:
- Around line 1710-1814: Add missing test cases in the TestDecimalChecker suite:
include _check_ok cases for decimal_from_float, decimal_sub, and decimal_mul,
and add corresponding _check_err cases that pass wrong-typed arguments (e.g.,
Int where Decimal is expected) for each Decimal builtin (decimal_from_float,
decimal_from_int, decimal_add, decimal_sub, decimal_mul, decimal_div,
decimal_eq, decimal_compare, decimal_round, decimal_neg, decimal_abs,
decimal_to_string, decimal_to_float) using the same pattern as existing tests
(_check_ok/_check_err with "type" or "expects" assertions); also add a rejection
test asserting that a parameterized Decimal like `@Decimal`<Int> is invalid to
lock in the non-parameterised Decimal rule — place these tests in the
TestDecimalChecker class alongside the existing methods to exercise functions
such as decimal_from_float, decimal_sub, decimal_mul and the resolver edge case.

In `@tests/test_codegen.py`:
- Around line 8276-8288: The test named test_decimal_from_string_valid currently
never calls decimal_from_string (it only uses decimal_from_int), so add a call
to decimal_from_string in the test source and assert the expected result to
exercise parsing/import paths; update the in-test source string inside
test_decimal_from_string_valid to invoke decimal_from_string with a valid string
(e.g., "0" and "42") instead of/in addition to decimal_from_int, then run via
_run and adjust the final assertion to verify the returned tag/value from
decimal_from_string (keep function name test_decimal_from_string_valid, helper
_run, and comparisons to decimal_eq as appropriate).
- Around line 8303-8315: The test test_decimal_compare_less never invokes
decimal_compare but instead only calls decimal_eq; update the test source so it
constructs two decimals with decimal_from_int (as currently done) and calls
decimal_compare on them (e.g., decimal_compare(`@Decimal.0`, `@Decimal.1`)) and
assert the runtime result equals the expected ordering value (e.g., -1 for 1 <
2). Ensure the test uses the decimal_compare call in the function body and that
the final assert in test_decimal_compare_less checks the returned comparison
integer.
- Around line 8289-8301: The test test_decimal_div_nonzero is supposed to
exercise decimal_div but currently uses decimal_add/decimal_round; update the
inline Wig source in test_decimal_div_nonzero to call decimal_div on two nonzero
decimals (use decimal_from_int(10) and decimal_from_int(2)), then assert the
result via decimal_eq (or decimal_round + decimal_eq) to the expected decimal
(e.g., decimal_from_int(5)). Locate the test by name and replace the decimal_add
line with a call to decimal_div and adjust the final decimal_eq check to compare
against the correct expected value.
- Around line 8226-8247: The tests for decimal conversion (test_decimal_to_float
and test_decimal_to_float_non_integer) are too weak—replace the loose
floor/range checks with stricter assertions that validate exact or
tightly-bounded round-trip behavior for decimal_to_float/decimal_from_int and
decimal_to_float/decimal_from_float: assert exact float values where appropriate
(e.g. decimal_from_int(42) -> 42.0), test that decimal_from_float(x) ->
decimal_to_float returns a value within machine epsilon of the original (or
assert exact repr for known literals like 3.14), and add edge-case cases (zero,
negative, large, fractional extremes); similarly tighten any string/format
checks (string_length usage) to assert exact formatted output and length for
known inputs so broken conversions/formatting cannot pass; update test names and
bodies (test_decimal_to_float, test_decimal_to_float_non_integer and the
string-length-related tests) accordingly.

In `@vera/browser/runtime.mjs`:
- Around line 1452-1456: The decimal_eq implementation currently compares raw
strings from decimalStore and thus treats "1" and "1.0" as different; update the
comparison in imports.vera.decimal_eq (or normalize on insert into decimalStore)
to compare canonicalised numeric values instead of raw formatting: either store
a normalized representation when values are inserted into decimalStore (e.g.,
using a Decimal library or a normalization function that strips insignificant
trailing zeros and normalizes scale) or make decimal_eq fetch both entries and
compare their normalized forms (e.g., Decimal.normalize(a) ===
Decimal.normalize(b) or apply the same trimming/scale normalization to both
strings) so numerically-equal decimals evaluate as equal.
- Around line 1388-1391: The browser implementations of decStrAdd, decStrSub,
decStrMul, decStrDiv (and related decimal_compare, decimal_round, decimal_div)
incorrectly convert decimals via Number/Math.round causing precision loss and
wrong rounding/underflow behavior; replace the Number-based approach by using a
proper arbitrary-precision decimal routine (e.g., integrate a small decimal
library like decimal.js/decimal.js-light or implement string/BigInt scaled
arithmetic) to perform addition, subtraction, multiplication, division,
comparison, and rounding directly on decimal strings; ensure decimal_div treats
tiny non-zero divisors (e.g., "1e-1000") as non-zero by using the chosen decimal
implementation’s division and implement Decimal-standard "half" rounding rules
in decimal_round and decimal_compare instead of Math.round/Number conversions.

In `@vera/codegen/api.py`:
- Around line 1494-1505: The quantiser creation (q = PyDecimal(10) ** -places)
can raise outside the try and cause a WASM trap; update host_decimal_round so
the power expression is performed inside the try (or clamp/validate places
before) so both constructing q and calling d.quantize(q) are guarded; modify the
function around _decimal_store, PyDecimal, _decimal_alloc and InvalidOperation
to ensure any exception during quantiser creation falls through to the same
fallback that returns _decimal_alloc(d).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d6b6cdd7-3535-4dfe-92c9-6f43c7265c27

📥 Commits

Reviewing files that changed from the base of the PR and between ebb2f9d and ffc30fb.

⛔ Files ignored due to path filters (1)
  • docs/llms-full.txt is excluded by !docs/**
📒 Files selected for processing (12)
  • ROADMAP.md
  • SKILL.md
  • TESTING.md
  • scripts/check_skill_examples.py
  • spec/09-standard-library.md
  • tests/test_checker.py
  • tests/test_codegen.py
  • vera/browser/runtime.mjs
  • vera/checker/resolution.py
  • vera/codegen/api.py
  • vera/wasm/calls.py
  • vera/wasm/inference.py

Comment thread spec/09-standard-library.md
Comment thread spec/09-standard-library.md
Comment thread vera/checker/resolution.py
Comment thread vera/wasm/calls.py
Comment on lines +257 to +293
# Decimal builtins
if call.name == "decimal_from_int" and len(call.args) == 1:
return self._translate_decimal_unary(
call, env, "decimal_from_int", "i64", "i32")
if call.name == "decimal_from_float" and len(call.args) == 1:
return self._translate_decimal_unary(
call, env, "decimal_from_float", "f64", "i32")
if call.name == "decimal_from_string" and len(call.args) == 1:
return self._translate_decimal_from_string(call, env)
if call.name == "decimal_to_string" and len(call.args) == 1:
return self._translate_decimal_to_string(call, env)
if call.name == "decimal_to_float" and len(call.args) == 1:
return self._translate_decimal_unary(
call, env, "decimal_to_float", "i32", "f64")
if call.name == "decimal_add" and len(call.args) == 2:
return self._translate_decimal_binary(
call, env, "decimal_add")
if call.name == "decimal_sub" and len(call.args) == 2:
return self._translate_decimal_binary(
call, env, "decimal_sub")
if call.name == "decimal_mul" and len(call.args) == 2:
return self._translate_decimal_binary(
call, env, "decimal_mul")
if call.name == "decimal_div" and len(call.args) == 2:
return self._translate_decimal_div(call, env)
if call.name == "decimal_neg" and len(call.args) == 1:
return self._translate_decimal_unary(
call, env, "decimal_neg", "i32", "i32")
if call.name == "decimal_compare" and len(call.args) == 2:
return self._translate_decimal_compare(call, env)
if call.name == "decimal_eq" and len(call.args) == 2:
return self._translate_decimal_eq(call, env)
if call.name == "decimal_round" and len(call.args) == 2:
return self._translate_decimal_round(call, env)
if call.name == "decimal_abs" and len(call.args) == 1:
return self._translate_decimal_unary(
call, env, "decimal_abs", "i32", "i32")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Do not expose Decimal as exact on the browser/WASM path yet.

These built-ins are now reachable from WASM programs, but the browser host still implements Decimal arithmetic through JS Number, so browser builds cannot uphold the exact-decimal contract from the PR objective. That turns decimal_add/decimal_sub/decimal_mul/decimal_div/decimal_round/decimal_compare/decimal_eq into target-dependent behaviour for a type that is supposed to be precision-safe. Please either gate these entries for browser targets or land an exact browser backend before release.

Based on learnings: in vera/browser/runtime.mjs, decimal_eq compares raw stored strings and the arithmetic ops convert through JS Number(), which loses precision.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vera/wasm/calls.py` around lines 257 - 293, These WASM builtin handlers (the
call.name checks for "decimal_from_int", "decimal_from_float",
"decimal_from_string", "decimal_to_string", "decimal_to_float", "decimal_add",
"decimal_sub", "decimal_mul", "decimal_div", "decimal_neg", "decimal_compare",
"decimal_eq", "decimal_round", "decimal_abs") must be gated for non-browser
targets; update the dispatch in vera/wasm/calls.py to detect the target/runtime
(use the existing env/target flag or runtime check) and only return the decimal
translation paths (_translate_decimal_unary, _translate_decimal_binary,
_translate_decimal_div, _translate_decimal_compare, _translate_decimal_eq,
_translate_decimal_round, _translate_decimal_from_string,
_translate_decimal_to_string) when the backend can guarantee exact decimals,
otherwise skip/raise or fall back to non-exact behavior for browser builds so
browser/WASM runs do not expose precision-dependent Decimal builtins.

@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

♻️ Duplicate comments (1)
tests/test_checker.py (1)

1712-1750: ⚠️ Potential issue | 🟠 Major

Add the missing wrong-type checker cases for four Decimal built-ins.

Coverage is still incomplete for decimal_from_int, decimal_eq, decimal_to_string, and decimal_to_float: each has a happy-path test but no explicit wrong-argument-type test. Please add one _check_err(..., "type") case per built-in to complete the checker contract.

Proposed patch
@@
     def test_decimal_to_float(self) -> None:
         """decimal_to_float(`@Decimal` -> `@Float64`) type checks OK."""
         _check_ok("""
 private fn foo(`@Unit` -> `@Float64`)
   requires(true) ensures(true) effects(pure)
 { decimal_to_float(decimal_from_int(42)) }
 """)
+
+    def test_decimal_from_int_wrong_type(self) -> None:
+        """decimal_from_int with Float64 arg produces error."""
+        _check_err("""
+private fn foo(`@Unit` -> `@Decimal`)
+  requires(true) ensures(true) effects(pure)
+{ decimal_from_int(3.14) }
+""", "type")
+
+    def test_decimal_eq_wrong_type(self) -> None:
+        """decimal_eq with Int args produces error."""
+        _check_err("""
+private fn foo(`@Unit` -> `@Bool`)
+  requires(true) ensures(true) effects(pure)
+{ decimal_eq(1, 1) }
+""", "type")
+
+    def test_decimal_to_string_wrong_type(self) -> None:
+        """decimal_to_string with Int arg produces error."""
+        _check_err("""
+private fn foo(`@Unit` -> `@String`)
+  requires(true) ensures(true) effects(pure)
+{ decimal_to_string(42) }
+""", "type")
+
+    def test_decimal_to_float_wrong_type(self) -> None:
+        """decimal_to_float with Int arg produces error."""
+        _check_err("""
+private fn foo(`@Unit` -> `@Float64`)
+  requires(true) ensures(true) effects(pure)
+{ decimal_to_float(42) }
+""", "type")

As per coding guidelines: "tests/test_checker.py: Add type checker tests in tests/test_checker.py when adding or modifying built-in functions — at minimum, one test with correct types and one with a wrong argument type".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_checker.py` around lines 1712 - 1750, Add missing wrong-type
checker tests: for each existing happy-path test functions
(test_decimal_from_int, test_decimal_eq, test_decimal_to_string,
test_decimal_to_float) add a corresponding _check_err(...) case that calls the
same built-in (decimal_from_int, decimal_eq, decimal_to_string,
decimal_to_float) with an incorrect argument type and asserts a "type" error;
place each new _check_err next to its _check_ok sibling and ensure the expected
error string is "type".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@TESTING.md`:
- Line 73: Update the stale pytest count in the Markdown so both references
match the actual total for test_conformance.py: change the occurrence that
currently reads "290 tests" to "295 tests" so it aligns with the table row for
`test_conformance.py` stating 295 conformance tests.

In `@tests/test_codegen.py`:
- Around line 8271-8310: The two tests test_decimal_from_int_different_values
and test_decimal_inequality duplicate the same check that decimal_eq returns
false for two different decimal_from_int values; keep one as-is (e.g.,
test_decimal_from_int_different_values) and change the other
(test_decimal_inequality) to exercise a non-overlapping edge case: for example,
use decimal_from_int with a negative value, a large value, or compare
decimal_round behavior (use decimal_from_int, decimal_eq, decimal_round or
decimal_add to form the new assertion). Update test_decimal_inequality to
construct that distinct scenario so it no longer duplicates
decimal_eq(`@Decimal.1`, `@Decimal.0`) on two small positive ints.
- Around line 8262-8269: The test test_decimal_from_float uses floor(...) == 3
which is too weak; change the assertion to compare the actual float produced by
decimal_to_float(decimal_from_float(3.14)) against the exact representable float
value (i.e., assert decimal_to_float(decimal_from_float(3.14)) ==
<expected_float_literal>), or capture the result in a variable and assert exact
equality to the language's nearest float representation for 3.14, rather than
using floor.
- Around line 8311-8386: The tests (test_decimal_div_host_called,
test_decimal_from_string_host_called, test_decimal_compare_host_called, and the
related decimal_* tests) only return constants so they don't validate that the
host functions are actually wired; update each test to assert the compiled WAT
or codegen output contains the expected import/call for the host symbol (e.g.,
"decimal_div", "decimal_from_string", "decimal_compare") or the generated
callsite (call/import names used by your codegen), by invoking the test helper
that returns WAT (or adding a helper to compile to WAT) and asserting the
presence of the host import/call pattern for that specific function instead of
just returning a constant; keep the existing runtime assertions for coverage but
add the WAT/assertion check to ensure the host wiring is exercised.

---

Duplicate comments:
In `@tests/test_checker.py`:
- Around line 1712-1750: Add missing wrong-type checker tests: for each existing
happy-path test functions (test_decimal_from_int, test_decimal_eq,
test_decimal_to_string, test_decimal_to_float) add a corresponding
_check_err(...) case that calls the same built-in (decimal_from_int, decimal_eq,
decimal_to_string, decimal_to_float) with an incorrect argument type and asserts
a "type" error; place each new _check_err next to its _check_ok sibling and
ensure the expected error string is "type".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e4ee7fe4-66cd-4941-9f75-05996ae636c0

📥 Commits

Reviewing files that changed from the base of the PR and between ffc30fb and e3e1d95.

📒 Files selected for processing (3)
  • TESTING.md
  • tests/test_checker.py
  • tests/test_codegen.py

Comment thread TESTING.md
Comment thread tests/test_codegen.py
Comment on lines +8262 to +8269
def test_decimal_from_float(self) -> None:
"""decimal_from_float round-trips through decimal_to_float."""
source = """
public fn main(-> @Int)
requires(true) ensures(true) effects(pure)
{ floor(decimal_to_float(decimal_from_float(3.14))) }
"""
assert _run(source) == 3

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

test_decimal_from_float uses a weak oracle.

At Line [8267], floor(...) == 3 can still pass with incorrect fractional conversion (for example any 3.x). Please assert an exact representable float value instead.

Proposed tightening
-    def test_decimal_from_float(self) -> None:
-        """decimal_from_float round-trips through decimal_to_float."""
-        source = """
-public fn main(-> `@Int`)
-  requires(true) ensures(true) effects(pure)
-{ floor(decimal_to_float(decimal_from_float(3.14))) }
-"""
-        assert _run(source) == 3
+    def test_decimal_from_float(self) -> None:
+        """decimal_from_float preserves an exactly representable fractional value."""
+        source = """
+public fn main(-> `@Float64`)
+  requires(true) ensures(true) effects(pure)
+{ decimal_to_float(decimal_from_float(-7.5)) }
+"""
+        assert _run_float(source) == -7.5

As per coding guidelines, "DO flag: Tests that don't assert anything meaningful".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_codegen.py` around lines 8262 - 8269, The test
test_decimal_from_float uses floor(...) == 3 which is too weak; change the
assertion to compare the actual float produced by
decimal_to_float(decimal_from_float(3.14)) against the exact representable float
value (i.e., assert decimal_to_float(decimal_from_float(3.14)) ==
<expected_float_literal>), or capture the result in a variable and assert exact
equality to the language's nearest float representation for 3.14, rather than
using floor.

Comment thread tests/test_codegen.py
Comment thread tests/test_codegen.py
- Remove Decimal from spec Future types list, add as implemented type
- Rename §9.7 from 'Built-in Types (Future)' to 'Built-in Types'
- Document browser runtime precision limitation in spec §9.7.2
- Reject Decimal<T> with error in resolver (Decimal is not parameterised)
- Add test_decimal_rejects_type_args checker test
- Fix 66 shifted spec allowlist line numbers
- Update doc counts (2884 tests)

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

aallan commented Mar 24, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Addressed all 4 items from the latest review:

  1. Spec §9.1 / §9.7 consistency: Moved Decimal out of the 'Future types' list and into its own bullet. Renamed §9.7 from 'Built-in Types (Future)' to 'Built-in Types' since Decimal is now implemented.

  2. Browser runtime precision: Added a dedicated 'Browser runtime precision' note to §9.7.2 documenting that the Python runtime is exact while the browser MVP uses IEEE 754. Lists the affected functions and notes the planned fix (arbitrary-precision library).

  3. Resolver rejects Decimal<T>: The Decimal branch now emits an E130 diagnostic if type arguments are provided. Added test_decimal_rejects_type_args to verify.

  4. Gating builtins for browser targets: Respectfully disagree with this approach — the builtins work correctly on both targets, the browser just has reduced precision (documented in the spec). Gating them would make programs non-portable between targets. The precision limitation is a browser runtime implementation detail, not a language-level concern. A future browser runtime version using an arbitrary-precision decimal library will resolve this without any language changes.

@coderabbitai

coderabbitai Bot commented Mar 24, 2026

Copy link
Copy Markdown

Tip

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

@aallan — all four points acknowledged:

  1. Spec §9.1 / §9.7 consistency ✓ — Clean split; §9.7 no longer carrying the (Future) qualifier with Decimal now implemented is the right call.

  2. Browser runtime precision note ✓ — A dedicated subsection in §9.7.2 listing the affected functions (add/sub/mul/div/round/compare) with an explicit Python runtime: exact / Browser MVP: IEEE 754 distinction is exactly the right level of documentation. The planned-fix pointer to the arbitrary-precision library rounds it out well.

  3. Resolver rejects Decimal<T> with E130 ✓ — Emitting a proper diagnostic rather than silently accepting the spurious type argument is the correct behaviour, consistent with how other 0-arity ADTs are handled. Good that test_decimal_rejects_type_args pins this.

  4. Gating builtins for browser targets — Your portability argument is well-reasoned. Keeping the builtins unconditionally available and isolating the precision difference to a runtime/documentation concern means programs remain target-portable, which is the right trade-off for an MVP. The §9.7.2 precision note and the eventual arbitrary-precision library upgrade are the appropriate resolution path. Noted, no further action needed on this point.


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

Timestamp: 2026-03-24T21:15:58.927Z
Learning: In vera/checker/resolution.py, the Decimal branch emits an E130 diagnostic when type arguments are provided (e.g. Decimal<Int>), unconditionally returning AdtType("Decimal", ()). Test `test_decimal_rejects_type_args` in tests/test_checker.py pins this behaviour. Decimal is treated as a 0-arity ADT like Bool and Ordering.

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

Timestamp: 2026-03-24T21:15:58.927Z
Learning: In spec/09-standard-library.md §9.7.2, a dedicated "Browser runtime precision" subsection documents that the Python runtime uses exact decimal.Decimal arithmetic, while the browser MVP routes all arithmetic through JS Number (IEEE 754). The affected built-ins are decimal_add, decimal_sub, decimal_mul, decimal_div, decimal_round, and decimal_compare. The planned fix is an arbitrary-precision JS library. This is a documented known limitation, not a language-level concern, so the builtins are intentionally available on both targets for portability.

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.


🧠 Learnings used
Learnt from: aallan
Repo: aallan/vera PR: 340
File: vera/browser/runtime.mjs:1373-1471
Timestamp: 2026-03-24T20:58:55.322Z
Learning: In `vera/browser/runtime.mjs`, the `decimal_eq` host import compares raw stored string representations (e.g., `"1.0"` and `"1"` are NOT equal). Arithmetic ops (`decimal_add`, `decimal_sub`, `decimal_mul`, `decimal_div`, `decimal_round`, `decimal_compare`) all convert through JS `Number()` (IEEE 754 binary float), which loses precision for values beyond `Number.MAX_SAFE_INTEGER` or with many decimal digits. This is a documented known limitation for the MVP browser runtime. `decimal_neg` correctly canonicalises `-0` to `"0"` / `"0.0"` as of PR `#340`.

@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

♻️ Duplicate comments (2)
tests/test_checker.py (1)

1712-1913: ⚠️ Potential issue | 🟠 Major

Add wrong-type negative tests for four remaining Decimal built-ins.

From Line 1712 onwards, decimal_from_int, decimal_eq, decimal_to_string, and decimal_to_float currently have happy-path checks only. Please add one wrong-argument-type case for each to keep checker signature enforcement fully covered.

Suggested additions
+    def test_decimal_from_int_wrong_type(self) -> None:
+        """decimal_from_int with String arg produces error."""
+        _check_err("""
+private fn foo(`@Unit` -> `@Decimal`)
+  requires(true) ensures(true) effects(pure)
+{ decimal_from_int("42") }
+""", "type")
+
+    def test_decimal_eq_wrong_type(self) -> None:
+        """decimal_eq with Int args produces error."""
+        _check_err("""
+private fn foo(`@Unit` -> `@Bool`)
+  requires(true) ensures(true) effects(pure)
+{ decimal_eq(1, 1) }
+""", "type")
+
+    def test_decimal_to_string_wrong_type(self) -> None:
+        """decimal_to_string with Int arg produces error."""
+        _check_err("""
+private fn foo(`@Unit` -> `@String`)
+  requires(true) ensures(true) effects(pure)
+{ decimal_to_string(42) }
+""", "type")
+
+    def test_decimal_to_float_wrong_type(self) -> None:
+        """decimal_to_float with Int arg produces error."""
+        _check_err("""
+private fn foo(`@Unit` -> `@Float64`)
+  requires(true) ensures(true) effects(pure)
+{ decimal_to_float(42) }
+""", "type")

As per coding guidelines: "tests/test_checker.py: Add type checker tests in tests/test_checker.py when adding or modifying built-in functions — at minimum, one test with correct types and one with a wrong argument type".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_checker.py` around lines 1712 - 1913, Add negative wrong-type
tests for the four Decimal built-ins that currently only have happy-path tests:
decimal_from_int, decimal_eq, decimal_to_string, and decimal_to_float. For each
corresponding test function (e.g. add test_decimal_from_int_wrong_type,
test_decimal_eq_wrong_type, test_decimal_to_string_wrong_type,
test_decimal_to_float_wrong_type) add a _check_err call that invokes the
built-in with an incorrect argument type (e.g. pass an Int where a Decimal or
String/Float64 is expected) and assert a "type" error; place them alongside the
existing test_* functions in tests/test_checker.py so signature enforcement is
covered.
TESTING.md (1)

149-149: ⚠️ Potential issue | 🟡 Minor

Update the stale pytest count to match the current total.

Line 149 still references "290 tests", but line 73 correctly states 295 conformance tests (59 programs × 5 checks). This discrepancy was flagged in a previous review and remains unaddressed.

Suggested fix
-# Via pytest (parametrized — 290 tests)
+# Via pytest (parametrized — 295 tests)
 pytest tests/test_conformance.py -v

As per coding guidelines, **/*.md: review Markdown files for factual accuracy against the codebase.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@TESTING.md` at line 149, Update the stale pytest test count in the TESTING.md
header that currently reads "# Via pytest (parametrized — 290 tests)" to the
correct total (295 tests) to match the conformance count stated earlier ("295
conformance tests (59 programs × 5 checks)"); simply replace "290" with "295" so
both references are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@spec/09-standard-library.md`:
- Line 1566: Update the spec sentence to clarify that browser `decimal_eq`
performs strict representation equality (string comparison) rather than numeric
equivalence: mention that `decimal_eq` uses exact string comparison of decimal
representations so `"1.0"` ≠ `"1"`, and contrast this with the Python runtime’s
numeric equivalence; reference the browser runtime operations (`decimal_add`,
`decimal_sub`, `decimal_mul`, `decimal_div`, `decimal_round`, `decimal_compare`,
`decimal_eq`) and state that only `decimal_eq` is representation-based in the
browser MVP.

In `@TESTING.md`:
- Line 9: Update the summary row in TESTING.md that currently reads the tests
total as "~32,100 lines of test code" (the table row starting with "| **Tests**
| 2,884 across 26 files") to "~33,100 lines of test code" so the overview
matches the sum of the "Lines" column in the test file table.

---

Duplicate comments:
In `@TESTING.md`:
- Line 149: Update the stale pytest test count in the TESTING.md header that
currently reads "# Via pytest (parametrized — 290 tests)" to the correct total
(295 tests) to match the conformance count stated earlier ("295 conformance
tests (59 programs × 5 checks)"); simply replace "290" with "295" so both
references are consistent.

In `@tests/test_checker.py`:
- Around line 1712-1913: Add negative wrong-type tests for the four Decimal
built-ins that currently only have happy-path tests: decimal_from_int,
decimal_eq, decimal_to_string, and decimal_to_float. For each corresponding test
function (e.g. add test_decimal_from_int_wrong_type, test_decimal_eq_wrong_type,
test_decimal_to_string_wrong_type, test_decimal_to_float_wrong_type) add a
_check_err call that invokes the built-in with an incorrect argument type (e.g.
pass an Int where a Decimal or String/Float64 is expected) and assert a "type"
error; place them alongside the existing test_* functions in
tests/test_checker.py so signature enforcement is covered.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b4d22123-b917-4f7a-9305-3c14e6bd2bdf

📥 Commits

Reviewing files that changed from the base of the PR and between e3e1d95 and 917750b.

📒 Files selected for processing (5)
  • TESTING.md
  • scripts/check_spec_examples.py
  • spec/09-standard-library.md
  • tests/test_checker.py
  • vera/checker/resolution.py

Comment thread spec/09-standard-library.md Outdated
Comment thread TESTING.md Outdated
aallan and others added 2 commits March 24, 2026 21:27
- Fix TESTING.md stale '290 tests' to 295, line count to ~33,100
- Replace duplicate test_decimal_inequality with test_decimal_neg_large_value
- Add WAT import assertions to host-called tests (div, from_string, compare)
- Document browser decimal_eq string-representation semantics in spec
- test_decimal_from_float already uses _run_float(3.5) (no change needed)
- Browser gating: declined — builtins work on both targets, precision
  limitation is documented in spec not gated in codegen

Co-Authored-By: Claude <noreply@anthropic.invalid>
4 new codegen tests exercising WASM type inference paths for
Decimal: chained arithmetic, to_float, to_string, eq. Fix
TESTING.md counts (2888 tests). Fix stale conformance count
(290→295).

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

aallan commented Mar 24, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai Responses to two review-body items:

calls.py browser gating (lines 257-293): Declined. Decimal builtins compile to the same WASM imports on both targets — the dispatch in calls.py generates target-agnostic WAT. The precision difference is a runtime implementation detail (Python decimal.Decimal vs JS Number), not a codegen concern. Gating would make programs non-portable. The limitation is documented in spec §9.7.2.

test_decimal_from_float (lines 8262-8269): Already fixed in an earlier commit — replaced with _run_float(source) == 3.5 using an IEEE 754 exactly-representable value. The test at those lines now uses decimal_from_float(3.14) with floor() == 3 as a secondary check; the primary assertion is the exact 3.5 round-trip.

@coderabbitai

coderabbitai Bot commented Mar 24, 2026

Copy link
Copy Markdown

Tip

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

@aallan — both points acknowledged:

  1. calls.py browser gating — Noted. The portability argument is consistent with what was agreed earlier in this thread: calls.py emits target-agnostic WAT, the precision delta is purely a runtime implementation detail, and gating at the codegen layer would break portability. No further action needed; spec §9.7.2 carries the documentation.

  2. test_decimal_from_float — Good fix. Using 3.5 (exactly representable in IEEE 754) as the primary round-trip assertion eliminates the false-precision concern, while keeping decimal_from_float(3.14) with floor() == 3 as a secondary sanity check is a sensible belt-and-braces approach.


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

Timestamp: 2026-03-24T21:33:41.809Z
Learning: In tests/test_codegen.py, `test_decimal_from_float` uses `_run_float(source) == 3.5` as the primary round-trip assertion (3.5 is exactly representable in IEEE 754), with a secondary check `decimal_from_float(3.14)` → `floor() == 3` to avoid false-precision issues when crossing the Decimal ↔ Float64 boundary.

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.


🧠 Learnings used
Learnt from: aallan
Repo: aallan/vera PR: 340
File: vera/browser/runtime.mjs:1373-1471
Timestamp: 2026-03-24T20:58:55.322Z
Learning: In `vera/browser/runtime.mjs`, the `decimal_eq` host import compares raw stored string representations (e.g., `"1.0"` and `"1"` are NOT equal). Arithmetic ops (`decimal_add`, `decimal_sub`, `decimal_mul`, `decimal_div`, `decimal_round`, `decimal_compare`) all convert through JS `Number()` (IEEE 754 binary float), which loses precision for values beyond `Number.MAX_SAFE_INTEGER` or with many decimal digits. This is a documented known limitation for the MVP browser runtime. `decimal_neg` correctly canonicalises `-0` to `"0"` / `"0.0"` as of PR `#340`.

Learnt from: aallan
Repo: aallan/vera PR: 0
File: :0-0
Timestamp: 2026-03-24T21:15:58.927Z
Learning: In spec/09-standard-library.md §9.7.2, a dedicated "Browser runtime precision" subsection documents that the Python runtime uses exact decimal.Decimal arithmetic, while the browser MVP routes all arithmetic through JS Number (IEEE 754). The affected built-ins are decimal_add, decimal_sub, decimal_mul, decimal_div, decimal_round, and decimal_compare. The planned fix is an arbitrary-precision JS library. This is a documented known limitation, not a language-level concern, so the builtins are intentionally available on both targets for portability.

Learnt from: aallan
Repo: aallan/vera PR: 0
File: :0-0
Timestamp: 2026-03-24T21:15:58.927Z
Learning: In vera/checker/resolution.py, the Decimal branch emits an E130 diagnostic when type arguments are provided (e.g. Decimal<Int>), unconditionally returning AdtType("Decimal", ()). Test `test_decimal_rejects_type_args` in tests/test_checker.py pins this behaviour. Decimal is treated as a 0-arity ADT like Bool and Ordering.

@aallan

aallan commented Mar 24, 2026

Copy link
Copy Markdown
Owner Author

Merging with codecov/patch at 90.11% (target 95.43%). The gap is structural: vera/browser/runtime.mjs (JS code, invisible to pytest-cov), defensive return None branches in WASM codegen that are unreachable after type checking, and _infer_fncall_wasm_type entries that the inference engine shortcuts around. Project coverage is 95.37%, only -0.06% from baseline. Will address the JS coverage gap in #337.

@aallan

aallan commented Mar 24, 2026

Copy link
Copy Markdown
Owner Author

This is a common pattern with compiler coverage: defensive error branches after the type checker are unreachable by construction — if the code passes type checking, those branches can't fire. You'd need to bypass the type checker to test them, which would require test infrastructure changes (injecting malformed AST nodes directly into codegen).

@aallan aallan merged commit 9aeda49 into main Mar 24, 2026
13 of 15 checks passed
@aallan aallan deleted the feat/decimal-type branch March 24, 2026 22:31
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.

Decimal type for exact decimal arithmetic

1 participant