fix(ui/log-drawer): paginate session trace list to surface logs beyond the first 50#28225
Conversation
…ing (BerriAI#28156) * feat(ui): add Interactions API support to playground with streaming Adds /v1beta/interactions as a selectable endpoint in the UI playground. Uses SSE streaming (stream=true) and parses content.delta events for real-time output. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(interactions): remove forced gemini provider so all providers work via interactions API Proxy endpoint was hardcoding custom_llm_provider="gemini" before routing, preventing non-Gemini models from using the litellm_responses bridge. Also reverts the UI Gemini-only model filter. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(interactions): fix streaming for non-gemini providers via bridge Two bugs in LiteLLMResponsesInteractionsStreamingIterator: 1. content.delta was emitted without "type":"text" in delta dict, so the UI type-check always failed and no tokens were displayed 2. First OutputTextDeltaEvent was silently dropped (used to emit content.start with empty text); fixed by handling ResponsePartAddedEvent for content.start so text deltas go directly to content.delta Co-authored-by: Cursor <cursoragent@cursor.com> * undo unrelated changes * fix(ui): extract model from top-level field in interactions bridge events Co-authored-by: Yassin Kortam <yassin@berri.ai> * test(interactions): remove tautological gemini-provider assertion The test_no_forced_gemini_provider_in_request_data check only asserted against dict literals it had just constructed, so it always passed and did not exercise the create_interaction endpoint. The endpoint deliberately defaults custom_llm_provider to gemini, so the assertion was also factually incorrect. Drop the misleading test. Co-authored-by: Yassin Kortam <yassin@berri.ai> * fix(interactions): use ContentPartAddedEvent and guard interaction.start ordering - ResponsePartAddedEvent corresponds to reasoning summary parts, not text content parts. Use ContentPartAddedEvent which is the event emitted before text output deltas (type response.content_part.added). - Mirror the OutputTextDeltaEvent ordering guard: if interaction.start has not been sent yet, emit it first before content.start to honor the documented event ordering contract. Co-authored-by: Yassin Kortam <yassin@berri.ai> * test(interactions): cover ContentPartAddedEvent ordering and no-op paths * fix(tests): treat corrupt VCR cassette payloads as cache miss + use gpt-realtime in OpenAI realtime guardrails test VCR redis persister was raising UnicodeDecodeError on cached payloads that fail to UTF-8 decode (e.g. legacy entries written by another version of the persister), failing tests at fixture setup instead of degrading to a cache miss. Wrap decode+deserialize in a try/except so corrupt cache entries are treated as CassetteNotFoundError, surfacing the failure via the existing _record_cache_failure / VCRCassetteCacheWarning path. OpenAI shut down gpt-4o-realtime-preview-2024-12-17 (and the entire gpt-4o-realtime-preview family) on 2026-05-07. The live realtime guardrails integration test now fails with model_not_found instead of receiving session.created. Point OPENAI_REALTIME_URL at the current GA model gpt-realtime, and relax the assertion in test_text_message_blocked_by_guardrail_no_ai_response to also accept the model's refusal-to-repeat the block message (gpt-realtime declines verbatim-repeat instructions, which is still a safe outcome since the original user message was blocked before reaching OpenAI). The BLOCKED_PHRASE leak check is preserved as a hard invariant. * fix(tests): migrate realtime + nvidia_nim rerank tests off shut-down upstream models OpenAI shut down the entire gpt-4o-realtime-preview family (including the undated alias) on 2026-05-07. The live realtime tests still connected with that dead alias and failed with messages_received=1 (an error event 'The model gpt-4o-realtime-preview does not exist' instead of session.created). Point the live OpenAI realtime tests at gpt-realtime, the current GA realtime model: - test_openai_realtime_simple.py: get_model() -> gpt-realtime - test_openai_realtime.py: test_openai_realtime_direct_call_no_intent and test_openai_realtime_direct_call_with_intent -> openai/gpt-realtime Mocked unit tests (test_realtime_query_params_construction, test_realtime_query_params_use_normalized_model_name) are left as-is: they never hit the network and assert string plumbing only. NVIDIA reached end-of-life for the hosted nvidia/llama-3.2-nv-rerankqa-1b-v2 rerank API on 2026-05-18 with no published replacement, so the live BaseLLMRerankTest.test_basic_rerank for nvidia_nim now returns HTTP 410 ('Gone'). NVIDIA's hosted catalog rotates on a schedule, so swapping in another live model would only defer the failure. Override test_basic_rerank in TestNvidiaNim to mock the sync/async HTTP transport (same pattern as test_nvidia_nim_rerank_ranking_endpoint in this file) and inject a fake NVIDIA_NIM_API_KEY via monkeypatch. The request/response transformation and cost calculation stay covered offline. * test(callbacks): harden flaky proxy callback-leak detector The proxy callback-leak detector (test_check_num_callbacks_on_lowest_latency) was failing on this PR with 'abs(85 - 95) <= 4' — a bounded one-time registration jump caused by switching to latency-based-routing (+LowestLatencyLoggingHandler, +SlackAlerting). The count then plateaus under load, so this is pollution from the test's own config update, not a leak. Replace the brittle two-sample diff threshold with a sampler that settles past the deliberate config switch and only flags sustained monotonic per-type growth, with a terminal-burst confirmation pass for leaks that would otherwise escape the >=2-interval guard. Normalizes instance addresses so identical callbacks at different memory locations collapse, and names the leaking type on failure. * fix(interactions): preserve first text token when both start events are missing When OutputTextDeltaEvent arrived before any ResponseCreatedEvent or ContentPartAddedEvent, the double-fallback path emitted interaction.start and silently dropped the first delta's text — the second delta's content.start carried only that chunk's delta, and the first token never made it to any content.delta event consumed by the UI. Queue a content.start that carries the first delta's text alongside the interaction.start emission, and drain pending events before pulling the next upstream chunk. * chore(ui): remove unused InteractionOutput/InteractionResponse interfaces Co-authored-by: Yassin Kortam <yassin@berri.ai> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Yassin Kortam <yassin@berri.ai> Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
…mGenerateContent (BerriAI#27444) (BerriAI#28213) * fix(proxy): decode bytes and pass-through SSE for Google-native streamGenerateContent (BerriAI#27444) * fix(proxy): address Greptile review on Google-native SSE bytes path Remove unreachable try/except around SSE pass-through yield and add a unit test covering pre-formatted SSE bytes, terminator padding, and non-SSE byte fallback wrapping. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Tai An <antai12232931@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
BerriAI#28189) * refactor(bedrock/sagemaker): switch to lazy loading for response stream shapes - Replace eager loading of BEDROCK_RESPONSE_STREAM_SHAPE and SAGEMAKER_RESPONSE_STREAM_SHAPE with lazy loading via get_bedrock_response_stream_shape() and get_sagemaker_response_stream_shape() respectively. - This change optimizes performance by avoiding unnecessary imports and logging warnings unless the response stream shapes are actually needed. - Update relevant classes and tests to utilize the new lazy loading functions, ensuring consistent behavior across the codebase. * test(bedrock/sagemaker): add fixtures to clear response stream shape cache - Introduced `_reset_bedrock_response_stream_shape_cache` and `_reset_sagemaker_response_stream_shape_cache` fixtures to prevent lru_cache leakage between tests in their respective modules. - Updated tests to utilize these fixtures, ensuring that the response stream shape cache is cleared before and after each test run. - Added `pytest.importorskip("botocore")` to ensure that tests are skipped if the botocore library is not available.
|
|
Merging this PR will not alter performance
Comparing 
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes a silent 50-entry truncation in the session log sidebar by threading backend pagination (
Confidence Score: 5/5UI-only change scoped to the session log sidebar; no backend modifications, no auth or data-mutation paths touched. The two issues raised in the previous review round are both resolved: No files require special attention.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/networking.tsx | Adds page and page_size query params to sessionSpendLogsCall with backwards-compatible defaults; no issues found. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailsDrawer.tsx | Adds sessionPage state, useCallback-stabilised page-nav functions, a second useEffect to reset on sessionId change, and the sessionTotal fix for the req-count display; both previously flagged issues are resolved. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/DrawerHeader.tsx | Adds optional DoubleLeft/DoubleRight page-nav buttons that only render in session mode; interface and defaults are clean. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/useKeyboardNavigation.ts | Adds Shift+J/Shift+K page shortcuts with correct e.shiftKey guard; onPreviousPage/onNextPage correctly added to effect deps; pre-existing onClose/onSelectLog stale-closure pattern unchanged. |
| ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/useKeyboardNavigation.test.ts | New vitest spec covers J/K row navigation, Shift+J/K page navigation, boundary no-ops, safe-no-op without handler, Escape, closed-drawer guard, and stable-callback regression test; no issues found. |
Reviews (5): Last reviewed commit: "fix(ui/log-drawer): stabilise page-nav c..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Paginate Seesion Log Details drawer |
|
🤖 litellm-agent: This PR is currently BLOCKED from merge. Score: 2/5 ❌ Why blocked:
Details: Score docked for: 1 PR-related CI failure (Greptile gate: score 3/5 below required 4/5 — request a Greptile review ( Fix the issues above and push an update — the bot will re-review automatically.
|
|
No Seesion Log Details drawer |
|
Thanks for the review @greptileai — both findings were valid. Pushed P1: P2: per-page aggregate stats labeled as session totals
Tests in @greptileai please re-review. |
|
@greptileai re: the unstable function refs comment — fixed in the latest push:
Vitest in Please re-review when convenient. |
|
Hi maintainers — the
But Could you confirm which branch external/fork PRs should actually target? Happy to retarget the base of this PR once I know the right one. CONTRIBUTING.md doesn't currently document this either — I can also send a small docs PR to clarify if helpful. Source of the message: |
|
Update: retargeted the base to |
|
@greptileai review |
|
@greptileai please re-review the latest commits — |
|
🤖 litellm-agent: This PR is currently BLOCKED from merge. Score: 0/5 ❌ Why blocked:
Details: Score docked for: all Phase B agent checks non-approving (karpathy + security + coverage gap). Fix the issues above and push an update — the bot will re-review automatically.
|
|
@oss-pr-review-agent-shin Could you please post the specific feedback from each Phase B agent (karpathy / security / coverage gap)? The summary score (0/5) doesn't include actionable details, so I can't tell what needs to change. For context, this PR is a UI-only change in
Happy to address concrete findings, but I need to know what they are. |
|
Hi maintainers — could someone help unblock this PR? The For context, this PR is purely a UI-only change in
Two questions:
Happy to address concrete findings — just need visibility into them. Thanks! |
…cuts The session-mode trace sidebar was implicitly capped at the backend default page_size=50, so sessions with more than 50 spend logs silently truncated to the earliest 50. This wires the existing /spend/logs/session/ui pagination through the UI: - sessionSpendLogsCall now accepts page/page_size and forwards them. - LogDetailsDrawer keeps a sessionPage state, includes it in the React Query key, and reads total_pages from the response. Page-prev / next callbacks reset the selected request id so the effect picks the first row of the new page. - DrawerHeader's NavigationSection grows two new buttons (DoubleLeft / DoubleRight) that mirror the K/J style and only render in session mode; they disable at page boundaries. - useKeyboardNavigation routes Shift+J / Shift+K to onNextPage / onPreviousPage. Plain J/K still walks rows within the current page (we now check e.shiftKey to keep the two paths mutually exclusive). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…yboardNavigation The split between row-walk (J/K) and page (Shift+J/K) is the riskiest part of the previous commit — with shift held, e.key is already "J"/"K" (uppercase), so the natural switch on e.key conflates the two paths. This adds a vitest spec asserting: - J / K walk the current page within bounds and no-op at head/tail - Shift+J / Shift+K invoke the page handlers and never fall through to the row-walk handlers - Shift+J without an onNextPage handler is a safe no-op (doesn't silently walk to the next row) - Escape and isOpen=false honor existing semantics Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two issues raised by Greptile (BerriAI#28225): P1 — sessionId change while drawer open The page-reset effect only watched [open, isSessionMode], so swapping sessionId without closing the drawer could request a non-existent page (e.g. clicking from a 137-row session on page 3 directly to a 20-row session). Add a dedicated effect keyed on sessionId that clears sessionPage and the selected request id. P2 — per-page aggregates were labeled as session totals totalSessionCost / sessionDurationSeconds / llmCount / agentCount / mcpCount are now derived from the current page only, but the sidebar header presented them as session totals. Surface the backend's total-row count for "{N} req", and append a "page X/Y (cost/duration/ counts shown for this page)" disclosure when total_pages > 1 so the narrower scope of the other aggregates is explicit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…r churn Greptile P2 (BerriAI#28225): goToPreviousPage / goToNextPage were plain inline arrow functions, so they got new identities every render. Because useKeyboardNavigation depends on them in its effect deps, the global keydown listener was torn down and re-attached on every parent re-render. - Wrap goToPreviousPage / goToNextPage with useCallback, scoped to the matching boundary flag (canGoPreviousPage / canGoNextPage). - Drop the `boundary ? fn : undefined` ternary at the call site — the callbacks self-guard against out-of-bounds, and the ternary itself produced a fresh value each render, defeating the useCallback. - Add a regression test in useKeyboardNavigation.test.ts that spies on window.addEventListener and asserts the keydown listener is not re-registered when the hook re-renders with identical prop refs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
shenshouer
force-pushed
the
feat/log-drawer-session-pagination
branch
from
31dccf1 to
0bd812d
Compare
|
recheck |
|
@ishaan-jaff @mateo-berri sorry to ping — would either of you have a chance to take a look when you have a moment? Quick status:
The change is UI-only (4 frontend files + 1 new vitest spec) and fixes the silent 50-entry truncation in the session log sidebar described in #28224. Happy to address any concerns. |
|
@yuneng-berri sorry to ping — saw you've been active in the repo recently and you're already a CLA signer on this PR. Would you have a moment to take a look or help route this for review/merge? Quick status:
The change is UI-only — fixes the silent 50-entry truncation in the session log sidebar (#28224). Happy to address any concerns. Thanks! |
shenshouer
changed the base branch from
litellm_oss_staging
to
shin_agent_oss_staging_05_21_2026
|
Update: I noticed today's external PRs (e.g. #28431, #28430, #28449) all target If a different dated branch is preferred, happy to redirect again. |
4 participants
Relevant issues
Fixes #28224
Pre-Submission checklist
tests/test_litellm/directory — see note below; tests live next to the changed files inui/litellm-dashboard/...per existing convention.make test-unit— UI-only PR; rannpx vitest run src/components/view_logs/LogDetailsDrawer/→ 109 specs pass (Pythonmake test-unitis not applicable to a frontend-only change).@greptileaiand received a Confidence Score of at least 4/5 — Greptile gave 5/5 (see review comment dated 2026-05-19T07:10Z); both initially flagged P1/P2 issues addressed in follow-up commits.Type
🐛 Bug Fix
Changes
When the Log Details drawer opens in session mode, the left "Session" sidebar fetches
/spend/logs/session/uito enumerate every spend log for that session. The frontend ignored the endpoint's pagination params, so on sessions with more than 50 entries the sidebar silently truncated to the earliest 50 (ordered bystartTime ASC) — see #28224 for the reproduction.This PR threads the existing backend pagination (
page,page_size,total_pages) through the UI:ui/litellm-dashboard/src/components/networking.tsx—sessionSpendLogsCallnow takespage/page_sizeand forwards them in the query string. Defaults preserve the previous behavior.view_logs/LogDetailsDrawer/LogDetailsDrawer.tsx— addssessionPagestate, includes it in the React Query key so the page is keyed independently, and readstotal_pagesfrom the response.goToPreviousPage/goToNextPageclear the selected request id so the existing effect picks the first row of the new page. Page state resets to 1 on drawer close.view_logs/LogDetailsDrawer/DrawerHeader.tsx—NavigationSectiongrows two new buttons (DoubleLeftOutlined/DoubleRightOutlined) that mirror the K/J style, only render in session mode, and disable at page boundaries.view_logs/LogDetailsDrawer/useKeyboardNavigation.ts— routesShift+J/Shift+KtoonNextPage/onPreviousPage. PlainJ/Kstill walks rows within the current page; the hook now checkse.shiftKeyso the two paths stay mutually exclusive (with shift held,e.keyis already"J"/"K", which would otherwise conflate them).Tests
view_logs/LogDetailsDrawer/useKeyboardNavigation.test.ts(new) covers:J/Kwalk the current page within bounds and no-op at head/tailShift+J/Shift+Kinvoke the page handlers and never fall through to row-walkShift+Jwithout anonNextPagehandler is a safe no-op (doesn't silently walk forward)EscapeandisOpen=falsehonor existing semanticsManual verification
⇧Kbutton and Shift+K disabled.⇧J/ Shift+J → page 2, sidebar refreshes,currentLogjumps to the new page's first row, both prev and next page controls are enabled.⇧Jagain → page 3,⇧Jbutton disabled.J/Kcontinue to walk only within the visible page (50 rows).session_id): the page-control buttons are not rendered, andShift+J/Shift+Kare no-ops; existing behavior unchanged.What this does NOT do
page_size=50or upper bound100. If a maintainer would prefer auto-fetching all pages over surfacing pagination controls, happy to re-spin.Screenshots / Proof of Fix
(UI-only change; will attach screenshots in a follow-up comment after the PR opens.)