From b0b81d641ada8a1d269ef25b79f171b0a6108b2d Mon Sep 17 00:00:00 2001 From: furionw Date: Tue, 28 Jul 2026 13:38:54 -0700 Subject: [PATCH 1/7] refactor(preprocessor)!: let workers declare their per-image token Kimi-K3's prompt carries one placeholder per image, and the serving engines disagree on which token belongs there: vLLM builds the native media sequence from `<|kimi_image_placeholder|>`, while SGLang only repeats `<|media_pad|>` up to the feature count. `expands_image_pad_token` encoded that as a mode flag naming how SGLang's processor works internally, which the preprocessor then used to pick between two renderer-supplied strings and rewrite the rendered segments after the fact. The flag was only ever a proxy for the string, and the rewrite only existed because rendering ran before the engine's preference was consulted. Carry the string instead. `image_placeholder_token: Option` is read from the processor the worker actually loaded, so it states that engine's real contract rather than an assumption about it, and the formatter renders it directly. `substitute_image_pad_token` and `swap_marker_segments` are gone; so is the notion of pad expansion. Containment is now structural rather than incidental. The token is threaded only into `kimi_k3_formatter_for`; no Jinja-templated family has a parameter to receive one, so SGLang declaring a token for every multimodal model it serves cannot reach Qwen-VL or anything else. The old rewrite ran for every model on every request and was a no-op only because Jinja prompts happen to carry no segments. Workers that declare nothing are unchanged: vLLM keeps rendering the checkpoint default, and `skip_serializing_if` keeps them byte-identical on the wire. Also adds `image_placeholder_token` to the `_core.pyi` stub, which the removed flag never had despite mypy covering the SGLang component. BREAKING CHANGE: `ModelRuntimeConfig.expands_image_pad_token` is replaced by `ModelRuntimeConfig.image_placeholder_token`. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: furionw --- components/src/dynamo/sglang/register.py | 64 +++++++++- .../sglang/tests/test_runtime_metadata.py | 64 ++++++++++ lib/bindings/python/rust/llm/local_model.rs | 8 +- lib/bindings/python/src/dynamo/_core.pyi | 1 + lib/llm/src/local_model/runtime_config.rs | 36 +++--- lib/llm/src/preprocessor.rs | 113 +----------------- lib/llm/src/preprocessor/prompt.rs | 4 + lib/llm/tests/preprocessor.rs | 53 ++++++++ 8 files changed, 201 insertions(+), 142 deletions(-) diff --git a/components/src/dynamo/sglang/register.py b/components/src/dynamo/sglang/register.py index 1268bdaae158..04a4bfd74372 100644 --- a/components/src/dynamo/sglang/register.py +++ b/components/src/dynamo/sglang/register.py @@ -344,6 +344,59 @@ def _eagle_enabled_for(speculative_algorithm: Optional[str]) -> bool: return False +def _get_image_placeholder_token(engine: sgl.Engine) -> Optional[str]: + """Literal token this worker's multimodal processor expects, one per image. + + The frontend renders whatever we report here rather than assuming a token + per engine, so this must come from the processor SGLang actually loaded. + ``TokenizerManager.mm_processor`` is None for text-only models, and each + multimodal processor builds an ``mm_tokens`` spec in its ``__init__`` (the + Kimi ones declare ``<|media_pad|>``). + + Reaches into SGLang internals, so it is defensive in the style of + ``_compat.ensure_sglang_tensor_image_size``: these attributes move between + releases. Any miss returns None, which leaves the frontend on the + formatter's own default. + """ + try: + tokenizer_manager = getattr(engine, "tokenizer_manager", None) + mm_processor = getattr(tokenizer_manager, "mm_processor", None) + if mm_processor is None: + return None # text-only model, or no tokenizer manager + + mm_tokens = getattr(mm_processor, "mm_tokens", None) + if mm_tokens is None: + logging.warning( + "SGLang multimodal processor %s exposes no `mm_tokens`; not " + "declaring an image placeholder token. Multimodal requests on " + "models rendered by a native Dynamo formatter (Kimi-K3) will " + "fail this worker's placeholder validation.", + type(mm_processor).__name__, + ) + return None + + # MultimodalSpecialTokens.image_token is Optional[Union[str, List[str]]]; + # `.build()` backfills the string form from image_token_id, so a plain + # str is the common case. Several spellings can't be reduced to the one + # token the renderer emits, so decline rather than guess. + image_token = getattr(mm_tokens, "image_token", None) + if isinstance(image_token, str) and image_token: + return image_token + if isinstance(image_token, (list, tuple)): + logging.info( + "SGLang processor %s declares multiple image-token spellings " + "(%r); not declaring one to the frontend.", + type(mm_processor).__name__, + list(image_token), + ) + return None + except Exception as e: + logging.warning( + "Could not derive the image placeholder token from SGLang: %s", e + ) + return None + + async def _get_runtime_config( engine: sgl.Engine, server_args: ServerArgs, dynamo_args: DynamoConfig ) -> Optional[ModelRuntimeConfig]: @@ -376,12 +429,11 @@ async def _get_runtime_config( runtime_config.enable_local_indexer = ( dynamo_args.enable_local_indexer and not is_decode_worker ) - # SGLang's multimodal processors expand a single image pad token to the - # image's feature count and never rebuild the model's native media - # sequence, so the frontend must send the pad rather than the placeholder - # marker. Engines that re-derive the sequence from the marker leave this - # false. - runtime_config.expands_image_pad_token = True + # This worker's multimodal processor owns the literal token it expects to + # see once per image; the frontend renders that token directly instead of + # assuming one. None for text-only workers, and inert for models whose + # per-image token is fixed by their Jinja chat template. + runtime_config.image_placeholder_token = _get_image_placeholder_token(engine) start_dp_rank, end_dp_rank = model_card_dp_rank_bounds(server_args) registered_dp_size = end_dp_rank - start_dp_rank diff --git a/components/src/dynamo/sglang/tests/test_runtime_metadata.py b/components/src/dynamo/sglang/tests/test_runtime_metadata.py index 043b3b205c92..b2a3aa26fcdd 100644 --- a/components/src/dynamo/sglang/tests/test_runtime_metadata.py +++ b/components/src/dynamo/sglang/tests/test_runtime_metadata.py @@ -190,3 +190,67 @@ def fail_hicache_publish(self, key, value): assert ( "Failed to attach native offloading capacity from SGLang HiCache" in caplog.text ) + + +# NOTE: import lazily, per the `_eagle_enabled_for` test above -- register.py does +# `from sglang.srt.environ import envs`, absent in the `pytest-marker-report` +# pre-commit hook's collection env. + + +def test_image_placeholder_token_is_none_for_text_only_workers(): + """No mm_processor means no images, so nothing to declare.""" + from dynamo.sglang.register import _get_image_placeholder_token + + engine = SimpleNamespace(tokenizer_manager=SimpleNamespace(mm_processor=None)) + + assert _get_image_placeholder_token(engine) is None + + +def test_image_placeholder_token_comes_from_the_loaded_processor(): + """The declaration must be read from the processor, not hardcoded per engine.""" + from dynamo.sglang.register import _get_image_placeholder_token + + engine = SimpleNamespace( + tokenizer_manager=SimpleNamespace( + mm_processor=SimpleNamespace( + mm_tokens=SimpleNamespace(image_token="<|media_pad|>") + ) + ) + ) + + assert _get_image_placeholder_token(engine) == "<|media_pad|>" + + +def test_image_placeholder_token_declines_multiple_spellings(): + """MultimodalSpecialTokens.image_token may be a list; the renderer emits one + token, so several spellings can't be reduced to a declaration.""" + from dynamo.sglang.register import _get_image_placeholder_token + + engine = SimpleNamespace( + tokenizer_manager=SimpleNamespace( + mm_processor=SimpleNamespace( + mm_tokens=SimpleNamespace(image_token=["<|a|>", "<|b|>"]) + ) + ) + ) + + assert _get_image_placeholder_token(engine) is None + + +def test_image_placeholder_token_warns_when_processor_has_no_mm_tokens(caplog): + """Version drift: a multimodal processor we can't read is worth a breadcrumb, + because K3 requests on this worker will then fail placeholder validation.""" + from dynamo.sglang.register import _get_image_placeholder_token + + engine = SimpleNamespace( + tokenizer_manager=SimpleNamespace(mm_processor=SimpleNamespace()) + ) + + assert _get_image_placeholder_token(engine) is None + assert "exposes no `mm_tokens`" in caplog.text + + +def test_image_placeholder_token_survives_an_engine_without_a_tokenizer_manager(): + from dynamo.sglang.register import _get_image_placeholder_token + + assert _get_image_placeholder_token(SimpleNamespace()) is None diff --git a/lib/bindings/python/rust/llm/local_model.rs b/lib/bindings/python/rust/llm/local_model.rs index cee1679b5b63..3b53dd48ce9b 100644 --- a/lib/bindings/python/rust/llm/local_model.rs +++ b/lib/bindings/python/rust/llm/local_model.rs @@ -158,8 +158,8 @@ impl ModelRuntimeConfig { } #[setter] - fn set_expands_image_pad_token(&mut self, expands_image_pad_token: bool) { - self.inner.expands_image_pad_token = expands_image_pad_token; + fn set_image_placeholder_token(&mut self, image_placeholder_token: Option) { + self.inner.image_placeholder_token = image_placeholder_token; } #[setter] @@ -246,8 +246,8 @@ impl ModelRuntimeConfig { } #[getter] - fn expands_image_pad_token(&self) -> bool { - self.inner.expands_image_pad_token + fn image_placeholder_token(&self) -> Option { + self.inner.image_placeholder_token.clone() } #[getter] diff --git a/lib/bindings/python/src/dynamo/_core.pyi b/lib/bindings/python/src/dynamo/_core.pyi index a6595b6ae081..4bdd637e5870 100644 --- a/lib/bindings/python/src/dynamo/_core.pyi +++ b/lib/bindings/python/src/dynamo/_core.pyi @@ -864,6 +864,7 @@ class ModelRuntimeConfig: data_parallel_start_rank: int data_parallel_size: int enable_local_indexer: bool + image_placeholder_token: str | None kv_state_endpoint: str | None enable_eagle: bool taints: Set[str] diff --git a/lib/llm/src/local_model/runtime_config.rs b/lib/llm/src/local_model/runtime_config.rs index d0d2011ec9f6..7e0d1f958a58 100644 --- a/lib/llm/src/local_model/runtime_config.rs +++ b/lib/llm/src/local_model/runtime_config.rs @@ -171,28 +171,24 @@ pub struct ModelRuntimeConfig { #[serde(default = "default_local_indexer")] pub enable_local_indexer: bool, - /// Whether this engine expands a single image pad token to the image's - /// feature count instead of rebuilding the model's native media sequence. + /// Literal token this worker's multimodal processor expects to see exactly + /// once per image in the prompt it receives. /// - /// Multimodal families whose prompt carries one placeholder marker per - /// image (currently Kimi-K3) need that marker turned into the model's real - /// media sequence before the vision embeddings can bind to it, and engines - /// split on who does it: + /// Read from the processor the worker actually loaded, so it reflects that + /// engine's real contract rather than an assumption about it. Engines + /// disagree: SGLang's processors consume `<|media_pad|>` and repeat it to + /// the feature count, while vLLM consumes the checkpoint's + /// `<|kimi_image_placeholder|>` and builds the native media sequence itself. /// - /// - `false` (default): the engine re-derives the media sequence from the - /// frontend's placeholder marker, reading image dimensions from the - /// multimodal payload. The preprocessor passes the marker through - /// untouched; pre-substituting here would collide with the engine's own - /// expansion. - /// - `true`: the engine only repeats a pad token up to the feature count - /// and never constructs the media sequence, so it needs exactly one pad - /// token per image in the prompt. The preprocessor substitutes the - /// formatter's `image_pad_token()` for the marker. + /// `None` (the default) leaves the renderer on its own per-family default. /// - /// Ignored for formatters that declare no pad token, which is every family - /// except Kimi-K3. - #[serde(default)] - pub expands_image_pad_token: bool, + /// Consumed only by native formatters that emit image placeholders as + /// discrete segments — currently Kimi-K3, via `kimi_k3_formatter_for`. No + /// Jinja-templated family has a way to receive it, so declaring one is + /// inert for every other model. Also not consulted on the `use_raw_prompt` + /// path, which bypasses the formatter entirely. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub image_placeholder_token: Option, /// Endpoint whose event sources describe this worker's KV state. /// @@ -298,7 +294,7 @@ impl Default for ModelRuntimeConfig { exclude_tools_when_tool_choice_none: default_exclude_tools_when_tool_choice_none(), data_parallel_start_rank: default_data_parallel_start_rank(), data_parallel_size: default_data_parallel_size(), - expands_image_pad_token: false, + image_placeholder_token: None, enable_local_indexer: true, kv_state_endpoint: None, runtime_data: HashMap::new(), diff --git a/lib/llm/src/preprocessor.rs b/lib/llm/src/preprocessor.rs index cb8c90278321..88d139e9557a 100644 --- a/lib/llm/src/preprocessor.rs +++ b/lib/llm/src/preprocessor.rs @@ -897,24 +897,13 @@ impl OpenAIPreprocessor { let mut builder = self.builder(request)?; let template_start = Instant::now(); - let mut formatted_prompt = { + let formatted_prompt = { let _nvtx = dynamo_nvtx_range!("preprocess.template"); self.apply_template(request) .with_context(|| "Failed to apply prompt template")? }; TEMPLATE_SECONDS.observe(template_start.elapsed().as_secs_f64()); - // Engines that only repeat a pad token to the image's feature count - // never build the model's native media sequence, so hand them the pad - // instead of the placeholder marker. Engines that rebuild the sequence - // themselves keep the marker (the default) — substituting here would - // leave them nothing to match on. - if self.runtime_config.expands_image_pad_token - && let Some(prompt) = formatted_prompt.as_mut() - { - self.substitute_image_pad_token(prompt); - } - // Generic reasoning parsers start from ``; MiniMax M3 starts // from ``. If the chat template injected that opener at the // end of the prompt, the model completion starts mid-reasoning. @@ -1289,48 +1278,6 @@ impl OpenAIPreprocessor { } } - /// Swap each image-placeholder marker segment for the formatter's pad - /// token, for engines that expand a pad rather than rebuilding the model's - /// media sequence from the marker. - /// - /// A strict no-op for formatters that declare no pad token (every family - /// except Kimi K3) and for requests rendered as raw text, which have no - /// segment boundaries to rewrite. Cardinality is unchanged — one marker - /// becomes one pad — so the engine's own placeholder count still holds. - fn substitute_image_pad_token(&self, prompt: &mut RenderedPrompt) { - let (Some(marker), Some(pad)) = ( - self.formatter.image_placeholder_template(), - self.formatter.image_pad_token(), - ) else { - return; - }; - Self::swap_marker_segments(marker, pad, prompt); - } - - /// Segment-rewriting core of [`Self::substitute_image_pad_token`]. - fn swap_marker_segments(marker: &str, pad: &str, prompt: &mut RenderedPrompt) { - let Some(segments) = prompt.segments() else { - return; // raw_prompt path → no segments to rewrite - }; - if !segments.iter().any(|seg| seg.text == marker) { - return; - } - let rewritten = segments - .iter() - .map(|seg| { - if seg.text == marker { - crate::tokenizers::EncodeSegment { - text: pad.to_string(), - allow_special: true, - } - } else { - seg.clone() - } - }) - .collect(); - *prompt = RenderedPrompt::segmented(rewritten); - } - /// Replace inline `data:` URLs with empty strings in message content parts. /// Preserves HTTP(S) URLs, text content, and overall message structure. fn strip_inline_data_urls(messages: &mut serde_json::Value) { @@ -3945,64 +3892,6 @@ mod tests { use crate::protocols::common::preprocessor::MultimodalData; use crate::protocols::common::{OutputOptions, SamplingOptions, StopConditions}; - const K3_MARKER: &str = "<|kimi_image_placeholder|>"; - const K3_PAD: &str = "<|media_pad|>"; - - fn seg(text: &str, allow_special: bool) -> crate::tokenizers::EncodeSegment { - crate::tokenizers::EncodeSegment { - text: text.to_string(), - allow_special, - } - } - - #[test] - fn swap_marker_segments_replaces_each_marker_with_one_pad() { - let mut prompt = RenderedPrompt::segmented(vec![ - seg("<|open|>message role=\"user\"<|sep|>", true), - seg(K3_MARKER, true), - seg("and", false), - seg(K3_MARKER, true), - seg("compare them", false), - ]); - - OpenAIPreprocessor::swap_marker_segments(K3_MARKER, K3_PAD, &mut prompt); - - let segments = prompt.segments().expect("still segmented"); - let pads = segments.iter().filter(|s| s.text == K3_PAD).count(); - // Cardinality is preserved 1:1 -- the engine expands each pad itself. - assert_eq!(pads, 2); - assert!(!segments.iter().any(|s| s.text == K3_MARKER)); - // The pad must stay a special segment or it tokenizes as literal text. - assert!(segments.iter().all(|s| s.text != K3_PAD || s.allow_special)); - // Surrounding structure and ordinary text are untouched. - assert_eq!(segments[2].text, "and"); - assert!(!segments[2].allow_special); - } - - #[test] - fn swap_marker_segments_is_a_noop_without_markers() { - let original = vec![seg("<|open|>message<|sep|>", true), seg("hello", false)]; - let mut prompt = RenderedPrompt::segmented(original.clone()); - - OpenAIPreprocessor::swap_marker_segments(K3_MARKER, K3_PAD, &mut prompt); - - let segments = prompt.segments().expect("still segmented"); - assert_eq!(segments.len(), original.len()); - assert_eq!(segments[0].text, original[0].text); - assert_eq!(segments[1].text, original[1].text); - } - - #[test] - fn swap_marker_segments_leaves_raw_text_prompts_alone() { - // The raw_prompt path has no segment boundaries, so a marker inside user - // text must never be rewritten into prompt structure. - let mut prompt = RenderedPrompt::text(format!("please describe {K3_MARKER}")); - - OpenAIPreprocessor::swap_marker_segments(K3_MARKER, K3_PAD, &mut prompt); - - assert_eq!(prompt.as_str(), format!("please describe {K3_MARKER}")); - } - #[test] fn prompt_invalid_request_maps_to_invalid_argument() { let error = PromptRenderError::invalid_request("unsupported model parameter").into(); diff --git a/lib/llm/src/preprocessor/prompt.rs b/lib/llm/src/preprocessor/prompt.rs index 97dd6ade10e6..3db601176e03 100644 --- a/lib/llm/src/preprocessor/prompt.rs +++ b/lib/llm/src/preprocessor/prompt.rs @@ -188,10 +188,14 @@ pub fn prompt_formatter_from_mdc(mdc: &ModelDeploymentCard) -> Result) -> String { + let mut mdc = ModelDeploymentCard::load_from_disk(MODEL_PATH, None).unwrap(); + mdc.runtime_config.image_placeholder_token = image_placeholder_token.map(str::to_string); + + let PromptFormatter::OAI(formatter) = prompt_formatter_from_mdc(&mdc).unwrap(); + + let messages: Vec = + serde_json::from_str(r#"[{"role": "user", "content": "What is deep learning?"}]"#) + .unwrap(); + let inner = dynamo_protocols::types::CreateChatCompletionRequestArgs::default() + .model("test-model") + .messages(messages) + .build() + .unwrap(); + let request = NvCreateChatCompletionRequest { + inner, + common: Default::default(), + nvext: None, + chat_template_args: None, + thinking: None, + media_io_kwargs: None, + return_tokens_as_token_ids: None, + unsupported_fields: Default::default(), + }; + + formatter.render(&request).unwrap() + } + + /// The containment guarantee: only native formatters that emit per-image + /// segments take the declared token as a constructor argument, so a + /// declaration made for some other family cannot reach a Jinja model. + #[test] + fn declared_token_does_not_change_a_jinja_rendered_prompt() { + assert_eq!(render_with(None), render_with(Some(QWEN_VL_TOKEN))); + } +} From 9e17b773beb236a7b7e5fc60080ff32548fe83fe Mon Sep 17 00:00:00 2001 From: furionw Date: Tue, 28 Jul 2026 13:56:52 -0700 Subject: [PATCH 2/7] fix(preprocessor): warn when a declared image token is not atomic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the parent commit turned up that `allow_special: true` on a rendered segment only selects tiktoken's special-aware encode method — it is not a membership check. A token the checkpoint never registered falls through to ordinary BPE and silently shatters, so one image becomes several ids and the worker's one-token-per-image expectation breaks with no error on any path. Check the declaration once at preprocessor construction and warn. This lives in `new_with_parts` rather than `new` because the frontend's discovery path builds the formatter and preprocessor separately and never calls `new`. It is diagnostic only: a worker reading its own processor is a better authority than this heuristic, and silently overriding it would be harder to debug than a loud log. Also from review: - Blank declarations now fall back on `trim()`, not just `is_empty()`. Whitespace survived `push_segment`, so it kept cardinality while encoding to a meaningless id. - Split the SGLang accessor's three None branches by log level. The EPD encode worker registers with `engine=None` and can never declare, which is expected; a renamed SGLang attribute is not. Returning None for both made version drift indistinguishable from a text-only worker. The case that breaks multimodal serving is now an error, not a warning. - `_get_image_placeholder_token` and `_get_runtime_config` take `Optional[sgl.Engine]`; both are called with None. - Dropped the claim that `use_raw_prompt` bypasses this. Chat's `raw_prompt()` returns None, so it falls through to the formatter and the token is emitted; it holds only for completions, which carry no images. - Trimmed the field doc, and recorded that only the first-registered worker's declaration takes effect, since `mdcsum` does not cover `runtime_config`. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: furionw --- components/src/dynamo/sglang/register.py | 44 ++++++++++++++++--- .../sglang/tests/test_runtime_metadata.py | 6 ++- lib/llm/src/local_model/runtime_config.rs | 22 ++++------ lib/llm/src/preprocessor.rs | 41 +++++++++++++++++ lib/llm/src/preprocessor/prompt.rs | 3 -- 5 files changed, 90 insertions(+), 26 deletions(-) diff --git a/components/src/dynamo/sglang/register.py b/components/src/dynamo/sglang/register.py index 04a4bfd74372..745fb67d0fb1 100644 --- a/components/src/dynamo/sglang/register.py +++ b/components/src/dynamo/sglang/register.py @@ -344,7 +344,7 @@ def _eagle_enabled_for(speculative_algorithm: Optional[str]) -> bool: return False -def _get_image_placeholder_token(engine: sgl.Engine) -> Optional[str]: +def _get_image_placeholder_token(engine: Optional[sgl.Engine]) -> Optional[str]: """Literal token this worker's multimodal processor expects, one per image. The frontend renders whatever we report here rather than assuming a token @@ -355,18 +355,41 @@ def _get_image_placeholder_token(engine: sgl.Engine) -> Optional[str]: Reaches into SGLang internals, so it is defensive in the style of ``_compat.ensure_sglang_tensor_image_size``: these attributes move between - releases. Any miss returns None, which leaves the frontend on the - formatter's own default. + releases. Every miss returns None, leaving the frontend on the formatter's + own default -- but the misses are logged at different levels, because + "text-only worker" and "SGLang renamed the attribute we read" are + indistinguishable by return value and very distinguishable in consequence. """ try: + # The EPD encode worker registers with engine=None (it owns the chat + # surface but delegates generation), so it can never declare. Expected, + # not drift. + if engine is None: + logging.debug( + "No engine supplied; not declaring an image placeholder token." + ) + return None + tokenizer_manager = getattr(engine, "tokenizer_manager", None) + if tokenizer_manager is None: + logging.warning( + "SGLang engine %s exposes no `tokenizer_manager`; not declaring " + "an image placeholder token.", + type(engine).__name__, + ) + return None + mm_processor = getattr(tokenizer_manager, "mm_processor", None) if mm_processor is None: - return None # text-only model, or no tokenizer manager + logging.debug( + "No multimodal processor on this worker (text-only model); not " + "declaring an image placeholder token." + ) + return None mm_tokens = getattr(mm_processor, "mm_tokens", None) if mm_tokens is None: - logging.warning( + logging.error( "SGLang multimodal processor %s exposes no `mm_tokens`; not " "declaring an image placeholder token. Multimodal requests on " "models rendered by a native Dynamo formatter (Kimi-K3) will " @@ -383,12 +406,19 @@ def _get_image_placeholder_token(engine: sgl.Engine) -> Optional[str]: if isinstance(image_token, str) and image_token: return image_token if isinstance(image_token, (list, tuple)): - logging.info( + logging.error( "SGLang processor %s declares multiple image-token spellings " "(%r); not declaring one to the frontend.", type(mm_processor).__name__, list(image_token), ) + else: + logging.error( + "SGLang processor %s declares no usable `image_token` (%r); not " + "declaring one to the frontend.", + type(mm_processor).__name__, + image_token, + ) return None except Exception as e: logging.warning( @@ -398,7 +428,7 @@ def _get_image_placeholder_token(engine: sgl.Engine) -> Optional[str]: async def _get_runtime_config( - engine: sgl.Engine, server_args: ServerArgs, dynamo_args: DynamoConfig + engine: Optional[sgl.Engine], server_args: ServerArgs, dynamo_args: DynamoConfig ) -> Optional[ModelRuntimeConfig]: """Extract runtime configuration from SGLang engine and args. diff --git a/components/src/dynamo/sglang/tests/test_runtime_metadata.py b/components/src/dynamo/sglang/tests/test_runtime_metadata.py index b2a3aa26fcdd..36c361e8e012 100644 --- a/components/src/dynamo/sglang/tests/test_runtime_metadata.py +++ b/components/src/dynamo/sglang/tests/test_runtime_metadata.py @@ -197,13 +197,15 @@ def fail_hicache_publish(self, key, value): # pre-commit hook's collection env. -def test_image_placeholder_token_is_none_for_text_only_workers(): - """No mm_processor means no images, so nothing to declare.""" +def test_image_placeholder_token_is_none_for_text_only_workers(caplog): + """No mm_processor means no images, so nothing to declare -- and this is + expected, so it must not look like the version-drift case below.""" from dynamo.sglang.register import _get_image_placeholder_token engine = SimpleNamespace(tokenizer_manager=SimpleNamespace(mm_processor=None)) assert _get_image_placeholder_token(engine) is None + assert "exposes no" not in caplog.text def test_image_placeholder_token_comes_from_the_loaded_processor(): diff --git a/lib/llm/src/local_model/runtime_config.rs b/lib/llm/src/local_model/runtime_config.rs index 7e0d1f958a58..0c6940aad47c 100644 --- a/lib/llm/src/local_model/runtime_config.rs +++ b/lib/llm/src/local_model/runtime_config.rs @@ -171,22 +171,16 @@ pub struct ModelRuntimeConfig { #[serde(default = "default_local_indexer")] pub enable_local_indexer: bool, - /// Literal token this worker's multimodal processor expects to see exactly - /// once per image in the prompt it receives. + /// Literal token this worker's multimodal processor expects once per image, + /// read from the processor the worker actually loaded. /// - /// Read from the processor the worker actually loaded, so it reflects that - /// engine's real contract rather than an assumption about it. Engines - /// disagree: SGLang's processors consume `<|media_pad|>` and repeat it to - /// the feature count, while vLLM consumes the checkpoint's - /// `<|kimi_image_placeholder|>` and builds the native media sequence itself. + /// Consumed only by native formatters that render images as discrete + /// segments (currently Kimi-K3, via `kimi_k3_formatter_for`). `None` leaves + /// the renderer on its own default. /// - /// `None` (the default) leaves the renderer on its own per-family default. - /// - /// Consumed only by native formatters that emit image placeholders as - /// discrete segments — currently Kimi-K3, via `kimi_k3_formatter_for`. No - /// Jinja-templated family has a way to receive it, so declaring one is - /// inert for every other model. Also not consulted on the `use_raw_prompt` - /// path, which bypasses the formatter entirely. + /// Only one worker's declaration takes effect per model: `mdcsum` does not + /// cover `runtime_config`, so a model served by workers that disagree gets + /// whichever card registered first. #[serde(default, skip_serializing_if = "Option::is_none")] pub image_placeholder_token: Option, diff --git a/lib/llm/src/preprocessor.rs b/lib/llm/src/preprocessor.rs index 88d139e9557a..60ba32d7136f 100644 --- a/lib/llm/src/preprocessor.rs +++ b/lib/llm/src/preprocessor.rs @@ -636,6 +636,7 @@ impl OpenAIPreprocessor { ) -> Result> { let mdcsum = mdc.mdcsum().to_string(); let tokenizer: Arc = (*tokenizer).clone(); + Self::warn_if_image_placeholder_token_is_not_atomic(&mdc, tokenizer.as_ref()); let lora_name = mdc.lora.as_ref().map(|l| l.name.clone()); let Some(ref model_info) = mdc.model_info else { anyhow::bail!( @@ -1278,6 +1279,46 @@ impl OpenAIPreprocessor { } } + /// Warn when a worker-declared image token does not encode to exactly one + /// token id. + /// + /// The renderer emits the declaration as a segment with `allow_special`, + /// but that only selects tiktoken's special-aware method — it is not a + /// membership check. A token the checkpoint never registered silently + /// BPE-shatters, so one image becomes several ids and the worker's + /// one-token-per-image expectation breaks with no error anywhere. Nothing + /// downstream re-checks this, so say it once at construction. + /// + /// Diagnostic only: the declaration is still honored. A worker that knows + /// its own processor is a better authority than this heuristic, and + /// silently overriding it would be harder to debug than a loud log. + fn warn_if_image_placeholder_token_is_not_atomic( + mdc: &ModelDeploymentCard, + tokenizer: &dyn Tokenizer, + ) { + let Some(token) = mdc.runtime_config.image_placeholder_token.as_deref() else { + return; + }; + match tokenizer.encode(token) { + Ok(encoding) if encoding.token_ids().len() == 1 => {} + Ok(encoding) => tracing::warn!( + image_placeholder_token = %token, + token_ids = ?encoding.token_ids(), + model = %mdc.display_name, + "Worker-declared image placeholder token does not encode to a \ + single token id; each image will expand to {} ids and the \ + worker's per-image count will not match", + encoding.token_ids().len(), + ), + Err(err) => tracing::warn!( + image_placeholder_token = %token, + error = %err, + model = %mdc.display_name, + "Worker-declared image placeholder token failed to encode", + ), + } + } + /// Replace inline `data:` URLs with empty strings in message content parts. /// Preserves HTTP(S) URLs, text content, and overall message structure. fn strip_inline_data_urls(messages: &mut serde_json::Value) { diff --git a/lib/llm/src/preprocessor/prompt.rs b/lib/llm/src/preprocessor/prompt.rs index 3db601176e03..5228e794dd6e 100644 --- a/lib/llm/src/preprocessor/prompt.rs +++ b/lib/llm/src/preprocessor/prompt.rs @@ -188,9 +188,6 @@ pub fn prompt_formatter_from_mdc(mdc: &ModelDeploymentCard) -> Result Date: Tue, 28 Jul 2026 14:09:24 -0700 Subject: [PATCH 3/7] revert(preprocessor): drop the declared-image-token atomicity warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check read `runtime_config.image_placeholder_token` with no gate on whether the formatter consumed it, so it fired on every Qwen3-VL-on-SGLang startup: SGLang declares `<|vision_start|><|image_pad|><|vision_end|>`, that is three token ids, and the warning claimed the per-image count would not match — for a declaration the Jinja path never receives. Inert by design; that is the containment property this series relies on. It was also blind to the case that is actually known-bad: the default `<|kimi_image_placeholder|>` shattering on the vLLM path. vLLM declares nothing, and the default lives inside the formatter, so a check over the declared field cannot see it. Silent where it mattered, noisy where it did not. Checking atomicity needs to key off the token the formatter actually rendered, which needs the read-back accessor deferred with the mm-routing work. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: furionw --- lib/llm/src/preprocessor.rs | 41 ------------------------------------- 1 file changed, 41 deletions(-) diff --git a/lib/llm/src/preprocessor.rs b/lib/llm/src/preprocessor.rs index 60ba32d7136f..88d139e9557a 100644 --- a/lib/llm/src/preprocessor.rs +++ b/lib/llm/src/preprocessor.rs @@ -636,7 +636,6 @@ impl OpenAIPreprocessor { ) -> Result> { let mdcsum = mdc.mdcsum().to_string(); let tokenizer: Arc = (*tokenizer).clone(); - Self::warn_if_image_placeholder_token_is_not_atomic(&mdc, tokenizer.as_ref()); let lora_name = mdc.lora.as_ref().map(|l| l.name.clone()); let Some(ref model_info) = mdc.model_info else { anyhow::bail!( @@ -1279,46 +1278,6 @@ impl OpenAIPreprocessor { } } - /// Warn when a worker-declared image token does not encode to exactly one - /// token id. - /// - /// The renderer emits the declaration as a segment with `allow_special`, - /// but that only selects tiktoken's special-aware method — it is not a - /// membership check. A token the checkpoint never registered silently - /// BPE-shatters, so one image becomes several ids and the worker's - /// one-token-per-image expectation breaks with no error anywhere. Nothing - /// downstream re-checks this, so say it once at construction. - /// - /// Diagnostic only: the declaration is still honored. A worker that knows - /// its own processor is a better authority than this heuristic, and - /// silently overriding it would be harder to debug than a loud log. - fn warn_if_image_placeholder_token_is_not_atomic( - mdc: &ModelDeploymentCard, - tokenizer: &dyn Tokenizer, - ) { - let Some(token) = mdc.runtime_config.image_placeholder_token.as_deref() else { - return; - }; - match tokenizer.encode(token) { - Ok(encoding) if encoding.token_ids().len() == 1 => {} - Ok(encoding) => tracing::warn!( - image_placeholder_token = %token, - token_ids = ?encoding.token_ids(), - model = %mdc.display_name, - "Worker-declared image placeholder token does not encode to a \ - single token id; each image will expand to {} ids and the \ - worker's per-image count will not match", - encoding.token_ids().len(), - ), - Err(err) => tracing::warn!( - image_placeholder_token = %token, - error = %err, - model = %mdc.display_name, - "Worker-declared image placeholder token failed to encode", - ), - } - } - /// Replace inline `data:` URLs with empty strings in message content parts. /// Preserves HTTP(S) URLs, text content, and overall message structure. fn strip_inline_data_urls(messages: &mut serde_json::Value) { From 0a68adbe1906ce66d1113c789ddf53041be67ffd Mon Sep 17 00:00:00 2001 From: furionw Date: Tue, 28 Jul 2026 14:23:12 -0700 Subject: [PATCH 4/7] fix(sglang): keep the image-token probe quiet for models without images The probe runs during registration for every SGLang model, so anything it says has to be true for all of them. It wasn't: five audio-only processors (qwen_audio, voxtral, glmasr, midashenglm, qwen3_asr) populate audio_token and leave image_token unset, and the previous version logged an error for each of them at startup. Having no image token is not a fault; it means the worker does not serve images. The mistake was letting the worker judge whether the absence mattered. It cannot: it has no idea whether the frontend will pick a native formatter for this model. Only the frontend knows, and it already logs which token it resolved and whether a worker declared one. So the probe now just reports what it finds and returns None otherwise. On the blast radius, since this is unconditional: the whole path is two to four reads of plain instance attributes -- `mm_processor` is assigned directly in `init_tokenizer_and_processor`, not a property -- wrapped in `except Exception`. There is no I/O, no side effect, and nothing that can fail registration for a model that doesn't need it. Gating the call on the model being K3 would buy nothing and would put the model knowledge this series removed back into the worker. Collapses the six single-shape tests into one parametrised case covering every "no image token" shape, asserting nothing at WARNING or above. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: furionw --- components/src/dynamo/sglang/register.py | 83 ++++++----------- .../sglang/tests/test_runtime_metadata.py | 90 ++++++++++--------- 2 files changed, 77 insertions(+), 96 deletions(-) diff --git a/components/src/dynamo/sglang/register.py b/components/src/dynamo/sglang/register.py index 745fb67d0fb1..c52fe73cdf41 100644 --- a/components/src/dynamo/sglang/register.py +++ b/components/src/dynamo/sglang/register.py @@ -353,72 +353,45 @@ def _get_image_placeholder_token(engine: Optional[sgl.Engine]) -> Optional[str]: multimodal processor builds an ``mm_tokens`` spec in its ``__init__`` (the Kimi ones declare ``<|media_pad|>``). - Reaches into SGLang internals, so it is defensive in the style of - ``_compat.ensure_sglang_tensor_image_size``: these attributes move between - releases. Every miss returns None, leaving the frontend on the formatter's - own default -- but the misses are logged at different levels, because - "text-only worker" and "SGLang renamed the attribute we read" are - indistinguishable by return value and very distinguishable in consequence. + Runs for every model, so it must be total and quiet. "No image token" is a + normal answer, not a fault: text-only models have no ``mm_processor`` at + all, audio-only processors (``qwen_audio``, ``voxtral``, ``glmasr``, ...) + populate ``audio_token`` and leave ``image_token`` unset, and the EPD encode + worker registers with ``engine=None``. None of those are worth a warning. + + This deliberately does not judge whether the absence matters. Only the + frontend knows that -- it is the side that picks a native formatter, and it + logs which token it resolved and whether a worker declared it. + + Reaches into SGLang internals (all plain instance attributes, no properties), + so it is defensive in the style of ``_compat.ensure_sglang_tensor_image_size``: + these move between releases. Any miss returns None, leaving the frontend on + the formatter's own default. """ try: - # The EPD encode worker registers with engine=None (it owns the chat - # surface but delegates generation), so it can never declare. Expected, - # not drift. - if engine is None: - logging.debug( - "No engine supplied; not declaring an image placeholder token." - ) - return None - - tokenizer_manager = getattr(engine, "tokenizer_manager", None) - if tokenizer_manager is None: - logging.warning( - "SGLang engine %s exposes no `tokenizer_manager`; not declaring " - "an image placeholder token.", - type(engine).__name__, - ) - return None - - mm_processor = getattr(tokenizer_manager, "mm_processor", None) + mm_processor = getattr( + getattr(engine, "tokenizer_manager", None), "mm_processor", None + ) if mm_processor is None: - logging.debug( - "No multimodal processor on this worker (text-only model); not " - "declaring an image placeholder token." - ) - return None - - mm_tokens = getattr(mm_processor, "mm_tokens", None) - if mm_tokens is None: - logging.error( - "SGLang multimodal processor %s exposes no `mm_tokens`; not " - "declaring an image placeholder token. Multimodal requests on " - "models rendered by a native Dynamo formatter (Kimi-K3) will " - "fail this worker's placeholder validation.", - type(mm_processor).__name__, - ) + # engine=None (EPD encode worker), text-only model, or an SGLang + # whose attribute names moved. return None # MultimodalSpecialTokens.image_token is Optional[Union[str, List[str]]]; # `.build()` backfills the string form from image_token_id, so a plain # str is the common case. Several spellings can't be reduced to the one # token the renderer emits, so decline rather than guess. - image_token = getattr(mm_tokens, "image_token", None) + image_token = getattr( + getattr(mm_processor, "mm_tokens", None), "image_token", None + ) if isinstance(image_token, str) and image_token: return image_token - if isinstance(image_token, (list, tuple)): - logging.error( - "SGLang processor %s declares multiple image-token spellings " - "(%r); not declaring one to the frontend.", - type(mm_processor).__name__, - list(image_token), - ) - else: - logging.error( - "SGLang processor %s declares no usable `image_token` (%r); not " - "declaring one to the frontend.", - type(mm_processor).__name__, - image_token, - ) + logging.debug( + "SGLang processor %s declares no single image token (%r); not " + "declaring one to the frontend.", + type(mm_processor).__name__, + image_token, + ) return None except Exception as e: logging.warning( diff --git a/components/src/dynamo/sglang/tests/test_runtime_metadata.py b/components/src/dynamo/sglang/tests/test_runtime_metadata.py index 36c361e8e012..adffabc7a75a 100644 --- a/components/src/dynamo/sglang/tests/test_runtime_metadata.py +++ b/components/src/dynamo/sglang/tests/test_runtime_metadata.py @@ -1,6 +1,7 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 +import logging from types import SimpleNamespace import pytest @@ -197,15 +198,57 @@ def fail_hicache_publish(self, key, value): # pre-commit hook's collection env. -def test_image_placeholder_token_is_none_for_text_only_workers(caplog): - """No mm_processor means no images, so nothing to declare -- and this is - expected, so it must not look like the version-drift case below.""" - from dynamo.sglang.register import _get_image_placeholder_token +@pytest.mark.parametrize( + "engine", + [ + # Text-only model: no multimodal processor at all. + SimpleNamespace(tokenizer_manager=SimpleNamespace(mm_processor=None)), + # EPD encode worker: registers with engine=None. + None, + # No tokenizer manager, or an SGLang whose attribute names moved. + SimpleNamespace(), + # Audio-only processor (qwen_audio, voxtral, glmasr, ...): populates + # audio_token and leaves image_token unset. + SimpleNamespace( + tokenizer_manager=SimpleNamespace( + mm_processor=SimpleNamespace( + mm_tokens=SimpleNamespace(image_token=None, audio_token="<|audio|>") + ) + ) + ), + # Multimodal processor we can't read (version drift). + SimpleNamespace( + tokenizer_manager=SimpleNamespace(mm_processor=SimpleNamespace()) + ), + # Several spellings can't be reduced to the one token the renderer emits. + SimpleNamespace( + tokenizer_manager=SimpleNamespace( + mm_processor=SimpleNamespace( + mm_tokens=SimpleNamespace(image_token=["<|a|>", "<|b|>"]) + ) + ) + ), + ], + ids=[ + "text-only", + "no-engine", + "no-tokenizer-manager", + "audio-only", + "drift", + "multiple-spellings", + ], +) +def test_absent_image_token_is_quiet(engine, caplog): + """Every "no image token" shape returns None without shouting. - engine = SimpleNamespace(tokenizer_manager=SimpleNamespace(mm_processor=None)) + This runs for every SGLang model, so a spurious warning here would fire on + unrelated deployments -- audio-only workers in particular are perfectly + healthy and simply have no image token to declare. + """ + from dynamo.sglang.register import _get_image_placeholder_token assert _get_image_placeholder_token(engine) is None - assert "exposes no" not in caplog.text + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] def test_image_placeholder_token_comes_from_the_loaded_processor(): @@ -221,38 +264,3 @@ def test_image_placeholder_token_comes_from_the_loaded_processor(): ) assert _get_image_placeholder_token(engine) == "<|media_pad|>" - - -def test_image_placeholder_token_declines_multiple_spellings(): - """MultimodalSpecialTokens.image_token may be a list; the renderer emits one - token, so several spellings can't be reduced to a declaration.""" - from dynamo.sglang.register import _get_image_placeholder_token - - engine = SimpleNamespace( - tokenizer_manager=SimpleNamespace( - mm_processor=SimpleNamespace( - mm_tokens=SimpleNamespace(image_token=["<|a|>", "<|b|>"]) - ) - ) - ) - - assert _get_image_placeholder_token(engine) is None - - -def test_image_placeholder_token_warns_when_processor_has_no_mm_tokens(caplog): - """Version drift: a multimodal processor we can't read is worth a breadcrumb, - because K3 requests on this worker will then fail placeholder validation.""" - from dynamo.sglang.register import _get_image_placeholder_token - - engine = SimpleNamespace( - tokenizer_manager=SimpleNamespace(mm_processor=SimpleNamespace()) - ) - - assert _get_image_placeholder_token(engine) is None - assert "exposes no `mm_tokens`" in caplog.text - - -def test_image_placeholder_token_survives_an_engine_without_a_tokenizer_manager(): - from dynamo.sglang.register import _get_image_placeholder_token - - assert _get_image_placeholder_token(SimpleNamespace()) is None From cd92ad2343a97a5470c838053aad22d4c0d150d2 Mon Sep 17 00:00:00 2001 From: furionw Date: Tue, 28 Jul 2026 14:26:47 -0700 Subject: [PATCH 5/7] fix(sglang): only read the image token for models that need it The probe ran during registration for every SGLang model, to serve one. Kimi-K3 is the only family Dynamo renders with a native Rust formatter that has to choose a per-image token; everything else carries its per-image token in its own chat template, so the frontend never reads a declaration for it. Gate on `model_type` (`config.json` says `kimi_k3`, the same string `is_kimi_k3` matches on in the renderer, so worker scope and formatter selection cannot drift). The scope is also what makes reading `mm_tokens.image_token` sound in the first place. That field is "the string this processor scans for", and its shape differs per model: one token for Kimi (`<|media_pad|>`), a three-token wrapper for Qwen-VL (`<|vision_start|><|image_pad|><|vision_end|>`), a text pattern for MiniCPM, indirected attributes for InternVL/Phi4MM/Mllama4. Only for Kimi does it coincide with "the one token to emit per image". Reading it for everyone meant interpreting a field we understand for exactly one family and publishing the result into every multimodal worker's card, where nothing consumes it. The spurious error on audio-only processors was the first symptom of that, not an unlucky edge case. Scoping also drops the coupling this had to SGLang's `mm_tokens` layout for models that never needed it, which `components/src/dynamo/sglang/CLAUDE.md` warns against: SGLang is pre-1.0 and moves internal APIs between releases. With the probe scoped, a miss becomes actionable again -- the engine is known to be one that will not consume the formatter's default -- so an unreadable K3 processor warns rather than staying quiet. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: furionw --- components/src/dynamo/sglang/register.py | 70 +++++++++------ .../sglang/tests/test_runtime_metadata.py | 88 +++++++++---------- 2 files changed, 88 insertions(+), 70 deletions(-) diff --git a/components/src/dynamo/sglang/register.py b/components/src/dynamo/sglang/register.py index c52fe73cdf41..634921f26625 100644 --- a/components/src/dynamo/sglang/register.py +++ b/components/src/dynamo/sglang/register.py @@ -37,6 +37,12 @@ SGLANG_HICACHE_MOONCAKE_RUNTIME_KEY = "sglang_hicache_mooncake" SPEC_DECODE_RUNTIME_KEY = "spec_decode" +# `config.json` model_types whose per-image token the frontend has to be told, +# because Dynamo renders them with a native Rust formatter instead of a chat +# template. Everything else carries its per-image token in its own template and +# needs no declaration -- see `_get_image_placeholder_token`. +_NATIVE_IMAGE_TOKEN_MODEL_TYPES = frozenset({"kimi_k3"}) + def _register_model_source_path(engine: sgl.Engine, server_args: ServerArgs) -> str: """Pick the path passed to `register_model` for MDC construction. @@ -353,43 +359,55 @@ def _get_image_placeholder_token(engine: Optional[sgl.Engine]) -> Optional[str]: multimodal processor builds an ``mm_tokens`` spec in its ``__init__`` (the Kimi ones declare ``<|media_pad|>``). - Runs for every model, so it must be total and quiet. "No image token" is a - normal answer, not a fault: text-only models have no ``mm_processor`` at - all, audio-only processors (``qwen_audio``, ``voxtral``, ``glmasr``, ...) - populate ``audio_token`` and leave ``image_token`` unset, and the EPD encode - worker registers with ``engine=None``. None of those are worth a warning. - - This deliberately does not judge whether the absence matters. Only the - frontend knows that -- it is the side that picks a native formatter, and it - logs which token it resolved and whether a worker declared it. - - Reaches into SGLang internals (all plain instance attributes, no properties), - so it is defensive in the style of ``_compat.ensure_sglang_tensor_image_size``: - these move between releases. Any miss returns None, leaving the frontend on - the formatter's own default. + Scoped to Kimi-K3 on purpose. It is the only family Dynamo renders with a + native Rust formatter that has to choose a per-image token; every other + model gets its per-image token from its own chat template, so the frontend + never reads a declaration for them. + + The scope is also what makes reading ``mm_tokens.image_token`` sound. + That field is "the string this processor scans for", and its shape differs + per model -- one token for Kimi (``<|media_pad|>``), a three-token wrapper + for Qwen-VL (``<|vision_start|><|image_pad|><|vision_end|>``), a text + pattern for MiniCPM. Only for Kimi does it coincide with "the one token to + emit per image", so only for Kimi do we read it. + + Reaches into SGLang internals (plain instance attributes, no properties), + which are pre-1.0 and move between releases -- so every miss returns None + and leaves the frontend on the formatter's own default. """ try: - mm_processor = getattr( - getattr(engine, "tokenizer_manager", None), "mm_processor", None + tokenizer_manager = getattr(engine, "tokenizer_manager", None) + if tokenizer_manager is None: + return None # engine=None (EPD encode worker), or renamed upstream + + model_type = getattr( + getattr( + getattr(tokenizer_manager, "model_config", None), "hf_config", None + ), + "model_type", + None, ) - if mm_processor is None: - # engine=None (EPD encode worker), text-only model, or an SGLang - # whose attribute names moved. + if model_type not in _NATIVE_IMAGE_TOKEN_MODEL_TYPES: return None # MultimodalSpecialTokens.image_token is Optional[Union[str, List[str]]]; # `.build()` backfills the string form from image_token_id, so a plain - # str is the common case. Several spellings can't be reduced to the one - # token the renderer emits, so decline rather than guess. + # str is the common case. image_token = getattr( - getattr(mm_processor, "mm_tokens", None), "image_token", None + getattr( + getattr(tokenizer_manager, "mm_processor", None), "mm_tokens", None + ), + "image_token", + None, ) if isinstance(image_token, str) and image_token: return image_token - logging.debug( - "SGLang processor %s declares no single image token (%r); not " - "declaring one to the frontend.", - type(mm_processor).__name__, + + logging.warning( + "Could not read an image placeholder token from the SGLang processor " + "for model_type=%r (got %r); the frontend will fall back to its " + "default, which this engine does not consume.", + model_type, image_token, ) return None diff --git a/components/src/dynamo/sglang/tests/test_runtime_metadata.py b/components/src/dynamo/sglang/tests/test_runtime_metadata.py index adffabc7a75a..d04e2dc1ce55 100644 --- a/components/src/dynamo/sglang/tests/test_runtime_metadata.py +++ b/components/src/dynamo/sglang/tests/test_runtime_metadata.py @@ -198,52 +198,46 @@ def fail_hicache_publish(self, key, value): # pre-commit hook's collection env. +def _engine(model_type=None, mm_tokens=..., with_tm=True): + """Minimal stand-in for the attribute chain the probe walks.""" + if not with_tm: + return SimpleNamespace() + mm_processor = None if mm_tokens is ... else SimpleNamespace(mm_tokens=mm_tokens) + return SimpleNamespace( + tokenizer_manager=SimpleNamespace( + model_config=SimpleNamespace( + hf_config=SimpleNamespace(model_type=model_type) + ), + mm_processor=mm_processor, + ) + ) + + @pytest.mark.parametrize( "engine", [ - # Text-only model: no multimodal processor at all. - SimpleNamespace(tokenizer_manager=SimpleNamespace(mm_processor=None)), - # EPD encode worker: registers with engine=None. None, - # No tokenizer manager, or an SGLang whose attribute names moved. - SimpleNamespace(), - # Audio-only processor (qwen_audio, voxtral, glmasr, ...): populates - # audio_token and leaves image_token unset. - SimpleNamespace( - tokenizer_manager=SimpleNamespace( - mm_processor=SimpleNamespace( - mm_tokens=SimpleNamespace(image_token=None, audio_token="<|audio|>") - ) - ) - ), - # Multimodal processor we can't read (version drift). - SimpleNamespace( - tokenizer_manager=SimpleNamespace(mm_processor=SimpleNamespace()) + _engine(with_tm=False), + _engine( + model_type="qwen3_vl", + mm_tokens=SimpleNamespace( + image_token="<|vision_start|><|image_pad|><|vision_end|>" + ), ), - # Several spellings can't be reduced to the one token the renderer emits. - SimpleNamespace( - tokenizer_manager=SimpleNamespace( - mm_processor=SimpleNamespace( - mm_tokens=SimpleNamespace(image_token=["<|a|>", "<|b|>"]) - ) - ) + _engine( + model_type="qwen2_audio", + mm_tokens=SimpleNamespace(image_token=None, audio_token="<|audio|>"), ), + _engine(model_type="llama"), ], - ids=[ - "text-only", - "no-engine", - "no-tokenizer-manager", - "audio-only", - "drift", - "multiple-spellings", - ], + ids=["no-engine", "no-tokenizer-manager", "qwen-vl", "audio-only", "text-only"], ) -def test_absent_image_token_is_quiet(engine, caplog): - """Every "no image token" shape returns None without shouting. +def test_no_declaration_and_no_probe_for_non_k3_models(engine, caplog): + """The frontend only reads this for Kimi-K3, so nothing else is probed. - This runs for every SGLang model, so a spurious warning here would fire on - unrelated deployments -- audio-only workers in particular are perfectly - healthy and simply have no image token to declare. + Qwen-VL is the case that matters: its processor DOES expose an image_token, + but it is a three-token wrapper meaning something different, and its chat + template already emits it. Reading it would publish a value nobody consumes. """ from dynamo.sglang.register import _get_image_placeholder_token @@ -252,15 +246,21 @@ def test_absent_image_token_is_quiet(engine, caplog): def test_image_placeholder_token_comes_from_the_loaded_processor(): - """The declaration must be read from the processor, not hardcoded per engine.""" + """For K3 the declaration is read from the processor, never hardcoded.""" from dynamo.sglang.register import _get_image_placeholder_token - engine = SimpleNamespace( - tokenizer_manager=SimpleNamespace( - mm_processor=SimpleNamespace( - mm_tokens=SimpleNamespace(image_token="<|media_pad|>") - ) - ) + engine = _engine( + model_type="kimi_k3", mm_tokens=SimpleNamespace(image_token="<|media_pad|>") ) assert _get_image_placeholder_token(engine) == "<|media_pad|>" + + +def test_unreadable_k3_processor_warns(): + """Now that the probe only runs for K3, a miss IS actionable -- the engine + will not consume the formatter's default, so say so.""" + from dynamo.sglang.register import _get_image_placeholder_token + + engine = _engine(model_type="kimi_k3", mm_tokens=SimpleNamespace(image_token=None)) + + assert _get_image_placeholder_token(engine) is None From 94fac66a7809132f151b00087a7407c679ebaca1 Mon Sep 17 00:00:00 2001 From: furionw Date: Wed, 29 Jul 2026 12:13:20 -0700 Subject: [PATCH 6/7] refactor(vllm)!: convert the K3 pad worker-side instead of declaring it Adopts the review proposal on #12281 and drops this branch's earlier `image_placeholder_token` runtime-config field. The frontend now emits one canonical `<|media_pad|>` per image for every engine (ai-dynamo/frontend-crates#155). SGLang consumes that directly. vLLM's K3 processor instead matches the checkpoint's `<|kimi_image_placeholder|>` and expands it into the media sequence, so the conversion happens in the vLLM adapter's `build_tokens_prompt()`, gated on raw media being present. Converting in this direction is what makes it dependable: `<|media_pad|>` is a single vocabulary id (`config.json`'s `media_placeholder_token_id`), so locating it is exact. `<|kimi_image_placeholder|>` is a plain string that is NOT in the vocabulary -- it BPE-shatters into several ids whose boundaries move with surrounding text -- so a frontend emitting it would leave the worker nothing reliable to anchor on. vLLM's own processor concedes this, falling back to decode/string-replace/re-encode when the token match fails. Doing it here rather than through discovery keeps it off the public Python bindings surface, where it would be sticky to remove, and leaves room for video/audio to need the same treatment without a field each. Both values come from checkpoint metadata, so no registration field and no SGLang processor introspection are needed -- the probe added earlier on this branch is gone entirely. Mismatched pad counts are rejected rather than guessed: silently misaligning images against embedding slots is worse than a hard error. An already-native prompt has no pads and passes through untouched, which is the rollout path -- deploy this adapter first, then switch the frontend to emit pads. BREAKING CHANGE: `ModelRuntimeConfig.expands_image_pad_token` is removed with no replacement field. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: furionw --- components/src/dynamo/sglang/register.py | 81 +---------------- .../sglang/tests/test_runtime_metadata.py | 74 ---------------- .../multimodal_utils/request_processor.py | 87 ++++++++++++++++++- .../test_vllm_request_processor.py | 79 +++++++++++++++++ lib/bindings/python/rust/llm/local_model.rs | 10 --- lib/bindings/python/src/dynamo/_core.pyi | 1 - lib/llm/src/local_model/runtime_config.rs | 14 --- lib/llm/src/preprocessor/prompt.rs | 1 - lib/llm/tests/preprocessor.rs | 53 ----------- 9 files changed, 166 insertions(+), 234 deletions(-) diff --git a/components/src/dynamo/sglang/register.py b/components/src/dynamo/sglang/register.py index 634921f26625..60ac61a11e21 100644 --- a/components/src/dynamo/sglang/register.py +++ b/components/src/dynamo/sglang/register.py @@ -37,12 +37,6 @@ SGLANG_HICACHE_MOONCAKE_RUNTIME_KEY = "sglang_hicache_mooncake" SPEC_DECODE_RUNTIME_KEY = "spec_decode" -# `config.json` model_types whose per-image token the frontend has to be told, -# because Dynamo renders them with a native Rust formatter instead of a chat -# template. Everything else carries its per-image token in its own template and -# needs no declaration -- see `_get_image_placeholder_token`. -_NATIVE_IMAGE_TOKEN_MODEL_TYPES = frozenset({"kimi_k3"}) - def _register_model_source_path(engine: sgl.Engine, server_args: ServerArgs) -> str: """Pick the path passed to `register_model` for MDC construction. @@ -350,76 +344,8 @@ def _eagle_enabled_for(speculative_algorithm: Optional[str]) -> bool: return False -def _get_image_placeholder_token(engine: Optional[sgl.Engine]) -> Optional[str]: - """Literal token this worker's multimodal processor expects, one per image. - - The frontend renders whatever we report here rather than assuming a token - per engine, so this must come from the processor SGLang actually loaded. - ``TokenizerManager.mm_processor`` is None for text-only models, and each - multimodal processor builds an ``mm_tokens`` spec in its ``__init__`` (the - Kimi ones declare ``<|media_pad|>``). - - Scoped to Kimi-K3 on purpose. It is the only family Dynamo renders with a - native Rust formatter that has to choose a per-image token; every other - model gets its per-image token from its own chat template, so the frontend - never reads a declaration for them. - - The scope is also what makes reading ``mm_tokens.image_token`` sound. - That field is "the string this processor scans for", and its shape differs - per model -- one token for Kimi (``<|media_pad|>``), a three-token wrapper - for Qwen-VL (``<|vision_start|><|image_pad|><|vision_end|>``), a text - pattern for MiniCPM. Only for Kimi does it coincide with "the one token to - emit per image", so only for Kimi do we read it. - - Reaches into SGLang internals (plain instance attributes, no properties), - which are pre-1.0 and move between releases -- so every miss returns None - and leaves the frontend on the formatter's own default. - """ - try: - tokenizer_manager = getattr(engine, "tokenizer_manager", None) - if tokenizer_manager is None: - return None # engine=None (EPD encode worker), or renamed upstream - - model_type = getattr( - getattr( - getattr(tokenizer_manager, "model_config", None), "hf_config", None - ), - "model_type", - None, - ) - if model_type not in _NATIVE_IMAGE_TOKEN_MODEL_TYPES: - return None - - # MultimodalSpecialTokens.image_token is Optional[Union[str, List[str]]]; - # `.build()` backfills the string form from image_token_id, so a plain - # str is the common case. - image_token = getattr( - getattr( - getattr(tokenizer_manager, "mm_processor", None), "mm_tokens", None - ), - "image_token", - None, - ) - if isinstance(image_token, str) and image_token: - return image_token - - logging.warning( - "Could not read an image placeholder token from the SGLang processor " - "for model_type=%r (got %r); the frontend will fall back to its " - "default, which this engine does not consume.", - model_type, - image_token, - ) - return None - except Exception as e: - logging.warning( - "Could not derive the image placeholder token from SGLang: %s", e - ) - return None - - async def _get_runtime_config( - engine: Optional[sgl.Engine], server_args: ServerArgs, dynamo_args: DynamoConfig + engine: sgl.Engine, server_args: ServerArgs, dynamo_args: DynamoConfig ) -> Optional[ModelRuntimeConfig]: """Extract runtime configuration from SGLang engine and args. @@ -450,11 +376,6 @@ async def _get_runtime_config( runtime_config.enable_local_indexer = ( dynamo_args.enable_local_indexer and not is_decode_worker ) - # This worker's multimodal processor owns the literal token it expects to - # see once per image; the frontend renders that token directly instead of - # assuming one. None for text-only workers, and inert for models whose - # per-image token is fixed by their Jinja chat template. - runtime_config.image_placeholder_token = _get_image_placeholder_token(engine) start_dp_rank, end_dp_rank = model_card_dp_rank_bounds(server_args) registered_dp_size = end_dp_rank - start_dp_rank diff --git a/components/src/dynamo/sglang/tests/test_runtime_metadata.py b/components/src/dynamo/sglang/tests/test_runtime_metadata.py index d04e2dc1ce55..043b3b205c92 100644 --- a/components/src/dynamo/sglang/tests/test_runtime_metadata.py +++ b/components/src/dynamo/sglang/tests/test_runtime_metadata.py @@ -1,7 +1,6 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -import logging from types import SimpleNamespace import pytest @@ -191,76 +190,3 @@ def fail_hicache_publish(self, key, value): assert ( "Failed to attach native offloading capacity from SGLang HiCache" in caplog.text ) - - -# NOTE: import lazily, per the `_eagle_enabled_for` test above -- register.py does -# `from sglang.srt.environ import envs`, absent in the `pytest-marker-report` -# pre-commit hook's collection env. - - -def _engine(model_type=None, mm_tokens=..., with_tm=True): - """Minimal stand-in for the attribute chain the probe walks.""" - if not with_tm: - return SimpleNamespace() - mm_processor = None if mm_tokens is ... else SimpleNamespace(mm_tokens=mm_tokens) - return SimpleNamespace( - tokenizer_manager=SimpleNamespace( - model_config=SimpleNamespace( - hf_config=SimpleNamespace(model_type=model_type) - ), - mm_processor=mm_processor, - ) - ) - - -@pytest.mark.parametrize( - "engine", - [ - None, - _engine(with_tm=False), - _engine( - model_type="qwen3_vl", - mm_tokens=SimpleNamespace( - image_token="<|vision_start|><|image_pad|><|vision_end|>" - ), - ), - _engine( - model_type="qwen2_audio", - mm_tokens=SimpleNamespace(image_token=None, audio_token="<|audio|>"), - ), - _engine(model_type="llama"), - ], - ids=["no-engine", "no-tokenizer-manager", "qwen-vl", "audio-only", "text-only"], -) -def test_no_declaration_and_no_probe_for_non_k3_models(engine, caplog): - """The frontend only reads this for Kimi-K3, so nothing else is probed. - - Qwen-VL is the case that matters: its processor DOES expose an image_token, - but it is a three-token wrapper meaning something different, and its chat - template already emits it. Reading it would publish a value nobody consumes. - """ - from dynamo.sglang.register import _get_image_placeholder_token - - assert _get_image_placeholder_token(engine) is None - assert not [r for r in caplog.records if r.levelno >= logging.WARNING] - - -def test_image_placeholder_token_comes_from_the_loaded_processor(): - """For K3 the declaration is read from the processor, never hardcoded.""" - from dynamo.sglang.register import _get_image_placeholder_token - - engine = _engine( - model_type="kimi_k3", mm_tokens=SimpleNamespace(image_token="<|media_pad|>") - ) - - assert _get_image_placeholder_token(engine) == "<|media_pad|>" - - -def test_unreadable_k3_processor_warns(): - """Now that the probe only runs for K3, a miss IS actionable -- the engine - will not consume the formatter's default, so say so.""" - from dynamo.sglang.register import _get_image_placeholder_token - - engine = _engine(model_type="kimi_k3", mm_tokens=SimpleNamespace(image_token=None)) - - assert _get_image_placeholder_token(engine) is None diff --git a/components/src/dynamo/vllm/multimodal_utils/request_processor.py b/components/src/dynamo/vllm/multimodal_utils/request_processor.py index 8bf8bacd7420..2ddfb1cd51a7 100644 --- a/components/src/dynamo/vllm/multimodal_utils/request_processor.py +++ b/components/src/dynamo/vllm/multimodal_utils/request_processor.py @@ -255,6 +255,89 @@ def __init__( ) self.use_unified_vision_chunk = use_unified_vision_chunk + def _kimi_k3_pad_expansion(self) -> Optional[tuple[int, list[int]]]: + """``(pad_id, native_ids)`` for Kimi-K3, or None for every other model. + + The frontend emits one ``<|media_pad|>`` per image for every engine. + vLLM's K3 processor instead matches the checkpoint's + ``<|kimi_image_placeholder|>`` and expands *that* into + ``<|media_begin|>image WxH<|media_content|>...<|media_end|>``, so the + two forms are converted here rather than diverging in the renderer. + + Converting in this direction is what makes the contract reliable: + ``<|media_pad|>`` is a single vocabulary id, so locating it is exact. + ``<|kimi_image_placeholder|>`` is a plain string that is not in the + vocabulary -- its token boundaries shift with surrounding text -- so a + frontend emitting it would leave the worker nothing dependable to find. + + Both values come from checkpoint metadata, so no registration field or + engine introspection is required. + """ + if getattr(self, "_k3_expansion_resolved", False): + return self._k3_expansion + self._k3_expansion_resolved = True + self._k3_expansion = None + try: + model_config = getattr( + getattr(self.engine_client, "vllm_config", None), "model_config", None + ) + hf_config = getattr(model_config, "hf_config", None) + if getattr(hf_config, "model_type", None) != "kimi_k3": + return None + pad_id = hf_config.media_placeholder_token_id + tokenizer = self.engine_client.get_tokenizer() + native_ids = list( + tokenizer.encode(hf_config.image_placeholder, add_special_tokens=False) + ) + if not isinstance(pad_id, int) or not native_ids: + logger.warning( + "Kimi-K3 placeholder metadata unusable (pad_id=%r, native_ids=%r); " + "leaving prompt token ids untouched", + pad_id, + native_ids, + ) + return None + self._k3_expansion = (pad_id, native_ids) + except Exception as e: + logger.warning("Could not resolve the Kimi-K3 placeholder mapping: %s", e) + return self._k3_expansion + + def _expand_kimi_k3_pads( + self, token_ids: list[int], multi_modal_data: Optional[dict[str, Any]] + ) -> list[int]: + """Replace each structural pad id with the checkpoint-native sequence. + + Raw-media path only -- inputs that already carry processed multimodal + state are left alone. During rollout a prompt may already be in the + native form, in which case there are no pads and this is a no-op. + """ + expansion = self._kimi_k3_pad_expansion() + if expansion is None or not multi_modal_data: + return token_ids + pad_id, native_ids = expansion + + pad_count = token_ids.count(pad_id) + if pad_count == 0: + return token_ids # already native form + + images = multi_modal_data.get("image") + expected = len(images) if isinstance(images, (list, tuple)) else 1 + if pad_count != expected: + # Guessing here would silently misalign images against their + # embedding slots, so refuse instead. + raise ValueError( + f"Kimi-K3 prompt carries {pad_count} <|media_pad|> token(s) but " + f"{expected} image(s) were supplied; refusing to expand." + ) + + expanded: list[int] = [] + for token_id in token_ids: + if token_id == pad_id: + expanded.extend(native_ids) + else: + expanded.append(token_id) + return expanded + @staticmethod def _multimodal_disabled_error() -> ValueError: return ValueError( @@ -562,7 +645,9 @@ def build_tokens_prompt( ) prompt_kwargs: dict[str, Any] = { - "prompt_token_ids": request["token_ids"], + "prompt_token_ids": self._expand_kimi_k3_pads( + request["token_ids"], multi_modal_data + ), "multi_modal_data": multi_modal_data, } if mm_uuids is not None: diff --git a/components/src/dynamo/vllm/tests/multimodal_utils/test_vllm_request_processor.py b/components/src/dynamo/vllm/tests/multimodal_utils/test_vllm_request_processor.py index 635ef83f635b..1fe2ce62d232 100644 --- a/components/src/dynamo/vllm/tests/multimodal_utils/test_vllm_request_processor.py +++ b/components/src/dynamo/vllm/tests/multimodal_utils/test_vllm_request_processor.py @@ -819,3 +819,82 @@ def test_qwen_handoff_accepts_encoder_embeddings(): "image_grid_thw": [[1, 16, 16]], "embeddings_shape": [1, 256, 1024], } + + +# --- Kimi-K3 structural-pad -> checkpoint-native expansion ------------------- +# +# The frontend emits one <|media_pad|> per image for every engine. vLLM's K3 +# processor matches the checkpoint's <|kimi_image_placeholder|> instead, so the +# worker converts. See VllmMultimodalRequestProcessor._expand_kimi_k3_pads. + +_K3_PAD_ID = 163605 +_K3_NATIVE_IDS = [27, 91, 74, 30223, 11947, 114136, 91, 29] + + +def _k3_processor(model_type: str = "kimi_k3") -> mod.VllmMultimodalRequestProcessor: + engine_client = SimpleNamespace( + vllm_config=SimpleNamespace( + model_config=SimpleNamespace( + hf_config=SimpleNamespace( + model_type=model_type, + media_placeholder_token_id=_K3_PAD_ID, + image_placeholder="<|kimi_image_placeholder|>", + use_unified_vision_chunk=False, + ) + ) + ), + get_tokenizer=lambda: SimpleNamespace( + encode=lambda text, add_special_tokens=False: list(_K3_NATIVE_IDS) + ), + ) + return mod.VllmMultimodalRequestProcessor( + model="moonshot-ai/Kimi-K3", + engine_client=engine_client, + enable_multimodal=True, + ) + + +def test_k3_pad_expands_to_the_checkpoint_native_sequence(): + proc = _k3_processor() + out = proc._expand_kimi_k3_pads([1, _K3_PAD_ID, 2], {"image": ["img"]}) + + assert out == [1, *_K3_NATIVE_IDS, 2] + + +def test_k3_expansion_is_one_per_image(): + proc = _k3_processor() + out = proc._expand_kimi_k3_pads([_K3_PAD_ID, 7, _K3_PAD_ID], {"image": ["a", "b"]}) + + assert out.count(_K3_PAD_ID) == 0 + assert out == [*_K3_NATIVE_IDS, 7, *_K3_NATIVE_IDS] + + +def test_k3_mismatched_pad_count_is_rejected_not_guessed(): + """Misaligning images against embedding slots is worse than failing.""" + proc = _k3_processor() + with pytest.raises(ValueError, match="refusing to expand"): + proc._expand_kimi_k3_pads([_K3_PAD_ID], {"image": ["a", "b"]}) + + +def test_k3_already_native_prompt_is_untouched(): + """Rollout compatibility: a frontend still emitting the native form has no + pads to replace, so it passes through.""" + proc = _k3_processor() + native = [1, *_K3_NATIVE_IDS, 2] + + assert proc._expand_kimi_k3_pads(native, {"image": ["img"]}) == native + + +def test_non_k3_models_are_never_rewritten(): + proc = _k3_processor(model_type="qwen3_vl") + tokens = [1, _K3_PAD_ID, 2] + + assert proc._expand_kimi_k3_pads(tokens, {"image": ["img"]}) == tokens + + +def test_k3_without_raw_media_is_untouched(): + """Processed-input paths must not be rewritten.""" + proc = _k3_processor() + tokens = [1, _K3_PAD_ID, 2] + + assert proc._expand_kimi_k3_pads(tokens, None) == tokens diff --git a/lib/bindings/python/rust/llm/local_model.rs b/lib/bindings/python/rust/llm/local_model.rs index 3b53dd48ce9b..751d1d2c8e91 100644 --- a/lib/bindings/python/rust/llm/local_model.rs +++ b/lib/bindings/python/rust/llm/local_model.rs @@ -157,11 +157,6 @@ impl ModelRuntimeConfig { self.inner.enable_local_indexer = enable_local_indexer; } - #[setter] - fn set_image_placeholder_token(&mut self, image_placeholder_token: Option) { - self.inner.image_placeholder_token = image_placeholder_token; - } - #[setter] fn set_kv_state_endpoint(&mut self, kv_state_endpoint: Option) { self.inner.kv_state_endpoint = kv_state_endpoint.as_deref().map(EndpointId::from); @@ -245,11 +240,6 @@ impl ModelRuntimeConfig { self.inner.enable_local_indexer } - #[getter] - fn image_placeholder_token(&self) -> Option { - self.inner.image_placeholder_token.clone() - } - #[getter] fn kv_state_endpoint(&self) -> Option { self.inner diff --git a/lib/bindings/python/src/dynamo/_core.pyi b/lib/bindings/python/src/dynamo/_core.pyi index 4bdd637e5870..a6595b6ae081 100644 --- a/lib/bindings/python/src/dynamo/_core.pyi +++ b/lib/bindings/python/src/dynamo/_core.pyi @@ -864,7 +864,6 @@ class ModelRuntimeConfig: data_parallel_start_rank: int data_parallel_size: int enable_local_indexer: bool - image_placeholder_token: str | None kv_state_endpoint: str | None enable_eagle: bool taints: Set[str] diff --git a/lib/llm/src/local_model/runtime_config.rs b/lib/llm/src/local_model/runtime_config.rs index 0c6940aad47c..57ed1b7afed7 100644 --- a/lib/llm/src/local_model/runtime_config.rs +++ b/lib/llm/src/local_model/runtime_config.rs @@ -171,19 +171,6 @@ pub struct ModelRuntimeConfig { #[serde(default = "default_local_indexer")] pub enable_local_indexer: bool, - /// Literal token this worker's multimodal processor expects once per image, - /// read from the processor the worker actually loaded. - /// - /// Consumed only by native formatters that render images as discrete - /// segments (currently Kimi-K3, via `kimi_k3_formatter_for`). `None` leaves - /// the renderer on its own default. - /// - /// Only one worker's declaration takes effect per model: `mdcsum` does not - /// cover `runtime_config`, so a model served by workers that disagree gets - /// whichever card registered first. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub image_placeholder_token: Option, - /// Endpoint whose event sources describe this worker's KV state. /// /// When unset, consumers use the worker's serving endpoint. This keeps existing @@ -288,7 +275,6 @@ impl Default for ModelRuntimeConfig { exclude_tools_when_tool_choice_none: default_exclude_tools_when_tool_choice_none(), data_parallel_start_rank: default_data_parallel_start_rank(), data_parallel_size: default_data_parallel_size(), - image_placeholder_token: None, enable_local_indexer: true, kv_state_endpoint: None, runtime_data: HashMap::new(), diff --git a/lib/llm/src/preprocessor/prompt.rs b/lib/llm/src/preprocessor/prompt.rs index 5228e794dd6e..97dd6ade10e6 100644 --- a/lib/llm/src/preprocessor/prompt.rs +++ b/lib/llm/src/preprocessor/prompt.rs @@ -192,7 +192,6 @@ pub fn prompt_formatter_from_mdc(mdc: &ModelDeploymentCard) -> Result) -> String { - let mut mdc = ModelDeploymentCard::load_from_disk(MODEL_PATH, None).unwrap(); - mdc.runtime_config.image_placeholder_token = image_placeholder_token.map(str::to_string); - - let PromptFormatter::OAI(formatter) = prompt_formatter_from_mdc(&mdc).unwrap(); - - let messages: Vec = - serde_json::from_str(r#"[{"role": "user", "content": "What is deep learning?"}]"#) - .unwrap(); - let inner = dynamo_protocols::types::CreateChatCompletionRequestArgs::default() - .model("test-model") - .messages(messages) - .build() - .unwrap(); - let request = NvCreateChatCompletionRequest { - inner, - common: Default::default(), - nvext: None, - chat_template_args: None, - thinking: None, - media_io_kwargs: None, - return_tokens_as_token_ids: None, - unsupported_fields: Default::default(), - }; - - formatter.render(&request).unwrap() - } - - /// The containment guarantee: only native formatters that emit per-image - /// segments take the declared token as a constructor argument, so a - /// declaration made for some other family cannot reach a Jinja model. - #[test] - fn declared_token_does_not_change_a_jinja_rendered_prompt() { - assert_eq!(render_with(None), render_with(Some(QWEN_VL_TOKEN))); - } -} From d369e476664cdbf1054cd13979caae96fcb1ea47 Mon Sep 17 00:00:00 2001 From: furionw Date: Wed, 29 Jul 2026 12:38:35 -0700 Subject: [PATCH 7/7] perf(vllm): splice around K3 pads instead of walking every token Prompts on this path reach 100k+ tokens while pads number in the single digits, so the per-token Python loop was the wrong shape: it paid O(n) interpreter overhead to relocate a handful of ids. Locate the pads with `list.index` and splice with slices instead. Both are C-level, and `index` short-circuits at the first hit, so an already-native prompt (the rollout path, and every non-K3 request that gets this far) costs one scan and zero copies rather than a full traversal plus a `count()` pre-pass. Measured on a 131072-token prompt with one image: 2.0ms -> 0.9ms. The no-pad case is unchanged at ~0.44ms, since both spellings do a single C-level scan. Differentially checked against the previous implementation over 309 cases including empty, leading, trailing and adjacent pads. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: furionw --- .../multimodal_utils/request_processor.py | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/components/src/dynamo/vllm/multimodal_utils/request_processor.py b/components/src/dynamo/vllm/multimodal_utils/request_processor.py index 2ddfb1cd51a7..233097f357e8 100644 --- a/components/src/dynamo/vllm/multimodal_utils/request_processor.py +++ b/components/src/dynamo/vllm/multimodal_utils/request_processor.py @@ -316,26 +316,41 @@ def _expand_kimi_k3_pads( return token_ids pad_id, native_ids = expansion - pad_count = token_ids.count(pad_id) - if pad_count == 0: - return token_ids # already native form + # Prompts here reach 100k+ tokens while pads number in the single + # digits, so locate the rare token and splice around it rather than + # walking every id in Python. `list.index` short-circuits at the first + # pad, so the no-pad case (already-native prompt) costs one C-level + # scan and zero copies. + try: + first = token_ids.index(pad_id) + except ValueError: + return token_ids # already in native form -- nothing to replace + + pad_positions = [first] + while True: + try: + pad_positions.append(token_ids.index(pad_id, pad_positions[-1] + 1)) + except ValueError: + break images = multi_modal_data.get("image") expected = len(images) if isinstance(images, (list, tuple)) else 1 - if pad_count != expected: + if len(pad_positions) != expected: # Guessing here would silently misalign images against their # embedding slots, so refuse instead. raise ValueError( - f"Kimi-K3 prompt carries {pad_count} <|media_pad|> token(s) but " - f"{expected} image(s) were supplied; refusing to expand." + f"Kimi-K3 prompt carries {len(pad_positions)} <|media_pad|> " + f"token(s) but {expected} image(s) were supplied; refusing to " + f"expand." ) expanded: list[int] = [] - for token_id in token_ids: - if token_id == pad_id: - expanded.extend(native_ids) - else: - expanded.append(token_id) + prev = 0 + for position in pad_positions: + expanded.extend(token_ids[prev:position]) + expanded.extend(native_ids) + prev = position + 1 + expanded.extend(token_ids[prev:]) return expanded @staticmethod