fix(compressor): prevent orphan user turn after compaction (turn-pair preservation) - #200
fix(compressor): prevent orphan user turn after compaction (turn-pair preservation)#200hashbender wants to merge 1 commit into
Conversation
|
Review Complete Files Reviewed: 3 By Severity:
A bug in the new Files Reviewed (3 files) |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: 🟠 High (62/100) — 1 medium finding · 194 LOC across 3 files
Summary
PR #200 introduces a Causal Coupling guard in agent/context_compressor.py that ensures the compaction boundary respects complete user→assistant→tool turn-pairs. However, the new helper _find_turn_pair_end (line 2406–2431) only consumes the first assistant message and its immediate tool results, stopping at the next non-tool role.
Issue
In real Hermes agent loops, a single user turn routinely triggers multiple rounds of model calls: user → assistant(tool_calls) → tool → assistant(tool_calls) → tool → … → assistant(text). The current implementation returns after the first assistant→tool cycle, splitting a multi-step turn across the compaction boundary. This causes:
- The compression summary sees only a partial turn (user asked + first tool result, no completion)
- Remaining mid-turn messages land in the tail without user context
- The model on next session may see orphaned assistant/tool messages with no preceding user instruction
Fix
Rewrite _find_turn_pair_end to walk forward until the next user-role message (the natural turn boundary), capturing all assistant and tool messages belonging to the same user turn. A test case for multi-step tool-call turns should be added to tests/agent/test_context_compressor.py.
| def _find_turn_pair_end( | ||
| self, | ||
| messages: List[Dict[str, Any]], | ||
| user_idx: int, | ||
| ) -> int: | ||
| """Return the index *after* the complete turn-pair starting at *user_idx*. | ||
|
|
||
| A turn-pair is: ``user`` -> ``assistant`` [-> zero-or-more ``tool`` | ||
| results]. Returns the index of the first message that does *not* | ||
| belong to the pair, i.e. the natural cut point that keeps the pair | ||
| intact on one side of the boundary. | ||
|
|
||
| If *user_idx* is the last message (no assistant reply yet), returns | ||
| ``user_idx + 1`` so the user message itself is minimally covered. | ||
| """ | ||
| n = len(messages) | ||
| idx = user_idx + 1 | ||
| if idx >= n: | ||
| return idx # user is the very last message — no reply yet | ||
| if messages[idx].get("role") != "assistant": | ||
| return idx # no assistant reply immediately following | ||
| idx += 1 | ||
| # Include any tool results that belong to this assistant turn. | ||
| while idx < n and messages[idx].get("role") == "tool": | ||
| idx += 1 | ||
| return idx |
There was a problem hiding this comment.
🟡 _find_turn_pair_end truncates multi-step tool-call turns, leaking mid-turn messages into the tail without user context (bug)
The new _find_turn_pair_end helper (agent/context_compressor.py:2406-2431) is called by the Causal Coupling guard in _ensure_last_user_message_in_tail (line 2394) to push the compaction boundary forward past the complete user→assistant→tool turn-pair when the user sits at the head boundary and cannot be pulled into the tail. However, the function only walks forward past the FIRST assistant message and its immediate consecutive tool results, then stops at the next non-tool role. This misses subsequent assistant(tool_calls)→tool(result) cycles and the assistant's final text reply, all of which are part of the same user turn. In real Hermes agent loops, a single user message commonly triggers multiple rounds of tool calls (the agent loops: call model → get tool_calls → execute → add tool results → call model again → … → get text reply). When the Causal Coupling guard fires with such a multi-step turn, the truncated pair_end causes: (1) only the first segment enters the compression summary (so the summarizer sees user asked + assistant started work + first tool result, but no completion); (2) the remaining mid-turn messages (subsequent assistant tool_calls + results + final text reply) land in the tail without their originating user context. The model on the next session may see mid-turn messages with no user instruction, causing confusion or re-execution of already-completed work.
💡 Suggestion: Rewrite _find_turn_pair_end to walk forward until the next user-role message (or end of list), which is the natural boundary between turns. This captures the complete multi-step turn: all assistant and tool messages belonging to the same user message stay together on one side of the compaction cut.
📋 Prompt for AI Agents
In agent/context_compressor.py, replace the body of _find_turn_pair_end (lines 2421-2431) with a loop that walks forward past ALL non-user messages: idx = user_idx + 1; while idx < n and messages[idx].get('role') != 'user': idx += 1; return idx. Update the docstring to document multi-step tool-call handling: change 'A turn-pair is: user -> assistant [-> zero-or-more tool results]' to describe the full agent-loop pattern including multiple assistant→tool cycles. Add a test case in tests/agent/test_context_compressor.py (TestTurnPairPreservation class) for a multi-step turn: user → assistant(tool_calls) → tool → assistant(tool_calls) → tool → assistant(text) → next_user, verifying pair_end returns the index of next_user.
1 participant
Summary
Compaction no longer orphans the last user turn. When the last user message sits exactly at
head_end(the first compressible index), the completed turn-pair is summarised as a unit instead of being split — so the summariser stops recording a finished ask as "pending" and the next session stops re-executing already-done work.Root cause:
_ensure_last_user_message_in_tail's finalmax(last_user_idx, head_end + 1)clamp returnshead_end + 1whenlast_user_idx == head_end, pushing the user past the cut into the compressed region without its assistant reply.Salvages NousResearch#22523 by @H2KFORGIVEN — reimplemented on current
main(the original branch was ~6000 commits stale; the code moved from line ~1190 to ~2332).Changes
agent/context_compressor.py: add_find_turn_pair_end()and a Causal Coupling guard in_ensure_last_user_message_in_tail— when the clamp would orphan the user, push the cut forward topair_endso(user → assistant → tool results)lands together in the summary.tests/agent/test_context_compressor.py: 8 new tests inTestTurnPairPreservation(4 for_find_turn_pair_end, 4 for the anchor incl. an end-to-end no-orphan invariant).scripts/release.py: AUTHOR_MAP entry for @H2KFORGIVEN.Validation
E2E-verified on current
main: the orphan reproduces exactly as described (returnshead_end+1, user pushed out of the tail); the fix keeps the completed pair together and the tail starts on an assistant reply. NousResearch#10896's mid-conversation pullback is unchanged (guard only fires whenlast_user_idx == head_end).head_end, cut>head_endhead_end+1→ user orphaned in summarypair_end→ user+reply summarised togetheruseraskassistantreplyInfographic
Mirror-of: NousResearch#56121
NousResearch#56121