[y-null]

Summary

This PR lands four validated items on top of main. The branch is now merged with main (9778e52bd); the sampling-kernel benchmark below was measured before that merge (branch 7fd4b38d6 vs main 21ef5b4bb), and the batched-cache-split table is the latest same-day measurement.

1. Triton fused sampling kernel for the MiniCPM-o 4.5 talker (talker_codec_sample.py): merges repetition-penalty lookup and temperature scaling into a single Triton kernel (_rep_pen_temp_kernel), a bit-for-bit twin of the eager _repetition_penalty_scaling + logits / temperature pair (integer counts, same exp2/log2 pow form, same op order). Removes per-token host-side op dispatch in the stage1 sampling path. If the kernel fails to compile or launch at runtime, a process-level latch switches the sampling path back to the eager chain for the rest of the process; there is no env switch to manage, and talker_codec_fused_prelude: false in the stage connector's extra block forces the eager chain for accuracy triage (a no-op off NPU or without Triton).
2. Device speaker-embedding placement (minicpmo_4_5_code2wav.py): token2wav_device_spk: false in the stage extra config falls back to the CPU onnxruntime path, for accuracy triage. A logger line records which branch is taken.
3. Unit tests for the fused prelude (tests/model_executor/models/minicpmo_4_5/test_talker_codec_fused_prelude.py): bit-exactness of the fused path against the eager chain across 5 cases (history_len < window, repeated ids, negative logits, -inf entries, penalty+temperature), plus capture and replay of the kernel under torch.npu.NPUGraph compared against eager.
4. Batched per-request cache split in Code2Wav (batched_token2wav.py): _split_flow_cache now splits the conformer cnn/att caches and estimator_cnn with three batched index_select calls over cached row-order tensors, instead of two clone calls and one cat per row (3B small eager launches -> 3). The CFG-interleaved order [cond0, uncond0, cond1, ...] reproduces the previous per-row cat([row, batch + row], dim=2) selection exactly, and index_select allocates a fresh buffer, preserving the graph-output isolation the clones provided. estimator_att_cache keeps its existing per-request copy/direct path (coupled to keep_ranges/_copy_frame_segments).

Hardware / config

Ascend 910B3, single card / single replica, MiniCPM-o 4.5, num_speculative_tokens: 7, codec_chunk_frames: 25, CFM Euler steps = 10.

Bench: main vs this branch for the sampling kernel (same-day A/B, seed-tts en, 256 prompts per tier)

Three tiers (client concurrency x stage2 replicas), bench serve, mean / median audio RTF:

Tier (conc x repl)	main (21ef5b4)	this branch (7fd4b38)	delta (mean)
32 x 1	0.3435 / 0.3326	0.3383 / 0.3310	-1.5% (within run-to-run noise)
64 x 4	0.7486 / 0.7291	0.7105 / 0.7069	-5.1%
128 x 8	1.2724 / 1.2421	1.2119 / 1.1698	-4.8%

The gains concentrate on the concurrent tiers, consistent with the fused kernel removing host-side sampling dispatch whose cost scales with request rate. The 32x1 tier runs mostly warm single-request paths and stays neutral.

Three-tier A/B: batched per-request cache split (item 4)

Same card (910B3, card 5), same day, seed-tts en, bench_ab_tier.sh, median audio RTF. Baseline arm = parent commit b5ec4b663; the test arm runs the _split_flow_cache change in this commit. (The measurement snapshot additionally carried a HiFT graph-runner hunk that is unreachable when enable_hift_graph: true — the shipped config — and is not part of this commit, so it cannot affect these numbers.)

Tier (conc x repl)	without batched split	with batched split	delta
32 x 1	0.3255	0.3223	-1.0%
64 x 4	0.6837	0.6791	-0.7%
128 x 8	1.1724	1.1522	-1.7%

The split saves 3(B - 1) eager launches per chunk, where B is the number of rows in the split batch and is capped by stage2 max_num_seqs: 8. That predicts no gain at 32x1 (B=1: 3 -> 3 launches, so the -1.0% is within the c1 run-to-run spread) and a gain that grows with concurrency, which matches the measured deltas. This is a sub-5% item kept as a low-risk incremental change; the cache contents are bit-identical (see the WER gate).

Accuracy gate (WER, seed-tts en, 20 prompts, disable-shuffle protocol, CPU ASR eval)

Branch / arm	mean WER	ASR failures
main (21ef5b4)	0.01218	0
this branch	0.01218	0
batched-split A/B (both arms)	0.0122	0

WER is bit-for-bit identical in all arms and matches the 22-run historical baseline, as expected from the exact numeric twin kernel and the order-preserving cache split.

Notes on graph capture (per review)

The capture scope in the test is the fused kernel itself (the unit production wraps), not the whole prepare_codec_logits chain: the shared censoring step allocates its stop_index tensor per call from a Python tuple (H2D), which is not a capture-safe pattern and is independent of the fused prelude. Worth a separate follow-up if full-chain per-request capture lands.

Test plan

• Same-day A/B three-tier bench for the sampling kernel (above)
• WER gate on both branches
• Same-day A/B three-tier bench for the batched cache split (above)
• New unit tests: 5 bit-exactness cases + NPUGraph capture/replay, 6 passed on NPU (910B3)
• Code2Wav batching tests: 135 passed; test_npu_patch_preserves_ragged_estimator_contract fails identically on the parent commit (container subprocess aborts on missing vllm._deepselect_C), pre-existing environment issue
• pre-commit (ruff / format / mypy / test marks / SPDX / DCO) passes

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-19T03:50:29.830856Z	164bceb	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[y-null]

@amy-why-3459 @natureofnature PTAL

[vllm-omni-review-bot]

This PR appears to belong to: docs/design/module/ar_runtime.md, docs/design/module/model_integration.md.

Module owners: @tzhouam @fake0fan @Gaohan123

Routing: @tzhouam via module of the changed files, CODEOWNERS; @fake0fan via module of the changed files; @Gaohan123 via module of the changed files

@y-null, 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.

[amy-why-3459]

Thanks for the same-day A/B and the WER gate. A few comments inline, mainly about test coverage for the new default-on fused path and the shuffle switch overlapping an existing flag.

[amy-why-3459]

This makes the Triton prologue the default for every stochastic codec sample on NPU, but the PR adds no test for it. Since the argument rests on bit-for-bit parity with _repetition_penalty_scaling + / temperature, could you add a unit test (skipped without NPU/Triton) that compares prepare_codec_logits with the fused path on and off? Cases worth covering: history_len < HISTORY_WINDOW, repeated ids in the window, negative logits, -inf stop-censored entries, and a penalty other than 1.0. The 20-prompt WER gate only checks parity indirectly, and only at one penalty value.

Also, _codec_sample_setup in minicpmo_4_5_omni_tts.py says this sampler runs under a per-request sampling graph. If that capture is active on NPU, the same test should capture and replay with the fused kernel too. A Triton launch under graph capture is a different support path from an eager launch.

[y-null]

Done - added tests/model_executor/models/minicpmo_4_5/test_talker_codec_fused_prelude.py (skipped without NPU + Triton, same as the fused path ship target):

• 5 bit-exactness cases vs the eager chain through prepare_codec_logits with the switch forced on/off: history_len < HISTORY_WINDOW, repeated ids inside the window, negative logits, -inf entries, penalty != 1.0 + temperature != 1.0. All torch.equal on 910B3.
• Graph capture: capture + replay of the kernel under torch.npu.NPUGraph (the production wrappers use torch.npu.NPUGraph + torch.npu.graph, not the CUDA-only API, which is a dummy on NPU). Replay output matches the eager chain bit-for-bit.
• Capture scope note: the test captures the fused kernel itself, not the whole prepare_codec_logits chain - the shared censoring step allocates its stop_index tensor per call from a Python tuple (H2D), which is not capture-safe and is independent of the fused prelude. Recorded in the PR description; happy to follow up if full-chain per-request capture lands. 6 passed on NPU.

[amy-why-3459]

TALKER_CODEC_FUSED_PRELUDE defaults to on and is read once at import, but the PR description doesn't mention it (it only lists the other two switches). Could you add it there, so anyone bisecting a codec-sampling change knows there's an off switch?

[y-null]

Updated the PR description: the TALKER_CODEC_FUSED_PRELUDE=0 opt-out (read once at import, reverts to the eager chain) is now documented in the summary.

[amy-why-3459]

benchmarks/patch/patch.py already passes disable_shuffle=getattr(args, "disable_shuffle", False) into this loader, and the perf configs (e.g. tests/dfx/perf/tests/test_minicpmo_4_5.json) already set "disable_shuffle": true. So --disable-shuffle already pins the order. An env-only switch that silently overrides the constructor argument means the command line alone no longer tells you whether a run was shuffled. Could the WER protocol use --disable-shuffle instead, and drop this check?

[y-null]

Agreed - --disable-shuffle already covers the protocol (and the perf config sets it), and a silent env override is worse than an explicit CLI flag. Dropped the seed_tts_dataset.py change from this PR (reverted to upstream); the disable-shuffle protocol is what the WER numbers in the description use.

[amy-why-3459]

The neighbouring toggles in _build_backend (token2wav_trt, token2wav_trt_dtype) read from the stage extra config with an env fallback, but this one is env-only. Reading it from extra with VLLM_OMNI_DEVICE_SPK as the fallback would keep it consistent and let it be set per deployment. A logger.info on the CPU-fallback branch would also show in the logs which speaker-embedding path a triage run actually used.

[y-null]

Done - _build_backend now reads token2wav_device_spk from the stage extra config (default true) with VLLM_OMNI_DEVICE_SPK=0 as an env opt-out, matching the neighbours extra-then-env pattern, and both branches log which path is taken.

[amy-why-3459]

Thanks for the quick turnaround. I looked at the changes since my last review (new fused-prelude test, token2wav_device_spk config switch, dropped shuffle env). The config/env switch and the logging look good, and moving the WER protocol to --disable-shuffle makes sense. One issue in the new test file, plus a small docstring nit, inline.

[amy-why-3459]

This runs at import time, so the skipif guards don't protect it. Without torch_npu loaded, npu isn't a registered device type, and torch.device("npu") raises RuntimeError during collection. Any CPU/GPU job that collects tests/model_executor/models/ (marker filtering happens only after import) would then report a collection error for this file instead of skipping it. The existing NPU tests (moss_tts/test_npu_codec_parity.py, common/test_alias_free_activation_npu.py) build torch.device("npu") inside the test bodies for this reason. Could you make it lazy, e.g. a fixture or a local in _make_state/_run_both, and maybe add a module-level pytest.mark.skipif(not current_omni_platform.is_npu(), ...) like test_alias_free_activation_npu.py does?

[natureofnature]

Agree with @amy-why-3459. At 7fd4b38, _DEVICE is constructed at module scope before the availability guards, and hardware_marks only adds markers. On CPU/CUDA installations without the NPU backend registered, importing this file fails before pytest can skip or deselect its tests. Deferring device construction until after the availability check should let an unfiltered run of this file skip all six cases, and CPU/CUDA marker-filtered collection deselect them cleanly; the six NPU cases should still pass.

[y-null]

Fixed in b5ec4b6. Module scope no longer constructs a device: _DEVICE = torch.device("npu") is gone, and a lazy _device() helper is called from _make_state, _run_both and the test bodies. I also added the module-level pytest.mark.skipif(not current_omni_platform.is_npu(), reason="requires Ascend NPU") marker, following test_alias_free_activation_npu.py.

Verification: 6 passed on 910B3 (card 5). I could not reproduce a bare CPU-only environment inside the NPU container - blocking torch_npu there also breaks the shared tests/helpers/fixtures/config plugin import, which is unrelated to this file - so the skip path was checked by substituting a non-NPU platform marker: all 6 cases report SKIPPED ... requires Ascend NPU instead of a collection error.

[y-null]

Fixed in b5ec4b6 exactly as described: the module-level _DEVICE is gone, device construction is deferred to a lazy _device() invoked from the helpers and test bodies, and the module now carries pytest.mark.skipif(not current_omni_platform.is_npu(), ...). With a non-NPU platform marker all six cases skip (6 skipped), and the six NPU cases still pass on 910B3 (card 5, 6 passed).

[amy-why-3459]

Nit: the docstring says the capture-phase output is compared on the captured inputs, but the test checks the eager warmup launch on raw_a, and the comment below correctly notes that static_out is meaningless before replay. Could you reword it to match what's actually asserted? Same for _run_both: its docstring says monkeypatch restores the flag, but the code uses try/finally.

[y-null]

Fixed. The graph test docstring now says the eager warmup launch (which also compiles the kernel) is what gets asserted on the captured inputs, and that the capture-phase static_out is explicitly not asserted because capture records ops without executing them. _run_both now says the flag is restored in a try/finally instead of implying monkeypatch. Also corrected the module docstring from CUDA graph to torch.npu.NPUGraph, which is what the test actually uses.

[natureofnature]

