WIP: Knowledge for drafts #385
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a comprehensive Knowledge Base feature. It adds new UI components for adding, editing, and deleting knowledge entries, a dedicated API route, and corresponding database changes via Prisma. Additionally, several new AI functionalities have been implemented to extract and incorporate knowledge into email reply drafting. The changes also include revised tests, updated documentation, and adjustments to the email generation flow with enhanced model-selection logic, environment configurations, and utility improvements for logging and token retrieval. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant KB as KnowledgeBase UI
participant KF as KnowledgeForm Dialog
participant A as API (create/update/delete)
participant DB as Database (Prisma)
U->>KB: Click "Add Knowledge"
KB->>KF: Open KnowledgeForm for new entry
KF->>A: Submit createKnowledgeAction
A->>DB: Validate, insert new knowledge entry
DB-->>A: Confirmation and inserted data
A-->>KF: Return success
KF->>KB: Close dialog and refresh list
KB->>U: Display updated knowledge entries
sequenceDiagram
participant U as User (Email Composer)
participant E as Email Module
participant DB as Database (Knowledge Entries)
participant AI1 as aiExtractRelevantKnowledge
participant AI2 as aiDraftWithKnowledge
U->>E: Initiate email reply draft
E->>DB: Fetch thread messages & knowledge entries
E->>AI1: Extract relevant details from knowledge
AI1-->>E: Return extracted information
E->>AI2: Draft email using email history + knowledge
AI2-->>E: Return drafted email reply
E->>U: Display draft for review
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/env.tsOops! Something went wrong! :( ESLint: 9.23.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (14)
apps/web/components/ConfirmDialog.tsx (1)
25-50: LGTM: Clean component implementation with sensible defaults.The component implementation is clean, with appropriate default values for confirmText and cancelText props. The AlertDialog structure follows best practices from the Radix UI library.
One enhancement to consider in the future would be adding a loading state to the confirm button for async operations, preventing multiple clicks during processing.
.cursor/rules/knowledge.mdc (2)
1-5: Consider completing the YAML front matter.The YAML front matter at the top of the file has empty values for
descriptionandglobs. Consider adding appropriate values to these fields to make the documentation more complete.
62-63: Fix typo in documentation.There's a typo in the last line: "contentm" should be "content".
-For example, the knowledge base may include 100 pages of contentm, and the LLM extracts half a page of knowledge to pass to the more drafter LLM. +For example, the knowledge base may include 100 pages of content, and the LLM extracts half a page of knowledge to pass to the more drafter LLM.apps/web/__tests__/ai-extract-knowledge.test.ts (1)
99-116: Consider removing debug logs in tests.While debug logs can be helpful during test development, consider removing or conditionally enabling them for committed tests to keep the test output clean.
- console.debug( - "Generated content for Instagram query:\n", - result.data?.relevantContent, - ); + // Uncomment for debugging + // console.debug( + // "Generated content for Instagram query:\n", + // result.data?.relevantContent, + // );apps/web/app/(app)/automation/knowledge/KnowledgeForm.tsx (1)
79-87: Consider adding placeholder text to form fields.Adding placeholder text to the form fields would improve user experience by providing guidance on what kind of content is expected.
<Input type="text" name="content" label="Content" autosizeTextarea rows={5} registerProps={register("content")} error={errors.content} + placeholder="Enter the knowledge base content here..." />apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx (2)
72-74: Enhance error handling for table content.
LoadingContentneatly handles theloadinganderrorstates. However, if the API returns an emptydatawith no error, you might want to explicitly handle or message the user, ensuring a clear and consistent experience.
83-91: Improve “no entries” state messaging.When
data?.items.length === 0, you provide a basic prompt to add an entry. This is good, but consider adding a “refresh” or “retry” button if the empty state is unexpected or caused by partial loading issues.apps/web/prisma/schema.prisma (1)
102-103: Consider indexing on userId for large knowledge base queries.If the
Knowledgemodel grows big, queries filtered by user might slow down. Adding an index onuserIdor(userId, updatedAt)can help optimize lookups and ordering.model User { ... - Knowledge Knowledge[] + Knowledge Knowledge[] + @@index([userId, updatedAt]) // Potential index in the Knowledge model }apps/web/utils/ai/knowledge/extract.ts (2)
36-63: Consider truncation or summarization for large knowledge bases.If
knowledgeBasecan be huge, sending it all to LLM in one request might be expensive or exceed token limits. Adding logic to truncate or chunk the knowledge content helps manage cost and token usage, preserving essential information.
70-111: Add optional length enforcement.Although you advise limiting the extracted content to 2000 characters, there's no explicit constraint in the code. If you want to enforce the limit, consider adding a string slice or validation step before sending it to the LLM.
apps/web/utils/actions/knowledge.ts (3)
16-35: Good implementation of createKnowledgeAction with authentication and validationThe implementation properly authenticates the user, validates input data using Zod schemas, and creates the database entry with proper data structure.
Consider adding error handling for the database operation and returning a success indicator:
export const createKnowledgeAction = withActionInstrumentation( "createKnowledge", async (unsafeData: CreateKnowledgeBody) => { const session = await auth(); const userId = session?.user.id; if (!userId) return { error: "Not logged in" }; const { data, success, error } = createKnowledgeBody.safeParse(unsafeData); if (!success) return { error: error.message }; + try { await prisma.knowledge.create({ data: { ...data, userId, }, }); revalidatePath("/automation"); + return { success: true }; + } catch (err) { + return { error: "Failed to create knowledge entry" }; + } }, );
37-57: Consistent pattern for updateKnowledgeActionThe update action follows the same pattern as create, providing consistent error handling and user validation.
Consider adding error handling and checking if the record exists before updating:
export const updateKnowledgeAction = withActionInstrumentation( "updateKnowledge", async (unsafeData: UpdateKnowledgeBody) => { const session = await auth(); const userId = session?.user.id; if (!userId) return { error: "Not logged in" }; const { data, success, error } = updateKnowledgeBody.safeParse(unsafeData); if (!success) return { error: error.message }; + try { + const existingEntry = await prisma.knowledge.findUnique({ + where: { id: data.id, userId }, + }); + + if (!existingEntry) { + return { error: "Knowledge entry not found" }; + } + await prisma.knowledge.update({ where: { id: data.id, userId }, data: { title: data.title, content: data.content, }, }); revalidatePath("/automation"); + return { success: true }; + } catch (err) { + return { error: "Failed to update knowledge entry" }; + } }, );
59-75: Properly implemented deleteKnowledgeActionThe delete action follows the same authentication and validation pattern as the other actions.
Consider adding error handling and checking if the record exists:
export const deleteKnowledgeAction = withActionInstrumentation( "deleteKnowledge", async (unsafeData: DeleteKnowledgeBody) => { const session = await auth(); const userId = session?.user.id; if (!userId) return { error: "Not logged in" }; const { data, success, error } = deleteKnowledgeBody.safeParse(unsafeData); if (!success) return { error: error.message }; + try { + const existingEntry = await prisma.knowledge.findUnique({ + where: { id: data.id, userId }, + }); + + if (!existingEntry) { + return { error: "Knowledge entry not found" }; + } + await prisma.knowledge.delete({ where: { id: data.id, userId }, }); revalidatePath("/automation"); + return { success: true }; + } catch (err) { + return { error: "Failed to delete knowledge entry" }; + } }, );apps/web/components/ui/alert-dialog.tsx (1)
7-7: Fix extra slash in import pathThere's a double slash in the import path for the button component.
-import { buttonVariants } from "@/components/ui//button"; +import { buttonVariants } from "@/components/ui/button";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.cursor/rules/knowledge.mdc(1 hunks).cursor/rules/llm-test.mdc(1 hunks)apps/web/__tests__/ai-extract-knowledge.test.ts(1 hunks)apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx(1 hunks)apps/web/app/(app)/automation/knowledge/KnowledgeForm.tsx(1 hunks)apps/web/app/(app)/automation/page.tsx(3 hunks)apps/web/app/api/knowledge/route.ts(1 hunks)apps/web/components/ConfirmDialog.tsx(1 hunks)apps/web/components/ui/alert-dialog.tsx(1 hunks)apps/web/package.json(1 hunks)apps/web/prisma/schema.prisma(2 hunks)apps/web/utils/actions/knowledge.ts(1 hunks)apps/web/utils/actions/knowledge.validation.ts(1 hunks)apps/web/utils/ai/knowledge/extract.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
apps/web/app/(app)/automation/page.tsx (1)
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx (1)
KnowledgeBase(33-147)
apps/web/components/ConfirmDialog.tsx (1)
apps/web/components/ui/alert-dialog.tsx (9)
AlertDialog(130-130)AlertDialogTrigger(133-133)AlertDialogContent(134-134)AlertDialogHeader(135-135)AlertDialogTitle(137-137)AlertDialogDescription(138-138)AlertDialogFooter(136-136)AlertDialogCancel(140-140)AlertDialogAction(139-139)
apps/web/utils/actions/knowledge.ts (1)
apps/web/utils/actions/knowledge.validation.ts (6)
CreateKnowledgeBody(8-8)createKnowledgeBody(3-6)UpdateKnowledgeBody(16-16)updateKnowledgeBody(10-14)DeleteKnowledgeBody(22-22)deleteKnowledgeBody(18-20)
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx (4)
apps/web/app/api/knowledge/route.ts (1)
GetKnowledgeResponse(7-9)apps/web/app/(app)/automation/knowledge/KnowledgeForm.tsx (1)
KnowledgeForm(23-93)apps/web/components/ConfirmDialog.tsx (1)
ConfirmDialog(25-50)apps/web/utils/actions/knowledge.ts (1)
deleteKnowledgeAction(59-75)
🔇 Additional comments (33)
.cursor/rules/llm-test.mdc (1)
16-17: Improve Visual Clarity of Test File StructureThe changes to lines 16–17 simplify the directory structure representation by removing the leading characters, resulting in a cleaner and more consistent display. This update improves readability without affecting functionality.
apps/web/package.json (1)
44-44: LGTM: Added Radix UI alert dialog dependency.The addition of
@radix-ui/react-alert-dialogaligns with the existing Radix UI component usage pattern in the project and supports the new confirmation dialog functionality.apps/web/app/(app)/automation/page.tsx (3)
20-20: LGTM: KnowledgeBase component import added.The import statement correctly references the new KnowledgeBase component required for the knowledge base tab.
58-58: LGTM: Knowledge Base tab trigger added.The TabsTrigger is properly implemented with a descriptive label and consistent value naming convention.
96-98: LGTM: Knowledge Base tab content section added.The TabsContent section is correctly implemented with proper className for consistent styling and contains the KnowledgeBase component.
apps/web/components/ConfirmDialog.tsx (2)
1-14: LGTM: Component imports and client directive.The client directive is correctly placed at the top of the file, and the imports are properly organized. All required AlertDialog components are imported.
16-23: LGTM: Well-defined props interface.The ConfirmDialogProps interface is well-structured with clear type definitions for all required and optional props. The Promise | void return type for onConfirm properly supports both async and sync handlers.
apps/web/app/api/knowledge/route.ts (5)
1-6: LGTM: API route imports.All necessary imports are included for authentication, database access, and error handling.
7-9: LGTM: Clear response type definition.The GetKnowledgeResponse type is well-defined with clear structure for the returned data.
11-15: LGTM: Proper authentication check.The route correctly verifies user authentication before proceeding with database operations.
16-24: LGTM: Well-structured database query.The Prisma query is properly structured with appropriate filtering by userId and sorting by updatedAt in descending order.
25-28: LGTM: Clear response formatting.The response is correctly formatted according to the defined type and returned as JSON.
apps/web/utils/actions/knowledge.validation.ts (3)
3-6: LGTM: The schema for creating knowledge appears well defined.The validation schema properly validates that both title and content are required fields with appropriate error messages. This ensures data integrity for new knowledge entries.
10-14: LGTM: The update schema correctly extends the create schema.The update schema appropriately includes an ID field along with the title and content fields from the create schema, ensuring that knowledge entries can be properly identified for updates.
18-20: LGTM: The delete schema is appropriately minimal.The delete schema only requires an ID field, which is all that's needed to identify the knowledge entry for deletion. This is a clean approach.
.cursor/rules/knowledge.mdc (2)
8-36: LGTM: Well-structured documentation of the knowledge base feature.The documentation clearly explains the purpose, overview, and database schema of the knowledge base feature. This will be helpful for developers working on this feature.
37-53: LGTM: Comprehensive documentation of implementation details.The documentation provides a clear overview of the main files, directories, and features of the knowledge base implementation. This will help developers understand the architecture and functionality.
apps/web/__tests__/ai-extract-knowledge.test.ts (4)
1-12: LGTM: Test setup and configuration.The test file is well-configured with appropriate imports, mocks, and environment variable checks. The approach to conditionally skip tests based on the
RUN_AI_TESTSenvironment variable is a good practice for AI-related tests.
13-92: LGTM: Well-structured test fixtures.The helper functions
getUser()andgetKnowledgeBase()provide clean, reusable test fixtures with comprehensive test data covering various knowledge entry scenarios.
94-166: LGTM: Good test coverage for basic scenarios.The test cases for extracting knowledge about Instagram, YouTube, and TikTok sponsorships are well-structured and include appropriate assertions. The test for handling an empty knowledge base properly tests the error case.
168-246: LGTM: Comprehensive test coverage for additional scenarios.The test cases for multiple platforms, speaking engagements, consulting services, and brand ambassador programs provide thorough coverage of various use cases for the knowledge extraction functionality.
apps/web/app/(app)/automation/knowledge/KnowledgeForm.tsx (4)
1-22: LGTM: Clean imports and dependencies.The imports are well-organized and include all necessary dependencies for the component to function correctly.
23-47: LGTM: Well-structured component with clean props interface.The component has a clean props interface and properly initializes the form with appropriate default values based on whether it's in editing mode.
49-68: LGTM: Good form submission handling with proper error handling.The form submission logic appropriately handles both create and update operations, with clean error handling and success notifications. The use of
isActionErrorensures proper type checking.
70-92: LGTM: Clean form structure with appropriate input components.The form UI is well-structured with appropriate input components for the title and content fields. The button correctly reflects the current operation (create or update).
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx (1)
36-37: Validate potential undefined data states.Since you're using
data?.items, consider whetheritemscould beundefinedif the request fails or returns no data. You might introduce a stronger guarantee or handle anull/undefinedcase in the UI to avoid unexpected undefined object access.Would you like to confirm the returned data shape with additional logs or tests to ensure safe handling?
apps/web/prisma/schema.prisma (1)
442-451: Check for large content fields.
contentcan grow large for knowledge entries. Confirm that your column type can handle large text if you expect extensive knowledge content. Alternatively, consider a text-based or JSON field if you anticipate complex data.apps/web/components/ui/alert-dialog.tsx (6)
15-28: Well-implemented AlertDialogOverlay componentThe overlay component is properly implemented with forwarded refs and appropriate styling. The use of Radix UI's primitive and Tailwind's utility classes provides a good foundation for the alert dialog.
30-46: Good implementation of AlertDialogContent with responsive designThe content component is implemented with proper positioning, animations, and responsive design considerations.
48-74: Good layout components with responsive designBoth
AlertDialogHeaderandAlertDialogFootercomponents are well-implemented with responsive design considerations. The header switches between centered and left-aligned text based on screen size, while the footer adjusts button layout for different screen sizes.
76-99: Well-styled text componentsThe
AlertDialogTitleandAlertDialogDescriptioncomponents are properly styled with appropriate text sizes and weights.
101-127: Good implementation of action buttonsThe
AlertDialogActionandAlertDialogCancelcomponents use the button variants system for consistent styling. The cancel button has proper spacing adjustments for different screen sizes.
1-141: Great implementation of accessible dialog componentsOverall, this alert dialog implementation follows best practices:
- Uses Radix UI for accessibility
- Implements responsive design
- Provides flexible styling options
- Uses animations for better UX
- Proper component composition
| onConfirm={async () => { | ||
| const result = await deleteKnowledgeAction({ | ||
| id: item.id, | ||
| }); | ||
| if (isActionError(result)) { | ||
| toastError({ | ||
| title: "Error deleting knowledge base entry", | ||
| description: result.error, | ||
| }); | ||
| return; | ||
| } | ||
| toastSuccess({ | ||
| description: | ||
| "Knowledge base entry deleted successfully", | ||
| }); | ||
| mutate(); | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle potential race conditions with the delete flow.
When deleting knowledge entries, ensure there are no race conditions if multiple users or sessions try deleting simultaneously. One approach is to lock the row or check if it still exists before deletion. Currently, you rely on the server’s logic, but adding a quick client-side re-check or better error feedback could help.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx (1)
110-135: Add loading state to the delete buttonConsider adding a loading state to the delete button during the deletion process to prevent users from clicking multiple times, which could lead to multiple deletion requests.
<ConfirmDialog trigger={ <Button variant="outline" size="sm"> <Trash2 className="h-4 w-4" /> </Button> } title="Delete Knowledge Base Entry" description={`Are you sure you want to delete "${item.title}"? This action cannot be undone.`} + onConfirmProps={{ isLoading: deleteLoading }} onConfirm={async () => { + try { + setDeleteLoading(true); const result = await deleteKnowledgeAction({ id: item.id, }); if (isActionError(result)) { toastError({ title: "Error deleting knowledge base entry", description: result.error, }); return; } toastSuccess({ description: "Knowledge base entry deleted successfully", }); mutate(); + } finally { + setDeleteLoading(false); + } }} />You'll need to add a state for the loading status:
const [deleteLoading, setDeleteLoading] = useState(false);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx(1 hunks)apps/web/app/(app)/automation/knowledge/KnowledgeForm.tsx(1 hunks)apps/web/components/editor/Tiptap.tsx(3 hunks)apps/web/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/app/(app)/automation/knowledge/KnowledgeForm.tsx
- apps/web/package.json
🔇 Additional comments (3)
apps/web/components/editor/Tiptap.tsx (1)
5-5: Well-implemented markdown support!The addition of markdown functionality to the Tiptap editor is clean and complete. The implementation properly checks for editor existence before accessing markdown storage, and the interface is properly extended with the new
getMarkdownmethod.Also applies to: 12-12, 42-42, 73-76
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx (2)
118-134: Handle potential race conditions with the delete flow.When deleting knowledge entries, ensure there are no race conditions if multiple users or sessions try deleting simultaneously. One approach is to lock the row or check if it still exists before deletion. Currently, you rely on the server's logic, but adding a quick client-side re-check or better error feedback could help.
33-147: Well-structured knowledge base component!The implementation of the KnowledgeBase component is clean, follows React best practices, and provides a good user experience. The use of SWR for data fetching, proper dialog management, and appropriate error handling are all well done.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx (2)
119-127: Consider adding accessibility attributes to the Edit buttonTo improve accessibility, consider adding an aria-label to the Edit button.
<Button variant="outline" size="sm" + aria-label={`Edit ${item.title}`} onClick={() => { setEditingItem(item); }} > Edit </Button>
83-159: Consider adding a view option for contentCurrently, users can only see the title and updated date in the table, with no way to view the actual content without entering edit mode. Consider adding a view option or expandable rows to display content.
<TableRow key={item.id}> <TableCell>{item.title}</TableCell> <TableCell> {formatDateSimple(new Date(item.updatedAt))} </TableCell> <TableCell className="text-right"> <div className="flex items-center justify-end gap-2"> + <Button + variant="outline" + size="sm" + aria-label={`View ${item.title}`} + onClick={() => { + // Add view logic here + }} + > + View + </Button> <Button variant="outline" size="sm" onClick={() => { setEditingItem(item); }} > Edit </Button> <ConfirmDialog /* ... existing code ... */ /> </div> </TableCell> </TableRow>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx (4)
apps/web/app/api/knowledge/route.ts (1)
GetKnowledgeResponse(7-9)apps/web/app/(app)/automation/knowledge/KnowledgeForm.tsx (1)
KnowledgeForm(27-123)apps/web/components/ConfirmDialog.tsx (1)
ConfirmDialog(25-50)apps/web/utils/actions/knowledge.ts (1)
deleteKnowledgeAction(59-75)
🔇 Additional comments (6)
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx (6)
1-32: Well-organized imports and componentsThe imports are neatly structured and grouped by functionality. Good use of named imports to keep the code clean and maintainable.
33-48: Good state management and component structureThe component uses appropriate React hooks for state management. The
useCallbackfor dialog handlers helps optimize performance by preventing unnecessary re-renders.
51-70: Clean dialog implementation for form handlingThe dialog implementation is well-structured with conditional rendering based on whether you're adding or editing an item. Good separation of concerns by having a separate
KnowledgeFormcomponent.
83-109: Comprehensive empty state with helpful guidanceThe empty state provides clear instructions and useful notes for users. This is good UX design that helps users understand the purpose and functionality of the knowledge base.
128-153: Handle potential race conditions with the delete flowWhen deleting knowledge entries, ensure there are no race conditions if multiple users or sessions try deleting simultaneously. Consider adding a loading state to the delete button to prevent multiple clicks.
<ConfirmDialog trigger={ <Button variant="outline" size="sm"> <Trash2 className="h-4 w-4" /> </Button> } title="Delete Knowledge Base Entry" description={`Are you sure you want to delete "${item.title}"? This action cannot be undone.`} onConfirm={async () => { + try { + // Local state for tracking delete operation + let isDeleting = true; const result = await deleteKnowledgeAction({ id: item.id, }); if (isActionError(result)) { toastError({ title: "Error deleting knowledge base entry", description: result.error, }); return; } toastSuccess({ description: "Knowledge base entry deleted successfully", }); mutate(); + } finally { + isDeleting = false; + } }} />
36-37:✅ Verification successful
Add error handling feedback for SWR data fetching
While you're using
LoadingContentwhich likely handles this, ensure that users receive appropriate feedback when SWR data fetching fails.
🏁 Script executed:
#!/bin/bash # Check how LoadingContent handles errors fd "LoadingContent\.tsx" --type f --exec cat {} \;Length of output: 756
Error handling feedback confirmed
The
LoadingContentcomponent already provides robust error feedback by checking for an error (and displaying it via anerrorComponentor defaulting toErrorDisplay). Please verify that the error returned fromuseSWRis correctly passed toLoadingContentso that users see the appropriate error message when data fetching fails.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/app/(app)/automation/knowledge/KnowledgeForm.tsx (3)
27-55: Form setup with conditional validation is robustThe form setup uses react-hook-form with zodResolver correctly. The conditional validation schema based on whether it's creating or editing is a good approach. However, the default title "How to draft replies" seems specific - consider making it more generic or empty.
: { - title: "How to draft replies", + title: "", },
59-85: Well-structured form submission logic with proper error handlingThe submission logic correctly handles both create and update cases with appropriate error handling and success notifications. The use of
isActionErrorfor error detection is clean.Consider adding protection against unsaved changes when the dialog is closed:
// Add at component level const formRef = useRef<HTMLFormElement>(null); const [isDirty, setIsDirty] = useState(false); // Update form to track dirty state <form ref={formRef} onSubmit={handleSubmit(onSubmit)} onChange={() => setIsDirty(true)} className="space-y-4" > // Add confirmation before closing dialog // Modify closeDialog prop to accept a function like: const handleCloseDialog = () => { if (isDirty && !confirm("You have unsaved changes. Are you sure you want to close?")) { return; } closeDialog(); };
87-122: Form controls well-implemented with proper error displayThe form layout is clean with appropriate controls. The Controller component is correctly used for the Tiptap editor since it can't be directly registered. The error messaging for both fields is properly implemented.
For better user experience, consider adding an aria-describedby attribute to link error messages to form controls for accessibility:
<Controller name="content" control={control} render={({ field }) => ( <div className="max-h-[600px] overflow-y-auto"> <Tiptap ref={editorRef} initialContent={field.value ?? ""} className="mt-1" autofocus={false} + aria-describedby={errors.content ? "content-error" : undefined} /> </div> )} /> {errors.content && ( - <p className="mt-1 text-sm text-destructive"> + <p id="content-error" className="mt-1 text-sm text-destructive" aria-live="polite"> {errors.content.message} </p> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/app/(app)/automation/knowledge/KnowledgeForm.tsx(1 hunks)apps/web/components/editor/Tiptap.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/components/editor/Tiptap.tsx
🧰 Additional context used
🧬 Code Definitions (1)
apps/web/app/(app)/automation/knowledge/KnowledgeForm.tsx (1)
apps/web/components/editor/Tiptap.tsx (2)
TiptapHandle(10-13)Tiptap(15-95)
🔇 Additional comments (3)
apps/web/app/(app)/automation/knowledge/KnowledgeForm.tsx (3)
1-26: Well-organized imports and client-side directiveThe imports are properly organized and the "use client" directive is correctly placed at the top of the file, indicating this component will run on the client-side in Next.js.
57-58: Proper use of ref for accessing editor methodsUsing a ref to access the Tiptap editor's methods is the correct approach for getting the markdown content imperatively.
123-128: Submit button displays correct action text and loading stateThe submit button correctly shows different text based on whether it's creating or updating, and properly shows a loading state during submission.
… token handling. Update message retrieval to use batch processing and streamline previous conversation message parsing.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
apps/web/utils/llms/helpers.ts (1)
1-3: Good encapsulation of date formatting logic.Creating this utility function helps maintain consistency across the application and reduces code duplication. The implementation is clean and straightforward.
Consider adding an explicit return type for better type safety:
-export function getTodayForLLM() { +export function getTodayForLLM(): string { return `Today's date is: ${new Date().toISOString().split("T")[0]}.`; }apps/web/utils/gmail/client.ts (1)
47-52: Consider safer access to Gmail client internals.While the function provides a useful utility for extracting access tokens, it relies on accessing internal properties of the Gmail client that might not be stable across library versions.
Consider these improvements:
- Add a comment explaining why this approach was chosen
- Use more type-safe access patterns if possible
- Add error handling for potential structural changes
export const getAccessTokenFromClient = (client: gmail_v1.Gmail): string => { + // Accessing internal properties of the Gmail client to extract the access token + // This approach is used because there's no public API to get the token from an existing client const accessToken = (client.context._options.auth as any).credentials .access_token; if (!accessToken) throw new Error("No access token"); + + // Additional error handling could be added here for more robustness return accessToken; };apps/web/utils/ai/knowledge/extract-from-email-history.ts (1)
70-119: Avoid returning null for no historical messagesThe function returns
nullwhen there are no historical messages. This may require extra null checks downstream. For clearer handling, consider returning an empty string or a more descriptive object to indicate that no historical context is available.- if (historicalMessages.length === 0) return null; + if (historicalMessages.length === 0) { + logger.info("No historical messages found, returning empty summary"); + return ""; + }apps/web/utils/reply-tracker/generate-reply.ts (3)
103-108: Knowledge base database integrationThis implementation retrieves knowledge base entries for the current user, sorted by most recently updated first. Consider adding pagination if the knowledge base could grow large to prevent performance issues.
Consider adding a limit to the query to prevent fetching an excessive number of knowledge entries:
const knowledgeBase = await prisma.knowledge.findMany({ where: { userId: user.id }, orderBy: { updatedAt: "desc" }, + take: 50, // Limit to most recent 50 entries });
109-120: Knowledge extraction implementationThe code extracts relevant knowledge from the knowledge base using the last message content, which helps in generating more contextually relevant replies.
However, there's no error handling if
knowledgeResultdoesn't contain the expected data structure.Consider adding explicit error handling for the knowledge extraction result:
const knowledgeResult = await aiExtractRelevantKnowledge({ knowledgeBase, emailContent: lastMessageContent, user, }); + if (knowledgeResult.error) { + logger.warn("Failed to extract knowledge", { error: knowledgeResult.error }); + }
145-152: AI-powered draft generation with knowledge integrationThe implementation correctly uses the
aiDraftWithKnowledgefunction with all necessary parameters, including the extracted knowledge and email history.However, consider enhancing the error handling for cases where
aiDraftWithKnowledgereturns an error object.Add specific handling for error objects returned from
aiDraftWithKnowledge:const text = await aiDraftWithKnowledge({ messages, user, instructions, knowledgeBaseContent: knowledgeResult.data?.relevantContent || null, emailHistorySummary, }); + // Handle error object returned from aiDraftWithKnowledge + if (typeof text === "object" && "error" in text) { + logger.error("Error in draft generation", { error: text.error }); + throw new Error(`Failed to draft reply: ${text.error}`); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/utils/ai/knowledge/extract-from-email-history.ts(1 hunks)apps/web/utils/ai/knowledge/extract.ts(1 hunks)apps/web/utils/ai/reply/draft-with-knowledge.ts(1 hunks)apps/web/utils/ai/reply/generate-nudge.ts(2 hunks)apps/web/utils/ai/reply/generate-reply.ts(2 hunks)apps/web/utils/gmail/client.ts(1 hunks)apps/web/utils/llms/helpers.ts(1 hunks)apps/web/utils/logger.ts(1 hunks)apps/web/utils/reply-tracker/generate-reply.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/ai/reply/draft-with-knowledge.ts
🧰 Additional context used
🧬 Code Definitions (4)
apps/web/utils/ai/reply/generate-nudge.ts (1)
apps/web/utils/llms/helpers.ts (1)
getTodayForLLM(1-3)
apps/web/utils/ai/reply/generate-reply.ts (1)
apps/web/utils/llms/helpers.ts (1)
getTodayForLLM(1-3)
apps/web/utils/reply-tracker/generate-reply.ts (4)
apps/web/utils/gmail/client.ts (1)
getAccessTokenFromClient(47-52)apps/web/utils/ai/knowledge/extract.ts (1)
aiExtractRelevantKnowledge(71-115)apps/web/utils/ai/knowledge/extract-from-email-history.ts (1)
aiExtractFromEmailHistory(70-119)apps/web/utils/ai/reply/draft-with-knowledge.ts (1)
aiDraftWithKnowledge(100-149)
apps/web/utils/ai/knowledge/extract.ts (1)
apps/web/utils/llms/model-selector.ts (1)
getEconomyModel(16-48)
🔇 Additional comments (13)
apps/web/utils/ai/reply/generate-nudge.ts (2)
6-6: LGTM! Good use of the new utility function.The import of the
getTodayForLLMfunction is appropriate and helps maintain consistent date formatting across the application.
38-38: LGTM! Consistent date formatting implementation.Using the centralized
getTodayForLLM()function instead of inline date formatting improves maintainability and ensures consistent date representation across all AI prompts.apps/web/utils/ai/reply/generate-reply.ts (2)
6-6: LGTM! Good use of the new utility function.The import of the
getTodayForLLMfunction is appropriate and helps maintain consistent date formatting across the application.
63-63: LGTM! Consistent date formatting implementation.Using the centralized
getTodayForLLM()function instead of inline date formatting improves maintainability and ensures consistent date representation across all AI prompts.apps/web/utils/logger.ts (1)
39-43: Improve development-time message formattingReplacing literal
\nwith actual newlines in development logs is a clear readability enhancement. However, consider whether other escape sequences (e.g.,\t) might also benefit from similar handling if multi-line logs become more complex.apps/web/utils/ai/knowledge/extract-from-email-history.ts (1)
62-68: Enforce the max 500-character limitAlthough the system prompt requests a concise summary “max 500 characters,” the
summaryfield inextractionSchemadoes not enforce a.max(500)constraint. Consider adding.max(500)to validate the output length and prevent overly long summaries.- summary: z.string() + summary: z.string().max(500)apps/web/utils/ai/knowledge/extract.ts (1)
71-115: Return standardized response objectsThis function returns
{ data: ... }on success and{ error: ... }on failure. This pattern is fine, but ensure all consumers handle these two different return shapes correctly. Also consider logging a message when the knowledge base is empty for easier debugging.apps/web/utils/reply-tracker/generate-reply.ts (6)
6-18: Comprehensive import updates for knowledge base integrationThe new imports reflect the addition of knowledge-related functionality, including knowledge extraction from both the knowledge base and email history. This aligns well with the PR objective of integrating knowledge base features for draft generation.
52-57: Good addition of historical context for draft generationFetching previous conversation messages provides valuable context for the AI to generate more relevant drafts. The implementation correctly uses the
getMessagesBatchfunction with appropriate access token handling.
68-70: Robust error handling for draft generationThis type check ensures the system fails explicitly when the generation doesn't return a string result, preventing potential runtime errors downstream.
81-81: Updated function signature for enhanced contextual awarenessThe addition of the
previousConversationMessagesparameter aligns with the changes in thegenerateDraftfunction and enables knowledge extraction from historical conversations.
121-143: Email history context extractionThis section effectively prepares and processes historical messages to provide additional context for draft generation. The code correctly handles the case when no historical messages are available.
154-160: Type-safe reply savingThe code now properly checks that the result is a string before saving it to Redis. This improves type safety and prevents potential errors.
| const extractionSchema = z.object({ | ||
| relevantContent: z.string(), | ||
| explanation: z.string(), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce the max 2000-character limit
The system prompt mentions keeping extracted content concise (max 2000 characters), but there’s no corresponding .max(2000) in the schema for relevantContent or explanation. Adding a maximum length constraint ensures alignment with the prompt requirements and avoids oversized outputs.
const extractionSchema = z.object({
- relevantContent: z.string(),
- explanation: z.string(),
+ relevantContent: z.string().max(2000),
+ explanation: z.string().max(2000),
});📝 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 extractionSchema = z.object({ | |
| relevantContent: z.string(), | |
| explanation: z.string(), | |
| }); | |
| const extractionSchema = z.object({ | |
| relevantContent: z.string().max(2000), | |
| explanation: z.string().max(2000), | |
| }); |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/utils/ai/knowledge/extract.ts (1)
67-101: Good implementation of knowledge extraction with proper error handling.The function correctly handles edge cases (empty knowledge base) and provides comprehensive logging for debugging. Using
useEconomyModel: trueis appropriate for this task as it involves processing large knowledge bases.One observation: The system prompt (lines 9-28) mentions two fields in the response format (
relevantContentandexplanation), but the schema only captures therelevantContent. Consider either updating the schema to include both fields or removing theexplanationfield from the prompt if it's not needed.apps/web/utils/reply-tracker/generate-reply.ts (1)
104-120: Comprehensive knowledge retrieval and extraction implementation.The code effectively fetches knowledge base entries and uses them to enhance reply drafting. The length limit of 10,000 characters for
lastMessageContentis quite high - consider if this could be reduced to improve performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/utils/ai/knowledge/extract-from-email-history.ts(1 hunks)apps/web/utils/ai/knowledge/extract.ts(1 hunks)apps/web/utils/llms/index.ts(11 hunks)apps/web/utils/llms/model.ts(2 hunks)apps/web/utils/reply-tracker/generate-reply.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/ai/knowledge/extract-from-email-history.ts
🧰 Additional context used
🧬 Code Definitions (4)
apps/web/utils/llms/model.ts (2)
apps/web/env.ts (1)
env(5-219)apps/web/utils/llms/config.ts (2)
Provider(5-12)Model(14-29)
apps/web/utils/reply-tracker/generate-reply.ts (4)
apps/web/utils/gmail/client.ts (1)
getAccessTokenFromClient(47-52)apps/web/utils/ai/knowledge/extract.ts (1)
aiExtractRelevantKnowledge(67-101)apps/web/utils/ai/knowledge/extract-from-email-history.ts (1)
aiExtractFromEmailHistory(72-115)apps/web/utils/ai/reply/draft-with-knowledge.ts (1)
aiDraftWithKnowledge(99-148)
apps/web/utils/llms/index.ts (1)
apps/web/utils/llms/model.ts (1)
getModel(37-151)
apps/web/utils/ai/knowledge/extract.ts (1)
apps/web/utils/llms/index.ts (1)
chatCompletionObject(120-124)
🔇 Additional comments (10)
apps/web/utils/llms/model.ts (2)
37-40: Well-designed model selection flow with economy option.The addition of the
useEconomyModelparameter creates a clean extension to the model selection logic without disrupting existing functionality. This implementation follows good separation of concerns by delegating economy model selection to a dedicated function.Also applies to: 45-46
153-195:✅ Verification successful
Excellent fallback strategy for economy model selection.
The implementation provides a thoughtful fallback hierarchy:
- Uses explicitly configured economy provider and model
- Falls back to default model for specified provider
- Defaults to Gemini Flash if Google is available
- Ultimately falls back to user's default model
The clear documentation also explains the appropriate use cases for economy models.
There's a potential edge case to consider:
🏁 Script executed:
#!/bin/bash # Check for potential recursion edge case ast-grep --pattern $'function getEconomyModel(userAi: UserAIFields) { $$$ return getModel(userAi, true); $$$ }'Length of output: 118
Clarification and Final Approval: Economy Model Fallback is Robust
The fallback hierarchy in the
getEconomyModelfunction is well implemented:
- It first uses an explicitly configured economy provider and model.
- It then defaults to the provider's default model if only the provider is specified.
- It further falls back to Gemini Flash when a Google API key is available.
- Finally, it resorts to the user's default model.
I executed an
ast-grepcheck for a potential recursive edge case (i.e., a call likegetModel(userAi, true)) and did not find any instances in the codebase. This confirms that the recursion edge case is not present.apps/web/utils/ai/knowledge/extract.ts (1)
65-65: Enforce the max 2000-character limitThe system prompt mentions keeping extracted content concise (max 2000 characters), but there's no corresponding
.max(2000)in the schema forrelevantContent. Adding a maximum length constraint ensures alignment with the prompt requirements and avoids oversized outputs.-const extractionSchema = z.object({ relevantContent: z.string() }); +const extractionSchema = z.object({ relevantContent: z.string().max(2000) });apps/web/utils/reply-tracker/generate-reply.ts (6)
53-56: Good addition of historical context for better reply generation.Fetching previous conversation messages provides valuable context for the AI to generate more relevant replies. This enhancement will likely improve the quality of generated drafts.
68-70: Strong defensive type checking.This type verification ensures that only valid string responses are used for draft creation, preventing potential runtime errors when creating email drafts.
78-83: Function signature updated appropriately to include conversation history.The updated parameter list properly reflects the new functionality while maintaining backward compatibility.
128-144: Well-structured historical context extraction.The code properly transforms previous messages for the AI extraction process and only attempts extraction if historical messages are available, avoiding unnecessary AI calls.
145-152: Effective integration of knowledge and history into drafting.The drafting process now leverages both knowledge base content and email history summary, which should result in more contextually relevant replies.
154-160: Good defensive programming for reply storage.Only storing the reply when it's a valid string prevents potential errors in the Redis storage operation.
apps/web/utils/llms/index.ts (1)
59-99: Systematic implementation of economy model option across all LLM functions.The changes consistently add the
useEconomyModelparameter to all relevant functions, maintaining a uniform approach throughout the codebase. This implementation enables cost optimization for appropriate use cases like knowledge extraction and email history analysis.Also applies to: 126-163, 165-203, 205-223, 229-269, 272-317
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/app/(app)/automation/RuleForm.tsx (1)
598-599: Consider adding a defensive check for action type.Direct access to
actionInputs[action.type].fieldscould cause runtime errors if an unknown action type is encountered.-const fields = actionInputs[action.type].fields; +const fields = actionInputs[action.type]?.fields || [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(app)/automation/RuleForm.tsx(4 hunks)apps/web/package.json(3 hunks)apps/web/utils/actions/rule.validation.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/package.json
🔇 Additional comments (5)
apps/web/utils/actions/rule.validation.ts (1)
71-72: LGTM: ThesetManuallyproperty is properly typed.The addition of the
setManuallyproperty as a nullable boolean is appropriate for the schema. The comment indicating it's "only needed for frontend" is useful context.apps/web/app/(app)/automation/RuleForm.tsx (4)
87-98: Good implementation of conditional defaultValues for form initialization.The logic correctly sets
setManuallyto true for DRAFT_EMAIL actions that have content, ensuring proper initialization of the form state based on whether the rule already exists.
622-640: LGTM: Clear UI for AI-generated draft mode.The conditional rendering for DRAFT_EMAIL actions provides a good user experience by showing a message about AI-generated replies when
setManuallyis false.
641-740: LGTM: Good handling of field rendering based on setManually state.The code correctly renders the form fields only when
setManuallyis true or for other action types, maintaining proper separation between automatic and manual modes.
743-754: LGTM: Good toggle between manual and automatic modes.The "Auto draft" button is appropriately displayed only when
setManuallyis true, allowing users to easily switch back to AI-generated drafts.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores