feat: added block editor for managing page details#4
Conversation
WalkthroughAdds BlockNote-powered page blocks with GET/POST /pages/:pageId/blocks endpoints, a client-side BlockNote editor and store, Zod schemas for blocks/endpoints, HttpError subclasses and route-wrapper error handling, lazy environment/auth/logger initialization, an auth-clear script, scoped CSS, and CI/package/docs updates. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Page as Page View
participant Store as $currentPageBlocks
participant Editor as PageDetailEditor
participant API as /api/v1/pages/:id/blocks
participant Repo as Repository/DB
User->>Page: open page
Page->>Store: set $currentPageBlocksId(id)
Store->>API: GET /pages/{id}/blocks
API->>Repo: query page scoped to user
Repo-->>API: return page.blocks
API-->>Store: { blocks: [...] }
Store-->>Page: emit blocks
Page->>Editor: render(initialContent)
User->>Editor: edit content
Note over Editor: debounce 5000ms
Editor->>Page: onUpdate(blocks)
Page->>API: POST /pages/{id}/blocks (SetPageBlocksBody)
API->>Repo: update page.blocks
Repo-->>API: confirmation
API-->>Page: success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
scripts/clear-auth.ts (1)
9-12: Fix SQL formatting inconsistencies.The DELETE statements have inconsistent spacing and casing (
FROMvsfrom).Apply this diff for consistent formatting:
-await connection.execute('DELETE FROM verification;'); -await connection.execute('DELETE from session;'); -await connection.execute('DELETE from account;'); -await connection.execute('DELETE FROM user;'); +await connection.execute('DELETE FROM verification;'); +await connection.execute('DELETE FROM session;'); +await connection.execute('DELETE FROM account;'); +await connection.execute('DELETE FROM user;');src/components/organisms/page-detail-editor.tsx (2)
17-28: Consider reducing debounce delay for better UX.The 5000ms (5 second) debounce delay means users won't see their changes saved for 5 seconds after stopping typing. This could lead to:
- Users thinking their changes aren't being saved
- Data loss if they close the tab within the 5-second window
- Poor user experience
Consider reducing to 1000-2000ms:
- { delay: 5000, flushOnUnmount: true } + { delay: 1000, flushOnUnmount: true }Alternatively, implement a visual "saving..." indicator so users know their changes are pending.
16-28: Add error handling for the onUpdate callback.The
onUpdatecallback has no error handling. If the callback throws or the save operation fails, users won't be notified and could lose data.Add error handling:
const onChange = useDebouncedCallback( useCallback( (editor: BlockNoteEditor) => { if (!onUpdate) { return; } - onUpdate(editor.document); + try { + onUpdate(editor.document); + } catch (error) { + console.error('Failed to save changes:', error); + // TODO: Show error notification to user + } }, [onUpdate] ), { delay: 5000, flushOnUnmount: true } );.github/copilot-instructions.md (1)
5-5: Minor grammar fix: use hyphen in compound adjective.The phrase "High level" should be hyphenated as "High-level" when used as a compound adjective before a noun.
Apply this diff:
-High level +High-levelsrc/types/schemas/entities/container.ts (1)
24-25: Placeholder schema defeats type safety.The
blockSchemais currently defined asz.any()(insrc/types/schemas/blocks.ts), which bypasses Zod's validation and TypeScript's type checking for the blocks field. This creates a type safety gap where invalid block structures could pass validation.Consider generating proper Zod schemas from BlockNote's block types to restore type safety. Would you like me to help create a schema generator or open an issue to track this work?
src/types/schemas/blocks.ts (1)
3-4: Type safety bypass withz.any()placeholder.Using
z.any()forblockSchemadisables runtime validation and compile-time type checking for all block data flowing through the API. This means invalid or malformed blocks could be accepted and persisted, potentially causing runtime errors in the editor or corrupting data.To restore type safety, generate Zod schemas that match BlockNote's block structure. Would you like help creating a schema generator or opening an issue to prioritize this work?
src/app/globals.css (1)
5-7: Consider if !important is necessary.The
!importantflag typically indicates specificity issues. If BlockNote's default styles can be overridden through more specific selectors or theme configuration, that would be preferable.src/app/pages/[id]/page.tsx (1)
82-82: Consider loading state for page blocks.The editor only renders when
pageBlocksexists, but there's no visual feedback while blocks are loading. Users might see an empty page briefly before the editor appears.Consider showing a loading indicator while blocks are being fetched:
- {pageBlocks && <PageDetailEditor initialContent={pageBlocks.blocks} onUpdate={updateBlocks} />} + {!pageBlocks && loading && <Loader />} + {pageBlocks && <PageDetailEditor initialContent={pageBlocks.blocks} onUpdate={updateBlocks} />}Note: You'll need to also destructure
loadingfrom the$currentPageBlocksstore on line 19.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
.github/copilot-instructions.md(1 hunks)package.json(1 hunks)scripts/clear-auth.ts(1 hunks)src/app/api/v1/pages/[id]/blocks/route.ts(1 hunks)src/app/api/v1/pages/[id]/route.ts(2 hunks)src/app/globals.css(1 hunks)src/app/layout.tsx(1 hunks)src/app/pages/[id]/create/page.tsx(1 hunks)src/app/pages/[id]/page.tsx(2 hunks)src/components/molecules/tree-node.tsx(2 hunks)src/components/organisms/page-detail-editor.tsx(1 hunks)src/lib/api/route-wrapper.ts(2 hunks)src/lib/auth/client.ts(1 hunks)src/lib/auth/session.ts(2 hunks)src/lib/errors/http-error.ts(1 hunks)src/lib/errors/not-authorized-error.ts(1 hunks)src/lib/errors/not-found-error.ts(1 hunks)src/lib/store/query/get-page-blocks.ts(1 hunks)src/types/api/endpoints/get-page-blocks.ts(1 hunks)src/types/api/endpoints/set-page-blocks.ts(1 hunks)src/types/api/entities.ts(1 hunks)src/types/schemas/blocks.ts(1 hunks)src/types/schemas/entities/container.ts(2 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Prefer TypeScript type aliases (
type) overinterfacewhen defining shapes
Files:
src/app/api/v1/pages/[id]/route.tssrc/lib/errors/not-authorized-error.tsscripts/clear-auth.tssrc/lib/errors/not-found-error.tssrc/lib/errors/http-error.tssrc/types/schemas/entities/container.tssrc/lib/auth/client.tssrc/lib/api/route-wrapper.tssrc/types/api/endpoints/get-page-blocks.tssrc/components/molecules/tree-node.tsxsrc/types/schemas/blocks.tssrc/types/api/entities.tssrc/app/pages/[id]/page.tsxsrc/lib/auth/session.tssrc/types/api/endpoints/set-page-blocks.tssrc/app/layout.tsxsrc/components/organisms/page-detail-editor.tsxsrc/lib/store/query/get-page-blocks.tssrc/app/api/v1/pages/[id]/blocks/route.tssrc/app/pages/[id]/create/page.tsx
src/app/api/**/route.@(ts|tsx)
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Implement API endpoints as Next.js route handlers under
src/app/api/**/route.ts(x)
Files:
src/app/api/v1/pages/[id]/route.tssrc/app/api/v1/pages/[id]/blocks/route.ts
src/components/@(atoms|molecules|organisms|templates)/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Organize UI components following Atomic Design: atoms, molecules, organisms, templates
Files:
src/components/molecules/tree-node.tsxsrc/components/organisms/page-detail-editor.tsx
src/{components,app}/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Use Mantine UI 8 components for building React UI
Files:
src/components/molecules/tree-node.tsxsrc/app/pages/[id]/page.tsxsrc/app/layout.tsxsrc/components/organisms/page-detail-editor.tsxsrc/app/pages/[id]/create/page.tsx
src/app/**/@(page|layout|loading|error).tsx
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Define routes using Next.js App Router conventions under
src/appwithpage.tsx,layout.tsx,loading.tsx, anderror.tsx
Files:
src/app/pages/[id]/page.tsxsrc/app/layout.tsxsrc/app/pages/[id]/create/page.tsx
🧬 Code graph analysis (14)
src/app/api/v1/pages/[id]/route.ts (2)
src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)src/lib/errors/not-found-error.ts (1)
NotFoundError(3-7)
src/lib/errors/not-authorized-error.ts (1)
src/lib/errors/http-error.ts (1)
HttpError(1-17)
scripts/clear-auth.ts (1)
src/lib/database/index.ts (1)
getDatabase(8-18)
src/lib/errors/not-found-error.ts (1)
src/lib/errors/http-error.ts (1)
HttpError(1-17)
src/types/schemas/entities/container.ts (1)
src/types/schemas/blocks.ts (1)
blockSchema(4-4)
src/lib/api/route-wrapper.ts (2)
src/lib/logger.ts (1)
logger(4-17)src/lib/errors/http-error.ts (1)
HttpError(1-17)
src/types/api/endpoints/get-page-blocks.ts (1)
src/types/schemas/blocks.ts (1)
Block(6-6)
src/components/molecules/tree-node.tsx (1)
src/components/atoms/tree-item.tsx (1)
TreeItem(11-79)
src/app/pages/[id]/page.tsx (6)
src/lib/store/query/get-page-details.ts (2)
$currentPage(7-7)$currentPageId(5-5)src/lib/store/query/get-page-blocks.ts (2)
$currentPageBlocks(7-11)$currentPageBlocksId(5-5)src/lib/hooks/use-cud-api.ts (1)
useCudApi(5-44)src/types/schemas/blocks.ts (1)
Block(6-6)src/types/api/endpoints/set-page-blocks.ts (1)
SetPageBlocksBody(18-18)src/components/organisms/page-detail-editor.tsx (1)
PageDetailEditor(16-42)
src/lib/auth/session.ts (1)
src/lib/errors/not-authorized-error.ts (1)
NotAuthorizedError(3-7)
src/types/api/endpoints/set-page-blocks.ts (1)
src/types/schemas/blocks.ts (1)
blockSchema(4-4)
src/components/organisms/page-detail-editor.tsx (1)
src/types/schemas/blocks.ts (1)
Block(6-6)
src/lib/store/query/get-page-blocks.ts (2)
src/lib/store/fetcher.ts (1)
createFetcherStore(4-6)src/types/api/endpoints/get-page-blocks.ts (1)
GetPageBlocksResponse(17-19)
src/app/api/v1/pages/[id]/blocks/route.ts (7)
src/app/api/v1/pages/[id]/route.ts (1)
GET(8-35)src/lib/api/route-wrapper.ts (1)
apiRoute(15-126)src/types/api/endpoints/get-page-blocks.ts (3)
GetPageBlocksResponse(17-19)GetPageBlocksParameters(10-10)getPageBlocksParametersSchema(6-8)src/lib/database/index.ts (1)
getContainerRepository(20-23)src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)src/lib/errors/not-found-error.ts (1)
NotFoundError(3-7)src/types/api/endpoints/set-page-blocks.ts (2)
setPageBlocksBodySchema(14-16)setPageBlocksParametersSchema(7-9)
🪛 LanguageTool
.github/copilot-instructions.md
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...ately productive working on Thoth. High level - Monorepo managed with pnpm. The...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (19)
src/components/molecules/tree-node.tsx (1)
49-49: LGTM: Explicit null coalescing improves type safety.The change from
page.emojitopage.emoji ?? nullensures the emoji prop is explicitlynullrather thanundefined, which aligns well with the TreeItem component's handling and the stricter TypeScript configuration.Also applies to: 74-74
src/lib/auth/client.ts (1)
5-5: LGTM: Bracket notation aligns with strict TypeScript settings.The change to bracket notation for environment variable access is consistent with the strictest TypeScript configuration and maintains the same runtime behavior.
src/lib/errors/http-error.ts (1)
1-17: LGTM: Well-designed error class with sensible defaults.The HttpError class provides a clean abstraction for HTTP errors with appropriate defaults (500 status, hidden from users) and clear documentation. The implementation correctly extends Error and sets the name property.
package.json (1)
19-21: No security advisories found for BlockNote dependencies.Verification of
@blocknote/core,@blocknote/mantine, and@blocknote/reactat version 0.41.1 returned no known security vulnerabilities in the GitHub Security Advisory database.src/components/organisms/page-detail-editor.tsx (1)
30-35: The suggested type assertion is not a meaningful improvement over @ts-expect-error.Your imports and package versions are correctly aligned per BlockNote's documentation:
useCreateBlockNotefrom@blocknote/reactandBlockNoteViewfrom@blocknote/mantineare both at^0.41.1. The web search indicates version mismatches and wrong imports are typical culprits, but your setup avoids both.The type assertion workaround in the review (
as Parameters<typeof useCreateBlockNote>[0]) is semantically equivalent to the current@ts-expect-error—both suppress the error without addressing root cause. If the error stems from BlockNote's own typing limitations in 0.41.1, swapping syntax forms doesn't resolve it.Your current suppressions are well-documented with specific explanations (empty array runtime workaround, known typing issue). Without the actual TypeScript error message, it's unclear whether:
- BlockNote 0.41.1 has unresolved typing issues in its library itself
- Adding explicit schema types or generics would resolve it
- A version upgrade fixes the problem
To make a definitive decision, check BlockNote's GitHub issues for 0.41.1 or run TypeScript with the suppressions removed to see exact errors—that would clarify whether these are unavoidable library issues or addressable in your code.
src/lib/auth/session.ts (1)
3-3: LGTM - improved error handling with domain-specific error class.The migration from generic
ErrortoNotAuthorizedErrorprovides better error semantics and consistent HTTP status code handling (401) across the API surface.Also applies to: 12-12
src/app/api/v1/pages/[id]/route.ts (2)
4-4: LGTM - standardized error handling with NotFoundError.The migration to
NotFoundErrorprovides proper 404 HTTP status and visibility control, improving error consistency across API routes.Also applies to: 20-20
15-15: LGTM - safe removal of optional chaining.Direct property access
params.idis safe here since theapiRoutewrapper validates and resolves params before calling the handler, guaranteeing the property exists.src/lib/api/route-wrapper.ts (2)
108-110: LGTM - proper REST semantics for void responses.Returning 204 No Content when a handler returns
undefinedfollows REST best practices for successful operations with no response body.
116-121: LGTM - secure and structured error handling.The
HttpErrorhandling properly respects thevisibleErrorflag, preventing sensitive error details from leaking to clients while maintaining appropriate HTTP status codes.src/lib/errors/not-authorized-error.ts (1)
1-7: LGTM - clean error class implementation.The
NotAuthorizedErrorclass follows the establishedHttpErrorpattern with appropriate defaults (401 status, visible error message). This provides consistent authorization error handling across the application.src/types/api/entities.ts (1)
3-7: LGTM: Blocks correctly decoupled from pageSchema.This change appropriately separates blocks from the core page schema, aligning with the new blocks management endpoints and avoiding duplication of blocks data in page responses.
src/lib/errors/not-found-error.ts (1)
3-7: LGTM: NotFoundError properly implemented.The error class correctly extends HttpError with appropriate defaults for 404 responses. Setting
visibleErrortotrueby default is correct since "Not Found" messages are safe to expose to end users.src/app/pages/[id]/page.tsx (2)
21-33: LGTM: Proper state management and cleanup.The effect correctly initializes both page and blocks IDs, and properly cleans up on unmount by resetting both stores to null.
37-46: LGTM: updateBlocks implementation is correct.The callback properly guards against missing pageId and uses the correct endpoint path and types.
src/types/api/endpoints/set-page-blocks.ts (1)
14-16: Body schema relies on placeholder block validation.The
blocksarray usesblockSchemafromsrc/types/schemas/blocks.ts, which is currently defined asz.any()(a placeholder). This means the body validation doesn't enforce BlockNote's Block structure at runtime.This is related to the TODO in
get-page-blocks.ts. Once proper block validation schemas are implemented, this will automatically benefit from stricter validation.src/app/api/v1/pages/[id]/blocks/route.ts (2)
12-31: LGTM: GET handler properly secured and handles edge cases.The handler correctly:
- Authenticates via session
- Scopes query by userId to prevent unauthorized access
- Throws NotFoundError for missing pages
- Returns empty array when blocks are undefined
33-52: LGTM: POST handler updates blocks securely.The handler properly:
- Validates body and parameters using schemas
- Authenticates and scopes by userId
- Throws NotFoundError for missing pages
- Persists the updated blocks
Note: The body validation currently accepts
z.any()for blocks due to the placeholderblockSchema. Once proper BlockNote validation schemas are implemented (per the TODO inget-page-blocks.ts), this endpoint will automatically benefit from stricter validation.src/lib/store/query/get-page-blocks.ts (1)
5-11: LGTM: Store follows established patterns.The implementation correctly mirrors the structure of
$currentPagestore fromget-page-details.ts, maintaining consistency across the codebase. The fetcher store properly composes the URL path from the ID atom.
| await connection.execute('DELETE FROM verification;'); | ||
| await connection.execute('DELETE from session;'); | ||
| await connection.execute('DELETE from account;'); | ||
| await connection.execute('DELETE FROM user;'); |
There was a problem hiding this comment.
Add safeguards to prevent accidental data loss.
This script performs destructive database operations without any safety checks. Consider adding:
- A confirmation prompt before execution
- Environment validation to prevent running in production
- Error handling with try/catch
Apply this diff to add safeguards:
+import { confirm } from '@inquirer/prompts';
+
// Run this script to clear the auth database
+// Safety check
+if (process.env.NODE_ENV === 'production') {
+ console.error('Cannot run this script in production!');
+ process.exit(1);
+}
+
+const confirmed = await confirm({
+ message: 'This will delete all authentication data. Are you sure?',
+ default: false,
+});
+
+if (!confirmed) {
+ console.log('Operation cancelled.');
+ process.exit(0);
+}
+
const database = await getDatabase();
const connection = await database.getConnection<Connection>();
-await connection.execute('DELETE FROM verification;');
-await connection.execute('DELETE from session;');
-await connection.execute('DELETE from account;');
-await connection.execute('DELETE FROM user;');
+try {
+ await connection.execute('DELETE FROM verification;');
+ await connection.execute('DELETE FROM session;');
+ await connection.execute('DELETE FROM account;');
+ await connection.execute('DELETE FROM user;');
+ console.log('Auth database cleared.');
+} catch (error) {
+ console.error('Failed to clear auth database:', error);
+ process.exit(1);
+}
-
-console.log('Auth database cleared.');Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/clear-auth.ts around lines 9 to 12, the code runs destructive DELETE
statements without safeguards; add a confirmation prompt (e.g., read user input
and require an explicit "yes" or a --force flag), validate the environment
(abort unless NODE_ENV is 'development' or a specific non-production env, or
check an allowlist and require --force to override), wrap the DB operations in a
try/catch and log errors and exit non‑zero on failure, and perform the deletes
inside a transaction so you can rollback on error; ensure the script exits
immediately if confirmation or environment checks fail.
| export default function CreateSubpagePage() { | ||
| const parameters = useParams(); | ||
| const parentId = parameters.id as string; | ||
| const parentId = `${parameters['id']}`; |
There was a problem hiding this comment.
Unsafe type coercion may produce invalid parentId.
The template literal ${parameters['id']} will stringify undefined as "undefined" or join array values with commas. This differs from the previous type assertion and could break the CreatePageForm.
Apply this diff to safely handle the parameter:
- const parentId = `${parameters['id']}`;
+ const parentId = typeof parameters['id'] === 'string' ? parameters['id'] : Array.isArray(parameters['id']) ? parameters['id'][0] : '';Or more safely with validation:
+ const id = parameters['id'];
+
+ if (!id || Array.isArray(id)) {
+ throw new Error('Invalid page ID');
+ }
+
- const parentId = `${parameters['id']}`;
+ const parentId = id;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const parentId = `${parameters['id']}`; | |
| const parentId = typeof parameters['id'] === 'string' ? parameters['id'] : Array.isArray(parameters['id']) ? parameters['id'][0] : ''; |
| const parentId = `${parameters['id']}`; | |
| const id = parameters['id']; | |
| if (!id || Array.isArray(id)) { | |
| throw new Error('Invalid page ID'); | |
| } | |
| const parentId = id; |
🤖 Prompt for AI Agents
In src/app/pages/[id]/create/page.tsx around line 8, replace the unsafe template
literal coercion of parameters['id'] with runtime validation: read
parameters['id'] into a local variable, check if it's a string and use it as
parentId; if it's an array decide explicitly (e.g., take the first element or
treat as invalid) and if it's undefined or not a string/array handle it
deterministically (throw, return a redirect/notFound, or render an error) so
parentId is never the literal "undefined" or a joined array string.
| import z from 'zod'; | ||
| import { Block } from '@blocknote/core'; | ||
|
|
||
| export const GET_PAGE_BLOCKS_ENDPOINT = '/api/v1/pages/:pageId/blocks'; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify endpoint path consistency across the codebase.
This constant includes the /api/v1 prefix, but the fetcher store in src/lib/store/query/get-page-blocks.ts (lines 7-11) builds the path as /pages/${id}/blocks without the prefix. Similarly, SET_PAGE_BLOCKS in src/types/api/endpoints/set-page-blocks.ts (line 5) uses /pages/:pageId/blocks without /api/v1.
Ensure that endpoint constants and actual usage paths are consistent. Run this script to check how these endpoint constants are used:
🏁 Script executed:
#!/bin/bash
# Check usage of GET_PAGE_BLOCKS_ENDPOINT and SET_PAGE_BLOCKS constants
echo "=== Searching for GET_PAGE_BLOCKS_ENDPOINT usage ==="
rg -n "GET_PAGE_BLOCKS_ENDPOINT" --type ts
echo -e "\n=== Searching for SET_PAGE_BLOCKS usage ==="
rg -n "SET_PAGE_BLOCKS" --type ts
echo -e "\n=== Checking fetcher store path construction ==="
rg -n -A2 "createFetcherStore.*pages.*blocks" --type ts
echo -e "\n=== Checking client-side API calls to blocks endpoints ==="
rg -n "post.*pages.*blocks|get.*pages.*blocks" --type ts --type tsxLength of output: 453
🏁 Script executed:
#!/bin/bash
# Check where GET_PAGE_BLOCKS_ENDPOINT is imported/used
echo "=== GET_PAGE_BLOCKS_ENDPOINT imports/usage ==="
rg -n "GET_PAGE_BLOCKS_ENDPOINT" --type ts -B1 -A1
# Check where SET_PAGE_BLOCKS is imported/used
echo -e "\n=== SET_PAGE_BLOCKS imports/usage ==="
rg -n "SET_PAGE_BLOCKS" --type ts -B1 -A1
# Check the fetcher store file directly
echo -e "\n=== Content of get-page-blocks.ts fetcher store ==="
cat src/lib/store/query/get-page-blocks.ts 2>/dev/null || echo "File not found"
# Search for actual page blocks API calls
echo -e "\n=== Search for /pages/ and blocks path patterns ==="
rg -n "pages.*blocks|blocks.*endpoint" --type ts -iLength of output: 1697
🏁 Script executed:
#!/bin/bash
# More flexible search without strict file type
echo "=== Searching all text files for GET_PAGE_BLOCKS_ENDPOINT ==="
rg -n "GET_PAGE_BLOCKS_ENDPOINT" -B2 -A2
echo -e "\n=== Searching all text files for SET_PAGE_BLOCKS ==="
rg -n "SET_PAGE_BLOCKS" -B2 -A2Length of output: 878
🏁 Script executed:
#!/bin/bash
# Check createFetcherStore implementation
echo "=== createFetcherStore implementation ==="
cat src/lib/store/fetcher.ts 2>/dev/null || echo "File not found"
# Check how post() client function works
echo -e "\n=== Search for post() function definition ==="
rg -n "export.*function\s+post|export\s+const\s+post" --type ts -A10
# Check if there's any middleware adding API prefix
echo -e "\n=== Search for /api/v1 prefix being added ==="
rg -n "/api/v1" --type ts -B2 -A2Length of output: 1015
🏁 Script executed:
#!/bin/bash
# Find post() function implementation
echo "=== Search for post() implementation ==="
rg -n "export.*post\s*[=:(]|const\s+post\s*=" --type ts -A15
# Check lib/api directory structure
echo -e "\n=== Check lib/api directory ==="
find src/lib/api -type f -name "*.ts" 2>/dev/null | head -20
# Look for uses of these exported constants in the entire codebase
echo -e "\n=== Verify GET_PAGE_BLOCKS_ENDPOINT and SET_PAGE_BLOCKS have zero imports ==="
rg -n "import.*GET_PAGE_BLOCKS_ENDPOINT|import.*SET_PAGE_BLOCKS" --type tsLength of output: 203
🏁 Script executed:
#!/bin/bash
# Check page.tsx to see where post() comes from
echo "=== Check app/pages/[id]/page.tsx imports ==="
head -50 src/app/pages/[id]/page.tsx
# Check route-wrapper.ts
echo -e "\n=== Check src/lib/api/route-wrapper.ts ==="
cat src/lib/api/route-wrapper.ts
# Search for where post is exported from
echo -e "\n=== Search for 'export' of post in all files ==="
rg -n "^\s*export.*post" --type ts -A5Length of output: 5610
🏁 Script executed:
#!/bin/bash
# Find useCudApi implementation
echo "=== Check useCudApi hook ==="
cat src/lib/hooks/use-cud-api.ts 2>/dev/null || find src -name "*use-cud-api*" -type f
# Also search for any file that might have post function
echo -e "\n=== Search for where useCudApi is defined ==="
rg -n "export.*useCudApi|function useCudApi|const useCudApi" --type ts -A10Length of output: 2077
Remove or properly implement unused endpoint constants; GET_PAGE_BLOCKS_ENDPOINT is malformed.
The constants are defined but never imported or used. More critically, GET_PAGE_BLOCKS_ENDPOINT incorrectly includes /api/v1 (line 4 in src/types/api/endpoints/get-page-blocks.ts), which is redundant and would break if used—the apiClient already has baseURL: '/api/v1'. The actual code correctly uses paths without the prefix (/pages/${pageId}/blocks), relying on the client's baseURL for the full URL.
Fix: Either remove these unused constants or redefine GET_PAGE_BLOCKS_ENDPOINT as /pages/:pageId/blocks to match SET_PAGE_BLOCKS and actual usage patterns, then update code to use them instead of hardcoding paths.
🤖 Prompt for AI Agents
In src/types/api/endpoints/get-page-blocks.ts around line 4, the exported
GET_PAGE_BLOCKS_ENDPOINT is unused and malformed because it includes the
redundant '/api/v1' prefix; either delete the unused constant or change it to
'/pages/:pageId/blocks' to match the client baseURL and SET_PAGE_BLOCKS; then
update any callers to import and use this constant instead of hardcoding
'/pages/${pageId}/blocks' so paths remain consistent with the apiClient baseURL.
| // TODO generate/get block validation schemas for blocks | ||
| export const getPageBlocksResponseSchema = z.object({ | ||
| blocks: z.array(z.any()), | ||
| }); | ||
|
|
||
| export type GetPageBlocksResponse = { | ||
| blocks: Block[]; | ||
| }; |
There was a problem hiding this comment.
Address type safety gap between schema and TypeScript type.
The response schema uses z.any() for blocks validation (line 14), but the TypeScript type expects Block[] (line 18). This creates a mismatch where runtime validation accepts any type but compile-time expects BlockNote's Block type.
The TODO comment acknowledges this needs proper block validation. Would you like me to help generate a proper Zod schema that validates BlockNote's Block structure, or is this intentionally deferred?
| @@ -1,4 +1,5 @@ | |||
| { | |||
| "extends": ["@tsconfig/strictest"], | |||
There was a problem hiding this comment.
🧩 Analysis chain
Verify that extending strictest config doesn't break existing code.
Adding @tsconfig/strictest will enable the most restrictive TypeScript checks. While this improves code quality, it may surface new type errors in existing code that weren't caught before.
Run the following script to verify that the codebase compiles without errors:
🏁 Script executed:
#!/bin/bash
# Description: Verify TypeScript compilation succeeds with strictest config
# Test: Run TypeScript compiler in noEmit mode
pnpm run lint:tscLength of output: 16731
Critical TypeScript compilation failures with strictest config enabled.
Verification confirms the concern: The codebase does NOT compile with @tsconfig/strictest enabled. The TypeScript compiler reports 112+ errors including:
- Missing type declarations for core dependencies (@mantine/core, react, better-auth, nanostores, zod, etc.)
- Missing @types packages (@types/react, @types/node)
- Implicit 'any' types in function parameters across multiple components
- Unhandled possibly-undefined values in strict null checking mode
- Type mismatches and missing required properties in components
Before adopting the strictest config, the codebase requires:
- Installing missing @types packages
- Fixing implicit parameter types in callbacks and functions
- Addressing property type mismatches (e.g., TreeNodeProperties shape)
- Handling strict null-safety issues throughout the codebase
🤖 Prompt for AI Agents
In tsconfig.json around line 2, enabling "@tsconfig/strictest" causes 112+
TypeScript errors; fix by (1) installing missing type packages (add
@types/react, @types/node and any other @types for third-party libs or switch to
explicit package-provided types), (2) add missing type declarations for
third-party libs (create declare module stubs under src/types/*.d.ts for
libraries without types), (3) annotate implicit any function/callback parameters
and return types across components, (4) resolve strict null checks by narrowing
or using non-null assertions only where safe and adding optional
chaining/guards, and (5) correct component prop typings and interfaces (e.g.,
adjust TreeNodeProperties shape and other mismatched props) so the project
compiles under the strictest config.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/ci.yaml(2 hunks)next.config.ts(1 hunks)src/lib/auth/config.ts(1 hunks)src/lib/database/index.ts(2 hunks)src/lib/environment.ts(2 hunks)src/lib/logger.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Prefer TypeScript type aliases (
type) overinterfacewhen defining shapes
Files:
src/lib/database/index.tsnext.config.tssrc/lib/logger.tssrc/lib/auth/config.tssrc/lib/environment.ts
🧬 Code graph analysis (3)
src/lib/database/index.ts (1)
src/lib/environment.ts (1)
getEnvironment(21-26)
src/lib/logger.ts (1)
src/lib/environment.ts (1)
getEnvironment(21-26)
src/lib/auth/config.ts (2)
src/lib/database/index.ts (1)
getDatabase(8-18)src/lib/environment.ts (1)
getEnvironment(21-26)
🔇 Additional comments (7)
.github/workflows/ci.yaml (2)
12-13: Good practice: Disabling Next.js telemetry in CI.Adding
NEXT_TELEMETRY_DISABLED: 1prevents telemetry data collection during CI builds, which is a standard best practice.
65-65: The review comment is incorrect—this is a single-package project, not a monorepo.The pnpm-workspace.yaml contains only
minimumReleaseAge: 1440with no package definitions, and only one package.json exists at the root. There are no packages/, apps/, or other subdirectory packages. The build command change frompnpm -r buildtopnpm buildis appropriate and correct for this structure.Likely an incorrect or invalid review comment.
next.config.ts (1)
4-4: No action required—deployment infrastructure does not currently exist in this repository.The review comment assumes deployment infrastructure that isn't present. The
docker-compose.ymldefines only a local MySQL service for development; there is no Dockerfile, no Next.js deployment configuration, and no CI/CD pipeline in this repository.The
output: 'standalone'configuration is a valid optimization for future deployment setups, not a breaking change for the current codebase. When deployment infrastructure is added later, it should follow Next.js documentation for handling the standalone build output—but that is future work, not a concern for this change.Likely an incorrect or invalid review comment.
src/lib/auth/config.ts (1)
6-6: LGTM! Consistent migration to environment getter.All references to the environment have been correctly updated to use
getEnvironment(). The multiple calls at module initialization are acceptable since the function caches its result after the first invocation.Also applies to: 11-11, 17-20, 26-27
src/lib/logger.ts (1)
2-2: LGTM! Consistent with the environment getter pattern.The logger configuration correctly uses
getEnvironment()for accessing the log level. Module-level initialization is acceptable since the environment will be lazily loaded on first access.Also applies to: 5-5
src/lib/database/index.ts (1)
3-3: LGTM! Proper lazy initialization pattern.The database initialization correctly uses
getEnvironment()to access the database configuration. SincegetDatabase()is called lazily, the environment will be properly initialized when needed.Also applies to: 13-13
src/lib/environment.ts (1)
19-26: LGTM! Clean caching pattern with lazy initialization.The implementation correctly uses a cached environment instance with lazy initialization on first access. The null check is appropriate, and the pattern will work well for singleton environment access throughout the application.
| - uses: actions/cache@v4 | ||
| name: Setup nextjs cache | ||
| with: | ||
| # See here for caching with `yarn`, `bun` or other package managers https://github.com/actions/cache/blob/main/examples.md or you can leverage caching with actions/setup-node https://github.com/actions/setup-node | ||
| path: | | ||
| ~/.npm | ||
| ${{ github.workspace }}/.next/cache | ||
| # Generate a new cache whenever packages or source files change. | ||
| key: ${{ runner.os }}-nextjs-${{ hashFiles('**/package-lock.json') }}-${{ hashFiles('**/*.js', '**/*.jsx', '**/*.ts', '**/*.tsx') }} | ||
| # If source files changed but packages didn't, rebuild from a prior cache. | ||
| restore-keys: | | ||
| ${{ runner.os }}-nextjs-${{ hashFiles('**/package-lock.json') }}- |
There was a problem hiding this comment.
Incorrect lock file in Next.js cache key: using package-lock.json instead of pnpm-lock.yaml.
The cache key hash at line 59 references **/package-lock.json, but this project uses pnpm with pnpm-lock.yaml (as shown by the existing pnpm store cache at line 38). The cache will not invalidate correctly when dependencies change, leading to stale builds.
Apply this diff to fix the cache key:
- key: ${{ runner.os }}-nextjs-${{ hashFiles('**/package-lock.json') }}-${{ hashFiles('**/*.js', '**/*.jsx', '**/*.ts', '**/*.tsx') }}
+ key: ${{ runner.os }}-nextjs-${{ hashFiles('**/pnpm-lock.yaml') }}-${{ hashFiles('**/*.js', '**/*.jsx', '**/*.ts', '**/*.tsx') }}
# If source files changed but packages didn't, rebuild from a prior cache.
restore-keys: |
- ${{ runner.os }}-nextjs-${{ hashFiles('**/package-lock.json') }}-
+ ${{ runner.os }}-nextjs-${{ hashFiles('**/pnpm-lock.yaml') }}-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/cache@v4 | |
| name: Setup nextjs cache | |
| with: | |
| # See here for caching with `yarn`, `bun` or other package managers https://github.com/actions/cache/blob/main/examples.md or you can leverage caching with actions/setup-node https://github.com/actions/setup-node | |
| path: | | |
| ~/.npm | |
| ${{ github.workspace }}/.next/cache | |
| # Generate a new cache whenever packages or source files change. | |
| key: ${{ runner.os }}-nextjs-${{ hashFiles('**/package-lock.json') }}-${{ hashFiles('**/*.js', '**/*.jsx', '**/*.ts', '**/*.tsx') }} | |
| # If source files changed but packages didn't, rebuild from a prior cache. | |
| restore-keys: | | |
| ${{ runner.os }}-nextjs-${{ hashFiles('**/package-lock.json') }}- | |
| - uses: actions/cache@v4 | |
| name: Setup nextjs cache | |
| with: | |
| # See here for caching with `yarn`, `bun` or other package managers https://github.com/actions/cache/blob/main/examples.md or you can leverage caching with actions/setup-node https://github.com/actions/setup-node | |
| path: | | |
| ~/.npm | |
| ${{ github.workspace }}/.next/cache | |
| # Generate a new cache whenever packages or source files change. | |
| key: ${{ runner.os }}-nextjs-${{ hashFiles('**/pnpm-lock.yaml') }}-${{ hashFiles('**/*.js', '**/*.jsx', '**/*.ts', '**/*.tsx') }} | |
| # If source files changed but packages didn't, rebuild from a prior cache. | |
| restore-keys: | | |
| ${{ runner.os }}-nextjs-${{ hashFiles('**/pnpm-lock.yaml') }}- |
🤖 Prompt for AI Agents
.github/workflows/ci.yaml around lines 51 to 62: the Next.js cache key is
hashing package-lock.json which is incorrect for this pnpm-based repo; update
the cache key to hash pnpm-lock.yaml (and any relevant lockfiles like
pnpm-workspace.yaml if present) so the cache invalidates when pnpm dependencies
change, and adjust the restore-keys line to use the same pnpm-lock.yaml hash.
| }); | ||
| } as const; | ||
|
|
||
| type Environment = ReturnType<typeof cleanEnv<typeof environmentSchema>>; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the type definition syntax.
The syntax ReturnType<typeof cleanEnv<typeof environmentSchema>> may not be valid TypeScript. You typically cannot partially apply generic type parameters to a function type like this. Consider these alternatives:
// Option A: Derive from the function itself
type Environment = ReturnType<typeof getEnvironment>;
// Option B: Use a temporary const
const _env = cleanEnv(process.env, environmentSchema);
type Environment = typeof _env;Verify that this compiles without errors and provides correct type inference:
🏁 Script executed:
#!/bin/bash
# Check if the TypeScript compilation succeeds and examine the inferred type
# Find and compile the environment.ts file
npx tsc --noEmit src/lib/environment.ts 2>&1 | head -20
# Also check if there are any type errors in files that use Environment type
npx tsc --noEmit 2>&1 | grep -i "environment" | head -20Length of output: 543
I need to gather more information. The errors are about missing dependencies, not the syntax itself. Let me examine the actual file and search for details about the envalid package.
🏁 Script executed:
#!/bin/bash
# View the environment.ts file to see the full context
cat src/lib/environment.ts
# Also check package.json to understand the dependencies
echo "=== package.json ==="
head -30 package.jsonLength of output: 1806
🌐 Web query:
envalid cleanEnv generic type parameters TypeScript
💡 Result:
Short answer: use envalid's types and the validators' generic parameters. For most cases either (A) give the validator a type parameter (especially json) or (B) assert/annotate the returned shape with envalid's CleanedEnv generic.
Examples:
-
Use validator generic (recommended for json/structured values)
- const env = cleanEnv(process.env, { PAYLOAD: json() }) // env.PAYLOAD has type MyType. [1][2]
-
Annotate the cleanEnv result with CleanedEnv if you want an explicit overall shape
- import type { CleanedEnv } from 'envalid'
- type Specs = { PORT: number; MODE?: string }
- const env = cleanEnv(process.env, { PORT: num(), MODE: str({ default: undefined }) }) as CleanedEnv
- Now env.PORT is number and env.MODE is string | undefined. [1][2]
Notes: envalid exposes the CleanedEnv utility type and many validators (makeValidator/makeExactValidator/json) accept type parameters to control output typing — give json an explicit type parameter to avoid any. [1][2]
Sources:
[1] envalid GitHub README (usage, TypeScript notes).
[2] envalid API/type docs (CleanedEnv and json validator notes).
---
🌐 Web query:
```web
TypeScript typeof generic type parameters syntax validation
💡 Result:
Short answer: You cannot use the typeof type operator on a generic type parameter (e.g. typeof T) — typeof in a type position only accepts value identifiers (variables or property accesses). Use type-constraints, mapped/conditional types, or pass a value/constructor to derive a type with typeof instead. [1][2]
Why:
- typeof in a type context refers to the compile-time type of a runtime value (an identifier or its property). It is illegal on arbitrary type identifiers. The docs explicitly limit typeof to identifiers/properties. [2][3]
Examples and alternatives:
- Wrong: typeof on a type parameter (error)
- type Bad = typeof T; // invalid — T is a type parameter, not a value. [2]
- If you want the type of a value passed alongside a generic, use typeof on that value:
- const v = { x: 1 };
- type V = typeof v; // OK — V is { x: number }. [2]
- If you want to constrain/inspect a generic, use extends / conditional / indexed access types:
- function id(arg: T): T { return arg; }
- type HasX = T extends { x: infer U } ? U : never; // extracts property type
- function getProp<T, K extends keyof T>(o: T, k: K) { return o[k]; } // use keyof constraint. [1]
- If you need a type from a constructor/value passed as a generic parameter, accept the value (or its constructor) and use typeof:
- function make<T extends new (...a:any)=>any>(ctor: T) { type Instance = InstanceType; } // or pass value and use typeof. [1]
References:
- TypeScript handbook: The typeof type operator (limitations). [2]
- TypeScript handbook: Generics / Using type parameters in constraints. [1]
Import and use envalid's CleanedEnv type instead.
The syntax ReturnType<typeof cleanEnv<typeof environmentSchema>> is invalid—TypeScript does not allow generic syntax within typeof expressions. The typeof operator only accepts value identifiers or property accesses, not type parameters.
Replace with:
import type { CleanedEnv } from 'envalid';
type Environment = CleanedEnv<typeof environmentSchema>;This is the idiomatic approach per envalid's API and ensures correct type inference for your environment variables.
🤖 Prompt for AI Agents
In src/lib/environment.ts around line 17, the type declaration uses an invalid
construct ReturnType<typeof cleanEnv<typeof environmentSchema>>; replace it by
importing envalid's CleanedEnv type and declare Environment as CleanedEnv<typeof
environmentSchema>. Update the imports to include "import type { CleanedEnv }
from 'envalid';" and change the type alias to use CleanedEnv with the
environmentSchema so TypeScript infers the cleaned env shape correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/api/route-wrapper.ts (2)
33-37: Move connection() inside the try block.Errors thrown during connection currently bypass your catch and won’t get logged/normalized.
- return async (request: NextRequest, { params }: { params: Promise<ExpectedParameters> }) => { - await connection(); - try { + return async (request: NextRequest, { params }: { params: Promise<ExpectedParameters> }) => { + try { + await connection();
32-40: Fix Next.js handler signature and params typing.In App Router, params is synchronous. Typing it as Promise is misleading; drop the Promise and the await. Also make it mutable if you’ll assign the parsed value.
- return async (request: NextRequest, { params }: { params: Promise<ExpectedParameters> }) => { + return async (request: NextRequest, { params }: { params: ExpectedParameters }) => { try { - // Get session + // Get session const session = await getSession(); - - // Resolve params - const resolvedParameters = await params; + // Resolve params + let resolvedParameters = params as ExpectedParameters;
♻️ Duplicate comments (1)
src/lib/environment.ts (1)
17-17: Invalid type construction with ReturnType<typeof cleanEnv<…>>.Use envalid’s CleanedEnv utility instead. This has been flagged before; repeating here for the new commit.
-import { cleanEnv, str, url } from 'envalid'; +import { cleanEnv, str, url } from 'envalid'; +import type { CleanedEnv } from 'envalid'; @@ -type Environment = ReturnType<typeof cleanEnv<typeof environmentSchema>>; +type Environment = CleanedEnv<typeof environmentSchema>;
🧹 Nitpick comments (9)
src/app/layout.tsx (1)
8-10: Make sidebar optional to support layouts without @Sidebar.If some routes don’t provide the @Sidebar slot, loosen the prop to avoid typing friction.
-type Properties = PropsWithChildren & { - sidebar: ReactNode; -}; +type Properties = PropsWithChildren & { + sidebar?: ReactNode; +};.github/copilot-instructions.md (2)
5-5: Hyphenation nit.Use “High‑level” as a compound adjective.
-High level +High‑level
20-21: Clarify route file paths to ‘route.ts’.Next App Router uses route.ts files; make examples precise.
-See `src/app/api/v1/pages/*.ts` for examples (GET, POST, tree endpoints). +See `src/app/api/v1/pages/**/route.ts` for examples (GET, POST, tree endpoints).And similarly adjust other references to point to
**/route.ts.Also applies to: 45-48, 54-57
src/lib/api/route-wrapper.ts (4)
66-83: Use Zod‑parsed data for query (coercions/transforms), not the raw object.You validate but don’t replace query with validationResult.data; also query is const so it can’t be reassigned.
- const url = new URL(request.url); - const query = Object.fromEntries(url.searchParams.entries()) as ExpectedQuery; + const url = new URL(request.url); + const rawQuery = Object.fromEntries(url.searchParams.entries()); + let query = rawQuery as unknown as ExpectedQuery; @@ - if (options.expectedQuerySchema) { - const validationResult = options.expectedQuerySchema.safeParse(query); + if (options.expectedQuerySchema) { + const validationResult = options.expectedQuerySchema.safeParse(rawQuery); if (!validationResult.success) { return NextResponse.json( @@ } + query = validationResult.data as ExpectedQuery; }
84-97: Same for route params: assign parsed data back.You validate params but don’t use the parsed result.
- if (options.expectedParamsSchema) { - const validationResult = options.expectedParamsSchema.safeParse(resolvedParameters); + if (options.expectedParamsSchema) { + const validationResult = options.expectedParamsSchema.safeParse(resolvedParameters); if (!validationResult.success) { return NextResponse.json( @@ } + resolvedParameters = validationResult.data as ExpectedParameters; }
109-114: Pass through Response/NextResponse results; only wrap plain data.Allow handlers to return a Response for custom status/headers.
- if (result === undefined) { + if (result === undefined) { return new Response(null, { status: 204 }); } - // Return the result - return NextResponse.json({ data: result }); + // Pass through Response/NextResponse, otherwise wrap + if (result instanceof Response) { + return result; + } + return NextResponse.json({ data: result as unknown });
115-123: Logging: include request context/correlation id (optional).Consider adding request id and route info to logs to aid tracing.
src/lib/logger.ts (1)
19-44: Lazy initialization looks solid. Two small logging tweaks.
- Console transport overrides the base format; stack traces may be hidden. Prefer a printf format that prints error.stack when present.
- Ensure LOG_LEVEL choices align with Winston’s defaults (verbose/silly exist; trace does not). See env schema comment.
- transports: [ - new winston.transports.Console({ - format: winston.format.combine(winston.format.colorize(), winston.format.simple()), - }), - ], + transports: [ + new winston.transports.Console({ + format: winston.format.combine( + winston.format.colorize(), + winston.format.printf(info => + info.stack ? `${info.level}: ${info.stack}` : `${info.level}: ${info.message}` + ) + ), + }), + ],src/lib/environment.ts (1)
6-9: Align LOG_LEVEL choices with Winston defaults.Winston doesn’t have “trace” by default; add “verbose” and “silly”, drop “trace”, or map it explicitly.
LOG_LEVEL: str({ - choices: ['error', 'warn', 'info', 'http', 'debug', 'trace'], + choices: ['error', 'warn', 'info', 'http', 'verbose', 'debug', 'silly'], default: 'info', }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/copilot-instructions.md(1 hunks)src/app/api/auth/[...auth]/route.ts(1 hunks)src/app/layout.tsx(1 hunks)src/lib/api/route-wrapper.ts(3 hunks)src/lib/auth/config.ts(1 hunks)src/lib/auth/session.ts(1 hunks)src/lib/database/index.ts(2 hunks)src/lib/environment.ts(2 hunks)src/lib/logger.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/auth/config.ts
- src/lib/auth/session.ts
- src/lib/database/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Prefer TypeScript type aliases (
type) overinterfacewhen defining shapes
Files:
src/lib/api/route-wrapper.tssrc/lib/logger.tssrc/lib/environment.tssrc/app/api/auth/[...auth]/route.tssrc/app/layout.tsx
src/app/api/**/route.@(ts|tsx)
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Implement API endpoints as Next.js route handlers under
src/app/api/**/route.ts(x)
Files:
src/app/api/auth/[...auth]/route.ts
src/app/**/@(page|layout|loading|error).tsx
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Define routes using Next.js App Router conventions under
src/appwithpage.tsx,layout.tsx,loading.tsx, anderror.tsx
Files:
src/app/layout.tsx
src/{components,app}/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/basics.mdc)
Use Mantine UI 8 components for building React UI
Files:
src/app/layout.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-23T17:57:41.115Z
Learnt from: CR
PR: danships/thoth#0
File: .cursor/rules/basics.mdc:0-0
Timestamp: 2025-10-23T17:57:41.115Z
Learning: Applies to src/app/api/**/route.@(ts|tsx) : Implement API endpoints as Next.js route handlers under `src/app/api/**/route.ts(x)`
Applied to files:
src/app/api/auth/[...auth]/route.ts
🧬 Code graph analysis (4)
src/lib/api/route-wrapper.ts (2)
src/lib/logger.ts (1)
getLogger(42-44)src/lib/errors/http-error.ts (1)
HttpError(1-17)
src/lib/logger.ts (1)
src/lib/environment.ts (1)
getEnvironment(21-26)
src/app/api/auth/[...auth]/route.ts (1)
src/lib/auth/config.ts (1)
getAuth(68-71)
src/app/layout.tsx (1)
src/lib/auth/config.ts (1)
getAuth(68-71)
🪛 LanguageTool
.github/copilot-instructions.md
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...ately productive working on Thoth. High level - Monorepo managed with pnpm. The...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (5)
src/app/api/auth/[...auth]/route.ts (3)
1-2: LGTM!The imports correctly support the lazy initialization pattern.
4-15: LGTM!Excellent documentation explaining the rationale for lazy initialization.
17-27: Caching is properly implemented via singleton pattern—no action needed.The
authInstanceis a module-level variable initialized asnull, andinitializeAuth()checks this before initialization, ensuring it only initializes once. Theconnection()function is fromnext/serverand handles connection pooling internally. WhilegetAuth()is called on every request, the initialization overhead is minimal after the first request. The lazy initialization approach is correct and aligns with Next.js best practices to avoid initializing at build time.Optional: Consider extracting the handler delegation pattern to reduce duplication.
Both GET and POST handlers follow identical logic. You could refactor to a helper function:
async function createAuthHandler(method: 'GET' | 'POST') { return async (request: Request) => { const auth = await getAuth(); const handlers = toNextJsHandler(auth); return handlers[method](request); }; } export const GET = createAuthHandler('GET'); export const POST = createAuthHandler('POST');However, the current explicit approach is also clear and maintainable.
src/app/layout.tsx (2)
12-16: Good async usage of headers() with Next.js 15.Calling await headers() and awaiting getAuth() in an async RootLayout is correct for Next 15’s async Request APIs. No changes needed.
If you’re using a parallel route for @Sidebar, ignore; otherwise consider making sidebar optional (see next comment).
6-6: Global CSS import is in the right place.globals.css belongs in the root layout in App Router. LGTM.
| @@ -1,9 +1,10 @@ | |||
| import type { NextRequest } from 'next/server'; | |||
| import { NextResponse } from 'next/server'; | |||
| import { connection, NextResponse } from 'next/server'; | |||
There was a problem hiding this comment.
Wrong import: ‘connection’ is not exported by next/server.
This will fail at compile time. Import your DB connection helper from your database module instead. Also move the call inside the try (see next comment).
-import { connection, NextResponse } from 'next/server';
+import { NextResponse } from 'next/server';
+import { connection } from '@/lib/database';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { connection, NextResponse } from 'next/server'; | |
| import { NextResponse } from 'next/server'; | |
| import { connection } from '@/lib/database'; |
🤖 Prompt for AI Agents
In src/lib/api/route-wrapper.ts around line 2, remove the incorrect import of
"connection" from 'next/server' and instead import your DB connection helper
from your database module (e.g., src/lib/db or the actual module that exports
the connection function), then move the call that acquires the DB connection
inside the try block where the request is handled; specifically replace the
wrong import, import the correct helper (matching its real exported name), and
ensure the connection call is executed within the try so any errors are caught
and handled before using the connection.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores