Fix #1493: fix: /product/add returns 200 success but stores no memory (No add/update items

[Memtensor-AI]

Description

Fixed /product/add silently dropping memories under sync mode (#1493).

Root cause: SimpleStructMemReader._get_llm_response in src/memos/mem_reader/simple_struct.py returned its salvage-fallback dict under the key "memory_list" (with underscore), while every downstream consumer in the same file (_process_chat_data line 397, _process_transfer_chat_data line 434) reads "memory list" (with space). The fallback was therefore dead code: when the LLM returned non-strict JSON for short inputs like "I like strawberry." (a common Kimi-K2 failure mode), _safe_parse returned {}, the fallback fired but emitted the wrong key, _process_chat_data yielded zero items, text_mem.add([]) wrote nothing to Neo4j/Qdrant, and /product/add still returned 200 + data: [] while the scheduler's downstream log_add_messages audit logged "No add/update items prepared". This is a duplicate of #1355 in symptom and root cause; the #1355 fix was merged on a different branch and had not propagated to dev-20260624-v2.0.22.

Fix: one-character key rename from "memory_list" to "memory list" in the fallback branch (lines 288-299 of simple_struct.py), bringing the dead fallback back into alignment with the two consumers and the two sibling readers (multi_modal_struct.py:500, strategy_struct.py:67). Added 2 unit-level regression tests in tests/mem_reader/test_simple_struct_fallback.py that (a) pin the fallback dict key contract and (b) prove end-to-end that _process_chat_data(mode="fine") emits at least one TextualMemoryItem when the LLM output is unparseable. Both tests fail red on the buggy code (verified by running them pre-fix) and pass green after the one-character change.

Verification: python3 -m pytest tests/mem_reader/test_simple_struct_fallback.py -v → 2 passed; python3 -m pytest tests/mem_reader/ -q → 44 passed + 2 pre-existing markitdown-extras failures (confirmed unrelated via git stash/rerun); python3 -m pytest tests/mem_scheduler/ -q → 61 passed + 1 skipped; ruff check and ruff format --check both clean on the two touched files. Committed b241194d and pushed bugfix/autodev-1493 to origin. opsp artifacts (task.md, proposal.md, spec.md, design.md, verification-report.md) are written under .ai-tasks/ and openspec/changes/ (excluded from the working branch via .git/info/exclude) and were synced to the sibling memos-autodev-specs repo at commit 6e2e41b on main.

Related Issue (Required): Fixes #1493

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (does not change functionality, e.g. code style improvements, linting)
• Documentation update

How Has This Been Tested?

Automated tests are pending.

• Unit Test
• Test Script Or Test Steps (please provide)
• Pipeline Automated API Test (please provide)

Checklist

• I have performed a self-review of my own code
• I have commented my code in hard-to-understand areas
• I have added tests that prove my fix is effective or that my feature works
• I have created related documentation issue/PR in MemOS-Docs (if applicable)
• I have linked the issue to this PR (if applicable)
• I have mentioned the person who will review this PR

@MatthewZhuang, @CarltonXiang, @syzsunshine219, @World-controller please review this PR.

Reviewer Checklist

• closes fix: /product/add returns 200 success but stores no memory (No add/update items prepared) #1493
• Made sure Checks passed
• Tests have been provided

[Memtensor-AI]

✅ Automated Test Results: PASSED

No applicable test scope for the changed files — automated tests skipped. Changed paths do not map to any configured scope (env.yaml source_mapping). Manual review recommended.

Branch: bugfix/autodev-1493

[Memtensor-AI]

⚠️ Automated Test Results: NO TEST SCOPE

Automated tests were not run because the changed files do not map to an executable test scope.

Details: No tests were executed (scopes matched: memos_python_core). Check scope source_mapping / execution commands in memos-test env.yaml. No executable test scope maps to the changed files. Automated tests were not run; add env.yaml source_mapping + execution for this path family, then rerun. Changed files: src/memos/mem_reader/simple_struct.py, tests/mem_reader/test_simple_struct_fallback.py
Manual review or env.yaml source_mapping/execution coverage is required before merge.

Branch: bugfix/autodev-1493

[Memtensor-AI]

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: All 6 AI-generated tests failed with ModuleNotFoundError: No module named 'pydantic'. The test environment is missing a core dependency required to import the memos package. AI-generated tests: 0/132 passed, 6 failed (branch test/auto-gen-6d548ec04e483fce-20260701). Scopes with no tests executed: memos_python_core.
Branch: bugfix/autodev-1493

[Memtensor-AI]

⚠️ Automated Test Results: NO TEST SCOPE

Automated tests were not run because the changed files do not map to an executable test scope.

Details: No tests were executed (scopes matched: memos_python_core). Check scope source_mapping / execution commands in memos-test env.yaml. No executable test scope maps to the changed files. Automated tests were not run; add env.yaml source_mapping + execution for this path family, then rerun. Changed files: src/memos/mem_reader/simple_struct.py, tests/mem_reader/test_simple_struct_fallback.py
Manual review or env.yaml source_mapping/execution coverage is required before merge.

Branch: bugfix/autodev-1493

[Memtensor-AI]

⚠️ Automated Test Results: NO TEST SCOPE

Automated tests were not run because the changed files do not map to an executable test scope.

Details: No tests were executed (scopes matched: memos_python_core). Check scope source_mapping / execution commands in memos-test env.yaml. No executable test scope maps to the changed files. Automated tests were not run; add env.yaml source_mapping + execution for this path family, then rerun. Changed files: src/memos/mem_reader/simple_struct.py, tests/mem_reader/test_simple_struct_fallback.py
Manual review or env.yaml source_mapping/execution coverage is required before merge.

Branch: bugfix/autodev-1493

[Memtensor-AI]

❌ Automated Test Results: FAILED

Auto-fix retry 1/2 triggered.

Failed tests:

• test_reader_yields_at_least_one_node_on_llm_failure[wrong_top_level_key-{"totally_unrelated": "value"}]

Error details

The fallback dict in `_get_llm_response` uses the key `"memory list"` (with space) instead of `"memory_list"` (underscore), causing downstream consumers to find no memories and return an empty list — reproducing the #1493 regression. AI-generated tests: 70/71 passed, 1 failed (branch test/auto-gen-6d548ec04e483fce-20260701190144). Scopes with no tests executed: memos_python_core.

Branch: bugfix/autodev-1493

[Memtensor-AI]

⚠️ The previous ❌ FAILED is a false positive — do NOT run the auto-fix

After verifying against the actual source on this branch, the conclusion is:

The correct key is "memory list" (with a space), and this PR's fix is correct:

• Both consumers read the space form: _process_chat_data (simple_struct.py:397) and _process_transfer_chat_data (:434) call resp.get("memory list", ...).
• The fixed fallback (:290) also uses "memory list" (space), so both sides are now aligned.
• This PR's own regression test test_get_llm_response_fallback_key_matches_consumer asserts exactly this: "Fallback dict must use the consumer-side key 'memory list' (with space). Using 'memory_list' (underscore) leaves the fallback unreachable."

The AI-generated test in the previous FAILED comment (test_reader_yields_at_least_one_node_on_llm_failure) has the key inverted — it expects "memory_list" (underscore) and flags the correct space form as the bug. This is a fail_new_test_defect (a defect in the generated test itself), the same class as #2003 — not a real code bug.

Root cause

That run happened on the pre-fix test-engine, where AI-generated tests were still gating. That behavior has now been fixed: auto-generated tests are advisory / non-gating.

✅ Verified on the fixed engine

A fresh run on the deployed fix returned:

• verdict: pass — gating scope memos_python_core: 2/2 passed, no failed scopes.
• AI-generated tests: 49/78 passed, 1 failed — advisory / non-gating (do NOT affect the PR verdict).
• No auto-fix triggered (shouldRetryWithFix: false).

The single remaining generated-test failure is the inverted-key defect described above, now correctly treated as advisory.

Conclusion: the code fix is correct. The earlier FAILED gate can be safely overridden; proceed through the normal review / merge flow.

Verified by cloud-assistant.

[Memtensor-AI]

🤖 Open Code Review

Target: PR #1989
Task: manual-ocr:MemTensor/MemOS#1989
Base: dev-v2.0.22
Head: bugfix/autodev-1493

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

---

1. tests/mem_reader/test_simple_struct_fallback.py (L47)

After __init__ runs with config.general_llm = None, the constructor sets self.general_llm = self.llm (the factory-returned mock). The test then replaces self.reader.llm with a new MagicMock(), but self.reader.general_llm still points to the old factory mock. The two attributes are now inconsistent. While _get_llm_response only calls self.llm (via _safe_generate), any code path that uses self.general_llm would exercise the stale mock instead of the newly configured one. To keep both in sync, assign self.reader.general_llm after self.reader.llm is assigned, or replace both in one step.

💡 Suggested Change

Before:

        self.reader.general_llm = self.reader.llm

After:

        self.reader.llm = MagicMock()
        self.reader.general_llm = self.reader.llm  # keep both in sync

---

2. tests/mem_reader/test_simple_struct_fallback.py (L95-L104)

The assertion assertGreaterEqual(len(result), 1) is too permissive for a single-message fallback. If a future bug accidentally duplicates the fallback item or appends it multiple times, this assertion still passes. With one user message and the fallback producing exactly one item, use assertEqual(len(result), 1) to catch both under- and over-production.

💡 Suggested Change

Before:

        result = self.reader._process_chat_data(scene_data_info, info, mode="fine")

        self.assertIsInstance(result, list)
        self.assertGreaterEqual(
            len(result),
            1,
            "Fallback must produce at least one TextualMemoryItem when LLM "
            "output cannot be parsed; otherwise /product/add stores nothing "
            "while still returning 200.",
        )

After:

        result = self.reader._process_chat_data(scene_data_info, info, mode="fine")

        self.assertIsInstance(result, list)
        self.assertEqual(
            len(result),
            1,
            "Fallback must produce exactly one TextualMemoryItem for a single "
            "user message when LLM output cannot be parsed.",
        )

---

3. tests/mem_reader/test_simple_struct_fallback.py (L92-L95)

The module-level docstring explicitly states that _process_transfer_chat_data (line 434 in simple_struct.py) is also affected by the same fallback key bug — it also reads response_json.get("memory list", []). However, there is no test exercising that code path. A regression in _process_transfer_chat_data would go undetected. Consider adding a test case that calls _process_transfer_chat_data with an unparseable LLM response.

Generated by cloud-assistant via Open Code Review.