fix(ui): restore the cache control Role and Index field hints (#37437)

* fix(ui): restore the cache control Role and Index field hints

The add_model cache control editor lost both field hints when it moved off
antd Form.Item in #37392. "LiteLLM will mark all messages of this role as
cacheable" and "(Optional) If set litellm will mark the message at this index
as cacheable" went with the Form.Item tooltip props and neither string exists
in dashboard source any more. The Index hint was the only thing telling a user
that field is optional, so this is lost information rather than styling.

Both come back as shadcn tooltips beside their labels, matching how the
surviving switch-level hint is already rendered.

Also adds the payload characterization net this graph did not have. Before
this commit the seven suites over add_model and model_add held 37 cases, no
antd module mock, and zero toStrictEqual, so nothing pinned the submit
payload. AddModelPanel.integration.test.tsx drives the real panel, the real
antd store and the real prepareModelAddRequest, and asserts the object handed
to modelCreateCall.

It pins the distinctions only a strict assertion can see: litellm_credential_name
arrives as null from its initialValue while api_key, api_base, mode and
access_groups arrive as undefined, and team_id is absent entirely until the
Team-BYOK switch mounts it. It also pins the mount gate in both directions,
since a collapsed Advanced Settings drops both its keys and anything typed
into it while re-expanding restores them, and the empty-string skip, since a
cleared api_base must vanish rather than arrive as "".

Every fixture was captured from the running component rather than written by
hand. A 12-mutation battery over the bindings, the empty-string skip, the two
required rules and an added keepMounted all go red, each run gated on having
executed the expected case count.

* refactor(ui): drop the explanatory comments from the add-model payload net

The repo bans explanatory source comments. The liveness-gate pairing the
second one described now lives in the test name instead, where a reader
deleting the paired case will actually see it.

* fix(ui): make the cache control field hints reachable without a pointer

SimpleTooltip renders its trigger as a bare span with no tabindex, so the
guidance behind it is mouse-only and the focus-visible ring classes it
already carries can never fire. The antd tooltips these hints replaced set
tabIndex null too, so this is an improvement on the original rather than a
restoration of it.

The primitive is shared by 20 files and is CLI-managed, so the trigger is
composed at the call site instead: a real button, an accessible name that
says which field it explains, and the icon marked aria-hidden.

Two tests cover it by tabbing to each trigger rather than counting keys,
so they keep working when a field is added between them. Swapping the
button for a span with role=button turns exactly those two red and leaves
the other five green.
This commit is contained in:
yuneng-jiang 2026-08-18 22:38:07 -07:00 committed by GitHub
parent b9a267c693
commit 413fc7e517
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 477 additions and 3 deletions

View file

@ -0,0 +1,364 @@
import { renderWithProviders, screen, waitFor } from "../../../../../tests/test-utils";
import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event";
import { beforeEach, describe, expect, it, vi } from "vitest";
import AddModelPanel from "./AddModelPanel";
const modelCreateCall = vi.fn();
const mockPtuEnabled = vi.fn();
const mockAuthorized = vi.fn();
vi.mock("@/components/networking", async (importOriginal) => {
const actual = await importOriginal<typeof import("@/components/networking")>();
return {
...actual,
modelCreateCall: (accessToken: string, model: unknown) => modelCreateCall(accessToken, model),
modelAvailableCall: vi.fn().mockResolvedValue({ data: [{ id: "group-a" }] }),
};
});
vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ default: () => mockAuthorized() }));
vi.mock("@/app/(dashboard)/hooks/uiSettings/usePtuCostAttributionEnabled", () => ({
usePtuCostAttributionEnabled: () => mockPtuEnabled(),
}));
vi.mock("@/app/(dashboard)/hooks/models/useModelCostMap", () => ({ useModelCostMap: () => ({ data: {} }) }));
vi.mock("@/app/(dashboard)/hooks/credentials/useCredentials", () => ({
useCredentials: () => ({ data: { credentials: [] } }),
}));
vi.mock("@/app/(dashboard)/hooks/teams/useTeams", () => ({
useTeams: () => ({ data: [] }),
useInfiniteTeams: () => ({
data: { pages: [{ teams: [], total: 0, page: 1, page_size: 20, total_pages: 1 }] },
fetchNextPage: vi.fn(),
hasNextPage: false,
isFetchingNextPage: false,
isLoading: false,
}),
}));
vi.mock("@/app/(dashboard)/hooks/guardrails/useGuardrails", () => ({
useGuardrails: () => ({ data: { guardrails: [{ guardrail_name: "g-1" }] }, isLoading: false, error: null }),
}));
vi.mock("@/app/(dashboard)/hooks/tags/useTags", () => ({
useTags: () => ({ data: {}, isLoading: false, error: null }),
}));
vi.mock("@/app/(dashboard)/hooks/providers/useProviderFields", () => ({
useProviderFields: () => ({
data: [
{
provider: "OpenAI",
provider_display_name: "OpenAI",
litellm_provider: "openai",
default_model_placeholder: "gpt-4o",
credential_fields: [
{ key: "api_key", label: "API Key", field_type: "password", required: false },
{ key: "api_base", label: "API Base", field_type: "text", required: false },
],
},
],
isLoading: false,
error: null,
}),
}));
vi.mock("@/components/vector_store_management/VectorStoreSelector", () => ({
default: () => <div data-testid="vector-store-selector" />,
}));
const lastCreatedModel = () => modelCreateCall.mock.calls.at(-1)?.[1];
const PROXY_ADMIN = {
token: "t",
accessToken: "test-access-token",
userId: "user-1",
userEmail: "a@b.c",
userRole: "proxy_admin",
premiumUser: true,
disabledPersonalKeyCreation: false,
showSSOBanner: false,
};
const alwaysMounted = {
api_key: undefined,
api_base: undefined,
custom_llm_provider: "openai",
litellm_credential_name: null,
model: "gpt-4o",
};
const advancedOpenExtras = {
guardrails: undefined,
tags: undefined,
use_in_pass_through: undefined,
vector_store_ids: undefined,
};
const baseModelInfo = { access_groups: undefined, mode: undefined };
const { api_base: _omitted, ...ALWAYS_MOUNTED_WITHOUT_API_BASE } = alwaysMounted;
const setup = async () => {
const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never });
renderWithProviders(<AddModelPanel />);
await screen.findByText("Provider");
const openAdvanced = async () => {
await user.click(screen.getByText("Advanced Settings"));
await screen.findByText("Tags");
};
const closeAdvanced = async () => {
await user.click(screen.getByText("Advanced Settings"));
await waitFor(() => expect(screen.queryByText("Tags")).not.toBeInTheDocument());
};
const fillRequired = async (modelName = "gpt-4o") => {
await user.click(screen.getByRole("combobox", { name: /provider/i }));
await user.click(await screen.findByText("OpenAI"));
await user.type(await screen.findByPlaceholderText("gpt-3.5-turbo"), modelName);
};
const submit = async () => {
await user.click(screen.getByTestId("add-model-btn"));
await waitFor(() => expect(modelCreateCall).toHaveBeenCalled());
};
const submitExpectingRejection = async (message: string) => {
await user.click(screen.getByTestId("add-model-btn"));
await screen.findByText(message);
};
return { user, openAdvanced, closeAdvanced, fillRequired, submit, submitExpectingRejection };
};
describe("AddModelPanel submit payload contract", () => {
beforeEach(() => {
vi.clearAllMocks();
mockPtuEnabled.mockReturnValue(false);
mockAuthorized.mockReturnValue(PROXY_ADMIN);
});
it("sends only the always-mounted fields while Advanced Settings stays closed", async () => {
const { fillRequired, submit } = await setup();
await fillRequired();
await submit();
expect(lastCreatedModel()).toStrictEqual({
model_name: "gpt-4o",
litellm_params: { ...alwaysMounted },
model_info: { ...baseModelInfo },
});
});
it("registers four more keys as undefined once Advanced Settings opens", async () => {
const { openAdvanced, fillRequired, submit } = await setup();
await fillRequired();
await openAdvanced();
await submit();
expect(lastCreatedModel()).toStrictEqual({
model_name: "gpt-4o",
litellm_params: { ...alwaysMounted, ...advancedOpenExtras },
model_info: { ...baseModelInfo },
});
});
it("merges typed LiteLLM Params into litellm_params", async () => {
const { user, openAdvanced, fillRequired, submit } = await setup();
await fillRequired();
await openAdvanced();
await user.type(screen.getByLabelText("LiteLLM Params"), '{{"rpm": 7}');
await submit();
expect(lastCreatedModel()).toStrictEqual({
model_name: "gpt-4o",
litellm_params: { ...alwaysMounted, ...advancedOpenExtras, rpm: 7 },
model_info: { ...baseModelInfo },
});
});
it("drops a collapsed section's keys and the value typed into it", async () => {
const { user, openAdvanced, closeAdvanced, fillRequired, submit } = await setup();
await fillRequired();
await openAdvanced();
await user.type(screen.getByLabelText("LiteLLM Params"), '{{"rpm": 7}');
await closeAdvanced();
await submit();
expect(lastCreatedModel()).toStrictEqual({
model_name: "gpt-4o",
litellm_params: { ...alwaysMounted },
model_info: { ...baseModelInfo },
});
});
it("restores the typed value when the section is expanded again", async () => {
const { user, openAdvanced, closeAdvanced, fillRequired, submit } = await setup();
await fillRequired();
await openAdvanced();
await user.type(screen.getByLabelText("LiteLLM Params"), '{{"rpm": 7}');
await closeAdvanced();
await openAdvanced();
expect(screen.getByLabelText("LiteLLM Params")).toHaveValue('{"rpm": 7}');
await submit();
expect(lastCreatedModel()).toStrictEqual({
model_name: "gpt-4o",
litellm_params: { ...alwaysMounted, ...advancedOpenExtras, rpm: 7 },
model_info: { ...baseModelInfo },
});
});
it("converts per-million pricing to per-token and falls back to input cost for cache reads", async () => {
const { user, openAdvanced, fillRequired, submit } = await setup();
await fillRequired();
await openAdvanced();
await user.click(screen.getByLabelText("Custom Pricing"));
await user.type(await screen.findByLabelText("Input Cost (per 1M tokens)"), "3");
await user.type(screen.getByLabelText("Output Cost (per 1M tokens)"), "9");
await submit();
expect(lastCreatedModel()).toStrictEqual({
model_name: "gpt-4o",
litellm_params: {
...alwaysMounted,
...advancedOpenExtras,
input_cost_per_token: 0.000003,
output_cost_per_token: 0.000009,
cache_read_input_token_cost: 0.000003,
},
model_info: { ...baseModelInfo },
});
});
it("sends the seeded injection point when cache control is switched on", async () => {
const { user, openAdvanced, fillRequired, submit } = await setup();
await fillRequired();
await openAdvanced();
await user.click(screen.getByLabelText("Cache Control Injection Points"));
await screen.findByText("Add Injection Point");
await submit();
expect(lastCreatedModel()).toStrictEqual({
model_name: "gpt-4o",
litellm_params: {
...alwaysMounted,
...advancedOpenExtras,
cache_control_injection_points: [{ location: "message" }],
},
model_info: { ...baseModelInfo },
});
});
it("carries a role picked inside the injection point editor, with the index kept a string", async () => {
const { user, openAdvanced, fillRequired, submit } = await setup();
await fillRequired();
await openAdvanced();
await user.click(screen.getByLabelText("Cache Control Injection Points"));
await screen.findByText("Add Injection Point");
await user.click(screen.getByText("Select a role"));
await user.click(await screen.findByText("System"));
await user.type(screen.getByPlaceholderText("Optional"), "3");
await submit();
expect(lastCreatedModel()).toStrictEqual({
model_name: "gpt-4o",
litellm_params: {
...alwaysMounted,
...advancedOpenExtras,
cache_control_injection_points: [{ location: "message", role: "system", index: "3" }],
},
model_info: { ...baseModelInfo },
});
});
it("mounts team_id only once the Team-BYOK switch is on", async () => {
const { user, fillRequired, submit } = await setup();
await fillRequired();
await user.click(screen.getByRole("switch", { name: "Team-BYOK Model" }));
await screen.findByText("Select Team");
await submit();
expect(lastCreatedModel()).toStrictEqual({
model_name: "gpt-4o",
litellm_params: { ...alwaysMounted },
model_info: { ...baseModelInfo, team_id: undefined },
});
});
});
describe("AddModelPanel empty-string skip", () => {
beforeEach(() => {
vi.clearAllMocks();
mockPtuEnabled.mockReturnValue(false);
mockAuthorized.mockReturnValue(PROXY_ADMIN);
});
it("sends a typed api_base, so the binding behind the next case is known to be live", async () => {
const { user, fillRequired, submit } = await setup();
await fillRequired();
await user.type(screen.getByLabelText("API Base"), "https://example.test");
await submit();
expect(lastCreatedModel().litellm_params).toStrictEqual({
...alwaysMounted,
api_base: "https://example.test",
});
});
it("omits api_base entirely once it is cleared, rather than sending an empty string", async () => {
const { user, fillRequired, submit } = await setup();
await fillRequired();
const apiBase = screen.getByLabelText("API Base");
await user.type(apiBase, "https://example.test");
await user.clear(apiBase);
await submit();
const params = lastCreatedModel().litellm_params;
expect(params).not.toHaveProperty("api_base");
expect(params).toStrictEqual(ALWAYS_MOUNTED_WITHOUT_API_BASE);
});
});
describe("AddModelPanel validation gates", () => {
beforeEach(() => {
vi.clearAllMocks();
mockPtuEnabled.mockReturnValue(true);
mockAuthorized.mockReturnValue(PROXY_ADMIN);
});
it("blocks the submit when a PTU count carries no effective-from date", async () => {
const { user, openAdvanced, fillRequired, submitExpectingRejection } = await setup();
await fillRequired();
await openAdvanced();
await user.type(screen.getByLabelText("PTU Count"), "15");
await user.type(screen.getByLabelText("Calculated Cost per PTU / Hour (USD)"), "2");
await submitExpectingRejection("PTU Effective From is required when PTU Count is set");
expect(modelCreateCall).not.toHaveBeenCalled();
});
it("hides the PTU fields entirely when the capability is off", async () => {
mockPtuEnabled.mockReturnValue(false);
const { openAdvanced, fillRequired } = await setup();
await fillRequired();
await openAdvanced();
expect(screen.queryByLabelText("PTU Count")).not.toBeInTheDocument();
});
it("requires a model before anything is sent", async () => {
const { user, submitExpectingRejection } = await setup();
await user.click(screen.getByRole("combobox", { name: /provider/i }));
await user.click(await screen.findByText("OpenAI"));
await submitExpectingRejection("Please enter at least one model.");
expect(modelCreateCall).not.toHaveBeenCalled();
});
});

