Daily branch 2025 08 18 #10
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds an optional EXA web-search API key to env template, wires geolocation into the chat API and tool factory, introduces an Exa-backed webSearch tool, converts the sidebar to a content union (file | terminal) with streaming updates, and updates related UI, types, and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant API as /api/chat
participant Geo as Vercel Geolocation
participant Tools as createTools
participant LLM as streamText
C->>API: POST /api/chat
API->>Geo: geolocation(req)
Geo-->>API: userLocation
API->>Tools: createTools(userID, writer, mode, execMode, userLocation)
API->>LLM: streamText(..., headers: getAIHeaders())
LLM-->>C: streamed response
sequenceDiagram
participant Agent as Agent Tools
participant Hook as useSidebarAutoOpen
participant GS as GlobalState
participant UI as ComputerSidebar
Agent-->>Hook: tool status updates (input-available / streaming / output-available)
Hook->>GS: openSidebar({ command, output: "", isExecuting: true, toolCallId })
Agent-->>Hook: data-terminal streaming parts
Hook->>GS: updateSidebarContent({ output: append(stream) })
GS-->>UI: sidebarContent changed
UI-->>User: sidebar terminal view updates live
sequenceDiagram
participant Tool as webSearch Tool
participant Exa as Exa API
participant Ctx as ToolContext
Tool->>Ctx: read userLocation
Tool->>Exa: search(query, userLocation?)
alt error mentions userLocation/country
Tool->>Exa: retry search(query) without location
end
Exa-->>Tool: results
Tool-->>Caller: results array or error string
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (7)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
types/agent.ts (1)
15-20: EnsureuserLocationis passed and remove unintended runtime imports of@vercel/functionsYour new required
userLocation: GeoonToolContextis a breaking change—everycreateTools(…)call must now include that fourth argument. Also, we’ve detected runtime imports of@vercel/functionsthat may leak into client bundles.• lib/utils/safe-wait-until.ts:
import { waitUntil } from "@vercel/functions";→ Confirm this module is only used server-side. If it ever ends up in client code, convert to a
import type { … }or relocate it behind a server boundary.• app/api/chat/route.ts (server):
import { geolocation } from "@vercel/functions"; // OK for server … const { tools, getSandbox } = createTools( userID, writer, mode, // ← missing userLocation! );Update to:
const { tools, getSandbox } = createTools( userID, writer, mode, geolocation // pass as userLocation: Geo );Review any other
createTools(...)invocations to ensure the fourthuserLocationargument is supplied, and verify no client-side imports of@vercel/functionsremain.app/components/TerminalCodeBlock.tsx (1)
161-168: Fix invalid Tailwind class for word-break
[&_pre]:word-break-break-wordisn’t a valid Tailwind utility. Usebreak-wordsinstead; combined withwhitespace-pre-wrap, it achieves the intended wrapping.Apply this diff:
- const baseClasses = `shiki not-prose relative bg-card text-sm font-[450] text-card-foreground [&_pre]:!bg-transparent [&_pre]:px-[1em] [&_pre]:py-[1em] [&_pre]:rounded-none [&_pre]:m-0 [&_pre]:min-w-0 ${heightClasses} ${ - isWrapped - ? "[&_pre]:whitespace-pre-wrap [&_pre]:break-words [&_pre]:overflow-visible [&_pre]:word-break-break-word" - : "[&_pre]:overflow-x-auto [&_pre]:max-w-full" - }`; + const baseClasses = `shiki not-prose relative bg-card text-sm font-[450] text-card-foreground [&_pre]:!bg-transparent [&_pre]:px-[1em] [&_pre]:py-[1em] [&_pre]:rounded-none [&_pre]:m-0 [&_pre]:min-w-0 ${heightClasses} ${ + isWrapped + ? "[&_pre]:whitespace-pre-wrap [&_pre]:break-words [&_pre]:overflow-visible" + : "[&_pre]:overflow-x-auto [&_pre]:max-w-full" + }`;
🧹 Nitpick comments (22)
lib/auth/server.ts (1)
24-26: Consider reducing error verbosity and using a structured loggerconsole.error may include sensitive/verbose details and can be noisy under load. Prefer a structured logger with levels/sampling, and avoid logging full error objects unless necessary.
- } catch (error) { - console.error("Failed to get user session:", error); + } catch (error) { + // TODO: route through structured logger and reduce verbosity in production + console.error("Failed to get user session");app/components/MessagePartHandler.tsx (1)
21-35: Optional: treat all non-assistant roles as plain textTo be conservative, you can render Markdown only for assistant messages and treat all other roles (user/system/tool) as plain text. This reduces accidental Markdown rendering for roles you might add later.
Apply this diff:
- const isUser = message.role === "user"; + const isAssistant = message.role === "assistant"; - // For user messages, render plain text to avoid markdown processing - if (isUser) { + // For non-assistant roles, render plain text to avoid markdown processing + if (!isAssistant) { return ( <div key={partId} className="whitespace-pre-wrap"> {part.text ?? ""} </div> ); } - // For assistant messages, use markdown rendering + // Assistant messages: use markdown renderingapp/components/ComputerCodeBlock.tsx (2)
111-121: A11y: reflect toggle state with aria-pressed.Expose the pressed state for assistive tech on the wrap toggle button.
<button onClick={handleToggleWrap} className={`inline-flex items-center justify-center rounded-md px-3 py-1 text-xs transition-colors ${ isWrapped ? "bg-background text-foreground shadow-sm" : "text-muted-foreground hover:bg-background hover:text-foreground" }`} aria-label={ isWrapped ? "Disable text wrapping" : "Enable text wrapping" } + aria-pressed={isWrapped} >
61-64: Prefer falling back to download if File System Access API fails.If the native save dialog throws (not user cancel), returning early prevents the fallback download. Let it continue to the fallback for better resilience.
- } catch { - toast.error("Failed to save file"); - return; - } + } catch { + toast.error("Failed to save file, falling back to download"); + // Continue to fallback below + }lib/actions/index.ts (1)
34-38: Centralize header construction to avoid duplication and drift.Since similar headers are also set in
app/api/chat/route.ts, consider factoring a small helper to keep them in sync.Create
lib/ai/providerHeaders.ts:// lib/ai/providerHeaders.ts export const buildProviderHeaders = () => ({ "HTTP-Referer": process.env.NEXT_PUBLIC_APP_URL ?? "https://www.hackerai.co", "X-Title": process.env.NODE_ENV === "development" ? "HackerAI-Dev" : "HackerAI", });Then here:
- headers: { - "HTTP-Referer": "https://www.hackerai.co", - "X-Title": - process.env.NODE_ENV === "development" ? "HackerAI-Dev" : "HackerAI", - }, + headers: buildProviderHeaders(),And mirror that usage in your route handler.
types/chat.ts (1)
24-34: Prefer a discriminated union over ad-hoc property checks for future-proofing.Current type guards rely on "path" and "command" existence. This works but can become brittle if shapes evolve. A discriminant (e.g., kind: "file" | "terminal") simplifies narrowing and prevents accidental property collisions.
Apply this diff to introduce a discriminant while keeping existing fields:
export interface SidebarFile { + kind: "file"; path: string; content: string; language?: string; range?: { start: number; end: number; }; action?: "reading" | "creating" | "editing" | "writing"; } export interface SidebarTerminal { + kind: "terminal"; command: string; output: string; isExecuting: boolean; isBackground?: boolean; toolCallId: string; } export type SidebarContent = SidebarFile | SidebarTerminal; -export const isSidebarFile = ( - content: SidebarContent, -): content is SidebarFile => { - return "path" in content; -}; +export const isSidebarFile = ( + content: SidebarContent, +): content is SidebarFile => content.kind === "file"; -export const isSidebarTerminal = ( - content: SidebarContent, -): content is SidebarTerminal => { - return "command" in content; -}; +export const isSidebarTerminal = ( + content: SidebarContent, +): content is SidebarTerminal => content.kind === "terminal";Note: This change will require updating producers to set kind accordingly.
app/components/Header.tsx (2)
25-69: Responsive split looks good; minor navigation improvement suggested.Desktop/mobile split with mutually exclusive visibility is clear. Consider client-side navigation for a faster UX instead of full page reloads via window.location.
Apply this diff to use Next.js router:
import React from "react"; import { HackerAISVG } from "@/components/icons/hackerai-svg"; import { Button } from "@/components/ui/button"; import { useAppAuth } from "../hooks/useAppAuth"; import { isWorkOSEnabled } from "@/lib/auth/client"; +import { useRouter } from "next/navigation"; const Header: React.FC = () => { - const { user, loading } = useAppAuth(); + const { user, loading } = useAppAuth(); + const router = useRouter(); const handleSignIn = () => { - window.location.href = "/login"; + router.push("/login"); }; const handleSignUp = () => { - window.location.href = "/signup"; + router.push("/signup"); }; const handleSignOut = async () => { - window.location.href = "/logout"; + router.push("/logout"); };
72-114: Consistent mobile UX; consider making the logo/title a home link.Nice mobile treatment and button sizing. As a small UX boost, wrap the logo/title in a link to “/” to provide a predictable tap target for returning home.
Example:
- <div className="flex items-center gap-2"> - <HackerAISVG theme="dark" scale={0.12} /> - <span className="text-foreground text-lg font-semibold"> - HackerAI - </span> - </div> + <a href="/" className="flex items-center gap-2" aria-label="Go to home"> + <HackerAISVG theme="dark" scale={0.12} /> + <span className="text-foreground text-lg font-semibold"> + HackerAI + </span> + </a>app/components/Footer.tsx (1)
13-37: Add noopener for security when using target="_blank".To avoid reverse tabnabbing, include noopener alongside noreferrer.
Apply this diff:
- rel="noreferrer" + rel="noopener noreferrer" > Terms </a>{" "} and have read our{" "} <a href="/privacy-policy" target="_blank" - rel="noreferrer" + rel="noopener noreferrer" > Privacy Policy </a>app/components/tools/FileToolsHandler.tsx (1)
64-69: Line-number stripping and range mapping are sensible; guard offset/limit interpretation.Truthiness checks on offset/limit assume 1-based offsets. If offset could be 0, these branches may misbehave. Consider explicit undefined checks to avoid edge cases.
Apply this diff:
- if (readInput.offset && readInput.limit) { + if (readInput.offset !== undefined && readInput.limit !== undefined) { return ` L${readInput.offset}-${readInput.offset + readInput.limit - 1}`; } - if (!readInput.offset && readInput.limit) { + if (readInput.offset === undefined && readInput.limit !== undefined) { return ` L1-${readInput.limit}`; }And for the range payload:
- const range = - readInput.offset && readInput.limit + const range = + readInput.offset !== undefined && readInput.limit !== undefined ? { start: readInput.offset, end: readInput.offset + readInput.limit - 1, } : undefined;app/page.tsx (1)
25-25: Leverageloadingto prevent premature login redirects.Right now
loadingis destructured but never used. If WorkOS auth is still resolving, a quick submit could incorrectly redirect to /login. Gate the redirect on!loadingto avoid flicker and false negatives.Example adjustment inside handleSubmit:
// Only require authentication in WorkOS mode if (isWorkOSEnabled()) { if (loading) return; // wait for auth to resolve if (!user) { window.location.href = "/login"; return; } }lib/ai/tools/web-search.ts (2)
31-35: Validate non-empty queries.A zero-length query wastes a tool call. Enforce a minimum length.
- query: z - .string() + query: z + .string() + .min(1, "Query must not be empty")
84-90: Standardize return shape for success and error.Returning an array on success and a string on error forces downstream consumers to branch on types. Consider returning a consistent object like
{ results, error }.If you want to proceed, confirm the tool consumer(s) can handle the new shape. I can generate the change across call sites.
app/components/tools/TerminalToolHandler.tsx (4)
41-47: Preserve separation between stdout and stderr.Concatenating without a delimiter can blur outputs. Join with a newline to retain readability.
- const stdout = terminalOutput?.result?.stdout ?? ""; - const stderr = terminalOutput?.result?.stderr ?? ""; - const combinedOutput = stdout + stderr; + const stdout = terminalOutput?.result?.stdout ?? ""; + const stderr = terminalOutput?.result?.stderr ?? ""; + const combinedOutput = + [stdout, stderr].filter(Boolean).join("\n");
78-90: Add keyboard focusability to clickable ToolBlock (input-available).You pass onKeyDown but not tabIndex/role; without them, keyboard users may not be able to focus the element.
<ToolBlock key={toolCallId} icon={<Terminal />} action="Executing" target={terminalInput?.command || ""} isShimmer={true} isClickable={true} onClick={handleOpenInSidebar} onKeyDown={handleKeyDown} + tabIndex={0} + role="button" />
97-105: Add keyboard focusability to clickable ToolBlock (output-available).Same accessibility concern for the executed state.
<ToolBlock key={toolCallId} icon={<Terminal />} action={hasOutput ? "Executed" : "Command completed"} target={terminalInput?.command || ""} isClickable={true} onClick={handleOpenInSidebar} onKeyDown={handleKeyDown} + tabIndex={0} + role="button" />
70-75: Minor:keyprop on non-array children has no effect.These components aren’t rendered as list items, so
key={toolCallId}is unnecessary. Safe to remove.Also applies to: 80-85, 98-100
app/components/ComputerSidebar.tsx (1)
69-85: Fallback action label could be friendlier.Returning "Unknown action" (Line 83) is jarring in UI. Consider a neutral default like "Processing" to avoid confusing users.
- return "Unknown action"; + return "Processing";app/hooks/useSidebarAutoOpen.ts (2)
34-55: Nit: DefaultisBackgroundto booleanIf upstream changes ever omit
is_background, this can propagateundefined. Coercing to a boolean is safer and consistent with the prop type used by consumers.Apply this diff:
- isBackground: input.is_background, + isBackground: Boolean(input.is_background),
31-33: Prefer typed tool-part narrowing overanyUsing
anyfor tool parts erodes type safety in two hot paths. Consider introducing minimal discriminated unions for the terminal tool and data parts to avoid runtime surprises.For example, define these locally or in your types module and use them for narrowing:
type TerminalToolPart = { type: "tool-runTerminalCmd"; state: "input-available" | "output-available" | string; input?: { command?: string; is_background?: boolean }; toolCallId: string; }; type TerminalDataPart = { type: "data-terminal"; data?: { toolCallId?: string; terminal?: string }; };Then:
const toolPart = part as TerminalToolPart; // ... const terminalToolPart = (lastAssistantMessage.parts ?? []).find( (part): part is TerminalToolPart => (part as any).type === "tool-runTerminalCmd" && (part as any).toolCallId === sidebarContent.toolCallId, );Also applies to: 156-169
app/components/TerminalCodeBlock.tsx (1)
67-68: Cross-platform timeout type
NodeJS.Timeoutmay not match the browser typings depending on tsconfig libs. UseReturnType<typeof setTimeout>for universal compatibility.Apply this diff:
- const renderTimeoutRef = useRef<NodeJS.Timeout | null>(null); + const renderTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);app/contexts/GlobalState.tsx (1)
66-74: Guarded content merge is fine, but consider variant-safe updatesCurrent shallow merge can accidentally mix fields across union variants (e.g., add
outputto a file content). Not harmful at runtime, but type-safety and invariants can drift. Optionally narrow by current variant and whitelist fields.Apply this diff to narrow updates safely without importing extra types:
- const updateSidebarContent = (updates: Partial<SidebarContent>) => { - setSidebarContent((current) => { - if (current) { - return { ...current, ...updates }; - } - return current; - }); - }; + const updateSidebarContent = (updates: Partial<SidebarContent>) => { + setSidebarContent((current) => { + if (!current) return current; + // Discriminate on union by presence of 'path' + if ("path" in current) { + const { + path, + content, + range, + action, + // ignore terminal-only fields + command: _c, + output: _o, + isExecuting: _ie, + isBackground: _ib, + toolCallId: _tc, + ...rest + } = updates as any; + return { + ...current, + ...(path !== undefined ? { path } : {}), + ...(content !== undefined ? { content } : {}), + ...(range !== undefined ? { range } : {}), + ...(action !== undefined ? { action } : {}), + }; + } else { + const { + command, + output, + isExecuting, + isBackground, + toolCallId, + // ignore file-only fields + path: _p, + content: _fc, + range: _r, + action: _a, + ...rest + } = updates as any; + return { + ...current, + ...(command !== undefined ? { command } : {}), + ...(output !== undefined ? { output } : {}), + ...(isExecuting !== undefined ? { isExecuting } : {}), + ...(isBackground !== undefined ? { isBackground } : {}), + ...(toolCallId !== undefined ? { toolCallId } : {}), + }; + } + }); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.env.local.example(1 hunks)app/api/chat/route.ts(4 hunks)app/components/ComputerCodeBlock.tsx(1 hunks)app/components/ComputerSidebar.tsx(3 hunks)app/components/Footer.tsx(1 hunks)app/components/Header.tsx(1 hunks)app/components/MessagePartHandler.tsx(1 hunks)app/components/TerminalCodeBlock.tsx(4 hunks)app/components/tools/FileToolsHandler.tsx(4 hunks)app/components/tools/TerminalToolHandler.tsx(2 hunks)app/contexts/GlobalState.tsx(5 hunks)app/hooks/useSidebarAutoOpen.ts(6 hunks)app/page.tsx(4 hunks)lib/actions/index.ts(1 hunks)lib/ai/tools/index.ts(3 hunks)lib/ai/tools/web-search.ts(1 hunks)lib/auth/server.ts(1 hunks)package.json(1 hunks)types/agent.ts(2 hunks)types/chat.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (11)
app/components/Footer.tsx (1)
app/hooks/useAppAuth.ts (1)
useAppAuth(8-18)
lib/ai/tools/web-search.ts (1)
types/agent.ts (1)
ToolContext(15-20)
app/components/tools/TerminalToolHandler.tsx (2)
app/contexts/GlobalState.tsx (1)
useGlobalState(105-111)types/chat.ts (1)
SidebarTerminal(14-20)
app/components/Header.tsx (3)
components/icons/hackerai-svg.tsx (1)
HackerAISVG(8-67)lib/auth/client.ts (1)
isWorkOSEnabled(15-16)components/ui/button.tsx (1)
Button(59-59)
app/components/TerminalCodeBlock.tsx (1)
app/components/ShimmerText.tsx (1)
ShimmerText(12-39)
app/components/ComputerSidebar.tsx (4)
types/chat.ts (2)
isSidebarFile(24-28)isSidebarTerminal(30-34)components/ui/code-action-buttons.tsx (1)
CodeActionButtons(22-187)app/components/ComputerCodeBlock.tsx (1)
ComputerCodeBlock(19-165)app/components/TerminalCodeBlock.tsx (1)
TerminalCodeBlock(195-283)
app/page.tsx (2)
app/hooks/useAppAuth.ts (1)
useAppAuth(8-18)app/components/ChatInput.tsx (1)
ChatInput(28-183)
lib/ai/tools/index.ts (3)
types/chat.ts (1)
ChatMode(1-1)types/agent.ts (1)
ExecutionMode(22-22)lib/ai/tools/web-search.ts (1)
createWebSearchTool(10-93)
app/components/tools/FileToolsHandler.tsx (1)
app/contexts/GlobalState.tsx (1)
useGlobalState(105-111)
app/contexts/GlobalState.tsx (1)
types/chat.ts (1)
SidebarContent(22-22)
app/hooks/useSidebarAutoOpen.ts (2)
types/chat.ts (2)
SidebarContent(22-22)isSidebarFile(24-28)app/contexts/GlobalState.tsx (1)
useGlobalState(105-111)
🔇 Additional comments (27)
lib/auth/server.ts (3)
12-12: Docblock spacing change is fineThe extra blank line in the JSDoc improves readability and is harmless.
20-23: Good use of dynamic import to keep client bundle cleanDynamically importing "@workos-inc/authkit-nextjs" only when needed prevents accidental inclusion in client bundles. Nice.
6-8: No issues found with server-only import
Thelib/auth/client.tsmodule does not contain a"use client"directive and is explicitly marked as safe for both client and server use. You can keep theisWorkOSEnabledimport here without changes.app/components/MessagePartHandler.tsx (2)
21-30: Good call: render user text as plain textRendering user messages as plain text avoids unintended Markdown parsing of untrusted input and reduces XSS surface. The whitespace-pre-wrap choice preserves user-entered formatting nicely.
32-35: Raw HTML is safely escaped by default in MemoizedMarkdown
MemoizedMarkdown invokes ReactMarkdown without therehypeRawplugin, so any raw HTML in the input is escaped rather than rendered. It also opens links withtarget="_blank"andrel="noopener noreferrer". No additional sanitization configuration is required unless you intend to enable raw‐HTML support.types/agent.ts (1)
3-3: Type-only import is correct and safe for client bundles.Using
import type { Geo } from "@vercel/functions";ensures no runtime import leakage into client code.package.json (2)
29-29: Adding exa-js is appropriate for web search.Dependency looks good and aligns with the new tool. Keep it server-only to avoid inflating client bundles.
29-29: All exa-js imports and EXA_API_KEY references are confined to server-side modules
- lib/ai/tools/web-search.ts imports Exa from “exa-js” and reads process.env.EXA_API_KEY, and contains no “use client” directive.
- lib/ai/tools/index.ts conditionally wires up webSearch based on process.env.EXA_API_KEY, also in a server-only file.
No client components import exa-js or reference EXA_API_KEY. ✅
lib/actions/index.ts (1)
34-38: Provider headers look correct for OpenRouter-style metadata.Setting HTTP-Referer and X-Title at call time is aligned with provider requirements and mirrors the route handler.
.env.local.example (1)
9-12: Good addition: EXA_API_KEY template is clearly marked optional and not exposed client-side.This aligns with the server-gated web search tool and avoids accidental client-side exposure.
types/chat.ts (1)
14-22: Solid union modeling for sidebar content.Introducing SidebarTerminal and unifying via SidebarContent reads cleanly and aligns with the consuming components’ needs. Required fields on SidebarTerminal are sensible for streaming states, and SidebarFile remains backward compatible.
app/api/chat/route.ts (1)
33-33: Ensure geolocation(req) safely falls back on non-Vercel runtimesFile: app/api/chat/route.ts:33
We call:
const userLocation = geolocation(req);– this works on Vercel, but on other platforms you must confirm:
geolocation(req)never throws and returns a default object (e.g.{ country: null, region: null, ... }) when native geolocation isn’t available.- Downstream in
createTools(…, userLocation)(around lines 63–65) handles missing or null location fields without error.app/components/tools/FileToolsHandler.tsx (2)
11-11: API rename adoption looks correct.Switching to openSidebar aligns with the new SidebarContent model and keeps payload shape consistent for file content.
132-146: Good: opening written file in sidebar with action context.Click and keyboard handlers are both wired to openSidebar with the right payload. Nice a11y consideration with Enter/Space handling.
app/page.tsx (2)
119-124: Solid container height fix.Using h-full on the chat column aligns its height with the parent wrapper and prevents layout gaps when the sidebar toggles. Looks good.
165-193: Empty state UX improvements are clear and cohesive.Centering the ChatInput and adding Footer in the empty state creates a focused first-run experience without impacting the messages view.
lib/ai/tools/index.ts (1)
47-58: Conditional wiring of webSearch looks good.Gating on EXA_API_KEY and exposing webSearch in ask mode alongside readFile is sane and keeps the surface area minimal.
app/components/ComputerSidebar.tsx (2)
16-20: Unified sidebar state integration is clean.Switching to
sidebarContentwith type guards is a solid improvement and removes file-only assumptions.
176-193: Header and content composition for file/terminal are well-factored.
- Left title area switching between filename and Terminal icon is intuitive.
- CodeActionButtons content, filename, and language derivation for terminal vs file are sensible.
- TerminalCodeBlock’s sidebar variant use aligns with the new sidebar UX.
No changes requested.
Also applies to: 197-214, 233-257
app/hooks/useSidebarAutoOpen.ts (4)
141-151: Good guardrails for terminal streaming updatesNice gating: you exit early when there’s no content, it’s a file, the terminal isn’t executing, or the status isn’t streaming. This prevents unnecessary work and avoids accidental updates to file-type sidebars.
76-85: LGTM: readFile mapping and range calculationStrips line numbers, computes inclusive end, and marks action as reading. Looks correct and consistent.
91-99: LGTM: writeFile sidebar payloadWrites the target path and contents with an explicit "writing" action. Matches the new SidebarContent shape.
124-139: Viewport-gated auto-open is prudentOnly opening the sidebar on >= 950px avoids jarring layout shifts on small screens. The SSR guard on
windowis also correct.app/components/TerminalCodeBlock.tsx (2)
215-259: Sidebar vs default variants are cleanly separatedGood separation of concerns. Default variant renders a full terminal UI with header; sidebar variant renders a compact, full-bleed ANSI block. The “executing” shimmer conditions are coherent.
101-107: Verify bundling of the “houston” theme and ANSI grammarI ran a search across the repo and didn’t find any explicit imports or Shiki config that would pre-bundle the “houston” theme or the “ansi” grammar. Without those assets registered up front, each call to
codeToHtml(code, { lang: "ansi", theme: "houston" })may fail, fall back to unstyled plain text, and spam the console—hurting both performance and user experience.Please verify and, if needed, add explicit theme/grammar registration, for example:
- Import the theme JSON so your bundler includes it:
import houston from "shiki/themes/houston.json";- Register or preload the ANSI grammar:
import ansi from "shiki/languages/ansi.tmLanguage.json";- Then initialize your highlighter once, e.g.:
const highlighter = await getHighlighter({ theme: houston, langs: [ansi], });- Or adjust your build/plugin settings to bundle all Shiki themes and grammars you’ll use.
That will prevent repeated load errors and fallback renders at runtime.
app/contexts/GlobalState.tsx (2)
61-64: openSidebar transition is correctSetting content then opening the sidebar ensures consumers can render immediately without a flash of empty content.
80-96: Global state API surface looks goodThe migration from file-based to content-based sidebar state (openSidebar/updateSidebarContent/sidebarContent) is coherent and matches the hook and component changes in this PR.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
app/components/tools/WebSearchToolHandler.tsx (4)
4-7: Type ‘part’ precisely instead of any.Using any hides shape issues at compile time. Define a dedicated WebSearchPart type to catch mismatches early.
Apply this diff to tighten the prop type:
interface WebSearchToolHandlerProps { - part: any; + part: WebSearchPart; status: "ready" | "submitted" | "streaming" | "error"; }Add these types above the interface:
type WebSearchState = "input-streaming" | "input-available" | "output-available"; interface WebSearchInput { query: string; explanation?: string; } interface WebSearchPart { toolCallId: string; state: WebSearchState; input?: WebSearchInput; }
13-17: Guard against missing or non-string input.query.If input is undefined or query is not a string, target={webInput.query} could pass an unexpected value. Be defensive and normalize to string | undefined.
Apply this diff:
- const { toolCallId, state, input } = part; - const webInput = input as { - query: string; - explanation?: string; - }; + const { toolCallId, state, input } = part; + const webInput = (input ?? {}) as Partial<{ + query: string; + explanation?: string; + }>; + const query = typeof webInput.query === "string" ? webInput.query.trim() : undefined;- action="Searching web" - target={webInput.query} + action="Searching web" + target={query}- action="Searched web" - target={webInput.query} + action="Searched web" + target={query}Also applies to: 35-36, 45-46
19-50: Reduce repetition and optionally handle error status.The three branches repeat identical props (key, icon). Consider extracting baseProps and collapsing conditions. Also, a simple error-state block could improve UX.
Example refactor:
const baseProps = { key: toolCallId, icon: <Search /> }; if (status === "error") { return <ToolBlock {...baseProps} action="Web search failed" target={query} />; } if (state === "input-streaming" && status === "streaming") { return <ToolBlock {...baseProps} action="Searching web" isShimmer />; } if (state === "input-available" && status === "streaming") { return <ToolBlock {...baseProps} action="Searching web" target={query} isShimmer />; } if (state === "output-available") { return <ToolBlock {...baseProps} action="Searched web" target={query} />; } return null;
25-25: User-facing copy: minor grammar improvement.“Searching the web” / “Searched the web” reads more naturally.
Apply this diff:
- action="Searching web" + action="Searching the web"- action="Searching web" + action="Searching the web"- action="Searched web" + action="Searched the web"Also applies to: 34-34, 44-44
app/components/MessagePartHandler.tsx (1)
22-33: Consider word-break utilities for long unbroken user input.Rare but possible: very long tokens (e.g., pasted hashes/URLs) can overflow. Add break-words or break-all if this becomes an issue.
Example:
- <div key={partId} className="whitespace-pre-wrap"> + <div key={partId} className="whitespace-pre-wrap break-words">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/components/MessagePartHandler.tsx(3 hunks)app/components/tools/WebSearchToolHandler.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/components/MessagePartHandler.tsx (1)
app/components/tools/WebSearchToolHandler.tsx (1)
WebSearchToolHandler(9-51)
🔇 Additional comments (6)
app/components/tools/WebSearchToolHandler.tsx (2)
39-47: LGTM: Output rendering is clear.The final, non-shimmer ToolBlock with action and target communicates the result succinctly.
1-2: ToolBlock import path validatedThe file
components/ui/tool-block.tsxexists with the exact lowercase name, matching the import inapp/components/tools/WebSearchToolHandler.tsx. Alias resolution is configured correctly and there are no casing mismatches—no changes required.app/components/MessagePartHandler.tsx (4)
5-5: LGTM: Added web-search handler import.Import path looks correct relative to this file.
22-33: Good call rendering user text as plain text.Avoiding Markdown for user messages reduces XSS/formatting surprises and keeps UX consistent for user-authored content.
51-53: LGTM: Added case for tool-webSearch.The handler wiring aligns with the new tool UI component.
51-53: Validation complete: “tool-webSearch” part is correctly emitted
- In lib/ai/tools/index.ts the factory spreads
{ webSearch: createWebSearchTool(context) }.- In app/components/MessagePartHandler.tsx there is a
case "tool-webSearch"branch rendering<WebSearchToolHandler/>.The tool key (
webSearch) and the case label (tool-webSearch) align—no gaps detected.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/page.tsx (1)
11-11: Footer import and conditional rendering LGTM; optional semantic tweakImporting and placing
Footerin the empty-state area works, and the component self-gates on auth state. Optionally, wrap it in a semanticfootertag.Apply this small semantic tweak:
- <div className="flex-shrink-0"> - <Footer /> - </div> + <footer className="flex-shrink-0"> + <Footer /> + </footer>Also applies to: 193-197
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
app/components/Footer.tsx(1 hunks)app/components/Header.tsx(1 hunks)app/components/MessagePartHandler.tsx(3 hunks)app/components/tools/WebSearchToolHandler.tsx(1 hunks)app/page.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- app/components/MessagePartHandler.tsx
- app/components/Footer.tsx
- app/components/tools/WebSearchToolHandler.tsx
- app/components/Header.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/page.tsx (3)
app/hooks/useAppAuth.ts (1)
useAppAuth(8-18)app/components/MemoizedMarkdown.tsx (2)
h1(26-32)p(89-93)app/components/ChatInput.tsx (1)
ChatInput(28-183)
🔇 Additional comments (3)
app/page.tsx (3)
110-117: Empty-state header placement LGTMWrapping the Header in a
flex-shrink-0container and showing it only in the empty state is a clean layout separation. No issues spotted.
118-126: Good flex and overflow guardsThe switch to
flex max-w-full flex-1 min-h-0and addingh-fullplusmin-w-0on the chat panel addresses nested scrolling and overflow clipping. Solid layout hardening.Also applies to: 121-121
167-190: Centered empty-state UX looks solidThe centered title + subtitle and
ChatInputwithisCenteredprovide a clean first-use experience. Structure and spacing look good.
Summary by CodeRabbit
New Features
Refactor
Style
Documentation
Chores