fix: dedupe persisted compaction handoffs - #47274
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
What was changed
Fix deduplication of persisted compaction handoffs. Previously, re-compression cycles would keep old handoff-only messages and accumulate duplicate [CONTEXT COMPACTION] markers. The new _strip_context_summary_handoff_message removes or unwraps these stale handoff messages during compression.
Observations
The _strip_context_summary_handoff_message classmethod correctly identifies summary messages, finds the _SUMMARY_END_MARKER, and either unwraps to reveal the content beneath or drops the message entirely if it was handoff-only. The fix is well-scoped to the context compressor.
Testing
Existing compression tests should cover this edge case. The fix adds a clear new code path for stripping, which is correctly integrated into the compress loop.
Security
No concerns.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the protected-handoff duplication path. The current-main premise is still present: compress() rehydrates a handoff at agent/context_compressor.py:2957-2966, then copies the protected head at agent/context_compressor.py:3063-3073 before adding a new summary.
Problems
agent/context_compressor.py:1754assumes a merged handoff has tail text after_SUMMARY_END_MARKER. Current main deliberately writes merged content as prior tail text,_MERGED_SUMMARY_DELIMITER, summary, then the end marker (agent/context_compressor.py:3176-3189,a1a8a967e). This helper would therefore returnNoneand discard the preserved tail message.- The added merged-prefix test covers only that older ordering, not the current delimiter-based format recognized by
_strip_summary_prefix()atagent/context_compressor.py:2233-2249.
Suggested changes
- Parse the current merged-tail delimiters and preserve the tail segment while removing the old embedded summary/metadata.
- Add a recompression test against that current serialization, asserting preserved tail text and exactly one fresh handoff.
Automated hermes-sweeper review.
| return message.copy() | ||
|
|
||
| if isinstance(content, str): | ||
| marker_idx = content.find(_SUMMARY_END_MARKER) |
There was a problem hiding this comment.
Current main serializes merged tail summaries as prior tail content + _MERGED_SUMMARY_DELIMITER + summary + this end marker (agent/context_compressor.py:3176-3189, a1a8a967e). This therefore has no remainder and returns None, dropping the preserved tail message. Parse the current delimiters and retain the tail segment instead.
86edada to
0551744
Compare
|
Addressed the current-main review in You were right about the serialization mismatch: the previous helper assumed real tail text followed The revised cleanup now:
The new current-format regression failed on clean |
The summary_idx head-copy skip (from #69302) dropped the entire merged handoff message, deleting the genuine prior-tail user content that #47274's _strip_context_summary_handoff_message correctly unwraps. Strip handles both shapes: standalone handoffs drop, merged handoffs keep their real content. Caught by test_recompression_of_current_merged_handoff_preserves_prior_tail_once when both PRs landed together.
The summary_idx head-copy skip (from #69302) dropped the entire merged handoff message, deleting the genuine prior-tail user content that #47274's _strip_context_summary_handoff_message correctly unwraps. Strip handles both shapes: standalone handoffs drop, merged handoffs keep their real content. Caught by test_recompression_of_current_merged_handoff_preserves_prior_tail_once when both PRs landed together.
…he decay scan Composing #57835's multi-fossil summary scan with #47274's merged-handoff unwrap: when the restart-decay path pulls a merged handoff into the compression window, its genuine prior-tail user content must enter the summarizer input (folded into the fresh summary) rather than being dropped with the summary row. Standalone handoffs still drop. The continuity test now pins the composed contract: recovered verbatim OR via summarizer input, never silently deleted, never duplicated.
…he decay scan Composing #57835's multi-fossil summary scan with #47274's merged-handoff unwrap: when the restart-decay path pulls a merged handoff into the compression window, its genuine prior-tail user content must enter the summarizer input (folded into the fresh summary) rather than being dropped with the summary row. Standalone handoffs still drop. The continuity test now pins the composed contract: recovered verbatim OR via summarizer input, never silently deleted, never duplicated.
The summary_idx head-copy skip (from NousResearch#69302) dropped the entire merged handoff message, deleting the genuine prior-tail user content that NousResearch#47274's _strip_context_summary_handoff_message correctly unwraps. Strip handles both shapes: standalone handoffs drop, merged handoffs keep their real content. Caught by test_recompression_of_current_merged_handoff_preserves_prior_tail_once when both PRs landed together.
…he decay scan Composing NousResearch#57835's multi-fossil summary scan with NousResearch#47274's merged-handoff unwrap: when the restart-decay path pulls a merged handoff into the compression window, its genuine prior-tail user content must enter the summarizer input (folded into the fresh summary) rather than being dropped with the summary row. Standalone handoffs still drop. The continuity test now pins the composed contract: recovered verbatim OR via summarizer input, never silently deleted, never duplicated.
The summary_idx head-copy skip (from NousResearch#69302) dropped the entire merged handoff message, deleting the genuine prior-tail user content that NousResearch#47274's _strip_context_summary_handoff_message correctly unwraps. Strip handles both shapes: standalone handoffs drop, merged handoffs keep their real content. Caught by test_recompression_of_current_merged_handoff_preserves_prior_tail_once when both PRs landed together.
…he decay scan Composing NousResearch#57835's multi-fossil summary scan with NousResearch#47274's merged-handoff unwrap: when the restart-decay path pulls a merged handoff into the compression window, its genuine prior-tail user content must enter the summarizer input (folded into the fresh summary) rather than being dropped with the summary row. Standalone handoffs still drop. The continuity test now pins the composed contract: recovered verbatim OR via summarizer input, never silently deleted, never duplicated.
Summary
Fixes a recompression path where a persisted
[CONTEXT COMPACTION]handoff can survive in the protected head/tail while the compressor also emits a fresh handoff. Repeated compression then carries duplicate handoff blocks forward instead of keeping only the folded summary.The fix removes stale handoff-only messages during compression assembly. When a handoff was merged into a real tail message, it removes only the stale summary wrapper and preserves the original tail content.
Current merged-handoff format
Current
mainserializes merged handoffs as:_MERGED_PRIOR_CONTEXT_HEADER_MERGED_SUMMARY_DELIMITER_SUMMARY_END_MARKERThe cleanup now parses that ordering and preserves everything before
_MERGED_SUMMARY_DELIMITERexcept the wrapper header. It supports both string content and multimodal content blocks. Persisted legacy handoffs using the oldersummary + end marker + real tailordering remain supported as well.Why nearby compaction fixes do not cover this
This is related to the same broad compaction-handoff symptom family as #6212, #42830, #42895, and #43184, but it is a different root cause:
Changes
_strip_context_summary_handoff_message()to drop stale handoff-only messages.Validation
RED/GREEN against current
origin/main(bb5fc723b):CURRENT-MERGED-OLD-SUMMARYremained in the compressed output.Commands run: