feat(zmq): typed structured outputs for the direct backend, with e2e coverage - #2041
feat(zmq): typed structured outputs for the direct backend, with e2e coverage#2041slin1237 wants to merge 10 commits into
Conversation
Run the existing local (non-PD/EPD) e2e cases over the ZMQ direct backend, in addition to gRPC/HTTP, for both vLLM and TokenSpeed. - Add ConnectionMode.ZMQ and an E2E_CONNECTION_MODE per-lane override (mirrors how E2E_RUNTIME picks the engine): a grpc/http case runs over ZMQ without a separate parametrize value. - Worker: derive the ipc:// base_url from serve._zmq_ipc_url, skip the worker health wait (readiness is gateway-gated), and build the headless vLLM/TokenSpeed commands by delegating to the serve launchers so engine flags and the FNV-1a handshake port stay in lockstep with production. - Gateway: pass --router-backend explicitly (the router cannot probe the wire from an ipc:// URL). - Worker pool: do not cache ZMQ workers (an engine dials one gateway's handshake sockets and cannot be reused); stop them with the gateway. - Exclude the gRPC-only families from ZMQ lanes (PD, EPD, multi-worker) and collapse grpc/http twins onto a single ZMQ run, via pytest_collection_modifyitems using only public API. - CI: add an e2e-1gpu-chat-zmq lane (vllm, tokenspeed) and thread the connection_mode input through the gpu job. TokenSpeed keeps its pinned install commit. - Add no-GPU unit tests for the ZMQ command builders and base_url. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds ZMQ as an E2E connection mode and adds typed vLLM structured-output support for the ZMQ EngineCore client. It updates worker and gateway lifecycle handling, test selection, GPU CI coverage, protocol serialization, request translation, and validation tests. ChangesZMQ E2E support
vLLM structured outputs
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant e2e-gpu-job
participant setup_backend
participant Worker
participant Gateway
CI->>e2e-gpu-job: Start with connection_mode=zmq
e2e-gpu-job->>setup_backend: Set E2E_CONNECTION_MODE
setup_backend->>Worker: Build and start ZMQ worker
setup_backend->>Gateway: Start gateway with backend and readiness timeout
Gateway->>Worker: Validate readiness
setup_backend->>Worker: Stop ZMQ worker during teardown
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| # holds the GPUs the caller is about to claim. ZMQ workers are | ||
| # likewise uncached: the engine dials one gateway's handshake | ||
| # sockets and cannot be reused by the next class's gateway. | ||
| if worker_type != WorkerType.REGULAR or mode == ConnectionMode.ZMQ: |
There was a problem hiding this comment.
🟡 Nit: The condition now also matches WorkerType.REGULAR + ConnectionMode.ZMQ, but the log message on line 99 still says "non-REGULAR". For a ZMQ regular worker, the log would read "evicting … for non-REGULAR regular/…" which is contradictory. Consider updating the message to something like "non-cacheable":
| if worker_type != WorkerType.REGULAR or mode == ConnectionMode.ZMQ: | |
| if worker_type != WorkerType.REGULAR or mode == ConnectionMode.ZMQ: | |
| if self._key is not None: | |
| logger.info( | |
| "WorkerPool: evicting %s to free GPUs for non-cacheable %s/%s", |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
e2e_test/fixtures/hooks.py (1)
158-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win🟡 Nit Add collection tests for the ZMQ item filter and deduplication key.
E2E_CONNECTION_MODE=zmqnow depends on_filter_zmq_items, but the e2e tests only cover ZMQ command builders and base URLs. Add tests for grpc/http twin collapse, http-only retention, PD/EPD/empty-tuple exclusions, multi-worker exclusion, and cases withoutsetup_backend.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e_test/fixtures/hooks.py` around lines 158 - 183, Add collection-level tests covering `_filter_zmq_items` and `_zmq_dedup_key`: verify grpc/http twins collapse to one run, http-only cases remain, PD/EPD cases including empty topology tuples are deselected, multi-worker items are excluded, and items without `setup_backend` remain kept. Use representative pytest items or existing fixture helpers and assert both kept and deselected collections.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-test-rust.yml:
- Around line 546-580: The e2e-gpu-job workflow generates identical worker-log
artifact names for the gRPC and ZMQ chat jobs. Update the artifact naming logic
in e2e-gpu-job.yml to include E2E_CONNECTION_MODE in the suffix, preserving the
existing engine, GPU tier, and test-directory components so ZMQ logs remain
uniquely restorable.
In `@e2e_test/fixtures/setup_backend.py`:
- Around line 149-153: Reject ConnectionMode.ZMQ after resolving engine unless
the engine is vllm or tokenspeed, using a shared validation helper. Apply this
validation at e2e_test/fixtures/setup_backend.py lines 149-153 and 484-489, and
add parameterized tests covering supported and unsupported runtime/mode
combinations.
In `@e2e_test/infra/constants.py`:
- Around line 140-141: Update the connection-mode parsing around
ENV_CONNECTION_MODE to distinguish an unset variable from an explicitly empty
value: return None only when the environment lookup returns None, and raise
ValueError for an empty string before calling ConnectionMode. Preserve
case-insensitive handling for valid values and rejection of invalid values, and
add tests covering unset, empty, valid mixed-case, and invalid inputs.
In `@e2e_test/infra/test_zmq_cmd_builders.py`:
- Around line 7-8: Move the module-level pytest.importorskip("smg.serve") from
the top of test_zmq_cmd_builders.py into the ZMQ-specific tests or fixture that
requires serve, so test_grpc_worker_still_uses_grpc_url always runs
independently of smg.serve availability.
---
Nitpick comments:
In `@e2e_test/fixtures/hooks.py`:
- Around line 158-183: Add collection-level tests covering `_filter_zmq_items`
and `_zmq_dedup_key`: verify grpc/http twins collapse to one run, http-only
cases remain, PD/EPD cases including empty topology tuples are deselected,
multi-worker items are excluded, and items without `setup_backend` remain kept.
Use representative pytest items or existing fixture helpers and assert both kept
and deselected collections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fcf2da9c-5d27-4e54-8b44-71c108d7f055
📒 Files selected for processing (10)
.github/workflows/e2e-gpu-job.yml.github/workflows/pr-test-rust.ymle2e_test/fixtures/hooks.pye2e_test/fixtures/setup_backend.pye2e_test/infra/__init__.pye2e_test/infra/constants.pye2e_test/infra/gateway.pye2e_test/infra/test_zmq_cmd_builders.pye2e_test/infra/worker.pye2e_test/infra/worker_pool.py
There was a problem hiding this comment.
Clean, well-structured test-infrastructure PR. The ZMQ lane integration is thorough: collection filtering correctly deselects PD/EPD/multi-worker families, the dedup logic for grpc/http twins is sound, worker lifecycle is properly handled (no caching for ZMQ, explicit teardown), and the CI wiring threads the connection_mode input through correctly. One minor nit on a log message.
The ZMQ chat lane failed CI because gateway.py passed --router-backend, which launch_router.py does not accept (the flag is --backend); the router crashed on every launch and the lane timed out. Switch to --backend. Also address review feedback: - disambiguate worker-log artifact names by connection mode so the gRPC and ZMQ vllm chat legs no longer collide - reject ConnectionMode.ZMQ for engines that can't speak it via a shared validation helper, failing fast instead of timing out - distinguish an unset E2E_CONNECTION_MODE from a set-but-empty/invalid value, which now raises - scope the smg.serve importorskip to the ZMQ command-builder tests so the gRPC test runs without the wheel - add unit tests for connection-mode parsing, engine validation, and the ZMQ collection filter Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
137b453 to
4e49efa
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
e2e_test/fixtures/test_connection_mode_validation.py (1)
8-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the validation test independent of cloud fixture dependencies.
pytest.importorskip("fixtures.setup_backend")makese2e_test/fixtures/test_connection_mode_validation.pyunreachable when fixtures.setup_backend cannot import, including when Anthropic/OpenAI/GenAI cloud modules are unavailable. Move_validate_connection_modeandZMQ_CAPABLE_ENGINESout to an unused dependency-free E2E infra module, import it from bothfixtures/setup_backend.pyand this test file, and require CI to collect and run all three parametrized test cases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e_test/fixtures/test_connection_mode_validation.py` around lines 8 - 9, Move _validate_connection_mode and ZMQ_CAPABLE_ENGINES from the cloud-dependent fixture path into a dependency-free E2E infrastructure module, then import and reuse them in both fixtures/setup_backend.py and test_connection_mode_validation.py. Remove the pytest.importorskip dependency from the validation test so all three parametrized cases are collected and run even when cloud SDKs are unavailable.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e-gpu-job.yml:
- Line 177: Update the workflow step around MODE so it reuses the already
assigned E2E_CONNECTION_MODE instead of interpolating inputs.connection_mode
into shell source. Validate connection_mode against the supported wire-mode enum
before using it as an artifact suffix or writing it to GITHUB_OUTPUT, and reject
unsupported values.
---
Nitpick comments:
In `@e2e_test/fixtures/test_connection_mode_validation.py`:
- Around line 8-9: Move _validate_connection_mode and ZMQ_CAPABLE_ENGINES from
the cloud-dependent fixture path into a dependency-free E2E infrastructure
module, then import and reuse them in both fixtures/setup_backend.py and
test_connection_mode_validation.py. Remove the pytest.importorskip dependency
from the validation test so all three parametrized cases are collected and run
even when cloud SDKs are unavailable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dd8af8a5-1b21-4c2d-9151-e1a1fd2e22d3
📒 Files selected for processing (8)
.github/workflows/e2e-gpu-job.ymle2e_test/fixtures/setup_backend.pye2e_test/fixtures/test_connection_mode_validation.pye2e_test/fixtures/test_hooks_zmq_filter.pye2e_test/infra/constants.pye2e_test/infra/gateway.pye2e_test/infra/test_connection_mode.pye2e_test/infra/test_zmq_cmd_builders.py
🚧 Files skipped from review as they are similar to previous changes (2)
- e2e_test/infra/gateway.py
- e2e_test/infra/test_zmq_cmd_builders.py
| E2E_RUNTIME: ${{ inputs.engine }} | ||
| E2E_GPU_TIER: ${{ inputs.gpu_tier }} | ||
| E2E_VLLM_KV_BACKEND: ${{ inputs.vllm_kv_backend }} | ||
| E2E_CONNECTION_MODE: ${{ inputs.connection_mode }} |
There was a problem hiding this comment.
🔴 Important: This env var is set unconditionally for every job that calls this reusable workflow. The connection_mode input defaults to "" (line 60), so every existing caller that doesn't pass connection_mode (e.g. e2e-1gpu-chat, e2e-1gpu-completions, …) will run with E2E_CONNECTION_MODE="".
get_connection_mode_override() in constants.py (line 146-147) intentionally raises ValueError on a set-but-empty value. Since pytest_collection_modifyitems calls this function at collection time, all non-ZMQ e2e jobs will crash before running any tests.
The workflow default and the Python validation are in conflict. One fix — treat empty the same as unset in the Python code:
if not value:
return NoneOr conditionally set the env var only when non-empty:
| E2E_CONNECTION_MODE: ${{ inputs.connection_mode }} | |
| E2E_CONNECTION_MODE: ${{ inputs.connection_mode || '' }} |
(Though that still sets it to "" — the Python-side fix is cleaner.)
The workflow always exports E2E_CONNECTION_MODE and leaves it empty for the non-override lanes, so raising on a set-but-blank value broke every regular e2e lane at collection time. Map blank to None (the default) while still rejecting genuinely-unrecognized values. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
…ckend Port the engine-core structured-outputs params (guided decoding) into the engine ZMQ client and wire the proto constraint oneof onto the EngineCore wire, so ZMQ-backed vLLM workers honor json/regex/grammar/choice/ json_object/structural_tag constraints instead of returning unconstrained text. The client exposes an enum-backed StructuredOutputsParams and serializes through a private sparse wire struct (omit-defaults, skip-if-false) that matches Python engine-core; the backend is always lowered to guidance. translate_sampling maps json_object=false to no constraint. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
ZMQ workers spawn and return immediately (readiness is gated by the gateway, not the worker), so model load happens inside the gateway's readiness wait. That wait used DEFAULT_ROUTER_TIMEOUT (60s), too short for large models. Use the model's startup_timeout for ZMQ gateways (never shrinking an explicitly larger gateway timeout); gRPC/HTTP unchanged. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
github-actions
Bot
added
grpc
| @@ -0,0 +1,315 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
There was a problem hiding this comment.
🟡 Nit: The merge base commit (4bec9d6, PR #2043) removed SPDX-License-Identifier headers from every other file in this crate. This new file re-introduces one. Drop it to stay consistent:
| // SPDX-License-Identifier: Apache-2.0 | |
| // Ported from the Apache-2.0 reference `vllm-engine-core-client` |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e_test/fixtures/setup_backend.py (1)
281-312: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win🔴 Important: Guard ZMQ worker cleanup before gateway operations.
In
_setup_localandbackend_router,_start_workers_tracked(...)/get_pool().acquire()can return ZMQ workers before the protected region starts. If_gateway_readiness_timeout(...)orGateway()raises,stop_workers(workers)is skipped. Move worker acquisition into thetryand placestop_workers(workers)in the innerfinally; initializegateway = Nonefirst and only callgateway.shutdown()when it is notNone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e_test/fixtures/setup_backend.py` around lines 281 - 312, Update the setup flow around `_setup_local` and `backend_router` so ZMQ worker acquisition occurs inside the protected `try` block, with `workers` cleanup in an inner `finally` that always calls `stop_workers(workers)`. Initialize `gateway` to `None` before gateway creation and only invoke `gateway.shutdown()` when a gateway was successfully created, including when `_gateway_readiness_timeout(...)` or `Gateway()` raises.
🧹 Nitpick comments (2)
e2e_test/fixtures/setup_backend.py (1)
115-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win🟡 Nit: Add direct tests for
_gateway_readiness_timeout.Test these cases: non-ZMQ returns
base_timeout, ZMQ preserves a largerbase_timeout, and ZMQ uses a largerstartup_timeout. Also test the explicitDEFAULT_STARTUP_TIMEOUTfallback.The supplied test context covers
_validate_connection_mode, but it does not show coverage for these timeout branches. Add or confirm parameterized coverage.As per coding guidelines: “Run the pr-test-analyzer agent on changed files to verify that tests adequately cover new or changed functionality.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e_test/fixtures/setup_backend.py` around lines 115 - 133, Add direct parameterized tests for _gateway_readiness_timeout covering non-ZMQ returning base_timeout, ZMQ retaining a larger base_timeout, ZMQ selecting a larger model startup_timeout, and ZMQ using DEFAULT_STARTUP_TIMEOUT when startup_timeout is absent. Mock get_model_spec as needed, and ensure the changed fixture file is included in pr-test-analyzer verification.Source: Coding guidelines
model_gateway/src/routers/grpc/zmq_client.rs (1)
1397-1454: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win🟡 Nit Test the structured-output path over ZMQ.
Line 1397 only checks in-memory translation. It does not verify that
StructuredOutputsParamsreaches EngineCore after ZMQ MessagePack serialization. Extend an existing VLLM ZMQ test with a constraint and assert the mock engine receives it. AddGrammar,StructuralTag, and invalid-JSON-schema fallback cases here so everytranslate_constraintbranch has coverage.As per coding guidelines, “Run the pr-test-analyzer agent to verify that tests adequately cover new or changed functionality.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model_gateway/src/routers/grpc/zmq_client.rs` around lines 1397 - 1454, Extend the existing VLLM ZMQ integration test around the structured-output request path, rather than only testing translate_request in memory. Add a structured-output constraint to the request and assert the mock EngineCore receives the deserialized StructuredOutputsParams after ZMQ MessagePack transport; cover Grammar, StructuralTag, and invalid JSON-schema fallback branches in translate_constraint, then run the pr-test-analyzer agent to verify coverage.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@e2e_test/fixtures/setup_backend.py`:
- Around line 281-312: Update the setup flow around `_setup_local` and
`backend_router` so ZMQ worker acquisition occurs inside the protected `try`
block, with `workers` cleanup in an inner `finally` that always calls
`stop_workers(workers)`. Initialize `gateway` to `None` before gateway creation
and only invoke `gateway.shutdown()` when a gateway was successfully created,
including when `_gateway_readiness_timeout(...)` or `Gateway()` raises.
---
Nitpick comments:
In `@e2e_test/fixtures/setup_backend.py`:
- Around line 115-133: Add direct parameterized tests for
_gateway_readiness_timeout covering non-ZMQ returning base_timeout, ZMQ
retaining a larger base_timeout, ZMQ selecting a larger model startup_timeout,
and ZMQ using DEFAULT_STARTUP_TIMEOUT when startup_timeout is absent. Mock
get_model_spec as needed, and ensure the changed fixture file is included in
pr-test-analyzer verification.
In `@model_gateway/src/routers/grpc/zmq_client.rs`:
- Around line 1397-1454: Extend the existing VLLM ZMQ integration test around
the structured-output request path, rather than only testing translate_request
in memory. Add a structured-output constraint to the request and assert the mock
EngineCore receives the deserialized StructuredOutputsParams after ZMQ
MessagePack transport; cover Grammar, StructuralTag, and invalid JSON-schema
fallback branches in translate_constraint, then run the pr-test-analyzer agent
to verify coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 88f856d3-1aab-436a-95ce-37e5764de010
📒 Files selected for processing (6)
crates/engine_zmq_client/src/error.rscrates/engine_zmq_client/src/protocol/vllm/mod.rscrates/engine_zmq_client/src/protocol/vllm/sampling.rscrates/engine_zmq_client/src/protocol/vllm/structured_outputs.rse2e_test/fixtures/setup_backend.pymodel_gateway/src/routers/grpc/zmq_client.rs
Use #[expect] instead of #[allow] for the trivially_copy_pass_by_ref predicate (clippy::allow_attributes is deny-by-default), apply nightly rustfmt to the ported module and the constraint translator, and let ruff-format normalize the readiness-timeout helper calls. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
slin1237
changed the title
|
👋 The PR description doesn't fully follow
Please update the PR description so reviewers have the context they need. |
EngineCore requires a concrete max_tokens (the wire field is non-optional and serde-defaults to 16). The ZMQ path bypasses vLLM's OpenAI frontend, which would otherwise default an unset max_tokens to max_model_len - prompt_len. Without that, requests fell back to 16 tokens and truncated generation -- e.g. streaming reasoning never reached content past the </think> transition. Take max_model_len from the connected engine's ready handshake and apply the same default; a connected engine is now required to translate a request. Validated on Qwen3-30B-A3B over ZMQ: reasoning and content both stream with finish delivered. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
The ZMQ direct path only forwarded per-token logprobs, dropping the top_logprobs alternatives that clients request via `logprobs: N`. The engine returns PositionLogprobs.entries as [sampled, top1..topk] (k+1 columns); take the first N entries per position to match what the gRPC servicer produces, and accumulate them for the completion aggregate. Adds a unit test asserting the requested-count shaping. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
Stop-string detection needs detokenized text, which vLLM does in its frontend OutputProcessor. Over gRPC the Python servicer forwards the stop strings to the engine, so vLLM trims the output and returns finish_reason=stop itself. The ZMQ direct path talks to EngineCore, which has no stop-string detection, so it ran to max_tokens and reported "length" even though the gateway's own StopSequenceDecoder had already trimmed the text correctly. Synthesize finish_reason=stop from the local decoder signal across the OpenAI chat (stream + non-stream), completions (non-stream), and Anthropic Messages (stream + non-stream) paths, mirroring the existing completions-streaming precedent. This is a no-op for gRPC: vLLM trims the stop string before returning, so the local decoder never fires, and matched_stop falls back to the engine value via or_else so token-stop and length results are preserved. Adds StopSequenceDecoder::matched_stop() so the matched string flows into the OpenAI matched_stop field and the Anthropic stop_reason= stop_sequence / stop_sequence field, matching gRPC parity. Unit tests cover the string-match and token-stop cases. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
Engines that can't match string `stop` sequences server-side previously handled them in three divergent ways: SGLang gRPC (skip_tokenizer_init) had a dedicated resolver that dropped the strings and converted single-token stops to stop_token_ids; the ZMQ vLLM path silently dropped strings with no early-stop conversion; and the ZMQ TokenSpeed path rejected any request carrying `stop` with a 400. Consolidate all of them onto one helper, resolve_string_stops, applied in the chat/completion/generate/messages request-building stages. For backends whose engine sees token ids only — SGLang on any transport, and every direct-ZMQ backend — it drops the string `stop` list (the router-side StopSequenceDecoder trims the text) and forwards single-token stops as stop_token_ids for early stopping; multi-token stops fall to the decoder. gRPC vLLM (servicer detokenizes), TRT-LLM (server-side), and MLX keep their strings untouched, gated by the backend's is_zmq() flag. This fixes the TokenSpeed 400 (stop strings now work over ZMQ) and gives the ZMQ vLLM path the single-token early-stop optimization it lacked. The finish_reason=stop synthesis added earlier is already backend-neutral, so the two halves together give consistent stop-string behavior across gRPC-SGLang, ZMQ-vLLM, and ZMQ-TokenSpeed. Supersedes #1877 (SGLang-only input-side fix). Adds unit tests for the shared resolver and for TokenSpeed accepting residual stop strings. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
1 participant
Summary
Adds end-to-end coverage for the ZMQ direct backend connector and folds in
the two pieces of engine-side support the e2e lanes exercise:
E2E harness for ZMQ — a new
ConnectionMode.ZMQpath through theworker/gateway fixtures, worker-pool eviction that accounts for the
single-gateway handshake sockets, ZMQ command builders, connection-mode
validation, and the CI lanes (
e2e-1gpu-chat-zmqfor vLLM andTokenSpeed).
Typed structured outputs — ports the structured-outputs (guided
decoding) protocol module into
crates/engine_zmq_clientand wires theproto
constraintoneof onto typedStructuredOutputsParamsin thegateway's ZMQ client, replacing the previous reject-guard. Constraints
supported: JSON schema, regex, grammar, structural tag, choice, and
json-object. The backend lowers to guidance engine-side, matching the
reference behavior.
Readiness timing — gRPC/HTTP workers are health-checked (model
loaded) by the pool before the gateway starts, but ZMQ workers return
immediately and load the model inside the gateway's readiness gate. The
fixtures now widen the gateway readiness timeout to the model's
startup_timeout(default 300s) for ZMQ so model load isn't clipped bythe 60s router default.
Testing
engine-zmq-clientunit tests (structured-outputs round-trip, constraintvalidation: exactly-one / missing / multiple / json-object=false).
zmq_clienttranslation tests for each constraint kind.