From de0190a3bf8fca3a13f22c392f5819615ea15f7c Mon Sep 17 00:00:00 2001 From: RerankerGuo Date: Tue, 13 Oct 2026 16:39:55 +0800 Subject: [PATCH] fix(mem_reader): stop duplicating parent sources on every split chunk (#2453) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MultiModalStructMemReader._split_large_memory_item passed `sources=item.metadata.sources or []` to _make_memory_item for every chunk, so each chunk item carried the full list of source messages from the parent. With N chunks and a parent carrying N original SourceMessages, every chunk ended up with N byte-identical full-text SourceMessage objects — a multiplicative duplication of memory and graph size that scaled with the square of how chunked a message was. Empirically from the issue: one 1,456-character message split into 4 chunks produced 15 memory items, all with sources length 4 and all 4 elements byte-identical (length [1456, 1456, 1456, 1456], identical MD5). 27.8% of memories in a sampled 320-memory production instance showed the same bloat. Chunks are derived fragments, not original messages, so they should not inherit the parent's sources list. Provenance is already preserved through `internal_info.chunk_index / chunk_total / ingest_batch_id` (set immediately after _make_memory_item), which gives downstream consumers the same reconstruction ability without the duplication. The fix drops the sources= argument from the _make_memory_item call in _split_large_memory_item. _make_memory_item's default is sources=None, which is normalised to [] inside TextualMemoryMetadata, so chunks now carry no sources (the same as if they had been created fresh from the raw chunk text). - New tests/mem_reader/test_split_sources_no_duplication.py with three red/green cases: chunks must not carry parent sources (red against the unpatched code); chunks must still carry internal_info provenance; empty parent sources still produce empty chunk sources. - The parent's sources are preserved on the parent (no change there), so downstream aggregation paths that read the parent's sources continue to work. Fixes #2453. --- src/memos/mem_reader/multi_modal_struct.py | 12 +- .../test_split_sources_no_duplication.py | 125 ++++++++++++++++++ 2 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 tests/mem_reader/test_split_sources_no_duplication.py diff --git a/src/memos/mem_reader/multi_modal_struct.py b/src/memos/mem_reader/multi_modal_struct.py index eb226c27d..4213ad0d9 100644 --- a/src/memos/mem_reader/multi_modal_struct.py +++ b/src/memos/mem_reader/multi_modal_struct.py @@ -159,14 +159,22 @@ def _create_chunk_item(chunk_idx: int, chunk): "chunk_index": chunk_idx, "chunk_total": chunk_total, } - # Create a new memory item for each chunk, preserving original metadata + # Create a new memory item for each chunk, preserving original + # metadata. We drop the parent's ``sources`` list deliberately: + # the parent already carries those source messages, and chunks + # are *derived* fragments — not original messages — so they + # should not multiply the full source list (issue #2453, which + # produced N byte-identical ``SourceMessage`` copies per chunk + # when the parent had N sources and the item was split into N + # chunks). Provenance of each chunk is preserved through + # ``internal_info.chunk_index / chunk_total / ingest_batch_id`` + # below. split_item = self._make_memory_item( value=chunk_text, info=chunk_info, memory_type=item.metadata.memory_type, tags=item.metadata.tags or [], key=item.metadata.key, - sources=item.metadata.sources or [], background=item.metadata.background or "", need_embed=False, ) diff --git a/tests/mem_reader/test_split_sources_no_duplication.py b/tests/mem_reader/test_split_sources_no_duplication.py new file mode 100644 index 000000000..bbd59b014 --- /dev/null +++ b/tests/mem_reader/test_split_sources_no_duplication.py @@ -0,0 +1,125 @@ +"""Regression tests for issue #2453. + +``MultiModalStructMemReader._split_large_memory_item`` previously passed +``sources=item.metadata.sources or []`` to ``_make_memory_item`` for every +chunk, so each chunk item carried the full list of source messages from the +parent. With N chunks and a parent carrying N original sources, every chunk +ended up with N byte-identical full-text ``SourceMessage`` objects — a +multiplicative duplication of memory and graph size that scaled with the +square of how chunked a message was. + +These tests pin the new contract: each split chunk item's ``sources`` list +must not contain the parent's sources. Provenance of split chunks is +already carried on ``metadata.internal_info`` via ``chunk_index`` / +``chunk_total`` / ``ingest_batch_id`` (see +``MultiModalStructMemReader._split_large_memory_item``). +""" + +from __future__ import annotations + +import unittest + +from unittest.mock import MagicMock + +from memos.mem_reader.multi_modal_struct import MultiModalStructMemReader +from memos.memories.textual.item import ( + SourceMessage, + TextualMemoryItem, + TreeNodeTextualMemoryMetadata, +) + + +def _build_parent_item(sources: list[SourceMessage]) -> TextualMemoryItem: + return TextualMemoryItem( + memory="long " * 200, # > parent's token threshold + metadata=TreeNodeTextualMemoryMetadata( + user_id="user-1", + session_id="session-1", + memory_type="LongTermMemory", + sources=sources, + info={}, + internal_info={}, + ), + ) + + +def _build_reader(chunks: list[str]) -> MultiModalStructMemReader: + """Construct a minimal MultiModalStructMemReader with a mock chunker. + + The real constructor pulls in the full LLM/embedder stack; for this + targeted test we bypass __init__ and attach only the attributes that + ``_split_large_memory_item`` touches. + """ + reader = MultiModalStructMemReader.__new__(MultiModalStructMemReader) + + class _Chunk: + def __init__(self, text): + self.text = text + + reader.chunker = MagicMock() + reader.chunker.chunk = MagicMock(return_value=[_Chunk(t) for t in chunks]) + # Force the splitter to enter the chunking branch. + reader._count_tokens = MagicMock(return_value=10_000) + # The inherited ``_make_memory_item`` calls the embedder only when + # need_embed=True; ``_split_large_memory_item`` already sets it False, + # so a no-op embedder is sufficient. + reader.embedder = MagicMock() + reader.embedder.embed = MagicMock(return_value=[[0.0]]) + return reader + + +class TestSplitSourcesNotDuplicated(unittest.TestCase): + """Pins issue #2453: split chunks must not carry the parent's sources.""" + + def test_split_chunks_do_not_carry_parent_sources(self): + parent_sources = [ + SourceMessage(type="chat", role="user", content=f"msg {i}") for i in range(4) + ] + parent = _build_parent_item(parent_sources) + reader = _build_reader(["chunk 1", "chunk 2", "chunk 3"]) + + chunks = reader._split_large_memory_item(parent, max_tokens=512) + + self.assertEqual(len(chunks), 3) + for chunk in chunks: + # Each carry is a TextualMemoryItem produced by ``_make_memory_item``. + self.assertIsInstance(chunk, TextualMemoryItem) + self.assertEqual( + chunk.metadata.sources, + [], + f"split chunk must not carry parent's sources; got " + f"{len(chunk.metadata.sources or [])} source(s)", + ) + + def test_split_chunks_preserve_provenance_via_internal_info(self): + """The parent-id/chunk-index provenance must still be populated so + downstream consumers can reconstruct chunk ordering.""" + parent_sources = [ + SourceMessage(type="chat", role="user", content=f"msg {i}") for i in range(3) + ] + parent = _build_parent_item(parent_sources) + reader = _build_reader(["chunk 1", "chunk 2"]) + + chunks = reader._split_large_memory_item(parent, max_tokens=512) + + self.assertEqual(len(chunks), 2) + for idx, chunk in enumerate(chunks): + self.assertIsNotNone(chunk.metadata.internal_info) + self.assertEqual(chunk.metadata.internal_info["chunk_index"], idx) + self.assertEqual(chunk.metadata.internal_info["chunk_total"], 2) + self.assertIn("ingest_batch_id", chunk.metadata.internal_info) + + def test_split_with_no_parent_sources_yields_no_sources(self): + """Sanity: a parent without sources still produces sources-less + chunks (no spurious empty list creation).""" + parent = _build_parent_item([]) + reader = _build_reader(["chunk 1"]) + + chunks = reader._split_large_memory_item(parent, max_tokens=512) + + self.assertEqual(len(chunks), 1) + self.assertEqual(chunks[0].metadata.sources, []) + + +if __name__ == "__main__": + unittest.main()