mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
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.
This commit is contained in:
parent
3eba0b332a
commit
ff7ba4c6df
5 changed files with 143 additions and 59 deletions
|
|
@ -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",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<typeof getReferencedModelsError>[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<AddAutoRouterTabProps> = ({
|
|||
};
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<ComplexityRouterConfigValue, "classifier_type" | "classifier_llm_config">,
|
||||
): 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,
|
||||
|
|
|
|||
|
|
@ -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<EditAutoRouterModalProps> = ({
|
|||
: 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<EditAutoRouterModalProps> = ({
|
|||
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);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue