Skip to content

Litellm ryan march 16#23822

Merged
ryan-crabbe merged 13 commits intomainfrom
litellm_ryan_march_16
Mar 17, 2026
Merged

Litellm ryan march 16#23822
ryan-crabbe merged 13 commits intomainfrom
litellm_ryan_march_16

Conversation

@ryan-crabbe
Copy link
Copy Markdown
Contributor

@ryan-crabbe ryan-crabbe commented Mar 17, 2026

daily branch

All tests passing: https://app.circleci.com/pipelines/github/BerriAI/litellm?branch=litellm_ryan_march_16

#23819
#23791
#23787
#23666

  • fix(ui): CSV export empty on Global Usage page. Added resolveEntities() to reconstruct entities by grouping api_keys on team_id
  • fix: default user role Pydantic default out of sync with runtime fallback, Changed DefaultInternalUserParams.user_role default from INTERNAL_USER to INTERNAL_USER_VIEW_ONLY to match the runtime SSO/SCIM/JWT provisioning fallback.
  • feat: fetch blog posts from docs RSS feed instead of
    static JSON
  • chore(ui): migrate DefaultUserSettings buttons from Tremor to antd, replaced Tremor Button with antd Button, using danger prop and proper icon JSX syntax.

ryan-crabbe and others added 13 commits March 14, 2026 15:28
…llback

The Pydantic default for user_role was INTERNAL_USER, but all runtime
provisioning paths (SSO, SCIM, JWT) fall back to INTERNAL_USER_VIEW_ONLY
when no settings are saved. This caused the UI to show "Internal User"
on fresh instances while new users actually got "Internal Viewer".
Asserts that GET /get/internal_user_settings returns
INTERNAL_USER_VIEW_ONLY on a fresh DB with no saved settings,
matching the runtime fallback in SSO/SCIM/JWT provisioning.
…endpoints.py

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…-not-synced-with-ui

fix: align DefaultInternalUserParams Pydantic default with runtime fallback
…ettings-antd

chore(ui): migrate DefaultUserSettings buttons from Tremor to antd
Aggregated endpoint returns empty breakdown.entities; fall back to
grouping breakdown.api_keys by team_id.
fix(ui): CSV export empty on Global Usage page
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Mar 17, 2026 4:17pm

Request Review

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq bot commented Mar 17, 2026

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing litellm_ryan_march_16 (302292c) with main (245a3d2)

Open in CodSpeed

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 17, 2026

Greptile Summary

This PR bundles four independent improvements: fixes empty CSV exports on the Global Usage page by adding a resolveEntities() fallback that reconstructs entity data from api_keys when breakdown.entities is empty; aligns the Pydantic default for DefaultInternalUserParams.user_role with the SSO/SCIM/JWT runtime fallback (INTERNAL_USER_VIEW_ONLY); switches blog post fetching from a static GitHub JSON file to parsing the live docs RSS feed; and migrates DefaultUserSettings buttons from Tremor to antd.

Key findings:

  • The resolveEntities() aggregation logic and the new unit tests are well-structured; the CSV fix correctly handles the aggregated endpoint's empty-entities response.
  • generateDailyWithModelsData has a pre-existing attribution bug (the modelData variable from day.breakdown.models is captured but never used; all API-key totals are written into every model bucket) that is now newly reachable via the resolveEntities fallback, meaning "Daily with Models" CSV exports for the aggregated endpoint will show inflated and identical numbers across all models.
  • The resolveEntities() function is called twice per day inside generateDailyWithModelsData (see prior review thread for the caching fix suggestion).
  • The inline comment on the resolveEntities aggregation test incorrectly describes the team-to-key mapping (key1+key2 / key3 instead of key1+key1b / key2).
  • All new tests are mock-only — no real network calls are introduced.
  • The DefaultInternalUserParams.user_role default change is backwards-incompatible for non-SSO deployments that relied on the Pydantic default (see prior review thread for the feature-flag approach).
  • The RSS parser uses the standard library xml.etree.ElementTree, which is vulnerable to XML entity expansion attacks when the URL is user-configurable (see prior review thread for the defusedxml recommendation).

Confidence Score: 3/5

  • Mergeable after addressing the model-attribution bug newly exposed by the resolveEntities fallback path and the open backwards-compatibility concern on the default user role.
  • The CSV fix and RSS migration are functionally correct and well-tested. However, the resolveEntities fallback now routes aggregated-endpoint data through generateDailyWithModelsData's pre-existing attribution bug, making "Daily with Models" exports produce over-counted, model-undifferentiated numbers for that endpoint. Combined with the unguarded backwards-incompatible user_role default change (noted in prior thread) and the XML entity expansion risk (noted in prior thread), the PR carries enough unresolved risk to hold at 3.
  • ui/litellm-dashboard/src/components/EntityUsageExport/utils.ts — generateDailyWithModelsData model attribution logic needs review before the resolveEntities fallback goes live.

Important Files Changed

Filename Overview
ui/litellm-dashboard/src/components/EntityUsageExport/utils.ts Adds resolveEntities() and aggregateApiKeysIntoEntities() to fix empty CSV exports when the aggregated endpoint returns empty entities. The fallback logic itself is correct, but generateDailyWithModelsData has a pre-existing attribution bug (all API key metrics credited to every model) that is now newly reachable via the aggregated endpoint path; also resolveEntities is invoked twice per day in that function (flagged in prior review thread).
ui/litellm-dashboard/src/components/EntityUsageExport/utils.test.ts Adds comprehensive tests for resolveEntities and the aggregated-endpoint fallback across all four export functions. Test logic is sound, but one inline comment mis-labels the key-to-team mapping (key1+key2/key3 instead of key1+key1b/key2).
litellm/litellm_core_utils/get_blog_posts.py Replaces static JSON fetching with RSS XML parsing. Parser logic is clean and the fallback chain is preserved. XML security concern (entity expansion via configurable URL) was flagged in a prior thread.
tests/test_litellm/test_get_blog_posts.py Tests updated to reflect the RSS feed approach. All network calls are properly mocked; no real network calls introduced. New parse_rss_to_posts tests cover happy path, multi-item, missing channel, and invalid XML cases.
litellm/proxy/_types.py Default user_role for DefaultInternalUserParams changed from INTERNAL_USER to INTERNAL_USER_VIEW_ONLY to align Pydantic default with runtime SSO/SCIM/JWT fallback. Backwards-compatibility concern flagged in prior thread.
tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py Adds a mock-only test asserting that a fresh DB returns INTERNAL_USER_VIEW_ONLY as the default role. No real network calls; test accurately validates the Pydantic default change.
ui/litellm-dashboard/src/components/DefaultUserSettings.tsx Straightforward Tremor→antd Button migration. size="sm"→"small", icon prop updated to JSX element syntax, danger prop added for the delete button, type="primary" added for Save/Edit buttons. No logic changes.
litellm/init.py Single-line change: default blog_posts_url updated from GitHub raw JSON to the docs RSS feed URL.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CSV Export triggered] --> B{breakdown.entities populated?}
    B -- Yes --> C[Use entities directly]
    B -- No --> D[aggregateApiKeysIntoEntities]
    D --> E{breakdown.api_keys present?}
    E -- No --> F[Return empty object]
    E -- Yes --> G[Group api_keys by metadata.team_id]
    G --> H[Accumulate METRIC_KEYS per team]
    H --> I[Build api_key_breakdown per team]
    I --> J[Return reconstructed entities map]
    C --> K[generateDailyData / generateDailyWithKeysData / generateDailyWithModelsData]
    J --> K
    K --> L[CSV / JSON output]

    subgraph Blog Posts
        M[get_blog_posts called] --> N{LITELLM_LOCAL_BLOG_POSTS=true?}
        N -- Yes --> O[Load local blog_posts.json]
        N -- No --> P{Cache valid?}
        P -- Yes --> Q[Return cached posts]
        P -- No --> R[httpx.GET RSS URL]
        R --> S{Network error?}
        S -- Yes --> O
        S -- No --> T[ET.fromstring XML parse]
        T --> U{Parse error?}
        U -- Yes --> O
        U -- No --> V[Extract up to max_posts items]
        V --> W{validate_blog_posts}
        W -- Empty list --> O
        W -- Valid --> X[Cache and return posts]
    end
Loading

Comments Outside Diff (1)

  1. ui/litellm-dashboard/src/components/EntityUsageExport/utils.ts, line 248-272 (link)

    P1 modelData captured but never used — all API key metrics attributed to every model

    In generateDailyWithModelsData, the outer loop iterates over every model in day.breakdown.models, but modelData (the per-model metrics) is destructured and then never referenced inside the loop body. Instead, the inner loop sums the API key's total metrics into every model bucket:

    Object.entries(day.breakdown.models || {}).forEach(([model, modelData]: [string, any]) => {
      // modelData is never read
      Object.entries(apiKeyBreakdown).forEach(([apiKey, apiKeyData]: [string, any]) => {
        dailyEntityModels[entity][model].spend += apiKeyData.metrics.spend || 0;
        // ...
      });
    });

    If an entity has M api keys and there are N models, each api key's full spend/requests are added N times across all models. The resulting CSV rows for "Daily with Models" will show inflated — and identical — numbers for every model rather than a per-model breakdown.

    This logic was present before this PR, but the new resolveEntities fallback now allows the aggregated endpoint's data to flow through this path, making the bug newly reachable for deployments using that endpoint. The fix would be to attribute only the fraction of each API key's usage that belongs to each model, or to source per-model metrics from modelData instead of from the API key totals.

