mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
Merge pull request #32978 from BerriAI/litellm_auto_router_qol
fix(auto_router): filter embedding models in complexity tab dropdowns, require all tiers, inline validation
This commit is contained in:
commit
8d358574ea
8 changed files with 241 additions and 14 deletions
|
|
@ -4,9 +4,10 @@ import { vi } from "vitest";
|
|||
import ComplexityRouterConfig, { ComplexityRouterConfigValue } from "./ComplexityRouterConfig";
|
||||
|
||||
const mockModelInfo = [
|
||||
{ model_group: "gpt-4" },
|
||||
{ model_group: "gpt-3.5-turbo" },
|
||||
{ model_group: "claude-3-opus" },
|
||||
{ model_group: "gpt-4", mode: "chat" },
|
||||
{ model_group: "gpt-3.5-turbo", mode: "chat" },
|
||||
{ model_group: "claude-3-opus", mode: "chat" },
|
||||
{ model_group: "text-embedding-3-small", mode: "embedding" },
|
||||
] as any[];
|
||||
|
||||
const defaultValue: ComplexityRouterConfigValue = {
|
||||
|
|
@ -207,4 +208,54 @@ describe("ComplexityRouterConfig", () => {
|
|||
await user.click(screen.getByRole("switch"));
|
||||
expect(onSemanticMatchingEnabledChange).toHaveBeenCalledWith(true, expect.anything());
|
||||
});
|
||||
|
||||
it("excludes embedding-mode models from the tier and classifier dropdowns", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(<ComplexityRouterConfig {...baseProps} />);
|
||||
|
||||
const simpleTierSection = screen.getByText("Simple Tier").closest(".mb-4") as HTMLElement;
|
||||
const combobox = within(simpleTierSection).getByRole("combobox");
|
||||
await user.click(combobox);
|
||||
|
||||
expect((await screen.findAllByText("gpt-3.5-turbo")).length).toBeGreaterThan(0);
|
||||
expect(screen.queryAllByText("text-embedding-3-small")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("does not show tier validation errors by default", () => {
|
||||
renderWithProviders(<ComplexityRouterConfig {...baseProps} />);
|
||||
expect(screen.queryByText("This tier is required")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows an inline error on the classifier model select when llm is selected without a model", () => {
|
||||
const llmValue: ComplexityRouterConfigValue = {
|
||||
...defaultValue,
|
||||
classifier_type: "llm",
|
||||
classifier_llm_config: { model: "", timeout_ms: 3000 },
|
||||
};
|
||||
renderWithProviders(<ComplexityRouterConfig {...baseProps} value={llmValue} showValidationErrors={true} />);
|
||||
fireEvent.click(screen.getByText("Advanced: Classification Method"));
|
||||
expect(screen.getByText("A classifier model is required")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("does not show the classifier model error once a classifier model is set", () => {
|
||||
const llmValue: ComplexityRouterConfigValue = {
|
||||
...defaultValue,
|
||||
classifier_type: "llm",
|
||||
classifier_llm_config: { model: "gpt-3.5-turbo", timeout_ms: 3000 },
|
||||
};
|
||||
renderWithProviders(<ComplexityRouterConfig {...baseProps} value={llmValue} showValidationErrors={true} />);
|
||||
fireEvent.click(screen.getByText("Advanced: Classification Method"));
|
||||
expect(screen.queryByText("A classifier model is required")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows a validation error only under unfilled tiers when showValidationErrors is true", () => {
|
||||
renderWithProviders(
|
||||
<ComplexityRouterConfig
|
||||
{...baseProps}
|
||||
value={{ ...defaultValue, tiers: { ...defaultValue.tiers, REASONING: "" } }}
|
||||
showValidationErrors={true}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getAllByText("This tier is required")).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ interface ComplexityRouterConfigProps {
|
|||
onEmbeddingModelChange?: (model: string) => void;
|
||||
matchThreshold?: number;
|
||||
onMatchThresholdChange?: (threshold: number) => void;
|
||||
showValidationErrors?: boolean;
|
||||
}
|
||||
|
||||
const TIER_DESCRIPTIONS: Record<keyof ComplexityTiers, { label: string; description: string; examples: string }> = {
|
||||
|
|
@ -84,12 +85,18 @@ const ComplexityRouterConfig: React.FC<ComplexityRouterConfigProps> = ({
|
|||
onEmbeddingModelChange = () => {},
|
||||
matchThreshold = 0.5,
|
||||
onMatchThresholdChange = () => {},
|
||||
showValidationErrors = false,
|
||||
}) => {
|
||||
// Prepare model options for dropdowns
|
||||
const modelOptions = modelInfo.map((model) => ({
|
||||
value: model.model_group,
|
||||
label: model.model_group,
|
||||
}));
|
||||
// Embedding models can't serve a chat-completion role, so they're excluded here.
|
||||
const modelOptions = modelInfo
|
||||
.filter((model) => model.mode !== "embedding")
|
||||
.map((model) => ({
|
||||
value: model.model_group,
|
||||
label: model.model_group,
|
||||
}));
|
||||
|
||||
const classifierModelMissing =
|
||||
showValidationErrors && value.classifier_type === "llm" && !value.classifier_llm_config?.model;
|
||||
|
||||
const handleTierChange = (tier: keyof ComplexityTiers, model: string) => {
|
||||
onChange({
|
||||
|
|
@ -148,6 +155,7 @@ const ComplexityRouterConfig: React.FC<ComplexityRouterConfigProps> = ({
|
|||
<Card>
|
||||
{(Object.keys(TIER_DESCRIPTIONS) as Array<keyof ComplexityTiers>).map((tier, index) => {
|
||||
const tierInfo = TIER_DESCRIPTIONS[tier];
|
||||
const tierMissing = showValidationErrors && !value.tiers[tier];
|
||||
return (
|
||||
<div key={tier}>
|
||||
{index > 0 && <Divider style={{ margin: "16px 0" }} />}
|
||||
|
|
@ -170,7 +178,13 @@ const ComplexityRouterConfig: React.FC<ComplexityRouterConfigProps> = ({
|
|||
showSearch
|
||||
style={{ width: "100%" }}
|
||||
options={modelOptions}
|
||||
status={tierMissing ? "error" : undefined}
|
||||
/>
|
||||
{tierMissing && (
|
||||
<Text type="danger" style={{ fontSize: 12 }}>
|
||||
This tier is required
|
||||
</Text>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
|
@ -222,7 +236,13 @@ const ComplexityRouterConfig: React.FC<ComplexityRouterConfigProps> = ({
|
|||
showSearch
|
||||
style={{ width: "100%" }}
|
||||
options={modelOptions}
|
||||
status={classifierModelMissing ? "error" : undefined}
|
||||
/>
|
||||
{classifierModelMissing && (
|
||||
<Text type="danger" style={{ fontSize: 12 }}>
|
||||
A classifier model is required
|
||||
</Text>
|
||||
)}
|
||||
</div>
|
||||
<div>
|
||||
<Text strong style={{ display: "block", marginBottom: 4 }}>
|
||||
|
|
@ -323,6 +343,7 @@ const ComplexityRouterConfig: React.FC<ComplexityRouterConfigProps> = ({
|
|||
matchThreshold={matchThreshold}
|
||||
onMatchThresholdChange={onMatchThresholdChange}
|
||||
modelInfo={modelInfo}
|
||||
showValidationErrors={showValidationErrors}
|
||||
/>
|
||||
</>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,53 @@
|
|||
import { renderWithProviders, screen } from "../../../tests/test-utils";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { vi } from "vitest";
|
||||
import SemanticKeywordMatching from "./SemanticKeywordMatching";
|
||||
|
||||
const mockModelInfo = [
|
||||
{ model_group: "gpt-4", mode: "chat" },
|
||||
{ model_group: "text-embedding-3-small", mode: "embedding" },
|
||||
{ model_group: "voyage-3-5", mode: "embedding" },
|
||||
{ model_group: "legacy-model" },
|
||||
] as any[];
|
||||
|
||||
const baseProps = {
|
||||
enabled: true,
|
||||
onEnabledChange: vi.fn(),
|
||||
embeddingModel: undefined,
|
||||
onEmbeddingModelChange: vi.fn(),
|
||||
matchThreshold: 0.5,
|
||||
onMatchThresholdChange: vi.fn(),
|
||||
modelInfo: mockModelInfo,
|
||||
};
|
||||
|
||||
describe("SemanticKeywordMatching", () => {
|
||||
it("only lists embedding-mode models in the embedding model dropdown", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(<SemanticKeywordMatching {...baseProps} />);
|
||||
|
||||
const combobox = screen.getByRole("combobox");
|
||||
await user.click(combobox);
|
||||
|
||||
expect((await screen.findAllByText("text-embedding-3-small")).length).toBeGreaterThan(0);
|
||||
expect(screen.getAllByText("voyage-3-5").length).toBeGreaterThan(0);
|
||||
expect(screen.queryAllByText("gpt-4")).toHaveLength(0);
|
||||
expect(screen.queryAllByText("legacy-model")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("does not show a validation error by default", () => {
|
||||
renderWithProviders(<SemanticKeywordMatching {...baseProps} showValidationErrors={false} />);
|
||||
expect(screen.queryByText("An embedding model is required")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows a validation error when showValidationErrors is true and no embedding model is set", () => {
|
||||
renderWithProviders(<SemanticKeywordMatching {...baseProps} showValidationErrors={true} />);
|
||||
expect(screen.getByText("An embedding model is required")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("hides the validation error once an embedding model is set", () => {
|
||||
renderWithProviders(
|
||||
<SemanticKeywordMatching {...baseProps} showValidationErrors={true} embeddingModel="voyage-3-5" />,
|
||||
);
|
||||
expect(screen.queryByText("An embedding model is required")).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
@ -15,6 +15,7 @@ interface SemanticKeywordMatchingProps {
|
|||
matchThreshold: number;
|
||||
onMatchThresholdChange: (threshold: number) => void;
|
||||
modelInfo: ModelGroup[];
|
||||
showValidationErrors?: boolean;
|
||||
}
|
||||
|
||||
const SemanticKeywordMatching: React.FC<SemanticKeywordMatchingProps> = ({
|
||||
|
|
@ -25,11 +26,14 @@ const SemanticKeywordMatching: React.FC<SemanticKeywordMatchingProps> = ({
|
|||
matchThreshold,
|
||||
onMatchThresholdChange,
|
||||
modelInfo,
|
||||
showValidationErrors = false,
|
||||
}) => {
|
||||
const modelOptions = Array.from(new Set(modelInfo.map((model) => model.model_group))).map((model_group) => ({
|
||||
const embeddingModels = modelInfo.filter((model) => model.mode === "embedding");
|
||||
const modelOptions = Array.from(new Set(embeddingModels.map((model) => model.model_group))).map((model_group) => ({
|
||||
value: model_group,
|
||||
label: model_group,
|
||||
}));
|
||||
const embeddingModelMissing = showValidationErrors && !embeddingModel;
|
||||
|
||||
return (
|
||||
<Card className="mb-4">
|
||||
|
|
@ -60,7 +64,13 @@ const SemanticKeywordMatching: React.FC<SemanticKeywordMatchingProps> = ({
|
|||
showSearch
|
||||
style={{ width: "100%" }}
|
||||
options={modelOptions}
|
||||
status={embeddingModelMissing ? "error" : undefined}
|
||||
/>
|
||||
{embeddingModelMissing && (
|
||||
<Text type="danger" style={{ fontSize: 12 }}>
|
||||
An embedding model is required
|
||||
</Text>
|
||||
)}
|
||||
</div>
|
||||
<div>
|
||||
<Text className="text-sm font-medium mb-1 block">Minimum match score</Text>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,40 @@
|
|||
import { renderWithProviders, screen } from "../../../tests/test-utils";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { vi } from "vitest";
|
||||
import { Form } from "antd";
|
||||
import AddAutoRouterTab from "./add_auto_router_tab";
|
||||
import NotificationManager from "../molecules/notifications_manager";
|
||||
|
||||
vi.mock("../networking", () => ({
|
||||
modelAvailableCall: vi.fn().mockResolvedValue({ data: [] }),
|
||||
}));
|
||||
|
||||
vi.mock("@/components/llm_calls/fetch_models", () => ({
|
||||
fetchAvailableModels: vi.fn().mockResolvedValue([]),
|
||||
}));
|
||||
|
||||
vi.mock("./handle_add_auto_router_submit", () => ({
|
||||
handleAddAutoRouterSubmit: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../molecules/notifications_manager", () => ({
|
||||
default: { fromBackend: vi.fn() },
|
||||
}));
|
||||
|
||||
const Harness = () => {
|
||||
const [form] = Form.useForm();
|
||||
return <AddAutoRouterTab form={form} handleOk={vi.fn()} accessToken="token" userRole="Admin" />;
|
||||
};
|
||||
|
||||
describe("AddAutoRouterTab", () => {
|
||||
it("flags every mandatory field when Add Auto Router is clicked with nothing filled", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(<Harness />);
|
||||
|
||||
await user.click(screen.getByRole("button", { name: /add auto router/i }));
|
||||
|
||||
expect(await screen.findByText("Auto router name is required")).toBeInTheDocument();
|
||||
expect(screen.getAllByText("This tier is required")).toHaveLength(4);
|
||||
expect(NotificationManager.fromBackend).toHaveBeenCalledWith("Please enter an Auto Router Name");
|
||||
});
|
||||
});
|
||||
|
|
@ -11,7 +11,11 @@ import RouterConfigBuilder from "./RouterConfigBuilder";
|
|||
import ComplexityRouterConfig, { ComplexityRouterConfigValue } from "./ComplexityRouterConfig";
|
||||
import { KeywordTierRule } from "./KeywordTierRules";
|
||||
import { DEFAULT_MATCH_THRESHOLD } from "./SemanticKeywordMatching";
|
||||
import { buildComplexityRouterConfig, getSemanticConfigError } from "./build_complexity_router_config";
|
||||
import {
|
||||
buildComplexityRouterConfig,
|
||||
getMissingTiersError,
|
||||
getSemanticConfigError,
|
||||
} from "./build_complexity_router_config";
|
||||
import { buildAutoRouterTestTargets, AutoRouterTestTarget } from "./build_auto_router_test_targets";
|
||||
import AutoRouterConnectionTest from "./auto_router_connection_test";
|
||||
import NotificationManager from "../molecules/notifications_manager";
|
||||
|
|
@ -43,6 +47,7 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({ form, handleOk, acc
|
|||
const [semanticMatchingEnabled, setSemanticMatchingEnabled] = useState<boolean>(false);
|
||||
const [embeddingModel, setEmbeddingModel] = useState<string | undefined>(undefined);
|
||||
const [matchThreshold, setMatchThreshold] = useState<number>(DEFAULT_MATCH_THRESHOLD);
|
||||
const [showValidationErrors, setShowValidationErrors] = useState<boolean>(false);
|
||||
|
||||
// Semantic router config (existing)
|
||||
const [routerConfig, setRouterConfig] = useState<any>(null);
|
||||
|
|
@ -86,19 +91,22 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({ form, handleOk, acc
|
|||
classifier_llm_config: classifierLlmConfig,
|
||||
} = complexityRouterConfig;
|
||||
|
||||
const filledTiers = Object.values(tiers).filter(Boolean);
|
||||
if (filledTiers.length === 0) {
|
||||
NotificationManager.fromBackend("Please select at least one model for a complexity tier");
|
||||
const missingTiersError = getMissingTiersError(tiers);
|
||||
if (missingTiersError) {
|
||||
setShowValidationErrors(true);
|
||||
NotificationManager.fromBackend(missingTiersError);
|
||||
return;
|
||||
}
|
||||
|
||||
if (classifierType === "llm" && !classifierLlmConfig?.model) {
|
||||
setShowValidationErrors(true);
|
||||
NotificationManager.fromBackend("Please select a classifier model, or switch back to Heuristic");
|
||||
return;
|
||||
}
|
||||
|
||||
const semanticError = getSemanticConfigError({ semanticMatchingEnabled, embeddingModel, keywordTierRules });
|
||||
if (semanticError) {
|
||||
setShowValidationErrors(true);
|
||||
NotificationManager.fromBackend(semanticError);
|
||||
return;
|
||||
}
|
||||
|
|
@ -190,6 +198,8 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({ form, handleOk, acc
|
|||
const handleAutoRouterSubmit = () => {
|
||||
const name = form.getFieldValue("auto_router_name");
|
||||
if (!name) {
|
||||
setShowValidationErrors(true);
|
||||
form.validateFields(["auto_router_name"]).catch(() => undefined);
|
||||
NotificationManager.fromBackend("Please enter an Auto Router Name");
|
||||
return;
|
||||
}
|
||||
|
|
@ -230,7 +240,14 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({ form, handleOk, acc
|
|||
<Card className="mb-4">
|
||||
<div className="mb-4">
|
||||
<Text className="text-sm font-medium mb-2 block">Router Type</Text>
|
||||
<Radio.Group value={routerType} onChange={(e) => setRouterType(e.target.value)} className="w-full">
|
||||
<Radio.Group
|
||||
value={routerType}
|
||||
onChange={(e) => {
|
||||
setRouterType(e.target.value);
|
||||
setShowValidationErrors(false);
|
||||
}}
|
||||
className="w-full"
|
||||
>
|
||||
<Space direction="vertical" className="w-full">
|
||||
<Radio value="recommended" className="w-full">
|
||||
<div className="flex items-center gap-2">
|
||||
|
|
@ -296,6 +313,7 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({ form, handleOk, acc
|
|||
onEmbeddingModelChange={setEmbeddingModel}
|
||||
matchThreshold={matchThreshold}
|
||||
onMatchThresholdChange={setMatchThreshold}
|
||||
showValidationErrors={showValidationErrors}
|
||||
/>
|
||||
</div>
|
||||
) : (
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import {
|
||||
buildComplexityRouterConfig,
|
||||
getMissingTiersError,
|
||||
getSemanticConfigError,
|
||||
BuildComplexityRouterConfigParams,
|
||||
} from "./build_complexity_router_config";
|
||||
|
|
@ -124,6 +125,31 @@ describe("buildComplexityRouterConfig", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("getMissingTiersError", () => {
|
||||
it("returns null when all four tiers have a model", () => {
|
||||
expect(getMissingTiersError(tiers)).toBeNull();
|
||||
});
|
||||
|
||||
it("names the specific missing tier when only one is blank", () => {
|
||||
expect(getMissingTiersError({ ...tiers, REASONING: "" })).toBe(
|
||||
"Select a model for the following tier(s): REASONING",
|
||||
);
|
||||
});
|
||||
|
||||
it("names multiple missing tiers in SIMPLE/MEDIUM/COMPLEX/REASONING order", () => {
|
||||
expect(getMissingTiersError({ ...tiers, SIMPLE: "", REASONING: "" })).toBe(
|
||||
"Select a model for the following tier(s): SIMPLE, REASONING",
|
||||
);
|
||||
});
|
||||
|
||||
it("names all four tiers when none are filled", () => {
|
||||
const noTiers = { SIMPLE: "", MEDIUM: "", COMPLEX: "", REASONING: "" };
|
||||
expect(getMissingTiersError(noTiers)).toBe(
|
||||
"Select a model for the following tier(s): SIMPLE, MEDIUM, COMPLEX, REASONING",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("getSemanticConfigError", () => {
|
||||
const rule = { id: "r1", keywords: ["k8s"], tier: "REASONING" as const };
|
||||
|
||||
|
|
|
|||
|
|
@ -30,6 +30,14 @@ export interface ComplexityRouterConfigPayload {
|
|||
match_threshold?: number;
|
||||
}
|
||||
|
||||
const TIER_KEYS: Array<keyof ComplexityTiers> = ["SIMPLE", "MEDIUM", "COMPLEX", "REASONING"];
|
||||
|
||||
export const getMissingTiersError = (tiers: ComplexityTiers): string | null => {
|
||||
const missing = TIER_KEYS.filter((tier) => !tiers[tier]);
|
||||
if (missing.length === 0) return null;
|
||||
return `Select a model for the following tier(s): ${missing.join(", ")}`;
|
||||
};
|
||||
|
||||
export const getSemanticConfigError = ({
|
||||
semanticMatchingEnabled,
|
||||
embeddingModel,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue