mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-04 02:31:27 +00:00
feat(ui): verify preset availability freshly at submit, with visible loading state
The backend never validates an auto-router's referenced model names against the caller's access at creation time (POST /model/new only checks whether the caller can create a model at all, not whether they can use the specific models a complexity_router's tiers name). The frontend's cached availability check is the only thing that catches this, and it can go stale: nothing invalidates an already-selected preset, and a failed background refetch deliberately keeps trusting the cache (by design, so a passive hiccup doesn't wrongly block a still-valid preset). That combination meant a caller whose access narrowed at exactly the wrong moment could still create a router referencing models they no longer have. Extracted the fix into verifyPresetStillAvailable: a small, named async check that forces a fresh fetch right before creating the router, independent of whatever's cached, and only for the preset path (Custom never claimed this guarantee). Doing this invisibly inside the click handler would leave the button looking unresponsive for a real network round trip, so it's surfaced the same way this file already surfaces async submit work: a loading state on the button itself, matching the existing Test Connection convention. Extracting the check into its own function also pulled submitRecommendedRouter back under the complexity budget threshold it had just crossed.
This commit is contained in:
parent
af5af4d0d7
commit
b61da43e02
2 changed files with 135 additions and 56 deletions
|
|
@ -202,11 +202,16 @@ describe("AddAutoRouterTab", () => {
|
|||
});
|
||||
|
||||
// react-query keeps the last successful list when a later refetch fails, so a background
|
||||
// refetch error must not treat an already-verified, still-cached preset as unverifiable: the
|
||||
// caller never sees a stale error message, and a preset they already picked stays submittable.
|
||||
// refetch error must not treat an already-verified, still-cached preset as unverifiable in the
|
||||
// UI: the dropdown stays selectable and the caller never sees a stale error just from a passive
|
||||
// hiccup. Submit still forces its own fresh check (separately tested below); here that fresh
|
||||
// check succeeds, representing the hiccup having been transient.
|
||||
it("keeps a selected preset submit-reachable when a background refetch fails but cached models remain valid", async () => {
|
||||
const user = userEvent.setup();
|
||||
mockFetchAvailableModels.mockResolvedValueOnce(ALL_FAMILY_MODELS).mockRejectedValue(new Error("boom"));
|
||||
mockFetchAvailableModels
|
||||
.mockResolvedValueOnce(ALL_FAMILY_MODELS)
|
||||
.mockRejectedValueOnce(new Error("boom"))
|
||||
.mockResolvedValueOnce(ALL_FAMILY_MODELS);
|
||||
|
||||
renderWithProviders(<Harness />);
|
||||
openTemplateDropdown();
|
||||
|
|
@ -216,6 +221,10 @@ describe("AddAutoRouterTab", () => {
|
|||
await act(async () => {
|
||||
await testQueryClient.refetchQueries({ queryKey: ["availableModels", "autoRouter", "token"] });
|
||||
});
|
||||
// The passive refetch failure must not have re-disabled the already-selected preset's option.
|
||||
openTemplateDropdown();
|
||||
expect(isOptionDisabled(optionByLabel("Anthropic Family")!)).toBe(false);
|
||||
openTemplateDropdown();
|
||||
|
||||
await user.type(screen.getByPlaceholderText(/smart_router/i), "resilient-router");
|
||||
await user.click(screen.getByRole("button", { name: /add auto router/i }));
|
||||
|
|
@ -226,6 +235,60 @@ describe("AddAutoRouterTab", () => {
|
|||
);
|
||||
});
|
||||
|
||||
// The backend does not re-check a router's referenced model names against the caller's access,
|
||||
// so submit is the only place that can catch a genuinely stale preset: force a fresh fetch right
|
||||
// before creating the router rather than trusting the cache, and block if that fresh check can't
|
||||
// confirm availability (a real outage, or the caller's access having actually narrowed).
|
||||
it("blocks submit when a fresh re-check at submit time cannot confirm the preset's models", async () => {
|
||||
const user = userEvent.setup();
|
||||
mockFetchAvailableModels.mockResolvedValueOnce(ALL_FAMILY_MODELS).mockRejectedValue(new Error("boom"));
|
||||
|
||||
renderWithProviders(<Harness />);
|
||||
openTemplateDropdown();
|
||||
await waitFor(() => expect(isOptionDisabled(optionByLabel("Anthropic Family")!)).toBe(false));
|
||||
fireEvent.click(optionByLabel("Anthropic Family")!);
|
||||
|
||||
await user.type(screen.getByPlaceholderText(/smart_router/i), "unconfirmed-router");
|
||||
await user.click(screen.getByRole("button", { name: /add auto router/i }));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(NotificationManager.fromBackend).toHaveBeenCalledWith(
|
||||
"This template's models are no longer available. Please reselect a template or switch to Custom.",
|
||||
),
|
||||
);
|
||||
expect(mockHandleAddAutoRouterSubmit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// The fresh re-check is a real network round trip, not instant, so the submit button must show
|
||||
// its own loading state (matching the Test Connection button's existing convention) rather than
|
||||
// silently doing nothing until it settles. This also proves the button can't be clicked again
|
||||
// mid-check.
|
||||
it("shows a loading state on the submit button while the submit-time re-check is in flight", async () => {
|
||||
const user = userEvent.setup();
|
||||
let resolveRecheck: (models: ModelGroup[]) => void = () => undefined;
|
||||
mockFetchAvailableModels
|
||||
.mockResolvedValueOnce(ALL_FAMILY_MODELS)
|
||||
.mockReturnValueOnce(new Promise<ModelGroup[]>((resolve) => (resolveRecheck = resolve)));
|
||||
|
||||
renderWithProviders(<Harness />);
|
||||
openTemplateDropdown();
|
||||
await waitFor(() => expect(isOptionDisabled(optionByLabel("Anthropic Family")!)).toBe(false));
|
||||
fireEvent.click(optionByLabel("Anthropic Family")!);
|
||||
|
||||
await user.type(screen.getByPlaceholderText(/smart_router/i), "loading-state-router");
|
||||
const submitButton = screen.getByRole("button", { name: /add auto router/i });
|
||||
await user.click(submitButton);
|
||||
|
||||
await waitFor(() => expect(submitButton).toHaveClass("ant-btn-loading"));
|
||||
await user.click(submitButton);
|
||||
expect(mockFetchAvailableModels).toHaveBeenCalledTimes(2);
|
||||
|
||||
resolveRecheck(ALL_FAMILY_MODELS);
|
||||
|
||||
await waitFor(() => expect(submitButton).not.toHaveClass("ant-btn-loading"));
|
||||
expect(mockHandleAddAutoRouterSubmit).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// The headline behavior: selecting a preset must pre-fill the tier config so the created
|
||||
// router carries the preset's models. Real tier validation runs here (getMissingTiersError is
|
||||
// not stubbed), so if selection stopped pre-filling, the empty tiers would either block the
|
||||
|
|
|
|||
|
|
@ -92,6 +92,7 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
|
||||
const [isTestModalVisible, setIsTestModalVisible] = useState<boolean>(false);
|
||||
const [isTestingConnection, setIsTestingConnection] = useState<boolean>(false);
|
||||
const [isSubmittingRouter, setIsSubmittingRouter] = useState<boolean>(false);
|
||||
const [connectionTestId, setConnectionTestId] = useState<number>(0);
|
||||
const [testTargets, setTestTargets] = useState<AutoRouterTestTarget[]>([]);
|
||||
|
||||
|
|
@ -191,25 +192,34 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
setEscalationKeywords(config.escalation_keywords ?? DEFAULT_ESCALATION_KEYWORDS);
|
||||
};
|
||||
|
||||
const submitRecommendedRouter = (name: string) => {
|
||||
// The dropdown and handlePresetChange trust the cached model list, so a caller's access can
|
||||
// narrow without ever being reflected here (nothing invalidates a preset already applied, and a
|
||||
// background refetch failure keeps trusting the stale cache by design - see modelsUnverifiable
|
||||
// above). The backend does not re-check a router's referenced model names against the caller's
|
||||
// access either, so this is the only place that can catch it: force a fresh fetch right before
|
||||
// creating the router, rather than trusting whatever's cached.
|
||||
const verifyPresetStillAvailable = async (presetKey: string): Promise<boolean> => {
|
||||
const preset = getPresetByKey(presetKey);
|
||||
if (!preset) return false;
|
||||
const { data: freshModels, isError: freshError } = await refetchModels();
|
||||
if (freshError) return false;
|
||||
const freshSet = new Set((freshModels ?? []).map((m) => m.model_group));
|
||||
return getMissingModelsInPreset(preset, freshSet).length === 0;
|
||||
};
|
||||
|
||||
const submitRecommendedRouter = async (name: string) => {
|
||||
if (!selectedPreset) {
|
||||
setShowValidationErrors(true);
|
||||
NotificationManager.fromBackend("Please select a template, or choose Custom Configuration");
|
||||
return;
|
||||
}
|
||||
|
||||
// handlePresetChange only ever applies a preset that was available at selection time; that
|
||||
// guarantee can go stale by submit time (e.g. the caller's model access narrowed since), so
|
||||
// re-verify here rather than trust state gathered earlier.
|
||||
if (selectedPreset !== "custom") {
|
||||
const preset = getPresetByKey(selectedPreset);
|
||||
if (!preset || presetAvailability(preset).kind !== "available") {
|
||||
setShowValidationErrors(true);
|
||||
NotificationManager.fromBackend(
|
||||
"This template's models are no longer available. Please reselect a template or switch to Custom.",
|
||||
);
|
||||
return;
|
||||
}
|
||||
if (selectedPreset !== "custom" && !(await verifyPresetStillAvailable(selectedPreset))) {
|
||||
setShowValidationErrors(true);
|
||||
NotificationManager.fromBackend(
|
||||
"This template's models are no longer available. Please reselect a template or switch to Custom.",
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const {
|
||||
|
|
@ -256,48 +266,48 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
auto_router_default_model: defaultModel,
|
||||
});
|
||||
|
||||
form
|
||||
.validateFields(requiresTeamScope ? ["auto_router_name", "team_id"] : ["auto_router_name"])
|
||||
.then((values) => {
|
||||
const complexityRouterConfigParams = {
|
||||
tiers,
|
||||
classifierType,
|
||||
classifierLlmConfig,
|
||||
classifierContextWindowSize,
|
||||
classifierContextPerTurnChars,
|
||||
classifierContextIncludeAssistantTurns,
|
||||
sessionAffinity,
|
||||
customTechnicalKeywords,
|
||||
keywordTierRules,
|
||||
semanticMatchingEnabled,
|
||||
embeddingModel,
|
||||
matchThreshold,
|
||||
escalationKeywords,
|
||||
adaptive,
|
||||
adaptiveWeights,
|
||||
tierDistancePenalty,
|
||||
adaptiveEligible,
|
||||
returnRawModelName,
|
||||
};
|
||||
try {
|
||||
const values = await form.validateFields(
|
||||
requiresTeamScope ? ["auto_router_name", "team_id"] : ["auto_router_name"],
|
||||
);
|
||||
const complexityRouterConfigParams = {
|
||||
tiers,
|
||||
classifierType,
|
||||
classifierLlmConfig,
|
||||
classifierContextWindowSize,
|
||||
classifierContextPerTurnChars,
|
||||
classifierContextIncludeAssistantTurns,
|
||||
sessionAffinity,
|
||||
customTechnicalKeywords,
|
||||
keywordTierRules,
|
||||
semanticMatchingEnabled,
|
||||
embeddingModel,
|
||||
matchThreshold,
|
||||
escalationKeywords,
|
||||
adaptive,
|
||||
adaptiveWeights,
|
||||
tierDistancePenalty,
|
||||
adaptiveEligible,
|
||||
returnRawModelName,
|
||||
};
|
||||
|
||||
const submitValues = {
|
||||
...values,
|
||||
auto_router_name: name,
|
||||
auto_router_default_model: defaultModel,
|
||||
model_type: "complexity_router",
|
||||
complexity_router_config: buildComplexityRouterConfig(complexityRouterConfigParams),
|
||||
model_access_group: form.getFieldValue("model_access_group"),
|
||||
};
|
||||
const submitValues = {
|
||||
...values,
|
||||
auto_router_name: name,
|
||||
auto_router_default_model: defaultModel,
|
||||
model_type: "complexity_router",
|
||||
complexity_router_config: buildComplexityRouterConfig(complexityRouterConfigParams),
|
||||
model_access_group: form.getFieldValue("model_access_group"),
|
||||
};
|
||||
|
||||
handleAddAutoRouterSubmit(submitValues, accessToken, form, handleOk);
|
||||
})
|
||||
.catch((error) => {
|
||||
console.error("Validation failed:", error);
|
||||
NotificationManager.fromBackend("Please fill in all required fields");
|
||||
});
|
||||
await handleAddAutoRouterSubmit(submitValues, accessToken, form, handleOk);
|
||||
} catch (error) {
|
||||
console.error("Validation failed:", error);
|
||||
NotificationManager.fromBackend("Please fill in all required fields");
|
||||
}
|
||||
};
|
||||
|
||||
const handleAutoRouterSubmit = () => {
|
||||
const handleAutoRouterSubmit = async () => {
|
||||
const name = form.getFieldValue("auto_router_name");
|
||||
if (!name) {
|
||||
setShowValidationErrors(true);
|
||||
|
|
@ -306,7 +316,12 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
return;
|
||||
}
|
||||
|
||||
submitRecommendedRouter(name);
|
||||
setIsSubmittingRouter(true);
|
||||
try {
|
||||
await submitRecommendedRouter(name);
|
||||
} finally {
|
||||
setIsSubmittingRouter(false);
|
||||
}
|
||||
};
|
||||
|
||||
const handleTestConnection = () => {
|
||||
|
|
@ -483,8 +498,9 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
<Button
|
||||
type="primary"
|
||||
onClick={() => {
|
||||
handleAutoRouterSubmit();
|
||||
void handleAutoRouterSubmit();
|
||||
}}
|
||||
loading={isSubmittingRouter}
|
||||
>
|
||||
Add Auto Router
|
||||
</Button>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue