You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(llm): keep the SSE keep-alive fallback for thinking-disabled MiniMax M3 streams - #18015
A MiniMax M3 stream can send nothing for a long time and be cut off by an idle timeout. The reported setup is the public M3.1 Flash template, a streaming request with reasoning_effort: "none" and force_nonempty_content: true, no DYN_HTTP_SSE_KEEP_ALIVE_INTERVAL_MS, and about 90 seconds of reasoning. No frame goes on the wire during that time.
Since #17120, the M3 reasoning parser still runs for a thinking-disabled request when the rendered prompt leaves reasoning open. This template ignores thinking_mode, so it does. With force_nonempty_content, the stream then holds back every data frame while the model reasons. The HTTP handlers choose the 15-second fallback keep-alive before the prompt is rendered. They still use the request-only check, which says this request holds nothing back, so they send no keep-alive. The defect is in Dynamo.
Reproduction
Without the fix
With the fix
Note
Exact failure on the reported setup
Not run
—
Needs the MiniMax M3.1 Flash weights served with its public template and a long reasoning stream. Those weights do not fit the single A100 80 GB the coding run had.
Exact failure on a similar setup
Not run
—
Needs a live deployment that runs the M3 reasoning parser. The mock worker cannot set a reasoning parser, and a small vLLM model does not render an M3 prompt.
Exact error in a test
❌ Exact error
✅ Passed
The new predicate test and the changed Responses wrapper test ran on CPU. Before the fix, both returned false for an M3 force_nonempty_content request with thinking disabled, so the stream got no fallback keep-alive. With the fix, both return true.
Proposed fix
The frontend now treats every M3 stream with force_nonempty_content as one that may hold back all output, even when the request disables thinking. This applies to chat completions, Responses, and Anthropic Messages. It matches the fix on the release branch (#17197). The result is conservative: when an M3 template does close the reasoning, the stream still gets comment frames. A configured DYN_HTTP_SSE_KEEP_ALIVE_INTERVAL_MS still wins.
Open #17381 sends a keep-alive on every Anthropic stream. If it merges first, the Anthropic part of this fix is redundant but harmless.
Core change: about 3 lines (+2/−1). The logic is in 1 file, in reading order:
about 3 lines in preprocessor.rs: Makes the keep-alive check count a MiniMax M3 reasoning parser as possibly holding back output, whatever the request says about thinking.
The rest of the +71/−6 diff is a doc comment and unit tests.
Validation
This PR adds one unit test to lib/llm/src/preprocessor.rs and extends one unit test in lib/llm/src/http/service/openai.rs. Both run on CPU with cargo test -p dynamo-llm --lib, with no model and no GPU.
Behavior checked
Checked by
Without the fix
With the fix
A thinking-disabled M3 stream that may hold back all output gets the fallback keep-alive, through both the predicate and the OpenAI handler wrapper
New unit test minimax_m3_disabled_thinking_force_nonempty_stream_can_defer_all_output, and changed unit test responses_force_nonempty_request_requires_fallback_keep_alive, which had no M3 case before this PR; also the reproduction above
❌ Exact error
✅ Passed
An M3 stream with thinking unset or adaptive still gets the fallback
New unit test
Not run
✅ Passed
An M3 stream without force_nonempty_content gets no fallback
New unit test
Not run
✅ Passed
A non-M3 parser (kimi_k25) that disables thinking still gets no fallback
New unit test
Not run
✅ Passed
The before-fix run stopped at the new test's first assertion, so its later control assertions did not run without the fix.
Improved handling of streamed MiniMax M3 responses when non-empty output is required. Requests with reasoning disabled can now be treated as potentially deferring output, helping avoid premature assumptions about when response content will arrive.
Other reasoning parsers retain their existing behavior when reasoning is disabled.
…Max M3 streams
stream_can_defer_all_output used the request-only reasoning gate, so a
MiniMax M3 force_nonempty_content stream with thinking_mode=disabled (or
reasoning_effort=none) reported that it defers nothing. Since ai-dynamo#17120 the
stream builder keeps the M3 parser on when the rendered prompt leaves
reasoning open, so such a stream can withhold every data frame with no
keep-alive. Treat the M3 reasoning parser as possibly deferring, as the
release-branch fix in ai-dynamo#17197 does.
Refs DYN-5298
Signed-off-by: svc-glamr@nvidia.com <svc-glamr@nvidia.com>
👋 Hi glamr-agent! Thank you for contributing to ai-dynamo/dynamo.
Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.
Checks the changed Rust crates with cargo check and Clippy.
Passed (exit 0)
Workspace cargo check --all-targets, Python bindings cargo check, and workspace Clippy with -D warnings at a13eea9b03
No live MiniMax M3 stream ran: the weights do not fit the single GPU available. The unit tests cover the keep-alive decision for the reported request, not a real 90-second reasoning stream.
Detailed evidence — validation complete
Left out of the table because CI repeats them: none.
Evidence: 1/1 validated.
Advisory AI assessment: sound.
Validation result: pass. The new M3 keep-alive assertions fail on main's predicate and pass with the fix. Workspace check, workspace Clippy, sidecar tests, and dynamo-llm lib tests pass, except 19 GPT-OSS tests that fail identically on unchanged main because the Harmony vocab file cannot be downloaded in the validation environment. Evidence audit: complete, 1/1 validated; the command report comes from recorded runs.
Commands and results [1/1 validated]
Generated from the commands recorded during this run.
Check 1
Checks the changed Rust crates with cargo check and Clippy.
Result: Passed (exit 0)
Command:
Not shown because the exact command contained private run data.
No actionable comments were generated in the recent review.
Walkthrough
The preprocessor now treats MiniMax M3 as able to defer all output when forced nonempty content is enabled, even if thinking is disabled. Tests cover both M3 parser aliases and confirm that other parsers retain their existing behavior.
Changes
MiniMax M3 deferred-output eligibility
Layer / File(s)
Summary
Update eligibility rule and test coverage lib/llm/src/preprocessor.rs, lib/llm/src/http/service/openai.rs
The preprocessor allows MiniMax M3 to defer output when force_nonempty_content is enabled, even if thinking is disabled. Tests cover both M3 aliases, multiple thinking modes, and unchanged behavior for other parsers.
The M3 keep-alive change is mergeable after normal checks. An explicit zero interval may still produce fallback comments on deferred streams, but this narrow behavior does not block the change.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title clearly and concisely identifies the MiniMax M3 SSE keep-alive fallback fix.
Description check
The description is detailed and on-topic. It explains the failure behavior, proposed fix, validation, limitations, and related issues. It uses different headings from the template and omits the pull r…
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Description check
Explanation
The description is detailed and on-topic. It explains the failure behavior, proposed fix, validation, limitations, and related issues. It uses different headings from the template and omits the pull request checklist, but the required technical content is largely present.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files. (1 skipped: 1 too large.)
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Reviewing files that changed from the base of the PR and between 182039d and a13eea9.
Files selected for processing: 2
lib/llm/src/http/service/openai.rs
lib/llm/src/preprocessor.rs
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
nursery: Merged the base fix in commit e869932 to address Devin’s finding. In test_live_retune_frontend_config.py, the frontend and worker parser tests now use device-aware-weighted, which the parser accepts. Separate cases check that the removed power-of-two mode is rejected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GitHub Actions
Inactive
github-actions
Bot
added
external-contribution
Failure behavior
A MiniMax M3 stream can send nothing for a long time and be cut off by an idle timeout. The reported setup is the public M3.1 Flash template, a streaming request with
reasoning_effort: "none"andforce_nonempty_content: true, noDYN_HTTP_SSE_KEEP_ALIVE_INTERVAL_MS, and about 90 seconds of reasoning. No frame goes on the wire during that time.Since #17120, the M3 reasoning parser still runs for a thinking-disabled request when the rendered prompt leaves reasoning open. This template ignores
thinking_mode, so it does. Withforce_nonempty_content, the stream then holds back every data frame while the model reasons. The HTTP handlers choose the 15-second fallback keep-alive before the prompt is rendered. They still use the request-only check, which says this request holds nothing back, so they send no keep-alive. The defect is in Dynamo.falsefor an M3force_nonempty_contentrequest with thinking disabled, so the stream got no fallback keep-alive. With the fix, both returntrue.Proposed fix
The frontend now treats every M3 stream with
force_nonempty_contentas one that may hold back all output, even when the request disables thinking. This applies to chat completions, Responses, and Anthropic Messages. It matches the fix on the release branch (#17197). The result is conservative: when an M3 template does close the reasoning, the stream still gets comment frames. A configuredDYN_HTTP_SSE_KEEP_ALIVE_INTERVAL_MSstill wins.force_nonempty_content: trueforce_nonempty_contentOpen #17381 sends a keep-alive on every Anthropic stream. If it merges first, the Anthropic part of this fix is redundant but harmless.
Validation
This PR adds one unit test to
lib/llm/src/preprocessor.rsand extends one unit test inlib/llm/src/http/service/openai.rs. Both run on CPU withcargo test -p dynamo-llm --lib, with no model and no GPU.minimax_m3_disabled_thinking_force_nonempty_stream_can_defer_all_output, and changed unit testresponses_force_nonempty_request_requires_fallback_keep_alive, which had no M3 case before this PR; also the reproduction aboveadaptivestill gets the fallbackforce_nonempty_contentgets no fallbackkimi_k25) that disables thinking still gets no fallbackThe before-fix run stopped at the new test's first assertion, so its later control assertions did not run without the fix.
Relates to #17120 and #17197.
Closes DYN-5298.
Summary by CodeRabbit