Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 100 additions & 5 deletions src/octop/api/routers/chat/serialize.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@
from typing import Any

from octop.api.common.agent_workspace import resolve_agent_workspace_dir
from octop.i18n.domains.attachment import attachment_empty_image
from octop.infra.gateway.process.message_keys import (
COMPOSER_CTX_KEY,
INBOUND_ATTACHMENTS_KEY,
)
from octop.infra.utils.llm_text import strip_thinking as _strip_thinking
from octop.infra.utils.locale import normalize_locale

logger = logging.getLogger(__name__)

Expand All @@ -24,6 +26,21 @@
re.IGNORECASE,
)

# Matches the lightweight placeholder that ``MediaOffloadMiddleware`` writes
# into LangGraph state for already-offloaded inline images / audio. Format
# (see harness_agent.middleware.media_offload._placeholder_text_block):
# [<btype> offloaded: sha=<short_sha> path=<path> size=<n>B mime=<m>;
# use read_file to retrieve bytes]
# We strip these on history serialization because the original bytes are
# still available via ``inbound_attachments`` and the dashboard renders the
# thumbnail from there — leaving the placeholder visible made the chat show
# a "[image offloaded: sha=… path=…]" line under every user image after the
# second turn (when the middleware first re-encountered the block).
_OFFLOAD_PLACEHOLDER_RE = re.compile(
r"^\s*\[\s*(?:image|audio)\s+offloaded\s*:",
re.IGNORECASE,
)

# Must match harness_agent.agent.CHECKPOINT_TS_KEY (epoch-ms in additional_kwargs).
CHECKPOINT_TS_KEY = "checkpoint_ts"

Expand All @@ -35,6 +52,73 @@ def _clamp_history_limit(limit: int) -> int:
return max(1, min(limit, HISTORY_MAX_LIMIT))


def _is_offload_placeholder_block(block: Any) -> bool:
"""True when *block* is a ``MediaOffloadMiddleware`` placeholder.

The middleware rewrites an inline image/audio block into a single text
block of the form ``[image offloaded: sha=… path=… size=…B mime=…; use
read_file to retrieve bytes]`` on every turn after the first one. We
must not surface that text in the dashboard: the original attachment
is still available via ``inbound_attachments`` and the UI renders the
image from there. Showing the placeholder underneath is a UX bug.
"""
if not isinstance(block, dict):
return False
if str(block.get("type") or "").lower() != "text":
return False
text = str(block.get("text") or "")
return bool(_OFFLOAD_PLACEHOLDER_RE.match(text))


def _user_message_has_image_attachment(additional_kwargs: Any) -> bool:
"""True if the persisted ``INBOUND_ATTACHMENTS_KEY`` carries any image."""
if not isinstance(additional_kwargs, dict):
return False
raw = additional_kwargs.get(INBOUND_ATTACHMENTS_KEY)
if not isinstance(raw, list):
return False
for item in raw:
if not isinstance(item, dict):
continue
kind = str(item.get("kind") or "").lower()
media_type = str(item.get("media_type") or item.get("mediaType") or "")
if kind == "image" or media_type.lower().startswith("image/"):
return True
return False


def _strip_image_only_text_blocks(
blocks: list[dict[str, Any]],
*,
locale: str,
) -> list[dict[str, Any]]:
"""Drop placeholders + the LLM-facing "User sent an image." sentinel.

Only safe when the original image is also being delivered to the
dashboard via ``inbound_attachments``; if not, removing the text
would make a pure-image turn look empty in the UI.
"""
empty_image = attachment_empty_image(normalize_locale(locale)).strip()
out: list[dict[str, Any]] = []
for block in blocks:
if _is_offload_placeholder_block(block):
continue
if (
empty_image
and isinstance(block, dict)
and str(block.get("type") or "").lower() == "text"
and str(block.get("text") or "").strip() == empty_image
):
continue
out.append(block)
return out


def _user_locale(user: Any) -> str:
raw = getattr(user, "locale", None) if user is not None else None
return normalize_locale(str(raw) if raw else None)


