Litellm memory improvements v2 - #26541
Conversation
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
`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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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
|
|
Greptile SummaryThis PR refines the memory PUT endpoint's handling of Confidence Score: 4/5Safe to merge; all findings are P2 quality/consistency notes, no runtime-breaking issues. Only P2 findings: a SQL NULL vs jsonb litellm/proxy/memory/memory_endpoints.py — review the create-path null handling around line 503 relative to the update-path change.
|
| Filename | Overview |
|---|---|
| litellm/proxy/memory/memory_endpoints.py | Changes metadata: null handling in PUT from a no-op to an active write (JSON "null" → jsonb 'null'), fixing silent discard; minor SQL NULL vs jsonb null asymmetry exists between create and update paths within the same PUT handler. |
| tests/test_litellm/proxy/memory/test_memory_endpoints.py | Tests updated to reflect new explicit-null-clears semantic; renames and assertion changes are intentional, not regression-hiding; small gap in null-metadata-only test (value field not re-asserted). |
| ui/litellm-dashboard/src/components/MemoryView/MemoryView.tsx | Replaces inline Modal.confirm delete flow with the shared DeleteResourceModal component; adds deleteRow state, confirmDelete handler, and a requiredConfirmation keyed on deleteRow.key for safer UX. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["PUT /v1/memory/{key}"] --> B{Row exists?}
B -- No --> C["Create path\nif metadata is not None → JSON-encode\notherwise omit → SQL NULL"]
B -- Yes --> D["Update path\nif 'metadata' in model_fields_set\n→ json.dumps(metadata)\n→ NULL → 'null' string → jsonb null"]
D --> E{data dict empty?}
C --> F["prisma.create(data)"]
E -- Yes --> G["HTTP 400"]
E -- No --> H["prisma.update(where=memory_id, data=data)"]
H --> I["_row_to_model → response"]
F --> I
Comments Outside Diff (2)
-
litellm/proxy/memory/memory_endpoints.py, line 492-504 (link)SQL NULL vs jsonb
nullinconsistency in PUT create vs update pathsWhen a PUT request creates a new row (
existing is None), the create-path on line 503 still guards withif body.metadata is not None:, meaning an explicit{"value": "x", "metadata": null}on a new key stores a true SQL NULL. But if the same key already exists, the update-path (line 452) writes the JSON literal"null"→jsonb 'null'. Both deserialize to PythonNoneon reads, but they differ in SQL —WHERE metadata IS NULLmatches SQL NULL but notjsonb 'null', andWHERE metadata = 'null'::jsonbmatches the opposite. If any current or future query filters on nullness of themetadatacolumn, rows created via PUT withmetadata: nulland rows updated via PUT withmetadata: nullwill behave differently. -
tests/test_litellm/proxy/memory/test_memory_endpoints.py, line 700-706 (link)Missing assertion that
valueis preserved after null-metadata-only PUTtest_put_memory_null_metadata_alone_clears_fieldsends{"metadata": null}without avalue, but only asserts thatmetadatabecomesNone. It doesn't verify that the storedvaluefield (originally"v") is still"v"after the update. Addingassert resp.json()["value"] == "v"andassert table.rows[0].value == "v"would guard against a regression where a partial-update accidentally clears unincluded fields.
Reviews (1): Last reviewed commit: "Merge branch 'litellm_internal_staging' ..." | Re-trigger Greptile
4ed3e71
into
litellm_internal_staging
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com>
* 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. * 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. * 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`). * 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 ---------
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes