feat: refactor logs rendering to use virtual lists#628
Conversation
Signed-off-by: vcashwin <vcashwin99@gmail.com>
📝 WalkthroughWalkthroughThis PR introduces virtualization to the LogPanel component via the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/LogPanel.tsx`:
- Around line 65-69: Auto-scroll stops after the buffer cap because the effect
only depends on filteredLogs.length, which doesn't change when the head is
trimmed in useLogStream; update the effect to depend on something that changes
when new tail entries arrive (for example an incrementing sequence or last-log
identifier emitted by useLogStream) so the effect reruns when the visible tail
moves. Concretely, expose a stable counter or lastLogId from useLogStream (or a
trimmedCount) and add that symbol to the useEffect dependency list alongside
filteredLogs.length, isOpen, virtualizer and autoScrollRef so
virtualizer.scrollToIndex(...) is invoked whenever new logs arrive even if
filteredLogs.length remains constant.
- Around line 77-82: The clipboard write in handleCopy should be wrapped in a
try/catch to prevent unhandled promise rejections and provide user feedback on
failure: call navigator.clipboard.writeText(text) inside try, setCopied(true) on
success and schedule reset, and in catch log or surface the error (and
optionally set a failure state or fallback copy method); ensure references are
to handleCopy, filteredLogs, navigator.clipboard.writeText, setCopied so you
update that exact function and its success/failure handling.
- Around line 58-63: The virtualizer is using index-based identity which breaks
when filteredLogs changes; update the useVirtualizer calls (the one creating
virtualizer in LogPanel.tsx and the second instance around the other block
referenced) to provide getItemKey: (index) => filteredLogs[index].id so the
virtualizer tracks stable log IDs, and then use the virtualRow.key (from
virtualizer.getVirtualItems()) instead of raw index when rendering each row to
keep render keys consistent with the virtualizer's identity scheme.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53f60b8b-2829-41b6-9a21-0bf293b0ac8e
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
frontend/package.jsonfrontend/src/components/LogPanel.tsxfrontend/src/hooks/useLogStream.ts
| const virtualizer = useVirtualizer({ | ||
| count: filteredLogs.length, | ||
| getScrollElement: () => scrollRef.current, | ||
| estimateSize: () => 20, | ||
| overscan: 20, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the LogPanel.tsx file
find . -name "LogPanel.tsx" -type fRepository: daydreamlive/scope
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Read the relevant sections of LogPanel.tsx
# Get file stats first
wc -l frontend/src/components/LogPanel.tsxRepository: daydreamlive/scope
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Read the useVirtualizer configuration (around lines 58-63)
sed -n '50,75p' frontend/src/components/LogPanel.tsxRepository: daydreamlive/scope
Length of output: 940
🏁 Script executed:
#!/bin/bash
# Read the render section (around lines 165-170)
sed -n '155,180p' frontend/src/components/LogPanel.tsxRepository: daydreamlive/scope
Length of output: 896
🌐 Web query:
TanStack Virtual getItemKey documentation default behavior index
💡 Result:
In TanStack Virtual, the getItemKey option defaults to returning the item’s index:
getItemKey?: (index: number) => Keyand “the default functionality … is to return the index of the item”. [1]- Consequently, each
VirtualItem’skeyis the item index by default, unless you overridegetItemKey. [2]
Sources: [1] [2]
🏁 Script executed:
#!/bin/bash
# Check if there are any other uses of virtualizer in the codebase
rg "useVirtualizer\|virtualRow\.key\|getItemKey" --type tsx --type tsRepository: daydreamlive/scope
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Search without file type filter (more flexible)
rg "useVirtualizer|virtualRow\.|getItemKey" frontend/src/components/LogPanel.tsxRepository: daydreamlive/scope
Length of output: 332
🏁 Script executed:
#!/bin/bash
# Check what properties virtualRow has in the render - look at how it's used
rg "virtualRow\." frontend/src/components/LogPanel.tsx -A 2Repository: daydreamlive/scope
Length of output: 393
🌐 Web query:
TanStack Virtual VirtualItem properties key index API
💡 Result:
In TanStack Virtual, each rendered row/column is represented by a VirtualItem with this core shape:
type Key = number | string | bigint
export interface VirtualItem {
key: Key
index: number
start: number
end: number
size: number
lane: number
}[1][2]
Key vs index
index: the item’s numeric position in your data (0…count-1). [2]key: the stable identifier for that item, produced by the virtualizer optiongetItemKey(index). By default, it’s the same asindex, but you should override it when items can be inserted/removed/reordered. [3][1]
Common usage
- React key:
key={virtualItem.key} - Positioning: translate to
virtualItem.start - Size:
virtualItem.size(andendis typicallystart + size) [1][2]
Sources:
[1] VirtualItem API docs (TanStack Virtual) (tanstack.com)
[2] @tanstack/virtual-core type definition (VirtualItem) (app.unpkg.com)
[3] Virtualizer option getItemKey docs (tanstack.com)
Citations:
- 1: https://tanstack.com/virtual/v3/docs/api/virtual-item?utm_source=openai
- 2: https://app.unpkg.com/%40tanstack/virtual-core%403.13.12/files/src/index.ts?utm_source=openai
- 3: https://tanstack.com/virtual/latest/docs/api/virtualizer?utm_source=openai
Use getItemKey to pass stable log IDs to the virtualizer.
The virtualizer currently defaults to index-based item identity, but when filteredLogs changes due to filtering (errors/cloud/all), indices shift without updating the virtualizer's measurements. With variable-height rows from text wrapping and dynamic measurement, this can cause stale measurements to be reused for different logs. Pass log.id through getItemKey so the virtualizer tracks identity correctly, then use virtualRow.key in the render to stay consistent with the virtualizer's identity scheme. (tanstack.com)
💡 Proposed fix
+ const getItemKey = useCallback(
+ (index: number) => filteredLogs[index]?.id ?? index,
+ [filteredLogs]
+ );
+
const virtualizer = useVirtualizer({
count: filteredLogs.length,
getScrollElement: () => scrollRef.current,
estimateSize: () => 20,
overscan: 20,
+ getItemKey,
});
...
- key={log.id}
+ key={virtualRow.key}Also applies to: 165-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/LogPanel.tsx` around lines 58 - 63, The virtualizer
is using index-based identity which breaks when filteredLogs changes; update the
useVirtualizer calls (the one creating virtualizer in LogPanel.tsx and the
second instance around the other block referenced) to provide getItemKey:
(index) => filteredLogs[index].id so the virtualizer tracks stable log IDs, and
then use the virtualRow.key (from virtualizer.getVirtualItems()) instead of raw
index when rendering each row to keep render keys consistent with the
virtualizer's identity scheme.
| useEffect(() => { | ||
| if (isOpen && autoScroll && bottomRef.current) { | ||
| bottomRef.current.scrollIntoView({ block: "end" }); | ||
| if (isOpen && autoScrollRef.current && filteredLogs.length > 0) { | ||
| virtualizer.scrollToIndex(filteredLogs.length - 1, { align: "end" }); | ||
| } | ||
| }, [logs, filter, autoScroll, isOpen]); | ||
| }, [filteredLogs.length, isOpen, virtualizer]); |
There was a problem hiding this comment.
Auto-scroll stops once the buffer reaches the cap.
frontend/src/hooks/useLogStream.ts Lines 44-48 start trimming from the head at 2,000 rows, so the tail keeps changing while filteredLogs.length stays flat. After that point this effect no longer reruns, and the panel stops following incoming logs even when auto-scroll is still enabled.
💡 Proposed fix
+ const lastVisibleLogId = filteredLogs[filteredLogs.length - 1]?.id;
+
useEffect(() => {
if (isOpen && autoScrollRef.current && filteredLogs.length > 0) {
virtualizer.scrollToIndex(filteredLogs.length - 1, { align: "end" });
}
- }, [filteredLogs.length, isOpen, virtualizer]);
+ }, [filteredLogs.length, lastVisibleLogId, isOpen, virtualizer]);📝 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.
| useEffect(() => { | |
| if (isOpen && autoScroll && bottomRef.current) { | |
| bottomRef.current.scrollIntoView({ block: "end" }); | |
| if (isOpen && autoScrollRef.current && filteredLogs.length > 0) { | |
| virtualizer.scrollToIndex(filteredLogs.length - 1, { align: "end" }); | |
| } | |
| }, [logs, filter, autoScroll, isOpen]); | |
| }, [filteredLogs.length, isOpen, virtualizer]); | |
| const lastVisibleLogId = filteredLogs[filteredLogs.length - 1]?.id; | |
| useEffect(() => { | |
| if (isOpen && autoScrollRef.current && filteredLogs.length > 0) { | |
| virtualizer.scrollToIndex(filteredLogs.length - 1, { align: "end" }); | |
| } | |
| }, [filteredLogs.length, lastVisibleLogId, isOpen, virtualizer]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/LogPanel.tsx` around lines 65 - 69, Auto-scroll stops
after the buffer cap because the effect only depends on filteredLogs.length,
which doesn't change when the head is trimmed in useLogStream; update the effect
to depend on something that changes when new tail entries arrive (for example an
incrementing sequence or last-log identifier emitted by useLogStream) so the
effect reruns when the visible tail moves. Concretely, expose a stable counter
or lastLogId from useLogStream (or a trimmedCount) and add that symbol to the
useEffect dependency list alongside filteredLogs.length, isOpen, virtualizer and
autoScrollRef so virtualizer.scrollToIndex(...) is invoked whenever new logs
arrive even if filteredLogs.length remains constant.
| const handleCopy = useCallback(async () => { | ||
| const text = filteredLogs.map(l => l.text).join("\n"); | ||
| await navigator.clipboard.writeText(text); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| }; | ||
| }, [filteredLogs]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "LogPanel.tsx" -type fRepository: daydreamlive/scope
Length of output: 100
🏁 Script executed:
cat -n ./frontend/src/components/LogPanel.tsx | sed -n '70,90p'Repository: daydreamlive/scope
Length of output: 842
🏁 Script executed:
cat -n ./frontend/src/components/LogPanel.tsx | head -100Repository: daydreamlive/scope
Length of output: 3798
🌐 Web query:
navigator.clipboard.writeText rejection error handling secure context
💡 Result:
navigator.clipboard.writeText() is async and rejects its Promise when the browser blocks clipboard writes—most commonly because you’re not in a secure context or you’re missing user activation / permissions / policy.
When/why it rejects
- Not a secure context: Clipboard writes only work on HTTPS (or other secure contexts like
localhost); otherwisenavigator.clipboardmay be missing or the call is blocked. [1] - NotAllowedError (DOMException): Thrown when “writing to the clipboard is not allowed” (e.g., no user gesture, permission denied, blocked by policy). [1]
- User activation / permissions differences:
- Chromium: writing requires
clipboard-writepermission or transient activation. [2] - Firefox & Safari: writing requires transient activation (typically a direct click/tap handler). [2]
- Chromium: writing requires
- If in an
<iframe>: the iframe must be allowed via Permissions-Policyclipboard-read/clipboard-write. [2]
Recommended rejection handling pattern
async function copyText(text) {
if (!window.isSecureContext) {
// e.g., you’re on http:// (not localhost)
throw new Error("Clipboard requires a secure context (HTTPS or localhost).");
}
if (!navigator.clipboard?.writeText) {
throw new Error("Clipboard API not available in this browser/context.");
}
try {
await navigator.clipboard.writeText(text);
return true;
} catch (err) {
// Common case: DOMException name === "NotAllowedError"
console.error("Clipboard write failed:", err);
return false; // optionally fall back to another copy method
}
}Practical fixes when you see rejections
- Call it directly inside a user gesture (e.g., button
clickhandler), not afterawait fetch(...)or a timer. [2] - Ensure the page is served over HTTPS / secure context. [1]
- If embedded, set appropriate Permissions-Policy for the iframe. [2]
Sources: MDN Clipboard.writeText() (secure context + NotAllowedError) [1]; MDN Clipboard API security considerations (permissions/user activation/iframe policy and browser differences) [2].
Guard the clipboard write path with error handling.
navigator.clipboard.writeText() is async and rejects when the browser blocks clipboard writes—whether due to missing secure context (non-HTTPS), denied permissions, missing user activation, or unavailable API. Without a try/catch, rejections surface as unhandled Promise rejections and the user gets no feedback if the copy fails.
💡 Proposed fix
const handleCopy = useCallback(async () => {
- const text = filteredLogs.map(l => l.text).join("\n");
- await navigator.clipboard.writeText(text);
- setCopied(true);
- setTimeout(() => setCopied(false), 2000);
+ try {
+ const text = filteredLogs.map(l => l.text).join("\n");
+ await navigator.clipboard.writeText(text);
+ setCopied(true);
+ setTimeout(() => setCopied(false), 2000);
+ } catch {
+ setCopied(false);
+ }
}, [filteredLogs]);📝 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 handleCopy = useCallback(async () => { | |
| const text = filteredLogs.map(l => l.text).join("\n"); | |
| await navigator.clipboard.writeText(text); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 2000); | |
| }; | |
| }, [filteredLogs]); | |
| const handleCopy = useCallback(async () => { | |
| try { | |
| const text = filteredLogs.map(l => l.text).join("\n"); | |
| await navigator.clipboard.writeText(text); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 2000); | |
| } catch { | |
| setCopied(false); | |
| } | |
| }, [filteredLogs]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/LogPanel.tsx` around lines 77 - 82, The clipboard
write in handleCopy should be wrapped in a try/catch to prevent unhandled
promise rejections and provide user feedback on failure: call
navigator.clipboard.writeText(text) inside try, setCopied(true) on success and
schedule reset, and in catch log or surface the error (and optionally set a
failure state or fallback copy method); ensure references are to handleCopy,
filteredLogs, navigator.clipboard.writeText, setCopied so you update that exact
function and its success/failure handling.
🚀 fal.ai Preview Deployment
TestingConnect to this preview deployment by running this on your branch: 🧪 E2E tests will run automatically against this deployment. |
❌ E2E Tests failed
Test ArtifactsCheck the workflow run for screenshots, traces, and failure details. |
1 participant
Log Panel Improvements
With the way the current log panel is implemented there is scope for improvement in how logs are rendered given they are accumulated over time and increase the DOM nodes that are rendered. Virtual lists are a way to help with this and in this PR we are:
Virtualized log rendering — Replaced the naive
.map()over up to 2,000 log lines with@tanstack/react-virtual[tiny 2Kb package]. Only visible rows (plus an overscan buffer) are now in the DOM. Auto-scroll uses the virtualizer's native
scrollToIndexinstead ofscrollIntoViewon a sentinel div.Stable log line IDs — Each
LogLinenow carries a monotonically increasingid, giving React stable keys that survive array trimming when the 2,000-line cap kicks in (previously used array index as key).Memoized filtering and row rendering —
filteredLogsis wrapped inuseMemo; individual rows are extracted into aReact.memo-wrappedLogRowcomponent so unchanged lines skip re-rendering entirely.Eliminated scroll-triggered re-renders — Converted
autoScrollfromuseStatetouseRefand stabilized handlers withuseCallback, so scrolling no longer causes the entire log panel to re-render.Summary by CodeRabbit
New Features
Refactor