From fe2ea68cd860712cccfcc4071cfbb1e746de5377 Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 31 Oct 2026 22:49:45 +0000 Subject: [PATCH 1/2] fix(github): page review threads 25 at a time to cut GraphQL cost GitHub prices a GraphQL page by the rows it may return. ReviewThreads asked for 100 threads, each with 10 comments and their reactors, so every read cost 14 points even on a pull request with no threads. At 25 threads a page the read costs 6. The thread loop now allows 40 pages, so callers keep the same 1,000-thread ceiling; the activity and dismissal loops keep theirs. Co-Authored-By: Claude Opus 5.5 --- .../src/server/GitHubPullRequestApi.test.ts | 22 ++++++++++++------- .../src/server/GitHubPullRequestApi.ts | 5 ++++- .../src/server/pullRequestReviews.graphql | 9 +++++--- .../src/server/pullRequestReviews.graphql.ts | 2 +- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts index 5a5c46f28d99a..4bf4200afe503 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts @@ -3612,18 +3612,23 @@ layer("GitHubPullRequestApi.layer", (it) => { }), ); - it.effect("stops at the thread bound and says the conversation was cut short", () => + it.effect("stops at a thousand threads and says the conversation was cut short", () => Effect.gen(function* () { - mockedExecute.mockReturnValue( - Effect.succeed( + // Serves whatever page size the document asks for, so the bound holds in threads. + mockedExecute.mockImplementation((call) => { + const query = call.kind === "graphql" ? call.query : ""; + const first = Number(/reviewThreads\(first:(\d+)/.exec(query)?.[1]); + const start = Number(call.kind === "graphql" ? (call.variables?.["after"] ?? 0) : 0); + const ids = Array.from({ length: first }, (_, index) => `PRRT_${start + index + 1}`); + return Effect.succeed( output( reviewThreadsPage( - [{ ...thread("PRRT_1", "c1"), comments: threadComments(["c1"], "Y3Vyc29yOjI", 3) }], - "Y3Vyc29yOjE", + ids.map((id) => thread(id, `c_${id}`)), + String(start + first), ), ), - ), - ); + ); + }); const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; const conversation = yield* cli.listReviewThreadComments({ @@ -3633,7 +3638,8 @@ layer("GitHubPullRequestApi.layer", (it) => { number: 7, }); - assert.strictEqual(mockedExecute.mock.calls.length, 10); + assert.strictEqual(conversation.reviewThreads.length, 1_000); + assert.strictEqual(conversation.reviewThreads.at(-1)?.id, "PRRT_1000"); assert.isTrue(conversation.truncated); assert.isTrue(conversation.reviewThreadsTruncated); }), diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.ts index 2defbb6684919..57e1e8c8fe04b 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.ts @@ -358,6 +358,9 @@ export const NODE_ID_CACHE_CAPACITY = 128; const REVIEW_THREAD_PAGES = 10; +// `ReviewThreads` reads 25 threads a page; forty pages keep the 1,000-thread ceiling. +const REVIEW_THREAD_LIST_PAGES = 40; + const CHECK_CONTEXT_PAGES = 10; export interface GitHubPullRequestListBatch { @@ -2208,7 +2211,7 @@ export const make = Effect.gen(function* () { } cursor = read.nextCursor; page += 1; - } while (cursor !== null && page < REVIEW_THREAD_PAGES); + } while (cursor !== null && page < REVIEW_THREAD_LIST_PAGES); if (dismissalCursor !== null) { let after: string | null = dismissalCursor; diff --git a/packages/source-control-github/src/server/pullRequestReviews.graphql b/packages/source-control-github/src/server/pullRequestReviews.graphql index 96c35783dc1b6..fc9028f8118b9 100644 --- a/packages/source-control-github/src/server/pullRequestReviews.graphql +++ b/packages/source-control-github/src/server/pullRequestReviews.graphql @@ -65,7 +65,10 @@ query ReviewThreads( } repository(owner: $owner, name: $name) { pullRequest(number: $number) { - reviewThreads(first: 100, after: $after) { + # GitHub prices a page by the rows it may return, not the rows that exist: this read costs + # 14 points at a hundred threads a page even with no threads, and 6 at twenty-five. Only a + # pull request past fifty threads pays more, for its extra pages. + reviewThreads(first: 25, after: $after) { totalCount pageInfo { hasNextPage @@ -79,8 +82,8 @@ query ReviewThreads( # Null once the thread's line has left the diff, which `isOutdated` reports. line diffSide - # Only ten comments ride with each thread. A hundred threads times a hundred comments - # made GitHub reserve 10,000 nested rows and charge 104 points; unfinished threads are + # Only ten comments ride with each thread. A hundred, on a hundred-thread page, made + # GitHub reserve 10,000 nested rows and charge 104 points; unfinished threads are # paged from their own cursor by `ReviewThreadComments`. comments(first: 10) { totalCount diff --git a/packages/source-control-github/src/server/pullRequestReviews.graphql.ts b/packages/source-control-github/src/server/pullRequestReviews.graphql.ts index 0711ed9ebcfa5..0273836f09d68 100644 --- a/packages/source-control-github/src/server/pullRequestReviews.graphql.ts +++ b/packages/source-control-github/src/server/pullRequestReviews.graphql.ts @@ -413,7 +413,7 @@ export const ReviewThreads = GraphQL.query("ReviewThreads", { ], selections: [ { name: "viewer", body: ["{login}"] }, - { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){pullRequest(number:$", "number", "){reviewThreads(first:100 after:$", "after", "){totalCount pageInfo{hasNextPage endCursor}nodes{id isResolved isOutdated path line diffSide comments(first:10){totalCount pageInfo{hasNextPage endCursor}nodes{...ThreadComment}}}}viewerCanUpdate viewerDidAuthor author{__typename login avatarUrl}...Reactions comments(first:100){nodes{id lastEditedAt author{__typename login avatarUrl}...Reactions}}reviews(first:100){nodes{id lastEditedAt author{__typename login avatarUrl}...Reactions}}reviewRequests(first:$", "requestedReviewers", "){nodes{requestedReviewer{__typename...on User{login name avatarUrl}...on Bot{login avatarUrl}...on Team{combinedSlug teamName:name teamAvatarUrl:avatarUrl}}}}latestReviews(first:50){nodes{state author{__typename login avatarUrl}}}reviewDismissals:timelineItems(itemTypes:[REVIEW_DISMISSED_EVENT]first:100){pageInfo{hasNextPage endCursor}nodes{__typename...on ReviewDismissedEvent{dismissalMessage review{id}}}}commits(last:100){nodes{commit{oid messageHeadline committedDate authors(first:3){nodes{name avatarUrl user{login}}}}}}}}"] } + { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){pullRequest(number:$", "number", "){reviewThreads(first:25 after:$", "after", "){totalCount pageInfo{hasNextPage endCursor}nodes{id isResolved isOutdated path line diffSide comments(first:10){totalCount pageInfo{hasNextPage endCursor}nodes{...ThreadComment}}}}viewerCanUpdate viewerDidAuthor author{__typename login avatarUrl}...Reactions comments(first:100){nodes{id lastEditedAt author{__typename login avatarUrl}...Reactions}}reviews(first:100){nodes{id lastEditedAt author{__typename login avatarUrl}...Reactions}}reviewRequests(first:$", "requestedReviewers", "){nodes{requestedReviewer{__typename...on User{login name avatarUrl}...on Bot{login avatarUrl}...on Team{combinedSlug teamName:name teamAvatarUrl:avatarUrl}}}}latestReviews(first:50){nodes{state author{__typename login avatarUrl}}}reviewDismissals:timelineItems(itemTypes:[REVIEW_DISMISSED_EVENT]first:100){pageInfo{hasNextPage endCursor}nodes{__typename...on ReviewDismissedEvent{dismissalMessage review{id}}}}commits(last:100){nodes{commit{oid messageHeadline committedDate authors(first:3){nodes{name avatarUrl user{login}}}}}}}}"] } ], fragments: [ { name: "ThreadComment", body: ["on PullRequestReviewComment{id author{__typename login avatarUrl}body createdAt lastEditedAt url...Reactions}"] }, From 7542fe8f5aca405afc785b49ceed92a236886259 Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 31 Oct 2026 23:00:49 +0000 Subject: [PATCH 2/2] fix(github): keep the review-thread cost comment to the pricing rule Co-Authored-By: Claude Opus 5.5 --- .../src/server/pullRequestReviews.graphql | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/source-control-github/src/server/pullRequestReviews.graphql b/packages/source-control-github/src/server/pullRequestReviews.graphql index fc9028f8118b9..3b46ad39681e5 100644 --- a/packages/source-control-github/src/server/pullRequestReviews.graphql +++ b/packages/source-control-github/src/server/pullRequestReviews.graphql @@ -65,9 +65,8 @@ query ReviewThreads( } repository(owner: $owner, name: $name) { pullRequest(number: $number) { - # GitHub prices a page by the rows it may return, not the rows that exist: this read costs - # 14 points at a hundred threads a page even with no threads, and 6 at twenty-five. Only a - # pull request past fifty threads pays more, for its extra pages. + # GitHub prices a page by the rows it may return, not the rows that exist, so a page of + # twenty-five threads costs 6 points even on a pull request with none. reviewThreads(first: 25, after: $after) { totalCount pageInfo { @@ -82,8 +81,8 @@ query ReviewThreads( # Null once the thread's line has left the diff, which `isOutdated` reports. line diffSide - # Only ten comments ride with each thread. A hundred, on a hundred-thread page, made - # GitHub reserve 10,000 nested rows and charge 104 points; unfinished threads are + # Only ten comments ride with each thread. A hundred threads times a hundred comments + # made GitHub reserve 10,000 nested rows and charge 104 points; unfinished threads are # paged from their own cursor by `ReviewThreadComments`. comments(first: 10) { totalCount