From 302843c89f22d1ce2ad10619acd33382c9f48111 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 20 Jul 2026 13:11:29 -0700 Subject: [PATCH] Release: compact completed image tool-results (#6315, @sjungwon03) (#6371) * fix(session): compact completed native vision results * streaming: guard image-part compaction against unhashable part.type (isinstance str) + regression test [maintainer fix on @sjungwon03 #6315] * docs(changelog): stamp #6315 completed-image tool-result compaction --------- Co-authored-by: sjungwon03 Co-authored-by: nesquena-hermes --- CHANGELOG.md | 2 + api/routes.py | 2 + api/streaming.py | 74 +++++++- tests/test_context_history_sanitization.py | 167 ++++++++++++++++++ tests/test_issue1217_transcript_compaction.py | 28 +++ tests/test_run_journal_streaming_static.py | 11 ++ 6 files changed, 283 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e9dc877..7aebca53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ ### Fixed +- **Completed image tool-results are compacted from persisted history, cutting long-session memory and save/load cost.** After a turn finishes, base64 image parts in persisted *tool* results are replaced with a small `[screenshot]` text placeholder, so JSON sidecar saves/loads and session API responses no longer scale with image bytes (the active model still gets the full image while the tool call is running). The transform preserves every non-image part in place (text, documents, provider-specific parts) without flattening or reordering, only touches `role: tool` messages (user attachments are untouched), runs on all live and settled persistence writebacks, and is idempotent. Thanks @sjungwon03. (#6315, #1217) + - **Large embedded images no longer slow down API response redaction.** Native raster image data URIs (`data:image/png|jpeg|gif|webp|bmp;base64,…`) are skipped by the credential-redaction pass instead of being scanned byte-by-byte on every response — a multi-megabyte image is opaque binary, not text a credential regex should rewrite. The fast path only applies after the *entire* base64 payload is validated (canonical round-trip + structural walk), so a string that merely starts with an image header but carries trailing or appended text still goes through full redaction — a credential can never ride along in an image's tail. Non-image strings, and sibling secrets in the same payload, are unaffected. Thanks @inch772. (#6311) - **Gateway-backed sessions can once again send with a MoA (Mixture-of-Agents) default model.** A legitimate gateway-default MoA send was being rejected as if it were a disallowed per-request override. The chat-start handler now distinguishes the two: a send is allowed only when the *configured* provider is MoA, the resolved model matches the configured default, and no explicit picker override was made — all evaluated against one request-owned config snapshot captured atomically, so a concurrent profile/config change can't flip the decision mid-request. Explicit `moa_config` overrides on gateway sessions still return 409. Thanks @rodboev. (#5869, #5853) diff --git a/api/routes.py b/api/routes.py index ed3c7229..97e8044a 100644 --- a/api/routes.py +++ b/api/routes.py @@ -22364,6 +22364,7 @@ def _handle_chat_sync(handler, body): _restore_display_reasoning_metadata, _restore_reasoning_metadata, _sanitize_messages_for_api, + _compact_session_image_parts_for_persistence, _context_messages_for_new_turn, _workspace_context_prefix, ) @@ -22441,6 +22442,7 @@ def _handle_chat_sync(handler, body): msg, source=getattr(s, "pending_user_source", None) or "webui", ) + _compact_session_image_parts_for_persistence(s) # Only auto-generate title when still default; preserves user renames if s.title == "Untitled": s.title = title_from(s.messages, s.title) diff --git a/api/streaming.py b/api/streaming.py index d7f91ccd..a05997c1 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -4225,6 +4225,76 @@ def _should_strip_reasoning_content( return False +def _compact_image_parts_for_persistence(messages) -> int: + """Replace persisted image parts with text placeholders after a completed turn. + + The active model receives native image parts while a tool call is running. Once + the turn has completed, retaining base64 data URLs in both the visible + transcript and ``context_messages`` makes every JSON sidecar save/load and + session API response scale with the image bytes. Hermes Agent's durable + session store applies the same text-only policy for completed multimodal + tool results. Keep text parts and the surrounding tool-call chain intact so + future turns retain the conversational record and can re-open the original + image from the preceding tool-call arguments when needed. + + This intentionally mutates the owned session message rows in place. It is + called only after ``run_conversation()`` has returned, so it never removes + image parts that the current model invocation still needs. + """ + changed = 0 + for message in messages or (): + # Mirror Hermes Agent's durable-session policy: native *tool* results + # are transient input for the current model call. User attachments are + # a separate product contract and must remain intact here. + if not isinstance(message, dict) or message.get('role') != 'tool': + continue + content = message.get('content') + if not isinstance(content, list): + continue + + compacted_content = [] + image_parts = 0 + for part in content: + if not isinstance(part, dict): + # A provider can legally return scalar content alongside typed + # parts. Preserve it unchanged rather than flattening/reordering + # the structured result during durable-session compaction. + compacted_content.append(part) + continue + part_type = part.get('type') + # Guard the set-membership with an isinstance check: a JSON-valid + # part can carry an unhashable ``type`` (e.g. a list), and + # ``unhashable in {...}`` raises TypeError — which would turn an + # otherwise-complete streaming send into the error path before the + # session is saved. Only the three string image types are compacted; + # every other part (including non-string ``type`` values) is preserved. + if isinstance(part_type, str) and part_type in {'image', 'image_url', 'input_image'}: + compacted_content.append({'type': 'text', 'text': '[screenshot]'}) + image_parts += 1 + else: + compacted_content.append(part) + + if image_parts: + message['content'] = compacted_content + changed += image_parts + return changed + + +def _compact_session_image_parts_for_persistence(session) -> int: + """Compact completed native-vision tool results in both durable histories.""" + changed = ( + _compact_image_parts_for_persistence(getattr(session, 'context_messages', None)) + + _compact_image_parts_for_persistence(getattr(session, 'messages', None)) + ) + if changed: + logger.info( + "Compacted %d completed image message part(s) for session %s", + changed, + getattr(session, 'session_id', None), + ) + return changed + + def _sanitize_messages_for_api( messages, *, @@ -9045,6 +9115,7 @@ def _run_agent_streaming( msg_text, source=getattr(s, 'pending_user_source', None) or 'webui', ) + _compact_session_image_parts_for_persistence(s) _advance_truncation_watermark_after_commit(s) # #3831 # Strip XML tool-call blocks from assistant message content. # DeepSeek and some other providers emit ... @@ -9389,8 +9460,8 @@ def _run_agent_streaming( msg_text, source=getattr(s, 'pending_user_source', None) or 'webui', ) + _compact_session_image_parts_for_persistence(s) _advance_truncation_watermark_after_commit(s) # #3831 - # Skip the error block — jump directly to the # normal post-result persistence path by # leaving _assistant_added truthy (set below). _assistant_added = True # prevent re-entering guard @@ -10609,6 +10680,7 @@ def _run_agent_streaming( msg_text, source=getattr(s, 'pending_user_source', None) or 'webui', ) + _compact_session_image_parts_for_persistence(s) _advance_truncation_watermark_after_commit(s) # #3831 s.save() logger.info('[webui] self-heal (except path): retry succeeded') diff --git a/tests/test_context_history_sanitization.py b/tests/test_context_history_sanitization.py index aadfd352..f412e425 100644 --- a/tests/test_context_history_sanitization.py +++ b/tests/test_context_history_sanitization.py @@ -7,6 +7,8 @@ from types import SimpleNamespace from unittest.mock import patch from api.streaming import ( + _compact_image_parts_for_persistence, + _compact_session_image_parts_for_persistence, _restore_reasoning_metadata, _sanitize_messages_for_api, _strip_oob_blocks, @@ -24,6 +26,171 @@ def _contains_oob(value) -> bool: return "OUT-OF-BAND USER MESSAGE" in json.dumps(value) + +def test_compact_image_parts_for_persistence_keeps_text_and_drops_base64(): + image_data_url = "data:image/png;base64," + ("A" * 1_000_000) + messages = [ + { + "role": "assistant", + "content": "", + "tool_calls": [{"id": "vision-1"}], + }, + { + "role": "tool", + "tool_call_id": "vision-1", + "content": [ + {"type": "text", "text": "Image loaded into your context."}, + {"type": "image_url", "image_url": {"url": image_data_url}}, + ], + }, + ] + + changed = _compact_image_parts_for_persistence(messages) + + assert changed == 1 + assert messages[1]["content"] == [ + {"type": "text", "text": "Image loaded into your context."}, + {"type": "text", "text": "[screenshot]"}, + ] + assert "data:image" not in json.dumps(messages) + assert messages[0]["tool_calls"] == [{"id": "vision-1"}] + + + + +def test_compact_image_parts_for_persistence_counts_each_image_part(): + messages = [ + { + "role": "tool", + "content": [ + {"type": "text", "text": "Before and after"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}, + {"type": "input_image", "image_url": {"url": "data:image/png;base64,BBBB"}}, + ], + } + ] + + changed = _compact_image_parts_for_persistence(messages) + + assert changed == 2 + assert messages[0]["content"] == [ + {"type": "text", "text": "Before and after"}, + {"type": "text", "text": "[screenshot]"}, + {"type": "text", "text": "[screenshot]"}, + ] + + +def test_compact_image_parts_for_persistence_preserves_interleaved_non_image_parts(): + document = {"type": "document", "document": {"name": "report.pdf", "id": "doc-1"}} + scalar = "provider extension payload" + messages = [ + { + "role": "tool", + "content": [ + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}, + {"type": "text", "text": "between"}, + document, + scalar, + {"type": "input_image", "image_url": {"url": "data:image/png;base64,BBBB"}}, + {"type": "input_text", "content": "after"}, + ], + } + ] + + changed = _compact_image_parts_for_persistence(messages) + + assert changed == 2 + assert messages[0]["content"] == [ + {"type": "text", "text": "[screenshot]"}, + {"type": "text", "text": "between"}, + document, + scalar, + {"type": "text", "text": "[screenshot]"}, + {"type": "input_text", "content": "after"}, + ] + assert "data:image" not in json.dumps(messages) + assert _compact_image_parts_for_persistence(messages) == 0 + + +def test_compact_image_parts_for_persistence_survives_unhashable_part_type(): + # A JSON-valid part can carry a non-string (unhashable) ``type`` such as a + # list or dict. ``part.get("type") in {...}`` would raise TypeError on an + # unhashable value, turning an otherwise-complete streaming send into the + # error path before the session is saved. Such parts must be preserved + # unchanged, and real image parts alongside them must still compact. + list_type = {"type": ["provider-extension"], "data": "x"} + dict_type = {"type": {"nested": "kind"}, "data": "y"} + messages = [ + { + "role": "tool", + "content": [ + list_type, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}, + dict_type, + ], + } + ] + + changed = _compact_image_parts_for_persistence(messages) + + assert changed == 1 + assert messages[0]["content"] == [ + list_type, + {"type": "text", "text": "[screenshot]"}, + dict_type, + ] + assert "data:image" not in json.dumps(messages) + # Idempotent: a second pass changes nothing and still does not raise. + assert _compact_image_parts_for_persistence(messages) == 0 + + +def test_compact_session_image_parts_for_persistence_compacts_both_histories(): + image = {"type": "image_url", "image_url": {"url": "data:image/png;base64," + ("A" * 4096)}} + session = SimpleNamespace( + session_id="vision-turn", + messages=[{"role": "tool", "content": [{"type": "text", "text": "display"}, image.copy()]}], + context_messages=[{"role": "tool", "content": [{"type": "text", "text": "context"}, image.copy()]}], + ) + + changed = _compact_session_image_parts_for_persistence(session) + + assert changed == 2 + assert session.messages[0]["content"] == [ + {"type": "text", "text": "display"}, + {"type": "text", "text": "[screenshot]"}, + ] + assert session.context_messages[0]["content"] == [ + {"type": "text", "text": "context"}, + {"type": "text", "text": "[screenshot]"}, + ] + + +def test_compact_image_parts_for_persistence_keeps_user_image_attachment(): + messages = [ + { + "role": "user", + "content": [ + {"type": "text", "text": "Inspect this attachment"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}, + ], + } + ] + + changed = _compact_image_parts_for_persistence(messages) + + assert changed == 0 + assert messages[0]["content"][1]["type"] == "image_url" + + +def test_compact_image_parts_for_persistence_leaves_text_history_unchanged(): + messages = [{"role": "tool", "content": "plain tool output"}] + + changed = _compact_image_parts_for_persistence(messages) + + assert changed == 0 + assert messages == [{"role": "tool", "content": "plain tool output"}] + + def test_strip_oob_blocks_string(): content = f"visible before\n{OOB_BLOCK}\nvisible after" diff --git a/tests/test_issue1217_transcript_compaction.py b/tests/test_issue1217_transcript_compaction.py index 78953ec6..f2954b44 100644 --- a/tests/test_issue1217_transcript_compaction.py +++ b/tests/test_issue1217_transcript_compaction.py @@ -767,8 +767,21 @@ def test_handle_chat_sync_writeback_dedupes_full_context_replay(tmp_path, monkey ) session.save(touch_updated_at=False) + native_result = [ + {"role": "assistant", "content": "", "tool_calls": [{"id": "vision-1"}]}, + { + "role": "tool", + "tool_call_id": "vision-1", + "content": [ + {"type": "text", "text": "Vision result"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}}, + {"type": "document", "document": {"id": "doc-1"}}, + ], + }, + ] replayed_result = previous_context + previous_context + [ {"role": "user", "content": "simple follow-up"}, + *native_result, {"role": "assistant", "content": "short answer"}, ] @@ -793,6 +806,7 @@ def test_handle_chat_sync_writeback_dedupes_full_context_replay(tmp_path, monkey assert handler.status == 200 reloaded = Session.load(session.session_id) + assert reloaded is not None # Stable per-message ids (#context-message-stable-id) are now stamped on # context rows; compare on the semantic fields so the dedup invariant is @@ -807,9 +821,23 @@ def test_handle_chat_sync_writeback_dedupes_full_context_replay(tmp_path, monkey {"role": "user", "content": "[Session Arc Summary (d1, node 39)]\n" + "old context\n" * 400}, {"role": "assistant", "content": "previous answer"}, {"role": "user", "content": "simple follow-up"}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "vision-1"}]}, + { + "role": "tool", + "tool_call_id": "vision-1", + "content": [ + {"type": "text", "text": "Vision result"}, + {"type": "text", "text": "[screenshot]"}, + {"type": "document", "document": {"id": "doc-1"}}, + ], + }, {"role": "assistant", "content": "short answer"}, ] assert _no_id(reloaded.context_messages) == expected + assert "data:image" not in json.dumps(reloaded.context_messages) + assert "data:image" not in json.dumps(reloaded.messages) + assert any(row.get("tool_call_id") == "vision-1" for row in reloaded.context_messages) + assert any(row.get("tool_call_id") == "vision-1" for row in reloaded.messages) assert _no_id(reloaded.context_messages).count(expected[0]) == 1 # The new turn's rows carry unique stable ids. (This session preloads # id-less legacy rows, which stay id-less until they age out of context; diff --git a/tests/test_run_journal_streaming_static.py b/tests/test_run_journal_streaming_static.py index a2542171..c61ba1b8 100644 --- a/tests/test_run_journal_streaming_static.py +++ b/tests/test_run_journal_streaming_static.py @@ -25,6 +25,17 @@ def test_streaming_journals_sse_events_before_queue_delivery(): assert "queue_item = (event, data, event_id) if event_id and hasattr(q, \"subscribe_with_snapshot\") else (event, data)" in block + +def test_streaming_compacts_all_successful_agent_result_writebacks(): + src = Path("api/streaming.py").read_text(encoding="utf-8") + run_src = src[src.index("def _run_agent_streaming("):] + + # Normal completion plus both credential self-heal retry-success paths must + # each compact after committing their own result shape. The retries use + # different local variable names, so guard the durable invariant itself. + assert run_src.count("_compact_session_image_parts_for_persistence(s)") == 3 + + def test_visible_process_echo_compare_ignores_all_whitespace(): token_text = "先把 issue 4249 拉下来\n\n先看正文和评论" interim_text = "先把 issue 4249 拉下来先看正文和评论"