When enabling reply tracker check if any old emails need replies#323
When enabling reply tracker check if any old emails need replies#323
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 8 minutes and 34 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 (1)
WalkthroughThis pull request introduces a new constant Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/Session
participant A as enableReplyTrackerAction
participant G as GmailClient (via getGmailClient)
participant P as processPreviousSentEmails
U->>A: Request to enable reply tracker
A->>G: Retrieve Gmail Client (using access token)
A->>P: Process previous sent emails (pass Gmail client & user data)
P-->>A: Return completion status
sequenceDiagram
participant P as processPreviousSentEmails
participant M as getMessages
participant H as handleOutboundReply
P->>M: Fetch last 20 sent messages
alt Messages Found
P->>P: Group messages by thread
P->>M: Retrieve thread messages & sort by internalDate
alt Message labeled "SENT"
P->>H: Process outbound reply
else Other message labels
P->>P: (Placeholder for inbound reply handling)
end
else No messages
P->>P: Log informational message and exit
end
P->>P: Log completion of processing
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: 3
🧹 Nitpick comments (1)
apps/web/utils/reply-tracker/check-previous-emails.ts (1)
54-59: Consider adding retry logic for failed message processing.Currently, if processing a message fails, the error is logged but the function continues to the next message. Consider adding retry logic for transient failures.
+ const maxRetries = 3; + let retryCount = 0; + while (retryCount < maxRetries) { try { if (latestMessage.labelIds?.includes(GmailLabel.SENT)) { logger.info("Processing outbound reply", loggerOptions); await handleOutboundReply(user, latestMessage, gmail); } else { // TODO: inbound } + break; } catch (error) { + retryCount++; + if (retryCount === maxRetries) { logger.error("Error processing message for reply tracking", { ...loggerOptions, error, + retryCount, }); + } else { + logger.warn("Retrying message processing", { + ...loggerOptions, + error, + retryCount, + }); + await new Promise(resolve => setTimeout(resolve, 1000 * retryCount)); + } } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(app)/reply-tracker/page.tsx(1 hunks)apps/web/utils/actions/reply-tracking.ts(3 hunks)apps/web/utils/reply-tracker/check-previous-emails.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/app/(app)/reply-tracker/page.tsx
🔇 Additional comments (3)
apps/web/utils/reply-tracker/check-previous-emails.ts (3)
13-17: LGTM! Function signature is well-defined.The function parameters are properly typed with sensible defaults.
44-54: Consider implementing inbound reply processing.There's a TODO comment for inbound reply processing, but no implementation yet.
Would you like me to help implement the inbound reply processing logic or create an issue to track this task?
32-34: Potential issue with sorting messages.The sort function assumes
internalDateis always present. While the fallback to 0 prevents crashes, it could lead to incorrect sorting.Consider validating that messages have valid dates before sorting:
- const latestMessage = threadMessages.sort( - (a, b) => (Number(b.internalDate) || 0) - (Number(a.internalDate) || 0), - )[0]; + const validMessages = threadMessages.filter(msg => msg.internalDate); + if (validMessages.length === 0) continue; + const latestMessage = validMessages.sort( + (a, b) => Number(b.internalDate!) - Number(a.internalDate!), + )[0];✅ Verification successful
Error: No shell scripts or web queries found
Review: Sorting by
internalDatemay yield incorrect ordering if any emails lack valid datesAfter reviewing the code and considering the potential implications of falling back to a
0value, it appears that messages without a validinternalDatecould be mistakenly treated as having an epoch timestamp. This can lead to them being incorrectly prioritized in the sorted array. Filtering out messages without a valid date—like the proposed change—ensures that only meaningful timestamps are used for determining the "latest" message.
- The current fallback (
|| 0) prevents runtime errors but might result in inaccurate sorting.- The suggested update, which filters out messages lacking
internalDate, would address the risk of misordering.- Please verify that skipping messages with invalid or missing dates aligns with the intended behavior in this workflow.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/utils/reply-tracker/check-previous-emails.ts (1)
29-61: Consider batch processing and rate limiting.The sequential processing of messages could be slow for users with many emails. Additionally, rapid Gmail API calls might hit rate limits.
Consider these improvements:
- Process messages in parallel with controlled concurrency
- Add rate limiting for Gmail API calls
- for (const threadId of Object.keys(messagesByThreadId)) { + const batchSize = 5; // Process 5 threads at a time + const threadIds = Object.keys(messagesByThreadId); + for (let i = 0; i < threadIds.length; i += batchSize) { + const batch = threadIds.slice(i, i + batchSize); + await Promise.all( + batch.map(async (threadId) => { const threadMessages = await getThreadMessages(threadId, gmail); // ... rest of the processing logic + }) + ); + // Add delay between batches to respect rate limits + if (i + batchSize < threadIds.length) { + await new Promise(resolve => setTimeout(resolve, 1000)); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/utils/ai/choose-rule/ai-choose-rule.ts(1 hunks)apps/web/utils/reply-tracker/check-previous-emails.ts(1 hunks)apps/web/utils/reply-tracker/inbound.ts(2 hunks)apps/web/utils/reply-tracker/index.ts(1 hunks)apps/web/utils/reply-tracker/outbound.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/utils/ai/choose-rule/ai-choose-rule.ts
🔇 Additional comments (1)
apps/web/utils/reply-tracker/outbound.ts (1)
142-167: Consider potential race condition in resolveReplyTrackers.The database update and label removal operations are independent, which could lead to inconsistency if one operation fails.
Consider using a transaction or implementing a retry mechanism:
async function resolveReplyTrackers( gmail: gmail_v1.Gmail, userId: string, threadId: string, needsReplyLabelId: string, ) { - const updateDbPromise = prisma.threadTracker.updateMany({ - where: { - userId, - threadId, - resolved: false, - type: ThreadTrackerType.NEEDS_REPLY, - }, - data: { - resolved: true, - }, - }); - - const labelPromise = removeNeedsReplyLabel( - gmail, - threadId, - needsReplyLabelId, - ); - - await Promise.allSettled([updateDbPromise, labelPromise]); + const maxRetries = 3; + let retries = 0; + + while (retries < maxRetries) { + try { + await prisma.$transaction(async (tx) => { + await tx.threadTracker.updateMany({ + where: { + userId, + threadId, + resolved: false, + type: ThreadTrackerType.NEEDS_REPLY, + }, + data: { + resolved: true, + }, + }); + + await removeNeedsReplyLabel(gmail, threadId, needsReplyLabelId); + }); + break; + } catch (error) { + retries++; + if (retries === maxRetries) { + logger.error("Failed to resolve reply trackers after retries", { + userId, + threadId, + error, + }); + throw error; + } + await new Promise(resolve => setTimeout(resolve, 1000 * retries)); + } + } }
1 participant
Summary by CodeRabbit