[gateway] WebSocket transport for the Responses API — up to 45% lower TTFT, 22–48% lower ITL p99 tail, 17–21% less gateway CPU by Venkat2811 · Pull Request #1770 · lightseekorg/smg

Venkat2811 · 2026-06-18T00:19:07Z

Problem

The Responses API is HTTP/SSE-only at the gateway. Every previous_response_id continuation re-hydrates the whole conversation from the history backend (get_response_chain → one HGETALL per prior turn on redis) — an O(turns) reconstruction per turn, O(turns²) over a chain. There's no persistent path for multi-turn agentic clients.

Solution

A WebSocket transport for GET /v1/responses (scoped to the gRPC regular worker). Clients upgrade and send response.create events over one persistent socket; previous_response_id resolves from a connection-local cache in O(1) before falling back to durable storage (--history-backend memory|redis|…), while still persisting identically for cross-reconnect/HTTP retrieval. The WS handler reuses the existing streaming pipeline through a transport-neutral ResponseEventSink. Mirrors OpenAI's Responses WebSocket mode.

What the benchmarks show

Re-measured on our hardware — A100-80GB + RTX 3060; Qwen2.5 3B/7B/32B; sglang and vLLM workers; redis and in-memory backends. Δ = (WS − HTTP)/HTTP; negative latency = WS faster.

TTFT is the win, and it grows with chain length — −28% → −41% p50 as chains grow 8 → 40 turns (sglang-7B; −32% → −45% on vLLM), with tail p99 −33% to −48% at c=1. Under concurrency (c=8–64): −22% to −38% p50, −10% to −41% p99. Same shape on both engines.
Per-turn history hydration is eliminated — HTTP's chain-walk issues ∑(k−1) HGETALL reads over a T-turn chain (counted on the c=1 lanes: 672 / 4 560 / 18 720 at 8/20/40 turns — the triangular sum T(T−1)/2, confirming the O(turns²) cost). WS serves each continuation from its connection-local cache → 0 reads, with identical writes; the concurrency cells eliminate the same reads by the same cache path.
The win is gateway-side, not storage-speed — present with an in-memory backend too (TTFT −27% / −31% p50, zero reads). Gateway CPU/turn −17% to −21% (32B), with worker CPU at parity — the saving is the per-turn reconstruction WS skips.
E2E and throughput at parity; per-token delivery is no slower, and tail-smoother — E2E p50 within ±3% (mean −0.4%); E2E p99 sample-noisy at n=320 (mean +0.6%, single tail samples swing it both ways); throughput ±2% on both req/s and output tok/s. Directly-measured inter-token latency (ITL) is at parity at p50 (mean −0.6%) and −22% to −48% at p99 for WS across all 16 cells — consistent with HTTP/SSE coalescing tokens into bursts while WS flushes per frame, so the per-token tail is tighter.
Small-model / loopback / single-turn = parity, as expected; the mechanism pays on multi-turn + larger models. Output verified coherent — identical prompt over HTTP and WS returns equivalent completed text on both engines.

Changes

ResponseEventSink seam (grpc/common/responses/streaming.rs, grpc/regular/responses/streaming.rs) — transport-neutral Sse/Ws sinks + sink-generic stream entrypoints. Also stamps the finalized response with the emitter resp_ id.
WS transport (new routers/ws_responses.rs, grpc/regular/responses/websocket.rs, responses_validation.rs) — connection session/cache/lifetime, single in-flight response.create, 60-min cap, shared HTTP/WS validation.
Continuation cache (grpc/regular/responses/common.rs) — load_conversation_history_with_cache: cache-first, falls back to storage; identical persist path (HTTP passes None, WS passes the cache).
Wiring (routers/mod.rs, grpc/router.rs, router_manager.rs, server.rs) — RouterTrait::route_responses_ws + dispatch; GET /v1/responses upgrade route; restore NoDelayAcceptor (TCP_NODELAY).
Deps — validator moved dev→runtime (used by responses_validation).
Tests — tests/api/responses_ws_test.rs: 22 protocol tests + negative controls. Docs — docs/reference/api/responses.md: WebSocket Mode section.

Benchmarks (detail)

Workload: synthetic multi-turn continuation chains, max_output_tokens=24, chained via previous_response_id + store=true. Loopback (client+gateway+worker co-located). Chains/cell: 320 (concurrency; c=64 is R=3, n=7 680), 24 (c=1 lanes), 8 (32B CPU). RTX 3060 is an n=3 validation (p99 omitted).

A100 · 7B · multi-turn chains (c=1, n=24) — TTFT grows with chain length; history reads → 0

sglang · redis — latency in ms; HTTP / WS raw with Δ per metric.

Chain	HTTP TTFTp50	WS TTFTp50	Δp50	HTTP TTFTp99	WS TTFTp99	Δp99	HTTP E2Ep50	WS E2Ep50	Δ	HGETALL (H→W)
8-turn	35.1	25.3	−27.7%	40.5	26.5	−34.6%	251.7	240.7	−4.4%	672 → 0
20-turn	40.8	26.5	−35.0%	50.8	31.4	−38.2%	276.6	275.6	−0.3%	4 560 → 0
40-turn	51.8	30.5	−41.2%	73.7	39.9	−45.8%	290.6	278.1	−4.3%	18 720 → 0

vLLM · redis — latency in ms; HTTP / WS raw with Δ per metric.

Chain	HTTP TTFTp50	WS TTFTp50	Δp50	HTTP TTFTp99	WS TTFTp99	Δp99	HTTP E2Ep50	WS E2Ep50	Δ	HGETALL (H→W)
8-turn	32.3	22.1	−31.6%	38.4	25.8	−32.8%	224.3	222.7	−0.7%	672 → 0
20-turn	38.8	23.3	−39.9%	50.5	28.4	−43.9%	265.4	263.0	−0.9%	4 560 → 0
40-turn	50.1	27.4	−45.4%	72.5	37.6	−48.1%	278.3	267.8	−3.8%	18 720 → 0

A100 · 7B · concurrency — full 4-way matrix (sglang+vLLM × redis+memory; 16 cells, zero failures)

WS vs HTTP, same session (mode=both). Each metric shows raw HTTP and WS absolutes with Δ = (WS−HTTP)/HTTP in its own column, at p50 and p99. Latencies in ms.

TTFT (ms)

backend	c	HTTP p50	WS p50	Δp50	HTTP p99	WS p99	Δp99
sglang · redis	8	54.1	40.0	−26.0%	88.1	60.0	−31.9%
sglang · redis	16	59.7	43.8	−26.6%	97.4	68.1	−30.1%
sglang · redis	32	74.8	49.3	−34.1%	117.4	69.9	−40.5%
sglang · redis	64	94.1	58.7	−37.7%	187.3	113.2	−39.6%
sglang · memory	8	50.7	38.8	−23.4%	84.3	57.0	−32.4%
sglang · memory	16	55.2	42.7	−22.6%	91.1	63.1	−30.8%
sglang · memory	32	68.1	50.1	−26.4%	104.3	73.4	−29.6%
sglang · memory	64	86.5	57.1	−34.0%	139.0	116.7	−16.0%
vLLM · redis	8	42.9	32.0	−25.5%	58.4	36.5	−37.4%
vLLM · redis	16	46.2	33.7	−27.0%	60.2	45.5	−24.4%
vLLM · redis	32	60.2	39.2	−34.8%	70.7	63.7	−9.8%
vLLM · redis	64	75.3	51.5	−31.5%	111.9	98.8	−11.7%
vLLM · memory	8	42.6	32.3	−24.3%	51.6	35.9	−30.5%
vLLM · memory	16	45.2	34.0	−24.8%	57.8	45.3	−21.5%
vLLM · memory	32	53.9	39.2	−27.3%	70.8	53.9	−23.9%
vLLM · memory	64	73.4	51.6	−29.6%	104.5	89.5	−14.4%

E2E latency (ms)

backend	c	HTTP p50	WS p50	Δp50	HTTP p99	WS p99	Δp99
sglang · redis	8	350.6	355.5	+1.4%	441.4	439.7	−0.4%
sglang · redis	16	413.2	421.2	+1.9%	529.8	545.4	+2.9%
sglang · redis	32	496.2	489.7	−1.3%	710.6	647.5	−8.9%
sglang · redis	64	588.7	596.3	+1.3%	830.2	993.6	+19.7%
sglang · memory	8	350.7	347.7	−0.9%	445.0	433.2	−2.7%
sglang · memory	16	419.0	421.5	+0.6%	527.7	537.7	+1.9%
sglang · memory	32	497.7	501.4	+0.7%	642.6	648.1	+0.9%
sglang · memory	64	574.5	581.3	+1.2%	774.6	812.6	+4.9%
vLLM · redis	8	247.3	240.1	−2.9%	298.5	286.6	−4.0%
vLLM · redis	16	258.9	255.8	−1.2%	315.5	307.2	−2.6%
vLLM · redis	32	298.7	293.8	−1.7%	369.3	362.4	−1.9%
vLLM · redis	64	373.7	366.1	−2.0%	486.8	502.9	+3.3%
vLLM · memory	8	243.1	240.6	−1.0%	288.1	289.4	+0.4%
vLLM · memory	16	258.5	256.5	−0.8%	312.6	310.0	−0.8%
vLLM · memory	32	297.8	293.0	−1.6%	369.2	363.3	−1.6%
vLLM · memory	64	370.4	367.7	−0.7%	496.3	488.0	−1.7%

ITL — inter-token latency (ms)

backend	c	HTTP p50	WS p50	Δp50	HTTP p99	WS p99	Δp99
sglang · redis	8	11.8	11.8	+0.3%	55.0	33.5	−39.1%
sglang · redis	16	13.3	13.8	+3.9%	69.5	50.6	−27.2%
sglang · redis	32	15.2	15.0	−1.4%	81.3	55.6	−31.6%
sglang · redis	64	21.0	18.8	−10.7%	98.1	74.5	−24.0%
sglang · memory	8	11.8	11.6	−2.0%	53.7	32.0	−40.5%
sglang · memory	16	14.1	13.9	−1.4%	64.4	50.3	−21.9%
sglang · memory	32	16.6	15.9	−4.3%	76.8	55.8	−27.3%
sglang · memory	64	19.2	19.5	+1.5%	95.7	72.5	−24.3%
vLLM · redis	8	10.7	10.7	−0.4%	23.3	12.6	−45.9%
vLLM · redis	16	11.2	11.1	−0.3%	24.8	13.6	−45.2%
vLLM · redis	32	13.0	13.3	+2.1%	29.3	15.5	−47.3%
vLLM · redis	64	15.5	15.6	+0.2%	39.0	22.5	−42.3%
vLLM · memory	8	10.7	10.7	−0.3%	23.1	12.7	−45.1%
vLLM · memory	16	11.2	11.1	−1.0%	25.0	13.6	−45.7%
vLLM · memory	32	12.9	13.3	+3.0%	29.5	15.3	−48.0%
vLLM · memory	64	15.4	15.6	+1.2%	39.6	22.4	−43.5%

Throughput

backend	c	HTTP req/s	WS req/s	Δ	HTTP tok/s	WS tok/s	Δ
sglang · redis	8	22.6	22.5	−0.4%	472	469	−0.6%
sglang · redis	16	38.5	37.7	−2.0%	801	786	−1.9%
sglang · redis	32	63.6	64.5	+1.4%	1326	1348	+1.6%
sglang · redis	64	105.6	103.6	−1.9%	2199	2164	−1.6%
sglang · memory	8	22.8	22.9	+0.3%	475	476	+0.3%
sglang · memory	16	38.1	37.8	−0.7%	795	793	−0.3%
sglang · memory	32	63.8	63.7	−0.2%	1332	1329	−0.2%
sglang · memory	64	108.4	106.5	−1.8%	2266	2222	−1.9%
vLLM · redis	8	31.6	32.2	+1.8%	659	670	+1.7%
vLLM · redis	16	59.5	60.2	+1.2%	1235	1255	+1.7%
vLLM · redis	32	102.9	103.3	+0.4%	2143	2144	+0.1%
vLLM · redis	64	163.6	166.6	+1.8%	3409	3472	+1.9%
vLLM · memory	8	32.0	32.0	+0.3%	667	665	−0.3%
vLLM · memory	16	59.5	60.4	+1.4%	1245	1254	+0.7%
vLLM · memory	32	103.8	103.4	−0.4%	2169	2143	−1.2%
vLLM · memory	64	164.7	165.9	+0.7%	3434	3454	+0.6%

ITL p99 is −22% to −48% for WS in every cell — smoother per-token delivery (HTTP/SSE coalesces tokens into bursts; WS flushes per frame). ITL p50 is at parity (mean −0.6%; the few +2–4% cells are run-to-run variance, e.g. c=64 swings −1% to −11%).
E2E p99 is sample-noisy at n=320 — single tail samples swing it (one +19.7% at sglang·redis c=64, one −8.9% at c=32); mean +0.6% = parity.
Throughput is at parity on both axes: req/s (shown) and output tok/s each within ±2% (output-tok/s mean +0.0%, max |Δ| 1.9%, no outliers). Absolute WS output rate scales ~470 tok/s (c=8) → ~3 470 tok/s (c=64), matching HTTP within ±2%. (Prompt/input-token throughput isn't separately recorded by the harness — total_tok_per_s equals output here; the noisier peak-1s burst metric is omitted.)

A100 · backend redis vs in-memory (8-turn, c=1) — the win is gateway-side

TTFT (ms), 8-turn chain, c=1; HTTP / WS raw with Δ.

Engine	Backend	HTTP TTFTp50	WS TTFTp50	Δp50	HTTP TTFTp99	WS TTFTp99	Δp99
sglang	redis	35.1	25.3	−27.7%	40.5	26.5	−34.6%
sglang	in-memory	35.2	25.7	−26.9%	41.1	31.7	−22.9%
vLLM	redis	32.3	22.1	−31.6%	38.4	25.8	−32.8%
vLLM	in-memory	32.3	22.1	−31.6%	34.8	24.0	−30.9%

Near-identical with no redis → the win is the gateway's per-turn reconstruction WS skips, not a redis effect.

A100 · 32B · gateway resource cost (chain=20, c=1; CPU = /proc utime+stime, aggregate)

Engine · backend	Gateway CPU/turn (HTTP→WS)	Worker CPU/turn (control)	redis CPU/turn
vLLM · redis	47.1 → 39.3 ms (−17%)	2356 → 2375 (parity)	20.3 → 14.9 ms
sglang · redis	50.5 → 39.8 ms (−21%)	54.5 → 52.4 (parity)	22.4 → 17.9 ms

Worker CPU unchanged → the saving is gateway-side. WS conversation cache adds sub-MB gateway RSS at this scale.

RTX 3060 · 3B — commodity reproduction (loopback, n=3, in-memory)

Metric	8-turn	16-turn
First-content p50 (ms)	47.2 → 47.1	47.3 → 47.2
Full-chain total (ms)	4 076 → 4 120 (+1.1%)	8 252 → 8 094 (−1.9%)

redis (24-turn): 112 HGETALL → 0; completed-turn 524 → 505 ms. Near-parity per-turn (3B + loopback); proves the mechanism + correctness (7/7 e2e smoke, sglang + vLLM) on commodity hardware.

Out of scope

Shared/durable connection cache — the cache here is in-process, per-connection, single-entry (latest response, dropped on disconnect); a reconnect (or, behind a load balancer, a turn landing on another replica) falls back to durable storage. Backing it with a shared store (e.g. redis) is possible but not implemented.
Remote-storage latency gain — on a remote history backend, HTTP's per-turn reads also cost network RTT that WS avoids; deployment-dependent, so not claimed here. All numbers above are on local backends.

Test Plan

cargo test — 1087 lib + 127 api green (incl. 22 WS protocol tests), 0 failures.
e2e WS smoke 7/7 on real inference, sglang AND vLLM gRPC workers: create+stream, same-socket + reconnect continuation, store=false not-chainable, conversation/invalid-JSON/unknown-event rejection, HTTP-worker → 501.
Output coherence on 32B; speed tests reproduced on A100-80GB + RTX 3060.

Checklist

cargo +nightly fmt clean
cargo clippy -p smg --all-targets — no findings in the changed files. The strict workspace gate (--workspace --all-targets --all-features -- -D warnings) trips only on pre-existing lints in crates this PR doesn't touch (reasoning_parser, benches, mesh), all present on main.
cargo test — 1087 lib / 127 api, 0 failures
Net-new tests (22 WS protocol) + e2e smoke (sglang + vLLM)
Docs — docs/reference/api/responses.md WebSocket Mode section

Refs: sgl-project/sglang#22366 (upstream gateway implementation; re-ported here).

Summary by CodeRabbit

Release Notes

New Features
- Added WebSocket Mode for GET /v1/responses with ordered multi-turn streaming, warmup (generate:false), and durable continuation support (store:true vs store:false).
- Enforced per-connection constraints (single in-flight response.create, capped session lifetime, concurrency handling) and improved continuation across reconnects.
Documentation
- Updated the Responses API reference for WebSocket protocol, event/payload shapes, persistence rules, and error events (including a Python example).
Bug Fixes
- Improved WebSocket framing to avoid double-framing, strengthened upstream error parsing, and surfaced worker error frames correctly.
Tests
- Added end-to-end WebSocket integration coverage for validation, concurrency, continuation, error cases, and handler panic recovery.

coderabbitai · 2026-06-18T00:19:16Z

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a WebSocket transport for the /v1/responses endpoint, enabling persistent multi-turn agentic conversations. A new ResponseEventSink trait decouples event emission from SSE/WS transports. ResponseStreamEventEmitter gains status-aware finalization and sink-generic methods for both non-MCP and MCP tool-loop streaming. The ws_responses module implements per-connection session state, lifetime management, concurrency control, and a protocol parser. GrpcWsResponsesExecutor implements the executor trait for gRPC backends with request validation, conversation history cache-first loading, and error mapping. Server routing registers GET /v1/responses as a WebSocket upgrade endpoint with middleware wiring and model validation. Comprehensive integration tests and API documentation complete the implementation.

Changes

WebSocket Responses Transport

Layer / File(s)	Summary
`ResponseEventSink` trait and SSE/WS implementations `model_gateway/src/routers/grpc/common/responses/streaming.rs`	Introduces `ResponseEventSink` trait with `send_event()/send_raw_json()` methods. Implements `SseResponseEventSink` (SSE framing via `event:\ndata:\n\n`) and `WsResponseEventSink` (JSON text frames via bounded `try_send` with backpressure/disconnect errors). Adds blanket impl for `mpsc::UnboundedSender` to keep existing SSE call sites compatible. Unit test validates WS backpressure behavior and recovery after draining.
ResponseStreamEventEmitter status-aware finalization and sink migration `model_gateway/src/routers/grpc/common/responses/streaming.rs`	Adds `emit_completed_with_status()` (terminal event without draining state) and `finalize_with_status()` (persistence with explicit `ResponseStatus`). Rewires `emit_reasoning_item`, `process_chunk`, `send_event`, `send_event_best_effort`, `emit_error`, `emit_mcp_list_tools_sequence` to accept generic sink instead of SSE senders. Existing `finalize()` delegates to `finalize_with_status(..., Completed)`.
Regular responses streaming refactor for SSE and WS `model_gateway/src/routers/grpc/regular/responses/streaming.rs`	Refactors `process_and_transform_sse_stream` to call `drive_non_mcp_stream(drain_completed=true)`, which uses sink, prevents double-framing on non-chat events via `send_raw_json`, and returns materialized `ResponsesResponse`. Adds `execute_non_mcp_stream_with_sink` and `execute_tool_loop_streaming_with_sink` for WS with `drain_completed=false`. MCP tool-loop becomes sink-generic with explicit `(terminal_usage, terminal_status)` exit tuples for all paths (max-tool-calls, function-calls, early exits). Adds `StreamingResponseAccumulator::status()` helper and refactors finalization. Tests add `CollectingSink` and validate response-ID stamping and JSON passthrough framing.
WS session state, executor trait, and protocol loop `model_gateway/src/routers/ws_responses.rs`	Defines `CachedWsResponse` (response + input items for continuation), `WsClientError` builder, `WsResponseCreateOptions`, and `WsResponsesExecutor` trait. Implements `serve_responses_ws_with_config`: bounded outbound channel, spawned writer task, 60-minute lifetime timeout, main receive loop (text only, reject binary/unknown), ping/pong echo. Implements `handle_text_event` (parse, validate, single in-flight slot guard, executor spawn with panic conversion, cache update). Implements `acquire_request_slot` with bounded wait and `Notify` logic for missed-wakeup protection. Adds `parse_client_event` (nested/inlined `response.create` payloads, warmup probes), error senders. Unit tests cover slot acquisition, concurrent rejection, missed-notify recovery.
Request validation, input normalization, and cache-first conversation history `model_gateway/src/routers/responses_validation.rs`, `model_gateway/src/routers/grpc/regular/responses/common.rs`	Adds `normalize_and_validate_responses_request` with conditional relaxation for `function_call_output`-only continuations when `previous_response_id` is present. Adds `normalize_request_input_items` (flattens `ResponsesRequest.input` to items). Adds `load_conversation_history_with_cache`: checks `CachedWsResponse` first, otherwise loads durable storage with configurable strict mode; `strict=false` returns `bad_request` on missing, `strict=true` returns `not_found`. Scopes durable reads with `data_connector::with_request_context`. Existing `load_conversation_history` delegates with `strict=false`.
GrpcWsResponsesExecutor and WebSocket-specific error handling `model_gateway/src/routers/grpc/regular/responses/websocket.rs`, `model_gateway/src/routers/grpc/regular/responses/mod.rs`	Implements `GrpcWsResponsesExecutor::execute_response_create`: forces streaming, validates (rejects `conversation`, Harmony models), loads cached conversation history, branches on `generate` (warmup for `false`, streaming for MCP/non-MCP), persists, returns `CachedWsResponse`. Adds `response_to_ws_error` (extracts error from headers, parses JSON body capped at 1 MiB, preserves type, falls back to status-based message, attaches `previous_response_id` for not-found). Adds `warmup_response_create`, `send_ws_message` (`try_send`, 499 on disconnect), `WsClientErrorExt`. Unit tests validate error-body parsing. Registers websocket submodule export.
HTTP server endpoint, router wiring, middleware refactoring, and dependencies `model_gateway/src/server.rs`, `model_gateway/src/routers/mod.rs`, `model_gateway/src/routers/grpc/router.rs`, `model_gateway/src/routers/router_manager.rs`, `model_gateway/src/middleware/storage_context.rs`, `model_gateway/Cargo.toml`	Adds `v1_responses_ws` handler (validates `model` param, 400 on missing). Registers `GET /v1/responses` in realtime route group with shared middleware (auth/admission/tenant-meta, no WASM). Enables `TCP_NODELAY` via `NoDelayAcceptor` on non-TLS bind. Adds default `route_responses_ws` to `RouterTrait` (501). Routes through `RouterManager` (404 if unavailable). Exports `responses_validation` and `ws_responses` modules. Implements `route_responses_ws_impl` in `GrpcRouter` (worker check, Harmony 501, WebSocketUpgrade extraction, ResponsesContext rebuild via `build_storage_request_context`, GrpcWsResponsesExecutor construction, serve_responses_ws delegation). Adds `storage_context_headers` field to `GrpcRouter`. Refactors `build_storage_request_context` to accept `HashMap<String, String>` instead of `RouterConfig`. Updates `storage_context_middleware` to use refactored function and tests. Moves `validator` from dev-dependencies to dependencies.
Integration tests and WebSocket API documentation `model_gateway/tests/api/responses_ws_test.rs`, `model_gateway/tests/api/mod.rs`, `docs/reference/api/responses.md`	Adds ~1,597 lines of tests via fake executors (StubWsExecutor, DelayedReturnWsExecutor, FunctionCallWsExecutor, FailedResponseWsExecutor, PanicOnceWsExecutor) and router stubs, plus SemanticWsExecutor with in-memory durable cache. Tests cover protocol (non-upgrade rejected, unsupported routers 501, unknown events/binary/invalid JSON rejected, ping/pong, 60-min session lifetime), event ordering (response.create, function calls), payload shapes (nested/inlined), concurrency rejection, continuation with `store:true/false`, warmup, caching, parameter validation, event_id echo, and panic recovery. Docs describe WebSocket Mode (persistent multi-turn, gRPC regular only), connection setup (`GET /v1/responses?model=...`), `response.create` format (mirrors POST body, no `stream`/`background`), multi-turn continuation and store semantics, constraints (single in-flight, 60-min lifetime, conversation rejected), error event delivery, and Python websockets example.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Server as server.rs v1_responses_ws
  participant GrpcRouter
  participant Session as serve_responses_ws_with_config
  participant Executor as GrpcWsResponsesExecutor
  participant Emitter as ResponseStreamEventEmitter
  participant Sink as WsResponseEventSink

  Client->>Server: GET /v1/responses?model=... (WS Upgrade)
  Server->>GrpcRouter: route_responses_ws(req, model)
  GrpcRouter->>GrpcRouter: check worker availability
  GrpcRouter->>GrpcRouter: reject Harmony → 501
  GrpcRouter->>Session: WebSocket upgrade with executor
  Session->>Session: spawn writer task (bounded outbound_tx)
  Session->>Session: spawn 60-min lifetime timeout
  Client->>Session: {"type":"response.create", "response":{...}}
  Session->>Session: acquire_request_slot (single in-flight guard)
  Session->>Executor: execute_response_create
  Executor->>Executor: normalize + load_conversation_history_with_cache
  Executor->>Emitter: stream response events
  Emitter->>Sink: send_event (response.created, deltas, completed)
  Sink->>Sink: try_send JSON text frame
  Session-->>Client: WebSocket text frames
  Executor-->>Session: Ok(CachedWsResponse)
  Session->>Session: cache response, release in-flight slot
  Client->>Session: {"type":"response.create", "previous_response_id":"resp_…"}
  Session->>Executor: execute_response_create(cached=Some(...))
  Executor-->>Client: next turn events
  Client->>Client: disconnect or 60-min timeout
  Session-->>Client: close frame

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

lightseekorg/smg#807: Introduces configurable storage_context_headers and storage context middleware, which this PR builds upon for WebSocket upgrade request-context rebuilding outside task-local middleware scope.
lightseekorg/smg#713: Makes model_id required (&str instead of Option<&str>) in the routing interface; the new WebSocket Responses routing entrypoints depend on this signature change.
lightseekorg/smg#690: Related router abstraction modifications for WebSocket endpoint dispatch via trait methods (this PR adds route_responses_ws while that PR adds route_realtime_ws).

Suggested labels

openai

Suggested reviewers

CatherineSue
key4ng
slin1237
claude

Poem

🐰 Hop, hop — a new wire is spun,
WebSockets carry each turn, one by one.
response.create flies down the line,
previous_response_id threads through time fine.
Sixty minutes of life, then a gentle close —
The rabbit types fast, and the stream always flows! 🌟

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the main change: adding WebSocket transport for the Responses API with documented performance improvements.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

gemini-code-assist

Code Review

This pull request implements a WebSocket transport mode for the Responses API (GET /v1/responses) to support persistent, multi-turn agentic workflows. It decouples the streaming event emission from the wire format using a transport-neutral ResponseEventSink trait, adds connection-local caching to bypass durable storage for same-socket continuations, and disables Nagle's algorithm to ensure low-latency streaming. Feedback on the implementation suggests optimizing the acquire_request_slot function to eliminate a missed-notify race condition and its associated 50ms delay by registering the Notified future while holding the lock. Additionally, it is recommended to simplify the duration formatting helper by using the standard modulo operator (%) instead of relying on external traits for divisibility checks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-18T00:21:35Z

+    loop {
+        let notify = {
+            let mut guard = session.lock().await;
+            if !guard.active_request {
+                guard.active_request = true;
+                return true;
+            }
+            guard.request_done.clone()
+        };
+
+        let remaining = deadline.saturating_duration_since(tokio::time::Instant::now());
+        if remaining.is_zero() {
+            return false;
+        }
+        // Wake on completion OR after `remaining` (catches a missed notify);
+        // either way the loop re-checks `active_request` under the lock.
+        let _ = tokio::time::timeout(remaining, notify.notified()).await;
+    }


The current implementation of acquire_request_slot uses a loop with a timeout to mitigate a missed-notify race condition where a completion event fires between dropping the lock and registering the .notified() future.

We can completely eliminate this race condition and the associated 50ms delay by registering the Notified future while still holding the lock, and then awaiting it outside the lock. Since the waiter is registered before the lock is dropped, any subsequent notify_waiters() call is guaranteed to wake up the future immediately without any missed wakeups.

loop { let notify; let notified = { let mut guard = session.lock().await; if !guard.active_request { guard.active_request = true; return true; } notify = guard.request_done.clone(); notify.notified() }; let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); if remaining.is_zero() { return false; } // Since notified was registered while holding the lock, we are guaranteed // to receive any wakeup triggered after the lock is dropped, avoiding the // missed-notify race condition entirely. let _ = tokio::time::timeout(remaining, notified).await; }

Fixed in 388405f. Notify::notified() only registers the waiter on first poll, not at construction, so building it inside the lock would still miss a notify_waiters() fired before that poll. We arm it with Notified::enable() before re-checking the slot, which closes the window.

gemini-code-assist · 2026-06-18T00:21:35Z

+    if seconds < 3_600 && seconds.is_multiple_of(60) {
+        return pluralize(u128::from(seconds / 60), "minute");
+    }
+
+    if seconds.is_multiple_of(3_600) {
+        return pluralize(u128::from(seconds / 3_600), "hour");
+    }


Using seconds.is_multiple_of(60) and seconds.is_multiple_of(3_600) relies on external traits (like num::Integer) being in scope, which can lead to maintainability issues or compilation errors if dependencies change.

We can simplify this by using the standard modulo operator (%), which is self-contained, highly idiomatic, and does not require any external traits.

if seconds < 3_600 && seconds % 60 == 0 { return pluralize(u128::from(seconds / 60), "minute"); } if seconds % 3_600 == 0 { return pluralize(u128::from(seconds / 3_600), "hour"); }

is_multiple_of here is the inherent unsigned-integer method, so no external trait (num::Integer) is in scope or required and there is no dependency-drift risk. Keeping it intentionally: it reads more clearly than % == 0, and Clippy's manual_is_multiple_of lint flags the % form as the thing to replace with exactly this method, so reverting would trip it. Leaving as-is.

Add WebSocket transport for GET /v1/responses (gRPC regular worker). Clients upgrade and send response.create over one persistent socket; previous_response_id resolves from a connection-local cache before falling back to durable storage, eliminating the per-turn history hydration (HGETALL chain-walk) that stateless HTTP pays on every continuation. Reuses the existing streaming pipeline via a transport-neutral ResponseEventSink (no SSE re-parse); HTTP-worker/Harmony return 501. Refs: sgl-project/sglang#22366 Signed-off-by: Venkat Raman <vraman2811@gmail.com>

21 stub-based protocol tests: streaming, rejection matrix, store semantics, same-socket and reconnect continuation, connection lifetime, plus negative controls. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

Add a WebSocket Mode section to the Responses API reference: connection upgrade, the response.create envelope, multi-turn continuation via the connection-local cache, constraints (gRPC-regular-only, single in-flight, 60-min cap, rejection matrix), error events, and a client example. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

…doff acquire_request_slot registered the request_done waiter via notify.notified() only after dropping the session lock, so a notify_waiters() firing in that window was lost (Notify stores no permit). The prior loop masked this with a 50ms timeout re-check, so a missed wakeup cost up to ACTIVE_REQUEST_HANDOFF_TIMEOUT before the next turn's response.create was admitted. Arm the waiter with Notified::enable() before re-reading active_request each iteration, so any completion after the lock is released is guaranteed to wake us. The handoff now resolves on the real completion (sub-ms) instead of stalling for the timeout; the bounded timeout is retained only as a safety re-check, and a genuinely concurrent second create still rejects at the deadline. A 5000-cycle handoff micro-benchmark drops acquire-latency max from ~50.7ms (one missed-notify stall) to ~0.37ms, p50 unchanged (7us). Existing slot-handoff tests still pass. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 388405f9fa

ℹ️ 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".

chatgpt-codex-connector · 2026-06-18T21:02:27Z

+            // group's middleware (auth + admission + tenant-meta, NO WASM — the
+            // WASM OnResponse phase drops the upgrade-future extensions). It
+            // coexists with `POST /v1/responses` (SSE) via the final `.merge()`.
+            .route("/v1/responses", get(v1_responses_ws)),


Preserve storage context on Responses WS route

When storage_context_headers is configured, this new route is in realtime_routes, whose middleware stack below only applies tenant metadata and auth, not storage_context_middleware like the protected POST /v1/responses route. The WS gRPC handler then captures smg_data_connector::current_request_context() for persistence and previous_response_id loading, so it will be None here and store: true WebSocket responses are written/read without the tenant/user hook context that scopes normal Responses storage.

Useful? React with 👍 / 👎.

Fixed in d2053467. The WS upgrade rebuilds the storage request-context from the request headers and threads it into the WS executor (the get_response_chain read is wrapped in with_request_context). No-op when storage_context_headers is unset, matching the middleware.

chatgpt-codex-connector · 2026-06-18T21:02:27Z

-                    if tx.send(Ok(Bytes::from(format!("{event}\n\n")))).is_err() {
-                        return Err("Client disconnected".to_string());
-                    }
+                    sink.send_raw_json(event)


Avoid double-framing non-chat SSE events

In this branch event is still the full SSE line (for example data: {"error":...}), but SseResponseEventSink::send_raw_json prepends its own data: framing. When the upstream chat stream emits a non-chat event such as an error, HTTP streaming clients now receive data: data: {...} instead of the pass-through event that the old code sent, so they cannot parse the error frame correctly.

Useful? React with 👍 / 👎.

Fixed in a87cc9f6: forward the stripped json_str, not the data:-prefixed line, so SSE is single-framed and WS gets clean JSON. Added a regression test.

chatgpt-codex-connector · 2026-06-18T21:02:27Z

+        let sink = WsResponseEventSink::new(outbound_tx);
+
+        let final_response = if has_mcp_tools {
+            execute_tool_loop_streaming_with_sink(


Persist store=true MCP WebSocket responses

When has_mcp_tools is true, this branch only returns the materialized response to the connection cache; unlike the non-MCP WebSocket path and warmup path, it never calls persist_response_if_needed. For a tool-using response.create with the default store: true, same-socket continuation works from the cache, but after reconnect GET /v1/responses/{id} or previous_response_id fallback to durable storage will fail even though the request asked to store the response.

Useful? React with 👍 / 👎.

Fixed in 01c8b295. persist_response_if_needed now runs once after the MCP/non-MCP branch, so store=true MCP responses are durably persisted and reconnect GET /v1/responses/{id} / previous_response_id resolution works for them too.

coderabbitai

Actionable comments posted: 8

🤖 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 `@docs/reference/api/responses.md`:
- Around line 388-389: The documentation table entry for malformed payload error
handling incorrectly specifies the error code as `invalid_request`, but the WS
protocol tests show that malformed JSON is actually handled with `invalid_json`.
Update the table row that currently states "Malformed payload | Rejected with an
`error` event (`invalid_request`)" to use the correct error code `invalid_json`
to match the actual implementation behavior, or if there are different types of
malformed payloads with different error codes, clarify the distinction in the
documentation.

In `@model_gateway/src/routers/grpc/regular/responses/streaming.rs`:
- Around line 566-568: The MCP streaming response handling in the SSE and WS
code paths does not persist the materialized response before discarding it,
breaking durability for responses with store=true. Add a call to
persist_response_if_needed for the materialized MCP response before it is
returned or discarded, following the same pattern used in
execute_non_mcp_stream_with_sink. This fix needs to be applied at all three
affected locations: the error path around line 566-568, the response handling
block around lines 636-645, and the WS wrapper response return around lines
1099-1102.
- Around line 307-315: The `execute_chat` function call in the streaming
response path is passing `None` as the tenant metadata argument, which drops the
resolved tenant identity for WebSocket responses and can bypass tenant
attribution, quotas, or billing. Instead of passing `None`, pass the resolved
tenant metadata (which should be available from the request context or earlier
processing steps) just as the SSE path does. This applies to both the non-MCP
WebSocket path and the MCP WebSocket path (around lines 623-628), ensuring
tenant information is properly threaded through the `execute_chat` execution.
- Around line 787-791: Terminal response events are not being emitted for all
exits from the MCP loop in the streaming response handler. Specifically, the
break statements at max_tool_calls (around line 787) and function-tool-call
exits (around lines 1048-1050 and 1086-1102) bypass the only response.completed
emission block, causing WebSocket clients to hang waiting for terminal events.
Move the response.completed emission logic outside and after the main loop, so
that it executes for all possible break exits that return (terminal_usage,
terminal_status) tuples, ensuring every loop exit emits a terminal response
event.
- Around line 456-461: In the status method, the match statement on
self.finish_reason.as_deref() currently groups "stop" and "length" together to
return ResponseStatus::Completed. Separate these match arms so that Some("stop")
continues to return ResponseStatus::Completed while Some("length") now returns
ResponseStatus::Incomplete instead, since a truncated response due to token
limits should be marked as incomplete rather than completed.
- Around line 243-247: In the error handling block within the streaming response
handler (the Err(_) match arm around line 243-247), the sink.send_raw_json
method is being called with the raw event variable which still contains the SSE
data: prefix. This causes double-prefixing for SSE responses and sends non-JSON
text to WebSocket clients. Replace the event argument in the sink.send_raw_json
call with json_str instead, which contains the extracted JSON payload without
the SSE prefix.

In `@model_gateway/src/routers/grpc/regular/responses/websocket.rs`:
- Around line 191-207: The response_to_ws_error function extracts error.code and
error.message from the parsed error JSON but ignores error.type, causing mapped
server errors to default to invalid_request_error. Extract the error.type field
from parsed_error using the same and_then pattern applied to error.code and
error.message (accessing the "type" key and converting to string), then include
this extracted type when constructing or configuring the WsClientError object
via an appropriate builder method or constructor parameter alongside error_code
and error_message.

In `@model_gateway/tests/api/responses_ws_test.rs`:
- Around line 798-802: The assertion in the test function currently uses a loose
check with !response.status().is_success() which accepts any non-2xx status
code, allowing the test to pass even on unrelated failures like 500 errors.
Replace this broad assertion with explicit checks for the expected HTTP status
codes that should be returned when upgrade headers are missing (typically 400
Bad Request or 426 Upgrade Required). This ensures the test specifically
validates the upgrade rejection logic rather than just any non-success response.

🪄 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: ASSERTIVE

Plan: Pro

Run ID: fbc7d6a0-d54d-454b-a589-831bcb652d45

📥 Commits

Reviewing files that changed from the base of the PR and between a18bbdd and 388405f.

📒 Files selected for processing (15)

docs/reference/api/responses.md
model_gateway/Cargo.toml
model_gateway/src/routers/grpc/common/responses/streaming.rs
model_gateway/src/routers/grpc/regular/responses/common.rs
model_gateway/src/routers/grpc/regular/responses/mod.rs
model_gateway/src/routers/grpc/regular/responses/streaming.rs
model_gateway/src/routers/grpc/regular/responses/websocket.rs
model_gateway/src/routers/grpc/router.rs
model_gateway/src/routers/mod.rs
model_gateway/src/routers/responses_validation.rs
model_gateway/src/routers/router_manager.rs
model_gateway/src/routers/ws_responses.rs
model_gateway/src/server.rs
model_gateway/tests/api/mod.rs
model_gateway/tests/api/responses_ws_test.rs

coderabbitai · 2026-06-18T21:07:51Z

+    let chat_response = ctx
+        .pipeline
+        .execute_chat(
+            chat_request,
+            headers,
+            model_id.unwrap_or_else(|| original_request.model.clone()),
+            ctx.components.clone(),
+            None,
+        )


⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Thread tenant metadata through the WS streaming paths.

The non-MCP WS path calls execute_chat with None, and the MCP WS path synthesizes TenantIdentity::Anonymous. That drops the resolved tenant/charge identity for WebSocket Responses while the SSE path passes TenantRequestMeta into execute_chat, which can bypass tenant attribution, quotas, or billing.

Also applies to: 623-628

🤖 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/regular/responses/streaming.rs` around lines 307 - 315, The `execute_chat` function call in the streaming response path is passing `None` as the tenant metadata argument, which drops the resolved tenant identity for WebSocket responses and can bypass tenant attribution, quotas, or billing. Instead of passing `None`, pass the resolved tenant metadata (which should be available from the request context or earlier processing steps) just as the SSE path does. This applies to both the non-MCP WebSocket path and the MCP WebSocket path (around lines 623-628), ensuring tenant information is properly threaded through the `execute_chat` execution.

Inert today: nothing reads the threaded tenant_request_meta (every reference into the pipeline is a write; the only tenant consumer, priority_admission_middleware, reads RouteRequestMeta from the request extensions, and request_charge_id is never metered), so there is no WS-vs-SSE attribution difference. It is a latent gap, though, so we are tracking a follow-up to thread it through the WS executors before any per-tenant billing/quota lands.

@Venkat2811, thanks for the detailed context — that's a clear explanation. The concern is inert today since execute_chat's tenant_request_meta parameter is write-only from the pipeline's perspective and priority_admission_middleware resolves tenant identity independently from RouteRequestMeta in the request extensions, making the WS/SSE paths functionally equivalent right now.

Would you like me to open a GitHub tracking issue for threading TenantRequestMeta through the WS executors (execute_non_mcp_stream_with_sink, execute_tool_loop_streaming_with_sink) before per-tenant billing/quota lands, so it doesn't get lost?

✏️ Learnings added

Learnt from: Venkat2811 Repo: lightseekorg/smg PR: 1770 File: model_gateway/src/routers/grpc/regular/responses/streaming.rs:311-319 Timestamp: 2026-06-19T00:32:57.446Z Learning: In model_gateway (lightseekorg/smg), the `tenant_request_meta` argument passed to `pipeline.execute_chat(...)` is currently inert/write-only from the pipeline's perspective. The only tenant consumer, `priority_admission_middleware`, reads `RouteRequestMeta` from request extensions — not from this parameter — and `request_charge_id` is not metered. As a result, passing `None` vs a real `TenantRequestMeta` to `execute_chat` produces no observable WS-vs-SSE attribution difference today. This is a tracked latent gap: tenant metadata should be threaded through WS executors (`execute_non_mcp_stream_with_sink`, `execute_tool_loop_streaming_with_sink`) before any per-tenant billing or quota enforcement lands.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used

Learnt from: slin1237 Repo: lightseekorg/smg PR: 447 File: model_gateway/src/routers/grpc/client.rs:312-328 Timestamp: 2026-02-17T20:30:27.647Z Learning: Actionable guideline: In model_gateway gRPC metadata discovery (specifically in model_gateway/src/routers/grpc/...), verify how keys are handled for different proto sources. SGLang uses short-form keys (tp_size, dp_size, pp_size) via pick_prost_fields() without normalization, while vLLM/TRT-LLM use long-form keys (tensor_parallel_size, pipeline_parallel_size) that pass through flat_labels() and are normalized by normalize_grpc_keys() in discover_metadata.rs after model_info.to_labels() and device/server_info.to_labels(). Ensure reviewers check that the code paths correctly reflect these normalization rules and that tests cover both code paths.

Learnt from: XinyueZhang369 Repo: lightseekorg/smg PR: 399 File: protocols/src/interactions.rs:505-509 Timestamp: 2026-02-19T03:08:50.192Z Learning: In code reviews for Rust projects using the validator crate (v0.20.0), ensure that custom validation functions for numeric primitive types (e.g., f32, i32, u32, i16, etc.) accept the value by value, not by reference. Example: fn validate(value: f32) { ... }. The validator derive macro has a hardcoded list of numeric types that are passed by value, while all other types are passed by reference. Apply this guideline whenever validating numeric fields to align with the derive macro behavior.

Learnt from: slin1237 Repo: lightseekorg/smg PR: 489 File: model_gateway/src/core/token_bucket.rs:58-63 Timestamp: 2026-02-21T02:30:51.443Z Learning: For lint-only/Clippy enforcement PRs in this repository, avoid introducing behavioral changes (e.g., new input validation or logic changes). Treat such PRs as non-functional changes and plan a separate follow-up issue/PR for hardening or behavior changes. This applies broadly to Rust files across the repo; during review, focus on lint/style corrections and clearly note any intentional exceptions.

Learnt from: slin1237 Repo: lightseekorg/smg PR: 489 File: protocols/src/responses.rs:928-931 Timestamp: 2026-02-21T02:36:00.882Z Learning: In Rust code across the repository, use the marker INVARIANT: to document assumptions in safe code. Reserve SAFETY: for explaining why unsafe blocks are sound. This improves clarity of invariants and safety reasoning. Example reference: protocols/src/responses.rs near validate_tool_choice_with_tools().

Learnt from: slin1237 Repo: lightseekorg/smg PR: 489 File: mesh/src/sync.rs:83-83 Timestamp: 2026-02-21T02:37:01.416Z Learning: General Rust formatting rule: format! with implicit captures only supports simple identifiers, not full expressions like {state.model_id}. For cases where you want to interpolate a field or expression, bind the value first and interpolate the binding, e.g., let model_id = &state.model_id; and then use format!("policy:{}", model_id). In the specific file mesh/src/sync.rs, prefer format!("policy:{}", state.model_id) or bind to a local variable if you need named interpolation, to keep clarity and avoid unintended captures.

Learnt from: zhaowenzi Repo: lightseekorg/smg PR: 807 File: model_gateway/src/middleware.rs:61-81 Timestamp: 2026-03-18T21:32:00.041Z Learning: In Rust code using the http crate, HeaderMap::get() is effectively case-insensitive because HeaderName normalizes keys to lowercase on insertion and lookup. Do not require or perform explicit .to_lowercase() before HeaderMap::get() calls. Mark as not a concern for case-sensitivity in lookups; only consider normalization when inserting or comparing via HeaderName, not in lookups.

Learnt from: key4ng Repo: lightseekorg/smg PR: 867 File: tui/src/app.rs:798-813 Timestamp: 2026-03-22T20:13:55.778Z Learning: In this repo (lightseekorg/smg), treat the workspace `Cargo.toml`’s `package.rust-version` (MSRV) as the source of truth (e.g., `rust-version = "1.85"`). When reviewing Rust changes, do not flag usage of Rust language/library features that were stabilized on or before the MSRV (e.g., `Option::is_none_or`, stabilized in 1.82, is compatible with an MSRV of 1.85). Always verify the MSRV from the workspace `Cargo.toml` rather than relying on issue templates.

Learnt from: CatherineSue Repo: lightseekorg/smg PR: 937 File: model_gateway/src/core/worker.rs:0-0 Timestamp: 2026-03-27T03:20:19.917Z Learning: When calling `worker.record_outcome(status_code: u16)` (the unified Circuit Breaker outcome recording API), it’s valid to pass *synthetic* HTTP status codes for transport/connection errors where no real HTTP response was received. For example, callers may pass `502` (send error), `504` (timeout), or other appropriate `502/503/504`-style synthetic codes to preserve CB feedback. Do not flag these calls as incorrect usage of `record_outcome`. Health checks should still handle reachability separately.

coderabbitai · 2026-06-18T21:07:51Z

+    fn status(&self) -> ResponseStatus {
+        match self.finish_reason.as_deref() {
+            Some("stop") | Some("length") => ResponseStatus::Completed,
+            Some("tool_calls") => ResponseStatus::InProgress,
+            Some("failed") | Some("error") => ResponseStatus::Failed,
+            _ => ResponseStatus::Completed,


⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Map finish_reason="length" to Incomplete.

Line 458 reports truncated generations as Completed, even though the response status domain includes Incomplete. This hides max-token truncation from WS completion status and cached responses.

🐛 Proposed status mapping

match self.finish_reason.as_deref() { - Some("stop") | Some("length") => ResponseStatus::Completed, + Some("stop") => ResponseStatus::Completed, + Some("length") => ResponseStatus::Incomplete, Some("tool_calls") => ResponseStatus::InProgress, Some("failed") | Some("error") => ResponseStatus::Failed, _ => ResponseStatus::Completed,

🤖 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/regular/responses/streaming.rs` around lines 456 - 461, In the status method, the match statement on self.finish_reason.as_deref() currently groups "stop" and "length" together to return ResponseStatus::Completed. Separate these match arms so that Some("stop") continues to return ResponseStatus::Completed while Some("length") now returns ResponseStatus::Incomplete instead, since a truncated response due to token limits should be marked as incomplete rather than completed.

finish_reason="length" maps to completed exactly as upstream #22366 does, and IncompleteReason::MaxOutputTokens is constructed nowhere in either repo (the type does not exist upstream at all), so this is a pre-existing shared gap rather than something introduced by this re-port. We would rather surface length as incomplete across SSE, WS and non-streaming together in a focused follow-up than diverge from the referenced PR here.

@Venkat2811, understood — keeping parity with upstream PR #22366 while deferring the length → Incomplete lift to a cross-cutting follow-up makes sense. I'll withdraw this comment.

Would you like me to open a GitHub issue to track the work of mapping finish_reason="length" to ResponseStatus::Incomplete consistently across the SSE, WS, and non-streaming paths?

🧠 Learnings used

Learnt from: slin1237 Repo: lightseekorg/smg PR: 447 File: model_gateway/src/routers/grpc/client.rs:312-328 Timestamp: 2026-02-17T20:30:27.647Z Learning: Actionable guideline: In model_gateway gRPC metadata discovery (specifically in model_gateway/src/routers/grpc/...), verify how keys are handled for different proto sources. SGLang uses short-form keys (tp_size, dp_size, pp_size) via pick_prost_fields() without normalization, while vLLM/TRT-LLM use long-form keys (tensor_parallel_size, pipeline_parallel_size) that pass through flat_labels() and are normalized by normalize_grpc_keys() in discover_metadata.rs after model_info.to_labels() and device/server_info.to_labels(). Ensure reviewers check that the code paths correctly reflect these normalization rules and that tests cover both code paths.

Learnt from: XinyueZhang369 Repo: lightseekorg/smg PR: 399 File: protocols/src/interactions.rs:505-509 Timestamp: 2026-02-19T03:08:50.192Z Learning: In code reviews for Rust projects using the validator crate (v0.20.0), ensure that custom validation functions for numeric primitive types (e.g., f32, i32, u32, i16, etc.) accept the value by value, not by reference. Example: fn validate(value: f32) { ... }. The validator derive macro has a hardcoded list of numeric types that are passed by value, while all other types are passed by reference. Apply this guideline whenever validating numeric fields to align with the derive macro behavior.

Learnt from: slin1237 Repo: lightseekorg/smg PR: 489 File: model_gateway/src/core/token_bucket.rs:58-63 Timestamp: 2026-02-21T02:30:51.443Z Learning: For lint-only/Clippy enforcement PRs in this repository, avoid introducing behavioral changes (e.g., new input validation or logic changes). Treat such PRs as non-functional changes and plan a separate follow-up issue/PR for hardening or behavior changes. This applies broadly to Rust files across the repo; during review, focus on lint/style corrections and clearly note any intentional exceptions.

Learnt from: slin1237 Repo: lightseekorg/smg PR: 489 File: protocols/src/responses.rs:928-931 Timestamp: 2026-02-21T02:36:00.882Z Learning: In Rust code across the repository, use the marker INVARIANT: to document assumptions in safe code. Reserve SAFETY: for explaining why unsafe blocks are sound. This improves clarity of invariants and safety reasoning. Example reference: protocols/src/responses.rs near validate_tool_choice_with_tools().

Learnt from: slin1237 Repo: lightseekorg/smg PR: 489 File: mesh/src/sync.rs:83-83 Timestamp: 2026-02-21T02:37:01.416Z Learning: General Rust formatting rule: format! with implicit captures only supports simple identifiers, not full expressions like {state.model_id}. For cases where you want to interpolate a field or expression, bind the value first and interpolate the binding, e.g., let model_id = &state.model_id; and then use format!("policy:{}", model_id). In the specific file mesh/src/sync.rs, prefer format!("policy:{}", state.model_id) or bind to a local variable if you need named interpolation, to keep clarity and avoid unintended captures.

Learnt from: zhaowenzi Repo: lightseekorg/smg PR: 807 File: model_gateway/src/middleware.rs:61-81 Timestamp: 2026-03-18T21:32:00.041Z Learning: In Rust code using the http crate, HeaderMap::get() is effectively case-insensitive because HeaderName normalizes keys to lowercase on insertion and lookup. Do not require or perform explicit .to_lowercase() before HeaderMap::get() calls. Mark as not a concern for case-sensitivity in lookups; only consider normalization when inserting or comparing via HeaderName, not in lookups.

Learnt from: key4ng Repo: lightseekorg/smg PR: 867 File: tui/src/app.rs:798-813 Timestamp: 2026-03-22T20:13:55.778Z Learning: In this repo (lightseekorg/smg), treat the workspace `Cargo.toml`’s `package.rust-version` (MSRV) as the source of truth (e.g., `rust-version = "1.85"`). When reviewing Rust changes, do not flag usage of Rust language/library features that were stabilized on or before the MSRV (e.g., `Option::is_none_or`, stabilized in 1.82, is compatible with an MSRV of 1.85). Always verify the MSRV from the workspace `Cargo.toml` rather than relying on issue templates.

Learnt from: CatherineSue Repo: lightseekorg/smg PR: 937 File: model_gateway/src/core/worker.rs:0-0 Timestamp: 2026-03-27T03:20:19.917Z Learning: When calling `worker.record_outcome(status_code: u16)` (the unified Circuit Breaker outcome recording API), it’s valid to pass *synthetic* HTTP status codes for transport/connection errors where no real HTTP response was received. For example, callers may pass `502` (send error), `504` (timeout), or other appropriate `502/503/504`-style synthetic codes to preserve CB feedback. Do not flag these calls as incorrect usage of `record_outcome`. Health checks should still handle reachability separately.

…n every MCP tool-loop exit drive_non_mcp_stream forwarded the data:-prefixed line to send_raw_json, which re-frames as `data: data: …` for SSE and ships non-JSON text to WS clients; forward the stripped json_str instead (matches upstream #22366). The MCP tool loop only emitted response.completed on the no-tool-calls exit; the max-tool-calls and function-tool-call breaks returned without a terminal frame, hanging streaming clients that block on response.completed/error. Emit the terminal event once after the loop for all exits. Persistence for the non-MCP WS path moves to the caller so both paths persist identically (see websocket.rs). Adds a regression test (and CollectingSink now records send_raw_json payloads). Signed-off-by: Venkat Raman <vraman2811@gmail.com>

…error.type execute_response_create now calls persist_response_if_needed once after the MCP/non-MCP branch, so store=true MCP (tool-using) responses are durably persisted too — reconnect GET /v1/responses/{id} and durable previous_response_id resolution previously failed for them (only the non-MCP path persisted). Mirrors upstream #22366's after-branch persist. response_to_ws_error now carries the upstream error.type when present instead of flattening every mapped server error to the default invalid_request_error. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

… tighten upgrade-rejection assertion Malformed JSON is rejected with code `invalid_json` (schema-invalid payloads use `invalid_request`); the docs table conflated both as `invalid_request`. Split the row to match the implemented behavior. The upgrade-rejection test asserted only !is_success(); a plain GET without Connection: upgrade headers returns 400 deterministically (axum WebSocketUpgrade), so assert the exact status. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dd834ada6f

ℹ️ 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".

chatgpt-codex-connector · 2026-06-18T22:33:49Z

+        let router = self.select_router_for_request(Some(model));
+        if let Some(router) = router {
+            router.route_responses_ws(req, model).await


Route Responses WebSockets only to gRPC regular

In IGW/multi-router deployments, this generic selection can choose any router weighted for the model (grpc-pd, http-pd, grpc-regular, or http-regular); the non-gRPC-regular routers inherit the default route_responses_ws implementation that returns 501. When a model is served by both regular gRPC and another backend, GET /v1/responses?model=... can therefore intermittently fail the WebSocket upgrade even though a supported gRPC regular worker exists. This path should select a router that actually supports Responses WebSockets rather than using the generic request router.

Useful? React with 👍 / 👎.

The WS Responses route uses the same generic select_router_for_request as the existing route_realtime_ws, so this is not WS-Responses-specific, and a single gRPC-regular backend (V1's target) is unaffected on both enable_igw shapes. Capability-aware selection for multi-router/IGW deployments is a cross-cutting follow-up spanning both WS routes.

…storage-context The WS `GET /v1/responses` upgrade lives in the realtime route group, which does not carry storage_context_middleware, and its handler runs detached from the request task — so current_request_context() was None and store=true WS reads/writes ran unscoped while the HTTP path is tenant/user-scoped. Rebuild the storage request-context directly from the upgrade request's headers (build_storage_request_context is now pub(crate)) and thread it into the WS ResponsesContext; wrap the get_response_chain read in with_request_context so tenant-scoped storage hooks see the scope. No-op when storage_context_headers is unconfigured (the default), matching the middleware. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d205346728

ℹ️ 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".

chatgpt-codex-connector · 2026-06-18T23:38:53Z

+    });
+    if drain_completed {
+        // SSE: drain emitter state (terminal; byte-identical to prior output).
+        let event = emitter.emit_completed(terminal_usage_json.as_ref());


Preserve pending status on SSE MCP terminal frames

When the MCP streaming loop exits via function tool calls or the max-tool-call limit, terminal_status is InProgress, but the HTTP SSE path (drain_completed == true, used by execute_tool_loop_streaming) calls emit_completed(), which hardcodes response.status to completed; only the WS branch preserves the explicit status. In those SSE scenarios clients receive a terminal response that looks finished even though the turn is pending for function_call_output or stopped by the tool limit, so they may not continue the tool workflow correctly.

Useful? React with 👍 / 👎.

Fixed in 979e481e. SSE was draining to completed on every exit, so the function-tool-call / max-tool-calls early exits reported done instead of in_progress. Now only a genuinely completed turn drains; early exits keep their status. Matches upstream #22366.

coderabbitai

Actionable comments posted: 1

🤖 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 `@model_gateway/src/routers/grpc/regular/responses/common.rs`:
- Around line 233-241: The conversation storage calls `get_conversation` (around
lines 323-326) and `list_items` (around lines 351-355) need to be wrapped with
`with_request_context` to ensure tenant/user-scoped storage hooks have access to
the request context. Apply the same pattern used for `get_response_chain` in the
diff: for each conversation storage call, wrap it in a match statement that
checks if `ctx.request_context` is Some, and if so, call
`data_connector::with_request_context(rc.clone(), storage_call).await`,
otherwise await the storage call directly without wrapping.

🪄 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: ASSERTIVE

Plan: Pro

Run ID: 35ce65cd-4db8-4a78-8098-c62fc49b4d8c

📥 Commits

Reviewing files that changed from the base of the PR and between dd834ad and d205346.

📒 Files selected for processing (3)

model_gateway/src/middleware/storage_context.rs
model_gateway/src/routers/grpc/regular/responses/common.rs
model_gateway/src/routers/grpc/router.rs

…WS conversation reads Addressing re-review: The MCP tool loop's terminal emission drained (emit_completed, status=completed) on the SSE path for ALL exits, hiding the InProgress status on the function-tool-call and max-tool-call early exits so SSE clients saw a 'completed' turn that is actually pending. Drain only for a normal Completed turn; preserve the explicit status (emit_completed_with_status) on the early exits. WS already did this. Matches upstream #22366. load_conversation_history_with_cache wrapped only get_response_chain in with_request_context; wrap the sibling get_conversation / list_items reads the same way so tenant/user storage hooks are scoped when the function runs outside the middleware (the WS executor). Signed-off-by: Venkat Raman <vraman2811@gmail.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 979e481e49

ℹ️ 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".

chatgpt-codex-connector · 2026-06-19T00:49:22Z

+                        if should_evict_cached_response {
+                            session_guard.cached_response = None;


Keep cached parents after failed continuations

When a continuation references the connection-local cached response and then fails for any later reason (for example a transient worker/MCP error or an unsupported parameter), this branch evicts the previously successful parent. If that parent was created with store: false, the docs say it remains chainable within the live connection, but a retry of the same previous_response_id now misses both the cache and durable storage and returns previous_response_not_found; the failed child request should not invalidate the cached parent.

Useful? React with 👍 / 👎.

Fixed in 70a4859e. A failed child no longer evicts the cached parent. A store:false parent lives only in the connection cache, so dropping it on an unrelated failure broke retries with previous_response_not_found. Flipped the test to assert the parent survives.

chatgpt-codex-connector · 2026-06-19T00:49:23Z

+            execute_tool_loop_streaming_with_sink(
+                ctx,
+                modified_request.clone(),
+                &request,


Preserve history across WS MCP tool iterations

For a WebSocket continuation with MCP tools, the first model call uses modified_request with the cached/durable previous_response_id history, but the MCP loop later seeds ToolLoopState from original_request.input and build_next_request rebuilds subsequent iterations from that state. As soon as the model makes an MCP tool call, the next model call drops the prior conversation and includes only the current turn plus tool output, so multi-turn WS tool workflows lose context after the first tool iteration.

Useful? React with 👍 / 👎.

Real, but not WS-specific: the MCP loop reseeds from original_request.input on both the SSE and WS paths, so history drops after the first tool iteration either way (same as upstream #22366). Fixing it touches the shared loop, so I'll do it as a follow-up rather than diverge here.

The map_err closure in load_conversation_history_with_cache was over-indented and failed `cargo fmt --check`. Normalize it; whitespace only, no behavior change. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

The sink-generic streaming refactor kept store=true persistence only at the WS caller, so the SSE MCP tool-loop path stopped persisting: its fire-and-forget task discarded the finalized response. Streaming tool-call responses (default store=true) then 404'd on GET /v1/responses/{id} and broke previous_response_id chaining. Persist Ok(final_response) in the spawned task, mirroring the non-MCP SSE path and the WS executor. convert_and_accumulate_stream silently dropped any non-chat frame, so a mid-stream worker {"error":...} was swallowed and the loop finalized a bogus response.completed. Surface error frames as Err so the caller fails the turn; non-error informational frames stay lenient. WS session machinery hardening: - The 60-min lifetime cap only queued a Close; an idle client parked on stream.next() that never echoed Close leaked the reader task/fd. Signal a shutdown Notify from the timeout and select on it in the reader loop. - A panic in the executor left active_request set, wedging the connection so every later response.create rejected with concurrent_response_create. Catch the panic (AssertUnwindSafe + catch_unwind) so it routes through the normal error path: slot released, referenced cache evicted, error surfaced. - The writer kept draining after sending Close (RFC 6455 violation); break once Close is flushed. Tests: worker-error-frame and benign-unknown-frame unit tests for the MCP accumulator; a panicking-executor WS test proving the next turn is accepted rather than rejected with concurrent_response_create. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dc7c87f514

ℹ️ 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".

chatgpt-codex-connector · 2026-06-19T01:59:31Z

+    if let Some(handle) = executor_handle.take() {
+        handle.abort();


Don't abort completed turns before persistence finishes

When a client closes the socket immediately after it receives response.completed, the executor can still be awaiting the post-stream persist_response_if_needed/cache update because WS events are enqueued before execute_response_create persists the final response. Aborting the task here cancels that post-stream work, so a store: true response that was already delivered to the client can be missing from GET /v1/responses/{id} or reconnect continuation; only pre-terminal work should be aborted on disconnect.

Useful? React with 👍 / 👎.

Fixed in 4e98d09c. Persist is post-terminal, so detached it from the abort. Disconnect only cancels pre-terminal generation now.

chatgpt-codex-connector · 2026-06-19T01:59:31Z

+            execute_non_mcp_stream_with_sink(
+                ctx,
+                Arc::new(chat_request),
+                request.clone(),


Emit function-call items on the non-MCP WS path

For a response.create that declares only ordinary function tools (no MCP/builtin tools), ensure_mcp_connection returns has_mcp_tools=false, so this branch drives the non-MCP stream. That path accumulates tool_calls only in StreamingResponseAccumulator; ResponseStreamEventEmitter::process_chunk never emits or stores them, so when the worker finishes with finish_reason=tool_calls the WS response.completed sent to the client contains the fallback empty message instead of the function_call item/call_id needed for function_call_output continuation.

Useful? React with 👍 / 👎.

Not WS-specific: drive_non_mcp_stream is shared with SSE and only emits text, so plain function tools drop the same way on both. Shared change, follow-up.

The WS session evicted the connection-local cached parent whenever a continuation that referenced it failed for any reason (unsupported parameter, transient worker or MCP error, or a caught panic). A store=false parent is chainable only through that cache, so the failed child made it unretryable: a retry of the same previous_response_id then missed both the cache and durable storage and returned previous_response_not_found. A failed child must not invalidate a still-valid parent, so drop the eviction. Flip the regression test to assert the parent survives. Addresses the cached-parent-eviction review on ws_responses.rs. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 70a4859e3b

ℹ️ 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".

chatgpt-codex-connector · 2026-06-19T02:38:31Z

+                        session_guard.cached_response = (cached_response.response.status
+                            != ResponseStatus::Failed)
+                            .then_some(cached_response);


Retain prior cache when materialized WS turns fail

This success branch still replaces the session cache with None whenever the executor returns Ok with response.status == Failed. If a store:false parent is cached and a following response.create reaches a terminal failed response (for example a worker finish_reason of error/failed rather than an Err), the parent is evicted even though the failed child is not chainable; retrying that same previous_response_id on the live socket then falls through to durable storage and returns previous_response_not_found. Fresh evidence after the earlier cache-eviction fix: the Err arm now preserves the cache, but this Ok(ResponseStatus::Failed) arm still clears it.

Useful? React with 👍 / 👎.

Fixed in 4e98d09c. The Ok(Failed) arm still cleared the cache. Now it only updates on success, so a failed child no longer evicts the parent. Test added.

…r + message status) The streaming tool-loop's finalized response (persisted on the SSE store=true path, cached on the WS path) was losing its output items for MCP turns. Two causes, both surfaced once the SSE MCP path began persisting: 1. Ordering: execute_tool_loop_streaming_internal emitted the terminal event (emit_completed DRAINS emitter state via take()) BEFORE finalize_with_status (which reads non-destructively), so the finalized response saw emptied state. Materialize the finalized response first; the terminal event still drains afterwards for byte-identical SSE output. The WS path was unaffected (it uses the non-draining emit_completed_with_status). 2. Message item shape: the emitter built the text message output item without a status field. ResponseOutputItem::Message has status: Option<String> with no serde default, so the item failed to deserialize in finalize_with_status and was silently dropped (.ok()), losing the model's final answer from the persisted/cached response. Add status="completed". Verified end-to-end on an A100 (SGLang worker + a live MCP server): a streaming MCP store=true response now persists mcp_list_tools + mcp_call + message, and GET /v1/responses/{id} returns the full response (was empty, then tool-calls-only). Added a finalize_with_status regression test. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 22894b2a43

ℹ️ 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".

chatgpt-codex-connector · 2026-06-19T03:36:05Z

+            // group's middleware (auth + admission + tenant-meta, NO WASM — the
+            // WASM OnResponse phase drops the upgrade-future extensions). It
+            // coexists with `POST /v1/responses` (SSE) via the final `.merge()`.
+            .route("/v1/responses", get(v1_responses_ws)),


Enforce admission for each WS response.create

When legacy or priority admission is configured, this route only admits the initial GET upgrade. I checked concurrency_limit_middleware and priority_admission_middleware: their permits are tied to next.run(req) and the HTTP response body, while WebSocketUpgrade::on_upgrade drives the later response.create loop after the upgrade response is produced. A client that opens one admitted socket can therefore run unlimited serial model generations without consuming the admission slots/rate-limit tokens that protect POST /v1/responses; the WS path should acquire/release admission around each response.create or otherwise hold a permit while generation is active.

Useful? React with 👍 / 👎.

Same as the existing /v1/realtime WS route: with_admission_layer gates the upgrade, neither holds a per-message permit. Per-create admission touches both WS routes, so follow-up.

…urns Two more durability edges on the WS Responses path, both from Codex re-review: 1. Ok(status=Failed) evicted the cached parent. The success arm set the session cache to (status != Failed).then_some(child), overwriting a valid store:false parent with None when a child materializes a Failed response (a worker finish_reason=failed, not an Err). A failed child is not chainable and must not evict the parent, so only update the cache on a successful turn. Mirrors the earlier Err-arm fix in 70a4859. 2. Client disconnect cancelled post-terminal persistence. The session aborts the executor task on disconnect to stop pre-terminal generation, but the durable write runs after the terminal event is already enqueued, so aborting it could leave a store:true response the client already received missing from GET and reconnect. Detach the persist so only pre-terminal work is abortable. Added a regression test for the Ok(Failed) cache arm. Gate green: 1090 lib + 129 api. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4e98d09cc4

ℹ️ 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".

chatgpt-codex-connector · 2026-06-19T09:53:05Z

+        )
+        .await;
+
+    let (_parts, body) = chat_response.into_parts();


Propagate non-2xx chat responses on WS streams

When the inline WebSocket path gets a non-2xx response from execute_chat (for example a stage or worker-selection error before an SSE stream is produced), this drops the status and headers and hands only the body to drive_non_mcp_stream. That helper emits response.created / response.in_progress and ignores non-data: JSON error bodies, so the client can receive response.completed and the session can cache/persist an empty success instead of surfacing the upstream error. Check chat_response.status() and convert the error before starting the Responses event stream.

Useful? React with 👍 / 👎.

Fixed in 7cbcf6a1. Now checks chat_response.status() before streaming; a non-2xx surfaces as a failed turn instead of an empty response.completed.

chatgpt-codex-connector · 2026-06-19T09:53:05Z

+                    sink.send_raw_json(json_str)
+                        .map_err(|_| "Client disconnected".to_string())?;


Fail non-MCP WS turns on upstream error frames

When the non-MCP WebSocket path receives an upstream SSE data: {"error": ...} frame, this branch now forwards the stripped JSON after the framing fix but does not mark the turn failed; the loop continues to the terminal block, emits response.completed, and returns Ok(final_response) for cache/persistence. In that worker-error scenario a WS client sees an error-shaped frame followed by a completed response for the same turn, so detect error payloads here and return an error instead of pass-through on WS.

Useful? React with 👍 / 👎.

Fixed in 7cbcf6a1. The non-MCP path now fails on data: {"error":...} frames instead of forwarding then completing. Mirrors the MCP path; benign frames stay pass-through.

Venkat2811 · 2026-06-22T11:43:11Z

@slin1237 @CatherineSue @key4ng could you pls enable CI for this PR ?

mergify · 2026-06-22T12:05:47Z

Hi @Venkat2811, this PR has merge conflicts that must be resolved before it can be merged. Please rebase your branch:

git fetch origin main
git rebase origin/main
# resolve any conflicts, then:
git push --force-with-lease

Resolve model_gateway/Cargo.toml: take upstream's jsonwebtoken 10.4 (rust_crypto) + validator 0.20.0 (the WS Responses branch adds no jsonwebtoken usage). Signed-off-by: Venkat Raman <vraman2811@gmail.com>

…r frame) Two more error-surfacing gaps on the non-MCP WS streaming path, both from Codex: 1. Non-2xx chat-pipeline responses were streamed as success. execute_chat can return a non-2xx (worker-selection / stage error) before any SSE is produced, and that error lives in the body, not a data: frame. drive_non_mcp_stream ignores non-data: bodies, so the client got response.created/in_progress plus a bogus response.completed and an empty response was cached/persisted. Check chat_response.status() before streaming and surface a non-2xx as a failed turn. 2. Upstream error frames were forwarded but not failed. A data: {"error":...} frame is now forwarded stripped (the earlier framing fix), but the loop still emitted response.completed for the same turn. Detect error payloads and return Err, mirroring the MCP path; benign non-chat frames stay pass-through. Tests: error-frame-fails on the non-MCP path; the framing test now uses a benign informational frame. Gate green on venkat-pc (rustc 1.95): 1112 lib + 129 api. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7cbcf6a188

ℹ️ 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".

chatgpt-codex-connector · 2026-06-22T14:25:04Z

        // Stream text naturally - it only appears on final iteration (tool iterations have empty content)
        let accumulated_response =
-            convert_and_accumulate_stream(response.into_body(), &mut emitter, &tx).await?;
+            convert_and_accumulate_stream(response.into_body(), &mut emitter, sink).await?;


Check MCP chat status before accumulating

When a response.create declares MCP/builtin tools and execute_chat returns a non-2xx response before producing SSE (for example a worker-selection or stage error), this branch still feeds the error body into convert_and_accumulate_stream. That helper only processes data: frames, so a normal JSON error body materializes as an empty successful chat response; the loop then emits/caches/persists a completed Responses turn. The non-MCP WS branch now checks chat_response.status(), but this MCP branch needs the same guard before consuming into_body().

Useful? React with 👍 / 👎.

Fixed in b4f75b5f. Added the same status() guard to the MCP branch before consuming the body, so a non-2xx fails the turn instead of materializing an empty success.

chatgpt-codex-connector · 2026-06-22T14:25:04Z

+        json!({ "type": "response.completed", "response": response.clone() }),
+    )?;
+
+    persist_response_if_needed(


Detach warmup persistence after completion

Fresh evidence after the earlier disconnect fix: warmup_response_create still emits response.completed and then awaits persist_response_if_needed inline. For generate:false with the default store:true, if the client closes after receiving the completed event, serve_responses_ws aborts this executor task and can cancel the storage write; on slow storage it also keeps active_request held after completion, so an immediate next turn can be rejected as concurrent. The warmup path should protect post-terminal persistence the same way as the normal WS path.

Useful? React with 👍 / 👎.

Fixed in b4f75b5f. Detached warmup's persist like the main path, so a disconnect can't cancel the write and the slot frees without waiting on storage.

slin1237 · 2026-06-22T14:43:39Z

Is this purely performance driven or there is a solid use case?
websocket is much harder to manage in general, and its countless security issues.
are you guys actively using it?
I'm a bit reluctant to enable websocket for responses api, the api itself is super complex already

…warmup paths Two follow-ups from Codex re-review, completing the prior round consistently: 1. MCP streaming branch lacked the non-2xx status guard. The non-MCP path now checks chat_response.status() before streaming, but the MCP tool loop still fed a non-2xx error body into convert_and_accumulate_stream (which only consumes data: frames), materializing an empty success and emitting/persisting a completed turn. Add the same status guard before into_body(). 2. Warmup persistence was still inline. warmup_response_create awaited persist_response_if_needed after emitting response.completed, so a client disconnect could cancel the store=true write and a slow write held the in-flight slot. Detach it like the main WS path. Gate green on venkat-pc: 1112 lib + 129 api. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

Venkat2811 · 2026-06-22T15:36:16Z

Is this purely performance driven or there is a solid use case? websocket is much harder to manage in general, and its countless security issues. are you guys actively using it? I'm a bit reluctant to enable websocket for responses api, the api itself is super complex already

Yes, It started as perf driven when i saw openai blog post, it's also very convenient for agentic use-cases - i'm playing with agent harness and loops. smg has all the infra needed (req_id so that one need not send entire chat history everytime, so http already introducing state in api layer).. this PR is just enabling protocol and long lived connections.. i'm reusing most of existing infra and capabilities except for protocol and long lived session, nothing is new here

Original announcement:

https://openai.com/index/speeding-up-agentic-workflows-with-websockets/

Launch partners:
https://x.com/cline/status/2026031848791630033

https://x.com/aisdk/status/2026031263925039591

Recent one:
https://x.com/AmpCode/status/2063009699369513117

As HW n kernels are getting faster and faster, double digit perf gains in router matters (ttft and itl gains just by switching to ws). With all new good oss models, people are starting to really try it, so i think providing better devex and perf is good. i'm also a bit surprised that wider oss inference ecosystem didn't adopt this standard yet. if this project adopts it, wider ecosystem will too

having said that, i agree with you:

with mcp, and other features, this PR is substantially more work than i initially estimated, and things i missed - see the review and fixes commit trails
i'm open to your suggestion, i ran benchmarks in personal capacity (small gpus and models), running on a larger model and deployment and measure maybe ?
ws could be optional and off by default

github-actions · 2026-07-06T20:56:26Z

This pull request has been automatically marked as stale because it has not had any activity within 14 days. It will be automatically closed if no further activity occurs within 16 days. Leave a comment if you feel this pull request should remain open. Thank you!

github-actions Bot added documentation Improvements or additions to documentation dependencies Dependency updates grpc gRPC client and router changes tests Test changes model-gateway Model gateway crate changes labels Jun 18, 2026

gemini-code-assist Bot reviewed Jun 18, 2026

View reviewed changes

Venkat2811 added 4 commits June 18, 2026 20:31

test(gateway): WebSocket Responses protocol tests

6f60bec

21 stub-based protocol tests: streaming, rejection matrix, store semantics, same-socket and reconnect continuation, connection lifetime, plus negative controls. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

Venkat2811 force-pushed the feat/responses-ws-transport branch 2 times, most recently from 980a6d0 to 388405f Compare June 18, 2026 19:41

Venkat2811 changed the title ~~[gateway] WebSocket transport for the Responses API — up to 45% lower TTFT on multi-turn chains~~ [gateway] WebSocket transport for the Responses API — up to 45% lower TTFT, 22–48% lower ITL p99 tail, 17–21% less gateway CPU Jun 18, 2026

Venkat2811 marked this pull request as ready for review June 18, 2026 20:56

Venkat2811 requested review from CatherineSue, key4ng and slin1237 as code owners June 18, 2026 20:56

chatgpt-codex-connector Bot reviewed Jun 18, 2026

View reviewed changes

coderabbitai Bot requested changes Jun 18, 2026

View reviewed changes

Venkat2811 added 3 commits June 19, 2026 00:27

chatgpt-codex-connector Bot reviewed Jun 18, 2026

View reviewed changes

coderabbitai Bot requested changes Jun 18, 2026

View reviewed changes

Comment thread model_gateway/src/routers/grpc/regular/responses/common.rs

chatgpt-codex-connector Bot reviewed Jun 19, 2026

View reviewed changes

style(gateway): fix closure indentation in responses/common.rs

6d2e38c

The map_err closure in load_conversation_history_with_cache was over-indented and failed `cargo fmt --check`. Normalize it; whitespace only, no behavior change. Signed-off-by: Venkat Raman <vraman2811@gmail.com>

chatgpt-codex-connector Bot reviewed Jun 19, 2026

View reviewed changes

mergify Bot added the needs-rebase PR has merge conflicts that need to be resolved label Jun 22, 2026

Venkat2811 added 2 commits June 22, 2026 15:53

Merge upstream/main into feat/responses-ws-transport

465d7fb

Resolve model_gateway/Cargo.toml: take upstream's jsonwebtoken 10.4 (rust_crypto) + validator 0.20.0 (the WS Responses branch adds no jsonwebtoken usage). Signed-off-by: Venkat Raman <vraman2811@gmail.com>

mergify Bot removed the needs-rebase PR has merge conflicts that need to be resolved label Jun 22, 2026

chatgpt-codex-connector Bot reviewed Jun 22, 2026

View reviewed changes

github-actions Bot added the stale PR has been inactive for 14+ days label Jul 6, 2026

		if should_evict_cached_response {
		session_guard.cached_response = None;

		if let Some(handle) = executor_handle.take() {
		handle.abort();

		sink.send_raw_json(json_str)
		.map_err(\|_\| "Client disconnected".to_string())?;

Uh oh!

Conversation

Venkat2811 commented Jun 18, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

What the benchmarks show

Changes

Benchmarks (detail)

TTFT (ms)

E2E latency (ms)

ITL — inter-token latency (ms)

Throughput

Out of scope

Test Plan

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Venkat2811 commented Jun 18, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 18, 2026 •

edited

Loading

coderabbitai Bot Jun 18, 2026 •

edited

Loading

coderabbitai Bot Jun 18, 2026 •

edited

Loading