-
Notifications
You must be signed in to change notification settings - Fork 1
docs(math): deterministic harness notes #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughDocumentation updates marking completion of Rust-based math validation harness implementation and adding cross-language parity notes to the deterministic math specification, tracking which operations have Rust implementations versus remaining TODO items. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Possibly related PRs
Poem
From: Linus Torvalds torvalds@linux-foundation.org Look, documentation is great. It really is. But let's be honest here: you're updating markdown files to track what's already been implemented elsewhere. That's not a feature, that's notes about features. The Rust harness is done. Good. Vec3 has add/sub/scale/dot/cross? Fantastic. But for Vec2 and Vec4 you left them as TODO. TODO. In documentation. That's like leaving a half-eaten sandwich on the kitchen counter and documenting that you did it. Either finish it or don't, but don't make me read about your incomplete work. That said, at least you're being transparent about what's missing. I respect that. The parity tracking is sensible. Just don't make a habit of committing aspirational documentation without the code to back it up. BunBun Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
docs/math-validation-plan.md(2 hunks)docs/spec-deterministic-math.md(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/spec-deterministic-math.md
50-50: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
56-56: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
62-62: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Security Audit
🔇 Additional comments (3)
docs/math-validation-plan.md (2)
45-47: Documentation accurately reflects completed Rust harness strategy. The Tooling section now correctly describes the shift from ephemeral Vitest snapshots to a deterministic, referenceable Rust validation harness backed by JSON fixtures. Clear roadmap to TypeScript mirroring. ✓
61-62: Task completion tracking is consistent with PR objectives. Fixture generation and Rust-based validation suite implementation are correctly marked complete; remaining tasks properly remain open. No discrepancies detected. ✓docs/spec-deterministic-math.md (1)
29-29: Parity notes are well-placed, but verify they track actual Rust implementation status. The six Rust parity additions (lines 29, 48, 54, 60, 65, 91) clearly document cross-language capability gaps and provide valuable context for FFI/WASM adapter developers. However, these notes make specific claims about method availability that could become stale if Rust implementations progress separately. Ensure this documentation stays synchronized with actual rmg-core code as development continues.Can you confirm these parity claims match the current state of rmg-core implementations?
- Vec3: add/sub/scale/dot/cross/length/normalize present; Vec2/Vec4 actually TODO?
- Mat4: multiply/transform_point present; identity/fromRotation/invert actually pending?
- Quat: identity/fromAxisAngle/multiply/normalize/to_mat4 present; slerp actually TBD?
- PRNG: seeding/next_f32/next_int present; state/jump actually deferred?
Also applies to: 48-48, 54-54, 60-60, 65-65, 91-91
Brings over small doc additions from
docs/update-math-planinto main:No code changes. Keeps runtime determinism contract untouched.