From cc84af9fad1c4bb943eb0717763d08af46695d5d Mon Sep 17 00:00:00 2001 From: wernerhp Date: Wed, 15 Jul 2026 19:16:47 +0000 Subject: [PATCH] fix(memory): preserve genuine pre-delimiter content in merged compaction rows teknium1 review on #57690: harvesting logic was skipping the ENTIRE merged row when a compaction summary was appended to the tail message, discarding real prior user content that context_compressor retains before the _MERGED_SUMMARY_DELIMITER. Extract and harvest that pre-delimiter segment instead of dropping it wholesale. Revert-to-fail: reverting plugins/memory/holographic/__init__.py alone drops test_merged_into_tail_preserves_genuine_pre_delimiter_preference (19 passed, 1 failed); restoring the fix returns 20/20 passed. --- plugins/memory/holographic/__init__.py | 40 +++++++++++++++++-- .../memory/test_holographic_auto_extract.py | 26 +++++++++++- 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/plugins/memory/holographic/__init__.py b/plugins/memory/holographic/__init__.py index f44b880bb68..7531469755a 100644 --- a/plugins/memory/holographic/__init__.py +++ b/plugins/memory/holographic/__init__.py @@ -374,7 +374,32 @@ class HolographicMemoryProvider(MemoryProvider): def _auto_extract_facts(self, messages: list) -> None: # Local import (pattern used in initialize()): the compressor module is # heavier than this plugin and is only needed when auto_extract is on. - from agent.context_compressor import is_compaction_summary_message + from agent.context_compressor import ( + _MERGED_PRIOR_CONTEXT_HEADER, + _MERGED_SUMMARY_DELIMITER, + is_compaction_summary_message, + ) + + def _pre_delimiter_user_segment(msg: dict): + """Return the genuine user text preceding a merged-into-tail + compaction summary, or None when the whole message is a summary. + + Merge-into-tail messages (agent/context_compressor.py ~3163-3190) + wrap real prior tail content BEFORE ``_MERGED_SUMMARY_DELIMITER``, + prefixed with ``_MERGED_PRIOR_CONTEXT_HEADER``, then append the + generated handoff summary AFTER the delimiter. Dropping the whole + row (as ``is_compaction_summary_message`` alone would suggest) + discards that genuine pre-delimiter content too (#57690 review). + Only the summary suffix must be excluded from harvesting. + """ + content = msg.get("content", "") + if not isinstance(content, str) or _MERGED_SUMMARY_DELIMITER not in content: + return None + pre = content.split(_MERGED_SUMMARY_DELIMITER, 1)[0] + if pre.startswith(_MERGED_PRIOR_CONTEXT_HEADER): + pre = pre[len(_MERGED_PRIOR_CONTEXT_HEADER):] + pre = pre.strip() + return pre or None _PREF_PATTERNS = [ re.compile(r'\bI\s+(?:prefer|like|love|use|want|need)\s+(.+)', re.IGNORECASE), @@ -393,10 +418,17 @@ class HolographicMemoryProvider(MemoryProvider): # Compaction handoff summaries can be inserted as role="user" # messages; their prose reliably matches the decision patterns, so # without this guard the compactor's own output is stored as a - # durable "fact" on every rollover (#57682). - if is_compaction_summary_message(msg): + # durable "fact" on every rollover (#57682). A merge-into-tail + # summary also carries genuine pre-delimiter user content in the + # SAME row; harvest that segment instead of dropping the whole + # message (#57690 review). + pre_delimiter_segment = _pre_delimiter_user_segment(msg) + if pre_delimiter_segment is not None: + content = pre_delimiter_segment + elif is_compaction_summary_message(msg): continue - content = msg.get("content", "") + else: + content = msg.get("content", "") if not isinstance(content, str) or len(content) < 10: continue diff --git a/tests/plugins/memory/test_holographic_auto_extract.py b/tests/plugins/memory/test_holographic_auto_extract.py index 11b7cf3afef..1caec55bd6d 100644 --- a/tests/plugins/memory/test_holographic_auto_extract.py +++ b/tests/plugins/memory/test_holographic_auto_extract.py @@ -102,9 +102,10 @@ def test_metadata_marked_summary_not_harvested(tmp_path): provider.shutdown() -def test_merged_into_tail_summary_not_harvested(tmp_path): +def test_merged_into_tail_summary_suffix_not_harvested_prefix_content_ignored(tmp_path): """Merge-into-tail summaries embed the handoff prefix after the delimiter, - not at the start of the message.""" + not at the start of the message. The wrapped pre-delimiter segment here has + no fact-pattern match, so nothing is harvested from either side.""" provider = _make_provider(tmp_path, auto_extract=True) merged = _user( f"{_MERGED_PRIOR_CONTEXT_HEADER}\nplease fix the login bug\n" @@ -115,6 +116,27 @@ def test_merged_into_tail_summary_not_harvested(tmp_path): provider.shutdown() +def test_merged_into_tail_preserves_genuine_pre_delimiter_preference(tmp_path): + """#57690 review: teknium1 noted the ENTIRE merged row was being skipped, + discarding genuine pre-delimiter user content (context_compressor.py + ~3163-3190 retains real prior tail text before the summary). The fix must + extract and harvest that segment while still excluding the summary + suffix.""" + provider = _make_provider(tmp_path, auto_extract=True) + merged = _user( + f"{_MERGED_PRIOR_CONTEXT_HEADER}\n" + "I prefer tabs over spaces for indentation\n" + f"{_MERGED_SUMMARY_DELIMITER}\n{SUMMARY_MSG}" + ) + provider.on_session_end([merged]) + facts = _fact_contents(provider) + assert len(facts) == 1 + assert "tabs over spaces" in facts[0] + assert "kanban board" not in facts[0] + assert "PostgreSQL" not in facts[0] + provider.shutdown() + + def test_real_user_messages_still_extracted_alongside_summary(tmp_path): """The guard must skip only the summary, not suppress extraction for the genuine user turns around it."""