MAINT: Consolidate OpenAI response handling - #2381
MAINT: Consolidate OpenAI response handling#2381Roman Lutz (romanlutz) wants to merge 2 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| bool: True if content filter detected, False otherwise. | ||
| """ | ||
| return False | ||
| return self._response_adapter.is_content_filter(response=response) |
There was a problem hiding this comment.
should we just eliminate these functions in favor of creating the adapter objects ?
There was a problem hiding this comment.
I kept these protected hooks as compatibility delegates. They predate this adapter and subclasses can override them; calling the adapter directly from _handle_openai_request_async would bypass those extension points. In particular, _validate_response deliberately computes truncation through the overridable _is_truncated_response hook before delegating to the adapter. The format-specific logic still lives in the adapters.
There was a problem hiding this comment.
but should we update the docstring to point people to the adapter ?
|
should we also include OpenAI video ? |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b3a8d42-bfe2-4b5e-90ae-fc211028c2ad
|
On including OpenAI video: I kept it out of this consolidation because video uses a distinct asynchronous job/status response model, while this PR is specifically aligning Chat Completions, Completions, and Responses text formats. Video currently only shares the base request wrapper and has its own content-filter/status construction semantics; forcing it into this adapter set would broaden the PR without meaningful shared validation, truncation, partial-content, or metadata behavior. The concrete base adapter still lets video retain its existing protected hook behavior, and the video target tests pass unchanged. |
hannahwestra25
left a comment
hannahwestra25
left a comment
There was a problem hiding this comment.
small nit related to docstring :)
Description
OpenAI Chat Completions, Completions, and Responses targets previously implemented response validation, filtering, truncation detection, partial-content extraction, and metadata capture across multiple target classes. This made behavior harder to compare and maintain as the API formats evolved.
This change introduces a small typed response-adapter seam with dedicated adapters for each wire format and a no-op default for other OpenAI targets. Existing protected target hooks remain compatibility delegates, including subclass truncation overrides. The consolidation preserves each format's existing behavior, including legacy Completions semantics, content-filter handling, partial output, stop reasons, token usage, and graceful truncated responses.
Tests and Documentation
git diff --check.