fix(admin): sum global stats costs in numeric - #3547
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe change updates admin and worker cost aggregation to use double-precision intermediates. Count aggregates now use ChangesCost aggregation precision
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Cost columns on the stats tables are float4, and Postgres accumulates SUM(real) in float4 as well — roughly 7 significant digits. At the all-time scale the global stats page shows, the running sum's ulp is larger than a cent, so the result depends on how the rows happened to be grouped: the headline total and the per-mode / per-kind slices of the same rows drifted apart by double-digit dollars. Sum through numeric (via double precision, because real::numeric goes through float4's 6-digit display form and loses even more) so every grouping of a row set yields the identical total. Also stop narrowing the all-time request/error/cache counts back to int4. The worker had the same problem on the write side: an hour's worth of logs was summed in float4 before being stored, so the buckets were already wrong. Cast to double precision there too — this also fixes data storage cost, which was being rounded from numeric to float4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0f2c22c to
29a6343
Compare
Problem
On the admin Global Stats page the cost card does not add up: the headline total and the per-mode / per-kind slices printed underneath it are the same rows grouped two ways, yet they disagree by double-digit dollars at all-time scale. Narrow the view to a small enough slice and it reconciles.
Request counts reconcile everywhere; only money drifts, and only at scale. That is the tell.
Every cost column on the stats tables is
real(float4), and Postgres accumulatesSUM(real)in float4 too —sum(real) → real, about 7 significant digits. Once a running sum is large enough its ulp exceeds a cent, so the result depends on the order and grouping of the rows. The headline (oneSUMover everything) and the composition slices (SUMper mode / per kind) therefore diverge. Small slices reconcile because they stay well inside float4's range.Reproduced standalone:
Three different answers for the same 800 rows.
Fix
Read path (
GET /admin/global-stats) — accumulate cost sums inNUMERIC, which is exact, so any grouping of a row set yields a bit-identical total.The cast chain matters and is not decorative:
real::numericroutes through float4's 6-digit display form (1234.5678::real::numeric→1234.57), losing more than the float4 sum did.real::double precisionis the exact stored value, so the cast isreal → float8 → numeric, then back tofloat8for JSON.Also stopped narrowing the all-time request/error/cache counts back to
int4— this endpoint is cross-tenant and all-time, so the count will outgrow 2^31 eventually and fail hard.Write path (
apps/workerhourly aggregation) — same bug a layer down: an hour is tens of thousands of log rows, summed in float4 before being stored, so the buckets were already wrong before anything read them. Now cast todouble precisionfirst; the target column is still float4, so only the final value is rounded rather than every partial sum along the way. This coversglobal_*_stats,project_hourly_*,api_key_hourly_*andprovider_key_hourly_stats.While there:
data_storage_costwas being summed ascast(... as real)even thoughlog.data_storage_costisnumeric— it was downcast for no reason.Not in this PR
realcolumns. Per-row rounding is ~5e-8 of the row's own value; the error that was visible came overwhelmingly from the accumulation, which is what this fixes.SUM(<real column>)sites acrossanalytics.ts, the model/mapping history rollups andstats-calculator.tshave the same latent bug, but they are per-project or per-model and orders of magnitude smaller, so the drift is currently invisible. Left for a follow-up rather than bundled into a bug fix.org_kindwas never backfilled when the column was added, andused_modeonly partially. That is addressed in feat(scripts): backfill global stats attribution #3549.Verification
New spec
large all-time sums stay exact and grouping-independentbuilds a synthetic production-magnitude fixture (700 days × 4 mode/kind buckets) and asserts the headline matches the mode composition, the kind composition, and the timeseries.Against the unfixed code it reproduces the reported symptom at scale:
apps/api— 930 tests pass (70 files)apps/worker— 114 tests pass (9 files)pnpm format,pnpm buildcleanNo screenshots: this is a numeric correctness fix behind an existing card, nothing about the layout changes.
🤖 Generated with Claude Code
Summary by CodeRabbit