Skip to content

[Bugfix][EPD] Preserve media processing options in encoder requests - #56786

Merged
Isotr0py merged 2 commits into
vllm-project:mainfrom
jiangkuaixue123:codex/fix-epd-encoder-media-options
Sep 14, 2026
Merged

Isotr0py merged 2 commits into
vllm-project:mainfrom
jiangkuaixue123:codex/fix-epd-encoder-media-options

Conversation

@jiangkuaixue123

@jiangkuaixue123 jiangkuaixue123 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Purpose

The EPD proxy drops per-request media processing options when constructing encoder primer requests. An encoder can therefore use the default image size while PD applies the requested override, producing incompatible embeddings and placeholders. With Qwen3.5-35B-A3B, the pre-fix proxy reproduced an HTTP 500 and Attempted to assign 54 = 54 multimodal tokens to 475 placeholders.

Forward mm_processor_kwargs, media_io_kwargs, priority, and session_id to each encoder request when present. Preserve the encoder-reported hash as the key for connector handles so NIXL consumers can find handles after processing options derive a new hash. Extend the existing HTTP round-trip test to cover requests with and without these options, derived encoder hashes, streaming, optional prefill, and decode retries.

Related to #52409 (not closing the umbrella tracker).

Duplicate-work checks: reviewed #52409 and its comments, searched open PRs referencing it and EPD/media-processing keywords, and inspected related changes. #50164 and #55842 address the Rust frontend; #56242 addresses shared encoder output reuse; #54176 adds dynamic proxy registration/routing. None implements this encoder-primer field forwarding fix. #56657, which introduced the existing HTTP round-trip coverage, is already merged.

Test Plan

.venv/bin/pre-commit run --files \
  examples/disaggregated/disaggregated_encoder/disagg_epd_proxy.py \
  tests/v1/ec_connector/unit/test_epd_proxy_retry.py

# On the H20 allocation, from the source checkout:
/home/david_cwq/jcz/software/vllm-pr41567/bin/python -m pytest \
  tests/v1/ec_connector/unit/test_epd_proxy_retry.py -q

# Real-model comparison harness saved outside the repository:
srun --jobid=2982 --overlap --nodes=1 --ntasks=1 bash -lc \
  'ulimit -l unlimited; /home/david_cwq/jcz/software/vllm-pr41567/bin/python -u /home/david_cwq/jcz/runs/epd-media-real-2982/run.py'

Real-model setup: 2 × H20-3e; Qwen3.5-35B-A3B checkpoint revision 59d61f3ce65a6d9863b86d2e96597125219dc754; sequential single-instance baseline and 1E + 1PD with ECMooncakeConnector. Logs confirm RDMA (RoCE/iWARP). Both deployments use TP=1, eager mode, CPU media preprocessing, and disabled prefix caching.

Send the same repository image (tests/multimodal/assets/rgba.png, 800×600 RGBA), prompt, and settings to both endpoints: temperature=0, seed=42, max_tokens=128, chat_template_kwargs={"enable_thinking":false}. Prompt: “Describe the objects and their colors in this image in one short sentence.” Non-default processing options:

{
  "mm_processor_kwargs": {
    "size": {"shortest_edge": 16384, "longest_edge": 65536}
  },
  "media_io_kwargs": {
    "image": {"rgba_background_color": [0, 0, 0]}
  }
}

For the negative control, run the pre-fix d5051840e0^ proxy from an external copy against the same E/PD servers and send the identical non-default request. The checked-out engine code remains unchanged.

Test Result

  • All applicable pre-commit hooks passed.
  • Proxy regression suite: 16 passed. The first remote run lacked pytest-asyncio; installing the repository-pinned pytest-asyncio==1.4.0 resolved that test-environment issue. The initial local run with repository-wide fixtures could not start because PyTorch was absent.
  • Real requests on commit d5051840e065f36f535913076065147b383ce464:
Deployment Processing options HTTP Prompt tokens Completion tokens Output
Single instance Default 200 503 24 Default baseline
Fixed E+PD Default 200 503 24 Exact match to default baseline
Single instance Non-default 200 82 23 Non-default baseline
Fixed E+PD Non-default 200 82 23 Exact match to non-default baseline
Pre-fix proxy + same E/PD Non-default 500 Token/placeholder mismatch

The fixed EPD response message and complete usage object exactly match their baseline counterparts in both cases. Shared non-default output:

Four colorful dice—blue, red, green, and yellow—are suspended in mid-air against a black background.

The negative control logs show encoder metadata image_grid_thw=[1,38,50] versus PD's processed grid [1,12,18], followed by:

