Conversation
WalkthroughAdds an Insights Call History feature spanning frontend, backend, types, and translations. Frontend: new /insights/call-history page with metadata and permission guard, a Call History table component (facets, search, filters, pagination), a Call Details sheet component, navigation entry, and English locale keys. Backend: new authed TRPC endpoint viewer.aiVoiceAgent.listCalls with Zod input schema and handler that resolves accessible phone numbers (memberships, phone-number repository), validates date filters, and delegates to an AI phone service. calAIPhone/Retell provider stack extended with listCalls types, SDK client method, service/provider wrappers, and a CallService implementation. Repository helpers for phone numbers and membership queries were added. Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
✨ 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 |
keithwillcode
added
core
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
dosubot
bot
added
ai
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/calAIPhone/providers/retellAI/index.ts (1)
66-86: Fix inconsistent listCalls typings — align to RetellCallListParams / RetellCallListResponse
- Issue: Some layers use an inline param shape ({ userId, organizationId?, limit?, offset?, filters? }) while other parts use
RetellCallListParams/RetellCallListResponse.- Locations to update/inspect:
- packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:369
- packages/features/calAIPhone/providers/retellAI/RetellAIService.ts:246
- packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts:294
- packages/features/calAIPhone/providers/retellAI/types.ts:184
- packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts:241
- packages/features/calAIPhone/providers/retellAI/services/CallService.ts:290
- Fix: Converge on
RetellCallListParams/RetellCallListResponseacross the interface, service, provider and SDK — either change signatures to those types or add a single adapter at the service boundary that maps the inline shape toRetellCallListParamsbefore callingretellRepository.listCalls. Re-run type checks/CI.
🧹 Nitpick comments (21)
packages/lib/server/repository/membership.ts (1)
480-507: Clarify inclusion of unaccepted memberships and tighten types (optional)This returns all memberships unless filters.accepted is provided. If call sites use this for auth scoping, you likely want accepted: true by default to avoid pulling pending invites.
If intentional, ignore. Otherwise, consider defaulting accepted to true at the caller or here.
Optionally, expose a typed return DTO to avoid accidental shape drift in consumers.
packages/features/calAIPhone/providers/retellAI/RetellAIService.ts (1)
246-254: Avoidanyfor filters; align with input schemaUse a precise type for filters to prevent accidental shape drift between TRPC input, services, and repository.
Apply this diff:
- async listCalls(params: { - userId: number; - organizationId?: number; - limit?: number; - offset?: number; - filters?: any; - }) { + async listCalls(params: { + userId: number; + organizationId?: number; + limit?: number; + offset?: number; + filters?: { phoneNumberId?: string[]; startDate?: string; endDate?: string }; + }) { return this.callService.listCalls(params); }packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts (1)
294-302: Type filters and confirm org-level scopeMirror the filter shape here to keep provider surface strongly typed. Also confirm that
organizationId(notteamId) is the intended scope for listing calls.Apply this diff:
- async listCalls(params: { - userId: number; - organizationId?: number; - limit?: number; - offset?: number; - filters?: any; - }) { + async listCalls(params: { + userId: number; + organizationId?: number; + limit?: number; + offset?: number; + filters?: { phoneNumberId?: string[]; startDate?: string; endDate?: string }; + }) { return await this.service.listCalls(params); }packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.schema.ts (1)
1-15: Strengthen date validation and enforce start ≤ end (optional)Tighten filters to catch malformed dates and inverted ranges at the boundary.
Apply this diff:
export const ZListCallsInputSchema = z.object({ limit: z.number().min(1).max(1000).default(50), offset: z.number().min(0).default(0), filters: z .object({ phoneNumberId: z.array(z.string()).optional(), - startDate: z.string().optional(), - endDate: z.string().optional(), + startDate: z.string().datetime().optional(), + endDate: z.string().datetime().optional(), }) + .refine( + (f) => !f?.startDate || !f?.endDate || new Date(f.startDate) <= new Date(f.endDate), + { message: "endDate must be on or after startDate" } + ) .optional(), });packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (1)
67-74: Honor phone number filter from input.
filters.phoneNumberIdexists in schema but isn’t applied. If intended to filter by specific numbers, apply it here; otherwise remove from schema to avoid dead params.Apply:
- const calls = await aiService.listCalls({ - phoneNumbers: uniquePhoneNumbers, + const scopedPhoneNumbers = + input.filters?.phoneNumberId?.length + ? uniquePhoneNumbers.filter((pn) => input.filters!.phoneNumberId!.includes(pn)) + : uniquePhoneNumbers; + + const calls = await aiService.listCalls({ + phoneNumbers: scopedPhoneNumbers, limit: input.limit, offset: input.offset, filters: { ...(startTimestamp && { startTimestamp }), }, });If
phoneNumberIdtruly refers to DB IDs, map IDs → numbers via repository before filtering.apps/web/app/(use-page-wrapper)/insights/call-history/page.tsx (1)
7-14: Remove redundant await in generateMetadata.No need to await and then return; just return the promise.
-export const generateMetadata = async () => - await _generateMetadata( +export const generateMetadata = () => + _generateMetadata( (t) => t("call_history"), (t) => t("call_history_subtitle"), undefined, undefined, "/insights/call-history" );apps/web/modules/insights/insights-call-history-view.tsx (4)
16-16: Remove unused branding plumbing.useOrgBranding() and _domain are unused; drop both to reduce noise.
-import { useOrgBranding } from "@calcom/features/ee/organizations/context/provider"; @@ -function CallHistoryContent({ org: _org }: CallHistoryProps) { - const orgBranding = useOrgBranding(); - const _domain = orgBranding?.fullDomain ?? WEBAPP_URL; +function CallHistoryContent({ org: _org }: CallHistoryProps) {Also applies to: 73-75
116-132: Handle missing timestamps in renderer.Avoid creating Date("") or showing current time when data is absent.
- const date = new Date(row.original.time); - return ( + const hasTime = Boolean(row.original.time); + return hasTime ? ( + (() => { + const date = new Date(row.original.time); + return ( + <div className="text-sm"> + <div>{date.toLocaleDateString()}</div> + <div className="text-subtle">{date.toLocaleTimeString()}</div> + </div> + ); + })() + ) : ( <div className="text-sm"> - <div>{date.toLocaleDateString()}</div> - <div className="text-subtle">{date.toLocaleTimeString()}</div> + <div>{t("unknown")}</div> </div> );
169-181: Status mapping: add fallback instead of coercing to "failed".Unknown provider statuses shouldn’t be labeled failed.
- const status = row.original.sessionStatus; + const status = row.original.sessionStatus ?? "unknown"; - const variant = status === "completed" ? "green" : status === "ongoing" ? "blue" : "red"; - return <Badge variant={variant}>{status}</Badge>; + const variant = status === "completed" ? "green" : status === "ongoing" ? "blue" : status === "failed" ? "red" : "gray"; + return <Badge variant={variant}>{status}</Badge>;And in the mapping above:
- sessionStatus: - call.call_status === "ended" ? "completed" : call.call_status === "ongoing" ? "ongoing" : "failed", + sessionStatus: + call.call_status === "ended" + ? "completed" + : call.call_status === "ongoing" + ? "ongoing" + : undefined,
251-273: Localize facet labels.Hardcoded English breaks i18n. Use t() for labels.
- { label: "Web Call", value: "web_call" }, - { label: "Phone Call", value: "phone_call" }, + { label: t("web_call"), value: "web_call" }, + { label: t("phone_call"), value: "phone_call" }, @@ - { label: "Completed", value: "completed" }, - { label: "Ongoing", value: "ongoing" }, - { label: "Failed", value: "failed" }, + { label: t("completed"), value: "completed" }, + { label: t("ongoing"), value: "ongoing" }, + { label: t("failed"), value: "failed" }, @@ - { label: "Positive", value: "positive" }, - { label: "Neutral", value: "neutral" }, - { label: "Negative", value: "negative" }, + { label: t("positive"), value: "positive" }, + { label: t("neutral"), value: "neutral" }, + { label: t("negative"), value: "negative" },packages/features/ee/workflows/components/CallDetailsSheet.tsx (2)
95-96: Localize "Unknown".Avoid hardcoded English; use t("unknown") or similar.
- {selectedCall.call_analysis?.user_sentiment || "Unknown"} + {selectedCall.call_analysis?.user_sentiment || t("unknown")}
115-126: Guard recording section and localize audio fallback.Render recording only when a URL exists and localize the fallback text.
- {/* Recording Section */} - <div className="space-y-3"> + {/* Recording Section */} + {selectedCall.recording_url && ( + <div className="space-y-3"> <h3 className="text-emphasis text-base font-semibold">{t("recording")}</h3> <div className="flex items-center gap-3"> - <audio controls className="h-11 w-[258px]" src={selectedCall.recording_url}> - Your browser does not support the audio element. + <audio controls className="h-11 w-[258px]" src={selectedCall.recording_url}> + {t("audio_not_supported")} </audio> <Button color="minimal" href={selectedCall.recording_url} target="_blank" StartIcon="download"> {t("download")} </Button> </div> </div> + )}packages/lib/server/repository/calAiPhoneNumber.ts (1)
28-55: Dedup at the DB level and add explicit return types.Small perf/readability wins:
- Use Prisma
distinct: ["phoneNumber"]in each finder to reduce duplicates early.- Add explicit return types for the helper methods.
export class CalAiPhoneNumberRepository { - static async getUserPhoneNumbers(userId: number) { + static async getUserPhoneNumbers( + userId: number + ): Promise<Array<{ phoneNumber: string }>> { return prisma.calAiPhoneNumber.findMany({ where: { userId }, select: { phoneNumber: true }, + distinct: ["phoneNumber"], }); } - static async getOrganizationTeamPhoneNumbers(organizationId: number) { + static async getOrganizationTeamPhoneNumbers( + organizationId: number + ): Promise<Array<{ phoneNumber: string }>> { return prisma.calAiPhoneNumber.findMany({ where: { team: { parentId: organizationId }, }, select: { phoneNumber: true }, + distinct: ["phoneNumber"], }); } - static async getTeamPhoneNumbers(teamIds: number[]) { + static async getTeamPhoneNumbers( + teamIds: number[] + ): Promise<Array<{ phoneNumber: string }>> { return prisma.calAiPhoneNumber.findMany({ where: { teamId: { in: teamIds } }, select: { phoneNumber: true }, + distinct: ["phoneNumber"], }); }packages/features/ee/workflows/pages/call-history.tsx (8)
150-158: Localize channel type cell text.Replace raw strings with t().
- cell: ({ row }) => <span>{row.original.channelType}</span>, + cell: ({ row }) => ( + <span> + {row.original.channelType === "web_call" ? t("web_call") : t("phone_call")} + </span> + ),
173-185: Localize session status badge text.Use t() for user-visible strings.
- return <Badge variant={variant}>{status}</Badge>; + return ( + <Badge variant={variant}> + {status === "completed" ? t("completed") : status === "ongoing" ? t("ongoing") : t("failed")} + </Badge> + );
187-199: Localize sentiment badge text.Replace with t().
- return <Badge variant={variant}>{sentiment}</Badge>; + return ( + <Badge variant={variant}> + {sentiment === "positive" ? t("positive") : sentiment === "negative" ? t("negative") : t("neutral")} + </Badge> + );
255-277: Localize facet filter labels.These should use t() to match the rest of the UI.
case "channelType": return convertFacetedValuesToMap([ - { label: "Web Call", value: "web_call" }, - { label: "Phone Call", value: "phone_call" }, + { label: t("web_call"), value: "web_call" }, + { label: t("phone_call"), value: "phone_call" }, ]); case "sessionStatus": return convertFacetedValuesToMap([ - { label: "Completed", value: "completed" }, - { label: "Ongoing", value: "ongoing" }, - { label: "Failed", value: "failed" }, + { label: t("completed"), value: "completed" }, + { label: t("ongoing"), value: "ongoing" }, + { label: t("failed"), value: "failed" }, ]); case "userSentiment": return convertFacetedValuesToMap([ - { label: "Positive", value: "positive" }, - { label: "Neutral", value: "neutral" }, - { label: "Negative", value: "negative" }, + { label: t("positive"), value: "positive" }, + { label: t("neutral"), value: "neutral" }, + { label: t("negative"), value: "negative" }, ]);
74-83: Remove unused locals or document intentional side-effects.
_domainis unused._columnFiltersand_searchTermlook intentionally unused for provider side-effects; add a short comment, or wire them to the query (see next comment).- const orgBranding = useOrgBranding(); - const _domain = orgBranding?.fullDomain ?? WEBAPP_URL; + const orgBranding = useOrgBranding(); // branding may be used later; keep hook order + // const _domain = orgBranding?.fullDomain ?? WEBAPP_URL; // unused
238-255: Drop rowSelection plumbing since selection is disabled.Simplify: remove local rowSelection state and related props.
- const [rowSelection, setRowSelection] = useState({}); ... const table = useReactTable({ data: callHistoryData, columns, enableRowSelection: false, manualPagination: true, - state: { - rowSelection, - }, + state: {}, ... - onRowSelectionChange: setRowSelection, - getRowId: (row) => row.id, + getRowId: (row) => row.id,Also applies to: 78-79
201-211: Use a matching translation key for the "to" headeren/common.json defines "to" (line 1728) while the "from" column uses "from_header" (line 2477). In packages/features/ee/workflows/pages/call-history.tsx (lines 201–211) either change t("to") → t("to_header") and add "to_header": "To" to the locales file, or keep t("to") to match the existing key — preferred: add "to_header" and use it.
84-94: Wire supported table filters to listCalls (schema supports phoneNumberId, startDate, endDate — not searchTerm/sorting)
- Update packages/features/ee/workflows/pages/call-history.tsx (lines 84–94) to pass the table’s filter values into trpc.viewer.aiVoiceAgent.listCalls (use filters: { phoneNumberId, startDate, endDate }) instead of filters: {}.
- If you need searchTerm or sorting, first extend ZListCallsInputSchema and listCalls.handler (and propagate to aiService.listCalls) to accept and implement those fields.
📜 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 (20)
apps/web/app/(use-page-wrapper)/insights/call-history/page.tsx(1 hunks)apps/web/modules/insights/insights-call-history-view.tsx(1 hunks)apps/web/public/static/locales/en/common.json(3 hunks)packages/features/calAIPhone/index.ts(1 hunks)packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts(2 hunks)packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/RetellAIService.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts(2 hunks)packages/features/calAIPhone/providers/retellAI/index.ts(3 hunks)packages/features/calAIPhone/providers/retellAI/services/CallService.ts(2 hunks)packages/features/calAIPhone/providers/retellAI/types.ts(2 hunks)packages/features/ee/workflows/components/CallDetailsSheet.tsx(1 hunks)packages/features/ee/workflows/components/types.ts(1 hunks)packages/features/ee/workflows/pages/call-history.tsx(1 hunks)packages/features/shell/navigation/Navigation.tsx(1 hunks)packages/lib/server/repository/calAiPhoneNumber.ts(1 hunks)packages/lib/server/repository/membership.ts(1 hunks)packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts(2 hunks)packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts(1 hunks)packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.schema.ts(1 hunks)
🧰 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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.schema.tspackages/features/calAIPhone/providers/retellAI/types.tspackages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.tspackages/lib/server/repository/membership.tspackages/features/ee/workflows/components/types.tspackages/lib/server/repository/calAiPhoneNumber.tspackages/features/calAIPhone/providers/retellAI/services/CallService.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/providers/retellAI/index.tspackages/features/calAIPhone/index.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/_router.tspackages/features/calAIPhone/providers/retellAI/RetellSDKClient.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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.schema.tspackages/features/calAIPhone/providers/retellAI/types.tspackages/features/shell/navigation/Navigation.tsxpackages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.tspackages/lib/server/repository/membership.tspackages/features/ee/workflows/components/types.tspackages/lib/server/repository/calAiPhoneNumber.tspackages/features/calAIPhone/providers/retellAI/services/CallService.tspackages/features/ee/workflows/components/CallDetailsSheet.tsxpackages/features/calAIPhone/interfaces/AIPhoneService.interface.tsapps/web/modules/insights/insights-call-history-view.tsxpackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/providers/retellAI/index.tspackages/features/calAIPhone/index.tsapps/web/app/(use-page-wrapper)/insights/call-history/page.tsxpackages/features/ee/workflows/pages/call-history.tsxpackages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/_router.tspackages/features/calAIPhone/providers/retellAI/RetellSDKClient.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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.schema.tspackages/features/calAIPhone/providers/retellAI/types.tspackages/features/shell/navigation/Navigation.tsxpackages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.tspackages/lib/server/repository/membership.tspackages/features/ee/workflows/components/types.tspackages/lib/server/repository/calAiPhoneNumber.tspackages/features/calAIPhone/providers/retellAI/services/CallService.tspackages/features/ee/workflows/components/CallDetailsSheet.tsxpackages/features/calAIPhone/interfaces/AIPhoneService.interface.tsapps/web/modules/insights/insights-call-history-view.tsxpackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/providers/retellAI/index.tspackages/features/calAIPhone/index.tsapps/web/app/(use-page-wrapper)/insights/call-history/page.tsxpackages/features/ee/workflows/pages/call-history.tsxpackages/features/calAIPhone/providers/retellAI/RetellAIService.tspackages/trpc/server/routers/viewer/aiVoiceAgent/_router.tspackages/features/calAIPhone/providers/retellAI/RetellSDKClient.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/shell/navigation/Navigation.tsxpackages/features/ee/workflows/components/CallDetailsSheet.tsxapps/web/modules/insights/insights-call-history-view.tsxapps/web/app/(use-page-wrapper)/insights/call-history/page.tsxpackages/features/ee/workflows/pages/call-history.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/CallService.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.ts
🧠 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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.schema.tspackages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.tspackages/trpc/server/routers/viewer/aiVoiceAgent/_router.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/lib/server/repository/calAiPhoneNumber.tspackages/features/calAIPhone/providers/retellAI/services/CallService.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/providers/retellAI/index.tspackages/features/calAIPhone/index.ts
📚 Learning: 2025-08-05T13:17:23.491Z
Learnt from: Udit-takkar
PR: calcom/cal.com#21827
File: packages/lib/server/repository/phoneNumber.ts:153-160
Timestamp: 2025-08-05T13:17:23.491Z
Learning: In the Cal.com Cal AI phone feature, the deletePhoneNumber repository method validation is properly handled in the service layer (RetellAIService.deletePhoneNumber) which validates user ownership and authorization before calling the repository method. The repository layer correctly focuses on data access only.
Applied to files:
packages/lib/server/repository/calAiPhoneNumber.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/lib/server/repository/calAiPhoneNumber.tspackages/features/calAIPhone/interfaces/AIPhoneService.interface.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/providers/retellAI/index.tspackages/features/calAIPhone/index.tspackages/features/calAIPhone/providers/retellAI/RetellAIService.ts
📚 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/CallService.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/_router.ts
🧬 Code graph analysis (12)
packages/features/calAIPhone/providers/retellAI/types.ts (2)
packages/features/calAIPhone/index.ts (2)
RetellCallListParams(48-48)RetellCallListResponse(49-49)packages/features/calAIPhone/providers/retellAI/index.ts (2)
RetellCallListParams(66-66)RetellCallListResponse(67-67)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (4)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.schema.ts (1)
TListCallsInputSchema(15-15)packages/lib/server/repository/calAiPhoneNumber.ts (1)
CalAiPhoneNumberRepository(4-55)packages/features/calAIPhone/index.ts (1)
createDefaultAIPhoneServiceProvider(22-22)apps/api/v2/src/lib/logger.bridge.ts (1)
error(77-79)
packages/lib/server/repository/membership.ts (2)
packages/platform/libraries/index.ts (1)
MembershipRole(34-34)packages/prisma/index.ts (1)
prisma(56-61)
packages/features/ee/workflows/components/types.ts (1)
packages/trpc/react/trpc.ts (1)
RouterOutputs(143-143)
packages/features/calAIPhone/providers/retellAI/services/CallService.ts (4)
packages/features/calAIPhone/index.ts (1)
RetellCallListResponse(49-49)packages/features/calAIPhone/providers/retellAI/index.ts (1)
RetellCallListResponse(67-67)packages/features/calAIPhone/providers/retellAI/types.ts (1)
RetellCallListResponse(14-14)apps/api/v2/src/lib/logger.bridge.ts (1)
error(77-79)
packages/features/ee/workflows/components/CallDetailsSheet.tsx (1)
packages/features/ee/workflows/components/types.ts (2)
CallDetailsState(10-12)CallDetailsAction(14-21)
apps/web/modules/insights/insights-call-history-view.tsx (7)
packages/trpc/react/trpc.ts (2)
RouterOutputs(143-143)trpc(54-138)packages/features/ee/workflows/components/types.ts (2)
CallDetailsState(10-12)CallDetailsAction(14-21)packages/features/data-table/DataTableProvider.tsx (1)
DataTableProvider(93-426)packages/features/data-table/hooks/useSegments.ts (1)
useSegments(9-66)packages/ui/components/test-setup.tsx (1)
useOrgBranding(37-39)packages/lib/constants.ts (1)
WEBAPP_URL(12-18)packages/features/ee/workflows/components/CallDetailsSheet.tsx (1)
CallDetailsSheet(23-182)
packages/features/calAIPhone/providers/retellAI/index.ts (2)
packages/features/calAIPhone/index.ts (2)
RetellCallListParams(48-48)RetellCallListResponse(49-49)packages/features/calAIPhone/providers/retellAI/types.ts (2)
RetellCallListParams(13-13)RetellCallListResponse(14-14)
apps/web/app/(use-page-wrapper)/insights/call-history/page.tsx (3)
apps/web/app/_utils.tsx (1)
_generateMetadata(53-81)apps/web/app/(use-page-wrapper)/insights/checkInsightsPagePermission.ts (1)
checkInsightsPagePermission(11-34)apps/web/modules/insights/insights-call-history-view.tsx (1)
InsightsCallHistoryPage(316-320)
packages/features/ee/workflows/pages/call-history.tsx (8)
apps/web/modules/insights/insights-call-history-view.tsx (1)
CallHistoryProps(40-42)packages/trpc/react/trpc.ts (2)
RouterOutputs(143-143)trpc(54-138)packages/features/ee/workflows/components/types.ts (2)
CallDetailsState(10-12)CallDetailsAction(14-21)packages/features/data-table/DataTableProvider.tsx (1)
DataTableProvider(93-426)packages/features/data-table/hooks/useSegments.ts (1)
useSegments(9-66)packages/ui/components/test-setup.tsx (1)
useOrgBranding(37-39)packages/lib/constants.ts (1)
WEBAPP_URL(12-18)packages/features/ee/workflows/components/CallDetailsSheet.tsx (1)
CallDetailsSheet(23-182)
packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts (2)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.schema.ts (1)
ZListCallsInputSchema(3-13)packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (1)
listCallsHandler(19-87)
packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts (2)
packages/features/calAIPhone/providers/retellAI/types.ts (2)
RetellCallListParams(13-13)RetellCallListResponse(14-14)apps/api/v2/src/lib/logger.bridge.ts (1)
error(77-79)
⏰ 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). (10)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (17)
apps/web/public/static/locales/en/common.json (1)
2469-2477: No duplicate keys found — original comment is incorrectSearch of apps/web/public/static/locales/en/common.json shows each referenced call-history key appears exactly once (lines 2469–2477 and 3639–3678).
Likely an incorrect or invalid review comment.
packages/features/calAIPhone/index.ts (1)
48-50: LGTM: surfaced provider list-calls typesType re-exports are consistent with existing patterns and keep consumers decoupled from provider internals.
packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts (1)
17-19: LGTM: SDK passthrough with useful loggingMethod wiring to
this.client.call.listlooks good and logs avoid PII. Ensure the underlying SDK response is an array (current code logsresponse.length).If Retell changes the response shape (e.g., to
{ calls, totalCount }), update the log line accordingly.Also applies to: 241-259
packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts (2)
68-75: Good wiring for listCalls endpoint.Dynamic import + authedProcedure pattern matches the rest of the router.
68-75: Verify we don't return provider secrets from listCallsrg output shows no direct
credential.keymatches but does show provider API-key usage and provider responses that includeaccess_token. Confirm that packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (and any provider methods it calls) do NOT forward: provider API keys, cal_api_key, credential.* fields, or long‑lived tokens to clients. Returning an ephemeral call access_token for a web‑call flow is OK only if intended and short‑lived. Check these files as starting points:
- packages/features/calAIPhone/AIPhoneServiceRegistry.ts (resolveApiKey / provider config)
- packages/features/calAIPhone/index.ts (example apiKey)
- packages/features/calAIPhone/providers/retellAI/types.ts (createWebCall => access_token)
- packages/features/calAIPhone/providers/retellAI/index.ts
- packages/features/calAIPhone/providers/retellAI/RetellSDKClient.ts
- packages/features/calAIPhone/providers/retellAI/services/AgentService.ts (cal_api_key usage)
packages/features/calAIPhone/providers/retellAI/types.ts (1)
12-15: Type aliases for Retell list calls look correct.Clear, minimal surface tying to upstream SDK types.
packages/features/calAIPhone/providers/retellAI/index.ts (1)
22-24: Exports and provider type map extension are consistent.Public types re-exported and TypeMap extended appropriately.
Also applies to: 66-86
packages/features/ee/workflows/components/types.ts (1)
1-21: UI types derived from TRPC outputs look good.The
CallDatainference keeps frontend tightly bound to backend shape.packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (1)
76-79: totalCount likely incorrect for pagination.
totalCount: calls.lengthreturns the page size, not the dataset size. If the provider returns a total/has_more/next_offset, surface that instead; otherwise rename topageCount.Provide the provider response shape back to this handler or compute a separate
totalCount.apps/web/app/(use-page-wrapper)/insights/call-history/page.tsx (1)
16-20: LGTM: permission gate before render.Server-side permission check runs before rendering the client view. Looks good.
apps/web/modules/insights/insights-call-history-view.tsx (1)
276-297: Resolve — prop name is correct: onRowMouseclickDataTableWrapper and DataTable define and forward onRowMouseclick, so the handler will be invoked.
packages/features/calAIPhone/providers/retellAI/services/CallService.ts (1)
10-10: Confirm return type shape.You import RetellCallListResponse and later return an array with extra field sessionOutcome. Ensure the alias actually represents an array of calls; otherwise responses will be structurally inconsistent across layers.
packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (2)
92-99: Type surface additions look good.Generic aliases for ListCalls params/response are clear and keep the interface provider-agnostic.
366-375: Align org/team context naming across API.Other methods use teamId; listCalls introduces organizationId. Confirm which is canonical to avoid consumer confusion.
packages/lib/server/repository/calAiPhoneNumber.ts (2)
6-9: Good: minimal Prisma selects (no include).Only phoneNumber is selected per our guideline. This keeps responses lean and avoids sensitive fields.
Also applies to: 13-19, 22-25
12-26: Indexes already present in Prisma schema — no action required.
CalAiPhoneNumber has @@index([userId]) and @@index([teamId]) and Team has @@index([parentId]) in packages/prisma/schema.prisma.packages/features/ee/workflows/pages/call-history.tsx (1)
320-326: Default export for a page is fine.As this module represents a page component, keeping a default export aligns with our guideline exemptions.
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/features/shell/navigation/Navigation.tsx (1)
125-130: Localization key looks correct; resolves prior feedbackUsing the "call_history" i18n key is consistent with the rest of the menu and addresses the earlier comment.
🧹 Nitpick comments (2)
packages/features/shell/navigation/Navigation.tsx (1)
125-130: Optional: surface a phone icon for consistency with other entriesUncommenting the icon improves visual scan and aligns with other nav items.
{ name: "call_history", href: "/insights/call-history", - // icon: "phone", + icon: "phone", isCurrent: ({ pathname: path }) => path?.startsWith("/insights/call-history") ?? false, },packages/features/calAIPhone/providers/retellAI/RetellAIService.ts (1)
245-256: Add explicit Retell types to listCalls; skip local page-cap (CallService already defaults)RetellCallListParams/RetellCallListResponse are exported from ./types — import and update the method signature. CallService already applies limit = 50, so no additional capping in RetellAIService is necessary. TRPC checks returned no credential.key or Prisma include leaks.
- async listCalls(params: { - limit?: number; - offset?: number; - filters?: { - fromNumber: string[]; - toNumber?: string[]; - startTimestamp?: { lower_threshold?: number; upper_threshold?: number }; - }; - }) { - return this.callService.listCalls(params); - } + async listCalls(params: RetellCallListParams): Promise<RetellCallListResponse> { + return this.callService.listCalls(params); + }Add import near existing ./types imports:
import type { RetellCallListParams, RetellCallListResponse } from "./types";
📜 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 (8)
apps/web/modules/insights/insights-call-history-view.tsx(1 hunks)packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts(2 hunks)packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/RetellAIService.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/CallService.ts(2 hunks)packages/features/shell/navigation/Navigation.tsx(1 hunks)packages/lib/server/repository/calAiPhoneNumber.ts(1 hunks)packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts
- packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts
- packages/features/calAIPhone/providers/retellAI/services/CallService.ts
- packages/lib/server/repository/calAiPhoneNumber.ts
- apps/web/modules/insights/insights-call-history-view.tsx
- packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts
🧰 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.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.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/features/shell/navigation/Navigation.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/RetellAIService.tspackages/features/shell/navigation/Navigation.tsx
**/*.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/shell/navigation/Navigation.tsx
🧠 Learnings (3)
📚 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.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/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.ts
⏰ 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
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (2)
31-37: Guard org-scoped filters when organizationId is undefinedPrevent teams with
parentId === undefinedfrom slipping in when there’s no org context.- const adminTeamIds = userMemberships - .filter((m) => m.team?.parentId === organizationId) - .map((m) => m.teamId); + const adminTeamIds = organizationId + ? userMemberships + .filter((m) => m.team?.parentId === organizationId) + .map((m) => m.teamId) + : []; - const isOrgOwner = userMemberships.some( - (m) => m.role === MembershipRole.OWNER && m.team?.parentId === organizationId - ); + const isOrgOwner = + !!organizationId && + userMemberships.some( + (m) => m.role === MembershipRole.OWNER && m.team?.parentId === organizationId + );
83-91: Fix truthiness check; handle epoch 0 correctlyUsing truthiness skips validation when a bound is exactly 0. Compare against
undefined.- if ( - startTimestamp.lower_threshold && - startTimestamp.upper_threshold && - startTimestamp.lower_threshold > startTimestamp.upper_threshold - ) { + if ( + startTimestamp.lower_threshold !== undefined && + startTimestamp.upper_threshold !== undefined && + startTimestamp.lower_threshold > startTimestamp.upper_threshold + ) { throw new TRPCError({ code: "BAD_REQUEST", message: "startDate must be before or equal to endDate", }); }
🧹 Nitpick comments (2)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (2)
61-81: Prefer Number.isNaN over global isFinite for Date.parse validation
Date.parsereturns a number orNaN;Number.isNaNis the precise check and avoids global-coercion semantics.- if (!isFinite(parsedStartDate)) { + if (Number.isNaN(parsedStartDate)) { throw new TRPCError({ code: "BAD_REQUEST", message: "Invalid startDate format", }); } ... - if (!isFinite(parsedEndDate)) { + if (Number.isNaN(parsedEndDate)) { throw new TRPCError({ code: "BAD_REQUEST", message: "Invalid endDate format", }); }
104-107: Sanitize outbound payload; never return secretsBefore returning, ensure the payload cannot contain
credential.keyor similar secret fields from downstream providers. Prefer an allowlist projection of fields needed by the UI.
📜 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/modules/insights/insights-call-history-view.tsx(1 hunks)packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/modules/insights/insights-call-history-view.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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts
🧠 Learnings (2)
📚 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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.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/listCalls.handler.ts
🧬 Code graph analysis (1)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (3)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.schema.ts (1)
TListCallsInputSchema(15-15)packages/lib/server/repository/calAiPhoneNumber.ts (1)
CalAiPhoneNumberRepository(3-54)packages/features/calAIPhone/index.ts (1)
createDefaultAIPhoneServiceProvider(22-22)
🔇 Additional comments (2)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (2)
104-107: Confirm provider totalCount; avoid using page length as totalcalls.length is the page size, not the dataset total. Use the provider's returned total (e.g., response.totalCount or response.meta.total) and return that instead of calls.length.
Location: packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (lines 104–107). Repo search found no listCalls/ListCalls signature — confirm the provider method and its return shape and update this handler to pass through provider.totalCount.
95-102: Enforce sane bounds for limit/offsetCap limit (max 100) and ensure offset is an integer >= 0 in the Zod input schema (reject negatives or coerce to 0).
Location: packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (lines 95–102) — update ZListCallsInputSchema or add router-level validation.
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (4)
20-20: Fix unsafe access to profiles array.
ctx.user.profiles[0]can throw whenprofilesis undefined. Use optional chaining on the index.- const organizationId = ctx.user.organizationId ?? ctx.user.profiles[0]?.organizationId; + const organizationId = ctx.user.organizationId ?? ctx.user.profiles?.[0]?.organizationId;
31-33: Guard org-scoped adminTeamIds when organizationId is undefined.Current logic includes all admin team IDs when no org context, potentially widening phone-number access beyond the intended org. Short-circuit to an empty array when
organizationIdis falsy.- const adminTeamIds = userMemberships - .filter((m) => !organizationId || m.team?.parentId === organizationId) - .map((m) => m.teamId); + const adminTeamIds = organizationId + ? userMemberships + .filter((m) => m.team?.parentId === organizationId) + .map((m) => m.teamId) + : [];
107-113: Preserve TRPCError status; don’t mask BAD_REQUEST with 500.Re-throw known TRPCError and only wrap unknowns.
- } catch (error) { - logger.error(`Failed to list calls for user ${ctx.user.id}:`, error); - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: "Failed to retrieve call history", - }); - } + } catch (error) { + if (error instanceof TRPCError) { + throw error; + } + logger.error(`Failed to list calls for user ${ctx.user.id}:`, error); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Failed to retrieve call history", + }); + }
82-91: Fix truthiness check for date bounds.Using truthiness skips the comparison when a timestamp is 0 (Unix epoch). Compare against
undefinedexplicitly.- if ( - startTimestamp.lower_threshold && - startTimestamp.upper_threshold && - startTimestamp.lower_threshold > startTimestamp.upper_threshold - ) { + if ( + startTimestamp.lower_threshold !== undefined && + startTimestamp.upper_threshold !== undefined && + startTimestamp.lower_threshold > startTimestamp.upper_threshold + ) {
🧹 Nitpick comments (3)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (3)
54-54: Consider reusing/injecting the AI service provider.If creation is non-trivial, initialize once at module scope or inject for easier testing and lower overhead.
103-106: totalCount should reflect the full result set if available.If the provider returns a server-side total, surface it; otherwise document that
totalCountequals the current page length.
95-96: Schema already enforces limit/offset; no handler clamp required.ZListCallsInputSchema (packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.schema.ts) defines limit: z.number().min(1).max(1000).default(50) and offset: z.number().min(0).default(0), so input.limit/input.offset are validated. Optional: add .int() and/or reduce max to 100 to tighten page-size caps.
📜 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 (1)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts
🧠 Learnings (3)
📚 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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.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/listCalls.handler.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/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts
🧬 Code graph analysis (1)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (4)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.schema.ts (1)
TListCallsInputSchema(15-15)packages/lib/server/repository/membership.ts (1)
MembershipRepository(84-532)packages/lib/server/repository/calAiPhoneNumber.ts (1)
CalAiPhoneNumberRepository(3-54)packages/features/calAIPhone/index.ts (1)
createDefaultAIPhoneServiceProvider(22-22)
⏰ 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: Linters / lint
🔇 Additional comments (3)
packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts (3)
35-37: Owner check is correctly scoped.
isOrgOwneronly evaluates truthy whenorganizationIdis defined and matches the membership’s parent. Looks good.
46-52: Good early return when no accessible numbers.Avoids an unnecessary provider call and keeps behavior explicit.
94-101: Confirm provider timestamp units; camelCase keys are already correct.
- Confirmed: the interface and provider wrapper expect filters.fromNumber (string[]) and filters.startTimestamp { lower_threshold?: number; upper_threshold?: number } — see packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts and packages/features/calAIPhone/providers/retellAI/services/CallService.ts (which maps to filter_criteria.from_number / start_timestamp).
- Observed: listCalls.handler sets startTimestamp using Date.parse(...) (milliseconds) and passes it straight through (packages/trpc/server/routers/viewer/aiVoiceAgent/listCalls.handler.ts).
- Action: verify whether the external Retell SDK/API expects start_timestamp in seconds or milliseconds; if it expects seconds, convert ms -> s (Math.floor(ms / 1000)) before calling aiService.listCalls.
E2E results are ready! |
| roles?: MembershipRole[]; | ||
| }; | ||
| }) { | ||
| return prisma.membership.findMany({ |
There was a problem hiding this comment.
This needs a follow-up PR to add integration tests to ensure its functionality.
| .filter((m) => !organizationId || m.team?.parentId === organizationId) | ||
| .map((m) => m.teamId); | ||
|
|
||
| const isOrgOwner = organizationId |
There was a problem hiding this comment.
This needs a follow up. This sort of logic should not live in this code. It shouldn't know how to calculate if this person if an org owner or not. That belongs in orgs code.
keithwillcode
left a comment
keithwillcode
left a comment
There was a problem hiding this comment.
Approving since we have a tight timeframe but added a couple of follow-ups that need to happen ASAP.
2 participants
What does this PR do?
Clicking on an item opens sheet with transcript, recording etc.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?