[Misc] Add vLLM-Omni PR review skill (#5871)
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
This commit is contained in:
1 parent
81b48e83ec
commit
f663d120c7
33 files changed
+1639
No files matched your search
@@ -0,0 +1,231 @@
|
||||
---
|
||||
name: review-pr
|
||||
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.
|
||||
|
||||
## 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
|
||||
below; do not load unrelated module references.
|
||||
|
||||
Each concise reference links to the maintained
|
||||
[vLLM-Omni documentation](https://docs.vllm.ai/projects/vllm-omni/en/latest/).
|
||||
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. |
|
||||
|
||||
### Primary module contract
|
||||
|
||||
| Reference | Read when |
|
||||
| --- | --- |
|
||||
| [entrypoints.md](references/modules/entrypoints.md) | Offline/CLI/API ingress, validation, rendering, streaming, or sessions change. |
|
||||
| [configuration.md](references/modules/configuration.md) | Config construction, deploy/stage schema, defaults, registry, or topology changes. |
|
||||
| [input-output-modality.md](references/modules/input-output-modality.md) | Requests, messages, serialization, output types, accumulation, or completion change. |
|
||||
| [error-contracts.md](references/modules/error-contracts.md) | Error classification, fatality, propagation, sanitization, or rendering changes. |
|
||||
| [engine-orchestration.md](references/modules/engine-orchestration.md) | Cross-stage routing, request state, output ordering, RPC correlation, or terminal convergence changes. |
|
||||
| [stage-runtime.md](references/modules/stage-runtime.md) | Placement, startup, readiness, replica identity, affinity, membership, or shutdown changes. |
|
||||
| [omni-connector.md](references/modules/omni-connector.md) | Cross-stage/process/device/node transport or synchronization changes. |
|
||||
| [model-integration.md](references/modules/model-integration.md) | Registration, preprocessing, loading, runners, or model-specific execution changes. |
|
||||
| [ar-runtime.md](references/modules/ar-runtime.md) | AR scheduling, request/cache state, adapters, workers, or upstream vLLM semantics change. |
|
||||
| [diffusion.md](references/modules/diffusion.md) | Diffusion runtime, models, batching, parallelism, or offload changes. |
|
||||
| [execution-platforms.md](references/modules/execution-platforms.md) | Hardware selection, capabilities, vendor workers, kernels, or patches change. |
|
||||
| [cache-management.md](references/modules/cache-management.md) | Cache identity, reuse, validity, reset, eviction, or teardown changes. |
|
||||
| [quantization.md](references/modules/quantization.md) | Quantization selection, checkpoint metadata, layer mapping, precision, or constraints change. |
|
||||
| [observability.md](references/modules/observability.md) | Metrics, logs, units, labels, correlation, or lifecycle changes. |
|
||||
| [profiling.md](references/modules/profiling.md) | Profiling instrumentation, traces, start/stop lifecycle, or overhead changes. |
|
||||
| [benchmarking.md](references/modules/benchmarking.md) | Benchmark workload, metric calculation, CLI, or result metadata changes. |
|
||||
|
||||
### Feature-design overlays
|
||||
|
||||
| Reference | Read when |
|
||||
| --- | --- |
|
||||
| [runtime-stage-execution.md](references/features/runtime-stage-execution.md) | Disaggregated inference, async chunk/output/materialization, or prefix caching changes. |
|
||||
| [communication.md](references/features/communication.md) | A concrete OmniConnector backend or its deployment contract changes. |
|
||||
| [diffusion-acceleration.md](references/features/diffusion-acceleration.md) | Diffusion parallelism, attention, quantization, cache, batching, or offload changes. |
|
||||
| [infrastructure-performance.md](references/features/infrastructure-performance.md) | Metrics infrastructure or documented speech optimization stacks change. |
|
||||
|
||||
### Evidence and quality checks
|
||||
|
||||
| Reference | Read when |
|
||||
| --- | --- |
|
||||
| [model-addition-checklist.md](references/checks/model-addition-checklist.md) | A model, architecture, loader, processor, registry, or stage 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. |
|
||||
|
||||
### 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.
|
||||
|
||||
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.
|
||||
|
||||
When a diff adds or expands a helper, class, fallback, compatibility branch, or
|
||||
public behavior, run a subtraction pass: remove out-of-scope behavior and check
|
||||
whether each new abstraction can be deleted, merged, moved, or inlined.
|
||||
|
||||
### 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
|
||||
[review-requests.md](references/delivery/review-requests.md). Rank path-matched
|
||||
CODEOWNERS with the frozen module page's owners or required reviewers and
|
||||
documented governance expertise; propose one to three focused reviewers with an
|
||||
explicit contract rationale.
|
||||
|
||||
Identifying or suggesting reviewers is read-only. Requesting reviewers or
|
||||
posting `@mention` comments changes external state and requires explicit user
|
||||
authorization. When authorized, recheck the head, deduplicate existing
|
||||
requests, and post at most one consolidated comment. Do not infer this
|
||||
permission from a request to review the code.
|
||||
@@ -0,0 +1,4 @@
|
||||
interface:
|
||||
display_name: "vLLM-Omni PR Review"
|
||||
short_description: "Review vLLM-Omni PRs and route owner requests"
|
||||
default_prompt: "Use $review-pr to review this vLLM-Omni pull request with owner-specific rules and evidence-backed findings."
|
||||
@@ -0,0 +1,121 @@
|
||||
# Model Addition Checklist
|
||||
|
||||
Read this reference when a PR adds or expands a model architecture, pipeline,
|
||||
loader, processor, registry entry, or stage configuration.
|
||||
|
||||
Official docs: [model contribution guides](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/model/),
|
||||
[adding an omni model](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/model/adding_omni_model/),
|
||||
[adding a diffusion model](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/model/adding_diffusion_model/),
|
||||
[adding a TTS model](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/model/adding_tts_model/),
|
||||
[supported models](https://docs.vllm.ai/projects/vllm-omni/en/latest/models/supported_models/),
|
||||
[diffusion features](https://docs.vllm.ai/projects/vllm-omni/en/latest/user_guide/diffusion_features/),
|
||||
and [feature compatibility](https://docs.vllm.ai/projects/vllm-omni/en/latest/user_guide/feature_compatibility/).
|
||||
|
||||
## Match claims to the diff
|
||||
|
||||
- Reconcile every claimed architecture, modality, variant, feature, and example
|
||||
with changed files and registry entries.
|
||||
- Resolve config model types, architecture names, callable paths, exports, and
|
||||
dependency extras against live symbols.
|
||||
- Check sibling generations sharing an architecture name are disambiguated by
|
||||
a stable config predicate rather than import order or an accidental default.
|
||||
|
||||
## Close the integration chain
|
||||
|
||||
Trace:
|
||||
|
||||
```text
|
||||
public model id -> config/pipeline selection -> registry -> loader/processor
|
||||
-> stage inputs -> model execution -> stage/public output
|
||||
```
|
||||
|
||||
Verify optional dependencies fail with an actionable message, weight names and
|
||||
dtypes map correctly, and each advertised serving mode reaches the production
|
||||
dispatcher. Confirm outputs are non-empty and valid for their modality, shape,
|
||||
sample rate, or response schema.
|
||||
|
||||
## Keep support documentation truthful
|
||||
|
||||
- Build an evidence-backed inventory of the new model's exact architecture and
|
||||
checkpoint identifiers, modalities and tasks, hardware backends, serving
|
||||
modes, and supported acceleration, parallelism, cache, offload, and
|
||||
quantization features. Treat untested support as unknown, not supported.
|
||||
- Update `docs/models/supported_models.md` with the architecture, model family,
|
||||
example checkpoints, and only the backends validated by the PR.
|
||||
- For a diffusion model, add it to the applicable ImageGen, VideoGen, or
|
||||
AudioGen table in `docs/user_guide/diffusion_features.md`. Fill every feature
|
||||
column according to its legend and document partial support or constraints.
|
||||
- Update `docs/user_guide/feature_compatibility.md` when the model changes valid
|
||||
feature combinations, configuration constraints, examples, or limitations,
|
||||
and update any other branch-local support table affected by the claims.
|
||||
- Cross-check every documented support mark against the registry, configuration,
|
||||
production-path tests, and PR evidence; do not copy a sibling model's support
|
||||
claims without validation.
|
||||
|
||||
## Ship the model recipe
|
||||
|
||||
- Require every new model PR to add or update one model-family recipe under
|
||||
`recipes/<vendor>/` and its row in the `recipes/README.md` Available Recipes
|
||||
table. Follow [`recipes/TEMPLATE.md`](../../../../../recipes/TEMPLATE.md) and
|
||||
the layout rules in
|
||||
[`recipes/README.md`](../../../../../recipes/README.md); the
|
||||
external recipes repository is a structural reference, not a substitute.
|
||||
- Include only tested task, serving-mode, and hardware configurations. Keep the
|
||||
exact model identifiers, flags, commands, verification, feature support,
|
||||
constraints, and limitations consistent with code, tests, examples, and the
|
||||
support documentation.
|
||||
- Cover each validated platform in its own hardware section. Use one recipe per
|
||||
model family by default; justify a split when configurations cannot be kept
|
||||
clear in one file, and do not add placeholder sections for untested platforms.
|
||||
|
||||
## Compare the reference implementation
|
||||
|
||||
Require every new model PR to include a head-versus-reference accuracy or
|
||||
quality and performance comparison:
|
||||
|
||||
- Name the canonical implementation and pin its repository revision, package
|
||||
version, configuration, and checkpoint. If it cannot run, explain why and use
|
||||
the closest runnable reference; do not silently omit the comparison.
|
||||
- Match inputs or dataset, seed, output settings, precision or quantization,
|
||||
hardware, software, warmup, repetitions, synchronization, and timing and
|
||||
memory scope. Include exact commands and environment details for both sides.
|
||||
- Compare accuracy or output quality with a task-appropriate metric and explicit
|
||||
tolerance. When no reliable metric exists, provide paired fixed-input outputs,
|
||||
artifacts, and evaluation criteria, and explain expected numerical or
|
||||
qualitative differences.
|
||||
- Report end-to-end latency, task-appropriate throughput, peak device memory,
|
||||
run-to-run variability, and head/reference ratios at equivalent output quality.
|
||||
- Prefer a timing split for preprocessing or encoding, each model stage,
|
||||
inter-stage transfer or communication, decoding or postprocessing, and output
|
||||
assembly. Define the boundaries, account for overlap, reconcile the split with
|
||||
end-to-end time, and explain any unavailable stage measurement.
|
||||
|
||||
Use [perf-verification.md](perf-verification.md) for the comparison protocol and
|
||||
repository benchmark and profiling links.
|
||||
|
||||
## Remove accidental surface
|
||||
|
||||
Search bounded call sites for:
|
||||
|
||||
- inference-dead training `forward()` paths, unused factories/wrappers, never-set
|
||||
branch keys, and immediately discarded parameters;
|
||||
- duplicate payload strings, validation, or shape coercion across stages;
|
||||
- compatibility aliases in brand-new code without an existing caller;
|
||||
- private symbols re-exported as public API or module-level side effects.
|
||||
|
||||
Keep code only when it has a distinct live caller, invariant, or compatibility
|
||||
contract. Prefer one typed producer-consumer schema to repeated string keys.
|
||||
|
||||
## Require proportionate evidence
|
||||
|
||||
- Run a representative production-path inference and assert output content, not
|
||||
only process survival.
|
||||
- Require profiling or A/B tables when the PR makes performance, memory,
|
||||
precision, or quality claims, or when a suspected hot-path/device bug needs
|
||||
that evidence. Do not impose fixed utilization or regression thresholds that
|
||||
the repository does not define.
|
||||
- Require focused registry/config/loading tests, a representative E2E path, and
|
||||
user-facing model/usage documentation.
|
||||
|
||||
Use [diffusion.md](../modules/diffusion.md) for diffusion models and
|
||||
[perf-verification.md](perf-verification.md) for quantitative claims.
|
||||
@@ -0,0 +1,51 @@
|
||||
# Performance and Accuracy Verification
|
||||
|
||||
Activate this reference when the PR adds a model or claims or intentionally
|
||||
changes latency, throughput, memory, scaling, precision, or output quality.
|
||||
|
||||
Official docs: [profiling](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/profiling/),
|
||||
[serving benchmarks](https://docs.vllm.ai/projects/vllm-omni/en/latest/cli/bench/serve/),
|
||||
and [metrics](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/metrics/).
|
||||
|
||||
## Comparable A/B contract
|
||||
|
||||
Compare frozen base and head for an existing path. For a new model absent from
|
||||
base, compare head with a pinned canonical reference implementation. Use the same:
|
||||
|
||||
- hardware, driver, software versions, model/checkpoint, and dependencies;
|
||||
- input or dataset, seed, precision, batch/concurrency, topology, and feature
|
||||
flags;
|
||||
- warmup, measured repetitions, synchronization, timing scope, and memory
|
||||
collection method.
|
||||
|
||||
Keep exact commands and report variability rather than a cherry-picked run.
|
||||
Use the repository's benchmark when it covers the claim; otherwise use the
|
||||
smallest reproducible workload that reaches the changed production path.
|
||||
|
||||
## Evidence table
|
||||
|
||||
| Dimension | Typical evidence |
|
||||
| --- | --- |
|
||||
| Latency | End-to-end latency and, when available, TTFT plus a defined per-stage and transfer-time split. |
|
||||
| Throughput/scaling | Requests, tokens, frames, or audio duration per second across the claimed concurrency/topology. |
|
||||
| Memory | Peak allocated/reserved device memory and OOM boundary if relevant. |
|
||||
| Quality/accuracy | Repository metric or known-good output comparison with an explicit tolerance; paired samples when no metric exists. |
|
||||
|
||||
Every optimization needs correctness or quality evidence. A faster result on a
|
||||
different workload, precision, seed, or topology is not a valid comparison.
|
||||
|
||||
## Reviewer verification ladder
|
||||
|
||||
1. Run base/head A/B on suitable hardware when affordable.
|
||||
2. If only one side can run, verify the script and request a comparable pair.
|
||||
3. If hardware or assets are unavailable, audit methodology, code path, and
|
||||
contributor evidence; name the exact unverified claim.
|
||||
|
||||
Classify discrepancies before reporting: implementation regression, benchmark
|
||||
bug, environmental drift, noise, or unsupported claim. Explain material
|
||||
regressions against the PR's stated goal or repository contract; do not invent
|
||||
universal percentage thresholds.
|
||||
|
||||
Report claimed and measured values together, bind them to the frozen SHAs and
|
||||
environment, and keep unavailable hardware as a validation gap rather than a
|
||||
fabricated pass.
|
||||
@@ -0,0 +1,39 @@
|
||||
# Test Quality Evaluation
|
||||
|
||||
Read this reference when tests change, a risky production path has no test, or
|
||||
a test-only PR may not prove the intended behavior.
|
||||
|
||||
Official docs: [test system overview](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/ci/test_system_overview/)
|
||||
and [test writing guide](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/ci/test_writing_guide/).
|
||||
|
||||
## Static proof check
|
||||
|
||||
For each changed semantic path, ask whether the test would fail if that behavior
|
||||
were reverted or broken. Check:
|
||||
|
||||
- assertions pin the contract, not only non-null output, logs, or “no crash”;
|
||||
- the production dispatcher, registry, connector, or scheduler is reached
|
||||
instead of the changed behavior being mocked away;
|
||||
- fakes preserve relevant types, MRO, shapes, devices, async behavior, and
|
||||
lifecycle transitions;
|
||||
- seeds, ordering, timing, synchronization, external services, and numeric
|
||||
tolerances are deterministic or explicitly controlled;
|
||||
- normal, invalid, boundary, feature-off, failure/cancellation, and regression
|
||||
paths are covered where the diff changes them;
|
||||
- run-level and domain markers place hardware/model tests in the intended CI
|
||||
lane without silently skipping the contract.
|
||||
|
||||
Map source symbols to tests with bounded `rg` searches; do not assume the test
|
||||
tree mirrors production paths.
|
||||
|
||||
## Runtime check
|
||||
|
||||
Run the narrowest affected tests supported by the environment. Record skipped
|
||||
hardware/model cases as gaps. Classify failures as code, test, infrastructure,
|
||||
or flaky before turning them into findings; one passing rerun does not erase a
|
||||
flakiness signal.
|
||||
|
||||
Keep grades or matrices internal. Report only concrete code bugs or the one or
|
||||
two test defects that leave changed behavior materially unprotected. Use
|
||||
[tests-docs-checklist.md](tests-docs-checklist.md) for CI and documentation
|
||||
coverage.
|
||||
@@ -0,0 +1,48 @@
|
||||
# Tests and Documentation Checklist
|
||||
|
||||
Use this reference to connect changed behavior to coverage, CI selection,
|
||||
examples, documentation, and reproducible PR evidence.
|
||||
|
||||
Official docs: [test writing guide](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/ci/test_writing_guide/),
|
||||
[test execution guide](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/ci/test_execution_guide/),
|
||||
and [documentation guide](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/DOCS_GUIDE/).
|
||||
|
||||
## Coverage packet
|
||||
|
||||
For each high-risk change, record a compact internal packet:
|
||||
|
||||
```text
|
||||
change and failure risk -> existing unit/E2E coverage -> uncovered boundary
|
||||
-> smallest test and stable assertion that closes the gap
|
||||
```
|
||||
|
||||
Prefer a regression test that fails on the frozen base for bug fixes. Check
|
||||
numeric tolerances, deterministic inputs, realistic mocks, hardware/domain
|
||||
markers, and Buildkite/CI wiring using the documentation present on the reviewed
|
||||
branch. A skipped test is not a pass.
|
||||
|
||||
## Documentation sync
|
||||
|
||||
Require user-facing updates when the diff changes a model, feature, CLI/API,
|
||||
config key, default, compatibility behavior, or supported platform. Verify:
|
||||
|
||||
- the actual docs navigation and support tables used by the branch;
|
||||
- exact identifiers, defaults, constraints, known limitations, and migration;
|
||||
- a minimal runnable example for newly exposed behavior;
|
||||
- examples and docs agree on commands, parameters, output schema, and supported
|
||||
modes;
|
||||
- links and the bounded live contract build or resolve correctly.
|
||||
|
||||
Do not request docs for internal behavior with no user-visible contract.
|
||||
|
||||
## PR evidence
|
||||
|
||||
For contributor-run tests or benchmarks, require enough provenance to evaluate
|
||||
the claim: hardware, relevant software versions, exact commands, inputs/model,
|
||||
result, and approximate runtime. Performance or accuracy evidence must compare
|
||||
base and head under equivalent conditions; use
|
||||
[perf-verification.md](perf-verification.md).
|
||||
|
||||
Turn a missing test or document into a finding only when it protects or explains
|
||||
changed behavior. Name the smallest useful addition rather than requesting a
|
||||
broad coverage expansion.
|
||||
@@ -0,0 +1,36 @@
|
||||
# Active Verification
|
||||
|
||||
Read this reference when a runnable affected path and suitable local or remote
|
||||
hardware are available. Never claim device evidence from static analysis or a
|
||||
simulated backend.
|
||||
|
||||
Official docs: [installation](https://docs.vllm.ai/projects/vllm-omni/en/latest/getting_started/installation/)
|
||||
and [test execution guide](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/ci/test_execution_guide/).
|
||||
|
||||
## Choose the narrowest level
|
||||
|
||||
| Available environment | Verification |
|
||||
| --- | --- |
|
||||
| Matching accelerator, model/assets, and server | Targeted unit/integration test plus one representative E2E request. |
|
||||
| Matching accelerator without serving setup | Targeted model/runtime test and output or metric inspection. |
|
||||
| CPU/static environment only | Import/version preflight, focused CPU tests, config/registry resolution, and static contract checks. |
|
||||
| Missing dependencies or inaccessible hardware | Record the exact gap and evaluate contributor/CI evidence. |
|
||||
|
||||
## Execute safely
|
||||
|
||||
1. Bind the run to the frozen head SHA and record Python, platform, dependency,
|
||||
model, device, and relevant environment fingerprints.
|
||||
2. Run the smallest affected unit test before an E2E workload.
|
||||
3. Exercise the changed public or production-dispatch path with bounded inputs.
|
||||
4. Compare actual output, metrics, cleanup, and exit behavior with the PR claim.
|
||||
5. For base/head claims, use the equivalent A/B protocol in
|
||||
[perf-verification.md](perf-verification.md).
|
||||
|
||||
Avoid disturbing shared servers, ports, models, or accelerators without user
|
||||
authorization. Do not install large dependencies, download gated assets, or
|
||||
start remote workloads merely because hardware might exist.
|
||||
|
||||
Report the command, environment, result, and any skipped path. Classify failures
|
||||
as change-induced, pre-existing, infrastructure, environment, or flaky before
|
||||
making a finding. A hardware gap lowers confidence; it is not automatic proof
|
||||
of a defect.
|
||||
@@ -0,0 +1,105 @@
|
||||
# Maintainer Review Style
|
||||
|
||||
Use this reference only when turning proved findings into comments. The baseline
|
||||
maintainer samples favor short, direct comments and reserve long explanations
|
||||
for architecture or non-obvious failure paths.
|
||||
|
||||
Official docs: [contributing and review process](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/).
|
||||
|
||||
## Evidence basis
|
||||
|
||||
This calibration used 928 PRs merged from 2026-05-10 through 2026-08-08.
|
||||
Candidates were ranked by substantive human inline-review threads, then by
|
||||
distinct reviewers and review rounds. Author self-comments, bots and AI
|
||||
reviewers, and bodyless approvals were excluded. Counts selected the sample;
|
||||
the practices below came from reading root comments and follow-up replies.
|
||||
|
||||
The main set contains the six leaders after filtering; three other high-ranked
|
||||
PRs add core-runtime and benchmark coverage. All were merged.
|
||||
|
||||
| PR | Review boundary |
|
||||
| --- | --- |
|
||||
| [#2906](https://github.com/vllm-project/vllm-omni/pull/2906) | New TTS model, shared utilities, recipes, and request state. |
|
||||
| [#3907](https://github.com/vllm-project/vllm-omni/pull/3907) | Full-duplex API, session lifecycle, concurrency, and realtime evidence. |
|
||||
| [#2162](https://github.com/vllm-project/vllm-omni/pull/2162) | Diffusion world model, OpenPI serving, tests, examples, and docs. |
|
||||
| [#3454](https://github.com/vllm-project/vllm-omni/pull/3454) | Diffusion model, generic CLI surface, loaders, and configuration scope. |
|
||||
| [#4425](https://github.com/vllm-project/vllm-omni/pull/4425) | Structured configuration, typing, defaults, and compatibility. |
|
||||
| [#5397](https://github.com/vllm-project/vllm-omni/pull/5397) | Distributed offload, collectives, failure handling, and load semantics. |
|
||||
| [#4281](https://github.com/vllm-project/vllm-omni/pull/4281) | Composable parallelism, configuration contracts, and examples. |
|
||||
| [#3628](https://github.com/vllm-project/vllm-omni/pull/3628) | Stage metrics, benchmark definitions, opt-in API behavior, and cleanup. |
|
||||
| [#3855](https://github.com/vllm-project/vllm-omni/pull/3855) | Stage runtime, thread safety, startup, and compatibility. |
|
||||
|
||||
Historical comments calibrate delivery; they do not create merge policy. Live
|
||||
code, tests, repository docs, and the current `CODEOWNERS` file remain
|
||||
authoritative. When refreshing this study, promote a practice only when it
|
||||
recurs across at least three PRs or is supported by current repository guidance.
|
||||
|
||||
## Recurring review behavior
|
||||
|
||||
- Start from a changed contract, not a style preference. State the exact
|
||||
request, configuration, topology, or failure path that triggers the issue.
|
||||
- Protect repository boundaries. Keep model-specific behavior in its domain,
|
||||
move genuinely reusable logic to a shared module, and reuse existing loaders,
|
||||
protocols, definitions, and config sources instead of bypassing them.
|
||||
- Challenge unrelated scope and abstractions. Ask why a new flag, config, class,
|
||||
helper, fallback, or file is needed when an existing path appears sufficient.
|
||||
- Trace concurrency and terminal paths: overlapping requests, disconnect,
|
||||
cancellation, partial failure, stale replies, collective schedule mismatch,
|
||||
cleanup, and retry. Prefer an explicit startup error for unsupported
|
||||
combinations over silently running with incomplete state or wrong weights.
|
||||
- Preserve user-visible behavior across variants. An accepted CLI/API/config
|
||||
field must be applied, explicitly rejected, or visibly warned about; compare
|
||||
normal and headless, offline and online, and streaming and non-streaming paths.
|
||||
- Ask for evidence at the affected boundary: a regression test using the real
|
||||
tensor or protocol shape, correct pytest markers so CI runs it, comparable A/B
|
||||
performance results, or an SLO-oriented latency/quality measurement.
|
||||
- Keep interfaces explicit: avoid `Any` and opaque strings where a precise
|
||||
union, enum, or typed envelope exists; remove dead code and duplicated
|
||||
constants; validate requests before expensive execution.
|
||||
- Document module scope and non-obvious invariants. Update examples, recipes,
|
||||
supported-model/feature docs, and hardware assumptions when users rely on
|
||||
them.
|
||||
- When expertise crosses an ownership boundary, ask a named owner to inspect a
|
||||
specific file or contract and explain why. Do not send a generic `PTAL` list.
|
||||
- Keep non-blocking cleanup in a linked follow-up only after the current PR is
|
||||
safe. If a revision fixes only part of a failure chain, continue the existing
|
||||
thread with the remaining trigger instead of opening a duplicate finding.
|
||||
|
||||
## Write the finding
|
||||
|
||||
- Lead with the issue or decisive question; omit a review preamble.
|
||||
- Name the concrete trigger and impact, then the smallest fix direction.
|
||||
- Keep obvious fixes to one sentence. Use more detail only when the call path or
|
||||
ownership boundary is otherwise unclear.
|
||||
- Prefer one root-cause comment over repeated symptom comments.
|
||||
- Hedge only when evidence is genuinely incomplete; mark that item as a
|
||||
question or validation gap rather than overstating severity.
|
||||
- Say briefly that no findings were found when the review is clean; zero
|
||||
comments is a valid outcome when posting is authorized.
|
||||
|
||||
Useful forms:
|
||||
|
||||
```text
|
||||
This leaves <state> allocated when <failure/cancellation path>. Release it in
|
||||
the shared terminal cleanup path.
|
||||
```
|
||||
|
||||
```text
|
||||
Does this producer also update <consumer>? It still reads the old field here.
|
||||
```
|
||||
|
||||
```text
|
||||
With <configuration>, this bypasses <existing loader/validator>, so <impact>.
|
||||
Reuse that path or reject this combination before startup.
|
||||
```
|
||||
|
||||
```text
|
||||
@<owner>, could you check <specific boundary>? This changes <contract> used by
|
||||
<consumer>.
|
||||
```
|
||||
|
||||
Use a priority label only for a proved high-impact defect. Avoid generic praise,
|
||||
routine “Nit:” prefixes, decorative severity labels, audit templates, rule IDs,
|
||||
comment-count narration, and “I left a few comments.” Keep approvals, review
|
||||
events, reviewer requests, mentions, and GitHub replies subject to the user's
|
||||
explicit authorization.
|
||||
@@ -0,0 +1,99 @@
|
||||
# Focused Code-Owner Review Requests
|
||||
|
||||
Read only when the user asks to identify, suggest, request, add, or ping
|
||||
reviewers. Finding candidates is read-only; GitHub review requests and
|
||||
`@mention` comments are external writes.
|
||||
|
||||
Owner sources, in order:
|
||||
|
||||
1. the reviewed head's
|
||||
[CODEOWNERS](https://github.com/vllm-project/vllm-omni/blob/main/.github/CODEOWNERS)
|
||||
for path ownership;
|
||||
2. the active module design page's `owners` and `required_reviewers` metadata
|
||||
for contract ownership and mandatory design review; and
|
||||
3. documented
|
||||
[governance expertise](https://docs.vllm.ai/projects/vllm-omni/en/latest/community/governance/)
|
||||
for specialization and tie-breaking.
|
||||
|
||||
Resolve all three from the reviewed head when available because ownership
|
||||
changes over time. Use published docs only to discover a page absent from an
|
||||
older checkout, never to override the frozen head, and do not copy a static
|
||||
owner list into review output. CODEOWNERS remains the path authority; module
|
||||
metadata identifies contract experts and required design reviewers.
|
||||
|
||||
## Rank candidates
|
||||
|
||||
1. Freeze the PR head and collect changed files, author, requested reviewers,
|
||||
submitted reviews, and existing comments.
|
||||
2. Resolve the **last matching CODEOWNERS rule** for each changed file. Do not
|
||||
union an earlier broad rule with a later specific rule.
|
||||
3. Resolve the primary module and feature overlays through
|
||||
[design-contracts.md](../process/design-contracts.md). Read the active module
|
||||
page's status, `owners`, and `required_reviewers`; do not treat owners on an
|
||||
archived, superseded, or unrelated page as current authority.
|
||||
4. Group files by the selected primary owner and the exact module or feature
|
||||
contract needing review. Weight production owners and live consumers above
|
||||
tests/docs/CI owners and the repository-wide fallback. Include an applicable
|
||||
`required_reviewers` entry even when it is not the path owner, and label why.
|
||||
5. Use documented governance specialties only to choose among matching owners:
|
||||
for example serving versus configuration, diffusion model versus cache or
|
||||
parallelism, TTS versus generic model execution, or CI versus tests.
|
||||
6. Exclude the PR author, bots, users who already completed the needed review,
|
||||
and duplicate pending requests. Do not infer availability or invent
|
||||
expertise from a username.
|
||||
7. Select one reviewer for a single-owner change and at most three for a real
|
||||
cross-module change. If more are plausible, rank them but do not ping a
|
||||
broad group; ask the user before exceeding three.
|
||||
|
||||
If every specific owner is excluded, consider one repository-wide fallback
|
||||
owner and label that choice as a fallback rather than a path match. Never fan
|
||||
out to the full fallback list. Prefer a qualified owner who covers multiple
|
||||
critical path groups over separate reviewers for each file. If CODEOWNERS is
|
||||
missing or invalid, state that gap and use governance only as a labeled fallback.
|
||||
|
||||
Recent file history may break a tie only when CODEOWNERS and governance remain
|
||||
ambiguous. Never use commit count alone to override an explicit current owner.
|
||||
|
||||
## Preview before writing
|
||||
|
||||
Show the user the exact candidate set and rationale:
|
||||
|
||||
```text
|
||||
@owner — <owned paths or module role>; please verify <specific module or feature contract>.
|
||||
```
|
||||
|
||||
Draft one consolidated comment tied to the frozen head:
|
||||
|
||||
```text
|
||||
Requesting focused review for `<short-head-sha>`:
|
||||
|
||||
- @owner1: could you check the <module contract> in `<path/group>`?
|
||||
- @owner2: could you check the <feature or cross-boundary contract> in `<path/group>`?
|
||||
|
||||
Relevant validation: <one concise result or exact remaining gap>.
|
||||
```
|
||||
|
||||
Mention only owners being requested. Keep the reason specific enough that each
|
||||
person knows what decision or invariant needs attention; do not post a generic
|
||||
“please review” or a generated ownership audit.
|
||||
|
||||
## External-write gate
|
||||
|
||||
- “Who should review?”, “suggest reviewers”, or “draft a request” authorizes
|
||||
only the preview.
|
||||
- “Request/add these reviewers” authorizes the GitHub review-request mutation.
|
||||
- “Ping/comment/tag the owners” authorizes the consolidated comment.
|
||||
- When the user explicitly asks to request reviews **with an owner comment**,
|
||||
both writes are authorized. Obey narrower wording when only one action is
|
||||
requested.
|
||||
|
||||
Immediately before an authorized write, verify the PR is still open, the head
|
||||
SHA is unchanged, and the candidate is not the author or already satisfied.
|
||||
Report draft/WIP state and avoid notifications unless the user explicitly wants
|
||||
reviewers requested before ready-for-review.
|
||||
|
||||
Use the GitHub connector for reviewer requests and the single top-level comment;
|
||||
use `gh` only when connector coverage is unavailable. Do not submit an approval,
|
||||
request-changes event, label, assignment, or multiple comments as part of this
|
||||
action. After writing, reread the PR and report the reviewers requested and the
|
||||
comment URL. If the head changed, discard the draft and rerank before posting.
|
||||
@@ -0,0 +1,24 @@
|
||||
# Communication Features
|
||||
|
||||
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/) |
|
||||
| Shared memory | [SharedMemoryConnector](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/omni_connectors/shared_memory_connector/) |
|
||||
| Yuanrong store | [YuanrongConnector](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/omni_connectors/yuanrong_connector/) |
|
||||
| Yuanrong transfer engine | [YuanrongTransferEngineConnector](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/omni_connectors/yuanrong_transfer_engine_connector/) |
|
||||
|
||||
## Feature checks
|
||||
|
||||
- Verify dependency, service, device/network, port, memory-pool, and environment
|
||||
prerequisites with actionable startup errors.
|
||||
- Match key/metadata construction, sender and receiver roles, allocation,
|
||||
ownership, completion, and supported fast/serialized paths.
|
||||
- Exercise timeout, retry, stale sender/buffer reclamation, partial transfer,
|
||||
cancellation, and connector close semantics.
|
||||
- Validate the smallest supported same-host or cross-node topology and document
|
||||
backend limitations and operational diagnostics.
|
||||
@@ -0,0 +1,26 @@
|
||||
# Diffusion Acceleration Features
|
||||
|
||||
Use with the exact diffusion submodule plus platform, cache, quantization, or
|
||||
benchmarking contracts selected by the live path.
|
||||
|
||||
| Feature family | Designs |
|
||||
| --- | --- |
|
||||
| Parallelism | [CFG](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/cfg_parallel/), [Expert](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/expert_parallel/), [HSDP](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/hsdp/), [Pipeline](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/pipeline_parallel/), [Sequence](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/sequence_parallel/), [Tensor](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/tensor_parallel/), [VAE patch](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/vae_parallel/) |
|
||||
| Attention | [Skip-Softmax](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/skip_softmax/) |
|
||||
| Quantization | [Quantization](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/quantization/) |
|
||||
| Cache | [Cache-DiT](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/cache_dit/), [TeaCache](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/teacache/) |
|
||||
| Batching | [Diffusion Continuous Batching](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/diffusion_continuous_batching/) |
|
||||
| Offload | [Distributed Layerwise Offload](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/distributed_layerwise_offload/) |
|
||||
|
||||
## Feature checks
|
||||
|
||||
- Verify feature selection, supported models/platforms/topologies, defaults,
|
||||
dependencies, and every touched feature combination.
|
||||
- Preserve feature-off and single-rank correctness; reject unsupported
|
||||
combinations before collectives, loading, or execution begin.
|
||||
- Trace shapes, shards, ranks, collectives, precision, cache validity, residency,
|
||||
and per-request state through the selected runtime hooks.
|
||||
- Compare output quality or accuracy against the disabled/reference path before
|
||||
accepting latency or memory improvements.
|
||||
- Require exact commands, topology, environment, warmup, repetitions, and
|
||||
measurements for performance claims; update feature compatibility docs.
|
||||
@@ -0,0 +1,22 @@
|
||||
# Infrastructure and Performance Features
|
||||
|
||||
Load the matching design from the reviewed head:
|
||||
|
||||
- [Prometheus Metrics](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/metrics/)
|
||||
for logger wrapping, gating, lifecycle, stage transfer, and metric definitions.
|
||||
- [Speech Generation Performance Optimizations](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/qwen3_omni_tts_performance_optimization/)
|
||||
for stage batching, CUDA graphs, async chunk, streaming, re-prefill, compile,
|
||||
and deployment evidence.
|
||||
|
||||
## Feature checks
|
||||
|
||||
- Route instrumentation changes through observability or profiling and result
|
||||
methodology through benchmarking and the performance evidence check.
|
||||
- Preserve opt-in/default behavior, units, labels, aggregation, timing scope,
|
||||
lifecycle resets, and bounded overhead.
|
||||
- For stacked optimizations, measure each increment against the same baseline
|
||||
and explain interactions, quality guards, regressions, and limitations.
|
||||
- Require exact model, checkpoint, hardware, software, workload, topology,
|
||||
concurrency, warmup, repetitions, and production commands.
|
||||
- Update the design page when metric meaning, supported optimization stack, or
|
||||
deployment guidance changes.
|
||||
@@ -0,0 +1,24 @@
|
||||
# Runtime and Stage Execution Features
|
||||
|
||||
Load the matching design from the reviewed head:
|
||||
|
||||
| Feature | Design | Primary module intersections |
|
||||
| --- | --- | --- |
|
||||
| Disaggregated inference | [Design](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/disaggregated_inference/) | Configuration, stage runtime, OmniConnector, I/O contracts |
|
||||
| Async chunk | [Design](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/async_chunk/) | Model integration, orchestration, stage runtime, connector, I/O |
|
||||
| Async diffusion output | [Design](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/async_diffusion_output/) | Diffusion runtime, I/O contracts, entrypoints |
|
||||
| Async Omni output materialization | [Design](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/omni_async_output_materialization/) | AR runtime, model integration, I/O contracts, orchestration |
|
||||
| Automatic prefix caching | [Design](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/feature/prefix_caching/) | AR runtime, cache management, I/O contracts |
|
||||
|
||||
## Feature checks
|
||||
|
||||
- Confirm the feature's supported models, entrypoints, stage types, topology,
|
||||
configuration, default, and compatibility limitations.
|
||||
- Trace enablement from public/config ingress through every participating module
|
||||
and verify the advertised production dispatch actually selects it.
|
||||
- Preserve a correct disabled or fallback path and make unsupported combinations
|
||||
fail or warn as documented.
|
||||
- Test overlap, ordering, identity, backpressure, cancellation, failure, final
|
||||
partial output, and cleanup at each asynchronous boundary.
|
||||
- Update the feature page, support matrices, examples, and recipes when behavior
|
||||
or coverage changes.
|
||||
@@ -0,0 +1,22 @@
|
||||
# Autoregressive Runtime
|
||||
|
||||
Primary design: [Autoregressive Runtime](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/ar_runtime/).
|
||||
|
||||
Use for AR request queues, scheduling state, token and memory budgets, KV/input
|
||||
readiness, workers, runners, and upstream vLLM request/cache transitions.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Preserve upstream request-state, scheduling, cache, and preemption semantics
|
||||
unless an Omni-specific difference is documented and tested.
|
||||
- Convert modality-specific data through explicit input or output adapters; do
|
||||
not inject it through unrelated scheduler state.
|
||||
- Keep workers and model runners focused on assigned execution rather than
|
||||
admission or cross-stage routing.
|
||||
- Synchronize token, sequence, batch, and memory budgets with worker-visible
|
||||
state and prevent starvation, double counting, or stale readiness.
|
||||
- Preserve legal transitions and one terminal cleanup path across completion,
|
||||
abort, timeout, preemption, worker failure, and shutdown.
|
||||
|
||||
Test request lifecycle, scheduling decisions, abort, cache state, adapters, and
|
||||
every affected worker execution mode against the pinned upstream contract.
|
||||
@@ -0,0 +1,24 @@
|
||||
# Benchmarking
|
||||
|
||||
Primary design: [Benchmarking](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/benchmarking/).
|
||||
|
||||
Use when benchmark code, workloads, metric calculations, CLI benchmark
|
||||
entrypoints, or result metadata change. Use the performance check separately
|
||||
when a PR uses benchmark results as evidence.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Record model and revision, code SHA, hardware, software, configuration,
|
||||
workload, precision, topology, concurrency, warmup, repetitions, and
|
||||
measurement window.
|
||||
- Validate successful and semantically acceptable outputs before accepting
|
||||
timing, memory, or scaling results.
|
||||
- Define every metric's unit, population, aggregation, synchronization, and
|
||||
timing scope.
|
||||
- Keep workload parsing and metric computation deterministic and reusable
|
||||
across base and head.
|
||||
- Reject comparisons that change material workload or runtime settings without
|
||||
isolating and explaining that difference.
|
||||
|
||||
Test workload parsing and metric calculations with deterministic fixtures and
|
||||
publish exact commands plus variability for quantitative claims.
|
||||
@@ -0,0 +1,22 @@
|
||||
# Cache Management
|
||||
|
||||
Primary design: [Cache Management](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/cache_management/).
|
||||
|
||||
Use for AR or diffusion cache identity, reusable state, validity, lifecycle,
|
||||
reset, eviction, and teardown.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Include every model, request, input, timestep, precision, device, topology,
|
||||
and feature property that can change a reused value in cache identity.
|
||||
- Preserve a correct disabled and miss path; never reuse after validity inputs
|
||||
change.
|
||||
- Isolate request-scoped state across concurrent requests and define ownership
|
||||
for model-scoped state.
|
||||
- Make reset, invalidation, eviction, capacity, and teardown explicit on
|
||||
completion, cancellation, failure, model reload, and shutdown.
|
||||
- Keep cache optimizations behind the owning runtime's hooks rather than adding
|
||||
a second request lifecycle.
|
||||
|
||||
Test hit, miss, invalidation, disabled behavior, concurrency, capacity, reset,
|
||||
and teardown, with quality comparison when reuse is approximate.
|
||||
@@ -0,0 +1,24 @@
|
||||
# vLLM-Omni Configuration
|
||||
|
||||
Primary design: [vLLM-Omni Configuration](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/vllm_omni_config/).
|
||||
|
||||
Use for config construction, deploy YAML, registries, schema, defaults,
|
||||
aliases, CLI projection, stage config, and topology. Inspect the reviewed
|
||||
page's status: while its contract remains deferred or draft, current code and
|
||||
tests—not proposed precedence rules—are authoritative.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Resolve each supported structured, legacy, CLI, environment, default, alias,
|
||||
and stage-override producer through its actual runtime consumer.
|
||||
- Reject unknown or owner-mismatched fields explicitly; keep process-local
|
||||
runtime objects out of transportable configuration.
|
||||
- Verify default, explicit, and feature-off values reach every live factory and
|
||||
consumer without silent reinterpretation.
|
||||
- Treat stage count, placement, connector, parallelism, device, and memory
|
||||
fields as one topology contract and reject impossible combinations early.
|
||||
- Preserve supported construction-path parity until a documented migration
|
||||
removes a path.
|
||||
|
||||
Require normalization and schema tests, a live-consumer assertion, and docs for
|
||||
public keys, defaults, constraints, precedence, or migration behavior.
|
||||
@@ -0,0 +1,33 @@
|
||||
# Diffusion Module Family
|
||||
|
||||
Primary design: [Diffusion Modules](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/diffusion/).
|
||||
|
||||
Use for diffusion-stage behavior, then load the exact submodule page from the
|
||||
reviewed head:
|
||||
|
||||
| Submodule | Signals | Design |
|
||||
| --- | --- | --- |
|
||||
| Runtime | Admission, scheduling, execution, progress, outputs, cancellation, hooks, cleanup | [Diffusion Runtime](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/diffusion/diffusion_runtime/) |
|
||||
| Model Integration | Pipelines, registry, loader, adapters, shared layers, model-specific processing | [Diffusion Model Integration](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/diffusion/diffusion_model_integration/) |
|
||||
| Continuous Batching | Compatibility, request/step batches, per-request progress, capacity | [Continuous Batching](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/diffusion/continuous_batching/) |
|
||||
| Parallelism | Rank topology, process groups, sharding, collectives, distributed execution | [Parallelism](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/diffusion/parallelism/) |
|
||||
| Offloader | Residency, transfers, memory accounting, prefetch, teardown | [Offloader](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/diffusion/offloader/) |
|
||||
|
||||
## Family-wide checks
|
||||
|
||||
- Keep one scheduler-owned lifecycle from admission through completion,
|
||||
cancellation, failure, and cleanup; optional features must use defined hooks.
|
||||
- Select pipelines through registry and loader contracts instead of scattered
|
||||
model-name conditionals; keep genuine model differences in model directories.
|
||||
- Trace latent, conditioning, timestep, generator, shape, layout, dtype, device,
|
||||
batch expansion, and output conversion through the actual consumer.
|
||||
- Make batch compatibility and per-request isolation explicit; never associate
|
||||
output by unstable batch position.
|
||||
- Derive topology from validated configuration, define every sharding boundary,
|
||||
keep collectives symmetric, and preserve a supported single-rank path.
|
||||
- Track one residency owner per offloaded component, wait for transfer readiness,
|
||||
preserve model state, bound retained copies, and clean up deterministically.
|
||||
|
||||
Load matching feature designs for cache, quantization, parallelism, continuous
|
||||
batching, or offload. Require feature-off correctness, representative inference,
|
||||
terminal-path tests, and quality evidence for optimizations.
|
||||
@@ -0,0 +1,23 @@
|
||||
# Engine Orchestration
|
||||
|
||||
Primary design: [Engine Orchestration](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/engine_orchestration/).
|
||||
|
||||
Use for request-state creation, cross-stage routing, output ordering, companion
|
||||
tracking, RPC correlation, cancellation propagation, and terminal convergence.
|
||||
Orchestration does not own replica placement, stage-process startup, public
|
||||
rendering, connector transport, or payload and error schemas.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Keep one owner for cross-stage routing; entrypoints, models, and stage clients
|
||||
must not independently select the next logical stage.
|
||||
- Preserve request identity and output order across concurrent stages and
|
||||
correlate every control or RPC result to the originating request and wave.
|
||||
- Make terminal state monotonic: no new forwarding after success, abort,
|
||||
request failure, or fatal engine failure.
|
||||
- Propagate cancellation and failure to every active stage and release
|
||||
orchestrator-owned tasks, queues, companions, and request state.
|
||||
- Preserve shutdown ordering across the orchestrator and stage runtime.
|
||||
|
||||
Test representative multi-stage routing, ordering, correlation, cancellation,
|
||||
failure, and repeated shutdown.
|
||||
@@ -0,0 +1,25 @@
|
||||
# Entrypoints and Serving Boundaries
|
||||
|
||||
Primary design: [Entrypoints and Serving Boundaries](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/entrypoints/).
|
||||
|
||||
Use for offline APIs, CLI and server composition, OpenAI-compatible routes,
|
||||
request validation, response conversion, streaming, sessions, and engine
|
||||
handoff. Entrypoints adapt public contracts; they do not own cross-stage
|
||||
routing, stage lifecycle, configuration precedence, payload schemas, or error
|
||||
classification.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Normalize and validate public values once before engine submission.
|
||||
- Preserve request identity, modality, ordering, and exactly one terminal event
|
||||
across streaming, disconnect, cancellation, and failure.
|
||||
- Translate internal outputs and errors explicitly for each advertised offline,
|
||||
HTTP, SSE, WebSocket, or asynchronous-job path.
|
||||
- Keep model-specific behavior behind a common adapter or processor instead of
|
||||
adding model-name policy to generic entrypoints.
|
||||
- Require API or CLI changes to explain necessity, alternatives, compatibility,
|
||||
migration, and the supported route or transport matrix.
|
||||
|
||||
Test the affected public protocol, normalization, engine handoff, rendering,
|
||||
and terminal outcomes. Route orchestration, I/O schema, and semantic error
|
||||
changes to their module contracts.
|
||||
@@ -0,0 +1,24 @@
|
||||
# Error Classification, Propagation, and Rendering
|
||||
|
||||
Primary design: [Error Classification, Propagation, and Rendering](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/error_contracts/).
|
||||
|
||||
Use when a change classifies failures, crosses process boundaries, decides
|
||||
fatal versus request-scoped impact, sanitizes context, or renders offline,
|
||||
HTTP, SSE, WebSocket, or asynchronous-job errors. The I/O module owns the wire
|
||||
schema; this module owns the meaning of its error fields.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Keep request-scoped failure isolated unless the underlying engine is no
|
||||
longer usable.
|
||||
- Preserve stable classification and request context across process and
|
||||
protocol boundaries without exposing prompts, outputs, secrets, or internal
|
||||
stack details.
|
||||
- Keep cancellation distinguishable from internal failure.
|
||||
- Terminate each public transport exactly once with consistent status, body,
|
||||
event, finish reason, and metrics classification.
|
||||
- Maintain compatibility across mixed or older producers only when the
|
||||
reviewed contract defines that behavior.
|
||||
|
||||
Test table-driven classification, wire round trips, fatal/request-scoped
|
||||
propagation, cancellation, sanitization, and every affected public renderer.
|
||||
@@ -0,0 +1,23 @@
|
||||
# Execution Platforms
|
||||
|
||||
Primary design: [Execution Platforms](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/execution_platforms/).
|
||||
|
||||
Use for hardware capability detection, worker selection, patches, kernels,
|
||||
attention backends, and vendor-specific configuration or connectors.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Guard hardware behavior through platform selection or capability detection
|
||||
and fail clearly when unsupported.
|
||||
- Keep portable modules free of direct vendor imports when a common selector
|
||||
exists; verify portable imports without the target accelerator.
|
||||
- Override only behavior that differs from common or upstream implementations,
|
||||
and compare sibling worker hooks when inheritance does not provide parity.
|
||||
- Preserve dtype, layout, rank, stream, synchronization, workspace,
|
||||
graph-capture, and resource-lifetime contracts.
|
||||
- Do not let simplified CPU doubles hide platform MRO, initialization, or
|
||||
dispatch failures.
|
||||
|
||||
Run portable checks first, then the smallest matching-device test in a fresh
|
||||
environment. Report unavailable hardware as a validation gap, never simulated
|
||||
device evidence.
|
||||
@@ -0,0 +1,23 @@
|
||||
# Input, Output, and Modality Contracts
|
||||
|
||||
Primary design: [Input, Output, and Modality Contracts](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/input_output_modality_contracts/).
|
||||
|
||||
Use for requests, prompt and modality types, message schemas, serialization,
|
||||
output types, accumulation, completion, and compatibility shims crossing
|
||||
entrypoint, orchestration, stage, connector, or model boundaries.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Give every boundary value an explicit modality and validated typed schema.
|
||||
- Preserve request and stage identity across conversion, serialization,
|
||||
streaming updates, batching, cancellation, and errors.
|
||||
- Distinguish partial, final, empty, and failed output states for every modality.
|
||||
- Verify producer and consumer agreement on shape, layout, dtype, device,
|
||||
ownership, metadata, ordering, and optional or unknown fields.
|
||||
- Keep internal objects out of public protocols; translate them at the owning
|
||||
entrypoint.
|
||||
- Preserve wire compatibility deliberately and give deprecated shims a bounded
|
||||
migration path.
|
||||
|
||||
Require construction, round-trip, accumulation, completion, rejection, and
|
||||
compatibility tests at each changed producer-consumer boundary.
|
||||
@@ -0,0 +1,25 @@
|
||||
# Model Integration
|
||||
|
||||
Primary design: [Model Integration](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/model_integration/).
|
||||
|
||||
Use for model registration, configuration adapters, preprocessing, checkpoint
|
||||
loading, stage inputs, model-specific hooks, shared layers, runners, and model
|
||||
execution. Use the diffusion family reference for diffusion models.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Declare explicitly how the model class, loader, input processor, stage
|
||||
configuration, and runtime capabilities are selected.
|
||||
- Preserve weight-name mapping, dtype/device placement, optional-dependency
|
||||
errors, and checkpoint-specific loading semantics.
|
||||
- Trace each typed input and output through preprocessing, runner state, model
|
||||
call, and its actual consumer.
|
||||
- Reuse upstream vLLM behavior when sufficient; document and test the precise
|
||||
difference for every override or patch.
|
||||
- Keep model code out of admission, cross-stage routing, public policy, and
|
||||
terminal lifecycle ownership.
|
||||
- Place reusable behavior in shared modules and test it with more than one
|
||||
integration.
|
||||
|
||||
Require focused registry, loader, processor, runner, and representative model
|
||||
execution tests. Apply the model-addition check for new or expanded support.
|
||||
@@ -0,0 +1,21 @@
|
||||
# Observability
|
||||
|
||||
Primary design: [Observability](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/observability/).
|
||||
|
||||
Use for metrics, logs, correlation fields, and cross-stage request visibility.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Preserve stable request and stage identity across entrypoints, orchestration,
|
||||
stages, workers, errors, and terminal cleanup.
|
||||
- Treat metric names, units, labels, aggregation, and lifecycle as public
|
||||
contracts; document any semantic change.
|
||||
- Keep labels bounded: never use request IDs, prompts, outputs, paths, or other
|
||||
unbounded user values.
|
||||
- Register and reset metrics exactly once across repeated startup and shutdown,
|
||||
and keep disabled instrumentation inexpensive.
|
||||
- Align emitted values with their actual timing and lifecycle owner rather than
|
||||
reconstructing them from ambiguous client strings.
|
||||
|
||||
Test registration, label sets, units, resets, error classification, and
|
||||
representative multi-stage emission.
|
||||
@@ -0,0 +1,24 @@
|
||||
# OmniConnector
|
||||
|
||||
Primary design: [OmniConnector](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/omni_connector/).
|
||||
|
||||
Use for connector contracts and model-agnostic data transfer across stages,
|
||||
processes, devices, or nodes. Connector implementations transport and
|
||||
synchronize data; they do not choose the next stage or implement model policy.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Prove producer and consumer agreement on request/stage identity, shape,
|
||||
layout, dtype, placement, ownership, ordering, and completion.
|
||||
- Keep backend selection and metadata/key construction explicit; do not silently
|
||||
reinterpret or drop payloads across SHM, store, network, or vendor paths.
|
||||
- Bound connect, put/get, retry, synchronization, and readiness waits with
|
||||
backpressure, cancellation, and actionable errors.
|
||||
- Prevent route, endpoint, port, key, and buffer collisions across repeated
|
||||
startup, concurrent requests, and replica replacement.
|
||||
- Release connections, handles, buffers, listener work, and stale sender state
|
||||
on success, cancellation, failure, and shutdown.
|
||||
|
||||
Load the matching communication feature design for a concrete backend. Require
|
||||
focused producer-consumer tests and the smallest runnable multi-process or
|
||||
multi-node path.
|
||||
@@ -0,0 +1,21 @@
|
||||
# Profiling
|
||||
|
||||
Primary design: [Profiling](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/profiling/).
|
||||
|
||||
Use for opt-in instrumentation and trace collection across supported profiler
|
||||
backends.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Keep expensive collection disabled by default and bounded by explicit start,
|
||||
stop, timeout, and cleanup behavior.
|
||||
- Preserve scheduling, generated outputs, resource ownership, and concurrency
|
||||
semantics apart from documented synchronization overhead.
|
||||
- Include stage, rank, worker, operation, and request correlation where useful
|
||||
without embedding user payloads or secrets.
|
||||
- Make repeated start/stop and partial failure safe, including multi-rank trace
|
||||
naming and artifact ownership.
|
||||
- Measure disabled and enabled overhead before making performance claims.
|
||||
|
||||
Test the disabled path, repeated lifecycle, failure cleanup, trace context, and
|
||||
each affected backend.
|
||||
@@ -0,0 +1,22 @@
|
||||
# Quantization
|
||||
|
||||
Primary design: [Quantization](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/quantization/).
|
||||
|
||||
Use for method registration, checkpoint interpretation, layer replacement,
|
||||
reduced precision, platform capability, and runtime constraints.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Validate checkpoint metadata, selected method, supported layers, device
|
||||
capability, required dependencies, and parallel layout before loading.
|
||||
- Preserve loader and weight-name mapping semantics, dtype, scale/granularity,
|
||||
zero point, padding, and buffer handling through the real kernel consumer.
|
||||
- Fail clearly for unsupported combinations; do not silently choose a different
|
||||
method or precision.
|
||||
- Keep online and pre-quantized checkpoint paths explicit and ensure patches or
|
||||
fallbacks do not bypass model-owned loaders.
|
||||
- Require numerical comparison against a defined reference with explicit
|
||||
tolerance before treating performance results as valid.
|
||||
|
||||
Test registration, loading, representative layers, metadata, platform guards,
|
||||
parallel combinations, and numerical correctness separately from performance.
|
||||
@@ -0,0 +1,23 @@
|
||||
# Stage Runtime and Replica Lifecycle
|
||||
|
||||
Primary design: [Stage Runtime and Replica Lifecycle](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/module/stage_runtime/).
|
||||
|
||||
Use for local or distributed placement, startup, readiness, stage pools,
|
||||
replica identity and membership, selection, affinity, liveness, draining, and
|
||||
stage-client or process lifecycle. Cross-stage request policy belongs to engine
|
||||
orchestration; model scheduling belongs to the owning runtime.
|
||||
|
||||
## Contract checks
|
||||
|
||||
- Acquire, replace, and retire replicas through one stage-runtime lifecycle.
|
||||
- Preserve stable replica identity and request affinity until termination or a
|
||||
declared replica loss.
|
||||
- Make readiness explicit before admission; surface partial-startup and
|
||||
membership failures without leaving half-initialized capacity.
|
||||
- Keep local and distributed semantics aligned for supported modes.
|
||||
- Make drain, shutdown, and cleanup idempotent across repeated signals and
|
||||
process failure.
|
||||
|
||||
Test startup, readiness, selection, affinity, membership changes, abort,
|
||||
process loss, draining, and repeated shutdown on affected local/distributed
|
||||
paths.
|
||||
@@ -0,0 +1,34 @@
|
||||
# Resolve Design Contracts
|
||||
|
||||
Use after freezing the review head and before selecting module or feature
|
||||
references. The maintained [design index](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/)
|
||||
defines two routing axes:
|
||||
|
||||
- **Module designs** own code boundaries and invariants. Select one primary
|
||||
module and a second only when the live producer-consumer path crosses it.
|
||||
- **Feature designs** describe behavior spanning modules. Load every feature
|
||||
page whose implementation, default, support, or contract changes.
|
||||
|
||||
## Resolution order
|
||||
|
||||
1. Open `docs/design/index.md` in the reviewed head, not the reviewer's checkout
|
||||
or published latest tree.
|
||||
2. Match changed behavior against each module page's `primary_code_paths`,
|
||||
`primary_path_exceptions`, ownership boundary, and `depends_on` metadata.
|
||||
3. Read `status`, `architecture_state`, `last_verified_commit`, and any linked
|
||||
in-flight work before treating prose as current behavior.
|
||||
4. Use current code and tests when a page is draft, deferred, stale, absent, or
|
||||
conflicts with the frozen implementation. Report material documentation
|
||||
drift separately.
|
||||
5. Treat candidate invariants and promotion gates as review questions until
|
||||
repository policy, current code/tests, or owner-approved normative text
|
||||
makes them enforceable. Do not turn a draft identifier into an automatic
|
||||
blocker.
|
||||
6. Use published latest docs only for discovery when the reviewed head lacks a
|
||||
page; never import a newer contract into an older branch silently.
|
||||
|
||||
When behavior changes an active module or feature contract, require the same PR
|
||||
to update that page. Require a new page only when repository policy, an accepted
|
||||
RFC/issue, or explicit maintainer acceptance criteria do so; otherwise treat it
|
||||
as a non-blocking design suggestion. Do not demand a second page when an existing
|
||||
design already owns the exact contract.
|
||||
@@ -0,0 +1,129 @@
|
||||
# General Review Checks
|
||||
|
||||
Apply these checks to every PR after freezing the review surface. A pattern is
|
||||
not a finding until the diff introduces or exposes a reachable trigger, impact,
|
||||
and smallest safe fix. Do not report unrelated backlog.
|
||||
|
||||
Official docs: [contributing guide](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/)
|
||||
[feature compatibility](https://docs.vllm.ai/projects/vllm-omni/en/latest/user_guide/feature_compatibility/),
|
||||
and [design documents](https://docs.vllm.ai/projects/vllm-omni/en/latest/design/).
|
||||
|
||||
## Contract and scope
|
||||
|
||||
- Match the title/body claims to every changed production path, test, config,
|
||||
dependency, generated artifact, and user-visible document.
|
||||
- Trace each changed value through public ingress, validation/defaulting,
|
||||
producer, transformations, process or device boundaries, final consumer, and
|
||||
terminal cleanup.
|
||||
- Check every applicable sync/async, offline/online, streaming/non-streaming,
|
||||
feature-off, topology, and compatibility path without demanding unsupported
|
||||
modes merely for symmetry.
|
||||
|
||||
## Module, API, and class design
|
||||
|
||||
Apply these rules to new or materially changed production code; do not require
|
||||
unrelated cleanup from untouched files.
|
||||
|
||||
1. Start each source module with a concise module docstring that defines its
|
||||
responsibility and boundaries and identifies its primary classes and key
|
||||
functions. Place it after any required license header and before imports.
|
||||
2. Give every added or materially changed class and function a clear docstring
|
||||
covering its responsibility, parameters, return value, raised exceptions,
|
||||
and non-obvious invariants. Use explicit typed parameters at owned API
|
||||
boundaries; avoid opaque `**kwargs` and `Any`. Allow them only when an
|
||||
upstream or compatibility interface requires them, and document and validate
|
||||
the accepted keys and concrete types.
|
||||
3. Move stable helpers shared by multiple features into an appropriately scoped
|
||||
utility module. Do not let feature modules accumulate large collections of
|
||||
unrelated private utility functions; keep a private helper local only when
|
||||
it is single-purpose and tightly coupled to that feature.
|
||||
4. Keep every method on a key class within that class's stated responsibility,
|
||||
and document non-obvious design decisions and invariants. Before adding a
|
||||
method, prefer reuse, consolidation, or extension of an existing abstraction;
|
||||
split the class when new behavior creates a separate responsibility or makes
|
||||
the class unnecessarily long.
|
||||
5. Require changes to user-facing behavior to update `docs/`. When an existing
|
||||
active module or feature design page owns the changed contract, update that
|
||||
page. Require a new design page only when the reviewed branch's repository
|
||||
policy, an accepted RFC/issue, or explicit maintainer acceptance criteria do
|
||||
so; otherwise suggest it as non-blocking. For a major architectural change,
|
||||
apply the branch's contributing policy for a design RFC or issue; the current
|
||||
policy uses more than 500 changed production-code lines, excluding kernel,
|
||||
data, config, and test changes. Resolve page status and authority through
|
||||
[design-contracts.md](design-contracts.md), and do not demand a duplicate page
|
||||
when an existing design owns the exact contract. When required, cover scope,
|
||||
data flow, public contracts, failure behavior, alternatives, validation,
|
||||
limitations, and rollout.
|
||||
6. Require every performance PR to provide reproducible base-versus-head A/B
|
||||
results under the same hardware, software, model, workload, precision,
|
||||
topology, warmup, repetitions, and measurement scope. Include exact commands,
|
||||
variability, and a correctness or quality guard; one-sided or non-comparable
|
||||
numbers are incomplete evidence. For a new model absent from the frozen base,
|
||||
compare head with its pinned canonical reference implementation under rule 12
|
||||
instead. Follow
|
||||
[perf-verification.md](../checks/perf-verification.md) for the full protocol.
|
||||
7. Require every addition, removal, default change, or semantic change to a
|
||||
user-facing inference API or CLI to explain why it is necessary, which
|
||||
alternatives were considered, and why the chosen interface is preferable.
|
||||
Record compatibility, deprecation or migration behavior, and the rationale
|
||||
in the PR. Update the corresponding module and feature design documents when
|
||||
they define the changed interface or workflow, or when policy requires one.
|
||||
8. Require every bug-fix PR to add an automated regression test that reproduces
|
||||
the defect on the frozen base and passes on the fixed head. Exercise the
|
||||
production path or nearest stable boundary and assert the corrected behavior,
|
||||
not only process survival. Record the exact base/head command and result, and
|
||||
place hardware- or model-specific coverage in the correct CI lane. A manual
|
||||
reproduction supplements but does not replace the regression test.
|
||||
9. Require every new or materially changed recipe to follow
|
||||
[`recipes/TEMPLATE.md`](../../../../../recipes/TEMPLATE.md) and the naming and
|
||||
layout conventions in
|
||||
[`recipes/README.md`](../../../../../recipes/README.md).
|
||||
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
|
||||
contract makes that evidence a merge requirement.
|
||||
@@ -0,0 +1,191 @@
|
||||
# Review Execution and Delivery
|
||||
|
||||
Use this reference for every review. It defines snapshot collection, gates,
|
||||
validation records, delivery permissions, maintainer tone, and re-review.
|
||||
|
||||
Official docs: [contributing guide](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/)
|
||||
and [CI failure triage](https://docs.vllm.ai/projects/vllm-omni/en/latest/contributing/ci/failures/).
|
||||
|
||||
## Contents
|
||||
|
||||
- [Freeze the review surface](#freeze-the-review-surface)
|
||||
- [Report status before analysis](#report-status-before-analysis)
|
||||
- [Apply review gates](#apply-review-gates)
|
||||
- [Run bounded validation](#run-bounded-validation)
|
||||
- [Deliver maintainer-style findings](#deliver-maintainer-style-findings)
|
||||
- [Re-review safely](#re-review-safely)
|
||||
|
||||
## Freeze the review surface
|
||||
|
||||
For a GitHub PR, read the base/head SHA before and after fetching metadata and
|
||||
the diff:
|
||||
|
||||
```bash
|
||||
gh api "repos/vllm-project/vllm-omni/pulls/<PR>" \
|
||||
--jq '{base_sha: .base.sha, head_sha: .head.sha}'
|
||||
REVIEW_FIELDS="number,url,title,body,isDraft,baseRefName,headRefName,mergeable,mergeStateStatus,statusCheckRollup,files"
|
||||
gh pr view <PR> --repo vllm-project/vllm-omni \
|
||||
--json "${REVIEW_FIELDS}"
|
||||
gh pr diff <PR> --repo vllm-project/vllm-omni
|
||||
gh api "repos/vllm-project/vllm-omni/pulls/<PR>" \
|
||||
--jq '{base_sha: .base.sha, head_sha: .head.sha}'
|
||||
```
|
||||
|
||||
Discard the snapshot if either SHA changed. Do not mix comments or validation
|
||||
from different heads.
|
||||
|
||||
After the SHAs stabilize, fetch and materialize a trusted `head_sha` in an
|
||||
isolated detached worktree. Run every source read, `rg` search, import, and test
|
||||
from that snapshot, not the caller's checkout. A detached worktree provides
|
||||
snapshot isolation, not a security boundary.
|
||||
|
||||
Treat a fork head as untrusted unless the user and environment policy explicitly
|
||||
establish trust. Never run its imports, tests, builds, hooks, package setup, or
|
||||
repo-configurable linters/plugins on the reviewer host. Execute them only in a
|
||||
disposable sandbox or VM with no credentials or inherited secrets, no host agent
|
||||
or service sockets, minimal read-only host mounts, disabled or explicitly
|
||||
allowlisted network, resource/time limits, and destruction after the run. If
|
||||
that boundary is unavailable, limit the review to the remote diff, SHA-addressed
|
||||
reads such as `git show <head_sha>:<path>`, and existing CI evidence; report all
|
||||
executable validation as a gap.
|
||||
|
||||
Before the first source read, record a pristine snapshot fingerprint outside
|
||||
the reviewed worktree. Include `HEAD`, the NUL-delimited index entries and blob
|
||||
IDs, and a NUL-safe manifest of every worktree entry except Git metadata,
|
||||
including tracked, untracked, and ignored paths with file type, mode, symlink
|
||||
target, and content hash. Before each validation group and delivery, recompute
|
||||
and byte-compare the fingerprint as well as asserting `HEAD == head_sha`. If a
|
||||
tool changes or creates any entry, discard affected evidence and recreate the
|
||||
snapshot before another group; do not rely on `HEAD` alone or clean an unknown
|
||||
worktree in place. If an exact snapshot cannot be created, use SHA-addressed
|
||||
reads and report filesystem-dependent validation as a gap.
|
||||
|
||||
For a local branch/worktree, determine the target ref from the user, current PR,
|
||||
or configured upstream; never infer it from the branch name. Resolve the target
|
||||
once and include all task-owned worktree state:
|
||||
|
||||
```bash
|
||||
git status --porcelain=v2 -z
|
||||
git rev-parse HEAD
|
||||
git rev-parse <target-ref>
|
||||
git merge-base <target-base-sha> HEAD
|
||||
git diff --stat <comparison-commit>
|
||||
git diff --name-status <comparison-commit>
|
||||
git diff --binary <comparison-commit>
|
||||
git diff --cached --binary <comparison-commit>
|
||||
git diff --binary
|
||||
git ls-files --others --exclude-standard -z
|
||||
```
|
||||
|
||||
Freeze the exact bytes of every in-scope untracked file from the NUL-delimited
|
||||
list, plus a NUL-safe path, file-type, mode, and content-hash manifest. Also
|
||||
fingerprint `HEAD`, the target and merge base, porcelain status, and both binary
|
||||
index and worktree patches. Recording only untracked names is insufficient.
|
||||
Keep these snapshots in the evidence packet and do not mutate the reviewed
|
||||
checkout during a read-only review.
|
||||
|
||||
For local changes without PR metadata, mark the title/body as synthetic. Base
|
||||
them on the user request and commit messages without inventing validation claims.
|
||||
|
||||
If no PR, branch, or usable worktree is identifiable, ask for a PR URL/number or
|
||||
explicit base and head rather than guessing.
|
||||
|
||||
## Report status before analysis
|
||||
|
||||
Within 60 seconds of starting, and before source searches or tests, send this
|
||||
host update:
|
||||
|
||||
```text
|
||||
Pinned head: <SHA>
|
||||
Base/comparison: <ref and SHA>
|
||||
CI: <pass/fail/pending/not applicable>
|
||||
Mergeability: <state/not applicable>
|
||||
Preliminary findings: <brief finding or none yet>
|
||||
```
|
||||
|
||||
Mark early findings as preliminary and continue. This is not a GitHub comment.
|
||||
|
||||
## Apply review gates
|
||||
|
||||
- Report draft/WIP state. Continue a local review when explicitly requested,
|
||||
but do not publish review events without separate authorization.
|
||||
- Record DCO, pre-commit, required CI, and mergeability. Pending or unknown gates
|
||||
do not block source review.
|
||||
- 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.
|
||||
@@ -0,0 +1,76 @@
|
||||
# Module and Feature Review Routing
|
||||
|
||||
Read [design-contracts.md](design-contracts.md) first. Route from the frozen
|
||||
head's live producer-consumer behavior, then confirm the result against its
|
||||
`docs/design/` metadata. Titles and paths are evidence, not the deciding rule.
|
||||
|
||||
## Primary module contract
|
||||
|
||||
Choose one primary module and a second only when the live call path crosses a
|
||||
documented dependency or primary-path exception.
|
||||
|
||||
| Module | Live behavior signals | Read |
|
||||
| --- | --- | --- |
|
||||
| Entrypoints | Offline/CLI/API ingress, validation, rendering, streaming, sessions | [entrypoints.md](../modules/entrypoints.md) |
|
||||
| Configuration | Schema, defaults, deploy/stage construction, registry, topology | [configuration.md](../modules/configuration.md) |
|
||||
| I/O and Modality | Requests, messages, serialization, output types, accumulation, completion | [input-output-modality.md](../modules/input-output-modality.md) |
|
||||
| Error Contracts | Classification, fatality, propagation, sanitization, public rendering | [error-contracts.md](../modules/error-contracts.md) |
|
||||
| Engine Orchestration | Cross-stage routing, request state, ordering, RPC correlation, terminal convergence | [engine-orchestration.md](../modules/engine-orchestration.md) |
|
||||
| Stage Runtime | Placement, startup, readiness, replicas, affinity, membership, shutdown | [stage-runtime.md](../modules/stage-runtime.md) |
|
||||
| OmniConnector | Cross-stage/process/device/node transport and synchronization | [omni-connector.md](../modules/omni-connector.md) |
|
||||
| Model Integration | Registration, preprocessing, loading, runners, model-specific execution | [model-integration.md](../modules/model-integration.md) |
|
||||
| AR Runtime | Scheduling, request/cache state, adapters, workers, upstream vLLM semantics | [ar-runtime.md](../modules/ar-runtime.md) |
|
||||
| Diffusion Family | Diffusion runtime, models, batching, parallelism, or offload | [diffusion.md](../modules/diffusion.md) |
|
||||
| Execution Platforms | Hardware selection, capabilities, vendor workers, kernels, patches | [execution-platforms.md](../modules/execution-platforms.md) |
|
||||
| Cache Management | Cache identity, validity, reuse, eviction, reset, teardown | [cache-management.md](../modules/cache-management.md) |
|
||||
| Quantization | Method/metadata selection, layer mapping, reduced precision, constraints | [quantization.md](../modules/quantization.md) |
|
||||
| Observability | Metrics, logs, labels, units, correlation, lifecycle | [observability.md](../modules/observability.md) |
|
||||
| Profiling | Opt-in instrumentation, traces, start/stop lifecycle, overhead | [profiling.md](../modules/profiling.md) |
|
||||
| Benchmarking | Workloads, metric computation, benchmark CLI, result metadata | [benchmarking.md](../modules/benchmarking.md) |
|
||||
|
||||
For tests-, docs-, CI-, or recipe-only changes, route to the production module
|
||||
whose contract they protect. When none exists, use the applicable evidence
|
||||
check without inventing a production owner.
|
||||
|
||||
## Feature-design overlays
|
||||
|
||||
Load every category whose documented behavior, enablement, default, support, or
|
||||
compatibility changes.
|
||||
|
||||
| Feature-design section | Read |
|
||||
| --- | --- |
|
||||
| Runtime and stage execution | [runtime-stage-execution.md](../features/runtime-stage-execution.md) |
|
||||
| Communication and concrete connectors | [communication.md](../features/communication.md) |
|
||||
| Diffusion acceleration | [diffusion-acceleration.md](../features/diffusion-acceleration.md) |
|
||||
| Infrastructure and performance | [infrastructure-performance.md](../features/infrastructure-performance.md) |
|
||||
|
||||
## Evidence and change overlays
|
||||
|
||||
| Signal | Read | Optional repo-local skill |
|
||||
| --- | --- | --- |
|
||||
| New or expanded model, loader, processor, registry, or stage config | [model-addition-checklist.md](../checks/model-addition-checklist.md) | [`add-tts-model`](../../../add-tts-model/SKILL.md) or [`add-diffusion-model`](../../../add-diffusion-model/SKILL.md) |
|
||||
| Latency, throughput, memory, scaling, precision, or quality claim | [perf-verification.md](../checks/perf-verification.md) | [`diffusion-perf-opt`](../../../diffusion-perf-opt/SKILL.md) for diffusion |
|
||||
| Tests changed, absent for risky behavior, or test-only | [test-quality-evaluation.md](../checks/test-quality-evaluation.md) | [`vllm-omni-test`](../../../vllm-omni-test/SKILL.md) |
|
||||
| CI, examples, docs, public behavior, or contributor evidence | [tests-docs-checklist.md](../checks/tests-docs-checklist.md) | None |
|
||||
| Suitable hardware/server and runnable affected path | [verification.md](../checks/verification.md) | None |
|
||||
| User asks who should review or requests owner notification | [review-requests.md](../delivery/review-requests.md) | None |
|
||||
|
||||
Use [`quantization`](../../../quantization/SKILL.md) for a quantization module
|
||||
change and [`$vllm-omni-npu-model-runner-upgrade`](../../../vllm-omni-npu-upgrade/SKILL.md)
|
||||
for an NPU execution-platform change when those skills are available.
|
||||
|
||||
For a bug fix, require a reachable reproduction and regression test. For a
|
||||
refactor, prove parity and remove obsolete paths. For a feature, verify the
|
||||
public/config contract, module integration, compatibility/default behavior,
|
||||
production dispatch, feature design, and docs.
|
||||
|
||||
## Calibrate findings
|
||||
|
||||
- **P0:** security exposure, data corruption, or broad project unusability.
|
||||
- **P1:** reachable runtime failure, wrong output, compatibility break, or
|
||||
unsafe lifecycle in the changed behavior.
|
||||
- **P2:** real non-blocking defect with a concrete future failure mode.
|
||||
|
||||
Treat draft candidate invariants, missing hardware, and unsupported claims as
|
||||
questions or validation gaps unless current code, tests, or policy makes them a
|
||||
merge requirement.
|
||||
Reference in new issue
Block a user