fix(trpc): raise leaderboard publish row caps so heavy days can publish - #6908
fix(trpc): raise leaderboard publish row caps so heavy days can publish#6908hairez wants to merge 1 commit into
Conversation
The caps in publishSchema sit on the procedure input, so zod fails the whole days array when one element is out of range and the mutation never reaches the handler. Every other day in that payload is dropped with it. cachedInput counts cache reads, which scale with session concurrency, so one model on one day of heavily parallel agent work clears both the 50B token cap and the $10,000 estimate together.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe leaderboard schema increases the maximum tokens per row to 1 trillion and the maximum USD value per row to 1 million. The associated comments now describe the applicable numeric constraints. ChangesLeaderboard validation bounds
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: 🔵 Low · up to This change allows heavy leaderboard days to publish, but accumulated values can exceed the exact integer range used for leaderboard totals and tier calculations, potentially causing inaccurate displayed results. The PR is mergeable with explicit owner awareness and follow-up to make aggregate handling bigint-safe. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. ✨ Finishing Touches🧪 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 |
|
hey, direction's right, and the two things you scoped out are the real damage - let's land this. One ask before merging: everything so far shows the schema rejects those rows, not that the numbers are correct, and raising the cap publishes them to a public board. Two checks: // bun run check.ts
import { computeLeaderboardPayload } from "./packages/host-service/src/trpc/router/usage/history/leaderboard-days";
const { days } = await computeLeaderboardPayload(30);
for (const d of [...days].sort((a, b) => b.cachedInput - a.cachedInput).slice(0, 10)) {
console.log(d.day, d.provider, d.model, {
cachedInput: d.cachedInput,
sessions: d.sessions,
approximate: d.approximate,
readsPerOutput: Math.round(d.cachedInput / Math.max(1, d.output)),
});
}
Small correction: 1T x 9,007 treats a row as one field, but tokens is the 5-field sum (leaderboard.ts:529-534) and that's what recomputeTotals rolls up. So ~1,800 rows, and the old cap allowed ~36,000. Doesn't change your point, but 1,800 is one publish away since model is free-form. And yes - please move the guarantee into the rollup read path here. Once Number() is off the bigint sums (queries.ts:93, :230) the cap only has to be plausible, not load-bearing. |
2 participants
What & why
A single oversized row rejects an entire leaderboard publish, so heavy users stop publishing without noticing.
The caps in
publishSchemasit on the procedure input, which means zod fails the wholedaysarray when one element is out of range and the mutation never reaches the handler. Every other day in that payload is dropped with it:They are also below real usage.
cachedInputcounts cache reads, which scale with session concurrency, so one model on one day of heavily parallel agent work clears the token cap and the $10,000 estimate together.usdEstimateis notional list price, not billed spend.This raises the two caps so those days validate. 1T per field per row leaves about an order of magnitude over the largest row I have observed in practice, and $1M stays two orders of magnitude under the
numeric(14,6)column the per-row value lands in.Worth flagging for review, since I do not think the cap can carry the guarantee its comment claims: the rollup on
leaderboard_participantsis read back throughbigint({ mode: "number" }), so the real ceiling is 2^53, and rows accumulate without bound across days, models and hosts. 1T x 9,007 rows reaches 2^53; the old 50B allowed 180,143. Neither is an invariant. If you want one it belongs in the rollup read path rather than in an input cap, and I am happy to do that instead if you would rather go that way.Two related problems are out of scope here, but they are what turns this from an error into silent data loss, so they are worth their own fix:
publishWindowDaysshrinks the window back to 2 days nothing revisits them, and leaving and rejoining re-backfills only 30 days. Raising the caps fixes new publishes, not history.Reported in Discord: https://discord.com/channels/1446776342577283114/1446776344204677132/1542510608828862515
How I tested it
packages/trpc/src/router/leaderboard/schema.tsunmodified frommainand from this branch. Onmain, 7 of 80 rows fail andpublishSchema.safeParserejects the whole payload with the sameToo bigmessages as the console error above. On this branch all 80 rows parse and the payload is accepted.bun run lintpasses repo-wide: 6,615 files checked, no diagnostics.bun run typecheckpasses: 37 of 37 turbo tasks successful. Run locally with bun 1.4.0 rather than the pinned 1.3.14, so treat it as a strong signal rather than a CI-equivalent one.bun run test. The change is two numeric literals and a comment, and neither constant is referenced anywhere else in the repo.Checklist
type(scope): subject)bun run lintandbun run typecheckpass (CI fails on lint warnings too)