fix: add missing is_finished param to llm2code2wav_async_chunk (fixes #1569)#1638
fix: add missing is_finished param to llm2code2wav_async_chunk (fixes #1569)#1638dubin555 wants to merge 3 commits intovllm-project:mainfrom
is_finished param to llm2code2wav_async_chunk (fixes #1569)#1638Conversation
…ixes vllm-project#1569) The call site in chunk_transfer_adapter.py:211 passes is_finished=is_finished to llm2code2wav_async_chunk(), but the function signature did not accept this parameter, causing a TypeError crash in MiMo-Audio async chunk mode. Add is_finished: bool = False to the function signature (matching thinker2talker_async_chunk in qwen3_omni.py) and replace all request.is_finished() calls with the is_finished parameter, which correctly uses the state snapshot captured at task creation time.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45b375e3b8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if "vllm.inputs" not in sys.modules: | ||
| stub = types.ModuleType("vllm.inputs") | ||
| stub.TextPrompt = type("TextPrompt", (), {}) | ||
| sys.modules["vllm"] = types.ModuleType("vllm") |
There was a problem hiding this comment.
Keep test from poisoning global import state
Importing this test module mutates sys.modules at module scope by assigning stub packages (including vllm and later vllm_omni) without restoring them, so a normal pytest run that imports this file early can cause unrelated tests to fail with errors like 'vllm_omni' is not a package when they import real modules afterward. This makes suite behavior order-dependent and can break full-repo test execution.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — the module-scope sys.modules mutation is indeed problematic for full-suite runs. I've moved the stub injection and module loading into setUpModule() with a sys.modules snapshot, and added tearDownModule() to restore the original state. This way the stubs are scoped to this test module only and won't affect other tests.
Move stub injection and module loading into setUpModule() with a sys.modules snapshot, and restore the original state in tearDownModule(). This prevents the test from poisoning the global import state for other tests in the suite.
Move test_fix_mimo_audio_is_finished.py from repo root into tests/ to follow project conventions. Update path computation accordingly.
|
Are the changes in this PR the same as in 1570? #1570 |
Duplicate of Already-Merged PR #1570This PR fixes the same bug as PR #1570, which was already merged on 2026-03-02.
Both PRs make the exact same change:
Recommendations
Credit to @dubin555 for writing comprehensive tests independently! The test coverage is good and would be a welcome addition as a standalone PR. |
Purpose
Fixes #1569
llm2code2wav_async_chunk()inmimo_audio.pyis missing theis_finishedkeyword argument that_send_single_request()inchunk_transfer_adapter.py:211passes to it. This causes aTypeErrorcrash when MiMo-Audio is used in async chunk mode:This PR:
is_finished: bool = Falseto thellm2code2wav_async_chunk()signature, matching the pattern used bythinker2talker_async_chunkinqwen3_omni.pyrequest.is_finished()calls inside the function with theis_finishedparameter. This is the correct behavior because the caller (save_async) snapshots the finished state at task creation time, so the explicit parameter should be used rather than querying the request object which may have changed state by the time the background thread processes it.Test Plan
Unit tests verifying the fix:
Tests cover:
is_finishedparameter (matchingthinker2talker_async_chunkpattern)is_finishedkwarg no longer raisesTypeErroris_finished=Truecorrectly sets the finished flag in the output payloadis_finished=Falsecorrectly buffers until chunk is fullis_finished=Truereturns sentinelTest Result
Before fix — all 5 tests fail with
TypeError: llm2code2wav_async_chunk() got an unexpected keyword argument 'is_finished':After fix — all 5 tests pass:
Essential Elements of an Effective PR Description Checklist