Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a new Lookahead forecasting feature: a client-side LookaheadDashboard with upload/polling/version history, SWR hooks and API helpers, lookahead UI state persistence and types, a server /api/contact POST route, landing-page demo modal/CTA integration, and small navigation/ui adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Client
participant Dashboard as LookaheadDashboard
participant Proxy as Upload Proxy (/api/proxy)
participant Poller as Polling Loop
participant Server as Backend API
participant Store as UI Intent Store
User->>Dashboard: select project & upload file
Dashboard->>Proxy: POST FormData (returns uploadId)
Dashboard->>Dashboard: set state -> uploading, start polling
loop polling
Poller->>Server: GET /programmes/{uploadId}/status
Server-->>Poller: UploadStatusResponse
alt status == "committed"
Poller->>Server: GET snapshot/alerts/versions
Server-->>Dashboard: snapshot, alerts, versions
Dashboard->>Store: persist window size / update intent
Poller->>Dashboard: stop polling (done)
else status == "processing"
Poller-->>Poller: continue polling
else status == "degraded" or error
Poller->>Dashboard: set error state, stop polling
end
end
Dashboard->>User: render heatmap, stats, versions, alerts
sequenceDiagram
participant User as User/Client
participant Modal as ContactModal (client)
participant Validator as Zod Validator
participant API as /api/contact (server)
participant Mailtrap as Mailtrap API
User->>Modal: open -> fill -> submit
Modal->>Validator: validate payload
alt validation fails
Validator-->>Modal: field errors
Modal->>User: show errors
else validation passes
Modal->>API: POST contact payload
API->>API: validate & build email HTML
alt missing prod config or sandbox without inbox
API-->>Modal: { ok: true } (no external send)
else
API->>Mailtrap: POST email (with timeout)
alt 2xx
Mailtrap-->>API: success
API-->>Modal: { ok: true }
Modal->>User: show success
else
Mailtrap-->>API: non-OK
API-->>Modal: 500 error
Modal->>User: show server error
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
src/components/landing/ContactModal.tsx (1)
328-344: Fail fast whenDemoRequestCTAis missing its provider.Because
DemoModalCtxdefaults tonulland Line 340 uses optional chaining, this exported CTA silently renders a dead button when it is used outsideDemoModalProvider. That kind of wiring mistake is much easier to catch if the component throws or at least warns immediately.🧭 Suggested fix
export function DemoRequestCTA({ label = "Book a Demo", className, }: { label?: string; className?: string; }) { const ctx = useContext(DemoModalCtx); + if (!ctx) { + throw new Error("DemoRequestCTA must be used within DemoModalProvider"); + } + return ( <button type="button" className={className} - onClick={() => ctx?.openModal()} + onClick={() => ctx.openModal()} > {label} </button> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/landing/ContactModal.tsx` around lines 328 - 344, DemoRequestCTA silently no-ops when used outside its provider; update the component (DemoRequestCTA) to fail fast by checking the value from useContext(DemoModalCtx) and throwing or logging a clear error if it's null/undefined (e.g., "DemoRequestCTA must be used within DemoModalProvider"), and replace the optional chaining onClick (ctx?.openModal()) with a direct call (ctx.openModal()) after the null check so the component either works or immediately surfaces the missing DemoModalProvider.src/components/landing/LandingPage.tsx (1)
45-49: Minor JSX indentation inconsistency.The
<div>on line 46 should be indented inside<DemoModalProvider>for better readability. This is purely cosmetic but improves code clarity.<DemoModalProvider> - <div + <div id="landing-root"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/landing/LandingPage.tsx` around lines 45 - 49, Adjust JSX indentation so the <div id="landing-root"> is indented as a child of <DemoModalProvider> for readability; locate the opening <DemoModalProvider> in LandingPage.tsx and re-indent the nested <div id="landing-root" ...> (and its closing tag) to be visually nested under DemoModalProvider.src/hooks/lookahead/api.ts (1)
47-49: Consider using a key builder for consistency.
deleteProgrammeVersionhardcodes the path/programmes/${uploadId}while other functions uselookaheadKeys.*. IflookaheadKeysdoesn't have a delete path, consider adding one for consistency and centralized URL management.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/lookahead/api.ts` around lines 47 - 49, deleteProgrammeVersion currently hardcodes the endpoint string `/programmes/${uploadId}` instead of using the shared key builder; add a delete route in the lookaheadKeys key builder (e.g., a method or property like lookaheadKeys.deleteProgramme or lookaheadKeys.programme.delete) that accepts uploadId and returns the path, then update deleteProgrammeVersion to call api.delete(lookaheadKeys.deleteProgramme(uploadId)) so all lookahead endpoints are centralized and consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(dashboard)/lookahead/_components/LookaheadDashboard.tsx:
- Around line 222-243: The delete handler currently only logs errors in
handleDeleteVersion when deleteProgrammeVersion fails; update the catch block to
surface the error to the user (e.g., show a toast or set an inline error state)
using the error message from getApiErrorMessage(err), while keeping the existing
console.error and finally logic; ensure the UI is updated consistently by
invoking the existing mutation functions (mutateVersions, mutateSnapshot,
mutateAlerts) or updating setUploadPhase as appropriate after showing the error
so the user sees failure feedback tied to the failing uploadId.
- Around line 196-219: The startPolling implementation can run forever if status
stays "processing"; modify startPolling (and related pollingRef/stopPolling) to
enforce a max retry count or timeout: add a counter (e.g., attempts) or deadline
(startTime + POLL_TIMEOUT_MS) inside the setInterval callback and
increment/check it each tick, and if exceeded call stopPolling(), setUploadPhase
to an error/timeout kind, and perform the same
mutateSnapshot/mutateAlerts/mutateVersions flow as appropriate; make
POLL_MAX_ATTEMPTS or POLL_TIMEOUT_MS constants (or props) so limits are
configurable and ensure stopPolling clears pollingRef and any timers to avoid
leaks.
In `@src/app/api/contact/route.ts`:
- Around line 71-79: The handler currently returns { ok: true } and logs the
full submission when email delivery is skipped; change this so skipped-delivery
only returns a non-success or explicit "skipped" response in non-production
(check process.env.NODE_ENV !== 'production' or use a dedicated env like
NEXT_PUBLIC_ALLOW_NO_MAIL) and do not acknowledge success to callers in
production. Replace console.info/console.warn logging of the raw data with a
non-PII debug log (e.g., log only that delivery was skipped and minimal
identifiers), and gate the demo/noop branch behind a dev/sandbox flag so
functions/variables apiToken, toEmail, useSandbox, inboxId and the
NextResponse.json call reflect a failure/skipped response in production rather
than { ok: true }.
- Around line 92-112: The Mailtrap fetch can reject or hang causing the route to
throw or timeout; wrap the outbound fetch(url, { ... }) call in a try/catch and
use an AbortController with a short timeout (e.g., a few seconds) to abort long
requests, then handle non-ok responses as you already do. Specifically, create
an AbortController, pass controller.signal to fetch, set a timer that calls
controller.abort(), await the fetch inside try, and on any catch (including
abort) log the error (console.error with the thrown error) and return the same
NextResponse.json({ error: "Failed to send email. Please try again." }, {
status: 500 }). Keep the existing handling for mailtrapRes.ok and parsing
errBody when responses come back.
- Around line 14-45: The buildEmailHtml function currently interpolates raw user
input into the email markup; update it to HTML-escape every dynamic value before
constructing tableRows (e.g., create or use a helper like escapeHtml and apply
it to data.name, data.email, data.company, data.role, data.phone,
data.projectSize, data.message while preserving the existing nullish "—"
fallbacks) so that tableRows uses escaped values and prevents injection/phishing
risks.
In `@src/components/landing/ContactModal.tsx`:
- Around line 291-297: The dialog is rendered with z-50 (Dialog.Portal /
Dialog.Overlay / Dialog.Content) which puts it behind the landing mobile drawer
that uses z-[98]/z-[99]; update the modal stacking to a higher z-index so it
appears above the mobile drawer (for example change z-50 to z-[100] or another
value >99) in the Dialog.Overlay and Dialog.Content className strings to ensure
the modal is on top on mobile.
---
Nitpick comments:
In `@src/components/landing/ContactModal.tsx`:
- Around line 328-344: DemoRequestCTA silently no-ops when used outside its
provider; update the component (DemoRequestCTA) to fail fast by checking the
value from useContext(DemoModalCtx) and throwing or logging a clear error if
it's null/undefined (e.g., "DemoRequestCTA must be used within
DemoModalProvider"), and replace the optional chaining onClick
(ctx?.openModal()) with a direct call (ctx.openModal()) after the null check so
the component either works or immediately surfaces the missing
DemoModalProvider.
In `@src/components/landing/LandingPage.tsx`:
- Around line 45-49: Adjust JSX indentation so the <div id="landing-root"> is
indented as a child of <DemoModalProvider> for readability; locate the opening
<DemoModalProvider> in LandingPage.tsx and re-indent the nested <div
id="landing-root" ...> (and its closing tag) to be visually nested under
DemoModalProvider.
In `@src/hooks/lookahead/api.ts`:
- Around line 47-49: deleteProgrammeVersion currently hardcodes the endpoint
string `/programmes/${uploadId}` instead of using the shared key builder; add a
delete route in the lookaheadKeys key builder (e.g., a method or property like
lookaheadKeys.deleteProgramme or lookaheadKeys.programme.delete) that accepts
uploadId and returns the path, then update deleteProgrammeVersion to call
api.delete(lookaheadKeys.deleteProgramme(uploadId)) so all lookahead endpoints
are centralized and consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 07e5e887-2eb4-4fe2-9b05-94f0e0dd396b
📒 Files selected for processing (15)
src/app/(dashboard)/lookahead/_components/LookaheadDashboard.tsxsrc/app/(dashboard)/lookahead/page.tsxsrc/app/api/contact/route.tssrc/components/SideNav.tsxsrc/components/landing/ClientDynamics.tsxsrc/components/landing/ContactModal.tsxsrc/components/landing/LandingPage.tsxsrc/components/landing/ScrollAnimations.tsxsrc/components/landing/ShowcaseSection.tsxsrc/components/landing/TopBar.tsxsrc/hooks/lookahead/api.tssrc/hooks/lookahead/keys.tssrc/hooks/lookahead/useLookaheadData.tssrc/stores/uiIntentStore.tssrc/types/index.ts
Comment on lines
+71
to
+79
| if (!apiToken || !toEmail) { | ||
| console.info("[contact] Demo request received (no Mailtrap token configured):", data); | ||
| return NextResponse.json({ ok: true }); | ||
| } | ||
|
|
||
| if (useSandbox && !inboxId) { | ||
| console.warn("[contact] MAILTRAP_USE_SANDBOX=true but MAILTRAP_INBOX_ID not set"); | ||
| return NextResponse.json({ ok: true }); | ||
| } |
There was a problem hiding this comment.
Don't return success when the email was never sent.
The branches on Lines 71-79 acknowledge the request with { ok: true } even though delivery is skipped, so the UI shows success while the lead is silently dropped. Line 72 also logs the full submission object, which includes PII. At minimum, gate this noop behavior to non-production and stop logging the raw payload.
🚨 Suggested fix
+ const allowNoop = process.env.NODE_ENV !== "production";
+
if (!apiToken || !toEmail) {
- console.info("[contact] Demo request received (no Mailtrap token configured):", data);
- return NextResponse.json({ ok: true });
+ console.error("[contact] Mail delivery is not configured");
+ return NextResponse.json(
+ allowNoop
+ ? { ok: true }
+ : { error: "Demo requests are temporarily unavailable. Please try again later." },
+ { status: allowNoop ? 200 : 503 },
+ );
}
if (useSandbox && !inboxId) {
- console.warn("[contact] MAILTRAP_USE_SANDBOX=true but MAILTRAP_INBOX_ID not set");
- return NextResponse.json({ ok: true });
+ console.error("[contact] Mailtrap sandbox is enabled but inbox ID is missing");
+ return NextResponse.json(
+ allowNoop
+ ? { ok: true }
+ : { error: "Demo requests are temporarily unavailable. Please try again later." },
+ { status: allowNoop ? 200 : 503 },
+ );
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/api/contact/route.ts` around lines 71 - 79, The handler currently
returns { ok: true } and logs the full submission when email delivery is
skipped; change this so skipped-delivery only returns a non-success or explicit
"skipped" response in non-production (check process.env.NODE_ENV !==
'production' or use a dedicated env like NEXT_PUBLIC_ALLOW_NO_MAIL) and do not
acknowledge success to callers in production. Replace console.info/console.warn
logging of the raw data with a non-PII debug log (e.g., log only that delivery
was skipped and minimal identifiers), and gate the demo/noop branch behind a
dev/sandbox flag so functions/variables apiToken, toEmail, useSandbox, inboxId
and the NextResponse.json call reflect a failure/skipped response in production
rather than { ok: true }.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(dashboard)/lookahead/_components/LookaheadDashboard.tsx:
- Line 131: The dismissedAlerts Set (state variable dismissedAlerts and updater
setDismissedAlerts) currently persists across uploads and hides fresh anomalies;
reset it whenever a new upload arrives by clearing dismissedAlerts when
latestVersion?.upload_id changes (e.g., add a useEffect that calls
setDismissedAlerts(new Set()) with [latestVersion?.upload_id] as dependency), or
alternatively include upload_id in the keys stored in dismissedAlerts so
dismissal is scoped per-upload; update the logic that reads dismissedAlerts
accordingly (references: dismissedAlerts, setDismissedAlerts, latestVersion).
- Around line 189-194: The polling cleanup currently only clears the interval in
stopPolling(), but in-flight async calls from startPolling() (e.g.,
fetchUploadStatus()) can still resolve after handleProjectSelect() switches
projects and then call setUploadPhase()/mutate*() for the wrong project; fix
this by adding a cancellation mechanism: store an AbortController (or a
per-polling "generation" id) in a ref (e.g., pollingAbortRef or
pollingGenerationRef) when startPolling() begins each cycle, pass its signal (or
capture the generation) into fetchUploadStatus(), and in stopPolling() /
handleProjectSelect() call abort() (or advance the generation) so any resolved
promises ignore results for stale projects before calling setUploadPhase(),
mutateUploads(), mutateArtifacts(), etc.; update fetchUploadStatus() and any
callers to accept and respect the abort signal or generation check.
- Around line 163-169: The effect only sets windowSizeLocal when
useUIIntentStore.getState().getLookaheadIntent(uiScopeKey) returns a
persisted.windowSize, so switching to a scope with no saved intent leaves the
previous window active; update the effect (which depends on hasUIIntentHydrated
and uiScopeKey) to explicitly reset windowSizeLocal when persisted is missing
(e.g., call setWindowSizeLocal to the component's default/initial value or
undefined) so the UI falls back to the default when
getLookaheadIntent(uiScopeKey) returns nothing.
In `@src/components/landing/ContactModal.tsx`:
- Around line 293-297: The transition is targeting "scale" which doesn't animate
because Tailwind sets scale via the transform property; in Dialog.Content update
the inline style transition string (the one currently containing "scale 300ms
cubic-bezier(0.16,1,0.3,1)") to animate "transform" instead (e.g., "transform
300ms ...") so the data-[state=open]/data-[state=closed] scale classes smoothly
interpolate; refer to the Dialog.Content element and its inline style and the
data-[state=...] scale classes when making the change.
- Around line 41-67: The Field component renders a label and error but doesn't
associate them with the actual control; update Field to accept an id (string)
prop and use it to set label's htmlFor and to generate an error id (e.g.,
`${id}-error`) that is applied to the error <p> as its id; then ensure callers
give the inner input/select/textarea that same id and set aria-invalid={!!error}
and aria-describedby={error ? `${id}-error` : undefined}. Reference the Field
component and the error element id pattern when making the changes so labels and
errors are properly linked to their controls.
- Around line 77-80: The fields state is initialized empty which makes required
keys undefined and causes Zod to emit generic type errors; update initialization
or normalization so required ContactFields keys start as empty strings (e.g.,
initialize useState<Partial<ContactFields>> with { name: "", email: "", message:
"" }) or, alternatively, before calling zodSchema.safeParse, map any
missing/undefined ContactFields keys on the current fields object to "" so the
schema runs your custom validations and returns user-friendly messages; adjust
any related form handlers that assume empty->undefined (references: fields,
setFields, ContactFields, errors, setErrors, and the safeParse call).
In `@src/hooks/lookahead/api.ts`:
- Around line 73-79: The error handling when response.ok is false only inspects
payload.detail and falls back to a generic message; change it to prefer
payload.message (if a string) first, then payload.detail (if a string), and only
then the generic "Upload failed. Please try again." Update the branch that
builds detail (around the response.json() result and the const detail assignment
in src/hooks/lookahead/api.ts) so it checks typeof payload?.message === "string"
? payload.message : typeof payload?.detail === "string" ? payload.detail :
"Upload failed. Please try again.", then throw the Error(detail).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46966eda-06db-40a9-b81a-f340f04be115
📒 Files selected for processing (5)
src/app/(dashboard)/lookahead/_components/LookaheadDashboard.tsxsrc/app/api/contact/route.tssrc/components/landing/ContactModal.tsxsrc/hooks/lookahead/api.tssrc/hooks/lookahead/keys.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks/lookahead/keys.ts
- src/app/api/contact/route.ts
Comment on lines
+293
to
+297
| <Dialog.Content | ||
| className="fixed left-1/2 top-1/2 z-[100] w-full max-w-2xl -translate-x-1/2 -translate-y-1/2 rounded-2xl border border-white/10 bg-[#0a0a14] p-6 shadow-2xl max-h-[90vh] overflow-y-auto data-[state=closed]:opacity-0 data-[state=open]:opacity-100 data-[state=open]:scale-100 data-[state=closed]:scale-[0.93]" | ||
| style={{ | ||
| transition: "opacity 220ms ease, scale 300ms cubic-bezier(0.16,1,0.3,1)", | ||
| }} |
There was a problem hiding this comment.
Animate transform, not scale.
The open/closed classes here change Tailwind's transform output, so transition: "... scale 300ms ..." will not animate the zoom. The dialog will snap between scales.
🎞️ Proposed fix
style={{
- transition: "opacity 220ms ease, scale 300ms cubic-bezier(0.16,1,0.3,1)",
+ transition: "opacity 220ms ease, transform 300ms cubic-bezier(0.16,1,0.3,1)",
}}📝 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
| <Dialog.Content | |
| className="fixed left-1/2 top-1/2 z-[100] w-full max-w-2xl -translate-x-1/2 -translate-y-1/2 rounded-2xl border border-white/10 bg-[#0a0a14] p-6 shadow-2xl max-h-[90vh] overflow-y-auto data-[state=closed]:opacity-0 data-[state=open]:opacity-100 data-[state=open]:scale-100 data-[state=closed]:scale-[0.93]" | |
| style={{ | |
| transition: "opacity 220ms ease, scale 300ms cubic-bezier(0.16,1,0.3,1)", | |
| }} | |
| <Dialog.Content | |
| className="fixed left-1/2 top-1/2 z-[100] w-full max-w-2xl -translate-x-1/2 -translate-y-1/2 rounded-2xl border border-white/10 bg-[`#0a0a14`] p-6 shadow-2xl max-h-[90vh] overflow-y-auto data-[state=closed]:opacity-0 data-[state=open]:opacity-100 data-[state=open]:scale-100 data-[state=closed]:scale-[0.93]" | |
| style={{ | |
| transition: "opacity 220ms ease, transform 300ms cubic-bezier(0.16,1,0.3,1)", | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/landing/ContactModal.tsx` around lines 293 - 297, The
transition is targeting "scale" which doesn't animate because Tailwind sets
scale via the transform property; in Dialog.Content update the inline style
transition string (the one currently containing "scale 300ms
cubic-bezier(0.16,1,0.3,1)") to animate "transform" instead (e.g., "transform
300ms ...") so the data-[state=open]/data-[state=closed] scale classes smoothly
interpolate; refer to the Dialog.Content element and its inline style and the
data-[state=...] scale classes when making the change.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/(dashboard)/lookahead/_components/LookaheadDashboard.tsx (1)
277-284:⚠️ Potential issue | 🟡 MinorStop active polling when switching projects.
While the generation-based invalidation correctly prevents stale state updates,
handleProjectSelectdoesn't callstopPolling(). If polling is active when switching projects, the interval continues making API calls for the old project until timeout, which is wasteful and could show a confusing timeout error.🛡️ Proposed fix
const handleProjectSelect = (proj: ApiProject) => { if (!proj?.id) return; + stopPolling(); setShowProjectSelector(false); setProjectId(proj.id); setUploadPhase({ kind: "idle" }); setDismissedAlerts(new Set()); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(dashboard)/lookahead/_components/LookaheadDashboard.tsx around lines 277 - 284, handleProjectSelect currently switches project state but never calls stopPolling(), so any active polling for the previous project continues; modify the handleProjectSelect function to call stopPolling() (if available) before or immediately after hiding the selector and before changing project state (i.e., call stopPolling() alongside setShowProjectSelector(false), then setProjectId(proj.id), setUploadPhase({ kind: "idle" }) and setDismissedAlerts(new Set()) ) so background intervals are cancelled when switching projects.
🧹 Nitpick comments (2)
src/app/api/contact/route.ts (2)
73-79: Consider documenting the sandbox-default behavior.
useSandboxdefaults totruewhenMAILTRAP_USE_SANDBOXis unset. If a production deployment setsMAILTRAP_INBOX_IDbut forgets to setMAILTRAP_USE_SANDBOX=false, emails will silently route to the sandbox inbox instead of actual delivery. Consider either:
- Adding a startup log warning when
NODE_ENV=productionanduseSandbox=true- Documenting required environment variables in a README or
.env.example🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/contact/route.ts` around lines 73 - 79, The code currently defaults useSandbox to true when MAILTRAP_USE_SANDBOX is unset which can silently route production mail to the sandbox; update the startup flow to detect this and either log a clear warning or fail-fast: when process.env.NODE_ENV === "production" and the useSandbox variable (computed from MAILTRAP_USE_SANDBOX) is true while MAILTRAP_INBOX_ID is present, emit a prominent processLogger.warn (or processLogger.error and exit) explaining that MAILTRAP_USE_SANDBOX is unset/true in production, and add/update documentation (.env.example or README) to list MAILTRAP_USE_SANDBOX and MAILTRAP_INBOX_ID requirements so operators must explicitly set MAILTRAP_USE_SANDBOX=false for real deliveries.
4-12: Consider adding max length constraints to prevent abuse.The schema validates minimum lengths but has no upper bounds. Malicious users could submit extremely large values, potentially causing issues with email generation or downstream services.
🛡️ Suggested fix
const contactSchema = z.object({ - name: z.string().min(2), - email: z.string().email(), - company: z.string().min(1), - role: z.string().min(1), - phone: z.string().optional(), - projectSize: z.string().optional(), - message: z.string().optional(), + name: z.string().min(2).max(100), + email: z.string().email().max(254), + company: z.string().min(1).max(200), + role: z.string().min(1).max(100), + phone: z.string().max(30).optional(), + projectSize: z.string().max(100).optional(), + message: z.string().max(5000).optional(), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/contact/route.ts` around lines 4 - 12, contactSchema currently enforces minimums but no upper bounds; add z.string().max(...) constraints to each field to prevent extremely large inputs (e.g., name → max 100, email → max 254, company → max 100, role → max 100, phone → max 20, projectSize → max 50, message → max 2000) by updating the contactSchema definition (the named symbol contactSchema and its properties name, email, company, role, phone, projectSize, message) to use z.string().max(n) where appropriate and keep optional() on optional fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(dashboard)/lookahead/_components/LookaheadDashboard.tsx:
- Around line 528-556: The file input's value isn't cleared before
programmatically opening the file dialog, so selecting the same file twice won't
trigger onChange; update the Button's onClick handler (where
fileInputRef.current?.click() is called) to first clear the input value on
fileInputRef.current (e.g., reset .value or set to null) and then call .click(),
ensuring handleFileSelected still handles the selected file from the input's
onChange.
---
Duplicate comments:
In `@src/app/`(dashboard)/lookahead/_components/LookaheadDashboard.tsx:
- Around line 277-284: handleProjectSelect currently switches project state but
never calls stopPolling(), so any active polling for the previous project
continues; modify the handleProjectSelect function to call stopPolling() (if
available) before or immediately after hiding the selector and before changing
project state (i.e., call stopPolling() alongside setShowProjectSelector(false),
then setProjectId(proj.id), setUploadPhase({ kind: "idle" }) and
setDismissedAlerts(new Set()) ) so background intervals are cancelled when
switching projects.
---
Nitpick comments:
In `@src/app/api/contact/route.ts`:
- Around line 73-79: The code currently defaults useSandbox to true when
MAILTRAP_USE_SANDBOX is unset which can silently route production mail to the
sandbox; update the startup flow to detect this and either log a clear warning
or fail-fast: when process.env.NODE_ENV === "production" and the useSandbox
variable (computed from MAILTRAP_USE_SANDBOX) is true while MAILTRAP_INBOX_ID is
present, emit a prominent processLogger.warn (or processLogger.error and exit)
explaining that MAILTRAP_USE_SANDBOX is unset/true in production, and add/update
documentation (.env.example or README) to list MAILTRAP_USE_SANDBOX and
MAILTRAP_INBOX_ID requirements so operators must explicitly set
MAILTRAP_USE_SANDBOX=false for real deliveries.
- Around line 4-12: contactSchema currently enforces minimums but no upper
bounds; add z.string().max(...) constraints to each field to prevent extremely
large inputs (e.g., name → max 100, email → max 254, company → max 100, role →
max 100, phone → max 20, projectSize → max 50, message → max 2000) by updating
the contactSchema definition (the named symbol contactSchema and its properties
name, email, company, role, phone, projectSize, message) to use
z.string().max(n) where appropriate and keep optional() on optional fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d9865cd-c738-408d-981c-9a0ef3f776a8
📒 Files selected for processing (4)
src/app/(dashboard)/lookahead/_components/LookaheadDashboard.tsxsrc/app/api/contact/route.tssrc/components/landing/ContactModal.tsxsrc/hooks/lookahead/api.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks/lookahead/api.ts
- src/components/landing/ContactModal.tsx
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This was referenced Mar 27, 2026
This was referenced May 6, 2026
This was referenced May 26, 2026
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
1 participant
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
Updates & Improvements