From f8ddb209656f4bb67429de47d3dd5af8e60fc0b5 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:21:09 -0700 Subject: [PATCH] fix(ui): render team_metadata_schema keys as fixed labels (#41482) * fix(ui): render team_metadata_schema keys as fixed labels Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(ui): update team metadata schema tests for fixed labels Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(ui): derive team metadata schema labels from live key values Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: jesus Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../src/components/Teams.test.tsx | 32 +-- .../MetadataKeyValueFields.test.tsx | 208 +++++++++++++++--- .../MetadataKeyValueFields.tsx | 109 +++++++-- .../src/components/team/TeamInfo.test.tsx | 18 +- 4 files changed, 291 insertions(+), 76 deletions(-) diff --git a/ui/litellm-dashboard/src/components/Teams.test.tsx b/ui/litellm-dashboard/src/components/Teams.test.tsx index 3b89ebdd318..85c42b2d09b 100644 --- a/ui/litellm-dashboard/src/components/Teams.test.tsx +++ b/ui/litellm-dashboard/src/components/Teams.test.tsx @@ -892,16 +892,16 @@ describe("Teams - schema-declared metadata fields in team create", () => { }); }; - it("should prepopulate the declared key as an ordinary pair row and submit its value", async () => { + it("should show the declared key as a fixed label and submit its value under the declared key", async () => { await openCreateModal(); fireEvent.change(screen.getByLabelText(/team name/i), { target: { value: "Test Team" } }); fireEvent.change(screen.getByTestId("create-team-models-select"), { target: { value: "gpt-4" } }); - await waitFor(() => { - expect((screen.getByPlaceholderText("Key") as HTMLInputElement).value).toBe("cost_center"); - }); - fireEvent.change(screen.getByPlaceholderText("Value"), { target: { value: "CC-1001" } }); + expect(await screen.findByTestId("metadata-schema-label")).toHaveTextContent("Cost Center"); + expect(screen.queryByPlaceholderText("Key")).not.toBeInTheDocument(); + expect(screen.queryByLabelText("Remove key-value pair")).not.toBeInTheDocument(); + fireEvent.change(screen.getByLabelText("Cost Center"), { target: { value: "CC-1001" } }); const createTeamSubmitButtons = screen.getAllByRole("button", { name: /create team/i }); fireEvent.click(createTeamSubmitButtons[createTeamSubmitButtons.length - 1]); @@ -922,10 +922,7 @@ describe("Teams - schema-declared metadata fields in team create", () => { fireEvent.change(screen.getByLabelText(/team name/i), { target: { value: "Test Team" } }); fireEvent.change(screen.getByTestId("create-team-models-select"), { target: { value: "gpt-4" } }); - await waitFor(() => { - expect((screen.getByPlaceholderText("Key") as HTMLInputElement).value).toBe("cost_center"); - }); - fireEvent.change(screen.getByPlaceholderText("Value"), { target: { value: "CC-9999" } }); + fireEvent.change(await screen.findByLabelText("Cost Center"), { target: { value: "CC-9999" } }); const createTeamSubmitButtons = screen.getAllByRole("button", { name: /create team/i }); fireEvent.click(createTeamSubmitButtons[createTeamSubmitButtons.length - 1]); @@ -945,16 +942,12 @@ describe("Teams - schema-declared metadata fields in team create", () => { expect(screen.queryByRole("button", { name: /add key-value pair/i })).not.toBeInTheDocument(); }); - it("should re-seed declared keys when the create modal is closed and reopened", async () => { + it("should re-seed the declared key and drop free-form rows when the create modal is closed and reopened", async () => { await openCreateModal(); - await waitFor(() => { - expect((screen.getByPlaceholderText("Key") as HTMLInputElement).value).toBe("cost_center"); - }); - fireEvent.click(screen.getByLabelText("Remove key-value pair")); - await waitFor(() => { - expect(screen.queryByPlaceholderText("Key")).not.toBeInTheDocument(); - }); + fireEvent.change(await screen.findByLabelText("Cost Center"), { target: { value: "CC-1001" } }); + fireEvent.click(screen.getByRole("button", { name: /add key-value pair/i })); + fireEvent.change(await screen.findByPlaceholderText("Key"), { target: { value: "region" } }); fireEvent.click(screen.getByRole("button", { name: /^close$/i })); await waitFor(() => { @@ -966,9 +959,8 @@ describe("Teams - schema-declared metadata fields in team create", () => { fireEvent.click(createButton); }); - await waitFor(() => { - expect((screen.getByPlaceholderText("Key") as HTMLInputElement).value).toBe("cost_center"); - }); + expect(await screen.findByLabelText("Cost Center")).toHaveValue(""); + expect(screen.queryByPlaceholderText("Key")).not.toBeInTheDocument(); }); }); diff --git a/ui/litellm-dashboard/src/components/common_components/MetadataKeyValueFields.test.tsx b/ui/litellm-dashboard/src/components/common_components/MetadataKeyValueFields.test.tsx index f62cd418775..5b26cd592b0 100644 --- a/ui/litellm-dashboard/src/components/common_components/MetadataKeyValueFields.test.tsx +++ b/ui/litellm-dashboard/src/components/common_components/MetadataKeyValueFields.test.tsx @@ -101,21 +101,24 @@ interface HarnessProps { initialMetadata?: MetadataPair[]; schemaFields?: TeamMetadataField[]; schemaLoading?: boolean; + fieldsHidden?: boolean; } const harnessSchema = z.object({ metadata: metadataPairsSchema }); -const Harness: React.FC = ({ onFinish, initialMetadata, schemaFields, schemaLoading }) => { +const Harness: React.FC = ({ onFinish, initialMetadata, schemaFields, schemaLoading, fieldsHidden }) => { const form = useZodForm(harnessSchema, { defaultValues: { metadata: initialMetadata ?? [] } }); return (
onFinish(values))}> - + {!fieldsHidden && ( + + )} ); @@ -218,17 +221,41 @@ describe("MetadataKeyValueFields with a declared schema", () => { { key: "app_name", label: "Application Name" }, ]; - it("should prepopulate one ordinary editable pair row per declared key", async () => { + it("should render each declared key as a fixed label with only the value editable", async () => { render(); await waitFor(() => { - expect(screen.getAllByPlaceholderText("Key").map((input) => (input as HTMLInputElement).value)).toEqual([ - "cost_center", - "app_name", + expect(screen.getAllByTestId("metadata-schema-label").map((label) => label.textContent)).toEqual([ + "Cost Center", + "Application Name", ]); }); - screen.getAllByPlaceholderText("Key").forEach((input) => expect(input).toBeEnabled()); - expect(screen.getAllByLabelText("Remove key-value pair")).toHaveLength(2); + expect(screen.queryByPlaceholderText("Key")).not.toBeInTheDocument(); + expect(screen.queryByLabelText("Remove key-value pair")).not.toBeInTheDocument(); + expect(screen.getByLabelText("Cost Center")).toHaveAttribute("placeholder", "Value"); + }); + + it("should fall back to the key when a declared field has no label", async () => { + render(); + + expect(await screen.findByTestId("metadata-schema-label")).toHaveTextContent("cost_center"); + }); + + it("should treat an existing pair that matches a declared key as fixed too", async () => { + render( + , + ); + + expect(await screen.findByLabelText("Cost Center")).toHaveValue("CC-1001"); + expect(screen.getAllByPlaceholderText("Key").map((input) => (input as HTMLInputElement).value)).toEqual(["region"]); + expect(screen.getAllByLabelText("Remove key-value pair")).toHaveLength(1); }); it("should submit a prepopulated key with its typed value", async () => { @@ -250,9 +277,9 @@ describe("MetadataKeyValueFields with a declared schema", () => { ); await waitFor(() => { - expect(screen.getAllByPlaceholderText("Key").map((input) => (input as HTMLInputElement).value)).toEqual([ - "cost_center", - "app_name", + expect(screen.getAllByTestId("metadata-schema-label").map((label) => label.textContent)).toEqual([ + "Cost Center", + "Application Name", ]); }); expect(screen.getAllByPlaceholderText("Value").map((input) => (input as HTMLInputElement).value)).toEqual([ @@ -261,18 +288,147 @@ describe("MetadataKeyValueFields with a declared schema", () => { ]); }); - it("should let the user remove a prepopulated row", async () => { + it("should still let the user add and remove free-form pairs below the declared keys", async () => { const user = userEvent.setup(); - render(); + const onFinish = vi.fn(); + render(); - await screen.findAllByPlaceholderText("Key"); - await user.click(screen.getAllByLabelText("Remove key-value pair")[0]); + await screen.findByTestId("metadata-schema-label"); + await user.click(screen.getByRole("button", { name: /add key-value pair/i })); + fireEvent.change(screen.getByPlaceholderText("Key"), { target: { value: "region" } }); + fireEvent.change(screen.getAllByPlaceholderText("Value")[1], { target: { value: "us" } }); + fireEvent.change(screen.getByLabelText("Cost Center"), { target: { value: "CC-1001" } }); + await user.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { - expect(screen.getAllByPlaceholderText("Key").map((input) => (input as HTMLInputElement).value)).toEqual([ - "app_name", + expect(onFinish).toHaveBeenCalledWith({ + metadata: [ + { key: "cost_center", value: "CC-1001" }, + { key: "region", value: "us" }, + ], + }); + }); + + await user.click(screen.getByLabelText("Remove key-value pair")); + await waitFor(() => { + expect(screen.queryByPlaceholderText("Key")).not.toBeInTheDocument(); + }); + expect(screen.getByTestId("metadata-schema-label")).toBeInTheDocument(); + }); + + it("should keep duplicate keys in a free-form row", async () => { + const user = userEvent.setup(); + const onFinish = vi.fn(); + render(); + + expect(await screen.findByTestId("metadata-schema-label")).toHaveTextContent("Cost Center"); + await user.click(screen.getByRole("button", { name: /add key-value pair/i })); + fireEvent.change(screen.getByPlaceholderText("Key"), { target: { value: "cost_center" } }); + + expect(screen.getAllByPlaceholderText("Key")).toHaveLength(1); + expect(screen.getByPlaceholderText("Key")).toHaveValue("cost_center"); + expect(screen.getAllByLabelText("Remove key-value pair")).toHaveLength(1); + expect(screen.getAllByTestId("metadata-schema-label")).toHaveLength(1); + + await user.click(screen.getByRole("button", { name: "Save" })); + + await waitFor(() => { + expect(screen.getByText("Duplicate key")).toBeInTheDocument(); + }); + expect(onFinish).not.toHaveBeenCalled(); + }); + + it("should keep an existing row above a declared row editable while a declared key is typed into it", async () => { + const user = userEvent.setup(); + const onFinish = vi.fn(); + render( + , + ); + await waitFor(() => { + expect(screen.getAllByTestId("metadata-schema-label").map((label) => label.textContent)).toEqual([ + "Cost", + "Cost Center", ]); }); + + const keyInput = screen.getByPlaceholderText("Key"); + await user.clear(keyInput); + await user.type(keyInput, "cost_center"); + + expect(keyInput).toHaveValue("cost_center"); + expect(keyInput).toHaveFocus(); + + await user.click(screen.getByRole("button", { name: /add key-value pair/i })); + expect(screen.getAllByPlaceholderText("Key").map((input) => (input as HTMLInputElement).value)).toEqual([ + "cost_center", + "", + ]); + await user.click(screen.getAllByLabelText("Remove key-value pair")[1]); + + expect(screen.getAllByPlaceholderText("Key").map((input) => (input as HTMLInputElement).value)).toEqual([ + "cost_center", + ]); + expect(screen.getAllByLabelText("Remove key-value pair")).toHaveLength(1); + expect(screen.getAllByTestId("metadata-schema-label").map((label) => label.textContent)).toEqual([ + "Cost", + "Cost Center", + ]); + + await user.click(screen.getByRole("button", { name: "Save" })); + + expect(await screen.findByText("Duplicate key")).toBeInTheDocument(); + expect(onFinish).not.toHaveBeenCalled(); + }); + + it("should keep one editable row per duplicated declared key when the editor remounts", async () => { + const user = userEvent.setup(); + const onFinish = vi.fn(); + const schemaFields = [{ key: "cost_center", label: "Cost Center" }]; + const initialMetadata = [{ key: "region", value: "us" }]; + const { rerender } = render( + , + ); + expect(await screen.findByTestId("metadata-schema-label")).toHaveTextContent("Cost Center"); + fireEvent.change(screen.getByPlaceholderText("Key"), { target: { value: "cost_center" } }); + + rerender( + , + ); + rerender(); + + expect(screen.getAllByTestId("metadata-schema-label")).toHaveLength(1); + expect(screen.getByPlaceholderText("Key")).toHaveValue("cost_center"); + expect(screen.getAllByLabelText("Remove key-value pair")).toHaveLength(1); + + await user.click(screen.getByRole("button", { name: "Save" })); + + expect(await screen.findByText("Duplicate key")).toBeInTheDocument(); + expect(onFinish).not.toHaveBeenCalled(); + }); + + it("should label only rows whose current key is declared when the schema arrives after an edit", () => { + const onFinish = vi.fn(); + const initialMetadata = [{ key: "cost_center", value: "1" }]; + const { rerender } = render(); + fireEvent.change(screen.getByPlaceholderText("Key"), { target: { value: "region" } }); + + rerender( + , + ); + + expect(screen.getByPlaceholderText("Key")).toHaveValue("region"); + expect(screen.getAllByTestId("metadata-schema-label").map((label) => label.textContent)).toEqual(["Cost Center"]); }); it("should show a skeleton instead of the editor while the schema is loading", () => { @@ -289,9 +445,9 @@ describe("MetadataKeyValueFields with a declared schema", () => { rerender(); await waitFor(() => { - expect(screen.getAllByPlaceholderText("Key").map((input) => (input as HTMLInputElement).value)).toEqual([ - "cost_center", - "app_name", + expect(screen.getAllByTestId("metadata-schema-label").map((label) => label.textContent)).toEqual([ + "Cost Center", + "Application Name", ]); }); }); diff --git a/ui/litellm-dashboard/src/components/common_components/MetadataKeyValueFields.tsx b/ui/litellm-dashboard/src/components/common_components/MetadataKeyValueFields.tsx index baa154ffb3b..22862b5f6eb 100644 --- a/ui/litellm-dashboard/src/components/common_components/MetadataKeyValueFields.tsx +++ b/ui/litellm-dashboard/src/components/common_components/MetadataKeyValueFields.tsx @@ -1,5 +1,5 @@ import { CircleMinus, Plus } from "lucide-react"; -import React, { useEffect, useRef } from "react"; +import React, { useEffect, useRef, useState } from "react"; import { useFieldArray, type Control, @@ -14,6 +14,7 @@ import { TeamMetadataField } from "@/app/(dashboard)/hooks/teams/useTeamMetadata import { FormField } from "@/components/shared/form/FormField"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; import { Skeleton } from "@/components/ui/skeleton"; export interface MetadataPair { @@ -78,6 +79,64 @@ interface MetadataKeyValueFieldsProps { schemaLoading?: boolean; } +interface MetadataRowProps { + control: Control; + name: FieldArrayPath; + index: number; + rowId: string; + schemaLabel: string | undefined; + onRemove: () => void; +} + +const MetadataRow = ({ + control, + name, + index, + rowId, + schemaLabel, + onRemove, +}: MetadataRowProps) => { + return ( +
+ {schemaLabel === undefined ? ( + }> + {({ ref, value, ...rest }) => } + + ) : ( + + )} + }> + {({ ref, value, id, ...rest }) => ( + + )} + + {schemaLabel === undefined && ( + + )} +
+ ); +}; + const MetadataKeyValueFields = ({ control, getValues, @@ -87,6 +146,24 @@ const MetadataKeyValueFields = ({ }: MetadataKeyValueFieldsProps) => { const { fields, append, remove } = useFieldArray({ control, name }); const seededRef = useRef(false); + const schemaLabelsByKey = new Map(schemaFields.map((field) => [field.key, field.label || field.key])); + const schemaReady = !schemaLoading && schemaFields.length > 0; + const livePairs: readonly (Partial | undefined)[] = + getValues(name as unknown as FieldPath) ?? []; + const [keysAtMount, setKeysAtMount] = useState>(() => new Map()); + const unseenKeys = schemaReady + ? fields.flatMap((field, index) => (keysAtMount.has(field.id) ? [] : [[field.id, livePairs[index]?.key] as const])) + : []; + if (unseenKeys.length > 0) { + setKeysAtMount(new Map([...keysAtMount, ...unseenKeys])); + } + const rowKeysAtMount = fields.map((field, index) => + keysAtMount.has(field.id) || !schemaReady ? keysAtMount.get(field.id) : livePairs[index]?.key, + ); + const schemaLabelAt = (index: number): string | undefined => { + const key = rowKeysAtMount[index]; + return key === undefined || rowKeysAtMount.indexOf(key) !== index ? undefined : schemaLabelsByKey.get(key); + }; useEffect(() => { if (seededRef.current || schemaLoading || schemaFields.length === 0) return; @@ -115,27 +192,15 @@ const MetadataKeyValueFields = ({ return ( <> {fields.map((field, index) => ( -
- }> - {({ ref, value, ...rest }) => ( - - )} - - }> - {({ ref, value, ...rest }) => ( - - )} - - -
+ remove(index)} + /> ))}