Core: add deterministic math module

[flyingrobots]

Summary

• introduce deterministic math module covering scalar, vector, matrix, quaternion, and PRNG operations
• add fixture-driven test suite to guarantee repeatable results across runs
• update execution plan to reflect math validation work and line up next intent

Testing

• cargo test

[coderabbitai]

Summary by CodeRabbit

• New Features

  • Added math module providing deterministic vector, matrix, and quaternion operations for 3D calculations.
  • Added deterministic pseudo-random number generator with configurable seeding and integer/float generation.
  • Exported math module for use across the library.

• Tests

  • Added comprehensive test suite with fixture-based validation for all math primitives and PRNG behavior.

Walkthrough

Adds a deterministic math module (scalars, Vec3, Mat4, Quat, Prng) and exports it from lib.rs; adds JSON fixtures and a fixture-driven Rust test harness; adds dev-dependencies for test fixtures; updates execution plan and decision log docs.

Changes

Cohort / File(s)	Summary
Math Module Implementation crates/rmg-core/src/math.rs	New module: scalar helpers (clamp, deg_to_rad, rad_to_deg), Vec3, Mat4 (column-major), Quat, and Prng (xoroshiro128+). Includes const constructors, conversion impls, transforms, RNG sampling with rejection sampling, deterministic edge-case handling, and unit tests for PRNG behaviors.
Module Export crates/rmg-core/src/lib.rs	Exported new public module: pub mod math;.
Test Fixtures crates/rmg-core/tests/fixtures/math-fixtures.json	Added JSON fixtures specifying tolerances and expected values for scalars, Vec3, Mat4, Quat, and PRNG sequences for deterministic validation.
Test Harness crates/rmg-core/tests/math_validation.rs	New test file: loads fixtures via a Lazy static, deserializes with serde, and asserts math primitives and PRNG outputs against fixtures with configurable tolerances.
Development Dependencies crates/rmg-core/Cargo.toml	Added dev-dependencies: once_cell, serde (with derive feature), and serde_json for fixture-driven tests.
Documentation docs/execution-plan.md, docs/decision-log.md	Execution plan updated (shifted focus toward ECS storage planning); decision log entry added for landing math validation harness and fixture work.

Sequence Diagram(s)

sequenceDiagram
  participant Test as Test Harness
  participant Fixture as JSON Fixture
  participant Math as rmg_core::math
  participant Assert as Assertion

  Note over Test,Fixture: Load and deserialize fixtures at test startup
  Test->>Fixture: read fixtures (serde_json)
  Fixture-->>Test: structured fixture data

  loop per fixture case
    Test->>Math: call operation (clamp / deg_to_rad / Vec3 / Mat4 / Quat / Prng)
    Math-->>Test: computed result (f32 / arrays / sequences)
    Test->>Assert: compare result vs expected within tolerance
    Assert-->>Test: pass / fail
  end

  alt failure
    Note over Test: test runner reports failure with details
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Pay attention to:
  • PRNG: seeding (SplitMix64), xoroshiro128+ state transitions, rejection sampling correctness for inclusive ranges.
  • Quaternion logic: axis-angle edge cases, normalization, multiplication order, and mat4 conversion sign/indexing.
  • Matrix indexing and transform_point vs transform_direction (column-major handling and w-component).
  • Normalization branches and near-zero handling across Vec3/Quat to avoid NaNs.
  • Test fixtures: EPS/tolerance values and parity with f32 deterministic expectations.

Poem

Vectors fold and quaternions spin,
Matrices map the world within,
Seeds whisper numbers neat and small,
Fixtures hold each expected call,
Tests confirm the math for all.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "Core: add deterministic math module" is directly aligned with the primary addition in this changeset. The math.rs module with its scalar utilities, vector/matrix/quaternion types, and PRNG implementation constitutes the core substance of the PR. The title is concise, clear, and immediately conveys the primary purpose without vague fluff. While the changeset also includes test fixtures, validation harness, and documentation updates, these are properly scoped as supporting changes rather than primary features—the title correctly prioritizes the math module as the headline.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch echo/math-validation-fixtures

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d2a5958 and 523398b.

📒 Files selected for processing (4)

