Skip to content

fix(agent): keep compaction summaries as user role (#42768) - #42830

Closed
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/42768-compaction-role
Closed

fix(agent): keep compaction summaries as user role (#42768)#42830
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/42768-compaction-role

Conversation

@Tranquil-Flow

Copy link
Copy Markdown
Contributor

What changed

Fixes #42768 by making context compaction handoff summaries consistently behave as user-provided background context instead of assistant output.

  • Always inserts standalone compaction summaries with role="user".
  • Always appends the explicit --- END OF CONTEXT SUMMARY ... --- marker, closing the assistant-role path left uncovered by fix(compaction): mark end of context summary in role=user fallback (salvage #17121) #20169.
  • Counts deterministic fallback summaries as ineffective compression attempts so the anti-thrashing guard can stop retry loops after fallback summary generation.
  • Updates context-compressor regression tests to cover user-role summaries, end markers, structured/list tail content, and deterministic fallback anti-thrashing.

Why

The issue reports sessions where compaction summaries leaked into chat as visible assistant responses. The root cause is that the old role-alternation logic could select or merge the handoff into an assistant message. Some models then treated that handoff as their own prior output and echoed it verbatim, causing repeated compaction loops.

The safer invariant is: compaction handoffs are background context from the user side, never assistant output. Consecutive same-role user messages are tolerated by supported providers and are less dangerous than representing the handoff as an assistant response.

Competitor check

Open competing PRs were reviewed before publication:

This PR covers both reported layers with regression tests.

Verification

Run locally from the worktree with Python 3.11:

/Users/evinova-self/.hermes/hermes-agent/venv/bin/python3 -m pytest tests/agent/test_context_compressor.py -v -o "addopts=" --tb=short

Result: 97 passed, 1 warning in 3.77s.

Auto-published by Moonsong via Path B automated pipeline.

@Tranquil-Flow
Tranquil-Flow force-pushed the fix/42768-compaction-role branch from dd374ad to 7b29a5b Compare June 9, 2026 12:31
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Jun 9, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

✅ Verified — Compaction summary role simplification looks solid

Reviewed the full diff across agent/context_compressor.py and tests/agent/test_context_compressor.py.

Changes verified:

  • Summary role is now unconditionally user, eliminating the role-flipping and merge-into-tail paths. The rationale (preventing assistant-role echo loops from [Bug]: Context compaction summaries leak into chat as assistant responses when inserted with role="assistant" #42768) is well-motivated — sacrificing role alternation to avoid a compaction loop is the correct tradeoff.
  • The end marker (END OF CONTEXT SUMMARY — respond to the message below, not the summary above) is always appended, which is the right behavior regardless of role.
  • Anti-thrashing improvement: fallback_summary_used correctly advances _ineffective_compression_count even when the fallback saves >10% tokens, preventing the compressor from thrashing on repeated LLM failures.
  • All 5 modified test methods correctly update assertions from assistantuser and remove the alternation checks that no longer apply.

No issues found. The simplification removes ~40 lines of complex role-collision logic while making the behavior more predictable. Clean contribution.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jun 21, 2026
@teknium1

teknium1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for the thorough writeup and competitor comparison, Evi — the root-cause analysis on both bugs was correct.

Closing this as superseded / already fixed on main. Since this branch was opened (it's ~2793 commits behind), agent/context_compressor.py saw heavy churn and every piece of the reported bug is now handled:

  1. Always-append END OF CONTEXT SUMMARY marker (your Bug 1, the primary leak) — fixed by fix(compression): append END OF CONTEXT SUMMARY marker to all standalone summaries (salvages #33346) #45153. The marker is now appended unconditionally to standalone summaries; the old summary_role == "user" guard that left the assistant-role path uncovered is gone.

  2. Compaction-loop / role selection — addressed by bug(adapter): HTTP 400 after double context compression — first message is assistant, not user #52160 (batch fix(compressor): batch salvage \u2014 orphan tool_calls, cooldown abort flags, system-head summary role (#51225 #52056 #52167) #56197). Rather than always forcing role="user", main keeps the head/tail-aware alternation and force-pins user only when the protected head is the system prompt (the case Anthropic rejects). Forcing user in every case would create consecutive user → user messages when the head is a user turn, which the current logic specifically avoids — so we can't take the unconditional pin without re-breaking that alternation invariant.

  3. Deterministic-fallback anti-thrashing (your Bug 2) — the ineffective-compression guard already increments on the fallback path via savings_pct < 10 (your repro's 249→224 is <10% savings, so it's caught). The extra fallback_summary_used or flag only diverges when a fallback did achieve ≥10% savings, in which case counting it ineffective wouldn't be correct.

tests/agent/test_context_compressor.py on current main is green (141 passed) and covers the always-user-with-marker behavior.

Really appreciate the contribution and the honest scoring of the competing PRs — the diagnosis here directly matches what shipped. 🙏

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

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Context compaction summaries leak into chat as assistant responses when inserted with role="assistant"

4 participants