Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces several modifications across multiple files, primarily focusing on simplifying and enhancing the handling of AI-generated arguments and logging mechanisms. Key changes include updating button labels in the UI, consolidating function calls for action item generation, and implementing a scoped logging utility to replace console logging. The overall structure and functionality of existing components remain intact, but improvements have been made to streamline code and enhance maintainability. 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: 7
🧹 Outside diff range and nitpick comments (15)
apps/web/utils/logger.ts (3)
1-2: Consider enhancing type definitions with JSDoc and additional log levels.The types are well-structured, but could benefit from documentation and additional common log levels.
+/** Supported log levels for the scoped logger */ -type LogLevel = "log" | "error" | "warn" | "trace"; +type LogLevel = "log" | "error" | "warn" | "trace" | "info" | "debug"; +/** Supported message types for logging */ type LogMessage = string | Record<string, unknown>;
4-10: Enhance color constants maintainability and type safety.The color implementation could be improved for better maintainability and type safety.
+/** ANSI color codes for different log levels */ +const ANSI_COLORS = { + WHITE: "\x1b[0m", + RED: "\x1b[31m", + YELLOW: "\x1b[33m", + CYAN: "\x1b[36m", + BLUE: "\x1b[34m", + GREEN: "\x1b[32m", + RESET: "\x1b[0m", +} as const; -const colors = { - log: "\x1b[0m", // white - error: "\x1b[31m", // red - warn: "\x1b[33m", // yellow - trace: "\x1b[36m", // cyan - reset: "\x1b[0m", -} as const; +const colors: Record<LogLevel | 'reset', string> = { + log: ANSI_COLORS.WHITE, + error: ANSI_COLORS.RED, + warn: ANSI_COLORS.YELLOW, + trace: ANSI_COLORS.CYAN, + info: ANSI_COLORS.BLUE, + debug: ANSI_COLORS.GREEN, + reset: ANSI_COLORS.RESET, } as const;
27-43: Consider enhancing logger architecture for better flexibility and debugging.The current implementation could benefit from additional features for better debugging and customization.
Consider implementing:
- Configurable output destination (console, file, remote service)
- Minimum log level setting
- Stack traces for errors
- Timestamps in logs
- Log batching for performance
Example enhancement:
type LoggerConfig = { minLevel?: LogLevel; destination?: 'console' | 'file' | 'remote'; includeTimestamp?: boolean; batchSize?: number; }; export function createScopedLogger(scope: string, config?: LoggerConfig) { const defaultConfig: LoggerConfig = { minLevel: 'log', destination: 'console', includeTimestamp: true, batchSize: 1, ...config }; // Implementation details... }apps/web/app/api/google/webhook/static-rule.ts (1)
Line range hint
60-60: Address or track the TODO commentThere's a TODO comment about implementing thread checking to skip processing. This should be addressed to prevent potential duplicate processing of emails in the same thread.
Would you like me to help create a GitHub issue to track this TODO item?
apps/web/utils/ai/choose-rule/choose-and-execute.ts (2)
90-103: Consider batch size limits for createMany operation.The
createManyoperation could potentially handle a large number of action items. Consider adding a batch size limit to prevent performance issues or database constraints.Consider implementing batching:
actionItems: { createMany: { data: plannedAct.actionItems?.map((a) => ({ type: a.type, label: a.label, subject: a.subject, content: a.content, to: a.to, cc: a.cc, bcc: a.bcc, - })) || [], + })).slice(0, MAX_BATCH_SIZE) || [], + skipDuplicates: true, // Optional: Add if duplicate actions should be skipped }, },
90-103: Add type safety for action item fields.The current implementation assumes all fields exist on the action item object. Consider adding type guards or default values to handle potential undefined fields.
Consider adding type safety:
actionItems: { createMany: { data: plannedAct.actionItems?.map((a) => ({ - type: a.type, - label: a.label, - subject: a.subject, - content: a.content, - to: a.to, - cc: a.cc, - bcc: a.bcc, + type: a.type ?? 'unknown', + label: a.label ?? '', + subject: a.subject ?? '', + content: a.content ?? '', + to: a.to ?? null, + cc: a.cc ?? null, + bcc: a.bcc ?? null, })) || [], }, },apps/web/utils/ai/actions.ts (1)
CC field modification requires security controls
Based on the code analysis:
- The
replyfunction inactions.tsdirectly passes the CC field from AI args to the email sending function without any validation- The email schema shows CC is optional but lacks any security restrictions
- No permission checks or validations are present for CC field modifications
This could lead to potential security issues where the AI could add unauthorized recipients to email threads. Recommend implementing:
- Validation to ensure CC recipients are within allowed domains
- Permission checks for CC modifications
- Consider restricting AI's ability to modify CC fields entirely
🔗 Analysis chain
Line range hint
284-290: Address TODO comment regarding CC field permissions.The TODO comment raises an important security consideration about whether the AI should be allowed to modify CC recipients. This could lead to unintended information disclosure if not properly controlled.
Let's check if there are any existing restrictions or validations for CC modifications:
Would you like help implementing proper validation and restrictions for CC field modifications?
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for CC-related validations or restrictions rg -i "cc.*validation|validate.*cc|restrict.*cc" --type typescriptLength of output: 103
Script:
#!/bin/bash # Search for CC-related code and validation patterns with ripgrep rg -l "cc" --type ts # Get the content of the file under review to see the TODO comment cat "apps/web/utils/ai/actions.ts" # Search for any email validation or permission checks rg -i "email.*validation|validate.*email|permission.*email" --type tsLength of output: 17687
Script:
#!/bin/bash # Search for email permission checks in the codebase rg -A 5 "permission.*email|email.*permission" apps/web/utils/gmail/permissions.ts apps/web/utils/actions/permissions.ts # Look for any CC-related code in the email sending functions rg -A 5 "cc:" apps/web/utils/gmail/mail.ts # Check for any security-related validation in the actions rg -A 5 "validate|check|restrict" apps/web/utils/ai/actions.ts apps/web/utils/actions/validation.tsLength of output: 1000
apps/web/app/api/google/webhook/process-history.ts (3)
264-264: Consider using structured logging for errorsWhile the error logging is functional, consider using structured logging to make it easier to parse and analyze errors in production.
- logger.error(`Error processing history item. email: ${email}`, error); + logger.error('Error processing history item', { email, error });
296-298: Consider structured logging for better observabilityThe logging additions are good, but could be improved by using structured logging consistently across all log statements. This would make it easier to query and analyze logs in production.
- logger.log( - `Getting message... email: ${user.email} messageId: ${messageId} threadId: ${threadId}`, - ); + logger.log('Getting message', { email: user.email, messageId, threadId }); - logger.log("Running cold email blocker..."); + logger.log('Running cold email blocker', { email: user.email, messageId }); - logger.log("Running rules..."); + logger.log('Running rules', { email: user.email, messageId });Also applies to: 345-346, 390-391
Line range hint
27-405: Consider implementing a logging schemaWhile the migration to a scoped logger is a good improvement, consider defining a standard logging schema across the application. This would ensure consistency in log structure and make it easier to implement log aggregation and analysis.
Key points to consider:
- Define standard fields (e.g., userId, messageId, operation)
- Establish log levels usage guidelines
- Document structured logging patterns
apps/web/utils/ai/choose-rule/ai-choose-args.ts (5)
Line range hint
77-163: Ensure consistent return types by returning an empty array instead ofundefinedThe function
getArgsAiResponsecurrently returnsundefinedwhen there are no tool calls or whentoolCall?.toolNameis undefined. Returning an empty array instead ofundefinedcan simplify the calling code and reduce the need for additional null checks.Apply this diff to modify the function:
async function getArgsAiResponse({ email, user, selectedRule, actionsRequiringAi, }: { email: EmailForLLM; user: Pick<User, "email" | "about"> & UserAIFields; selectedRule: RuleWithActions; actionsRequiringAi: ActionRequiringAi[]; - }): Promise<ActionWithAiArgs[] | undefined> { + }): Promise<ActionWithAiArgs[]> { logger.log( `Generating args for rule ${selectedRule.name} (${selectedRule.id})`, ); if (!actionsRequiringAi.length) { logger.log( `Skipping. No parameters for rule ${selectedRule.name} (${selectedRule.id})`, ); - return; + return []; } // ... rest of the code ... if (!toolCall?.toolName) { logger.error("No tool call received from AI response."); - return; + return []; } // ... rest of the code ... }
39-46: InitializeaiGeneratedActionItemsas an empty arrayCurrently,
aiGeneratedActionItemsis initialized asundefinedand assigned a value only ifactionsRequiringAi.lengthis truthy. Initializing it as an empty array simplifies the code and eliminates the need for optional chaining later.Apply this diff:
-let aiGeneratedActionItems: ActionWithAiArgs[] | undefined; +let aiGeneratedActionItems: ActionWithAiArgs[] = []; if (actionsRequiringAi.length) { aiGeneratedActionItems = await getArgsAiResponse({ email, user, selectedRule, actionsRequiringAi, }); }
51-53: Remove unnecessary optional chaining foraiGeneratedActionItemsAfter initializing
aiGeneratedActionItemsas an empty array, you can remove the optional chaining when accessing it, as it will always be defined.Apply this diff:
-const aiGeneratedActionItem = aiGeneratedActionItems?.find( +const aiGeneratedActionItem = aiGeneratedActionItems.find( (item) => item.actionId === action.id, );
Line range hint
124-138: Add error handling for the external API call tochatCompletionToolsThe call to
chatCompletionToolscould fail or throw an exception. Wrapping this call in atry-catchblock ensures that errors are handled gracefully and prevents unhandled promise rejections.Apply this diff:
logger.trace(`Prompt: ${prompt}`); // logger.trace("Zod parameters:", zodToJsonSchema(zodParameters)); +try { const aiResponse = await chatCompletionTools({ userAi: user, prompt, system, tools: { apply_rule: { description: "Apply the rule with the given arguments", parameters: zodParameters, }, }, label: "Args for rule", userEmail: user.email || "", }); +} catch (error) { + logger.error(`Failed to get AI response: ${error.message}`); + return []; +} const toolCall = aiResponse.toolCalls[0];
Line range hint
85-96: Sanitize user-provided content to prevent prompt injection attacksIncluding
user.aboutdirectly in the system prompt may expose the AI to prompt injection attacks if the user content is maliciously crafted. To mitigate this risk, sanitizeuser.aboutbefore including it in the prompt.Consider applying sanitization:
const system = `You are an AI assistant that helps people manage their emails. Never put placeholders in your email responses. Do not mention you are an AI assistant when responding to people. ${ - user.about + sanitize(user.about) ? `\nSome additional information the user has provided about themselves:\n\n${user.about}` : "" }`; +function sanitize(input: string): string { + // Implement sanitization logic here (e.g., remove special characters, limit length) + return input.replace(/[^\w\s.,'-]/g, '').slice(0, 500); +}Ensure that the
sanitizefunction properly escapes or removes any potentially harmful content fromuser.about.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
apps/web/app/(app)/automation/Rules.tsx(1 hunks)apps/web/app/api/google/webhook/group-rule.ts(2 hunks)apps/web/app/api/google/webhook/process-history.ts(16 hunks)apps/web/app/api/google/webhook/static-rule.ts(2 hunks)apps/web/utils/ai/actions.ts(2 hunks)apps/web/utils/ai/choose-rule/ai-choose-args.ts(4 hunks)apps/web/utils/ai/choose-rule/choose-and-execute.ts(1 hunks)apps/web/utils/ai/choose-rule/choose.ts(2 hunks)apps/web/utils/logger.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/app/(app)/automation/Rules.tsx
🔇 Additional comments (8)
apps/web/utils/ai/choose-rule/choose.ts (2)
2-2: LGTM! Import consolidation improves code organization.
The consolidation of AI argument handling into a single import simplifies the code and makes it easier to maintain.
42-46: Implementation looks good, but let's verify the consolidated function.
The refactoring simplifies the code while maintaining proper error handling. However, we should verify the behavior of getActionItemsWithAiArgs across the codebase.
Let's check the implementation and usage of the new consolidated function:
✅ Verification successful
Let me gather more information about the implementation of getActionItemsWithAiArgs to verify its behavior and error handling.
Implementation is well-structured with proper error handling and type safety
The implementation of getActionItemsWithAiArgs is robust and includes:
- Strong type safety with Zod schema validation
- Proper error handling and logging
- Consistent handling of AI-generated arguments
- Clear separation of concerns between static and AI-generated parameters
- Proper null checks and edge case handling (e.g., duplicate action types)
The function is used consistently across the codebase in three locations:
apps/web/utils/ai/choose-rule/choose.tsapps/web/app/api/google/webhook/static-rule.tsapps/web/app/api/google/webhook/group-rule.ts
All usages follow the same parameter pattern, maintaining consistency in the implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation and usage of getActionItemsWithAiArgs
# Check the implementation
echo "Checking implementation of getActionItemsWithAiArgs:"
ast-grep --pattern 'export $_ getActionItemsWithAiArgs' -A 10
# Check all usages
echo "Checking all usages of getActionItemsWithAiArgs:"
rg "getActionItemsWithAiArgs" -A 3
Length of output: 2935
Script:
#!/bin/bash
# Get the full implementation of getActionItemsWithAiArgs
echo "Implementation of getActionItemsWithAiArgs:"
cat apps/web/utils/ai/choose-rule/ai-choose-args.ts
# Check the type definitions used
echo -e "\nType definitions:"
rg "interface.*ActionItems" --type ts
rg "type.*ActionItems" --type ts
Length of output: 7281
apps/web/app/api/google/webhook/group-rule.ts (1)
5-5: LGTM! Import consolidation improves code organization.
The consolidated import simplifies the codebase by reducing the number of imported functions while maintaining clear functionality.
apps/web/app/api/google/webhook/static-rule.ts (2)
4-4: LGTM: Import statement is well-organized
The new import for getActionItemsWithAiArgs is properly placed with related AI utility imports.
51-56: Verify error handling and data completeness
While the consolidation of action item generation is a good improvement, please ensure:
- Error handling is properly implemented in
getActionItemsWithAiArgs - All necessary data from the previous implementation is passed to the new function
Let's verify the implementation of the new function and its error handling:
✅ Verification successful
Let's gather more information about the implementation and error handling in the function:
Error handling and data completeness are properly implemented
The implementation of getActionItemsWithAiArgs is well-structured with:
- Proper error handling through the logger utility
- Type safety using zod schema validation
- Complete data validation for all required parameters
- Graceful handling of AI-generated args with fallbacks
The function is used consistently across both static and group rule handlers, with all necessary data being passed correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the implementation of getActionItemsWithAiArgs
ast-grep --pattern 'export async function getActionItemsWithAiArgs($_) {
$$$
}'
# Check for error handling patterns in related files
rg -A 5 'try.*catch.*getActionItemsWithAiArgs'
# Verify if there are any other callers that might be affected
rg 'getActionItemsWithAiArgs'
Length of output: 1062
Script:
#!/bin/bash
# Check the full implementation of the file containing getActionItemsWithAiArgs
cat apps/web/utils/ai/choose-rule/ai-choose-args.ts
# Check the implementation of files that call this function
cat apps/web/app/api/google/webhook/static-rule.ts
cat apps/web/app/api/google/webhook/group-rule.ts
Length of output: 13019
apps/web/utils/ai/choose-rule/choose-and-execute.ts (1)
90-103: 🛠️ Refactor suggestion
Consider adding data validation for email-related fields.
The mapping of action items includes email-related fields (to, cc, bcc) which should be validated to ensure they contain valid email addresses before saving to the database.
Let's check if there's any validation in place:
Consider adding validation:
actionItems: {
createMany: {
data:
plannedAct.actionItems?.map((a) => ({
type: a.type,
label: a.label,
subject: a.subject,
content: a.content,
- to: a.to,
- cc: a.cc,
- bcc: a.bcc,
+ to: validateEmailAddresses(a.to),
+ cc: validateEmailAddresses(a.cc),
+ bcc: validateEmailAddresses(a.bcc),
})) || [],
},
},apps/web/utils/ai/actions.ts (1)
12-14: LGTM! Well-structured logging setup.
The introduction of a scoped logger with a descriptive identifier "ai-actions" follows good practices for structured logging.
apps/web/app/api/google/webhook/process-history.ts (1)
27-29: LGTM: Logger setup is appropriate
The scoped logger implementation provides better context for debugging and monitoring.
| const formatMessage = (level: LogLevel, message: LogMessage) => { | ||
| const prefix = `[${scope}]: `; | ||
|
|
||
| if (process.env.NODE_ENV === "development") { | ||
| return `${colors[level]}${prefix} ${ | ||
| typeof message === "string" ? message : JSON.stringify(message, null, 2) | ||
| }${colors.reset}`; | ||
| } | ||
|
|
||
| return `${prefix} ${ | ||
| typeof message === "string" ? message : JSON.stringify(message) | ||
| }`; | ||
| }; |
There was a problem hiding this comment.
Add safety measures for object serialization.
The current implementation might face issues with circular references and large objects.
const formatMessage = (level: LogLevel, message: LogMessage) => {
const prefix = `[${scope}]: `;
+ const safeStringify = (obj: Record<string, unknown>) => {
+ const cache = new Set();
+ return JSON.stringify(obj, (key, value) => {
+ if (typeof value === 'object' && value !== null) {
+ if (cache.has(value)) {
+ return '[Circular]';
+ }
+ cache.add(value);
+ }
+ return value;
+ }, process.env.NODE_ENV === "development" ? 2 : undefined);
+ };
if (process.env.NODE_ENV === "development") {
return `${colors[level]}${prefix} ${
- typeof message === "string" ? message : JSON.stringify(message, null, 2)
+ typeof message === "string" ? message : safeStringify(message)
}${colors.reset}`;
}
return `${prefix} ${
- typeof message === "string" ? message : JSON.stringify(message)
+ typeof message === "string" ? message : safeStringify(message)
}`;
};📝 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 formatMessage = (level: LogLevel, message: LogMessage) => { | |
| const prefix = `[${scope}]: `; | |
| if (process.env.NODE_ENV === "development") { | |
| return `${colors[level]}${prefix} ${ | |
| typeof message === "string" ? message : JSON.stringify(message, null, 2) | |
| }${colors.reset}`; | |
| } | |
| return `${prefix} ${ | |
| typeof message === "string" ? message : JSON.stringify(message) | |
| }`; | |
| }; | |
| const formatMessage = (level: LogLevel, message: LogMessage) => { | |
| const prefix = `[${scope}]: `; | |
| const safeStringify = (obj: Record<string, unknown>) => { | |
| const cache = new Set(); | |
| return JSON.stringify(obj, (key, value) => { | |
| if (typeof value === 'object' && value !== null) { | |
| if (cache.has(value)) { | |
| return '[Circular]'; | |
| } | |
| cache.add(value); | |
| } | |
| return value; | |
| }, process.env.NODE_ENV === "development" ? 2 : undefined); | |
| }; | |
| if (process.env.NODE_ENV === "development") { | |
| return `${colors[level]}${prefix} ${ | |
| typeof message === "string" ? message : safeStringify(message) | |
| }${colors.reset}`; | |
| } | |
| return `${prefix} ${ | |
| typeof message === "string" ? message : safeStringify(message) | |
| }`; | |
| }; |
| type LogLevel = "log" | "error" | "warn" | "trace"; | ||
| type LogMessage = string | Record<string, unknown>; | ||
|
|
||
| const colors = { | ||
| log: "\x1b[0m", // white | ||
| error: "\x1b[31m", // red | ||
| warn: "\x1b[33m", // yellow | ||
| trace: "\x1b[36m", // cyan | ||
| reset: "\x1b[0m", | ||
| } as const; | ||
|
|
||
| export function createScopedLogger(scope: string) { | ||
| const formatMessage = (level: LogLevel, message: LogMessage) => { | ||
| const prefix = `[${scope}]: `; | ||
|
|
||
| if (process.env.NODE_ENV === "development") { | ||
| return `${colors[level]}${prefix} ${ | ||
| typeof message === "string" ? message : JSON.stringify(message, null, 2) | ||
| }${colors.reset}`; | ||
| } | ||
|
|
||
| return `${prefix} ${ | ||
| typeof message === "string" ? message : JSON.stringify(message) | ||
| }`; | ||
| }; | ||
|
|
||
| return { | ||
| log: (message: LogMessage, ...args: unknown[]) => | ||
| console.log(formatMessage("log", message), ...args), | ||
|
|
||
| error: (message: LogMessage, ...args: unknown[]) => | ||
| console.error(formatMessage("error", message), ...args), | ||
|
|
||
| warn: (message: LogMessage, ...args: unknown[]) => | ||
| console.warn(formatMessage("warn", message), ...args), | ||
|
|
||
| trace: (message: LogMessage, ...args: unknown[]) => { | ||
| if (process.env.NODE_ENV === "development") { | ||
| console.log(formatMessage("trace", message), ...args); | ||
| } | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Based on the search results, I can now generate the final response:
Migration to scoped logger needed across multiple utility files
The new logger utility is currently used in only 3 files:
apps/web/utils/ai/actions.tsapps/web/utils/ai/choose-rule/ai-choose-args.tsapps/web/app/api/google/webhook/process-history.ts
However, there are numerous direct console.* calls across the codebase that should be migrated to use this new scoped logger:
- Multiple utility files under
apps/web/utils/(middleware.ts, json.ts, posthog.ts, etc.) - API route handlers under
apps/web/app/api/ - Action handlers under
apps/web/utils/actions/
The migration should be done systematically to ensure consistent logging across the application.
🔗 Analysis chain
Verify logger integration and usage patterns.
Since this is a new utility replacing existing console.* calls, let's verify its integration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify logger usage and potential console.* calls that should be migrated
# Check for remaining console.* calls that should use the logger
echo "Checking for console.* calls that should be migrated to the logger..."
rg -t typescript -t javascript 'console\.(log|error|warn|debug)' --glob '!apps/web/utils/logger.ts'
# Check current usage patterns of the logger
echo "Checking current logger usage patterns..."
rg -t typescript -t javascript 'createScopedLogger'
Length of output: 465
Script:
#!/bin/bash
# Let's try with simpler commands without file type filters
# Check for remaining console.* calls that should use the logger
echo "Checking for console.* calls that should be migrated to the logger..."
rg 'console\.(log|error|warn|debug)' --glob '!apps/web/utils/logger.ts'
# Check current usage patterns of the logger
echo "Checking current logger usage patterns..."
rg 'createScopedLogger'
# Check for any TypeScript/JavaScript files to understand the codebase structure
echo "Checking for TypeScript/JavaScript files..."
fd '\.(ts|js|tsx|jsx)$'
Length of output: 46513
| // get action items with args | ||
| const actionItems = await getActionItemsWithAiArgs({ | ||
| email, | ||
| user, | ||
| selectedRule: match.rule, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling for AI operations.
The AI operation could fail due to various reasons (API limits, network issues, invalid responses). Consider adding proper error handling to ensure graceful degradation.
// get action items with args
- const actionItems = await getActionItemsWithAiArgs({
- email,
- user,
- selectedRule: match.rule,
- });
+ let actionItems;
+ try {
+ actionItems = await getActionItemsWithAiArgs({
+ email,
+ user,
+ selectedRule: match.rule,
+ });
+ } catch (error) {
+ console.error('Failed to generate AI action items:', error);
+ // Consider your error handling strategy:
+ // 1. Return early with an error status
+ // 2. Continue with empty actionItems
+ // 3. Retry with backoff
+ actionItems = [];
+ }Committable suggestion skipped: line range outside the PR's diff.
💡 Codebase verification
Migration to consolidated AI function needs attention
The verification shows that while the new getActionItemsWithAiArgs function is properly used across multiple files, the old getArgsAiResponse function still exists and is being used in apps/web/utils/ai/choose-rule/ai-choose-args.ts. This indicates an incomplete migration:
apps/web/utils/ai/choose-rule/ai-choose-args.ts: Contains both the old and new implementation, with the oldgetArgsAiResponsefunction still being actively used.
🔗 Analysis chain
Verify complete migration to consolidated AI function.
Let's ensure all previous AI argument handling code has been properly migrated to use the new consolidated function.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining usage of old AI argument handling functions
rg -l "getArgsAiResponse|getActionItemsFromAiArgsResponse" --type typescript
# Search for the new consolidated function usage
rg -l "getActionItemsWithAiArgs" --type typescript
Length of output: 199
Script:
#!/bin/bash
# Search for any remaining usage of old AI argument handling functions without file type restriction
rg -l "getArgsAiResponse|getActionItemsFromAiArgsResponse"
# Search for the new consolidated function usage
rg -l "getActionItemsWithAiArgs"
# Let's also check for function definitions to understand the migration
ast-grep --pattern 'function getArgsAiResponse'
ast-grep --pattern 'function getActionItemsFromAiArgsResponse'
ast-grep --pattern 'function getActionItemsWithAiArgs'
Length of output: 494
Script:
#!/bin/bash
# Let's check the content of ai-choose-args.ts to see if old functions are just imported or actually used
rg "getArgsAiResponse|getActionItemsFromAiArgsResponse" apps/web/utils/ai/choose-rule/ai-choose-args.ts -C 3
# Let's also check how getActionItemsWithAiArgs is used across files
rg "getActionItemsWithAiArgs" apps/web/utils/ai/choose-rule/choose.ts -C 3
rg "getActionItemsWithAiArgs" apps/web/utils/ai/choose-rule/ai-choose-args.ts -C 3
rg "getActionItemsWithAiArgs" apps/web/app/api/google/webhook/static-rule.ts -C 3
rg "getActionItemsWithAiArgs" apps/web/app/api/google/webhook/group-rule.ts -C 3
Length of output: 2513
| logger.log("Running action", { actionType: action.type, userEmail }); | ||
| logger.trace("Running action:", action); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider security and performance implications of logging.
A few concerns about the current logging implementation:
- The trace log includes the entire action object which might contain sensitive information (e.g., email addresses, message content).
- The userEmail being logged could be considered PII.
- No conditional check for trace level before object serialization, which could impact performance.
Consider applying these improvements:
- logger.log("Running action", { actionType: action.type, userEmail });
- logger.trace("Running action:", action);
+ logger.log("Running action", {
+ actionType: action.type,
+ userEmail: userEmail.replace(/(.{2}).*@/, '$1***@') // Mask email
+ });
+ if (logger.isTraceEnabled()) {
+ const sanitizedAction = {
+ ...action,
+ content: action.content ? '[REDACTED]' : undefined,
+ to: action.to ? '[REDACTED]' : undefined,
+ cc: action.cc ? '[REDACTED]' : undefined,
+ bcc: action.bcc ? '[REDACTED]' : undefined
+ };
+ logger.trace("Running action details:", sanitizedAction);
+ }Committable suggestion skipped: line range outside the PR's diff.
| console.debug(`does not have hasAiOrColdEmailAccess. email: ${email}`); | ||
| await unwatchEmails(account); | ||
| return NextResponse.json({ ok: true }); |
There was a problem hiding this comment.
Replace console.debug with logger.debug for consistency
There's a remaining console.debug call that should be updated to use the scoped logger.
- console.debug(`does not have hasAiOrColdEmailAccess. email: ${email}`);
+ logger.debug(`does not have hasAiOrColdEmailAccess. email: ${email}`);Committable suggestion skipped: line range outside the PR's diff.
| const args: Record<string, string> = toolCallArgs[key]; // e.g. { label: "Draft", subject: "X", content: "Y" } | ||
|
|
There was a problem hiding this comment.
Handle potential undefined values in toolCallArgs[key]
When accessing toolCallArgs[key], there's a possibility that it might be undefined. To prevent runtime errors, add a check or provide a default value to ensure args is always an object.
Apply this diff:
const [key] = actionParameter; // e.g. key = "draft_email"
-const args: Record<string, string> = toolCallArgs[key]; // e.g. { label: "Draft", subject: "X", content: "Y" }
+const args: Record<string, string> = toolCallArgs[key] || {}; // Ensure args is an object even if undefined📝 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 args: Record<string, string> = toolCallArgs[key]; // e.g. { label: "Draft", subject: "X", content: "Y" } | |
| const args: Record<string, string> = toolCallArgs[key] || {}; // Ensure args is an object even if undefined | |
1 participant
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes