From c19001ce2ec46e372c1777bfffbbca2de662f6a7 Mon Sep 17 00:00:00 2001 From: Trevor Mack Date: Wed, 16 Sep 2026 16:02:49 -0400 Subject: [PATCH] fix(ui): don't show loading/error budget as "No limit"; add wiring test Addresses review findings on the Max Budget tile fix: - P1: while the selected user's info is loading or errored, the tile fed `null` and falsely rendered "No limit". Thread the query's loading/error state through a `budgetLoading` prop so the tile shows a neutral placeholder until the budget is resolved. - Add a UsagePageView wiring regression test proving the tile receives the selected user's budget/duration (not the admin's) and is marked loading while unresolved, plus a ViewUserSpend loading-state test. - Remove explanatory source comments per the repo comment policy. --- .../(dashboard)/hooks/users/useUserInfo.ts | 9 +-- .../components/UsagePageView.test.tsx | 76 ++++++++++++++++--- .../_components/components/UsagePageView.tsx | 13 ++-- .../src/components/view_user_spend.test.tsx | 8 +- .../src/components/view_user_spend.tsx | 18 +++-- 5 files changed, 93 insertions(+), 31 deletions(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUserInfo.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUserInfo.ts index 8f3d1712f69..e17265e34a0 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUserInfo.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUserInfo.ts @@ -5,14 +5,7 @@ import { createQueryKeys } from "../common/queryKeysFactory"; const userKeys = createQueryKeys("users"); -/** - * Fetch a SPECIFIC user's info from /v2/user/info?user_id=. - * - * Companion to useCurrentUser (which self-looks-up the caller): pass the user - * you actually want to display. Disabled when userId is null so the caller can - * fall back to a global/unfiltered view without firing a request. Admins may - * query any user; the backend authorizes the lookup. - */ +/** Fetch a specific user's info from `/v2/user/info?user_id=`; disabled when userId is null. */ export const useUserInfo = (userId: string | null): UseQueryResult => { const { accessToken } = useAuthorized(); return useQuery({ diff --git a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.test.tsx index 7702488f5bf..888717bb847 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/usage/_components/components/UsagePageView.test.tsx @@ -4,6 +4,7 @@ import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; import useIsOrgAdmin from "@/app/(dashboard)/hooks/useIsOrgAdmin"; import { useCurrentUser } from "@/app/(dashboard)/hooks/users/useCurrentUser"; import { useInfiniteUsers } from "@/app/(dashboard)/hooks/users/useUsers"; +import { useUserInfo } from "@/app/(dashboard)/hooks/users/useUserInfo"; import { act, fireEvent, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; @@ -40,7 +41,20 @@ vi.mock("@/components/activity_metrics", () => ({ })); vi.mock("@/components/view_user_spend", () => ({ - default: () =>
View User Spend
, + default: ({ userMaxBudget, budgetDuration, budgetLoading }: any) => ( +
+ View User Spend +
+ ), +})); + +vi.mock("@/app/(dashboard)/hooks/users/useUserInfo", () => ({ + useUserInfo: vi.fn(), })); vi.mock("@/components/UsagePage/components/EntityUsage/TopKeyView", () => ({ @@ -166,6 +180,7 @@ describe("UsagePage", () => { const mockUseAuthorized = vi.mocked(useAuthorized); const mockUseCurrentUser = vi.mocked(useCurrentUser); const mockUseInfiniteUsers = vi.mocked(useInfiniteUsers); + const mockUseUserInfo = vi.mocked(useUserInfo); const mockSpendData = { results: [ @@ -411,6 +426,11 @@ describe("UsagePage", () => { isLoading: false, error: null, } as any); + mockUseUserInfo.mockReturnValue({ + data: undefined, + isLoading: false, + isError: false, + } as any); }); it("should render and fetch usage data on mount", async () => { @@ -1350,19 +1370,57 @@ describe("UsagePage", () => { }); }); - describe("tab navigation in global view", () => { - it("should render all expected tabs", async () => { + describe("selected-user budget wiring", () => { + const userSelectCombobox = (): HTMLElement => { + let node: HTMLElement | null = screen.getByText("Filter by user"); + while (node && !node.querySelector('[role="combobox"]')) { + node = node.parentElement; + } + return node!.querySelector('[role="combobox"]') as HTMLElement; + }; + + it("feeds the tile the selected user's budget and duration, not the admin's", async () => { + mockUseCurrentUser.mockReturnValue({ + data: { user_id: "user-123", max_budget: 3000 }, + isLoading: false, + error: null, + } as any); + mockUseUserInfo.mockReturnValue({ + data: { user_id: "user-001", max_budget: 600, budget_duration: "30d" }, + isLoading: false, + isError: false, + } as any); + renderWithProviders(); + await waitFor(() => expect(mockUserDailyActivityAggregatedCall).toHaveBeenCalled()); + + await userEvent.setup().click(userSelectCombobox()); + await userEvent.setup().click(screen.getByText("Alice (user-001)")); await waitFor(() => { - expect(mockUserDailyActivityAggregatedCall).toHaveBeenCalled(); + expect(screen.getByTestId("view-user-spend")).toHaveAttribute("data-max-budget", "600"); }); + expect(screen.getByTestId("view-user-spend")).toHaveAttribute("data-budget-duration", "30d"); + expect(mockUseUserInfo).toHaveBeenCalledWith("user-001"); + }); - expect(screen.getByText("Cost")).toBeInTheDocument(); - expect(screen.getByText("Model Activity")).toBeInTheDocument(); - expect(screen.getByText("Key Activity")).toBeInTheDocument(); - expect(screen.getByText("MCP Server Activity")).toBeInTheDocument(); - expect(screen.getByText("Endpoint Activity")).toBeInTheDocument(); + it("marks the tile loading (not unlimited) while the selected user's info is unresolved", async () => { + mockUseCurrentUser.mockReturnValue({ + data: { user_id: "user-123", max_budget: 3000 }, + isLoading: false, + error: null, + } as any); + mockUseUserInfo.mockReturnValue({ data: undefined, isLoading: true, isError: false } as any); + + renderWithProviders(); + await waitFor(() => expect(mockUserDailyActivityAggregatedCall).toHaveBeenCalled()); + + await userEvent.setup().click(userSelectCombobox()); + await userEvent.setup().click(screen.getByText("Alice (user-001)")); + + await waitFor(() => { + expect(screen.getByTestId("view-user-spend")).toHaveAttribute("data-budget-loading", "true"); + }); }); }); }); 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 bb5afc20459..137c13027e5 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 @@ -141,12 +141,12 @@ const UsagePage: React.FC = ({ teams, organizations }) => { // For non-admins or "my-usage" view, always pass their own user_id const effectiveUserId = usageView === "my-usage" || !isAdmin ? userID || null : selectedUserId; - // Budget shown in the header tile must reflect the SELECTED user, not the - // logged-in admin (currentUser). Fetch the effective user's own record so the - // Max Budget tile and its reset period track "Filter by user". Falls back to - // currentUser only while the per-user fetch is unresolved or when no user is - // selected (global view has no single budget to show). - const { data: selectedUserInfo } = useUserInfo(effectiveUserId); + const { + data: selectedUserInfo, + isLoading: isSelectedUserLoading, + isError: isSelectedUserError, + } = useUserInfo(effectiveUserId); + const isSelectedUserResolved = effectiveUserId === null || (!isSelectedUserLoading && !isSelectedUserError); const effectiveMaxBudget = effectiveUserId !== null ? selectedUserInfo?.max_budget ?? null : currentUser?.max_budget ?? null; const effectiveBudgetDuration = effectiveUserId !== null ? selectedUserInfo?.budget_duration ?? null : null; @@ -591,6 +591,7 @@ const UsagePage: React.FC = ({ teams, organizations }) => { selectedTeam={null} userMaxBudget={effectiveMaxBudget} budgetDuration={effectiveBudgetDuration} + budgetLoading={!isSelectedUserResolved} /> diff --git a/ui/litellm-dashboard/src/components/view_user_spend.test.tsx b/ui/litellm-dashboard/src/components/view_user_spend.test.tsx index 73b05e89580..42aff37bc60 100644 --- a/ui/litellm-dashboard/src/components/view_user_spend.test.tsx +++ b/ui/litellm-dashboard/src/components/view_user_spend.test.tsx @@ -3,7 +3,6 @@ import { render, screen } from "@testing-library/react"; import React from "react"; import ViewUserSpend from "./view_user_spend"; -// ViewUserSpend fetches available models in an effect; stub the network call. vi.mock("./networking", () => ({ modelAvailableCall: vi.fn().mockResolvedValue({ data: [] }), })); @@ -18,7 +17,6 @@ describe("ViewUserSpend — Max Budget tile", () => { it("shows a finite cap with its reset period when budgetDuration is set", () => { render(); expect(screen.getByText(/\$600\.0000 limit/)).toBeInTheDocument(); - // 30d maps to "monthly" via getBudgetDurationLabel expect(screen.getByText(/over monthly/)).toBeInTheDocument(); }); @@ -33,4 +31,10 @@ describe("ViewUserSpend — Max Budget tile", () => { expect(screen.getByText(/\$300\.0000 limit/)).toBeInTheDocument(); expect(screen.queryByText(/over/)).not.toBeInTheDocument(); }); + + it('does not render "No limit" while the budget is loading', () => { + render(); + expect(screen.queryByText("No limit")).not.toBeInTheDocument(); + expect(screen.getByText("—")).toBeInTheDocument(); + }); }); diff --git a/ui/litellm-dashboard/src/components/view_user_spend.tsx b/ui/litellm-dashboard/src/components/view_user_spend.tsx index 5773a7420bc..31581a2d093 100644 --- a/ui/litellm-dashboard/src/components/view_user_spend.tsx +++ b/ui/litellm-dashboard/src/components/view_user_spend.tsx @@ -10,15 +10,15 @@ interface ViewUserSpendProps { userSpend: number | null; userMaxBudget: number | null; selectedTeam: any | null; - // Optional reset period paired with userMaxBudget (e.g. "24h", "30d"), rendered - // as a human-readable window next to the cap. Omitted/null → no period shown. budgetDuration?: string | null; + budgetLoading?: boolean; } const ViewUserSpend: React.FC = ({ userSpend, userMaxBudget, selectedTeam, budgetDuration = null, + budgetLoading = false, }) => { const { accessToken, userRole, userId: userID } = useAuthorized(); let [spend, setSpend] = useState(userSpend !== null ? userSpend : 0.0); @@ -116,12 +116,18 @@ const ViewUserSpend: React.FC = ({ modelsToDisplay = userModels; } - const displayMaxBudget = maxBudget !== null ? `$${formatNumberWithCommas(Number(maxBudget), 4)} limit` : "No limit"; + let displayMaxBudget: string; + if (budgetLoading) { + displayMaxBudget = "—"; + } else if (maxBudget !== null) { + displayMaxBudget = `$${formatNumberWithCommas(Number(maxBudget), 4)} limit`; + } else { + displayMaxBudget = "No limit"; + } - // Show the reset window (e.g. "over monthly") only for a finite cap that has a - // paired duration; an unlimited budget or a cap with no duration shows nothing. const durationLabel = maxBudget !== null && budgetDuration ? getBudgetDurationLabel(budgetDuration) : null; - const budgetPeriodSuffix = durationLabel && durationLabel !== "Not set" ? ` over ${durationLabel}` : ""; + const budgetPeriodSuffix = + !budgetLoading && durationLabel && durationLabel !== "Not set" ? ` over ${durationLabel}` : ""; const roundedSpend = spend !== undefined ? formatNumberWithCommas(spend, 4) : null;