From a7abe57123b9f613165e1065c742ce9707f8b25c Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Sat, 15 Aug 2026 12:06:35 -0700 Subject: [PATCH] fix(ui): mirror backend precedence when resolving a complexity router default The edit modal only recognized a pin stored in complexity_router_config.default_model, so a router whose default lived solely in litellm_params.complexity_router_default_model lost it on the next save. That field cannot be trusted outright either: before this PR every save wrote a tier-derived value into it, so treating any value as a pin would freeze legacy routers away from their tiers. Hydration now takes the config marker as authoritative and falls back to litellm_params only when it diverges from what the tiers alone derive, which is only reachable through an external API or config write. Test Connection had the mirror-image bug: it fell back to complexity_router_config.default_model, a UI-only marker init_complexity_router_deployment never reads, so it could probe a model the router would never call. It now follows router.py exactly: litellm_params, else pure tier-derivation. Also reword a tooltip that hardcoded the Default Model select's position on the page, and document the dual write and the create-vs-edit validation asymmetry. --- .../add_model/ClassificationMethodConfig.tsx | 2 +- .../add_model/add_auto_router_tab.tsx | 4 + .../build_complexity_router_config.ts | 6 ++ .../edit_auto_router_modal.test.tsx | 86 +++++++++++++++++++ .../edit_auto_router_modal.tsx | 46 +++++++--- .../src/components/model_info_view.test.tsx | 38 ++++++++ .../src/components/model_info_view.tsx | 9 +- 7 files changed, 177 insertions(+), 14 deletions(-) diff --git a/ui/litellm-dashboard/src/components/add_model/ClassificationMethodConfig.tsx b/ui/litellm-dashboard/src/components/add_model/ClassificationMethodConfig.tsx index a7af270cf8e..6c7c086c8a2 100644 --- a/ui/litellm-dashboard/src/components/add_model/ClassificationMethodConfig.tsx +++ b/ui/litellm-dashboard/src/components/add_model/ClassificationMethodConfig.tsx @@ -230,7 +230,7 @@ const ClassificationMethodConfig: React.FC = ({ 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 78019b8a547..e4c8b9ac1f4 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 @@ -365,6 +365,10 @@ const AddAutoRouterTab: React.FC = ({ form .validateFields(requiresTeamScope ? ["auto_router_name", "team_id"] : ["auto_router_name"]) .then((values) => { + // auto_router_default_model (-> litellm_params, read by the backend at init) and + // complexity_router_config.default_model (-> the pin marker read back on edit, see + // hydratePinnedDefaultModel in edit_auto_router_modal.tsx) must both come from the same + // `defaultModel`, or the two fields diverge and hydration's divergence check misfires. const submitValues = { ...values, auto_router_name: name, 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 1692c6b0244..1a45bfbd635 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 @@ -106,6 +106,12 @@ export const getTierLabelsError = (tierLabels: ComplexityTierLabels | undefined) return null; }; +// Requires all 4 tiers non-empty, so the create form can never reach the +// resolveComplexityDefaultModel(tiers, ...) === undefined case — MEDIUM (or SIMPLE) is always +// populated. The edit modal has no equivalent of this check (it allows saving with only some +// tiers filled), which is why it needs its own explicit `!defaultModel` guard after deriving — +// see edit_auto_router_modal.tsx's save handler. A future contributor copying this form's submit +// handler elsewhere should not assume the same guarantee holds without this check. export const getMissingTiersError = (tiers: ComplexityTiers): string | null => { const missing = TIER_KEYS.filter((tier) => tiers[tier].length === 0); if (missing.length === 0) return null; 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 208a63d37df..b38d5492a45 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 @@ -577,6 +577,27 @@ describe("EditAutoRouterModal default model", () => { />, ); + // No config blob marker — only litellm_params.complexity_router_default_model, as an untouched + // router looked before this PR's marker existed, or one an external API call wrote directly to. + const renderWithLitellmParamsDefaultOnly = (complexityRouterDefaultModel: string) => + renderWithProviders( + , + ); + it("preserves a stored pin through an untouched open-and-save", async () => { const user = userEvent.setup(); renderWithStoredPin("out-of-band-default"); @@ -611,6 +632,71 @@ describe("EditAutoRouterModal default model", () => { expect(savedConfig()).toMatchObject({ default_model: STORED_CONFIG.tiers.MEDIUM[0] }); }); + // Greptile P1 on #36615: with no config blob marker, a litellm_params default that merely + // matches what the tiers derive is indistinguishable from the pre-PR auto-derive-and-write + // behavior (main always wrote a tier-derived value there on every save). Treating it as a pin + // would freeze every pre-existing router's default away from its tiers, so it stays unpinned. + it("treats a litellm_params default matching tier-derivation as unpinned, not a frozen-in pin", async () => { + const user = userEvent.setup(); + renderWithLitellmParamsDefaultOnly(STORED_CONFIG.tiers.MEDIUM[0]); + + const select = await screen.findByRole("combobox", { name: "Default model" }); + expect(select.closest(".ant-select")?.querySelector(".ant-select-selection-item")).toBeNull(); + + await user.click(screen.getByRole("button", { name: /save changes/i })); + await waitFor(() => expect(modelPatchUpdateCall).toHaveBeenCalled()); + expect(savedConfig()).not.toHaveProperty("default_model"); + expect(savedDefaultModel()).toBe(STORED_CONFIG.tiers.MEDIUM[0]); + }); + + // Greptile P1 on #36615: a litellm_params default that diverges from tier-derivation could only + // have gotten there via an explicit override — set by the API directly, since this UI's own + // save path keeps it in sync with tiers whenever there's no pin. That divergence must survive + // the next save instead of being silently recomputed away. + it("treats a diverging litellm_params default as an external pin and preserves it", async () => { + const user = userEvent.setup(); + renderWithLitellmParamsDefaultOnly("claude-sonnet-4"); + + const select = await screen.findByRole("combobox", { name: "Default model" }); + expect(within(select.closest(".ant-select") as HTMLElement).getByTitle("claude-sonnet-4")).toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: /save changes/i })); + await waitFor(() => expect(modelPatchUpdateCall).toHaveBeenCalled()); + expect(savedConfig()).toMatchObject({ default_model: "claude-sonnet-4" }); + expect(savedDefaultModel()).toBe("claude-sonnet-4"); + }); + + // The config blob marker is this UI's own authoritative record of intent (see + // hydratePinnedDefaultModel), so it wins even over a litellm_params value that disagrees — + // e.g. a stale value from before the operator most recently changed the pin. + it("prefers the config blob marker over a diverging litellm_params value", async () => { + const user = userEvent.setup(); + renderWithProviders( + , + ); + + const select = await screen.findByRole("combobox", { name: "Default model" }); + expect(within(select.closest(".ant-select") as HTMLElement).getByTitle("blob-pin")).toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: /save changes/i })); + await waitFor(() => expect(modelPatchUpdateCall).toHaveBeenCalled()); + expect(savedConfig()).toMatchObject({ default_model: "blob-pin" }); + }); + // This modal only requires one non-empty tier, so a COMPLEX-only router is reachable here even // though the backend raises on it. The block keeps that failure at save time instead of init. it("blocks a save when neither the tiers nor a pin give the backend a default", async () => { 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 7d98d758de7..ae01584e174 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 @@ -19,6 +19,7 @@ import { DEFAULT_MATCH_THRESHOLD } from "../add_model/SemanticKeywordMatching"; import { hydrateKeywordTierRules, serializeKeywordTierRules } from "../add_model/complexity_router_keywords"; import ComplexityRouterConfig, { ComplexityRouterConfigValue, + ComplexityTiers, DEFAULT_ADAPTIVE_WEIGHTS, DEFAULT_SESSION_AFFINITY, DEFAULT_DEPLOYMENT_AFFINITY, @@ -82,8 +83,23 @@ const toRecord = (value: unknown): Record => { : {}; }; -export const hydratePinnedDefaultModel = (stored: unknown): string | undefined => - typeof stored === "string" && stored.trim() ? stored : undefined; +// A pin lives in two places: complexity_router_config.default_model (this UI's own marker, added +// by PR #36615) and litellm_params.complexity_router_default_model (what the backend reads). Only +// the marker proves an operator picked it, because before #36615 every save wrote a tier-derived +// value into litellm_params. So with no marker, a litellm_params value counts as a pin only when +// it diverges from what the tiers alone derive; a match stays unpinned and keeps tracking tiers. +export const hydratePinnedDefaultModel = ( + storedConfigDefaultModel: unknown, + litellmParamsDefaultModel: string | null | undefined, + tiers: ComplexityTiers, +): string | undefined => { + if (typeof storedConfigDefaultModel === "string" && storedConfigDefaultModel.trim()) { + return storedConfigDefaultModel; + } + const tierDerived = resolveComplexityDefaultModel(tiers); + const externalOverride = litellmParamsDefaultModel?.trim(); + return externalOverride && externalOverride !== tierDerived ? externalOverride : undefined; +}; export interface KeywordMatchingState { keywordTierRules: KeywordTierRule[]; @@ -241,14 +257,20 @@ const EditAutoRouterModal: React.FC = ({ parsedConfig = JSON.parse(parsedConfig); } + const hydratedTiers: ComplexityTiers = { + SIMPLE: normalizeTierModels(parsedConfig.tiers?.SIMPLE), + MEDIUM: normalizeTierModels(parsedConfig.tiers?.MEDIUM), + COMPLEX: normalizeTierModels(parsedConfig.tiers?.COMPLEX), + REASONING: normalizeTierModels(parsedConfig.tiers?.REASONING), + }; + const hydratedComplexityRouterConfig: ComplexityRouterConfigValue = { - tiers: { - SIMPLE: normalizeTierModels(parsedConfig.tiers?.SIMPLE), - MEDIUM: normalizeTierModels(parsedConfig.tiers?.MEDIUM), - COMPLEX: normalizeTierModels(parsedConfig.tiers?.COMPLEX), - REASONING: normalizeTierModels(parsedConfig.tiers?.REASONING), - }, - default_model: hydratePinnedDefaultModel(parsedConfig.default_model), + tiers: hydratedTiers, + default_model: hydratePinnedDefaultModel( + parsedConfig.default_model, + modelData.litellm_params?.complexity_router_default_model, + hydratedTiers, + ), tier_labels: hydrateTierLabels(parsedConfig.tier_labels), classifier_type: parsedConfig.classifier_type || "heuristic", classifier_llm_config: parsedConfig.classifier_llm_config, @@ -369,7 +391,8 @@ const EditAutoRouterModal: React.FC = ({ } // Unlike the create form, this modal only requires one non-empty tier, so a router can reach - // here with nothing the backend would pick as a default. init_complexity_router_deployment + // here with nothing the backend would pick as a default (see getMissingTiersError in + // build_complexity_router_config.ts for why create never can). init_complexity_router_deployment // raises in that case (litellm/router.py), so block it rather than saving a router that // fails at init. const defaultModel = resolveComplexityDefaultModel(tiers, complexityRouterConfig.default_model); @@ -381,6 +404,9 @@ const EditAutoRouterModal: React.FC = ({ return; } + // Dual write: complexity_router_config.default_model (the pin marker hydratePinnedDefaultModel + // reads back) and complexity_router_default_model (what the backend routes on) must always be + // written together from the same value. Same pairing in add_auto_router_tab.tsx. const updatedLitellmParams = { ...modelData.litellm_params, complexity_router_config: buildUpdatedComplexityRouterConfig( diff --git a/ui/litellm-dashboard/src/components/model_info_view.test.tsx b/ui/litellm-dashboard/src/components/model_info_view.test.tsx index a3ce40494eb..0fff7d46e14 100644 --- a/ui/litellm-dashboard/src/components/model_info_view.test.tsx +++ b/ui/litellm-dashboard/src/components/model_info_view.test.tsx @@ -987,6 +987,44 @@ describe("ModelInfoView", () => { expect(mockTestModelGroupConnection).not.toHaveBeenCalled(); }); + // Bugbot finding on #36615: complexity_router_config.default_model is a UI-only bookkeeping + // marker — init_complexity_router_deployment (litellm/router.py) never reads it, falling back + // to tier-derivation instead when litellm_params.complexity_router_default_model is absent. + // Probing the blob field here would test a model the running router never calls. + it("ignores an unused config blob pin when litellm_params has no default, matching the backend's own tier-derivation fallback", async () => { + const complexityRouterModelData = { + ...defaultModelData, + litellm_params: { + ...defaultModelData.litellm_params, + model: "auto_router/complexity_router", + complexity_router_config: { + tiers: { SIMPLE: ["gpt-4o-mini"], MEDIUM: [], COMPLEX: [], REASONING: [] }, + default_model: "unused-blob-pin", + }, + // no complexity_router_default_model + }, + }; + + mockUseModelsInfo.mockReturnValue({ + data: { + data: [complexityRouterModelData], + }, + isLoading: false, + error: null, + }); + mockTestModelGroupConnection.mockResolvedValue({ status: "success" }); + + render(, { wrapper }); + const testConnectionButton = await screen.findByTestId("test-connection-button"); + await userEvent.click(testConnectionButton); + + await waitFor(() => { + expect(mockTestModelGroupConnection).toHaveBeenCalledWith("test-token", "gpt-4o-mini", "chat"); + }); + expect(mockTestModelGroupConnection).not.toHaveBeenCalledWith("test-token", "unused-blob-pin", "chat"); + expect(mockTestModelGroupConnection).toHaveBeenCalledTimes(1); + }); + it("should display model access groups field", async () => { render(, { wrapper }); await waitFor(() => { diff --git a/ui/litellm-dashboard/src/components/model_info_view.tsx b/ui/litellm-dashboard/src/components/model_info_view.tsx index 6f29443f1c5..d4025f184f7 100644 --- a/ui/litellm-dashboard/src/components/model_info_view.tsx +++ b/ui/litellm-dashboard/src/components/model_info_view.tsx @@ -178,14 +178,17 @@ const buildComplexityRouterTestTargets = ( REASONING: normalizeTierModels(config.tiers?.REASONING), }; - // litellm_params overrides the config blob at init (complexity_router.py), so it wins here too. - const pinned = modelData?.litellm_params?.complexity_router_default_model || config.default_model; + // Mirrors init_complexity_router_deployment (litellm/router.py): litellm_params wins, otherwise + // pure tier-derivation. complexity_router_config.default_model is a UI-only marker the backend + // never reads — folding it in here could point Test Connection at a model the router never + // calls (see PR #36615 discussion). + const effectiveDefaultModel = modelData?.litellm_params?.complexity_router_default_model || undefined; const testTargetParams = { tiers, semanticMatchingEnabled: Boolean(config.semantic_keyword_matching), embeddingModel: config.embedding_model, - defaultModel: resolveComplexityDefaultModel(tiers, pinned), + defaultModel: resolveComplexityDefaultModel(tiers, effectiveDefaultModel), }; return buildAutoRouterTestTargets(testTargetParams); };