Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 76 additions & 1 deletion ui/providers/BottomDrawer/containers/LogViewer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ interface Props {

const LogViewerContainer: React.FC<Props> = ({ sessionId, source, toolbarPrefix, toolbarActions }) => {
const parentRef = useRef<HTMLDivElement>(null);
const [scrollElement, setScrollElement] = useState<HTMLDivElement | null>(null);
const scrollRefCallback = useCallback((node: HTMLDivElement | null) => {
parentRef.current = node;
setScrollElement(node);
}, []);
const [showTimestamps, setShowTimestamps] = useState(false);
const [showSources, setShowSources] = useState(false);
const [showLineNumbers, setShowLineNumbers] = useState(false);
Expand Down Expand Up @@ -153,6 +158,76 @@ const LogViewerContainer: React.FC<Props> = ({ sessionId, source, toolbarPrefix,
}
}, [follow, filteredLineCount, version]);

// Keep refs in sync for use inside ResizeObserver (avoids re-subscribing)
const followRef = useRef(follow);
followRef.current = follow;
const filteredLineCountRef = useRef(filteredLineCount);
filteredLineCountRef.current = filteredLineCount;
const filteredEntriesRef = useRef(filteredEntries);
filteredEntriesRef.current = filteredEntries;

// Re-pin to bottom (or restore position) when the scroll container resizes.
// This covers drawer drag-resize, minimize/re-expand, and fullscreen toggle.
// We store a timestamp (not an index) so the anchor survives buffer eviction.
const savedVisibleTimestampRef = useRef<string | null>(null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Anchor restore to a unique log entry.

Saving only entry.timestamp makes this restore ambiguous: findEntryIndexByTime() returns the first row with timestamp >= target, so duplicate timestamps snap back to the earliest sibling instead of the row that was actually visible. Save a composite anchor for the visible entry (for example timestamp + sourceId + lineNumber) and resolve that exact row first, falling back to the timestamp lookup only if the entry has been evicted or filtered out.

🔧 Suggested change
-  // We store a timestamp (not an index) so the anchor survives buffer eviction.
-  const savedVisibleTimestampRef = useRef<string | null>(null);
+  // Store a stable row identity so restore stays exact across eviction/filter changes.
+  const savedVisibleAnchorRef =
+    useRef<Pick<LogEntry, 'timestamp' | 'sourceId' | 'lineNumber'> | null>(null);

...
-        } else if (prevHeight === 0 && savedVisibleTimestampRef.current !== null) {
+        } else if (prevHeight === 0 && savedVisibleAnchorRef.current !== null) {
+          const anchor = savedVisibleAnchorRef.current;
           const idx = findEntryIndexByTime(
             filteredEntriesRef.current,
-            new Date(savedVisibleTimestampRef.current),
+            new Date(anchor.timestamp),
           );
-          savedVisibleTimestampRef.current = null;
-          if (idx >= 0) {
+          let restoreIdx = idx;
+          for (let i = idx; i >= 0 && i < filteredEntriesRef.current.length; i += 1) {
+            const candidate = filteredEntriesRef.current[i];
+            if (candidate.timestamp !== anchor.timestamp) break;
+            if (
+              candidate.sourceId === anchor.sourceId
+              && candidate.lineNumber === anchor.lineNumber
+            ) {
+              restoreIdx = i;
+              break;
+            }
+          }
+          savedVisibleAnchorRef.current = null;
+          if (restoreIdx >= 0) {
             isAutoScrolling.current = true;
-            rowVirtualizer.scrollToIndex(idx, { align: 'start' });
+            rowVirtualizer.scrollToIndex(restoreIdx, { align: 'start' });
             requestAnimationFrame(() => {
               requestAnimationFrame(() => {
                 isAutoScrolling.current = false;
               });
             });
           }
         }
...
-          if (entry?.timestamp) {
-            savedVisibleTimestampRef.current = entry.timestamp;
+          if (entry?.timestamp) {
+            savedVisibleAnchorRef.current = {
+              timestamp: entry.timestamp,
+              sourceId: entry.sourceId,
+              lineNumber: entry.lineNumber,
+            };
           }

Also applies to: 194-203, 217-219

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ui/providers/BottomDrawer/containers/LogViewer/index.tsx` at line 172, The
savedVisibleTimestampRef currently stores only entry.timestamp which makes
restore ambiguous for duplicate timestamps; change the saved anchor to a
composite key (e.g., timestamp + sourceId + lineNumber) wherever
savedVisibleTimestampRef is set and read, then on restore attempt to locate the
exact entry by that composite (match timestamp, sourceId, lineNumber) before
falling back to findEntryIndexByTime(targetTimestamp). Update the code paths
that set/read savedVisibleTimestampRef (the savedVisibleTimestampRef declaration
and its setters/consumers around the restore logic) and ensure the restore logic
resolves the exact row first and only uses the timestamp-only lookup if that
composite entry is missing due to eviction or filtering.

const lastContainerHeightRef = useRef(0);

React.useEffect(() => {
if (!scrollElement) return;

const observer = new ResizeObserver((entries) => {
const rect = entries[0]?.contentRect;
if (!rect) return;
const newHeight = rect.height;
const prevHeight = lastContainerHeightRef.current;

if (newHeight > 0 && filteredLineCountRef.current > 0) {
if (followRef.current) {
// Follow mode: always pin to bottom
isAutoScrolling.current = true;
rowVirtualizer.scrollToIndex(filteredLineCountRef.current - 1, { align: 'end' });
requestAnimationFrame(() => {
requestAnimationFrame(() => {
isAutoScrolling.current = false;
});
});
} else if (prevHeight === 0 && savedVisibleTimestampRef.current !== null) {
// Restoring from minimized without follow: resolve saved timestamp to current index
const idx = findEntryIndexByTime(
filteredEntriesRef.current,
new Date(savedVisibleTimestampRef.current),
);
savedVisibleTimestampRef.current = null;
if (idx >= 0) {
isAutoScrolling.current = true;
rowVirtualizer.scrollToIndex(idx, { align: 'start' });
requestAnimationFrame(() => {
requestAnimationFrame(() => {
isAutoScrolling.current = false;
});
});
}
}
}

// Save the first visible entry's timestamp before the container collapses
if (prevHeight > 0 && newHeight === 0 && !followRef.current) {
const range = rowVirtualizer.range;
if (range) {
const entry = filteredEntriesRef.current[range.startIndex];
if (entry?.timestamp) {
savedVisibleTimestampRef.current = entry.timestamp;
}
}
}

lastContainerHeightRef.current = newHeight;
});

observer.observe(scrollElement);
return () => observer.disconnect();
}, [scrollElement, rowVirtualizer]);

// Detect scroll position to engage/disengage follow
const handleScroll = useCallback(() => {
if (!parentRef.current || isAutoScrolling.current) return;
Expand Down Expand Up @@ -600,7 +675,7 @@ const LogViewerContainer: React.FC<Props> = ({ sessionId, source, toolbarPrefix,
) : (
/* Virtual log list */
<Box
ref={parentRef}
ref={scrollRefCallback}
onScroll={handleScroll}
onKeyDown={handleLogKeyDown}
onMouseDown={handleLogMouseDown}
Expand Down
Loading