Repository navigation
[Other] serve: a tool's picture starts the elastic encoder, and its turn counts - #3379
Open
jerry78424 wants to merge 1 commit into
Open
jerry78424 wants to merge 1 commit into
jerry78424 wants to merge 1 commit into
Conversation
_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.
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
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.
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_swithout a picture, default 120 s), the tool's picture writes to a pipe that no longer exists: theAttributeErroron theNoneproc escapes theexcept (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 atoolmessage whose content carries animageblock (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'(insideVision.encode'sself.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 (3709c15, wrapped into_turnby 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 the #2455 successor (itscached()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 updateslast_image_atin afinally- 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()'sRuntimeErrorjoins 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.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 - andClientGoneis a bare Exception, so a client that left still propagates). No native files touched.Validation
Passed:
Visionwith the proc unloaded raises theAttributeErrorverbatim.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 - ate88ee39d(v0.2.0-beta-12), rebased 2026-10-20, Windows 11 26200, Python 3.12 venv. On the previous basef57c0adcthe same files were 9/9 + 32 OK.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