Skip to content

Fixes for AI generated responses #258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elie222 merged 8 commits into from
Nov 22, 2024
Merged

Fixes for AI generated responses #258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f8f9306
Handle ai args better for duplicate action types
elie222 Nov 22, 2024
8c87b8a
Don't add extra fields to db
elie222 Nov 22, 2024
4910a1b
Prettier logger
elie222 Nov 22, 2024
77f5a7e
Adjust copy
elie222 Nov 22, 2024
f4105c4
Adjust logger
elie222 Nov 22, 2024
26e99bc
Fix ai generated fields
elie222 Nov 22, 2024
bec7cbf
Adjust file order
elie222 Nov 22, 2024
a889a7d
adjust logger
elie222 Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/web/app/(app)/automation/Rules.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ export function Rules() {
<Button asChild variant="outline">
<Link href="/automation?tab=prompt">
<PenIcon className="mr-2 hidden h-4 w-4 md:block" />
Add Rule via Prompt File
Add Rule via Prompt Tab
</Link>
</Button>
<Button asChild variant="outline">
Expand Down
26 changes: 7 additions & 19 deletions apps/web/app/api/google/webhook/group-rule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@ import type { gmail_v1 } from "@googleapis/gmail";
import type { ParsedMessage } from "@/utils/types";
import type { User } from "@prisma/client";
import { emailToContent } from "@/utils/mail";
import {
getActionItemsFromAiArgsResponse,
getActionsWithParameters,
getArgsAiResponse,
} from "@/utils/ai/choose-rule/ai-choose-args";
import { getActionItemsWithAiArgs } from "@/utils/ai/choose-rule/ai-choose-args";
import {
findMatchingGroup,
getGroups,
Expand Down Expand Up @@ -60,20 +56,12 @@ export async function handleGroupRule({
}),
};

// generate args
const aiArgsResponse =
getActionsWithParameters(match.rule.actions).length > 0
? await getArgsAiResponse({
email,
selectedRule: match.rule,
user,
})
: undefined;

const actionItems = getActionItemsFromAiArgsResponse(
aiArgsResponse,
match.rule.actions,
);
// get action items with args
const actionItems = await getActionItemsWithAiArgs({
email,
user,
selectedRule: match.rule,
});
Comment on lines +59 to +64
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 22, 2024

Choose a reason for hiding this comment

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

🛠️ 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 old getArgsAiResponse function 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


// handle action
// TODO isThread check to skip
Expand Down
47 changes: 24 additions & 23 deletions apps/web/app/api/google/webhook/process-history.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import { runRulesOnMessage } from "@/utils/ai/choose-rule/run-rules";
import { blockUnsubscribedEmails } from "@/app/api/google/webhook/block-unsubscribed-emails";
import { categorizeSender } from "@/utils/actions/categorize";
import { unwatchEmails } from "@/app/api/google/watch/controller";
import { createScopedLogger } from "@/utils/logger";

const logger = createScopedLogger("Process History");

export async function processHistoryForUser(
decodedData: {
Expand Down Expand Up @@ -70,7 +73,7 @@ export async function processHistoryForUser(
});

if (!account) {
console.error(`Google webhook: Account not found. email: ${email}`);
logger.error(`Account not found. email: ${email}`);
return NextResponse.json({ ok: true });
}

Expand All @@ -79,7 +82,7 @@ export async function processHistoryForUser(
: undefined;

if (!premium) {
console.log(`Google webhook: Account not premium. email: ${email}`);
logger.log(`Account not premium. email: ${email}`);
await unwatchEmails(account);
return NextResponse.json({ ok: true });
}
Expand All @@ -94,9 +97,7 @@ export async function processHistoryForUser(
);

if (!userHasAiAccess && !userHasColdEmailAccess) {
console.debug(
`Google webhook: does not have hasAiOrColdEmailAccess. email: ${email}`,
);
console.debug(`does not have hasAiOrColdEmailAccess. email: ${email}`);
await unwatchEmails(account);
return NextResponse.json({ ok: true });
Comment on lines +100 to 102
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 22, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.

}
Expand All @@ -107,21 +108,21 @@ export async function processHistoryForUser(
account.user.coldEmailBlocker !== ColdEmailSetting.DISABLED;
if (!hasAutomationRules && !shouldBlockColdEmails) {
console.debug(
`Google webhook: has no rules set and cold email blocker disabled. email: ${email}`,
`has no rules set and cold email blocker disabled. email: ${email}`,
);
return NextResponse.json({ ok: true });
}

if (!account.access_token || !account.refresh_token) {
console.error(
logger.error(
`Missing access or refresh token. User needs to re-authenticate. email: ${email}`,
);
return NextResponse.json({ ok: true });
}

if (!account.user.email) {
// shouldn't ever happen
console.error("Missing user email.", email);
logger.error(`Missing user email: ${email}`);
return NextResponse.json({ ok: true });
}

Expand All @@ -145,8 +146,8 @@ export async function processHistoryForUser(
historyId - 500, // avoid going too far back
).toString();

console.log(
`Webhook: Listing history... Start: ${startHistoryId} lastSyncedHistoryId: ${account.user.lastSyncedHistoryId} gmailHistoryId: ${startHistoryId} email: ${email}`,
logger.log(
`Listing history... Start: ${startHistoryId} lastSyncedHistoryId: ${account.user.lastSyncedHistoryId} gmailHistoryId: ${startHistoryId} email: ${email}`,
);

const history = await gmail.users.history.list({
Expand All @@ -160,8 +161,8 @@ export async function processHistoryForUser(
});

if (history.data.history) {
console.log(
`Webhook: Processing... email: ${email} startHistoryId: ${startHistoryId} historyId: ${history.data.historyId}`,
logger.log(
`Processing... email: ${email} startHistoryId: ${startHistoryId} historyId: ${history.data.historyId}`,
);

await processHistory({
Expand All @@ -186,8 +187,8 @@ export async function processHistoryForUser(
},
});
} else {
console.log(
`Webhook: No history. startHistoryId: ${startHistoryId}. ${JSON.stringify(decodedData)}`,
logger.log(
`No history. startHistoryId: ${startHistoryId}. ${JSON.stringify(decodedData)}`,
);

// important to save this or we can get into a loop with never receiving history
Expand All @@ -197,12 +198,12 @@ export async function processHistoryForUser(
});
}

console.log(`Webhook: Completed. ${JSON.stringify(decodedData)}`);
logger.log(`Completed. ${JSON.stringify(decodedData)}`);

return NextResponse.json({ ok: true });
} catch (error) {
captureException(error, { extra: { decodedData } }, email);
console.error("Error processing webhook", error, decodedData);
logger.error("Error processing webhook", error, decodedData);
return NextResponse.json({ error: true });
// be careful about calling an error here with the wrong settings, as otherwise PubSub will call the webhook over and over
// return NextResponse.error();
Expand Down Expand Up @@ -260,7 +261,7 @@ async function processHistory(options: ProcessHistoryOptions) {
{ extra: { email, messageId: m.message?.id } },
email,
);
console.error(`Error processing history item. email: ${email}`, error);
logger.error(`Error processing history item. email: ${email}`, error);
}
}
}
Expand Down Expand Up @@ -292,7 +293,7 @@ async function processHistoryItem(
if (!messageId) return;
if (!threadId) return;

console.log(
logger.log(
`Getting message... email: ${user.email} messageId: ${messageId} threadId: ${threadId}`,
);

Expand All @@ -309,7 +310,7 @@ async function processHistoryItem(

// if the rule has already been executed, skip
if (hasExistingRule) {
console.log("Skipping. Rule already exists.");
logger.log("Skipping. Rule already exists.");
return;
}

Expand All @@ -326,7 +327,7 @@ async function processHistoryItem(
});

if (blocked) {
console.log(
logger.log(
`Skipping. Blocked unsubscribed email. email: ${user.email} messageId: ${messageId} threadId: ${threadId}`,
);
return;
Expand All @@ -341,7 +342,7 @@ async function processHistoryItem(
);

if (shouldRunBlocker) {
console.log("Running cold email blocker...");
logger.log("Running cold email blocker...");

const hasPreviousEmail = await hasPreviousEmailsFromSenderOrDomain(
gmail,
Expand Down Expand Up @@ -386,7 +387,7 @@ async function processHistoryItem(
}

if (hasAutomationRules && hasAiAutomationAccess) {
console.log("Running rules...");
logger.log("Running rules...");

await runRulesOnMessage({
gmail,
Expand All @@ -399,7 +400,7 @@ async function processHistoryItem(
} catch (error: any) {
// gmail bug or snoozed email: https://stackoverflow.com/questions/65290987/gmail-api-getmessage-method-returns-404-for-message-gotten-from-listhistory-meth
if (error.message === "Requested entity was not found.") {
console.log(
logger.log(
`Message not found. email: ${user.email} messageId: ${messageId} threadId: ${threadId}`,
);
return;
Expand Down
26 changes: 7 additions & 19 deletions apps/web/app/api/google/webhook/static-rule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import type { gmail_v1 } from "@googleapis/gmail";
import type { ParsedMessage, RuleWithActions } from "@/utils/types";
import { RuleType, type User } from "@prisma/client";
import {
getActionItemsFromAiArgsResponse,
getActionsWithParameters,
getArgsAiResponse,
} from "@/utils/ai/choose-rule/ai-choose-args";
import { getActionItemsWithAiArgs } from "@/utils/ai/choose-rule/ai-choose-args";
import { emailToContent } from "@/utils/mail";
import { saveExecutedRule } from "@/utils/ai/choose-rule/choose-and-execute";
import { executeAct } from "@/utils/ai/choose-rule/execute";
Expand Down Expand Up @@ -52,20 +48,12 @@ export async function handleStaticRule({
}),
};

// generate args
const aiArgsResponse =
getActionsWithParameters(staticRule.actions).length > 0
? await getArgsAiResponse({
email,
selectedRule: staticRule,
user,
})
: undefined;

const actionItems = getActionItemsFromAiArgsResponse(
aiArgsResponse,
staticRule.actions,
);
// get action items with args
const actionItems = await getActionItemsWithAiArgs({
email,
user,
selectedRule: staticRule,
});

// handle action
// TODO isThread check to skip
Expand Down
6 changes: 5 additions & 1 deletion apps/web/utils/ai/actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import {
} from "@/utils/gmail/label";
import { markSpam } from "@/utils/gmail/spam";
import type { Attachment } from "@/utils/types/mail";
import { createScopedLogger } from "@/utils/logger";

const logger = createScopedLogger("ai-actions");

export type EmailForAction = {
threadId: string;
Expand Down Expand Up @@ -369,7 +372,8 @@ export const runActionFunction = async (
action: ActionItem,
userEmail: string,
) => {
console.log("Running action", { actionType: action.type, userEmail });
logger.log("Running action", { actionType: action.type, userEmail });
logger.trace("Running action:", action);
Comment on lines +375 to +376
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 22, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider security and performance implications of logging.

A few concerns about the current logging implementation:

  1. The trace log includes the entire action object which might contain sensitive information (e.g., email addresses, message content).
  2. The userEmail being logged could be considered PII.
  3. 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.


const { type, ...args } = action;
switch (type) {
Expand Down
Loading