From 3d3c6554fe5b0954f5013781d09423e932dbf81f Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Fri, 28 Aug 2026 10:03:35 -0700 Subject: [PATCH] refactor(ui): type search tool params from the generated schema (#38633) The search tool create and edit forms both built a payload carrying api_base, timeout and max_retries read off form values that neither zod schema declares, so all three were always undefined. Drop them. JSON.stringify omits undefined-valued keys, so the request body on the wire is unchanged. SearchToolLiteLLMParams and SearchToolInfo in the page's types.tsx were hand-rolled with a [key: string]: any index signature, which is why a param could go missing from a form with nothing complaining. SearchToolLiteLLMParams is now the generated OpenAPI component and neither type carries an index signature, so the payload builder can only set params the backend declares. Also remove a stray ", ]" text node that rendered as visible garbage next to the connection test dialog's Close button. --- ui/litellm-dashboard/CLAUDE.md | 2 ++ .../CreateSearchTools.integration.test.tsx | 3 --- .../_components/CreateSearchTools.tsx | 1 - .../SearchTools.integration.test.tsx | 9 ------- .../_components/searchToolPayload.test.ts | 27 ++++++------------- .../_components/searchToolPayload.ts | 18 +++---------- .../search-tools/_components/types.tsx | 12 +++------ .../src/lib/http/searchToolTypes.test-d.ts | 27 +++++++++++++++++++ 8 files changed, 44 insertions(+), 55 deletions(-) create mode 100644 ui/litellm-dashboard/src/lib/http/searchToolTypes.test-d.ts diff --git a/ui/litellm-dashboard/CLAUDE.md b/ui/litellm-dashboard/CLAUDE.md index f79258600c1..7b1234e1cf3 100644 --- a/ui/litellm-dashboard/CLAUDE.md +++ b/ui/litellm-dashboard/CLAUDE.md @@ -23,3 +23,5 @@ A test may reach for a component library's own CSS class only when that library Rules beyond the enabled set were measured against the whole suite and left off rather than recorded in a budget file, because a ceiling that permits a violation anywhere is worse than an honest gap. `no-node-access` and `no-container` are the ones worth revisiting first, since they catch the DOM archaeology the rules above only discourage. `prefer-implicit-assert` and `prefer-explicit-assert` contradict each other, so neither is enabled Never run the full unit suite (`npx vitest run` with no path). It is 380 files and thousands of tests, it saturates the machine for many minutes, and CI runs it anyway. Run only the test files your change touches, plus any file whose failure your change could plausibly explain, by passing explicit paths + +Type tests are `*.test-d.ts` files run by the `types` vitest project (`npm run test:types`). Keep them out of the `src/app/(dashboard)/` route group. Vitest matches a tsc error back to the test file by path, the parentheses break that match, and `ignoreSourceErrors: true` then drops the error as if it came from a source file. The test still collects and still reports as passing, so a `.test-d.ts` under a parenthesized directory is green no matter what it asserts. Confirm any new one has teeth by breaking the type it guards and watching it fail diff --git a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/CreateSearchTools.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/CreateSearchTools.integration.test.tsx index 8933b773c57..b7962321333 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/CreateSearchTools.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/CreateSearchTools.integration.test.tsx @@ -69,9 +69,6 @@ describe("CreateSearchTools submit payload", () => { litellm_params: { search_provider: "perplexity", api_key: "sk-secret", - api_base: undefined, - timeout: undefined, - max_retries: undefined, }, search_tool_info: { description: "finds things" }, }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/CreateSearchTools.tsx b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/CreateSearchTools.tsx index d58edd84f91..a724d979af5 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/CreateSearchTools.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/CreateSearchTools.tsx @@ -345,7 +345,6 @@ const CreateSearchTool: React.FC = ({ > Close - , ] diff --git a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/SearchTools.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/SearchTools.integration.test.tsx index 5073c721e08..5dc69d5660c 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/SearchTools.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/SearchTools.integration.test.tsx @@ -95,9 +95,6 @@ describe("SearchTools edit payload", () => { litellm_params: { search_provider: "perplexity", api_key: "sk-test-key", - api_base: undefined, - timeout: undefined, - max_retries: undefined, }, search_tool_info: { description: "Test description" }, }); @@ -136,9 +133,6 @@ describe("SearchTools edit payload", () => { litellm_params: { search_provider: "perplexity", api_key: "sk-test-key", - api_base: undefined, - timeout: undefined, - max_retries: undefined, }, search_tool_info: undefined, }); @@ -181,9 +175,6 @@ describe("SearchTools edit payload", () => { litellm_params: { search_provider: "perplexity", api_key: null, - api_base: undefined, - timeout: undefined, - max_retries: undefined, }, search_tool_info: undefined, }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/searchToolPayload.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/searchToolPayload.test.ts index 73c9fa9ac60..9031b144ebc 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/searchToolPayload.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/searchToolPayload.test.ts @@ -14,15 +14,12 @@ describe("buildSearchToolPayload", () => { ); }); - it("keeps the full key set in the object even when the optional params are absent", () => { + it("builds only the params a form actually collects", () => { expect(buildSearchToolPayload(minimal)).toStrictEqual({ search_tool_name: "tool", litellm_params: { search_provider: "perplexity", api_key: undefined, - api_base: undefined, - timeout: undefined, - max_retries: undefined, }, search_tool_info: undefined, }); @@ -32,7 +29,7 @@ describe("buildSearchToolPayload", () => { expect(buildSearchToolPayload({ ...minimal, api_key: "sk-secret" }).litellm_params.api_key).toBe("sk-secret"); }); - it("keeps an explicitly emptied api key as an empty string, matching the antd store", () => { + it("keeps an explicitly emptied api key as an empty string, so the backend clears it", () => { expect(buildSearchToolPayload({ ...minimal, api_key: "" }).litellm_params.api_key).toBe(""); }); @@ -46,19 +43,11 @@ describe("buildSearchToolPayload", () => { expect(buildSearchToolPayload({ ...minimal, description: "" }).search_tool_info).toBeUndefined(); }); - it("parses timeout as a float", () => { - expect(buildSearchToolPayload({ ...minimal, timeout: "2.5" }).litellm_params.timeout).toBe(2.5); - }); - - it("parses max_retries as an integer and truncates a decimal", () => { - expect(buildSearchToolPayload({ ...minimal, max_retries: "3.9" }).litellm_params.max_retries).toBe(3); - }); - - it('parses a "0" timeout as 0, because the original guard tests the string not the number', () => { - expect(buildSearchToolPayload({ ...minimal, timeout: "0" }).litellm_params.timeout).toBe(0); - }); - - it("treats an empty timeout string as absent", () => { - expect(buildSearchToolPayload({ ...minimal, timeout: "" }).litellm_params.timeout).toBeUndefined(); + it("sends the same wire body with an api key and a description as it did before the params were pruned", () => { + expect( + JSON.stringify(buildSearchToolPayload({ ...minimal, api_key: "sk-secret", description: "finds things" })), + ).toBe( + '{"search_tool_name":"tool","litellm_params":{"search_provider":"perplexity","api_key":"sk-secret"},"search_tool_info":{"description":"finds things"}}', + ); }); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/searchToolPayload.ts b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/searchToolPayload.ts index b6a32ee7eb2..10573292c82 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/searchToolPayload.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/searchToolPayload.ts @@ -1,23 +1,16 @@ +import type { SearchToolInfo, SearchToolLiteLLMParams } from "./types"; + export interface SearchToolFormValues { search_tool_name: string; search_provider: string; api_key?: string | null; - api_base?: string; - timeout?: string; - max_retries?: string; description?: string | null; } export interface SearchToolPayload { search_tool_name: string; - litellm_params: { - search_provider: string; - api_key: string | null | undefined; - api_base: string | undefined; - timeout: number | undefined; - max_retries: number | undefined; - }; - search_tool_info: { description: string } | undefined; + litellm_params: SearchToolLiteLLMParams; + search_tool_info: SearchToolInfo | undefined; } export const buildSearchToolPayload = (values: SearchToolFormValues): SearchToolPayload => ({ @@ -25,9 +18,6 @@ export const buildSearchToolPayload = (values: SearchToolFormValues): SearchTool litellm_params: { search_provider: values.search_provider, api_key: values.api_key, - api_base: values.api_base, - timeout: values.timeout ? parseFloat(values.timeout) : undefined, - max_retries: values.max_retries ? parseInt(values.max_retries, 10) : undefined, }, search_tool_info: values.description ? { description: values.description } : undefined, }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/types.tsx b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/types.tsx index 3cd2bb93e79..db0050ce868 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/types.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/search-tools/_components/types.tsx @@ -1,15 +1,9 @@ -export interface SearchToolLiteLLMParams { - search_provider: string; - api_key?: string | null; - api_base?: string; - timeout?: number; - max_retries?: number; - [key: string]: any; -} +import type { components } from "@/lib/http/schema"; + +export type SearchToolLiteLLMParams = components["schemas"]["SearchToolLiteLLMParams"]; export interface SearchToolInfo { description?: string | null; - [key: string]: any; } export interface SearchTool { diff --git a/ui/litellm-dashboard/src/lib/http/searchToolTypes.test-d.ts b/ui/litellm-dashboard/src/lib/http/searchToolTypes.test-d.ts new file mode 100644 index 00000000000..9eac7fd3b28 --- /dev/null +++ b/ui/litellm-dashboard/src/lib/http/searchToolTypes.test-d.ts @@ -0,0 +1,27 @@ +import { describe, expectTypeOf, test } from "vitest"; + +import type { components } from "@/lib/http/schema"; +import type { SearchToolInfo, SearchToolLiteLLMParams } from "@/app/(dashboard)/search-tools/_components/types"; +import type { SearchToolPayload } from "@/app/(dashboard)/search-tools/_components/searchToolPayload"; + +describe("search tool types", () => { + test("litellm params are the generated OpenAPI component", () => { + expectTypeOf().toEqualTypeOf(); + }); + + test("a param the backend has not declared does not type-check", () => { + // @ts-expect-error search_engine_id type-checks only once litellm/types/search.py declares it + const params: SearchToolLiteLLMParams = { search_provider: "google_pse", search_engine_id: "cx-123" }; + expectTypeOf(params).toExtend<{ search_provider: string }>(); + }); + + test("tool info carries a description and nothing else", () => { + // @ts-expect-error search_tool_info has no owner field + const info: SearchToolInfo = { description: "finds things", owner: "platform-team" }; + expectTypeOf(info).toExtend<{ description?: string | null }>(); + }); + + test("the payload sends litellm params the backend declares", () => { + expectTypeOf().toEqualTypeOf(); + }); +});