Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds an end-to-end presentation feature: Prisma schema with ChangesPresentation Feature End-to-End
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.tanstack/tmp/2ab4fcb8-66ad50854acd56d5165e5eb0a371b193 (1)
1-13: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove generated temp file from version control.
This file in
.tanstack/tmp/is an auto-generated artifact by TanStack Router's file-based routing. It should not be committed to git. The.gitignorereduction in this PR appears to have removed the pattern that previously ignored such generated files.Add
.tanstack/tmp/or similar to.gitignoreand remove this file from the PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.tanstack/tmp/2ab4fcb8-66ad50854acd56d5165e5eb0a371b193 around lines 1 - 13, This is an auto-generated TanStack Router artifact under .tanstack/tmp and should not be versioned. Restore the ignore rule in .gitignore for .tanstack/tmp/ (or an equivalent pattern) so generated routing files stay out of git, and remove this specific temporary file from the commit/PR. Use the existing route generation flow around createFileRoute and Route as the reference when locating the artifact.
🧹 Nitpick comments (1)
src/routes/presentation.$presentationId.tsx (1)
63-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
AUTH_LOGIN_PATHhere instead of hardcoding/login.The middleware already redirects via the shared constant from
src/lib/auth-path.ts, so keeping this route on a string literal makes the auth flow drift the next time that path changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/presentation`.$presentationId.tsx around lines 63 - 66, The redirect in the presentation route is hardcoding the login path instead of using the shared auth constant. Update the `session` guard in the presentation route to reference `AUTH_LOGIN_PATH` from `src/lib/auth-path.ts` rather than the `/login` string literal, matching the existing auth middleware behavior and keeping the redirect path centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prisma.config.ts`:
- Line 12: The Prisma config is relying on a type assertion for DATABASE_URL,
which can still allow undefined at runtime and defer the failure to Prisma.
Update the database URL assignment in prisma.config.ts to use
env("DATABASE_URL") or add an explicit guard before constructing the config so
the process fails immediately with a clear error when the environment variable
is missing.
In `@prisma/migrations/20260627170407_presentation_slides/migration.sql`:
- Around line 22-41: The slide schema currently allows duplicate sequence
positions within the same presentation, so update the slide definition to
enforce uniqueness on the presentationId and order pair. Add the composite
unique constraint in the slide model in prisma/schema.prisma and mirror it in
the migration SQL for the slide table, using the existing slide, presentationId,
and order symbols to locate the change.
In `@prisma/schema.prisma`:
- Around line 57-67: The slide ordering in the schema is not constrained per
presentation, so duplicate values can make the sequence nondeterministic. Update
the Slide model in prisma/schema.prisma to enforce a unique ordering within each
presentation by adding a composite uniqueness constraint on presentationId and
order, and keep the existing getPresentationWithSlides ordering logic unchanged
since it will then rely on a stable contract.
In `@src/features/presentations/actions/presentation-mutations.ts`:
- Around line 26-33: The presentation mutation flow is leaving items stuck in
GENERATING because the generation job is never dispatched after the status
update. In the relevant mutation handlers in presentation-mutations.ts, restore
the queued generation step by re-enabling the inngest.send call (or otherwise
dispatching the generation work) for both the create and regenerate paths so the
status transition is followed by actual processing.
In `@src/features/presentations/api/presentation-queries.ts`:
- Around line 18-27: The getPresentation server function currently throws a
generic Error for missing records, which bypasses the router’s not-found
handling. Update the getPresentation handler to call notFound() for every
missing-record path, including stale/deleted IDs after the
prisma.presentation.findFirst lookup, and keep the user lookup via
requirePresentationUserId intact. Also make sure the presentation detail fetch
is performed through useServerFn so the client-side route can receive and react
to the not-found state correctly.
In `@src/features/presentations/hooks/usePresentation-detail.ts`:
- Around line 47-57: The sync in usePresentationDetail is overwriting local
edits whenever query.data changes, which wipes dirty form state on refetch.
Update the useEffect that calls setForm so it only seeds the form once per
presentation or only when the form is not dirty/has not been initialized, and
use a guard keyed off the current presentation/query identity to avoid resetting
user edits during polling or invalidation refetches.
In `@src/features/presentations/lib/server-helpers.ts`:
- Around line 15-19: In requirePresentationUserId, the missing-session path
currently throws a generic Error and gets surfaced as a 500; change this auth
failure to use the auth middleware flow or explicitly set a 401 response status
before failing so presentation queries/mutations return an auth response instead
of an internal server error. Keep the fix localized to requirePresentationUserId
and preserve the existing successful return of session.user.id.
In `@src/features/presentations/types/schema.ts`:
- Line 27: The schema validation for prompt/title in the presentation types
still allows whitespace-only values because z.string().min(1) is checked before
trimming; update the prompt and title validators in schema.ts so they trim input
before enforcing min/max length. Apply the same fix to both the create/update
variants referenced by the prompt and title fields, using the existing schema
definitions in that file to ensure blank-looking strings are rejected
consistently.
In `@src/integrations/inngest/functions.ts`:
- Around line 5-7: The handler in the inngest function assumes event.data.email
is always present, which can cause a crash or return an invalid greeting for
malformed events. Update the async handler for the test/hello.world event to
validate the event payload before using event.data.email, and handle
missing/invalid data with an explicit fallback or failure path instead of
dereferencing it directly. Keep the fix localized to the function body that
calls step.sleep and builds the Hello message.
In `@src/routes/index.tsx`:
- Around line 81-99: The create flow in createMut/onSuccess assumes generation
has already started, but createPresentation currently only inserts the
presentation row and returns it. Update the server action in
presentation-mutations to actually kick off the generation job by restoring the
inngest.send(...) call (or equivalent job dispatch) inside createPresentation so
the returned GENERATING record is backed by a running job. Keep the route logic
in src/routes/index.tsx as the success handler only after the mutation truly
starts generation.
In `@src/routes/presentation`.$presentationId.tsx:
- Around line 191-213: The presentation route still exposes slide-related
controls without any working slide-viewing behavior. In
presentation.$presentationId.tsx, either restore the missing slide flow by
wiring back SlidePreview, the sidebar slide list, the slideshow modal, and the
export action handlers, or remove/hide the Button controls and related UI until
they are functional. Make sure the JSX branches around slides.length,
setShowSlideshow, and the export button are consistent so users never see empty
preview areas or non-working controls.
- Around line 147-149: The detail view can lose its preview when the slide list
shrinks because presentationThumbnailUrl, activeSlideIndex, and
slides.at(activeSlideIndex) still assume the old index is valid. In
src/routes/presentation.$presentationId.tsx, update the logic around
activeSlideIndex so it is clamped or reset whenever slides.length changes, and
ensure the selected slide is always derived from a valid index before rendering
the preview.
---
Outside diff comments:
In @.tanstack/tmp/2ab4fcb8-66ad50854acd56d5165e5eb0a371b193:
- Around line 1-13: This is an auto-generated TanStack Router artifact under
.tanstack/tmp and should not be versioned. Restore the ignore rule in .gitignore
for .tanstack/tmp/ (or an equivalent pattern) so generated routing files stay
out of git, and remove this specific temporary file from the commit/PR. Use the
existing route generation flow around createFileRoute and Route as the reference
when locating the artifact.
---
Nitpick comments:
In `@src/routes/presentation`.$presentationId.tsx:
- Around line 63-66: The redirect in the presentation route is hardcoding the
login path instead of using the shared auth constant. Update the `session` guard
in the presentation route to reference `AUTH_LOGIN_PATH` from
`src/lib/auth-path.ts` rather than the `/login` string literal, matching the
existing auth middleware behavior and keeping the redirect path centralized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc5db608-56d9-4893-9c99-248b350d3310
⛔ Files ignored due to path filters (16)
generated/prisma/browser.tsis excluded by!**/generated/**generated/prisma/client.tsis excluded by!**/generated/**generated/prisma/commonInputTypes.tsis excluded by!**/generated/**generated/prisma/enums.tsis excluded by!**/generated/**generated/prisma/internal/class.tsis excluded by!**/generated/**generated/prisma/internal/prismaNamespace.tsis excluded by!**/generated/**generated/prisma/internal/prismaNamespaceBrowser.tsis excluded by!**/generated/**generated/prisma/models.tsis excluded by!**/generated/**generated/prisma/models/Account.tsis excluded by!**/generated/**generated/prisma/models/Presentation.tsis excluded by!**/generated/**generated/prisma/models/Session.tsis excluded by!**/generated/**generated/prisma/models/Slide.tsis excluded by!**/generated/**generated/prisma/models/User.tsis excluded by!**/generated/**generated/prisma/models/Verification.tsis excluded by!**/generated/**generated/prisma/query_engine-windows.dll.nodeis excluded by!**/generated/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (23)
.gitignore.tanstack/tmp/2ab4fcb8-66ad50854acd56d5165e5eb0a371b193package.jsonprisma.config.tsprisma/migrations/20260627170407_presentation_slides/migration.sqlprisma/schema.prismasrc/components/ui/alert-dialog.tsxsrc/features/presentations/actions/presentation-mutations.tssrc/features/presentations/api/presentation-queries.tssrc/features/presentations/components/generation-status.tsxsrc/features/presentations/constants/presentation-options.tssrc/features/presentations/hooks/query-keys.tssrc/features/presentations/hooks/usePresentation-detail.tssrc/features/presentations/lib/server-helpers.tssrc/features/presentations/types/schema.tssrc/features/presentations/utils/thumbnail-url.tssrc/integrations/inngest/client.tssrc/integrations/inngest/functions.tssrc/middleware/auth.tssrc/routeTree.gen.tssrc/routes/api/inngest.tssrc/routes/index.tsxsrc/routes/presentation.$presentationId.tsx
💤 Files with no reviewable changes (1)
- src/routeTree.gen.ts
| }, | ||
| datasource: { | ||
| url: process.env["DATABASE_URL"], | ||
| url: process.env["DATABASE_URL"] as string, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect the relevant area with line numbers.
git ls-files | rg '^prisma\.config\.ts$|prisma\.config\.ts$'
wc -l prisma.config.ts
cat -n prisma.config.ts | sed -n '1,120p'
# Search for any existing env validation pattern used in the repo.
rg -n "DATABASE_URL|throw new Error\\(|process\\.env\\[" -S . --glob '!node_modules' --glob '!dist' --glob '!build'Repository: xoloAyush/AI_ppt_v2
Length of output: 2012
🌐 Web query:
Prisma config defineConfig datasource url process.env DATABASE_URL missing runtime error fail fast
💡 Result:
The Prisma env() helper function in prisma.config.ts is designed to fail fast when an environment variable is missing [1][2]. This behavior ensures that configuration errors are caught immediately rather than causing unpredictable runtime failures [3]. If your environment variable (e.g., DATABASE_URL) is not required for all Prisma commands—such as when running prisma generate in CI/CD pipelines where a database connection is not needed—this strict behavior will cause those commands to fail [1][4]. To resolve this and avoid the fail-fast behavior, you should replace the env() helper with a direct process.env access [1][2]: // Change this: datasource: { url: env("DATABASE_URL"), }, // To this: datasource: { url: process.env.DATABASE_URL, }, Alternatively, you can provide a fallback value using the nullish coalescing operator if you want to avoid undefined values while still preventing an error: datasource: { url: process.env.DATABASE_URL?? "", }, Additionally, ensure that you have initialized your environment variables correctly. Since Prisma Config does not automatically load .env files for you, you must manually import a loader like dotenv at the top of your prisma.config.ts file if you rely on a .env file for your configuration [5][6]: import "dotenv/config"; import { defineConfig } from "prisma/config"; export default defineConfig({ //... }); By using process.env instead of env(), you maintain flexibility for commands that do not require an active database connection [1][7].
Citations:
- 1: https://www.prisma.io/docs/orm/reference/prisma-config-reference
- 2: https://www.prisma.io/docs/orm/v6/reference/prisma-config-reference
- 3: feat(config): add
envhelper prisma/orm#28305 - 4: Cannot run any Prisma commands or generate the client if the
DATABASE_URLenvironment variable is not set prisma/orm#28869 - 5: Environment variable not found: DATABASE_URL prisma/orm#28408
- 6: Prisma config file loses env url prisma/orm#27855
- 7: Prisma 7 says missing datasource in docker build prisma/orm#28576
🌐 Web query:
Prisma defineConfig datasource.url process.env.DATABASE_URL as string generated prisma.config.ts template
💡 Result:
To configure the datasource URL in a prisma.config.ts file, you should use the defineConfig helper from prisma/config along with the env helper [1][2]. You must also ensure that environment variables are loaded by importing dotenv/config at the top of your configuration file [1][3]. Below is a template for a prisma.config.ts file: import "dotenv/config"; import { defineConfig, env } from "prisma/config"; export default defineConfig({ schema: "prisma/schema.prisma", datasource: { // The env helper ensures the variable is present and provides better type safety [1][3] url: env("DATABASE_URL"), }, }); Key Considerations: 1. Environment Variable Loading: Importing "dotenv/config" at the top of the file is required to load variables from your.env file into process.env so that the env helper can access them [1][3]. 2. Handling Optional Variables: If the environment variable is not guaranteed to exist (for example, in certain CI/CD environments where you only need to run type-checking), you should use process.env directly rather than the env helper [1][4]. // Alternative approach for optional environment variables import "dotenv/config"; import { defineConfig } from "prisma/config"; export default defineConfig({ schema: "prisma/schema.prisma", datasource: { // Uses process.env directly with a fallback or non-null assertion [1][4] url: process.env.DATABASE_URL?? "", }, }); 3. Prisma Version Compatibility: Note that the ability to configure the datasource URL in prisma.config.ts is a feature introduced in later versions of Prisma ORM (notably becoming standard in v7 for configuring schema location and datasource settings) [4][5]. If you are using an earlier version, these settings remain in the datasource block of your schema.prisma file [4].
Citations:
- 1: https://www.prisma.io/docs/orm/reference/prisma-config-reference
- 2: https://www.prisma.io/docs/orm/prisma-client/setup-and-configuration/introduction
- 3: https://docs.prisma.io/docs/orm/reference/prisma-config-reference
- 4: https://www.prisma.io/docs/orm/v6/reference/prisma-config-reference
- 5: https://www.prisma.io/docs/orm/prisma-schema/overview/location
Fail fast if DATABASE_URL is missing.
as string only suppresses TypeScript’s check; at runtime it can still pass undefined, so Prisma will fail later with a less clear config error. Use env("DATABASE_URL") or an explicit guard here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prisma.config.ts` at line 12, The Prisma config is relying on a type
assertion for DATABASE_URL, which can still allow undefined at runtime and defer
the failure to Prisma. Update the database URL assignment in prisma.config.ts to
use env("DATABASE_URL") or add an explicit guard before constructing the config
so the process fails immediately with a clear error when the environment
variable is missing.
| CREATE TABLE "slide" ( | ||
| "id" TEXT NOT NULL, | ||
| "presentationId" TEXT NOT NULL, | ||
| "order" INTEGER NOT NULL, | ||
| "title" TEXT NOT NULL, | ||
| "content" TEXT NOT NULL, | ||
| "notes" TEXT, | ||
| "imageUrl" TEXT, | ||
| "imagePrompt" TEXT, | ||
| "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| "updatedAt" TIMESTAMP(3) NOT NULL, | ||
|
|
||
| CONSTRAINT "slide_pkey" PRIMARY KEY ("id") | ||
| ); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "presentation_userId_idx" ON "presentation"("userId"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "slide_presentationId_idx" ON "slide"("presentationId"); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enforce unique slide positions per presentation.
Every read path treats order as the slide sequence, but this schema still allows duplicate ("presentationId", "order") pairs. That makes the orderBy: { order: 'asc' } contract nondeterministic and opens the door to conflicting slide writes. Add a composite unique constraint here (and the matching @@unique in prisma/schema.prisma).
Suggested migration change
-- CreateIndex
CREATE INDEX "slide_presentationId_idx" ON "slide"("presentationId");
+
+-- CreateIndex
+CREATE UNIQUE INDEX "slide_presentationId_order_key" ON "slide"("presentationId", "order");📝 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.
| CREATE TABLE "slide" ( | |
| "id" TEXT NOT NULL, | |
| "presentationId" TEXT NOT NULL, | |
| "order" INTEGER NOT NULL, | |
| "title" TEXT NOT NULL, | |
| "content" TEXT NOT NULL, | |
| "notes" TEXT, | |
| "imageUrl" TEXT, | |
| "imagePrompt" TEXT, | |
| "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| "updatedAt" TIMESTAMP(3) NOT NULL, | |
| CONSTRAINT "slide_pkey" PRIMARY KEY ("id") | |
| ); | |
| -- CreateIndex | |
| CREATE INDEX "presentation_userId_idx" ON "presentation"("userId"); | |
| -- CreateIndex | |
| CREATE INDEX "slide_presentationId_idx" ON "slide"("presentationId"); | |
| CREATE TABLE "slide" ( | |
| "id" TEXT NOT NULL, | |
| "presentationId" TEXT NOT NULL, | |
| "order" INTEGER NOT NULL, | |
| "title" TEXT NOT NULL, | |
| "content" TEXT NOT NULL, | |
| "notes" TEXT, | |
| "imageUrl" TEXT, | |
| "imagePrompt" TEXT, | |
| "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | |
| "updatedAt" TIMESTAMP(3) NOT NULL, | |
| CONSTRAINT "slide_pkey" PRIMARY KEY ("id") | |
| ); | |
| -- CreateIndex | |
| CREATE INDEX "presentation_userId_idx" ON "presentation"("userId"); | |
| -- CreateIndex | |
| CREATE INDEX "slide_presentationId_idx" ON "slide"("presentationId"); | |
| -- CreateIndex | |
| CREATE UNIQUE INDEX "slide_presentationId_order_key" ON "slide"("presentationId", "order"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prisma/migrations/20260627170407_presentation_slides/migration.sql` around
lines 22 - 41, The slide schema currently allows duplicate sequence positions
within the same presentation, so update the slide definition to enforce
uniqueness on the presentationId and order pair. Add the composite unique
constraint in the slide model in prisma/schema.prisma and mirror it in the
migration SQL for the slide table, using the existing slide, presentationId, and
order symbols to locate the change.
| order Int | ||
| title String | ||
| content String @db.Text | ||
| notes String? @db.Text | ||
| imageUrl String? | ||
| imagePrompt String? | ||
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
|
|
||
| @@index([presentationId]) | ||
| @@map("slide") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enforce unique slide ordering per presentation.
getPresentationWithSlides sorts by order, but this schema still allows two slides in the same presentation to share the same value. That makes the slide sequence nondeterministic and weakens the data contract for any UI/navigation built on it.
Suggested schema fix
model Slide {
id String `@id` `@default`(cuid())
presentationId String
presentation Presentation `@relation`(fields: [presentationId], references: [id], onDelete: Cascade)
order Int
title String
content String `@db.Text`
notes String? `@db.Text`
imageUrl String?
imagePrompt String?
createdAt DateTime `@default`(now())
updatedAt DateTime `@updatedAt`
- @@index([presentationId])
+ @@unique([presentationId, order])
@@map("slide")
}📝 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.
| order Int | |
| title String | |
| content String @db.Text | |
| notes String? @db.Text | |
| imageUrl String? | |
| imagePrompt String? | |
| createdAt DateTime @default(now()) | |
| updatedAt DateTime @updatedAt | |
| @@index([presentationId]) | |
| @@map("slide") | |
| order Int | |
| title String | |
| content String `@db.Text` | |
| notes String? `@db.Text` | |
| imageUrl String? | |
| imagePrompt String? | |
| createdAt DateTime `@default`(now()) | |
| updatedAt DateTime `@updatedAt` | |
| @@unique([presentationId, order]) | |
| @@map("slide") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prisma/schema.prisma` around lines 57 - 67, The slide ordering in the schema
is not constrained per presentation, so duplicate values can make the sequence
nondeterministic. Update the Slide model in prisma/schema.prisma to enforce a
unique ordering within each presentation by adding a composite uniqueness
constraint on presentationId and order, and keep the existing
getPresentationWithSlides ordering logic unchanged since it will then rely on a
stable contract.
| status: 'GENERATING', | ||
| }, | ||
| }) | ||
|
|
||
| // await inngest.send({ | ||
| // name: 'presentation/generate', | ||
| // data: { presentationId: presentation.id }, | ||
| // }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
These mutations leave presentations stuck in GENERATING.
Both paths flip the status to GENERATING, but neither one actually dispatches generation work right now. With the queue call commented out, newly created/regenerated presentations will sit in the loading state indefinitely.
Also applies to: 75-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/presentations/actions/presentation-mutations.ts` around lines 26
- 33, The presentation mutation flow is leaving items stuck in GENERATING
because the generation job is never dispatched after the status update. In the
relevant mutation handlers in presentation-mutations.ts, restore the queued
generation step by re-enabling the inngest.send call (or otherwise dispatching
the generation work) for both the create and regenerate paths so the status
transition is followed by actual processing.
| export const getPresentation = createServerFn({ method: 'GET' }) | ||
| .inputValidator((data: unknown) => presentationIdInputSchema.parse(data)) | ||
| .handler(async ({ data }) => { | ||
| const userId = await requirePresentationUserId() | ||
| const row = await prisma.presentation.findFirst({ | ||
| where: { id: data.id, userId }, | ||
| }) | ||
| if (!row) throw new Error('Not found') | ||
| return row | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== presentation-queries.ts ==\n'
sed -n '1,220p' src/features/presentations/api/presentation-queries.ts
printf '\n== search notFound / 404 helpers ==\n'
rg -n "notFound|404|Not found|createServerFn|throw new Error\\('Not found'\\)" src . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'
printf '\n== locate related presentation handlers ==\n'
rg -n "presentation-queries|presentation-mutations|requirePresentationUserId|presentationIdInputSchema" src --glob '!**/node_modules/**'Repository: xoloAyush/AI_ppt_v2
Length of output: 8182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files importing notFound ==\n'
rg -n "from ['\"][^'\"]*notFound|\\bnotFound\\(" src . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'
printf '\n== package deps mentioning tanstack/router/start ==\n'
sed -n '1,220p' package.json 2>/dev/null || true
if [ -f pnpm-lock.yaml ]; then
rg -n "\"`@tanstack/`(react-router|start|router)\"" pnpm-lock.yaml | head -n 40
fi
if [ -f package-lock.json ]; then
rg -n "\"`@tanstack/`(react-router|start|router)\"" package-lock.json | head -n 40
fi
if [ -f yarn.lock ]; then
rg -n "`@tanstack/`(react-router|start|router)" yarn.lock | head -n 40
fiRepository: xoloAyush/AI_ppt_v2
Length of output: 189
🌐 Web query:
TanStack React Start createServerFn notFound 404 server function error handling
💡 Result:
In TanStack Start, you handle "Not Found" states in server functions by throwing the notFound() utility imported from @tanstack/react-router [1][2]. When a server function throws notFound(), the framework automatically serializes this to the client, allowing the router to handle it and trigger the appropriate notFoundComponent [1][3]. Key implementation details and best practices include: Using the notFound utility: Within your server function handler, check for the absence of a resource and throw notFound() [1][4]. import { createServerFn } from '@tanstack/react-start' import { notFound } from '@tanstack/react-router' export const getPost = createServerFn.validator((id: string) => id).handler(async ({ data }) => { const post = await db.findPost(data) if (!post) { throw notFound } return post }) Requirement for useServerFn: If a server function is capable of throwing notFound() or redirect(), it should be invoked using the useServerFn hook on the client [5]. The useServerFn hook is responsible for "wiring" these thrown router exceptions into the TanStack Router lifecycle, ensuring that the router correctly performs the navigation or renders the not-found boundary [5]. Calling the server function directly is safe for functions that only return data, but useServerFn is the recommended standard when these control-flow exceptions are involved [5]. Handling the visual result: To ensure a good user experience, you must configure your application to display the not-found state [6]. This is done by adding a notFoundComponent to your root route or specific child routes within your TanStack Router configuration [6]. If no notFoundComponent is defined, the framework will fall back to a default, basic "Not Found" message [6]. Important considerations: - Troubleshooting: If you encounter 500 errors instead of 404s, ensure you are using useServerFn if the function throws, and avoid manual HTTP response manipulation (like setResponseStatus(404)) which may bypass the router's error handling mechanisms [7][5]. - Granularity: You can optionally pass a routeId to the notFound() function if you need to target a specific route's not-found boundary rather than the nearest one [6][8].
Citations:
- 1: https://tanstack.com/start/latest/docs/framework/react/guide/server-functions
- 2: https://tanstack-router-63.mintlify.app/start/guides/server-functions
- 3: https://tanstack.com/start/v0/docs/framework/react/guide/server-functions
- 4: https://tanstack-router-63.mintlify.app/start/react/server-functions
- 5: https://unpkg.com/@tanstack/start-client-core@1.170.12/skills/start-core/server-functions/SKILL.md
- 6: https://tanstack.com/router/v1/docs/guide/not-found-errors
- 7: 404 error handling broken in loaders and server functions - HTML fragments shown instead of error messages TanStack/router#5960
- 8: https://tanstack.com/router/v1/docs/api/router/notFoundFunction
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== usePresentation-detail.ts ==\n'
sed -n '1,220p' src/features/presentations/hooks/usePresentation-detail.ts
printf '\n== route usage of detail hook ==\n'
rg -n "usePresentation-detail|getPresentationWithSlides|presentation detail|presentation not found|Not found" src/routes src/features/presentations --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'Repository: xoloAyush/AI_ppt_v2
Length of output: 4978
Use notFound() for both missing-record branches, and invoke the detail fetch through useServerFn. Error('Not found') turns stale or deleted IDs into generic query failures instead of the router’s not-found state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/presentations/api/presentation-queries.ts` around lines 18 - 27,
The getPresentation server function currently throws a generic Error for missing
records, which bypasses the router’s not-found handling. Update the
getPresentation handler to call notFound() for every missing-record path,
including stale/deleted IDs after the prisma.presentation.findFirst lookup, and
keep the user lookup via requirePresentationUserId intact. Also make sure the
presentation detail fetch is performed through useServerFn so the client-side
route can receive and react to the not-found state correctly.
| export const presentationIdInputSchema = z.object({ id: z.string().min(1) }) | ||
|
|
||
| export const createPresentationInputSchema = z.object({ | ||
| prompt: z.string().min(1).max(50_000), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Trim prompt/title before validating length.
z.string().min(1) accepts whitespace-only input, so create/update can still persist effectively blank prompts or titles.
Suggested validation change
export const createPresentationInputSchema = z.object({
- prompt: z.string().min(1).max(50_000),
+ prompt: z.string().trim().min(1).max(50_000),
slideCount: z.number().int().min(3).max(20),
style: presentationStyleSchema,
tone: presentationToneSchema,
layout: presentationLayoutSchema,
})
export const updatePresentationInputSchema = z
.object({
id: z.string().min(1),
- title: z.string().min(1).max(200).optional(),
- prompt: z.string().min(1).max(50_000).optional(),
+ title: z.string().trim().min(1).max(200).optional(),
+ prompt: z.string().trim().min(1).max(50_000).optional(),
slideCount: z.number().int().min(3).max(20).optional(),
style: presentationStyleSchema.optional(),
tone: presentationToneSchema.optional(),
layout: presentationLayoutSchema.optional(),
})Also applies to: 37-38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/presentations/types/schema.ts` at line 27, The schema validation
for prompt/title in the presentation types still allows whitespace-only values
because z.string().min(1) is checked before trimming; update the prompt and
title validators in schema.ts so they trim input before enforcing min/max
length. Apply the same fix to both the create/update variants referenced by the
prompt and title fields, using the existing schema definitions in that file to
ensure blank-looking strings are rejected consistently.
| async ({ event, step }) => { | ||
| await step.sleep("wait-a-moment", "1s"); | ||
| return { message: `Hello ${event.data.email}!` }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate the event payload before dereferencing email.
This handler assumes every test/hello.world event includes data.email. A malformed or partial event will either throw on event.data.email or return Hello undefined!, which turns a bad input into a failed run.
Suggested fix
export const helloWorld = inngest.createFunction(
{ id: "hello-world", triggers: [{ event: "test/hello.world" }] },
async ({ event, step }) => {
+ const email = event.data?.email;
+ if (!email) {
+ throw new Error("Missing email in test/hello.world event");
+ }
+
await step.sleep("wait-a-moment", "1s");
- return { message: `Hello ${event.data.email}!` };
+ return { message: `Hello ${email}!` };
},
);📝 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.
| async ({ event, step }) => { | |
| await step.sleep("wait-a-moment", "1s"); | |
| return { message: `Hello ${event.data.email}!` }; | |
| export const helloWorld = inngest.createFunction( | |
| { id: "hello-world", triggers: [{ event: "test/hello.world" }] }, | |
| async ({ event, step }) => { | |
| const email = event.data?.email; | |
| if (!email) { | |
| throw new Error("Missing email in test/hello.world event"); | |
| } | |
| await step.sleep("wait-a-moment", "1s"); | |
| return { message: `Hello ${email}!` }; | |
| }, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/integrations/inngest/functions.ts` around lines 5 - 7, The handler in the
inngest function assumes event.data.email is always present, which can cause a
crash or return an invalid greeting for malformed events. Update the async
handler for the test/hello.world event to validate the event payload before
using event.data.email, and handle missing/invalid data with an explicit
fallback or failure path instead of dereferencing it directly. Keep the fix
localized to the function body that calls step.sleep and builds the Hello
message.
| const createMut = useMutation({ | ||
| mutationFn: ()=> createPresentation({ | ||
| data:{ | ||
| prompt : form.content.trim(), | ||
| slideCount: form.slideCount, | ||
| style: form.style, | ||
| tone: form.tone, | ||
| layout: form.layout, | ||
| }, | ||
| }), | ||
| onSuccess: (presentation)=>{ | ||
| toast.success('Presentation created') | ||
| queryClient.invalidateQueries({ | ||
| queryKey: presentationQueryKeys.list() | ||
| }) | ||
| navigate({ | ||
| to: '/presentation/$presentationId', | ||
| params: {presentationId: presentation.id} | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Start the generation job before treating this as a successful create flow.
This success path assumes createPresentation kicks off generation, but the server action currently only inserts a row with status: 'GENERATING' and returns it. In src/features/presentations/actions/presentation-mutations.ts, Lines 27-30, the inngest.send(...) call is still commented out, so this route will navigate users to a presentation that never progresses.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/index.tsx` around lines 81 - 99, The create flow in
createMut/onSuccess assumes generation has already started, but
createPresentation currently only inserts the presentation row and returns it.
Update the server action in presentation-mutations to actually kick off the
generation job by restoring the inngest.send(...) call (or equivalent job
dispatch) inside createPresentation so the returned GENERATING record is backed
by a running job. Keep the route logic in src/routes/index.tsx as the success
handler only after the mutation truly starts generation.
| const data = query.data | ||
| const thumb = presentationThumbnailUrl(data.id) | ||
| const activeSlide = slides.at(activeSlideIndex) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clamp the selected slide when the slide count shrinks.
If the presentation is regenerated or edited down to fewer slides than activeSlideIndex, slides.at(activeSlideIndex) becomes undefined and the detail view loses its preview even though slides still exist. Reset or clamp the index whenever slides.length changes.
Possible fix
-import { useCallback, useState } from 'react'
+import { useCallback, useEffect, useState } from 'react' const {
query,
slides,
@@
} = usePresentationDetail(presentationId, {
onDeleted: () => navigate({ to: '/' }),
})
+
+ useEffect(() => {
+ if (slides.length === 0) {
+ setActiveSlideIndex(0)
+ return
+ }
+
+ setActiveSlideIndex((i) => Math.min(i, slides.length - 1))
+ }, [slides.length])📝 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 data = query.data | |
| const thumb = presentationThumbnailUrl(data.id) | |
| const activeSlide = slides.at(activeSlideIndex) | |
| import { useCallback, useEffect, useState } from 'react' |
| const data = query.data | |
| const thumb = presentationThumbnailUrl(data.id) | |
| const activeSlide = slides.at(activeSlideIndex) | |
| const { | |
| query, | |
| slides, | |
| @@ | |
| } = usePresentationDetail(presentationId, { | |
| onDeleted: () => navigate({ to: '/' }), | |
| }) | |
| useEffect(() => { | |
| if (slides.length === 0) { | |
| setActiveSlideIndex(0) | |
| return | |
| } | |
| setActiveSlideIndex((i) => Math.min(i, slides.length - 1)) | |
| }, [slides.length]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/presentation`.$presentationId.tsx around lines 147 - 149, The
detail view can lose its preview when the slide list shrinks because
presentationThumbnailUrl, activeSlideIndex, and slides.at(activeSlideIndex)
still assume the old index is valid. In
src/routes/presentation.$presentationId.tsx, update the logic around
activeSlideIndex so it is clamped or reset whenever slides.length changes, and
ensure the selected slide is always derived from a valid index before rendering
the preview.
| {slides.length > 0 && ( | ||
| <> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| className="rounded-xl gap-1" | ||
| onClick={() => setShowSlideshow(true)} | ||
| > | ||
| <Play className="size-4" /> | ||
| <span className="hidden sm:inline">Slideshow</span> | ||
| </Button> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| className="rounded-xl gap-1" | ||
| // onClick={handleExportPptx} | ||
| // disabled={isExporting} | ||
| > | ||
| <Download className="size-4" /> | ||
| {/* <span className="hidden sm:inline"> | ||
| {isExporting ? 'Exporting…' : 'Export'} | ||
| </span> */} | ||
| </Button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
This route still ships without a working slide-viewing flow.
SlidePreview, the sidebar slide list, and the slideshow modal are all commented out, and the export button is rendered without an action. For presentations that already have slides, users end up with an empty preview area plus controls that don't do anything. Hide these controls or wire the rendering components back in before merge.
Also applies to: 417-430, 496-522
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/presentation`.$presentationId.tsx around lines 191 - 213, The
presentation route still exposes slide-related controls without any working
slide-viewing behavior. In presentation.$presentationId.tsx, either restore the
missing slide flow by wiring back SlidePreview, the sidebar slide list, the
slideshow modal, and the export action handlers, or remove/hide the Button
controls and related UI until they are functional. Make sure the JSX branches
around slides.length, setShowSlideshow, and the export button are consistent so
users never see empty preview areas or non-working controls.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes