mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
fix(memory/holographic): don't harvest compaction summaries; honor auto_extract=false string
Two compounding defects in the holographic memory provider (#57682): 1. The on_session_end gate used plain truthiness on auto_extract, but the plugin's own config schema declares it as a string enum with default "false" — and not "false" is False, so extraction ran for users who had it configured off. Coerce with the shared utils.is_truthy_value (same fix class as the merged byterover no-op fix). 2. _auto_extract_facts scanned every role=user message. Context-compaction handoff summaries can be inserted as role=user messages and their prose reliably matches the decision patterns (we decided/agreed, the project uses), so the compactor's own output was persisted as a durable project fact on every rollover following a compaction — recreated even after manual deletion. Adds agent.context_compressor.is_compaction_summary_message(), a public helper that prefers the in-process COMPRESSED_SUMMARY_METADATA_KEY marker and falls back to _is_context_summary_content() (covers merged-into-tail and historical prefixes), since the metadata key is stripped by wire sanitizers and doesn't survive all session-store round-trips. The plugin skips summary messages before pattern matching. Fixes #57682
This commit is contained in:
parent
cca7b93bfe
commit
004de13f14
3 changed files with 186 additions and 1 deletions
|
|
@ -3913,3 +3913,26 @@ This compaction should PRIORITISE preserving all information related to the focu
|
|||
self._last_compression_made_progress = True
|
||||
|
||||
return compressed
|
||||
|
||||
|
||||
def is_compaction_summary_message(message: Any) -> bool:
|
||||
"""Return True when *message* is a context-compaction handoff summary.
|
||||
|
||||
Public API for consumers outside the compressor (memory providers,
|
||||
frontends) that must not treat compaction summaries as real user or
|
||||
assistant turns — e.g. fact extraction harvesting the compactor's own
|
||||
output as user statements (#57682).
|
||||
|
||||
Prefers the in-process ``COMPRESSED_SUMMARY_METADATA_KEY`` marker and
|
||||
falls back to the content heuristics in ``_is_context_summary_content``
|
||||
(which cover the merged-into-tail and historical-prefix cases), because
|
||||
the metadata key is stripped by the wire sanitizers and does not survive
|
||||
all session-store round-trips.
|
||||
"""
|
||||
if isinstance(message, dict):
|
||||
if message.get(COMPRESSED_SUMMARY_METADATA_KEY):
|
||||
return True
|
||||
content = message.get("content")
|
||||
else:
|
||||
content = message
|
||||
return ContextCompressor._is_context_summary_content(content)
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ from typing import Any, Dict, List
|
|||
|
||||
from agent.memory_provider import MemoryProvider
|
||||
from tools.registry import tool_error
|
||||
from utils import is_truthy_value
|
||||
from .store import MemoryStore
|
||||
from .retrieval import FactRetriever
|
||||
from hermes_cli.config import cfg_get
|
||||
|
|
@ -235,7 +236,10 @@ class HolographicMemoryProvider(MemoryProvider):
|
|||
return tool_error(f"Unknown tool: {tool_name}")
|
||||
|
||||
def on_session_end(self, messages: List[Dict[str, Any]]) -> None:
|
||||
if not self._config.get("auto_extract", False):
|
||||
# is_truthy_value: the config schema declares auto_extract as a string
|
||||
# enum ("false"/"true"), and a plain truthiness check treats the string
|
||||
# "false" as enabled (#57682).
|
||||
if not is_truthy_value(self._config.get("auto_extract", False)):
|
||||
return
|
||||
if not self._store or not messages:
|
||||
return
|
||||
|
|
@ -368,6 +372,10 @@ class HolographicMemoryProvider(MemoryProvider):
|
|||
# -- Auto-extraction (on_session_end) ------------------------------------
|
||||
|
||||
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
|
||||
|
||||
_PREF_PATTERNS = [
|
||||
re.compile(r'\bI\s+(?:prefer|like|love|use|want|need)\s+(.+)', re.IGNORECASE),
|
||||
re.compile(r'\bmy\s+(?:favorite|preferred|default)\s+\w+\s+is\s+(.+)', re.IGNORECASE),
|
||||
|
|
@ -382,6 +390,12 @@ class HolographicMemoryProvider(MemoryProvider):
|
|||
for msg in messages:
|
||||
if msg.get("role") != "user":
|
||||
continue
|
||||
# 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):
|
||||
continue
|
||||
content = msg.get("content", "")
|
||||
if not isinstance(content, str) or len(content) < 10:
|
||||
continue
|
||||
|
|
|
|||
148
tests/plugins/memory/test_holographic_auto_extract.py
Normal file
148
tests/plugins/memory/test_holographic_auto_extract.py
Normal file
|
|
@ -0,0 +1,148 @@
|
|||
"""Regression tests for #57682 — holographic auto_extract harvested
|
||||
context-compaction handoff summaries into fact_store, and ran even when
|
||||
configured off.
|
||||
|
||||
Two compounding defects:
|
||||
|
||||
1. Gate: the plugin's config schema declares ``auto_extract`` as a string enum
|
||||
(``"false"``/``"true"``), and the ``on_session_end`` gate used plain
|
||||
truthiness — ``not "false"`` is ``False`` — so extraction ran despite being
|
||||
explicitly configured off.
|
||||
|
||||
2. Eligibility: ``_auto_extract_facts`` scanned every ``role == "user"``
|
||||
message. Compaction handoff summaries can be inserted as ``role="user"``
|
||||
messages, and their prose reliably matches the decision patterns
|
||||
(``we decided/agreed/chose``, ``the project uses/needs/requires``), so the
|
||||
compactor's own output was stored as a durable ``project`` fact on every
|
||||
session rollover that followed a compaction.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.context_compressor import (
|
||||
COMPRESSED_SUMMARY_METADATA_KEY,
|
||||
SUMMARY_PREFIX,
|
||||
_MERGED_PRIOR_CONTEXT_HEADER,
|
||||
_MERGED_SUMMARY_DELIMITER,
|
||||
is_compaction_summary_message,
|
||||
)
|
||||
from plugins.memory.holographic import HolographicMemoryProvider
|
||||
|
||||
|
||||
def _make_provider(tmp_path, **config):
|
||||
base = {"db_path": str(tmp_path / "memory_store.db"), "hrr_dim": 64}
|
||||
base.update(config)
|
||||
provider = HolographicMemoryProvider(config=base)
|
||||
provider.initialize(session_id="test-session")
|
||||
return provider
|
||||
|
||||
|
||||
def _fact_contents(provider):
|
||||
return [f["content"] for f in provider._store.list_facts(limit=100)]
|
||||
|
||||
|
||||
def _user(content, **extra):
|
||||
msg = {"role": "user", "content": content}
|
||||
msg.update(extra)
|
||||
return msg
|
||||
|
||||
|
||||
DECISION_MSG = "we decided to use PostgreSQL for the persistence layer"
|
||||
SUMMARY_MSG = (
|
||||
f"{SUMMARY_PREFIX}\n## Historical Task Snapshot\n"
|
||||
"The project uses a kanban board for all dispatch. "
|
||||
"We agreed to route reviews through the fan-in consumer."
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Defect 1 — string-boolean gate
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize("off_value", [False, "false", "False", "no", "off", "0", None, ""])
|
||||
def test_auto_extract_off_values_disable_extraction(tmp_path, off_value):
|
||||
provider = _make_provider(tmp_path, auto_extract=off_value)
|
||||
provider.on_session_end([_user(DECISION_MSG)])
|
||||
assert _fact_contents(provider) == []
|
||||
provider.shutdown()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("on_value", [True, "true", "True", "1", "yes", "on"])
|
||||
def test_auto_extract_on_values_enable_extraction(tmp_path, on_value):
|
||||
provider = _make_provider(tmp_path, auto_extract=on_value)
|
||||
provider.on_session_end([_user(DECISION_MSG)])
|
||||
facts = _fact_contents(provider)
|
||||
assert len(facts) == 1
|
||||
assert "PostgreSQL" in facts[0]
|
||||
provider.shutdown()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Defect 2 — compaction summaries harvested as facts
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_compaction_summary_not_harvested(tmp_path):
|
||||
"""The exact failure mode from #57682: summary prose matches the decision
|
||||
patterns but must not become a fact."""
|
||||
provider = _make_provider(tmp_path, auto_extract=True)
|
||||
provider.on_session_end([_user(SUMMARY_MSG)])
|
||||
assert _fact_contents(provider) == []
|
||||
provider.shutdown()
|
||||
|
||||
|
||||
def test_metadata_marked_summary_not_harvested(tmp_path):
|
||||
"""In-process summaries carry COMPRESSED_SUMMARY_METADATA_KEY even if a
|
||||
future prefix rewrite changes the content sentinel."""
|
||||
provider = _make_provider(tmp_path, auto_extract=True)
|
||||
marked = _user(DECISION_MSG, **{COMPRESSED_SUMMARY_METADATA_KEY: True})
|
||||
provider.on_session_end([marked])
|
||||
assert _fact_contents(provider) == []
|
||||
provider.shutdown()
|
||||
|
||||
|
||||
def test_merged_into_tail_summary_not_harvested(tmp_path):
|
||||
"""Merge-into-tail summaries embed the handoff prefix after the delimiter,
|
||||
not at the start of the message."""
|
||||
provider = _make_provider(tmp_path, auto_extract=True)
|
||||
merged = _user(
|
||||
f"{_MERGED_PRIOR_CONTEXT_HEADER}\nplease fix the login bug\n"
|
||||
f"{_MERGED_SUMMARY_DELIMITER}\n{SUMMARY_MSG}"
|
||||
)
|
||||
provider.on_session_end([merged])
|
||||
assert _fact_contents(provider) == []
|
||||
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."""
|
||||
provider = _make_provider(tmp_path, auto_extract=True)
|
||||
provider.on_session_end(
|
||||
[
|
||||
_user(SUMMARY_MSG),
|
||||
_user("I prefer tabs over spaces for indentation"),
|
||||
]
|
||||
)
|
||||
facts = _fact_contents(provider)
|
||||
assert len(facts) == 1
|
||||
assert "tabs over spaces" in facts[0]
|
||||
provider.shutdown()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# is_compaction_summary_message — public helper contract
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_helper_detects_prefix_metadata_and_merged_forms():
|
||||
assert is_compaction_summary_message(_user(SUMMARY_MSG))
|
||||
assert is_compaction_summary_message(
|
||||
_user("anything", **{COMPRESSED_SUMMARY_METADATA_KEY: True})
|
||||
)
|
||||
assert is_compaction_summary_message(
|
||||
_user(f"prior tail\n{_MERGED_SUMMARY_DELIMITER}\n{SUMMARY_MSG}")
|
||||
)
|
||||
assert not is_compaction_summary_message(_user(DECISION_MSG))
|
||||
assert not is_compaction_summary_message(_user(""))
|
||||
Loading…
Add table
Add a link
Reference in a new issue