Open
Conversation
carlagn
requested review from
AmanVarshney01,
ArthurGamby,
aidankmcalister,
ankur-arch,
mhartington and
mhessdev
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/site/src/lib/shiki_prisma.ts (1)
225-243: Use promise-based singleton for more robust highlighter initializationThe current null-check pattern can race if
activeStepchanges during the initialcreateHighlighter()call, potentially instantiating multiple highlighters. While the impact is minor (browser-side, single call site), using the nullish coalescing operator with a promise is cleaner and more resilient.Suggested pattern
-let prisma_highlighter: Awaited<ReturnType<typeof createHighlighter>> | null = - null; +let prismaHighlighterPromise: ReturnType<typeof createHighlighter> | null = null; async function getHighlighter() { - if (!prisma_highlighter) { - prisma_highlighter = await createHighlighter({ + prismaHighlighterPromise ??= createHighlighter({ themes: [prismaTheme], langs: [ "typescript", "javascript", "jsx", "tsx", "json", "bash", "sh", "prisma", "sql", "diff", ], - }); - } - return prisma_highlighter; + }); + return prismaHighlighterPromise; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/lib/shiki_prisma.ts` around lines 225 - 243, The current null-check in getHighlighter can race; switch to a promise-based singleton by assigning the createHighlighter promise to prisma_highlighter before awaiting it (e.g. in getHighlighter use prisma_highlighter ||= createHighlighter({...}) and then await prisma_highlighter), so multiple concurrent calls reuse the same promise; update any related type to reflect that prisma_highlighter holds the creation promise and ensure getHighlighter awaits and returns the resolved highlighter (references: getHighlighter, prisma_highlighter, createHighlighter).apps/site/src/components/migrate/hero-code.tsx (1)
51-57: Makediff-adddetection line-start based, not token-presence based.Current logic can mark lines as added when
+appears later in the line (not as a diff prefix).💡 Proposed refactor
const processedSchemaHtml = schemaHtml.replace( /<span class="line">([\s\S]*?)(<\/span>)/g, (match, content, closing) => { - // Check if line starts with + (accounting for any leading spans) - const startsWithPlus = - /<span[^>]*>\+/.test(content) || content.trim().startsWith("+"); + const plainTextLine = content.replace(/<[^>]+>/g, ""); + const startsWithPlus = plainTextLine.trimStart().startsWith("+"); if (startsWithPlus) { return `<span class="line diff-add">${content}${closing}`; } return match; }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/migrate/hero-code.tsx` around lines 51 - 57, The current startsWithPlus detection in the processedSchemaHtml.replace callback incorrectly flags lines with a '+' anywhere in the token content; instead determine if the first visible character of the line is '+' by stripping any leading span tags and whitespace from the captured content and then checking the very first character — update the logic in the replace callback (where processedSchemaHtml and the captured content variable are used) to remove leading <span...> tags and whitespace and only then test for a leading '+' so only true diff-prefix additions are marked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/site/src/app/migrate/page.tsx`:
- Around line 146-148: Update the h4 heading text currently rendered as
"Deterministic/ Repeatable" inside the JSX element (the <h4 className="text-xl
text-foreground-neutral font-sans-display font-extrabold">) to correct the slash
spacing; change it to either "Deterministic/Repeatable" (no space) or
"Deterministic / Repeatable" (space on both sides) for consistent punctuation
spacing in the UI copy.
- Around line 340-344: The paragraph element with className
"text-foreground-neutral-weak" under the "Streamlined collaboration" card is
duplicating the previous card's version-control copy; update that paragraph to
match the "Streamlined collaboration" heading by replacing the text with a
description about collaborative workflows (e.g., shared migration history, team
reviews, and safe rollbacks) so the card content aligns with its title; locate
the JSX block in page.tsx that renders the "Streamlined collaboration" card and
change the inner text of the paragraph element accordingly.
In `@apps/site/src/components/migrate/hero-code.tsx`:
- Around line 20-22: The component assumes steps has entries and dereferences
steps[activeStep] unconditionally, which will crash for empty arrays; update the
HeroCode component (types HeroCodeProps / HeroCodeStep, variables steps and
activeStep) to first guard that steps?.length > 0 (or return a lightweight
fallback/placeholder) before accessing steps[activeStep], clamp activeStep to
the valid index range (e.g., Math.min(activeStep, steps.length - 1)), and
replace direct property access with optional chaining/default values so all uses
(the places referencing steps[activeStep]) safely handle an empty steps array.
- Around line 69-73: When catching highlight errors in the try/catch (currently
using isMounted and setIsLoading), also clear the component's highlighted HTML
state so stale code isn't shown; call the state updater that stores the rendered
HTML (e.g., setHighlightedHtml('') or setHighlightedCode('') — whichever exists
in this component) inside the catch before setting isLoading to false, and only
do both calls when isMounted is true.
---
Nitpick comments:
In `@apps/site/src/components/migrate/hero-code.tsx`:
- Around line 51-57: The current startsWithPlus detection in the
processedSchemaHtml.replace callback incorrectly flags lines with a '+' anywhere
in the token content; instead determine if the first visible character of the
line is '+' by stripping any leading span tags and whitespace from the captured
content and then checking the very first character — update the logic in the
replace callback (where processedSchemaHtml and the captured content variable
are used) to remove leading <span...> tags and whitespace and only then test for
a leading '+' so only true diff-prefix additions are marked.
In `@apps/site/src/lib/shiki_prisma.ts`:
- Around line 225-243: The current null-check in getHighlighter can race; switch
to a promise-based singleton by assigning the createHighlighter promise to
prisma_highlighter before awaiting it (e.g. in getHighlighter use
prisma_highlighter ||= createHighlighter({...}) and then await
prisma_highlighter), so multiple concurrent calls reuse the same promise; update
any related type to reflect that prisma_highlighter holds the creation promise
and ensure getHighlighter awaits and returns the resolved highlighter
(references: getHighlighter, prisma_highlighter, createHighlighter).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c627cf95-fa6a-4625-a15b-1e94ae7b7d3d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
apps/site/package.jsonapps/site/src/app/global.cssapps/site/src/app/layout.tsxapps/site/src/app/migrate/page.tsxapps/site/src/components/migrate/hero-code.tsxapps/site/src/lib/cn.tsapps/site/src/lib/shiki_prisma.tspackages/eclipse/src/components/button.tsxpackages/ui/src/styles/globals.css
Comment on lines
+146
to
+148
| <h4 className="text-xl text-foreground-neutral font-sans-display font-extrabold"> | ||
| Deterministic/ Repeatable | ||
| </h4> |
Contributor
There was a problem hiding this comment.
Fix heading punctuation spacing
Deterministic/ Repeatable has an extra space pattern around the slash and reads unpolished in the UI copy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/app/migrate/page.tsx` around lines 146 - 148, Update the h4
heading text currently rendered as "Deterministic/ Repeatable" inside the JSX
element (the <h4 className="text-xl text-foreground-neutral font-sans-display
font-extrabold">) to correct the slash spacing; change it to either
"Deterministic/Repeatable" (no space) or "Deterministic / Repeatable" (space on
both sides) for consistent punctuation spacing in the UI copy.
Comment on lines
+340
to
+344
| <p className="text-foreground-neutral-weak"> | ||
| With Prisma Migrate, generated migrations are tracked in your | ||
| Git repository, allowing you to make changes to your database | ||
| schema in tandem with your application code. | ||
| </p> |
Contributor
There was a problem hiding this comment.
Card body copy is duplicated and mismatched to its title
The “Streamlined collaboration” card repeats the previous card’s version-control description, so the content does not match the heading.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/app/migrate/page.tsx` around lines 340 - 344, The paragraph
element with className "text-foreground-neutral-weak" under the "Streamlined
collaboration" card is duplicating the previous card's version-control copy;
update that paragraph to match the "Streamlined collaboration" heading by
replacing the text with a description about collaborative workflows (e.g.,
shared migration history, team reviews, and safe rollbacks) so the card content
aligns with its title; locate the JSX block in page.tsx that renders the
"Streamlined collaboration" card and change the inner text of the paragraph
element accordingly.
Comment on lines
+20
to
+22
| interface HeroCodeProps { | ||
| steps: HeroCodeStep[]; | ||
| } |
Contributor
There was a problem hiding this comment.
Guard against empty steps to prevent runtime crashes.
steps is typed as possibly empty, but Line [40], Line [119], Line [152], and Line [180] dereference steps[activeStep] unconditionally.
💡 Proposed fix
interface HeroCodeProps {
- steps: HeroCodeStep[];
+ steps: [HeroCodeStep, ...HeroCodeStep[]];
}
const HeroCode: React.FC<HeroCodeProps> = ({ steps }) => {
const [activeStep, setActiveStep] = useState(0);
+ const currentStep = steps[activeStep] ?? steps[steps.length - 1];
@@
- highlighter.codeToHtml(steps[activeStep].schema, {
+ highlighter.codeToHtml(currentStep.schema, {
lang: "prisma",
theme: "prisma-dark",
}),
- highlighter.codeToHtml(steps[activeStep].migrateFileContents, {
+ highlighter.codeToHtml(currentStep.migrateFileContents, {
lang: "sql",
theme: "prisma-dark",
}),
@@
- {steps[activeStep].title}
+ {currentStep.title}
@@
- {steps[activeStep].migrateFileName}
+ {currentStep.migrateFileName}
@@
- transform: `translate(${steps[activeStep].arrowOffset.x}px, ${steps[activeStep].arrowOffset.y}px) rotate(${steps[activeStep].arrowOffset.rotation}deg)`,
+ transform: `translate(${currentStep.arrowOffset.x}px, ${currentStep.arrowOffset.y}px) rotate(${currentStep.arrowOffset.rotation}deg)`,Also applies to: 40-45, 119-120, 152-153, 180-180
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/components/migrate/hero-code.tsx` around lines 20 - 22, The
component assumes steps has entries and dereferences steps[activeStep]
unconditionally, which will crash for empty arrays; update the HeroCode
component (types HeroCodeProps / HeroCodeStep, variables steps and activeStep)
to first guard that steps?.length > 0 (or return a lightweight
fallback/placeholder) before accessing steps[activeStep], clamp activeStep to
the valid index range (e.g., Math.min(activeStep, steps.length - 1)), and
replace direct property access with optional chaining/default values so all uses
(the places referencing steps[activeStep]) safely handle an empty steps array.
Comment on lines
+69
to
+73
| } catch (error) { | ||
| console.error("Failed to highlight code:", error); | ||
| if (isMounted) { | ||
| setIsLoading(false); | ||
| } |
Contributor
There was a problem hiding this comment.
Clear highlighted content on highlight failure.
If highlighting fails, old HTML remains rendered while the button title/filename can reflect a different step, which is misleading state.
💡 Proposed fix
} catch (error) {
console.error("Failed to highlight code:", error);
if (isMounted) {
+ setHighlightedSchema("");
+ setHighlightedMigration("");
setIsLoading(false);
}
}📝 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.
Suggested change
| } catch (error) { | |
| console.error("Failed to highlight code:", error); | |
| if (isMounted) { | |
| setIsLoading(false); | |
| } | |
| } catch (error) { | |
| console.error("Failed to highlight code:", error); | |
| if (isMounted) { | |
| setHighlightedSchema(""); | |
| setHighlightedMigration(""); | |
| setIsLoading(false); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/components/migrate/hero-code.tsx` around lines 69 - 73, When
catching highlight errors in the try/catch (currently using isMounted and
setIsLoading), also clear the component's highlighted HTML state so stale code
isn't shown; call the state updater that stores the rendered HTML (e.g.,
setHighlightedHtml('') or setHighlightedCode('') — whichever exists in this
component) inside the catch before setting isLoading to false, and only do both
calls when isMounted is true.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/site/src/components/migrate/hero-code.tsx (2)
69-73:⚠️ Potential issue | 🟡 MinorClear highlighted HTML on highlight failure to avoid stale UI state.
When highlighting fails, the component keeps previous HTML visible while step metadata can change, which is misleading.
💡 Suggested fix
} catch (error) { console.error("Failed to highlight code:", error); if (isMounted) { + setHighlightedSchema(""); + setHighlightedMigration(""); setIsLoading(false); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/migrate/hero-code.tsx` around lines 69 - 73, On highlight failure, clear the previously rendered highlighted HTML so stale content isn't shown: inside the catch block where you currently call console.error and setIsLoading(false) (guarded by isMounted), also call setHighlightedHtml('') (and any related state like setHighlightedLang('') if present) while still checking isMounted so the component shows no highlighted output after an error.
20-22:⚠️ Potential issue | 🟠 MajorGuard
stepsaccess and clamp active index to prevent render/runtime crashes.
steps[activeStep]is dereferenced in several places without guarding empty arrays or out-of-range indices after prop changes. This can crash rendering and the highlighter path.💡 Suggested fix
interface HeroCodeProps { - steps: HeroCodeStep[]; + steps: [HeroCodeStep, ...HeroCodeStep[]]; } const HeroCode: React.FC<HeroCodeProps> = ({ steps }) => { const [activeStep, setActiveStep] = useState(0); + if (!steps.length) return null; + const safeStepIndex = Math.min(activeStep, steps.length - 1); + const currentStep = steps[safeStepIndex]; @@ - highlighter.codeToHtml(steps[activeStep].schema, { + highlighter.codeToHtml(currentStep.schema, { lang: "prisma", theme: "prisma-dark", }), - highlighter.codeToHtml(steps[activeStep].migrateFileContents, { + highlighter.codeToHtml(currentStep.migrateFileContents, { lang: "sql", theme: "prisma-dark", }), @@ - activeStep === steps.length - 1 - ? setActiveStep(0) - : setActiveStep(activeStep + 1); + setActiveStep((prev) => (prev >= steps.length - 1 ? 0 : prev + 1)); @@ - {steps[activeStep].title} + {currentStep.title} @@ - {steps[activeStep].migrateFileName} + {currentStep.migrateFileName} @@ - transform: `translate(${steps[activeStep].arrowOffset.x}px, ${steps[activeStep].arrowOffset.y}px) rotate(${steps[activeStep].arrowOffset.rotation}deg)`, + transform: `translate(${currentStep.arrowOffset.x}px, ${currentStep.arrowOffset.y}px) rotate(${currentStep.arrowOffset.rotation}deg)`,Also applies to: 40-45, 86-89, 119-120, 152-156, 183-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/migrate/hero-code.tsx` around lines 20 - 22, The component dereferences steps[activeStep] in multiple places (e.g., rendering the highlighted path and using step properties) without guarding for an empty or out-of-range steps array; update the component that uses HeroCodeProps (references: steps, activeStep, HeroCode) to (1) clamp/normalize activeStep to the valid range (0 .. steps.length-1) whenever props change (use Math.max/Math.min or similar) and (2) guard all usages of steps[activeStep] with a safe fallback (check steps.length > 0 or use optional chaining and a default empty step object) before passing values into the highlighter or rendering to prevent runtime crashes when steps is empty or activeStep is out-of-bounds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/site/src/components/migrate/hero-code.tsx`:
- Around line 69-73: On highlight failure, clear the previously rendered
highlighted HTML so stale content isn't shown: inside the catch block where you
currently call console.error and setIsLoading(false) (guarded by isMounted),
also call setHighlightedHtml('') (and any related state like
setHighlightedLang('') if present) while still checking isMounted so the
component shows no highlighted output after an error.
- Around line 20-22: The component dereferences steps[activeStep] in multiple
places (e.g., rendering the highlighted path and using step properties) without
guarding for an empty or out-of-range steps array; update the component that
uses HeroCodeProps (references: steps, activeStep, HeroCode) to (1)
clamp/normalize activeStep to the valid range (0 .. steps.length-1) whenever
props change (use Math.max/Math.min or similar) and (2) guard all usages of
steps[activeStep] with a safe fallback (check steps.length > 0 or use optional
chaining and a default empty step object) before passing values into the
highlighter or rendering to prevent runtime crashes when steps is empty or
activeStep is out-of-bounds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 43767418-04b3-4537-961f-c5e93c55faa0
📒 Files selected for processing (1)
apps/site/src/components/migrate/hero-code.tsx
aidankmcalister
approved these changes
Mar 25, 2026
Contributor
|
Hi @carlagn, I added a few comments on slack here : https://prisma-company.slack.com/archives/C02UA0D53KM/p1774518475702329?thread_ts=1774462124.529269&cid=C02UA0D53KM Could you also ask claude quickly to take a look a this Major warning to see if it's a false alarm or not Let me know if you need more infos |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Style
Chores