fix(agent): bound historical image payloads per request

[Kewe63]

Summary

Long-running multimodal sessions can accumulate inline base64 images in user and tool-result messages. Every subsequent model request re-sends those images, allowing the HTTP request body to grow by multiple megabytes even while the estimated token count remains well below the context-compression threshold.

This can trigger provider HTTP 413 responses. Compression alone may not recover because the existing _strip_historical_media pass only runs during compaction and anchors on the newest image-bearing user message. As a result, tool-result images and images in the first user turn can survive repeated compaction attempts.

This PR adds a request-time image bound independent of context compression:

• Keep all image parts in the newest three image-bearing messages.
• Replace image parts in older messages with a stable text placeholder.
• Preserve the message rows and tool_call_id fields so tool-call/result pairing remains valid.
• Apply the transformation only to the per-request API copy; persisted conversation history remains untouched.
• Drop stale api_content sidecars from rewritten request messages so removed image bytes cannot be restored during replay.
• Apply the same policy to the main conversation loop and the independently assembled max-iteration summary request.

The transformation supports the three multimodal shapes Hermes currently handles:

• Chat Completions image_url
• Responses API input_image
• Anthropic-native image

Removing an image necessarily changes the request prefix once when that image leaves the retention window. The projection is deterministic afterward: repeated requests over identical history are byte-identical, and each historical image ages out only once. The transformation runs before prompt-cache planning so cache markers describe the actual post-eviction request. This mirrors the existing Anthropic computer-use screenshot retention policy.

---

Existing PR Relationship

This change was compared against the existing image-payload PRs:

• fix(agent): evict old tool-result screenshots on the OpenAI-compatible path #64440 focuses on tool-result screenshots in the OpenAI-compatible path.
• fix(agent): stop old user images from bloating every model request #87555 focuses on historical user images.
• fix(agent): drop vision payloads before compressing on a 413 #89776 reactively removes vision payloads after a 413 response.

This PR addresses #89938's combined failure mode proactively across user and tool messages, all supported image-part formats, the main request path, and the max-iteration summary path.

---

Changes Made

agent/context_compressor.py

• Add _strip_old_image_parts.
• Retain the newest three image-bearing messages.
• Replace older image parts with stable placeholders.
• Remove stale api_content sidecars from rewritten request rows.
• Preserve message and tool-pair structure.

agent/conversation_loop.py

• Apply request-time image eviction after message sanitization and before prompt-cache planning.

agent/chat_completion_helpers.py

• Apply the same image bound to the independently assembled max-iteration summary request.

tests/agent/test_request_image_eviction.py

• Cover mixed user/tool roles.
• Cover per-message rather than per-image retention counting.
• Cover image-only tool results and tool-pair preservation.
• Cover stale api_content cleanup.
• Cover Chat Completions, Responses API, and Anthropic image shapes.
• Verify deterministic request projection and one-time age-out behavior.

tests/run_agent/test_413_compression.py

• Exercise the real conversation request path.
• Verify only three image-bearing messages reach the provider.
• Verify canonical history retains all original images.

tests/run_agent/test_run_agent.py

• Exercise the real max-iteration summary path.
• Verify summary requests are bounded while internal history remains unchanged.

---

How to Test

1. Start a session with a vision-capable model.
2. Attach an image in the first user message.
3. Produce at least five image-bearing vision_analyze or other multimodal tool results.
4. Continue the conversation.
5. Verify the provider request contains image parts only in the newest three image-bearing messages.
6. Verify older image parts are represented by text placeholders.
7. Verify the persisted conversation history still contains every original image.

Focused canonical test suite:

scripts/run_tests.sh \
  tests/agent/test_request_image_eviction.py \
  tests/agent/test_compressor_historical_media.py \
  tests/agent/test_prompt_caching.py \
  tests/gateway/test_cached_agent_max_iterations.py \
  tests/run_agent/test_413_compression.py \
  tests/run_agent/test_run_agent.py

✅ 334 tests passed, 0 failed

Adjacent multimodal and compression suites:

scripts/run_tests.sh \
  tests/run_agent/test_multimodal_tool_content_recovery.py \
  tests/run_agent/test_vision_aware_preprocessing.py \
  tests/agent/test_context_compressor.py

