fix(vllm): preserve user-provided --runner flag (rebased #7680) by tzulingk · Pull Request #9710 · ai-dynamo/dynamo

tzulingk · 2026-05-18T19:34:45Z

Overview:

Rebased, CI-passing successor to #7680 by @pecord-ent. The original PR had a black formatting failure that prevented merge; the bug it fixes (#7670) is still present on main despite #7670 being marked as closed via a commit (7856cb2) that turned out to live on a branch which is 1 ahead, 853 behind main — never landed.

Details:

update_engine_config_with_dynamo() in components/src/dynamo/vllm/args.py placed "runner": "generate" in the unconditional defaults dict and the subsequent setattr loop applied it without checking whether the user had set --runner explicitly. Embedding/pooling models that need --runner pooling (e.g. google/embeddinggemma-300m, Qwen/Qwen3-Embedding-0.6B) crashed at startup with:

pydantic_core._pydantic_core.ValidationError: 1 validation error for ModelConfig
  Value error, This model does not support `--runner generate`.

Why remove the override entirely (rather than guard it)

An earlier iteration of this PR kept the override but guarded it with engine_config.runner == "auto" — only defaulting to "generate" when the user hadn't passed anything. Review feedback from @dynamo-ops and @nnshah1 pointed out that this still silently rewrote an explicit --runner auto to generate, making vLLM's actual auto-detection unreachable through Dynamo, and asked whether Dynamo needs to set the default at all.

It doesn't. The Dynamo-forced default predates a working --runner auto. Today vLLM's auto correctly detects the runner from the model's config.json architectures (*ForCausalLM/*LMHeadModel → generate, *Model/*ForSequenceClassification/*EmbeddingModel → pooling, etc.) and falls back to suffix matching otherwise. Three cases now work correctly:

Generation models without --runner → vLLM sees auto, resolves to generate. Same behavior as before for callers who relied on the implicit default.
Embedding models that auto-detect → vLLM sees auto, resolves to pooling. New, correct behavior — was previously impossible to reach through Dynamo.
Embedding models with shared architecture (e.g. an embedding head on a *ForCausalLM base) → user passes --runner pooling explicitly; Dynamo now preserves it (the original update_engine_config_with_dynamo unconditionally overrides --runner, breaking embedding models #7670 bug).

After this PR, update_engine_config_with_dynamo no longer touches the runner field at all — just logs it.

Where should the reviewer start?

components/src/dynamo/vllm/args.py:240-256 — runner removed from the unconditional defaults dict; the conditional block is gone.
components/src/dynamo/vllm/tests/test_vllm_unit.py::TestRunnerPreservation — five cases:
- test_runner_auto_is_preserved — explicit auto stays auto (vLLM autodetects).
- test_runner_pooling_preserved — embedding model case from update_engine_config_with_dynamo unconditionally overrides --runner, breaking embedding models #7670.
- test_runner_generate_explicit_preserved — explicit generate still works.
- test_runner_draft_preserved — draft preserved.
- test_no_runner_attr_skipped_gracefully — older vLLM compat.

Original author @pecord-ent is credited via Co-authored-by: trailer.

Fixes #7670
Refs DYN-3048 / DIS-2091
Supersedes #7680

coderabbitai · 2026-05-18T19:37:54Z

Walkthrough

This PR fixes update_engine_config_with_dynamo() to respect user-provided --runner CLI arguments instead of unconditionally overwriting them with "generate". The implementation now checks if runner is unset (defaulting to "auto") before applying the vLLM 0.13+ default, and preserves explicitly configured runners. Tests validate the fix across embedding, draft, and legacy configuration scenarios.

Changes

Runner Preservation Fix

Layer / File(s)	Summary
Conditional runner default application `components/src/dynamo/vllm/args.py`	Removes unconditional `"runner": "generate"` default from the vLLM defaults mapping. Adds conditional logic that sets `defaults["runner"] = "generate"` only when `engine_config.runner` is `"auto"`, otherwise preserving and logging user-provided `runner` values.
Runner preservation test suite `components/src/dynamo/vllm/tests/test_vllm_unit.py`	Imports `update_engine_config_with_dynamo` and adds test helpers `_make_dynamo_config` and `_make_engine_config_with_runner`. Introduces `TestRunnerPreservation` test suite covering auto→generate conversion, preservation of non-auto runners (pooling, draft), unchanged explicit generate, and missing runner attribute scenarios.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main fix: preserving the user-provided --runner flag with reference to the rebased PR `#7680`.
Linked Issues check	✅ Passed	The PR fully addresses issue `#7670` by moving the runner default out of the unconditional defaults dict and guarding it with an 'auto' check, preserving explicit user values while maintaining backward compatibility [`#7670`].
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the runner flag preservation issue: the conditional logic in args.py and comprehensive test coverage in test_vllm_unit.py for the fix.
Docstring Coverage	✅ Passed	Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.
Description check	✅ Passed	The pull request description comprehensively covers the overview, detailed changes, reviewer guidance, and related issues with clear context and examples.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Adds a new vLLM worker shape: a pooling-mode `AsyncLLM` fronted by a new `EmbeddingWorkerHandler` that handles OpenAI `/v1/embeddings` requests. The Rust frontend's `/v1/embeddings` route and `ModelType::Embedding` plumbing have existed since the SGLang embedding backend shipped; this PR registers a vLLM worker on the same endpoint. Architecture: selection of worker shape happens in `WorkerFactory.create()`, gated on a new `--embedding-worker` flag. Selected first because it crosses worker shapes (pooling AsyncLLM, ModelType.Embedding) rather than being a variant of decode. The embedding worker intentionally skips machinery that doesn't apply to pooling models: - No KV-events publisher: pooling models have no KV cache, nothing to publish. - No forward-pass-metrics relay: relays decode-phase ZMQ metrics; no decode here. - No InstrumentedScheduler: it hard-codes `pooling_params=None` (see instrumented_scheduler.py), which would silently disable the pooling pass. Embedding workers reject `--benchmark-mode` via existing validation paths, so the scheduler is never installed on this path. - No P/D disagg: rejected at parse time — `--embedding-worker` combined with `--disaggregation-mode prefill|decode|encode` raises ValueError. `EmbeddingWorkerHandler` is a standalone class — it does NOT inherit `BaseWorkerHandler`. The base does generation-only init (media loaders, KV-block lookup via `get_dp_range_for_worker`, embedding cache manager) that would either fail or be meaningless on a pooling engine. Embedding inference is a single forward pass with no KV cache, no multimodal data, and no streamed decode, so a separate class is clearer than overriding most of the base's behavior. The handler calls `AsyncLLM.encode(prompt, pooling_params, request_id)` once per input (vLLM v1's API takes a single prompt per call, unlike SGLang's batched `async_encode`), collects the final `PoolingRequestOutput`, converts `outputs.data` (torch.Tensor) to a flat `list[float]`, and yields an OpenAI-shaped response with `usage.prompt_tokens` summed across batched inputs. Customer ask: AWS Amazon Ads (DIS-2091) wants to move their Qwen3-Embedding-0.6B deployment from vanilla-vLLM-on-ECS to Dynamo with their exact existing engine args: --runner pooling --dtype float32 --pooler-config '{"pooling_type": "MEAN", "use_activation": false}' The launch script `examples/backends/vllm/launch/agg_embed.sh` sets those defaults and demonstrates the curl invocation. PR #9710 is a hard prerequisite — without it, `--runner pooling` is silently overwritten to `generate` and the engine crashes at startup. This branch is stacked on top of #9710. Closes DIS-2092 (MVP scope). Follow-ups tracked separately: - DIS-2093: `dimensions` truncation + `encoding_format=base64` - DIS-2094: embedding-specific Prometheus metrics - DIS-2095: vanilla-vLLM-on-L4 baseline Test plan: - Unit tests in test_vllm_unit.py cover flag parsing and validation: default-false, true-with-pooling-runner, rejects-prefill-disagg, rejects-decode-disagg, rejects-multimodal-combo. - GPU integration test in tests/serve/test_vllm.py deferred to follow-up PR. Manual validation via `bash agg_embed.sh` once this lands. Stacks on: #9710 Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

@dynamo-ops

Review feedback on the previous version pointed out two problems with the `runner == "auto"` guard: 1. Explicit `--runner auto` (a valid user-provided value) was still silently overwritten to "generate", making vLLM's auto-detection unreachable through Dynamo. 2. The override predates vLLM having a working `--runner auto` and is no longer necessary at all — vLLM correctly detects generation, pooling, and draft runners from the model config. Remove the override entirely. vLLM's auto mode handles generation models that previously relied on this default, and pooling models that need `--runner pooling` are no longer clobbered (the original #7670 bug). Behavior change for callers who relied on the implicit `auto -> generate` substitution: none. vLLM's auto-detection resolves to the same runner for any generation model. Test updated: `test_runner_defaults_to_generate_when_auto` becomes `test_runner_auto_is_preserved`. The four other cases (pooling, explicit generate, draft, missing-attr) are unchanged. Addresses review comments from @dynamo-ops and @nnshah1 on PR #9710. Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

Adds a new vLLM worker shape: a pooling-mode `AsyncLLM` fronted by a new `EmbeddingWorkerHandler` that handles OpenAI `/v1/embeddings` requests. The Rust frontend's `/v1/embeddings` route and `ModelType::Embedding` plumbing have existed since the SGLang embedding backend shipped; this PR registers a vLLM worker on the same endpoint. Architecture: selection of worker shape happens in `WorkerFactory.create()`, gated on a new `--embedding-worker` flag. Selected first because it crosses worker shapes (pooling AsyncLLM, ModelType.Embedding) rather than being a variant of decode. The embedding worker intentionally skips machinery that doesn't apply to pooling models: - No KV-events publisher: pooling models have no KV cache, nothing to publish. - No forward-pass-metrics relay: relays decode-phase ZMQ metrics; no decode here. - No InstrumentedScheduler: it hard-codes `pooling_params=None` (see instrumented_scheduler.py), which would silently disable the pooling pass. Embedding workers reject `--benchmark-mode` via existing validation paths, so the scheduler is never installed on this path. - No P/D disagg: rejected at parse time — `--embedding-worker` combined with `--disaggregation-mode prefill|decode|encode` raises ValueError. `EmbeddingWorkerHandler` is a standalone class — it does NOT inherit `BaseWorkerHandler`. The base does generation-only init (media loaders, KV-block lookup via `get_dp_range_for_worker`, embedding cache manager) that would either fail or be meaningless on a pooling engine. Embedding inference is a single forward pass with no KV cache, no multimodal data, and no streamed decode, so a separate class is clearer than overriding most of the base's behavior. The handler calls `AsyncLLM.encode(prompt, pooling_params, request_id)` once per input (vLLM v1's API takes a single prompt per call, unlike SGLang's batched `async_encode`), collects the final `PoolingRequestOutput`, converts `outputs.data` (torch.Tensor) to a flat `list[float]`, and yields an OpenAI-shaped response with `usage.prompt_tokens` summed across batched inputs. Customer ask: AWS Amazon Ads (DIS-2091) wants to move their Qwen3-Embedding-0.6B deployment from vanilla-vLLM-on-ECS to Dynamo with their exact existing engine args: --runner pooling --dtype float32 --pooler-config '{"pooling_type": "MEAN", "use_activation": false}' The launch script `examples/backends/vllm/launch/agg_embed.sh` sets those defaults and demonstrates the curl invocation. PR #9710 is a hard prerequisite — without it, `--runner pooling` is silently overwritten to `generate` and the engine crashes at startup. This branch is stacked on top of #9710. Closes DIS-2092 (MVP scope). Follow-ups tracked separately: - DIS-2093: `dimensions` truncation + `encoding_format=base64` - DIS-2094: embedding-specific Prometheus metrics - DIS-2095: vanilla-vLLM-on-L4 baseline Test plan: - Unit tests in test_vllm_unit.py cover flag parsing and validation: default-false, true-with-pooling-runner, rejects-prefill-disagg, rejects-decode-disagg, rejects-multimodal-combo. - GPU integration test in tests/serve/test_vllm.py deferred to follow-up PR. Manual validation via `bash agg_embed.sh` once this lands. Stacks on: #9710 Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

dynamo-ops

Previous review comments have been addressed. Approving.

Adds a new vLLM worker shape that serves OpenAI ``/v1/embeddings`` requests, registered as ``ModelType::Embedding`` against the existing Rust frontend route. Customer ask (DIS-2091): Amazon Ads wants to move their ``Qwen3-Embedding-0.6B`` deployment from vanilla-vLLM on ECS to Dynamo+vLLM, with their existing engine args: --runner pooling --dtype float32 --pooler-config '{"pooling_type": "MEAN", "use_activation": false}' Architecture: a new ``--embedding-worker`` flag in ``backend_args.py`` selects a different worker shape -- pooling-mode ``AsyncLLM`` fronted by a new ``EmbeddingWorkerHandler`` that calls ``AsyncLLM.encode(prompt, pooling_params, request_id)`` and converts the resulting ``PoolingRequestOutput`` to an OpenAI-shaped response. Dispatch happens in ``WorkerFactory.create()`` -- the embedding branch is checked first, before the existing ``disaggregation_mode`` switch, because it crosses worker shapes rather than being a variant of decode. ``EmbeddingWorkerHandler`` is a standalone class, NOT inheriting from ``BaseWorkerHandler``. The base does generation-only init (media loaders, KV-block lookup via ``get_dp_range_for_worker``, embedding cache manager) that would either fail or be meaningless on a pooling engine. A separate class is clearer than overriding most of the base's behavior. What's intentionally skipped on the embedding path: - KV-events publisher (no KV cache, nothing to publish). - Forward-pass-metrics relay (relays decode-phase ZMQ metrics). - ``InstrumentedScheduler`` (hard-codes ``pooling_params=None`` and emits decode-shaped metrics; only installed when ``--benchmark-mode`` is set, which the new validation rejects). - P/D disaggregation (rejected at parse time via ``_validate_embedding_worker_exclusivity``). Engine flags wired into the launch script (``examples/backends/vllm/launch/agg_embed.sh``): - ``--max-model-len 2048``: vLLM's KV-cache pre-check sizes by max_model_len even for pooling models; the model's native 32K would require ~7 GiB of KV reservation against the test fleet's ~559 MiB allocation, so the engine crashes on init. Capped to 2K, well above the customer's 60-80 token ISL; user-overridable via ``MAX_MODEL_LEN``. - ``--no-enable-prefix-caching``: vLLM warns "this model does not officially support prefix caching ... may cause the engine to crash or produce incorrect outputs"; defaults from ``args.py`` enable it, so the launch script disables it explicitly. Handler output shape: ``AsyncLLM`` returns ``PoolingOutput.data`` as ``torch.Tensor`` shape ``(1, hidden_dim)`` (singleton batch dim), not ``(hidden_dim,)``. Naive ``.tolist()`` produces ``[[f32, ...]]`` which the Rust frontend's ``NvCreateEmbeddingResponse::from_annotated_stream`` aggregator rejects (``Embedding.embedding`` is ``Vec<f32>``). The helper flattens with ``.detach().cpu().flatten().tolist()`` so the output is always 1D. Decision recorded on DIS-2092: not adding ``--benchmark-mode embed`` now. The flag's value is in tuning decode workloads with many interacting knobs; embedding workloads are ``(batch_size x ISL -> latency)`` with no KV interactions, so external HTTP load testing (DIS-2095) covers the capacity-planning need. Re-trigger only if the Dynamo planner needs in-process embedding capability curves for auto-scaling. Test coverage: Unit tests (``test_vllm_unit.py::TestEmbeddingWorkerFlag``, 5 cases): - ``test_default_false``: flag defaults to False. - ``test_flag_sets_true``: ``--embedding-worker --runner pooling`` parses cleanly on a cached test model. - ``test_rejects_prefill_disagg``: rejects ``--disaggregation-mode prefill``. - ``test_rejects_decode_disagg``: rejects ``--disaggregation-mode decode``. - ``test_rejects_multimodal_combo``: rejects ``--enable-multimodal``. Worker-factory dispatch test (``test_vllm_worker_factory.py::TestCreate::test_embedding_worker_takes_priority``): asserts ``--embedding-worker`` short-circuits before the ``disaggregation_mode`` dispatch. The existing ``_make_config`` mock also gained ``embedding_worker: False`` default so the other ``TestCreate`` tests don't accidentally hit the new branch via Mock's truthy attribute access. GPU integration test (``tests/serve/test_vllm.py::embedding_agg``): launches the full stack via ``agg_embed.sh`` on ``Qwen3-Embedding-0.6B``, sends three OpenAI ``/v1/embeddings`` payloads (default batched, single-string, three-string list), validates the response shape and embedding dimensions. Validated end-to-end on dynamo-aks-dev with an A100-80GB. Stacks on: #9710 (preserves user-provided ``--runner`` flag). Closes DIS-2092. Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

nnshah1

LGTM - but let's simplify / remove comment

biswapanda

lgtm

dynamo-ops

Previous review comments have been addressed. Approving.

@pecord-ent

…ig_with_dynamo `update_engine_config_with_dynamo()` placed `"runner": "generate"` in the unconditional `defaults` dict, which the subsequent setattr loop applied without checking whether the user had set the value. Any `--runner pooling` (required for embedding models like Qwen3-Embedding-0.6B / google/embeddinggemma-300m) was silently overwritten and the engine crashed with: pydantic_core._pydantic_core.ValidationError: 1 validation error for ModelConfig Value error, This model does not support `--runner generate`. Move the `runner` default out of the unconditional dict and guard it: only set it to `"generate"` when `engine_config.runner == "auto"` (vLLM's default when the user did not pass `--runner`). User-provided values (`pooling`, `draft`, or explicit `generate`) are preserved. Adds `TestRunnerPreservation` with five cases: - auto -> generate default - pooling preserved (embedding model use case from #7670 / DYN-3048) - explicit generate preserved - draft preserved - missing `runner` attr (older vLLM) handled gracefully This is a rebased + black-formatted version of #7680 by @pecord-ent. GitHub issue #7670 was marked as fixed in a commit (7856cb2) that lives on a diverged branch (1 ahead, 853 behind main) and never landed, so the bug is still present on main today. Fixes #7670 Refs DYN-3048 Co-authored-by: pecord-ent <patrick.ecord@ent.ai> Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

After moving "runner" out of the literal `defaults` dict, mypy inferred the remaining literal as `dict[str, bool]` (only False values left). This broke the subsequent assignments of `kv_events_config` (Any | None), `scheduler_cls` (str), and the new conditional `runner` (str) default: args.py:256: error: Incompatible types in assignment (expression has type "str", target has type "bool") [assignment] args.py:261: error: Incompatible types in assignment (expression has type "Any | None", target has type "bool") [assignment] args.py:274: error: Incompatible types in assignment (expression has type "str", target has type "bool") [assignment] args.py:297: error: Incompatible types in assignment (expression has type "str", target has type "bool") [assignment] The dict was always heterogeneous in practice — the prior code worked only because the literal happened to include a string ("runner": "generate") which forced mypy to widen the value type. Make that explicit with an annotation rather than relying on literal inference. `Dict` and `Any` are already imported from `typing` at the top of the file. Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

…ring Removes the "# --- update_engine_config_with_dynamo: --runner preservation tests ---" section divider and rewords the class docstring so it documents the invariant rather than the originating bug report. Per code-review feedback that ticket-tagged dividers / docstrings don't age well — the why is in the commit history. Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

@dynamo-ops

Review feedback on the previous version pointed out two problems with the `runner == "auto"` guard: 1. Explicit `--runner auto` (a valid user-provided value) was still silently overwritten to "generate", making vLLM's auto-detection unreachable through Dynamo. 2. The override predates vLLM having a working `--runner auto` and is no longer necessary at all — vLLM correctly detects generation, pooling, and draft runners from the model config. Remove the override entirely. vLLM's auto mode handles generation models that previously relied on this default, and pooling models that need `--runner pooling` are no longer clobbered (the original #7670 bug). Behavior change for callers who relied on the implicit `auto -> generate` substitution: none. vLLM's auto-detection resolves to the same runner for any generation model. Test updated: `test_runner_defaults_to_generate_when_auto` becomes `test_runner_auto_is_preserved`. The four other cases (pooling, explicit generate, draft, missing-attr) are unchanged. Addresses review comments from @dynamo-ops and @nnshah1 on PR #9710. Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

@nnshah1

@nnshah1 review feedback: the comment in args.py only made sense in the context of the previous version (the one that still set a default). Now that the function doesn't touch runner at all, the comment block reads as dead context for someone arriving fresh. Move the rationale to the PR description instead. The `logger.debug(...)` line is kept — that's a runtime breadcrumb, not a code comment. Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

@dynamo-ops

Review feedback on the previous version pointed out two problems with the `runner == "auto"` guard: 1. Explicit `--runner auto` (a valid user-provided value) was still silently overwritten to "generate", making vLLM's auto-detection unreachable through Dynamo. 2. The override predates vLLM having a working `--runner auto` and is no longer necessary at all — vLLM correctly detects generation, pooling, and draft runners from the model config. Remove the override entirely. vLLM's auto mode handles generation models that previously relied on this default, and pooling models that need `--runner pooling` are no longer clobbered (the original #7670 bug). Behavior change for callers who relied on the implicit `auto -> generate` substitution: none. vLLM's auto-detection resolves to the same runner for any generation model. Test updated: `test_runner_defaults_to_generate_when_auto` becomes `test_runner_auto_is_preserved`. The four other cases (pooling, explicit generate, draft, missing-attr) are unchanged. Addresses review comments from @dynamo-ops and @nnshah1 on PR #9710. Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

Adds a new vLLM worker shape that serves OpenAI ``/v1/embeddings`` requests, registered as ``ModelType::Embedding`` against the existing Rust frontend route. Customer ask (DIS-2091): Amazon Ads wants to move their ``Qwen3-Embedding-0.6B`` deployment from vanilla-vLLM on ECS to Dynamo+vLLM, with their existing engine args: --runner pooling --dtype float32 --pooler-config '{"pooling_type": "MEAN", "use_activation": false}' Architecture: a new ``--embedding-worker`` flag in ``backend_args.py`` selects a different worker shape -- pooling-mode ``AsyncLLM`` fronted by a new ``EmbeddingWorkerHandler`` that calls ``AsyncLLM.encode(prompt, pooling_params, request_id)`` and converts the resulting ``PoolingRequestOutput`` to an OpenAI-shaped response. Dispatch happens in ``WorkerFactory.create()`` -- the embedding branch is checked first, before the existing ``disaggregation_mode`` switch, because it crosses worker shapes rather than being a variant of decode. ``EmbeddingWorkerHandler`` is a standalone class, NOT inheriting from ``BaseWorkerHandler``. The base does generation-only init (media loaders, KV-block lookup via ``get_dp_range_for_worker``, embedding cache manager) that would either fail or be meaningless on a pooling engine. A separate class is clearer than overriding most of the base's behavior. What's intentionally skipped on the embedding path: - KV-events publisher (no KV cache, nothing to publish). - Forward-pass-metrics relay (relays decode-phase ZMQ metrics). - ``InstrumentedScheduler`` (hard-codes ``pooling_params=None`` and emits decode-shaped metrics; only installed when ``--benchmark-mode`` is set, which the new validation rejects). - P/D disaggregation (rejected at parse time via ``_validate_embedding_worker_exclusivity``). Engine flags wired into the launch script (``examples/backends/vllm/launch/agg_embed.sh``): - ``--max-model-len 2048``: vLLM's KV-cache pre-check sizes by max_model_len even for pooling models; the model's native 32K would require ~7 GiB of KV reservation against the test fleet's ~559 MiB allocation, so the engine crashes on init. Capped to 2K, well above the customer's 60-80 token ISL; user-overridable via ``MAX_MODEL_LEN``. - ``--no-enable-prefix-caching``: vLLM warns "this model does not officially support prefix caching ... may cause the engine to crash or produce incorrect outputs"; defaults from ``args.py`` enable it, so the launch script disables it explicitly. Handler output shape: ``AsyncLLM`` returns ``PoolingOutput.data`` as ``torch.Tensor`` shape ``(1, hidden_dim)`` (singleton batch dim), not ``(hidden_dim,)``. Naive ``.tolist()`` produces ``[[f32, ...]]`` which the Rust frontend's ``NvCreateEmbeddingResponse::from_annotated_stream`` aggregator rejects (``Embedding.embedding`` is ``Vec<f32>``). The helper flattens with ``.detach().cpu().flatten().tolist()`` so the output is always 1D. Decision recorded on DIS-2092: not adding ``--benchmark-mode embed`` now. The flag's value is in tuning decode workloads with many interacting knobs; embedding workloads are ``(batch_size x ISL -> latency)`` with no KV interactions, so external HTTP load testing (DIS-2095) covers the capacity-planning need. Re-trigger only if the Dynamo planner needs in-process embedding capability curves for auto-scaling. Test coverage: Unit tests (``test_vllm_unit.py::TestEmbeddingWorkerFlag``, 5 cases): - ``test_default_false``: flag defaults to False. - ``test_flag_sets_true``: ``--embedding-worker --runner pooling`` parses cleanly on a cached test model. - ``test_rejects_prefill_disagg``: rejects ``--disaggregation-mode prefill``. - ``test_rejects_decode_disagg``: rejects ``--disaggregation-mode decode``. - ``test_rejects_multimodal_combo``: rejects ``--enable-multimodal``. Worker-factory dispatch test (``test_vllm_worker_factory.py::TestCreate::test_embedding_worker_takes_priority``): asserts ``--embedding-worker`` short-circuits before the ``disaggregation_mode`` dispatch. The existing ``_make_config`` mock also gained ``embedding_worker: False`` default so the other ``TestCreate`` tests don't accidentally hit the new branch via Mock's truthy attribute access. GPU integration test (``tests/serve/test_vllm.py::embedding_agg``): launches the full stack via ``agg_embed.sh`` on ``Qwen3-Embedding-0.6B``, sends three OpenAI ``/v1/embeddings`` payloads (default batched, single-string, three-string list), validates the response shape and embedding dimensions. Validated end-to-end on dynamo-aks-dev with an A100-80GB. Stacks on: #9710 (preserves user-provided ``--runner`` flag). Closes DIS-2092. Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

…) (ai-dynamo#9710) Signed-off-by: Tzu-Ling <tzulingk@nvidia.com> Co-authored-by: pecord-ent <patrick.ecord@ent.ai>

…) (ai-dynamo#9710) Signed-off-by: Tzu-Ling <tzulingk@nvidia.com> Co-authored-by: pecord-ent <patrick.ecord@ent.ai> Signed-off-by: Michael Feil <63565275+michaelfeil@users.noreply.github.com>

… framing Per repository policy (also flagged on PR #9710 review), source comments shouldn't reference specific deployments or "customer" context. Rewrite the four affected comments in agg_embed.sh to describe the defaults purely in terms of the technical context (typical ISL range, model's native max, pooler config, dtype rationale). Functional contents (model name, max_model_len, pooler args, dtype) are unchanged. Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>

tzulingk requested review from a team as code owners May 18, 2026 19:34

pull-request-size Bot added the size/M label May 18, 2026

github-actions Bot added fix backend::vllm Relates to the vllm backend labels May 18, 2026

tzulingk enabled auto-merge (squash) May 18, 2026 19:44

tzulingk requested review from biswapanda and nnshah1 May 18, 2026 19:46

copy-pr-bot Bot temporarily deployed to GITLAB May 18, 2026 19:47 Inactive

dynamo-ops reviewed May 18, 2026

View reviewed changes

Comment thread components/src/dynamo/vllm/args.py Outdated

nnshah1 reviewed May 18, 2026

View reviewed changes

Comment thread components/src/dynamo/vllm/args.py Outdated

copy-pr-bot Bot temporarily deployed to GITLAB May 18, 2026 20:39 Inactive

tzulingk mentioned this pull request May 18, 2026

feat(vllm): add aggregated text-embedding worker (DIS-2092) #9713

Merged

copy-pr-bot Bot temporarily deployed to GITLAB May 19, 2026 00:10 Inactive

copy-pr-bot Bot temporarily deployed to GITLAB May 19, 2026 00:34 Inactive

tzulingk requested review from dynamo-ops and nnshah1 May 19, 2026 00:49

dynamo-ops approved these changes May 19, 2026

View reviewed changes

copy-pr-bot Bot temporarily deployed to GITLAB May 19, 2026 02:20 Inactive

nnshah1 reviewed May 19, 2026

View reviewed changes

Comment thread components/src/dynamo/vllm/args.py Outdated

nnshah1 approved these changes May 19, 2026

View reviewed changes

copy-pr-bot Bot temporarily deployed to GITLAB May 19, 2026 16:41 Inactive

copy-pr-bot Bot temporarily deployed to GITLAB May 19, 2026 16:43 Inactive

biswapanda approved these changes May 19, 2026

View reviewed changes

dynamo-ops approved these changes May 19, 2026

View reviewed changes

tzulingk and others added 5 commits May 19, 2026 14:44

tzulingk force-pushed the fix/preserve-runner-flag-rebased branch from c7236e5 to df5c99c Compare May 19, 2026 19:44

copy-pr-bot Bot temporarily deployed to GITLAB May 19, 2026 19:44 Inactive

copy-pr-bot Bot had a problem deploying to GITLAB May 19, 2026 20:00 Failure

tzulingk merged commit d904e02 into main May 19, 2026
82 of 83 checks passed

tzulingk deleted the fix/preserve-runner-flag-rebased branch May 19, 2026 20:38

Navigation Menu

Uh oh!

fix(vllm): preserve user-provided --runner flag (rebased #7680) - #9710

fix(vllm): preserve user-provided --runner flag (rebased #7680)#9710
tzulingk merged 5 commits into
mainfrom
fix/preserve-runner-flag-rebased

tzulingk commented May 18, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

dynamo-ops left a comment

Uh oh!

Uh oh!

nnshah1 left a comment

Uh oh!

biswapanda left a comment

Uh oh!

dynamo-ops left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

tzulingk commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview:

Details:

Why remove the override entirely (rather than guard it)

Where should the reviewer start?

Uh oh!

coderabbitai Bot commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

Uh oh!

Uh oh!

dynamo-ops left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nnshah1 left a comment

Choose a reason for hiding this comment

Uh oh!

biswapanda left a comment

Choose a reason for hiding this comment

Uh oh!

dynamo-ops left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tzulingk commented May 18, 2026 •

edited

Loading

coderabbitai Bot commented May 18, 2026 •

edited

Loading