Repository navigation
fix(perf): ignore metadata-only chunks in TTFT and ITL - #1645
Conversation
|
Thanks for the focused fix. Separating output-token timing from general SSE event timing correctly prevents metadata-only chunks from distorting TTFT and ITL while preserving end-to-end latency and usage parsing. Please rebase onto the latest main, resolve the Ruff-related conflict in default_api.py, run Ruff formatting, and rerun CI. Optional: annotate iter_chunks and cover a trailing usage chunk with choices: []. |
Yunnglin
left a comment
There was a problem hiding this comment.
The functional change looks correct and I found no blocking logic issue. Requesting changes only because the branch no longer merges cleanly with the latest main. Please rebase, resolve the conflict in default_api.py, apply the current Ruff formatting, and rerun CI; after that this should be ready to merge.
|
Thanks for the review. I've rebased the branch locally onto the latest main, resolved the Ruff conflict in The current main tip ( |
06fcee5 to
3dec1e0
Compare
…ate (#1820) Since #1645, inter_chunk_latency holds only chunks with output, but the speculative estimate still takes its chunk count from it. A plain stream whose steps sometimes decode to an empty delta (a partial multi-byte token, the EOS step) then has fewer chunks than tokens, so L > 1 and the run reports speculative decoding. Count every chunk from the first output on, empty or not, for L; ITL is unchanged. Co-authored-by: Claude Opus 5.5 <[email protected]>
Problem
OpenAI-compatible chat streams can emit
choicesevents without generated output. For example,vLLM sends an initial role-only delta with
content: "", and services may send a separate emptydelta carrying
finish_reason.DefaultApiPlugin.process_request()currently records TTFT for the first event with a non-emptychoicesarray and records every laterchoicesevent as an ITL sample, even when the extractedtext and reasoning content are empty. This can report TTFT before the first visible output and add
metadata gaps to ITL.
For a deterministic stream with events at 0.10s (role), 0.45s (
H), 0.65s (i), and 0.90s(finish), the current code reports:
The output-based result should be:
Fix
Track the last non-empty output timestamp separately from the last response-event timestamp.
Verification
The new asynchronous regression reproduces role-only, content, finish-only, usage, and
[DONE]events with deterministic timestamps. Before the fix, it fails with
0.10 != 0.45at the TTFTassertion. After the fix:
main.compileallandgit diff --checkpass.The regression also verifies that generated text, response retention, usage parsing, and
end-to-end query latency remain unchanged.