fix(agent): capture on_pre_compress return value and pass to compressor - #22576
fix(agent): capture on_pre_compress return value and pass to compressor#22576hbtjm9000 wants to merge 2 commits into
Conversation
Two-channel pattern: 1. Side effect: persist tagged insights (decisions, artifacts, blockers, config changes) to Honcho via create_conclusion() on a daemon thread. Survives compression; feeds future sessions through dialectic layer. 2. Return: structured summary string injected into compression LLM prompt. Activates automatically once upstream fixes discarded return value (GH #7192). Scope: last 12 messages (6 turns) — current task cycle only. Honcho's per-turn dialectic already captures user-model facts; this targets operational facts the compression LLM needs. Tested: guard paths (no-session, empty-messages, cron_skipped), extraction + threading + create_conclusion call all verified.
MemoryProvider.on_pre_compress() return value was silently discarded at the call site in run_agent.py. This broke all memory plugins (Honcho, Holographic, Mem0, etc.) that rely on this hook to inject context into the compression summary. Changes: - run_agent.py: capture return value as pre_compress_context, pass memory_context=pre_compress_context to ContextCompressor.compress() - agent/context_compressor.py: add memory_context parameter to compress() and _generate_summary(); inject ## Memory Provider Context section into compression prompt when non-empty; update all 3 retry call sites to pass memory_context through - tests/agent/test_context_compressor.py: add TestOnPreCompressIntegration (3 tests) Fixes #7192
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying a real contract break: MemoryProvider.on_pre_compress() and MemoryManager.on_pre_compress() explicitly return text intended for the compression prompt (agent/memory_provider.py:220-230, agent/memory_manager.py:883-900). Current main still discards it at agent/conversation_compression.py:631-644.
Problems
- The PR applies the handoff in
run_agent.py, but compression orchestration was extracted toagent/conversation_compression.pyby5311d9959e19477aac8aa7deca46c1ee0b8e7000. The current production path will need the handoff there. - The added compressor-only tests do not exercise that orchestrator boundary, where the defect currently exists.
Suggested changes
- Port the capture and forwarding logic to
agent/conversation_compression.py:631-644, then thread it throughContextCompressor.compress()and_generate_summary()including retry paths. - Add an orchestration regression test with a provider returning a sentinel and assert the summarizer prompt receives it.
This is an automated hermes-sweeper review.
| assert parsed["content"].endswith("...[truncated]") | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- |
There was a problem hiding this comment.
Please add a regression test for the actual orchestration boundary: a MemoryManager/provider returning a sentinel must cause the current compression flow to place that sentinel in the summary prompt. The production defect is now at agent/conversation_compression.py:631-644, so direct ContextCompressor.compress() coverage alone would not catch a dropped handoff.
|
This fix has landed on main via PR #67938 (salvaged from PR #64342 by @GottZ, which consolidated the on_pre_compress return-value forwarding with sanitization, engine-signature compatibility, and lock hardening). Your PR correctly identified the same root cause — the provider's return value being silently discarded before compression. The merged implementation supersedes this one, so closing it. First-submitter credit goes to @jairodriguez (#11236, Apr 16); thank you for the independent confirmation and fix. |
What does this PR do?
Captures the return value of
MemoryProvider.on_pre_compress()and passes it through to the context compressor's summarization prompt. Previously, the return value was silently discarded — every memory plugin returning compression context was broken without any error.The fix threads
memory_contextthrough three layers:run_agent.py— capture return value aspre_compress_contextContextCompressor.compress()— acceptmemory_contextparameterContextCompressor._generate_summary()— inject into compression promptRelated Issue
Fixes #7192
Type of Change
🐛Bug fix (non-breaking change that fixes an issue)Changes Made
run_agent.py(+28 lines)on_pre_compress()return value aspre_compress_contextmemory_context=pre_compress_contexttoself._context_compressor.compress()agent/context_compressor.py(+36 lines)compress(): addmemory_context: str = ""parameter + docstring note_generate_summary(): addmemory_context: str = ""parameter## Memory Provider Contextsection when non-empty (using pre-defined variable to avoid Python 3.11 f-string backslash restriction)memory_contextthroughtests/agent/test_context_compressor.py(+83 lines)TestOnPreCompressIntegrationclass with 3 tests:test_compress_accepts_memory_context_kwargtest_generate_summary_injects_memory_context_into_prompttest_empty_memory_context_does_not_add_sectionHow to Test
All 75 existing tests continue to pass.
Checklist
fix(agent):Conventional Commitspytest tests/agent/test_context_compressor.py -qpasses