Repository navigation
fix(memory): decode JSON-encoded internal_info metadata - #2517
Conversation
🤖 Open Code ReviewTarget: PR #2517 ✅ OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s). Generated by cloud-assistant via Open Code Review. |
|
Thanks for the review. A clarification on this finding: the two exception paths are mutually exclusive.
I checked this directly with Python 3.12 / Pydantic 2.12.4: inspecting Both paths are covered by the regression tests, and the full textual-memory test directory passed (80 tests) in an isolated backend container. The suggested Reproducible control-flow evidenceI also traced the actual validator, not a reimplementation, at commit import sys
from pydantic import ValidationError
from memos.memories.textual.item import TextualMemoryMetadata
validator = TextualMemoryMetadata._decode_internal_info.__func__
for raw in ("{", "[]"):
executed = []
def trace(frame, event, arg):
if frame.f_code is validator.__code__ and event == "line":
executed.append(frame.f_lineno)
return trace
sys.settrace(trace)
try:
TextualMemoryMetadata(internal_info=raw)
except ValidationError as exc:
sys.settrace(None)
original = exc.errors()[0]["ctx"]["error"]
print(repr(raw), type(original).__name__, executed)
finally:
sys.settrace(None)Actual output on Python 3.12 / Pydantic 2.12.4: Line 167 is This directly disproves the stated reason that the next |
✅ Automated Test Results: PASSEDAll tests passed (20/20 executed). memos_python_core/changed-repo-python: 20/20. Duration: 10s Branch: |
Description
Neo4j-backed recall can return an empty result after
TextualMemoryItemrejects a JSON-encodedmetadata.internal_info. The native ingest reader can also skip the same nodes when constructingTreeNodeTextualMemoryMetadata.The chunking path creates
internal_infoas a dictionary. Neo4j serializes dictionary properties as JSON, but its read conversion does not restore this field before Pydantic validation. A short conversation withinternal_info=Nonedoes not exercise the failure.Add a before-validator on
TextualMemoryMetadata.internal_info, shared by the search and reader metadata models. Decode string values once and require a JSON object. Existing dictionaries andNonekeep their behavior; malformed JSON, encoded arrays/scalars/null, and double-encoded strings remain validation errors. No storage migration, write-path change, request/response route change, new dependency, LLM request, or retrieval-policy change is required.Related issue: #2288. Earlier PR #2289 was closed without merging; the failure remains reproducible on current
main(a7367d0). This change deliberately retains validation errors for malformed values instead of silently replacing them withNone.Type of change
How Has This Been Tested?
Before the fix: new regression file has 5 failed, 15 passed, reproducing the JSON-string validation error.
After the fix: new regression file has 20 passed.
In an isolated Python 3.12 / Pydantic 2.12.4 backend container, with networking disabled, no credentials and no business volumes:
80 passed. Coverage includes all four metadata subclasses, search
from_dict, direct reader metadata construction, unchanged source/session/content fields, repeated storage JSON round trips, existing dictionary/None values, and rejection of malformed or non-object JSON.The image does not include pytest-asyncio, producing an existing unknown
asyncio_modeconfiguration warning; one existing naive-memory test also reports a Pydantic serialization warning.make format: passed (All checks passed!;631 files left unchanged), using the repository-pinned Ruff 0.11.8.pre-commit run --files src/memos/memories/textual/item.py tests/memories/textual/test_item.py: passed (all applicable hooks).Checklist
Maintainer review and upstream CI are pending; the full repository suite was not run locally.