Repository navigation
feat(beam): Beam 1.0 model support for the 1.6.0 Beam release - #16702
Conversation
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: The echoed prompt logprobs still set
text_offsetto an empty vector while adding one entry per prompt token to the other parallel arrays. Both streaming and aggregatedecho: trueresponses therefore return offsets only for generated tokens, so clients cannot associate echoed prompt tokens with positions. Populate cumulative prompt-token offsets before appending the shifted generated offsets. - Original discussion: This resolved defect remains at the reviewed head: echoed prompt logprobs still add entries to
tokens,token_logprobs, andtop_logprobswhile settingtext_offsetto an empty vector, soecho: trueresponses expose non-parallel per-token arrays. Populate one cumulative prompt-text offset per echoed token before appending shifted generated offsets.
|
|
||
|
|
||
| def _resolve_engine_utils() -> tuple[Callable[..., Any], Callable[..., Any]]: | ||
| """Bind the vLLM engine-launch helpers this pool needs, failing with a clear message. |
There was a problem hiding this comment.
This private helper docstring duplicates the immediately preceding explanation and the function signature/body: _resolve_engine_utils already shows that it binds and returns the two helpers, raises a tailored error, and synchronizes the module attributes used by tests. It adds no lasting constraint beyond that nearby documentation.
🤖 AI Fix
Remove the redundant _resolve_engine_utils docstring.
| #[test] | ||
| fn preserve_thinking_is_rejected_off_the_beam_route() { | ||
| let preprocessor = synthetic_preprocessor(false, HISTORY_TEMPLATE); | ||
| for value in [true, false] { |
There was a problem hiding this comment.
The true and false iterations cannot detect different regressions: the non-Beam production branch rejects solely on request.preserve_thinking.is_some(), without inspecting the Boolean value. Keep one supplied value to protect rejection of the field; the existing omitted case still protects acceptance when the field is absent.
🤖 AI Fix
Replace the [true, false] loop with one representative supplied preserve_thinking value.
|
|
||
| #[test] | ||
| fn preserve_thinking_rejects_non_bool_values() { | ||
| for bad in [json!("true"), json!("yes"), json!(1), json!({})] { |
There was a problem hiding this comment.
"true" and "yes" are both JSON strings and exercise the identical Serde string-to-Option<bool> type rejection; their contents are never interpreted. Retaining one string plus the numeric and object cases preserves the distinct invalid JSON-type coverage.
🤖 AI Fix
Remove either json!("true") or json!("yes") from the invalid-value cases.
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: Both JSON string cases remain in the invalid-value table even though Serde rejects them through the same string-to-Boolean type path; retain one string alongside the numeric and object cases.
- Original discussion: The
[true, false]loop remains, although the non-Beam branch rejects solely onpreserve_thinking.is_some(); retain one supplied value and the omitted case. - Original discussion: The echoed prompt logprobs still add one entry per prompt token to
tokens,token_logprobs, andtop_logprobswhile constructingtext_offsetas an empty vector. AfterCompletionEcho::prepend_tomerges generated offsets, both streaming and aggregatedecho: trueresponses expose non-parallel per-token arrays and omit positions for every echoed prompt token. Populate cumulative prompt-token offsets before appending the shifted generated offsets. - Original discussion: The echoed prompt still adds one entry per prompt token to
tokens,token_logprobs, andtop_logprobswhile constructingtext_offsetas an empty vector, leaving the response’s parallel per-token arrays misaligned. Populate cumulative prompt-token offsets before appending shifted generated offsets. - Original discussion: The redundant
_resolve_engine_utilsdocstring remains atcomponents/src/dynamo/vllm/embedding_worker_processes.py:44-48, duplicating the preceding module comment and the helper's signature/body. - Original discussion: The same redundant explanation that either parser slot selects Beam remains, moved to the test comment at
lib/llm/src/preprocessor/structural_tag.rs:1259-1260; the assertions already show both parser-slot cases. - Original discussion: Verified still present: echoed prompt logprobs populate
tokens,token_logprobs, andtop_logprobsper prompt token but leavetext_offsetempty. After folding,echo: trueresponses expose non-parallel arrays and offsets only for generated tokens. Populate cumulative prompt-token offsets before appending shifted generated offsets.
| } | ||
| } | ||
|
|
||
| /// `retain_model_stop_token_text` keeps the model's own EOS text (what a parser like |
There was a problem hiding this comment.
This test doc comment repeats the function name and the immediately visible EOS/user-stop/string cases without preserving an additional constraint.
🤖 AI Fix
Remove the redundant test doc comment.
| assert_eq!(tools[0].function.strict, None); | ||
| assert_eq!(tools[1].function.strict, Some(true)); | ||
| assert!(ValidateRequest::validate(&beam).is_ok()); | ||
| // Idempotent. |
There was a problem hiding this comment.
This comment merely labels the repeated call and zero-result assertion as idempotent; the code already expresses that property.
🤖 AI Fix
Remove the Idempotent comment.
| } | ||
| } | ||
|
|
||
| /// Apply a non-default framing before the parser consumes its first byte. |
There was a problem hiding this comment.
This private-helper doc comment restates apply_argument_framing and its explicit non-default check without recording any non-obvious requirement.
🤖 AI Fix
Remove the redundant helper doc comment.
| }, | ||
| ArgumentFraming::SchemaConstrained, | ||
| ), | ||
| ( |
There was a problem hiding this comment.
The frames_answer: true row cannot distinguish a framing regression from the retained frames_answer: false row: argument_framing matches BeamTurnGrammar { .. } and deliberately ignores this payload. The later assertions in this same test already cover the payload's distinct structured-response behavior, so keeping one Beam row here still protects the SchemaConstrained mapping.
🤖 AI Fix
Remove the BeamTurnGrammar { frames_answer: true } entry from the argument_framing table.
| let echo = self.echo.as_mut().expect("checked by wants_echo"); | ||
| let logprobs = match &prompt_logprobs { | ||
| Some(payload) if enable_logprobs => Some(echo.logprobs(payload, return_as_ids)), | ||
| None if enable_logprobs => { |
There was a problem hiding this comment.
Supported workers attach engine_data.prompt_logprobs to the final chunk (the vLLM handler explicitly captures the early engine payload and moves it there), but this path treats their absence on the first generated chunk as final and immediately records the choice in emitted. Consequently, ordinary multi-chunk echo: true requests with logprobs return only echoed text and permanently drop the requested prompt logprobs. Preserve streaming order while waiting for the prompt payload, or make the backend provide it on the first emitted choice chunk, and only then mark the echo emitted.
🤖 AI Fix
Buffer an echo + logprobs choice until its prompt-logprobs payload is available, or attach that payload to its first backend chunk, and mark the choice emitted only after constructing the complete echo.
…e engine reports them Problem (review on #16702): vLLM and SGLang attach the prompt logprobs to the final chunk. Echoing everything on the first chunk therefore dropped the prompt logprobs of every `echo: true` + `logprobs` request; echoing everything on the chunk that carries them put generated text before the prompt for streaming clients. Fix: the echo now travels in two parts. The prompt text rides the first chunk of each choice; the prompt logprobs ride the chunk that carries the engine's payload (text empty when it already went out). The unary aggregator merges the parts; `fold_echo` prepends whatever a chunk carries. If no payload arrives by the choice's end, the text-only echo stands and a warning is logged. Also from the review: one representative value in the preserve_thinking rejection tests, the duplicate `frames_answer: true` framing row dropped, and four comments that restated their code removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: doxu <doxu@nvidia.com>
|
Previously reported defects still present:
lib/llm/src/preprocessor.rs:3529 — The pinned renderer explicitly allows a legacy 🤖 AI FixResolve Beam built-in declarations into a local effective-tools request view and reuse that view for template rendering, turn-grammar construction, and parser tool definitions. |
Problem: the 1.6.0 Beam release images were built from the Dynamo Beam source on a private branch, which the public release branch does not carry. Root cause: Beam support was developed on a private branch while the model was unreleased; the frontend crates it needs (dynamo-renderer 8.1.0, dynamo-parsers-v2 0.10.0) are on the frontend-crates `release/1.6.0-beam-1.0-501b-dev.1` line, not on crates.io. Fix: bring the Beam source onto the release branch as one commit. - Beam parser family served through the unified v2 route; the parser starts in the channel an assistant prefill leaves open; only the model's stop-token text is kept on the Beam route. - Beam turn grammar (structural tag) on every chat turn, with forced tool choices kept on the Harmony wire; `builtin_tools` declared by name for the 1.0 template; response_format held in the final channel and held tool arguments framed. - Beam route policy: non-compliant `strict` tools served as non-strict with one warning per request; `preserve_thinking=false` accepted as a no-op; `allow_missing_thinking` defaults to true. - /v1/completions echoes the prompt with its logprobs. - vLLM: import chat_utils from its new location when needed. - Cargo: pin dynamo-renderer 8.1.0 and dynamo-parsers-v2 0.10.0 from the public frontend-crates repository at the Beam release revision. Signed-off-by: doxu <doxu@nvidia.com> Co-Authored-By:
Signed-off-by: Nate Mailhot <nmailhot@nvidia.com> (cherry picked from commit d806094) Co-Authored-By:
…replies Problem (review on #16702): with `echo: true`, batch completions remapped `choice.index` by `prompt_idx * n` but left `echo.choice_index` as the engine saw it, so the echo of every prompt after the first found no choice; and when the engine reported no prompt logprobs the echo rode the choice's final chunk, so a streaming client saw generated text before the prompt. Fix: - Remap `echo.choice_index` with the same offset as the choices. - Echo on the first chunk of each choice: with prompt logprobs when that chunk carries them (engines report them with the first token), text only otherwise. Test updated for the new order. - Drop three doc comments that only restated the code. Signed-off-by: doxu <doxu@nvidia.com> Co-Authored-By:
…heck Problem: `dynamo-truthy`'s repository scan (`no_bool_parse_forks`) fails CI on the hand-rolled `"0" | "false" | "off" | "no"` match that reads `DYN_BEAM_TURN_GRAMMAR`. Fix: use `dynamo_runtime::config::env_is_falsey` on the variable; unset keeps the grammar on, as before. Signed-off-by: doxu <doxu@nvidia.com> Co-Authored-By:
frontend-crates #430 squash-merged into `release/1.6.0-beam-1.0-501b-dev.1` as ef8f4013; move dynamo-renderer 8.1.0 and dynamo-parsers-v2 0.10.0 from the PR head to that commit and refresh the three lock files. Signed-off-by: doxu <doxu@nvidia.com> Co-Authored-By:
…e engine reports them Problem (review on #16702): vLLM and SGLang attach the prompt logprobs to the final chunk. Echoing everything on the first chunk therefore dropped the prompt logprobs of every `echo: true` + `logprobs` request; echoing everything on the chunk that carries them put generated text before the prompt for streaming clients. Fix: the echo now travels in two parts. The prompt text rides the first chunk of each choice; the prompt logprobs ride the chunk that carries the engine's payload (text empty when it already went out). The unary aggregator merges the parts; `fold_echo` prepends whatever a chunk carries. If no payload arrives by the choice's end, the text-only echo stands and a warning is logged. Also from the review: one representative value in the preserve_thinking rejection tests, the duplicate `frames_answer: true` framing row dropped, and four comments that restated their code removed. Signed-off-by: doxu <doxu@nvidia.com> Co-Authored-By:
2eb9443 to
1b4e060
Compare
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: Beam rendering still accepts and advertises a legacy
builtin_toolsdeclaration absent from the request'stools, while grammar construction and parser definitions use only the original tools. Build one effective tool set and reuse it for rendering, turn-grammar construction, and parsing. - Original discussion: Streaming
echo + logprobsstill emits generated-token logprobs before the prompt logprobs that supported workers attach to the final chunk, producing completion-prefix, prompt, completion-tail order. Buffer these choice chunks until the prompt payload arrives, or attach the prompt payload before the first generated chunk. - Original discussion: The previously reported explanation that either parser slot selects Beam remains, moved to
lib/llm/src/preprocessor/structural_tag.rs:1259-1260; the adjacent test cases already express both parser-slot paths. - Original discussion: The redundant
_resolve_engine_utilsdocstring remains atcomponents/src/dynamo/vllm/embedding_worker_processes.py:44-48, duplicating the preceding compatibility explanation and the helper's signature/body. - Original discussion: Verified still present: Beam's
builtin_tools_viewchanges only the request view used for template rendering, while turn-grammar construction and parser tool definitions still use the original request tools. A legacy built-in accepted and advertised in the prompt can therefore be absent from the grammar and parser schema. Resolve one effective tool set and reuse it for rendering, grammar construction, and parsing. - Original discussion: Verified still present:
EchoState::logprobscreates prompt entries intokens,token_logprobs, andtop_logprobsbut leavestext_offsetempty, soecho: trueresponses expose non-parallel per-token arrays and omit offsets for echoed prompt tokens. Populate cumulative prompt-token offsets before appending shifted generated offsets. - Original discussion: Verified still present: supported workers provide prompt logprobs on the final chunk after generated-token logprobs have already streamed, producing the client-visible order generated prefix, prompt, generated tail instead of prompt followed by completion. Buffer
echo + logprobschoice chunks until the prompt payload arrives, or attach the prompt payload before the first generated chunk. - Original discussion: Verified still present: supported workers provide prompt logprobs on the final chunk, while earlier generated-token logprobs are already streamed. The final chunk prepends prompt logprobs only within that chunk, so SSE order is generated-prefix, prompt, generated-tail rather than prompt followed by completion. Buffer
echo + logprobschunks until the prompt payload arrives or attach that payload before the first generated chunk. - Original discussion:
EchoState::logprobsstill creates prompt entries intokens,token_logprobs, andtop_logprobswhile leavingtext_offsetempty, so echoed completion responses expose non-parallel per-token arrays. Populate a cumulative offset for every prompt token before appending shifted generated offsets.
| let tools_before = serde_json::to_value(&request.inner.tools).unwrap(); | ||
| let rendered = preprocessor.apply_template(&request).unwrap().unwrap(); | ||
| assert_eq!(rendered.as_str(), "SYS:bash:string;DEV:weather;"); | ||
| // The request's real tools and kwargs are untouched. |
There was a problem hiding this comment.
This test comment only narrates the two immediately following assertions, which directly verify that tools and builtin_tools remain unchanged; it preserves no additional invariant.
🤖 AI Fix
Remove the redundant test comment.
…ompletions until their prompt logprobs Problem (review on #16702): - A legacy `builtin_tools` declaration object absent from `tools` was rendered into the prompt but unknown to the turn grammar and the parser, so the model could be shown a tool it could not call. - With `echo: true` and `logprobs`, generated logprobs streamed before the prompt logprobs that vLLM and SGLang attach to the final chunk. Fix: - `declare_legacy_builtin_tools_for_beam` adds such declarations to the request's `tools` on the Beam route, before validation, in both the HTTP handler and the in-process preprocessor entry; the renderer then sees them as declared and the grammar and parser build from the same list. - The completions delta generator holds a choice's generated text and logprobs while its prompt logprobs are pending and releases them, prompt positions first, on the chunk that carries the payload. - Two test comments that restated their assertions removed. Signed-off-by: doxu <doxu@nvidia.com>
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: The previously reported echo-logprobs defect remains:
EchoState::logprobsstill creates onetokens,token_logprobs, andtop_logprobsentry per prompt token while leavingtext_offsetempty, so echoed completion responses expose non-parallel per-token arrays. Populate a cumulative prompt-text offset for every echoed token before merging generated offsets. - Original discussion: The redundant
_resolve_engine_utilsdocstring remains at lines 44–48, duplicating the preceding module comment and the helper’s signature and body. - Original discussion: The reported
echo: truelogprob defect remains:EchoState::logprobspopulatestokens,token_logprobs, andtop_logprobsfor each prompt token while leavingtext_offsetempty, so echoed responses expose non-parallel per-token arrays. Populate cumulative prompt-token offsets before merging generated offsets.
| Some(&plain), | ||
| ); | ||
| assert!(!prefill.unified && !prefill.response); | ||
| // Another family's prompt never sets the Beam response prefill. |
There was a problem hiding this comment.
This test comment merely narrates the immediately following Qwen-family input and assert!(!prefill.response); it preserves no additional invariant.
🤖 AI Fix
Remove the redundant test comment.
| assert_eq!(applied, GuidedToolConstraint::None); | ||
| assert!(installed_structural_tag(&common_request).is_none()); | ||
|
|
||
| // Other parsers see no Beam grammar. |
There was a problem hiding this comment.
This comment only labels the hermes case whose subsequent assertion already shows that a non-Beam parser installs no Beam structural tag.
🤖 AI Fix
Remove the redundant test comment.
| .iter_mut() | ||
| .find(|choice| choice.index == index) | ||
| { | ||
| if prompt_logprobs.is_none() && finish_reason.is_none() { |
There was a problem hiding this comment.
For streaming completions with both echo and logprobs, supported vLLM/SGLang workers attach prompt logprobs to the final chunk, so this branch removes every generated text/logprobs delta and accumulates it until generation finishes. The response therefore behaves effectively as unary after the initial echoed-prompt chunk, increasing time-to-first-generated-token to full generation time and retaining the complete output per choice in memory. Make prompt logprobs available on the first emitted choice chunk so subsequent generated deltas can stream normally.
🤖 AI Fix
Attach the engine's prompt-logprobs payload to each choice's first emitted chunk and emit subsequent generated text and logprobs without accumulating them in EchoState::held.
Signed-off-by: neilkg <33635204+neilkg@users.noreply.github.com>
Signed-off-by: neilkg <33635204+neilkg@users.noreply.github.com>
b90142f
into
release/1.6.0-beam-1.0-501b-dev.1
Ports the Beam 1.0 support merged into release/1.6.0-beam-1.0-501b-dev.1 (#16702) onto main, pinned to the published crates instead of the release line: dynamo-parsers-v2 0.7.29 (the Beam unified family, frontend-crates #438), dynamo-renderer 8.1.0 (the Beam builtin_tools rendering view) and dynamo-tokenizers 3.1.2 (required by renderer 8.1.0). (cherry picked from commit b90142f) Signed-off-by: doxu <doxu@nvidia.com> Signed-off-by: Nate Mailhot <nmailhot@nvidia.com> Signed-off-by: neilkg <33635204+neilkg@users.noreply.github.com> Co-authored-by: Pei Li <peili@nvidia.com> Co-authored-by: Nate Mailhot <nmailhot@nvidia.com> Co-authored-by: neilkg <33635204+neilkg@users.noreply.github.com> Signed-off-by: doxu <doxu@nvidia.com>
Ports the Beam 1.0 support merged into release/1.6.0-beam-1.0-501b-dev.1 (#16702) onto main, pinned to the published crates instead of the release line: dynamo-parsers-v2 0.7.29 (the Beam unified family, frontend-crates #438), dynamo-renderer 8.1.0 (the Beam builtin_tools rendering view) and dynamo-tokenizers 3.1.2 (required by renderer 8.1.0). (cherry picked from commit b90142f) Signed-off-by: doxu <doxu@nvidia.com> Signed-off-by: Nate Mailhot <nmailhot@nvidia.com> Signed-off-by: neilkg <33635204+neilkg@users.noreply.github.com> Co-authored-by: Pei Li <peili@nvidia.com> Co-authored-by: Nate Mailhot <nmailhot@nvidia.com> Co-authored-by: neilkg <33635204+neilkg@users.noreply.github.com> Signed-off-by: doxu <doxu@nvidia.com>
Ports the Beam 1.0 support merged into release/1.6.0-beam-1.0-501b-dev.1 (#16702) onto main, pinned to the published crates instead of the release line: dynamo-parsers-v2 0.7.29 (the Beam unified family, frontend-crates #438), dynamo-renderer 8.1.0 (the Beam builtin_tools rendering view) and dynamo-tokenizers 3.1.2 (required by renderer 8.1.0). (cherry picked from commit b90142f) Signed-off-by: doxu <doxu@nvidia.com> Signed-off-by: Nate Mailhot <nmailhot@nvidia.com> Signed-off-by: neilkg <33635204+neilkg@users.noreply.github.com> Co-authored-by: Pei Li <peili@nvidia.com> Co-authored-by: Nate Mailhot <nmailhot@nvidia.com> Co-authored-by: neilkg <33635204+neilkg@users.noreply.github.com> Signed-off-by: doxu <doxu@nvidia.com>
Ports the Beam 1.0 support merged into release/1.6.0-beam-1.0-501b-dev.1 (#16702) onto main, pinned to the published crates instead of the release line: dynamo-parsers-v2 0.7.29 (the Beam unified family, frontend-crates #438), dynamo-renderer 8.1.0 (the Beam builtin_tools rendering view) and dynamo-tokenizers 3.1.2 (required by renderer 8.1.0). (cherry picked from commit b90142f) Signed-off-by: doxu <doxu@nvidia.com> Signed-off-by: Nate Mailhot <nmailhot@nvidia.com> Signed-off-by: neilkg <33635204+neilkg@users.noreply.github.com> Co-authored-by: Pei Li <peili@nvidia.com> Co-authored-by: Nate Mailhot <nmailhot@nvidia.com> Co-authored-by: neilkg <33635204+neilkg@users.noreply.github.com> Signed-off-by: doxu <doxu@nvidia.com>
Ports the Beam 1.0 support merged into release/1.6.0-beam-1.0-501b-dev.1 (#16702) onto main, pinned to the published crates instead of the release line: dynamo-parsers-v2 0.7.29 (the Beam unified family, frontend-crates #438), dynamo-renderer 8.1.0 (the Beam builtin_tools rendering view) and dynamo-tokenizers 3.1.2 (required by renderer 8.1.0). (cherry picked from commit b90142f) Signed-off-by: doxu <doxu@nvidia.com> Signed-off-by: Nate Mailhot <nmailhot@nvidia.com> Signed-off-by: neilkg <33635204+neilkg@users.noreply.github.com> Co-authored-by: Pei Li <peili@nvidia.com> Co-authored-by: Nate Mailhot <nmailhot@nvidia.com> Co-authored-by: neilkg <33635204+neilkg@users.noreply.github.com> Signed-off-by: doxu <doxu@nvidia.com>
Summary
Brings the Dynamo Beam 1.0 model support to
release/1.6.0-beam-1.0-501b-dev.1, as built into the 1.6.0 Beam release images. Counterpart of frontend-crates #430.Three commits on top of main
c5773baaa5(the commit the release images were built from):fix(vllm): cherry-pick of upstream fix(vllm): import engine-launch helpers lazily so workers start on new vLLM #16378 (lazy vLLM engine-launch imports), needed with the Beam vLLM engine. The upstream squash commit carries no DCO trailer, so this cherry-pick is re-authored with the original author as co-author.feat(beam): the Beam source as one commit.docs(rl): cherry-pick of upstream docs(rl): fix dead Miles link failing lychee #16573 (dead link that fails the lychee check on this base).builtin_toolsdeclared by name for the 1.0 template;response_formatheld in the final channel with held tool arguments framed.stricttools served as non-strict with one warning per request;preserve_thinking=falseaccepted as a no-op;allow_missing_thinkingdefaults to true./v1/completionsechoes the prompt with its logprobs.chat_utilsimported from its new location when needed.dynamo-renderer8.1.0 anddynamo-parsers-v20.10.0 pinned from frontend-crates at the Beam release revision (ef8f4013, the Request Documentation/Examples for Running DeepSeek R1 671B with Prefill-Decode Separation in AI-Dynamo #430 squash merge on that branch).Base branch
The release branch was fast-forwarded to
c5773baaa5on 2026-10-17, the commit the release images were built from; this PR is the two commits above on top of it.Validation