fix(spend): never join blank model_ids in the team-model exclusion

LiteLLM_SpendLogs.model_id defaults to '' for rows that never resolved to
a router deployment, and /model/new honors a caller-supplied
model_info.id, so a deployment row whose primary key is '' can be
created. That single row matched every unresolved spend row, letting the
exclusion erase most of a billing report.

Short-circuit NULL and empty model_ids before the anti-join, the same
guard /global/spend/provider already applies to that column.
This commit is contained in:
ryan-crabbe-berri 2026-07-27 10:32:57 -07:00
parent e118ebec55
commit 687f09ebfb
2 changed files with 28 additions and 4 deletions

View file

@ -518,6 +518,10 @@ def team_model_exclusion_clause(exclude_team_models: bool) -> str:
exclusion survives a rename of the deployment's mangled `model_name`. `->>` yields NULL
on an absent key or a non-object `model_info`, which fails open to "included".
Blank `model_id` is the default for spend rows that never resolved to a router deployment,
so it identifies no deployment and must never join; `/global/spend/provider` guards the
same column the same way.
Only a literal True opts in, so every existing query stays byte-identical; in-process
callers that omit the argument leave FastAPI's `Query` default object in place, and that
object is truthy.
@ -525,8 +529,9 @@ def team_model_exclusion_clause(exclude_team_models: bool) -> str:
if exclude_team_models is not True:
return ""
return (
'\n AND NOT EXISTS (SELECT 1 FROM "LiteLLM_ProxyModelTable" pm '
"WHERE pm.model_id = sl.model_id AND pm.model_info ->> 'team_id' IS NOT NULL)"
"\n AND (sl.model_id IS NULL OR length(sl.model_id) = 0 OR NOT EXISTS ("
'SELECT 1 FROM "LiteLLM_ProxyModelTable" pm '
"WHERE pm.model_id = sl.model_id AND pm.model_info ->> 'team_id' IS NOT NULL))"
)

View file

@ -507,8 +507,9 @@ async def test_global_spend_report_team_group_forwards_team_id(monkeypatch):
_OMITTED = object()
_EXCLUSION_PREDICATE = (
'\n AND NOT EXISTS (SELECT 1 FROM "LiteLLM_ProxyModelTable" pm '
"WHERE pm.model_id = sl.model_id AND pm.model_info ->> 'team_id' IS NOT NULL)"
"\n AND (sl.model_id IS NULL OR length(sl.model_id) = 0 OR NOT EXISTS ("
'SELECT 1 FROM "LiteLLM_ProxyModelTable" pm '
"WHERE pm.model_id = sl.model_id AND pm.model_info ->> 'team_id' IS NOT NULL))"
)
_REPORT_BRANCHES = {
@ -579,6 +580,24 @@ async def test_global_spend_report_excludes_team_models_when_opted_in(monkeypatc
)
@pytest.mark.parametrize("branch", sorted(_REPORT_BRANCHES))
@pytest.mark.asyncio
async def test_global_spend_report_never_joins_blank_model_ids(monkeypatch, branch):
"""
LiteLLM_SpendLogs.model_id defaults to '' for rows that never resolved to a deployment, and
/model/new honors a caller-supplied model_info.id, so a deployment row can be created whose
primary key is ''. Without this guard that single row matches every unresolved spend row and
the flag erases most of the report.
"""
sql = await _capture_report_sql(monkeypatch, _REPORT_BRANCHES[branch], exclude_team_models=True)
guard = "sl.model_id IS NULL OR length(sl.model_id) = 0"
assert guard in sql, f"{branch} must short-circuit blank model_ids before the join. SQL was:\n{sql}"
assert sql.index(guard) < sql.index('"LiteLLM_ProxyModelTable"'), (
f"the blank-id guard must precede the anti-join so a '' deployment row can never match. SQL was:\n{sql}"
)
@pytest.mark.parametrize("branch", sorted(_REPORT_BRANCHES))
@pytest.mark.parametrize("exclude_team_models", [False, _OMITTED], ids=["explicit_false", "omitted"])
@pytest.mark.asyncio