Reviewed all three changed files and the sampling/state and speaker-embedding call chains. The new equality cases and reported six passing tests on 910B3 address the earlier fused-kernel parity/capture concern; I haven't independently run those hardware tests. The remaining blocking issue is the non-NPU test-collection failure discussed in the existing thread: #9077 (comment).

【Conclusion: Found blocking code issues in commit 7fd4b38.】
【Action required: Defer NPU device construction until after the availability check; verify non-NPU runs skip all six cases cleanly and NPU tests still pass before merging.】

[amy-why-3459]

Re-checked b5ec4b6 against 7fd4b38 (test-only change). Both earlier points are addressed: nothing at module scope constructs an npu device anymore (_device() is only called from _make_state, _run_both and the test bodies, and CASES holds plain Python values), and the module-level skipif(not current_omni_platform.is_npu()) matches test_alias_free_activation_npu.py. The reworded docstrings now describe what is actually asserted (eager warmup on the captured inputs, replay vs. eager on the replayed inputs, try/finally restore). No further comments on this delta.

[amy-why-3459]

Looked at the new commit 7bd410c (batched per-request cache split); the merge from main was skipped. The CFG-interleaved order_2n reproduces the old cat([row, B + row], dim=2) selection, and index_select still gives a buffer that is not aliased with the graph's shared outputs, so the isolation is kept. One thing on memory lifetime, inline.

[amy-why-3459]

These are now views into one B-row buffer per key, not owned tensors. Before, each .clone()/cat gave every request its own storage. Now a request's stored flow_cache keeps the whole batch's cnn_split/catt_split/ecnn_split alive until every sibling row from that chunk is replaced. When requests from one chunk are stepped in different later groups (onset/continuation mix, ragged merge, uneven token arrival, one stream finishing early), old full-batch buffers stay pinned. Worst case, the memory for these three caches grows with B per live group instead of with the live row count. conformer_att_cache is the large one, since it holds the full trimmed frame window. Also, catt_split[:, row:row+1] and ecnn_split[:, :, 2r:2r+2] are now non-contiguous. _stack_flow_cache and FlowEncoderGraphs.run (torch.cat(..., out=)) handle that fine, but any op that later needs a contiguous input would add a copy.

Could you check the peak and steady-state allocated memory at 128x8 against the parent commit? With a ~1% RTF gain, it would be good to confirm this doesn't trade back memory headroom. If it does, torch.split/unbind won't help (same storage). Either keep the per-row copy for conformer_att_cache only, or document the shared-lifetime assumption here. Minor: index_select(0, arange(B)) on conformer_cnn_cache is just a .clone(), so _split_order_row can be dropped.

[y-null]

Measured at 128x8 against the parent commit, with an identical procedure on both arms (fresh service on card 1, warm tier 4x1, then 128x8; npu-smi HBM sampled every 2 s for the whole tier):

arm	steady-state median	peak
parent b5ec4b6	58492 MB	58495 MB
this branch (7bd410c + fix)	58810 MB	58813 MB

That is +318 MB (+0.5% of 65536 MB), so free headroom goes from 7.04 GB to 6.72 GB. estimator_att_cache (the ~0.5 GiB/request cache) is untouched, so the delta comes from the three small keys; it matches one extra chunk's buffers staying pinned while a chunk's rows are replaced. Each generation is replaced every chunk, so the cost is bounded by B rows of these keys rather than cumulative. (An earlier 59235 MB sample was taken after the full 32/1 + 64/4 + 128/8 + WER sequence on the same process; it also holds the graphs those tiers captured, so it is not comparable to the parent arm.)

For the strided rows, the consumers are fine: _stack_flow_cache concatenates them (batched_token2wav.py:1688/1691), and FlowEncoderGraphs.run copies them into its static views with torch.cat(..., out=...) (flow_encoder_graph.py:361-362), so the non-contiguous [:, r:r+1] / [:, :, 2r:2r+2] rows do not add a copy.

Took the minor as well: conformer_cnn/conformer_att now use .clone() (the arange index_select was an identity permutation), and _split_order_row is gone. I documented the shared-lifetime assumption in the docstring rather than keeping the per-row copy for conformer_att_cache, given the measured trade (318 MB against 6.7 GB of headroom); if you would rather have the per-row copy for att, it is a one-line change and I can re-measure. Commit 087e0e9.

[amy-why-3459]

