From d78a016bc5730408c6c9fbe07b85343ebda41ab8 Mon Sep 17 00:00:00 2001 From: mubashir1osmani Date: Sat, 8 Aug 2026 12:01:44 -0700 Subject: [PATCH] fix(ui): green playground shadcn CI after staging merge Normalize MCP description nullability for MultiSelect, preserve model selection via functional setState, and update endpoint/vector-store selector tests for the shadcn combobox API --- .../components/chat_ui/ChatUI.test.tsx | 2 +- .../playground/components/chat_ui/ChatUI.tsx | 9 +- .../chat_ui/EndpointSelector.test.tsx | 11 +- .../VectorStoreSelector.test.tsx | 206 ++++++------------ 4 files changed, 82 insertions(+), 146 deletions(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/ChatUI.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/ChatUI.test.tsx index f9db7710593..7c2fc47bd63 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/ChatUI.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/ChatUI.test.tsx @@ -1,4 +1,4 @@ -import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; import ChatUI from "./ChatUI"; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/ChatUI.tsx b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/ChatUI.tsx index f6e1b9fbcf6..32f9ff3395b 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/ChatUI.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/ChatUI.tsx @@ -421,10 +421,9 @@ const ChatUI: React.FC = ({ setModelInfo(uniqueModels); - const hasSelection = uniqueModels.some((m) => m.model_group === selectedModel); - if (!uniqueModels.length || !hasSelection) { - setSelectedModel(undefined); - } + setSelectedModel((currentModel) => + uniqueModels.some((model) => model.model_group === currentModel) ? currentModel : undefined, + ); } catch (error) { if (cancelled) { return; @@ -633,7 +632,7 @@ const ChatUI: React.FC = ({ options.push({ value: server.server_id, label: server.alias || server.server_name || server.server_id, - description: server.description, + description: server.description ?? undefined, }); } return options; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/EndpointSelector.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/EndpointSelector.test.tsx index 85c120fb903..2a17c9f72ba 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/EndpointSelector.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/playground/components/chat_ui/EndpointSelector.test.tsx @@ -7,9 +7,9 @@ import { ENDPOINT_OPTIONS } from "./chatConstants"; describe("EndpointSelector", () => { Object.values(ENDPOINT_OPTIONS).forEach((endpointType) => { it(`should render the endpoint selector for ${endpointType.value}`, async () => { - const { getByText } = render( {}} />); + render( {}} />); await waitFor(() => { - expect(getByText(endpointType.label)).toBeInTheDocument(); + expect(screen.getByRole("combobox")).toHaveValue(endpointType.label); }); }); }); @@ -18,10 +18,9 @@ describe("EndpointSelector", () => { const user = userEvent.setup(); render( {}} />); - const combobox = screen.getByRole("combobox"); - await user.click(combobox); - - const input = await screen.findByRole("combobox"); + const input = screen.getByRole("combobox"); + await user.click(input); + await user.clear(input); await user.type(input, "audio"); expect(await screen.findByText("/v1/audio/speech")).toBeInTheDocument(); diff --git a/ui/litellm-dashboard/src/components/vector_store_management/VectorStoreSelector.test.tsx b/ui/litellm-dashboard/src/components/vector_store_management/VectorStoreSelector.test.tsx index 67476b5559d..07468abc582 100644 --- a/ui/litellm-dashboard/src/components/vector_store_management/VectorStoreSelector.test.tsx +++ b/ui/litellm-dashboard/src/components/vector_store_management/VectorStoreSelector.test.tsx @@ -3,78 +3,64 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { VectorStore } from "./types"; import VectorStoreSelector from "./VectorStoreSelector"; -// Mock dependencies const mockVectorStoreListCall = vi.fn(); vi.mock("../networking", () => ({ - vectorStoreListCall: (...args: any[]) => mockVectorStoreListCall(...args), + vectorStoreListCall: (...args: unknown[]) => mockVectorStoreListCall(...args), })); -// Mock antd Select component -vi.mock("antd", () => ({ - Select: vi.fn(), +vi.mock("@/components/shared/MultiSelect", () => ({ + MultiSelect: vi.fn(), })); -// Import the mocked Select -import { Select as MockedSelect } from "antd"; +import { MultiSelect as MockedMultiSelect } from "@/components/shared/MultiSelect"; -// Configure the mock to render a simple div with data attributes -(MockedSelect as any).mockImplementation((props: any) => { - const { - onChange, - value, - placeholder, - loading, - className, - disabled, - options, - mode, - showSearch, - optionFilterProp, - style, - } = props; +(MockedMultiSelect as unknown as ReturnType).mockImplementation( + (props: { + onValueChange?: (value: string[]) => void; + value?: string[]; + placeholder?: string; + loading?: boolean; + className?: string; + disabled?: boolean; + options?: Array<{ value: string; label: string; description?: string }>; + }) => { + const { onValueChange, value, placeholder, loading, className, disabled, options } = props; - return ( -
{ - // For testing purposes, allow simulating different selection behaviors - // The test can control this by setting data attributes on the element - const testSelection = e.target.getAttribute("data-test-selection"); - if (testSelection && onChange) { - onChange(JSON.parse(testSelection)); - } else if (onChange && options?.length > 0) { - // Default behavior: select first option - onChange([options[0].value]); - } - }} - > - {options?.map((opt: any) => ( -
- {opt.label} -
- ))} -
- ); -}); + return ( +
{ + const testSelection = (e.target as HTMLElement).getAttribute("data-test-selection"); + if (testSelection && onValueChange) { + onValueChange(JSON.parse(testSelection) as string[]); + } else if (onValueChange && options && options.length > 0) { + onValueChange([options[0].value]); + } + }} + > + {options?.map((opt) => ( +
+ {opt.label} +
+ ))} +
+ ); + }, +); -// Test helpers const mockOnChange = vi.fn(); const mockAccessToken = "test-token"; @@ -98,7 +84,6 @@ const mockVectorStores: VectorStore[] = [ { vector_store_id: "store-3", custom_llm_provider: "pg_vector", - // No vector_store_name to test fallback to vector_store_id vector_store_description: "Store without name", created_at: "2024-01-03T00:00:00Z", updated_at: "2024-01-03T00:00:00Z", @@ -110,7 +95,6 @@ const defaultProps = { accessToken: mockAccessToken, }; -// Helper functions const renderComponent = (props = {}) => { return render(); }; @@ -124,7 +108,7 @@ const waitForDataFetch = async () => { const getSelectElement = () => screen.getByTestId("vector-store-select"); const getOptionElements = () => - screen.getAllByTestId(/^vector-store-select/).filter((el) => el.hasAttribute("data-option-value")); + screen.queryAllByTestId(/^option-/).filter((el) => el.hasAttribute("data-option-value")); describe("VectorStoreSelector", () => { beforeEach(() => { @@ -142,56 +126,27 @@ describe("VectorStoreSelector", () => { it("should render with default placeholder", () => { renderComponent(); - const select = getSelectElement(); - expect(select).toHaveAttribute("data-placeholder", "Select vector stores"); + expect(getSelectElement()).toHaveAttribute("data-placeholder", "Select vector stores"); }); it("should render with custom placeholder", () => { renderComponent({ placeholder: "Choose stores" }); - const select = getSelectElement(); - expect(select).toHaveAttribute("data-placeholder", "Choose stores"); + expect(getSelectElement()).toHaveAttribute("data-placeholder", "Choose stores"); }); it("should apply custom className", () => { renderComponent({ className: "custom-class" }); - const select = getSelectElement(); - expect(select).toHaveClass("custom-class"); + expect(getSelectElement()).toHaveClass("custom-class"); }); it("should render with disabled state", () => { renderComponent({ disabled: true }); - const select = getSelectElement(); - expect(select).toHaveAttribute("data-disabled", "true"); + expect(getSelectElement()).toHaveAttribute("data-disabled", "true"); }); it("should render with enabled state by default", () => { renderComponent(); - const select = getSelectElement(); - expect(select).toHaveAttribute("data-disabled", "false"); - }); - - it("should render with multiple mode", () => { - renderComponent(); - const select = getSelectElement(); - expect(select).toHaveAttribute("data-mode", "multiple"); - }); - - it("should render with showSearch enabled", () => { - renderComponent(); - const select = getSelectElement(); - expect(select).toHaveAttribute("data-show-search", "true"); - }); - - it("should render with optionFilterProp set to label", () => { - renderComponent(); - const select = getSelectElement(); - expect(select).toHaveAttribute("data-option-filter-prop", "label"); - }); - - it("should render with full width style", () => { - renderComponent(); - const select = getSelectElement(); - expect(select).toHaveStyle({ width: "100%" }); + expect(getSelectElement()).toHaveAttribute("data-disabled", "false"); }); }); @@ -207,10 +162,10 @@ describe("VectorStoreSelector", () => { const { rerender } = render(); expect(mockVectorStoreListCall).not.toHaveBeenCalled(); - rerender(); + rerender(); expect(mockVectorStoreListCall).not.toHaveBeenCalled(); - rerender(); + rerender(); expect(mockVectorStoreListCall).not.toHaveBeenCalled(); }); @@ -228,7 +183,7 @@ describe("VectorStoreSelector", () => { }); it("should set loading state while fetching", async () => { - let resolvePromise: (value: any) => void; + let resolvePromise: (value: unknown) => void; const promise = new Promise((resolve) => { resolvePromise = resolve; }); @@ -247,8 +202,7 @@ describe("VectorStoreSelector", () => { it("should clear loading state after successful fetch", async () => { renderComponent(); await waitForDataFetch(); - const select = getSelectElement(); - expect(select).toHaveAttribute("data-loading", "false"); + expect(getSelectElement()).toHaveAttribute("data-loading", "false"); }); it("should clear loading state after failed fetch", async () => { @@ -258,8 +212,7 @@ describe("VectorStoreSelector", () => { renderComponent(); await waitForDataFetch(); - const select = getSelectElement(); - expect(select).toHaveAttribute("data-loading", "false"); + expect(getSelectElement()).toHaveAttribute("data-loading", "false"); consoleErrorSpy.mockRestore(); }); }); @@ -280,7 +233,7 @@ describe("VectorStoreSelector", () => { const option1 = screen.getByText("My Store (store-1)"); expect(option1).toBeInTheDocument(); - expect(option1).toHaveAttribute("data-option-title", "A test store"); + expect(option1).toHaveAttribute("data-option-description", "A test store"); }); it("should fallback to vector_store_id when vector_store_name is missing", async () => { @@ -289,19 +242,18 @@ describe("VectorStoreSelector", () => { const option3 = screen.getByText("store-3 (store-3)"); expect(option3).toBeInTheDocument(); - // When vector_store_name is missing, title uses vector_store_description if available, otherwise vector_store_id - expect(option3).toHaveAttribute("data-option-title", "Store without name"); + expect(option3).toHaveAttribute("data-option-description", "Store without name"); }); - it("should use vector_store_description as title when available", async () => { + it("should use vector_store_description as description when available", async () => { renderComponent(); await waitForDataFetch(); const option1 = screen.getByText("My Store (store-1)"); - expect(option1).toHaveAttribute("data-option-title", "A test store"); + expect(option1).toHaveAttribute("data-option-description", "A test store"); }); - it("should fallback to vector_store_id as title when vector_store_description is missing", async () => { + it("should omit description when vector_store_description is missing", async () => { const storesWithoutDescription: VectorStore[] = [ { vector_store_id: "store-no-desc", @@ -318,7 +270,7 @@ describe("VectorStoreSelector", () => { await waitForDataFetch(); const option = screen.getByText("store-no-desc (store-no-desc)"); - expect(option).toHaveAttribute("data-option-title", "store-no-desc"); + expect(option).not.toHaveAttribute("data-option-description"); }); it("should use vector_store_id as option value", async () => { @@ -337,8 +289,7 @@ describe("VectorStoreSelector", () => { renderComponent(); await waitForDataFetch(); - const options = getOptionElements(); - expect(options.length).toBe(0); + expect(getOptionElements().length).toBe(0); }); it("should handle response without data property", async () => { @@ -347,8 +298,7 @@ describe("VectorStoreSelector", () => { renderComponent(); await waitForDataFetch(); - const options = getOptionElements(); - expect(options.length).toBe(0); + expect(getOptionElements().length).toBe(0); }); }); @@ -357,27 +307,21 @@ describe("VectorStoreSelector", () => { renderComponent({ value: ["store-1", "store-2"] }); await waitForDataFetch(); - const select = getSelectElement(); - const dataValue = select.getAttribute("data-value"); - expect(dataValue).toBe(JSON.stringify(["store-1", "store-2"])); + expect(getSelectElement().getAttribute("data-value")).toBe(JSON.stringify(["store-1", "store-2"])); }); it("should handle empty value array", async () => { renderComponent({ value: [] }); await waitForDataFetch(); - const select = getSelectElement(); - const dataValue = select.getAttribute("data-value"); - expect(dataValue).toBe(JSON.stringify([])); + expect(getSelectElement().getAttribute("data-value")).toBe(JSON.stringify([])); }); it("should handle undefined value", async () => { renderComponent({ value: undefined }); await waitForDataFetch(); - const select = getSelectElement(); - const dataValue = select.getAttribute("data-value"); - expect(dataValue).toBeNull(); // undefined value results in no data-value attribute + expect(getSelectElement().getAttribute("data-value")).toBeNull(); }); }); @@ -387,7 +331,6 @@ describe("VectorStoreSelector", () => { await waitForDataFetch(); const select = getSelectElement(); - // Simulate selecting store-1 by setting test data attribute select.setAttribute("data-test-selection", '["store-1"]'); fireEvent.click(select); @@ -399,7 +342,6 @@ describe("VectorStoreSelector", () => { await waitForDataFetch(); const select = getSelectElement(); - // Simulate selecting multiple values select.setAttribute("data-test-selection", '["store-1", "store-2"]'); fireEvent.click(select); @@ -411,7 +353,6 @@ describe("VectorStoreSelector", () => { await waitForDataFetch(); const select = getSelectElement(); - // Simulate deselecting store-1 select.setAttribute("data-test-selection", '["store-2"]'); fireEvent.click(select); @@ -450,7 +391,6 @@ describe("VectorStoreSelector", () => { renderComponent(); await waitForDataFetch(); - // Component should still render expect(getSelectElement()).toBeInTheDocument(); consoleErrorSpy.mockRestore(); }); @@ -474,8 +414,7 @@ describe("VectorStoreSelector", () => { await waitForDataFetch(); expect(screen.getByText("minimal-store (minimal-store)")).toBeInTheDocument(); - const option = screen.getByText("minimal-store (minimal-store)"); - expect(option).toHaveAttribute("data-option-title", "minimal-store"); + expect(screen.getByText("minimal-store (minimal-store)")).not.toHaveAttribute("data-option-description"); }); it("should handle very long vector store names", async () => { @@ -495,8 +434,7 @@ describe("VectorStoreSelector", () => { renderComponent(); await waitForDataFetch(); - const expectedLabel = `${"A".repeat(200)} (store-long)`; - expect(screen.getByText(expectedLabel)).toBeInTheDocument(); + expect(screen.getByText(`${"A".repeat(200)} (store-long)`)).toBeInTheDocument(); }); it("should handle special characters in vector store names", async () => {