Repository navigation
chore(deps): [cherry-pick 1.5.1] bump dynamo-renderer to 5.1.2 and dynamo-protocols to 5.4.1 (#14595, #14154) - #14806
Conversation
Signed-off-by: Kay Yan <kay.yan@daocloud.io>
Signed-off-by: doxu <doxu@nvidia.com>
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Reviewed as a cherry-pick of #14595 onto release/1.5.0. Everything below was run locally against head f67175cb on macOS ARM64 (cargo test -p dynamo-llm --lib --no-default-features --locked), not read off the diff.
What the cherry-pick actually contains
gh api repos/ai-dynamo/dynamo/compare/release/1.5.0...f67175cb is two upstream commits, not one: #14595 (dynamo-renderer 5.1.0 -> 5.1.2, dynamo-tokenizers 1.8.0 -> 1.8.1) and #14154 (dynamo-protocols 5.4.0 -> 5.4.1).
The second one is required, not stray churn: the crates.io sparse index records dynamo-renderer 5.1.2 as depending on dynamo-protocols ^5.4.1, and this workspace pins that exactly, so 5.1.2 cannot resolve against =5.4.0. On main it was already present (#14154 merged a week before #14595), which is why #14595's own diff is Cargo.toml +2/-2 while this one is +3/-3 — and the extra line is exactly dynamo-protocols. The cherry-pick is faithful to both.
Verified
- The regression test pins the bump. Reverting only
Cargo.tomland the three lockfiles to the merge-base (08a2c4f6) while keeping the new test,test_nonleading_system_message_normalized_for_strict_templatefails on renderer 5.1.0 withInvalidArgument: invalid operation: System message must be at the beginning. (in tool_use:1), and passes on 5.1.2. Control on the same tree:test_restrictive_template_accepts_request_with_user_messagepasses both ways, so that is the bump and not the environment. - The lockfiles are genuine resolutions, not a hand-merge. All three checksums match
index.crates.ioexactly (dynamo-renderer 5.1.2->3ae2eaa1...,dynamo-tokenizers 1.8.1->ed232a9d...,dynamo-protocols 5.4.1->dbaf3179...), andcargo metadata --locked --no-depsis accepted for the root workspace and for both binding workspaces (lib/bindings/python,lib/bindings/kvbm). - One pin site, no drift surface. All three are
[workspace.dependencies]entries consumed viaworkspace = true(lib/llm,lib/backend-common,deploy/inference-gateway/ext-proc); grep finds no second copy of any of the three version literals outsideCargo.tomland the lockfiles, anddocs/fern/scripts/tests/test_gen_rust_api.pydiscovers versions rather than hard-coding them.
Two P2s inline, both about the gap between what the one-line body promises and what this lands on the release branch. Neither asks for a code change. No P0/P1.
I also replied on the existing dynamo-review-agent thread with a measured refutation of its premise.
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Approving. Zero P0/P1; the two outstanding items are both P2 and both ask for description text, not a code change.
Verified by execution (macOS ARM64, head f67175cb):
- The cherry-pick is faithful and complete.
compare/release/1.5.0...f67175cbcarries #14595 and #14154. The second is required, not stray:dynamo-renderer 5.1.2declaresdynamo-protocols ^5.4.1in the crates.io index against an exact workspace pin, so 5.1.2 cannot resolve on=5.4.0. The delta from #14595's ownmaindiff (Cargo.toml +2/-2there,+3/-3here) is exactly that one line. - The new test pins the bump. With only
Cargo.tomland the three lockfiles reverted to the merge-base08a2c4f6and the test kept,test_nonleading_system_message_normalized_for_strict_templatefails on renderer 5.1.0 (InvalidArgument: invalid operation: System message must be at the beginning. (in tool_use:1)) and passes on 5.1.2. Controltest_restrictive_template_accepts_request_with_user_messagepasses on both trees. - The lockfiles are real resolutions. All three registry checksums match
index.crates.io, andcargo metadata --locked --no-depsis accepted for the root workspace and both binding workspaces. - One pin site. All three versions live only in
[workspace.dependencies]and reach consumers viaworkspace = true; no second copy of any version literal exists outsideCargo.tomland the lockfiles.
Remaining, both P2, both non-blocking:
Cargo.toml:71— the body says "add #14595 to release" but this also lands #14154's client-visible/v1/chat/completionsserialization change (nulls omitted,refusalalways present) onrelease/1.5.0, and no other cherry-pick PR carries it here.Cargo.toml:63— 5.1.1 rides along and changes the default DeepSeek V4 prompt: measured 73 -> 549 characters for an identical request with noreasoning_effort, inserted at index 0.
Neither needs a code change. Please fold both into the description so they reach the 1.5.0 release notes.
Deliberately deferred: I ran the targeted test plus one control rather than the full dynamo-llm suite locally — rust-tests (.) is green on this head and covers it. The DeepSeek V4 evidence is renderer-level rendering through deepseek_formatter_for, the exact entry point prompt_formatter_from_mdc uses; I did not exercise it end to end against a served V4 model.
I also replied on the dynamo-review-agent thread: its premise does not hold. The normalization is gated on a load-time probe, a template that accepts a non-leading system is left untouched, and where demotion does happen the pre-bump behaviour was a hard 400 — so the change lowers that content's authority rather than raising it.
|
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. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
add #14595 to release