fix(compressor): widen compaction-time image aging to first-message and envelope shapes
Widen #90001's compaction-time strip to cover the gaps #89965 identified, applied at compaction only per the cache ruling (request-time eviction changes the per-call prefix and breaks prompt caching; compaction is the one sanctioned cache break): - Rule 1b: the opening attachment (anchor == 0) ages out once a newer tool-result image supersedes it. The reported session opened with a ~200KB poster that previously survived every compaction. The row keeps a non-empty text placeholder, so the zero-user-turn guard (#58753) and role alternation are untouched. - Native {_multimodal: True, content: [...]} dict envelopes now both anchor (newest is kept) and strip (older collapse to their text_summary via _strip_images_from_tool_msg, which also drops the stale api_content sidecar per #97125's drop_stale_api_content). - All three wire shapes (Chat Completions image_url, Responses input_image, Anthropic-native image) were already matched by _IMAGE_PART_TYPES; tests now pin each shape explicitly, plus determinism (double-run is a no-op returning the same object). Refs #89938, #89965
This commit is contained in:
@@ -1869,9 +1869,19 @@ def _strip_historical_media(messages: List[Dict[str, Any]]) -> List[Dict[str, An
|
||||
session whose images arrive from tools rather than attachments has no
|
||||
anchor to be "before" and keeps every blob forever (#89938).
|
||||
|
||||
The opening attachment gets the same keep-newest treatment: when the only
|
||||
image-bearing user message is the very first one and a newer tool-result
|
||||
image exists, the first message's images are replaced too (rule 1b) —
|
||||
otherwise a session that opens with an attachment re-ships it forever.
|
||||
|
||||
Tool results are matched in both shapes: OpenAI-style content-part lists
|
||||
and the native ``{_multimodal: True, content: [...]}`` dict envelope.
|
||||
Image parts of all three wire shapes (Chat Completions ``image_url``,
|
||||
Responses ``input_image``, Anthropic-native ``image``) are recognized.
|
||||
|
||||
If no message carries images at all, the list is returned unchanged. So
|
||||
is a list whose only image-bearing user message is the very first one and
|
||||
which has no tool-result images (nothing to strip in either rule).
|
||||
which has no tool-result images (nothing to strip in any rule).
|
||||
|
||||
Shallow copies of touched messages only; input is never mutated.
|
||||
Port of Kilo-Org/kilocode#9434 (adapted for the OpenAI-style message
|
||||
@@ -1912,7 +1922,12 @@ def _strip_historical_media(messages: List[Dict[str, Any]]) -> List[Dict[str, An
|
||||
continue
|
||||
if msg.get("role") != "tool":
|
||||
continue
|
||||
if _content_has_images(msg.get("content")):
|
||||
# ``_tool_content_has_images`` (not the bare list matcher) so the
|
||||
# native ``{_multimodal: True, content: [...]}`` dict envelope that
|
||||
# vision_analyze can leave in the live list anchors here too —
|
||||
# otherwise the newest envelope-shaped result is invisible to the
|
||||
# scan and rule 2 strips it as if it were stale (#89938/#89965 gap).
|
||||
if _tool_content_has_images(msg.get("content")):
|
||||
tool_anchor = i
|
||||
break
|
||||
|
||||
@@ -1927,6 +1942,18 @@ def _strip_historical_media(messages: List[Dict[str, Any]]) -> List[Dict[str, An
|
||||
# kind but still sits before that anchor keeps today's behaviour.
|
||||
if 0 < anchor and index < anchor:
|
||||
return True
|
||||
# Rule 1b: the opening attachment ages out once something newer
|
||||
# supersedes it. When the ONLY image-bearing user message is the very
|
||||
# first one (``anchor == 0``) and newer tool-result images exist, the
|
||||
# model has moved on — but the opening base64 blob used to survive
|
||||
# every compaction forever, which is half the wedge in #89938 (the
|
||||
# reported session opened with a ~200KB poster). The strip replaces
|
||||
# the image with a text placeholder, so the row keeps non-empty
|
||||
# user-role text and the zero-user-turn guard (#58753) is satisfied.
|
||||
# When nothing newer exists the opening image IS the newest image and
|
||||
# is kept, consistent with keep-newest everywhere else.
|
||||
if anchor == 0 and index == 0 and tool_anchor > 0:
|
||||
return True
|
||||
# Rule 2: a tool result whose image has been superseded by a newer
|
||||
# one. Applies inside the protected tail as well -- the tail exists to
|
||||
# preserve conversational continuity, not to pin bytes the model has
|
||||
@@ -1940,6 +1967,24 @@ def _strip_historical_media(messages: List[Dict[str, Any]]) -> List[Dict[str, An
|
||||
result.append(msg)
|
||||
continue
|
||||
content = msg.get("content")
|
||||
# Native multimodal dict envelope ({_multimodal: True, content: [...]})
|
||||
# — the shape vision_analyze hands back before adapters unwrap it.
|
||||
# ``_strip_images_from_content`` only understands part lists, so route
|
||||
# this through the tool-message stripper, which collapses the envelope
|
||||
# to its text summary and drops the stale api_content sidecar.
|
||||
if (
|
||||
msg.get("role") == "tool"
|
||||
and isinstance(content, dict)
|
||||
and content.get("_multimodal")
|
||||
and _tool_content_has_images(content)
|
||||
):
|
||||
new_msg = _strip_images_from_tool_msg(msg)
|
||||
if new_msg is None:
|
||||
result.append(msg)
|
||||
continue
|
||||
result.append(new_msg)
|
||||
changed = True
|
||||
continue
|
||||
if not _content_has_images(content):
|
||||
result.append(msg)
|
||||
continue
|
||||
|
||||
@@ -140,11 +140,74 @@ class TestStripHistoricalMedia:
|
||||
]
|
||||
out = _strip_historical_media(msgs)
|
||||
|
||||
# The opening attachment keeps today's treatment: nothing precedes it.
|
||||
assert _content_has_images(out[0]["content"])
|
||||
# Rule 1b: newer tool images supersede the opening attachment, so
|
||||
# its bytes age out too — the row survives with a text placeholder.
|
||||
assert not _content_has_images(out[0]["content"])
|
||||
assert out[0]["role"] == "user"
|
||||
assert not _content_has_images(out[1]["content"])
|
||||
assert _content_has_images(out[2]["content"])
|
||||
|
||||
def test_first_message_image_kept_when_it_is_the_only_image(self):
|
||||
"""Rule 1b only fires when something newer supersedes the opener."""
|
||||
msgs = [
|
||||
{"role": "user", "content": [TEXT, IMG_URL]},
|
||||
{"role": "assistant", "content": "looked"},
|
||||
{"role": "tool", "tool_call_id": "a", "content": [TEXT]},
|
||||
]
|
||||
assert _strip_historical_media(msgs) is msgs
|
||||
|
||||
def test_multimodal_envelope_tool_results_age_out(self):
|
||||
"""The native ``{_multimodal: True}`` dict envelope must strip too.
|
||||
|
||||
vision_analyze hands back this shape before adapters unwrap it; the
|
||||
bare list matcher never saw it, so envelope-shaped results kept their
|
||||
base64 through every compaction (#89965's shape-coverage gap).
|
||||
"""
|
||||
env = {
|
||||
"_multimodal": True,
|
||||
"text_summary": "a poster",
|
||||
"content": [TEXT, IMG_URL],
|
||||
}
|
||||
msgs = [
|
||||
{"role": "user", "content": "look"},
|
||||
{"role": "tool", "tool_call_id": "a", "content": dict(env), "api_content": "stale"},
|
||||
{"role": "tool", "tool_call_id": "b", "content": dict(env)},
|
||||
]
|
||||
out = _strip_historical_media(msgs)
|
||||
|
||||
# Older envelope collapses to its text summary; sidecar dropped.
|
||||
assert isinstance(out[1]["content"], str)
|
||||
assert "a poster" in out[1]["content"]
|
||||
assert "api_content" not in out[1]
|
||||
assert out[1]["tool_call_id"] == "a"
|
||||
# Newest envelope is the anchor and survives byte-for-byte.
|
||||
assert out[2] is msgs[2]
|
||||
|
||||
def test_all_three_wire_shapes_strip_in_tool_results(self):
|
||||
"""Chat Completions, Responses, and Anthropic-native parts all age."""
|
||||
for img in (IMG_URL, INPUT_IMG, ANTHROPIC_IMG):
|
||||
msgs = [
|
||||
{"role": "tool", "tool_call_id": "a", "content": [TEXT, dict(img)]},
|
||||
{"role": "tool", "tool_call_id": "b", "content": [TEXT, dict(img)]},
|
||||
]
|
||||
out = _strip_historical_media(msgs)
|
||||
assert not _content_has_images(out[0]["content"]), img["type"]
|
||||
assert _content_has_images(out[1]["content"]), img["type"]
|
||||
|
||||
def test_deterministic_double_run(self):
|
||||
"""Running the strip twice yields byte-identical output."""
|
||||
import json
|
||||
|
||||
msgs = [
|
||||
{"role": "user", "content": [TEXT, IMG_URL]},
|
||||
{"role": "tool", "tool_call_id": "a", "content": [TEXT, IMG_URL]},
|
||||
{"role": "tool", "tool_call_id": "b", "content": [TEXT, INPUT_IMG]},
|
||||
]
|
||||
first = _strip_historical_media(msgs)
|
||||
second = _strip_historical_media(first)
|
||||
assert json.dumps(first, sort_keys=True) == json.dumps(second, sort_keys=True)
|
||||
assert second is first # second pass is a no-op
|
||||
|
||||
def test_newest_tool_image_survives_inside_the_protected_tail(self):
|
||||
msgs = [
|
||||
{"role": "user", "content": "hi"},
|
||||
|
||||
Reference in New Issue
Block a user