Conversation
…rst 100 The reviewThreads query requested reviewThreads(first: 100) with no pageInfo and no cursor, and the reader issued it once. On a PR with more than 100 review threads every thread past the first page was invisible, so an unresolved thread beyond page 1 never blocked the readiness decision and the PR was reported ready with no unresolved threads. Paginate like the check-rollup query already does for contexts: add and pageInfo to the query, return one page from the reader, and loop in resolveReviewThreads until hasNextPage is false. Splitting the parser into parseReviewThreadPage and buildReviewThreads keeps the Bugbot pass count global: it is derived from every thread in the response, so accumulating raw pages first preserves the existing count instead of computing it per page. Fixes cursor#471
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 #471
Problem
REVIEW_THREADS_QUERYasked forreviewThreads(first: 100)with nopageInfoand no cursor, andreviewThreads()issued that query exactly once.On a PR with more than 100 review threads, every thread past the first page was invisible. Because
readSnapshotbuilds the PR snapshot from that list, an unresolved thread beyond the first page could not block the readiness decision — the watcher reported the PR as having no unresolved threads and could report it ready while a review comment was still open.Fix
Paginate the query the way the check-rollup query in the same file already does for
contexts:$after: StringandpageInfo { hasNextPage endCursor }toreviewThreads(first: 100, after: $after)reviewThreadPage) instead of a finished listresolveReviewThreadsuntilhasNextPageis false, mirroringresolveChecksOne detail worth reviewing
parseReviewThreadsdid two jobs: parse the nodes, and derivebugbotReviewPassesfrom every thread in the response. That count is global, so it is now split:parseReviewThreadPage→ one page of raw threads plus the cursorbuildReviewThreads→ the existing filter and Bugbot pass count, run once over all accumulated pagesAccumulating raw pages before counting keeps the pass count identical to today. Computing it per page would have reported
1per page on any PR with more than one Bugbot pass, so that ordering is load-bearing rather than cosmetic.A page missing
pageInfois rejected as amissing-keyquery error rather than silently treated as a single page, consistent with howcheckRollupPagehandlescontexts.pageInfo.Tests
Six new cases in
github.test.ts, covering: walking to the last page, seeing an unresolved thread that sits only on page 2, stopping after one page when the cursor is exhausted, Bugbot passes counted across pages, resolved threads dropped from later pages, and the missing-pageInfofailure.I verified these are not vacuous by reverting
resolveReviewThreadsto the old single-shot read and re-running: the three page-2 and multi-page cases fail, and pass again with the loop restored.bun testfor the watcher suite: 35 pass, 0 fail.tsc --noEmit: clean.cli.test.tsstill fails on a pre-existing missingcommanderdependency, unrelated to this change and failing the same way on the base commit.Note
Comments are still fetched with
comments(first: 10)without pagination. Only thread-level resolution is used downstream, so I left that alone rather than widen the change — but it is the same latent truncation if a thread's resolution state ever moves into those comments.Note
Medium Risk
Changes how merge-blocking review threads are discovered for readiness decisions; incorrect pagination would have caused false-ready results, though the new behavior aligns with existing check-rollup patterns and is covered by new tests.
Overview
Fixes review-thread fetching so PRs with more than 100 threads are evaluated correctly. Previously the watcher only loaded the first GraphQL page, so unresolved threads beyond that could be ignored and readiness could be wrong.
The
reviewThreadsGraphQL query now supports$afterandpageInfo, mirroring check-rollup pagination.GitHubReaderexposesreviewThreadPageinstead of a one-shotreviewThreads;resolveReviewThreadswalks pages until the cursor ends, thenbuildReviewThreadsapplies the same unresolved filtering and global Bugbot pass count as before (raw pages are accumulated first so pass counts stay correct across pages). Responses withoutpageInfofail as query errors rather than assuming a single page.readSnapshotusesresolveReviewThreadsfor thread blockers. Tests cover multi-page walks, page-2 unresolved threads, cross-page Bugbot counts, and missingpageInfo.Reviewed by Cursor Bugbot for commit 58c4e6f. Bugbot is set up for automated code reviews on this repo. Configure here.