diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index bb26350e1b1..5e1d55efa7c 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -938,6 +938,10 @@ class LiteLLMRoutes(enum.Enum): # Model cost map maintenance views (read-only status / source). "/schedule/model_cost_map_reload/status", "/model/cost_map/source", + # Assembles a classifier prompt from the caller's own tier definitions and returns it. + # A POST only because the prompt must not ride in a URL; it reads nothing and writes + # nothing, so it belongs with the GET on the same path that default-allow already covers. + "/auto_router/classifier/default_prompt", ] # Spend tracking reads (/spend/logs, /spend/logs/ui, /spend/keys, # /spend/users, /spend/tags, /spend/calculate, /cost/estimate). Admin diff --git a/litellm/proxy/management_endpoints/model_management_endpoints.py b/litellm/proxy/management_endpoints/model_management_endpoints.py index 7c9f19cc86f..5978aaa9e41 100644 --- a/litellm/proxy/management_endpoints/model_management_endpoints.py +++ b/litellm/proxy/management_endpoints/model_management_endpoints.py @@ -16,10 +16,10 @@ import json from collections.abc import Awaitable, Mapping, Sequence from json import JSONDecodeError from types import MappingProxyType -from typing import TYPE_CHECKING, Final, Literal, Protocol, cast +from typing import TYPE_CHECKING, Annotated, Final, Literal, Protocol, cast from fastapi import APIRouter, Depends, Header, HTTPException, Request, status -from pydantic import BaseModel, ConfigDict, Field, ValidationError +from pydantic import BaseModel, ConfigDict, Field, ValidationError, field_validator from litellm._logging import verbose_proxy_logger from litellm._uuid import uuid @@ -84,6 +84,7 @@ from litellm.router_strategy.complexity_router import ( TierDefinition, classification_system_prompt, custom_tier_classification_prompt, + normalize_classification_prompt, ) from litellm.router_utils.auto_router_model_naming import ( STRATEGY_ROUTER_PARAM_FIELDS, @@ -2233,9 +2234,11 @@ class AutoRouterClassifierPromptPreviewRequest(BaseModel): """ tier_definitions: tuple[TierDefinition, ...] - context_window_size: int = DEFAULT_CLASSIFIER_CONTEXT_WINDOW_SIZE + context_window_size: Annotated[int, Field(ge=0)] = DEFAULT_CLASSIFIER_CONTEXT_WINDOW_SIZE classification_prompt: str | None = None + _normalize_prompt = field_validator("classification_prompt")(normalize_classification_prompt) + @router.post( "/auto_router/classifier/default_prompt", @@ -2257,14 +2260,6 @@ async def preview_auto_router_classifier_prompt( TierDefinition validation is what makes the built-in-criteria lookup total: it rejects a non-built-in name carrying no description. Payload validity stays the dry-run's job. """ - if request.context_window_size < 0: - raise ProxyException( - message="context_window_size must be non-negative", - type=ProxyErrorTypes.bad_request_error, - code=status.HTTP_400_BAD_REQUEST, - param="context_window_size", - ) - return AutoRouterClassifierDefaultPromptResponse( system_prompt=custom_tier_classification_prompt( request.tier_definitions, request.classification_prompt, request.context_window_size diff --git a/litellm/router_strategy/complexity_router/__init__.py b/litellm/router_strategy/complexity_router/__init__.py index 9121249abd6..6cec118c0a8 100644 --- a/litellm/router_strategy/complexity_router/__init__.py +++ b/litellm/router_strategy/complexity_router/__init__.py @@ -20,6 +20,7 @@ from litellm.router_strategy.complexity_router.config import ( ComplexityTier, ReminderMarkerPair, TierDefinition, + normalize_classification_prompt, ) __all__ = [ @@ -33,4 +34,5 @@ __all__ = [ "TierDefinition", "classification_system_prompt", "custom_tier_classification_prompt", + "normalize_classification_prompt", ] diff --git a/litellm/router_strategy/complexity_router/config.py b/litellm/router_strategy/complexity_router/config.py index 2cc39f36db7..fc1773bfb87 100644 --- a/litellm/router_strategy/complexity_router/config.py +++ b/litellm/router_strategy/complexity_router/config.py @@ -99,6 +99,23 @@ MAX_TIER_DESCRIPTION_CHARS: Final[int] = 500 MAX_CLASSIFICATION_PROMPT_CHARS: Final[int] = 2000 +def normalize_classification_prompt(value: str | None) -> str | None: + """Strip, reject blank, and cap an operator-written classifier preamble. + + The single owner of the rule, so the dashboard's prompt preview normalizes exactly what the + write gate stores: previewing the raw value would render leading whitespace the router strips, + or an over-long prompt the write then rejects. + """ + if value is None: + return None + stripped: Final = value.strip() + if not stripped: + raise ValueError("must be non-empty; omit the field instead") + if len(stripped) > MAX_CLASSIFICATION_PROMPT_CHARS: + raise ValueError(f"classification_prompt exceeds {MAX_CLASSIFICATION_PROMPT_CHARS} characters") + return stripped + + class TierDefinition(BaseModel): """An operator-defined tier: the name the LLM classifier must return and its rubric description.""" @@ -1012,7 +1029,7 @@ class ComplexityRouterConfig(BaseModel): ) return self - @field_validator("fallback_tier", "classification_prompt") + @field_validator("fallback_tier") @classmethod def _reject_blank_optional_text(cls, value: str | None) -> str | None: if value is None: @@ -1024,10 +1041,8 @@ class ComplexityRouterConfig(BaseModel): @field_validator("classification_prompt") @classmethod - def _cap_classification_prompt(cls, value: str | None) -> str | None: - if value is not None and len(value) > MAX_CLASSIFICATION_PROMPT_CHARS: - raise ValueError(f"classification_prompt exceeds {MAX_CLASSIFICATION_PROMPT_CHARS} characters") - return value + def _normalize_classification_prompt_field(cls, value: str | None) -> str | None: + return normalize_classification_prompt(value) @property def has_custom_tiers(self) -> bool: diff --git a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py index 6c2f134d28e..78bada31e88 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py @@ -4324,6 +4324,87 @@ class TestAutoRouterClassifierDefaultPrompt: assert "tier_definitions" not in inspect.signature(get_auto_router_classifier_default_prompt).parameters assert list(inspect.signature(preview_auto_router_classifier_prompt).parameters) == ["request"] + @pytest.mark.asyncio + async def test_the_preview_normalizes_the_prompt_the_same_way_the_write_gate_stores_it(self): + """The preview exists to show what the router sends, so it must apply the write gate's own + normalization: an untrimmed preamble previewed raw shows whitespace the router strips.""" + from litellm.proxy.management_endpoints.model_management_endpoints import ( + AutoRouterClassifierPromptPreviewRequest, + preview_auto_router_classifier_prompt, + ) + from litellm.router_strategy.complexity_router.config import ComplexityRouterConfig + + raw = " Route for a payments team. " + response = await preview_auto_router_classifier_prompt( + AutoRouterClassifierPromptPreviewRequest.model_validate( + { + "tier_definitions": [ + {"name": "TRIAGE", "description": "quick"}, + {"name": "AUDIT", "description": "deep"}, + ], + "classification_prompt": raw, + } + ) + ) + stored = ComplexityRouterConfig.model_validate( + { + "tiers": {"TRIAGE": ["a"], "AUDIT": ["b"]}, + "tier_definitions": [ + {"name": "TRIAGE", "description": "quick"}, + {"name": "AUDIT", "description": "deep"}, + ], + "fallback_tier": "TRIAGE", + "classifier_type": "llm", + "classifier_llm_config": {"model": "m", "timeout_ms": 1}, + "classification_prompt": raw, + } + ).classification_prompt + assert response.system_prompt.startswith(stored) + + @pytest.mark.asyncio + async def test_the_preview_refuses_a_prompt_the_write_gate_would_reject(self): + """Previewing an over-long prompt would let an operator compose one that looks fine and then + fails on save, which is the drift this endpoint exists to prevent.""" + from pydantic import ValidationError as PydanticValidationError + + from litellm.proxy.management_endpoints.model_management_endpoints import ( + AutoRouterClassifierPromptPreviewRequest, + ) + from litellm.router_strategy.complexity_router.config import MAX_CLASSIFICATION_PROMPT_CHARS + + definitions = [{"name": "TRIAGE", "description": "quick"}, {"name": "AUDIT", "description": "deep"}] + for bad in ("x" * (MAX_CLASSIFICATION_PROMPT_CHARS + 1), " "): + with pytest.raises(PydanticValidationError): + AutoRouterClassifierPromptPreviewRequest.model_validate( + {"tier_definitions": definitions, "classification_prompt": bad} + ) + + @pytest.mark.asyncio + async def test_a_negative_context_window_is_rejected_by_the_field_not_a_hand_rolled_branch(self): + from pydantic import ValidationError as PydanticValidationError + + from litellm.proxy.management_endpoints.model_management_endpoints import ( + AutoRouterClassifierPromptPreviewRequest, + ) + + with pytest.raises(PydanticValidationError): + AutoRouterClassifierPromptPreviewRequest.model_validate( + { + "tier_definitions": [ + {"name": "TRIAGE", "description": "quick"}, + {"name": "AUDIT", "description": "deep"}, + ], + "context_window_size": -1, + } + ) + + def test_the_prompt_preview_is_readable_by_an_admin_viewer_like_the_get_beside_it(self): + """Both methods on this path are pure reads, so a role that may call the GET must not be + refused the POST purely because default-allow only covers safe methods.""" + from litellm.proxy._types import LiteLLMRoutes + + assert "/auto_router/classifier/default_prompt" in LiteLLMRoutes.admin_viewer_routes.value + @pytest.mark.asyncio async def test_malformed_tier_definitions_are_rejected_rather_than_silently_ignored(self): """A definition the router could not build a bullet from must fail loudly here rather than diff --git a/ui/litellm-dashboard/src/components/add_model/ClassificationMethodConfig.tsx b/ui/litellm-dashboard/src/components/add_model/ClassificationMethodConfig.tsx index eee1bcbf7a9..7a4dee87027 100644 --- a/ui/litellm-dashboard/src/components/add_model/ClassificationMethodConfig.tsx +++ b/ui/litellm-dashboard/src/components/add_model/ClassificationMethodConfig.tsx @@ -10,7 +10,7 @@ import { RadioGroup, RadioGroupItem } from "@/components/ui/radio-group"; import { Switch } from "@/components/ui/switch"; import React from "react"; import ClassifierPromptEditor from "./ClassifierPromptEditor"; -import ClassificationPromptEditor from "./ClassificationPromptEditor"; +import CustomTierPromptEditor from "./CustomTierPromptEditor"; import { Restricted, restrictedBy } from "./TierRestrictions"; import HeuristicScoringConfig from "./HeuristicScoringConfig"; import { useComplexityScorerDefaults } from "@/app/(dashboard)/hooks/autoRouter/useComplexityScorerDefaults"; @@ -416,7 +416,7 @@ const ClassificationMethodConfig: React.FC = ({
Classifier Prompt {value.custom_tier_set ? ( - ({ getAutoRouterCustomTierPromptCall: vi.fn(), @@ -19,7 +19,7 @@ const tierRows = [ const renderEditor = (classificationPrompt?: string) => { const onChange = vi.fn(); renderWithProviders( - { ); }); -describe("ClassificationPromptEditor", () => { +describe("CustomTierPromptEditor", () => { it("shows the prompt the proxy assembled rather than one rebuilt in the browser", async () => { renderEditor(); fireEvent.click(screen.getByRole("button", { name: "Edit prompt" })); diff --git a/ui/litellm-dashboard/src/components/add_model/ClassificationPromptEditor.tsx b/ui/litellm-dashboard/src/components/add_model/CustomTierPromptEditor.tsx similarity index 80% rename from ui/litellm-dashboard/src/components/add_model/ClassificationPromptEditor.tsx rename to ui/litellm-dashboard/src/components/add_model/CustomTierPromptEditor.tsx index 17d87fc3854..a60882a3de2 100644 --- a/ui/litellm-dashboard/src/components/add_model/ClassificationPromptEditor.tsx +++ b/ui/litellm-dashboard/src/components/add_model/CustomTierPromptEditor.tsx @@ -6,7 +6,7 @@ import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle } from " import { Textarea } from "@/components/ui/textarea"; import { TierRow, activeTierName } from "./tier_rows"; -interface ClassificationPromptEditorProps { +interface CustomTierPromptEditorProps { classificationPrompt: string | undefined; onChange: (classificationPrompt: string | undefined) => void; tierRows: readonly TierRow[]; @@ -25,7 +25,7 @@ const wireDefinitions = (tierRows: readonly TierRow[]): { name: string; descript ...(row.definition.trim() && { description: row.definition.trim() }), })); -const ClassificationPromptEditor: React.FC = ({ +const CustomTierPromptEditor: React.FC = ({ classificationPrompt, onChange, tierRows, @@ -34,7 +34,9 @@ const ClassificationPromptEditor: React.FC = ({ const { accessToken } = useAuthorized(); const [isOpen, setIsOpen] = useState(false); const [draft, setDraft] = useState(""); - const [preview, setPreview] = useState(null); + const [preview, setPreview] = useState< + { status: "loading" } | { status: "error" } | { status: "ready"; text: string } + >({ status: "loading" }); const isOverridden = Boolean(classificationPrompt?.trim()); // Debounced so the preview follows the draft without a request per keystroke. Nothing is saved @@ -42,11 +44,17 @@ const ClassificationPromptEditor: React.FC = ({ const refreshPreview = useCallback(async () => { if (!accessToken) return; try { - setPreview( - await getAutoRouterCustomTierPromptCall(accessToken, contextWindowSize, wireDefinitions(tierRows), draft), + const text = await getAutoRouterCustomTierPromptCall( + accessToken, + contextWindowSize, + wireDefinitions(tierRows), + draft, ); + setPreview({ status: "ready", text }); } catch { - setPreview(null); + // Distinct from loading: a role that may not call the preview, or a prompt the write gate + // would reject, otherwise leaves the panel claiming it is still fetching, forever. + setPreview({ status: "error" }); } }, [accessToken, contextWindowSize, tierRows, draft]); @@ -58,7 +66,7 @@ const ClassificationPromptEditor: React.FC = ({ const openEditor = () => { setDraft(classificationPrompt ?? ""); - setPreview(null); + setPreview({ status: "loading" }); setIsOpen(true); }; @@ -108,14 +116,20 @@ const ClassificationPromptEditor: React.FC = ({

What this router sends

- {preview === null ? ( + {preview.status === "loading" && (

Loading the assembled prompt…

- ) : ( + )} + {preview.status === "error" && ( +

+ Could not load the assembled prompt. Your text is still saved as written. +

+ )} + {preview.status === "ready" && ( 
-                {preview}
+                {preview.text}
)}
@@ -134,4 +148,4 @@ const ClassificationPromptEditor: React.FC = ({ ); }; -export default ClassificationPromptEditor; +export default CustomTierPromptEditor; 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 374b7ce60c3..c84464e2d77 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 @@ -70,10 +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.fn().mockResolvedValue({ valid: true }), + validateAutoRouterConfig, })); vi.mock("@/components/llm_calls/fetch_models", () => ({ @@ -190,6 +194,37 @@ describe("AddAutoRouterTab", () => { expect(vi.mocked(handleAddAutoRouterSubmit).mock.calls.at(-1)?.[0]).toMatchObject({ team_id: "team-1" }); }); + // The dry-run exists so a config the write gate would refuse shows the backend's own message + // inline instead of coming back as a raw 400. Nothing asserted that its verdict actually stops + // the submit, so the whole gate could be deleted with the suite still green. + 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(); + 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(); + 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()); + }); + // 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. diff --git a/ui/litellm-dashboard/src/components/edit_auto_router/edit_auto_router_modal.test.tsx b/ui/litellm-dashboard/src/components/edit_auto_router/edit_auto_router_modal.test.tsx index 01a734beb68..022dd0b7cad 100644 --- a/ui/litellm-dashboard/src/components/edit_auto_router/edit_auto_router_modal.test.tsx +++ b/ui/litellm-dashboard/src/components/edit_auto_router/edit_auto_router_modal.test.tsx @@ -10,17 +10,19 @@ vi.mock( async () => await import("../../../tests/mocks/complexityScorerDefaults"), ); -const { modelPatchUpdateCall, modelAvailableCall, getAutoRouterClassifierDefaultPromptCall } = vi.hoisted(() => ({ - modelPatchUpdateCall: vi.fn().mockResolvedValue({}), - modelAvailableCall: vi.fn().mockResolvedValue({ data: [] }), - getAutoRouterClassifierDefaultPromptCall: vi.fn().mockResolvedValue("Classify the request into exactly one tier."), -})); +const { modelPatchUpdateCall, modelAvailableCall, getAutoRouterClassifierDefaultPromptCall, validateAutoRouterConfig } = + vi.hoisted(() => ({ + validateAutoRouterConfig: vi.fn().mockResolvedValue({ valid: true }), + modelPatchUpdateCall: vi.fn().mockResolvedValue({}), + modelAvailableCall: vi.fn().mockResolvedValue({ data: [] }), + getAutoRouterClassifierDefaultPromptCall: vi.fn().mockResolvedValue("Classify the request into exactly one tier."), + })); vi.mock("../networking", () => ({ modelPatchUpdateCall, modelAvailableCall, getAutoRouterClassifierDefaultPromptCall, - validateAutoRouterConfig: vi.fn().mockResolvedValue({ valid: true }), + validateAutoRouterConfig, })); vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ default: () => ({ accessToken: "sk-test" }) })); @@ -97,6 +99,23 @@ describe("EditAutoRouterModal keyword matching", () => { expect(config.match_threshold).toBe(0.72); }); + // Same gate as the create form: the dry-run's verdict has to stop the PATCH, or an operator sees + // a raw 400 instead of the inline message the dry-run was added to give them. + 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. 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 fe0f6f4baba..36704a6d490 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 @@ -264,9 +264,10 @@ export const buildUpdatedComplexityRouterConfig = ( if (keywordMatching !== undefined && KEYWORD_MATCHING_KEYS.has(key)) return true; return customTechnicalKeywords !== undefined && key === "custom_technical_keywords"; }; - // A custom save drops the stored keys an edited tier set forbids; a built-in save drops a stored - // classification_prompt, which the backend accepts only beside tier_definitions. - const dropped: readonly string[] = value.custom_tier_set ? CUSTOM_TIER_OMITTED_KEYS : ["classification_prompt"]; + // A custom save drops the stored keys an edited tier set forbids. classification_prompt needs no + // entry here: it is a managed key, so a built-in save already drops it through isManaged and the + // built-in branch of the builder never re-emits it. + const dropped: readonly string[] = value.custom_tier_set ? CUSTOM_TIER_OMITTED_KEYS : []; const preservedConfig = Object.fromEntries( Object.entries(toRecord(storedConfig)).filter(([key]) => !isManaged(key) && !dropped.includes(key)), );