mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
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.
This commit is contained in:
parent
0eb7c3ad05
commit
3d3c6554fe
8 changed files with 44 additions and 55 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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" },
|
||||
});
|
||||
|
|
|
|||
|
|
@ -345,7 +345,6 @@ const CreateSearchTool: React.FC<CreateSearchToolProps> = ({
|
|||
>
|
||||
Close
|
||||
</Button>
|
||||
, ]
|
||||
</DialogFooter>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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"}}',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
27
ui/litellm-dashboard/src/lib/http/searchToolTypes.test-d.ts
Normal file
27
ui/litellm-dashboard/src/lib/http/searchToolTypes.test-d.ts
Normal file
|
|
@ -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<SearchToolLiteLLMParams>().toEqualTypeOf<components["schemas"]["SearchToolLiteLLMParams"]>();
|
||||
});
|
||||
|
||||
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<SearchToolPayload["litellm_params"]>().toEqualTypeOf<SearchToolLiteLLMParams>();
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue