Fix #2329: add_memory 返回成功但立即查询同一 memory_id 404 - #2330
Closed
Memtensor-AI wants to merge 2 commits into
Closed
Conversation
…event 404 In async mode, add_memory writes fast-mode nodes and returns their IDs to the caller. The scheduler's MemReadMessageHandler would then create refined nodes with brand-new UUIDs and hard-delete the original IDs, causing any get_memory(original_id) call to return 404. Fix: when fine_transfer_simple_mem returns exactly one enhanced memory per input item (1:1 mapping), reuse the original node ID on the enhanced item. Neo4j uses MERGE semantics so the node is updated in-place and the caller's handle stays valid. The reused IDs are excluded from the delete list so the updated node is never immediately destroyed. The existing delete-and-replace behaviour is preserved for 1:N expansions and zero-output cases. Fixes MemTensor#2329
Memtensor-AI
added
ai:generated
Collaborator
Author
🤖 Open Code ReviewTarget: PR #2330 ✅ OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s). Generated by cloud-assistant via Open Code Review. |
Collaborator
Author
🔧 Open Code Review requested Agent fixOpen Code Review found 2 issue(s). I have resumed the development Agent to fix them.
The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed. |
- Assert text_mem.add was called before checking delete args so the happy path is verified (avoids a silent no-op when the handler exits early with empty memory_items). - Simplify redundant `if processed_memories and len(...) > 0:` to `if processed_memories:` — the length check was dead code.
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
3 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
Fixed the add_memory → get_memory 404 race condition in async mode.
Root cause: In async mode,
add_memorywrites fast-mode LongTermMemory nodes and immediately returns their IDs (IDs_A) to the caller. The scheduler'sMemReadMessageHandler._process_memories_with_readerwould then callfine_transfer_simple_memwhich produces brand-new UUID nodes (IDs_B), write those, and hard-delete IDs_A. Anyget_memory(IDs_A)call issued after the scheduler ran would return 404 because the original nodes no longer existed.Fix applied in
src/memos/mem_scheduler/task_schedule_modules/handlers/mem_read_handler.py: whenfine_transfer_simple_memproduces exactly one enhanced memory per input item (the common 1:1 case), the enhanced item's.idis overwritten with the original node's ID before writing. Neo4j usesMERGEsemantics, so the graph node is updated in-place and the caller's handle remains permanently valid. Reused IDs are then excluded from the subsequent delete list so the freshly-updated node is never destroyed. The existing delete-and-replace behavior is fully preserved for 1:N expansions and zero-output cases.Tests: 4 new unit tests added in
tests/mem_scheduler/test_mem_read_handler_id_stability.pycovering the 1:1 reuse path (ID assigned, ID not deleted), the 1:N expansion fallback (old ID still deleted), and the zero-output fallback. All 50 scheduler tests pass. Ruff clean.Related Issue (Required): Fixes #2329
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