Skip to content

MAINT: Consolidate OpenAI response handling - #2381

Open
Roman Lutz (romanlutz) wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-consolidate-openai-responses
Open

MAINT: Consolidate OpenAI response handling#2381
Roman Lutz (romanlutz) wants to merge 1 commit into
microsoft:mainfrom
romanlutz:romanlutz-consolidate-openai-responses

Conversation

@romanlutz

Copy link
Copy Markdown
Contributor

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

  • Added adapter contract and parity tests for malformed, filtered, truncated, and metadata-bearing responses across all three formats.
  • Added regression coverage ensuring protected truncation overrides remain honored.
  • Ran 336 relevant OpenAI target and parser tests.
  • Ran Ruff check and format, ty, all changed-file pre-commit hooks, and git diff --check.
  • Documentation changes were not applicable because public APIs and behavior are unchanged.
  • JupyText was not applicable.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@hannahwestra25 hannahwestra25 self-assigned this Aug 13, 2026
class OpenAIResponseAdapter(Protocol[ResponseT]):
"""The response-format contract used by ``OpenAITarget``."""

def is_content_filter(self, *, response: ResponseT) -> bool:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this makes it seem like we're detecting whether the repsonse is a content filter vs whether it was filtered so maybe rename to is_content_filtered

bool: True if content filter detected, False otherwise.
"""
return False
return self._response_adapter.is_content_filter(response=response)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we just eliminate these functions in favor of creating the adapter objects ?

...


class NoOpOpenAIResponseAdapter:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this noop class seems weird to me. can we just make the OpenAIResponseAdapter be a base class (rathter than a protocol) with this functionality ?

@hannahwestra25

Copy link
Copy Markdown
Contributor

should we also include OpenAI video ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants