Add Random effect for non-determinism (closes #465, v0.0.115)#492
Merged
Conversation
New built-in `Random` effect with three operations: - Random.random_int(@int, @int) -> @int # inclusive range - Random.random_float(@Unit) -> @float64 # uniform [0.0, 1.0) - Random.random_bool(@Unit) -> @Bool # coin flip Functions drawing random values declare `effects(<Random>)`, making non-determinism visible in the type signature. Mirrors the Http/Inference host-import effect pattern. Implementation trail: - environment.py: new EffectInfo for Random with three OpInfo entries using INT/UNIT/FLOAT64/BOOL types. - codegen/compilability.py: `elif eff.name == "Random"` in the effect allowlist (no memory requirement — no allocations or heap returns). Scan adds Random QualifiedCall op names to `_random_ops_used`. - codegen/core.py: new `_random_ops_used: set[str]` field and propagation to all four CompileResult construction sites. - codegen/functions.py: propagate from WasmContext back to core. - codegen/assembly.py: three import declarations gated on `_random_ops_used`: (import "vera" "random_int" (func $vera.random_int (param i64 i64) (result i64))) (import "vera" "random_float" (func $vera.random_float (result f64))) (import "vera" "random_bool" (func $vera.random_bool (result i32))) - codegen/api.py: host_random_int / host_random_float / host_random_bool using Python's `random` module. Lazy import — only pulled in when `random_ops_used` is non-empty. CompileResult gains a `random_ops_used` field. - wasm/calls.py: Random joins `_host_import_qualifiers`; dispatch emits `call $vera.random_{int,float,bool}` and tracks via `_random_ops_used`. Op names already begin with `random_`, so no double-prefixing like http_/inference_ needed. - wasm/context.py: `_random_ops_used` set initialized for mixin propagation. `_is_void_expr` explicitly returns False for Random qualifiers (all three ops produce values). - browser/runtime.mjs: three Math.random-backed handlers. random_int uses BigInt arithmetic for i64 boundary. Tests: - tests/conformance/ch07_random_effect.vera — verify level; covers signature plumbing and effect-row carrying without wall-clock flake. - tests/test_codegen.py: TestRandomEffect with 4 tests exercising inclusive range, singleton range, float unit interval, and bool distribution (Bernoulli bounded away from extremes). - tests/test_browser.py: 3 parallel browser tests confirming Math.random-based paths behave identically (in distribution). Documentation: - SKILL.md: new "Random effect" subsection; effects-row example list updated. - spec/07-effects.md: §7.7.4 `Random` added after Diverge; §7.7.1 IO table updated from 7 → 10 ops (missed when shipping #463). - spec/12-runtime.md: §12.4.5 Random Operations added with §12.4.5.1/2/3 subsections for the three ops; §12.2.2 import table gets three new rows. - FAQ.md: built-in effects list gains `<Random>`. - Doc-count sweep: 75 → 76 conformance, 3,356 → 3,368 tests, plus test-file line drift. Also bundled: ROADMAP → HISTORY shuffle for #465 (per workflow — closing PR owns the shuffle). Version bump 0.0.114 → 0.0.115 in pyproject.toml, vera/__init__.py, docs/index.html, README.md, and uv.lock. Co-Authored-By: Claude <noreply@anthropic.invalid>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #492 +/- ##
==========================================
- Coverage 90.28% 90.25% -0.04%
==========================================
Files 58 58
Lines 20052 20137 +85
Branches 228 233 +5
==========================================
+ Hits 18104 18174 +70
- Misses 1944 1959 +15
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a built-in Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Compiler as Compiler
participant Wasm as WasmModule
participant Host as HostRuntime
Dev->>Compiler: compile program with effects(<Random>)
Compiler->>Compiler: record random_ops_used
Compiler->>Wasm: emit WAT imports for vera.random_*
Dev->>Wasm: instantiate WasmModule (provide imports)
Host->>Wasm: provide imports.vera.random_* (Python random / Math.random)
Wasm->>Host: call vera.random_int/float/bool
Host-->>Wasm: return random value
Wasm-->>Dev: program produces non-deterministic output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ROADMAP.md`:
- Line 11: The ROADMAP.md summary line omits the newly added algebraic effect
Random; update the sentence listing effects to include "Random" among (IO, Http,
State, Exceptions, Async, Inference) so it reads e.g. "(IO, Http, State,
Exceptions, Async, Inference, Random)"; ensure the wording around "algebraic
effects" in the same sentence (the line describing built-in functions and
effects) is adjusted accordingly and remains grammatically correct.
In `@spec/12-runtime.md`:
- Line 406: Update the import declaration so the external import name matches
the function symbol: change the import string that currently says "random_int"
to "random_bool" for the declaration that defines (func $vera.random_bool
(result i32)), ensuring the imported module name "vera" remains unchanged.
In `@tests/test_codegen.py`:
- Around line 10271-10273: The probabilistic assertion should be replaced with a
deterministic seeded run that verifies both outcomes occur: seed the RNG (e.g.,
call random.seed(0) or the appropriate PRNG used by the code) before executing
the function, run execute(result, fn_name="main") multiple times (e.g., 100),
collect the .value results into a set, and assert that both 0 and 1 are present
(e.g., assert {0,1}.issubset(observed) or assert observed == {0,1}) so the test
is stable and still exercises both Bernoulli outcomes (reference: execute(...,
fn_name="main") and the random_bool usage).
- Around line 10220-10221: The test currently only checks len(produced) >= 4
which doesn't verify inclusive-boundary semantics; update the assertion logic in
tests/test_codegen.py to explicitly assert that both boundary values appear by
adding checks that low in produced and high in produced (in addition to the
existing distribution check), referencing the existing variables produced, low,
and high so the test enforces the [low, high] contract.
- Around line 10188-10274: Add per-operation WAT import-gating assertions to
each Random effect test (e.g., test_random_int_in_range,
test_random_int_singleton_range, test_random_float_in_unit_interval,
test_random_bool_produces_both): after calling _compile_ok(source) inspect the
compiled module's WAT (or the string returned on the result object) and assert
that the specific host import for the operation used in that test
(Random.random_int, Random.random_float, or Random.random_bool) is present and
that the other Random operation imports are absent; this ensures
_random_ops_used gating is exercised and prevents emitting unused sibling
imports.
In `@vera/browser/runtime.mjs`:
- Around line 1738-1744: The random_int implementation (imports.vera.random_int)
converts BigInt bounds (lowBig, highBig) to Number which loses precision for
values outside the 53-bit safe-integer range; add a runtime guard that checks
that both lowBig and highBig are within
Number.MIN_SAFE_INTEGER..Number.MAX_SAFE_INTEGER and that highBig >= lowBig, and
if not throw a descriptive Error (e.g. "random_int bounds exceed JavaScript safe
integer range; use smaller bounds or adjust runtime") so callers get a clear
failure instead of silently incorrect span/return values; keep the existing
logic (low/high/span) only when the guard passes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e60d4e73-5419-4cb4-b132-f2908befdcd5
⛔ Files ignored due to path filters (8)
docs/SKILL.mdis excluded by!docs/**docs/index.htmlis excluded by!docs/**docs/index.mdis excluded by!docs/**docs/llms-full.txtis excluded by!docs/**docs/llms.txtis excluded by!docs/**docs/sitemap.xmlis excluded by!docs/**tests/conformance/ch07_random_effect.verais excluded by!**/*.verauv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (25)
AGENTS.mdCHANGELOG.mdCLAUDE.mdFAQ.mdHISTORY.mdREADME.mdROADMAP.mdSKILL.mdTESTING.mdpyproject.tomlspec/07-effects.mdspec/12-runtime.mdtests/conformance/manifest.jsontests/test_browser.pytests/test_codegen.pyvera/__init__.pyvera/browser/runtime.mjsvera/codegen/api.pyvera/codegen/assembly.pyvera/codegen/compilability.pyvera/codegen/core.pyvera/codegen/functions.pyvera/environment.pyvera/wasm/calls.pyvera/wasm/context.py
The IO operations table at §7.7.1 grew from 7 → 10 rows in this PR (adding sleep/time/stderr ops listed since #463/v0.0.114). That shifted every subsequent line in spec/07-effects.md down by 3, including two existing allowlisted code blocks: - effect Diverge {} (line 315 → 318) — empty body needs FRAGMENT - fn foo calls undefined bar/baz (line 367 → 390) — needs INCOMPLETE Updated both entries in scripts/check_spec_examples.py. fix_allowlists.py didn't catch the drift (it only handles a subset of pattern shifts); manual update. CI lint's check_spec_examples .py step now passes locally. Co-Authored-By: Claude <noreply@anthropic.invalid>
…ing 1) Part of addressing the #492 review cycle; committed separately so the branch is in a valid state between sessions. Remaining findings to address on resume: - Declined: finding 2 (spec/12 import name already correct; CodeRabbit misread) - Fix: finding 3 (random_bool test — seed + both-outcomes assertion) - Fix: finding 4 (random_int test — assert boundary values in produced set) - Fix: finding 5 (per-test WAT import-gating assertions) - Fix: finding 6 (browser random_int — BigInt safe-integer range guard) Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ROADMAP.md`:
- Line 271: The bold metrics line "**810+ commits, 113 tagged releases, 3,368
tests, 96% coverage, 76 conformance programs, 30 examples, 13 spec chapters.**"
is inconsistent with README.md which reports 115 releases; update the
release-count in that bold stats string to match the canonical count (115) or
replace the hard count with a note like "115 tagged releases (as of v0.0.115)"
to make the source/timestamp explicit so ROADMAP and README remain consistent;
ensure you edit the bold stats text in ROADMAP.md (the shown metrics string)
and, if you choose the note approach, reference v0.0.115 as the datum.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f6bb4dc-fbd9-478e-9a73-aa25a5c9e329
📒 Files selected for processing (2)
README.mdROADMAP.md
Seven findings across two reviews; five fixed, one declined, plus
the CI lint failure resolved.
Fixed:
- ROADMAP "Where we are" paragraph now lists Random in the algebraic
effects enumeration; mirrored in README's "Key features delivered"
line so both docs are in sync.
- ROADMAP bottom "**by the numbers**" bold string updated from
"113 tagged releases" to "115 tagged releases (as of v0.0.115)" —
matches README's v0.0.115 count and makes the datum explicit.
- random_int test seeded with `random.seed(0)` and now asserts
`low in produced and high in produced` — explicitly verifies the
inclusive-range contract rather than just the cardinality of the
produced set.
- random_bool test also seeded; replaces the probabilistic
`25 <= total <= 75` bound with a deterministic
`{0, 1}.issubset(observed)` — no more flake risk.
- All four Random tests gain WAT import-gating assertions: each
asserts the op-specific `$vera.random_*` import IS present, and
the other two Random imports are NOT — confirms
`_random_ops_used` gating works op-by-op and catches regressions
where all three imports would be emitted unconditionally.
- Browser runtime's `random_int` gains a safe-integer range guard:
if `lowBig < Number.MIN_SAFE_INTEGER` or `highBig > Number
.MAX_SAFE_INTEGER`, throws a descriptive Error ("bounds exceed
JavaScript safe integer range…") instead of silently producing
wrong values via BigInt→Number precision loss. Also checks
`highBig >= lowBig` with its own message.
Declined:
- "spec/12-runtime.md line 406: import says random_int, should say
random_bool" — file already says "random_bool" in both places
(verified via `git show 0f3049b:spec/12-runtime.md`); CodeRabbit
misread the diff. No change needed.
CI lint fix:
- SKILL.md allowlist line-number drift from the Random section
insertion: 25 entries past the insertion point shifted by +1 or
+35 (depending on which hunk they fell after). `fix_allowlists
.py --fix` failed to detect the drift (same silent-OK bug as on
#491). Manually updated all 25 entries; `check_skill_examples
.py` now reports all 105 Vera blocks parse or are allowlisted.
Co-Authored-By: Claude <noreply@anthropic.invalid>
Bandit S311 flags `random.randint` / `random.random` as "pseudo- random generators are not suitable for cryptographic purposes". True — but the Random effect in #465 is explicitly scoped to games, simulations, and Monte Carlo (noted in the issue and in spec §7.7.4). Cryptographic randomness would warrant a separate `Crypto` effect with `secrets.randbelow` etc. Added `# noqa: S311` to the three host functions in `vera/codegen/api.py` with comments pointing at the design decision so a future reader doesn't have to re-discover the scoping. CI runs `ruff check --select S vera/` which I'd missed locally — noqa'ing the three sites clears the lint. Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
TESTING.md (1)
116-129:⚠️ Potential issue | 🟡 MinorMissing skip entry for
test_run[ch07_random_effect].Line 116 states "12 skipped tests", but line 9 reports "13 skipped". The discrepancy is that
ch07_random_effectis declared at theverifylevel (line 112 confirms this), sotest_run[ch07_random_effect]should be skipped and listed in the table alongside the other verify-level run skips.The table currently lists 4 verify-level run skips (ch03_slot_let_chains, ch03_slot_noncommutative, ch07_cross_module_contracts, ch07_io_sleep), but with the addition of ch07_random_effect there should now be 5.
📝 Suggested fix
Update line 116 from "12 skipped tests" to "13 skipped tests", and add this row to the level-limited skips table:
| `test_run[ch07_io_sleep]` | `ch07_io_sleep.vera` | `verify` | `run` | `verify`-level programs don't get a `run` test | +| `test_run[ch07_random_effect]` | `ch07_random_effect.vera` | `verify` | `run` | `verify`-level programs don't get a `run` test |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TESTING.md` around lines 116 - 129, Update the skipped-tests summary and table to include the missing verify-level run skip for ch07_random_effect: change the header "12 skipped tests" to "13 skipped tests" and add a table row for test_run[ch07_random_effect] with Program = ch07_random_effect.vera, Declared level = verify, Skipped stage = run, Reason = verify-level programs don't get a `run` test so it appears with the other verify-level run skips.
🤖 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 10198-10260: Add a new unit test next to test_random_int_in_range
/ test_random_int_singleton_range that exercises a zero-crossing range (e.g.
low=-2, high=2) and/or a zero singleton (0,0); seed the RNG (random.seed(0)) as
in test_random_int_in_range, compile the same source calling
Random.random_int(low, high), assert WAT imports only include "$vera.random_int"
and not "$vera.random_float" or "$vera.random_bool", then draw multiple times
with execute(result, fn_name="main") to assert every value stays within [low,
high], that both boundaries (when applicable) appear, and that the produced set
meets a minimal distribution check (e.g. len(produced) >= 4 for ranges with
multiple values) so zero-boundary behavior is explicitly covered.
---
Outside diff comments:
In `@TESTING.md`:
- Around line 116-129: Update the skipped-tests summary and table to include the
missing verify-level run skip for ch07_random_effect: change the header "12
skipped tests" to "13 skipped tests" and add a table row for
test_run[ch07_random_effect] with Program = ch07_random_effect.vera, Declared
level = verify, Skipped stage = run, Reason = verify-level programs don't get a
`run` test so it appears with the other verify-level run skips.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1d2e1a2f-ba2a-49fa-9878-8a297c55cad1
📒 Files selected for processing (5)
ROADMAP.mdTESTING.mdscripts/check_skill_examples.pytests/test_codegen.pyvera/browser/runtime.mjs
Two findings; both fixed.
Inline finding: add a zero-crossing random_int test. Prior tests
used all-positive ranges ([5,10], [7,7]) which don't exercise the
signed-integer paths in Python's random.randint, the WASM i64
marshalling, or the browser's BigInt → Number conversion. New
test_random_int_zero_crossing_range covers low=-2, high=2, seeded,
with:
- WAT import-gating (only $vera.random_int emitted)
- every draw inside [low, high]
- both boundaries produced
- explicit `assert 0 in produced` — zero value must be reachable
- distribution sanity (≥4 of 5 values in 100 draws)
Outside-diff finding: TESTING.md Skipped-tests table is missing
test_run[ch07_random_effect], the 13th skip introduced when the
new ch07_random_effect.vera conformance test at verify level was
added. Updated the intro sentence ("13 skipped tests") and added
the row to the level-limited skips table matching the existing
verify-level pattern.
Also bumped doc counts: test_codegen.py 871→872 tests and
10,803→10,846 lines; TESTING.md summary 3,368→3,369 tests,
3,355→3,356 passed; ROADMAP test-count reference updated.
Co-Authored-By: Claude <noreply@anthropic.invalid>
Owner
Author
|
@coderabbitai Fixed in f1ee528 — TESTING.md Skipped-tests section updated:
Also reconciled the overview |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings used |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New built-in
Randomeffect with three operations. Closes #465.Random.random_int(@Int, @Int) -> @Int— inclusive range[low, high]Random.random_float(@Unit) -> @Float64— uniform[0.0, 1.0)Random.random_bool(@Unit) -> @Bool— coin flipFunctions drawing random values declare
effects(<Random>), making non-determinism visible in the type signature. Mirrors the Http/Inference host-import pattern: ops are tracked via_random_ops_used, dispatched through prefixed WASM imports (vera.random_intetc.), and backed by Python'srandommodule (Python runtime) orMath.random()(browser runtime).No seeding API yet — handler-based deterministic testing via
handle[Random]is explicitly noted as future work.Why it matters
Unblocks games, simulations, shuffling, Monte Carlo methods, and randomized initial states. Was flagged as "transformative" by the Conway's Life model that drove #463 — needed alongside
IO.sleepto go from "dump 20 static frames" to "animated random cellular automaton."Tests
tests/conformance/ch07_random_effect.vera— verify level (no run-level assertions; randomness makes them flaky). Covers signature plumbing.tests/test_codegen.py::TestRandomEffect— 4 tests: inclusive range, singleton range, float in[0, 1), bool distribution (Bernoulli bounded).tests/test_browser.py— 3 parallel browser tests confirming Math.random paths behave in-distribution.Documentation
<Random>.Version bump
v0.0.114 → v0.0.115. Tagged on branch; will move to main HEAD after merge.
Test plan
mypy vera/cleanpytest tests/ -q— 3,355 passed, 13 skipped (7 new)vera runon a program using all three Random ops🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores