github-tools: paginate PR diff and review-comment fetches - #437
Merged
TheGreatAxios merged 5 commits intoAug 29, 2026
Merged
TheGreatAxios merged 5 commits into
TheGreatAxios merged 5 commits into
Conversation
Cover the >100-item case github-tools currently drops: a fake fetch serving two pages (merged via Link: rel="next" and via a page= fallback), a single short page (unchanged), and a page-bound case that must report truncated: true. code-review's fixtures gain the new truncated/comments shape. Fixes CL-7140.
fetchPullRequestDiff and fetchPullRequestReviewComments issued one request at per_page=100 and stopped, silently dropping files or comments past the first page — code-review's dedupe then re-posted findings it had already made. Both now follow the Link: rel="next" header (falling back to page= when GitHub omits it), capped at 30 pages, and report truncated: true if the cap is hit. Fixes CL-7140.
The 0.0.7 pin bump missed this test's hardcoded expectation.
Cover both truncation sources (diff.truncated and the posted-comments page) at the aggregation layer and end to end through runPullRequestReview, plus the untruncated case posting no note.
A truncated diff or already-posted-comments page was silently reviewed as if it were complete, which could re-flag a finding whose earlier post fell outside the read page. aggregateReview now posts one plain note when either source is truncated. Also: renamed MAX_FILES_PER_PAGE to MAX_ITEMS_PER_PAGE (it bounds both the files and comments endpoints), and fetchAllPages now shares requestJSON's error formatting via a common fetchJSON helper instead of duplicating it. Bumped @corbits/github-tools to 0.0.8 for the src change and updated its pin literals.
TheGreatAxios
force-pushed
the
cl-7140-github-tools-pr-diff-and-review-comment-fetches-stop-at-page
branch
from
August 29, 2026 04:46
4a46ad1 to
0f9b41e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes CL-7140 — https://linear.app/abklabs/issue/CL-7140
Problem
fetchPullRequestDiff(packages/github-tools/src/pull-requests.ts) andfetchPullRequestReviewCommentssetper_page=100and issued a singlerequest, with no follow of the
Link: rel="next"header and nopage=loop. A pull request with more than 100 changed files silently lost
files 101+; more than 100 review comments silently lost the rest, and
packages/code-review's dedupe then re-posted findings it had alreadymade because it never saw the earlier comments.
Change
fetchAllPages, a paginating GET helper that follows theLink: rel="next"header when GitHub sends one, and falls back toincrementing
page=when it doesn't (some servers/tests omit it).Capped at 30 pages; hitting the cap sets
truncated: true.PullRequestDiffgains atruncated: booleanfield.fetchPullRequestReviewCommentsnow returns{ comments, truncated }instead of a bare array.(
PullRequestFilesResponse,ReviewCommentsResponse), now validatedagainst the merged page rather than a single page.
packages/code-review'saggregateReviewnow posts one plain notein the review body ("This review may be incomplete: ...") whenever
either the diff or the already-posted-comments page was truncated —
a truncated fetch was previously reviewed silently as if it were
complete, which risked re-flagging a finding whose earlier post fell
outside the read page. Threaded through
CodeReviewGitHub.listPostedCommentsandrunPullRequestReview.MAX_FILES_PER_PAGEtoMAX_ITEMS_PER_PAGE(it bounds boththe files and comments endpoints) and had
fetchAllPagesreuserequestJSON's error formatting via a sharedfetchJSONhelperinstead of duplicating it.
@corbits/github-toolsto 0.0.8 (two src changes) andupdated the
{ name, version }pin literals that reference it(
workflows/code-review,workflows/last-30-days-research, andthat workflow's test fixture).
Tests
packages/github-tools/src/pull-requests.test.ts: a fake fetch servingtwo pages (merged via
Link: rel="next"and via apage=fallback), asingle short page (unchanged behavior), and a page-bound case that
reports
truncated: true— for bothfetchPullRequestDiff's file listand
fetchPullRequestReviewComments.packages/code-review/src/aggregate.test.tsandreview-run.test.ts:red/green coverage that a truncated diff and a truncated comments page
each surface the incompleteness note (at the aggregation layer and
end-to-end through
runPullRequestReview), and that an untruncatedrun posts no note.
Local gate note: the machine was under heavy load, so the full
monorepo-wide
bun run checkcould not be run reliably (it wasgetting OOM-killed partway through, on packages this change never
touches). Ran targeted gates instead —
tsc --noEmitandbun testfor
packages/github-toolsandpackages/code-review,check:tool-package-pins,check:tool-package-freshness— all green.CI's
bun run checkis the authoritative gate for this PR, and it isgreen on every job.