From cb89c7aa8f8331e7bc45839a7c4e02d7f2449734 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Thu, 20 Aug 2026 16:42:49 -0700 Subject: [PATCH] fix(ui): stop the Add Model mapping table from looping the page (#37741) Entering a custom model name on the Add Model form crashed the whole page to "This page couldn't load" (React error #185, maximum update depth exceeded), taking the provider credential fields down with it, so the model could never be created. ConditionalPublicModelName kept a `tableKey` counter and bumped it from an effect on every run to force the mappings table to remount. That was harmless under antd, whose useWatch handed back the stored array. React Hook Form's useWatch returns a fresh array each render, so the effect's dependency changed every render, the effect bumped state again, and the render loop never settled. The table is driven by its `data` prop, so the remount counter buys nothing: drop it, key the effects off the selection contents rather than the array identity, and write model_mappings only when they actually change. The two `react-hooks/set-state-in-effect` suppressions on this file, which were recording exactly this bug, go with it. --- ui/litellm-dashboard/eslint-suppressions.json | 3 -- .../conditional_public_model_name.test.tsx | 44 +++++++++++++++++++ .../conditional_public_model_name.tsx | 23 ++++++---- 3 files changed, 58 insertions(+), 12 deletions(-) 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}