From 8f4d195d5f5aa3ba8fc89ee3cdd2dd4b41c7d582 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Wed, 1 Jul 2026 14:11:59 +0530 Subject: [PATCH] fix(compressor): pin summary role to user when only system prompt is protected (#52160) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the first compaction protect_first_n decays, so on a later compaction the only protected head message can be the system prompt. Adapters like Anthropic and Bedrock send the system prompt as a separate parameter, so the summary becomes the first message in messages[] — and Anthropic rejects any request whose first message is not role=user (HTTP 400). Pin the summary to role=user when the head is system-only, and stop the collision-flip logic from reverting it back to assistant. Salvaged from #52167. Co-authored-by: liuhao1024 --- agent/context_compressor.py | 12 ++- tests/agent/test_context_compressor.py | 100 +++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 2 deletions(-) diff --git a/agent/context_compressor.py b/agent/context_compressor.py index 5a555a7f17c..0a5574e8e85 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -2818,9 +2818,17 @@ This compaction should PRIORITISE preserving all information related to the focu _merge_summary_into_tail = False last_head_role = messages[compress_start - 1].get("role", "user") if compress_start > 0 else "user" first_tail_role = messages[compress_end].get("role", "user") if compress_end < n_messages else "user" + # When the only protected head message is the system prompt, the + # summary becomes the first *visible* message in the API request + # (most adapters — Anthropic, Bedrock — send the system prompt as + # a separate ``system`` parameter, not inside ``messages[]``). + # Anthropic unconditionally rejects requests whose first message + # is not role=user, so we must pin the summary to "user" and + # prevent the flip logic below from reverting it (#52160). + _force_user_leading = last_head_role == "system" # Pick a role that avoids consecutive same-role with both neighbors. # Priority: avoid colliding with head (already committed), then tail. - if last_head_role in {"assistant", "tool"}: + if last_head_role in {"assistant", "tool"} or _force_user_leading: summary_role = "user" else: summary_role = "assistant" @@ -2828,7 +2836,7 @@ This compaction should PRIORITISE preserving all information related to the focu # collide with the head, flip it. if summary_role == first_tail_role: flipped = "assistant" if summary_role == "user" else "user" - if flipped != last_head_role: + if flipped != last_head_role and not _force_user_leading: summary_role = flipped else: # Both roles would create consecutive same-role messages diff --git a/tests/agent/test_context_compressor.py b/tests/agent/test_context_compressor.py index f87681b86ef..bfb749122a4 100644 --- a/tests/agent/test_context_compressor.py +++ b/tests/agent/test_context_compressor.py @@ -3104,3 +3104,103 @@ class TestCooldownReentryAbort: ) assert c._last_compress_aborted is True assert c._last_summary_fallback_used is False + + + + +class TestDoubleCompactionSummaryRole: + """PR #52160 (salvaged from #52167): when only the system prompt is + protected, the summary must lead with role=user (Anthropic/Bedrock send + system as a separate param, so the summary is the first visible message).""" + + def test_double_compaction_summary_must_be_user_when_only_system_protected(self): + """After the first compression, protect_first_n decays to 0. + + On the second compression the only protected head message is the + system prompt (role=system). The summary becomes the first + *visible* message in the API request because adapters like + Anthropic and Bedrock send the system prompt as a separate + ``system`` parameter. The summary MUST be role=user or the + provider rejects with HTTP 400 (#52160). + """ + mock_response = MagicMock() + mock_response.choices = [MagicMock()] + mock_response.choices[0].message.content = "summary of earlier turns" + + with patch("agent.context_compressor.get_model_context_length", return_value=100000): + c = ContextCompressor( + model="test", quiet_mode=True, protect_first_n=2, protect_last_n=2, + ) + # Simulate second compression: protect_first_n decays to 0. + c.compression_count = 1 + + # compress_start will be 1 (system only), last_head_role = "system". + # Without the fix, summary_role would be "assistant". + msgs = [ + {"role": "system", "content": "You are a helpful assistant."}, + {"role": "user", "content": "msg 1"}, + {"role": "assistant", "content": "msg 2"}, + {"role": "user", "content": "msg 3"}, + {"role": "assistant", "content": "msg 4"}, + {"role": "user", "content": "msg 5"}, + {"role": "assistant", "content": "msg 6"}, + ] + with patch("agent.context_compressor.call_llm", return_value=mock_response): + result = c.compress(msgs) + + # The system message must still be at index 0. + assert result[0]["role"] == "system" + # The summary (first non-system message) must be role=user. + non_system = [m for m in result if m.get("role") != "system"] + assert non_system, "expected at least one non-system message" + assert non_system[0]["role"] == "user", ( + f"first non-system message must be role=user for Anthropic " + f"compatibility, got role={non_system[0]['role']!r}" + ) + + def test_double_compaction_user_tail_merges_into_tail(self): + """When the summary is forced to role=user (system-only head) and + the first tail message is also user, the summary must merge into + the tail rather than flipping back to assistant (#52160). + """ + mock_response = MagicMock() + mock_response.choices = [MagicMock()] + mock_response.choices[0].message.content = "summary of earlier turns" + + with patch("agent.context_compressor.get_model_context_length", return_value=100000): + c = ContextCompressor( + model="test", quiet_mode=True, protect_first_n=2, protect_last_n=2, + ) + c.compression_count = 1 # decay protect_first_n + + # tail starts with user → would collide with forced summary_role=user. + # The fix should merge into tail instead of flipping to assistant. + msgs = [ + {"role": "system", "content": "You are a helpful assistant."}, + {"role": "user", "content": "msg 1"}, + {"role": "assistant", "content": "msg 2"}, + {"role": "user", "content": "msg 3"}, + {"role": "assistant", "content": "msg 4"}, + {"role": "user", "content": "msg 5"}, # tail start (user) + {"role": "assistant", "content": "msg 6"}, + {"role": "user", "content": "msg 7"}, + ] + with patch("agent.context_compressor.call_llm", return_value=mock_response): + result = c.compress(msgs) + + # No standalone summary message should exist (merged into tail). + summary_msgs = [ + m for m in result + if m.get("_compressed_summary") and "msg 5" not in (m.get("content") or "") + ] + assert len(summary_msgs) == 0, ( + "summary should be merged into tail, not standalone" + ) + # The first non-system message must be role=user. + non_system = [m for m in result if m.get("role") != "system"] + assert non_system[0]["role"] == "user" + # The merged tail should contain the summary text. + assert any( + "summary of earlier turns" in (m.get("content") or "") + for m in result + )