fix(report): stop summaries rendering an empty cost as "-0.00" - #1056
Merged
Conversation
`Sum for f64` folds from `-0.0` — the additive identity that preserves the
sign of every addend — so summing an empty or all-zero set yields `-0.0`,
and the CLI's `format!("${:.2}", ..)` renders that as "$-0.00". Users hit
it whenever a submission excluded every row: "Total cost: $-0.00" next to
"Total tokens: 0".
The three report aggregators in lib.rs already normalize with a trailing
`+ 0.0`; calculate_summary was the one site that missed it.
test_calculate_summary_empty asserted `total_cost == 0.0`, which is true
for `-0.0` under IEEE, so it could never catch this. It now also asserts
the sign bit, and a companion test pins the rendered string.
Confidence: high
Scope-risk: narrow
Directive: `+ 0.0` on an f64 sum is deliberate sign normalization, not
redundant arithmetic — do not "simplify" it away
Not-tested: whether any frontend consumer of the JSON summary distinguishes
-0.0 from 0.0
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
…vers The comment said an "empty (or all-zero)" sum yields -0.0. Only the empty fold reaches the identity untouched; a single +0.0 addend already normalized it, since -0.0 + 0.0 == +0.0. As written it implied every zero-cost day was affected. An all--0.0 set does still fold to -0.0, so that case is named. Adds a test pinning the boundary, so the distinction is checkable rather than asserted in prose. Confidence: high Scope-risk: narrow
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
calculate_summaryreturns-0.0for an empty or all-zero contribution set, and the CLI renders it as$-0.00.Reproduced end-to-end on
main(healthy pricing, every row excluded from the submission):Cause
Rust's
impl Sum for f64folds from-0.0, not0.0. That is deliberate upstream:-0.0is the additive identity that preserves the sign of every addend (-0.0 + x == xfor allx, whereas0.0 + -0.0 == +0.0would discard it). So[].iter().sum::<f64>()is-0.0— bits0x8000000000000000— and{:.2}prints-0.00.The three report aggregators in
lib.rsalready normalize this with a trailing+ 0.0.calculate_summaryinaggregator.rswas the one site that missed the idiom.Why no test caught it
test_calculate_summary_emptyasserted:-0.0 == 0.0is true under IEEE 754, so that assertion passes on the buggy value. It now additionally asserts the sign bit, and a companion test pins the rendered string.Tests
cargo test -p tokscale-core --lib— 1,489 passed, 0 failedcargo fmt --all --checkclean+ 0.0again makestest_calculate_summary_emptyfail; with it, all 5calculate_summarytests passNote
Found while auditing the batch merged in #1037, #1045, #1051 and #1055 — it is pre-existing and independent of all four.
Summary by cubic
Fixes summary cost rendering so empty summaries show "$0.00", not "$-0.00". Normalizes the
f64sum and tightens tests and comments to prevent regressions.calculate_summarytotal cost withsum::<f64>() + 0.0; clarify only empty sets (or all-0.0) would format as negative zero.tokscale-corefor non-negative zero and "$0.00" on empty summaries, plus a boundary test proving a single+0.0contribution already yields+0.0.Written for commit f09abf8. Summary will update on new commits.