From bb2be8a901376bf5e09c8e6294e165de99db1777 Mon Sep 17 00:00:00 2001 From: 47chan <47chan@hermes> Date: Sat, 9 May 2026 19:35:03 +0800 Subject: [PATCH] fix(compressor): prevent orphan user turn after compaction via turn-pair preservation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When _ensure_last_user_message_in_tail pulls cut_idx back to include the last user message, the call-site in _find_tail_cut_by_tokens still applies max(cut_idx, head_end + 1). When the user message sits at or near head_end this max() pushes the cut *past* the user, leaving it in the compressed region without its assistant reply. The LLM summariser then sees an unanswered request and marks it as 'Pending User Asks'. The next session's model sees the dangling user turn after the summary, treats it as a new instruction, and re-executes the already-completed task (e.g. turning lights off again, deleting a file twice, re-sending a message). Fix: apply the Causal Coupling principle — a compaction boundary must never split a (user → assistant [→ tool results]) turn-pair. Both halves must land on the same side of the cut. Implement this via two additions: 1. _find_turn_pair_end(messages, user_idx): returns the index after the complete turn-pair (user + assistant + any tool results), so the caller can locate the natural cut point that keeps the pair intact. 2. Causal Coupling guard in _ensure_last_user_message_in_tail: detects when the adjusted cut would land between the user and its reply, then pushes cut_idx to pair_end so the whole pair is summarised together and correctly marked as completed in the summary. The existing 81 compressor tests all pass unchanged. Eight new tests in TestTurnPairPreservation cover _find_turn_pair_end (4 unit tests) and _ensure_last_user_message_in_tail (4 tests including an end-to-end no-orphan invariant). --- agent/context_compressor.py | 62 +++++++++-- tests/agent/test_context_compressor.py | 144 +++++++++++++++++++++++++ 2 files changed, 198 insertions(+), 8 deletions(-) diff --git a/agent/context_compressor.py b/agent/context_compressor.py index dfb93b30aa3f..def7d42c0bf1 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -1190,7 +1190,7 @@ def _ensure_last_user_message_in_tail( cut_idx: int, head_end: int, ) -> int: - """Guarantee the most recent user message is in the protected tail. + """Guarantee the most recent user message (and its reply) land on the same side of the cut. Context compressor bug (#10896): ``_align_boundary_backward`` can pull ``cut_idx`` past a user message when it tries to keep tool_call/result @@ -1201,10 +1201,18 @@ def _ensure_last_user_message_in_tail( the active context, causing the agent to stall, repeat completed work, or silently drop the user's latest request. - Fix: if the last user-role message is not already in the tail - (``messages[cut_idx:]``), walk ``cut_idx`` back to include it. We - then re-align backward one more time to avoid splitting any - tool_call/result group that immediately precedes the user message. + Fix: + 1. If the last user-role message is not already in the tail, pull + ``cut_idx`` back to include it. + 2. Causal Coupling guard: ``_find_tail_cut_by_tokens`` applies + ``max(cut_idx, head_end + 1)`` at its call site, which can push the + cut *past* the user message when ``last_user_idx == head_end``. + This splits the turn-pair — user lands in the compressed region + without its assistant reply, so the LLM summariser sees an + unanswered ask and marks it as pending, causing re-execution in the + next session. When this split is detected, push the cut *forward* + to ``pair_end`` so the complete pair (user + reply + tool results) + is summarised together and correctly marked as completed. """ last_user_idx = self._find_last_user_message_idx(messages, head_end) if last_user_idx < 0: @@ -1212,7 +1220,9 @@ def _ensure_last_user_message_in_tail( return cut_idx if last_user_idx >= cut_idx: - # Already in the tail; nothing to do. + # User message (and its subsequent reply) are already in the tail. + # Everything from cut_idx onwards is preserved, so the turn-pair + # is intact — nothing to do. return cut_idx # The last user message is in the middle (compressed) region. @@ -1228,8 +1238,44 @@ def _ensure_last_user_message_in_tail( last_user_idx, cut_idx, ) - # Safety: never go back into the head region. - return max(last_user_idx, head_end + 1) + + # Causal Coupling guard: detect whether the call-site's final + # ``max(cut_idx, head_end + 1)`` would push the cut *past* the user + # message, splitting the turn-pair. When a split is inevitable, + # push the cut to ``pair_end`` so the full pair lands in the + # compressed region and is summarised as a completed unit. + adjusted = max(last_user_idx, head_end + 1) + if adjusted > last_user_idx: + # User would end up in the compressed region; include its reply too. + pair_end = self._find_turn_pair_end(messages, last_user_idx) + return max(pair_end, head_end + 1) + return adjusted + + 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. + + 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 def _find_tail_cut_by_tokens( self, messages: List[Dict[str, Any]], head_end: int, diff --git a/tests/agent/test_context_compressor.py b/tests/agent/test_context_compressor.py index 572ebce12fa5..4cd6f4016bad 100644 --- a/tests/agent/test_context_compressor.py +++ b/tests/agent/test_context_compressor.py @@ -1533,3 +1533,147 @@ def test_pass3_emits_valid_json_for_downstream_provider(self): parsed = _json.loads(shrunk) assert parsed["path"] == "~/.hermes/skills/shopping/browser-setup-notes.md" assert parsed["content"].endswith("...[truncated]") + + +class TestTurnPairPreservation: + """Causal Coupling guard: compaction must never create an orphan user turn. + + The original #10896 fix pulled cut_idx back to include the last user + message in the tail. However ``_find_tail_cut_by_tokens`` applies + ``max(cut_idx, head_end + 1)`` at its call site, which can push the + boundary *forward* past the user message again when the user message sits + at or near ``head_end``. The result: user lands in the compressed region + without its assistant reply, the summariser marks it as pending, and the + next session re-executes the already-completed request. + + These tests verify the Causal Coupling guard in + ``_ensure_last_user_message_in_tail`` and ``_find_turn_pair_end``. + """ + + @pytest.fixture + def compressor(self): + return ContextCompressor( + model="test/model", + threshold_percent=0.85, + protect_first_n=1, + protect_last_n=0, + quiet_mode=True, + ) + + # ------------------------------------------------------------------ + # _find_turn_pair_end unit tests + # ------------------------------------------------------------------ + + def test_pair_end_user_only(self, compressor): + """User at end of list — no reply yet — pair_end is user+1.""" + msgs = [ + {"role": "user", "content": "hello"}, + ] + assert compressor._find_turn_pair_end(msgs, 0) == 1 + + def test_pair_end_user_with_assistant_reply(self, compressor): + """User + assistant — pair_end skips both.""" + msgs = [ + {"role": "user", "content": "do x"}, + {"role": "assistant", "content": "done"}, + ] + assert compressor._find_turn_pair_end(msgs, 0) == 2 + + def test_pair_end_user_assistant_with_tools(self, compressor): + """User + assistant + tool results — pair_end skips the whole group.""" + msgs = [ + {"role": "user", "content": "run it"}, + {"role": "assistant", "content": None, + "tool_calls": [{"function": {"name": "exec", "arguments": "{}"}}]}, + {"role": "tool", "tool_call_id": "c1", "content": "ok"}, + {"role": "tool", "tool_call_id": "c2", "content": "ok"}, + ] + assert compressor._find_turn_pair_end(msgs, 0) == 4 + + def test_pair_end_stops_at_next_user(self, compressor): + """pair_end must not cross into the next user turn.""" + msgs = [ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "reply"}, + {"role": "user", "content": "second"}, + ] + assert compressor._find_turn_pair_end(msgs, 0) == 2 + + # ------------------------------------------------------------------ + # _ensure_last_user_message_in_tail unit tests + # ------------------------------------------------------------------ + + def test_user_already_in_tail_unchanged(self, compressor): + """When the user message is already past cut_idx, nothing changes.""" + msgs = [ + {"role": "user", "content": "head"}, + {"role": "assistant", "content": "head reply"}, + {"role": "user", "content": "last user"}, + {"role": "assistant", "content": "last reply"}, + ] + # cut_idx=2 → tail = msgs[2:], last user is at 2 → already in tail + result = compressor._ensure_last_user_message_in_tail(msgs, cut_idx=2, head_end=1) + assert result == 2 + + def test_user_in_compressed_region_pulled_back(self, compressor): + """User that ended up in the compressed region triggers cut pullback.""" + msgs = [ + {"role": "user", "content": "head"}, # 0 + {"role": "assistant", "content": "hi"}, # 1 + {"role": "user", "content": "do thing"}, # 2 ← last user + {"role": "assistant", "content": "done"}, # 3 + ] + # cut_idx=3 puts user at idx 2 in compressed region (2 < 3) + result = compressor._ensure_last_user_message_in_tail(msgs, cut_idx=3, head_end=0) + # Should pull cut back to ≤ 2 so user is in tail + assert result <= 2 + + def test_orphan_prevention_user_in_compressed_region(self, compressor): + """Regression: user answered in-session ends up in compressed region. + + Scenario: last user is at index 4, cut_idx=5 (token walk stopped there). + _ensure finds last_user_idx=4 < cut_idx=5 → pulls back. + The resulting tail must include the user at 4. + """ + msgs = [ + {"role": "user", "content": "q1"}, # 0 + {"role": "assistant", "content": "a1"}, # 1 + {"role": "user", "content": "q2"}, # 2 + {"role": "assistant", "content": "a2"}, # 3 + {"role": "user", "content": "turn lights off"}, # 4 ← last user + {"role": "assistant", "content": "lights off done"}, # 5 + ] + head_end = 3 + result = compressor._ensure_last_user_message_in_tail(msgs, cut_idx=5, head_end=head_end) + # User at 4 < cut_idx=5 → must pull cut back to ≤ 4 + assert result <= 4, ( + f"Expected cut ≤ 4 to keep user at idx 4 in tail, got {result}" + ) + + def test_no_orphan_after_full_compaction_cycle(self, compressor): + """End-to-end: after _find_tail_cut_by_tokens, no orphan user turns exist. + + Build a conversation where the last user message is answered and the + conversation has enough history to trigger compression. Verify that the + tail never starts with an unanswered user message. + """ + msgs = [ + {"role": "user", "content": "initial"}, + {"role": "assistant", "content": "ok"}, + ] + for i in range(5): + msgs.append({"role": "user", "content": f"step {i}"}) + msgs.append({"role": "assistant", "content": f"done {i}"}) + msgs.append({"role": "user", "content": "lights off please"}) + msgs.append({"role": "assistant", "content": "lights are off"}) + + head_end = compressor.protect_first_n + cut = compressor._find_tail_cut_by_tokens(msgs, head_end) + tail = msgs[cut:] + + # Tail must not start with an unanswered user message + if tail and tail[0].get("role") == "user": + assert len(tail) >= 2 and tail[1].get("role") == "assistant", ( + f"Orphan user turn detected at tail start: {tail[0]['content']!r} " + f"— next role is {tail[1].get('role') if len(tail) > 1 else 'nothing'}" + )