From 23da5a27bd986c3ecbab856b3dc8a4465bb4953a Mon Sep 17 00:00:00 2001 From: Kalven Schraut Date: Sat, 5 Sep 2026 00:17:18 -0500 Subject: [PATCH 1/2] fix(pull-requests): preserve native Gitea review pagination --- .../pullRequest/GiteaPullRequestApi.test.ts | 142 +++++++++++++++++- .../src/pullRequest/GiteaPullRequestApi.ts | 25 ++- .../GiteaPullRequestProvider.activity.test.ts | 6 +- 3 files changed, 163 insertions(+), 10 deletions(-) diff --git a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts index 6d5bdc394c27..f2c10b36c44f 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts @@ -942,7 +942,147 @@ layer("GiteaPullRequestApi", (it) => { ], }), ]); - expect(callAt(1).path).toBe("/repos/acme/web/pulls/7/reviews/21/comments"); + expect(callAt(1).path).toBe("/repos/acme/web/pulls/7/reviews/21/comments?page=1&limit=50"); + }), + ); + + it.effect( + "marks review activity truncated when nested review comments exceed the conversation bound", + () => + Effect.gen(function* () { + mockedRequest + .mockReturnValueOnce( + Effect.succeed( + response([ + { + id: 21, + body: "Review", + state: "COMMENT", + submitted_at: "2026-09-03T11:00:00Z", + }, + ]), + ), + ) + .mockReturnValueOnce( + Effect.succeed( + response( + [ + { + id: 31, + body: "First", + path: "src/a.ts", + position: 1, + created_at: "2026-09-03T11:01:00Z", + }, + ], + { "x-total-count": "501" }, + ), + ), + ) + .mockReturnValueOnce(Effect.succeed(response([], { "x-total-count": "501" }))) + .mockReturnValueOnce(Effect.succeed(response([], { "x-total-count": "501" }))) + .mockReturnValueOnce(Effect.succeed(response([], { "x-total-count": "501" }))) + .mockReturnValueOnce(Effect.succeed(response([], { "x-total-count": "501" }))); + const api = yield* GiteaPullRequestApi.make; + const result = yield* api.listReviews({ + host: "forge.example.test", + repository: "acme/web", + number: 7, + }); + + assert.isTrue(result.truncated); + expect(result.comments).toContainEqual( + expect.objectContaining({ id: "review-comment:31" }), + ); + expect(callAt(5).path).toContain("page=5"); + }), + ); + + it.effect("does not repeat an unpaginated native review-comment response at the page size", () => + Effect.gen(function* () { + mockedRequest + .mockReturnValueOnce( + Effect.succeed( + response([ + { + id: 21, + body: "Review", + state: "COMMENT", + submitted_at: "2026-09-03T11:00:00Z", + }, + ]), + ), + ) + .mockReturnValueOnce( + Effect.succeed( + response( + Array.from({ length: 51 }, (_, index) => ({ + id: index + 31, + body: `Comment ${index + 1}`, + path: "src/a.ts", + position: index + 1, + created_at: "2026-09-03T11:01:00Z", + })), + ), + ), + ); + const api = yield* GiteaPullRequestApi.make; + const result = yield* api.listReviews({ + host: "forge.example.test", + repository: "acme/web", + number: 7, + }); + + assert.isFalse(result.truncated); + assert.strictEqual( + result.comments.filter((comment) => comment.kind === "review-comment").length, + 51, + ); + assert.strictEqual(mockedRequest.mock.calls.length, 2); + }), + ); + + it.effect("does not mark an exact unpaginated review-comment safety bound as truncated", () => + Effect.gen(function* () { + mockedRequest + .mockReturnValueOnce( + Effect.succeed( + response([ + { + id: 21, + body: "Review", + state: "COMMENT", + submitted_at: "2026-09-03T11:00:00Z", + }, + ]), + ), + ) + .mockReturnValueOnce( + Effect.succeed( + response( + Array.from({ length: 200 }, (_, index) => ({ + id: index + 31, + body: `Comment ${index + 1}`, + path: "src/a.ts", + position: index + 1, + created_at: "2026-09-03T11:01:00Z", + })), + ), + ), + ); + const api = yield* GiteaPullRequestApi.make; + const result = yield* api.listReviews({ + host: "forge.example.test", + repository: "acme/web", + number: 7, + }); + + assert.isFalse(result.truncated); + assert.strictEqual( + result.comments.filter((comment) => comment.kind === "review-comment").length, + 200, + ); + assert.strictEqual(mockedRequest.mock.calls.length, 2); }), ); diff --git a/apps/server/src/pullRequest/GiteaPullRequestApi.ts b/apps/server/src/pullRequest/GiteaPullRequestApi.ts index b000063652bc..364ad6cefcf5 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestApi.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestApi.ts @@ -957,6 +957,7 @@ export const make = Effect.gen(function* () { repository: string; path: string; limit: number; + requirePaginationEvidence?: boolean; }) { const rows: Array = []; let path = input.path; @@ -978,14 +979,18 @@ export const make = Effect.gen(function* () { rowsSeen, headers: result.headers, }); + const hasPaginationEvidence = + nextLink(result.headers) !== null || totalCount(result.headers) !== null; + const paginationNext = + input.requirePaginationEvidence && !hasPaginationEvidence ? null : next; if (result.rows.length > remaining || rows.length >= input.limit) { return { rows, - truncated: result.rows.length > remaining || next !== null, + truncated: result.rows.length > remaining || paginationNext !== null, }; } - if (next === null) return { rows, truncated: false }; - path = next; + if (paginationNext === null) return { rows, truncated: false }; + path = paginationNext; } return { rows, truncated: true }; }); @@ -1183,7 +1188,7 @@ export const make = Effect.gen(function* () { } const comments: Array = []; const threads: Array = []; - const commentsTruncated = reviewsTruncated; + let commentsTruncated = reviewsTruncated; for (const row of reviewRows) { const review = decodeReview(row); if (Option.isNone(review)) continue; @@ -1200,11 +1205,17 @@ export const make = Effect.gen(function* () { reviewState: review.value.state?.toLowerCase().replaceAll("_", " ") ?? null, }); } - const codeRows = yield* readUnknownArray({ + const codeRows = yield* readUnknownSlice({ operation: "listReviewComments", ...input, - path: `${basePath(input.repository)}/pulls/${input.number}/reviews/${review.value.id}/comments`, + path: query( + `${basePath(input.repository)}/pulls/${input.number}/reviews/${review.value.id}/comments`, + { page: 1, limit: PAGE_SIZE }, + ), + limit: PAGE_SIZE * CONVERSATION_PAGES, + requirePaginationEvidence: true, }); + commentsTruncated ||= codeRows.truncated; const grouped = new Map< string, Array<{ @@ -1216,7 +1227,7 @@ export const make = Effect.gen(function* () { readonly comment: PullRequestReviewThread["comments"][number]; }> >(); - for (const codeRow of codeRows) { + for (const codeRow of codeRows.rows) { const decoded = decodeReviewComment(codeRow); if (Option.isNone(decoded)) continue; const mapped = decoded.value; diff --git a/apps/server/src/pullRequest/GiteaPullRequestProvider.activity.test.ts b/apps/server/src/pullRequest/GiteaPullRequestProvider.activity.test.ts index 333def121618..b9b7d70dc1c5 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestProvider.activity.test.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestProvider.activity.test.ts @@ -15,7 +15,9 @@ const response = (value: unknown) => ({ headers: {}, }); const failure = () => - Effect.fail(new GiteaApi.GiteaApiError({ operation: "test", reason: "failed", detail: "offline" })); + Effect.fail( + new GiteaApi.GiteaApiError({ operation: "test", reason: "failed", detail: "offline" }), + ); const pull = { number: 7, @@ -58,7 +60,7 @@ function route(viewerFails: boolean, reactionsFail: boolean) { return Effect.succeed( response([{ id: 2, body: "summary", submitted_at: "2026-01-01T00:00:00Z" }]), ); - if (input.path === "/repos/acme/web/pulls/7/reviews/2/comments") + if (input.path === "/repos/acme/web/pulls/7/reviews/2/comments?page=1&limit=50") return Effect.succeed( response([ { id: 3, body: "inline", created_at: "2026-01-01T00:00:00Z", path: "a.ts", position: 1 }, From a09a525142247cd748df67026c1b374ba2d2888f Mon Sep 17 00:00:00 2001 From: Kalven Schraut Date: Sat, 5 Sep 2026 00:21:46 -0500 Subject: [PATCH 2/2] test(pull-requests): fill bounded Gitea comment pages --- .../pullRequest/GiteaPullRequestApi.test.ts | 51 ++++++++----------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts index f2c10b36c44f..2b89fa03e514 100644 --- a/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts +++ b/apps/server/src/pullRequest/GiteaPullRequestApi.test.ts @@ -950,51 +950,42 @@ layer("GiteaPullRequestApi", (it) => { "marks review activity truncated when nested review comments exceed the conversation bound", () => Effect.gen(function* () { - mockedRequest - .mockReturnValueOnce( - Effect.succeed( - response([ - { - id: 21, - body: "Review", - state: "COMMENT", - submitted_at: "2026-09-03T11:00:00Z", - }, - ]), - ), - ) - .mockReturnValueOnce( + mockedRequest.mockReturnValueOnce( + Effect.succeed( + response([ + { id: 21, body: "Review", state: "COMMENT", submitted_at: "2026-09-03T11:00:00Z" }, + ]), + ), + ); + for (let page = 0; page < 4; page += 1) { + mockedRequest.mockReturnValueOnce( Effect.succeed( response( - [ - { - id: 31, - body: "First", - path: "src/a.ts", - position: 1, - created_at: "2026-09-03T11:01:00Z", - }, - ], + Array.from({ length: 50 }, (_, index) => ({ + id: 31 + page * 50 + index, + body: "Comment", + path: "src/a.ts", + position: 1, + created_at: "2026-09-03T11:01:00Z", + })), { "x-total-count": "501" }, ), ), - ) - .mockReturnValueOnce(Effect.succeed(response([], { "x-total-count": "501" }))) - .mockReturnValueOnce(Effect.succeed(response([], { "x-total-count": "501" }))) - .mockReturnValueOnce(Effect.succeed(response([], { "x-total-count": "501" }))) - .mockReturnValueOnce(Effect.succeed(response([], { "x-total-count": "501" }))); + ); + } const api = yield* GiteaPullRequestApi.make; const result = yield* api.listReviews({ host: "forge.example.test", repository: "acme/web", number: 7, }); - assert.isTrue(result.truncated); expect(result.comments).toContainEqual( expect.objectContaining({ id: "review-comment:31" }), ); - expect(callAt(5).path).toContain("page=5"); + expect(result.comments).toHaveLength(201); + expect(callAt(4).path).toContain("page=4"); + expect(mockedRequest).toHaveBeenCalledTimes(5); }), );