✅ 157 tests passed, 0 failed

Additional checks:
ruff check: passed
py_compile: passed
git diff --check: passed

The regression tests were also sabotage-tested with the eviction helper disabled: 4 expected failures, proving the tests exercise the fix.

---

Checklist

• Follows Contributing Guide and Conventional Commits
• Searched for existing PRs and documented related implementations above
• Changes scoped to this fix/feature only — no unrelated commits
• pytest tests/ -q — pending full-suite CI
• Tests added for the change
• Tested on Ubuntu under WSL2
• Relevant behavior and prompt-cache trade-offs documented in code; no user-facing docs change required
• No configuration keys added or changed
• No contributor workflow or architecture guide changes
• Cross-platform impact considered — transformation operates only on Python message structures
• No tool descriptions or schemas changed

---

Risk & Impact

Low. The transformation applies only to the per-request API copy — persisted conversation history is never touched. Tool-call/result pairing is preserved via retained tool_call_id fields. The projection is deterministic: repeated requests over identical history are byte-identical, and prompt-cache markers are computed after eviction so cache planning reflects the actual outgoing request.

Type: 🐛 Bug fix / ✅ Tests
Fixes: #89938

[andrexibiza]

Reviewed exact head fd77a2b4d67612d45cd3d859e1417a9163ca038a against base/current main 13ce0c5c675e843af70d19c9e5144249cd51c8d1, the #89938 reproduction, the request-assembly paths in conversation_loop.py and the independently assembled max-iteration summary, exact-head CI, and the overlapping image-lifecycle work in #64440/#63850/#87555/#89776.

The core direction is right: this belongs at request projection, not in durable history. I verified that the main loop structurally clones each message before the new transform, and the summary path's shallow copy is still safe here because _strip_old_image_parts() replaces the copied row's top-level content list rather than mutating nested image parts in place. Running before prompt-cache planning is also the correct ordering, and covering the summary path closes the same independent-send seam that earlier screenshot work had to discover separately.

I would not merge this exact head yet, because the unified newest-three policy drops the active user's image while that same user turn is still in flight.

Blocker — image recency is being counted across roles/messages, but the active user turn is semantic input, not historical media

_strip_old_image_parts() walks every image-bearing message newest-first with one shared remaining = 3 budget. It has no notion of current_turn_user_idx, role, or turn boundary. That means three later image-bearing tool results are enough to evict the image(s) from the user message that initiated the current turn.

The new integration test actually pins this behavior: it builds one image-bearing user message followed by five image-bearing vision_analyze tool results, then asserts only three image-bearing messages reach the provider. Under this implementation those three survivors are the newest tool results; the initiating user image is replaced by the placeholder before the model produces the final answer.

That is a correctness regression even though persistence is intact. A request such as “compare this screenshot against the references you inspect” can lose the screenshot the user is asking about after the agent has made three vision/tool calls during the same turn. The max-iteration summary path has the same problem: by the time it asks the model for the final response, enough tool-image messages can have displaced the active user image from the keep window.

The repository already has the useful split in the adjacent work:

• #64440 by @bnikanjam bounds historical tool-result screenshots while deliberately leaving user uploads alone.
• #87555 by @fangliquanflq bounds historical user images but explicitly preserves the newest/current user turn in full.
• #63850 by @bnikanjam is the complementary producer-side per-embed cap for browser_vision.
• #89776 by @AP3X-Dev is complementary reactive 413 recovery: drop retained vision bytes before spending a compression attempt.

#89965 is the stronger candidate to supersede the first two request-projection implementations because it unifies formats/roles and covers both send sites, but it needs to preserve their ownership distinction rather than flatten it.

Required fix: anchor the policy to the current turn. The current user message (and, if needed, other user-authored multimodal input belonging to that active turn) must remain visible while the turn is executing; apply the newest-N eviction to historical media/tool results outside that protected input. Add a regression with user(image) -> >=3 image-bearing tool results -> final model call proving the user image is still present while older tool/history images are bounded. Mirror the same invariant in the max-iteration summary test.

I would also preserve contributor provenance explicitly if this PR supersedes #64440/#87555 rather than merely listing them as related: those PRs independently established the two halves of the request-time lifecycle this branch is combining.

CI / composition

The branch is based directly on current main. Nix and Docker are green. The CI workflow is red only in Python slice 8 on tests/tools/test_image_generation.py::TestFalCatalog::test_upscale_defaults_are_all_off (xai/grok-imagine-image/v2.0/text-to-image still has upscale=True), with 3,946 tests passing in that slice; that failure is outside this six-file diff and is the known current-main Grok catalog regression, not evidence against this patch. The changed-area lint, OS-specific, E2E, and the displayed Python slices are otherwise green.

Re-review gate: preserve the active-turn user multimodal input, keep the request-only/deterministic projection and both send-site coverage, retain the historical tool/user eviction witnesses, and attach the exact-head focused regression. I did not find a reason to reject the overall request-time eviction architecture.

[jackulau]

I had the other half of #89938 built when this appeared, so rather than open a competing PR I opened #90001 for the part this one leaves in place, and here is the one thing in this diff I would want a second look at before it merges.

The two changes are complementary, not alternatives

This PR bounds the bytes on every outgoing request, so the 413 stops happening. It does not touch _strip_historical_media, which is what the 413 handler's recovery compaction ends up calling - and in the reported session that function is a no-op, because the anchor is the newest image-bearing user message and the reproduction has exactly one, at index 0. anchor <= 0 returns the list untouched.

That is the "7 compactions in 13 minutes, all below 200K tokens" in the report: the recovery pass ran seven times and freed nothing. With only this PR merged the wedge becomes unreachable in the common case but it is still there - any window that exceeds the provider's body limit on its own (a burst larger than keep_recent, or a provider whose cap is smaller than three screenshots) drops straight back into it with no exit.

#90001 is that fix and only that fix: Refs #89938, not Fixes, one function, no overlap with any line here. If the maintainers want one change rather than two, this is the one to take - it is the larger fix and it is the one that prevents the failure.

The prefix churn is bigger than "once per image"

Removing an image necessarily changes the request prefix once when that image leaves the retention window. The projection is deterministic afterward: repeated requests over identical history are byte-identical.

Both sentences are true, and I think they undersell the cost, because the property that matters for prompt caching is stability across turns, not across repeats of the same history.

Walk two consecutive turns of a vision session, keep_recent=3, where messages A, B, C, D each carry an image:

• Turn N, history [A, B, C]: all three are within the window, all keep their pixels.
• Turn N+1, history [A, B, C, D]: the window is now B, C, D, so A is rewritten - and A sits in the prefix, ahead of everything retained.

The request sent at turn N+1 therefore diverges from the request sent at turn N at message A, which is about as early as a divergence can be. Every subsequent cached token is lost. In a session where most turns carry an image - which is the exact session this PR is for - that is a cache break on essentially every turn, not once per image.

Running the pass before cache planning is the right call and I would keep it: it means prompt_cache_key describes the payload actually sent rather than a phantom. But it prevents a wrong key, not a missed one, and this file has a lot of history invested in not missing (_content_cache_key and the scope work in #51395 / #78941 / #79017 exist to keep recurring sessions landing on a warm prefix).

Three ways to buy the stability back, none of which I would insist on:

1. Quantize the window. Recompute the strip boundary only when the image-bearing count crosses a multiple of K, so the prefix stays byte-stable for K turns and then moves once. Cheap, and it converts one break per turn into one per K.
2. Budget by bytes, not by message count. keep_recent=3 is a proxy for "small enough body"; three 4MB screenshots still 413, and thirty 20KB thumbnails never would. A cumulative-byte budget walked newest-first strips exactly as much as it must, and on a light session strips nothing at all - which means no prefix churn at all in the common case.
3. Make it reactive. Arm the bound only after this session has seen a 413. That gives up the "never 413 in the first place" property, which is most of the value here, so I mention it for completeness rather than as a recommendation.

Whatever the answer, it is worth a sentence in the PR body: right now a reader comes away thinking the cache cost is one break per image, and it is one per image-bearing turn.

Two small ones

_strip_old_image_parts is now module-spanning API under a private name. It is imported by agent/conversation_loop.py and agent/chat_completion_helpers.py, so the leading underscore no longer describes it. Renaming costs nothing now and gets awkward once there are three call sites and a test suite pinned to the old spelling.

Two failures you will see in CI are not yours. On a pristine 13ce0c5c67 here, tests/agent/test_compression_review_76354.py::TestF6ExecutorSaturation::test_cancelled_fence_skips_summary_work_before_start fails, as do four in the -k "image or vision" sweep (test_image_routing.py x2, test_save_url_image.py, test_vision_routing_31179.py). I confirmed each with the working tree stashed. Worth knowing before you go looking for them in this diff.

Happy to rebase #90001 onto this one in whichever order they land, or to fold it in here if you would rather ship a single change - it is 20 lines and its tests are self-contained.

[andrexibiza]

@jackulau I checked #90001 at 2499a77fd000f2e687402847a82541bb0d0069e7 against this exact head fd77a2b4d67612d45cd3d859e1417a9163ca038a and the current 413 branch. Your cache correction is right, but the recovery topology is three-way, not two-way:

1. fix(agent): bound historical image payloads per request #89965 owns proactive request projection: bound image payloads before every normal send, on the per-request copy, in both the main loop and max-iteration summary path.
2. fix(agent): drop vision payloads before compressing on a 413 #89776 owns the first provider-proven 413 reaction: strip image-bearing tool content from the already-built api_messages before spending a compression attempt, then retry that request immediately.
3. fix(compressor): age out stale tool-result images during compaction #90001 changes generic compaction policy: when _compress_context(messages) runs, _strip_historical_media ages tool-result images out of the compacted transcript so the outer loop can rebuild a smaller request.

So “no overlapping line” is true, but it is not the same as no behavioral overlap. #89776 and #90001 both occupy the reactive 413/recovery class at different representations and different moments. #90001 is also not strictly a 413-only fix: _strip_historical_media runs for threshold/manual compaction too, so “keep only the newest tool image” becomes a general compaction invariant.

I agree with the central topology point: #89965 alone does not close the provider-body-size class. keep_recent=3 is a count heuristic, not a byte bound; three sufficiently large screenshots can still exceed a provider cap. Exact head also has the more immediate correctness blocker from my review: three later tool images can evict the active user’s image while that same turn is still executing. Until that is fixed and the recovery seam is accounted for, Fixes #89938 overclaims; this should be Refs #89938, or issue closure should be explicitly contingent on the accepted recovery composition.

Your prompt-cache correction is also correct. “Each image ages out once” describes per-message idempotence, not cross-turn prefix stability. Once the window is saturated, each new image-bearing turn advances the strip boundary and invalidates the warm suffix from that boundary forward. Running before cache planning guarantees an honest key for the payload actually sent; it does not preserve a hit. The PR body/docstring should say that plainly.

Of the three alternatives you listed, I would not make quantization a merge requirement: it reduces invalidation frequency by accepting a looser payload ceiling and larger periodic cache breaks. A byte budget is the policy most directly aligned with HTTP 413, but it needs an explicit active-turn floor and a defensible estimate of serialized request bytes across transports. At minimum, the current PR must describe keep_recent=3 as a bounded heuristic rather than proof that the body fits.

Agree on renaming _strip_old_image_parts now. It is imported by two other modules and pinned by tests; something like bound_request_image_payloads() would describe its ownership better than a private helper name.

One caution on #90001 for its own review: keeping only the newest tool image can discard multiple tool images gathered intentionally during the active turn, and because the hook is generic it can do so during ordinary compaction, not only emergency recovery. That may still be the right shedding policy, but it needs a deliberate turn/byte invariant and a regression for a current-turn multi-image comparison. I would review that independently rather than fold it into this PR by default.

My exact-head merge position on #89965 therefore remains unchanged: preserve active-turn user multimodal input, prove that invariant in both send sites, correct the cache/body-bound language, rename the shared helper, and interlock—not flatten—the provenance and recovery roles of #64440, #87555, #89776, and #90001.