nes: measure ttft only when we get content #2387
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the accuracy of TTFT (Time To First Token) measurement in the xtab provider by ensuring it's only recorded when actual content is received, not on empty string responses. This prevents premature TTFT measurements that could occur when the streaming API sends an initial empty response before sending actual content.
Key Changes
- Added a non-empty text check (
text !== '') to the TTFT measurement condition in the streaming callback
| firstTokenReceived.complete(); | ||
| } | ||
| if (ttft === undefined) { | ||
| if (ttft === undefined && text !== '') { |
There was a problem hiding this comment.
[nitpick] Consider checking the delta parameter for other content types beyond text. The FetchStreamRecorder class in src/platform/chat/common/chatMLFetcher.ts (line 128) uses a more comprehensive check that includes delta.text, delta.beginToolCalls, delta.thinking?.text, and delta.copilotToolCalls.
If this provider can receive non-text content like tool calls or thinking tokens, TTFT should be measured when any of these are present, not just when text is non-empty. For example:
if (ttft === undefined && (text !== '' || delta.beginToolCalls || delta.thinking?.text || delta.copilotToolCalls)) {However, if this specific context only deals with text responses, the current implementation is sufficient.
| if (ttft === undefined && text !== '') { | |
| if ( | |
| ttft === undefined && | |
| ( | |
| text !== '' || | |
| (delta && ( | |
| delta.beginToolCalls || | |
| (delta.thinking && delta.thinking.text) || | |
| delta.copilotToolCalls | |
| )) | |
| ) | |
| ) { |
No description provided.