Knowledge for drafts and clean schema clean up #385
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces extensive improvements across multiple modules. The changes add a comprehensive Knowledge Base feature with new UI components, API routes, actions, and validations. New tests and AI extraction functions have been implemented. Enhancements to the editor, package scripts, environment variables, and Prisma schema are included. Furthermore, the reply generation and action processing logic has been overhauled to incorporate a new TRACK_THREAD action and an economy model option. Several legacy functions and analytics files were removed and replaced with streamlined alternatives. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant KB as KnowledgeBase UI
participant API as Knowledge API
participant DB as Database
U->>KB: Open Knowledge Base
KB->>API: Fetch knowledge entries
API->>DB: Query Knowledge table
DB-->>API: Return entries
API-->>KB: Send data
KB->>U: Render knowledge table with CRUD options
sequenceDiagram
participant U as User
participant E as Email Client
participant AI as AI Service
participant KB as KnowledgeBase System
U->>E: Compose/send email
E->>AI: Request draft reply with email content
AI->>KB: Extract relevant knowledge (aiExtractRelevantKnowledge)
KB-->>AI: Return extracted info
AI->>E: Generate draft reply (aiDraftWithKnowledge)
E->>U: Display generated reply
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
Scope: all 2 workspace projects This error happened while installing a direct dependency of /tmp/eslint/packages/eslint-config ✨ Finishing Touches
🪧 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: 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
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 (1)
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx (1)
49-72: Consider implementing test coverage for the dialog flow.Adding tests (e.g., component tests or integration tests) can verify that the dialog opens correctly, the form fields accept input, and the dialog closes properly after submission. This helps prevent regressions in your knowledge creation and editing workflows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx(1 hunks)apps/web/app/(app)/automation/page.tsx(3 hunks)apps/web/app/(app)/reply-zero/ReplyTrackerSettings.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- apps/web/app/(app)/reply-zero/ReplyTrackerSettings.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(app)/automation/page.tsx
🧰 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-128)apps/web/components/ConfirmDialog.tsx (1)
ConfirmDialog(25-50)apps/web/utils/actions/knowledge.ts (1)
deleteKnowledgeAction(59-75)
🔇 Additional comments (3)
apps/web/app/(app)/automation/knowledge/KnowledgeBase.tsx (3)
1-32: All good in the initial code setup.Your imports are tidy, and the file’s structure at the top is clean and ergonomic. Great job maintaining clarity for future readers.
78-130: User-friendly fallback UI.The empty state message clearly guides new users on how to add and use knowledge entries. It’s a nice touch to address usability from the start.
156-185: Revisit the concurrency concern.This mirrors the previously raised point regarding potential race conditions in the delete flow. If multiple sessions attempt a delete simultaneously, consider re-checking the row’s existence or providing more robust error feedback.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/utils/llms/model.ts (1)
154-173: Missing provider mappings in getProviderApiKey function.The function signature includes "bedrock" and "ollama" as possible providers, but the
providerApiKeysobject doesn't include mappings for these providers. This could cause issues if someone tries to use these as economy providers.function getProviderApiKey( provider: | "openai" | "anthropic" | "google" | "groq" | "openrouter" | "bedrock" | "ollama", ) { const providerApiKeys = { [Provider.ANTHROPIC]: env.ANTHROPIC_API_KEY, [Provider.OPEN_AI]: env.OPENAI_API_KEY, [Provider.GOOGLE]: env.GOOGLE_API_KEY, [Provider.GROQ]: env.GROQ_API_KEY, [Provider.OPENROUTER]: env.OPENROUTER_API_KEY, + bedrock: undefined, // Bedrock uses AWS credentials instead of an API key + ollama: undefined, // Ollama uses a base URL instead of an API key }; return providerApiKeys[provider]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/utils/llms/model.ts(2 hunks)
🔇 Additional comments (3)
apps/web/utils/llms/model.ts (3)
13-21: Well-implemented feature for economy model selection!The addition of the optional
useEconomyModelparameter to thegetModelfunction is cleanly implemented. The early return pattern is appropriate and maintains backward compatibility.
129-152: Great documentation and implementation of the economy model feature.The JSDoc comments clearly explain the purpose and use cases for the economy model functionality. The implementation is clean and has appropriate fallbacks if no economy configuration is found.
175-197: Good relocation of the getDefaultProvider function.Moving this function after the new helper functions improves code organization and maintains the same functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/utils/llms/model.ts(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/web/utils/llms/model.ts (2)
apps/web/env.ts (1)
env(5-227)apps/web/utils/llms/config.ts (2)
Provider(5-12)supportsOllama(3-3)
🔇 Additional comments (3)
apps/web/utils/llms/model.ts (3)
13-23: Good implementation of economy model supportThe getModel function has been nicely extended to support economy models with a clean boolean parameter. This enhancement allows for cost optimization when processing large knowledge bases or performing bulk operations.
136-159: Well-documented economy model selection functionThe selectEconomyModel function is well-documented with clear JSDoc comments explaining its purpose and use cases. The implementation includes proper environment variable checks and graceful fallbacks when required configuration is missing.
182-204: Properly relocated getDefaultProvider functionThe getDefaultProvider function has been correctly relocated without functional changes, maintaining the same logic for determining the default AI provider based on environment variables.
| function getProviderApiKey( | ||
| provider: | ||
| | "openai" | ||
| | "anthropic" | ||
| | "google" | ||
| | "groq" | ||
| | "openrouter" | ||
| | "bedrock" | ||
| | "ollama", | ||
| ) { | ||
| const providerApiKeys = { | ||
| [Provider.ANTHROPIC]: env.ANTHROPIC_API_KEY, | ||
| [Provider.OPEN_AI]: env.OPENAI_API_KEY, | ||
| [Provider.GOOGLE]: env.GOOGLE_API_KEY, | ||
| [Provider.GROQ]: env.GROQ_API_KEY, | ||
| [Provider.OPENROUTER]: env.OPENROUTER_API_KEY, | ||
| }; | ||
|
|
||
| return providerApiKeys[provider]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Provider API key function missing some providers
The getProviderApiKey function doesn't include mappings for all providers that appear in the Provider object - specifically 'bedrock' and 'ollama' are missing from the providerApiKeys mapping.
function getProviderApiKey(
provider:
| "openai"
| "anthropic"
| "google"
| "groq"
| "openrouter"
| "bedrock"
| "ollama",
) {
const providerApiKeys = {
[Provider.ANTHROPIC]: env.ANTHROPIC_API_KEY,
[Provider.OPEN_AI]: env.OPENAI_API_KEY,
[Provider.GOOGLE]: env.GOOGLE_API_KEY,
[Provider.GROQ]: env.GROQ_API_KEY,
[Provider.OPENROUTER]: env.OPENROUTER_API_KEY,
+ bedrock: env.BEDROCK_ACCESS_KEY && env.BEDROCK_SECRET_KEY ? "available" : undefined,
+ [Provider.OLLAMA]: env.OLLAMA_BASE_URL ? "available" : undefined,
};
return providerApiKeys[provider];
}For 'bedrock' and 'ollama', we're not returning actual API keys but indicators of availability since they use different authentication mechanisms.
📝 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.
| function getProviderApiKey( | |
| provider: | |
| | "openai" | |
| | "anthropic" | |
| | "google" | |
| | "groq" | |
| | "openrouter" | |
| | "bedrock" | |
| | "ollama", | |
| ) { | |
| const providerApiKeys = { | |
| [Provider.ANTHROPIC]: env.ANTHROPIC_API_KEY, | |
| [Provider.OPEN_AI]: env.OPENAI_API_KEY, | |
| [Provider.GOOGLE]: env.GOOGLE_API_KEY, | |
| [Provider.GROQ]: env.GROQ_API_KEY, | |
| [Provider.OPENROUTER]: env.OPENROUTER_API_KEY, | |
| }; | |
| return providerApiKeys[provider]; | |
| } | |
| function getProviderApiKey( | |
| provider: | |
| | "openai" | |
| | "anthropic" | |
| | "google" | |
| | "groq" | |
| | "openrouter" | |
| | "bedrock" | |
| | "ollama", | |
| ) { | |
| const providerApiKeys = { | |
| [Provider.ANTHROPIC]: env.ANTHROPIC_API_KEY, | |
| [Provider.OPEN_AI]: env.OPENAI_API_KEY, | |
| [Provider.GOOGLE]: env.GOOGLE_API_KEY, | |
| [Provider.GROQ]: env.GROQ_API_KEY, | |
| [Provider.OPENROUTER]: env.OPENROUTER_API_KEY, | |
| bedrock: env.BEDROCK_ACCESS_KEY && env.BEDROCK_SECRET_KEY ? "available" : undefined, | |
| [Provider.OLLAMA]: env.OLLAMA_BASE_URL ? "available" : undefined, | |
| }; | |
| return providerApiKeys[provider]; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/utils/ai/knowledge/extract-from-email-history.ts (2)
88-88: Add more descriptive logging when skipping extraction.When there are no historical messages, the function correctly returns null, but it would be helpful to add a log message explaining why the extraction is being skipped for better observability.
- if (historicalMessages.length === 0) return null; + if (historicalMessages.length === 0) { + logger.info("Skipping extraction: No historical messages available"); + return null; + }
26-60: Add return type annotation to getUserPrompt function.For better code clarity and type safety, consider adding a return type annotation to the getUserPrompt function.
const getUserPrompt = ({ currentThreadMessages, historicalMessages, user, }: { currentThreadMessages: EmailForLLM[]; historicalMessages: EmailForLLM[]; user: UserEmailWithAI; -}) => { +}): string => {apps/web/utils/reply-tracker/outbound.ts (1)
194-217: Robust implementation for message orderingThis helper function has several good aspects:
- Properly handles the empty array case
- Creates a new array with spread operator before sorting (avoiding mutation)
- Uses proper date comparison
- Returns both the result and the sorted messages for reuse
- Provides detailed logging when a message isn't the latest
One minor concern is that the log levels are inconsistent between the info in
handleOutboundReplyand warn in this function for the same condition.Consider using the same log level in both places for consistency:
- logger.warn( + logger.info( "Skipping outbound reply check: message is not the latest in the thread", { processingMessageId: message.id, actualLatestMessageId: actualLatestMessage?.id, }, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/app/api/ai/categorize/controller.ts(0 hunks)apps/web/utils/ai/clean/ai-clean.ts(1 hunks)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/check-if-needs-reply.ts(2 hunks)apps/web/utils/reply-tracker/outbound.ts(3 hunks)apps/web/utils/zod.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/app/api/ai/categorize/controller.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/ai/knowledge/extract.ts
🧰 Additional context used
🧬 Code Definitions (4)
apps/web/utils/ai/clean/ai-clean.ts (1)
apps/web/utils/zod.ts (1)
preprocessBooleanLike(16-23)
apps/web/utils/reply-tracker/outbound.ts (2)
apps/web/utils/reply-tracker/label.ts (1)
getReplyTrackingLabels(58-71)apps/web/utils/ai/reply/check-if-needs-reply.ts (1)
aiCheckIfNeedsReply(26-81)
apps/web/utils/ai/reply/check-if-needs-reply.ts (1)
apps/web/utils/zod.ts (1)
preprocessBooleanLike(16-23)
apps/web/utils/ai/knowledge/extract-from-email-history.ts (3)
apps/web/utils/llms/helpers.ts (1)
getTodayForLLM(1-3)apps/web/utils/zod.ts (1)
preprocessBooleanLike(16-23)apps/web/utils/llms/index.ts (1)
chatCompletionObject(128-132)
🔇 Additional comments (24)
apps/web/utils/ai/knowledge/extract-from-email-history.ts (4)
62-71: Enforce the 500-character limit in the schema.Although the system prompt (line 20) mentions a maximum of 500 characters for the summary, the schema does not enforce this constraint. Consider adding a
.max(500)constraint to ensure the returned summary remains within the specified limit.- .string() + .string().max(500)
73-116: Well-implemented extraction function with proper error handling.The implementation of
aiExtractFromEmailHistoryis robust with appropriate error handling, logging, and early returns. The use of the economy model is a good choice for this type of extraction task that doesn't require maximum performance.
12-24: Clear and well-structured system prompt.The system prompt clearly defines the AI agent's task with a structured approach for analyzing email threads. It provides specific focus areas for the summary, which helps guide the AI to produce relevant and useful information.
39-45: Good handling of empty historical messages case.The template handling for historical messages properly addresses the case when no historical messages are available, providing a clear message instead of an empty section.
apps/web/utils/reply-tracker/outbound.ts (7)
29-34: Good improvement with early feature checkAdding an early check for the outbound tracking feature is a good practice that prevents unnecessary processing when the feature is disabled.
53-55: Appropriate error level upgradeChanging this to an error log is appropriate since this condition would prevent the function from working correctly.
57-68: Good validation for message sequencingAdding a check to ensure we're only processing the latest message in a thread prevents incorrect tracking of old messages and improves efficiency.
70-72: Good refactoring with data preparationExtracting the data preparation logic into a separate function improves readability and maintainability.
Also applies to: 74-79
94-95: Improved logging with rationaleIncluding the AI's rationale in the log message provides valuable context for debugging and analysis.
186-192: Well-implemented feature check functionThe implementation of
isOutboundTrackingEnabledfollows best practices:
- Uses the database to check user settings
- Returns a boolean with proper type annotation
- Handles null/undefined cases with the double-bang operator
219-245: Well-structured data preparation functionThe implementation of
prepareDataForAICheckhas several good qualities:
- Clear return type definition
- Appropriate maxLength values with explanatory comments (2000 for current, 500 for context)
- Limiting context to just the 2 latest messages improves efficiency
- Properly filters out the current message to avoid duplication
This is a good example of extracting complex logic into a focused helper function.
apps/web/utils/ai/clean/ai-clean.ts (1)
10-17: Adoption ofpreprocessBooleanLikeforarchivefield
Switching from a directz.boolean()toz.preprocess(preprocessBooleanLike, z.boolean())is a neat solution for handling string representations of booleans. This adds flexibility while preserving clarity in howarchiveis interpreted.apps/web/utils/zod.ts (2)
9-9: Confirming the transform logic inzodTrueFalse
The transform function properly maps"true"to a booleantrue. No issues found here.
10-23: Introduction ofpreprocessBooleanLike
The logic correctly handles common boolean-like strings ("true","false","yes","no"). The documentation is thorough and consistent with the intended usage.apps/web/utils/ai/reply/check-if-needs-reply.ts (10)
11-11: ImportingpreprocessBooleanLike
This import aligns with the new schema changes and guarantees consistent handling of boolean-like input across modules.
15-23: New schema for AI reply check
Defining bothrationale(string) andneedsReply(preprocessed boolean) is a clean approach that provides clarity on the decision-making process.
28-29: Destructuring in function parameters
ExposingmessageToSendandthreadContextMessagesat the top level simplifies the function signature. Nicely done.
32-34: Typing considerations for new parameters
EnsuringmessageToSendandthreadContextMessagesconform toEmailForLLMfosters strong type safety and consistency.
35-37: Null-check fallback
Returning a default object ifmessageToSendis missing helps avoid runtime errors. Good defensive approach.
39-39: Explicitly defininguserMessageForPrompt
This clarifies the variable’s purpose when building prompts, improving readability.
46-46: Prompt context improvement
Adding explanatory text ("We are sending the following message:") ensures the AI has clear instructions on the content it’s evaluating.
49-49: Use ofstringifyEmailSimple
Its usage here remains consistent with other parts of the codebase, ensuring uniform email formatting for the AI.
52-62: Thread context inclusion
Conditionally including prior messages for contextual awareness is a sound practice. Consider monitoring performance for especially large threads.
80-80: Casting AI result toAICheckResult
The explicit type cast clarifies expected structure and usage in the rest of the application.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/utils/llms/model.ts (2)
159-161: Add comprehensive error handling for missing economy LLM configurationThe function silently falls back to the default model if the API key is missing, but it doesn't log this behavior which could make troubleshooting difficult.
if (env.ECONOMY_LLM_PROVIDER && env.ECONOMY_LLM_MODEL) { const apiKey = getProviderApiKey(env.ECONOMY_LLM_PROVIDER); - if (!apiKey) return selectModel(userAi); + if (!apiKey) { + logger.warn("Economy LLM provider configured but API key not found", { + provider: env.ECONOMY_LLM_PROVIDER, + }); + return selectModel(userAi); + }
28-32: Consider adding more context to the log messageThe current log message contains basic information. Adding more context would be helpful for debugging.
logger.trace("Using model", { useEconomyModel, provider: model.provider, model: model.model, + isEconomy: useEconomyModel && model.provider === env.ECONOMY_LLM_PROVIDER, + isDefault: model.provider === getDefaultProvider(), });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/utils/ai/actions.ts(5 hunks)apps/web/utils/gmail/label.ts(1 hunks)apps/web/utils/llms/model.ts(2 hunks)apps/web/utils/reply-tracker/outbound.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/ai/actions.ts
🧰 Additional context used
🧬 Code Definitions (1)
apps/web/utils/reply-tracker/outbound.ts (2)
apps/web/utils/reply-tracker/label.ts (1)
getReplyTrackingLabels(58-71)apps/web/utils/ai/reply/check-if-needs-reply.ts (1)
aiCheckIfNeedsReply(26-81)
🔇 Additional comments (21)
apps/web/utils/gmail/label.ts (1)
49-50: Good addition for improved observability.Adding trace-level logging to the
labelThreadfunction provides valuable debugging information without affecting normal operation. This change aligns well with existing logging patterns and will help with troubleshooting issues related to Gmail label operations.apps/web/utils/reply-tracker/outbound.ts (16)
3-3: Appropriate type import enhancementThe import statement has been updated to include
EmailForLLMwhich is used in the newly added functions. This keeps the imports properly aligned with the code's needs.
29-34: Early validation of feature enablement improves efficiencyThe new approach of checking if outbound tracking is enabled at the beginning of the function allows for an early exit, preventing unnecessary processing. This is a good practice that follows the fail-fast principle.
38-38: Clear step numbering improves readabilityAdding numbered comments for each processing step makes the code more maintainable and easier to follow. This is especially valuable in a complex function with multiple operations.
42-42: Descriptive comment clarifies purposeThe comment clearly describes that this step is resolving existing trackers, which helps developers understand the intended functionality.
45-45: Parameter ordering consistencyThe function call to
resolveReplyTrackersnow correctly referencesuser.idinstead of a previous inconsistent reference.
50-50: Descriptive step commentClear labeling of the thread context retrieval step improves readability.
53-53: Improved error loggingChanged to use
errorlevel logging which is appropriate for a condition that prevents further processing.
57-68: Effective message sequence validationThe extracted
isMessageLatestInThreadfunction provides a clean way to verify if the current message is the most recent one in the thread. Early exit with appropriate logging if not the latest message prevents unnecessary processing.
70-73: Well-structured data preparation logicExtracting the data preparation logic into a separate function improves code organization and makes the main function more readable. The function has a clear purpose and returns well-named objects.
74-79: Clean AI check implementationUsing the prepared data for the AI check makes the code more readable and maintainable. The variable names clearly indicate their purpose.
81-82: Clear intent with numbered commentThe commented step number and descriptive text make it easy to follow the processing flow.
86-86: Consistent property referencingUsing
user.iddirectly is more consistent with the rest of the codebase.
94-94: Appropriate log level adjustmentChanged to
tracelevel for the "No need to reply" message, which is appropriate for this informational output that doesn't require attention.
186-192: Clean implementation of feature flag checkThe
isOutboundTrackingEnabledfunction provides a clean way to check if the outbound reply tracking feature is enabled for a user. It properly handles the case where the user settings might not exist.
194-217: Robust thread message validationThe
isMessageLatestInThreadfunction:
- Handles empty thread messages gracefully
- Uses a non-mutating sort by creating a new array
- Provides detailed logging when the message is not the latest
- Returns both the boolean result and the sorted messages for reuse
219-245: Well-structured data preparation functionThe
prepareDataForAICheckfunction:
- Creates appropriately sized message content for the AI check
- Uses different max lengths for the current message vs. context messages
- Properly filters out the current message from context
- Limits context to the 2 most recent messages
- Returns a clean object with well-named properties
apps/web/utils/llms/model.ts (4)
12-14: Good addition of structured loggingAdding scoped logging will help with debugging and tracing model selection, which is particularly valuable when troubleshooting issues in a complex LLM integration.
16-35: Clean implementation of economy model optionThe updated
getModelfunction with the optionaluseEconomyModelparameter is well-structured. The logging of model selection details will be helpful for debugging.
148-171: Well-documented economy model selectionThe
selectEconomyModelfunction is clearly documented with specific use cases, making it easy for other developers to understand when to use it. The implementation properly handles fallback scenarios when configuration is missing.
173-192: Provider API key function missing some providersThe getProviderApiKey function doesn't include mappings for all providers that appear in the Provider object - specifically 'bedrock' and 'ollama' are missing from the providerApiKeys mapping.
function getProviderApiKey( provider: | "openai" | "anthropic" | "google" | "groq" | "openrouter" | "bedrock" | "ollama", ) { const providerApiKeys = { [Provider.ANTHROPIC]: env.ANTHROPIC_API_KEY, [Provider.OPEN_AI]: env.OPENAI_API_KEY, [Provider.GOOGLE]: env.GOOGLE_API_KEY, [Provider.GROQ]: env.GROQ_API_KEY, [Provider.OPENROUTER]: env.OPENROUTER_API_KEY, + bedrock: env.BEDROCK_ACCESS_KEY && env.BEDROCK_SECRET_KEY ? "available" : undefined, + [Provider.OLLAMA]: env.OLLAMA_BASE_URL ? "available" : undefined, }; return providerApiKeys[provider]; }For 'bedrock' and 'ollama', we're not returning actual API keys but indicators of availability since they use different authentication mechanisms.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/utils/llms/helpers.ts (1)
1-3: Consider making the function more testable.To improve testability, consider accepting an optional
dateparameter:-export function getTodayForLLM() { +export function getTodayForLLM(date: Date = new Date()) { - return `Today's date and time is: ${new Date().toISOString()}.`; + return `Today's date and time is: ${date.toISOString()}.`; }This change would make unit testing easier by allowing injection of specific dates for predictable test outcomes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/env.ts(1 hunks)apps/web/utils/llms/helpers.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/env.ts
🔇 Additional comments (1)
apps/web/utils/llms/helpers.ts (1)
1-3: Good implementation of date formatting for LLM context.This function effectively centralizes date formatting logic for LLM context, which enhances maintainability across the codebase.
Summary by CodeRabbit
New Features
UI Improvements
Performance & Reliability