fix: prevent double draft emails when multi-rule selection enabled#1163
fix: prevent double draft emails when multi-rule selection enabled#1163
Conversation
…ave DRAFT_EMAIL Moves limitDraftEmailActions to execute AFTER conversation meta-rules are resolved, so it can see all DRAFT_EMAIL actions and properly limit to one draft.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe PR refactors AI-powered meeting briefing generation from a sequential perplexity-based research approach to an agentic workflow with configurable search tools, updates rule execution logic to better handle conversation meta-rules and draft email limit enforcement, and consolidates research caching under a generic source-aware abstraction. Changes
Sequence Diagram(s)sequenceDiagram
participant Host as Host/App
participant Agent as AI Agent
participant Tools as Search Tools
participant Cache as Cache (Redis)
participant Model as LLM Model
Host->>Agent: Start agentic briefing<br/>(guests, context, tools)
Agent->>Cache: Check cached results<br/>(perplexity, websearch)
Cache-->>Agent: Return cached results<br/>(if available)
rect rgb(220, 240, 255)
note over Agent,Model: Agent Loop (max steps)
Agent->>Model: Process prompt + tools<br/>(with tool definitions)
Model->>Agent: Next action or tool call
alt Tool Call Needed
Agent->>Tools: Execute search<br/>(perplexity or web search)
Tools-->>Agent: Return search results
Agent->>Cache: Store results<br/>(per source)
Cache-->>Agent: Ack
else Finalization
Agent->>Agent: Prepare final briefing
break Agent Done
Agent-->>Host: Return BriefingContent
end
end
end
rect rgb(245, 245, 220)
note over Host,Cache: Fallback Path
Agent-->>Host: Return empty/fallback<br/>(if no tools or agent timeout)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Prevent duplicate draft email actions when multi-rule selection is enabled by limiting
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| if ( | ||
| env.DEFAULT_LLM_PROVIDER === Provider.OPEN_AI || | ||
| env.DEFAULT_LLM_PROVIDER === Provider.GOOGLE || | ||
| env.DEFAULT_LLM_PROVIDER === Provider.OPENROUTER | ||
| ) { | ||
| availableTools.push("webSearch"); | ||
| } |
There was a problem hiding this comment.
webSearch is advertised based only on env.DEFAULT_LLM_PROVIDER, so the prompt may claim the tool exists even when the required API key isn’t set, causing runtime errors. Consider also gating webSearch on the provider’s API key to match actual tool availability.
| if ( | |
| env.DEFAULT_LLM_PROVIDER === Provider.OPEN_AI || | |
| env.DEFAULT_LLM_PROVIDER === Provider.GOOGLE || | |
| env.DEFAULT_LLM_PROVIDER === Provider.OPENROUTER | |
| ) { | |
| availableTools.push("webSearch"); | |
| } | |
| if ( | |
| (env.DEFAULT_LLM_PROVIDER === Provider.OPEN_AI && !!env.OPENAI_API_KEY) || | |
| (env.DEFAULT_LLM_PROVIDER === Provider.GOOGLE && !!env.GOOGLE_API_KEY) || | |
| (env.DEFAULT_LLM_PROVIDER === Provider.OPENROUTER && !!env.OPENROUTER_API_KEY) | |
| ) { | |
| availableTools.push("webSearch"); | |
| } |
🚀 Want me to fix this? Reply ex: "fix it for me".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/utils/ai/meeting-briefs/generate-briefing.ts (4)
75-143: Result capture via closure side-effect.The
resultvariable is set via side-effect inside the tool'sexecutecallback. While this works, it relies on the tool execution happening synchronously within thegenerateTextcall. Consider adding a comment explaining this pattern for future maintainers.The early return for empty guests and the fallback when the agent doesn't finalize are good defensive patterns.
185-224: Provider string inconsistency.Line 196 uses the string literal
"perplexity"while other parts of the codebase use theProviderenum from@/utils/llms/config. While Perplexity isn't in the standard Provider enum (it's a search-specific service), consider adding a comment or defining a constant for consistency.🔎 Suggested improvement
+// Note: "perplexity" is not in the standard Provider enum as it's a search-specific service const perplexityGenerateText = createGenerateText({ emailAccount, label: "Perplexity Search", modelOptions: { modelName: "sonar-pro", model: perplexity("sonar-pro"), provider: "perplexity", backupModel: null, }, });
248-272: Consider extracting provider check to a shared helper.The logic for determining web search availability is duplicated:
- Here in
getWebSearchConfig(lines 249-271)- In
buildPrompt(lines 366-372)If new providers are added, both locations need updating.
🔎 Suggested refactor
+function isWebSearchProvider(provider: string): boolean { + return [Provider.OPEN_AI, Provider.GOOGLE, Provider.OPENROUTER].includes(provider as any); +} + function getWebSearchConfig(): WebSearchConfig | null { switch (env.DEFAULT_LLM_PROVIDER) { case Provider.OPEN_AI: // ...Then in
buildPrompt:- if ( - env.DEFAULT_LLM_PROVIDER === Provider.OPEN_AI || - env.DEFAULT_LLM_PROVIDER === Provider.GOOGLE || - env.DEFAULT_LLM_PROVIDER === Provider.OPENROUTER - ) { + if (isWebSearchProvider(env.DEFAULT_LLM_PROVIDER)) { availableTools.push("webSearch"); }
363-377: Tool availability check is duplicated.This logic mirrors
buildSearchToolsandgetWebSearchConfig. See earlier comment about extracting to a shared helper.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/meeting-briefs/research-guest.tsapps/web/utils/email.tsapps/web/utils/meeting-briefs/gather-context.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tsapps/web/utils/user/delete.tspackages/resend/emails/meeting-briefing.tsx
💤 Files with no reviewable changes (1)
- apps/web/utils/ai/meeting-briefs/research-guest.ts
🧰 Additional context used
📓 Path-based instructions (24)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)
**/*.{ts,tsx}: For API GET requests to server, use theswrpackage
Useresult?.serverErrorwithtoastErrorfrom@/components/Toastfor error handling in async operations
**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls
**/*.{ts,tsx}: For early access feature flags, create hooks using the naming conventionuse[FeatureName]Enabledthat return a boolean fromuseFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming conventionuse[FeatureName]Variantthat define variant types, useuseFeatureFlagVariantKey()with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g.,inbox-cleaner,pricing-options-2)
Always define types for A/B test variant flags (e.g.,type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the!postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Useas constinstead of literal types and type annotations
Use eitherT[]orArray<T>consistently
Initialize each enum member value explicitly
Useexport typefor types
Use `impo...
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tspackages/resend/emails/meeting-briefing.tsxapps/web/utils/meeting-briefs/gather-context.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)
Always import Prisma enums from
@/generated/prisma/enumsinstead of@/generated/prisma/clientto avoid Next.js bundling errors in client componentsImport Prisma using the project's centralized utility:
import prisma from '@/utils/prisma'
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tspackages/resend/emails/meeting-briefing.tsxapps/web/utils/meeting-briefs/gather-context.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g.,
import groupBy from 'lodash/groupBy')
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Do not export types/interfaces that are only used within the same file. Export later if needed
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tsapps/web/utils/meeting-briefs/gather-context.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/security.mdc)
**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma'sselectoption. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. AllfindUnique/findFirstcalls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
AllfindManyqueries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g.,emailAccount: { id: emailAccountId }) to validate ownership
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tsapps/web/utils/meeting-briefs/gather-context.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Usenext/imagepackage for images
For API GET requests to server, use theswrpackage with hooks likeuseSWRto fetch data
For text inputs, use theInputcomponent withregisterPropsfor form integration and error handling
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tspackages/resend/emails/meeting-briefing.tsxapps/web/utils/meeting-briefs/gather-context.ts
**/*.{tsx,ts,css}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Implement responsive design with Tailwind CSS using a mobile-first approach
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tspackages/resend/emails/meeting-briefing.tsxapps/web/utils/meeting-briefs/gather-context.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useaccessKeyattribute on any HTML element
Don't setaria-hidden="true"on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like<marquee>or<blink>
Only use thescopeprop on<th>elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assigntabIndexto non-interactive HTML elements
Don't use positive integers fortabIndexproperty
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include atitleelement for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
AssigntabIndexto non-interactive HTML elements witharia-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include atypeattribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden)
Always include alangattribute on the html element
Always include atitleattribute for iframe elements
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress
AccompanyonMouseOver/onMouseOutwithonFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tspackages/resend/emails/meeting-briefing.tsxapps/web/utils/meeting-briefs/gather-context.ts
!(pages/_document).{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't use the next/head module in pages/_document.js on Next.js projects
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tspackages/resend/emails/meeting-briefing.tsxapps/web/utils/meeting-briefs/gather-context.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g.,import groupBy from 'lodash/groupBy')
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tspackages/resend/emails/meeting-briefing.tsxapps/web/utils/meeting-briefs/gather-context.ts
**/{utils,helpers,lib}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
Logger should be passed as a parameter to helper functions instead of creating their own logger instances
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tsapps/web/utils/meeting-briefs/gather-context.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx,js,jsx}: Use@/path aliases for imports from project root
Prefer self-documenting code over comments; use descriptive variable and function names instead of explaining intent with comments
Add helper functions to the bottom of files, not the top
All imports go at the top of files, no mid-file dynamic imports
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tsapps/web/utils/meeting-briefs/gather-context.ts
apps/web/**/*.{ts,tsx,js,jsx,json,css}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Format code with Prettier
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tsapps/web/utils/meeting-briefs/gather-context.ts
apps/web/**/*.{example,ts,json}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Add environment variables to
.env.example,env.ts, andturbo.json
Files:
apps/web/utils/email.tsapps/web/utils/user/delete.tsapps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/redis/research-cache.tsapps/web/utils/meeting-briefs/gather-context.ts
apps/web/__tests__/**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-test.mdc)
apps/web/__tests__/**/*.test.ts: Place all LLM-related tests inapps/web/__tests__/directory
Use vitest imports (describe,expect,test,vi,beforeEach) in LLM test files
Mock 'server-only' module with empty object in LLM test files:vi.mock("server-only", () => ({}))
Set timeout constantconst TIMEOUT = 15_000;for LLM tests
Usedescribe.runIf(isAiTest)with environment variableRUN_AI_TESTS === "true"to conditionally run LLM tests
Useconsole.debug()for outputting generated LLM content in tests, e.g.,console.debug("Generated content:\n", result.content);
Prefer using existing helpers from@/__tests__/helpers.ts(getEmailAccount,getEmail,getRule,getMockMessage,getMockExecutedRule) instead of creating custom test data helpers
Files:
apps/web/__tests__/ai-meeting-briefing.test.ts
apps/web/{utils/ai,utils/llms,__tests__}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm.mdc)
LLM-related code must be organized in specific directories:
apps/web/utils/ai/for main implementations,apps/web/utils/llms/for core utilities and configurations, andapps/web/__tests__/for LLM-specific tests
Files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Don't nest describe() blocks too deeply in test files
Don't use callbacks in asynchronous tests and hooks
Don't have duplicate hooks in describe blocks
Don't use export or module.exports in test files
Don't use focused tests
Make sure the assertion function, like expect, is placed inside an it() function call
Don't use disabled tests
Files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/choose-rule/run-rules.test.ts
**/{scripts,tests,__tests__}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)
Use createScopedLogger only for code that doesn't run within a middleware chain, such as standalone scripts or tests
Files:
apps/web/__tests__/ai-meeting-briefing.test.ts
apps/web/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Co-locate test files next to source files (e.g.,
utils/example.test.ts). Only E2E and AI tests go in__tests__/
Files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/choose-rule/run-rules.test.ts
**/*.test.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/notes.mdc)
Co-locate test files next to source files (e.g.,
utils/example.test.ts). Only E2E and AI tests go in__tests__/
Files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/choose-rule/run-rules.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
**/*.test.{ts,tsx}: Usevitestas the testing framework
Colocate test files next to the tested file with.test.tsor.test.tsxnaming convention (e.g.,dir/format.tsanddir/format.test.ts)
Mockserver-onlyusingvi.mock("server-only", () => ({}))
Mock Prisma usingvi.mock("@/utils/prisma")and the provided mock from@/utils/__mocks__/prisma
Use test helper functionsgetEmail,getEmailAccount, andgetRulefrom@/__tests__/helpersfor creating mock data
Clear all mocks between tests usingbeforeEach(() => { vi.clearAllMocks(); })
Use descriptive test names that clearly indicate what is being tested
Do not mock the Logger in tests
Files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/choose-rule/run-rules.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
Place AI tests in the
__tests__directory and do not run them by default as they use a real LLM
Files:
apps/web/__tests__/ai-meeting-briefing.test.ts
apps/web/utils/ai/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/llm.mdc)
apps/web/utils/ai/**/*.ts: LLM feature functions must import fromzodfor schema validation, usecreateScopedLoggerfrom@/utils/logger,chatCompletionObjectandcreateGenerateObjectfrom@/utils/llms, and importEmailAccountWithAItype from@/utils/llms/types
LLM feature functions must follow a standard structure: accept options withinputDataandemailAccountparameters, implement input validation with early returns, define separate system and user prompts, create a Zod schema for response validation, and usecreateGenerateObjectto execute the LLM call
System prompts must define the LLM's role and task specifications
User prompts must contain the actual data and context, and should be kept separate from system prompts
Always define a Zod schema for LLM response validation and make schemas as specific as possible to guide the LLM output
Use descriptive scoped loggers for each LLM feature, log inputs and outputs with appropriate log levels, and include relevant context in log messages
Implement early returns for invalid LLM inputs, use proper error types and logging, implement fallbacks for AI failures, and add retry logic for transient failures usingwithRetry
Use XML-like tags to structure data in prompts, remove excessive whitespace and truncate long inputs, and format data consistently across similar LLM functions
Use TypeScript types for all LLM function parameters and return values, and define clear interfaces for complex input/output structures
Keep related AI functions in the same file or directory, extract common patterns into utility functions, and document complex AI logic with clear comments
Files:
apps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
**/*.tsx: Use theLoadingContentcomponent to handle loading states instead of manual loading state management
For text areas, use theInputcomponent withtype='text',autosizeTextareaprop set to true, andregisterPropsfor form integration
Files:
packages/resend/emails/meeting-briefing.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't use unnecessary fragments
Don't pass children as props
Don't use the return value of React.render
Make sure all dependencies are correctly specified in React hooks
Make sure all React hooks are called from the top level of component functions
Don't forget key props in iterators and collection literals
Don't define React components inside other components
Don't use event handlers on non-interactive elements
Don't assign to React component props
Don't use bothchildrenanddangerouslySetInnerHTMLprops on the same element
Don't use dangerous JSX props
Don't use Array index in keys
Don't insert comments as text nodes
Don't assign JSX properties multiple times
Don't add extra closing tags for components without children
Use<>...</>instead of<Fragment>...</Fragment>
Watch out for possible "wrong" semicolons inside JSX elements
Make sure void (self-closing) elements don't have children
Don't usetarget="_blank"withoutrel="noopener"
Don't use<img>elements in Next.js projects
Don't use<head>elements in Next.js projects
Files:
packages/resend/emails/meeting-briefing.tsx
🧠 Learnings (25)
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Use `describe.runIf(isAiTest)` with environment variable `RUN_AI_TESTS === "true"` to conditionally run LLM tests
Applied to files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2026-01-01T10:42:29.767Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-01-01T10:42:29.767Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Place AI tests in the `__tests__` directory and do not run them by default as they use a real LLM
Applied to files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Keep related AI functions in the same file or directory, extract common patterns into utility functions, and document complex AI logic with clear comments
Applied to files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/gather-context.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Use XML-like tags to structure data in prompts, remove excessive whitespace and truncate long inputs, and format data consistently across similar LLM functions
Applied to files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/choose-rule/run-rules.ts
📚 Learning: 2025-12-21T12:21:37.794Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-12-21T12:21:37.794Z
Learning: Applies to apps/web/**/*.test.{ts,tsx} : Co-locate test files next to source files (e.g., `utils/example.test.ts`). Only E2E and AI tests go in `__tests__/`
Applied to files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : LLM feature functions must import from `zod` for schema validation, use `createScopedLogger` from `@/utils/logger`, `chatCompletionObject` and `createGenerateObject` from `@/utils/llms`, and import `EmailAccountWithAI` type from `@/utils/llms/types`
Applied to files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.tsapps/web/utils/meeting-briefs/gather-context.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Implement early returns for invalid LLM inputs, use proper error types and logging, implement fallbacks for AI failures, and add retry logic for transient failures using `withRetry`
Applied to files:
apps/web/__tests__/ai-meeting-briefing.test.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/gather-context.ts
📚 Learning: 2026-01-01T10:42:29.767Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-01-01T10:42:29.767Z
Learning: Applies to **/*.test.{ts,tsx} : Use descriptive test names that clearly indicate what is being tested
Applied to files:
apps/web/__tests__/ai-meeting-briefing.test.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : System prompts must define the LLM's role and task specifications
Applied to files:
apps/web/__tests__/ai-meeting-briefing.test.ts
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Use vitest imports (`describe`, `expect`, `test`, `vi`, `beforeEach`) in LLM test files
Applied to files:
apps/web/__tests__/ai-meeting-briefing.test.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : LLM feature functions must follow a standard structure: accept options with `inputData` and `emailAccount` parameters, implement input validation with early returns, define separate system and user prompts, create a Zod schema for response validation, and use `createGenerateObject` to execute the LLM call
Applied to files:
apps/web/utils/ai/meeting-briefs/generate-briefing.tsapps/web/utils/ai/choose-rule/run-rules.test.tsapps/web/utils/ai/choose-rule/run-rules.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : User prompts must contain the actual data and context, and should be kept separate from system prompts
Applied to files:
apps/web/utils/ai/meeting-briefs/generate-briefing.ts
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Prefer using existing helpers from `@/__tests__/helpers.ts` (`getEmailAccount`, `getEmail`, `getRule`, `getMockMessage`, `getMockExecutedRule`) instead of creating custom test data helpers
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2026-01-01T10:42:29.767Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-01-01T10:42:29.767Z
Learning: Applies to **/*.test.{ts,tsx} : Use test helper functions `getEmail`, `getEmailAccount`, and `getRule` from `@/__tests__/helpers` for creating mock data
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/{utils/ai,utils/llms,__tests__}/**/*.ts : LLM-related code must be organized in specific directories: `apps/web/utils/ai/` for main implementations, `apps/web/utils/llms/` for core utilities and configurations, and `apps/web/__tests__/` for LLM-specific tests
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2026-01-01T10:42:29.767Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-01-01T10:42:29.767Z
Learning: Applies to **/*.test.{ts,tsx} : Mock Prisma using `vi.mock("@/utils/prisma")` and the provided mock from `@/utils/__mocks__/prisma`
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2025-11-25T14:39:23.326Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to app/api/**/*.ts : Use `withEmailAccount` middleware for operations scoped to a specific email account (reading/writing emails, rules, schedules, etc.) - provides `emailAccountId`, `userId`, and `email` in `request.auth`
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2025-11-25T14:39:27.909Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:27.909Z
Learning: Applies to **/app/api/**/*.ts : Use `withEmailAccount` middleware for operations scoped to a specific email account, including reading/writing emails, rules, schedules, or any operation using `emailAccountId`
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2025-11-25T14:42:11.919Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-25T14:42:11.919Z
Learning: Applies to utils/**/*.{js,ts,jsx,tsx} : The `utils` folder contains core app logic such as Next.js Server Actions and Gmail API requests
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2026-01-01T10:42:29.767Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-01-01T10:42:29.767Z
Learning: Applies to **/*.test.{ts,tsx} : Mock `server-only` using `vi.mock("server-only", () => ({}))`
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2025-11-25T14:39:49.448Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-11-25T14:39:49.448Z
Learning: Applies to apps/web/utils/actions/*.ts : Use `actionClient` when both authenticated user context and a specific emailAccountId are needed, with emailAccountId bound when calling from the client
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Mock 'server-only' module with empty object in LLM test files: `vi.mock("server-only", () => ({}))`
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.test.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Use TypeScript types for all LLM function parameters and return values, and define clear interfaces for complex input/output structures
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.ts
📚 Learning: 2025-11-25T14:38:07.606Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-11-25T14:38:07.606Z
Learning: Applies to apps/web/utils/ai/**/*.ts : Use descriptive scoped loggers for each LLM feature, log inputs and outputs with appropriate log levels, and include relevant context in log messages
Applied to files:
apps/web/utils/ai/choose-rule/run-rules.tsapps/web/utils/meeting-briefs/process.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Applied to files:
apps/web/utils/meeting-briefs/gather-context.ts
🧬 Code graph analysis (6)
apps/web/utils/user/delete.ts (1)
apps/web/utils/redis/research-cache.ts (1)
clearCachedResearchForUser(39-80)
apps/web/__tests__/ai-meeting-briefing.test.ts (4)
apps/web/utils/logger.ts (1)
createScopedLogger(18-82)apps/web/utils/meeting-briefs/gather-context.ts (2)
CalendarEvent(19-19)MeetingBriefingData(26-31)apps/web/utils/ai/meeting-briefs/generate-briefing.ts (3)
buildPrompt(345-397)formatMeetingForContext(498-509)BriefingContent(41-41)apps/web/__tests__/helpers.ts (2)
getMockMessage(139-179)getEmailAccount(26-46)
apps/web/utils/ai/meeting-briefs/generate-briefing.ts (8)
packages/resend/emails/meeting-briefing.tsx (1)
BriefingContent(19-21)apps/web/utils/meeting-briefs/gather-context.ts (1)
MeetingBriefingData(26-31)apps/web/utils/llms/types.ts (1)
EmailAccountWithAI(10-32)apps/web/utils/logger.ts (1)
Logger(6-6)apps/web/utils/llms/index.ts (1)
createGenerateText(46-137)apps/web/env.ts (1)
env(17-258)apps/web/utils/redis/research-cache.ts (2)
getCachedResearch(82-98)setCachedResearch(100-129)apps/web/utils/llms/config.ts (1)
Provider(3-12)
apps/web/utils/ai/choose-rule/run-rules.test.ts (1)
apps/web/utils/ai/choose-rule/run-rules.ts (1)
limitDraftEmailActions(587-638)
apps/web/utils/redis/research-cache.ts (2)
apps/web/utils/logger.ts (1)
createScopedLogger(18-82)apps/web/env.ts (1)
env(17-258)
packages/resend/emails/meeting-briefing.tsx (1)
apps/web/components/new-landing/common/Section.tsx (1)
Section(9-15)
⏰ 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). (5)
- GitHub Check: Baz Reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Macroscope - Correctness Check
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (23)
apps/web/utils/ai/choose-rule/run-rules.test.ts (4)
24-47: LGTM! Comprehensive mocking for integration tests.The mocks are properly configured for testing
runRulesin isolation. Theaftermock executing the callback synchronously is appropriate for testing purposes.
457-520: Well-structured test for the core bug scenario.This test directly validates the fix—ensuring that when both a custom rule and a resolved TO_REPLY rule have
DRAFT_EMAILactions, the static draft (withcontent) is retained while the dynamic draft (nullcontent) is removed. The assertions also verify thatisConversationRuleandresolvedReasonflags are preserved through the limiting process.
522-553: Good coverage for the fallback case.This test ensures deterministic behavior when multiple AI-generated drafts (both with null content) exist—the first one encountered is retained.
556-675: Excellent integration test for the double draft prevention fix.This test validates the complete flow end-to-end:
- Sets up both a custom rule (with static draft) and a TO_REPLY rule (with dynamic draft)
- Mocks
findMatchingRulesto return the custom rule + meta rule- Mocks
determineConversationStatusto resolve to TO_REPLY with draft- Tracks what drafts are actually executed
- Asserts exactly one draft is executed with the expected content
This would have caught the original bug where
limitDraftEmailActionsran before the conversation meta-rule was resolved. As per coding guidelines, the test is co-located with the source file and uses the proper helper functions from@/__tests__/helpers.apps/web/utils/ai/choose-rule/run-rules.ts (4)
108-114: Clean separation of match types.Using
filterandfindinstead of a complex loop improves readability. The separation allows the conversation meta-rule to be resolved independently before draft limiting.
116-144: Core fix implemented correctly.The resolution flow ensures
limitDraftEmailActions(called at line 146) can now observe allDRAFT_EMAILactions, including those from the resolved conversation rule. TheisConversationRuleandresolvedReasonflags provide useful metadata for downstream processing.
181-193: Simplified execution loop.The refactor removes in-loop re-evaluation of conversation status. Using
result.isConversationRulefor conditional logic is cleaner than re-checking rule IDs or system types.
587-589: Type-safe generic signature.The generic constraint specifies the minimum required properties while preserving all additional properties (like
isConversationRuleandresolvedReason) through the spread operator in the return statement. This maintains type safety while allowing the function to work with extended match types.apps/web/utils/email.ts (1)
145-145: Approved — mail.com is a legitimate public email provider.The addition of mail.com to PUBLIC_EMAIL_DOMAINS is appropriate. mail.com is a long-established public email service (founded 1995, operated by United Internet AG) offering free webmail with ads and alternative domain options, making it consistent with the other major public providers already in the set.
apps/web/utils/meeting-briefs/process.ts (1)
203-207: LGTM! Logger integration enhances traceability.The addition of the event-scoped logger parameter to
aiGenerateMeetingBriefingenables better diagnostic traceability for AI briefing generation. The logger is correctly scoped to the specific event context.Based on learnings, this follows the pattern of passing loggers as parameters to utility functions rather than creating logger instances within them.
apps/web/utils/user/delete.ts (1)
12-12: LGTM! Consistent refactoring to generalized research cache.The changes correctly migrate from Perplexity-specific research caching to a source-agnostic research cache system. Calling
clearCachedResearchForUser(userId)without a source parameter appropriately clears all research sources for the user being deleted.Also applies to: 71-74
packages/resend/emails/meeting-briefing.tsx (1)
100-109: LGTM! Disclaimer appropriately manages user expectations.The addition of the AI-generated content disclaimer is good practice and helps manage user expectations. The specific caveat about "common names" demonstrates awareness of entity disambiguation challenges in AI-generated briefings.
apps/web/utils/meeting-briefs/gather-context.ts (1)
88-96: LGTM! Guest mapping correctly aligned with interface.The simplified mapping of external guests correctly matches the
ExternalGuestinterface definition (lines 21-24). The removal of theaiResearchfield is consistent with the PR's broader refactoring away from Perplexity-specific research workflows.apps/web/__tests__/ai-meeting-briefing.test.ts (1)
1-492: LGTM! Comprehensive test coverage for AI briefing generation.This test file provides excellent coverage of the AI meeting briefing feature with well-structured tests across three sections:
- buildPrompt tests (6 tests): Cover meeting details, guest context, email history, meeting history, and multiple guests
- formatMeetingForContext tests (3 tests): Verify formatting, description inclusion, and truncation
- aiGenerateMeetingBriefing tests (6 conditional AI tests): Cover new contacts, email history, multiple guests, past meetings, edge cases, and rich context scenarios
The test file correctly follows all coding guidelines:
- ✓ Uses vitest imports appropriately
- ✓ Mocks
server-onlymodule- ✓ Conditionally runs AI tests with
describe.runIf(isAiTest)- ✓ Uses
console.debug()for LLM output- ✓ Placed in
__tests__/directory (correct for AI tests)- ✓ Uses helper functions from
@/__tests__/helpers- ✓ Descriptive test names
- ✓ Appropriate timeout (60s) for agentic AI workflow
The AI tests wisely provide email context to avoid sole reliance on external research APIs, making the tests more robust and predictable.
apps/web/utils/ai/meeting-briefs/generate-briefing.ts (5)
1-21: LGTM!Imports are well-organized, using the appropriate AI SDK packages and project utilities. Logger type imported correctly for dependency injection pattern.
23-26: LGTM!Good use of named constants for agent limits. These provide clear configuration points and prevent magic numbers throughout the code.
43-73: LGTM!The agentic system prompt is well-structured with clear workflow steps, search tips, and briefing guidelines. The searchInputSchema correctly captures the required fields for caching.
274-342: LGTM!The
createWebSearchToolfunction follows a clean pattern:
- Checks cache first
- Falls back to actual search on cache miss
- Fire-and-forget caching with error logging
- Graceful error handling returning a user-friendly message
145-155: LGTM!The fallback briefing provides a sensible default when the agent fails, and
formatGuestContextcorrectly handles the no-prior-context case with guidance to use search tools.Also applies to: 407-420
apps/web/utils/redis/research-cache.ts (4)
6-12: LGTM!Good refactoring to support multiple research sources. The
ResearchSourcetype provides type safety for the source parameter, and the updated logger scope accurately reflects the module's purpose.
18-37: LGTM!The key structure
research:{source}:{userId}:{hash}is well-designed:
- Source-specific caching prevents collisions between Perplexity and web search results
- The wildcard patterns in
getUserKeyPatterncorrectly support both source-specific and all-source clearing
39-80: LGTM!The
clearCachedResearchForUserfunction properly handles:
- Redis configuration check
- Cursor-based scanning for large key sets
- Batch deletion with count tracking
- Comprehensive logging including source context
82-129: LGTM!Both
getCachedResearchandsetCachedResearchproperly:
- Check Redis configuration before operations
- Include source in cache keys and logging
- Validate content before caching (empty check, size limit)
- Handle errors gracefully with appropriate logging
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| if (conversationMatch) { | ||
| const { rule, reason } = await determineConversationStatus({ | ||
| conversationRules, | ||
| message, | ||
| emailAccount, | ||
| provider, | ||
| modelType, |
There was a problem hiding this comment.
When the conversation meta-rule is present we now call determineConversationStatus and only push the resolved match into matchesWithFlags when rule is truthy. If determineConversationStatus returns undefined there is no branch that records the SKIPPED result with the provided reason, so the conversation match is silently dropped and, if there are no other matches, we fall through to the generic "No rules matched" branch without surfacing the actual conversation skip reason. Could we keep emitting the SKIPPED result/reason when the conversation rule resolves to nothing so we still know why nothing executed?
Finding type: Logical Bugs
There was a problem hiding this comment.
Commit e1c7c88 addressed this comment by introducing a skippedConversationReason variable that captures the reason when determineConversationStatus returns a falsy rule. The code now prioritizes this specific conversation skip reason over generic fallback messages when no matches are found, ensuring the actual conversation skip reason is surfaced rather than being silently dropped.
There was a problem hiding this comment.
Fixed in e1c7c88. Now tracking skippedConversationReason when the conversation rule resolves to nothing, and using it in the SKIPPED result if no other matches exist.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 11 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/utils/ai/choose-rule/run-rules.ts">
<violation number="1" location="apps/web/utils/ai/choose-rule/run-rules.ts:136">
P2: When `determineConversationStatus` returns no rule but provides a skip reason, that reason is silently discarded. Consider recording a SKIPPED result with the reason so debugging can surface why the conversation rule resolved to nothing instead of falling through to a generic "No rules matched" message.</violation>
</file>
<file name="apps/web/utils/user/delete.ts">
<violation number="1" location="apps/web/utils/user/delete.ts:71">
P2: `clearCachedResearchForUser` catches and returns on errors internally, so this `.catch(...)` (and `captureException`) likely never runs. Either remove the `.catch` block and rely on the internal logger, or change `clearCachedResearchForUser` to rethrow so callers can handle failures.</violation>
</file>
<file name="apps/web/utils/ai/meeting-briefs/generate-briefing.ts">
<violation number="1" location="apps/web/utils/ai/meeting-briefs/generate-briefing.ts:117">
P3: Off-by-one in agent step limit: `> MAX_AGENT_STEPS` allows one extra step. Use `>=` to enforce the configured max.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| modelType, | ||
| isTest, | ||
| }); | ||
| if (rule) { |
There was a problem hiding this comment.
P2: When determineConversationStatus returns no rule but provides a skip reason, that reason is silently discarded. Consider recording a SKIPPED result with the reason so debugging can surface why the conversation rule resolved to nothing instead of falling through to a generic "No rules matched" message.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/utils/ai/choose-rule/run-rules.ts, line 136:
<comment>When `determineConversationStatus` returns no rule but provides a skip reason, that reason is silently discarded. Consider recording a SKIPPED result with the reason so debugging can surface why the conversation rule resolved to nothing instead of falling through to a generic "No rules matched" message.</comment>
<file context>
@@ -105,7 +105,45 @@ export async function runRules({
+ modelType,
+ isTest,
+ });
+ if (rule) {
+ matchesWithFlags.push({
+ rule,
</file context>
✅ Addressed in e1c7c88
There was a problem hiding this comment.
Commit e1c7c88 addressed this comment by capturing the skip reason from determineConversationStatus in a new variable skippedConversationReason and prioritizing it when creating SKIPPED results. This ensures the specific reason why a conversation rule resolved to nothing is preserved for debugging instead of being silently discarded.
There was a problem hiding this comment.
Fixed in e1c7c88. Added skippedConversationReason tracking - when the conversation rule resolves to nothing, we now preserve the skip reason and use it in the SKIPPED result.
|
|
||
| clearCachedPerplexityResearchForUser(userId).catch((error) => { | ||
| logger.error("Error clearing cached Perplexity research", { error }); | ||
| clearCachedResearchForUser(userId).catch((error) => { |
There was a problem hiding this comment.
P2: clearCachedResearchForUser catches and returns on errors internally, so this .catch(...) (and captureException) likely never runs. Either remove the .catch block and rely on the internal logger, or change clearCachedResearchForUser to rethrow so callers can handle failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/utils/user/delete.ts, line 71:
<comment>`clearCachedResearchForUser` catches and returns on errors internally, so this `.catch(...)` (and `captureException`) likely never runs. Either remove the `.catch` block and rely on the internal logger, or change `clearCachedResearchForUser` to rethrow so callers can handle failures.</comment>
<file context>
@@ -68,8 +68,8 @@ export async function deleteUser({
- clearCachedPerplexityResearchForUser(userId).catch((error) => {
- logger.error("Error clearing cached Perplexity research", { error });
+ clearCachedResearchForUser(userId).catch((error) => {
+ logger.error("Error clearing cached research", { error });
captureException(error);
</file context>
| stopWhen: (stepResult) => | ||
| stepResult.steps.some((step) => | ||
| step.toolCalls?.some((call) => call.toolName === "finalizeBriefing"), | ||
| ) || stepResult.steps.length > MAX_AGENT_STEPS, |
There was a problem hiding this comment.
P3: Off-by-one in agent step limit: > MAX_AGENT_STEPS allows one extra step. Use >= to enforce the configured max.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/utils/ai/meeting-briefs/generate-briefing.ts, line 117:
<comment>Off-by-one in agent step limit: `> MAX_AGENT_STEPS` allows one extra step. Use `>=` to enforce the configured max.</comment>
<file context>
@@ -22,55 +38,307 @@ const briefingSchema = z.object({
+ stopWhen: (stepResult) =>
+ stepResult.steps.some((step) =>
+ step.toolCalls?.some((call) => call.toolName === "finalizeBriefing"),
+ ) || stepResult.steps.length > MAX_AGENT_STEPS,
+ tools: {
+ ...tools,
</file context>
| ) || stepResult.steps.length > MAX_AGENT_STEPS, | |
| ) || stepResult.steps.length >= MAX_AGENT_STEPS, |
| @@ -141,35 +185,10 @@ export async function runRules({ | |||
| const executedRules: RunRulesResult[] = []; | |||
There was a problem hiding this comment.
When a conversation meta-rule matches but determineConversationStatus returns no rule, the skip isn’t recorded if other rules also matched. Consider adding a SKIPPED result (and DB record) for the conversation rule even when other rules apply, so the skip is captured consistently.
🚀 Want me to fix this? Reply ex: "fix it for me".
There was a problem hiding this comment.
Not addressing this. When other rules match and execute, they get stored in the database - the user can see what happened. Recording a SKIPPED result for the conversation rule alongside executed rules would add noise. The skip reason is primarily useful when nothing at all matched, which is now handled with skippedConversationReason.
1 participant
User description
Fixes the double drafting bug where users with multi-rule selection enabled receive two draft replies for the same email.
TLDR: Moves
limitDraftEmailActionsto execute after conversation meta-rules are resolved, so it sees all DRAFT_EMAIL actions.limitDraftEmailActionswas called before the conversation meta-rule was resolved to the actual TO_REPLY rule, so it couldn't see both draftsSummary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Generated description
Below is a concise technical summary of the changes proposed in this PR:
graph LR subgraph "inbox-zero-ai" ["inbox-zero-ai"] runMeetingBrief_("runMeetingBrief"):::modified aiGenerateMeetingBriefing_("aiGenerateMeetingBriefing"):::modified buildSearchTools_("buildSearchTools"):::added createWebSearchTool_("createWebSearchTool"):::added getCachedResearch_("getCachedResearch"):::added setCachedResearch_("setCachedResearch"):::added deleteUser_("deleteUser"):::modified clearCachedResearchForUser_("clearCachedResearchForUser"):::added runMeetingBrief_ -- "Passes event logger into aiGenerateMeetingBriefing, enabling agent logs." --> aiGenerateMeetingBriefing_ aiGenerateMeetingBriefing_ -- "Builds agent search tools (perplexity/web) for guest research." --> buildSearchTools_ buildSearchTools_ -- "Wraps provider-specific web search as tool for agent." --> createWebSearchTool_ buildSearchTools_ -- "Checks Redis cache for prior guest research by source." --> getCachedResearch_ buildSearchTools_ -- "Caches fresh external research results keyed by user." --> setCachedResearch_ createWebSearchTool_ -- "Retrieves cached web-search summaries for guest before querying." --> getCachedResearch_ createWebSearchTool_ -- "Stores web-search findings in Redis for reuse." --> setCachedResearch_ deleteUser_ -- "Evicts all research cache entries for user upon deletion." --> clearCachedResearchForUser_ classDef added stroke:#15AA7A classDef removed stroke:#CD5270 classDef modified stroke:#EDAC4C linkStyle default stroke:#CBD5E1,font-size:13px end subgraph "@inboxzero/resend" ["@inboxzero/resend"] classDef added stroke:#15AA7A classDef removed stroke:#CD5270 classDef modified stroke:#EDAC4C linkStyle default stroke:#CBD5E1,font-size:13px endRefactor the AI rule execution flow to prevent duplicate email drafts by ensuring
limitDraftEmailActionsprocesses all resolved rules, and significantly enhance AI meeting briefing generation with an agentic workflow that leverages configurable web search tools for comprehensive guest research. Additionally, improve email domain recognition and add a disclaimer to AI-generated briefing content.aiGenerateMeetingBriefingthat dynamically integrates various web search tools (e.g., Perplexity, OpenAI, Google search) for comprehensive external guest research. This enhancement replaces the previous direct Perplexity research module, generalizes the caching mechanism for all research results, and introduces a user-facing disclaimer in the briefing email to indicate AI-generated content. It also includes updates to how meeting context is gathered and processed for the new agentic system.Modified files (8)
Latest Contributors(1)
Modified files (1)
Latest Contributors(2)
runRulesfunction to correctly resolve conversation meta-rules before applyinglimitDraftEmailActions. This ensures that only a singleDRAFT_EMAILaction is executed, even when multiple rules suggest drafting an email, thereby fixing a critical double drafting bug. The change involves separating regular matches from conversation meta-rules, resolving the conversation status, and then applying the draft limiting logic. This also includes new integration tests to validate the fix.Modified files (2)
Latest Contributors(2)