Repository navigation
build(container): add Inkling vLLM 0.28.0 runtime support - #13954
Merged
ynpandey-nv merged 30 commits intoAug 31, 2026
Conversation
…rocessing
vLLM reasoning parsers expose adjust_request() so they can declare the sampling
params they need. Parsers that split thinking from the answer on text markers
require `skip_special_tokens=False`, otherwise the markers are stripped before
the parser ever sees them and the split silently does not happen.
`_prepare_request` calls the TOOL parser's adjust_request but there is no
equivalent for the reasoning parser -- it does not even receive
`reasoning_parser_class`, since the parser is constructed later, in
StreamingPostProcessor, long after the sampling request is finalised.
Net effect: `reasoning_content` comes back empty unless the CLIENT sends
`skip_special_tokens: false`. Ordinary OpenAI-compatible clients do not, so on
such models the reasoning channel silently does nothing for normal traffic. The
tool path is additionally gated on `tool_choice != "none"`, so a plain
tools-free chat request gets no adjust_request call from either parser.
Mirror the tool-parser path: construct the reasoning parser during request
preparation and let it adjust the request.
The gating matters and is deliberately identical to StreamingPostProcessor's own
parser construction (`reasoning_parser_class and not thinking_disabled`, where
`thinking_disabled = chat_template_kwargs.get("enable_thinking") is False`). If
the sampling params were adjusted for a parser that then ends up None, the
model's control markers would reach the client as visible text. The call is
therefore placed after chat_template_kwargs is resolved.
This is a no-op for parsers that do not need it: the base
ReasoningParser.adjust_request is `return request`.
Two files are required. prepost.py alone is inert -- vllm_processor.py must pass
reasoning_parser_class into preprocess_chat_request, otherwise the new parameter
defaults to None and the gate never fires.
Adds unit coverage for the wiring and, importantly, for the gate: the parser must
NOT be constructed or consulted when enable_thinking is False.
Signed-off-by: Yifan Jiang <19356972+yifjiang@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review feedback on ai-dynamo#12541: adjust_request() can change more than skip_special_tokens. A reasoning parser that rewrites the caller's constraint -- converting response_format into the structural tag its model actually speaks -- had that rewrite dropped. parser_guided_decoding was gated on `tool_parser is not None`, and build_tool_call_guided_decoding returns early when the request has no tools, so the rewrite never reached guided_decoding. The request carried the structural tag while the engine was handed the pre-adjustment JSON schema captured before _prepare_request ran. Recompute the parser guidance when either parser is active, and let a rewrite that only the reasoning parser can have made (no tool parser in the request, so it is attributable) take precedence over the caller's pre-adjustment copy. Scoped deliberately: when a tool parser is also active the change is not attributable to either parser, and parser-vs-assistant precedence is already settled there -- the caller's constraint wins, pinned by test_assistant_guidance_takes_precedence_over_auto_tool_guidance and the response-format-precedence row of TOOL_GUIDANCE_PARITY_CASES. Revisiting that policy is a separate change. Also single-source the reasoning-parser gate as _reasoning_parser_enabled(), a TypeGuard now shared by _prepare_request, preprocess_chat_request and StreamingPostProcessor. The three must agree: if they drift, the sampling params get adjusted for a parser that then does not exist and the model's control markers reach the client as visible text. Tests: four cases in TestReasoningParserGuidanceForwarding covering the rewrite with no tool parser, a pass-through parser, no parser, and the thinking-disabled gate. The first fails without this change with the reported symptom ({"json": {"type": "object"}} instead of the structural tag). The reasoning-parser stubs move to a per-test fixture so no mutable registry is shared at module scope, per .ai/pytest-guidelines.md (CodeRabbit RUF012, Devin). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqtTc1QsXejXiEdpmLZ4Wz
Removes the tradeoff in the previous commit rather than settling it.
Both parsers rewrite the same request inside _prepare_request -- tool first,
reasoning second -- and the code diffed the guidance across both calls at once.
That single diff cannot say which parser moved it, so attribution was inferred
from `tool_parser is None`. The inference is what forced a choice: with both
parsers active a reasoning rewrite was unattributable, so it lost to the
caller's copy, which is the bug this PR exists to fix.
Take the reading between the two calls and attribution stops being a guess:
client -> after_tool_parser changed by the tool parser
after_tool_parser -> final changed by the reasoning parser
Each rewrite then gets its own precedence, which is what was wanted all along.
A reasoning rewrite wins whether or not a tool parser is active, closing the
gap. A tool rewrite still loses to the caller's explicit constraint, so the
pinned policy is untouched. When both rewrote it there is nothing to arbitrate:
the reasoning parser ran last, on the already-tool-adjusted request, so the
final value IS the composition -- forwarding it is forwarding what the request
holds, which is the invariant the report was about.
_prepare_request takes an optional write-only guidance_snapshots dict rather
than a sixth return value: the return is unpacked at 22 call sites across the
two test files and widening it would have churned all of them.
The snapshot deliberately uses request_for_sampling.structured_outputs, the same
accessor as adjusted_structured_guidance, since it is only ever compared against
that value; extract_structured_outputs() is a different view and would have made
the diff spurious.
Deletes reasoning_only_guided_decoding and its scoping rationale.
Validated against the previous commit, same container, prepost.py swapped
underneath:
test_vllm_processor_unit.py before 11 failed / 117 passed
after 10 failed / 118 passed
regressions: none; the only delta is the new gap test
tests/frontend/test_prepost.py 15 passed both
The gap test fails on the previous commit with the exact symptom from the
report -- {'json': {'type': 'object'}} instead of the structural tag -- and the
three pinned cases pass under both.
Signed-off-by: Yifan Jiang <19356972+yifjiang@users.noreply.github.com>
…snapshot Addresses three review threads on ai-dynamo#12541. [P1] model_config. ReasoningParser.__init__ reads model_config out of **kwargs, and BaseCohereCommandReasoningParser.adjust_request returns the request untouched when it is absent (`if model_architecture is None: return request`). Constructing without it made this PR's whole adjust_request() call a no-op for the only shipped parser that overrides it. Verified against real vLLM: same parser, same request, model_config absent -> response_format kept and structured_outputs None; present -> structural tag produced and response_format cleared. The config is already reachable as self.input_processor.model_config (InputProcessor sets it from vllm_config), so no constructor change is needed. Threaded to all three construction sites -- _prepare_request, StreamingPostProcessor, and _build_reasoning_parser_metadata. The third does not read it, but constructing one of them differently is exactly the drift that hid this. NOT added to parser_kwargs: that dict goes on the wire to the worker and ModelConfig is not serializable. [P2] Snapshot cost. The guidance snapshot moved inside the _reasoning_parser_enabled() gate, since its only reader sits behind the same test and short-circuits. The deepcopy is gone: adjust_request() replaces request.structured_outputs with a fresh object rather than mutating it, so a plain reference still denotes the pre-adjustment value -- and aliasing lets the caller's `!=` short-circuit on identity instead of walking the tree. Measured on a 1.49 MB schema: 56.081 ms -> 0.0001 ms for the copy, plus 0.7155 ms -> 0.0001 ms on the comparison it feeds. Tool-only and no-parser requests now pay neither. [P2] Boundary test. test_generator_inner_forwards_reasoning_parser_and_model_config drives the real _generator_inner with a spy and asserts both wiring arguments arrive. Deleting either line fails only this test; before it, deleting reasoning_parser_class left all 143 green. Traded out the overlapping test_no_reasoning_parser_keeps_the_caller_constraint per the review. Also adds _ModelConfigGatedReasoningParser, a fake that mirrors Cohere's actual contract (no model_config => no rewrite). The existing fakes ignored model_config entirely, which is why a no-op passed them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqtTc1QsXejXiEdpmLZ4Wz
…i-dynamo#12510) Signed-off-by: Yifan Jiang <19356972+yifjiang@users.noreply.github.com> Co-authored-by: Ryan McCormick <rmccormick@nvidia.com>
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
sungsooha
temporarily deployed
to
external_collaborator
August 28, 2026 04:44 — with
GitHub Actions
Inactive
sungsooha
temporarily deployed
to
external_collaborator
August 28, 2026 04:44 — with
GitHub Actions
Inactive
Contributor
|
👋 Hi sungsooha! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
sungsooha
marked this pull request as ready for review
August 28, 2026 04:51
Contributor
Author
|
/ok to test f1bf02e |
sungsooha
temporarily deployed
to
external_collaborator
August 28, 2026 05:01 — with
GitHub Actions
Inactive
sungsooha
added a commit
to sungsooha/dynamo
that referenced
this pull request
Aug 28, 2026
… marker Refreshes the vendored patch from vllm-project/vllm#54120, which now begins the tool/text tags at `<|content_invoke_tool_json|>` / `<|content_text|>` instead of at `<|message_model|>`. Inkling's chat template prefills the role marker into the generation prompt (it ends `...<|end_message|><|message_model|>`), so generation resumes inside the model message and never re-emits it. The tags are `tags_with_separator` with `at_least_one=true`, so they anchor rather than trigger -- the old grammar therefore required a SECOND role marker. Compiling the tag with xgrammar shows the inversion, for forced/required/auto alike: suffix-only with duplicate marker before rejected accepted after accepted rejected It appeared to work only because the parser absorbed the duplicate, while manufacturing exactly the unconsumed-control-marker residue ai-dynamo#12510 strips. Validated on Inkling-NVFP4 / GB300, vLLM 0.28.0, MTP 8, 2-replica AGG, applied over the shipped image via ConfigMap: forced tool choice honoured 5/5 against a prompt matching the other tool; required = 1 call; parallel_tool_calls true/false = 2/1; no control-marker residue in content -- all in both streaming and non-streaming. Full p0 blocking count unchanged at 2 (the unrelated reasoning_budget_zero vLLM limitation). Note the probe suite could NOT have caught this: control_marker_residue passed before the fix too, because the parser was absorbing the duplicate. The grammar-level acceptance test is what exposed it. Reported by dynamo-review-agent on ai-dynamo#13954. Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
sungsooha
temporarily deployed
to
external_collaborator
August 28, 2026 05:17 — with
GitHub Actions
Inactive
Contributor
Author
|
/ok to test ac199a6 |
Dropping the preflight script. The review correctly found it fails OPEN -- a PyPI fetch error was printed and ignored, so an unreachable index would exit 0 and falsely validate. A gate that passes when it cannot check is worse than no gate. That is a symptom rather than the reason to remove it. The script only compared pinned packages against EACH OTHER, so it never covered transitive dependencies; the container build already catches this class and everything metadata cannot see. It was added reacting to my own mistake, and it costs reviewers a second source of truth about dependency resolution that can drift from what uv does. What prevents recurrence is the comment kept in container/context.yaml: it records that vLLM-Omni, not vLLM, is what binds transformers below 5.15, and that the thin dev image carrying 5.15.1 does not install omni at all -- so it was never evidence that the combination resolves. That sits at the point of edit, which is where the mistake was made. Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
sungsooha
force-pushed
the
sungsooha/release-1.5.0-inkling-dev.1-container
branch
from
August 28, 2026 16:01
5260ed7 to
818ec76
Compare
sungsooha
temporarily deployed
to
external_collaborator
August 28, 2026 16:01 — with
GitHub Actions
Inactive
sungsooha
added a commit
to sungsooha/vllm
that referenced
this pull request
Aug 28, 2026
…arker
The tag constrains the GENERATED SUFFIX, but it began at `<|message_model|>`.
Inkling's chat template prefills that role marker into the generation prompt --
the prompt ends `...<|end_message|><|message_model|>` -- so generation resumes
inside the model message and never re-emits it.
Because the tool/text tags are `tags_with_separator` with `at_least_one=true`,
they anchor rather than merely trigger, so the grammar required a SECOND role
marker. Compiling the tag with xgrammar and feeding it token sequences shows the
inversion directly, for forced/required/auto alike:
suffix-only with duplicate marker
before rejected accepted
after accepted rejected
It appeared to work only because the parser absorbed the duplicate -- while
manufacturing exactly the unconsumed-control-marker residue that dynamo#12510
exists to strip.
Begin the tags at `<|content_invoke_tool_json|>` / `<|content_text|>` instead;
`_INKLING_MESSAGE_MODEL` is then unused and removed.
Validated on Inkling-NVFP4 / GB300 (vLLM 0.28.0, MTP 8), 2-replica AGG: forced
tool choice honoured 5/5 against a prompt matching the other tool, required = 1
call, parallel_tool_calls true/false = 2/1, and no control-marker residue in
content -- in both streaming and non-streaming. p0 blocking count unchanged at 2
(the unrelated reasoning_budget_zero limitation).
Reported by dynamo-review-agent on ai-dynamo/dynamo#13954.
Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
sungsooha
added a commit
to sungsooha/vllm
that referenced
this pull request
Aug 28, 2026
… tag
The tag permitted only `<|end_message|>`, but the Inkling parser treats
`<|content_model_end_sampling|>` as a co-equal terminator:
* the parser docstring states "sampling may also end a block with the
standalone <|content_model_end_sampling|> token";
* `is_reasoning_end()` returns True on it (`token_id in (text_id,
end_sampling_id)`);
* `count_reasoning_tokens()` closes a reasoning block on either
(`end_ids = {END_MESSAGE, CONTENT_MODEL_END_SAMPLING}`).
Constraining to one terminator therefore masks a valid natural stop and forces
the model to keep generating to reach the permitted token -- and it removes one
of the two paths that flip `is_reasoning_end`, which the reasoning plumbing
keys on.
`TagFormat.end` accepts `list[str]`, so both are permitted directly rather than
via an OrFormat; `AnyTextFormat.excludes` now lists both so the text block
cannot swallow either terminator.
Verified by compiling the tag with xgrammar, forced/required/auto:
<|end_message|> <|content_model_end_sampling|>
before accepted REJECTED
after accepted accepted
The duplicate-role-marker fix from the previous commit is unaffected (a leading
`<|message_model|>` stays rejected).
Reported by dynamo-review-agent on ai-dynamo/dynamo#13954.
Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
…fusers breaks
Two independent CI failures, neither in the shipped code paths.
1. test_engine_core_struct_contract. vLLM 0.28.0 appends `session_id` to
EngineCoreRequest, after `abort_immediately`. The observed tuple differs from
the nearest allowed variant by that one field. Added session_id variants
(plain and vllm-omni), mirroring how the test already handles
abort_immediately and model_intermediate_buffer.
No production change: the test docstring claims vllm_processor.py reads
EngineCoreRequest positionally, but it does not -- it uses attribute access
(vllm_preproc.mm_features) and builds EngineCoreOutput by keyword
(EngineCoreOutput(**output_kwargs)). A trailing append is therefore
compatible, which is what the model_intermediate_buffer comment already says.
2. test_vllm_omni.py errored 4x and test_realtime_omni_bridge failed with "Main
server process exited with code 1", both from:
RuntimeError: Failed to import diffusers.pipelines.pipeline_utils ...
cannot import name 'get_cached_repo_tree' from 'huggingface_hub'
The omni guard caught (ImportError, OSError, NotImplementedError). diffusers
imports its pipelines lazily and re-raises ANY underlying failure as
RuntimeError, so a broken transitive dependency never reaches that tuple and
the module ERRORS instead of skipping. Added RuntimeError.
test_realtime_omni_bridge had no omni guard at all; its worker
(realtime_omni_mock_worker.py) imports vllm_omni and dynamo.vllm.omni at
module scope, so the same failure killed the worker and surfaced as an opaque
server-exit. It now probes those imports and skips.
These tests could not run at all before vllm-omni became installable on this
branch, so they were silently skipping rather than passing. Making omni
installable is what exposed the guard gap.
Measured, not inferred: the base image ships huggingface_hub 1.28.0, which DOES
export get_cached_repo_tree (verified in the wheel), and the build never
installs, moves or downgrades hub. So this is a diffusers-0.40.0-vs-hub-1.28.0
incompatibility, not a pin conflict introduced here -- worth reporting upstream
separately. Skipping restores the behaviour the suite already intended.
Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
sungsooha
temporarily deployed
to
external_collaborator
August 28, 2026 17:43 — with
GitHub Actions
Inactive
The request-side assertion now passes; this is the output-side one behind it. vLLM 0.28.0 appends `mm_cache_miss_hashes` and `new_sampling_mask` after num_nans_in_logits -- after vLLM's own fields, before anything vllm-omni adds. No production change. vllm_processor.py builds EngineCoreOutput BY KEYWORD from four required kwargs and feature-detects optional fields through __struct_fields__, so appended fields take their msgspec defaults. That is not just an argument from the code: this exact path served ~7000 requests on vLLM 0.28.0 during validation (p0 plus a full 3541-request moontrace replay on both 2-replica AGG and 1P1D) with no error, so the defaults are demonstrably acceptable for detokenization. Rather than enumerate another four hand-written tuples, the variants are now generated as a product of (base | cached-token) x (with | without the 0.28.0 extras) x (with | without the omni extras) -- 8 variants from the same three building blocks the test already defined. Verified the CI-observed tuple is accepted and all four previously-valid shapes still are. Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
sungsooha
temporarily deployed
to
external_collaborator
August 28, 2026 18:10 — with
GitHub Actions
Inactive
The previous commit replaced the hand-written omni output tuples with a generated product, leaving omni_output_fields and omni_cached_token_output_fields assigned but unused -- ruff F841, which I pushed without reading the pre-commit result. They are referenced nowhere else; the generated variants cover the same shapes. Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
sungsooha
temporarily deployed
to
external_collaborator
August 28, 2026 18:12 — with
GitHub Actions
Inactive
`pytest.skip(..., allow_module_level=True)` removes the module from
COLLECTION, not just from running. The CI step
`-m "(pre_merge and vllm and gpu_1) and not profiled_vram_gib"` has
exactly one match in the repo -- `test_omni_serve_deployment[omni_audio]`
in tests/serve/test_vllm_omni.py -- so once that module stopped being
collected, the step selected nothing and pytest exited 5 ("no tests ran"),
which the runner reports as a job failure.
The test was already marked `pytest.mark.skip`, so before this it was
collected and reported as skipped and the step exited 0. Guard the probe
import into a flag and skip via `pytest.mark.skipif` instead: same intent,
but the items stay collected and are reported as skipped.
Verified in the runtime container with the omni import blocked, running
that exact marker expression against tests/serve/test_vllm_omni.py:
module-level skip -> "1 skipped", exit 5; skipif -> "1 skipped, 5
deselected", exit 0.
tests/frontend/test_realtime_omni_bridge.py carried the same pattern and
gets the same treatment, so it cannot empty a selection later.
Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
…rocessor path
Two request-rejection paths in the Python chat processor
(`--dyn-chat-processor vllm`) surfaced as HTTP 500 with no usable
explanation. The Rust OpenAIPreprocessor handles both correctly; the
Python path does not go through it.
1. A worker-side rejection -- an unsupported sampling parameter, an
unparseable grammar -- crosses the Rust boundary as a plain exception
whose text is `BackendInvalidArgument: {"message":...,"code":400}`.
The generic `except Exception` turned that into an internal error,
discarding both the status and the reason.
`backend_invalid_argument_to_http_error()` recovers them, mirroring
`extract_backend_error_if_present` in
lib/llm/src/http/service/openai.rs.
2. `logprobs`/`top_logprobs` are not implemented on this path (see the
existing TODO), but the code warned and continued anyway, emitting a
response with an empty logprobs struct that the HTTP layer cannot
deserialize ("invalid length 0, expected struct ChatChoiceLogprobs
with 2 elements"). It now rejects up front.
The rejection is 400 rather than the 501 used for other unimplemented
fields (`previous_response_id`, `background`, `prompt`): those are raised
inside the Rust HTTP service, whereas a status asserted by a Python
engine is triaged by SanitizedError::for_backend_status, which forwards a
4xx verbatim but replaces the body of any 5xx. A 501 raised here reaches
the client as an opaque 500 with the reason lost -- observed, not assumed.
Measured on Inkling-NVFP4 / GB300 (2-replica AGG, vLLM 0.28.0): min_p,
malformed json_schema, logprobs and top_logprobs all go 500 -> 400
carrying the backend's own message; the other seven probe cases are
unchanged.
test_generator_preserves_zero_top_logprobs pinned the warn-and-continue
behaviour, so it is repointed at the rejection and renamed. It still
guards the subtlety it always did: `logprobs=True` with `top_logprobs=0`
projects to 0, so a truthiness guard would let exactly that request
through and re-create the 500 -- the guard tests `is not None`.
Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
(cherry picked from commit 894ee68)
sungsooha
temporarily deployed
to
external_collaborator
August 28, 2026 20:59 — with
GitHub Actions
Inactive
…sagg
On a disaggregated deployment a request the prefill worker had correctly
rejected with 400 reached the client as a 500. The worker's error was
attached to `PrefillError::PrefillError` as a `#[source]` only, and every
path out of the prefill router reaches the frontend through `to_pyerr`,
which formats with `Display` and drops the error chain
(`lib/bindings/python/rust/lib.rs`). The frontend therefore saw the bare
"Prefill router returned error in output" and could not tell an invalid
argument from an engine fault.
Fold the worker's text into the message at the two sites that wrap a real
error, so the `BackendInvalidArgument: {...}` discriminator survives the
Display-only boundary. The HTTP layer and the Python chat processors
already key on it, so no other change is needed for the status to reach
the client. `match_error_chain` is unaffected -- it matches error TYPES
through the chain, never this string -- and the sourceless
"no output (stream ended)" case is left alone.
Also stop yielding untagged error dicts from the vLLM chat processor. The
binding only treats a dict as an `Annotated` envelope when it carries
`_dynamo_annotated` (`depythonize_annotated`); a bare `{"error": {...}}`
is parsed as a completion chunk, fails with "missing field `id`", and the
client gets an opaque 500 in place of the message -- so the reason never
arrives even when the frontend has one. `as_error_envelope()` tags it;
the shared helpers keep their shape, so the SGLang processor and the
existing tests are untouched.
Measured on Inkling-NVFP4 / GB300 1P1D disagg, where min_p and two
malformed json_schema payloads returned 500 3/3 while logprobs (rejected
frontend-side, so path-independent) correctly returned 400.
Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
The internal-failure test pinned the old bare {"error": {...}} yield. That
shape is what the binding cannot deserialize ("missing field `id`"), so
the yield is now a tagged _dynamo_annotated frame. Assert the new invariant
alongside the original intent: a genuine engine fault is still yielded
rather than mapped to a 4xx.
Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
The wrapped form was hand-written; rustfmt joins it, since the single line is exactly at the default max_width of 100. Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
Review follow-up.
Two more error paths yielded a bare `{"error": {...}}` dict: the engine and
postprocessor choice-index checks. They are written inline rather than
through `make_internal_error`, so the first pass over the call sites missed
them, and they fail the same way -- the binding reads the dict as a
completion chunk, fails with "missing field `id`", and the client gets a 500
with no message. Both now go through `as_error_envelope`, so no untagged
error yield is left in this path.
Also add the missing docstring on the internal-failure test, and cut the
comments down. They restated the same mechanism in two files and narrated
the original bug, which belongs in the commit message rather than the
source. The invariant now sits once on `PrefillError::PrefillError`, where a
future caller reads it, and the call site points at it.
Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
sungsooha
temporarily deployed
to
external_collaborator
August 31, 2026 19:19 — with
GitHub Actions
Inactive
Contributor
|
Please rebase on main: statement from description is stale: |
dmitry-tokarev-nv
approved these changes
Aug 31, 2026
dmitry-tokarev-nv
left a comment
Contributor
There was a problem hiding this comment.
I missed that this PR is for ai-dynamo:release/1.5.0-inkling-dev.1 branch and not main
0.28.0 final is now on PyPI, so there is no reason to ship an rc in a release image. Version only -- the pin stays per-device. Per-device is still required: only cuda13.0 is on vLLM 0.28.0, while xpu and cpu remain on 0.27.1, so a single global ref would put omni 0.28.0 into images running the older vLLM. Note that omni declares no `vllm` dependency at all (93 requires, none for vllm), and install_vllm_omni.sh pins the base image's packages, so such a mismatch would install cleanly and only surface at runtime on devices we do not test. The transformers pin stays at 5.14.1: 0.28.0 still declares `transformers>=5.10.1,<5.15`, so the bound has not moved. Signed-off-by: Sungsoo Ha <sungsooh@nvidia.com>
sungsooha
temporarily deployed
to
external_collaborator
August 31, 2026 20:05 — with
GitHub Actions
Inactive
ynpandey-nv
merged commit Aug 31, 2026
9e2a04c
into
ai-dynamo:release/1.5.0-inkling-dev.1
115 checks passed
This branch was previously deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
Adds Inkling vLLM runtime support to
release/1.5.0-inkling-dev.1, following the Nemotron-3 Ultra pattern in #10234, so QA can be handed an official release container instead of our hand-built image.Bumps the vLLM runtime base to v0.28.0 and layers the Inkling structural-tag patch onto it.
Why each piece is here
container/context.yaml— vLLMv0.27.1-ubuntu2404→v0.28.0-ubuntu2404container/deps/vllm/patches/v0.28.0/inkling/0001-inkling-structural-tag.patchcontainer/deps/vllm/validate_inkling_runtime.pypatch's exit codecontainer/templates/vllm_runtime.DockerfileThe bug the patch fixes is silent. With no registered structural tag, Dynamo finds no grammar to install for a tool request and degrades to
tool_choice="auto"— HTTP 200, no warning, and a forced tool choice simply is not honoured. Upstream as vllm-project/vllm#54120.Why the vLLM bump is required
This is a branch-wide change, so it is worth being precise about why it is needed. It is not needed by the structural-tag patch — that patch applies cleanly to 0.27.1 and every symbol it touches is present there. It is needed because the Inkling NVIDIA path, including the MTP support this recipe runs on, moved after 0.27.1:
models/inkling/nvidia/mtp.py—InklingMTPgainsSupportsMultiModalEmbeddings; also reworksv1/spec_decode/llm_base_proposermodels/inkling/nvidia/ops/fa4_rel_attention.py(+400) andattention.py— FA4 warmup migrationmodel.py/moe.py/ops/lamport.py— shared-expert partial addition fused into the Lamport collectiveconfigs.py— HF-config compat for InklingThree Inkling parser fixes land in the same range (#49876, #50528, #51391) — the behaviour the frontend commits on this branch interact with.
Separately: every Inkling number we have — p0, IFEval/GPQA-D, and the moontrace throughput ladder — was measured on 0.28.0. Shipping 0.27.1 would hand QA an engine combination that has never been validated for this model.
xpuandcpuare deliberately left at 0.27.1.Carried commits
This branch also carries two frontend fixes so the container is usable end to end:
main) — merged 30 minutes after this release branch was cut, so it is not in the branch. Carried rather than waiting on a branch refresh.035378f8) — taken via fast-forward to the PR head, so original authorship is preserved and the author's own reconciliation withmaincomes with it. Not yet merged at the time of writing; the head carried here is byte-identical to the approved one.Verification
vLLM patch — generated against the exact
v0.28.0tree and dry-run applied with the Dockerfile's own command (patch --batch --forward -p1), clean on both files. Authoring againstmainand shipping against the tag is a real trap; this is the check for it.Validator — run against the real patched runtime (passes), and confirmed to fail on two negative controls: structural tag unregistered, and vLLM version mismatch. A validator that cannot fail is not a validator.
Template —
render.py --framework vllm --device cudarenders clean and carries the block;--device xpucorrectly does not.Tests — the reasoning-accounting suite passes on this merged tree (9/9). Running it here is what caught that #12541 threads
model_configinto the reasoning-parser constructor, which the test fake did not accept — the cherry-pick auto-merged without conflict, which is not the same as being correct. Remaining suite failures in my local run are allpytest-asyncionot being installed in that environment (42/42 artifacts, 0 real).Reasoning-token fix, measured on Inkling-NVFP4 / GB300 with vLLM 0.28.0 — reported
reasoning_tokensvs an independent tokenization of the returnedreasoning_content, 3 runs each:Before, the tool case reported a constant 7 whatever the real count was (212, 242, 295).
⛔ The image alone is not sufficient
The worker must also run with both:
Without the second,
StructuredOutputManager.should_fill_bitmasknever fills the grammar bitmask for a model with a reasoning parser, so the grammar compiles and is inert — HTTP 200, no error, tool choice unconstrained. That belongs in the recipe, not in this PR.Not included
xpu/cpustay at vLLM 0.27.1 — untested at 0.28.0, and Inkling does not use them.baseline_sbomand thetransformers_version: 5.14.1pin are justified against 0.27.1 and want re-checking at 0.28.0.reasoning_budget_zero(NVBug 6678449a) is a vLLM-side limitation with no patch; unchanged.Draft — opening so CI can build the staging container.