Skip to content

Conversation

@0x2b3bfa0
Copy link
Contributor

It looks like page-based pagination is broken when there are 100+ pages to fetch.

2025-11-16T23:49:31.398: Requesting https://api.github.com/repos/owner/repository/issues?per_page=100&page=100&filter=all&state=closed
{
  "message": "Pagination with the page parameter is not supported for large datasets, please use cursor based pagination (after/before)",
  "documentation_url": "https://docs.github.com/rest/issues/issues#list-repository-issues",
  "status": "422"
}
2025-11-16T23:49:31.730: API request returned HTTP 422: Unprocessable Entity

This pull request has been proven to work, but was entirely vibe-coded with Claude Code 4.5; sorry, I didn't have time to write it properly. Happy to see it closed and re-implemented. 🙈

@Iamrodos
Copy link
Contributor

This is a good change.

The thing I like is that some GitHub APIs use page-based pagination (such as pulls and comments), and others now use cursor-based pagination (such as issues). The code is compatible with both and acts on whichever is returned by parsing the Link header. It doesn't need to know which pagination type an endpoint uses - it simply extracts the next URL from the Link header. The old code ignored the Link header and manually incremented page numbers, which caused 422 errors after ~100 pages on endpoints that require cursor-based pagination. If GitHub migrates more endpoints to supporting cursor-based pagination, they will just start to work.

Given there are now some tests for this package, it would be good to add some for this change. I will look at that and put in another comment.

@Iamrodos
Copy link
Contributor

@0x2b3bfa0 here is a test file that exercises the pagination logic and verifies both cursor-based and page-based Link headers are handled correctly. Tried to keep it clear. Claude did the draft of this which I then crafted from there.

If you think this is of value to enhance your PR, would you consider either:

  1. Adding me as a collaborator on your fork so I can push a commit, or
  2. Including this with a co-author attribution in your commit?

Keen to see more tests on key code in this package.

test_pagination.py

@Iamrodos Iamrodos mentioned this pull request Nov 17, 2025
@josegonzalez
Copy link
Owner

@Iamrodos I'll merge this and then you can make a PR to the repo with your tests?

@josegonzalez josegonzalez merged commit 6dfba7a into josegonzalez:master Nov 17, 2025
10 checks passed
@josegonzalez
Copy link
Owner

Will wait to release until the tests land.

@Iamrodos
Copy link
Contributor

@Iamrodos I'll merge this and then you can make a PR to the repo with your tests?

Sounds good.

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