⚡ Bolt: [performance improvement] optimize chat history rendering#95
⚡ Bolt: [performance improvement] optimize chat history rendering#95iotserver24 wants to merge 2 commits into
Conversation
💡 What: Refactored the `ChatHistory` component to use a single O(N) pass for session grouping instead of an O(4N) pass. Extracted the list items into a `ChatHistoryItem` component wrapped with `React.memo` and stabilized the `handleDelete` function with `useCallback` to prevent cascading VDOM re-renders. 🎯 Why: In a long list of chat sessions, any change in `activeSessionId` would trigger an O(4N) array processing step, re-evaluate numerous date strings, and cause the entire DOM list to re-render. 📊 Impact: Reduces array iteration from 4N to N on chat session refresh. Drops list item re-renders down from N to 2 when switching between active sessions. 🔬 Measurement: Verify by adding `console.log` into `ChatHistoryItem` or monitoring the React DevTools profiler while clicking between different sessions in the Chat History sidebar. Expect only two re-renders per switch (the old active session and the new one).
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughChatHistory component is refactored to optimize re-render performance by introducing memoized session grouping, stabilizing the delete callback with ChangesChatHistory Memoization and Performance
🎯 2 (Simple) | ⏱️ ~12 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@copilot resolve the merge conflicts in this pull request |
There was a problem hiding this comment.
Pull request overview
This PR optimizes the desktop renderer’s ChatHistory sidebar to reduce unnecessary work and re-renders when switching sessions in long chat histories.
Changes:
- Refactors session grouping into a single-pass
useMemocomputation (reducing repeated filtering/date parsing). - Extracts a memoized
ChatHistoryItemcomponent and stabilizeshandleDeletewithuseCallbackto avoid cascading list re-renders. - Adds a Bolt learning note about stable callback props for
React.memolist items.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| plan_review_request.md | Adds a short plan note describing the intended ChatHistory optimization. |
| packages/desktop/src/renderer/components/ChatHistory.tsx | Implements O(N) memoized grouping, memoized list items, and stable delete handler. |
| .jules/bolt.md | Documents a performance lesson about React.memo requiring stable callback props. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // ⚡ Bolt: Memoize the grouping of sessions into an O(N) single pass to avoid O(4N) filtering and Date parsing on every render | ||
| const groups = useMemo(() => { | ||
| const today = new Date(); | ||
| today.setHours(0, 0, 0, 0); | ||
| const yesterday = new Date(today); | ||
| yesterday.setDate(yesterday.getDate() - 1); | ||
| const weekAgo = new Date(today); | ||
| weekAgo.setDate(weekAgo.getDate() - 7); |
| <button | ||
| onClick={() => onSelect(session.id)} | ||
| className={cn( | ||
| "flex w-full items-center gap-2.5 rounded-lg px-2 py-1.5 text-left transition-all duration-200 group relative", | ||
| isActive | ||
| ? "text-xibe-text font-semibold before:absolute before:left-0 before:top-1/4 before:bottom-1/4 before:w-1 before:bg-xibe-text before:rounded-r" | ||
| : "text-xibe-text-secondary hover:bg-xibe-surface-hover/30 hover:text-xibe-text" | ||
| )} | ||
| > | ||
| <MessageSquare className={cn("h-3.5 w-3.5 shrink-0", isActive ? "text-xibe-text" : "text-xibe-text-dim/40 group-hover:text-xibe-text-dim/70")} /> | ||
| <span className="flex-1 truncate text-[12px] font-medium leading-tight">{session.title}</span> | ||
|
|
||
| {/* ⚡ Bolt: Used CSS group-hover instead of React state to toggle delete button visibility to prevent O(N) re-renders of the entire list on hover */} | ||
| <span | ||
| onClick={(e) => onDelete(e, session.id)} | ||
| className="shrink-0 rounded p-1 text-xibe-text-dim/50 hover:text-xibe-error hover:bg-xibe-error/10 transition-colors animate-fade-in hidden group-hover:block" | ||
| title="Delete chat" | ||
| > | ||
| <Trash2 className="h-3.5 w-3.5" /> | ||
| </span> | ||
|
|
||
| {/* ⚡ Bolt: Hide timestamp on hover using group-hover to make room for delete button without React state overhead */} | ||
| <span className="shrink-0 text-[10px] text-xibe-text-dim/40 tabular-nums block group-hover:hidden">{relativeTime(session.updated)}</span> | ||
| </button> |
…oization-13284515533250718070 # Conflicts: # packages/desktop/src/renderer/components/ChatHistory.tsx Co-authored-by: iotserver24 <147928812+iotserver24@users.noreply.github.com>
|
Hey @iotserver24! 👋 I'll go through the changes and help you out with an automated review! 🔍 Starting the review now... |
🔍 Code Review for @iotserver24Great work on this performance-focused PR. The refactoring correctly addresses the core issue of needless re-renders in the chat history sidebar. Below is my comprehensive review, starting with security (none critical), then diving into the performance aspects you asked about. 🔴 Security ConcernsNo critical security issues found. The changes introduce zero hardcoded credentials, API keys, or unsafe data flow.
🎯 User-Specific Analysis: Performance OptimizationYou set out to reduce the chat history rendering from O(4N) to O(N) and limit re-renders when switching active sessions. Let’s verify each element: ✅ Achieved:
🔧 Recommended Changes
📊 Comprehensive Analysis
Test recommendation: 📊 Review SummaryFiles reviewed: 3
Recommendation: ✅ Approve (with minor actionable suggestions). The core performance optimization is sound and well-implemented. Address the two suggestions to ensure the memoization benefits are fully realised in production. Thanks for the excellent optimisation, @iotserver24! 🤖 Powered by Xibe AI • Auto-generated |
Resolved in commit Screenshot: unable to capture a runtime UI screenshot in this runner due Playwright browser-profile permission issues ( |
![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 (2)
plan_review_request.md (1)
4-4: ⚡ Quick winConsider explicitly mentioning
useCallbackfor callback stabilization.The plan mentions extracting
ChatHistoryItemas aReact.memocomponent but doesn't explicitly note that callback props (likeonDelete) must be wrapped inuseCallbackto prevent defeating the memoization. While this is documented in.jules/bolt.md, explicitly including it here would make the plan more complete and self-contained.📝 Suggested enhancement
-2. **Extract `ChatHistoryItem` as a `React.memo` component.** Currently, when `activeSessionId` changes, the entire list of `button` components re-renders because they are defined inline inside the mapping. By extracting the item to a `React.memo` component, only the previously active item and the newly active item will re-render, preventing O(N) cascading VDOM re-renders for unchanged chat history items. +2. **Extract `ChatHistoryItem` as a `React.memo` component.** Currently, when `activeSessionId` changes, the entire list of `button` components re-renders because they are defined inline inside the mapping. By extracting the item to a `React.memo` component and wrapping callback props (like `onDelete`) in `useCallback` to maintain stable references, only the previously active item and the newly active item will re-render, preventing O(N) cascading VDOM re-renders for unchanged chat history items.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plan_review_request.md` at line 4, The plan omits stabilizing callback props which will defeat React.memo — wrap handlers passed to the memoized ChatHistoryItem (e.g., onDelete, onSelect, any onClick handlers) with useCallback in the parent so their references are stable across renders when activeSessionId changes; update the plan to call out adding useCallback around the functions that are passed as props to ChatHistoryItem and mention which props (onDelete, onSelect) need stabilization to preserve memo benefits.packages/desktop/src/renderer/components/ChatHistory.tsx (1)
83-87: ⚡ Quick winHandle refresh failures in delete flow.
At Line 86,
refresh()is not awaited/caught. If list refresh fails, the rejection path is silent and UI consistency is unclear. Preferawait refresh()with error handling in this callback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/desktop/src/renderer/components/ChatHistory.tsx` around lines 83 - 87, In handleDelete, the refresh call after await xibe.session.delete(id) is not awaited or error-handled; change it to await refresh() and wrap the delete+refresh sequence in try/catch to handle and log/display failures (e.g., inside handleDelete use try { await xibe.session.delete(id); await refresh(); } catch (err) { process error via logger or UI feedback }). This ensures failures in refresh() are not silently ignored and keeps UI consistent; reference handleDelete, xibe.session.delete, and refresh in your change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.jules/bolt.md:
- Line 70: Update the dated heading "## 2025-02-23 - Memoizing List Items
Requires Stable Callback Props" so the date matches when this learning was
derived (e.g., change 2025-02-23 to 2026-05-19 or 2026-05), ensuring the entry's
heading reflects the correct timestamp for the current PR; locate and edit that
exact header line in .jules/bolt.md to replace the old date with the correct
2026 date.
In `@packages/desktop/src/renderer/components/ChatHistory.tsx`:
- Around line 90-123: The time-bucket grouping computed in the useMemo for
`groups` only depends on `sessions`, so labels like "Today/Yesterday/This Week"
won't update after midnight until `sessions` changes; fix it by adding a
date-based dependency (e.g., compute a `dayKey = new Date().setHours(0,0,0,0)`
or a `currentDay` state updated at midnight) and include that
`dayKey`/`currentDay` in the useMemo dependency array alongside `sessions` so
the grouping recomputes on date rollover; update the `groups` useMemo and the
surrounding logic that defines `today`, `yesterday`, and `weekAgo` to use the
same `dayKey` reference.
- Around line 59-65: The delete control is an interactive <span> nested inside a
row <button>, causing invalid semantics and no keyboard access; update the row
element used in ChatHistory (the parent that currently renders the session row
as a <button>) to be a non-nested interactive container (e.g., <div
role="button">) that implements keyboard handlers (onKeyDown handling
Enter/Space to invoke the row activation function), and replace the delete
<span> (the element that calls onDelete with session.id and renders <Trash2 />)
with a real <button> element that has its own onClick, onKeyDown support, an
accessible aria-label/title, and calls e.stopPropagation() to avoid activating
the row when deleting. Ensure onDelete(session.id) still receives session.id and
that the new button is focusable via keyboard.
---
Nitpick comments:
In `@packages/desktop/src/renderer/components/ChatHistory.tsx`:
- Around line 83-87: In handleDelete, the refresh call after await
xibe.session.delete(id) is not awaited or error-handled; change it to await
refresh() and wrap the delete+refresh sequence in try/catch to handle and
log/display failures (e.g., inside handleDelete use try { await
xibe.session.delete(id); await refresh(); } catch (err) { process error via
logger or UI feedback }). This ensures failures in refresh() are not silently
ignored and keeps UI consistent; reference handleDelete, xibe.session.delete,
and refresh in your change.
In `@plan_review_request.md`:
- Line 4: The plan omits stabilizing callback props which will defeat React.memo
— wrap handlers passed to the memoized ChatHistoryItem (e.g., onDelete,
onSelect, any onClick handlers) with useCallback in the parent so their
references are stable across renders when activeSessionId changes; update the
plan to call out adding useCallback around the functions that are passed as
props to ChatHistoryItem and mention which props (onDelete, onSelect) need
stabilization to preserve memo benefits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f1c05a05-3177-4c6d-82ae-701541691bca
📒 Files selected for processing (3)
.jules/bolt.mdpackages/desktop/src/renderer/components/ChatHistory.tsxplan_review_request.md
| ## 2025-05-17 - Replace O(N²) array search with O(N) Set lookup in session listing | ||
| **Learning:** Using `array.some(...)` with string operations (like `replace()`) inside a loop over files creates an unnecessary O(N²) performance bottleneck, particularly when reading directories with many files (like the sessions history directory). | ||
| **Action:** When filtering files against another list of files, pre-compute a `Set` of base names outside the loop for O(1) lookups, changing the overall time complexity from O(N²) to O(N). | ||
| ## 2025-02-23 - Memoizing List Items Requires Stable Callback Props |
There was a problem hiding this comment.
Date inconsistency in learning entry.
The learning entry is dated 2025-02-23, but this PR was created on 2026-05-19 (~15 months later). If this learning was derived from the current work, the date should be updated to reflect when it was actually learned (likely 2026-05).
📅 Proposed fix
-## 2025-02-23 - Memoizing List Items Requires Stable Callback Props
+## 2026-05-19 - Memoizing List Items Requires Stable Callback Props📝 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.
| ## 2025-02-23 - Memoizing List Items Requires Stable Callback Props | |
| ## 2026-05-19 - Memoizing List Items Requires Stable Callback Props |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.jules/bolt.md at line 70, Update the dated heading "## 2025-02-23 -
Memoizing List Items Requires Stable Callback Props" so the date matches when
this learning was derived (e.g., change 2025-02-23 to 2026-05-19 or 2026-05),
ensuring the entry's heading reflects the correct timestamp for the current PR;
locate and edit that exact header line in .jules/bolt.md to replace the old date
with the correct 2026 date.
| <span | ||
| onClick={(e) => onDelete(e, session.id)} | ||
| className="shrink-0 rounded p-1 text-xibe-text-dim/50 hover:text-xibe-error hover:bg-xibe-error/10 transition-colors animate-fade-in hidden group-hover:block" | ||
| title="Delete chat" | ||
| > | ||
| <Trash2 className="h-3.5 w-3.5" /> | ||
| </span> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/desktop/src/renderer/components/ChatHistory.tsx | head -80Repository: iotserver24/Xibecode
Length of output: 3460
Delete action is not keyboard-accessible and is nested in an interactive control.
At line 59, the delete affordance is a clickable <span> nested inside the row <button>. This creates two accessibility issues: (1) invalid HTML semantics (interactive elements should not nest), and (2) keyboard users cannot access delete since the span lacks keyboard event handlers. Please restructure the row as a <div role="button"> with keyboard support, and render delete as a proper <button> element.
Suggested structure change
- <button
+ <div
onClick={() => onSelect(session.id)}
className={cn(
"flex w-full items-center gap-2.5 rounded-lg px-2 py-1.5 text-left transition-all duration-200 group",
isActive
? "text-xibe-text font-semibold bg-xibe-surface-raised"
: "text-xibe-text-secondary hover:bg-xibe-surface-raised/50 hover:text-xibe-text"
)}
+ role="button"
+ tabIndex={0}
+ onKeyDown={(e) => (e.key === 'Enter' || e.key === ' ') && onSelect(session.id)}
- >
+ >
<MessageSquare className={cn("h-3.5 w-3.5 shrink-0", isActive ? "text-xibe-text" : "text-xibe-text-dim/40 group-hover:text-xibe-text-dim/70")} />
<span className="flex-1 truncate text-[12px] font-medium leading-tight">{session.title}</span>
- <span
+ <button
+ type="button"
onClick={(e) => onDelete(e, session.id)}
className="shrink-0 rounded p-1 text-xibe-text-dim/50 hover:text-xibe-error hover:bg-xibe-error/10 transition-colors animate-fade-in hidden group-hover:block"
title="Delete chat"
+ aria-label={`Delete chat ${session.title}`}
>
<Trash2 className="h-3.5 w-3.5" />
- </span>
+ </button>
<span className="shrink-0 text-[10px] text-xibe-text-dim/40 tabular-nums block group-hover:hidden">{relativeTime(session.updated)}</span>
- </button>
+ </div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/desktop/src/renderer/components/ChatHistory.tsx` around lines 59 -
65, The delete control is an interactive <span> nested inside a row <button>,
causing invalid semantics and no keyboard access; update the row element used in
ChatHistory (the parent that currently renders the session row as a <button>) to
be a non-nested interactive container (e.g., <div role="button">) that
implements keyboard handlers (onKeyDown handling Enter/Space to invoke the row
activation function), and replace the delete <span> (the element that calls
onDelete with session.id and renders <Trash2 />) with a real <button> element
that has its own onClick, onKeyDown support, an accessible aria-label/title, and
calls e.stopPropagation() to avoid activating the row when deleting. Ensure
onDelete(session.id) still receives session.id and that the new button is
focusable via keyboard.
| const groups = useMemo(() => { | ||
| const today = new Date(); | ||
| today.setHours(0, 0, 0, 0); | ||
| const yesterday = new Date(today); | ||
| yesterday.setDate(yesterday.getDate() - 1); | ||
| const weekAgo = new Date(today); | ||
| weekAgo.setDate(weekAgo.getDate() - 7); | ||
|
|
||
| const todayItems: SessionItem[] = []; | ||
| const yesterdayItems: SessionItem[] = []; | ||
| const weekItems: SessionItem[] = []; | ||
| const olderItems: SessionItem[] = []; | ||
|
|
||
| for (const s of sessions) { | ||
| const d = new Date(s.updated); | ||
| if (d >= today) { | ||
| todayItems.push(s); | ||
| } else if (d >= yesterday) { | ||
| yesterdayItems.push(s); | ||
| } else if (d >= weekAgo) { | ||
| weekItems.push(s); | ||
| } else { | ||
| olderItems.push(s); | ||
| } | ||
| } | ||
|
|
||
| const g: { label: string; items: SessionItem[] }[] = []; | ||
| if (todayItems.length) g.push({ label: 'Today', items: todayItems }); | ||
| if (yesterdayItems.length) g.push({ label: 'Yesterday', items: yesterdayItems }); | ||
| if (weekItems.length) g.push({ label: 'This Week', items: weekItems }); | ||
| if (olderItems.length) g.push({ label: 'Older', items: olderItems }); | ||
| return g; | ||
| }, [sessions]); | ||
|
|
There was a problem hiding this comment.
Time-bucket grouping can go stale after date rollover.
At Line 90, groups only depends on sessions, so “Today/Yesterday/This Week” may remain wrong after midnight until sessions change. Include a day key dependency (or a midnight timer tick) so buckets refresh when the date changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/desktop/src/renderer/components/ChatHistory.tsx` around lines 90 -
123, The time-bucket grouping computed in the useMemo for `groups` only depends
on `sessions`, so labels like "Today/Yesterday/This Week" won't update after
midnight until `sessions` changes; fix it by adding a date-based dependency
(e.g., compute a `dayKey = new Date().setHours(0,0,0,0)` or a `currentDay` state
updated at midnight) and include that `dayKey`/`currentDay` in the useMemo
dependency array alongside `sessions` so the grouping recomputes on date
rollover; update the `groups` useMemo and the surrounding logic that defines
`today`, `yesterday`, and `weekAgo` to use the same `dayKey` reference.
iotserver24
deleted the
bolt/chat-history-memoization-13284515533250718070
branch
💡 What: Refactored the
ChatHistorycomponent to use a single O(N) pass for session grouping instead of an O(4N) pass. Extracted the list items into aChatHistoryItemcomponent wrapped withReact.memoand stabilized thehandleDeletefunction withuseCallbackto prevent cascading VDOM re-renders.🎯 Why: In a long list of chat sessions, any change in
activeSessionIdwould trigger an O(4N) array processing step, re-evaluate numerous date strings, and cause the entire DOM list to re-render.📊 Impact: Reduces array iteration from 4N to N on chat session refresh. Drops list item re-renders down from N to 2 when switching between active sessions.
🔬 Measurement: Verify by adding
console.logintoChatHistoryItemor monitoring the React DevTools profiler while clicking between different sessions in the Chat History sidebar. Expect only two re-renders per switch (the old active session and the new one).PR created automatically by Jules for task 13284515533250718070 started by @iotserver24
Summary by CodeRabbit
Documentation
Refactor