[CI] Add the platform import-boundary test from RFC #6691 by armaanamatya · Pull Request #8029 · vllm-project/vllm-omni

armaanamatya · 2026-09-23T01:05:49Z

Purpose

Rollout step 1 of #6691: land the import-boundary tests before any platform-tree code moves, so the boundary is enforced while the debt is paid down one consumer domain at a time.

tests/platforms/test_platform_import_boundary.py checks two rules statically over vllm_omni/:

Neutral code never imports a concrete vllm_omni.platforms.<backend> module (R2), at any nesting depth, including lazy imports inside functions. Everything outside vllm_omni/platforms/<backend>/ counts as neutral, including the detector in vllm_omni/platforms/__init__.py. TYPE_CHECKING blocks are skipped; relative imports and from pkg import submodule are resolved against the tree.
Importing a backend's platform class does not eagerly load implementation bodies (R4). Starting from vllm_omni.platforms.<backend> and .platform, module-level imports are followed transitively through other vllm_omni.platforms modules (thin hooks are platform-owned) and rejected when they reach platforms/<any>/{models,layers,quant,omni_connectors} or a consumer implementation subsystem (model_executor.models, model_executor.layers, diffusion.models, diffusion.layers, diffusion.attention.backends except the registry, quantization, diffusion.quantization, distributed.omni_connectors). Function-level imports in hooks are allowed, which is what R4 asks for.

Existing violations are an exact, named allowlist. The test fails on a new violation and also on an entry whose import no longer exists, so the list can only shrink. Each entry names the RFC migration item that retires it:

Neutral module	Imports	RFC item
`diffusion/attention/backends/flash_attn.py`	`platforms.npu.quant.kv_quant_npu`	NPU group 2
`distributed/omni_connectors/__init__.py`	`platforms.npu.omni_connectors.yuanrong_transfer_engine_connector`	NPU group 3 / step 9
`distributed/omni_connectors/connectors/yuanrong_transfer_engine_connector.py`	same (reverse-direction re-export)	NPU group 3 / step 9
`distributed/omni_connectors/factory.py`	`platforms.npu.omni_connectors`	NPU group 3 / step 9
`model_executor/models/fish_omni/fish_omni_model.py`	`platforms.npu.models.fish_omni`	NPU group 1 (new consumer from #9106)
`model_executor/models/fish_omni/model_state.py`	`platforms.npu.models.fish_omni_model_state`	NPU group 1 (new module from #9106)
`model_executor/models/fish_omni/quantization.py`	`platforms.npu.quant.fish_omni`	quant-provider follow-up, RFC step 9 (new module from #9106)
`model_executor/models/minicpmo_4_5/batched_token2wav.py`	`platforms.npu.models.step_audio2_token2wav`	NPU group 1
`model_executor/models/minicpmo_4_5/batched_token2wav.py`	`platforms.npu.models.cosyvoice2_dit_attn`	NPU group 1 (new import from #8908)
`model_executor/models/minicpmo_4_5/batched_token2wav.py`	`platforms.npu.models.minicpmo_fused`	NPU group 1 (new module from #8908)
`model_executor/models/minicpmo_4_5/cuda_graph_wrapper.py`	`platforms.npu.models.step_audio2_token2wav`	NPU group 1 (new consumer from #8735)
`model_executor/models/minicpmo_4_5/encoder_graph.py`	`platforms.npu.graph_tools`	NPU group 1 (new consumers from #8515)
`model_executor/models/minicpmo_4_5/flow_encoder_graph.py`	`platforms.npu.graph_tools`	NPU group 1 (new consumers from #8515)
`model_executor/models/moss_tts/modeling_moss_tts_codec.py`	`platforms.npu.models.moss_tts_streaming_decode_wrapper`	NPU group 1 (added after the RFC inventory)
`model_executor/models/step_audio2/step_audio2_token2wav.py`	`platforms.npu.models.step_audio2_token2wav`	NPU group 1
`platforms/__init__.py` (detector)	`platforms.xpu`	XPU housekeeping: `dist_backend` is set during detection

The eager-import allowlist is empty: every backend's platform class is already clean. rocm/platform.py and xpu/platform.py import their patch hooks at module level, but those hooks only import the patch bodies inside apply_patches(), which runs from the platform constructor.

A subprocess test covers the runtime side of the same contract from the RFC's enforcement section: with torch_npu, vllm_ascend, aiter, yr, torchada, vllm_musa and amdsmi blocked in sys.modules, the detector, platforms.interface, the attention backend registry and distributed.omni_connectors (+ factory) import, detection resolves, no blocked SDK gets populated, and no platform class other than the detected one is loaded.

docs/design/module/execution_platforms.md gets an enforcement pointer under PLATFORM-INV-002. No production code changes.

Not in this PR: the behavior freezes step 1 also lists (fallback, patch timing, packed metadata, unsupported-case behavior). Those need NPU/ROCm hardware to pin and belong with the owners of each migration item.

Test Coverage

pytest tests/platforms/test_platform_import_boundary.py -m "core_model and cpu" at head b02913c (main d6bd0fd merged in, vLLM 0.32.0): 9 passed (1 discovery, 2 neutral-rule, 5 per-backend eager-rule, 1 SDK-isolation subprocess).
Negative probes, run locally and reverted: a temporary neutral module with a function-level from vllm_omni.platforms.npu import platform fails the neutral rule naming the module and line; appending from vllm_omni.platforms.npu import omni_connectors to cuda/platform.py fails the CUDA eager rule; rewriting one allowlisted import fails with the "remove this stale entry" message.
tools/pre_commit/check_test_marks.py, ruff 0.14.10 check/format, markdownlint: clean.

Validation

L1 only (CPU). tests/platforms/ is already collected by the core_model and cpu step, and tests/helpers/ is not touched, so no Buildkite change.

armaanamatya · 2026-09-23T01:06:00Z

Self-review:

Grepped the tree for every vllm_omni.platforms.<backend> import outside the platform packages before writing the allowlist; the test finds the same eight and nothing else, and the allowlist maps each to the [RFC]: In-Tree Platform Ownership Boundary and Migration Plan #6691 migration item that removes it. The detector's from vllm_omni.platforms.xpu import XPUOmniPlatform is the only non-NPU entry: it sets dist_backend during detection, which the RFC's XPU row already lists as housekeeping.
The eager-import rule follows module-level imports transitively through vllm_omni.platforms so a thin hook cannot hide a body import, and it treats diffusion.attention.backends.registry as an accepted registry. rocm/platform.py and xpu/platform.py import their patch hooks at module level, but both hooks only import the bodies inside apply_patches(), which runs from the constructor, so the rule is clean on all five backends today.
TYPE_CHECKING blocks are skipped in both rules. Relative imports are resolved against the package; from pkg import name resolves to pkg.name when that module exists on disk, otherwise to pkg.
Negative probes (reverted): a temporary neutral module with a function-level from vllm_omni.platforms.npu import platform fails the neutral rule naming the module and line; appending from vllm_omni.platforms.npu import omni_connectors to cuda/platform.py fails the CUDA eager rule; rewriting one allowlisted import fails with the stale-entry message. All three restored to a clean tree.
The subprocess probe blocks torch_npu, vllm_ascend, aiter, yr, torchada, vllm_musa, amdsmi via sys.modules[name] = None; the detector and the connector package import, and no platform class other than the detected one is loaded.
9 passed locally (WSL, vLLM 0.30.0); check_test_marks.py, ruff and markdownlint clean. No production code changes, L1 CPU only.

Not included: the behaviour freezes step 1 also lists (fallback, patch timing, packed metadata, unsupported cases) need NPU/ROCm hardware and belong with each migration item's owner.

@hsliuustc0106 (proposal owner) @gcanlin @tjtanaa @xuechendi (platforms) @yenuo26 (CI) @princepride (connectors, most of the allowlist) @Isotr0py @david6666666 (diffusion, the flash_attn entry): rollout step 1 of #6691, PTAL.

@vllm-omni-review-bot

vllm-omni-review-bot · 2026-09-23T01:09:19Z

This PR appears to belong to: docs/design/module/execution_platforms.md.

Module owners: @gcanlin @tjtanaa @xuechendi

Routing: @gcanlin via semantic router; @tjtanaa via semantic router; @xuechendi via semantic router

@armaanamatya, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer.

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

armaanamatya · 2026-09-26T05:25:10Z

@gcanlin @tjtanaa @xuechendi could one of you review this? It adds the import-boundary test from #6691: it fails when code outside vllm_omni/platforms/ imports a backend package directly, with an allowlist of the eight existing imports, each mapped to the #6691 item that removes it. Checks are green and it is mergeable.

armaanamatya · 2026-10-01T04:32:23Z

Re-validated against current main (27a6321a): with this PR's commit applied on top, tests/platforms/test_platform_import_boundary.py gives 9 passed (CPU, vLLM 0.30.0), including the subprocess SDK-isolation test. The allowlist is still exact: main has gained no new neutral import of a concrete platform package and lost none of the eight listed. All GitHub checks are green and it is mergeable.

@gcanlin @tjtanaa @xuechendi could one of you review and add ready so Buildkite runs? It adds tests and a docs pointer only, no production code.

@vllm-omni-review-bot please review.

vllm-omni-review-bot · 2026-10-06T20:03:19Z

Omni ReviewBot routing record

Assigned Strict on zcode (GLM-5.3-Flash) under experiment fleet-strict-cursor-grok46-zcode-glm53flash-5050-c5-z10-20261002.

armaanamatya · 2026-10-07T20:17:20Z

Rebased onto main (744c202d8), no conflicts; head is now c9e0c04e8. Main has since taken the vLLM 0.31.0 rebase and several model changes, so I re-ran the test: tests/platforms/test_platform_import_boundary.py gives 9 passed (CPU, vLLM 0.31.0), including the subprocess SDK-isolation test. The allowlist is still exact: no new neutral import of a concrete platform package on main, and none of the eight listed has gone.

@gcanlin @tjtanaa @xuechendi could one of you review and add ready so Buildkite runs? It adds one test file and no runtime code.

@vllm-omni-review-bot please review.

vllm-omni-review-bot · 2026-10-10T03:30:18Z

Omni ReviewBot attempt record

Review attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: eaf9929d-33c1-489e-8919-11d6b6bff035) — check zcode login and the CLI version; retrying strict/zcode/GLM-5.3-Flash in 120s (try ).

vllm-omni-review-bot · 2026-10-10T04:08:45Z

Omni ReviewBot attempt record

Review attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: 3d538586-8ad6-4bbe-b1fc-b65932da9f54) — check zcode login and the CLI version; retrying strict/zcode/GLM-5.3-Flash in 600s (try ).

vllm-omni-review-bot · 2026-10-10T04:54:58Z

Omni ReviewBot attempt record

Review attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: 45b5fca0-69ee-4a61-9d31-8cc9b50b82ea) — check zcode login and the CLI version; falling back to direct/cursor/auto).

vllm-omni-review-bot

Omni ReviewBot review

0 actionable finding(s).

CI at c9e0c04e8d4e (2026-10-10T04:55:02.039483+00:00): required check(s) blocking: buildkite/vllm-omni (missing).

Note: The assigned review arm strict/zcode/GLM-5.3-Flash could not complete this review, so it was produced by the fallback arm direct/cursor/auto. It is excluded from the routing experiment.

Full review analysis

PR description

This change adds a CPU import-boundary test and a short enforcement note under PLATFORM-INV-002. The test parses vllm_omni with ast and fails when neutral modules import a concrete vllm_omni.platforms.<backend> module, or when importing a backend package and its platform module eagerly reaches model, layer, quantization, attention-backend, or connector bodies. Eight existing neutral imports stay on a named allowlist that also fails if an entry no longer exists; the eager-import allowlist is empty. A subprocess imports the detector, the platform interface, the attention-backend registry, and distributed.omni_connectors with optional hardware SDKs blocked in sys.modules. No production runtime path changes.

Change flow

flowchart LR
  tree["[EXISTING] vllm_omni import graph"]:::existing
  test["[NEW] tests/platforms/test_platform_import_boundary.py"]:::new
  debt["[NEW] Eight-entry neutral allowlist"]:::new
  docs["[CHANGED] PLATFORM-INV-002 enforcement note"]:::changed
  ci["[EXISTING] Simple Other Test core_model and cpu"]:::existing
  tree --> test
  debt --> test
  test --> docs
  test --> ci
  classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
  classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
  classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
  classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px

No actionable findings.

_{🤖 This review was generated by InferMatrix Copilot, an open-source repo-maintenance agent for PR review, CI debugging and issue triage. Try it on your own repo, and ⭐ star it if it helped!}

Neutral modules may not import a concrete vllm_omni.platforms.<backend> module, and importing a backend's platform class may not eagerly load model, layer, quantization or connector implementation bodies. Both rules are checked statically over the source tree; the eight existing neutral violations are recorded as an exact, named allowlist that fails the test when it goes stale, so it can only shrink. A subprocess test imports the detector and the neutral connector code with every optional hardware SDK blocked. Rollout step 1 of vllm-project#6691. Signed-off-by: Armaan Amatya <armaanamatya2014@gmail.com>

…raph_tools vllm-project#8515 added function-level imports of vllm_omni.platforms.npu.graph_tools to the MiniCPM-o 4.5 encoder graphs, which the neutral-import rule rejects. Add the two (module, target) pairs to NEUTRAL_IMPORT_ALLOWLIST under NPU group 1, the RFC item that moves the MiniCPM-o graph helper to its consumer package. Signed-off-by: Armaan Amatya <armaanamatya2014@gmail.com>

armaanamatya · 2026-10-11T05:15:13Z

Rebased onto main (b1fc5fcfa); head is now 373072826. Main's #8515 added function-level imports of vllm_omni.platforms.npu.graph_tools to the MiniCPM-o 4.5 encoder graphs (encoder_graph.py, flow_encoder_graph.py), which the neutral rule catches, so the rebased test failed until I added those two (module, target) pairs to the allowlist under NPU group 1, the RFC item that moves the MiniCPM-o graph helper to its consumer package. That is the second commit, and the table in the description has the two rows. 9 passed (CPU, vLLM 0.31.0), ruff clean. For what it is worth, 67 commits of main drift produced exactly one new boundary crossing, which is the case this test exists for; the list is at ten entries.

@gcanlin @tjtanaa @xuechendi @hsliuustc0106 could one of you review and add ready so Buildkite runs? One test file, no runtime code.

@vllm-omni-review-bot please review.

…ms.npu.models.step_audio2_token2wav Main's vllm-project#8735 added a function-level import of the StepAudio2 token2wav NPU SDPA context to minicpmo_4_5/cuda_graph_wrapper.py, which the neutral rule catches. It is the same NPU group 1 debt as the existing batched_token2wav entry. Signed-off-by: Armaan Amatya <armaanamatya2014@gmail.com>

armaanamatya · 2026-10-17T07:25:08Z

Merged main (959b483cf) into the branch; head is now 021d98fe5. Main's #8735 added a function-level import of vllm_omni.platforms.npu.models.step_audio2_token2wav to minicpmo_4_5/cuda_graph_wrapper.py (the NPU SDPA context for Code2Wav graph capture), so the neutral rule failed on the merged tree. I added that pair to the allowlist under NPU group 1, next to the existing batched_token2wav entry with the same target, and added the row to the table in the description. tests/platforms/test_platform_import_boundary.py -m "core_model and cpu": 9 passed.

…d CFM DiT modules Main's vllm-project#8908 added function-level imports of vllm_omni.platforms.npu.models.cosyvoice2_dit_attn and vllm_omni.platforms.npu.models.minicpmo_fused to minicpmo_4_5/batched_token2wav.py. List both under NPU group 1. Signed-off-by: Armaan Amatya <armaanamatya2014@gmail.com>

armaanamatya · 2026-10-21T02:06:53Z

Merged main (aa3fd1d9b) into the branch; head is now e79ab67fd. Main's #8908 added function-level imports of vllm_omni.platforms.npu.models.cosyvoice2_dit_attn and vllm_omni.platforms.npu.models.minicpmo_fused to minicpmo_4_5/batched_token2wav.py, so the neutral rule failed on the merged tree. I listed both under NPU group 1 and added the two rows to the description's table. tests/platforms/test_platform_import_boundary.py -m "core_model and cpu": 9 passed (CPU, vLLM 0.32.0), ruff clean.

@hsliuustc0106 @yenuo26 @NickCao @Gaohan123 @david6666666 could one of you review and add ready? It is one test file and no runtime code. Each week of main drift adds another boundary crossing, and this test is what catches them.

@vllm-omni-review-bot please review.

vllm-omni-review-bot · 2026-10-28T02:18:24Z

Omni ReviewBot: no human activity for 7 days

@armaanamatya this pull request has had no human commit, comment or review since 2026-10-21. Please confirm the current plan and next step. The author or a maintainer decides whether to change the PR state.

To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline.

Signed-off-by: Armaan Amatya <armaanamatya2014@gmail.com>

armaanamatya · 2026-10-29T01:13:41Z

Plan: the PR is complete and waiting for a review from the platform/CI owners; there is no open blocker on my side.

Merged main d6bd0fd16; head is now b02913c47. #9106 added three function-level NPU imports under model_executor/models/fish_omni/ (NPU model patch, NPU model state, NPU quant method), which the neutral rule caught on the merged tree. They are tracked in NEUTRAL_IMPORT_ALLOWLIST with their RFC items and added to the table in the description. tests/platforms/test_platform_import_boundary.py: 9 passed.

@yenuo26 @NickCao @hsliuustc0106 @Gaohan123 @david6666666 could one of you review? The allowlist drifts with every new NPU import on main, so it is easiest to land soon.

@vllm-omni-review-bot please review.

yenuo26

Reviewed tip b02913c47 (personal PTAL / requested review).

Looks good for landing. The static AST walk correctly covers nested/control-flow imports, skips TYPE_CHECKING, resolves relative/from pkg import name against on-disk modules, and the shrink-only allowlists (unexpected + stale) match the RFC #6691 debt model. Empty PLATFORM_EAGER_IMPORT_ALLOWLIST is the right bar for R4 today. The SDK-isolation subprocess probe (blocked optional hardware modules + detector/neutral probes + no non-detected platform class load) is a useful runtime complement to the static rules. Docs enforcement note under PLATFORM-INV-002 points at the right test.

CI selection: no Buildkite YAML change needed — tests/platforms/test_platform_import_boundary.py is marked core_model + cpu and is covered by READY’s catch-all pytest -sv tests/ -m 'core_model and cpu' (with the usual diffusion/model_executor/entrypoints ignores). No redundant tee/upload or one-off _surviving_labels selection tests.

Author evidence is solid (9 passed locally incl. subprocess; grepped allowlist; negative probes reverted). Soft non-blocking notes:

The neutral allowlist has grown with main drift (#8515/#8735/#8908/#9106). That is the intended ratchet, but please keep landing the matching RFC migration items so the list shrinks rather than becoming a permanent catalog.
Soft residual: Buildkite READY still needs the ready label to run; once green this is fine to merge from my side.

No blockers from me.

armaanamatya requested review from Gaohan123, NickCao, david6666666, hsliuustc0106 and yenuo26 as code owners September 23, 2026 01:05

hsliuustc0106 added CI/CD codes related to changes to CI/CD high priority high priority issue, needs to be done asap labels Sep 24, 2026

armaanamatya force-pushed the platform-import-boundary branch from 6f116b9 to c9e0c04 Compare October 7, 2026 20:11

This was referenced Oct 8, 2026

[Bugfix][Distributed] Resolve optional connectors lazily instead of at import #8624

Open

[Bug][Ascend]: importing vllm_omni aborts the process at exit when the Mooncake NPU wheel is installed #8628

Closed

vllm-omni-review-bot reviewed Oct 10, 2026

View reviewed changes

armaanamatya added 2 commits October 11, 2026 00:11

armaanamatya force-pushed the platform-import-boundary branch from c9e0c04 to 3730728 Compare October 11, 2026 05:14

armaanamatya and others added 3 commits October 13, 2026 18:01

Merge branch 'main' into platform-import-boundary

51f7490

Merge branch 'main' into platform-import-boundary

6b2b9d9

armaanamatya added 2 commits October 20, 2026 21:04

Merge remote-tracking branch 'origin/main' into platform-import-boundary

56bec4e

armaanamatya added 2 commits October 28, 2026 19:53

Merge remote-tracking branch 'origin/main' into merge-8029-1028

4713fdf

[CI] Track the Drama 3 NPU imports added by vllm-project#9106

b02913c

Signed-off-by: Armaan Amatya <armaanamatya2014@gmail.com>

yenuo26 reviewed Oct 30, 2026

View reviewed changes

Conversation

armaanamatya commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Test Coverage

Validation

Uh oh!

armaanamatya commented Sep 23, 2026

Uh oh!

vllm-omni-review-bot commented Sep 23, 2026

Uh oh!

armaanamatya commented Sep 26, 2026

Uh oh!

armaanamatya commented Oct 1, 2026

Uh oh!

vllm-omni-review-bot commented Oct 6, 2026

Uh oh!

armaanamatya commented Oct 7, 2026

Uh oh!

vllm-omni-review-bot commented Oct 10, 2026

Uh oh!

vllm-omni-review-bot commented Oct 10, 2026

Uh oh!

vllm-omni-review-bot commented Oct 10, 2026

Uh oh!

vllm-omni-review-bot left a comment

Choose a reason for hiding this comment

Omni ReviewBot review

PR description

Change flow

Uh oh!

armaanamatya commented Oct 11, 2026

Uh oh!

armaanamatya commented Oct 17, 2026

Uh oh!

armaanamatya commented Oct 21, 2026

Uh oh!

vllm-omni-review-bot commented Oct 28, 2026

Omni ReviewBot: no human activity for 7 days

Uh oh!

armaanamatya commented Oct 29, 2026

Uh oh!

yenuo26 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

armaanamatya commented Sep 23, 2026 •

edited

Loading