fix(passthrough): use Responses API config for /v1/responses logging - #29666
fix(passthrough): use Responses API config for /v1/responses logging#29666DanielMaly wants to merge 1 commit into
Conversation
Greptile SummaryThis PR fixes a bug in the OpenAI passthrough logging handler where the
Confidence Score: 4/5The change is a targeted swap of one parser call for a correctly typed one, with a pre-existing outer try/except fallback in the handler, so a mis-parse cannot break the passthrough pipeline. The core handler change is straightforward and well-tested by both the updated existing test and the new regression test. The only observation is that the dual-path usage assertion in the tests could silently accept the model_construct fallback path instead of failing loudly on a schema mismatch, so a future regression there would be harder to catch. The test file's dual isinstance guard on usage (lines 696-704 and 768-775) deserves a second look to ensure it enforces the expected type rather than accepting both shapes.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_types.py | Adds ResponsesAPIResponse to the PassThroughEndpointLoggingResultValues union type — a necessary companion change to the handler fix. |
| litellm/proxy/pass_through_endpoints/llm_provider_handlers/openai_passthrough_logging_handler.py | Replaces OpenAIConfig.transform_response() (chat completions parser) with OpenAIResponsesAPIConfig.transform_response_api_response() for the is_responses branch, fixing zero-token logging. Also expands the endpoint_type debug string to cover "responses" and "unknown". |
| tests/test_litellm/proxy/pass_through_endpoints/llm_provider_handlers/test_openai_passthrough_logging_handler.py | Removes the get_provider_config mock that papered over the bug, now exercises the real OpenAIResponsesAPIConfig path. Adds a regression test confirming non-zero usage. Usage assertion uses a dual isinstance guard (dict vs model) that could silently mask model_construct fallback. |
Reviews (1): Last reviewed commit: "fix(passthrough): use Responses API conf..." | Re-trigger Greptile
| # usage may be a ResponseAPIUsage model or a plain dict depending on | ||
| # how the response was constructed (model_construct vs strict init) | ||
| usage = result["result"].usage | ||
| if isinstance(usage, dict): | ||
| assert usage["input_tokens"] == 20 | ||
| assert usage["output_tokens"] == 15 | ||
| else: | ||
| assert usage.input_tokens == 20 | ||
| assert usage.output_tokens == 15 |
There was a problem hiding this comment.
Dual-path usage assertion may silently accept model_construct fallback
The test accepts both dict and Pydantic-model shapes for usage. With valid JSON the strict ResponsesAPIResponse(...) constructor should always succeed, making usage a proper ResponseAPIUsage object — not a dict. If transform_response_api_response unexpectedly falls back to model_construct (e.g. a schema change), the dict branch would hide that regression. Asserting the concrete type with assert not isinstance(usage, dict) before the attribute access would make the intent explicit and catch the fallback earlier.
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!
There was a problem hiding this comment.
Good catch — addressed in the amended push. The test now asserts not isinstance(usage, dict) so any unexpected model_construct fallback would fail immediately rather than being silently accepted. Also added total_tokens to the mock usage dicts so the strict ResponsesAPIResponse() constructor succeeds (it was previously falling back to model_construct because total_tokens was missing).
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ugh logging The is_responses branch in openai_passthrough_logging_handler used OpenAIConfig().transform_response() (chat completions parser) which expects 'choices' in the response JSON. Responses API responses have 'output'/'usage' instead, causing APIError → empty ModelResponse → Langfuse logs input=0, output=0. Fix: use OpenAIResponsesAPIConfig().transform_response_api_response() for the is_responses branch, which correctly parses Responses API JSON. Also adds ResponsesAPIResponse to PassThroughEndpointLoggingResultValues union type so the return type is accurate. Refs: BerriAI#29575
eb5021e to
534e755
Compare
|
Closing as superseded by #29728, which merged the same Responses API passthrough transformer fix and also includes the outer dispatch-gate coverage and stronger dispatch tests. Thanks for getting this in! |
Problem
The
is_responsesbranch inopenai_passthrough_logging_handler.pycallsOpenAIConfig().transform_response()— the chat completions parser — for Responses API passthrough requests. This parser expects achoiceskey in the response JSON, but Responses API responses haveoutput/usageinstead, causing anAPIErrorand producing an emptyModelResponse.Downstream, Langfuse reads the empty
ModelResponseand logsinput_tokens=0, output_tokens=0, total_tokens=0even though LiteLLM's internal spend tracker has the correct token counts (tracked via a separate billing path).Fix
Use
OpenAIResponsesAPIConfig().transform_response_api_response()for theis_responsesbranch, which correctly parses Responses API JSON (output,usagewithinput_tokens/output_tokens) and returns a properResponsesAPIResponse.Also:
ResponsesAPIResponseto thePassThroughEndpointLoggingResultValuesunion typeendpoint_typedebug log to include"responses"test_responses_api_cost_trackingtest to exercise the real code path (removed mock ofget_provider_configthat was papering over the bug)test_responses_api_returns_usage_not_zeroregression test documenting the root causeRelationship to PR #29574
This PR is a separate, complementary fix to #29574:
langfuse.py) to correctly read usage fromResponsesAPIResponseobjectsResponsesAPIResponseobjects instead of emptyModelResponseobjectsBoth fixes are needed for complete Langfuse observability of Responses API traffic through passthrough endpoints.
Refs: #29575