From 2ff503f606020a3467a17d8bbbd39f06d3665c72 Mon Sep 17 00:00:00 2001 From: Ryan Crabbe Date: Sat, 16 May 2026 15:03:22 -0700 Subject: [PATCH] refactor(ui): simplify model auth editing; fix stale credential branch Drop the onFieldsResolved/authFieldKeys round trip: the parent now resolves provider auth field keys itself via the new useProviderAuthFieldKeys hook (same metadata ProviderSpecificFields renders), removing the report-up effect and its stable-reference footgun. ProviderSpecificFields keeps only excludeKeys (real need: suppress duplicate visible inputs). Fix the stale Authentication branch: derive it from the live litellm_credential_name form value (Form.useWatch) instead of the server snapshot, so clearing/adding a credential mid-edit shows the right UI. Also skip inline auth updates entirely when a named credential is selected, so we never submit a credential name and raw inline auth together. --- .../provider_specific_fields.test.tsx | 43 +++++++------ .../add_model/provider_specific_fields.tsx | 48 +++++++++++---- .../src/components/model_info_view.tsx | 60 ++++++++++++------- 3 files changed, 96 insertions(+), 55 deletions(-) diff --git a/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.test.tsx b/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.test.tsx index 8b0057a1ffe..51fbc7ee54b 100644 --- a/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.test.tsx +++ b/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.test.tsx @@ -1,9 +1,10 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { render, screen, waitFor } from "@testing-library/react"; +import { render, renderHook, screen, waitFor } from "@testing-library/react"; import { Form } from "antd"; +import React from "react"; import { beforeAll, describe, expect, it, vi } from "vitest"; import { Providers } from "../provider_info_helpers"; -import ProviderSpecificFields from "./provider_specific_fields"; +import ProviderSpecificFields, { useProviderAuthFieldKeys } from "./provider_specific_fields"; vi.mock("../networking", async () => { const actual = await vi.importActual("../networking"); @@ -183,17 +184,12 @@ describe("ProviderSpecificFields", () => { }); }); - it("excludeKeys removes parent-owned fields and reports only the remaining keys", async () => { + it("excludeKeys removes parent-owned fields from the rendered set", async () => { const queryClient = createQueryClient(); - const onFieldsResolved = vi.fn(); render(
- +
, ); @@ -204,22 +200,31 @@ describe("ProviderSpecificFields", () => { }); expect(screen.queryByPlaceholderText("https://api.openai.com/v1")).not.toBeInTheDocument(); expect(screen.queryByPlaceholderText("[OPTIONAL] my-unique-org")).not.toBeInTheDocument(); - expect(onFieldsResolved).toHaveBeenCalledWith(["api_key"]); }); - it("onFieldsResolved reports the full key set when nothing is excluded", async () => { + it("useProviderAuthFieldKeys returns the full key set when nothing is excluded", async () => { const queryClient = createQueryClient(); - const onFieldsResolved = vi.fn(); - render( - -
- - -
, + const wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} ); + const { result } = renderHook(() => useProviderAuthFieldKeys(Providers.OpenAI), { wrapper }); await waitFor(() => { - expect(onFieldsResolved).toHaveBeenCalledWith(["api_base", "organization", "api_key"]); + expect(result.current).toEqual(["api_base", "organization", "api_key"]); + }); + }); + + it("useProviderAuthFieldKeys excludes parent-owned keys", async () => { + const queryClient = createQueryClient(); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + const { result } = renderHook(() => useProviderAuthFieldKeys(Providers.OpenAI, ["api_base", "organization"]), { + wrapper, + }); + + await waitFor(() => { + expect(result.current).toEqual(["api_key"]); }); }); diff --git a/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.tsx b/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.tsx index 440dc775fd2..3fbad31db78 100644 --- a/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.tsx +++ b/ui/litellm-dashboard/src/components/add_model/provider_specific_fields.tsx @@ -3,7 +3,7 @@ import { UploadOutlined } from "@ant-design/icons"; import { Text, TextInput } from "@tremor/react"; import { Button as Button2, Col, Form, Input, Row, Select, Typography, Upload, UploadProps } from "antd"; import React from "react"; -import { CredentialItem, ProviderCredentialFieldMetadata } from "../networking"; +import { CredentialItem, ProviderCreateInfo, ProviderCredentialFieldMetadata } from "../networking"; import { provider_map, Providers } from "../provider_info_helpers"; const { Link } = Typography; @@ -14,9 +14,6 @@ interface ProviderSpecificFieldsProps { // inputs (e.g. the model edit form has a dedicated "API Base" field) so we // don't create a duplicate Form.Item bound to the same name. excludeKeys?: string[]; - // Called whenever the set of rendered field keys changes, so a parent can - // know exactly which form values belong to this provider's auth fields. - onFieldsResolved?: (keys: string[]) => void; } interface ProviderCredentialField { @@ -59,6 +56,38 @@ const mapFieldMetadataToUiField = (field: ProviderCredentialFieldMetadata): Prov }; }; +// Resolve the credential UI fields for a provider from fetched metadata. +// Matches the component's resolution order (display-name enum or raw slug). +const resolveProviderFields = ( + providerMetadata: ProviderCreateInfo[] | undefined, + selectedProvider: Providers, +): ProviderCredentialField[] => { + if (!providerMetadata) return []; + const selectedProviderEnum = Providers[selectedProvider as keyof typeof Providers] as Providers; + const providerInfo = providerMetadata.find( + (p) => + p.provider_display_name === selectedProviderEnum || + p.provider === selectedProvider || + p.litellm_provider === selectedProvider, + ); + return providerInfo ? providerInfo.credential_fields.map(mapFieldMetadataToUiField) : []; +}; + +// The auth-field keys a parent form should harvest for this provider, minus +// any keys the parent already owns (excludeKeys). Lets the parent collect the +// right form values on submit without the child reporting them back upward. +export const useProviderAuthFieldKeys = (selectedProvider: Providers, excludeKeys?: string[]): string[] => { + const { data: providerMetadata } = useProviderFields(); + const excludeDep = (excludeKeys ?? []).join("\0"); + return React.useMemo(() => { + const excludeSet = new Set(excludeKeys ?? []); + return resolveProviderFields(providerMetadata, selectedProvider) + .map((f) => f.key) + .filter((k) => !excludeSet.has(k)); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [providerMetadata, selectedProvider, excludeDep]); +}; + // In-memory cache of provider credential fields keyed by provider display name. // This lets us reuse the data across multiple mounts and also supports // non-React helpers like createCredentialFromModel. @@ -103,7 +132,6 @@ const ProviderSpecificFields: React.FC = ({ selectedProvider, uploadProps, excludeKeys, - onFieldsResolved, }) => { const selectedProviderEnum = Providers[selectedProvider as keyof typeof Providers] as Providers; const form = Form.useFormInstance(); // Get form instance from context @@ -179,20 +207,14 @@ const ProviderSpecificFields: React.FC = ({ return mapped; }, [selectedProviderEnum, selectedProvider, providerMetadata]); - const excludeKeySet = React.useMemo(() => new Set(excludeKeys ?? []), [excludeKeys?.join(",")]); + // eslint-disable-next-line react-hooks/exhaustive-deps + const excludeKeySet = React.useMemo(() => new Set(excludeKeys ?? []), [(excludeKeys ?? []).join("\0")]); const visibleFields = React.useMemo( () => (excludeKeySet.size > 0 ? allFields.filter((f) => !excludeKeySet.has(f.key)) : allFields), [allFields, excludeKeySet], ); - // Report the rendered field keys upward (string-joined so the effect only - // fires when the actual set changes, not on every parent render). - const visibleKeyList = React.useMemo(() => visibleFields.map((f) => f.key).join(","), [visibleFields]); - React.useEffect(() => { - onFieldsResolved?.(visibleKeyList ? visibleKeyList.split(",") : []); - }, [visibleKeyList, onFieldsResolved]); - const handleUpload = { name: "file", accept: ".json", diff --git a/ui/litellm-dashboard/src/components/model_info_view.tsx b/ui/litellm-dashboard/src/components/model_info_view.tsx index 95347a4c850..a36f19569ef 100644 --- a/ui/litellm-dashboard/src/components/model_info_view.tsx +++ b/ui/litellm-dashboard/src/components/model_info_view.tsx @@ -40,7 +40,7 @@ import { testConnectionRequest, } from "./networking"; import { getProviderLogoAndName, Providers } from "./provider_info_helpers"; -import ProviderSpecificFields from "./add_model/provider_specific_fields"; +import ProviderSpecificFields, { useProviderAuthFieldKeys } from "./add_model/provider_specific_fields"; import NumericalInput from "./shared/numerical_input"; import { Tag } from "./tag_management/types"; import { getDisplayModelName } from "./view_model/model_name_display"; @@ -55,6 +55,11 @@ interface ModelInfoViewProps { modelAccessGroups: string[] | null; } +// Auth fields the model edit form already renders as dedicated inputs — kept +// out of the inline provider auth section so we don't bind two Form.Items to +// the same name or submit a duplicate value. +const AUTH_FIELD_EXCLUDE_KEYS = ["api_base", "organization", "custom_llm_provider"]; + export default function ModelInfoView({ modelId, onClose, @@ -79,9 +84,6 @@ export default function ModelInfoView({ const [guardrailsList, setGuardrailsList] = useState([]); const [tagsList, setTagsList] = useState>({}); const [credentialsList, setCredentialsList] = useState([]); - // Provider auth-field keys reported by ProviderSpecificFields, so the save - // handler knows which form values are this provider's credential fields. - const [authFieldKeys, setAuthFieldKeys] = useState([]); // Fetch model data using hook const { data: rawModelDataResponse, isLoading: isLoadingModel } = useModelsInfo(1, 50, undefined, modelId); @@ -118,6 +120,20 @@ export default function ModelInfoView({ modelData?.litellm_params?.litellm_credential_name != null && modelData?.litellm_params?.litellm_credential_name != undefined; + // Track the live credential dropdown so the Authentication section reacts to + // edits in this session (not just the server snapshot). When a named + // credential is selected the inline provider fields are hidden and never + // submitted, so we can't save a credential name and raw inline auth together. + const liveCredentialName = Form.useWatch("litellm_credential_name", form); + const isUsingCredentialInForm = isEditing ? !!liveCredentialName : usingExistingCredential; + + // Provider auth-field keys, resolved from the same metadata + // ProviderSpecificFields renders, minus the fields the edit form already + // owns. The save handler harvests exactly these from the form values. + const authProvider = (localModelData?.litellm_params?.custom_llm_provider || + modelData?.provider) as Providers; + const authFieldKeys = useProviderAuthFieldKeys(authProvider, AUTH_FIELD_EXCLUDE_KEYS); + // Initialize localModelData from modelData when available useEffect(() => { if (modelData && !localModelData) { @@ -248,14 +264,18 @@ export default function ModelInfoView({ // Provider auth fields the user actually entered. Blank/null/undefined // are omitted (never sent as "") so an untouched secret is preserved by // the backend's merge instead of being overwritten. Mirrors the submit - // filter used by EditCredentialModal. - const authFieldUpdates = authFieldKeys.reduce>((acc, key) => { - const v = values[key]; - if (v !== "" && v !== undefined && v !== null) { - acc[key] = v; - } - return acc; - }, {}); + // filter used by EditCredentialModal. Skipped entirely when a named + // credential is selected — that path owns auth, and sending both would + // leave conflicting config on the backend. + const authFieldUpdates = values.litellm_credential_name + ? {} + : authFieldKeys.reduce>((acc, key) => { + const v = values[key]; + if (v !== "" && v !== undefined && v !== null) { + acc[key] = v; + } + return acc; + }, {}); let updatedLitellmParams = { ...values.litellm_params, @@ -1084,13 +1104,11 @@ export default function ModelInfoView({ {isEditing && (
Authentication - {usingExistingCredential ? ( + {isUsingCredentialInForm ? (
This model uses the shared credential{" "} - - {localModelData.litellm_params?.litellm_credential_name} - - . Update its keys from the LLM Credentials tab. + {liveCredentialName}. Update its keys from the LLM + Credentials tab.
) : (
@@ -1098,12 +1116,8 @@ export default function ModelInfoView({ Leave a field blank to keep its current value. Enter a new value to rotate it.
)}