View file

@ -0,0 +1,83 @@
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { describe, expect, it, vi } from "vitest";
import CacheControlInjectionPoints, { type CacheControlInjectionPoint } from "./cache_control_settings";
const ROLE_HINT = "LiteLLM will mark all messages of this role as cacheable";
const INDEX_HINT = "(Optional) If set litellm will mark the message at this index as cacheable";
const ONE_POINT: CacheControlInjectionPoint[] = [{ location: "message" }];
const tabTo = async (user: ReturnType<typeof userEvent.setup>, name: string): Promise<void> => {
for (let step = 0; step < 8; step++) {
await user.tab();
if (document.activeElement === screen.getByRole("button", { name })) {
return;
}
}
throw new Error(`${name} is not reachable by keyboard`);
};
describe("CacheControlInjectionPoints field hints", () => {
it("explains on the Role field that the role marks every message of that role cacheable", async () => {
const user = userEvent.setup();
render(<CacheControlInjectionPoints value={ONE_POINT} onChange={vi.fn()} />);
await user.hover(screen.getByRole("button", { name: "Role help" }));
expect(await screen.findByText(ROLE_HINT)).toBeInTheDocument();
});
it("explains on the Index field that it is optional and marks that message cacheable", async () => {
const user = userEvent.setup();
render(<CacheControlInjectionPoints value={ONE_POINT} onChange={vi.fn()} />);
await user.hover(screen.getByRole("button", { name: "Index help" }));
expect(await screen.findByText(INDEX_HINT)).toBeInTheDocument();
});
it("reveals the Role hint on keyboard focus, so it is reachable without a pointer", async () => {
const user = userEvent.setup();
render(<CacheControlInjectionPoints value={ONE_POINT} onChange={vi.fn()} />);
await tabTo(user, "Role help");
expect(await screen.findByText(ROLE_HINT)).toBeInTheDocument();
});
it("reveals the Index hint on keyboard focus, so it is reachable without a pointer", async () => {
const user = userEvent.setup();
render(<CacheControlInjectionPoints value={ONE_POINT} onChange={vi.fn()} />);
await tabTo(user, "Index help");
expect(await screen.findByText(INDEX_HINT)).toBeInTheDocument();
});
it("keeps both hints behind a hover or a focus rather than rendering them inline", () => {
render(<CacheControlInjectionPoints value={ONE_POINT} onChange={vi.fn()} />);
expect(screen.queryByText(ROLE_HINT)).not.toBeInTheDocument();
expect(screen.queryByText(INDEX_HINT)).not.toBeInTheDocument();
});
it("gives every row its own pair of hints", () => {
render(
<CacheControlInjectionPoints value={[{ location: "message" }, { location: "message" }]} onChange={vi.fn()} />,
);
expect(screen.getAllByRole("button", { name: "Role help" })).toHaveLength(2);
expect(screen.getAllByRole("button", { name: "Index help" })).toHaveLength(2);
});
it("still reports a typed index as a string through onChange", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
render(<CacheControlInjectionPoints value={ONE_POINT} onChange={onChange} />);
await user.type(screen.getByPlaceholderText("Optional"), "3");
expect(onChange).toHaveBeenLastCalledWith([{ location: "message", index: "3" }]);
});
});

