Skip to content

fix: omit pagination token when all shard cursors are null#720

Open
PirosB3 wants to merge 4 commits into
mainfrom
dan/address-pagination-bug
Open

fix: omit pagination token when all shard cursors are null#720
PirosB3 wants to merge 4 commits into
mainfrom
dan/address-pagination-bug

Conversation

@PirosB3
Copy link
Copy Markdown
Contributor

@PirosB3 PirosB3 commented Nov 11, 2025

Fixes a pagination bug that occurs when pageToken is null across all shards. In that scenario the implementation serialized [null, null, ...] and returned it as the next page token, instead of omitting the cursor. Affects all multi-shard paginated endpoints: get_events, get_casts_by_parent, get_casts_by_mention, get_reactions_by_cast, get_reactions_by_target, get_links_by_target.

Result: clients receive duplicate results and keep paginating in a loop.

The HTTP layer had related issues that are also fixed here:

  • EventsResponse was missing the nextPageToken field entirely, so HTTP clients couldn't paginate events at all.
  • map_proto_messages_response_to_json_paged_response coerced None into Some("") instead of omitting the field.

Tests: adds test_get_casts_by_parent_returns_none_token_when_exhausted, which drains a query across two shards and asserts next_page_token is None once exhausted. Fails on main, passes on this branch.

@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 11, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
snapchain-docs Ready Ready Preview, Comment Apr 28, 2026 11:03pm

Request Review

Comment thread src/network/http_server.rs
@PirosB3 PirosB3 requested a review from aditiharini November 11, 2025 17:14
@manan19 manan19 removed the request for review from aditiharini April 28, 2026 22:25
Asserts next_page_token is None when all shards have exhausted
pagination, locking in the fix for the [null,null,...] cursor bug.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 28, 2026

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • src/network/http_server.rs (89.7%): Missing lines 2967-2969
  • src/network/server.rs (68.4%): Missing lines 1876,1880,1952,1956,2459,2463

Summary

  • Total: 48 lines
  • Missing: 9 lines
  • Coverage: 81%

src/network/http_server.rs

  2963                 .events
  2964                 .iter()
  2965                 .map(|e| map_proto_hub_event_to_json_hub_event(e.clone()).unwrap())
  2966                 .collect(),
! 2967             next_page_token: events_response
! 2968                 .next_page_token
! 2969                 .map(|t| BASE64_STANDARD.encode(t)),
  2970         })
  2971     }
  2972 
  2973     async fn get_event_by_id(&self, req: EventRequest) -> Result<HubEvent, ErrorResponse> {

src/network/server.rs

  1872             .collect();
  1873 
  1874         let next_page_tokens: Vec<Option<Vec<u8>>> =
  1875             pages.into_iter().map(|page| page.next_page_token).collect();
! 1876         let next_page_token = serialize_page_token_if_any_shard_has_next(next_page_tokens)?;
  1877 
  1878         let response = MessagesResponse {
  1879             messages: combined_messages,
! 1880             next_page_token,
  1881         };
  1882 
  1883         Ok(Response::new(response))
  1884     }

  1948             .collect();
  1949 
  1950         let next_page_tokens: Vec<Option<Vec<u8>>> =
  1951             pages.into_iter().map(|page| page.next_page_token).collect();
! 1952         let next_page_token = serialize_page_token_if_any_shard_has_next(next_page_tokens)?;
  1953 
  1954         let response = MessagesResponse {
  1955             messages: combined_messages,
! 1956             next_page_token,
  1957         };
  1958 
  1959         Ok(Response::new(response))
  1960     }

  2455             .collect();
  2456 
  2457         let next_page_tokens: Vec<Option<Vec<u8>>> =
  2458             pages.into_iter().map(|page| page.next_page_token).collect();
! 2459         let next_page_token = serialize_page_token_if_any_shard_has_next(next_page_tokens)?;
  2460 
  2461         let response = MessagesResponse {
  2462             messages: combined_messages,
! 2463             next_page_token,
  2464         };
  2465 
  2466         Ok(Response::new(response))
  2467     }

@manan19 manan19 requested a review from Copilot April 28, 2026 22:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a cross-shard pagination bug where an “exhausted” cursor was still being returned as a serialized [null, null, ...] token, causing clients to paginate indefinitely and receive duplicate results.

Changes:

  • Add serialize_page_token_if_not_empty helper to omit next_page_token when all shards are exhausted (all per-shard tokens are None).
  • Apply the helper across several multi-shard paginated RPC responses.
  • Update HTTP JSON pagination token behavior to omit nextPageToken when absent, and add nextPageToken support for events; add a regression test for get_casts_by_parent.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/network/server.rs Introduces and uses a helper to avoid emitting a pagination token when all shards are exhausted.
src/network/server_tests.rs Adds a regression test ensuring get_casts_by_parent returns None next_page_token when drained.
src/network/http_server.rs Ensures nextPageToken is omitted (not "") when absent; adds events nextPageToken field support.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/network/server.rs
Comment thread src/network/server.rs
Comment thread src/network/server_tests.rs
Comment thread src/network/http_server.rs
@manan19 manan19 changed the title Bug: broken pagination token when cursor is null on all shards fix: omit pagination token when all shard cursors are null Apr 28, 2026
- Rename `serialize_page_token_if_not_empty` to
  `serialize_page_token_if_any_shard_has_next` to describe the actual
  predicate (per Copilot review).
- Add `test_get_events_returns_none_token_when_exhausted` to cover a
  second call site of the helper.
- Add unit tests in `http_server` verifying that
  `map_proto_messages_response_to_json_paged_response` omits
  `nextPageToken` when the proto field is `None` and base64-encodes it
  when `Some(_)`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants