feat: voice and language support in cal.ai#23865
Conversation
WalkthroughAdds optional language and voice support across the Cal AI Phone stack. The AIPhoneService interface gains a new exported Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
packages/trpc/server/routers/viewer/aiVoiceAgent/update.schema.ts (1)
3-43: Avoid duplicating the language list; derive from a single source of truth.Define/export a shared constant of language codes (as const) in retellAI/types.ts and reuse it here to eliminate drift.
Apply this diff to consume a shared constant:
-import type { Language } from "@calcom/features/calAIPhone/providers/retellAI/types"; +import { LANGUAGES, type Language } from "@calcom/features/calAIPhone/providers/retellAI/types"; -const languageSchema = z.enum([ - "en-US", - "en-IN", - "en-GB", - "en-AU", - "en-NZ", - "de-DE", - "es-ES", - "es-419", - "hi-IN", - "fr-FR", - "fr-CA", - "ja-JP", - "pt-PT", - "pt-BR", - "zh-CN", - "ru-RU", - "it-IT", - "ko-KR", - "nl-NL", - "nl-BE", - "pl-PL", - "tr-TR", - "th-TH", - "vi-VN", - "ro-RO", - "bg-BG", - "ca-ES", - "da-DK", - "fi-FI", - "el-GR", - "hu-HU", - "id-ID", - "no-NO", - "sk-SK", - "sv-SE", - "multi", -]) as z.ZodType<Language>; +const languageSchema = z.enum(LANGUAGES) as z.ZodType<Language>;And in
retellAI/types.ts(outside this diff), expose the constant:// packages/features/calAIPhone/providers/retellAI/types.ts export const LANGUAGES = [ "en-US","en-IN","en-GB","en-AU","en-NZ","de-DE","es-ES","es-419","hi-IN","fr-FR","fr-CA", "ja-JP","pt-PT","pt-BR","zh-CN","ru-RU","it-IT","ko-KR","nl-NL","nl-BE","pl-PL","tr-TR", "th-TH","vi-VN","ro-RO","bg-BG","ca-ES","da-DK","fi-FI","el-GR","hu-HU","id-ID","no-NO", "sk-SK","sv-SE","multi", ] as const; export type Language = typeof LANGUAGES[number];packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (1)
314-315: Type consistency: consider provider-typed language (non-blocking).Interface uses
language?: stringwhile the Retell provider expectsLanguage. It’s fine at the boundary, but you could tighten generics later (e.g., addLanguageto the provider type map) to improve type safety.If you want to explore this refactor, I can sketch an incremental change to the type map that won’t ripple widely.
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx (4)
72-111: Localize option labels and avoid module-level t() blockers.Per guideline, user-facing text should use t(). Build options inside the component (e.g., useMemo) so labels are localized, and keep the code list centralized to avoid drift.
Apply this diff to remove the hard-coded, non-localized options:
-// Language options for the dropdown -const LANGUAGE_OPTIONS = [ - { value: "en-US", label: "English (US)" }, - { value: "en-IN", label: "English (India)" }, - { value: "en-GB", label: "English (UK)" }, - { value: "en-AU", label: "English (Australia)" }, - { value: "en-NZ", label: "English (New Zealand)" }, - { value: "de-DE", label: "German" }, - { value: "es-ES", label: "Spanish (Spain)" }, - { value: "es-419", label: "Spanish (Latin America)" }, - { value: "hi-IN", label: "Hindi" }, - { value: "fr-FR", label: "French (France)" }, - { value: "fr-CA", label: "French (Canada)" }, - { value: "ja-JP", label: "Japanese" }, - { value: "pt-PT", label: "Portuguese (Portugal)" }, - { value: "pt-BR", label: "Portuguese (Brazil)" }, - { value: "zh-CN", label: "Chinese (Simplified)" }, - { value: "ru-RU", label: "Russian" }, - { value: "it-IT", label: "Italian" }, - { value: "ko-KR", label: "Korean" }, - { value: "nl-NL", label: "Dutch (Netherlands)" }, - { value: "nl-BE", label: "Dutch (Belgium)" }, - { value: "pl-PL", label: "Polish" }, - { value: "tr-TR", label: "Turkish" }, - { value: "th-TH", label: "Thai" }, - { value: "vi-VN", label: "Vietnamese" }, - { value: "ro-RO", label: "Romanian" }, - { value: "bg-BG", label: "Bulgarian" }, - { value: "ca-ES", label: "Catalan" }, - { value: "da-DK", label: "Danish" }, - { value: "fi-FI", label: "Finnish" }, - { value: "el-GR", label: "Greek" }, - { value: "hu-HU", label: "Hungarian" }, - { value: "id-ID", label: "Indonesian" }, - { value: "no-NO", label: "Norwegian" }, - { value: "sk-SK", label: "Slovak" }, - { value: "sv-SE", label: "Swedish" }, - { value: "multi", label: "Multilingual" }, -];Add inside the component (outside this diff) after
const { t } = useLocale();:// Add useMemo import at top: import { useState, useRef, useEffect, useMemo } from "react"; const LANGUAGE_CODES = [ "en-US","en-IN","en-GB","en-AU","en-NZ","de-DE","es-ES","es-419","hi-IN","fr-FR","fr-CA", "ja-JP","pt-PT","pt-BR","zh-CN","ru-RU","it-IT","ko-KR","nl-NL","nl-BE","pl-PL","tr-TR", "th-TH","vi-VN","ro-RO","bg-BG","ca-ES","da-DK","fi-FI","el-GR","hu-HU","id-ID","no-NO", "sk-SK","sv-SE","multi", ] as const; const languageOptions = useMemo( () => LANGUAGE_CODES.map((code) => ({ value: code, // Add translation keys in en/common.json; lingo.dev will propagate label: t(`language_label_${code.replace('-', '_')}`), })), [t] );
116-117: Constrain to supported values instead of free-form string.Align the form schema with the backend enum to prevent invalid submissions and remove the later type cast.
- language: z.string().optional(), + language: z.enum([ + "en-US","en-IN","en-GB","en-AU","en-NZ","de-DE","es-ES","es-419","hi-IN","fr-FR","fr-CA", + "ja-JP","pt-PT","pt-BR","zh-CN","ru-RU","it-IT","ko-KR","nl-NL","nl-BE","pl-PL","tr-TR", + "th-TH","vi-VN","ro-RO","bg-BG","ca-ES","da-DK","fi-FI","el-GR","hu-HU","id-ID","no-NO", + "sk-SK","sv-SE","multi", + ]).optional(),If you adopt the
LANGUAGE_CODESsnippet from the previous comment, replace the array above withz.enum(LANGUAGE_CODES).
486-499: Make Select robust to empty values and use localized options.Use the memoized
languageOptions, handle null safely, and add a localized placeholder.- <Select - value={LANGUAGE_OPTIONS.find((option) => option.value === field.value)} - onChange={(option) => field.onChange(option?.value)} - options={LANGUAGE_OPTIONS} - isDisabled={readOnly} - className="mb-4" - /> + <Select + value={languageOptions.find((o) => o.value === field.value) ?? null} + onChange={(option) => field.onChange(option?.value ?? null)} + options={languageOptions} + placeholder={t("select_language")} + isDisabled={readOnly} + className="mb-4" + />
336-337: Drop the cast once the schema is an enum.With
z.enum(...)above,data.languageis already typed; no need toas Language.- language: data.language as Language, + language: data.language,packages/features/calAIPhone/providers/retellAI/services/AgentService.ts (1)
576-581: Add language to error-context logs.This helps debugging language updates.
updates: { generalPrompt: !!generalPrompt, beginMessage: !!beginMessage, generalTools: !!generalTools, voiceId: !!voiceId, + language: !!language, },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts(2 hunks)packages/features/calAIPhone/providers/retellAI/RetellAIService.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/AgentService.ts(4 hunks)packages/features/ee/workflows/components/AgentConfigurationSheet.tsx(7 hunks)packages/trpc/server/routers/viewer/aiVoiceAgent/update.handler.ts(1 hunks)packages/trpc/server/routers/viewer/aiVoiceAgent/update.schema.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/update.handler.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.tspackages/trpc/server/routers/viewer/aiVoiceAgent/update.schema.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/update.handler.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.tspackages/trpc/server/routers/viewer/aiVoiceAgent/update.schema.tspackages/features/ee/workflows/components/AgentConfigurationSheet.tsxpackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/update.handler.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.tspackages/trpc/server/routers/viewer/aiVoiceAgent/update.schema.tspackages/features/ee/workflows/components/AgentConfigurationSheet.tsxpackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/features/insights/components/booking/LeastCompletedBookings.tsx:31-31
Timestamp: 2025-08-21T05:55:35.187Z
Learning: Cal.com uses an automated i18n system with lingo.dev that automatically propagates new translation keys from en/common.json to all other locale files and creates PRs with proper translations. The system includes a check-missing-translations.ts script that adds English placeholders for missing keys, and a GitHub workflow that triggers lingo.dev automation to translate and create PRs.
📚 Learning: 2025-08-08T09:27:23.896Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.896Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/update.handler.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-08-17T22:00:16.329Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts:117-126
Timestamp: 2025-08-17T22:00:16.329Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts, the enabled input parameter in the update endpoint is intentionally not forwarded to aiService.updateAgentConfiguration() as the enabled/disabled agent functionality is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/update.handler.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-08-08T09:29:11.681Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-08-14T10:48:52.586Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/ai/_router.ts:46-84
Timestamp: 2025-08-14T10:48:52.586Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/ai/_router.ts, the voiceId input parameter in the create endpoint is intentionally not forwarded to aiService.createAgent() as voice customization is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/update.handler.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-08-08T10:26:13.362Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Applied to files:
packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, the variable insertion pattern in AgentConfigurationSheet.tsx follows the same implementation as existing workflow code. The current approach of building variable tokens directly from the input variable (with toUpperCase and space replacement) is the established pattern and doesn't need to be changed for locale-agnostic concerns, per maintainer Udit-takkar.
Applied to files:
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx
📚 Learning: 2025-08-27T12:15:43.830Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/testCall.handler.ts:41-44
Timestamp: 2025-08-27T12:15:43.830Z
Learning: In calcom/cal.com, the AgentService.getAgent() method in packages/features/calAIPhone/providers/retellAI/services/AgentService.ts does NOT include authorization checks - it only validates the agentId parameter and directly calls the repository without verifying user/team access. This contrasts with other methods like getAgentWithDetails() which properly use findByIdWithUserAccessAndDetails() for authorization. When reviewing updateToolsFromAgentId() calls, always verify both agent ownership and eventType ownership are checked.
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/AgentService.ts
🧬 Code graph analysis (4)
packages/features/calAIPhone/providers/retellAI/RetellAIService.ts (1)
packages/features/calAIPhone/providers/retellAI/types.ts (1)
Language(48-84)
packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts (1)
packages/features/calAIPhone/providers/retellAI/types.ts (1)
Language(48-84)
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx (1)
packages/features/calAIPhone/providers/retellAI/types.ts (1)
Language(48-84)
packages/features/calAIPhone/providers/retellAI/services/AgentService.ts (2)
packages/features/calAIPhone/providers/retellAI/index.ts (1)
Language(64-64)packages/features/calAIPhone/providers/retellAI/types.ts (1)
Language(48-84)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (9)
packages/trpc/server/routers/viewer/aiVoiceAgent/update.schema.ts (1)
64-65: Schema addition looks correct.Optional field with validated enum aligns with downstream usage.
packages/features/calAIPhone/providers/retellAI/RetellAIService.ts (1)
174-175: LGTM: language plumbed through the service.Signature and forwarding to AgentService are consistent.
packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts (1)
25-25: LGTM: provider surface updated to accept Language.Import and param typing align with RetellAIService.
Also applies to: 246-247
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx (2)
205-206: Sensible default.Defaulting to "en-US" matches the UX expectation and backend defaults.
216-218: Populate from existing data (LGTM).Graceful fallback to "en-US" if not set.
packages/features/calAIPhone/providers/retellAI/services/AgentService.ts (3)
510-522: LGTM: language added to method contract.The parameter is threaded through consistently.
544-546: LGTM: change detection updated.Including
voiceIdandlanguageinhasRetellUpdatesavoids unnecessary calls.
564-569: LGTM: agent update batching.Building one payload for
voice_idand/orlanguageis clean and reduces external calls.packages/trpc/server/routers/viewer/aiVoiceAgent/update.handler.ts (1)
28-29: Verified: language codes match across provider types, TRPC schema, and UI.
packages/features/calAIPhone/providers/retellAI/types.ts, packages/trpc/server/routers/viewer/aiVoiceAgent/update.schema.ts, and packages/features/ee/workflows/components/AgentConfigurationSheet.tsx contain the same 36 language values. No action required.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (15)
packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts (1)
86-92: New listVoices route wired correctly.Auth + dynamic import matches existing patterns.
Consider attaching route metadata (e.g., rate‑limit key) if this is called by type‑ahead UIs.
packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts (1)
275-287: SDK voice listing implementation is straightforward and robust.Logs, calls, returns, and rethrows as expected.
If the SDK exposes pagination or filtering, consider parameters to avoid fetching large lists unnecessarily.
packages/trpc/server/routers/viewer/aiVoiceAgent/listVoices.handler.ts (1)
11-15: Handler is minimal and consistent.It doesn’t use
ctxbeyond auth; optional to passctx.user.idinto a sub‑logger for audit trails.packages/features/calAIPhone/providers/retellAI/RetellAIService.ts (1)
15-15: Annotate listVoices return type (and import RetellVoice)Make the public surface explicit and self-documenting.
Apply this diff:
import { VoiceService } from "./services/VoiceService"; import type { RetellLLM, RetellCall, RetellAgent, RetellPhoneNumber, RetellDynamicVariables, AIConfigurationSetup, AIConfigurationDeletion, DeletionResult, RetellAIRepository, RetellLLMGeneralTools, - Language, + Language, + RetellVoice, } from "./types"; @@ - async listVoices() { - return this.voiceService.listVoices(); - } + async listVoices(): Promise<RetellVoice[]> { + return this.voiceService.listVoices(); + }Also applies to: 262-264
packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts (2)
25-25: Type the listVoices provider method and import RetellVoiceExpose the precise return type at the provider boundary.
Apply this diff:
-import type { RetellAIRepository, Language } from "./types"; +import type { RetellAIRepository, Language, RetellVoice } from "./types"; @@ - async listVoices() { - return await this.service.listVoices(); - } + async listVoices(): Promise<RetellVoice[]> { + return await this.service.listVoices(); + }Also applies to: 307-309
236-249: Avoid forwardingenableddownstream (defensive)
enabledis intentionally not acted on in the Retell flow. To prevent accidental propagation to deeper layers now or later, omit it when delegating.Apply this diff:
async updateAgentConfiguration(params: { id: string; userId: number; teamId?: number; name?: string; enabled?: boolean; generalPrompt?: string | null; beginMessage?: string | null; generalTools?: AIPhoneServiceTools<AIPhoneServiceProviderType.RETELL_AI>; voiceId?: string; language?: Language; }): Promise<{ message: string }> { - return await this.service.updateAgentConfiguration(params); + const { enabled: _enabled, ...rest } = params; + return await this.service.updateAgentConfiguration(rest); }If you prefer keeping it as-is, confirm AgentService.updateAgentConfiguration ignores unknown props (especially
enabled) before merging.packages/features/ee/workflows/components/VoiceSelectionDialog.tsx (5)
19-27: Use shared RetellVoice type instead of redefining VoiceAvoid drift by reusing the backend-exported type.
Apply this diff:
-import { showToast } from "@calcom/ui/components/toast"; - -type Voice = { - voice_id: string; - voice_name: string; - voice_type: string; - gender?: string; - accent?: string; - preview_audio_url?: string; -}; +import { showToast } from "@calcom/ui/components/toast"; +import type { RetellVoice as Voice } from "@calcom/features/calAIPhone/providers/retellAI/types";
60-65: Localize toast messagesStrings in TSX must use t(). Wrap the toasts.
Apply this diff:
- if (!previewUrl) { - showToast("Preview not available for this voice", "error"); + if (!previewUrl) { + showToast(t("preview_not_available_for_this_voice"), "error"); return; } @@ - audio.play().catch(() => { + audio.play().catch(() => { setPlayingVoiceId(null); setCurrentAudio(null); - showToast("Failed to play voice preview", "error"); + showToast(t("failed_to_play_voice_preview"), "error"); }); @@ - audio.onerror = () => { + audio.onerror = () => { setPlayingVoiceId(null); setCurrentAudio(null); - showToast("Failed to play voice preview", "error"); + showToast(t("failed_to_play_voice_preview"), "error"); }; @@ - const handleUseVoice = (voiceId: string) => { + const handleUseVoice = (voiceId: string) => { onVoiceSelect(voiceId); - showToast("Voice selected successfully", "success"); + showToast(t("voice_selected_successfully"), "success"); };Also applies to: 80-85, 91-95, 98-101
45-58: Add audio cleanup on unmount to prevent leaksPause/cleanup any playing preview when the component unmounts.
Apply this diff:
const [currentAudio, setCurrentAudio] = useState<HTMLAudioElement | null>(null); @@ const [rowSelection, setRowSelection] = useState({}); + + // Cleanup preview audio on unmount + useEffect(() => { + return () => { + if (currentAudio) { + currentAudio.pause(); + } + }; + }, [currentAudio]);
35-41: Avoid unnecessary segment fetching/URL query churn in a modal tableThis dialog table doesn’t need segments or URL-bound state. Disable segment fetching to reduce network and prevent query param mutations.
Apply this diff:
-import { useSegments } from "@calcom/features/data-table/hooks/useSegments"; @@ -function VoiceSelectionTable({ selectedVoiceId, onVoiceSelect }: { selectedVoiceId?: string; onVoiceSelect: (voiceId: string) => void }) { +function VoiceSelectionTable({ selectedVoiceId, onVoiceSelect }: { selectedVoiceId?: string; onVoiceSelect: (voiceId: string) => void }) { return ( - <DataTableProvider useSegments={useSegments} defaultPageSize={1000}> + <DataTableProvider segments={[]} defaultPageSize={1000}> <VoiceSelectionContent selectedVoiceId={selectedVoiceId} onVoiceSelect={onVoiceSelect} /> </DataTableProvider> ); }Also applies to: 215-224
47-48: Remove unused rowSelection stateRow selection is disabled; the extra state increases complexity.
Apply this diff:
- const [rowSelection, setRowSelection] = useState({}); @@ const table = useReactTable({ data: voiceData, columns, enableRowSelection: false, manualPagination: false, - state: { - rowSelection, - }, + state: {}, getCoreRowModel: getCoreRowModel(), getSortedRowModel: getSortedRowModel(), - onRowSelectionChange: setRowSelection, getRowId: (row) => row.voice_id, });Also applies to: 199-211
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx (4)
73-111: Localize language option labels; avoid hard-coded EnglishPer TSX guidelines, replace hard-coded labels with t() so the dropdown localizes correctly. Consider centralizing supported languages in a shared constant (single source of truth).
If you want, I can generate the language i18n keys for apps/web/public/static/locales/en/common.json.
113-119: Constrain language with Zod enum derived from optionsValidate against the supported set instead of allowing any string.
Apply this diff:
-const agentSchema = z.object({ +const LANGUAGE_VALUES = LANGUAGE_OPTIONS.map((o) => o.value) as const; +type LanguageValue = (typeof LANGUAGE_VALUES)[number]; + +const agentSchema = z.object({ generalPrompt: z.string().min(1, "General prompt is required"), beginMessage: z.string().min(1, "Begin message is required"), numberToCall: z.string().optional(), - language: z.string().optional(), + language: z.enum(LANGUAGE_VALUES).optional(), voiceId: z.string().optional(),
489-505: Select: prefer stable options and value typesUsing LANGUAGE_OPTIONS objects is fine. Once labels are localized, ensure the Select’s value prop stays stable (string code) and options are memoized to avoid unnecessary re-renders.
You can lift LANGUAGE_OPTIONS inside the component with useMemo(() => optionsWithT, [t]).
341-343: Only send changed fields (optional)To reduce no-op updates, send language/voiceId only if changed from agentData.retellData.
I can add a small diff that compares current vs. original and prunes unchanged keys.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
apps/web/public/icons/sprite.svgis excluded by!**/*.svg
📒 Files selected for processing (13)
apps/web/public/static/locales/en/common.json(1 hunks)packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts(3 hunks)packages/features/calAIPhone/providers/retellAI/RetellAIService.ts(5 hunks)packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts(2 hunks)packages/features/calAIPhone/providers/retellAI/services/VoiceService.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/types.ts(2 hunks)packages/features/ee/workflows/components/AgentConfigurationSheet.tsx(10 hunks)packages/features/ee/workflows/components/VoiceSelectionDialog.tsx(1 hunks)packages/features/ee/workflows/components/WebCallDialog.tsx(1 hunks)packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts(1 hunks)packages/trpc/server/routers/viewer/aiVoiceAgent/listVoices.handler.ts(1 hunks)packages/ui/components/icon/icon-list.mjs(1 hunks)packages/ui/components/icon/icon-names.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/features/ee/workflows/components/WebCallDialog.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/calAIPhone/providers/retellAI/types.tspackages/features/calAIPhone/providers/retellAI/services/VoiceService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/listVoices.handler.tspackages/ui/components/icon/icon-names.tspackages/trpc/server/routers/viewer/aiVoiceAgent/_router.tspackages/features/calAIPhone/providers/retellAI/RetellSDKClient.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/calAIPhone/providers/retellAI/types.tspackages/features/calAIPhone/providers/retellAI/services/VoiceService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/listVoices.handler.tspackages/ui/components/icon/icon-names.tspackages/features/ee/workflows/components/VoiceSelectionDialog.tsxpackages/trpc/server/routers/viewer/aiVoiceAgent/_router.tspackages/features/calAIPhone/providers/retellAI/RetellSDKClient.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/ee/workflows/components/AgentConfigurationSheet.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/calAIPhone/providers/retellAI/types.tspackages/features/calAIPhone/providers/retellAI/services/VoiceService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/listVoices.handler.tspackages/ui/components/icon/icon-names.tspackages/features/ee/workflows/components/VoiceSelectionDialog.tsxpackages/trpc/server/routers/viewer/aiVoiceAgent/_router.tspackages/features/calAIPhone/providers/retellAI/RetellSDKClient.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/ee/workflows/components/AgentConfigurationSheet.tsx
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/features/calAIPhone/providers/retellAI/services/VoiceService.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/VoiceSelectionDialog.tsxpackages/features/ee/workflows/components/AgentConfigurationSheet.tsx
🧠 Learnings (6)
📚 Learning: 2025-08-14T10:48:52.586Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/ai/_router.ts:46-84
Timestamp: 2025-08-14T10:48:52.586Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/ai/_router.ts, the voiceId input parameter in the create endpoint is intentionally not forwarded to aiService.createAgent() as voice customization is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/VoiceService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/listVoices.handler.tspackages/trpc/server/routers/viewer/aiVoiceAgent/_router.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts
📚 Learning: 2025-08-17T22:00:16.329Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts:117-126
Timestamp: 2025-08-17T22:00:16.329Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts, the enabled input parameter in the update endpoint is intentionally not forwarded to aiService.updateAgentConfiguration() as the enabled/disabled agent functionality is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
Applied to files:
packages/trpc/server/routers/viewer/aiVoiceAgent/listVoices.handler.tspackages/trpc/server/routers/viewer/aiVoiceAgent/_router.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.ts
📚 Learning: 2025-08-08T09:27:23.896Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.896Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts
📚 Learning: 2025-08-08T09:29:11.681Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, the variable insertion pattern in AgentConfigurationSheet.tsx follows the same implementation as existing workflow code. The current approach of building variable tokens directly from the input variable (with toUpperCase and space replacement) is the established pattern and doesn't need to be changed for locale-agnostic concerns, per maintainer Udit-takkar.
Applied to files:
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx
🧬 Code graph analysis (7)
packages/features/calAIPhone/providers/retellAI/services/VoiceService.ts (2)
packages/features/calAIPhone/providers/retellAI/types.ts (2)
RetellAIRepository(162-193)RetellVoice(9-9)apps/api/v2/src/lib/logger.bridge.ts (1)
error(77-79)
packages/features/ee/workflows/components/VoiceSelectionDialog.tsx (3)
packages/features/data-table/DataTableProvider.tsx (1)
DataTableProvider(93-426)packages/features/data-table/hooks/useSegments.ts (1)
useSegments(9-66)packages/ui/components/button/Button.tsx (1)
Button(221-349)
packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts (1)
packages/trpc/server/routers/viewer/aiVoiceAgent/listVoices.handler.ts (1)
listVoicesHandler(11-15)
packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts (1)
packages/features/calAIPhone/providers/retellAI/types.ts (1)
RetellVoice(9-9)
packages/features/calAIPhone/providers/retellAI/RetellAIService.ts (3)
packages/features/calAIPhone/providers/retellAI/services/VoiceService.ts (1)
VoiceService(6-30)packages/features/calAIPhone/providers/retellAI/types.ts (1)
Language(49-85)packages/features/calAIPhone/providers/retellAI/index.ts (1)
Language(64-64)
packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts (1)
packages/features/calAIPhone/providers/retellAI/types.ts (1)
Language(49-85)
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx (2)
packages/features/calAIPhone/providers/retellAI/types.ts (1)
Language(49-85)packages/features/ee/workflows/components/VoiceSelectionDialog.tsx (1)
VoiceSelectionDialog(227-258)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
🔇 Additional comments (9)
packages/features/calAIPhone/providers/retellAI/types.ts (2)
9-9: Type alias addition is correct and consistent with SDK.
Retell.VoiceResponseasRetellVoicematches the SDK surface.
191-193: Resolved: all RetellAIRepository implementers include listVoices()Confirmed implementations in:
- packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts
- packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts
- packages/features/calAIPhone/providers/retellAI/RetellAIService.ts
- packages/features/calAIPhone/providers/retellAI/services/VoiceService.ts
- packages/trpc/server/routers/viewer/aiVoiceAgent/listVoices.handler.ts
No further action required.
packages/ui/components/icon/icon-names.ts (1)
108-108: IconName union additions look good.Matches usage for play/pause UI and aligns with icon list.
If this file is generated (“build:icons”), ensure the generator includes these names to avoid drift on the next build.
Also applies to: 114-114
packages/ui/components/icon/icon-list.mjs (1)
101-101: Set updated to include new icons.LGTM; aligns with IconName.
Also applies to: 106-106
packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts (1)
19-20: Type import is correct.Keeps SDK client strongly typed.
packages/features/calAIPhone/providers/retellAI/RetellAIService.ts (2)
36-36: Good addition: voice service wiring is cohesiveInjecting VoiceService and initializing it in the constructor is consistent with the existing service composition.
Also applies to: 54-55
177-178: Language passthrough looks correctExtending updateAgentConfiguration with language?: Language and forwarding via ...params matches the types and upstream changes.
packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts (1)
246-247: Language param passthrough is consistentAccepting language?: Language here keeps the provider API in sync with the underlying service.
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx (1)
1135-1142: Voice dialog integration looks goodState wiring and dirty-flag update on selection are correct.
packages/features/calAIPhone/providers/retellAI/services/VoiceService.ts
Show resolved
Hide resolved
packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts
Outdated
Show resolved
Hide resolved
packages/features/ee/workflows/components/VoiceSelectionDialog.tsx
Outdated
Show resolved
Hide resolved
packages/trpc/server/routers/viewer/aiVoiceAgent/update.schema.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/features/ee/workflows/hooks/useVoicePreview.ts (6)
1-1: Add missing import for useEffectRequired for the unmount cleanup.
-import { useState } from "react"; +import { useEffect, useState } from "react";
31-50: Bind event handlers before play() to avoid missing very-short “ended” eventsSet onended/onerror before calling play() to prevent races on very short clips.
- const audio = new Audio(previewUrl); - setCurrentAudio(audio); - setPlayingVoiceId(voiceId || null); - - audio.play().catch(() => { - setPlayingVoiceId(null); - setCurrentAudio(null); - showToast("Failed to play voice preview", "error"); - }); - - audio.onended = () => { + const audio = new Audio(previewUrl); + audio.onended = () => { setPlayingVoiceId(null); setCurrentAudio(null); }; - audio.onerror = () => { + audio.onerror = () => { setPlayingVoiceId(null); setCurrentAudio(null); showToast("Failed to play voice preview", "error"); }; + + setCurrentAudio(audio); + setPlayingVoiceId(voiceId || null); + + audio.play().catch(() => { + setPlayingVoiceId(null); + setCurrentAudio(null); + showToast("Failed to play voice preview", "error"); + });
9-16: Release media resources fully when stoppingPausing is good; also clear src and call load() to abort any in-flight fetch and free memory sooner.
const stopCurrentAudio = () => { if (currentAudio) { currentAudio.pause(); - currentAudio.currentTime = 0; + try { + // Abort network and free buffers + currentAudio.src = ""; + currentAudio.load(); + currentAudio.currentTime = 0; + } catch {} setCurrentAudio(null); } setPlayingVoiceId(null); };
18-23: Use i18n/localized messages instead of hard‑coded English stringsThis PR is about language/voice; toast strings should be localized or injectable.
Minimal change: accept an optional messages param with sane defaults.
-export function useVoicePreview() { +export function useVoicePreview(messages = { + notAvailable: "Preview not available for this voice", + playFailed: "Failed to play voice preview", +}) { @@ - if (!previewUrl) { - showToast("Preview not available for this voice", "error"); + if (!previewUrl) { + showToast(messages.notAvailable, "error"); return; } @@ - audio.play().catch(() => { + audio.play().catch(() => { setPlayingVoiceId(null); setCurrentAudio(null); - showToast("Failed to play voice preview", "error"); + showToast(messages.playFailed, "error"); }); @@ - audio.onerror = () => { + audio.onerror = () => { setPlayingVoiceId(null); setCurrentAudio(null); - showToast("Failed to play voice preview", "error"); + showToast(messages.playFailed, "error"); };If you prefer central i18n, swap the default with t("…") at call sites.
Also applies to: 35-39, 46-50
7-7: Consider useRef for currentAudio to avoid extra re-renderscurrentAudio being state causes a re-render on every play/stop though UI can derive from playingVoiceId. Switching to useRef can reduce churn in heavy lists.
Refactor idea (illustrative):
- const [currentAudio, setCurrentAudio] = useState<HTMLAudioElement | null>(null); + const currentAudioRef = useRef<HTMLAudioElement | null>(null); @@ - if (currentAudio) { ...; setCurrentAudio(null); } + if (currentAudioRef.current) { ...; currentAudioRef.current = null; } @@ - const audio = new Audio(previewUrl); - setCurrentAudio(audio); + const audio = new Audio(previewUrl); + currentAudioRef.current = audio;Then adjust the cleanup effect to read currentAudioRef.current.
53-57: API shape: consider returning a convenience isPlaying helperCallers often need to check if a specific row’s voice is playing.
return { playingVoiceId, + isPlaying: (id?: string | null) => !!id && id === playingVoiceId, handlePlayVoice, stopCurrentAudio, };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/web/public/static/locales/en/common.json(1 hunks)packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts(2 hunks)packages/features/ee/workflows/components/VoiceSelectionDialog.tsx(1 hunks)packages/features/ee/workflows/hooks/useVoicePreview.ts(1 hunks)packages/trpc/server/routers/viewer/aiVoiceAgent/update.schema.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/public/static/locales/en/common.json
- packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts
- packages/trpc/server/routers/viewer/aiVoiceAgent/update.schema.ts
- packages/features/ee/workflows/components/VoiceSelectionDialog.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/ee/workflows/hooks/useVoicePreview.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/workflows/hooks/useVoicePreview.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/workflows/hooks/useVoicePreview.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/ai/_router.ts:46-84
Timestamp: 2025-08-14T10:48:52.586Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/ai/_router.ts, the voiceId input parameter in the create endpoint is intentionally not forwarded to aiService.createAgent() as voice customization is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
🔇 Additional comments (2)
packages/features/ee/workflows/hooks/useVoicePreview.ts (2)
24-27: Nice UX: toggle to stop when the same voice is clickedSimple, predictable behavior. No changes needed.
5-5: Good call on named exportAligns with our guideline to avoid default exports.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/features/ee/workflows/hooks/useVoicePreview.ts (1)
63-71: Cleanup on unmount is now in place — resolves prior “ghost” audioMatches the earlier review ask to pause and reset on unmount.
🧹 Nitpick comments (5)
packages/features/calAIPhone/providers/retellAI/index.ts (1)
1-24: Optional: switch to direct type re‑exports to drop the extra importSlightly cleaner and avoids keeping two lists in sync.
-import type { - RetellAIRepository, - CreateLLMRequest, - CreateAgentRequest, - UpdateLLMRequest, - AIConfigurationSetup, - AIConfigurationDeletion, - DeletionResult, - RetellLLM, - RetellAgent, - RetellCall, - CreatePhoneCallParams, - RetellPhoneNumber, - UpdatePhoneNumberParams, - CreatePhoneNumberParams, - ImportPhoneNumberParams, - UpdateAgentRequest, - RetellLLMGeneralTools, - RetellAgentWithDetails, - Language, - RetellDynamicVariables, - RetellCallListParams, - RetellCallListResponse, -} from "./types"; +// (types re-exported below) @@ -export type { - RetellAIRepository, - CreateLLMRequest, - CreateAgentRequest, - UpdateLLMRequest, - AIConfigurationSetup, - AIConfigurationDeletion, - DeletionResult, - RetellLLM, - RetellAgent, - RetellCall, - CreatePhoneCallParams, - RetellPhoneNumber, - UpdatePhoneNumberParams, - CreatePhoneNumberParams, - ImportPhoneNumberParams, - UpdateAgentRequest, - RetellLLMGeneralTools, - RetellAgentWithDetails, - Language, - RetellDynamicVariables, - RetellCallListParams, - RetellCallListResponse, - RetellVoice, -}; +export type { + RetellAIRepository, + CreateLLMRequest, + CreateAgentRequest, + UpdateLLMRequest, + AIConfigurationSetup, + AIConfigurationDeletion, + DeletionResult, + RetellLLM, + RetellAgent, + RetellCall, + CreatePhoneCallParams, + RetellPhoneNumber, + UpdatePhoneNumberParams, + CreatePhoneNumberParams, + ImportPhoneNumberParams, + UpdateAgentRequest, + RetellLLMGeneralTools, + RetellAgentWithDetails, + Language, + RetellDynamicVariables, + RetellCallListParams, + RetellCallListResponse, + RetellVoice, +} from "./types";Also applies to: 46-70
packages/features/ee/workflows/hooks/useVoicePreview.ts (3)
35-46: Guard against setState after unmount and stale audioEvent handlers may still fire after unmount; guard updates against a mounted ref and ensure we only clear the active audio instance.
@@ - const audioRef = useRef<HTMLAudioElement | null>(null); + const audioRef = useRef<HTMLAudioElement | null>(null); + const mountedRef = useRef(true); @@ - audio.onended = () => { - setPlayingVoiceId(null); - setCurrentAudio(null); - audioRef.current = null; - }; + audio.onended = () => { + if (!mountedRef.current || audioRef.current !== audio) return; + setPlayingVoiceId(null); + setCurrentAudio(null); + audioRef.current = null; + }; @@ - audio.onerror = () => { - setPlayingVoiceId(null); - setCurrentAudio(null); - audioRef.current = null; - showToast("Failed to play voice preview", "error"); - }; + audio.onerror = () => { + if (!mountedRef.current || audioRef.current !== audio) return; + setPlayingVoiceId(null); + setCurrentAudio(null); + audioRef.current = null; + showToast("Failed to play voice preview", "error"); + }; @@ - audio.play() + audio.play() .then(() => { - setPlayingVoiceId(voiceId || null); + if (!mountedRef.current || audioRef.current !== audio) return; + setPlayingVoiceId(voiceId || null); }) .catch(() => { - setPlayingVoiceId(null); - setCurrentAudio(null); - audioRef.current = null; - showToast("Failed to play voice preview", "error"); + if (!mountedRef.current || audioRef.current !== audio) return; + setPlayingVoiceId(null); + setCurrentAudio(null); + audioRef.current = null; + showToast("Failed to play voice preview", "error"); });Add mount flag management:
@@ - useEffect(() => { - return () => { + useEffect(() => { + return () => { + mountedRef.current = false; if (audioRef.current) { audioRef.current.pause(); audioRef.current.currentTime = 0; audioRef.current = null; } }; }, []);Also applies to: 51-60
6-18: Optional: remove currentAudio from state; use only a refDOM nodes in React state cause extra re-renders; the ref is sufficient. This simplifies logic and avoids stale state.
- const [currentAudio, setCurrentAudio] = useState<HTMLAudioElement | null>(null); + // Track active audio element via ref only const audioRef = useRef<HTMLAudioElement | null>(null); @@ - if (currentAudio) { - currentAudio.pause(); - currentAudio.currentTime = 0; - setCurrentAudio(null); - audioRef.current = null; - } + if (audioRef.current) { + audioRef.current.pause(); + audioRef.current.currentTime = 0; + audioRef.current = null; + } @@ - setCurrentAudio(audio); audioRef.current = audio;Also applies to: 48-50
21-24: Localize toast messagesReplace hardcoded strings with i18n keys to match the rest of the UI.
Also applies to: 41-46, 55-60
packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (1)
317-318: Prefer provider-typed Language over plain stringStrengthen type-safety by threading provider language type via the type map (similar to Voice).
Primary change here:
- language?: string; + language?: AIPhoneServiceLanguage<T>;Add this alias near the other generic aliases:
+export type AIPhoneServiceLanguage<T extends AIPhoneServiceProviderType = AIPhoneServiceProviderType> = + AIPhoneServiceProviderTypeMap[T]["Language"];And extend the provider type map for Retell to expose Language (in providers/retellAI/index.ts):
export interface RetellAIPhoneServiceProviderTypeMap { @@ ListCallsResponse: RetellCallListResponse; - Voice: RetellVoice; + Voice: RetellVoice; + Language: Language; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts(3 hunks)packages/features/calAIPhone/providers/retellAI/index.ts(3 hunks)packages/features/ee/workflows/hooks/useVoicePreview.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/calAIPhone/providers/retellAI/index.tspackages/features/ee/workflows/hooks/useVoicePreview.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/calAIPhone/providers/retellAI/index.tspackages/features/ee/workflows/hooks/useVoicePreview.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/calAIPhone/providers/retellAI/index.tspackages/features/ee/workflows/hooks/useVoicePreview.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/ai/_router.ts:46-84
Timestamp: 2025-08-14T10:48:52.586Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/ai/_router.ts, the voiceId input parameter in the create endpoint is intentionally not forwarded to aiService.createAgent() as voice customization is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
📚 Learning: 2025-08-14T10:48:52.586Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/ai/_router.ts:46-84
Timestamp: 2025-08-14T10:48:52.586Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/ai/_router.ts, the voiceId input parameter in the create endpoint is intentionally not forwarded to aiService.createAgent() as voice customization is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/index.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.ts
📚 Learning: 2025-08-08T09:29:11.681Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts
📚 Learning: 2025-08-08T09:27:23.896Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.896Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts
🧬 Code graph analysis (2)
packages/features/calAIPhone/providers/retellAI/index.ts (1)
packages/features/calAIPhone/providers/retellAI/types.ts (1)
RetellVoice(9-9)
packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (1)
packages/features/calAIPhone/index.ts (1)
AIPhoneServiceProviderType(16-16)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (5)
packages/features/calAIPhone/providers/retellAI/index.ts (3)
33-33: Barrel export for VoiceService — good additionKeeps provider surface cohesive and discoverable.
69-70: Publicly exposing RetellVoice type — aligns with new APIThis enables strong typing for listVoices across layers.
88-89: Provider type map extended with Voice — correct wiringThis unblocks AIPhoneServiceVoice resolution in the interface layer.
packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (2)
99-101: AIPhoneServiceVoice type alias — correct and usefulThis cleanly bridges provider map to interface consumers.
382-385: listVoices — implemented for RETELL_AI; run TS build to confirmlistVoices is implemented and typed for RETELL_AI and exposed via tRPC. Key files: packages/features/calAIPhone/providers/retellAI/{RetellAIPhoneServiceProvider.ts, RetellAIService.ts, services/VoiceService.ts, RetellSDKClient.ts, types.ts, index.ts}; interface: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts; tRPC: packages/trpc/server/routers/viewer/aiVoiceAgent/{listVoices.handler.ts,_router.ts}. AIPhoneServiceProviderTypeMap only lists RETELL_AI. Run the TypeScript build/CI to confirm there are no type/build errors.
emrysal
left a comment
There was a problem hiding this comment.
thanks for the fixes, lets get it merged 🚀
E2E results are ready! |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/features/ee/workflows/components/VoiceSelectionDialog.tsx (1)
47-49: Nice: preview logic extracted into a hook.This aligns with prior feedback to move preview state/side-effects out of the component.
🧹 Nitpick comments (5)
packages/features/ee/workflows/components/VoiceSelectionDialog.tsx (5)
54-57: Localize toast message.Hardcoded string should use t() per frontend i18n guidelines.
Apply this diff:
- showToast("Voice selected successfully", "success"); + showToast(t("voice_selected_successfully"), "success");
73-86: Add accessible label to the play/pause icon button.Screen readers need an aria-label describing the action.
Apply this diff:
<Button type="button" variant="icon" color="secondary" size="sm" + aria-label={ + playingVoiceId === row.original.voice_id + ? t("pause_preview") + : t("play_preview") + } onClick={() => handlePlayVoice(row.original.preview_audio_url, row.original.voice_id)} className="rounded-full">
21-29: Reuse shared voice type to prevent drift with backend/provider.Prefer the exported AIPhoneServiceVoice over a local duplicate.
Apply this diff:
+import type { AIPhoneServiceVoice } from "@calcom/features/calAIPhone/interfaces/AIPhoneService.interface";-type Voice = { - voice_id: string; - voice_name: string; - provider: 'elevenlabs' | 'openai' | 'deepgram'; - gender: 'male' | 'female'; - accent?: string; - age?: string; - preview_audio_url?: string; -};- const voiceData: Voice[] = useMemo(() => { + const voiceData: AIPhoneServiceVoice[] = useMemo(() => { if (!voices) return []; return voices; }, [voices]);- const columns = useMemo<ColumnDef<Voice>[]>( + const columns = useMemo<ColumnDef<AIPhoneServiceVoice>[]>(- <DataTableWrapper<Voice> + <DataTableWrapper<AIPhoneServiceVoice> testId="voice-selection-data-table" table={table} isPending={isLoading} totalRowCount={voiceData?.length || 0} paginationMode="standard" className="h-auto" />Also applies to: 59-63, 64-71, 175-181, 170-171
49-50: Drop unused row selection state.Row selection is disabled; no need to track state or wire selection handlers.
Apply this diff:
- const [rowSelection, setRowSelection] = useState({});const table = useReactTable({ data: voiceData, columns, - enableRowSelection: false, - manualPagination: false, - state: { - rowSelection, - }, + enableRowSelection: false, + manualPagination: false, getCoreRowModel: getCoreRowModel(), getSortedRowModel: getSortedRowModel(), - onRowSelectionChange: setRowSelection, getRowId: (row) => row.voice_id, });Also applies to: 162-170
48-49: Follow-up: localize error toasts inside useVoicePreview.Hook currently emits hardcoded toasts; mirror t() usage there for consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/public/static/locales/en/common.json(1 hunks)packages/features/ee/workflows/components/VoiceSelectionDialog.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/public/static/locales/en/common.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/VoiceSelectionDialog.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/workflows/components/VoiceSelectionDialog.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/workflows/components/VoiceSelectionDialog.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/ai/_router.ts:46-84
Timestamp: 2025-08-14T10:48:52.586Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/ai/_router.ts, the voiceId input parameter in the create endpoint is intentionally not forwarded to aiService.createAgent() as voice customization is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
📚 Learning: 2025-08-14T10:48:52.586Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/ai/_router.ts:46-84
Timestamp: 2025-08-14T10:48:52.586Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/ai/_router.ts, the voiceId input parameter in the create endpoint is intentionally not forwarded to aiService.createAgent() as voice customization is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
Applied to files:
packages/features/ee/workflows/components/VoiceSelectionDialog.tsx
🧬 Code graph analysis (1)
packages/features/ee/workflows/components/VoiceSelectionDialog.tsx (3)
packages/features/data-table/DataTableProvider.tsx (1)
DataTableProvider(93-426)packages/features/data-table/hooks/useSegments.ts (1)
useSegments(9-66)packages/features/ee/workflows/hooks/useVoicePreview.ts (1)
useVoicePreview(5-78)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
What does this PR do?
This PR adds changes to customize the agent's language and voice
Screen.Recording.2025-09-16.at.4.18.56.PM.mov
You can test a web call using ngrok.
You can check instructions here #23527
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?