From fc2fac73bd1a843b9ba7737b6396fa9b01156a8f Mon Sep 17 00:00:00 2001 From: H2KFORGIVEN <22971845+H2KFORGIVEN@users.noreply.github.com> Date: Wed, 1 Jul 2026 00:14:10 -0700 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 the last user message sits exactly at head_end (the first compressible index), _ensure_last_user_message_in_tail's final max(last_user_idx, head_end + 1) clamp returns head_end + 1, pushing the user into the compressed region without its assistant reply. The summariser then records it as a pending ask, and the next session re-executes the already-completed task (lights off twice, file deleted twice, message re-sent). Fix: apply Causal Coupling — a compaction boundary must never split a (user -> assistant [-> tool results]) turn-pair. Add _find_turn_pair_end and, when the clamp would orphan the user, push the cut forward to pair_end so the completed pair is summarised together and marked done. 8 new tests in TestTurnPairPreservation; 133 compressor tests pass. --- agent/context_compressor.py | 56 +++++++++- tests/agent/test_context_compressor.py | 137 +++++++++++++++++++++++++ 2 files changed, 192 insertions(+), 1 deletion(-) diff --git a/agent/context_compressor.py b/agent/context_compressor.py index 3b37af7b8ba..6859a28a0ea 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -2350,6 +2350,17 @@ This compaction should PRIORITISE preserving all information related to the focu (``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. + + Causal Coupling guard (#22523): the final ``max(last_user_idx, + head_end + 1)`` clamp can push the cut *past* the user message when + the user sits at ``head_end`` (the first compressible index) — the + only case where ``head_end + 1 > last_user_idx``. That splits the + turn-pair: the user lands in the compressed region without its + assistant reply, so the summariser records it as a pending ask and + the next session re-executes the already-completed task. When this + split is unavoidable, push the cut *forward* to ``pair_end`` so the + full 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: @@ -2374,7 +2385,50 @@ This compaction should PRIORITISE preserving all information related to the focu cut_idx, ) # Safety: never go back into the head region. - return max(last_user_idx, head_end + 1) + adjusted = max(last_user_idx, head_end + 1) + if adjusted > last_user_idx: + # The clamp would leave the user in the compressed region without + # its reply. Keep the pair intact by pushing the cut forward past + # the whole (user + assistant + tool results) turn-pair so it is + # summarised as a completed unit rather than a dangling ask. + pair_end = self._find_turn_pair_end(messages, last_user_idx) + if not self.quiet_mode: + logger.debug( + "Causal Coupling: cut would split turn-pair at user %d; " + "pushing cut forward to pair_end %d so the completed pair " + "is summarised together (#22523)", + last_user_idx, + pair_end, + ) + 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 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 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 cd23d13480c..fe0bf3b4b5e 100644 --- a/tests/agent/test_context_compressor.py +++ b/tests/agent/test_context_compressor.py @@ -2782,3 +2782,140 @@ class TestPreflightSentinelGuard: compressor.last_prompt_tokens = 50_000 result = self._seed(compressor.last_prompt_tokens, 10_000) assert result == 50_000 + + +class TestTurnPairPreservation: + """Causal Coupling guard (#22523): compaction must never orphan a user turn. + + ``_ensure_last_user_message_in_tail`` pulls the cut back to keep the last + user message in the tail (fixes #10896). But its final + ``max(last_user_idx, head_end + 1)`` clamp pushes the cut *past* the user + when the user sits at ``head_end`` (the first compressible index) — the + only case where ``head_end + 1 > last_user_idx``. The user then lands in + the compressed region without its assistant reply; the summariser marks it + as a pending ask and the next session re-executes the completed task. + + The guard detects that split and pushes the cut forward to ``pair_end`` so + the complete (user -> assistant [-> tool results]) pair is summarised as a + finished unit. + """ + + @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"}, + ] + 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 in the middle (not at head_end) is pulled into the tail (#10896).""" + msgs = [ + {"role": "user", "content": "head"}, # 0 + {"role": "assistant", "content": "hi"}, # 1 + {"role": "user", "content": "do thing"}, # 2 <- last user + {"role": "assistant", "content": "done"}, # 3 + ] + # head_end=0, so head_end+1=1 <= last_user_idx=2: the #10896 pullback + # applies and the user stays in the tail (no forward push). + result = compressor._ensure_last_user_message_in_tail(msgs, cut_idx=3, head_end=0) + assert result <= 2 + + def test_orphan_prevented_user_at_head_end(self, compressor): + """Causal Coupling: user at head_end pushes the WHOLE pair into the summary. + + This is the #22523 case: last_user_idx == head_end, so the clamp would + return head_end+1 and orphan the user. The guard instead pushes the + cut forward to pair_end so user + reply + tool results are summarised + together and the tail never starts with a dangling user ask. + """ + msgs = [ + {"role": "user", "content": "first exchange"}, # 0 head + {"role": "user", "content": "THE ACTIVE ASK"}, # 1 = head_end, last user + {"role": "assistant", "content": "done"}, # 2 reply + {"role": "tool", "tool_call_id": "c1", "content": "toolout"}, # 3 + {"role": "assistant", "content": "final reply"}, # 4 + ] + head_end = 1 + result = compressor._ensure_last_user_message_in_tail(msgs, cut_idx=3, head_end=head_end) + # Whole pair (indices 1..3) lands in the compressed region; tail starts at 4. + assert result == 4 + tail = msgs[result:] + assert tail and tail[0]["role"] == "assistant" + + def test_no_orphan_after_full_compaction_cycle(self, compressor): + """End-to-end: after _find_tail_cut_by_tokens, 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:] + + if tail and tail[0].get("role") == "user": + assert len(tail) >= 2 and tail[1].get("role") == "assistant", ( + f"Orphan user turn at tail start: {tail[0]['content']!r} — " + f"next role is {tail[1].get('role') if len(tail) > 1 else 'nothing'}" + )