fix(backend/copilot): fix initial load missing messages + forward pagination for completed sessions - #12796
Conversation
…ination for completed sessions Completed copilot sessions with many messages were showing an empty view because the backend returned only the newest 50 (all tool calls, no user messages) and the frontend silently dropped messages with empty content. Backend changes: - get_chat_messages_paginated: add from_start (ASC) and after_sequence (forward cursor) modes alongside the existing before_sequence (DESC) backward mode - PaginatedMessages: expose newest_sequence for forward-pagination cursors - routes.py: detect completed sessions on initial load (no active stream) and use from_start=True; expose newest_sequence + forward_paginated in SessionDetailResponse; accept after_sequence query param - openapi.json: add after_sequence param + newest_sequence / forward_paginated fields to SessionDetailResponse schema Frontend changes: - convertChatSessionToUiMessages: never drop user messages with empty content - useLoadMoreMessages: support forward pagination via after_sequence cursor; append pages to end rather than prepending for completed sessions - ChatMessagesContainer: move LoadMoreSentinel to bottom for forward pagination - useChatSession / useCopilotPage / ChatContainer: wire up newestSequence and forwardPaginated props end-to-end Tests: add 9 new unit tests for from_start and after_sequence pagination modes
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
|
/review |
github-actions
Bot
added
platform/frontend
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
🟡 Medium Risk — Some Line OverlapThese PRs have some overlapping changes:
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 3 conflict(s), 2 medium risk, 5 low risk (out of 10 PRs with file overlap) Auto-generated on push. Ignores: |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #12796 +/- ##
==========================================
+ Coverage 65.14% 65.41% +0.27%
==========================================
Files 1831 1850 +19
Lines 135945 137462 +1517
Branches 14534 14735 +201
==========================================
+ Hits 88555 89923 +1368
- Misses 44670 44747 +77
- Partials 2720 2792 +72
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds forward cursor pagination: backend accepts Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as Chat UI
participant Hook as useCopilotPage
participant Session as useChatSession
participant API as Backend API
participant DB as Database
User->>UI: Open session / resume
UI->>Hook: initial render
Hook->>Session: request session (no cursors)
Session->>API: GET /api/chat/sessions/{id}?before_sequence=None&after_sequence=None
API->>DB: lookup active stream / session state
DB-->>API: active vs completed
alt Active
API->>DB: fetch messages (DESC newest-first)
DB-->>API: messages + oldest_sequence
API-->>Session: SessionDetailResponse(forward_paginated=false, oldest_sequence=...)
else Completed
API->>DB: fetch messages (ASC from start / from_start)
DB-->>API: messages + newest_sequence
API-->>Session: SessionDetailResponse(forward_paginated=true, newest_sequence=...)
end
Session-->>Hook: messages + forwardPaginated
Hook-->>UI: render messages
User->>UI: scroll -> load more
UI->>Hook: onLoadMore()
alt forward_paginated=true
Hook->>Session: request after_sequence=X
Session->>API: GET /api/chat/sessions/{id}?after_sequence=X
API->>DB: fetch messages ASC where sequence > X
DB-->>API: newer messages
API-->>Session: paged messages (append)
else
Hook->>Session: request before_sequence=Y
Session->>API: GET /api/chat/sessions/{id}?before_sequence=Y
API->>DB: fetch messages DESC where sequence < Y
DB-->>API: older messages
API-->>Session: paged messages (prepend)
end
Session-->>Hook: updated pagedMessages
Hook-->>UI: rerender with merged messages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 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 |
…ompact openapi.json
- routes.py: replace f-string log with %s lazy formatting
- routes.py: compute forward_paginated once and reuse across both return paths
- openapi.json: revert reformatting noise, keep only the 3 new field additions
(newest_sequence, forward_paginated on SessionDetailResponse;
after_sequence query param on GET /sessions/{id})
|
|
… load to avoid stale closure
…o avoid React-18 batching bug The functional updater passed to setPagedRawMessages is not called synchronously in React 18 (automatic batching) — mutations inside it are invisible until the next render. Using `mergedLength = 0` and reading it after `setPagedRawMessages(fn)` always saw 0, so hasMore was never forced false for backward pagination at the MAX_OLDER_MESSAGES cap. Replace with `estimatedTotal = pagedRawMessages.length + newRaw.length` computed before the state update using the stale-but-correct closure value. Worst case: one extra page loaded before the cap triggers.
|
Round 5 fix — React 18 batching bug in The Root cause: ```ts Fix: Compute the estimate before the state update using the closure-captured ```ts Worst-case: the closure value is one render stale (one extra load allowed before cap). This is acceptable — the cap is a memory safety guard, not a hard cutoff. The All 20 frontend unit tests + 30 backend tests pass. Commit: |
…pletes When a session transitions from active (forwardPaginated=false) to complete (forwardPaginated=true), backward-paginated older messages would be appended after currentMessages instead of before, causing chronological disorder. Add a useEffect that calls resetPaged() on the false→true transition.
562a546 to
507055f
Compare
… and error paths - useCopilotPage: test message ordering (forward/backward) and resetPaged on forwardPaginated false→true transition - useLoadMoreMessages: cover setHasMore=false after 3 non-200 responses and epoch guard in catch block (stale error discard)
…l items are re-fetched When forward pagination causes merged.length > MAX_OLDER_MESSAGES, the tail items are dropped but the cursor (newestSequence) was still advanced to the server's newest_sequence. If the server also reported has_more_messages=false, those discarded items became permanently inaccessible. Fix: when truncation occurs, compute the last KEPT item's sequence from the raw response and set newestSequence to that value, then force hasMore=true so the sentinel re-fetches the discarded items on the next load-more. The fallback when pagedRawMessages is already at MAX_OLDER_MESSAGES (lastKeptIdx<0) sets hasMore=false to prevent an infinite re-fetch loop at the display cap.
github-project-automation
Bot
moved this from 🚧 Needs work
to 👍🏼 Mergeable
in AutoGPT development kanban
github-project-automation
Bot
moved this from 👍🏼 Mergeable
to ✅ Done
in AutoGPT development kanban
…antee for blank chat PR #12796 changed completed sessions to load from sequence 0 forward, which broke the standard chat UX (users land at the beginning instead of the end). This reverts the forward pagination approach and instead adds a visibility guarantee: after fetching the newest messages, if the entire page is tool messages (invisible in UI), expand backward until at least one user/assistant message is included so the chat never appears blank. Backend: - Remove after_sequence, from_start, forward_paginated, newest_sequence - Always use backward (newest-first) pagination for all sessions - Add _expand_for_visibility helper that scans up to 200 messages back - Extract _expand_tool_boundary helper from inline code Frontend: - Remove forwardPaginated/newestSequence plumbing from all hooks - Remove bottom LoadMoreSentinel and forward pagination props - Simplify useLoadMoreMessages to backward-only - Always prepend paged messages before current messages
…antee for blank chat (#12831) ## Why / What / How **Why:** PR #12796 changed completed copilot sessions to load messages from sequence 0 forward (ascending), which broke the standard chat UX — users now land at the beginning of the conversation instead of the most recent messages. Reported in Discord. **What:** Reverts the forward pagination approach and replaces it with a visibility guarantee that ensures every page contains at least one user/assistant message. **How:** - **Backend**: Removed after_sequence, from_start, forward_paginated, newest_sequence — always use backward (newest-first) pagination. Added _expand_for_visibility() helper: after fetching, if the entire page is tool messages (invisible in UI), expand backward up to 200 messages until a visible user/assistant message is found. - **Frontend**: Removed all forwardPaginated/newestSequence plumbing from hooks and components. Removed bottom LoadMoreSentinel. Simplified message merge to always prepend paged messages. ### Changes - routes.py: Reverted to simple backward pagination, removed TOCTOU re-fetch logic - db.py: Removed forward mode, extracted _expand_tool_boundary() and added _expand_for_visibility() - SessionDetailResponse: Removed newest_sequence and forward_paginated fields - openapi.json: Removed after_sequence param and forward pagination response fields - Frontend hooks/components: Removed forward pagination props and logic (-1000 lines) - Updated all tests (backend: 63 pass, frontend: 1517 pass) ### Checklist - [x] I have clearly listed my changes in the PR description - [x] Backend unit tests: 63 pass - [x] Frontend unit tests: 1517 pass - [x] Frontend lint + types: clean - [x] Backend format + pyright: clean
4 participants
Why / What / How
Why: Completed copilot sessions with many messages showed a completely empty chat view. A user reported a 158-message session that appeared blank on reload.
What: Two bugs fixed:
How: For completed sessions (no active stream), the backend now loads from sequence 0 in ASC order. Active/streaming sessions keep newest-first for streaming context. A new after_sequence forward cursor enables infinite-scroll for subsequent pages (sentinel moves to bottom). The frontend wires forward_paginated + newest_sequence end-to-end.
Changes 🏗️
Checklist 📋
For code changes: