perf(middleware): stop checkpointing view_image base64 payloads - #5014

Why

ViewImageMiddleware feeds viewed images to the model by injecting a hidden HumanMessage carrying their base64 payloads. It did that from before_model and undid it from after_model.

before_model, model, and after_model are separate graph nodes, so every turn that follows a view_image call paid for two extra nodes and two state writes, and up to 20 MB of base64 sat in two checkpoints for the duration of the model call. The payload is transient context that nothing downstream reads back — it exists only to be handed to the provider on one call.

The window also had a failure mode: a run interrupted between the two hooks (user cancel, restart, timeout) left the payload checkpointed with no cleanup pass ever coming, so it stayed in the thread's history.

What changed

• ViewImageMiddleware injects from wrap_model_call / awrap_model_call instead of the before_model / after_model pair. The message lives only in ModelRequest.messages and is never returned as a state update, so no checkpoint carries it. _remove_image_context_messages and its RemoveMessage bookkeeping are gone with the hooks; the async path keeps the existing asyncio.to_thread() offload for the file read and base64 encode.
• Injection is now decided against request.messages rather than state, so the gate judges the messages the model will actually receive.
• _inject sweeps this middleware's own message out of the request before rebuilding it. Dropping after_model also drops the cleanup it performed on every call, so without the sweep a payload stranded by an older interrupted run would keep riding along in every later request for the life of the thread. The sweep requires both the reserved id prefix and the server-owned marker, and Gateway strips that marker from client input, so a user-authored message can never be dropped.
• Chain position is unchanged. wrap_model_call nests first-registered-outermost, so TokenBudgetMiddleware still sees the image message and enforces the input budget against it; SummarizationMiddleware is before_model-only and did not see it before either.
• Docs updated in the same change: middleware chain entry 23, backend AGENTS.md Vision Support, the middleware-execution-flow.md hook matrix and diagrams, and the strip_data_url_image_blocks docstring.

Not addressed, deliberately: checkpoint rows that already hold a stranded payload keep it on disk. It is inert — never sent to a provider, and strip_data_url_image_blocks keeps it off the wire — and reclaiming it would mean keeping the after_model node this change removes.

Surface area

• Frontend UI — page / component / setting / interaction under frontend/
• Backend API — endpoint / SSE event / request-response shape under backend/app
• Agents / LangGraph — agent node, graph wiring, langgraph.json, or prompt change
• Sandbox — docker/ or sandboxed execution
• Skills — change under skills/
• Dependencies — new/upgraded entry in backend/pyproject.toml or frontend/package.json
• Default behavior change — changes existing behavior without the user opting in
• Docs / tests / CI only — no runtime behavior change

On the default-behavior box: the image context message no longer appears in checkpointed messages, and therefore no longer appears in values stream frames. It was already hide_from_ui and already stripped from history responses by strip_data_url_image_blocks, so no UI or IM surface changes — but the persisted data shape does, which is why the box is checked.

Validation

• cd backend && make lint
  • All checks passed! / 1228 files already formatted
• cd backend && make test
  • 12307 passed, 78 skipped
• uv run pytest tests/test_view_image_middleware.py -q
  • 43 passed
• uv run ruff check / ruff format --check on the three changed Python files
  • passed / 3 files already formatted
• git diff --check
  • passed

tests/test_view_image_middleware.py is rewritten around the new hook: sync and async at both the unit and graph level, the stranded-payload sweep, and the protection that stops a client-supplied message wearing the reserved id prefix from being swept.

Two behavioural claims in "What changed" were verified against the real graph rather than assumed: graph.ainvoke dispatches to awrap_model_call (so the 20 MB read stays off the event loop), and a middleware registered after ViewImageMiddleware sees the injected message while one registered before it does not.

AI assistance

Tool(s) used: Claude Code

How you used it: I forked the repo because I spotted that ViewImageMiddleware still used a before_model/after_model pair to inject and then remove the image payload — wasteful per turn, and it strands the payload if a run dies mid-call. I decided the direction (move it into wrap_model_call so it never enters state) and had Claude Code implement it, along with the tests and doc updates.

I pushed back over several rounds and asked it to re-verify its own claims rather than accepting the first answer; that caught a real regression — the old after_model also swept payloads stranded by earlier interrupted runs, and the first version silently dropped that cleanup — which is why _inject now sweeps the request. It also verified the ordering and async-dispatch claims against a real graph instead of assuming them.

I reviewed the final diff myself before opening this.

• I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output.