mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-06 08:16:43 +00:00
fix(auto_router): filter embedding models out of tier selects, require all tiers, add inline validation
The Add Auto Router complexity tab let chat models fill the embedding-model slot (and vice versa) since neither dropdown filtered on ModelGroup.mode, and submit only required at least one of the four tiers instead of all four. Adds getMissingTiersError alongside the existing getSemanticConfigError, and highlights unfilled tier/embedding selects inline once a submit attempt fails.
This commit is contained in:
parent
81db114c40
commit
0de308a5b8
7 changed files with 160 additions and 13 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,32 @@ 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 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,15 @@ 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 handleTierChange = (tier: keyof ComplexityTiers, model: string) => {
|
||||
onChange({
|
||||
|
|
@ -148,6 +152,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 +175,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>
|
||||
);
|
||||
|
|
@ -323,6 +334,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>
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -296,6 +304,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