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 an AI-powered support chat: server endpoints (support-chat streaming API, sync-embeddings), Upstash vector integration and seeding tools, Anthropic streaming with a createSupportTicket tool, embed middleware and layout, client chat UI (bubble/embedded) and supporting components (comboboxes, messages, starter questions). Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Client as Chat UI
participant API as Support Chat API
participant Upstash as Upstash Vector
participant Claude as Anthropic Claude
participant Plain as Plain (Tickets)
User->>Client: Send message
Client->>API: POST /api/ai/support-chat (stream)
API->>API: Validate session & enforce rate limit
API->>Upstash: Query vectors for last user message
Upstash-->>API: Return doc chunks
API->>Claude: Stream request (system prompt + docs + messages + tools)
Claude-->>API: Stream assistant tokens
API-->>Client: Stream UI message response
Client->>User: Render AI response
alt Escalation to human
Claude->>API: invoke createSupportTicket (tool)
API->>Plain: Create thread with chat history
Plain-->>API: Thread created
API-->>Client: Ticket creation result
Client->>User: Show confirmation
end
sequenceDiagram
participant Parent as Parent Window
participant Embed as Embed Page
participant Messenger as DynamicHeightMessenger
participant Observer as ResizeObserver
Parent->>Embed: Load embed page
Embed->>Messenger: mount
Messenger->>Parent: postMessage { type: "PAGE_HEIGHT", height: H } (initial)
Messenger->>Observer: attach ResizeObserver to body
Observer->>Messenger: body resized
Messenger->>Parent: postMessage { type: "PAGE_HEIGHT", height: H' } (updates)
alt External control
Parent->>Embed: postMessage SUPPORT_CHAT_OPEN
Embed->>Embed: open chat UI
Embed->>Parent: postMessage SUPPORT_CHAT_CLOSED when closed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
packages/ui/src/carousel/nav-bar.tsx (1)
91-94: LGTM - Type assertion correctly ensures numeric division.The
as numberassertion properly addresses potential type ambiguity from the autoplay plugin's delay option. The expression parses correctly due to TypeScript'sasprecedence rules.For slightly improved readability, you could wrap the entire cast in parentheses to make the precedence explicit:
duration: ((autoplay?.options.delay ?? AUTOPLAY_DEFAULT_DELAY) as number) / 1000,This is purely optional since the current code works correctly.
,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/carousel/nav-bar.tsx` around lines 91 - 94, The current duration expression in nav-bar's JSX uses a type assertion that relies on precedence: change the expression to make the cast precedence explicit by wrapping the asserted value in parentheses — i.e., wrap (autoplay?.options.delay ?? AUTOPLAY_DEFAULT_DELAY) in parentheses before dividing by 1000 so duration uses ((autoplay?.options.delay ?? AUTOPLAY_DEFAULT_DELAY) as number) / 1000; update the line referencing autoplay?.options.delay and AUTOPLAY_DEFAULT_DELAY accordingly for clarity.apps/web/ui/support/chat-interface.tsx (2)
66-66: Unreachable check forcontext === undefined.The
contextprop is typed asSupportChatContext("app" | "partners" | "docs"), which doesn't includeundefined. The|| context === undefinedcheck is unreachable code.Simplified condition
- const isDocsContext = context === "docs" || context === undefined; + const isDocsContext = context === "docs";🤖 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` at line 66, The check "|| context === undefined" in the isDocsContext assignment is unreachable because the context prop is typed as SupportChatContext ("app" | "partners" | "docs"); either remove the unreachable clause from the isDocsContext expression (leave const isDocsContext = context === "docs") or, if undefined is a valid runtime value, update the SupportChatContext type to include undefined (or make the prop optional) and adjust callers; reference the isDocsContext variable and the context prop / SupportChatContext type when applying the change.
130-140: Type assertions withanyfor tool invocation checks.The
(p as any)casts are used to accesstoolName,state, andresultproperties. If the AI SDK provides proper types for tool invocations, consider using those instead. Otherwise, this is acceptable given the dynamic nature of message parts.🤖 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 130 - 140, The code uses unsafe (p as any) casts when checking tool invocation properties in the ticketCreated computation; replace these casts by importing the proper tool-invocation part type from the AI SDK (or declare a local type/interface) and use a type guard that narrows parts to that type before accessing toolName, state, and result; update the logic inside the ticketCreated predicate (which iterates messages -> parts) to first assert p.type === "tool-invocation" and then narrow p to the concrete ToolInvocationPart so you can safely read p.toolName, p.state, and p.result?.success (referencing ticketCreated, parts, toolName, state, result, and the createSupportTicket tool name).apps/web/ui/support/types.ts (1)
4-8:Exclude<SupportChatContext, undefined>is redundant.
SupportChatContextis"app" | "partners" | "docs"and doesn't includeundefined, so theExcludewrapper has no effect.Simplified type
-const CONTEXT_MAP: Record<string, Exclude<SupportChatContext, undefined>> = { +const CONTEXT_MAP: Record<string, SupportChatContext> = { app: "app", partners: "partners", docs: "docs", };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/support/types.ts` around lines 4 - 8, The type annotation on CONTEXT_MAP is overly complex because SupportChatContext is already the union "app" | "partners" | "docs"; remove the unnecessary Exclude wrapper and use Record<string, SupportChatContext> (or a narrower key type if appropriate) for the CONTEXT_MAP declaration to simplify the type; update the declaration that currently references CONTEXT_MAP and SupportChatContext accordingly.apps/web/app/app.dub.co/embed/support-chat/page.tsx (1)
11-16: Consider usingstringtypes for raw searchParams.The
searchParamstype declaresvariantandcontextasSupportChatVariantandSupportChatContext, but URL query parameters are always strings at runtime. While the parse functions handlestring | undefinedcorrectly, the type annotation is slightly imprecise.Suggested type adjustment
export default async function SupportChatEmbedPage(props: { searchParams: Promise<{ - variant?: SupportChatVariant; - context?: SupportChatContext; + variant?: string; + context?: string; external?: string; }>; }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/app.dub.co/embed/support-chat/page.tsx` around lines 11 - 16, The searchParams parameter type on SupportChatEmbedPage is too specific for raw URL data; change the Promise generic so variant and context are typed as string | undefined (e.g., Promise<{ variant?: string; context?: string; external?: string }>) so the runtime string query values match the parse functions (e.g., parseSupportChatVariant/parseSupportChatContext) that accept string | undefined; update the function signature accordingly.
🤖 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 67-99: queryText and contextChunks are unbounded and must be
capped: before calling vectorIndex.query, truncate queryText (derived from
lastUserMessage) to a safe char/token limit (e.g., 500-1000 chars or N tokens)
and pass that truncated string to vectorIndex.query; after receiving results
from vectorIndex.query, limit the number of results (topK) if needed and build
contextChunks by truncating each meta.content/meta.heading to a per-chunk
char/token budget and by stopping concatenation once a total context token/char
budget is reached (drop or trim remaining chunks), ensuring the final
contextChunks string is clipped to a global limit to avoid exceeding model input
sizes.
- Around line 38-41: Validate the request body returned by req.json() before
casting: replace the direct cast that sets "const { messages, context = 'app' }
= (await req.json()) as { messages: UIMessage[]; context?: SupportChatContext;
}" with runtime checks that ensure messages is an array (and optionally that
each item conforms to UIMessage shape) and that context, if present, is a string
and one of the allowed SupportChatContext values; if validation fails, return a
400 error, otherwise safely assign the validated values (and only then call
context.toUpperCase()) so downstream logic (including the code that uses
context.toUpperCase()) cannot crash on malformed input.
In `@apps/web/app/app.dub.co/embed/support-chat/dynamic-height-messenger.tsx`:
- Around line 7-27: The effect in dynamic-height-messenger.tsx sets
document.body.style.overflow = "hidden" but never restores it; capture the
existing overflow (e.g., const prevOverflow = document.body.style.overflow)
before changing it, then in the cleanup returned by the effect restore
document.body.style.overflow = prevOverflow in addition to calling
resizeObserver.disconnect(), so the page's scroll state is returned when the
component unmounts.
In `@apps/web/ui/support/types.ts`:
- Around line 16-20: parseSupportChatContext currently returns
CONTEXT_MAP[value] directly which yields undefined for unknown keys; change it
to explicitly fall back to the default CONTEXT_MAP.docs when the lookup is
undefined (e.g., use a nullish-coalescing or existence check) so the function
always returns a valid SupportChatContext; update the implementation referencing
parseSupportChatContext and CONTEXT_MAP to return CONTEXT_MAP[value] ??
CONTEXT_MAP.docs (or check hasOwnProperty/value in CONTEXT_MAP) to satisfy the
return type.
In `@apps/web/ui/support/workspace-combobox.tsx`:
- Around line 49-52: The setSelected callback dereferences opt.value without
checking for null which crashes when Combobox passes null on re-select; update
the setSelected handler in workspace-combobox.tsx to first check if opt is null
and handle that case (e.g., call onSelect(undefined) or clear selection) and
only when opt is non-null find the workspace via workspaces.find(w => w.slug ===
opt.value) and call onSelect(ws) if found; ensure references to setSelected,
workspaces, and onSelect are updated accordingly.
---
Nitpick comments:
In `@apps/web/app/app.dub.co/embed/support-chat/page.tsx`:
- Around line 11-16: The searchParams parameter type on SupportChatEmbedPage is
too specific for raw URL data; change the Promise generic so variant and context
are typed as string | undefined (e.g., Promise<{ variant?: string; context?:
string; external?: string }>) so the runtime string query values match the parse
functions (e.g., parseSupportChatVariant/parseSupportChatContext) that accept
string | undefined; update the function signature accordingly.
In `@apps/web/ui/support/chat-interface.tsx`:
- Line 66: The check "|| context === undefined" in the isDocsContext assignment
is unreachable because the context prop is typed as SupportChatContext ("app" |
"partners" | "docs"); either remove the unreachable clause from the
isDocsContext expression (leave const isDocsContext = context === "docs") or, if
undefined is a valid runtime value, update the SupportChatContext type to
include undefined (or make the prop optional) and adjust callers; reference the
isDocsContext variable and the context prop / SupportChatContext type when
applying the change.
- Around line 130-140: The code uses unsafe (p as any) casts when checking tool
invocation properties in the ticketCreated computation; replace these casts by
importing the proper tool-invocation part type from the AI SDK (or declare a
local type/interface) and use a type guard that narrows parts to that type
before accessing toolName, state, and result; update the logic inside the
ticketCreated predicate (which iterates messages -> parts) to first assert
p.type === "tool-invocation" and then narrow p to the concrete
ToolInvocationPart so you can safely read p.toolName, p.state, and
p.result?.success (referencing ticketCreated, parts, toolName, state, result,
and the createSupportTicket tool name).
In `@apps/web/ui/support/types.ts`:
- Around line 4-8: The type annotation on CONTEXT_MAP is overly complex because
SupportChatContext is already the union "app" | "partners" | "docs"; remove the
unnecessary Exclude wrapper and use Record<string, SupportChatContext> (or a
narrower key type if appropriate) for the CONTEXT_MAP declaration to simplify
the type; update the declaration that currently references CONTEXT_MAP and
SupportChatContext accordingly.
In `@packages/ui/src/carousel/nav-bar.tsx`:
- Around line 91-94: The current duration expression in nav-bar's JSX uses a
type assertion that relies on precedence: change the expression to make the cast
precedence explicit by wrapping the asserted value in parentheses — i.e., wrap
(autoplay?.options.delay ?? AUTOPLAY_DEFAULT_DELAY) in parentheses before
dividing by 1000 so duration uses ((autoplay?.options.delay ??
AUTOPLAY_DEFAULT_DELAY) as number) / 1000; update the line referencing
autoplay?.options.delay and AUTOPLAY_DEFAULT_DELAY accordingly for clarity.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/web/app/api/ai/support-chat/route.tsapps/web/app/app.dub.co/embed/support-chat/dynamic-height-messenger.tsxapps/web/app/app.dub.co/embed/support-chat/layout.tsxapps/web/app/app.dub.co/embed/support-chat/page.tsxapps/web/lib/middleware/embed.tsapps/web/lib/upstash/vector.tsapps/web/package.jsonapps/web/ui/support/chat-bubble.tsxapps/web/ui/support/chat-interface.tsxapps/web/ui/support/embedded-chat.tsxapps/web/ui/support/message.tsxapps/web/ui/support/program-combobox.tsxapps/web/ui/support/starter-questions.tsxapps/web/ui/support/types.tsapps/web/ui/support/workspace-combobox.tsxpackages/ui/src/carousel/nav-bar.tsx
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/sync-embeddings/route.ts`:
- Around line 29-36: Replace brittle startsWith checks on the raw url with a
parsed URL validation: use the URL constructor on the incoming url string,
ensure protocol === "https:", hostname === "dub.co" (or optionally handle
"www.dub.co"), and check parsed.pathname startsWith "/docs/" or "/help/"; if
validation fails return the 400 Response, otherwise pass the normalized
parsedUrl.toString() (or pathname+origin) into seedArticle (reference: url
variable and seedArticle) and apply the same parsing/validation logic to the
other occurrence mentioned (lines 39-40).
- Around line 42-45: Replace the practice of returning the raw error string to
the client: keep logging the detailed error with console.error(err) but change
the Response.json call (the one returning { success: false, url, error:
String(err) }) to return a stable, generic error message (e.g. error: "Internal
server error" or "Failed to seed embeddings") while preserving the 500 status;
reference the Response.json invocation and the err variable so you only change
the payload sent to clients and not the internal logging.
In `@apps/web/lib/ai/seed-article.ts`:
- Around line 199-204: The current fallback in seed-article.ts accepts any
string starting with "http" that contains "/docs/" or "/help/", which can allow
external domains; instead, parse the candidate (the variable trimmed) with the
URL constructor and only push it (via the existing
urls.push(trimmed.replace(/\.md$/, ""))) when it is either a relative path
(startsWith "/") or an absolute URL whose hostname matches our site host/allowed
domain(s) (e.g., compare url.hostname to a configured SITE_DOMAIN or
window.location.host); update the condition around trimmed.startsWith("http") to
perform this hostname check before calling urls.push so external domains are
excluded.
- Around line 101-113: Duplicate headings produce identical vector IDs because
id is built only from url and slug; modify the logic around
currentHeading/slug/id before chunks.push to ensure unique IDs by tracking seen
slugs per-article (e.g., a Map or object keyed by slug) and appending a numeric
suffix when a slug repeats (update both id and url fields), and apply the same
change to the other block that constructs ids at the referenced 121-124 region;
reference the variables currentHeading, slug, id, and chunks.push so you add the
slug-counting logic in that scope.
In `@apps/web/scripts/seed-support-embeddings.ts`:
- Around line 16-23: The code currently treats a present --url flag with no
following value as “not provided” and proceeds to seed all; update the branch
that checks urlFlagIdx to detect when args[urlFlagIdx + 1] is missing and fail
fast: when urlFlagIdx !== -1 and (!args[urlFlagIdx + 1]) log or throw a clear
error and exit (do not fall through to bulk seeding). Modify the block that uses
urlFlagIdx and seedArticle to validate the URL argument and return/terminate
immediately on missing value so seedArticle(url) is only called when a value
exists.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
apps/web/app/api/ai/sync-embeddings/route.tsapps/web/lib/ai/seed-article.tsapps/web/scripts/seed-support-embeddings.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/web/lib/ai/seed-article.ts (3)
101-113:⚠️ Potential issue | 🟠 MajorPrevent vector ID collisions for repeated headings.
At Line 106, IDs are derived from
url + slugonly. Repeated H2/H3 headings overwrite earlier vectors.Proposed fix
const lines = content.split("\n"); const chunks: ArticleChunk[] = []; + const slugCounts = new Map<string, number>(); @@ - const slug = currentHeading + const baseSlug = currentHeading .toLowerCase() .replace(/[^a-z0-9]+/g, "-") .replace(/^-+|-+$/g, ""); + const count = (slugCounts.get(baseSlug) ?? 0) + 1; + slugCounts.set(baseSlug, count); + const slug = count === 1 ? baseSlug : `${baseSlug}-${count}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/ai/seed-article.ts` around lines 101 - 113, IDs for chunks are built only from url and slug (variables currentHeading and url), causing collisions when the same heading appears multiple times; modify the logic that constructs id and url in the chunk push (the id variable and the url field inside chunks.push) to append a disambiguator such as a per-slug counter or a short hash of the heading/text (maintain a map like slugCounts keyed by slug and increment to produce suffixes) so each id becomes unique (e.g., `${url}#${slug}-${countOrHash}`) and ensure the url field in the chunk uses the same unique value.
202-207:⚠️ Potential issue | 🟠 MajorRestrict fallback URL extraction to dub.co only.
At Line 203, the fallback accepts any
httpURL containing/docs/or/help/, which can ingest external domains.Proposed fix
- } else if ( - trimmed.startsWith("http") && - (trimmed.includes("/docs/") || trimmed.includes("/help/")) - ) { - urls.push(trimmed.replace(/\.md$/, "")); + } else if (trimmed.startsWith("http")) { + try { + const parsed = new URL(trimmed); + if ( + parsed.hostname === "dub.co" && + (parsed.pathname.startsWith("/docs/") || + parsed.pathname.startsWith("/help/")) + ) { + urls.push(parsed.toString().replace(/\.md$/, "")); + } + } catch { + // ignore malformed absolute URLs + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/ai/seed-article.ts` around lines 202 - 207, The fallback URL extraction currently accepts any http URL with "/docs/" or "/help/" (the conditional using trimmed.startsWith("http") and trimmed.includes("/docs/") || trimmed.includes("/help/")) which may pull external domains; restrict this to only allow dub.co by checking the host/domain (e.g., ensure trimmed starts with "http" and includes "://dub.co/" or matches the dub.co hostname) before calling urls.push(trimmed.replace(/\.md$/, "")) so only dub.co docs/help URLs are added.
149-154:⚠️ Potential issue | 🟠 MajorValidate/allowlist article URLs before network fetch.
At Line 154,
fetch(mdUrl)uses caller-providedurl. If upstream validation is bypassed, this is an SSRF path.Use this script to confirm whether all call sites enforce strict hostname/path checks before calling
seedArticle:#!/bin/bash set -euo pipefail fd 'seed-article.ts|seed-support-embeddings.ts|route.ts' apps/web -t f rg -nP --type=ts -C4 '\bseedArticle\s*\(' apps/web rg -nP --type=ts -C6 '(safeParse|zod|new URL|hostname|dub\.co|/docs/|/help/|fetch\()' apps/web/app/api/ai/sync-embeddings/route.ts apps/web/scripts/seed-support-embeddings.ts 2>/dev/null || trueExpected result: every external input path is constrained to
https://dub.co/docs/*orhttps://dub.co/help/*beforeseedArticleis invoked.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/ai/seed-article.ts` around lines 149 - 154, The fetch call in seedArticle currently uses caller-provided mdUrl (mdUrl and fetch(mdUrl)) and must be protected by an allowlist; modify seedArticle to parse the mdUrl with new URL(mdUrl), enforce scheme === 'https:', hostname === 'dub.co' (or explicitly allowed hostnames), and pathname startsWith('/docs/') || startsWith('/help/'), and throw or return skipped=true for disallowed URLs before calling fetch; ensure the logic runs after you append ".md" so validation covers the final URL and update any callers if they relied on upstream checks.
🤖 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/lib/ai/seed-article.ts`:
- Around line 97-100: In the flush() helper, when the chunk is too short and you
early-return, clear the buffer by resetting currentLines = [] before returning
so the short content doesn't leak into the next chunk; apply the same fix to the
other early-return branch referenced (the similar short-chunk check around the
second flush call), ensuring both locations reset currentLines (and any related
state like currentHeading if applicable) before exiting.
---
Duplicate comments:
In `@apps/web/lib/ai/seed-article.ts`:
- Around line 101-113: IDs for chunks are built only from url and slug
(variables currentHeading and url), causing collisions when the same heading
appears multiple times; modify the logic that constructs id and url in the chunk
push (the id variable and the url field inside chunks.push) to append a
disambiguator such as a per-slug counter or a short hash of the heading/text
(maintain a map like slugCounts keyed by slug and increment to produce suffixes)
so each id becomes unique (e.g., `${url}#${slug}-${countOrHash}`) and ensure the
url field in the chunk uses the same unique value.
- Around line 202-207: The fallback URL extraction currently accepts any http
URL with "/docs/" or "/help/" (the conditional using trimmed.startsWith("http")
and trimmed.includes("/docs/") || trimmed.includes("/help/")) which may pull
external domains; restrict this to only allow dub.co by checking the host/domain
(e.g., ensure trimmed starts with "http" and includes "://dub.co/" or matches
the dub.co hostname) before calling urls.push(trimmed.replace(/\.md$/, "")) so
only dub.co docs/help URLs are added.
- Around line 149-154: The fetch call in seedArticle currently uses
caller-provided mdUrl (mdUrl and fetch(mdUrl)) and must be protected by an
allowlist; modify seedArticle to parse the mdUrl with new URL(mdUrl), enforce
scheme === 'https:', hostname === 'dub.co' (or explicitly allowed hostnames),
and pathname startsWith('/docs/') || startsWith('/help/'), and throw or return
skipped=true for disallowed URLs before calling fetch; ensure the logic runs
after you append ".md" so validation covers the final URL and update any callers
if they relied on upstream checks.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
apps/web/scripts/seed-support-embeddings.ts (1)
16-21:⚠️ Potential issue | 🟡 MinorReject
--urlwhen the next token is another flag.This still accepts inputs like
--url --dry-runas a URL. Fail fast instead of passing it toseedArticle.💡 Proposed fix
if (urlFlagIdx !== -1) { const url = args[urlFlagIdx + 1]; - if (!url) { + if (!url || url.startsWith("--")) { console.error("Error: --url requires a value (e.g. --url https://dub.co/docs/...)"); process.exit(1); }🤖 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` around lines 16 - 21, The URL parsing currently treats the token after --url as a value even if it's another flag (e.g. --url --dry-run); update the validation around urlFlagIdx in the script so that after retrieving const url = args[urlFlagIdx + 1] you also check that url is defined and does not start with '-' (e.g. url.startsWith('-')) and if it does treat it as a missing value: log the same error and exit; this change affects the block handling urlFlagIdx in seed-support-embeddings.ts and prevents passing flag tokens into seedArticle.apps/web/app/api/ai/support-chat/route.ts (1)
40-53:⚠️ Potential issue | 🟠 MajorValidate each message object shape before casting.
You validate
messagesas an array, but not the element structure. Later reads ofmsg.partsand text-part casts can still throw on malformed payloads.💡 Proposed fix
const body = await req.json().catch(() => null); + const isValidMessageShape = (m: unknown): m is UIMessage => { + if (!m || typeof m !== "object") return false; + const msg = m as { role?: unknown; parts?: unknown }; + return ( + typeof msg.role === "string" && + Array.isArray(msg.parts) && + msg.parts.every((p) => p && typeof p === "object" && "type" in p) + ); + }; + if ( !body || !Array.isArray(body.messages) || + !body.messages.every(isValidMessageShape) || (body.context !== undefined && !VALID_CONTEXTS.includes(body.context as SupportChatContext)) ) { return new Response("Invalid request body.", { status: 400 }); }Also applies to: 76-83, 163-168
🤖 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/route.ts` around lines 40 - 53, Validate each message element before casting by iterating over body.messages and ensuring every item is an object with a parts array and that each part is an object with expected fields (e.g., type === "text" and text is a string) before using msg.parts or casting to UIMessage; update the checks around the destructuring of messages/context and the other similar blocks that access msg.parts (look for usages of messages, UIMessage, and msg.parts) to return 400 on any malformed message, and where appropriate coerce/normalize safe defaults rather than directly assuming shape.
🤖 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 126-130: The call to convertToModelMessages(messages) forwards the
entire chat history which can grow unbounded; before passing messages into
streamText (where model and systemPrompt are set), truncate the history to a
recent window (e.g., last N entries or by token budget) by adding a short helper
(e.g., truncateMessages or trimMessagesToTokenLimit) and use that return value
in convertToModelMessages(...) so streamText receives only the bounded
conversation; update references in this file where
convertToModelMessages(messages) is used to call
convertToModelMessages(truncateMessages(messages)) (or
convertToModelMessages(await truncateMessagesByTokens(messages))) and ensure
stepCountIs(5) logic remains unchanged.
In `@apps/web/app/app.dub.co/embed/support-chat/page.tsx`:
- Around line 12-19: searchParams in props can be string|string[]|undefined but
the code treats variant/context as string|undefined; update the extraction to
normalize array-valued params before calling parseSupportChatVariant and
parseSupportChatContext: await props.searchParams into searchParams, then for
each param (e.g., variantParam and contextParam) set it to
Array.isArray(searchParams.variant) ? searchParams.variant[0] :
searchParams.variant (same for context) and pass those normalized
string|undefined values into parseSupportChatVariant and parseSupportChatContext
so array query values (like ?variant=a&variant=b) are handled predictably.
In `@apps/web/lib/ai/seed-article.ts`:
- Around line 184-187: The code builds mdUrl by appending ".md" to
parsedUrl.toString(), which incorrectly puts the extension after query/hash;
instead append ".md" to the parsedUrl.pathname and then rebuild the full URL so
query and hash remain intact. Locate parsedUrl, normalizedUrl and mdUrl in the
function and change the logic to set parsedUrl.pathname =
parsedUrl.pathname.endsWith(".md") ? parsedUrl.pathname : parsedUrl.pathname +
".md" (or equivalent) and then use parsedUrl.toString() for mdUrl so the
resulting URL is "<path>.md" with original query and fragment preserved.
In `@apps/web/ui/support/program-combobox.tsx`:
- Around line 53-55: The setSelected handler accesses opt.value without checking
for null; update the setSelected callback in program-combobox.tsx to guard
against opt being null (as Combobox single-select can pass null on deselect) —
e.g., if opt is null, handle the deselect case (return early or call
onSelect(null/undefined) depending on expected behavior) and only call
enrollments?.find(e => e.program.slug === opt.value) and
onSelect(enrollment.program) when opt is non-null.
---
Duplicate comments:
In `@apps/web/app/api/ai/support-chat/route.ts`:
- Around line 40-53: Validate each message element before casting by iterating
over body.messages and ensuring every item is an object with a parts array and
that each part is an object with expected fields (e.g., type === "text" and text
is a string) before using msg.parts or casting to UIMessage; update the checks
around the destructuring of messages/context and the other similar blocks that
access msg.parts (look for usages of messages, UIMessage, and msg.parts) to
return 400 on any malformed message, and where appropriate coerce/normalize safe
defaults rather than directly assuming shape.
In `@apps/web/scripts/seed-support-embeddings.ts`:
- Around line 16-21: The URL parsing currently treats the token after --url as a
value even if it's another flag (e.g. --url --dry-run); update the validation
around urlFlagIdx in the script so that after retrieving const url =
args[urlFlagIdx + 1] you also check that url is defined and does not start with
'-' (e.g. url.startsWith('-')) and if it does treat it as a missing value: log
the same error and exit; this change affects the block handling urlFlagIdx in
seed-support-embeddings.ts and prevents passing flag tokens into seedArticle.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/app/api/ai/support-chat/route.tsapps/web/app/api/ai/sync-embeddings/route.tsapps/web/app/app.dub.co/embed/support-chat/page.tsxapps/web/lib/ai/seed-article.tsapps/web/scripts/seed-support-embeddings.tsapps/web/ui/support/chat-bubble.tsxapps/web/ui/support/program-combobox.tsxapps/web/ui/support/types.tsapps/web/ui/support/workspace-combobox.tsxpackages/ui/src/combobox/index.tsxpackages/ui/src/popover.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/app/api/ai/sync-embeddings/route.ts
- apps/web/ui/support/types.ts
- apps/web/ui/support/workspace-combobox.tsx
- apps/web/ui/support/chat-bubble.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web/app/api/ai/support-chat/route.ts (1)
47-60:⚠️ Potential issue | 🟠 MajorValidate message item shape, not just
messagesarray existence.The body check at Line 50 only validates that
messagesis an array. Later accesses at Line 87 and Line 174 assume each message hasparts; malformed items can still crash this handler.💡 Proposed fix
+ const isValidMessages = (input: unknown): input is UIMessage[] => + Array.isArray(input) && + input.every((m) => { + if (!m || typeof m !== "object") return false; + const msg = m as { + role?: unknown; + parts?: unknown; + }; + return ( + (msg.role === "user" || + msg.role === "assistant" || + msg.role === "system" || + msg.role === "tool") && + Array.isArray(msg.parts) + ); + }); + const body = await req.json().catch(() => null); if ( !body || - !Array.isArray(body.messages) || + !isValidMessages(body.messages) || (body.context !== undefined && !VALID_CONTEXTS.includes(body.context as SupportChatContext)) ) { return new Response("Invalid request body.", { status: 400 }); }🤖 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/route.ts` around lines 47 - 60, The request body validation currently only checks that body.messages is an array; update the validation in route.ts to ensure each item in messages matches the expected UIMessage shape (e.g., has a parts property that is an array of strings and any required fields like role or id if used later). Specifically, iterate over messages after parsing (before destructuring) and verify for every message that Array.isArray(message.parts) and parts.every(p => typeof p === "string") (and validate other fields you rely on), and return a 400 response if any item is malformed; reference the existing body/messages variables and the UIMessage shape used later in the handler.
🧹 Nitpick comments (1)
apps/web/lib/ai/seed-article.ts (1)
207-220: Batch vector upserts instead of issuing one request per chunk.Line 207 currently does N sequential network calls. This can make seeding slow and increase timeout/rate-limit risk when articles have many sections. The
@upstash/vectorv1.2.2 SDK supports batch upserts via array payloads, with Upstash recommending batches of ≤1,000 records for optimal performance.Proposed refactor
- for (const chunk of chunks) { - await vectorIndex.upsert([ - { - id: chunk.id, - data: chunk.content, - metadata: { - url: chunk.url, - heading: chunk.heading, - type: chunk.type, - content: chunk.content.slice(0, MAX_METADATA_CONTENT), - }, - }, - ]); - } + const vectors = chunks.map((chunk) => ({ + id: chunk.id, + data: chunk.content, + metadata: { + url: chunk.url, + heading: chunk.heading, + type: chunk.type, + content: chunk.content.slice(0, MAX_METADATA_CONTENT), + }, + })); + + const BATCH_SIZE = 100; + for (let i = 0; i < vectors.length; i += BATCH_SIZE) { + await vectorIndex.upsert(vectors.slice(i, i + BATCH_SIZE)); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/ai/seed-article.ts` around lines 207 - 220, The loop is performing N sequential network calls by calling vectorIndex.upsert for each chunk; change this to batch upserts using the SDK's array payload support: partition the chunks array into batches (<=1000 per Upstash recommendation), map each chunk to the upsert record shape (id, data, metadata with url, heading, type, and content sliced by MAX_METADATA_CONTENT), and call await vectorIndex.upsert(batchRecords) once per batch instead of per chunk; ensure you replace the per-chunk call inside the loop with batching logic that still preserves chunk.id, chunk.content, chunk.url, chunk.heading, chunk.type and MAX_METADATA_CONTENT.
🤖 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 141-224: The createSupportTicket tool's execute handler currently
risks duplicate Plain threads because it lacks idempotency; update execute (the
function on createSupportTicket) to accept and use the provided toolCallId
(second arg) as an idempotency key: before calling createPlainThread check
durable storage for an existing record by toolCallId (e.g.,
db.toolExecutions.findUnique), atomically insert a "started" record if none
exists (or use upsert with a unique constraint on toolCallId) to avoid races,
proceed to call createPlainThread only if this invocation is the first, and then
mark the record "completed" (update) after successful creation; on errors return
the appropriate cached/previous result or failure state so repeated invocations
are safe.
In `@apps/web/lib/ai/seed-article.ts`:
- Around line 237-252: The URL normalization currently uses string
replace(/\.md$/, "") on linkMatch[1] and trimmed which fails when
queries/fragments are present; update the logic in the block handling linkMatch
and the else branch (symbols: linkMatch, trimmed, urls, allowedHostnames,
allowedPathPrefixes) to parse the URL via new URL(...), remove a trailing ".md"
from parsed.pathname only, then rebuild a normalized URL (hostname + pathname
without .md, and omit query and fragment or normalize them consistently) before
pushing into urls so deduplication works correctly.
In `@apps/web/ui/support/program-combobox.tsx`:
- Line 33: The fallback avatar URL concatenates raw program.name into
`${OG_AVATAR_URL}${e.program.name}`, which can break for reserved characters;
update the JSX src expression (where e.program.logo is used) to URL-encode the
program name (e.g., use encodeURIComponent(e.program.name) or an equivalent
encoder) when building the fallback `${OG_AVATAR_URL}${...}` so the constructed
URL is safe for all program names.
---
Duplicate comments:
In `@apps/web/app/api/ai/support-chat/route.ts`:
- Around line 47-60: The request body validation currently only checks that
body.messages is an array; update the validation in route.ts to ensure each item
in messages matches the expected UIMessage shape (e.g., has a parts property
that is an array of strings and any required fields like role or id if used
later). Specifically, iterate over messages after parsing (before destructuring)
and verify for every message that Array.isArray(message.parts) and parts.every(p
=> typeof p === "string") (and validate other fields you rely on), and return a
400 response if any item is malformed; reference the existing body/messages
variables and the UIMessage shape used later in the handler.
---
Nitpick comments:
In `@apps/web/lib/ai/seed-article.ts`:
- Around line 207-220: The loop is performing N sequential network calls by
calling vectorIndex.upsert for each chunk; change this to batch upserts using
the SDK's array payload support: partition the chunks array into batches (<=1000
per Upstash recommendation), map each chunk to the upsert record shape (id,
data, metadata with url, heading, type, and content sliced by
MAX_METADATA_CONTENT), and call await vectorIndex.upsert(batchRecords) once per
batch instead of per chunk; ensure you replace the per-chunk call inside the
loop with batching logic that still preserves chunk.id, chunk.content,
chunk.url, chunk.heading, chunk.type and MAX_METADATA_CONTENT.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/app/api/ai/support-chat/route.tsapps/web/app/app.dub.co/embed/support-chat/page.tsxapps/web/lib/ai/seed-article.tsapps/web/ui/support/program-combobox.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/app.dub.co/embed/support-chat/page.tsx
steven-tey
approved these changes
Mar 3, 2026
This was referenced Mar 3, 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
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
Docs / Tools
Bug Fixes