From 78b0425902b105e02dc5732966fe57e949a3550c Mon Sep 17 00:00:00 2001 From: starship-s <45587122+starship-s@users.noreply.github.com> Date: Sat, 22 Aug 2026 09:47:08 -0600 Subject: [PATCH] fix(sessions): reconcile native image message mirrors (#7230) * fix(sessions): reconcile native image message mirrors Keep the rich WebUI sidecar row when Hermes Agent stores the same exact-timestamp native image turn as scalar screenshot-marker text. Assisted-by: Hermes Agent:gpt-5.6-sol Assisted-by: Codex:gpt-5.6-luna Assisted-by: OpenCode:opencode/x-preview-f-free Assisted-by: Claude Code:claude-opus-5 * docs(replay): document native image mirror contract Record the exact cross-store identity requirements and fail-closed behavior for native image session reconciliation. Assisted-by: Hermes Agent:gpt-5.6-sol * fix(sessions): fail closed on state mirror identity gaps Treat present-versus-absent candidate identity fields as ambiguous so session restore preserves distinct state-only turns and their metadata. Assisted-by: Hermes Agent:gpt-5.6-sol Assisted-by: Codex:gpt-5.6-luna --- api/models.py | 175 ++++++++++- .../webui-run-state-consistency-contract.md | 12 +- ...test_issue5339_restart_stale_user_dedup.py | 297 ++++++++++++++++++ 3 files changed, 480 insertions(+), 4 deletions(-) diff --git a/api/models.py b/api/models.py index 00246533..62702deb 100644 --- a/api/models.py +++ b/api/models.py @@ -8643,7 +8643,7 @@ def _message_timestamp_as_float(msg): return timestamp if math.isfinite(timestamp) else None -def _session_message_api_content_key(msg: dict): +def _session_message_api_content_key(msg: dict | None): """Return the exact trusted provider sidecar used in duplicate identity.""" if not isinstance(msg, dict): return None @@ -8662,6 +8662,74 @@ def _session_message_key_with_sidecar(base_key: tuple, msg: dict) -> tuple: return base_key if sidecar is None else (*base_key, sidecar) +_SESSION_MESSAGE_IMAGE_PART_TYPES = {"image", "image_url", "input_image"} + + +def _agent_durable_multimodal_content(msg: dict) -> str | None: + """Project one native image-bearing turn to Hermes Agent's stored text.""" + if not isinstance(msg, dict): + return None + content = msg.get("content") + if not isinstance(content, list): + return None + normalized_parts = [] + image_parts = 0 + for part in content: + if not isinstance(part, dict): + return None + part_type = part.get("type") + if isinstance(part_type, str) and part_type in _SESSION_MESSAGE_IMAGE_PART_TYPES: + normalized_parts.append("[screenshot]") + image_parts += 1 + elif part_type == "text": + text = part.get("text") + if not isinstance(text, str): + return None + normalized_parts.append(text) + else: + return None + if not image_parts: + return None + return " ".join("\n".join(normalized_parts).split()) + + +def _session_message_multimodal_mirror_key( + msg: dict, + *, + require_image_parts: bool = False, +): + """Return exact cross-store identity for a native multimodal mirror.""" + if not isinstance(msg, dict): + return None + role = str(msg.get("role") or "").strip().lower() + if role != "user": + return None + raw_content = msg.get("content") + content = _agent_durable_multimodal_content(msg) + if require_image_parts and content is None: + return None + if content is None: + if not isinstance(raw_content, str): + return None + content = _normalized_session_message_content(msg) + timestamp, timestamp_valid = _message_exact_timestamp_details(msg) + if not timestamp_valid or timestamp is None: + return None + tool_calls = msg.get("tool_calls") + tool_calls_key = json.dumps(tool_calls, sort_keys=True, default=str) if tool_calls else "" + # api_content is checked as private identity below, not projected content: + # one store may carry the trusted provider sidecar while the other does not. + return ( + "multimodal_mirror", + role, + content, + timestamp, + str(msg.get("tool_call_id") or ""), + str(msg.get("tool_name") or msg.get("name") or ""), + tool_calls_key, + ) + + def _session_message_merge_key(msg: dict): if not isinstance(msg, dict): return ("non_dict", repr(msg)) @@ -8805,7 +8873,7 @@ def _stable_message_identity_details(message: dict | None) -> tuple[str | None, return (next(iter(values)) if values else None), True -def _message_identity_compatible(target: dict | None, source: dict | None) -> bool: +def _message_private_identity_compatible(target: dict | None, source: dict | None) -> bool: """Return whether private identities do not contradict one another.""" target_stable, target_stable_valid = _stable_message_identity_details(target) source_stable, source_stable_valid = _stable_message_identity_details(source) @@ -8815,6 +8883,29 @@ def _message_identity_compatible(target: dict | None, source: dict | None) -> bo return False target_row_id, target_row_id_valid = _state_db_row_identity_details(target) source_row_id, source_row_id_valid = _state_db_row_identity_details(source) + if not target_row_id_valid or not source_row_id_valid: + return False + if target_row_id is not None and source_row_id is not None and target_row_id != source_row_id: + return False + target_api_content = _session_message_api_content_key(target) + source_api_content = _session_message_api_content_key(source) + return not ( + target_api_content is not None + and source_api_content is not None + and target_api_content != source_api_content + ) + + +def _message_identity_compatible(target: dict | None, source: dict | None) -> bool: + """Check legacy ordinary visible-content identity; intentionally excludes api_content.""" + target_stable, target_stable_valid = _stable_message_identity_details(target) + source_stable, source_stable_valid = _stable_message_identity_details(source) + if not target_stable_valid or not source_stable_valid: + return False + if target_stable is not None and source_stable is not None and target_stable != source_stable: + return False + target_row_id, target_row_id_valid = _state_db_row_identity_details(target) + source_row_id, source_row_id_valid = _state_db_row_identity_details(source) if not target_row_id_valid or not source_row_id_valid: return False if target_row_id is not None and source_row_id is not None and target_row_id != source_row_id: @@ -8903,7 +8994,15 @@ def _copy_api_content_sidecar(target: dict | None, source: dict | None) -> bool: if target_role is None or target_role != source_role: return False if not _visible_content_compatible(target, source): - return False + target_mirror_key = _session_message_multimodal_mirror_key( + target, + require_image_parts=True, + ) + if ( + target_mirror_key is None + or target_mirror_key != _session_message_multimodal_mirror_key(source) + ): + return False if target.get("api_content") not in (None, ""): return True api_content = source.get("api_content") @@ -10001,6 +10100,8 @@ def merge_session_messages_append_only( sidecar_visible_messages = [] sidecar_visible_keys = set() sidecar_visible_counts = {} + sidecar_multimodal_mirrors = {} + ambiguous_multimodal_mirrors = set() merged_by_message_key = {} merged_by_dedup_key = {} merged_by_visible_key = {} @@ -10038,11 +10139,51 @@ def merge_session_messages_append_only( sidecar_visible_counts[visible_key] = sidecar_visible_counts.get(visible_key, 0) + 1 sidecar_visible_sequence.append(visible_key) sidecar_visible_messages.append(msg) + multimodal_mirror_key = _session_message_multimodal_mirror_key( + msg, + require_image_parts=True, + ) + if multimodal_mirror_key is not None: + if multimodal_mirror_key in sidecar_multimodal_mirrors: + ambiguous_multimodal_mirrors.add(multimodal_mirror_key) + else: + sidecar_multimodal_mirrors[multimodal_mirror_key] = msg merged_messages.append(msg) _remember_merged_message(msg, source="sidecar") if _sidecar_has_terminal_partial_error(sidecar_messages): return merged_messages sidecar_visible_lookup = _build_visible_duplicate_lookup(sidecar_visible_keys) + state_multimodal_mirror_keys = {} + ambiguous_state_multimodal_mirrors = set() + if sidecar_multimodal_mirrors: + state_multimodal_mirror_identities = {} + for state_message in state_messages: + mirror_key = _session_message_multimodal_mirror_key(state_message) + state_multimodal_mirror_keys[id(state_message)] = mirror_key + if ( + mirror_key is not None + and mirror_key in sidecar_multimodal_mirrors + and mirror_key not in ambiguous_multimodal_mirrors + ): + stable_id, stable_id_valid = _stable_message_identity_details( + state_message + ) + row_id, row_id_valid = _state_db_row_identity_details(state_message) + if not stable_id_valid or not row_id_valid: + ambiguous_state_multimodal_mirrors.add(mirror_key) + continue + identities = state_multimodal_mirror_identities.setdefault( + mirror_key, set() + ) + identities.add( + ( + stable_id, + row_id, + _session_message_api_content_key(state_message), + ) + ) + if len(identities) > 1: + ambiguous_state_multimodal_mirrors.add(mirror_key) state_replay_idx = 0 skipped_state_visible_counts = {} # Loop-invariant: a session whose original truncate cutoff (truncation_boundary) @@ -10063,6 +10204,34 @@ def merge_session_messages_append_only( dedup_key = _cached_message_key(msg, "dedup") visible_key = _cached_message_key(msg, "visible_state") content_key = _cached_message_key(msg, "content_state") + multimodal_mirror_key = ( + state_multimodal_mirror_keys.get(id(msg)) + if sidecar_multimodal_mirrors + else None + ) + multimodal_replay_target = ( + sidecar_multimodal_mirrors.get(multimodal_mirror_key) + if ( + multimodal_mirror_key not in ambiguous_multimodal_mirrors + and multimodal_mirror_key not in ambiguous_state_multimodal_mirrors + ) + else None + ) + if ( + multimodal_replay_target is not None + and not _message_private_identity_compatible(multimodal_replay_target, msg) + ): + multimodal_replay_target = None + if multimodal_replay_target is not None: + _copy_api_content_sidecar(multimodal_replay_target, msg) + _merge_session_display_metadata(multimodal_replay_target, msg) + if ( + state_replay_idx < len(sidecar_visible_messages) + and sidecar_visible_messages[state_replay_idx] is multimodal_replay_target + ): + state_replay_idx += 1 + seen_dedup_keys.add(dedup_key) + continue replays_sidecar_prefix = False replay_target = None if state_replay_idx < len(sidecar_visible_sequence): diff --git a/docs/rfcs/webui-run-state-consistency-contract.md b/docs/rfcs/webui-run-state-consistency-contract.md index a3d5a3e0..ca185de6 100644 --- a/docs/rfcs/webui-run-state-consistency-contract.md +++ b/docs/rfcs/webui-run-state-consistency-contract.md @@ -3,7 +3,7 @@ - **Status:** Proposed - **Author:** @franksong2702 - **Created:** 2026-05-16 -- **Updated:** 2026-07-16 +- **Updated:** 2026-08-22 - **Tracking issue:** [#2361](https://github.com/nesquena/hermes-webui/issues/2361) - **Related architecture:** [#1925](https://github.com/nesquena/hermes-webui/issues/1925), [`hermes-run-adapter-contract.md`](hermes-run-adapter-contract.md), [`stable-assistant-turn-anchors.md`](stable-assistant-turn-anchors.md) @@ -99,6 +99,16 @@ and 5; it does not mark every run-state boundary implemented. browser-facing timeline renderer as live SSE events so recovery does not downgrade a structured Thinking / progress / tool / compression turn into a separate flattened presentation. + When session loading combines a WebUI sidecar with Hermes Agent `state.db`, a + native-image user turn may appear as both rich multipart content and scalar + text that replaces each image part with `[screenshot]`. Reconciliation may + treat those rows as one turn only when the multipart value contains text and + recognized native-image parts, its exact scalar projection matches, role and + tool shape match, timestamps match exactly, stable IDs and provider metadata + do not conflict, and the pairing is unambiguous. Keep the rich sidecar row; + if any requirement is missing or contradictory, preserve both rows rather + than deduplicating. Literal scalar `[screenshot]` text alone is not identity + evidence. Visible interim assistant progress must remain visible timeline content; a compact Activity disclosure may summarize adjacent tool/debug detail, but it must not be the only place where the user can see emitted progress text. diff --git a/tests/test_issue5339_restart_stale_user_dedup.py b/tests/test_issue5339_restart_stale_user_dedup.py index 55d449f5..3adf0031 100644 --- a/tests/test_issue5339_restart_stale_user_dedup.py +++ b/tests/test_issue5339_restart_stale_user_dedup.py @@ -186,6 +186,303 @@ def test_content_key_keeps_distinct_user_messages_distinct(): assert _session_message_content_key(one) != _session_message_content_key(two) +def test_native_multimodal_sidecar_reconciles_with_agent_text_projection(): + """A completed native-image turn keeps the rich sidecar row exactly once.""" + from api.models import merge_session_messages_append_only + + timestamp = 1766352000.123456 + prompt = ( + "[Workspace::v1: /workspace]\n" + "Describe this image\n\n" + "[Attached files: /absolute/path/image.png]" + ) + sidecar = { + "id": "webui-112", + "role": "user", + "content": [ + {"type": "text", "text": prompt}, + { + "type": "image_url", + "image_url": { + "url": "data:image/png;base64,ZmFrZS1pbWFnZS1ieXRlcw==", + }, + }, + ], + "attachments": ["image.png"], + "timestamp": timestamp, + } + state = { + "role": "user", + "content": f"{prompt}\n[screenshot]", + "timestamp": timestamp, + "api_content": "trusted provider wire content", + } + + merged = merge_session_messages_append_only([sidecar], [state]) + + assert merged == [sidecar] + assert merged[0] is sidecar + assert merged[0]["content"] is sidecar["content"] + assert merged[0]["attachments"] is sidecar["attachments"] + assert merged[0]["api_content"] == "trusted provider wire content" + + +def test_ordinary_reconciliation_preserves_conflicting_provider_sidecars(): + """Conflicting provider payloads remain separate ordinary rows.""" + from api.models import merge_session_messages_append_only + + timestamp = 1766352000.123456 + sidecar = { + "role": "assistant", + "content": "same answer", + "timestamp": timestamp, + "_state_db_row_id": 7, + "api_content": "wire-a", + } + state = { + "role": "assistant", + "content": "same answer", + "timestamp": timestamp, + "_state_db_row_id": 7, + "api_content": "wire-b", + } + + assert merge_session_messages_append_only([sidecar], [state]) == [sidecar, state] + + +def test_multimodal_mirror_requires_exact_timestamp_and_image_parts(): + """Cross-store image identity never participates in timestamp-free replay.""" + from api.models import ( + _session_message_multimodal_mirror_key, + merge_session_messages_append_only, + ) + + timestamp = 1766352000.123456 + multimodal = { + "id": "webui-image", + "role": "user", + "content": [ + {"type": "text", "text": "type [screenshot] literally"}, + {"type": "image", "image": "data:one"}, + {"type": "input_image", "image_url": "data:two"}, + ], + "timestamp": timestamp, + } + exact_mirror = { + "role": "user", + "content": "type [screenshot] literally\n[screenshot]\n[screenshot]", + "timestamp": timestamp, + } + mixed_case_role = {**exact_mirror, "role": " UsEr "} + assert _session_message_multimodal_mirror_key(mixed_case_role) == ( + _session_message_multimodal_mirror_key(exact_mirror) + ) + assert merge_session_messages_append_only([multimodal], [exact_mirror]) == [multimodal] + + later_literal = {**exact_mirror, "timestamp": timestamp + 0.5} + later_sidecar = { + "id": "later-assistant", + "role": "assistant", + "content": "later sidecar row", + "timestamp": timestamp + 1, + } + merged = merge_session_messages_append_only( + [multimodal, later_sidecar], + [later_literal], + ) + assert merged == [multimodal, later_literal, later_sidecar] + + unknown_parts = [ + {"type": "text", "text": "keep the original list identity"}, + {"type": "document", "document": {"id": "doc-1"}}, + ] + text_only_parts = [{"type": "text", "text": "not an image-bearing list"}] + malformed_text_parts = [ + {"type": "text", "text": 7}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AA=="}}, + ] + for parts in (unknown_parts, text_only_parts, malformed_text_parts, []): + sidecar = { + "id": f"structured-{len(parts)}", + "role": "user", + "content": parts, + "timestamp": timestamp, + } + assert _session_message_multimodal_mirror_key( + sidecar, + require_image_parts=True, + ) is None + + distinct_state = { + "role": "user", + "content": "different text\n[screenshot]\n[screenshot]", + "timestamp": timestamp, + } + assert len(merge_session_messages_append_only([multimodal], [distinct_state])) == 2 + + +def test_multimodal_mirror_collapses_identical_duplicate_state_rows(): + """Byte-identical scalar mirrors reconcile to the rich sidecar row.""" + from api.models import merge_session_messages_append_only + + timestamp = 1766352000.123456 + sidecar = { + "role": "user", + "content": [ + {"type": "text", "text": "describe this image"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AA=="}}, + ], + "timestamp": timestamp, + } + state = { + "role": "user", + "content": "describe this image\n[screenshot]", + "timestamp": timestamp, + } + + merged = merge_session_messages_append_only([sidecar], [state, dict(state)]) + + assert merged == [sidecar] + assert merged[0] is sidecar + + +def test_multimodal_mirror_state_identity_ambiguity_is_order_independent(): + """Contradictory state provenance does not fold every mirror into sidecar.""" + from api.models import merge_session_messages_append_only + + timestamp = 1766352000.123456 + sidecar = { + "role": "user", + "content": [ + {"type": "text", "text": "describe this image"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AA=="}}, + ], + "timestamp": timestamp, + } + state_rows = [ + { + "role": "user", + "content": "describe this image\n[screenshot]", + "timestamp": timestamp, + "_state_db_row_id": 7, + }, + { + "role": "user", + "content": "describe this image\n[screenshot]", + "timestamp": timestamp, + }, + { + "role": "user", + "content": "describe this image\n[screenshot]", + "timestamp": timestamp, + "_state_db_row_id": 8, + }, + ] + + merged = merge_session_messages_append_only([sidecar], state_rows) + + assert merged == [sidecar, state_rows[0]] + assert merged[0]["content"] is sidecar["content"] + assert merged[1]["_state_db_row_id"] == 7 + + +def test_multimodal_mirror_partial_identity_ambiguity_preserves_distinct_state_row(): + """An absent identity field never wildcards a distinct state-only row.""" + from api.models import merge_session_messages_append_only + + timestamp = 1766352000.123456 + + def make_rows(candidate_order): + sidecar = { + "role": "user", + "content": [ + {"type": "text", "text": "describe this image"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AA=="}}, + ], + "timestamp": timestamp, + } + identity_free_mirror = { + "role": "user", + "content": "describe this image\n[screenshot]", + "timestamp": timestamp, + } + identified_distinct_row = { + "role": "user", + "content": "describe this image\n[screenshot]", + "timestamp": timestamp, + "_state_db_row_id": 7, + "api_content": "distinct-turn-wire-content", + } + assistant = { + "role": "assistant", + "content": "distinct answer", + "timestamp": timestamp + 1, + "_state_db_row_id": 8, + "api_content": "assistant-wire-content", + } + candidates = [identity_free_mirror, identified_distinct_row] + ordered_candidates = ( + candidates + if candidate_order == "identity-free-first" + else list(reversed(candidates)) + ) + state_rows = [*ordered_candidates, assistant] + return ( + sidecar, + identity_free_mirror, + identified_distinct_row, + assistant, + state_rows, + ) + + for candidate_order in ("identity-free-first", "identified-first"): + sidecar, identity_free_mirror, distinct_row, assistant, state_rows = make_rows( + candidate_order + ) + merged = merge_session_messages_append_only([sidecar], state_rows) + + assert merged == [sidecar, *state_rows] + assert identity_free_mirror in merged + assert distinct_row in merged + assert assistant in merged + assert sidecar.get("api_content") is None + assert identity_free_mirror.get("api_content") is None + assert distinct_row["api_content"] == "distinct-turn-wire-content" + assert assistant["api_content"] == "assistant-wire-content" + + +def test_multimodal_mirror_rejects_conflicting_private_identity(): + """A projected image mirror never overrides contradictory provenance.""" + from api.models import merge_session_messages_append_only + + timestamp = 1766352000.123456 + base_sidecar = { + "role": "user", + "content": [ + {"type": "text", "text": "describe this image"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,AA=="}}, + ], + "timestamp": timestamp, + } + base_state = { + "role": "user", + "content": "describe this image\n[screenshot]", + "timestamp": timestamp, + } + conflicts = ( + ({"id": "sidecar-a", "message_id": "sidecar-b"}, {}), + ({"id": "sidecar"}, {"id": "state"}), + ({}, {"_state_db_row_id": True}), + ({"_state_db_row_id": 7}, {"_state_db_row_id": 8}), + ({"api_content": "wire-a"}, {"api_content": "wire-b"}), + ) + + for sidecar_identity, state_identity in conflicts: + sidecar = {**base_sidecar, **sidecar_identity} + state = {**base_state, **state_identity} + assert merge_session_messages_append_only([sidecar], [state]) == [sidecar, state] + + # --------------------------------------------------------------------------- # Delta-level: state_db_delta_after_context must not append a duplicate. # ---------------------------------------------------------------------------