Repository navigation
[Bugfix][MiniCPM-o] Fix async-chunk snapshot replacement and prompt cleanup - #6406
Conversation
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
@R2-Y PTAL |
|
This PR appears to be related to model: minicpm. Model owners: @y-null @natureofnature, 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. |
|
Thanks — this is the right second split from #5102. Talker capacity Request changes on the Code2Wav replay path. The duplicate turn_end payload is only
so the empty snapshot is written onto The producer test only checks the marker exists; the runner test Should-fix:
Happy to re-review once the empty snapshot is actually consumed. |
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
|
Self-review and reviewer update for
Validation after the fix:
I also rechecked the scope: snapshot replacement remains gated by the explicit producer marker plus the MiniCPM Code2Wav model opt-in; unmarked generation/diffusion models retain merge semantics. @amy-why-3459 PTAL when convenient. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c04fc9fcad
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ec3817f3d
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
|
@codex review |
|
@tzhouam PTAL |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@Gaohan123 PTAL |
|
Thanks for the follow-up. Rechecked d5e206c. The previous blocking replay path is fixed: a duplicate native-duplex Approve with comments. Should-fix / follow-up:
Snapshot replace remaining opt-in (producer marker + Code2Wav flag) is |
Signed-off-by: natureofnature <wzliu@connect.hku.hk>
…leanup (vllm-project#6406) Signed-off-by: natureofnature <wzliu@connect.hku.hk> Co-authored-by: amy-why-3459 <wuhaiyan17@huawei.com>
…leanup (vllm-project#6406) Signed-off-by: natureofnature <wzliu@connect.hku.hk> Co-authored-by: amy-why-3459 <wuhaiyan17@huawei.com>
…leanup (vllm-project#6406) Signed-off-by: natureofnature <wzliu@connect.hku.hk> Co-authored-by: amy-why-3459 <wuhaiyan17@huawei.com> Signed-off-by: AndyZhou952 <jzhoubc@connect.ust.hk>
…leanup (vllm-project#6406) Signed-off-by: natureofnature <wzliu@connect.hku.hk> Co-authored-by: amy-why-3459 <wuhaiyan17@huawei.com>
…odec chunk `OmniGPUModelRunner._update_additional_information` skipped a newly scheduled request's `additional_information` whenever the request also carried a non-empty `model_intermediate_buffer` and the model opted into replace semantics. That guard exists so a fresh boundary marker is not clobbered by the stale chunk it supersedes (vllm-project#6406), but it also fires when the buffer is only the request's own setup state. MiniCPM-o 4.5 Code2Wav is the one model with `replace_runtime_additional_information = True`. Async-chunk pre-warm gives its Stage-2 requests a setup buffer holding `duplex` and `global_request_id`, and every codec chunk that admits such a request travels in `additional_information`. So the first chunk of every Talker generation reached the vocoder with all producer metadata stripped and `_parse_item` substituted defaults, which killed the Stage-2 engine two different ways: * A real codec window lost `cache_epoch` and `chunk_seq`, so the vocoder recorded epoch 0 / chunk 0 for what was really epoch 1 / chunk 0. The next chunk arrived correctly labelled and raised `new_epoch_requires_first_chunk` (vllm-project#7979). * The control-only segment boundary is one placeholder token plus `code_flat_numel == 0`. Losing that zero made the placeholder look like codec data, so a one-frame non-final window raised `chunk_below_lookahead_window` (vllm-project#7978). Decide by what the buffer contains: a buffer carrying an `OmniPayload` section is a producer snapshot and still wins, while a buffer holding only setup state no longer shadows the chunk. Two Code2Wav changes keep a stage-wide failure off the table when a chunk does go missing anyway. A stream-position gap now resets or skips the affected request instead of raising, since the transport can legitimately lose a payload and killing the engine drops every session on the replica; a backward position is still refused, because replaying it would advance the codec cache twice. A non-final window narrower than the encoder's pre-lookahead kernel is withheld and rides on the next window rather than being vocoded alone. Signed-off-by: chickeyton <ngton2014@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…odec chunk `OmniGPUModelRunner._update_additional_information` skipped a newly scheduled request's `additional_information` whenever the request also carried a non-empty `model_intermediate_buffer` and the model opted into replace semantics. That guard exists so a fresh boundary marker is not clobbered by the stale chunk it supersedes (vllm-project#6406), but it also fires when the buffer is only the request's own setup state. MiniCPM-o 4.5 Code2Wav is the one model with `replace_runtime_additional_information = True`. Async-chunk pre-warm gives its Stage-2 requests a setup buffer holding `duplex` and `global_request_id`, and every codec chunk that admits such a request travels in `additional_information`. So the first chunk of every Talker generation reached the vocoder with all producer metadata stripped and `_parse_item` substituted defaults, which killed the Stage-2 engine two different ways: * A real codec window lost `cache_epoch` and `chunk_seq`, so the vocoder recorded epoch 0 / chunk 0 for what was really epoch 1 / chunk 0. The next chunk arrived correctly labelled and raised `new_epoch_requires_first_chunk` (vllm-project#7979). * The control-only segment boundary is one placeholder token plus `code_flat_numel == 0`. Losing that zero made the placeholder look like codec data, so a one-frame non-final window raised `chunk_below_lookahead_window` (vllm-project#7978). Decide by what the buffer contains: a buffer carrying an `OmniPayload` section is a producer snapshot and still wins, while a buffer holding only setup state no longer shadows the chunk. Two Code2Wav changes keep a stage-wide failure off the table when a chunk does go missing anyway. A stream-position gap now resets or skips the affected request instead of raising, since the transport can legitimately lose a payload and killing the engine drops every session on the replica; a backward position is still refused, because replaying it would advance the codec cache twice. A non-final window narrower than the encoder's pre-lookahead kernel is withheld and rides on the next window rather than being vocoded alone. Signed-off-by: chickeyton <ngton2014@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…leanup (vllm-project#6406) Signed-off-by: natureofnature <wzliu@connect.hku.hk> Co-authored-by: amy-why-3459 <wuhaiyan17@huawei.com>
Purpose
This is the second correctness-only split from #5102, following the resumable cleanup fix in #6360. It fixes MiniCPM-o 4.5 async-chunk snapshot replacement and explicit prompt-replacement cleanup without including the OmniInteract benchmark client, Nightly workflow, production YAML changes, final-input lifecycle work, force-listen policy, or automatic context-capacity rollover.
What broke
Reproduction
The targeted tests exercise both failures directly:
Root cause
The Code2Wav path had only incremental merge semantics for runtime additional information. Separately, an explicit async prompt replacement updated the request payload without first going through the scheduler's cache-release and connector-reset path.
Fix
codes.refwhen 1-D audio is represented by prompt placeholders.OmniNPUModelRunneralready derives fromOmniGPUModelRunnerand explicitly delegates Omni state updates to it.Scope decision
This revision deliberately removes the automatic Talker context-capacity rollover that was previously in this PR. We do not yet have a targeted long single-Talker-turn E2E showing that automatic replacement is required, and replacing an accumulated prompt is a model-semantics decision rather than a safe transport-only fix. If the long-video E2E demonstrates a real context-limit failure, that policy can return in a separate PR with model-quality and continuity evidence. Explicit turn-boundary replacement and its cache/watermark cleanup remain in this PR.
Test Plan
vLLM Version: target pin
0.27.0; compatibility unit-test container reports vLLM-Omni0.25.0vLLM-Omni Commit:
d5e206cd6b4cf5744fff19ef6a5a77d07f1d3d9eTest Result
13 passed, 17 warnings in 1.79s2 passed, 17 warnings in 0.11s181 passed, 17 warnings in 2.92s24 passed, 17 warnings in 0.33stests/platforms/npu/for Ascend CIAll checks passed; all changed Python files are formattedcompileallandgit diff --check: passedThe snapshot replacement path requires both an explicit payload marker and the MiniCPM Code2Wav model opt-in. Explicit prompt replacement is producer-marked; there is no automatic context-limit policy in this PR.