fix(ui): send router_settings list/dict fields as JSON, surface save errors

tests/test_litellm/proxy/proxy_server/test_config_update_routing_groups.py

@@ -0,0 +1,156 @@
+"""Functional + contract tests for POST /config/update router_settings.
+
+Regression coverage for the bug where the Admin UI's "Reliability & Retries"
+form serialized the ``routing_groups`` List field as the JSON *string* ``"[]"``
+and POSTed it to ``/config/update``. FastAPI validated the body against
+``ConfigYAML`` and rejected the string with a ``list_type`` error, returning a
+422 for the *entire* router_settings payload — silently dropping every other
+field in the section (allowed_fails, cooldown_time, timeout, ...).
+
+The UI fix sends a real list; these tests pin both the rejection of the old
+(string) shape and the acceptance + persistence of the new (list) shape.
+"""
+
+from __future__ import annotations
+
+import json
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+from pydantic import ValidationError
+
+from litellm.proxy import proxy_server
+from litellm.proxy._types import ConfigYAML, LitellmUserRoles
+
+# ---------------------------------------------------------------------------
+# Body-model contract (what FastAPI validates before the handler runs)
+# ---------------------------------------------------------------------------
+
+
+def test_config_yaml_rejects_routing_groups_string():
+    """ConfigYAML.router_settings.routing_groups rejects a JSON string."""
+    with pytest.raises(ValidationError) as exc_info:
+        ConfigYAML(router_settings={"routing_groups": "[]"})
+
+    assert any(
+        err["loc"][-2:] == ("router_settings", "routing_groups")
+        and err["type"] == "list_type"
+        for err in exc_info.value.errors()
+    )
+
+
+def test_config_yaml_accepts_routing_groups_list():
+    """An empty list plus a sibling numeric field validate cleanly."""
+    cfg = ConfigYAML(router_settings={"routing_groups": [], "num_retries": 3})
+
+    assert cfg.router_settings is not None
+    assert cfg.router_settings.routing_groups == []
+    assert cfg.router_settings.num_retries == 3
+
+
+# ---------------------------------------------------------------------------
+# HTTP layer — reproduces the reported 422 and proves the fix is accepted
+# ---------------------------------------------------------------------------
+
+
+def test_config_update_rejects_routing_groups_string_returns_422(client, auth_as):
+    """POST /config/update with routing_groups="[]" -> 422 (the reported bug).
+
+    Validation fails before the handler, so no DB is required.
+    """
+    with auth_as(LitellmUserRoles.PROXY_ADMIN):
+        response = client.post(
+            "/config/update",
+            json={"router_settings": {"routing_groups": "[]"}},
+        )
+
+    assert response.status_code == 422
+    detail = response.json()["detail"]
+    assert isinstance(detail, list)
+    assert any(
+        err.get("type") == "list_type" and err.get("loc", [])[-1] == "routing_groups"
+        for err in detail
+    )
+
+
+def test_config_update_accepts_routing_groups_list(
+    client, auth_as, mock_prisma, monkeypatch
+):
+    """POST /config/update with routing_groups=[] -> 200 and persists a list.
+
+    The serialized router_settings written to the config table must contain a
+    real list for routing_groups, never the string "[]".
+    """
+    config_table = MagicMock()
+    config_table.find_first = AsyncMock(return_value=None)
+    config_table.upsert = AsyncMock()
+    mock_prisma.db.litellm_config = config_table
+
+    monkeypatch.setattr(proxy_server, "prisma_client", mock_prisma)
+    monkeypatch.setattr(proxy_server, "invalidate_config_param", AsyncMock())
+    monkeypatch.setattr(proxy_server.proxy_config, "add_deployment", AsyncMock())
+
+    with auth_as(LitellmUserRoles.PROXY_ADMIN):
+        response = client.post(
+            "/config/update",
+            json={"router_settings": {"routing_groups": [], "num_retries": 3}},
+        )
+
+    assert response.status_code == 200
+
+    # Locate the router_settings upsert and decode the persisted value.
+    persisted = None
+    for call in config_table.upsert.await_args_list:
+        if call.kwargs.get("where", {}).get("param_name") == "router_settings":
+            persisted = json.loads(call.kwargs["data"]["create"]["param_value"])
+
+    assert persisted is not None, "router_settings was never upserted"
+    assert persisted["routing_groups"] == []
+    assert isinstance(persisted["routing_groups"], list)
+    assert persisted["num_retries"] == 3
+
+
+def test_config_update_persists_full_reliability_payload(
+    client, auth_as, mock_prisma, monkeypatch
+):
+    """Backward compatibility: a realistic Reliability & Retries payload (no
+    routing_groups) still returns 200 and persists every field.
+
+    The reported symptom was "none of these values save". This proves the
+    common case — the full set of fields the section sends — round-trips
+    intact through /config/update.
+    """
+    config_table = MagicMock()
+    config_table.find_first = AsyncMock(return_value=None)
+    config_table.upsert = AsyncMock()
+    mock_prisma.db.litellm_config = config_table
+
+    monkeypatch.setattr(proxy_server, "prisma_client", mock_prisma)
+    monkeypatch.setattr(proxy_server, "invalidate_config_param", AsyncMock())
+    monkeypatch.setattr(proxy_server.proxy_config, "add_deployment", AsyncMock())
+
+    payload = {
+        "allowed_fails": 5,
+        "cooldown_time": 30.0,
+        "num_retries": 2,
+        "timeout": 60.0,
+        "retry_after": 1,
+        "retry_policy": {"RateLimitErrorRetries": 3},
+    }
+
+    with auth_as(LitellmUserRoles.PROXY_ADMIN):
+        response = client.post(
+            "/config/update",
+            json={"router_settings": payload},
+        )
+
+    assert response.status_code == 200
+
+    persisted = None
+    for call in config_table.upsert.await_args_list:
+        if call.kwargs.get("where", {}).get("param_name") == "router_settings":
+            persisted = json.loads(call.kwargs["data"]["create"]["param_value"])
+
+    assert persisted is not None, "router_settings was never upserted"
+    for key, value in payload.items():
+        assert persisted[key] == value, f"{key} did not round-trip"

tests/test_litellm/types/test_router_types.py

@@ -1,6 +1,53 @@
 """Unit tests for litellm.types.router config models."""
 
-from litellm.types.router import RetryPolicy, UpdateRouterConfig
+import pytest
+from pydantic import ValidationError
+
+from litellm.types.router import RetryPolicy, RoutingGroup, UpdateRouterConfig
+
+
+def test_update_router_config_rejects_routing_groups_json_string():
+    """A JSON *string* like "[]" must not be accepted for routing_groups.
+
+    The Admin UI used to render this List field as a text input and POST its
+    value as the string "[]". Pydantic rejected it with a ``list_type`` error,
+    which made FastAPI 422 the whole router_settings body and silently dropped
+    every other Reliability & Retries value. Pin the strict behaviour so the
+    contract the UI must satisfy stays explicit.
+    """
+    with pytest.raises(ValidationError) as exc_info:
+        UpdateRouterConfig(routing_groups="[]")
+
+    assert any(
+        err["loc"] == ("routing_groups",) and err["type"] == "list_type"
+        for err in exc_info.value.errors()
+    )
+
+
+def test_update_router_config_accepts_empty_routing_groups_list():
+    """An actual empty list (what the fixed UI now sends) is valid."""
+    cfg = UpdateRouterConfig(routing_groups=[])
+
+    assert cfg.routing_groups == []
+
+
+def test_update_router_config_parses_routing_groups_entries():
+    """Populated routing_groups are coerced into RoutingGroup models."""
+    cfg = UpdateRouterConfig(
+        routing_groups=[
+            {
+                "group_name": "group-a",
+                "models": ["gpt-4"],
+                "routing_strategy": "simple-shuffle",
+            }
+        ]
+    )
+
+    assert cfg.routing_groups is not None
+    assert len(cfg.routing_groups) == 1
+    assert isinstance(cfg.routing_groups[0], RoutingGroup)
+    assert cfg.routing_groups[0].group_name == "group-a"
+    assert cfg.routing_groups[0].models == ["gpt-4"]
 
 
 def test_update_router_config_retains_retry_policy():

ui/litellm-dashboard/src/components/router_settings/index.test.tsx

@@ -158,8 +158,139 @@ describe("RouterSettings", () => {
     });
     await user.click(screen.getByRole("button", { name: /save changes/i }));
 
