fix: place 1e18 in the top regular histogram bucket

[vsaraikin]

Fixes #130.

Bug

Histogram.Update misclassifies exactly 1e18 into the +Inf overflow bucket instead of the top regular bucket.

h := &metrics.Histogram{}
h.Update(1e18)
h.VisitNonZeroBuckets(func(vmrange string, _ uint64) { fmt.Println(vmrange) })
// prints: 1.000e+18...+Inf     (wrong)
// want:   8.799e+17...1.000e+18

Root cause

bucketIdx = (log10(v) - e10Min) * bucketsPerDecimal. With e10Min = -9, bucketsPerDecimal = 18 and bucketsCount = 486, the value 1e18 gives bucketIdx = (18 - (-9)) * 18 = 486, exactly equal to bucketsCount. The overflow guard used >=:

} else if bucketIdx >= bucketsCount {
    h.upper++

so 1e18 was routed to +Inf before reaching the regular-bucket path — which already contains the 10^n edge case (idx--) that places a power of ten in the bucket whose inclusive upper bound is that power of ten. Every interior 10^n (e.g. 100 → 8.799e+01...1.000e+02) is placed correctly; only the maximum, 1e18, was intercepted by the >= guard.

Fix

Use > so only values strictly greater than 1e18 overflow:

} else if bucketIdx > bucketsCount {
    h.upper++

1e18 (the only value with bucketIdx == 486) now falls through to the regular path, where the existing 10^n edge case decrements idx to 485 — the 8.799e+17...1.000e+18 bucket. math.Inf(1) and any value above 1e18 still produce bucketIdx > 486 and remain in +Inf.

Testing

Added TestHistogramMaxPowerOfTenBoundary asserting 1e18 and 9.999e17 land in 8.799e+17...1.000e+18, while 1.0001e18 and math.Inf(1) remain in 1.000e+18...+Inf. It fails on the current code and passes with the fix. The full package test suite (including -race) passes unchanged.

[cubic-dev-ai]

All reported issues were addressed across 2 files

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[vsaraikin]

Good catch, thanks @cubic-dev-ai — that's a real regression. For values slightly above 1e18 (e.g. math.Nextafter(1e18, +Inf), which is 128 greater), math.Log10 still returns exactly 18, so bucketIdx == 486 and the relaxed > guard let them fall into the last regular bucket.

Fixed by routing on the value directly: the overflow guard is now bucketIdx > bucketsCount || v > maxRegularBucketValue (where maxRegularBucketValue = math.Pow10(e10Max) == 1e18), so only values > 1e18 overflow while exactly 1e18 still lands in 8.799e+17...1.000e+18. Added a regression case for Nextafter(1e18, +Inf) → +Inf; full suite (incl. -race) is green.

[cubic-dev-ai]

Good catch, thanks @cubic-dev-ai — that's a real regression. For values slightly above 1e18 (e.g. math.Nextafter(1e18, +Inf), which is 128 greater), math.Log10 still returns exactly 18, so bucketIdx == 486 and the relaxed > guard let them fall into the last regular bucket.

Fixed by routing on the value directly: the overflow guard is now bucketIdx > bucketsCount || v > maxRegularBucketValue (where maxRegularBucketValue = math.Pow10(e10Max) == 1e18), so only values > 1e18 overflow while exactly 1e18 still lands in 8.799e+17...1.000e+18. Added a regression case for Nextafter(1e18, +Inf) → +Inf; full suite (incl. -race) is green.

@vsaraikin Yep, you're right, and the fix looks correct.

The original suggestion (bucketIdx > bucketsCount) was too naive — it assumed log10 rounding would push values above 1e18 past that threshold naturally, but IEEE 754 double precision means math.Log10 still rounds to exactly 18 for values up to ~1e18 + 128. So the guard would have just shifted the off-by-one bug from 1e18 itself to the values immediately above it.

Your value-level check (v > maxRegularBucketValue) is the right approach — it bypasses the floating-point ambiguity entirely. The same pattern applies again for the Nextafter(1e18, +Inf) test case you added, which nicely closes the loop.

The var maxRegularBucketValue = math.Pow10(e10Max) computed at init is also clean — avoids recalculating 1e18 on every call. Looks good to me.

_{Tip: get faster answers by chatting with cubic’s review copilot in the review UI}