Repository navigation
fix(responses): unwrap completed response for cost tracking on bridge path - #38188
willcai1984 wants to merge 1 commit into
Conversation
|
|
Greptile SummaryThis PR unwraps completed Responses API events before success logging so force-streamed responses drained by non-streaming bridge callers can produce standard cost records. It also includes broader provider, health-check, authentication, stream-reconstruction, passthrough-header, and Anthropic token-counting changes.
Confidence Score: 4/5The PR appears safe to merge from a behavioral perspective, with only non-blocking repository-style issues in the newly added source. The completed-response logging fix and surrounding stream changes have focused regression coverage, and the accepted findings concern added comments and annotation syntax rather than runtime correctness. Files Needing Attention: litellm/llms/chatgpt/responses/transformation.py, litellm/responses/streaming_iterator.py
|
| Filename | Overview |
|---|---|
| litellm/responses/streaming_iterator.py | Adds response-output reconstruction and unwraps completed events for success logging; no concrete blocking failure was established. |
| litellm/llms/chatgpt/responses/transformation.py | Adds per-auth-file credential selection, request normalization, and SSE output cleanup, with repository-style violations in the new implementation. |
| litellm/main.py | Allows provider configurations to select a default health-check mode when callers omit one. |
| litellm/llms/anthropic/count_tokens/transformation.py | Resolves the count-token endpoint from a custom Anthropic API base while preserving the default endpoint. |
| litellm/passthrough/utils.py | Prevents client Accept-Encoding negotiation from being forwarded to upstream passthrough targets. |
| tests/test_litellm/responses/test_streaming_iterator.py | Adds sync and async regression coverage confirming success handlers receive an unwrapped ResponsesAPIResponse. |
Reviews (1): Last reviewed commit: "fix(responses): unwrap completed respons..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # v3: v1.94.0 的 anthropic->responses 调用链会丢失 deployment 的 | ||
| # chatgpt_auth_file 参数（get_litellm_params 白名单过滤 + 中途 kwargs 重建）， | ||
| # 导致多席位全部回退默认 auth.json。此处按 model_info.id 反查 auth 文件兜底。 | ||
| # v7: /v1/messages 桥接路径在 config 模式下同样丢参（terra 实为命中本映射才正常）， | ||
| # 新增 chatgpt/gpt-5.6-luna 席位（2026-08-19 上线，与 terra 同批 seat01+seat03）。 |
There was a problem hiding this comment.
The newly added implementation-history comment conflicts with the repository's no-new-comments convention and increases the maintenance burden; the same pattern occurs in the new logging and token-counting code.
Context Used: CLAUDE.md (source)
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!
| self._authenticators: Dict[str, Authenticator] = {} | ||
| self.authenticator = self._get_authenticator(None) | ||
|
|
||
| def _get_authenticator(self, auth_file: Optional[str]) -> Authenticator: |
There was a problem hiding this comment.
The new authentication helpers use Optional[str] and Optional[GenericLiteLLMParams] instead of the repository-standard T | None syntax, adding an unnecessary Optional import and diverging from the established typing convention.
Rule Used: In this repo, prefer str | None (PEP 604 union s... (source)
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!
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
… path When a non-streaming caller drains a Responses API streaming iterator (e.g. the Anthropic /v1/messages -> Responses API bridge against a force-streaming provider like chatgpt/*), _log_completed_response passed the ResponseCompletedEvent wrapper to dispatch_success_handlers. The success handlers only unwrap it in their assembled-stream branch, which a non-streaming caller never reaches — so standard_logging_object was never built, and _PROXY_track_cost_callback raised "Cost tracking failed: standard_logging_object not found". Fix: add _response_for_success_logging() that copies and unwraps .completed_response to the inner ResponsesAPIResponse before passing it to the success handlers. This lets _is_recognized_call_type_for_logging match in the non-streaming branch so standard_logging_object is built. Related: BerriAI#36426, BerriAI#36450
|
Rebuilt this branch from the latest The previous head accidentally carried a batch of unrelated local ops patches (health-check, count-tokens, seat mapping, transformation snapshots), which is what made the diff 14 files and triggered the conflicting state and the earlier review threads — those all applied to now-removed commits. New diff: 2 files, +83/−7. Happy to re-request review once CI is green. |
2466d14 to
6a4e558
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Closing after rebasing onto current staging revealed the original bridge cost-logging failure is already fixed upstream: |
2 participants
What does this PR do?
Fixes a Cost tracking failed: standard_logging_object not found error that occurs when a non-streaming caller drains a Responses API streaming iterator — most notably the Anthropic /v1/messages to Responses API bridge path against force-streaming providers like chatgpt/*.
The problem
When Claude Code sends a streaming /v1/messages request to a ChatGPT model, the bridge calls litellm.aresponses(stream=True). The inner Responses API streaming iterator completes and calls _log_completed_response, which passes self.completed_response (a ResponseCompletedEvent wrapper) to dispatch_success_handlers.
The success handlers only unwrap ResponseCompletedEvent to ResponsesAPIResponse in their assembled-stream branch, which a non-streaming caller draining the iterator never reaches. So _success_handler_helper_fn sees result=None (for the proxy deferred-logging path) or the unrecognised wrapper event, never builds standard_logging_object, and _PROXY_track_cost_callback raises:
Cost tracking failed for model=gpt-5.6-luna. Debug info - standard_logging_object not found
This produced ~550 error log lines per 6 hours in production (51% of all successful /v1/messages requests). Verified harmless: LiteLLM_SpendLogs and LiteLLM_DailyUserSpend reconcile exactly — the SpendLog row is written later by the iterator's own async success callback.
The fix
Add _response_for_success_logging() that copies self.completed_response via model_dump/model_validate (preserving the existing fix for #17192) and unwraps .response to the inner ResponsesAPIResponse before passing it to success handlers. This lets _is_recognized_call_type_for_logging match in the non-streaming branch so standard_logging_object and response_cost are built correctly.
Related
Test plan