feat(compressor): keep image URLs and unknown-part markers in summarizer serialization
Follow-up on @AlexFucuson9's cherry-picked multimodal flattening: - http(s) image parts render as '[image: <url>]' so the summary keeps a referenceable handle after compaction (base64 data: URLs still collapse to '[image]' — no reusable reference, and leaking them is the bug being fixed) - unknown part types (document, future shapes) render as '[<type>]' instead of being silently dropped - test docstrings corrected: the original crash premise is stale (redact_sensitive_text grew str() coercion in #52147); the live bug is repr-noise + base64 leakage into the summarizer input
This commit is contained in:
parent
868a2f7d17
commit
704bbcca80
2 changed files with 63 additions and 11 deletions
|
|
@ -586,6 +586,27 @@ def _strip_historical_media(messages: List[Dict[str, Any]]) -> List[Dict[str, An
|
|||
return result if changed else messages
|
||||
|
||||
|
||||
def _image_part_label(part: Dict[str, Any]) -> str:
|
||||
"""Render a multimodal image part as a short text label for the summarizer.
|
||||
|
||||
Keeps a real, referenceable URL when the image lives at an http(s)
|
||||
address — the summary can then preserve the handle so the agent (or a
|
||||
later vision_analyze call) can still reach the image after compaction.
|
||||
Base64 ``data:`` URLs carry no reusable reference and would flood the
|
||||
summarizer input, so they collapse to ``[image]``.
|
||||
"""
|
||||
url = ""
|
||||
if isinstance(part.get("image_url"), dict):
|
||||
url = str(part["image_url"].get("url") or "")
|
||||
elif isinstance(part.get("image_url"), str):
|
||||
url = part["image_url"]
|
||||
elif isinstance(part.get("url"), str):
|
||||
url = part["url"]
|
||||
if url.startswith(("http://", "https://")):
|
||||
return f"[image: {url}]"
|
||||
return "[image]"
|
||||
|
||||
|
||||
def _str_arg(args: dict, key: str, default: str = "") -> str:
|
||||
"""Safely get a string argument from parsed tool args.
|
||||
|
||||
|
|
@ -1669,12 +1690,15 @@ class ContextCompressor(ContextEngine):
|
|||
text_parts: list[str] = []
|
||||
for part in content:
|
||||
if isinstance(part, dict):
|
||||
if part.get("type") == "text":
|
||||
ptype = part.get("type")
|
||||
if ptype == "text":
|
||||
text_parts.append(part.get("text", ""))
|
||||
elif part.get("type") in {
|
||||
"image", "image_url", "input_image",
|
||||
}:
|
||||
text_parts.append("[image]")
|
||||
elif ptype in {"image", "image_url", "input_image"}:
|
||||
text_parts.append(_image_part_label(part))
|
||||
else:
|
||||
# Unknown part type — keep a marker so the
|
||||
# summarizer knows content existed here.
|
||||
text_parts.append(f"[{ptype or 'attachment'}]")
|
||||
elif isinstance(part, str):
|
||||
text_parts.append(part)
|
||||
content = "\n".join(text_parts)
|
||||
|
|
|
|||
|
|
@ -56,12 +56,11 @@ class TestMediaDirectiveStripping:
|
|||
assert result.count("[media attachment]") == 2
|
||||
|
||||
def test_multimodal_list_content_does_not_crash(self, compressor):
|
||||
"""content as a list (multimodal) must not crash redact_sensitive_text.
|
||||
"""content as a list (multimodal) must be flattened to clean text.
|
||||
|
||||
Regression: msg.get('content') can return a list of parts for
|
||||
multimodal messages (images + text). The old code passed this
|
||||
list directly to redact_sensitive_text(str), which raised
|
||||
AttributeError on list.replace().
|
||||
Without flattening, str() coercion in redact_sensitive_text dumps
|
||||
the raw part-dict repr — including base64 image data — into the
|
||||
summarizer input, burning summary budget on noise.
|
||||
"""
|
||||
turns = [
|
||||
{
|
||||
|
|
@ -72,10 +71,39 @@ class TestMediaDirectiveStripping:
|
|||
],
|
||||
},
|
||||
]
|
||||
# Must not raise
|
||||
result = compressor._serialize_for_summary(turns)
|
||||
assert "What is in this image?" in result
|
||||
assert "[image]" in result
|
||||
assert "base64" not in result
|
||||
|
||||
def test_multimodal_remote_image_keeps_url(self, compressor):
|
||||
"""http(s) image parts keep their URL as a referenceable handle."""
|
||||
turns = [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{"type": "text", "text": "look at this"},
|
||||
{"type": "image_url", "image_url": {"url": "https://example.com/a.png"}},
|
||||
],
|
||||
},
|
||||
]
|
||||
result = compressor._serialize_for_summary(turns)
|
||||
assert "[image: https://example.com/a.png]" in result
|
||||
|
||||
def test_multimodal_unknown_part_type_keeps_marker(self, compressor):
|
||||
"""Unknown part types are not silently dropped."""
|
||||
turns = [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{"type": "text", "text": "see attachment"},
|
||||
{"type": "document", "title": "spec.pdf"},
|
||||
],
|
||||
},
|
||||
]
|
||||
result = compressor._serialize_for_summary(turns)
|
||||
assert "see attachment" in result
|
||||
assert "[document]" in result
|
||||
|
||||
def test_multimodal_list_text_parts_extracted(self, compressor):
|
||||
"""Text parts from multimodal list content are preserved in output."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue