fix(ui): stale filters applied after sort/page/time change on Request… - #25789
Conversation
… Logs
The useEffect that re-fetches logs on sort/page/time changes:
useEffect(() => {
if (hasBackendFilters && accessToken) {
performSearch(filters, currentPage);
}
}, [sortBy, sortOrder, currentPage, startTime, endTime, isCustomDate]);
intentionally omits `filters` and `hasBackendFilters` from its dep array
to avoid double-fetches when a filter is applied. The side-effect is a
stale-closure bug: the effect captures `filters` and `hasBackendFilters`
from the render where its deps last changed, not from the render where
the user selected, e.g., a Key Alias.
Reproduce: set Key Alias → results appear correctly → change page or
sort → the effect fires with the OLD `filters` snapshot (no key_alias)
→ API request is sent without the filter → table shows unfiltered data.
Fix: store the latest `filters` and `hasBackendFilters` in refs that are
kept in sync on every render. The sort/page/time effect reads from the
refs instead of the closure so it always uses the current filter state
without altering the dep array.
Co-Authored-By: Claude Sonnet 4 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR fixes a stale-closure bug in The fix introduces Confidence Score: 5/5Safe to merge — targeted one-file fix with no new dependencies, correct React refs pattern, and tests passing. The fix correctly applies the 'ref as always-current value' pattern. The sync effect is defined before the sort/page/time effect, so React's in-order effect execution guarantees refs are up-to-date when consumed. Initial ref values are consistent with initial state, no spurious API calls are introduced on mount, and the existing handleFilterChange → debouncedSearch path is untouched. No P0/P1 findings. No files require special attention.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/view_logs/log_filter_logic.tsx | Adds filtersRef/hasBackendFiltersRef with a sync effect (defined before the sort/page/time effect) to eliminate stale-closure capture; logic is correct and effect ordering guarantees refs are fresh when consumed. |
Sequence Diagram
sequenceDiagram
actor User
participant UI as RequestLogs UI
participant Hook as useLogFilterLogic
participant Refs as filtersRef / hasBackendFiltersRef
participant API as uiSpendLogsCall
User->>UI: Set Key Alias filter
UI->>Hook: handleFilterChange(newFilters)
Hook->>Hook: setFilters(updatedFilters)
Note over Hook: Re-render → hasBackendFilters = true
Hook->>Refs: sync effect updates filtersRef.current and hasBackendFiltersRef.current
Hook->>Hook: debouncedSearch(filters, page=1)
Hook->>API: performSearch(filters, 1) — correct filters ✓
User->>UI: Change sort column / page / time range
UI->>Hook: sortBy/currentPage/startTime prop changes
Note over Hook: Re-render → sort/page/time effect fires
Hook->>Refs: Read filtersRef.current (latest) and hasBackendFiltersRef.current (true)
Hook->>Hook: debouncedSearch.cancel()
Hook->>API: performSearch(filtersRef.current, currentPage) — filters intact ✓
Reviews (1): Last reviewed commit: "fix(ui): stale filters applied after sor..." | Re-trigger Greptile
|
LGTM |
c26e304
into
BerriAI:litellm_oss_staging_04_22_2026
… Logs (BerriAI#25789) The useEffect that re-fetches logs on sort/page/time changes: useEffect(() => { if (hasBackendFilters && accessToken) { performSearch(filters, currentPage); } }, [sortBy, sortOrder, currentPage, startTime, endTime, isCustomDate]); intentionally omits `filters` and `hasBackendFilters` from its dep array to avoid double-fetches when a filter is applied. The side-effect is a stale-closure bug: the effect captures `filters` and `hasBackendFilters` from the render where its deps last changed, not from the render where the user selected, e.g., a Key Alias. Reproduce: set Key Alias → results appear correctly → change page or sort → the effect fires with the OLD `filters` snapshot (no key_alias) → API request is sent without the filter → table shows unfiltered data. Fix: store the latest `filters` and `hasBackendFilters` in refs that are kept in sync on every render. The sort/page/time effect reads from the refs instead of the closure so it always uses the current filter state without altering the dep array. Co-authored-by: Bytechoreographer <Bytechoreographer@users.noreply.github.com>
PR 2: fix(ui): stale filters applied after sort/page/time change on Request Logs
Relevant issues
Pre-Submission checklist
npm run testpasses for affected files (29/29)@greptileaiand get Confidence Score ≥ 4/5 before requesting maintainer reviewType
🐛 Bug Fix
Changes
Root cause
useLogFilterLogichas auseEffectthat re-fetches logs whenever sort,page, or time range changes while backend filters are active:
filtersandhasBackendFiltersare intentionally omitted from the deparray to prevent double-fetches when a filter is applied (filter changes are
handled by
handleFilterChange → debouncedSearch).The side-effect is a stale-closure bug: React captures
filtersandhasBackendFiltersfrom the render where the effect was last recreated —i.e., when
sortBy,sortOrder,currentPage,startTime,endTime, orisCustomDatelast changed. If the user sets a filter (e.g. Key Alias)after that point, the effect still holds the old snapshot that predates
the filter selection.
Reproduce:
filters(nokey_alias) → API requestsent without the filter → table shows unfiltered data ✗
This is why the filter appears to "sometimes work, sometimes not": the initial
debounce-triggered search uses the correct filters, but any subsequent
sort/page/time interaction resets the results.
Fix
Store the latest
filtersandhasBackendFiltersin refs kept in sync onevery render. The sort/page/time effect reads from the refs instead of the
closure, so it always uses the current filter state without requiring
those values to be in the dep array:
Files changed
ui/litellm-dashboard/src/components/view_logs/log_filter_logic.tsxfiltersRef/hasBackendFiltersRef, sync effect, update sort/page/time effect to read from refs