RuntimeError: shape mismatch: value tensor of shape [54, 2048] cannot be broadcast to indexing result of shape [475, 2048]
ValueError: Attempted to assign 54 = 54 multimodal tokens to 475 placeholders

This is a targeted one-image serving regression check, not a broad accuracy benchmark. priority and session_id forwarding are covered by the HTTP unit test, not separately by real-model requests. The existing precompiled environment was sufficient; its installed version label remains 0.1.dev20522+g9a855442d, while editable Python source was the tested PR commit. No vLLM/precompiled reinstall was performed.

Full requests, responses, process commands, and logs are retained on the test host under /home/david_cwq/jcz/runs/epd-media-real-2982.

AI assistance (OpenAI Codex) was used to prepare the change, run validation, and write this description.

NIXL handle hash regression validation

The amended revision preserves ec_params[ec_mm_hash] rather than re-keying handles by the original UUID. The existing HTTP backend fixture now returns a distinct encoder hash when processing kwargs are supplied; assertions verify that the handle retains this key after forwarding and retries. The retry-isolation test also checks the exact reported hash key.

.venv/bin/python -m pytest --confcutdir=tests/v1/ec_connector/unit \
  tests/v1/ec_connector/unit/test_epd_proxy_retry.py -q

With the strengthened tests and the old handle assignment: 5 failed, 11 passed. With the fix: 16 passed. All applicable pre-commit hooks passed. --confcutdir skips unrelated repository-wide model fixtures for this standalone HTTP proxy suite.

The real-model Mooncake results above were obtained on the explicitly recorded earlier commit; the NIXL hash follow-up has HTTP regression coverage, not a new NIXL real-model run.

Remote confirmation on amended commit 0784c83c1c6b24422c723408e13e5bf70cd662fe: after server-side git pull --ff-only, the normal pytest command above (without --confcutdir) also completed with 16 passed on the H20 allocation.

Co-authored-by: Codex <noreply@openai.com>

Signed-off-by: jiangkuaixue123 <jiangxiaozhou111@163.com>
@mergify

mergify Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Documentation preview: https://vllm--56786.org.readthedocs.build/en/56786/

@mergify mergify Bot added documentation Improvements or additions to documentation bug Something isn't working kv-connector labels Sep 14, 2026
@jiangkuaixue123
jiangkuaixue123 marked this pull request as ready for review September 14, 2026 08:16

@claude claude 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@jiangkuaixue123

Copy link
Copy Markdown
Contributor Author

cc @gty111 @Isotr0py

@Isotr0py Isotr0py left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@Isotr0py Isotr0py added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 14, 2026
@github-actions

Copy link
Copy Markdown

@jiangkuaixue123, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

"media_io_kwargs",
"priority",
"session_id",
):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we also preserve the encoder-reported hash when forwarding NIXL handles below? With non-default processing kwargs, the consumer looks up the derived hash, but the handle is still keyed by the original UUID, so no remote load is started. Mooncake's ec_items already uses the derived hash.

Suggested change in fanout_encoder_primer:

-                ec_params[item_uuids.get(idx, ec_mm_hash)] = reported
+                ec_params[ec_mm_hash] = reported

Preserve the main merge and address the NIXL handle hash review.

Co-authored-by: Codex <noreply@openai.com>

Signed-off-by: jiangkuaixue123 <jiangxiaozhou111@163.com>
@jiangkuaixue123
jiangkuaixue123 force-pushed the codex/fix-epd-encoder-media-options branch from d98c9b2 to 0784c83 Compare September 14, 2026 08:52
@Isotr0py
Isotr0py enabled auto-merge (squash) September 14, 2026 09:04
@Isotr0py

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #88773 for commit 0784c83c1c6b.

@Isotr0py
Isotr0py merged commit 1678b39 into vllm-project:main Sep 14, 2026
37 checks passed
Shreya-gaur pushed a commit to Shreya-gaur/vllm_private that referenced this pull request Sep 14, 2026
jiaran-king added a commit to jiaran-king/vllm that referenced this pull request Sep 15, 2026
Merge upstream main at 3192898.
Include the EPD proxy serialization optimization (vllm-project#56657), JSON-array
multimodal metadata (vllm-project#56090), and media processing options forwarding
(vllm-project#56786). Git merged the overlapping Mooncake KV worker and tests without
conflicts; no manual code adjustments were needed.

Validation: 61 EC/cache/runner/proxy CPU tests and 171 Mooncake KV Store
worker CPU tests passed. PR-file pre-commit, including mypy 3.10, passed.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Zhou ziheng <jiaranran2@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation kv-connector ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants