From 7d97bab405105e2e64496efddcf9db699c90f482 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Tue, 18 Aug 2026 11:43:47 -0700 Subject: [PATCH] refactor(ui): migrate auto router and credential forms to react-hook-form and shadcn (#37304) * refactor(ui): migrate auto router and credential forms to react-hook-form and shadcn Moves four antd Form surfaces onto useZodForm plus the shared FormField and FieldGroup primitives: the routing group modal, the auto router edit modal, the add auto router tab, and the reuse credentials modal. Field labels that carried an antd tooltip= keep it as a hover Tooltip on a help icon, and hardcoded greys give way to semantic color tokens. Two Base UI combobox wrappers come out of the two auto router surfaces that shared the same antd controls: AccessGroupTagsCombobox replaces mode="tags" for model access groups, and ModelChoiceCombobox replaces the searchable single select for default and embedding models. Payload building for the routing group modal moves to routingGroupPayload.ts so the JSON args parsing and the four bound fields can be asserted directly instead of through a render. handle_add_auto_router_submit now takes a resetForm callback rather than an antd form instance, which drops one any from its signature. No change to what any of these forms submit. The reuse credentials payload keeps the same key set, with the stored credential values rendered read-only and merged back in at submit rather than copied into form state. * refactor(ui): type the auto router create payload boundary handleAddAutoRouterSubmit took its values as any, so a change to either side of the auto router create payload passed static checking. It now takes an exported AddAutoRouterValues, and add_auto_router_tab annotates the object it builds with that same type, so the producer and the consumer cannot drift. Its model_info is built in one shot rather than assigned into after the fact, which drops the second any and keeps the two conditional keys exactly as they were. Adds a routing group case that saves an untouched edit of a group whose stored arguments are null, which is the shape the proxy returns for an unset field. --- ui/litellm-dashboard/eslint-suppressions.json | 7 +- .../add_model/AccessGroupTagsCombobox.tsx | 102 ++++ .../add_model/ModelChoiceCombobox.tsx | 69 +++ .../add_model/add_auto_router_tab.tsx | 494 +++++++++-------- .../handle_add_auto_router_submit.tsx | 73 ++- .../edit_auto_router_modal.tsx | 515 ++++++++++-------- .../model_add/reuse_credentials.test.tsx | 117 ++++ .../model_add/reuse_credentials.tsx | 131 +++-- .../routing_groups/RoutingGroupModal.test.tsx | 296 ++++++++++ .../routing_groups/RoutingGroupModal.tsx | 292 +++++----- .../routingGroupPayload.test.ts | 153 ++++++ .../routing_groups/routingGroupPayload.ts | 49 ++ 12 files changed, 1629 insertions(+), 669 deletions(-) create mode 100644 ui/litellm-dashboard/src/components/add_model/AccessGroupTagsCombobox.tsx create mode 100644 ui/litellm-dashboard/src/components/add_model/ModelChoiceCombobox.tsx create mode 100644 ui/litellm-dashboard/src/components/model_add/reuse_credentials.test.tsx create mode 100644 ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.test.tsx create mode 100644 ui/litellm-dashboard/src/components/routing_groups/routingGroupPayload.test.ts create mode 100644 ui/litellm-dashboard/src/components/routing_groups/routingGroupPayload.ts diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 0555884026c..dd90eadad08 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -1934,7 +1934,7 @@ "count": 1 }, "no-restricted-imports": { - "count": 2 + "count": 1 } }, "src/components/add_model/add_model_modes.tsx": { @@ -2284,9 +2284,6 @@ "local/filename-pascal-case": { "count": 1 }, - "no-restricted-imports": { - "count": 2 - }, "react-hooks/immutability": { "count": 1 } @@ -2431,7 +2428,7 @@ "count": 1 }, "no-restricted-imports": { - "count": 2 + "count": 1 } }, "src/components/model_dashboard/ModelSettingsModal/ModelSettingsModal.tsx": { diff --git a/ui/litellm-dashboard/src/components/add_model/AccessGroupTagsCombobox.tsx b/ui/litellm-dashboard/src/components/add_model/AccessGroupTagsCombobox.tsx new file mode 100644 index 00000000000..bbf0c7f1bdd --- /dev/null +++ b/ui/litellm-dashboard/src/components/add_model/AccessGroupTagsCombobox.tsx @@ -0,0 +1,102 @@ +"use client"; + +import React, { useState } from "react"; +import { + Combobox, + ComboboxChip, + ComboboxChips, + ComboboxChipsInput, + ComboboxContent, + ComboboxEmpty, + ComboboxItem, + ComboboxList, + ComboboxValue, + useComboboxAnchor, +} from "@/components/ui/combobox"; + +interface AccessGroupTagsComboboxProps { + id: string; + value: string[] | undefined; + onChange: (value: string[]) => void; + options: string[]; + ariaInvalid: true | undefined; + ariaDescribedBy: string | undefined; +} + +const AccessGroupTagsCombobox: React.FC = ({ + id, + value, + onChange, + options, + ariaInvalid, + ariaDescribedBy, +}) => { + const anchor = useComboboxAnchor(); + const [query, setQuery] = useState(""); + const selected = value ?? []; + const trimmedQuery = query.trim(); + const items = trimmedQuery && !options.includes(trimmedQuery) ? [...options, trimmedQuery] : options; + + const commit = (next: string[]) => { + onChange(Array.from(new Set(next))); + setQuery(""); + }; + + const handleInputValueChange = (next: string) => { + if (!next.includes(",")) { + setQuery(next); + return; + } + commit([ + ...selected, + ...next + .split(",") + .map((tag) => tag.trim()) + .filter(Boolean), + ]); + }; + + return ( + + }> + + {(groups: string[]) => ( + <> + {groups.map((group) => ( + + {group} + + ))} + + + )} + + + + No access groups found + + {(group: string) => ( + + {group} + + )} + + + + ); +}; + +export default AccessGroupTagsCombobox; diff --git a/ui/litellm-dashboard/src/components/add_model/ModelChoiceCombobox.tsx b/ui/litellm-dashboard/src/components/add_model/ModelChoiceCombobox.tsx new file mode 100644 index 00000000000..9021536f22a --- /dev/null +++ b/ui/litellm-dashboard/src/components/add_model/ModelChoiceCombobox.tsx @@ -0,0 +1,69 @@ +"use client"; + +import React from "react"; +import { + Combobox, + ComboboxContent, + ComboboxEmpty, + ComboboxInput, + ComboboxItem, + ComboboxList, +} from "@/components/ui/combobox"; + +export interface ModelChoice { + value: string; + label: string; +} + +interface ModelChoiceComboboxProps { + id: string; + value: string; + onChange: (value: string) => void; + choices: ModelChoice[]; + placeholder: string; + ariaInvalid: true | undefined; + ariaDescribedBy: string | undefined; +} + +const ModelChoiceCombobox: React.FC = ({ + id, + value, + onChange, + choices, + placeholder, + ariaInvalid, + ariaDescribedBy, +}) => { + const selected = value ? choices.find((choice) => choice.value === value) ?? { value, label: value } : null; + + return ( + onChange(choice?.value ?? "")} + itemToStringLabel={(choice: ModelChoice) => choice.label} + isItemEqualToValue={(choice: ModelChoice, current: ModelChoice) => choice.value === current.value} + > + + + No models found + + {(choice: ModelChoice) => ( + + {choice.label} + + )} + + + + ); +}; + +export default ModelChoiceCombobox; diff --git a/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.tsx b/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.tsx index 126f80b58fd..7537c91849e 100644 --- a/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.tsx +++ b/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.tsx @@ -1,13 +1,22 @@ import React, { useEffect, useState } from "react"; import { useQuery } from "@tanstack/react-query"; -import { Card, Form, Button, Tooltip, Typography, Select as AntdSelect, Modal } from "antd"; -import { DownOutlined, RightOutlined } from "@ant-design/icons"; -import { TextInput } from "@tremor/react"; +import { useWatch } from "react-hook-form"; +import { Card, Select as AntdSelect, Modal } from "antd"; +import { ChevronDown, ChevronRight, CircleHelp } from "lucide-react"; +import { z } from "zod/v4"; +import { FieldGroup } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; +import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; +import { useZodForm } from "@/lib/forms/useZodForm"; +import AccessGroupTagsCombobox from "./AccessGroupTagsCombobox"; import { modelAvailableCall } from "../networking"; import { all_admin_roles } from "@/utils/roles"; import { type ModelWriteScope } from "@/utils/modelPermissions"; import TeamDropdown from "../common_components/team_dropdown"; -import { handleAddAutoRouterSubmit } from "./handle_add_auto_router_submit"; +import { type AddAutoRouterValues, handleAddAutoRouterSubmit } from "./handle_add_auto_router_submit"; import { fetchAvailableModels } from "@/components/llm_calls/fetch_models"; import { autoRouterListKey, fetchAllModelDeployments } from "@/app/(dashboard)/hooks/models/useModels"; import ComplexityRouterConfig, { @@ -121,6 +130,47 @@ const getSubmitBlockedReason = ( getKeywordTierRulesError(keywordTierRules) ?? getReferencedModelsError(referencedModelsParams, availability); +const autoRouterSchema = (requiresTeamScope: boolean) => + z.object({ + auto_router_name: z.string().min(1, "Auto router name is required"), + team_id: requiresTeamScope ? z.string().min(1, "Please select a team to continue") : z.string(), + model_access_group: z.array(z.string()).optional(), + }); + +type AddAutoRouterFormValues = z.infer>; + +const EMPTY_FORM_VALUES: AddAutoRouterFormValues = { + auto_router_name: "", + team_id: "", + model_access_group: undefined, +}; + +const labelWithHint = (label: string, hint: string): React.ReactNode => ( + <> + {label} + + } /> + {hint} + + +); + +const teamScopePayload = (requiresTeamScope: boolean, teamId: string): { team_id?: string } => + requiresTeamScope ? { team_id: teamId } : {}; + +const BlockedReasonTooltip: React.FC<{ reason: string | null; children: React.ReactElement }> = ({ + reason, + children, +}) => + reason === null ? ( + children + ) : ( + + + {reason} + + ); + const AddAutoRouterTab: React.FC = ({ handleOk, accessToken, @@ -129,7 +179,9 @@ const AddAutoRouterTab: React.FC = ({ createScope = "unscoped-ok", }) => { const requiresTeamScope = createScope === "team-required"; - const [form] = Form.useForm(); + const form = useZodForm(autoRouterSchema(requiresTeamScope), { defaultValues: EMPTY_FORM_VALUES }); + const watchedName = useWatch({ control: form.control, name: "auto_router_name" }); + const watchedTeamId = useWatch({ control: form.control, name: "team_id" }); const [modelAccessGroups, setModelAccessGroups] = useState([]); const [complexityRouterConfig, setComplexityRouterConfig] = useState({ @@ -308,7 +360,7 @@ const AddAutoRouterTab: React.FC = ({ dimensionWeights: complexityRouterConfig.dimension_weights, }; - const submitRecommendedRouter = (name: string) => { + const submitRecommendedRouter = async (name: string) => { const { tiers, tierLabels, classifierType, classifierLlmConfig } = complexityRouterConfigParams; const missingTiersError = getMissingTiersError(tiers); @@ -345,8 +397,8 @@ const AddAutoRouterTab: React.FC = ({ return; } - // submitBlockedReason already disables the button for this, but Form's onFinish (wired to this - // same handler) fires on Enter regardless of the button's disabled state - without this check, + // submitBlockedReason already disables the button for this, but the form's submit handler (wired to + // this same function) fires on Enter regardless of the button's disabled state - without this check, // Enter in the name field could still create a router referencing a model that disappeared from // availableModelSet after the tiers were filled in. const referencedModelsError = getReferencedModelsError(referencedModelsParams, groupsOnlyAvailability); @@ -357,48 +409,41 @@ const AddAutoRouterTab: React.FC = ({ } const defaultModel = resolveComplexityDefaultModel(tiers, complexityRouterConfig.default_model); + const validatedFields = requiresTeamScope + ? (["auto_router_name", "team_id"] as const) + : (["auto_router_name"] as const); - form.setFieldsValue({ - custom_llm_provider: "auto_router", - model: name, - api_key: "not_required_for_auto_router", + if (!(await form.trigger(validatedFields))) { + toast.fromError("Please fill in all required fields"); + return; + } + + // auto_router_default_model (-> litellm_params, read by the backend at init) and + // complexity_router_config.default_model (-> the pin marker read back on edit, see + // hydratePinnedDefaultModel in edit_auto_router_modal.tsx) must both come from the same + // `defaultModel`, or the two fields diverge and hydration's divergence check misfires. + const submitValues: AddAutoRouterValues = { + auto_router_name: name, + ...teamScopePayload(requiresTeamScope, form.getValues("team_id")), auto_router_default_model: defaultModel, - }); + model_type: "complexity_router", + complexity_router_config: buildComplexityRouterConfig(complexityRouterConfigParams), + model_access_group: form.getValues("model_access_group"), + }; - form - .validateFields(requiresTeamScope ? ["auto_router_name", "team_id"] : ["auto_router_name"]) - .then((values) => { - // auto_router_default_model (-> litellm_params, read by the backend at init) and - // complexity_router_config.default_model (-> the pin marker read back on edit, see - // hydratePinnedDefaultModel in edit_auto_router_modal.tsx) must both come from the same - // `defaultModel`, or the two fields diverge and hydration's divergence check misfires. - const submitValues = { - ...values, - auto_router_name: name, - auto_router_default_model: defaultModel, - model_type: "complexity_router", - complexity_router_config: buildComplexityRouterConfig(complexityRouterConfigParams), - model_access_group: form.getFieldValue("model_access_group"), - }; - - handleAddAutoRouterSubmit(submitValues, accessToken, form, handleOk); - }) - .catch((error) => { - console.error("Validation failed:", error); - toast.fromError("Please fill in all required fields"); - }); + handleAddAutoRouterSubmit(submitValues, accessToken, () => form.reset(EMPTY_FORM_VALUES), handleOk); }; - const handleAutoRouterSubmit = () => { - const name = form.getFieldValue("auto_router_name"); + const handleAutoRouterSubmit = async () => { + const name = form.getValues("auto_router_name"); if (!name) { setShowValidationErrors(true); - form.validateFields(["auto_router_name"]).catch(() => undefined); + void form.trigger("auto_router_name"); toast.fromError("Please enter an Auto Router Name"); return; } - submitRecommendedRouter(name); + await submitRecommendedRouter(name); }; const handleTestConnection = () => { @@ -422,196 +467,204 @@ const AddAutoRouterTab: React.FC = ({ }; return ( - <> + -
- - - - -
- - handleAutoRouterSubmit())} noValidate> + + - {sortedPresetOptions.map(({ preset, availability: presetState }) => { - const disabledHint = presetDisabledHint(presetState); - const isDisabled = disabledHint !== null; - const hintClass = isPresetHintAlarming(presetState) ? "text-red-500" : "text-gray-400"; - const matchedHint = - presetState.kind === "available" && presetState.viaDeployments ? "Matches your deployments" : null; + {({ ref, ...field }) => } + - return ( - -
-
{preset.label}
-
{preset.description}
- {disabledHint &&
{disabledHint}
} - {matchedHint &&
{matchedHint}
} -
- - ); - })} - -
-
Custom Configuration
-
Define your auto router from scratch
-
- - - {modelsUnverifiable && ( -
- Could not load available models.{" "} - -
- )} -
- - {requiresTeamScope && ( - - - - )} - -
- - {detailsExpanded && ( -
- -
- )} -
- - {/* Model Access Groups - Admin only */} - {isAdmin && ( - +
+ ({ - value: group, - label: group, - }))} - maxTagCount="responsive" - allowClear - /> - - )} + value={selectedPreset} + onChange={handlePresetChange} + placeholder="Choose a template or select Custom to define your own" + className="w-full" + optionLabelProp="label" + data-testid="template-selector" + > + {sortedPresetOptions.map(({ preset, availability: presetState }) => { + const disabledHint = presetDisabledHint(presetState); + const isDisabled = disabledHint !== null; + const hintClass = isPresetHintAlarming(presetState) + ? "text-red-500 dark:text-red-400" + : "text-muted-foreground"; + const matchedHint = + presetState.kind === "available" && presetState.viaDeployments ? "Matches your deployments" : null; -
- - Need Help? - -
- - + return ( + +
+
{preset.label}
+
{preset.description}
+ {disabledHint &&
{disabledHint}
} + {matchedHint && ( +
{matchedHint}
+ )} +
+ + ); + })} + +
+
Custom Configuration
+
Define your auto router from scratch
+
+ + + {modelsUnverifiable && ( +
+ Could not load available models.{" "} + +
+ )} +
+ + {requiresTeamScope && ( + + {({ id, value, onChange }) => } + + )} + +
+ + {detailsExpanded && ( +
+ +
+ )} +
+ + {isAdmin && ( + + {({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => ( + + )} + + )} + +
+ + + Need Help? + + } + /> + Get help on our github - { +
+ + + - } - - - + + + +
-
- + + = ({ destroyOnHidden onCancel={() => setIsRoutingTestVisible(false)} footer={[ - , ]} @@ -634,8 +687,8 @@ const AddAutoRouterTab: React.FC = ({ complexityRouterConfig.tiers, complexityRouterConfig.default_model, )} - routerName={form.getFieldValue("auto_router_name")} - teamId={requiresTeamScope ? form.getFieldValue("team_id") : undefined} + routerName={watchedName} + teamId={requiresTeamScope ? watchedTeamId : undefined} /> )} @@ -650,6 +703,7 @@ const AddAutoRouterTab: React.FC = ({ footer={[ - - - - + {submitBlockedReason === null ? ( + + ) : ( + + + Save Changes + + } + /> + {submitBlockedReason} + + )} + + ); diff --git a/ui/litellm-dashboard/src/components/model_add/reuse_credentials.test.tsx b/ui/litellm-dashboard/src/components/model_add/reuse_credentials.test.tsx new file mode 100644 index 00000000000..6d41d180efd --- /dev/null +++ b/ui/litellm-dashboard/src/components/model_add/reuse_credentials.test.tsx @@ -0,0 +1,117 @@ +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; + +import { renderWithProviders, screen } from "@/../tests/test-utils"; + +import type { CredentialItem } from "../networking"; +import ReuseCredentialsModal from "./reuse_credentials"; + +const EXISTING_CREDENTIAL: CredentialItem = { + credential_name: "openai-prod", + credential_values: { api_key: "sk-stored-value", api_base: "https://api.example.com" }, + credential_info: { custom_llm_provider: "openai" }, +}; + +const renderModal = (existingCredential: CredentialItem | null = EXISTING_CREDENTIAL) => { + const onAddCredential = vi.fn(); + const onCancel = vi.fn(); + const setIsCredentialModalOpen = vi.fn(); + renderWithProviders( + , + ); + return { onAddCredential, onCancel, setIsCredentialModalOpen }; +}; + +const submit = async (user: ReturnType) => + await user.click(screen.getByRole("button", { name: "Reuse Credentials" })); + +describe("ReuseCredentialsModal", () => { + it("submits the typed name alongside every stored credential value", async () => { + const user = userEvent.setup(); + const { onAddCredential, setIsCredentialModalOpen } = renderModal(); + + const nameInput = screen.getByLabelText("Credential Name:"); + await user.clear(nameInput); + await user.type(nameInput, "reused-openai"); + await submit(user); + + expect(onAddCredential).toHaveBeenCalledTimes(1); + expect(onAddCredential).toHaveBeenCalledWith({ + credential_name: "reused-openai", + api_key: "sk-stored-value", + api_base: "https://api.example.com", + }); + expect(setIsCredentialModalOpen).toHaveBeenCalledWith(false); + }); + + it("seeds the name from the existing credential and submits it untouched", async () => { + const user = userEvent.setup(); + const { onAddCredential } = renderModal(); + + expect(screen.getByLabelText("Credential Name:")).toHaveValue("openai-prod"); + await submit(user); + + expect(onAddCredential).toHaveBeenCalledWith({ + credential_name: "openai-prod", + api_key: "sk-stored-value", + api_base: "https://api.example.com", + }); + }); + + it("renders the stored values as read-only inputs", () => { + renderModal(); + + expect(screen.getByLabelText("api_key")).toBeDisabled(); + expect(screen.getByLabelText("api_key")).toHaveValue("sk-stored-value"); + expect(screen.getByLabelText("api_base")).toBeDisabled(); + }); + + it("blocks the submit and shows the required message when the name is cleared", async () => { + const user = userEvent.setup(); + const { onAddCredential } = renderModal(); + + await user.clear(screen.getByLabelText("Credential Name:")); + await submit(user); + + expect(await screen.findByText("Credential name is required")).toBeInTheDocument(); + expect(onAddCredential).not.toHaveBeenCalled(); + }); + + it("submits only the name when the credential carries no stored values", async () => { + const user = userEvent.setup(); + const { onAddCredential } = renderModal({ + credential_name: "bare", + credential_values: {}, + credential_info: {}, + }); + + await submit(user); + + expect(onAddCredential).toHaveBeenCalledWith({ credential_name: "bare" }); + }); + + it("closes without submitting when Cancel is clicked", async () => { + const user = userEvent.setup(); + const { onAddCredential, onCancel } = renderModal(); + + await user.click(screen.getByRole("button", { name: "Cancel" })); + + expect(onCancel).toHaveBeenCalledTimes(1); + expect(onAddCredential).not.toHaveBeenCalled(); + }); + + it("submits on Enter from the name field", async () => { + const user = userEvent.setup(); + const { onAddCredential } = renderModal(); + + await user.type(screen.getByLabelText("Credential Name:"), "{Enter}"); + + expect(onAddCredential).toHaveBeenCalledTimes(1); + }); +}); diff --git a/ui/litellm-dashboard/src/components/model_add/reuse_credentials.tsx b/ui/litellm-dashboard/src/components/model_add/reuse_credentials.tsx index 85ba82ea046..0e5c8ebca29 100644 --- a/ui/litellm-dashboard/src/components/model_add/reuse_credentials.tsx +++ b/ui/litellm-dashboard/src/components/model_add/reuse_credentials.tsx @@ -1,17 +1,33 @@ import React from "react"; -import { Form, Button, Tooltip, Typography, Modal } from "antd"; -import { TextInput } from "@tremor/react"; +import { Modal } from "antd"; +import { z } from "zod/v4"; +import { Field, FieldGroup, FieldLabel } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; +import { useZodForm } from "@/lib/forms/useZodForm"; import { CredentialItem } from "../networking"; -const { Link } = Typography; interface ReuseCredentialsModalProps { isVisible: boolean; onCancel: () => void; - onAddCredential: (values: any) => void; + onAddCredential: (values: Record) => void; existingCredential: CredentialItem | null; setIsCredentialModalOpen: (isVisible: boolean) => void; } +const reuseCredentialsSchema = z.object({ + credential_name: z.string().min(1, "Credential name is required"), +}); + +type ReuseCredentialsFormValues = z.infer; + +const storedValuesOf = (existingCredential: CredentialItem | null): Record => { + const values: unknown = existingCredential?.credential_values; + return typeof values === "object" && values !== null ? (values as Record) : {}; +}; + const ReuseCredentialsModal: React.FC = ({ isVisible, onCancel, @@ -19,63 +35,72 @@ const ReuseCredentialsModal: React.FC = ({ existingCredential, setIsCredentialModalOpen, }) => { - const [form] = Form.useForm(); + const fieldIdPrefix = React.useId(); + const storedValues = storedValuesOf(existingCredential); + const form = useZodForm(reuseCredentialsSchema, { + defaultValues: { credential_name: existingCredential?.credential_name ?? "" }, + }); - const handleSubmit = (values: any) => { - onAddCredential(values); - form.resetFields(); + const handleSubmit = (values: ReuseCredentialsFormValues) => { + onAddCredential({ ...storedValues, ...values }); + form.reset(); setIsCredentialModalOpen(false); }; + const handleCancel = () => { + onCancel(); + form.reset(); + }; + return ( - { - onCancel(); - form.resetFields(); - }} - footer={null} - width={600} - > -
- {/* Credential Name */} - - - + + + + + + {({ ref, ...field }) => ( + + )} + - {/* Display Credential Values of existingCredential, don't allow user to edit. Credential values is a dictionary */} - {Object.entries(existingCredential?.credential_values || {}).map(([key, value]) => ( - - - - ))} + {Object.entries(storedValues).map(([key, value]) => ( + + {key} + + + ))} - {/* Modal Footer */} -
- - Need Help? - +
+ + + Need Help? + + } + /> + Get help on our github + -
- - -
-
- +
+ + +
+
+ + +
); }; diff --git a/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.test.tsx b/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.test.tsx new file mode 100644 index 00000000000..3699a57a657 --- /dev/null +++ b/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.test.tsx @@ -0,0 +1,296 @@ +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; + +import { renderWithProviders, screen } from "@/../tests/test-utils"; + +import RoutingGroupModal from "./RoutingGroupModal"; +import type { RoutingGroup } from "./types"; + +const STRATEGIES = ["simple-shuffle", "latency-based-routing", "usage-based-routing"]; +const MODEL_OPTIONS = ["gpt-4o", "claude-sonnet", "gemini-pro"]; +const STRATEGY_DESCRIPTIONS = { "simple-shuffle": "Spreads requests evenly across the group." }; + +const EXPECTED_STORED_PAYLOAD: RoutingGroup = { + group_name: "already-taken", + models: ["gpt-4o", "claude-sonnet"], + routing_strategy: "latency-based-routing", + routing_strategy_args: { ttl: 3600 }, +}; + +const SEEDED_CREATE: RoutingGroup = { group_name: "", models: ["gemini-pro"], routing_strategy: "simple-shuffle" }; + +const STORED_GROUP: RoutingGroup = { + group_name: "already-taken", + models: ["gpt-4o", "claude-sonnet"], + routing_strategy: "latency-based-routing", + routing_strategy_args: { ttl: 3600 }, +}; + +const STORED_GROUP_NULL_ARGS: RoutingGroup = { + group_name: "already-taken", + models: ["gpt-4o"], + routing_strategy: "latency-based-routing", + routing_strategy_args: null, +}; + +const EXPECTED_NULL_ARGS_PAYLOAD: RoutingGroup = { + group_name: "already-taken", + models: ["gpt-4o"], + routing_strategy: "latency-based-routing", + routing_strategy_args: null, +}; + +const renderModal = (overrides: Partial> = {}) => { + const onSubmit = vi.fn(); + const onClose = vi.fn(); + renderWithProviders( + , + ); + return { onSubmit, onClose }; +}; + +const typeName = async (user: ReturnType, name: string) => { + const input = screen.getByLabelText("Group Name"); + await user.clear(input); + await user.type(input, name); +}; + +const setArgs = async (user: ReturnType, json: string) => { + const textarea = screen.getByLabelText("Strategy Arguments (JSON)"); + await user.clear(textarea); + if (json) { + await user.type(textarea, json); + } +}; + +const pickModels = async (user: ReturnType, ...models: string[]) => { + await user.click(screen.getByLabelText("Models")); + for (const model of models) { + await user.click(await screen.findByRole("option", { name: model })); + } +}; + +const pickStrategy = async (user: ReturnType, strategy: string) => { + await user.click(screen.getByLabelText("Routing Strategy")); + await user.click(await screen.findByRole("option", { name: strategy })); +}; + +const save = async (user: ReturnType, name: string) => + await user.click(screen.getByRole("button", { name })); + +describe("RoutingGroupModal", () => { + it("submits an untouched edit of a group whose stored arguments are null", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ mode: "edit", initialValue: STORED_GROUP_NULL_ARGS }); + + await save(user, "Save Changes"); + + expect(onSubmit).toHaveBeenCalledWith(EXPECTED_NULL_ARGS_PAYLOAD); + }); + + it("submits an untouched edit with the stored models, strategy and parsed arguments", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ mode: "edit", initialValue: STORED_GROUP }); + + await save(user, "Save Changes"); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit.mock.calls[0][0]).toStrictEqual(EXPECTED_STORED_PAYLOAD); + }); + + it("carries a typed group name into the payload", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ initialValue: SEEDED_CREATE }); + + await typeName(user, "fast-chat"); + await save(user, "Create Group"); + + const expected: RoutingGroup = { + group_name: "fast-chat", + models: ["gemini-pro"], + routing_strategy: "simple-shuffle", + routing_strategy_args: null, + }; + expect(onSubmit.mock.calls[0][0]).toStrictEqual(expected); + }); + + it("sends null arguments when the selected strategy does not take them", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ + mode: "edit", + initialValue: { ...STORED_GROUP, routing_strategy: "simple-shuffle" }, + }); + + expect(screen.queryByLabelText("Strategy Arguments (JSON)")).not.toBeInTheDocument(); + await save(user, "Save Changes"); + + const expected: RoutingGroup = { + group_name: "already-taken", + models: ["gpt-4o", "claude-sonnet"], + routing_strategy: "simple-shuffle", + routing_strategy_args: null, + }; + expect(onSubmit.mock.calls[0][0]).toStrictEqual(expected); + }); + + it("sends null arguments when the argument box is emptied", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ mode: "edit", initialValue: STORED_GROUP }); + + await setArgs(user, ""); + await save(user, "Save Changes"); + + expect(onSubmit.mock.calls[0][0]?.routing_strategy_args).toBeNull(); + }); + + it("edits the arguments into the payload", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ mode: "edit", initialValue: STORED_GROUP }); + + await setArgs(user, '{{"ttl": 60, "lowest_latency_buffer": 0}'); + await save(user, "Save Changes"); + + expect(onSubmit.mock.calls[0][0]?.routing_strategy_args).toStrictEqual({ ttl: 60, lowest_latency_buffer: 0 }); + }); + + it("blocks the save and flags the field when the arguments are not valid JSON", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ mode: "edit", initialValue: STORED_GROUP }); + + await setArgs(user, "not json"); + await save(user, "Save Changes"); + + expect(await screen.findByText("Must be valid JSON")).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it("requires a group name", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ + initialValue: { group_name: "", models: ["gemini-pro"], routing_strategy: "simple-shuffle" }, + }); + + await save(user, "Create Group"); + + expect(await screen.findByText("Group name is required")).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it("requires at least one model", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal(); + + await typeName(user, "no-models"); + await save(user, "Create Group"); + + expect(await screen.findByText("Select at least one model")).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it("rejects a name longer than 64 characters", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ + initialValue: { group_name: "", models: ["gemini-pro"], routing_strategy: "simple-shuffle" }, + }); + + await typeName(user, "a".repeat(65)); + await save(user, "Create Group"); + + expect(await screen.findByText("Must be 64 characters or fewer")).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it("rejects a name with characters outside the allowed set", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ + initialValue: { group_name: "", models: ["gemini-pro"], routing_strategy: "simple-shuffle" }, + }); + + await typeName(user, "bad name"); + await save(user, "Create Group"); + + expect(await screen.findByText("Only letters, numbers, dot, underscore, and dash are allowed")).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it("rejects a name another group already uses, ignoring case", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal({ + initialValue: { group_name: "", models: ["gemini-pro"], routing_strategy: "simple-shuffle" }, + }); + + await typeName(user, "Other-Group"); + await save(user, "Create Group"); + + expect(await screen.findByText("A group with this name already exists")).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it("locks the name in edit mode and pretty-prints the stored arguments", () => { + renderModal({ mode: "edit", initialValue: STORED_GROUP }); + + expect(screen.getByLabelText("Group Name")).toHaveValue("already-taken"); + expect(screen.getByLabelText("Group Name")).toBeDisabled(); + expect(screen.getByLabelText("Strategy Arguments (JSON)")).toHaveValue('{\n "ttl": 3600\n}'); + }); + + it("carries picked models and a picked strategy into the payload", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal(); + + await typeName(user, "probe-group"); + await pickModels(user, "gpt-4o", "claude-sonnet"); + await pickStrategy(user, "latency-based-routing"); + await setArgs(user, '{{"ttl": 99}'); + await save(user, "Create Group"); + + const expected: RoutingGroup = { + group_name: "probe-group", + models: ["gpt-4o", "claude-sonnet"], + routing_strategy: "latency-based-routing", + routing_strategy_args: { ttl: 99 }, + }; + expect(onSubmit.mock.calls[0][0]).toStrictEqual(expected); + }); + + it("forgets arguments typed before the strategy stopped taking them", async () => { + const user = userEvent.setup(); + const { onSubmit } = renderModal(); + + await typeName(user, "probe-group"); + await pickModels(user, "gpt-4o"); + await pickStrategy(user, "latency-based-routing"); + await setArgs(user, '{{"ttl": 99}'); + await pickStrategy(user, "simple-shuffle"); + expect(screen.queryByLabelText("Strategy Arguments (JSON)")).not.toBeInTheDocument(); + await pickStrategy(user, "latency-based-routing"); + + expect(screen.getByLabelText("Strategy Arguments (JSON)")).toHaveValue(""); + + await save(user, "Create Group"); + const expected: RoutingGroup = { + group_name: "probe-group", + models: ["gpt-4o"], + routing_strategy: "latency-based-routing", + routing_strategy_args: null, + }; + expect(onSubmit.mock.calls[0][0]).toStrictEqual(expected); + }); + + it("describes the selected strategy", async () => { + renderModal(); + + expect(await screen.findByText("Spreads requests evenly across the group.")).toBeInTheDocument(); + }); +}); diff --git a/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.tsx b/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.tsx index 5b7c0fce6ef..48d707e833e 100644 --- a/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.tsx +++ b/ui/litellm-dashboard/src/components/routing_groups/RoutingGroupModal.tsx @@ -1,10 +1,36 @@ "use client"; -import React, { useMemo } from "react"; -import { Form, Input, Modal, Select, Space, Typography } from "antd"; -import type { RoutingGroup, RoutingStrategy } from "./types"; - -const { Text, Paragraph } = Typography; +import React, { useEffect, useMemo } from "react"; +import { Modal } from "antd"; +import { useWatch } from "react-hook-form"; +import { z } from "zod/v4"; +import { FieldGroup } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { + Combobox, + ComboboxChip, + ComboboxChips, + ComboboxChipsInput, + ComboboxContent, + ComboboxEmpty, + ComboboxItem, + ComboboxList, + ComboboxValue, + useComboboxAnchor, +} from "@/components/ui/combobox"; +import { Input } from "@/components/ui/input"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; +import { Textarea } from "@/components/ui/textarea"; +import { useZodForm } from "@/lib/forms/useZodForm"; +import { + GROUP_NAME_MAX_LENGTH, + GROUP_NAME_PATTERN, + STRATEGIES_WITH_ARGS, + argsForStrategy, + buildRoutingGroupPayload, + toRoutingGroupFormValues, +} from "./routingGroupPayload"; +import type { RoutingGroup } from "./types"; interface RoutingGroupModalProps { open: boolean; @@ -19,17 +45,9 @@ interface RoutingGroupModalProps { saving?: boolean; } -interface FormValues { - group_name: string; - models: string[]; - routing_strategy: RoutingStrategy | string; - routing_strategy_args?: string; -} - -const STRATEGIES_WITH_ARGS = new Set(["latency-based-routing", "usage-based-routing"]); - -const GROUP_NAME_PATTERN = /^[A-Za-z0-9._-]+$/; -const GROUP_NAME_MAX_LENGTH = 64; +const ARGS_EXAMPLES: Record = { + "latency-based-routing": 'Example: { "ttl": 3600, "lowest_latency_buffer": 0 }', +}; const RoutingGroupModal: React.FC = ({ open, @@ -43,47 +61,44 @@ const RoutingGroupModal: React.FC = ({ onSubmit, saving, }) => { - const [form] = Form.useForm(); - const selectedStrategy = Form.useWatch("routing_strategy", form); - - const initialValues: FormValues = { - group_name: initialValue?.group_name ?? "", - models: initialValue?.models ?? [], - routing_strategy: initialValue?.routing_strategy ?? availableStrategies[0] ?? "simple-shuffle", - routing_strategy_args: initialValue?.routing_strategy_args - ? JSON.stringify(initialValue.routing_strategy_args, null, 2) - : "", - }; + const modelsAnchor = useComboboxAnchor(); + const strategyItems = availableStrategies.map((strategy) => ({ label: strategy, value: strategy })); const reservedNames = useMemo(() => { const others = existingGroupNames.filter((n) => n !== initialValue?.group_name); return new Set(others.map((n) => n.toLowerCase())); }, [existingGroupNames, initialValue]); - const handleSubmit = async () => { - const values = await form.validateFields(); - const strategySupportsArgs = STRATEGIES_WITH_ARGS.has(String(values.routing_strategy)); - let parsedArgs: Record | null = null; - if (strategySupportsArgs && values.routing_strategy_args && values.routing_strategy_args.trim()) { - try { - parsedArgs = JSON.parse(values.routing_strategy_args); - } catch { - form.setFields([ - { - name: "routing_strategy_args", - errors: ["Must be valid JSON"], - }, - ]); - return; - } - } + const schema = useMemo(() => { + const shape = { + group_name: z + .string() + .min(1, "Group name is required") + .max(GROUP_NAME_MAX_LENGTH, `Must be ${GROUP_NAME_MAX_LENGTH} characters or fewer`) + .regex(GROUP_NAME_PATTERN, "Only letters, numbers, dot, underscore, and dash are allowed") + .refine((value) => !reservedNames.has(value.trim().toLowerCase()), "A group with this name already exists"), + models: z.array(z.string()).min(1, "Select at least one model"), + routing_strategy: z.string().min(1, "Strategy is required"), + routing_strategy_args: z.string(), + }; + return z.object(shape); + }, [reservedNames]); - await onSubmit({ - group_name: values.group_name.trim(), - models: values.models, - routing_strategy: values.routing_strategy, - routing_strategy_args: parsedArgs, - }); + const form = useZodForm(schema, { defaultValues: toRoutingGroupFormValues(initialValue, availableStrategies) }); + + useEffect(() => { + form.reset(toRoutingGroupFormValues(initialValue, availableStrategies)); + }, [open, initialValue, availableStrategies, form]); + + const selectedStrategy = useWatch({ control: form.control, name: "routing_strategy" }); + + const handleSubmit = async (values: z.infer) => { + const payload = buildRoutingGroupPayload(values); + if (!payload.ok) { + form.setError("routing_strategy_args", { message: payload.argsError }); + return; + } + await onSubmit(payload.group); }; return ( @@ -91,92 +106,115 @@ const RoutingGroupModal: React.FC = ({ title={mode === "create" ? "Create Routing Group" : `Edit ${initialValue?.group_name ?? ""}`} open={open} onCancel={onClose} - onOk={handleSubmit} + onOk={() => void form.handleSubmit(handleSubmit)()} okText={mode === "create" ? "Create Group" : "Save Changes"} cancelText="Cancel" confirmLoading={saving} destroyOnClose width={560} > - - key={mode === "edit" ? `edit-${initialValue?.group_name ?? ""}` : "create"} - form={form} - layout="vertical" - preserve={false} - initialValues={initialValues} - > - { - if (!value) return Promise.resolve(); - if (reservedNames.has(value.trim().toLowerCase())) { - return Promise.reject(new Error("A group with this name already exists")); - } - return Promise.resolve(); - }, - }, - ]} - extra="Use this name as the model in API calls — LiteLLM routes the request to one of the group's models." - > - - - - - ({ label: s, value: s }))} placeholder="Select strategy" /> - - - {selectedStrategy && strategyDescriptions[selectedStrategy] && ( - {strategyDescriptions[selectedStrategy]} - )} - - {STRATEGIES_WITH_ARGS.has(String(selectedStrategy)) && ( - event.preventDefault()} noValidate> + + - - - )} + {({ ref, ...field }) => } + - - + + {({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => ( + + }> + + {(selected: string[]) => ( + <> + {selected.map((model) => ( + + {model} + + ))} + + + )} + + + + No models found + + {(model: string) => ( + + {model} + + )} + + + + )} + + + + {({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => ( + + )} + + + {STRATEGIES_WITH_ARGS.has(selectedStrategy) && ( + + {({ ref, ...field }) => ( +