fix(bench): stream reasoning_content and accept clean zero-content finishes#18
Conversation
…nishes Reasoning/diffusion models stream first tokens as delta.reasoning_content and 1-token prefill points can finish with no streamed content. The strict content-only parser failed those requests (response_shape) and would have nil-derefed firstTokenAt. Now reasoning_content seeds TTFT/ITL, a clean finish_reason with zero tokens counts as completed (no fabricated TTFT), and only a stream with neither token nor finish reason is malformed.
…mples Codex P1: the reorder put the TPOT calc before sample.CompletionTokens was set, dropping TPOT for all multi-token streamed completions. Assign tokens first; test now asserts positive TPOT.
…for streamed completion tokens Codex P2: a zero-content stream that finishes without a usage chunk would borrow request.OutputTokensExpected and fabricate throughput. Fall back to the observed streamed-chunk count instead (0 for empty streams); usage still wins when present. Test covers no-usage empty and content cases.
|
Review complete: two consecutive P2-only rounds (no P0/P1), stopping per the review rule. Addressed: reasoning_content TTFT, clean zero-content finish acceptance, nil-deref guard, and the TPOT-ordering P1. Known limitation (documented, not blocking): when usage is absent from a streamed response, completion tokens fall back to the observed SSE chunk count. This can undercount for a non-conforming backend that both omits usage AND batches multiple tokens per chunk. In practice localperf always sends stream_options.include_usage=true and vLLM honors it, so the fallback is never exercised; it is strictly more honest than the prior behavior (which credited the requested output length). A tokenizer-based count would be the follow-up if a non-usage backend is ever targeted. |
Summary
The streaming TTFT path from PR #14 failed valid responses on reasoning and diffusion models.
When a re-run of the GB10 sweep hit the two Gemma models, their prefill points all failed with
response_shape: "stream produced no completion content"(diffusiongemma dropped from 31 completed to 12).Root cause: the parser only counted
delta.contentas a token, so reasoning models — which stream their first tokens asdelta.reasoning_content— and 1-token prefill points that finish with no streamed content were both rejected. It would also have nil-derefedfirstTokenAthad that path been reachable.What Changed
The stream parser now understands reasoning output and clean empty completions.
openAIMessagegainsreasoning_content;streamChoiceTextreturns it whencontentis empty, so a reasoning model's first token seeds TTFT and ITL timing.finish_reasonbut zero streamed tokens (a 1-token prefill, or a reasoning-only turn) now counts as completed rather than failing. It carries no TTFT — honest, not a fabricated zero.response_shape-malformed.applyToSampleno longer dereferencesfirstTokenAtwhen nil (TTFT/TPOT/ITL are set only when a token was actually observed).finalize()(keeps CRAP under budget).Testing
TestStreamingReasoningContentCountsForTTFT(reasoning_content seeds TTFT+ITL),TestStreamingFinishWithNoContentCompletes(clean zero-content finish completes with no TTFT). Retargeted the old no-content test to the genuinely-malformed case (TestStreamWithNeitherTokenNorFinishFailsShape).go test ./...,go vet,gofmt,simpledoc,slophammer check/dry,check-crap.sh,golangci-lint, dry-run + artifact check — all clean.content).Risks
Low. Non-streaming path untouched; qwen (normal content) behavior unchanged. The only behavior change is that clean zero-content streamed finishes now complete instead of failing, which matches the existing non-streaming zero-token handling.