From bea8c9380bed3f12d6071ca9deae7f4eb80e5061 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Thu, 2 Jul 2026 14:09:39 -0700 Subject: [PATCH] refactor(ui): drive cache settings form from a typed frontend schema (#31939) * refactor(ui): drive cache settings form from a typed frontend schema The Cache Settings form was dynamically generated from field metadata shipped by the backend, and read its values back out of the DOM with document.querySelector. That loses type safety and makes client-side validation awkward, which is a poor fit for a form whose shape only changes when a developer edits code. Move the field definitions (name, label, type, default, help text, which redis type they apply to, section, and validation rules) into a typed frontend module and render them through antd Form with controlled state. The GET /cache/settings endpoint is still used to populate current values, and the save/test payload shape sent to POST /cache/settings and /cache/settings/test is unchanged. Per-field validation now lives on each field's antd rules, so an inline error can surface before and on submit; this is where the upcoming Redis URL validation will slot in. The backend's fields output in GET /cache/settings is no longer consumed by the UI, but is left in place since removing it is a separate backend change. * refactor(ui): validate list-field JSON inline so bad input blocks save sentinel_nodes and redis_startup_nodes had no validation rule, so malformed JSON passed validateFields, was caught while building the save payload, and the field was silently omitted; the user's cluster/sentinel config was discarded with no feedback. Add a jsonListRule (same shape as portRule) to both list fields so an invalid value surfaces inline and blocks save. * fix(ui): show valid-JSON examples for cache list fields and clarify the error The Startup Nodes and Sentinel Nodes help text showed Python-style single-quoted examples (e.g. [{'host': '127.0.0.1', 'port': '7001'}]), which the JSON validator correctly rejects, so pasting the example we display failed. Switch both examples to valid JSON with double quotes and change the parse-error message to "Must be a valid JSON array (use double quotes)" so the hint points at the fix. Also add a regression test asserting a numeric field (Database Index) is included in the save payload. * fix(ui): validate numeric cache fields as text so bad input blocks save Numeric fields (Database Index, TTL, Max Connections, Similarity Threshold) rendered as antd InputNumber, which silently coerces non-numeric input to empty. Because the fields are optional, an invalid entry like a full connection URL pasted into Database Index passed validation and was silently dropped from the save payload. Render numeric fields as text inputs with a validation rule (non-negative integer for Database Index and Max Connections, number for TTL and Similarity Threshold), mirroring how Port already works, so invalid input is preserved, flagged inline, and blocks submit instead of vanishing. The save payload still coerces these to real numbers. Adds a regression test for a non-numeric value entered into a numeric field. --- ui/litellm-dashboard/eslint-metrics.json | 4 +- ui/litellm-dashboard/eslint-suppressions.json | 5 - .../cache_settings/CacheFieldGroup.test.tsx | 138 --------- .../cache_settings/CacheFieldGroup.tsx | 47 ---- .../CacheFieldRenderer.test.tsx | 126 --------- .../cache_settings/CacheFieldRenderer.tsx | 149 ---------- .../cache_settings/CacheFieldSection.tsx | 42 +++ .../cache_settings/CacheFormField.tsx | 54 ++++ .../cache_settings/RedisTypeSelector.tsx | 2 +- .../cache_settings/cacheSettingsFields.ts | 261 ++++++++++++++++++ .../cache_settings/cacheSettingsUtils.test.ts | 86 ++++++ .../cache_settings/cacheSettingsUtils.ts | 169 +++++------- .../components/cache_settings/index.test.tsx | 147 ++++++++++ .../components/cache_settings/index.tsx | 246 ++++++++--------- 14 files changed, 780 insertions(+), 696 deletions(-) delete mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.test.tsx delete mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.tsx delete mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.test.tsx delete mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.tsx create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldSection.tsx create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFormField.tsx create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/cacheSettingsFields.ts create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/cacheSettingsUtils.test.ts create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/index.test.tsx diff --git a/ui/litellm-dashboard/eslint-metrics.json b/ui/litellm-dashboard/eslint-metrics.json index 92c5a991eb6..4475ce0e74b 100644 --- a/ui/litellm-dashboard/eslint-metrics.json +++ b/ui/litellm-dashboard/eslint-metrics.json @@ -1,5 +1,5 @@ { - "@typescript-eslint/no-explicit-any": 2013, + "@typescript-eslint/no-explicit-any": 1991, "complexity": 126, - "max-depth": 61 + "max-depth": 59 } diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 7ea13deb934..44c0d8f55ef 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -834,11 +834,6 @@ "count": 1 } }, - "src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "src/app/(dashboard)/caching/components/cache_settings/RedisTypeSelector.tsx": { "no-restricted-imports": { "count": 1 diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.test.tsx deleted file mode 100644 index 6eba0718948..00000000000 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.test.tsx +++ /dev/null @@ -1,138 +0,0 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; -import { render, screen, cleanup } from "@testing-library/react"; -import CacheFieldGroup from "./CacheFieldGroup"; - -describe("CacheFieldGroup", () => { - vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ - default: () => ({ - token: "mock-token", - accessToken: "mock-access-token", - userId: "mock-user-id", - userEmail: "test@example.com", - userRole: "Admin", - premiumUser: false, - disabledPersonalKeyCreation: false, - showSSOBanner: false, - }), - })); - - beforeEach(() => { - cleanup(); - }); - - it("should filter and render fields based on redisType", () => { - /** - * Tests that CacheFieldGroup filters fields based on redis_type and redisType prop. - * This is the core functionality that shows/hides fields based on Redis deployment type. - */ - const fields = [ - { - field_name: "host", - field_type: "String", - ui_field_name: "Host", - redis_type: null, // Applies to all types - }, - { - field_name: "redis_startup_nodes", - field_type: "List", - ui_field_name: "Startup Nodes", - redis_type: "cluster", // Only for cluster - }, - { - field_name: "sentinel_nodes", - field_type: "List", - ui_field_name: "Sentinel Nodes", - redis_type: "sentinel", // Only for sentinel - }, - ]; - - const cacheSettings = { - host: "localhost", - redis_startup_nodes: [], - }; - - // Test with cluster type - should show host and redis_startup_nodes - const { rerender } = render( - , - ); - - expect(screen.getByText("Cluster Settings")).toBeInTheDocument(); - expect(screen.getAllByText("Host")).toHaveLength(1); - expect(screen.getByText("Startup Nodes")).toBeInTheDocument(); - expect(screen.queryByText("Sentinel Nodes")).not.toBeInTheDocument(); - - // Test with sentinel type - should show host and sentinel_nodes - rerender( - , - ); - - expect(screen.getByText("Sentinel Settings")).toBeInTheDocument(); - expect(screen.getAllByText("Host")).toHaveLength(1); - expect(screen.getByText("Sentinel Nodes")).toBeInTheDocument(); - expect(screen.queryByText("Startup Nodes")).not.toBeInTheDocument(); - - // Test with node type - should only show host - rerender(); - - expect(screen.getByText("Node Settings")).toBeInTheDocument(); - expect(screen.getAllByText("Host")).toHaveLength(1); - expect(screen.queryByText("Startup Nodes")).not.toBeInTheDocument(); - expect(screen.queryByText("Sentinel Nodes")).not.toBeInTheDocument(); - }); - - it("should return null when no fields are visible", () => { - /** - * Tests that CacheFieldGroup returns null when no fields match the redisType. - * This prevents rendering empty sections in the UI. - */ - const fields = [ - { - field_name: "redis_startup_nodes", - field_type: "List", - ui_field_name: "Startup Nodes", - redis_type: "cluster", // Only for cluster - }, - ]; - - const cacheSettings = {}; - - const { container } = render( - , - ); - - // Component should return null, so container should be empty - expect(container.firstChild).toBeNull(); - }); - - it("should use field_default when currentValue is not available", () => { - /** - * Tests that CacheFieldGroup falls back to field_default when currentValue is missing. - * This ensures fields display default values when cache settings are not set. - */ - const fields = [ - { - field_name: "port", - field_type: "Integer", - ui_field_name: "Port", - field_default: 6379, - redis_type: null, - }, - ]; - - const cacheSettings = {}; // No port value set - - render( - , - ); - - const input = screen.getByRole("spinbutton", { name: "" }); - expect(input).toBeInTheDocument(); - expect(input).toHaveAttribute("name", "port"); - expect(input).toHaveValue(6379); - }); -}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.tsx deleted file mode 100644 index 21eb0199853..00000000000 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldGroup.tsx +++ /dev/null @@ -1,47 +0,0 @@ -import React from "react"; -import CacheFieldRenderer from "./CacheFieldRenderer"; - -interface CacheFieldGroupProps { - title: string; - fields: any[]; - cacheSettings: { [key: string]: any }; - redisType: string; - gridCols?: string; -} - -const CacheFieldGroup: React.FC = ({ - title, - fields, - cacheSettings, - redisType, - gridCols = "grid-cols-1 gap-6 sm:grid-cols-2", -}) => { - const shouldShowField = (field: any): boolean => { - // Show field if it applies to all types (redis_type is null/undefined) or to current selected type - if (field.redis_type === null || field.redis_type === undefined) { - return true; - } - - return field.redis_type === redisType; - }; - - const visibleFields = fields.filter(shouldShowField); - - if (visibleFields.length === 0) { - return null; - } - - return ( -
-

{title}

-
- {visibleFields.map((field) => { - const currentValue = cacheSettings[field.field_name] ?? field.field_default ?? ""; - return ; - })} -
-
- ); -}; - -export default CacheFieldGroup; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.test.tsx deleted file mode 100644 index 3cfacac7fda..00000000000 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.test.tsx +++ /dev/null @@ -1,126 +0,0 @@ -import { describe, it, expect, vi } from "vitest"; -import { render, screen } from "@testing-library/react"; -import CacheFieldRenderer from "./CacheFieldRenderer"; - -// Mock the useAuthorized hook to avoid Next.js router dependency -vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ - default: () => ({ - token: "mock-token", - accessToken: "mock-access-token", - userId: "mock-user-id", - userEmail: "test@example.com", - userRole: "Admin", - premiumUser: false, - disabledPersonalKeyCreation: false, - showSSOBanner: false, - }), -})); - -describe("CacheFieldRenderer", () => { - it("should render a checkbox for Boolean field type", () => { - /** - * Tests that Boolean fields render as checkboxes with proper defaultChecked value. - * This is the core functionality for boolean cache settings. - */ - const field = { - field_name: "ssl", - field_type: "Boolean", - ui_field_name: "Enable SSL", - field_description: "Enable SSL encryption", - }; - - render(); - - const checkbox = screen.getByRole("checkbox", { name: "" }); - expect(checkbox).toBeInTheDocument(); - expect(checkbox).toBeChecked(); - expect(screen.getByText("Enable SSL")).toBeInTheDocument(); - expect(screen.getByText("Enable SSL encryption")).toBeInTheDocument(); - }); - - it("should render a textarea for List field type", () => { - /** - * Tests that List fields render as textareas with JSON stringified values. - * This handles array/list cache settings like redis_startup_nodes. - */ - const field = { - field_name: "redis_startup_nodes", - field_type: "List", - ui_field_name: "Redis Startup Nodes", - field_description: "List of Redis cluster nodes", - }; - - const currentValue = [ - { host: "localhost", port: 6379 }, - { host: "localhost", port: 6380 }, - ]; - - render(); - - const textarea = screen.getByRole("textbox"); - expect(textarea).toBeInTheDocument(); - expect(textarea.tagName).toBe("TEXTAREA"); - expect(textarea).toHaveValue(JSON.stringify(currentValue, null, 2)); - expect(screen.getByText("Redis Startup Nodes")).toBeInTheDocument(); - }); - - it("should render a password input for password field", () => { - /** - * Tests that password fields render as password inputs. - * This ensures sensitive data is masked in the UI. - */ - const field = { - field_name: "password", - field_type: "String", - ui_field_name: "Password", - field_description: "Redis password", - }; - - render(); - - const input = screen.getByPlaceholderText("Redis password"); - expect(input).toBeInTheDocument(); - expect(input).toHaveAttribute("type", "password"); - expect(input).toHaveValue("secret123"); - }); - - it("should render a number input for Integer field type", () => { - /** - * Tests that Integer fields render as number inputs. - * This ensures proper validation for numeric cache settings. - */ - const field = { - field_name: "port", - field_type: "Integer", - ui_field_name: "Port", - field_description: "Redis port number", - }; - - render(); - - const input = screen.getByPlaceholderText("Redis port number"); - expect(input).toBeInTheDocument(); - expect(input).toHaveAttribute("type", "number"); - expect(input).toHaveValue(6379); - }); - - it("should render a text input for String field type", () => { - /** - * Tests that String fields render as text inputs. - * This is the default rendering for text-based cache settings. - */ - const field = { - field_name: "host", - field_type: "String", - ui_field_name: "Host", - field_description: "Redis host address", - }; - - render(); - - const input = screen.getByPlaceholderText("Redis host address"); - expect(input).toBeInTheDocument(); - expect(input).toHaveAttribute("type", "text"); - expect(input).toHaveValue("localhost"); - }); -}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.tsx deleted file mode 100644 index 27d9fc57200..00000000000 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldRenderer.tsx +++ /dev/null @@ -1,149 +0,0 @@ -"use client"; - -import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; -import { NumberInput, TextInput } from "@tremor/react"; -import { Select } from "antd"; -import React, { useEffect, useState } from "react"; -import { fetchAvailableModels, ModelGroup } from "@/components/llm_calls/fetch_models"; -import NumericalInput from "@/components/shared/numerical_input"; - -interface CacheFieldRendererProps { - field: any; - currentValue: any; -} - -const CacheFieldRenderer: React.FC = ({ field, currentValue }) => { - const [modelInfo, setModelInfo] = useState([]); - const [selectedModel, setSelectedModel] = useState(currentValue || ""); - const { accessToken } = useAuthorized(); - - useEffect(() => { - if (!accessToken) return; - - const loadModels = async () => { - try { - const uniqueModels = await fetchAvailableModels(accessToken); - console.log("Fetched models for selector:", uniqueModels); - - if (uniqueModels.length > 0) { - setModelInfo(uniqueModels); - } - } catch (error) { - console.error("Error fetching model info:", error); - } - }; - - loadModels(); - }, [accessToken]); - - if (field.field_type === "Boolean") { - return ( -
- -
- - {field.field_description} -
-
- ); - } - - if (field.field_type === "Integer" || field.field_type === "Float") { - return ( -
- - -

{field.field_description}

-
- ); - } - - if (field.field_type === "List") { - return ( -
- -