Restore server-side message pagination on HTTP endpoint#21580
Conversation
…istory pagination (#21562) Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat: wire server-side pagination into handleListMessages HTTP endpoint Part of #21550 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: gate pagination fields behind isPaginated and regenerate openapi spec Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: remove trim-after-pagination and add 60s safety timeout for stuck loading state Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use minimal state clear instead of resetMessagePagination() on timeout Replace resetMessagePagination() in the 60s timeout handler with a targeted clear of isLoadingMoreMessages + loadMoreTimeoutTask. This preserves the user's scroll position, historyCursor, and hasMoreHistory so they can retry by scrolling up, rather than collapsing the visible window. Also update doc comments to reflect the 30s/60s two-stage timeout behavior and the accepted misclassification tradeoff. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Part of #21550. Adds 9 test cases covering backward compatibility, cursor pagination, strict exclusivity, hasMore logic, metadata correctness, empty conversations, and input validation. Co-authored-by: Vellum Assistant <assistant@vellum.ai> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53aa620718
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (beforeTimestamp !== undefined) { | ||
| conditions.push(lt(messages.createdAt, beforeTimestamp)); | ||
| } |
There was a problem hiding this comment.
Include message-id tie-breaker in pagination filter
The pagination query filters only on created_at < beforeTimestamp, so if multiple messages share the same millisecond as the page boundary, the next page permanently skips the remaining same-timestamp messages. This is a real data-loss-in-pagination case for legacy/imported conversations where duplicate millisecond timestamps exist, and the commit already exposes oldestMessageId but never uses it to disambiguate cursors. Please paginate with a stable (created_at, id) cursor (and request-side beforeMessageId) to avoid dropped history rows.
Useful? React with 👍 / 👎.
… after first unwrap)
Summary
Restores server-side message pagination that was lost during the WebSocket→HTTP transport migration (#16207). The daemon's
GET /v1/messagesendpoint now honorsbeforeTimestampfor cursor-based pagination, enabling the client to load older messages when scrolling up. Uses Option A: only paginate whenbeforeTimestampis present, so initial load and reconnect behavior is unchanged (zero regression risk).Changes
(conversation_id, created_at)andgetMessagesPaginated()/getLastAssistantTimestampBefore()query functionshandleListMessages— parsesbeforeTimestamp/limit, returnshasMore/oldestTimestamp/oldestMessageId, seeds interface diffs correctlytrimOldMessagesIfNeeded()from pagination branch (prevents infinite fetch-trim loop), added 60s timeout to clear stuck loading stateMilestone PRs (merged into feature branch)
Project issue
Closes #21550
Test plan
cd assistant && bun test src/__tests__/list-messages-attachments.test.ts— all 17 tests pass