Repository navigation
fix(sglang): [cherry-pick 1.5.1] support min_tokens on tokenizer-free decode workers (#14276, #14378) - #14951
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c14512f325
ℹ️ 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".
| /// In the future, this method may kick off async cpu/tokio tasks and or async cuda tasks to | ||
| /// handle logits post-processing and/or other tasks. | ||
| pub fn step(&mut self, token_id: TokenIdType) -> Result<StepResult> { | ||
| let below_min_tokens = self.generated_tokens < self.min_tokens; |
There was a problem hiding this comment.
Preserve stop-prefix state on the min-th token
For a tokenizer-free request with min_tokens > 0, this pre-increment check bypasses all stop-sequence processing for the min-th generated token. If a hidden string stop begins in that token and completes in a later token, its prefix is released instead of jailed, so the decoder misses the stop and generation continues until another stop or max_tokens; SGLang cannot catch this because token-format requests do not forward string stops. Suppress completed stops before the floor, but still retain partial-match state from the min-th token.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed by execution, and I agree with P2. Measured, this pull request moves the leak boundary by exactly one token: the stop is missed when its first fragment falls on the min_tokens-th token.
Measured on this branch, with the pre-pick token order as the control
Probe added to lib/llm/tests/test_stop_behavior.rs, hidden stop sequence " delta epsilon" split across two tokens, token stream [HI, DELTA, EPSILON, ZETA]. macOS ARM64, cargo test -p dynamo-llm --no-default-features.
min_tokens |
pre-pick order (increment first) | this head |
|---|---|---|
| 0 | HiddenStopSequenceDetected |
HiddenStopSequenceDetected |
| 1 | HiddenStopSequenceDetected |
HiddenStopSequenceDetected |
| 2 | HiddenStopSequenceDetected |
text "hi delta epsilon zeta", trigger None |
| 3 | text "hi delta epsilon zeta", trigger None |
same |
The control is the only edit: self.generated_tokens += 1; moved back above the below_min_tokens read at backend.rs:824, everything else unchanged. Rows 0 and 1 are unchanged in both directions, so the probe is not an environment artifact.
So the class of leak already exists on release/1.5.0 (row 3 leaks on both). This pull request extends it by one token, because the pre-increment read skips one more token than the old post-increment read. The exposure is narrow: the stop prefix must begin on exactly that token.
Not blocking, and identical to the code approved on main. Retaining partial-match state below the floor, as you suggest, is the right shape.
There was a problem hiding this comment.
Re-measured on x86_64 Linux this round, and the result holds: this pick moves the leak boundary by exactly one token at lib/llm/src/backend.rs:824. Three of the four rows did not move, so the probe is not an environment artifact.
Measured on this head, with the merge-base token order as the control
Probe added as a separate test file, so no existing file was edited for run A. Hidden stop sequence " delta epsilon" split across two tokens, stream [HI, DELTA, EPSILON, ZETA]. Built with cargo test -p dynamo-llm --no-default-features on x86_64 Linux, 32 cores.
The unmutated tree is green first: test_stop_behavior gives 15 passed; 0 failed.
min_tokens |
control, merge-base order | this head |
|---|---|---|
| 0 | HiddenStopSequenceDetected(" delta epsilon") |
same |
| 1 | HiddenStopSequenceDetected(" delta epsilon") |
same |
| 2 | HiddenStopSequenceDetected(" delta epsilon") |
text "hi delta epsilon zeta", trigger None |
| 3 | text "hi delta epsilon zeta", trigger None |
same |
The control is one edit and nothing else: the increment moved back above the read, which is the merge-base order at 55589a5e72.
// increment the generated tokens
self.generated_tokens += 1;
let below_min_tokens = self.generated_tokens < self.min_tokens;Baseline sha of backend.rs 95e8e4900acd438a6b9d92bfba3d2407917831d7, mutated c15e46a3f97aac774f77976c558e1b0ccc634314, restored 95e8e4900acd438a6b9d92bfba3d2407917831d7.
Row 3 leaks on both sides, so the class of leak already exists at the merge base. This pick extends it by one token, because the read before the increment skips one more token than the read after it. The stop prefix has to begin on exactly that token.
Still not blocking, and byte-identical to what merged on main: main after #14276 has the same two lines at backend.rs:824 and :826. The release branch gained +90/-0 in backend.rs since the merge base, and all of it is inside mod tests, so it does not touch this site.
| if meta_info.get("finish_reason"): | ||
| request_ids.discard(sglang_request_id) |
There was a problem hiding this comment.
Retain shared request IDs until every choice finishes
For n > 1, SGLang chunks can share one meta_info["id"] while carrying distinct choice indexes, as the existing fixtures in test_sglang_decode_handler.py:1218-1230 demonstrate. Discarding that ID when any one choice reports a finish leaves request_ids empty while other choices are still running; if the client cancels or closes the stream in that interval, both the monitor and context-manager cleanup abort nothing, allowing the remaining generation to continue orphaned. Track finished choice indexes and remove the shared ID only after all choices complete.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The premise does not hold on the engine this branch pins, so I do not think there is anything to fix here.
SGLang 0.5.18 gives each choice its own id under n > 1
release/1.5.0 pins lmsysorg/sglang:v0.5.18-cu130-runtime at container/context.yaml:123. In that image, python/sglang/srt/managers/tokenizer_manager.py takes the parallel_sample_num > 1 branch and calls tmp_obj.regenerate_rid() once per choice before sending each one:
for i in range(batch_size):
for _ in range(obj.parallel_sample_num):
...
tokenized_obj.rid = tmp_obj.regenerate_rid()
self._send_one_request(tokenized_obj)
Each choice therefore carries a distinct meta_info["id"], and request_ids.discard(sglang_request_id) removes only the choice that reported the finish. The shared "id": "request-1" in the fixtures you cite is a test artifact, not engine behavior.
What I did not verify: a live multi-choice generation on a GPU. This is read from the pinned image source plus the parameter path, not from a served request.
There was a problem hiding this comment.
Re-checked this round, and the premise still does not hold. The engine pin has not moved, so each choice still gets its own identifier and there is nothing to fix here.
The pin, read at three refs
container/context.yaml holds runtime_image: lmsysorg/sglang with runtime_image_tag: v0.5.18-cu130-runtime at all three of:
| ref | line |
|---|---|
head 859c8f2732 |
container/context.yaml:123 |
merge base 55589a5e72 |
container/context.yaml:123 |
release/1.5.0 tip 32b8b2f8c6 |
container/context.yaml:126 |
The release branch changed container/context.yaml since the merge base, but the only change is three added lines under vllm.xpu, so the sglang tag shifts down by three lines and keeps its value. A tag change would revive this, so it is worth re-reading if the pin ever moves.
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: Still present at the reviewed head:
Decoder::stepreturns before stop-sequence processing whilegenerated_tokens < min_tokens, so the min-th generated token does not record a hidden stop-sequence prefix and can leak or miss a later split stop. - Original discussion: Still present at the reviewed head:
_process_token_streamdiscards a shared SGLang request id as soon as any choice reportsfinish_reason, so for n>1 streams with the samemeta_info.ida later cancellation can leave unfinished choices un-aborted. - Original discussion: Still present:
Decoder::stepreturns early whilebelow_min_tokensis true, so the min-th token bypasses hidden stop-sequence processing and cannot retain a prefix that completes in a later token. - Original discussion: Still present: a shared SGLang request ID is removed when any parallel choice finishes, so cancellation can leave remaining choices running without an abort target.
- Original discussion: Still present: each chunk with
finish_reasondiscards its request ID without tracking choice completion. Shared SGLang IDs can therefore be removed while another choice remains active, leaving cancellation unable to abort it. - Original discussion: Still present: the min-th generated token bypasses stop-sequence processing, so a hidden string-stop prefix beginning there is released instead of retained for a later matching token.
- Original discussion: Verified:
Decoder::stepbypasses all hidden-stop sequence processing while belowmin_tokens, so a prefix emitted in the minimum-token window is released instead of retained. A later token cannot complete that hidden stop, and tokenizer-free workers continue until another stop or max_tokens. - Original discussion: Verified:
_process_token_streamdiscards a request ID as soon as any chunk with that shared SGLang ID finishes. Forn > 1, remaining choices can share the ID; cancellation or stream closure then aborts no active generation. - Original discussion: Verified: parallel choices can share one SGLang request ID, but line 802 removes that ID when the first choice finishes. A cancellation before another unfinished choice emits its next chunk then aborts no request, leaving generation orphaned.
- Original discussion: Verified: the pre-increment minimum-token guard returns before updating hidden stop-sequence prefix state. A hidden string stop beginning on the min-th token can leak its prefix and fail to stop when completed later.
| if let Some(offset) = galil_seiferas::gs_find(self.jail.as_bytes(), seq.as_bytes()) | ||
| { | ||
| // return only new bytes after pre_append .. offset (excluding stop sequence) | ||
| // return only new bytes after release_start .. offset (excluding stop sequence) |
There was a problem hiding this comment.
This only paraphrases the following slice expression; the surrounding comments already explain why release_start exists and the branch below covers the partial-match case.
🤖 AI Fix
Remove this redundant implementation-narration comment.
| { | ||
| // For visible stop sequences, include the stop string in the output | ||
| // Return all text from pre_append up to and including the stop sequence | ||
| // Return all text from release_start up to and including the stop sequence |
There was a problem hiding this comment.
The preceding comment already records the visible-stop behavior, and stop_end plus the slice make this line a restatement of the implementation.
🤖 AI Fix
Remove this redundant implementation-narration comment.
| )); | ||
| } | ||
|
|
||
| /// A hidden stop sequence split across several decode fragments must not leak any |
There was a problem hiding this comment.
This restates the test name hidden_stop_sequence_split_across_tokens_is_not_leaked without adding a separate invariant or failure condition.
🤖 AI Fix
Remove the redundant test doc comment.
| ); | ||
| } | ||
|
|
||
| /// A withheld candidate prefix that turns out not to be part of the stop sequence must be |
There was a problem hiding this comment.
This repeats the test name withheld_prefix_is_released_once_it_cannot_complete; the assertions supply the concrete behavior.
🤖 AI Fix
Remove the redundant test doc comment.
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Cherry-pick review
Target branch release/1.5.0. Diffed against the two source squash merges on main: 7cc262f4eeb065fe4413c09079a53f375cefe120 (#14378) and e5ef9d778b9b34a8ffaa7916162b897ccd55495a (#14276). One P1: the pick is two commits short of #14276.
Fidelity, file by file, against the two source merge commits
#14378 pick 74a5cef4 is byte-identical to the source merge across all 14 files (added and removed lines compared after stripping index, @@ and context lines). Zero differences.
#14276 pick c14512f3:
| File | Result |
|---|---|
lib/llm/src/backend.rs |
End state identical to main, except 3 unrelated spots that already differed between the branches (prompt_token_ids deref at :132, detokenize_start shape at :829). 15 diff lines total. |
lib/llm/tests/test_stop_behavior.rs |
End state byte-identical to main. |
components/src/dynamo/sglang/tests/test_sglang_decode_handler.py |
Added and removed lines identical to the source. |
components/.../decode_handler.py _build_sampling_params |
The 56-line region is byte-identical to main. |
container/deps/requirements.test.txt |
Present here, absent from the #14276 squash. Explained, not extra: the branch commit f0ac9f34a5 added networkx, and main already had it from #14771, so the squash carried no delta. release/1.5.0 does not have #14771, and benchmarks/prefix_data_generator/{graph_utils,synthesizer}.py import networkx on this branch. |
components/.../decode_handler.py _process_text_stream and handler_base.py _abort_requests |
Two source commits missing. See the inline P1. |
Both sources were squash merges, so each is one commit, and both are present. The gap is that the pick was cut at 7ca35cee32 (18:47 UTC) and #14276 merged at 00:12 UTC the next day.
The defect reproduces on release/1.5.0 against the engine this branch pins, and is gone at head
container/context.yaml:123 pins lmsysorg/sglang:v0.5.18-cu130-runtime on the sglang cuda13.0 lane. main pins v0.5.19. The xpu lane pins sglang_ref: v0.5.11 on both branches.
The premise holds on v0.5.18. raise_if_tokenizer_required() in python/sglang/srt/sampling/sampling_params.py rejects a positive min_new_tokens when skip_tokenizer_init=True in v0.5.18 and v0.5.19. v0.5.11 (the xpu lane) has no such check, so that lane loses nothing.
Measured in …dynamo:f495b8f9…-sglang-runtime-test (SGLang 0.5.18), base tree and head tree in the same container, params fed to the real SamplingParams.normalize(tokenizer=None):
| Case | Base params | Base engine | Head params | Head engine |
|---|---|---|---|---|
min_tokens=0 (control) |
min_new_tokens=0 |
ACCEPTED | same | ACCEPTED |
min_tokens=32 > max_tokens=4 |
min_new_tokens=32 |
ValueError: min_new_tokens=32 is unavailable when skip_tokenizer_init=True |
{ignore_eos, max_new_tokens=4} |
ACCEPTED |
min_tokens=8 with a stop string |
min_new_tokens=8 |
ValueError: min_new_tokens=8 is unavailable when skip_tokenizer_init=True |
{ignore_eos, max_new_tokens=16} |
ACCEPTED |
The min_tokens=0 row is the control: identical on both trees.
Tests run on this branch, with the CI marker expression measured and a mutation control
Lane: pr.yaml:853, cpu_only_test_markers: pre_merge and sglang and gpu_0, sglang cuda13.0 flavor. Run, not read:
pytest components/src/dynamo/sglang/tests/test_sglang_decode_handler.py \
--collect-only -m "pre_merge and sglang and gpu_0"
-> 111 tests collected, 0 deselected
All four new cases are in that selection: test_suppressed_stop_token_ids_includes_plain_and_hidden_stop_token_ids, test_remove_suppressed_stop_tokens_removes_matched_tail_sequence, test_remove_suppressed_stop_tokens_keeps_visible_or_non_tail_matches, test_build_sampling_params_omits_string_stop_for_token_requests.
Mutation control, same image, four edits to decode_handler.py:
| Mutation | Result |
|---|---|
| none | 111 passed |
always send min_new_tokens |
1 failed (…holds_tokenizer_free_engine_open_for_min_tokens) |
if hold_engine_open: to if False: |
1 failed (same) |
drop stop_token_ids_hidden from _suppressed_stop_token_ids |
1 failed (…includes_plain_and_hidden_stop_token_ids) |
drop the tail-position test in _remove_suppressed_stop_tokens |
2 failed |
Rust, macOS ARM64, cargo test -p dynamo-llm --no-default-features: 13 of 13 backend::tests pass, including the new test_sglang_local_stop_discards_all_suffix_tokens.
Nothing left behind on main after #14276 merged
No commit on origin/main after 2026-09-17T00:12:18Z touches decode_handler.py, handler_base.py or lib/llm/src/backend.rs. No other pull request references #14276.
Where verification stopped: no GPU model was served. The reproduction drives the real SGLang 0.5.18 parameter validation, not a live engine, so the end-to-end streaming behavior on a GPU is unverified.
| pending_top_logprobs_per_choice: dict[int, list[Any]] = {} | ||
| completion_tokens_per_choice: dict[int, int] = {} | ||
| async with self._cancellation_monitor(request_id_future, context): | ||
| async with self._cancellation_monitor(request_id_future, context, request_ids): |
There was a problem hiding this comment.
[P1] The pick stops two commits short of #14276. components/src/dynamo/sglang/request_handlers/llm/decode_handler.py:792 got the per-choice abort set, but _process_text_stream at line 990 did not. A cancelled n>1 text request still aborts one choice. Please also pick 21638df03d and c66ce8a852.
The two commits that landed on #14276 after this pick was cut
The pick commit c14512f3 was authored 2026-09-16T19:10Z. #14276 merged 2026-09-17T00:12Z. Three commits landed in between:
| Commit | Time | Carried here |
|---|---|---|
7ca35cee32 test(sglang): provide stop state in shutdown context stub |
18:47Z | yes, the pick is cut here |
c66ce8a852 fix(sglang): retain context logging after request aborts |
20:08Z | no |
21638df03d fix(sglang): abort every unfinished text-stream choice |
20:23Z | no |
21638df03d is the functional one. It gives _process_text_stream the same treatment _process_token_stream already has here:
- async with self._cancellation_monitor(request_id_future, context):
+ request_ids: set[str] = set()
+ async with self._cancellation_monitor(request_id_future, context, request_ids):
Without it, _cancellation_monitor falls back to {sglang_request_id}, the id of the first chunk only. SGLang gives each choice its own id under n>1: tokenizer_manager.py calls tmp_obj.regenerate_rid() once per parallel_sample_num in the SGLang 0.5.18 image. So the other choices keep generating to max_new_tokens after the client goes away.
c66ce8a852 is logging only. It adds context to _abort_requests and one logging.info line. Left as it is, handler_base.py and decode_handler.py differ from main in signature, which makes the next pick on these files conflict.
Both are still absent at head 859c8f27. Checked with gh api repos/ai-dynamo/dynamo/commits/<sha> against the head tree.
There was a problem hiding this comment.
[P1] still open at head 859c8f27, and I was wrong to call c66ce8a852 "logging only". The pick deletes logging.info(f"Aborted Request ID: {context.id()}") from components/src/dynamo/sglang/request_handlers/handler_base.py:1269 and never puts it back, so the string is absent from that file. Please pick both commits.
The deleted log line traced across refs, the runtime probe, and the test that polls for it
I re-ran this against the commit that merged on main (e5ef9d778b, a single-parent squash), not the branch head.
Count of Aborted Request ID under components/:
| ref | hits | sglang handler_base.py |
|---|---|---|
merge base 55589a5e72 |
4 | :1269 logging.info(f"Aborted Request ID: {context.id()}") |
after commit 1 74a5cef4da |
4 | :1269, same line |
after commit 2 c14512f325 |
3 | absent |
head 859c8f2732 |
3 | absent |
release/1.5.0 tip 32b8b2f8c6 |
4 | :1202, present |
main before #14276 232f881e07 |
4 | :1235, present |
main after #14276 e5ef9d778b |
4 | :1179 logging.info("Aborted Request ID: %s", context.id()) |
#14276 removed the old line and re-added it inside _abort_requests in c66ce8a852. The pick took the removal and not the re-add. The hunk in c14512f325:
- self.engine.tokenizer_manager.abort_request(
- rid=sglang_request_id, abort_all=False
- )
- logging.info(f"Aborted Request ID: {context.id()}")
+ self._abort_requests(
+ request_ids if request_ids is not None else {sglang_request_id}
Runtime probe in the sglang runtime-test image, PR tree at /tmp/work, module path printed to prove which tree loaded:
### MODULE PATH PROOF: /tmp/work/components/src/dynamo/sglang/request_handlers/handler_base.py
signature args : (self, request_ids: set[str]) -> None
emits 'Aborted Request ID' : False
'Aborted Request ID' anywhere in handler_base.py : False
tests/fault_tolerance/cancellation/test_sglang.py:290 and :409 poll for f"Aborted Request ID: {request_id}". Both are marked gpu_2 and pre_merge, and the lane sglang-runtime / 2-GPU Test cuda13.0, amd64 selects them with pre_merge and sglang and gpu_2. I collected that expression against this tree and got 5 tests, 2 of them from that file. Both failed on this head:
FAILED tests/fault_tolerance/cancellation/test_sglang.py::test_request_cancellation_sglang_decode_cancel[nats] - AssertionError: Failed to find 'Aborted Request ID: bbb351ad-1046-4b28-ab01-062333fa31f6' pattern after 100 iterations (500ms)
FAILED tests/fault_tolerance/cancellation/test_sglang.py::test_request_cancellation_sglang_decode_cancel[tcp] - AssertionError: Failed to find 'Aborted Request ID: 9d87c48e-841d-45aa-af1c-f4e21e908c4c' pattern after 100 iterations (500ms)
21638df03d is still missing too. _process_text_stream at decode_handler.py:990 still calls self._cancellation_monitor(request_id_future, context) with two arguments, while _process_token_stream at :792 passes request_ids.
Per-file comparison of the pick against the main squash, added and removed lines checksummed on each side:
| file | added main |
added pick | removed main |
removed pick |
|---|---|---|---|---|
handler_base.py |
28 (12323242) |
27 (a223981d) |
19 (51548e4e) |
19 (51548e4e) |
decode_handler.py |
38 (b2312646) |
28 (559bc111) |
12 (d4e1fdb3) |
7 (55e00c9d) |
test_sglang_decode_handler.py |
57 (2df75e94) |
57 (2df75e94) |
11 (5a2608ff) |
11 (5a2608ff) |
test_stop_behavior.rs |
7 (f9624810) |
7 (f9624810) |
1 (7e098327) |
1 (7e098327) |
The two test files are byte-identical to main. The two source files are not. The empty-string control for this checksum is da39a3ee, and no cell above carries it.
No unit test pins either gap. Mutating _process_text_stream to apply 21638df03d left the suite at 111 passed, and the control mutation that strips request_ids from _process_token_stream also left it at 111 passed. Both mutations landed: baseline sha 1ee20a96fb26, mutated d504aafd4da4 and 416258669c93, restored 1ee20a96fb26 both times. The liveness control proves the suite can fail: disabling the hold_engine_open gate gave 1 failed, 110 passed on test_build_sampling_params_holds_tokenizer_free_engine_open_for_min_tokens.
Not verified: a live multi-choice cancellation on two GPUs. This box has one.
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Follow-up round: not approved, one P1 remains
Head is unmoved at 859c8f2732, and the P1 is unchanged: the pick stops two commits short of #14276, and one of the two gaps now shows up as a deleted log line that this branch's own cancellation test polls for. Details are on the existing threads.
Push shape, established by ancestry rather than by the label
The reviewed commit 859c8f2732 is still the head. The issue timeline records no head_ref_force_pushed event. The branch carries one merge commit, 859c8f2732, "Merge branch 'release/1.5.0' into dagil-nvidia/cp-14276-release-1.5.0", with parents c14512f325 and 55589a5e72. Committer dates did not collapse to one instant: 74a5cef4da has author date 2026-09-11 and committer date 2026-09-16, which is the normal shape of a cherry-pick.
Branch point is ffd7c1a90e. Merge base with release/1.5.0 is 55589a5e72. The branch is three commits: the #14378 backport, the #14276 pick, and the merge.
The release branch has moved, and it does not already carry this change
release/1.5.0 is now at 32b8b2f8c6, five commits past the merge base 55589a5e72.
The release branch has not gained #14276 by another route. _abort_requests does not exist anywhere under components/src/dynamo/sglang/ at the release tip, and no commit on that branch mentions #14276. This pull request is not redundant.
Interaction check. Six files in this diff also changed on release/1.5.0 since the merge base, all from one commit, 4166326a56:
| file | release change |
|---|---|
components/src/dynamo/sglang/request_handlers/handler_base.py |
+59 / -126 |
components/src/dynamo/sglang/request_handlers/llm/decode_handler.py |
+23 / -171 |
components/src/dynamo/sglang/tests/test_sglang_decode_handler.py |
+72 / -517 |
lib/llm/src/backend.rs |
+90 / -0 |
lib/llm/src/protocols/openai/chat_completions/delta.rs |
+40 / -90 |
lib/llm/src/protocols/openai/completions/delta.rs |
+42 / -92 |
I merged the release tip into the head myself. The backend.rs change is entirely inside mod tests, so it does not touch the site of the P2, and the P2 numbers below were measured on the head as it stands. _process_text_stream and _process_token_stream both survive at the release tip, so the P1 survives the merge as well.
Test selection, run rather than inferred
Collected inside the sglang runtime-test image against this tree, with the module path printed to prove the source tree loaded and not an installed package.
| lane | marker expression | collected | from the files at issue |
|---|---|---|---|
sglang-runtime / Test cuda13.0, amd64 |
pre_merge and sglang and gpu_0 |
831 | 111 from test_sglang_decode_handler.py |
sglang-runtime / Test cuda13.0, amd64 |
pre_merge and sglang and gpu_1 |
41 | 0 |
sglang-runtime / 2-GPU Test cuda13.0, amd64 |
pre_merge and sglang and gpu_2 |
5 | 2 from cancellation/test_sglang.py |
The Rust side runs under rust-tests (.). I ran cargo test -p dynamo-llm --no-default-features --test test_stop_behavior on the head and got 15 passed; 0 failed.
The collection run also reported 72 collection errors. Those come from my own overlay of the whole source tree onto the image and are not caused by this diff. None of them is in the files above, which collected cleanly.
What I retracted, and what stays open
Retracted: nothing this round. The thread at decode_handler.py:802 stays retracted from last round, and I re-confirmed the reason, which is that the engine pin did not move.
Corrected: I called c66ce8a852 "logging only" last round. That was too weak. It deletes a line that tests/fault_tolerance/cancellation/test_sglang.py asserts on, and the assertion fails on this head.
Open: one P1, the two missing commits. Plus the P2 at backend.rs:824, which I re-measured and which is byte-identical to what merged on main, so it belongs to the original change and not to this port.
Restore the context-ID abort log required by cancellation CI and track every unfinished text-stream choice, including choices first seen after cancellation. Add regressions covering token/text streams, early closure, completed choices, and late responses. Backports follow-ups c66ce8a and 21638df from #14276, which landed after #14951 was originally cherry-picked. Co-authored-by: GLAMR <svc-glamr@nvidia.com> Signed-off-by: jain-ria <riajain@NVIDIA.com>
…rry-pick #14276 to release/1.5.0) Cherry-pick of #14276 for NVBug 6714438 / DYN-4263, the last open P0 for 1.5.0. Applied on top of #14378, which it depends on. Main-side PR is approved (Dillon Cullinan, Ishan Dhanani) with CI re-triggered at 12:06 PT. Signed-off-by: Dan Gil <dagil@nvidia.com> Signed-off-by: jain-ria <riajain@NVIDIA.com>
Restore the context-ID abort log required by cancellation CI and track every unfinished text-stream choice, including choices first seen after cancellation. Add regressions covering token/text streams, early closure, completed choices, and late responses. Backports follow-ups c66ce8a and 21638df from #14276, which landed after #14951 was originally cherry-picked. Co-authored-by: GLAMR <svc-glamr@nvidia.com> Signed-off-by: jain-ria <riajain@NVIDIA.com>
Remove the obsolete checkpoint source link and regenerate the operator references, adopt the split ModelExpress refit examples, and link the GB200 recipe to the release branch where it remains available. Adapted from #14353 and #14774; all replacement targets return HTTP 200. Signed-off-by: jain-ria <riajain@NVIDIA.com>
20e88dd to
5b8122b
Compare
|
/ok to test 5b8122b |
|
Held for Dynamo 1.5.1. release/1.5.0 is closed at RC11, the 1.5.0 GA build, so this cherry-pick must not merge there. Converted to draft; it retargets to release/1.5.1 as soon as that branch is cut (patch targeted for 2026-09-30). Tracked on the Dynamo 1.5.1 cherry-pick canvas. |
Signed-off-by: jain-ria <riajain@NVIDIA.com>
Signed-off-by: jain-ria <riajain@NVIDIA.com>
Signed-off-by: jain-ria <riajain@NVIDIA.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Backport #14276 to
release/1.5.0for NVBug 6714438 / DYN-4263. Positivemin_tokensrequests on tokenizer-free SGLang decode workers leave minimum-token enforcement to Dynamo instead of sending SGLang an unsupportedmin_new_tokensparameter.Includes prerequisite #14378, which adds the decoder's
stream_ended/pending_flushstate, and the final cancellation follow-ups from #14276: preserve the context-ID abort log and abort every observed unfinished text-stream choice, including choices first observed after cancellation. Regression coverage exercises token and text streams, client cancellation, early stream closure, completed choices, and late responses.Also repairs stale documentation links that failed the release branch's link check, adapting the relevant fixes from #14353 and #14774.
Validation
859c8f2732757f0a7ad10000efbf9daf34691fb3) and pass against20e88dda30304a631e6100b6e75357d0ebaef21cin an isolated Python harness executing the actual stream/cancellation method bodies. This does not substitute for full-module or GPU integration tests.5b8122b3dd1d5aaf56d8c82f86813c54af8bdd8a. Its complete source tree is identical to validated head20e88dda30304a631e6100b6e75357d0ebaef21c; only history and attribution metadata changed to repair DCO. Fresh full CI is queued for this head. The previous full-CI run had a Planner build timeout; the earlier Triton and Dynamo runtime build cancellations were also one-hour timeouts.