Repository navigation
Implement OpenAITTSHandler.synthesize() for text-to-speech synthesis - #619
adarsh02125 with Copilot wants to merge 2 commits into
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughThe PR adds an OpenAI text-to-speech handler implementation with the OpenAITTSHandler class supporting six voices and audio formats, speed control, quality selection, comprehensive input validation, error handling, and logging, alongside type definitions, package exports, documentation, and unit tests. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client/App
participant Handler as OpenAITTSHandler
participant Validation as Input Validator
participant OpenAI as OpenAI API
participant Converter as Buffer Converter
App->>Handler: synthesize(options)
Handler->>Validation: validateInputs(text, voice, format, speed)
alt Validation Failed
Validation-->>Handler: throw ValidationError
Handler-->>App: Promise<Error>
else Validation Passed
Validation-->>Handler: ✓ valid
Handler->>Handler: selectModel(quality)
Note over Handler: tts-1 or tts-1-hd
Handler->>OpenAI: audio.speech.create({model, voice, input, response_format, speed})
OpenAI-->>Handler: ArrayBuffer
Handler->>Converter: convert(ArrayBuffer)
Converter-->>Handler: Buffer
Handler->>Handler: estimateDuration(text, speed)
Note over Handler: Calculate latency & duration
Handler-->>App: TTSResult {buffer, format, size, duration, voice, sampleRate}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements text-to-speech synthesis functionality using OpenAI's TTS API. It introduces a new OpenAITTSHandler class that provides a type-safe interface for generating speech from text with support for multiple voices, formats, and quality levels.
- Adds comprehensive TTS type system with interfaces for handlers, options, and results
- Implements OpenAI TTS handler with full validation, error handling, and metadata tracking
- Provides 14 unit tests covering edge cases and validation scenarios
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/types/tts.ts | Defines TTS type system including ITTSHandler interface, audio format types, and synthesis options/results |
| src/lib/types/index.ts | Exports new TTS types from the main types module |
| src/lib/adapters/tts/openaiTTSHandler.ts | Implements OpenAI TTS handler with model selection, voice support, format conversion, speed control, and comprehensive validation |
| src/lib/adapters/tts/index.ts | Exports the OpenAI TTS handler for public API access |
| src/lib/adapters/tts/README.md | Provides usage documentation with examples for all features and options |
| test/unit/adapters/openaiTTSHandler.test.ts | Adds 14 unit tests covering constructor, voice/format support, and input validation including edge cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0acb251 to
a6abf33
Compare
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
@copilot workflows have failed, verify the details and implement the fix for the same |
Fixed the workflow failures. The issue was a missing Root cause: The Fix applied (commit
Verification:
The CI workflows should now pass. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
5cc00a6 to
8b6a368
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
test/unit/adapters/openaiTTSHandler.test.ts (2)
12-16: Clean up environment variable after test to prevent pollution.Setting
process.env.OPENAI_API_KEYpersists across tests and could affect other test files. Consider usingbeforeEach/afterEachor Vitest's environment mocking.+ describe("constructor", () => { + const originalEnv = process.env.OPENAI_API_KEY; + + afterEach(() => { + process.env.OPENAI_API_KEY = originalEnv; + }); + it("should create handler with provided API key", () => { const handler = new OpenAITTSHandler("custom-key"); expect(handler).toBeInstanceOf(OpenAITTSHandler); }); it("should create handler with env var API key", () => { process.env.OPENAI_API_KEY = "env-key"; const handler = new OpenAITTSHandler(); expect(handler).toBeInstanceOf(OpenAITTSHandler); }); });
77-90: Consider mocking the OpenAI client for cleaner tests.The current approach works but relies on distinguishing validation errors from API errors by message content. Mocking the OpenAI client would make these tests more reliable and faster.
Example with Vitest mocking:
import { vi } from "vitest"; vi.mock("openai", () => ({ default: vi.fn().mockImplementation(() => ({ audio: { speech: { create: vi.fn().mockResolvedValue({ arrayBuffer: () => Promise.resolve(new ArrayBuffer(100)), }), }, }, })), }));src/lib/types/tts.ts (2)
34-68: Consider provider-specific typing or generics forvoice/modeland handler methods
TTSSynthesisOptions.voice/modelandITTSHandler.getSupportedVoices()are typed as plainstring, even though you haveOpenAITTSVoiceand provider-specific model sets.If you later want stronger compile-time guarantees per provider, consider either:
- defining provider-specific option types (e.g.,
OpenAITTSSynthesisOptionswithvoice?: OpenAITTSVoice), or- making
ITTSHandlergeneric, e.g.ITTSHandler<TVoice extends string = string>and threading that throughTTSSynthesisOptions.Not urgent for this PR, but could improve ergonomics and type safety as more providers are added.
Based on learnings, this aligns with the goal of strict, domain-organized TypeScript types.
Also applies to: 128-147
73-123: Node-specificBufferin publicTTSResultmay limit non-Node runtimesTyping
TTSResult.bufferasBufferis fine if this SDK is explicitly Node-only, but it constrains you if you later want browser/edge support.If multi-runtime support is a goal, consider:
- using a more neutral type (e.g.,
Uint8Array), or- introducing an alias like
export type TTSAudioBuffer = Buffer | Uint8Array;and using that inTTSResult.That way adapters can still return
Bufferin Node while keeping the public type surface more portable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
package.json(1 hunks)src/lib/adapters/tts/README.md(1 hunks)src/lib/adapters/tts/index.ts(1 hunks)src/lib/adapters/tts/openaiTTSHandler.ts(1 hunks)src/lib/types/index.ts(1 hunks)src/lib/types/tts.ts(1 hunks)test/unit/adapters/openaiTTSHandler.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: vigneshJuspay
Repo: juspay/neurolink PR: 237
File: memory-bank/tts-provider-implementation-plan.md:92-106
Timestamp: 2025-11-17T13:53:20.209Z
Learning: In PR 237's TTS modality implementation approach, TTS functionality uses GOOGLE_AI_API_KEY (not GOOGLE_TTS_API_KEY) when using the google-ai provider. TTS is implemented as an output modality that leverages the existing google-ai provider authentication.
📚 Learning: 2025-11-17T13:53:20.209Z
Learnt from: vigneshJuspay
Repo: juspay/neurolink PR: 237
File: memory-bank/tts-provider-implementation-plan.md:92-106
Timestamp: 2025-11-17T13:53:20.209Z
Learning: In PR 237's TTS modality implementation approach, TTS functionality uses GOOGLE_AI_API_KEY (not GOOGLE_TTS_API_KEY) when using the google-ai provider. TTS is implemented as an output modality that leverages the existing google-ai provider authentication.
Applied to files:
src/lib/adapters/tts/README.mdsrc/lib/types/tts.tssrc/lib/adapters/tts/openaiTTSHandler.ts
📚 Learning: 2025-12-01T08:39:22.794Z
Learnt from: CR
Repo: juspay/neurolink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T08:39:22.794Z
Learning: Maintain strict TypeScript type safety across all modules and organize types by domain (providers, generation, streaming, MCP, etc.) to avoid circular dependencies
Applied to files:
src/lib/types/tts.ts
📚 Learning: 2025-09-17T17:55:15.261Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 173
File: src/lib/index.ts:16-16
Timestamp: 2025-09-17T17:55:15.261Z
Learning: In src/lib/types/providers.ts, ProviderConfig was renamed to AIModelProviderConfig to deduplicate type names, as there was an existing ProviderConfig type that better suited the "ProviderConfig" name. This was an intentional breaking change for better type organization.
Applied to files:
src/lib/types/tts.tssrc/lib/adapters/tts/openaiTTSHandler.tssrc/lib/types/index.ts
📚 Learning: 2025-11-11T14:02:21.868Z
Learnt from: vigneshJuspay
Repo: juspay/neurolink PR: 214
File: src/lib/index.ts:205-233
Timestamp: 2025-11-11T14:02:21.868Z
Learning: In the NeuroLink TTS SDK (src/lib/tts/), use GOOGLE_TTS_API_KEY environment variable specifically for Google Cloud Text-to-Speech access. GOOGLE_AI_API_KEY does not provide TTS access and should not be used for TTS functionality. The keys are intentionally kept separate for better access control and separation of concerns.
Applied to files:
src/lib/types/tts.tssrc/lib/adapters/tts/openaiTTSHandler.ts
📚 Learning: 2025-11-04T22:14:18.719Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 0
File: :0-0
Timestamp: 2025-11-04T22:14:18.719Z
Learning: In the juspay/neurolink repository, all new type definitions must be placed in src/lib/types/. New type definitions outside this directory should be flagged and blocked in code reviews.
Applied to files:
src/lib/types/index.ts
📚 Learning: 2025-12-01T08:39:22.794Z
Learnt from: CR
Repo: juspay/neurolink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T08:39:22.794Z
Learning: Use MessageBuilder (src/lib/utils/messageBuilder.ts) as the central component for constructing all messages with multimodal support
Applied to files:
src/lib/types/index.ts
📚 Learning: 2025-09-02T13:50:42.770Z
Learnt from: YasmeenOgo
Repo: juspay/neurolink PR: 145
File: src/lib/core/types.ts:0-0
Timestamp: 2025-09-02T13:50:42.770Z
Learning: The APIVersions enum in src/lib/core/types.ts now contains comprehensive API version constants for all major AI providers: Azure OpenAI (latest, stable, legacy), OpenAI (current, beta), Google AI (current, beta), and Anthropic (current). This centralization helps avoid API version drift across the codebase.
Applied to files:
package.json
🧬 Code graph analysis (2)
src/lib/adapters/tts/openaiTTSHandler.ts (1)
src/lib/types/tts.ts (5)
ITTSHandler(128-147)OpenAITTSVoice(23-29)TTSAudioFormat(13-13)TTSSynthesisOptions(34-68)TTSResult(73-123)
test/unit/adapters/openaiTTSHandler.test.ts (2)
src/lib/adapters/tts/index.ts (1)
OpenAITTSHandler(9-9)src/lib/adapters/tts/openaiTTSHandler.ts (1)
OpenAITTSHandler(25-201)
🔇 Additional comments (11)
src/lib/types/index.ts (1)
216-218: LGTM!The TTS type re-exports follow the established pattern and are properly placed in the centralized types directory. Based on learnings, this aligns with the project convention that all new type definitions must be placed in
src/lib/types/.src/lib/adapters/tts/index.ts (1)
1-9: LGTM!Clean barrel file pattern with proper JSDoc documentation. The structure allows for easy addition of future TTS handlers (e.g., GoogleTTSHandler, AzureTTSHandler).
test/unit/adapters/openaiTTSHandler.test.ts (2)
19-45: Good coverage for capability getters.The tests properly verify all expected voices and formats are returned.
47-68: Thorough text validation testing.Good coverage of empty text, whitespace-only text, and text exceeding the 4096 character limit.
src/lib/adapters/tts/openaiTTSHandler.ts (6)
1-21: Good module documentation and imports.Clear documentation of supported models, voices, and formats. Proper type imports from the types module.
44-52: Constructor follows standard pattern.Lazy API key validation (deferred to API call time) is acceptable. The OpenAI SDK will throw a clear error if the key is missing when
synthesize()is called.
99-102: Duration estimation is a rough heuristic — consider documenting limitations.The 150 words/minute estimate is reasonable for typical speech, but actual duration varies by voice, language, and content. The
metadata.durationis typed as optional (duration?: number), which correctly signals it's an estimate.
125-140: Solid error handling with context preservation.Logging before re-throwing aids debugging. The error wrapping provides useful context about the operation that failed.
151-184: Comprehensive input validation.All inputs are validated with clear error messages. The validation order (text → voice → format → speed) ensures users get the most relevant error first.
186-200: LGTM!Defensive copying via spread operator prevents external mutation of internal state.
src/lib/types/tts.ts (1)
13-29: Union types for formats/quality/voices look solid and aligned with requirementsThe string-literal unions for
TTSAudioFormat,TTSQuality, andOpenAITTSVoicecleanly model the supported capabilities and give good autocomplete/validation. This matches the stated OpenAI TTS formats and voices.
|
@copilot review the comments that coderabbit has given, if valid implement the changes else skip |
Reviewed and implemented CodeRabbit's feedback (commit ✅ Fixed Issues:
Validation:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
b1c4551 to
6c88275
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
@copilot read through the complete implementation and fix the same. build is failing, seems like it was a bad rebase |
Fixed all implementation issues from the bad rebase (commit Issues resolved:
Type consistency:
Build should now pass successfully. |
ac9d964 to
ea93477
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
ea93477 to
398a721
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
398a721 to
3cd8e26
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lib/adapters/tts/README.md (1)
7-9: Import path issue resolved.The import path
@juspay/neurolink/adapters/ttsis now valid sincepackage.jsonexports have been updated to include./adapters/tts. This addresses the previous review feedback.
🧹 Nitpick comments (6)
src/lib/types/ttsTypes.ts (1)
22-25: Consider makingtextrequired inTTSOptionstype.The
textfield is optional (text?: string), but thesynthesizemethod throws if text is missing. Making it required at the type level would provide compile-time safety rather than relying solely on runtime validation.However, this may be intentional if
TTSOptionsis used in other contexts where text is not always required (e.g., configuration objects).export type TTSOptions = { /** Text to synthesize */ - text?: string; + text: string; /** Enable TTS output */test/unit/adapters/openaiTTSHandler.test.ts (1)
12-16: Clean up environment variable after test.Setting
process.env.OPENAI_API_KEYwithout cleanup can leak state to subsequent tests, potentially causing flaky behavior.+import { beforeEach, afterEach, describe, it, expect } from "vitest"; + +// At the top of the describe block or in a beforeEach/afterEach: +let originalApiKey: string | undefined; + +beforeEach(() => { + originalApiKey = process.env.OPENAI_API_KEY; +}); + +afterEach(() => { + if (originalApiKey === undefined) { + delete process.env.OPENAI_API_KEY; + } else { + process.env.OPENAI_API_KEY = originalApiKey; + } +});src/lib/adapters/tts/openaiTTSHandler.ts (4)
22-25: Consider reusing types from ttsTypes.ts to prevent drift.
OpenAIAudioFormatis defined locally but partially overlaps withAudioFormatfromttsTypes.ts. IfAudioFormatchanges (e.g., adding"ogg"variants), this local type won't be updated, potentially causing inconsistencies.You could derive this type or add a comment noting the intentional subset:
/** * Audio formats supported by OpenAI's TTS API * Note: Intentionally excludes "ogg" which is in AudioFormat but not supported by OpenAI */ type OpenAIAudioFormat = Exclude<AudioFormat, "ogg">;
72-79: Potential issue with "ogg" format handling.The
formatvalue comes fromTTSOptions.formatwhich can be"ogg"(part ofAudioFormat), but"ogg"is not insupportedFormats. The cast toOpenAIAudioFormaton line 74 occurs before validation on line 79, so an unsupported format will be caught, but the type cast is technically unsafe.Consider validating format before casting or using a type guard:
-const format = (options.format || "mp3") as OpenAIAudioFormat; +const format = options.format || "mp3"; + +// Validate inputs (including format check) +this.validateInputs(options.text, voice, format, speed); + +// Safe cast after validation +const validatedFormat = format as OpenAIAudioFormat;
67-70: Consider using ErrorFactory for typed errors instead of plain Error objects.Per coding guidelines,
src/lib/**/*.tsfiles should useErrorFactoryfor creating typed errors. This file uses plainErrorobjects throughout (lines 69, ~104, ~136, ~140, ~145, ~152, ~159). ImportErrorFactoryfromsrc/lib/utils/errorHandling.tsand replace all instances with appropriate factory methods.
94-100: Add timeout protection to the OpenAI TTS API call.The OpenAI API call at lines 94-100 lacks timeout protection. Per coding guidelines, async operations in
src/lib/**/*.tsshould use thewithTimeoututility to prevent hanging requests. The pattern from the codebase shows wrapping with an Error object created by ErrorFactory.-import OpenAI from "openai"; -import { logger } from "../../utils/logger.js"; +import OpenAI from "openai"; +import { logger } from "../../utils/logger.js"; +import { withTimeout } from "../../utils/errorHandling.js"; +import { ErrorFactory } from "../../utils/errorHandling.js"; // Call OpenAI TTS API - const response = await this.client.audio.speech.create({ + const response = await withTimeout( + this.client.audio.speech.create({ model, voice, input: options.text, response_format: format, speed, - }); + }), + 30000, + ErrorFactory.toolTimeout("openai-tts", 30000) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
package.json(2 hunks)src/lib/adapters/tts/README.md(1 hunks)src/lib/adapters/tts/index.ts(1 hunks)src/lib/adapters/tts/openaiTTSHandler.ts(1 hunks)src/lib/types/ttsTypes.ts(4 hunks)test/unit/adapters/openaiTTSHandler.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/lib/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/lib/**/*.ts: Use ErrorFactory for creating typed errors in error handling
Use withTimeout utility to wrap async operations for timeout protection
Files:
src/lib/types/ttsTypes.tssrc/lib/adapters/tts/index.tssrc/lib/adapters/tts/openaiTTSHandler.ts
src/lib/types/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Add model definitions to appropriate model enum when adding a new provider
Files:
src/lib/types/ttsTypes.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.ts: Mock external API calls for unit tests and use real API calls sparingly in integration tests
Validate multimodal content handling in tests when adding new file types
Files:
test/unit/adapters/openaiTTSHandler.test.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: vigneshJuspay
Repo: juspay/neurolink PR: 237
File: memory-bank/tts-provider-implementation-plan.md:92-106
Timestamp: 2025-11-17T13:53:20.209Z
Learning: In PR 237's TTS modality implementation approach, TTS functionality uses GOOGLE_AI_API_KEY (not GOOGLE_TTS_API_KEY) when using the google-ai provider. TTS is implemented as an output modality that leverages the existing google-ai provider authentication.
Learnt from: swaroopvarma1
Repo: juspay/neurolink PR: 141
File: docs/REAL-TIME-SPEECH-AGENTS.md:124-149
Timestamp: 2025-09-01T14:12:14.227Z
Learning: In the NeuroLink Speech-to-Speech agent system, the team prefers simple void-returning APIs (sendAudioFrame, sendText, flush) over Promise-based backpressure mechanisms, prioritizing ease of use and implementation simplicity for real-time speech processing.
📚 Learning: 2025-09-02T13:50:42.770Z
Learnt from: YasmeenOgo
Repo: juspay/neurolink PR: 145
File: src/lib/core/types.ts:0-0
Timestamp: 2025-09-02T13:50:42.770Z
Learning: The APIVersions enum in src/lib/core/types.ts now contains comprehensive API version constants for all major AI providers: Azure OpenAI (latest, stable, legacy), OpenAI (current, beta), Google AI (current, beta), and Anthropic (current). This centralization helps avoid API version drift across the codebase.
Applied to files:
src/lib/types/ttsTypes.tspackage.json
📚 Learning: 2025-09-17T17:55:15.261Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 173
File: src/lib/index.ts:16-16
Timestamp: 2025-09-17T17:55:15.261Z
Learning: In src/lib/types/providers.ts, ProviderConfig was renamed to AIModelProviderConfig to deduplicate type names, as there was an existing ProviderConfig type that better suited the "ProviderConfig" name. This was an intentional breaking change for better type organization.
Applied to files:
src/lib/types/ttsTypes.ts
📚 Learning: 2025-09-01T22:58:39.149Z
Learnt from: sudharsan-juspay
Repo: juspay/neurolink PR: 140
File: src/lib/core/types.ts:198-203
Timestamp: 2025-09-01T22:58:39.149Z
Learning: In src/lib/core/types.ts, StreamOptions (imported from streamTypes.js) and StreamingOptions are intentionally different types with different use cases. StreamingOptions is for unified AI requests with multiple provider configurations, while StreamOptions is for individual streaming operations.
Applied to files:
src/lib/types/ttsTypes.ts
📚 Learning: 2025-12-06T11:08:18.370Z
Learnt from: CR
Repo: juspay/neurolink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T11:08:18.370Z
Learning: Applies to src/lib/types/index.ts : Add new provider names to the AIProviderName enum
Applied to files:
src/lib/types/ttsTypes.ts
📚 Learning: 2025-12-06T11:08:18.370Z
Learnt from: CR
Repo: juspay/neurolink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T11:08:18.370Z
Learning: Applies to test/**/*.test.ts : Validate multimodal content handling in tests when adding new file types
Applied to files:
src/lib/types/ttsTypes.tstest/unit/adapters/openaiTTSHandler.test.ts
📚 Learning: 2025-11-17T13:53:20.209Z
Learnt from: vigneshJuspay
Repo: juspay/neurolink PR: 237
File: memory-bank/tts-provider-implementation-plan.md:92-106
Timestamp: 2025-11-17T13:53:20.209Z
Learning: In PR 237's TTS modality implementation approach, TTS functionality uses GOOGLE_AI_API_KEY (not GOOGLE_TTS_API_KEY) when using the google-ai provider. TTS is implemented as an output modality that leverages the existing google-ai provider authentication.
Applied to files:
src/lib/types/ttsTypes.tssrc/lib/adapters/tts/openaiTTSHandler.ts
📚 Learning: 2025-12-06T11:08:18.370Z
Learnt from: CR
Repo: juspay/neurolink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T11:08:18.370Z
Learning: Applies to test/**/*.test.ts : Mock external API calls for unit tests and use real API calls sparingly in integration tests
Applied to files:
test/unit/adapters/openaiTTSHandler.test.ts
📚 Learning: 2025-11-04T22:14:18.719Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 0
File: :0-0
Timestamp: 2025-11-04T22:14:18.719Z
Learning: In the juspay/neurolink repository, all new type definitions must be placed in src/lib/types/. New type definitions outside this directory should be flagged and blocked in code reviews.
Applied to files:
src/lib/adapters/tts/README.md
📚 Learning: 2025-11-05T20:31:04.103Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 227
File: src/lib/utils/redis.ts:15-15
Timestamp: 2025-11-05T20:31:04.103Z
Learning: In the juspay/neurolink repository, type centralization rules (requiring types in src/lib/types/) do not apply to private, non-exported utility types used within a single file. Simple readability helpers like `type RedisClient = ReturnType<typeof createClient>` should remain in their implementation file when they are not exported and only used locally. Only exported types shared across modules, business/domain types, and public API types require centralization.
Applied to files:
src/lib/adapters/tts/README.md
📚 Learning: 2025-11-04T22:14:18.719Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 0
File: :0-0
Timestamp: 2025-11-04T22:14:18.719Z
Learning: In the juspay/neurolink repository, do not flag existing type or interface definitions located outside src/lib/types/ - these are part of a phased migration plan and will be addressed in upcoming PRs. Only enforce type centralization rules on new code going forward.
Applied to files:
src/lib/adapters/tts/README.md
📚 Learning: 2025-11-04T22:14:18.719Z
Learnt from: RajuSudhar
Repo: juspay/neurolink PR: 0
File: :0-0
Timestamp: 2025-11-04T22:14:18.719Z
Learning: In the juspay/neurolink repository, new type definitions should use the `type` keyword instead of `interface`, unless there is a valid and justified exception. Flag new interface declarations in code reviews.
Applied to files:
src/lib/adapters/tts/README.md
📚 Learning: 2025-11-11T14:02:21.868Z
Learnt from: vigneshJuspay
Repo: juspay/neurolink PR: 214
File: src/lib/index.ts:205-233
Timestamp: 2025-11-11T14:02:21.868Z
Learning: In the NeuroLink TTS SDK (src/lib/tts/), use GOOGLE_TTS_API_KEY environment variable specifically for Google Cloud Text-to-Speech access. GOOGLE_AI_API_KEY does not provide TTS access and should not be used for TTS functionality. The keys are intentionally kept separate for better access control and separation of concerns.
Applied to files:
src/lib/adapters/tts/README.md
🧬 Code graph analysis (2)
test/unit/adapters/openaiTTSHandler.test.ts (1)
src/lib/adapters/tts/openaiTTSHandler.ts (1)
OpenAITTSHandler(30-206)
src/lib/adapters/tts/openaiTTSHandler.ts (1)
src/lib/types/ttsTypes.ts (5)
ITTSHandler(164-183)OpenAITTSVoice(153-159)TTSOptions(22-39)TTSResult(44-57)AudioFormat(12-12)
🔇 Additional comments (13)
src/lib/adapters/tts/index.ts (1)
1-9: LGTM!Clean barrel export file with proper ESM
.jsextension and module documentation.package.json (2)
149-154: LGTM!The new
./adapters/ttsexport is correctly configured with types, import, and default paths matching the existing export patterns. This resolves the previously flagged documentation import path issue.
197-197: OpenAI dependency version looks good.The
openai@^6.10.0version addresses the previous review feedback about using the latest v6.x series.src/lib/adapters/tts/README.md (1)
1-186: Comprehensive documentation.The README thoroughly covers all handler capabilities with clear examples for voices, formats, quality levels, speed control, error handling, and result structure. Well organized and matches the implementation.
src/lib/types/ttsTypes.ts (3)
12-12: LGTM!The
AudioFormatunion correctly extends to include all OpenAI-supported formats (aac,flac,pcm) while maintaining backward compatibility with existing formats.
149-159: LGTM!The
OpenAITTSVoicetype correctly uses thetypekeyword (per repo conventions) and enumerates all six OpenAI voices.
161-183: LGTM!The
ITTSHandlertype is well-defined with proper JSDoc comments and usestypeinstead ofinterfaceper repository conventions.test/unit/adapters/openaiTTSHandler.test.ts (1)
1-137: Good validation test coverage.The tests comprehensively cover constructor behavior, supported voices/formats, and input validation boundaries (empty text, whitespace, max length, invalid voice/format, speed limits). The validation error assertions are well-targeted.
src/lib/adapters/tts/openaiTTSHandler.ts (5)
1-21: LGTM!Clear module documentation and proper imports using ESM
.jsextensions.
109-112: Duration estimation is a rough approximation.The comment clearly documents this limitation. Consider adding this caveat to the
TTSResult.durationJSDoc or README to set proper expectations for consumers.
130-145: Good error handling with context logging.The error handling properly logs failure context (latency, model, voice, format) and wraps the error with enhanced information. This aids debugging production issues.
156-189: LGTM!Comprehensive input validation covering text presence, length limits, voice/format support, and speed boundaries. Error messages are clear and actionable.
191-205: LGTM!Helper methods correctly return copies of internal arrays to prevent mutation.
| it("should accept all supported voices", async () => { | ||
| const handler = new OpenAITTSHandler("test-key"); | ||
| const voices = ["alloy", "echo", "fable", "onyx", "nova", "shimmer"]; | ||
|
|
||
| for (const voice of voices) { | ||
| // This will fail with API error but validation should pass | ||
| try { | ||
| await handler.synthesize({ text: "test", voice }); | ||
| } catch (error) { | ||
| // Should not be a validation error | ||
| expect((error as Error).message).not.toContain("Unsupported voice"); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Mock the OpenAI API calls in unit tests.
This test makes real API calls to OpenAI, which violates the coding guideline to "mock external API calls for unit tests." The test pattern of catching errors and checking they're "not validation errors" is fragile and depends on network availability.
Consider mocking the OpenAI client:
import { vi } from "vitest";
// Mock the OpenAI module
vi.mock("openai", () => ({
default: vi.fn().mockImplementation(() => ({
audio: {
speech: {
create: vi.fn().mockResolvedValue({
arrayBuffer: () => Promise.resolve(new ArrayBuffer(100)),
}),
},
},
})),
}));As per coding guidelines, external API calls should be mocked for unit tests.
🤖 Prompt for AI Agents
In test/unit/adapters/openaiTTSHandler.test.ts around lines 77 to 90, the test
performs real OpenAI API calls; replace those with a mock of the OpenAI client
so unit tests don’t hit the network. Mock the module used by OpenAITTSHandler
(e.g., with vi.mock or your test runner’s mocking utility) to return a fake
client whose audio.speech.create resolves with a predictable ArrayBuffer (or
rejects for specific cases), then update the test to call handler.synthesize and
assert behavior against the mocked response (keeping the original check that
thrown errors are not validation errors).
| it("should accept speed of exactly 0.25", async () => { | ||
| const handler = new OpenAITTSHandler("test-key"); | ||
| try { | ||
| await handler.synthesize({ text: "test", speed: 0.25 }); | ||
| } catch (error) { | ||
| // Should not be a speed validation error | ||
| expect((error as Error).message).not.toContain("Speed must be between"); | ||
| } | ||
| }); | ||
|
|
||
| it("should accept speed of exactly 4.0", async () => { | ||
| const handler = new OpenAITTSHandler("test-key"); | ||
| try { | ||
| await handler.synthesize({ text: "test", speed: 4.0 }); | ||
| } catch (error) { | ||
| // Should not be a speed validation error | ||
| expect((error as Error).message).not.toContain("Speed must be between"); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Mock API calls for speed boundary tests.
These tests also make real API calls. With proper mocking, you can assert that synthesize completes successfully rather than relying on error message inspection.
With mocked API calls, these tests become deterministic:
it("should accept speed of exactly 0.25", async () => {
const handler = new OpenAITTSHandler("test-key");
const result = await handler.synthesize({ text: "test", speed: 0.25 });
expect(result.buffer).toBeDefined();
});As per coding guidelines, external API calls should be mocked for unit tests.
🤖 Prompt for AI Agents
In test/unit/adapters/openaiTTSHandler.test.ts around lines 117 to 135, the two
boundary tests currently rely on real API calls and catch errors to assert no
speed validation error; replace those with deterministic mocks of the OpenAI TTS
API (use Jest mocks or nock to intercept the handler's network call or the
underlying client method) so synthesize resolves successfully for speed=0.25 and
speed=4.0, then assert the returned result (e.g., result.buffer or equivalent)
is defined; ensure the mock returns the same shape the handler expects and
remove the try/catch error-inspection assertions.
|
@copilot review coderabbit's recent comments on improving test files and make necessary changes. Use the prompts that coderabbit has suggested in the comments and complete this task |
I don't see CodeRabbit's specific comments in the PR thread. Could you please share the specific suggestions or prompts that CodeRabbit provided? Alternatively, I can:
Which improvements would you like me to prioritize?
Enhanced the test file with comprehensive improvements (commit Major improvements:
Tests now properly validate the implementation without requiring OpenAI API access. |
…overage Co-authored-by: adarsh-bhaskar <207358788+adarsh-bhaskar@users.noreply.github.com>
|
Closing as part of project audit (2026-03-29). This PR does not align with current project architecture/conventions, or is broken/empty. See docs/project-audit-2026-03-29.md for details. |
Pull Request
Description
Implements core text-to-speech synthesis via OpenAI's TTS API. Adds
OpenAITTSHandlerclass withsynthesize()method supporting all OpenAI TTS models, voices, formats, and speed controls. The implementation extends and integrates with existing TTS type infrastructure from the release branch for consistency and maintainability.Type of Change
Related Issues
Implements TTS-008
Changes Made
Core Implementation
src/lib/types/ttsTypes.ts: Extended existing TTS type system with OpenAI-specific typesAudioFormatto include all 7 OpenAI formats:"mp3" | "wav" | "ogg" | "opus" | "aac" | "flac" | "pcm"VALID_AUDIO_FORMATSarray to include all supported formatsOpenAITTSVoicetype for OpenAI voices (alloy, echo, fable, onyx, nova, shimmer)ITTSHandlertype (not interface) for TTS provider implementations following codebase conventionstextfield toTTSOptionstype for synthesis operationsTTSOptionsandTTSResulttypes for consistencysrc/lib/adapters/tts/openaiTTSHandler.ts: Full OpenAI TTS handler (200+ lines)tts-1-hdfor HD quality,tts-1for standardalloy, supports all 6 OpenAI voicesclient.audio.speech.create(), converts ArrayBuffer to BufferTTSResultstructure matching existing type system:{ buffer, format, size, duration, voice, sampleRate }textfield with clear error messagesType System Integration
src/lib/types/tts.tsin favor of extending existingttsTypes.tsTTSResultstructure (not nested metadata) matching type definitionTTSSynthesisOptionsto standardTTSOptionstypeITTSHandlerfrom interface to type to follow codebase convention of using types over interfacestypes/tts.jsto correcttypes/ttsTypes.jsTTSOptionsandAudioFormatDependency & Package Configuration
package.json: Addedopenai@^6.10.0dependency (upgraded from v4 for latest security patches and features)"./adapters/tts"export mapping to enable proper module resolution for consumersTesting & Documentation
test/unit/adapters/openaiTTSHandler.test.ts: Comprehensive test suite with 39 unit testsvi.mock()to enable testing without API keysrc/lib/adapters/tts/README.md: Usage examples updated to use flatTTSResultstructureresult.size,result.duration,result.voiceExample Usage
AI Provider Impact
Component Impact
Testing
Test Environment
Performance Impact
Breaking Changes
None. Additive feature only. The OpenAI v6 upgrade maintains full backward compatibility with existing TTS API. Implementation uses existing TTS type system for consistency across the codebase.
Screenshots/Demo
N/A
Checklist
Additional Notes
Follows existing adapter patterns (similar to
providerImageAdapter.ts). Reuses logger utility. Type-safe with full TypeScript coverage. Ready for integration into provider-level TTS features (TTS-003, TTS-020).CI Fix: Added missing
openainpm package dependency that was causing TypeScript compilation errors in CI workflows. The TTS implementation requires the official OpenAI SDK.Code Review Updates: Addressed CodeRabbit feedback by:
"./adapters/tts"subpath to fix module resolution for consumers using the documented import pathMerge Conflict Resolution: Successfully merged with release branch and integrated with existing TTS infrastructure:
ttsTypes.tsinstead of maintaining duplicate type definitionsTTSResultstructure matching existing codebase patternssrc/lib/types/tts.tsfileType System Refinement: Followed codebase convention by:
ITTSHandlerfrom interface to type definition (export type ITTSHandler = { ... })textfield toTTSOptionstype for synthesis operationsImport Path Corrections: Fixed build failures by:
types/tts.jstotypes/ttsTypes.jsTTSSynthesisOptions→TTSOptions,TTSAudioFormat→AudioFormatoptions.textto ensure type safetyRebase Resolution: Fixed issues from bad rebase by:
AudioFormattype to include all OpenAI formats (aac, flac, pcm) in type definitionTTSResultstructure - changed from nested metadata object to flat structure matching type definition:{ buffer, format, size, duration, voice, sampleRate }VALID_AUDIO_FORMATSarray to include all 7 supported formatsresult.sizevsresult.metadata.size)Test Enhancement: Significantly improved test coverage and quality:
vi.mock()- tests no longer require API keyexpect.arrayContaining()and array immutability checksAll 39 TTS unit tests pass with OpenAI v6, confirming backward compatibility, proper integration with existing type system, and comprehensive validation of all functionality.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.