mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
fix(ui): let the model picker remove selections that are no longer available (#43944)
* fix(ui): let ModelSelect deselect selections that are no longer offered A selected model that is no longer served (removed from config, deleted, or outside the org ceiling) never appeared in the dropdown, so once it sat past the fifth chip nothing could remove it. List such selections in an Unavailable group ahead of the offered options so they can be found and unchecked. * fix(ui): skip the Unavailable group when no live models were loaded A failed or empty model list made every selection look unavailable. Only flag selections as unavailable when there is a live model list to compare against, and cover ordering with several unavailable selections. * fix(ui): only suppress the Unavailable group when the model list failed to load Keying the guard on the context-filtered list hid the group when an organization ceiling excluded every selection, which brought the dead end back for team forms. Key it on the proxy model list having loaded. * fix(ui): keep other selections when removing one beside a special option Removing an unavailable model while a special option stayed selected collapsed the whole selection to the special option, dropping the other saved values. Collapse only when a special option is newly picked. Also skip the Unavailable group while an org team's model ceiling is unknown, since the offered list is empty for that reason alone.
This commit is contained in:
parent
0d17f954c0
commit
a7ff709f3d
2 changed files with 210 additions and 14 deletions
|
|
@ -1,6 +1,6 @@
|
|||
import type { ProxyModel } from "@/app/(dashboard)/hooks/models/useModels";
|
||||
import type { Organization } from "@/components/networking";
|
||||
import { screen } from "@testing-library/react";
|
||||
import { screen, within } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { renderWithProviders } from "../../../tests/test-utils";
|
||||
|
|
@ -660,4 +660,181 @@ describe("ModelSelect", () => {
|
|||
expect(screen.getByLabelText("model-4")).toBeInTheDocument();
|
||||
expect(screen.queryByLabelText("model-5")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should let a selected model that is no longer offered be found and deselected", async () => {
|
||||
const user = userEvent.setup();
|
||||
const liveModels: ProxyModel[] = Array.from({ length: 6 }, (_, i) => ({
|
||||
id: `model-${i}`,
|
||||
object: "model",
|
||||
created: 1234567890,
|
||||
owned_by: "test",
|
||||
}));
|
||||
mockUseAllProxyModels.mockReturnValue({
|
||||
data: { data: liveModels },
|
||||
isLoading: false,
|
||||
} as unknown as ReturnType<typeof useAllProxyModels>);
|
||||
const liveIds = liveModels.map((m) => m.id);
|
||||
|
||||
renderWithProviders(<ModelSelect onChange={mockOnChange} value={[...liveIds, "retired-model"]} context="global" />);
|
||||
|
||||
await openModelList(user);
|
||||
await user.type(screen.getAllByRole("combobox")[0], "retired");
|
||||
const retired = await screen.findByRole("option", { name: "retired-model" });
|
||||
expect(retired).toHaveAttribute("aria-selected", "true");
|
||||
|
||||
await user.click(retired);
|
||||
|
||||
expect(mockOnChange).toHaveBeenCalledWith(liveIds);
|
||||
});
|
||||
|
||||
it("should list selections that are no longer offered in an Unavailable group ahead of every other group", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(
|
||||
<ModelSelect
|
||||
onChange={mockOnChange}
|
||||
value={["retired-b", "gpt-4", "retired-a", "retired-c", "retired-d", "retired-e", "retired-f"]}
|
||||
context="user"
|
||||
options={{ showAllProxyModelsOverride: true, includeSpecialOptions: true }}
|
||||
/>,
|
||||
);
|
||||
|
||||
await openModelList(user);
|
||||
|
||||
const groups = within(screen.getByRole("listbox")).getAllByRole("group");
|
||||
expect(
|
||||
groups.map((group) =>
|
||||
within(group)
|
||||
.getAllByRole("option")
|
||||
.map((option) => option.textContent),
|
||||
),
|
||||
).toEqual([
|
||||
["retired-b", "retired-a", "retired-c", "retired-d", "retired-e", "retired-f"],
|
||||
["All Proxy Models", "No Default Models"],
|
||||
["All Openai models", "All Anthropic models"],
|
||||
["gpt-4", "claude-3"],
|
||||
]);
|
||||
expect(screen.getByText("Unavailable")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should keep an unavailable selection removable while a special option is selected", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(
|
||||
<ModelSelect
|
||||
onChange={mockOnChange}
|
||||
value={["all-proxy-models", "retired-model"]}
|
||||
context="user"
|
||||
options={{ showAllProxyModelsOverride: true, includeSpecialOptions: true }}
|
||||
/>,
|
||||
);
|
||||
|
||||
await openModelList(user);
|
||||
const retired = screen.getByRole("option", { name: "retired-model" });
|
||||
expect(retired).not.toHaveAttribute("aria-disabled", "true");
|
||||
|
||||
await user.click(retired);
|
||||
|
||||
expect(mockOnChange).toHaveBeenCalledWith(["all-proxy-models"]);
|
||||
});
|
||||
|
||||
it("should not mark selections Unavailable when the model list could not be loaded", async () => {
|
||||
const user = userEvent.setup();
|
||||
mockUseAllProxyModels.mockReturnValue({
|
||||
data: undefined,
|
||||
isLoading: false,
|
||||
} as unknown as ReturnType<typeof useAllProxyModels>);
|
||||
|
||||
renderWithProviders(
|
||||
<ModelSelect
|
||||
onChange={mockOnChange}
|
||||
value={["gpt-4", "claude-3"]}
|
||||
context="global"
|
||||
options={{ includeSpecialOptions: true }}
|
||||
/>,
|
||||
);
|
||||
|
||||
await openModelList(user);
|
||||
|
||||
expectOffered("All Proxy Models");
|
||||
expectNotOffered("Unavailable");
|
||||
});
|
||||
|
||||
it("should list selections outside the organization's model ceiling as Unavailable", async () => {
|
||||
const user = userEvent.setup();
|
||||
mockUseOrganization.mockReturnValue({
|
||||
data: createMockOrganization(["other-model"]),
|
||||
isLoading: false,
|
||||
} as unknown as ReturnType<typeof useOrganization>);
|
||||
|
||||
renderWithProviders(
|
||||
<ModelSelect onChange={mockOnChange} value={["gpt-4", "claude-3"]} context="team" organizationID="org-1" />,
|
||||
);
|
||||
|
||||
await openModelList(user);
|
||||
await user.click(screen.getByRole("option", { name: "claude-3" }));
|
||||
|
||||
expect(mockOnChange).toHaveBeenCalledWith(["gpt-4"]);
|
||||
});
|
||||
|
||||
it("should keep the other selections when removing one unavailable model alongside a special option", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(
|
||||
<ModelSelect
|
||||
onChange={mockOnChange}
|
||||
value={["all-proxy-models", "retired-a", "retired-b"]}
|
||||
context="user"
|
||||
options={{ showAllProxyModelsOverride: true, includeSpecialOptions: true }}
|
||||
/>,
|
||||
);
|
||||
|
||||
await openModelList(user);
|
||||
await user.click(screen.getByRole("option", { name: "retired-a" }));
|
||||
|
||||
expect(mockOnChange).toHaveBeenCalledWith(["all-proxy-models", "retired-b"]);
|
||||
});
|
||||
|
||||
it("should not mark team selections Unavailable while the organization's model ceiling is unknown", async () => {
|
||||
const user = userEvent.setup();
|
||||
mockUseOrganization.mockReturnValue({
|
||||
data: undefined,
|
||||
isLoading: false,
|
||||
} as unknown as ReturnType<typeof useOrganization>);
|
||||
mockUseTeam.mockReturnValue({
|
||||
data: { team_id: "team-1", organization_models: null },
|
||||
isLoading: false,
|
||||
isFetching: false,
|
||||
} as unknown as ReturnType<typeof useTeam>);
|
||||
|
||||
renderWithProviders(
|
||||
<ModelSelect
|
||||
onChange={mockOnChange}
|
||||
value={["gpt-4", "claude-3"]}
|
||||
context="team"
|
||||
teamID="team-1"
|
||||
organizationID="org-1"
|
||||
options={{ includeSpecialOptions: true }}
|
||||
/>,
|
||||
);
|
||||
|
||||
await openModelList(user);
|
||||
|
||||
expectOffered("No Default Models");
|
||||
expectNotOffered("Unavailable");
|
||||
});
|
||||
|
||||
it("should not show an Unavailable group when every selection is offered", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(
|
||||
<ModelSelect
|
||||
onChange={mockOnChange}
|
||||
value={["gpt-4", "openai/*"]}
|
||||
context="user"
|
||||
options={{ showAllProxyModelsOverride: true }}
|
||||
/>,
|
||||
);
|
||||
|
||||
await openModelList(user);
|
||||
|
||||
expect(screen.getByRole("option", { name: "gpt-4" })).toHaveAttribute("aria-selected", "true");
|
||||
expectNotOffered("Unavailable");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -126,6 +126,24 @@ const filterModels = (
|
|||
return filterFn(filterArgs);
|
||||
};
|
||||
|
||||
const isOfferedListKnown = (
|
||||
proxyModelsLoaded: boolean,
|
||||
context: ModelSelectProps["context"],
|
||||
organizationID: string | undefined,
|
||||
organizationModels: string[] | undefined,
|
||||
) => proxyModelsLoaded && !(context === "team" && organizationID !== undefined && organizationModels === undefined);
|
||||
|
||||
const unavailableGroups = (
|
||||
selectedOptions: ModelOption[],
|
||||
offeredByValue: Map<string, ModelOption>,
|
||||
offeredListKnown: boolean,
|
||||
): ModelOptionGroup[] => {
|
||||
if (!offeredListKnown) return [];
|
||||
const items = selectedOptions.filter((option) => !offeredByValue.has(option.value));
|
||||
if (items.length === 0) return [];
|
||||
return [{ label: "Unavailable", items }];
|
||||
};
|
||||
|
||||
export const ModelSelect = (props: ModelSelectProps) => {
|
||||
const anchor = useComboboxAnchor();
|
||||
const { id, teamID, organizationID, options, context, dataTestId, value = [], onChange, style } = props;
|
||||
|
|
@ -151,17 +169,10 @@ export const ModelSelect = (props: ModelSelectProps) => {
|
|||
|
||||
const handleChange = (selected: ModelOption[]) => {
|
||||
const values = selected.map((option) => option.value);
|
||||
const specialValues = values.filter(isSpecialOption);
|
||||
const addedSpecialValues = values.filter((v) => isSpecialOption(v) && !value.includes(v));
|
||||
const addedSpecial = addedSpecialValues[addedSpecialValues.length - 1];
|
||||
|
||||
let finalValues: string[];
|
||||
if (specialValues.length > 0) {
|
||||
const lastSelectedSpecial = specialValues[specialValues.length - 1];
|
||||
finalValues = [lastSelectedSpecial];
|
||||
} else {
|
||||
finalValues = values;
|
||||
}
|
||||
|
||||
onChange(finalValues);
|
||||
onChange(addedSpecial === undefined ? values : [addedSpecial]);
|
||||
};
|
||||
|
||||
const filteredModels = filterModels(allProxyModels?.data ?? [], props, {
|
||||
|
|
@ -171,7 +182,7 @@ export const ModelSelect = (props: ModelSelectProps) => {
|
|||
|
||||
const { wildcard, regular } = splitWildcardModels(filteredModels);
|
||||
|
||||
const groups: ModelOptionGroup[] = [
|
||||
const offeredGroups: ModelOptionGroup[] = [
|
||||
...(includeSpecialOptions
|
||||
? [
|
||||
{
|
||||
|
|
@ -228,8 +239,16 @@ export const ModelSelect = (props: ModelSelectProps) => {
|
|||
},
|
||||
];
|
||||
|
||||
const optionsByValue = new Map(groups.flatMap((group) => group.items).map((option) => [option.value, option]));
|
||||
const selectedOptions = value.map((v) => optionsByValue.get(v) ?? { label: v, value: v });
|
||||
const offeredByValue = new Map(offeredGroups.flatMap((group) => group.items).map((option) => [option.value, option]));
|
||||
const selectedOptions = value.map((v) => offeredByValue.get(v) ?? { label: v, value: v });
|
||||
const groups: ModelOptionGroup[] = [
|
||||
...unavailableGroups(
|
||||
selectedOptions,
|
||||
offeredByValue,
|
||||
isOfferedListKnown(allProxyModels !== undefined, context, organizationID, organizationModels),
|
||||
),
|
||||
...offeredGroups,
|
||||
];
|
||||
const overflowOptions = selectedOptions.slice(MAX_VISIBLE_MODEL_CHIPS);
|
||||
|
||||
return (
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue