Skip to content

Learn from label removal patterns for newsletters#697

Closed
edulelis wants to merge 14 commits intoelie222:mainfrom
edulelis:learned-patterns-from-labels-v2
Closed

Learn from label removal patterns for newsletters#697
edulelis wants to merge 14 commits intoelie222:mainfrom
edulelis:learned-patterns-from-labels-v2

Conversation

@edulelis
Copy link
Collaborator

@edulelis edulelis commented Aug 19, 2025

Summary by CodeRabbit

  • New Features

    • AI-powered analysis when you remove a label to refine auto-sorting rules.
    • Automatically adds or removes learned patterns based on your feedback.
    • Stores a short “reasoning” note for each learned pattern for clearer context.
    • Broader recognition of system labels (e.g., Sent, Drafts, Spam) to avoid false triggers.
    • Improved handling when removing the “Cold Email” label to keep statuses accurate.
  • Tests

    • Added extensive test coverage for AI label-removal analysis scenarios.

@vercel
Copy link

vercel bot commented Aug 19, 2025

@edulelis is attempting to deploy a commit to the Inbox Zero OSS Program Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 19, 2025

Walkthrough

Renames and refactors the Gmail label-removed handler to a modular process with AI-driven analysis. Adds learned-pattern deletion and optional reasoning storage. Updates schema and migration to persist reasoning. Adjusts imports/call sites and tests. Introduces a new AI analysis utility and a new test suite. Removes auth origin fields.

Changes

Cohort / File(s) Summary
Webhook handler rename and modularization
apps/web/app/api/google/webhook/process-history-item.ts, apps/web/app/api/google/webhook/process-label-removed-event.ts
Renames handleLabelRemovedEvent to processLabelRemoval. Refactors flow: system-label resolution, optional AI analysis, and learned-pattern updates/removals. Adds helpers getSystemRuleLabels, processLabelRemovalAnalysis; expands filtered system labels; updates logging.
AI analysis utility (new)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts
Adds aiAnalyzeLabelRemoval with Zod-validated LabelRemovalAnalysis (actions: EXCLUDE/REMOVE/NO_ACTION; optional pattern with reasoning/type/value/exclude). Builds prompts and returns structured analysis.
Learned patterns utilities
apps/web/utils/rule/learned-patterns.ts
Adds optional reasoning to save APIs. Introduces removeLearnedPattern. Upserts now persist reasoning; deletion path added with logging.
Tests update and addition
apps/web/app/api/google/webhook/process-label-removed-event.test.ts, apps/web/__tests__/ai-analyze-label-removal.test.ts
Renames tests to processLabelRemoval. Mocks Gmail/label resolution; updates invocations and options. Adds comprehensive AI analysis test suite with fixtures and schema assertions.
Schema and migration
apps/web/prisma/schema.prisma, apps/web/prisma/migrations/20250819203156_add_reasoning_to_group_item/migration.sql
Adds optional reasoning field to GroupItem; migration adds reasoning TEXT column.
Auth config
apps/web/utils/auth.ts
Removes baseURL and trustedOrigins from betterAuth configuration.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Gmail as Gmail Webhook
  participant API as processHistoryItem
  participant Handler as processLabelRemoval
  participant Provider as Email Provider
  participant AI as aiAnalyzeLabelRemoval
  participant Rules as Learned Patterns
  participant DB as Database

  Gmail->>API: History: LABEL_REMOVED
  API->>Handler: processLabelRemoval(item, { emailAccount, provider })
  Handler->>Provider: Fetch message + labels
  Handler->>Handler: Derive removed label names (exclude system)
  alt Matching system rule found
    Handler->>AI: Analyze({ rule, email, account })
    AI-->>Handler: LabelRemovalAnalysis { action, pattern? }
    Handler->>Handler: processLabelRemovalAnalysis(...)
    alt action == REMOVE and pattern
      Handler->>Rules: removeLearnedPattern(ruleName, pattern)
      Rules->>DB: Delete GroupItem
      DB-->>Rules: OK
    else action == EXCLUDE and pattern
      Handler->>Rules: saveLearnedPatterns([{..., reasoning?}])
      Rules->>DB: Upsert GroupItem(s)
      DB-->>Rules: OK
    else NO_ACTION
      Handler-->>API: return
    end
  else No matching rule
    Handler-->>API: Skip AI analysis
  end
  API-->>Gmail: 200 OK
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

A label hops off with a whisper—no more!
I twitch my ears, consult the lore.
Patterns learned, some kept, some pruned,
Reasoning tucked, conclusions tuned.
With careful paws I tidy the bin,
AI hums—let the inbox win.
(\_/) ✅

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbit or @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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

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/rule/learned-patterns.ts (2)

54-69: Upsert update path ignores reasoning — existing items won’t get updated

If a FROM pattern already exists, reasoning won’t be written because update is empty. Include reasoning in the update path (guarded to avoid writing undefined).

Apply this diff:

   await prisma.groupItem.upsert({
     where: {
       groupId_type_value: {
         groupId,
         type: GroupItemType.FROM,
         value: from,
       },
     },
-    update: {},
+    update: {
+      ...(typeof reasoning !== "undefined" ? { reasoning } : {}),
+    },
     create: {
       groupId,
       type: GroupItemType.FROM,
       value: from,
       reasoning,
     },
   });

143-154: Avoid overwriting fields on update when input is undefined

The current update unconditionally sets exclude to false when not provided and sets reasoning to undefined. This can unintentionally clear/flip existing data. Update conditionally only when values are provided.

Apply this diff:

-        update: {
-          exclude: pattern.exclude || false,
-          reasoning: pattern.reasoning,
-        },
+        update: {
+          ...(pattern.exclude !== undefined ? { exclude: pattern.exclude } : {}),
+          ...(pattern.reasoning !== undefined ? { reasoning: pattern.reasoning } : {}),
+        },
🧹 Nitpick comments (8)
apps/web/prisma/schema.prisma (1)

506-506: Reasoning field added — consider bounding size and data shape

Storing AI “reasoning” as free-form TEXT is fine, but LLM outputs can be large. Consider one of:

  • Truncate to a sane length before persisting (e.g., 2–4KB).
  • If you’ll later want structured insights, use Json? instead of String? to avoid future migrations.

This can be handled in the write path (see suggested clamp in learned-patterns.ts comment).

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

108-108: Direct awaited invocation is cleaner — add coverage for reasoning persistence

The simplification looks good. To fully exercise the new “reasoning” path, add a test that mocks the AI analysis to return patterns with reasoning and asserts saveLearnedPatterns is called with reasoning.

Example test block you can add:

it("persists learned patterns with reasoning when AI returns patterns", async () => {
  const patterns = [
    { type: GroupItemType.FROM, value: "sender@example.com", reasoning: "User rejected cold email label" },
  ];
  vi.doMock("@/utils/ai/label-analysis/analyze-label-removal", () => ({
    aiAnalyzeLabelRemoval: vi.fn().mockResolvedValue({
      actions: [],
      patterns,
    }),
  }));

  const historyItem = createLabelRemovedHistoryItem("123", "thread-123", ["label-1"]);
  await handleLabelRemovedEvent(historyItem.item, defaultOptions);

  expect(saveLearnedPatterns).toHaveBeenCalledWith(
    expect.objectContaining({
      ruleName: expect.any(String),
      patterns: expect.arrayContaining([expect.objectContaining({ reasoning: "User rejected cold email label" })]),
    }),
  );
});
apps/web/utils/rule/learned-patterns.ts (1)

156-161: Nit: prefer nullish coalescing for booleans

Use ?? over || to avoid surprising truthiness behavior (even though exclude is boolean-typed).

Apply this diff:

-        create: {
+        create: {
           groupId,
           type: pattern.type,
           value: pattern.value,
-          exclude: pattern.exclude || false,
+          exclude: pattern.exclude ?? false,
           reasoning: pattern.reasoning,
         },

Additionally, consider clamping reasoning length before persisting to prevent unbounded growth. For example:

const clamp = (s: string, max = 2000) => (s.length > max ? `${s.slice(0, max)}…` : s);
// …
reasoning: pattern.reasoning ? clamp(pattern.reasoning) : undefined,
apps/web/app/api/google/webhook/process-label-removed-event.ts (4)

61-72: Consider adding JSDoc documentation for the new public API.

Since processLabelRemoval is now a public export that other modules depend on, it would benefit from JSDoc documentation describing its purpose, parameters, and any thrown errors.

Add documentation like this:

+/**
+ * Processes a label removal event from Gmail webhook, analyzing the removal
+ * pattern and potentially learning from it for future email processing.
+ * 
+ * @param message - The Gmail history label removed event
+ * @param options - Configuration including Gmail client, email account, and provider
+ * @throws Will log errors but not throw to avoid webhook failure
+ */
 export async function processLabelRemoval(

134-150: Consider renaming the function for clarity.

The function name analyseLabelRemoval uses British spelling while the rest of the codebase uses American spelling (e.g., aiAnalyzeLabelRemoval). Also, since this is an internal function that specifically handles the analysis workflow, consider a more descriptive name.

-async function analyseLabelRemoval({
+async function analyzeLabelRemovalWithAI({

207-238: Consider extracting supported system types to a constant.

Currently, only SystemType.NEWSLETTER is supported for AI analysis. As more system types are added, consider extracting the supported types to a constant for easier maintenance.

+const AI_SUPPORTED_SYSTEM_TYPES = [SystemType.NEWSLETTER] as const;

 // Matching rule by SystemType and not by name
-if (matchingRule.systemType === SystemType.NEWSLETTER) {
+if (AI_SUPPORTED_SYSTEM_TYPES.includes(matchingRule.systemType as any)) {

240-250: Consider adding JSDoc for the analysis processing function.

This public export handles the results of AI analysis and persists learned patterns. Documentation would help other developers understand the flow.

+/**
+ * Processes the results of AI label removal analysis and persists any learned patterns.
+ * 
+ * @param options.analysis - The AI analysis result containing action and pattern details
+ * @param options.emailAccountId - The email account ID for pattern association
+ * @param options.labelName - The name of the label that was removed
+ * @param options.ruleName - The name of the rule to update with learned patterns
+ */
 export async function processLabelRemovalAnalysis({
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (1)

50-61: Consider adding JSDoc documentation for the main function.

This is a key AI analysis function that other modules depend on. Adding JSDoc would improve maintainability and help developers understand the expected inputs and outputs.

+/**
+ * Analyzes why a user removed a label from an email and recommends an action
+ * for future email processing based on the detected pattern.
+ * 
+ * @param options.label - The label that was removed, including name and instructions
+ * @param options.email - The email content in LLM format
+ * @param options.emailAccount - The email account with AI configuration
+ * @returns Analysis result with recommended action and optional pattern details
+ */
 export async function aiAnalyzeLabelRemoval({
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between dd7e1a3 and e7579a7.

📒 Files selected for processing (7)
  • apps/web/app/api/google/webhook/process-history-item.ts (2 hunks)
  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts (1 hunks)
  • apps/web/app/api/google/webhook/process-label-removed-event.ts (5 hunks)
  • apps/web/prisma/migrations/20250819203156_add_reasoning_to_group_item/migration.sql (1 hunks)
  • apps/web/prisma/schema.prisma (1 hunks)
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts (1 hunks)
  • apps/web/utils/rule/learned-patterns.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
apps/web/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use @/ for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
  • apps/web/app/api/google/webhook/process-history-item.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
apps/web/app/**

📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)

NextJS app router structure with (app) directory

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
  • apps/web/app/api/google/webhook/process-history-item.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
!{.cursor/rules/*.mdc}

📄 CodeRabbit Inference Engine (.cursor/rules/cursor-rules.mdc)

Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
  • apps/web/prisma/migrations/20250819203156_add_reasoning_to_group_item/migration.sql
  • apps/web/app/api/google/webhook/process-history-item.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/prisma/schema.prisma
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
**/*.ts

📄 CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)

**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
  • apps/web/app/api/google/webhook/process-history-item.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/logging.mdc)

**/*.{ts,tsx}: Use createScopedLogger for logging in backend TypeScript files
Typically add the logger initialization at the top of the file when using createScopedLogger
Only use .with() on a logger instance within a specific function, not for a global logger

Import Prisma in the project using import prisma from "@/utils/prisma";

**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
  • apps/web/app/api/google/webhook/process-history-item.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
**/*.test.{ts,js}

📄 CodeRabbit Inference Engine (.cursor/rules/security.mdc)

Include security tests in your test suites to verify authentication, authorization, and error handling.

Files:

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

📄 CodeRabbit Inference Engine (.cursor/rules/testing.mdc)

**/*.test.{ts,js,tsx,jsx}: Tests are colocated next to the tested file (e.g., dir/format.ts and dir/format.test.ts)
Use vi.mock("server-only", () => ({})); to mock the server-only module in tests
Mock @/utils/prisma in tests using vi.mock("@/utils/prisma") and use the provided prisma mock
Mock external dependencies in tests
Clean up mocks between tests
Do not mock the Logger

Files:

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

📄 CodeRabbit Inference Engine (.cursor/rules/security-audit.mdc)

apps/web/app/api/**/*.{ts,js}: All API route handlers in 'apps/web/app/api/' must use authentication middleware: withAuth, withEmailAccount, or withError (with custom authentication logic).
All Prisma queries in API routes must include user/account filtering (e.g., emailAccountId or userId in WHERE clauses) to prevent unauthorized data access.
All parameters used in API routes must be validated before use; do not use parameters from 'params' or request bodies directly in queries without validation.
Request bodies in API routes should use Zod schemas for validation.
API routes should only return necessary fields using Prisma's 'select' and must not include sensitive data in error messages.
Error messages in API routes must not reveal internal details; use generic errors and SafeError for user-facing errors.
All QStash endpoints (API routes called via publishToQstash or publishToQstashQueue) must use verifySignatureAppRouter to verify request authenticity.
All cron endpoints in API routes must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak or plaintext secrets in API route files; secrets must not be directly assigned as string literals.
Review all new withError usage in API routes to ensure custom authentication is implemented where required.

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
  • apps/web/app/api/google/webhook/process-history-item.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 elements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
  • apps/web/app/api/google/webhook/process-history-item.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
!pages/_document.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)

!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
  • apps/web/prisma/migrations/20250819203156_add_reasoning_to_group_item/migration.sql
  • apps/web/app/api/google/webhook/process-history-item.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/prisma/schema.prisma
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)

**/*.{test,spec}.{js,jsx,ts,tsx}: Don't use export or module.exports in test files.
Don't use focused tests.
Don't use disabled tests.
Make sure the assertion function, like expect, is placed inside an it() function call.
Don't nest describe() blocks too deeply in test files.
Don't use focused tests.
Don't use disabled tests.
Don't use export or module.exports in test files.

Files:

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

📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)

Create utility functions in utils/ folder for reusable logic

Files:

  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
apps/web/utils/**/*.ts

📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)

apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size

Files:

  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
apps/web/prisma/schema.prisma

📄 CodeRabbit Inference Engine (.cursor/rules/prisma.mdc)

The Prisma schema file must be located at apps/web/prisma/schema.prisma

Files:

  • apps/web/prisma/schema.prisma
apps/web/utils/{ai,llms}/**/*.ts

📄 CodeRabbit Inference Engine (.cursor/rules/llm.mdc)

apps/web/utils/{ai,llms}/**/*.ts: Place LLM-related implementation code under apps/web/utils/ai or apps/web/utils/llms
Keep system and user prompts separate; system defines role/task, user contains data/context
Always validate LLM responses with a specific Zod schema
Use descriptive scoped loggers per feature and log inputs/outputs with appropriate levels and context
Implement early returns for invalid inputs and use proper error types with logging
Add fallbacks for AI failures and include retry logic for transient errors using withRetry
Format prompts with XML-like tags; remove excessive whitespace; truncate overly long inputs; keep formatting consistent
Use TypeScript types for all parameters/returns and define interfaces for complex IO structures
Keep related AI functions co-located; extract shared logic into utilities; document complex AI logic with clear comments
Call LLMs via createGenerateObject; pass system, prompt, and a Zod schema; return the validated result.object
Derive model options using getModel(...) and pass them to createGenerateObject and the generate call

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
🧠 Learnings (1)
📚 Learning: 2025-08-17T16:57:25.825Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.825Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Keep related AI functions co-located; extract shared logic into utilities; document complex AI logic with clear comments

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
🧬 Code Graph Analysis (4)
apps/web/app/api/google/webhook/process-label-removed-event.test.ts (1)
apps/web/app/api/google/webhook/process-label-removed-event.ts (1)
  • handleLabelRemovedEvent (46-59)
apps/web/app/api/google/webhook/process-history-item.ts (1)
apps/web/app/api/google/webhook/process-label-removed-event.ts (1)
  • processLabelRemoval (61-132)
apps/web/app/api/google/webhook/process-label-removed-event.ts (7)
apps/web/utils/logger.ts (1)
  • createScopedLogger (17-65)
apps/web/utils/llms/types.ts (1)
  • EmailAccountWithAI (10-29)
apps/web/utils/types.ts (1)
  • ParsedMessage (55-70)
apps/web/utils/label.ts (1)
  • inboxZeroLabels (32-68)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (4)
  • aiAnalyzeLabelRemoval (50-124)
  • LabelRemovalAnalysis (48-48)
  • LabelRemovalAction (12-16)
  • LabelRemovalAction (18-19)
apps/web/utils/get-email-from-message.ts (1)
  • getEmailForLLM (6-25)
apps/web/utils/rule/learned-patterns.ts (1)
  • saveLearnedPatterns (76-181)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (6)
apps/web/utils/logger.ts (1)
  • createScopedLogger (17-65)
apps/web/utils/types.ts (1)
  • EmailForLLM (117-131)
apps/web/utils/llms/types.ts (1)
  • EmailAccountWithAI (10-29)
apps/web/utils/stringify-email.ts (1)
  • stringifyEmail (4-27)
apps/web/utils/llms/model.ts (1)
  • getModel (27-41)
apps/web/utils/llms/index.ts (1)
  • createGenerateObject (118-166)
⏰ 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). (2)
  • GitHub Check: Static Code Analysis Js
  • GitHub Check: Jit Security
🔇 Additional comments (12)
apps/web/prisma/migrations/20250819203156_add_reasoning_to_group_item/migration.sql (1)

1-2: LGTM: additive, non-breaking migration

The migration aligns with the Prisma schema and is safe (nullable TEXT). No concerns.

apps/web/app/api/google/webhook/process-history-item.ts (1)

29-29: Good extraction: route label-removal logic through the new handler

Replacing the direct handling with processLabelRemoval keeps concerns isolated and leverages its internal error handling/logging. Return + await behavior is correct for an async function.

Also applies to: 70-74

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

17-23: API surface extended with optional reasoning — looks consistent

Adding reasoning?: string to saveLearnedPattern is consistent with the schema/migration changes and downstream usage.


87-88: Patterns now accept reasoning — type shape matches the persistence layer

Extending the patterns item type to include reasoning?: string aligns with the new column and downstream code.

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

30-44: LGTM! Clean implementation of filtered rule fetching.

The function properly filters rules with non-null systemType and returns only the necessary fields. This is a good separation of concerns for retrieving system-typed rules.


189-198: Good defensive check for rule existence.

The code properly validates that a matching rule exists before proceeding with AI analysis. The logging provides good visibility into why the AI analysis is skipped.


251-291: Good implementation of pattern learning with reasoning.

The function properly handles different action types and correctly persists the learned patterns with the new reasoning field. The logging provides good visibility into the actions taken.

apps/web/utils/ai/label-analysis/analyze-label-removal.ts (5)

1-10: LGTM! Well-organized imports and logger initialization.

The imports are properly structured and the scoped logger is correctly initialized at the top of the file, following the coding guidelines.


21-46: Well-structured Zod schema with clear descriptions.

The schema properly defines all fields with appropriate types and descriptions. The optional fields are correctly marked, and the use of z.nativeEnum for type safety is good practice.


62-91: Excellent prompt engineering with clear decision framework.

The system prompt is well-structured with:

  • Clear role definition
  • Emphasis on high confidence requirements
  • Detailed decision framework with examples
  • Comprehensive context considerations

This follows best practices for prompt engineering.


92-103: Good use of XML-like tags for prompt structure.

The prompt properly uses XML-like tags to structure the input, which aligns with the coding guidelines. The fallback for missing instructions is appropriate.


104-124: Proper implementation following AI utility patterns.

The function correctly:

  • Uses trace logging for inputs and outputs
  • Gets model options using the economy tier
  • Creates a scoped generateObject with proper parameters
  • Passes the schema for validation
  • Returns the validated object

This follows all the coding guidelines for AI/LLM implementations.

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: 3

🧹 Nitpick comments (6)
apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts (3)

74-79: Avoid dynamic RegExp construction; assert membership directly

Constructing a regex from variable input is unnecessary here and can trigger ReDoS linters. Assert that action is one of the allowed values instead.

Apply this diff:

-    expect(result).toMatchObject({
-      action: expect.stringMatching(
-        new RegExp(`^(${Object.values(LabelRemovalAction).join("|")})$`),
-      ),
-      reasoning: expect.any(String),
-    });
+    expect(Object.values(LabelRemovalAction)).toContain(result.action);
+    expect(typeof result.reasoning).toBe("string");

160-186: Solid schema assertions; consider asserting invariants by action

The shape checks are good. Optionally assert invariants: patternType/patternValue required when action is EXCLUDE_PATTERN/NOT_INCLUDE, and omitted when NO_ACTION. This keeps contract tight even if the model drifts.

If helpful, I can provide a test that mocks the LLM result to validate these invariants without calling the AI.


222-227: Preserve aiApiKey when overriding user AI config

Overriding user removes aiApiKey entirely, which may be read downstream (getModel/provider setup). Preserve it explicitly to avoid surprises.

Apply this diff:

-        emailAccount: getEmailAccount({
-          user: { ...config },
-        }),
+        emailAccount: getEmailAccount({
+          user: { aiApiKey: null, ...config },
+        }),

Please verify whether getModel or provider initialization reads aiApiKey; if yes, add a config with a non-null key for the positive path too.

apps/web/utils/ai/label-analysis/analyze-label-removal.ts (3)

21-46: Encode invariants in Zod schema to prevent inconsistent outputs

Currently patternType/patternValue are optional regardless of action. Tighten with a refinement so:

  • When action is EXCLUDE_PATTERN or NOT_INCLUDE: patternType and patternValue are required.
  • When action is NO_ACTION: patternType, patternValue, and exclude should be omitted.

This prevents downstream ambiguity.

Apply this diff:

-const schema = z.object({
+const schema = z
+  .object({
     action: z
       .nativeEnum(LabelRemovalAction)
       .describe("The recommended action based on the label removal analysis"),
     reasoning: z
       .string()
       .describe(
         "Detailed explanation of why this action was chosen, including context about the user's behavior",
       ),
     patternType: z
       .nativeEnum(GroupItemType)
       .optional()
       .describe("Type of pattern to learn from this removal (if applicable)"),
     patternValue: z
       .string()
       .optional()
       .describe(
         "Specific value for the pattern (e.g., email address, domain, subject keyword)",
       ),
     exclude: z
       .boolean()
       .optional()
       .describe(
         "Whether this pattern should be excluded (true) or just not included (false)",
       ),
-});
+  })
+  .superRefine((val, ctx) => {
+    const needsPattern =
+      val.action === LabelRemovalAction.EXCLUDE_PATTERN ||
+      val.action === LabelRemovalAction.NOT_INCLUDE;
+    if (needsPattern) {
+      if (!val.patternType) {
+        ctx.addIssue({
+          code: z.ZodIssueCode.custom,
+          path: ["patternType"],
+          message:
+            "patternType is required when action is exclude_pattern or not_include",
+        });
+      }
+      if (!val.patternValue) {
+        ctx.addIssue({
+          code: z.ZodIssueCode.custom,
+          path: ["patternValue"],
+          message:
+            "patternValue is required when action is exclude_pattern or not_include",
+        });
+      }
+    } else {
+      if (
+        val.patternType !== undefined ||
+        val.patternValue !== undefined ||
+        val.exclude !== undefined
+      ) {
+        ctx.addIssue({
+          code: z.ZodIssueCode.custom,
+          message:
+            "patternType, patternValue, and exclude must be omitted when action is no_action",
+        });
+      }
+    }
+  });

99-107: Escape label fields embedded in XML-like prompt

If label name or instructions contain characters like <, >, or &, the XML-like structure becomes malformed. Escape these values to keep formatting robust.

Apply this diff (and add the import in the imports block):

+import escape from "lodash/escape";
@@
   <label_name>${name}</label_name>
-  <instructions_for_adding_label>${instructions || "No instructions provided"}</instructions_for_adding_label>
+  <instructions_for_adding_label>${escape(
+    instructions || "No instructions provided",
+  )}</instructions_for_adding_label>

Optionally escape the label name as well:

-  <label_name>${name}</label_name>
+  <label_name>${escape(name)}</label_name>

21-24: Optional: Prefer z.enum(Object.values(...)) for const objects

z.nativeEnum works with enum-like objects in newer Zod versions, but z.enum(Object.values(LabelRemovalAction)) avoids edge cases and makes intent explicit for const objects.

Confirm the project’s Zod version supports z.nativeEnum on const objects. If not, switch to z.enum(Object.values(LabelRemovalAction) as [typeof values…]).

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 947d1e9 and e055b1d.

📒 Files selected for processing (3)
  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts (2 hunks)
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts (1 hunks)
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
🧰 Additional context used
📓 Path-based instructions (12)
apps/web/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use @/ for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
!{.cursor/rules/*.mdc}

📄 CodeRabbit Inference Engine (.cursor/rules/cursor-rules.mdc)

Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
**/*.ts

📄 CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)

**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/logging.mdc)

**/*.{ts,tsx}: Use createScopedLogger for logging in backend TypeScript files
Typically add the logger initialization at the top of the file when using createScopedLogger
Only use .with() on a logger instance within a specific function, not for a global logger

Import Prisma in the project using import prisma from "@/utils/prisma";

**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
apps/web/utils/**

📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)

Create utility functions in utils/ folder for reusable logic

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
apps/web/utils/**/*.ts

📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)

apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)

**/*.{js,jsx,ts,tsx}: Don't use elements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
!pages/_document.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)

!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
apps/web/utils/{ai,llms}/**/*.ts

📄 CodeRabbit Inference Engine (.cursor/rules/llm.mdc)

apps/web/utils/{ai,llms}/**/*.ts: Place LLM-related implementation code under apps/web/utils/ai or apps/web/utils/llms
Keep system and user prompts separate; system defines role/task, user contains data/context
Always validate LLM responses with a specific Zod schema
Use descriptive scoped loggers per feature and log inputs/outputs with appropriate levels and context
Implement early returns for invalid inputs and use proper error types with logging
Add fallbacks for AI failures and include retry logic for transient errors using withRetry
Format prompts with XML-like tags; remove excessive whitespace; truncate overly long inputs; keep formatting consistent
Use TypeScript types for all parameters/returns and define interfaces for complex IO structures
Keep related AI functions co-located; extract shared logic into utilities; document complex AI logic with clear comments
Call LLMs via createGenerateObject; pass system, prompt, and a Zod schema; return the validated result.object
Derive model options using getModel(...) and pass them to createGenerateObject and the generate call

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
**/*.test.{ts,js}

📄 CodeRabbit Inference Engine (.cursor/rules/security.mdc)

Include security tests in your test suites to verify authentication, authorization, and error handling.

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
**/*.test.{ts,js,tsx,jsx}

📄 CodeRabbit Inference Engine (.cursor/rules/testing.mdc)

**/*.test.{ts,js,tsx,jsx}: Tests are colocated next to the tested file (e.g., dir/format.ts and dir/format.test.ts)
Use vi.mock("server-only", () => ({})); to mock the server-only module in tests
Mock @/utils/prisma in tests using vi.mock("@/utils/prisma") and use the provided prisma mock
Mock external dependencies in tests
Clean up mocks between tests
Do not mock the Logger

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)

**/*.{test,spec}.{js,jsx,ts,tsx}: Don't use export or module.exports in test files.
Don't use focused tests.
Don't use disabled tests.
Make sure the assertion function, like expect, is placed inside an it() function call.
Don't nest describe() blocks too deeply in test files.
Don't use focused tests.
Don't use disabled tests.
Don't use export or module.exports in test files.

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
🧠 Learnings (4)
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Keep related AI functions co-located; extract shared logic into utilities; document complex AI logic with clear comments

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
📚 Learning: 2025-07-18T15:06:10.570Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.570Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Test both AI and non-AI paths in LLM-related tests

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
📚 Learning: 2025-07-18T15:06:10.570Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.570Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Include standard test cases: happy path, error handling, edge cases (empty input, null values), different user configurations, and various input formats in LLM-related tests

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
📚 Learning: 2025-07-18T15:06:10.570Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.570Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Use descriptive console.debug statements for generated content in LLM-related tests

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
🧬 Code Graph Analysis (2)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (6)
apps/web/utils/logger.ts (1)
  • createScopedLogger (17-65)
apps/web/utils/types.ts (1)
  • EmailForLLM (117-131)
apps/web/utils/llms/types.ts (1)
  • EmailAccountWithAI (10-29)
apps/web/utils/stringify-email.ts (1)
  • stringifyEmail (4-27)
apps/web/utils/llms/model.ts (1)
  • getModel (27-41)
apps/web/utils/llms/index.ts (1)
  • createGenerateObject (118-166)
apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts (2)
apps/web/__tests__/helpers.ts (1)
  • getEmail (20-37)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (3)
  • aiAnalyzeLabelRemoval (50-129)
  • LabelRemovalAction (12-16)
  • LabelRemovalAction (18-19)
🪛 ast-grep (0.38.6)
apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts

[warning] 75-75: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^(${Object.values(LabelRemovalAction).join("|")})$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html

(regexp-from-variable)

⏰ 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). (2)
  • GitHub Check: Static Code Analysis Js
  • GitHub Check: Jit Security
🔇 Additional comments (4)
apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts (2)

12-15: Good gating of AI tests with describe.runIf

Conditional execution via RUN_AI_TESTS keeps CI stable and avoids hitting live AI unintentionally. LGTM.


116-159: Good breadth of scenarios

The label and content matrices, special chars, and long content cases exercise the main paths well and align with the retrieved learnings. Timeouts are reasonable.

Also applies to: 187-214

apps/web/utils/ai/label-analysis/analyze-label-removal.ts (2)

10-10: Good: scoped logger initialized at top-level

Complies with logging guidelines for backend TS files.


111-118: Model selection + generate call match project standards

getModel("economy") and createGenerateObject usage align with the AI utilities guidance. Returning aiResponse.object after schema validation is correct.

Also applies to: 119-124

@elie222
Copy link
Owner

elie222 commented Aug 25, 2025

Remove WIP when done

@edulelis edulelis changed the title WIP: Add prompt to learn from label removal patterns of newsletters Add prompt to learn from label removal patterns of newsletters Aug 26, 2025
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

Caution

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

⚠️ Outside diff range comments (1)
apps/web/app/api/google/webhook/process-label-removed-event.test.ts (1)

108-116: Rename handleLabelRemovedEvent to processLabelRemoval in webhook tests

The tests in apps/web/app/api/google/webhook/process-label-removed-event.test.ts still reference the old export and need updating:

• Line 4:

-import { handleLabelRemovedEvent } from "./process-label-removed-event";
+import { processLabelRemoval } from "./process-label-removed-event";

• Line 108:

-describe("handleLabelRemovedEvent", () => {
+describe("processLabelRemoval", () => {

• Lines 114, 141, 151, 161, 171, 181, 192, 203: update each call site:

-      await handleLabelRemovedEvent(historyItem, defaultOptions);
+      await processLabelRemoval(historyItem, defaultOptions);

Please apply these changes to ensure the tests import and call the correct symbol.

🧹 Nitpick comments (11)
apps/web/app/api/google/webhook/process-label-removed-event.ts (7)

17-17: Nit: normalize the logger scope for consistency.

Consider aligning the scope with existing patterns (e.g., "google/webhook/label-removed") to make cross-webhook log filtering easier.

-const logger = createScopedLogger("webhook-label-removal");
+const logger = createScopedLogger("google/webhook/label-removed");

115-127: Map label IDs to names via a lookup to reduce O(n×m) scans.

Build a Map once and then resolve IDs in O(1). This is minor here but cleaner and scales better.

-    const removedLabelNames = removedLabelIds
-      .map((labelId: string) => {
-        const label = labels.find((l) => l.id === labelId);
-        return label?.name;
-      })
-      .filter(
-        (labelName: string | null | undefined): labelName is string =>
-          !!labelName && !SYSTEM_LABELS.includes(labelName),
-      );
+    const labelMap = new Map(labels.map((l) => [l.id, l.name]));
+    const removedLabelNames = removedLabelIds
+      .map((labelId) => labelMap.get(labelId))
+      .filter(
+        (labelName: string | undefined): labelName is string =>
+          !!labelName && !SYSTEM_LABELS.includes(labelName),
+      );

129-138: Prefetch system rule labels once and pass into analyseLabelRemoval.

Currently, analyseLabelRemoval calls the DB for each removed label. Fetch once per event, then reuse. Optionally, process labels concurrently if provider/AI rate limits allow.

-  for (const labelName of removedLabelNames) {
-      await analyseLabelRemoval({
-        labelName,
-        messageId,
-        threadId,
-        emailAccountId,
-        parsedMessage,
-        emailAccount,
-      });
-    }
+  const systemRuleLabels = await getSystemRuleLabels(emailAccountId);
+  for (const labelName of removedLabelNames) {
+    await analyseLabelRemoval({
+      labelName,
+      messageId,
+      threadId,
+      emailAccountId,
+      parsedMessage,
+      emailAccount,
+      systemRuleLabels,
+    });
+  }

And update the analyseLabelRemoval signature and usage:

-async function analyseLabelRemoval({
+async function analyseLabelRemoval({
   labelName,
   messageId,
   threadId,
   emailAccountId,
   parsedMessage,
   emailAccount,
+  systemRuleLabels,
 }: {
   labelName: string;
   messageId: string;
   threadId: string;
   emailAccountId: string;
   parsedMessage: ParsedMessage;
   emailAccount: EmailAccountWithAI;
+  systemRuleLabels: Map<SystemType, { labels: string[]; instructions: string | null; ruleName: string }>;
 }) {
@@
-  const systemTypeData = await getSystemRuleLabels(emailAccountId);
+  const systemTypeData = systemRuleLabels;

Optional concurrency (verify side effects/rate limits before adopting):

// const results = await Promise.all(removedLabelNames.map((labelName) => analyseLabelRemoval({ ... })));
// (void) results;

Please confirm Gmail/LLM rate limits and DB write constraints before switching to Promise.all.


139-147: Log errors safely to preserve stack/message without lossy JSON.stringify(Error).

JSON.stringify on Error yields {}. Serialize explicitly.

-  } catch (error) {
-    logger.error("Error processing label removal", {
-      error,
-      email: userEmail,
-      messageId,
-      threadId,
-    });
-  }
+  } catch (error) {
+    const err =
+      error instanceof Error
+        ? { message: error.message, stack: error.stack }
+        : error;
+    logger.error("Error processing label removal", {
+      error: err,
+      email: userEmail,
+      messageId,
+      threadId,
+    });
+  }

204-220: Type matchedRule.systemType as SystemType for accurate comparisons.

This aligns with the SystemType.NEWSLETTER check later and prevents accidental mis-keys.

-  let matchedRule: {
-    systemType: string;
-    instructions: string | null;
-    ruleName: string;
-  } | null = null;
+  let matchedRule:
+    | {
+        systemType: SystemType;
+        instructions: string | null;
+        ruleName: string;
+      }
+    | null = null;
@@
-  for (const [systemType, data] of systemTypeData) {
+  for (const [systemType, data] of systemTypeData) {
     if (data.labels.includes(labelName)) {
       matchedRule = {
         systemType,
         instructions: data.instructions,
         ruleName: data.ruleName,
       };
       break;
     }
   }

255-260: Apply the same safe error serialization for AI errors.

-  } catch (error) {
-    logger.error("Error analyzing label removal with AI", {
-      emailAccountId,
-      error,
-    });
-  }
+  } catch (error) {
+    const err =
+      error instanceof Error
+        ? { message: error.message, stack: error.stack }
+        : error;
+    logger.error("Error analyzing label removal with AI", {
+      emailAccountId,
+      error: err,
+    });
+  }

298-309: Check and log saveLearnedPatterns result to surface persistence errors.

saveLearnedPatterns returns either { success: true } or an object with errors; currently ignored.

-        await saveLearnedPatterns({
+        const result = await saveLearnedPatterns({
           emailAccountId,
           ruleName,
           patterns: [
             {
               type: analysis.patternType,
               value: analysis.patternValue,
               exclude: excludeValue,
               reasoning: analysis.reasoning,
             },
           ],
         });
+        if (result && "errors" in result && result.errors?.length) {
+          logger.warn("Failed to persist learned patterns", {
+            emailAccountId,
+            ruleName,
+            errors: result.errors,
+          });
+        }
apps/web/app/api/google/webhook/process-label-removed-event.test.ts (4)

66-83: Remove unused mockGmail; the new handler doesn’t use Gmail API client directly.

The production code uses provider.getLabels(), not gmail.users.labels.list. This mock is dead weight and can confuse future readers.

-  const mockGmail = {
-    users: {
-      labels: {
-        list: vi.fn().mockImplementation((_params) => {
-          return Promise.resolve({
-            data: {
-              labels: [
-                { id: "label-1", name: inboxZeroLabels.cold_email.name },
-                { id: "label-2", name: "Newsletter" },
-                { id: "label-3", name: "Marketing" },
-                { id: "label-4", name: "To Reply" },
-              ],
-            },
-          });
-        }),
-      },
-    },
-  } as any;
+  // No gmail client mock needed; the handler uses provider.getLabels().

102-106: Drop the unused gmail option from defaultOptions.

-  const defaultOptions = {
-    gmail: mockGmail,
-    emailAccount: mockEmailAccount,
-    provider: mockProvider,
-  };
+  const defaultOptions = {
+    emailAccount: mockEmailAccount,
+    provider: mockProvider,
+  };

49-53: Add a default mock for prisma.action.findMany to prevent unintended crashes on newsletter paths.

analyseLabelRemoval now queries for system rules. Default to no rules in these tests unless you explicitly set them.

 describe("process-label-removed-event", () => {
   beforeEach(() => {
     vi.clearAllMocks();
+    // By default, no system-type label rules exist in these tests
+    (prisma.action.findMany as any)?.mockResolvedValue?.([]);
   });

108-108: Rename the suite to match the current API.

-  describe("handleLabelRemovedEvent", () => {
+  describe("processLabelRemoval", () => {
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e055b1d and 3c72055.

📒 Files selected for processing (4)
  • apps/web/app/api/google/webhook/process-history-item.ts (2 hunks)
  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts (3 hunks)
  • apps/web/app/api/google/webhook/process-label-removed-event.ts (4 hunks)
  • apps/web/prisma/schema.prisma (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/web/prisma/schema.prisma
  • apps/web/app/api/google/webhook/process-history-item.ts
🧰 Additional context used
📓 Path-based instructions (11)
apps/web/**/*.{ts,tsx}

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

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use @/ for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX

Files:

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

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

NextJS app router structure with (app) directory

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
!{.cursor/rules/*.mdc}

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

Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location

Files:

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

📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)

**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod

Files:

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

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

**/*.{ts,tsx}: Use createScopedLogger for logging in backend TypeScript files
Typically add the logger initialization at the top of the file when using createScopedLogger
Only use .with() on a logger instance within a specific function, not for a global logger

Import Prisma in the project using import prisma from "@/utils/prisma";

**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.

Files:

  • apps/web/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/**/*.{ts,js}

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

apps/web/app/api/**/*.{ts,js}: All API route handlers in 'apps/web/app/api/' must use authentication middleware: withAuth, withEmailAccount, or withError (with custom authentication logic).
All Prisma queries in API routes must include user/account filtering (e.g., emailAccountId or userId in WHERE clauses) to prevent unauthorized data access.
All parameters used in API routes must be validated before use; do not use parameters from 'params' or request bodies directly in queries without validation.
Request bodies in API routes should use Zod schemas for validation.
API routes should only return necessary fields using Prisma's 'select' and must not include sensitive data in error messages.
Error messages in API routes must not reveal internal details; use generic errors and SafeError for user-facing errors.
All QStash endpoints (API routes called via publishToQstash or publishToQstashQueue) must use verifySignatureAppRouter to verify request authenticity.
All cron endpoints in API routes must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak or plaintext secrets in API route files; secrets must not be directly assigned as string literals.
Review all new withError usage in API routes to ensure custom authentication is implemented where required.

Files:

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

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

**/*.{js,jsx,ts,tsx}: Don't use elements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
!pages/_document.{js,jsx,ts,tsx}

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

!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.

Files:

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

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

Include security tests in your test suites to verify authentication, authorization, and error handling.

Files:

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

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

**/*.test.{ts,js,tsx,jsx}: Tests are colocated next to the tested file (e.g., dir/format.ts and dir/format.test.ts)
Use vi.mock("server-only", () => ({})); to mock the server-only module in tests
Mock @/utils/prisma in tests using vi.mock("@/utils/prisma") and use the provided prisma mock
Mock external dependencies in tests
Clean up mocks between tests
Do not mock the Logger

Files:

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

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

**/*.{test,spec}.{js,jsx,ts,tsx}: Don't use export or module.exports in test files.
Don't use focused tests.
Don't use disabled tests.
Make sure the assertion function, like expect, is placed inside an it() function call.
Don't nest describe() blocks too deeply in test files.
Don't use focused tests.
Don't use disabled tests.
Don't use export or module.exports in test files.

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
🧬 Code graph analysis (2)
apps/web/app/api/google/webhook/process-label-removed-event.ts (8)
apps/web/utils/logger.ts (1)
  • createScopedLogger (17-65)
apps/web/utils/types.ts (1)
  • ParsedMessage (55-70)
apps/web/utils/llms/types.ts (1)
  • EmailAccountWithAI (10-29)
apps/web/utils/email.ts (1)
  • extractEmailAddress (19-52)
apps/web/utils/label.ts (1)
  • inboxZeroLabels (32-68)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (4)
  • aiAnalyzeLabelRemoval (50-129)
  • LabelRemovalAnalysis (48-48)
  • LabelRemovalAction (12-16)
  • LabelRemovalAction (18-19)
apps/web/utils/get-email-from-message.ts (1)
  • getEmailForLLM (6-25)
apps/web/utils/rule/learned-patterns.ts (1)
  • saveLearnedPatterns (76-181)
apps/web/app/api/google/webhook/process-label-removed-event.test.ts (1)
apps/web/utils/label.ts (1)
  • inboxZeroLabels (32-68)
🪛 GitHub Check: test
apps/web/app/api/google/webhook/process-label-removed-event.test.ts

[failure] 114-114: app/api/google/webhook/process-label-removed-event.test.ts > process-label-removed-event > handleLabelRemovedEvent > should process Cold Email label removal and update ColdEmail status
TypeError: handleLabelRemovedEvent is not a function
❯ app/api/google/webhook/process-label-removed-event.test.ts:114:13

🪛 GitHub Actions: Run Tests
apps/web/app/api/google/webhook/process-label-removed-event.test.ts

[error] 114-114: TypeError: handleLabelRemovedEvent is not a function


[error] 141-141: TypeError: handleLabelRemovedEvent is not a function


[error] 151-151: TypeError: handleLabelRemovedEvent is not a function


[error] 161-161: TypeError: handleLabelRemovedEvent is not a function


[error] 171-171: TypeError: handleLabelRemovedEvent is not a function


[error] 181-181: TypeError: handleLabelRemovedEvent is not a function


[error] 192-192: TypeError: handleLabelRemovedEvent is not a function


[error] 203-203: TypeError: handleLabelRemovedEvent is not a function

🔇 Additional comments (4)
apps/web/app/api/google/webhook/process-label-removed-event.ts (3)

3-16: Good adoption of strongly typed imports and the scoped logger utility.

Brings in only what’s needed and uses the shared logger as per guidelines. No issues here.


222-228: Sanity check: do we intend to skip Gmail category labels (SOCIAL, PROMOTIONS, UPDATES, FORUMS, etc.)?

SYSTEM_LABELS excludes category labels, so category removals will enter AI/learning. If that’s unintentional, add them to SYSTEM_LABELS.

I can open a follow-up to either add categories to SYSTEM_LABELS or add tests clarifying the intended behavior.


30-80: Use SystemType as Map Key and Remove Non-null Assertions

Per the Prisma schema, Rule has a unique constraint on (emailAccountId, systemType), so there can never be more than one rule per systemType for a given account. Aggregating all labels under a single SystemType key—and carrying its single instructions/ruleName payload—is therefore safe and deterministic.

Apply this refactor to:

  • Change the Map key from stringSystemType
  • Remove all ! non-null assertions
  • Simplify the aggregation logic to push into an existing entry or create a new one
-export async function getSystemRuleLabels(
-  emailAccountId: string,
-): Promise<
-  Map<
-    string,
-    { labels: string[]; instructions: string | null; ruleName: string }
-  >
-> {
+export async function getSystemRuleLabels(
+  emailAccountId: string,
+): Promise<
+  Map<
+    SystemType,
+    { labels: string[]; instructions: string | null; ruleName: string }
+  >
+> {
   const actions = await prisma.action.findMany({
     where: {
       type: ActionType.LABEL,
       rule: {
         emailAccountId,
         systemType: {
           not: null,
         },
       },
     },
     select: {
       label: true,
       rule: {
         select: {
           name: true,
           systemType: true,
           instructions: true,
         },
       },
     },
   });

-  const systemTypeData = new Map<
-    string,
-    { labels: string[]; instructions: string | null; ruleName: string }
-  >();
+  const systemTypeData = new Map<
+    SystemType,
+    { labels: string[]; instructions: string | null; ruleName: string }
+  >();

   for (const action of actions) {
-    if (action.label && action.rule?.systemType && action.rule?.name) {
-      const systemType = action.rule.systemType;
-      if (!systemTypeData.has(systemType)) {
-        systemTypeData.set(systemType, {
-          labels: [],
-          instructions: action.rule.instructions,
-          ruleName: action.rule.name,
-        });
-      }
-      systemTypeData.get(systemType)!.labels.push(action.label);
-    }
+    if (action.label && action.rule?.systemType && action.rule?.name) {
+      const systemType = action.rule.systemType as SystemType;
+      const existing = systemTypeData.get(systemType);
+      if (existing) {
+        existing.labels.push(action.label);
+      } else {
+        systemTypeData.set(systemType, {
+          labels: [action.label],
+          instructions: action.rule.instructions,
+          ruleName: action.rule.name,
+        });
+      }
+    }
   }

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

136-185: Outdated assertions/descriptions imply only Cold Email is supported.

With the new AI path, Newsletter removals may trigger analysis if a matching SystemType.NEWSLETTER rule exists. Your “skip learning” assertions remain valid only when no such rule is present (we’ve mocked findMany to return []). Consider adding a positive-path test (with a mocked NEWSLETTER rule and mocked aiAnalyzeLabelRemoval) to assert saveLearnedPatterns gets called appropriately.

I can draft a follow-up test that:

  • mocks prisma.action.findMany to return a NEWSLETTER rule,
  • stubs aiAnalyzeLabelRemoval to return an EXCLUDE_PATTERN,
  • asserts saveLearnedPatterns is called with the inferred pattern.

@edulelis edulelis changed the title Add prompt to learn from label removal patterns of newsletters WIP: Add prompt to learn from label removal patterns of newsletters Aug 26, 2025
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

Caution

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

⚠️ Outside diff range comments (1)
apps/web/app/api/google/webhook/process-label-removed-event.ts (1)

30-317: Update residual references to handleLabelRemovedEvent in tests

The grep output shows that the test file is still importing and invoking the old handleLabelRemovedEvent identifier rather than the new processLabelRemoval function:

• File: apps/web/app/api/google/webhook/process-label-removed-event.test.ts
– import line 4: import { handleLabelRemovedEvent } from "./process-label-removed-event";
– multiple calls and describes around handleLabelRemovedEvent

Please update the test suite to:

  • Replace all imports of handleLabelRemovedEvent with processLabelRemoval.
  • Rename any test descriptions, calls, and mock expectations from handleLabelRemovedEvent to processLabelRemoval.

This will ensure the tests correctly reference and cover the refactored webhook handler.

🧹 Nitpick comments (15)
apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts (4)

91-96: Console logging in tests: keep, but gate to avoid noisy/PII logs

Project guideline says “don’t use console,” but retrieved learnings allow console.debug in LLM tests. Still, consider gating with an env flag to limit CI noise and potential PII exposure.

Apply this diff to wrap the debug logs:

-      console.debug(
+      if (process.env.DEBUG_AI_TEST_LOGS === "true") console.debug(
         "Generated patterns:\n",
         JSON.stringify(result.patterns, null, 2),
       );

Repeat the same wrapping for the other console.debug calls in this file.

Also applies to: 200-205, 333-337


235-252: Preserve aiApiKey when overriding user AI config in tests

Overriding user with { ...config } drops aiApiKey. Merge with base user to keep shape consistent.

-      const testData = getTestData({
-        emailAccount: getEmailAccount({
-          user: { ...config },
-        }),
-      });
+      const testData = getTestData({
+        emailAccount: getEmailAccount({
+          user: { ...getUser(), ...config },
+        }),
+      });

340-375: Strengthen assertion: verify an exclude pattern is actually learned

Given the scenario, assert at least one pattern has exclude === true, and optionally constrain type to FROM/SUBJECT to catch regressions.

-      const excludePatterns = result.patterns.filter((p) => p.exclude === true);
-      expect(excludePatterns.length).toBeGreaterThan(0);
+      const excludePatterns = result.patterns.filter((p) => p.exclude === true);
+      expect(excludePatterns.length).toBeGreaterThan(0);
+      // Optional: ensure learned pattern type is sensible
+      expect(excludePatterns.some((p) => [GroupItemType.FROM, GroupItemType.SUBJECT].includes(p.type))).toBe(true);

300-316: Error-path test is weak; make it deterministic

Currently relies on content length to maybe trigger errors. Force an error by mocking the generator to throw, then assert error handling path.

-    const testData = getTestData({
-      email: getEmail({
-        content: "A".repeat(10_000), // Very long content that might exceed limits
-      }),
-    });
-
-    try {
-      const result = await aiAnalyzeLabelRemoval(testData);
-      expect(result.patterns).toBeDefined();
-    } catch (error) {
-      // If the AI processing fails, we should get a meaningful error
-      expect(error).toBeDefined();
-      console.debug("Error occurred as expected:\n", error);
-    }
+    // Force the underlying generate call to throw
+    const { createGenerateObject } = await vi.importActual<any>("@/utils/llms");
+    const spy = vi.spyOn(await import("@/utils/llms"), "createGenerateObject")
+      .mockReturnValue(async () => { throw new Error("forced"); });
+    const testData = getTestData();
+    await expect(aiAnalyzeLabelRemoval(testData)).rejects.toThrow("forced");
+    spy.mockRestore();
apps/web/app/api/google/webhook/process-label-removed-event.ts (7)

19-28: Filter Gmail category labels too

Avoid learning from Gmail’s auto categories. Add CATEGORY_* constants if available.

   GmailLabel.INBOX,
+  GmailLabel.CATEGORY_PERSONAL,
+  GmailLabel.CATEGORY_SOCIAL,
+  GmailLabel.CATEGORY_PROMOTIONS,
+  GmailLabel.CATEGORY_UPDATES,
+  GmailLabel.CATEGORY_FORUMS,
   GmailLabel.SENT,

If these enum members don’t exist in GmailLabel, map by name strings or extend the enum wrapper accordingly.


30-37: Type the map by SystemType, not string

Prevents typos and mismatches later.

-export async function getSystemRuleLabels(
-  emailAccountId: string,
-): Promise<
-  Map<
-    string,
-    { labels: string[]; instructions: string | null; ruleName: string }
-  >
-> {
+export async function getSystemRuleLabels(
+  emailAccountId: string,
+): Promise<Map<SystemType, { labels: string[]; instructions: string | null; ruleName: string }>> {

Also update local generics at Lines 60-77 to use Map<SystemType, ...>.


65-77: Multiple rules per SystemType: current structure loses rule multiplicity

If more than one rule shares a SystemType, ruleName/instructions get overwritten, and patterns are saved only against whichever survived. Consider storing an array per SystemType or keying by ruleId instead of SystemType.

I can draft a refactor mapping label name → array of { systemType, ruleName, instructions } to preserve all matches if desired.


119-127: Build a labelId→name map once

Minor perf/readability: avoid O(n×m) searches.

-  const removedLabelNames = removedLabelIds
-    .map((labelId: string) => {
-      const label = labels.find((l) => l.id === labelId);
-      return label?.name;
-    })
+  const nameById = new Map(labels.map((l) => [l.id, l.name]));
+  const removedLabelNames = removedLabelIds
+    .map((labelId: string) => nameById.get(labelId))
     .filter(

231-242: NEWSLETTER-only AI: log includes messageId, good; consider expanding later

Current scope is intentional. When expanding, drive by SystemType and add targeted prompts.


295-309: Truncate reasoning before save to avoid DB constraint surprises

If GroupItem.reasoning has a length limit, truncate defensively.

-    const patternsToSave = analysis.patterns.map((pattern) => {
+    const patternsToSave = analysis.patterns.map((pattern) => {
+      const maxReasonLen = 500;
+      const reasoning =
+        typeof pattern.reasoning === "string"
+          ? pattern.reasoning.slice(0, maxReasonLen)
+          : undefined;
       return {
         type: pattern.type,
         value: pattern.value,
         exclude: pattern.exclude,
-        reasoning: pattern.reasoning,
+        reasoning,
       };
     });

82-146: Rename analyseLabelRemoval to analyzeLabelRemoval for spelling consistency

The internal helper is currently using British spelling (“analyse”), but elsewhere in this file—and throughout the codebase—we use American spelling (“analyze”), so let’s align on “analyzeLabelRemoval.”

Files to update:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
    • Line 129: update the call site
    • Line 147: update the function declaration

Suggested diff:

--- a/apps/web/app/api/google/webhook/process-label-removed-event.ts
+++ b/apps/web/app/api/google/webhook/process-label-removed-event.ts
@@ -126,7 +126,7 @@ export async function processLabelRemoval(
     emailAccount,
   }: {
     emailAccount: EmailAccountWithAI;
     provider: EmailProvider;
   },
 ) {
   …
-  await analyseLabelRemoval({
+  await analyzeLabelRemoval({
     labelNames: removedLabelNames,
     messageId,
     threadId,
@@ -144,7 +144,7 @@ export async function processLabelRemoval(
   } catch (error) {
     logger.error("Error processing label removal", {
       error,
       email: userEmail,
       messageId,
       threadId,
     });
 }

-async function analyseLabelRemoval({
+async function analyzeLabelRemoval({
   labelNames,
   messageId,
   threadId,
   emailAccountId,
   parsedMessage,
   emailAccount,
 }: {
   labelNames: string[];
   messageId: string;
   threadId: string;
   emailAccountId: string;
   parsedMessage: GmailMessage;
   emailAccount: EmailAccountWithAI;
 }): Promise<void> {
   // …
}
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (4)

9-17: Dead code: LabelRemovalAction is unused

Remove until needed or include in the schema/output.

-export const LabelRemovalAction = {
-  NO_ACTION: "no_action",
-  EXCLUDE_PATTERN: "exclude_pattern",
-  NOT_INCLUDE: "not_include",
-} as const;
-
-export type LabelRemovalAction =
-  (typeof LabelRemovalAction)[keyof typeof LabelRemovalAction];
+// (reserved) LabelRemovalAction: add when action routing is implemented

27-31: Description mismatch for pattern.type

“SUBJECT (sender/domain/subject keyword)” is confusing; SUBJECT should not mention sender/domain.

-          .describe(
-            "The pattern type which can be FROM (sender/domain), SUBJECT (sender/domain/subject keyword), or BODY (content)",
-          ),
+          .describe(
+            "Pattern type: FROM (address or domain), SUBJECT (keywords), or BODY (content keywords)",
+          ),

63-88: Tighten the system prompt to reduce false positives and output size

Cap patterns, scope to newsletters, and bound reasoning length for consistency with DB and logs.

   const system = `You are an assistant that manages learned patterns in Inbox Zero.  
 You cannot act directly on the inbox; only can propose learned patterns or no action.
@@
 Guidelines
 - Only propose a learned pattern if you are highly confident that the removal reflects a repeated behavior.
 Do not generate an exclude pattern if the AI correctly categorized the email; only create excludes when a label was wrongly applied and removed by the user.  
 - Use the labels removed and any instructions for adding them in the first place as context to determine the appropriate action and whether a learned pattern should be generated.
+Output constraints
+- Return at most 3 patterns.
+- Prefer domain-level patterns over full addresses when safe (e.g., example.com instead of alice@example.com).
+- Keep reasoning concise (<= 240 chars).
+- If the label is not clearly a misclassification of a newsletter, return patterns: null.
 `;

55-62: Strongly type systemType as Prisma SystemType

Prevents accidental string drift between producer and consumer.

-import type { EmailForLLM } from "@/utils/types";
+import type { EmailForLLM } from "@/utils/types";
+import type { SystemType } from "@prisma/client";
@@
-  matchedRules: {
-    systemType: string;
+  matchedRules: {
+    systemType: SystemType;
     instructions: string | null;
     ruleName: string;
   }[];

Ensure callers (process-label-removed-event.ts) pass SystemType.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8743fb2 and 9c99ac1.

📒 Files selected for processing (3)
  • apps/web/app/api/google/webhook/process-label-removed-event.ts (4 hunks)
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts (1 hunks)
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
apps/web/**/*.{ts,tsx}

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

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use @/ for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
apps/web/app/**

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

NextJS app router structure with (app) directory

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
!{.cursor/rules/*.mdc}

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

Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)

**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
**/*.{ts,tsx}

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

**/*.{ts,tsx}: Use createScopedLogger for logging in backend TypeScript files
Typically add the logger initialization at the top of the file when using createScopedLogger
Only use .with() on a logger instance within a specific function, not for a global logger

Import Prisma in the project using import prisma from "@/utils/prisma";

**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
apps/web/app/api/**/*.{ts,js}

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

apps/web/app/api/**/*.{ts,js}: All API route handlers in 'apps/web/app/api/' must use authentication middleware: withAuth, withEmailAccount, or withError (with custom authentication logic).
All Prisma queries in API routes must include user/account filtering (e.g., emailAccountId or userId in WHERE clauses) to prevent unauthorized data access.
All parameters used in API routes must be validated before use; do not use parameters from 'params' or request bodies directly in queries without validation.
Request bodies in API routes should use Zod schemas for validation.
API routes should only return necessary fields using Prisma's 'select' and must not include sensitive data in error messages.
Error messages in API routes must not reveal internal details; use generic errors and SafeError for user-facing errors.
All QStash endpoints (API routes called via publishToQstash or publishToQstashQueue) must use verifySignatureAppRouter to verify request authenticity.
All cron endpoints in API routes must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak or plaintext secrets in API route files; secrets must not be directly assigned as string literals.
Review all new withError usage in API routes to ensure custom authentication is implemented where required.

Files:

  • 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 elements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
!pages/_document.{js,jsx,ts,tsx}

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

!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
**/*.test.{ts,js}

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

Include security tests in your test suites to verify authentication, authorization, and error handling.

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
**/*.test.{ts,js,tsx,jsx}

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

**/*.test.{ts,js,tsx,jsx}: Tests are colocated next to the tested file (e.g., dir/format.ts and dir/format.test.ts)
Use vi.mock("server-only", () => ({})); to mock the server-only module in tests
Mock @/utils/prisma in tests using vi.mock("@/utils/prisma") and use the provided prisma mock
Mock external dependencies in tests
Clean up mocks between tests
Do not mock the Logger

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
apps/web/utils/**

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

Create utility functions in utils/ folder for reusable logic

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
apps/web/utils/**/*.ts

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

apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
**/*.{test,spec}.{js,jsx,ts,tsx}

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

**/*.{test,spec}.{js,jsx,ts,tsx}: Don't use export or module.exports in test files.
Don't use focused tests.
Don't use disabled tests.
Make sure the assertion function, like expect, is placed inside an it() function call.
Don't nest describe() blocks too deeply in test files.
Don't use focused tests.
Don't use disabled tests.
Don't use export or module.exports in test files.

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
apps/web/utils/{ai,llms}/**/*.ts

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

apps/web/utils/{ai,llms}/**/*.ts: Place LLM-related implementation code under apps/web/utils/ai or apps/web/utils/llms
Keep system and user prompts separate; system defines role/task, user contains data/context
Always validate LLM responses with a specific Zod schema
Use descriptive scoped loggers per feature and log inputs/outputs with appropriate levels and context
Implement early returns for invalid inputs and use proper error types with logging
Add fallbacks for AI failures and include retry logic for transient errors using withRetry
Format prompts with XML-like tags; remove excessive whitespace; truncate overly long inputs; keep formatting consistent
Use TypeScript types for all parameters/returns and define interfaces for complex IO structures
Keep related AI functions co-located; extract shared logic into utilities; document complex AI logic with clear comments
Call LLMs via createGenerateObject; pass system, prompt, and a Zod schema; return the validated result.object
Derive model options using getModel(...) and pass them to createGenerateObject and the generate call

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
🧠 Learnings (9)
📚 Learning: 2025-07-18T15:06:10.570Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.570Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Test both AI and non-AI paths in LLM-related tests

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Keep related AI functions co-located; extract shared logic into utilities; document complex AI logic with clear comments

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
📚 Learning: 2025-07-18T15:06:10.570Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.570Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Include standard test cases: happy path, error handling, edge cases (empty input, null values), different user configurations, and various input formats in LLM-related tests

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
📚 Learning: 2025-07-18T15:06:10.570Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.570Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Use descriptive console.debug statements for generated content in LLM-related tests

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Place LLM-related implementation code under apps/web/utils/ai or apps/web/utils/llms

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Format prompts with XML-like tags; remove excessive whitespace; truncate overly long inputs; keep formatting consistent

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Keep system and user prompts separate; system defines role/task, user contains data/context

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Use descriptive scoped loggers per feature and log inputs/outputs with appropriate levels and context

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Derive model options using getModel(...) and pass them to createGenerateObject and the generate call

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
🧬 Code graph analysis (3)
apps/web/app/api/google/webhook/process-label-removed-event.ts (8)
apps/web/utils/logger.ts (1)
  • createScopedLogger (17-65)
apps/web/utils/types.ts (1)
  • ParsedMessage (55-70)
apps/web/utils/llms/types.ts (1)
  • EmailAccountWithAI (10-29)
apps/web/utils/email.ts (1)
  • extractEmailAddress (19-52)
apps/web/utils/label.ts (1)
  • inboxZeroLabels (32-68)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (2)
  • aiAnalyzeLabelRemoval (50-127)
  • LabelRemovalAnalysis (48-48)
apps/web/utils/get-email-from-message.ts (1)
  • getEmailForLLM (6-25)
apps/web/utils/rule/learned-patterns.ts (1)
  • saveLearnedPatterns (76-181)
apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts (2)
apps/web/__tests__/helpers.ts (1)
  • getEmail (20-37)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (1)
  • aiAnalyzeLabelRemoval (50-127)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (5)
apps/web/utils/types.ts (1)
  • EmailForLLM (117-131)
apps/web/utils/llms/types.ts (1)
  • EmailAccountWithAI (10-29)
apps/web/utils/stringify-email.ts (1)
  • stringifyEmail (4-27)
apps/web/utils/llms/model.ts (1)
  • getModel (27-41)
apps/web/utils/llms/index.ts (1)
  • createGenerateObject (118-166)
⏰ 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). (2)
  • GitHub Check: Static Code Analysis Js
  • GitHub Check: Jit Security
🔇 Additional comments (7)
apps/web/utils/ai/label-analysis/analyze-label-removal.test.ts (2)

9-12: Good guard to keep AI tests opt-in

RUN_AI_TESTS gate avoids flaky CI. LGTM.


126-152: Note: Integration path only invokes AI for NEWSLETTER

This unit test exercises the AI function directly across multiple system types, which is fine. Ensure integration tests cover that non-NEWSLETTER removals skip AI.

If not already covered in apps/web/app/api/google/webhook/process-label-removed-event.test.ts, add tests asserting AI is skipped for TO_REPLY/IMPORTANT, and that we log “System type not yet supported for AI analysis”.

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

147-171: Good early return when sender missing

Prevents learning without a target. LGTM.


174-200: Cold Email upsert path looks solid

Handles both create and update; covered by outer try/catch. LGTM.


223-229: Helpful skip log when no system rules match

Nice guardrail. LGTM.


256-261: AI error handling is adequate

Logs error with account context; outer flow continues. LGTM.

apps/web/utils/ai/label-analysis/analyze-label-removal.ts (1)

111-127: No raw prompt/system logging here — good

Relies on createGenerateObject’s safe truncation. LGTM.

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

Caution

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

⚠️ Outside diff range comments (1)
apps/web/app/api/google/webhook/process-label-removed-event.test.ts (1)

136-144: Update Newsletter label removal test to expect learning

The test case at lines 136–144 still asserts saveLearnedPatterns is not called, but per this PR removing the “Newsletter” label should now trigger AI-driven learning. Please update as follows:

• Rename the test description

-    it("should skip learning when Newsletter label is removed (only Cold Email is supported)", async () => {
+    it("should learn patterns when Newsletter label is removed", async () => {

• Mock the AI analyzer to return a sample patterns array before invoking processLabelRemoval

vi.mock("@/utils/ai/label-analysis/analyze-label-removal", () => ({
  aiAnalyzeLabelRemoval: vi.fn().mockResolvedValue({
    patterns: [
      { type: "from", value: "newsletter@example.com", exclude: false, reasoning: "matched newsletter" }
    ]
  }),
}));

• Replace the expectation to assert saveLearnedPatterns is called once with the expected shape

      await processLabelRemoval(historyItem.item, defaultOptions);
-     expect(saveLearnedPatterns).not.toHaveBeenCalled();
+     expect(saveLearnedPatterns).toHaveBeenCalledTimes(1);
+     expect(saveLearnedPatterns).toHaveBeenCalledWith(
+       expect.objectContaining({
+         emailAccountId: "email-account-id",
+         ruleName: expect.any(String),
+         patterns: expect.arrayContaining([
+           expect.objectContaining({ value: "newsletter@example.com" })
+         ])
+       })
+     );

This aligns the test with the new AI-driven learning behavior for Newsletter label removals.

🧹 Nitpick comments (4)
apps/web/app/api/google/webhook/process-label-removed-event.test.ts (4)

66-83: Remove unused Gmail REST mock.

processLabelRemoval uses provider.getLabels(), not gmail.users.labels.list. This mock is unused and adds noise.

Apply:

-  const mockGmail = {
-    users: {
-      labels: {
-        list: vi.fn().mockImplementation((_params) => {
-          return Promise.resolve({
-            data: {
-              labels: [
-                { id: "label-1", name: inboxZeroLabels.cold_email.name },
-                { id: "label-2", name: "Newsletter" },
-                { id: "label-3", name: "Marketing" },
-                { id: "label-4", name: "To Reply" },
-              ],
-            },
-          });
-        }),
-      },
-    },
-  } as any;

102-106: Drop unused defaultOptions.gmail.

After removing the Gmail mock, prune the options object.

Apply:

-  const defaultOptions = {
-    gmail: mockGmail,
-    emailAccount: mockEmailAccount,
-    provider: mockProvider,
-  };
+  const defaultOptions = {
+    emailAccount: mockEmailAccount,
+    provider: mockProvider,
+  };

88-101: Eliminate as any by typing the provider mocks.

Prefer typed vi.fn generics for return shapes to keep tests type-safe.

Apply:

-  const mockProvider = {
-    getMessage: vi.fn().mockResolvedValue({
-      headers: {
-        from: "sender@example.com",
-      },
-    }),
-    getLabels: vi.fn().mockResolvedValue([
+  const mockProvider = {
+    getMessage: vi
+      .fn<[string], Promise<{ headers: { from: string } }>>()
+      .mockResolvedValue({
+        headers: { from: "sender@example.com" },
+      }),
+    getLabels: vi
+      .fn<[], Promise<Array<{ id: string; name: string; type: "user" }>>>()
+      .mockResolvedValue([
       { id: "label-1", name: inboxZeroLabels.cold_email.name, type: "user" },
       { id: "label-2", name: "Newsletter", type: "user" },
       { id: "label-3", name: "Marketing", type: "user" },
       { id: "label-4", name: "To Reply", type: "user" },
-    ]),
-  } as any;
+      ]),
+  } as const;

156-165: These two tests are currently indistinguishable; refine scenarios or dedupe.

Both use label-2 and assert "skip learning" with identical setup. Consider stubbing the analyzer downstream to simulate:

  • "no executed rule exists"
  • "no matching LABEL action"
    to make each test assert a distinct branch, or merge into one.

Also applies to: 166-175

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9c99ac1 and 50cca0c.

📒 Files selected for processing (1)
  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts (10 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
apps/web/**/*.{ts,tsx}

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

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use @/ for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX

Files:

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

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

NextJS app router structure with (app) directory

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
!{.cursor/rules/*.mdc}

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

Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location

Files:

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

📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)

**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod

Files:

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

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

**/*.{ts,tsx}: Use createScopedLogger for logging in backend TypeScript files
Typically add the logger initialization at the top of the file when using createScopedLogger
Only use .with() on a logger instance within a specific function, not for a global logger

Import Prisma in the project using import prisma from "@/utils/prisma";

**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.

Files:

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

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

Include security tests in your test suites to verify authentication, authorization, and error handling.

Files:

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

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

**/*.test.{ts,js,tsx,jsx}: Tests are colocated next to the tested file (e.g., dir/format.ts and dir/format.test.ts)
Use vi.mock("server-only", () => ({})); to mock the server-only module in tests
Mock @/utils/prisma in tests using vi.mock("@/utils/prisma") and use the provided prisma mock
Mock external dependencies in tests
Clean up mocks between tests
Do not mock the Logger

Files:

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

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

apps/web/app/api/**/*.{ts,js}: All API route handlers in 'apps/web/app/api/' must use authentication middleware: withAuth, withEmailAccount, or withError (with custom authentication logic).
All Prisma queries in API routes must include user/account filtering (e.g., emailAccountId or userId in WHERE clauses) to prevent unauthorized data access.
All parameters used in API routes must be validated before use; do not use parameters from 'params' or request bodies directly in queries without validation.
Request bodies in API routes should use Zod schemas for validation.
API routes should only return necessary fields using Prisma's 'select' and must not include sensitive data in error messages.
Error messages in API routes must not reveal internal details; use generic errors and SafeError for user-facing errors.
All QStash endpoints (API routes called via publishToQstash or publishToQstashQueue) must use verifySignatureAppRouter to verify request authenticity.
All cron endpoints in API routes must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak or plaintext secrets in API route files; secrets must not be directly assigned as string literals.
Review all new withError usage in API routes to ensure custom authentication is implemented where required.

Files:

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

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

**/*.{js,jsx,ts,tsx}: Don't use elements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
!pages/_document.{js,jsx,ts,tsx}

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

!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.

Files:

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

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

**/*.{test,spec}.{js,jsx,ts,tsx}: Don't use export or module.exports in test files.
Don't use focused tests.
Don't use disabled tests.
Make sure the assertion function, like expect, is placed inside an it() function call.
Don't nest describe() blocks too deeply in test files.
Don't use focused tests.
Don't use disabled tests.
Don't use export or module.exports in test files.

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
🧠 Learnings (1)
📚 Learning: 2025-06-23T12:27:30.570Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-06-23T12:27:30.570Z
Learning: When mocking Prisma in Vitest, import the Prisma mock from '@/utils/__mocks__/prisma', mock '@/utils/prisma', and clear all mocks in a beforeEach hook to ensure test isolation.

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.test.ts
🧬 Code graph analysis (1)
apps/web/app/api/google/webhook/process-label-removed-event.test.ts (2)
apps/web/utils/label.ts (1)
  • inboxZeroLabels (32-68)
apps/web/app/api/google/webhook/process-label-removed-event.ts (1)
  • processLabelRemoval (82-145)
⏰ 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). (2)
  • GitHub Check: Static Code Analysis Js
  • GitHub Check: Jit Security
🔇 Additional comments (8)
apps/web/app/api/google/webhook/process-label-removed-event.test.ts (8)

4-4: Import rename aligns with the new public API.

Good switch to processLabelRemoval; keeps tests in sync with the module rename.


108-108: Suite rename is accurate.

describe("processLabelRemoval") matches the new handler name.


112-134: Cold Email flow assertion looks solid.

Upsert shape and USER_REJECTED_COLD expectation match intended behavior.


146-154: Non-NEWSLETTER (To Reply) correctly skipped.

Expectation to not persist learning for To Reply is appropriate.


176-185: Multiple non-NEWSLETTER removals correctly skipped.

OK to assert no learning for Marketing.


192-195: Missing messageId guard covered.

Good negative path coverage.


203-206: Missing threadId guard covered.

Good negative path coverage.


18-43: The full-module mock is safe in this case; no refactor needed

I verified that process-label-removed-event.ts defines its own SYSTEM_LABELS constant locally and does not import it (or any other shared constants) from @/utils/gmail/label. The SUT only imports GmailLabel and getLabelById from that module, so fully mocking it in the test will not hide any additional exports that the implementation relies on. You can safely keep the existing mock.

Likely an incorrect or invalid review comment.

@edulelis edulelis changed the title WIP: Add prompt to learn from label removal patterns of newsletters Learn from label removal patterns for newsletters Aug 27, 2025
Copy link
Owner

Choose a reason for hiding this comment

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

Why would we be doing "Include" pattern here if the user has just removed a label from an email?

It's not given other potential rules so when would it ever want to do this?

I think what we'd want to do:

  1. if the rule matched because it matched a learned pattern, ask the AI if the learned pattern should be removed or converted to an EXCLUDE

  2. If the rule matched because of an AI instructions match, then the AI should decide if we want an EXCLUDE rule.

  3. If the rule matched because it was a static condition match, don't do anything. This is expected behaviour. If the user wants to change this they can adjust their rules.

1 and 2 can be specific focused prompts to the task at hand.

Overall it still seems there's a lot of irrelevant noise in the prompt. Words for the sake of it. Rather than focusing on the actual task.

The prompt should ideally be something along these lines:

We manage this user's inbox. We have a given set of rules and we choose the rule that best matches a given email.
Our AI system matched this email EMAIL to this rule RULE.
The user then removed the label LABEL. Should we adjust our learned patterns?
(And explain what learned patterns are)
And explain we don't always need to learn something, and that it should provide the reasoning.

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: 3

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/rule/learned-patterns.ts (2)

54-70: Upsert should update reasoning on existing FROM patterns.

Currently update: {} drops new reasoning when the item already exists.

   await prisma.groupItem.upsert({
     where: {
       groupId_type_value: {
         groupId,
         type: GroupItemType.FROM,
         value: from,
       },
     },
-    update: {},
+    update: {
+      reasoning,
+    },
     create: {
       groupId,
       type: GroupItemType.FROM,
       value: from,
       reasoning,
     },
   });

108-134: Avoid creating a second group on duplicate; fetch existing instead.

Creating a timestamped duplicate group fragments learned patterns across groups for the same rule.

-  } catch (error) {
-      if (isDuplicateError(error)) {
-        logger.error("Group already exists", { emailAccountId, ruleName });
-        const newGroup2 = await prisma.group.create({
-          data: {
-            emailAccountId,
-            name: `${ruleName} (${new Date().toISOString()})`,
-            rule: { connect: { id: rule.id } },
-          },
-        });
-        groupId = newGroup2.id;
-      } else {
+  } catch (error) {
+      if (isDuplicateError(error)) {
+        logger.warn("Group already exists, re-querying", { emailAccountId, ruleName });
+        const existing = await prisma.group.findFirst({
+          where: { emailAccountId, rule: { id: rule.id } },
+          select: { id: true },
+        });
+        if (!existing) {
+          logger.error("Duplicate detected but group missing after re-query", { emailAccountId, ruleName });
+          return { error: "Group exists but not found" };
+        }
+        groupId = existing.id;
+      } else {
         logger.error("Error creating learned patterns group", { error });
         return { error: "Error creating learned patterns group" };
       }
     }
♻️ Duplicate comments (1)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (1)

120-127: No raw prompt/output logging here — good.

Previous concern about logging full prompt/output in this module is resolved; continue relying on createGenerateObject’s centralized, truncated logging.

🧹 Nitpick comments (8)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (2)

83-89: Typo fix in guidance text.

Missing space after NO_ACTION.

- - Prefer NO_ACTIONif label removal seems like a mistake or if unsure about pattern inference.  
+ - Prefer NO_ACTION if label removal seems like a mistake or if unsure about pattern inference.  

63-89: Trim prompt/system before sending to model.

Minor hygiene; helps avoid accidental leading/trailing whitespace.

-  const system = `You are an email expert managing a user's inbox. Focus only on label removals.
+  const system = `You are an email expert managing a user's inbox. Focus only on label removals.
 ...
-`;
+`.trim();
...
-  const aiResponse = await generateObject({
+  const aiResponse = await generateObject({
     ...modelOptions,
-    system,
-    prompt,
+    system,
+    prompt: prompt.trim(),
     schema,
   });

Also applies to: 112-118, 120-127

apps/web/__tests__/ai-analyze-label-removal.test.ts (3)

250-256: Preserve required user fields when overriding AI config.

Overriding with { ...config } drops properties like aiApiKey and about; merge into the base user object to avoid undefined fields at runtime.

-      const testData = getTestData({
-        emailAccount: getEmailAccount({
-          user: { ...config },
-        }),
-      });
+      const testData = getTestData({
+        emailAccount: getEmailAccount({
+          user: { ...getUser(), ...config },
+        }),
+      });

58-70: Deduplicate helper: reuse shared getEmail() to reduce drift.

You already have apps/web/tests/helpers.ts with getEmail(). Import it rather than maintaining a local copy.

+import { getEmail as makeEmail } from "../__tests__/helpers";
...
-  function getEmail(overrides = {}) {
-    return {
-      id: "test-email-id",
-      from: "sender@example.com",
-      to: "user@test.com",
-      subject: "Test Email Subject",
-      content:
-        "This is a test email body that contains some content for analysis.",
-      date: new Date("2024-01-01T00:00:00Z"),
-      attachments: [],
-      ...overrides,
-    };
-  }
+  const getEmail = (overrides = {}) =>
+    makeEmail({
+      from: "sender@example.com",
+      to: "user@test.com",
+      subject: "Test Email Subject",
+      content:
+        "This is a test email body that contains some content for analysis.",
+      ...overrides,
+    });

186-209: Add assertions aligned with schema invariants.

If you adopt the schema refinement (pattern required for EXCLUDE/REMOVE), add targeted tests asserting pattern exists when action != NO_ACTION and exclude===true for EXCLUDE.

Want me to push a follow-up test case that forces EXCLUDE and verifies pattern presence and exclude flag?

Also applies to: 308-348

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

13-23: Return a result for saveLearnedPattern for consistency.

Other functions return { success | error }. Harmonize to enable callers to handle failures uniformly.

 export async function saveLearnedPattern({
 ...
-}) {
+}): Promise<{ success: true } | { error: string }> {
 ...
-  await prisma.groupItem.upsert({ ... });
-}
+  try {
+    await prisma.groupItem.upsert({ ... });
+    return { success: true };
+  } catch (e) {
+    logger.error("Error saving learned pattern", { e, emailAccountId, ruleName, from });
+    return { error: "Error saving learned pattern" };
+  }
+}

Also applies to: 70-70

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

196-204: Consistent spelling: analyzeLabelRemoval (US) to match aiAnalyzeLabelRemoval.

Minor naming consistency; avoids mixed spellings in imports/search.

-    await analyseLabelRemoval({
+    await analyzeLabelRemoval({
       labelNames: removedLabelNames,
       messageId,
       threadId,
       emailAccountId,
       parsedMessage,
       emailAccount,
     });
...
-async function analyseLabelRemoval({
+async function analyzeLabelRemoval({
   labelNames,
   messageId,
   threadId,
   emailAccountId,
   parsedMessage,
   emailAccount,
 }: {

Also applies to: 214-228


34-67: N+1 avoidance thought: prefetch learned patterns in one query.

Current flow fetches system rules, then learned patterns per match. If labelNames is large or grows, consider prefetching all groups/items keyed by ruleName to avoid extra round-trips. Optional at this scale.

Also applies to: 97-147

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 306bbbf and 3a61231.

📒 Files selected for processing (4)
  • apps/web/__tests__/ai-analyze-label-removal.test.ts (1 hunks)
  • apps/web/app/api/google/webhook/process-label-removed-event.ts (4 hunks)
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts (1 hunks)
  • apps/web/utils/rule/learned-patterns.ts (5 hunks)
🧰 Additional context used
📓 Path-based instructions (18)
apps/web/**/*.{ts,tsx}

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

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use @/ for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
!{.cursor/rules/*.mdc}

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

Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/form-handling.mdc)

**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
apps/web/__tests__/**/*

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

Place all LLM-related tests in apps/web/tests/

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
apps/web/__tests__/**/*.test.ts

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

apps/web/__tests__/**/*.test.ts: Always create helper functions for common test data in LLM-related tests
Include standard test cases: happy path, error handling, edge cases (empty input, null values), different user configurations, and various input formats in LLM-related tests
Set appropriate timeouts for LLM calls in tests (e.g., 15,000ms for long-running operations)
Use descriptive console.debug statements for generated content in LLM-related tests
Do not mock the LLM call in LLM-related tests; call the actual LLM
Test both AI and non-AI paths in LLM-related tests

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
**/*.{ts,tsx}

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

**/*.{ts,tsx}: Use createScopedLogger for logging in backend TypeScript files
Typically add the logger initialization at the top of the file when using createScopedLogger
Only use .with() on a logger instance within a specific function, not for a global logger

Import Prisma in the project using import prisma from "@/utils/prisma";

**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/*.test.{ts,js}

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

Include security tests in your test suites to verify authentication, authorization, and error handling.

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
**/*.test.{ts,js,tsx,jsx}

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

**/*.test.{ts,js,tsx,jsx}: Tests are colocated next to the tested file (e.g., dir/format.ts and dir/format.test.ts)
Use vi.mock("server-only", () => ({})); to mock the server-only module in tests
Mock @/utils/prisma in tests using vi.mock("@/utils/prisma") and use the provided prisma mock
Mock external dependencies in tests
Clean up mocks between tests
Do not mock the Logger

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
**/__tests__/**/*.{ts,js,tsx,jsx}

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

AI tests are placed in the __tests__ directory and are not run by default (they use a real LLM)

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
**/*.{js,jsx,ts,tsx}

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

**/*.{js,jsx,ts,tsx}: Don't use elements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
!pages/_document.{js,jsx,ts,tsx}

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

!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
  • apps/web/app/api/google/webhook/process-label-removed-event.ts
**/*.{test,spec}.{js,jsx,ts,tsx}

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

**/*.{test,spec}.{js,jsx,ts,tsx}: Don't use export or module.exports in test files.
Don't use focused tests.
Don't use disabled tests.
Make sure the assertion function, like expect, is placed inside an it() function call.
Don't nest describe() blocks too deeply in test files.
Don't use focused tests.
Don't use disabled tests.
Don't use export or module.exports in test files.

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
apps/web/__tests__/**/*.ts

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

Place LLM-specific tests under apps/web/tests

Files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
apps/web/utils/**

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

Create utility functions in utils/ folder for reusable logic

Files:

  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
apps/web/utils/**/*.ts

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

apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size

Files:

  • apps/web/utils/rule/learned-patterns.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
apps/web/utils/{ai,llms}/**/*.ts

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

apps/web/utils/{ai,llms}/**/*.ts: Place LLM-related implementation code under apps/web/utils/ai or apps/web/utils/llms
Keep system and user prompts separate; system defines role/task, user contains data/context
Always validate LLM responses with a specific Zod schema
Use descriptive scoped loggers per feature and log inputs/outputs with appropriate levels and context
Implement early returns for invalid inputs and use proper error types with logging
Add fallbacks for AI failures and include retry logic for transient errors using withRetry
Format prompts with XML-like tags; remove excessive whitespace; truncate overly long inputs; keep formatting consistent
Use TypeScript types for all parameters/returns and define interfaces for complex IO structures
Keep related AI functions co-located; extract shared logic into utilities; document complex AI logic with clear comments
Call LLMs via createGenerateObject; pass system, prompt, and a Zod schema; return the validated result.object
Derive model options using getModel(...) and pass them to createGenerateObject and the generate call

Files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
apps/web/app/**

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

NextJS app router structure with (app) directory

Files:

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

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

apps/web/app/api/**/*.{ts,js}: All API route handlers in 'apps/web/app/api/' must use authentication middleware: withAuth, withEmailAccount, or withError (with custom authentication logic).
All Prisma queries in API routes must include user/account filtering (e.g., emailAccountId or userId in WHERE clauses) to prevent unauthorized data access.
All parameters used in API routes must be validated before use; do not use parameters from 'params' or request bodies directly in queries without validation.
Request bodies in API routes should use Zod schemas for validation.
API routes should only return necessary fields using Prisma's 'select' and must not include sensitive data in error messages.
Error messages in API routes must not reveal internal details; use generic errors and SafeError for user-facing errors.
All QStash endpoints (API routes called via publishToQstash or publishToQstashQueue) must use verifySignatureAppRouter to verify request authenticity.
All cron endpoints in API routes must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak or plaintext secrets in API route files; secrets must not be directly assigned as string literals.
Review all new withError usage in API routes to ensure custom authentication is implemented where required.

Files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
🧠 Learnings (10)
📚 Learning: 2025-07-18T15:06:10.570Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.570Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Test both AI and non-AI paths in LLM-related tests

Applied to files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Keep related AI functions co-located; extract shared logic into utilities; document complex AI logic with clear comments

Applied to files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
📚 Learning: 2025-07-18T15:06:10.570Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.570Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Use descriptive console.debug statements for generated content in LLM-related tests

Applied to files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
📚 Learning: 2025-07-18T15:06:10.570Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-07-18T15:06:10.570Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Include standard test cases: happy path, error handling, edge cases (empty input, null values), different user configurations, and various input formats in LLM-related tests

Applied to files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Place LLM-related implementation code under apps/web/utils/ai or apps/web/utils/llms

Applied to files:

  • apps/web/__tests__/ai-analyze-label-removal.test.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Format prompts with XML-like tags; remove excessive whitespace; truncate overly long inputs; keep formatting consistent

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Keep system and user prompts separate; system defines role/task, user contains data/context

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Use descriptive scoped loggers per feature and log inputs/outputs with appropriate levels and context

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
📚 Learning: 2025-08-17T16:57:25.834Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-17T16:57:25.834Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Derive model options using getModel(...) and pass them to createGenerateObject and the generate call

Applied to files:

  • apps/web/utils/ai/label-analysis/analyze-label-removal.ts
📚 Learning: 2025-07-18T15:05:34.899Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-07-18T15:05:34.899Z
Learning: Applies to apps/web/utils/gmail/**/*.ts : Keep provider-specific implementation details isolated in the appropriate utils subfolder (e.g., 'apps/web/utils/gmail/')

Applied to files:

  • apps/web/app/api/google/webhook/process-label-removed-event.ts
🧬 Code graph analysis (4)
apps/web/__tests__/ai-analyze-label-removal.test.ts (2)
apps/web/__tests__/helpers.ts (1)
  • getEmail (20-37)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (1)
  • aiAnalyzeLabelRemoval (46-128)
apps/web/utils/rule/learned-patterns.ts (1)
apps/web/app/api/outlook/webhook/logger.ts (1)
  • logger (3-3)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (5)
apps/web/utils/types.ts (1)
  • EmailForLLM (117-131)
apps/web/utils/llms/types.ts (1)
  • EmailAccountWithAI (10-29)
apps/web/utils/stringify-email.ts (1)
  • stringifyEmail (4-27)
apps/web/utils/llms/model.ts (1)
  • getModel (27-41)
apps/web/utils/llms/index.ts (1)
  • createGenerateObject (118-166)
apps/web/app/api/google/webhook/process-label-removed-event.ts (8)
apps/web/utils/logger.ts (1)
  • createScopedLogger (17-65)
apps/web/utils/types.ts (1)
  • ParsedMessage (55-70)
apps/web/utils/llms/types.ts (1)
  • EmailAccountWithAI (10-29)
apps/web/utils/email.ts (1)
  • extractEmailAddress (19-52)
apps/web/utils/label.ts (1)
  • inboxZeroLabels (32-68)
apps/web/utils/ai/label-analysis/analyze-label-removal.ts (2)
  • aiAnalyzeLabelRemoval (46-128)
  • LabelRemovalAnalysis (44-44)
apps/web/utils/get-email-from-message.ts (1)
  • getEmailForLLM (6-25)
apps/web/utils/rule/learned-patterns.ts (2)
  • removeLearnedPattern (186-242)
  • saveLearnedPatterns (76-181)
⏰ 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). (2)
  • GitHub Check: Static Code Analysis Js
  • GitHub Check: Jit Security
🔇 Additional comments (1)
apps/web/app/api/google/webhook/process-label-removed-event.ts (1)

269-286: Good: conservative skip when no matching system rule.

Prevents unnecessary LLM calls and cost. Nice.

Comment on lines +321 to +361
if (analysis.action === "NO_ACTION") {
logger.info("No action needed for this label removal", {
emailAccountId,
matchedRule,
});
return;
}

if (analysis.action === "REMOVE" && analysis.pattern) {
logger.info("Removing learned pattern from rule", {
emailAccountId,
ruleName: matchedRule.ruleName,
patternType: analysis.pattern.type,
patternValue: analysis.pattern.value,
});

const result = await removeLearnedPattern({
emailAccountId,
ruleName: matchedRule.ruleName,
pattern: {
type: analysis.pattern.type,
value: analysis.pattern.value,
},
});

if (result.error) {
logger.error("Failed to remove learned pattern", {
emailAccountId,
ruleName: matchedRule.ruleName,
error: result.error,
});
} else {
logger.info("Successfully removed learned pattern", {
emailAccountId,
ruleName: matchedRule.ruleName,
patternType: analysis.pattern.type,
patternValue: analysis.pattern.value,
});
}
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Guard against actions missing pattern per runtime (even with schema).

If the AI or schema ever lets a missing pattern slip through, log and bail to avoid incorrect DB ops.

   if (analysis.action === "NO_ACTION") {
     logger.info("No action needed for this label removal", {
       emailAccountId,
       matchedRule,
     });
     return;
   }
 
-  if (analysis.action === "REMOVE" && analysis.pattern) {
+  if (analysis.action === "REMOVE" && analysis.pattern) {
     logger.info("Removing learned pattern from rule", {
       emailAccountId,
       ruleName: matchedRule.ruleName,
       patternType: analysis.pattern.type,
-      patternValue: analysis.pattern.value,
+      patternValuePreview: analysis.pattern.value.slice(0, 64),
     });
@@
-    if (result.error) {
+    if (result.error) {
       logger.error("Failed to remove learned pattern", {
         emailAccountId,
         ruleName: matchedRule.ruleName,
         error: result.error,
       });
     } else {
       logger.info("Successfully removed learned pattern", {
         emailAccountId,
         ruleName: matchedRule.ruleName,
         patternType: analysis.pattern.type,
-        patternValue: analysis.pattern.value,
+        patternValuePreview: analysis.pattern.value.slice(0, 64),
       });
     }
     return;
   }
 
-  if (analysis.action === "EXCLUDE" && analysis.pattern) {
+  if (analysis.action === "EXCLUDE" && analysis.pattern) {
     const patternToSave = {
       type: analysis.pattern.type,
       value: analysis.pattern.value,
       exclude: analysis.pattern.exclude,
       reasoning: analysis.pattern.reasoning,
     };
 
     logger.info("Adding learned pattern to rule", {
       emailAccountId,
       ruleName: matchedRule.ruleName,
       patternType: patternToSave.type,
-      patternValue: patternToSave.value,
+      patternValuePreview: patternToSave.value.slice(0, 64),
       exclude: patternToSave.exclude,
     });
 
-    await saveLearnedPatterns({
+    const res = await saveLearnedPatterns({
       emailAccountId,
       ruleName: matchedRule.ruleName,
       patterns: [patternToSave],
     });
+    if (res?.error) {
+      logger.error("Failed to save learned pattern", {
+        emailAccountId,
+        ruleName: matchedRule.ruleName,
+        error: res.error,
+      });
+    }
+    return;
   }
+
+  logger.warn("Action requires a pattern but none provided", {
+    emailAccountId,
+    action: analysis.action,
+  });

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

🤖 Prompt for AI Agents
In apps/web/app/api/google/webhook/process-label-removed-event.ts around lines
321 to 361, add a runtime guard for the REMOVE action to ensure analysis.pattern
exists before doing any DB operations: if analysis.action === "REMOVE" but
analysis.pattern is falsy, log an error/warning with emailAccountId and
matchedRule.ruleName indicating the missing pattern and return early to avoid
incorrect removeLearnedPattern calls; keep the existing success/error logging
when pattern is present.

Comment on lines +9 to +42
const schema = z.object({
action: z
.enum(["EXCLUDE", "REMOVE", "NO_ACTION"])
.describe(
"The action to take, EXCLUDE to add an exclude learned pattern, REMOVE to remove an existing pattern, NO_ACTION if nothing should be done",
),
pattern: z
.object({
reasoning: z
.string()
.describe(
"A short human-readable explanation of why this learned pattern is important to learn",
),
type: z
.nativeEnum(GroupItemType)
.describe(
"The pattern type which can be FROM (sender/domain), SUBJECT (sender/domain/subject keyword), or BODY (content)",
),
value: z
.string()
.describe(
"The specific value for the learned pattern (e.g., email address, domain, subject keyword)",
),
exclude: z
.boolean()
.describe(
"Whether this learned pattern should be excluded (true) or just not included (false)",
),
})
.nullish()
.describe(
"The pattern to learn from or remove based on this label removal",
),
});
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

Enforce schema invariants (pattern required; EXCLUDE must be true; REMOVE must not set exclude).

Without cross-field validation the AI can return EXCLUDE with exclude=false or actions without a pattern, leading to wrong DB writes.

Apply:

 const schema = z.object({
   action: z
     .enum(["EXCLUDE", "REMOVE", "NO_ACTION"])
     .describe(
       "The action to take, EXCLUDE to add an exclude learned pattern, REMOVE to remove an existing pattern, NO_ACTION if nothing should be done",
     ),
   pattern: z
     .object({
-      reasoning: z
-        .string()
+      reasoning: z
+        .string()
+        .min(5)
         .describe(
           "A short human-readable explanation of why this learned pattern is important to learn",
         ),
       type: z
         .nativeEnum(GroupItemType)
         .describe(
-          "The pattern type which can be FROM (sender/domain), SUBJECT (sender/domain/subject keyword), or BODY (content)",
+          "The pattern type which can be FROM (sender email or domain), SUBJECT (subject keyword), or BODY (content)",
         ),
-      value: z
-        .string()
+      value: z
+        .string()
+        .min(1)
+        .trim()
         .describe(
           "The specific value for the learned pattern (e.g., email address, domain, subject keyword)",
         ),
       exclude: z
         .boolean()
         .describe(
           "Whether this learned pattern should be excluded (true) or just not included (false)",
         ),
     })
     .nullish()
     .describe(
       "The pattern to learn from or remove based on this label removal",
     ),
 });
+
+const schemaWithRules = schema.superRefine((val, ctx) => {
+  if (val.action !== "NO_ACTION" && !val.pattern) {
+    ctx.addIssue({ code: z.ZodIssueCode.custom, message: "pattern is required when action is EXCLUDE or REMOVE" });
+  }
+  if (val.action === "EXCLUDE" && !(val.pattern && val.pattern.exclude === true)) {
+    ctx.addIssue({ code: z.ZodIssueCode.custom, message: "exclude must be true when action is EXCLUDE" });
+  }
+  if (val.action === "REMOVE" && val.pattern?.exclude === true) {
+    ctx.addIssue({ code: z.ZodIssueCode.custom, message: "exclude should be false/omitted when action is REMOVE" });
+  }
+});

Then replace uses of schema below with schemaWithRules.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const schema = z.object({
action: z
.enum(["EXCLUDE", "REMOVE", "NO_ACTION"])
.describe(
"The action to take, EXCLUDE to add an exclude learned pattern, REMOVE to remove an existing pattern, NO_ACTION if nothing should be done",
),
pattern: z
.object({
reasoning: z
.string()
.describe(
"A short human-readable explanation of why this learned pattern is important to learn",
),
type: z
.nativeEnum(GroupItemType)
.describe(
"The pattern type which can be FROM (sender/domain), SUBJECT (sender/domain/subject keyword), or BODY (content)",
),
value: z
.string()
.describe(
"The specific value for the learned pattern (e.g., email address, domain, subject keyword)",
),
exclude: z
.boolean()
.describe(
"Whether this learned pattern should be excluded (true) or just not included (false)",
),
})
.nullish()
.describe(
"The pattern to learn from or remove based on this label removal",
),
});
// apps/web/utils/ai/label-analysis/analyze-label-removal.ts
const schema = z.object({
action: z
.enum(["EXCLUDE", "REMOVE", "NO_ACTION"])
.describe(
"The action to take, EXCLUDE to add an exclude learned pattern, REMOVE to remove an existing pattern, NO_ACTION if nothing should be done",
),
pattern: z
.object({
reasoning: z
.string()
.min(5)
.describe(
"A short human-readable explanation of why this learned pattern is important to learn",
),
type: z
.nativeEnum(GroupItemType)
.describe(
"The pattern type which can be FROM (sender email or domain), SUBJECT (subject keyword), or BODY (content)",
),
value: z
.string()
.min(1)
.trim()
.describe(
"The specific value for the learned pattern (e.g., email address, domain, subject keyword)",
),
exclude: z
.boolean()
.describe(
"Whether this learned pattern should be excluded (true) or just not included (false)",
),
})
.nullish()
.describe(
"The pattern to learn from or remove based on this label removal",
),
});
const schemaWithRules = schema.superRefine((val, ctx) => {
if (val.action !== "NO_ACTION" && !val.pattern) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: "pattern is required when action is EXCLUDE or REMOVE",
});
}
if (val.action === "EXCLUDE" && !(val.pattern && val.pattern.exclude === true)) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: "exclude must be true when action is EXCLUDE",
});
}
if (val.action === "REMOVE" && val.pattern?.exclude === true) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: "exclude should be false/omitted when action is REMOVE",
});
}
});
// ...later in this file, replace all uses of `schema` with `schemaWithRules` when parsing/validating.
🤖 Prompt for AI Agents
In apps/web/utils/ai/label-analysis/analyze-label-removal.ts around lines 9 to
42, the Zod schema allows invalid cross-field combinations (e.g., actions
without a pattern, EXCLUDE with pattern.exclude=false, or REMOVE with
pattern.exclude=true); add a new schemaWithRules that wraps the existing schema
and enforces these invariants via a superRefine: ensure when action is "EXCLUDE"
or "REMOVE" that pattern is present, enforce that when action === "EXCLUDE"
pattern.exclude === true, and when action === "REMOVE" pattern.exclude !== true
(or false/undefined) as required, and then replace all downstream uses of schema
with schemaWithRules.

Comment on lines +91 to +110
const prompt = `The rule:

<rule>
<system_type>${matchedRule.systemType}</system_type>
<label>${matchedRule.labelName}</label>
<instructions>${matchedRule.instructions || "No instructions provided"}</instructions>
<learned_patterns>
${matchedRule.learnedPatterns.map((pattern) => `<pattern_type>${pattern.type}</pattern_type><pattern_value>${pattern.value}</pattern_value><pattern_exclude>${pattern.exclude}</pattern_exclude><pattern_reasoning>${pattern.reasoning || "User provided"}</pattern_reasoning>`).join("\n")}
</learned_patterns>
</rule>

The email:
<email>
${stringifyEmail(email, 1000)}
</email>

The label:
<label>
${matchedRule.labelName}
</label>`;
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Escape XML and structure learned patterns to avoid malformed prompt.

Raw values (subject, instructions, pattern.value/reasoning) can break tags. Wrap each pattern and escape content.

+  const escapeXml = (s: string) =>
+    String(s)
+      .replaceAll("&", "&amp;")
+      .replaceAll("<", "&lt;")
+      .replaceAll(">", "&gt;")
+      .replaceAll('"', "&quot;")
+      .replaceAll("'", "&apos;");
+
-  const prompt = `The rule:
+  const patternsXml = matchedRule.learnedPatterns
+    .map(
+      (p) => `<pattern>
+  <type>${escapeXml(p.type)}</type>
+  <value>${escapeXml(p.value)}</value>
+  <exclude>${p.exclude}</exclude>
+  ${p.reasoning ? `<reasoning>${escapeXml(p.reasoning)}</reasoning>` : ""}
+</pattern>`,
+    )
+    .join("\n");
+
+  const prompt = `The rule:
 
 <rule>
-  <system_type>${matchedRule.systemType}</system_type>
-  <label>${matchedRule.labelName}</label>
-  <instructions>${matchedRule.instructions || "No instructions provided"}</instructions>
+  <system_type>${escapeXml(matchedRule.systemType)}</system_type>
+  <label>${escapeXml(matchedRule.labelName)}</label>
+  <instructions>${escapeXml(matchedRule.instructions || "No instructions provided")}</instructions>
   <learned_patterns>
-    ${matchedRule.learnedPatterns.map((pattern) => `<pattern_type>${pattern.type}</pattern_type><pattern_value>${pattern.value}</pattern_value><pattern_exclude>${pattern.exclude}</pattern_exclude><pattern_reasoning>${pattern.reasoning || "User provided"}</pattern_reasoning>`).join("\n")}
+${patternsXml}
   </learned_patterns>
 </rule>
 
 The email:
 <email>
 ${stringifyEmail(email, 1000)}
 </email>
 
 The label:
 <label>
-${matchedRule.labelName}
+${escapeXml(matchedRule.labelName)}
 </label>`;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const prompt = `The rule:
<rule>
<system_type>${matchedRule.systemType}</system_type>
<label>${matchedRule.labelName}</label>
<instructions>${matchedRule.instructions || "No instructions provided"}</instructions>
<learned_patterns>
${matchedRule.learnedPatterns.map((pattern) => `<pattern_type>${pattern.type}</pattern_type><pattern_value>${pattern.value}</pattern_value><pattern_exclude>${pattern.exclude}</pattern_exclude><pattern_reasoning>${pattern.reasoning || "User provided"}</pattern_reasoning>`).join("\n")}
</learned_patterns>
</rule>
The email:
<email>
${stringifyEmail(email, 1000)}
</email>
The label:
<label>
${matchedRule.labelName}
</label>`;
// Escape XML special characters in arbitrary strings
const escapeXml = (s: string) =>
String(s)
.replaceAll("&", "&amp;")
.replaceAll("<", "&lt;")
.replaceAll(">", "&gt;")
.replaceAll('"', "&quot;")
.replaceAll("'", "&apos;");
// Build a well-formed XML block for each learned pattern
const patternsXml = matchedRule.learnedPatterns
.map(
(p) => `<pattern>
<type>${escapeXml(p.type)}</type>
<value>${escapeXml(p.value)}</value>
<exclude>${p.exclude}</exclude>
${p.reasoning ? `<reasoning>${escapeXml(p.reasoning)}</reasoning>` : ""}
</pattern>`,
)
.join("\n");
const prompt = `The rule:
<rule>
<system_type>${escapeXml(matchedRule.systemType)}</system_type>
<label>${escapeXml(matchedRule.labelName)}</label>
<instructions>${escapeXml(matchedRule.instructions || "No instructions provided")}</instructions>
<learned_patterns>
${patternsXml}
</learned_patterns>
</rule>
The email:
<email>
${stringifyEmail(email, 1000)}
</email>
The label:
<label>
${escapeXml(matchedRule.labelName)}
</label>`;
🤖 Prompt for AI Agents
In apps/web/utils/ai/label-analysis/analyze-label-removal.ts around lines 91 to
110, the prompt builds XML-like tags using raw user content which can break
tags; update it to escape XML entities (&, <, >, ", ') for any interpolated
user-controlled fields including subject/email output, matchedRule.instructions,
matchedRule.labelName, and each learned pattern's value/reasoning, and
restructure learned_patterns so each pattern is wrapped in its own
<pattern>...</pattern> block containing child tags (<pattern_type>,
<pattern_value>, <pattern_exclude>, <pattern_reasoning>) instead of inlining
multiple sibling tags on one line; ensure you provide safe defaults (e.g., "No
instructions provided" / "User provided") and apply the escaping function to
values before interpolation.

@edulelis edulelis closed this Aug 29, 2025
@edulelis edulelis deleted the learned-patterns-from-labels-v2 branch August 29, 2025 14:37
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.

2 participants

Comments