Thanks for measuring both arms. Looked at 087e0e9 only. Dropping _split_order_row in favor of a plain .clone() for conformer_cnn_cache/conformer_att_cache is equivalent to the old identity index_select, and +318 MB at 128x8 with estimator_att_cache untouched matches the one-extra-chunk explanation. One wording note on the new lifetime docstring inline.

[amy-why-3459]

The "at most one extra chunk's buffers are pinned" bound only holds when every sibling row is in the next decode call. Regrouping inside one call is fine, since exact_groups/encoder_groups still replace all rows in that step. But a request that isn't in the next batch at all (e.g. its next token chunk isn't ready yet while siblings advance) keeps its whole B-row cnn_split/catt_split/ecnn_split alive until it steps again. The 128x8 run is the uniform case. Maybe phrase this as an assumption ("when a chunk's rows advance together") and mention that a lagging row pins its chunk's buffers, so the next person changing scheduling knows to re-check memory.

[y-null]

Agreed -- reworded as an assumption in b1047e7. The Lifetime paragraph now scopes the one-extra-chunk bound to the case where a chunk's rows advance together (the uniform 128x8 run), and it states that a request dropping out of the next batch keeps its whole B-row cnn/att/estimator_cnn buffers alive until it steps again, so a scheduling change that can strand rows across batches must re-check the bound.

[hsliuustc0106]

[P2] Classify the new environment-variable reads to repair the failing CPU test.

@y-null, current-head CUDA build 17583 / Simple Other Test fails test_statically_resolvable_environment_accesses_are_classified: the new TALKER_CODEC_FUSED_PRELUDE read and VLLM_OMNI_DEVICE_SPK read are not classified in the reviewed inventory. The actual job reports 1 failed, 6244 passed, 42 skipped; this failure is not an infrastructure diagnosis.

• Add both entries to vllm_omni/config/environment_variable_inventory.py with explicit category/disposition and update the reviewed snapshot counts. If classified public, also satisfy the public naming/documentation contract.
• Run pytest tests/config/test_environment_variables.py and report fix SHA, environment and result. Acceptance: the complete file passes, including inventory counts and public-name/documentation checks.

Scope: CI diagnosis and the two changed reads, not a completed full-PR review. I read the CI log and source; no local tests executed. AI assistance: OpenAI assistant prepared this review.

[hsliuustc0106]

Review of 0a7dc5075865e77ca2fde55e845b082080eb4b18:

• P2: Parse the speaker-embedding opt-out strictly. @y-null, the new bool(extra.get("token2wav_device_spk", True)) branch treats a quoted YAML/JSON value "false" (or "0") as true. The connector extra mapping is passed through unchanged, so a deployment attempting to isolate an NPU speaker-embedding accuracy regression silently keeps the device path enabled instead of selecting CPU onnxruntime. Please apply the same strict true/false/1/0 parsing already used for talker_codec_fused_prelude, or reject non-boolean values explicitly. Acceptance: regression coverage reaches the actual backend-selection branch with false and quoted false, proves enable_device_spk_embedding() is not called, and rejects invalid values. Status: awaiting author fix and regression evidence.

Verified complete separately: the two environment-variable reads from my earlier CI repair request are absent from these current production files; this removes that source-level cause, not proof of a new CUDA pass.

Coverage: complete six-file diff and relevant sampler, setup/config and cache-split consumers inspected. Source-only plus a Python built-in truthiness check; no model/NPU/CUDA tests executed. Current-head CUDA evidence remains unavailable. AI assistance: OpenAI assistant prepared this review.

[y-null]

P2 fixed in 4a7486cc4.

• _build_backend now parses token2wav_device_spk with the same strict _bool_flag helper used for talker_codec_fused_prelude, so false / "false" / 0 / "0" actually select the CPU onnxruntime fallback, and anything else raises at startup instead of silently picking a branch via bool("false") is True.
• New regression test tests/model_executor/models/minicpmo_4_5/test_code2wav_device_spk_flag.py drives the real backend-selection branch in _build_backend (only the assets and MiniCPMO45Token2wav are faked, the platform is forced to NPU): it asserts enable_device_spk_embedding() is not called for False / "false" / " FALSE " / 0 / "0", is called exactly once for unset / True / "true" / 1 / "1", and that invalid values ("maybe", "yes", 2, 1.5, []) are rejected.
• Verification: 15 passed with the fix; reverting just that one line makes 8 of those cases fail (the quoted-false ones plus all invalid-value ones).

[amy-why-3459]

Looked at the new commit (4a7486c, strict token2wav_device_spk parsing + test). The fix and the test look right to me; two small notes inline.

[amy-why-3459]

Small coupling nit: importing the private _bool_flag from minicpmo_4_5_omni_tts pulls the whole talker module (LlamaModel, vLLM Sampler, worker_v2.omni_sampler, the duplex handoff, etc.) into the code2wav stage just for a ~15-line parser. Could _bool_flag live in a small shared helper (or in this file next to _tf32_mode) and be imported by both?

[y-null]

Fixed in 63f193c36.

_bool_flag now lives in vllm_omni/model_executor/models/minicpmo_4_5/config_flags.py as parse_bool_flag -- a dependency-free module (only typing), so the code2wav stage imports the parser without pulling in LlamaModel, Sampler, worker_v2.omni_sampler or the duplex handoff. minicpmo_4_5_omni_tts imports it under the old private name (parse_bool_flag as _bool_flag), so the talker_codec_fused_prelude call site and its tests are untouched, and minicpmo_4_5_code2wav imports it at module top instead of doing a function-local import from the talker module.

[amy-why-3459]

The same bool("false") is True issue still applies to other flags read from the same stage extra in this file, e.g. token2wav_float16 (L1482, a quoted "false" would build Token2wav in fp16), token2wav_trt (L1501), code2wav_bfloat16_attention_cache (L1547), cfm_graph_precapture (L1553), and in __init__ enable_hift_graph, enable_cfm_graph, token2wav_ref_prefetch, cfm_onset_merge. Maybe worth routing them through the same parser, either here or separately.

Also, since parsing only happens inside the NPU branch, an invalid value is still accepted silently on CUDA, so "fails at startup" from the commit message holds on NPU only. Parsing before the platform branch would make a typo in a shared YAML fail the same way everywhere.

[y-null]

Fixed in 63f193c36 -- all of them now go through the same strict parser:

• token2wav_float16, token2wav_trt
• code2wav_bfloat16_attention_cache, cfm_graph_precapture
• token2wav_ref_prefetch, token2wav_ref_prefetch_setup
• enable_hift_graph, enable_cfm_graph, cfm_onset_merge

Defaults are unchanged, so existing configs keep their behaviour; only values bool() mis-read ("false", "0", quoted YAML/JSON overrides) change branch, and invalid values now raise at startup instead of silently becoming True.

token2wav_device_spk also moved above the platform branches, so a typo in a shared stage config fails on CUDA/CPU too instead of only where the toggle is consumed.

Tests: test_code2wav_device_spk_flag.py now covers the off-NPU path plus one strict-parse case per flag and the quoted-"false" behaviour; 11 new cases fail on the previous code and pass with this commit.

[amy-why-3459]

Thanks for moving the parser into config_flags.py and parsing token2wav_device_spk before the platform branches, that resolves both of my earlier points. Looked at 63f193c only (the merge from main skipped). One follow-up on coverage below.

[amy-why-3459]

A few booleans from the same extra in this class still go through bool(), so they keep the "false" -> True problem the commit message says is fixed for "the remaining stage-extra booleans":

• enable_code2wav_encoder_graph and code2wav_encoder_capture_after_precapture (_chunk_encoder_graph_config, just above)
• cfm_fused_body, cfm_slot_pool and enable_whole_euler in this dict. cfm_fused_body defaults to on for CUDA (TF32 on) and NPU, so a quoted "false" used to triage the fused body would silently keep it on, which is the same failure mode as token2wav_device_spk.

The ... is True reads (cfm_row_offset_merge, cfm_encoder_cuda_graph, cfm_cross_turn_buckets, cfm_precapture_empty_cache, cfm_drop_upstream_att_buffer) fail the other way: a quoted "true" is silently ignored. Routing them through parse_bool_flag too would make every flag in this class behave the same way.

Small related behaviour change: an explicit null (e.g. cfm_graph_precapture: ~ in a YAML override) used to evaluate to False via bool(None) and now raises. That's probably fine, but it slightly contradicts "defaults are unchanged; only values bool() mis-read change behaviour".

[y-null]

All of these are routed through the parser in 505d911c5, plus the null point.

config_flags.py now also has read_bool_flag(extra, key, default): strict like parse_bool_flag, but an explicit null (cfm_graph_precapture: ~) means "unset" and falls back to the default, so removing/blanking a key never flips a feature off -- that restores the "defaults are unchanged" promise.

