From a1eb204c7e9c307b5ef5857b4000381b9a36d3aa Mon Sep 17 00:00:00 2001 From: zhengxinliang Date: Mon, 5 Oct 2026 18:52:32 +0800 Subject: [PATCH] fix(logs): address key alias filter review feedback --- .../spend_management_endpoints.py | 2 +- .../test_spend_management_endpoints.py | 16 +++- ui/litellm-dashboard/eslint-suppressions.json | 84 ++++++++++++------- .../logs/request/RequestLogsFilters.tsx | 17 ++-- .../logs/request/RequestLogsPanel.test.tsx | 9 ++ .../logs/request/RequestLogsPanel.tsx | 44 ++++++---- 6 files changed, 113 insertions(+), 59 deletions(-) diff --git a/litellm/proxy/spend_tracking/spend_management_endpoints.py b/litellm/proxy/spend_tracking/spend_management_endpoints.py index b45f9b0c5fb..1cd93e3e9c2 100644 --- a/litellm/proxy/spend_tracking/spend_management_endpoints.py +++ b/litellm/proxy/spend_tracking/spend_management_endpoints.py @@ -2912,7 +2912,7 @@ async def ui_view_spend_logs( sql_conditions.append(f"metadata->>'user_api_key_alias' LIKE ${p}") sql_params.append(f"%{key_alias}%") p += 1 - if exclude_key_alias is not None: + if exclude_key_alias: sql_conditions.append(f"metadata->>'user_api_key_alias' IS DISTINCT FROM ${p}") sql_params.append(exclude_key_alias) p += 1 diff --git a/tests/unit/proxy/spend_tracking/test_spend_management_endpoints.py b/tests/unit/proxy/spend_tracking/test_spend_management_endpoints.py index bdd6f08f94d..f4c93432943 100644 --- a/tests/unit/proxy/spend_tracking/test_spend_management_endpoints.py +++ b/tests/unit/proxy/spend_tracking/test_spend_management_endpoints.py @@ -3623,7 +3623,9 @@ async def test_ui_view_spend_logs_excludes_exact_key_alias_before_pagination(cli monkeypatch.setattr( "litellm.proxy.proxy_server.prisma_client", - make_ui_spend_logs_mock_prisma(logs, filter_by_alias, query_observer=lambda sql, params: queries.append(sql)), + make_ui_spend_logs_mock_prisma( + logs, filter_by_alias, query_observer=lambda sql, params: queries.append((sql, params)) + ), ) app.dependency_overrides[ps.user_api_key_auth] = lambda: UserAPIKeyAuth( user_role=LitellmUserRoles.PROXY_ADMIN, user_id="admin_user" @@ -3651,7 +3653,12 @@ async def test_ui_view_spend_logs_excludes_exact_key_alias_before_pagination(cli ["req1", "req2"], ["req3"], ] - assert all("user_api_key_alias' IS DISTINCT FROM" in query for query in queries if "WHERE" in query) + for sql, params in queries: + if "WHERE" not in sql: + continue + alias_placeholder = re.search(r"user_api_key_alias' IS DISTINCT FROM \$(\d+)", sql) + assert alias_placeholder is not None + assert params[int(alias_placeholder.group(1)) - 1] == "noisy" finally: app.dependency_overrides.pop(ps.user_api_key_auth, None) @@ -7199,8 +7206,9 @@ async def test_ui_view_spend_logs_grouped_exclusion_reaches_page_count_and_repre queries = [call.args for call in mock_prisma.db.query_raw.await_args_list] assert len(queries) >= 3 for sql, *params in queries[:3]: - assert "metadata->>'user_api_key_alias' IS DISTINCT FROM $3" in sql - assert params[2] == "noisy" + alias_placeholder = re.search(r"user_api_key_alias' IS DISTINCT FROM \$(\d+)", sql) + assert alias_placeholder is not None + assert params[int(alias_placeholder.group(1)) - 1] == "noisy" finally: app.dependency_overrides.pop(ps.user_api_key_auth, None) diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 280d3cc4115..7e4e59e0c9f 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -1706,37 +1706,6 @@ "count": 1 } }, - "src/components/logs/detail/LogDetailsDrawer.tsx": { - "no-nested-ternary": { - "count": 2 - }, - "react-hooks/set-state-in-effect": { - "count": 2 - } - }, - "src/components/logs/detail/sections/EvalViewer/EvalViewer.tsx": { - "no-nested-ternary": { - "count": 1 - } - }, - "src/components/logs/detail/sections/GuardrailViewer/CompliancePanel.tsx": { - "no-nested-ternary": { - "count": 2 - }, - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, - "src/components/logs/detail/sections/GuardrailViewer/ContentFilterDetails.tsx": { - "no-nested-ternary": { - "count": 1 - } - }, - "src/components/logs/detail/sections/GuardrailViewer/GuardrailViewer.tsx": { - "no-nested-ternary": { - "count": 3 - } - }, "src/components/mcp_server_management/MCPToolPermissions.tsx": { "local/no-complex-jsx-arrow": { "count": 1 @@ -2297,6 +2266,57 @@ "count": 1 } }, + "src/components/logs/detail/sections/EvalViewer/EvalViewer.tsx": { + "no-nested-ternary": { + "count": 1 + } + }, + "src/components/logs/detail/sections/GuardrailViewer/CompliancePanel.tsx": { + "no-nested-ternary": { + "count": 2 + }, + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/logs/detail/sections/GuardrailViewer/ContentFilterDetails.tsx": { + "no-nested-ternary": { + "count": 1 + } + }, + "src/components/logs/detail/sections/GuardrailViewer/GuardrailViewer.tsx": { + "no-nested-ternary": { + "count": 3 + } + }, + "src/components/logs/detail/LogDetailsDrawer.tsx": { + "no-nested-ternary": { + "count": 2 + }, + "react-hooks/set-state-in-effect": { + "count": 2 + } + }, + "src/components/logs/detail/useKeyboardNavigation.ts": { + "react-hooks/immutability": { + "count": 2 + } + }, + "src/components/logs/types.ts": { + "local/filename-pascal-case": { + "count": 1 + } + }, + "src/components/logs/request/useLogFilterLogic.ts": { + "local/filename-pascal-case": { + "count": 1 + } + }, + "src/components/logs/request/timeRange.ts": { + "local/filename-pascal-case": { + "count": 1 + } + }, "src/components/view_model/model_name_display.tsx": { "local/filename-pascal-case": { "count": 1 @@ -2394,4 +2414,4 @@ "count": 1 } } -} \ No newline at end of file +} diff --git a/ui/litellm-dashboard/src/components/logs/request/RequestLogsFilters.tsx b/ui/litellm-dashboard/src/components/logs/request/RequestLogsFilters.tsx index d289736f72d..5822a8d0b5f 100644 --- a/ui/litellm-dashboard/src/components/logs/request/RequestLogsFilters.tsx +++ b/ui/litellm-dashboard/src/components/logs/request/RequestLogsFilters.tsx @@ -29,6 +29,7 @@ const KEY_ALIAS_OPERATORS = [ { value: "is", label: "is" }, { value: "is not", label: "is not" }, ] as const; +type KeyAliasOperator = (typeof KEY_ALIAS_OPERATORS)[number]["value"]; const STATUS_FILTER_ITEMS = [ { value: ALL_VALUE, label: "All Statuses" }, @@ -107,14 +108,14 @@ function KeyAliasFilterField({ teamId: string; }) { const [search, setSearch] = useState(""); - const [operator, setOperator] = useState<"is" | "is not">("is"); - const selectedOperator = (): "is" | "is not" => { - if (excludeValue !== "") return "is not"; - if (value !== "") return "is"; - return operator; - }; - const activeOperator = selectedOperator(); - const handleOperatorChange = (next: "is" | "is not" | null) => { + const [operator, setOperator] = useState("is"); + let activeOperator: KeyAliasOperator = operator; + if (excludeValue !== "") { + activeOperator = "is not"; + } else if (value !== "") { + activeOperator = "is"; + } + const handleOperatorChange = (next: KeyAliasOperator | null) => { if (next === null || next === activeOperator) return; const selectedAlias = value || excludeValue || undefined; setOperator(next); diff --git a/ui/litellm-dashboard/src/components/logs/request/RequestLogsPanel.test.tsx b/ui/litellm-dashboard/src/components/logs/request/RequestLogsPanel.test.tsx index 710a73d6588..16f6ce77866 100644 --- a/ui/litellm-dashboard/src/components/logs/request/RequestLogsPanel.test.tsx +++ b/ui/litellm-dashboard/src/components/logs/request/RequestLogsPanel.test.tsx @@ -526,6 +526,15 @@ describe("RequestLogsPanel", () => { await waitFor(() => expect(lastCall()?.params?.exclude_key_alias).toBeUndefined()); }); + it("treats conflicting Key Alias URL parameters as one mutually exclusive filter", async () => { + renderPanel("key_alias=quiet&exclude_key_alias=noisy"); + + await waitFor(() => expect(lastCall()?.params?.exclude_key_alias).toBe("noisy")); + expect(lastCall()?.params?.key_alias).toBeUndefined(); + expect(screen.getByTestId("filter-chip-exclude_key_alias")).toHaveTextContent("noisy"); + expect(screen.queryByTestId("filter-chip-key_alias")).not.toBeInTheDocument(); + }); + it("keeps an excluded Key Alias on the next page", async () => { vi.mocked(uiSpendLogsCall).mockResolvedValue({ data: Array.from({ length: 25 }, (_, index) => logEntry({ request_id: `req-${index}` })), diff --git a/ui/litellm-dashboard/src/components/logs/request/RequestLogsPanel.tsx b/ui/litellm-dashboard/src/components/logs/request/RequestLogsPanel.tsx index ac5f62b0f03..c25858f34c0 100644 --- a/ui/litellm-dashboard/src/components/logs/request/RequestLogsPanel.tsx +++ b/ui/litellm-dashboard/src/components/logs/request/RequestLogsPanel.tsx @@ -5,7 +5,7 @@ import { useQuery, type UseQueryOptions } from "@tanstack/react-query"; import type { ColumnFiltersState, OnChangeFn, PaginationState, SortingState } from "@tanstack/react-table"; import moment from "moment"; import { parseAsString, useQueryStates } from "nuqs"; -import { useCallback, useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { DEFAULT_PAGE_SIZE_OPTIONS } from "@/components/shared/DataTable"; import { Button } from "@/components/ui/button"; @@ -56,14 +56,19 @@ export default function RequestLogsPanel({ accessToken, token, userRole, userID, { key_alias: parseAsString, exclude_key_alias: parseAsString }, { history: "push" }, ); - const columnFilters = useMemo( - () => [ - ...localFilters, - ...(key_alias ? [{ id: LOG_FILTER_IDS.KEY_ALIAS, value: key_alias }] : []), - ...(exclude_key_alias ? [{ id: LOG_FILTER_IDS.EXCLUDE_KEY_ALIAS, value: exclude_key_alias }] : []), - ], - [localFilters, key_alias, exclude_key_alias], - ); + const columnFilters = useMemo(() => { + const aliasFilters: ColumnFiltersState = []; + if (exclude_key_alias) { + aliasFilters.push({ id: LOG_FILTER_IDS.EXCLUDE_KEY_ALIAS, value: exclude_key_alias }); + } else if (key_alias) { + aliasFilters.push({ id: LOG_FILTER_IDS.KEY_ALIAS, value: key_alias }); + } + return [...localFilters, ...aliasFilters]; + }, [localFilters, key_alias, exclude_key_alias]); + const columnFiltersRef = useRef(columnFilters); + useEffect(() => { + columnFiltersRef.current = columnFilters; + }, [columnFilters]); const [sessionCursors, setSessionCursors] = useState>({}); const [timeRange, setTimeRange] = useState(defaultLogsTimeRange); @@ -209,20 +214,31 @@ export default function RequestLogsPanel({ accessToken, token, userRole, userID, const handleColumnFiltersChange = useCallback>( (updaterOrValue) => { - const next = typeof updaterOrValue === "function" ? updaterOrValue(columnFilters) : updaterOrValue; + const next = typeof updaterOrValue === "function" ? updaterOrValue(columnFiltersRef.current) : updaterOrValue; + const nextKeyAlias = getFilterValue(next, LOG_FILTER_IDS.KEY_ALIAS); + const nextExcludeKeyAlias = getFilterValue(next, LOG_FILTER_IDS.EXCLUDE_KEY_ALIAS); + const normalizedNext = next.filter( + (filter) => filter.id !== LOG_FILTER_IDS.KEY_ALIAS && filter.id !== LOG_FILTER_IDS.EXCLUDE_KEY_ALIAS, + ); + if (nextExcludeKeyAlias !== undefined) { + normalizedNext.push({ id: LOG_FILTER_IDS.EXCLUDE_KEY_ALIAS, value: nextExcludeKeyAlias }); + } else if (nextKeyAlias !== undefined) { + normalizedNext.push({ id: LOG_FILTER_IDS.KEY_ALIAS, value: nextKeyAlias }); + } + columnFiltersRef.current = normalizedNext; setLocalFilters( - next.filter( + normalizedNext.filter( (filter) => filter.id !== LOG_FILTER_IDS.KEY_ALIAS && filter.id !== LOG_FILTER_IDS.EXCLUDE_KEY_ALIAS, ), ); void setAliasParams({ - key_alias: getFilterValue(next, LOG_FILTER_IDS.KEY_ALIAS) ?? null, - exclude_key_alias: getFilterValue(next, LOG_FILTER_IDS.EXCLUDE_KEY_ALIAS) ?? null, + key_alias: nextExcludeKeyAlias === undefined ? nextKeyAlias ?? null : null, + exclude_key_alias: nextExcludeKeyAlias ?? null, }); setSessionCursors({}); setPagination((previous) => ({ ...previous, pageIndex: 0 })); }, - [columnFilters, setAliasParams], + [setAliasParams], ); const resetToFirstPage = useCallback(() => {