Repository navigation
[Core] Add token_ids output type and enforce canonical names - #7843
Conversation
360a61e to
3879c59
Compare
4aeb622 to
bbf1c2a
Compare
Make token_ids an independent output type for discrete vocabulary/codebook indices and enforce canonical names while preserving None defaults and canonical combinations. Wire default output naming and DELTA retention. Keep uniform producer-key normalization without a token_ids-specific hidden exception. Preserve GLM-Image's cumulative and source-image ID handoff fields, and document auxiliary payload naming and runner limitations. Signed-off-by: Jungle430 <junglece430@gmail.com> Co-authored-by: Codex <noreply@openai.com>
Cover canonical parsing, uniform producer-key naming, explicit auxiliary fields, dim-0 accumulation, per-key overrides, and DELTA retention. Verify config projection and startup rejection/cleanup. Check synthetic GLM-Image T2I/I2I handoff with and without generic hidden payloads, keeping cumulative generated IDs and source-image conditioning independent of the normalized auxiliary field. Signed-off-by: Jungle430 <junglece430@gmail.com> Co-authored-by: Codex <noreply@openai.com>
bbf1c2a to
bbfd7fc
Compare
|
This PR appears to belong to: docs/design/module/entrypoints.md, docs/design/module/input_output_modality_contracts.md. Module owners: @alex-jw-brooks @linyueqian @NickCao Routing: @alex-jw-brooks via module of the changed files, semantic router; @linyueqian via module of the changed files; @NickCao via module of the changed files @Jungle430, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
|
I reviewed the diff, focusing on:
Validation on the current commit End-to-end validation covers eager execution on a single NVIDIA H100. Coverage limits for other paths are documented in the PR, alongside result images and reproduction code. cc @tzhouam |
Omni ReviewBot routing recordAssigned Direct under experiment |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
PR description
This change makes engine_output_type accept only the lowercase names text, image, audio, latent, and token_ids, and it treats token_ids as its own modality instead of a text alias. Empty or omitted values stay distinct: None still defaults to text, while an explicit empty string is kept in stage engine args and fails when the LLM output processor is built. Generic producer keys such as model_outputs and hidden are renamed to the declared type, including token_ids, and those tensors stay accumulated across DELTA steps; GLM-Image’s AR-to-diffusion bridge still reads generated IDs from cumulative_token_ids and source-image IDs from ids.prior_image.
Change flow
flowchart TD
A["[EXISTING] StagePipelineConfig.engine_output_type"]:::existing
B["[CHANGED] _build_engine_args keeps empty strings"]:::changed
C["[REMOVED] _MODALITY_ALIASES"]:::removed
D["[CHANGED] OutputModality.from_string canonical parse"]:::changed
E["[NEW] TOKEN_IDS type and DELTA retention"]:::new
F["[CHANGED] MultimodalOutputProcessor default keys"]:::changed
G["[EXISTING] GLM-Image ar2diffusion reads cumulative_token_ids"]:::existing
A --> B --> D
C --> D
D --> E --> F --> G
classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px
CI at
bbfd7fc54b63(2026-09-28T02:33:59.379129+00:00): required check(s) blocking:buildkite/vllm-omni(missing), and -5 more.
No actionable findings.
|
Reviewed at
With the aliases removed, What I checked and found fine: no |
bbfd7fc to
65df1d6
Compare
Replace the removed latents alias with latent and include token_ids in the OmniModelConfig and OmniEngineArgs output-type docstrings. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Jungle430 <junglece430@gmail.com>
65df1d6 to
ad7a940
Compare
|
@tzhouam Fixed both docstrings in |
Merge main at e34f8c4 while preserving all existing PR commits. Include the upstream NPU vLLM 0.30.0 upgrade (vllm-project#8027) and inline diffusion worker cleanup fix (vllm-project#7986). Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Jungle430 <junglece430@gmail.com>
|
@tzhouam The GitHub Actions checks have passed after merging the latest main. Need to trigger the Buildkite pipelines. |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
PR description
This PR adds a first-class TOKEN_IDS output modality and stops treating token_ids as a TEXT alias, so discrete image/codebook IDs get their own payload key and retention rules. Parsing now accepts only exact lowercase canonical names (text, image, audio, latent, token_ids), including +/, compounds, while rejecting legacy aliases, case variants, whitespace, and empty components. Explicit empty engine_output_type values are preserved through stage engine-args so invalid names fail when the LLM output processor is built, with replica cleanup covered by tests. GLM-Image keeps reading generated IDs from cumulative_token_ids and source-image IDs from ids.prior_image; the AR→Diffusion bridge is unchanged.
Change flow
flowchart TD
A["[EXISTING] Stage engine_output_type<br/>pipeline / deploy config"]:::existing --> B["[CHANGED] _build_engine_args<br/>preserve explicit empty values"]:::changed
B --> C["[CHANGED] OutputModality.from_string<br/>canonical names only; aliases removed"]:::changed
C --> D["[NEW] OutputModality.TOKEN_IDS<br/>default key + CONCAT_DIM0 + non-drainable"]:::new
D --> E["[CHANGED] MultimodalOutputProcessor<br/>naming / DELTA retention"]:::changed
E --> F["[EXISTING] GLM-Image ar2diffusion<br/>cumulative_token_ids + ids.prior_image"]:::existing
classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px
CI at
536de30c1679(2026-09-30T22:49:52.052671+00:00): all required and operator-watched checks reported green. Observed Buildkite:buildkite/vllm-omni(passed),buildkite/vllm-omni-amd-ci(failed),buildkite/vllm-omni-npu-ci(failed), and 1 more.
No actionable findings.
🤖 This review was generated by InferMatrix Copilot, an open-source repo-maintenance agent for PR review, CI debugging and issue triage. Try it on your own repo, and ⭐ star it if it helped!
|
@Jungle430 this PR is labeled Could you please take a look and push an update to get CI green? Once the checks pass we can proceed with review/merge. Thanks! |
Merge main at b171731 without rewriting existing PR commits. Retain canonical output-type validation and TOKEN_IDS semantics while preserving upstream per-key accumulation defaults for prompt conditioning. Codex assisted with conflict resolution and validation. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Jungle430 <junglece430@gmail.com>
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
PR description
This PR makes engine_output_type accept only canonical lowercase names and adds a real TOKEN_IDS modality instead of treating token_ids as a text alias. Invalid or legacy names now fail when the LLM output processor is built at stage init, while explicit empty strings are preserved so that failure still happens there and replicas are shut down. GLM-Image keeps emitting generated IDs through cumulative_token_ids / ids.prior_image; generic model_outputs/hidden fields simply rename to the declared default key, including token_ids.
Change flow
flowchart TD
A["[EXISTING] Stage engine_output_type<br/>pipeline / deploy / engine args"]:::existing --> B["[CHANGED] _build_engine_args<br/>preserve explicit empty values"]:::changed
B --> C["[CHANGED] OutputModality.from_string<br/>canonical names only"]:::changed
C --> D["[NEW] TOKEN_IDS modality<br/>naming + DELTA retention"]:::new
D --> E["[CHANGED] MultimodalOutputProcessor<br/>default key + accumulate"]:::changed
E --> F["[EXISTING] GLM-Image ar2diffusion<br/>cumulative_token_ids / prior_image"]:::existing
classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px
CI at
1680ae2c747f(2026-10-04T03:36:20.495849+00:00): required check(s) blocking:pre-commit(pending),buildkite/vllm-omni(missing),docs/readthedocs.org:vllm-omni(pending).
No actionable findings.
🤖 This review was generated by InferMatrix Copilot, an open-source repo-maintenance agent for PR review, CI debugging and issue triage. Try it on your own repo, and ⭐ star it if it helped!
|
@hsliuustc0106 Thanks for checking. I’ve merged the latest main and resolved the conflicts in The linked failed builds ran on the previous commit. Could you please trigger the Buildkite pipelines on the updated commit so we can verify whether any failures remain? The GitHub Actions checks have passed. cc @tzhouam |
|
@tzhouam need to trigger buildkite NPU ci again, pass it then merge. |




Purpose
Related to #7841.
GLM-Image’s AR stage produces discrete image token IDs, but the current alias mapping parses
token_idsasTEXT. This PR introduces an independentTOKEN_IDStype and enforces canonical output-type names._MODALITY_ALIASES; accept onlytext,image,audio,latent, andtoken_ids. PreserveNonedefaults and+/,combinations. Reject legacy aliases, unknown names, case variants, whitespace, and empty components.model_outputsandhiddenfollow the shared default-key naming rule, without atoken_ids-specific exception. Keep dim-0 accumulation and the precedence of existing per-key strategies.cumulative_token_ids, and source-image IDs remain inids.prior_image; noar2diffusion()changes are needed.Existing
latentmodels are unchanged. Normalization only renames fields; it does not convert or validate tensor values. Independent auxiliary outputs need distinct, explicit keys. A richer per-field contract for modality, representation, and update rules is follow-up work.Test Plan
hiddenis normalized totoken_idswithout changing its tensor values. GLM-Image’s bridge ignores this auxiliary field and reads generated IDs fromcumulative_token_ids. The default GPU path omits auxiliary hidden output; NPU and some optional runner/cache paths may still emit it and require separate validation.Pytest Validation Report
Tested commit:
bbfd7fc54. Test date: 2026-09-23.Environment
core_model and cpumarkersTest Results
core_model and cpu; canonical parsing, enum/name routing, config propagation, startup validation/cleanup, payload normalization, accumulation, DELTA retention, and GLM-Image T2I/I2I handoff.qwen3_omni_moepipeline registry case requires an HF config to resolve.This is the focused suite for this change. The 19 warnings comprise 14 PyTorch deprecations, one vLLM/vLLM-Omni development-version warning, three deprecated diffusion-config warnings, and one FlashInfer/Cutlass deprecation.
Pytest command
python -m pytest -vv -ra -m "core_model and cpu" --run-level=core_model \ tests/engine/test_output_modality.py \ tests/engine/test_output_processor.py \ tests/engine/test_stage_metadata.py \ tests/engine/test_stage_engine_args.py \ tests/engine/test_async_omni_engine_stage_init.py \ tests/model_executor/stage_input_processors/test_glm_image.pyGLM-Image Regression Report
PR commit:
bbfd7fc54. Baseline:59db6a428. Test date: 2026-09-23.Environment
GLM-Image regression setup
zai-org/GLM-Image2c433cc0cbc293bde2ac8ca9624f279b5d23fcf4async_chunk=false.gpu_memory_utilization=0.5,max_model_len=32768,max_num_seqs=1.guidance_scale=1.5;seed=42.POST /v1/chat/completionsEnd-to-end results
All four runs exited with code 0 and returned a decodable 1024 × 1024 PNG. Returned image payloads match the saved PNG files. File bytes, decoded RGB pixels and SHA-256 hashes were independently checked from the archived artifacts. The timings are individual request measurements, not a performance benchmark.
Image comparison
T2I — text-to-image:
59db6a428)bbfd7fc54)I2I — the same source image was used for both runs:
59db6a428)bbfd7fc54)The comparisons above use the original archived PNGs. Each PR/base pair is byte-for-byte identical; the maximum decoded RGB pixel difference is 0.
Reproduce the fixed-input NVIDIA comparison (inline code)
Use a clean, dedicated checkout containing both tested commits and an editable vLLM-Omni installation. Activate the environment with the versions listed above; the client also uses
requestsand Pillow. Run both commits in the same environment on the same GPU, restarting the service between them.This condensed client sends the same request JSON as the archived test scripts. The reported timings come from the original runs. No evidence archive is required to use the code below.
1. Prepare the pinned model, fixed input and deployment config. Run from the repository root; adjust the model directory as needed. This YAML retains the tested NVIDIA settings and omits unused NPU overrides.
2. Save the client below. It checks the input hash, sends both requests, and saves requests, responses and PNGs.
3. Start the PR server in the first terminal.
After
Application startup complete, activate the same environment in a second terminal and run:4. Repeat on the baseline. Stop the PR server with Ctrl+C in the first terminal and wait for its GPU workers to exit. Keep the environment and configuration unchanged, then run there:
After startup completes, run in the second terminal:
python "$GLM_REG_DIR/client.py" base5. Compare the requests and outputs. Run in the second terminal:
The recorded runs passed all comparisons for both cases. These checks establish PR/base parity for the fixed inputs; they do not measure general image quality or performance.
AR → Diffusion handoff
Both commits recorded the same handoff values:
prior_image_ids=yes)Scope and log notes
async_chunk=false,stream=false. One fixed input and seed per case.hiddenpayloadBoth versions logged the same
processor/model_typeconfiguration-probing errors during startup, then completed startup, inference and shutdown successfully. Neither server log contains a traceback or OOM.Other Validation