mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
fix(ui): check the built config's models, not which preset it came from
Deleting the preset-specific submit-time re-check removed the one place that verified availability at all, leaving a real gap: a caller whose access narrows after a model already entered a tier (via a preset or picked by hand) had nothing catching a now-unavailable model before creation, and the backend does not validate this either.
Generalized autorouter_presets.ts's getRequiredModelsInPreset/getMissingModelsInPreset into config-shaped getRequiredModels/getMissingModels (the preset-specific versions are now thin wrappers), so the same accessor works on a preset's bundled config or on complexityRouterConfig as actually built. submitBlockedReason now also checks the config's referenced models against availableModelSet, which is reactively kept current by the existing useQuery, no new fetch or async gap involved. This checks the config that will actually be submitted regardless of whether it arrived via a preset or Custom, closing the gap Bugbot's first finding pointed at directly ("verifies preset, not config").
This commit is contained in:
parent
45e54e0003
commit
13c99829c7
3 changed files with 59 additions and 9 deletions
|
|
@ -305,6 +305,27 @@ describe("AddAutoRouterTab", () => {
|
|||
expect(isOptionDisabled(optionByLabel("OpenAI Family")!)).toBe(true);
|
||||
});
|
||||
|
||||
// A preset only prefills once; nothing re-checks selectedPreset afterward. So the config itself,
|
||||
// not the preset it came from, has to stay checked against availableModelSet - here the caller's
|
||||
// access narrows after a preset already filled in a model, and submitBlockedReason must catch it
|
||||
// the same way it would catch an empty tier, not let a stale reference through.
|
||||
it("blocks submit when a model already in the config is no longer in the caller's available list", async () => {
|
||||
mockFetchAvailableModels
|
||||
.mockResolvedValueOnce(ALL_FAMILY_MODELS)
|
||||
.mockResolvedValueOnce(ALL_FAMILY_MODELS.filter((m) => m.model_group !== "claude-opus-5"));
|
||||
|
||||
const { rerender } = renderWithProviders(
|
||||
<AddAutoRouterTab handleOk={vi.fn()} accessToken="caller-a" userRole="Admin" />,
|
||||
);
|
||||
openTemplateDropdown();
|
||||
await waitFor(() => expect(isOptionDisabled(optionByLabel("Anthropic Family")!)).toBe(false));
|
||||
fireEvent.click(optionByLabel("Anthropic Family")!);
|
||||
|
||||
rerender(<AddAutoRouterTab handleOk={vi.fn()} accessToken="caller-b" userRole="Admin" />);
|
||||
|
||||
await waitFor(() => expect(screen.getByRole("button", { name: /add auto router/i })).toBeDisabled());
|
||||
});
|
||||
|
||||
// Prefill must preserve a preset's deliberately-falsy fields (a 0 match threshold, an empty
|
||||
// escalation list). Using `||` instead of `??` would swap the 0 for the create-form default and
|
||||
// re-enable escalation the preset meant to turn off, so this asserts the exact submitted values.
|
||||
|
|
|
|||
|
|
@ -27,7 +27,13 @@ import {
|
|||
import { buildAutoRouterTestTargets, AutoRouterTestTarget } from "./build_auto_router_test_targets";
|
||||
import AutoRouterConnectionTest from "./auto_router_connection_test";
|
||||
import NotificationManager from "../molecules/notifications_manager";
|
||||
import { getAllPresets, getPresetByKey, getMissingModelsInPreset, AutoRouterPreset } from "@/lib/autorouter_presets";
|
||||
import {
|
||||
getAllPresets,
|
||||
getPresetByKey,
|
||||
getMissingModelsInPreset,
|
||||
getMissingModels,
|
||||
AutoRouterPreset,
|
||||
} from "@/lib/autorouter_presets";
|
||||
|
||||
type PresetAvailability =
|
||||
| { kind: "available" }
|
||||
|
|
@ -193,15 +199,25 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
setEscalationKeywords(config.escalation_keywords ?? DEFAULT_ESCALATION_KEYWORDS);
|
||||
};
|
||||
|
||||
// Checks the config actually being built, not which preset (if any) it came from: a model that
|
||||
// was available when it entered a tier, whether via a preset or picked by hand, can have gone
|
||||
// missing since (the caller's access narrowed, or a background refetch never caught it).
|
||||
const missingReferencedModels = getMissingModels(
|
||||
{
|
||||
tiers: complexityRouterConfig.tiers,
|
||||
classifier_llm_config: complexityRouterConfig.classifier_llm_config,
|
||||
embedding_model: embeddingModel,
|
||||
},
|
||||
availableModelSet,
|
||||
);
|
||||
|
||||
// Why the submit is unavailable, or null when it is available. The button reads this to disable
|
||||
// itself and to say what is missing, so the two can never give different answers.
|
||||
const submitBlockedReason =
|
||||
getMissingTiersError(complexityRouterConfig.tiers) ?? getKeywordTierRulesError(keywordTierRules);
|
||||
getMissingTiersError(complexityRouterConfig.tiers) ??
|
||||
getKeywordTierRulesError(keywordTierRules) ??
|
||||
(missingReferencedModels.length > 0 ? `Model(s) no longer available: ${missingReferencedModels.join(", ")}` : null);
|
||||
|
||||
// A preset only ever prefills complexityRouterConfig at selection time (handlePresetChange);
|
||||
// after that it's edited exactly like Custom, so there is nothing preset-specific left to verify
|
||||
// at submit. The tier selects already only ever offer models from modelInfo, so submitBlockedReason
|
||||
// covering the actual config is the whole check, regardless of how it got there.
|
||||
const submitRecommendedRouter = async (name: string) => {
|
||||
const {
|
||||
tiers,
|
||||
|
|
|
|||
|
|
@ -22,11 +22,24 @@ export const getAllPresets = (): AutoRouterPreset[] => PRESETS;
|
|||
|
||||
export const getPresetByKey = (key: string): AutoRouterPreset | undefined => PRESETS.find((p) => p.key === key);
|
||||
|
||||
export const getRequiredModelsInPreset = (preset: AutoRouterPreset): Set<string> => {
|
||||
const { tiers, classifier_llm_config: classifier, embedding_model: embedding } = preset.complexity_router_config;
|
||||
// Generalized over ComplexityRouterConfigPayload (a preset's bundled config) so the same accessors
|
||||
// can check either a preset's own models or a caller's actually-built config - the two need to
|
||||
// agree, since a preset only prefills once and the config is edited freely after.
|
||||
export const getRequiredModels = (
|
||||
config: Pick<ComplexityRouterConfigPayload, "tiers" | "classifier_llm_config" | "embedding_model">,
|
||||
): Set<string> => {
|
||||
const { tiers, classifier_llm_config: classifier, embedding_model: embedding } = config;
|
||||
const models = [...tiers.SIMPLE, ...tiers.MEDIUM, ...tiers.COMPLEX, ...tiers.REASONING, classifier?.model, embedding];
|
||||
return new Set(models.filter((model): model is string => model != null));
|
||||
};
|
||||
|
||||
export const getMissingModels = (
|
||||
config: Pick<ComplexityRouterConfigPayload, "tiers" | "classifier_llm_config" | "embedding_model">,
|
||||
availableModels: Set<string>,
|
||||
): string[] => [...getRequiredModels(config)].filter((model) => !availableModels.has(model)).sort();
|
||||
|
||||
export const getRequiredModelsInPreset = (preset: AutoRouterPreset): Set<string> =>
|
||||
getRequiredModels(preset.complexity_router_config);
|
||||
|
||||
export const getMissingModelsInPreset = (preset: AutoRouterPreset, availableModels: Set<string>): string[] =>
|
||||
[...getRequiredModelsInPreset(preset)].filter((model) => !availableModels.has(model)).sort();
|
||||
getMissingModels(preset.complexity_router_config, availableModels);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue