Merge 9a7a47f893 into 02f61c9c42

litellm/proxy/proxy_server.py

@@ -311,7 +311,6 @@ from litellm.constants import (
 from litellm.exceptions import RejectedRequestError
 from litellm.integrations.custom_guardrail import CustomGuardrail, ModifyResponseException
 from litellm.integrations.custom_logger import CustomLogger
-from litellm.integrations.SlackAlerting.slack_alerting import SlackAlerting
 from litellm.litellm_core_utils.agentic_loop_settings import (
     validated_max_agentic_loops,
 )
@@ -16729,6 +16728,58 @@ def _nested_setting_source(
     return "db" if field_name in db_values else unset_source
 
 
+def _alerting_field_value(
+    source: FieldSource,
+    field_name: str,
+    config_values: Mapping[str, JsonValue],
+    db_values: Mapping[str, JsonValue],
+) -> JsonValue | None:
+    if source == "config":
+        return config_values.get(field_name)
+    if source == "db":
+        return db_values.get(field_name)
+    return None
+
+
+def _alerting_stored_in_db(source: FieldSource) -> bool | None:
+    if source == "db":
+        return True
+    if source == "config":
+        return False
+    return None
+
+
+def _alerting_field_response(
+    *,
+    settings: SettingsStore,
+    db_values: Mapping[str, JsonValue],
+    config_values: Mapping[str, JsonValue],
+    allowed_args: Mapping[str, str],
+    field_name: str,
+    field_info: FieldInfo,
+) -> ConfigList:
+    field_default: Final[JsonValue] = _get_field_default(field_info)
+    field_source: Final[FieldSource] = _nested_setting_source(
+        settings,
+        db_values,
+        "alerting_args",
+        field_name,
+        field_default,
+    )
+    field_value: Final = _alerting_field_value(field_source, field_name, config_values, db_values)
+    stored_in_db: Final = _alerting_stored_in_db(field_source)
+    return ConfigList(
+        field_name=field_name,
+        field_type=allowed_args[field_name],
+        field_description=field_info.description or "",
+        field_value=field_value,
+        stored_in_db=stored_in_db,
+        source=field_source,
+        field_default_value=field_default,
+        premium_field=field_name == "region_outage_alert_ttl",
+    )
+
+
 @router.get(
     "/alerting/settings",
     description="Return the configurable alerting param, description, and current value",
@@ -16739,7 +16790,7 @@ def _nested_setting_source(
 async def alerting_settings(
     user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
 ):
-    global proxy_logging_obj, prisma_client
+    global prisma_client
     """
     Used by UI to generate 'alerting settings' page
     {
@@ -16801,8 +16852,10 @@ async def alerting_settings(
         }
     )
 
-    _slack_alerting: Final[SlackAlerting] = proxy_logging_obj.slack_alerting_instance
-    _slack_alerting_args_dict: Final = _slack_alerting.alerting_args.model_dump()
+    config_alerting_args_value: Final = settings.config_value("alerting_args")
+    config_alerting_args: Final[Mapping[str, JsonValue]] = (
+        config_alerting_args_value if isinstance(config_alerting_args_value, Mapping) else MappingProxyType({})
+    )
 
     return_val: Final = []
 
@@ -16826,30 +16879,16 @@ async def alerting_settings(
 
     for field_name, field_info in SlackAlertingArgs.model_fields.items():
         if field_name in allowed_args:
-            field_default: JsonValue = _get_field_default(field_info)
-            _stored_in_db: bool | None = None
-            if field_name in alerting_args_dict:
-                _stored_in_db = True
-            else:
-                _stored_in_db = False
-
-            _response_obj = ConfigList(
-                field_name=field_name,
-                field_type=allowed_args[field_name],
-                field_description=field_info.description or "",
-                field_value=_slack_alerting_args_dict.get(field_name, field_default),
-                stored_in_db=_stored_in_db,
-                source=_nested_setting_source(
-                    settings,
-                    alerting_args_dict,
-                    "alerting_args",
-                    field_name,
-                    field_default,
-                ),
-                field_default_value=field_default,
-                premium_field=(True if field_name == "region_outage_alert_ttl" else False),
+            return_val.append(
+                _alerting_field_response(
+                    settings=settings,
+                    db_values=alerting_args_dict,
+                    config_values=config_alerting_args,
+                    allowed_args=allowed_args,
+                    field_name=field_name,
+                    field_info=field_info,
+                )
             )
-            return_val.append(_response_obj)
     return return_val
 
 

tests/unit/proxy/proxy_server/test_routes_model_metrics.py

@@ -402,3 +402,109 @@ def test_alerting_settings_happy(client, auth_as, monkeypatch):
         "first_field_value": False,
         "first_field_type": "Boolean",
     }
+
+def test_alerting_settings_only_returns_explicitly_configured_values(client, auth_as, monkeypatch):
+    pc = MagicMock()
+    pc.db.litellm_config.find_first = AsyncMock(return_value=None)
+    monkeypatch.setattr(proxy_server, "prisma_client", pc)
+
+    logging_obj = MagicMock()
+    args_model = MagicMock()
+    args_model.model_dump = MagicMock(return_value={"budget_alert_ttl": -30})
+    logging_obj.slack_alerting_instance.alerting_args = args_model
+    monkeypatch.setattr(proxy_server, "proxy_logging_obj", logging_obj)
+    settings = SettingsStore("general_settings")
+    settings.load_yaml({})
+    monkeypatch.setattr(proxy_server.proxy_config, "settings", settings)
+
+    with auth_as(LitellmUserRoles.PROXY_ADMIN):
+        response = client.get("/alerting/settings")
+
+    assert response.status_code == 200
+    setting = next(item for item in response.json() if item["field_name"] == "budget_alert_ttl")
+    assert setting["field_value"] is None
+    assert setting["stored_in_db"] is None
+
+
+def test_alerting_settings_returns_configured_values(client, auth_as, monkeypatch):
+    pc = MagicMock()
+    pc.db.litellm_config.find_first = AsyncMock(return_value=None)
+    monkeypatch.setattr(proxy_server, "prisma_client", pc)
+    settings = SettingsStore("general_settings")
+    settings.load_yaml({"alerting_args": {"budget_alert_ttl": 60}})
+    monkeypatch.setattr(proxy_server.proxy_config, "settings", settings)
+
+    with auth_as(LitellmUserRoles.PROXY_ADMIN):
+        response = client.get("/alerting/settings")
+
+    assert response.status_code == 200
+    setting = next(item for item in response.json() if item["field_name"] == "budget_alert_ttl")
+    assert setting["field_value"] == 60
+    assert setting["stored_in_db"] is False
+
+
+def test_alerting_settings_returns_database_values(client, auth_as, monkeypatch):
+    db_settings = MagicMock()
+    db_settings.param_value = {"alerting_args": {"budget_alert_ttl": 60}}
+    pc = MagicMock()
+    pc.db.litellm_config.find_first = AsyncMock(return_value=db_settings)
+    monkeypatch.setattr(proxy_server, "prisma_client", pc)
+    settings = SettingsStore("general_settings")
+    settings.load_yaml({})
+    monkeypatch.setattr(proxy_server.proxy_config, "settings", settings)
+
+    with auth_as(LitellmUserRoles.PROXY_ADMIN):
+        response = client.get("/alerting/settings")
+
+    assert response.status_code == 200
+    setting = next(item for item in response.json() if item["field_name"] == "budget_alert_ttl")
+    assert setting["field_value"] == 60
+    assert setting["stored_in_db"] is True
+
+@pytest.mark.parametrize(
+    ("source", "expected_value", "expected_stored_in_db"),
+    [
+        ("config", 60, False),
+        ("db", 90, True),
+        ("unset", None, None),
+        ("default", None, None),
+    ],
+)
+def test_alerting_field_helpers_cover_each_source(
+    source, expected_value, expected_stored_in_db
+):
+    config_values = {"budget_alert_ttl": 60}
+    db_values = {"budget_alert_ttl": 90}
+
+    assert (
+        proxy_server._alerting_field_value(
+            source,
+            "budget_alert_ttl",
+            config_values,
+            db_values,
+        )
+        == expected_value
+    )
+    assert proxy_server._alerting_stored_in_db(source) is expected_stored_in_db
+
+
+def test_alerting_field_response_uses_explicit_config_value():
+    settings = SettingsStore("general_settings")
+    settings.load_yaml({"alerting_args": {"budget_alert_ttl": 60}})
+    field_info = proxy_server.SlackAlertingArgs.model_fields["budget_alert_ttl"]
+
+    result = proxy_server._alerting_field_response(
+        settings=settings,
+        db_values={},
+        config_values={"budget_alert_ttl": 60},
+        allowed_args={"budget_alert_ttl": "Integer"},
+        field_name="budget_alert_ttl",
+        field_info=field_info,
+    )
+
+    assert result.field_name == "budget_alert_ttl"
+    assert result.field_type == "Integer"
+    assert result.field_value == 60
+    assert result.stored_in_db is False
+    assert result.source == "config"
+

ui/litellm-dashboard/src/components/alerting/alerting_settings.integration.test.tsx

@@ -0,0 +1,147 @@
+import React from "react";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { act, fireEvent, screen, waitFor } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { renderWithProviders } from "@/../tests/test-utils";
+import AlertingSettings from "./alerting_settings";
+
+const { alertingSettingsCall, updateConfigFieldSetting } = vi.hoisted(() => ({
+  alertingSettingsCall: vi.fn(),
+  updateConfigFieldSetting: vi.fn(),
+}));
+
+vi.mock("@/components/networking", () => ({ alertingSettingsCall, updateConfigFieldSetting }));
+
+vi.mock("@/lib/toast", () => ({
+  toast: { error: vi.fn(), success: vi.fn() },
+}));
+
+const settingsResponse = [
+  {
+    field_name: "slack_alerting",
+    field_type: "Boolean",
+    field_value: false,
+    field_default_value: null,
+    field_description: "Enable Slack alerting",
+    stored_in_db: null,
+    premium_field: false,
+  },
+  {
+    field_name: "budget_alert_ttl",
+    field_type: "Integer",
+    field_value: 60,
+    field_default_value: 30,
+    field_description: "Configured threshold",
+    stored_in_db: true,
+    premium_field: false,
+  },
+  {
+    field_name: "outage_alert_ttl",
+    field_type: "Integer",
+    field_value: null,
+    field_default_value: 10,
+    field_description: "Unset threshold",
+    stored_in_db: null,
+    premium_field: false,
+  },
+];
+
+describe("AlertingSettings", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    alertingSettingsCall.mockResolvedValue(settingsResponse);
+    updateConfigFieldSetting.mockResolvedValue({});
+  });
+
+  it("does not submit unset fields when updating another alerting setting", async () => {
+    const user = userEvent.setup();
+    renderWithProviders(<AlertingSettings accessToken="sk-test" premiumUser />);
+
+    await screen.findByDisplayValue("60");
+    const numberInputs = screen.getAllByRole("spinbutton");
+    fireEvent.change(numberInputs[1], { target: { value: "5" } });
+    fireEvent.change(numberInputs[1], { target: { value: "" } });
+    await user.click(screen.getByRole("switch", { name: "slack_alerting" }));
+    await user.click(screen.getByRole("button", { name: "Update Settings" }));
+
+    await waitFor(() => {
+      expect(updateConfigFieldSetting).toHaveBeenNthCalledWith(1, "sk-test", "alerting_args", {
+        budget_alert_ttl: 60,
+      });
+    });
+    expect(updateConfigFieldSetting).toHaveBeenNthCalledWith(2, "sk-test", "alerting", ["slack"]);
+  });
+
+  it("does not restore a stale typed value after reset", async () => {
+    const user = userEvent.setup();
+    renderWithProviders(<AlertingSettings accessToken="sk-test" premiumUser />);
+
+    const budgetInput = await screen.findByDisplayValue("60");
+    fireEvent.change(budgetInput, { target: { value: "75" } });
+
+    await user.click(screen.getByRole("button", { name: "Reset budget_alert_ttl" }));
+    await user.click(screen.getByRole("button", { name: "Update Settings" }));
+
+    await waitFor(() => {
+      expect(updateConfigFieldSetting).toHaveBeenCalledWith("sk-test", "alerting_args", {});
+    });
+  });
+
+  it("clears reset and form state when the access token changes", async () => {
+    const refreshedSettings = settingsResponse.map((setting) =>
+      setting.field_name === "budget_alert_ttl" ? { ...setting, field_value: 90 } : setting,
+    );
+    alertingSettingsCall.mockImplementation((token: string) =>
+      Promise.resolve(token === "sk-new" ? refreshedSettings : settingsResponse),
+    );
+
+    const user = userEvent.setup();
+    const { rerender } = renderWithProviders(<AlertingSettings accessToken="sk-old" premiumUser />);
+
+    const budgetInput = await screen.findByDisplayValue("60");
+    fireEvent.change(budgetInput, { target: { value: "75" } });
+    await user.click(screen.getByRole("button", { name: "Reset budget_alert_ttl" }));
+
+    rerender(<AlertingSettings accessToken="sk-new" premiumUser />);
+    await screen.findByDisplayValue("90");
+
+    await user.click(screen.getByRole("switch", { name: "slack_alerting" }));
+    await user.click(screen.getByRole("button", { name: "Update Settings" }));
+
+    await waitFor(() => {
+      expect(updateConfigFieldSetting).toHaveBeenNthCalledWith(1, "sk-new", "alerting_args", {
+        budget_alert_ttl: 90,
+      });
+    });
+    expect(updateConfigFieldSetting).toHaveBeenNthCalledWith(2, "sk-new", "alerting", ["slack"]);
+  });
+
+  it("ignores an older settings response after the access token changes", async () => {
+    const oldRequest = Promise.withResolvers<typeof settingsResponse>();
+    const newRequest = Promise.withResolvers<typeof settingsResponse>();
+    const refreshedSettings = settingsResponse.map((setting) =>
+      setting.field_name === "budget_alert_ttl" ? { ...setting, field_value: 90 } : setting,
+    );
+
+    alertingSettingsCall.mockImplementation((token: string) =>
+      token === "sk-new" ? newRequest.promise : oldRequest.promise,
+    );
+
+    const { rerender } = renderWithProviders(<AlertingSettings accessToken="sk-old" premiumUser />);
+    rerender(<AlertingSettings accessToken="sk-new" premiumUser />);
+
+    await act(async () => {
+      newRequest.resolve(refreshedSettings);
+      await newRequest.promise;
+    });
+    await screen.findByDisplayValue("90");
+
+    await act(async () => {
+      oldRequest.resolve(settingsResponse);
+      await oldRequest.promise;
+    });
+
+    expect(screen.getByDisplayValue("90")).toBeInTheDocument();
+    expect(screen.queryByDisplayValue("60")).not.toBeInTheDocument();
+  });
+});

ui/litellm-dashboard/src/components/alerting/alerting_settings.tsx

@@ -24,19 +24,32 @@ interface AlertingSettingsProps {
 
 const AlertingSettings: React.FC<AlertingSettingsProps> = ({ accessToken, premiumUser }) => {
   const [alertingSettings, setAlertingSettings] = useState<alertingSettingsItem[]>([]);
+  const [resetFields, setResetFields] = useState<Set<string>>(new Set());
 
   useEffect(() => {
-    // get values
     if (!accessToken) {
       return;
     }
+
+    const controller = new AbortController();
     alertingSettingsCall(accessToken).then((data) => {
+      if (controller.signal.aborted) {
+        return;
+      }
+      setResetFields(new Set());
       setAlertingSettings(data);
     });
+
+    return () => {
+      controller.abort();
+    };
   }, [accessToken]);
 
   const handleInputChange = (fieldName: string, newValue: any) => {
-    // Update the value in the state
+    setResetFields((previous) =>
+      previous.has(fieldName) ? new Set([...previous].filter((name) => name !== fieldName)) : previous,
+    );
+
     const updatedSettings = alertingSettings.map((setting) =>
       setting.field_name === fieldName ? { ...setting, field_value: newValue } : setting,
     );
@@ -49,21 +62,32 @@ const AlertingSettings: React.FC<AlertingSettingsProps> = ({ accessToken, premiu
       return;
     }
 
-    let fieldValue = formValues;
+    const fieldValue = formValues;
 
     if (fieldValue == null || fieldValue == undefined) {
       return;
     }
 
-    const initialFormValues: Record<string, any> = {};
+    const configuredAlertingArgs: Record<string, unknown> = Object.fromEntries(
+      alertingSettings
+        .filter(
+          (setting) =>
+            setting.field_name !== "slack_alerting" &&
+            !resetFields.has(setting.field_name) &&
+            setting.field_value != null,
+        )
+        .map((setting) => [setting.field_name, setting.field_value]),
+    );
 
-    alertingSettings.forEach((setting) => {
-      initialFormValues[setting.field_name] = setting.field_value;
-    });
-
-    // Merge initialFormValues with actual formValues
-    const mergedFormValues = { ...formValues, ...initialFormValues };
-    const { slack_alerting, ...alertingArgs } = mergedFormValues;
+    const { slack_alerting, ...updatedAlertingArgs } = formValues;
+    const alertingArgs = {
+      ...configuredAlertingArgs,
+      ...Object.fromEntries(
+        Object.entries(updatedAlertingArgs).filter(
+          ([fieldName, value]) => !resetFields.has(fieldName) && value != null && value !== "",
+        ),
+      ),
+    };
     try {
       await updateConfigFieldSetting(accessToken, "alerting_args", alertingArgs);
       if (typeof slack_alerting === "boolean") {
@@ -86,8 +110,7 @@ const AlertingSettings: React.FC<AlertingSettingsProps> = ({ accessToken, premiu
     }
 
     try {
-      //   deleteConfigFieldSetting(accessToken, fieldName);
-      // update value in state
+      setResetFields((previous) => new Set([...previous, fieldName]));
 
       const updatedSettings = alertingSettings.map((setting) =>
         setting.field_name === fieldName
@@ -106,6 +129,7 @@ const AlertingSettings: React.FC<AlertingSettingsProps> = ({ accessToken, premiu
 
   return (
     <DynamicForm
+      key={accessToken ?? "no-access-token"}
       alertingSettings={alertingSettings}
       handleInputChange={handleInputChange}
       handleResetField={handleResetField}