Skip to content

fix(watch-pr): paginate review threads instead of reading only the first 100 - #484

Open
Yi-111-a wants to merge 1 commit into
cursor:mainfrom
Yi-111-a:fix/watch-pr-review-thread-pagination
Open

Yi-111-a wants to merge 1 commit into
cursor:mainfrom
Yi-111-a:fix/watch-pr-review-thread-pagination

Conversation

@Yi-111-a

@Yi-111-a Yi-111-a commented Oct 2, 2026 •

Copy link
Copy Markdown

Fixes #471

Problem

REVIEW_THREADS_QUERY asked for reviewThreads(first: 100) with no pageInfo and no cursor, and reviewThreads() issued that query exactly once.

On a PR with more than 100 review threads, every thread past the first page was invisible. Because readSnapshot builds 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:

  • add $after: String and pageInfo { hasNextPage endCursor } to reviewThreads(first: 100, after: $after)
  • have the reader return a single page (reviewThreadPage) instead of a finished list
  • loop in resolveReviewThreads until hasNextPage is false, mirroring resolveChecks

One detail worth reviewing

parseReviewThreads did two jobs: parse the nodes, and derive bugbotReviewPasses from every thread in the response. That count is global, so it is now split:

  • parseReviewThreadPage → one page of raw threads plus the cursor
  • buildReviewThreads → the existing filter and Bugbot pass count, run once over all accumulated pages

Accumulating raw pages before counting keeps the pass count identical to today. Computing it per page would have reported 1 per page on any PR with more than one Bugbot pass, so that ordering is load-bearing rather than cosmetic.

A page missing pageInfo is rejected as a missing-key query error rather than silently treated as a single page, consistent with how checkRollupPage handles contexts.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-pageInfo failure.

I verified these are not vacuous by reverting resolveReviewThreads to 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 test for the watcher suite: 35 pass, 0 fail. tsc --noEmit: clean. cli.test.ts still fails on a pre-existing missing commander dependency, 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 reviewThreads GraphQL query now supports $after and pageInfo, mirroring check-rollup pagination. GitHubReader exposes reviewThreadPage instead of a one-shot reviewThreads; resolveReviewThreads walks pages until the cursor ends, then buildReviewThreads applies the same unresolved filtering and global Bugbot pass count as before (raw pages are accumulated first so pass counts stay correct across pages). Responses without pageInfo fail as query errors rather than assuming a single page.

readSnapshot uses resolveReviewThreads for thread blockers. Tests cover multi-page walks, page-2 unresolved threads, cross-page Bugbot counts, and missing pageInfo.

Reviewed by Cursor Bugbot for commit 58c4e6f. Bugbot is set up for automated code reviews on this repo. Configure here.

…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
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.

watch-pr: review threads are fetched without pagination (first 100 only)

1 participant