Skip to content

Keep message images on pretokenized chat requests - #5

Open
celve wants to merge 48 commits into
sglang-milesfrom
LIN-2043/add-image-support-to-the-miles
Open

celve wants to merge 48 commits into
sglang-milesfrom
LIN-2043/add-image-support-to-the-miles

Conversation

@celve

@celve celve commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

What this does

A chat request that carries input_ids skipped template rendering, and it also dropped every image in its messages (image_data=None in _process_messages). The engine then generated from bare placeholder tokens. Now the pretokenized branch collects the messages' media in message order and attaches it, as the token-first encoders (inkling, kimi_k3) already do. The ids must hold one placeholder per media item, in that order, and the multimodal processor expands them in place exactly as on /generate.

This is the SGLang half of image support in the Miles TITO session server, celve/miles#3: image turns go through the chat route like text turns, so agents need no adapter.

  • _collect_messages_media is the media loop the inkling/kimi_k3 branch already ran, now shared with the pretokenized branch.
  • A regression test sends a pretokenized request with images in two messages. It checks that both reach GenerateReqInput in message order, with the caller's ids unchanged.

Verification

  • This exact commit, live: installed into its own environment the way docker/Dockerfile does (sglang-kernel 0.4.9, the package with its Rust extensions against the cu130 torch index, torch 2.14.1 and transformers 5.19.0, flashinfer-cubin 0.7.1); it reports 0.0.0.dev1+gb3b1b2e58. It served Qwen3-VL-2B-Instruct behind sglang_router. Miles's session server sent three pretokenized image turns through the chat route. On each, the engine's returned prompt_token_ids equalled the session server's own expansion of the placeholders, so the images were attached and expanded in place: 19/19 checks on the resulting training sample.
  • The whole test_serving_chat.py on this exact commit: 157 passed with 113 subtests (run by the reviewer).
  • The regression test, before and after: on a devbox's 4076a68 build it fails without the patch (TypeError: 'NoneType' object is not iterable, since image_data is None) and passes with it; that file goes from 148 passed and 1 failed to 149 passed.

Reviewed together with the Miles PR by a second agent (sol), in two rounds ending in a pass with no open findings.

Kangyan-Zhou and others added 30 commits October 13, 2026 23:15
…om the PD prefill server to release/v0.5.22 (sgl-project#44002) (sgl-project#44212)

Co-authored-by: William Arnold <7565007+Aphoh@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: ishandhanani <82981111+ishandhanani@users.noreply.github.com>
…arse+MTP B300 test to release/v0.5.22 (sgl-project#44192) (sgl-project#44214)

Co-authored-by: Mohammad Miadh Angkad <176301910+mmangkad@users.noreply.github.com>
Co-authored-by: Mohammad Angkad <mohammad.angkad@radixark.ai>
…lication (sgl-project#27268, sgl-project#31759, sgl-project#30913, sgl-project#30912, sgl-project#33299, sgl-project#36384, sgl-project#38165)

Register adapter identities, stream their tensors within weight-update sessions, and publish checksum-verified fresh versions alongside generation. Preserve per-rank failure cleanup and request-prefix cancellation for adapter lifecycle management.

Co-authored-by: Mathew Han <mathewjhan@gmail.com>
Co-authored-by: Ethan (Yusheng) Su <yushengsu.thu@gmail.com>
Co-authored-by: Yueming Yuan <yym022502@gmail.com>
…ism (sgl-project#21278, sgl-project#22663, sgl-project#33478)

Expose rank topology for P2P weight mapping and create replicas using scoped runtime-context overrides. Restore the previous configuration on exit, including failed context entry.

Co-authored-by: yueming-yuan <yym022502@gmail.com>
…RL weight checks (sgl-project#29339)

Co-authored-by: yueming-yuan <yym022502@gmail.com>
…30499, sgl-project#37466, sgl-project#37887)

Co-authored-by: fzyzcjy <ch271828n@outlook.com>
Co-authored-by: yueming-yuan <yym022502@gmail.com>
…weight checks (sgl-project#37649)

Co-authored-by: Zhichenzzz <zczeng@uw.edu>
…e MXFP4 reload (sgl-project#37704, sgl-project#41074, sgl-project#41925)

Co-authored-by: Xinyu Jiang <xinyuj2@andrew.cmu.edu>
Co-authored-by: Zhiyao Jiang <jessicajiang324@gmail.com>
…load; restore bf16 MoE layout (sgl-project#40821)

Co-authored-by: Yueming Yuan <yym022502@gmail.com>
Rockdu and others added 18 commits October 15, 2026 16:16
…PC LoRA updates (sgl-project#40786)

Keep H3 rollout and streamed-weight behavior on the release implementation. The release already contains the LoRA wrapper type guards.

Co-authored-by: WenhaoZhang <42087078+niehen6174@users.noreply.github.com>
Co-authored-by: niehen6174 <niehen6174@gmail.com>
… graph registration (sgl-project#40648, sgl-project#40978, sgl-project#40979, sgl-project#40980, sgl-project#40977, sgl-project#41972)

Retain colocated training fixes above the release MiMo implementation, including address-stable Marlin reload and mandatory TopK layer IDs. Disable NCCL graph registration for TP LM-head all-to-all and pausable graph pools.
…y from metadata (sgl-project#39265)

Co-authored-by: yueming-yuan <yym022502@gmail.com>
…] fix: report the port each scheduler actually bound to its clients (sgl-project#42470)

Signed-off-by: rockdu <kangrdu@gmail.com>
… updates (sgl-project#39392)

Signed-off-by: Bo Li <22713281+bobboli@users.noreply.github.com>
…V budget, UE8M0 partial blocks, mxfp8 fused indexer, NVFP4 input scales (sgl-project#43251)
`DeepseekV4DecoderLayer.bind_next` (run from `post_load_weights`) assigned the
next decoder layer to `self._next_layer`, which `nn.Module.__setattr__`
registers as a submodule. After the first load, `named_parameters()` dedups
layers 1.. under `model.layers.0._next_layer[._next_layer...]`, so every later
`load_weights` call (update_weights_from_tensor / _disk / _distributed) misses
`model.layers.{i}.*` and crashes with a KeyError. Only models with
`hc_pre_from_prev_sublayer` (V4.1) bind a next layer.

Hold the next layer as a plain attribute instead.
Since sgl-project#42589 the GPT-OSS and Inkling weight loaders read moe_tp_rank,
moe_tp_size, moe_ep_rank and moe_ep_size off `self.experts`. With LoRA
attached that module is FusedMoEWithLoRA, which only kept them as tp_rank /
tp_size, so the first GPT-OSS LoRA weight load raised
AttributeError: 'FusedMoEWithLoRA' object has no attribute 'moe_tp_rank'.
The wrapper now forwards all four, like the other FusedMoE attributes the
model reads on it.

The unit test's base-layer stand-in also gains named_parameters, which
BaseLayerWithLoRA.__init__ has read since the MoE LoRA support commit; its
two existing cases failed without it.
… kernel

The legacy CUDA kernel sizes its grid from the element count, so an empty
input launches zero blocks. The launch is unchecked and leaves
cudaErrorInvalidValue pending until the next kernel launch on the thread.

The attention TP group now selects a custom communicator (sgl-project#35330), so with
SGLANG_OPT_USE_CUSTOM_ALL_REDUCE_V2=0 an idle DP-attention rank all-reduces
the zero-row embedding output through this path, and its next kernel fails
with "CUDA error: invalid argument". An empty all-reduce exchanges nothing,
so return the empty output directly.
A chat request that carries input_ids skipped template rendering and also
dropped every image in its messages, so the engine saw bare placeholder
tokens. Collect the messages' media in message order, as the token-first
encoders already do, and attach it to the request.
@celve
celve marked this pull request as ready for review October 17, 2026 11:02
Copilot AI balanced review requested due to automatic review settings October 17, 2026 11:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-17T11:05:17.740395Z b3b1b2e Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b3b1b2e581

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

audio_data=None,
video_data=None,
modalities=[],
image_data=image_data or None,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Nest multiple images before parallel sampling

When a pretokenized request contains multiple images and sets n > 1, this flat image_data list reaches GenerateReqInput.normalize_batch_and_arguments(). Parallel sampling first converts the single prompt into a one-item batch, after which _normalize_image_data() requires len(image_data) == batch_size; two or more collected images therefore raise ValueError: The length of image_data should be equal to the batch size instead of producing the requested completions. The new test does not call normalization, so it misses this runtime path; the collected images need to be represented as one request's nested image list before fan-out.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.