Pull requests / #3379

#3379 [Other] serve: a tool's picture starts the elastic encoder, and its turn counts

open · @jerry78424 · 0 comments · View on GitHub

Server & APIAMD / HIPNVIDIA / CUDAModels & quantsSecurityWindows

Description

## Problem and result
`_note_unreadable_tool_images` - the method that promises "a picture a tool returned that this server cannot read becomes a short note in its place instead of a 400" - encodes the picture without starting the elastic encoder first. Once the idle sweep has stopped the encoder (`elastic_idle_s` without a picture, default 120 s), the tool's picture writes to a pipe that no longer exists: the `AttributeError` on the `None` proc escapes the `except (ValueError, OSError)` filter and the route's body-shape filter, and the request dies as a **400 "the request is malformed (AttributeError...)"** - the opposite of what the method exists to do. And because clients re-send the whole history every turn, every later request of the conversation dies the same way at this method's own line: the conversation stays broken until a picture from another conversation (or a restart) starts the encoder.

Reproduce: `"elastic": true` + `"vram_elastic": true`; send one picture, let the encoder idle-stop (log: `[strata] images: the image encoder stopped after N s ...`); then send a `tool` message whose content carries an `image` block (Claude Code's Read of a picture - the exact input 3709c15f added this method for). Expected per the method's own docstring: a `[image omitted: ...]` note. Actual: `AttributeError: 'NoneType' object has no attribute 'stdin'` (inside `Vision.encode`'s `self.proc.stdin.write`) surfaces as a 400.

After the change the tool picture starts the encoder first, the note is written as documented, and the conversation survives.

## Related discussion
Follows #2641 (same branch and author; closed by the 0.2.0-beta tracker reset, and GitHub would not reopen it after the rebase force-push). Fixes #3171. The prepare() path does not have this problem - its encode block calls `_vision_up()` inside the FIFO turn; the tool path (3709c15f, wrapped into `_turn` by 873a0d66) never got the wake, and the elastic idle stop made the missing wake fatal. Related: #2435 (the sibling: a start that fails on the prepare path), #2343 (MCP picture results will send more traffic through this exact path). Independent of the #2455 successor (its `cached()` skip lands in this same method later - textual overlap only). #3174 is a parallel PR with the core wake alone; this one is the same fix plus the two sides that adding a wake to this path brings.

## Approach
- `serve/server.py`: inside the tool path's turn, `_vision_up()` before the encode (the caller holds the FIFO, as prepare does); the turn's end updates `last_image_at` in a `finally` - without it the idle sweep judges the just-started encoder by the previous picture's timestamp and reclaims it at once, and a tool-picture-only conversation never moves the timer; `start()`'s `RuntimeError` joins the filter, so a failing start becomes the documented note, not a dead request.
- `serve/test_vision_elastic.py`: two cases - a tool picture wakes a stopped encoder, and the turn holds it across the idle check; a start that raises becomes the note.
- Compatibility: the widened filter matches what can still raise (`encode()`'s own pipe death already becomes ValueError/OSError internally - #1317's read timeout raises ValueError, a write to a dead pipe is OSError - and `ClientGone` is a bare Exception, so a client that left still propagates). No native files touched.

## Validation

**Passed:**
- Both new cases fail on the base tree at the fake's "a picture reached an encoder that was not started" assert (the bug's exact signature), and a real `Vision` with the proc unloaded raises the `AttributeError` verbatim.
- `python -m unittest serve.test_vision_elastic serve.test_vision_busy` (CPU-only, no engine): 13 OK; `python -m unittest serve.test_images`: 19 OK - at `e88ee39d` (v0.2.0-beta-12), rebased 2026-10-20, Windows 11 26200, Python 3.12 venv. On the previous base `f57c0adc` the same files were 9/9 + 32 OK.
- With #2455 on top (cherry-picked, auto-merge clean): the three files stay 19 + 19 OK - the two PRs conflict in neither order.

**Failed:** none observed.

**Not run:** CUDA/HIP/SYCL builds (no native files touched); the live GPU end-to-end is #3171's reporter's log, reproduced here by the two CPU tests' fail-on-base/pass-after signature.

## Limits
- The wake reuses the same FIFO turn the encode already runs in, so a tool picture now costs the encoder start (~seconds, logged) on an idle-stopped encoder - the same cost the user-picture path already pays; it is what the note path cannot afford to skip.
- No native files touched; nothing here changes non-elastic behavior.

Related on this site

Quick links to related pages.