Skip to content

feat(providers): add OpenAI API package and migrate capabilities to mixins#82

Merged
Kamilbenkirane merged 8 commits intomainfrom
api/openai
Dec 15, 2025
Merged

feat(providers): add OpenAI API package and migrate capabilities to mixins#82
Kamilbenkirane merged 8 commits intomainfrom
api/openai

Conversation

@Kamilbenkirane
Copy link
Copy Markdown
Member

Add standalone provider package for OpenAI APIs with mixin pattern for capability-agnostic reuse across multiple endpoints. Migrate all capability clients to use the new provider mixins, eliminating code duplication and centralizing API-specific logic.

Provider Package (celeste-openai)

Images API (OpenAIImagesClient)

  • HTTP POST/streaming to /v1/images/generations endpoint
  • Supports DALL-E 2/3 (b64_json format) and gpt-image-1 (streaming)
  • Usage parsing: input_tokens, output_tokens, total_tokens
  • Content extraction from data array
  • Metadata building with model, size, quality, style, revised_prompt

Responses API (OpenAIResponsesClient)

  • HTTP POST/streaming to /v1/responses endpoint
  • Unified API for text generation capabilities
  • Usage parsing with cached_tokens and reasoning_tokens support
  • Content extraction from output array
  • Finish reason mapping (completed status)
  • Structured output support via response_format

Videos API (OpenAIVideosClient)

  • Async polling workflow for video generation
  • Phase 1: POST to /v1/videos to create job
  • Phase 2: Poll GET /v1/videos/{id} until completed/failed
  • Phase 3: GET /v1/videos/{id}/content to retrieve video
  • Usage parsing: billing_units (seconds)
  • Multipart request support for input_reference images

Audio API (OpenAIAudioClient)

  • HTTP POST to /v1/audio/speech endpoint
  • Binary audio response handling
  • Response format mapping (mp3, opus, aac, flac, wav, pcm)
  • MIME type conversion to AudioMimeType

All clients follow the mixin pattern for reuse across capabilities.

Capability Migrations

Image Generation

  • OpenAI client inherits from OpenAIImagesClient mixin
  • Parameter mappers inherit from provider package mappers (AspectRatioMapper, SizeMapper, QualityMapper, StyleMapper)
  • Remove unused config.py file (config now in provider package)
  • Code reduction: ~188 lines removed

Speech Generation

  • OpenAI client inherits from OpenAIAudioClient mixin
  • Parameter mappers inherit from provider package mappers (VoiceMapper, SpeedMapper, OutputFormatMapper)
  • Add SpeechGenerationFinishReason type for consistency
  • Remove unused config.py file
  • Fix: Rename RESPONSE_FORMATOUTPUT_FORMAT in ElevenLabs models
  • Code reduction: ~187 lines removed

Text Generation

  • OpenAI client inherits from OpenAIResponsesClient mixin
  • Parameter mappers inherit from provider package mappers (TemperatureMapper, MaxTokensMapper, TopPMapper, FrequencyPenaltyMapper, PresencePenaltyMapper, StopSequencesMapper, ResponseFormatMapper)
  • Streaming class inherits from OpenAIResponsesStream mixin
  • Remove unused config.py file
  • Code reduction: ~429 lines removed

Video Generation

  • OpenAI client inherits from OpenAIVideosClient mixin
  • Parameter mappers inherit from provider package mappers (AspectRatioMapper, DurationMapper)
  • Remove unused config.py file
  • Remove async polling logic (now in provider mixin)
  • Code reduction: ~178 lines removed

Other Changes

Fixes

  • Add _parse_usage() to GoogleVeoClient mixin (returns empty dict, Veo API doesn't provide usage)
  • Add celeste-openai workspace dependency to all capability pyproject.toml files

Code Statistics

  • Files changed: 51
  • Lines added: 1,383
  • Lines removed: 924
  • Net reduction: 459 lines
  • Provider package: 18 Python files across 4 API modules

Breaking Changes

Speech Generation Parameter Rename

  • SpeechGenerationParameter.RESPONSE_FORMATSpeechGenerationParameter.OUTPUT_FORMAT
  • Affects: SpeechGenerationParameters class, all provider models (ElevenLabs, OpenAI), and parameter mappers
  • Migration: Update code using RESPONSE_FORMAT to use OUTPUT_FORMAT instead

Add standalone provider package for OpenAI APIs with mixin pattern
for capability-agnostic reuse across multiple endpoints.

## Images API (OpenAIImagesClient)
- HTTP POST/streaming to /v1/images/generations endpoint
- Supports DALL-E 2/3 (b64_json format) and gpt-image-1 (streaming)
- Usage parsing: input_tokens, output_tokens, total_tokens
- Content extraction from data array
- Revised prompt handling in metadata

## Responses API (OpenAIResponsesClient)
- HTTP POST/streaming to /v1/responses endpoint
- Unified API for text generation capabilities
- Usage parsing with cached and reasoning tokens support
- Content extraction from output array
- Finish reason mapping (completed status)

## Videos API (OpenAIVideosClient)
- Async polling workflow for video generation
- Phase 1: POST to /v1/videos to create job
- Phase 2: Poll GET /v1/videos/{id} until completed/failed
- Phase 3: GET /v1/videos/{id}/content to retrieve video
- Usage parsing: billing units (seconds)
- Multipart request support for input_reference images

## Audio API (OpenAIAudioClient)
- HTTP POST to /v1/audio/speech endpoint
- Binary audio response handling
- Response format mapping (mp3, opus, aac, flac, wav, pcm)
- MIME type conversion to AudioMimeType

All clients follow the mixin pattern for reuse across capabilities.
Add _parse_usage method to GoogleVeoClient to resolve mypy type checking error.
GoogleVeoClient was missing this implementation, causing unsafe super() call
in GoogleVideoGenerationClient.

Google Veo API doesn't return usage data in the response, so this method
returns an empty dict that capability clients can wrap in their Usage type.
Standardize parameter naming across speech generation capability.
Changes unified parameter name from RESPONSE_FORMAT to OUTPUT_FORMAT
to match enum definition and be more consistent with other capabilities.

Breaking change: Updates parameter name in:
- SpeechGenerationParameter enum
- SpeechGenerationParameters class
- All ElevenLabs models (9 occurrences)
- All OpenAI models (3 occurrences)
- Provider parameter mappers
Migrate image generation capability clients to use provider package mixins,
eliminating code duplication and centralizing API-specific logic.

## Changes
- OpenAI client now inherits from OpenAIImagesClient mixin
- Parameter mappers inherit from provider package mappers
- Google client uses super()._parse_usage() pattern
- Remove unused config.py file (config now in provider package)
- Remove revised_prompt handling from provider mixin (handled in capability)

## Code Reduction
- ~188 lines removed across client and parameter files
- Significant deduplication of HTTP request logic
…r mixins

Migrate speech generation capability clients to use provider package mixins,
eliminating code duplication and centralizing API-specific logic.

## Changes
- OpenAI client now inherits from OpenAIAudioClient mixin
- Parameter mappers inherit from provider package mappers
- Add SpeechGenerationFinishReason type for consistency
- Remove unused config.py file (config now in provider package)
- Update _create_inputs to use parameters.get() pattern
- Simplify VoiceConstraint docstring
- Update tests to reflect new structure

## Code Reduction
- ~187 lines removed across client and parameter files
- Significant deduplication of HTTP request and parameter mapping logic
Migrate video generation capability clients to use provider package mixins,
eliminating code duplication and centralizing API-specific logic.

## Changes
- OpenAI client now inherits from OpenAIVideosClient mixin
- Parameter mappers inherit from provider package mappers
- Google client uses super()._parse_usage() pattern (after Commit 1 fix)
- Remove unused config.py file (config now in provider package)
- Remove async polling logic (now in provider mixin)
- Simplify _parse_usage to use mixin's implementation

## Code Reduction
- ~178 lines removed across client and parameter files
- Significant deduplication of HTTP request and polling logic
…ve config files

Migrate text generation capability client to use provider package mixins,
eliminating code duplication and centralizing API-specific logic.

## Changes
- OpenAI client now inherits from OpenAIResponsesClient mixin
- Parameter mappers inherit from provider package mappers
- Streaming class inherits from OpenAIResponsesStream mixin
- Remove unused config.py file (config now in provider package)
- Simplify _parse_content to use mixin's output array parsing
- Simplify _parse_finish_reason to use mixin's implementation

## Code Reduction
- ~429 lines removed across client, parameters, and streaming files
- Significant deduplication of HTTP request, streaming, and schema logic
Add celeste-openai to [tool.uv.sources] in all capability packages that
now import from the OpenAI provider package after the mixin migration.

## Changes
- image-generation: Add celeste-openai (imports celeste_openai.images)
- speech-generation: Add celeste-openai (imports celeste_openai.audio)
- text-generation: Add celeste-openai (imports celeste_openai.responses)
- video-generation: Add celeste-openai (imports celeste_openai.videos)

This ensures workspace dependencies are properly declared for the
refactored capability clients that use OpenAI provider mixins.
@claude
Copy link
Copy Markdown

claude Bot commented Dec 15, 2025

Pull Request Review: OpenAI Provider Package & Mixin Migration

This is a well-architected refactoring that successfully eliminates code duplication through a mixin pattern. The PR achieves a net reduction of 459 lines while centralizing OpenAI API logic.


✅ Strengths

Architecture & Design

  • Excellent mixin pattern with clean separation of concerns
  • Good abstraction boundaries - mixins return raw data that capability clients transform
  • Proper MRO inheritance order throughout

Code Quality

  • Comprehensive type annotations with proper type: ignore comments
  • Consistent error handling with descriptive messages
  • Good documentation with clear usage examples

Breaking Change Handling

  • Well documented RESPONSE_FORMAT → OUTPUT_FORMAT rename
  • Complete migration across all affected files

🔍 Issues & Concerns

1. Missing Test Coverage for New Provider Package ⚠️ HIGH PRIORITY

The new celeste-openai provider package has zero tests. Core functionality like HTTP requests, usage parsing, SSE streaming, and async polling is untested at the provider level.

Recommendation: Add unit tests covering:

  • Mocking HTTP responses and verifying request construction
  • Usage parsing edge cases (missing fields, None values)
  • SSE event parsing in streaming
  • Async polling timeout/failure scenarios

Note: Integration tests exist at the capability level, but unit tests at the provider level are essential for faster feedback and easier debugging.


2. Potential Configuration Issue in Videos Client ⚠️ MEDIUM PRIORITY

File: packages/providers/openai/src/celeste_openai/videos/client.py:90-115

The polling loop has hardcoded configuration:

  • Fixed 5-minute timeout (60 polls × 5s) may be insufficient for longer videos
  • No user configurability
  • Error message doesn't account for actual API request time

Recommendation: Consider making polling configuration injectable or model-specific, and add exponential backoff.


3. Inconsistent Finish Reason Handling ⚠️ LOW-MEDIUM PRIORITY

Mixins return FinishReason but capability clients override to return their specific types (ImageGenerationFinishReason, etc.). This creates type mismatches and redundant overrides.

Recommendation: Have mixins return dict or None, matching the pattern used for _parse_usage().


4. Parameter Name Inconsistency ⚠️ LOW PRIORITY

File: packages/capabilities/speech-generation/src/celeste_speech_generation/providers/openai/client.py:74-76

Using SpeechGenerationParameter.OUTPUT_FORMAT as dict key instead of string "output_format".


5. Empty py.typed File ⚠️ LOW PRIORITY

Consider adding a comment to clarify it's a PEP 561 marker file.


6. Hardcoded Model Logic in Mixin ⚠️ LOW PRIORITY

File: packages/providers/openai/src/celeste_openai/images/client.py:44-47

Model-specific DALL-E logic in mixin creates coupling. Consider moving to capability client.


🎯 Best Practices Validation

Security

  • No injection vulnerabilities
  • Proper authentication header handling
  • Safe base64 encoding/decoding
  • Image upload size validation

Performance

  • Async/await used correctly
  • Efficient streaming
  • Non-blocking polling with asyncio.sleep

Code Organization

  • Clear module structure
  • Proper all exports
  • Config constants separated

📋 Recommendations Summary

Critical: Add unit tests for celeste-openai provider package

Should Address: Review polling configuration, refactor finish reason handling

Nice to Have: Fix parameter access pattern, add py.typed comment, extract model logic


📊 Migration Impact

Breaking Changes: RESPONSE_FORMAT → OUTPUT_FORMAT is well-documented and consistently applied.

Recommendation: Consider deprecation warnings before complete removal for external users.


🎓 Conclusion

This is a high-quality refactoring with excellent mixin pattern implementation. The main concern is lack of dedicated tests for the provider package.

Recommendation: Approve after addressing test coverage (or create follow-up issue).

Great work - the codebase is significantly cleaner and more maintainable!


Reviewed with: Code analysis, architecture review, security scan, best practices validation

@Kamilbenkirane Kamilbenkirane merged commit 415a614 into main Dec 15, 2025
11 checks passed
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.

1 participant