Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@elie222 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 5 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis pull request introduces broad changes across the application. New guidelines for utility functions are added, and architectural sections are reorganized to highlight AI personal assistant, reply tracking, and cold email blocking features. Updates include modifications to test cases, UI components, and form handling; the legacy editor is replaced by a new Tiptap-based implementation. Several files related to legacy slash commands and formatting options have been removed. Significant new functionality for reply tracking has been implemented with accompanying API routes, migrations, and utilities. Various hooks, package dependencies, and Prisma schema models have also been updated. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as EnableFeatureCard
participant API as Reply Tracker API
participant DB as Database
participant UI2 as UI (Toast/Refresh)
U->>UI: Click "Enable Reply Tracker"
UI->>API: Call enableReplyTrackerAction()
API->>DB: Update/Create rule (trackReplies enabled)
DB-->>API: Confirmation response
API->>UI: Return success status
UI->>UI2: Display success toast and update view
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (41)
apps/web/package.json (1)
70-72: New Tiptap Dependencies IntegrationThe additions of the Tiptap libraries (
@tiptap/pm,@tiptap/react, and@tiptap/starter-kitat version2.11.5) are aligned with the removal of the legacy editor and the move to a Tiptap-based implementation. Please ensure that these dependencies are consistently integrated across all rich-text editing components and that their versions are compatible with the rest of the codebase.apps/web/app/(app)/stats/LargestEmails.tsx (1)
43-43: Fix typo in comment."calculte" should be "calculate"
- // Function to calculte total attachement size + // Function to calculate total attachment sizeapps/web/utils/date.ts (1)
43-50: Consider adding input validation and explicit type conversion.The function could benefit from more robust input handling and explicit type conversion.
export function internalDateToDate(internalDate?: string | null): Date { if (!internalDate) return new Date(); - const date = new Date(+internalDate); + const timestamp = Number(internalDate); + if (Number.isNaN(timestamp)) return new Date(); + + const date = new Date(timestamp); if (Number.isNaN(date.getTime())) return new Date(); return date; }apps/web/components/email-list/EmailPanel.tsx (3)
169-178: Consider extracting the scroll delay as a constant.The scroll delay of 300ms is hardcoded. Consider extracting it to a named constant for better maintainability.
+const SCROLL_DELAY_MS = 300; + useEffect(() => { if (defaultShowReply && replyRef.current) { setTimeout(() => { replyRef.current?.scrollIntoView({ behavior: "smooth" }); - }, 300); + }, SCROLL_DELAY_MS); } }, [defaultShowReply]);
189-219: Remove debug console.log statements.Debug console.log statements should be removed before merging to production.
- console.log( - "🚀 ~ prepareReplyingToEmail ~ sentFromUser:", - sentFromUser, - normalizedFrom, - normalizedUserEmail, - );Consider simplifying the email normalization logic.
The email normalization logic could be simplified by combining the operations.
- const normalizedFrom = normalizeEmailAddress( - extractEmailAddress(message.headers.from), - ); - const normalizedUserEmail = normalizeEmailAddress(userEmail); + const normalizedFrom = normalizeEmailAddress(extractEmailAddress(message.headers.from)); + const normalizedUserEmail = normalizeEmailAddress(userEmail);
322-354: Consider using more specific types for the event handler.The event type in the onLoad handler could be more specific using HTMLIFrameElement.
- (event: SyntheticEvent<HTMLIFrameElement, Event>) => { + (event: SyntheticEvent<HTMLIFrameElement, HTMLIFrameElement['onload']>) => {apps/web/app/(app)/compose/ComposeEmailForm.tsx (3)
61-66: Consider defaulting to replyingToEmail’s message contents.
Currently,messageHtmlandmessageTextare both initialized as empty strings. If you want the editor’s initial text to already show the quoted message (for example), consider integratingreplyingToEmail?.messageTextandreplyingToEmail?.messageHtmldirectly into these default values.defaultValues: { replyToEmail: replyingToEmail, subject: replyingToEmail?.subject, to: replyingToEmail?.to, cc: replyingToEmail?.cc, - messageHtml: "", - messageText: "", + messageHtml: replyingToEmail?.messageHtml ?? "", + messageText: replyingToEmail?.messageText ?? "", }
80-81: Leverage error response content for a more informative toast.
Currently, the code toasts a generic error message on anyisActionError. You might make use of any specificres.errororres.messagefor more detailed feedback.if (isActionError(res)) - toastError({ description: "There was an error sending the email :(" }); + toastError({ + description: res.message || "There was an error sending the email :(" + });
136-143: HTML stripping approach is simplistic.
Using a regex<[^>]*>is workable but may lose spacing, newlines, or block-level formatting. Consider a richer HTML-to-text conversion if formatting is critical.I can provide a helper function or library recommendation (e.g., “html-to-text”) if you want a more robust plain-text extraction.
apps/web/components/Tiptap.tsx (1)
9-46: Solid Tiptap setup.
- Using
StarterKitis a great starting point.- The
autofocusprop & default content are well handled.- The
onUpdatecallback properly invokes the parent’sonChangeto retrieve HTML.- Consider adding other Tiptap extensions for links, images, mentions, or custom markdown if needed.
apps/web/hooks/useDisplayedEmail.ts (1)
32-33: Consider documenting the return type.The hook's return type has been expanded but lacks explicit TypeScript type definitions. Consider adding an interface to document the shape of the returned object.
+interface DisplayedEmailHook { + threadId: string | null; + messageId: string | null; + showEmail: (options: { + threadId: string; + messageId?: string; + showReplyButton?: boolean; + autoOpenReplyForMessageId?: string; + } | null) => void; + showReplyButton: boolean; + autoOpenReplyForMessageId: string | null; +} -export const useDisplayedEmail = () => { +export const useDisplayedEmail = (): DisplayedEmailHook => {apps/web/components/EmailViewer.tsx (1)
41-51: Consider extracting interface for EmailContent props.The inline prop types could be extracted to a reusable interface, especially since these props might be used in other components.
+interface EmailContentProps { + threadId: string; + showReplyButton: boolean; + autoOpenReplyForMessageId?: string; + userEmail: string; +} -function EmailContent({ - threadId, - showReplyButton, - autoOpenReplyForMessageId, - userEmail, -}: { - threadId: string; - showReplyButton: boolean; - autoOpenReplyForMessageId?: string; - userEmail: string; -}) +function EmailContent({ + threadId, + showReplyButton, + autoOpenReplyForMessageId, + userEmail, +}: EmailContentProps)apps/web/utils/reply-tracker/outbound.ts (3)
65-65: Use Consistent Logging Instead ofconsole.logOther parts of this file use a scoped logger (e.g.,
logger.infoorlogger.error). For consistency and better log management, consider usinglogger.info("No need to reply")rather thanconsole.log("No need to reply").- console.log("No need to reply"); + logger.info("No need to reply");
69-111: Handle Partial Failures increateReplyTrackerOutboundUsing
Promise.allSettledwill not short-circuit on an error, which is often desirable. However, the code does not log or surface partial failures (e.g., if the upsert fails but the label creation succeeds). Logging or retrying failed operations could improve reliability and debuggability.
113-138: Add Logging or Recovery for Partial Failure inresolveReplyTrackersSimilar to
createReplyTrackerOutbound, this function silently absorbs errors due toPromise.allSettled. Consider logging or taking corrective action (e.g., retrying, alerting the user) if one of the promises fails, to maintain better traceability of errors.apps/web/utils/actions/reply-tracking.ts (2)
16-156: Guard Againstuser.rulesPromptBeing Null or UndefinedIn lines 129-137, the code appends text to
user.rulesPromptbut does not explicitly handle ifuser.rulesPromptisnullorundefined. This could lead to unintended string concatenation results. Consider a safe default or a null check before concatenation.- rulesPrompt: `${user.rulesPrompt}\n\n* Label ... + rulesPrompt: `${user.rulesPrompt || ""}\n\n* Label ...
165-188: Add Optional Validation or Feedback for Resolved Threads
resolveThreadTrackerActionupdates theresolvedstatus of a thread but has no checks to confirm the thread exists or was previously unresolved. For clarity, you might want to return the number of records updated or handle the scenario in which the thread is already marked resolved.apps/web/app/api/resend/summary/route.ts (2)
61-106: Consider Distinct BythreadIdin a Single QueryYou already note the potential mismatch if the same thread appears multiple times. To avoid manual grouping and multiple queries for different thread types (
NEEDS_REPLY,AWAITING,NEEDS_ACTION), you could consider a single query with conditional grouping or a more direct approach to collecting unique threads.
161-167: Threshold Check inshouldSendEmailThis condition triggers emails as soon as any of the tracked items exists (cold emailers, pending rules, or thread tracker items). If there's a risk of sending too frequently, consider introducing a threshold to limit the volume of summary emails, especially for large organizations or very active email threads.
packages/resend/emails/summary.tsx (2)
370-371: Use a standardized date/time format to avoid inconsistenciesRelying on the environment’s default locale with
toLocaleDateString()can lead to inconsistent date displays across different regions. Consider specifying a locale or using a library (e.g., date-fns) for a predictable format.
394-394: Ensure uniqueness of keys when mapping over emailsUsing
email.from + email.subjectas a key might lead to collisions when two messages share the same sender and subject. Consider using an internal unique ID if available, or a more robust concatenation including thesentAttimestamp.apps/web/hooks/useThreadsByIds.ts (1)
4-13: Consider adding error handling or fallback dataCurrently, there’s no explicit handling for failed requests or partial data. You might add error boundaries or a fallback to ensure the UI remains stable if the SWR request fails or returns incomplete data.
apps/web/app/(app)/reply-tracker/date-filter.ts (1)
7-21: Clarify date range logic for "1m" caseUsing
now.setMonth(now.getMonth() - 1)might skip entire days depending on the current date (e.g., from March 31 to February’s last day). Consider subtracting a fixed number of days if a strict 30-day window is desired, or document the behavior to avoid confusion.apps/web/app/(app)/reply-tracker/AwaitingReply.tsx (1)
1-32: Consider extracting common logic to reduce duplication.The implementation is correct but shares significant code with
NeedsReply.tsxandNeedsAction.tsx. Consider extracting the common logic into a reusable component or hook.Example refactor:
// apps/web/app/(app)/reply-tracker/components/TrackerView.tsx import { ThreadTrackerType } from "@prisma/client"; import { ReplyTrackerEmails } from "@/app/(app)/reply-tracker/ReplyTrackerEmails"; import { getPaginatedThreadTrackers } from "@/app/(app)/reply-tracker/fetch-trackers"; import type { TimeRange } from "@/app/(app)/reply-tracker/date-filter"; interface TrackerViewProps { userId: string; userEmail: string; page: number; timeRange: TimeRange; type: ThreadTrackerType; } export async function TrackerView({ userId, userEmail, page, timeRange, type, }: TrackerViewProps) { const { trackers, totalPages } = await getPaginatedThreadTrackers({ userId, type, page, timeRange, }); return ( <ReplyTrackerEmails trackers={trackers} userEmail={userEmail} type={type} totalPages={totalPages} /> ); }Then use it in each view:
// apps/web/app/(app)/reply-tracker/AwaitingReply.tsx export async function AwaitingReply(props: Omit<TrackerViewProps, 'type'>) { return <TrackerView {...props} type={ThreadTrackerType.AWAITING} />; }apps/web/hooks/useFeatureFlags.ts (1)
6-8: Consider centralizing feature flag names.Feature flag names are currently hardcoded across different hooks. Consider centralizing them in a constants file to improve maintainability.
Example:
// apps/web/constants/feature-flags.ts export const FEATURE_FLAGS = { REPLY_TRACKER: 'reply-tracker', HERO: 'hero-copy-5', LANDING_PAGE_AI_ASSISTANT: 'landing-page-ai-assistant', PRICING_OPTIONS: 'pricing-options', } as const; // Then use in hooks: export function useReplyTrackingEnabled() { return useFeatureFlagEnabled(FEATURE_FLAGS.REPLY_TRACKER); }apps/web/app/(app)/reply-tracker/EnableReplyTracker.tsx (1)
4-5: Consolidate duplicate imports.The
Toastimports can be combined into a single line.-import { toastSuccess } from "@/components/Toast"; -import { toastError } from "@/components/Toast"; +import { toastSuccess, toastError } from "@/components/Toast";apps/web/utils/ai/reply/check-reply-tracking.ts (1)
13-49: LGTM! Well-implemented AI utility with proper validation and logging.The function is well-structured with:
- Strong typing using TypeScript
- Input validation using Zod
- Comprehensive logging for debugging
- Clear XML-like structure for AI prompt
Consider adding JSDoc comments to document the function's purpose and parameters.
+/** + * Finds a rule designed to track email replies using AI. + * @param rules - Array of rules to search through + * @param user - User information for AI context + * @returns Promise resolving to an object with nullable replyTrackingRuleId + */ export async function aiFindReplyTrackingRule({apps/web/utils/ai/reply/check-if-needs-reply.ts (1)
13-47: LGTM! Well-implemented AI utility with proper validation and logging.The function is well-structured with:
- Strong typing using TypeScript
- Input validation using Zod
- Message history limit of 5 messages
- Comprehensive logging for debugging
Consider adding JSDoc comments and extracting the magic number 5 into a named constant.
+const MAX_MESSAGES_TO_CHECK = 5; + +/** + * Checks if a reply is needed for a thread using AI. + * @param user - User information including background for AI context + * @param messages - Array of messages to analyze + * @returns Promise resolving to an object with needsReply boolean + */ export async function aiCheckIfNeedsReply({ user, messages, }: { user: Pick<User, "id" | "about"> & UserEmailWithAI; messages: EmailForLLM[]; }) { // ... ${messages - .slice(-5) + .slice(-MAX_MESSAGES_TO_CHECK) .map((message) => `<message>${stringifyEmail(message, 500)}</message>`)apps/web/components/EnableFeatureCard.tsx (1)
19-53: LGTM! Well-structured reusable component.The component is well-implemented with:
- Proper use of Next.js Image component
- Responsive styling
- Clean conditional rendering
- Good use of semantic HTML elements
Consider adding prop validation to ensure either
hreforonEnableis provided, but not both.interface EnableFeatureCardProps { title: string; description: string; imageSrc: string; imageAlt: string; buttonText: string; - href?: string; - onEnable?: () => void; + href: string | undefined; + onEnable: (() => void) | undefined; } export function EnableFeatureCard({ title, description, imageSrc, imageAlt, buttonText, href, onEnable, }: EnableFeatureCardProps) { + if (href && onEnable) { + throw new Error("Cannot provide both href and onEnable props"); + } + if (!href && !onEnable) { + throw new Error("Must provide either href or onEnable prop"); + } return (apps/web/app/(app)/reply-tracker/TimeRangeFilter.tsx (1)
21-52: Consider type safety improvements and URL param key extraction.The implementation is clean but could be improved:
- Derive the
TimeRangetype fromtimeRangeOptionsfor better type safety.- Extract the URL param key "timeRange" to a constant.
Apply this diff to improve type safety and maintainability:
-import type { TimeRange } from "@/app/(app)/reply-tracker/date-filter"; +const TIME_RANGE_PARAM = "timeRange" as const; +type TimeRange = typeof timeRangeOptions[number]["value"]; export function TimeRangeFilter() { const router = useRouter(); const pathname = usePathname(); const searchParams = useSearchParams(); - const timeRange = (searchParams.get("timeRange") as TimeRange) || "all"; + const timeRange = (searchParams.get(TIME_RANGE_PARAM) as TimeRange) || "all"; const createQueryString = (value: TimeRange) => { const params = new URLSearchParams(searchParams); - params.set("timeRange", value); + params.set(TIME_RANGE_PARAM, value); return params.toString(); };apps/web/app/(app)/reply-tracker/fetch-trackers.ts (1)
10-60: LGTM! Consider caching the count query.The implementation is well-structured with proper error handling, parameter validation, and SQL injection prevention. The use of
Promise.allfor concurrent execution anddistinctfor preventing duplicates is a good practice.Consider caching the count query results for a short duration (e.g., 1 minute) to improve performance, especially for users with many threads. This would reduce database load when users navigate through pages quickly.
apps/web/app/api/google/threads/batch/route.ts (1)
34-58: Consider adding rate limiting.The implementation is well-structured with proper authentication, validation, and error handling. However, since this is a batch endpoint, it could benefit from rate limiting to prevent abuse.
Consider implementing rate limiting based on:
- Maximum number of requests per user per minute
- Maximum number of thread IDs per request
- Maximum concurrent requests per user
This will help protect the Gmail API quota and server resources.
apps/web/utils/ai/choose-rule/execute.ts (1)
66-68: Add logging for skipped label actions.Consider adding debug logging when skipping label actions to aid in troubleshooting.
// we handle the reply tracking labelling below instead -if (isReplyTrackingRule && action.type === ActionType.LABEL) continue; +if (isReplyTrackingRule && action.type === ActionType.LABEL) { + logger.debug("Skipping label action for reply tracking rule", { + userEmail, + executedRuleId: executedRule.id, + actionId: action.id, + }); + continue; +}apps/web/app/(app)/reply-tracker/page.tsx (2)
56-63: Remove commented out code.Remove the commented out "Needs Action" tab if it's not currently needed. If it's planned for future implementation, track it in an issue instead.
41-68: Enhance tab accessibility.Add ARIA labels to improve accessibility for screen readers.
- <TabsList> + <TabsList aria-label="Email tracking categories"> <TabsTrigger value="needsReply" className="flex items-center gap-2" + aria-label="Emails that need a reply" >apps/web/utils/ai/choose-rule/run-rules.ts (1)
48-48: Enhance logging for better debugging.Add more context to the trace log to make debugging easier.
- logger.trace("Matching rule", { result }); + logger.trace("Matching rule result", { + ruleId: result.rule?.id, + ruleName: result.rule?.name, + matched: !!result.rule, + reason: result.reason, + messageId: message.id, + threadId: message.threadId + });apps/web/utils/assistant/process-assistant-email.ts (1)
76-84: Consider enhancing error messages for better debugging.While the error handling is good, the error messages could be more specific to help with debugging:
- "No thread messages found" could include the thread ID
- "No first message to assistant found" could include the thread ID and total message count
- logger.error("No thread messages found", loggerOptions); + logger.error("No thread messages found in thread", { ...loggerOptions, threadId: message.threadId }); await replyToEmail( gmail, message, - "Something went wrong. I couldn't read any messages.", + "Something went wrong. I couldn't read any messages in this thread.", ); - logger.error("No first message to assistant found", { - messageId: message.id, + logger.error("No first message to assistant found in thread", { + messageId: message.id, + threadId: message.threadId, + messageCount: threadMessages.length, });Also applies to: 93-103
packages/resend/package.json (1)
10-10: Consider using caret version range for consistency.Other dependencies in the file use caret ranges (^). Consider updating this line to use a caret range for consistency:
- "@react-email/render": "1.0.4", + "@react-email/render": "^1.0.4",ARCHITECTURE.md (3)
164-173: AI Personal Assistant Section – Clarify and Enhance Style.
The "AI Personal Assistant" section provides a thorough explanation of how prompt files are converted into database rules and highlights the inherent challenges of a two-way sync system. For improved clarity and consistency, consider updating:
- "two way sync system" (line 167) to "two-way sync system".
- "prompt based" (line 170) to "prompt-based".
Also, adding a comma before "however" in line 170 could improve readability.🧰 Tools
🪛 LanguageTool
[misspelling] ~169-~169: This word is normally spelled with a hyphen.
Context: ...ules and not the prompt file. We have a two way sync system between the db rules and th...(EN_COMPOUNDS_TWO_WAY)
174-178: Reply Tracking Section – Refine Wording.
The reply tracking description successfully communicates its integration with the AI personal assistant. To further streamline the text, consider:
- Replacing "built off of" (line 175) with "based on" or "that builds upon" for conciseness.
- Reworking line 178 to improve clarity. For example:
"This means each user has their own reply tracking prompt, which complicates global updates compared to the cold email blocker prompt."
🧰 Tools
🪛 LanguageTool
[uncategorized] ~174-~174: The adjective “prompt-based” is spelled with a hyphen.
Context: ... often each is called. If it were fully prompt based this wouldn't be possible. This is a po...(BASED_HYPHEN)
[typographical] ~174-~174: Consider adding a comma before the interrupter.
Context: ... a potentially minor benefit to the user however. - Because actions are static (unless u...(HOWEVER_COMMA)
179-181: Cold Email Blocker Section – Streamline Description.
The section on the cold email blocker is clear but could benefit from sentence restructuring for better readability. For example, consider:
- Splitting line 180 into two sentences:
"The cold email blocker monitors incoming emails. If the user has never emailed us before, each email is processed through an LLM to determine if it is a cold email."
This minor change improves clarity and formality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (68)
.cursorrules(1 hunks)ARCHITECTURE.md(1 hunks)apps/web/__tests__/ai-choose-args.test.ts(1 hunks)apps/web/__tests__/ai-choose-rule.test.ts(0 hunks)apps/web/__tests__/ai-process-user-request.test.ts(1 hunks)apps/web/app/(app)/cold-email-blocker/ColdEmailList.tsx(2 hunks)apps/web/app/(app)/cold-email-blocker/ColdEmailPromptForm.tsx(1 hunks)apps/web/app/(app)/cold-email-blocker/ColdEmailSettings.tsx(1 hunks)apps/web/app/(app)/compose/ComposeEmailForm.tsx(7 hunks)apps/web/app/(app)/compose/SlashCommand.tsx(0 hunks)apps/web/app/(app)/compose/extensions.ts(0 hunks)apps/web/app/(app)/compose/selectors/ai-selector.tsx(0 hunks)apps/web/app/(app)/compose/selectors/color-selector.tsx(0 hunks)apps/web/app/(app)/compose/selectors/link-selector.tsx(0 hunks)apps/web/app/(app)/compose/selectors/node-selector.tsx(0 hunks)apps/web/app/(app)/compose/selectors/text-buttons.tsx(0 hunks)apps/web/app/(app)/reply-tracker/AwaitingReply.tsx(1 hunks)apps/web/app/(app)/reply-tracker/EnableReplyTracker.tsx(1 hunks)apps/web/app/(app)/reply-tracker/NeedsAction.tsx(1 hunks)apps/web/app/(app)/reply-tracker/NeedsReply.tsx(1 hunks)apps/web/app/(app)/reply-tracker/ReplyTrackerEmails.tsx(1 hunks)apps/web/app/(app)/reply-tracker/Resolved.tsx(1 hunks)apps/web/app/(app)/reply-tracker/TimeRangeFilter.tsx(1 hunks)apps/web/app/(app)/reply-tracker/date-filter.ts(1 hunks)apps/web/app/(app)/reply-tracker/fetch-trackers.ts(1 hunks)apps/web/app/(app)/reply-tracker/page.tsx(1 hunks)apps/web/app/(app)/stats/LargestEmails.tsx(2 hunks)apps/web/app/api/ai/summarise/route.ts(1 hunks)apps/web/app/api/google/threads/batch/route.ts(1 hunks)apps/web/app/api/google/webhook/process-history.ts(4 hunks)apps/web/app/api/resend/summary/route.ts(4 hunks)apps/web/components/EmailViewer.tsx(4 hunks)apps/web/components/EnableFeatureCard.tsx(1 hunks)apps/web/components/SideNav.tsx(5 hunks)apps/web/components/Tiptap.tsx(1 hunks)apps/web/components/Toggle.tsx(2 hunks)apps/web/components/email-list/EmailList.tsx(1 hunks)apps/web/components/email-list/EmailListItem.tsx(2 hunks)apps/web/components/email-list/EmailPanel.tsx(11 hunks)apps/web/hooks/useDisplayedEmail.ts(1 hunks)apps/web/hooks/useFeatureFlags.ts(1 hunks)apps/web/hooks/useThreadsByIds.ts(1 hunks)apps/web/package.json(1 hunks)apps/web/prisma/migrations/20250202092329_reply_tracker/migration.sql(1 hunks)apps/web/prisma/migrations/20250202154501_remove_deprecated_action/migration.sql(1 hunks)apps/web/prisma/migrations/20250203174037_reply_tracker_sent_at/migration.sql(1 hunks)apps/web/prisma/schema.prisma(4 hunks)apps/web/utils/actions/ai-rule.ts(2 hunks)apps/web/utils/actions/mail.ts(2 hunks)apps/web/utils/actions/reply-tracking.ts(1 hunks)apps/web/utils/ai/actions.ts(1 hunks)apps/web/utils/ai/choose-rule/execute.ts(4 hunks)apps/web/utils/ai/choose-rule/run-rules.ts(2 hunks)apps/web/utils/ai/reply/check-if-needs-reply.ts(1 hunks)apps/web/utils/ai/reply/check-reply-tracking.ts(1 hunks)apps/web/utils/assistant/process-assistant-email.ts(2 hunks)apps/web/utils/date.ts(2 hunks)apps/web/utils/email.ts(1 hunks)apps/web/utils/gmail/label.ts(2 hunks)apps/web/utils/gmail/thread.ts(2 hunks)apps/web/utils/redis/label.ts(1 hunks)apps/web/utils/reply-tracker/inbound.ts(1 hunks)apps/web/utils/reply-tracker/label.ts(1 hunks)apps/web/utils/reply-tracker/outbound.ts(1 hunks)apps/web/utils/rule/rule.ts(1 hunks)packages/resend/emails/summary.tsx(2 hunks)packages/resend/package.json(1 hunks)packages/resend/src/send.tsx(2 hunks)
💤 Files with no reviewable changes (8)
- apps/web/tests/ai-choose-rule.test.ts
- apps/web/app/(app)/compose/selectors/text-buttons.tsx
- apps/web/app/(app)/compose/extensions.ts
- apps/web/app/(app)/compose/selectors/ai-selector.tsx
- apps/web/app/(app)/compose/SlashCommand.tsx
- apps/web/app/(app)/compose/selectors/node-selector.tsx
- apps/web/app/(app)/compose/selectors/link-selector.tsx
- apps/web/app/(app)/compose/selectors/color-selector.tsx
✅ Files skipped from review due to trivial changes (2)
- apps/web/app/(app)/cold-email-blocker/ColdEmailPromptForm.tsx
- apps/web/app/(app)/cold-email-blocker/ColdEmailSettings.tsx
🧰 Additional context used
🪛 LanguageTool
ARCHITECTURE.md
[misspelling] ~169-~169: This word is normally spelled with a hyphen.
Context: ...ules and not the prompt file. We have a two way sync system between the db rules and th...
(EN_COMPOUNDS_TWO_WAY)
[uncategorized] ~174-~174: The adjective “prompt-based” is spelled with a hyphen.
Context: ... often each is called. If it were fully prompt based this wouldn't be possible. This is a po...
(BASED_HYPHEN)
[typographical] ~174-~174: Consider adding a comma before the interrupter.
Context: ... a potentially minor benefit to the user however. - Because actions are static (unless u...
(HOWEVER_COMMA)
[style] ~183-~183: ‘off of’ might be wordy. Consider a shorter alternative.
Context: ...# Reply Tracking This feature is built off of the AI personal assistant. There's a sp...
(EN_WORDINESS_PREMIUM_OFF_OF)
[grammar] ~184-~184: A word such as an article or a possessive pronoun before ‘reply’ seems to be missing.
Context: ...ant. There's a special type of rule for reply tracking. I considered making it a sepa...
(FOR_REPLY)
[formatting] ~186-~186: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...acking prompt (but this is also annoying, because it makes it hard for us to do a global ...
(COMMA_BEFORE_BECAUSE)
[style] ~190-~190: In some texts, you can shorten this phrase. Consider using “emailed us”.
Context: ... incoming emails, if the user has never sent us an email before we run it through an LLM to deci...
(SEND_PRP_AN_EMAIL)
[style] ~190-~190: In contexts where ‘if’ is followed by ‘or’, using ‘whether’ may be more appropriate (and formal).
Context: ...fore we run it through an LLM to decide if it's a cold email or not. This feature ...
(IF_WHETHER)
🔇 Additional comments (78)
apps/web/app/(app)/stats/LargestEmails.tsx (2)
16-16: LGTM! Import changes align with date handling improvements.The addition of
internalDateToDateimport follows the pattern of separating date parsing from formatting concerns.
85-91: LGTM! Enhanced date formatting with better separation of concerns.The changes improve maintainability by:
- Separating date parsing from formatting
- Adding explicit formatting options for better control
apps/web/utils/date.ts (1)
1-5: LGTM! Time constants are well-defined.The time constants are clearly defined with proper calculations based on standard time units.
apps/web/components/email-list/EmailListItem.tsx (1)
23-23: LGTM! Improved date handling.Good use of the new utility function for consistent date handling across the application.
Also applies to: 174-174
apps/web/utils/ai/actions.ts (1)
42-51: LGTM! Type definition properly updated.The EmailForAction type has been correctly extended to include the internalDate property, maintaining consistency with the new date handling functionality.
apps/web/components/email-list/EmailPanel.tsx (1)
1-8: LGTM!The imports and prop changes are well-structured and properly typed.
Also applies to: 34-61
apps/web/components/email-list/EmailList.tsx (1)
548-551: LGTM!The userEmail prop is correctly passed from the session data with proper fallback.
apps/web/prisma/schema.prisma (5)
16-16: Review Unique Constraint on Account.userId
The addition of@uniqueon theuserIdfield in theAccountmodel makes this a one-to-one mapping between users and accounts. This is a critical design decision because it deviates from the common pattern where a user may have multiple linked authentication accounts. Please verify that this aligns with your business logic and that no multi-provider account scenarios are required in your application.
88-97: Review Added Relational Fields in User Model
New relation fields—promptHistory,labels,rules,executedRules,newsletters,coldEmails,groups,apiKeys,categories, and especiallythreadTrackers—have been added to theUsermodel. This enhances the user’s associations, particularly supporting the new reply tracking functionality. Make sure that your queries and API resolvers are updated to handle these new relations appropriately.
174-186: Review New Field in Rule Model: trackReplies
A new optional fieldtrackReplieshas been added to theRulemodel to indicate if a rule is intended for reply tracking. Since this field is optional, please confirm that this design fits the intended behavior. Consider whether a default value (for example,false) might make the semantics clearer during rule creation, unless there is a specific reason to allow null.
357-375: Review New Model: ThreadTracker
The newThreadTrackermodel is structured well to support the reply tracking functionality. Key points include:
- Clear fields for tracking sent time, thread and message IDs, and a
resolvedflag with a default offalse.- Optional relation to the
Rulemodel allows flexibility in defining reply-tracking logic.- The unique constraint on
[userId, threadId, messageId]ensures that duplicate tracking records aren’t created per user, and the index on[userId, resolved]should help optimize common queries filtering by unresolved threads.Double-check that these constraints are consistent with your expected query patterns and performance requirements.
483-487: Review New Enum: ThreadTrackerType
The newThreadTrackerTypeenum with valuesAWAITING,NEEDS_REPLY, andNEEDS_ACTIONis a clear and expressive way to classify thread statuses. Ensure that all parts of the application using this enum are updated accordingly and that the enum values correctly represent the intended workflow in reply tracking.apps/web/utils/actions/mail.ts (1)
23-27: LGTM!The imports are well-organized and follow the established patterns in the file.
apps/web/app/(app)/compose/ComposeEmailForm.tsx (9)
10-10: No issues found.
The import line looks good.
24-27: No issues found.
The additional imports forisActionError,SendEmailBody,Tiptap, andsendEmailActionappear correct and are used appropriately later in the file.
40-46: Props destructuring looks neat.
Destructuring the props enhances clarity and streamlines usage within the component.
98-99: Correct usage of dependency array.
IncludingreplyingToEmail?.messageHtmlandreplyingToEmail?.messageTextensures that changes to the email’s original content trigger an updated callback, preventing stale data inonSubmit.
146-147: Useful prompt for “Draft.”
This toggle-based approach (editReply) is a straightforward way to show/hide fields for an in-progress draft.Also applies to: 150-150
285-289: Tiptap integration looks good.
Switching from a custom editor to Tiptap simplifies the editor code and helps maintain a clean architecture.
294-294: Dynamic styling usage is appropriate.
ApplyingsubmitButtonClassNamevia thecnutility is a good example of composable styling.
297-297: No issues found with the send button.
The asynchronous submission approach is consistent with React Hook Form.
302-302: Discard button and callback are well-structured.
This is a concise, consistent pattern for discarding in-progress drafts.Also applies to: 307-307, 309-309
apps/web/components/Tiptap.tsx (1)
1-2: Appropriate “use client” usage.
Marking this component as a client component is essential for Tiptap’s interactive features.apps/web/hooks/useDisplayedEmail.ts (2)
7-9: LGTM! Good use of URL state for reply tracking.Using
useQueryStateforautoOpenReplyForMessageIdis a good choice as it allows for deep linking and sharing of reply states via URLs.
25-25: LGTM! Dependencies are properly updated.The dependency array correctly includes
setAutoOpenReplyForMessageIdfor theuseCallbackhook.apps/web/components/EmailViewer.tsx (2)
61-63: LGTM! Props are properly passed to EmailThread.The new props are correctly passed down to the
EmailThreadcomponent.
18-19: Verify user email availability and add fallback handling.The user email is used with a fallback to an empty string. Consider:
- Adding a loading state while the session data is being fetched
- Handling cases where the user is not authenticated
Also applies to: 33-33
apps/web/utils/reply-tracker/outbound.ts (2)
1-16: Imports and Logger Initialization Look GoodThese foundational imports and logger creation appear correct and well-structured. No immediate issues or improvement recommendations.
39-46: Return Without Throwing May Mask Internal ErrorsWhen no thread messages are found, the function simply logs an error and returns. If this scenario indicates an unexpected condition, consider throwing an error or returning a more explicit status to prompt upstream handling, ensuring the calling code can respond appropriately.
apps/web/utils/actions/reply-tracking.ts (1)
16-156: Confirm Rule Uniqueness inenableReplyTrackerActionWhen creating or updating a rule to track replies, the code checks for an existing rule based on the same user ID and
trackReplies: true. If multiple rules match these criteria in future expansions, it may cause conflicts. Consider enforcing a unique constraint or adding further conditions to ensure only one "reply tracking" rule is enabled per user.apps/web/app/api/resend/summary/route.ts (3)
29-36: Conditional Filter Logic Is ClearUsing an object spread approach for the
forceparameter provides a neat way to bypass the date cutoff. This logic is readable and avoids duplicating query conditions.
126-128: Log or Handle Missing Access TokensWhen the user lacks a valid
access_token, the code gracefully falls back to an empty array formessages. Consider logging or notifying to help diagnose why messages are unavailable. This can aid in debugging user configurations.
211-213: Verifyforce: trueLogic for GET vs. Cron POSTThe
GEThandler forces sending a summary email, while the cron-basedPOSTuses the samesendEmailbut typically not forced. Verify that the forced summary logic inGETaligns with user expectations and that running both endpoints won’t introduce conflicting behaviors (e.g., double emailing).Also applies to: 225-226
apps/web/app/(app)/reply-tracker/NeedsReply.tsx (1)
1-31: LGTM! Clean and well-structured implementation.The implementation follows best practices with proper typing, modular design, and clear separation of concerns. The async function correctly handles data fetching and component composition.
apps/web/app/(app)/reply-tracker/NeedsAction.tsx (1)
1-32: Same code duplication issue as in AwaitingReply.tsx.Please refer to the refactoring suggestion in the
AwaitingReply.tsxreview comment, which addresses the code duplication across all three views.apps/web/hooks/useFeatureFlags.ts (1)
6-8: LGTM! Clean hook implementation.The implementation follows React hooks conventions and correctly uses PostHog's feature flag hooks.
apps/web/app/(app)/reply-tracker/EnableReplyTracker.tsx (1)
9-34: LGTM! Well-structured component with proper error handling.The component is well-implemented with:
- Clear user feedback through toast notifications
- Proper error handling using
isActionError- Good use of the reusable
EnableFeatureCardcomponentapps/web/components/EnableFeatureCard.tsx (1)
9-17: LGTM! Well-defined props interface.The props interface is clear and comprehensive with proper TypeScript types.
apps/web/utils/redis/label.ts (1)
17-17: LGTM! Good encapsulation practice.Making
getUserLabelsinternal is a good practice since it's only used within this module. The change maintains proper encapsulation while preserving the public API through the exported functions.apps/web/app/api/ai/summarise/route.ts (1)
6-6: Verify if removing summary caching is intentional.The code checks for cached summaries but no longer saves new summaries to Redis. This could lead to cache misses for future requests.
Run the following script to check if summary caching is handled elsewhere:
Also applies to: 30-31
✅ Verification successful
Confirmation: Caching is Handled in the Controller
- The
route.tsfile solely retrieves cached summaries usinggetSummary.- New summary caching is executed in
apps/web/app/api/ai/summarise/controller.tsviasaveSummary.- The Redis caching implementation in
apps/web/utils/redis/summary.tsremains active.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if summary caching is handled in other files. # Search for saveSummary usage rg -l "saveSummary" # Search for Redis summary operations rg "redis.*summary" --type tsLength of output: 382
apps/web/app/(app)/reply-tracker/TimeRangeFilter.tsx (1)
13-19: LGTM! Well-defined time range options.The time range options are well-structured with clear labels and values. Good use of const assertion for type safety.
apps/web/utils/reply-tracker/label.ts (4)
11-21: LGTM!The function follows the single responsibility principle and properly delegates to the
labelMessageutility.
23-33: LGTM!The function follows the single responsibility principle and properly delegates to the
labelThreadutility.
35-45: LGTM!The function follows the single responsibility principle and properly delegates to the
labelMessageutility.
47-57: LGTM!The function follows the single responsibility principle and properly delegates to the
labelThreadutility.apps/web/components/Toggle.tsx (4)
5-5: LGTM! Well-structured interface updates.The new optional props
tooltipTextandbgClassenhance the component's flexibility while maintaining type safety.Also applies to: 11-11, 16-16
20-27: LGTM! Clean props destructuring.Props destructuring is well-organized with a sensible default for
bgClass.
33-36: LGTM! Consistent tooltip implementation.The tooltip is consistently implemented for both left and right labels with proper text wrapping prevention.
Also applies to: 56-59
42-42: LGTM! Dynamic styling implementation.The dynamic background color implementation maintains visual consistency while adding customization options.
packages/resend/src/send.tsx (2)
2-2: LGTM! Enhanced email accessibility.Adding plain text alternative content improves email accessibility and compatibility across email clients.
Also applies to: 31-32
34-34: Verify email authentication for the new sender address.Please ensure that proper email authentication (SPF, DKIM, DMARC) is configured for the new sender address to maintain deliverability.
apps/web/utils/ai/choose-rule/execute.ts (1)
6-9: LGTM! Well-structured type updates.The new imports and type updates are properly organized and maintain type safety.
Also applies to: 21-21, 27-27
apps/web/__tests__/ai-choose-args.test.ts (1)
193-193: LGTM!The
trackRepliesproperty is correctly initialized as null in thegetRulehelper function.apps/web/app/(app)/cold-email-blocker/ColdEmailList.tsx (2)
27-28: LGTM!Clean up of imports improves code maintainability by removing unused dependencies.
209-218: Great improvement to the empty state UI!The use of
EnableFeatureCardprovides a better user experience by clearly explaining the feature and providing a clear call-to-action..cursorrules (1)
213-220: LGTM! Clear guidelines for utility functions.The new section provides clear guidance on:
- Using lodash utilities efficiently
- Importing specific functions to optimize bundle size
- Organizing utility functions in the codebase
apps/web/utils/assistant/process-assistant-email.ts (1)
14-14: LGTM! Simplified thread message retrieval.The change from
getThreadtogetThreadMessagesstreamlines the message retrieval process while maintaining proper error handling.Also applies to: 74-74
apps/web/utils/gmail/label.ts (2)
102-118: LGTM! Improved function signature readability.The destructured parameters make the function signature clearer and more maintainable.
257-294: Well-implemented batch label creation optimization!The new
getOrCreateLabelsfunction efficiently handles multiple labels by:
- Fetching existing labels once
- Using Map for O(1) lookups
- Including proper validation
- Handling edge cases
apps/web/components/SideNav.tsx (3)
25-25: LGTM! Imports align with reply tracking feature.The new imports support the reply tracking functionality.
Also applies to: 41-41
61-64: LGTM! Navigation items properly organized.The Reply Tracker and Smart Categories items are well-positioned in the navigation menu.
Also applies to: 79-83
96-101: LGTM! Feature flag implementation is correct.The navigation filtering logic properly uses the reply tracking feature flag.
apps/web/app/api/google/webhook/process-history.ts (4)
27-27: LGTM!The import statement is correctly placed and follows the project's import style.
89-92: LGTM! Enhanced logging for premium account debugging.The logging statement now includes
lemonSqueezyRenewsAt, which provides valuable context for debugging premium account issues.
398-405: LGTM! Proper handling of outbound emails.The code correctly:
- Identifies outbound emails using the SENT label
- Processes them with
handleOutboundReply- Skips further processing to avoid unnecessary operations
500-505: LGTM! Improved type safety in error handling.The code now:
- Uses
unknowntype instead ofanyfor better type safety- Properly checks if the error is an instance of
Errorbefore accessing its propertiesapps/web/utils/actions/ai-rule.ts (2)
265-265: LGTM! Efficient data fetching.The code correctly uses select to fetch only the required
trackRepliesfield from the rule.
274-274: LGTM! Safe parameter handling.The code correctly:
- Passes
isReplyTrackingRuletoexecuteAct- Uses nullish coalescing to safely handle undefined
trackRepliesapps/web/prisma/migrations/20250203174037_reply_tracker_sent_at/migration.sql (1)
1-2: Verify data migration strategy for non-nullable column.The migration adds a non-nullable
sentAtcolumn to theThreadTrackertable. Since this is a non-nullable column being added to an existing table, ensure there's a strategy to handle existing rows.Consider:
- Adding a default value for existing rows
- Running a data migration script
- Ensuring the table is empty before running the migration
apps/web/prisma/migrations/20250202154501_remove_deprecated_action/migration.sql (2)
1-11: Comprehensive Warnings and Documentation.
The migration script begins with clear, detailed warnings indicating that the columns being dropped will result in permanent data loss. This explicit commentary helps prevent accidental execution without full awareness of the consequences.
12-19: ALTER TABLE Command – Verify Column Dependencies.
The SQL command correctly drops the six specified columns from the "Action" table in one operation. Ensure that any application logic or queries that reference these columns have been updated accordingly to avoid runtime issues once the migration is executed.apps/web/prisma/migrations/20250202092329_reply_tracker/migration.sql (7)
1-3: Enum Type Creation is Clear.
The new enum type "ThreadTrackerType" is created properly with three distinct statuses. Verify that any application code leveraging this enum is updated accordingly.
7-21: Review of "ThreadTracker" Table Definition.
The table definition for "ThreadTracker" is well structured and includes a primary key and appropriate data types. Note that "updatedAt" is defined as NOT NULL without a default value; ensure that the application logic updates this column on record modification to prevent errors.
22-24: Index on "userId" and "resolved".
The index on the "userId" and "resolved" columns is appropriate for optimizing queries that filter based on these fields.
25-27: Unique Index on Thread Identifiers.
The unique index for the combination of "userId", "threadId", and "messageId" ensures that duplicate thread entries for a user are prevented, supporting data consistency.
28-30: Foreign Key Constraint on "ruleId".
The foreign key constraint linking "ThreadTracker.ruleId" to "Rule.id" with cascading deletes/updates is implemented correctly. Ensure that the "Rule" table’s identifier is properly indexed for optimal performance.
31-33: Foreign Key Constraint on "userId".
The constraint linking "ThreadTracker.userId" to "User.id" using cascading actions is correctly defined to maintain referential integrity.
34-37: Partial Unique Index on Rule's "trackReplies".
The creation of a partial unique index on the "Rule" table ensures that only one rule per user can have the "trackReplies" flag set to true. This design effectively enforces business rules regarding reply tracking.
| logger.info("Sending summary email to user", { | ||
| shouldSendEmail, | ||
| coldEmailers: coldEmailers.length, | ||
| pendingCount, | ||
| needsReplyCount: typeCounts[ThreadTrackerType.NEEDS_REPLY], | ||
| awaitingReplyCount: typeCounts[ThreadTrackerType.AWAITING], | ||
| needsActionCount: typeCounts[ThreadTrackerType.NEEDS_ACTION], | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential PII Concern in Logs
Logging email may be sensitive if your organization treats email addresses as personally identifiable information (PII). Consider hashing or obfuscating the email in the logs to protect privacy.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
apps/web/utils/assistant/process-assistant-email.ts (1)
279-291: Consider adding error types for better debugging.While the error handling is more robust now with separate logging for each label retrieval failure, consider adding error types to make debugging easier.
- logger.error("Error getting processing label", { + const error = processingLabelResult.reason as Error; + logger.error("Error getting processing label", { + name: error.name, + message: error.message, error: processingLabelResult.reason, }); - logger.error("Error getting assistant label", { + const error = assistantLabelResult.reason as Error; + logger.error("Error getting assistant label", { + name: error.name, + message: error.message, error: assistantLabelResult.reason, });apps/web/utils/auth.ts (1)
140-159: LGTM! Consider enhancing error handling and logging.The use of
Promise.allSettledis a great choice for handling multiple independent operations. The error handling is comprehensive but could be improved.Consider these improvements:
- Abstract the duplicate error handling logic
- Add success logging
- Include more error details
const [loopsResult, resendResult] = await Promise.allSettled([ createLoopsContact(user.email, user.name?.split(" ")?.[0]), createResendContact({ email: user.email }), ]); -if (loopsResult.status === "rejected") { - logger.error("Error creating Loops contact", { - email: user.email, - error: loopsResult.reason, - }); - captureException(loopsResult.reason, undefined, user.email); -} - -if (resendResult.status === "rejected") { - logger.error("Error creating Resend contact", { - email: user.email, - error: resendResult.reason, - }); - captureException(resendResult.reason, undefined, user.email); -} +const handleContactResult = ( + result: PromiseSettledResult<unknown>, + service: string, + email: string, +) => { + if (result.status === "fulfilled") { + logger.info(`Successfully created ${service} contact`, { email }); + } else { + logger.error(`Error creating ${service} contact`, { + email, + error: result.reason, + errorStack: result.reason instanceof Error ? result.reason.stack : undefined, + }); + captureException(result.reason, undefined, email); + } +}; + +handleContactResult(loopsResult, "Loops", user.email); +handleContactResult(resendResult, "Resend", user.email);apps/web/utils/gmail/trash.ts (2)
45-51: Consider adding retry logic for publish action.While the error logging is good, the publish action might benefit from retry logic since it's a non-critical operation that could be retried without affecting the main trash functionality.
Example implementation:
async function retryPublishDelete(options: { ownerEmail: string; threadId: string; actionSource: TinybirdEmailAction["actionSource"]; maxRetries?: number; backoffMs?: number; }) { const { maxRetries = 3, backoffMs = 1000 } = options; let lastError: unknown; for (let i = 0; i < maxRetries; i++) { try { return await publishDelete({ ownerEmail: options.ownerEmail, threadId: options.threadId, actionSource: options.actionSource, timestamp: Date.now(), }); } catch (error) { lastError = error; if (i < maxRetries - 1) { await new Promise(resolve => setTimeout(resolve, backoffMs * Math.pow(2, i))); } } } throw lastError; }
56-66: Consider adding consistent error logging.For consistency with
trashThread, consider adding similar structured error logging to thetrashMessagefunction.Example implementation:
export async function trashMessage(options: { gmail: gmail_v1.Gmail; messageId: string; ownerEmail: string; }) { const { gmail, messageId, ownerEmail } = options; try { return await gmail.users.messages.trash({ userId: "me", id: messageId, }); } catch (error) { logger.error("Failed to trash message", { email: ownerEmail, messageId, error, }); throw error; } }apps/web/utils/user/delete.ts (1)
44-57: Add type safety to error handling.The error handling is good but could benefit from explicit type safety.
Apply this diff to improve type safety:
- const failures = results.filter((r) => r.status === "rejected"); + const failures = results.filter( + (result): result is PromiseRejectedResult => result.status === "rejected" + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/app/api/user/complete-registration/route.ts(2 hunks)apps/web/utils/ai/choose-rule/execute.ts(4 hunks)apps/web/utils/assistant/process-assistant-email.ts(3 hunks)apps/web/utils/auth.ts(1 hunks)apps/web/utils/gmail/trash.ts(2 hunks)apps/web/utils/reply-tracker/inbound.ts(1 hunks)apps/web/utils/reply-tracker/outbound.ts(1 hunks)apps/web/utils/user/delete.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/ai/choose-rule/execute.ts
🔇 Additional comments (17)
apps/web/utils/assistant/process-assistant-email.ts (2)
14-14: LGTM: Import statement updated correctly.The import statement has been updated to use
getThreadMessagesinstead ofgetThread, which aligns with the new implementation.
74-84: LGTM: Thread message retrieval simplified.The implementation has been improved by directly fetching thread messages using
getThreadMessages. The error handling is comprehensive and provides user-friendly feedback.apps/web/utils/auth.ts (1)
335-356: LGTM! Well-structured type declarations.The interface declarations provide comprehensive type safety for error handling scenarios.
apps/web/utils/gmail/trash.ts (2)
1-5: LGTM! Well-structured logging setup.The addition of scoped logging and structured imports enhances the code's observability and maintainability.
36-43: LGTM! Improved error logging for Gmail trash operation.The structured logging with relevant context (email, threadId, error) will help with debugging and monitoring.
apps/web/utils/user/delete.ts (2)
11-13: LGTM! Good logging practice.Creating a scoped logger for user deletion operations improves observability and debugging capabilities.
28-42: LGTM! Good use of Promise.allSettled.Using Promise.allSettled is a good choice here as it:
- Allows all deletion operations to complete regardless of individual failures
- Executes operations concurrently for better performance
- Makes the code more maintainable by grouping related operations
apps/web/utils/reply-tracker/inbound.ts (4)
13-21: LGTM! Well-structured function signature and initialization.The function is well-defined with clear parameter types, and label retrieval is correctly performed before any operations.
23-53: LGTM! Good use of database transaction.The database operations are correctly wrapped in a transaction to ensure atomicity.
55-60: LGTM! Clear and focused label operations.The label operations are well-structured and follow the single responsibility principle.
62-90: LGTM! Comprehensive error handling.The error handling is well-implemented with:
- Granular error handling for each operation type
- Relevant context in error logs
- Clear error messages
apps/web/app/api/user/complete-registration/route.ts (2)
9-11: LGTM! Good logging practice.The introduction of a scoped logger enhances error tracking and debugging capabilities.
43-60: Improved error handling with Promise.allSettled.The error handling implementation is robust, ensuring both tracking events are attempted regardless of individual failures.
Review PII logging for compliance.
Consider whether logging user emails complies with your data protection policies. If needed, hash or redact the email before logging.
apps/web/utils/reply-tracker/outbound.ts (4)
1-17: LGTM! Well-organized imports and logger setup.The imports are properly typed and organized, and the logger setup follows best practices.
69-134: LGTM! Well-implemented concurrent operations with proper error handling.The function effectively manages concurrent database and labeling operations with comprehensive error handling.
29-36: Consider Transactional Updates or Error Handling AroundresolveReplyTrackers.Currently, the code resolves existing "NEEDS_REPLY" trackers before performing further logic (e.g., AI checks, creating new trackers). If an error occurs afterward, the state might end up partially updated. To ensure atomic or consistent updates, you could consider wrapping these steps in a transaction or introducing fallback/error handling logic.
While the current implementation uses Promise.allSettled for concurrent operations, consider using Prisma transactions to ensure database consistency:
+ await prisma.$transaction(async (tx) => { // When we send a reply, resolve any existing "NEEDS_REPLY" trackers await resolveReplyTrackers( gmail, userId, message.threadId, needsReplyLabelId, ); // ... rest of the logic ... + });
136-161: 🛠️ Refactor suggestionAdd error handling for Promise.allSettled results.
Similar to
createReplyTrackerOutbound, this function should handle potential failures in the concurrent operations.- await Promise.allSettled([updateDbPromise, labelPromise]); + const [updateResult, labelResult] = await Promise.allSettled([ + updateDbPromise, + labelPromise, + ]); + + const errorOptions = { + userId, + threadId, + }; + + if (updateResult.status === "rejected") { + logger.error("Failed to update reply trackers", { + ...errorOptions, + error: updateResult.reason, + }); + } + + if (labelResult.status === "rejected") { + logger.error("Failed to remove needs reply label", { + ...errorOptions, + error: labelResult.reason, + }); + }Likely invalid or redundant comment.
1 participant
Summary by CodeRabbit
New Features
Refactor