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.
This commit is contained in:
Mrinal Chanshetty 2026-10-20 14:37:26 -07:00
parent 3e8973da24
commit 0f253ef01d
6 changed files with 12 additions and 25 deletions

View file

@ -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))

View file

@ -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)}"

View file

@ -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:%')"
)

View file

@ -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:

View file

@ -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"))

View file

@ -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")