Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThreads Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant MailAction as mail-bulk-action.ts
participant Provider as Gmail/Outlook Provider
participant OutlookBatch as outlook/batch.ts
participant Tinybird
participant Prisma
Caller->>MailAction: request bulk archive/trash (froms, ownerEmail, emailAccountId)
MailAction->>Provider: bulkArchiveFromSenders(froms, ownerEmail, emailAccountId)
Provider->>OutlookBatch: per-sender processing (fetch, batch move/archive/trash)
loop Per sender (paginated)
OutlookBatch->>OutlookBatch: fetch messageIds (nextLink / skipToken)
OutlookBatch->>OutlookBatch: move messages in batches
OutlookBatch->>Prisma: updateEmailMessagesForSender(sender, messageIds, emailAccountId, action)
OutlookBatch->>Tinybird: publishBulkActionToTinybird(threadIds, action, ownerEmail)
end
OutlookBatch-->>Provider: done per-sender
Provider-->>MailAction: complete
MailAction-->>Caller: complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/utils/email/bulk-action-tracking.ts (1)
20-37: Surface Tinybird failure details for debugging clarity.We only log the failure counts when Tinybird publish operations reject, which makes it hard to understand what actually failed in production. Surfacing a handful of sample reasons keeps logs bounded while giving enough signal to debug.
Apply this diff to enrich the log context:
await Promise.allSettled( batch.map((threadId) => publishFn({ ownerEmail, threadId, actionSource: "user", timestamp, }), ), ).then((results) => { const failures = results.filter((r) => r.status === "rejected"); if (failures.length > 0) { + const failureReasons = failures + .slice(0, 3) + .map((failure) => + failure.status === "rejected" + ? failure.reason instanceof Error + ? failure.reason.message + : String(failure.reason) + : undefined, + ) + .filter((reason): reason is string => Boolean(reason)); logger.error("Failed to publish some events to Tinybird", { failureCount: failures.length, totalCount: batch.length, + sampleReasons: failureReasons, }); } });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/utils/actions/mail-bulk-action.ts(2 hunks)apps/web/utils/email/bulk-action-tracking.ts(1 hunks)apps/web/utils/email/google.ts(5 hunks)apps/web/utils/email/microsoft.ts(2 hunks)apps/web/utils/email/types.ts(1 hunks)apps/web/utils/outlook/batch.ts(5 hunks)apps/web/utils/outlook/move-sender-messages.ts(0 hunks)version.txt(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/utils/outlook/move-sender-messages.ts
🧰 Additional context used
📓 Path-based instructions (10)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/utils/email/bulk-action-tracking.tsapps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.tsapps/web/utils/email/microsoft.tsapps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/utils/email/bulk-action-tracking.tsapps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.tsversion.txtapps/web/utils/email/microsoft.tsapps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/utils/email/bulk-action-tracking.tsapps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.tsapps/web/utils/email/microsoft.tsapps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/utils/email/bulk-action-tracking.tsapps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.tsapps/web/utils/email/microsoft.tsapps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
apps/web/utils/**
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logic
Files:
apps/web/utils/email/bulk-action-tracking.tsapps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.tsapps/web/utils/email/microsoft.tsapps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
apps/web/utils/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size
Files:
apps/web/utils/email/bulk-action-tracking.tsapps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.tsapps/web/utils/email/microsoft.tsapps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/utils/email/bulk-action-tracking.tsapps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.tsapps/web/utils/email/microsoft.tsapps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/utils/email/bulk-action-tracking.tsapps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.tsversion.txtapps/web/utils/email/microsoft.tsapps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
apps/web/utils/actions/**/*.ts
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/utils/actions/**/*.ts: Use server actions for all mutations (create/update/delete operations)
next-safe-actionprovides centralized error handling
Use Zod schemas for validation on both client and server
UserevalidatePathin server actions for cache invalidation
apps/web/utils/actions/**/*.ts: Use server actions (withnext-safe-action) for all mutations (create/update/delete operations); do NOT use POST API routes for mutations.
UserevalidatePathin server actions to invalidate cache after mutations.
Files:
apps/web/utils/actions/mail-bulk-action.ts
apps/web/utils/actions/*.ts
📄 CodeRabbit inference engine (.cursor/rules/server-actions.mdc)
apps/web/utils/actions/*.ts: Implement all server actions using thenext-safe-actionlibrary for type safety, input validation, context management, and error handling. Refer toapps/web/utils/actions/safe-action.tsfor client definitions (actionClient,actionClientUser,adminActionClient).
UseactionClientUserwhen only authenticated user context (userId) is needed.
UseactionClientwhen both authenticated user context and a specificemailAccountIdare needed. TheemailAccountIdmust be bound when calling the action from the client.
UseadminActionClientfor actions restricted to admin users.
Access necessary context (likeuserId,emailAccountId, etc.) provided by the safe action client via thectxobject in the.action()handler.
Server Actions are strictly for mutations (operations that change data, e.g., creating, updating, deleting). Do NOT use Server Actions for data fetching (GET operations). For data fetching, use dedicated GET API Routes combined with SWR Hooks.
UseSafeErrorfor expected/handled errors within actions if needed.next-safe-actionprovides centralized error handling.
Use the.metadata({ name: "actionName" })method to provide a meaningful name for monitoring. Sentry instrumentation is automatically applied viawithServerActionInstrumentationwithin the safe action clients.
If an action modifies data displayed elsewhere, userevalidatePathorrevalidateTagfromnext/cachewithin the action handler as needed.Server action files must start with
use server
Files:
apps/web/utils/actions/mail-bulk-action.ts
🧠 Learnings (8)
📚 Learning: 2025-07-18T15:05:34.899Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-07-18T15:05:34.899Z
Learning: Applies to apps/web/utils/gmail/**/*.ts : Keep provider-specific implementation details isolated in the appropriate utils subfolder (e.g., 'apps/web/utils/gmail/')
Applied to files:
apps/web/utils/email/bulk-action-tracking.tsapps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.tsapps/web/utils/email/microsoft.tsapps/web/utils/email/google.ts
📚 Learning: 2025-09-17T22:05:28.646Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-09-17T22:05:28.646Z
Learning: Applies to apps/web/utils/usage.ts : Implement usage tracking and monitoring in apps/web/utils/usage.ts
Applied to files:
apps/web/utils/email/bulk-action-tracking.ts
📚 Learning: 2025-09-17T22:05:28.646Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-09-17T22:05:28.646Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.{ts,tsx} : Keep related AI functions co-located and extract common patterns into utilities; document complex AI logic with clear comments
Applied to files:
apps/web/utils/email/bulk-action-tracking.tsapps/web/utils/email/microsoft.ts
📚 Learning: 2025-07-18T17:27:58.249Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-07-18T17:27:58.249Z
Learning: Applies to apps/web/utils/actions/*.ts : Use `actionClient` when both authenticated user context and a specific `emailAccountId` are needed. The `emailAccountId` must be bound when calling the action from the client.
Applied to files:
apps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.tsapps/web/utils/email/microsoft.ts
📚 Learning: 2025-07-18T15:04:30.467Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-07-18T15:04:30.467Z
Learning: Applies to apps/web/app/api/**/route.ts : Use `withEmailAccount` for email-account-level operations
Applied to files:
apps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.ts
📚 Learning: 2025-07-18T17:27:46.389Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-07-18T17:27:46.389Z
Learning: Applies to **/api/**/route.ts : Use `withEmailAccount` middleware for API routes that operate on a specific email account (i.e., use or require `emailAccountId`).
Applied to files:
apps/web/utils/email/types.tsapps/web/utils/actions/mail-bulk-action.ts
📚 Learning: 2025-10-02T23:23:48.064Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-10-02T23:23:48.064Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Prefer existing helpers from @/__tests__/helpers.ts (getEmailAccount, getEmail, getRule, getMockMessage, getMockExecutedRule) over custom helpers
Applied to files:
apps/web/utils/actions/mail-bulk-action.tsapps/web/utils/email/microsoft.ts
📚 Learning: 2025-07-19T17:50:28.270Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-19T17:50:28.270Z
Learning: The `utils` folder also contains core app logic such as Next.js Server Actions and Gmail API requests.
Applied to files:
apps/web/utils/email/microsoft.tsapps/web/utils/email/google.ts
🧬 Code graph analysis (4)
apps/web/utils/email/bulk-action-tracking.ts (1)
apps/web/utils/logger.ts (1)
createScopedLogger(17-80)
apps/web/utils/email/microsoft.ts (1)
apps/web/utils/outlook/batch.ts (1)
moveMessagesForSenders(164-276)
apps/web/utils/email/google.ts (2)
apps/web/utils/gmail/message.ts (1)
getMessages(257-286)apps/web/utils/email/bulk-action-tracking.ts (2)
updateEmailMessagesForSender(41-95)publishBulkActionToTinybird(7-39)
apps/web/utils/outlook/batch.ts (4)
apps/web/utils/logger.ts (1)
createScopedLogger(17-80)apps/web/utils/outlook/client.ts (1)
OutlookClient(19-80)apps/web/utils/outlook/odata-escape.ts (1)
escapeODataString(13-20)apps/web/utils/email/bulk-action-tracking.ts (2)
updateEmailMessagesForSender(41-95)publishBulkActionToTinybird(7-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 8 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="apps/web/utils/outlook/batch.ts">
<violation number="1" location="apps/web/utils/outlook/batch.ts:203">
The error handling for paginated message retrieval in the Outlook provider is inconsistent with the Gmail provider. If the API call to fetch messages fails for Outlook, it will throw an unhandled exception and terminate the entire bulk action. The Gmail implementation correctly handles this error, logging it and continuing with other senders.</violation>
<violation number="2" location="apps/web/utils/outlook/batch.ts:213">
Only add IDs to processedMessageIds after moveMessagesInBatches completes successfully. Adding them beforehand can skip retries on failures and leave messages unmoved.</violation>
<violation number="3" location="apps/web/utils/outlook/batch.ts:232">
Add thread IDs to publishedThreadIds only after publishBulkActionToTinybird resolves. Marking them early can drop analytics events on publish failures and prevent retries.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/utils/outlook/batch.ts (1)
229-253: Move thread marking to after tracking completes.The threads are being marked as published (line 232) before the tracking promises complete (line 253). If
publishBulkActionToTinybirdorupdateEmailMessagesForSenderfails, these threads are still marked as published in-memory and will never be retried, causing analytics data loss.Apply this diff to only mark threads as published after tracking succeeds:
const newThreadIds = Array.from(batchThreadIds).filter( (threadId) => !publishedThreadIds.has(threadId), ); - newThreadIds.forEach((threadId) => publishedThreadIds.add(threadId)); const promises = [ updateEmailMessagesForSender({ sender, messageIds, emailAccountId, action, }), ]; if (newThreadIds.length > 0) { promises.push( publishBulkActionToTinybird({ threadIds: newThreadIds, action, ownerEmail, }), ); } await Promise.all(promises); + + newThreadIds.forEach((threadId) => publishedThreadIds.add(threadId));
🧹 Nitpick comments (1)
apps/web/utils/email/google.ts (1)
345-352: Clarify error handling behavior.The comment says "continue processing remaining pages" but the code sets
nextPageToken = undefined, which breaks the loop and stops processing this sender. Either update the comment to reflect the actual behavior (e.g., "stop processing this sender and move to next") or change the logic to actually continue with the next page if that's the intent.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/utils/email/google.ts(5 hunks)apps/web/utils/outlook/batch.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
apps/web/utils/**
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logic
Files:
apps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
apps/web/utils/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size
Files:
apps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/utils/email/google.tsapps/web/utils/outlook/batch.ts
🧠 Learnings (2)
📚 Learning: 2025-07-18T15:05:34.899Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-07-18T15:05:34.899Z
Learning: Applies to apps/web/utils/gmail/**/*.ts : Keep provider-specific implementation details isolated in the appropriate utils subfolder (e.g., 'apps/web/utils/gmail/')
Applied to files:
apps/web/utils/email/google.ts
📚 Learning: 2025-07-19T17:50:28.270Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-19T17:50:28.270Z
Learning: The `utils` folder also contains core app logic such as Next.js Server Actions and Gmail API requests.
Applied to files:
apps/web/utils/email/google.ts
🧬 Code graph analysis (2)
apps/web/utils/email/google.ts (2)
apps/web/utils/gmail/message.ts (1)
getMessages(257-286)apps/web/utils/email/bulk-action-tracking.ts (2)
updateEmailMessagesForSender(41-95)publishBulkActionToTinybird(7-39)
apps/web/utils/outlook/batch.ts (3)
apps/web/utils/outlook/client.ts (1)
OutlookClient(19-80)apps/web/utils/outlook/odata-escape.ts (1)
escapeODataString(13-20)apps/web/utils/email/bulk-action-tracking.ts (2)
updateEmailMessagesForSender(41-95)publishBulkActionToTinybird(7-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
apps/web/utils/email/google.ts (3)
10-13: LGTM!The new imports for bulk action tracking are correctly added and align with the tracking integration throughout the file.
359-463: LGTM with minor note.The two-phase approach (aggregate threads, then trash and track) is appropriate for the trash operation. The logic correctly tracks only successfully trashed threads, and error handling allows processing to continue for other threads and senders.
Note: The error handling at line 402 has the same pattern as in
archiveMessagesFromSenderswhere the comment may not match the actual behavior.
465-483: LGTM!The public wrapper methods correctly delegate to the private implementation methods with the updated signatures including
ownerEmailandemailAccountId.apps/web/utils/outlook/batch.ts (6)
3-7: LGTM!The new imports are correctly added to support the per-sender batch moving functionality and bulk action tracking integration.
11-30: LGTM!Making these types and constants internal is appropriate since they are implementation details of the batch processing logic.
32-102: LGTM!The refactoring to make this function internal with destructured parameters improves the API design. The early return for empty requests is a good addition.
104-162: LGTM!The addition of the early return for empty
messageIdsand the refactoring to destructured parameters both improve the code quality.
212-224: LGTM!The message processing correctly marks messages as processed (line 223) only after
moveMessagesInBatchessucceeds, which addresses the concern from the previous review about premature marking.
254-265: LGTM!The error handling appropriately logs detailed context and continues processing remaining batches/senders, which aligns with the resilient bulk operation pattern.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/web/utils/outlook/batch.ts (2)
231-233: Move thread ID tracking to after the publish succeeds.Thread IDs are added to
publishedThreadIdsbeforePromise.allawaits the Tinybird publish on line 254. IfpublishBulkActionToTinybirdrejects, the catch block logs but doesn't throw, so those thread IDs remain marked as "published" even though the analytics event was never recorded. The next batch filters them (line 229), silently dropping the events.Apply this diff to mark threads as published only after the publish completes:
const newThreadIds = Array.from(batchThreadIds).filter( (threadId) => !publishedThreadIds.has(threadId), ); - newThreadIds.forEach((threadId) => - publishedThreadIds.add(threadId), - ); const promises = [ updateEmailMessagesForSender({ sender, messageIds, emailAccountId, action, }), ]; if (newThreadIds.length > 0) { promises.push( publishBulkActionToTinybird({ threadIds: newThreadIds, action, ownerEmail, }), ); } await Promise.all(promises); + + // Mark threads as published only after successful publish + newThreadIds.forEach((threadId) => + publishedThreadIds.add(threadId), + );
264-266: Move ID tracking to the success path to enable retry on failure.The
finallyblock marks messages as processed regardless of whethermoveMessagesInBatchessucceeds. When the move throws (line 217), the catch block logs the error, but the finally block still adds IDs toprocessedMessageIds. The next page then filters these IDs (line 210), so failed messages are never retried—they remain in the source folder but are silently skipped.Apply this diff to mark IDs as processed only after a successful move:
await moveMessagesInBatches({ client, messageIds, destinationId, action, }); + // Mark as processed only after successful move + messageIds.forEach((id) => processedMessageIds.add(id)); + const batchThreadIds = new Set( allMessages.map((msg) => msg.conversationId), ); const newThreadIds = Array.from(batchThreadIds).filter( (threadId) => !publishedThreadIds.has(threadId), ); newThreadIds.forEach((threadId) => publishedThreadIds.add(threadId), ); const promises = [ updateEmailMessagesForSender({ sender, messageIds, emailAccountId, action, }), ]; if (newThreadIds.length > 0) { promises.push( publishBulkActionToTinybird({ threadIds: newThreadIds, action, ownerEmail, }), ); } await Promise.all(promises); } catch (error) { logger.error("Failed to move or track messages", { action, sender, ownerEmail, destinationId, messageIds, error: error instanceof Error ? error.message : error, }); - } finally { - messageIds.forEach((id) => processedMessageIds.add(id)); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/utils/outlook/batch.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/utils/outlook/batch.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit inference engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/utils/outlook/batch.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/utils/outlook/batch.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/utils/outlook/batch.ts
apps/web/utils/**
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logic
Files:
apps/web/utils/outlook/batch.ts
apps/web/utils/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size
Files:
apps/web/utils/outlook/batch.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/utils/outlook/batch.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/utils/outlook/batch.ts
🧠 Learnings (3)
📚 Learning: 2025-09-17T22:05:28.646Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-09-17T22:05:28.646Z
Learning: Applies to apps/web/utils/ai/**/*.{ts,tsx} : Use proper error types and logging for failures
Applied to files:
apps/web/utils/outlook/batch.ts
📚 Learning: 2025-09-17T22:05:28.646Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-09-17T22:05:28.646Z
Learning: Applies to apps/web/utils/ai/**/*.{ts,tsx} : Implement fallbacks for AI failures
Applied to files:
apps/web/utils/outlook/batch.ts
📚 Learning: 2025-07-18T15:05:34.899Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-07-18T15:05:34.899Z
Learning: Applies to apps/web/utils/gmail/**/*.ts : Keep provider-specific implementation details isolated in the appropriate utils subfolder (e.g., 'apps/web/utils/gmail/')
Applied to files:
apps/web/utils/outlook/batch.ts
🧬 Code graph analysis (1)
apps/web/utils/outlook/batch.ts (3)
apps/web/utils/outlook/client.ts (1)
OutlookClient(19-80)apps/web/utils/outlook/odata-escape.ts (1)
escapeODataString(13-20)apps/web/utils/email/bulk-action-tracking.ts (2)
updateEmailMessagesForSender(41-95)publishBulkActionToTinybird(7-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (4)
apps/web/utils/outlook/batch.ts (4)
1-11: LGTM!Imports and logger setup follow project conventions correctly.
11-31: LGTM!Internalizing batch types and constants is appropriate—these are implementation details not needed by external callers.
32-102: LGTM!The refactored signature and early return improve readability and defensive coding practices.
104-162: LGTM!The refactored signature and early guard are good. Note that
stopOnError: truemeans any batch error will throw, so callers must handle failures carefully to avoid losing retry capability.
| if (messageIds.length > 0) { | ||
| try { | ||
| await moveMessagesInBatches({ | ||
| client, | ||
| messageIds, | ||
| destinationId, | ||
| action, | ||
| }); | ||
|
|
||
| const batchThreadIds = new Set( | ||
| allMessages.map((msg) => msg.conversationId), | ||
| ); | ||
|
|
||
| const newThreadIds = Array.from(batchThreadIds).filter( | ||
| (threadId) => !publishedThreadIds.has(threadId), | ||
| ); | ||
| newThreadIds.forEach((threadId) => | ||
| publishedThreadIds.add(threadId), | ||
| ); | ||
|
|
||
| const promises = [ | ||
| updateEmailMessagesForSender({ | ||
| sender, | ||
| messageIds, | ||
| emailAccountId, | ||
| action, | ||
| }), | ||
| ]; | ||
|
|
||
| if (newThreadIds.length > 0) { | ||
| promises.push( | ||
| publishBulkActionToTinybird({ | ||
| threadIds: newThreadIds, | ||
| action, | ||
| ownerEmail, | ||
| }), | ||
| ); | ||
| } | ||
|
|
||
| await Promise.all(promises); | ||
| } catch (error) { | ||
| logger.error("Failed to move or track messages", { | ||
| action, | ||
| sender, | ||
| ownerEmail, | ||
| destinationId, | ||
| messageIds, | ||
| error: error instanceof Error ? error.message : error, | ||
| }); | ||
| } finally { | ||
| messageIds.forEach((id) => processedMessageIds.add(id)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Combined tracking issues create silent data loss.
The premature tracking updates (thread IDs before publish + message IDs in finally) compound each other:
- Move fails → catch logs → finally marks IDs processed → messages stay in source but are skipped on retry
- Publish fails → thread IDs already marked → analytics lost but threads skipped on retry
This creates inconsistency between the mailbox state, local tracking state, and analytics.
The diffs in the previous comments address both issues. Ensure:
- Message IDs are added to
processedMessageIdsonly aftermoveMessagesInBatchessucceeds - Thread IDs are added to
publishedThreadIdsonly afterPromise.allresolves
1 participant
Summary by CodeRabbit
Improvements
Chores