Skip to content

Comments

Cold Email: refactor to use learned patterns#1180

Merged
elie222 merged 25 commits intomainfrom
feat/cold-email-and-ai-rules
Jan 4, 2026
Merged

Cold Email: refactor to use learned patterns#1180
elie222 merged 25 commits intomainfrom
feat/cold-email-and-ai-rules

Conversation

@elie222
Copy link
Owner

@elie222 elie222 commented Jan 2, 2026

User description

We previously had a separate table for storing cold emailers.
I've now changed this so this table is no longer in use. It was inconsistent with how many other things worked.
We already have learned patterns and this is now used for cold emailers too.

Improved cold email detection and handling, updated AI rule execution logic, and added a migration script for cold emails to group items.

  • Update cold email detection logic and tests
  • Improve AI rule matching and execution
  • Add migration script for cold email data
  • Update Prisma schema for cold email changes


Generated description

Below is a concise technical summary of the changes proposed in this PR:

graph LR
executeMatchedRule_("executeMatchedRule"):::modified
saveLearnedPattern_("saveLearnedPattern"):::modified
getOrCreateGroupForRule_("getOrCreateGroupForRule"):::added
PRISMA_("PRISMA"):::modified
isColdEmail_("isColdEmail"):::modified
learnFromRemovedLabel_("learnFromRemovedLabel"):::modified
handleLabelRemovedEvent_("handleLabelRemovedEvent"):::modified
sendEmail_("sendEmail"):::modified
executeMatchedRule_ -- "Saves AI-detected sender (from, messageId, threadId) when executed." --> saveLearnedPattern_
saveLearnedPattern_ -- "Ensures rule group exists before upserting FROM pattern." --> getOrCreateGroupForRule_
saveLearnedPattern_ -- "Upserts groupItem with groupId,type FROM,value and metadata." --> PRISMA_
isColdEmail_ -- "Queries groupItem FROM entries to detect matches or exclusions." --> PRISMA_
learnFromRemovedLabel_ -- "Records exclusion via saveLearnedPattern (exclude=true) for sender." --> saveLearnedPattern_
learnFromRemovedLabel_ -- "Finds rule by labelId to validate learnable system type." --> PRISMA_
handleLabelRemovedEvent_ -- "Iterates removed labels and delegates learning per labelId." --> learnFromRemovedLabel_
sendEmail_ -- "Fetches executedRule cold-email entries to include recent messages." --> PRISMA_
classDef added stroke:#15AA7A
classDef removed stroke:#CD5270
classDef modified stroke:#EDAC4C
linkStyle default stroke:#CBD5E1,font-size:13px
Loading

Refactor the cold email management system to utilize the existing Learned Patterns infrastructure, deprecating the dedicated ColdEmail table. Update the AI rule execution logic and user actions to store and retrieve cold email data consistently within the GroupItem model.

TopicDetails
Refactor Cold Email Logic Refactor the logic for detecting, saving, and handling cold emails to consistently use the saveLearnedPattern utility and the GroupItem model. This includes updates to AI rule execution, user actions (like marking an email as 'not cold'), and handling label removal events to record exclusions.
Modified files (13)
  • apps/web/utils/rule/consts.ts
  • apps/web/utils/cold-email/cold-email-rule.ts
  • apps/web/utils/ai/choose-rule/types.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
  • apps/web/utils/ai/choose-rule/run-rules.test.ts
  • apps/web/utils/ai/choose-rule/match-rules.ts
  • apps/web/utils/ai/choose-rule/match-rules.test.ts
  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/rule/learned-patterns.test.ts
Latest Contributors(2)
UserCommitDate
elie222loggerDecember 18, 2025
eduardoleliss@gmail.comHandle-only-cold-email...August 15, 2025
Migrate Cold Email Data Migrate historical cold email data from the deprecated ColdEmail table to the unified GroupItem (learned patterns) system, updating data retrieval logic across the application for summaries and lists. This includes schema changes to GroupItem to support cold email specific fields like reason, threadId, messageId, and source.
Modified files (14)
  • apps/web/app/(app)/[emailAccountId]/assistant/group/ViewLearnedPatterns.tsx
  • apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/cold-email/is-cold-email.test.ts
  • apps/web/prisma/schema.prisma
  • apps/web/prisma/migrations/20260104000000_add_label_removed_to_group_item_source/migration.sql
  • apps/web/prisma/migrations/20260103000000_migrate_cold_emails_to_group_items/migration.sql
  • apps/web/components/EmailMessageCell.tsx
  • apps/web/app/api/user/cold-email/route.ts
  • apps/web/app/api/resend/summary/route.ts
  • apps/web/app/(app)/[emailAccountId]/cold-email-blocker/ColdEmailList.tsx
  • apps/web/app/(app)/[emailAccountId]/assistant/settings/WritingStyleSetting.tsx
  • apps/web/app/(app)/[emailAccountId]/assistant/group/LearnedPatterns.tsx
  • .vscode/settings.json
Latest Contributors(2)
UserCommitDate
elie222Conversational-rule-pr...December 24, 2025
joshwerner001@gmail.comRemove-conditional-tit...December 10, 2025
This pull request is reviewed by Baz. Review like a pro on (Baz).

@vercel
Copy link

vercel bot commented Jan 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
inbox-zero Ready Ready Preview Jan 4, 2026 0:19am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 2, 2026

Note

Other AI code review bot(s) detected

CodeRabbit 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.

📝 Walkthrough

Walkthrough

This PR migrates the cold email detection and learning system from direct ColdEmail records to a GroupItem-based learned patterns approach, introducing the GroupItemSource enum. Multiple API routes and utilities are updated to use saveLearnedPattern instead of direct ColdEmail operations, with a comprehensive Prisma migration transferring existing cold email data to the new structure.

Changes

Cohort / File(s) Summary
Prisma Schema & Migration
apps/web/prisma/schema.prisma, apps/web/prisma/migrations/20260103233401_migrate_cold_emails_to_group_items/migration.sql
Deprecated EmailAccount.coldEmails and DigestItem.coldEmail fields; added contextual fields (reason, threadId, messageId, source) to GroupItem; introduced GroupItemSource enum (AI, USER, LABEL\_REMOVED); created two-phase SQL migration that generates/reuses Groups for COLD\_EMAIL rules and migrates ColdEmail records to GroupItem with proper status and source mapping.
Learned Patterns Core
apps/web/utils/rule/learned-patterns.ts, apps/web/utils/rule/learned-patterns.test.ts
Updated saveLearnedPattern signature to accept ruleId (instead of ruleName), added optional fields (exclude, reason, threadId, messageId, source); introduced getOrCreateGroupForRule helper to manage group creation/linking with duplicate error handling; comprehensive test coverage for success/error paths.
Cold Email Detection & Actions
apps/web/utils/cold-email/is-cold-email.ts, apps/web/utils/cold-email/is-cold-email.test.ts, apps/web/utils/cold-email/cold-email-rule.ts, apps/web/utils/actions/cold-email.ts
Replaced isKnownColdEmailSender logic with groupId-based pattern checks; removed saveColdEmail function; updated isColdEmail to query GroupItem by FROM value with exclusion handling; refactored markNotColdEmailAction to call saveLearnedPattern with exclude flag; changed removeColdEmailLabelFromSender to accept coldEmailRule as parameter; added groupId to getColdEmailRule select.
AI Rule Selection Pipeline
apps/web/utils/ai/choose-rule/run-rules.ts, apps/web/utils/ai/choose-rule/match-rules.ts, apps/web/utils/ai/choose-rule/types.ts, apps/web/utils/ai/choose-rule/run-rules.test.ts, apps/web/utils/ai/choose-rule/match-rules.test.ts
Replaced saveColdEmail calls with saveLearnedPattern in COLD\_EMAIL branch, passing ruleId, source (GroupItemSource.AI), and contextual data (messageId, threadId, reason); updated match-rules to use aiReason fallback; made SerializedMatchReason non-exported; standardized test helpers with destructured defaults.
API Route Handlers
apps/web/app/api/ai/analyze-sender-pattern/route.ts, apps/web/app/api/google/webhook/process-label-removed-event.ts, apps/web/app/api/google/webhook/process-label-removed-event.test.ts, apps/web/app/api/resend/summary/route.ts, apps/web/app/api/user/cold-email/route.ts
Migrated analyze-sender-pattern to call saveLearnedPattern with source: GroupItemSource.AI; refactored process-label-removed-event to use learned pattern saving instead of ColdEmail upsert with exclude/reason fields; updated resend summary to fetch coldExecutedRules via rule-based lookup instead of direct coldEmails; changed user/cold-email endpoint to use groupItem queries with groupId migration path.
UI & Configuration
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx, apps/web/app/(app)/[emailAccountId]/cold-email-blocker/ColdEmailList.tsx, apps/web/app/(app)/[emailAccountId]/assistant/settings/WritingStyleSetting.tsx, apps/web/utils/rule/consts.ts
Updated RuleForm to disable LearnedPatternsDialog for conversation-status types (not multiple-condition check); refactored ColdEmailList to use toggleRuleAction for enable workflow with success/error toasts; expanded WritingStyleSetting placeholder text; added shouldLearn property to ruleConfig entries and introduced shouldLearnFromLabelRemoval function.
VSCode Configuration
.vscode/settings.json
Added prisma.pinToPrisma6: true setting.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant API Routes
    participant AI Pipeline
    participant Utilities
    participant Database as Database (Prisma)
    
    rect rgb(200, 220, 255)
    Note over Client,Database: AI-Learned Pattern Flow
    Client->>API Routes: Message arrives / AI analysis
    API Routes->>AI Pipeline: run-rules / analyze-sender-pattern
    alt Rule matches & matches historical rule
        AI Pipeline->>Utilities: saveLearnedPattern(ruleId, from, source: AI, ...)
        Utilities->>Database: Query rule by ruleId
        Database-->>Utilities: Rule + groupId
        Utilities->>Database: Create/reuse group via getOrCreateGroupForRule
        Utilities->>Database: Upsert GroupItem (type: FROM, value, source: AI)
        Database-->>Utilities: Success
        Utilities-->>API Routes: Pattern saved
    else Rule not found
        Utilities->>Utilities: Log error, early return
    end
    end
    
    rect rgb(220, 255, 220)
    Note over Client,Database: User Action Flow
    Client->>API Routes: markNotColdEmailAction
    API Routes->>Utilities: saveLearnedPattern(ruleId, exclude: true, source: USER, ...)
    Utilities->>Database: Upsert GroupItem with exclude: true
    Database-->>Utilities: Success
    Utilities->>Utilities: removeLabels (parallel)
    Utilities-->>API Routes: Complete
    end
    
    rect rgb(255, 240, 200)
    Note over Client,Database: Label Removal Flow
    Client->>API Routes: Label removed webhook
    API Routes->>Utilities: shouldLearnFromLabelRemoval(systemType)
    Utilities-->>API Routes: shouldLearn flag
    API Routes->>Database: Query rules by label action
    Database-->>API Routes: Matching rules
    API Routes->>Utilities: saveLearnedPattern(ruleId, exclude: true, source: LABEL_REMOVED, ...)
    Utilities->>Database: Upsert GroupItem
    Database-->>Utilities: Success
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • PR #575: Modifies saveLearnedPatterns/saveLearnedPattern behavior and error handling in learned-patterns utilities, directly touching the same core migration logic.
  • PR #689: Updates process-label-removed-event webhook to use saveLearnedPattern with GroupItemSource, evolving the label-removal learning flow that this PR introduces.
  • PR #779: Modifies AI sender-pattern learning in analyze-sender-pattern to propagate GroupItemSource.AI, directly complementing this PR's schema and learning infrastructure.

Suggested reviewers

  • edulelis
  • anakarentorosserrano-star
  • baz-reviewer

Poem

Hopping through patterns, both old and new,
GroupItems gather what cold senders do,
From ColdEmail's nest to a learned array,
With sources—AI, USER, and labels at play,
My burrow's alive with migrations so bright! 🐰✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.79% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Cold Email: refactor to use learned patterns' accurately summarizes the main objective of the PR, which is a comprehensive refactoring to migrate cold email handling from a dedicated ColdEmail table to the existing learned-patterns system.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@macroscopeapp
Copy link
Contributor

macroscopeapp bot commented Jan 2, 2026

Refactor cold email handling to store learned patterns on GroupItem by ruleId and switch detection, APIs, webhooks, and server actions to use group-linked patterns

Migrate cold email logic from ColdEmail rows to GroupItem patterns keyed by ruleId, update AI execution and label-removed webhook to persist source and metadata, reroute summary and list APIs to query executed rules and group items, and gate learned pattern UI on conversation status types.

📍Where to Start

Start with saveLearnedPattern and getOrCreateGroupForRule in learned-patterns.ts, then follow usages in AI execution at run-rules.ts and the label removal flow at process-label-removed-event.ts.


Macroscope summarized f735e5e.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 18 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="apps/web/utils/cold-email/is-cold-email.ts">

<violation number="1" location="apps/web/utils/cold-email/is-cold-email.ts:63">
P2: The reason `&quot;hasPreviousEmail&quot;` is returned when the sender is explicitly excluded, but this is semantically incorrect. The UI will display &quot;This person has previously emailed you&quot; which is misleading. Consider adding a new reason type (e.g., `&quot;excluded&quot;`) to properly represent explicitly excluded senders.</violation>
</file>

<file name="apps/web/app/api/google/webhook/process-label-removed-event.test.ts">

<violation number="1" location="apps/web/app/api/google/webhook/process-label-removed-event.test.ts:42">
P2: The mock includes `SOCIAL_PROMOTIONS` which doesn&#39;t exist in the actual `GmailLabel` constant. This appears to be a copy-paste error from the `PROMOTIONS` line above.</violation>
</file>

<file name="apps/web/utils/actions/cold-email.ts">

<violation number="1" location="apps/web/utils/actions/cold-email.ts:39">
P2: Using hardcoded `ruleName: &quot;Cold Email&quot;` is fragile - the cold email rule is identified by `systemType: SystemType.COLD_EMAIL`, not by name. Consider fetching the rule first with `getColdEmailRule(emailAccountId)` and using `ruleId` instead for reliable lookup.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

from: email.from,
});
return { isColdEmail: true, reason: "ai-already-labeled" };
return { isColdEmail: false, reason: "hasPreviousEmail" };
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The reason "hasPreviousEmail" is returned when the sender is explicitly excluded, but this is semantically incorrect. The UI will display "This person has previously emailed you" which is misleading. Consider adding a new reason type (e.g., "excluded") to properly represent explicitly excluded senders.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/utils/cold-email/is-cold-email.ts, line 63:

<comment>The reason `&quot;hasPreviousEmail&quot;` is returned when the sender is explicitly excluded, but this is semantically incorrect. The UI will display &quot;This person has previously emailed you&quot; which is misleading. Consider adding a new reason type (e.g., `&quot;excluded&quot;`) to properly represent explicitly excluded senders.</comment>

<file context>
@@ -43,16 +45,22 @@ export async function isColdEmail({
       from: email.from,
     });
-    return { isColdEmail: true, reason: &quot;ai-already-labeled&quot; };
+    return { isColdEmail: false, reason: &quot;hasPreviousEmail&quot; };
   }
 
</file context>

✅ Addressed in 48f320c

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 48f320c addressed this comment by changing the misleading reason from "hasPreviousEmail" to "excluded" on line 63. This creates semantic consistency between the log message stating the sender is "explicitly excluded" and the reason returned, ensuring the UI will properly display that the sender was excluded rather than showing misleading text about previous email history.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Changed reason to 'excluded' for explicitly excluded senders.

PERSONAL: "CATEGORY_PERSONAL",
SOCIAL: "CATEGORY_SOCIAL",
PROMOTIONS: "CATEGORY_PROMOTIONS",
SOCIAL_PROMOTIONS: "CATEGORY_PROMOTIONS",
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The mock includes SOCIAL_PROMOTIONS which doesn't exist in the actual GmailLabel constant. This appears to be a copy-paste error from the PROMOTIONS line above.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/app/api/google/webhook/process-label-removed-event.test.ts, line 42:

<comment>The mock includes `SOCIAL_PROMOTIONS` which doesn&#39;t exist in the actual `GmailLabel` constant. This appears to be a copy-paste error from the `PROMOTIONS` line above.</comment>

<file context>
@@ -30,6 +39,7 @@ vi.mock(&quot;@/utils/gmail/label&quot;, () =&gt; ({
     PERSONAL: &quot;CATEGORY_PERSONAL&quot;,
     SOCIAL: &quot;CATEGORY_SOCIAL&quot;,
     PROMOTIONS: &quot;CATEGORY_PROMOTIONS&quot;,
+    SOCIAL_PROMOTIONS: &quot;CATEGORY_PROMOTIONS&quot;,
     FORUMS: &quot;CATEGORY_FORUMS&quot;,
     UPDATES: &quot;CATEGORY_UPDATES&quot;,
</file context>

✅ Addressed in 48f320c

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 48f320c addressed this comment by removing the erroneous SOCIAL_PROMOTIONS: "CATEGORY_PROMOTIONS" line from the mock. The problematic property that didn't exist in the actual GmailLabel constant has been completely eliminated, fixing the copy-paste error identified in the comment.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Removed the incorrect SOCIAL_PROMOTIONS mock.

saveLearnedPattern({
emailAccountId,
from: sender,
ruleName: "Cold Email",
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Using hardcoded ruleName: "Cold Email" is fragile - the cold email rule is identified by systemType: SystemType.COLD_EMAIL, not by name. Consider fetching the rule first with getColdEmailRule(emailAccountId) and using ruleId instead for reliable lookup.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/utils/actions/cold-email.ts, line 39:

<comment>Using hardcoded `ruleName: &quot;Cold Email&quot;` is fragile - the cold email rule is identified by `systemType: SystemType.COLD_EMAIL`, not by name. Consider fetching the rule first with `getColdEmailRule(emailAccountId)` and using `ruleId` instead for reliable lookup.</comment>

<file context>
@@ -31,16 +32,14 @@ export const markNotColdEmailAction = actionClient
+        saveLearnedPattern({
+          emailAccountId,
+          from: sender,
+          ruleName: &quot;Cold Email&quot;,
+          exclude: true,
+          logger,
</file context>

✅ Addressed in 48f320c

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 48f320c addressed this comment by replacing the hardcoded ruleName: "Cold Email" with ruleId: coldEmailRule.id after fetching the rule using getColdEmailRule(emailAccountId). The code now uses the reliable rule ID lookup method as suggested, eliminating the fragile hardcoded string approach.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Using ruleId now for reliable lookup.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! I've saved this as a new learning to improve future reviews.

Comment on lines 673 to 677
// model ColdEmail {
// id String @id @default(cuid())
// createdAt DateTime @default(now())
// updatedAt DateTime @updatedAt
// fromEmail String
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we drop the commented ColdEmail array/fields/model (lines 159, 280, 673-691) instead of keeping dead schema? It's confusing.

Suggested change
// model ColdEmail {
// id String @id @default(cuid())
// createdAt DateTime @default(now())
// updatedAt DateTime @updatedAt
// fromEmail String

Finding type: Code Hygiene

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restored the models for now to support fallback/migration.

Comment on lines 54 to 64
if (patternMatch === "include") {
logger.info("Known cold email sender", { from: email.from });
return { isColdEmail: true, reason: "ai-already-labeled" };
}

if (patternMatch === "exclude") {
logger.info("Sender explicitly excluded from cold email blocker", {
from: email.from,
});
return { isColdEmail: true, reason: "ai-already-labeled" };
return { isColdEmail: false, reason: "hasPreviousEmail" };
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a learned pattern exists with exclude = true, this branch short‑circuits with reason: "hasPreviousEmail". The UI (e.g. the cold-email test card) uses that reason to decide whether to show "This person has previously emailed you…", so manually excluded or AI‑learned senders will now be reported as if the sender simply had history instead of being explicitly excluded, which misleads users.

Suggested change
if (patternMatch === "include") {
logger.info("Known cold email sender", { from: email.from });
return { isColdEmail: true, reason: "ai-already-labeled" };
}
if (patternMatch === "exclude") {
logger.info("Sender explicitly excluded from cold email blocker", {
from: email.from,
});
return { isColdEmail: true, reason: "ai-already-labeled" };
return { isColdEmail: false, reason: "hasPreviousEmail" };
}
if (patternMatch === "include") {
logger.info("Known cold email sender", { from: email.from });
return { isColdEmail: true, reason: "ai-already-labeled" };
}
if (patternMatch === "exclude") {
logger.info("Sender explicitly excluded from cold email blocker", {
from: email.from,
});
return { isColdEmail: false, reason: "excluded" };
}

Finding type: Logical Bugs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 48f320c addressed this comment by changing the return reason from "hasPreviousEmail" to "excluded" when a sender is explicitly excluded from cold email blocking. This fix ensures that the UI will correctly distinguish between senders who have previous email history versus those who are explicitly excluded, preventing user confusion.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Reason changed to 'excluded'.

Comment on lines 71 to 86
function extractFromEmail(matchMetadata: any): string | null {
if (!matchMetadata || !Array.isArray(matchMetadata)) return null;

const reasons = matchMetadata as SerializedMatchReason[];

for (const reason of reasons) {
if (reason.type === "AI" && reason.from) {
return reason.from;
}
if (reason.type === "LEARNED_PATTERN" && reason.groupItem?.value) {
return reason.groupItem.value;
}
}

return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extractFromEmail only looks for a sender when a reason is type "AI" with a from or when type "LEARNED_PATTERN" carries a groupItem.value. The cold-email branch in match-rules.ts now records matchReason.type as "LEARNED_PATTERN" for ai-already-labeled results but never stores a groupItem, so the only usable data is from; because extractFromEmail ignores the from field when type === "LEARNED_PATTERN", those executions are silently dropped from coldEmailers and the summary no longer shows repeated cold senders.


Finding type: Logical Bugs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 48f320c addressed this comment by modifying the extractFromEmail function to check for the from field as a fallback when type === "LEARNED_PATTERN" and no groupItem.value exists. This fixes the issue where cold-email executions with "LEARNED_PATTERN" type were being silently dropped when they only had a from field but no groupItem.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Updated extractFromEmail to be more robust and changed the match reason type back to AI where appropriate.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (8)
apps/web/utils/actions/cold-email.ts (1)

34-46: Use ruleId instead of hardcoded rule name.

The hardcoded ruleName: "Cold Email" is fragile. Cold email rules are identified by systemType: SystemType.COLD_EMAIL, not by name. If the rule doesn't have this exact name, saveLearnedPattern will silently fail (logs error and returns early), while labels are still removed—creating inconsistent state.

Consider fetching the cold email rule first and using its ruleId:

const coldEmailRule = await getColdEmailRule(emailAccountId);
if (!coldEmailRule) {
  throw new SafeError("Cold email rule not found");
}

await Promise.all([
  saveLearnedPattern({
    emailAccountId,
    from: sender,
    ruleId: coldEmailRule.id,
    exclude: true,
    logger,
    source: GroupItemSource.USER,
  }),
  removeColdEmailLabelFromSender(emailAccountId, emailProvider, sender),
]);
apps/web/utils/ai/choose-rule/match-rules.ts (1)

84-92: Critical: Type violation will cause runtime crash.

Using ConditionType.LEARNED_PATTERN without the required group and groupItem fields violates the LearnedPatternMatch type contract. When serializeMatchReasons (line 86 in types.ts) tries to serialize this, it will crash accessing reason.group.id and reason.groupItem.*:

case "LEARNED_PATTERN":
  return {
    type: "LEARNED_PATTERN",
    group: {
      id: reason.group.id,  // ❌ undefined
      name: reason.group.name,
    },
    // ...
  };

Use ConditionType.AI with the from field for both cases:

🔎 Proposed fix
         matches: [
           {
             rule: coldRule,
             matchReasons: [
               {
-                type:
-                  coldEmailResult.reason === "ai-already-labeled"
-                    ? ConditionType.LEARNED_PATTERN
-                    : ConditionType.AI,
+                type: ConditionType.AI,
                 from: message.headers.from,
               },
             ],
           },
         ],
apps/web/prisma/schema.prisma (1)

673-691: Consider removing the commented-out ColdEmail model after migration is complete.

The commented code adds schema noise. Once the migration script has been run in production and verified, consider deleting these lines rather than keeping them as dead code.

apps/web/app/api/google/webhook/process-label-removed-event.test.ts (1)

42-42: Remove the erroneous SOCIAL_PROMOTIONS mock property.

This property doesn't exist in the actual GmailLabel constant and appears to be a copy-paste error from the PROMOTIONS line above.

🔎 Proposed fix
     PROMOTIONS: "CATEGORY_PROMOTIONS",
-    SOCIAL_PROMOTIONS: "CATEGORY_PROMOTIONS",
     FORUMS: "CATEGORY_FORUMS",
apps/web/scripts/migrateColdEmailsToGroupItems.ts (1)

73-103: Validate fromEmail before upserting to prevent invalid writes.

If a ColdEmail record has a null or empty fromEmail, the upsert will either fail or create invalid data. Add a guard before the upsert.

🔎 Proposed fix
      const source =
        coldEmail.status === "USER_REJECTED_COLD"
          ? GroupItemSource.USER
          : GroupItemSource.AI;

+     if (!coldEmail.fromEmail) {
+       console.warn(
+         `Missing fromEmail for account ${emailAccountId}. Skipping record.`,
+       );
+       continue;
+     }
+
      // 5. Upsert GroupItem to avoid duplicates and preserve context
      await prisma.groupItem.upsert({
apps/web/utils/rule/learned-patterns.ts (1)

60-71: Race condition: Group creation lacks duplicate handling.

Unlike saveLearnedPatterns, this function doesn't handle the case where a concurrent request creates the group between the !groupId check and prisma.group.create. This could throw an error and break the learned pattern saving flow.

🔎 Proposed fix - wrap in try/catch similar to saveLearnedPatterns
   if (!groupId) {
     // Create a new group for this rule if one doesn't exist
-    const newGroup = await prisma.group.create({
-      data: {
-        emailAccountId,
-        name: rule.name,
-        rule: { connect: { id: rule.id } },
-      },
-    });
-
-    groupId = newGroup.id;
+    try {
+      const newGroup = await prisma.group.create({
+        data: {
+          emailAccountId,
+          name: rule.name,
+          rule: { connect: { id: rule.id } },
+        },
+      });
+      groupId = newGroup.id;
+    } catch (error) {
+      if (isDuplicateError(error)) {
+        // Refetch the rule to get the newly created groupId
+        const updatedRule = await prisma.rule.findUnique({
+          where: { id: rule.id },
+          select: { groupId: true },
+        });
+        if (!updatedRule?.groupId) {
+          logger.error("Failed to get groupId after duplicate error", { ruleId: rule.id });
+          return;
+        }
+        groupId = updatedRule.groupId;
+      } else {
+        logger.error("Error creating group", { error });
+        return;
+      }
+    }
   }
apps/web/app/api/resend/summary/route.ts (1)

71-86: extractFromEmail misses from for LEARNED_PATTERN without groupItem.

Based on the past review comment, when match-rules.ts records matchReason.type as "LEARNED_PATTERN" for ai-already-labeled results but doesn't store a groupItem, the only usable data is the from field. However, extractFromEmail ignores the from field when type === "LEARNED_PATTERN", causing those executions to be silently dropped from coldEmailers.

Consider also checking for a from field on LEARNED_PATTERN reasons, similar to how AI reasons are handled:

🔎 Proposed fix
 function extractFromEmail(matchMetadata: any): string | null {
   if (!matchMetadata || !Array.isArray(matchMetadata)) return null;

   const reasons = matchMetadata as SerializedMatchReason[];

   for (const reason of reasons) {
     if (reason.type === "AI" && reason.from) {
       return reason.from;
     }
     if (reason.type === "LEARNED_PATTERN" && reason.groupItem?.value) {
       return reason.groupItem.value;
     }
+    // Fallback: check if LEARNED_PATTERN has a 'from' field (for ai-already-labeled)
+    if (reason.type === "LEARNED_PATTERN" && (reason as any).from) {
+      return (reason as any).from;
+    }
   }

   return null;
 }
apps/web/utils/cold-email/is-cold-email.ts (1)

59-64: Semantic mismatch: "hasPreviousEmail" reason is misleading for explicitly excluded senders.

When a sender is explicitly excluded (patternMatch === "exclude"), returning reason: "hasPreviousEmail" is semantically incorrect. The log message says "Sender explicitly excluded from cold email blocker" but the reason suggests prior email communication. The UI will display misleading information like "This person has previously emailed you."

Consider adding a new reason type (e.g., "excluded") to ColdEmailBlockerReason to properly represent this case.

🔎 Proposed fix
-type ColdEmailBlockerReason = "hasPreviousEmail" | "ai" | "ai-already-labeled";
+type ColdEmailBlockerReason = "hasPreviousEmail" | "ai" | "ai-already-labeled" | "excluded";

   if (patternMatch === "exclude") {
     logger.info("Sender explicitly excluded from cold email blocker", {
       from: email.from,
     });
-    return { isColdEmail: false, reason: "hasPreviousEmail" };
+    return { isColdEmail: false, reason: "excluded" };
   }
🧹 Nitpick comments (1)
apps/web/app/api/resend/summary/route.ts (1)

211-221: Consider populating subject for cold emailers if available.

The subject field is always set to an empty string. If the summary email template displays subject lines for cold emailers, this would result in empty values. If subjects are needed, consider fetching them from the executed rule metadata or the associated message.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 8 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="apps/web/utils/rule/learned-patterns.ts">

<violation number="1" location="apps/web/utils/rule/learned-patterns.ts:104">
P2: Silent error swallowing without logging can make debugging difficult. Since `logger` is available, consider logging the failure for observability while still allowing the flow to continue.

(Based on your team&#39;s feedback about implementing internal try/catch and logging inside functions so failures don&#39;t impact the main flow.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/web/prisma/schema.prisma">

<violation number="1">
P0: Prisma schema validation will fail. The `ColdEmail` model is uncommented but its required relation fields in `EmailAccount` (`coldEmails ColdEmail[]`) and `DigestItem` (`coldEmail ColdEmail?`, `coldEmailId String?`) are still commented out. Relations must be defined on both sides.</violation>
</file>

<file name="apps/web/scripts/migrateColdEmailsToGroupItems.ts">

<violation number="1" location="apps/web/scripts/migrateColdEmailsToGroupItems.ts:74">
P2: Validation for `fromEmail` should happen before potentially creating a new group. If `fromEmail` is missing and the rule has no groupId, a new group will be created (lines 53-59), but the record will still be skipped, leaving an orphaned group in the database. Move this check to right after finding the rule (after line 47).</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

where: { id: rule.id },
data: { groupId },
})
.catch(() => {});
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Silent error swallowing without logging can make debugging difficult. Since logger is available, consider logging the failure for observability while still allowing the flow to continue.

(Based on your team's feedback about implementing internal try/catch and logging inside functions so failures don't impact the main flow.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/utils/rule/learned-patterns.ts, line 104:

<comment>Silent error swallowing without logging can make debugging difficult. Since `logger` is available, consider logging the failure for observability while still allowing the flow to continue.

(Based on your team&#39;s feedback about implementing internal try/catch and logging inside functions so failures don&#39;t impact the main flow.) </comment>

<file context>
@@ -59,15 +59,58 @@ export async function saveLearnedPattern({
+                where: { id: rule.id },
+                data: { groupId },
+              })
+              .catch(() =&gt; {});
+          } else {
+            // If we still don&#39;t have a groupId, rethrow
</file context>
Suggested change
.catch(() => {});
.catch((e) => logger.warn("Failed to link group to rule", { error: e, ruleId: rule.id, groupId }));

✅ Addressed in e980e31

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit f0ce85b addressed this comment by completely removing the problematic code section that contained the silent error swallowing (.catch(() => {})). The entire complex error handling logic for group creation and linking has been refactored out of the function, eliminating the concern about unlogged failures.

: GroupItemSource.AI;

// 5. Upsert GroupItem to avoid duplicates and preserve context
if (!coldEmail.fromEmail) {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Validation for fromEmail should happen before potentially creating a new group. If fromEmail is missing and the rule has no groupId, a new group will be created (lines 53-59), but the record will still be skipped, leaving an orphaned group in the database. Move this check to right after finding the rule (after line 47).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/scripts/migrateColdEmailsToGroupItems.ts, line 74:

<comment>Validation for `fromEmail` should happen before potentially creating a new group. If `fromEmail` is missing and the rule has no groupId, a new group will be created (lines 53-59), but the record will still be skipped, leaving an orphaned group in the database. Move this check to right after finding the rule (after line 47).</comment>

<file context>
@@ -71,6 +71,13 @@ async function main() {
           : GroupItemSource.AI;
 
       // 5. Upsert GroupItem to avoid duplicates and preserve context
+      if (!coldEmail.fromEmail) {
+        console.warn(
+          `Missing sender email for account ${emailAccountId}. Skipping record.`,
</file context>

✅ Addressed in 982a413

Comment on lines 53 to 64
// 3. Create a group if the rule doesn't have one associated yet
if (!groupId) {
const newGroup = await prisma.group.create({
data: {
emailAccountId,
name: rule.name,
rule: { connect: { id: rule.id } },
},
});
groupId = newGroup.id;
console.log(`Created new group for account ${emailAccountId}`);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we create a new group for a Cold Email rule that had no groupId, we stop after inserting the Group, but never update the rule to point to that group. All later runtime logic still sees rule.groupId as null and continues to fall back to the old ColdEmail table instead of using the new GroupItems, so the migration cannot make any of the rules go through the new path.


Finding type: Logical Bugs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 982a413 addressed this comment by completely removing the migration script file apps/web/scripts/migrateColdEmailsToGroupItems.ts. Since the entire file containing the logical bug has been deleted, the issue where rules weren't updated to point to newly created groups no longer exists in the codebase.

@elie222 elie222 changed the title Cold Email: Improve handling and AI rule execution Cold Email: refactor to use learned patterns Jan 3, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
apps/web/utils/cold-email/is-cold-email.ts (1)

19-19: Type mismatch: "excluded" reason not in ColdEmailBlockerReason.

Line 63 returns reason: "excluded" but the ColdEmailBlockerReason type at line 19 doesn't include "excluded". This should cause a TypeScript error (unless the pipeline failure is masking it).

🔎 Fix to add "excluded" to the type
-type ColdEmailBlockerReason = "hasPreviousEmail" | "ai" | "ai-already-labeled";
+type ColdEmailBlockerReason = "hasPreviousEmail" | "ai" | "ai-already-labeled" | "excluded";
apps/web/app/(app)/[emailAccountId]/assistant/settings/WritingStyleSetting.tsx (1)

117-126: Fix placeholder string syntax: use template literals for multi-line string.

The placeholder attribute uses double quotes for a string that spans multiple lines (117-126), which is invalid JavaScript syntax. Multi-line strings require either template literals (backticks) or escaped newlines (\n). Change the placeholder to use backticks:

Fix
placeholder={`Typical Length: 2-3 sentences

Formality: Informal but professional

Common Greeting: Hey,

Notable Traits:
- Uses contractions frequently
- Concise and direct responses
- Minimal closings`}
♻️ Duplicate comments (5)
apps/web/scripts/migrateColdEmailsToGroupItems.ts (2)

53-64: Critical: Update rule.groupId after creating new group.

When creating a new group for a rule that has no groupId, the code never updates the rule to reference this group. The connect clause on line 59 only establishes the relationship from Group→Rule, but rule.groupId remains null in the database.

This means runtime logic checking rule.groupId will still see null and won't use the newly migrated GroupItems, defeating the migration's purpose.

🔎 Proposed fix
       // 3. Create a group if the rule doesn't have one associated yet
       if (!groupId) {
         const newGroup = await prisma.group.create({
           data: {
             emailAccountId,
             name: rule.name,
             rule: { connect: { id: rule.id } },
           },
         });
         groupId = newGroup.id;
+        // Update the rule to point to the new group
+        await prisma.rule.update({
+          where: { id: rule.id },
+          data: { groupId: newGroup.id },
+        });
         console.log(`Created new group for account ${emailAccountId}`);
       }

74-79: Move validation before group creation to prevent orphaned groups.

If fromEmail is missing and the rule has no groupId, a new group will be created (lines 53-64), but then this validation will skip the record, leaving an orphaned group in the database with no GroupItems.

Move this check immediately after finding the rule (after line 49, before the group creation logic).

🔎 Proposed fix
       if (!rule) {
         console.warn(
           `No Cold Email rule found for account ${emailAccountId}. Skipping sender: ${coldEmail.fromEmail}`,
         );
         continue;
       }
+
+      // Validate required fields before any database operations
+      if (!coldEmail.fromEmail) {
+        console.warn(
+          `Missing sender email for account ${emailAccountId}. Skipping record.`,
+        );
+        continue;
+      }
 
       let groupId = rule.groupId;
 
       // 3. Create a group if the rule doesn't have one associated yet
       if (!groupId) {
         const newGroup = await prisma.group.create({
           data: {
             emailAccountId,
             name: rule.name,
             rule: { connect: { id: rule.id } },
           },
         });
         groupId = newGroup.id;
         console.log(`Created new group for account ${emailAccountId}`);
       }
 
       // 4. Map the old status to the new exclude/source system
       const exclude = coldEmail.status === "USER_REJECTED_COLD";
       const source =
         coldEmail.status === "USER_REJECTED_COLD"
           ? GroupItemSource.USER
           : GroupItemSource.AI;
 
       // 5. Upsert GroupItem to avoid duplicates and preserve context
-      if (!coldEmail.fromEmail) {
-        console.warn(
-          `Missing sender email for account ${emailAccountId}. Skipping record.`,
-        );
-        continue;
-      }
-
       await prisma.groupItem.upsert({
apps/web/utils/rule/learned-patterns.ts (1)

99-104: Add logging when group-to-rule linking fails.

The .catch(() => {}) silently swallows errors, which makes debugging difficult. Since logger is available, consider logging the failure for observability while still allowing the flow to continue.

🔎 Suggested fix to add logging
             // Attempt to link it, but ignore if it fails (e.g. already linked to another rule)
             await prisma.rule
               .update({
                 where: { id: rule.id },
                 data: { groupId },
               })
-              .catch(() => {});
+              .catch((e) => logger.warn("Failed to link group to rule", { error: e, ruleId: rule.id, groupId }));
apps/web/prisma/schema.prisma (2)

280-285: Critical: Uncomment DigestItem relation fields to fix Prisma validation.

Similarly, the ColdEmail model defines digestItems DigestItem[] but the reverse relation in DigestItem is commented out, causing the second pipeline error.

🔎 Fix to uncomment the relation fields
   action      ExecutedAction? @relation(fields: [actionId], references: [id], onDelete: Cascade)
-  // coldEmailId String?
-  // coldEmail   ColdEmail?      @relation(fields: [coldEmailId], references: [id])
+  coldEmailId String?
+  coldEmail   ColdEmail?      @relation(fields: [coldEmailId], references: [id])

   @@unique([digestId, threadId, messageId])
   @@index([actionId])
-  // @@index([coldEmailId])
+  @@index([coldEmailId])

159-159: Critical: Uncomment relation field to fix Prisma schema validation.

The ColdEmail model at line 673 is uncommented and references emailAccount EmailAccount @relation(...), but the reverse relation coldEmails ColdEmail[] is still commented out here. Prisma requires relations to be defined on both sides.

This is causing the pipeline failure: The relation field 'emailAccount' on model 'ColdEmail' is missing an opposite relation field on the model 'EmailAccount'.

🔎 Fix to uncomment the relation field
   newsletters      Newsletter[]
-  // coldEmails       ColdEmail[]
+  coldEmails       ColdEmail[]
   groups           Group[]
🧹 Nitpick comments (4)
apps/web/app/api/google/webhook/process-label-removed-event.test.ts (1)

14-23: Consider using the standard Prisma mock for consistency.

The coding guidelines recommend using "the provided mock from @/utils/__mocks__/prisma" rather than inline mocks. While this custom mock works for these specific tests, using the standard mock would improve consistency across the test suite.

📖 Recommended approach
-vi.mock("@/utils/prisma", () => ({
-  default: {
-    rule: {
-      findUnique: vi.fn(),
-    },
-    groupItem: {
-      deleteMany: vi.fn(),
-    },
-  },
-}));
+vi.mock("@/utils/prisma");
+import prisma from "@/utils/__mocks__/prisma";

Then access the mocked methods directly:

prisma.rule.findUnique
prisma.groupItem.deleteMany

As per coding guidelines for test files.

apps/web/scripts/migrateColdEmailsToGroupItems.ts (1)

1-2: Add note about schema state for migration execution.

The pipeline failure indicates Prisma generate is failing. This is expected if the ColdEmail model has been removed from the schema before running this migration.

Consider adding a note to the execution instructions about the expected schema state.

-// Run with: `npx tsx scripts/migrateColdEmailsToGroupItems.ts`. Make sure to set ENV vars
+// Run with: `npx tsx scripts/migrateColdEmailsToGroupItems.ts`. Make sure to set ENV vars.
+// Note: Run this BEFORE removing the ColdEmail model from the Prisma schema.
apps/web/app/api/resend/summary/route.ts (1)

71-95: Consider using a type guard for safer type assertion.

The type assertion as SerializedMatchReason[] at line 74 bypasses TypeScript's type checking. If matchMetadata contains unexpected data, this could lead to runtime errors when accessing properties like reason.type.

🔎 Suggested improvement with runtime validation
 function extractFromEmail(matchMetadata: unknown): string | null {
   if (!matchMetadata || !Array.isArray(matchMetadata)) return null;
 
-  const reasons = matchMetadata as SerializedMatchReason[];
-
-  for (const reason of reasons) {
+  for (const reason of matchMetadata) {
+    if (!reason || typeof reason !== "object" || !("type" in reason)) continue;
+
     if (reason.type === "AI" && reason.from) {
       return reason.from;
     }
     if (reason.type === "LEARNED_PATTERN") {
-      const serializedReason = reason as SerializedMatchReason & {
-        from?: string;
-        groupItem?: { value: string };
-      };
-      if (serializedReason.groupItem?.value) {
-        return serializedReason.groupItem.value;
+      const groupItem = (reason as { groupItem?: { value?: string } }).groupItem;
+      if (groupItem?.value) {
+        return groupItem.value;
       }
-      if (serializedReason.from) {
-        return serializedReason.from;
+      const from = (reason as { from?: string }).from;
+      if (from) {
+        return from;
       }
     }
   }
 
   return null;
 }
apps/web/utils/cold-email/is-cold-email.ts (1)

48-52: Remove redundant ?? undefined.

coldEmailRule?.groupId already evaluates to undefined when coldEmailRule is null/undefined or when groupId is undefined. The ?? undefined is redundant.

🔎 Simplified code
   const patternMatch = await checkColdEmailPattern({
     from: email.from,
     emailAccountId: emailAccount.id,
-    groupId: coldEmailRule?.groupId ?? undefined,
+    groupId: coldEmailRule?.groupId,
   });

Comment on lines 12 to 120
async function main() {
console.log("Starting migration of ColdEmail records to GroupItem...");

// 1. Fetch all ColdEmail records using raw query since the model is commented out in Prisma
// We use double quotes to handle case-sensitivity in Postgres
let coldEmails: any[] = [];
try {
coldEmails = await prisma.$queryRawUnsafe(`SELECT * FROM "ColdEmail"`);
} catch (error) {
console.error(
"Could not find ColdEmail table. It may have already been deleted or renamed.",
);
return;
}

console.log(`Found ${coldEmails.length} records to migrate.`);

for (const coldEmail of coldEmails) {
try {
const emailAccountId = coldEmail.emailAccountId;

// 2. Find the Cold Email rule for this account
const rule = await prisma.rule.findUnique({
where: {
emailAccountId_systemType: {
emailAccountId,
systemType: SystemType.COLD_EMAIL,
},
},
select: { id: true, groupId: true, name: true },
});

if (!rule) {
console.warn(
`No Cold Email rule found for account ${emailAccountId}. Skipping sender: ${coldEmail.fromEmail}`,
);
continue;
}

let groupId = rule.groupId;

// 3. Create a group if the rule doesn't have one associated yet
if (!groupId) {
const newGroup = await prisma.group.create({
data: {
emailAccountId,
name: rule.name,
rule: { connect: { id: rule.id } },
},
});
groupId = newGroup.id;
console.log(`Created new group for account ${emailAccountId}`);
}

// 4. Map the old status to the new exclude/source system
const exclude = coldEmail.status === "USER_REJECTED_COLD";
const source =
coldEmail.status === "USER_REJECTED_COLD"
? GroupItemSource.USER
: GroupItemSource.AI;

// 5. Upsert GroupItem to avoid duplicates and preserve context
if (!coldEmail.fromEmail) {
console.warn(
`Missing sender email for account ${emailAccountId}. Skipping record.`,
);
continue;
}

await prisma.groupItem.upsert({
where: {
groupId_type_value: {
groupId,
type: GroupItemType.FROM,
value: coldEmail.fromEmail,
},
},
update: {
exclude,
reason: coldEmail.reason,
threadId: coldEmail.threadId,
messageId: coldEmail.messageId,
source,
createdAt: coldEmail.createdAt,
updatedAt: coldEmail.updatedAt,
},
create: {
groupId,
type: GroupItemType.FROM,
value: coldEmail.fromEmail,
exclude,
reason: coldEmail.reason,
threadId: coldEmail.threadId,
messageId: coldEmail.messageId,
source,
createdAt: coldEmail.createdAt,
updatedAt: coldEmail.updatedAt,
},
});
} catch (error) {
console.error(
`Error migrating record for ${coldEmail.fromEmail}:`,
error,
);
}
}

console.log("Migration finished successfully.");
}
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Use createScopedLogger instead of console methods.

The coding guidelines specify using createScopedLogger for standalone scripts. This migration script should use structured logging instead of console methods.

🔎 Proposed refactor

Add at the top of the file after imports:

 import {
   SystemType,
   GroupItemType,
   GroupItemSource,
 } from "@/generated/prisma/enums";
+import { createScopedLogger } from "@/utils/logger";
 
 const prisma = new PrismaClient();
+const logger = createScopedLogger("migrate-cold-emails");

Then replace console calls with logger methods:

-  console.log("Starting migration of ColdEmail records to GroupItem...");
+  logger.info("Starting migration of ColdEmail records to GroupItem...");

-  console.error(
-    "Could not find ColdEmail table. It may have already been deleted or renamed.",
-  );
+  logger.error("Could not find ColdEmail table. It may have already been deleted or renamed.");

-  console.log(`Found ${coldEmails.length} records to migrate.`);
+  logger.info({ count: coldEmails.length }, "Found records to migrate");

-  console.warn(
-    `No Cold Email rule found for account ${emailAccountId}. Skipping sender: ${coldEmail.fromEmail}`,
-  );
+  logger.warn({ emailAccountId, sender: coldEmail.fromEmail }, "No Cold Email rule found, skipping");

-  console.log(`Created new group for account ${emailAccountId}`);
+  logger.info({ emailAccountId, groupId }, "Created new group");

-  console.warn(
-    `Missing sender email for account ${emailAccountId}. Skipping record.`,
-  );
+  logger.warn({ emailAccountId }, "Missing sender email, skipping record");

-  console.error(
-    `Error migrating record for ${coldEmail.fromEmail}:`,
-    error,
-  );
+  logger.error({ sender: coldEmail.fromEmail, error }, "Error migrating record");

-  console.log("Migration finished successfully.");
+  logger.info("Migration finished successfully");

As per coding guidelines, standalone scripts should use structured logging with createScopedLogger.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 982a413 addressed this comment by completely removing the migration script file apps/web/scripts/migrateColdEmailsToGroupItems.ts. Since the file containing all the console method calls that needed to be replaced with structured logging no longer exists, the refactoring concern has been resolved.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipped: comment is from another GitHub bot.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3e28d30 and 982a413.

📒 Files selected for processing (2)
  • apps/web/prisma/migrations/20260103000000_migrate_cold_emails_to_group_items/migration.sql
  • apps/web/prisma/schema.prisma
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/prisma/schema.prisma
🧰 Additional context used
📓 Path-based instructions (1)
!(pages/_document).{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

Don't use the next/head module in pages/_document.js on Next.js projects

Files:

  • apps/web/prisma/migrations/20260103000000_migrate_cold_emails_to_group_items/migration.sql
⏰ 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: Baz Reviewer
  • GitHub Check: test
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/web/prisma/migrations/20260103000000_migrate_cold_emails_to_group_items/migration.sql (3)

1-2: LGTM!

The enum creation is straightforward and correctly defines the two source types needed for attribution.


4-8: LGTM!

The new columns are appropriately nullable for backward compatibility with existing GroupItem records.


54-96: No issues found — schema constraints prevent the scenario described.

The Rule model has a @unique constraint on groupId, preventing multiple rules from having different groups. Therefore, the concern about multiple COLD_EMAIL rules per emailAccountId creating duplicate GroupItem entries across different groups cannot occur by design.

The status mapping is also correct and complete. The ColdEmailStatus enum contains three values: AI_LABELED_COLD, USER_REJECTED_COLD, and NOT_COLD_EMAIL. The migration properly handles all three:

  • USER_REJECTED_COLDexclude=true, source='USER'
  • Other statuses (AI_LABELED_COLD, NOT_COLD_EMAIL) → exclude=false, source='AI'

This matches the semantics in the codebase (e.g., app/api/user/cold-email/route.ts).

Likely an incorrect or invalid review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 982a413 and 0a13926.

📒 Files selected for processing (4)
  • .vscode/settings.json
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/prisma/schema.prisma
  • apps/web/utils/rule/consts.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/prisma/schema.prisma
🧰 Additional context used
📓 Path-based instructions (17)
!(pages/_document).{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

Don't use the next/head module in pages/_document.js on Next.js projects

Files:

  • .vscode/settings.json
  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)

**/*.{ts,tsx}: For API GET requests to server, use the swr package
Use result?.serverError with toastError from @/components/Toast for error handling in async operations

**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls

**/*.{ts,tsx}: For early access feature flags, create hooks using the naming convention use[FeatureName]Enabled that return a boolean from useFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming convention use[FeatureName]Variant that define variant types, use useFeatureFlagVariantKey() with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g., inbox-cleaner, pricing-options-2)
Always define types for A/B test variant flags (e.g., type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
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 TypeScript namespaces
Don't use non-null assertions with the ! postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Use as const instead of literal types and type annotations
Use either T[] or Array<T> consistently
Initialize each enum member value explicitly
Use export type for types
Use `impo...

Files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)

Always import Prisma enums from @/generated/prisma/enums instead of @/generated/prisma/client to avoid Next.js bundling errors in client components

Import Prisma using the project's centralized utility: import prisma from '@/utils/prisma'

Files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Do not export types/interfaces that are only used within the same file. Export later if needed

Files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/security.mdc)

**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma's select option. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. All findUnique/findFirst calls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
All findMany queries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g., emailAccount: { id: emailAccountId }) to validate ownership

Files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Use next/image package for images
For API GET requests to server, use the swr package with hooks like useSWR to fetch data
For text inputs, use the Input component with registerProps for form integration and error handling

Files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/*.{tsx,ts,css}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

Implement responsive design with Tailwind CSS using a mobile-first approach

Files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{js,jsx,ts,tsx}: Don't use accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like <marquee> or <blink>
Only use the scope prop on <th> elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex property
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include a title element for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
Assign tabIndex to non-interactive HTML elements with aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include a type attribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden)
Always include a lang attribute on the html element
Always include a title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...

Files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)

**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

Files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/{utils,helpers,lib}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)

Logger should be passed as a parameter to helper functions instead of creating their own logger instances

Files:

  • apps/web/utils/rule/consts.ts
apps/web/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

apps/web/**/*.{ts,tsx,js,jsx}: Use @/ path aliases for imports from project root
Prefer self-documenting code over comments; use descriptive variable and function names instead of explaining intent with comments
Add helper functions to the bottom of files, not the top
All imports go at the top of files, no mid-file dynamic imports

Files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
apps/web/**/*.{ts,tsx,js,jsx,json,css}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

Format code with Prettier

Files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
apps/web/**/*.{example,ts,json}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

Add environment variables to .env.example, env.ts, and turbo.json

Files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
apps/web/app/api/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/security-audit.mdc)

apps/web/app/api/**/*.{ts,tsx}: API routes must use withAuth, withEmailAccount, or withError middleware for authentication
All database queries must include user scoping with emailAccountId or userId filtering in WHERE clauses
Request parameters must be validated before use; avoid direct parameter usage without type checking
Use generic error messages instead of revealing internal details; throw SafeError instead of exposing user IDs, resource IDs, or system information
API routes should only return necessary fields using select in database queries to prevent unintended information disclosure
Cron endpoints must use hasCronSecret or hasPostCronSecret to validate cron requests and prevent unauthorized access
Request bodies should use Zod schemas for validation to ensure type safety and prevent injection attacks

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/app/api/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/security.mdc)

**/app/api/**/*.ts: ALL API routes that handle user data MUST use appropriate middleware: use withEmailAccount for email-scoped operations, use withAuth for user-scoped operations, or use withError with proper validation for public/custom auth endpoints
Use withEmailAccount middleware for operations scoped to a specific email account, including reading/writing emails, rules, schedules, or any operation using emailAccountId
Use withAuth middleware for user-level operations such as user settings, API keys, and referrals that use only userId
Use withError middleware only for public endpoints, custom authentication logic, or cron endpoints. For cron endpoints, MUST use hasCronSecret() or hasPostCronSecret() validation
Cron endpoints without proper authentication can be triggered by anyone. CRITICAL: All cron endpoints MUST validate cron secret using hasCronSecret(request) or hasPostCronSecret(request) and capture unauthorized attempts with captureException()
Always validate request bodies using Zod schemas to ensure type safety and prevent invalid data from reaching database operations
Maintain consistent error response format across all API routes to avoid information disclosure while providing meaningful error feedback

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
apps/web/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

Follow NextJS app router structure with (app) directory

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
apps/web/app/api/**/*.ts

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

apps/web/app/api/**/*.ts: Create GET API routes wrapped with withAuth or withEmailAccount middleware for fetching data
Export response types from GET API routes using export type GetXResponse = Awaited<ReturnType<typeof getData>>

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
🧠 Learnings (16)
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Use TypeScript types for all LLM function parameters and return values, and define clear interfaces for complex input/output structures

Applied to files:

  • apps/web/utils/rule/consts.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : System prompts must define the LLM's role and task specifications

Applied to files:

  • apps/web/utils/rule/consts.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : LLM feature functions must import from `zod` for schema validation, use `createScopedLogger` from `@/utils/logger`, `chatCompletionObject` and `createGenerateObject` from `@/utils/llms`, and import `EmailAccountWithAI` type from `@/utils/llms/types`

Applied to files:

  • apps/web/utils/rule/consts.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-11-25T14:39:23.326Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to app/api/**/*.ts : Use `SafeError` for error responses to prevent information disclosure - provide generic messages (e.g., 'Rule not found' not 'Rule {id} does not exist for user {userId}') without revealing internal IDs or ownership details

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Prefer using existing helpers from `@/__tests__/helpers.ts` (`getEmailAccount`, `getEmail`, `getRule`, `getMockMessage`, `getMockExecutedRule`) instead of creating custom test data helpers

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : LLM feature functions must follow a standard structure: accept options with `inputData` and `emailAccount` parameters, implement input validation with early returns, define separate system and user prompts, create a Zod schema for response validation, and use `createGenerateObject` to execute the LLM call

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-12-31T23:49:09.597Z
Learnt from: rsnodgrass
Repo: elie222/inbox-zero PR: 1154
File: apps/web/app/api/user/setup-progress/route.ts:0-0
Timestamp: 2025-12-31T23:49:09.597Z
Learning: In apps/web/app/api/user/setup-progress/route.ts, Reply Zero enabled status should be determined solely by checking if the TO_REPLY rule is enabled, as it is the critical/canonical rule that Reply Zero is based on. The other conversation status types (FYI, AWAITING_REPLY, ACTIONED) should not be checked for determining Reply Zero setup progress.
<!-- </add_learning>

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to apps/web/utils/gmail/**/*.{ts,tsx} : Always use wrapper functions from @/utils/gmail/ for Gmail API operations instead of direct provider API calls

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to apps/web/utils/gmail/**/*.{ts,tsx} : Keep Gmail provider-specific implementation details isolated within the apps/web/utils/gmail/ directory

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/{pages,routes,components}/**/*.{ts,tsx} : Never call Gmail API directly from routes or components - always use wrapper functions from the utils folder

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-11-25T14:42:11.919Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-25T14:42:11.919Z
Learning: Applies to utils/**/*.{js,ts,jsx,tsx} : The `utils` folder contains core app logic such as Next.js Server Actions and Gmail API requests

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2026-01-01T10:42:29.775Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-01-01T10:42:29.775Z
Learning: Applies to **/*.test.{ts,tsx} : Use test helper functions `getEmail`, `getEmailAccount`, and `getRule` from `@/__tests__/helpers` for creating mock data

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
📚 Learning: 2025-11-25T14:42:16.602Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-25T14:42:16.602Z
Learning: The `utils` folder contains core app logic such as Next.js Server Actions and Gmail API requests

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
🧬 Code graph analysis (1)
apps/web/app/api/google/webhook/process-label-removed-event.ts (3)
apps/web/utils/logger.ts (1)
  • Logger (6-6)
apps/web/utils/rule/consts.ts (1)
  • shouldLearnFromLabelRemoval (138-140)
apps/web/utils/rule/learned-patterns.ts (1)
  • saveLearnedPattern (13-142)
⏰ 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). (4)
  • GitHub Check: Baz Reviewer
  • GitHub Check: test
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Macroscope - Correctness Check
🔇 Additional comments (6)
apps/web/utils/rule/consts.ts (1)

17-140: LGTM! Clean addition of learning configuration.

The new shouldLearn property and shouldLearnFromLabelRemoval helper are well-designed. The flag values make semantic sense—conversation-state rules (TO_REPLY, FYI, etc.) don't learn from label removal, while categorization rules (NEWSLETTER, MARKETING, COLD_EMAIL, etc.) do. The helper provides a clean API for querying this configuration.

apps/web/app/api/google/webhook/process-label-removed-event.ts (5)

2-10: LGTM! Imports follow project conventions.

The imports correctly use the centralized Prisma utility and import enums from the designated path, as per coding guidelines.


119-136: LGTM! Clean refactor to ID-based processing.

The loop correctly iterates over label IDs and properly handles errors for each label removal event.


139-160: LGTM! Proper parameter refactor and early validation.

The function signature correctly uses labelId, enriches logging context appropriately, and handles the missing-sender edge case with an early return.


163-182: Well-scoped query with proper validation.

The database query correctly scopes to emailAccountId and only selects necessary fields. The logic properly validates that a learnable rule was found before proceeding. The query runs inside the loop (one per removed label), but given label removal events are infrequent, this is an acceptable trade-off.


184-198: Excellent! Past review issue fully resolved.

The saveLearnedPattern call now uses rule.id dynamically instead of a hardcoded rule name, addressing the previous review comment. The function is called with proper provenance tracking (messageId, threadId, reason, source), and the logger is correctly passed as a parameter per coding guidelines.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
apps/web/utils/rule/learned-patterns.ts (1)

48-99: Add logging to error handler for observability.

Line 90 silently swallows all errors without logging, making debugging difficult. Since logger is available, log the failure while still allowing the flow to continue.

🔎 Proposed fix
             // Attempt to link it, but ignore if it fails (e.g. already linked to another rule)
             await prisma.rule
               .update({
                 where: { id: rule.id },
                 data: { groupId },
               })
-              .catch(() => {});
+              .catch((e) => logger.warn("Failed to link group to rule", { error: e, ruleId: rule.id, groupId }));
           } else {

Based on learnings, internal helper functions should implement try/catch with logging so failures don't silently impact the main flow.

🧹 Nitpick comments (1)
apps/web/utils/cold-email/is-cold-email.ts (1)

55-55: Remove redundant || undefined operator.

The optional chaining coldEmailRule?.groupId already returns undefined when the value is nullish, making the || undefined operator redundant.

🔎 Proposed fix
   const patternMatch = await checkColdEmailPattern({
     from: email.from,
     emailAccountId: emailAccount.id,
-    groupId: coldEmailRule?.groupId || undefined,
+    groupId: coldEmailRule?.groupId,
   });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 87c4357 and f0ce85b.

📒 Files selected for processing (5)
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/actions/cold-email.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
🧰 Additional context used
📓 Path-based instructions (26)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)

**/*.{ts,tsx}: For API GET requests to server, use the swr package
Use result?.serverError with toastError from @/components/Toast for error handling in async operations

**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls

**/*.{ts,tsx}: For early access feature flags, create hooks using the naming convention use[FeatureName]Enabled that return a boolean from useFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming convention use[FeatureName]Variant that define variant types, use useFeatureFlagVariantKey() with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g., inbox-cleaner, pricing-options-2)
Always define types for A/B test variant flags (e.g., type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
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 TypeScript namespaces
Don't use non-null assertions with the ! postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Use as const instead of literal types and type annotations
Use either T[] or Array<T> consistently
Initialize each enum member value explicitly
Use export type for types
Use `impo...

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
apps/web/utils/actions/*.ts

📄 CodeRabbit inference engine (.cursor/rules/fullstack-workflow.mdc)

apps/web/utils/actions/*.ts: Use next-safe-action with Zod schemas for all server actions (create/update/delete mutations), storing validation schemas in apps/web/utils/actions/*.validation.ts
Server actions should use 'use server' directive and automatically receive authentication context (emailAccountId) from the actionClient

apps/web/utils/actions/*.ts: Create corresponding server action implementation files using the naming convention apps/web/utils/actions/NAME.ts with 'use server' directive
Use 'use server' directive at the top of server action implementation files
Implement all server actions using the next-safe-action library with actionClient, actionClientUser, or adminActionClient for type safety and validation
Use actionClientUser when only authenticated user context (userId) is needed
Use actionClient when both authenticated user context and a specific emailAccountId are needed, with emailAccountId bound when calling from the client
Use adminActionClient for actions restricted to admin users
Add metadata with a meaningful action name using .metadata({ name: "actionName" }) for Sentry instrumentation and monitoring
Use .schema() method with Zod validation schemas from corresponding .validation.ts files in next-safe-action configuration
Access context (userId, emailAccountId, etc.) via the ctx object parameter in the .action() handler
Use revalidatePath or revalidateTag from 'next/cache' within server action handlers when mutations modify data displayed elsewhere

Files:

  • apps/web/utils/actions/cold-email.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)

Always import Prisma enums from @/generated/prisma/enums instead of @/generated/prisma/client to avoid Next.js bundling errors in client components

Import Prisma using the project's centralized utility: import prisma from '@/utils/prisma'

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
apps/web/utils/actions/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

apps/web/utils/actions/**/*.ts: Server actions must be located in apps/web/utils/actions folder
Server action files must start with use server directive

apps/web/utils/actions/**/*.ts: Use proper error handling with try/catch blocks
Use next-safe-action with Zod schemas for server actions to handle mutations
Use revalidatePath in server actions for cache invalidation after mutations

Files:

  • apps/web/utils/actions/cold-email.ts
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Do not export types/interfaces that are only used within the same file. Export later if needed

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/security.mdc)

**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma's select option. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. All findUnique/findFirst calls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
All findMany queries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g., emailAccount: { id: emailAccountId }) to validate ownership

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Use next/image package for images
For API GET requests to server, use the swr package with hooks like useSWR to fetch data
For text inputs, use the Input component with registerProps for form integration and error handling

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
**/*.{tsx,ts,css}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

Implement responsive design with Tailwind CSS using a mobile-first approach

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{js,jsx,ts,tsx}: Don't use accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like <marquee> or <blink>
Only use the scope prop on <th> elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex property
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include a title element for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
Assign tabIndex to non-interactive HTML elements with aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include a type attribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden)
Always include a lang attribute on the html element
Always include a title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
!(pages/_document).{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

Don't use the next/head module in pages/_document.js on Next.js projects

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)

**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
**/{utils,helpers,lib}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)

Logger should be passed as a parameter to helper functions instead of creating their own logger instances

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
apps/web/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

apps/web/**/*.{ts,tsx,js,jsx}: Use @/ path aliases for imports from project root
Prefer self-documenting code over comments; use descriptive variable and function names instead of explaining intent with comments
Add helper functions to the bottom of files, not the top
All imports go at the top of files, no mid-file dynamic imports

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
apps/web/**/*.{ts,tsx,js,jsx,json,css}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

Format code with Prettier

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
apps/web/utils/actions/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

Infer types from Zod schemas using z.infer<typeof schema> instead of duplicating as separate interfaces

Files:

  • apps/web/utils/actions/cold-email.ts
apps/web/**/*.{example,ts,json}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

Add environment variables to .env.example, env.ts, and turbo.json

Files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
apps/web/app/api/**/route.ts

📄 CodeRabbit inference engine (.cursor/rules/fullstack-workflow.mdc)

apps/web/app/api/**/route.ts: Create GET API routes using withAuth or withEmailAccount middleware in apps/web/app/api/*/route.ts, export response types as GetExampleResponse type alias for client-side type safety
Always export response types from GET routes as Get[Feature]Response using type inference from the data fetching function for type-safe client consumption
Do NOT use POST API routes for mutations - always use server actions with next-safe-action instead

Files:

  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
**/app/**/route.ts

📄 CodeRabbit inference engine (.cursor/rules/get-api-route.mdc)

**/app/**/route.ts: Always wrap GET API route handlers with withAuth or withEmailAccount middleware for consistent error handling and authentication in Next.js App Router
Infer and export response type for GET API routes using Awaited<ReturnType<typeof functionName>> pattern in Next.js
Use Prisma for database queries in GET API routes
Return responses using NextResponse.json() in GET API routes
Do not use try/catch blocks in GET API route handlers when using withAuth or withEmailAccount middleware, as the middleware handles error handling

Files:

  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
apps/web/app/**/[!.]*/route.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Use kebab-case for route directories in Next.js App Router (e.g., api/hello-world/route)

Files:

  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
apps/web/app/api/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/security-audit.mdc)

apps/web/app/api/**/*.{ts,tsx}: API routes must use withAuth, withEmailAccount, or withError middleware for authentication
All database queries must include user scoping with emailAccountId or userId filtering in WHERE clauses
Request parameters must be validated before use; avoid direct parameter usage without type checking
Use generic error messages instead of revealing internal details; throw SafeError instead of exposing user IDs, resource IDs, or system information
API routes should only return necessary fields using select in database queries to prevent unintended information disclosure
Cron endpoints must use hasCronSecret or hasPostCronSecret to validate cron requests and prevent unauthorized access
Request bodies should use Zod schemas for validation to ensure type safety and prevent injection attacks

Files:

  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
**/app/api/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/security.mdc)

**/app/api/**/*.ts: ALL API routes that handle user data MUST use appropriate middleware: use withEmailAccount for email-scoped operations, use withAuth for user-scoped operations, or use withError with proper validation for public/custom auth endpoints
Use withEmailAccount middleware for operations scoped to a specific email account, including reading/writing emails, rules, schedules, or any operation using emailAccountId
Use withAuth middleware for user-level operations such as user settings, API keys, and referrals that use only userId
Use withError middleware only for public endpoints, custom authentication logic, or cron endpoints. For cron endpoints, MUST use hasCronSecret() or hasPostCronSecret() validation
Cron endpoints without proper authentication can be triggered by anyone. CRITICAL: All cron endpoints MUST validate cron secret using hasCronSecret(request) or hasPostCronSecret(request) and capture unauthorized attempts with captureException()
Always validate request bodies using Zod schemas to ensure type safety and prevent invalid data from reaching database operations
Maintain consistent error response format across all API routes to avoid information disclosure while providing meaningful error feedback

Files:

  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
**/{app,pages}/**/{route,+page}.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)

**/{app,pages}/**/{route,+page}.{ts,tsx}: Use middleware wrappers (withError, withAuth, withEmailAccount, withEmailProvider) that automatically create loggers with request context in API routes
Enrich logger context within route handlers using logger.with() to add request-specific fields like messageId

Files:

  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
apps/web/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

Follow NextJS app router structure with (app) directory

Files:

  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
apps/web/app/api/**/*.ts

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

apps/web/app/api/**/*.ts: Create GET API routes wrapped with withAuth or withEmailAccount middleware for fetching data
Export response types from GET API routes using export type GetXResponse = Awaited<ReturnType<typeof getData>>

Files:

  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
apps/web/{utils/ai,utils/llms,__tests__}/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/llm.mdc)

LLM-related code must be organized in specific directories: apps/web/utils/ai/ for main implementations, apps/web/utils/llms/ for core utilities and configurations, and apps/web/__tests__/ for LLM-specific tests

Files:

  • apps/web/utils/ai/choose-rule/run-rules.ts
apps/web/utils/ai/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/llm.mdc)

apps/web/utils/ai/**/*.ts: LLM feature functions must import from zod for schema validation, use createScopedLogger from @/utils/logger, chatCompletionObject and createGenerateObject from @/utils/llms, and import EmailAccountWithAI type from @/utils/llms/types
LLM feature functions must follow a standard structure: accept options with inputData and emailAccount parameters, implement input validation with early returns, define separate system and user prompts, create a Zod schema for response validation, and use createGenerateObject to execute the LLM call
System prompts must define the LLM's role and task specifications
User prompts must contain the actual data and context, and should be kept separate from system prompts
Always define a Zod schema for LLM response validation and make schemas as specific as possible to guide the LLM output
Use descriptive scoped loggers for each LLM feature, log inputs and outputs with appropriate log levels, and include relevant context in log messages
Implement early returns for invalid LLM inputs, use proper error types and logging, implement fallbacks for AI failures, and add retry logic for transient failures using withRetry
Use XML-like tags to structure data in prompts, remove excessive whitespace and truncate long inputs, and format data consistently across similar LLM functions
Use TypeScript types for all LLM function parameters and return values, and define clear interfaces for complex input/output structures
Keep related AI functions in the same file or directory, extract common patterns into utility functions, and document complex AI logic with clear comments

Files:

  • apps/web/utils/ai/choose-rule/run-rules.ts
🧠 Learnings (24)
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls

Applied to files:

  • apps/web/utils/actions/cold-email.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to apps/web/utils/gmail/**/*.{ts,tsx} : Always use wrapper functions from @/utils/gmail/ for Gmail API operations instead of direct provider API calls

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : LLM feature functions must import from `zod` for schema validation, use `createScopedLogger` from `@/utils/logger`, `chatCompletionObject` and `createGenerateObject` from `@/utils/llms`, and import `EmailAccountWithAI` type from `@/utils/llms/types`

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
📚 Learning: 2025-11-25T14:39:27.909Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:27.909Z
Learning: Applies to **/app/api/**/*.ts : Use `withEmailAccount` middleware for operations scoped to a specific email account, including reading/writing emails, rules, schedules, or any operation using `emailAccountId`

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to apps/web/utils/gmail/**/*.{ts,tsx} : Keep Gmail provider-specific implementation details isolated within the apps/web/utils/gmail/ directory

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : LLM feature functions must follow a standard structure: accept options with `inputData` and `emailAccount` parameters, implement input validation with early returns, define separate system and user prompts, create a Zod schema for response validation, and use `createGenerateObject` to execute the LLM call

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
📚 Learning: 2025-12-31T23:49:09.597Z
Learnt from: rsnodgrass
Repo: elie222/inbox-zero PR: 1154
File: apps/web/app/api/user/setup-progress/route.ts:0-0
Timestamp: 2025-12-31T23:49:09.597Z
Learning: In apps/web/app/api/user/setup-progress/route.ts, Reply Zero enabled status should be determined solely by checking if the TO_REPLY rule is enabled, as it is the critical/canonical rule that Reply Zero is based on. The other conversation status types (FYI, AWAITING_REPLY, ACTIONED) should not be checked for determining Reply Zero setup progress.
<!-- </add_learning>

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/utils/cold-email/is-cold-email.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Implement early returns for invalid LLM inputs, use proper error types and logging, implement fallbacks for AI failures, and add retry logic for transient failures using `withRetry`

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/utils/cold-email/is-cold-email.ts
  • apps/web/utils/rule/learned-patterns.ts
📚 Learning: 2025-11-25T14:42:16.602Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-25T14:42:16.602Z
Learning: The `utils` folder contains core app logic such as Next.js Server Actions and Gmail API requests

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
📚 Learning: 2025-11-25T14:38:37.508Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/prisma-enum-imports.mdc:0-0
Timestamp: 2025-11-25T14:38:37.508Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Always import Prisma enums from `@/generated/prisma/enums` instead of `@/generated/prisma/client` to avoid Next.js bundling errors in client components

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
📚 Learning: 2025-11-25T14:38:42.022Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/prisma.mdc:0-0
Timestamp: 2025-11-25T14:38:42.022Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Import Prisma using the project's centralized utility: `import prisma from '@/utils/prisma'`

Applied to files:

  • apps/web/utils/actions/cold-email.ts
📚 Learning: 2025-11-25T14:42:11.919Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-25T14:42:11.919Z
Learning: Applies to utils/**/*.{js,ts,jsx,tsx} : The `utils` folder contains core app logic such as Next.js Server Actions and Gmail API requests

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
📚 Learning: 2025-11-25T14:37:09.306Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/fullstack-workflow.mdc:0-0
Timestamp: 2025-11-25T14:37:09.306Z
Learning: Applies to apps/web/utils/actions/*.ts : Server actions should use 'use server' directive and automatically receive authentication context (`emailAccountId`) from the `actionClient`

Applied to files:

  • apps/web/utils/actions/cold-email.ts
📚 Learning: 2025-11-25T14:37:22.822Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/get-api-route.mdc:0-0
Timestamp: 2025-11-25T14:37:22.822Z
Learning: Applies to **/app/**/route.ts : Use Prisma for database queries in GET API routes

Applied to files:

  • apps/web/utils/actions/cold-email.ts
📚 Learning: 2026-01-01T10:42:29.775Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-01-01T10:42:29.775Z
Learning: Applies to **/*.test.{ts,tsx} : Use test helper functions `getEmail`, `getEmailAccount`, and `getRule` from `@/__tests__/helpers` for creating mock data

Applied to files:

  • apps/web/utils/actions/cold-email.ts
  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Design Gmail wrapper functions to be provider-agnostic to support future email providers like Outlook and ProtonMail

Applied to files:

  • apps/web/utils/actions/cold-email.ts
📚 Learning: 2025-11-25T14:39:49.448Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-11-25T14:39:49.448Z
Learning: Applies to apps/web/utils/actions/*.ts : Use `actionClient` when both authenticated user context and a specific emailAccountId are needed, with emailAccountId bound when calling from the client

Applied to files:

  • apps/web/utils/actions/cold-email.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Keep related AI functions in the same file or directory, extract common patterns into utility functions, and document complex AI logic with clear comments

Applied to files:

  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/rule/learned-patterns.ts
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Prefer using existing helpers from `@/__tests__/helpers.ts` (`getEmailAccount`, `getEmail`, `getRule`, `getMockMessage`, `getMockExecutedRule`) instead of creating custom test data helpers

Applied to files:

  • apps/web/app/api/ai/analyze-sender-pattern/route.ts
  • apps/web/utils/cold-email/is-cold-email.ts
📚 Learning: 2025-11-25T14:39:23.326Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to app/api/**/*.ts : Use `withEmailAccount` middleware for operations scoped to a specific email account (reading/writing emails, rules, schedules, etc.) - provides `emailAccountId`, `userId`, and `email` in `request.auth`

Applied to files:

  • apps/web/utils/cold-email/is-cold-email.ts
📚 Learning: 2025-11-25T14:39:23.326Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to app/api/**/*.ts : Use `SafeError` for error responses to prevent information disclosure - provide generic messages (e.g., 'Rule not found' not 'Rule {id} does not exist for user {userId}') without revealing internal IDs or ownership details

Applied to files:

  • apps/web/utils/rule/learned-patterns.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Use descriptive scoped loggers for each LLM feature, log inputs and outputs with appropriate log levels, and include relevant context in log messages

Applied to files:

  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/choose-rule/run-rules.ts
🧬 Code graph analysis (2)
apps/web/utils/actions/cold-email.ts (5)
apps/web/utils/email/provider.ts (1)
  • createEmailProvider (14-32)
apps/web/utils/cold-email/cold-email-rule.ts (1)
  • getColdEmailRule (8-32)
apps/web/utils/error.ts (1)
  • SafeError (87-97)
apps/web/utils/rule/learned-patterns.ts (1)
  • saveLearnedPattern (13-128)
apps/web/utils/email/types.ts (1)
  • EmailProvider (45-251)
apps/web/app/api/ai/analyze-sender-pattern/route.ts (1)
apps/web/utils/rule/learned-patterns.ts (1)
  • saveLearnedPattern (13-128)
⏰ 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: Baz Reviewer
  • GitHub Check: test
  • GitHub Check: Macroscope - Correctness Check
🔇 Additional comments (9)
apps/web/utils/ai/choose-rule/run-rules.ts (1)

343-353: LGTM! Enhanced context for cold email tracking.

The addition of ruleId and logger parameters appropriately extends the cold email tracking with contextual data. This follows the coding guideline that logger should be passed as a parameter to helper functions rather than creating logger instances within them.

As per coding guidelines, passing the logger parameter enables proper scoped logging within the cold email save path.

apps/web/app/api/ai/analyze-sender-pattern/route.ts (1)

181-198: LGTM! Solid pattern-learning integration.

The changes correctly resolve the rule by name, validate its existence, and persist the learned pattern with appropriate context (ruleId and source). The error logging provides helpful debugging information when a rule can't be found.

apps/web/utils/cold-email/is-cold-email.ts (2)

58-68: LGTM! Pattern-based detection working correctly.

The pattern matching logic properly distinguishes between included senders (cold email), excluded senders, and no pattern, with appropriate logging and reason codes.


187-212: LGTM! Clean integration with learned patterns.

The refactor to use saveLearnedPattern correctly provides all required context (ruleId, reason, messageId, threadId, source) for proper pattern tracking.

apps/web/utils/actions/cold-email.ts (3)

27-38: LGTM! Efficient parallel fetching with proper validation.

The parallel execution of createEmailProvider and getColdEmailRule improves performance, and the early validation with SafeError ensures the rule exists before proceeding with pattern updates.


40-51: LGTM! Parallel side effects properly orchestrated.

Both operations (saving the learned pattern and removing labels) execute concurrently for efficiency, with the learned pattern correctly marked with exclude: true and source: GroupItemSource.USER to track user-initiated exclusions.


55-74: LGTM! Simplified label removal logic.

The refactored function correctly derives labelIds from the rule's actions and removes them from all threads from the sender in bulk using removeThreadLabels.

apps/web/utils/rule/learned-patterns.ts (2)

13-32: LGTM! Signature updated to support ruleId-based lookup with pattern metadata.

The updated parameters properly support the new pattern-learning flow with provenance tracking (source), context (reason, threadId, messageId), and exclusion semantics.


102-127: LGTM! GroupItem upsert correctly persists pattern metadata.

The upsert properly handles both update and create paths with all new fields (exclude, reason, threadId, messageId, source) for comprehensive pattern tracking.

Comment on lines 35 to 39
const where = {
emailAccountId,
status,
groupId: coldEmailRule.groupId,
type: GroupItemType.FROM,
exclude: status === ColdEmailStatus.USER_REJECTED_COLD,
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using exclude as a binary proxy for ColdEmailStatus means any non-USER_REJECTED_COLD status returns AI_LABELED_COLD items. Consider mapping each ColdEmailStatus to the correct filter and short-circuit when unsupported so queries match the intended status.

-  const where = {
-    groupId: coldEmailRule.groupId,
-    type: GroupItemType.FROM,
-    exclude: status === ColdEmailStatus.USER_REJECTED_COLD,
-  };
+  const excludeFilter =
+    status === ColdEmailStatus.USER_REJECTED_COLD
+      ? true
+      : status === ColdEmailStatus.AI_LABELED_COLD
+      ? false
+      : null;
+  if (excludeFilter === null) {
+    return { coldEmails: [], totalPages: 0 };
+  }
+  const where = {
+    groupId: coldEmailRule.groupId,
+    type: GroupItemType.FROM,
+    exclude: excludeFilter,
+  };

🚀 Want me to fix this? Reply ex: "fix it for me".

Comment on lines 85 to 90
await prisma.rule
.update({
where: { id: rule.id },
data: { groupId },
})
.catch(() => {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest not swallowing the failure to link existingGroup to this rule. If the group belongs to another rule, we’ll add the pattern to the wrong group. Consider verifying the update succeeded (e.g., re‑read the rule and confirm groupId matches) and abort if not.

Suggested change
await prisma.rule
.update({
where: { id: rule.id },
data: { groupId },
})
.catch(() => {});
await prisma.rule
.update({
where: { id: rule.id },
data: { groupId },
})
.catch(() => null);
const linked = await prisma.rule.findUnique({
where: { id: rule.id },
select: { groupId: true },
});
if (linked?.groupId !== groupId) {
throw new Error("Existing group is linked to a different rule; aborting to avoid adding patterns to the wrong group");
}

🚀 Want me to fix this? Reply ex: "fix it for me".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit e980e31 addressed this comment by refactoring the group creation logic and replacing the silent error swallowing (.catch(() => {})) with proper error logging that includes detailed context about the failure. While the exact verification suggestion wasn't implemented, the core concern about ignoring link failures has been resolved.

Comment on lines 34 to 37
const rule = await prisma.rule.findUnique({
where: {
name_emailAccountId: {
name: ruleName,
emailAccountId,
},
},
select: { id: true, groupId: true },
where: { id: ruleId, emailAccountId },
select: { id: true, name: true, groupId: true },
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prisma.rule.findUnique only allows unique fields in its where input. emailAccountId is not part of the RuleWhereUniqueInput (only id, groupId, and the named composite uniques exist), so calling findUnique({ where: { id: ruleId, emailAccountId } }) will not compile and would throw Unknown arg "emailAccountId" if forced. This needs to be a findFirst with both filters or a findUnique on id alone with an additional check that the rule belongs to the account.

Suggested change
const rule = await prisma.rule.findUnique({
where: {
name_emailAccountId: {
name: ruleName,
emailAccountId,
},
},
select: { id: true, groupId: true },
where: { id: ruleId, emailAccountId },
select: { id: true, name: true, groupId: true },
});
const rule = await prisma.rule.findFirst({
where: { id: ruleId, emailAccountId },
select: { id: true, name: true, groupId: true },
});

Finding type: Logical Bugs

status: ColdEmailStatus.USER_REJECTED_COLD,
},
// Mark as excluded so AI doesn't match it again
saveLearnedPattern({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sender is saved unnormalized, so patterns may not match lookups that use extractEmailAddress. Consider normalizing (extractEmailAddress(sender) || sender) before calling saveLearnedPattern.

🚀 Want me to fix this? Reply ex: "fix it for me".

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (3)
apps/web/app/(app)/[emailAccountId]/cold-email-blocker/ColdEmailList.tsx (1)

193-204: Missing required ruleId parameter will cause validation failure.

The toggleRuleAction requires three parameters: ruleId, systemType, and enabled. However, the current implementation only passes systemType and enabled (lines 217-218), omitting the required ruleId. This will fail input validation when executed.

To fix this, you need to either:

  1. Find the existing COLD_EMAIL rule from the rules array and pass its ruleId
  2. Use a different action that can enable rules by systemType without requiring an id
  3. Create the rule first if it doesn't exist
🔎 Proposed fix: Find and use the existing rule ID
  const { executeAsync: enableColdEmailBlocker } = useAction(
    toggleRuleAction.bind(null, emailAccountId),
    {
      onSuccess: () => {
        toastSuccess({ description: "Cold email blocker enabled!" });
        mutateRules();
      },
      onError: () => {
        toastError({ description: "Error enabling cold email blocker" });
      },
    },
  );
+
+  const coldEmailRule = rules?.find(
+    (rule) => rule.systemType === SystemType.COLD_EMAIL
+  );

  if (!isColdEmailBlockerEnabled(rules || [])) {
    return (
      <div className="mb-10">
        <EnableFeatureCard
          title="Cold Email Blocker"
          description="Our AI identifies cold outreach from senders you've never communicated with before. You can customize the prompt after enabling."
          imageSrc="/images/illustrations/calling-help.svg"
          imageAlt="Cold email blocker"
          buttonText="Enable"
          onEnable={async () => {
+           if (!coldEmailRule) {
+             toastError({ description: "Cold email rule not found" });
+             return;
+           }
            await enableColdEmailBlocker({
+             ruleId: coldEmailRule.id,
              systemType: SystemType.COLD_EMAIL,
              enabled: true,
            });
          }}
          hideBorder
        />
      </div>
    );
  }

Also applies to: 215-220

apps/web/utils/rule/learned-patterns.ts (2)

32-35: Critical: findUnique cannot accept non-unique fields.

prisma.rule.findUnique only accepts unique fields in its where clause. Since emailAccountId is not part of RuleWhereUniqueInput, this code will fail to compile with a TypeScript error. Additionally, this creates an IDOR vulnerability if only id is checked without verifying ownership.

Based on coding guidelines and the past review comment, this must be changed to use findFirst with both filters.

🔎 Proposed fix
-  const rule = await prisma.rule.findUnique({
-    where: { id: ruleId, emailAccountId },
+  const rule = await prisma.rule.findFirst({
+    where: { id: ruleId, emailAccountId },
     select: { id: true, name: true, groupId: true },
   });

211-226: Major: Linking group without ownership verification.

When an existing group is found by name, the code attempts to link it to the current rule without verifying that the group isn't already linked to a different rule. This could cause learned patterns to be added to the wrong rule's group.

Consider verifying the group is safe to link before attempting the update, or explicitly checking after the update that the link succeeded as expected.

🔎 Suggested verification approach
   if (existingGroup) {
+    // Verify the group isn't linked to a different rule
+    const groupWithRule = await prisma.group.findUnique({
+      where: { id: existingGroup.id },
+      select: { ruleId: true },
+    });
+    
+    if (groupWithRule?.ruleId && groupWithRule.ruleId !== ruleId) {
+      throw new Error(
+        `Group "${ruleName}" is already linked to a different rule`
+      );
+    }
+
     // Attempt to link it (ignore failures from concurrent updates)
     await prisma.rule
       .update({ where: { id: ruleId }, data: { groupId: existingGroup.id } })

Comment on lines +199 to +203
const updatedRule = await prisma.rule.findUnique({
where: { id: ruleId },
select: { groupId: true },
});
if (updatedRule?.groupId) return updatedRule.groupId;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: IDOR vulnerability - unscoped rule lookup.

The rule lookup uses only id without verifying emailAccountId, creating an Insecure Direct Object Reference (IDOR) vulnerability. An attacker could pass a ruleId belonging to another user, and this function would use it.

Per coding guidelines: "ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access."

🔎 Proposed fix
   // Handle duplicate: check if rule was concurrently updated with a group
   const updatedRule = await prisma.rule.findUnique({
-    where: { id: ruleId },
+    where: { id: ruleId, emailAccountId },
     select: { groupId: true },
   });

Note: This requires changing findUnique to findFirst since emailAccountId is not a unique field (same issue as the earlier comment):

   // Handle duplicate: check if rule was concurrently updated with a group
-  const updatedRule = await prisma.rule.findUnique({
-    where: { id: ruleId },
+  const updatedRule = await prisma.rule.findFirst({
+    where: { id: ruleId, emailAccountId },
     select: { groupId: true },
   });
🤖 Prompt for AI Agents
In apps/web/utils/rule/learned-patterns.ts around lines 199 to 203, the DB query
uses only ruleId which creates an IDOR; change the query to scope by
authenticated account by including emailAccountId in the WHERE clause and switch
from findUnique to findFirst (emailAccountId is not unique), i.e. query where {
id: ruleId, emailAccountId: currentAccountId } and return updatedRule.groupId as
before; ensure you pass the correct account id from the calling context and
handle the case where no record is found.

Comment on lines +213 to +224
await prisma.rule
.update({ where: { id: ruleId }, data: { groupId: existingGroup.id } })
.catch((error) => {
logger.warn(
"Failed to link existing group to rule (likely concurrent update)",
{
ruleId,
groupId: existingGroup.id,
error,
},
);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: IDOR vulnerability - unscoped rule update.

The rule update uses only id in the where clause without verifying emailAccountId. This allows updating any user's rule, creating an IDOR vulnerability.

Per coding guidelines: "Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations."

🔎 Proposed fix

Add emailAccountId to the where clause:

     await prisma.rule
-      .update({ where: { id: ruleId }, data: { groupId: existingGroup.id } })
+      .update({ 
+        where: { id: ruleId, emailAccountId }, 
+        data: { groupId: existingGroup.id } 
+      })
       .catch((error) => {

Note: Similar to previous comments, rule.update with a composite where clause may require using a different approach. Verify that Prisma's RuleWhereUniqueInput supports this, or use a two-step verification:

+    // Verify ownership before updating
+    const ruleToUpdate = await prisma.rule.findFirst({
+      where: { id: ruleId, emailAccountId },
+      select: { id: true },
+    });
+    
+    if (!ruleToUpdate) {
+      logger.error("Cannot link group: rule not found or access denied", {
+        ruleId,
+        groupId: existingGroup.id,
+      });
+      throw new Error("Rule not found or access denied");
+    }
+
     await prisma.rule
       .update({ where: { id: ruleId }, data: { groupId: existingGroup.id } })

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/web/utils/rule/learned-patterns.ts around lines 213 to 224, the update
uses only the rule id which creates an IDOR by allowing updates to rules not
owned by the current emailAccount; ensure ownership is enforced by scoping the
update to the emailAccountId — either by using a where that includes both id and
emailAccountId if Prisma supports that composite unique input, or perform a
two-step: first fetch/verify the rule belongs to the current emailAccountId,
then perform the update by id (or use updateMany with where: { id: ruleId,
emailAccountId } to atomically restrict by owner); update the logger payload
similarly and keep the catch logic intact.

Comment on lines +87 to +96
JOIN "Rule" r ON r."emailAccountId" = ce."emailAccountId" AND r."systemType" = 'COLD_EMAIL'
WHERE r."groupId" IS NOT NULL
AND ce."fromEmail" IS NOT NULL
-- Avoid duplicates: only insert if this pattern doesn't already exist
AND NOT EXISTS (
SELECT 1 FROM "GroupItem" gi
WHERE gi."groupId" = r."groupId"
AND gi.type = 'FROM'
AND gi.value = ce."fromEmail"
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The NOT EXISTS clause only checks for existing GroupItems before the INSERT starts, so it does not deduplicate among multiple ColdEmail rows that share the same (groupId, type, value). When ColdEmail already contains repeated senders for a rule, every copy passes the NOT EXISTS guard because the GroupItem rows that will be inserted later do not yet exist, so the INSERT tries to write duplicate keys and will violate the GroupItem unique constraint ([groupId,type,value]). The migration should deduplicate the ColdEmail rows (e.g., DISTINCT/ GROUP BY) or otherwise guard against duplicates before relying on the constraint, otherwise the migration can fail for accounts that have repeated ColdEmail entries for the same sender.

Suggested change
JOIN "Rule" r ON r."emailAccountId" = ce."emailAccountId" AND r."systemType" = 'COLD_EMAIL'
WHERE r."groupId" IS NOT NULL
AND ce."fromEmail" IS NOT NULL
-- Avoid duplicates: only insert if this pattern doesn't already exist
AND NOT EXISTS (
SELECT 1 FROM "GroupItem" gi
WHERE gi."groupId" = r."groupId"
AND gi.type = 'FROM'
AND gi.value = ce."fromEmail"
);
JOIN "Rule" r ON r."emailAccountId" = ce."emailAccountId" AND r."systemType" = 'COLD_EMAIL'
WHERE r."groupId" IS NOT NULL
AND ce."fromEmail" IS NOT NULL
-- Avoid duplicates: only insert if this pattern doesn't already exist
AND NOT EXISTS (
SELECT 1 FROM "GroupItem" gi
WHERE gi."groupId" = r."groupId"
AND gi.type = 'FROM'
AND gi.value = ce."fromEmail"
)
GROUP BY r."groupId", ce."fromEmail", ce."createdAt", ce."updatedAt", ce.reason, ce."threadId", ce."messageId", ce.status;

Finding type: Logical Bugs

@elie222 elie222 merged commit 0a03402 into main Jan 4, 2026
18 checks passed
@elie222 elie222 deleted the feat/cold-email-and-ai-rules branch January 4, 2026 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant