diff --git a/CHANGELOG.md b/CHANGELOG.md index b98c7d8..6e0af00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,91 @@ the GitHub Release body, so a release with no entry here fails. Versioning follows [docs/versioning.md](docs/versioning.md). +## [0.10.0] - 2026-09-08 + +### Added + +- A tool can return images that the model itself looks at, instead of prose from + a second model describing them. Return + `tool_content(text, images=[image_attachment(png_bytes, "image/png", label=path)])` + from `agent_core.tool_content` — a plain JSON dict, not a dataclass, so it + survives the `json.dumps` boundary a sandbox-native tool's return value + crosses. `ToolResult` grows an `images` field; `result` remains the full text, + so observers, spill/recovery, repeat detection and the trajectory see exactly + what they saw before. + + Whether the pixels reach the provider is decided by the loop, from + `ModelProfile.supports_images` (default `False`, so nothing changes for a + product that does not opt in) and the wire protocol. Only + `chat_completions` is implemented: the attachment becomes an OpenAI + `image_url` content part carrying a data URI. The Anthropic block spelling is + **not** interchangeable — a served OpenAI-compatible endpoint rejects it with + HTTP 400 and a pydantic union error, taking the whole turn down, so an + unimplemented protocol withholds rather than guessing. + + `HistoryPolicy.max_images_in_history` (default 5) now does something: after + each tool batch the loop keeps that many of the newest images and replaces + the rest with text. Both flags existed as unread placeholders before this + release. + + **An image that leaves the context always leaves a sentence behind** — when + it is withheld for capability reasons, and when it is evicted for room. This + is not cosmetic. In the calibration run, deleting the image block from an + otherwise working request did not make the model report a missing image: it + reported a four-digit code and three shapes, all invented. A tool result that + reads as though an image were delivered will be answered as though one were. + Products adding their own image paths should preserve this property. + +### Fixed + +- Structured image results now reject Base64-valid data that is not a + recognizable PNG, JPEG, WEBP or GIF, and reject a declared MIME type that + disagrees with the payload. Lightweight container checks also reject common + truncation and corruption before delivery (including PNG chunk/CRC damage). + The Apodex endpoint otherwise fails the entire completion while decoding the + bad image. Token-accounting dimensions are always read from the payload, so + stale producer metadata cannot price a 4K image as one token and bypass the + context guard. +- `ExtensionsConfig.has_changed` compares a digest of the file's bytes instead + of `st_mtime > loaded_mtime`, so a skill toggled on disk is actually picked up + by `get_enabled_skills`. Timestamps are much coarser than the edits they were + being asked to order: the filesystem clock advances in 1 ms steps and two + consecutive writes collide on a single mtime about 92% of the time, so a + change landing in the same millisecond as the load was invisible. The strict + `>` also could not see a timestamp moving BACKWARD -- restoring a backup, a + `git checkout`, an `rsync --times` of an older revision -- and it reported a + change for an identical rewrite, forcing a reload with nothing to reload. + This surfaced as an intermittent failure in `test_skills_loader_reload.py` + whose rate tracked machine speed; the regression tests now pin both + timestamps to one value and fail deterministically without the fix. + +### Changed + +- `messages.text_of` renders an `image_url` content block as + `[image — not visible in this text-only rendering]` instead of the empty + string. Every caller is either sizing a message or building a text-only + rendering — the Anthropic message translation, a compaction summary, a + trajectory line — and in the rendering case the image is being dropped at + that call. **If a product already puts `image_url` blocks in `content`, its + flattened text changes**; nothing else in this package produced such blocks + before 0.10.0. +- `tokens.estimate_message_tokens` charges for inline images, read off the + `image_meta` message key written at attach time (~1 token per 1024 pixels, + fitted against measured `prompt_tokens`; a 1080p screenshot is ~2.4K tokens, + a 4K one ~8.5K). It previously returned the length of the caption alone, so a + history of screenshots measured as nearly empty to the context guard and to + every compaction trigger. Estimates for image-free histories are unchanged. +- `TrajectoryFileObserver` writes `[N KB of image data elided from trace]` in + place of an inline image's base64, via the new + `tool_content.redacted_for_trace`. Live tool-result JSON and JSONL entries + record the same redacted shape from `ToolResult.images`; previously that path + kept only the text and omitted that the tool returned an image. The block + keeps its `image_url` type and states its size, without copying the Base64. +- `Message` gains the in-process key `image_meta`, positionally aligned with the + `image_url` blocks in `content`. It is outside `WIRE_MESSAGE_KEYS`, so + `for_wire` strips it. It is message-level rather than per-block because a + content part carrying an unknown key is rejected by the served endpoint. + ## [0.9.1] - 2026-09-08 ### Fixed diff --git a/agent_core/components/observers/trajectory.py b/agent_core/components/observers/trajectory.py index b11649b..8df03d2 100644 --- a/agent_core/components/observers/trajectory.py +++ b/agent_core/components/observers/trajectory.py @@ -24,6 +24,7 @@ ToolResult, TurnContext, ) +from agent_core.tool_content import redacted_for_trace, redacted_tool_result_content _FORMATS: tuple[str, ...] = ("json", "jsonl") _DEFAULT_FORMATS: tuple[str, ...] = _FORMATS @@ -399,7 +400,10 @@ def _message_to_dict(self, m: Any) -> dict | None: is a copy. Anything that isn't a role-bearing dict is dropped. """ if isinstance(m, dict) and m.get("role"): - return dict(m) + # ``redacted_for_trace`` swaps an inline image's base64 for its + # size. Verbatim, one 1080p screenshot writes ~137 KB here for + # every turn it stays in history. + return dict(redacted_for_trace(m)) return None # ── Lifecycle hooks ───────────────────────────────────────────────── @@ -563,7 +567,7 @@ async def on_tool_result( # — that fallback advances ``_tool_results_seen``, so sharing it would # double-count, and a synthesised id matches nothing outside the # snapshot anyway. Empty here means the runtime itself had no id. - self._write_jsonl({ + jsonl_record: dict[str, Any] = { "t": "result", "turn": ctx.turn, "name": result.name, @@ -571,7 +575,12 @@ async def on_tool_result( "result": result.result, "error": result.is_error, "ms": result.duration_ms, - }) + } + if result.images: + # The result event predates message attachment/capability gating, so + # this states what the TOOL returned, not that every profile saw it. + jsonl_record["images"] = redacted_tool_result_content("", result.images) + self._write_jsonl(jsonl_record) if "json" in self._formats: cid = getattr(result, "tool_call_id", "") or "" @@ -584,10 +593,14 @@ async def on_tool_result( ) self._tool_results_seen[ctx.turn] = seen + 1 body = _clip(self._stringify(result.result), _BODY_MAX_CHARS) + rendered_body = f"[error] {body}" if result.is_error else body self._append_message({ "role": "tool", "tool_call_id": cid, - "content": f"[error] {body}" if result.is_error else body, + "content": redacted_tool_result_content( + rendered_body, + result.images, + ), }) self._flush_json() return None diff --git a/agent_core/components/skills/extensions_config.py b/agent_core/components/skills/extensions_config.py index 49abe6c..046114c 100644 --- a/agent_core/components/skills/extensions_config.py +++ b/agent_core/components/skills/extensions_config.py @@ -7,10 +7,10 @@ from __future__ import annotations +import hashlib import json import logging import os -from contextlib import suppress from pathlib import Path from typing import Any @@ -56,7 +56,8 @@ class ExtensionsConfig(BaseModel): skills: dict[str, SkillStateConfig] = Field(default_factory=dict) _file_path: Path | None = PrivateAttr(default=None) - _file_mtime: float = PrivateAttr(default=0.0) + # Digest of the bytes this config was parsed from -- see ``has_changed``. + _file_digest: str = PrivateAttr(default="") model_config = {"populate_by_name": True} @@ -79,14 +80,15 @@ def from_file(cls, config_path: str | Path | None = None) -> ExtensionsConfig: return cls() try: - with open(resolved, encoding="utf-8") as f: - data = json.load(f) + raw = resolved.read_bytes() + data = json.loads(raw.decode("utf-8")) _resolve_env_variables(data) logger.info("Loaded extensions config from %s", resolved) instance = cls.model_validate(data) instance._file_path = resolved - with suppress(OSError): - instance._file_mtime = resolved.stat().st_mtime + # Digest the exact bytes that were parsed, so the stored fingerprint + # and the loaded state can never describe different file contents. + instance._file_digest = _digest(raw) return instance except Exception as e: logger.warning("Failed to load extensions config %s: %s", resolved, e) @@ -103,11 +105,33 @@ def source_path(self) -> Path | None: return self._file_path def has_changed(self) -> bool: - """Return True if the backing file has been modified since load.""" + """Return True if the backing file's contents differ from what we hold. + + Compares a digest of the bytes, not the modification time. Two reasons, + both of which bit this code: + + Timestamps are far coarser than the edits they are meant to order. The + filesystem clock here advances in 1 ms steps, and two consecutive writes + land on an identical mtime about 92% of the time -- so a change made + within a millisecond of the load was simply invisible, and an operator + toggling a skill got the old state until something else touched the + file. That was reaching the test suite as an intermittent failure whose + rate tracked how fast the machine happened to be running. + + A strict ``>`` also cannot see a file whose timestamp moves BACKWARD, + which is the normal outcome of restoring a backup, a ``git checkout``, + or an ``rsync --times`` of an older revision. The content changed; the + config went on reporting that it had not. + + The file is a small JSON document and this is called from + ``get_enabled_skills``, which its callers cache -- reading it is cheaper + than being wrong about it. An identical rewrite correctly reports no + change, since nothing needs reloading. + """ if self._file_path is None or not self._file_path.is_file(): return False try: - return self._file_path.stat().st_mtime > self._file_mtime + return _digest(self._file_path.read_bytes()) != self._file_digest except OSError: return False @@ -117,6 +141,11 @@ def is_skill_enabled(self, skill_name: str) -> bool: return state.enabled if state else True +def _digest(raw: bytes) -> str: + """Content fingerprint. Not a security boundary -- just change detection.""" + return hashlib.blake2b(raw, digest_size=16).hexdigest() + + def _resolve_env_variables(obj: Any) -> Any: """Recursively replace $VAR_NAME with environment variable values.""" if isinstance(obj, str) and obj.startswith("$"): diff --git a/agent_core/loop_types.py b/agent_core/loop_types.py index 0bae9a6..4920482 100644 --- a/agent_core/loop_types.py +++ b/agent_core/loop_types.py @@ -312,6 +312,13 @@ class ToolResult: error_kind: str = "" # Opaque host-owned handle for a result body shed from model context. result_id: str = "" + # Image attachments the tool returned alongside ``result``, validated by + # ``agent_core.tool_content.parse_tool_content``. ``result`` stays the + # complete TEXT of the result, so every consumer that reads a string -- + # observers, spill/recovery, repeat detection, the trajectory -- is + # unaffected by a tool that attaches images. Only the message built for the + # provider looks at this, and only when the model can actually see it. + images: list[dict[str, Any]] = field(default_factory=list[dict[str, Any]]) # Host-provided repeated-invocation metadata. Execution is never skipped. repeat_count: int = 1 repeat_recovery_id: str = "" diff --git a/agent_core/messages.py b/agent_core/messages.py index 52225db..97f5f16 100644 --- a/agent_core/messages.py +++ b/agent_core/messages.py @@ -75,6 +75,15 @@ class Message(TypedDict, total=False): # the text back, which is what makes an index distinguishable from a summary # that happens to quote one. Filtered out by ``for_wire``. spill_refs: list[str] + # Per-image bookkeeping for the ``image_url`` blocks in ``content``, in the + # same order: ``{"label": str, "tokens": int}`` each. Written by + # ``runtime.loop.image_attach.attach_images`` and read back by eviction (to + # name an image it is removing) and by the token estimate (so a history of + # images is not costed at zero). It cannot live inside the content blocks: + # a content part carrying an unknown key is rejected outright by the served + # endpoint's pydantic union, whereas a message-level key outside + # ``WIRE_MESSAGE_KEYS`` is dropped by ``for_wire``. + image_meta: list[dict[str, Any]] # ── Wire boundary ──────────────────────────────────────────────────────── @@ -222,6 +231,10 @@ def assistant_msg_with_reasoning( # ── Helpers ────────────────────────────────────────────────────────────── +# What an inline image renders as once its blocks are flattened away. +_IMAGE_PLACEHOLDER = "[image — not visible in this text-only rendering]" + + def text_of(content: Any) -> str: """Flatten an OpenAI/Anthropic message content to plain text.""" if content is None: @@ -235,6 +248,18 @@ def text_of(content: Any) -> str: parts.append(block) elif isinstance(block, dict): content_block = cast(dict[str, object], block) + # An image flattened to text must leave a mark. Every caller is + # either estimating size or building a text-only rendering -- + # the Anthropic translation, a compaction summary, a trajectory + # line -- and in the rendering case the image is being dropped + # right here. Returning nothing for it yields a tool result that + # reads as though it were pure text and never mentioned an + # image, which is the exact input that made the calibration + # model invent a reading of a picture it could not see (see + # ``runtime.loop.image_attach``). + if content_block.get("type") == "image_url": + parts.append(_IMAGE_PLACEHOLDER) + continue val = content_block.get("text") or content_block.get("content") or "" if isinstance(val, str): parts.append(val) diff --git a/agent_core/runtime/loop/agent_loop.py b/agent_core/runtime/loop/agent_loop.py index 5992128..f9b5889 100644 --- a/agent_core/runtime/loop/agent_loop.py +++ b/agent_core/runtime/loop/agent_loop.py @@ -49,6 +49,7 @@ DefaultMessageCompactor, estimate_tokens, ) +from agent_core.runtime.loop.image_attach import attach_images, evict_old_images from agent_core.runtime.loop.llm_client import ( RUNAWAY_STATE_KEY, TRUNCATION_CONTINUATION_GUIDANCE, @@ -527,6 +528,8 @@ async def _run_loop_inner( runtime.body_has_spill_reference, runtime.render_tool_result, result_max_chars=tool_result_cap, + profile=profile, + max_images_in_history=policy.max_images_in_history, ) total_tool_calls += tool_calls_executed if stop_reason: @@ -1259,6 +1262,10 @@ async def _execute_tool_calls( ], *, result_max_chars: int | None = None, + profile: ModelProfile | None = None, + # -1 disables eviction. Not 0: a defaulted caller must not silently mean + # "throw every image away", which is what a 0 default would spell. + max_images_in_history: int = -1, ) -> tuple[str, int]: executable: list[tuple[int, dict]] = [] synthetic: list[tuple[int, ToolResult]] = [] @@ -1356,8 +1363,26 @@ async def _execute_tool_calls( tr_result.tool_call_id, ) cast("dict[str, Any]", history_message).update(message_metadata) + # After the recovery handle and the host metadata, so the text the model + # reads is final before it becomes the text block of a multimodal + # message. Called unconditionally: when the model cannot see images + # ``attach_images`` writes the note saying so, and a caller that skipped + # it on capability grounds would produce the one shape that is actually + # dangerous -- a result reading as though an image had been delivered, + # with no image in it. + attach_images( + history_message, + tr_result.images, + profile=profile or ModelProfile(model_id="default", provider="openai"), + ) messages.append(history_message) + # Bound the history's image count once the batch is in, not before: the + # newest results are the ones worth keeping, and evicting first would let a + # turn that returned several images push out its own. + if max_images_in_history >= 0: + evict_old_images(messages, max_images_in_history) + if any(result.interrupted for result in results): wait_interventions = await notify_observers(obs, "on_tool_wait_interrupted", ctx) merged_wait = merge_interventions(wait_interventions) diff --git a/agent_core/runtime/loop/image_attach.py b/agent_core/runtime/loop/image_attach.py new file mode 100644 index 0000000..5f3a9a3 --- /dev/null +++ b/agent_core/runtime/loop/image_attach.py @@ -0,0 +1,234 @@ +"""Put a tool's image attachments into the message the provider sees. + +Two decisions live here and nowhere else: whether an attachment is allowed onto +the wire at all, and how many stay in history as the run goes on. Both resolve +the same way when the answer is no -- the image is replaced by a sentence saying +an image was there and is not any more. + +That is the point of the module rather than an incidental nicety. The +calibration run behind this feature (MiroHarness +``internal-docs/designs/2026-09-08-native-image-in-tool-result-calibration.md``, +case E) took a working request and deleted only the image block. The model was instructed to +answer ``NO_IMAGE`` if it could not see an image. It instead reported a +four-digit code and three shapes, all confidently wrong. A model handed a tool +result that reads like an image was delivered will describe the image it +expects; only text that contradicts that expectation stops it. + +Per-image bookkeeping (label, estimated tokens) rides on the message under +``image_meta``, positionally aligned with the ``image_url`` blocks in +``content``. It is deliberately message-level and not inside the blocks: a +content part with an unrecognised key is rejected by the served endpoint's +pydantic union, while a message-level key outside ``WIRE_MESSAGE_KEYS`` is +dropped by :func:`agent_core.messages.for_wire` before the request is built. +Keeping the token estimate here is also what stops the context guard from +having to base64-decode the whole history on every turn to find out how big it +is. +""" + +from __future__ import annotations + +import logging +from typing import Any, cast + +from agent_core.messages import Message +from agent_core.runtime.loop.model_profile import ModelProfile +from agent_core.tool_content import ( + image_tokens, + message_image_meta, + message_image_tokens, +) + +logger = logging.getLogger(__name__) + +__all__ = [ + "attach_images", + "evict_old_images", + "image_blocks_in", + "message_image_tokens", +] + +# Wire protocols whose inline-image block shape is the one built below. +# Anthropic Messages and the OpenAI Responses API both accept images and both +# spell the block differently (``{"type": "image", "source": {...}}`` / +# ``{"type": "input_image", ...}``). This is not a shape a server tolerates and +# ignores when wrong: the calibration run sent the Anthropic spelling to the +# OpenAI-compatible endpoint and got HTTP 400 with a pydantic union error, which +# fails the entire turn including the other tool results in it. So the protocol +# is checked rather than assumed, and an unhandled one withholds with a note. +_INLINE_IMAGE_PROTOCOLS = frozenset({"chat_completions"}) + + +def _as_block(block: Any) -> dict[str, Any] | None: + """A content entry as a keyed block, or ``None`` if it is not one.""" + return cast("dict[str, Any]", block) if isinstance(block, dict) else None + + +def _is_image(block: Any) -> bool: + entry = _as_block(block) + return entry is not None and entry.get("type") == "image_url" + + +def attach_images( + message: Message, + images: list[dict[str, Any]], + *, + profile: ModelProfile, +) -> None: + """Add *images* to a tool *message*, in place. + + When the model cannot take them, the message keeps plain-string content and + gains an explicit note. Callers never branch on capability: they always call + this, and the message is correct either way. + """ + if not images: + return + + why = "" + if not profile.supports_images: + why = f"the model in use ({profile.model_id}) cannot accept images" + elif profile.protocol not in _INLINE_IMAGE_PROTOCOLS: + why = ( + f"inline images are not implemented for the {profile.protocol} " + "wire protocol" + ) + if why: + logger.info("withholding %d image(s) from tool message: %s", len(images), why) + text = _text_content(message) + note = _withheld_note(images, why) + message["content"] = f"{text}\n\n{note}" if text else note + return + + text = _text_content(message) + blocks: list[dict[str, Any]] = [] + if text: + blocks.append({"type": "text", "text": text}) + meta: list[dict[str, Any]] = [] + for image in images: + blocks.append({ + "type": "image_url", + "image_url": { + "url": f"data:{image['mime_type']};base64,{image['data']}", + }, + }) + meta.append({ + "label": str(image.get("label") or ""), + "tokens": image_tokens(image), + }) + message["content"] = blocks + cast("dict[str, Any]", message)["image_meta"] = meta + + +def evict_old_images(messages: list[Message], max_images: int) -> int: + """Keep only the newest *max_images* inline images; return how many went. + + Walks newest-first, so what survives is what the model is most likely to + still be working from. An evicted image leaves a sentence behind, and a + message left with no images goes back to plain-string content -- there is no + reason to keep a block list, and a string is what every checkpoint, replay + and text-flattening path handles most cheaply. + """ + if max_images < 0: + return 0 + + kept = 0 + evicted = 0 + for message in messages[::-1]: + content = message.get("content") + if not isinstance(content, list): + continue + meta = message_image_meta(message) + blocks = cast("list[Any]", content) + image_positions = [ + index for index, block in enumerate(blocks) if _is_image(block) + ] + if not image_positions: + continue + # Newest-first WITHIN the message too, not just across messages. A + # single tool result can return several images (a PDF rendered page by + # page), and walking its blocks forward while walking the history + # backward keeps the wrong end of that result. + drop_at: set[int] = set() + for index in reversed(image_positions): + if kept < max_images: + kept += 1 + continue + drop_at.add(index) + evicted += 1 + if not drop_at: + continue + at_image = set(image_positions) + rebuilt: list[Any] = [] + surviving: list[dict[str, Any]] = [] + seen = 0 + for index, block in enumerate(blocks): + if index not in at_image: + rebuilt.append(block) + continue + entry = meta[seen] if seen < len(meta) else {} + seen += 1 + if index in drop_at: + rebuilt.append({"type": "text", "text": _evicted_note(entry)}) + else: + rebuilt.append(block) + surviving.append(entry) + if surviving: + message["content"] = rebuilt + cast("dict[str, Any]", message)["image_meta"] = surviving + else: + message["content"] = "\n".join( + str(entry.get("text") or "") + for entry in (_as_block(block) for block in rebuilt) + if entry is not None and entry.get("text") + ) + cast("dict[str, Any]", message).pop("image_meta", None) + if evicted: + logger.info( + "evicted %d image(s) from history, keeping the newest %d", + evicted, max_images, + ) + return evicted + + +def image_blocks_in(message: Message) -> int: + """How many inline images this message currently carries.""" + content = message.get("content") + if not isinstance(content, list): + return 0 + return sum(1 for block in cast("list[Any]", content) if _is_image(block)) + + +def _withheld_note(images: list[dict[str, Any]], why: str) -> str: + named = ", ".join( + label for label in (str(i.get("label") or "") for i in images) if label + ) + subject = "1 image" if len(images) == 1 else f"{len(images)} images" + where = f" ({named})" if named else "" + return ( + f"[{subject}{where} could not be shown to you: {why}. You have NOT " + "seen this image. Do not describe, transcribe, or draw any conclusion " + "from its contents.]" + ) + + +def _evicted_note(entry: dict[str, Any]) -> str: + label = str(entry.get("label") or "") + where = f" {label}" if label else "" + return ( + f"[An image{where} was here and has been dropped from context to make " + "room. You can no longer see it. Read it again if you still need it, " + "and do not rely on remembering what it showed.]" + ) + + +def _text_content(message: Message) -> str: + content = message.get("content") + if isinstance(content, str): + return content + if isinstance(content, list): + parts = [ + str(entry.get("text") or "") + for entry in (_as_block(block) for block in cast("list[Any]", content)) + if entry is not None and entry.get("type") == "text" + ] + return "\n".join(part for part in parts if part) + return "" if content is None else str(content) diff --git a/agent_core/runtime/loop/tool_exec.py b/agent_core/runtime/loop/tool_exec.py index 10c97cb..ba027a1 100644 --- a/agent_core/runtime/loop/tool_exec.py +++ b/agent_core/runtime/loop/tool_exec.py @@ -13,6 +13,7 @@ from typing import Any, Protocol, runtime_checkable from agent_core.loop_types import ToolResult +from agent_core.tool_content import parse_tool_content logger = logging.getLogger(__name__) @@ -335,7 +336,16 @@ async def _run_one(call: dict[str, Any], index: int) -> ToolResult: else: raw = await invocation - result = runtime.transform_result(name, str(raw) if raw is not None else "") + # A tool may hand back text plus image attachments instead of a + # bare string. Split them BEFORE the stringify: ``str()`` on the + # envelope would put a base64 blob's repr into the model's context + # and lose the attachment at the same time. + structured = parse_tool_content(raw) + if structured is not None: + raw_text, images = structured + else: + raw_text, images = (str(raw) if raw is not None else ""), [] + result = runtime.transform_result(name, raw_text) metadata_raw = _safe_hook( "result_metadata", lambda: runtime.result_metadata(name, args, raw, result), @@ -354,6 +364,7 @@ async def _run_one(call: dict[str, Any], index: int) -> ToolResult: duration_ms=int((time.monotonic() - start) * 1000), tool_call_id=tool_call_id, is_error=bool(error_kind), + images=images, interrupted=woke_for_interrupt, error_kind=error_kind, result_id=str(metadata.get("result_id") or ""), diff --git a/agent_core/tokens.py b/agent_core/tokens.py index 05cd7e5..106b9c3 100644 --- a/agent_core/tokens.py +++ b/agent_core/tokens.py @@ -32,6 +32,7 @@ from typing import Any, cast from agent_core.messages import text_of +from agent_core.tool_content import message_image_tokens # Per-message wire overhead (role, delimiters, the trailing separator). Same # constant ``compact.estimate_tokens`` adds when it sums a whole history. @@ -84,4 +85,10 @@ def estimate_message_tokens(message: Any) -> int: tokens = estimate_text_tokens(text_of(content)) + _PER_MESSAGE_OVERHEAD if tool_calls: tokens += estimate_text_tokens(_tool_calls_text(tool_calls)) + # Inline images contribute no text, so without this a tool result carrying a + # 1080p screenshot -- about 2.4K prompt tokens on the model this was + # calibrated against -- measures as the length of its caption. The context + # guard and every compaction trigger read this number, so undercounting it + # is how a history walks into an overflow the guard reported as comfortable. + tokens += message_image_tokens(message) return tokens diff --git a/agent_core/tool_content.py b/agent_core/tool_content.py new file mode 100644 index 0000000..4fcf1a2 --- /dev/null +++ b/agent_core/tool_content.py @@ -0,0 +1,510 @@ +"""Structured tool output: text plus image attachments the model itself reads. + +A tool normally returns a string and that string becomes the whole tool message. +Some tools have something to hand back that is not text — an image. The +alternative to this module is what the products did before it: call a separate +vision model, and put ITS prose transcript into the history. That is a lossy +round trip (the main model never sees the pixels, only another model's summary +of them, and cannot go back and look again when a later turn raises a new +question about the same picture) and it is not cheaper — a transcript runs +2-6K tokens where the image it describes is a few hundred. + +The wire shape a tool returns is a plain JSON dict, never a dataclass, because +sandbox-native tools are executed in a child process and their return value +crosses a ``json.dumps(default=str)`` boundary on the way back +(``plugins/tool_runtime/server.py`` in MiroHarness). A dataclass survives that +trip as its repr. Build it with :func:`tool_content`:: + + return tool_content( + "Screenshot of the failing dialog:", + images=[image_attachment(png_bytes, "image/png", label="/tmp/shot.png")], + ) + +Nothing downstream is obliged to honour it. Whether the attachment reaches the +provider is decided in the loop, from ``ModelProfile.supports_images`` and the +wire protocol — a model with no vision gets the text and an explicit note that +an image was withheld. That note is not politeness. In the calibration run for +this feature the control case removed the image block and left everything else +identical: the model did not report a missing image, it invented a confident, +wrong reading of one. An image that silently vanishes from a tool result is +therefore a correctness bug, not a degraded capability, and every path here that +declines to attach says so in the text instead. + +The measurements behind the constants in this module — the pixels-per-token +figure, the caps, the block shape the server actually accepts — are in +MiroHarness ``internal-docs/designs/2026-09-08-native-image-in-tool-result-calibration.md``. +Read it before changing any of them. +""" + +from __future__ import annotations + +import base64 +import binascii +import logging +import zlib +from typing import Any, cast + +logger = logging.getLogger(__name__) + +__all__ = [ + "IMAGE_MIME_TYPES", + "MAX_IMAGES_PER_RESULT", + "MAX_IMAGE_BYTES", + "TOOL_CONTENT_MARKER", + "image_attachment", + "image_tokens", + "message_image_meta", + "message_image_tokens", + "parse_tool_content", + "redacted_for_trace", + "redacted_tool_result_content", + "sniff_image_size", + "tool_content", +] + +# Present and truthy on a dict that means "structured tool output". A marker key +# rather than duck-typing on ``{"text", "images"}``: tools are free to return +# ordinary dicts, and one that happens to carry those two keys must keep +# stringifying the way it always did. +TOOL_CONTENT_MARKER = "__tool_content__" + +# What the OpenAI content-part schema accepts as an inline image. Enforced here +# because an unsupported type is a 400 from the server, and a 400 fails the whole +# turn rather than just the attachment. +IMAGE_MIME_TYPES = frozenset( + {"image/png", "image/jpeg", "image/webp", "image/gif"} +) + +# Per-image and per-result ceilings. Both are about the context window, not the +# HTTP body: a 4K screenshot measured 8.5K prompt tokens against apodex-1.1-mini, +# so a handful of them at full resolution is the whole budget. Producers are +# expected to downscale; these are the backstop for producers that did not. +MAX_IMAGE_BYTES = 6 * 1024 * 1024 +MAX_IMAGES_PER_RESULT = 8 + +# Pixels per token, fitted against apodex-1.1-mini prompt_tokens over the same +# image at 640x360 / 1280x720 / 1920x1080 / 3840x2160 (227 / 908 / 2043 / 8170 +# image tokens, r² > 0.999 on a straight px term). Providers differ, and a +# provider that tiles differently will be off by a constant factor — that is +# acceptable for a budget estimate, and catastrophically better than the zero +# this used to contribute, which let a history full of images look empty to the +# context guard. +_PIXELS_PER_TOKEN = 1024 +# Charged when dimensions cannot be read. Deliberately not "0" and not the 4K +# figure: an unknown image is assumed to be roughly a 1080p screenshot. +_UNKNOWN_IMAGE_TOKENS = 2400 + + +def tool_content(text: str, *, images: list[dict[str, Any]] | None = None) -> dict[str, Any]: + """Build the structured return value for a tool with attachments. + + ``text`` is what the tool would have returned on its own, and stays the + tool's result string everywhere in the loop — observers, spill/recovery, + repeat detection and the trajectory all keep seeing a plain string. The + images ride alongside it. + """ + return { + TOOL_CONTENT_MARKER: 1, + "text": text, + "images": list(images or []), + } + + +def image_attachment( + data: bytes | str, + mime_type: str, + *, + label: str = "", + width: int = 0, + height: int = 0, +) -> dict[str, Any]: + """One image attachment. ``data`` may be raw bytes or an existing base64 str. + + ``label`` is what the model is told the image IS (usually the path it asked + for). It is carried separately from the text so a placeholder can name the + image after the bytes are gone. + """ + b64 = base64.b64encode(data).decode("ascii") if isinstance(data, bytes) else data + attachment: dict[str, Any] = {"mime_type": mime_type, "data": b64} + if label: + attachment["label"] = label + if width > 0 and height > 0: + attachment["width"] = int(width) + attachment["height"] = int(height) + return attachment + + +def parse_tool_content(raw: Any) -> tuple[str, list[dict[str, Any]]] | None: + """Split a tool return value into ``(text, images)``, or ``None``. + + ``None`` means "this is not structured output" and the caller should keep + its existing ``str(raw)`` behaviour. A malformed attachment inside a + well-formed envelope is dropped and reported in the text, never silently: + see the module docstring on why a vanishing image is worse than no image. + """ + if not isinstance(raw, dict): + return None + envelope = cast("dict[str, Any]", raw) + if not envelope.get(TOOL_CONTENT_MARKER): + return None + text = envelope.get("text") + text = text if isinstance(text, str) else ("" if text is None else str(text)) + + raw_images = envelope.get("images") + candidates: list[Any] = [] + images: list[dict[str, Any]] = [] + rejected: list[str] = [] + if isinstance(raw_images, list): + candidates = list(cast("list[Any]", raw_images)) + elif raw_images is not None: + rejected.append("images field (must be a list)") + for index, candidate in enumerate(candidates): + if len(images) >= MAX_IMAGES_PER_RESULT: + rejected.append( + f"image {index + 1} and the ones after it (more than " + f"{MAX_IMAGES_PER_RESULT} images in one result)" + ) + break + accepted, why = _validated_image(candidate) + if accepted is None: + rejected.append(f"image {index + 1} ({why})") + continue + images.append(accepted) + + if rejected: + note = "not attached: " + "; ".join(rejected) + logger.warning("tool content dropped %s", note) + text = f"{text}\n\n[{note}]" if text else f"[{note}]" + return text, images + + +def _validated_image(candidate: Any) -> tuple[dict[str, Any] | None, str]: + if not isinstance(candidate, dict): + return None, "not an object" + image = cast("dict[str, Any]", candidate) + mime = str(image.get("mime_type") or "") + if mime not in IMAGE_MIME_TYPES: + return None, f"unsupported type {mime or 'unset'!r}" + data = image.get("data") + if not isinstance(data, str) or not data: + return None, "no base64 payload" + # Validated here rather than at the provider: a bad payload fails the whole + # completion (HTTP 500 on the calibrated Apodex endpoint), including the + # turn's other tool results. + try: + decoded = base64.b64decode(data, validate=True) + except (binascii.Error, ValueError): + return None, "payload is not valid base64" + decoded_size = len(decoded) + if decoded_size == 0: + return None, "payload is empty" + if decoded_size > MAX_IMAGE_BYTES: + return None, ( + f"{decoded_size // 1024} KB exceeds the " + f"{MAX_IMAGE_BYTES // (1024 * 1024)} MB per-image cap" + ) + + actual_mime = _sniff_image_mime(decoded) + if actual_mime is None: + return None, "payload is not a recognizable supported image" + if actual_mime != mime: + return None, f"payload is {actual_mime}, not declared {mime}" + if not _has_complete_image_container(decoded, actual_mime): + return None, "payload is truncated or structurally invalid" + width, height = sniff_image_size(decoded) + if width <= 0 or height <= 0: + return None, "image dimensions could not be read" + + accepted: dict[str, Any] = { + "mime_type": mime, + "data": data, + # Always derive accounting dimensions from the payload. A producer's + # stale or incorrect declaration must not turn a 4K image into a + # one-token image and bypass the context-overflow guard. + "width": width, + "height": height, + } + label = image.get("label") + if isinstance(label, str) and label: + accepted["label"] = label + return accepted, "" + + +def _declared_size(image: dict[str, Any]) -> tuple[int, int]: + try: + return int(image.get("width") or 0), int(image.get("height") or 0) + except (TypeError, ValueError): + return 0, 0 + + +def sniff_image_size(data: bytes) -> tuple[int, int]: + """Pixel dimensions from an image header, or ``(0, 0)``. + + Header parsing rather than a decode: the only consumer is the token + estimate, this runs on every attachment, and AgentCore has no imaging + dependency to decode with. + """ + try: + if data[:8] == b"\x89PNG\r\n\x1a\n" and data[12:16] == b"IHDR": + return ( + int.from_bytes(data[16:20], "big"), + int.from_bytes(data[20:24], "big"), + ) + if data[:3] == b"GIF": + return ( + int.from_bytes(data[6:8], "little"), + int.from_bytes(data[8:10], "little"), + ) + if data[:4] == b"RIFF" and data[8:12] == b"WEBP": + return _webp_size(data) + if data[:2] == b"\xff\xd8": + return _jpeg_size(data) + except (IndexError, ValueError): + return 0, 0 + return 0, 0 + + +def _sniff_image_mime(data: bytes) -> str | None: + """Recognize one of the supported image encodings from its header.""" + if len(data) >= 24 and data[:8] == b"\x89PNG\r\n\x1a\n" and data[12:16] == b"IHDR": + return "image/png" + if len(data) >= 10 and data[:6] in (b"GIF87a", b"GIF89a"): + return "image/gif" + if len(data) >= 25 and data[:4] == b"RIFF" and data[8:12] == b"WEBP": + return "image/webp" + if len(data) >= 11 and data[:2] == b"\xff\xd8": + return "image/jpeg" + return None + + +def _has_complete_image_container(data: bytes, mime: str) -> bool: + """Reject common truncation/corruption without adding an image decoder.""" + if mime == "image/png": + return _has_complete_png_container(data) + if mime == "image/jpeg": + return data.endswith(b"\xff\xd9") + if mime == "image/gif": + return data.endswith(b"\x3b") + if mime == "image/webp": + return len(data) >= 12 and int.from_bytes(data[4:8], "little") + 8 == len(data) + return False + + +def _has_complete_png_container(data: bytes) -> bool: + """Check PNG chunk bounds, CRCs, ordering anchors, and the final IEND.""" + offset = 8 + saw_header = False + saw_data = False + while offset + 12 <= len(data): + body_size = int.from_bytes(data[offset:offset + 4], "big") + chunk_end = offset + 12 + body_size + if chunk_end > len(data): + return False + chunk_type = data[offset + 4:offset + 8] + body = data[offset + 8:offset + 8 + body_size] + expected_crc = int.from_bytes(data[offset + 8 + body_size:chunk_end], "big") + if zlib.crc32(chunk_type + body) & 0xFFFFFFFF != expected_crc: + return False + if not saw_header: + if chunk_type != b"IHDR" or body_size != 13: + return False + saw_header = True + elif chunk_type == b"IDAT": + saw_data = True + elif chunk_type == b"IEND": + return body_size == 0 and saw_data and chunk_end == len(data) + offset = chunk_end + return False + + +def _webp_size(data: bytes) -> tuple[int, int]: + chunk = data[12:16] + if chunk == b"VP8X" and len(data) >= 30: + return ( + int.from_bytes(data[24:27], "little") + 1, + int.from_bytes(data[27:30], "little") + 1, + ) + if chunk == b"VP8 " and len(data) >= 30: + return ( + int.from_bytes(data[26:28], "little") & 0x3FFF, + int.from_bytes(data[28:30], "little") & 0x3FFF, + ) + if chunk == b"VP8L" and len(data) >= 25 and data[20] == 0x2F: + bits = int.from_bytes(data[21:25], "little") + return (bits & 0x3FFF) + 1, ((bits >> 14) & 0x3FFF) + 1 + return 0, 0 + + +def _jpeg_size(data: bytes) -> tuple[int, int]: + # Walk the marker chain to the frame header. SOF0/1/2/3/5/6/7/9-11/13-15 all + # carry the dimensions at the same offset; DHT/DAC/RST/SOS do not and are + # skipped by length like any other segment. + offset = 2 + end = len(data) + while offset + 9 < end: + if data[offset] != 0xFF: + offset += 1 + continue + marker = data[offset + 1] + if marker in (0xD8, 0x01) or 0xD0 <= marker <= 0xD7: + offset += 2 + continue + if marker == 0xDA: # start of scan — no frame header past here + return 0, 0 + segment_length = int.from_bytes(data[offset + 2:offset + 4], "big") + if segment_length < 2: + return 0, 0 + if 0xC0 <= marker <= 0xCF and marker not in (0xC4, 0xC8, 0xCC): + return ( + int.from_bytes(data[offset + 7:offset + 9], "big"), + int.from_bytes(data[offset + 5:offset + 7], "big"), + ) + offset += 2 + segment_length + return 0, 0 + + +def image_tokens(image: dict[str, Any]) -> int: + """Estimated prompt tokens an attachment costs. Never zero.""" + width, height = _declared_size(image) + if width <= 0 or height <= 0: + return _UNKNOWN_IMAGE_TOKENS + return max(1, (width * height) // _PIXELS_PER_TOKEN) + + +def message_image_meta(message: Any) -> list[dict[str, Any]]: + """The ``image_meta`` list on a message, positionally aligned with images.""" + if not isinstance(message, dict): + return [] + raw = cast("dict[str, Any]", message).get("image_meta") + if not isinstance(raw, list): + return [] + return [ + cast("dict[str, Any]", entry) + for entry in cast("list[Any]", raw) + if isinstance(entry, dict) + ] + + +def message_image_tokens(message: Any) -> int: + """Estimated prompt tokens the inline images on *message* cost. + + Counted from the ``image_url`` blocks actually present, priced from + ``image_meta``. Driven by the content rather than by the metadata because a + compactor is free to rewrite ``content`` to a plain string -- which drops + the images -- without knowing that a bookkeeping key describes them. Trusting + the metadata there would keep charging for pixels no longer in the request. + + The prices are read off ``image_meta`` rather than measured: the bytes are a + data URI by this point, and base64-decoding the whole history on every + estimate would make the context guard cost more than the turn it guards. An + image with no usable price still charges something -- an unknown image is + not a free one. + + Lives here rather than beside the rest of the message-image handling in + ``runtime.loop.image_attach`` for one reason: ``tokens`` needs it, and this + module imports nothing from ``agent_core``, so there is no import cycle to + get wrong later. + """ + if not isinstance(message, dict): + return 0 + content = cast("dict[str, Any]", message).get("content") + if not isinstance(content, list): + return 0 + blocks = sum( + 1 + for block in cast("list[Any]", content) + if isinstance(block, dict) and cast("dict[str, Any]", block).get("type") == "image_url" + ) + if not blocks: + return 0 + meta = message_image_meta(message) + total = 0 + for index in range(blocks): + entry = meta[index] if index < len(meta) else {} + try: + priced = max(0, int(entry.get("tokens") or 0)) + except (TypeError, ValueError): + priced = 0 + total += priced or _UNKNOWN_IMAGE_TOKENS + return total + + +def redacted_for_trace(message: Any) -> Any: + """A copy of *message* with inline image payloads replaced by a marker. + + For anything that writes a message somewhere other than the provider: a + trajectory file, a log line, an event record. The base64 of a single 1080p + screenshot is ~137 KB, and a trace that copies messages verbatim writes that + again for every turn the image survives in history -- a few screenshots turn + a readable trajectory into tens of megabytes of unreadable one. + + The block KEEPS its ``image_url`` type and gains a stated size, so a reader + can still see that an image was in the request and how big it was. That + matters for the same reason the loop narrates evictions: a trace that shows + no image where the model saw one misrepresents what the model was answering. + + Returns the message unchanged (not a copy) when it carries no inline image, + which is nearly every message. + """ + if not isinstance(message, dict): + return message + typed = cast("dict[str, Any]", message) + content = typed.get("content") + if not isinstance(content, list): + return typed + blocks = cast("list[Any]", content) + if not any( + isinstance(block, dict) + and cast("dict[str, Any]", block).get("type") == "image_url" + for block in blocks + ): + return typed + + redacted: list[Any] = [] + for block in blocks: + entry = cast("dict[str, Any]", block) if isinstance(block, dict) else None + if entry is None or entry.get("type") != "image_url": + redacted.append(block) + continue + url = entry.get("image_url") + raw = str(cast("dict[str, Any]", url).get("url") or "") if isinstance(url, dict) else "" + redacted.append({ + "type": "image_url", + "image_url": {"url": _elided_data_uri(raw)}, + }) + return {**typed, "content": redacted} + + +def redacted_tool_result_content( + text: str, + images: list[dict[str, Any]], +) -> str | list[dict[str, Any]]: + """Render returned images for a trace without retaining their Base64. + + This describes what the tool returned. Delivery to the provider remains a + separate loop decision, so trace consumers must not interpret these blocks + as proof that a text-only profile received the pixels. + """ + if not images: + return text + blocks: list[dict[str, Any]] = [] + if text: + blocks.append({"type": "text", "text": text}) + for image in images: + mime = str(image.get("mime_type") or "image/unknown") + payload = str(image.get("data") or "") + prefix = f"data:{mime}" + blocks.append({ + "type": "image_url", + "image_url": {"url": _elided_data_uri(f"{prefix};base64,{payload}")}, + }) + return blocks + + +def _elided_data_uri(url: str) -> str: + """``data:image/png;base64,<...>`` → a same-shaped string stating the size.""" + if not url.startswith("data:") or ";base64," not in url: + return url + prefix, payload = url.split(";base64,", 1) + approx_kb = max(1, (len(payload) * 3 // 4) // 1024) + return f"{prefix};base64,[{approx_kb} KB of image data elided from trace]" diff --git a/docs/agent-loop-boundary.md b/docs/agent-loop-boundary.md index dbec126..b49c6ef 100644 --- a/docs/agent-loop-boundary.md +++ b/docs/agent-loop-boundary.md @@ -98,3 +98,55 @@ product words its note. Two consequences worth stating: - AgentCore still words nothing about repeats. A product moving off its own loop copy must port its note into `render_tool_result`; nothing here will fail if it forgets, which is why this paragraph exists. + +## Image attachments: the core decides visibility, the product supplies pixels + +A tool returns images by returning the `agent_core.tool_content.tool_content` +envelope — a plain JSON dict — instead of a string. Sandbox-native tools run in +a child process and their return value crosses `json.dumps(default=str)` on the +way back, so the envelope is a dict by requirement, not by taste; a dataclass +arrives as its repr. + +The split of responsibility: + +- **The product** decides what is worth attaching, and downscales. Pixel count + is the cost driver (roughly 1 token per 1024 px against the model this was + calibrated on — a 1080p screenshot ~2.4K tokens, a 4K one ~8.5K), and + AgentCore has no imaging dependency with which to resize. The caps in + `tool_content` (6 MB and 8 images per result) are a backstop against a + runaway producer, not a resize policy. +- **AgentCore** decides whether an attachment is shown, from + `ModelProfile.supports_images` and `ModelProfile.protocol`, and how many stay + in history, from `HistoryPolicy.max_images_in_history`. Products do not + pre-filter on capability: `attach_images` is called unconditionally and writes + the withheld note itself. Before that decision, core verifies that the decoded + bytes have a supported image header matching the declared MIME type and a + complete lightweight container shape (including PNG chunk CRCs). It also + derives dimensions from those bytes rather than trusting producer metadata; + the dimensions drive context-budget accounting and cannot be allowed to + understate the actual image. + +`ToolResult.images` is populated by core (`tool_exec` parses the envelope) and +consumed by core (`agent_loop` builds the message). It is not a host-supplied +field, so unlike `result_id` and the repeated-invocation metadata above it needs +no boundary exemption. + +### Why every disappearance is narrated + +Both the capability path and the eviction path replace the image with a sentence +saying an image was there and is not visible. The calibration run behind this +feature (MiroHarness +`internal-docs/designs/2026-09-08-native-image-in-tool-result-calibration.md`) +removed the image block from an otherwise working request and changed nothing +else. The model had been told to answer `NO_IMAGE` if it could not see +an image; it instead produced a four-digit code and three shapes, every one of +them invented. A tool result whose text reads as though an image were delivered +will be answered as though one were, so the text has to say otherwise. A product +that adds its own image-bearing path — or its own compactor that touches these +messages — inherits that obligation. + +For the same reason `messages.text_of` renders an `image_url` block as a +placeholder rather than as nothing: every flattening caller (Anthropic message +translation, compaction summaries, trajectory lines) is dropping the image at +that call, and a transcript that never mentions the picture is the same trap in +a different place. diff --git a/pyproject.toml b/pyproject.toml index e61789e..d67ba85 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "apodex-agent-core" -version = "0.9.1" +version = "0.10.0" description = "Shared, product-neutral runtime primitives for Apodex agents" readme = "README.md" license = "Apache-2.0" diff --git a/tests/test_agent_loop_image_results.py b/tests/test_agent_loop_image_results.py new file mode 100644 index 0000000..e5cd195 --- /dev/null +++ b/tests/test_agent_loop_image_results.py @@ -0,0 +1,253 @@ +"""End-to-end: a tool that attaches an image, driven through ``run_agent_loop``. + +The unit tests in ``test_tool_content_images`` cover the pieces. These cover the +wiring -- that a tool's envelope actually survives execution, rendering, the +recovery-handle step and the history append, and that the model profile is what +decides whether the pixels go on the wire. +""" + +from __future__ import annotations + +import struct +import zlib +from typing import Any + +import pytest + +from agent_core.llm import LLMResponse +from agent_core.loop_types import LoopConfig, LoopPolicy +from agent_core.messages import for_wire +from agent_core.runtime.loop.agent_loop import run_agent_loop +from agent_core.runtime.loop.model_profile import HistoryPolicy, ModelProfile +from agent_core.tool_content import image_attachment, tool_content + + +def _png(width: int, height: int) -> bytes: + def chunk(tag: bytes, body: bytes) -> bytes: + return ( + struct.pack(">I", len(body)) + + tag + + body + + struct.pack(">I", zlib.crc32(tag + body) & 0xFFFFFFFF) + ) + + ihdr = struct.pack(">IIBBBBB", width, height, 8, 2, 0, 0, 0) + raw = b"".join(b"\x00" + b"\x00\x00\x00" * width for _ in range(height)) + return ( + b"\x89PNG\r\n\x1a\n" + + chunk(b"IHDR", ihdr) + + chunk(b"IDAT", zlib.compress(raw)) + + chunk(b"IEND", b"") + ) + + +class SequenceLLM: + def __init__(self, responses: list[LLMResponse]) -> None: + self.responses = responses + self.calls: list[list[dict[str, Any]]] = [] + + async def chat(self, messages, **_kwargs) -> LLMResponse: + self.calls.append([dict(message) for message in messages]) + return self.responses.pop(0) + + def stream(self, messages, **_kwargs): + raise AssertionError("streaming was not requested") + + +class ShotTool: + """A view_image-shaped tool: a caption plus the bytes themselves.""" + + name = "view_image" + + def __init__(self, count: int = 1) -> None: + self.count = count + + async def ainvoke(self, args: dict[str, Any]) -> Any: + path = str(args.get("path") or "/tmp/shot.png") + return tool_content( + f"Image {path} (64x32) is attached below.", + images=[ + image_attachment(_png(64, 32), "image/png", label=path) + for _ in range(self.count) + ], + ) + + def to_openai_schema(self) -> dict[str, Any]: + return { + "type": "function", + "function": { + "name": self.name, + "description": "read an image", + "parameters": {"type": "object"}, + }, + } + + +class BrokenShotTool(ShotTool): + async def ainvoke(self, args: dict[str, Any]) -> Any: + path = str(args.get("path") or "/tmp/broken.png") + return tool_content( + f"Image {path} is attached below.", + images=[image_attachment(b"not an image", "image/png", label=path)], + ) + + +def _call(index: int, path: str) -> dict[str, Any]: + return { + "id": f"tc{index}", + "type": "function", + "function": {"name": "view_image", "arguments": f'{{"path":"{path}"}}'}, + } + + +def _config() -> LoopConfig: + return LoopConfig( + max_turns=6, loop_policy=LoopPolicy(no_tool_behavior="stop"), max_llm_retries=1, + ) + + +async def _run( + profile: ModelProfile, + *, + paths: list[str], + policy: HistoryPolicy | None = None, + images_per_call: int = 1, +): + responses = [ + LLMResponse(content="", tool_calls=[_call(index, path)]) + for index, path in enumerate(paths) + ] + responses.append(LLMResponse(content="done")) + llm = SequenceLLM(responses) + result = await run_agent_loop( + system_prompt="system", + user_message="look", + llm=llm, + tools=[ShotTool(images_per_call)], + config=_config(), + model_profile=profile, + history_policy=policy or HistoryPolicy(), + ) + return llm, result + + +def _tool_messages(messages: list[dict[str, Any]]) -> list[dict[str, Any]]: + return [m for m in messages if m.get("role") == "tool"] + + +@pytest.mark.asyncio +async def test_image_reaches_history_as_a_content_part() -> None: + _, result = await _run( + ModelProfile(model_id="apodex-1.1-mini", provider="apodex", supports_images=True), + paths=["/tmp/a.png"], + ) + (message,) = _tool_messages(result.messages) + content = message["content"] + assert isinstance(content, list) + assert content[0]["type"] == "text" + assert "/tmp/a.png" in content[0]["text"] + assert content[1]["image_url"]["url"].startswith("data:image/png;base64,") + + +@pytest.mark.asyncio +async def test_the_image_is_still_there_on_the_next_request() -> None: + """A tool result is only useful if it survives into the following turn.""" + llm, _ = await _run( + ModelProfile(model_id="apodex-1.1-mini", provider="apodex", supports_images=True), + paths=["/tmp/a.png"], + ) + sent = llm.calls[-1] + images = [ + block + for message in sent + if isinstance(message.get("content"), list) + for block in message["content"] + if isinstance(block, dict) and block.get("type") == "image_url" + ] + assert len(images) == 1 + # And nothing in-process rode along with it. + assert all("image_meta" not in message for message in for_wire(sent)) + + +@pytest.mark.asyncio +async def test_a_text_only_model_gets_the_warning_instead_of_the_bytes() -> None: + _, result = await _run( + ModelProfile(model_id="text-only", provider="openai"), paths=["/tmp/a.png"], + ) + (message,) = _tool_messages(result.messages) + content = message["content"] + assert isinstance(content, str) + assert "base64" not in content + assert "have NOT" in content + + +@pytest.mark.asyncio +async def test_invalid_image_is_reported_and_never_reaches_the_llm() -> None: + llm = SequenceLLM([ + LLMResponse(content="", tool_calls=[_call(0, "/tmp/broken.png")]), + LLMResponse(content="done"), + ]) + result = await run_agent_loop( + system_prompt="system", + user_message="look", + llm=llm, + tools=[BrokenShotTool()], + config=_config(), + model_profile=ModelProfile( + model_id="apodex-1.1-mini", + provider="apodex", + supports_images=True, + ), + ) + + (history_message,) = _tool_messages(result.messages) + assert isinstance(history_message["content"], str) + assert "not a recognizable supported image" in history_message["content"] + assert "base64" not in history_message["content"] + + (sent_message,) = _tool_messages(llm.calls[-1]) + assert isinstance(sent_message["content"], str) + assert "not a recognizable supported image" in sent_message["content"] + + +@pytest.mark.asyncio +async def test_history_is_bounded_by_max_images_in_history() -> None: + profile = ModelProfile( + model_id="apodex-1.1-mini", provider="apodex", supports_images=True, + ) + _, result = await _run( + profile, + paths=[f"/tmp/{index}.png" for index in range(4)], + policy=HistoryPolicy(max_images_in_history=2), + ) + messages = _tool_messages(result.messages) + assert len(messages) == 4 + live = [ + message for message in messages if isinstance(message.get("content"), list) + ] + assert len(live) == 2 + # The two that went are named, not silently missing. + gone = [m for m in messages if isinstance(m.get("content"), str)] + assert all("no longer see it" in m["content"] for m in gone) + assert "/tmp/0.png" in gone[0]["content"] + + +@pytest.mark.asyncio +async def test_one_result_with_several_images_is_not_self_evicting() -> None: + """A multi-page read must not push out its own earlier pages first.""" + profile = ModelProfile( + model_id="apodex-1.1-mini", provider="apodex", supports_images=True, + ) + _, result = await _run( + profile, + paths=["/tmp/doc.png"], + policy=HistoryPolicy(max_images_in_history=5), + images_per_call=3, + ) + (message,) = _tool_messages(result.messages) + urls = [ + block + for block in message["content"] + if isinstance(block, dict) and block.get("type") == "image_url" + ] + assert len(urls) == 3 diff --git a/tests/test_skills_loader_reload.py b/tests/test_skills_loader_reload.py index 38b799d..14f0db6 100644 --- a/tests/test_skills_loader_reload.py +++ b/tests/test_skills_loader_reload.py @@ -152,3 +152,51 @@ def test_toggle_on_a_fresh_loader_is_not_a_silent_no_op(skill_dir, config_file): def test_toggle_still_reports_a_genuinely_missing_skill(skill_dir, config_file): loader = _loader(skill_dir, config_file) assert loader.toggle_skill("no-such-skill", False) is False + + +def _pin_mtime(path, when: float = 1_700_000_000.0) -> None: + """Force an exact mtime, so a change is invisible to a timestamp check.""" + os.utime(path, (when, when)) + + +def test_change_within_one_clock_tick_is_still_seen(skill_dir, config_file): + """Two edits sharing an mtime must not look like no edit at all. + + This is the real shape of the flake that used to surface here: the + filesystem clock advances in 1 ms steps, consecutive writes collide on a + single value about 92% of the time, and the old ``st_mtime > loaded_mtime`` + check reported "unchanged" for every one of those. It only passed as often + as it did because the work between the two writes usually spilled into the + next millisecond. Pinning both timestamps to the same value reproduces it + every run instead of a quarter of them. + """ + _write(config_file, {"debug": False}) + _pin_mtime(config_file) + loader = _loader(skill_dir, config_file) + assert {s.skill_id for s in loader.get_enabled_skills()} == {"code-review"} + + _write(config_file, {"code-review": False}) + _pin_mtime(config_file) + assert {s.skill_id for s in loader.get_enabled_skills()} == {"debug"} + + +def test_a_timestamp_moving_backward_is_still_a_change(skill_dir, config_file): + """Restoring a backup, a git checkout, an rsync --times of an older tree.""" + _write(config_file, {"debug": False}) + _pin_mtime(config_file, 1_700_000_000.0) + loader = _loader(skill_dir, config_file) + assert {s.skill_id for s in loader.get_enabled_skills()} == {"code-review"} + + _write(config_file, {"code-review": False}) + _pin_mtime(config_file, 1_600_000_000.0) + assert {s.skill_id for s in loader.get_enabled_skills()} == {"debug"} + + +def test_an_identical_rewrite_is_not_a_change(skill_dir, config_file): + """Nothing to reload, so nothing should be reloaded.""" + _write(config_file, {"debug": False}) + loader = _loader(skill_dir, config_file) + assert {s.skill_id for s in loader.get_enabled_skills()} == {"code-review"} + + _write(config_file, {"debug": False}) + assert loader._extensions_config.has_changed() is False diff --git a/tests/test_tool_content_images.py b/tests/test_tool_content_images.py new file mode 100644 index 0000000..1e7cc35 --- /dev/null +++ b/tests/test_tool_content_images.py @@ -0,0 +1,579 @@ +"""Images returned by a tool: envelope parsing, attachment, eviction, cost. + +The behavioural anchor for the whole feature is +``test_withheld_image_says_so_in_the_text`` and its eviction twin. Everything +else here is plumbing; those two encode why the plumbing is shaped this way. +See ``agent_core/runtime/loop/image_attach.py`` for the calibration run. +""" + +from __future__ import annotations + +import base64 +import struct +import zlib +from typing import Any + +import pytest + +from agent_core.messages import for_wire, text_of, tool_msg +from agent_core.runtime.loop.image_attach import ( + attach_images, + evict_old_images, + image_blocks_in, +) +from agent_core.runtime.loop.model_profile import ModelProfile +from agent_core.tokens import estimate_message_tokens +from agent_core.tool_content import ( + MAX_IMAGE_BYTES, + MAX_IMAGES_PER_RESULT, + image_attachment, + image_tokens, + parse_tool_content, + sniff_image_size, + tool_content, +) + + +def _png(width: int, height: int) -> bytes: + """A real, minimal PNG of the requested size (no imaging dependency).""" + def chunk(tag: bytes, body: bytes) -> bytes: + return ( + struct.pack(">I", len(body)) + + tag + + body + + struct.pack(">I", zlib.crc32(tag + body) & 0xFFFFFFFF) + ) + + ihdr = struct.pack(">IIBBBBB", width, height, 8, 2, 0, 0, 0) + raw = b"".join(b"\x00" + b"\x00\x00\x00" * width for _ in range(height)) + return ( + b"\x89PNG\r\n\x1a\n" + + chunk(b"IHDR", ihdr) + + chunk(b"IDAT", zlib.compress(raw)) + + chunk(b"IEND", b"") + ) + + +_GIF_2X3 = base64.b64decode( + "R0lGODlhAgADAPAAAP8AAAAAACH5BAAAAAAALAAAAAACAAMAAAIChF8AOw==" +) +_JPEG_2X3 = base64.b64decode( + "/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgG" + "BgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/2wBDAQMD" + "AwQDBAgEBAgQCwkLEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQ" + "EBAQEBAQEBAQEBAQEBD/wAARCAADAAIDAREAAhEBAxEB/8QAFAABAAAAAAAAAAAAAA" + "AAAAAACP/EABQQAQAAAAAAAAAAAAAAAAAAAAD/xAAVAQEBAAAAAAAAAAAAAAAAAAAHC" + "f/EABQRAQAAAAAAAAAAAAAAAAAAAAD/2gAMAwEAAhEDEQA/ADoDFU3/2Q==" +) +_WEBP_2X3 = base64.b64decode( + "UklGRjwAAABXRUJQVlA4IDAAAADQAQCdASoCAAMAAgA0JaACdLoB+AADsAD+8MQL" + "/yC5YXXI1/8gP+QH/ID/+PIAAAA=" +) + + +def _real_images() -> list[tuple[str, bytes]]: + return [ + ("image/png", _png(2, 3)), + ("image/gif", _GIF_2X3), + ("image/jpeg", _JPEG_2X3), + ("image/webp", _WEBP_2X3), + ] + + +def _vision_profile() -> ModelProfile: + return ModelProfile( + model_id="apodex-1.1-mini", provider="apodex", supports_images=True, + ) + + +def _blind_profile() -> ModelProfile: + return ModelProfile(model_id="text-only", provider="openai") + + +def _attached(text: str, images: list[dict[str, Any]], profile: ModelProfile): + message = tool_msg(text, "call_1") + attach_images(message, images, profile=profile) + return message + + +# ── envelope ────────────────────────────────────────────────────────────── + + +def test_plain_returns_are_not_envelopes() -> None: + assert parse_tool_content("just text") is None + assert parse_tool_content(None) is None + # A tool returning an ordinary dict that happens to have these keys keeps + # its existing stringify behaviour -- the marker is what opts in. + assert parse_tool_content({"text": "hi", "images": []}) is None + + +def test_envelope_splits_text_from_images() -> None: + envelope = tool_content( + "screenshot:", + images=[image_attachment(_png(64, 32), "image/png", label="/tmp/a.png")], + ) + parsed = parse_tool_content(envelope) + assert parsed is not None + text, images = parsed + assert text == "screenshot:" + assert len(images) == 1 + assert images[0]["label"] == "/tmp/a.png" + assert (images[0]["width"], images[0]["height"]) == (64, 32) + + +@pytest.mark.parametrize( + ("image", "reason"), + [ + ({"mime_type": "image/tiff", "data": "aGk="}, "unsupported type"), + ({"mime_type": "image/png", "data": "not base64!!"}, "not valid base64"), + ({"mime_type": "image/png", "data": ""}, "no base64 payload"), + ({"mime_type": "image/png"}, "no base64 payload"), + ("not-a-dict", "not an object"), + ], +) +def test_a_rejected_image_is_reported_not_dropped(image: Any, reason: str) -> None: + parsed = parse_tool_content(tool_content("body", images=[image])) + assert parsed is not None + text, images = parsed + assert images == [] + assert "not attached" in text + assert reason in text + + +def test_base64_valid_non_image_is_rejected_before_the_provider() -> None: + image = image_attachment(b"not an image", "image/png") + parsed = parse_tool_content(tool_content("image follows", images=[image])) + assert parsed is not None + text, images = parsed + assert images == [] + assert "not a recognizable supported image" in text + + +def test_declared_mime_must_match_the_payload() -> None: + image = image_attachment(_png(8, 8), "image/jpeg") + parsed = parse_tool_content(tool_content("image follows", images=[image])) + assert parsed is not None + text, images = parsed + assert images == [] + assert "payload is image/png, not declared image/jpeg" in text + + +@pytest.mark.parametrize(("mime", "payload"), _real_images()) +def test_each_supported_real_image_format_is_accepted( + mime: str, + payload: bytes, +) -> None: + parsed = parse_tool_content( + tool_content("image follows", images=[image_attachment(payload, mime)]) + ) + assert parsed is not None + text, images = parsed + assert text == "image follows" + assert len(images) == 1 + assert images[0]["mime_type"] == mime + assert (images[0]["width"], images[0]["height"]) == (2, 3) + + +@pytest.mark.parametrize(("mime", "payload"), _real_images()) +def test_truncated_real_images_are_rejected( + mime: str, + payload: bytes, +) -> None: + parsed = parse_tool_content( + tool_content("image follows", images=[image_attachment(payload[:-1], mime)]) + ) + assert parsed is not None + text, images = parsed + assert images == [] + assert "truncated or structurally invalid" in text + + +def test_corrupt_png_crc_is_rejected() -> None: + payload = bytearray(_png(2, 3)) + payload[-1] ^= 0x01 + parsed = parse_tool_content( + tool_content( + "image follows", + images=[image_attachment(bytes(payload), "image/png")], + ) + ) + assert parsed is not None + text, images = parsed + assert images == [] + assert "structurally invalid" in text + + +def test_malformed_images_field_is_reported() -> None: + parsed = parse_tool_content({ + "__tool_content__": 1, + "text": "image follows", + "images": {"mime_type": "image/png", "data": "aGk="}, + }) + assert parsed is not None + text, images = parsed + assert images == [] + assert "images field (must be a list)" in text + + +def test_mixed_valid_and_invalid_images_keep_the_valid_one_and_report_the_other() -> None: + parsed = parse_tool_content( + tool_content( + "two images", + images=[ + image_attachment(_png(2, 3), "image/png", label="valid"), + image_attachment(b"not an image", "image/png", label="invalid"), + ], + ) + ) + assert parsed is not None + text, images = parsed + assert [image["label"] for image in images] == ["valid"] + assert "image 2" in text + assert "not a recognizable supported image" in text + + +def test_oversized_image_is_rejected_with_its_size() -> None: + payload = base64.b64encode(b"\x89PNG\r\n\x1a\n" + b"\x00" * MAX_IMAGE_BYTES) + parsed = parse_tool_content( + tool_content("body", images=[{"mime_type": "image/png", "data": payload.decode()}]) + ) + assert parsed is not None + text, images = parsed + assert images == [] + assert "per-image cap" in text + + +def test_image_count_is_capped_and_the_overflow_is_named() -> None: + one = image_attachment(_png(8, 8), "image/png") + parsed = parse_tool_content( + tool_content("body", images=[dict(one) for _ in range(MAX_IMAGES_PER_RESULT + 3)]) + ) + assert parsed is not None + text, images = parsed + assert len(images) == MAX_IMAGES_PER_RESULT + assert "more than" in text + + +# ── header sniffing and cost ────────────────────────────────────────────── + + +def test_png_dimensions_come_off_the_header() -> None: + assert sniff_image_size(_png(1920, 4)) == (1920, 4) + + +def test_unreadable_header_still_costs_something() -> None: + assert sniff_image_size(b"not an image") == (0, 0) + # An image whose size cannot be read must not be free; a zero here is how a + # history of images measures as empty to the context guard. + assert image_tokens({}) > 1000 + + +def test_token_estimate_tracks_pixels() -> None: + small = image_tokens({"width": 640, "height": 360}) + large = image_tokens({"width": 1920, "height": 1080}) + assert small == pytest.approx(225, abs=30) + assert large == pytest.approx(2043, abs=200) + + +def test_payload_dimensions_override_an_incorrect_declaration() -> None: + envelope = tool_content( + "screenshot:", + images=[ + image_attachment( + _png(1920, 1080), + "image/png", + width=1, + height=1, + ) + ], + ) + parsed = parse_tool_content(envelope) + assert parsed is not None + _, images = parsed + assert (images[0]["width"], images[0]["height"]) == (1920, 1080) + assert image_tokens(images[0]) > 1500 + + +def test_estimate_counts_attached_images() -> None: + caption = tool_msg("screenshot:", "call_1") + text_only = estimate_message_tokens(caption) + withimage = _attached( + "screenshot:", + [image_attachment(_png(1920, 1080), "image/png")], + _vision_profile(), + ) + assert estimate_message_tokens(withimage) - text_only > 1500 + + +# ── attachment ──────────────────────────────────────────────────────────── + + +def test_attached_image_becomes_an_openai_content_part() -> None: + message = _attached( + "screenshot:", + [image_attachment(_png(64, 32), "image/png", label="/tmp/a.png")], + _vision_profile(), + ) + content = message["content"] + assert isinstance(content, list) + assert content[0] == {"type": "text", "text": "screenshot:"} + assert content[1]["type"] == "image_url" + assert content[1]["image_url"]["url"].startswith("data:image/png;base64,") + # The block itself carries nothing extra: the served endpoint's pydantic + # union rejects an unknown key inside a content part outright. + assert set(content[1]) == {"type", "image_url"} + assert set(content[1]["image_url"]) == {"url"} + + +def test_bookkeeping_never_reaches_the_wire() -> None: + message = _attached( + "screenshot:", [image_attachment(_png(64, 32), "image/png", label="/tmp/a.png")], + _vision_profile(), + ) + assert message["image_meta"][0]["label"] == "/tmp/a.png" + assert "image_meta" not in for_wire([message])[0] + + +def test_withheld_image_says_so_in_the_text() -> None: + """The anchor case: a model that cannot see the image must be told. + + Removing the image block and changing nothing else is what made the + calibration model report a confident, entirely invented reading of the + picture. Silence here is a correctness bug, not a missing feature. + """ + message = _attached( + "screenshot:", + [image_attachment(_png(64, 32), "image/png", label="/tmp/a.png")], + _blind_profile(), + ) + content = message["content"] + assert isinstance(content, str) + assert "screenshot:" in content + assert "have NOT" in content + assert "/tmp/a.png" in content + assert "text-only" in content + + +def test_unsupported_protocol_withholds_rather_than_guessing_a_block_shape() -> None: + profile = ModelProfile( + model_id="claude", provider="anthropic", + supports_images=True, protocol="anthropic", + ) + message = _attached("shot:", [image_attachment(_png(8, 8), "image/png")], profile) + assert isinstance(message["content"], str) + assert "anthropic" in message["content"] + + +def test_no_images_leaves_the_message_exactly_as_it_was() -> None: + message = tool_msg("plain result", "call_1") + attach_images(message, [], profile=_vision_profile()) + assert message == {"content": "plain result", "role": "tool", "tool_call_id": "call_1"} + + +def test_flattening_to_text_leaves_a_mark() -> None: + message = _attached( + "shot:", [image_attachment(_png(8, 8), "image/png")], _vision_profile(), + ) + # Anthropic translation, compaction summaries and the trajectory all read + # content through text_of; an image that flattens to nothing produces a + # transcript claiming a picture was never there. + assert "[image" in text_of(message["content"]) + + +# ── eviction ────────────────────────────────────────────────────────────── + + +def _history(count: int) -> list[Any]: + history: list[Any] = [] + for index in range(count): + history.append( + _attached( + f"shot {index}:", + [image_attachment(_png(16, 16), "image/png", label=f"/tmp/{index}.png")], + _vision_profile(), + ) + ) + return history + + +def test_eviction_keeps_the_newest_images() -> None: + history = _history(5) + assert evict_old_images(history, 2) == 3 + assert [image_blocks_in(m) for m in history] == [0, 0, 0, 1, 1] + + +def test_evicted_image_leaves_a_sentence_naming_it() -> None: + history = _history(2) + evict_old_images(history, 1) + oldest = history[0]["content"] + assert isinstance(oldest, str) + assert "shot 0:" in oldest + assert "/tmp/0.png" in oldest + assert "no longer see it" in oldest + + +def test_eviction_drops_the_bookkeeping_it_no_longer_describes() -> None: + history = _history(3) + evict_old_images(history, 1) + assert "image_meta" not in history[0] + assert len(history[-1]["image_meta"]) == 1 + + +def test_partial_eviction_within_one_message_keeps_the_rest_aligned() -> None: + message = _attached( + "pages:", + [ + image_attachment(_png(16, 16), "image/png", label="/tmp/p1.png"), + image_attachment(_png(16, 16), "image/png", label="/tmp/p2.png"), + ], + _vision_profile(), + ) + assert evict_old_images([message], 1) == 1 + assert image_blocks_in(message) == 1 + # The surviving entry must be the one still present -- p2, the newer. + assert [entry["label"] for entry in message["image_meta"]] == ["/tmp/p2.png"] + assert "/tmp/p1.png" in text_of(message["content"]) + + +def test_eviction_is_idempotent_and_ignores_text_messages() -> None: + history = _history(2) + history.insert(0, tool_msg("no images here", "call_x")) + assert evict_old_images(history, 1) == 1 + assert evict_old_images(history, 1) == 0 + assert history[0]["content"] == "no images here" + + +def test_negative_budget_disables_eviction() -> None: + history = _history(3) + assert evict_old_images(history, -1) == 0 + assert sum(image_blocks_in(m) for m in history) == 3 + + +def test_zero_budget_evicts_everything_but_says_so_each_time() -> None: + history = _history(2) + assert evict_old_images(history, 0) == 2 + assert all(isinstance(m["content"], str) for m in history) + assert all("no longer see it" in m["content"] for m in history) + + +# ── interaction with compaction ─────────────────────────────────────────── + + +def test_stale_bookkeeping_stops_charging_once_the_images_are_gone() -> None: + """A compactor may rewrite content to a string without knowing about images. + + ``compress_tool_results`` flattens tool content through ``text_of`` and + assigns a plain string. The images are gone from the request at that point, + so the estimate must stop charging for them even though ``image_meta`` is + still on the message. + """ + message = _attached( + "shot:", [image_attachment(_png(1920, 1080), "image/png")], _vision_profile(), + ) + assert estimate_message_tokens(message) > 1500 + message["content"] = "…condensed by a compactor…" + assert "image_meta" in message + assert estimate_message_tokens(message) < 100 + + +def test_images_without_bookkeeping_are_still_charged() -> None: + """History restored from a checkpoint written before ``image_meta`` existed.""" + message = _attached( + "shot:", [image_attachment(_png(1920, 1080), "image/png")], _vision_profile(), + ) + del message["image_meta"] + assert estimate_message_tokens(message) > 1000 + + +# ── trace redaction ─────────────────────────────────────────────────────── + + +def test_trace_redaction_keeps_the_shape_and_states_the_size() -> None: + from agent_core.tool_content import redacted_for_trace + + message = _attached( + "shot:", [image_attachment(_png(1920, 1080), "image/png")], _vision_profile(), + ) + traced = redacted_for_trace(message) + url = traced["content"][1]["image_url"]["url"] + assert "base64" in url + assert "elided from trace" in url + assert "KB" in url + # Still visibly an image, so a trace does not misrepresent what the model saw. + assert traced["content"][1]["type"] == "image_url" + assert traced["content"][0] == {"type": "text", "text": "shot:"} + # And the real message is untouched. + assert message["content"][1]["image_url"]["url"].startswith("data:image/png;base64,i") + + +def test_trace_redaction_is_a_no_op_for_ordinary_messages() -> None: + from agent_core.tool_content import redacted_for_trace + + plain = tool_msg("no images", "call_1") + assert redacted_for_trace(plain) is plain + assert redacted_for_trace("not a message") == "not a message" + + +def test_the_trajectory_observer_does_not_write_base64() -> None: + import json + import tempfile + from pathlib import Path + + from agent_core.components.observers.trajectory import TrajectoryFileObserver + + message = _attached( + "shot:", [image_attachment(_png(1920, 1080), "image/png")], _vision_profile(), + ) + with tempfile.TemporaryDirectory() as tmp: + observer = TrajectoryFileObserver(Path(tmp)) + rendered = observer._message_to_dict(message) + assert rendered is not None + body = json.dumps(rendered) + assert "elided from trace" in body + # A 1080p PNG is ~137 KB of base64; the trace entry must not carry it. + assert len(body) < 2000 + + +@pytest.mark.asyncio +async def test_live_trajectory_records_returned_images_without_base64(tmp_path) -> None: + import json + from types import SimpleNamespace + + from agent_core.components.observers.trajectory import TrajectoryFileObserver + from agent_core.loop_types import LoopConfig, ToolResult + + attachment = image_attachment( + _png(64, 32), + "image/png", + label="/tmp/live.png", + ) + observer = TrajectoryFileObserver( + tmp_path, + filename="live", + formats=["json", "jsonl"], + ) + await observer.on_loop_start(LoopConfig(task_id="probe")) + await observer.on_tool_result( + SimpleNamespace(turn=1), + ToolResult( + name="view_image", + args={"path": "/tmp/live.png"}, + result="image follows", + duration_ms=1, + tool_call_id="call_1", + is_error=False, + images=[attachment], + ), + ) + observer._flush_json(force=True) + + envelope = json.loads((tmp_path / "live.json").read_text(encoding="utf-8")) + content = envelope["messages"][-1]["content"] + assert content[1]["type"] == "image_url" + assert "elided from trace" in content[1]["image_url"]["url"] + + jsonl = (tmp_path / "live.jsonl").read_text(encoding="utf-8") + assert '"images"' in jsonl + assert "elided from trace" in jsonl + assert attachment["data"] not in json.dumps(envelope) + assert attachment["data"] not in jsonl diff --git a/uv.lock b/uv.lock index 3d5cbca..0d01738 100644 --- a/uv.lock +++ b/uv.lock @@ -50,7 +50,7 @@ wheels = [ [[package]] name = "apodex-agent-core" -version = "0.9.1" +version = "0.10.0" source = { editable = "." } dependencies = [ { name = "anthropic", extra = ["bedrock"] },