mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
feat(ui): dry-run an auto-router config against the backend before saving it (#38595)
* feat(ui): dry-run an auto-router config against the backend before saving it Both auto-router forms built a payload and posted it, so anything the write gate refused came back as a raw 400 with the backend's message buried in it. They now POST the exact payload to /auto_router/validate_complexity_router_config first and surface its verdict inline. One dryRunRejection owns the gate, and it reads valid alone. The verdict's two fields arrive independently, so gating on the error message would let a rejection that carried none through to the write. A transport failure fails open as valid, leaving the write gate authoritative rather than blocking a save on a flaky network. Applies to every auto-router, built-in tiers included. * fix(ui): hold the auto-router create closed for the full dry-run and create sequence A second submit while the dry-run round-trip was pending started another create against the non-idempotent /model/new. The submit handler now refuses re-entry and the button disables for the whole sequence, matching the edit modal's loading guard. Also drops the explanatory comments this PR had added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
22a349ee70
commit
77bbf4b5b7
7 changed files with 167 additions and 18 deletions
|
|
@ -70,9 +70,14 @@ const { mockFetchAvailableModels, mockFetchAllModelDeployments } = vi.hoisted(()
|
|||
mockFetchAllModelDeployments: vi.fn(),
|
||||
}));
|
||||
|
||||
const { validateAutoRouterConfig } = vi.hoisted(() => ({
|
||||
validateAutoRouterConfig: vi.fn().mockResolvedValue({ valid: true }),
|
||||
}));
|
||||
|
||||
vi.mock("../networking", () => ({
|
||||
modelAvailableCall: vi.fn().mockResolvedValue({ data: [] }),
|
||||
testAutoRouterRouting: vi.fn(),
|
||||
validateAutoRouterConfig,
|
||||
}));
|
||||
|
||||
vi.mock("@/components/llm_calls/fetch_models", () => ({
|
||||
|
|
@ -189,6 +194,60 @@ describe("AddAutoRouterTab", () => {
|
|||
expect(vi.mocked(handleAddAutoRouterSubmit).mock.calls.at(-1)?.[0]).toMatchObject({ team_id: "team-1" });
|
||||
});
|
||||
|
||||
it("does not submit when the backend's dry-run rejects the config", async () => {
|
||||
const user = userEvent.setup();
|
||||
vi.mocked(getMissingTiersError).mockReturnValue(null);
|
||||
validateAutoRouterConfig.mockResolvedValueOnce({
|
||||
valid: false,
|
||||
error: "session_affinity cannot be combined with tier_definitions",
|
||||
});
|
||||
|
||||
renderWithProviders(<Harness />);
|
||||
await user.type(screen.getByPlaceholderText(/smart_router/i), "rejected-router");
|
||||
await user.click(screen.getByRole("button", { name: /add auto router/i }));
|
||||
|
||||
await waitFor(() => expect(validateAutoRouterConfig).toHaveBeenCalled());
|
||||
expect(handleAddAutoRouterSubmit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("submits when the dry-run passes, so the gate is not simply blocking everything", async () => {
|
||||
const user = userEvent.setup();
|
||||
vi.mocked(getMissingTiersError).mockReturnValue(null);
|
||||
validateAutoRouterConfig.mockResolvedValueOnce({ valid: true });
|
||||
|
||||
renderWithProviders(<Harness />);
|
||||
await user.type(screen.getByPlaceholderText(/smart_router/i), "accepted-router");
|
||||
await user.click(screen.getByRole("button", { name: /add auto router/i }));
|
||||
|
||||
await waitFor(() => expect(handleAddAutoRouterSubmit).toHaveBeenCalled());
|
||||
});
|
||||
|
||||
// A second submit while the dry-run round-trip is pending must not start another create: the
|
||||
// button disables, and the handler itself refuses re-entry since a form submit (Enter) fires it
|
||||
// regardless of the button's disabled state.
|
||||
it("creates the router once when the form is submitted again mid dry-run", async () => {
|
||||
vi.mocked(getMissingTiersError).mockReturnValue(null);
|
||||
let resolveVerdict: (verdict: { valid: boolean }) => void = () => {};
|
||||
validateAutoRouterConfig.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveVerdict = resolve;
|
||||
}),
|
||||
);
|
||||
|
||||
const { container } = renderWithProviders(<Harness />);
|
||||
fireEvent.change(screen.getByPlaceholderText(/smart_router/i), { target: { value: "double-submit-router" } });
|
||||
|
||||
fireEvent.submit(container.querySelector("form")!);
|
||||
await waitFor(() => expect(screen.getByRole("button", { name: /add auto router/i })).toBeDisabled());
|
||||
fireEvent.submit(container.querySelector("form")!);
|
||||
|
||||
resolveVerdict({ valid: true });
|
||||
await waitFor(() => expect(screen.getByRole("button", { name: /add auto router/i })).toBeEnabled());
|
||||
expect(validateAutoRouterConfig).toHaveBeenCalledTimes(1);
|
||||
expect(handleAddAutoRouterSubmit).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// LIT-5133: "Add keyword rule" seeds a row with no keywords, and the semantic toggle that used
|
||||
// to be the only thing checking them is off by default. The row was dropped on the way to the
|
||||
// payload, so the create succeeded and the caller's rule was gone with nothing said about it.
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/comp
|
|||
import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner";
|
||||
import { useZodForm } from "@/lib/forms/useZodForm";
|
||||
import AccessGroupTagsCombobox from "./AccessGroupTagsCombobox";
|
||||
import { modelAvailableCall } from "../networking";
|
||||
import { modelAvailableCall, validateAutoRouterConfig } from "../networking";
|
||||
import { all_admin_roles } from "@/utils/roles";
|
||||
import { type ModelWriteScope } from "@/utils/modelPermissions";
|
||||
import TeamDropdown from "../common_components/team_dropdown";
|
||||
|
|
@ -33,6 +33,7 @@ import { DEFAULT_MATCH_THRESHOLD } from "./SemanticKeywordMatching";
|
|||
import {
|
||||
BuildComplexityRouterConfigParams,
|
||||
buildComplexityRouterConfig,
|
||||
dryRunRejection,
|
||||
getKeywordTierRulesError,
|
||||
getClassifierModelError,
|
||||
getMissingTiersError,
|
||||
|
|
@ -196,6 +197,7 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
const [matchThreshold, setMatchThreshold] = useState<number>(DEFAULT_MATCH_THRESHOLD);
|
||||
const [escalationKeywords, setEscalationKeywords] = useState<string[]>(DEFAULT_ESCALATION_KEYWORDS);
|
||||
const [showValidationErrors, setShowValidationErrors] = useState<boolean>(false);
|
||||
const [isSubmitting, setIsSubmitting] = useState<boolean>(false);
|
||||
|
||||
const [selectedPreset, setSelectedPreset] = useState<string | undefined>(undefined);
|
||||
// Closed by default: a caller opens it deliberately, either by clicking it or by choosing Custom
|
||||
|
|
@ -401,6 +403,19 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
return;
|
||||
}
|
||||
|
||||
const complexityRouterConfigPayload = buildComplexityRouterConfig(complexityRouterConfigParams);
|
||||
const serverVerdict = await validateAutoRouterConfig(
|
||||
accessToken,
|
||||
complexityRouterConfigPayload as unknown as Record<string, unknown>,
|
||||
requiresTeamScope ? form.getValues("team_id") : undefined,
|
||||
);
|
||||
const dryRunError = dryRunRejection(serverVerdict);
|
||||
if (dryRunError) {
|
||||
setShowValidationErrors(true);
|
||||
toast.fromError(dryRunError);
|
||||
return;
|
||||
}
|
||||
|
||||
// auto_router_default_model (-> litellm_params, read by the backend at init) and
|
||||
// complexity_router_config.default_model (-> the pin marker read back on edit, see
|
||||
// hydratePinnedDefaultModel in edit_auto_router_modal.tsx) must both come from the same
|
||||
|
|
@ -410,14 +425,15 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
...teamScopePayload(requiresTeamScope, form.getValues("team_id")),
|
||||
auto_router_default_model: defaultModel,
|
||||
model_type: "complexity_router",
|
||||
complexity_router_config: buildComplexityRouterConfig(complexityRouterConfigParams),
|
||||
complexity_router_config: complexityRouterConfigPayload,
|
||||
model_access_group: form.getValues("model_access_group"),
|
||||
};
|
||||
|
||||
handleAddAutoRouterSubmit(submitValues, accessToken, () => form.reset(EMPTY_FORM_VALUES), handleOk);
|
||||
await handleAddAutoRouterSubmit(submitValues, accessToken, () => form.reset(EMPTY_FORM_VALUES), handleOk);
|
||||
};
|
||||
|
||||
const handleAutoRouterSubmit = async () => {
|
||||
if (isSubmitting) return;
|
||||
const name = form.getValues("auto_router_name");
|
||||
if (!name) {
|
||||
setShowValidationErrors(true);
|
||||
|
|
@ -426,7 +442,12 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
return;
|
||||
}
|
||||
|
||||
await submitRecommendedRouter(name);
|
||||
setIsSubmitting(true);
|
||||
try {
|
||||
await submitRecommendedRouter(name);
|
||||
} finally {
|
||||
setIsSubmitting(false);
|
||||
}
|
||||
};
|
||||
|
||||
const handleTestConnection = () => {
|
||||
|
|
@ -640,11 +661,12 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
<BlockedReasonTooltip reason={submitBlockedReason}>
|
||||
<Button
|
||||
type="button"
|
||||
disabled={submitBlockedReason !== null}
|
||||
disabled={submitBlockedReason !== null || isSubmitting}
|
||||
onClick={() => {
|
||||
void handleAutoRouterSubmit();
|
||||
}}
|
||||
>
|
||||
{isSubmitting && <UiLoadingSpinner className="size-4" />}
|
||||
Add Auto Router
|
||||
</Button>
|
||||
</BlockedReasonTooltip>
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
getTierLabelsError,
|
||||
hydrateTierLabels,
|
||||
BuildComplexityRouterConfigParams,
|
||||
dryRunRejection,
|
||||
} from "./build_complexity_router_config";
|
||||
import { activeTierRows } from "./tier_rows";
|
||||
|
||||
|
|
@ -759,3 +760,22 @@ describe("heuristic_first", () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("dryRunRejection", () => {
|
||||
it("blocks the save on a rejection whose message is missing, which the write would return as a raw 400", () => {
|
||||
expect(dryRunRejection({ valid: false })).toBe("The proxy rejected this auto-router configuration");
|
||||
expect(dryRunRejection({ valid: false, error: null })).toBe("The proxy rejected this auto-router configuration");
|
||||
expect(dryRunRejection({ valid: false, error: " " })).toBe("The proxy rejected this auto-router configuration");
|
||||
});
|
||||
|
||||
it("surfaces the backend's own message when it sent one", () => {
|
||||
expect(dryRunRejection({ valid: false, error: "session_affinity cannot be combined with tier_definitions" })).toBe(
|
||||
"session_affinity cannot be combined with tier_definitions",
|
||||
);
|
||||
});
|
||||
|
||||
it("lets a valid verdict through, including the fail-open one a transport failure returns", () => {
|
||||
expect(dryRunRejection({ valid: true })).toBeNull();
|
||||
expect(dryRunRejection({ valid: true, error: null })).toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -108,6 +108,9 @@ export interface BuildComplexityRouterConfigParams {
|
|||
tierModelParams?: TierModelParamsByTier;
|
||||
}
|
||||
|
||||
export const dryRunRejection = (verdict: { valid: boolean; error?: string | null }): string | null =>
|
||||
verdict.valid ? null : verdict.error?.trim() || "The proxy rejected this auto-router configuration";
|
||||
|
||||
export interface ComplexityRouterConfigPayload {
|
||||
tiers: ComplexityTiers;
|
||||
default_model?: string;
|
||||
|
|
|
|||
|
|
@ -16,10 +16,15 @@ const { modelPatchUpdateCall, modelAvailableCall, getAutoRouterClassifierDefault
|
|||
getAutoRouterClassifierDefaultPromptCall: vi.fn().mockResolvedValue("Classify the request into exactly one tier."),
|
||||
}));
|
||||
|
||||
const { validateAutoRouterConfig } = vi.hoisted(() => ({
|
||||
validateAutoRouterConfig: vi.fn().mockResolvedValue({ valid: true }),
|
||||
}));
|
||||
|
||||
vi.mock("../networking", () => ({
|
||||
modelPatchUpdateCall,
|
||||
modelAvailableCall,
|
||||
getAutoRouterClassifierDefaultPromptCall,
|
||||
validateAutoRouterConfig,
|
||||
}));
|
||||
|
||||
vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ default: () => ({ accessToken: "sk-test" }) }));
|
||||
|
|
@ -96,6 +101,21 @@ describe("EditAutoRouterModal keyword matching", () => {
|
|||
expect(config.match_threshold).toBe(0.72);
|
||||
});
|
||||
|
||||
it("does not PATCH when the backend's dry-run rejects the config", async () => {
|
||||
const user = userEvent.setup();
|
||||
validateAutoRouterConfig.mockResolvedValueOnce({
|
||||
valid: false,
|
||||
error: "tier_labels cannot be combined with tier_definitions",
|
||||
});
|
||||
|
||||
renderModal();
|
||||
await screen.findByText(/Escalation Keywords/i);
|
||||
await user.click(screen.getByRole("button", { name: /save changes/i }));
|
||||
|
||||
await waitFor(() => expect(validateAutoRouterConfig).toHaveBeenCalled());
|
||||
expect(modelPatchUpdateCall).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// The create form blocks this; the edit modal renders the same controls, so it must block it
|
||||
// too. The backend raises on semantic_keyword_matching without an embedding model or keyword
|
||||
// rules, so skipping the guard turns a friendly inline message into a raw 400.
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner";
|
|||
import { useZodForm } from "@/lib/forms/useZodForm";
|
||||
import AccessGroupTagsCombobox from "../add_model/AccessGroupTagsCombobox";
|
||||
import ModelChoiceCombobox, { type ModelChoice } from "../add_model/ModelChoiceCombobox";
|
||||
import { modelAvailableCall, modelPatchUpdateCall } from "../networking";
|
||||
import { modelAvailableCall, modelPatchUpdateCall, validateAutoRouterConfig } from "../networking";
|
||||
import { fetchAvailableModels, ModelGroup } from "@/components/llm_calls/fetch_models";
|
||||
import RouterConfigBuilder from "../add_model/RouterConfigBuilder";
|
||||
import { hydrateTierModelParams, normalizeTierModels } from "../add_model/complexity_router_tiers";
|
||||
|
|
@ -26,6 +26,7 @@ import {
|
|||
getPlanModeTierError,
|
||||
getTierLabelsError,
|
||||
hydrateTierLabels,
|
||||
dryRunRejection,
|
||||
} from "../add_model/build_complexity_router_config";
|
||||
import { KeywordTierRule } from "../add_model/KeywordTierRules";
|
||||
import { DEFAULT_MATCH_THRESHOLD } from "../add_model/SemanticKeywordMatching";
|
||||
|
|
@ -516,23 +517,26 @@ const EditAutoRouterModal: React.FC<EditAutoRouterModalProps> = ({
|
|||
return;
|
||||
}
|
||||
|
||||
const updatedConfig = buildUpdatedComplexityRouterConfig(
|
||||
modelData.litellm_params?.complexity_router_config,
|
||||
complexityRouterConfig,
|
||||
customTechnicalKeywords,
|
||||
{ keywordTierRules, escalationKeywords, semanticMatchingEnabled, embeddingModel, matchThreshold },
|
||||
);
|
||||
const serverVerdict = await validateAutoRouterConfig(accessToken, updatedConfig, modelData?.model_info?.team_id);
|
||||
const dryRunError = dryRunRejection(serverVerdict);
|
||||
if (dryRunError) {
|
||||
setShowValidationErrors(true);
|
||||
toast.fromError(dryRunError);
|
||||
return;
|
||||
}
|
||||
|
||||
// Dual write: complexity_router_config.default_model (the pin marker hydratePinnedDefaultModel
|
||||
// reads back) and complexity_router_default_model (what the backend routes on) must always be
|
||||
// written together from the same value. Same pairing in add_auto_router_tab.tsx.
|
||||
const updatedLitellmParams = {
|
||||
...modelData.litellm_params,
|
||||
complexity_router_config: buildUpdatedComplexityRouterConfig(
|
||||
modelData.litellm_params?.complexity_router_config,
|
||||
complexityRouterConfig,
|
||||
customTechnicalKeywords,
|
||||
{
|
||||
keywordTierRules,
|
||||
escalationKeywords,
|
||||
semanticMatchingEnabled,
|
||||
embeddingModel,
|
||||
matchThreshold,
|
||||
},
|
||||
),
|
||||
complexity_router_config: updatedConfig,
|
||||
complexity_router_default_model: defaultModel,
|
||||
};
|
||||
const updatedModelInfo = {
|
||||
|
|
|
|||
|
|
@ -8074,3 +8074,24 @@ export const deleteMemory = async (accessToken: string, key: string): Promise<vo
|
|||
throw new Error(errorData);
|
||||
}
|
||||
};
|
||||
|
||||
export interface ComplexityRouterConfigValidation {
|
||||
valid: boolean;
|
||||
error?: string | null;
|
||||
}
|
||||
|
||||
export const validateAutoRouterConfig = async (
|
||||
accessToken: string,
|
||||
complexityRouterConfig: Record<string, unknown>,
|
||||
teamId?: string,
|
||||
): Promise<ComplexityRouterConfigValidation> => {
|
||||
try {
|
||||
return await apiClient.post<ComplexityRouterConfigValidation>("/auto_router/validate_complexity_router_config", {
|
||||
accessToken,
|
||||
body: { complexity_router_config: complexityRouterConfig, ...(teamId && { team_id: teamId }) },
|
||||
});
|
||||
} catch (error) {
|
||||
console.warn("Could not dry-run the complexity router config; the save will be validated server side", error);
|
||||
return { valid: true };
|
||||
}
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue