Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a two-step support-ticket flow (requestSupportTicket → createSupportTicket), a multi-file upload UI and server upload endpoint, persists per-user chat state, threads Changes
Sequence DiagramsequenceDiagram
participant User
participant ChatUI as Chat UI
participant BrowserStorage as Browser Storage
participant Server as API Server
participant Plain as Plain Service
User->>ChatUI: Request support ticket
ChatUI->>ChatUI: Invoke requestSupportTicket tool (render upload form)
User->>ChatUI: Select files & enter ticketDetails
ChatUI->>Server: POST /api/ai/support-chat/upload (fileName, fileSize)
Server->>Plain: Create attachment upload URL (customerId, file meta)
Plain-->>Server: Return attachmentId, uploadFormUrl, uploadFormData
Server-->>ChatUI: Return upload form data
ChatUI->>Plain: Upload file(s) to uploadFormUrl (pre-signed)
Plain-->>ChatUI: Confirm uploaded attachment(s)
ChatUI->>BrowserStorage: Persist attachmentIds & messages (per-user key)
User->>ChatUI: Submit ticket (attachmentIds, ticketDetails)
ChatUI->>Server: POST /api/ai/support-chat (ticket payload)
Server->>Plain: createSupportTicket (attachments + details)
Plain-->>Server: Ticket created
Server-->>ChatUI: Success
ChatUI->>User: Show confirmation
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 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. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/lib/ai/create-support-ticket.ts (1)
6-6:⚠️ Potential issue | 🟡 MinorSwitch to
zod/v4namespace import for consistency.The codebase predominantly uses
import * as z from "zod/v4"acrossapps/web. Update this file to match the pattern.♻️ Proposed fix
-import { z } from "zod"; +import * as z from "zod/v4";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/ai/create-support-ticket.ts` at line 6, Replace the current named import "import { z } from 'zod';" with the project-consistent namespace import "import * as z from 'zod/v4'": update the import at the top of create-support-ticket.ts so all references to z (e.g., z.object, z.string) continue to work under the namespace import and match the rest of the codebase.apps/web/lib/ai/upsert-docs-embedding.ts (1)
245-254:⚠️ Potential issue | 🟠 MajorLogic error in URL validation: pathname compared against hostnames.
Line 248 compares
parsedUrl.pathnameagainstALLOWED_HOSTNAMES, which will never match (e.g.,/docs/apiis not in["dub.co", "www.dub.co"]). This creates a confusing condition that relies on the subsequent path prefix check to pass valid URLs.🔧 Suggested fix - remove the erroneous hostname check on pathname
if ( parsedUrl.protocol !== "https:" || !ALLOWED_HOSTNAMES.includes(parsedUrl.hostname) || - (!ALLOWED_HOSTNAMES.includes(parsedUrl.pathname) && - !ALLOWED_PATH_PREFIXES.some((p) => parsedUrl.pathname.startsWith(p))) || + !ALLOWED_PATH_PREFIXES.some((p) => parsedUrl.pathname.startsWith(p)) || parsedUrl.pathname.includes("..") ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/ai/upsert-docs-embedding.ts` around lines 245 - 254, The URL validation incorrectly compares parsedUrl.pathname to ALLOWED_HOSTNAMES; update the conditional in the validation block so it checks: parsedUrl.protocol === "https:", parsedUrl.hostname is in ALLOWED_HOSTNAMES, parsedUrl.pathname does not include "..", and parsedUrl.pathname either is allowed via ALLOWED_PATH_PREFIXES (use .some(p => parsedUrl.pathname.startsWith(p))) or any other intended path check — i.e., remove the erroneous (!ALLOWED_HOSTNAMES.includes(parsedUrl.pathname)) clause and ensure the if-statement uses ALLOWED_HOSTNAMES for hostname checks and ALLOWED_PATH_PREFIXES for pathname checks (referencing parsedUrl, ALLOWED_HOSTNAMES, ALLOWED_PATH_PREFIXES).
🧹 Nitpick comments (5)
apps/web/lib/ai/request-support-ticket.ts (1)
2-2: Use the repo-standard Zod import (zod/v4namespace).The
apps/webcodebase has adoptedimport * as z from "zod/v4"as the standard convention across 578 files. Line 2 should follow this pattern for consistency.♻️ Proposed fix
-import { z } from "zod"; +import * as z from "zod/v4";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/ai/request-support-ticket.ts` at line 2, Replace the current Zod import with the repo-standard namespace import: update the import used at the top of this module (where `z` is currently imported) to use the `zod/v4` namespace pattern (importing as `* as z`) so this file matches the project's convention (affects the import statement referencing `z` in this module).apps/web/app/api/ai/sync-embeddings/route.ts (1)
53-54: Consider caching or optimizing the Plausible pageviews fetch for single-article syncs.Every POST request fetches the complete pageviews map (up to 10,000 pages) from Plausible, even though only one article's pageviews are needed. For frequent webhook triggers, this could:
- Hit Plausible API rate limits
- Add unnecessary latency to each sync
Consider either caching the pageviews map with a TTL, or passing only the specific pathname to a lighter-weight fetch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/ai/sync-embeddings/route.ts` around lines 53 - 54, The handler currently calls fetchPlausiblePageviews() for every POST and then passes the full pageviews map into upsertDocsEmbeddings(normalizedUrl, pageviewsMap), causing unnecessary work and potential rate limits; change this by either (A) adding a TTL-cached wrapper around fetchPlausiblePageviews() so repeated calls return a cached map for a configurable duration, or (B) creating a new lighter-weight function (e.g., fetchPlausiblePageviewsForPathname) that accepts the single article pathname derived from normalizedUrl and only requests that metric from Plausible, then update the route to call the cache or the pathname-specific fetch and pass its result into upsertDocsEmbeddings instead of always fetching the full map.apps/web/lib/ai/upsert-docs-embedding.ts (1)
228-232: Incomplete JSDoc forpageviewsMapparameter.The
@param pageviewsMapdescription is empty. Consider documenting what the map contains and how it's used.📝 Suggested documentation
* - * `@param` pageviewsMap - * - * + * `@param` pageviewsMap Optional map of pathname → pageview count from Plausible analytics. + * Used to enrich chunk metadata with popularity data. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/ai/upsert-docs-embedding.ts` around lines 228 - 232, The JSDoc for the pageviewsMap parameter is missing—update the comment for the function in apps/web/lib/ai/upsert-docs-embedding.ts to document pageviewsMap: state its type (e.g., Map or Record), the key and value shapes (for example keys are doc IDs/URLs and values are pageview counts or objects with {views, lastViewed}), and how the function uses it (e.g., to weight embeddings, filter low-traffic docs, or attach analytics metadata); reference the pageviewsMap symbol so reviewers can verify the description matches the code behavior in the upsert embedding routine.apps/web/ui/support/chat-interface.tsx (2)
106-130: Consider consolidating localStorage persistence into a single effect.Two separate effects independently read, modify, and write to localStorage. While React's batching likely prevents race conditions, consolidating into one effect would be cleaner and more efficient.
♻️ Suggested consolidation
- useEffect(() => { - if (!storageKey || !restoredRef.current) return; - - try { - const raw = localStorage.getItem(storageKey); - const stored = raw ? JSON.parse(raw) : {}; - localStorage.setItem( - storageKey, - JSON.stringify({ ...stored, selection }), - ); - } catch { } - }, [selection, storageKey]); - - useEffect(() => { - if (!storageKey || !restoredRef.current || status === "streaming") return; - - try { - const raw = localStorage.getItem(storageKey); - const stored = raw ? JSON.parse(raw) : {}; - localStorage.setItem( - storageKey, - JSON.stringify({ ...stored, messages, ticketSubmitted }), - ); - } catch { } - }, [messages, ticketSubmitted, status, storageKey]); + useEffect(() => { + if (!storageKey || !restoredRef.current || status === "streaming") return; + + try { + localStorage.setItem( + storageKey, + JSON.stringify({ selection, messages, ticketSubmitted }), + ); + } catch { } + }, [selection, messages, ticketSubmitted, status, storageKey]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/support/chat-interface.tsx` around lines 106 - 130, Multiple useEffect hooks read/modify/write the same localStorage key; consolidate them into a single effect that reads the existing stored object once and writes an updated object containing selection, messages, and ticketSubmitted, while skipping updates if !storageKey, !restoredRef.current, or status === "streaming". In the single effect (replace the two useEffect blocks), perform one try/catch around JSON.parse(localStorage.getItem(storageKey)) to get stored (default {}), then setItem with JSON.stringify({ ...stored, selection, messages, ticketSubmitted }); ensure the effect's dependency array includes selection, messages, ticketSubmitted, status, and storageKey so updates trigger correctly and preserve existing stored fields.
91-104: Session restoration lacks schema validation.The restored data from localStorage is used directly without validating its structure. Malformed or outdated data could cause runtime errors or unexpected behavior.
🛡️ Suggested validation
try { const raw = localStorage.getItem(storageKey); if (!raw) return; const stored = JSON.parse(raw); - if (stored.selection) setSelection(stored.selection); - if (stored.messages?.length) setMessages(stored.messages); - if (stored.ticketSubmitted) setTicketSubmitted(true); + if (stored.selection && typeof stored.selection === "object") { + setSelection(stored.selection); + } + if (Array.isArray(stored.messages) && stored.messages.length) { + setMessages(stored.messages); + } + if (stored.ticketSubmitted === true) { + setTicketSubmitted(true); + } } catch { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/support/chat-interface.tsx` around lines 91 - 104, The restoration useEffect reads JSON from localStorage and applies it directly (storageKey, restoredRef, localStorage.getItem, setSelection, setMessages, setTicketSubmitted); add explicit schema validation after JSON.parse to ensure the object has the expected shapes/types (e.g., selection is the expected type, messages is an array of message objects with required fields, ticketSubmitted is boolean) before calling setSelection/setMessages/setTicketSubmitted, and if validation fails discard the data and optionally clear the key or log a warning; keep the existing try/catch but perform these type checks (or use a small schema helper) to avoid applying malformed or outdated data.
🤖 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/web/app/api/ai/support-chat/route.ts`:
- Around line 16-22: Add runtime validation immediately after parsing req.json()
in the route handler: verify attachmentIds is either undefined or an array of
strings, enforce a max count (e.g., N items) and per-id length limit, and verify
ticketDetails is either undefined or a short string under a max length; if any
check fails return a 400/Bad Request early. Update the code that uses
attachmentIds and ticketDetails (references: attachmentIds, ticketDetails,
messages, globalContext, and the parsed payload from req.json()) to rely on
these guards so malformed or oversized payloads are rejected before any tool
invocation.
In `@apps/web/app/api/ai/support-chat/upload/route.ts`:
- Around line 23-27: The server currently only checks fileName and fileSizeBytes
superficially; add strict backend validation in the upload handler (route.ts) to
enforce the UI limits and accepted formats by: define constants (e.g.,
MAX_UPLOAD_SIZE_BYTES and ACCEPTED_FILE_TYPES) and reject requests where
fileSizeBytes > MAX_UPLOAD_SIZE_BYTES or <= 0; validate file type by checking
the provided content-type header and/or the fileName extension against
ACCEPTED_FILE_TYPES and, if possible, sniff the uploaded bytes (magic bytes)
before processing; respond with clear 400 errors when size or type validation
fails and ensure any downstream processing (the function handling the file
upload) only runs after these checks pass.
In `@apps/web/lib/ai/create-support-ticket.ts`:
- Around line 51-64: ticketDetails is added directly into the ticket components
and can be unbounded; truncate and/or sanitize it before constructing
componentText to avoid huge payloads. In create-support-ticket (the block that
builds componentText/componentDivider), compute a boundedTicketDetails =
ticketDetails?.slice(0, <SAFE_LIMIT>) (and append "… (truncated)" if truncated)
and only include the componentText/componentDivider when boundedTicketDetails is
non-empty; this keeps the symbols componentText, componentDivider and
ComponentDividerSpacingSize.M the same but ensures the ticket payload is
size-limited and reliable.
In `@apps/web/lib/ai/upsert-docs-embedding.ts`:
- Around line 169-176: Replace the hardcoded site_id "pedroladeira.com" in the
request body inside apps/web/lib/ai/upsert-docs-embedding.ts with a configurable
value: read from an environment variable (e.g. process.env.SITE_ID) falling back
to the production site string "dub.co"; locate the JSON body construction where
site_id is set (the object with metrics/date_range/dimensions/pagination) and
update it to use the env var fallback so the production site is used by default
and developer overrides remain possible.
In `@apps/web/ui/support/ticket-upload.tsx`:
- Around line 194-223: The dropzone div is mouse-only; make it
keyboard-accessible by adding tabIndex={0}, role="button", and an onKeyDown
handler that triggers the same action as onClick (call
fileInputRef.current?.click()) when Enter or Space is pressed (ensure default is
prevented for Space) and keep existing focus/visual styles via
isDragging/setIsDragging; update the JSX for the div that contains
onDragOver/onDragLeave/onDrop/onClick to include these attributes and use the
same addFiles/handleDrop logic and ACCEPTED_TYPES validation.
- Around line 95-123: The bug is that entries are created for all dropped files
and uploaded regardless of the MAX_FILES cap; to fix, limit which entries are
both added to state and uploaded by computing the allowed slice against the
current files in state inside the setFiles callback: in the setFiles updater use
prev to create combined = [...prev, ...entries].slice(0, MAX_FILES) and also
compute allowedEntries = combined.slice(prev.length) (or compute allowedCount =
MAX_FILES - prev.length and take entries.slice(0, allowedCount)); then only call
uploadToPlain for those allowedEntries. Update the entries.forEach upload loop
to iterate over allowedEntries and keep using uploadToPlain, setFiles, entry.id,
and result handling as before.
---
Outside diff comments:
In `@apps/web/lib/ai/create-support-ticket.ts`:
- Line 6: Replace the current named import "import { z } from 'zod';" with the
project-consistent namespace import "import * as z from 'zod/v4'": update the
import at the top of create-support-ticket.ts so all references to z (e.g.,
z.object, z.string) continue to work under the namespace import and match the
rest of the codebase.
In `@apps/web/lib/ai/upsert-docs-embedding.ts`:
- Around line 245-254: The URL validation incorrectly compares
parsedUrl.pathname to ALLOWED_HOSTNAMES; update the conditional in the
validation block so it checks: parsedUrl.protocol === "https:",
parsedUrl.hostname is in ALLOWED_HOSTNAMES, parsedUrl.pathname does not include
"..", and parsedUrl.pathname either is allowed via ALLOWED_PATH_PREFIXES (use
.some(p => parsedUrl.pathname.startsWith(p))) or any other intended path check —
i.e., remove the erroneous (!ALLOWED_HOSTNAMES.includes(parsedUrl.pathname))
clause and ensure the if-statement uses ALLOWED_HOSTNAMES for hostname checks
and ALLOWED_PATH_PREFIXES for pathname checks (referencing parsedUrl,
ALLOWED_HOSTNAMES, ALLOWED_PATH_PREFIXES).
---
Nitpick comments:
In `@apps/web/app/api/ai/sync-embeddings/route.ts`:
- Around line 53-54: The handler currently calls fetchPlausiblePageviews() for
every POST and then passes the full pageviews map into
upsertDocsEmbeddings(normalizedUrl, pageviewsMap), causing unnecessary work and
potential rate limits; change this by either (A) adding a TTL-cached wrapper
around fetchPlausiblePageviews() so repeated calls return a cached map for a
configurable duration, or (B) creating a new lighter-weight function (e.g.,
fetchPlausiblePageviewsForPathname) that accepts the single article pathname
derived from normalizedUrl and only requests that metric from Plausible, then
update the route to call the cache or the pathname-specific fetch and pass its
result into upsertDocsEmbeddings instead of always fetching the full map.
In `@apps/web/lib/ai/request-support-ticket.ts`:
- Line 2: Replace the current Zod import with the repo-standard namespace
import: update the import used at the top of this module (where `z` is currently
imported) to use the `zod/v4` namespace pattern (importing as `* as z`) so this
file matches the project's convention (affects the import statement referencing
`z` in this module).
In `@apps/web/lib/ai/upsert-docs-embedding.ts`:
- Around line 228-232: The JSDoc for the pageviewsMap parameter is
missing—update the comment for the function in
apps/web/lib/ai/upsert-docs-embedding.ts to document pageviewsMap: state its
type (e.g., Map or Record), the key and value shapes (for example keys are doc
IDs/URLs and values are pageview counts or objects with {views, lastViewed}),
and how the function uses it (e.g., to weight embeddings, filter low-traffic
docs, or attach analytics metadata); reference the pageviewsMap symbol so
reviewers can verify the description matches the code behavior in the upsert
embedding routine.
In `@apps/web/ui/support/chat-interface.tsx`:
- Around line 106-130: Multiple useEffect hooks read/modify/write the same
localStorage key; consolidate them into a single effect that reads the existing
stored object once and writes an updated object containing selection, messages,
and ticketSubmitted, while skipping updates if !storageKey,
!restoredRef.current, or status === "streaming". In the single effect (replace
the two useEffect blocks), perform one try/catch around
JSON.parse(localStorage.getItem(storageKey)) to get stored (default {}), then
setItem with JSON.stringify({ ...stored, selection, messages, ticketSubmitted
}); ensure the effect's dependency array includes selection, messages,
ticketSubmitted, status, and storageKey so updates trigger correctly and
preserve existing stored fields.
- Around line 91-104: The restoration useEffect reads JSON from localStorage and
applies it directly (storageKey, restoredRef, localStorage.getItem,
setSelection, setMessages, setTicketSubmitted); add explicit schema validation
after JSON.parse to ensure the object has the expected shapes/types (e.g.,
selection is the expected type, messages is an array of message objects with
required fields, ticketSubmitted is boolean) before calling
setSelection/setMessages/setTicketSubmitted, and if validation fails discard the
data and optionally clear the key or log a warning; keep the existing try/catch
but perform these type checks (or use a small schema helper) to avoid applying
malformed or outdated data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ef59df0b-41a7-4e53-95d6-4e0f18bfdf2f
📒 Files selected for processing (14)
apps/web/app/api/ai/support-chat/route.tsapps/web/app/api/ai/support-chat/upload/route.tsapps/web/app/api/ai/sync-embeddings/route.tsapps/web/lib/ai/build-system-prompt.tsapps/web/lib/ai/create-support-ticket.tsapps/web/lib/ai/request-support-ticket.tsapps/web/lib/ai/upsert-docs-embedding.tsapps/web/scripts/seed-support-embeddings.tsapps/web/ui/support/chat-bubble.tsxapps/web/ui/support/chat-interface.tsxapps/web/ui/support/clear-chat-button.tsxapps/web/ui/support/code-block.tsxapps/web/ui/support/embedded-chat.tsxapps/web/ui/support/ticket-upload.tsx
Collaborator
Author
|
@coderabbitai update the description with the following fixes too:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainThe PR description images show the Dub Support chat widget UI:
Let me fetch the current PR description first, then update it. 🏁 Script executed: Length of output: 1048 🏁 Script executed: Length of output: 476
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/web/app/api/ai/support-chat/upload/route.ts`:
- Around line 49-53: The current validation allows decimal fileSizeBytes; update
the upload route's size check to reject non-integer/NaN values by adding
Number.isInteger (and keeping the existing typeof/size bounds). Concretely, in
the route handler where fileSizeBytes and MAX_UPLOAD_SIZE_BYTES are used, change
the condition to also require Number.isInteger(fileSizeBytes) (or equivalently
check !Number.isInteger(fileSizeBytes) in the OR chain) so that decimal/NaN
values are rejected before creating upload URLs.
In `@apps/web/lib/ai/upsert-docs-embedding.ts`:
- Around line 249-250: The startsWith check is too permissive and allows routes
like "/docs-v2"; update the predicate that tests ALLOWED_PATH_PREFIXES (the
lambda using parsedUrl.pathname.startsWith) to only accept either an exact match
(parsedUrl.pathname === p) or a true nested subpath
(parsedUrl.pathname.startsWith(p + '/')). Modify the condition in
apps/web/lib/ai/upsert-docs-embedding.ts where ALLOWED_PATH_PREFIXES.some(...)
is used so it uses the combined exact-or-slash-prefixed check against
parsedUrl.pathname.
In `@apps/web/ui/support/ticket-upload.tsx`:
- Around line 76-77: The component currently only gates clicks by upload state,
allowing concurrent ticket creation if the user rapidly re-clicks; fix by
introducing an explicit in-flight guard (e.g., isSubmitting state or useRef
isSubmittingRef) and checking it at the top of the local submit handler (the
function that calls the onSubmit prop), returning early if already true; set
isSubmitting=true before awaiting props.onSubmit(attachmentIds, details) and
reset it in a finally block so duplicate requests are prevented and the UI
button remains disabled while the promise is pending; ensure you update any
handlers that call onSubmit (the local submit handler referenced around onSubmit
and onCancel usage) to use this pattern and keep proper error handling/logging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 95e40656-9cd9-4e3e-9243-35d14a8ae307
📒 Files selected for processing (5)
apps/web/app/api/ai/support-chat/route.tsapps/web/app/api/ai/support-chat/upload/route.tsapps/web/lib/ai/create-support-ticket.tsapps/web/lib/ai/upsert-docs-embedding.tsapps/web/ui/support/ticket-upload.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/api/ai/support-chat/route.ts
Comment on lines
+49
to
+53
| if ( | ||
| typeof fileSizeBytes !== "number" || | ||
| fileSizeBytes <= 0 || | ||
| fileSizeBytes > MAX_UPLOAD_SIZE_BYTES | ||
| ) { |
Contributor
There was a problem hiding this comment.
Reject non-integer fileSizeBytes values.
The current check allows decimal values, but file sizes are byte counts and should be finite integers to avoid downstream upload-URL creation errors.
🔧 Proposed fix
if (
- typeof fileSizeBytes !== "number" ||
+ typeof fileSizeBytes !== "number" ||
+ !Number.isFinite(fileSizeBytes) ||
+ !Number.isInteger(fileSizeBytes) ||
fileSizeBytes <= 0 ||
fileSizeBytes > MAX_UPLOAD_SIZE_BYTES
) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/api/ai/support-chat/upload/route.ts` around lines 49 - 53, The
current validation allows decimal fileSizeBytes; update the upload route's size
check to reject non-integer/NaN values by adding Number.isInteger (and keeping
the existing typeof/size bounds). Concretely, in the route handler where
fileSizeBytes and MAX_UPLOAD_SIZE_BYTES are used, change the condition to also
require Number.isInteger(fileSizeBytes) (or equivalently check
!Number.isInteger(fileSizeBytes) in the OR chain) so that decimal/NaN values are
rejected before creating upload URLs.
Comment on lines
+249
to
250
| !ALLOWED_PATH_PREFIXES.some((p) => parsedUrl.pathname.startsWith(p)) | ||
| ) { |
Contributor
There was a problem hiding this comment.
Tighten path-prefix matching to avoid indexing unintended routes.
The current startsWith check accepts paths like /docs-v2; use exact prefix or nested subpath matching.
🔧 Proposed fix
+ const hasAllowedPathPrefix = ALLOWED_PATH_PREFIXES.some(
+ (p) => parsedUrl.pathname === p || parsedUrl.pathname.startsWith(`${p}/`),
+ );
+
if (
parsedUrl.protocol !== "https:" ||
!ALLOWED_HOSTNAMES.includes(parsedUrl.hostname) ||
parsedUrl.pathname.includes("..") ||
- !ALLOWED_PATH_PREFIXES.some((p) => parsedUrl.pathname.startsWith(p))
+ !hasAllowedPathPrefix
) {📝 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
| !ALLOWED_PATH_PREFIXES.some((p) => parsedUrl.pathname.startsWith(p)) | |
| ) { | |
| const hasAllowedPathPrefix = ALLOWED_PATH_PREFIXES.some( | |
| (p) => parsedUrl.pathname === p || parsedUrl.pathname.startsWith(`${p}/`), | |
| ); | |
| if ( | |
| parsedUrl.protocol !== "https:" || | |
| !ALLOWED_HOSTNAMES.includes(parsedUrl.hostname) || | |
| parsedUrl.pathname.includes("..") || | |
| !hasAllowedPathPrefix | |
| ) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/lib/ai/upsert-docs-embedding.ts` around lines 249 - 250, The
startsWith check is too permissive and allows routes like "/docs-v2"; update the
predicate that tests ALLOWED_PATH_PREFIXES (the lambda using
parsedUrl.pathname.startsWith) to only accept either an exact match
(parsedUrl.pathname === p) or a true nested subpath
(parsedUrl.pathname.startsWith(p + '/')). Modify the condition in
apps/web/lib/ai/upsert-docs-embedding.ts where ALLOWED_PATH_PREFIXES.some(...)
is used so it uses the combined exact-or-slash-prefixed check against
parsedUrl.pathname.
Comment on lines
+76
to
+77
| onSubmit: (attachmentIds: string[], details: string) => void; | ||
| onCancel?: () => void; |
Contributor
There was a problem hiding this comment.
Prevent duplicate ticket creation on rapid re-clicks.
Submission is only gated by upload state; repeated clicks while onSubmit is still running can fire multiple create-ticket requests.
🛡️ Proposed fix
export function TicketUpload({
onSubmit,
onCancel,
submitted = false,
}: {
- onSubmit: (attachmentIds: string[], details: string) => void;
+ onSubmit: (attachmentIds: string[], details: string) => void | Promise<void>;
onCancel?: () => void;
submitted?: boolean;
}) {
const [files, setFiles] = useState<FileEntry[]>([]);
const [isDragging, setIsDragging] = useState(false);
const [details, setDetails] = useState("");
+ const [isSubmitting, setIsSubmitting] = useState(false);
const fileInputRef = useRef<HTMLInputElement>(null);
const isUploading = files.some((f) => f.status === "uploading");
- const canSubmit = !isUploading;
+ const canSubmit = !isUploading && !isSubmitting;
@@
- const handleSubmit = () => {
+ const handleSubmit = async () => {
+ if (!canSubmit) return;
+ setIsSubmitting(true);
const attachmentIds = files
.filter((f) => f.status === "done" && f.attachmentId)
.map((f) => f.attachmentId!);
- onSubmit(attachmentIds, details.trim());
+ try {
+ await onSubmit(attachmentIds, details.trim());
+ } finally {
+ setIsSubmitting(false);
+ }
};
@@
- {isUploading ? "Uploading..." : "Submit ticket"}
+ {isUploading
+ ? "Uploading..."
+ : isSubmitting
+ ? "Submitting..."
+ : "Submit ticket"}
</button>
</div>Also applies to: 85-87, 137-142, 258-270
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/ui/support/ticket-upload.tsx` around lines 76 - 77, The component
currently only gates clicks by upload state, allowing concurrent ticket creation
if the user rapidly re-clicks; fix by introducing an explicit in-flight guard
(e.g., isSubmitting state or useRef isSubmittingRef) and checking it at the top
of the local submit handler (the function that calls the onSubmit prop),
returning early if already true; set isSubmitting=true before awaiting
props.onSubmit(attachmentIds, details) and reset it in a finally block so
duplicate requests are prevented and the UI button remains disabled while the
promise is pending; ensure you update any handlers that call onSubmit (the local
submit handler referenced around onSubmit and onCancel usage) to use this
pattern and keep proper error handling/logging.
steven-tey
approved these changes
Mar 4, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/lib/ai/upsert-docs-embedding.ts (2)
9-11:asynckeyword is unnecessary.
cleanMdxis markedasyncbut contains noawaitexpressions. This adds unnecessary overhead by wrapping the return value in a Promise.♻️ Suggested fix
-async function cleanMdx( - raw: string, -): Promise<{ content: string; title: string }> { +function cleanMdx(raw: string): { content: string; title: string } {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/ai/upsert-docs-embedding.ts` around lines 9 - 11, The function cleanMdx is declared async but contains no await; remove the async keyword from cleanMdx, change its return type from Promise<{ content: string; title: string }> to { content: string; title: string }, and update any call sites that expect a Promise to handle the synchronous return (i.e., remove unnecessary await or .then usages where cleanMdx is invoked); ensure function signature and all references (cleanMdx) are consistently adjusted.
177-181: Incomplete JSDoc forpageviewsMapparameter.The
@param pageviewsMapannotation has no description and is followed by empty lines.📝 Suggested fix
/** * Fetch, clean, chunk, and upsert a single article URL into Upstash Vector. * Uses heading-level chunks directly — no sentence-level fragmentation. * Validates URL to restrict fetches to dub.co docs/help (SSRF guard). - * - * `@param` pageviewsMap - * - * + * `@param` url - The article URL to process and upsert. + * `@param` pageviewsMap - Optional map of pathname to pageview counts for metadata enrichment. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/ai/upsert-docs-embedding.ts` around lines 177 - 181, Add a clear JSDoc description for the pageviewsMap parameter in apps/web/lib/ai/upsert-docs-embedding.ts: update the `@param` pageviewsMap line to describe the expected type (e.g., Map<string, number> or Record<string, number>), what the keys and values represent (page path/ID -> view count), and how the function uses it (e.g., to weight embeddings or filter pages), ensuring the comment matches the actual parameter name in the function signature (pageviewsMap) and removes the empty lines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/lib/ai/upsert-docs-embedding.ts`:
- Around line 9-11: The function cleanMdx is declared async but contains no
await; remove the async keyword from cleanMdx, change its return type from
Promise<{ content: string; title: string }> to { content: string; title: string
}, and update any call sites that expect a Promise to handle the synchronous
return (i.e., remove unnecessary await or .then usages where cleanMdx is
invoked); ensure function signature and all references (cleanMdx) are
consistently adjusted.
- Around line 177-181: Add a clear JSDoc description for the pageviewsMap
parameter in apps/web/lib/ai/upsert-docs-embedding.ts: update the `@param`
pageviewsMap line to describe the expected type (e.g., Map<string, number> or
Record<string, number>), what the keys and values represent (page path/ID ->
view count), and how the function uses it (e.g., to weight embeddings or filter
pages), ensuring the comment matches the actual parameter name in the function
signature (pageviewsMap) and removes the empty lines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94a1e69c-3656-43bf-a138-0871f7ee8449
📒 Files selected for processing (3)
apps/web/app/api/ai/sync-embeddings/fetch-plausible-pageviews.tsapps/web/app/api/ai/sync-embeddings/route.tsapps/web/lib/ai/upsert-docs-embedding.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/api/ai/sync-embeddings/route.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/app/api/ai/sync-embeddings/fetch-plausible-pageviews.ts (1)
12-31: Consider adding a fetch timeout for resilience.The fetch call has no timeout configured. If Plausible is slow or unresponsive, this could hang indefinitely, potentially blocking the embedding sync process.
⏱️ Proposed fix using AbortController
try { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30_000); // 30s timeout + const res = await fetch("https://plausible.io/api/v2/query", { method: "POST", headers: { Authorization: `Bearer ${apiKey}`, "Content-Type": "application/json", }, body: JSON.stringify({ site_id: "dub.co", metrics: ["pageviews"], date_range: "12mo", dimensions: ["event:page"], filters: [ ["or", [ ["contains", "event:page", ["/docs"]], ["contains", "event:page", ["/help"]], ]], ], }), + signal: controller.signal, }); + + clearTimeout(timeoutId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/ai/sync-embeddings/fetch-plausible-pageviews.ts` around lines 12 - 31, Add a timeout to the fetch in fetch-plausible-pageviews.ts by using an AbortController: create an AbortController before calling fetch, pass controller.signal into the fetch options (where Authorization/Content-Type are set), start a timer (e.g., const timeout = setTimeout(() => controller.abort(), <timeoutMs>)) and clear it (clearTimeout) after the response is received; ensure you handle the abort scenario (catch the thrown DOMException or check error.name === 'AbortError') to fail the embedding sync gracefully instead of hanging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/app/api/ai/sync-embeddings/fetch-plausible-pageviews.ts`:
- Around line 12-31: Add a timeout to the fetch in fetch-plausible-pageviews.ts
by using an AbortController: create an AbortController before calling fetch,
pass controller.signal into the fetch options (where Authorization/Content-Type
are set), start a timer (e.g., const timeout = setTimeout(() =>
controller.abort(), <timeoutMs>)) and clear it (clearTimeout) after the response
is received; ensure you handle the abort scenario (catch the thrown DOMException
or check error.name === 'AbortError') to fail the embedding sync gracefully
instead of hanging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 07182a8b-7b2c-47c1-8287-df17d08e69c9
📒 Files selected for processing (1)
apps/web/app/api/ai/sync-embeddings/fetch-plausible-pageviews.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/scripts/seed-support-embeddings.ts (1)
10-10: Avoid importing from theapp/apilayer in this standalone script.Line 10 couples a CLI script to route-layer code. Prefer moving
fetchPlausiblePageviewsto alibmodule and importing it from there to keep boundaries clean and runtime-agnostic.Suggested direction
-import { fetchPlausiblePageviews } from "app/api/ai/sync-embeddings/fetch-plausible-pageviews"; +import { fetchPlausiblePageviews } from "../lib/ai/fetch-plausible-pageviews";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/seed-support-embeddings.ts` at line 10, The script imports fetchPlausiblePageviews from the route-layer (app/api) which couples this standalone CLI to framework/runtime code; move the fetchPlausiblePageviews implementation out of the route layer into a runtime-agnostic library (e.g., apps/web/lib/fetch-plausible-pageviews or a shared lib) and update the import in seed-support-embeddings.ts to import from that lib instead of "app/api/…"; ensure the function signature and any dependencies it needs are adjusted so the new lib has no route-layer imports and the script can run independently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/scripts/seed-support-embeddings.ts`:
- Line 10: The script imports fetchPlausiblePageviews from the route-layer
(app/api) which couples this standalone CLI to framework/runtime code; move the
fetchPlausiblePageviews implementation out of the route layer into a
runtime-agnostic library (e.g., apps/web/lib/fetch-plausible-pageviews or a
shared lib) and update the import in seed-support-embeddings.ts to import from
that lib instead of "app/api/…"; ensure the function signature and any
dependencies it needs are adjusted so the new lib has no route-layer imports and
the script can run independently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 494d2093-63f9-4b27-8814-0d16b97c4e42
📒 Files selected for processing (1)
apps/web/scripts/seed-support-embeddings.ts
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
2 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
Improvements