(SP: 1) [Database] Optimize Neon usage to reduce CU-hours#103
Conversation
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughReplace Postgres Pool with Neon serverless, add environment-aware DB URLs and a Netlify preview guard, introduce 5-minute ISR/revalidation and React/Next caching for several queries/routes, add DB indexes, tighten OAuth selects and image fallbacks, and update seed/migrate scripts to use the guard. Changes
Sequence Diagram(s)(Skipped — changes are broad but do not introduce a single new multi-component control flow that benefits from a sequence diagram.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-30T16:01:02.523ZApplied to files:
🔇 Additional comments (3)
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. Comment |
ViktorSvertoka
requested review from
LesiaUKR,
kryvosheyin and
liudmylasovetovs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
frontend/next.config.ts (1)
34-38: Consider caching strategy for Dicebear avatars.The Dicebear API pattern introduces an external service dependency. While this is fine for generated avatars, consider implementing a caching strategy (e.g., storing generated avatars in your CDN or serving them through Next.js Image optimization) to reduce external API calls and improve performance. This would align well with the PR's goal of reducing infrastructure costs.
frontend/app/api/auth/google/callback/route.ts (1)
156-156: Remove trailing whitespace.Line 156 has trailing whitespace after the closing brace.
🔎 Proposed fix
-} +}frontend/app/api/questions/[category]/route.ts (1)
112-125: Consider brief caching for error responses.The error response path (line 115-124) does not set Cache-Control headers. While this may be intentional to avoid caching transient errors, adding a short-lived cache (e.g.,
public, s-maxage=10) could reduce load during temporary database issues or incident scenarios.🔎 Optional: Add short-lived caching to error response
return NextResponse.json( { items: [], total: 0, page: 1, totalPages: 0, locale: DEFAULT_LOCALE, }, - { status: 500 } + { + status: 500, + headers: { + 'Cache-Control': 'public, s-maxage=10, stale-while-revalidate=20' + } + } );frontend/app/api/auth/github/callback/route.ts (2)
176-176: Remove trailing whitespace.Line 176 has trailing whitespace (same issue as in the Google callback route).
🔎 Proposed fix
-} +}
133-136: Pattern is consistent across both OAuth callbacks and properly handles legacy data.The defensive
emailUser.image !== "null"check in the GitHub callback (lines 133-136) matches the identical pattern in the Google callback. Both routes correctly handle legacy data where the literal string"null"was stored instead of a NULL value.If there is historical data with
image = 'null', consider a one-time cleanup:SELECT COUNT(*) FROM users WHERE image = 'null'; UPDATE users SET image = NULL WHERE image = 'null';frontend/db/queries/leaderboard.ts (2)
34-34: Redundant fallback for points field.The expression
u.points || 0is redundant because thepointscolumn in the database schema has.notNull().default(0), guaranteeing it will never be NULL. While this doesn't cause issues, it's unnecessary defensive code.🔎 Simplification
- points: u.points || 0, + points: u.points,
26-27: Consider pinning or documenting Dicebear API version.The avatar URL uses Dicebear API version
9.x, which may receive breaking changes. While the floating version provides automatic updates, it could cause avatar inconsistencies if the API changes.Consider either:
- Pinning to a specific version (e.g.,
9.4) for stability- Adding a comment documenting the version choice
This is a low-priority optimization and can be addressed in future work if avatar rendering issues arise.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
frontend/app/[locale]/leaderboard/page.tsxfrontend/app/api/auth/github/callback/route.tsfrontend/app/api/auth/google/callback/route.tsfrontend/app/api/questions/[category]/route.tsfrontend/db/index.tsfrontend/db/queries/leaderboard.tsfrontend/db/queries/quiz.tsfrontend/db/schema/questions.tsfrontend/db/schema/quiz.tsfrontend/db/schema/shop.tsfrontend/db/schema/users.tsfrontend/lib/env/index.tsfrontend/next.config.tsfrontend/package.jsonfrontend/scripts/guard-non-preview.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-30T16:01:02.523Z
Learnt from: kryvosheyin
Repo: DevLoversTeam/devlovers.net PR: 88
File: frontend/drizzle/schema.ts:153-153
Timestamp: 2025-12-30T16:01:02.523Z
Learning: The file frontend/drizzle/schema.ts is a legacy file not used by Drizzle or the runtime. The canonical schemas live in frontend/db/schema/* as configured in drizzle-config.ts, and that's what migrations and runtime use.
Applied to files:
frontend/db/index.tsfrontend/db/schema/users.tsfrontend/package.jsonfrontend/db/schema/shop.ts
🧬 Code graph analysis (8)
frontend/app/[locale]/leaderboard/page.tsx (1)
frontend/app/api/questions/[category]/route.ts (1)
revalidate(6-6)
frontend/db/schema/users.ts (3)
frontend/drizzle/schema.ts (2)
users(11-26)quizAttempts(224-245)frontend/drizzle/relations.ts (1)
usersRelations(13-17)frontend/db/schema/quiz.ts (1)
quizAttempts(129-163)
frontend/db/schema/questions.ts (2)
frontend/drizzle/schema.ts (2)
questions(166-179)categories(58-65)frontend/db/schema/categories.ts (1)
categories(12-19)
frontend/app/api/auth/github/callback/route.ts (3)
frontend/db/schema/users.ts (1)
users(11-45)frontend/drizzle/schema.ts (1)
users(11-26)frontend/db/index.ts (1)
db(33-33)
frontend/app/api/auth/google/callback/route.ts (3)
frontend/db/schema/users.ts (1)
users(11-45)frontend/drizzle/schema.ts (1)
users(11-26)frontend/db/index.ts (1)
db(33-33)
frontend/db/queries/leaderboard.ts (3)
frontend/components/leaderboard/types.ts (1)
User(1-8)frontend/db/index.ts (1)
db(33-33)frontend/db/schema/users.ts (1)
users(11-45)
frontend/app/api/questions/[category]/route.ts (3)
frontend/app/[locale]/leaderboard/page.tsx (1)
revalidate(4-4)frontend/drizzle/schema.ts (1)
categories(58-65)frontend/db/schema/categories.ts (1)
categories(12-19)
frontend/db/queries/quiz.ts (3)
frontend/db/index.ts (1)
db(33-33)frontend/db/schema/quiz.ts (3)
quizzes(19-42)quizTranslations(44-57)quizAttempts(129-163)frontend/db/schema/categories.ts (2)
categories(12-19)categoryTranslations(21-33)
🔇 Additional comments (25)
frontend/next.config.ts (1)
24-38: Implementation is correct and remote patterns are necessary.The three remote image patterns are properly configured and actively used in the application:
- api.dicebear.com: Directly used throughout the codebase for generated avatar images
- avatars.githubusercontent.com and lh3.googleusercontent.com: Required for OAuth avatar handling (GitHub and Google authentication routes are present)
All patterns follow Next.js best practices with HTTPS protocol and appropriate pathname matching.
Consider implementing caching strategies for Dicebear API responses to further reduce external API calls and align with the PR's cost-reduction objectives.
frontend/scripts/guard-non-preview.ts (4)
6-9: LGTM! Preview guard logic is correct.The guard correctly exits with code 0 in preview contexts, allowing the npm script chain to continue without triggering errors while safely skipping DB operations. This aligns perfectly with the PR objective to reduce Neon costs in preview environments.
11-16: LGTM! Command validation is proper.The command extraction from
process.argv.slice(2)correctly skips the node executable and script path. The validation with an informative error message and exit code 1 provides good developer feedback when the guard is misused.
18-19: LGTM! Command execution with proper error handling.The use of
spawnSyncwithstdio: 'inherit'correctly pipes output to the parent process. The defensive?? 1fallback properly handles cases whereresult.statusisnullorundefined, ensuring the script exits with a failure code if the spawned command status is unavailable.Note: While the script executes arbitrary commands, this is acceptable since it's only invoked from controlled npm scripts defined in package.json.
1-4: The environment detection logic is correct.CONTEXTis Netlify's standard read-only environment variable, anddeploy-previewandbranch-deployare the exact context values Netlify uses. The fallback toNETLIFY_CONTEXTprovides additional robustness, thoughCONTEXTshould be available in all Netlify builds.frontend/package.json (2)
15-15: LGTM! Migration script correctly integrates the guard.The syntax for wrapping the drizzle-kit migrate command is correct. The guard will receive
drizzle-kitas the command with the migrate subcommand and config flag as arguments, properly executing the migration in non-preview contexts. The drizzle config file and guard script both exist in the expected locations.
10-12: LGTM! Seed scripts correctly integrate the guard.All referenced seed files exist (
seed-categories.ts,seed-questions.ts,seed-users.ts) and the guard script is in place. The syntax for wrapping seed commands through the guard script is correct—the guard will receivetsxas the command and the seed file paths as arguments, properly executing them in non-preview contexts.frontend/db/schema/shop.ts (1)
7-7: LGTM! Foreign key indexes improve join performance.The added indexes on
order_items.order_idandproduct_prices.product_idare well-placed. These foreign key columns are frequently used in joins and lookups (e.g., fetching all items for an order, or prices for a product), and the indexes will significantly reduce query costs by avoiding table scans.The naming convention is consistent and the implementation is correct.
Also applies to: 184-184, 253-253
frontend/db/schema/questions.ts (1)
10-10: LGTM! Composite index optimizes category-based question queries.The composite index on
(categoryId, sortOrder)is well-designed for queries that fetch questions within a category, ordered by their sort position. The column order is optimal: filtering bycategoryIdfirst, then ordering bysortOrder.This aligns with the PR's objective to reduce CU-hours by avoiding full table scans on filtered queries.
Also applies to: 15-37
frontend/lib/env/index.ts (1)
9-10: Implementation correctly handles optional database URLs.The new optional environment variables
DATABASE_URL_PREVIEWandDATABASE_URL_DEVare properly consumed inresolveDatabaseUrl()atfrontend/db/index.tswith appropriate fallback logic:
- Preview deploys require
DATABASE_URL_PREVIEWand explicitly throw an error if missing- Development environments optionally use
DATABASE_URL_DEVif available, then fall back toDATABASE_URL- Production and other environments use
DATABASE_URLwith error handlingNo runtime errors will occur from accessing undefined optional variables.
frontend/db/schema/quiz.ts (1)
40-40: The slug-only index is justified and necessary.Verification confirms that the application performs slug-only lookups on the
quizzestable:
- getQuizBySlugCached (frontend/db/queries/quiz.ts, line 98) queries by
slugwithout filtering bycategoryId:.where(eq(quizzes.slug, slug))- Public API endpoint (frontend/app/api/quiz/[slug]/route.ts) uses this cached query to expose quizzes publicly by slug alone
- Database seed scripts also rely on slug-only lookups across multiple files
The composite unique index
(categoryId, slug)cannot efficiently serve these slug-only queries sincecategoryIdis the leading column. The single-column index onslugis therefore necessary for query performance.frontend/app/api/auth/google/callback/route.ts (2)
86-105: Explicit field selections optimize data transfer.The explicit field selections for both
googleUserandemailUserqueries are well-aligned with the PR's goal to reduce Neon CU-hours by minimizing data transfer. This ensures only necessary fields are retrieved, reducing bandwidth and query cost.
111-114: Defensive "null" string check is appropriate and appears intentional across the codebase.The check for
emailUser.image !== "null"is confirmed in three separate locations (Google OAuth, GitHub OAuth, and leaderboard queries), all using identical defensive patterns. This suggests the check addresses a real or anticipated data quality issue rather than being unnecessary paranoia.The pattern indicates that the
imagefield may contain the literal string"null"rather than a proper NULL value. While the exact source of these strings cannot be determined from code inspection (likely legacy data, external API responses, or historical imports), the consistent implementation across multiple features confirms this is a known concern.The code correctly handles this edge case. To confirm whether cleanup is needed, query the production database for records where
image = 'null'on theuserstable.frontend/app/api/questions/[category]/route.ts (3)
6-6: ISR configuration aligns with PR caching strategy.The 5-minute revalidation interval (
revalidate = 300) is consistent with the caching strategy introduced across the PR (e.g., leaderboard page, unstable_cache in queries) and will reduce database hits for this API route.
36-36: Optimized category lookup reduces data transfer.Selecting only
categories.idinstead of all fields reduces unnecessary data transfer, which directly supports the PR's goal to minimize Neon CU-hours.
42-53: Cache-Control headers properly configured.The Cache-Control headers (
public, s-maxage=300, stale-while-revalidate=600) are correctly applied to both the not-found and success response paths. Thes-maxage=300aligns with the ISR revalidation interval, andstale-while-revalidate=600provides a reasonable grace period for background revalidation.Also applies to: 100-111
frontend/app/[locale]/leaderboard/page.tsx (1)
4-4: ISR configuration effectively reduces database load.Replacing
dynamic = 'force-dynamic'withrevalidate = 300is an excellent optimization. This shifts the page from rendering on every request to incremental static regeneration with a 5-minute revalidation interval, significantly reducing database hits while maintaining reasonably fresh data. This change directly supports the PR's goal to minimize Neon CU-hours.The 5-minute interval is consistent with the caching strategy in
getLeaderboardData()and other routes in this PR.frontend/app/api/auth/github/callback/route.ts (1)
109-127: Explicit field selections optimize data transfer.The explicit field selections for both
githubUserandemailUserqueries are well-aligned with the PR's goal to reduce Neon CU-hours. This pattern is consistently applied across OAuth providers (see identical changes infrontend/app/api/auth/google/callback/route.ts), ensuring minimal data transfer.frontend/db/queries/leaderboard.ts (3)
1-2: Double caching pattern correctly implemented.The combination of
unstable_cache(Next.js server-side cache with 5-minute revalidation) and Reactcache(request-level deduplication) is a correct and beneficial pattern:
unstable_cache: Caches the database query result across requests with a 300-second TTL- React
cache: Deduplicates calls within a single request/render cycleThis two-tier caching strategy will significantly reduce database load:
- Multiple components calling
getLeaderboardData()in the same request will share one result (React cache)- Subsequent requests within 5 minutes will use the cached data (unstable_cache)
The implementation correctly defines
getLeaderboardDataCachedonce (outside), then wraps its invocation with React cache. This aligns perfectly with the PR's goal to minimize Neon CU-hours.Also applies to: 8-46
11-16: Explicit field selection optimizes query performance.Selecting only the required fields (
id,username,points,avatar) reduces data transfer and query cost, directly supporting the PR's database optimization goals.
23-28: Avatar fallback with "null" check is well-implemented.The avatar generation logic properly handles missing or invalid avatar data:
- Checks for both falsy values and the literal string
"null"(consistent with OAuth callback changes)- Generates deterministic Dicebear avatars using username and ID as seed
- Ensures all users have a valid avatar URL
frontend/db/index.ts (3)
9-29: LGTM! Environment-based database URL resolution is well-structured.The logic correctly routes preview, development, and production environments to their respective databases with clear error messages.
One clarification: In development (lines 20-22), if
DATABASE_URL_DEVis not set, the function falls through toDATABASE_URL. Is this fallback intentional to allow developers to optionally use a production replica?
31-33: LGTM! Neon serverless initialization is correct.The drizzle instance is properly initialized with the neon HTTP client and schema. This eliminates persistent connections and aligns with the PR's goal to reduce CU-hours.
1-2: Compatibility verified — no changes needed.The migration to neon-http is compatible with drizzle-orm 0.44.7 and Next.js 16.0.7. The imports follow the documented Neon and Drizzle patterns, and the HTTP API usage is appropriate for the serverless environment.
frontend/db/queries/quiz.ts (1)
310-314: LGTM! Excellent query optimization.Selecting only the required fields (
score,percentage,completedAt) instead of all columns reduces data transfer and aligns with the PR's performance goals.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
frontend/db/queries/quiz.ts (2)
68-113: Cache key now correctly includes parameters—critical issue resolved.The cache key at line 107 now includes both
slugandlocale:['quiz-by-slug', slug, locale]. This resolves the critical cache collision issue flagged in previous reviews where different quizzes/locales would return the same cached result.The layered caching approach is sound:
cache()deduplicates calls within a single requestunstable_cache()caches across requests with 5-minute revalidation
115-158: Cache key now correctly includes locale—critical issue resolved.The cache key at line 152 now includes the
localeparameter:['active-quizzes', locale]. This resolves the critical internationalization bug flagged in previous reviews where all locales would return the same cached quiz list.
🧹 Nitpick comments (1)
frontend/db/schema/users.ts (1)
6-6: Consider usinguniqueconstraint instead ofuniqueIndexfor OAuth data integrity.The critical security issue has been addressed—duplicate (provider, providerId) pairs are now prevented. However, the legacy schema used a
uniqueconstraint, and for logical data integrity requirements (preventing duplicate OAuth associations), a UNIQUE constraint is semantically more appropriate than a unique index:
unique(): Declares a data integrity rule as part of the table's logical schemauniqueIndex(): Creates an index that happens to enforce uniqueness (typically for query optimization)Both enforce uniqueness at the database level, but for OAuth authentication—where preventing duplicate provider associations is a business rule rather than a performance optimization—a constraint better expresses the intent.
🔎 Suggested refinement to align with legacy schema semantics
import { pgTable, text, integer, timestamp, - uniqueIndex, + unique, } from "drizzle-orm/pg-core";table => ({ - providerProviderIdUq: uniqueIndex( + providerProviderIdUq: unique( "users_provider_provider_id_unique" ).on(table.provider, table.providerId), })Also applies to: 40-42
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/db/queries/quiz.tsfrontend/db/schema/users.tsfrontend/db/seed-users.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-30T16:01:02.523Z
Learnt from: kryvosheyin
Repo: DevLoversTeam/devlovers.net PR: 88
File: frontend/drizzle/schema.ts:153-153
Timestamp: 2025-12-30T16:01:02.523Z
Learning: The file frontend/drizzle/schema.ts is a legacy file not used by Drizzle or the runtime. The canonical schemas live in frontend/db/schema/* as configured in drizzle-config.ts, and that's what migrations and runtime use.
Applied to files:
frontend/db/schema/users.ts
🧬 Code graph analysis (2)
frontend/db/schema/users.ts (1)
frontend/drizzle/relations.ts (1)
usersRelations(13-17)
frontend/db/queries/quiz.ts (3)
frontend/db/index.ts (1)
db(33-33)frontend/db/schema/quiz.ts (3)
quizzes(19-42)quizTranslations(44-57)quizAttempts(129-163)frontend/db/schema/categories.ts (2)
categories(12-19)categoryTranslations(21-33)
🔇 Additional comments (6)
frontend/db/schema/users.ts (1)
11-44: LGTM! Table structure is well-defined.The table restructuring to the 3-argument form with a table callback is correct, all field definitions match the legacy schema, and the constraints are properly configured. The unique constraint on email (line 21) and the table-level uniqueness enforcement on (provider, providerId) both work as intended.
frontend/db/seed-users.ts (3)
4-4: LGTM — aligns with serverless architecture.Removing
closeDbfrom imports is consistent with the migration to neon-http serverless connections. HTTP-based database connections are stateless and don't require explicit closure.
60-65: LGTM — appropriate exit handling for serverless context.Removing
closeDb()calls and using explicitprocess.exit()is correct for the neon-http serverless architecture. The process termination handles all cleanup automatically.
42-46: Index onusers.emailalready exists via unique constraint.The
.unique()modifier, which creates a unique index. This provides the optimization needed to avoid full table scans when filtering by email in the seed query.frontend/db/queries/quiz.ts (2)
1-2: Cache imports are in place; critical key issues from past reviews resolved.The imports for React's
cacheand Next.js'sunstable_cacheare correctly added. Past reviews flagged concerns aboutunstable_cachebeing experimental—that guidance remains valid. The critical cache key issues (missing slug/locale parameters) identified in prior reviews have now been addressed in lines 107 and 152.
310-314: Optimization verified—all selected fields are used safely.The three fields selected (
score,percentage,completedAt) are exactly those accessed by the only consumer infrontend/app/[locale]/dashboard/page.tsx: the dashboard calculatesrealPointsfromscore,averageScorefrompercentage, andlastActiveDatefromcompletedAt. No breaking changes will occur.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/db/index.ts (1)
7-7: Consider conditional dotenv loading.In serverless environments like Netlify, environment variables are pre-populated and
dotenv.config()is redundant. Consider loading dotenv only in development to avoid potential issues if.envfiles are accidentally deployed.🔎 Proposed refactor
-dotenv.config(); +if (process.env.NODE_ENV === 'development') { + dotenv.config(); +}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/db/index.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-30T16:01:02.523Z
Learnt from: kryvosheyin
Repo: DevLoversTeam/devlovers.net PR: 88
File: frontend/drizzle/schema.ts:153-153
Timestamp: 2025-12-30T16:01:02.523Z
Learning: The file frontend/drizzle/schema.ts is a legacy file not used by Drizzle or the runtime. The canonical schemas live in frontend/db/schema/* as configured in drizzle-config.ts, and that's what migrations and runtime use.
Applied to files:
frontend/db/index.ts
🔇 Additional comments (3)
frontend/db/index.ts (3)
9-35: Well-structured environment-based URL resolution.The function correctly prioritizes preview deployments, then development-specific URLs, and finally the default production URL. The error handling is appropriate, and the fallback warning for preview builds (lines 18-21) provides visibility while preventing complete failure.
37-37: LGTM: Neon serverless client initialization.The use of
neon(resolveDatabaseUrl())correctly creates a serverless HTTP-based database client. The URL is resolved at module initialization, which is appropriate for serverless environments where process lifecycles are short.
39-39: LGTM: Drizzle initialization with Neon serverless.The migration from Pool-based connections to
drizzle(sql, { schema })with the Neon HTTP client correctly implements serverless-safe database access, directly supporting the PR's goal of reducing CU-hours through connection pooling elimination.
(SP: 1) [Database] Optimize Neon usage to reduce CU-hours
1 participant
What was done
This PR focuses on reducing Neon CU-hours usage by optimizing database access
patterns without changing application behavior or business logic.
Key improvements:
Why
Neon billing is primarily driven by:
These changes significantly reduce unnecessary database work, leading to:
Expected impact
Out of scope
Summary by CodeRabbit
Performance Improvements
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.