Repository navigation
fix(ui): show user attribution in Top Virtual Keys usage tables - #40729
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
|
|
@greptileai please re-review after the follow-up commit that addresses both earlier threads, CI is green on the new tip |
|
bugbot run |
ryan-crabbe-berri
left a comment
There was a problem hiding this comment.
UsagePageView's new user_email line is fully mocked out in tests, and the User column disappears for owners that have a user_id but no email
|
@ryan-crabbe-berri addressed in 142766d: aggregation moved to getGlobalTopKeys with direct unit tests, and the User column now falls back to user_id |
|
@greptile re review |
|
bugbot run |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…database /user/daily/activity now reports key_exists on each api key's metadata, true only when the key is in the active key table that /key/info reads. Top Virtual Keys renders the Key ID as plain text with an explanatory tooltip and ignores chart bar clicks when key_exists is false, so deleted keys and CLI/SSO session keys no longer dead-end on a "Key not found in database" toast
9379463 to
89bf870
Compare
The key_exists chart test added a second direct DOM lookup for the Recharts bars, which exposes no role or label, and pushed testing-library/no-node-access over its budget (709 > 707). Both chart tests now go through one helper
|
@greptile re review |
|
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 82fd632. Configure here.
Both requested changes landed: the aggregation has direct unit tests and the User column falls back to the user id when there is no email
TLDR
Problem this solves:
How it solves it:
user_email, falling back touser_id) from the daily activity breakdown into the top keys rowsTopKeyItem) so both producers must supply itgetGlobalTopKeysso it is unit tested directlykey_existsper key, true only for keys/key/infocan openUser Flow
Before: an admin whose users log in through the CLI cannot tell who is spending on the Usage page
After: the same table shows the owning user on every row, and only offers a key lookup that can succeed
Relevant issues
Follow-up to #40275 (email label fallback). Reported by a customer via Pylon #8500
Linear ticket
Resolves LIT-7572
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Focused Vitest runs pass for
TopKeyView.test.tsx,EntityUsage.test.tsxandtests/top_key_view.test.tsx(the new cases covergetGlobalTopKeyssumming spend across days and carrying the owner, theuser_idfallback in both aggregators, a null alias with a user email, the User header appearing only when a row has a user, and keys withkey_exists: falsebeing skipped by both the table click and the chart bar click).test_common_daily_activity.pycovers the backend flag for an active, a deleted and a never-stored keyLive run against a local proxy on this branch, with real
gpt-5.5spend from four keys: two alias-less keys owned by users with an email, one alias-less key owned by a user with no email, and one aliased key. These are regular user-owned keys without an alias, which hit the same render path as CLI session keys (no alias, owner in the breakdown metadata), since a real CLI SSO login needs an IdPBefore (base), 1440px wide
After (this PR), 1440px wide. The User column shows the email, or the user id for the owner with no email. Note that at this width the table is wider than its card (556px in 494px), so Spend (USD) is only reachable by scrolling the table sideways
After (this PR), 1920px wide, where all four columns fit
Key lookup. On the same proxy I deleted one of the seeded keys with
POST /key/deleteand sent one moregpt-5.5request with the master key, which is never stored as a key row. The activity endpoint now marks both as not existing:Before (previous commit on this branch): every Key ID is a clickable pill, and clicking the deleted key fires
GET /key/infoand toasts "Failed to fetch key info - Key not found in database"After: the deleted key and the master key row are plain text with an explanatory tooltip. Clicking the deleted key sent zero
/key/inforequests and raised no toast, while the keys that still exist stay clickableTo reproduce:
POST /user/newwithuser_idanduser_email, thenPOST /key/generatewith thatuser_idPOST /key/delete, and wait about a minute for the daily spend flush?page=usageis the Old Usage page)Type
Bug Fix
Caveats (if any)
Medium
Low
key_existsis a new optional response field, so existing clients are unaffectedkey_existsvalue stay clickable, matching the old behaviorLink to Devin session: https://app.devin.ai/sessions/d7dab4487de544488c1871e08c8aa0d7
Open in Devin Desktop: https://app.devin.ai/desktop/session/d7dab4487de544488c1871e08c8aa0d7?variant=devin
Note
Low Risk
Adds an optional API field and UI-only gating on key detail lookup; spend aggregation logic is unchanged aside from metadata enrichment.
Overview
Adds
key_existsto daily-activityKeyMetadataso the API marks keys as present only when they still exist in the active verification-token table (deleted, session, and master keys default to false while historical alias/owner metadata can remain).On the Usage dashboard, Top Virtual Keys now shows a conditional User column (
user_email, thenuser_id), centralizes global top-key aggregation ingetGlobalTopKeys, and types rows asTopKeyItem.TopKeyViewdisables/key/infofor keys withkey_exists === false(plain Key ID + tooltip; chart clicks ignored) while keeping existing keys clickable.Reviewed by Cursor Bugbot for commit 82fd632. Bugbot is set up for automated code reviews on this repo. Configure here.