-    expect(NotificationsManager.success).toHaveBeenCalledWith(
-      "router settings updated successfully"
-    );
+    await waitFor(() => {
+      expect(NotificationsManager.success).toHaveBeenCalledWith(
+        "router settings updated successfully"
+      );
+    });
+  });
+
+  it("should send list-typed settings as arrays, not JSON strings", async () => {
+    // Reproduces the 422 reported on /config/update: routing_groups (a List
+    // field) was rendered as the text input "[]" and sent back as the string
+    // "[]", which the backend rejected ("Input should be a valid list"),
+    // silently dropping every other Reliability & Retries value.
+    vi.mocked(getCallbacksCall).mockResolvedValue({
+      router_settings: {
+        routing_strategy: "simple-shuffle",
+        num_retries: 3,
+        routing_groups: [{ group_name: "group-a", models: ["gpt-4"], routing_strategy: "simple-shuffle" }],
+      },
+    });
+
+    const user = userEvent.setup();
+    renderWithProviders(<RouterSettings {...defaultProps} />);
+
+    await waitFor(() => {
+      expect(screen.getByRole("textbox", { name: /routing_groups/i })).toBeInTheDocument();
+    });
+
+    await user.click(screen.getByRole("button", { name: /save changes/i }));
+
+    await waitFor(() => {
+      expect(setCallbacksCall).toHaveBeenCalled();
+    });
+
+    const payload = vi.mocked(setCallbacksCall).mock.calls[0][1] as {
+      router_settings: Record<string, unknown>;
+    };
+    const sentRoutingGroups = payload.router_settings.routing_groups;
+    expect(Array.isArray(sentRoutingGroups)).toBe(true);
+    expect(sentRoutingGroups).toEqual([
+      { group_name: "group-a", models: ["gpt-4"], routing_strategy: "simple-shuffle" },
+    ]);
+    // num_retries must still be coerced to a number, not left as "3".
+    expect(payload.router_settings.num_retries).toBe(3);
+  });
+
+  it("should surface the backend error and not show success when saving fails", async () => {
+    vi.mocked(setCallbacksCall).mockRejectedValueOnce(new Error("Input should be a valid list"));
+
+    const user = userEvent.setup();
+    renderWithProviders(<RouterSettings {...defaultProps} />);
+
+    await waitFor(() => {
+      expect(screen.getByTestId("strategy-select")).toBeInTheDocument();
+    });
+
+    await user.click(screen.getByRole("button", { name: /save changes/i }));
+
+    await waitFor(() => {
+      expect(NotificationsManager.fromBackend).toHaveBeenCalled();
+    });
+    expect(NotificationsManager.success).not.toHaveBeenCalled();
+  });
+
+  // Backward-compatibility regression tests. The fix refactored
+  // parseInputValue (introducing field_type-aware helpers). These pin the
+  // serialization behavior of the field types that already worked BEFORE the
+  // change, so the refactor cannot silently alter them.
+  describe("backward compatibility — existing field types still serialize correctly", () => {
+    const saveAndGetPayload = async () => {
+      const user = userEvent.setup();
+      renderWithProviders(<RouterSettings {...defaultProps} />);
+      await waitFor(() => {
+        expect(screen.getByRole("button", { name: /save changes/i })).toBeInTheDocument();
+      });
+      await user.click(screen.getByRole("button", { name: /save changes/i }));
+      await waitFor(() => {
+        expect(setCallbacksCall).toHaveBeenCalled();
+      });
+      return (vi.mocked(setCallbacksCall).mock.calls[0][1] as { router_settings: Record<string, unknown> })
+        .router_settings;
+    };
+
+    it("should still coerce number fields to numbers, not strings", async () => {
+      vi.mocked(getCallbacksCall).mockResolvedValue({
+        router_settings: { routing_strategy: "simple-shuffle", num_retries: 5, timeout: 42 },
+      });
+
+      const sent = await saveAndGetPayload();
+
+      expect(sent.num_retries).toBe(5);
+      expect(typeof sent.num_retries).toBe("number");
+      expect(sent.timeout).toBe(42);
+      expect(typeof sent.timeout).toBe("number");
+    });
+
+    it("should still parse retry_policy back into an object", async () => {
+      vi.mocked(getCallbacksCall).mockResolvedValue({
+        router_settings: {
+          routing_strategy: "simple-shuffle",
+          retry_policy: { RateLimitErrorRetries: 2, InternalServerErrorRetries: 5 },
+        },
+      });
+
+      const sent = await saveAndGetPayload();
+
+      expect(sent.retry_policy).toEqual({
+        RateLimitErrorRetries: 2,
+        InternalServerErrorRetries: 5,
+      });
+    });
+
+    it("should still parse model_group_alias back into an object", async () => {
+      vi.mocked(getCallbacksCall).mockResolvedValue({
+        router_settings: {
+          routing_strategy: "simple-shuffle",
+          model_group_alias: { "gpt-4": "azure-gpt-4" },
+        },
+      });
+
+      const sent = await saveAndGetPayload();
+
+      expect(sent.model_group_alias).toEqual({ "gpt-4": "azure-gpt-4" });
+    });
+
+    it("should still convert boolean-valued fields to booleans", async () => {
+      vi.mocked(getCallbacksCall).mockResolvedValue({
+        router_settings: { routing_strategy: "simple-shuffle", set_verbose: true },
+      });
+
+      const sent = await saveAndGetPayload();
+
+      expect(sent.set_verbose).toBe(true);
+      expect(typeof sent.set_verbose).toBe("boolean");
+    });
   });
 });