fix: enable load more for realtime bots on non-live intervals#258
fix: enable load more for realtime bots on non-live intervals#258alexanderwanyoike merged 2 commits intodevfrom
Conversation
📝 WalkthroughWalkthroughPagination for bot logs now surfaces REST-derived Changes
Sequence DiagramsequenceDiagram
participant User
participant Component as BotDetailPanel
participant Hook as useBotLogsStream
participant API as REST API
rect rgba(100,150,200,0.5)
Note over Component,API: REST pagination (Load More)
User->>Component: Click "Load More"
Component->>Hook: call loadMore()
Hook->>Hook: advance offset, set query
Hook->>API: fetchLogs(query with offset)
API-->>Hook: return logs + hasMore
Hook->>Hook: setHasMore(result.hasMore), merge "append", dedupe, trim
Hook-->>Component: updated logs & hasMore
Component->>Component: re-render, update load button
end
rect rgba(100,200,100,0.5)
Note over Component,Hook: Clear date filter -> SSE reconnect
User->>Component: Clear date range
Component->>Hook: setDateFilter(null)
Hook->>Hook: abort in-flight REST, setHasMore(false), reconnect SSE
Hook-->>Component: connected: true, hasMore: false
Component->>Component: disable "Load More", show live stream
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/hooks/use-bot-logs-stream.ts`:
- Around line 400-409: loadMore currently sets the next offset early and calls
fetchLogs(nextQuery, true) which triggers the prepend branch, causing page N+1
to be inserted before page N and allowing duplicate/failed pagination; change
loadMore to use a dedicated loadingMore guard (e.g., loadingMore state), set
loadingMore = true and ensure a loading flag is set before calling fetchLogs,
call fetchLogs with an explicit append mode (or a new parameter/useCase that
causes logs to be appended to the tail rather than prepended), await the
request, and only on success call setQuery(nextQuery) to commit the new offset
and clear loadingMore; on failure clear loadingMore without advancing the offset
so retries don't skip pages. Ensure footer/endReached handlers respect
loadingMore to avoid duplicate requests.
- Around line 353-356: The SSE reconnection branch resets pagination then calls
connectSSE() while a REST fetchLogs() may still be resolving; abort the
in-flight REST request first by calling abortAll() or explicitly aborting
restAbortRef.current before connectSSE() so stale REST results cannot overwrite
the live buffer/hasMore—update the else branch that calls setHasMore(false) and
connectSSE() to abort restAbortRef.current (or call abortAll()) immediately
prior to invoking connectSSE().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 28272be7-f420-4e42-8ee6-065aee77e6a8
📒 Files selected for processing (3)
frontend/src/components/dashboard/bot-detail-panel.tsxfrontend/src/hooks/__tests__/use-bot-logs-stream.test.tsfrontend/src/hooks/use-bot-logs-stream.ts
| const loadMore = useCallback(async () => { | ||
| if (loading || !hasMore || !botId || !user) return; | ||
|
|
||
| const nextQuery = { | ||
| ...query, | ||
| offset: (query.offset || 0) + (query.limit || 100), | ||
| }; | ||
| setQuery(nextQuery); | ||
| await fetchLogs(nextQuery, true); | ||
| }, [loading, hasMore, botId, user, query, fetchLogs]); |
There was a problem hiding this comment.
loadMore() is using the wrong merge direction and optimistic state update.
At Line 408, fetchLogs(nextQuery, true) reuses the prepend branch from Line 185, so page N+1 is inserted before page N even though date-range pagination advances forward by offset. Also, append requests never set loading, and setQuery(nextQuery) happens before the request succeeds, so repeated footer/endReached callbacks can fire duplicate requests and a failed fetch will skip a page on retry. Use a dedicated loadingMore guard, append forward pages at the tail, and only commit the new offset after success.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/hooks/use-bot-logs-stream.ts` around lines 400 - 409, loadMore
currently sets the next offset early and calls fetchLogs(nextQuery, true) which
triggers the prepend branch, causing page N+1 to be inserted before page N and
allowing duplicate/failed pagination; change loadMore to use a dedicated
loadingMore guard (e.g., loadingMore state), set loadingMore = true and ensure a
loading flag is set before calling fetchLogs, call fetchLogs with an explicit
append mode (or a new parameter/useCase that causes logs to be appended to the
tail rather than prepended), await the request, and only on success call
setQuery(nextQuery) to commit the new offset and clear loadingMore; on failure
clear loadingMore without advancing the offset so retries don't skip pages.
Ensure footer/endReached handlers respect loadingMore to avoid duplicate
requests.
There was a problem hiding this comment.
This has been addressed please review
4f0f42c to
ff421f3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/hooks/use-bot-logs-stream.ts (1)
403-412:⚠️ Potential issue | 🟠 Major
loadMorecan fire duplicate requests and skip pages on failure.Two issues remain from the previous review:
No loading guard for append:
fetchLogswith"append"mode doesn't setloadingtotrue(line 145), so theloadingguard at line 404 won't prevent duplicate requests from rapid clicks.Optimistic offset update:
setQuery(nextQuery)at line 410 commits the new offset before the request succeeds. If the fetch fails, the offset is already advanced, causing the next retry to skip a page.Consider adding a dedicated
loadingMorestate:Suggested fix
+ const [loadingMore, setLoadingMore] = useState(false); + const loadMore = useCallback(async () => { - if (loading || !hasMore || !botId || !user) return; + if (loadingMore || !hasMore || !botId || !user) return; const nextQuery = { ...query, offset: (query.offset || 0) + (query.limit || 100), }; - setQuery(nextQuery); - await fetchLogs(nextQuery, "append"); - }, [loading, hasMore, botId, user, query, fetchLogs]); + setLoadingMore(true); + try { + await fetchLogs(nextQuery, "append"); + setQuery(nextQuery); // Only commit offset on success + } finally { + setLoadingMore(false); + } + }, [loadingMore, hasMore, botId, user, query, fetchLogs]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/use-bot-logs-stream.ts` around lines 403 - 412, The loadMore handler can send duplicate requests and prematurely advance pagination; add a dedicated loadingMore state (e.g., const [loadingMore, setLoadingMore]) and check it in the early-return guard alongside loading/hasMore/botId/user in loadMore; inside loadMore setLoadingMore(true) before calling fetchLogs and wrap the await fetchLogs(nextQuery, "append") in try/finally to setLoadingMore(false) so rapid clicks are blocked; do not call setQuery(nextQuery) optimistically—only call setQuery(nextQuery) after fetchLogs resolves successfully (or after verifying the returned page) so the offset is not advanced on failure. Ensure references to loadMore, fetchLogs, setQuery, and the new loadingMore state are used consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/hooks/use-bot-logs-stream.ts`:
- Line 193: The current call to setHasMore(result.hasMore) updates
forward-pagination state even when fetchLogs is called with mode "prepend" (used
by loadEarlierLogs), which breaks the semantics; modify the handler in
use-bot-logs-stream (the fetch response processing for
fetchLogs/loadEarlierLogs) to only call setHasMore(result.hasMore) when the
fetch mode is not "prepend" (e.g., if (mode !== "prepend") setHasMore(...)),
leaving prepend responses to only update the log list without touching hasMore.
---
Duplicate comments:
In `@frontend/src/hooks/use-bot-logs-stream.ts`:
- Around line 403-412: The loadMore handler can send duplicate requests and
prematurely advance pagination; add a dedicated loadingMore state (e.g., const
[loadingMore, setLoadingMore]) and check it in the early-return guard alongside
loading/hasMore/botId/user in loadMore; inside loadMore setLoadingMore(true)
before calling fetchLogs and wrap the await fetchLogs(nextQuery, "append") in
try/finally to setLoadingMore(false) so rapid clicks are blocked; do not call
setQuery(nextQuery) optimistically—only call setQuery(nextQuery) after fetchLogs
resolves successfully (or after verifying the returned page) so the offset is
not advanced on failure. Ensure references to loadMore, fetchLogs, setQuery, and
the new loadingMore state are used consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e96e3cdc-1cb5-48fc-a3b2-15feb1e328ca
📒 Files selected for processing (3)
frontend/src/components/dashboard/bot-detail-panel.tsxfrontend/src/hooks/__tests__/use-bot-logs-stream.test.tsfrontend/src/hooks/use-bot-logs-stream.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/hooks/tests/use-bot-logs-stream.test.ts
5b2eba5 to
774a3cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
frontend/src/hooks/use-bot-logs-stream.ts (2)
397-406:⚠️ Potential issue | 🟠 Major
loadMore()still commits the next offset before the page is secured.Append fetches never toggle
loading, so repeated footer/endReached calls can re-enter here. BecausesetQuery(nextQuery)runs beforefetchLogs()completes—andfetchLogs()aborts any prior REST request—the second invocation can cancel page N while still advancing to page N+1. Add a dedicatedloadingMoreguard and havefetchLogs()return a success signal so the offset is only committed after a successful append.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/use-bot-logs-stream.ts` around lines 397 - 406, The loadMore function advances the offset before the append completes, allowing concurrent calls to cancel in-flight fetches; add a new loadingMore guard checked alongside loading/hasMore/botId/user, set loadingMore = true before calling fetchLogs, have fetchLogs return a success boolean (true on successful append), and only call setQuery(nextQuery) after fetchLogs resolves true; finally ensure loadingMore is set false in a finally block so subsequent loads are allowed. Reference functions/vars: loadMore, fetchLogs, setQuery, loading, hasMore, botId, user, query, loadingMore.
183-183:⚠️ Potential issue | 🟠 MajorKeep
hasMorescoped to the active historical query.
hasMoresurvives filter-to-filter switches, and Line 183 still updates it from"prepend"/ non-filtered REST fetches. That leaves the previous footer visible long enough forendReachedor a click to request page 2 of the new range before page 1 has landed. Clear it before each new date/dateRange fetch, and only repopulate it from non-prepend historical requests.Suggested fix
- setHasMore(result.hasMore); + if (merge !== "prepend" && (effectiveQuery.date || effectiveQuery.dateRange)) { + setHasMore(result.hasMore); + }if (date) { sseAbortRef.current?.abort(); sseAbortRef.current = null; stopPolling(); setConnected(false); + setHasMore(false); fetchLogs(updatedQuery); }stopPolling(); setConnected(false); + setHasMore(false); fetchLogs(updatedQuery);Also applies to: 342-347, 372-376
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/use-bot-logs-stream.ts` at line 183, The hasMore state is being updated globally and leaks across filter/date-range switches; modify the logic in useBotLogsStream so that before initiating any new date or dateRange historical fetch you clear hasMore (call setHasMore(false)), and only setHasMore(result.hasMore) from the historical REST responses when the response indicates a non-prepend historical fetch (e.g., guard the setHasMore call with a check that the fetch source is not "prepend" or that the request was a historical non-prepend fetch in the functions that process historical results such as the handler that processes fetchHistoricalLogs / the historical branch in the main fetch logic), and apply the same guard/update change to the other affected spots referenced around the file (the blocks at ~lines 342-347 and 372-376).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/hooks/use-bot-logs-stream.ts`:
- Around line 174-177: The current prepend branch builds combined = [...unique,
...prev] then always returns combined.slice(-MAX_LOG_ENTRIES), which drops the
newly prepended page when the buffer is full; update the return so that when
merge === "prepend" you keep the newest items from the front (use
combined.slice(0, MAX_LOG_ENTRIES)) and when merge !== "prepend" keep the tail
(combined.slice(-MAX_LOG_ENTRIES)), ensuring loadEarlierLogs()/hasEarlierLogs
actually adds earlier rows; adjust the logic around
combined/merge/MAX_LOG_ENTRIES in use-bot-logs-stream.ts accordingly.
---
Duplicate comments:
In `@frontend/src/hooks/use-bot-logs-stream.ts`:
- Around line 397-406: The loadMore function advances the offset before the
append completes, allowing concurrent calls to cancel in-flight fetches; add a
new loadingMore guard checked alongside loading/hasMore/botId/user, set
loadingMore = true before calling fetchLogs, have fetchLogs return a success
boolean (true on successful append), and only call setQuery(nextQuery) after
fetchLogs resolves true; finally ensure loadingMore is set false in a finally
block so subsequent loads are allowed. Reference functions/vars: loadMore,
fetchLogs, setQuery, loading, hasMore, botId, user, query, loadingMore.
- Line 183: The hasMore state is being updated globally and leaks across
filter/date-range switches; modify the logic in useBotLogsStream so that before
initiating any new date or dateRange historical fetch you clear hasMore (call
setHasMore(false)), and only setHasMore(result.hasMore) from the historical REST
responses when the response indicates a non-prepend historical fetch (e.g.,
guard the setHasMore call with a check that the fetch source is not "prepend" or
that the request was a historical non-prepend fetch in the functions that
process historical results such as the handler that processes
fetchHistoricalLogs / the historical branch in the main fetch logic), and apply
the same guard/update change to the other affected spots referenced around the
file (the blocks at ~lines 342-347 and 372-376).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6a6e4a0a-c2e3-4a6b-b2de-a333a4240e61
📒 Files selected for processing (3)
frontend/src/components/dashboard/bot-detail-panel.tsxfrontend/src/hooks/__tests__/use-bot-logs-stream.test.tsfrontend/src/hooks/use-bot-logs-stream.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/dashboard/bot-detail-panel.tsx
- frontend/src/hooks/tests/use-bot-logs-stream.test.ts
774a3cd to
f38226c
Compare
The stream hook (useBotLogsStream) hardcoded hasMore=false, so when a realtime bot switched to a date range interval its REST fetches never exposed pagination. This adds hasMore state tracking and a loadMore callback to the stream hook for its REST mode, and wires them through bot-detail-panel so the console shows "load more" for historical browsing on realtime bots. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
f38226c to
8f06172
Compare
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests