fix: populate team member emails missing from the roster snapshot - #37759
Conversation
Greptile SummaryThe PR hydrates missing team-roster emails from user records while preserving stored snapshot values. It also resolves member identities without mutating submitted models and consolidates roster deduplication Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains No blocking failure remains
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/team_endpoints.py | Adds read-time email hydration and immutable bidirectional identity resolution for team roster entries |
| tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py | Adds focused coverage for hydration, stored-email preservation, query skipping, and single or bulk identity resolution |
Reviews (2): Last reviewed commit: "fix: populate team member emails missing..." | Re-trigger Greptile
|
|
||
| user_rows: Final[Sequence[LiteLLM_UserTable]] = await _user_db(prisma_client).find_many( | ||
| where={"user_id": {"in": sorted(missing_user_ids)}} |
There was a problem hiding this comment.
Direct request-path database query
For rosters containing a member with a user ID but no stored email, _hydrate_member_emails calls find_many directly from /team/info, bypassing the repository's required user-lookup helpers and adding an independently managed database round trip to the request path.
Rule Used: What: In critical path of request, there should be... (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!
| member.user_id = user.user_id | ||
| break |
There was a problem hiding this comment.
Identity resolution mutates inputs
_resolve_member_identity assigns resolved fields directly onto the supplied Member; in the bulk branch these are the request's original objects, so later consumers observe values not present in the submitted model and identity resolution becomes coupled to mutable shared state.
Context Used: CLAUDE.md (source)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
d769564 to
28b9175
Compare
`members_with_roles` is a denormalized JSON snapshot written at add-time. `_update_team_members_list` backfilled `user_id` from `user_email` but never the reverse, so a member added by `user_id` alone was stored with `user_email=None` permanently - and `/team/info` returns that blob verbatim with no join to `LiteLLM_UserTable`, so the Admin UI's member table renders "-" for a user that plainly has an email. Fix both ends: - write path: `_resolve_member_identity` resolves identity both ways off the user rows the add just touched, so new roster entries stop being born blank. - read path: `/team/info` fills blank emails from `LiteLLM_UserTable` in one indexed `user_id IN (...)` query, repairing rows already in the database. Members that already carry an email are passed through untouched and cost no query, so this only ever turns a null into the right value.
28b9175 to
16cd080
Compare
|
@greptileai re review |
tin-berri
left a comment
There was a problem hiding this comment.
Clean fix for a real display bug — members_with_roles is an add-time snapshot, so a member added by user_id alone carries user_email=None forever even after the user row gets one. Two solid pieces: _resolve_member_identity makes write-time identity resolution bidirectional (previously only user_id was backfilled from email; now email is backfilled from user_id too), and _hydrate_member_emails fills blanks at read time in /team/info via one batched find_many query, never touching a stored value. No auth/security surface — this is read-only enrichment of display data, no new write paths. Test coverage is thorough: fill-blanks-only, never-overwrite, no-op when the user row also has no email, skip-the-query-when-nothing's-missing, plus both the single-member and bulk-add write paths and an end-to-end /team/info test. CI green. Approved.
Title
fix: populate team member emails missing from the roster snapshot
Relevant issues
The Admin UI's team member table renders
-under User Email for a member whose user row plainly has an email.Root cause
members_with_rolesis a denormalized JSON snapshot onLiteLLM_TeamTable, written once when the member is added./team/inforeturns that blob verbatim — there is no join toLiteLLM_UserTableanywhere in the read path — so once an entry is stored withuser_email: null, nothing ever repairs it.Entries land with a null email legitimately: a member added by
user_idwhose user row had no email yet (_validate_and_populate_member_user_infocorrectly has nothing to copy), or a roster row written before that populate step existed. The moment that user gets an email, the roster is stale and stays stale forever.Reproduced live against the dev proxy:
Before / after
Same team, same database rows, same UI — only the proxy code differs.
Before —
ccd211ed-…shows-, even though that user's row holdsryan@berri.ai:After — the email resolves, with no change to the stored roster row:
Default Proxy Adminstill shows-in both — that user row genuinely has no email. The fix resolves what exists; it does not invent values.And the same thing at the API layer:
The fix
/team/infofills blank emails fromLiteLLM_UserTablebefore responding, so the rows already sitting in the database display correctly with no migration._update_team_members_listbackfilleduser_idfromuser_emailbut never the reverse._resolve_member_identitynow resolves both directions off the user rows the add just touched, so the helper is no longer one-way for any caller that reaches it without going through_validate_and_populate_member_user_info. The add/dedupe logic was also duplicated across the single-member and bulk branches; both now share_member_already_in_team.Is this a behavior change to
/team/info?Only in the sense that a
nullbecomes the correct value. Explicitly not a contract change:nullrather than inventing one.Cost
One extra
WHERE user_id IN (...)on the primary-key index, and only for the members actually missing an email — a roster that is already complete pays for no query at all./team/infoalready issues unbounded queries for all team keys and all team memberships, so this is noise next to what the endpoint does today.Type of change
Testing
Live proxy verification is the before/after above. Unit tests added in
tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py:test_hydrate_member_emails_fills_in_emails_the_roster_snapshot_never_capturedtest_hydrate_member_emails_never_overwrites_a_stored_emailtest_hydrate_member_emails_leaves_members_alone_when_the_user_row_has_no_emailtest_hydrate_member_emails_skips_the_query_when_every_member_has_onetest_team_info_hydrates_member_emails_from_the_user_table(endpoint-level; also asserts only the blank member is looked up)test_update_team_members_list_stamps_email_for_a_member_added_by_user_idtest_update_team_members_list_stamps_email_for_each_member_in_a_bulk_addbasedpyrighterror counts onteam_endpoints.pyare identical before and after, andruff check/ruff format --checkare clean on the changed source file.