mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
refactor(ui): simplify model auth editing; fix stale credential branch
Drop the onFieldsResolved/authFieldKeys round trip: the parent now resolves provider auth field keys itself via the new useProviderAuthFieldKeys hook (same metadata ProviderSpecificFields renders), removing the report-up effect and its stable-reference footgun. ProviderSpecificFields keeps only excludeKeys (real need: suppress duplicate visible inputs). Fix the stale Authentication branch: derive it from the live litellm_credential_name form value (Form.useWatch) instead of the server snapshot, so clearing/adding a credential mid-edit shows the right UI. Also skip inline auth updates entirely when a named credential is selected, so we never submit a credential name and raw inline auth together.
This commit is contained in:
parent
8af663f990
commit
2ff503f606
3 changed files with 96 additions and 55 deletions
|
|
@ -1,9 +1,10 @@
|
|||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { render, screen, waitFor } from "@testing-library/react";
|
||||
import { render, renderHook, screen, waitFor } from "@testing-library/react";
|
||||
import { Form } from "antd";
|
||||
import React from "react";
|
||||
import { beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { Providers } from "../provider_info_helpers";
|
||||
import ProviderSpecificFields from "./provider_specific_fields";
|
||||
import ProviderSpecificFields, { useProviderAuthFieldKeys } from "./provider_specific_fields";
|
||||
|
||||
vi.mock("../networking", async () => {
|
||||
const actual = await vi.importActual("../networking");
|
||||
|
|
@ -183,17 +184,12 @@ describe("ProviderSpecificFields", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("excludeKeys removes parent-owned fields and reports only the remaining keys", async () => {
|
||||
it("excludeKeys removes parent-owned fields from the rendered set", async () => {
|
||||
const queryClient = createQueryClient();
|
||||
const onFieldsResolved = vi.fn();
|
||||
render(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<Form>
|
||||
<ProviderSpecificFields
|
||||
selectedProvider={Providers.OpenAI}
|
||||
excludeKeys={["api_base", "organization"]}
|
||||
onFieldsResolved={onFieldsResolved}
|
||||
/>
|
||||
<ProviderSpecificFields selectedProvider={Providers.OpenAI} excludeKeys={["api_base", "organization"]} />
|
||||
</Form>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
|
|
@ -204,22 +200,31 @@ describe("ProviderSpecificFields", () => {
|
|||
});
|
||||
expect(screen.queryByPlaceholderText("https://api.openai.com/v1")).not.toBeInTheDocument();
|
||||
expect(screen.queryByPlaceholderText("[OPTIONAL] my-unique-org")).not.toBeInTheDocument();
|
||||
expect(onFieldsResolved).toHaveBeenCalledWith(["api_key"]);
|
||||
});
|
||||
|
||||
it("onFieldsResolved reports the full key set when nothing is excluded", async () => {
|
||||
it("useProviderAuthFieldKeys returns the full key set when nothing is excluded", async () => {
|
||||
const queryClient = createQueryClient();
|
||||
const onFieldsResolved = vi.fn();
|
||||
render(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<Form>
|
||||
<ProviderSpecificFields selectedProvider={Providers.OpenAI} onFieldsResolved={onFieldsResolved} />
|
||||
</Form>
|
||||
</QueryClientProvider>,
|
||||
const wrapper = ({ children }: { children: React.ReactNode }) => (
|
||||
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
|
||||
);
|
||||
const { result } = renderHook(() => useProviderAuthFieldKeys(Providers.OpenAI), { wrapper });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(onFieldsResolved).toHaveBeenCalledWith(["api_base", "organization", "api_key"]);
|
||||
expect(result.current).toEqual(["api_base", "organization", "api_key"]);
|
||||
});
|
||||
});
|
||||
|
||||
it("useProviderAuthFieldKeys excludes parent-owned keys", async () => {
|
||||
const queryClient = createQueryClient();
|
||||
const wrapper = ({ children }: { children: React.ReactNode }) => (
|
||||
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
|
||||
);
|
||||
const { result } = renderHook(() => useProviderAuthFieldKeys(Providers.OpenAI, ["api_base", "organization"]), {
|
||||
wrapper,
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current).toEqual(["api_key"]);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import { UploadOutlined } from "@ant-design/icons";
|
|||
import { Text, TextInput } from "@tremor/react";
|
||||
import { Button as Button2, Col, Form, Input, Row, Select, Typography, Upload, UploadProps } from "antd";
|
||||
import React from "react";
|
||||
import { CredentialItem, ProviderCredentialFieldMetadata } from "../networking";
|
||||
import { CredentialItem, ProviderCreateInfo, ProviderCredentialFieldMetadata } from "../networking";
|
||||
import { provider_map, Providers } from "../provider_info_helpers";
|
||||
const { Link } = Typography;
|
||||
|
||||
|
|
@ -14,9 +14,6 @@ interface ProviderSpecificFieldsProps {
|
|||
// inputs (e.g. the model edit form has a dedicated "API Base" field) so we
|
||||
// don't create a duplicate Form.Item bound to the same name.
|
||||
excludeKeys?: string[];
|
||||
// Called whenever the set of rendered field keys changes, so a parent can
|
||||
// know exactly which form values belong to this provider's auth fields.
|
||||
onFieldsResolved?: (keys: string[]) => void;
|
||||
}
|
||||
|
||||
interface ProviderCredentialField {
|
||||
|
|
@ -59,6 +56,38 @@ const mapFieldMetadataToUiField = (field: ProviderCredentialFieldMetadata): Prov
|
|||
};
|
||||
};
|
||||
|
||||
// Resolve the credential UI fields for a provider from fetched metadata.
|
||||
// Matches the component's resolution order (display-name enum or raw slug).
|
||||
const resolveProviderFields = (
|
||||
providerMetadata: ProviderCreateInfo[] | undefined,
|
||||
selectedProvider: Providers,
|
||||
): ProviderCredentialField[] => {
|
||||
if (!providerMetadata) return [];
|
||||
const selectedProviderEnum = Providers[selectedProvider as keyof typeof Providers] as Providers;
|
||||
const providerInfo = providerMetadata.find(
|
||||
(p) =>
|
||||
p.provider_display_name === selectedProviderEnum ||
|
||||
p.provider === selectedProvider ||
|
||||
p.litellm_provider === selectedProvider,
|
||||
);
|
||||
return providerInfo ? providerInfo.credential_fields.map(mapFieldMetadataToUiField) : [];
|
||||
};
|
||||
|
||||
// The auth-field keys a parent form should harvest for this provider, minus
|
||||
// any keys the parent already owns (excludeKeys). Lets the parent collect the
|
||||
// right form values on submit without the child reporting them back upward.
|
||||
export const useProviderAuthFieldKeys = (selectedProvider: Providers, excludeKeys?: string[]): string[] => {
|
||||
const { data: providerMetadata } = useProviderFields();
|
||||
const excludeDep = (excludeKeys ?? []).join("\0");
|
||||
return React.useMemo(() => {
|
||||
const excludeSet = new Set(excludeKeys ?? []);
|
||||
return resolveProviderFields(providerMetadata, selectedProvider)
|
||||
.map((f) => f.key)
|
||||
.filter((k) => !excludeSet.has(k));
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [providerMetadata, selectedProvider, excludeDep]);
|
||||
};
|
||||
|
||||
// In-memory cache of provider credential fields keyed by provider display name.
|
||||
// This lets us reuse the data across multiple mounts and also supports
|
||||
// non-React helpers like createCredentialFromModel.
|
||||
|
|
@ -103,7 +132,6 @@ const ProviderSpecificFields: React.FC<ProviderSpecificFieldsProps> = ({
|
|||
selectedProvider,
|
||||
uploadProps,
|
||||
excludeKeys,
|
||||
onFieldsResolved,
|
||||
}) => {
|
||||
const selectedProviderEnum = Providers[selectedProvider as keyof typeof Providers] as Providers;
|
||||
const form = Form.useFormInstance(); // Get form instance from context
|
||||
|
|
@ -179,20 +207,14 @@ const ProviderSpecificFields: React.FC<ProviderSpecificFieldsProps> = ({
|
|||
return mapped;
|
||||
}, [selectedProviderEnum, selectedProvider, providerMetadata]);
|
||||
|
||||
const excludeKeySet = React.useMemo(() => new Set(excludeKeys ?? []), [excludeKeys?.join(",")]);
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
const excludeKeySet = React.useMemo(() => new Set(excludeKeys ?? []), [(excludeKeys ?? []).join("\0")]);
|
||||
|
||||
const visibleFields = React.useMemo(
|
||||
() => (excludeKeySet.size > 0 ? allFields.filter((f) => !excludeKeySet.has(f.key)) : allFields),
|
||||
[allFields, excludeKeySet],
|
||||
);
|
||||
|
||||
// Report the rendered field keys upward (string-joined so the effect only
|
||||
// fires when the actual set changes, not on every parent render).
|
||||
const visibleKeyList = React.useMemo(() => visibleFields.map((f) => f.key).join(","), [visibleFields]);
|
||||
React.useEffect(() => {
|
||||
onFieldsResolved?.(visibleKeyList ? visibleKeyList.split(",") : []);
|
||||
}, [visibleKeyList, onFieldsResolved]);
|
||||
|
||||
const handleUpload = {
|
||||
name: "file",
|
||||
accept: ".json",
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ import {
|
|||
testConnectionRequest,
|
||||
} from "./networking";
|
||||
import { getProviderLogoAndName, Providers } from "./provider_info_helpers";
|
||||
import ProviderSpecificFields from "./add_model/provider_specific_fields";
|
||||
import ProviderSpecificFields, { useProviderAuthFieldKeys } from "./add_model/provider_specific_fields";
|
||||
import NumericalInput from "./shared/numerical_input";
|
||||
import { Tag } from "./tag_management/types";
|
||||
import { getDisplayModelName } from "./view_model/model_name_display";
|
||||
|
|
@ -55,6 +55,11 @@ interface ModelInfoViewProps {
|
|||
modelAccessGroups: string[] | null;
|
||||
}
|
||||
|
||||
// Auth fields the model edit form already renders as dedicated inputs — kept
|
||||
// out of the inline provider auth section so we don't bind two Form.Items to
|
||||
// the same name or submit a duplicate value.
|
||||
const AUTH_FIELD_EXCLUDE_KEYS = ["api_base", "organization", "custom_llm_provider"];
|
||||
|
||||
export default function ModelInfoView({
|
||||
modelId,
|
||||
onClose,
|
||||
|
|
@ -79,9 +84,6 @@ export default function ModelInfoView({
|
|||
const [guardrailsList, setGuardrailsList] = useState<string[]>([]);
|
||||
const [tagsList, setTagsList] = useState<Record<string, Tag>>({});
|
||||
const [credentialsList, setCredentialsList] = useState<CredentialItem[]>([]);
|
||||
// Provider auth-field keys reported by ProviderSpecificFields, so the save
|
||||
// handler knows which form values are this provider's credential fields.
|
||||
const [authFieldKeys, setAuthFieldKeys] = useState<string[]>([]);
|
||||
|
||||
// Fetch model data using hook
|
||||
const { data: rawModelDataResponse, isLoading: isLoadingModel } = useModelsInfo(1, 50, undefined, modelId);
|
||||
|
|
@ -118,6 +120,20 @@ export default function ModelInfoView({
|
|||
modelData?.litellm_params?.litellm_credential_name != null &&
|
||||
modelData?.litellm_params?.litellm_credential_name != undefined;
|
||||
|
||||
// Track the live credential dropdown so the Authentication section reacts to
|
||||
// edits in this session (not just the server snapshot). When a named
|
||||
// credential is selected the inline provider fields are hidden and never
|
||||
// submitted, so we can't save a credential name and raw inline auth together.
|
||||
const liveCredentialName = Form.useWatch("litellm_credential_name", form);
|
||||
const isUsingCredentialInForm = isEditing ? !!liveCredentialName : usingExistingCredential;
|
||||
|
||||
// Provider auth-field keys, resolved from the same metadata
|
||||
// ProviderSpecificFields renders, minus the fields the edit form already
|
||||
// owns. The save handler harvests exactly these from the form values.
|
||||
const authProvider = (localModelData?.litellm_params?.custom_llm_provider ||
|
||||
modelData?.provider) as Providers;
|
||||
const authFieldKeys = useProviderAuthFieldKeys(authProvider, AUTH_FIELD_EXCLUDE_KEYS);
|
||||
|
||||
// Initialize localModelData from modelData when available
|
||||
useEffect(() => {
|
||||
if (modelData && !localModelData) {
|
||||
|
|
@ -248,14 +264,18 @@ export default function ModelInfoView({
|
|||
// Provider auth fields the user actually entered. Blank/null/undefined
|
||||
// are omitted (never sent as "") so an untouched secret is preserved by
|
||||
// the backend's merge instead of being overwritten. Mirrors the submit
|
||||
// filter used by EditCredentialModal.
|
||||
const authFieldUpdates = authFieldKeys.reduce<Record<string, any>>((acc, key) => {
|
||||
const v = values[key];
|
||||
if (v !== "" && v !== undefined && v !== null) {
|
||||
acc[key] = v;
|
||||
}
|
||||
return acc;
|
||||
}, {});
|
||||
// filter used by EditCredentialModal. Skipped entirely when a named
|
||||
// credential is selected — that path owns auth, and sending both would
|
||||
// leave conflicting config on the backend.
|
||||
const authFieldUpdates = values.litellm_credential_name
|
||||
? {}
|
||||
: authFieldKeys.reduce<Record<string, any>>((acc, key) => {
|
||||
const v = values[key];
|
||||
if (v !== "" && v !== undefined && v !== null) {
|
||||
acc[key] = v;
|
||||
}
|
||||
return acc;
|
||||
}, {});
|
||||
|
||||
let updatedLitellmParams = {
|
||||
...values.litellm_params,
|
||||
|
|
@ -1084,13 +1104,11 @@ export default function ModelInfoView({
|
|||
{isEditing && (
|
||||
<div>
|
||||
<Text className="font-medium">Authentication</Text>
|
||||
{usingExistingCredential ? (
|
||||
{isUsingCredentialInForm ? (
|
||||
<div className="mt-1 p-2 bg-gray-50 rounded text-sm text-gray-600">
|
||||
This model uses the shared credential{" "}
|
||||
<span className="font-mono">
|
||||
{localModelData.litellm_params?.litellm_credential_name}
|
||||
</span>
|
||||
. Update its keys from the LLM Credentials tab.
|
||||
<span className="font-mono">{liveCredentialName}</span>. Update its keys from the LLM
|
||||
Credentials tab.
|
||||
</div>
|
||||
) : (
|
||||
<div className="mt-2">
|
||||
|
|
@ -1098,12 +1116,8 @@ export default function ModelInfoView({
|
|||
Leave a field blank to keep its current value. Enter a new value to rotate it.
|
||||
</Text>
|
||||
<ProviderSpecificFields
|
||||
selectedProvider={
|
||||
(localModelData.litellm_params?.custom_llm_provider ||
|
||||
modelData.provider) as Providers
|
||||
}
|
||||
excludeKeys={["api_base", "organization", "custom_llm_provider"]}
|
||||
onFieldsResolved={setAuthFieldKeys}
|
||||
selectedProvider={authProvider}
|
||||
excludeKeys={AUTH_FIELD_EXCLUDE_KEYS}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue