diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 3bc93b7ebc4..b7c578d8ec6 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -1468,9 +1468,6 @@ }, "local/no-complex-jsx-arrow": { "count": 1 - }, - "react-hooks/set-state-in-effect": { - "count": 2 } }, "src/components/add_model/handle_add_auto_router_submit.tsx": { diff --git a/ui/litellm-dashboard/src/components/add_model/conditional_public_model_name.test.tsx b/ui/litellm-dashboard/src/components/add_model/conditional_public_model_name.test.tsx index 967fc9c458a..07ec36639b4 100644 --- a/ui/litellm-dashboard/src/components/add_model/conditional_public_model_name.test.tsx +++ b/ui/litellm-dashboard/src/components/add_model/conditional_public_model_name.test.tsx @@ -1,8 +1,28 @@ import { render, screen } from "@testing-library/react"; +import React, { useEffect, useRef } from "react"; +import { useFormContext, useWatch } from "react-hook-form"; import { describe, expect, it } from "vitest"; import { MountedFormHost } from "../../../tests/mounted-form-host"; +import type { MountedFormValues } from "../common_components/MountedFormField"; import ConditionalPublicModelName from "./conditional_public_model_name"; +const WRITE_BUDGET = 20; + +const LoopGuard: React.FC = () => { + const form = useFormContext(); + const mappings = useWatch({ control: form.control, name: "model_mappings" }); + const writes = useRef(0); + + useEffect(() => { + writes.current += 1; + if (writes.current > WRITE_BUDGET) { + throw new Error(`model_mappings changed ${WRITE_BUDGET}+ times: the mapping effects are looping`); + } + }, [mappings]); + + return null; +}; + describe("ConditionalPublicModelName", () => { it("should render", () => { render( @@ -25,4 +45,28 @@ describe("ConditionalPublicModelName", () => { expect(screen.getByText("Public Model Name")).toBeInTheDocument(); expect(screen.getByText("LiteLLM Model Name")).toBeInTheDocument(); }); + + it("settles after rewriting the custom placeholder mapping to the entered model name", () => { + render( + + + + , + ); + + expect(screen.getByDisplayValue("my-custom-model")).toBeInTheDocument(); + expect(screen.getByText("my-custom-model")).toBeInTheDocument(); + expect(screen.queryByDisplayValue("custom")).not.toBeInTheDocument(); + }); }); diff --git a/ui/litellm-dashboard/src/components/add_model/conditional_public_model_name.tsx b/ui/litellm-dashboard/src/components/add_model/conditional_public_model_name.tsx index 3dc2d781922..b9c128a3d51 100644 --- a/ui/litellm-dashboard/src/components/add_model/conditional_public_model_name.tsx +++ b/ui/litellm-dashboard/src/components/add_model/conditional_public_model_name.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from "react"; +import React, { useEffect, useMemo } from "react"; import type { ColumnDef } from "@tanstack/react-table"; import { useFormContext, useWatch } from "react-hook-form"; import { DataTable } from "@/components/shared/DataTable"; @@ -13,6 +13,13 @@ interface ModelMapping { litellm_model: string; } +const sameMappings = (left: readonly ModelMapping[], right: readonly ModelMapping[]): boolean => + left.length === right.length && + left.every( + (mapping, index) => + mapping.public_name === right[index].public_name && mapping.litellm_model === right[index].litellm_model, + ); + const modelMappingsRule = { validator: async (_: unknown, value: unknown) => { if (!value || (value as ModelMapping[]).length === 0) { @@ -29,15 +36,14 @@ const modelMappingsRule = { const ConditionalPublicModelName: React.FC = () => { const form = useFormContext(); - const [tableKey, setTableKey] = useState(0); // Add a key to force table re-render - // Watch the 'model' field for changes and ensure it's always an array const modelValue = useWatch({ control: form.control, name: "model" }) || []; - const selectedModels = Array.isArray(modelValue) ? modelValue : [modelValue]; + const selectionKey = JSON.stringify(Array.isArray(modelValue) ? modelValue : [modelValue]); + const selectedModels = useMemo(() => JSON.parse(selectionKey) as string[], [selectionKey]); const customModelName = useWatch({ control: form.control, name: "custom_model_name" }) as string | undefined; const showPublicModelName = !selectedModels.includes("all-wildcard"); const selectedProvider = useWatch({ control: form.control, name: "custom_llm_provider" }); - // Force table to re-render when custom model name changes + useEffect(() => { if (customModelName && selectedModels.includes("custom")) { const currentMappings = (form.getValues("model_mappings") as ModelMapping[]) || []; @@ -56,8 +62,9 @@ const ConditionalPublicModelName: React.FC = () => { } return mapping; }); - form.setValue("model_mappings", updatedMappings); - setTableKey((prev) => prev + 1); // Force table re-render + if (!sameMappings(currentMappings, updatedMappings)) { + form.setValue("model_mappings", updatedMappings); + } } }, [customModelName, selectedModels, selectedProvider, form]); @@ -109,7 +116,6 @@ const ConditionalPublicModelName: React.FC = () => { }); form.setValue("model_mappings", mappings); - setTableKey((prev) => prev + 1); // Force table re-render } } }, [selectedModels, customModelName, selectedProvider, form]); @@ -210,7 +216,6 @@ const ConditionalPublicModelName: React.FC = () => { > {(control) => ( row.litellm_model}