Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughCentralizes onboarding into a callback-driven seven-step flow, converts category/rule shapes to an array-based model, adds persona API/hooks and server actions, updates Prisma with personaAnalysis and role, refactors rule creation/validation and categoryConfig, and updates onboarding UI primitives and provider helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OnboardingContent
participant Analytics
participant PersonaAPI as /api/user/persona
participant EmailActions as email-account actions
User->>OnboardingContent: Open onboarding (step N)
OnboardingContent->>Analytics: onStart()
OnboardingContent->>PersonaAPI: GET /api/user/persona
alt no personaAnalysis
OnboardingContent->>EmailActions: analyzePersonaAction(emailAccountId)
EmailActions-->>PersonaAPI: persist personaAnalysis
PersonaAPI-->>OnboardingContent: updated data (mutate)
end
User->>OnboardingContent: Click Continue
OnboardingContent->>Analytics: onNext(step+1)
OnboardingContent-->>User: Advance via onNext callback
sequenceDiagram
participant User
participant CategoriesSetup
participant StepLabels
participant RuleAction as createRulesOnboardingAction
participant Server
User->>CategoriesSetup: Configure categories (categories[])
User->>StepLabels: Submit categories
StepLabels->>RuleAction: submit(categories[])
RuleAction->>Server: create/update system & custom rules, update prompts
Server-->>RuleAction: results
RuleAction-->>StepLabels: done
StepLabels-->>User: Advance via onNext
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (8)
apps/web/utils/ai/knowledge/persona.ts (3)
87-95: Prefer XML-like structure in the prompt and avoid duplicating the email address.You already use tags for emails; wrap the task directive similarly and avoid repeating the address in free text. This tightens structure and reduces prompt ambiguity.
Apply this diff to re-structure the changed prompt segment:
- const prompt = `The user's email address is: ${emailAccount.email} - -This is important: You are analyzing the persona of ${emailAccount.email}. Look at what they write about, how they communicate, and who they interact with to determine their professional role. - -Here are the emails they've sent: -<emails> -${emails.map((e) => `<email>\n${stringifyEmail(e, 1000)}\n</email>`).join("\n")} -</emails>`; + const prompt = `<user> +<email>${emailAccount.email}</email> +</user> +<task> +Analyze the persona of the user above. Determine their professional role by examining what they write about, how they communicate, and who they interact with. +</task> +<emails> +${emails.map((e) => `<email>\n${stringifyEmail(e, 1000)}\n</email>`).join("\n")} +</emails>`;
28-33: Constrain responsibilities to 3–5 items to guide the LLM output.The description asks for 3–5 responsibilities, but the schema doesn’t enforce it. Add min/max constraints to improve output reliability.
Outside the changed lines, update the schema as follows:
responsibilities: z .array(z.string().min(1)) .min(3) .max(5) .describe( "An array of 3-5 key responsibilities evident from their email patterns and communications", ),
104-111: Add retry logic around the model call to handle transient failures.Per our AI guidelines, add retries with jitter/backoff. You already depend on p-retry.
Outside the changed lines, consider:
import pRetry from "p-retry"; // ... const result = await pRetry( () => generateObject({ ...modelOptions, system, prompt, schema: personaAnalysisSchema, }), { retries: 2, factor: 2, minTimeout: 500, maxTimeout: 2500, }, );Also consider logging success/failure at debug level with non-PII metadata (email count, model name).
apps/web/utils/ai/choose-rule/ai-choose-rule.ts (1)
87-97: Tighten the schema to reduce empty or verbose responses.Make strings non-empty and keep the reason concise, aligning with your instructions.
Apply this diff to the schema:
- schema: z.object({ - reason: z - .string() - .describe("The reason you chose the rule. Keep it concise"), - ruleName: z - .string() - .describe("The exact name of the rule you want to apply"), - noMatchFound: z - .boolean() - .describe("True if no match was found, false otherwise"), - }), + schema: z.object({ + reason: z + .string() + .min(1) + .max(200) + .describe("The reason you chose the rule. Keep it concise"), + ruleName: z + .string() + .min(1) + .describe("The exact name of the rule you want to apply"), + noMatchFound: z + .boolean() + .describe("True if no match was found, false otherwise"), + }),apps/web/utils/actions/cold-email.ts (1)
106-132: Guard against missing ctx.provider (runtime failure risk) and ensure safe-action client binds itProvider was previously sourced from the DB; now it’s read from ctx. If the client forgets to bind provider,
createEmailProvidermay throw. Add an explicit guard (and optionally a fallback) to fail fast with a clear error.Apply this minimal guard to prevent silent failures:
async ({ - ctx: { emailAccountId, provider }, + ctx: { emailAccountId, provider }, parsedInput: { from, subject, textHtml, textPlain, snippet, threadId, messageId, date, }, }) => { const emailAccount = await prisma.emailAccount.findUnique({ where: { id: emailAccountId }, include: { user: { select: { aiProvider: true, aiModel: true, aiApiKey: true } }, }, }); if (!emailAccount) throw new SafeError("Email account not found"); + if (!provider) { + throw new SafeError( + "Missing provider in action context. Ensure the safe action client binds `provider` when invoking this action.", + ); + } + const emailProvider = await createEmailProvider({ emailAccountId, - provider, + provider, });If you want a resilient fallback instead of throwing, include
account: { select: { provider: true } }in thefindUniqueinclude and useprovider ?? emailAccount.account?.provider. I can draft that patch if preferred.apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx (1)
41-51: Schema/typing mismatch: CreateRulesOnboardingBody is array-based, but component treats it like a keyed object.The new CreateRulesOnboardingBody is an array (see rule.validation), yet the component still uses keyed semantics via keyof and per-field IDs. This leads to incorrect typing and brittle assertions. Since this UI no longer edits per-category actions, consider removing keyed props and aligning to the array shape, or migrate to the new OnboardingCategories flow.
Apply this diff to decouple from keyed types and avoid misleading assertions:
-}: { - defaultValues: CreateRulesOnboardingBody; -}) { +}: { + defaultValues: CreateRulesOnboardingBody; +}) { const router = useRouter(); const { emailAccountId } = useAccount(); const [showExampleDialog, setShowExampleDialog] = useState(false); const form = useForm<CreateRulesOnboardingBody>({ resolver: zodResolver(createRulesOnboardingBody), - defaultValues, + defaultValues, });apps/web/app/(app)/[emailAccountId]/onboarding/StepWho.tsx (1)
60-83: Scrolling to selected role uses mismatched indexing and may scroll to the wrong item.You compute the selected index from USER_ROLES but render roles from usersRolesInfo (object entry order can differ). Prefer selecting by data attribute.
- useEffect(() => { - if (defaultRole && scrollContainerRef.current) { - // Find the button with the selected role - const selectedIndex = USER_ROLES.findIndex( - (role) => role.value === defaultRole, - ); - if (selectedIndex !== -1) { - const buttons = scrollContainerRef.current.querySelectorAll( - 'button[type="button"]', - ); - const selectedButton = buttons[selectedIndex]; - if (selectedButton) { - // Use setTimeout to ensure the DOM is ready - setTimeout(() => { - selectedButton.scrollIntoView({ - behavior: "smooth", - block: "center", - }); - }, 100); - } - } - } - }, [defaultRole]); + useEffect(() => { + if (defaultRole && scrollContainerRef.current) { + const selectedButton = scrollContainerRef.current.querySelector( + `button[data-role="${CSS.escape(defaultRole)}"]`, + ) as HTMLButtonElement | null; + if (selectedButton) { + setTimeout(() => { + selectedButton.scrollIntoView({ behavior: "smooth", block: "center" }); + }, 100); + } + } + }, [defaultRole]); ... - {Object.entries(usersRolesInfo).map(([roleName, role]) => { + {Object.entries(usersRolesInfo).map(([roleName, role]) => { const Icon = role.icon; const description = USER_ROLES.find( (r) => r.value === roleName, )?.description; return ( <button type="button" key={roleName} + data-role={roleName} className={cn( "rounded-xl border bg-card p-4 text-card-foreground shadow-sm text-left flex items-center gap-4 transition-all", watchedRole === roleName && "border-blue-600 ring-2 ring-blue-100", )} onClick={() => { setValue("role", roleName); if (roleName !== "Other") { setCustomRole(""); } }} >Also applies to: 141-169
apps/web/app/api/user/categorization-preferences/route.ts (1)
51-71: Action required: onboarding name mismatch — GET returns RuleName (display) but onboarding action expects lowercase keysShort: getUserPreferences() (GET) returns category.name values from RuleName (e.g. "Newsletter", "Cold Email"), but createRulesOnboardingAction expects the legacy lowercase keys ("newsletter", "coldEmail", "toReply", ...). This will cause the assistant onboarding flow (which fetches defaults from the GET and posts them back) to treat system categories as custom and skip applying settings.
Files to fix / inspect:
- apps/web/app/api/user/categorization-preferences/route.ts — GET returns name: RuleName.*.
- apps/web/utils/rule/consts.ts — RuleName values are display strings ("Newsletter", "Cold Email", ...).
- apps/web/utils/actions/rule.ts — createRulesOnboardingAction uses systemCategories = ["toReply","newsletter",..."] and later indexes systemCategoryMap with RuleName.* (mismatch).
- apps/web/app/(app)/[emailAccountId]/assistant/onboarding/page.tsx — fetches GET and passes defaultValues to the assistant form.
- apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx — submits form data to createRulesOnboardingAction.
- apps/web/utils/category-config.tsx — UI uses lowercase keys (categoryConfig.key).
Suggested fixes (pick one):
- Preferable: normalize incoming names in createRulesOnboardingAction (map display names → legacy keys) before building systemCategoryMap, so the action accepts both display and legacy keys and remains backward-compatible.
- Alternative: change the GET to return the lowercase category keys instead of RuleName display strings (make GET align with what the action expects).
Example normalization approach (implement in createRulesOnboardingAction):
- Build a mapping from RuleName.* → lowercase key, normalize category.name = mapping[category.name] ?? category.name, then proceed using the normalized name and use the lowercase keys when reading systemCategoryMap.
🧹 Nitpick comments (28)
apps/web/app/(landing)/welcome/survey.ts (2)
75-75: Leverage literal inference withas constfor stronger types and immutability.This will infer literal unions for the question keys/types and the choice strings, improving DX and preventing accidental mutation.
-}; +} as const;
60-62: Minor copy nit: “HackerNews” ➜ “Hacker News”.Common styling uses a space. Optional but improves polish.
- "HackerNews", + "Hacker News",apps/web/utils/ai/choose-rule/ai-choose-rule.ts (1)
118-173: Define and use a stable return type for aiChooseRule.Current early return shape differs from the main return (missing rule). Define a single result type to make consumers safer.
Outside the changed lines, consider:
export type ChooseRuleResult<T> = { rule?: T; reason: string }; export async function aiChooseRule<T extends { name: string; instructions: string }>(/* ... */): Promise<ChooseRuleResult<T>> { // ... if (!rules.length) return { reason: "No rules" }; // ... if (aiResponse.noMatchFound) return { rule: undefined, reason: "No match found" }; // ... return { rule: selectedRule, reason: aiResponse?.reason ?? "" }; }apps/web/prisma/schema.prisma (1)
96-116: Consider indexing role if you’ll query/filter by it.If onboarding or analytics need fast lookups by EmailAccount.role (e.g., suggesting labels by role), add a simple index to avoid future full scans.
Outside the changed lines, you could add:
@@index([role])to the EmailAccount model.
apps/web/app/(app)/[emailAccountId]/onboarding/ContinueButton.tsx (1)
15-21: Decide whether consumers can override size/variant; adjust spread order accordinglyCurrently
{...props}comes aftersizeandvariant, so consumers can override these defaults. If you intend to enforcesm+primaryBlue, place{...props}first.Apply this diff if you want to make size/variant authoritative:
-export function ContinueButton(props: ButtonProps) { - return ( - <Button size="sm" variant="primaryBlue" {...props}> +export function ContinueButton(props: ButtonProps) { + return ( + <Button {...props} size="sm" variant="primaryBlue"> Continue <ArrowRightIcon className="size-4 ml-2" /> </Button> ); }apps/web/app/(app)/[emailAccountId]/assistant/Rules.tsx (1)
390-391: Nice UX improvement: include rule name in delete confirmationIncluding the rule name makes the action clearer to users.
If rule names may contain quotes, consider escaping them to avoid awkward visuals:
-`Are you sure you want to delete the rule "${rule.name}"?` +`Are you sure you want to delete the rule "${rule.name.replaceAll('"', '\\"')}"?`apps/web/app/(app)/[emailAccountId]/onboarding/StepIntro.tsx (1)
69-69: Consider the UX impact of switching from viewport-relative to fixed height.Changing from
h-[50vh]toh-[400px]provides consistency but may impact user experience on smaller screens where 400px could be too large, or on very large screens where it might appear too small compared to the previous 50% viewport height.Consider using responsive height classes to maintain good UX across different screen sizes:
- <CardBasic className="rounded-2xl shadow-none grid grid-cols-5 p-0 pl-4 pt-4 gap-8 h-[400px]"> + <CardBasic className="rounded-2xl shadow-none grid grid-cols-5 p-0 pl-4 pt-4 gap-8 h-[300px] sm:h-[400px] lg:h-[450px]">apps/web/app/(app)/[emailAccountId]/onboarding/page.tsx (1)
17-30: Clean up commented authentication code.The commented-out authentication and user validation code should be removed if it's no longer needed, or properly implemented if authentication is required for the onboarding flow.
Consider removing the commented code if authentication will be handled elsewhere:
- // const session = await auth(); - - // if (!session?.user) redirect("/login"); - - // const user = await prisma.user.findUnique({ - // where: { id: session.user.id }, - // select: { completedAppOnboardingAt: true, surveyRole: true }, - // }); - - // if (!user) redirect("/login"); - - // if (!searchParams.force && user.completedAppOnboardingAt) { - // redirect(env.NEXT_PUBLIC_APP_HOME_PATH); - // }apps/web/utils/actions/rule.validation.ts (1)
219-219: Consider renaming exported type to avoid confusion with UI CategoryConfigThis file exports a CategoryConfig type that may be conflated with UI-facing categoryConfig from apps/web/utils/category-config.tsx. Consider a more specific name (e.g., RulesOnboardingCategoryConfig) to reduce ambiguity across layers.
apps/web/app/(app)/[emailAccountId]/onboarding/StepLabels.tsx (1)
32-38: Add responsive sizes to Next/Image for better performanceSince the image is hidden below xl and fixed at 50vw on xl+, provide a sizes prop so Next can serve optimally sized images.
<Image src="/images/assistant/labels.png" alt="Categorize your emails" width={1200} height={800} - className="mx-auto rounded border-4 border-blue-50 shadow-sm" + className="mx-auto rounded border-4 border-blue-50 shadow-sm" + sizes="(min-width: 1280px) 50vw, 0px" />apps/web/app/(app)/[emailAccountId]/onboarding/config.ts (1)
21-27: Give role keys a typed union for stronger type-safetyUsing Record<string, ...> allows typos and makes lookups less safe. Define a RoleName union and use it here. This preserves DX and refactors without runtime impact.
Apply this diff:
-export const usersRolesInfo: Record< - string, - { - icon: LucideIcon; - suggestedLabels?: { label: string; description: string }[]; - } -> = { +export const usersRolesInfo: Record< + RoleName, + { + icon: LucideIcon; + suggestedLabels?: { label: string; description: string }[]; + } +> = {And add these declarations above usersRolesInfo:
export const USER_ROLE_NAMES = [ "Founder", "Executive", "Small Business Owner", "Software Engineer", "Assistant", "Investor", "Sales", "Marketing", "Customer Support", "Realtor", "Content Creator", "Consultant", "E-commerce", "Student", "Individual", "Other", ] as const; export type RoleName = (typeof USER_ROLE_NAMES)[number];apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx (3)
93-103: Remove unused/incorrect props when rendering CategoryCard (id, form).CategoryCard no longer consumes id/form and the id type is incorrect for an array schema. Keep the props surface minimal and consistent.
{categoryConfig.map((category) => ( <CategoryCard key={category.key} - id={category.key as keyof CreateRulesOnboardingBody} label={category.label} tooltipText={category.tooltipText} Icon={category.Icon} iconColor={category.iconColor} - form={form} /> ))}
56-59: Explicitly mark fire-and-forget server action to avoid unhandled Promise rejections.The action is intentionally not awaited. Prefix with void so linters don’t flag unhandled Promises.
- createRulesOnboardingAction(emailAccountId, data); + void createRulesOnboardingAction(emailAccountId, data);
29-35: Remove unused import and dead/commented code.
- useDelayedActionsEnabled is imported but unused.
- Large commented block adds noise. Prefer removing it or adding a TODO with a clear follow-up.
-import { useDelayedActionsEnabled } from "@/hooks/useFeatureFlags";And consider deleting the long commented FormField block to keep this component lean.
Also applies to: 144-146, 160-202
apps/web/utils/actions/email-account.ts (2)
12-20: Consider cache invalidation after updating email account role.If any pages or SWR keys depend on EmailAccount.role, revalidate relevant paths/tags to reflect changes immediately.
Example:
+import { revalidatePath } from "next/cache"; ... await prisma.emailAccount.update({ where: { id: emailAccountId }, data: { role }, }); + revalidatePath(`/${emailAccountId}/onboarding`);Adjust the path/tag to match your data fetching strategy (pages vs. route handlers vs. SWR tags).
47-53: Verify pagination limits and prompt size to control cost and latency.
- getMessagesWithPagination({ maxResults: 200 }) may be heavy for some accounts. Consider making this configurable or sampling.
- getEmailForLLM with maxLength: 2000 could still produce large prompts when multiplied across 200 emails.
Do you want me to add an optional input schema to analyzePersonaAction to accept maxResults and a sampling strategy and wire it through? Happy to send a follow-up diff.
Also applies to: 61-66
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingCategories.tsx (4)
81-89: Mark background action as intentional and consider basic error handling.Prefix with void to avoid unhandled Promise warnings and keep the intent clear. If you ever need UX feedback, await and use toastError/toastSuccess per guidelines.
- // runs in background so we can move on to next step faster - createRulesOnboardingAction(emailAccountId, allCategories); + // runs in background so we can move on to next step faster + void createRulesOnboardingAction(emailAccountId, allCategories);
180-182: Button is outside a form; use type="button" to avoid misleading semantics.There’s no
here, so submit type is unnecessary.- <ContinueButton type="submit" onClick={onSubmit} /> + <ContinueButton type="button" onClick={onSubmit} />
146-163: Avoid O(n^2) lookups and ensure stable keys.
- Repeated find inside map is minor here but easy to avoid with a map/dictionary.
- Use a stable unique key (config.key) instead of label.
- {basicCategories.map((category, index) => { - const config = categoryConfig.find((c) => c.key === category.name); + {basicCategories.map((category, index) => { + const config = categoryConfig.find((c) => c.key === category.name); if (!config) return null; return ( <CategoryCard - key={config.label} + key={config.key} index={index} label={config.label} description={config.tooltipText} Icon={config.Icon} iconColor={config.iconColor} update={updateBasicCategory} value={category.action} /> ); })}Optional: Precompute a map: const configMap = useMemo(() => new Map(categoryConfig.map(c => [c.key, c])), []);
46-48: Remove stale “copy paste of old file” comment.Keeps the file clean and avoids confusion.
-// copy paste of old file export function CategoriesSetup() {apps/web/app/(app)/[emailAccountId]/onboarding/StepWho.tsx (2)
31-34: Fix redundant prop typing in function signature.emailAccountId is already part of StepWhoProps. Remove the duplicate intersection.
-export function StepWho({ - initialRole, - emailAccountId, -}: StepWhoProps & { emailAccountId: string }) { +export function StepWho({ initialRole, emailAccountId }: StepWhoProps) {
103-128: Add basic error handling on submission.Per guidelines, prefer handling server action results (result?.serverError) and showing toastError/toastSuccess. Currently, any exception will crash the handler or silently navigate. Consider using next-safe-action’s client result pattern.
I can wire this up to use action results and toasts if you share the project’s toast utilities import path.
apps/web/utils/actions/rule.ts (5)
668-679: Local function name shadows importeddeleteRule(readability/maintainability).The inner
async function deleteRule(...)shadows the importeddeleteRuleused earlier in this file. Rename to avoid confusion.Apply this diff to rename the local function:
- async function deleteRule(systemType: SystemType, emailAccountId: string) { + async function deleteSystemRule(systemType: SystemType, emailAccountId: string) {And update its call sites accordingly in this block (e.g., where you currently call
deleteRule(SystemType.NEWSLETTER, emailAccountId)etc.).
511-512: Type nit: UsePromise<void>[]instead ofPromise<any>[].Avoids leaking
anyand clarifies intent that side-effects are awaited but no values are consumed.Apply this diff:
- const promises: Promise<any>[] = []; + const promises: Promise<void>[] = [];
543-557: Background fetch lacks error capture; consider wrapping to avoid unhandled rejections in edge cases.The fire-and-forget
fetchruns in the.thenchain and isn't awaited. If it fails (e.g.,NEXT_PUBLIC_BASE_URLmisconfig), errors are dropped. Add a.catchwithlogger.errorfor observability.Apply this diff:
- fetch( + fetch( `${env.NEXT_PUBLIC_BASE_URL}/api/reply-tracker/process-previous`, { method: "POST", headers: { "Content-Type": "application/json", [INTERNAL_API_KEY_HEADER]: env.INTERNAL_API_KEY, }, body: JSON.stringify({ emailAccountId, } satisfies ProcessPreviousBody), }, - ); + ).catch((error) => { + logger.error("Failed to process previous replies", { error }); + });
786-794: Consider revalidating assistant/automation pages after onboarding changes.Rules and prompts drive UI; revalidate relevant pages after the onboarding batch completes to ensure fresh state.
Apply this diff:
await prisma.emailAccount.update({ where: { id: emailAccountId }, data: { rulesPrompt: `${emailAccount.rulesPrompt || ""}\n${rules .map((r) => `* ${r}`) .join("\n")}`.trim(), }, }); + revalidatePath(prefixPath(emailAccountId, "/assistant")); + revalidatePath(prefixPath(emailAccountId, "/automation"));
395-452: Delete action: minor cleanup opportunity and safety checks.
- The permission check on Line 402 is redundant since the initial
findUniqueincludesemailAccountIdin the where clause. Not harmful, but unnecessary.- Consider guarding against accidentally clearing
rulesPromptif the AI returns an empty string (defensive check), although current generator returns the existing prompt when it can't find the rule.You can safely remove the redundant permission check:
- if (rule.emailAccountId !== emailAccountId) - throw new SafeError("You don't have permission to delete this rule");Optionally, avoid setting an empty prompt:
- await prisma.emailAccount.update({ + if (updatedPrompt?.trim()) { + await prisma.emailAccount.update({ where: { id: emailAccountId }, data: { rulesPrompt: updatedPrompt }, - }); + }); + }apps/web/app/api/user/categorization-preferences/route.ts (1)
106-122: getToReplySetting: clarity suggestion.Currently returns
undefinedwhen no rules exist, relying on the spread-site fallback. Consider always returning aCategoryConfigto simplify callers and types, e.g.,{ action: "none", hasDigest: false }when disabled/missing.Example refactor (no diff applied here to keep behavior unchanged):
- Return
{ action: "none", hasDigest: false }instead ofundefinedon empty rules and when not found.- Do the same for
getRuleSettingandgetColdEmailSetting.
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/CategoriesSetup.tsx
Show resolved
Hide resolved
| RuleName.Notification, | ||
| "Notifications: Alerts, status updates, or system messages", | ||
| "Label all notifications as @[Notifications]", | ||
| false, | ||
| notification.action, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent label: "Notifications" vs "Notification".
Prompt text uses "@[Notifications]" but the created label is "Notification". This will misalign prompt vs actual label.
Apply this diff to keep them consistent:
- "Label all notifications as @[Notifications]",
+ "Label all notifications as @[Notification]",📝 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.
| RuleName.Notification, | |
| "Notifications: Alerts, status updates, or system messages", | |
| "Label all notifications as @[Notifications]", | |
| false, | |
| notification.action, | |
| RuleName.Notification, | |
| "Notifications: Alerts, status updates, or system messages", | |
| "Label all notifications as @[Notification]", | |
| false, | |
| notification.action, |
🤖 Prompt for AI Agents
In apps/web/utils/actions/rule.ts around lines 752 to 756, the prompt references
"@[Notifications]" but the created label string is "Notification", causing
mismatch; update the label string to "Notifications" (plural) so the prompt and
actual label match, i.e., replace "Notification" with "Notifications" for this
rule entry.
| const categoryConfig = z.object({ | ||
| action: categoryAction.optional(), | ||
| hasDigest: z.boolean().optional(), | ||
| action: categoryAction.nullish(), | ||
| hasDigest: z.boolean().nullish(), | ||
| name: z.string(), | ||
| description: z.string(), | ||
| }); | ||
| export type CategoryConfig = z.infer<typeof categoryConfig>; | ||
|
|
||
| export const createRulesOnboardingBody = z.object({ | ||
| toReply: categoryConfig, | ||
| newsletter: categoryConfig, | ||
| marketing: categoryConfig, | ||
| calendar: categoryConfig, | ||
| receipt: categoryConfig, | ||
| coldEmail: categoryConfig, | ||
| notification: categoryConfig, | ||
| }); | ||
| export const createRulesOnboardingBody = z.array(categoryConfig); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tighten onboarding schema: trim and bound strings; require at least one category
- Enforce trimmed, non-empty, and reasonably bounded name/description to meet our “validate type, format, and length” guideline.
- Ensure the onboarding payload has at least one category.
Apply this diff:
-const categoryConfig = z.object({
- action: categoryAction.nullish(),
- hasDigest: z.boolean().nullish(),
- name: z.string(),
- description: z.string(),
-});
+const categoryConfig = z.object({
+ action: categoryAction.nullish(),
+ hasDigest: z.boolean().nullish(),
+ name: z
+ .string()
+ .trim()
+ .min(1, "Please enter a name")
+ .max(40, "Please keep names under 40 characters"),
+ description: z
+ .string()
+ .trim()
+ .min(1, "Please enter a description")
+ .max(140, "Please keep descriptions under 140 characters"),
+});
export type CategoryConfig = z.infer<typeof categoryConfig>;
-export const createRulesOnboardingBody = z.array(categoryConfig);
+export const createRulesOnboardingBody = z
+ .array(categoryConfig)
+ .min(1, "Please add at least one category");📝 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 categoryConfig = z.object({ | |
| action: categoryAction.optional(), | |
| hasDigest: z.boolean().optional(), | |
| action: categoryAction.nullish(), | |
| hasDigest: z.boolean().nullish(), | |
| name: z.string(), | |
| description: z.string(), | |
| }); | |
| export type CategoryConfig = z.infer<typeof categoryConfig>; | |
| export const createRulesOnboardingBody = z.object({ | |
| toReply: categoryConfig, | |
| newsletter: categoryConfig, | |
| marketing: categoryConfig, | |
| calendar: categoryConfig, | |
| receipt: categoryConfig, | |
| coldEmail: categoryConfig, | |
| notification: categoryConfig, | |
| }); | |
| export const createRulesOnboardingBody = z.array(categoryConfig); | |
| const categoryConfig = z.object({ | |
| action: categoryAction.nullish(), | |
| hasDigest: z.boolean().nullish(), | |
| name: z | |
| .string() | |
| .trim() | |
| .min(1, "Please enter a name") | |
| .max(40, "Please keep names under 40 characters"), | |
| description: z | |
| .string() | |
| .trim() | |
| .min(1, "Please enter a description") | |
| .max(140, "Please keep descriptions under 140 characters"), | |
| }); | |
| export type CategoryConfig = z.infer<typeof categoryConfig>; | |
| export const createRulesOnboardingBody = z | |
| .array(categoryConfig) | |
| .min(1, "Please add at least one category"); |
🤖 Prompt for AI Agents
In apps/web/utils/actions/rule.validation.ts around lines 213 to 221, tighten
the onboarding schema by changing name and description to trimmed, non-empty,
length-bounded strings (e.g., z.string().trim().min(1).max(100) for name and
z.string().trim().min(1).max(1000) for description) and require the onboarding
array to contain at least one category by using .min(1) on
createRulesOnboardingBody; keep action and hasDigest nullish as-is and update
the inferred types accordingly.
| export const categoryConfig: { | ||
| key: string; | ||
| label: string; | ||
| tooltipText: string; | ||
| Icon: React.ElementType; | ||
| iconColor: IconCircleColor; | ||
| }[] = [ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve CategoryKey union by switching to as const satisfies instead of a widening type
The explicit array type annotation widens key to string, so CategoryKey becomes string. Use satisfies with as const to preserve literal unions and keep strong typing across the app.
Apply this diff:
-export const categoryConfig: {
- key: string;
- label: string;
- tooltipText: string;
- Icon: React.ElementType;
- iconColor: IconCircleColor;
-}[] = [
+export const categoryConfig = [
@@
-];
+] as const satisfies readonly {
+ key: string;
+ label: string;
+ tooltipText: string;
+ Icon: React.ElementType;
+ iconColor: IconCircleColor;
+}[];Also applies to: 70-73
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/web/utils/category-config.tsx (1)
13-20: Preserve CategoryKey union by switching toas const satisfies(prevents key from widening tostring)With the explicit array type, the
keyproperty widens tostringandCategoryKeybecomesstring, losing strong typing. Useas const satisfiesto constrain the array while preserving literal keys.-export const categoryConfig: { - key: string; - label: string; - tooltipText: string; - Icon: React.ElementType; - iconColor: IconCircleColor; - action: CategoryAction; -}[] = [ +export const categoryConfig = [ @@ -]; +] as const satisfies readonly { + key: string; + label: string; + tooltipText: string; + Icon: React.ElementType; + iconColor: IconCircleColor; + action: CategoryAction; +}[];Also applies to: 79-81
🧹 Nitpick comments (1)
apps/web/app/(app)/[emailAccountId]/onboarding/config.ts (1)
25-26: Align icon type with the rest of the app for consistencyThis file uses
LucideIcon, whileapps/web/utils/category-config.tsxusesReact.ElementTypefor theIconfield. Consider standardizing on one (preferably the narrowerLucideIconif all icons come from lucide-react) to avoid friction in shared consumers.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingCategories.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/onboarding/config.ts(1 hunks)apps/web/utils/category-config.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingCategories.tsx
🧰 Additional context used
📓 Path-based instructions (13)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/config.tsapps/web/utils/category-config.tsx
apps/web/app/**
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
NextJS app router structure with (app) directory
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/config.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit Inference Engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/config.tsapps/web/utils/category-config.tsx
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/config.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/config.tsapps/web/utils/category-config.tsx
apps/web/app/(app)/*/**
📄 CodeRabbit Inference Engine (.cursor/rules/page-structure.mdc)
Components for the page are either put in page.tsx, or in the apps/web/app/(app)/PAGE_NAME folder
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/config.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/config.tsapps/web/utils/category-config.tsx
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/config.tsapps/web/utils/category-config.tsx
apps/web/**/*.tsx
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
apps/web/**/*.tsx: Follow tailwindcss patterns with prettier-plugin-tailwindcss
Prefer functional components with hooks
Use shadcn/ui components when available
Ensure responsive design with mobile-first approach
Follow consistent naming conventions (PascalCase for components)
Use LoadingContent component for async data
Useresult?.serverErrorwithtoastErrorandtoastSuccess
UseLoadingContentcomponent to handle loading and error states consistently
Passloading,error, and children props toLoadingContent
Files:
apps/web/utils/category-config.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)
**/*.tsx: Use React Hook Form with Zod for validation
Validate form inputs before submission
Show validation errors inline next to form fields
Files:
apps/web/utils/category-config.tsx
apps/web/utils/**
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logic
Files:
apps/web/utils/category-config.tsx
**/*.{jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't destructure props inside JSX components in Solid projects.
Don't use both children and dangerouslySetInnerHTML props on the same element.
Don't use Array index in keys.
Don't assign to React component props.
Don't define React components inside other components.
Don't use event handlers on non-interactive elements.
Don't assign JSX properties multiple times.
Don't add extra closing tags for components without children.
Use <>...</> instead of ....
Don't insert comments as text nodes.
Don't use the return value of React.render.
Make sure all dependencies are correctly specified in React hooks.
Make sure all React hooks are called from the top level of component functions.
Don't use unnecessary fragments.
Don't pass children as props.
Use semantic elements instead of role attributes in JSX.
Files:
apps/web/utils/category-config.tsx
**/*.{html,jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{html,jsx,tsx}: Don't use or elements.
Don't use accessKey attribute on any HTML element.
Don't set aria-hidden="true" on focusable elements.
Don't add ARIA roles, states, and properties to elements that don't support them.
Only use the scope prop on elements.
Don't assign non-interactive ARIA roles to interactive HTML elements.
Make sure label elements have text content and are associated with an input.
Don't assign interactive ARIA roles to non-interactive HTML elements.
Don't assign tabIndex to non-interactive HTML elements.
Don't use positive integers for tabIndex property.
Don't include "image", "picture", or "photo" in img alt prop.
Don't use explicit role property that's the same as the implicit/default role.
Make static elements with click handlers use a valid role attribute.
Always include a title element for SVG elements.
Give all elements requiring alt text meaningful information for screen readers.
Make sure anchors have content that's accessible to screen readers.
Assign tabIndex to non-interactive HTML elements with aria-activedescendant.
Include all required ARIA attributes for elements with ARIA roles.
Make sure ARIA properties are valid for the element's supported roles.
Always include a type attribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden).
Always include a lang attribute on the html element.
Always include a title attribute for iframe elements.
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress.
Accompany onMouseOver/onMouseOut with onFocus/onBlur.
Include caption tracks for audio and video elements.
Make sure all anchors are valid and navigable.
Ensure all ARIA properties (aria-*) are valid.
Use valid, non-abstract ARIA roles for elements with ARIA roles.
Use valid ARIA state and property values.
Use valid values for the autocomplete attribute on input eleme...Files:
apps/web/utils/category-config.tsx🧠 Learnings (1)
📚 Learning: 2025-07-20T09:03:06.318Z
Learnt from: CR PR: elie222/inbox-zero#0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-07-20T09:03:06.318Z Learning: Applies to **/*.{ts,tsx} : Use as const instead of literal types and type annotations.Applied to files:
apps/web/utils/category-config.tsx🧬 Code Graph Analysis (1)
apps/web/utils/category-config.tsx (2)
apps/web/app/(app)/[emailAccountId]/onboarding/IconCircle.tsx (1)
IconCircleColor(55-55)apps/web/utils/actions/rule.validation.ts (1)
CategoryAction(211-211)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Static Code Analysis Js
- GitHub Check: Jit Security
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/web/app/(app)/[emailAccountId]/onboarding/config.ts (1)
3-20: *Lucide icon imports with Icon suffix are valid for your installed version — no change neededQuick check: apps/web/package.json pins lucide-react@0.511.0. The project added support for *Icon-suffixed aliases around v0.456.0, so RocketIcon, BriefcaseIcon, StoreIcon, etc. are supported in 0.511.0 and the import in apps/web/app/(app)/[emailAccountId]/onboarding/config.ts is fine.
Relevant spots:
- apps/web/package.json — "lucide-react": "0.511.0"
- apps/web/app/(app)/[emailAccountId]/onboarding/config.ts — current import uses RocketIcon, BriefcaseIcon, …
- Repo already uses suffixed imports in many files (e.g., apps/web/components/ViewEmailButton.tsx — MailIcon), indicating the naming style is in use.
If you prefer to follow the canonical docs, switch to unsuffixed names (Mail, Rocket, etc.) or add the provided module-declaration/re-export to opt into suffixed typings — otherwise no fix required.
apps/web/utils/category-config.tsx (1)
26-29: Resolved — categoryAction Zod enum includes required valuesI verified apps/web/utils/actions/rule.validation.ts: the categoryAction z.enum contains "label", "label_archive", "label_archive_delayed", and "none", so server-side validation already accepts the values used in apps/web/utils/category-config.tsx.
- Verified location: apps/web/utils/actions/rule.validation.ts — categoryAction z.enum (≈ lines 205–211)
| export const usersRolesInfo: Record< | ||
| string, | ||
| { | ||
| icon: LucideIcon; | ||
| suggestedLabels: { label: string; description: string }[]; | ||
| } | ||
| > = { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve literal role key union and make labels immutable using as const satisfies; also export RoleName
The current Record<string, ...> widens keys to string, so downstream code loses the role name union. This hurts type safety (e.g., consumers can use any string). Use as const satisfies to keep literal keys while constraining the value shape, and export a RoleName union for reuse.
Apply:
-export const usersRolesInfo: Record<
- string,
- {
- icon: LucideIcon;
- suggestedLabels: { label: string; description: string }[];
- }
-> = {
+export const usersRolesInfo = {
@@
-};
+} as const satisfies Record<
+ string,
+ {
+ icon: LucideIcon;
+ suggestedLabels: readonly { label: string; description: string }[];
+ }
+>;
+
+export type RoleName = keyof typeof usersRolesInfo;Also applies to: 225-226
🤖 Prompt for AI Agents
In apps/web/app/(app)/[emailAccountId]/onboarding/config.ts around lines 22 to
28 (and also apply same pattern at lines 225-226), usersRolesInfo is typed as
Record<string, ...> which widens keys to string; change usersRolesInfo to be a
const literal object whose keys remain literal by using "as const satisfies
Record<...>" so the shape is constrained but keys stay literal, ensure
suggestedLabels strings are made readonly by using literal tuples/objects (as
const) within the value, and export a RoleName type as "export type RoleName =
keyof typeof usersRolesInfo" for downstream reuse. Ensure the satisfies type
matches the existing value shape (icon: LucideIcon; suggestedLabels: readonly {
label: string; description: string }[]) and apply the same change at lines
225-226.
|
Claude finished @elie222's task —— View job Code Review: PR #682 - New Onboarding FlowI've completed a comprehensive review of this major refactor. This PR introduces a sophisticated multi-step onboarding flow with persona analysis and role-based suggestions. Here's my detailed analysis: 🏗️ Architecture & ImplementationStrengths:
Areas for Improvement:
🔒 Security ConsiderationsCritical Issues:
Recommendations:
⚡ Performance AnalysisPositive Aspects:
Potential Issues:
Recommendations:
🧹 Code QualityGood Practices:
Issues Identified:
🐛 Potential Bugs & Issues
📝 Specific Recommendations
✅ Overall AssessmentThis is a well-architected feature that significantly improves the user onboarding experience. The codebase follows good practices overall, but there are several security and reliability concerns that should be addressed before merging. Priority Actions:
The implementation shows good understanding of Next.js patterns and modern React practices. With the recommended fixes, this will be a solid addition to the application. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx (1)
84-87: Remove console usage in client code (silent failure intended)Per coding guidelines, avoid console usage. The comment already states this should fail silently.
Apply this diff:
- .catch((error) => { - // Fail silently - persona analysis is optional enhancement - console.error("Failed to analyze persona:", error); - }); + .catch(() => { + // Fail silently - persona analysis is optional enhancement + });
🧹 Nitpick comments (2)
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx (2)
69-74: Harden onCompleted: don’t block navigation if the action failsIf completedOnboardingAction throws, the user won’t navigate off the step. Wrap in try/catch and proceed regardless. Optionally surface a toast, but at minimum don’t trap the user.
Apply this diff:
const onCompleted = useCallback(async () => { analytics.onComplete(); markOnboardingAsCompleted(ASSISTANT_ONBOARDING_COOKIE); - await completedOnboardingAction(); - onNext(); + try { + await completedOnboardingAction(); + } catch { + // Optionally toastError here; failure should not block progression + } + onNext(); }, [onNext, analytics]);
31-46: Optional: build steps after callbacks for clarityThese closures reference onNext/onCompleted/data declared later. It works due to closures, but moving steps construction after the callbacks improves readability and reduces cognitive load.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
apps/web/app/**
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
NextJS app router structure with (app) directory
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
apps/web/**/*.tsx
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
apps/web/**/*.tsx: Follow tailwindcss patterns with prettier-plugin-tailwindcss
Prefer functional components with hooks
Use shadcn/ui components when available
Ensure responsive design with mobile-first approach
Follow consistent naming conventions (PascalCase for components)
Use LoadingContent component for async data
Useresult?.serverErrorwithtoastErrorandtoastSuccess
UseLoadingContentcomponent to handle loading and error states consistently
Passloading,error, and children props toLoadingContent
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
!{.cursor/rules/*.mdc}
📄 CodeRabbit Inference Engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)
**/*.tsx: Use React Hook Form with Zod for validation
Validate form inputs before submission
Show validation errors inline next to form fields
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
apps/web/app/(app)/*/**
📄 CodeRabbit Inference Engine (.cursor/rules/page-structure.mdc)
Components for the page are either put in page.tsx, or in the apps/web/app/(app)/PAGE_NAME folder
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
apps/web/app/(app)/*/**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/page-structure.mdc)
If you need to use onClick in a component, that component is a client component and file must start with 'use client'
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
apps/web/app/(app)/*/**/**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/page-structure.mdc)
If we're in a deeply nested component we will use swr to fetch via API
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
apps/web/app/**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
Components with
onClickmust be client components withuse clientdirective
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
**/*.{jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't destructure props inside JSX components in Solid projects.
Don't use both children and dangerouslySetInnerHTML props on the same element.
Don't use Array index in keys.
Don't assign to React component props.
Don't define React components inside other components.
Don't use event handlers on non-interactive elements.
Don't assign JSX properties multiple times.
Don't add extra closing tags for components without children.
Use <>...</> instead of ....
Don't insert comments as text nodes.
Don't use the return value of React.render.
Make sure all dependencies are correctly specified in React hooks.
Make sure all React hooks are called from the top level of component functions.
Don't use unnecessary fragments.
Don't pass children as props.
Use semantic elements instead of role attributes in JSX.
Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
**/*.{html,jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{html,jsx,tsx}: Don't use or elements.
Don't use accessKey attribute on any HTML element.
Don't set aria-hidden="true" on focusable elements.
Don't add ARIA roles, states, and properties to elements that don't support them.
Only use the scope prop on elements.
Don't assign non-interactive ARIA roles to interactive HTML elements.
Make sure label elements have text content and are associated with an input.
Don't assign interactive ARIA roles to non-interactive HTML elements.
Don't assign tabIndex to non-interactive HTML elements.
Don't use positive integers for tabIndex property.
Don't include "image", "picture", or "photo" in img alt prop.
Don't use explicit role property that's the same as the implicit/default role.
Make static elements with click handlers use a valid role attribute.
Always include a title element for SVG elements.
Give all elements requiring alt text meaningful information for screen readers.
Make sure anchors have content that's accessible to screen readers.
Assign tabIndex to non-interactive HTML elements with aria-activedescendant.
Include all required ARIA attributes for elements with ARIA roles.
Make sure ARIA properties are valid for the element's supported roles.
Always include a type attribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden).
Always include a lang attribute on the html element.
Always include a title attribute for iframe elements.
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress.
Accompany onMouseOver/onMouseOut with onFocus/onBlur.
Include caption tracks for audio and video elements.
Make sure all anchors are valid and navigable.
Ensure all ARIA properties (aria-*) are valid.
Use valid, non-abstract ARIA roles for elements with ARIA roles.
Use valid ARIA state and property values.
Use valid values for the autocomplete attribute on input eleme...Files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx🧠 Learnings (4)
📚 Learning: 2025-07-08T13:14:07.449Z
Learnt from: elie222 PR: elie222/inbox-zero#537 File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34 Timestamp: 2025-07-08T13:14:07.449Z Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.Applied to files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx📚 Learning: 2025-08-10T22:08:49.243Z
Learnt from: CR PR: elie222/inbox-zero#0 File: .cursor/rules/llm.mdc:0-0 Timestamp: 2025-08-10T22:08:49.243Z Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Use proper error types and logging in LLM-related code.Applied to files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx📚 Learning: 2025-07-20T09:03:06.318Z
Learnt from: CR PR: elie222/inbox-zero#0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-07-20T09:03:06.318Z Learning: Applies to **/*.{js,jsx,ts,tsx} : Don't use Math.min and Math.max to clamp values when the result is constant.Applied to files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx📚 Learning: 2025-07-18T17:27:58.249Z
Learnt from: CR PR: elie222/inbox-zero#0 File: .cursor/rules/server-actions.mdc:0-0 Timestamp: 2025-07-18T17:27:58.249Z Learning: Applies to apps/web/utils/actions/*.ts : Use `actionClient` when both authenticated user context and a specific `emailAccountId` are needed. The `emailAccountId` must be bound when calling the action from the client.Applied to files:
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx🧬 Code Graph Analysis (1)
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx (13)
apps/web/app/(app)/[emailAccountId]/onboarding/StepIntro.tsx (1)
StepIntro(11-58)apps/web/app/(app)/[emailAccountId]/onboarding/StepFeatures.tsx (1)
StepFeatures(56-135)apps/web/app/(app)/[emailAccountId]/onboarding/StepWho.tsx (1)
StepWho(24-202)apps/web/app/(app)/[emailAccountId]/onboarding/StepLabels.tsx (1)
StepLabels(10-50)apps/web/app/(app)/[emailAccountId]/onboarding/StepDraft.tsx (1)
StepDraft(12-83)apps/web/app/(app)/[emailAccountId]/onboarding/StepCustomRules.tsx (1)
StepCustomRules(10-53)apps/web/app/(app)/[emailAccountId]/onboarding/StepExtension.tsx (1)
StepExtension(12-69)apps/web/hooks/usePersona.ts (1)
usePersona(4-6)apps/web/hooks/useAnalytics.ts (1)
useOnboardingAnalytics(4-20)apps/web/utils/path.ts (1)
prefixPath(1-4)apps/web/utils/cookies.ts (2)
markOnboardingAsCompleted(4-6)ASSISTANT_ONBOARDING_COOKIE(1-1)apps/web/utils/actions/onboarding.ts (1)
completedOnboardingAction(10-17)apps/web/utils/actions/email-account.ts (1)
analyzePersonaAction(22-73)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Jit Security
- GitHub Check: claude-review
🔇 Additional comments (1)
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx (1)
76-83: No changes needed for analyzePersonaAction callThe
analyzePersonaActionis built withactionClient.bindArgsSchemas<[emailAccountId: z.ZodString]>([z.string()]), so it requires theemailAccountIdas its first (and only) argument. CallinganalyzePersonaAction(emailAccountId)is therefore correct.
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
Outdated
Show resolved
Hide resolved
|
Claude finished @elie222's task —— View job 🔍 Code Review CompleteI've conducted a comprehensive review of the new onboarding flow implementation. Here's my detailed feedback: ✅ Overall AssessmentThis is a well-architected implementation of a multi-step onboarding flow with AI-powered persona analysis. The code quality is high with good separation of concerns and proper error handling. 📋 Detailed Findings🏗️ Architecture & Design✅ Strengths:
🔒 Security Review✅ Secure Implementation:
🗄️ Database & Data Flow✅ Well Designed:
⚡ Performance Considerations✅ Optimized:
🐛 Potential Issues & RecommendationsMedium Priority Issues:
Low Priority Improvements:
🧪 Testing Coverage✅ Excellent Test Strategy:
💻 Code Quality✅ High Standards:
🎯 Summary & RecommendationsOverall: This is production-ready code with minor improvements suggested. Immediate Actions (Optional):
Future Enhancements:
Great work on this implementation! The persona analysis feature adds significant value while maintaining security and performance standards. 🚀 Review completed: Examined 15+ files including components, server actions, AI analysis, database schema, and tests. |
|
Claude finished @elie222's task —— View job Code Review: PR #682 - New onboarding flowFinal Review Summary ✅ This PR implements a comprehensive multi-step onboarding flow with AI-powered persona analysis. The implementation demonstrates solid engineering practices with room for some improvements. 🎯 Overall Architecture Assessment: 8/10Strengths:
Areas for improvement:
🐛 Critical Issues Found1. Import Error (Fixed)✅ StepIntro.tsx exists and is properly imported - no runtime error 2. Race Condition Risk
3. Schema Validation
🔒 Security Assessment: 7/10Good practices:
Concerns:
🚀 Performance Assessment: 8/10Optimizations:
Recommendations:
💾 Database Schema: 9/10Excellent additions:
Minor suggestions:
🎨 Code Quality: 8/10Strengths:
Improvements:
📋 Final RecommendationsHigh Priority
Medium Priority
Nice to Have
✅ Approval StatusThis PR represents a solid implementation of a complex feature with good architectural decisions. While there are some security and performance optimizations that should be addressed, the core implementation is sound and ready for deployment with the high-priority fixes. Recommendation: Approve with requested changes - address the critical issues above before merging. The onboarding flow will significantly improve user experience and the persona analysis feature adds valuable AI-powered insights. Great work on this comprehensive feature! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/utils/email/provider-types.ts (1)
5-7: Mirror the type predicate for Microsoft to improve DXSame rationale as above; helps TS narrow the value after checks.
-export function isMicrosoftProvider(provider: string | null | undefined) { +export function isMicrosoftProvider(provider: string | null | undefined): provider is "microsoft" { return provider === "microsoft"; }apps/web/providers/EmailProvider.tsx (1)
38-58: Tighten types in mapLabelColor (provider can be null/undefined; label shouldn’t be any)
- The signature currently accepts
provider: stringbut immediately guards for falsy. Make the type reflect possible null/undefined to match call sites.- Avoid
anyforlabelto preserve type safety. At minimum, type the color shape you actually use.Apply within these lines:
-function mapLabelColor(provider: string, label: any): EmailLabel["color"] { +function mapLabelColor( + provider: "google" | "microsoft" | null | undefined, + label: { color?: { textColor?: string | null; backgroundColor?: string | null } | string | null } +): EmailLabel["color"] { if (!provider) { return undefined; } if (isGoogleProvider(provider)) { - return label.color; + // For Google, label.color is already the desired shape + return (label as { color?: { textColor?: string | null; backgroundColor?: string | null } }).color; } else if (isMicrosoftProvider(provider)) { - const presetColor = label.color as string; + const presetColor = label.color as string; const backgroundColor = OUTLOOK_COLOR_MAP[presetColor as keyof typeof OUTLOOK_COLOR_MAP] || "#95A5A6"; // Default gray if preset not found return { backgroundColor, textColor: null, }; } throw new Error(`Unsupported provider: ${provider}`); }apps/web/utils/actions/whitelist.ts (2)
13-15: Guard logic is correct; consider combining checks for brevityFunctionally fine as-is. If you prefer a single early return:
- if (!env.WHITELIST_FROM) return; - if (!isGoogleProvider(provider)) return; + if (!env.WHITELIST_FROM || !isGoogleProvider(provider)) return;
16-24: Optional: add defensive error handling around Gmail API callnext-safe-action will handle unexpected errors globally, but if you want a clearer failure signal for expected API issues, wrap with try/catch and rethrow a SafeError. Keep this lightweight if you don’t need custom messages.
- const gmail = await getGmailClientForEmail({ emailAccountId }); - - await createFilter({ - gmail, - from: env.WHITELIST_FROM, - addLabelIds: ["CATEGORY_PERSONAL"], - removeLabelIds: [GmailLabel.SPAM], - }); + try { + const gmail = await getGmailClientForEmail({ emailAccountId }); + await createFilter({ + gmail, + from: env.WHITELIST_FROM, + addLabelIds: ["CATEGORY_PERSONAL"], + removeLabelIds: [GmailLabel.SPAM], + }); + } catch (err) { + // Optional: import SafeError from your actions lib and include a concise message + // throw new SafeError("Failed to create Gmail whitelist filter"); + throw err; + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/onboarding/StepCustomRules.tsx(1 hunks)apps/web/providers/EmailProvider.tsx(2 hunks)apps/web/utils/actions/whitelist.ts(1 hunks)apps/web/utils/email/provider-types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/app/(app)/[emailAccountId]/onboarding/StepCustomRules.tsx
- apps/web/app/(app)/[emailAccountId]/onboarding/OnboardingContent.tsx
🧰 Additional context used
📓 Path-based instructions (14)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/providers/EmailProvider.tsxapps/web/utils/actions/whitelist.tsapps/web/utils/email/provider-types.ts
apps/web/**/*.tsx
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
apps/web/**/*.tsx: Follow tailwindcss patterns with prettier-plugin-tailwindcss
Prefer functional components with hooks
Use shadcn/ui components when available
Ensure responsive design with mobile-first approach
Follow consistent naming conventions (PascalCase for components)
Use LoadingContent component for async data
Useresult?.serverErrorwithtoastErrorandtoastSuccess
UseLoadingContentcomponent to handle loading and error states consistently
Passloading,error, and children props toLoadingContent
Files:
apps/web/providers/EmailProvider.tsx
!{.cursor/rules/*.mdc}
📄 CodeRabbit Inference Engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/providers/EmailProvider.tsxapps/web/utils/actions/whitelist.tsapps/web/utils/email/provider-types.ts
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)
**/*.tsx: Use React Hook Form with Zod for validation
Validate form inputs before submission
Show validation errors inline next to form fields
Files:
apps/web/providers/EmailProvider.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/providers/EmailProvider.tsxapps/web/utils/actions/whitelist.tsapps/web/utils/email/provider-types.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/providers/EmailProvider.tsxapps/web/utils/actions/whitelist.tsapps/web/utils/email/provider-types.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/providers/EmailProvider.tsxapps/web/utils/actions/whitelist.tsapps/web/utils/email/provider-types.ts
**/*.{jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't destructure props inside JSX components in Solid projects.
Don't use both children and dangerouslySetInnerHTML props on the same element.
Don't use Array index in keys.
Don't assign to React component props.
Don't define React components inside other components.
Don't use event handlers on non-interactive elements.
Don't assign JSX properties multiple times.
Don't add extra closing tags for components without children.
Use <>...</> instead of ....
Don't insert comments as text nodes.
Don't use the return value of React.render.
Make sure all dependencies are correctly specified in React hooks.
Make sure all React hooks are called from the top level of component functions.
Don't use unnecessary fragments.
Don't pass children as props.
Use semantic elements instead of role attributes in JSX.
Files:
apps/web/providers/EmailProvider.tsx
**/*.{html,jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{html,jsx,tsx}: Don't use or elements.
Don't use accessKey attribute on any HTML element.
Don't set aria-hidden="true" on focusable elements.
Don't add ARIA roles, states, and properties to elements that don't support them.
Only use the scope prop on elements.
Don't assign non-interactive ARIA roles to interactive HTML elements.
Make sure label elements have text content and are associated with an input.
Don't assign interactive ARIA roles to non-interactive HTML elements.
Don't assign tabIndex to non-interactive HTML elements.
Don't use positive integers for tabIndex property.
Don't include "image", "picture", or "photo" in img alt prop.
Don't use explicit role property that's the same as the implicit/default role.
Make static elements with click handlers use a valid role attribute.
Always include a title element for SVG elements.
Give all elements requiring alt text meaningful information for screen readers.
Make sure anchors have content that's accessible to screen readers.
Assign tabIndex to non-interactive HTML elements with aria-activedescendant.
Include all required ARIA attributes for elements with ARIA roles.
Make sure ARIA properties are valid for the element's supported roles.
Always include a type attribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden).
Always include a lang attribute on the html element.
Always include a title attribute for iframe elements.
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress.
Accompany onMouseOver/onMouseOut with onFocus/onBlur.
Include caption tracks for audio and video elements.
Make sure all anchors are valid and navigable.
Ensure all ARIA properties (aria-*) are valid.
Use valid, non-abstract ARIA roles for elements with ARIA roles.
Use valid ARIA state and property values.
Use valid values for the autocomplete attribute on input eleme...Files:
apps/web/providers/EmailProvider.tsxapps/web/utils/actions/**/*.ts
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
apps/web/utils/actions/**/*.ts: Use server actions for all mutations (create/update/delete operations)
next-safe-actionprovides centralized error handling
Use Zod schemas for validation on both client and server
UserevalidatePathin server actions for cache invalidation
apps/web/utils/actions/**/*.ts: Use server actions (withnext-safe-action) for all mutations (create/update/delete operations); do NOT use POST API routes for mutations.
UserevalidatePathin server actions to invalidate cache after mutations.Files:
apps/web/utils/actions/whitelist.ts**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using ZodFiles:
apps/web/utils/actions/whitelist.tsapps/web/utils/email/provider-types.tsapps/web/utils/actions/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/server-actions.mdc)
apps/web/utils/actions/*.ts: Implement all server actions using thenext-safe-actionlibrary for type safety, input validation, context management, and error handling. Refer toapps/web/utils/actions/safe-action.tsfor client definitions (actionClient,actionClientUser,adminActionClient).
UseactionClientUserwhen only authenticated user context (userId) is needed.
UseactionClientwhen both authenticated user context and a specificemailAccountIdare needed. TheemailAccountIdmust be bound when calling the action from the client.
UseadminActionClientfor actions restricted to admin users.
Access necessary context (likeuserId,emailAccountId, etc.) provided by the safe action client via thectxobject in the.action()handler.
Server Actions are strictly for mutations (operations that change data, e.g., creating, updating, deleting). Do NOT use Server Actions for data fetching (GET operations). For data fetching, use dedicated GET API Routes combined with SWR Hooks.
UseSafeErrorfor expected/handled errors within actions if needed.next-safe-actionprovides centralized error handling.
Use the.metadata({ name: "actionName" })method to provide a meaningful name for monitoring. Sentry instrumentation is automatically applied viawithServerActionInstrumentationwithin the safe action clients.
If an action modifies data displayed elsewhere, userevalidatePathorrevalidateTagfromnext/cachewithin the action handler as needed.Server action files must start with
use serverFiles:
apps/web/utils/actions/whitelist.tsapps/web/utils/**
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logicFiles:
apps/web/utils/actions/whitelist.tsapps/web/utils/email/provider-types.tsapps/web/utils/**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle sizeFiles:
apps/web/utils/actions/whitelist.tsapps/web/utils/email/provider-types.ts🧠 Learnings (3)
📚 Learning: 2025-07-18T15:05:34.899Z
Learnt from: CR PR: elie222/inbox-zero#0 File: .cursor/rules/gmail-api.mdc:0-0 Timestamp: 2025-07-18T15:05:34.899Z Learning: Applies to apps/web/utils/gmail/**/*.ts : Keep provider-specific implementation details isolated in the appropriate utils subfolder (e.g., 'apps/web/utils/gmail/')Applied to files:
apps/web/providers/EmailProvider.tsxapps/web/utils/actions/whitelist.tsapps/web/utils/email/provider-types.ts📚 Learning: 2025-07-08T13:14:07.449Z
Learnt from: elie222 PR: elie222/inbox-zero#537 File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34 Timestamp: 2025-07-08T13:14:07.449Z Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.Applied to files:
apps/web/providers/EmailProvider.tsx📚 Learning: 2025-07-18T17:27:58.249Z
Learnt from: CR PR: elie222/inbox-zero#0 File: .cursor/rules/server-actions.mdc:0-0 Timestamp: 2025-07-18T17:27:58.249Z Learning: Applies to apps/web/utils/actions/*.ts : Use `actionClient` when both authenticated user context and a specific `emailAccountId` are needed. The `emailAccountId` must be bound when calling the action from the client.Applied to files:
apps/web/utils/actions/whitelist.ts🧬 Code Graph Analysis (2)
apps/web/providers/EmailProvider.tsx (1)
apps/web/utils/email/provider-types.ts (1)
isGoogleProvider(1-3)apps/web/utils/actions/whitelist.ts (3)
apps/web/utils/actions/safe-action.ts (1)
actionClient(46-86)apps/web/env.ts (1)
env(16-225)apps/web/utils/email/provider-types.ts (1)
isGoogleProvider(1-3)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
🔇 Additional comments (3)
apps/web/utils/email/provider-types.ts (1)
1-3: Add a type predicate to isGoogleProvider for better narrowing
- Change the function signature so callers get control-flow type narrowing when checking for
"google".- Optionally, centralize allowed providers with a union type:
export type Provider = "google" | "microsoft";- Verified—no remaining references to
isGmailProviderwere found.-export function isGoogleProvider(provider: string | null | undefined) { +export function isGoogleProvider(provider: string | null | undefined): provider is "google" { return provider === "google"; }apps/web/providers/EmailProvider.tsx (1)
8-10: Import rename to isGoogleProvider is consistent with utils changeThe import swap aligns with the provider helper rename and keeps semantics intact.
apps/web/utils/actions/whitelist.ts (1)
8-9: Import update to isGoogleProvider is correctMatches the helper rename and keeps the Gmail-only guard explicit.
|
Claude finished @elie222's task —— View job Code Review: New Onboarding Flow (PR #682)I've completed a comprehensive review of this substantial onboarding implementation. Here's my analysis: ✅ StrengthsArchitecture & Design
Code Quality
User Experience
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/web/utils/actions/rule.validation.ts (2)
196-206: Tighten onboarding schema: trim and bound description; consider nullish for key
- description should be trimmed, non-empty, and reasonably bounded (mirrors name).
- Consider consistency: other fields use .nullish(); align key as well unless you specifically need null over undefined.
Apply this diff:
const categoryConfig = z.object({ action: categoryAction.nullish(), hasDigest: z.boolean().nullish(), name: z .string() .trim() .min(1, "Please enter a name") .max(40, "Please keep names under 40 characters"), - description: z.string(), - key: z.nativeEnum(SystemRule).nullable(), + description: z + .string() + .trim() + .min(1, "Please enter a description") + .max(140, "Please keep descriptions under 140 characters"), + key: z.nativeEnum(SystemRule).nullish(), });
209-209: Require at least one category in onboarding payloadThis aligns with our “validate type, format, and length” guideline and prior feedback.
-export const createRulesOnboardingBody = z.array(categoryConfig); +export const createRulesOnboardingBody = z + .array(categoryConfig) + .min(1, "Please add at least one category");apps/web/app/api/user/categorization-preferences/route.ts (1)
71-117: Critical: Object spread on possibly-undefined will throw; add nullish fallbacksEach helper can return undefined; spreading undefined throws at runtime and violates strict TS. Use a safe fallback.
Apply this diff:
return [ { name: RuleName.ToReply, key: SystemRule.ToReply, description: "", - ...getToReplySetting(emailAccount.rules), + ...(getToReplySetting(emailAccount.rules) ?? {}), }, { name: RuleName.ColdEmail, key: SystemRule.ColdEmail, description: "", - ...getColdEmailSetting( - emailAccount.coldEmailBlocker, - emailAccount.coldEmailDigest, - ), + ...(getColdEmailSetting( + emailAccount.coldEmailBlocker, + emailAccount.coldEmailDigest, + ) ?? {}), }, { name: RuleName.Newsletter, key: SystemRule.Newsletter, description: "", - ...getRuleSetting(SystemType.NEWSLETTER, emailAccount.rules), + ...(getRuleSetting(SystemType.NEWSLETTER, emailAccount.rules) ?? {}), }, { name: RuleName.Marketing, key: SystemRule.Marketing, description: "", - ...getRuleSetting(SystemType.MARKETING, emailAccount.rules), + ...(getRuleSetting(SystemType.MARKETING, emailAccount.rules) ?? {}), }, { name: RuleName.Calendar, key: SystemRule.Calendar, description: "", - ...getRuleSetting(SystemType.CALENDAR, emailAccount.rules), + ...(getRuleSetting(SystemType.CALENDAR, emailAccount.rules) ?? {}), }, { name: RuleName.Receipt, key: SystemRule.Receipt, description: "", - ...getRuleSetting(SystemType.RECEIPT, emailAccount.rules), + ...(getRuleSetting(SystemType.RECEIPT, emailAccount.rules) ?? {}), }, { name: RuleName.Notification, key: SystemRule.Notification, description: "", - ...getRuleSetting(SystemType.NOTIFICATION, emailAccount.rules), + ...(getRuleSetting(SystemType.NOTIFICATION, emailAccount.rules) ?? {}), }, ];
🧹 Nitpick comments (3)
apps/web/app/api/user/categorization-preferences/route.ts (3)
16-19: Avoid type name collision with exported CategoryConfig; rename local typeThis local CategoryConfig differs from the public CategoryConfig from rule.validation.ts and can confuse readers. Rename to CategorySettings (or similar) and adjust return types.
-type CategoryConfig = { +type CategorySettings = { action: CategoryAction | undefined; hasDigest: boolean | undefined; }; @@ -function getToReplySetting( +function getToReplySetting( rules: UserPreferences["rules"], -): CategoryConfig | undefined { +): CategorySettings | undefined { @@ -function getRuleSetting( +function getRuleSetting( systemType: SystemType, rules?: UserPreferences["rules"], -): CategoryConfig | undefined { +): CategorySettings | undefined { @@ -function getColdEmailSetting( +function getColdEmailSetting( setting?: ColdEmailSetting | null, hasDigest?: boolean, -): CategoryConfig | undefined { +): CategorySettings | undefined {Also applies to: 120-123, 138-141, 155-159
120-136: Optional: compute To-Reply in a single pass over rulesMinor perf/readability: avoid two find() calls by deriving both flags in one loop.
-function getToReplySetting( - rules: UserPreferences["rules"], -): CategoryConfig | undefined { - if (!rules.length) return undefined; - const rule = rules.find((rule) => - rule.actions.some((action) => action.type === ActionType.TRACK_THREAD), - ); - const replyRules = rules.find( - (rule) => rule.systemType === SystemType.TO_REPLY, - ); - const hasDigest = replyRules?.actions.some( - (action) => action.type === ActionType.DIGEST, - ); - - if (rule) return { action: "label", hasDigest }; - return { action: "none", hasDigest }; -} +function getToReplySetting( + rules: UserPreferences["rules"], +): CategorySettings | undefined { + if (!rules.length) return undefined; + let hasTrackThread = false; + let hasDigest = false; + for (const rule of rules) { + if (!hasTrackThread) { + hasTrackThread = rule.actions.some( + (a) => a.type === ActionType.TRACK_THREAD, + ); + } + if (!hasDigest && rule.systemType === SystemType.TO_REPLY) { + hasDigest = rule.actions.some((a) => a.type === ActionType.DIGEST); + } + if (hasTrackThread && hasDigest) break; + } + return { action: hasTrackThread ? "label" : "none", hasDigest }; +}
71-117: Follow-up: If description becomes required/trimmed, populate it hereIf you adopt the stricter schema (trim + min(1)), empty description "" will violate expectations downstream. Consider supplying non-empty copy derived from a constants map (pure TS, no React imports), e.g., a new utils/rule/metadata.ts that maps SystemRule -> description.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/app/api/user/categorization-preferences/route.ts(3 hunks)apps/web/utils/actions/rule.validation.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Path aliases: Use@/for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Leverage TypeScript inference for better DX
Files:
apps/web/utils/actions/rule.validation.tsapps/web/app/api/user/categorization-preferences/route.ts
apps/web/utils/actions/**/*.ts
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
apps/web/utils/actions/**/*.ts: Use server actions for all mutations (create/update/delete operations)
next-safe-actionprovides centralized error handling
Use Zod schemas for validation on both client and server
UserevalidatePathin server actions for cache invalidation
apps/web/utils/actions/**/*.ts: Use server actions (withnext-safe-action) for all mutations (create/update/delete operations); do NOT use POST API routes for mutations.
UserevalidatePathin server actions to invalidate cache after mutations.
Files:
apps/web/utils/actions/rule.validation.ts
!{.cursor/rules/*.mdc}
📄 CodeRabbit Inference Engine (.cursor/rules/cursor-rules.mdc)
Never place rule files in the project root, in subdirectories outside .cursor/rules, or in any other location
Files:
apps/web/utils/actions/rule.validation.tsapps/web/app/api/user/categorization-preferences/route.ts
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/form-handling.mdc)
**/*.ts: The same validation should be done in the server action too
Define validation schemas using Zod
Files:
apps/web/utils/actions/rule.validation.tsapps/web/app/api/user/categorization-preferences/route.ts
apps/web/utils/actions/*.validation.ts
📄 CodeRabbit Inference Engine (.cursor/rules/fullstack-workflow.mdc)
Define Zod schemas for validation in dedicated files and use them for both client and server validation.
Define input validation schemas using Zod in the corresponding
.validation.tsfile. These schemas are used bynext-safe-action(.schema()) and can also be reused on the client for form validation.
Files:
apps/web/utils/actions/rule.validation.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/logging.mdc)
**/*.{ts,tsx}: UsecreateScopedLoggerfor logging in backend TypeScript files
Typically add the logger initialization at the top of the file when usingcreateScopedLogger
Only use.with()on a logger instance within a specific function, not for a global loggerImport Prisma in the project using
import prisma from "@/utils/prisma";
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't use TypeScript const enum.
Don't use the TypeScript directive @ts-ignore.
Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use implicit any type on variable declarations.
Don't let variables evolve into any type through reassignments.
Don't use non-null assertions with the ! postfix operator.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use export type for types.
Use import type for types.
Don't declare empty interfaces.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use TypeScript namespaces.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use parameter properties in class constructors.
Use either T[] or Array consistently.
Initialize each enum member value explicitly.
Make sure all enum members are literal values.
Files:
apps/web/utils/actions/rule.validation.tsapps/web/app/api/user/categorization-preferences/route.ts
apps/web/utils/actions/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/server-actions.mdc)
apps/web/utils/actions/*.ts: Implement all server actions using thenext-safe-actionlibrary for type safety, input validation, context management, and error handling. Refer toapps/web/utils/actions/safe-action.tsfor client definitions (actionClient,actionClientUser,adminActionClient).
UseactionClientUserwhen only authenticated user context (userId) is needed.
UseactionClientwhen both authenticated user context and a specificemailAccountIdare needed. TheemailAccountIdmust be bound when calling the action from the client.
UseadminActionClientfor actions restricted to admin users.
Access necessary context (likeuserId,emailAccountId, etc.) provided by the safe action client via thectxobject in the.action()handler.
Server Actions are strictly for mutations (operations that change data, e.g., creating, updating, deleting). Do NOT use Server Actions for data fetching (GET operations). For data fetching, use dedicated GET API Routes combined with SWR Hooks.
UseSafeErrorfor expected/handled errors within actions if needed.next-safe-actionprovides centralized error handling.
Use the.metadata({ name: "actionName" })method to provide a meaningful name for monitoring. Sentry instrumentation is automatically applied viawithServerActionInstrumentationwithin the safe action clients.
If an action modifies data displayed elsewhere, userevalidatePathorrevalidateTagfromnext/cachewithin the action handler as needed.Server action files must start with
use server
Files:
apps/web/utils/actions/rule.validation.ts
apps/web/utils/**
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
Create utility functions in
utils/folder for reusable logic
Files:
apps/web/utils/actions/rule.validation.ts
apps/web/utils/**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
apps/web/utils/**/*.ts: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size
Files:
apps/web/utils/actions/rule.validation.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useelements in Next.js projects.
Don't use elements in Next.js projects.
Don't use namespace imports.
Don't access namespace imports dynamically.
Don't use global eval().
Don't use console.
Don't use debugger.
Don't use var.
Don't use with statements in non-strict contexts.
Don't use the arguments object.
Don't use consecutive spaces in regular expression literals.
Don't use the comma operator.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names th...
Files:
apps/web/utils/actions/rule.validation.tsapps/web/app/api/user/categorization-preferences/route.ts
!pages/_document.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/ultracite.mdc)
!pages/_document.{js,jsx,ts,tsx}: Don't import next/document outside of pages/_document.jsx in Next.js projects.
Don't import next/document outside of pages/_document.jsx in Next.js projects.
Files:
apps/web/utils/actions/rule.validation.tsapps/web/app/api/user/categorization-preferences/route.ts
apps/web/app/**
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
NextJS app router structure with (app) directory
Files:
apps/web/app/api/user/categorization-preferences/route.ts
apps/web/app/api/**/route.ts
📄 CodeRabbit Inference Engine (apps/web/CLAUDE.md)
apps/web/app/api/**/route.ts: UsewithAuthfor user-level operations
UsewithEmailAccountfor email-account-level operations
Do NOT use POST API routes for mutations - use server actions instead
No need for try/catch in GET routes when using middleware
Export response types from GET routes
apps/web/app/api/**/route.ts: Wrap all GET API route handlers withwithAuthorwithEmailAccountmiddleware for authentication and authorization.
Export response types from GET API routes for type-safe client usage.
Do not use try/catch in GET API routes when using authentication middleware; rely on centralized error handling.
Files:
apps/web/app/api/user/categorization-preferences/route.ts
**/api/**/route.ts
📄 CodeRabbit Inference Engine (.cursor/rules/security.mdc)
**/api/**/route.ts: ALL API routes that handle user data MUST use appropriate authentication and authorization middleware (withAuth or withEmailAccount).
ALL database queries in API routes MUST be scoped to the authenticated user/account (e.g., include userId or emailAccountId in query filters).
Always validate that resources belong to the authenticated user before performing operations (resource ownership validation).
UsewithEmailAccountmiddleware for API routes that operate on a specific email account (i.e., use or requireemailAccountId).
UsewithAuthmiddleware for API routes that operate at the user level (i.e., use or require onlyuserId).
UsewithErrormiddleware (with proper validation) for public endpoints, custom authentication, or cron endpoints.
Cron endpoints MUST usewithErrormiddleware and validate the cron secret usinghasCronSecret(request)orhasPostCronSecret(request).
Cron endpoints MUST capture unauthorized attempts withcaptureExceptionand return a 401 status for unauthorized requests.
All parameters in API routes MUST be validated for type, format, and length before use.
Request bodies in API routes MUST be validated using Zod schemas before use.
All Prisma queries in API routes MUST only return necessary fields and never expose sensitive data.
Error messages in API routes MUST not leak internal information or sensitive data; use generic error messages and SafeError where appropriate.
API routes MUST use a consistent error response format, returning JSON with an error message and status code.
AllfindUniqueandfindFirstPrisma calls in API routes MUST include ownership filters (e.g., userId or emailAccountId).
AllfindManyPrisma calls in API routes MUST be scoped to the authenticated user's data.
Never use direct object references in API routes without ownership checks (prevent IDOR vulnerabilities).
Prevent mass assignment vulnerabilities by only allowing explicitly whitelisted fields in update operations in AP...
Files:
apps/web/app/api/user/categorization-preferences/route.ts
apps/web/app/api/**/*.{ts,js}
📄 CodeRabbit Inference Engine (.cursor/rules/security-audit.mdc)
apps/web/app/api/**/*.{ts,js}: All API route handlers in 'apps/web/app/api/' must use authentication middleware: withAuth, withEmailAccount, or withError (with custom authentication logic).
All Prisma queries in API routes must include user/account filtering (e.g., emailAccountId or userId in WHERE clauses) to prevent unauthorized data access.
All parameters used in API routes must be validated before use; do not use parameters from 'params' or request bodies directly in queries without validation.
Request bodies in API routes should use Zod schemas for validation.
API routes should only return necessary fields using Prisma's 'select' and must not include sensitive data in error messages.
Error messages in API routes must not reveal internal details; use generic errors and SafeError for user-facing errors.
All QStash endpoints (API routes called via publishToQstash or publishToQstashQueue) must use verifySignatureAppRouter to verify request authenticity.
All cron endpoints in API routes must use hasCronSecret or hasPostCronSecret for authentication.
Do not hardcode weak or plaintext secrets in API route files; secrets must not be directly assigned as string literals.
Review all new withError usage in API routes to ensure custom authentication is implemented where required.
Files:
apps/web/app/api/user/categorization-preferences/route.ts
🧠 Learnings (4)
📚 Learning: 2025-07-18T15:05:16.146Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/fullstack-workflow.mdc:0-0
Timestamp: 2025-07-18T15:05:16.146Z
Learning: Applies to apps/web/utils/actions/*.validation.ts : Define Zod schemas for validation in dedicated files and use them for both client and server validation.
Applied to files:
apps/web/utils/actions/rule.validation.ts
📚 Learning: 2025-07-18T15:04:30.467Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-07-18T15:04:30.467Z
Learning: Applies to apps/web/utils/actions/**/*.ts : Use Zod schemas for validation on both client and server
Applied to files:
apps/web/utils/actions/rule.validation.ts
📚 Learning: 2025-07-18T17:27:58.249Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/server-actions.mdc:0-0
Timestamp: 2025-07-18T17:27:58.249Z
Learning: Applies to apps/web/utils/actions/*.validation.ts : Define input validation schemas using Zod in the corresponding `.validation.ts` file. These schemas are used by `next-safe-action` (`.schema()`) and can also be reused on the client for form validation.
Applied to files:
apps/web/utils/actions/rule.validation.ts
📚 Learning: 2025-08-10T22:08:49.243Z
Learnt from: CR
PR: elie222/inbox-zero#0
File: .cursor/rules/llm.mdc:0-0
Timestamp: 2025-08-10T22:08:49.243Z
Learning: Applies to apps/web/utils/{ai,llms}/**/*.ts : Make Zod schemas as specific as possible to guide the LLM output.
Applied to files:
apps/web/utils/actions/rule.validation.ts
🧬 Code Graph Analysis (2)
apps/web/utils/actions/rule.validation.ts (2)
apps/web/utils/rule/consts.ts (2)
SystemRule(12-20)SystemRule(21-21)apps/web/utils/category-config.tsx (1)
categoryConfig(14-80)
apps/web/app/api/user/categorization-preferences/route.ts (2)
apps/web/utils/actions/rule.validation.ts (2)
CreateRulesOnboardingBody(210-212)CategoryConfig(207-207)apps/web/utils/rule/consts.ts (1)
RuleName(2-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/web/utils/actions/rule.validation.ts (1)
10-10: Import looks correct; using SystemRule as a value for z.nativeEnum is finez.nativeEnum accepts objects with string values; importing the value export aligns with the usage below.
apps/web/app/api/user/categorization-preferences/route.ts (1)
14-14: LGTM: Using RuleName and SystemRule for labeling outputThis keeps the response aligned with the new array-based onboarding flow.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores