Skip to content

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

Closed
jerry78424 wants to merge 1 commit into
Niko1221:mainfrom
jerry78424:tool-picture-vision-up
Closed

jerry78424 wants to merge 1 commit into
Niko1221:mainfrom
jerry78424:tool-picture-vision-up

Conversation

@jerry78424

@jerry78424 jerry78424 commented Oct 15, 2026 •

Copy link
Copy Markdown

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 3709c15 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

Fixes #3171. The prepare() path does not have this problem - its encode block calls _vision_up() inside the FIFO turn; the tool path (3709c15, wrapped into _turn by 873a0d6) 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 #2455 (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 - serve: an untimed read of the engine/vision pipe can hold the request turn forever (intermittent hang; the #481 lost-step class) #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 [Other] serve: an all-cached image turn never wakes the elastic encoder #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.

_note_unreadable_tool_images calls vision.encode() without _vision_up():
once the elastic idle sweep stopped the encoder (elastic_idle_s without
a picture - Vision.unload() nulls the proc, the cache on disk stays),
the next picture a tool returned took the turn and wrote to a pipe that
was no longer there. The AttributeError on the None proc escapes the
(ValueError, OSError) filter and the route's shape filter (:5551), so
the request dies as a 400 "malformed" instead of the note this method
exists to write - and because clients re-send the history, every later
request of the conversation dies the same way at this method's own line,
before the user-picture encode below could start the encoder: the
conversation stays broken until some other conversation's picture (or a
restart) starts the encoder.

The fix mirrors what prepare() does for its own pictures: the encoder
starts inside the turn (_vision_up, the caller holds the FIFO), and the
turn's end updates last_image_at - without it the 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 at
all. start()'s own failure (RuntimeError, the three raises around the
READY read) joins the filter: it degrades to the documented note, it is
not a reason to kill the request.

Checked against the code: encode()'s own pipe death already becomes
ValueError/OSError internally (the Niko1221#1317 path raises ValueError, a write
to a dead pipe is OSError), so the widened tuple matches what can still
raise; ClientGone (a bare Exception, :312) passes through untouched;
_vision_up starts what it found down, so the None-proc AttributeError
cannot happen after it. Two cases in serve/test_vision_elastic.py pin
the behavior; on the base tree both fail at the fake's "a picture
reached an encoder that was not started" assert - the bug's exact
signature, and a real Vision with proc=None raises the AttributeError
verbatim; the file passes 9/9, and test_vision_busy / test_images with
it - 32 OK. Not measured against a live encoder. No native files
touched.
@Niko1221

Copy link
Copy Markdown
Owner

Thanks for the pull request.

We're resetting the tracker for 0.2.0-beta. There are more than 1,300 open issues and pull requests, too many for us to review properly, so we're closing everything that isn't being worked on right now. Closed here does not mean rejected.

To reopen it takes about two minutes:

  1. Rebase on main if it no longer applies, and re-run your checks on 0.2.0-beta.
  2. Start the title with its area: [Engine], [GUI], [TUI], [Benchmark], [Docs] or [Other].
  3. Fill in the pull request template: the problem and result, and the validation (what you ran, on which revision, what you saw).
  4. Comment /reopen. A check runs and either reopens the pull request or lists what is still missing.

The rules are in CONTRIBUTING.md.

@Niko1221 Niko1221 closed this Oct 19, 2026
@jerry78424 jerry78424 changed the title serve: a tool's picture starts the elastic encoder, and its turn counts [Other] serve: a tool's picture starts the elastic encoder, and its turn counts Oct 20, 2026
@jerry78424

Copy link
Copy Markdown
Author

/reopen

@github-actions

Copy link
Copy Markdown

The check passed, but GitHub would not reopen it (Validation Failed: ). If the branch was deleted, restore it or open a new pull request and link this one.

@jerry78424

Copy link
Copy Markdown
Author

Reopened as #3379 (GitHub could not reopen this one after the rebase force-push; same branch, checks re-run on e88ee39).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: elastic vision — a tool-returned image crashes the request (400 "malformed", AttributeError on a None encoder proc)

2 participants