-
Notifications
You must be signed in to change notification settings - Fork 1k
Feature/add support for OpenAI text to speech models #2829
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feature/add support for OpenAI text to speech models #2829
Conversation
…ricing, although with hardcoded open-ai tts values
…ricing, although with hardcoded open-ai tts values
apps/opik-backend/src/main/java/com/comet/opik/domain/cost/CostService.java
Outdated
Show resolved
Hide resolved
apps/opik-backend/src/main/java/com/comet/opik/domain/cost/SpanCostCalculator.java
Show resolved
Hide resolved
apps/opik-backend/src/test/java/com/comet/opik/domain/cost/CostServiceTest.java
Outdated
Show resolved
Hide resolved
apps/opik-backend/src/main/java/com/comet/opik/domain/cost/CostService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces comprehensive support for OpenAI Text-to-Speech (TTS) models by implementing end-to-end tracking, cost calculation, and UI integration. The changes enable Opik to trace audio.speech
requests, calculate costs based on character usage, and display TTS-specific data in the frontend.
- Implements TTS request tracing for both sync and streaming speech generation with audio attachment support
- Adds character-based cost calculation for TTS models (tts-1, tts-1-hd) in the backend pricing system
- Extends the frontend UI with a "Chars" column to display character count usage across traces, spans, threads, and projects
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
test_openai_speech.py |
Comprehensive integration tests for sync and streaming TTS tracing |
openai_speech_decorator.py |
Main decorator implementing TTS request tracking with audio buffer management |
speech_stream_*.py |
Supporting classes for streaming TTS: buffer management and event aggregation |
opik_usage*.py |
Usage tracking extensions to support character-based metrics for TTS models |
opik_tracker.py |
Integration of speech decorator into the main OpenAI tracking system |
stream_patchers.py |
Enhanced stream patching to support TTS chunk processing |
Frontend components | Addition of "Chars" column across TracesSpansTab, ThreadsTab, and ProjectsPage |
Backend cost system | Extended ModelPrice and cost calculation to support character-based pricing |
openai.mdx |
Updated documentation with TTS usage examples and cost information |
Comments suppressed due to low confidence (3)
apps/opik-backend/src/main/java/com/comet/opik/domain/cost/SpanCostCalculator.java:96
- [nitpick] The method name 'speechTtsCost' is redundant since TTS already stands for Text-to-Speech. Consider renaming to 'speechCost' or 'textToSpeechCost'.
public static BigDecimal speechTtsCost(ModelPrice modelPrice, Map<String, Integer> usage) {
apps/opik-backend/src/main/java/com/comet/opik/domain/cost/CostService.java:107
- Missing braces around the else clause. According to Java coding standards, all control flow statements should use braces for clarity and maintainability.
} else
apps/opik-backend/src/main/java/com/comet/opik/domain/cost/CostService.java:112
- Missing braces around the else clause. All control flow statements should use braces for consistency and maintainability.
} else
def __init__(self, audio: bytes): | ||
self._audio = audio | ||
|
||
def model_dump(self, mode: str = "json") -> dict: # noqa: D401 – follow OpenAI style |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment uses an en dash (–) instead of a hyphen (-). Use a standard hyphen for consistency.
def model_dump(self, mode: str = "json") -> dict: # noqa: D401 – follow OpenAI style | |
def model_dump(self, mode: str = "json") -> dict: # noqa: D401 - follow OpenAI style |
Copilot uses AI. Check for mistakes.
if error_info is None and buffer.should_attach(): | ||
path: Optional[Path] = buffer.flush_to_tempfile(".mp3") | ||
if path is not None: | ||
client = opik_client_module.get_client_cached() | ||
attach_obj = attachment_module.Attachment(data=str(path)) | ||
client.update_span( | ||
id=generators_span_to_end.id, | ||
trace_id=generators_span_to_end.trace_id, | ||
parent_span_id=generators_span_to_end.parent_span_id, | ||
project_name=generators_span_to_end.project_name or "", | ||
attachments=[attach_obj], | ||
) | ||
try: | ||
path.unlink() | ||
except Exception: | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The attachment logic is duplicated between sync and async handlers. Consider extracting this into a separate method to reduce code duplication.
if error_info is None and buffer.should_attach(): | |
path: Optional[Path] = buffer.flush_to_tempfile(".mp3") | |
if path is not None: | |
client = opik_client_module.get_client_cached() | |
attach_obj = attachment_module.Attachment(data=str(path)) | |
client.update_span( | |
id=generators_span_to_end.id, | |
trace_id=generators_span_to_end.trace_id, | |
parent_span_id=generators_span_to_end.parent_span_id, | |
project_name=generators_span_to_end.project_name or "", | |
attachments=[attach_obj], | |
) | |
try: | |
path.unlink() | |
except Exception: | |
pass | |
if error_info is None: | |
self._handle_attachment(buffer, generators_span_to_end) |
Copilot uses AI. Check for mistakes.
if error_info is None and buffer.should_attach(): | ||
path: Optional[Path] = buffer.flush_to_tempfile(".mp3") | ||
if path is not None: | ||
client = opik_client_module.get_client_cached() | ||
attach_obj = attachment_module.Attachment(data=str(path)) | ||
client.update_span( | ||
id=generators_span_to_end.id, | ||
trace_id=generators_span_to_end.trace_id, | ||
parent_span_id=generators_span_to_end.parent_span_id, | ||
project_name=generators_span_to_end.project_name or "", | ||
attachments=[attach_obj], | ||
) | ||
try: | ||
path.unlink() | ||
except Exception: | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This attachment logic is identical to the sync version. The duplication could be eliminated by extracting the attachment functionality into a helper method.
if error_info is None and buffer.should_attach(): | |
path: Optional[Path] = buffer.flush_to_tempfile(".mp3") | |
if path is not None: | |
client = opik_client_module.get_client_cached() | |
attach_obj = attachment_module.Attachment(data=str(path)) | |
client.update_span( | |
id=generators_span_to_end.id, | |
trace_id=generators_span_to_end.trace_id, | |
parent_span_id=generators_span_to_end.parent_span_id, | |
project_name=generators_span_to_end.project_name or "", | |
attachments=[attach_obj], | |
) | |
try: | |
path.unlink() | |
except Exception: | |
pass | |
if error_info is None: | |
self._attach_buffer_to_span(buffer, generators_span_to_end) |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @vladimirrotariu,
regarding changes in apps/opik-frontend/, could you please avoid reformatting large parts of files (like changing the order of imports) unless you’ve actually modified that code? It makes reviewing PRs a bit more complicated
Hi @andrescrz, is there anything I should do right now? |
@vladimirrotariu see comments raised by team as well as open issues and merge conflicts. Although we appreciate the contribution given the volume of changes and review required the team may close this PR as there are other submissions and/or close the bounty entirely. |
@vincentkoc but I reverted the changes to the non-formatted ones more than a week ago, now the changes strictly adheres to the implementation logic. Please explain further. |
Thanks for your contributions! As per the bounty guidelines, please note that we process one bounty PR at a time, so we'll focus first on PR #2850 since it appears closer to being ready. In the meantime, for this PR, could you please:
Once the first PR is merged, we'll review your other PRs one by one in accordance with the bounty process. Thanks for your patience and for helping improve Opik! |
Details
Introduces end-to-end support for OpenAI Text-to-Speech (TTS):
Python SDK
openai_speech_decorator.py
+speech_stream_buffer.py
+speech_stream_events_aggregator.py
– traceaudio.speech
requests (sync & streaming) and aggregate stream chunks.opik_tracker.py
&stream_patchers.py
patched to hook the new decorator and process chunks.openai_speech_usage.py
model records per-character usage for cost analytics.Java backend
ModelPrice.java
,SpanCostCalculator.java
,CostService.java
extended withinputCharacterPrice
and newspeechTtsCost
calculator to bill TTS spans by character.Front-end
TracesSpansTab.tsx
adds a Chars column, surfacingcharacter_count
and tying it into existing cost display.Documentation
openai.mdx
updated with TTS tracing instructions, environment setup, and pricing explanation.Issues
Resolves #2202
/claim #2202
Testing
test_openai_speech.py
verify sync & streaming paths.CostServiceTest.java
check character-based cost computation.Documentation
Primary guide updated at
apps/opik-documentation/documentation/fern/docs/tracing/integrations/openai.mdx
, including examples and cost tables for TTS.