[#169] Fix for stripping vision-tags that were sent by clients withou… - #170
Merged
Merged
Conversation
…ients without any images. * New _strip_stray_vision_tokens(prompt, ov_images) — if no image was provided and the prompt text contains the model's vision token, strip it (idempotent, logs a warning). This satisfies the invariant: 0 tags ↔ 0 images. * Applied in _resolve_prompt_and_images — the single choke point, so it covers all input paths (messages, raw prompt, and bench input_ids) for both streaming and non-streaming. Real image requests are untouched (prepare_inputs already pairs each image with exactly one tag). _run_generation enqueues an EOF sentinel on failure (except: streamer.text_queue.put_nowait(None); raise) so the drain loop always terminates, even when end() isn't called. (except Exception deliberately does not catch CancelledError, so cancellation is unaffected.) * Moved result = await gen_task out of finally into the try after the drain loop, and emit metrics only on the happy path — so a generation error now propagates to the caller → HTTP 500 instead of a silent hang.
Owner
|
@ecky-l this was a great find lol, great work. I will test this out over the weekend |
Updated warning message to clarify the reason for stripping tokens.
Owner
|
@ecky-l made a small update to the logged error with a reference to this PR. Excellent work finding this edge case! |
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
2 participants
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.
…t any images.
New _strip_stray_vision_tokens(prompt, ov_images) — if no image was provided and the prompt text contains the model's vision token, strip it (idempotent, logs a warning). This satisfies the invariant: 0 tags ↔ 0 images.
Applied in _resolve_prompt_and_images — the single choke point, so it covers all input paths (messages, raw prompt, and bench input_ids) for both streaming and non-streaming. Real image requests are untouched (prepare_inputs already pairs each image with exactly one tag). _run_generation enqueues an EOF sentinel on failure (except: streamer.text_queue.put_nowait(None); raise) so the drain loop always terminates, even when end() isn't called. (except Exception deliberately does not catch CancelledError, so cancellation is unaffected.)
Moved result = await gen_task out of finally into the try after the drain loop, and emit metrics only on the happy path — so a generation error now propagates to the caller → HTTP 500 instead of a silent hang.