diff --git a/agent/context_compressor.py b/agent/context_compressor.py index b4eaec660a4ea..1a24567d13f17 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -6264,16 +6264,18 @@ This compaction should PRIORITISE preserving all information related to the focu None, ) first_tail_role = None + first_tail_visible_idx: Optional[int] = None if tail_messages: - first_tail_role = next( + first_tail_visible_idx, first_tail_role = next( ( - role - for role in ( - _template_visible_role(m) for m in tail_messages + (idx, role) + for idx, role in ( + (idx, _template_visible_role(m)) + for idx, m in enumerate(tail_messages) ) if role is not None ), - None, + (None, None), ) # When the only protected head message is the system prompt, the # summary becomes the first *visible* message in the API request @@ -6300,11 +6302,27 @@ This compaction should PRIORITISE preserving all information related to the focu # If no user-role message survives in either the protected head or the # preserved tail, the summary MUST carry role="user" so the request # always has at least one user turn. + # + # A bare role check is not enough: the tail's sole surviving user + # turn can be image-only (a screenshot with no caption). The newest + # image-bearing user message is the ``_strip_historical_media`` + # anchor and is kept byte-for-byte, so it never gains a text + # placeholder — its role is "user" but its text content is empty, + # which backends checking for actual query text still reject. Count + # only user messages with non-empty text as "surviving"; when the + # guard fires, the real (never fabricated) summary text lands in a + # role="user" slot, which is always non-empty (falls back to + # ``_build_static_fallback_summary`` above when generation fails). if not _force_user_leading: + def _is_nonempty_user_turn(message: Dict[str, Any]) -> bool: + return message.get("role") == "user" and bool( + _content_text_for_contains(message.get("content")).strip() + ) + _user_survives = any( - message.get("role") == "user" for message in compressed + _is_nonempty_user_turn(message) for message in compressed ) or any( - message.get("role") == "user" for message in tail_messages + _is_nonempty_user_turn(message) for message in tail_messages ) if not _user_survives: _force_user_leading = True @@ -6360,9 +6378,27 @@ This compaction should PRIORITISE preserving all information related to the focu ), }) + # Default merge target: literal tail index 0. For an ordinary + # alternation collision the summary only has to stay *invisible* to + # the template, and a leading template-exempt row (bare tool-call + # assistant message, tool result) is the ideal carrier — it absorbs + # the summary without adding a visible turn, and it leaves the live + # tail user message intact as the model's actual prompt. Retargeting + # to the first template-visible row here would convert that live + # request into the summary carrier for no benefit. + # + # The forced repair path is the exception. There the merge is not + # about alternation but about guaranteeing at least one genuinely + # non-empty role="user" message (an image-only or otherwise + # text-empty surviving user row). An exempt carrier cannot satisfy + # that invariant, so the summary text must land on the + # template-visible row itself. + _merge_target_idx = 0 + if _force_user_leading and first_tail_visible_idx is not None: + _merge_target_idx = first_tail_visible_idx for tail_idx, msg in enumerate(tail_messages): - if _merge_summary_into_tail and tail_idx == 0: - # Merge the summary into the first (post-strip) tail message. + if _merge_summary_into_tail and tail_idx == _merge_target_idx: + # Merge the summary into the tail message that collided. old_content = msg.get("content", "") if _force_user_leading and summary_role == "user": # The summary must be part of the first user-visible diff --git a/tests/agent/test_compressor_zero_user_guard.py b/tests/agent/test_compressor_zero_user_guard.py index 9cf82c265b444..27973befc9f91 100644 --- a/tests/agent/test_compressor_zero_user_guard.py +++ b/tests/agent/test_compressor_zero_user_guard.py @@ -154,3 +154,123 @@ class TestCompressAlwaysKeepsAUserTurn: m.get("content") for m in out if isinstance(m.get("content"), str) ) assert "the latest live user question" in joined + + +def _has_nonempty_user_text(messages: list[dict]) -> bool: + """True if some role="user" message carries non-empty text. + + Deliberately narrower than "any message has text": a non-empty + ``role="assistant"`` summary does not satisfy backends that require an + actual user query, so checking role and content together is the point. + """ + from agent.context_compressor import _content_text_for_contains + + return any( + m.get("role") == "user" + and _content_text_for_contains(m.get("content")).strip() + for m in messages + ) + + +class TestCompressKeepsANonEmptyUserTurn: + """A bare ``role == "user"`` check is not enough: the surviving user + message can be image-only (no caption). ``_strip_historical_media`` + anchors on the newest image-bearing user message and keeps it + byte-for-byte — it never gains a text placeholder — so a role-only + guard is satisfied while the request still has zero *text* user + turns, which is what backends actually reject on. + """ + + def test_image_only_tail_user_turn_still_yields_non_empty_text(self, compressor): + from agent.context_compressor import SUMMARY_PREFIX + + c = compressor + c.compression_count = 1 + # No system message and no protected head (compression_count=1 + # decays protect_first_n to 0), same #58753 shape — except the + # only surviving user turn is an image with no caption text. + messages = [{"role": "user", "content": "work kanban task 42"}] + messages += _tool_turns(0, 12) + messages += [{ + "role": "user", + "content": [ + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}, + ], + }] + + mocked = f"{SUMMARY_PREFIX}\nrolled-up summary of the tool work" + with patch.object(c, "_generate_summary", return_value=mocked): + out = c.compress(messages, current_tokens=90_000) + + hist = _role_hist(out) + assert hist.get("user", 0) >= 1 + assert _has_nonempty_user_text(out), ( + "REGRESSION: an image-only user turn satisfied the role-only " + "zero-user-turn guard, leaving a transcript with a user-role " + "message but no actual query text — the exact class of " + "request backends reject with 'No user query found in " + f"messages'. Output: {out}" + ) + + def test_image_only_protected_head_still_yields_non_empty_text(self, compressor): + """Isolates the ``_user_survives`` text check from the merge-target + fix below: here ``compress_start != 0`` and there's no system + message, so neither existing force condition + (``compress_start == 0`` / ``last_head_role == "system"``) fires on + its own — only the broadened (text-aware) survival check does. + """ + from agent.context_compressor import SUMMARY_PREFIX + + c = compressor + c.protect_first_n = 1 + c.protect_last_n = 2 + c.tail_token_budget = 10 + c.compression_count = 0 + + messages = [{ + "role": "user", + "content": [ + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}, + ], + }] + messages += _tool_turns(0, 12) + + mocked = f"{SUMMARY_PREFIX}\nrolled-up summary of the tool work" + with patch.object(c, "_generate_summary", return_value=mocked): + out = c.compress(messages, current_tokens=90_000) + + assert _has_nonempty_user_text(out), ( + "REGRESSION: an image-only protected-head user turn satisfied " + f"the role-only zero-user-turn guard. Output: {out}" + ) + + def test_merge_targets_the_colliding_tail_message_not_index_zero(self, compressor): + """Template-exempt rows (bare tool-call assistant / tool messages) + ahead of the colliding tail user message must not divert the merge: + merging into literal tail index 0 would attach the summary to an + exempt row and leave the real (image-only) user message untouched + and still empty, silently defeating the forced role="user" this + block exists to guarantee. + """ + from agent.context_compressor import SUMMARY_PREFIX + + c = compressor + c.compression_count = 1 # decays protect_first_n to 0 -> compress_start == 0 + messages = [{"role": "user", "content": "work kanban task 42"}] + messages += _tool_turns(0, 12) + messages += [{ + "role": "user", + "content": [ + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}, + ], + }] + + mocked = f"{SUMMARY_PREFIX}\nrolled-up summary of the tool work" + with patch.object(c, "_generate_summary", return_value=mocked): + out = c.compress(messages, current_tokens=90_000) + + assert _has_nonempty_user_text(out), ( + "REGRESSION: the summary merged into tail index 0 (a " + "template-exempt row) instead of the colliding image-only " + f"user message, leaving zero non-empty user turns. Output: {out}" + )