Implement db setup and api routes#3
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds authenticated book upload and management: server-side MongoDB connection and models, Mongoose-backed server actions, a Vercel Blob upload API (POST/DELETE), client upload orchestration with PDF parsing/segmentation, an authenticated book detail page, and a themed Toaster in root layout. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Client)
participant Form as UploadForm (Client)
participant PDF as PDF Parser (lib/utils)
participant API as /api/upload (Node)
participant Blob as Vercel Blob
participant ServerAction as Server Action (createBook/saveBookSegments)
participant DB as MongoDB
User->>Form: Submit metadata + PDF (+ optional cover)
Form->>PDF: parsePDFFile(file)
PDF-->>Form: { segments, coverDataURL }
Form->>API: POST /api/upload (pdf file)
API->>Blob: put(file, { public: true })
Blob-->>API: { url, pathname }
API-->>Form: { url, pathname }
alt cover needed
Form->>API: POST /api/upload (cover dataURL)
API->>Blob: put(cover, { public: true })
Blob-->>API: { coverUrl, coverPath }
API-->>Form: { coverUrl, coverPath }
end
Form->>ServerAction: createBook(metadata + file/cover URLs)
ServerAction->>DB: insert Book
ServerAction-->>Form: bookId
Form->>ServerAction: saveBookSegments(bookId, clerkId, segments)
ServerAction->>DB: bulk insert BookSegment documents
ServerAction-->>Form: success
Form->>User: navigate to book page / show toast
sequenceDiagram
participant Client as Authenticated User
participant Page as Book Page (SSR)
participant Auth as Clerk Auth
participant DB as MongoDB
participant CDN as Image CDN
Client->>Page: Request /books/:slug
Page->>Auth: auth() -> { userId }
alt not authenticated
Auth-->>Page: null
Page-->>Client: notFound()
else authenticated
Auth-->>Page: userId
Page->>DB: getBookBySlug(slug, userId)
alt book found
DB-->>Page: book data
Page->>CDN: fetch cover image
CDN-->>Page: image delivered
Page-->>Client: render book details + PDF link
else
DB-->>Page: null
Page-->>Client: notFound()
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/UploadForm.tsx (1)
493-493:⚠️ Potential issue | 🟠 MajorButton disabled state inconsistency.
The button uses
form.formState.isSubmittingfor thedisabledprop, but the component uses a customisSubmittingstate (line 62) to track submission status and show the loading overlay. This could cause the button to be re-enabled while the loading overlay is still visible.🔧 Proposed fix
- <Button type="submit" className="form-btn" disabled={form.formState.isSubmitting}> + <Button type="submit" className="form-btn" disabled={isSubmitting}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/UploadForm.tsx` at line 493, The Button's disabled prop currently uses form.formState.isSubmitting while the component also tracks submission with a custom isSubmitting state (declared as isSubmitting around line 62), causing inconsistency; update the Button to use the same submission state (use isSubmitting) or synchronize the two by setting form.setSubmitting(...) whenever you toggle the custom isSubmitting so the button and loading overlay share the same source of truth (refer to the Button element and the isSubmitting state and any place where you toggle submission state).
🧹 Nitpick comments (13)
database/mongoose.ts (2)
43-44: "Connected to MongoDB" logs on every call, including cached connections.This log statement executes every time
connectToDb()is called successfully, even when returning a cached connection. This can create noisy logs in serverless environments where the function is called frequently.🛠️ Suggested fix - only log on fresh connections
try { + const isNewConnection = !cached!.conn; cached!.conn = await cached!.promise; + if (isNewConnection) { + console.info("Connected to MongoDB"); + } } catch (error) { cached!.promise = null; console.error("Error connecting to MongoDB. Please make sure MongoDB is running.", error); throw error; } - console.info("Connected to MongoDB"); return cached!.conn;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@database/mongoose.ts` around lines 43 - 44, The console.info("Connected to MongoDB") runs on every successful return from connectToDb(), causing noisy logs; change the function so the "Connected to MongoDB" log is emitted only when a new connection is established (i.e., in the branch where you create and assign cached and cached.conn), not when returning the cached.conn, by moving or guarding the console.info call to the fresh-connection path inside connectToDb() and keep cached/cached.conn logic untouched otherwise.
3-7: Non-null assertion executes before the validation check.The
!assertion on line 3 happens at module load time, before the check on line 5-7 can throw. While this works at runtime (the check will still throw), it's misleading and bypasses TypeScript's safety.🛠️ Suggested fix
-const MONGODB_URI = process.env.MONGODB_URI!; +const MONGODB_URI = process.env.MONGODB_URI; if (!MONGODB_URI) { throw new Error("MONGODB_URI is not defined"); }Then update usages of
MONGODB_URIin the file to use it as astring(TypeScript will narrow the type after the check).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@database/mongoose.ts` around lines 3 - 7, The non-null assertion on MONGODB_URI causes TypeScript to assume a value before your runtime check; change the declaration to read the raw environment value (e.g., assign process.env.MONGODB_URI without the trailing "!"/assertion or explicitly type it as string|undefined), keep the existing runtime guard (if (!MONGODB_URI) throw new Error(...)), and then use MONGODB_URI normally afterwards (TypeScript will narrow it to string after the check); update any uses in this file that relied on the prior assertion to rely on the narrowed type instead.database/models/book.model.ts (1)
4-16: Consider adding an index onclerkIdfor query performance.Based on the context from
lib/actions/book.actions.ts, books are queried by bothslugandclerkId. Whileslughas a unique index, queries filtering byclerkId(e.g., fetching a user's books) would benefit from an index.🛠️ Suggested addition after the schema definition
}, { timestamps: true }); +BookSchema.index({ clerkId: 1 }); +// Or for user-specific slug lookups: +// BookSchema.index({ clerkId: 1, slug: 1 }); + const Book = models.Book || model<IBook>("Book", BookSchema, "books");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@database/models/book.model.ts` around lines 4 - 16, BookSchema currently only defines a unique index on slug but queries also filter by clerkId (see BookSchema and usage in book.actions.ts); add an index to improve query performance by calling BookSchema.index({ clerkId: 1 }) after the schema definition (or create a compound index BookSchema.index({ clerkId: 1, slug: 1 }) if you need fast lookups by both clerkId and slug), then export the schema/model as before.next.config.ts (1)
12-16: Hardcoded Vercel Blob hostname may not work across environments.The hostname
04nyssjw34flysb1.public.blob.vercel-storage.comis tied to a specific Vercel Blob storage account. Based onapp/api/upload/route.ts, the blob URL is determined by theBLOB_READ_WRITE_TOKENenvironment variable, which may differ between environments.Consider using a wildcard pattern for Vercel Blob storage:
🛠️ Suggested change
{ protocol: "https", - hostname: "04nyssjw34flysb1.public.blob.vercel-storage.com", + hostname: "*.public.blob.vercel-storage.com", pathname: "/**", }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@next.config.ts` around lines 12 - 16, The remote image configuration in next.config.ts hardcodes a single Vercel Blob hostname; update the remotePatterns entry (the object with protocol/hostname/pathname) to accept wildcard Vercel Blob hostnames so it works across environments (e.g., use a wildcard host pattern like "*.public.blob.vercel-storage.com" or a glob pattern matching any subdomain of public.blob.vercel-storage.com) and keep pathname "/**"; ensure this aligns with how BLOB_READ_WRITE_TOKEN derives the blob URL in app/api/upload/route.ts.database/models/voice-session.model.ts (1)
6-16: Consider index efficiency.The individual index on
clerkId(line 6) may be redundant since the compound index(clerkId, billingPeriodStart)on line 16 can serve queries filtering byclerkIdalone (leftmost prefix). Similarly, the individualbillingPeriodStartindex (line 11) adds write overhead.Unless you have specific query patterns requiring the individual indexes, consider removing them to reduce index maintenance cost.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@database/models/voice-session.model.ts` around lines 6 - 16, The schema currently creates redundant single-field indexes on clerkId and billingPeriodStart while also adding a compound index VoiceSessionSchema.index({ clerkId: 1, billingPeriodStart: 1 }); remove the individual index flags on the clerkId field and the billingPeriodStart field (and/or delete any separate index declaration for billingPeriodStart) so only the compound index remains, leaving required/other options intact (e.g., keep clerkId: { type: String, required: true } and billingPeriodStart: { type: Date, required: true }); this reduces write/index overhead while preserving queries that filter by clerkId or by the pair (clerkId, billingPeriodStart).lib/utils.ts (2)
87-91:formatDurationmay produce unexpected results with decimal seconds.If
secondsis a floating-point number (e.g.,65.7), the modulo operation will preserve the decimal (5.7), butpadStarton a decimal string may look odd (e.g.,"1:5.7"). Consider rounding or flooring.🔧 Proposed fix
export const formatDuration = (seconds: number): string => { - const mins = Math.floor(seconds / 60); - const secs = seconds % 60; + const totalSecs = Math.round(seconds); + const mins = Math.floor(totalSecs / 60); + const secs = totalSecs % 60; return `${mins}:${secs.toString().padStart(2, '0')}`; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/utils.ts` around lines 87 - 91, formatDuration currently misformats when given non-integer seconds; floor the input first and compute minutes/seconds from that integer so decimals are dropped (e.g., in formatDuration, call Math.floor on seconds into a local totalSeconds, then compute mins = Math.floor(totalSeconds/60) and secs = totalSeconds % 60) and use secs.toString().padStart(2,'0') to ensure proper formatting.
117-133: Canvas element should be cleaned up after use.The canvas element is created dynamically but never removed. While
pdfDocument.destroy()is called, the canvas remains in memory. For repeated calls toparsePDFFile, this could lead to memory accumulation.🧹 Proposed cleanup
// Convert canvas to data URL const coverDataURL = canvas.toDataURL('image/png'); + // Clean up canvas + canvas.width = 0; + canvas.height = 0; + // Extract text from all pages🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/utils.ts` around lines 117 - 133, The dynamically created canvas used in parsePDFFile (created before firstPage.render and used to produce coverDataURL) is never cleaned up; after generating the data URL and before leaving the function, remove the canvas and clear references so it can be GC'd — call canvas.remove() (or detach it if ever appended), set canvas.width = canvas.height = 0 and null out the context reference (the variable holding the getContext result) and any local canvas variable reference so memory is released; keep these cleanup steps immediately after the coverDataURL is produced and before returning or calling pdfDocument.destroy().lib/zod.ts (1)
19-27: Simplify the cover image validation refinement.The
!filecheck in the refine callback is unnecessary because:
- The schema uses
.optional(), so if no file is provided, the refinement won't run- When the refinement runs,
fileis guaranteed to be aFileinstance due to.instanceof(File)- The
as stringcast onfile.typeis unnecessary sinceFile.typeis already typed asstring🧹 Proposed simplification
coverImage: z .instanceof(File) - .refine((file) => !file || ACCEPTED_IMAGE_TYPES.includes(file.type as string), { + .refine((file) => ACCEPTED_IMAGE_TYPES.includes(file.type), { message: "Only .jpg, .jpeg, .png, and .webp files are supported.", })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/zod.ts` around lines 19 - 27, The coverImage zod refinements contain redundant checks: inside the schema using z.instanceof(File) and .optional(), remove the unnecessary "!file" guard and the redundant "as string" cast on file.type in the MIME-type refine; keep the same logic but change the refine callback to accept a File and check ACCEPTED_IMAGE_TYPES.includes(file.type), and likewise for the size refine use file.size <= MAX_IMAGE_SIZE; reference the coverImage field, z.instanceof(File), ACCEPTED_IMAGE_TYPES, and MAX_IMAGE_SIZE when making the adjustments.app/(root)/books/[slug]/page.tsx (2)
14-16: Consider redirecting unauthenticated users to sign-in instead of 404.Calling
notFound()for unauthenticated users returns a 404 page, which may confuse users who expect to see the book detail page after logging in. Consider usingredirect('/sign-in')to guide users to authenticate.🔀 Alternative approach
+import { redirect } from "next/navigation"; + const { userId } = await auth(); if (!userId) { - notFound(); + redirect('/sign-in'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(root)/books/[slug]/page.tsx around lines 14 - 16, The current auth check in the page uses notFound() when userId is falsy (see userId and notFound() in page.tsx); change this to redirect unauthenticated users to the sign-in flow instead: replace the notFound() call with a redirect to your sign-in route (e.g., redirect('/sign-in')) or call a helper like requireAuth that redirects, ensuring the rest of the loader/page logic only runs for authenticated users.
35-39: Consider usingnext/imagefor optimized image loading.Using a native
<img>tag instead ofnext/imagemisses out on automatic image optimization, lazy loading, and format conversion. Since the AI summary mentions aligning withnext.config.tsremote image patterns, usingnext/imagewould be more consistent.🖼️ Proposed change
+import Image from "next/image"; + // ... in the JSX: - <img + <Image src={book.coverURL} alt={`${book.title} cover`} + width={220} + height={330} className="w-full rounded-xl border border-[var(--border-medium)] object-cover" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(root)/books/[slug]/page.tsx around lines 35 - 39, Replace the native <img> with Next.js' Image component to leverage automatic optimization and lazy loading: import Image from "next/image" at the top of page.tsx and swap the <img src={book.coverURL} alt={`${book.title} cover`} className="..."> usage for <Image> using the same book.coverURL and alt text (provide appropriate width/height or use layout/fill and preserve the existing className styling), ensuring the remote host matches the remotePatterns in next.config.ts; update any props (priority, placeholder) as needed to preserve behavior.app/api/upload/route.ts (1)
31-39: Consider adding server-side file size validation.The API route validates that a file exists but doesn't enforce a maximum file size. While client-side validation exists in the Zod schema (50MB for PDFs), a malicious client could bypass this. Consider adding server-side size validation for defense in depth.
🛡️ Proposed addition
if (!(file instanceof Blob)) { return NextResponse.json({ error: "Missing file" }, { status: 400 }); } + const MAX_FILE_SIZE = 50 * 1024 * 1024; // 50MB + if (file.size > MAX_FILE_SIZE) { + return NextResponse.json({ error: "File too large (max 50MB)" }, { status: 400 }); + } + const blob = await put(pathname, file, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/upload/route.ts` around lines 31 - 39, The handler currently only checks that file is a Blob but lacks server-side size enforcement; before calling put(pathname, file, ...) validate the incoming Blob's size (file.size) against the max allowed (e.g., 50 * 1024 * 1024 bytes) and return an appropriate error response (e.g., NextResponse.json({ error: "File too large" }, { status: 413 })) if exceeded; keep the existing type/contentType checks and only call put(pathname, file, { token: blobToken, access: "public", contentType: ... }) when the size is within limits.components/UploadForm.tsx (1)
67-71: Unnecessary delayed mount state.The
setTimeoutwith a 100ms delay for settingisMountedappears unused—theisMountedstate isn't referenced anywhere in the component after being set.🧹 Consider removing unused code
const UploadForm = () => { const [isSubmitting, setIsSubmitting] = useState(false); - const [isMounted, setIsMounted] = useState(false); const { userId } = useAuth(); const router = useRouter(); - - useEffect(() => { - setTimeout(() => { - setIsMounted(true); - }, 100); - }, []); - -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/UploadForm.tsx` around lines 67 - 71, The component contains an unused delayed mount state: remove the isMounted state and the useEffect that sets it (the setIsMounted call inside the setTimeout) from UploadForm.tsx; specifically delete the useState declaration for isMounted and the whole useEffect block that wraps setTimeout, and also remove any remaining references or imports tied only to that state (e.g., unused setIsMounted/useEffect if now unused) so there is no dead code left.lib/actions/book.actions.ts (1)
40-40: Remove or reduce verbosity of debug logging.Lines 40 and 54 contain
console.logstatements that output user data (clerkId) and operational details. Consider removing these or using a structured logger with appropriate log levels for production.🧹 Proposed cleanup
- console.log("Saving book segments", bookId, clerkId, segments); + // Debug logging removed for production // ... later ... - console.log("Book updated with total segments", bookId, savedSegments.length);Also applies to: 54-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/actions/book.actions.ts` at line 40, Remove or reduce verbose console.log usage that exposes user data in lib/actions/book.actions.ts: locate the console.log calls (currently printing "Saving book segments", clerkId, segments) and either delete them or replace them with a structured logger call at an appropriate log level (e.g., debug) that omits or redacts sensitive fields like clerkId and segment contents; ensure you use the project's logger utility (or create one) and call it from the same function where the console.log appears so operational details remain available without leaking user data.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ui/sonner.tsx`:
- Around line 39-43: The toast component's toastOptions uses classNames.toast
set to "cn-toast", but that CSS class isn't defined; either add a .cn-toast rule
to your global stylesheet (app/globals.css) with the desired styling, or replace
the "cn-toast" value in the toastOptions -> classNames -> toast property with
equivalent Tailwind utility classes (e.g., "px-4 py-2 rounded-lg bg-white
shadow-md") to ensure the toast is styled correctly.
In `@components/UploadForm.tsx`:
- Line 129: In components/UploadForm.tsx fix the typo in the toast text by
updating the toast.info call (toast.info("Book with the same titlealready
exists")) to include the missing space so the message reads "Book with the same
title already exists"; locate the toast.info invocation in the UploadForm
component and change the string accordingly.
- Around line 173-190: The code reads uploadedCover.url before checking
uploadedCover, and assigns coverUrl twice; instead, remove the unreachable falsy
check and the duplicate assignment: call uploadViaServer(...) and then use the
returned uploadedCover to set coverUrl and coverBlobPathname, and only handle
errors via try/catch around the fetch/upload (show toast.error and form.reset in
the catch). Update references to parsedPdf.cover, fileTitle, uploadViaServer,
coverUrl, and coverBlobPathname accordingly so you don't access
uploadedCover.url before ensuring the call succeeded.
- Line 223: In UploadForm.tsx the extra call setIsSubmitting(true) inside the
submit flow is redundant because isSubmitting was already set to true earlier
(see the submit handler, e.g., handleSubmit/onSubmit); remove this duplicate
setIsSubmitting(true) invocation from the submit path to avoid unnecessary state
churn, and ensure any final cleanup still sets setIsSubmitting(false) in the
error/finally branch so the form can re-enable correctly.
In `@lib/actions/book.actions.ts`:
- Around line 9-20: The duplicate check in createBook currently looks up
Book.findOne by slug only, causing cross-user collisions; update the check in
the createBook function to include the user's clerkId (e.g., use Book.findOne({
slug, clerkId: data.clerkId })) so it matches the same filtering used by
getBookBySlug, and return the serialized existingBook as before
(serializeData(existingBook)); ensure generateSlug(data.title) is still used and
that data.clerkId is present/used for the query.
- Around line 75-89: checkBookExists currently only checks by slug which breaks
user isolation; update the function signature checkBookExists(title: string,
clerkId: string) to accept clerkId, compute slug with generateSlug(title) as
before and change the DB lookup from Book.findOne({ slug }) to Book.findOne({
slug, clerkId }) (and keep .lean()). Also update any callers to pass the clerkId
from the same source used in createBook/getBookBySlug, and ensure returned data
still uses serializeData(existingBook) and error handling remains unchanged.
In `@lib/utils.ts`:
- Line 166: Remove the debug logging of the full PDF text: locate the
console.log("Full text", fullText) in lib/utils.ts (where PDF extraction code
runs) and delete it or wrap it behind a runtime debug flag (e.g.,
process.env.DEBUG_PDF or existing logger.debug) so fullText is never printed by
default; ensure sensitive content is never logged and, if you keep a debug path,
use a controlled logger method (logger.debug or similar) rather than
console.log.
---
Outside diff comments:
In `@components/UploadForm.tsx`:
- Line 493: The Button's disabled prop currently uses
form.formState.isSubmitting while the component also tracks submission with a
custom isSubmitting state (declared as isSubmitting around line 62), causing
inconsistency; update the Button to use the same submission state (use
isSubmitting) or synchronize the two by setting form.setSubmitting(...) whenever
you toggle the custom isSubmitting so the button and loading overlay share the
same source of truth (refer to the Button element and the isSubmitting state and
any place where you toggle submission state).
---
Nitpick comments:
In `@app/`(root)/books/[slug]/page.tsx:
- Around line 14-16: The current auth check in the page uses notFound() when
userId is falsy (see userId and notFound() in page.tsx); change this to redirect
unauthenticated users to the sign-in flow instead: replace the notFound() call
with a redirect to your sign-in route (e.g., redirect('/sign-in')) or call a
helper like requireAuth that redirects, ensuring the rest of the loader/page
logic only runs for authenticated users.
- Around line 35-39: Replace the native <img> with Next.js' Image component to
leverage automatic optimization and lazy loading: import Image from "next/image"
at the top of page.tsx and swap the <img src={book.coverURL} alt={`${book.title}
cover`} className="..."> usage for <Image> using the same book.coverURL and alt
text (provide appropriate width/height or use layout/fill and preserve the
existing className styling), ensuring the remote host matches the remotePatterns
in next.config.ts; update any props (priority, placeholder) as needed to
preserve behavior.
In `@app/api/upload/route.ts`:
- Around line 31-39: The handler currently only checks that file is a Blob but
lacks server-side size enforcement; before calling put(pathname, file, ...)
validate the incoming Blob's size (file.size) against the max allowed (e.g., 50
* 1024 * 1024 bytes) and return an appropriate error response (e.g.,
NextResponse.json({ error: "File too large" }, { status: 413 })) if exceeded;
keep the existing type/contentType checks and only call put(pathname, file, {
token: blobToken, access: "public", contentType: ... }) when the size is within
limits.
In `@components/UploadForm.tsx`:
- Around line 67-71: The component contains an unused delayed mount state:
remove the isMounted state and the useEffect that sets it (the setIsMounted call
inside the setTimeout) from UploadForm.tsx; specifically delete the useState
declaration for isMounted and the whole useEffect block that wraps setTimeout,
and also remove any remaining references or imports tied only to that state
(e.g., unused setIsMounted/useEffect if now unused) so there is no dead code
left.
In `@database/models/book.model.ts`:
- Around line 4-16: BookSchema currently only defines a unique index on slug but
queries also filter by clerkId (see BookSchema and usage in book.actions.ts);
add an index to improve query performance by calling BookSchema.index({ clerkId:
1 }) after the schema definition (or create a compound index BookSchema.index({
clerkId: 1, slug: 1 }) if you need fast lookups by both clerkId and slug), then
export the schema/model as before.
In `@database/models/voice-session.model.ts`:
- Around line 6-16: The schema currently creates redundant single-field indexes
on clerkId and billingPeriodStart while also adding a compound index
VoiceSessionSchema.index({ clerkId: 1, billingPeriodStart: 1 }); remove the
individual index flags on the clerkId field and the billingPeriodStart field
(and/or delete any separate index declaration for billingPeriodStart) so only
the compound index remains, leaving required/other options intact (e.g., keep
clerkId: { type: String, required: true } and billingPeriodStart: { type: Date,
required: true }); this reduces write/index overhead while preserving queries
that filter by clerkId or by the pair (clerkId, billingPeriodStart).
In `@database/mongoose.ts`:
- Around line 43-44: The console.info("Connected to MongoDB") runs on every
successful return from connectToDb(), causing noisy logs; change the function so
the "Connected to MongoDB" log is emitted only when a new connection is
established (i.e., in the branch where you create and assign cached and
cached.conn), not when returning the cached.conn, by moving or guarding the
console.info call to the fresh-connection path inside connectToDb() and keep
cached/cached.conn logic untouched otherwise.
- Around line 3-7: The non-null assertion on MONGODB_URI causes TypeScript to
assume a value before your runtime check; change the declaration to read the raw
environment value (e.g., assign process.env.MONGODB_URI without the trailing
"!"/assertion or explicitly type it as string|undefined), keep the existing
runtime guard (if (!MONGODB_URI) throw new Error(...)), and then use MONGODB_URI
normally afterwards (TypeScript will narrow it to string after the check);
update any uses in this file that relied on the prior assertion to rely on the
narrowed type instead.
In `@lib/actions/book.actions.ts`:
- Line 40: Remove or reduce verbose console.log usage that exposes user data in
lib/actions/book.actions.ts: locate the console.log calls (currently printing
"Saving book segments", clerkId, segments) and either delete them or replace
them with a structured logger call at an appropriate log level (e.g., debug)
that omits or redacts sensitive fields like clerkId and segment contents; ensure
you use the project's logger utility (or create one) and call it from the same
function where the console.log appears so operational details remain available
without leaking user data.
In `@lib/utils.ts`:
- Around line 87-91: formatDuration currently misformats when given non-integer
seconds; floor the input first and compute minutes/seconds from that integer so
decimals are dropped (e.g., in formatDuration, call Math.floor on seconds into a
local totalSeconds, then compute mins = Math.floor(totalSeconds/60) and secs =
totalSeconds % 60) and use secs.toString().padStart(2,'0') to ensure proper
formatting.
- Around line 117-133: The dynamically created canvas used in parsePDFFile
(created before firstPage.render and used to produce coverDataURL) is never
cleaned up; after generating the data URL and before leaving the function,
remove the canvas and clear references so it can be GC'd — call canvas.remove()
(or detach it if ever appended), set canvas.width = canvas.height = 0 and null
out the context reference (the variable holding the getContext result) and any
local canvas variable reference so memory is released; keep these cleanup steps
immediately after the coverDataURL is produced and before returning or calling
pdfDocument.destroy().
In `@lib/zod.ts`:
- Around line 19-27: The coverImage zod refinements contain redundant checks:
inside the schema using z.instanceof(File) and .optional(), remove the
unnecessary "!file" guard and the redundant "as string" cast on file.type in the
MIME-type refine; keep the same logic but change the refine callback to accept a
File and check ACCEPTED_IMAGE_TYPES.includes(file.type), and likewise for the
size refine use file.size <= MAX_IMAGE_SIZE; reference the coverImage field,
z.instanceof(File), ACCEPTED_IMAGE_TYPES, and MAX_IMAGE_SIZE when making the
adjustments.
In `@next.config.ts`:
- Around line 12-16: The remote image configuration in next.config.ts hardcodes
a single Vercel Blob hostname; update the remotePatterns entry (the object with
protocol/hostname/pathname) to accept wildcard Vercel Blob hostnames so it works
across environments (e.g., use a wildcard host pattern like
"*.public.blob.vercel-storage.com" or a glob pattern matching any subdomain of
public.blob.vercel-storage.com) and keep pathname "/**"; ensure this aligns with
how BLOB_READ_WRITE_TOKEN derives the blob URL in app/api/upload/route.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: de753edb-36a8-454d-872d-809af625b708
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
app/(root)/books/[slug]/page.tsxapp/(root)/page.tsxapp/api/upload/route.tsapp/layout.tsxcomponents/HeroSection.tsxcomponents/UploadForm.tsxcomponents/ui/sonner.tsxdatabase/models/book-segment.model.tsdatabase/models/book.model.tsdatabase/models/voice-session.model.tsdatabase/mongoose.tslib/actions/book.actions.tslib/utils.tslib/zod.tsnext.config.tspackage.json
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
lib/actions/book.actions.ts (2)
43-51: Redundant spread creates duplicatetextfield in saved documents.The
...segmentspreads all properties includingtext, but the schema expectscontent. This creates objects with both fields. Consider removing the spread for clarity.🧹 Proposed fix
const segmentsToSave = segments.map(segment => ({ - ...segment, bookId, clerkId, content: segment.text, - segmentIndex: segment.segmentIndex, - wordCount: segment.wordCount, + segmentIndex: segment.segmentIndex, pageNumber: segment.pageNumber, + wordCount: segment.wordCount, }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/actions/book.actions.ts` around lines 43 - 51, The mapping that builds segmentsToSave spreads the original segment (...segment) which leaves the original text property present alongside the new content property (content: segment.text); update the mapping in the segmentsToSave construction inside lib/actions/book.actions.ts to stop spreading ...segment and instead build the save object explicitly (e.g., { bookId, clerkId, content: segment.text, segmentIndex: segment.segmentIndex, wordCount: segment.wordCount, pageNumber: segment.pageNumber }) so the saved documents contain content only and no duplicate text field.
62-73: Consider using MongoDB transactions for atomic rollback.The compensating transaction pattern here could leave orphaned data if the cleanup operations fail. For production reliability, MongoDB multi-document transactions would ensure atomicity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/actions/book.actions.ts` around lines 62 - 73, The catch block currently performs manual compensating deletes (BookSegment.deleteMany and Book.findByIdAndDelete) which can leave orphaned data; change the save flow to use a MongoDB transaction via a Mongoose session: in the function that creates the Book and BookSegments, start a session (session.startTransaction()), perform all writes (Book.create/save, BookSegment.insertMany or save) with { session }, then on success await session.commitTransaction() and on any error await session.abortTransaction(); finally call session.endSession(). Replace the manual cleanup in the catch with aborting the transaction and ending the session, ensuring BookSegment.deleteMany and Book.findByIdAndDelete are not used as the primary rollback mechanism.lib/utils.ts (1)
86-91: Consider handling edge cases for fractional or negative seconds.If
secondscontains decimals (e.g.,65.7), the output becomes"1:05.699999..."; negative values also produce unexpected results. Consider rounding/flooring the seconds component.🔧 Proposed fix
export const formatDuration = (seconds: number): string => { + const totalSeconds = Math.max(0, Math.floor(seconds)); - const mins = Math.floor(seconds / 60); - const secs = seconds % 60; + const mins = Math.floor(totalSeconds / 60); + const secs = totalSeconds % 60; return `${mins}:${secs.toString().padStart(2, '0')}`; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/utils.ts` around lines 86 - 91, The formatDuration function doesn't handle fractional or negative inputs; convert the input to a non-negative integer before formatting by using Math.floor on the seconds and clamping to zero (e.g., secondsInt = Math.max(0, Math.floor(seconds))), then compute mins = Math.floor(secondsInt / 60) and secs = secondsInt % 60 and produce the padded string as before; update the export const formatDuration implementation to use this normalized secondsInt to avoid decimal tails and negative outputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/actions/book.actions.ts`:
- Line 41: Remove the debug console.log that prints full book content: replace
the line console.log("Saving book segments", bookId, clerkId, segments); with a
safe log that does not include the segments array (e.g., log only bookId,
clerkId and segments.length or a redacted indicator), or remove it entirely; use
the project's structured logger (processLogger or existing logger) instead of
console.log to avoid leaking sensitive/copyrighted text from the segments
variable.
- Around line 109-125: getAllBooks currently returns all books without
respecting user isolation; update getAllBooks to accept a clerkId parameter and
use it to filter the query (replace Book.find() with Book.find({ clerkId
}).sort(...)) so results are scoped to the requesting user, and propagate that
parameter where getAllBooks is called; reference function getAllBooks and the
Book.find call to locate and change the code.
In `@lib/utils.ts`:
- Around line 117-124: The code in lib/utils.ts calls document.createElement and
assumes a browser DOM (creating canvas, setting width/height, calling
getContext) which will throw during SSR; update the function that creates the
canvas (the block that calls document.createElement('canvas') /
canvas.getContext('2d')) to first guard for browser runtime (e.g. check typeof
window !== 'undefined' and typeof document !== 'undefined') and either throw a
clear error or return a fallback value when running server-side, or accept an
injectable Canvas/OffscreenCanvas parameter so callers can provide a
platform-appropriate canvas; ensure the guard is applied before
document.createElement is used and update any callers or docs to reflect that
this must run client-side or be provided a canvas.
---
Nitpick comments:
In `@lib/actions/book.actions.ts`:
- Around line 43-51: The mapping that builds segmentsToSave spreads the original
segment (...segment) which leaves the original text property present alongside
the new content property (content: segment.text); update the mapping in the
segmentsToSave construction inside lib/actions/book.actions.ts to stop spreading
...segment and instead build the save object explicitly (e.g., { bookId,
clerkId, content: segment.text, segmentIndex: segment.segmentIndex, wordCount:
segment.wordCount, pageNumber: segment.pageNumber }) so the saved documents
contain content only and no duplicate text field.
- Around line 62-73: The catch block currently performs manual compensating
deletes (BookSegment.deleteMany and Book.findByIdAndDelete) which can leave
orphaned data; change the save flow to use a MongoDB transaction via a Mongoose
session: in the function that creates the Book and BookSegments, start a session
(session.startTransaction()), perform all writes (Book.create/save,
BookSegment.insertMany or save) with { session }, then on success await
session.commitTransaction() and on any error await session.abortTransaction();
finally call session.endSession(). Replace the manual cleanup in the catch with
aborting the transaction and ending the session, ensuring BookSegment.deleteMany
and Book.findByIdAndDelete are not used as the primary rollback mechanism.
In `@lib/utils.ts`:
- Around line 86-91: The formatDuration function doesn't handle fractional or
negative inputs; convert the input to a non-negative integer before formatting
by using Math.floor on the seconds and clamping to zero (e.g., secondsInt =
Math.max(0, Math.floor(seconds))), then compute mins = Math.floor(secondsInt /
60) and secs = secondsInt % 60 and produce the padded string as before; update
the export const formatDuration implementation to use this normalized secondsInt
to avoid decimal tails and negative outputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5b79c007-cc71-4e9e-8070-13297d9e842a
📒 Files selected for processing (5)
.cursor/rules/nextjs-react-typescript-cursor-rules.mdc.gitignorecomponents/UploadForm.tsxlib/actions/book.actions.tslib/utils.ts
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- .cursor/rules/nextjs-react-typescript-cursor-rules.mdc
🚧 Files skipped from review as they are similar to previous changes (1)
- components/UploadForm.tsx
Comment on lines
+109
to
+125
| export const getAllBooks = async () => { | ||
| try { | ||
| await connectToDb(); | ||
| const books = await Book.find().sort({createdAt: -1}).lean(); | ||
| return { | ||
| success: true, | ||
| data: serializeData(books) | ||
| } | ||
| } catch (error) { | ||
| console.error("Error getting all books", error); | ||
| return { | ||
| success: false, | ||
| data: null, | ||
| error: error instanceof Error ? error.message : String(error) | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how getAllBooks is called in the codebase
rg -n "getAllBooks" --type ts --type tsx -A 3 -B 1Repository: prakashsingha/bookify
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Search for getAllBooks usage - fixed syntax
rg "getAllBooks" -A 3 -B 1Repository: prakashsingha/bookify
Length of output: 916
🏁 Script executed:
#!/bin/bash
# Check the actual file where getAllBooks is defined and surrounding context
head -130 lib/actions/book.actions.ts | tail -30Repository: prakashsingha/bookify
Length of output: 900
🏁 Script executed:
#!/bin/bash
# Look for the page component that uses getAllBooks
fd "page.tsx" -o "page.ts" | head -20Repository: prakashsingha/bookify
Length of output: 199
🏁 Script executed:
#!/bin/bash
# Read the full page component to understand its context and purpose
cat -n app/(root)/page.tsxRepository: prakashsingha/bookify
Length of output: 179
🏁 Script executed:
#!/bin/bash
# Check if there are user-specific book listing functions or if getAllBooks is the only way to list books
rg "\.find\(\)" lib/actions/book.actions.ts -B 2 -A 2Repository: prakashsingha/bookify
Length of output: 218
🏁 Script executed:
#!/bin/bash
# Check the Book model to understand if books have user/clerkId associations
fd -i "book" lib/models/ -type fRepository: prakashsingha/bookify
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Read the page component - proper quoting for path with parentheses
cat -n "app/(root)/page.tsx"Repository: prakashsingha/bookify
Length of output: 886
🏁 Script executed:
#!/bin/bash
# Find Book model file
find lib/models -name "*ook*" -o -name "*ook*"Repository: prakashsingha/bookify
Length of output: 111
🏁 Script executed:
#!/bin/bash
# List all model files
ls -la lib/models/Repository: prakashsingha/bookify
Length of output: 124
🏁 Script executed:
#!/bin/bash
# Find where Book model is defined
find . -name "*.ts" -o -name "*.js" | xargs grep -l "Book.*=" | grep -i modelRepository: prakashsingha/bookify
Length of output: 221
🏁 Script executed:
#!/bin/bash
# Search for mongoose model definitions
rg "mongoose.model.*Book|new Schema" --max-count 5Repository: prakashsingha/bookify
Length of output: 317
🏁 Script executed:
#!/bin/bash
# Look at directory structure
ls -la lib/Repository: prakashsingha/bookify
Length of output: 482
🏁 Script executed:
#!/bin/bash
# Read the Book model schema
cat -n database/models/book.model.tsRepository: prakashsingha/bookify
Length of output: 974
🏁 Script executed:
#!/bin/bash
# Check other book action functions to see their patterns with clerkId
rg "clerkId" lib/actions/book.actions.ts -B 2 -A 2Repository: prakashsingha/bookify
Length of output: 1385
🏁 Script executed:
#!/bin/bash
# Check if there are any other book listing functions that filter by clerkId
rg "Book\.find" lib/actions/book.actions.ts -B 2 -A 2Repository: prakashsingha/bookify
Length of output: 1506
🏁 Script executed:
#!/bin/bash
# Check if there's middleware or auth on the root page
rg "auth|middleware|currentUser|clerkId" "app/(root)/page.tsx"Repository: prakashsingha/bookify
Length of output: 47
getAllBooks returns all books without clerkId filtering, breaking the user isolation pattern used throughout the file.
Every other book function filters by clerkId for data isolation (createBook, checkBookExists, getBookBySlug all use Book.findOne({ slug, clerkId })), but getAllBooks uses Book.find() with no filter, exposing all users' books. Since the Book model requires clerkId and the root page has no auth context, either:
- This function should accept a
clerkIdparameter and filter by it, or - The root page should require authentication to view only the current user's books, or
- A separate public/shared book listing should be created if this intentional behavior is desired
Clarify the intended scope and update accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/actions/book.actions.ts` around lines 109 - 125, getAllBooks currently
returns all books without respecting user isolation; update getAllBooks to
accept a clerkId parameter and use it to filter the query (replace Book.find()
with Book.find({ clerkId }).sort(...)) so results are scoped to the requesting
user, and propagate that parameter where getAllBooks is called; reference
function getAllBooks and the Book.find call to locate and change the code.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/UploadForm.tsx (1)
150-154:⚠️ Potential issue | 🟡 MinorRemove unreachable post-upload null checks.
uploadViaServerthrows on failure and returns{ url, pathname }on success, so theseif (!uploadedPdf)/if (!uploadedCover)branches are dead code.✂️ Proposed cleanup
const uploadedPdf = await uploadViaServer( `${fileTitle}.pdf`, pdfFile, "application/pdf", ); - - if(!uploadedPdf){ - toast.error("Failed to upload PDF file. Please try again with a different file."); - form.reset(); - return; - } let uploadedCover: { url: string; pathname: string } | null = null; if (formData.coverImage && formData.coverImage.size > 0) { const coverFile = formData.coverImage; uploadedCover = await uploadViaServer( `${fileTitle}_cover.png`, coverFile, coverFile.type || "image/png", ); - - if(!uploadedCover){ - toast.error("Failed to upload cover image. Please try again with a different file."); - form.reset(); - return; - } }else{Also applies to: 166-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/UploadForm.tsx` around lines 150 - 154, The null-check branches after calling uploadViaServer (e.g., the if (!uploadedPdf) and if (!uploadedCover) blocks in UploadForm.tsx) are unreachable because uploadViaServer throws on failure and returns a {url, pathname} object on success; remove these dead branches and related toast/form.reset logic, leaving error handling to the existing try/catch around uploadViaServer (or adjust the surrounding try/catch to catch and handle errors from uploadViaServer). Also update any variable typing or subsequent code that assumed a nullable uploadedPdf/uploadedCover so they treat the results as non-null { url, pathname } objects returned by uploadViaServer (references: uploadViaServer, uploadedPdf, uploadedCover).
🧹 Nitpick comments (1)
components/UploadForm.tsx (1)
5-71: Remove the mount timer state (or at least clean it up).
isMountedis set via timeout but not used in rendering. This adds an unnecessary async state update and potential post-unmount setState. Prefer removing this state/effect entirely.♻️ Proposed simplification
-import { useEffect, useRef, useState } from "react"; +import { useRef, useState } from "react"; const UploadForm = () => { const [isSubmitting, setIsSubmitting] = useState(false); - const [isMounted, setIsMounted] = useState(false); const { userId } = useAuth(); const router = useRouter(); - useEffect(() => { - setTimeout(() => { - setIsMounted(true); - }, 100); - }, []); - - const pdfInputRef = useRef<HTMLInputElement>(null);As per coding guidelines: "Minimize 'use client', 'useEffect', and 'setState'; favor React Server Components (RSC)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/UploadForm.tsx` around lines 5 - 71, The isMounted state and its useEffect in UploadForm create an unnecessary async update and potential post-unmount setState; remove the isMounted and setIsMounted declarations and delete the associated useEffect and timeout call (or if you must keep the delay, store the timeout id and clear it in a cleanup function to avoid setState after unmount). Update any references to isMounted in the component (there are none now) and ensure only isSubmitting, useAuth(), and router remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/UploadForm.tsx`:
- Around line 144-213: After uploading PDF and cover (uploadedPdf and
uploadedCover), add compensating cleanup before any early return when createBook
or saveBookSegments fail (or when book.alreadyExists): call a server-side blob
delete API (e.g., deleteBlob/deleteUploadedBlob/deleteBlobByKey — implement if
missing) with uploadedPdf.pathname and uploadedCover.pathname to remove those
blobs, then proceed with the existing toast/reset/router returns; do this in the
failure branch after createBook returns !book.success, in the book.alreadyExists
branch before router.push, and in the branch where saveBookSegments returns
!segments.success so uploaded files aren't left orphaned.
---
Duplicate comments:
In `@components/UploadForm.tsx`:
- Around line 150-154: The null-check branches after calling uploadViaServer
(e.g., the if (!uploadedPdf) and if (!uploadedCover) blocks in UploadForm.tsx)
are unreachable because uploadViaServer throws on failure and returns a {url,
pathname} object on success; remove these dead branches and related
toast/form.reset logic, leaving error handling to the existing try/catch around
uploadViaServer (or adjust the surrounding try/catch to catch and handle errors
from uploadViaServer). Also update any variable typing or subsequent code that
assumed a nullable uploadedPdf/uploadedCover so they treat the results as
non-null { url, pathname } objects returned by uploadViaServer (references:
uploadViaServer, uploadedPdf, uploadedCover).
---
Nitpick comments:
In `@components/UploadForm.tsx`:
- Around line 5-71: The isMounted state and its useEffect in UploadForm create
an unnecessary async update and potential post-unmount setState; remove the
isMounted and setIsMounted declarations and delete the associated useEffect and
timeout call (or if you must keep the delay, store the timeout id and clear it
in a cleanup function to avoid setState after unmount). Update any references to
isMounted in the component (there are none now) and ensure only isSubmitting,
useAuth(), and router remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0791f2fe-542e-4881-8f59-2e4f0e3d8e1e
📒 Files selected for processing (3)
components/UploadForm.tsxcomponents/ui/sonner.tsxlib/actions/book.actions.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- components/ui/sonner.tsx
- lib/actions/book.actions.ts
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
components/UploadForm.tsx (1)
145-213:⚠️ Potential issue | 🟠 MajorRollback uploaded blobs on every post-upload failure path.
After
uploadedPdf/uploadedCoversucceed, the branches for!book.success,book.alreadyExists, and!segments.successreturn without deleting the blobs. That leaves orphaned public files and ongoing storage cost even though the book was never fully created. Add a server-side delete path and call it before each of those returns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/UploadForm.tsx` around lines 145 - 213, The upload paths for uploadedPdf and uploadedCover can leave orphaned blobs when createBook or saveBookSegments fail (branches checking !book.success, book.alreadyExists, and !segments.success); add a server-side blob-delete endpoint (e.g., deleteBlob or deleteBlobs) and call it before each early return to remove uploadedPdf.pathname and uploadedCover.pathname when they exist; update the UploadForm flow around uploadedPdf/uploadedCover, createBook, and saveBookSegments to invoke the delete API and await its completion (handle errors gracefully) prior to form.reset() / router.push() to ensure no orphaned files remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/UploadForm.tsx`:
- Around line 115-223: The native file inputs aren't cleared by form.reset() in
onSubmit, so create a single helper (e.g., resetFormAndFiles) that calls
form.reset() and also clears the file input refs (e.g.,
pdfInputRef.current.value = '' and coverInputRef.current.value = '' if they
exist) and use that helper everywhere you currently call form.reset() (error
branches, upload failures, and after success) so selecting the same file will
reliably trigger onChange; update onSubmit to call resetFormAndFiles instead of
form.reset().
In `@lib/actions/book.actions.ts`:
- Around line 12-13: After calling generateSlug(data.title) and assigning to
slug, add a guard that rejects requests where slug === "" (empty string) so we
return a clear validation error immediately instead of proceeding; locate the
slug assignment and following logic around generateSlug and Book.findOne in the
create flow (e.g., the function that computes slug and later calls Book.create)
and throw or return a ValidationError / HTTP 400 with a message like "Invalid
title: could not generate slug" before attempting any DB writes or file
processing.
- Around line 13-23: The action layer (createBook) checks uniqueness per user
via Book.findOne({ slug, clerkId }), but the Mongoose schema still marks slug as
globally unique; update the book model (database/models/book.model.ts) to remove
the singular unique: true on the slug field and instead declare a composite
unique index on { clerkId, slug } (unique: true) so DB constraints match the
createBook logic; also ensure any old global unique index is dropped or migrated
so duplicate-key errors for different clerkId values are avoided.
In `@lib/utils.ts`:
- Around line 113-182: The PDF parsing currently only calls
pdfDocument.destroy() on the success path, so exceptions leave the PDF/worker
alive; move the loadingTask/pdfDocument scope out so they can be cleaned up in a
finally block and always call pdfDocument.destroy() (and/or
loadingTask.destroy/cancel if available) when set; update the code around
pdfjsLib.getDocument(...) / loadingTask.promise and the subsequent pdfDocument
usage (getPage, render, getTextContent, streamTextContent) to ensure any thrown
error still triggers the finally that disposes pdfDocument and avoids resource
leaks while keeping the returned cover/segments logic unchanged.
---
Duplicate comments:
In `@components/UploadForm.tsx`:
- Around line 145-213: The upload paths for uploadedPdf and uploadedCover can
leave orphaned blobs when createBook or saveBookSegments fail (branches checking
!book.success, book.alreadyExists, and !segments.success); add a server-side
blob-delete endpoint (e.g., deleteBlob or deleteBlobs) and call it before each
early return to remove uploadedPdf.pathname and uploadedCover.pathname when they
exist; update the UploadForm flow around uploadedPdf/uploadedCover, createBook,
and saveBookSegments to invoke the delete API and await its completion (handle
errors gracefully) prior to form.reset() / router.push() to ensure no orphaned
files remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0520c3b6-03f4-498c-b4c5-2d61c2e68dc5
📒 Files selected for processing (4)
app/(root)/page.tsxcomponents/UploadForm.tsxlib/actions/book.actions.tslib/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/(root)/page.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/UploadForm.tsx (1)
530-533:⚠️ Potential issue | 🟡 MinorInconsistent disabled state between button and overlay.
The button uses
form.formState.isSubmitting(line 530) while the loading overlay uses the localisSubmittingstate (line 273). These could diverge, causing the button to be re-enabled while the overlay is still visible (or vice versa). Use one source of truth.♻️ Proposed fix
- <Button type="submit" className="form-btn" disabled={form.formState.isSubmitting}> + <Button type="submit" className="form-btn" disabled={isSubmitting}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/UploadForm.tsx` around lines 530 - 533, The button and the loading overlay are using two different submission states (form.formState.isSubmitting vs local isSubmitting) which can diverge; pick one source of truth (preferably form.formState.isSubmitting from your react-hook-form instance) and update the overlay to use form.formState.isSubmitting (or conversely switch the Button disabled to use local isSubmitting) so both UI elements read the same boolean; change all references of the other variable (the local isSubmitting or the form.formState.isSubmitting) to the chosen symbol to keep button, overlay, and any related logic consistent.
♻️ Duplicate comments (1)
lib/actions/book.actions.ts (1)
19-29:⚠️ Potential issue | 🟠 MajorDB schema uniqueness constraint still misaligned with action logic.
The action checks
{ slug, clerkId }butdatabase/models/book.model.tsstill hasslugas globallyunique: true. Two different users uploading the same title will hit a duplicate-key error at the database level despite passing the action'sfindOnecheck.🛠️ Required schema fix in database/models/book.model.ts
- slug: { type: String, required: true, unique: true, lowercase: true, trim: true }, + slug: { type: String, required: true, lowercase: true, trim: true }, }, { timestamps: true }); + +BookSchema.index({ clerkId: 1, slug: 1 }, { unique: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/actions/book.actions.ts` around lines 19 - 29, The model enforces slug as globally unique while the action logic treats uniqueness as per-user (slug + clerkId); update the Book Mongoose schema (database/models/book.model.ts) by removing unique: true from the slug field and add a compound unique index on { clerkId, slug } (e.g., schema.index({ clerkId: 1, slug: 1 }, { unique: true })) so the database uniqueness matches the action's check in Book-related actions (e.g., the flow that calls Book.findOne({ slug, clerkId: data.clerkId }) and Book.create(...)).
🧹 Nitpick comments (7)
components/UploadForm.tsx (4)
541-541: Consider using named export for the component.The coding guidelines recommend named exports for components for better refactoring support and explicit imports.
♻️ Proposed fix
-export default UploadForm; +export { UploadForm };As per coding guidelines: Favor named exports for components.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/UploadForm.tsx` at line 541, Replace the default export of the component with a named export: convert the UploadForm default export into a named export (e.g., export function UploadForm(...) or export const UploadForm = ...) and remove the trailing "export default UploadForm;"; then update any importing modules to import { UploadForm } instead of default-importing it; ensure the component identifier UploadForm remains unchanged so references are easy to update.
192-196: Unreachable null check afteruploadViaServer.
uploadViaServerthrows on failure (line 108), souploadedPdfwill never be falsy at this point. This check is dead code.♻️ Proposed fix
const uploadedPdf = await uploadViaServer( `${fileTitle}.pdf`, pdfFile, "application/pdf", ); - - if(!uploadedPdf){ - toast.error("Failed to upload PDF file. Please try again with a different file."); - resetFormAndFiles(); - return; - } let uploadedCover: { url: string; pathname: string } | null = null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/UploadForm.tsx` around lines 192 - 196, The null check for uploadedPdf after calling uploadViaServer is dead because uploadViaServer throws on failure; remove the unreachable block that checks if(!uploadedPdf) and the subsequent toast.error and resetFormAndFiles() call, or alternatively change uploadViaServer to return null on failure and handle that explicitly — update code around uploadViaServer, uploadedPdf, and resetFormAndFiles accordingly so only one failure-handling path remains.
63-71: Remove unusedisMountedstate and effect.
isMountedis set but never read anywhere in the component. This is dead code that adds unnecessary complexity.♻️ Proposed fix
const UploadForm = () => { const [isSubmitting, setIsSubmitting] = useState(false); - const [isMounted, setIsMounted] = useState(false); const { userId } = useAuth(); const router = useRouter(); - - useEffect(() => { - setTimeout(() => { - setIsMounted(true); - }, 100); - }, []); - -As per coding guidelines: Minimize 'use client', 'useEffect', and 'setState'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/UploadForm.tsx` around lines 63 - 71, Remove the dead isMounted logic: delete the useState declaration for isMounted/setIsMounted and the useEffect block that sets it (including the setTimeout), and then remove any now-unused imports (e.g., useState) from the top of the UploadForm component so no unused state or effects remain; search for symbols isMounted and setIsMounted in UploadForm.tsx to ensure all references are removed.
208-212: Unreachable null check afteruploadViaServer.Same issue as above —
uploadViaServerthrows on failure, so this check is unreachable.♻️ Proposed fix
uploadedCover = await uploadViaServer( `${fileTitle}_cover.png`, coverFile, coverFile.type || "image/png", ); - - if(!uploadedCover){ - toast.error("Failed to upload cover image. Please try again with a different file."); - resetFormAndFiles(); - return; - } }else{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/UploadForm.tsx` around lines 208 - 212, The null check for uploadedCover after calling uploadViaServer is unreachable because uploadViaServer throws on failure; remove the unreachable if(!uploadedCover){...} block and instead ensure the upload call is inside the surrounding try/catch so any thrown error triggers toast.error(...) and resetFormAndFiles(); specifically, keep the uploadViaServer call (where uploadedCover is assigned), remove the subsequent uploadedCover null branch, and handle failures in the catch that calls toast.error and resetFormAndFiles to avoid dead code.lib/utils.ts (2)
98-103: Replaceanytypes with more specific type definitions.ESLint flags
anyusage. While full pdfjs-dist types can be complex, you can define minimal interfaces for the shapes you use.♻️ Proposed fix
- let loadingTask: { - promise: Promise<any>; + interface PDFDocumentProxy { + destroy: () => Promise<void> | void; + numPages: number; + getPage: (pageNum: number) => Promise<PDFPageProxy>; + } + + interface PDFPageProxy { + getViewport: (options: { scale: number }) => { width: number; height: number }; + render: (params: { canvas: HTMLCanvasElement; canvasContext: CanvasRenderingContext2D; viewport: { width: number; height: number } }) => { promise: Promise<void> }; + getTextContent: () => Promise<{ items: Array<{ str?: string }> }>; + streamTextContent: () => ReadableStreamDefaultReader<{ items?: Array<{ str?: string }> }>; + } + + let loadingTask: { + promise: Promise<PDFDocumentProxy>; destroy?: () => Promise<void> | void; cancel?: () => void; } | null = null; - let pdfDocument: { destroy: () => Promise<void> | void; numPages: number; getPage: (pageNum: number) => Promise<any>; } | null = null; + let pdfDocument: PDFDocumentProxy | null = null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/utils.ts` around lines 98 - 103, Replace the broad any types by declaring minimal local interfaces for the PDF shapes and use them for loadingTask and pdfDocument: define an interface (e.g., LoadingTaskShape) with promise: Promise<unknown>, optional destroy(): Promise<void> | void, optional cancel(): void and another interface (e.g., PdfDocumentShape) with destroy(): Promise<void> | void, numPages: number, getPage(pageNum: number): Promise<unknown>; then change the types of loadingTask and pdfDocument to LoadingTaskShape | null and PdfDocumentShape | null (and update any usages expecting specific return types like promise and getPage to handle unknown/typed results). Ensure all references to promise, destroy, cancel, numPages, and getPage use these new interface names.
87-91: Consider adding input validation for edge cases.
formatDurationproduces unexpected results for negative numbers,NaN, orInfinity. If this function might receive untrusted input, add a guard.🛡️ Optional defensive guard
export const formatDuration = (seconds: number): string => { + if (!Number.isFinite(seconds) || seconds < 0) { + return '0:00'; + } const mins = Math.floor(seconds / 60); const secs = seconds % 60; return `${mins}:${secs.toString().padStart(2, '0')}`; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/utils.ts` around lines 87 - 91, formatDuration lacks input validation and will yield incorrect outputs for negative numbers, NaN, or Infinity; update the function (formatDuration) to first coerce/validate the input by ensuring seconds is a finite non-negative number (e.g., let s = Number(seconds); if (!Number.isFinite(s) || s < 0) s = 0 or throw), then compute mins/secs from Math.floor(s) and return the padded string; include this guard so untrusted or malformed inputs produce a safe fallback like "0:00" (or explicitly throw) instead of producing weird results.lib/actions/book.actions.ts (1)
47-55: Spreading...segmentincludes fields not in the schema.The spread includes
segment.textandsegment.segmentIndex, which are then overwritten by explicit assignments. While Mongoose's defaultstrict: truedrops unknown fields, the spread is unnecessary and confusing.♻️ Proposed fix
const segmentsToSave = segments.map(segment => ({ - ...segment, bookId, clerkId, content: segment.text, - segmentIndex: segment.segmentIndex, + segmentIndex: segment.segmentIndex, wordCount: segment.wordCount, pageNumber: segment.pageNumber, }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/actions/book.actions.ts` around lines 47 - 55, The code spreads ...segment into segmentsToSave which can include fields not in the Mongoose schema; remove the spread and construct each saved object explicitly using only the allowed properties (e.g., content: segment.text, segmentIndex: segment.segmentIndex, wordCount: segment.wordCount, pageNumber: segment.pageNumber) plus bookId and clerkId in the segmentsToSave mapping (the segmentsToSave constant and the segments.map callback are the symbols to update).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@components/UploadForm.tsx`:
- Around line 530-533: The button and the loading overlay are using two
different submission states (form.formState.isSubmitting vs local isSubmitting)
which can diverge; pick one source of truth (preferably
form.formState.isSubmitting from your react-hook-form instance) and update the
overlay to use form.formState.isSubmitting (or conversely switch the Button
disabled to use local isSubmitting) so both UI elements read the same boolean;
change all references of the other variable (the local isSubmitting or the
form.formState.isSubmitting) to the chosen symbol to keep button, overlay, and
any related logic consistent.
---
Duplicate comments:
In `@lib/actions/book.actions.ts`:
- Around line 19-29: The model enforces slug as globally unique while the action
logic treats uniqueness as per-user (slug + clerkId); update the Book Mongoose
schema (database/models/book.model.ts) by removing unique: true from the slug
field and add a compound unique index on { clerkId, slug } (e.g., schema.index({
clerkId: 1, slug: 1 }, { unique: true })) so the database uniqueness matches the
action's check in Book-related actions (e.g., the flow that calls Book.findOne({
slug, clerkId: data.clerkId }) and Book.create(...)).
---
Nitpick comments:
In `@components/UploadForm.tsx`:
- Line 541: Replace the default export of the component with a named export:
convert the UploadForm default export into a named export (e.g., export function
UploadForm(...) or export const UploadForm = ...) and remove the trailing
"export default UploadForm;"; then update any importing modules to import {
UploadForm } instead of default-importing it; ensure the component identifier
UploadForm remains unchanged so references are easy to update.
- Around line 192-196: The null check for uploadedPdf after calling
uploadViaServer is dead because uploadViaServer throws on failure; remove the
unreachable block that checks if(!uploadedPdf) and the subsequent toast.error
and resetFormAndFiles() call, or alternatively change uploadViaServer to return
null on failure and handle that explicitly — update code around uploadViaServer,
uploadedPdf, and resetFormAndFiles accordingly so only one failure-handling path
remains.
- Around line 63-71: Remove the dead isMounted logic: delete the useState
declaration for isMounted/setIsMounted and the useEffect block that sets it
(including the setTimeout), and then remove any now-unused imports (e.g.,
useState) from the top of the UploadForm component so no unused state or effects
remain; search for symbols isMounted and setIsMounted in UploadForm.tsx to
ensure all references are removed.
- Around line 208-212: The null check for uploadedCover after calling
uploadViaServer is unreachable because uploadViaServer throws on failure; remove
the unreachable if(!uploadedCover){...} block and instead ensure the upload call
is inside the surrounding try/catch so any thrown error triggers
toast.error(...) and resetFormAndFiles(); specifically, keep the uploadViaServer
call (where uploadedCover is assigned), remove the subsequent uploadedCover null
branch, and handle failures in the catch that calls toast.error and
resetFormAndFiles to avoid dead code.
In `@lib/actions/book.actions.ts`:
- Around line 47-55: The code spreads ...segment into segmentsToSave which can
include fields not in the Mongoose schema; remove the spread and construct each
saved object explicitly using only the allowed properties (e.g., content:
segment.text, segmentIndex: segment.segmentIndex, wordCount: segment.wordCount,
pageNumber: segment.pageNumber) plus bookId and clerkId in the segmentsToSave
mapping (the segmentsToSave constant and the segments.map callback are the
symbols to update).
In `@lib/utils.ts`:
- Around line 98-103: Replace the broad any types by declaring minimal local
interfaces for the PDF shapes and use them for loadingTask and pdfDocument:
define an interface (e.g., LoadingTaskShape) with promise: Promise<unknown>,
optional destroy(): Promise<void> | void, optional cancel(): void and another
interface (e.g., PdfDocumentShape) with destroy(): Promise<void> | void,
numPages: number, getPage(pageNum: number): Promise<unknown>; then change the
types of loadingTask and pdfDocument to LoadingTaskShape | null and
PdfDocumentShape | null (and update any usages expecting specific return types
like promise and getPage to handle unknown/typed results). Ensure all references
to promise, destroy, cancel, numPages, and getPage use these new interface
names.
- Around line 87-91: formatDuration lacks input validation and will yield
incorrect outputs for negative numbers, NaN, or Infinity; update the function
(formatDuration) to first coerce/validate the input by ensuring seconds is a
finite non-negative number (e.g., let s = Number(seconds); if
(!Number.isFinite(s) || s < 0) s = 0 or throw), then compute mins/secs from
Math.floor(s) and return the padded string; include this guard so untrusted or
malformed inputs produce a safe fallback like "0:00" (or explicitly throw)
instead of producing weird results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: abd09d36-0494-410a-9a27-edcbf3266a90
📒 Files selected for processing (4)
app/api/upload/route.tscomponents/UploadForm.tsxlib/actions/book.actions.tslib/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/api/upload/route.ts
This was referenced Apr 29, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
UI Changes
Chores