Skip to content

revert: "feat(usage): search team keys beyond the top-N in the Team usage view (#42857)" - #43377

Merged
yuneng-berri merged 2 commits into
mainfrom
litellm_revert_42857
Sep 28, 2026
Merged

yuneng-berri merged 2 commits into
mainfrom
litellm_revert_42857

Conversation

@yassin-berriai

Copy link
Copy Markdown
Contributor

Reverts #42857

TLDR

Problem this solves:

How it solves it:

  • Plain git revert of the squash commit c2eb549, no conflicts
  • Removes the GET /team/daily/activity/aggregated/search route, its route registration, tests and the Team usage page wiring

User Flow

Before: on Usage > Team Usage, typing the alias of a key below the top 100 finds it through a server search

  1. They open http://localhost:4000/ui/?page=team-usage, open Key Activity and type the alias
  2. The dashboard calls GET http://localhost:4000/team/daily/activity/aggregated/search?search=...&team_ids=... and shows the key

After: the page behaves as it did before #42857

  1. They open http://localhost:4000/ui/?page=team-usage, open Key Activity and type the alias
  2. The search filters only the loaded top-N keys; GET http://localhost:4000/team/daily/activity/aggregated/search returns 404

Type

🧹 Refactoring

Caveats (if any)

Low

🤖 Generated with Claude Code

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@greptile-apps

greptile-apps Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 3/5

[Medium risk] Removes a team usage search endpoint and related code.

The PR is not ready to merge because the removed route breaks existing clients and stale baseline entries fail CI

Findings

  1. P1 Existing clients lose search ▶
  2. P1 Stale baseline fails CI ▶
  3. P2 Surviving routes lose coverage ▶

Summary

This PR reverts server-side team-key search, including its proxy route, dashboard wiring, API types, and tests

  • The deleted route is incompatible with existing clients, and stale checker baseline entries will fail CI
  • A surviving role-and-route authorization assertion is lost with the removed test

Reviews (1) · Last reviewed commit: "Revert "feat(usage): search team keys be..."

@@ -6809,111 +6805,6 @@ async def get_team_daily_activity_aggregated(
)

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 Existing clients lose search Removing this route makes existing clients that call /team/daily/activity/aggregated/search receive 404. The repository requires a user-controlled flag for backwards-incompatible changes

Rule Used: What: avoid backwards-incompatible changes without user-controlled flags Why: This breaks current behaviour for users using existing functionality Example of BAD: this PR (#22164) introduced run_post_custom... (source)

@@ -6809,111 +6805,6 @@ async def get_team_daily_activity_aggregated(
)

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 Stale baseline fails CI Deleting _team_key_search_where leaves six entries for it in unbounded_in_baseline.txt. The code-quality checker rejects entries without a matching finding, so its CI job fails

assert outcome() == "allowed"


@pytest.mark.parametrize(

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 Surviving routes lose coverage Deleting this test also removes its internal view-only access checks for both routes that remain. The repository requires test changes not to weaken regression coverage, so this check must be restored before merging

Rule Used: What: Flag any modifications to existing tests and verify they don't weaken test coverage or mask regressions. Why: Developers may alter tests to make failing code pass rather than fix the actual bug, hiding regressions. Good: ``` // Test updated t... (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!

@codecov

codecov Bot commented Sep 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Base automatically changed from litellm_revert_42996 to main September 28, 2026 19:33
@yuneng-berri
yuneng-berri requested a review from a team September 28, 2026 19:33
@codspeed

codspeed Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_revert_42857 (1903cec) with main (22cfc66)

Open in CodSpeed

Comment thread litellm/proxy/_types.py
],
)

signoz: CallbackOnUI = CallbackOnUI(

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.

Please confirm if this is intentional. If not, please restore this

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.

Not intentional, the revert dropped it. Restored in the single commit a12070b, the signoz block now matches main

…)" (#43378)

* Revert "feat(usage): search keys beyond the top-N usage subset (#42827)"

* revert: "feat(proxy): add LiteLLM_DailyGlobalSpend key-free rollup for the usage dashboard (#41324)" (#43595)

* Revert "Merge pull request #41324 from BerriAI/litellm_daily_global_spend_table"

* Revert "Merge pull request #41293 from BerriAI/litellm_usage_key_free_aggregate_split" (#43596)

Co-authored-by: yassin <yassin@berri.ai>

---------

Co-authored-by: yassin <yassin@berri.ai>
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@yuneng-berri
yuneng-berri enabled auto-merge (squash) September 28, 2026 21:42
WHERE {where_clause}
GROUP BY GROUPING SETS (
(date),
(date, api_key),

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.

Low: Unbounded key rollups

The per-key grouping sets no longer have the top-key limit. A user with many historical keys, or a team administrator on a large team, can repeatedly request aggregated usage and make the proxy fetch, enrich, and serialize six per-key rollups across the requested dates. Keep a bound on per-key results or paginate that breakdown while preserving complete totals.

@veria-ai

veria-ai Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

PR overview

This PR reverts the Team usage view’s ability to search team keys beyond the top-N and changes how the proxy groups daily usage by key.

One issue remains open: per-key usage rollups are no longer bounded, so users with many historical keys or administrators of large teams can repeatedly trigger expensive aggregation and response serialization. The impact depends on the number of keys and the size of the requested date range. No issues have been addressed yet.

Open issues (1)

Fixed/addressed: 0 · PR risk: 4/10

@yuneng-berri
yuneng-berri merged commit 7e383c9 into main Sep 28, 2026
94 of 96 checks passed

This branch is waiting to be deployed

1 waiting deployment
e2e-changed — 1903cec2 Waiting Sep 28, 2026 by yuneng-berri via oauth #1671
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.

3 participants