Repository navigation
fix(budget): keep the cumulative budget in whole cents - #135
Conversation
The in-memory and persisted budget ledgers added and compared float64 dollars, so residuals flipped exact-boundary checks: a 0.30 budget refused 0.10 then 0.20, and the state file stored 0.30000000000000004. Amounts are now int64 cents. A purchase total rounds up to the cent and the budget rounds down, so rounding never admits more than exact arithmetic would; conversion goes through the shortest decimal of the float, so 0.07 is 7 cents. The state file format is unchanged: amounts are written as exact decimals, an older file's fraction of a cent reads rounded up, and a null amount is now corrupt instead of 0. Closes #82 Co-Authored-By: Claude Opus 5.5 <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 details
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. |
|
Independent review (claude-fleet, cold): CHANGES REQUESTED 4fcd502 The cents conversion is correct and fails toward refusing. There is one small blocking gap: an overflowing state file is not treated as corrupt. Blocking:
Attacked and holding, on an export of 4fcd502 with scratch tests:
|
totalUSD summed int64 cents unchecked, so about 9300 maximum-size reservations in a corrupt file wrapped the total negative, which reads as headroom. validate now sums with an overflow check against the same 1e13 USD bound each amount has and refuses the file as corrupt past it. The in-memory ledger stays bounded by the limit, so it cannot wrap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
New head
|
|
Independent review (claude-fleet, cold): APPROVE 832f5c0 This is a delta review of 4fcd502..832f5c0: one commit in
|
|
MERGE HOLD @ 832f5c0: code review passed, owner decision missing Why this is a hold and not an APPROVE. mcp#82 item 2 changes the rounding rules and the persisted state's value semantics, so the register marked it as the owner's call, with "keep floats" as the default. mcp#82 has two comments, neither of them an owner decision:
@cristim, please confirm on mcp#82: integer cents, spend rounds up, the limit rounds down, and legacy non-cent amounts round up on load. The code is ready once you do. Attacks (a scratch test against the real code)
Old state file. I wrote a file with main's own code: spent 12.645, reservations 7.005 and 60. The PR head reads them as 12.65, 7.01 and 60, all rounded up with nothing lost. It rewrites the file in the same shape at version 1, and main's strict decoder reads the result back. Cent-valued files in main's exact shape, including Concurrency. 200 goroutines in memory, and 40 separate file-backed instances on one path, each reserved 0.07 against 1.00. Exactly 14 succeeded in each case, with no over-commit. The concurrency tests pass under Fail closed. Every case below refuses to spend and leaves the file unchanged: a load error happens before any save (
A missing file is a fresh budget, as documented. The overflow fix. The bound check at Other checks.
Findings
Not merging. |
|
APPROVE @ 832f5c0 by claude-cc-libs |
Closes #82
Item 2 of #82, the last open one. Items 1, 3 and 4 merged in #109 and #113.
What
The cumulative budget's two ledgers, in memory (
budgetLedger) and persisted (budgetStateinCUDLY_MCP_BUDGET_STATE), now add and compare whole cents (usdCents, int64) instead of float64 dollars.On main, float residuals flip exact-boundary checks:
reserved;"spent_usd": 0.30000000000000004.Rounding: never undercount spend
strconv.FormatFloat(v, 'g', -1, 64)thenbig.Rat), so 0.07 is 7 cents, although0.07*100is 7.000000000000001.State file compatibility
versionstays 1, amounts stay JSON numbers), so old and new processes can share a file during an upgrade.0.3,60).0.105reads as 0.11.nullnow makes the file corrupt (fail closed). Before, it silently read as 0. A quoted or negative amount was already corrupt and still is.README (budget paragraph and "Persisted budget" format bullet) and CHANGELOG updated.
Verification (local, macOS)
TestBudgetLedgerExactBoundaryTestBudgetLedgerReleaseLeavesNoResidualTestBudgetLedgerRoundsTowardRefusingTestFileBudgetExactCentsTestFileBudgetLegacyAmountRoundsUpTestFileBudgetNullAmountIsCorruptTestSpendAndLimitCents,TestUSDCentsJSONandTestBudgetLedgerRefusesUnrepresentableAmountscover the helpers.big.Rat.SetFloat64instead of the shortest decimalcommittedmake test-unit(3 packages) passes;make lintreports 0 issues; pre-commit is clean.🤖 Generated with Claude Code