feat: Add sidebar navigation controls and auto-follow functionality #113
Conversation
- Add Previous/Next buttons and slider to navigate between tool executions - Implement 'Jump to Live' button to view latest tool result - Extract navigation logic to useSidebarNavigation hook for better organization - Add auto-follow: automatically update sidebar when at live position during streaming - Show green 'live' indicator when status is streaming - Pass status prop to ComputerSidebar to detect streaming state - Fix file content extraction to only include operations with output available - Remove useSidebarAutoOpen hook and auto-opener functionality - Update TerminalToolHandler to handle streaming updates directly
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
app/share/[shareId]/SharedChatView.tsx (1)
19-36: Shared sidebar wrappers andmessagesplumbing look correct; consider tightening typesThe desktop/mobile wrappers correctly bridge SharedChatContext into
ComputerSidebarBase, and usingmessages || []avoids undefined in navigation. You might later tighten the props toChatMessage[](or the shared message type) instead ofany[]for better type safety, but the current wiring is functionally sound.Also applies to: 43-58, 210-218
lib/utils/sidebar-utils.ts (2)
3-16: TightenMessagePart/Messagetyping instead of broadany+ index signaturesRight now
MessagePartandMessageare locally defined withanyfields and[key: string]: any. This loses type-safety and can hide mismatches with upstream message shapes (e.g.,data,input,output).Consider importing and extending the existing
MessagePart/message type fromlib/utils/message-utils.ts(or the canonical chat types) instead of redefining them here, so changes to message structure remain consistent and type-checked.
52-69: Consider extracting shared “result → string output” helperThe logic to turn
part.output?.resultinto a string (result.outputvsstdout/stderrvs direct string) is repeated for terminal, Python, and partially for file-edit tools. This makes it easy for these branches to drift if result formats evolve.A small internal helper like
extractResultText(result: any): stringused across these call sites would:
- Ensure consistent handling of new/legacy formats.
- Centralize any future adjustments to result shape.
- Reduce cognitive overhead in
extractAllSidebarContent.Not blocking, but would make this utility more maintainable.
Also applies to: 90-106, 170-176
app/components/ComputerSidebar.tsx (2)
75-106: Clarify auto-follow condition vs. “during streaming” commentThe auto-follow effect comment says “during streaming”, but the condition only checks
sidebarOpen,onNavigate, andtoolExecutions.length, notstatus:if (!sidebarOpen || !onNavigate || toolExecutions.length === 0) return;If
messageswere to change outside of an active streaming phase, this effect would still auto-jump when at live. If you intend the behavior to be strictly tied to a streaming state, consider includingstatus === "streaming"in the guard or updating the comment to match the actual behavior.
30-37: Use a concrete message type instead ofany[]formessages
messagesis currently typed asany[]on bothComputerSidebarPropsand theComputerSidebarwrapper:messages?: any[];Since
useSidebarNavigationandextractAllSidebarContentexpect a specific message shape, narrowing this to the actualMessagetype used in the chat layer will:
- Catch mismatches at compile time.
- Make usage clearer to callers.
- Avoid accidental misuse from other parts of the app.
Also applies to: 516-521
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/components/ComputerSidebar.tsx(4 hunks)app/components/Messages.tsx(1 hunks)app/components/chat.tsx(2 hunks)app/components/tools/TerminalToolHandler.tsx(3 hunks)app/hooks/useChatHandlers.ts(0 hunks)app/hooks/useSidebarAutoOpen.ts(0 hunks)app/hooks/useSidebarNavigation.ts(1 hunks)app/share/[shareId]/SharedChatView.tsx(4 hunks)lib/utils/sidebar-utils.ts(1 hunks)types/chat.ts(1 hunks)
💤 Files with no reviewable changes (2)
- app/hooks/useChatHandlers.ts
- app/hooks/useSidebarAutoOpen.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-18T20:33:42.057Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 10
File: app/components/ComputerCodeBlock.tsx:154-158
Timestamp: 2025-08-18T20:33:42.057Z
Learning: In ComputerCodeBlock component, the wrap state should be controlled by the parent component (ComputerSidebar) which passes the wrap prop. The internal isWrapped state conflicts with parent state management when used in ComputerSidebar context.
Applied to files:
app/share/[shareId]/SharedChatView.tsxapp/components/ComputerSidebar.tsxapp/components/chat.tsx
🧬 Code graph analysis (4)
lib/utils/sidebar-utils.ts (2)
lib/utils/message-utils.ts (1)
MessagePart(5-8)types/chat.ts (2)
SidebarContent(38-38)SidebarFile(10-20)
app/share/[shareId]/SharedChatView.tsx (2)
app/share/[shareId]/SharedChatContext.tsx (1)
useSharedChatContext(41-49)e2e/page-objects/ChatComponent.ts (1)
messages(28-32)
app/components/ComputerSidebar.tsx (2)
types/chat.ts (2)
SidebarContent(38-38)ChatStatus(77-77)app/hooks/useSidebarNavigation.ts (1)
useSidebarNavigation(16-124)
app/components/chat.tsx (2)
app/components/ComputerSidebar.tsx (1)
ComputerSidebar(516-533)e2e/page-objects/ChatComponent.ts (1)
messages(28-32)
🔇 Additional comments (5)
types/chat.ts (1)
10-20: AddingtoolCallIdtoSidebarFilelooks consistent with the sidebar modelMaking
toolCallIdoptional onSidebarFilewhile it remains required onSidebarTerminal/SidebarPythonfits the union and the matching logic in navigation; type guards remain valid.app/components/chat.tsx (1)
769-777: PassingmessagesandstatusintoComputerSidebaris consistent across layoutsBoth desktop and mobile sidebars now receive the live
messagesarray andstatusfromuseChat, which is exactly what the navigation/auto-follow logic needs. No issues spotted here.Also applies to: 790-794
app/components/Messages.tsx (1)
128-128: Comment accurately reflects removal of sidebar auto-open logicThe inline note makes it clear that sidebar opening is now entirely user-driven; no functional concerns here.
app/components/tools/TerminalToolHandler.tsx (1)
1-9: Terminal sidebar streaming sync and output resolution look correctThe memoized
streamingOutput/finalOutputlogic and theisSidebarActive-gatedupdateSidebarContenteffect give you live, accurate sidebar updates while keeping changes scoped to the active tool call. TheSidebarTerminalpayload (includingisExecuting) is consistent with the new sidebar model.Also applies to: 21-23, 32-41, 42-61, 62-74, 76-93
app/components/ComputerSidebar.tsx (1)
47-48: Wrap state controlled by parent sidebar aligns with prior guidance
isWrappedis managed inComputerSidebarBaseand passed down aswrap/isWrappedprops toComputerCodeBlockandTerminalCodeBlock, withonToggleWrapwired from the sidebar:const [isWrapped, setIsWrapped] = useState(true); // ... isWrapped={isWrapped} onToggleWrap={handleToggleWrap} // ... <ComputerCodeBlock ... wrap={isWrapped} /> <TerminalCodeBlock ... wrap={isWrapped} />This keeps wrapping behavior controlled from the parent sidebar, avoiding conflicting internal state inside
ComputerCodeBlock, which matches the earlier learning for this component’s usage. Based on learnings, this looks good.Also applies to: 326-327, 344-355, 370-397
… in code blocks - Add bg-background to content containers in ComputerSidebar to prevent bg-muted/20 bleed-through - Change TerminalCodeBlock from bg-card to bg-transparent for consistent backgrounds - Remove overflow-visible that was causing background bleed-through - Add transparent background classes to nested code and span elements - Clean up redundant bg-background classes in nested containers
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
app/hooks/useSidebarNavigation.ts (1)
98-101: ClampcurrentIndexwhen computing progress to avoid negative percentagesWhen
sidebarContentisnull,currentIndexis-1. With multiple tool executions,getProgressPercentagereturns a negative value, which can break slider UI.Clamp the index into range before computing:
- const getProgressPercentage = useMemo(() => { - if (toolExecutions.length <= 1) return 100; - return (currentIndex / (toolExecutions.length - 1)) * 100; - }, [currentIndex, toolExecutions.length]); + const getProgressPercentage = useMemo(() => { + if (toolExecutions.length <= 1) return 100; + const clampedIndex = Math.max( + 0, + Math.min(currentIndex, toolExecutions.length - 1), + ); + return (clampedIndex / (toolExecutions.length - 1)) * 100; + }, [currentIndex, toolExecutions.length]);lib/utils/sidebar-utils.ts (1)
163-167: Fix falsyoffset/limitcheck so ranges at offset 0 are not dropped
if (fileInput.offset && fileInput.limit)treats0as falsy, so a valid range starting at the beginning of a file won’t get arangeattached, even whenoffset/limitare provided.Use explicit numeric checks instead:
- if (fileInput.offset && fileInput.limit) { - range = { - start: fileInput.offset, - end: fileInput.offset + fileInput.limit - 1, - }; - } + if ( + typeof fileInput.offset === "number" && + typeof fileInput.limit === "number" + ) { + range = { + start: fileInput.offset, + end: fileInput.offset + fileInput.limit - 1, + }; + }This preserves existing behavior while correctly handling
offset = 0.app/components/ComputerSidebar.tsx (1)
339-408: CallgetProgressPercentage()when building slider stylesIn the slider track and handle styles you’re interpolating the function reference instead of its return value:
style={{ left: "0%", width: `${getProgressPercentage}%`, }} ... style={{ left: `${getProgressPercentage}%`, }}This will stringify the function (e.g.
"function getProgressPercentage() {...}") and produce invalid CSS, breaking the visual progress and handle position. You need to call the function:- style={{ - left: "0%", - width: `${getProgressPercentage}%`, - }} + style={{ + left: "0%", + width: `${getProgressPercentage()}%`, + }} ... - style={{ - left: `${getProgressPercentage}%`, - }} + style={{ + left: `${getProgressPercentage()}%`, + }}Everything else in the footer (prev/next buttons, live indicator based on
status, and “Jump to live” CTA) looks good.Also applies to: 410-514
🧹 Nitpick comments (5)
app/components/ComputerCodeBlock.tsx (1)
24-29: SyncisWrappedwithwrapprop to avoid parent/child desync
isWrappedis initialized fromwrapbut never updated whenwrapchanges. IfComputerSidebar(or another parent) treatswrapas a controlled prop, its toggles can drift out of sync with the local state and button styling.You can keep the local toggle while letting the parent remain source-of-truth on prop changes by syncing via an effect:
-import { useState, useMemo } from "react"; +import { useState, useMemo, useEffect } from "react"; const ComputerCodeBlock = ({ children, language, wrap = true, showButtons = true, }: ComputerCodeBlockProps) => { const codeContent = String(children); const [copied, setCopied] = useState(false); const [isWrapped, setIsWrapped] = useState(wrap); + + useEffect(() => { + setIsWrapped(wrap); + }, [wrap]);This preserves the local toggle behavior while keeping external wrap changes honored. Based on learnings, this avoids reintroducing the earlier parent/child wrap control conflict.
Also applies to: 153-195
app/hooks/useSidebarNavigation.ts (1)
10-24: Consider tighteningmessagestype for better safety
messages: any[]works but loses the structure thatextractAllSidebarContentexpects (assistant messages with parts). If you have a sharedMessage/UIMessagetype elsewhere, wiring it through here will make the hook safer and self-documenting.No change is strictly required for functionality; this is more about future-proofing.
app/components/tools/TerminalToolHandler.tsx (1)
32-61: Streaming/final output aggregation and live sidebar sync look solidThe use of
useMemoto aggregate streaming output and then layernewFormatOutput, legacystdout/stderr, streaming chunks,result.error, anderrorTextintofinalOutputis coherent and should cover both new and legacy formats.The
isSidebarActive+useEffectpattern for pushing{ output, isExecuting }intoupdateSidebarContentonly when the relevant terminal sidebar is open also avoids unnecessary global churn. One minor thing to keep in mind is thatisExecutinghere only checks forstate === "input-available" && status === "streaming", whereasextractAllSidebarContentalso treats"running"as executing; if you notice inconsistent “live” indicators between sidebar navigation and the active panel, it might be worth aligning those definitions.Functionally, this looks good to ship.
Also applies to: 62-95
app/components/ComputerSidebar.tsx (2)
30-37: Prop threading formessages,onNavigate, andstatuslooks consistent; consider tightening types laterThe new props (
messages,onNavigate,status) are wired cleanly throughComputerSidebarBaseand theComputerSidebarwrapper, andopenSidebarasonNavigatematches the intended navigation flow. The only thing I’d consider later is replacingmessages?: any[]with a more specific message type to get better TS safety.Also applies to: 39-46, 524-539
48-73: Navigation hook and auto-follow behavior match the PR intentThe integration with
useSidebarNavigationplus thepreviousToolCountRef+ dualuseEffectsetup correctly implements “auto-follow when at live”: new tools only triggeronNavigatewhen the user was/is on the last execution, and the guard conditions (!sidebarOpen,!onNavigate, emptytoolExecutions) prevent unnecessary work. The first effect’s dependency on justsidebarOpenalso ensures the initial ref baseline is set when the sidebar opens. This all looks coherent with the described behavior.Also applies to: 75-112, 16-16
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/components/ComputerCodeBlock.tsx(2 hunks)app/components/ComputerSidebar.tsx(5 hunks)app/components/TerminalCodeBlock.tsx(2 hunks)app/components/tools/TerminalToolHandler.tsx(3 hunks)app/hooks/useChatHandlers.ts(0 hunks)app/hooks/useSidebarNavigation.ts(1 hunks)lib/utils/sidebar-utils.ts(1 hunks)
💤 Files with no reviewable changes (1)
- app/hooks/useChatHandlers.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-18T20:33:42.057Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 10
File: app/components/ComputerCodeBlock.tsx:154-158
Timestamp: 2025-08-18T20:33:42.057Z
Learning: In ComputerCodeBlock component, the wrap state should be controlled by the parent component (ComputerSidebar) which passes the wrap prop. The internal isWrapped state conflicts with parent state management when used in ComputerSidebar context.
Applied to files:
app/components/ComputerSidebar.tsxapp/components/ComputerCodeBlock.tsx
🧬 Code graph analysis (4)
app/hooks/useSidebarNavigation.ts (2)
types/chat.ts (4)
SidebarContent(38-38)isSidebarTerminal(46-50)isSidebarFile(40-44)isSidebarPython(52-56)lib/utils/sidebar-utils.ts (1)
extractAllSidebarContent(18-200)
app/components/tools/TerminalToolHandler.tsx (2)
app/contexts/GlobalState.tsx (1)
useGlobalState(704-710)types/chat.ts (2)
SidebarTerminal(22-29)isSidebarTerminal(46-50)
lib/utils/sidebar-utils.ts (1)
types/chat.ts (2)
SidebarContent(38-38)SidebarFile(10-20)
app/components/ComputerCodeBlock.tsx (1)
lib/utils/shiki.tsx (1)
ShikiErrorBoundary(23-45)
🔇 Additional comments (4)
app/components/TerminalCodeBlock.tsx (2)
159-168: AnsiCodeBlock styling additions look correctAdding
bg-transparentfor pre/code/span and keeping the height utilities tied toisWrappedis consistent with the Shiki-based styling and shouldn’t introduce functional regressions.
261-280: Sidebar scroll container background change is safeUsing
bg-backgroundon the sidebar scroll container should fix transparency issues without affecting layout or streaming behavior.lib/utils/sidebar-utils.ts (1)
23-123: Overall sidebar content extraction logic is well-structuredThe separation between streaming maps (
terminalDataMap/pythonDataMap) and the second pass that builds Terminal/Python/File SidebarContent entries keeps the logic readable and makes it straightforward to add new tool types later. The output precedence (newresult.output→ legacy stdout/stderr → streaming →output.output) also provides sensible fallbacks for mixed-format histories.File operation handling (read/write/edit) with path normalization and read-file line-number stripping via
/^\s*\d+\|/gmshould give clean content for the sidebar. Aside from the range condition below, the behavior looks consistent with the SidebarFile contract.Also applies to: 125-195
app/components/ComputerSidebar.tsx (1)
47-47: Parent-controlled wrap state aligns with previousComputerCodeBlockguidanceManaging
isWrappedinComputerSidebarBaseand passingwrap/onToggleWrapthrough toCodeActionButtons,ComputerCodeBlock, andTerminalCodeBlockkeeps wrapping behavior centralized in the parent and avoids internal state conflicts inComputerCodeBlock. This matches the earlier preference that wrap should be driven by the sidebar rather than the code block itself.Based on learnings, this is the desired ownership of the wrap state.
Also applies to: 229-231, 332-333
- Remove diff-storage.ts utility (no longer needed) - Simplify FileToolsHandler to use only stream data from data-diff parts - Remove isTemporaryChat prop from FileToolsHandler (no longer needed) - Update sidebar-utils to only use stream data, no localStorage fallback - Diff view now only works during active session, shows result message after reload
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
lib/ai/tools/search-replace.ts (1)
5-7: Diff emission wiring looks good; consider guarding on missingtoolCallId.The new
SearchReplaceOutputtype, updatedexecutesignature, andwriter.write({ type: "data-diff", ... })payload align cleanly with the sidebar diff flow andFileToolsHandlerexpectations. One small improvement: iftoolCallIdcan ever be absent, you may want to skip emitting the diff (or log and return early) to avoid producing unmatchabledata-diffparts downstream.Also applies to: 10-11, 40-53, 105-115
app/components/DiffView.tsx (2)
20-76: Custom LCS diff is correct but can be expensive on large files.
computeLCS+computeDiffis logically sound, but the DP table is O(m·n) in both time and space. For very large files (thousands of lines), this could become slow or memory‑heavy. If you expect large diffs, consider a more incremental or chunked diff approach, or delegating to a battle‑tested diff library; otherwise this is fine for typical code‑sized inputs.Also applies to: 81-119, 129-133
134-149: Tab keyboard handling is slightly redundant; wrap control matches parent‑owned pattern.Since you’re using
<button>elements, they already handle Enter/Space activation; the extraonKeyDownthat also callssetViewModeis optional and may cause duplicate state updates. You can rely ononClickalone for simplicity. Thewrapprop being threaded through toComputerCodeBlockandDiffContentwithout internal state keeps wrapping under parent control, which aligns with the earlier ComputerSidebar/ComputerCodeBlock pattern. Based on learnings, this is the right ownership model.Also applies to: 151-199, 208-257
app/components/tools/FileToolsHandler.tsx (1)
1-3: Diff streaming integration for search/replace is well‑wired; only minor defensive tweaks are optional.The new
DiffDataParttype,diffDataFromStreamlookup keyed bytoolCallId, and updatedrenderSearchReplaceToolensure the sidebar opens withmodifiedContent(andoriginalContentwhen available) instead of the summary string, while gracefully falling back to the previous behavior if no diff part was streamed. You might optionally add a small guard likeif (!message.parts) return null;before the.findto make the diff extraction more defensive, but given how this component is used for tool parts, it’s not strictly necessary.Also applies to: 8-17, 24-44, 249-287, 288-299, 301-317
app/components/ComputerSidebar.tsx (1)
69-75: Auto‑follow effect ignoresstatusdespite “during streaming” intentThe auto‑follow effect comment says:
// Auto-follow new tools when at live position during streamingbut the effect only checks
sidebarOpen,onNavigate, andtoolExecutions.lengthand never reads thestatusprop. As written, any increase intoolExecutions.lengthwhile the sidebar is open and the user is at live will cause an automatic jump, regardless of whether the chat is actually in"streaming"state.If the intent is to limit auto‑follow to active streaming (and avoid unexpected jumps when, for example, historical messages are added or the status is
"ready"), you could gate onstatusand add it to the dependencies:- useEffect(() => { - if (!sidebarOpen || !onNavigate || toolExecutions.length === 0) { + useEffect(() => { + if ( + !sidebarOpen || + !onNavigate || + status !== "streaming" || + toolExecutions.length === 0 + ) { return; } @@ - }, [ - toolExecutions.length, - currentIndex, - sidebarOpen, - onNavigate, - toolExecutions, - ]); + }, [ + toolExecutions.length, + currentIndex, + sidebarOpen, + onNavigate, + toolExecutions, + status, + ]);If you do intend auto‑follow to apply regardless of
status, consider updating the comment to avoid confusion between code and documented behavior.Also applies to: 76-113
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/components/ComputerSidebar.tsx(6 hunks)app/components/DiffView.tsx(1 hunks)app/components/MessagePartHandler.tsx(1 hunks)app/components/tools/FileToolsHandler.tsx(2 hunks)app/hooks/useSidebarNavigation.ts(1 hunks)lib/ai/tools/search-replace.ts(3 hunks)lib/utils/sidebar-utils.ts(1 hunks)types/chat.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- types/chat.ts
- lib/utils/sidebar-utils.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-18T20:33:42.057Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 10
File: app/components/ComputerCodeBlock.tsx:154-158
Timestamp: 2025-08-18T20:33:42.057Z
Learning: In ComputerCodeBlock component, the wrap state should be controlled by the parent component (ComputerSidebar) which passes the wrap prop. The internal isWrapped state conflicts with parent state management when used in ComputerSidebar context.
Applied to files:
app/components/DiffView.tsxapp/components/ComputerSidebar.tsx
🧬 Code graph analysis (6)
app/components/DiffView.tsx (2)
app/components/MemoizedMarkdown.tsx (1)
a(14-26)app/components/ComputerCodeBlock.tsx (1)
ComputerCodeBlock(20-197)
app/components/MessagePartHandler.tsx (1)
app/components/tools/FileToolsHandler.tsx (1)
FileToolsHandler(24-402)
app/components/ComputerSidebar.tsx (5)
types/chat.ts (2)
SidebarContent(42-42)ChatStatus(81-81)components/ui/sidebar.tsx (1)
SidebarContent(753-753)app/hooks/useSidebarNavigation.ts (1)
useSidebarNavigation(20-133)app/components/DiffView.tsx (1)
DiffView(121-201)app/components/ComputerCodeBlock.tsx (1)
ComputerCodeBlock(20-197)
app/components/tools/FileToolsHandler.tsx (2)
types/chat.ts (1)
ChatStatus(81-81)app/contexts/GlobalState.tsx (1)
useGlobalState(704-710)
lib/ai/tools/search-replace.ts (1)
types/agent.ts (1)
ToolContext(19-29)
app/hooks/useSidebarNavigation.ts (2)
lib/utils/sidebar-utils.ts (2)
Message(12-16)extractAllSidebarContent(18-226)types/chat.ts (4)
SidebarContent(42-42)isSidebarTerminal(50-54)isSidebarFile(44-48)isSidebarPython(56-60)
🔇 Additional comments (2)
app/components/MessagePartHandler.tsx (1)
70-76: PassingmessageintoFileToolsHandleris consistent and necessary.Wiring
message={message}here matches how other tool handlers work and enables in‑memory diff lookup insideFileToolsHandlerwithout changing control flow.app/hooks/useSidebarNavigation.ts (1)
20-32: Navigation/index logic and progress clamping look solid.Matching by
toolCallIdfirst, then falling back to content‑based matching per sidebar type, gives robust index resolution. Slider clicks are correctly clamped to valid indices, andgetProgressPercentagenow guards against negative indices and out‑of‑range values, keeping progress strictly within[0, 100]even when nothing is currently selected.Also applies to: 35-63, 65-100, 102-119
| const [isWrapped, setIsWrapped] = useState(true); | ||
| const previousToolCountRef = useRef<number>(0); | ||
|
|
There was a problem hiding this comment.
Sidebar wrap toggle is currently out of sync with ComputerCodeBlock rendering
You manage isWrapped in ComputerSidebarBase and pass it both to CodeActionButtons and as wrap={isWrapped} to ComputerCodeBlock/TerminalCodeBlock, but ComputerCodeBlock still owns its own isWrapped state (initialized from wrap only once) and ignores subsequent prop changes. Because you set showButtons={false} here, there’s no way for the child to update its own isWrapped, so clicking the wrap control in the sidebar header updates only the sidebar’s isWrapped and not the actual code wrapping. The visible wrap toggle in the header will therefore not affect the file/Python/result blocks.
To make the new sidebar‑level wrap control actually work and avoid the parent/child conflict called out previously, I’d recommend syncing ComputerCodeBlock’s internal state from the wrap prop when it’s being controlled externally, e.g. something along these lines in ComputerCodeBlock.tsx:
// inside ComputerCodeBlock
const [isWrapped, setIsWrapped] = useState(wrap);
// When used in controlled mode (sidebar passes wrap and hides local buttons),
// keep internal state in sync with the prop.
useEffect(() => {
if (!showButtons) {
setIsWrapped(wrap);
}
}, [wrap, showButtons]);This lets standalone uses (with showButtons defaulting to true) keep their internal toggle, while sidebar uses (with showButtons={false} and a parent wrap prop) behave as a controlled component and reflect the sidebar’s isWrapped correctly. Based on learnings, this aligns with the goal of having the sidebar own wrap state.
Also applies to: 230-232, 308-336, 351-378
🤖 Prompt for AI Agents
In app/components/ComputerSidebar.tsx around lines 48-50 (and similarly at lines
230-232, 308-336, 351-378), the sidebar manages a wrap toggle but child
ComputerCodeBlock/TerminalCodeBlock components initialize their own isWrapped
state from the wrap prop only once and ignore subsequent prop changes; update
the child components so they initialize isWrapped from the wrap prop and add an
effect that, when the component is being externally controlled (showButtons is
false), synchronizes internal isWrapped with the incoming wrap prop (i.e., call
setIsWrapped(wrap) inside a useEffect that depends on [wrap, showButtons]); this
preserves standalone toggle behavior when showButtons is true and makes the
sidebar-controlled wrap toggle actually affect the rendered blocks when
showButtons is false.
| {isFile && ( | ||
| <ComputerCodeBlock | ||
| language={ | ||
| sidebarContent.language || | ||
| getLanguageFromPath(sidebarContent.path) | ||
| } | ||
| wrap={isWrapped} | ||
| showButtons={false} | ||
| > | ||
| {sidebarContent.content} | ||
| </ComputerCodeBlock> | ||
| <> | ||
| {/* Show DiffView for editing actions with diff data */} | ||
| {sidebarContent.action === "editing" && | ||
| sidebarContent.originalContent && | ||
| sidebarContent.modifiedContent ? ( | ||
| <DiffView | ||
| originalContent={sidebarContent.originalContent} | ||
| modifiedContent={sidebarContent.modifiedContent} | ||
| language={ | ||
| sidebarContent.language || | ||
| getLanguageFromPath(sidebarContent.path) | ||
| } | ||
| wrap={isWrapped} | ||
| /> | ||
| ) : ( | ||
| <ComputerCodeBlock | ||
| language={ | ||
| sidebarContent.language || | ||
| getLanguageFromPath(sidebarContent.path) | ||
| } | ||
| wrap={isWrapped} | ||
| showButtons={false} | ||
| > | ||
| {sidebarContent.content} | ||
| </ComputerCodeBlock> | ||
| )} | ||
| </> |
There was a problem hiding this comment.
Diff view guard will skip valid diffs when content is an empty string
The condition:
sidebarContent.action === "editing" &&
sidebarContent.originalContent &&
sidebarContent.modifiedContentrelies on truthiness, so cases where either side of the diff is an empty string (e.g. creating a new file, or clearing a file) will fall back to the plain ComputerCodeBlock path instead of showing a proper diff.
To treat “empty content” as valid while still requiring the fields to be present, consider tightening the check to explicit null/undefined:
- {sidebarContent.action === "editing" &&
- sidebarContent.originalContent &&
- sidebarContent.modifiedContent ? (
+ {sidebarContent.action === "editing" &&
+ sidebarContent.originalContent != null &&
+ sidebarContent.modifiedContent != null ? (This keeps the guard against missing data but allows empty strings to still render through DiffView.
🤖 Prompt for AI Agents
In app/components/ComputerSidebar.tsx around lines 351 to 378, the DiffView
guard uses truthiness for sidebarContent.originalContent and
sidebarContent.modifiedContent which wrongly rejects empty-string content;
change the condition to explicitly check for null/undefined (e.g.
sidebarContent.originalContent != null && sidebarContent.modifiedContent !=
null) while keeping sidebarContent.action === "editing", so empty strings are
treated as valid diff content but missing fields still block DiffView.
Summary by CodeRabbit
New Features
Bug Fixes / Changes
✏️ Tip: You can customize this high-level summary in your review settings.