Default to latest logs and fix scroll bar jump performance #54174#54447
Default to latest logs and fix scroll bar jump performance #54174#54447sujitha-saranam wants to merge 12 commits into
Conversation
There was a problem hiding this comment.
Thank you for the pull request.
I wouldn't reverse the logs in the frontend because it will cause many different problem, maybe we can just scroll to bottom on loading. (log reverse should be done in the backend, because if we start paginating / streaming logs reversing just in the UI will not work)
With the current implementation I see a few issues:
Groups are weird

First line is truncated

Once I click on the "scroll to bottom" button, I cannot scroll up.
Screen.Recording.2025-08-18.at.18.07.14.mov
Screencast.from.2025-08-20.18-59-02.mp4I have fixed the logs reversal from the backend and also addressed the UI issues. I’ve attached a screen recording of my local setup showing these updates. Could you please check and let me know if everything looks fine, or if there are any external changes I should also make? Thanks! |
…while jumping to the end of the logs
639c813 to
ac154bb
Compare
pierrejeambrun
left a comment
There was a problem hiding this comment.
Why do we need to update the backend ordering there? Also I didn't take a closer look but we most probably can't do that because that should be at the reader lvl to reverse order. (start from the end of the file and cursor back to the beginning)
jason810496
left a comment
There was a problem hiding this comment.
Even we have the another UI endpoint for this feature, the builit-in reversed will still cause the OOM problem for API server when the log is large.
|
Iactually would not like to read logs in reverse order. I this is made then it would need to be an optional feature, should not require all users to read logs in reverse order. |
| log_stream = task_log_reader.read_log_stream(ti, try_number, metadata) # type: ignore[arg-type] | ||
| ndjson_log_stream = list(task_log_reader.read_log_stream(ti, try_number, metadata)) # type: ignore[arg-type] | ||
| reversed_stream = reversed(ndjson_log_stream) | ||
| log_stream = (line for line in reversed_stream) |
There was a problem hiding this comment.
Isn’t this just iter(reversed_stream)?
|
I am reverting the back-end changes as they were optional. Please let me know if any other changes i should do. |
| return; | ||
| } | ||
|
|
||
| const targetIndex = Math.max(0, Math.min(parsedLogs.length - 1, Number(hash) || 0)); | ||
| const el = parentRef.current; | ||
|
|
||
| const total = rowVirtualizer.getTotalSize(); | ||
| const clientH = el?.clientHeight ?? 0; |
There was a problem hiding this comment.
Both clicking on a log line, or refreshing the page with a URL with a line number specified will not land where we want. http://localhost:28080/dags/big_logs/runs/manual__2025-09-02T12:02:44.060539+00:00/tasks/my_task?try_number=1#4806
There was a problem hiding this comment.
Can we use something like https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-margin ?
| return; | ||
| } | ||
|
|
||
| // === bottom === (instant jump even for huge lists) |
There was a problem hiding this comment.
There's a comment for bottom, but not for top. Remove it
| // === bottom === (instant jump even for huge lists) |
| try { | ||
| rowVirtualizer.scrollToOffset(offset); | ||
| } catch { | ||
| rowVirtualizer.scrollToIndex(parsedLogs.length - 1, { align: "end" }); | ||
| } |
There was a problem hiding this comment.
This should be enough. We can remove the typeof check. (undefined is not callable) (same above)
| requestAnimationFrame(() => { | ||
| el.scrollTop = offset; | ||
| requestAnimationFrame(() => { | ||
| el.scrollTop = offset; | ||
| const lastItem = el.querySelector<HTMLElement>( | ||
| `[data-testid="virtualized-item-${parsedLogs.length - 1}"]`, | ||
| ); | ||
|
|
||
| if (lastItem) { | ||
| lastItem.scrollIntoView({ behavior: "auto", block: "end" }); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
What are those requestAnimationFrame achieving? I Tried without them and it still look fine.
|
@sujitha-saranam can you address above comments? |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |

Overview
This PR addresses #54174. It fixes the default scroll to the most recent logs and ensures that jump-to-start/end buttons scroll instantly.
Root Cause:
Jumping to the end is slow because the virtualizer has to compute/measure positions for a large number of rows before it can accurately place the scroll. For very long logs, this causes the temporary slow scroll effect.
Solution
Changes Made
useLayoutEffectto compute exact scroll offsets based on virtual item positions.handleScrollToto combinescrollToIndexwithscrollTopviarequestAnimationFramefor instant jumps.bgColorcalculation for virtualized items to correctly highlight the log matching the hash.CodeandVStackcontainer styling to match virtualized content height and improve scroll behavior.closes: #54174
related: #54174
closes: #51788