From 4ed3e712e0a8642d60d87dd79f10d9df2d4be4f0 Mon Sep 17 00:00:00 2001 From: Krrish Dholakia Date: Sat, 25 Apr 2026 19:03:43 -0700 Subject: [PATCH] Litellm memory improvements v2 (#26541) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(memory): jsonify metadata before Prisma writes on /v1/memory The POST/PUT memory endpoints handed bare dicts (and bare `None`) to prisma-client-python for the `Json?` `metadata` column, which the client rejects with `MissingRequiredValueError` / `DataError: metadata should be of any of the following types: NullableJsonNullValueInput, Json`. Both the create and upsert paths now route writes through the existing `jsonify_object` helper used elsewhere in the proxy for `Json?` columns (e.g. `LiteLLM_VerificationToken.budget_limits`), and omit metadata when None so the column defaults to SQL NULL via the schema. Explicit `metadata: null` on PUT is now a no-op for the column to match how the rest of the proxy handles nullable JSON fields (no `JsonNull`/`DbNull` sentinel exists in prisma-client-python — see RobertCraigie/prisma-client-py#714). A payload with only `metadata: null` returns 400 instead of a misleading 200. Made-with: Cursor * fix(memory): JSON-encode non-dict metadata before Prisma writes `jsonify_object` only stringifies dict values, so list-shaped metadata still hit Prisma as raw Python objects and triggered the same DataError this PR is meant to fix. `metadata` is typed `Optional[Any]` so list payloads are valid input. Replace `jsonify_object` with a local `_serialize_metadata_for_prisma` helper that always `json.dumps` non-string values, applied at all three write sites (POST create, PUT update, PUT-create). Adds regression tests for list metadata on each path. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(memory): always json.dumps metadata, not just non-strings The str-passthrough in `_serialize_metadata_for_prisma` left plain Python strings (e.g. `metadata: "hello"`) unencoded — Postgres `jsonb` rejects bare-word strings as invalid JSON, reproducing the same DataError this PR is meant to fix. Always `json.dumps` regardless of input type so all `Optional[Any]` shapes (dict, list, scalar, str) become valid JSON. Adds a regression test for plain-string metadata. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(memory): encode explicit metadata:null as JSON null to clear field prisma-client-python has no JsonNull/DbNull sentinel for writing a true SQL NULL on `Json?` columns (RobertCraigie/prisma-client-py#714), so an earlier iteration of this PR treated `PUT {"metadata": null}` as a no-op. That doesn't match the natural caller expectation that explicit-null clears the field. Encode it as the JSON literal `null` instead — stored as Postgres `jsonb 'null'`, which prisma deserializes back to Python `None` on read. Subsequent reads return `metadata: null`, so the field is effectively cleared from the caller's perspective. Strict SQL NULL remains unreachable via the typed client and would require raw SQL. Also clean up stale `jsonify_object` references in test mock comments (replaced by `_serialize_metadata_for_prisma`). Co-Authored-By: Claude Opus 4.7 (1M context) * feat(memory ui): use shared DeleteResourceModal for memory deletion Swap the imperative `Modal.confirm` in MemoryView for the shared `DeleteResourceModal`, so memory deletion matches the rest of the dashboard: type-to-confirm guard on the key, in-flight loading state on the OK button, cancel disabled while the request is pending, and the modal stays open on error so the user can retry. Made-with: Cursor --------- Co-authored-by: Claude Opus 4.7 (1M context) --- litellm/proxy/memory/memory_endpoints.py | 27 +++++----- .../proxy/memory/test_memory_endpoints.py | 38 +++++++------- .../src/components/MemoryView/MemoryView.tsx | 51 ++++++++++++++----- 3 files changed, 73 insertions(+), 43 deletions(-) diff --git a/litellm/proxy/memory/memory_endpoints.py b/litellm/proxy/memory/memory_endpoints.py index 39d5a03f2d..4d161be426 100644 --- a/litellm/proxy/memory/memory_endpoints.py +++ b/litellm/proxy/memory/memory_endpoints.py @@ -431,24 +431,25 @@ async def upsert_memory( """ prisma_client = _require_prisma() - # `metadata` is a `Json?` column. prisma-client-python rejects raw - # Python values on `Json?` fields, and there is no `JsonNull`/`DbNull` - # sentinel yet (RobertCraigie/prisma-client-py#714) so we have no way - # to write a true SQL NULL via the typed client. We mirror the rest of - # the proxy's handling of nullable `Json?` columns: forward metadata - # only when the caller sent a non-null value (always JSON-encoded), - # and treat explicit `metadata: null` as a no-op for the column. This - # matches the prior crashing behavior the PR fixes — there is no - # regression of a previously-working "clear metadata" path, and the - # rest of the proxy gives the same treatment to nullable `Json?` - # fields elsewhere. + # `metadata` is a `Json?` column. prisma-client-python has no + # `JsonNull`/`DbNull` sentinel for writing a true SQL NULL + # (RobertCraigie/prisma-client-py#714), so an explicit `metadata: null` + # is encoded as the JSON literal `null` instead — stored as Postgres + # `jsonb 'null'`, which prisma deserializes back to Python `None` on + # read. From a caller's perspective `PUT {"metadata": null}` clears + # the field (subsequent reads return `metadata: null`), matching the + # natural expectation. Callers wanting a strict SQL NULL must use + # raw SQL — there is no typed-client path. + # + # When `metadata` is omitted from the request body entirely (not in + # `model_fields_set`), the column is preserved as-is. fields_sent = body.model_fields_set - metadata_explicit_value = "metadata" in fields_sent and body.metadata is not None + metadata_in_payload = "metadata" in fields_sent data: dict = {} if body.value is not None: data["value"] = body.value - if metadata_explicit_value: + if metadata_in_payload: data["metadata"] = _serialize_metadata_for_prisma(body.metadata) if not data: raise HTTPException( diff --git a/tests/test_litellm/proxy/memory/test_memory_endpoints.py b/tests/test_litellm/proxy/memory/test_memory_endpoints.py index 2ddcbb370b..ca011c77af 100644 --- a/tests/test_litellm/proxy/memory/test_memory_endpoints.py +++ b/tests/test_litellm/proxy/memory/test_memory_endpoints.py @@ -97,8 +97,8 @@ class _InMemoryMemoryTable: self._counter += 1 # Mirror real Prisma read-side behavior for `Json?` columns: writes # come in as JSON strings (the endpoint pre-processes via - # `jsonify_object`), and Prisma deserializes them back to Python - # values on read. + # `_serialize_metadata_for_prisma`), and Prisma deserializes them + # back to Python values on read. metadata = data.get("metadata") if isinstance(metadata, str): try: @@ -143,8 +143,9 @@ class _InMemoryMemoryTable: if r.memory_id == where["memory_id"]: for k, v in data.items(): # Mirror real Prisma's read behavior for `Json?` columns: - # the endpoint sends JSON strings via `jsonify_object`, - # and Prisma round-trips them back to Python values. + # the endpoint sends JSON strings via + # `_serialize_metadata_for_prisma`, and Prisma + # round-trips them back to Python values. if k == "metadata" and isinstance(v, str): try: v = _json.loads(v) @@ -647,15 +648,15 @@ class TestMemoryEndpoints: assert resp.json()["value"] == "new" assert len(table.rows) == 1 - def test_put_memory_explicit_null_metadata_is_noop(self): + def test_put_memory_explicit_null_metadata_clears_field(self): """ prisma-client-python can't write a true SQL NULL to a `Json?` column (no `JsonNull`/`DbNull` sentinel — see - RobertCraigie/prisma-client-py#714). We mirror the rest of the proxy - and treat `metadata: null` as "leave the column alone" rather than - 500ing or silently writing a JSON `null`. The caller can still - update other fields in the same request; existing metadata is - preserved. + RobertCraigie/prisma-client-py#714). We instead encode an explicit + `metadata: null` as the JSON literal `null` (Postgres `jsonb 'null'`). + prisma deserializes that back to Python `None` on read, so from a + caller's perspective the field is cleared — matching the natural + expectation of `PUT {"metadata": null}`. """ table = self.prisma.db.litellm_memorytable table.rows.append( @@ -676,14 +677,15 @@ class TestMemoryEndpoints: assert resp.status_code == 200, resp.text body = resp.json() assert body["value"] == "new" - assert body["metadata"] == {"tag": "old"} - assert table.rows[0].metadata == {"tag": "old"} + assert body["metadata"] is None + assert table.rows[0].metadata is None - def test_put_memory_null_metadata_alone_returns_400(self): + def test_put_memory_null_metadata_alone_clears_field(self): """ - With explicit-null treated as a no-op, a payload that ONLY carries - `metadata: null` has no effective fields to write — surface 400 so - the caller doesn't get a misleading 200 with no state change. + A payload that ONLY carries `metadata: null` should clear the + column — the field is effective, not skipped. (Earlier iterations + of this PR treated explicit-null as a no-op and surfaced 400; we + now write JSON `null` so the column reads back as None.) """ table = self.prisma.db.litellm_memorytable table.rows.append( @@ -699,7 +701,9 @@ class TestMemoryEndpoints: client = _make_client(_user_auth("user-a", "team-a")) with _patch_prisma(self.prisma): resp = client.put("/v1/memory/notes", json={"metadata": None}) - assert resp.status_code == 400 + assert resp.status_code == 200, resp.text + assert resp.json()["metadata"] is None + assert table.rows[0].metadata is None def test_put_memory_omitted_metadata_preserves_field(self): """PUT without a metadata field should NOT touch the stored metadata.""" diff --git a/ui/litellm-dashboard/src/components/MemoryView/MemoryView.tsx b/ui/litellm-dashboard/src/components/MemoryView/MemoryView.tsx index defe455abe..65b435b40d 100644 --- a/ui/litellm-dashboard/src/components/MemoryView/MemoryView.tsx +++ b/ui/litellm-dashboard/src/components/MemoryView/MemoryView.tsx @@ -8,7 +8,6 @@ import { Drawer, Empty, Input, - Modal, Space, Table, Tooltip, @@ -32,6 +31,7 @@ import { updateMemory, } from "../networking"; import { MemoryEditModal } from "./MemoryEditModal"; +import DeleteResourceModal from "../common_components/DeleteResourceModal"; const { Text, Paragraph, Title } = Typography; @@ -65,6 +65,7 @@ export const MemoryView: React.FC = ({ accessToken }) => { const [appliedSearch, setAppliedSearch] = useState(""); const [detailRow, setDetailRow] = useState(null); const [editRow, setEditRow] = useState(null); + const [deleteRow, setDeleteRow] = useState(null); const [isCreateOpen, setIsCreateOpen] = useState(false); const [currentPage, setCurrentPage] = useState(1); @@ -162,18 +163,18 @@ export const MemoryView: React.FC = ({ accessToken }) => { }); const handleDelete = (row: MemoryRow) => { - Modal.confirm({ - title: "Delete memory", - content: ( - - Delete memory key {row.key}? This cannot be undone. - - ), - okText: "Delete", - okType: "danger", - cancelText: "Cancel", - onOk: () => deleteMutation.mutateAsync(row.key).catch(() => {}), - }); + setDeleteRow(row); + }; + + const confirmDelete = async () => { + if (!deleteRow) return; + try { + await deleteMutation.mutateAsync(deleteRow.key); + setDeleteRow(null); + } catch { + // Error toast already surfaced by deleteMutation.onError; + // leave the modal open so the user can retry or cancel. + } }; const handleSave = async ( @@ -537,6 +538,30 @@ export const MemoryView: React.FC = ({ accessToken }) => { }} onSave={handleSave} /> + + {/* Delete confirmation modal */} + { + if (!deleteMutation.isPending) setDeleteRow(null); + }} + onOk={confirmDelete} + confirmLoading={deleteMutation.isPending} + requiredConfirmation={deleteRow?.key} + /> ); };