fix(ui): draw one Per Day savings bar per date on Cost Optimization (#37643)

* fix(ui): draw one Per Day savings bar per date on Cost Optimization

The page paged /user/daily/activity over raw rows, so a date spanning
pages arrived N times with partial metrics and rendered as N thin bars.
Switch to the single-shot aggregated endpoint, thread
include_current_utc_day through it to keep the live-end extension from
PR #36051, and merge the paginated fallback by date.

* fix(ui): keep aggregated call at four params and mock it in view tests

Trailing userId and includeCurrentUtcDay ride a named rest tuple so the
eslint max-params baseline stays at 23, and the CostOptimizationView
suites mock the new networking export their render now reaches.
This commit is contained in:
tin-berri 2026-08-20 11:14:04 -07:00 • committed by GitHub
parent 8672cd4df4
commit c164944d40
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 276 additions and 14 deletions

View file

@ -658,6 +658,7 @@ def _build_aggregated_sql_query(
api_key: str | list[str] | None, # mutable-ok: filter union shared with the paginated path
exclude_entity_ids: list[str] | None = None, # mutable-ok: filter union shared with the paginated path
timezone_offset_minutes: int | None = None,
include_current_utc_day: bool = False,
) -> tuple[str, list[str]]: # mutable-ok: SQL text plus its ordered $N params
"""Build a parameterized SQL GROUP BY query for aggregated daily activity.
@ -673,7 +674,9 @@ def _build_aggregated_sql_query(
if pg_table is None:
raise ValueError(f"Unknown table name: {table_name}")
adjusted_start, adjusted_end = _adjust_dates_for_timezone(start_date, end_date, timezone_offset_minutes)
adjusted_start, adjusted_end = _adjust_dates_for_timezone(
start_date, end_date, timezone_offset_minutes, include_current_utc_day
)
where_clause, sql_params = _build_aggregated_where_clause(
entity_id_field=entity_id_field,
@ -755,6 +758,7 @@ def _build_entity_rollup_sql_query(
api_key: str | list[str] | None, # mutable-ok: filter union shared with the paginated path
exclude_entity_ids: list[str] | None = None, # mutable-ok: filter union shared with the paginated path
timezone_offset_minutes: int | None = None,
include_current_utc_day: bool = False,
) -> tuple[str, list[str]]: # mutable-ok: SQL text plus its ordered $N params
"""Per-entity companion to _build_aggregated_sql_query.
@ -766,7 +770,9 @@ def _build_entity_rollup_sql_query(
if pg_table is None:
raise ValueError(f"Unknown table name: {table_name}")
adjusted_start, adjusted_end = _adjust_dates_for_timezone(start_date, end_date, timezone_offset_minutes)
adjusted_start, adjusted_end = _adjust_dates_for_timezone(
start_date, end_date, timezone_offset_minutes, include_current_utc_day
)
where_clause, sql_params = _build_aggregated_where_clause(
entity_id_field=entity_id_field,
@ -1256,6 +1262,7 @@ async def get_daily_activity_aggregated(
exclude_entity_ids: list[str] | None = None,
timezone_offset_minutes: int | None = None,
include_entity_breakdown: bool = False,
include_current_utc_day: bool = False,
) -> SpendAnalyticsPaginatedResponse:
"""Aggregated variant that returns the full result set (no pagination).
@ -1291,6 +1298,7 @@ async def get_daily_activity_aggregated(
api_key=api_key,
exclude_entity_ids=exclude_entity_ids,
timezone_offset_minutes=timezone_offset_minutes,
include_current_utc_day=include_current_utc_day,
)
entity_query: Final = (
@ -1304,6 +1312,7 @@ async def get_daily_activity_aggregated(
api_key=api_key,
exclude_entity_ids=exclude_entity_ids,
timezone_offset_minutes=timezone_offset_minutes,
include_current_utc_day=include_current_utc_day,
)
if include_entity_breakdown
else None

View file

@ -2790,6 +2790,13 @@ async def get_user_daily_activity_aggregated(
description="Timezone offset in minutes from UTC (e.g., 480 for PST). "
"Matches JavaScript's Date.getTimezoneOffset() convention.",
),
include_current_utc_day: bool = fastapi.Query(
default=False,
description="When the range ends on the caller's current local day, extend it to "
"today's UTC bucket so spend written after the caller's local midnight (in UTC "
"terms) is included. Requires the timezone parameter. Historical ranges are "
"never extended.",
),
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
) -> SpendAnalyticsPaginatedResponse:
"""
@ -2837,6 +2844,7 @@ async def get_user_daily_activity_aggregated(
model=model,
api_key=api_key,
timezone_offset_minutes=timezone,
include_current_utc_day=include_current_utc_day,
)
except HTTPException:

View file

@ -1,6 +1,6 @@
import os
import sys
from datetime import datetime, timezone
from datetime import datetime, timedelta, timezone
from types import SimpleNamespace
from typing import Final
from unittest.mock import AsyncMock, MagicMock
@ -928,6 +928,33 @@ class TestBuildAggregatedSqlQuery:
assert "date >= $1" in sql
assert "date <= $2" in sql
@pytest.mark.parametrize("build", [_build_aggregated_sql_query, _build_entity_rollup_sql_query])
def test_include_current_utc_day_extends_live_end_bound(self, build):
"""
An offset larger than 24h keeps the caller's local date behind UTC at any
wall-clock hour, so the live-end extension is deterministic: a range ending
on the caller's local today must reach today's UTC bucket (LIT-5818, guards
the #36051 behavior on the aggregated path).
"""
offset_minutes: Final = 1500
caller_local_today: Final = (datetime.now(timezone.utc) - timedelta(minutes=offset_minutes)).date().isoformat()
utc_today: Final = datetime.now(timezone.utc).date().isoformat()
_sql, params = build(
table_name="litellm_dailyuserspend",
entity_id_field="user_id",
entity_id="user-1",
start_date="2026-05-01",
end_date=caller_local_today,
model=None,
api_key=None,
timezone_offset_minutes=offset_minutes,
include_current_utc_day=True,
)
assert params[0] == "2026-05-01"
assert params[1] == utc_today
def test_optional_filters_appear_in_params_in_order(self):
sql, params = _build_aggregated_sql_query(
table_name="litellm_dailyuserspend",

View file

@ -2253,7 +2253,8 @@ async def test_get_user_daily_activity_aggregated_rejects_service_account_caller
@pytest.mark.asyncio
async def test_get_user_daily_activity_aggregated_admin_global_view(monkeypatch):
@pytest.mark.parametrize("include_current_utc_day", [False, True])
async def test_get_user_daily_activity_aggregated_admin_global_view(monkeypatch, include_current_utc_day):
"""
Test that admin users can call the aggregated endpoint without a user_id
to get a global view. Also verifies that the correct arguments are forwarded
@ -2291,6 +2292,7 @@ async def test_get_user_daily_activity_aggregated_admin_global_view(monkeypatch)
api_key=None,
user_id=None,
timezone=480,
include_current_utc_day=include_current_utc_day,
user_api_key_dict=admin_key_dict,
)
@ -2308,6 +2310,7 @@ async def test_get_user_daily_activity_aggregated_admin_global_view(monkeypatch)
model="gpt-4",
api_key=None,
timezone_offset_minutes=480,
include_current_utc_day=include_current_utc_day,
)

View file

@ -4,6 +4,7 @@ import { describe, expect, it, vi } from "vitest";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
const mockUserDailyActivityCall = vi.fn();
const mockUserDailyActivityAggregatedCall = vi.fn();
const { useAuthorizedMock, mockToolSpendResponse } = vi.hoisted(() => ({
useAuthorizedMock: vi.fn(),
mockToolSpendResponse: { by_tool: [], daily: [], start_date: null, end_date: null },
@ -15,6 +16,7 @@ vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({
vi.mock("@/components/networking", () => ({
userDailyActivityCall: (...args: unknown[]) => mockUserDailyActivityCall(...args),
userDailyActivityAggregatedCall: (...args: unknown[]) => mockUserDailyActivityAggregatedCall(...args),
getToolSpend: vi.fn().mockResolvedValue(mockToolSpendResponse),
getGeneralSettingsCall: vi.fn().mockResolvedValue([]),
organizationListCall: vi.fn().mockResolvedValue([]),
@ -48,7 +50,7 @@ const singlePage = {
describe("CostOptimizationView daily activity", () => {
it("fetches daily activity once for the page and shares it with every tab that needs it", async () => {
mockUserDailyActivityCall.mockResolvedValue(singlePage);
mockUserDailyActivityAggregatedCall.mockResolvedValue(singlePage);
useAuthorizedMock.mockReturnValue({ accessToken: "test-token", userId: "u1", userRole: "proxy_admin" });
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } });
@ -58,11 +60,12 @@ describe("CostOptimizationView daily activity", () => {
</QueryClientProvider>,
);
await waitFor(() => expect(mockUserDailyActivityCall).toHaveBeenCalledTimes(1));
await waitFor(() => expect(mockUserDailyActivityAggregatedCall).toHaveBeenCalledTimes(1));
fireEvent.click(getByRole("tab", { name: "Prompt Caching" }));
await findByTestId("caching-settings");
expect(mockUserDailyActivityCall).toHaveBeenCalledTimes(1);
expect(mockUserDailyActivityAggregatedCall).toHaveBeenCalledTimes(1);
expect(mockUserDailyActivityCall).not.toHaveBeenCalled();
});
});

View file

@ -14,6 +14,9 @@ vi.mock("@/components/networking", () => ({
userDailyActivityCall: vi
.fn()
.mockResolvedValue({ results: [], metadata: { total_pages: 1, has_more: false, page: 1 } }),
userDailyActivityAggregatedCall: vi
.fn()
.mockResolvedValue({ results: [], metadata: { total_pages: 1, has_more: false, page: 1 } }),
}));
vi.mock("./UsageTab", () => ({ __esModule: true, default: () => <div data-testid="usage-tab" /> }));

View file

@ -12,8 +12,10 @@ vi.mock("@/app/(dashboard)/usage/_components/hooks/usePaginatedDailyActivity", (
vi.mock("@/components/networking", () => ({
userDailyActivityCall: vi.fn(),
userDailyActivityAggregatedCall: vi.fn(),
}));
import { userDailyActivityAggregatedCall } from "@/components/networking";
import { useDailyActivityRange } from "./useDailyActivityRange";
const argsOfLastCall = () => mockUsePaginatedDailyActivity.mock.calls.at(-1)?.[0].args as unknown[];
@ -31,6 +33,14 @@ describe("useDailyActivityRange", () => {
expect(argsOfLastCall()).toEqual(["test-token", expect.any(Date), expect.any(Date), "u1", true]);
});
it("fetches through the single-shot aggregated endpoint first so days never fragment across pages", () => {
renderHook(() => useDailyActivityRange("test-token", "u1", "proxy_admin"));
expect(mockUsePaginatedDailyActivity).toHaveBeenLastCalledWith(
expect.objectContaining({ aggregatedFetchFn: userDailyActivityAggregatedCall }),
);
});
it("stays disabled until an access token is available", () => {
renderHook(() => useDailyActivityRange(null, "u1", "proxy_admin"));

View file

@ -1,6 +1,6 @@
import { useMemo, useState } from "react";
import { userDailyActivityCall } from "@/components/networking";
import { userDailyActivityAggregatedCall, userDailyActivityCall } from "@/components/networking";
import { DailyData } from "@/components/UsagePage/types";
import { all_admin_roles } from "@/utils/roles";
import { usePaginatedDailyActivity } from "@/app/(dashboard)/usage/_components/hooks/usePaginatedDailyActivity";
@ -35,6 +35,7 @@ export const useDailyActivityRange = (
const { data, loading, isFetchingMore } = usePaginatedDailyActivity({
fetchFn: userDailyActivityCall,
aggregatedFetchFn: userDailyActivityAggregatedCall,
args: [accessToken, startTime, endTime, effectiveUserId, true],
enabled: !!accessToken && !!startTime && !!endTime,
});

View file

@ -1,5 +1,7 @@
import { describe, expect, it } from "vitest";
import { sumMetadata } from "./usePaginatedDailyActivity";
import { renderHook, waitFor } from "@testing-library/react";
import { describe, expect, it, vi } from "vitest";
import { DailyData, SpendMetrics } from "@/components/UsagePage/types";
import { mergeDailyResults, sumMetadata, usePaginatedDailyActivity } from "./usePaginatedDailyActivity";
describe("sumMetadata", () => {
it("sums flat cost across pages instead of keeping the first page's value", () => {
@ -49,3 +51,108 @@ describe("sumMetadata", () => {
}
});
});
const metricsOf = (spend: number): SpendMetrics => ({
spend,
prompt_tokens: 0,
completion_tokens: 0,
total_tokens: 0,
api_requests: 1,
successful_requests: 1,
failed_requests: 0,
cache_read_input_tokens: 0,
cache_creation_input_tokens: 0,
compression_savings_spend: spend,
});
const dayOf = (date: string, spend: number, apiKey: string = "sk-1"): DailyData => ({
date,
metrics: metricsOf(spend),
breakdown: {
models: {
"gpt-4o": {
metrics: metricsOf(spend),
metadata: {},
api_key_breakdown: {
[apiKey]: { metrics: metricsOf(spend), metadata: { key_alias: "alias-1", team_id: null } },
},
},
},
model_groups: {},
mcp_servers: {},
providers: {},
api_keys: { [apiKey]: { metrics: metricsOf(spend), metadata: { key_alias: "alias-1", team_id: null } } },
entities: {},
},
});
describe("mergeDailyResults", () => {
it("collapses repeated dates into one entry with summed metrics (the LIT-5818 $2/$2/$1 case)", () => {
const merged = mergeDailyResults(mergeDailyResults([dayOf("2026-08-16", 2)], [dayOf("2026-08-16", 2)]), [
dayOf("2026-08-16", 1),
]);
expect(merged).toHaveLength(1);
expect(merged[0].metrics.spend).toBe(5);
expect(merged[0].metrics.compression_savings_spend).toBe(5);
});
it("appends unseen dates in arrival order", () => {
const merged = mergeDailyResults([dayOf("2026-08-16", 2)], [dayOf("2026-08-15", 0.5)]);
expect(merged.map((d) => d.date)).toEqual(["2026-08-16", "2026-08-15"]);
expect(merged[1].metrics.spend).toBe(0.5);
});
it("merges every breakdown level including the nested per-key breakdown", () => {
const merged = mergeDailyResults([dayOf("2026-08-16", 2, "sk-1")], [dayOf("2026-08-16", 3, "sk-1")]);
expect(merged[0].breakdown.models["gpt-4o"].metrics.spend).toBe(5);
expect(merged[0].breakdown.models["gpt-4o"].api_key_breakdown["sk-1"].metrics.spend).toBe(5);
expect(merged[0].breakdown.api_keys["sk-1"].metrics.spend).toBe(5);
expect(merged[0].breakdown.api_keys["sk-1"].metadata.key_alias).toBe("alias-1");
});
it("unions breakdown keys that appear on different pages", () => {
const merged = mergeDailyResults([dayOf("2026-08-16", 2, "sk-1")], [dayOf("2026-08-16", 3, "sk-2")]);
expect(merged[0].breakdown.api_keys["sk-1"].metrics.spend).toBe(2);
expect(merged[0].breakdown.api_keys["sk-2"].metrics.spend).toBe(3);
});
it("sums metric keys it has never heard of so a future backend column cannot silently freeze", () => {
const withExtra = (spend: number): DailyData => ({
...dayOf("2026-08-16", spend),
metrics: { ...metricsOf(spend), future_savings_spend: spend } as SpendMetrics,
});
const merged = mergeDailyResults([withExtra(2)], [withExtra(3)]);
expect((merged[0].metrics as Record<string, number>).future_savings_spend).toBe(5);
});
});
describe("usePaginatedDailyActivity page accumulation", () => {
it("returns one entry per date when a date's rows span multiple pages", async () => {
const pages = [
{ results: [dayOf("2026-08-16", 2)], metadata: { total_pages: 3, page: 1, total_spend: 2 } },
{ results: [dayOf("2026-08-16", 2)], metadata: { total_pages: 3, page: 2, total_spend: 2 } },
{
results: [dayOf("2026-08-16", 1), dayOf("2026-08-15", 0.5)],
metadata: { total_pages: 3, page: 3, total_spend: 1.5 },
},
];
const fetchFn = vi.fn((_token: string, _start: Date, _end: Date, page: number) => Promise.resolve(pages[page - 1]));
const start = new Date("2026-08-10");
const end = new Date("2026-08-17");
const { result } = renderHook(() =>
usePaginatedDailyActivity({ fetchFn, args: ["tok", start, end, null], enabled: true }),
);
await waitFor(() => expect(result.current.data.metadata.page).toBe(3), { timeout: 5000 });
expect(result.current.data.results.map((d) => d.date)).toEqual(["2026-08-16", "2026-08-15"]);
expect(result.current.data.results[0].metrics.spend).toBe(5);
expect(result.current.data.metadata.total_spend).toBe(5.5);
});
});

View file

@ -1,5 +1,11 @@
import { useCallback, useEffect, useRef, useState } from "react";
import { DailyData } from "@/components/UsagePage/types";
import {
BreakdownMetrics,
DailyData,
KeyMetricWithMetadata,
MetricWithMetadata,
SpendMetrics,
} from "@/components/UsagePage/types";
export interface PaginationProgress {
currentPage: number;
@ -89,6 +95,87 @@ export function sumMetadata(a: Record<string, any>, b: Record<string, any>): Rec
return result;
}
/**
* Sum the union of numeric metric keys so a metric column added to the backend
* later is summed automatically instead of silently frozen at one page's value
* (the drift hazard SUMMABLE_METADATA_KEYS documents above).
*/
const addMetrics = (a: SpendMetrics, b: SpendMetrics): SpendMetrics =>
Object.fromEntries(
Array.from(new Set([...Object.keys(a), ...Object.keys(b)])).map((key) => {
const left = a[key as keyof SpendMetrics];
const right = b[key as keyof SpendMetrics];
if (typeof left !== "number" && typeof right !== "number") return [key, left ?? right];
return [key, (typeof left === "number" ? left : 0) + (typeof right === "number" ? right : 0)];
}),
) as unknown as SpendMetrics;
const mergeBucketMaps = <T>(
a: Record<string, T> | undefined,
b: Record<string, T> | undefined,
mergeEntry: (left: T, right: T) => T,
): Record<string, T> => {
const left = a ?? {};
const right = b ?? {};
return Object.fromEntries(
Array.from(new Set([...Object.keys(left), ...Object.keys(right)])).map((key) => {
const leftEntry = left[key];
const rightEntry = right[key];
if (leftEntry === undefined) return [key, rightEntry];
if (rightEntry === undefined) return [key, leftEntry];
return [key, mergeEntry(leftEntry, rightEntry)];
}),
);
};
const mergeKeyMetric = (a: KeyMetricWithMetadata, b: KeyMetricWithMetadata): KeyMetricWithMetadata => ({
...a,
metrics: addMetrics(a.metrics, b.metrics),
});
const mergeMetricWithMetadata = (a: MetricWithMetadata, b: MetricWithMetadata): MetricWithMetadata => ({
...a,
metrics: addMetrics(a.metrics, b.metrics),
api_key_breakdown: mergeBucketMaps(a.api_key_breakdown, b.api_key_breakdown, mergeKeyMetric),
});
const mergeBreakdown = (a: BreakdownMetrics, b: BreakdownMetrics): BreakdownMetrics => ({
models: mergeBucketMaps(a.models, b.models, mergeMetricWithMetadata),
model_groups: mergeBucketMaps(a.model_groups, b.model_groups, mergeMetricWithMetadata),
mcp_servers: mergeBucketMaps(a.mcp_servers, b.mcp_servers, mergeMetricWithMetadata),
providers: mergeBucketMaps(a.providers, b.providers, mergeMetricWithMetadata),
api_keys: mergeBucketMaps(a.api_keys, b.api_keys, mergeKeyMetric),
entities: mergeBucketMaps(a.entities, b.entities, mergeMetricWithMetadata),
...(a.endpoints || b.endpoints
? { endpoints: mergeBucketMaps(a.endpoints, b.endpoints, mergeMetricWithMetadata) }
: {}),
});
/**
* The backend paginates over raw rows and re-groups per page, so a date whose
* rows span pages arrives as one partial DailyData per page. Merge by date so
* consumers never see the same date twice (LIT-5818: each day rendered as N
* partial bars). Exported so the contract can be tested directly.
*/
export function mergeDailyResults(existing: readonly DailyData[], incoming: readonly DailyData[]): DailyData[] {
return incoming.reduce<DailyData[]>(
(acc, day) => {
const index = acc.findIndex((existingDay) => existingDay.date === day.date);
if (index === -1) return [...acc, day];
return acc.map((existingDay, i) =>
i === index
? {
...existingDay,
metrics: addMetrics(existingDay.metrics, day.metrics),
breakdown: mergeBreakdown(existingDay.breakdown, day.breakdown),
}
: existingDay,
);
},
[...existing],
);
}
/**
* Hook that auto-paginates daily activity endpoints, updating state in batches
* so charts render progressively. Cancels on unmount, param changes, or
@ -203,7 +290,7 @@ export function usePaginatedDailyActivity({
setLoading(false);
setIsFetchingMore(true);
let accumulatedResults = [...firstPage.results];
let accumulatedResults = mergeDailyResults([], firstPage.results);
let accumulatedMetadata = { ...firstPage.metadata };
for (let page = 2; page <= totalPages; page++) {
@ -219,7 +306,7 @@ export function usePaginatedDailyActivity({
if (isStale()) return;
accumulatedResults = [...accumulatedResults, ...pageData.results];
accumulatedResults = mergeDailyResults(accumulatedResults, pageData.results);
accumulatedMetadata = sumMetadata(accumulatedMetadata, pageData.metadata);
accumulatedMetadata.total_pages = totalPages;
accumulatedMetadata.has_more = page < totalPages;

View file

@ -2502,11 +2502,12 @@ export const userDailyActivityAggregatedCall = async (
accessToken: string,
startTime: Date,
endTime: Date,
userId: string | null = null,
...options: [userId?: string | null, includeCurrentUtcDay?: boolean]
) => {
/**
* Get aggregated daily user activity (no pagination)
*/
const [userId = null, includeCurrentUtcDay = false] = options;
try {
const formatDate = (date: Date) => {
const year = date.getFullYear();
@ -2521,6 +2522,7 @@ export const userDailyActivityAggregatedCall = async (
end_date: formatDate(endTime),
timezone: new Date().getTimezoneOffset().toString(),
user_id: userId || undefined,
include_current_utc_day: includeCurrentUtcDay ? "true" : undefined,
},
});
} catch (error) {

View file

@ -55688,6 +55688,8 @@ export interface operations {
user_id?: string | null;
/** @description Timezone offset in minutes from UTC (e.g., 480 for PST). Matches JavaScript's Date.getTimezoneOffset() convention. */
timezone?: number | null;
/** @description When the range ends on the caller's current local day, extend it to today's UTC bucket so spend written after the caller's local midnight (in UTC terms) is included. Requires the timezone parameter. Historical ranges are never extended. */
include_current_utc_day?: boolean;
};
header?: never;
path?: never;