Fix #1355: [Bug] /product/add returns 200 but memory not stored in Neo4j#1977
Open
Memtensor-AI wants to merge 1 commit into
Open
Fix #1355: [Bug] /product/add returns 200 but memory not stored in Neo4j#1977Memtensor-AI wants to merge 1 commit into
Memtensor-AI wants to merge 1 commit into
Conversation
…1355) When the LLM response could not be parsed as JSON, SimpleStructMemReader._get_llm_response returned a salvage dict whose sole memory item was keyed by "memory_list" (with an underscore). Downstream consumers in _process_chat_data / _process_transfer_chat_data read the item via `resp.get("memory list", [])` (with a space), so the salvaged UserMemory was silently dropped. Consequence: /product/add (sync mode) returned 200 with "Memory added successfully" while writing zero nodes to Neo4j / Qdrant. The typo was introduced by PR #913 (commit d7aaf57) while refactoring the LLM exception path; both sibling readers (multi_modal_struct.py, strategy_struct.py) kept the original "memory list" spelling. Fix: restore the "memory list" key in the fallback and document the contract in an inline comment. Add two regression tests covering the unit-level fallback shape and the reader-stage end-to-end path. Closes #1355
Memtensor-AI
requested review from
CarltonXiang,
MatthewZhuang,
World-controller and
syzsunshine219
Collaborator
Author
✅ Automated Test Results: PASSEDNo 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: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #1355: /product/add returned HTTP 200 with "Memory added successfully" but wrote zero nodes to Neo4j / Qdrant when the LLM-extraction fallback path fired.
Root cause:
SimpleStructMemReader._get_llm_responseinsrc/memos/mem_reader/simple_struct.pyreturned a salvage dict whose memory item was keyed by"memory_list"(underscore), while downstream consumers_process_chat_dataand_process_transfer_chat_datareadresp.get("memory list", [])(single space). The salvagedUserMemoryitem was silently dropped whenever the LLM output failed strict JSON parsing (very common with Kimi-K2).mem_groupended up empty,text_mem.add([])did nothing, the scheduler then emitted the "No add/update items prepared" warning the user reported. The typo was introduced by PR #913 (commit d7aaf57) while refactoring the LLM exception path; both sibling readers (multi_modal_struct.py,strategy_struct.py) kept the correct"memory list"spelling, confirming this is a localised regression.Note: the bug report initially pointed at
mem_scheduler.task_schedule_modules.handlers.add_handler.log_add_messages. That handler is downstream of storage and only writes audit-log events — patching it cannot affect Neo4j persistence. The fix is one level upstream, in the mem-reader fallback. The "No add/update items prepared" warning is a symptom, not a cause.Change: one-character key correction in
simple_struct.pyplus an inline comment pinning the contract. Two TDD regression tests added intests/mem_reader/test_simple_structure.py(test_get_llm_response_fallback_key_matches_consumer,test_process_chat_data_fine_yields_node_when_llm_unparseable). Both tests fail on the buggy code and pass after the fix. Verification: 93 passed / 1 skipped acrosstests/mem_reader/+tests/mem_scheduler/; ruff lint and format both clean. The 2 pre-existingtest_coarse_memory_type.pyfailures require the optionalmarkitdownextra and are unrelated (confirmed viagit stash). No public API or OpenAPI changes; no new dependencies.Related Issue (Required): Fixes #1355
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Automated tests are pending.
Checklist
@MatthewZhuang, @CarltonXiang, @syzsunshine219, @World-controller please review this PR.
Reviewer Checklist