[RerankerGuo]

Summary

parse_json_result silently returned {} on every parse failure, so a malformed LLM extraction was reported as an empty (but successful) /product/add response. The HTTP layer returned {"code": 200, "data": []} for an extraction that actually failed, hiding the wasted tokens and never alerting operators that a request had silently dropped.

Root cause

src/memos/mem_reader/utils.py::parse_json_result had three silent return paths:

i = s.find("{")
if i == -1:
    return {}                                  # ← empty/no JSON object

# ... cheap-close attempt
return {}                                      # ← unrecoverable parse error

Both branches are reached when the LLM emits invalid JSON or non-JSON chatter. Downstream, every /product/add caller (multi_modal_struct, simple_struct, strategy_struct, file_content_parser) checks the empty {} against truthiness and either short-circuits to a "no items" log line or falls into an unintended code path — never raising, never alerting.

Fix

• Add LLMResponseParseError(ParserError) to memos.exceptions so callers can distinguish "no memories produced" from "could not parse the response".
• parse_json_result now raises on empty input, on missing JSON object, and on every unrecoverable JSONDecodeError. The chained __cause__ preserves the underlying decode error for debugging.
• Successful parses (including a literal {}) still return the parsed dict.
• The cheap-close and backslash-escape repair attempts remain; they only succeed when valid JSON comes out.

Caller audit

Every in-tree caller of parse_json_result already wraps the call in try/except Exception, so the new exception is caught and routed to the existing fallback instead of the silent-{} path:

caller	site	behavior with new raise
mem_reader/multi_modal_struct.py	:504	synthetic-memory-list fallback (was: silent {} → "no items prepared" → 200 OK with empty data)
mem_reader/multi_modal_struct.py	:600	returns None from merge
mem_reader/simple_struct.py	:118 (and _safe_parse at :269)	returns None, downstream fallback fires
mem_reader/strategy_struct.py	:55	synthetic-memory-list fallback
mem_reader/read_multi_modal/file_content_parser.py	:94	returns {}
memories/textual/tree_text_memory/retrieve/task_goal_parser.py	:117	existing retry loop raises ValueError after N attempts
memories/textual/tree_text_memory/retrieve/searcher.py	:1384	returns [query]

textual/general.py and textual/tree_text_memory/.../retrieve_utils.py define their own parse_json_result with a different body — those are not affected by this change.

Tests

• New tests/mem_reader/test_parse_json_result_strict.py — 10 red/green cases:
  • Success: parsed object, literal {}, markdown-fence stripping
  • Failure (raises): trailing-comma reproducer from the issue, no JSON object, unclosed fence, empty input, None input, prose-only "I cannot output JSON" symptom, exception-chain preservation
• Updated tests/mem_reader/test_simple_structure.py::test_parse_json_result_failure to expect the new raise
• Updated the docstring on tests/mem_reader/test_simple_struct_fallback.py::test_process_chat_data_fine_yields_node_when_llm_unparseable to describe the new exception path; the test still passes because _safe_parse catches the new exception and returns None, so the symptom-level chat fallback (which is the actual user-facing contract for fix: /product/add returns 200 success but stores no memory (No add/update items prepared) #1493) continues to emit a salvageable UserMemory item.

Verification

PYTHONPATH=src pytest tests/mem_reader/test_parse_json_result_strict.py -v
============================== 10 passed in 0.12s ==============================

ruff format --check  # all touched files pass
ruff check           # only pre-existing BLE001 errors remain; this change introduces 0 new findings

The full existing test_simple_structure.py suite pulls in the heavy LLM/embedder stack and was not exercised end-to-end here, but every individual test case the PR touches was verified against the new behavior in isolation.

Out of scope

• Adds no new dependencies.
• Does not modify the public API surface of memos or the OpenAPI contract.
• Does not touch DB schemas, migrations, or graph models.
• Does not touch Makefile / pyproject.toml / .pre-commit-config.yaml / .github/workflows/.

Fixes #2456.

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2487
Task: b17b2259d00b26a4
Base: dev-v2.0.36
Head: fix/issue-2456-malformed-json-detection

🔍 OpenCodeReview found 2 issue(s) in this PR.

⚠️ 2 warning(s) occurred during review.

---

1. src/memos/mem_reader/utils.py (L96-L98)

Raising here changes runtime behaviour at several existing call sites that wrap the call in a broad except Exception and then synthesise a fallback, so "unparseable" is silently converted into fabricated data rather than surfaced:

• multi_modal_struct.py:502-517 (_get_llm_response) and strategy_struct.py:53-68 build a {"memory list": [{"value": mem_str, ...}]} entry out of the raw prompt input.
• file_content_parser.py:92-97 returns {}, i.e. the same value the old code produced for every parse failure.

Previously a malformed extraction produced no memories (callers hit resp.get("memory list", []) -> empty); now those paths will emit a single garbage memory made from the whole conversation/document chunk. Please confirm the fallback behaviour at those sites is what you want for a parse failure (as opposed to an LLM transport failure), or narrow that fallback so issue #2456 is actually distinguishable end-to-end rather than caught by the same blanket handler.

---

2. src/memos/mem_reader/utils.py (L87-L90)

The old code let the Invalid \escape recovery path re-raise a raw json.JSONDecodeError; wrapping it in LLMResponseParseError removes that. memories/textual/general.py's extract() is decorated with retry_if_exception_type(json.JSONDecodeError), so that escape-corruption case will no longer be retried there. Either include LLMResponseParseError in that retry predicate or confirm the retry policy is intentionally narrowed.

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

❌ Automated Test Results: FAILED

Auto-fix retry 1/2 triggered.

Failed tests:

• collection
• collection
• collection

Error details

Tests failed. Failed cases: collection, collection, collection

Branch: fix/issue-2456-malformed-json-detection

[RerankerGuo]

Fixed in 05981a4 — the AutoTest collection failures and the build-matrix failures were both caused by this PR's new test module, not by the repo.

tests/mem_reader/test_parse_json_result_strict.py built a stub package tree and injected it into sys.modules at import time (_load_memos_submodules()), with the src/memos/... paths resolved relative to the current working directory. Two consequences:

• the stub memos.mem_reader.__path__ pointed at the CWD-relative src/memos/mem_reader, so collecting any module that imports memos.mem_reader.* failed at collection time outside the repo root — the three collection failures in the AutoTest report;
• the stub memos package replaced the real one for the rest of the pytest session, so every later test that imported the real package failed with AttributeError: module 'memos' has no attribute 'api' / 'graph_dbs' / 'memories' / 'llms' / 'mem_os' — 54 errors and 92 failures in build (ubuntu-latest, 3.10).

The module now imports the real modules directly (from memos.exceptions import LLMResponseParseError, from memos.mem_reader.utils import parse_json_result), the same way the sibling tests in tests/mem_reader/ do; pyproject.toml already sets pythonpath = "src".

This also covers the three OpenCodeReview findings on this file: the CWD-relative path loading, the permanent sys.modules stubs, and the weak or "Failed" in str(...) assertion, which is now assert isinstance(exc_info.value.__cause__, json.JSONDecodeError).

Production behaviour of parse_json_result is unchanged — this is a test-side fix only.

[Memtensor-AI]

✅ Automated Test Results: PASSED

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

Branch: fix/issue-2456-malformed-json-detection