From e9e30dffb68264e497d846763853e4f1c96939e7 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Wed, 8 Jul 2026 13:47:53 -0700 Subject: [PATCH] refactor(ui): reskin shared DataTable from tremor onto shadcn table primitives (#32209) * test(ui): characterize DataTable behavior before shadcn reskin Pins the shared view_logs DataTable contract with library-agnostic queries ahead of the tremor-to-shadcn table migration: loading and empty states, TanStack column defs with custom cell renderers, onRowClick payload, both expansion render paths (colspan sub-component and sibling child rows), the getRowCanExpand gate, and client-side sorting on and off. These must pass unchanged after the reskin. * refactor(ui): reskin shared DataTable from tremor onto shadcn table primitives Swaps the view_logs DataTable's presentational layer from @tremor/react to the in-repo components/ui/table primitives and hardens the seam that every later table migration copies: - getRowId is injected instead of hardcoded to request_id through an any cast; identity defaults to the row index and the logs page now passes request_id explicitly, keeping expansion state attached to the right row across refetch reorders - one expansion render path: renderChildRows had zero consumers and is removed; renderSubComponent (colspan cell) is the single path - the four consumers passing dead no-op renderSubComponent and getRowCanExpand boilerplate drop it - loading and empty defaults become generic (Loading... / No results) instead of log-specific The characterization tests from the previous commit pass unchanged except the dead child-rows path test, replaced by a reorder-stability test for injected getRowId plus coverage of the new generic defaults. First tremor removal of the tables track; view_logs/table.tsx no longer imports @tremor/react. * test(ui): assert child rows hidden before expansion in DataTable test * fix(ui): suppress row hover on DataTable placeholder rows * feat(ui): polish DataTable with skeleton loading, header band, and numeric column alignment * feat(ui): shape DataTable skeletons per column and keep stale rows during refetch * revert(ui): drop DataTable skeleton loading, restore text loading row * fix(ui): clip DataTable to its rounded wrapper and right-align Duration/TTFT values --- ui/litellm-dashboard/eslint-metrics.json | 2 +- ui/litellm-dashboard/eslint-suppressions.json | 5 -- .../components/EntityUsage/TopKeyView.tsx | 9 +-- .../components/EntityUsage/TopModelView.tsx | 12 ++- .../components/mcp_tools/MCPToolsetsTab.tsx | 2 - .../src/components/pass_through_settings.tsx | 2 - .../src/components/view_logs/columns.tsx | 10 ++- .../src/components/view_logs/index.tsx | 1 + .../src/components/view_logs/table.test.tsx | 81 ++++++++++++++++--- .../src/components/view_logs/table.tsx | 78 +++++++++--------- 10 files changed, 124 insertions(+), 78 deletions(-) diff --git a/ui/litellm-dashboard/eslint-metrics.json b/ui/litellm-dashboard/eslint-metrics.json index 219cb0580e7..51cef1169f9 100644 --- a/ui/litellm-dashboard/eslint-metrics.json +++ b/ui/litellm-dashboard/eslint-metrics.json @@ -1,5 +1,5 @@ { - "@typescript-eslint/no-explicit-any": 1990, + "@typescript-eslint/no-explicit-any": 1988, "complexity": 128, "max-depth": 59, "no-console": 15 diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 1c8f92b720f..67b19471aaf 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -2077,11 +2077,6 @@ "count": 1 } }, - "src/components/view_logs/table.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "src/components/view_user_spend.tsx": { "react-hooks/set-state-in-effect": { "count": 2 diff --git a/ui/litellm-dashboard/src/components/UsagePage/components/EntityUsage/TopKeyView.tsx b/ui/litellm-dashboard/src/components/UsagePage/components/EntityUsage/TopKeyView.tsx index d0748583c30..40bc41b3e8c 100644 --- a/ui/litellm-dashboard/src/components/UsagePage/components/EntityUsage/TopKeyView.tsx +++ b/ui/litellm-dashboard/src/components/UsagePage/components/EntityUsage/TopKeyView.tsx @@ -164,6 +164,7 @@ const TopKeyView: React.FC = ({ topKeys, teams, showTags = fals const spendColumn = { header: "Spend (USD)", accessorKey: "spend", + meta: { numeric: true }, cell: (info: any) => { const value = info.getValue(); return value > 0 && value < 0.01 ? "<$0.01" : `$${formatNumberWithCommas(value, 2)}`; @@ -247,13 +248,7 @@ const TopKeyView: React.FC = ({ topKeys, teams, showTags = fals ) : (
- <>} - getRowCanExpand={() => false} - isLoading={false} - /> +
)} diff --git a/ui/litellm-dashboard/src/components/UsagePage/components/EntityUsage/TopModelView.tsx b/ui/litellm-dashboard/src/components/UsagePage/components/EntityUsage/TopModelView.tsx index c69ba42f182..7562ef06a03 100644 --- a/ui/litellm-dashboard/src/components/UsagePage/components/EntityUsage/TopModelView.tsx +++ b/ui/litellm-dashboard/src/components/UsagePage/components/EntityUsage/TopModelView.tsx @@ -30,6 +30,7 @@ export default function TopModelView({ topModels, topModelsLimit, setTopModelsLi { header: "Spend (USD)", accessorKey: "spend", + meta: { numeric: true }, cell: (info: any) => { const value = info.getValue(); return `$${formatNumberWithCommas(value, 2)}`; @@ -38,16 +39,19 @@ export default function TopModelView({ topModels, topModelsLimit, setTopModelsLi { header: "Successful", accessorKey: "successful_requests", + meta: { numeric: true }, cell: (info: any) => {info.getValue()?.toLocaleString() || 0}, }, { header: "Failed", accessorKey: "failed_requests", + meta: { numeric: true }, cell: (info: any) => {info.getValue()?.toLocaleString() || 0}, }, { header: "Tokens", accessorKey: "tokens", + meta: { numeric: true }, cell: (info: any) => info.getValue()?.toLocaleString() || 0, }, ]; @@ -99,13 +103,7 @@ export default function TopModelView({ topModels, topModelsLimit, setTopModelsLi ) : (
- <>} - getRowCanExpand={() => false} - isLoading={false} - /> +
)} diff --git a/ui/litellm-dashboard/src/components/mcp_tools/MCPToolsetsTab.tsx b/ui/litellm-dashboard/src/components/mcp_tools/MCPToolsetsTab.tsx index 0198b830229..546df9ebc4b 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/MCPToolsetsTab.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/MCPToolsetsTab.tsx @@ -509,8 +509,6 @@ export function MCPToolsetsTab({ accessToken, userRole }: MCPToolsetsTabProps) {
} - getRowCanExpand={() => false} isLoading={isLoading} noDataMessage="No toolsets yet. Click 'New Toolset' to create one." loadingMessage="Loading toolsets..." diff --git a/ui/litellm-dashboard/src/components/pass_through_settings.tsx b/ui/litellm-dashboard/src/components/pass_through_settings.tsx index 0fdd9c632bf..63fe0f92961 100644 --- a/ui/litellm-dashboard/src/components/pass_through_settings.tsx +++ b/ui/litellm-dashboard/src/components/pass_through_settings.tsx @@ -263,8 +263,6 @@ const PassThroughSettings: React.FC = ({
} - getRowCanExpand={() => false} isLoading={false} noDataMessage="No pass-through endpoints configured" /> diff --git a/ui/litellm-dashboard/src/components/view_logs/columns.tsx b/ui/litellm-dashboard/src/components/view_logs/columns.tsx index 0508c562df8..7452992ed59 100644 --- a/ui/litellm-dashboard/src/components/view_logs/columns.tsx +++ b/ui/litellm-dashboard/src/components/view_logs/columns.tsx @@ -231,13 +231,14 @@ export const createColumns = (sortProps?: LogsSortProps): ColumnDef[] : "Cost", accessorKey: "spend", size: 110, + meta: { numeric: true }, cell: (info: any) => { const row = info.row.original; const mcpCount = row.mcp_tool_call_count || 0; const mcpSpend = row.mcp_tool_call_spend || 0; return ( -
+
{getSpendString(info.getValue() || 0)} @@ -263,13 +264,14 @@ export const createColumns = (sortProps?: LogsSortProps): ColumnDef[] ) : "Duration (s)", accessorKey: "request_duration_ms", + meta: { numeric: true }, cell: (info: any) => { const ms = info.getValue(); if (ms == null) return -; const seconds = (ms / 1000).toFixed(2); return ( - {seconds} + {seconds} ); }, @@ -287,6 +289,7 @@ export const createColumns = (sortProps?: LogsSortProps): ColumnDef[] ) : "TTFT (s)", accessorKey: "completionStartTime", + meta: { numeric: true }, cell: (info: any) => { const row = info.row.original; const completionStartTime = info.getValue(); @@ -298,7 +301,7 @@ export const createColumns = (sortProps?: LogsSortProps): ColumnDef[] const ttftSeconds = (ttftMs / 1000).toFixed(2); return ( - {ttftSeconds} + {ttftSeconds} ); }, @@ -395,6 +398,7 @@ export const createColumns = (sortProps?: LogsSortProps): ColumnDef[] : "Tokens", accessorKey: "total_tokens", size: 140, + meta: { numeric: true }, cell: (info: any) => { const row = info.row.original; return ( diff --git a/ui/litellm-dashboard/src/components/view_logs/index.tsx b/ui/litellm-dashboard/src/components/view_logs/index.tsx index 265e331e6a9..ee08712e56b 100644 --- a/ui/litellm-dashboard/src/components/view_logs/index.tsx +++ b/ui/litellm-dashboard/src/components/view_logs/index.tsx @@ -287,6 +287,7 @@ export default function SpendLogsTable({ accessToken, token, userRole, userID, p row.request_id} onRowClick={handleRowClick} isLoading={isLogsLoading} /> diff --git a/ui/litellm-dashboard/src/components/view_logs/table.test.tsx b/ui/litellm-dashboard/src/components/view_logs/table.test.tsx index 7299d280769..9a8469cfeba 100644 --- a/ui/litellm-dashboard/src/components/view_logs/table.test.tsx +++ b/ui/litellm-dashboard/src/components/view_logs/table.test.tsx @@ -74,6 +74,35 @@ describe("DataTable states", () => { expect(screen.getByText("Nothing here")).toBeInTheDocument(); }); + it("falls back to generic loading and empty defaults", () => { + const { rerender } = render(); + expect(screen.getByText("Loading...")).toBeInTheDocument(); + + rerender(); + expect(screen.getByText("No results")).toBeInTheDocument(); + }); + + it("suppresses the primitive's row hover on loading, empty, and expansion placeholder rows", async () => { + const user = userEvent.setup(); + const { rerender } = render(); + expect(screen.getByText("Loading...").closest("tr")).toHaveClass("hover:bg-transparent"); + + rerender(); + expect(screen.getByText("No results").closest("tr")).toHaveClass("hover:bg-transparent"); + + rerender( + true} + renderSubComponent={({ row }) =>
details for {row.original.request_id}
} + />, + ); + await user.click(screen.getByRole("button", { name: "expand r1" })); + expect(screen.getByText("details for r1").closest("tr")).toHaveClass("hover:bg-transparent"); + expect(screen.getByText("alpha").closest("tr")).not.toHaveClass("hover:bg-transparent"); + }); + it("renders row data through plain TanStack column defs, including custom cell renderers", () => { const columns: ColumnDef[] = [ { header: "A", accessorKey: "a" }, @@ -84,6 +113,29 @@ describe("DataTable states", () => { expect(screen.getByText("alpha")).toBeInTheDocument(); expect(screen.getByText("custom:beta")).toBeInTheDocument(); }); + + it("clips the table to the rounded wrapper so the header band cannot bleed past the corners", () => { + const { container } = render(); + + const wrapper = container.firstElementChild; + expect(wrapper).toHaveClass("rounded-lg", "overflow-hidden"); + }); + + it("right-aligns headers and cells with tabular figures for numeric meta columns", () => { + const columns: ColumnDef[] = [ + { header: "A", accessorKey: "a" }, + { header: "B", accessorKey: "b", meta: { numeric: true } }, + ]; + render(); + + const headers = screen.getAllByRole("columnheader"); + expect(headers[1].querySelector("div")).toHaveClass("justify-end"); + expect(headers[0].querySelector("div")).not.toHaveClass("justify-end"); + + const cells = screen.getAllByRole("cell"); + expect(cells[1]).toHaveClass("text-right", "tabular-nums"); + expect(cells[0]).not.toHaveClass("text-right"); + }); }); describe("DataTable row interaction", () => { @@ -129,28 +181,33 @@ describe("DataTable expansion", () => { expect(screen.queryByText("details for r1")).not.toBeInTheDocument(); }); - it("renders child rows as sibling table rows (child-rows path)", async () => { + it("keeps expansion attached to the same row through data reorders when getRowId is injected", async () => { const user = userEvent.setup(); - render( + const { rerender } = render( row.request_id} getRowCanExpand={() => true} - renderChildRows={({ row }) => ( - - child of {row.original.request_id} - - )} + renderSubComponent={({ row }) =>
details for {row.original.request_id}
} />, ); - expect(screen.queryByText("child of r2")).not.toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "expand r1" })); + expect(screen.getByText("details for r1")).toBeInTheDocument(); - await user.click(screen.getByRole("button", { name: "expand r2" })); + rerender( + row.request_id} + getRowCanExpand={() => true} + renderSubComponent={({ row }) =>
details for {row.original.request_id}
} + />, + ); - const childCell = screen.getByText("child of r2"); - expect(childCell.closest("tr")).not.toBeNull(); - expect(within(screen.getByRole("table")).getByText("child of r2")).toBeInTheDocument(); + expect(screen.getByText("details for r1")).toBeInTheDocument(); + expect(screen.queryByText("details for r2")).not.toBeInTheDocument(); }); it("does not expand rows when getRowCanExpand is missing even if a renderer is provided", () => { diff --git a/ui/litellm-dashboard/src/components/view_logs/table.tsx b/ui/litellm-dashboard/src/components/view_logs/table.tsx index 4510cc9a1f0..c96f34f9b93 100644 --- a/ui/litellm-dashboard/src/components/view_logs/table.tsx +++ b/ui/litellm-dashboard/src/components/view_logs/table.tsx @@ -1,6 +1,7 @@ import { Fragment, useState } from "react"; import { ColumnDef, + RowData, flexRender, getCoreRowModel, getExpandedRowModel, @@ -10,16 +11,21 @@ import { SortingState, } from "@tanstack/react-table"; -import { Table, TableHead, TableHeaderCell, TableBody, TableRow, TableCell } from "@tremor/react"; +import { Table, TableHeader, TableHead, TableBody, TableRow, TableCell } from "@/components/ui/table"; + +declare module "@tanstack/react-table" { + interface ColumnMeta { + numeric?: boolean; + } +} interface DataTableProps { data: TData[]; columns: ColumnDef[]; + getRowId?: (row: TData, index: number) => string; onRowClick?: (row: TData) => void; - /** Renders inside a single colspan cell (used by audit logs) */ + /** Renders inside a single colspan cell */ renderSubComponent?: (props: { row: Row }) => React.ReactElement; - /** Renders directly in tbody as sibling table rows (used by MCP children) */ - renderChildRows?: (props: { row: Row }) => React.ReactNode; getRowCanExpand?: (row: Row) => boolean; isLoading?: boolean; loadingMessage?: string; @@ -31,16 +37,16 @@ interface DataTableProps { export function DataTable({ data = [], columns, + getRowId, onRowClick, renderSubComponent, - renderChildRows, getRowCanExpand, isLoading = false, - loadingMessage = "🚅 Loading logs...", - noDataMessage = "No logs found", + loadingMessage = "Loading...", + noDataMessage = "No results", enableSorting = false, }: DataTableProps) { - const supportsExpansion = !!(renderSubComponent || renderChildRows) && !!getRowCanExpand; + const supportsExpansion = !!renderSubComponent && !!getRowCanExpand; const hasExplicitColumnSizes = columns.some((column) => column.size !== undefined); const [sorting, setSorting] = useState([]); @@ -55,58 +61,56 @@ export function DataTable({ enableSortingRemoval: false, }), ...(supportsExpansion && { getRowCanExpand }), - getRowId: (row: TData, index: number) => { - const _row: any = row as any; - return _row?.request_id ?? String(index); - }, + ...(getRowId && { getRowId }), getCoreRowModel: getCoreRowModel(), ...(enableSorting && { getSortedRowModel: getSortedRowModel() }), ...(supportsExpansion && { getExpandedRowModel: getExpandedRowModel() }), }); - const tableClassName = hasExplicitColumnSizes - ? "[&_td]:py-0.5 [&_th]:py-1 [&_table]:table-fixed" - : "[&_td]:py-0.5 [&_th]:py-1 table-fixed w-full box-border"; + const tableClassName = hasExplicitColumnSizes ? "table-fixed" : "table-fixed w-full box-border"; const tableStyle = hasExplicitColumnSizes ? { minWidth: table.getCenterTotalSize() } : { minWidth: "400px" }; return ( -
+
- + {table.getHeaderGroups().map((headerGroup) => ( - + {headerGroup.headers.map((header) => { const canSort = enableSorting && header.column.getCanSort(); const isSorted = header.column.getIsSorted(); + const numeric = header.column.columnDef.meta?.numeric; return ( - {header.isPlaceholder ? null : ( -
+
{flexRender(header.column.columnDef.header, header.getContext())} {canSort && ( - + {isSorted === "asc" ? "↑" : isSorted === "desc" ? "↓" : "⇅"} )}
)} - + ); })} ))} - + {isLoading ? ( - + -
+

{loadingMessage}

@@ -115,13 +119,15 @@ export function DataTable({ table.getRowModel().rows.map((row) => ( onRowClick?.(row.original)} > {row.getVisibleCells().map((cell) => ( {flexRender(cell.column.columnDef.cell, cell.getContext())} @@ -129,12 +135,8 @@ export function DataTable({ ))} - {/* Child rows rendered as real table rows (MCP children) */} - {supportsExpansion && row.getIsExpanded() && renderChildRows && renderChildRows({ row })} - - {/* Legacy sub-component in colspan cell (audit logs) */} - {supportsExpansion && row.getIsExpanded() && renderSubComponent && !renderChildRows && ( - + {supportsExpansion && row.getIsExpanded() && renderSubComponent && ( +
{renderSubComponent({ row })}
 @@ -143,11 +145,9 @@ export function DataTable({ )) ) : ( - - -
-

{noDataMessage}

-
+ + +

{noDataMessage}

 )}