diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 73af04bf6..9d3660c14 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -745,4 +745,6 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#526](https://github.com/mudler/vllm.cpp/issues/526) | `SERVE-TOOL-HISTORY-ARGS` | OpenAI multi-turn tool history reaches chat templates with string-valued arguments | bug | | [#1934](https://github.com/mudler/vllm.cpp/issues/1934) | `BACKEND-ROCM` | `RocmPlatform::needs_weight_staging()` is stale-false (a W0-era placeholder never revisited despite #523/#509/#506/ROCM_ATTN/hipGraph landing since), so `CheckDeviceWeightFit` — the #1123/#1870 load-time refusal, including the `policy_forces_full_expand` fix — never runs on ROCm: measured directly, `VT_DEVICE_WEIGHT_BUDGET_BYTES=1` produced no refusal on a real load. The actual device allocation the refusal guards is not gated on this flag, so #1870's crash stays reachable until this closes; owed, not fixed in flow, because flipping the flag also moves `DirectDeviceLoadEligible` and several GDN kernel-dispatch defaults that each need their own correctness check | bug | | [#1978](https://github.com/mudler/vllm.cpp/issues/1978) | `MODEL-MM-QWEN4-EXP` | **`Qwen/Qwen3.8-Flash-Next` declares `Qwen4ExpForConditionalGeneration` / `qwen4_exp`, a new architecture vLLM does not implement, so the port runs on a split oracle: transformers for the ALGORITHM, vLLM ops for the OPTIMIZED PATH.** Released 2026-08-24, 180B total / 6B activated, image-text-to-text. The `Qwen3.8` in the name is marketing continuity: `.agents/specs/qwen38-27b-bf16-gate.md`'s "one config key differs" precedent does NOT extend here. Read live 2026-08-26 at vLLM `origin/main` = `6a5e8f5979`: no `qwen4*` path, no registry entry, and a repository-wide GitHub search for `qwen4` returns ZERO results; `vllm-omni` likewise. That is absence from vLLM `main` rather than staleness in our pin `555967922`, so a pin advance does not reach it. What exists is transformers [#48337](https://github.com/huggingface/transformers/pull/48337) "Add Qwen4Exp model", MERGED 2026-08-26, 5211 lines, and SGLang [#36497](https://github.com/sgl-project/sglang/pull/36497), still OPEN and therefore inadmissible. **Developer direction 2026-08-26, recorded verbatim: "use transformers as oracle for algorithmic side. but use ops from vllm so we account for optimized path."** Justified rather than convenient: `Qwen4ExpTextQSAIndexer.forward` loops in Python over `(batch_idx, query_idx)` and says "we only allow eager and sdpa", so porting it as written yields a correct model at an indefensible speed, while AGENTS.md's mirror-vLLM polarity still binds every primitive vLLM implements. `Qwen4ExpTextModel` inherits from `Qwen3_5MoeTextModel` and leaves rotary, MLP, experts, TopK router and the ENTIRE vision tower unchanged (`class Qwen4ExpVisionModel(Qwen3_5MoeVisionModel): pass`), all of which this tree has; GDN is an exact match for our AOT gate (`K=V=128, Hg=16, Hv=48` against `src/vt/cuda/cuda_gdn.cu`'s `H in {48,32}`). The delta is four things, and **exactly two have no vLLM op at all**: the PLE dilated depthwise conv (kernel 4, dilation 3; `git grep dilation` over vLLM `layers/mamba/` = 0 hits) and the n-gram hashed embedding. **The survey's load-bearing finding, and it REVERSES this row's first reading: QSA's structural twin is DeepSeek-V4's C4 indexer lane, NOT MiniMax-M3.** The original call was that QSA, being plain GQA rather than MLA, had to map onto vLLM's non-MLA block-sparse case; that reasoning rested on treating `MLAAttentionSpec` as an MLA claim, and **it is not one** — M3's own indexer cache uses it while M3 is a plain-GQA model, with the comment "Key-only: MLAAttentionSpec budgets one vector/token (not 2x for K+V)". It is a budget shape. Remove that prop and the GQA-vs-MLA argument collapses. Verified line by line at `6a5e8f5979`: **nine independent structural matches with DSv4**, `compress_ratio == 4` literally the same number — MQA index with 1 key head at dim 128; `relu(q.k)` summed over index heads vs `(score.relu() * weights).sum(dim=0)`; `1/sqrt(head_dim)`; one score set per query token with no head axis vs `topk_indices_buffer[num_tokens, topk]`; pooling boundary `(position+1) % COMPRESS_RATIO == 0`; RMSNorm on the pooled key; **RoPE at the block-start position** vs `compressed_pos = (position // CR) * CR`; candidate count `visible // compress_ratio`; and one stored state per 4 tokens via `MLAAttentionSpec(tokens_per_state=compress_ratio)`, a first-class KV field documented as "Ints > 1 compress multiple tokens into one state (DSv4 sparse MLA)" that has no M3 equivalent. **M3 is a DIFFERENT ALGORITHM**, not a worse fit: its score is `tl.max(qk, axis=1)` over 128 RAW token dots with no pooling, no relu and no head reduction, it asserts `num_idx_heads == num_kv_heads` ("no topk index reduce") so it emits one block set PER KV HEAD, and its `SPARSE_BLOCK_SIZE = 128` is welded to the KV page size ("One sparse block == one KV page") on both the score and the attend side — moving it to 4 forces a page size of 4 and breaks `tl.dot`, whose tile needs >= 16. M3 contributes exactly ONE thing and it is a wiring precedent, not an algorithm: that a plain-GQA model can own a key-only side cache through `MLAAttentionSpec` and a private indexer backend. **The genuinely new work is the CONSUMER and nothing upstream supplies it** — every DSv4 sparse consumer attends to COMPRESSED MLA KV (one state per 4 tokens) and M3's attend to raw tokens only at page granularity, while QSA attends to RAW tokens selected at ratio-4 granularity. Two silent-failure traps follow: wiring QSA's top-k into a DSv4 sparse-MLA consumer attends a POOLED key/value and still emits plausible tokens, and **a short-prompt token gate cannot catch it because at context <= `indexer_budget` 2048 every candidate is selected** — so any QSA gate must run past 2048 tokens of context, which is now a stated `## Gates` requirement; and `SparseAttnCompressNormRopeStoreC4Kernel` does NOT mean-pool despite its name — it is a learned softmax pool over an OVERLAPPING window of 8 using a score channel this checkpoint does not have, and the CuteDSL variant refuses `overlap=False` at compile, so the **Triton** `head_dim=128` variant is the correct starting point. Two structural consequences beyond the module list: the residual stream is `hc_count * hidden_size` = **4 x 2560 = 10240 wide through the whole stack** with a low-rank read gate and per-branch scalar write gate around both attention and MLP, which is a change to the per-layer loop and every residual buffer rather than a drop-in module; and `number_of_conv_states = 3` on a PLE layer (GDN conv, PLE conv, n-gram token history) plus the indexer side cache, adjacent to [#1963](https://github.com/mudler/vllm.cpp/issues/1963) and [#1966](https://github.com/mudler/vllm.cpp/issues/1966). **NOTHING PUBLISHED FITS**, read live from the HF API against ~119 GB usable on GB10: BF16 ~360 GB, official FP8 ~180 GB, `RadixArk/...-NVFP4` ~128 GB (NVFP4 backbone with the n-gram table left at FP8, 51.2 GB) and `unsloth/...-GGUF` is a README with ZERO weight files. No GGUF exists and no tool can make one, because llama.cpp has no `qwen4_exp` either, so the standing k-quant requirement means authoring the arch on our side AND states that the quantized arms have NO llama.cpp oracle. **The chosen arm does NOT load today, and the blocker is neither the offload nor the budget: this tree cannot keep a gather table quantized at all.** `KeepQuantKDim` returns `-1` for `GgufTensorRole::kEmbeddingTable` (`src/vllm/model_executor/model_loader/gguf_keep_quant.cpp`), and `qwen3_5_gguf_weights.cpp` asserts it by name — "the embedding table cannot keep quant blocks" — so a Q4_K or Q8_0 n-gram table EXPANDS to bf16 and 51.2B params become **102.4 GB of anonymous memory**; the arm dies before the first forward. The reason was already sitting in a header comment ("a gather, not a GEMM ... A quantized-gather op is a follow-up row") and **no such row exists**. The only non-expanding gather residency is `kKeepF16`, requiring ggml type 1 exactly (102.4 GB on disk) and CPU-ONLY, because `EmbeddingKernelCuda` refuses anything but f32/bf16. **Second blocker:** `moe_intermediate_size = 640` makes `ffn_down_exps` Q4_K-illegal on its reduction dim (640 % 256 = 128), as does `hc_lowrank = 320`; llama.cpp's substitution is believed to be Q5_0 (**UNVERIFIED, owed against the pinned llama.cpp oracle**) and the dependent fact IS verified in-tree — our reader knows ggml ids `0,1,2,8,10..14,16,18,19,22..28,30,39,40,41,66` and has **no entry for 3, 6, 7 or 20**, so a stock `llama-quantize -Q4_K_M` file fails at header parse. We author the converter, so the fix is Q4_0 (block 32, same 4.5 bpw). **`ENG-WEIGHT-OFFLOAD` will not help** — it moves zero bytes today (`ConsiderWeight` has no production callers, pinned by a test) and is documented inert on GB10; the tier that DOES work already ships and is proven by the 2.4T model serving 369.97 GiB from a 119.631 GiB box at ~62 GiB resident: mmap `MAP_PRIVATE`, borrow in place, alias the host pointer, `prefault: false`. Corrected sizing: backbone ~67.7 GiB, whole process ~73.5 GiB of 119.631 at 32K single-stream, ~46 GiB of headroom for the page cache, so the ~76 GB estimate was right within 10%. The design works because per-token demand is **<= 64 KiB of reads** (16 lookups x 160 dims over at most 16 pages) against the 2.4T expert lane's 6.95 GB/token. The architecture supplies its own lever: the per-token n-gram cost is `(ngram_size-1)*heads_per_ngram` = 16 lookups of 160 dims, so **51 GB of the 180 GB, 28% of the model, is a table touched 16 times per token** and making it non-resident is the intended design point (RadixArk reached the same split independently). Sizing arithmetic, NOT measurement: Q8_0 throughout ~191 GB (no), Q4_K_M throughout ~109 GB (yes, ~10 GB left for KV), Q4_K_M backbone with the table non-resident ~76 GB. GB10 is UNIFIED memory so "offload to host" is not a move there; non-resident means disk-backed, and its cost is unmeasured. **Two decisions were put to the developer as explicit accept-or-reject and BOTH are settled 2026-08-26, recorded in place rather than left open.** (1) `.agents/oracles/transformers.md` pins transformers to 5.14.1, deliberately tied to what the pinned vLLM environment resolves so the environment cannot hold two `transformers` at once, and **5.14.1 does not contain `Qwen4Exp`**; the lane-scoped second pin is **ACCEPTED**, on the argument that the invariant guards a vLLM environment against drifting from its transformers and here there is no vLLM implementation to drift from, and it expires the moment vLLM registers `qwen4_exp`. **The lane pin is a real release, not a branch SHA**, which was not the expected outcome: `Qwen4Exp` merged to `main` at 12:03:40Z on 2026-08-26 and `v5.16.0` published at 12:35:15Z, and this was BOUNDED rather than assumed by fetching `models/qwen4_exp/modeling_qwen4_exp.py` at each tag — `v5.16.0` HTTP **200**, `v5.15.0` HTTP **404** — making 5.16.0 the FIRST release carrying the architecture and therefore the tightest available pin. The version string is UNMEASURED (it is the release proven to contain the model, not a `transformers.__version__` read off a running oracle) and `gateable` stays `no`. (2) The first runnable arm is the **Q4_K_M backbone with the n-gram table NON-RESIDENT** (~76 GB). Q8_0 was raised and does not fit at ~191 GB, and no partial-Q8 split reaches 119 GB with the backbone at 8 bits; Q4_K_M-throughout fits on paper at ~109 GB but leaves ~10 GB for KV and activations on a 262144-native-context model, which is not a margin. This promotes the non-resident table from a note to a first-class W6 deliverable, and it is NOT free: GB10 is UNIFIED memory, so the existing host-pinned offload seam (`ENG-WEIGHT-OFFLOAD`, mirroring vLLM's `cpu_offload_gb`) does not by itself solve it there and the mechanism must be disk-backed or genuinely unloaded — established before it is designed around. Spec: [`specs/qwen4-exp-flash-next.md`](specs/qwen4-exp-flash-next.md). No product code lands under the spec pull request | feature | +| [#1982](https://github.com/mudler/vllm.cpp/issues/1982) | `SERVE-STREAM-USAGE` | **`ChatSseStream::next` writes the `/v1/chat/completions` role frame before it reads anything from the engine, so `vllm bench serve --backend openai-chat` stamps TTFT on an empty frame and our TTFT through that harness is an HTTP round trip, not a time to first token.** Upstream builds the role chunk under `if first_iteration:` inside `async for res in result_generator:` (`vllm/entrypoints/openai/chat_completion/serving.py:477,487`) and says why at `:484-486`: an exception in the generator "needs to be sent as the FIRST response". `vllm/benchmarks/lib/endpoint_request_func.py:404-408` guards on the presence of `choices`, not on non-empty `delta.content`, and our role frame carries `delta.content = ""` with no `usage`. vLLM and SGLang order the frame after the first result, so their rows on the same harness are honest and only ours is not; this blocks the #1574 three-engine TTFT row. `.agents/specs/stream-options.md` scoped the buffering to continuous usage on purpose and both its passages are corrected here. Fixed by removing the `usage_.include_continuous_usage` guard around the first-result buffering loop, so the default path buffers too. Spec: [`specs/chat-role-frame-ordering.md`](specs/chat-role-frame-ordering.md) | bug | +| [#1992](https://github.com/mudler/vllm.cpp/issues/1992) | — | **Neither `ChatSseStream::next` nor `CompletionSseStream::next` converts an engine exception into a `data: {"error": …}` frame, so a streaming request that fails is a truncated 200 and the cause reaches only `stderr`.** Upstream yields the error frame and then `data: [DONE]` from the generator's `except GenerationError` / `except Exception` arms (`vllm/entrypoints/openai/chat_completion/serving.py:827-833` at the pin `555967922`), and that frame is what makes the first-iteration ordering at `:484-486` mean anything: the role chunk is built inside the loop so an exception can be the FIRST response, which needs a response to exist. Ours propagates out of `next()` into the cpp-httplib chunked content provider (`src/vllm/entrypoints/openai/api_server.cpp::ApiServer::register_routes`), which logs `sse: stream aborted mid-flight:` and aborts, so a client cannot tell a failed request from a short one. Found while fixing [#1982](https://github.com/mudler/vllm.cpp/issues/1982) and NOT fixed in that flow: upstream's `try` wraps the whole generator, so the frame is owed for mid-stream failures on both endpoints, and that is a different blast radius needing its own red-first cases for the payload shape, the trailing `[DONE]` and the separate `GenerationError` converter. Owed by [`specs/chat-role-frame-ordering.md`](specs/chat-role-frame-ordering.md) `## Owed` | bug | | [#1983](https://github.com/mudler/vllm.cpp/issues/1983) | `KV-GDN-STATE-BUDGET` | **The GDN recurrent-state pool is preallocated per CONFIGURED sequence, on an axis no flag bounds.** `GPUModelRunner::initialize_kv_cache` sizes `gdn_state_slots_ = max_num_reqs * (num_spec + 1)` and allocates one conv and one SSM buffer per GDN layer from it, each `Memset` to zero at construction, so every byte is resident before the first request. Re-derived for `Qwen3.8-27B` (48 linear-attention layers, `Hk/Hv/Dk/Dv/conv = 16/48/128/128/4`, `mamba_ssm_dtype = float32`) at `num_speculative_tokens = 8`: one slot costs 3,371,008 B per layer, 154.31 MiB across 48 layers, so one sequence costs 1.356 GiB and `--max-num-seqs 32` costs **43.40 GiB** that `--kv-cache-memory`, `--num-blocks` and `--gpu-memory-utilization` all fail to bound. The per-sequence cost is NOT the divergence — upstream charges the same `1 + num_speculative_blocks` state blocks (`vllm/v1/kv_cache_interface.py::MambaSpec.max_memory_usage_bytes`) and our `f32` SSM mirrors the checkpoint's own `mamba_ssm_dtype` — the AXIS is: `max_num_seqs` sizes no allocation anywhere in vLLM. Upstream raises the attention block size until one attention page holds one mamba page (`vllm/platforms/interface.py::Platform.check_and_update_config`), pads the mamba page to match, and then draws BOTH from one budgeted pool whose tensors are `shared_by` one layer from each group (`kv_cache_utils.py::_get_kv_cache_config_uniform_page_size`), so its recurrent allocation is a function of available memory and never of the concurrency cap. Fixed by mirroring that arithmetic in `ComputeHybridKvBudget` — `unified_block_tokens = align * cdiv(mamba_page, align * attn_bytes_per_token)`, `max_state_seqs = (num_blocks * block_size / unified_block_tokens) / (1 + num_spec)` — and resolving ONE `max_num_seqs` from it for the runner, the scheduler and the #371 guard alike. The bound reads no layer count (upstream's per-layer page equality cancels it), so it does not depend on the placeholder-layer-name repair owned by [#1963](https://github.com/mudler/vllm.cpp/issues/1963) and [#1966](https://github.com/mudler/vllm.cpp/issues/1966), and it lands in its own translation unit so the three rows share no edit surface. Spec: [`specs/gdn-state-kv-budget.md`](specs/gdn-state-kv-budget.md) | bug | diff --git a/.agents/specs/chat-role-frame-ordering.md b/.agents/specs/chat-role-frame-ordering.md new file mode 100644 index 000000000..7d96d1647 --- /dev/null +++ b/.agents/specs/chat-role-frame-ordering.md @@ -0,0 +1,234 @@ +# Chat SSE: the role frame waits for the first engine result + +**Row:** `SERVE-STREAM-USAGE` (the row that owns `ChatSseStream`) · +**Issue:** [#1982](https://github.com/mudler/vllm.cpp/issues/1982) · +**Kind:** bug fix in flow. + +## Now + +`SERVE-STREAM-USAGE` keeps its recorded state. This change repairs one +divergence inside the row's existing surface. It adds no capability and moves +no lifecycle state. + +## Scope + +`ChatSseStream::next` emits the `/v1/chat/completions` role frame before it +reads anything from the engine. Make the default path buffer the first engine +result, exactly as the continuous-usage path already does, so that: + +1. the role frame reaches the client only after the first engine output exists; +2. a request that fails before its first token raises at the stream seam + before any frame is written. + +Out of scope, and named so that the boundary is visible: + +- `/v1/completions`. `src/vllm/entrypoints/openai/serving_completion.cpp::CompletionSseStream` + (the hold-back in its `next`, at `:94-99`) already withholds + the empty chunked-prefill delta, mirroring + `vllm/entrypoints/openai/completion/serving.py:368-374`. It is not touched. +- The wire shape of the role frame. Same `delta.role`, same empty + `delta.content`, same position ahead of every content frame, same + continuous-usage attachment. Only its arrival time changes. +- The sync `LLMEngine` chat path. It renders every frame after generation ends, + so no ordering question exists there. +- Streaming error frames. Upstream converts an exception inside the generator + into a `data: {"error": …}` frame plus `data: [DONE]` + (`chat_completion/serving.py:827-833`). We have no such seam on either + endpoint. That gap is [#1992](https://github.com/mudler/vllm.cpp/issues/1992), + recorded under `## Owed` below; it is a second defect, not this one. + +## Upstream chain + +Read at the parity pin `555967922` +(`.agents/upstream-sync.md`), in `/home/mudler/_git/vllm`. + +| Upstream anchor | What it fixes here | +|---|---| +| `vllm/entrypoints/openai/chat_completion/serving.py::OpenAIServingChat.chat_completion_stream_generator` `:477` | `async for res in result_generator:` — the loop that must produce a result before anything is yielded. | +| the same function, `:484-486` | The reason, in upstream's own words: "We need to do it here, because if there are exceptions in the result_generator, it needs to be sent as the FIRST response (by the try...catch)." | +| the same function, `:487-534` | `if first_iteration:` builds and yields the role chunk, inside the loop body. | +| the same function, `:827-833` | `except GenerationError` / `except Exception` yields the streaming error response. This is the arm the ordering exists to protect. | +| `vllm/benchmarks/lib/endpoint_request_func.py:404-408` | `if choices := data.get("choices"):` then `if ttft == 0.0:` — TTFT is stamped on the first frame carrying a `choices` key, whatever `delta.content` holds. | + +## The measurement consequence + +Our role frame carries `choices[0].delta.content = ""` and no `usage`, so it +satisfies the TTFT guard above. Emitted before any engine work, it makes +`vllm bench serve --backend openai-chat` record the HTTP round trip to an empty +frame instead of the time to a first token. The number is near zero and does not +move with load. vLLM and SGLang emit their role frame after the first result, so +their rows on the same harness measure the real quantity and ours does not. +This blocks the `#1574` three-engine comparison, whose harness uses +`--backend openai-chat`. + +The artifact flatters this engine and only this engine, which is the property +that makes it a correctness problem rather than a benchmarking footnote. + +## Reversing a recorded decision + +`.agents/specs/stream-options.md` recorded the current behavior on purpose, in +two places: + +- `:110-113` — "Chat continuous usage may buffer the first `RequestOutput` long + enough to know the prompt-ID count before emitting the role frame; this + mirrors upstream, which emits that role frame only after the first result + arrives." +- `:197-199` — "Chat's role frame must not invent a prompt count. In continuous + mode it waits for/buffers the first engine result, matching upstream's + first-iteration ordering rather than reporting zero." + +Both sentences are true about continuous usage and both stop there. The recorded +reasoning treated the buffering as a means to a native prompt count, so it +scoped the wait to the mode that needs that count. Upstream's ordering has a +second and a third purpose that the record did not weigh: + +- upstream's stated purpose, error ordering (`:484-486`), which is independent + of usage mode; +- the TTFT stamping rule above, which did not exist in the analysis at all. + +The narrow reading is therefore wrong rather than outdated. This change updates +both passages in `stream-options.md` so that the record and the code agree. + +## Design + +One edit in `src/vllm/entrypoints/openai/serving_chat.cpp::ChatSseStream`, in its +`next`: remove the `if (usage_.include_continuous_usage)` guard around the +first-result buffering loop, so both modes run it. + +The loop already has the shape both modes need. It calls `WaitOutput`, returns +a standalone ping frame when the keepalive interval expires, records +`prompt_tokens_`, and stores the first result carrying an output (or a finished +result) in `buffered_response_`. The main content loop already consumes +`buffered_response_` before it waits again, so no result is dropped or +duplicated. + +Nothing else moves. The frame the default path emits is byte-identical, because +the default path attaches no `usage` and `prompt_tokens_` is not serialized +there. + +### What this costs a real client + +The first byte of the SSE body now arrives when the first token is ready rather +than when the request is admitted. A client that renders a typing indicator on +the role frame loses that early signal by the true prefill time. The first +*token* is not delayed: the token that used to arrive in frame two now arrives +in frame three, at the same instant, because both frames are written from one +`RequestOutput` that the stream already held. + +TTFT as measured by `vllm bench serve --backend openai-chat` gets worse, and +should. It was previously measuring an HTTP round trip. + +Worker-thread occupancy does not change. `create_chat_completion` still returns +without waiting, and the wait moves from the second `next()` call to the first, +on the same cpp-httplib worker thread that was going to block either way +(`src/vllm/entrypoints/openai/api_server.cpp::ApiServer::register_routes`, the +chunked content provider). `AsyncLLM` keeps batching every other request. No +concurrency decision is needed. + +## Tests + +New file `tests/vllm/entrypoints/openai/test_chat_stream_first_frame.cpp`, +driving the production `ApiServer::handle_chat_completions` dispatch over a real +`AsyncLLM` whose model runner the test controls. + +1. **Frame ordering.** The runner blocks inside `sample_tokens` until the test + releases it, and counts the steps it has sampled. A background thread calls + `next()` once. While the runner is gated, the test asserts its own + precondition (`sampled_steps() == 0`, so no token exists) and then asserts + that no frame has arrived. It releases the runner and asserts that the frame + then arrives, that it carries a `choices` key, and that a token existed by + the time it did. + + A shape assertion on the role frame would pass before the change and prove + nothing, so the discriminating assertion is the negative one taken while the + runner is gated. + +2. **Error ordering.** The runner throws inside `sample_tokens`. The engine + guard posts `ENGINE_CORE_DEAD`, `AsyncLLM` propagates the error to the + request's collector, and the collector rethrows on the consumer thread. The + test asserts that the *first* `next()` call throws and that it wrote no + frame. Before the change the first call returns the role frame and does not + throw. + +3. **No regression in the existing frame set.** `test_sse_keepalive`, + `test_api_server` and `test_serving` keep the role frame first among the data + frames, keep the continuous-usage counts, and keep the keepalive contract. + +### Reachability + +The production entry point is `ApiServer::handle_chat_completions`, which the +`/v1/chat/completions` route calls. Both new cases enter through it, not +through `ChatSseStream`, which is in an anonymous namespace and unreachable by +name. + +The reachability mutation deletes the buffering loop's production call site in a +scratch copy and reruns the focused gate. The gate must go red. + +## Gates + +1. Focused: the new target plus every neighbouring OpenAI suite. +2. Full CPU CTest on a CUDA-OFF build. +3. `scripts/agent-preflight.sh`, exit code captured explicitly. A tail that + reads clean here has exited 1 before, so the code is the verdict. + +No GPU axis is claimed. This change alters an HTTP arrival time, and the online +gate runs on `/v1/completions`, which this change does not touch. + +## Evidence + +Measured 2026-08-26 on `linux/x86_64`, GCC 13.3.0, CMake `Release`, +`-DVLLM_CPP_CUDA=OFF -DVLLM_CPP_BUILD_TESTS=ON`, in +`.wt/chat-role-frame-order` off base `21fe11cf1`. + +| What | Command | Result | +|---|---|---| +| Red, parent tree | `./build/tests/test_chat_stream_first_frame` | 2 cases failed, 4 assertions; the role frame appears in the failure text of both cases | +| Green, fixed head | the same | 2 cases passed, 21 of 21 assertions | +| Focused | `ctest -R 'test_chat_stream_first_frame\|test_sse_keepalive\|test_openai_api_server\|test_openai_serving\|test_openai_serving_chat_stream\|test_openai_protocol\|test_openai_conformance\|test_openai_logprobs'` | 8 of 8 passed, exit 0 | +| Full CPU CTest | `ctest -j 4 --output-on-failure` | 628 of 628 passed, exit 0 | +| Preflight | `scripts/agent-preflight.sh` | exit 1, one gate: `test_cpu_x86_llamacpp_floor`. Zero SKIPs. Every other gate `ok`, including `commit-trailers` and `commit-style` | + +The one preflight failure is [#618](https://github.com/mudler/vllm.cpp/issues/618), +not this change. Its contended-leg case is load dependent, and the recorded +signature is exactly what it printed: `NO_QUIET_WINDOW` (4) where it expects +`GIVING_UP` (2), here at `load=33.97` while the box carried other sessions' +builds. Run alone at load 21.38 the same file passes 10 of 10 in 20.8 s. + +### Mutations, on the final head + +The tree was restored from a byte copy after each one and `sha256sum -c` +confirmed both source files, and the suite was re-run green afterwards. The +mutation counts below were taken at 20 assertions, before one diagnostic +assertion was added to the puller thread; neither mutation touches it. + +| Mutation | Result | +|---|---| +| Reinstate `if (usage_.include_continuous_usage)` around the buffering loop | 2 cases failed, 4 assertions — the same 4 as the red | +| Delete the production call site `out.sse_stream = std::move(result.sse_stream)` in `ApiServer::handle_chat_completions` | 2 cases failed at 6 assertions; the gate cannot reach the code without the route handler | + +The second is the reachability mutation. It is what separates "the class works" +from "a client reaches it". + +## Owed + +- [#1982](https://github.com/mudler/vllm.cpp/issues/1982) — this change closes + the ordering half. +- [#1992](https://github.com/mudler/vllm.cpp/issues/1992) — a streaming error + frame. Neither `ChatSseStream::next` nor + `CompletionSseStream::next` converts an engine exception into + `data: {"error": …}` + `data: [DONE]` the way + `chat_completion/serving.py:827-833` does. The exception reaches the cpp-httplib + content provider (`src/vllm/entrypoints/openai/api_server.cpp`, the + `catch (...)` in the chunked provider), which logs to `stderr` and truncates + the body. After this change no role frame precedes that truncation, so the + client no longer sees a well-formed start to a request that died; it sees an + empty 200. That is an improvement and not a fix. + +## Stop conditions + +- Return `NEEDS_DECISION` if the buffering cannot be done without holding a + worker thread longer than the current path already holds it. Measured: it + cannot happen, because the wait moves rather than accumulates. +- Return `NEEDS_CONTEXT` if the pinned oracle tree cannot be read. It could: + `/home/mudler/_git/vllm` is a shallow clone whose tree at `555967922` is + complete. diff --git a/.agents/specs/stream-options.md b/.agents/specs/stream-options.md index a8d040317..b5e04b2bf 100644 --- a/.agents/specs/stream-options.md +++ b/.agents/specs/stream-options.md @@ -106,10 +106,15 @@ ratio. | FastAPI/Pydantic validation | existing cpp-httplib dispatch exception-to-400 seam in `src/vllm/entrypoints/openai/api_server.cpp` | The local pull-stream state machines retain one explicit pending-usage state -between the final choice and `[DONE]`. Chat continuous usage may buffer the -first `RequestOutput` long enough to know the prompt-ID count before emitting -the role frame; this mirrors upstream, which emits that role frame only after -the first result arrives. +between the final choice and `[DONE]`. Chat buffers the first `RequestOutput` +before it emits the role frame, in every usage mode; this mirrors upstream, +which emits that role frame only after the first result arrives. + +**Corrected 2026-08-26, [#1982](https://github.com/mudler/vllm.cpp/issues/1982).** +This paragraph scoped the buffering to continuous usage, and the code did the +same. Continuous usage needs the prompt-ID count, but that is not upstream's +only reason for the ordering, so the narrow reading was wrong. See +[chat-role-frame-ordering.md](chat-role-frame-ordering.md). ## Tests to port @@ -194,8 +199,15 @@ hardware checkpoint after the implementation commit. - The final usage frame follows the finish-reason choice and precedes `[DONE]`. Emitting it after `[DONE]`, attaching it only to the finish choice, or estimating it from text would not be wire-compatible. -- Chat's role frame must not invent a prompt count. In continuous mode it waits - for/buffers the first engine result, matching upstream's first-iteration - ordering rather than reporting zero. +- Chat's role frame must not invent a prompt count. It waits for and buffers + the first engine result in every usage mode, matching upstream's + first-iteration ordering rather than reporting zero. **Corrected 2026-08-26, + [#1982](https://github.com/mudler/vllm.cpp/issues/1982):** this bullet said + "in continuous mode", and the default path emitted the role frame before it + read the engine. Upstream orders the role frame after the first result so an + exception can be the first response, which no usage mode changes, and + `vllm/benchmarks/lib/endpoint_request_func.py:404-408` stamps TTFT on the + first `choices`-bearing frame whatever its content holds. See + [chat-role-frame-ordering.md](chat-role-frame-ordering.md). - The failed `8289cbd` arm is diagnostic only. It cannot be patched in place or reused after the implementation changes the commit SHA. diff --git a/src/vllm/entrypoints/openai/serving_chat.cpp b/src/vllm/entrypoints/openai/serving_chat.cpp index 6b73ee906..838626010 100644 --- a/src/vllm/entrypoints/openai/serving_chat.cpp +++ b/src/vllm/entrypoints/openai/serving_chat.cpp @@ -300,9 +300,10 @@ ShapedChatMessage ShapeChatMessageEngine( namespace { // chat_completion_stream_generator (serving.py:404-802) as W2's live, -// pull-based SSE source. Continuous usage waits for and buffers the first -// result so the role frame carries a native prompt-token count; subsequent -// calls block only on this request's collector. +// pull-based SSE source. It waits for and buffers the first result before the +// role frame, in every usage mode, mirroring upstream's first-iteration +// ordering (#1982); continuous usage reads that result's native prompt-token +// count. Subsequent calls block only on this request's collector. class ChatSseStream final : public SseStream { public: ChatSseStream(v1::AsyncLLM& engine, v1::AsyncRequest async_request, @@ -345,22 +346,37 @@ class ChatSseStream final : public SseStream { bool next(std::string& chunk) override { if (complete_) return false; if (role_pending_) { - // Upstream emits the role frame on the first engine result. We only need - // to buffer that result when continuous usage requires its native prompt - // count; the default path retains its immediately available role frame. - if (usage_.include_continuous_usage) { - for (;;) { - RequestOutput response; - if (!WaitOutput(response, chunk)) { - // WaitOutput filled chunk with a pure SSE ping — return it first. - return true; - } - prompt_tokens_ = - static_cast(response.prompt_token_ids.size()); - if (!response.outputs.empty() || response.finished) { - buffered_response_ = std::move(response); - break; - } + // Upstream emits the role frame on the FIRST ENGINE RESULT, in every + // usage mode: chat_completion/serving.py:487 builds the role chunk under + // `if first_iteration:` inside `async for res in result_generator:` + // (:477). Its comment at :484-486 gives the reason — "if there are + // exceptions in the result_generator, it needs to be sent as the FIRST + // response (by the try...catch)" — and that reason does not depend on + // usage. So this buffering runs unconditionally (#1982). + // + // It was scoped to continuous usage, which needs the native prompt count + // for the frame it attaches. The narrow reading cost two things. A + // request that died before its first token had already been answered 200 + // with a role frame, so the error could not be the first response. And + // `vllm/benchmarks/lib/endpoint_request_func.py:404-408` stamps TTFT on + // the first chunk carrying a `choices` key whatever `delta.content` + // holds, so `vllm bench serve --backend openai-chat` measured the HTTP + // round trip to this empty frame instead of a time to first token. + // + // The frame itself is unchanged, including its position ahead of every + // content frame. Only its arrival time moves, and the first TOKEN is not + // delayed: the token that used to ride in frame two now rides in frame + // three, at the same instant, out of the result buffered right here. + for (;;) { + RequestOutput response; + if (!WaitOutput(response, chunk)) { + // WaitOutput filled chunk with a pure SSE ping — return it first. + return true; + } + prompt_tokens_ = static_cast(response.prompt_token_ids.size()); + if (!response.outputs.empty() || response.finished) { + buffered_response_ = std::move(response); + break; } } role_pending_ = false; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c7f2cb1a0..97baa9f5c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1852,6 +1852,12 @@ if(VLLM_CPP_SERVER) endif() endif() vllm_cpp_add_test(test_openai_api_server vllm/entrypoints/openai/test_api_server.cpp) + # #1982: WHEN the first chat SSE frame reaches the client. Its own target so + # the gated-runner fixture cannot perturb the keepalive suite's timings, and + # inside this guard because it drives the production ApiServer dispatch, + # which only exists when VLLM_CPP_SERVER is ON. + vllm_cpp_add_test(test_chat_stream_first_frame + vllm/entrypoints/openai/test_chat_stream_first_frame.cpp) # /v1/audio/transcriptions dispatch + socket smoke run against the REAL # library transcription seam on the committed parakeet_e2e fixture # (ARCH-ONE-SURFACE ROW 1). diff --git a/tests/vllm/entrypoints/openai/test_chat_stream_first_frame.cpp b/tests/vllm/entrypoints/openai/test_chat_stream_first_frame.cpp new file mode 100644 index 000000000..30ff1321e --- /dev/null +++ b/tests/vllm/entrypoints/openai/test_chat_stream_first_frame.cpp @@ -0,0 +1,426 @@ +// #1982 — WHEN the first `/v1/chat/completions` SSE frame reaches the client. +// +// Upstream builds the role chunk under `if first_iteration:` INSIDE +// `async for res in result_generator:` +// (vllm/entrypoints/openai/chat_completion/serving.py:477,487 @ 555967922) and +// says why at :484-486: "We need to do it here, because if there are exceptions +// in the result_generator, it needs to be sent as the FIRST response (by the +// try...catch)." +// +// Ours wrote that frame before it read the engine at all, and two things follow. +// +// 1. A request that dies before its first token had already been answered 200 +// with a role frame, so the error could not be the first response. +// 2. `vllm/benchmarks/lib/endpoint_request_func.py:404-408` stamps TTFT on the +// first chunk carrying a `choices` key, REGARDLESS of whether +// `delta.content` is empty: +// +// if choices := data.get("choices"): +// content = choices[0]["delta"].get("content") +// # First token +// if ttft == 0.0: +// ttft = timestamp - st +// +// Our role frame carries `delta.content = ""` and no `usage`, so it +// satisfies that guard. Any TTFT taken against this endpoint with +// `vllm bench serve --backend openai-chat` was the HTTP round trip to an +// empty frame — near zero, and independent of load. +// +// WHAT THESE CASES MEASURE, in words, because a role-frame SHAPE assertion +// passes on both sides of the fix and proves nothing: the discriminating +// assertion is the NEGATIVE one, taken while the model runner is held inside +// sample_tokens so that NO token can exist yet. The runner counts its own +// sampled steps, so each case asserts that precondition rather than assuming +// it. +// +// Both cases enter through `ApiServer::handle_chat_completions`, which is what +// the production `/v1/chat/completions` route calls. `ChatSseStream` lives in an +// anonymous namespace and cannot be named from a test, so there is no way to +// reach this code except the way a client reaches it. +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "vllm/config/scheduler.h" +#include "vllm/entrypoints/openai/api_server.h" +#include "vllm/entrypoints/openai/protocol.h" +#include "vllm/entrypoints/openai/serving_chat.h" +#include "vllm/entrypoints/openai/serving_completion.h" +#include "vllm/entrypoints/openai/serving_models.h" +#include "vllm/outputs.h" +#include "vllm/tokenizer/bpe.h" +#include "vllm/tokenizer/tokenizer.h" +#include "vllm/transformers_utils/hf_config.h" +#include "vllm/v1/core/kv_cache_utils.h" +#include "vllm/v1/core/sched/scheduler.h" +#include "vllm/v1/engine/async_llm.h" +#include "vllm/v1/engine/core_client.h" +#include "vllm/v1/engine/input_processor.h" +#include "vllm/v1/engine/output_processor.h" +#include "vllm/v1/executor/executor.h" +#include "vllm/v1/kv_cache_interface.h" +#include "vllm/v1/worker/gpu/model_runner_base.h" +#include "vt/dtype.h" + +namespace oai = vllm::entrypoints::openai; +using json = nlohmann::json; + +namespace { + +constexpr int32_t kCannedToken = 17; // fixture token " world" + +// A runner the TEST clocks. sample_tokens blocks until release() is called, and +// sampled_steps() reports how many times it has produced tokens. That counter +// is the instrument's own precondition: while it reads 0, no token exists +// anywhere in the engine, so any SSE frame observed at that moment was produced +// without engine output. +// +// The wait carries a bounded timeout so a regression cannot hang the suite; the +// timeout is a safety net and never the path a passing run takes. +class GatedRunnerStub : public vllm::v1::ModelRunnerBase { + public: + std::optional execute_model( + const vllm::v1::SchedulerOutput& scheduler_output) override { + stashed_ = scheduler_output; + return std::nullopt; + } + + vllm::v1::ModelRunnerOutput sample_tokens( + const std::optional& /*grammar_output*/) + override { + { + std::unique_lock lock(mutex_); + released_cv_.wait_for(lock, std::chrono::seconds(30), + [this] { return released_; }); + } + vllm::v1::ModelRunnerOutput output; + int index = 0; + for (const auto& [request_id, num_tokens] : stashed_.num_scheduled_tokens) { + (void)num_tokens; + output.req_ids.push_back(request_id); + output.req_id_to_index[request_id] = index++; + output.sampled_token_ids.push_back({kCannedToken}); + } + sampled_steps_.fetch_add(1); + return output; + } + + void release() { + { + std::lock_guard lock(mutex_); + released_ = true; + } + released_cv_.notify_all(); + } + + int sampled_steps() const { return sampled_steps_.load(); } + + private: + vllm::v1::SchedulerOutput stashed_; + std::mutex mutex_; + std::condition_variable released_cv_; + bool released_ = false; + std::atomic sampled_steps_{0}; +}; + +// The poisoned twin: the engine dies before it can produce a token. The engine +// busy-loop guard posts ENGINE_CORE_DEAD, AsyncLLM's output handler calls +// propagate_error, and the request's collector rethrows on the consumer thread — +// which is the SSE stream's own thread. +class ThrowingRunnerStub : public vllm::v1::ModelRunnerBase { + public: + std::optional execute_model( + const vllm::v1::SchedulerOutput& /*scheduler_output*/) override { + return std::nullopt; + } + + vllm::v1::ModelRunnerOutput sample_tokens( + const std::optional& /*grammar_output*/) + override { + throw std::runtime_error("vt: CHAT_ROLE_FRAME_ORDER_SENTINEL"); + } +}; + +vllm::tok::Tokenizer BuildFixtureTokenizer() { + static int counter = 0; + const std::string path = + (std::filesystem::temp_directory_path() / + ("vllm_chat_first_frame_tok_" + std::to_string(counter++) + ".json")) + .string(); + json doc; + doc["version"] = "1.0"; + doc["added_tokens"] = json::array(); + doc["normalizer"] = nullptr; + doc["pre_tokenizer"] = {{"type", "ByteLevel"}, + {"add_prefix_space", false}, + {"trim_offsets", false}, + {"use_regex", true}}; + const json vocab = {{"h", 0}, {"e", 1}, {"l", 2}, {"o", 3}, + {"w", 4}, {"r", 5}, {"d", 6}, {"Ġ", 7}, + {"1", 8}, {"2", 9}, {"ll", 10}, {"he", 11}, + {"llo", 12}, {"hello", 13}, {"Ġw", 14}, {"or", 15}, + {"orld", 16}, {"Ġworld", 17}, {"ld", 18}}; + doc["model"] = { + {"type", "BPE"}, + {"ignore_merges", false}, + {"vocab", vocab}, + {"merges", json::array({json::array({"l", "l"}), json::array({"h", "e"}), + json::array({"ll", "o"}), + json::array({"he", "llo"}), + json::array({"Ġ", "w"}), json::array({"o", "r"}), + json::array({"l", "d"}), + json::array({"or", "ld"}), + json::array({"Ġw", "orld"})})}}; + { + std::ofstream out(path, std::ios::binary); + out << doc.dump(); + } + vllm::tok::Tokenizer tokenizer = vllm::tok::Tokenizer::FromHfJson(path); + std::remove(path.c_str()); + return tokenizer; +} + +vllm::SchedulerConfig MakeSchedulerConfig() { + vllm::SchedulerConfig cfg; + cfg.max_num_seqs = 8; + cfg.max_num_batched_tokens = 8192; + cfg.enable_chunked_prefill = true; + cfg.max_model_len = 8192; + cfg.watermark = 0.0; + return cfg; +} + +// #1999 clamps `max_num_seqs` to the seats the KV budget affords. It cannot +// bite here, for three independent reasons, and a future edit to this fixture +// should keep at least one of them true: +// 1. `ComputeHybridKvBudget` returns early on `mamba == nullptr` +// (hybrid_kv_budget.cpp), and this config carries one FullAttentionSpec +// and no MambaSpec, so the budget stays `kStateSeqsUnbounded` (-1) and +// `ClampMaxNumSeqsToStateBudget` passes the configured value through. +// 2. The only production caller is `model_loader.cpp`, and this fixture +// builds the Scheduler and AsyncLLM directly without the loader. +// 3. Each case issues exactly ONE streaming request, so `max_num_seqs = 8` +// is headroom rather than a requirement; even a clamp to 1 seat would +// leave both cases passing. +vllm::v1::KVCacheConfig MakeKvConfig() { + vllm::v1::KVCacheConfig kv; + kv.num_blocks = 1024; + kv.kv_cache_groups.emplace_back( + std::vector{"layer"}, + std::make_shared(16, 1, 1, vt::DType::kF32)); + return kv; +} + +vllm::HfConfig MakeHfConfig() { + vllm::HfConfig config; + config.max_position_embeddings = 8192; + config.raw = json::object(); + return config; +} + +vllm::v1::BlockHasher Hasher() { + static bool initialized = false; + if (!initialized) { + vllm::v1::init_none_hash(vllm::v1::sha256_cbor); + initialized = true; + } + return vllm::v1::get_request_block_hasher(16, vllm::v1::sha256_cbor); +} + +// The default role-join fallback renders "user: hello\nassistant:", whose +// characters exceed this minimal BPE fixture's vocabulary. ChatPromptFn is the +// documented renderer seam (serving_chat.h), so the fixture supplies one that +// emits a prompt the fixture can encode. The framing under test is downstream +// of the prompt and unaffected by its text. +oai::ChatPromptFn FixturePromptFn() { + return [](const std::vector&, bool, + const std::vector&, + const nlohmann::ordered_json&) -> std::string { return "hello"; }; +} + +// The production server stack the HTTP routes hold: AsyncLLM -> +// OpenAIServingChat -> ApiServer. `Runner` is the stub this case clocks. +template +struct ServerHarness { + ServerHarness() + : tokenizer(BuildFixtureTokenizer()), + scheduler(MakeSchedulerConfig(), MakeKvConfig(), /*block_size=*/16, + /*enable_caching=*/true), + executor(runner), + input_processor(tokenizer, MakeHfConfig()), + output_processor(&tokenizer), + engine(input_processor, scheduler, executor, output_processor, Hasher()), + models("test-model"), + completion(engine, "test-model"), + chat(engine, "test-model", FixturePromptFn()), + server(completion, chat, models, "9.9.9") {} + + vllm::tok::Tokenizer tokenizer; + vllm::v1::Scheduler scheduler; + Runner runner; + vllm::v1::Executor executor; + vllm::v1::InputProcessor input_processor; + vllm::v1::OutputProcessor output_processor; + vllm::v1::AsyncLLM engine; + oai::OpenAIServingModels models; + oai::OpenAIServingCompletion completion; + oai::OpenAIServingChat chat; + oai::ApiServer server; +}; + +constexpr const char* kStreamingChatBody = + R"({"model":"test-model","messages":[{"role":"user","content":"hello"}],)" + R"("max_tokens":2,"temperature":0.0,"stream":true})"; + +// Does this frame carry the key `vllm bench serve` stamps TTFT on? +bool CarriesChoices(const std::string& frame) { + if (frame.rfind("data: ", 0) != 0) return false; + const std::string payload = frame.substr(6); + if (payload.rfind("[DONE]", 0) == 0) return false; + const json parsed = json::parse(payload, nullptr, false); + return !parsed.is_discarded() && parsed.contains("choices") && + !parsed.at("choices").empty(); +} + +} // namespace + +// ── 1. Frame ordering ──────────────────────────────────────────────────────── +// +// The runner is held inside sample_tokens for the whole first phase, so the +// engine cannot have produced a token. The assertion that discriminates is +// CHECK_FALSE(first_frame_arrived) taken in that state. Before the fix the role +// frame is already on the wire microseconds after the request is admitted, so it +// fails; after the fix it cannot be produced while the gate is closed, whatever +// the load on the box. +// +// The 300 ms is a GRACE for the defective path to show itself, never a deadline +// the correct path has to beat. A slower box makes this case more reliable, not +// less. +TEST_CASE("chat SSE: no choices-bearing frame before the first token exists") { + ServerHarness h; + + oai::ApiServer::DispatchResult result = + h.server.handle_chat_completions(kStreamingChatBody); + REQUIRE(result.status == 200); + REQUIRE(result.streaming); + REQUIRE(result.sse_stream != nullptr); + std::shared_ptr stream = result.sse_stream; + + std::atomic first_frame_arrived{false}; + std::string first_frame; + std::string puller_error; + // An exception escaping this thread would std::terminate the whole binary and + // report nothing, so it is caught and turned into a named failure below. + std::thread puller([&] { + try { + if (stream->next(first_frame)) first_frame_arrived.store(true); + } catch (const std::exception& e) { + puller_error = e.what(); + } catch (...) { + puller_error = "unknown exception"; + } + }); + + std::this_thread::sleep_for(std::chrono::milliseconds(300)); + + // The instrument states its own precondition: 0 sampled steps means no token + // exists, so a frame observed now was produced with no engine output at all. + CHECK_MESSAGE(h.runner.sampled_steps() == 0, + "the gated runner sampled a step while still gated; this case " + "measured nothing"); + CHECK_MESSAGE(!first_frame_arrived.load(), + "a chat SSE frame reached the client before any engine output: " + << first_frame); + + h.runner.release(); + puller.join(); + + // ...and the frame is not merely late, it still arrives, with the shape the + // wire contract keeps: role "assistant", empty content, choices present. + CHECK_MESSAGE(puller_error.empty(), + "the first next() call threw instead of returning a frame: " + << puller_error); + CHECK(first_frame_arrived.load()); + CHECK(h.runner.sampled_steps() >= 1); + REQUIRE(first_frame.rfind("data: ", 0) == 0); + CHECK(CarriesChoices(first_frame)); + const json parsed = json::parse(first_frame.substr(6)); + CHECK(parsed.at("choices").at(0).at("delta").at("role") == "assistant"); + CHECK(parsed.at("choices").at(0).at("delta").at("content") == ""); + CHECK_FALSE(parsed.contains("usage")); + + // Drain so the request retires before the harness tears the engine down. + // + // This drain also guards the one regression the change could introduce: the + // buffered first result must be DELIVERED, not swallowed. Both sampled tokens + // are the fixture's `Ġworld`, so the concatenated content across every frame + // has to carry "world" exactly twice. Counting the text rather than the frames + // keeps the assertion independent of collector merging, which can fold two + // outputs into one frame when the consumer is slow. + std::string chunk; + std::vector rest; + std::string streamed; + while (stream->next(chunk)) { + rest.push_back(chunk); + if (!CarriesChoices(chunk)) continue; + const json frame = json::parse(chunk.substr(6)); + const json& delta = frame.at("choices").at(0).at("delta"); + if (delta.contains("content") && delta.at("content").is_string()) { + streamed += delta.at("content").get(); + } + } + REQUIRE_FALSE(rest.empty()); + CHECK(rest.back() == "data: [DONE]\n\n"); + size_t occurrences = 0; + for (size_t at = streamed.find("world"); at != std::string::npos; + at = streamed.find("world", at + 1)) { + ++occurrences; + } + CHECK_MESSAGE(occurrences == 2, + "the buffered first result was dropped or duplicated; streamed " + "content was: " + << streamed); +} + +// ── 2. Error ordering ──────────────────────────────────────────────────────── +// +// Upstream's stated reason for the ordering, made executable. The engine dies +// before its first token; the FIRST next() call must surface that, and must not +// have written a frame first. +// +// `chunk` carries a sentinel, so the case fails in two independent ways on the +// old behaviour: next() returns instead of throwing, AND the sentinel is +// overwritten by the role frame. +TEST_CASE("chat SSE: an engine that dies before the first token is not " + "preceded by a role frame") { + ServerHarness h; + + oai::ApiServer::DispatchResult result = + h.server.handle_chat_completions(kStreamingChatBody); + REQUIRE(result.status == 200); + REQUIRE(result.streaming); + REQUIRE(result.sse_stream != nullptr); + + std::string chunk = "SENTINEL_NOT_A_FRAME"; + CHECK_THROWS_MESSAGE(result.sse_stream->next(chunk), + "the first next() call returned a frame instead of " + "surfacing the engine failure"); + CHECK_MESSAGE(chunk == "SENTINEL_NOT_A_FRAME", + "a frame was written before the engine failure surfaced: " + << chunk); +}