Repository navigation
[Bugfix] Stop reading the removed OmniRequestOutput.request_output accessor - #6172
Merged
hsliuustc0106 merged 1 commit intoAug 14, 2026
Conversation
…cessor vllm-project#5146 made OmniRequestOutput inherit RequestOutput instead of holding one in a nested request_output field. The field is gone, but 21 call sites across docs, examples, tests and tools still read it, so three nightly CUDA jobs fail deterministically: AttributeError: 'OmniRequestOutput' object has no attribute 'request_output' Read the generation content off the output object itself. Two call sites had guarded the access with hasattr, so instead of the AttributeError they now take a branch that was never meant to run: the Flux2 VAE patch-parallel helper always raises ValueError("No request_output found in OmniRequestOutput"), and the H3 quantization-quality helper silently skips its worker peak-memory update. Both are dropped along with the accessor. The nested field only fails once a real model has produced an output, which is why a GPU nightly job was the first thing to notice. Add a CPU test in tests/docs that pins both halves of the contract - the field is really gone, and no doc, example, test or tool still reaches for it - so the next removal is caught before nightly. Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
MrlixiangWE
requested review from
Gaohan123,
NickCao,
david6666666,
hsliuustc0106,
yenuo26 and
ywang96
as code owners
August 13, 2026 17:39
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to be related to model: minicpm. Model owners: @y-null Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
khairulkabir1661
pushed a commit
to khairulkabir1661/vllm-omni
that referenced
this pull request
Sep 25, 2026
…cessor (vllm-project#6172) Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
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.
Purpose
Fixes #6133.
OmniRequestOutputused to hold a nestedRequestOutputin arequest_outputfield. #5146 madeit inherit
RequestOutput, so the generation content lives on the object itself and the nestedfield no longer exists. 21 call sites across 12 files still read it, and three nightly CUDA jobs
fail deterministically:
This PR routes every one of them at the output object directly (
outputs[0].imagesrather thanoutputs[0].request_output.images).Two things worth flagging that are not in the issue:
The blast radius is wider than the three jobs.
examples/offline_inference/image_to_video/README.md:99sits inside a python snippet that
tests/examples/offline_inference/test_image_to_video.pycollects and runs, so the image-to-video Doc Test fails the same way.
Two call sites fail differently, which is why the log-scraped issue did not group them. They
guarded the access with
hasattr, so instead of theAttributeErrorthey take a branch that wasnever meant to run:
tests/diffusion/distributed/test_flux2_vae_patch_parallel.pynow alwaysraises
ValueError("No request_output found in OmniRequestOutput"), andtests/diffusion/models/minimax_h3/test_minimax_h3_quantization_quality.pysilently skips itsworker peak-memory update. Both guards are dropped along with the accessor.
One correction to the issue's list:
more_cli_examples_003is the bash NextStep-1.1 snippet.Neither it, nor
text_to_image.py, nordiffusion/utils/image_output.pyreads the removedaccessor — #5146 already updated those — so that case is not addressed here and, if it is
still failing, it has another cause.
tests/e2e/accuracy/ltx/run_ltx_reference.pyandtests/diffusion/models/minimax_h3/test_minimax_h3_quantization_quality.pylose a few linesrather than gaining a rename: their nested-unwrap blocks became unreachable when the nesting went
away, and the code immediately around them already reads the content off the outer object.
Regression test
tests/docs/test_example_output_accessors.py(new,core_model+cpu, following thetests/docs/test_generate_examples.pyidiom) pins both halves of the contract: the field really isgone from the class, and no doc, example, test or tool reads it. The nested field only fails once a
real model has produced an output, which is why a GPU nightly job was the first thing to notice;
this turns that into a millisecond CPU check. #5146 is part of a series, so the next removal should
not have to wait for nightly either.
Test Plan
vLLM Version: 0.27.0
vLLM-Omni Commit: dbc0dd6 (upstream main)
Test Result
The accessor is gone and the replacement is the right one, against an output built the way
diffusion/output_formatter.pybuilds it:The snippets the Doc Test collects, read back through
ReadmeSnippet.extract_readme_snippets:The new test is red on unmodified main and green here:
Adjacent suites and collection:
pre-commit on the 13 changed files: all hooks pass (
actionlintskipped, no workflow filestouched).
Run on one A800 against upstream main
dbc0dd6d, vLLM 0.27.0. The three failing nightly jobsthemselves need the CUDA runners and are covered by the Diffusion X2I Doc Test, X2V Other Accuracy
Test and Distributed Test with H100 jobs.