fix: request streaming usage in Anthropic translation - #826
Conversation
935db50 to
9663554
Compare
There was a problem hiding this comment.
"basic-stream recordings for openai and vllm now include stream_options.include_usage on the upstream request and the replay helper mirrors that. However, upstream SSE recordings still end at the finish_reason chunk with usage:null and no trailing usage-only chunk. Expected client message_delta in the openai fixture still shows output_tokens:0 and input_tokens:null, so fixture replay does not demonstrate restored billing counters end-to-end."
There was a problem hiding this comment.
Thanks for pointing out the issue. I'll re-record the OpenAI fixture.
For vLLM, is there a shared environment I can use, or should the vLLM fixture stay as-is for now? It still passes and covers the fallback when the backend doesn't return usage.
praxis-bot
left a comment
There was a problem hiding this comment.
PR Review
Summary: Injects stream_options.include_usage on translated streaming requests and maps the upstream usage chunk into Anthropic message_delta usage fields.
Overall: Well-implemented with thorough test coverage. The streaming usage mapping is correct, including the cache subtraction logic using saturating_sub. One doc comment was inadvertently split.
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 0 |
| Medium | 1 |
Findings without inline placement
(none)
|
|
||
| /// Map Anthropic parameters to Chat Completions-compatible equivalents. | ||
| /// | ||
| /// Copy `stream` and request streaming usage when enabled. |
There was a problem hiding this comment.
[Medium] The convert_stream function was inserted between lines 1 and 3 of map_parameters' existing doc comment, splitting it. As a result, convert_stream now has map_parameters' summary ("Map Anthropic parameters to Chat Completions-compatible equivalents.") as its first doc line, which is inaccurate, and map_parameters lost its summary (now starting with the top_k detail paragraph).
Replace convert_stream's doc with an accurate single-line summary and restore map_parameters' summary:
/// Copy `stream` and inject `stream_options.include_usage` when enabled.
fn convert_stream(...)
/// Map Anthropic parameters to Chat Completions-compatible equivalents.
///
/// `top_k` has no standard Chat Completions equivalent but is preserved
/// as an extra body parameter for backends that support it
/// (e.g. vLLM).
fn map_parameters(...)There was a problem hiding this comment.
It's addressed in the latest commit.
|
Unsigned commits: 3fd7ce6, 9663554. Please sign your commits. |
|
AI tool authorship detected:
Sorry, this project does not accept commits authored by tools as valid. |
c8ed0f3 to
c5780c4
Compare
|
@EleanorWho please rebase |
…ation The Anthropic→OpenAI request translator now sets stream_options.include_usage=true for streaming requests so the Chat Completions backend returns token usage in its final chunk. The stream event transformer extracts prompt_tokens and prompt_tokens_details.cached_tokens from the usage chunk and maps them into the Anthropic message_delta usage fields, applying the same cache subtraction logic as the non-streaming path. Closes praxis-proxy#803 Signed-off-by: Eleanor Hu <ehu@redhat.com>
- Extract convert_stream() and collect_delta_usage() to satisfy clippy::too_many_lines. - Consolidate MessageDeltaUsage constructors into a single new(). - Add stream_options.include_usage to inference fixture recordings and the replay test helper. - Apply nightly rustfmt formatting. Signed-off-by: Eleanor Hu <ehu@redhat.com>
c5780c4 to
1b42f3d
Compare
…ream fixture Restore the map_parameters doc comment summary that was split by the convert_stream insertion. Re-record the OpenAI basic-stream fixture against a live provider with include_usage enabled, so the upstream response contains real token counts and the client message_delta demonstrates restored billing counters end-to-end. Signed-off-by: Eleanor Hu <ehu@redhat.com>
|
@leseb Hi Seb, can you please take a look at the recording fixture in the latest commit? The previous OpenAI recording had |
leseb
left a comment
There was a problem hiding this comment.
Two actionable fixture-governance issues remain:
The vLLM live fixture’s request was manually updated without re-recording its response. It still lacks the trailing usage chunk, although vLLM emits one for include_usage: true. Re-record it truthfully. vLLM documentation
coverage.yaml needs explicit feature inventory for the new streaming usage request/response transformation, per AGENTS.md, followed by README regeneration.
The provenance change is correct. Since this fixture was captured using cargo. |
Summary
stream_options.include_usage = trueon the translated Chat Completions request when an Anthropic streaming request is forwarded, so the backend returns token usage in its final chunk.prompt_tokens,completion_tokens, andprompt_tokens_details.cached_tokensfrom the upstream usage chunk and map them into the Anthropicmessage_deltausage fields.input_tokens = prompt_tokens - cached_tokens).Closes #803
Test plan
streaming_request_includes_usage_option— verifiesstream_options.include_usageis injected forstream: truenon_streaming_request_omits_stream_options— verifies nostream_optionswithout streamingstream_false_omits_stream_options— verifies nostream_optionsforstream: falsemessage_delta_usage_matches_anthropic_schema— verifiesoutput_tokensandinput_tokensare mappedmessage_delta_usage_with_cached_tokens— verifiesinput_tokens = prompt_tokens - cached_tokensandcache_read_input_tokensis setmessage_delta_usage_without_usage_chunk— verifies graceful default when no usage chunk is presentall_inference_fixtures_replay— updatedbasic-streamfixtures for openai and vllm providersmake testpassing (all unit + integration tests)make lintpassing (clippy + nightly fmt + machete)🤖 Generated with Claude Code