Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces modifications to coding guidelines and various components within a TypeScript project. Key changes include standardizing punctuation in examples, emphasizing naming conventions, and reinforcing the use of specific UI libraries. Updates to multiple components focus on enhancing placeholder text for clarity. Significant logic updates are made to group item retrieval and actions, with new functions introduced for better handling of user prompts and session management. Minor adjustments across utility files include grammatical corrections in comments, while telemetry features are enhanced in AI model handling. Changes
Possibly related PRs
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: 9
🧹 Outside diff range and nitpick comments (4)
.cursorrules (1)
Line range hint
1-100: Consider enhancing other sections of the guidelines.While the change made is beneficial, there might be opportunities to further improve the guidelines:
- Add examples for Shadcn UI and Tailwind CSS usage to reinforce their importance.
- Provide a brief example of responsive design implementation using Tailwind's mobile-first approach.
- Consider adding a section on code organization within components (e.g., order of hooks, memoization practices).
- Include guidelines on writing unit tests, if applicable to the project.
These additions could make the guidelines even more comprehensive and valuable for developers.
apps/web/app/(app)/automation/group/CreateGroupModal.tsx (1)
156-159: Excellent improvements to the AI prompt input field.The changes to the "prompt" input field significantly enhance user understanding and usability:
- The new label "AI Prompt (Optional)" clearly indicates the field's purpose and non-mandatory nature.
- The placeholder text now provides a concise example, guiding users on what to input.
- The added explanatory text clarifies the AI's role in group creation, setting clear expectations.
These modifications align well with the PR objective of generating groups with AI prompts and improve the overall user experience.
Consider adding a character limit to the prompt input to prevent excessively long inputs. This can be implemented using the
maxLengthattribute on thetextarea.<Input type="text" as="textarea" rows={3} name="prompt" label="AI Prompt (Optional)" placeholder="Describe the group members (e.g., 'Customers', or 'Marketing emails')" explainText="Our AI will analyze your recent email history and add matching contacts or subjects to this group. If not provided, an empty group will be created." registerProps={register("prompt")} error={errors.prompt} + maxLength={500} />apps/web/app/(app)/automation/group/ViewGroup.tsx (1)
299-299: Corrected placeholder abbreviationUpdated the placeholder text from "eg." to "e.g.", properly abbreviating "exempli gratia" (for example).
apps/web/utils/actions/group.ts (1)
Line range hint
268-319: Avoid code duplication by abstracting common logic.The functions
regenerateNewsletterGroupandregenerateReceiptGroupshare similar logic for filtering new items and creating them in the database. Consider abstracting this logic into a reusable function to adhere to the DRY (Don't Repeat Yourself) principle.Example:
async function addNewGroupItems( existingItems: ExistingGroup['items'], newItems: { type: GroupItemType; value: string }[], groupId: string, ) { const itemsToAdd = newItems.filter( (newItem) => !existingItems.find( (item) => item.value === newItem.value && item.type === newItem.type, ), ); await prisma.groupItem.createMany({ data: itemsToAdd.map((item) => ({ type: item.type, value: item.value, groupId, })), }); }Then modify the regeneration functions to use this utility function.
🧰 Tools
🪛 Biome
[error] 290-290: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (16)
- .cursorrules (1 hunks)
- apps/web/app/(app)/automation/RuleForm.tsx (5 hunks)
- apps/web/app/(app)/automation/TestRules.tsx (1 hunks)
- apps/web/app/(app)/automation/create/page.tsx (1 hunks)
- apps/web/app/(app)/automation/group/CreateGroupModal.tsx (1 hunks)
- apps/web/app/(app)/automation/group/ViewGroup.tsx (7 hunks)
- apps/web/app/api/user/group/[groupId]/items/route.ts (1 hunks)
- apps/web/utils/actions/group.ts (7 hunks)
- apps/web/utils/ai/choose-rule/ai-choose-args.ts (1 hunks)
- apps/web/utils/ai/group/create-group.ts (1 hunks)
- apps/web/utils/date.ts (1 hunks)
- apps/web/utils/gmail/batch.ts (1 hunks)
- apps/web/utils/llms/index.ts (4 hunks)
- apps/web/utils/parse/cta.ts (1 hunks)
- packages/tinybird-ai-analytics/src/delete.ts (1 hunks)
- packages/tinybird/src/delete.ts (1 hunks)
✅ Files skipped from review due to trivial changes (8)
- apps/web/app/(app)/automation/RuleForm.tsx
- apps/web/app/(app)/automation/TestRules.tsx
- apps/web/utils/ai/choose-rule/ai-choose-args.ts
- apps/web/utils/date.ts
- apps/web/utils/gmail/batch.ts
- apps/web/utils/parse/cta.ts
- packages/tinybird-ai-analytics/src/delete.ts
- packages/tinybird/src/delete.ts
🧰 Additional context used
🪛 Biome
apps/web/app/(app)/automation/create/page.tsx
[error] 89-89: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
apps/web/utils/actions/group.ts
[error] 64-64: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
[error] 23-24: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 242-265: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 244-265: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
[error] 290-290: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
[error] 317-317: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
apps/web/utils/ai/group/create-group.ts
[error] 5-6: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
🔇 Additional comments (14)
apps/web/app/api/user/group/[groupId]/items/route.ts (1)
Line range hint
7-19: Update function name and consider API compatibilityThe function
getGroupItemshas been significantly changed:
The function name no longer accurately describes its behavior. Consider renaming it to
getGrouporgetGroupWithItemsto better reflect its new functionality.The return type has changed from
{ items: ... }to{ group: ... }. This is a breaking change that will affect API consumers expecting anitemsarray in the response.The
GroupItemsResponsetype (line 6) needs to be updated to reflect the new return value of the function.To address these issues, consider the following changes:
- export type GroupItemsResponse = Awaited<ReturnType<typeof getGroupItems>>; + export type GroupResponse = Awaited<ReturnType<typeof getGroup>>; - async function getGroupItems({ + async function getGroup({ userId, groupId, }: { userId: string; groupId: string; }) { const group = await prisma.group.findUnique({ where: { id: groupId, userId }, select: { prompt: true, items: true }, }); return { group }; }Also, update the GET route handler to use the new function name:
- const result = await getGroupItems({ + const result = await getGroup({ userId: session.user.id, groupId: params.groupId, });To ensure this change doesn't break existing API consumers, run the following script:
If the script returns results, those areas may need to be updated to work with the new response structure.
✅ Verification successful
Verified: No usages of the old API response structure (
items) were found in the codebase. The proposed changes will not affect existing API consumers.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usages of the old API response structure # Test: Search for code that expects 'items' in the response rg --type typescript 'items\s*[:=].*response' ./appsLength of output: 89
Script:
#!/bin/bash # Description: Check for usages of the old API response structure # Test: Search for code that expects 'items' in the response within TypeScript files rg 'items\s*[:=].*response' -g '*.ts' -g '*.tsx' ./appsLength of output: 57
.cursorrules (1)
8-8: Improved clarity in naming conventions.The update provides more specific examples for route directories and component names, which enhances the clarity of the coding guidelines. This change aligns well with common practices in Next.js and React projects, reinforcing the use of kebab case for route directories and PascalCase for components.
apps/web/app/(app)/automation/group/CreateGroupModal.tsx (2)
147-147: Improved placeholder text for better user guidance.The updated placeholder text now provides clear, specific examples of group names. This change enhances user understanding and potentially reduces input errors, aligning with best practices for form design and user experience.
159-159: Approval of optional AI prompt with a suggestion for backend verification.The removal of the
requiredattribute from the "prompt" input is consistent with the new label and provides users with more flexibility in group creation. This change allows users to create empty groups if desired, which is a good user experience improvement.To ensure this change is fully supported, please verify that the backend logic for group creation correctly handles cases where no prompt is provided. Run the following script to check for any hardcoded expectations of a prompt in the group creation logic:
✅ Verification successful
Please run the following corrected script to verify the backend logic:
Backend correctly handles optional AI prompt.
The backend validation for group creation marks the "prompt" field as optional, ensuring that creating groups without a prompt is properly supported and consistent with the frontend changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for hardcoded expectations of a prompt in group creation logic # Test: Search for group creation logic that might expect a prompt rg --type typescript -g '!*.test.ts' -g '!*.spec.ts' -A 5 -B 5 'createGroup|createGroupAction'Length of output: 131
Script:
#!/bin/bash # Description: Check for hardcoded expectations of a prompt in group creation logic # Test: Search for group creation logic that might expect a prompt rg -g "*.ts" -g "!*.test.ts" -g "!*.spec.ts" -A5 -B5 'createGroup|createGroupAction'Length of output: 4830
apps/web/utils/llms/index.ts (1)
158-158: Addition ofmaxStepsparameter enhances flexibilityAdding
maxStepsas an optional parameter tochatCompletionToolsallows for better control over the number of steps in the tool processing. Ensure that any functions or components that callchatCompletionToolsare updated if they need to specifymaxSteps.Also applies to: 166-166
apps/web/app/(app)/automation/group/ViewGroup.tsx (8)
22-22: ImportedSectionDescriptioncomponent appropriatelyThe
SectionDescriptioncomponent is now imported from@/components/Typography, which is used to display the group's prompt in the UI.
27-27: AddedregenerateGroupActionto importsThe
regenerateGroupActionfunction is correctly imported from@/utils/actions/groupfor regenerating the group.
68-68: Adjusted margin for improved layoutThe margin-top has been changed from
mt-4tomt-2, refining the spacing in the modal content for better visual alignment.
95-99: Conditionally displaying group promptThe group's prompt is now displayed using the
SectionDescriptioncomponent whendata.group.promptis available. This provides users with valuable context about the group's purpose.
111-112: Enhanced condition for 'Regenerate Group' button visibilityUpdated the condition to include
data?.group?.prompt, ensuring the 'Regenerate Group' button is displayed not only for specific group names but also when a group has a custom prompt. This increases the functionality for groups with custom prompts.
118-118: Unified group regeneration actionReplaced specific regeneration functions with the generalized
regenerateGroupAction, simplifying the codebase and avoiding redundancy.
181-181: Corrected check for group itemsChanged the check to
data?.group?.items.length, ensuring that it correctly verifies the existence of group items before rendering the table.
191-191: Iterating over group items correctlyUsing
data?.group?.items.map((item) => { ... })to iterate over group items ensures robustness by handling cases whereitemsmight be undefined, thanks to optional chaining.apps/web/utils/actions/group.ts (1)
31-31: Verify the use ofThe check
if (!session?.user.email)relies on the user's email for authentication. Ensure that this is intentional and that all necessary session validations are properly handled. Typically, the user's ID is used for authentication checks.
| prompt, | ||
| system, | ||
| schema, | ||
| experimental_telemetry: { isEnabled: true }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making experimental_telemetry configurable
The experimental_telemetry parameter is hardcoded with { isEnabled: true } in the generateObject call. If you anticipate the need to enable or disable telemetry based on different environments or user preferences, consider passing experimental_telemetry as a parameter or deriving its value from a configuration setting.
| model: llmModel, | ||
| prompt, | ||
| system, | ||
| experimental_telemetry: { isEnabled: true }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making experimental_telemetry configurable
Similarly, in the streamText call within chatCompletionStream, experimental_telemetry is set to { isEnabled: true }. To increase flexibility, you might want to make this parameter configurable or derive its value from user settings or environment configurations.
| maxSteps, | ||
| experimental_telemetry: { isEnabled: true }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure maxSteps and experimental_telemetry are appropriately handled
In the generateText call, you've added maxSteps and experimental_telemetry parameters. Verify that maxSteps is handled correctly, especially when it's undefined or exceeds expected bounds. Also, consider whether hardcoding experimental_telemetry to { isEnabled: true } meets all use cases, or if it should be configurable.
| import { chatCompletionTools } from "@/utils/llms"; | ||
| import type { Group, User } from "@prisma/client"; | ||
| import { queryBatchMessages } from "@/utils/gmail/message"; | ||
| import { UserAIFields } from "@/utils/llms/types"; |
There was a problem hiding this comment.
Use import type for type-only imports
The static analysis tool indicates that the import on line 6 is only used as a type. To optimize the code by avoiding unnecessary module loading, you should use import type for type-only imports.
Apply this diff to fix the import:
-import { UserAIFields } from "@/utils/llms/types";
+import type { UserAIFields } from "@/utils/llms/types";📝 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.
| import { UserAIFields } from "@/utils/llms/types"; | |
| import type { UserAIFields } from "@/utils/llms/types"; |
🧰 Tools
🪛 Biome
[error] 5-6: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
| const { messages } = await queryBatchMessages(gmail, accessToken, { | ||
| query, | ||
| maxResults: 20, |
There was a problem hiding this comment.
Handle potential undefined messages to prevent runtime errors
If queryBatchMessages does not return any messages, the messages variable could be undefined. Attempting to call messages.map would result in a runtime error. It's safer to provide a default empty array to ensure that messages.map always executes without errors.
Apply this diff to fix the issue:
-const { messages } = await queryBatchMessages(gmail, accessToken, {
+const { messages = [] } = await queryBatchMessages(gmail, accessToken, {📝 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.
| const { messages } = await queryBatchMessages(gmail, accessToken, { | |
| query, | |
| maxResults: 20, | |
| const { messages = [] } = await queryBatchMessages(gmail, accessToken, { | |
| query, | |
| maxResults: 20, |
| import { aiGenerateGroupItems } from "@/utils/ai/group/create-group"; | ||
| import { UserAIFields } from "@/utils/llms/types"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use import type for type-only imports to optimize the build.
The imports on lines 23-24 are only used for type annotations. Changing them to import type can help with tree-shaking and reduce the final bundle size.
Apply this diff:
-import { UserAIFields } from "@/utils/llms/types";
+import type { UserAIFields } from "@/utils/llms/types";Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Biome
[error] 23-24: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/web/utils/actions/group.ts
Outdated
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify code by removing unnecessary else clause.
Since the previous if conditions return early with a return statement, the else clause starting at line 242 is unnecessary. Removing it will reduce nesting and improve readability.
Apply this diff:
} else if (existingGroup.prompt) {
+ }
+ if (existingGroup.prompt) {
const user = await prisma.user.findUnique({
where: { id: session.user.id },
select: {
email: true,
aiModel: true,
aiProvider: true,
aiApiKey: true,
},
});
if (!user) return { error: "User not found" };
await generateGroupItemsFromPrompt(
groupId,
user,
gmail,
token.token,
existingGroup.name,
existingGroup.prompt,
);
return;
}📝 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.
| } else if (existingGroup.name === GroupName.RECEIPT) { | |
| return await regenerateReceiptGroup(existingGroup, gmail, token.token); | |
| } else if (existingGroup.prompt) { | |
| const user = await prisma.user.findUnique({ | |
| where: { id: session.user.id }, | |
| select: { | |
| email: true, | |
| aiModel: true, | |
| aiProvider: true, | |
| aiApiKey: true, | |
| }, | |
| }); | |
| if (!user) return { error: "User not found" }; | |
| await generateGroupItemsFromPrompt( | |
| groupId, | |
| user, | |
| gmail, | |
| token.token, | |
| existingGroup.name, | |
| existingGroup.prompt, | |
| ); | |
| return; | |
| } | |
| } else if (existingGroup.name === GroupName.RECEIPT) { | |
| return await regenerateReceiptGroup(existingGroup, gmail, token.token); | |
| } | |
| if (existingGroup.prompt) { | |
| const user = await prisma.user.findUnique({ | |
| where: { id: session.user.id }, | |
| select: { | |
| email: true, | |
| aiModel: true, | |
| aiProvider: true, | |
| aiApiKey: true, | |
| }, | |
| }); | |
| if (!user) return { error: "User not found" }; | |
| await generateGroupItemsFromPrompt( | |
| groupId, | |
| user, | |
| gmail, | |
| token.token, | |
| existingGroup.name, | |
| existingGroup.prompt, | |
| ); | |
| return; | |
| } |
🧰 Tools
🪛 Biome
[error] 242-265: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 244-265: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
apps/web/utils/actions/group.ts
Outdated
There was a problem hiding this comment.
Handle potential errors when generating group items from prompt.
Within the if (prompt) block, consider wrapping the call to generateGroupItemsFromPrompt in a try-catch statement to handle any exceptions that may arise during the AI generation or database operations. This will improve error handling and provide more informative feedback to the user.
Apply this diff:
if (prompt) {
+ try {
const gmail = getGmailClient(session);
const token = await getGmailAccessToken(session);
const user = await prisma.user.findUnique({
where: { id: session.user.id },
select: {
email: true,
aiModel: true,
aiProvider: true,
aiApiKey: true,
},
});
if (!user) return { error: "User not found" };
if (!token.token) return { error: "No access token" };
await generateGroupItemsFromPrompt(
group.id,
user,
gmail,
token.token,
name,
prompt,
);
+ } catch (error) {
+ console.error("Error generating group items", error);
+ captureException(
+ error,
+ { extra: { groupId: group.id, name, prompt } },
+ session?.user.email,
+ );
+ return { error: "Error generating group items" };
+ }
}📝 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.
| if (prompt) { | |
| const gmail = getGmailClient(session); | |
| const token = await getGmailAccessToken(session); | |
| const user = await prisma.user.findUnique({ | |
| where: { id: session.user.id }, | |
| select: { | |
| email: true, | |
| aiModel: true, | |
| aiProvider: true, | |
| aiApiKey: true, | |
| }, | |
| }); | |
| if (!user) return { error: "User not found" }; | |
| if (!token.token) return { error: "No access token" }; | |
| await generateGroupItemsFromPrompt( | |
| group.id, | |
| user, | |
| gmail, | |
| token.token, | |
| name, | |
| prompt, | |
| ); | |
| } | |
| if (prompt) { | |
| try { | |
| const gmail = getGmailClient(session); | |
| const token = await getGmailAccessToken(session); | |
| const user = await prisma.user.findUnique({ | |
| where: { id: session.user.id }, | |
| select: { | |
| email: true, | |
| aiModel: true, | |
| aiProvider: true, | |
| aiApiKey: true, | |
| }, | |
| }); | |
| if (!user) return { error: "User not found" }; | |
| if (!token.token) return { error: "No access token" }; | |
| await generateGroupItemsFromPrompt( | |
| group.id, | |
| user, | |
| gmail, | |
| token.token, | |
| name, | |
| prompt, | |
| ); | |
| } catch (error) { | |
| console.error("Error generating group items", error); | |
| captureException( | |
| error, | |
| { extra: { groupId: group.id, name, prompt } }, | |
| session?.user.email, | |
| ); | |
| return { error: "Error generating group items" }; | |
| } | |
| } |
apps/web/utils/actions/group.ts
Outdated
There was a problem hiding this comment.
Handle potential errors during group regeneration with prompt.
Within the block that regenerates group items based on the prompt, consider adding error handling to catch any exceptions from database queries or AI generation, ensuring that the application can gracefully handle failures.
Apply this diff:
if (existingGroup.prompt) {
+ try {
const user = await prisma.user.findUnique({
where: { id: session.user.id },
select: {
email: true,
aiModel: true,
aiProvider: true,
aiApiKey: true,
},
});
if (!user) return { error: "User not found" };
await generateGroupItemsFromPrompt(
groupId,
user,
gmail,
token.token,
existingGroup.name,
existingGroup.prompt,
);
+ } catch (error) {
+ console.error("Error regenerating group items", error);
+ captureException(
+ error,
+ { extra: { groupId, groupName: existingGroup.name } },
+ session?.user.email,
+ );
+ return { error: "Error regenerating group items" };
+ }
return;
}📝 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.
| const user = await prisma.user.findUnique({ | |
| where: { id: session.user.id }, | |
| select: { | |
| email: true, | |
| aiModel: true, | |
| aiProvider: true, | |
| aiApiKey: true, | |
| }, | |
| }); | |
| if (!user) return { error: "User not found" }; | |
| await generateGroupItemsFromPrompt( | |
| groupId, | |
| user, | |
| gmail, | |
| token.token, | |
| existingGroup.name, | |
| existingGroup.prompt, | |
| ); | |
| return; | |
| } | |
| if (existingGroup.prompt) { | |
| try { | |
| const user = await prisma.user.findUnique({ | |
| where: { id: session.user.id }, | |
| select: { | |
| email: true, | |
| aiModel: true, | |
| aiProvider: true, | |
| aiApiKey: true, | |
| }, | |
| }); | |
| if (!user) return { error: "User not found" }; | |
| await generateGroupItemsFromPrompt( | |
| groupId, | |
| user, | |
| gmail, | |
| token.token, | |
| existingGroup.name, | |
| existingGroup.prompt, | |
| ); | |
| } catch (error) { | |
| console.error("Error regenerating group items", error); | |
| captureException( | |
| error, | |
| { extra: { groupId, groupName: existingGroup.name } }, | |
| session?.user.email, | |
| ); | |
| return { error: "Error regenerating group items" }; | |
| } | |
| return; | |
| } |
🧰 Tools
🪛 Biome
[error] 242-265: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 244-265: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
apps/web/app/(app)/automation/create/page.tsx (1)
89-89: Approve grammatical correction and suggest syntax improvementThe grammatical correction from "eg." to "e.g." is appropriate and improves the text's correctness. However, the use of template literals is unnecessary here as there's no string interpolation.
Consider simplifying the code by using a regular string literal:
- placeholder={`e.g. Forward receipts to alice@accountant.com.`} + placeholder="e.g. Forward receipts to alice@accountant.com."This change addresses the static analysis warning and slightly improves code readability.
🧰 Tools
🪛 Biome
[error] 89-89: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- .cursorrules (1 hunks)
- apps/web/app/(app)/automation/create/page.tsx (1 hunks)
- apps/web/utils/actions/group.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .cursorrules
🧰 Additional context used
🪛 Biome
apps/web/app/(app)/automation/create/page.tsx
[error] 89-89: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
apps/web/utils/actions/group.ts
[error] 65-65: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
[error] 23-24: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 174-174: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
[error] 207-207: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
[error] 246-269: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 249-269: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
[error] 295-295: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
[error] 322-322: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
🔇 Additional comments (3)
apps/web/utils/actions/group.ts (3)
243-269: Simplify code by removing unnecessaryelseclausesAs previously noted, the
elseclause starting at line 249 is unnecessary because the previousifconditions return early with areturnstatement. Removing theelseclause will reduce nesting and improve readability.Apply this diff:
} else if (existingGroup.name === GroupName.RECEIPT) { await regenerateReceiptGroup(existingGroup, gmail, token.token); return; - } else if (existingGroup.prompt) { + } + if (existingGroup.prompt) { const user = await prisma.user.findUnique({ where: { id: session.user.id }, select: { email: true, aiModel: true, aiProvider: true, aiApiKey: true, }, }); if (!user) return { error: "User not found" }; await generateGroupItemsFromPrompt( groupId, user, gmail, token.token, existingGroup.name, existingGroup.prompt, ); }🧰 Tools
🪛 Biome
[error] 246-269: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 249-269: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
39-63: Add error handling when generating group items from promptAs previously suggested, consider wrapping the call to
generateGroupItemsFromPromptin atry-catchblock to handle any exceptions that may arise during AI generation or database operations. This will improve the robustness of your application and provide more informative feedback to the user.Apply this diff:
if (prompt) { + try { const gmail = getGmailClient(session); const token = await getGmailAccessToken(session); const user = await prisma.user.findUnique({ where: { id: session.user.id }, select: { email: true, aiModel: true, aiProvider: true, aiApiKey: true, }, }); if (!user) return { error: "User not found" }; if (!token.token) return { error: "No access token" }; await generateGroupItemsFromPrompt( group.id, user, gmail, token.token, name, prompt, ); + } catch (error) { + console.error("Error generating group items", error); + captureException(error, { extra: { groupId: group.id, name, prompt } }, session?.user.email); + return { error: "Error generating group items" }; + } }
250-269: Add error handling during group regeneration with promptIt's recommended to handle potential errors when regenerating group items based on a prompt. Wrapping the call to
generateGroupItemsFromPromptin atry-catchblock will help manage exceptions and maintain application stability.Apply this diff:
} else if (existingGroup.prompt) { + try { const user = await prisma.user.findUnique({ where: { id: session.user.id }, select: { email: true, aiModel: true, aiProvider: true, aiApiKey: true, }, }); if (!user) return { error: "User not found" }; await generateGroupItemsFromPrompt( groupId, user, gmail, token.token, existingGroup.name, existingGroup.prompt, ); + } catch (error) { + console.error("Error regenerating group items", error); + captureException(error, { extra: { groupId, groupName: existingGroup.name } }, session?.user.email); + return { error: "Error regenerating group items" }; + } }🧰 Tools
🪛 Biome
[error] 246-269: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 249-269: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
apps/web/utils/actions/group.ts
Outdated
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace unnecessary template literals with string literals
At lines 65, 174, 207, 295, and 322, template literals are used without any interpolation or special characters. To improve readability and potentially optimize performance, consider replacing them with regular string literals.
Apply this diff to make the changes:
- revalidatePath(`/automation`);
+ revalidatePath('/automation');Also applies to: 174-174, 207-207, 295-295, 322-322
🧰 Tools
🪛 Biome
[error] 65-65: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
| import { findReceipts } from "@/utils/ai/group/find-receipts"; | ||
| import { getGmailClient, getGmailAccessToken } from "@/utils/gmail/client"; | ||
| import { GroupItemType } from "@prisma/client"; | ||
| import { GroupItemType, Prisma, User } from "@prisma/client"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use import type for type-only imports
The imports on line 16 are only used for type annotations. Changing them to import type can help with tree-shaking and reduce the final bundle size.
Apply this diff:
-import { GroupItemType, Prisma, User } from "@prisma/client";
+import { GroupItemType } from "@prisma/client";
+import type { Prisma, User } from "@prisma/client";📝 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.
| import { GroupItemType, Prisma, User } from "@prisma/client"; | |
| import { GroupItemType } from "@prisma/client"; | |
| import type { Prisma, User } from "@prisma/client"; |
🧰 Tools
🪛 Biome
[error] 15-16: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
| const result = await aiGenerateGroupItems(user, gmail, token, { | ||
| name, | ||
| prompt, | ||
| }); | ||
|
|
||
| await prisma.$transaction([ | ||
| ...result.senders.map((sender) => | ||
| prisma.groupItem.upsert({ | ||
| where: { | ||
| groupId_type_value: { | ||
| groupId, | ||
| type: GroupItemType.FROM, | ||
| value: sender, | ||
| }, | ||
| }, | ||
| update: {}, // No update needed if it exists | ||
| create: { | ||
| type: GroupItemType.FROM, | ||
| value: sender, | ||
| groupId, | ||
| }, | ||
| }), | ||
| ), | ||
| ...result.subjects.map((subject) => | ||
| prisma.groupItem.upsert({ | ||
| where: { | ||
| groupId_type_value: { | ||
| groupId, | ||
| type: GroupItemType.SUBJECT, | ||
| value: subject, | ||
| }, | ||
| }, | ||
| update: {}, // No update needed if it exists | ||
| create: { | ||
| type: GroupItemType.SUBJECT, | ||
| value: subject, | ||
| groupId, | ||
| }, | ||
| }), | ||
| ), | ||
| ]); | ||
| } |
There was a problem hiding this comment.
Add error handling in generateGroupItemsFromPrompt
The function generateGroupItemsFromPrompt performs asynchronous operations that may fail, such as calls to aiGenerateGroupItems and database transactions. Consider wrapping these operations in a try-catch block to handle potential errors gracefully and provide useful feedback.
Apply this diff:
async function generateGroupItemsFromPrompt(
groupId: string,
user: Pick<User, "email"> & UserAIFields,
gmail: gmail_v1.Gmail,
token: string,
name: string,
prompt: string,
) {
+ try {
const result = await aiGenerateGroupItems(user, gmail, token, {
name,
prompt,
});
await prisma.$transaction([
// ...transaction operations
]);
+ } catch (error) {
+ console.error("Error generating group items from prompt", error);
+ captureException(error, { extra: { groupId, name, prompt } }, user.email);
+ throw new Error("Failed to generate group items from prompt");
+ }
}📝 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.
| const result = await aiGenerateGroupItems(user, gmail, token, { | |
| name, | |
| prompt, | |
| }); | |
| await prisma.$transaction([ | |
| ...result.senders.map((sender) => | |
| prisma.groupItem.upsert({ | |
| where: { | |
| groupId_type_value: { | |
| groupId, | |
| type: GroupItemType.FROM, | |
| value: sender, | |
| }, | |
| }, | |
| update: {}, // No update needed if it exists | |
| create: { | |
| type: GroupItemType.FROM, | |
| value: sender, | |
| groupId, | |
| }, | |
| }), | |
| ), | |
| ...result.subjects.map((subject) => | |
| prisma.groupItem.upsert({ | |
| where: { | |
| groupId_type_value: { | |
| groupId, | |
| type: GroupItemType.SUBJECT, | |
| value: subject, | |
| }, | |
| }, | |
| update: {}, // No update needed if it exists | |
| create: { | |
| type: GroupItemType.SUBJECT, | |
| value: subject, | |
| groupId, | |
| }, | |
| }), | |
| ), | |
| ]); | |
| } | |
| async function generateGroupItemsFromPrompt( | |
| groupId: string, | |
| user: Pick<User, "email"> & UserAIFields, | |
| gmail: gmail_v1.Gmail, | |
| token: string, | |
| name: string, | |
| prompt: string, | |
| ) { | |
| try { | |
| const result = await aiGenerateGroupItems(user, gmail, token, { | |
| name, | |
| prompt, | |
| }); | |
| await prisma.$transaction([ | |
| ...result.senders.map((sender) => | |
| prisma.groupItem.upsert({ | |
| where: { | |
| groupId_type_value: { | |
| groupId, | |
| type: GroupItemType.FROM, | |
| value: sender, | |
| }, | |
| }, | |
| update: {}, // No update needed if it exists | |
| create: { | |
| type: GroupItemType.FROM, | |
| value: sender, | |
| groupId, | |
| }, | |
| }), | |
| ), | |
| ...result.subjects.map((subject) => | |
| prisma.groupItem.upsert({ | |
| where: { | |
| groupId_type_value: { | |
| groupId, | |
| type: GroupItemType.SUBJECT, | |
| value: subject, | |
| }, | |
| }, | |
| update: {}, // No update needed if it exists | |
| create: { | |
| type: GroupItemType.SUBJECT, | |
| value: subject, | |
| groupId, | |
| }, | |
| }), | |
| ), | |
| ]); | |
| } catch (error) { | |
| console.error("Error generating group items from prompt", error); | |
| captureException(error, { extra: { groupId, name, prompt } }, user.email); | |
| throw new Error("Failed to generate group items from prompt"); | |
| } | |
| } |
apps/web/utils/actions/group.ts
Outdated
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent Session User Property Checks Detected
The verification process revealed that session?.user.id and session?.user.email are used interchangeably across multiple files for session validation. This inconsistency can lead to potential authentication issues.
Affected Files:
apps/web/utils/actions/group.ts(uses bothidandemail)apps/web/utils/actions/user.ts(mix ofidandemail)apps/web/utils/actions/permissions.tsapps/web/utils/actions/premium.tsapps/web/app/api/user/settings/*.ts- Several other files as listed in the search results
Recommendation:
Standardize the session validation logic by consistently using a single user property (id or email) across the entire codebase to ensure uniform authentication behavior.
🔗 Analysis chain
Consistent session user property checks
In line 145, you're checking for session?.user.id, whereas in line 32, the check is for session?.user.email. Ensure consistency in your session validation logic to prevent potential authentication issues.
Run the following script to verify all session user property checks:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistency of session user property checks across the codebase.
# Test: Search for session user property checks. Expect: Consistent use of 'session?.user.id' or 'session?.user.email'.
rg --type js --type ts -A 1 -B 1 $'if (!session\\?\\.user\\.(id|email)) return \\{ error: "Not logged in" \\};'
Length of output: 107
Script:
#!/bin/bash
# Description: List all session user property checks to ensure consistency.
# Search for any checks on session?.user properties
rg 'session\?\.user\.(id|email)' --type js --type ts -A 2 -B 2
Length of output: 40440
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (9)
apps/web/__tests__/ai-create-group.test.ts (3)
12-27: Test suite and case setup looks goodThe test suite and case are well-structured with a clear description. The increased timeout for the AI call is a good practice.
For future improvements, consider:
- Adding more test cases to cover different scenarios (e.g., empty results, error handling).
- Using a
beforeEachblock to set up common mock data and reduce duplication if more tests are added.
29-57: Mock data and function setup is comprehensiveThe mock messages and function setup provide a good representation of real-world scenarios, including both relevant and irrelevant emails.
Consider adding a mock message with multiple recipients or CC'd addresses to test more complex email scenarios. This could help ensure the group generation handles various email structures correctly.
59-67: Test execution and assertions are solid, with room for enhancementThe current assertions effectively verify the basic functionality of
aiGenerateGroupItems.Consider adding the following assertions to make the test more robust:
- Check that the result doesn't include senders from external domains.
- Verify that the subjects array doesn't include irrelevant topics.
- Assert on the exact length of the
sendersandsubjectsarrays to ensure no unexpected items are included.Example:
expect(result.senders).not.toContain("@external.com"); expect(result.subjects).not.toContain("Industry News Digest"); expect(result.senders).toHaveLength(1); expect(result.subjects).toHaveLength(2);apps/web/utils/actions/group.ts (2)
32-32: Improved group creation with prompt handlingThe changes to
createGroupActionare well-implemented. The addition of prompt-based group item generation and improved error handling enhance the function's capabilities and robustness.Consider standardizing the error handling approach:
if (!user) return { error: "User not found" }; if (!token.token) return { error: "No access token" }; +if (isDuplicateError(error, "name")) + return { error: "Group with this name already exists" }; + +console.error("Error creating group", error); +captureException(error, { extra: { name, prompt } }, session?.user.email); -return { error: "Error creating group" }; +return { error: "An unexpected error occurred while creating the group" };This change provides more specific error messages and maintains consistency with the error handling style used earlier in the function.
Also applies to: 39-63, 67-71
220-277: Improved group regeneration with comprehensive handlingThe new
ExistingGrouptype and the modifications toregenerateGroupActionare well-implemented. They improve type safety and provide more comprehensive handling of different group types.To reduce code duplication in error handling, consider extracting the user retrieval and error checking into a separate function:
async function getUserForAI(userId: string) { const user = await prisma.user.findUnique({ where: { id: userId }, select: { email: true, aiModel: true, aiProvider: true, aiApiKey: true, }, }); if (!user) throw new Error("User not found"); return user; } // In regenerateGroupAction: const user = await getUserForAI(session.user.id).catch(() => ({ error: "User not found" })); if ('error' in user) return user; await generateGroupItemsFromPrompt( groupId, user, gmail, token.token, existingGroup.name, existingGroup.prompt, );This change will make the code more maintainable and consistent across different functions that need to retrieve user data for AI operations.
apps/web/utils/ai/group/create-group.ts (4)
15-16: Improve schema description for clarityReplace "like" with "such as" and adjust punctuation for better readability.
Apply this diff:
- "The senders in the group. Can also be part of the sender name like 'John Smith' or 'Acme Corp' or '@acme.com'.", + "The senders in the group. Can also be part of the sender name, such as 'John Smith', 'Acme Corp', or '@acme.com'.",
20-21: Improve schema description for claritySimilarly, update the description for
subjects.Apply this diff:
- "The subjects in the group. Can also be part of the subject line like 'meeting' or 'reminder' or 'invoice #'.", + "The subjects in the group. Can also be part of the subject line, such as 'meeting', 'reminder', or 'invoice #'.",
58-58: Use a logging library instead ofconsole.logReplace
console.logwith a proper logging mechanism to better manage log levels and outputs.Apply this diff:
- console.log(`aiGenerateGroupItems: ${group.name} - ${group.prompt}`); + // Use a logger instead of console.log + logger.info(`aiGenerateGroupItems: ${group.name} - ${group.prompt}`);
125-125: Use a logging library instead ofconsole.logSimilarly, replace
console.logwith a proper logging mechanism.Apply this diff:
- console.log(`verifyGroupItems: ${group.name} - ${group.prompt}`); + // Use a logger instead of console.log + logger.info(`verifyGroupItems: ${group.name} - ${group.prompt}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- apps/web/tests/ai-create-group.test.ts (1 hunks)
- apps/web/utils/actions/group.ts (9 hunks)
- apps/web/utils/ai/group/create-group.ts (1 hunks)
🧰 Additional context used
🪛 Biome
apps/web/__tests__/ai-create-group.test.ts
[error] 1-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 4-5: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/web/utils/actions/group.ts
[error] 23-24: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/web/utils/ai/group/create-group.ts
[error] 5-6: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
🔇 Additional comments (3)
apps/web/__tests__/ai-create-group.test.ts (1)
1-68: Overall, excellent test implementation with minor enhancement opportunitiesThis test file for
aiGenerateGroupItemsis well-structured and effectively verifies the core functionality. It demonstrates good practices such as proper mocking, realistic test data, and clear assertions.To further elevate the quality of this test suite:
- Optimize imports using
import typefor type-only imports.- Consider adding more test cases to cover edge cases and error scenarios.
- Enhance assertions to verify the exclusion of irrelevant data and exact result lengths.
These enhancements will contribute to a more robust and comprehensive test suite, ensuring the reliability of the
aiGenerateGroupItemsfunction across various scenarios.🧰 Tools
🪛 Biome
[error] 1-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 4-5: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/web/utils/actions/group.ts (2)
162-164: Improved error handling in group creation actionsThe addition of access token checks in
createNewsletterGroupActionandcreateReceiptGroupActionis a good improvement. This ensures that the functions fail early if the required access token is not available, preventing potential issues later in the execution.Also applies to: 201-203
341-341: Consistent path revalidation in group-related actionsThe addition of
revalidatePath("/automation")indeleteGroupAction,addGroupItemAction, anddeleteGroupItemActionis a good change. This ensures that the UI is updated consistently after any modifications to groups or group items.Also applies to: 360-360, 374-374
| import { describe, it, expect, vi } from "vitest"; | ||
| import { gmail_v1 } from "@googleapis/gmail"; | ||
| import { aiGenerateGroupItems } from "@/utils/ai/group/create-group"; | ||
| import { queryBatchMessages } from "@/utils/gmail/message"; | ||
| import { ParsedMessage } from "@/utils/types"; | ||
|
|
||
| vi.mock("server-only", () => ({})); | ||
| vi.mock("@/utils/gmail/message", () => ({ | ||
| queryBatchMessages: vi.fn(), | ||
| })); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize imports using import type for type-only imports
To ensure that type imports are removed during transpilation and avoid loading unnecessary modules, consider using import type for the following imports:
-import { gmail_v1 } from "@googleapis/gmail";
+import type { gmail_v1 } from "@googleapis/gmail";
-import { ParsedMessage } from "@/utils/types";
+import type { ParsedMessage } from "@/utils/types";This change aligns with best practices for TypeScript and can potentially improve build performance.
📝 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.
| import { describe, it, expect, vi } from "vitest"; | |
| import { gmail_v1 } from "@googleapis/gmail"; | |
| import { aiGenerateGroupItems } from "@/utils/ai/group/create-group"; | |
| import { queryBatchMessages } from "@/utils/gmail/message"; | |
| import { ParsedMessage } from "@/utils/types"; | |
| vi.mock("server-only", () => ({})); | |
| vi.mock("@/utils/gmail/message", () => ({ | |
| queryBatchMessages: vi.fn(), | |
| })); | |
| import { describe, it, expect, vi } from "vitest"; | |
| import type { gmail_v1 } from "@googleapis/gmail"; | |
| import { aiGenerateGroupItems } from "@/utils/ai/group/create-group"; | |
| import { queryBatchMessages } from "@/utils/gmail/message"; | |
| import type { ParsedMessage } from "@/utils/types"; | |
| vi.mock("server-only", () => ({})); | |
| vi.mock("@/utils/gmail/message", () => ({ | |
| queryBatchMessages: vi.fn(), | |
| })); |
🧰 Tools
🪛 Biome
[error] 1-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 4-5: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
|
|
||
| import { revalidatePath } from "next/cache"; | ||
| import prisma from "@/utils/prisma"; | ||
| import type { gmail_v1 } from "@googleapis/gmail"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize imports for better tree-shaking
Consider using import type for type-only imports to improve tree-shaking and potentially reduce bundle size. This applies to the following imports:
-import type { gmail_v1 } from "@googleapis/gmail";
+import { gmail_v1 } from "@googleapis/gmail";
-import { GroupItemType, Prisma, User } from "@prisma/client";
+import { GroupItemType } from "@prisma/client";
+import type { Prisma, User } from "@prisma/client";
-import { aiGenerateGroupItems } from "@/utils/ai/group/create-group";
-import { UserAIFields } from "@/utils/llms/types";
+import { aiGenerateGroupItems } from "@/utils/ai/group/create-group";
+import type { UserAIFields } from "@/utils/llms/types";Also applies to: 16-16, 23-24
| async function regenerateNewsletterGroup( | ||
| existingGroup: ExistingGroup, | ||
| gmail: gmail_v1.Gmail, | ||
| token: string, | ||
| ) { | ||
| const newsletters = await findNewsletters(gmail, token); | ||
|
|
||
| const newItems = newsletters.filter( | ||
| (newItem) => | ||
| !existingGroup.items.find( | ||
| (item) => item.value === newItem && item.type === GroupItemType.FROM, | ||
| ), | ||
| ); | ||
|
|
||
| await prisma.groupItem.createMany({ | ||
| data: newItems.map((item) => ({ | ||
| type: GroupItemType.FROM, | ||
| value: item, | ||
| groupId: existingGroup.id, | ||
| })), | ||
| }); | ||
|
|
||
| revalidatePath("/automation"); | ||
| } | ||
|
|
||
| async function regenerateReceiptGroup( | ||
| existingGroup: ExistingGroup, | ||
| gmail: gmail_v1.Gmail, | ||
| token: string, | ||
| ) { | ||
| const receipts = await findReceipts(gmail, token); | ||
|
|
||
| const newItems = receipts.filter( | ||
| (newItem) => | ||
| !existingGroup.items.find( | ||
| (item) => item.value === newItem.value && item.type === newItem.type, | ||
| ), | ||
| ); | ||
|
|
||
| await prisma.groupItem.createMany({ | ||
| data: newItems.map((item) => ({ | ||
| type: GroupItemType.FROM, | ||
| value: item.value, | ||
| groupId: existingGroup.id, | ||
| })), | ||
| }); | ||
|
|
||
| revalidatePath("/automation"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize and enhance error handling in group regeneration functions
The new regenerateNewsletterGroup and regenerateReceiptGroup functions are good additions. Consider the following improvements:
- Add error handling:
try {
// Existing function body
} catch (error) {
console.error(`Error regenerating ${existingGroup.name} group`, error);
captureException(error, { extra: { groupId: existingGroup.id } });
throw new Error(`Failed to regenerate ${existingGroup.name} group`);
}- Optimize database operations:
const newItems = receipts.filter(
(newItem) => !existingGroup.items.some(
(item) => item.value === newItem.value && item.type === newItem.type
)
);
if (newItems.length > 0) {
await prisma.groupItem.createMany({
data: newItems.map((item) => ({
type: item.type,
value: item.value,
groupId: existingGroup.id,
})),
skipDuplicates: true,
});
revalidatePath("/automation");
}These changes will improve error handling and potentially reduce unnecessary database operations and path revalidations.
| async function generateGroupItemsFromPrompt( | ||
| groupId: string, | ||
| user: Pick<User, "email"> & UserAIFields, | ||
| gmail: gmail_v1.Gmail, | ||
| token: string, | ||
| name: string, | ||
| prompt: string, | ||
| ) { | ||
| console.log(`generateGroupItemsFromPrompt: ${name} - ${prompt}`); | ||
|
|
||
| const result = await aiGenerateGroupItems(user, gmail, token, { | ||
| name, | ||
| prompt, | ||
| }); | ||
|
|
||
| console.log( | ||
| `generateGroupItemsFromPrompt result. Senders: ${result.senders.length}, Subjects: ${result.subjects.length}`, | ||
| ); | ||
|
|
||
| await prisma.$transaction([ | ||
| ...result.senders.map((sender) => | ||
| prisma.groupItem.upsert({ | ||
| where: { | ||
| groupId_type_value: { | ||
| groupId, | ||
| type: GroupItemType.FROM, | ||
| value: sender, | ||
| }, | ||
| }, | ||
| update: {}, // No update needed if it exists | ||
| create: { | ||
| type: GroupItemType.FROM, | ||
| value: sender, | ||
| groupId, | ||
| }, | ||
| }), | ||
| ), | ||
| ...result.subjects.map((subject) => | ||
| prisma.groupItem.upsert({ | ||
| where: { | ||
| groupId_type_value: { | ||
| groupId, | ||
| type: GroupItemType.SUBJECT, | ||
| value: subject, | ||
| }, | ||
| }, | ||
| update: {}, // No update needed if it exists | ||
| create: { | ||
| type: GroupItemType.SUBJECT, | ||
| value: subject, | ||
| groupId, | ||
| }, | ||
| }), | ||
| ), | ||
| ]); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance generateGroupItemsFromPrompt with error handling and performance optimizations
The new generateGroupItemsFromPrompt function is a great addition. Consider the following improvements:
- Add error handling:
+try {
const result = await aiGenerateGroupItems(user, gmail, token, {
name,
prompt,
});
// ... rest of the function
+} catch (error) {
+ console.error("Error generating group items from prompt", error);
+ captureException(error, { extra: { groupId, name, prompt } }, user.email);
+ throw new Error("Failed to generate group items from prompt");
+}- Optimize database operations:
-await prisma.$transaction([
+await prisma.$transaction(async (tx) => {
+ const existingItems = await tx.groupItem.findMany({
+ where: { groupId },
+ select: { type: true, value: true },
+ });
+
+ const newSenders = result.senders.filter(
+ (sender) => !existingItems.some(item => item.type === GroupItemType.FROM && item.value === sender)
+ );
+ const newSubjects = result.subjects.filter(
+ (subject) => !existingItems.some(item => item.type === GroupItemType.SUBJECT && item.value === subject)
+ );
+
+ await tx.groupItem.createMany({
+ data: [
+ ...newSenders.map(sender => ({ type: GroupItemType.FROM, value: sender, groupId })),
+ ...newSubjects.map(subject => ({ type: GroupItemType.SUBJECT, value: subject, groupId })),
+ ],
+ skipDuplicates: true,
+ });
+});These changes will improve error handling and reduce the number of database queries, potentially improving performance.
📝 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.
| async function generateGroupItemsFromPrompt( | |
| groupId: string, | |
| user: Pick<User, "email"> & UserAIFields, | |
| gmail: gmail_v1.Gmail, | |
| token: string, | |
| name: string, | |
| prompt: string, | |
| ) { | |
| console.log(`generateGroupItemsFromPrompt: ${name} - ${prompt}`); | |
| const result = await aiGenerateGroupItems(user, gmail, token, { | |
| name, | |
| prompt, | |
| }); | |
| console.log( | |
| `generateGroupItemsFromPrompt result. Senders: ${result.senders.length}, Subjects: ${result.subjects.length}`, | |
| ); | |
| await prisma.$transaction([ | |
| ...result.senders.map((sender) => | |
| prisma.groupItem.upsert({ | |
| where: { | |
| groupId_type_value: { | |
| groupId, | |
| type: GroupItemType.FROM, | |
| value: sender, | |
| }, | |
| }, | |
| update: {}, // No update needed if it exists | |
| create: { | |
| type: GroupItemType.FROM, | |
| value: sender, | |
| groupId, | |
| }, | |
| }), | |
| ), | |
| ...result.subjects.map((subject) => | |
| prisma.groupItem.upsert({ | |
| where: { | |
| groupId_type_value: { | |
| groupId, | |
| type: GroupItemType.SUBJECT, | |
| value: subject, | |
| }, | |
| }, | |
| update: {}, // No update needed if it exists | |
| create: { | |
| type: GroupItemType.SUBJECT, | |
| value: subject, | |
| groupId, | |
| }, | |
| }), | |
| ), | |
| ]); | |
| } | |
| async function generateGroupItemsFromPrompt( | |
| groupId: string, | |
| user: Pick<User, "email"> & UserAIFields, | |
| gmail: gmail_v1.Gmail, | |
| token: string, | |
| name: string, | |
| prompt: string, | |
| ) { | |
| console.log(`generateGroupItemsFromPrompt: ${name} - ${prompt}`); | |
| try { | |
| const result = await aiGenerateGroupItems(user, gmail, token, { | |
| name, | |
| prompt, | |
| }); | |
| console.log( | |
| `generateGroupItemsFromPrompt result. Senders: ${result.senders.length}, Subjects: ${result.subjects.length}`, | |
| ); | |
| await prisma.$transaction(async (tx) => { | |
| const existingItems = await tx.groupItem.findMany({ | |
| where: { groupId }, | |
| select: { type: true, value: true }, | |
| }); | |
| const newSenders = result.senders.filter( | |
| (sender) => !existingItems.some(item => item.type === GroupItemType.FROM && item.value === sender) | |
| ); | |
| const newSubjects = result.subjects.filter( | |
| (subject) => !existingItems.some(item => item.type === GroupItemType.SUBJECT && item.value === subject) | |
| ); | |
| await tx.groupItem.createMany({ | |
| data: [ | |
| ...newSenders.map(sender => ({ type: GroupItemType.FROM, value: sender, groupId })), | |
| ...newSubjects.map(subject => ({ type: GroupItemType.SUBJECT, value: subject, groupId })), | |
| ], | |
| skipDuplicates: true, | |
| }); | |
| }); | |
| } catch (error) { | |
| console.error("Error generating group items from prompt", error); | |
| captureException(error, { extra: { groupId, name, prompt } }, user.email); | |
| throw new Error("Failed to generate group items from prompt"); | |
| } | |
| } |
| (acc, { args }) => { | ||
| const typedArgs = args as z.infer<typeof generateGroupItemsSchema>; | ||
| return { | ||
| senders: [...acc.senders, ...typedArgs.senders], | ||
| subjects: [...acc.subjects, ...typedArgs.subjects], | ||
| }; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize array concatenation in reduce function
Using array spreads inside a reduce function can be inefficient due to creating new arrays on each iteration. Consider mutating the accumulator directly with Array.prototype.push to improve performance.
Apply this diff:
(acc, { args }) => {
const typedArgs = args as z.infer<typeof generateGroupItemsSchema>;
- return {
- senders: [...acc.senders, ...typedArgs.senders],
- subjects: [...acc.subjects, ...typedArgs.subjects],
- };
+ acc.senders.push(...typedArgs.senders);
+ acc.subjects.push(...typedArgs.subjects);
+ return acc;
},📝 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.
| (acc, { args }) => { | |
| const typedArgs = args as z.infer<typeof generateGroupItemsSchema>; | |
| return { | |
| senders: [...acc.senders, ...typedArgs.senders], | |
| subjects: [...acc.subjects, ...typedArgs.subjects], | |
| }; | |
| }, | |
| (acc, { args }) => { | |
| const typedArgs = args as z.infer<typeof generateGroupItemsSchema>; | |
| acc.senders.push(...typedArgs.senders); | |
| acc.subjects.push(...typedArgs.subjects); | |
| return acc; | |
| }, |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- apps/web/utils/ai/group/create-group.ts (1 hunks)
🧰 Additional context used
🪛 Biome
apps/web/utils/ai/group/create-group.ts
[error] 5-6: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
| accessToken: string, | ||
| group: Pick<Group, "name" | "prompt">, | ||
| ): Promise<z.infer<typeof generateGroupItemsSchema>> { | ||
| console.log(`aiGenerateGroupItems: ${group.name} - ${group.prompt}`); |
There was a problem hiding this comment.
Avoid logging sensitive user data to console
The console.log statement on line 57 logs group.prompt, which may contain sensitive user-provided information. It's recommended to avoid logging sensitive data to protect user privacy.
Consider removing or sanitizing the log statement:
- console.log(`aiGenerateGroupItems: ${group.name} - ${group.prompt}`);
+ console.log(`aiGenerateGroupItems: ${group.name}`);📝 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.
| console.log(`aiGenerateGroupItems: ${group.name} - ${group.prompt}`); | |
| console.log(`aiGenerateGroupItems: ${group.name}`); |
| You categorise emailers into groups. | ||
| initialItems: z.infer<typeof generateGroupItemsSchema>, | ||
| ): Promise<z.infer<typeof generateGroupItemsSchema>> { | ||
| console.log(`verifyGroupItems: ${group.name} - ${group.prompt}`); |
There was a problem hiding this comment.
Avoid logging sensitive user data to console
The console.log statement on line 125 logs group.prompt, which may contain sensitive user-provided information. It's advisable to avoid logging sensitive data to protect user privacy.
Consider removing or modifying the log statement:
- console.log(`verifyGroupItems: ${group.name} - ${group.prompt}`);
+ console.log(`verifyGroupItems: ${group.name}`);📝 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.
| console.log(`verifyGroupItems: ${group.name} - ${group.prompt}`); | |
| console.log(`verifyGroupItems: ${group.name}`); |
| senders: initialItems.senders.filter( | ||
| (sender) => !verificationResult.removedSenders.includes(sender), | ||
| ), | ||
| subjects: initialItems.subjects.filter( | ||
| (subject) => !verificationResult.removedSubjects.includes(subject), | ||
| ), | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize array filtering using Set for better performance
When filtering arrays using .includes, converting the arrays to Set objects can improve performance, especially with larger datasets.
Apply this refactor:
+ const removedSendersSet = new Set(verificationResult.removedSenders);
senders: initialItems.senders.filter(
- (sender) => !verificationResult.removedSenders.includes(sender),
+ (sender) => !removedSendersSet.has(sender),
),
+ const removedSubjectsSet = new Set(verificationResult.removedSubjects);
subjects: initialItems.subjects.filter(
- (subject) => !verificationResult.removedSubjects.includes(subject),
+ (subject) => !removedSubjectsSet.has(subject),
),📝 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.
| senders: initialItems.senders.filter( | |
| (sender) => !verificationResult.removedSenders.includes(sender), | |
| ), | |
| subjects: initialItems.subjects.filter( | |
| (subject) => !verificationResult.removedSubjects.includes(subject), | |
| ), | |
| }; | |
| const removedSendersSet = new Set(verificationResult.removedSenders); | |
| senders: initialItems.senders.filter( | |
| (sender) => !removedSendersSet.has(sender), | |
| ), | |
| const removedSubjectsSet = new Set(verificationResult.removedSubjects); | |
| subjects: initialItems.subjects.filter( | |
| (subject) => !removedSubjectsSet.has(subject), | |
| ), | |
| }; |
1 participant
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores
These updates aim to improve user experience and functionality while maintaining clarity and consistency throughout the application.