feat(tts): expose synthesis failure metadata and telemetry - #1204
Conversation
|
Someone is attempting to deploy a commit to the Sachin Sharma's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughTTS synthesis now returns structured attempt, success, latency, and error metadata. Both synthesis paths enforce timeouts, normalize errors, and record failure telemetry. ChangesTTS Error Observability
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant TTSProcessor
participant baseProvider
participant TelemetryHandler
participant TelemetryService
TTSProcessor->>baseProvider: Synthesize with timeout
TTSProcessor-->>baseProvider: Audio or normalized error
baseProvider->>TelemetryHandler: recordTTSFailure(provider, error, latency)
TelemetryHandler->>TelemetryService: Record failure counter and latency histogram
baseProvider-->>TTSProcessor: Return result with ttsMetadata
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/core/baseProvider.ts`:
- Around line 1349-1353: Wrap both asynchronous TTSProcessor.synthesize calls in
src/lib/core/baseProvider.ts at lines 1349-1353 and 1590-1594 with the existing
withTimeout utility, preserving their current arguments, result assignments, and
timeout configuration conventions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ece850ff-e9f7-4074-8bbc-b2460227adf9
📒 Files selected for processing (3)
src/lib/core/baseProvider.tssrc/lib/core/modules/TelemetryHandler.tssrc/lib/types/generate.ts
abb41ed to
31c4d7e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/core/baseProvider.ts (1)
1344-1374: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd focused tests for the new TTS outcome branches.
Cover Mode 1 and Mode 2 success, skipped synthesis, timeout,
NeuroLinkError, generic errors, metadata latency, and telemetry failure isolation. The linked objective explicitly requires unit tests for error scenarios.#!/bin/bash rg -n -C3 \ 'handleDirectTTSSynthesis|synthesizeAIResponseIfNeeded|getTTSErrorDetails|ttsMetadata|recordTTSFailure' \ --glob '*.{test,spec}.{ts,tsx}' .Also applies to: 1586-1655
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/core/baseProvider.ts` around lines 1344 - 1374, Add focused unit tests for the TTS outcome branches exercised by handleDirectTTSSynthesis and synthesizeAIResponseIfNeeded: Mode 1 and Mode 2 success, skipped synthesis, timeout, NeuroLinkError, generic errors, metadata latency, and telemetry failure isolation. Mock TTSProcessor.synthesize, withTimeoutFn, getTTSErrorDetails, and recordTTSFailure as needed, and assert audio plus ttsMetadata fields and failure behavior without changing production logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/core/baseProvider.ts`:
- Around line 1344-1374: Add focused unit tests for the TTS outcome branches
exercised by handleDirectTTSSynthesis and synthesizeAIResponseIfNeeded: Mode 1
and Mode 2 success, skipped synthesis, timeout, NeuroLinkError, generic errors,
metadata latency, and telemetry failure isolation. Mock TTSProcessor.synthesize,
withTimeoutFn, getTTSErrorDetails, and recordTTSFailure as needed, and assert
audio plus ttsMetadata fields and failure behavior without changing production
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a6343214-5673-4f8e-b977-b541f9257914
📒 Files selected for processing (3)
src/lib/core/baseProvider.tssrc/lib/core/modules/TelemetryHandler.tssrc/lib/types/generate.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/core/modules/TelemetryHandler.ts
- src/lib/types/generate.ts
|
@Harshita-Rupani29 can you resolve the conflicts? |
31c4d7e to
f446670
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/core/baseProvider.ts (1)
1430-1480: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMissing unit tests for the new TTS failure/skip scenarios.
Issue
#702explicitly calls for unit tests covering TTS error scenarios, but the PR summary states no automated tests are included. These two paths (handleDirectTTSSynthesis,synthesizeAIResponseIfNeeded) are new critical-path failure handling (timeout, provider error, skip) that would benefit from coverage — especially thegetTTSErrorDetailsbranching (timeout vsNeuroLinkErrorvs generic) and the skip-metadata case.Want me to draft unit tests for these scenarios (timeout, NeuroLinkError, generic error, and the missing-aiResponse/provider skip case)?
Also applies to: 1666-1736
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/core/baseProvider.ts` around lines 1430 - 1480, Add unit tests for handleDirectTTSSynthesis and synthesizeAIResponseIfNeeded covering TTS timeout errors, NeuroLinkError failures, generic errors, and the missing-aiResponse/provider skip path. Assert getTTSErrorDetails-derived metadata, failed/successful status, telemetry or error handling, and the expected skip metadata without invoking synthesis when prerequisites are absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/core/baseProvider.ts`:
- Around line 1738-1761: The getTTSErrorDetails method returns unsanitized error
messages in every branch. Wrap the messages from AsyncTimeoutError,
NeuroLinkError, and the fallback Error/String conversion with
redactUrlForError() before assigning them to the returned error metadata, while
preserving the existing codes and retriable values.
---
Nitpick comments:
In `@src/lib/core/baseProvider.ts`:
- Around line 1430-1480: Add unit tests for handleDirectTTSSynthesis and
synthesizeAIResponseIfNeeded covering TTS timeout errors, NeuroLinkError
failures, generic errors, and the missing-aiResponse/provider skip path. Assert
getTTSErrorDetails-derived metadata, failed/successful status, telemetry or
error handling, and the expected skip metadata without invoking synthesis when
prerequisites are absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2f298430-3ce6-4de4-826c-937c14271a51
📒 Files selected for processing (3)
src/lib/core/baseProvider.tssrc/lib/core/modules/TelemetryHandler.tssrc/lib/types/generate.ts
| private getTTSErrorDetails( | ||
| error: unknown, | ||
| ): NonNullable<TTSMetadata["error"]> { | ||
| if (error instanceof AsyncTimeoutError) { | ||
| return { | ||
| code: TTS_ERROR_CODES.SYNTHESIS_FAILED, | ||
| message: error.message, | ||
| retriable: true, | ||
| }; | ||
| } | ||
|
|
||
| if (error instanceof NeuroLinkError) { | ||
| return { | ||
| code: error.code, | ||
| message: error.message, | ||
| retriable: error.retriable, | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| code: TTS_ERROR_CODES.SYNTHESIS_FAILED, | ||
| message: error instanceof Error ? error.message : String(error), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
git ls-files | rg '(^|/)baseProvider\.ts$|utils/logSanitize|TTSError|TTS|redactUrlForError' || true
echo
echo "== inspect baseProvider relevant outline/imports =="
ast-grep outline src/lib/core/baseProvider.ts --view compact || true
echo
echo "== read relevant baseProvider sections =="
wc -l src/lib/core/baseProvider.ts
sed -n '1,120p' src/lib/core/baseProvider.ts
sed -n '1680,1790p' src/lib/core/baseProvider.ts
echo
echo "== search error propagation/usages =="
rg -n "getTTSErrorDetails|TTSError|error.message|NeuroLinkError|AsyncTimeoutError|generateWithTTS|EnhancedGenerateResult|TTS_ERROR_CODES" src/lib/core src/lib -g '*.ts' | head -200Repository: juspay/neurolink
Length of output: 30199
🌐 Web query:
CodeRabbit TTS provider error messages raw URLs redactUrlForError NeuroLinkError AsyncTimeoutError BaseProvider
💡 Result:
The terms and identifiers listed appear to relate to general software architecture for AI agents, TTS (Text-to-Speech) systems, and error handling patterns rather than being unique, proprietary error messages exclusive to the CodeRabbit service. Technical context for these terms: - TTS provider / BaseProvider: In architectures like CoderClaw (often discussed alongside AI agent tooling), BaseProvider is a common abstract class or interface used to standardize interaction with various external TTS engines (such as OpenAI, ElevenLabs, or Edge TTS) [1][2]. BaseProvider typically enforces a contract for methods like conversion, while TtsProviderAttempt tracks the outcome of these calls, including success or failure reasons [1]. - Error Handling (NeuroLinkError, AsyncTimeoutError, redactUrlForError): These are idiomatic patterns for handling asynchronous operations and logging in TypeScript/Node.js applications [1]. - AsyncTimeoutError is a standard exception for operations exceeding a specified time limit [1]. - redactUrlForError (or similar functions like redactSensitiveText) is a common security practice implemented in logging utilities to prevent sensitive information—such as API keys or private URLs—from being exposed in error logs [1][3]. - NeuroLinkError appears to be a specific custom error class associated with projects or documentation (such as the NeuroLink blog) that discuss the "Five-Stage Message Flow" for AI agents [4][5][6]. It is not a standard error message used by the CodeRabbit platform itself [7][8]. CodeRabbit, which provides AI-powered code reviews, maintains its own documentation on configuration and troubleshooting [7][8], but does not publicly list the specific identifiers you mentioned as native error codes for its platform. If you are encountering these errors, they likely originate from a custom integration, a specific SDK (like those used in the open-source projects referenced), or an underlying service provider's infrastructure [9][1].
Citations:
- 1: https://github.com/openclaw/openclaw/blob/main/packages/speech-core/src/tts.ts
- 2: https://docs.coderclaw.ai/tts/
- 3: Enhance embed URL handling and validation system ai-code-review-evaluation/discourse-coderabbit#4
- 4: https://blog.neurolink.ink/categories/documentation/
- 5: https://blog.neurolink.ink/categories/production/
- 6: https://blog.neurolink.ink/tags/frontend/
- 7: https://kb.coderabbit.ai/collections/6200836427-q%2Ba
- 8: https://docs.coderabbit.ai/support
- 9: https://www.twilio.com/docs/api/errors/64111
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== TTS processor outline =="
wc -l src/lib/utils/ttsProcessor.ts
ast-grep outline src/lib/utils/ttsProcessor.ts --view expanded || true
sed -n '1,280p' src/lib/utils/ttsProcessor.ts
echo
echo "== TTS handler outlines and imports =="
for f in src/lib/adapters/tts/googleTTSHandler.ts src/lib/voice/providers/*.ts src/lib/utils/ttsProcessor.ts src/lib/utils/logSanitize.ts; do
echo "---- $f ----"
wc -l "$f"
ast-grep outline "$f" --view expanded | sed -n '1,180p' || true
done
echo
echo "== all new lines mentioning URLs/requests/errors in TTS files =="
rg -n "fetch\\(|axios|response\\.url|url|URL|error\\.|message|NeuroLinkError|redactUrlForError|presign|token|auth|Authorization|error.message|throw new" src/lib/adapters/tts src/lib/voice/providers src/lib/utils/ttsProcessor.ts
echo
echo "== inspect TTS providers/handler error paths =="
sed -n '1,260p' src/lib/adapters/tts/googleTTSHandler.ts
sed -n '1,260p' src/lib/voice/providers/OpenAITTS.ts
sed -n '1,220p' src/lib/voice/providers/ElevenLabsTTS.ts
sed -n '1,220p' src/lib/voice/providers/AzureTTS.ts
sed -n '1,220p' src/lib/voice/providers/CartesiaTTS.tsRepository: juspay/neurolink
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect TTS processor implementation and tests for throw sources =="
rg -n "throw|rejection|response|fetch\\(|generate\\(|synthesiz|synthesize|NeuroLinkError|redactUrlForError" . -g '*test*' -g '*.test.ts' -g '*.spec.ts' -g '*example*' -g '*fixture*' 2>/dev/null | rg -i "tts|voice|audio|generate\\(" || true
echo
echo "== source-only throw/error construction in src lib tts/voice =="
python3 - <<'PY'
from pathlib import Path
for p in [Path('src/lib/utils/ttsProcessor.ts'), Path('src/lib/adapters/tts/googleTTSHandler.ts')]:
print(f'--- {p} ---')
txt=p.read_text()
for i,line in enumerate(txt.splitlines(),1):
if 'throw' in line or 'new NeuroLinkError' in line or 'rejection' in line or "reject" in line:
print(f'{i:4}: {line[:240]}')
PYRepository: juspay/neurolink
Length of output: 50372
Sanitize TTS error messages before returning them in ttsMetadata.error.message.
getTTSErrorDetails forwards raw error.message values into EnhancedGenerateResult.ttsMetadata.error, including AsyncTimeoutError, NeuroLinkError, and the fallback. Wrap these messages with redactUrlForError() before returning them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/core/baseProvider.ts` around lines 1738 - 1761, The
getTTSErrorDetails method returns unsanitized error messages in every branch.
Wrap the messages from AsyncTimeoutError, NeuroLinkError, and the fallback
Error/String conversion with redactUrlForError() before assigning them to the
returned error metadata, while preserving the existing codes and retriable
values.
Source: Learnings
f446670 to
ec2facb
Compare
|
🎉 This PR is included in version 10.8.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Pull Request
Description
What does this PR do?
This PR exposes structured metadata when Text-to-Speech synthesis succeeds, fails, or is skipped.
Callers can now distinguish between skipped and failed TTS synthesis, inspect structured error information, determine whether an error is retriable, and observe synthesis latency. TTS failures are also recorded through the existing telemetry system with provider and error-code labels.
Related Issues
Does this PR close any issues?
Fixes #702
Type of Change
Motivation and Context
TTS synthesis errors were previously logged but not exposed to callers. As a result:
This change preserves the existing graceful-degradation behavior while making the TTS outcome observable through the returned result and telemetry.
Changes Made
ttsMetadatafield toEnhancedGenerateResult.NeuroLinkErrorandTTSErrordetails.TTS_SYNTHESIS_FAILEDfallback code for unknown errors.TelemetryHandler.recordTTSFailure()to record provider-labelled failure counts and latency metrics.Breaking Changes
ttsMetadatais optional, so existing callers and result handling remain compatible.Testing
How has this been tested?
The following validation commands passed:
Test Coverage
No automated test file is included in this PR.
Manual Testing Steps
ttsMetadata.attemptedistrue.ttsMetadata.successisfalse.ttsMetadata.errorcontains a code, message, and retriable status when available.attempted: false.Code Quality
Documentation
/docsupdated (not required)Commit Message Format
type(scope): descriptionCommit message:
Dependencies
Performance Impact
The change only records timestamps around TTS synthesis and emits telemetry when synthesis fails.
Security Considerations
No credentials, request payloads, or sensitive provider data are added to telemetry labels. Error messages remain available in result metadata but are not used as metric labels.
Deployment Notes
Screenshots / Videos
Not applicable.
Reviewer Checklist
For reviewers:
Additional Notes
Pre-submission Checklist
CLAUDE.mdpnpm testpnpm buildpnpm run validate:alland all checks passSummary by CodeRabbit
New Features
ttsMetadataindicating whether TTS was attempted, whether it succeeded, structured error details, and synthesis latency.Bug Fixes
Monitoring