feat(ui): replace raw JSON input for optional_pre_call_checks with a multi-select

The Router Settings page rendered optional_pre_call_checks as free-text JSON,
requiring admins to know and correctly type the exact valid check names. Add
a dedicated multi-select populated from the field's known options (already
returned by /router/fields), matching how routing_strategy already gets its
own selector instead of a raw text field.

The value now flows through React state (like routing_strategy/enable_tag_filtering)
instead of the page's DOM-querySelector-based save mechanism, since an antd
Select doesn't produce a plain named <input> for that mechanism to read.
default_litellm_params keeps the raw-JSON editor since it has no fixed set of
keys to offer as options.
This commit is contained in:
Krrish Dholakia 2026-07-13 18:26:13 -07:00
parent 498aa2997d
commit 750e849d12
8 changed files with 325 additions and 17 deletions

View file

@ -0,0 +1,101 @@
import { describe, it, expect, vi } from "vitest";
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import OptionalPreCallChecksSelector from "./OptionalPreCallChecksSelector";
vi.mock("antd", async (importOriginal) => {
const actual = await importOriginal<typeof import("antd")>();
return {
...actual,
Select: ({ value, onChange, options, "data-testid": testId }: any) => (
<select
multiple
data-testid={testId}
value={value ?? []}
onChange={(e) => onChange(Array.from(e.target.selectedOptions).map((o: any) => o.value))}
>
{(options || []).map((option: any) => (
<option key={option.value} value={option.value}>
{option.label}
</option>
))}
</select>
),
};
});
const baseMetadata = {
optional_pre_call_checks: {
ui_field_name: "Optional Pre-call Checks",
field_description: "Extra checks the router runs before picking a deployment",
link: null,
},
};
const options = ["prompt_caching", "router_budget_limiting", "session_affinity"];
describe("OptionalPreCallChecksSelector", () => {
it("should render one option per entry in options", () => {
render(
<OptionalPreCallChecksSelector value={[]} options={options} routerFieldsMetadata={{}} onChange={vi.fn()} />,
);
const select = screen.getByTestId("optional-pre-call-checks-select") as HTMLSelectElement;
expect(Array.from(select.options).map((o) => o.value)).toEqual(options);
});
it("should display default label when no metadata is provided", () => {
render(
<OptionalPreCallChecksSelector value={[]} options={options} routerFieldsMetadata={{}} onChange={vi.fn()} />,
);
expect(screen.getByText("Optional Pre-call Checks")).toBeInTheDocument();
});
it("should display the label and description from metadata when provided", () => {
render(
<OptionalPreCallChecksSelector
value={[]}
options={options}
routerFieldsMetadata={baseMetadata}
onChange={vi.fn()}
/>,
);
expect(screen.getByText("Extra checks the router runs before picking a deployment")).toBeInTheDocument();
});
it("should render a Learn more link when metadata provides one", () => {
const metadata = {
optional_pre_call_checks: { ...baseMetadata.optional_pre_call_checks, link: "https://docs.example.com/checks" },
};
render(
<OptionalPreCallChecksSelector value={[]} options={options} routerFieldsMetadata={metadata} onChange={vi.fn()} />,
);
const link = screen.getByRole("link", { name: /learn more/i });
expect(link).toHaveAttribute("href", "https://docs.example.com/checks");
});
it("should call onChange with the selected checks", async () => {
const onChange = vi.fn();
const user = userEvent.setup();
render(
<OptionalPreCallChecksSelector value={[]} options={options} routerFieldsMetadata={{}} onChange={onChange} />,
);
await user.selectOptions(screen.getByTestId("optional-pre-call-checks-select"), "prompt_caching");
expect(onChange).toHaveBeenCalledWith(["prompt_caching"]);
});
it("should reflect an already-selected value", () => {
render(
<OptionalPreCallChecksSelector
value={["router_budget_limiting"]}
options={options}
routerFieldsMetadata={{}}
onChange={vi.fn()}
/>,
);
const select = screen.getByTestId("optional-pre-call-checks-select") as HTMLSelectElement;
const selected = Array.from(select.selectedOptions).map((o) => o.value);
expect(selected).toEqual(["router_budget_limiting"]);
});
});

View file

@ -0,0 +1,55 @@
import React from "react";
import { Select } from "antd";
interface OptionalPreCallChecksSelectorProps {
value: string[];
options: string[];
routerFieldsMetadata: { [key: string]: any };
onChange: (value: string[]) => void;
}
const OptionalPreCallChecksSelector: React.FC<OptionalPreCallChecksSelectorProps> = ({
value,
options,
routerFieldsMetadata,
onChange,
}) => {
const meta = routerFieldsMetadata["optional_pre_call_checks"];
return (
<div className="space-y-2 max-w-3xl">
<label className="block">
<span className="text-xs font-medium text-gray-700 uppercase tracking-wide">
{meta?.ui_field_name || "Optional Pre-call Checks"}
</span>
<p className="text-xs text-gray-500 mt-0.5 mb-2">
{meta?.field_description || ""}
{meta?.link && (
<>
{" "}
<a
href={meta.link}
target="_blank"
rel="noopener noreferrer"
className="text-blue-600 hover:text-blue-800 underline"
>
Learn more
</a>
</>
)}
</p>
<Select
mode="multiple"
value={value}
onChange={onChange}
options={options.map((option) => ({ value: option, label: option }))}
placeholder="No pre-call checks enabled"
className="w-full"
data-testid="optional-pre-call-checks-select"
/>
</label>
</div>
);
};
export default OptionalPreCallChecksSelector;

View file

@ -41,6 +41,18 @@ describe("ReliabilityRetriesSection", () => {
expect(inputNames).not.toContain("model_group_retry_policy");
});
it("should not render a raw-text input for optional_pre_call_checks (owned by its own multi-select)", () => {
render(
<ReliabilityRetriesSection
routerSettings={{ ...baseSettings, optional_pre_call_checks: ["prompt_caching"] }}
routerFieldsMetadata={{}}
/>,
);
const inputs = screen.queryAllByRole("textbox");
const inputNames = inputs.map((el) => el.getAttribute("name"));
expect(inputNames).not.toContain("optional_pre_call_checks");
});
it("should use ui_field_name from metadata as the label", () => {
const metadata = {
num_retries: { ui_field_name: "Number of Retries", field_description: "How many times to retry" },

View file

@ -28,7 +28,8 @@ const ReliabilityRetriesSection: React.FC<ReliabilityRetriesSectionProps> = ({
param != "enable_tag_filtering" &&
param != "retry_policy" &&
param != "model_group_retry_policy" &&
param != "routing_groups",
param != "routing_groups" &&
param != "optional_pre_call_checks",
)
.map(([param, value]) => (
<div key={param} className="space-y-2">

View file

@ -11,11 +11,29 @@ vi.mock("antd", async (importOriginal) => {
return {
...actual,
Select: Object.assign(
({ value, onChange, children }: any) => (
<select data-testid="strategy-select" value={value ?? ""} onChange={(e) => onChange(e.target.value)}>
{children}
</select>
),
({ value, onChange, children, mode, options, "data-testid": testId }: any) =>
mode === "multiple" ? (
<select
multiple
data-testid={testId || "multi-select"}
value={value ?? []}
onChange={(e) => onChange(Array.from(e.target.selectedOptions).map((o: any) => o.value))}
>
{(options || []).map((option: any) => (
<option key={option.value} value={option.value}>
{option.label}
</option>
))}
</select>
) : (
<select
data-testid={testId || "strategy-select"}
value={value ?? ""}
onChange={(e) => onChange(e.target.value)}
>
{children}
</select>
),
{
Option: ({ value, children }: any) => <option value={value}>{children}</option>,
},
@ -110,4 +128,44 @@ describe("RouterSettingsForm", () => {
render(<RouterSettingsForm {...baseProps} />);
expect(screen.getByText("Reliability & Retries")).toBeInTheDocument();
});
it("should not show the optional pre-call checks selector when no options are known", () => {
render(<RouterSettingsForm {...baseProps} />);
expect(screen.queryByTestId("optional-pre-call-checks-select")).not.toBeInTheDocument();
});
it("should show the optional pre-call checks selector populated from field metadata options", () => {
const props = {
...baseProps,
routerFieldsMetadata: {
optional_pre_call_checks: { ui_field_name: "Optional Pre-call Checks", options: ["prompt_caching", "router_budget_limiting"] },
},
};
render(<RouterSettingsForm {...props} />);
const select = screen.getByTestId("optional-pre-call-checks-select") as HTMLSelectElement;
const optionValues = Array.from(select.options).map((o) => o.value);
expect(optionValues).toEqual(["prompt_caching", "router_budget_limiting"]);
});
it("should call onChange with the updated optional_pre_call_checks when the selector changes", async () => {
const onChange = vi.fn();
const user = userEvent.setup();
const props = {
...baseProps,
onChange,
routerFieldsMetadata: {
optional_pre_call_checks: { ui_field_name: "Optional Pre-call Checks", options: ["prompt_caching"] },
},
};
render(<RouterSettingsForm {...props} />);
await user.selectOptions(screen.getByTestId("optional-pre-call-checks-select"), "prompt_caching");
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({
routerSettings: expect.objectContaining({ optional_pre_call_checks: ["prompt_caching"] }),
}),
);
});
});

View file

@ -1,5 +1,6 @@
import React from "react";
import LatencyBasedConfiguration from "./LatencyBasedConfiguration";
import OptionalPreCallChecksSelector from "./OptionalPreCallChecksSelector";
import ReliabilityRetriesSection from "./ReliabilityRetriesSection";
import RoutingStrategySelector from "./RoutingStrategySelector";
import TagFilteringToggle from "./TagFilteringToggle";
@ -39,6 +40,15 @@ const RouterSettingsForm: React.FC<RouterSettingsFormProps> = ({
});
};
const handleOptionalPreCallChecksChange = (checks: string[]) => {
onChange({
...value,
routerSettings: { ...value.routerSettings, optional_pre_call_checks: checks },
});
};
const optionalPreCallCheckOptions: string[] = routerFieldsMetadata["optional_pre_call_checks"]?.options || [];
return (
<div className="w-full space-y-8 py-2">
{/* Routing Settings Section */}
@ -65,6 +75,16 @@ const RouterSettingsForm: React.FC<RouterSettingsFormProps> = ({
routerFieldsMetadata={routerFieldsMetadata}
onToggle={handleTagFilteringToggle}
/>
{/* Optional Pre-call Checks */}
{optionalPreCallCheckOptions.length > 0 && (
<OptionalPreCallChecksSelector
value={value.routerSettings.optional_pre_call_checks || []}
options={optionalPreCallCheckOptions}
routerFieldsMetadata={routerFieldsMetadata}
onChange={handleOptionalPreCallChecksChange}
/>
)}
</div>
{/* Divider */}

View file

@ -8,11 +8,29 @@ vi.mock("antd", async (importOriginal) => {
return {
...actual,
Select: Object.assign(
({ value, onChange, children }: any) => (
<select data-testid="strategy-select" value={value ?? ""} onChange={(e) => onChange(e.target.value)}>
{children}
</select>
),
({ value, onChange, children, mode, options, "data-testid": testId }: any) =>
mode === "multiple" ? (
<select
multiple
data-testid={testId || "multi-select"}
value={value ?? []}
onChange={(e) => onChange(Array.from(e.target.selectedOptions).map((o: any) => o.value))}
>
{(options || []).map((option: any) => (
<option key={option.value} value={option.value}>
{option.label}
</option>
))}
</select>
) : (
<select
data-testid={testId || "strategy-select"}
value={value ?? ""}
onChange={(e) => onChange(e.target.value)}
>
{children}
</select>
),
{
Option: ({ value, children }: any) => <option value={value}>{children}</option>,
},
@ -188,11 +206,12 @@ describe("RouterSettings", () => {
expect(NotificationsManager.success).not.toHaveBeenCalled();
});
it("should round-trip default_litellm_params and optional_pre_call_checks as JSON on save", async () => {
// Regression test: these two fields hold dicts/lists (e.g. cache_control_injection_points,
// ["prompt_caching"]), not plain strings. Without listing them in the save handler's
// jsonKeys set, they'd be persisted as raw stringified text instead of parsed JSON,
// silently corrupting the setting the next time the router reads it.
it("should round-trip default_litellm_params and optional_pre_call_checks unmodified on save", async () => {
// Regression test: default_litellm_params holds a dict (e.g. cache_control_injection_points),
// not a plain string - without listing it in the save handler's jsonKeys set, it'd be persisted
// as raw stringified text instead of parsed JSON. optional_pre_call_checks is a list owned by
// its own multi-select (no DOM input); without the fallback-to-state path in parseInputValue,
// an untouched value would be silently dropped instead of round-tripping through Save.
vi.mocked(getCallbacksCall).mockResolvedValue({
router_settings: {
...mockCallbacksResponse.router_settings,
@ -221,4 +240,46 @@ describe("RouterSettings", () => {
),
);
});
it("should save a newly-selected optional pre-call check picked from the multi-select", async () => {
vi.mocked(getCallbacksCall).mockResolvedValue({
router_settings: {
...mockCallbacksResponse.router_settings,
optional_pre_call_checks: [],
},
});
vi.mocked(getRouterSettingsCall).mockResolvedValue({
...mockRouterSettingsResponse,
fields: [
...mockRouterSettingsResponse.fields,
{
field_name: "optional_pre_call_checks",
ui_field_name: "Optional Pre-call Checks",
field_description: "Extra checks the router runs before picking a deployment",
options: ["prompt_caching", "router_budget_limiting"],
link: null,
},
],
});
const user = userEvent.setup();
renderWithProviders(<RouterSettings {...defaultProps} />);
await waitFor(() => {
expect(screen.getByTestId("optional-pre-call-checks-select")).toBeInTheDocument();
});
await user.selectOptions(screen.getByTestId("optional-pre-call-checks-select"), "prompt_caching");
await user.click(screen.getByRole("button", { name: /save changes/i }));
await waitFor(() =>
expect(setCallbacksCall).toHaveBeenCalledWith(
"test-token",
expect.objectContaining({
router_settings: expect.objectContaining({
optional_pre_call_checks: ["prompt_caching"],
}),
}),
),
);
});
});

View file

@ -88,7 +88,7 @@ const RouterSettings: React.FC<RouterSettingsProps> = ({ accessToken, userRole,
const router_settings = formValue.routerSettings;
const numberKeys = new Set(["allowed_fails", "cooldown_time", "num_retries", "timeout", "retry_after"]);
const jsonKeys = new Set(["model_group_alias", "default_litellm_params", "optional_pre_call_checks"]);
const jsonKeys = new Set(["model_group_alias", "default_litellm_params"]);
// retry_policy and model_group_retry_policy are owned by the Model Retry Settings tab;
// routing_groups is owned by the Routing Groups tab. This page must not read or write them.
const tabOwnedKeys = new Set(["retry_policy", "model_group_retry_policy", "routing_groups"]);