mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
fix(ui): persist deletion of litellm params from the model edit JSON editor
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
ff02d5cfc0
commit
d01537c1bb
5 changed files with 108 additions and 18 deletions
|
|
@ -511,15 +511,19 @@ def update_db_model(db_model: Deployment, updated_patch: updateDeployment) -> Pr
|
|||
# passes through (which today re-sends the OLD pricing on every save) cannot
|
||||
# silently undo a litellm_params clear via .update().
|
||||
#
|
||||
# Restricted to SPECIAL_MODEL_INFO_PARAMS (input/output cost per token/character
|
||||
# and cache read/write costs) so this path cannot be used to null out privileged
|
||||
# model_info fields like team_id or access groups. SPECIAL_MODEL_INFO_PARAMS are
|
||||
# mirrored between litellm_params and model_info by Deployment.__init__, so the
|
||||
# clear propagates to both blobs.
|
||||
# Any litellm_params field sent as an explicit null is removed from the stored
|
||||
# params (PATCH semantics; the UI relies on this to persist deletions from the
|
||||
# LiteLLM Params JSON editor). "model" is exempt since a deployment without a
|
||||
# model is invalid. The model_info mirror is restricted to
|
||||
# SPECIAL_MODEL_INFO_PARAMS (pricing fields mirrored between the two blobs by
|
||||
# Deployment.__init__) so this path cannot null out privileged model_info
|
||||
# fields like team_id or access groups.
|
||||
if updated_patch.litellm_params:
|
||||
for field in updated_patch.litellm_params.model_fields_set:
|
||||
if field in SPECIAL_MODEL_INFO_PARAMS and getattr(updated_patch.litellm_params, field) is None:
|
||||
merged_litellm_params.pop(field, None)
|
||||
if field == "model" or getattr(updated_patch.litellm_params, field) is not None:
|
||||
continue
|
||||
merged_litellm_params.pop(field, None)
|
||||
if field in SPECIAL_MODEL_INFO_PARAMS:
|
||||
merged_model_info.pop(field, None)
|
||||
if updated_patch.model_info:
|
||||
for field in updated_patch.model_info.model_fields_set:
|
||||
|
|
|
|||
|
|
@ -3030,6 +3030,49 @@ class TestUpdateDBModelClearPricing:
|
|||
# team_id must survive
|
||||
assert info.get("team_id") == "team-keep-me"
|
||||
|
||||
def test_null_extra_param_removes_it_from_litellm_params(self):
|
||||
"""The UI's LiteLLM Params JSON editor sends explicit nulls for keys the
|
||||
user deleted. Those keys must be removed from the stored params instead
|
||||
of being resurrected by the merge (regression: deleted reasoning_effort
|
||||
reappeared after a page refresh)."""
|
||||
from litellm.proxy.management_endpoints.model_management_endpoints import (
|
||||
update_db_model,
|
||||
)
|
||||
from litellm.types.router import Deployment, LiteLLM_Params, ModelInfo, updateLiteLLMParams
|
||||
|
||||
db_model = Deployment(
|
||||
model_name="gpt-4o",
|
||||
litellm_params=LiteLLM_Params(model="openai/gpt-4o", reasoning_effort="high"),
|
||||
model_info=ModelInfo(id="dep-extra-0"),
|
||||
)
|
||||
|
||||
result = update_db_model(
|
||||
db_model=db_model,
|
||||
updated_patch=updateDeployment(
|
||||
litellm_params=updateLiteLLMParams(**{"reasoning_effort": None})
|
||||
),
|
||||
)
|
||||
|
||||
params = json.loads(result["litellm_params"])
|
||||
assert "reasoning_effort" not in params
|
||||
assert params["model"] == "openai/gpt-4o"
|
||||
|
||||
def test_null_model_is_never_cleared(self):
|
||||
"""A deployment without a model is invalid, so an explicit-null model
|
||||
must not remove the stored one."""
|
||||
from litellm.proxy.management_endpoints.model_management_endpoints import (
|
||||
update_db_model,
|
||||
)
|
||||
from litellm.types.router import updateLiteLLMParams
|
||||
|
||||
result = update_db_model(
|
||||
db_model=_build_db_model_with_pricing(),
|
||||
updated_patch=updateDeployment(litellm_params=updateLiteLLMParams(model=None)),
|
||||
)
|
||||
|
||||
params = json.loads(result["litellm_params"])
|
||||
assert params["model"] == "openai/*"
|
||||
|
||||
def test_clear_survives_model_info_passthrough_with_old_pricing(self):
|
||||
"""Realistic UI submit shape: the patch carries BOTH blobs. The
|
||||
model_info portion still has the old pricing because the form
|
||||
|
|
|
|||
|
|
@ -205,6 +205,15 @@ const perMillionTokens = (...rates: (number | null | undefined)[]): number | nul
|
|||
return rate == null ? null : rate * 1_000_000;
|
||||
};
|
||||
|
||||
export const editableExtraParams = (
|
||||
litellmParams: Record<string, unknown> | null | undefined,
|
||||
): Record<string, unknown> =>
|
||||
Object.fromEntries(
|
||||
Object.entries(litellmParams || {}).filter(
|
||||
([key, value]) => key !== "litellm_credential_name" && !isMaskedSecret(value),
|
||||
),
|
||||
);
|
||||
|
||||
export const toModelEditFormValues = (localModelData: any, isWildcardModel: boolean): ModelEditFormValues => ({
|
||||
model_name: localModelData.model_name,
|
||||
litellm_model_name: localModelData.litellm_model_name,
|
||||
|
|
@ -251,15 +260,7 @@ export const toModelEditFormValues = (localModelData: any, isWildcardModel: bool
|
|||
// antd never mounted this field for a non-wildcard model, so the key must be absent, not null.
|
||||
...(isWildcardModel ? { health_check_model: localModelData.model_info?.health_check_model } : {}),
|
||||
litellm_credential_name: localModelData.litellm_params?.litellm_credential_name || "",
|
||||
litellm_extra_params: JSON.stringify(
|
||||
Object.fromEntries(
|
||||
Object.entries(localModelData.litellm_params || {}).filter(
|
||||
([key, value]) => key !== "litellm_credential_name" && !isMaskedSecret(value),
|
||||
),
|
||||
),
|
||||
null,
|
||||
2,
|
||||
),
|
||||
litellm_extra_params: JSON.stringify(editableExtraParams(localModelData.litellm_params), null, 2),
|
||||
});
|
||||
|
||||
const displayCost = (localModelData: any, field: TouchedPricingField): string => {
|
||||
|
|
|
|||
|
|
@ -1571,6 +1571,34 @@ describe("ModelInfoView", () => {
|
|||
expect(payload.litellm_params.drop_params).toBe(true);
|
||||
});
|
||||
|
||||
it("sends an explicit null for a key deleted from the LiteLLM extra params", async () => {
|
||||
const withExtraParam = {
|
||||
...defaultModelData,
|
||||
litellm_params: { ...defaultModelData.litellm_params, reasoning_effort: "high" },
|
||||
};
|
||||
mockUseModelsInfo.mockReturnValue({ data: { data: [withExtraParam] }, isLoading: false, error: null });
|
||||
mockModelInfoV1Call.mockResolvedValue({ data: [withExtraParam] });
|
||||
const user = userEvent.setup();
|
||||
await enterEditMode(user);
|
||||
|
||||
const extraParams = screen
|
||||
.getAllByRole("textbox")
|
||||
.find(
|
||||
(input) =>
|
||||
input.tagName === "TEXTAREA" && (input as HTMLTextAreaElement).value.includes('"reasoning_effort"'),
|
||||
) as HTMLTextAreaElement;
|
||||
const withoutReasoningEffort = JSON.parse(extraParams.value);
|
||||
delete withoutReasoningEffort.reasoning_effort;
|
||||
await user.clear(extraParams);
|
||||
await user.paste(JSON.stringify(withoutReasoningEffort));
|
||||
|
||||
const payload = await save(user);
|
||||
|
||||
expect(payload.litellm_params.reasoning_effort).toBeNull();
|
||||
expect(payload.litellm_params.model).toBe("gpt-4");
|
||||
expect(payload.litellm_params.api_base).toBe("https://api.openai.com/v1");
|
||||
});
|
||||
|
||||
it("sends the credential picked in the selector", async () => {
|
||||
mockCredentialListCall.mockResolvedValue({
|
||||
credentials: [
|
||||
|
|
|
|||
|
|
@ -42,7 +42,11 @@ import {
|
|||
} from "./networking";
|
||||
import { Logo } from "@/components/molecules/logo/Logo";
|
||||
import UpdateModelCredentialsModal from "./update_model_credentials_modal";
|
||||
import ModelInfoEditForm, { type ModelEditFormValues, type TouchedPricingField } from "./ModelInfoEditForm";
|
||||
import ModelInfoEditForm, {
|
||||
editableExtraParams,
|
||||
type ModelEditFormValues,
|
||||
type TouchedPricingField,
|
||||
} from "./ModelInfoEditForm";
|
||||
import { Tag } from "./tag_management/types";
|
||||
import { getDisplayModelName } from "./view_model/model_name_display";
|
||||
import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle } from "@/components/ui/dialog";
|
||||
|
|
@ -426,6 +430,15 @@ export default function ModelInfoView({
|
|||
return;
|
||||
}
|
||||
|
||||
// Keys the user deleted from the LiteLLM Params JSON editor must be sent as
|
||||
// explicit nulls: the backend PATCH merges params, so an absent key would
|
||||
// silently keep its old value.
|
||||
for (const key of Object.keys(editableExtraParams(localModelData?.litellm_params))) {
|
||||
if (!(key in parsedExtraParams) && updatedLitellmParams[key] === undefined) {
|
||||
updatedLitellmParams[key] = null;
|
||||
}
|
||||
}
|
||||
|
||||
// Final guard: never PATCH a redacted secret. The /model/info snapshot that
|
||||
// seeds this form masks secrets, and any save re-sends the whole params blob;
|
||||
// without this strip a masked value would be re-encrypted over the real secret.
|
||||
|
|
@ -440,11 +453,12 @@ export default function ModelInfoView({
|
|||
|
||||
await modelPatchUpdateCall(accessToken, updateData, modelId);
|
||||
|
||||
// The backend removes explicitly-null keys, so mirror that removal locally.
|
||||
const updatedModelData = {
|
||||
...localModelData,
|
||||
model_name: values.model_name,
|
||||
litellm_model_name: values.litellm_model_name,
|
||||
litellm_params: safeLitellmParams,
|
||||
litellm_params: Object.fromEntries(Object.entries(safeLitellmParams).filter(([, value]) => value !== null)),
|
||||
model_info: updatedModelInfo,
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue