Skip to content

fix(perf): ignore metadata-only chunks in TTFT and ITL - #1645

Merged
Yunnglin merged 1 commit into
modelscope:mainfrom
Excelius-Wang:fix/ignore-empty-stream-metrics
Aug 31, 2026
Merged

Yunnglin merged 1 commit into
modelscope:mainfrom
Excelius-Wang:fix/ignore-empty-stream-metrics

Conversation

@Excelius-Wang

Copy link
Copy Markdown
Contributor

Problem

OpenAI-compatible chat streams can emit choices events without generated output. For example,
vLLM sends an initial role-only delta with content: "", and services may send a separate empty
delta carrying finish_reason.

DefaultApiPlugin.process_request() currently records TTFT for the first event with a non-empty
choices array and records every later choices event as an ITL sample, even when the extracted
text 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:

  • TTFT: 0.10s
  • ITL: [0.35s, 0.20s, 0.25s]

The output-based result should be:

  • TTFT: 0.45s
  • ITL: [0.20s]

Fix

Track the last non-empty output timestamp separately from the last response-event timestamp.

  • Update TTFT and ITL only when text or reasoning content is non-empty.
  • Keep the existing response timestamp for completion time and end-to-end query latency.
  • Continue retaining metadata events and parsing usage from them.

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.45 at the TTFT
assertion. After the fix:

  • 31 related offline tests pass on current main.
  • All configured pre-commit hooks pass for the two changed files.
  • compileall and git diff --check pass.

The regression also verifies that generated text, response retention, usage parsing, and
end-to-end query latency remain unchanged.

@Yunnglin

Copy link
Copy Markdown
Collaborator

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 Yunnglin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Excelius-Wang

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I've rebased the branch locally onto the latest main, resolved the Ruff conflict in default_api.py, and verified the focused offline suite (31 passed) along with all applicable pre-commit hooks.

The current main tip (44038031) has an unrelated duplicate evaluation_version keyword in ifbench_adapter.py, and all three main-branch checks are currently failing. I'll push the rebased branch once main is fixed so the PR can get a clean CI run.

@Excelius-Wang
Excelius-Wang force-pushed the fix/ignore-empty-stream-metrics branch from 06fcee5 to 3dec1e0 Compare August 30, 2026 15:52
@Yunnglin
Yunnglin merged commit addcc47 into modelscope:main Aug 31, 2026
0 of 6 checks passed
Yunnglin pushed a commit that referenced this pull request Oct 9, 2026
…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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants