Repository navigation
chore(release/1.5.0): cherry-pick #13640, #14317, #14525, #12856, #13951 - #15011
nv-nmailhot merged 5 commits into
Conversation
Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com> (cherry picked from commit 50c6fea)
Signed-off-by: William Arnold <7565007+Aphoh@users.noreply.github.com> Co-authored-by: Ryan McCormick <rmccormick@nvidia.com> (cherry picked from commit 8dcfd50)
Signed-off-by: William Arnold <7565007+Aphoh@users.noreply.github.com> (cherry picked from commit c3e05f0)
Signed-off-by: William Arnold <7565007+Aphoh@users.noreply.github.com> (cherry picked from commit 0949758)
Signed-off-by: William Arnold <7565007+Aphoh@users.noreply.github.com> Co-authored-by: Zero Rains <57100978+zeroRains@users.noreply.github.com> (cherry picked from commit 8c5a737)
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Backport review: 5 cherry-picks onto release/1.5.0
I compared each cherry-pick against the commit that merged on main, not against the
branch head, because all five landed as squash merges. 46 of the 47 file changes are
byte-identical. The one file that did not come across was already on the branch. The Rust
and the Python code both work on the older base.
Approved. One P3 below.
[P3] The description does not say that one file from #13640 was not carried.
lib/llm/src/protocols/openai/chat_completions/delta.rs. The omission is correct,
because the branch already holds that change at line 125. Please add one line to the
description so the next reader does not have to prove it again.
Per-pull-request comparison: source merge commit against cherry-pick
I split each diff per file, then took a SHA-256 of the added lines and of the removed
lines on each side. Equal byte counts prove nothing, so the checksums are the test.
| PR | source merge on main |
cherry-pick | files | result |
|---|---|---|---|---|
| #13640 | 50c6fea10d |
931bae38b8 |
11 vs 10 | 10 identical, 1 not carried |
| #14317 | 8dcfd50cde |
a086dffa25 |
12 vs 12 | all identical |
| #14525 | c3e05f0244 |
73c0a329c3 |
4 vs 4 | all identical |
| #12856 | 09497580bb |
06a4db49c3 |
12 vs 12 | all identical |
| #13951 | 8c5a737231 |
f27386479c |
8 vs 8 | all identical |
c3e05f0244 has 8dcfd50cde as its parent, so #14525 sat directly on top of #14317 on
main. The cherry-picks keep that order.
The one omission, and why it is correct
lib/llm/src/protocols/openai/chat_completions/delta.rs is the only file from a source
pull request that is not in its cherry-pick. #13640 changed it by +51/-23 on main.
The branch already had that change. I took all 48 added lines of the #13640 hunk and
looked for each one in the file at the merge base 3507149353. 47 match word for word,
including the test test_chat_logprobs_without_top_logprobs_include_sampled_token. The
48th line differs because the branch wraps that call in a guard the main version at
that time did not have:
let converted = if self.suppress_top_logprobs {
Vec::new()
} else {
convert_backend_top_logprobs(top_lps, t, *tid, lp, return_as_ids)
};That guard is at lib/llm/src/protocols/openai/chat_completions/delta.rs:125. It
survives all five cherry-picks: suppress_top_logprobs appears 4 times in the file at
the merge base and 4 times at the head.
Does each change still work on the older base
The branch differs from main in the files these patches touch, so a clean apply is not
proof. The largest gaps at the point each patch applied:
| file | branch against main |
|---|---|
lib/llm/src/preprocessor.rs |
+142/-1026 |
components/src/dynamo/frontend/sglang_prepost.py |
+55/-17 |
components/src/dynamo/sglang/request_handlers/handler_base.py |
+45/-12 |
lib/llm/src/backend.rs |
+3/-4 |
What the ported code calls, and whether the branch has it:
- #13640 calls
RequestTracker::set_prompt_token_idsandprompt_token_ids(). The same
set adds both inlib/llm/src/protocols/common/timing.rs. - #13951 calls
resolve_configured_engine_routesfrom
components/src/dynamo/sglang/engine_routes.py. That file is byte-identical on both
branches, so the"pause_generation:tm"descriptor form works. - #14317 renames
_strip_trailing_eos_token_idsto_strip_matched_stop_token_ids. No
caller of the old name is left on the branch. - #14317 gives
flush_pendinga new keyword-only argumentstop_terminatedwith no
default. Both call sites, atcomponents/src/dynamo/frontend/sglang_processor.py:878
and:915, pass it. - #13951 deletes
components/src/dynamo/sglang/pause.py. Nothing on the branch still
imports it. - The Slime example uses
apiVersion: nvidia.com/v1beta1. That version is served and is
the storage version on this branch, and 75 other examples already use it.
On the x86_64 Linux box with the pinned 1.96.1 toolchain:
cargo test -p dynamo-llm --no-default-features --lib
test result: ok. 2388 passed; 0 failed; 3 ignored
All 19 changed Python files compile.
Release-branch rules, push shape, and what the branch gained
- Base branch is
release/1.5.0, which is what the title says. - No version file, pin, or lock file is in the 41-file diff.
CODEOWNERSis generated. This set changes it together with
.github/codeowners/areas.yaml, one line each, and thecodeownerscheck passed.- The issue timeline has no
head_ref_force_pushedevent. The 5 commits sit in a line
on the merge base3507149353. - Since that merge base,
release/1.5.0gained #15017 and #15027. Neither touches any of
the 41 files, so no contract moved under this set. - The lineage carries no
v1.5.0release tag, only-devtags, so removing the two
control/*engine routes lands before the release, not after it.
Test lane, and mutation results with controls
The lane is sglang-runtime / Test cuda13.0, amd64, which runs
pytest -m "pre_merge and sglang and gpu_0" from .github/workflows/pr.yaml. Run
against the changed test files on this branch:
683/763 tests collected (80 deselected)
The new test tests/test_sglang_gms_runtime_routes.py::test_sglang_pause_resume_use_native_engine_routes
is in that set. The 4 tests in components/src/dynamo/thunderagent_router/tests/test_args.py
are not, because the file carries no sglang mark. They pass on their own.
I saw 14 failures in components/src/dynamo/sglang/tests/test_sglang_unit.py. The merge
base gives the same 14 failures with the same names, so they belong to the local
environment and not to this set.
Mutation, both directions, with a SHA-256 after each write:
| step | file checksum | result |
|---|---|---|
| baseline | de5bc2b05b0f92cd |
1 passed |
rename the pause route in tests/gpu_memory_service/common/runtime.py |
e8f27858a534bf3a |
1 failed |
| restore | de5bc2b05b0f92cd |
1 passed |
| baseline | 56f8d071f062b6ee |
1 passed |
return early in _strip_matched_stop_token_ids |
192eaf11dbebe002 |
1 failed |
| restore | 56f8d071f062b6ee |
1 passed |
Controls: components/src/dynamo/common/backend/tests/test_logprobs.py stayed at
39 passed, 1 skipped through every step of the first mutation. For the second, the
enclosing class went from 30 passed to 2 failed and 28 passed, so 28 tests did not move.
Claims in the description, checked
Every number in the Validation section is exact.
cargo fmt --all -- --check -> clean
delta -> 63 passed
extensions -> 47 passed
timing -> 32 passed
preprocessor -> 188 passed
sglang -> 19 passed
Summary
Cherry-picks the following merged PRs into
release/1.5.0as a single release branch PR:feat(llm): expose training metadata for RL rolloutsfix(sglang): preserve stop tokens and incremental streamsfeat(frontend): expose detailed finish reasonfeat(rl): add Slime external rollout examplefix(sglang): sync discovery from native pause statePR #14525 depends on the
prompt_token_idssupport introduced in #13640, so #13640 is included as a dependency.Validation
cargo fmt --allcargo test -p dynamo-llm --no-default-features --libfilters passed:delta(63 tests)extensions(47 tests)timing(32 tests)preprocessor(188 tests)sglang(19 tests)cargo test -p dynamo-llmrun shows unrelated network/timing failures in the local macOS environment.