From b057c12353634c26c9d21dbab85d6d34192acc89 Mon Sep 17 00:00:00 2001 From: MemOS AutoDev Bot Date: Tue, 14 Jul 2026 18:30:19 +0800 Subject: [PATCH 1/3] fix(multi_mem_cube): sanitize search_memories INFO log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SingleCubeView.search_memories was logging the full MOSSearchResult at INFO on every call. Under the default APISearchRequest (dedup="mmr"), each memory keeps its metadata.embedding vector, so the log line contained megabytes of floats per search — an explosion of noise plus a privacy/compliance risk. The paired `len(memories_result)` line was also buggy: it counted top-level dict keys (~7 constant), not memories. Introduce _summarize_search_result_for_log(result), a pure helper that returns a bounded dict with per-bucket counts, an aggregate total, a capped sample of {id, memory_type, relativity} entries per bucket, and a has_embedding boolean flag — never the raw vector. Replace the two offending logger.info lines with a single sanitized summary line using %s deferred formatting. Behaviour of the search API and its response payload is unchanged; this is a logging-only fix. Add regression tests under tests/multi_mem_cube/ covering the no-leak contract, per-bucket counts, sample bounding, both dedup branches (mmr/sim vs no), and malformed-input robustness. Tests import the helper lazily to work around the existing circular import in memos.api.handlers.__init__ (same workaround as tests/test_add_stage_logging.py). Fixes #2103 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/memos/multi_mem_cube/single_cube.py | 88 ++++- tests/multi_mem_cube/__init__.py | 0 .../test_search_log_sanitization.py | 301 ++++++++++++++++++ 3 files changed, 387 insertions(+), 2 deletions(-) create mode 100644 tests/multi_mem_cube/__init__.py create mode 100644 tests/multi_mem_cube/test_search_log_sanitization.py diff --git a/src/memos/multi_mem_cube/single_cube.py b/src/memos/multi_mem_cube/single_cube.py index f84fc60e1..fdd38b524 100644 --- a/src/memos/multi_mem_cube/single_cube.py +++ b/src/memos/multi_mem_cube/single_cube.py @@ -37,6 +37,89 @@ logger = get_logger(__name__) +# Cap on how many memories per bucket appear in the search log summary. The +# full count is still reported via ``summary["counts"]``; this just bounds the +# INFO-line size when top_k is large. +_LOG_SAMPLE_MAX_PER_BUCKET = 5 + +# Buckets we surface counts/samples for in the log summary. ``act_mem`` and +# ``para_mem`` are intentionally excluded — they are not populated by the text +# search path and would just add noise. +_LOG_SUMMARY_BUCKETS = ("text_mem", "pref_mem", "tool_mem", "skill_mem") + + +def _summarize_search_result_for_log( + result: dict[str, Any], +) -> dict[str, Any]: + """Build a compact, embedding-free summary of a ``MOSSearchResult`` for + logging. + + The previous ``search_memories`` INFO log dumped the full result dict, + which under the default ``dedup="mmr"`` (or ``dedup="sim"``) branch keeps + the raw ``metadata.embedding`` vector on every memory. That leaked + high-dimensional floats into the log stream, caused log explosion, and + raised privacy concerns (issue #2103). + + This helper produces a bounded dict containing: + * ``counts``: total memories per bucket (``text_mem`` / ``pref_mem`` / + ``tool_mem`` / ``skill_mem``). + * ``total``: sum of counts across all buckets. + * ``samples``: up to ``_LOG_SAMPLE_MAX_PER_BUCKET`` items per bucket, + exposing only ``id`` / ``memory_type`` / ``relativity`` — never the + embedding. + * ``has_embedding``: True iff at least one memory carries a non-empty + embedding list. The boolean is safe to log; the vector itself is not. + + The helper degrades gracefully on malformed input (missing buckets, + non-dict memory items). + """ + + counts: dict[str, int] = dict.fromkeys(_LOG_SUMMARY_BUCKETS, 0) + samples: dict[str, list[dict[str, Any]]] = {bucket: [] for bucket in _LOG_SUMMARY_BUCKETS} + has_embedding = False + + for bucket in _LOG_SUMMARY_BUCKETS: + groups = result.get(bucket) or [] + if not isinstance(groups, list): + continue + + for group in groups: + if not isinstance(group, dict): + continue + memories = group.get("memories") or [] + if not isinstance(memories, list): + continue + + counts[bucket] += len(memories) + + for mem in memories: + if not isinstance(mem, dict): + continue + + metadata = mem.get("metadata") or {} + if isinstance(metadata, dict): + embedding = metadata.get("embedding") + if isinstance(embedding, list) and len(embedding) > 0: + has_embedding = True + + if len(samples[bucket]) < _LOG_SAMPLE_MAX_PER_BUCKET: + metadata_dict = metadata if isinstance(metadata, dict) else {} + samples[bucket].append( + { + "id": mem.get("id"), + "memory_type": metadata_dict.get("memory_type"), + "relativity": metadata_dict.get("relativity"), + } + ) + + return { + "counts": counts, + "total": sum(counts.values()), + "samples": samples, + "has_embedding": has_embedding, + } + + if TYPE_CHECKING: from memos.api.product_models import APIADDRequest, APIFeedbackRequest, APISearchRequest from memos.mem_cube.navie import NaiveMemCube @@ -129,8 +212,9 @@ def search_memories(self, search_req: APISearchRequest) -> dict[str, Any]: self.cube_id, ) - self.logger.info(f"Search memories result: {memories_result}") - self.logger.info(f"Search {len(memories_result)} memories.") + self.logger.info( + "Search memories summary: %s", _summarize_search_result_for_log(memories_result) + ) return memories_result @timed diff --git a/tests/multi_mem_cube/__init__.py b/tests/multi_mem_cube/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/multi_mem_cube/test_search_log_sanitization.py b/tests/multi_mem_cube/test_search_log_sanitization.py new file mode 100644 index 000000000..7be0303b7 --- /dev/null +++ b/tests/multi_mem_cube/test_search_log_sanitization.py @@ -0,0 +1,301 @@ +""" +Regression tests for the sanitized ``search_memories`` INFO log emitted by +:mod:`memos.multi_mem_cube.single_cube`. + +Issue #2103: the previous implementation printed the full ``MOSSearchResult`` +dict, which under the default ``dedup="mmr"`` (or ``dedup="sim"``) branch keeps +``metadata.embedding`` as a high-dimensional float list. That leaks the raw +vectors into the INFO log stream, causing log explosion and privacy risk. + +The fix introduces :func:`_summarize_search_result_for_log` which produces a +bounded, embedding-free summary. These tests pin its contract so any regression +that re-enables the leak fails fast. + +NOTE: ``_summarize_search_result_for_log`` is imported lazily inside each test +to work around a known circular import in ``memos.api.handlers.__init__`` (see +``tests/test_add_stage_logging.py`` for the same workaround). +""" + +from __future__ import annotations + +import json + +import pytest + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _import_helper(): + # Preload memos.api.handlers first — importing single_cube directly would + # trigger a partially-initialised module during its own re-entrant import + # chain (see the circular-import workaround in tests/test_add_stage_logging.py). + import memos.api.handlers # noqa: F401 + + from memos.multi_mem_cube.single_cube import _summarize_search_result_for_log + + return _summarize_search_result_for_log + + +def _make_memory( + mem_id: str, + memory_type: str, + *, + embedding: list[float] | None = None, + relativity: float = 0.5, + cube_id: str = "cube-A", +) -> dict: + """Construct a single formatted memory dict matching what + ``format_memory_item`` produces downstream.""" + return { + "id": mem_id, + "memory": f"memory text for {mem_id}", + "ref_id": f"[{mem_id[:8]}]", + "metadata": { + "id": mem_id, + "memory_type": memory_type, + "relativity": relativity, + "embedding": embedding if embedding is not None else [], + "sources": [], + "usage": [], + }, + "cube_id": cube_id, + } + + +def _make_result( + text_mems: list[dict] | None = None, + pref_mems: list[dict] | None = None, + tool_mems: list[dict] | None = None, + skill_mems: list[dict] | None = None, + cube_id: str = "cube-A", +) -> dict: + """Build a ``MOSSearchResult``-shaped dict for a single cube.""" + return { + "text_mem": [ + { + "cube_id": cube_id, + "memories": text_mems or [], + "total_nodes": len(text_mems or []), + } + ], + "act_mem": [], + "para_mem": [], + "pref_mem": [ + { + "cube_id": cube_id, + "memories": pref_mems or [], + "total_nodes": len(pref_mems or []), + } + ], + "pref_note": "", + "tool_mem": [ + { + "cube_id": cube_id, + "memories": tool_mems or [], + "total_nodes": len(tool_mems or []), + } + ], + "skill_mem": [ + { + "cube_id": cube_id, + "memories": skill_mems or [], + "total_nodes": len(skill_mems or []), + } + ], + } + + +# --------------------------------------------------------------------------- +# Core sanitization contract +# --------------------------------------------------------------------------- + + +class TestNoEmbeddingLeak: + def test_embedding_floats_are_stripped_from_summary(self): + """The raw embedding float list must never appear in the summary.""" + summarize = _import_helper() + big_vector = [0.0123, -0.0456, 0.7890, -0.1234, 0.5678] * 50 # 250 floats + result = _make_result( + text_mems=[ + _make_memory("id-1", "UserMemory", embedding=big_vector), + _make_memory("id-2", "LongTermMemory", embedding=big_vector), + ] + ) + + summary = summarize(result) + rendered = json.dumps(summary, default=str) + + # None of the vector components may appear in the rendered log payload. + for value in (0.0123, -0.0456, 0.7890, 0.5678): + assert str(value) not in rendered, ( + f"Embedding float {value} leaked into log summary: {rendered!r}" + ) + + def test_summary_flags_embedding_presence_as_boolean(self): + """We still want to know *whether* embeddings were populated, but only + as a bool — not the vector itself.""" + summarize = _import_helper() + big_vector = [0.11, 0.22, 0.33] + result = _make_result(text_mems=[_make_memory("a", "UserMemory", embedding=big_vector)]) + + summary = summarize(result) + + assert summary["has_embedding"] is True + + def test_summary_marks_absent_embedding(self): + summarize = _import_helper() + result = _make_result(text_mems=[_make_memory("a", "UserMemory", embedding=[])]) + + summary = summarize(result) + + assert summary["has_embedding"] is False + + +# --------------------------------------------------------------------------- +# Debug value preservation +# --------------------------------------------------------------------------- + + +class TestSummaryContent: + def test_per_bucket_counts_are_reported(self): + summarize = _import_helper() + result = _make_result( + text_mems=[ + _make_memory("t1", "UserMemory"), + _make_memory("t2", "LongTermMemory"), + _make_memory("t3", "WorkingMemory"), + ], + pref_mems=[_make_memory("p1", "PreferenceMemory")], + tool_mems=[ + _make_memory("tool1", "ToolSchemaMemory"), + _make_memory("tool2", "ToolTrajectoryMemory"), + ], + skill_mems=[_make_memory("s1", "SkillMemory")], + ) + + summary = summarize(result) + counts = summary["counts"] + + assert counts["text_mem"] == 3 + assert counts["pref_mem"] == 1 + assert counts["tool_mem"] == 2 + assert counts["skill_mem"] == 1 + + def test_total_count_is_reported(self): + summarize = _import_helper() + result = _make_result( + text_mems=[_make_memory(f"t{i}", "UserMemory") for i in range(4)], + pref_mems=[_make_memory("p1", "PreferenceMemory")], + ) + + summary = summarize(result) + + # total should be sum across all four buckets + assert summary["total"] == 5 + + def test_sample_memory_ids_are_included_for_debug(self): + summarize = _import_helper() + result = _make_result( + text_mems=[ + _make_memory("id-alpha", "UserMemory", relativity=0.9), + _make_memory("id-beta", "LongTermMemory", relativity=0.8), + ] + ) + + summary = summarize(result) + + # We expect the per-bucket sample to expose ids + memory_type + relativity + text_samples = summary["samples"]["text_mem"] + assert len(text_samples) == 2 + ids = {item["id"] for item in text_samples} + assert ids == {"id-alpha", "id-beta"} + types = {item["memory_type"] for item in text_samples} + assert types == {"UserMemory", "LongTermMemory"} + + def test_sample_size_is_bounded(self): + """Samples must be capped to keep the log line small even when top_k + is large. The cap value itself is an implementation detail; here we + just assert it is bounded to <= 5 per bucket.""" + summarize = _import_helper() + text_mems = [_make_memory(f"id-{i}", "UserMemory") for i in range(20)] + result = _make_result(text_mems=text_mems) + + summary = summarize(result) + text_samples = summary["samples"]["text_mem"] + + assert len(text_samples) <= 5 + # But bucket count still reports the true value. + assert summary["counts"]["text_mem"] == 20 + + +# --------------------------------------------------------------------------- +# Dedup branches — regression coverage for issue #2103 +# --------------------------------------------------------------------------- + + +class TestDedupBranches: + @pytest.mark.parametrize( + "embedding_value", + [ + [0.001, 0.002, 0.003, 0.004, 0.005], # dedup="mmr" / "sim" + [], # dedup="no" (or anything else that drops embeddings) + ], + ) + def test_both_branches_produce_safe_output(self, embedding_value): + """Whether embeddings are populated (mmr/sim) or empty (no), the log + summary must remain safe and structurally identical.""" + summarize = _import_helper() + result = _make_result( + text_mems=[_make_memory("m1", "UserMemory", embedding=embedding_value)] + ) + + summary = summarize(result) + rendered = json.dumps(summary, default=str) + + # These specific floats must not leak. + for value in embedding_value: + assert str(value) not in rendered + + # Both branches must expose the same top-level keys. + assert set(summary.keys()) >= {"counts", "total", "samples", "has_embedding"} + + +# --------------------------------------------------------------------------- +# Robustness +# --------------------------------------------------------------------------- + + +class TestRobustness: + def test_missing_bucket_keys_do_not_raise(self): + """The helper must tolerate partially-populated results — e.g. when a + bucket is missing entirely (older callers, edge cases).""" + summarize = _import_helper() + result = {"text_mem": [], "pref_mem": []} + summary = summarize(result) + + assert summary["counts"]["text_mem"] == 0 + assert summary["counts"]["pref_mem"] == 0 + assert summary["counts"]["tool_mem"] == 0 + assert summary["counts"]["skill_mem"] == 0 + assert summary["total"] == 0 + + def test_non_dict_memory_items_do_not_raise(self): + """The helper should degrade gracefully rather than crash the whole + search path if a memory item is not the expected shape.""" + summarize = _import_helper() + result = _make_result() + # inject a malformed memory + result["text_mem"][0]["memories"] = [ + {"id": "ok", "metadata": {"memory_type": "UserMemory", "relativity": 0.1}}, + "not-a-dict", + None, + ] + + # Must not raise + summary = summarize(result) + + # The valid item is still counted + assert summary["counts"]["text_mem"] == 3 From 33e6043fb10cb65a2ed53b61e0d651097aaf04fa Mon Sep 17 00:00:00 2001 From: MemOS AutoDev Bot Date: Tue, 14 Jul 2026 18:55:03 +0800 Subject: [PATCH 2/3] fix(multi_mem_cube): tighten log summary count + dedup='no' leak assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address two Open Code Review findings on PR #2106: 1) src/memos/multi_mem_cube/single_cube.py — `_summarize_search_result_for_log` was doing `counts[bucket] += len(memories)` before the per-item `isinstance(mem, dict)` guard, so heterogeneous input (strings, None, etc.) silently inflated the reported bucket total and diverged from the docstring's "total memories per bucket" contract. Move the increment inside the loop, after validation, so only well-formed dict entries count. 2) tests/multi_mem_cube/test_search_log_sanitization.py — the parametrized `test_both_branches_produce_safe_output` case had zero leak assertions on the `embedding_value=[]` branch because the `for value in embedding_value` loop is a no-op. Add an explicit structural assertion that `'"embedding"'` never appears in the rendered summary so `dedup="no"` carries real leak-detection signal. Also update the malformed-input regression case (`test_non_dict_memory_items_do_not_raise`) to assert the new post-validation count semantics (1 valid dict, not 3 raw list entries). Related PR: #2106 Refs #2103 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/memos/multi_mem_cube/single_cube.py | 8 ++++++-- tests/multi_mem_cube/test_search_log_sanitization.py | 11 +++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/memos/multi_mem_cube/single_cube.py b/src/memos/multi_mem_cube/single_cube.py index fdd38b524..1ea4be4e5 100644 --- a/src/memos/multi_mem_cube/single_cube.py +++ b/src/memos/multi_mem_cube/single_cube.py @@ -90,12 +90,16 @@ def _summarize_search_result_for_log( if not isinstance(memories, list): continue - counts[bucket] += len(memories) - for mem in memories: if not isinstance(mem, dict): continue + # Count only validated dict entries so ``counts`` matches the + # docstring's "total memories per bucket" contract. Non-dict + # items (strings, ``None``, etc.) are silently ignored rather + # than inflating the reported total. + counts[bucket] += 1 + metadata = mem.get("metadata") or {} if isinstance(metadata, dict): embedding = metadata.get("embedding") diff --git a/tests/multi_mem_cube/test_search_log_sanitization.py b/tests/multi_mem_cube/test_search_log_sanitization.py index 7be0303b7..8c86b5ba2 100644 --- a/tests/multi_mem_cube/test_search_log_sanitization.py +++ b/tests/multi_mem_cube/test_search_log_sanitization.py @@ -259,6 +259,11 @@ def test_both_branches_produce_safe_output(self, embedding_value): for value in embedding_value: assert str(value) not in rendered + # For the empty-embedding branch the loop above is a no-op — parametrize + # gives us zero iterations, so without this the ``dedup="no"`` case + # would carry no leak-detection signal. Assert the structural key too. + assert '"embedding"' not in rendered + # Both branches must expose the same top-level keys. assert set(summary.keys()) >= {"counts", "total", "samples", "has_embedding"} @@ -297,5 +302,7 @@ def test_non_dict_memory_items_do_not_raise(self): # Must not raise summary = summarize(result) - # The valid item is still counted - assert summary["counts"]["text_mem"] == 3 + # Only validated dict entries contribute to the reported bucket count; + # the ``"not-a-dict"`` string and ``None`` are silently skipped so the + # summary matches the docstring's "total memories per bucket" contract. + assert summary["counts"]["text_mem"] == 1 From a2e12f8f6ff46c4e11bb8f00b9664857c4051a8c Mon Sep 17 00:00:00 2001 From: MemOS AutoDev Bot Date: Tue, 14 Jul 2026 19:02:03 +0800 Subject: [PATCH 3/3] test(multi_mem_cube): strengthen dedup='no' leak-detection assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the OCR round-2 finding on tests/multi_mem_cube/test_search_log_sanitization.py L265: The previous compensating check `assert '"embedding"' not in rendered` was trivially always True — `_summarize_search_result_for_log` never emits an `"embedding"` key at all (only the boolean `"has_embedding"`), so the assertion couldn't fail even if a future change re-introduced the raw vector under a different name (`"embed"`, `"vector"`, `"raw_embedding"`, …). Replace it with two assertions that carry real signal: * Parametrize `expected_has_embedding` alongside `embedding_value` and assert `summary["has_embedding"] is expected_has_embedding`. The empty branch now has a concrete non-trivial claim (must be False), and the populated branch pins the True case. * Pin the top-level keyset with `==` (not `>=`) so any surprise extra key — the most likely shape of an accidental future leak — fails the test. No production-code change; this is a pure test-strengthening fix. Related PR: #2106 Refs #2103 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../test_search_log_sanitization.py | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/tests/multi_mem_cube/test_search_log_sanitization.py b/tests/multi_mem_cube/test_search_log_sanitization.py index 8c86b5ba2..31053d035 100644 --- a/tests/multi_mem_cube/test_search_log_sanitization.py +++ b/tests/multi_mem_cube/test_search_log_sanitization.py @@ -238,13 +238,13 @@ def test_sample_size_is_bounded(self): class TestDedupBranches: @pytest.mark.parametrize( - "embedding_value", + ("embedding_value", "expected_has_embedding"), [ - [0.001, 0.002, 0.003, 0.004, 0.005], # dedup="mmr" / "sim" - [], # dedup="no" (or anything else that drops embeddings) + ([0.001, 0.002, 0.003, 0.004, 0.005], True), # dedup="mmr" / "sim" + ([], False), # dedup="no" (or anything else that drops embeddings) ], ) - def test_both_branches_produce_safe_output(self, embedding_value): + def test_both_branches_produce_safe_output(self, embedding_value, expected_has_embedding): """Whether embeddings are populated (mmr/sim) or empty (no), the log summary must remain safe and structurally identical.""" summarize = _import_helper() @@ -259,13 +259,18 @@ def test_both_branches_produce_safe_output(self, embedding_value): for value in embedding_value: assert str(value) not in rendered - # For the empty-embedding branch the loop above is a no-op — parametrize - # gives us zero iterations, so without this the ``dedup="no"`` case - # would carry no leak-detection signal. Assert the structural key too. - assert '"embedding"' not in rendered - - # Both branches must expose the same top-level keys. - assert set(summary.keys()) >= {"counts", "total", "samples", "has_embedding"} + # ``has_embedding`` is the one boolean signal we deliberately expose; + # pin it per branch so the empty-embedding case (where the loop above + # is a no-op) still carries a real, non-trivial assertion. + assert summary["has_embedding"] is expected_has_embedding + + # Pin the top-level structure *exactly* — using ``==`` rather than + # ``>=``. A future accidental leak under a different key name + # (e.g. ``"embed"`` / ``"vector"`` / ``"raw_embedding"``) would + # add an extra key here and fail this assertion, whereas the earlier + # ``'"embedding"' not in rendered`` check was trivially true since the + # helper never emits that literal key at all. + assert set(summary.keys()) == {"counts", "total", "samples", "has_embedding"} # ---------------------------------------------------------------------------