• crates/rmg-core/src/math.rs (1 hunks)
• crates/rmg-core/tests/fixtures/math-fixtures.json (1 hunks)
• crates/rmg-core/tests/math_validation.rs (1 hunks)
• docs/execution-plan.md (4 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

docs/execution-plan.md

📄 CodeRabbit inference engine (AGENTS.md)

Start each work session by updating Today’s Intent in docs/execution-plan.md

Files:

• docs/execution-plan.md

docs/**

📄 CodeRabbit inference engine (AGENTS.md)

Document meaningful design choices in the docs/ directory (specs, diagrams, memorials) and keep impacted docs in sync with code changes

Files:

• docs/execution-plan.md

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Public Rust APIs across crates (e.g., rmg-core, rmg-ffi, rmg-wasm) must include rustdoc comments explaining intent, invariants, and usage

Files:

• crates/rmg-core/tests/math_validation.rs
• crates/rmg-core/src/math.rs

🧠 Learnings (1)

📚 Learning: 2025-10-26T02:42:44.664Z

Learnt from: CR
PR: flyingrobots/echo#0
File: AGENTS.md:0-0
Timestamp: 2025-10-26T02:42:44.664Z
Learning: Applies to docs/execution-plan.md : Start each work session by updating Today’s Intent in docs/execution-plan.md

Applied to files:

• docs/execution-plan.md

🧬 Code graph analysis (1)

crates/rmg-core/tests/math_validation.rs (1)

crates/rmg-core/src/math.rs (11)

• new (44-46)
• new (145-147)
• new (229-231)
• clamp (12-15)
• deg_to_rad (18-20)
• rad_to_deg (23-25)
• from (126-128)
• from (210-212)
• from (348-350)
• from_axis_angle (246-262)
• from_seed (369-375)

🔇 Additional comments (22)

docs/execution-plan.md (3)

8-8: LGTM—Markdown formatting fixed.

All four headings now have the required blank lines. MD022 violations from past reviews are resolved.

Also applies to: 35-35, 104-104, 116-116

---

38-39: Intent and queue are finally aligned. Good.

Today's Intent focuses on ECS storage, and the Next Up Queue matches with ECS at position #1 marked in progress. The math validation work is documented in the decision log as established, which is what this PR delivers. Consistent.

Also applies to: 105-109

---

95-97: Decision log chronological order is correct.

Entries are now properly sorted in ascending order: two 2025-10-25 entries followed by 2025-10-26. This fixes the temporal ordering mess from past reviews.

crates/rmg-core/tests/fixtures/math-fixtures.json (3)

43-43: Edge case fixtures added as requested.

Zero vector normalization and zero-axis from_axis_angle are now covered. Both return deterministic fallbacks (zero vector and identity quaternion respectively).

Also applies to: 98-98

---

69-92: Transform fixtures properly named to match API semantics.

transform_point (w=1, translation applied) and transform_direction (w=0, translation ignored) are clearly distinguished. Past review confusion is resolved.

---

2-5: Tolerance block is properly integrated and actively used in all assertions.

The fixture's tolerance values flow through the entire test harness: deserialized into the Tolerance struct, passed to every assertion function (assert_scalar, assert_vec3, assert_quat, assert_mat4), and used to compute allowed error margins via tol.allowed_error(expected). Floating point comparisons are protected from flakiness. No issues here.

---

From: Linus Torvalds torvalds@linux-foundation.org
To: {PR_AUTHOR_NAME} <{PR_AUTHOR_EMAIL}>
Cc: code-rabbit@vger.kernel.org

Look, I actually verified this thing works instead of just cargo-culting assumptions. The tolerance block isn't some decorative JSON—it's wired into every single assertion. You're computing allowed errors correctly, the struct definitions line up, the fixtures deserialize properly. This is how you write tests that don't randomly fail at 3 AM. Good work.

BunBun

crates/rmg-core/tests/math_validation.rs (6)

14-19: Fixture loading error handling is fixed.

Parse errors now include the error message and file path in the panic. The validate() call ensures no fixture arrays are empty. Past review issues addressed.

---

32-56: Empty fixture validation prevents silent test passes.

The validate() method checks all fixture arrays are non-empty at initialization. Tests can't silently pass with zero coverage anymore.

---

340-372: Type mismatch panic messages are now informative.

Both length and normalize type-mismatch panics include the fixture index and input value. When a fixture has the wrong type, developers can immediately locate it in the JSON.

---

378-384: Mat4.multiply context includes fixture index.

The context string now includes the loop index and brief matrix info. Failures are traceable to specific fixtures.

---

386-397: Point semantics clarified with inline comment.

Line 389 explains that fixture vectors are treated as points with homogeneous w=1. This addresses the naming confusion from past reviews.

---

481-493: PRNG state coupling issue is FIXED.

Line 482 re-initializes the Prng from the fixture seed before testing integers. Float and int tests are now independent. The past review's fragility concern is resolved.

crates/rmg-core/src/math.rs (10)

11-15: Clamp validation fixed—assert in release mode.

Line 13 uses assert! (not debug_assert!), so invalid ranges panic in both debug and release builds. Past review issue resolved.

---

27-46: Vec3 docs clearly explain point vs direction semantics.

The struct docs reference Mat4 transform methods and explain when to treat vectors as points or directions. Constructor docs mention world-space metres and finite value expectations. Well documented.

Based on coding guidelines.

---

112-122: Vec3.normalize correctly uses len <= EPSILON.

Line 118 compares len directly without abs() since sqrt is always non-negative. Past review nit addressed. Zero-length behavior is documented.

---

131-206: Mat4 docs state column-major layout and transform semantics.

Struct docs explain column-major storage and affine/perspective handling. transform_point (w=1) and transform_direction (w=0) are clearly documented with homogeneous coordinate semantics. Excellent API docs.

Based on coding guidelines.

---

242-262: Quat.from_axis_angle docs now match implementation.

Lines 244-245 accurately state that zero-length axis returns identity quaternion. Past review's "docs lie" complaint is fixed—no more gaslighting.

---

353-375: PRNG docs are now accurate—xoroshiro128+, stateful, non-cryptographic.

Lines 353-357 correctly describe the algorithm as xoroshiro128+ (not "counter-based" or "**") and state it's deterministic within a single process. Past review's documentation accuracy issue is completely resolved.

---

389-397: next_f32 shift by 41 is correct for 23-bit mantissa.

Line 395 shifts right by 41 bits (64 - 23 = 41) to extract exactly 23 mantissa bits. Past review's hand-waving complaint is fixed.

---

399-424: next_int rejection sampling eliminates modulo bias and overflow.

Implementation uses i64 arithmetic for span calculation (line 405), fast path for span==1 (line 406-408), power-of-two masking when possible (line 410-411), and proper rejection sampling with bound calculation for non-power-of-two spans (lines 413-419). This completely addresses the past review's "unacceptable" criticism about bias and overflow.

---

426-442: SplitMix64 seeding adds ergonomic one-u64 constructor.

from_seed_u64 uses standard SplitMix64 constants to derive both state words from a single seed, providing better diffusion than the (0,0) remap. Past review suggestion implemented.

---

445-468: Unit tests cover next_int edge cases.

Tests validate equal bounds (min==max), full i32 range, and negative ranges. Past review's testing request is satisfied.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 10

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6dfda59 and bed9b60.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

• crates/rmg-core/Cargo.toml (1 hunks)
• crates/rmg-core/src/lib.rs (1 hunks)
• crates/rmg-core/src/math.rs (1 hunks)
• crates/rmg-core/tests/fixtures/math-fixtures.json (1 hunks)
• crates/rmg-core/tests/math_validation.rs (1 hunks)
• docs/execution-plan.md (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Public Rust APIs must have rustdoc comments explaining intent, invariants, and usage; treat missing docs as a failing test

Files:

• crates/rmg-core/src/lib.rs
• crates/rmg-core/tests/math_validation.rs
• crates/rmg-core/src/math.rs

🧠 Learnings (1)

📚 Learning: 2025-10-25T13:22:15.193Z

Learnt from: CR
PR: flyingrobots/echo#0
File: AGENTS.md:0-0
Timestamp: 2025-10-25T13:22:15.193Z
Learning: Use Neo4j threads: echo-devlog for daily journal/decisions/blockers and echo-spec for high-level proposals

Applied to files:

• docs/execution-plan.md

🧬 Code graph analysis (1)

crates/rmg-core/tests/math_validation.rs (1)

crates/rmg-core/src/math.rs (11)

• new (35-37)
• new (122-124)
• new (179-181)
• clamp (12-15)
• deg_to_rad (18-20)
• rad_to_deg (23-25)
• from (109-111)
• from (166-168)
• from (290-292)
• from_axis_angle (193-204)
• from_seed (303-309)

🔇 Additional comments (7)

crates/rmg-core/src/lib.rs (1)

14-15: Export looks fine. Don’t ship undocumented public APIs.

You’ve got #![deny(missing_docs)]. Ensure every public item in math has rustdoc with intent/invariants, or this will fail builds.

crates/rmg-core/Cargo.toml (1)

11-14: Fix missing toolchain pinning and MSRV, but edition = "2024" is valid.

The Rust 2024 edition is valid and won't break CI. However, add a rust-toolchain.toml file to pin the toolchain version, and add rust-version to Cargo.toml to specify the MSRV. This prevents unintended compiler version drift and ensures reproducible builds. The dev-dependencies are fine as-is.

crates/rmg-core/tests/math_validation.rs (5)

176-192: Fine, this one's acceptable.

Straightforward fixture-driven test. The logic is sound. Not much to screw up here.

---

236-251: Naming inconsistency, but I'll allow it.

The fixture is called transform_vec3 but you're calling transform_point. I assume that's the actual API name, so fine. But this is the kind of inconsistency that makes code harder to understand.

---

253-279: This is fine.

Straightforward fixture validation. No issues here.

---

11-144: Where's the documentation?

Yeah, I see these are test-internal structs, not public APIs, so the rustdoc guideline doesn't technically apply. But you've got 130+ lines of data structures with ZERO explanation of what they represent or how they map to the fixture format.

The FixtureValue enum with #[serde(untagged)] is particularly clever, but good luck to the next person trying to understand why some fixtures need scalars and others need vectors without reading through the entire test suite.

Add at least a module-level doc comment explaining the fixture structure.

⛔ Skipped due to learnings

Learnt from: CR
PR: flyingrobots/echo#0
File: AGENTS.md:0-0
Timestamp: 2025-10-25T13:22:15.193Z
Learning: Applies to **/*.rs : Public Rust APIs must have rustdoc comments explaining intent, invariants, and usage; treat missing docs as a failing test

---

146-146: Remove this review comment—it's based on a fundamental misunderstanding of how fixture-based tests work.

These aren't algorithmic tests that compute values subject to floating-point accumulation errors across different platforms. These are fixture-based regression tests: the expected values are hardcoded in JSON and loaded at compile time. The implementation either matches the fixture within 1e-6 or it doesn't. The epsilon doesn't need to account for "accumulated error across platforms" because there's no computation happening across platforms—just IEEE 754-compliant floating-point comparisons.

The fixture precision itself is ~7 decimal places (see 0.70710677 in the JSON), so 1e-6 tolerance is explicitly calibrated to that, not arbitrary. That choice is sound and will be deterministic on any system that respects IEEE 754, which is every modern architecture.

If you want to criticize something, criticize the lack of CI/CD infrastructure or the absence of documentation about where the fixtures came from—but don't imply the epsilon is a ticking time bomb on different platforms when the entire test structure prevents that problem.

Likely an incorrect or invalid review comment.

[coderabbitai]

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)

