refactor(ui): validate model edit prices in the zod schema (#46978)

Each price field now parses its per 1M text into a per token number or null inside the
schema, so the save handler gets clean values and the hand-rolled conversion goes away.
Negative or non-numeric prices now show a field error instead of being saved
This commit is contained in:
ryan-crabbe-berri 2026-10-16 15:25:21 -07:00 • committed by GitHub
parent b39eadcb82
commit cc24e7cfa4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 167 additions and 82 deletions

View file

@ -1,10 +1,10 @@
import { render, screen } from "@testing-library/react";
import { fireEvent, render, screen } from "@testing-library/react";
import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event";
import { describe, expect, it, vi } from "vitest";
import type { CredentialItem } from "@/components/networking";
import ModelInfoEditForm from "./ModelInfoEditForm";
import ModelInfoEditForm, { perMillionToPerToken } from "./ModelInfoEditForm";
vi.mock("@/components/networking", async (importOriginal) => {
const actual = await importOriginal<typeof import("@/components/networking")>();
@ -55,6 +55,69 @@ const renderForm = ({
/>,
);
describe("perMillionToPerToken", () => {
it.each([
["2.5", 0.0000025],
[3, 0.000003],
["0", 0],
["", null],
[" ", null],
[null, null],
[undefined, null],
])("parses %j into %j per token", (perMillion, perToken) => {
expect(perMillionToPerToken.parse(perMillion)).toBe(perToken);
});
it.each(["abc", "-1", -0.5, "Infinity", Number.NaN])("rejects %j", (perMillion) => {
expect(perMillionToPerToken.safeParse(perMillion).success).toBe(false);
});
});
describe("ModelInfoEditForm pricing fields", () => {
const storedPrices = {
model: "gpt-4o",
input_cost_per_token: 0.000003,
output_cost_per_token: 0.000015,
cache_read_input_token_cost: 0.0000003,
};
it("hands the submit handler per-token prices, with null for a cleared field", async () => {
const onSubmit = vi.fn().mockResolvedValue(undefined);
const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never });
renderForm({ onSubmit, litellmParams: storedPrices });
expect(screen.getByPlaceholderText("Enter input cost")).toHaveValue(3);
fireEvent.change(screen.getByPlaceholderText("Enter input cost"), { target: { value: "2.5" } });
fireEvent.change(screen.getByPlaceholderText("Enter output cost"), { target: { value: "" } });
await user.click(screen.getByRole("button", { name: /save changes/i }));
await vi.waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1));
const [values, isFieldTouched] = onSubmit.mock.calls[0];
const expectedPrices = {
input_cost: 0.0000025,
output_cost: null,
cache_read_cost: 0.0000003,
cache_write_cost: null,
};
expect(values).toMatchObject(expectedPrices);
expect(isFieldTouched("input_cost")).toBe(true);
expect(isFieldTouched("cache_read_cost")).toBe(false);
});
it("shows a field error and does not submit a negative price", async () => {
const onSubmit = vi.fn().mockResolvedValue(undefined);
const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never });
renderForm({ onSubmit, litellmParams: storedPrices });
fireEvent.change(screen.getByPlaceholderText("Enter output cost"), { target: { value: "-1" } });
await user.click(screen.getByRole("button", { name: /save changes/i }));
expect(await screen.findByText("Enter a price of 0 or more")).toBeInTheDocument();
expect(screen.getByPlaceholderText("Enter output cost")).toHaveAttribute("aria-invalid", "true");
expect(onSubmit).not.toHaveBeenCalled();
});
});
describe("ModelInfoEditForm existing-credentials picker", () => {
it("shows the display name beside the credential name, searches by it, and submits the name", async () => {
const onSubmit = vi.fn().mockResolvedValue(undefined);

View file

@ -80,44 +80,18 @@ const COST_SOURCES: Record<TouchedPricingField, string> = {
cache_write_cost_1h: "cache_creation_input_token_cost_above_1hr",
};
export interface ModelEditFormValues {
model_name?: string;
litellm_model_name?: string;
api_base?: string;
custom_llm_provider?: string;
organization?: string;
tpm?: string | number | null;
rpm?: string | number | null;
max_retries?: string | number | null;
timeout?: string | number | null;
stream_timeout?: string | number | null;
input_cost?: string | number | null;
output_cost?: string | number | null;
cache_read_cost?: string | number | null;
cache_write_cost?: string | number | null;
cache_write_cost_1h?: string | number | null;
ptu_count?: string | number | null;
cost_per_ptu_per_hour?: string | number | null;
ptu_effective_from?: Dayjs | null;
ptu_effective_to?: Dayjs | null;
cache_control?: boolean;
cache_control_injection_points?: CacheControlInjectionPoint[];
model_access_group?: string[];
guardrails?: string[];
vector_store_ids?: string[];
tags?: string[];
health_check_model?: string | null;
litellm_credential_name?: string | null;
litellm_extra_params?: string;
model_info?: string;
team_id?: string;
}
type ModelEditFieldName = keyof ModelEditFormValues;
const scalar = z.union([z.string(), z.number(), z.null()]).optional();
const textish = z.string().optional();
const isBlank = (value: string | number | null | undefined): boolean => value == null || String(value).trim() === "";
export const perMillionToPerToken = scalar
.refine(
(perMillion) => isBlank(perMillion) || (Number.isFinite(Number(perMillion)) && Number(perMillion) >= 0),
"Enter a price of 0 or more",
)
.transform((perMillion) => (isBlank(perMillion) ? null : Number(perMillion) / 1_000_000));
const modelEditShape = {
model_name: textish,
litellm_model_name: textish,
@ -129,11 +103,11 @@ const modelEditShape = {
max_retries: scalar,
timeout: scalar,
stream_timeout: scalar,
input_cost: scalar,
output_cost: scalar,
cache_read_cost: scalar,
cache_write_cost: scalar,
cache_write_cost_1h: scalar,
input_cost: perMillionToPerToken,
output_cost: perMillionToPerToken,
cache_read_cost: perMillionToPerToken,
cache_write_cost: perMillionToPerToken,
cache_write_cost_1h: perMillionToPerToken,
ptu_count: scalar,
cost_per_ptu_per_hour: scalar,
ptu_effective_from: z.custom<Dayjs | null>().nullish(),
@ -151,6 +125,8 @@ const modelEditShape = {
team_id: textish,
};
type ModelEditFieldName = keyof typeof modelEditShape;
const isJson = (value: string): boolean => {
try {
JSON.parse(value);
@ -210,6 +186,10 @@ const buildSchema = (ptuEnabled: boolean, isFieldTouched: (field: TouchedPricing
}
});
type ModelEditSchema = ReturnType<typeof buildSchema>;
export type ModelEditFormValues = z.input<ModelEditSchema>;
export type ModelEditSubmitValues = z.output<ModelEditSchema>;
const perMillionTokens = (...rates: (number | null | undefined)[]): number | null => {
const rate = rates.find((candidate) => candidate != null);
return rate == null ? null : rate * 1_000_000;
@ -295,7 +275,7 @@ interface ModelInfoEditFormProps {
showCacheControl: boolean;
setShowCacheControl: (checked: boolean) => void;
onCancel: () => void;
onSubmit: (values: ModelEditFormValues, isFieldTouched: (field: TouchedPricingField) => boolean) => Promise<void>;
onSubmit: (values: ModelEditSubmitValues, isFieldTouched: (field: TouchedPricingField) => boolean) => Promise<void>;
modelAccessGroups: string[] | null;
guardrailsList: string[];
tagsList: Record<string, Tag>;
@ -383,10 +363,10 @@ const ModelInfoEditForm: React.FC<ModelInfoEditFormProps> = ({
};
// react-hook-form refreshes control._options every render, so this rebuild is what the next submit runs.
const resolver: Resolver<ModelEditFormValues> = (values, context, options) =>
const resolver: Resolver<ModelEditFormValues, unknown, ModelEditSubmitValues> = (values, context, options) =>
zodResolver(buildSchema(ptuCostAttributionEnabled, isFieldTouched))(values, context, options);
const form = useForm<ModelEditFormValues>({
const form = useForm<ModelEditFormValues, unknown, ModelEditSubmitValues>({
resolver,
defaultValues: toModelEditFormValues(localModelData, isWildcardModel),
});

View file

@ -2148,6 +2148,72 @@ describe("ModelInfoView", () => {
expect(payload.litellm_params.input_cost_per_token).toBe(5 / 1_000_000);
});
const pricingInputs = () => [
screen.getByPlaceholderText("Enter input cost"),
screen.getByPlaceholderText("Enter output cost"),
...screen.getAllByPlaceholderText("Defaults to Input Cost if blank"),
screen.getByPlaceholderText("Defaults to the cost map 1 hour rate, else Cache Write Cost"),
];
it("refuses a negative price on every pricing field and never calls the API", async () => {
const user = userEvent.setup();
await enterEditMode(user);
for (const input of pricingInputs()) {
fireEvent.change(input, { target: { value: "-5" } });
}
await user.click(screen.getByRole("button", { name: /save changes/i }));
expect(await screen.findAllByText("Enter a price of 0 or more")).toHaveLength(5);
expect(mockModelPatchUpdateCall).not.toHaveBeenCalled();
});
it("sends a zero price as 0 and lets the cache read cost follow a zero input cost", async () => {
const user = userEvent.setup();
await enterEditMode(user);
setInputCost("0");
fireEvent.change(screen.getByPlaceholderText("Enter output cost"), { target: { value: "0" } });
const payload = await save(user);
expect(payload.litellm_params).toMatchObject({
input_cost_per_token: 0,
output_cost_per_token: 0,
cache_read_input_token_cost: 0,
});
});
it("sends an explicit null for a cleared cache read cost instead of falling back to the input cost", async () => {
const user = userEvent.setup();
await enterEditMode(user);
setInputCost("2");
const [cacheRead] = screen.getAllByPlaceholderText("Defaults to Input Cost if blank");
fireEvent.change(cacheRead, { target: { value: "1" } });
fireEvent.change(cacheRead, { target: { value: "" } });
const payload = await save(user);
expect(payload.litellm_params.input_cost_per_token).toBe(0.000002);
expect(payload.litellm_params.cache_read_input_token_cost).toBeNull();
});
it("keeps a stored cache read cost when only the input cost changes", async () => {
const withCacheRead = {
...defaultModelData,
litellm_params: { ...defaultModelData.litellm_params, cache_read_input_token_cost: 0.000001 },
};
mockUseModelsInfo.mockReturnValue({ data: { data: [withCacheRead] }, isLoading: false, error: null });
mockModelInfoV1Call.mockResolvedValue({ data: [withCacheRead] });
const user = userEvent.setup();
await enterEditMode(user);
setInputCost("2");
const payload = await save(user);
expect(payload.litellm_params.input_cost_per_token).toBe(0.000002);
expect(payload.litellm_params.cache_read_input_token_cost).toBe(0.000001);
});
it.fails(
"sends only the edited input cost (expected to fail until the forms revamp, tri-state PATCH tracker)",
async () => {

View file

@ -46,14 +46,11 @@ import {
import { Logo } from "@/components/molecules/logo/Logo";
import { ModelPricingSummary } from "@/components/molecules/models/ModelPricingSummary";
import UpdateModelCredentialsModal from "./update_model_credentials_modal";
import ModelInfoEditForm, { type ModelEditFormValues, type TouchedPricingField } from "./ModelInfoEditForm";
import ModelInfoEditForm, { type ModelEditSubmitValues, type TouchedPricingField } from "./ModelInfoEditForm";
import { Tag } from "./tag_management/types";
import { getDisplayModelName } from "./view_model/model_name_display";
import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle } from "@/components/ui/dialog";
const perTokenOrNull = (perMillion: string | number | null | undefined): number | null =>
perMillion === undefined || perMillion === null || perMillion === "" ? null : Number(perMillion) / 1_000_000;
interface ModelInfoViewProps {
modelId: string;
onClose: () => void;
@ -265,7 +262,7 @@ export default function ModelInfoView({
};
const handleModelUpdate = async (
values: ModelEditFormValues,
values: ModelEditSubmitValues,
isFieldTouched: (field: TouchedPricingField) => boolean,
) => {
try {
@ -298,45 +295,24 @@ export default function ModelInfoView({
};
if (isFieldTouched("input_cost")) {
if (values.input_cost !== undefined && values.input_cost !== null && values.input_cost !== "") {
updatedLitellmParams.input_cost_per_token = Number(values.input_cost) / 1_000_000;
} else {
// Explicit null signals the backend to remove the pricing override.
updatedLitellmParams.input_cost_per_token = null;
}
updatedLitellmParams.input_cost_per_token = values.input_cost;
}
if (isFieldTouched("output_cost")) {
if (values.output_cost !== undefined && values.output_cost !== null && values.output_cost !== "") {
updatedLitellmParams.output_cost_per_token = Number(values.output_cost) / 1_000_000;
} else {
updatedLitellmParams.output_cost_per_token = null;
updatedLitellmParams.output_cost_per_token = values.output_cost;
}
if (isFieldTouched("cache_read_cost")) {
updatedLitellmParams.cache_read_input_token_cost = values.cache_read_cost;
} else if (isFieldTouched("input_cost")) {
const cacheReadCost = values.cache_read_cost ?? values.input_cost;
if (cacheReadCost !== null) {
updatedLitellmParams.cache_read_input_token_cost = cacheReadCost;
}
}
// Cache Read Cost:
// - explicit value provided → use it
// - field touched but empty → explicit null (signals backend to remove override)
// - only input_cost touched → fall back to input_cost (guarded against null)
if (isFieldTouched("cache_read_cost") || isFieldTouched("input_cost")) {
if (values.cache_read_cost !== undefined && values.cache_read_cost !== null && values.cache_read_cost !== "") {
updatedLitellmParams.cache_read_input_token_cost = Number(values.cache_read_cost) / 1_000_000;
} else if (isFieldTouched("cache_read_cost")) {
updatedLitellmParams.cache_read_input_token_cost = null;
} else if (
updatedLitellmParams.input_cost_per_token !== undefined &&
updatedLitellmParams.input_cost_per_token !== null
) {
updatedLitellmParams.cache_read_input_token_cost = updatedLitellmParams.input_cost_per_token;
}
}
// Cache write tiers: explicit value if provided, else explicit null so the backend removes the
// override. Sending 0 would persist a zero rate even when the user intended to unset it.
if (isFieldTouched("cache_write_cost")) {
updatedLitellmParams.cache_creation_input_token_cost = perTokenOrNull(values.cache_write_cost);
updatedLitellmParams.cache_creation_input_token_cost = values.cache_write_cost;
}
if (isFieldTouched("cache_write_cost_1h")) {
updatedLitellmParams.cache_creation_input_token_cost_above_1hr = perTokenOrNull(values.cache_write_cost_1h);
updatedLitellmParams.cache_creation_input_token_cost_above_1hr = values.cache_write_cost_1h;
}
const storedCredentialName: string | null = localModelData?.litellm_params?.litellm_credential_name ?? null;