🛠️ Refactor suggestion | 🟠 Major

#!/bin/bash
# Description: Find all FastAPI app instance declarations

rg -n "app = FastAPI\(" --type=py

#!/bin/bash
sed -n '1,200p' api/index.py
echo "---"
sed -n '1,200p' backend/app/main.py

Consolidate FastAPI app definition
Remove the duplicate app = FastAPI(...) in api/index.py and instead import or include the existing app (or its routers) from backend/app/main.py to avoid conflicting FastAPI instances.

In api/index.py around lines 1 to 8 you are creating a second FastAPI instance
which conflicts with the existing app in backend/app/main.py; remove the
FastAPI(...) instantiation here and instead import the existing app (from
backend.app.main import app) or import and include the routers from
backend.app.main (from backend.app.main import router;
app.include_router(router)) so this module re-uses the main application rather
than creating a duplicate instance.

Avoid CORS wildcard with credentials

Initializing CORSMiddleware with allow_origins=["*"] while allow_credentials=True causes Starlette to raise ValueError at startup (“Cannot use allow_credentials True when allow_origins=['*']”). As written, the minimal API under api/ cannot even start, so any deployment that imports this module will crash before serving requests. Consider either listing specific origins or disabling credentials.

Useful? React with 👍 / 👎.

⚠️ Potential issue | 🔴 Critical

Critical security issue: Overly permissive CORS configuration.

The CORS configuration allows all origins (["*"]) with allow_credentials=True, which is a security vulnerability. This configuration:

• Exposes the API to CSRF attacks from any origin
• Violates the CORS specification (browsers reject Access-Control-Allow-Origin: * when credentials are included)
• Should be restricted to specific, trusted origins in production

Apply this diff to fix the security issue:

 app.add_middleware(
     CORSMiddleware,
-    allow_origins=["*"],
+    allow_origins=[
+        "http://localhost:3000",  # Local development
+        # Add production frontend URLs here
+    ],
     allow_credentials=True,
-    allow_methods=["*"],
-    allow_headers=["*"],
+    allow_methods=["GET", "POST", "PUT", "DELETE", "PATCH"],
+    allow_headers=["Content-Type", "Authorization"],
 )

In api/index.py around lines 11 to 17 the CORS setup is overly permissive
(allow_origins=["*"] with allow_credentials=True). Replace the wildcard origin
with a restricted list of trusted origins loaded from configuration or an
environment variable (e.g., ALLOWED_ORIGINS or config.ALLOWED_ORIGINS), ensure
allow_credentials remains True only when specific origins are provided (never
use "*" with credentials), and if running in non-production allow a development
fallback (e.g., localhost origins) while enforcing strict origins in production;
update tests/config docs accordingly.

⚠️ Potential issue | 🔴 Critical

Critical: Incorrect serverless handler implementation for FastAPI.

This handler implementation has two critical issues:

1. Incorrect ASGI invocation: FastAPI is an ASGI application and cannot be invoked directly as app(request, response). ASGI apps require an ASGI server (like Uvicorn) or proper ASGI-to-HTTP adapter.

2. Import path assumption: The relative import from app.main import app assumes execution from the backend/ directory, but Vercel serverless functions execute from the project root, which will cause import failures.

For Vercel deployment of FastAPI, use the Mangum adapter which properly bridges ASGI to AWS Lambda/Vercel's handler signature:

-from app.main import app
+from mangum import Mangum
+from backend.app.main import app

-# This is the Vercel serverless function handler
-def handler(request, response):
-    return app(request, response)
+# Vercel serverless function handler using Mangum adapter
+handler = Mangum(app, lifespan="off")

Add mangum to backend/requirements.txt:

 fastapi>=0.115.0
 uvicorn[standard]>=0.32.0
 sqlalchemy>=2.0.36
 pydantic>=2.10.0
 python-multipart>=0.0.17
 httpx>=0.28.0
+mangum>=0.19.0

Verify the Vercel configuration routes to this handler correctly:

#!/bin/bash
# Description: Check Vercel configuration for API routing

echo "=== Checking vercel.json for API routing configuration ==="
if [ -f "vercel.json" ]; then
  cat vercel.json | jq '.routes, .rewrites, .builds' 2>/dev/null || cat vercel.json
else
  echo "vercel.json not found"
fi

In backend/api/main.py lines 1-5, the handler incorrectly calls the FastAPI app
directly and uses a relative import that will fail from the project root;
replace this with the Mangum ASGI adapter and an absolute import: import the
FastAPI app using its package-absolute path (matching the project layout) and
create a Mangum adapter instance (e.g., handler = Mangum(app)) to expose as the
serverless entrypoint; also add mangum to backend/requirements.txt and verify
vercel.json routes/rewrites point to backend/api/main.py as the API entry so
Vercel invokes the correct handler.

⚠️ Potential issue | 🟡 Minor

Remove unused import.

The os module is imported but never used.

 from sqlalchemy import create_engine
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.orm import sessionmaker
-import os

In backend/app/database.py around line 4, the os module is imported but never
used; remove the unused import line "import os" from the top of the file to
eliminate the unused dependency and any related linter warnings.

🛠️ Refactor suggestion | 🟠 Major

Make database URL configurable via environment variables.

The database URL is hardcoded, which prevents using different databases for development, testing, and production environments.

+import os
+
 # SQLite database URL
-SQLALCHEMY_DATABASE_URL = "sqlite:///./ai_book_reader.db"
+SQLALCHEMY_DATABASE_URL = os.getenv(
+    "DATABASE_URL",
+    "sqlite:///./ai_book_reader.db"
+)

 # Create SQLite engine
 engine = create_engine(
-    SQLALCHEMY_DATABASE_URL, 
-    connect_args={"check_same_thread": False}
+    SQLALCHEMY_DATABASE_URL,
+    connect_args={"check_same_thread": False} if SQLALCHEMY_DATABASE_URL.startswith("sqlite") else {}
 )

In backend/app/database.py around lines 6 to 13, the database URL is hardcoded;
update the file to read the database URL from an environment variable (e.g.,
DATABASE_URL) with the current sqlite value as a safe default, and use that
value when building the SQLAlchemy engine; ensure
connect_args={"check_same_thread": False} is only passed for SQLite (detect by
checking if the URL starts with "sqlite://") so other DBs (Postgres, MySQL) are
created without SQLite-specific args.

⚠️ Potential issue | 🟠 Major

Replace with proper migration tooling for production.

Creating tables at module import time is acceptable for initial development but has significant limitations:

• No schema version control or migration history.
• Concurrent deployments (multiple serverless instances on Vercel) may race to create tables.
• Schema changes require manual intervention or risk data loss.

For production readiness, integrate Alembic to manage migrations with versioning and rollback capabilities.

Would you like me to generate an Alembic setup with initial migration scripts?

CORS config blocks all Vercel subdomains

The backend CORS setup lists "https://*.vercel.app" in allow_origins, but Starlette treats entries as literal strings, not glob patterns. As a result, an origin such as https://myapp.vercel.app will not match and browsers will reject cross‑origin requests, preventing the deployed Next.js frontend from calling the API. Use allow_origin_regex=r"https://.*\.vercel\.app" or enumerate allowed origins instead of a wildcard string.

Useful? React with 👍 / 👎.

⚠️ Potential issue | 🔴 Critical

Fix CORS configuration: wildcard origins incompatible with credentials.

The current CORS configuration has a critical flaw: browsers reject wildcard origins (https://*.vercel.app) when allow_credentials=True. This will cause CORS errors in production.

Additionally, allowing all methods and headers (["*"]) is overly permissive and expands the attack surface.

Solution 1 (recommended): Use specific allowed origins

-    allow_origins=["http://localhost:3000", "https://*.vercel.app"],
+    allow_origins=[
+        "http://localhost:3000",
+        "https://your-app.vercel.app",  # Replace with actual domain
+    ],
     allow_credentials=True,
-    allow_methods=["*"],
-    allow_headers=["*"],
+    allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS"],
+    allow_headers=["Content-Type", "Authorization"],

Solution 2: Dynamic origin validation (if multiple Vercel deployments needed)

from fastapi.middleware.cors import CORSMiddleware
import re

def is_allowed_origin(origin: str) -> bool:
    allowed_patterns = [
        r"^http://localhost:3000$",
        r"^https://.*\.vercel\.app$",
    ]
    return any(re.match(pattern, origin) for pattern in allowed_patterns)

app.add_middleware(
    CORSMiddleware,
    allow_origin_regex=r"^(http://localhost:3000|https://.*\.vercel\.app)$",
    allow_credentials=True,
    allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS"],
    allow_headers=["Content-Type", "Authorization"],
)

In backend/app/main.py around lines 18-25, the CORS config uses a wildcard
origin ("https://*.vercel.app") together with allow_credentials=True and overly
permissive methods/headers; browsers will reject wildcard origins with
credentials and the current allow_methods/allow_headers are too broad. Replace
the wildcard origin with either a concrete list of allowed origins or use
allow_origin_regex to validate Vercel subdomains (ensuring the regex matches
only intended hosts), keep allow_credentials=True only with these specific/or
regex-validated origins, and restrict allow_methods to the explicit HTTP verbs
your API uses (e.g., GET, POST, PUT, DELETE, OPTIONS) and allow_headers to only
required headers (e.g., Content-Type, Authorization) instead of ["*"].

⚠️ Potential issue | 🟠 Major

Replace deprecated .dict() with .model_dump().

The .dict() method is deprecated in Pydantic v2 (used by FastAPI 0.104.1+). Use .model_dump() instead to avoid future compatibility issues.

Apply this diff:

 def create_note(note: schemas.NoteCreate, db: Session = Depends(get_db)):
-    db_note = models.Note(**note.dict())
+    db_note = models.Note(**note.model_dump())
     db.add(db_note)
     db.commit()
     db.refresh(db_note)

In backend/app/main.py around lines 37 to 42, replace the deprecated Pydantic v1
usage of note.dict() with the v2-compatible note.model_dump() when constructing
the Note model; update the call to models.Note(**note.model_dump()) so the code
uses model_dump() (optionally pass exclusions like exclude_none=True if needed)
and leave the rest of the DB add/commit/refresh logic unchanged.

⚠️ Potential issue | 🟠 Major

Add database error handling for constraint violations.

Database operations can fail due to constraint violations (unique keys, foreign keys, check constraints). Without error handling, these will return 500 errors with raw SQL exceptions exposed to clients.

Wrap database operations with exception handling:

+from sqlalchemy.exc import IntegrityError
+
 def create_note(note: schemas.NoteCreate, db: Session = Depends(get_db)):
-    db_note = models.Note(**note.model_dump())
-    db.add(db_note)
-    db.commit()
-    db.refresh(db_note)
-    return db_note
+    try:
+        db_note = models.Note(**note.model_dump())
+        db.add(db_note)
+        db.commit()
+        db.refresh(db_note)
+        return db_note
+    except IntegrityError as e:
+        db.rollback()
+        raise HTTPException(status_code=400, detail="Database constraint violation")

Apply similar error handling to all create, update, and delete operations.

Committable suggestion skipped: line range outside the PR's diff.

In backend/app/main.py around lines 37 to 42, the create_note DB write lacks
error handling for constraint violations; wrap the add/commit/refresh sequence
in a try/except that catches SQLAlchemy exceptions (e.g.,
sqlalchemy.exc.IntegrityError and sqlalchemy.exc.DBAPIError), call db.rollback()
on exception, and raise an appropriate fastapi.HTTPException with a safe
client-facing status code and message (e.g., 400 or 409 with a concise
"constraint violation" note). Ensure the original exception is not leaked to the
client but is logged for debugging. Apply the same pattern (try/except,
rollback, HTTPException + logging) to all create, update, and delete DB
operations in the codebase.

⚠️ Potential issue | 🟠 Major

Replace deprecated .dict() with .model_dump() and add error handling.

Same issues as the notes endpoints: use .model_dump() instead of the deprecated .dict() method, and add database error handling for constraint violations.

Apply this diff:

+from sqlalchemy.exc import IntegrityError
+
 def create_bookmark(bookmark: schemas.BookmarkCreate, db: Session = Depends(get_db)):
-    db_bookmark = models.Bookmark(**bookmark.dict())
-    db.add(db_bookmark)
-    db.commit()
-    db.refresh(db_bookmark)
-    return db_bookmark
+    try:
+        db_bookmark = models.Bookmark(**bookmark.model_dump())
+        db.add(db_bookmark)
+        db.commit()
+        db.refresh(db_bookmark)
+        return db_bookmark
+    except IntegrityError as e:
+        db.rollback()
+        raise HTTPException(status_code=400, detail="Database constraint violation")

In backend/app/main.py around lines 81 to 86, replace the deprecated
BookmarkCreate.dict() call with BookmarkCreate.model_dump(), and wrap the DB
operations (add/commit/refresh) in a try/except that catches SQLAlchemy
IntegrityError (or relevant DB constraint exception), calls db.rollback() on
error, and raises an appropriate fastapi.HTTPException (e.g., 400 or 409) with a
clear message; also add the necessary imports for IntegrityError and
HTTPException at the top of the file.

⚠️ Potential issue | 🟠 Major

Fix updated_at to include server_default.

The updated_at column has onupdate=func.now() but no server_default, so it will be NULL on initial insert. This creates inconsistent behavior where the field is only populated after the first update.

Apply this diff to fix the issue:

-    updated_at = Column(DateTime(timezone=True), onupdate=func.now())
+    updated_at = Column(DateTime(timezone=True), server_default=func.now(), onupdate=func.now())

In backend/app/models.py around lines 14 to 15, the updated_at column uses
onupdate=func.now() but lacks a server_default so it will be NULL on initial
insert; update the Column definition to include server_default=func.now() (and
optionally nullable=False) so the timestamp is set on create and on subsequent
updates, ensuring consistent non-NULL values.