Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@elie222 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 38 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request refactors import paths and validation modules across various parts of the codebase, introduces several new Zod validation schemas, and adds reply tracking features. A new API route and custom hook are introduced, with accompanying database migration and schema updates for draft reply fields. Additional changes include UI enhancements such as a reply tracker dialog, minor CSS adjustments, and updates to AI and email utility functions including parameter renaming. Overall, the modifications standardize validation, improve rule management, and enhance reply tracking functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant B as Button (Dialog Trigger)
participant D as Dialog
participant RTS as ReplyTrackerSettings UI
participant S as Server (API/Update Action)
U->>B: Clicks on settings button
B->>D: Opens dialog trigger
D->>RTS: Renders ReplyTrackerSettings component
RTS->>U: Displays form for reply tracking settings
U->>RTS: Submits updated settings
RTS->>S: Calls updateRuleSettingsAction API
S-->>RTS: Returns update confirmation
RTS-->>D: Closes dialog, updates UI
sequenceDiagram
participant C as Client
participant R as API Route
participant A as Auth Check
participant P as Prisma DB
C->>R: Send GET /api/user/rules/[id]
R->>A: Validate session/authentication
A-->>R: Auth confirmed (or error)
R->>P: Query rule with given ruleId and userId
P-->>R: Return rule data
R-->>C: Respond with JSON rule data
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (12)
apps/web/utils/actions/categorize.validation.ts (1)
1-8: Well-structured Zod validation schema for category creationThe validation schema looks well-designed with appropriate constraints for each field. Using Zod for validation provides good type safety and runtime validation.
Consider a few minor enhancements that could make this validation more robust:
- Add
.min(1)to thenamefield to prevent empty names- Consider adding
.trim()to string fields to handle whitespace consistently- Add custom error messages for better user feedback
Example enhancement:
export const createCategoryBody = z.object({ id: z.string().nullish(), - name: z.string().max(30), + name: z.string().min(1, { message: "Name cannot be empty" }).max(30, { message: "Name cannot exceed 30 characters" }).trim(), description: z.string().max(300).nullish(), });apps/web/utils/actions/cold-email.validation.ts (1)
1-14: Good implementation of Zod schema validation for cold emails.The schema properly defines the structure for validating cold email data with appropriate nullable and optional fields. The type export using
z.inferis a good practice to maintain type safety throughout the application.Consider enhancing the validation rules for specific fields:
- Add email format validation for the
fromfield- Add more specific validation for
dateinstead of accepting both string and number- Consider adding min/max length constraints for text fields where appropriate
export const coldEmailBlockerBody = z.object({ - from: z.string(), + from: z.string().email("Invalid email format"), subject: z.string(), textHtml: z.string().nullable(), textPlain: z.string().nullable(), snippet: z.string().nullable(), // Hacky fix. Not sure why this happens. Is internalDate sometimes a string and sometimes a number? - date: z.string().or(z.number()).optional(), + date: z.coerce.date().optional(), threadId: z.string().nullable(), messageId: z.string().nullable(), });apps/web/app/(app)/automation/RuleForm.tsx (1)
795-824: New component for explaining Reply Zero Tracker functionality.The component provides clear information about what the Reply Zero Tracker does, including labeling emails that need replies and drafting responses. It also properly explains that these actions are automatically configured.
One suggestion: Consider adding more detailed information about how the AI determines when to draft replies.
apps/web/app/api/user/rules/[id]/route.ts (1)
1-28: New API endpoint for retrieving individual rules.This endpoint provides a clean way to fetch rules by ID for authenticated users. The implementation includes proper authentication checks and error handling for missing parameters.
Some suggestions for improvement:
- Consider adding more detailed error responses with appropriate HTTP status codes
- The rule query could include related data that might be needed by clients (e.g., actions, conditions)
- Add validation for the rule ID format before querying the database
- return NextResponse.json({ error: "Not authenticated" }); + return NextResponse.json({ error: "Not authenticated" }, { status: 401 });- if (!params.id) return NextResponse.json({ error: "Missing rule id" }); + if (!params.id) return NextResponse.json({ error: "Missing rule id" }, { status: 400 });- const rule = await prisma.rule.findUnique({ where: { id: ruleId, userId } }); + const rule = await prisma.rule.findUnique({ + where: { id: ruleId, userId }, + include: { + actions: true, + conditions: true + } + });apps/web/app/(app)/reply-zero/page.tsx (1)
93-106: Add an accessible label for the settings button.Since this button only uses an icon, including an
aria-label(and/ortitle) helps screen readers and improves accessibility. For instance:<Button variant="ghost" size="icon" + aria-label="Settings" + title="Settings" > <SettingsIcon className="size-4" /> </Button>apps/web/utils/actions/rule.validation.ts (3)
9-19: Consider usingz.nativeEnum(ActionType).Using
z.nativeEnum(ActionType)automatically stays in sync with the enum in Prisma when new actions are added or removed.
21-25: Usez.nativeEnum(RuleType)for better maintainability.This approach makes the schema more resilient to future additions or removals of rule types in your Prisma enum.
45-51: Consider using a discriminated union for conditions.Right now all fields from
zodAiCondition,zodStaticCondition, andzodCategoryConditionare merged, potentially allowing irrelevant fields to pass. A discriminated union withz.discriminatedUnion("type", {...})ensures strict field checks per condition type.apps/web/utils/reply-tracker/generate-reply.ts (1)
13-40: Add error handling and possibly return the created draft’s ID.Surrounding the draft creation logic with a try/catch or returning the draft’s ID could help debug and ensure the caller knows if the draft was successfully generated.
apps/web/app/(app)/reply-zero/ReplyTrackerSettings.tsx (1)
47-55: Add missing dependencies to useCallbackThe
useCallbackhook is missing dependencies. Even though your function isn't using external values directly in its body, it's callingupdateRuleSettingsActionwhich should be included in the dependency array.const onSubmit: SubmitHandler<UpdateRuleSettingsBody> = useCallback( async (data) => { const res = await updateRuleSettingsAction(data); if (isErrorMessage(res)) toastError({ description: "There was an error updating the settings" }); else toastSuccess({ description: "Settings updated" }); }, - [], + [updateRuleSettingsAction], );apps/web/utils/actions/ai-rule.validation.ts (1)
16-33: Consider improving error message placement for related fieldsThe refinement for requiring either
actualRuleIdorexpectedRuleIdshows the error on theexpectedRuleIdfield only. This might be confusing if a user is trying to provide onlyactualRuleId.Consider using a custom path like
["_errors"]or showing the error on both fields for better user experience:.refine((data) => data.actualRuleId != null || data.expectedRuleId != null, { message: "Either the actual or the expected rule must be provided", - path: ["expectedRuleId"], // This will show the error on the expectedRuleId field + path: ["_errors"], // This will show as a form-level error });apps/web/utils/reply-tracker/inbound.ts (1)
78-88: Improve error handling for draft generationThe draft generation is handled asynchronously outside the main
Promise.allSettledgroup with a basic error log. Consider implementing a more robust error handling strategy such as retrying or providing more context in the error log.const draftPromise = generateDraft(email, gmail, message); const [dbResult, removeLabelResult, newLabelResult] = await Promise.allSettled([dbPromise, removeLabelPromise, newLabelPromise]); // Avoid hitting Gmail rate limit by doing too much at once try { await draftPromise; } catch (error) { - logger.error("Failed to create draft", { error }); + logger.error("Failed to create draft", { + error, + messageId, + threadId, + reason: error instanceof Error ? error.message : 'Unknown error' + }); + // Consider implementing retry logic or user notification here }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
apps/web/__tests__/ai-choose-rule.test.ts(3 hunks)apps/web/app/(app)/automation/ReportMistake.tsx(1 hunks)apps/web/app/(app)/automation/RuleForm.tsx(4 hunks)apps/web/app/(app)/automation/RulesPrompt.tsx(1 hunks)apps/web/app/(app)/automation/TestCustomEmailForm.tsx(1 hunks)apps/web/app/(app)/automation/create/examples.tsx(1 hunks)apps/web/app/(app)/automation/group/ViewGroup.tsx(1 hunks)apps/web/app/(app)/cold-email-blocker/TestRules.tsx(1 hunks)apps/web/app/(app)/onboarding/OnboardingEmailAssistant.tsx(1 hunks)apps/web/app/(app)/reply-zero/ReplyTrackerSettings.tsx(1 hunks)apps/web/app/(app)/reply-zero/page.tsx(2 hunks)apps/web/app/(app)/settings/ApiKeysCreateForm.tsx(1 hunks)apps/web/app/(app)/smart-categories/CreateCategoryButton.tsx(1 hunks)apps/web/app/api/google/webhook/types.ts(1 hunks)apps/web/app/api/user/rules/[id]/route.ts(1 hunks)apps/web/components/Toggle.tsx(1 hunks)apps/web/hooks/useRule.tsx(1 hunks)apps/web/prisma/migrations/20250223190244_draft_replies/migration.sql(1 hunks)apps/web/prisma/schema.prisma(1 hunks)apps/web/scripts/encrypt-tokens.ts(1 hunks)apps/web/utils/actions/ai-rule.ts(2 hunks)apps/web/utils/actions/ai-rule.validation.ts(1 hunks)apps/web/utils/actions/api-key.ts(1 hunks)apps/web/utils/actions/api-key.validation.ts(1 hunks)apps/web/utils/actions/categorize.ts(1 hunks)apps/web/utils/actions/categorize.validation.ts(1 hunks)apps/web/utils/actions/cold-email.ts(1 hunks)apps/web/utils/actions/cold-email.validation.ts(1 hunks)apps/web/utils/actions/group.ts(1 hunks)apps/web/utils/actions/group.validation.ts(1 hunks)apps/web/utils/actions/reply-tracking.ts(3 hunks)apps/web/utils/actions/rule.ts(2 hunks)apps/web/utils/actions/rule.validation.ts(3 hunks)apps/web/utils/ai/choose-rule/execute.ts(4 hunks)apps/web/utils/ai/choose-rule/run-rules.ts(1 hunks)apps/web/utils/ai/reply/check-if-needs-reply.ts(1 hunks)apps/web/utils/ai/reply/generate-reply.ts(1 hunks)apps/web/utils/categorize/senders/categorize.ts(1 hunks)apps/web/utils/condition.ts(1 hunks)apps/web/utils/gmail/mail.ts(1 hunks)apps/web/utils/llms/types.ts(1 hunks)apps/web/utils/reply-tracker/check-previous-emails.ts(1 hunks)apps/web/utils/reply-tracker/consts.ts(1 hunks)apps/web/utils/reply-tracker/generate-reply.ts(1 hunks)apps/web/utils/reply-tracker/inbound.ts(5 hunks)apps/web/utils/reply-tracker/outbound.ts(1 hunks)
✅ Files skipped from review due to trivial changes (15)
- apps/web/components/Toggle.tsx
- apps/web/utils/actions/group.ts
- apps/web/utils/condition.ts
- apps/web/app/(app)/automation/RulesPrompt.tsx
- apps/web/app/(app)/onboarding/OnboardingEmailAssistant.tsx
- apps/web/app/(app)/smart-categories/CreateCategoryButton.tsx
- apps/web/app/(app)/automation/create/examples.tsx
- apps/web/utils/actions/categorize.ts
- apps/web/utils/actions/cold-email.ts
- apps/web/app/(app)/settings/ApiKeysCreateForm.tsx
- apps/web/app/(app)/automation/TestCustomEmailForm.tsx
- apps/web/app/(app)/automation/ReportMistake.tsx
- apps/web/utils/ai/choose-rule/run-rules.ts
- apps/web/app/(app)/automation/group/ViewGroup.tsx
- apps/web/app/(app)/cold-email-blocker/TestRules.tsx
🔇 Additional comments (45)
apps/web/utils/actions/group.validation.ts (1)
1-9: Looks good - clean validation schema implementation.This new file effectively establishes a dedicated validation schema for group items using Zod. The schema properly validates the required fields with appropriate types, and exports a type alias using Zod's inference capabilities for type safety throughout the application.
This change aligns with the PR's objective of refactoring validation modules across the codebase for better organization and standardization.
apps/web/utils/actions/api-key.validation.ts (1)
1-7: Well-structured validation schema implementation.The Zod schema definitions are clean, focused, and properly typed with corresponding TypeScript type exports. This approach allows for both runtime validation and static type checking.
apps/web/utils/actions/api-key.ts (1)
5-10: Good import refactoring.The import statement has been properly updated to use the new validation module. Using explicit type imports with the
typekeyword is a good TypeScript practice as it ensures these imports are removed during transpilation, leading to smaller bundle sizes.apps/web/utils/actions/cold-email.validation.ts (1)
9-10: Investigate the type inconsistency for thedatefield.The comment indicates uncertainty about why the
datefield sometimes has a string type and sometimes a number type. This inconsistency should be investigated at the source to determine if there's a more elegant solution than accepting multiple types.You might want to trace how the date is being provided to this validation schema. Is it coming from different API sources or being transformed differently in various parts of the application?
apps/web/scripts/encrypt-tokens.ts (1)
23-25: Ordering byupdatedAtis a good optimization.Sorting accounts by most recently updated first ensures that active accounts are processed with priority. This is especially beneficial if the script encounters issues that prevent full completion.
apps/web/prisma/migrations/20250223190244_draft_replies/migration.sql (1)
1-3: Schema changes look appropriate for draft reply functionality.The addition of
draftRepliesas a boolean flag anddraftRepliesInstructionsas a text field provides the necessary structure to support the auto draft reply feature mentioned in the PR title.apps/web/hooks/useRule.tsx (1)
1-8:Details
❓ Verification inconclusive
Clean implementation of the
useRulehook.The hook follows React best practices with conditional fetching (only when ruleId is provided) and proper TypeScript typing. One note: you're using the default fetcher for SWR - ensure this aligns with your API response handling needs.
Make sure your API endpoint returns data that matches the expected
RuleResponsetype:
🏁 Script executed:
#!/bin/bash # Verify if there's a corresponding API route for this hook fd --type file "route.ts" -p "api/user/rules/\[id\]"Length of output: 161
Action: Verify API Endpoint Route
The implementation of the
useRulehook looks clean and follows React best practices with conditional fetching and proper TypeScript typings. However, our initial attempt to verify the corresponding API route (i.e. the file implementing/api/user/rules/[id]) using the shell script encountered an issue due to an invalid search path.Please ensure that your repository contains the API endpoint file (typically something like
api/user/rules/[id]/route.ts) and that its response conforms to the expectedRuleResponsetype. You might want to re-run verification using a command similar to the one below:#!/bin/bash # Corrected verification: search for route.ts files in directories containing the expected API path segment fd --type file "route.ts" | grep -E "api/user/rules/\\[id\\]"Manually verify the location and content of the API route if the automated search does not produce the expected output.
apps/web/utils/reply-tracker/consts.ts (1)
3-12: Well-structured instructions for reply tracking.The
defaultReplyTrackerInstructionsconstant provides clear, comprehensive guidelines for identifying emails that need replies. Extracting this to a constant improves maintainability and consistency across the application.apps/web/utils/llms/types.ts (1)
4-4: Type signature enhancement looks good.The addition of the
idfield to theUserEmailWithAItype is a logical improvement that ensures consistent user identification across the application.apps/web/prisma/schema.prisma (1)
192-193: Clean implementation of draft reply feature fields.The new fields for draft replies functionality are well-placed within the reply tracking section of the Rule model and properly typed as optional.
apps/web/utils/ai/reply/generate-reply.ts (1)
27-31: Improved AI guidance for more professional replies.The additional instructions enhance the quality of generated replies by providing clearer guidelines on brevity, placeholder usage, and avoiding invention of information.
apps/web/utils/ai/reply/check-if-needs-reply.ts (1)
20-20:Details
❓ Verification inconclusive
Verify impact of removing id field from parameter type.
While simplifying the user parameter type by removing the
idfield seems intentional, this appears to contradict addingidtoUserEmailWithAIintypes.ts. Ensure this change doesn't break functionality in thechatCompletionObjectfunction or other dependent code.
🏁 Script executed:
#!/bin/bash # Check if chatCompletionObject uses the user.id field rg -A 5 -B 5 "chatCompletionObject" | grep -A 10 "userAi" # Check for other usages of user.id in reply-related functions rg "user\.id" --type ts "utils/ai/reply"Length of output: 9420
Action Required: Confirm Removal of
idField Does Not Break FunctionalityAfter running the initial verification, no direct usage of
user.idwas found in thechatCompletionObjector in reply-related functions. However, note that the type definition intypes.ts—whereUserEmailWithAIincludes anid—suggests that some code paths might expect this field. Please manually verify that the current compositionuser: Pick<User, "about"> & UserEmailWithAI;(which omits
idfrom the Pick portion) does not remove the necessaryidproperty at runtime or cause unexpected behavior in dependent functions.
- Verify in detail:
- Confirm that the
chatCompletionObjectand any reply-related logic do not rely on anidproperty directly from the combined type.- Ensure that the intersection effectively provides the
idfromUserEmailWithAI(if that is the intent) and that this change aligns with overall code expectations.Please perform a careful manual review of these impacts.
apps/web/utils/gmail/mail.ts (1)
237-239: Appropriate update to parameter optionality.Making
toandsubjectparameters optional is a good improvement since the function already handles missing values by using fallbacks from the original email (lines 249-255).apps/web/utils/actions/rule.ts (3)
13-15: LGTM! Good refactoring of validation imports.The addition of
updateRuleSettingsBodyandUpdateRuleSettingsBodyfrom the validation file shows good separation of concerns by moving validation logic to a dedicated file.
293-293: Good revalidation path addition for reply-zero.This ensures that the UI is updated after rule settings change, providing a consistent user experience.
259-297:Details
❓ Verification inconclusive
Well-structured new action function for updating rule settings.
The implementation follows the same pattern as other action functions in the file, making it consistent with the codebase. It properly validates input, performs error checking, and updates necessary related data.
However, there are commented-out fields in the data object (lines 278-279) that appear to be related to draft replies. Consider whether these should be implemented or removed if they're not needed.
🏁 Script executed:
#!/bin/bash # Check if these fields exist in the schema and if they're used elsewhere rg "autoDraftReply|draftReplyInstructions" -A 3 -B 3Length of output: 611
Action Required: Verify Handling of Commented Fields in Rule Settings Update
The new action function is well-structured and consistent with the rest of the file. However, note that the fields
autoDraftReplyanddraftReplyInstructionsremain commented out. Our grep search confirms they only appear here and aren’t referenced elsewhere. Please verify whether these fields are intended to be part of the update logic (in which case, ensure their proper implementation along with any required schema adjustments) or if they should be removed to keep the code clean.
- Confirm if the draft reply functionality is planned.
- If not needed, remove the commented-out code to reduce clutter.
apps/web/__tests__/ai-choose-rule.test.ts (3)
4-4: Good refactoring to use constants for rule instructions.Importing the
defaultReplyTrackerInstructionsconstant improves maintainability by centralizing the rule definition.
103-103: Improved maintainability by using the imported constant.Using
defaultReplyTrackerInstructionsinstead of a hardcoded string ensures consistency across the application and makes future updates easier.
147-163: Well-structured test case for meeting request detection.This test validates that the reply tracker correctly identifies emails requesting a meeting, which is an important use case. The test follows the same pattern as other test cases in the file.
apps/web/utils/categorize/senders/categorize.ts (1)
18-18:Details
✅ Verification successful
Simplifying the user parameter type.
This change suggests that
UserEmailWithAInow includes theidfield, making the use ofPick<User, "id">redundant. Ensure that all callers of this function provide an object with theidproperty.
🏁 Script executed:
#!/bin/bash # Check if UserEmailWithAI type includes id field rg -A 5 "type UserEmailWithAI" --glob="*.ts" # Check if all callers provide id property rg "categorizeSender\(" -A 5 -B 5Length of output: 3858
Simplified Parameter for
categorizeSenderVerifiedThe update now directly uses
UserEmailWithAIfor theuserparameter, which already includes theidfield (seeapps/web/utils/llms/types.ts). Verification of call sites—such as inapps/web/app/api/google/webhook/process-history-item.tsandapps/web/utils/actions/categorize.ts—confirms that all callers provide an object with anid.apps/web/utils/ai/choose-rule/execute.ts (3)
9-9: Type import update for more appropriate message representation.Changing from
EmailForActiontoParsedMessagesuggests a model refinement to better represent the data structure.
18-19: Consistent parameter renaming to match type change.Renaming the parameter from
messagebetter reflects its purpose and aligns with the type update toParsedMessage.Also applies to: 23-24
51-52: Updated function calls to use the renamed parameter.All occurrences of the renamed parameter have been consistently updated throughout the function, preventing potential errors.
Also applies to: 68-71
apps/web/app/(app)/automation/RuleForm.tsx (3)
33-33: Import path updated for better organization.The import path has been changed from
@/utils/actions/validationto@/utils/actions/rule.validation, which appears to be part of a broader restructuring of validation logic across files.
58-61: New constants imported for reply tracking feature.These constants are used in the newly added ReplyTrackerActionSection component to display consistent label names for awaiting and needed replies.
518-520: Added new section for Reply Zero Tracker in the form.This addition integrates the reply tracking feature into the rule form interface, providing users with information about how the feature works.
apps/web/utils/actions/ai-rule.ts (2)
26-30: Restructured imports for better organization.The import for
saveRulesPromptBodyand its type has been moved from@/utils/actions/validationto@/utils/actions/rule.validation, which aligns with the refactoring of validation logic across the codebase.
273-273: Parameter name updated for clarity.Changed parameter name from
messagein theexecuteActfunction call, which more accurately reflects the data type being passed (ParsedMessage) and maintains consistency with other related functions.apps/web/utils/actions/reply-tracking.ts (3)
12-15: Imported centralized reply tracker instructions constant.The previously hardcoded instructions have been replaced with a constant imported from a dedicated module, improving maintainability and consistency.
43-43: Added user ID field to selection.This ensures the user ID is available in the selected fields, which is necessary for subsequent operations.
104-104: Using standardized instructions from constant.This change replaces hardcoded instructions with the imported constant, making the code more maintainable and ensuring consistency across the application.
apps/web/app/(app)/reply-zero/page.tsx (1)
22-28: Nice integration of Dialog components.These imports align with common best practices for showing a dialog in place rather than navigating the user away.
apps/web/utils/actions/rule.validation.ts (5)
3-3: Import statements look good.No issues encountered with these additional imports for
ActionTypeandRuleType.Also applies to: 6-6
27-29: This AI condition schema looks straightforward.
31-36: Static condition schema is clearly defined.
38-43: Category condition schema looks good.
142-148: Confirm which fields should be optional vs. required.Currently, all properties (
instructions,draftReplies,draftRepliesInstructions) are mandatory. If users or upstream code sometimes omit these fields, consider.optional()or.nullish().apps/web/utils/reply-tracker/generate-reply.ts (2)
1-12: Import statements check out.The dependencies for Gmail, AI generation, Redis, and user data retrieval are well-organized.
42-74: Logic for generating and caching replies is well-structured.You cleanly avoid regenerating replies if they exist, minimizing redundant AI calls.
apps/web/app/(app)/reply-zero/ReplyTrackerSettings.tsx (2)
20-28: LGTM! Well-structured component with proper error handlingThe component correctly uses the
useRulehook to fetch data and handles loading and error states appropriately with theLoadingContentcomponent.
30-97: Form implementation follows best practicesThe form implementation is well-structured with:
- Proper form state management using react-hook-form
- Zod validation via zodResolver
- Conditional rendering for draft replies instructions
- Error handling and user feedback with toast messages
The code is clean, readable, and follows React best practices.
apps/web/utils/actions/ai-rule.validation.ts (2)
3-6: LGTM! Clear and concise validation schemaThe validation schema for the test content is well-defined with a meaningful error message.
8-14:Details
❓ Verification inconclusive
Consider clarifying the nullish rerun property
The
rerunproperty is defined asz.boolean().nullish(), which accepts null or undefined values. Verify if this is intentional or if you meant to make it optional withz.boolean().optional()instead.
🏁 Script executed:
#!/bin/bash # Search for usages of the runRulesBody schema to understand how rerun is used echo "Searching for imports of runRulesBody..." rg -A 2 "import.*runRulesBody" --type ts echo "Searching for usages of runRulesBody..." rg "runRulesBody" --type ts echo "Searching for places where rerun parameter is used..." rg "rerun.*\?.*:" --type tsLength of output: 921
Review Feedback: Clarify the
rerunProperty Handling
- In
apps/web/utils/actions/ai-rule.validation.ts(lines 8–14), thererunproperty is defined asz.boolean().nullish(), meaning it acceptstrue,false, as well asnullorundefined.- Notably, in
apps/web/app/(app)/automation/ProcessRules.tsx, the property is used as an optional boolean (rerun?: boolean), which suggests that onlytrue,false, orundefinedare expected.- Please confirm if allowing an explicit
nullvalue is intentional. If not, consider changing the schema toz.boolean().optional()for consistency with the expected usage.apps/web/utils/reply-tracker/inbound.ts (2)
17-17: LGTM! Good addition of draft generation functionalityAdding the draft generation capability is a valuable enhancement to the reply tracking system.
110-112:Details
✅ Verification successful
Verify impact of user parameter signature change
The signature of
handleInboundReplyhas been modified to remove theidfield from theuserparameter. Ensure all calling code has been updated to match this new signature.
🏁 Script executed:
#!/bin/bash # Search for places where handleInboundReply is called echo "Searching for calls to handleInboundReply..." rg "handleInboundReply\(" --type ts -A 3 -B 3Length of output: 1282
User Parameter Update Verified
I verified that the update to
handleInboundReply—removing theidfield from theuserparameter—is consistently reflected in the codebase. The only call site located inapps/web/utils/reply-tracker/check-previous-emails.tscorrectly passes the updated parameters (user,latestMessage, andgmail). No further changes are needed at this time.
|
|
||
| export async function handleOutboundReply( | ||
| user: Pick<User, "id" | "about"> & UserEmailWithAI, | ||
| user: Pick<User, "about"> & UserEmailWithAI, |
There was a problem hiding this comment.
Potential type inconsistency with parameter usage.
You've removed "id" from the Pick<User, "about"> type, but on line 23 there's still a reference to const userId = user.id;. This could cause TypeScript errors since the id property is no longer explicitly included in the type.
This inconsistency needs to be resolved by either:
- Restoring the "id" property in the parameter type, or
- Ensuring the
idproperty is coming from theUserEmailWithAItype - Modifying how
userIdis retrieved within the function
- user: Pick<User, "about"> & UserEmailWithAI,
+ user: Pick<User, "id" | "about"> & UserEmailWithAI,📝 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.
| user: Pick<User, "about"> & UserEmailWithAI, | |
| user: Pick<User, "id" | "about"> & UserEmailWithAI, |
| export async function processPreviousSentEmails( | ||
| gmail: gmail_v1.Gmail, | ||
| user: Pick<User, "id" | "about"> & UserEmailWithAI, | ||
| user: Pick<User, "about"> & UserEmailWithAI, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Type consistency check needed.
The id property has been removed from the user parameter type. Although there are no direct references to user.id in this function, verify that the id property isn't required for downstream functions like handleOutboundReply and handleInboundReply that are called with this user object.
🏁 Script executed:
#!/bin/bash
# Search for references to user.id in handleOutboundReply and handleInboundReply
echo "Checking for user.id references in related functions:"
rg -n "user\.id" "apps/web/utils/reply-tracker/outbound.ts" "apps/web/utils/reply-tracker/inbound.ts"Length of output: 519
Type consistency issue: Downstream functions require the user’s id
The removal of id from the user type in apps/web/utils/reply-tracker/check-previous-emails.ts is problematic. Our grep results show that both handleOutboundReply and handleInboundReply rely on user.id (see references in inbound.ts at lines 119 and 136, and in outbound.ts at lines 23 and 32). To resolve this, update the type signature to include the id property. For example:
- user: Pick<User, "about"> & UserEmailWithAI,
+ user: Pick<User, "id" | "about"> & UserEmailWithAI,Please update the type accordingly to ensure that all downstream calls receive the expected id property.
📝 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.
| user: Pick<User, "about"> & UserEmailWithAI, | |
| user: Pick<User, "id" | "about"> & UserEmailWithAI, |
| user: Pick< | ||
| User, | ||
| | "id" | ||
| | "about" | ||
| | "coldEmailPrompt" | ||
| | "coldEmailBlocker" | ||
| | "autoCategorizeSenders" | ||
| "about" | "coldEmailPrompt" | "coldEmailBlocker" | "autoCategorizeSenders" | ||
| > & |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify removal of "id" property across the codebase.
Removing the id property from the user type might be part of a broader refactoring. Check if any code using this type relies on accessing user.id.
🏁 Script executed:
#!/bin/bash
# Find files that import ProcessHistoryOptions and check for user.id usage
echo "Looking for files that import ProcessHistoryOptions:"
rg -l "ProcessHistoryOptions" "apps/web"
echo "\nChecking for user.id references in files that might be using ProcessHistoryOptions:"
rg -n "user\.id" $(rg -l "ProcessHistoryOptions" "apps/web")Length of output: 1302
Action Required: Address Remaining References to user.id
We've verified that after the removal of the id property from the user type in apps/web/app/api/google/webhook/types.ts, there are still active code paths that reference user.id. In particular, multiple occurrences in the file apps/web/app/api/google/webhook/process-history-item.ts (lines 60, 89, 115, and 159) indicate that the removal may break functionality where user.id is expected.
Please consider updating these references by either:
- Adding the
idproperty back into the user type where it is still needed, or - Refactoring the code in the affected files to remove dependency on
user.id.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/components/email-list/EmailMessage.tsx (1)
354-355: Remove debugging console.log statementThere's a console.log statement that appears to be left in for debugging purposes and should be removed before merging.
- console.log("🚀 ~ splitHtml:", splitHtml); -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/components/email-list/EmailMessage.tsx(5 hunks)apps/web/utils/parse/extract-reply.client.test.ts(1 hunks)apps/web/utils/parse/extract-reply.client.ts(1 hunks)
🔇 Additional comments (7)
apps/web/components/email-list/EmailMessage.tsx (5)
209-210: State variables renamed to better reflect functionalityThe state variables have been renamed from "isGeneratingNudge" and "nudge" to "isGeneratingReply" and "reply" respectively, which better reflects the actual functionality of generating replies rather than nudges.
222-248: Function renamed with improved state managementThe function has been renamed from "loadNudge" to "generateReply" with appropriate updates to its internal state management. The function maintains the same core functionality but with more accurate naming.
250-252: Improved condition for generating repliesThe condition for when to generate a reply has been updated to be more explicit: only generate a reply if
generateNudgeis true and there's no draft message. This prevents unnecessary API calls.
259-270: Updated memoization logic for reply contentThe memoization logic has been updated to use the new "reply" variable, maintaining the same functionality with more accurate naming that reflects the purpose of generating replies rather than nudges.
282-292: Loading state naming updated for consistencyThe loading state display has been updated to reflect the renamed state variable
isGeneratingReply, maintaining UI consistency with the backend functionality changes.apps/web/utils/parse/extract-reply.client.ts (1)
24-43: Well-implemented special case handling for Gmail's formatting.This addition neatly handles Gmail's specific format where replies are separated by
<br>tags before the quoted content. The approach is clean - extracting content before the separator, parsing it for the reply div, and maintaining proper structure in the returned object. This will improve reply extraction reliability for Gmail users.apps/web/utils/parse/extract-reply.client.test.ts (1)
95-113: Excellent test coverage for the new Gmail format handling.These tests thoroughly validate the new special case handling with realistic Gmail HTML structures. Good job including both a simple case and a more complex case with formatted text. The assertions properly check both the extracted draft content and the presence of the quote container in the original HTML.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style