Converted:

• bool() reads: enable_code2wav_encoder_graph, code2wav_encoder_capture_after_precapture, cfm_fused_body, cfm_slot_pool, enable_whole_euler
• ... is True reads: cfm_row_offset_merge, cfm_encoder_cuda_graph, cfm_cross_turn_buckets, cfm_precapture_empty_cache, cfm_drop_upstream_att_buffer

At this point I grepped every extra.get(...) key in the file: no bool(extra...) and no extra.get(...) is True/False reads of stage config remain in this class, so "false"/"true"/0/1 behave identically for all 19 flags.

Coverage added: one invalid-value case per flag (including these 10), quoted "false" turning off the default-on cfm_fused_body/enable_whole_euler, quoted "true" turning on the five former is True flags, and a null case asserting the defaults survive. 39 passed on the flag test file, 51 passed including the fused-prelude tests, ruff clean.

[amy-why-3459]

Re-checked the delta since 63f193c (505d911: all code2wav stage-extra booleans now go through read_bool_flag). The conversion covers every boolean read in __init__ / _build_backend, and the null-means-default rule plus the per-flag tests look good. One remaining gap inline.

[amy-why-3459]

_tf32_mode() is now the only boolean-style stage key in this class that bypasses the strict parser, and it feeds this default. token2wav_allow_tf32: ~ reaches return "tf32" if value else "off" and turns TF32 off (and with it the CUDA default for cfm_fused_body), which is the opposite of the null-means-default rule read_bool_flag just introduced. It also shadows the code2wav_allow_tf32 alias, since extra.get returns the explicit None. A typo like "ture" likewise maps silently to "off" instead of raising. Worth treating None as unset and rejecting unknown strings here too (keeping the tf32x3 / 3xtf32 / flow aliases).

[y-null]

Fixed in 21171a767 -- _tf32_mode now follows the same rule as the other stage booleans.

• Explicit null means unset: token2wav_allow_tf32: ~ falls through to code2wav_allow_tf32, and then to the "tf32" default, so it neither disables TF32 nor shadows the alias.
• Accepted values are unchanged (tf32/tf32x3/3xtf32/flow/true/yes/1 and off/false/no/0, plus bools and 0/1).
• Everything else now raises at startup, in the stage config and in MINICPMO_CODE2WAV_TF32 alike, instead of silently mapping to "off" -- so "ture" fails the same way a typo in any other boolean would, and the cfm_fused_body CUDA default can no longer be flipped by a typo.

test_tf32_mode_null_means_unset_and_typos_raise covers the null fall-through (with and without the alias), the accepted tokens and rejection in both paths; the existing test_tf32_mode_reads_shipped_yaml_key and the cfm_fused_body platform matrix still pass (48 passed together with the flag tests).

[amy-why-3459]

Re-checked the delta since 505d911 (21171a7: strict _tf32_mode). This resolves all three points from my last pass: an explicit null now falls through to code2wav_allow_tf32 and then the default, and unknown tokens raise. The new test covers each case. One follow-up on where the strict parse now runs.

[amy-why-3459]

Now that this raises, where it's called matters. On CUDA a bad value fails at startup through the fused_body default in __init__ and through _build_backend. On Ascend, though, both of those skip it: current_omni_platform.is_cuda() and _tf32_mode(extra) != "off" short-circuits, and _build_backend only calls it under is_cuda(). The first unconditional call is _stage_matmul_policy, which wraps forward() and run_idle_prefetch(). So on NPU a typo in token2wav_allow_tf32 or MINICPMO_CODE2WAV_TF32 passes startup and then raises inside every forward and idle prefetch.

Could you resolve the mode once in __init__ on every platform (for example self._tf32_mode = _tf32_mode(extra)) and have _stage_matmul_policy take the resolved value? That fails fast everywhere, and it also stops re-reading the env var and extra config on every forward. If you like, the policy can also skip touching torch.backends.cuda.matmul on non-CUDA platforms.

[y-null]

Resolved in e41618d94 exactly as suggested.

• __init__ resolves it once on every platform: self._tf32_mode = _tf32_mode(extra), right after the extra config is read, so an invalid value now fails at construction on Ascend too instead of inside forward().
• _stage_matmul_policy takes the resolved string instead of the extra config, so it no longer re-reads the stage config or MINICPMO_CODE2WAV_TF32 per forward/idle prefetch, and it is a no-op off CUDA (returns early without touching torch.backends.cuda.matmul).
• Both remaining call sites reuse the resolved value: the cfm_fused_body default and _build_backend.

