fix(streaming): stamp completion_start_time on first chunk for /v1/messages and /v1/responses - #32284
Conversation
Greptile SummaryThis PR fixes a missing
Confidence Score: 4/5The change is narrowly scoped to two streaming paths that previously never set completion_start_time; the write-once guard ensures nothing is overwritten if an outer wrapper already stamped it, and the control path (chat/completions) is entirely untouched. All changes are correct and well-tested. The only note is a minor code duplication in streaming_handler.py where the identical two-line stamp block is copy-pasted into both branches rather than extracted once before the branch — no behavioral impact but worth tidying. No files require special attention; all modified files are straightforward and the new test file follows existing conventions.
|
| Filename | Overview |
|---|---|
| litellm/proxy/pass_through_endpoints/streaming_handler.py | Stamps completion_start_time on the first upstream byte in both the hot path and the cost-injection branch via a write-once guard; logic is correct and handles both branches. |
| litellm/responses/streaming_iterator.py | Stamps completion_start_time on the first non-empty, non-DONE SSE data field in _process_chunk; write-once guard is correct and consistent with the streaming handler approach. |
| tests/test_litellm/proxy/pass_through_endpoints/test_streaming_handler_interrupt.py | Three new mock-only regression tests: first-chunk stamp, write-once no-overwrite, and cost-injection branch stamp. All tests are well-structured with no real network calls. |
| tests/test_litellm/responses/test_streaming_iterator.py | New test file with two mock-only tests covering first-chunk stamp and no-overwrite of prior completion_start_time for the ResponsesAPI streaming iterator. |
| tests/llm_responses_api_testing/test_base_responses_api_streaming_iterator.py | Existing tests updated to set completion_start_time = None on mocks so spec-constrained Mocks expose the attribute; no behavioral weakening of existing assertions. |
Reviews (1): Last reviewed commit: "fix(streaming): stamp completion_start_t..." | Re-trigger Greptile
| async for chunk in response.aiter_bytes(): | ||
| raw_bytes.append(chunk) | ||
| if litellm_logging_obj.completion_start_time is None: | ||
| litellm_logging_obj._update_completion_start_time(completion_start_time=datetime.now()) | ||
| if endpoint_type == EndpointType.VERTEX_AI: |
There was a problem hiding this comment.
The identical two-line stamp block appears in both the hot path and the cost-injection branch. The guard could live once, right after
raw_bytes.append(chunk), before the if endpoint_type branch — removing the duplication without changing behavior.
| async for chunk in response.aiter_bytes(): | |
| raw_bytes.append(chunk) | |
| if litellm_logging_obj.completion_start_time is None: | |
| litellm_logging_obj._update_completion_start_time(completion_start_time=datetime.now()) | |
| if endpoint_type == EndpointType.VERTEX_AI: | |
| async for chunk in response.aiter_bytes(): | |
| raw_bytes.append(chunk) | |
| if litellm_logging_obj.completion_start_time is None: | |
| litellm_logging_obj._update_completion_start_time( | |
| completion_start_time=datetime.now() | |
| ) | |
| if endpoint_type == EndpointType.VERTEX_AI: |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Greptile SummaryThis PR fixes missing
Confidence Score: 5/5Safe to merge — the change is a targeted, write-once telemetry stamp applied only on the first upstream chunk in two previously-unpatched streaming paths, with no effect on request routing, response content, or auth. Both production changes are minimal and isolated to the TTFT telemetry path. The write-once guard (is None) is correctly implemented and consistent with the existing CustomStreamWrapper pattern. datetime is imported in both modified files, _update_completion_start_time initializes the field to None in Logging.init, and the method is a simple two-line setter. Existing tests have been correctly updated to set the mock attribute to None so the guard is properly exercised. New regression tests cover both streaming branches. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/pass_through_endpoints/streaming_handler.py | Adds write-once completion_start_time stamping on first chunk in both the hot path and cost-injection path; guard and placement are correct. |
| litellm/responses/streaming_iterator.py | Stamps completion_start_time before JSON parsing in _process_chunk; correctly guards with both None-check on logging_obj and is-None on the attribute. |
| tests/test_litellm/responses/test_streaming_iterator.py | New regression test file with mock-only tests; verifies stamp fires exactly once on first chunk and is skipped when already set. |
| tests/test_litellm/proxy/pass_through_endpoints/test_streaming_handler_interrupt.py | Adds three new regression tests for completion_start_time stamping in both streaming handler branches (hot path and cost-injection path). |
| tests/llm_responses_api_testing/test_base_responses_api_streaming_iterator.py | Existing tests updated to set mock_logging_obj.completion_start_time = None, required so Mock(spec=...) doesn't return a truthy stub that bypasses the new is-None guard. |
Reviews (2): Last reviewed commit: "fix(streaming): stamp completion_start_t..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
b1e5d6f to
c0e8e68
Compare
…ssages and /v1/responses Streaming pass-through for native Anthropic /v1/messages and the /v1/responses streaming iterator never set logging_obj.completion_start_time, so _success_handler_helper_fn fell back to completion_start_time = end_time. Downstream TTFT consumers (Prometheus, OTEL, Langfuse, Admin UI, spend logs completionStartTime) then reported time-to-first-token equal to total request duration. Stamp completion_start_time on the first chunk in PassThroughStreamingHandler. chunk_processor and BaseResponsesAPIStreamingIterator._process_chunk, mirroring CustomStreamWrapper for /chat/completions. Resolves LIT-4185
c0e8e68 to
fff0039
Compare
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit fff0039. Configure here.
Relevant issues
Linear ticket
Resolves LIT-4185
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Live proxy + Postgres, hitting real provider APIs. Config declares a native Anthropic model, an OpenAI model, and a custom callback that prints what
async_log_success_eventreceives (completion_start_time - start_timevsend_time - start_time).Same three streaming curls on unfixed and fixed code; SpendLogs is the persisted signal Prometheus/OTEL/UI read.
Before (unfixed code)
Callback (only
acompletionreports honest TTFT; the other two hit the bug):SpendLogs (Prometheus / OTEL / Admin UI all read
completionStartTimefrom this row):anthropic_messagescollapses tocst_equals_end=texplicitly.aresponsesshowsttft = total - 1ms(the fallback in_success_handler_helper_fnsetscompletion_start_time = end_time).acompletionis the control and stays honest.After (fixed code)
Callback:
SpendLogs (top three rows are the fixed run, bottom three are the unfixed run for direct comparison):
TTFT is a real fraction of total for both previously-broken paths; the control keeps working; nothing collapses to
cst_equals_endType
🐛 Bug Fix
Changes
Streaming pass-through for native Anthropic
/v1/messages(PassThroughStreamingHandler.chunk_processor) and the/v1/responsesstreaming iterator (BaseResponsesAPIStreamingIterator._process_chunk) never setlogging_obj.completion_start_time.Logging._success_handler_helper_fnthen fell back tocompletion_start_time = end_time, so every downstream TTFT consumer (Prometheus, OTEL, Langfuse, Admin UI, spend logscompletionStartTime, custom callbacks) reported time-to-first-token equal to total request duration./v1/chat/completionsstreaming was unaffected becauseCustomStreamWrapperalready stamps the first chunkThe fix stamps
completion_start_timeon the first upstream byte in each of those two spots vialogging_obj._update_completion_start_time(...), guarded onis Noneso it's write-once and never overwrites a real value if something upstream already stampedThe agentic-hook path for
/v1/messages(AgenticAnthropicStreamingIterator) wraps the samechunk_processorstream, so fixing the underlyingchunk_processorcovers it transitively; no separate change is needed in the agentic iteratorNote
Low Risk
Narrow observability fix on streaming hot paths with write-once guards; no auth, billing, or API contract changes beyond more accurate TTFT timestamps.
Overview
Fixes LIT-4185: streaming on native Anthropic
/v1/messages(pass-through) and/v1/responsesnever setlogging_obj.completion_start_time, so success logging fell back tocompletion_start_time = end_timeand TTFT looked like full request duration in SpendLogs, Prometheus, OTEL, and callbacks. Chat completions streaming was already correct viaCustomStreamWrapper.Pass-through (
PassThroughStreamingHandler): new_stamp_first_chunk_if_neededruns on every upstream byte chunk (normal and cost-injection paths) and calls_update_completion_start_timeonly whencompletion_start_time is None.Responses API (
BaseResponsesAPIStreamingIterator._process_chunk): same write-once stamp on the first non-empty SSE payload (after[DONE]handling, before JSON parse).Tests add LIT-4185 regressions (first-chunk stamp, no overwrite on later chunks, cost-injection path) and align mocks with
completion_start_time = None.Reviewed by Cursor Bugbot for commit fff0039. Bugbot is set up for automated code reviews on this repo. Configure here.