Skip to content

fix(memory): forward on_pre_compress return value to compress() as pr… - #29598

Closed
buptwz wants to merge 1 commit into
NousResearch:mainfrom
buptwz:fix/memory-provider-compress-context-chain
Closed

fix(memory): forward on_pre_compress return value to compress() as pr…#29598
buptwz wants to merge 1 commit into
NousResearch:mainfrom
buptwz:fix/memory-provider-compress-context-chain

Conversation

@buptwz

@buptwz buptwz commented May 21, 2026

Copy link
Copy Markdown

Problem

Fixes #23367.

MemoryProvider.on_pre_compress() lets external memory backends return a hint string telling the summarisation LLM which facts to preserve
during compression. The return value was silently discarded — every provider implementing this hook was being ignored.

Root Cause

conversation_compression.py called on_pre_compress() but threw away the return value. ContextCompressor.compress() had no
provider_context parameter, so the hint had nowhere to go even if captured.

Solution

  • conversation_compression.py: capture on_pre_compress() return value, forward to compress()
  • context_compressor.py: add provider_context param to compress() / _generate_summary(); inject as MEMORY PROVIDER CONTEXT block
    in the LLM summarisation prompt
  • holographic plugin: add reference on_pre_compress() returning top-trust stored facts

Tests

10 new tests, all pass. Mutation-verified.

  • tests/agent/test_compress_focus.py (4): provider_context injected into LLM prompt, empty string skipped, coexists with focus_topic,
    forwarded through compress()
  • tests/agent/test_memory_provider.py (3): hint text passes through MemoryManager, empty and whitespace-only returns handled correctly
  • tests/run_agent/test_compression_boundary_hook.py (3): end-to-end integration — hint flows from on_pre_compress() into
    compress(provider_context=...) via real `AIAgent

Tested on: Linux (Ubuntu 24.04, Python 3.13)

…ovider_context

The return value of MemoryProvider.on_pre_compress() was silently
discarded in conversation_compression.py — it was never passed to
ContextCompressor.compress(), so memory providers had no way to
guide the summarisation LLM on which facts to preserve.

Changes:
- conversation_compression.py: capture on_pre_compress() return value
  and forward it as provider_context= kwarg to compress()
- context_compressor.py: add provider_context param to compress() and
  _generate_summary(); inject as MEMORY PROVIDER CONTEXT block in the
  LLM summarisation prompt when non-empty
- holographic plugin: add reference on_pre_compress() returning top-trust
  stored facts as compression hints
- tests: 10 new tests covering the full chain with mutation-verified
  coverage (breaking the chain causes tests to fail)

Fixes NousResearch#23367

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers P3 Low — cosmetic, nice to have labels May 21, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing PRs for the same on_pre_compress() return value bug (#7192): #7195, #11236, #21118, #22576. This PR also references #23367. All address the same root cause — on_pre_compress() return silently discarded.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the return value through the memory-provider and compression layers. The core premise remains valid on current main: agent/conversation_compression.py:634 still discards MemoryManager.on_pre_compress()'s result, despite agent/memory_provider.py:226-228 defining that return value as summary-prompt context.

Problems

  • Current compaction now derives summary_focus_topic before invoking _generate_summary() at agent/context_compressor.py:2997-2999; salvage must preserve that behavior while adding provider context, including the retry calls at agent/context_compressor.py:2178 and 2195.
  • agent/context_engine.py:87-105 does not advertise a provider-context argument. The existing TypeError fallback in agent/conversation_compression.py:640-644 reduces calls to current_tokens only, so a custom context engine that has not adopted the new argument will silently lose the hint and existing optional arguments.

Suggested changes

  • Rebase the implementation concept onto the current compression call chain and cover auto-derived focus plus retry preservation.
  • Make the context-engine compatibility path explicit and capability-aware rather than relying on the broad TypeError fallback.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 13, 2026
@teknium1 teknium1 added area/memory Memory subsystem: store, providers, sync, background reviews area/compression Context compression and continuation sessions labels Jul 19, 2026
@teknium1

Copy link
Copy Markdown
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/compression Context compression and continuation sessions area/memory Memory subsystem: store, providers, sync, background reviews comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Context compression should integrate memory provider context into the summary

3 participants