docs/execution-plan.md (1)

89-95: ---

Decision log violates reverse-chronological ordering—exact fix required.

The table is currently oldest-first (2025-10-23 through 2025-10-26), but you need newest-first. Your 2025-10-26 entry sits before a 2025-10-25 entry on line 95, which breaks the ordering you committed to. Reorder the entire table so 2025-10-26 entries come first, then both 2025-10-25 entries (they're separate decisions), then 10-24, then 10-23. A decision log that can't maintain basic chronology isn't a log—it's noise.

 | Date | Decision | Context | Follow-up |
 | ---- | -------- | ------- | --------- |
+| 2025-10-26 | Adopt RMG + Confluence as core architecture | RMG v2 (typed DPOi engine) + Confluence replication baseline | Scaffold rmg-core/ffi/wasm/cli crates; implement rewrite executor spike; integrate Rust CI; migrate TS prototype to `/reference` |
+| 2025-10-25 | Math validation fixtures & Rust test harness | Established deterministic scalar/vector/matrix/quaternion/PRNG coverage in rmg-core | Extend coverage to browser environments and fixed-point mode |
 | 2025-10-25 | Language direction pivot: Echo core to Rust | TypeScript validated specs; long-term determinism enforced via Rust + C ABI + Lua scripting | Update Phase 1 backlog: scaffold Rust workspace, port ECS/diff engine, FFI bindings |
 | 2025-10-24 | Branch tree spec v0.1: roaring bitmaps, chunk epochs, content-addressed IDs | Feedback loop to handle deterministic merges | Implement roaring bitmap integration |
 | 2025-10-23 | Monorepo seeded with pnpm & TypeScript skeleton | Baseline repo reset from Caverns to Echo | Implement Phase 0 specs |
-| 2025-10-26 | Adopt RMG + Confluence as core architecture | RMG v2 (typed DPOi engine) + Confluence replication baseline | Scaffold rmg-core/ffi/wasm/cli crates; implement rewrite executor spike; integrate Rust CI; migrate TS prototype to `/reference` |
-| 2025-10-25 | Math validation fixtures & Rust test harness | Established deterministic scalar/vector/matrix/quaternion/PRNG coverage in rmg-core | Extend coverage to browser environments and fixed-point mode |

BunBun

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bed9b60 and 67e38fc.

📒 Files selected for processing (1)

• docs/execution-plan.md (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

docs/execution-plan.md

📄 CodeRabbit inference engine (AGENTS.md)

Start each work session by updating Today’s Intent in docs/execution-plan.md

Files:

• docs/execution-plan.md

docs/**

📄 CodeRabbit inference engine (AGENTS.md)

Document meaningful design choices in the docs/ directory (specs, diagrams, memorials) and keep impacted docs in sync with code changes

Files:

• docs/execution-plan.md

🧠 Learnings (2)

📚 Learning: 2025-10-26T02:42:44.664Z

Learnt from: CR
PR: flyingrobots/echo#0
File: AGENTS.md:0-0
Timestamp: 2025-10-26T02:42:44.664Z
Learning: Applies to docs/execution-plan.md : Start each work session by updating Today’s Intent in docs/execution-plan.md

Applied to files:

• docs/execution-plan.md

📚 Learning: 2025-10-26T02:42:44.664Z

Learnt from: CR
PR: flyingrobots/echo#0
File: AGENTS.md:0-0
Timestamp: 2025-10-26T02:42:44.664Z
Learning: Applies to docs/decision-log.md : Record milestones, blockers, decisions, outcomes, and next steps in docs/decision-log.md

Applied to files:

• docs/execution-plan.md

[coderabbitai]

Actionable comments posted: 7

♻️ Duplicate comments (2)

crates/rmg-core/tests/fixtures/math-fixtures.json (1)

2-5: Good—you finally added tolerances. Tests might now stop flaking across libm/CPU.

Exactly what was requested previously. Keep this consistent with the harness. BunBun

crates/rmg-core/tests/math_validation.rs (1)

458-470: Decoupling PRNG int tests from float count is the correct fix.

Re-seeding for ints kills hidden state coupling. This prevents “works on my JSON” fragility. BunBun

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67e38fc and fec158e.

📒 Files selected for processing (4)

• crates/rmg-core/src/math.rs (1 hunks)
• crates/rmg-core/tests/fixtures/math-fixtures.json (1 hunks)
• crates/rmg-core/tests/math_validation.rs (1 hunks)
• docs/execution-plan.md (3 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Public Rust APIs across crates (e.g., rmg-core, rmg-ffi, rmg-wasm) must include rustdoc comments explaining intent, invariants, and usage

Files:

• crates/rmg-core/tests/math_validation.rs
• crates/rmg-core/src/math.rs

docs/execution-plan.md

📄 CodeRabbit inference engine (AGENTS.md)

Start each work session by updating Today’s Intent in docs/execution-plan.md

Files:

• docs/execution-plan.md

docs/**

📄 CodeRabbit inference engine (AGENTS.md)

Document meaningful design choices in the docs/ directory (specs, diagrams, memorials) and keep impacted docs in sync with code changes

Files:

• docs/execution-plan.md

🧠 Learnings (1)

📚 Learning: 2025-10-26T02:42:44.664Z

Learnt from: CR
PR: flyingrobots/echo#0
File: AGENTS.md:0-0
Timestamp: 2025-10-26T02:42:44.664Z
Learning: Applies to docs/decision-log.md : Record milestones, blockers, decisions, outcomes, and next steps in docs/decision-log.md

Applied to files:

• docs/execution-plan.md

🧬 Code graph analysis (1)

crates/rmg-core/tests/math_validation.rs (1)

crates/rmg-core/src/math.rs (11)

• new (35-37)
• new (127-129)
• new (197-199)
• clamp (12-15)
• deg_to_rad (18-20)
• rad_to_deg (23-25)
• from (114-116)
• from (184-186)
• from (313-315)
• from_axis_angle (211-227)
• from_seed (326-332)

🪛 markdownlint-cli2 (0.18.1)

docs/execution-plan.md

101-101: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

112-112: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

🔇 Additional comments (4)

crates/rmg-core/tests/math_validation.rs (3)

9-14: Finally, an error that tells humans what exploded. Keep this standard.

Parsing now panics with file path and serde error. This is the minimum bar. BunBun

---

28-50: Good: fixture sanity checks prevent “green” tests with zero coverage.

This closes a common footgun. Keep extending as new sections appear. BunBun

---

331-346: Type-mismatch panics now point to the exact offender. About time.

Including indices and values saves hours of binary searching the JSON. BunBun

Also applies to: 348-363

crates/rmg-core/src/math.rs (1)

11-15: Core math/PRNG fixes: these are the standards. Keep them.

• clamp asserts in release.
• from_axis_angle guards zero axis.
• PRNG docs now correctly state xoroshiro128+.
• next_int removes modulo bias and handles full i32 ranges.
  This is the bar from now on. BunBun

Also applies to: 210-218, 318-344, 353-375

[coderabbitai]

Actionable comments posted: 6

♻️ Duplicate comments (9)

crates/rmg-core/src/math.rs (4)

11-15: Clamp validates in release. Finally.

Release assertion instead of debug_assert is correct for deterministic behavior.

---

353-361: PRNG docs now correctly say xoroshiro128+.

Accurate and non‑crypto disclaimer is present. Good.

---

389-397: Float path uses exact 23‑bit mantissa (>> 41).

Uniform [0,1) for f32 via bit‑trick is correct.

---

399-424: Inclusive int range uses rejection sampling and 64‑bit math.

No modulo bias; full i32 span safe. Good.

crates/rmg-core/tests/fixtures/math-fixtures.json (1)

1-7: Tolerances included. Good.

Absolute/relative 1e‑6 prevent cross‑platform flakiness.

crates/rmg-core/tests/math_validation.rs (4)

14-19: Fixture parsing now debugs itself.

Clear panic with file path + error; validation step blocks empty sets. Solid.

---

336-351: Contextful failures for vec3 length/normalize.

Enumerated, self‑describing panic paths. Exactly what you want at 3am.

Also applies to: 353-368

---

371-381: Multiply tests identify the failing case.

Enumerating and passing context improves triage speed.

---

447-478: Decoupled PRNG int replay.

Re-seeding before int generation removes fragile coupling. Keep it.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fec158e and d2a5958.

📒 Files selected for processing (5)

• crates/rmg-core/src/math.rs (1 hunks)
• crates/rmg-core/tests/fixtures/math-fixtures.json (1 hunks)
• crates/rmg-core/tests/math_validation.rs (1 hunks)
• docs/decision-log.md (1 hunks)
• docs/execution-plan.md (3 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

docs/decision-log.md

📄 CodeRabbit inference engine (AGENTS.md)

Record milestones, blockers, decisions, outcomes, and next steps in docs/decision-log.md

Files:

• docs/decision-log.md

docs/**

📄 CodeRabbit inference engine (AGENTS.md)

Document meaningful design choices in the docs/ directory (specs, diagrams, memorials) and keep impacted docs in sync with code changes

Files:

• docs/decision-log.md
• docs/execution-plan.md

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Public Rust APIs across crates (e.g., rmg-core, rmg-ffi, rmg-wasm) must include rustdoc comments explaining intent, invariants, and usage

Files:

• crates/rmg-core/tests/math_validation.rs
• crates/rmg-core/src/math.rs

docs/execution-plan.md

📄 CodeRabbit inference engine (AGENTS.md)

Start each work session by updating Today’s Intent in docs/execution-plan.md

Files:

• docs/execution-plan.md

🧬 Code graph analysis (1)

crates/rmg-core/tests/math_validation.rs (1)

crates/rmg-core/src/math.rs (11)

• new (44-46)
• new (145-147)
• new (229-231)
• clamp (12-15)
• deg_to_rad (18-20)
• rad_to_deg (23-25)
• from (126-128)
• from (210-212)
• from (348-350)
• from_axis_angle (246-262)
• from_seed (369-375)

🪛 markdownlint-cli2 (0.18.1)

docs/execution-plan.md

101-101: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

112-112: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

🔇 Additional comments (2)

docs/decision-log.md (1)

13-13: Decision log entry reads clean; keep timeline sorted.

Chronology and scope match execution-plan. No changes needed. Keep adding file/commit refs when available.

docs/execution-plan.md (1)

94-96: Decision table order is chronological; keep it that way.

Entries progress 2025‑10‑23 → 10‑24 → 10‑25 → 10‑25 → 10‑26. Good. No action.