-
Notifications
You must be signed in to change notification settings - Fork 51
feat: computer sidebar UI #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a persistent sidebar file viewer and global sidebar state; extracts code action controls into a reusable CodeActionButtons component and adds ComputerCodeBlock and ToolBlock UI components; normalizes assistant messages (terminal aggregation); updates sandbox/terminal defaults, AI provider wiring, streaming, and responsive page layout. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MsgPart as MessagePartHandler
participant Global as GlobalState
participant Page
participant Sidebar as ComputerSidebar
participant CodeButtons as CodeActionButtons
participant CodeBlock as ComputerCodeBlock
User->>MsgPart: Click ToolBlock (file read/write)
MsgPart->>Global: openFileInSidebar({path, content, language, range, action})
Global-->>Page: sidebarOpen=true, sidebarFile set
Page->>Sidebar: Render with sidebarFile
Sidebar->>CodeButtons: Render (content, filename, language, isWrapped)
Sidebar->>CodeBlock: Render (language, wrap)
User->>CodeButtons: Toggle wrap / Copy / Download
CodeButtons-->>Sidebar: onToggleWrap updates isWrapped
sequenceDiagram
participant User
participant Buttons as CodeActionButtons
participant Clipboard as navigator.clipboard
participant FS as FileSystemAccess
participant Browser as BlobFallback
participant Toast as toast
User->>Buttons: Click Copy
Buttons->>Clipboard: writeText(content.trim())
Clipboard-->>Buttons: success
Buttons->>Toast: show "Copied"
User->>Buttons: Click Download
alt FS API available
Buttons->>FS: showSaveFilePicker + write file
FS-->>Buttons: success
Buttons->>Toast: show "Downloaded"
else Fallback
Buttons->>Browser: create Blob + anchor click
Browser-->>Buttons: success
Buttons->>Toast: show "Downloaded"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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. ✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
app/components/MessagePartHandler.tsx (1)
39-47: Fix line-range math for offset/limit and ensure consistent sidebar highlighting.Truthiness checks break when offset is 0 or undefined. Also, when only limit is provided, the sidebar currently gets no range. Compute ranges with explicit numeric checks and default start line to 1.
- const getFileRange = () => { - if (readInput.offset && readInput.limit) { - return ` L${readInput.offset}-${readInput.offset + readInput.limit - 1}`; - } - if (!readInput.offset && readInput.limit) { - return ` L1-${readInput.limit}`; - } - return ""; - }; + const getFileRange = () => { + const hasLimit = + typeof readInput.limit === "number" && readInput.limit > 0; + const hasOffset = + typeof readInput.offset === "number" && readInput.offset > 0; + if (!hasLimit) return ""; + const start = hasOffset ? readInput.offset! : 1; + const end = start + readInput.limit! - 1; + return ` L${start}-${end}`; + }; ... - const handleOpenInSidebar = () => { - const cleanContent = readOutput.result.replace(/^\s*\d+\|/gm, ""); - const range = - readInput.offset && readInput.limit - ? { - start: readInput.offset, - end: readInput.offset + readInput.limit - 1, - } - : undefined; - - openFileInSidebar({ - path: readInput.target_file, - content: cleanContent, - range, - action: "reading", - }); - }; + const handleOpenInSidebar = () => { + const cleanContent = readOutput.result.replace(/^\s*\d+\|/gm, ""); + const hasLimit = + typeof readInput.limit === "number" && readInput.limit > 0; + const hasOffset = + typeof readInput.offset === "number" && readInput.offset > 0; + const start = hasOffset ? readInput.offset! : 1; + const range = hasLimit + ? { start, end: start + readInput.limit! - 1 } + : undefined; + + openFileInSidebar({ + path: readInput.target_file, + content: cleanContent, + range, + action: "reading", + }); + };Also applies to: 79-95
🧹 Nitpick comments (15)
app/components/MessageActions.tsx (2)
64-64: Avoid implicit form submission and hide spacers from screen readers.
- Add type="button" to the Copy button to prevent accidental form submissions if used within a form.
- Mark invisible spacers as aria-hidden to avoid confusing screen readers.
- <button + <button + type="button" onClick={handleCopy} className="p-1.5 opacity-70 hover:opacity-100 transition-opacity rounded hover:bg-secondary text-muted-foreground" aria-label={copied ? "Copied!" : "Copy message"} >- <div className="p-1.5 w-7 h-7" /> + <div className="p-1.5 w-7 h-7" aria-hidden="true" /> {!isUser && isLastAssistantMessage && ( - <div className="p-1.5 w-7 h-7" /> + <div className="p-1.5 w-7 h-7" aria-hidden="true" /> )}Also applies to: 97-101
37-43: Prevent setState after unmount by cleaning up the copy timeout.The setTimeout created in handleCopy can fire after unmount and trigger a React warning. Track and clear it on unmount.
Outside the changed lines, apply:
// 1) Update imports // import { useState } from "react"; import { useState, useRef, useEffect } from "react"; // 2) Add a ref to store the timeout id const copyTimeoutRef = useRef<number | null>(null); // 3) Use it in handleCopy if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current); copyTimeoutRef.current = window.setTimeout(() => setCopied(false), 2000); // 4) Cleanup on unmount useEffect(() => { return () => { if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current); }; }, []);components/ui/code-action-buttons.tsx (2)
35-41: Preserve exact code when copying; avoid trimming.Trimming can drop meaningful trailing newlines/spaces from code. Copy the content as-is.
- await navigator.clipboard.writeText(content.trim()); + await navigator.clipboard.writeText(content);
89-93: Remove redundant ternary in success toast.Both branches return the same string; simplify for readability.
- toast.success( - variant === "sidebar" - ? "File downloaded successfully" - : "File downloaded successfully", - ); + toast.success("File downloaded successfully");app/components/CodeHighlight.tsx (2)
26-28: Use functional state update for wrap toggleAvoids potential stale-state issues when toggling quickly or in concurrent renders.
- const handleToggleWrap = () => { - setIsWrapped(!isWrapped); - }; + const handleToggleWrap = () => { + setIsWrapped((prev) => !prev); + };
12-17: Prop className is parsed for language but not applied to the DOMclassName is consumed to detect language and then discarded. If external className styling was relied upon by callers, this is a silent break. Consider applying it to the wrapper for non-inline blocks and to the inline code element.
- return !isInline ? ( - <div className="shiki not-prose relative rounded-lg bg-card border border-border my-2 overflow-hidden"> + return !isInline ? ( + <div className={`shiki not-prose relative rounded-lg bg-card border border-border my-2 overflow-hidden ${className ?? ""}`}> {/* Menu bar */} ... </div> ) : ( - <code - className="bg-muted text-muted-foreground px-1.5 py-0.5 rounded text-sm font-mono" + <code + className={`bg-muted text-muted-foreground px-1.5 py-0.5 rounded text-sm font-mono ${className ?? ""}`} {...props} > {children} </code> );Also applies to: 31-41, 73-79
app/components/ScrollToBottomButton.tsx (1)
15-21: Prevent unintended form submissionAdd type="button" to ensure this control never submits a surrounding form if reused within one.
return ( <button onClick={onClick} + type="button" className="bg-background border border-border rounded-full p-2 shadow-lg hover:shadow-xl transition-all duration-200 hover:scale-105 flex items-center justify-center" aria-label="Scroll to bottom" tabIndex={0} > <ChevronDown className="w-4 h-4 text-muted-foreground" /> </button> );types/chat.ts (1)
3-12: Type shape looks good; clarify range semantics and reuse action unionThe SidebarFile interface is clear. Two small improvements:
- Document whether range.start/end are 0- or 1-based and whether end is inclusive or exclusive.
- Extract the action union to a named type to keep it reusable and avoid typos elsewhere.
-export interface SidebarFile { +export type SidebarAction = "reading" | "creating" | "editing" | "writing"; + +/** + * Range semantics: specify whether 0- or 1-based and if `end` is inclusive or exclusive. + */ +export interface SidebarFile { path: string; content: string; language?: string; range?: { start: number; end: number; }; - action?: "reading" | "creating" | "editing" | "writing"; + action?: SidebarAction; }app/page.tsx (2)
101-107: Avoid RefObject casts by typing the hookCasting scrollRef/contentRef to RefObject<HTMLDivElement | null> hints a typing gap. Prefer typing useMessageScroll to return correctly typed refs to remove the need for casts.
If feasible, update useMessageScroll() to:
- Return refs typed as RefObject
- Or accept a generic param to control the element type
Then remove the as RefObject casts here.
18-30: Minor duplication of “mode” in transport and sendMessage bodyYou provide mode in both the DefaultChatTransport body and sendMessage options. It’s harmless, but one source of truth is preferable.
Consider relying on the transport’s body only and removing the per-call body unless you need to override per message.
Also applies to: 42-51
app/contexts/GlobalState.tsx (2)
58-66: Stabilize handlers with useCallback to reduce context churnWithout memoization, openFileInSidebar/closeSidebar identities change every render, potentially rerendering all consumers of useGlobalState. Memoize them.
-import React, { createContext, useContext, useState, ReactNode } from "react"; +import React, { createContext, useContext, useState, ReactNode, useCallback, useMemo } from "react"; ... - const openFileInSidebar = (file: SidebarFile) => { - setSidebarFile(file); - setSidebarOpen(true); - }; + const openFileInSidebar = useCallback((file: SidebarFile) => { + setSidebarFile(file); + setSidebarOpen(true); + }, []); - const closeSidebar = () => { - setSidebarOpen(false); - setSidebarFile(null); - }; + const closeSidebar = useCallback(() => { + setSidebarOpen(false); + setSidebarFile(null); + }, []);
68-83: Memoize the context valueProviding a new object each render forces all context consumers to re-render. Memoize value with useMemo using the relevant dependencies.
- const value: GlobalStateType = { - input, - setInput, - mode, - setMode, - chatTitle, - setChatTitle, - sidebarOpen, - setSidebarOpen, - sidebarFile, - setSidebarFile, - clearInput, - resetChat, - openFileInSidebar, - closeSidebar, - }; + const value: GlobalStateType = useMemo( + () => ({ + input, + setInput, + mode, + setMode, + chatTitle, + setChatTitle, + sidebarOpen, + setSidebarOpen, + sidebarFile, + setSidebarFile, + clearInput, + resetChat, + openFileInSidebar, + closeSidebar, + }), + [ + input, + mode, + chatTitle, + sidebarOpen, + sidebarFile, + clearInput, + resetChat, + openFileInSidebar, + closeSidebar, + ], + );app/components/ComputerSidebar.tsx (2)
64-72: Harden action text mapping with a safe default.If
sidebarFile.actioncontains an unexpected value, the current code returnsundefined. Add a fallback string to avoid UI glitches.const getActionText = (): string => { const actionMap = { reading: "Reading file", creating: "Creating file", editing: "Editing file", writing: "Writing file", }; - return actionMap[sidebarFile.action || "reading"]; + return actionMap[sidebarFile.action || "reading"] ?? "Reading file"; };
120-133: DRY: computefileNameonce and reuse (reduce repeatedsplit("/").pop()).Cleaner and faster, avoids multiple splits and keeps filename display consistent.
- const getActionText = (): string => { + const getActionText = (): string => { ... }; + const fileName = + sidebarFile.path.split("/").pop() || sidebarFile.path; + const handleClose = () => { closeSidebar(); }; ... <div title={`${getActionText()} ${sidebarFile.path}`} className="max-w-[100%] w-[max-content] truncate text-[13px] rounded-full inline-flex items-center px-[10px] py-[3px] border border-border bg-muted/30 text-foreground" > {getActionText()} <span className="flex-1 min-w-0 px-1 ml-1 text-[12px] font-mono max-w-full text-ellipsis overflow-hidden whitespace-nowrap text-muted-foreground"> <code> - {sidebarFile.path.split("/").pop() || sidebarFile.path} + {fileName} </code> </span> </div> ... <div className="max-w-[250px] truncate text-muted-foreground text-sm font-medium"> - {sidebarFile.path.split("/").pop() || sidebarFile.path} + {fileName} </div> </div> ... <CodeActionButtons content={sidebarFile.content} - filename={sidebarFile.path.split("/").pop() || "code.txt"} + filename={fileName || "code.txt"} language={ sidebarFile.language || getLanguageFromPath(sidebarFile.path) } isWrapped={isWrapped} onToggleWrap={handleToggleWrap} variant="sidebar" /> ... <ComputerCodeBlock language={ sidebarFile.language || getLanguageFromPath(sidebarFile.path) } wrap={isWrapped} showButtons={false} > - {sidebarFile.content} + {sidebarFile.content} </ComputerCodeBlock>Also applies to: 143-146, 150-156, 175-185
app/components/ComputerCodeBlock.tsx (1)
90-144: Deduplicate action UI by reusing CodeActionButtons (optional).You already have a shared CodeActionButtons component. Reusing it here eliminates duplicate logic for copy/download/wrap, unifies UX, and reduces maintenance.
+import { CodeActionButtons } from "@/components/ui/code-action-buttons"; ... return ( <div className="shiki not-prose relative h-full w-full bg-transparent overflow-hidden"> {/* Floating action buttons - only show if showButtons is true */} {showButtons && ( - <div className="absolute top-1 right-0 z-10 pl-1 pr-2"> - <div className="backdrop-blur-sm inline-flex h-7 items-center rounded-lg bg-muted/80 p-0.5 border border-border/50"> - <Tooltip> - <TooltipTrigger asChild> - <button - onClick={handleDownload} - className="inline-flex items-center justify-center rounded-md px-3 py-1 text-xs transition-colors text-muted-foreground hover:bg-background hover:text-foreground" - aria-label="Download" - > - <Download size={12} /> - </button> - </TooltipTrigger> - <TooltipContent>Download</TooltipContent> - </Tooltip> - - <Tooltip> - <TooltipTrigger asChild> - <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" - } - > - <WrapText size={12} /> - </button> - </TooltipTrigger> - <TooltipContent> - {isWrapped ? "Disable text wrapping" : "Enable text wrapping"} - </TooltipContent> - </Tooltip> - - <Tooltip> - <TooltipTrigger asChild> - <button - onClick={handleCopy} - className="inline-flex items-center justify-center rounded-md px-3 py-1 text-xs transition-colors text-muted-foreground hover:bg-background hover:text-foreground" - aria-label={copied ? "Copied!" : "Copy"} - > - {copied ? <Check size={12} /> : <Copy size={12} />} - </button> - </TooltipTrigger> - <TooltipContent>{copied ? "Copied!" : "Copy"}</TooltipContent> - </Tooltip> - </div> - </div> + <div className="absolute top-1 right-0 z-10 pl-1 pr-2"> + <CodeActionButtons + content={codeContent} + language={language} + isWrapped={isWrapped} + onToggleWrap={handleToggleWrap} + variant="codeblock" + /> + </div> )}Note: After this change, you can remove the now-unused imports for icons, Tooltip, and toast from this file.
📜 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 settings in your CodeRabbit configuration.
📒 Files selected for processing (13)
app/components/CodeHighlight.tsx(2 hunks)app/components/ComputerCodeBlock.tsx(1 hunks)app/components/ComputerSidebar.tsx(1 hunks)app/components/MessageActions.tsx(1 hunks)app/components/MessagePartHandler.tsx(4 hunks)app/components/ScrollToBottomButton.tsx(1 hunks)app/contexts/GlobalState.tsx(4 hunks)app/page.tsx(2 hunks)components/ui/code-action-buttons.tsx(1 hunks)components/ui/tool-block.tsx(1 hunks)lib/ai/providers.ts(1 hunks)lib/ai/tools/run-terminal-cmd.ts(1 hunks)types/chat.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
app/components/ComputerSidebar.tsx (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(92-98)components/ui/code-action-buttons.tsx (1)
CodeActionButtons(22-176)app/components/ComputerCodeBlock.tsx (1)
ComputerCodeBlock(19-165)
app/contexts/GlobalState.tsx (1)
types/chat.ts (1)
SidebarFile(3-12)
components/ui/tool-block.tsx (1)
app/components/ShimmerText.tsx (1)
ShimmerText(12-39)
app/components/MessageActions.tsx (1)
components/ui/tooltip.tsx (3)
Tooltip(61-61)TooltipTrigger(61-61)TooltipContent(61-61)
components/ui/code-action-buttons.tsx (2)
app/components/MemoizedMarkdown.tsx (1)
a(69-87)components/ui/tooltip.tsx (3)
Tooltip(61-61)TooltipTrigger(61-61)TooltipContent(61-61)
app/components/CodeHighlight.tsx (1)
components/ui/code-action-buttons.tsx (1)
CodeActionButtons(22-176)
app/components/MessagePartHandler.tsx (1)
app/contexts/GlobalState.tsx (1)
useGlobalState(92-98)
app/page.tsx (5)
app/contexts/GlobalState.tsx (1)
useGlobalState(92-98)app/components/Messages.tsx (1)
Messages(16-129)app/components/ChatInput.tsx (1)
ChatInput(28-183)app/components/ScrollToBottomButton.tsx (1)
ScrollToBottomButton(8-24)app/components/ComputerSidebar.tsx (1)
ComputerSidebar(13-195)
🔇 Additional comments (10)
app/components/CodeHighlight.tsx (2)
44-50: Good extraction: actions centralized via CodeActionButtonsReplacing bespoke copy/download/wrap UI with the shared CodeActionButtons improves consistency and reduces duplication. Props passed (content, language, isWrapped, onToggleWrap, variant="codeblock") look correct.
44-50: Possible UX regression: no toast on “Copy” for code blocksCodeActionButtons only toasts on copy when variant === "sidebar". With variant="codeblock", users will no longer see toast feedback (only tooltip state). If prior behavior included toasts here, confirm the change is intentional. If not, consider adding an option (e.g., showToasts) or extending the codeblock variant to optionally toast.
app/components/ScrollToBottomButton.tsx (1)
17-17: Decoupling positioning is correctDropping fixed/centering classes here is appropriate given Page now positions the wrapper responsively. Keeps this component reusable.
app/page.tsx (4)
145-150: Verify alignment math for centered button with sidebar openUsing left-1/4 with -translate-x-1/2 does center the button within the left half on md+. Just confirm this matches your design across breakpoints, especially when the left column width changes in the future.
68-79: Responsive split layout reads wellThe left pane width behavior (full on mobile, half on md+ when sidebar is open) is clear and uses minimal utility classes. Nice.
160-167: Desktop sidebar integration is correctConditionally rendering ComputerSidebar with md:w-1/2 complements the left pane and keeps the DOM lean when the sidebar is closed.
171-177: Mobile overlay behavior looks solidmd:hidden overlay with z-50 prevents interaction with the chat area and contains the sidebar content correctly.
app/contexts/GlobalState.tsx (2)
46-48: State additions for the sidebar look correctsidebarOpen and sidebarFile are initialized properly and align with the new interface.
53-57: Consider whether resetChat should also close the sidebarIf “reset chat” implies returning to a clean slate, you may want to close the sidebar and clear the file as well. If not, ignore.
app/components/MessagePartHandler.tsx (1)
121-172: LGTM on ToolBlock integration and sidebar wiring.Nice consolidation into ToolBlock, accessible keyboard handling, and consistent use of
openFileInSidebar. The UX improvements are clear.Also applies to: 174-199
- Add message processor to transform incomplete terminal commands - Convert tool-runTerminalCmd from input-available to output-available state - Collect terminal output from data-terminal parts and populate stdout - Remove data-terminal parts to clean up message structure - Set exit code 130 and stderr message for interrupted commands - Process messages in frontend before sending to API for better performance Fixes issue where incomplete terminal commands caused UI rendering problems and improves message structure consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
app/components/Messages.tsx (1)
80-89: Guard against undefined message.parts to avoid runtime errorsYou use optional chaining above but assume parts is always present here. Add a fallback to an empty array to be defensive.
Apply this diff:
- {message.parts.map((part, partIndex) => ( + {(message.parts ?? []).map((part, partIndex) => (app/page.tsx (1)
51-58: Avoid duplicating request body fields between prepareSendMessagesRequest and sendMessageYou already set mode (and messages) in prepareSendMessagesRequest. Passing body again in sendMessage is redundant and risks divergence depending on merging semantics.
Apply this diff:
- sendMessage( - { text: input }, - { - body: { - mode, - }, - }, - ); + sendMessage({ text: input });
♻️ Duplicate comments (1)
app/page.tsx (1)
154-159: Likely invalid Tailwind class “bottom-34” (use a valid token or an arbitrary value)Tailwind’s default spacing scale doesn’t include 34, so bottom-34 will be ignored. Use bottom-36 or an arbitrary value like bottom-[136px].
Apply this diff:
- <div - className={`fixed bottom-34 z-40 transition-all duration-300 ${ + <div + className={`fixed bottom-36 md:bottom-36 z-40 transition-all duration-300 ${ sidebarOpen ? "left-1/2 md:left-1/4 -translate-x-1/2" // Center of full screen on mobile, center of left half on desktop : "left-1/2 -translate-x-1/2" // Center of full screen when sidebar is closed }`} >
🧹 Nitpick comments (4)
app/components/Messages.tsx (1)
52-58: Align the inline part type with optional chaining usageYou’re using optional chaining on part.type but the inline type declares type as required. Make the annotation optional and simplify the text check.
Apply this diff:
- const hasTextContent = message.parts?.some( - (part: { type: string; text?: string }) => - (part.type === "text" && part.text && part.text.trim() !== "") || - part.type === "step-start" || - part.type?.startsWith("tool-"), - ); + const hasTextContent = message.parts?.some( + (part: { type?: string; text?: string }) => + (part.type === "text" && !!part.text?.trim()) || + part.type === "step-start" || + part.type?.startsWith("tool-"), + );lib/utils/message-processor.ts (2)
1-1: Use a single UIMessage type source across the appOther components import UIMessage from @ai-sdk/react. Importing from "ai" here risks subtle type drift over time. Align the import to avoid mismatches.
Apply this diff:
-import { UIMessage } from "ai"; +import { UIMessage } from "@ai-sdk/react";
7-11: Relax TerminalToolPart.input typing to match real-world payloadsexplanation and is_background are not guaranteed. Making them optional prevents unnecessary narrowing or casts.
Apply this diff:
input?: { command: string; - explanation: string; - is_background: boolean; + explanation?: string; + is_background?: boolean; };app/api/chat/route.ts (1)
45-47: Clarify comment about truncation vs. frontend processingThe server still truncates despite the note that processing moved to the frontend. Clarify that server-side truncation remains a safety net.
Apply this diff:
- // Truncate messages to stay within token limit (processing is now done on frontend) + // Truncate messages server-side as a safety net; frontend normalization/truncation also runs
📜 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 settings in your CodeRabbit configuration.
📒 Files selected for processing (5)
app/api/chat/route.ts(4 hunks)app/components/Messages.tsx(1 hunks)app/page.tsx(2 hunks)lib/ai/providers.ts(1 hunks)lib/utils/message-processor.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ai/providers.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/page.tsx (6)
app/contexts/GlobalState.tsx (1)
useGlobalState(92-98)lib/utils/message-processor.ts (1)
normalizeMessages(33-104)app/components/Messages.tsx (1)
Messages(16-131)app/components/ChatInput.tsx (1)
ChatInput(28-183)app/components/ScrollToBottomButton.tsx (1)
ScrollToBottomButton(8-24)app/components/ComputerSidebar.tsx (1)
ComputerSidebar(13-195)
🔇 Additional comments (2)
app/components/Messages.tsx (1)
55-57: Loader gating logic correctly accounts for step/tool partsIncluding step-start and tool-* parts in hasTextContent prevents premature loaders. Optional chaining on type is a safe guard.
app/page.tsx (1)
22-32: Good call to normalize messages on the client before sendingPreparing the request with normalized messages and mode in prepareSendMessagesRequest keeps the transport concerns centralized and consistent (also covers regenerate flows).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (3)
app/components/ComputerSidebar.tsx (2)
21-62: Use a Shiki-compatible fallback ("plaintext") and map .txt; also map TSX/JSX correctlyShiki doesn’t recognize "text". Using "plaintext" avoids load errors and renders unhighlighted text reliably. Mapping tsx/jsx to their respective grammars preserves JSX syntax highlighting.
const getLanguageFromPath = (filePath: string): string => { const extension = filePath.split(".").pop()?.toLowerCase() || ""; const languageMap: Record<string, string> = { - js: "javascript", - jsx: "javascript", - ts: "typescript", - tsx: "typescript", + js: "javascript", + jsx: "jsx", + ts: "typescript", + tsx: "tsx", + txt: "plaintext", py: "python", rb: "ruby", go: "go", rs: "rust", java: "java", c: "c", cpp: "cpp", h: "c", hpp: "cpp", css: "css", scss: "scss", sass: "sass", html: "html", xml: "xml", json: "json", yaml: "yaml", yml: "yaml", md: "markdown", sh: "bash", bash: "bash", zsh: "bash", fish: "bash", sql: "sql", php: "php", swift: "swift", kt: "kotlin", scala: "scala", clj: "clojure", hs: "haskell", elm: "elm", vue: "vue", svelte: "svelte", }; - return languageMap[extension] || "text"; + return languageMap[extension] || "plaintext"; };
74-83: Escape currently only works when the minimize button is focused; add a global listenerAttach a window-level keydown listener so Escape works regardless of focus, and clean up on unmount.
const handleClose = () => { closeSidebar(); }; + // Close on Escape regardless of focus + React.useEffect(() => { + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === "Escape") { + closeSidebar(); + } + }; + window.addEventListener("keydown", onKeyDown); + return () => window.removeEventListener("keydown", onKeyDown); + }, [closeSidebar]);Note: Using React.useEffect avoids adding a new named import.
app/api/chat/route.ts (1)
83-92: Resolved: Sandbox is now paused on stream errors.The onError handler mirrors onFinish and pauses the sandbox to prevent leaks. This addresses the earlier review note.
🧹 Nitpick comments (8)
README.md (2)
35-38: Specify fenced code block language to satisfy markdownlint (MD040).Add a language to the code fence for the env var snippet.
Apply this diff:
- ``` + ```env NEXT_PUBLIC_AGENT_MODEL=qwen/qwen3-coder NEXT_PUBLIC_TITLE_MODEL=qwen/qwen-turbo ```
34-38: Clarify that NEXT_PUBLIC_ env vars are client-exposed (avoid secrets).*These variables are bundled client-side; a brief note helps prevent accidental secret leakage.
Apply this diff:
- - Optionally customize the AI models: + - Optionally customize the AI models: + - Note: Variables prefixed with NEXT_PUBLIC_ are exposed to the client and must not contain secrets.app/components/ChatInput.tsx (1)
45-59: Consider adding Escape as an additional stop shortcut (non-destructive)On Windows, Ctrl+C is a common copy shortcut; intercepting it during generation can be surprising. Adding Esc provides an alternative stop gesture without removing Ctrl+C for power users.
Apply this diff to add Esc support alongside Ctrl+C:
// Handle keyboard shortcuts for stopping generation useHotkeys( - "ctrl+c", + "ctrl+c", (e) => { e.preventDefault(); onStop(); }, { enabled: isGenerating, enableOnFormTags: true, enableOnContentEditable: true, preventDefault: true, description: "Stop AI generation", }, [isGenerating, onStop], ); + + // Optional: also allow Escape to stop generation + useHotkeys( + "esc", + (e) => { + e.preventDefault(); + onStop(); + }, + { + enabled: isGenerating, + enableOnFormTags: true, + enableOnContentEditable: true, + preventDefault: true, + description: "Stop AI generation", + }, + [isGenerating, onStop], + );app/components/ComputerSidebar.tsx (1)
139-161: Derive filename once to avoid repeated split/computeYou compute the filename from the path multiple times. Pull it into a constant for readability and to avoid repetitive work.
Add this near the top after the early return:
- if (!sidebarOpen || !sidebarFile) { + if (!sidebarOpen || !sidebarFile) { return null; } + + const filename = sidebarFile.path.split("/").pop() || sidebarFile.path;And apply it here:
- <div className="max-w-[250px] truncate text-muted-foreground text-sm font-medium"> - {sidebarFile.path.split("/").pop() || sidebarFile.path} - </div> + <div className="max-w-[250px] truncate text-muted-foreground text-sm font-medium"> + {filename} + </div> ... - filename={sidebarFile.path.split("/").pop() || "code.txt"} + filename={filename || "code.txt"}lib/system-prompt.ts (1)
62-77: Reduce drift risk from hard-coded tool versions and capabilities.OS, language versions, and tool availability can drift from the actual sandbox. Consider centralizing these values (e.g., deriving from server-side constants/env or a single source of truth) or softening the claims (“e.g., Python 3.12.x”) to prevent stale or conflicting instructions to the model.
Would you like me to refactor this to reference a single config and add a minimal runtime check that populates the prompt once per deploy?
app/api/chat/route.ts (2)
83-92: Harden error-path cleanup with try/catch/finally.If
getSandbox()orpauseSandbox()throws, the onError handler could rethrow. Wrap cleanup in try/catch and use finally to await the title flow safely.Apply this diff:
onError: async (error) => { console.error("Error:", error); // Perform same cleanup as onFinish to prevent resource leaks - const sandbox = getSandbox(); - if (sandbox) { - await pauseSandbox(sandbox); - } - await titlePromise; + try { + const sandbox = getSandbox(); + if (sandbox) { + await pauseSandbox(sandbox); + } + } catch (cleanupError) { + console.error("Sandbox cleanup failed in onError:", cleanupError); + } finally { + // Ensure title flow is awaited; swallow any errors for resilience + await titlePromise.catch((e) => + console.error("Title finalization failed in onError:", e), + ); + } },
45-46: Clarify truncation comment to reflect server-side safety net.The comment says truncation is “now done on frontend”, but the code still truncates server-side. Reword to avoid confusion.
Apply this diff:
- // Truncate messages to stay within token limit (processing is now done on frontend) + // Safety net: additionally truncate on the server to stay within token limits + // (primary truncation already happens on the frontend)lib/ai/tools/utils/sandbox-utils.ts (1)
5-5: Plan for legacy sandboxes created with the old template.With the new
SANDBOX_TEMPLATE, previously created sandboxes usingtemporary-sandboxwon’t be discovered if you filter bytemplate. Either add a fallback (see suggested change inlib/ai/tools/utils/sandbox.ts) or a migration/cleanup step to reduce resource waste.Would you like a follow-up PR that adds a one-time fallback lookup and optional cleanup of legacy sandboxes?
📜 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 settings in your CodeRabbit configuration.
📒 Files selected for processing (12)
.env.local.example(1 hunks)README.md(1 hunks)app/api/chat/route.ts(5 hunks)app/components/ChatInput.tsx(1 hunks)app/components/ComputerSidebar.tsx(1 hunks)app/components/Messages.tsx(1 hunks)components/ui/code-action-buttons.tsx(1 hunks)components/ui/tool-block.tsx(1 hunks)lib/ai/providers.ts(1 hunks)lib/ai/tools/utils/sandbox-utils.ts(1 hunks)lib/ai/tools/utils/sandbox.ts(2 hunks)lib/system-prompt.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .env.local.example
🚧 Files skipped from review as they are similar to previous changes (4)
- components/ui/tool-block.tsx
- app/components/Messages.tsx
- lib/ai/providers.ts
- components/ui/code-action-buttons.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/components/ComputerSidebar.tsx (3)
app/contexts/GlobalState.tsx (1)
useGlobalState(92-98)components/ui/code-action-buttons.tsx (1)
CodeActionButtons(22-187)app/components/ComputerCodeBlock.tsx (1)
ComputerCodeBlock(19-165)
app/api/chat/route.ts (4)
lib/ai/providers.ts (1)
myProvider(4-13)lib/system-prompt.ts (1)
systemPrompt(9-77)lib/ai/tools/utils/sandbox-manager.ts (1)
getSandbox(16-37)lib/ai/tools/utils/sandbox.ts (1)
pauseSandbox(152-169)
🪛 markdownlint-cli2 (0.17.2)
README.md
35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (7)
app/components/ChatInput.tsx (2)
46-59: Good call: removing Cmd+C from the stop shortcut avoids hijacking copy on macOSLimiting the stop hotkey to Ctrl+C prevents conflicts with the standard Cmd+C copy gesture on macOS while preserving the terminal-like interrupt behavior.
45-59: Re-evaluate preventDefault on Ctrl+C to avoid blocking copy on WindowsWith Ctrl+C bound, preventDefault() blocks copying text while generating (common on Windows). If you want to allow users to copy while also stopping generation, consider not preventing the default behavior.
Two options:
- Keep current behavior (stop only, no copy while generating).
- Allow copy+stop by not preventing default:
useHotkeys( "ctrl+c", - (e) => { - e.preventDefault(); - onStop(); - }, + () => onStop(), { enabled: isGenerating, enableOnFormTags: true, enableOnContentEditable: true, - preventDefault: true, + preventDefault: false, description: "Stop AI generation", }, [isGenerating, onStop], );Please confirm the intended UX; I can align the tooltip and docs accordingly (and add platform-aware tooltips if desired).
app/components/ComputerSidebar.tsx (2)
168-175: Wrapping may be forced by container styles; rely on the code block’s wrap logicThe container sets whiteSpace: "pre-wrap", which can force wrapping regardless of the isWrapped toggle, potentially conflicting with ComputerCodeBlock’s wrap prop. Prefer letting the code block control wrapping.
Consider removing the inline whiteSpace style or tie it to isWrapped:
- <div - className="font-mono w-full text-xs leading-[18px] flex-1 min-h-0 h-full" - style={{ - overflowWrap: "break-word", - wordBreak: "normal", - whiteSpace: "pre-wrap", - }} - > + <div className="font-mono w-full text-xs leading-[18px] flex-1 min-h-0 h-full">If you do want container-level wrapping helpers, make them conditional:
- style={{ - overflowWrap: "break-word", - wordBreak: "normal", - whiteSpace: "pre-wrap", - }} + style={ + isWrapped + ? { overflowWrap: "break-word", wordBreak: "normal", whiteSpace: "pre-wrap" } + : undefined + }Please verify visually that toggling Wrap in the sidebar changes code wrapping as expected after this adjustment.
150-161: LGTM: good use of the shared CodeActionButtons with sidebar variantPassing content/filename/language and delegating wrap toggling keeps UI consistent with other code blocks and avoids duplicating button logic.
lib/ai/tools/utils/sandbox.ts (2)
123-131: LGTM: Include template in create metadata.Storing
templatealongsideuserIDimproves lookup precision and future migrations. This aligns with the list filter change.
28-36: Keep the template filter intact; don’t fallback to user‐only searches
Reusing a sandbox tied to a different template will load the wrong environment. The metadata.template filter is intentional to ensure consistency, so you should not implement a userID‐only fallback.Likely an incorrect or invalid review comment.
lib/ai/tools/utils/sandbox-utils.ts (1)
5-5: LGTM: Switch to persistent sandbox template.Using
persistent-sandboxis consistent with the PR’s goal of a persistent, sidebar-driven environment.
| const getActionText = (): string => { | ||
| const actionMap = { | ||
| reading: "Reading file", | ||
| creating: "Creating file", | ||
| editing: "Editing file", | ||
| writing: "Writing file", | ||
| }; | ||
| return actionMap[sidebarFile.action || "reading"]; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Provide a safe fallback for unknown action types
If sidebarFile.action is an unexpected value, this returns undefined and the UI will render nothing. Use a robust fallback.
const getActionText = (): string => {
const actionMap = {
reading: "Reading file",
creating: "Creating file",
editing: "Editing file",
writing: "Writing file",
};
- return actionMap[sidebarFile.action || "reading"];
+ const action = sidebarFile.action ?? "reading";
+ return actionMap[action as keyof typeof actionMap] ?? actionMap.reading;
};📝 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.
| const getActionText = (): string => { | |
| const actionMap = { | |
| reading: "Reading file", | |
| creating: "Creating file", | |
| editing: "Editing file", | |
| writing: "Writing file", | |
| }; | |
| return actionMap[sidebarFile.action || "reading"]; | |
| }; | |
| const getActionText = (): string => { | |
| const actionMap = { | |
| reading: "Reading file", | |
| creating: "Creating file", | |
| editing: "Editing file", | |
| writing: "Writing file", | |
| }; | |
| const action = sidebarFile.action ?? "reading"; | |
| return actionMap[action as keyof typeof actionMap] ?? actionMap.reading; | |
| }; |
🤖 Prompt for AI Agents
In app/components/ComputerSidebar.tsx around lines 64 to 72, getActionText
currently maps sidebarFile.action to a string but uses sidebarFile.action ||
"reading", which still returns undefined for unknown action values; update it to
lookup the action safely and provide a robust fallback by: define the actionMap,
then return actionMap[sidebarFile.action as keyof typeof actionMap] ?? "Reading
file" (or another explicit fallback like "Unknown action") so any unexpected or
undefined action will render the fallback string.
| <sandbox_environment> | ||
| System Environment: | ||
| - OS: Debian GNU/Linux 12 linux/amd64 (with internet access) | ||
| - User: \`root\` (with sudo privileges) | ||
| - Home directory: /home/user | ||
| - VPN connectivity is not available due to missing TUN/TAP device support in the sandbox environment | ||
|
|
||
| Development Environment: | ||
| - Python 3.12.10 (commands: python3, pip3) | ||
| - Node.js 20.19.2 (commands: node, npm) | ||
| - Golang 1.24.2 (commands: go) | ||
|
|
||
| Pre-installed Tools: | ||
| - curl, wget, nmap, iputils-ping, whois, traceroute, dnsutils, whatweb, wafw00f, subfinder, gobuster | ||
| - SecLists is pre-installed in /home/user and should be used by default for any fuzzing or wordlist needs | ||
| </sandbox_environment>`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix contradictory environment details (user vs home directory).
The block states the user is “root (with sudo privileges)” but the home directory is “/home/user”. Root’s home is typically “/root”, and root doesn’t need sudo. Recommend aligning this to avoid misleading the agent.
Apply this diff:
- - User: \`root\` (with sudo privileges)
+ - User: \`user\`📝 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.
| <sandbox_environment> | |
| System Environment: | |
| - OS: Debian GNU/Linux 12 linux/amd64 (with internet access) | |
| - User: \`root\` (with sudo privileges) | |
| - Home directory: /home/user | |
| - VPN connectivity is not available due to missing TUN/TAP device support in the sandbox environment | |
| Development Environment: | |
| - Python 3.12.10 (commands: python3, pip3) | |
| - Node.js 20.19.2 (commands: node, npm) | |
| - Golang 1.24.2 (commands: go) | |
| Pre-installed Tools: | |
| - curl, wget, nmap, iputils-ping, whois, traceroute, dnsutils, whatweb, wafw00f, subfinder, gobuster | |
| - SecLists is pre-installed in /home/user and should be used by default for any fuzzing or wordlist needs | |
| </sandbox_environment>`; | |
| <sandbox_environment> | |
| System Environment: | |
| - OS: Debian GNU/Linux 12 linux/amd64 (with internet access) | |
| - User: `user` | |
| - Home directory: /home/user | |
| - VPN connectivity is not available due to missing TUN/TAP device support in the sandbox environment | |
| Development Environment: | |
| - Python 3.12.10 (commands: python3, pip3) | |
| - Node.js 20.19.2 (commands: node, npm) | |
| - Golang 1.24.2 (commands: go) | |
| Pre-installed Tools: | |
| - curl, wget, nmap, iputils-ping, whois, traceroute, dnsutils, whatweb, wafw00f, subfinder, gobuster | |
| - SecLists is pre-installed in /home/user and should be used by default for any fuzzing or wordlist needs | |
| </sandbox_environment>`; |
🤖 Prompt for AI Agents
In lib/system-prompt.ts around lines 62 to 77, the sandbox environment block
contradicts user/home details: it lists the user as "root (with sudo
privileges)" while the home directory is "/home/user" and mentions sudo for
root; update the block so the user and home directory are consistent (either
change user to "user" with home "/home/user" and keep sudo noted appropriately,
or keep user as "root" and set home to "/root" and remove the "sudo"
implication), and ensure any mention of sudo privileges matches the chosen user.
| - Optionally customize the AI models: | ||
| ``` | ||
| NEXT_PUBLIC_AGENT_MODEL=qwen/qwen3-coder | ||
| NEXT_PUBLIC_TITLE_MODEL=qwen/qwen-turbo | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify README examples match provider defaults (potential mismatch for title model).
The README example uses NEXT_PUBLIC_TITLE_MODEL=qwen/qwen-turbo. Ensure this aligns with the actual default configured in lib/ai/providers.ts and the placeholder in .env.local.example (AI summary indicates a default of qwen/qwen3-30b-a3). Aligning these avoids confusion.
Run this script to confirm defaults and placeholders:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for provider defaults and env usage..."
rg -n -A 3 -B 3 'NEXT_PUBLIC_(AGENT|TITLE)_MODEL|agent-model|title-model' lib --hidden || true
echo
echo "Checking example env file for placeholders..."
fd -a '.env.local.example' | while read -r f; do
echo "File: $f"
rg -n 'NEXT_PUBLIC_(AGENT|TITLE)_MODEL' "$f" -A 1 -B 1 || true
doneLength of output: 1348
Align README title model example with configured defaults
The README currently suggests using NEXT_PUBLIC_TITLE_MODEL=qwen/qwen-turbo, but in lib/ai/providers.ts the fallback is qwen/qwen3-30b-a3b (and the .env.local.example comments reference qwen/qwen3-30b-a3). Update to keep these in sync and avoid confusion:
• In README.md (around lines 34–38), replace:
- NEXT_PUBLIC_TITLE_MODEL=qwen/qwen-turbo
+ NEXT_PUBLIC_TITLE_MODEL=qwen/qwen3-30b-a3b• Optionally, verify that .env.local.example’s default comment (default: qwen/qwen3-30b-a3) matches the actual code default or adjust it to qwen/qwen3-30b-a3b.
📝 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.
| - Optionally customize the AI models: | |
| ``` | |
| NEXT_PUBLIC_AGENT_MODEL=qwen/qwen3-coder | |
| NEXT_PUBLIC_TITLE_MODEL=qwen/qwen-turbo | |
| ``` | |
| - Optionally customize the AI models: |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In README.md around lines 34 to 38, the example title model value conflicts with
the code default in lib/ai/providers.ts; update the README example to use
NEXT_PUBLIC_TITLE_MODEL=qwen/qwen3-30b-a3b so it matches the fallback used by
the code, and also verify the comment in .env.local.example to ensure its
default comment reads qwen/qwen3-30b-a3b (or change the code/defaults to
consistently use whichever variant you prefer) so all three places (README,
.env.local.example comment, and lib/ai/providers.ts) are identical.
Summary by CodeRabbit
New Features
Refactor
Style
Chores