From 0f253ef01d943b85fec12f446ac1a24dcd539216 Mon Sep 17 00:00:00 2001 From: Mrinal Chanshetty Date: Tue, 20 Oct 2026 14:37:26 -0700 Subject: [PATCH] fix(proxy): resolve group_by=tag on the named team when team_ids has a stray comma group_by_tag_error counted team_ids without empty entries, but the resolver got the raw list back, so team_ids=team-a, passed the one-team check and then resolved to two teams. Admins got a 500 and members a 404 naming an empty team. Both now resolve the same non-empty names, so the request reads team-a's tags. Trims comments that restated the code. --- .../daily_activity_routes.py | 1 - .../daily_activity_scopes.py | 17 ++++++++--------- litellm/repositories/daily_activity_sql.py | 4 +--- litellm/types/repositories/daily_activity.py | 2 -- .../test_daily_activity_routes.py | 10 +++------- .../repositories/test_daily_activity_sql.py | 3 --- 6 files changed, 12 insertions(+), 25 deletions(-) diff --git a/litellm/proxy/management_endpoints/daily_activity_routes.py b/litellm/proxy/management_endpoints/daily_activity_routes.py index 1244efb7d33..8783f8f1f65 100644 --- a/litellm/proxy/management_endpoints/daily_activity_routes.py +++ b/litellm/proxy/management_endpoints/daily_activity_routes.py @@ -102,7 +102,6 @@ async def _resolved_scope( date_error: Final[InvalidDateRange | None] = _date_range_error(query, user_aggregated=user_aggregated) if date_error is not None: raise_public(date_error) - # A malformed request is a 400 before any permission lookup, the same as a bad date range. group_by_error: Final[str | None] = group_by_tag_error(query) if group_by_error is not None: raise_public(InvalidDateRange(reason=group_by_error)) diff --git a/litellm/proxy/management_endpoints/daily_activity_scopes.py b/litellm/proxy/management_endpoints/daily_activity_scopes.py index ac329c4affa..974d8dede96 100644 --- a/litellm/proxy/management_endpoints/daily_activity_scopes.py +++ b/litellm/proxy/management_endpoints/daily_activity_scopes.py @@ -40,7 +40,6 @@ class ResolvedScope: Entity = Literal["user", "team", "tag", "organization", "customer", "agent"] -# How the Team view buckets its breakdown: by team (default), or by tag for one team's spend. TeamGroupBy: TypeAlias = Literal["team", "tag"] EntityScopeResolution = ResolvedScope | ScopeDenied EntityScopeQuery = Callable[..., EntityQuery] @@ -127,8 +126,9 @@ async def _resolve_team( ) -> EntityScopeResolution: from litellm.proxy.proxy_server import proxy_logging_obj, user_api_key_cache + requested: Final = _named_team_ids(query) if query.group_by == "tag" else query.entity_ids team_scope: Final = await resolve_team_daily_activity_scope( - team_ids=",".join(query.entity_ids) if query.entity_ids is not None else None, + team_ids=",".join(requested) if requested is not None else None, exclude_team_ids=",".join(query.exclude_entity_ids) if query.exclude_entity_ids else None, api_key=query.api_key, user_api_key_dict=user_api_key_dict, @@ -146,9 +146,6 @@ async def _resolve_team( ) if query.group_by == "team": return team - # group_by=tag: the same permitted team and key filter, read from the tag table and bucketed - # by tag. group_by_tag_error already held the request to exactly one named team, and the - # resolver returns a named team list as given (or 404s it), so this is that one team. permitted: Final = tuple(team_scope.team_ids or ()) if len(permitted) != 1: raise ValueError(f"group_by=tag resolved to {len(permitted)} teams, expected exactly one") @@ -296,13 +293,15 @@ def _team_query( ) +def _named_team_ids(query: EntityQuery) -> tuple[str, ...]: + return tuple(team_id for team_id in query.entity_ids or () if team_id) + + def group_by_tag_error(query: EntityQuery) -> str | None: - """Why a group_by=tag request is malformed, else None. A team's tag breakdown is one team's - spend, so it needs exactly one team named in team_ids, whoever is asking: checked before any - permission lookup, since that lookup fills in every team a non-admin belongs to.""" + """Checked before any permission lookup, which fills in every team a non-admin belongs to.""" if query.group_by != "tag": return None - named: Final = tuple(team_id for team_id in query.entity_ids or () if team_id) + named: Final = _named_team_ids(query) if len(named) == 1: return None return f"group_by=tag needs exactly one team in team_ids, got {len(named)}" diff --git a/litellm/repositories/daily_activity_sql.py b/litellm/repositories/daily_activity_sql.py index e2cb74b4060..00eab423dd2 100644 --- a/litellm/repositories/daily_activity_sql.py +++ b/litellm/repositories/daily_activity_sql.py @@ -68,7 +68,6 @@ def build_where_clause(scope: DailyActivityScope, *, start_index: int = 1) -> tu exclusion_index: Final = entity_index + int(has_entity_array) model_index: Final = exclusion_index + int(bool(scope.exclude_entity_ids)) api_keys_index: Final = model_index + int(bool(scope.model)) - # Appended after every existing filter so a scope without a team binds exactly as before. team_index: Final = api_keys_index + int(bool(scope.api_keys)) conditions: Final = ( f"date >= ${start_index}", @@ -110,8 +109,7 @@ def build_where_clause(scope: DailyActivityScope, *, start_index: int = 1) -> tu return " AND ".join(conditions), params -# The proxy stamps a User-Agent tag on every request, so in a team's tag breakdown these would -# outnumber its real tags. Same rule as common_daily_activity._is_user_agent_tag. +# Same rule as common_daily_activity._is_user_agent_tag _NOT_USER_AGENT_TAG: Final = ( "NOT (LOWER(BTRIM(tag)) LIKE 'user-agent:%' OR LOWER(BTRIM(tag)) LIKE 'user agent:%')" ) diff --git a/litellm/types/repositories/daily_activity.py b/litellm/types/repositories/daily_activity.py index f6e5e5c6641..e279f64e00a 100644 --- a/litellm/types/repositories/daily_activity.py +++ b/litellm/types/repositories/daily_activity.py @@ -39,8 +39,6 @@ class DailyActivityScope: model: str | None timezone_offset_minutes: int | None include_current_utc_day: bool = False - # One team's tag spend (the Team view's Spend Per Tag card). Only the tag table records the - # sending team, and a team-scoped tag read leaves out the automatic User-Agent tags. team_id: str | None = None def __post_init__(self) -> None: diff --git a/tests/unit/proxy/management_endpoints/test_daily_activity_routes.py b/tests/unit/proxy/management_endpoints/test_daily_activity_routes.py index b4c42c068b3..8b08dce1478 100644 --- a/tests/unit/proxy/management_endpoints/test_daily_activity_routes.py +++ b/tests/unit/proxy/management_endpoints/test_daily_activity_routes.py @@ -1267,15 +1267,15 @@ def test_team_scope_applies_membership_and_user_key_filter( assert repository.aggregated.call_args.kwargs["include_entity_breakdown"] is True +@pytest.mark.parametrize("team_ids", ["team-a", "team-a,", ",team-a"]) def test_team_group_by_tag_reads_one_teams_tag_rows_with_the_members_key_filter( daily_activity_client: tuple[TestClient, _FakeRepository], + team_ids: str, ) -> None: - """group_by=tag keeps every team-route permission (membership, the member's own keys) and - only swaps the source to the tag table, scoped to that one team and bucketed by tag.""" client, repository = daily_activity_client response: Final = client.get( "/team/daily/activity/aggregated", - params={**_entity_params("team_ids", "team-a"), "group_by": "tag"}, + params={**_entity_params("team_ids", team_ids), "group_by": "tag"}, headers={"x-user-role": LitellmUserRoles.INTERNAL_USER.value, "x-user-id": "user-a"}, ) assert response.status_code == 200, response.text @@ -1300,8 +1300,6 @@ def test_team_group_by_tag_needs_exactly_one_named_team_for_every_caller( expected_count: int, role: LitellmUserRoles, ) -> None: - """Answered before any permission lookup, which would otherwise fill in every team a - non-admin belongs to and quietly turn "no team" into "all of my teams".""" client, repository = daily_activity_client response: Final = client.get( "/team/daily/activity/aggregated", @@ -1319,8 +1317,6 @@ def test_team_group_by_tag_carries_no_ptu_hours( daily_activity_client: tuple[TestClient, _FakeRepository], monkeypatch: pytest.MonkeyPatch, ) -> None: - """PTU hours are a whole-team figure; the same sized team that reports them by team reports - none per tag.""" client, _ = daily_activity_client monkeypatch.setenv("LITELLM_ENABLE_PTU_COST_ATTRIBUTION", "True") _inject_llm_router(client, _ptu_sized_router("rare-group")) diff --git a/tests/unit/repositories/test_daily_activity_sql.py b/tests/unit/repositories/test_daily_activity_sql.py index 2a31f764e47..28ce51470df 100644 --- a/tests/unit/repositories/test_daily_activity_sql.py +++ b/tests/unit/repositories/test_daily_activity_sql.py @@ -113,8 +113,6 @@ def test_where_clause_distinguishes_no_filter_from_empty_membership( def test_where_clause_scopes_tag_rows_to_one_team_without_user_agent_tags() -> None: - """The team filter binds after every existing filter, so it never renumbers them, and the - User-Agent tags the proxy stamps on every request are left out of a team's tag breakdown.""" scope = replace( _scope(table=DailyActivityTable.TAG, entity_ids=None, api_keys=("key-1",), model="gpt-test"), team_id="team-a", @@ -131,7 +129,6 @@ def test_where_clause_scopes_tag_rows_to_one_team_without_user_agent_tags() -> N @pytest.mark.parametrize("table", [table for table in DailyActivityTable if table is not DailyActivityTable.TAG]) def test_scope_rejects_a_team_id_on_any_table_but_the_tag_table(table: DailyActivityTable) -> None: - """Only LiteLLM_DailyTagSpend records the sending team in a team_id column next to its own entity.""" with pytest.raises(ValueError, match="team_id applies only to litellm_dailytagspend"): replace(_scope(table=table), team_id="team-a")