Tests: test_invalid_tf32_value_fails_at_startup_even_off_cuda (both keys, is_cuda forced False), the env-var variant, and test_tf32_mode_is_resolved_once_and_reused (mutating the stage config afterwards no longer changes the mode). test_prefetch_phases_run_under_the_stage_matmul_policy now configures the mode before construction and pins is_cuda, so it still covers the apply/restore path; 45 passed across the affected files, ruff clean.

[y-null]

Done in e41618d (with cfe0dd1 touching the test only):

• _tf32_mode(extra) is now called exactly once, unconditionally in __init__ (self._tf32_mode = _tf32_mode(extra)), on every platform. A typo in token2wav_allow_tf32 or MINICPMO_CODE2WAV_TF32 fails at construction time on Ascend too, instead of inside the first forward/idle prefetch.
• _stage_matmul_policy takes the resolved value (with _stage_matmul_policy(self._tf32_mode) in both forward() and run_idle_prefetch()), so neither path re-reads the stage extra or the env var per call.
• The policy now only touches torch.backends.cuda.matmul under current_omni_platform.is_cuda(); on other platforms the context manager is a plain yield (your optional suggestion).
• Startup behavior is platform-independent now, since the resolve in __init__ has no platform guard. test_tf32_mode_null_means_unset_and_typos_raise pins null-means-unset for both the YAML key and the env var plus rejection of unknown tokens, and test_tf32_mode_reads_shipped_yaml_key keeps the shipped-YAML mapping.

Local run at e41618d on a CUDA machine: tests/model_executor/models/minicpmo_4_5/test_code2wav_device_spk_flag.py, test_code2wav_batching.py, test_code2wav_ref_prefetch.py together: 223 passed in 37s. No NPU hardware available in this environment, so the Ascend-side startup claim is structural (unconditional __init__ call), not hardware-executed.

[y-null]

Supplementing with a construction-level check executed on the same CUDA machine (current_omni_platform.is_cuda() == True), since the old CUDA fail-fast path lived at exactly this level:

• stage extra token2wav_allow_tf32: "maybe" -> ValueError at construction: token2wav_allow_tf32/MINICPMO_CODE2WAV_TF32 must be 'tf32'/'off' (or a boolean), got 'maybe'
• env MINICPMO_CODE2WAV_TF32=maybe -> the same ValueError at construction
• explicit null -> constructs, falls through to the default (self._tf32_mode == "tf32")
• valid "off" / "tf32" -> constructs, self._tf32_mode mirrors the value

So on CUDA the fail-fast and null-means-default behavior is executed, not only structural; the parser-level unit tests in test_code2wav_batching.py pin the same matrix. The Ascend side remains a structural guarantee for the reason stated above (no NPU hardware in this environment).

[amy-why-3459]

Checked the delta since 21171a76 (cfe0dd13 test token swap + e41618d9). Resolving self._tf32_mode at the top of __init__ fixes my earlier point: an invalid token2wav_allow_tf32 / MINICPMO_CODE2WAV_TF32 now fails at construction on NPU as well, and forward() / run_idle_prefetch() no longer re-read the stage config and env var per call. _build_backend (including the lazy dummy-load path in forward) and the cfm_fused_body default both run after the attribute is set, and the only _stage_matmul_policy callers are the two updated ones, so the signature change is safe. Making the policy a no-op off CUDA is consistent with _build_backend already forcing "off" there. Nothing further from me on this delta.

[vllm-omni-review-bot]

Omni ReviewBot: superseded

The CI failure noted on e41618d94545 refers to an earlier head; the pull request now points at 56a0e5b631f4.

[amy-why-3459]

Checked the delta since e41618d9 (dfeb9900, test-only; the main merge 56a0e5b6 skipped). Both fixes match the current code: forward() now only reads self._tf32_mode and _forward_impl, so the SimpleNamespace(_tf32_mode="tf32", ...) stub is sufficient, and pinning is_cuda via monkeypatch.setattr(current_omni_platform, ...) does reach _stage_matmul_policy because vllm_omni.platforms caches the instance in _current_omni_platform, so the function-local re-import returns the same patched object. Setting _codec_fused_prelude = False on the __new__-built talker keeps the k-step tests on the eager chain. No further comments on this delta.