From 69c32122200131efe14ac5ca4b70e3609e46b910 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Tue, 15 Sep 2026 16:21:37 -0700 Subject: [PATCH] fix(ui): gate the usage export on range coverage, not on a fetch being in flight A loading flag only flips once the fetch effect runs, so the render right after a date or filter change still reported the previous range as loaded and let an export read its rows. Stamp the completed range on the hook and compare it during render instead, the way the tiles already do. Also stop the failure banner claiming a page loaded when the first request is what failed, which left it reading 1/1. --- .../components/EntityUsage/EntityUsage.tsx | 4 +- .../_components/components/UsagePageView.tsx | 5 +- .../hooks/usePaginatedDailyActivity.test.ts | 95 +++++++++++++++++++ .../hooks/usePaginatedDailyActivity.ts | 16 +++- .../EntityUsageExport/UsageExportHeader.tsx | 1 - .../exportBlockedReason.test.ts | 17 ++-- .../EntityUsageExport/exportBlockedReason.ts | 14 +-- .../shared/PaginationStatusAlerts.test.tsx | 17 +++- .../shared/PaginationStatusAlerts.tsx | 10 +- 9 files changed, 144 insertions(+), 35 deletions(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.tsx b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.tsx index debc5602277..6687bd4df03 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/EntityUsage/EntityUsage.tsx @@ -146,11 +146,11 @@ const EntityUsage: React.FC = ({ const { data: spendDataRaw, - loading, isFetchingMore, progress, cancelled, failed, + coversRange, cancel, } = usePaginatedDailyActivity({ fetchFn, @@ -664,7 +664,7 @@ const EntityUsage: React.FC = ({ { key: "endpoints", label: "Endpoint Activity", content: }, ]; - const spendFetchState = { loading, isFetchingMore, cancelled, failed }; + const spendFetchState = { coversRange, cancelled, failed }; return (
diff --git a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.tsx b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.tsx index 31c9288c911..691c5dc839a 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.tsx @@ -250,9 +250,10 @@ const UsagePage: React.FC = ({ teams, organizations }) => { const loading = aggregatedLoading || paginatedResult.loading; + // Read through the same range stamp as the tiles, so the export is blocked from the first + // render of a new range rather than from whenever the fetch effect gets around to running. const spendFetchState = { - loading, - isFetchingMore: paginatedResult.isFetchingMore, + coversRange: activeAggregated !== null || paginatedResult.coversRange, cancelled: paginatedResult.cancelled, failed: paginatedResult.failed, }; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/hooks/usePaginatedDailyActivity.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/hooks/usePaginatedDailyActivity.test.ts index 5257e5fc8a6..aa6cf64483a 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/hooks/usePaginatedDailyActivity.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/hooks/usePaginatedDailyActivity.test.ts @@ -180,6 +180,20 @@ describe("usePaginatedDailyActivity failure reporting", () => { consoleError.mockRestore(); }); + it("reports no pages loaded when the very first request is what failed", async () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const fetchFn = vi.fn(() => Promise.reject(new Error("page 1 never came back"))); + + const { result } = renderHook(() => + usePaginatedDailyActivity({ fetchFn, args: ["tok", start, end, null], enabled: true }), + ); + + await waitFor(() => expect(result.current.failed).toBe(true), { timeout: 5000 }); + + expect(result.current.progress).toEqual({ currentPage: 0, totalPages: 0 }); + consoleError.mockRestore(); + }); + it("stays unfailed when every page arrives", async () => { const pages = [ firstPage, @@ -222,3 +236,84 @@ describe("usePaginatedDailyActivity failure reporting", () => { consoleError.mockRestore(); }); }); + +describe("usePaginatedDailyActivity range coverage", () => { + const start = new Date("2026-08-10"); + const end = new Date("2026-08-17"); + const singlePage = { results: [dayOf("2026-08-16", 2)], metadata: { total_pages: 1, page: 1, total_spend: 2 } }; + + it("does not cover the range while the hook is disabled", () => { + const fetchFn = vi.fn(() => Promise.resolve(singlePage)); + + const { result } = renderHook(() => + usePaginatedDailyActivity({ fetchFn, args: ["tok", start, end, null], enabled: false }), + ); + + expect(result.current.coversRange).toBe(false); + expect(fetchFn).not.toHaveBeenCalled(); + }); + + it("covers the range only once every page of it has landed", async () => { + const pages = [ + { results: [dayOf("2026-08-16", 2)], metadata: { total_pages: 2, page: 1, total_spend: 2 } }, + { results: [dayOf("2026-08-15", 1)], metadata: { total_pages: 2, page: 2, total_spend: 1 } }, + ]; + const fetchFn = vi.fn((_token: string, _start: Date, _end: Date, page: number) => Promise.resolve(pages[page - 1])); + + const { result } = renderHook(() => + usePaginatedDailyActivity({ fetchFn, args: ["tok", start, end, null], enabled: true }), + ); + + expect(result.current.coversRange).toBe(false); + await waitFor(() => expect(result.current.coversRange).toBe(true), { timeout: 5000 }); + }); + + it("never reports a range as covered while the data on screen is empty", async () => { + // Disabling the hook empties the data. Re-enabling it asks for the same args the last + // completed fetch used, so coverage that survives the disable would vouch for nothing. + const seen: Array<{ coversRange: boolean; rows: number }> = []; + const fetchFn = vi.fn(() => Promise.resolve(singlePage)); + + const { result, rerender } = renderHook( + ({ enabled }: { enabled: boolean }) => { + const activity = usePaginatedDailyActivity({ fetchFn, args: ["tok", start, end, null], enabled }); + seen.push({ coversRange: activity.coversRange, rows: activity.data.results.length }); + return activity; + }, + { initialProps: { enabled: true } }, + ); + + await waitFor(() => expect(result.current.coversRange).toBe(true), { timeout: 5000 }); + + rerender({ enabled: false }); + rerender({ enabled: true }); + + await waitFor(() => expect(result.current.coversRange).toBe(true), { timeout: 5000 }); + expect(seen.filter((render) => render.coversRange && render.rows === 0)).toEqual([]); + }); + + it("stops covering the range on the very render the args change, not once an effect catches up", async () => { + // The render after a filter change still holds the previous filter's rows, so resetting + // coverage inside the fetch effect would leave a paint where the export reads them as the + // new range. That paint is the whole thing the gate exists to stop. + const seen: Array<{ filter: string; coversRange: boolean }> = []; + const fetchFn = vi.fn(() => Promise.resolve(singlePage)); + + const { result, rerender } = renderHook( + ({ filter }: { filter: string }) => { + const activity = usePaginatedDailyActivity({ fetchFn, args: ["tok", start, end, filter], enabled: true }); + seen.push({ filter, coversRange: activity.coversRange }); + return activity; + }, + { initialProps: { filter: "team-a" } }, + ); + + await waitFor(() => expect(result.current.coversRange).toBe(true), { timeout: 5000 }); + + rerender({ filter: "team-b" }); + + const rendersForNewFilter = seen.filter((render) => render.filter === "team-b"); + expect(rendersForNewFilter.length).toBeGreaterThan(0); + expect(rendersForNewFilter.map((render) => render.coversRange)).not.toContain(true); + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/hooks/usePaginatedDailyActivity.ts b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/hooks/usePaginatedDailyActivity.ts index aef0021cc3d..0c1d79cb112 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/hooks/usePaginatedDailyActivity.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/hooks/usePaginatedDailyActivity.ts @@ -61,8 +61,8 @@ interface UsePaginatedDailyActivityReturn { isFetchingMore: boolean; progress: PaginationProgress; cancelled: boolean; - /** A page request threw, so `data` covers only part of the requested range. */ failed: boolean; + coversRange: boolean; cancel: () => void; } @@ -203,6 +203,7 @@ export function usePaginatedDailyActivity({ }); const [cancelled, setCancelled] = useState(false); const [failed, setFailed] = useState(false); + const [completedKey, setCompletedKey] = useState(null); const fetchIdRef = useRef(0); const cancelledRef = useRef(false); @@ -216,6 +217,11 @@ export function usePaginatedDailyActivity({ // Stable serialised key so the effect only re-runs when the arg *values* change. const argsKey = JSON.stringify(args); + // Stamped like the data itself and compared during render, so the render that follows an arg + // change already reports the new range as uncovered. Clearing it inside the fetch effect would + // be one render too late, leaving a paint where an export reads the previous range's rows. + const coversRange = enabled && completedKey === argsKey; + const cancel = useCallback(() => { cancelledRef.current = true; setCancelled(true); @@ -234,6 +240,7 @@ export function usePaginatedDailyActivity({ setProgress({ currentPage: 0, totalPages: 0 }); setCancelled(false); setFailed(false); + setCompletedKey(null); return; } @@ -257,7 +264,7 @@ export function usePaginatedDailyActivity({ const currentArgs = argsRef.current; setLoading(true); setIsFetchingMore(false); - setProgress({ currentPage: 1, totalPages: 1 }); + setProgress({ currentPage: 0, totalPages: 0 }); if (aggregatedFetchFn) { try { @@ -266,6 +273,7 @@ export function usePaginatedDailyActivity({ setData(aggregated); setProgress({ currentPage: 1, totalPages: 1 }); setLoading(false); + setCompletedKey(argsKey); return; } catch (error) { if (isStale()) return; @@ -288,6 +296,7 @@ export function usePaginatedDailyActivity({ if (totalPages <= 1) { setLoading(false); + setCompletedKey(argsKey); return; } @@ -333,6 +342,7 @@ export function usePaginatedDailyActivity({ } setIsFetchingMore(false); + setCompletedKey(argsKey); } catch (error) { if (!isStale()) { console.error("Error fetching daily activity:", error); @@ -356,5 +366,5 @@ export function usePaginatedDailyActivity({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [enabled, fetchFn, aggregatedFetchFn, argsKey]); - return { data, loading, isFetchingMore, progress, cancelled, failed, cancel }; + return { data, loading, isFetchingMore, progress, cancelled, failed, coversRange, cancel }; } diff --git a/ui/litellm-dashboard/src/components/EntityUsageExport/UsageExportHeader.tsx b/ui/litellm-dashboard/src/components/EntityUsageExport/UsageExportHeader.tsx index 3c4c7f6f4ee..388a211d9bb 100644 --- a/ui/litellm-dashboard/src/components/EntityUsageExport/UsageExportHeader.tsx +++ b/ui/litellm-dashboard/src/components/EntityUsageExport/UsageExportHeader.tsx @@ -34,7 +34,6 @@ interface UsageExportHeaderProps { customTitle?: string; compactLayout?: boolean; teams?: Team[]; - /** Set to block the export and explain why; see getExportBlockedReason. */ exportBlockedReason?: string; } diff --git a/ui/litellm-dashboard/src/components/EntityUsageExport/exportBlockedReason.test.ts b/ui/litellm-dashboard/src/components/EntityUsageExport/exportBlockedReason.test.ts index 0b32aef52fa..e39b01a5dea 100644 --- a/ui/litellm-dashboard/src/components/EntityUsageExport/exportBlockedReason.test.ts +++ b/ui/litellm-dashboard/src/components/EntityUsageExport/exportBlockedReason.test.ts @@ -3,35 +3,30 @@ import { describe, expect, it } from "vitest"; import { getExportBlockedReason, type UsageFetchState } from "./exportBlockedReason"; const state = (overrides: Partial = {}): UsageFetchState => ({ - loading: false, - isFetchingMore: false, + coversRange: true, cancelled: false, failed: false, ...overrides, }); describe("getExportBlockedReason", () => { - it("lets the export through once the range has fully loaded", () => { + it("lets the export through once the data on screen covers the range", () => { expect(getExportBlockedReason(state())).toBeUndefined(); }); - it("blocks the first load, before any page has arrived", () => { - expect(getExportBlockedReason(state({ loading: true }))).toMatch(/still loading/i); - }); - - it("blocks while later pages are still arriving, which is when a CSV silently under-reports", () => { - expect(getExportBlockedReason(state({ isFetchingMore: true }))).toMatch(/still loading/i); + it("blocks whenever the data on screen does not cover the range, which is when a CSV silently under-reports", () => { + expect(getExportBlockedReason(state({ coversRange: false }))).toMatch(/still loading/i); }); it("blocks after a stopped fetch and says a reload is what fixes it", () => { - const reason = getExportBlockedReason(state({ cancelled: true })); + const reason = getExportBlockedReason(state({ coversRange: false, cancelled: true })); expect(reason).toMatch(/stopped/i); expect(reason).toMatch(/reload/i); }); it("blocks after a failed page and names the failure rather than the stop", () => { - const reason = getExportBlockedReason(state({ failed: true, cancelled: true })); + const reason = getExportBlockedReason(state({ coversRange: false, failed: true, cancelled: true })); expect(reason).toMatch(/failed to load/i); expect(reason).not.toMatch(/stopped/i); diff --git a/ui/litellm-dashboard/src/components/EntityUsageExport/exportBlockedReason.ts b/ui/litellm-dashboard/src/components/EntityUsageExport/exportBlockedReason.ts index ebe67f1ccfe..71408ba8f3f 100644 --- a/ui/litellm-dashboard/src/components/EntityUsageExport/exportBlockedReason.ts +++ b/ui/litellm-dashboard/src/components/EntityUsageExport/exportBlockedReason.ts @@ -1,21 +1,13 @@ export interface UsageFetchState { - loading: boolean; - isFetchingMore: boolean; + coversRange: boolean; cancelled: boolean; failed: boolean; } -/** Why exporting what is on screen would under-report, or undefined once it covers the whole range. */ -export const getExportBlockedReason = ({ - loading, - isFetchingMore, - cancelled, - failed, -}: UsageFetchState): string | undefined => { +export const getExportBlockedReason = ({ coversRange, cancelled, failed }: UsageFetchState): string | undefined => { if (failed) return "Some spend data failed to load, so an export would under-report. Reload the page to try again."; if (cancelled) return "Loading was stopped before the whole range arrived, so an export would under-report. Reload the page to load it all."; - if (loading || isFetchingMore) - return "Spend data is still loading, so an export would under-report. Wait for it to finish."; + if (!coversRange) return "Spend data is still loading, so an export would under-report. Wait for it to finish."; return undefined; }; diff --git a/ui/litellm-dashboard/src/components/shared/PaginationStatusAlerts.test.tsx b/ui/litellm-dashboard/src/components/shared/PaginationStatusAlerts.test.tsx index 1eefc43ecb4..93d464b6615 100644 --- a/ui/litellm-dashboard/src/components/shared/PaginationStatusAlerts.test.tsx +++ b/ui/litellm-dashboard/src/components/shared/PaginationStatusAlerts.test.tsx @@ -45,10 +45,25 @@ describe("PaginationStatusAlerts", () => { ); expect( - screen.getByText(/Fetching spend data failed, so the totals below cover only part of the range \(7\/42 pages/), + screen.getByText(/Fetching spend data failed, so the totals below cover only 7 of 42 pages of the range/), ).toBeInTheDocument(); }); + it("does not claim a page loaded when the very first request is what failed", () => { + render( + , + ); + + expect(screen.getByText(/failed before any of it arrived/)).toBeInTheDocument(); + expect(screen.queryByText(/pages of the range/)).not.toBeInTheDocument(); + }); + it("shows only the failure when a stopped fetch also failed", () => { render( + progress.currentPage === 0 + ? `Fetching ${subject} failed before any of it arrived, so the totals below are empty rather than final. Reload the page to try again.` + : `Fetching ${subject} failed, so the totals below cover only ${progress.currentPage} of ${progress.totalPages} pages of the range. Reload the page to try again.`; + const PaginationStatusAlerts = ({ isFetchingMore, cancelled, @@ -42,10 +47,7 @@ const PaginationStatusAlerts = ({ )} {failed && ( - - Fetching {subject} failed, so the totals below cover only part of the range ({progress.currentPage}/ - {progress.totalPages} pages loaded). Reload the page to try again. - + {failureMessage(subject, progress)} )} {cancelled && !failed && (