Repository navigation
fix(auto-router): count usage savings by selected UTC request day - #44115
Conversation
|
@greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
Score: 4/5 This is a well-scoped fix with a sound accounting model: per-UTC-day/router rows are written atomically with the session rollup, late baseline corrections update those rows, and the endpoint’s headline total now reads the same daily spend source as Overall. The cross-midnight, user/key scoping, legacy/unattributed history, failed projection rollback, and late-replay tests provide strong coverage. The UI also clearly distinguishes selected-day money from whole-session metrics and removes the misleading average-savings tile. I’m stopping at 4 rather than 5 because the required CI/e2e status is still pending, so the migration and concurrent read/write behavior have not yet been validated by the full pipeline. Subject to those checks passing, this looks ready to merge. |
|
98f9e4c to
4619950
Compare
|
@greptileai can you reply here with a score and why that score? All four findings are addressed in 4619950 (router_type in the day key, router-group baselines null under an unattributed residual, day-row retention on the session horizon, empty api_key rejected) |
|
bugbot run |
|
Score: 5/5 The follow-up commit addresses all four findings:
The added regression tests cover these cases, and the latest e2e status is passing. I have no remaining blocking concerns. |
4619950 to
d8a2425
Compare
|
@greptileai can you reply here with a score and why that score? d8a2425 only changes the unattributed-savings note wording to be accurate for a negative residual (Bugbot finding) |
|
bugbot run |
|
Score: 5/5 This remains a complete fix. The latest commit only adjusts the dashboard note so the residual is described by its magnitude ( I have no remaining blocking concerns; the current e2e check is in progress rather than failing. |
d8a2425 to
6d4f699
Compare
|
@greptileai can you reply here with a score and why that score? 6d4f699 rebases onto main (the PR was conflicting, so pull_request CI never ran). main's #43398/#43408 deleted the private daily-activity where builder this PR had made public, so the Auto-Router headline now reads through main's public |
|
bugbot run |
|
Score: 4/5 The rebase addresses the integration concern: the Auto-Router headline now uses main’s public I’m keeping this at 4/5 only because the required post-rebase e2e/CI check is currently pending, so the migration and end-to-end behavior have not yet been validated by CI. I have no new code-level blocking concern from the stated rebase changes; subject to that check passing, this is ready to merge. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@greptileai review |
6d4f699 to
63aa4f4
Compare
| merged: Final[dict[tuple[object, object], Mapping[str, object]]] = {} | ||
| for row in (*session_rows, *day_rows): | ||
| key = (row["router_name"], row["router_type"]) | ||
| merged[key] = {**merged.get(key, {}), **row} |
There was a problem hiding this comment.
New code mutates containers This merge assigns into a dictionary, and the new test helper appends to a list. Both violate the repository's no-mutation directive, which must be satisfied before merging
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed in 33eea0b by deleting the merge rather than rewriting it: session shape and day money now come from one SQL statement (sessions FULL OUTER JOIN days USING (router_name, router_type) in AUTOROUTER_BENCHMARKS_SQL), so _window_rows and its dict assignment are gone and the endpoint makes one query fewer. The test fake records calls through AsyncMock.await_args_list instead of appending to a list
The auto-router usage view summed the lifetime savings of every session overlapping the date range, so it disagreed with the Overall savings view, which sums daily rollups by request day. Record auto-routed money per UTC request day and router in one new table, written in the same statement as the session rollup and corrected in the same transaction as late baseline estimates. The all-router headline reads the same daily rows and filters as Overall; savings no router day row accounts for are reported as unattributed and void the baseline comparison. Session shape and caching stay whole-session and are labelled so; the savings-per-session tile is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
63aa4f4 to
33eea0b
Compare
|
@greptileai can you reply here with a score and why that score? 33eea0b addresses both P2s: the benchmark read is now one SQL statement (sessions FULL OUTER JOIN days), removing the mutating merge, and router groups with requests but no session rows report session averages as null instead of 0 |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 33eea0b. Configure here.
|
@greptileai can you reply here with a score and why that score? Re-requesting on 33eea0b: the benchmark read is one SQL statement now (sessions FULL OUTER JOIN days, removing the mutating merge), and groups with requests but no session rows report session averages as null. Full CI is green apart from Analyze (python), whose LogInjection.ql 2GiB failure also occurs on main at 71788fe and e5873ad |
|
Score: 5/5 The latest revision resolves the remaining concerns. The benchmark read is now a single SQL query combining session metrics with daily spend via The previously reviewed safeguards remain intact: UTC-day/router attribution, unattributed legacy residual handling, retention alignment, empty-filter validation, and regression coverage. Full CI is green aside from the Python Analyze |
TLDR
Problem this solves:
How it solves it:
Intentional product change: the "Avg saved per session" tile is removed, because it divided selected-day money by whole-session counts. Session averages and caching stay whole-session and are now labelled that way
User Flow
Before: an admin picking a date range sees more Auto-Router savings than Overall reports
After: both tabs report the savings earned on the selected days
Linear ticket
Resolves LIT-9171
Follow-up: LIT-9172 moves the per-router daily write into the daily spend writer, so per-router totals also add up with
disable_spend_logson and for sessionless requestsPre-Submission checklist
Screenshots / Proof of Fix
Rig: local proxy with Postgres 16 and a complexity auto-router whose tiers are real Anthropic models reached through the sandbox gateway. The September rows are seeded fixture records (one session earning 7, 3 and 11 on Aug 31, Sep 1 and Sep 2), not provider traffic. Each step compares
GET /user/daily/activity/aggregated(Overall) withGET /auto_router/benchmarks(Auto-Router) for the same datesBefore (d7e6154, merge base before rebase)
Selected-day windows
Dashboard
After (33eea0b)
Selected-day windows
unattributed_saved_spendset, baseline and percentage null on totals and on each router groupGET /auto_router/benchmarks?api_key=returns 422Real traffic
POST /v1/chat/completionstosmart-routerin one session: 200s routed to haiku, sonnet, haikuGET /auto_router/sessionstill reports the whole session: 3 turns, saved 0.031958Dashboard
Type
🐛 Bug Fix
🤖 Generated with Claude Code
Note
Medium Risk
Changes spend accounting SQL, a new migration, and a breaking API shape for benchmarks; correctness depends on daily upserts staying in sync with session rollups and baseline corrections.
Overview
Fixes Auto-Router usage reporting so spend and savings match the selected UTC date range (and the Overall tab), instead of summing whole overlapping sessions.
Adds
LiteLLM_AutoRouterDailySpend, written in the same upsert as session rollups and corrected when late baselines publish.AUTOROUTER_BENCHMARKS_SQLnow joins session rows (shape, cache,session_turns) with daily rows (money filtered by$5–$6). The benchmarks API sets the headlinesaved_spendfromLiteLLM_DailyUserSpend.autorouter_savings_spend, exposesunattributed_saved_spendwhen that total disagrees with per-router daily sums, and dropssaved_per_session. Session averages can be null when routed requests have no matching session row; cache coverage usessession_turns.Retention cleanup deletes expired daily rollup rows with session retention. The cost-optimization UI removes the avg-saved-per-session tile and clarifies selected-day vs whole-session metrics.
Reviewed by Cursor Bugbot for commit 33eea0b. Bugbot is set up for automated code reviews on this repo. Configure here.