Skip to content

fix(memory): decode JSON-encoded internal_info metadata - #2517

Merged
endxxxx merged 2 commits into
MemTensor:dev-v2.0.36from
linsheng-li:fix/decode-internal-info-json
Oct 24, 2026
Merged

endxxxx merged 2 commits into
MemTensor:dev-v2.0.36from
linsheng-li:fix/decode-internal-info-json

Conversation

@linsheng-li

Copy link
Copy Markdown

Description

Neo4j-backed recall can return an empty result after TextualMemoryItem rejects a JSON-encoded metadata.internal_info. The native ingest reader can also skip the same nodes when constructing TreeNodeTextualMemoryMetadata.

The chunking path creates internal_info as a dictionary. Neo4j serializes dictionary properties as JSON, but its read conversion does not restore this field before Pydantic validation. A short conversation with internal_info=None does 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 and None keep 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 with None.

Type of change

  • Bug fix

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:

    PYTHONPATH=src python -B -m pytest -p no:cacheprovider -o filterwarnings= -o addopts= -q tests/memories/textual/

    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_mode configuration 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

  • Self-reviewed the diff; only the metadata validator and its regression tests change.
  • Added an explanatory comment at the storage/model boundary.
  • Added tests that fail before the fix and pass afterward.
  • Linked the existing issue. No public API or documentation change is required.

Maintainer review and upstream CI are pending; the full repository suite was not run locally.

@Memtensor-AI Memtensor-AI added area:memory 记忆存储、检索、更新、召回逻辑 status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Oct 20, 2026
@Memtensor-AI

Memtensor-AI commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

🤖 Open Code Review

Target: PR #2517
Task: 8b85a1b9d5af959e
Base: dev-v2.0.36
Head: fix/decode-internal-info-json

✅ OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s).

Generated by cloud-assistant via Open Code Review.

@linsheng-li

linsheng-li commented Oct 20, 2026 •

Copy link
Copy Markdown
Author

Thanks for the review. A clarification on this finding: the two exception paths are mutually exclusive.

  • For malformed JSON such as {, json.loads(value) raises JSONDecodeError immediately. Execution never reaches the following if or its raise ValueError(...). The validator does not catch or replace the parsing exception; Pydantic wraps it as a ValidationError.
  • For valid JSON with a non-object result, such as [], parsing succeeds. The explicit ValueError then rejects that result. There is no preceding JSONDecodeError to chain in this case.

I checked this directly with Python 3.12 / Pydantic 2.12.4: inspecting exc.errors()[0]["ctx"]["error"] returns a JSONDecodeError for { and the explicit ValueError for [].

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 try/except ... raise ... from err would provide a different error message, but it is not needed to prevent the subsequent type check from overwriting the parsing exception. I have therefore kept the implementation unchanged.

Reproducible control-flow evidence

I also traced the actual validator, not a reimplementation, at commit fe70153. Here is a minimal reproducer:

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:

'{' JSONDecodeError [166, 167]
'[]' ValueError [166, 167, 168, 169]

Line 167 is json.loads; line 169 is the explicit raise ValueError. For malformed JSON, neither line 168 nor line 169 executes, and the original JSONDecodeError is still present in Pydantic's error context. For the valid non-object JSON case, line 169 executes but no JSON parsing exception was raised.

This directly disproves the stated reason that the next ValueError discards a preceding JSONDecodeError: these two errors cannot occur on the same execution path in this validator. A request for a uniform error message would be a separate style preference.

@endxxxx
endxxxx changed the base branch from main to dev-v2.0.36 October 24, 2026 13:56
@Memtensor-AI

Copy link
Copy Markdown
Collaborator

✅ Automated Test Results: PASSED

All tests passed (20/20 executed). memos_python_core/changed-repo-python: 20/20. Duration: 10s

Branch: fix/decode-internal-info-json

@Memtensor-AI Memtensor-AI added status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发 and removed status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Oct 24, 2026
@endxxxx
endxxxx merged commit 4bfddc8 into MemTensor:dev-v2.0.36 Oct 24, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:memory 记忆存储、检索、更新、召回逻辑 status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants