From ff7ba4c6df240f03087c881af415f89cd1618f95 Mon Sep 17 00:00:00 2001 From: tin-berri Date: Wed, 26 Aug 2026 18:56:41 -0700 Subject: [PATCH] fix(ui): block the auto-router submit on a missing classifier model and an orphaned keyword rule (#38427) Two gaps the create form and the edit modal share today. The submit gate never asked for a classifier model. Choosing the LLM classifier and no model leaves Test Routing and Add Auto Router enabled, so Test Routing posts a config the backend rejects and only the later save says why. The keyword-rule gate only looked for empty keyword rows. A rule's tier has been a free string since #37413, and the backend matches it exactly, so a rule naming a tier the router does not have cleared the gate and failed the save as a raw 400. Both gates now live in build_complexity_router_config.ts, and each form's submit handler reads the same blocked reason the button reads instead of re-deriving its own list, so a disabled button and a refused submit cannot disagree. --- .../add_model/add_auto_router_tab.test.tsx | 37 ++++++++++ .../add_model/add_auto_router_tab.tsx | 61 +++++------------ .../build_complexity_router_config.test.ts | 68 ++++++++++++++++--- .../build_complexity_router_config.ts | 25 ++++++- .../edit_auto_router_modal.tsx | 11 +-- 5 files changed, 143 insertions(+), 59 deletions(-) diff --git a/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.test.tsx b/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.test.tsx index 1219ac6138b..c5924bdc959 100644 --- a/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.test.tsx +++ b/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.test.tsx @@ -5,6 +5,8 @@ import AddAutoRouterTab from "./add_auto_router_tab"; import { toast } from "@/lib/toast"; import { handleAddAutoRouterSubmit } from "./handle_add_auto_router_submit"; import { getMissingTiersError } from "./build_complexity_router_config"; +import { getSubmitBlockedReason } from "./add_auto_router_tab"; +import { buildModelAvailability } from "@/lib/autorouter_presets"; import { testAutoRouterRouting } from "../networking"; import { ModelGroup } from "@/components/llm_calls/fetch_models"; import { getAllPresets, getPresetByKey, getRequiredModelsInPreset } from "@/lib/autorouter_presets"; @@ -864,3 +866,38 @@ describe("AddAutoRouterTab", () => { }); }); }); + +describe("getSubmitBlockedReason", () => { + const tiers = { + SIMPLE: ["gpt-4o-mini"], + MEDIUM: ["gpt-4o-mini"], + COMPLEX: ["gpt-4o-mini"], + REASONING: ["gpt-4o-mini"], + }; + const availability = buildModelAvailability(["gpt-4o-mini"], []); + const referenced = { + tiers, + classifierType: "heuristic" as const, + classifierLlmConfig: undefined, + semanticMatchingEnabled: false, + embeddingModel: undefined, + defaultModel: undefined, + }; + + it("lets a complete heuristic router through", () => { + expect(getSubmitBlockedReason({ tiers, classifier_type: "heuristic" }, [], referenced, availability)).toBeNull(); + }); + + it("blocks an LLM classifier with no model, which the button previously left enabled", () => { + expect(getSubmitBlockedReason({ tiers, classifier_type: "llm" }, [], referenced, availability)).toContain( + "Please select a classifier model", + ); + }); + + it("blocks a keyword rule aimed at a tier this router does not have", () => { + const rules = [{ id: "r1", keywords: ["audit"], tier: "AUDIT" }]; + expect(getSubmitBlockedReason({ tiers, classifier_type: "heuristic" }, rules, referenced, availability)).toContain( + "no longer has", + ); + }); +}); diff --git a/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.tsx b/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.tsx index 87c4754f56d..be4c09dcd22 100644 --- a/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.tsx +++ b/ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.tsx @@ -34,6 +34,7 @@ import { BuildComplexityRouterConfigParams, buildComplexityRouterConfig, getKeywordTierRulesError, + getClassifierModelError, getMissingTiersError, getPlanModeTierError, getSemanticConfigError, @@ -116,7 +117,7 @@ const tierConfigSummary = (config: ComplexityRouterConfigValue): string => { // itself and to say what is missing, so the two can never give different answers. Checks the // config actually being built, not which preset (if any) it came from: a preset only ever // prefills once (handlePresetChange), and everything after that is edited exactly like Custom. -const getSubmitBlockedReason = ( +export const getSubmitBlockedReason = ( config: ComplexityRouterConfigValue, keywordTierRules: KeywordTierRule[], referencedModelsParams: Parameters[0], @@ -125,7 +126,8 @@ const getSubmitBlockedReason = ( getMissingTiersError(activeTierRows(config)) ?? getTierLabelsError(config.tier_labels) ?? getPlanModeTierError(config.plan_mode_min_tier, activeTierRows(config)) ?? - getKeywordTierRulesError(keywordTierRules) ?? + getKeywordTierRulesError(keywordTierRules, activeTierRows(config)) ?? + getClassifierModelError(config) ?? getReferencedModelsError(referencedModelsParams, availability); const autoRouterSchema = (requiresTeamScope: boolean) => @@ -370,50 +372,21 @@ const AddAutoRouterTab: React.FC = ({ }; const submitRecommendedRouter = async (name: string) => { - const { tiers, tierLabels, classifierType, classifierLlmConfig } = complexityRouterConfigParams; + const { tiers } = complexityRouterConfigParams; - const missingTiersError = getMissingTiersError(activeTierRows(complexityRouterConfig)); - if (missingTiersError) { + // The one answer the submit button reads, so a disabled button and a refused submit cannot + // disagree about why. The handler needs it in its own right: the form fires this on Enter + // regardless of the button's disabled state. + const blockedReason = + getSubmitBlockedReason( + complexityRouterConfig, + keywordTierRules, + referencedModelsParams, + groupsOnlyAvailability, + ) ?? getSemanticConfigError({ semanticMatchingEnabled, embeddingModel, keywordTierRules }); + if (blockedReason) { setShowValidationErrors(true); - toast.fromError(missingTiersError); - return; - } - - const tierLabelsError = getTierLabelsError(tierLabels); - if (tierLabelsError) { - setShowValidationErrors(true); - toast.fromError(tierLabelsError); - return; - } - - if (classifierType === "llm" && !classifierLlmConfig?.model) { - setShowValidationErrors(true); - toast.fromError("Please select a classifier model, or switch back to Heuristic"); - return; - } - - const keywordRulesError = getKeywordTierRulesError(keywordTierRules); - if (keywordRulesError) { - setShowValidationErrors(true); - toast.fromError(keywordRulesError); - return; - } - - const semanticError = getSemanticConfigError({ semanticMatchingEnabled, embeddingModel, keywordTierRules }); - if (semanticError) { - setShowValidationErrors(true); - toast.fromError(semanticError); - return; - } - - // submitBlockedReason already disables the button for this, but the form's submit handler (wired to - // this same function) fires on Enter regardless of the button's disabled state - without this check, - // Enter in the name field could still create a router referencing a model that disappeared from - // availableModelSet after the tiers were filled in. - const referencedModelsError = getReferencedModelsError(referencedModelsParams, groupsOnlyAvailability); - if (referencedModelsError) { - setShowValidationErrors(true); - toast.fromError(referencedModelsError); + toast.fromError(blockedReason); return; } diff --git a/ui/litellm-dashboard/src/components/add_model/build_complexity_router_config.test.ts b/ui/litellm-dashboard/src/components/add_model/build_complexity_router_config.test.ts index 63545f5b7de..d85bc596bc7 100644 --- a/ui/litellm-dashboard/src/components/add_model/build_complexity_router_config.test.ts +++ b/ui/litellm-dashboard/src/components/add_model/build_complexity_router_config.test.ts @@ -3,6 +3,7 @@ import { getPlanModeTierError, normalizeClassifierLlmConfig, getKeywordTierRulesError, + getClassifierModelError, getMissingTiersError, getSemanticConfigError, getTierLabelsError, @@ -334,21 +335,24 @@ describe("getSemanticConfigError", () => { describe("getKeywordTierRulesError", () => { it("returns null when every rule carries a keyword", () => { expect( - getKeywordTierRulesError([ - { id: "r1", keywords: ["invoice"], tier: "MEDIUM" }, - { id: "r2", keywords: ["deploy to k8s"], tier: "REASONING" }, - ]), + getKeywordTierRulesError( + [ + { id: "r1", keywords: ["invoice"], tier: "MEDIUM" }, + { id: "r2", keywords: ["deploy to k8s"], tier: "REASONING" }, + ], + activeTierRows({ tiers }), + ), ).toBeNull(); }); it("returns null when there are no rules at all, since the section is optional", () => { - expect(getKeywordTierRulesError([])).toBeNull(); + expect(getKeywordTierRulesError([], activeTierRows({ tiers }))).toBeNull(); }); // The whole point of the ticket: the semantic toggle is off by default, and an unfilled row // used to be discarded silently on an otherwise successful create. it("rejects a row left empty while semantic matching is off", () => { - expect(getKeywordTierRulesError([{ id: "r1", keywords: [], tier: "COMPLEX" }])).toBe( + expect(getKeywordTierRulesError([{ id: "r1", keywords: [], tier: "COMPLEX" }], activeTierRows({ tiers }))).toBe( "Add at least one keyword to keyword rule(s): 1", ); }); @@ -357,7 +361,9 @@ describe("getKeywordTierRulesError", () => { ["whitespace only", [" "]], ["blank strings, as an unfilled row between filled ones leaves behind", ["", " ", ""]], ])("treats %s as empty rather than as a keyword", (_label, keywords) => { - expect(getKeywordTierRulesError([{ id: "r1", keywords, tier: "SIMPLE" }])).toMatch(/keyword rule\(s\): 1/); + expect(getKeywordTierRulesError([{ id: "r1", keywords, tier: "SIMPLE" }], activeTierRows({ tiers }))).toMatch( + /keyword rule\(s\): 1/, + ); }); // Row numbers have to survive rules that are fine, or the message points at the wrong input. @@ -373,7 +379,9 @@ describe("getKeywordTierRulesError", () => { }); it("keeps a keyword whose surrounding whitespace is the only thing trimmed", () => { - expect(getKeywordTierRulesError([{ id: "r1", keywords: [" invoice "], tier: "MEDIUM" }])).toBeNull(); + expect( + getKeywordTierRulesError([{ id: "r1", keywords: [" invoice "], tier: "MEDIUM" }], activeTierRows({ tiers })), + ).toBeNull(); }); }); @@ -674,3 +682,47 @@ describe("buildComplexityRouterConfig tier model params", () => { }); }); }); + +describe("getClassifierModelError", () => { + it("stays quiet for a heuristic router, which needs no classifier model", () => { + expect(getClassifierModelError({ classifier_type: "heuristic" })).toBeNull(); + }); + + it("blocks an LLM classifier with no model, which the router cannot start without", () => { + expect(getClassifierModelError({ classifier_type: "llm" })).toBe( + "Please select a classifier model, or switch back to Heuristic", + ); + }); + + it("stays quiet once a model is chosen", () => { + expect( + getClassifierModelError({ classifier_type: "llm", classifier_llm_config: { model: "m", timeout_ms: 3000 } }), + ).toBeNull(); + }); +}); + +describe("getKeywordTierRulesError orphaned tiers", () => { + const rows = activeTierRows({ tiers }); + + it("accepts a rule naming a tier the router has", () => { + expect(getKeywordTierRulesError([{ id: "r1", keywords: ["k"], tier: "COMPLEX" }], rows)).toBeNull(); + }); + + it("names the rule pointing at a tier this router does not have", () => { + expect(getKeywordTierRulesError([{ id: "r1", keywords: ["k"], tier: "AUDIT" }], rows)).toBe( + "Keyword rule(s) 1 route to a tier this router no longer has", + ); + }); + + it("rejects a differently cased tier, because _validate_keyword_rule_tiers matches exactly", () => { + expect(getKeywordTierRulesError([{ id: "r1", keywords: ["k"], tier: "complex" }], rows)).toBe( + "Keyword rule(s) 1 route to a tier this router no longer has", + ); + }); + + it("reports an empty keyword row before an orphaned tier, since that is the nearer problem", () => { + expect(getKeywordTierRulesError([{ id: "r1", keywords: [], tier: "AUDIT" }], rows)).toContain( + "Add at least one keyword", + ); + }); +}); diff --git a/ui/litellm-dashboard/src/components/add_model/build_complexity_router_config.ts b/ui/litellm-dashboard/src/components/add_model/build_complexity_router_config.ts index 241cae25705..9db569cb4e9 100644 --- a/ui/litellm-dashboard/src/components/add_model/build_complexity_router_config.ts +++ b/ui/litellm-dashboard/src/components/add_model/build_complexity_router_config.ts @@ -9,6 +9,7 @@ import { ClassifierLLMConfig, ClassifierType, ComplexityTierLabels, + ComplexityRouterConfigValue, ComplexityTiers, DimensionWeights, TIER_KEYS, @@ -187,12 +188,30 @@ export const getPlanModeTierError = (planModeMinTier: string | undefined, rows: return `The plan-mode minimum tier (${floor ? activeTierName(floor) : planModeMinTier}) has no models. Add one or turn the override off.`; }; -export const getKeywordTierRulesError = (keywordTierRules: KeywordTierRule[]): string | null => { +// The tier is a free string since #37413, and _validate_keyword_rule_tiers matches it EXACTLY, so a +// rule naming a tier this router does not have is a raw 400 unless the gate catches it first. +export const getKeywordTierRulesError = ( + keywordTierRules: KeywordTierRule[], + rows: readonly TierRow[], +): string | null => { const emptyRows = emptyKeywordTierRuleIndexes(keywordTierRules); - if (emptyRows.length === 0) return null; - return `Add at least one keyword to keyword rule(s): ${emptyRows.map((index) => index + 1).join(", ")}`; + if (emptyRows.length > 0) + return `Add at least one keyword to keyword rule(s): ${emptyRows.map((index) => index + 1).join(", ")}`; + const names = rows.map(activeTierName); + const orphaned = keywordTierRules.flatMap((rule, index) => (names.includes(rule.tier) ? [] : [index + 1])); + if (orphaned.length === 0) return null; + return `Keyword rule(s) ${orphaned.join(", ")} route to a tier this router no longer has`; }; +// The submit gate and the submit handler both read this, so a disabled button and a refused submit +// cannot disagree about why. +export const getClassifierModelError = ( + config: Pick, +): string | null => + config.classifier_type === "llm" && !config.classifier_llm_config?.model + ? "Please select a classifier model, or switch back to Heuristic" + : null; + export const getSemanticConfigError = ({ semanticMatchingEnabled, embeddingModel, diff --git a/ui/litellm-dashboard/src/components/edit_auto_router/edit_auto_router_modal.tsx b/ui/litellm-dashboard/src/components/edit_auto_router/edit_auto_router_modal.tsx index 8f4b06d80fb..4d7d33fa45a 100644 --- a/ui/litellm-dashboard/src/components/edit_auto_router/edit_auto_router_modal.tsx +++ b/ui/litellm-dashboard/src/components/edit_auto_router/edit_auto_router_modal.tsx @@ -20,6 +20,7 @@ import { isComplexityRouter } from "../add_model/auto_router_strategies"; import { type BuildComplexityRouterConfigParams, buildComplexityRouterConfig, + getClassifierModelError, getKeywordTierRulesError, getSemanticConfigError, getPlanModeTierError, @@ -268,7 +269,8 @@ const EditAutoRouterModal: React.FC = ({ : null) ?? getTierLabelsError(complexityRouterConfig.tier_labels) ?? getPlanModeTierError(complexityRouterConfig.plan_mode_min_tier, activeTierRows(complexityRouterConfig)) ?? - getKeywordTierRulesError(keywordTierRules); + getKeywordTierRulesError(keywordTierRules, activeTierRows(complexityRouterConfig)) ?? + getClassifierModelError(complexityRouterConfig); useEffect(() => { if (isVisible && modelData) { @@ -428,16 +430,17 @@ const EditAutoRouterModal: React.FC = ({ toast.fromError("Please select at least one model for a complexity tier"); return; } - if (classifier_type === "llm" && !classifier_llm_config?.model) { + const classifierError = getClassifierModelError(complexityRouterConfig); + if (classifierError) { setShowValidationErrors(true); - toast.fromError("Please select a classifier model, or switch back to Heuristic"); + toast.fromError(classifierError); return; } // Same guards the create form applies (add_auto_router_tab.tsx). The backend rejects a // keyword rule with no keyword, and semantic_keyword_matching without an embedding model // or keyword rules (complexity_router/config.py), so without these a save fails as a raw // 400 instead of an inline message. - const keywordRulesError = getKeywordTierRulesError(keywordTierRules); + const keywordRulesError = getKeywordTierRulesError(keywordTierRules, activeTierRows(complexityRouterConfig)); if (keywordRulesError) { setShowValidationErrors(true); toast.fromError(keywordRulesError);