View file

@ -1,9 +1,10 @@
import { Minus, Plus } from "lucide-react";
import { CircleHelp, Minus, Plus } from "lucide-react";
import React from "react";
import { Button } from "@/components/ui/button";
import { Label } from "@/components/ui/label";
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select";
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip";
import NumericalInput from "../shared/numerical_input";
@ -15,6 +16,10 @@ export const CACHE_CONTROL_TOOLTIP =
export const CACHE_CONTROL_DESCRIPTION =
"Providers like Anthropic, Bedrock API require users to specify where to inject cache control checkpoints, litellm can automatically add them for you as a cost saving feature.";
export const CACHE_CONTROL_ROLE_HINT = "LiteLLM will mark all messages of this role as cacheable";
export const CACHE_CONTROL_INDEX_HINT = "(Optional) If set litellm will mark the message at this index as cacheable";
export type CacheControlRole = "user" | "system" | "assistant";
export interface CacheControlInjectionPoint {
@ -33,6 +38,28 @@ const ROLE_ITEMS = [
{ value: "assistant", label: "Assistant" },
] as const;
const LabelWithHint: React.FC<{ label: string; hint: string }> = ({ label, hint }) => (
<div className="flex items-center">
<Label>{label}</Label>
<TooltipProvider>
<Tooltip>
<TooltipTrigger
render={
<button
type="button"
aria-label={`${label} help`}
className="ml-1 inline-flex cursor-help items-center rounded-sm text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring"
/>
}
>
<CircleHelp aria-hidden className="size-4" />
</TooltipTrigger>
<TooltipContent className="max-w-xs whitespace-normal">{hint}</TooltipContent>
</Tooltip>
</TooltipProvider>
</div>
);
interface CacheControlInjectionPointsProps {
value?: CacheControlInjectionPoint[];
onChange?: (points: CacheControlInjectionPoint[]) => void;
@ -72,7 +99,7 @@ const CacheControlInjectionPoints: React.FC<CacheControlInjectionPointsProps> =
</div>
<div className="w-[180px] space-y-1">
<Label>Role</Label>
<LabelWithHint label="Role" hint={CACHE_CONTROL_ROLE_HINT} />
<Select
items={ROLE_ITEMS}
value={point.role ?? null}
@ -95,7 +122,7 @@ const CacheControlInjectionPoints: React.FC<CacheControlInjectionPointsProps> =
</div>
<div className="w-[180px] space-y-1">
<Label>Index</Label>
<LabelWithHint label="Index" hint={CACHE_CONTROL_INDEX_HINT} />
<NumericalInput
type="number"
placeholder="Optional"