diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts index f46c91fd69570..451a623a40806 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts @@ -3786,18 +3786,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({ @@ -3807,7 +3812,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 ce38cc726a249..de90298bd9fbb 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.ts @@ -366,6 +366,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 { @@ -2231,7 +2234,7 @@ export const make = Effect.gen(function* () { let cursor = first.nextCursor; let page = 1; // Later pages carry the threads alone; the PR-wide fields above come from the first. - while (cursor !== null && page < REVIEW_THREAD_PAGES) { + while (cursor !== null && page < REVIEW_THREAD_LIST_PAGES) { const later: Pick< GitHubReviewThreadPage, "threads" | "nextCursor" | "avatarsByLogin" | "botLogins" diff --git a/packages/source-control-github/src/server/pullRequestReviews.graphql b/packages/source-control-github/src/server/pullRequestReviews.graphql index 94646874d803d..a3c8d4dc7663b 100644 --- a/packages/source-control-github/src/server/pullRequestReviews.graphql +++ b/packages/source-control-github/src/server/pullRequestReviews.graphql @@ -89,7 +89,9 @@ query ReviewThreads($owner: String!, $name: String!, $number: Int!, $requestedRe } repository(owner: $owner, name: $name) { pullRequest(number: $number) { - reviewThreads(first: 100) { + # 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) { ...ReviewThreadPage } viewerCanUpdate @@ -204,7 +206,7 @@ query ReviewThreadPages($owner: String!, $name: String!, $number: Int!, $after: } repository(owner: $owner, name: $name) { pullRequest(number: $number) { - reviewThreads(first: 100, after: $after) { + reviewThreads(first: 25, after: $after) { ...ReviewThreadPage } } diff --git a/packages/source-control-github/src/server/pullRequestReviews.graphql.ts b/packages/source-control-github/src/server/pullRequestReviews.graphql.ts index 9a1351f8cd138..f39c0d39a3cc5 100644 --- a/packages/source-control-github/src/server/pullRequestReviews.graphql.ts +++ b/packages/source-control-github/src/server/pullRequestReviews.graphql.ts @@ -416,7 +416,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){...ReviewThreadPage}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){...ReviewThreadPage}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: "ReviewThreadPage", body: ["on PullRequestReviewThreadConnection{totalCount pageInfo{hasNextPage endCursor}nodes{id isResolved isOutdated path line diffSide comments(first:10){totalCount pageInfo{hasNextPage endCursor}nodes{...ThreadComment}}}}"] }, @@ -467,7 +467,7 @@ export const ReviewThreadPages = GraphQL.query("ReviewThreadPages", { ], selections: [ { name: "viewer", body: ["{login}"] }, - { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){pullRequest(number:$", "number", "){reviewThreads(first:100 after:$", "after", "){...ReviewThreadPage}}}"] } + { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){pullRequest(number:$", "number", "){reviewThreads(first:25 after:$", "after", "){...ReviewThreadPage}}}"] } ], fragments: [ { name: "ReviewThreadPage", body: ["on PullRequestReviewThreadConnection{totalCount pageInfo{hasNextPage endCursor}nodes{id isResolved isOutdated path line diffSide comments(first:10){totalCount pageInfo{hasNextPage endCursor}nodes{...ThreadComment}}}}"] },