Last reviewed commit: 302292c

]
] = Field(
default=LitellmUserRoles.INTERNAL_USER,
default=LitellmUserRoles.INTERNAL_USER_VIEW_ONLY,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Backwards-incompatible permission downgrade without a feature flag

The default for user_role is changed from INTERNAL_USER to INTERNAL_USER_VIEW_ONLY. While the PR description explains this aligns the Pydantic default with the SSO/SCIM/JWT runtime fallback, it is still a backwards-incompatible change for users who rely on DefaultInternalUserParams in non-SSO provisioning code paths.

Any deployment that has default_internal_user_params configured without an explicit user_role key was previously getting INTERNAL_USER (write access) as the Pydantic default. After upgrading, those same deployments will silently get INTERNAL_USER_VIEW_ONLY (read-only) for all newly provisioned users — with no opt-out mechanism.

Per the repository's policy on backwards-incompatible changes, this should be guarded by a user-controlled flag (e.g., check whether the user has explicitly set user_role in their config before applying the new default, or expose a flag like litellm.new_user_default_role). Without such a flag, admins who upgrade will find their new users unexpectedly can't perform write operations.

Suggested change
default=LitellmUserRoles.INTERNAL_USER_VIEW_ONLY,
default=LitellmUserRoles.INTERNAL_USER_VIEW_ONLY,

(If this change is intentional and the SSO/SCIM/JWT paths were the only ones ever used in practice, please add a note in the PR description and a BREAKING CHANGE entry in the changelog so upgrading users are explicitly aware.)

Rule Used: What: avoid backwards-incompatible changes without... (source)

Comment on lines 275 to +276
Object.entries(dailyEntityModels).forEach(([entity, models]) => {
const entityData = day.breakdown.entities?.[entity];
const entityData = resolveEntities(day.breakdown)[entity];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 resolveEntities called twice per day in generateDailyWithModelsData

resolveEntities(day.breakdown) is invoked once at line 248 (inside the entity loop) and again on line 276 (to look up a single entity by key). In the fallback path, each call iterates over all api_keys and builds a new aggregated object, so this effectively processes the same keys twice per day result.

Consider caching the resolved entities per day:

    const resolvedEntities = resolveEntities(day.breakdown);

    Object.entries(resolvedEntities).forEach(([entity, entityData]: [string, any]) => {
      // ... existing entity loop body ...
    });

    Object.entries(dailyEntityModels).forEach(([entity, models]) => {
      const entityData = resolvedEntities[entity];
      // ...
    });

This eliminates the redundant re-computation and makes the intent clearer.


Extracts title, description, date (YYYY-MM-DD), and url from each <item>.
"""
root = ET.fromstring(xml_text)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 XML entity expansion risk with user-configurable URL

xml.etree.ElementTree.fromstring() does not protect against "billion laughs" / XML entity expansion attacks. Since LITELLM_BLOG_POSTS_URL is user-configurable via environment variable, a malicious XML payload at a custom URL could cause excessive memory/CPU consumption.

Consider replacing the standard library parser with defusedxml, which is already a common dependency in Python security-conscious projects:

import defusedxml.ElementTree as ET

This is a drop-in replacement — no other code changes are needed. The rest of the parsing logic remains identical.

const breakdown = aggregatedSpendData.results[0].breakdown;
const result = resolveEntities(breakdown);

// Two teams: team-1 (key1+key2) and team-2 (key3)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Misleading comment — wrong key names for each team

The comment states team-1 (key1+key2) and team-2 (key3), but the assertions directly below and the api_key_breakdown preservation test confirm the actual layout is:

  • team-1key1 + key1b
  • team-2key2

key2 and key3 are swapped, and key1b is missing from the description. This could mislead anyone debugging or extending these tests.

Suggested change
// Two teams: team-1 (key1+key2) and team-2 (key3)
// Two teams: team-1 (key1+key1b) and team-2 (key2)

Copy link
Copy Markdown
Contributor

@yuneng-jiang yuneng-jiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ryan-crabbe ryan-crabbe merged commit ef9cc33 into main Mar 17, 2026
99 of 101 checks passed
@ryan-crabbe ryan-crabbe deleted the litellm_ryan_march_16 branch March 17, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants