🐛 Check integer multipleOf exactly instead of via a lossy float#196
Conversation
The `multipleOf` check divided two `f64`s and accepted the result within a relative tolerance (`1e-9 * ratio`). That tolerance grows without bound as the value grows, and a large integer loses precision the moment it becomes an `f64`, so an odd integer such as `10000000000000001` validated as a multiple of `2`. When both the value and `multipleOf` are integers the check is now exact integer modulo, done on `i128` so it also covers big integers past `i64`. When either operand is a float it stays best-effort within floating-point precision, but the tolerance is scaled to the operands' magnitude (a few ULPs of the nearest multiple) rather than the ratio, so `3.0000001` is no longer a multiple of `1.0` while `0.3` still counts as a multiple of `0.1` (binary rounding). Verified against the `jsonschema` reference for the integer cases. Found by a differential sweep against `jsonschema`.
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #196 +/- ##
==========================================
+ Coverage 92.46% 92.71% +0.25%
==========================================
Files 40 40
Lines 11871 12087 +216
==========================================
+ Hits 10976 11206 +230
+ Misses 895 881 -14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness bug in the JSON Schema validator's multipleOf check (introduced in the related #190). The previous implementation divided two f64s and accepted the result within a relative tolerance (1e-9 * ratio), which grew unboundedly with magnitude and lost integer precision once a value became an f64 — so an odd integer like 10000000000000001 incorrectly validated as a multiple of 2. The fix performs an exact integer-modulo check (on i128, covering big integers past i64) when both operands are integers, and keeps a tighter, magnitude-scaled best-effort tolerance for float operands.
Changes:
- Rework
is_multiple_ofto take the rawValues and use exacti128modulo when both value andmultipleOfare integers, falling back to a ULP-scaled float comparison otherwise. - Add an
as_i128helper that extracts an exact integer fromValue::Int/Value::BigInt. - Add regression tests covering large odd integers, big integers past
i64, genuine multiples, and a non-integral float multiple.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/schema/mod.rs |
Replaces the lossy float multipleOf check with exact i128 integer modulo plus a tightened float fallback; adds as_i128 helper. |
tests/features/test_schema.py |
Adds test_multiple_of_is_exact_for_integers covering exact integer, big-integer, and non-integral-float cases. |
I verified the call-site signature update (single caller), the Value enum variants used by as_i128, the modulo-by-zero guard, and the float tolerance behavior for the documented cases (0.3/0.1 accepted, 3.0000001 rejected). The change is correct and well-scoped; my only note is a nit about pinning the intentional float-acceptance divergence with a regression test.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Breaking change
None that affects a correct schema: values only validate more correctly. A large integer that used to slip past
multipleOfnow fails, matchingjsonschema. One deliberate, documented divergence fromjsonschemais kept: a float like0.3is still accepted as a multiple of0.1(the user-friendly reading), wherejsonschema's exact-binary arithmetic rejects it.Proposed change
The
multipleOfcheck divided twof64s and accepted the result within a relative tolerance (1e-9 * ratio). That tolerance grows without bound as the value grows, and a large integer loses precision the moment it becomes anf64, so an odd integer such as10000000000000001validated as a multiple of2.When both the value and
multipleOfare integers the check is now exact integer modulo, done oni128so it also covers big integers pasti64. When either operand is a float it stays best-effort within floating-point precision, but the tolerance is scaled to the operands' magnitude (a few ULPs of the nearest multiple) rather than the ratio, so3.0000001is no longer a multiple of1.0while0.3still counts as a multiple of0.1(binary rounding). Verified against thejsonschemareference for the integer cases.Found by a differential sweep against
jsonschema.Type of change
Additional information
Checklist
uv run pytestpasses locally. A pull request cannot be merged unless CI is green.uv run ruff check .anduv run ruff format --check .pass.cargo fmt --checkandcargo clippy --all-targets -- -D warningspass.If the change is user-facing:
docs/is added or updated, anddocs/verify_examples.pystill passes.