feat(loop): let a tool hand the model an image, not a description of one - #36
Merged
Merged
Conversation
A tool that had an image to return could only send prose about it: the products called a separate vision model and put its transcript into the history. The main model never saw the pixels, could not look again when a later turn raised a new question about the same picture, and the round trip was not even cheap — a transcript runs 2-6K tokens where the image it describes is a few hundred. A tool can now return `tool_content(text, images=[image_attachment(...)])`. The envelope is a plain JSON dict rather than a dataclass because sandbox-native tools execute in a child process and their return value crosses `json.dumps(default=str)` on the way back; a dataclass arrives as its repr. `ToolResult.images` carries the attachments and `result` stays the full text, so observers, spill/recovery, repeat detection and the trajectory see exactly what they saw before. Visibility is the loop's decision, not the tool's. `ModelProfile.supports_images` (default False) and the wire protocol gate it; only `chat_completions` is implemented, as an OpenAI `image_url` part carrying a data URI. The Anthropic block spelling is not interchangeable — a served OpenAI-compatible endpoint answers it with HTTP 400 and a pydantic union error, failing the whole turn including the other tool results in it — so an unimplemented protocol withholds instead of guessing. `HistoryPolicy.max_images_in_history` (default 5) now bounds the history. Both flags existed as unread placeholders. An image that leaves the context always leaves a sentence behind, whether it was withheld for capability or evicted for room, and `text_of` renders an image block as a placeholder rather than as nothing. This is the finding the whole design is built around: 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, every one invented. A tool result that reads as though an image were delivered gets answered as though one were. `estimate_message_tokens` now charges for inline images, priced off the `image_meta` message key written at attach time (~1 token per 1024 px, fitted against measured prompt_tokens; 1080p ≈ 2.4K, 4K ≈ 8.5K). It previously returned the caption length alone, so a history of screenshots measured as nearly empty to the context guard and every compaction trigger. The estimate is driven by the image blocks actually present, so a compactor that rewrites content to a string stops the charge with them. Tests: 33 cases across tests/test_tool_content_images.py (envelope validation, header sniffing for PNG/JPEG/WEBP/GIF, attachment, withholding, eviction order, the stale-bookkeeping case) and tests/test_agent_loop_image_results.py (the same behaviours driven through run_agent_loop). Two bugs were found by writing them: a multi-image result evicted its own newest page first, and stale `image_meta` kept charging for images a compactor had already dropped. Generated with [Claude Code](https://claude.ai/code) via [Very Happy](https://github.com/Mereithhh/very-happy) Co-Authored-By: Claude <noreply@anthropic.com>
`ExtensionsConfig.has_changed` compared `st_mtime > loaded_mtime`, which is asking a coarse clock to order edits far finer than it can see. Measured on this filesystem: the timestamp advances in 1 ms steps, and two consecutive writes land on an identical mtime about 92% of the time. A skill toggled within a millisecond of the load was therefore invisible to `get_enabled_skills`, and the operator's change simply did not take. The same strict `>` could not see a timestamp moving BACKWARD either, which is the ordinary result of restoring a backup, a `git checkout`, or an `rsync --times` of an older revision: contents changed, config reported no change. And it fired for an identical rewrite, forcing a reload with nothing to reload. Compare a blake2b-128 digest of the bytes instead, taken from the exact bytes that were parsed so the fingerprint and the loaded state can never describe different contents. The file is a small JSON document and `get_enabled_skills` is cached by its callers, so reading it is cheaper than being wrong about it. This was already reaching CI as an intermittent failure in test_skills_loader_reload.py, at a rate that tracked machine speed — the test passed only because the work between its two writes usually spilled into the next millisecond, and a warm suite run did not. Its own `os.utime(+10)` defence never helped: it shifts both timestamps by the same constant. The three regression tests pin both mtimes to one value instead, so they fail deterministically against the old check rather than a quarter of the time, and they cover the backward-moving timestamp and the identical rewrite as well. Verified failing on the previous implementation and passing on this one; 60+ full-suite runs since, with no recurrence. Generated with [Claude Code](https://claude.ai/code) via [Very Happy](https://github.com/Mereithhh/very-happy) Co-Authored-By: Claude <noreply@anthropic.com>
`_message_to_dict` copied a history message verbatim, which was correct while content was always text. With inline images it means the base64 of every attachment is written into the trajectory again for each turn the image survives in history — a single 1080p screenshot is ~137 KB, so a handful of them turn a readable trace into tens of megabytes of unreadable one. `tool_content.redacted_for_trace` replaces the payload with its size and is used by the observer. It is exposed rather than inlined because the trajectory is not the only thing that writes a message somewhere other than the provider; a log line or an event record wants the same treatment. The block keeps its `image_url` type and states how large the image was instead of vanishing. Same reason the loop narrates an eviction: a trace that shows no image where the model was looking at one misrepresents what the model was answering, and a trace is read precisely when someone is trying to work out why an answer was what it was. Non-image messages are returned unchanged, by identity, so the common path allocates nothing. Generated with [Claude Code](https://claude.ai/code) via [Very Happy](https://github.com/Mereithhh/very-happy) Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A tool with an image to return could only send prose about it: the products called a separate vision model and put its transcript into the history. The main model never saw the pixels, could not look again when a later turn raised a new question about the same picture, and the round trip was not even cheap — a transcript runs 2-6K tokens where the image it describes is a few hundred.
Verified against the served endpoint before any of this was written, not assumed. Full matrix, numbers and raw errors: MiroHarness
internal-docs/designs/2026-09-08-native-image-in-tool-result-calibration.md.What
A tool returns
tool_content(text, images=[image_attachment(...)]). The envelope is a plain JSON dict, not a dataclass, because sandbox-native tools execute in a child process and their return value crossesjson.dumps(default=str)on the way back.ToolResult.imagescarries the attachments;resultstays the full text, so observers, spill/recovery, repeat detection and the trajectory see exactly what they saw before.Visibility is the loop's decision.
ModelProfile.supports_images(defaultFalse) and the wire protocol gate it; onlychat_completionsis implemented, as an OpenAIimage_urlpart carrying a data URI. The Anthropic block spelling is not interchangeable — the served endpoint answers it with HTTP 400 and a pydantic union error, failing the whole turn including the other tool results in it — so an unimplemented protocol withholds rather than guessing.HistoryPolicy.max_images_in_history(default 5) now bounds the history. Both flags existed as unread placeholders.The constraint the design is built around
An image that leaves the context always leaves a sentence behind — withheld for capability, evicted for room, or flattened by
text_of. 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, every one invented, having been told to answerNO_IMAGEif it could not see one. A tool result that reads as though an image were delivered gets answered as though one were.test_withheld_image_says_so_in_the_textis the anchor for this.Also here
estimate_message_tokenscharges for inline images (~1 token/1024 px, fitted against measuredprompt_tokens; 1080p ≈ 2.4K, 4K ≈ 8.5K). It previously returned the caption length alone, so a history of screenshots measured as nearly empty to the context guard and every compaction trigger.TrajectoryFileObserverno longer writes image base64 into the trace (~137 KB per 1080p image, per turn it survived). The block keeps itsimage_urltype and states its size.ExtensionsConfig.has_changedcomparedst_mtime >, which cannot order edits finer than the clock. Measured here: the filesystem timestamp advances in 1 ms steps and two consecutive writes collide about 92% of the time, so a skill toggled within a millisecond of the load was invisible; the strict>also could not see a timestamp moving backward (restored backup,git checkout,rsync --times) and fired for an identical rewrite. Now compares a content digest. This was reaching CI as an intermittenttest_skills_loader_reload.pyfailure whose rate tracked machine speed.Verification
1548 passed, 2 skipped· ruff clean · pyright 104 errors — identical to themainbaseline, zero added ·check_unconsumed_fieldsexit 0.Two bugs were found by writing the tests rather than by review: a multi-image result evicted its own newest page first (walking history backward but blocks forward), and stale
image_metakept charging tokens for images a compactor had already dropped.Also verified end-to-end from the consuming product through the real
run_agent_loopagainst live apodex-1.1-mini: with the flag on the model reads a random code and three shapes off the image and the tool message is a block list; with it off the content is plain text and nothing leaks; a 4K image is still read correctly after the product downscales it.Note on versioning
Bumped to 0.10.0 (new capability → MINOR per
docs/versioning.md), superseding the0.9.1this is rebased onto. The 0.9.1 changelog section is preserved above 0.9.0. No tag pushed — releasing is a separate, deliberate step once this is reviewed.🤖 Generated with Claude Code