fix(ui): paginate session logs in drawer sidebar with Shift+J/K shortcuts - #28761
fix(ui): paginate session logs in drawer sidebar with Shift+J/K shortcuts#28761shenshouer wants to merge 4 commits into
Conversation
…tcuts The session sidebar in the log details drawer fetched all logs for a session in a single call, which truncated long sessions and made the sidebar slow to render. This adds page/page_size forwarding to the existing /spend/logs/session/ui endpoint and surfaces page navigation in the drawer. - networking.tsx: sessionSpendLogsCall now accepts page (default 1) and page_size (default 50) and forwards them as query params - LogDetailsDrawer.tsx: track sessionPage state, reset to 1 on session_id change, include page in query key, render a "(this page)" disclosure on the sidebar header when paginated - DrawerHeader.tsx: render previous/next page buttons with DoubleLeft/DoubleRight icons and Shift+J/K shortcut labels when showPageControls is true - useKeyboardNavigation.ts: explicit shiftKey branch routes Shift+J/K to onNextPage/onPreviousPage; plain J/K continue selecting next/prev log within the page (mutually exclusive) - useKeyboardNavigation.test.ts: 9 vitest specs covering J/K bounds, Shift+J/K page calls and mutual exclusion with selection, optional handlers safe, Escape, isOpen=false ignored, and a regression test verifying addEventListener is not re-attached when callback identities are stable Re-submission of BerriAI#28225 (silently closed during dated-staging cleanup). Same root cause as the long-open BerriAI#21300.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes a long-standing truncation bug where the session-trace sidebar silently stopped at 50 entries by wiring
Confidence Score: 5/5Safe to merge — all changed paths are UI-only, the backend already supports the added pagination params and returns total_pages/total in its response, and the new logic is well-covered by tests. The networking change forwards well-typed params to an endpoint that already handles them. State management is scoped correctly with resets on all relevant lifecycle events. The memoization strategy for the keyboard-navigation callbacks is sound and the new test suite validates all key paths including the regression guard for listener churn. No files require special attention.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/networking.tsx | sessionSpendLogsCall now accepts page/page_size params and appends them to the query string; backward-compatible defaults (page=1, page_size=50) preserve existing callers. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailsDrawer.tsx | Adds sessionPage state with correct resets on drawer close and sessionId change; derives total/totalPages from backend response; memoizes goToPreviousPage/goToNextPage and handleSelectLog with proper useCallback deps. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/DrawerHeader.tsx | Adds optional DoubleLeft/DoubleRight page-navigation buttons with ⇧K/⇧J shortcut labels behind the showPageControls flag; disabled state driven by canGoPreviousPage/canGoNextPage props. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/useKeyboardNavigation.ts | Replaces switch/case with explicit shiftKey branch so Shift+J/K route to page handlers and plain J/K continue selecting rows; onClose/onSelectLog/onPreviousPage/onNextPage added to effect deps to close the pre-existing stale-closure gap. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/useKeyboardNavigation.test.ts | New vitest suite with 9 specs covering J/K bounds, Shift+J/K mutual exclusion, optional-handler safety, Escape, isOpen=false guard, and the addEventListener re-attachment regression. |
Reviews (2): Last reviewed commit: "fix(ui/log-drawer): hide page controls o..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…eyboard listener Greptile P2: the inline onSelectLog passed to useKeyboardNavigation was recreated every render, while the effect's dependency array omitted onSelectLog. This left the keydown listener holding a stale closure across re-renders. - Wrap the inline callback in useCallback so the parent passes a stable reference - Add onClose and onSelectLog to the effect's deps so the listener re-attaches if either ever changes (the regression test still passes — stable refs from the parent prevent churn on no-op re-renders)
|
@greptileai please re-review — the previous P2 about |
Greptile P2: showPageControls was tied to isSessionMode, so the Shift+K / Shift+J buttons rendered (both disabled) even when the session had only one page. Switch to isSessionPaginated, which is already computed and is the correct guard.
… unmount flicker Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@Sameerlite 👋 friendly ping — this PR targets Current state:
Could you let me know whether anything else is needed from me to move this forward — or whether I should be tagging someone else for the OSS staging merge flow? Happy to adjust as needed. Thank you! |
|
@yuneng-berri can you review this? |
|
Adding context: another user just reported hitting this exact bug on |
|
@ishaan-berri sorry to bother you directly — wanted to flag this small UI fix that's been waiting on review. Context:
Could you either take a look or point me to whoever owns the OSS staging merge queue right now? Happy to rebase / adjust if anything is needed. Thanks! |
2 participants
Relevant issues
Fixes #28224 — original bug report (still OPEN, filed 2026-05-19): the session trace sidebar in the Log Details drawer silently truncates to the first 50 entries.
Re-submission of #28225 (silently closed on 2026-05-22 during dated-staging cleanup; verified the fix is NOT in
main,litellm_internal_staging, orlitellm_oss_stagingas of 2026-05-25).Same root cause as #21300 (open since 2026-02-16, still unresolved) —
sessionSpendLogsCallinnetworking.tsxnot passing pagination params to the backend. That PR fetches all pages eagerly; this PR keeps a single-page-at-a-time UX with explicit user-controlled paging, which is more memory-friendly for very long sessions and matches the existing main-list pagination pattern.Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory — UI test coverage added inuseKeyboardNavigation.test.ts(9 vitest specs in the dashboard test suite, since this is a UI-only change with no Python surface)make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer review (Greptile previously confirmed 5/5 on fix(ui/log-drawer): paginate session trace list to surface logs beyond the first 50 #28225 — same diff)Type
🐛 Bug Fix
🆕 New Feature
Changes
The session sidebar in the log details drawer fetched all logs for a session in a single call. The backend defaults to
page_size=50, so long sessions silently truncated to 50 entries — anything beyond that was unreachable from the UI.This PR forwards
page/page_sizeto the existing/spend/logs/session/uiendpoint and surfaces page navigation directly in the drawer withShift+J/Shift+Kkeyboard shortcuts.networking.tsx—sessionSpendLogsCallnow acceptspage(default1) andpage_size(default50) and forwards them as query params.LogDetailsDrawer.tsx— tracksessionPagestate, reset to1onsession_idchange, include page in the query key, and render a(this page)disclosure on the sidebar header when paginated.DrawerHeader.tsx— render previous/next page buttons (DoubleLeft/DoubleRighticons) with⇧K/⇧Jshortcut labels whenshowPageControlsis true.useKeyboardNavigation.ts— explicitshiftKeybranch routesShift+J/KtoonNextPage/onPreviousPage. PlainJ/Kcontinue selecting next/prev log within the page (mutually exclusive —Shift+never fires both).useKeyboardNavigation.test.ts(new) — 9 vitest specs covering J/K bounds, Shift+J/K page calls and mutual exclusion with selection, optional handlers safe, Escape,isOpen=falseignored, and a regression test verifyingaddEventListeneris not re-attached when callback identities are stable.