Repository navigation
Reject a negative stored level and negative timestamps - #123
Conversation
The fillable domain check now refuses a negative bucket level, a negative stored timestamp and a negative timestamp argument at both reads and the fill, with LeakyBucketNegativeLevel and LeakyBucketNegativeTimestamp. mutants.toml named .mutation-test/suite.sh, which is not tracked; the tracked suite command is .mutation-test/check.sh. Refs #115 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Refs #115 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 40 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (5)
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 |
Refs #115 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Refs #115 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Refs #115 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…cord Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Keeps everything #122 and #123 landed. settle takes the domain check's new signature, so it refuses a negative level and negative timestamps as fill does. The settle mutants are renumbered M23 to M27 behind main's M20 to M22, and the scan record keeps both sides' entries. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Refs #115
Delivers the "prevent negatives" half of what #116 left open. The directed rounding half is not delivered; see below.
What changed
checkFillableDomainnow also refuses a negative storedlevel, a negative storedtimestampand a negativetimestampargument, atlevelAt,headroomAtandfill, before the amount is looked at.LeakyBucketNegativeLevel(Float level)andLeakyBucketNegativeTimestamp(Float timestamp). The second serves both the stored and the supplied timestamp and carries the offending value.mutants.tomlnamed.mutation-test/suite.sh, which is not tracked; it now names the tracked.mutation-test/check.sh. M09, M10 and M16-M18 are retargeted at the new spelling; M20-M22 are added.Directed rounding: not delivered
The ruling on #115 is that the level is never understated and the drain never overstated.
rain-math-float0.2.4 cannot express that: its own docs say "There is no concept of rounding modes", andadd,subandmulreport no remainder. Measured against 0.2.4 with a throwaway test (not committed):1 + 1e-70111 - 1e-700.99...9(67 nines)1 - 1e-8011(1e66+1)^21e132 + 2e661e132 + 2e66 + 1Everything truncates toward zero, at three sites: the exponent alignment in
LibDecimalFloatImplementation.add(signedCoefficientB /= 10 ** diff, with the smaller operand dropped whole pastADD_MAX_EXPONENT_DIFF),mulDivinmul, and the coefficient shrink inpackLossy. For the bucket,elapsed * leakRatealready rounds the safe way,level + amountrounds the unsafe way, andlevel - draingoes either way depending on the exponent gap.What the float library would need to expose:
add,subandmulvariants that take a rounding direction, or that return a lossless flag covering alignment,mulDivand packing together, plus a one-ulp step up and down. Nothing here works around the gap.QA
if (bucket.level.lt(0))->if (false), M21if (bucket.timestamp.lt(0))->if (false), M22if (timestamp.lt(0))->if (false): all three SURVIVED the pre-existing 58-test suite at f954fe8 (19/22), and are KILLED by testANegativeFractionIsRefusedLikeAnyNegative (M20 also by testANegativeLevelCannotBuyHeadroomAboveCapacity) at 25e49b1: 22/22 killed, 0 no-run, 0 harness errors, 63 tests; re-probed 22/22 at 4e60fed after a lint-only test fix.🤖 Generated with Claude Code