def _slice_message_page(
raw: list[Any],
*,
Expand Down Expand Up @@ -112,7 +196,7 @@ async def _load_thread_messages(
offset,
)
for m in raw_messages:
entry = _serialize_history_message(m)
entry = _serialize_history_message(m, user=user)
if entry is not None:
out.append(entry)
except Exception:
Expand Down Expand Up @@ -474,7 +558,7 @@ def _split_string_thinking(text: str) -> list[dict[str, Any]]:
return blocks


def _serialize_history_message(msg: Any) -> dict[str, Any] | None:
def _serialize_history_message(msg: Any, *, user: Any = None) -> dict[str, Any] | None:
"""Project a LangGraph checkpoint message into dashboard history shape."""
role = _message_role(msg)
if role in ("system", ""):
Expand Down Expand Up @@ -512,7 +596,19 @@ def _serialize_history_message(msg: Any) -> dict[str, Any] | None:
if role == "assistant":
blocks.extend(_tool_use_blocks(_msg_attr(msg, "tool_calls")))

if not blocks:
raw_att = (
additional_kwargs.get(INBOUND_ATTACHMENTS_KEY)
if isinstance(additional_kwargs, dict)
else None
)
has_user_attachments = isinstance(raw_att, list) and bool(raw_att)
if role == "user" and _user_message_has_image_attachment(additional_kwargs):
# The original image is being delivered through inbound_attachments;
# the image/audio offload placeholders and the LLM-only "User sent
# an image." sentinel are redundant noise on the dashboard.
blocks = _strip_image_only_text_blocks(blocks, locale=_user_locale(user))

if not blocks and not (role == "user" and has_user_attachments):
return None

entry = {"role": role, "content": blocks}
Expand All @@ -524,8 +620,7 @@ def _serialize_history_message(msg: Any) -> dict[str, Any] | None:
raw_ctx = additional_kwargs.get(COMPOSER_CTX_KEY)
if isinstance(raw_ctx, dict) and raw_ctx:
entry["composer_context"] = raw_ctx
raw_att = additional_kwargs.get(INBOUND_ATTACHMENTS_KEY)
if isinstance(raw_att, list) and raw_att:
if has_user_attachments:
entry["inbound_attachments"] = raw_att
ts_ms = _extract_message_timestamp_ms(msg)
if ts_ms is not None:
Expand Down
188 changes: 188 additions & 0 deletions tests/unit/api/test_chat_polish.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from __future__ import annotations

from types import SimpleNamespace
from typing import Any
from unittest.mock import AsyncMock, MagicMock

Expand All @@ -10,8 +11,10 @@

from octop.api.routers.chat.serialize import (
_entry_matches_thread,
_is_offload_placeholder_block,
_merge_adjacent_messages,
_serialize_history_message,
_strip_image_only_text_blocks,
_strip_thinking,
_ts_to_ms,
)
Expand Down Expand Up @@ -181,6 +184,191 @@ def test_ts_to_ms_converts_seconds() -> None:
assert _ts_to_ms(1_700_000_000.5) == 1_700_000_000_500


# ---------------------------------------------------------------------------
# Image-offload placeholder filtering on history serialization
# ---------------------------------------------------------------------------


_PLACEHOLDER_TEXT = (
"[image offloaded: sha=06bdd82d592a "
"path=C:\\Users\\me\\.octop\\agents\\DMQ318\\.media-cache/"
"06bdd82d592a5cd6371ccdb2ed49d347a4ffb0fca4b80ea57e991f51522e178e.png "
"size=173004B mime=image/png; use read_file to retrieve bytes]"
)


def test_is_offload_placeholder_block_matches_image_format() -> None:
assert _is_offload_placeholder_block({"type": "text", "text": _PLACEHOLDER_TEXT})
# Same shape, but a 12-char short sha and trailing "]".
assert _is_offload_placeholder_block(
{
"type": "text",
"text": " [audio offloaded: sha=abcdef012345 path=/x.y size=1B mime=audio/mpeg; use read_file to retrieve bytes] ",
}
)


def test_is_offload_placeholder_block_rejects_unrelated_text() -> None:
assert not _is_offload_placeholder_block({"type": "text", "text": "hello"})
assert not _is_offload_placeholder_block(
{"type": "text", "text": "[image] some other bracket text"}
)
# Image block (not text) is not a placeholder.
assert not _is_offload_placeholder_block(
{"type": "image_url", "image_url": {"url": "data:..."}}
)
assert not _is_offload_placeholder_block("not a dict")


def test_serialize_history_message_drops_image_offload_placeholder_for_image_user() -> None:
msg = HumanMessage(
content=[
{"type": "text", "text": "用户发送了图片。"},
{"type": "text", "text": _PLACEHOLDER_TEXT},
],
additional_kwargs={
"octop_inbound_attachments": [
{
"filename": "image.png",
"media_type": "image/png",
"kind": "image",
"workspace_path": "inbound/123_image.png",
}
],
},
)
user = SimpleNamespace(locale="zh")
entry = _serialize_history_message(msg, user=user)
assert entry is not None
# Both the localized "User sent an image." sentinel and the offload
# placeholder must be stripped — only the image (rendered from
# inbound_attachments) is meaningful on the dashboard.
assert entry["content"] == []
# Attachments are still propagated for the frontend to render the image.
assert entry["inbound_attachments"][0]["workspace_path"] == "inbound/123_image.png"


def test_serialize_history_message_keeps_user_caption_alongside_image_placeholder() -> None:
msg = HumanMessage(
content=[
{"type": "text", "text": "请帮我看看这张图"},
{"type": "text", "text": _PLACEHOLDER_TEXT},
],
additional_kwargs={
"octop_inbound_attachments": [
{
"filename": "image.png",
"media_type": "image/png",
"kind": "image",
"workspace_path": "inbound/123_image.png",
}
],
},
)
user = SimpleNamespace(locale="zh")
entry = _serialize_history_message(msg, user=user)
assert entry is not None
# Only the offload placeholder is dropped; the user-written caption
# is preserved verbatim so the dashboard still shows it.
assert entry["content"] == [{"type": "text", "text": "请帮我看看这张图"}]


def test_serialize_history_message_keeps_placeholder_when_no_image_attachment() -> None:
"""Without inbound_attachments, the placeholder is the only sign of media."""
msg = HumanMessage(
content=[{"type": "text", "text": _PLACEHOLDER_TEXT}],
)
# No additional_kwargs → no inbound_attachments → no filtering.
entry = _serialize_history_message(msg, user=SimpleNamespace(locale="en"))
assert entry is not None
assert entry["content"] == [{"type": "text", "text": _PLACEHOLDER_TEXT}]


def test_serialize_history_message_drops_placeholder_for_en_locale() -> None:
msg = HumanMessage(
content=[
{"type": "text", "text": "User sent an image."},
{"type": "text", "text": _PLACEHOLDER_TEXT},
],
additional_kwargs={
"octop_inbound_attachments": [
{
"filename": "image.png",
"media_type": "image/png",
"kind": "image",
"workspace_path": "inbound/123_image.png",
}
],
},
)
entry = _serialize_history_message(msg, user=SimpleNamespace(locale="en"))
assert entry is not None
assert entry["content"] == []


def test_strip_image_only_text_blocks_without_user_skips_zh_default() -> None:
"""Locale falls back to ``zh`` when no user is supplied."""
msg = HumanMessage(
content=[
{"type": "text", "text": "用户发送了图片。"},
{"type": "text", "text": _PLACEHOLDER_TEXT},
],
additional_kwargs={
"octop_inbound_attachments": [
{
"filename": "image.png",
"media_type": "image/png",
"kind": "image",
"workspace_path": "inbound/x.png",
}
],
},
)
# Pass no user — caller signature is ``user=None`` default.
entry = _serialize_history_message(msg)
assert entry is not None
assert entry["content"] == []


def test_strip_image_only_text_blocks_keeps_voice_caption() -> None:
"""A non-image attachment must not trigger placeholder stripping."""
msg = HumanMessage(
content=[
{"type": "text", "text": "请听这段录音"},
{"type": "text", "text": _PLACEHOLDER_TEXT}, # NOT real, but tests shape
],
additional_kwargs={
"octop_inbound_attachments": [
{
"filename": "voice.m4a",
"media_type": "audio/mp4",
"kind": "file",
"workspace_path": "inbound/voice.m4a",
}
],
},
)
entry = _serialize_history_message(msg, user=SimpleNamespace(locale="zh"))
assert entry is not None
# The audio file is rendered from inbound_attachments, not as a
# placeholder text block, so the on-disk placeholder is the only
# signal — leave it alone.
assert entry["content"] == [
{"type": "text", "text": "请听这段录音"},
{"type": "text", "text": _PLACEHOLDER_TEXT},
]


def test_strip_image_only_text_blocks_directly() -> None:
blocks = [
{"type": "text", "text": "用户发送了图片。"},
{"type": "text", "text": _PLACEHOLDER_TEXT},
{"type": "text", "text": "你好"},
]
out = _strip_image_only_text_blocks(blocks, locale="zh")
assert out == [{"type": "text", "text": "你好"}]


@pytest.mark.asyncio
async def test_load_checkpoint_messages_falls_back_to_graph_state() -> None:
from octop.api.routers.chat.serialize import _load_checkpoint_messages
Expand Down
Loading