pytest -s -v tests/e2e/offline_inference/test_wan22_t2v.py tests/e2e/online_serving/test_wan22_t2v.py -m 'advanced_model and cuda and diffusion' --run-level 'advanced_model'
"
## ISSUE depends on `diffusers` package: https://github.com/huggingface/diffusers/issues/13274
# - label: "Audio Generation Model Test"
# agent_pool: mi300_1
# depends_on: amd-build
# mirror_hardwares: [amdproduction]
# grade: Blocking
# commands:
# - export GPU_ARCHS=gfx942
# - export VLLM_LOGGING_LEVEL=DEBUG
# - export VLLM_WORKER_MULTIPROC_METHOD=spawn
# - timeout 20m pytest -s -v tests/e2e/offline_inference/test_stable_audio_expansion.py -m "advanced_model and diffusion and L4" --run-level advanced_model
pytest -s -v tests/e2e/online_serving/test_qwen_image.py -m 'advanced_model and cuda' --run-level 'advanced_model'
'
mirror_hardwares:h100_1
- label:"Diffusion · Wan22 Test"
source_file_dependencies:
- tests/e2e/offline_inference/test_wan22_t2v.py
- tests/e2e/online_serving/test_wan22_t2v.py
- vllm_omni/diffusion/models/wan2_2/
- vllm_omni/diffusion/models/dmd2/
commands:
- |
timeout 40m bash -c "
export VLLM_IMAGE_FETCH_TIMEOUT=60
pytest -s -v tests/e2e/offline_inference/test_wan22_t2v.py tests/e2e/online_serving/test_wan22_t2v.py -m 'advanced_model and cuda and diffusion' --run-level 'advanced_model'
vault.hashicorp.com/agent-inject-template-ca.pem:"{{- with secret \"internal/data/ascend/buildkitd\" -}}\n{{ .Data.data.RootCA }}\n{{- end }}"
vault.hashicorp.com/agent-inject-template-cert.pem:"{{- with secret \"internal/data/ascend/buildkitd\" -}}\n{{ .Data.data.ClientCaCert }}\n{{- end }}"
vault.hashicorp.com/agent-inject-template-config.json:"{{- with secret \"internal/data/ascend/buildkitd\" -}}\n{{ .Data.data.dockerConfig }}\n{{- end }}"
vault.hashicorp.com/agent-inject-template-key.pem:"{{- with secret \"internal/data/ascend/buildkitd\" -}}\n{{ .Data.data.ClientCaKey }}\n{{- end }}"
vault.hashicorp.com/agent-inject-template-ca.pem:"{{- with secret \"internal/data/ascend/buildkitd\" -}}\n{{ .Data.data.RootCA }}\n{{- end }}"
vault.hashicorp.com/agent-inject-template-cert.pem:"{{- with secret \"internal/data/ascend/buildkitd\" -}}\n{{ .Data.data.ClientCaCert }}\n{{- end }}"
vault.hashicorp.com/agent-inject-template-config.json:"{{- with secret \"internal/data/ascend/buildkitd\" -}}\n{{ .Data.data.dockerConfig }}\n{{- end }}"
vault.hashicorp.com/agent-inject-template-key.pem:"{{- with secret \"internal/data/ascend/buildkitd\" -}}\n{{ .Data.data.ClientCaKey }}\n{{- end }}"
- pytest -s -v tests/e2e/offline_inference/test_minicpmo_4_5.py tests/e2e/online_serving/test_minicpmo_4_5.py -m 'core_model and npu and A3 and omni' --run-level 'core_model'
- label:"Omni · MiniCPM-o 4.5 Duplex Test"
mirror_hardwares:a3_npu_2
env:
VLLM_WORKER_MULTIPROC_METHOD:spawn
VLLM_ALLOW_LONG_MAX_MODEL_LEN:"1"
commands:
- pytest -s -v tests/e2e/online_serving/test_minicpmo_4_5_duplex.py -m 'core_model and npu and A3 and omni' --run-level 'core_model'
- label:"Omni · MiniCPM-o 4.5 Perf Test"
timeout_in_minutes:60
mirror_hardwares:a3_npu_2
env:
VLLM_WORKER_MULTIPROC_METHOD:spawn
VLLM_ALLOW_LONG_MAX_MODEL_LEN:"1"
commands:
- export BENCHMARK_DIR=tests/dfx/perf/results
- pytest -s -v tests/dfx/perf/scripts/run_benchmark.py --test-config-file tests/dfx/perf/tests/test_minicpmo_4_5_ready.json -m 'core_model and npu and A3 and omni'
# detect version from any wheel in the commit directory
first_wheel_key=$($PYTHON -c "import json; obj=json.load(open('$obj_json')); print(next((c['Key'] for c in obj.get('Contents', []) if c['Key'].endswith('.whl')), ''))")
description:Add a new diffusion model (text-to-image, text-to-video, image-to-video, text-to-audio, image editing) to vLLM-Omni, including Cache-DiT acceleration and parallelism support (TP, SP/USP, CFG-Parallel, HSDP). Use when integrating a new diffusion model, porting a diffusers pipeline or a custom model repo to vllm-omni, creating a new DiT transformer adapter, adding diffusion model support, or enabling multi-GPU parallelism and cache acceleration for an existing model.
---
# Adding a Diffusion Model to vLLM-Omni
## Overview
This skill guides you through adding a new diffusion model to vLLM-Omni. The model may come from HuggingFace Diffusers (structured pipeline) or from a private/custom repo. The workflow differs significantly depending on the source.
## Prerequisites
Before starting, determine:
1.**Model category**: Text-to-Image, Text-to-Video, Image-to-Video, Image Editing, Text-to-Audio, or Omni
2.**Reference source**: Diffusers pipeline, custom repo, or a combination
3.**Model HuggingFace ID** or local checkpoint path
4.**Architecture**: Scheduler, text encoder, VAE, transformer/backbone
## Step 0: Classify the Migration Path
Check the model's HF repo for `model_index.json`. This determines your path:
| Scenario | How to identify | Migration path |
|----------|----------------|----------------|
| **Already supported** | `_class_name` in `model_index.json` matches a key in `_DIFFUSION_MODELS` in `registry.py` | Skip implementation, then validate model-specific examples, tests, and docs as needed |
| **Diffusers-based** | Has standard `model_index.json` with `_diffusers_version`, subfolders for `transformer/`, `vae/`, etc. | Follow **Path A** below |
| **Custom/private repo** | No diffusers `model_index.json`, weights in non-standard format, custom model code in a separate git repo | Follow **Path B** below |
| **Hybrid** | Has some diffusers components (VAE) but custom transformer/fusion | Mix of Path A and Path B |
## Path A: Diffusers-Based Model
For models with a standard diffusers layout. See [references/transformer-adaptation.md](references/transformer-adaptation.md) for detailed code patterns.
5. Add class attributes for acceleration features such as `_repeated_blocks` and `_layerwise_offload_blocks_attrs` (see [references/transformer-adaptation.md](references/transformer-adaptation.md) for examples).
For models without a diffusers pipeline — weights in custom format, model code in a private repo. Real examples: DreamID-Omni, BAGEL, HunyuanImage3. See [references/custom-model-patterns.md](references/custom-model-patterns.md) for concrete custom-model integration patterns.
| **Custom transformer/backbone** (ported and adapted to vllm-omni) | `vllm_omni/diffusion/models/<name>/<name>_transformer.py` or similar | `wan2_2.py`, `fusion.py`, `bagel_transformer.py` |
| **Custom sub-models** (VAE, fusion, autoencoder) | `vllm_omni/diffusion/models/<name>/` as separate files | `autoencoder.py`, `fusion.py` |
| **External dependency code** (original repo utilities) | **External repo**, installed via download script or pip | `dreamid_omni` package via git clone |
| **Hardcoded model configs** | Module-level dicts in pipeline file | `VIDEO_CONFIG`, `AUDIO_CONFIG` dicts |
"Failed to import from dependency 'external_model'. "
"Please run the download script first."
)
```
**Option 2: Port the code directly** (preferred when feasible)
Copy the essential model files into `vllm_omni/diffusion/models/<name>/` and adapt them. This avoids external dependencies. BAGEL does this — `autoencoder.py` and `bagel_transformer.py` are ported directly.
**Decision criteria**: Port if the code is self-contained and won't diverge. Use external deps if the model repo is actively maintained and the code is complex.
### B4. Handle custom weight loading
Custom models have two common patterns for weight loading:
**Pattern 1: Bypass standard loader** (DreamID-Omni style)
When the original model has complex custom init functions that load weights in `__init__`:
```python
classCustomPipeline(nn.Module):
def__init__(self,*,od_config,prefix=""):
super().__init__()
model=od_config.model
# Load everything eagerly in __init__ using custom helpers
Custom models need a `model_index.json` at the model root for vllm-omni to discover them. For custom models, this is minimal:
```json
{
"_class_name":"YourModelPipeline",
"custom_key":"path/to/custom_weights.safetensors"
}
```
The `_class_name` must match a key in `_DIFFUSION_MODELS` in `registry.py`. Additional keys are model-specific (accessed via `od_config.model_config`).
If the model's weights come from multiple HF repos, write a **download script** that:
1. Downloads from each repo
2. Assembles into a single directory
3. Generates `model_index.json`
4. Installs any external dependencies (git clone + `.pth` file)
Place at: `examples/offline_inference/<name>/download_<name>.py`
### B6. Handle multi-modal inputs
If the model accepts images, audio, or other multi-modal inputs, implement the protocol classes from `vllm_omni/diffusion/models/interface.py`:
# Protocol markers — the engine uses these to enable proper input routing
pass
```
Preprocessing for custom models is typically done **inside `forward()`** rather than via registered pre-process functions, since the logic is often tightly coupled to the model.
**Follow the [vllm-omni-test skill](../vllm-omni-test/SKILL.md)** for markers, file naming, Buildkite wiring, and run commands. Also read [l4_functionality_tests.inc.md](https://github.com/vllm-project/vllm-omni/blob/main/docs/contributing/ci/test_examples/l4_functionality_tests.inc.md), [test_system_overview.md](https://github.com/vllm-project/vllm-omni/blob/main/docs/contributing/ci/test_system_overview.md), and [test_writing_guide.md](https://github.com/vllm-project/vllm-omni/blob/main/docs/contributing/ci/test_writing_guide.md).
| **L2** | `tests/e2e/online_serving/test_{slug}.py` (and offline if the category is offline-first) | **`core_model` + `advanced_model`** (both on baseline smoke) + `diffusion` + `@hardware_test` / `hardware_marks` | `test-ready.yml` | Default deploy smoke — minimal `num_inference_steps`, single prompt |
| **L3** | `tests/e2e/online_serving/test_{slug}.py`**and**`tests/e2e/offline_inference/test_{slug}.py` when offline matters | Baseline smoke: **`core_model` + `advanced_model`**; heavier cases: `advanced_model` only (+ `diffusion`) | `test-merge.yml`**or** merged into nightly diffusion function job | Real weights, streaming/API paths, LoRA/offload smoke |
| **L4** | `tests/e2e/online_serving/test_{slug}_expansion.py` (+ offline expansion if needed) | `full_model` + `diffusion` | `test-nightly.yml` (X2I / X2V / X2A function groups) | Feature combos per [#1832](https://github.com/vllm-project/vllm-omni/issues/1832); perf → `tests/dfx/perf/tests/test_{model}_vllm_omni.json` with per-case **`mark`** (`hardware_marks` + `full_model` + `diffusion`) |
**L2 & L3 online — same file, dual marks on the baseline smoke:** The **first / simplest** case in `test_{slug}.py` (default deploy, minimal steps, single prompt) should carry **both**`@pytest.mark.core_model` and `@pytest.mark.advanced_model` on the **same** function so L2 (`test-ready.yml`) and L3 (`test-merge.yml`) share one smoke test. Heavier deploy variants or API paths in the same file use **`advanced_model` only**. When L3 moves to nightly, migrate those heavier cases into `test_{slug}_expansion.py` with `full_model` and remove the dedicated `test-merge.yml` job (see `test_longcat_image_expansion.py`, `test_qwen_image_expansion.py`).
**L4 design (high priority):** Combine multiple supported features (Cache-DiT, TP, USP, CFG, HSDP, CPU offload, quantization) into **few parametrized**`OmniServerParams` rows so each feature appears in at least one case without exploding GPU jobs. Shard single-GPU vs multi-GPU cases across the nightly X2I/X2V function steps (1-GPU vs `distributed_cuda`).
**L4 design (medium / low):** One or two parametrized rows covering the best quality/perf trade-off; skip perf JSON unless the model is high priority.
**Keep model-specific code inside test modules — not `tests/helpers/{slug}.py`:** deploy constants, prompts, sampling dicts, and inline `request_config` / `form_data` belong in each `test_{slug}.py` and `test_{slug}_expansion.py`. Do not add per-model files under `tests/helpers/`; reuse only repo-wide harness (`mark`, `media`, `runtime`, `stage_config`, `assertions`). **L2+ online/offline e2e:** reuse or add `send_*_request` in `tests/helpers/runtime.py` — tests call the handler, not raw `omni.generate` / HTTP. See [vllm-omni-test skill](../vllm-omni-test/SKILL.md) § **Runtime send helpers**.
### Step 9: Add Cache-DiT Acceleration
Cache-DiT accelerates inference by caching intermediate computation results across denoising steps. After your model is working correctly on a single GPU, add cache-dit support.
See [references/cache-dit-patterns.md](references/cache-dit-patterns.md) for detailed code patterns.
#### 9a. Determine your model type
| Model Type | Description | Action |
|------------|-------------|--------|
| **Standard single-transformer** | One transformer with one `ModuleList` of blocks | No code needed — `CacheDiTBackend` auto-detects via `enable_cache_for_dit()` |
| **Multi-block-list** | One transformer with multiple block lists (e.g., `transformer_blocks` + `single_transformer_blocks`) | Write custom enabler with `BlockAdapter` |
| **Dual-transformer** | Two transformers (e.g., high-noise + low-noise) | Write custom enabler with `BlockAdapter` wrapping both |
#### 9b. Standard models — verify automatic support
For standard single-transformer models, test directly:
```python
omni=Omni(
model="your-model-name",
cache_backend="cache_dit",
cache_config={
"Fn_compute_blocks":1,
"Bn_compute_blocks":0,
"max_warmup_steps":4,
}
)
```
Check logs for "Cache-dit enabled successfully on xxx". If it works, skip to Step 9e.
#### 9c. Custom architectures — write a custom enabler
For multi-block-list or dual-transformer models, write a custom enabler function:
After verifying correctness and implementing parallelism/caching, profile the model's performance to identify bottlenecks and ensure optimal execution.
See the [Profiling Single-Stage Diffusion](../../../docs/contributing/profiling.md#3-profiling-single-stage-diffusion) guide for detailed instructions on:
1. Using the PyTorch profiler (`profiler: "torch"`) to capture detailed CPU/CUDA traces.
2. Using Nsight Systems (`nsys`) with `profiler: "cuda"` for low-overhead CUDA traces.
3. Controlling profiling via `omni.start_profile()` and `omni.stop_profile()`.
Cache-DiT accelerates Diffusion Transformers by caching intermediate computation results across denoising steps. Adjacent steps produce similar features, so redundant computations can be skipped.
Three caching strategies:
- **DBCache**: Dynamic block-level caching — selectively computes or caches transformer blocks based on residual differences
- **TaylorSeer**: Calibration-based prediction using Taylor expansion to estimate block outputs
- **SCM** (Step Computation Masking): Dynamic step skipping based on configurable policies
**Typical speedup**: 1.5-2.5x depending on model and configuration.
| `Pattern_2` | `block(hidden_states, **kwargs)` → `(hidden_states, ...)` with different residual pattern | Wan2.2 blocks |
Inspect your block's `forward()` return type and residual connection pattern to choose the right one. See [Cache-DiT API Reference](https://cache-dit.readthedocs.io/en/latest/user_guide/CACHE_API/) for details.
1. Logs show "Cache-dit enabled successfully on xxx"
2. Performance: 1.5-2x speedup vs no cache
3. Quality: compare output with `cache_backend=None`
## Excluded Models
Models listed in `_NO_CACHE_ACCELERATION` in `vllm_omni/diffusion/registry.py` do not support cache-dit (e.g., `NextStep11Pipeline`, `StableDiffusionPipeline`). Check this set before attempting to enable cache-dit.
## Reference Implementations
| Model | Path | Notes |
|-------|------|-------|
| Standard DiT | `cachedit.backend::enable_cache_for_dit` | Default enabler, automatic |
├── pipeline_wan2_2.py # Pipeline: loads components via from_pretrained()
├── pipeline_wan2_2_i2v.py # Variant pipeline for image-to-video
└── wan2_2_transformer.py # Transformer: ported from diffusers, uses Attention layer
```
The transformer is loaded separately via `weights_sources` + `load_weights()`. Non-transformer components (VAE, text encoder) are loaded in `__init__` via `from_pretrained()`.
### Custom model with external deps (e.g., DreamID-Omni)
```
vllm_omni/diffusion/models/dreamid_omni/
├── __init__.py # Exports pipeline only
├── pipeline_dreamid_omni.py # Pipeline: loads ALL weights in __init__ via custom helpers
├── bagel_transformer.py # Full LLM backbone (Qwen2-MoT) ported into vllm-omni
└── autoencoder.py # Custom VAE ported from original repo
```
Model code is fully ported (no external dependency). Uses `weights_sources` and `load_weights()` with custom name remapping to handle non-diffusers safetensors format.
## Weight Loading Patterns
### Pattern 1: Standard diffusers flow (Wan2.2, Z-Image, FLUX)
The only **required** field is `_class_name` — it must match a key in `_DIFFUSION_MODELS` in `registry.py`. Other fields are model-specific and accessible via `od_config.model_config` dict.
Copy essential files from the original repo into `vllm_omni/diffusion/models/<name>/`. Adapt imports to use vllm-omni utilities. Benefits: no external dependency, no git clone step. Drawback: must maintain the ported code.
## Multi-Modal Input/Output Protocols
Custom models that handle images, audio, or video I/O should implement protocol classes:
Replace standard `nn.Linear` with vLLM's parallel linear layers. This is the most invasive change but provides direct VRAM savings and compute speedup.
### Layer replacement rules
| Pattern | vLLM Layer | When to Use |
|---------|-----------|-------------|
| Fan-out (first in FFN) | `ColumnParallelLinear` | Projection that splits output across ranks |
| Fan-in (second in FFN) | `RowParallelLinear` | Projection that gathers across ranks |
When RMSNorm sits between TP-sharded dimensions, use `DistributedRMSNorm` — it computes global RMS via all-reduce across TP ranks. See the Wan2.2 implementation for the pattern.
### TP Constraints
-`num_heads % tp_size == 0`
-`num_kv_heads % tp_size == 0`
- Use `self.to_qkv.num_heads` (local per-GPU count), not total heads, for split sizes
SP splits sequence tokens across GPUs using Ulysses (all-to-all) or Ring (P2P) communication. It is applied non-intrusively via the `_sp_plan` dict — no changes to `forward()` logic.
Use intrusive modification as a last resort — `_sp_plan` is preferred for maintainability.
### UAA Mode (Experimental)
`ulysses_mode="advanced_uaa"` handles arbitrary sequence lengths and head counts that aren't divisible by `ulysses_degree`. Uses variable all-to-all split sizes and temporary head padding.
### Combining SP methods
Ulysses and Ring can be combined: `ulysses_degree × ring_degree = total SP GPUs`.
```python
DiffusionParallelConfig(ulysses_degree=2,ring_degree=2)# 4 GPUs total
Auto-enables `--vae-use-tiling`. Uses `DistributedAutoencoderKLWan` or similar distributed VAE. Set `vae_patch_parallel_size` in `DiffusionParallelConfig`.
**Declaring the attention role.** Always pass a `role` string to `Attention(...)`. It is the lookup key users target with `--diffusion-attention-config.per_role.<role>.backend` to swap kernels per site without touching model code.
| Convention | When to use |
|---|---|
| `role="self"` | Q/K/V come from the same hidden state |
| `role="cross"` | K/V come from a separate `encoder_hidden_states`; pair with `skip_sequence_parallel=True` if K/V is replicated across SP ranks |
For multi-modal sites that don't fit `self` / `cross`, use a dot-namespaced role and provide `role_category` so user config can fall back to the generic category:
The transformer config values come from `model_index.json` → `config.json` in the transformer subfolder. The pipeline uses `get_transformer_config_kwargs(od_config.tf_model_config, TransformerClass)` to filter config keys to match the `__init__` signature.
### Weight Loading
The `load_weights` method receives an iterable of `(name, tensor)` from safetensors files, with the prefix (e.g., `"transformer."`) already stripped by the loader.
1.**Wrong latent normalization**: Check VAE expects latents scaled by `vae.config.scaling_factor`
2.**Wrong scheduler**: Using the wrong scheduler class or wrong `flow_shift`
3.**Missing CFG**: Some models require `guidance_scale > 1.0` with negative prompt
4.**Wrong timestep format**: Some schedulers expect float, others expect int/long
5.**Missing post-processing**: Raw VAE output may need denormalization
**Quick test**: Run with diffusers directly using the same seed and compare latents at each step.
### OOM (Out of Memory)
**Solutions** (in order of preference):
1.`--enforce-eager` to disable torch.compile (saves compile memory)
2.`--enable-cpu-offload` for model-level offload
3.`--enable-layerwise-offload` for block-level offload (better for large models)
4.`--vae-use-slicing --vae-use-tiling` for VAE memory reduction
5. Reduce resolution: `--height 480 --width 832`
6. Use TP: `--tensor-parallel-size 2`
### Different Output vs Diffusers Reference
**Common causes**:
1.**Attention backend difference**: FlashAttention vs SDPA may produce slightly different results. Force SDPA via `--diffusion-attention-backend TORCH_SDPA` (or `DIFFUSION_ATTENTION_BACKEND=TORCH_SDPA`) to match diffusers; for finer-grained per-role overrides see `--diffusion-attention-config.per_role.<role>.backend`
2.**Float precision**: vLLM-Omni may use bfloat16 where diffusers uses float32 for some operations
3.**Missing normalization**: Check all LayerNorm/RMSNorm are preserved
4.**Scheduler rounding**: Some schedulers have numerical sensitivity
### Tensor Parallel Errors
**Symptom**: `AssertionError: not divisible` or incorrect output with TP>1
description:"Integrate a new text-to-speech model into vLLM-Omni from HuggingFace reference implementation through production-ready serving with streaming and CUDA graph acceleration. Use when adding a new TTS model, wiring stage separation for speech synthesis, enabling online voice generation serving, debugging TTS integration behavior, or building audio output pipelines."
- **Two-stage pipeline** (e.g. Qwen3-TTS, Fish Speech, CosyVoice3): AR
code-predictor → audio decoder, connected via async_chunk for low-latency
streaming. Use this for maximum performance.
- **Single-stage AR via generator** (e.g. MOSS-TTS-Nano): entire model runs
inside one AR worker, streaming audio chunks through a per-request
`inference_stream()` generator. Use this when the upstream model bundles AR
+ codec inseparably. See [references/single-stage-ar.md](references/single-stage-ar.md).
- **Single-stage, vLLM-native base LM + side computation** (e.g. VoxCPM2):
the base language model runs under vLLM's PagedAttention as a normal AR
model; diffusion / VAE / side computations run outside vLLM and are
attached via the runner post-processing hook. This is a distinct pattern
from the generator approach above — do not confuse the two.
The single-stage variants skip Phase 4 (async_chunk) but Phase 5 (CUDA graph)
is still encouraged for the inner AR loop.
## Cross-Cutting Invariants
These rules apply to every TTS model regardless of architecture (AR vs AR+diffusion, single-stage vs two-stage, codec-based vs VAE-based). They surface repeatedly across PRs — check them at the end of every phase.
### I1. Streaming output contract
Pick exactly one per-step semantics for `forward()` and document it in the docstring:
- **Delta**: yield only new audio samples produced this step. Preferred — linear cost, low memory.
- **Cumulative**: re-decode from step 0 every call. O(N²); only acceptable if the codec has no streaming decode path.
If you choose **delta**, verify the full emit→consolidate→consume chain:
2.`_consolidate_multimodal_tensors()` in `vllm_omni/engine/output_processor.py` concatenates the audio key into one tensor at finish. If it skips the key (`continue`), offline consumers receive only the final chunk. See `output_processor.py` for the concrete list of handled modality keys.
3. Streaming consumers (SSE, Gradio) receive per-step deltas; offline consumers (`engine.generate()`) receive a single concatenated tensor.
Cumulative-vs-delta mismatch is the most common silent bug — offline RTF benchmarks pass, but users hear replays or truncation.
### I2. Multimodal output consumer hygiene
`outputs[0].outputs[0].multimodal_output[<key>]` can be any of `Tensor`, `list[Tensor]` (pre-consolidation snapshot), `np.ndarray`, or scalar. When writing tests, examples, and benchmarks:
- **Never** use `dict.get("a") or dict.get("b")` on tensor values — Python evaluates the tensor's boolean, raising `RuntimeError: Boolean value of Tensor with more than one value is ambiguous`. Use explicit `if x is None` chains.
- Always defensively handle the list form: `if isinstance(x, list): x = torch.cat([t.reshape(-1) for t in x], dim=0)`.
- Assert `shape` / `dtype` / `duration` explicitly; do not rely on truthiness for presence checks.
### I3. Hot-loop GPU discipline
Inside any per-step model loop (AR decode, diffusion solver, CFM Euler, vocoder block loop):
- No `tensor.item()`, `.cpu()`, or `.tolist()` — each triggers a GPU→CPU sync; at 10 steps × 60 frames × 4 ops that is 2400 syncs per request.
- Prefer `dst.copy_(src)` over `dst.fill_(src.item())` when writing a scalar tensor into a buffer.
- Prefer `torch.compile(Model.forward, fullgraph=False)` on the whole forward over per-submodule compile — fewer dispatch boundaries, larger fusion regions. Measure before choosing granularity.
- No Python-side control flow that depends on tensor values; use `torch.where` / masking instead.
Profile first, optimize second. See the profiling docs / project memory for the trace-analysis workflow.
### I4. Validation pyramid
Offline RTF alone is necessary but not sufficient. Every new TTS model must pass all three:
| Concurrent requests | Per-request state leaks, codec window round-robin gaps | `max_num_seqs>1` smoke test with 4+ parallel prompts |
Declaring a model "done" without all three has shipped regressions more than once.
### I5. Per-request state is owned by the request, not the model
If the model caches *anything* across `forward()` calls (streaming generators, codec buffers, sliding-window pads, CUDA graph state), key it by request ID:
```python
self._state:dict[str,YourState]={}# request_key → state
6.**Tensor layout**: Codebook-major vs frame-major ordering
7.**dtype**: Float32 for codec decoders (autocast can corrupt audio)
### Streaming Correctness Rules (single-stage and two-stage)
These bugs appear in almost every new TTS PR. Check all before the first push. See also the cross-cutting invariants I1 (output contract) and I5 (per-request state) above — the rules below are the Phase 2-specific instances of those invariants:
- **Accumulate codes across AR steps** — each `forward()` appends new codes; do not reset between steps or audio will be truncated (fish speech: `fix: accumulate audio_codes across steps`)
- **Emit delta audio, not full waveform** — in streaming mode yield only the new chunk per step, not the re-decoded full waveform from step 0 (fish speech: `fix: emit delta audio not full waveform`)
- **All return paths must emit `model_outputs`** — if any early-return branch skips setting `model_outputs`, the serving layer silently drops that step's audio (fish speech: `fix: ensure ALL return paths emit model_outputs`)
- **Per-request state isolation** — for batched concurrent requests, key all state by request ID; a shared buffer corrupts audio across requests (fish speech: `fix: per-request vocode + delta emission`)
- **Codec tensor device** — move codec codes to the codec decoder's device before calling decode; mismatches cause silent CPU fallback or crashes (fish speech: `fix: use model device for CUDA stream`)
- **AR stage `max_num_seqs`** — set to at least 4 in production deploy configs; for single-stage models this is the only stage. For two-stage models, Stage 0 (AR) needs `max_num_seqs ≥ 4` to pipeline concurrent requests; Stage 1 (codec decoder) is model-specific and may intentionally use `max_num_seqs: 1`. Defaulting the AR stage to 1 causes audio gaps under concurrency because the codec window round-robins across requests (RFC #2568)
### Optional Dependency Handling
Patch optional dependencies (`torchaudio` / `torchcodec` / `soundfile`) at
the top of `load_weights()`, not at module import. Failures to do so cause
cryptic errors only on environments missing the optional package — after
the model is already deployed. See
[references/optional-deps.md](references/optional-deps.md) for the full
pattern, signature constraints, and MOSS-TTS-Nano reference.
### Single-Stage AR Pattern (alternative to two-stage)
When the upstream model cannot be cleanly split into an AR stage and a
separate decoder, run the full pipeline inside a single AR worker and
stream audio through a per-request `inference_stream()` generator keyed by
`_omni_req_id`. Define one `StagePipelineConfig` with
`final_output=True`, and `owns_tokenizer=True`. Set `async_chunk: false` in
the deploy YAML. Only extract params from
`additional_information` that you actually forward, or pre-commit fails
`ruff F841`.
Full walkthrough with the complete `forward()` / `_create_stream_gen()`
skeleton plus pipeline/deploy definitions:
[references/single-stage-ar.md](references/single-stage-ar.md). For an
in-tree reference, look for any single-stage AR model under
`vllm_omni/model_executor/models/`, such as MOSS-TTS-Nano.
**VoxCPM2 is a different pattern** and should not reuse this skeleton — it
runs the base LM under vLLM PagedAttention with external side-computation.
See `plan/voxcpm2_native_ar_design.md`.
### Deliverables
- Model files in `vllm_omni/model_executor/models/<model_name>/`
- Registered `pipeline.py` topology
- Deploy YAML under `vllm_omni/deploy/`
- Working `end2end.py` at `examples/offline_inference/text_to_speech/<model>/end2end.py`
- New section in `examples/offline_inference/text_to_speech/README.md` (table row + per-model section). Do **not** create a top-level `examples/offline_inference/<model>/` dir or a per-model `README.md` inside `text_to_speech/<model>/` — the hub README is the documented surface and the mkdocs `generate_examples` hook only descends one level into `examples/<category>/`.
## Phase 3: Online Serving
**Goal**: Expose the model via `/v1/audio/speech` API endpoint.
### Steps
1.**Write one adapter** under `vllm_omni/entrypoints/openai/tts_adapters/`.
`serving_speech.py` should not need an edit — detection, stage discovery and
dispatch are all derived from what the adapter declares.
"""Build prompt with reference audio for voice cloning, called from the adapter."""
audio_bytes = Path(ref_audio_path).read_bytes()
codes = codec.encode(audio_bytes) # Encode on CPU using model's codec (e.g., DAC)
token_ids = [code + codec.vocab_offset for code in codes.flatten().tolist()]
return [
{"role": "system", "content": f"<|voice|>{''.join(chr(t) for t in token_ids)}"},
{"role": "user", "content": text},
]
```
### Test Case Writing (CI Levels)
**Follow the [vllm-omni-test skill](../vllm-omni-test/SKILL.md)** for markers, file naming (`test_{slug}.py` / `test_{slug}_expansion.py`), Buildkite wiring, and copy-paste run commands. Also read [test_system_overview.md](https://github.com/vllm-project/vllm-omni/blob/main/docs/contributing/ci/test_system_overview.md) and [test_writing_guide.md](https://github.com/vllm-project/vllm-omni/blob/main/docs/contributing/ci/test_writing_guide.md).
Classify the model's **CI priority** first (high / medium / low). High-priority TTS models are typically those on the integration hot path or listed in tracking issues such as [#1832](https://github.com/vllm-project/vllm-omni/issues/1832); medium and low tiers cover the long tail. When unsure, ask the reviewer which tier applies.
| **L4** | `tests/e2e/online_serving/test_{slug}_expansion.py`, optional offline expansion | `full_model` + `tts` | `test-nightly.yml` (`:full_moon: TTS · Function Test with L4`) | Feature matrix; perf → `tests/dfx/perf/tests/test_tts.json` |
**L2 & L3 online — same file, dual marks on the baseline smoke:** The **first / simplest** case in `test_{slug}.py` (default deploy, single non-streaming `/v1/audio/speech` or equivalent offline path) should carry **both** `@pytest.mark.core_model` **and** `@pytest.mark.advanced_model` on the **same** function so it runs in L2 (`test-ready.yml`, `--run-level core_model`, basic validation) and L3 (`test-merge.yml`, `--run-level advanced_model`, deeper validation) without duplicating the test. In-tree examples: `test_voxcpm2_tts.py::test_text_to_audio_001`, `test_qwen3_tts_customvoice.py::test_text_to_audio_001`.
Heavier scenarios in the same file use **`advanced_model` only** (streaming, extra languages, concurrency, async_chunk, batch). Example: `test_voice_clone_en_streaming_001` → `advanced_model` only. When migrating L3 to nightly, move those heavier cases into `test_{slug}_expansion.py` with `full_model` and drop the dedicated merge job (see `test_ming_tts_expansion.py`, `test_glm_tts_expansion.py`).
**L4 consolidation:** Prefer parametrized `OmniServerParams` rows (`default`, `async_chunk`, feature flags) in one expansion module rather than many merge-only files ([#1832](https://github.com/vllm-project/vllm-omni/issues/1832)).
**L4 performance (high-priority models):** Add latency / throughput / stress rows in `tests/dfx/perf/tests/test_tts.json`, or a dedicated `tests/dfx/perf/tests/test_{slug}.json` when the model must not join the shared nightly server matrix before integration lands (see VoxCPM2 / Coqui XTTS pattern). Register the model in `benchmarks/tts/model_configs.yaml` for local `bench_tts.py`. Wire a **separate** `test-nightly.yml` Perf Test step when the JSON is not merged into `test_tts.json` yet.
**Keep model-specific code inside test modules — not `tests/helpers/{slug}.py`:**
- Put `MODEL`, deploy path, vendored `REF_AUDIO_URL`, `get_prompt()`, and inline `request_config` dicts **in each** `test_{slug}.py`, `test_{slug}_expansion.py`, offline `test_{slug}.py`, and L1 `test_{slug}_*.py` as needed.
- **Do not** add `tests/helpers/{slug}.py` (or `tests/helpers/{model_name}.py`) to deduplicate constants or request builders across those files. A little duplication is intentional; follow in-tree references such as `tests/e2e/online_serving/test_glm_tts.py` and `tests/e2e/online_serving/test_cosyvoice3_tts_expansion.py`.
- `tests/helpers/` is for **repo-wide** harness code only (`mark.py`, `media.py`, `runtime.py`, `stage_config.py`, `assertions.py`, `fixtures/`). Import those; do not extend the tree with per-model modules.
**Runtime send helpers (`tests/helpers/runtime.py`) — online and offline e2e:**
2. **No matching helper** → add `send_<feature>_request` (or `send_<route>_http_request` for negative/dfx) in **`runtime.py`** with general `assert_*` bundled inside, **then** call it from the test.
3. **Test file** holds `request_config` dicts only — not `omni.generate`, not `_collect_audio()`, not raw HTTP/SDK.
4. Wire `tests/helpers/runtime.py` into Buildkite `source_file_dependencies` when you add helpers.
See [vllm-omni-test skill](../vllm-omni-test/SKILL.md) § **Runtime send helpers** for full tables and exceptions.
### Deliverables
- One adapter file under `tts_adapters/` plus its line in the package import block
- Client scripts and server launcher under `examples/online_serving/text_to_speech/<model>/`
- Gradio demo with streaming and voice cloning UI in the same dir
- E2E tests per **Test Case Writing (CI Levels)** above (priority tier determines L1–L4 scope)
- Buildkite wired per level: `test-ready.yml` (L1/L2), `test-merge.yml` or nightly function job (L3), `test-nightly.yml` (L4) — see [vllm-omni-test skill](../vllm-omni-test/SKILL.md)
- New section in `examples/online_serving/text_to_speech/README.md` (table row + per-model section). Do **not** create a top-level `examples/online_serving/<model>/` dir or a per-model `README.md` inside `text_to_speech/<model>/`.
### E2E test pitfalls to avoid
- **One `OmniServerParams` set per file.** `omni_server` is module-scoped; a second
id in the same file forces mid-module teardown/restart and exposes startup
races (`APIConnectionError` on the first request post-restart). Split variants
into separate files instead.
- **No external URL fetches from the server.** CI and some dev hosts can't
reach `raw.githubusercontent.com` over TLS. Inline ref audio as
`data:audio/wav;base64,...`; the serving layer accepts both URL and data URL.
- **Use the harness readiness gate.** The fixture waits for HTTP 200 on
`/health`; don't add `time.sleep` in tests. If warmup is incomplete, make
`/health` return non-200 until you're actually ready.
- **Mark tests per the CI Levels table** — baseline smoke: `core_model` + `advanced_model`; heavier cases: `advanced_model` only; L4 expansion: `full_model`
- **No per-model helper modules** — do not create `tests/helpers/{slug}.py`; keep constants and `request_config` payloads in the test file
- **Online and offline e2e go through `runtime.py`** — `openai_client.send_audio_speech_request` (online); `omni_runner_handler.send_audio_speech_request` (Qwen-style offline) or `send_single_stage_tts_request` (single-stage offline). Add a new `send_*_request` in `runtime.py` when none fits; do not embed `omni.generate` or HTTP in tests
## Phase 4: Async Chunk (Streaming)
**Goal**: Enable inter-stage streaming so audio chunks are produced while AR generation continues.
### Steps
1. **Update the pipeline topology** so the producing stage declares its async
handoff processor with `async_chunk_process_next_stage_input_func`.
2. **Update the deploy YAML** to enable async chunk and configure the connector:
- [Adding a TTS model (developer guide)](https://github.com/vllm-project/vllm-omni/blob/main/docs/contributing/model/adding_tts_model.md)
- `plan/voxcpm2_native_ar_design.md` — VoxCPM2's vLLM-native AR + side-computation pattern (distinct from the generator-based single-stage described above)
| `check-spdx-header` | Missing header, or copyright still says `vLLM project` | Two-line Omni header (`vLLM-Omni project`); restage the rewrite |
| `check-forbidden-imports` | Stdlib `re`/`base64`, pickle, Hugging Face Hub API, or direct Triton/TileLang | `import regex as re` and `pybase64`; use `vllm.transformers_utils.repo_utils`. Do **not** add the file to `allowed_files` without review |
| `check-torch-cuda-call` | New `torch.cuda.*` call site | Use `current_omni_platform` / `OmniPlatform`; do not grow `ALLOWED_FILES` without review |
| `check-tts-adapter-migration` | New `self._tts_model_type` branch in `serving_speech.py` | Put per-model logic in `tts_adapters/`. Lower `MAX_MODEL_TYPE_BRANCHES` when removing branches; do not raise it without review |
| `check-test-ci-coverage` | New `tests/**/test_*.py` missing level or hardware mark | Add `core_model`/`advanced_model`/… plus `cpu`/`cuda`/`hardware_test(` |
| `mypy-3.10` | Type error on changed `vllm_omni/` files | Fix the types; model trees are excluded. Extra versions: `pre-commit run --hook-stage manual mypy-3.12` |
description:Diagnose and optimize vLLM Omni diffusion workloads, especially Wan/Qwen/Flux-style image and video generation. Use when Codex is asked to analyze profiling traces, choose parallel strategies, inspect torch profiler trace.json or trace.json.gz timelines, estimate optimization ROI, investigate GPU idle/free bubbles, compare USP/CFG/HSDP/VAE parallelism, or design operator/host/quantization optimizations for vLLM Omni.
---
# vLLM Omni Performance Optimization
Use this skill to run a disciplined optimization loop for vLLM Omni diffusion workloads. Keep two ideas separate: real performance baselines are collected with low overhead, while torch profiler traces are diagnostic artifacts and may distort latency.
## First Questions
Before proposing changes, ask for the optimization scene if it is not already known:
- GPU model, card count, topology, and whether NVLink is present.
- Model and pipeline, for example Wan2.2 I2V A14B.
- Three runnable commands for each model/workload, or scripts that generate them:
- **Server startup command**: exact `vllm serve` command, environment variables, model path, port, parallelism flags, profiler flags, and precision/compile settings.
- **User request command**: exact single-request client command (for example curl or Python client) with fixed prompt/media/seed/size/steps. Use this to validate correctness and collect per-request stage timings.
- **User benchmark command**: exact repeatable benchmark command or script with warmup count, measured iteration count, concurrency/batch policy, output directory, and summary format.
- Optimization target: latency, throughput, memory, cost, or quality-preserving speed.
- Precision/quality tolerance: bf16/fp8/quantization/sparsity/approximate attention allowed or not.
- Quality validation method: exact-output comparison when feasible, or metrics
such as SSIM, PSNR, LPIPS, cosine similarity, MAE, MSE, temporal flicker
checks, baseline self-run variance, and reviewable output artifacts.
## Workflow
1.**Freeze the measurement protocol and commands.**
- Before analyzing parallel strategies or traces, establish the three commands for the target model/workload: server startup, single user request, and user benchmark.
- Prefer checked-in scripts or one-off shell scripts over free-form commands in chat. The scripts should make workload variables explicit, including model path, port/base URL, prompt/media inputs, resolution, frames, steps, seed, warmup, measured iterations, concurrency, and output path.
- Measurement reports must preserve the concrete process, not just the final numbers. For each tested configuration, write down the server startup command, the user request command, the user benchmark command or polling loop, the final server response metrics, and the client-side timing/HTTP result.
- Present all duration and latency values in **milliseconds (ms)** in tables and summaries. If an API returns seconds, convert to ms before reporting. Keep raw units only inside quoted raw logs or source snippets.
- Keep profiler modes separate in the command set:
- Baseline/benchmark commands should avoid torch profiler and stack collection.
- Diagnostic commands may enable torch profiler after a baseline identifies a bottleneck.
- If any of the three commands is missing, ask for it or create a proposed script before proceeding.
- Make clear which command is authoritative for correctness validation and which command is authoritative for performance numbers.
2.**Collect a real baseline.**
- Disable PyTorch profiler and stack collection.
- Disable or fix torch.compile state so A/B is fair.
- Avoid `--enforce-eager` for production-speed baselines unless eager is the target.
- Prefer `--log-stats` and `--enable-diffusion-pipeline-profiler` for low-overhead stage timing. PR 3069 has the relevant metrics/log-stats changes; if local code does not include them, fetch or cherry-pick the minimal metrics changes rather than merging unrelated PR drift.
- Run warmup requests and exclude first-request lazy init.
3.**Model the parallel strategy before testing.**
- Estimate compute and communication for self-attention, cross-attention, FFN, CFG, VAE encode/decode, and HSDP.
- Select a small candidate matrix rather than exhaustively testing everything.
- Typical 4-card candidates: `USP=4`, `CFG=2 x USP=2`, `USP=2 x HSDP`, VAE parallel on-off.
- Typical 8-card candidates for official CFG-enabled video diffusion models:
- Primary candidate: `CFG=2 x USP=4` with VAE patch parallel across all 8 ranks.
- Compare against: `CFG=1 x USP=8` to test whether larger sequence parallel groups beat CFG branch parallelism.
- Isolate VAE parallelism: keep the DiT strategy fixed and compare VAE patch parallel on/off or different VAE patch sizes.
- If long-video `diffuse` remains dominant and Ulysses all-to-all is suspected, test a hybrid sequence strategy such as `CFG=2 x USP=2 x Ring=2`.
- Test HSDP off only if the model fits without it; treat HSDP primarily as a memory strategy until A/B proves otherwise.
- Prefer USP/SP for long video token sequences; prefer CFG parallel when CFG doubles transformer forwards and sequence length is modest.
- Convert workload shape to latent/patch token counts before choosing candidates. For video models, record frames, latent frames, latent HxW, patch size, approximate token count, and which stages should scale with the token count.
4.**Search for the best parallel configuration.**
- Start with a small matrix that answers one question per comparison:
-`CFG parallel vs larger USP`: compare `CFG=2 x USP=world/2` against `CFG=1 x USP=world` for CFG-enabled workloads.
-`VAE patch parallel value`: compare the best DiT strategy with VAE patch parallel enabled and disabled.
-`HSDP cost`: compare HSDP on/off only when both configurations fit memory.
-`Ulysses vs Ring`: test a Ring/Ulysses hybrid only after long-sequence `diffuse` is confirmed dominant.
- For each configuration, create or record a stable config id, for example `A_cfg2_usp4_vaepp8_hsdp_tiling`.
- For each config id, capture:
- exact server command and environment variables,
- observed distributed setup from logs, such as SP groups, CFG groups, HSDP shard/replicate sizes, VAE patch size,
- exact request command for every scenario,
- final server response metrics and client-side elapsed time,
- output artifact paths and any failed/empty responses.
- Use one warmup request per scenario, then at least three measured repeats for the shortlisted config. For exploratory matrix pruning, one measured request is acceptable only if the margin is large; label it as one-shot.
- Compare configurations by stage, not only end-to-end latency. A configuration can improve `diffuse` while hurting `vae.decode`; record both effects.
- Select the best config only after checking the target metric, dominant stages, memory headroom, and output correctness.
- Record latency, stage timings, memory, output quality, and logs.
- Report comparison tables in ms. Include at least end-to-end client time, server `inference_time`, server stage generation time if available, `vae.encode`, `diffuse`, `vae.decode`, and peak memory.
6.**Enforce the quality and precision gate for every optimization.**
- Do not mark an optimization ready until quality is checked on the same
- For math-preserving changes, require exact/near-exact agreement or show
that differences are within baseline self-run variance.
- For precision, quantization, approximate attention, sparse/custom kernels,
or backend changes, include reviewable artifacts and metrics such as SSIM,
PSNR, LPIPS, cosine similarity, MAE, MSE, and temporal flicker checks.
- If quality tolerance is not stated, default to quality-preserving behavior.
Failed or inconclusive quality validation blocks a ready-to-merge claim.
7.**Collect diagnostic trace only after narrowing hypotheses.**
- Use torch profiler for a small number of requests.
- Run two separate diagnostic traces instead of mixing concerns:
- **Operator/shape trace**: enable `torch_profiler_record_shapes=True` and keep stack collection disabled. Use this to rank CUDA kernels, NCCL collectives, attention/MLP/norm/RoPE work, and shape-specific hot operators.
- **Host-stack trace**: enable `torch_profiler_with_stack=True` and normally keep shape collection disabled. Use this to map CPU/Python host gaps, synchronization points, scheduler paths, and request handling overhead.
- Keep both trace commands and reports separate from baseline/benchmark commands. Torch profiler latency is diagnostic only and must not be used as the final latency claim.
- Prefer profiling only the narrowed dominant scenario, for example the highest-resolution/video-length case where `diffuse` dominates.
- If profiler endpoints are available, run one warmup request first, then call `/start_profile`, run one profiled request, and call `/stop_profile`. This keeps model initialization and warmup out of the diagnostic trace.
- Start by analyzing rank 0 only. Expand to more ranks only if rank 0 suggests imbalance, unclear GPU idle/free bubbles, high NCCL wait, server timing mismatch, CFG branch imbalance, or USP group stragglers.
- Diagnostic reports must be written to disk and preserve: server command, profiler config, warmup/request/polling commands, trace artifact paths, rank analyzed, analyzer output or summary, and the decision about whether additional ranks are necessary.
- Analyze both rank-level balance and device-level free bubbles when additional ranks are opened.
8.**Analyze host, communication, and operators.**
- Find GPU idle/free intervals and map each large gap to the enclosing CPU/Python code.
- Separate real GPU idle from profiler overhead such as CUPTI `Command Buffer Full`.
- Compare NCCL kernel time to user annotations; annotations can overcount nested intervals.
- Rank operator work by total CUDA time and by repeated small-kernel launch count.
9.**Produce an optimization plan.**
- Classify candidates as P0/P1/P2.
- For each candidate, state necessity, expected benefit, implementation path, validation plan, and quality risk.
- Include a concrete quality/precision validation gate for each candidate.
- Do not implement high-risk operator rewrites before proving the operator is a bottleneck for the target shapes.
- End the plan with a user-facing candidate selection table. The assistant
should not automatically choose a risky optimization just because it is
technically possible. Present the options clearly and let the user decide
which item is worth implementing based on latency target, engineering
budget, memory headroom, and quality tolerance.
- Organize the plan by optimization layer, not by a flat list of ideas:
- host/runtime optimization,
- measurement/benchmark reliability,
- parallelism and communication,
- VAE encode/decode and media pre/post processing,
- operator fusion and layout cleanup,
- attention main-path optimization,
- algorithmic, precision, or approximation changes.
- For each layer, tie every candidate back to evidence from baseline
metrics, diagnostic trace, source code, or output quality requirements.
Do not list generic optimizations without a trace or workload reason.
## Priority Rules
- **P0:** low risk, likely useful, or required for trustworthy measurement. Examples: real baseline, warmup, targeted parallel A/B, disabling avoidable `empty_cache`, scheduler coefficient caching.
- **P1:** meaningful code changes with contained risk. Examples: cross-attention KV caching, VAE gather/broadcast reduction, AdaLayerNorm/RMSNorm/RoPE fusion after trace evidence.
- **P2:** high implementation or quality risk. Examples: FA to LA replacement, custom Triton/CUDA fused kernels, FP8/quantization, sparsity/Rainfusion-style acceleration.
Every implemented optimization needs A/B validation and a passing
quality/precision gate, including math-preserving changes such as padding trim,
layout cleanup, cache reuse, or host/runtime cleanup. Use objective metrics
when available, keep reviewable artifacts, and compare against baseline
self-run variance for precision, quantization, approximate kernels, or backend
changes. If quality validation fails or is inconclusive, do not present the
optimization as ready to merge.
## Optimization Layers
After baseline, parallel-search, and diagnostic traces, summarize optimization
opportunities by layer. This is the core of the performance analysis: the goal
is to connect evidence to a scoped implementation and a validation plan.
blocks, `Top GPU/operator events by total duration`, and `Top NCCL-like events
by category`. Treat `cat=user_annotation` NCCL ranges as enclosing annotations;
prefer `cat=kernel` or `cat=gpu_user_annotation` for real device work.
The analyzer summarizes timing only. It does not parse tensor shapes, attribute
overlap to individual streams, prove quality, or provide final latency claims.
Use `ops_rankN.xlsx` or PyTorch key averages for shape analysis, and re-test any
optimization with non-profiler baseline commands.
Read `references/optimization-playbook.md` when drafting the optimization table or comparing candidate techniques.
## vLLM Omni Heuristics
- Cross-attention usually should not use USP/SP when text/image condition token count is much smaller than latent video tokens. Confirm via trace; in Wan2.2 I2V, self-attention dominates cross-attention.
- VAE bf16/autocast is often worthwhile but requires visual quality checks.
- VAE patch parallel can help decode/encode but may add gather/merge/broadcast overhead. Check whether all ranks need the final decoded tensor.
- HSDP/FSDP is primarily a memory strategy. If the model fits without it, run an on/off latency comparison.
- Scheduler work can create small host/device gaps; cache tiny solve coefficients when timesteps/order are known.
-`torch.cuda.empty_cache()` can prevent OOM but creates synchronization/idle. Make it optional if memory headroom is sufficient.
-`Command Buffer Full` in profiler output is profiler overhead, not a model optimization target.
short_description:"Profile and optimize vLLM Omni diffusion performance."
default_prompt:"Analyze and optimize a vLLM Omni diffusion workload using baselines, traces, parallel strategy modeling, and prioritized optimization plans."
description:Find evidence-backed simplification candidates in vLLM-Omni and, when requested, turn them into focused proposals or code changes. Use for audits of dead, duplicated, speculative, over-generalized, unnecessarily defensive, or hand-rolled code. Use review-pr for ordinary correctness review and diffusion-perf-opt for performance-first optimization.
---
# Find vLLM-Omni Simplifications
Find a small number of well-proven ways to reduce code, state, APIs, or maintenance cost without weakening supported behavior. Treat this as an architecture and ownership audit, not a dead-code search or style cleanup.
The default deliverable is a read-only report. Do not add TODOs, design documents, issues, or code unless the user asks for those changes.
## Establish the Contract
Before judging a candidate:
1. Read the repository instructions supplied by the host.
3. Select only the relevant active module and feature documents from [`docs/design/index.md`](../../../docs/design/index.md). Archived design pages are historical context, not current contracts.
4. Identify the comparison base with `git merge-base HEAD origin/main`; do not assume the current branch represents the intended baseline.
5. If tests or CI coverage may change, use [`vllm-omni-test`](../vllm-omni-test/SKILL.md) for test placement, markers, and commands.
Separate an implementation accident from an intentional boundary. In particular:
- vLLM-Omni extends and sometimes overrides upstream vLLM. A similar upstream implementation is evidence for reuse only after comparing API, lifecycle, and multi-stage semantics.
- Registries, import strings, model configuration, deployment YAML, plugins, and optional dependencies create dynamic consumers that a static reference count can miss.
- Hardware backends, model families, execution modes, and connector implementations may be intentionally parallel. Do not propose collapsing one only because it is inactive in the local environment.
- Stage, replica, request, cache, and transport state can look redundant while representing different owners. Prove that two fields or barriers encode the same fact before merging them.
- A shorter hot path is not a simplification if it adds synchronization, tensor copies, host materialization, recompilation, or a measurable quality/performance regression.
## Strong Candidates
A strong candidate removes a meaningful surface and has a clear replacement or deletion boundary. Examples include:
- A public method, configuration option, registry entry, route, message field, or event has no production consumer and no compatibility obligation.
- Tests or documentation are the only consumers of behavior that is not part of a supported contract.
- Multiple layers mirror the same request, lifecycle, cache-validity, readiness, or topology state.
- AR, diffusion, platform, or model-specific implementations duplicate a stable common operation and can share it without hiding performance-critical differences.
- An inherited upstream vLLM primitive is reimplemented locally without an Omni-specific semantic difference.
- Compatibility or fallback branches protect environments the project no longer supports.
- Serialization, IPC, output formatting, or connector code materializes the same payload more than once or converts between equivalent representations.
- A helper, abstraction, package split, or extension point has only one real implementation and adds more indirection than policy.
- Hand-written infrastructure can be replaced by an existing project dependency, Python standard-library feature, PyTorch primitive, or upstream vLLM facility with net deletion and equal observability.
- Tests maintain large mocks, snapshots, or expected data solely for an unused API or representation.
Do not elevate typo fixes, formatting, isolated renames, or vague complexity complaints into simplification candidates. Bundle small cleanups only when they support one substantive removal.
## Survey by Ownership Boundary
For broad audits, cover the largest or riskiest production deltas first. Useful domains are:
-`vllm_omni/worker/` and AR model execution: upstream vLLM overlays, scheduler/cache state, collective RPC, adapters, and weight loading.
-`vllm_omni/diffusion/`: request/step execution, batching, output materialization, IPC, offload, parallelism, and model pipelines.
-`vllm_omni/entrypoints/`: duplicated validation/defaulting, offline/online parity, streaming, and route compatibility.
- Connectors and distributed code: ownership, copies, barriers, handles, cleanup, and failure propagation across process/device/node boundaries.
- Platform backends: genuine vendor differences versus copied control flow that has drifted.
- Configuration, registries, recipes, and examples: duplicate sources of truth and options that never reach a runtime consumer.
- Tests and CI: redundant fixtures, mocks of obsolete contracts, and expensive coverage that can move to a cheaper level without losing the guarded behavior.
When the user explicitly requests a repository-wide or many-candidate audit and parallel agent work is available and authorized, divide these domains among agents and require the same evidence format from each. Otherwise survey them sequentially.
## Prove or Reject Each Candidate
Use `rg` first, then read every relevant caller and implementation. Search exact Python symbols plus serialized names, route paths, configuration keys, registry strings, YAML values, and CLI spellings.
Classify evidence:
- **Production:** `vllm_omni/`, runtime configuration, registries, serving entrypoints, connectors, recipes used for supported deployments, and executable loader paths.
- **Validation/support:** `tests/`, benchmarks, examples, docs, CI, and developer tooling.
- **External/dynamic:** upstream vLLM contracts, third-party plugins, model repositories, optional backends, and string-loaded objects. State what was checked and what remains unknown.
For each candidate, answer:
1. What exact surface would disappear or merge?
2. Who owns the state or behavior today?
3. Which production consumers exist, including dynamic consumers?
4. What supported behavior, compatibility, performance, memory, or observability could change?
5. What is the smallest validation that would catch a bad simplification?
6. Is the result net deletion after tests, adapters, migration glue, and documentation are counted?
Reject or downgrade the candidate when a live consumer exists and removal is actually a feature decision; when an active design contract explains the separation; when the change only moves complexity; or when hardware/model evidence is unavailable for a performance-sensitive claim.
## Lifecycle and Data-Movement Audits
For asynchronous or distributed code, map the ownership graph before recommending deletion:
Associate every lock, event, sentinel, queue, readiness flag, timeout, abort path, and cache marker with an owner and transition. Coalesce mechanisms only when they represent the same state and have the same failure boundary. Preserve separate mechanisms when they protect publication versus rollback, local versus remote ownership, first-terminal-outcome arbitration, or cleanup after partial failure.
For tensors and media, also trace device, process, and ownership transitions. Count GPU-to-CPU copies, shared-memory/object-store handles, serialization, decode/encode steps, and unlink/release responsibility. Prefer eliminating a transfer or representation over merely hiding it behind a helper.
## Dependency Replacements
Prefer facilities already available in the repository before adding a dependency. For a proposed replacement:
- Name the exact local behavior it covers and any semantics that remain in glue code.
- Compare net deletion, test burden, import/startup cost, and runtime behavior.
- Benchmark when the replaced code is on a model, scheduler, connector, serialization, or serving hot path.
A wrapper that retains the same state machine or conversion logic is not a simplification.
## Deliver the Result
Report only candidates supported by concrete evidence. For each one include:
- **Surface:** files and symbols involved.
- **Evidence:** production consumers found, dynamic paths checked, and relevant design contract.
- **Simplification:** exactly what to remove, merge, inline, or reuse.
- **Benefit:** deleted API/state/code, reduced data movement, or reduced maintenance/test burden.
- **Risk and validation:** behavior that could change and the narrowest useful tests or measurements.
- **Confidence:** high or medium, with the unresolved evidence named for medium-confidence items.
Also list representative ideas that were rejected when that prevents repeated investigation. Prefer three high-confidence candidates over a long speculative inventory.
If the user asks for a durable design proposal, place it under `docs/design/feature/` and register it in `docs/design/index.md`; use current design-document conventions and keep implementation details proportional to the decision. If the user asks for implementation, make the smallest coherent change, update affected tests/docs, and preserve public compatibility unless the user explicitly accepts a breaking change.
## Folding Work from Another Branch
Diff the source branch against its own merge base with `origin/main`, not against the current feature branch. Port only independently justified candidates. Consolidate overlaps into the candidate with stronger evidence, and do not preserve a candidate count by carrying weaker duplicates.
## Validation
Match validation to the outgoing diff:
- Documentation or skill changes: run the repository skill validator when available, check links/paths, search for stale source-specific terminology, and run `git diff --check`.
- Python changes: run the narrowest relevant pytest coverage through the repository environment, then lint/format the changed files.
- Distributed, model, hardware, performance, or accuracy changes: run available CPU/static checks and state the exact GPU/NPU/model gap; never present simulated evidence as device validation.
- Before submission, use [`precheck-pr`](../precheck-pr/SKILL.md) for the branch-level self-review.
When summarizing, distinguish checks that passed from checks that were not runnable, and state the snapshot or branch against which the audit was performed.
description:Self-check your branch before creating a PR — catch dead code, prevent new model-specific Python examples, verify accuracy/perf claims, validate PR title format, and confirm merge readiness. Use when the user says "precheck", "self review", "pre-submit check", or "check my PR before I open it." Never posts to GitHub.
---
# PR Pre-Check
Self-review your branch before creating a PR against `vllm-project/vllm-omni`. Two modes: **quick** catches showstoppers, **full** does a thorough maintainer-grade review. Never posts to GitHub; the report is for the contributor's terminal only.
## Mode Selection
| Mode | When | Time |
|------|------|------|
| **Quick** | About to push, final sanity check | ~3 min |
| **Full** | Ready for review, want maintainer-level scan | ~10 min |
Default to quick if unsure. Run full before marking a PR "ready for review."
## Workflow
### Step 1: Detect Base Branch
```bash
BASE_SHA=$(git merge-base HEAD origin/main 2>/dev/null \
|| git merge-base HEAD main 2>/dev/null \
||echo origin/main)
echo"diffing against ${BASE_SHA}"
git diff --name-only ${BASE_SHA}...HEAD
```
### Step 2: Validate PR Title
Check the most recent commit message (or branch name if no commit yet) against the project convention. Valid prefixes:
| Prefix | Applies to |
|--------|-----------|
| `[Bugfix]` | Bug fixes |
| `[CI/Build]` | Build or CI improvements |
| `[Doc]` | Documentation changes |
| `[Model]` | New/improved models (include model name) |
✗ if: missing prefix, wrong case (`[bugfix]`), or WIP/Draft in title.
⚠ if: `[Model]` prefix without the model identifier (e.g., `[Model] Add new model` — should be `[Model] Add <ModelName> ...`).
### Step 3: Categorize the PR
| Diff contains | PR type |
|---------------|---------|
| New files under `vllm_omni/model_executor/models/<name>/` | **New Model** |
| Changes to `vllm_omni/diffusion/` | **Diffusion Model** |
| `[Bugfix]` prefix or single-file fix | **Bug Fix** |
| Perf/benchmark/throughput claims in commit msg or diff | **Performance** |
| Everything else | **General** |
If multiple rows apply (e.g., a diffusion model is also a new model), union the checklists.
### Step 4: Run Checklist
Ask: "Quick mode or full mode?" Then walk the checklist for the detected PR type from [references/checklists.md](references/checklists.md). Each item produces ✓, ✗, or ⚠.
**Also run the Code-Quality sweep on every PR**, regardless of type or mode: the five diff-scoped checks in [references/code-quality.md](references/code-quality.md) — kwargs fragility, broad-except swallow, `Any`/wrong type hints, hot-path `.clone()`/`deepcopy`, and event-loop blocking — plus the advisory conventions (log level, structured logging, synchronization, cleanup, dependencies, naming) in [checklists.md](references/checklists.md). These count only lines the PR *adds* — the pre-existing backlog across the repo is out of scope.
**Also run the Examples-Policy check on every PR** using [references/examples-policy.md](references/examples-policy.md). Inspect only Python paths introduced by the diff. Treat a new model-, checkpoint-, vendor-, or family-specific Python example as blocking; do not report pre-existing example debt.
**Also run the Simplification pass on every PR** using [find-simplifications](../find-simplifications/SKILL.md). Keep it diff-scoped: inspect added or modified public APIs, state, abstractions, fallback and compatibility paths, data movement, and only the adjacent ownership needed to prove a candidate. Do not expand the pass into a repository-wide audit. Zero candidates is a valid pass; report only evidence-backed opportunities. Mark a candidate as blocking only when it proves a correctness or merge-readiness problem, such as newly unreachable code. Otherwise report it as a warning.
**Also confirm local pre-commit gates** from [docs/contributing/README.md](../../../docs/contributing/README.md#linting). GitHub Actions `SKIP` does not prove they passed. Full new-hook list: Omni SPDX (`vLLM-Omni project`); forbidden imports (stdlib `re`/`base64`, pickle, Hugging Face Hub API, Triton/TileLang); no new `torch.cuda` call sites; shellcheck (macOS/Windows must install the binary); mypy-3.10; test files have CI level + hardware marks; TTS `_tts_model_type` branches in `serving_speech.py` must not increase; markdownlint on `docs/`; Buildkite YAML schema. ✗ for growing `allowed_files` / `ALLOWED_FILES` or raising `MAX_MODEL_TYPE_BRANCHES` without review.
✗ Fix Ovis image text encoder dtype ← missing prefix
✗ [bugfix] Fix Ovis image text encoder ← wrong case
✗ [WIP] Add model ← WIP in title
```
- [ ]**Title has valid prefix** — ✗ if missing prefix, wrong case, or WIP/Draft in title. For `[Model]` PRs, the title MUST also include the model identifier (e.g., `[Model] Add <ModelName> ...`). ⚠ if missing the model name.
---
## All PRs: Code Quality (run on every PR, quick & full)
Diff-scoped sweep for five fragility patterns that are pervasive in the existing codebase — **only lines the PR adds count**, not the pre-existing backlog. Detection commands, codebase examples, and full severity rules live in [code-quality.md](code-quality.md).
- [ ]**No new `**kwargs` string-lookup plumbing:** the diff adds no `**kwargs` + `kwargs["..."]` / `kwargs.get("...")` / `"..." in kwargs`. ⚠ any new instance; ✗ if a new string key is duplicated across ≥2 files without a shared constant, or unknown keys are silently dropped on a fail-fast path.
- [ ]**No new broad exception swallow:** the diff adds no `except Exception:` / bare `except:`. ⚠ any new instance; ✗ for `except: pass` / `except Exception: return None` / `continue` on a fail-fast path (init, config validation, weight loading, request handling).
- [ ]**No new `Any` / wrong type hints:** the diff adds no `: Any` / `-> Any` / untyped production signature, and no new `SimpleNamespace` in tests faking an object that already has a real typed stub. ⚠ any new instance; ✗ for a *wrong* (actively misleading) annotation or a `SimpleNamespace` test fake of a typed object.
- [ ]**No new hot-path copy:** the diff adds no `.clone()` / `.copy_()` / `copy.deepcopy(...)` inside a per-step loop or per-request path. ⚠ any new instance; ✗ for `.clone()` of a full latent/activations tensor inside an AR or diffusion step loop without a comment, or `deepcopy` of a scheduler / sampling-config on the request path.
- [ ]**No new event-loop blocking:** the diff adds no `time.sleep` / blocking HTTP (`requests`, `urllib`) / `await` inside a held lock in `async def` code. ⚠ any new instance; ✗ for blocking sleep or HTTP on the serving path (`engine/`, `entrypoints/`, async `connectors/`), or a lock held across an `await`. Blocking inside a dedicated worker thread is fine.
Roll the ⚠/✗ counts into the report as a single **Code quality** dimension row.
---
## All PRs: Examples Policy (run on every PR, quick & full)
Inspect Python files introduced under `examples/` using
[examples-policy.md](examples-policy.md). This is a diff-scoped ratchet: existing
model-specific examples may be edited or removed without triggering the check.
- [ ]**No new model-specific Python examples:** the diff adds, copies, or renames no Python file whose path or behavior is specific to one model, checkpoint, vendor, or model family. ✗ for any violation. Route inference through a shared task/protocol entrypoint, model request behavior through `vllm_omni/model_extras`, and commands through task docs or `recipes/`.
- [ ]**New generic example code is justified:** any new task-neutral Python file under `examples/` is an actual user-facing entrypoint rather than reusable production logic, a benchmark, an app, a setup tool, or a reproducer. ⚠ when the placement is questionable.
Roll the result into the report as a separate **Examples policy** dimension.
### Conventions (eyeball — no grep)
Judgment calls that don't grep cleanly — apply them while reading the diff, not as a mechanical check:
- **Right log level** — `info` is for end-user-visible events; routine internal events are `debug`; hot paths are `trace` or removed. Logging is not free.
- **Structured fields over f-strings** — pass dynamic values as structured fields (`logger.info("msg", extra={...})`), not interpolated strings, so they stay greppable and parseable.
- **No reflexive synchronization** — don't add `threading.Lock` / `asyncio.Lock` / `deepcopy` / re-wrap shared state unless there is real concurrent mutation; the GIL covers most Python code. Owners decide their own synchronization — don't pre-lock in a constructor.
- **Context managers for cleanup** — use `with` / `contextlib` for files, locks, CUDA streams, and profilers; not raw `open()` + manual `.close()` or ad-hoc `finally`.
- **No new dependencies without justification** — prefer the stdlib and existing vllm-omni deps; a new top-level dep needs a reason (check `pyproject.toml` / `requirements*.txt`).
- **Naming / interface discipline** — don't rename or change interfaces just for taste in this PR, but do fix names that actively mislead (e.g. `serve` implying a long-running server, `Instance` where multiple instance kinds coexist).
---
## Bug Fix PRs
### Quick
- [ ]**PR body has 4 sections:** what broke (error message), repro steps, root cause, fix description
- [ ]**Fix is minimal:** diff touches only files directly related to the bug. No unrelated refactors or style changes.
- [ ]**Repro is runnable:** the PR body includes a copy-paste command that reproduces the bug
- [ ]**Branch is rebased:**`git merge-base HEAD origin/main` is recent, no merge conflicts
### Full (adds)
- [ ]**Regression test exists:**`git diff --name-only` includes at least one `tests/` file
- [ ]**Regression test reproduces the original error:** the test would fail on main, pass on this branch
- [ ]**Fix matches root cause exactly:** no "fix the symptom + something else" — if the root cause is dtype, the fix is dtype, not dtype + reformatting
- [ ]**No silent failure risk:** no bare `except: pass`, no `try: ... except: return None`, no empty fallback added. See the broad-except pattern in [code-quality.md](code-quality.md#2-broad-exception-swallow) for the full severity rule (⚠ any new broad catch; ✗ for a swallow on a fail-fast path).
- [ ]**Upstream pattern match:** the fix follows the same pattern as existing code for similar cases (grep for analogous `from_pretrained` calls, etc.)
- [ ]**Environment documented:** torch/transformers/vllm versions listed if the bug is version-dependent
---
## Performance PRs
### Quick
- [ ]**Before/after numbers in PR body:** at minimum, one metric (RTF, RPS, latency, VRAM) with main vs PR values
- [ ]**Hardware specified:** GPU model, count, VRAM. "L4" or "H20" is sufficient. "tested on GPU" is not.
- [ ]**Concurrency scaling:** if the optimization claims better throughput, numbers at 2+ concurrency levels
- [ ]**No unexplained regressions:** if any metric goes down at any concurrency level, explain why
- [ ]**Branch is rebased:**`git merge-base HEAD origin/main` is recent, no merge conflicts
### Full (adds)
- [ ]**Software versions:** torch, CUDA, vllm, vllm-omni versions in PR body
- [ ]**`__init__.py` exports match registry:** each registered class appears in `__init__.py` and `__all__`
- [ ]**Shell script modes match README:**`run_curl.sh` supports every mode the README documents
- [ ]**Branch is rebased:**`git merge-base HEAD origin/main` is recent, no merge conflicts
### Full (adds)
- [ ]**Dead code scan (5 patterns):**
1. Dead `forward()` — defined but never called through the pipeline path
2. Dead factory/builder functions — instantiated nowhere
3. Dead wrappers — thin wrappers with no callers
4. Dead branch guards — `if version > X` where X is always satisfied
5. Unused parameters — `__init__` args never accessed
- [ ]**Copy-paste detection:** no string constant defined in 3+ files, no cross-module validation duplication, no near-identical shape coercion fns
- [ ]**Import hygiene:** no re-export-only imports, no module-level side effects without docs, no `import os` in function body when already at top. Function-body imports are acceptable when the import is only reachable from a closure or factory return value (i.e., `import` would otherwise execute at module load even though the user may never trigger that path). Either lift to top-of-file or annotate with `# noqa: PLC0415`.
- [ ]**Accuracy:** at least one test compares output against reference; audio = valid WAV at correct sample rate; determinism verified if seed claimed. ⚠ if missing and not explicitly deferred to a follow-up PR or RFC section; — if explicitly deferred (the deferral target SHOULD be linked in the PR body).
- [ ]**Performance:** hardware, software versions, warmup, and metrics (RTF, VRAM, RPS, latency) stated. ⚠ if missing and not explicitly deferred; — if explicitly deferred.
- [ ]**Pipeline has both offline and online paths:**`forward()` for batch generation, streaming path for online serving
- [ ]**Cache-DiT integration:** at least one acceleration (TeaCache, FBCache, etc.) is wired up. ✗ for new **top-level** pipelines. For **subclass** pipelines that inherit `__call__` from a parent where Cache-DiT is already absent, downgrade to ⚠ and recommend filing a follow-up issue against the parent pipeline family.
- [ ]**Parallelism config:** TP/SP/USP/CFG-Parallel options exposed in pipeline config
- [ ]**Docs table updated:** model added to `docs/models/supported_models.md` with correct metadata
- [ ]**E2E test exists:** at least one test exercises the full generate path
---
## General PRs
### Quick
- [ ]**PR body matches diff:** description doesn't claim changes not in the diff
- [ ]**Branch is rebased:**`git merge-base HEAD origin/main` is recent, no merge conflicts
- [ ]**CI gates passing:** DCO, pre-commit, build — check `gh pr view --json statusCheckRollup` if a PR already exists
- [ ]**Local pre-commit gates:** full list in [Linting](../../../../docs/contributing/README.md#linting). Blocking when the diff introduces: stale `vLLM project` SPDX; stdlib `re`/`base64` or pickle in library code; new `torch.cuda` call sites; `tests/**/test_*.py` without level+hardware marks; new `_tts_model_type` branches in `serving_speech.py`; invalid `.buildkite` YAML. macOS/Windows need a native `shellcheck`. ✗ for growing `CHECK_IMPORTS[*].allowed_files`, `ALLOWED_FILES`, or `MAX_MODEL_TYPE_BRANCHES` without review.
### Full (adds)
- [ ]**No new dead code in changed files:** scan for unreachable paths, unused imports
- [ ]**Test coverage:** if core code changed (`engine/`, `stages/`, `connectors/`), tests are added or existing tests cover the path
- [ ]**Import hygiene:** no redundant imports added
Five fragility patterns to sweep on **every** PR, in both quick and full mode. These are pervasive in the existing codebase (hundreds of pre-existing sites) — so the checks are **diff-scoped**: only lines the PR *adds* count. Whole-repo greps light up every PR with the backlog and are useless.
All commands assume:
```bash
BASE="$(git merge-base HEAD origin/main)"
```
**Severity:** ⚠ for any *new* instance introduced by the diff; ✗ for the acute sub-cases defined per pattern. Acute ✗ findings belong in the report's blocking count.
---
## 1. `**kwargs` string-lookup passthrough
### Anti-pattern
```python
defforward(self,**kwargs):
# BLOCKER-prone: raw dict + string keys duplicated across files
if"runtime_additional_information"inkwargsand \
"model_intermediate_buffer"notinkwargs:
...
```
Real instances of this exact guard are copy-pasted across `cosyvoice3.py`, `indextts2_talker.py` (×2), `indextts2_s2mel_decoder.py`, `qwen3_tts_talker.py`, `qwen3_omni.py`. A typo in any string key silently no-ops; there is no type check, no IDE completion, and unknown keys are silently dropped.
### Why dangerous
Raw `dict` + `**kwargs` + lists of param-name strings is **fragile by construction**: the compiler can't see a missing or misspelled key, so failures are silent. Each duplicated copy is a new place for the strings to drift.
### Severity
- ⚠ any *new*`**kwargs` signature whose body does `kwargs["..."]` / `kwargs.get("...")` / `"..." in kwargs`.
- ✗ when the diff introduces a **new** string key that is duplicated across ≥2 files without a shared module constant, or when `**kwargs` plumbing **silently drops unknown keys** on a fail-fast path (init, config validation, weight loading).
Prefer explicit typed params. The repo already has the right primitive: `msgspec.Struct` with `forbid_unknown_fields=True` rejects unknown keys instead of silently dropping them — see `_StructBase` in `vllm_omni/data_entry_keys.py:105` and `MoriPullRequest` in `vllm_omni/distributed/omni_connectors/connectors/mori_transfer_engine_connector.py:81`. A `@dataclass` or `TypedDict` works too.
If `**kwargs` is unavoidable (vLLM base-class compat), centralize the key strings as module-level constants behind **one** typed accessor, instead of repeating the literal across files.
---
## 2. Broad exception swallow
### Anti-pattern
```python
# BLOCKER-prone: catches everything, returns None on any failure
try:
evt=build_event(...)
returnevt
exceptException:
returnNone
```
Real clusters: `vllm_omni/metrics/stats.py:240,274,367,450` and `vllm_omni/metrics/{modality,utils}.py` — all `except Exception: return None`.
### Why dangerous
A generic `except Exception:` is **too flexible**. If we catch an exception type we don't expect, it is almost always an unhandled edge case that needs to be *fixed* — not swallowed into `None`/`pass`/`continue`. Broad catches turn fixable bugs into silent wrong behavior and make debugging impossible.
The repo backs this with ruff `BLE001` (broad-exception catch); it is currently suppressed via `# noqa: BLE001` in `vllm_omni/patch.py:413`. Prefer fixing over suppressing.
### Severity
- ⚠ any *new*`except Exception:` / `except BaseException:` / bare `except:`.
- ✗ for `except:` / `except Exception: pass|return None|continue` on a **fail-fast path** (init, config validation, weight loading, request handling, connector setup). This extends the Bug-Fix "No silent failure risk" ✗ in [checklists.md](checklists.md).
# Note: this matches only the immediate-colon swallow forms (`except :`,
# `except Exception:`) — it deliberately excludes specific catches like
# `except ValueError:` and the `except Exception as e:` variant. Eyeball
# the latter; `as e` usually means it is logged, but still verify.
```
### Fix
Catch the **specific** types you actually expect (`ValueError`, `KeyError`, `AttributeError`, `OSError`, `torch.cuda.OutOfMemoryError`, `msgspec.ValidationError`, …). At a genuine top-level / best-effort boundary (metrics, signal handlers), at minimum **log** the exception; never swallow into a bare `pass`/`return None` on a path that should fail loudly.
---
## 3. `Any` / wrong type annotations
### Anti-pattern
```python
# ⚠ Any param + Any return — no contract, no narrowing
`Any` disables the type checker for that position, so typos and wrong shapes pass silently. The repo has 636 `: Any` params, 244 `-> Any` returns, and **1,173 `SimpleNamespace` in `tests/`**. The `Any` leak is largely test-driven: vibe-coded unit tests fake objects with `SimpleNamespace`, which forces the production code consuming them to be typed `Any`, and the `Any` then propagates outward into public signatures.
### Severity
- ⚠ any *new*`: Any` / `-> Any` / fully-untyped signature in production code.
- ✗ for a **wrong** annotation (actively misleading — e.g. `-> bool` that returns `Optional[bool]`), or a *new*`SimpleNamespace` in a test that mimics an object which already has a real typed stub / `@dataclass` / `TypedDict` / `Protocol`.
### Detect (diff-scoped)
```bash
# Any leaking into new signatures:
git diff ${BASE}...HEAD -- "*.py"| grep "^+[^+]"\
| grep -E ':\s*Any\b|->\s*Any\b'
# SimpleNamespace newly added in tests (the Any-leak source):
Replace `Any` with the concrete type, a `Protocol`, or a `Union`. If a type is genuinely dynamic, prefer `object` + `isinstance` narrowing over `Any` — `object` keeps the checker engaged. In tests, replace `SimpleNamespace` with the real class, a small `@dataclass`/`TypedDict`, or a `Protocol` + fake, so the system under test stays fully typed.
---
## 4. Unnecessary copy — `.clone()` / `copy.deepcopy` in hot paths
### Anti-pattern
```python
# Per-step in a diffusion/AR loop: clones the full latent every iteration
fortintimesteps:
trajectory_latents.append(x_t.clone())# O(N) GPU alloc + memcpy each step
```
Real clusters: `vllm_omni/diffusion/models/bagel/bagel_transformer.py` calls `x_t.clone()` ~9× across a trajectory loop (lines 1820–2186); the cache hooks `vllm_omni/diffusion/cache/teacache/hook.py:155` and `magcache/hook.py:182` clone `hidden_states` per step. Separately, `copy.deepcopy(self.scheduler)` / `copy.deepcopy(req.sampling_params)` is copy-pasted across diffusion pipelines — `pipeline_qwen_image.py:786`, `pipeline_hunyuan_image3.py:1904`, `pipeline_ltx2*.py` (×3), `pipeline_dreamzero.py:1085`, `diffusion_model_runner.py:398` — a full object-graph deepcopy on the request path. (Contrast `dreamzero/causal_wan_model.py:203`, which carries a `# No .clone() needed.` comment — the aware pattern.)
### Why dangerous
A `.clone()` of a GPU tensor or a `copy.deepcopy` of a config object inside a per-step / per-request path is a hidden allocator: it reads as one Python line but triggers a GPU memcpy or a deep object-graph walk every iteration. Across 50 diffusion steps or hundreds of AR steps it dominates latency and VRAM bandwidth. `deepcopy` additionally ignores any custom device / `__copy__` logic and can drag tensors through CPU.
### Severity
- ⚠ any *new*`.clone()` / `.copy_()` / `copy.deepcopy(...)` inside a per-step loop or per-request path.
- ✗ for `.clone()` of a full latent/activations tensor inside an AR or diffusion step loop without an explanatory comment, or `copy.deepcopy` of a scheduler / sampling-config on the request hot path.
Tensors: prefer a view (`x[..., :k]`, `torch.narrow`, `as_strided`) or write into a pre-allocated buffer with `dst.copy_(src)` instead of `.clone()`; if the caller no longer needs the original, `move`/`del` it rather than copy. Schedulers / configs: build a fresh lightweight instance per request, or split out the mutable per-request bits so the shared immutable part isn't deepcopied. If a copy is genuinely required (e.g. the source is mutated in place later), keep it and add a one-line comment saying why.
---
## 5. Blocking the asyncio event loop / lock held across `await`
### Anti-pattern
```python
# BLOCKER-prone: blocking sleep / blocking HTTP / lock held across an await,
# all inside an async (event-loop) function
asyncdefhandle(self,req):
withself._lock:
resp=awaitself._client.call(req)# lock held across await -> serializes all requests
time.sleep(0.1)# stalls the single loop for every concurrent request
```
### Why dangerous
vLLM-Omni serves over a single asyncio event loop. Any synchronous blocking work on an `async def` path — `time.sleep`, blocking HTTP (`requests`/`urllib`), heavy CPU work, or a lock held across an `await` — stalls the loop for *every* concurrent request, not just the blocking one. A lock held across `await` silently serializes the whole pipeline; blocking I/O tanks throughput and inflates tail latency.
### Severity
- ⚠ any *new*`time.sleep(`, blocking HTTP call (`requests.get/post`, `urllib.request`, `urlopen`), or `await` inside a `with Lock` / `async with` critical section, in `async def` code.
- ✗ for `time.sleep` or blocking HTTP on the serving path (`engine/`, `entrypoints/`, async `connectors/`), or a lock acquired and held across an `await` on the request path.
### Detect (diff-scoped)
```bash
# New blocking primitives introduced by the diff — then eyeball whether each
# sits in an async function on the loop vs a dedicated worker thread:
Note: blocking inside a **dedicated worker thread is correct** — the repo already does this (`diffusion/model_loader/hub_prefetch.py`, `distributed/omni_connectors/kv_transfer_manager.py`, the `mooncake`/`mori` connectors poll with `time.sleep` on their own threads). The check targets the *event-loop* path only.
### Fix
Move blocking work off the loop with `asyncio.to_thread(...)` / `run_in_executor`, or use a native async client (`httpx.AsyncClient` / `aiohttp`). Never `await` while holding a lock — drop the lock before the `await` and re-acquire after, or restructure so the critical section contains no `await`.
---
## Running the sweep
Run all five detections against `${BASE}...HEAD`, then for each hit decide ⚠ vs ✗ using the severity rules above. Roll the results into the report as a single **Code quality** dimension row (count of ⚠ and ✗), alongside the type-specific checklist from [checklists.md](checklists.md).
description:Work on vLLM-Omni quantization for diffusion, autoregressive, omni, or multi-stage models. Use when choosing or adding methods such as fp8, int8, gguf, mxfp8, mxfp4, mxfp4_dualscale, ModelOpt, AutoRound, INC, msModelSlim, awq, or gptq; debugging quantized loading; or validating memory, speed, and output quality.
---
# vLLM-Omni Quantization
Use this skill for quantization work in `vllm-omni`. Start from the local
code and docs, then pick the smallest validated path for the model, method,
hardware, and quality target.
## First Checks
Before changing code or recommending a command, identify:
- Model and task: AR text, omni/TTS, text-to-image, text-to-video, or multi-stage diffusion.
- Hardware backend: CUDA generation, Ascend NPU, Intel XPU, or another platform.
- Quantization mode: online from BF16/FP16 weights, pre-quantized checkpoint, runtime KV/FA quantization, or per-component routing.
- Scope: transformer/DiT, thinker language model, text encoder, VAE, audio/vision encoder, talker, or a specific stage.
- Validation target: memory reduction, latency/throughput, output quality, or all three.
Do not assume a method is supported just because the CLI accepts the string.
Check `docs/user_guide/quantization/` and the closest implementation first.
## Quick Decision
| Task | Start With |
|------|------------|
| Choose a method or command | `references/methods.md` and `references/modality-compat.md` |
| Use `build_quant_config()` or per-component routing | `references/methods.md` |
| Work on diffusion quantization | `references/diffusion.md` |
| Add quantization to a new model | `references/adding-models.md` |
| Convert or load ModelOpt FP8 checkpoints | `references/modelopt-fp8.md` |
| Debug ModelOpt, GGUF, AutoRound, MXFP, or serialized Int8 loading | `references/diffusion.md` and method docs under `docs/user_guide/quantization/` |
component scoping, GGUF or ModelOpt checkpoint adapters, model-specific
prefix mapping, docs, examples, and validation.
If a new method needs missing generic kernels or loader behavior, fix upstream
`vllm` first. In `vllm-omni`, add thin integration and model wiring.
## Working Loop
1. Read the local method doc under `docs/user_guide/quantization/`.
2. Inspect the active factory and config class in `vllm_omni/quantization/`.
3. Inspect the target model pipeline and transformer for stable prefixes and
whether every relevant vLLM linear layer receives `quant_config`.
4. For pre-quantized checkpoints, inspect `transformer/config.json` and the
checkpoint tensor names before running full generation.
5. Validate with the same prompt, seed, size, scheduler, steps, dtype, eager or
non-eager mode, and parallelism as the BF16 baseline.
6. Report quality, latency, and memory separately. Do not call a method
supported until quality is checked.
## Common Mistakes
| Symptom | Likely Cause | Fix |
|---------|--------------|-----|
| `--quantization` has no visible effect | Wrong scope or unsupported model path | Check component routing and method docs |
| Some layers stay BF16 unexpectedly | `quant_config` was not threaded into all vLLM linear layers | Audit transformer constructors and prefixes |
| Quality collapses but loading succeeds | Too many sensitive layers were quantized | Add model-specific `ignored_layers` and compare to BF16 |
| ModelOpt checkpoint loads but output is corrupted | Prefixes, packed-module mapping, scale routing, or BF16 fallback is wrong | Use `references/modelopt-fp8.md` |
description:Review pull requests and local branches for vllm-project/vllm-omni with a frozen snapshot, module-design ownership, feature-design overlays, targeted validation, and concise evidence-backed findings. Use for default, detailed, or repeat maintainer reviews; checking correctness, compatibility, tests, benchmarks, model additions, distributed changes, or breaking behavior; and identifying or explicitly requesting the most relevant code-owner reviewers. Use precheck-pr instead for an author's pre-submit self-check.
---
# Review vLLM-Omni Pull Requests
Review like a maintainer: direct, selective, and focused on issues that CI does
not prove. Prefer a few high-confidence findings over exhaustive commentary.
Zero findings is a valid result.
## Quality contract
Make every finding:
- **Correct:** prove a reachable failure, not a suspicion.
- **Prioritized:** lead with merge blockers and high-impact defects.
- **Actionable:** identify the smallest safe fix direction.
- **Evidence-based:** cite code, tests, docs, CI, or measurements.
- **Concise:** avoid review templates and repeated summaries.
- **Calibrated:** match severity to user and maintainer impact.
Do not report unrelated backlog, style already enforced by pre-commit, or a
missing test that would not protect changed behavior. Do report new allowlist
or budget entries in `check_forbidden_imports.py` / `check_torch_cuda.py` /
`check_tts_adapter.py` / `check_buildkite.py` unless the PR justifies them:
those are policy changes, not lint noise.
## Select the input and depth
Use `vllm-project/vllm-omni` as the base repository. Accept its forks and local
checkouts; use another skill for unrelated repositories.
| Input | Review surface |
| --- | --- |
| PR number or URL | Frozen PR metadata, full diff, and relevant threads. |
| Local branch/worktree | Frozen target-base SHA through committed, staged, unstaged, and in-scope untracked changes. |
| Pre-filled context | Reuse supplied metadata; fetch only missing facts and the full diff. |
Default to maintainer brevity. A detailed or audit request expands coverage and
lists `path:line` findings, but keeps the same confidence and severity bar.
## Reference guide
Load references in review order: process, one primary module contract, matching
feature designs, evidence checks, then delivery. Every file is linked directly
For branch-specific behavior, inspect the matching `docs/` file in the reviewed
checkout first; use the published latest docs for current guidance and discovery.
If docs and live code disagree, verify the code/tests and report the drift.
### Review process
| Reference | Read when |
| --- | --- |
| [review-execution.md](references/process/review-execution.md) | Every review; freeze inputs, inspect safely, and deliver against the same snapshot. |
| [general-checks.md](references/process/general-checks.md) | Every review; apply repository-wide correctness and evidence rules. |
| [design-contracts.md](references/process/design-contracts.md) | Every production review; resolve branch-local module and feature design status. |
| [review-routing.md](references/process/review-routing.md) | After the diff census; select one primary module and conditional overlays. |
| [model-addition-checklist.md](references/checks/model-addition-checklist.md) | A model, architecture, loader, processor, registry, pipeline config, or deploy config is added. |
| [perf-verification.md](references/checks/perf-verification.md) | The PR makes a latency, throughput, memory, or quality claim. |
| [test-quality-evaluation.md](references/checks/test-quality-evaluation.md) | Tests change, are absent for risky code, or may not exercise production behavior. |
| [tests-docs-checklist.md](references/checks/tests-docs-checklist.md) | Coverage, CI markers, examples, user docs, or PR evidence need review. |
| [verification.md](references/checks/verification.md) | Hardware, a server, or a runnable affected path is available for active verification. |
| [examples-policy.md](../precheck-pr/references/examples-policy.md) | The PR adds, copies, or renames Python under `examples/`; apply the canonical policy shared with `precheck-pr`. |
| [find-simplifications](../find-simplifications/SKILL.md) | Every review; run a diff-scoped subtraction and simplification pass after correctness blockers. |
### Delivery and reviewer coordination
| Reference | Read when |
| --- | --- |
| [maintainer-style-study.md](references/delivery/maintainer-style-study.md) | Findings are ready for concise maintainer-style delivery. |
| [review-requests.md](references/delivery/review-requests.md) | The user asks to identify, suggest, request, or ping code-owner reviewers. |
## Workflow
### 1. Freeze and report the snapshot
Pin the base and head before reading source or running validation. Within 60 seconds,
report the pinned head, CI, mergeability, and preliminary findings in the host
conversation. Do not wait for CI or post this update to GitHub.
If the target changes while fetching, discard the evidence and retry once. If
it changes again, report the churn and wait for a stable target.
For a trusted PR head, materialize the pinned head in an isolated detached
worktree. A worktree freezes identity but is not a security sandbox. Treat fork
heads as untrusted unless the user and environment policy explicitly establish
otherwise: execute them only in a disposable, secret-free sandbox with restricted
filesystem, network, and resources; without one, use static SHA-addressed reads
and CI evidence only. For a local review, freeze the committed, index, worktree,
and NUL-safe in-scope untracked contents. Follow
[review-execution.md](references/process/review-execution.md) for trust gates,
state fingerprints, and byte-for-byte staleness checks.
### 2. Build the diff census
Group files into production code, tests, docs, configuration, build/CI, and
generated artifacts. Map each changed production file and test group to the PR
goal. Compare the title/body claims with the actual diff; use linked issues only
when they define the contract or reproduction.
Mark unrelated scope and unexplained generated artifacts. Do not infer behavior
from the PR description without tracing the live code.
### 3. Route from the live behavior
Trace each claimed behavior through the changed producer to its live consumer,
then use [design-contracts.md](references/process/design-contracts.md) and
[review-routing.md](references/process/review-routing.md) to select one primary
module contract, a second only for a real documented cross-boundary call path,
and every matching feature-design and evidence overlay. Treat titles and paths
as hints; live behavior and the frozen head's current design metadata are
authoritative. For docs-, tests-, or CI-only changes, route to the production
contract they protect or use only the applicable evidence checks.
### 4. Run the blocker scan
Apply every category in [general-checks.md](references/process/general-checks.md) before
lower-priority comments.
If the diff census contains an added, copied, or renamed Python path under
`examples/`, read and apply the canonical
[examples policy](../precheck-pr/references/examples-policy.md). Treat a new
model-specific Python example as blocking. Do not flag model-specific example
debt that the PR only modifies or removes, and do not run the rest of the
author-oriented `precheck-pr` workflow.
For each changed value or behavior, trace:
```text
public ingress -> validation/defaulting -> producer -> transformations
-> stage/worker/connector boundary -> final consumer -> terminal cleanup
```
Cover every applicable offline/online, streaming/non-streaming, sync/async,
feature-on/off, topology, and compatibility path. Search bounded callers and
sibling implementations rather than assuming the changed hunk is the only path.
### 5. Apply module and feature contracts
Apply the reference set selected in step 3 and any matching repo-local skill.
Read the exact module and feature pages in the frozen head, including status,
ownership boundary, dependencies, candidate invariants, safe-change guide, and
promotion gate. Candidate or draft rules are questions, not blockers, unless
current code, tests, or policy enforce them. Inspect both sides of any config,
registry, serialization, connector, cache, or stage boundary.
Read and apply [find-simplifications](../find-simplifications/SKILL.md) on every
review. Constrain it to the diff and the adjacent ownership, callers, or
consumers needed to prove a candidate. Check whether added or expanded helpers,
classes, state, fallback and compatibility branches, data movement, or public
behavior can be deleted, merged, moved, or inlined. Zero candidates is a valid
result. Do not widen the review into repository backlog or report speculative
style preferences as simplification findings.
### 6. Verify the changed path
Before each validation group, verify the frozen SHA plus the tracked, index,
untracked, and ignored-file fingerprint, or recreate a pristine snapshot. On a
trusted head or inside the required sandbox, run an import/version preflight,
then the narrowest relevant tests and low-cost static checks. Bind every result
to the head SHA, snapshot fingerprint, and environment fingerprint. Never run
imports, tests, builds, hooks, or repo-configurable tooling from an untrusted
head on the reviewer host.
- Treat CI as status evidence; inspect only the first overlapping failure.
- For docs-only changes, use diff hygiene, links/build checks, and bounded live
contract verification instead of dependency setup or pytest.
- For hardware-dependent paths, run available static/CPU checks and name the
exact GPU/NPU gap. Never simulate device evidence.
- For performance or accuracy claims, require comparable base/head runs with
the same environment, workload, warmup, repetitions, and quality criteria.
Stop when each changed semantic path has a supported finding or an explicit
no-issue conclusion. Do not search further only to increase confidence.
### 7. Consolidate and deliver
Verify each finding against the current diff, deduplicate by root cause, and
order by severity.
Re-read the remote head and reverify or recreate the pristine validation
snapshot immediately before delivery. If either changed, mark the review stale
and restart from the new snapshot.
Return findings first. Use
[maintainer-style-study.md](references/delivery/maintainer-style-study.md) to keep them
direct and brief. Each finding must include an exact `path:line`, trigger or
call path, current behavior, impact, and smallest fix direction. If there are
no findings, say so briefly and name material validation gaps.
Keep the review read-only unless the user explicitly authorizes posting. Do not
submit `APPROVE`, `COMMENT`, or `REQUEST_CHANGES`, add labels, edit code, or push
commits as an implied part of review.
### 8. Optionally request focused owner reviews
Only when the user asks to identify or request reviewers, read
Use with the OmniConnector, stage-runtime, configuration, and I/O module
contracts. Load the concrete backend design from the reviewed head:
| Backend | Design |
| --- | --- |
| Mooncake store | [MooncakeStoreConnector](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/omni_connectors/mooncake_store_connector/) |
| Mooncake transfer engine | [MooncakeTransferEngineConnector](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/omni_connectors/mooncake_transfer_engine_connector/) |
| Mori transfer engine | [MoriTransferEngineConnector](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/omni_connectors/mori_transfer_engine_connector/) |
Complete every applicable summary, use-case, reference, hardware,
environment, exact-command, verification, and notes section with tested,
reproducible details; explain any intentional deviation in the PR.
10. Require every new model to verify its supported modalities, platforms,
execution modes, and features with production-path evidence. Update
`docs/models/supported_models.md` and every applicable feature support or
compatibility table; do not claim support that the PR does not validate.
Follow
[model-addition-checklist.md](../checks/model-addition-checklist.md) for the
model-specific documentation checks.
11. Require every new model PR to add or update its in-repository model-family
recipe under `recipes/` and its `recipes/README.md` index entry. Apply rule
9, and keep the recipe's model identifiers, tasks, serving modes, hardware,
commands, supported features, and limitations aligned with validated PR
evidence. An external recipe link does not replace the in-repository recipe.
12. Require every new model to compare accuracy or output quality and performance
against a pinned baseline or canonical reference implementation. Match the
checkpoint, inputs, precision, hardware, workload, and measurement method;
provide exact commands, results, and explanations for material differences.
Prefer a detailed timing split for each stage and explain when it is
unavailable. Follow
[model-addition-checklist.md](../checks/model-addition-checklist.md) and
[perf-verification.md](../checks/perf-verification.md) for the evidence
contract.
## Blocking risk scan
| Risk | Prove before reporting |
| --- | --- |
| Correctness | A live input reaches a wrong output, exception, silent drop, shape/dtype/device mismatch, or partially initialized state. |
| Compatibility | A supported API, config key, default, serialization schema, model identifier, or caller breaks without validation, migration, or deprecation. |
| Lifecycle | Allocation or startup lacks cleanup on partial failure, timeout, cancellation, shutdown, or repeated requests. |
| Concurrency/distribution | Ordering, identity, rank, world size, completion, timeout, backpressure, or collective assumptions fail on a supported path. |
| Cache/shared state | Identity omits a correctness-affecting input, or isolation, invalidation, disabled behavior, eviction, or cleanup is incomplete. |
| Async behavior | Blocking I/O, sleep, a lock across `await`, or unnecessary serialization harms a reachable async path. |
| Security/data handling | Secrets, user payloads, unsafe deserialization/eval/shell, unvalidated paths, or unbounded metric labels are reachable. |
| Validation evidence | Tests bypass the production dispatcher, mock away the change, use unrealistic types/MRO, or cannot fail when the defect returns. |
| User contract | User-visible model, feature, API, CLI, config, default, or compatibility behavior changes without accurate docs or examples. |
Search bounded callers and sibling implementations before reporting dead code,
duplication, or a missed consumer. Keep new abstractions only when they have a
distinct live caller, invariant, or compatibility need.
## Finding bar
Anchor each finding to a changed `path:line`; name the trigger or call path,
current behavior, impact, and smallest fix direction. Treat pending CI, missing
hardware, or unsupported measurements as validation gaps unless the repository
`MAX_MODEL_TYPE_BRANCHES`, or Buildkite `SKIP_FILES` is a policy change.
Do not rubber-stamp allowlist/budget growth; require justification and
prefer fixing the call site.
- Treat a failed gate as its own evidence. Do not restate its formatting/lint
output as a new code-review finding.
- Open CI logs only when the first failing step overlaps the frozen diff or
blocks the verdict. Start with the first error, not the last cascade.
- Treat the PR body as navigation. Commands, benchmark tables, and claimed tests
become evidence only after their provenance and relevance are checked.
## Run bounded validation
Keep one evidence packet for files read, bounded searches, callers, tests, CI,
hardware, routes, and findings. Reuse it rather than refetching facts.
Before pytest, run a short import/version compatibility check. Record each
validation result with:
```text
repo, head SHA, command, result, Python/platform, dependency or lock fingerprint
```
After preflight, run targeted tests and low-cost static checks alongside source
inspection when possible. Imports, tests, builds, and linters may create caches
or rewrite files, so isolate each potentially mutating group in a disposable
snapshot or recreate the pinned snapshot before continuing. Map source symbols
to tests with bounded `rg` searches rather than assuming the test directory
mirrors production paths.
Classify failures as code, test, infrastructure, or flaky before reporting.
Skipped hardware tests are gaps, not passes. For available hardware, verify the
smallest representative unit/E2E path and compare actual output with PR claims.
## Deliver maintainer-style findings
Default to roughly 1-5 short comments, but treat that as calibration rather than
a quota: report every real blocker and report no finding when there is no issue.
Write each finding as:
```text
[P1] Short imperative title — path/to/file.py:<line>
<Trigger or call path>. <Current behavior and impact>. <Smallest fix direction>.
```
Use [maintainer-style-study.md](../delivery/maintainer-style-study.md) for
comment tone.
Keep rule IDs, grades, and audit matrices internal unless the user asks for the
complete audit.
Verify every inline `path:line` against the frozen diff before delivery. For a
PR, re-read the remote head and byte-compare the detached snapshot fingerprint,
or recreate it from `head_sha`, before asserting current lines. For a local
review, recompute and byte-compare the frozen `HEAD`, target, merge base, status,
index patch, worktree patch, and untracked-content manifest. Any mismatch makes
the review stale; discard the affected evidence and restart. Prefer one
root-cause comment to several symptom comments.
Local presentation is the default. Only explicit authorization permits GitHub
posting. When authorized, publish one consolidated final review after
validation; do not post preliminary or incremental comments. Submit a review
event (`APPROVE`, `COMMENT`, or `REQUEST_CHANGES`) only when the user explicitly
chooses that event.
Reviewer requests and owner `@mention` comments are separate external writes;
follow [review-requests.md](../delivery/review-requests.md) when the user asks
for them.
## Re-review safely
Freeze the new head and compare it with the previously reviewed SHA. Then:
1. inspect the delta and every file affected by conflict resolution or rebase;
2. revalidate earlier findings against current lines and behavior;
3. read unresolved, non-outdated threads and the author's evidence;
4. rerun only checks invalidated by the delta;
5. look for new regressions introduced by the fix;
6. do not repeat resolved or outdated comments.
If the target head changes during re-review, discard stale validation and start
again from the new snapshot.
Loaded 100 of 3180 files, more files were not shown because too many files have changed in this diff.
Show more
Reference in new issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Hongsheng Liu