🐛 Strip underscore separators from a big int in the schema numeric checks#199
Conversation
…ecks A YAML 1.1 big-int literal keeps its `_` digit separators in the resolved value (`20_000_000_000_000_000_001`). The schema numeric helpers parsed that lexeme directly: `as_f64` failed on the underscores and returned `None`, so `check_numeric` bailed out at its first line and skipped every numeric keyword, and `as_i128` failed too, so even the exact `multipleOf` path was unreachable. The net effect was that an odd big integer with separators passed `multipleOf: 2` (and slipped past `minimum`/`maximum`). Both helpers now strip `_` before parsing, matching how the value itself resolves, so the numeric checks run and are exact again. Follow-up to #196. Found while addressing the same underscore issue in the OPT_SORT_KEYS review.
frenck
added
the
bugfix
|
Warning Review limit reached
Next review available in: 24 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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes a schema-validation bug where a YAML 1.1 big-integer literal with _ digit separators (e.g. 20_000_000_000_000_000_001) silently skipped every numeric JSON Schema keyword. Because the resolved BigInt retains its source lexeme (underscores included), the schema helpers as_f64 and as_i128 failed to parse it: as_f64 returning None short-circuited check_numeric at its entry gate, so minimum/maximum/exclusiveMinimum/exclusiveMaximum/multipleOf were all skipped, and as_i128 failing disabled the exact multipleOf modulo path. Both helpers now strip _ before parsing, matching how the value itself resolves. This is a focused follow-up to #196 (exact integer multipleOf) and the underscore issue noted in #197.
Changes:
- Strip
_separators inas_i128before parsing aBigIntlexeme, restoring the exactmultipleOfinteger path. - Strip
_separators inas_f64before parsing aBigIntlexeme, socheck_numericno longer bails out and bound checks run. - Add a regression test covering underscore-separated YAML 1.1 big integers against
multipleOfandminimum.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/schema/mod.rs | Strips _ from BigInt lexemes in as_i128 and as_f64 so numeric schema keywords apply to underscored YAML 1.1 big integers. |
| tests/features/test_schema.py | Adds test_numeric_checks_handle_yaml_1_1_underscore_big_ints verifying multipleOf and minimum are enforced on underscored big-int literals. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #199 +/- ##
=======================================
Coverage 92.54% 92.54%
=======================================
Files 40 40
Lines 11931 11931
=======================================
Hits 11041 11041
Misses 890 890 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Breaking change
None. It makes the schema numeric keywords apply to a big-integer value that has YAML 1.1
_digit separators, where they were previously skipped. Values only become more correctly validated.Proposed change
A YAML 1.1 big-int literal keeps its
_digit separators in the resolved value (20_000_000_000_000_000_001). The schema numeric helpers parsed that lexeme directly:as_f64failed on the underscores and returnedNone, socheck_numericbailed out at its first line and skipped every numeric keyword, andas_i128failed too, so even the exactmultipleOfpath was unreachable. The net effect was that an odd big integer with separators passedmultipleOf: 2(and slipped pastminimum/maximum).Both helpers now strip
_before parsing, matching how the value itself resolves, so the numeric checks run and are exact again. Follow-up to #196.Found while addressing the same underscore issue flagged in the
OPT_SORT_KEYSreview (#197).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.