Repository navigation
feat(sidecar): forward image, video, and audio inputs to SGLang - #16203
Conversation
✅ Dynamo PR CI passed — run 41426284787 (attempt 1) on
|
9840880 to
6dd6e34
Compare
6dd6e34 to
9f73d8b
Compare
Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
…mm_hashes field Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
Signed-off-by: Krishnan Prashanth <kprashanth@nvidia.com>
9f73d8b to
84eea18
Compare
Findings
|
| Layer / File(s) | Summary |
|---|---|
Media fields and capability discovery lib/sidecar/sglang/proto/sglang.proto, lib/sidecar/sglang/proto/README.md, lib/sidecar/sglang/src/client.rs, lib/sidecar/sglang/tests/proto_contract.rs |
GenerateRequest adds repeated image, video, and audio fields at tags 19–21. The client derives per-modality support from grpc_features. Tests cover feature parsing and protobuf field tags. |
Media mapping and request validation lib/sidecar/sglang/src/protocol.rs, lib/sidecar/sglang/src/protocol/request_tests.rs |
Request construction maps HTTP, HTTPS, and data URLs to modality fields. Validation rejects unsupported media sources, media kinds, and payload forms. Tests cover mapping, refusal cases, and support checks. |
Engine capability checks and documentation lib/sidecar/sglang/src/engine.rs, lib/sidecar/sglang/README.md, docs/fern/pages/developer-guide/knowledge-base/modular-components/backends/sglang/sidecar.md |
The engine stores discovered media support and checks it before sending a generation request. The documentation describes media support, feature gating, and SGLang URL-fetch restrictions. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Merge Risk: 
Moderate · up to 84eea
-
Image, video and audio URLs from clients are now forwarded to SGLang, which by default fetches them from any host, including internal addresses. Deployments should enforce a destination policy or require --allowed-media-domains before enabling media input.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Description check
-
Explanation
The description includes the required Overview, Key Changes, Validation, and Issues and Ongoing Work sections. It explains the implementation, testing, dependency, issue, and known limitation. The optional pull request checklist section is not included, but the description is otherwise complete.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 11 functions across 5 files. (4 skipped: 4 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
#17653 (SGLang 0.5.23) set GenerateRequest's image_data, video_data and audio_data to empty, and #16203 set the same fields to the forwarded media. Both merged into main, so the struct named each field twice and the SGLang sidecar stopped compiling. Keep the forwarded media. Signed-off-by: Julien Darve <jdarve@NVIDIA.com>
- Engine loss (#17972): api.v1 has no state stream, so poll GetModelInfo every 1 s, each poll bounded by min(connect_attempt_timeout, 5 s), and report "SGLang engine lost: ..." through main's engine_lost hook after 3 failed polls in a row. HealthCheck is not polled: it runs a probe generation and fails while the scheduler is paused. - Media (#16203): not ported. Requests with multi_modal_data keep the stack's "multimodal payloads are not supported" refusal; MediaSupport, media_support, require_media_support and the runtime.v1 media wire-tag test are dropped. Main's mm_processor_kwargs, media-uuid and encoder_result refusals stay. - Native HTTP (#16983): keep the per-dispatch rid and inspect(). Abort goes through the api.v1 Abort RPC with the dispatch rid, falling back to ctx.id(); main's HTTP /abort_request call is dropped, and its malformed-output abort cases now check the Abort RPC. - #16570: drop the default-chat-template-kwargs warning and its test; api.v1 GetServerInfo has no such field. - Keep #17710's migration mapping and #16303's logit_bias refusal. - Remove the runtime.v1 sglang.proto and headless.rs. proto/README.md now names sglang#45859 and #46637 as service.proto's sources. - .pre-commit-config.yaml: union of both exclude patterns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: jain-ria <riajain@NVIDIA.com>
Overview
The SGLang sidecar returns 400 for every image, video, and audio request, because the SGLang
runtime.v1GenerateRequesthad no media fields. SGLang 0.5.23 adds theimage_data,video_data, andaudio_datafields, and lists inGetServerInfogrpc_featuresthe media kinds that the model can read. The sidecar now forwards each media URL to its field and refuses each media kind that the server does not list.Key Changes
About 140 lines of core logic.
lib/sidecar/sglang/src/protocol.rs: puts eachimage_url,video_url, andaudio_urlitem in the field for its kind, in request order. It accepts onlyhttp://,https://, anddata:sources, because SGLang readsfile://from its own disk. It refuses inputs that SGLang would drop, such asmm_processor_kwargs.lib/sidecar/sglang/src/client.rs,engine.rs: readgrpc_featuresat startup and refuse each unlisted media kind. Otherwise an older server drops unknown fields, a processor drops media with no placeholder, and the model answers without the media.lib/sidecar/sglang/proto/sglang.proto: the 0.5.23 contract, the same bytes as in chore(sglang): bump SGLang to 0.5.23 #17653.Validation
SGLang 0.5.23 from PyPI, main with #17653, RTX 6000 Ada.
file://URLtests/serve/test_sidecar.py(4 SGLang configs)request_tests.rscovers the mapping and each refusal, andproto_contract.rscovers the wire tags.Issues and Ongoing Work
protocol.rs.Summary by CodeRabbit