From d3b564c2a07b9247f85e14b1522b46ab0399b1b5 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 21 Oct 2026 01:24:02 +0000 Subject: [PATCH 1/6] fix(gitcafe): report line counts for linked and listed pull requests --- .../server/GitCafePullRequestProvider.test.ts | 39 +++++++++++ .../src/server/GitCafePullRequestProvider.ts | 67 +++++++++++++++++-- 2 files changed, 99 insertions(+), 7 deletions(-) diff --git a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts index f5115c99af0d1..0afa242347c8a 100644 --- a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts +++ b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts @@ -547,6 +547,45 @@ describe("GitCafePullRequestProvider", () => { }).pipe(Effect.provide(server.layer)); }); + it.effect("summarizes line counts once per revision", () => { + const snapshot = { version: 4, headOid, comparisonBaseOid: baseOid }; + const server = fakeGitCafe({ + "GET /repos/owner/repo/pulls/7": pull, + "GET /repos/owner/repo/pulls/7/changes": { + ...snapshot, + items: [{ path: "a.ts" }, { path: "b.ts" }], + next: null, + }, + "POST /repos/owner/repo/pulls/7/diff-files": (request: SentRequest) => ({ + ...snapshot, + items: (request.body as { paths: ReadonlyArray }).paths.map((path) => ({ + path, + additions: 3, + deletions: 1, + })), + }), + }); + return Effect.gen(function* () { + const provider = yield* GitCafePullRequestProvider.make; + const first = yield* provider.getChangeRequestSummary!(target); + const second = yield* provider.getChangeRequestSummary!(target); + const listed = yield* provider.listChangeRequestStats!({ + cwd: target.cwd, + host: target.host, + changeRequests: [{ repository: "owner/repo", number: 7 }], + }); + assert.deepInclude(first, { additions: 6, deletions: 2, changedFiles: 2 }); + assert.deepInclude(second, { additions: 6, deletions: 2, changedFiles: 2 }); + assert.deepStrictEqual(listed, [ + { repository: "owner/repo", number: 7, additions: 6, deletions: 2 }, + ]); + assert.lengthOf( + server.sent.filter((request) => request.path.endsWith("/changes")), + 1, + ); + }).pipe(Effect.provide(server.layer)); + }); + it.effect("fences a native merge on the target branch's current commit", () => { const branchOid = "fedcba9876543210fedcba9876543210fedcba98"; const server = fakeGitCafe({ diff --git a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts index 41683de1767d7..bb1495c3b30ba 100644 --- a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts +++ b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts @@ -53,6 +53,7 @@ const DIFF_FILES_BATCH = 64; */ const LINE_STATS_MAX_FILES = DIFF_FILES_BATCH * 4; const MAX_CHANGE_PAGES = 20; +const LINE_STATS_CACHE_SIZE = 256; const chunk = (items: ReadonlyArray, size: number): Array> => Array.from({ length: Math.ceil(items.length / size) }, (_, index) => @@ -390,7 +391,7 @@ export const make = Effect.gen(function* () { chunk(detailablePaths(files), DIFF_FILES_BATCH), (paths) => readDiffFiles(input, revision, paths), { concurrency: 4 }, - ).pipe(Effect.orElseSucceed(() => [[]]))).flat(); + )).flat(); return { changedFiles: files.length, additions: counted.reduce((total, file) => total + (file.additions ?? 0), 0), @@ -398,6 +399,27 @@ export const make = Effect.gen(function* () { }; }); + // GitCafe's pulls carry no line totals, and linked pull requests are summarized every minute, + // so totals are held per revision rather than recounted from hunks on every read. + const lineStatsByRevision = new Map>>(); + const lineStats = ( + input: PullRef, + pull: { readonly version: number; readonly headOid: string }, + ) => { + const key = `${input.host} ${input.repository.toLowerCase()} ${input.number} ${pull.version} ${pull.headOid}`; + const held = lineStatsByRevision.get(key); + if (held !== undefined) return Effect.succeed(held); + return readLineStats(input, pull.version).pipe( + Effect.tap((stats) => + Effect.sync(() => { + if (lineStatsByRevision.size >= LINE_STATS_CACHE_SIZE) + lineStatsByRevision.delete(lineStatsByRevision.keys().next().value!); + lineStatsByRevision.set(key, stats); + }), + ), + ); + }; + const merge = Effect.fn("GitCafePullRequestProvider.merge")(function* ( input: Parameters[0], ) { @@ -525,14 +547,43 @@ export const make = Effect.gen(function* () { }; }, ), - getChangeRequestSummary: (input) => - readPull("getChangeRequestSummary", input).pipe( - Effect.map((pull) => ({ + listChangeRequestStats: (input) => + Effect.forEach( + input.changeRequests, + ({ repository, number }) => { + const ref = { cwd: input.cwd, host: input.host, repository, number }; + return readPull("listChangeRequestStats", ref).pipe( + Effect.flatMap((pull) => + pull.headOid === null + ? Effect.succeed([]) + : lineStats(ref, { version: pull.version, headOid: pull.headOid }).pipe( + Effect.map(({ additions, deletions }) => [ + { repository, number, additions, deletions }, + ]), + ), + ), + Effect.orElseSucceed(() => []), + ); + }, + { concurrency: 4 }, + ).pipe(Effect.map((stats) => stats.flat())), + getChangeRequestSummary: Effect.fn("GitCafePullRequestProvider.getChangeRequestSummary")( + function* (input) { + const pull = yield* readPull("getChangeRequestSummary", input); + const changes = + pull.headOid === null + ? null + : yield* lineStats(input, { version: pull.version, headOid: pull.headOid }).pipe( + Effect.orElseSucceed(() => null), + ); + return { ...Json.toChangeRequest(pull, input), + ...(changes === null ? {} : changes), closedAt: pull.closedAt, mergedAt: pull.mergedAt, - })), - ), + }; + }, + ), getChangeRequest: Effect.fn("GitCafePullRequestProvider.getChangeRequest")(function* (input) { const pull = yield* readPull("getChangeRequest", input); const path = yield* pullPath("getChangeRequest", input); @@ -550,7 +601,9 @@ export const make = Effect.gen(function* () { ), pull.headOid === null ? Effect.succeed(null) - : readLineStats(input, pull.version).pipe(Effect.orElseSucceed(() => null)), + : lineStats(input, { version: pull.version, headOid: pull.headOid }).pipe( + Effect.orElseSucceed(() => null), + ), ], { concurrency: 3 }, ); From db9d59bc8880bcbc657b8f2c037d1fd4435ae2be Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 21 Oct 2026 02:03:07 +0000 Subject: [PATCH 2/6] fix(gitcafe): count lines only for linked pull requests, once per head --- .../server/GitCafePullRequestProvider.test.ts | 14 +++----- .../src/server/GitCafePullRequestProvider.ts | 35 +++++-------------- 2 files changed, 12 insertions(+), 37 deletions(-) diff --git a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts index 0afa242347c8a..3e7d63e0e24a1 100644 --- a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts +++ b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts @@ -547,10 +547,12 @@ describe("GitCafePullRequestProvider", () => { }).pipe(Effect.provide(server.layer)); }); - it.effect("summarizes line counts once per revision", () => { + it.effect("summarizes line counts once per head commit", () => { const snapshot = { version: 4, headOid, comparisonBaseOid: baseOid }; + let pullReads = 0; const server = fakeGitCafe({ - "GET /repos/owner/repo/pulls/7": pull, + // A comment moves the version without moving the head. + "GET /repos/owner/repo/pulls/7": () => ({ ...pull, version: 4 + pullReads++ }), "GET /repos/owner/repo/pulls/7/changes": { ...snapshot, items: [{ path: "a.ts" }, { path: "b.ts" }], @@ -569,16 +571,8 @@ describe("GitCafePullRequestProvider", () => { const provider = yield* GitCafePullRequestProvider.make; const first = yield* provider.getChangeRequestSummary!(target); const second = yield* provider.getChangeRequestSummary!(target); - const listed = yield* provider.listChangeRequestStats!({ - cwd: target.cwd, - host: target.host, - changeRequests: [{ repository: "owner/repo", number: 7 }], - }); assert.deepInclude(first, { additions: 6, deletions: 2, changedFiles: 2 }); assert.deepInclude(second, { additions: 6, deletions: 2, changedFiles: 2 }); - assert.deepStrictEqual(listed, [ - { repository: "owner/repo", number: 7, additions: 6, deletions: 2 }, - ]); assert.lengthOf( server.sent.filter((request) => request.path.endsWith("/changes")), 1, diff --git a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts index bb1495c3b30ba..9997178a3c9d8 100644 --- a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts +++ b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts @@ -400,21 +400,22 @@ export const make = Effect.gen(function* () { }); // GitCafe's pulls carry no line totals, and linked pull requests are summarized every minute, - // so totals are held per revision rather than recounted from hunks on every read. - const lineStatsByRevision = new Map>>(); + // so totals are held per head commit rather than recounted from hunks on every read. The + // pull's version also moves on comments and reviews, which change no lines. + const lineStatsByHead = new Map>>(); const lineStats = ( input: PullRef, pull: { readonly version: number; readonly headOid: string }, ) => { - const key = `${input.host} ${input.repository.toLowerCase()} ${input.number} ${pull.version} ${pull.headOid}`; - const held = lineStatsByRevision.get(key); + const key = `${input.host} ${input.repository.toLowerCase()} ${input.number} ${pull.headOid}`; + const held = lineStatsByHead.get(key); if (held !== undefined) return Effect.succeed(held); return readLineStats(input, pull.version).pipe( Effect.tap((stats) => Effect.sync(() => { - if (lineStatsByRevision.size >= LINE_STATS_CACHE_SIZE) - lineStatsByRevision.delete(lineStatsByRevision.keys().next().value!); - lineStatsByRevision.set(key, stats); + if (lineStatsByHead.size >= LINE_STATS_CACHE_SIZE) + lineStatsByHead.delete(lineStatsByHead.keys().next().value!); + lineStatsByHead.set(key, stats); }), ), ); @@ -547,26 +548,6 @@ export const make = Effect.gen(function* () { }; }, ), - listChangeRequestStats: (input) => - Effect.forEach( - input.changeRequests, - ({ repository, number }) => { - const ref = { cwd: input.cwd, host: input.host, repository, number }; - return readPull("listChangeRequestStats", ref).pipe( - Effect.flatMap((pull) => - pull.headOid === null - ? Effect.succeed([]) - : lineStats(ref, { version: pull.version, headOid: pull.headOid }).pipe( - Effect.map(({ additions, deletions }) => [ - { repository, number, additions, deletions }, - ]), - ), - ), - Effect.orElseSucceed(() => []), - ); - }, - { concurrency: 4 }, - ).pipe(Effect.map((stats) => stats.flat())), getChangeRequestSummary: Effect.fn("GitCafePullRequestProvider.getChangeRequestSummary")( function* (input) { const pull = yield* readPull("getChangeRequestSummary", input); From 61c100dc364f8e807aeab0c947ea5d0e6d6df7ac Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 21 Oct 2026 04:49:43 +0000 Subject: [PATCH 3/6] chore: retrigger ci From 736b01f41c21ee9d0ae4581e6cdc54fea857e724 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 21 Oct 2026 04:58:52 +0000 Subject: [PATCH 4/6] fix(gitcafe): leave summary line counts out when they cannot be read --- .../src/server/GitCafePullRequestProvider.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts index 9997178a3c9d8..477661b1936f3 100644 --- a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts +++ b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts @@ -557,8 +557,14 @@ export const make = Effect.gen(function* () { : yield* lineStats(input, { version: pull.version, headOid: pull.headOid }).pipe( Effect.orElseSucceed(() => null), ); + // A count that could not be read is absent rather than zero. + const { + additions: _additions, + deletions: _deletions, + ...summary + } = Json.toChangeRequest(pull, input); return { - ...Json.toChangeRequest(pull, input), + ...summary, ...(changes === null ? {} : changes), closedAt: pull.closedAt, mergedAt: pull.mergedAt, From a898a06f6875d662bae1748fbcdb54762cfc9f4a Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 21 Oct 2026 05:13:43 +0000 Subject: [PATCH 5/6] fix(gitcafe): key cached line totals on the compared commits --- .../server/GitCafePullRequestProvider.test.ts | 33 ++++++---- .../src/server/GitCafePullRequestProvider.ts | 64 ++++++++----------- 2 files changed, 48 insertions(+), 49 deletions(-) diff --git a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts index 3e7d63e0e24a1..8ee4568495fd3 100644 --- a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts +++ b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts @@ -547,36 +547,45 @@ describe("GitCafePullRequestProvider", () => { }).pipe(Effect.provide(server.layer)); }); - it.effect("summarizes line counts once per head commit", () => { - const snapshot = { version: 4, headOid, comparisonBaseOid: baseOid }; + it.effect("recounts summary lines only when the compared commits move", () => { + const landedBase = "fedcba9876543210fedcba9876543210fedcba98"; + let comparisonBaseOid = baseOid; let pullReads = 0; const server = fakeGitCafe({ - // A comment moves the version without moving the head. + // A comment moves the version without moving the compared commits. "GET /repos/owner/repo/pulls/7": () => ({ ...pull, version: 4 + pullReads++ }), - "GET /repos/owner/repo/pulls/7/changes": { - ...snapshot, + "GET /repos/owner/repo/pulls/7/changes": () => ({ + version: 4, + headOid, + comparisonBaseOid, items: [{ path: "a.ts" }, { path: "b.ts" }], next: null, - }, + }), "POST /repos/owner/repo/pulls/7/diff-files": (request: SentRequest) => ({ - ...snapshot, + version: 4, + headOid, + comparisonBaseOid, items: (request.body as { paths: ReadonlyArray }).paths.map((path) => ({ path, - additions: 3, + additions: comparisonBaseOid === baseOid ? 3 : 1, deletions: 1, })), }), }); + const hunkReads = () => + server.sent.filter((request) => request.path.endsWith("/diff-files")).length; return Effect.gen(function* () { const provider = yield* GitCafePullRequestProvider.make; const first = yield* provider.getChangeRequestSummary!(target); const second = yield* provider.getChangeRequestSummary!(target); assert.deepInclude(first, { additions: 6, deletions: 2, changedFiles: 2 }); assert.deepInclude(second, { additions: 6, deletions: 2, changedFiles: 2 }); - assert.lengthOf( - server.sent.filter((request) => request.path.endsWith("/changes")), - 1, - ); + assert.strictEqual(hunkReads(), 1); + // A landed parent layer moves the comparison base under the same head. + comparisonBaseOid = landedBase; + const third = yield* provider.getChangeRequestSummary!(target); + assert.deepInclude(third, { additions: 2, deletions: 2, changedFiles: 2 }); + assert.strictEqual(hunkReads(), 2); }).pipe(Effect.provide(server.layer)); }); diff --git a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts index 477661b1936f3..c3c344e2bd5cd 100644 --- a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts +++ b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts @@ -355,6 +355,11 @@ export const make = Effect.gen(function* () { return yield* failure(operation, `GitCafe's stack operation ${outcome.state}.`); }); + // GitCafe's pulls carry no line totals, and linked pull requests are summarized every minute, + // so hunk totals are held per compared pair of commits. The file list that names the pair is + // cheap; a retarget or a landed parent layer moves the base without moving the head. + const lineTotals = new Map(); + const readLineStats = Effect.fn("GitCafePullRequestProvider.readLineStats")(function* ( input: PullRef, version: number, @@ -382,45 +387,34 @@ export const make = Effect.gen(function* () { after = batch.next; if (after === null || after === previous || batch.items.length === 0) break; } - const counted = - files.some((file) => file.additions !== undefined) || - files.length > LINE_STATS_MAX_FILES || - revision === undefined - ? files - : (yield* Effect.forEach( + if (revision === undefined) return { changedFiles: 0, additions: 0, deletions: 0 }; + const key = `${input.host} ${input.repository.toLowerCase()} ${input.number} ${revision.baseOid} ${revision.headOid}`; + const held = lineTotals.get(key); + if (held !== undefined) return { changedFiles: files.length, ...held }; + const details = + files.some((file) => file.additions !== undefined) || files.length > LINE_STATS_MAX_FILES + ? Option.some(files) + : yield* Effect.forEach( chunk(detailablePaths(files), DIFF_FILES_BATCH), (paths) => readDiffFiles(input, revision, paths), { concurrency: 4 }, - )).flat(); - return { - changedFiles: files.length, + ).pipe( + Effect.map((batches) => batches.flat()), + Effect.option, + ); + // Unread hunks leave the lines unknown rather than zero; the file count still stands. + if (Option.isNone(details)) return { changedFiles: files.length }; + const counted = details.value; + const totals = { additions: counted.reduce((total, file) => total + (file.additions ?? 0), 0), deletions: counted.reduce((total, file) => total + (file.deletions ?? 0), 0), }; + if (lineTotals.size >= LINE_STATS_CACHE_SIZE) + lineTotals.delete(lineTotals.keys().next().value!); + lineTotals.set(key, totals); + return { changedFiles: files.length, ...totals }; }); - // GitCafe's pulls carry no line totals, and linked pull requests are summarized every minute, - // so totals are held per head commit rather than recounted from hunks on every read. The - // pull's version also moves on comments and reviews, which change no lines. - const lineStatsByHead = new Map>>(); - const lineStats = ( - input: PullRef, - pull: { readonly version: number; readonly headOid: string }, - ) => { - const key = `${input.host} ${input.repository.toLowerCase()} ${input.number} ${pull.headOid}`; - const held = lineStatsByHead.get(key); - if (held !== undefined) return Effect.succeed(held); - return readLineStats(input, pull.version).pipe( - Effect.tap((stats) => - Effect.sync(() => { - if (lineStatsByHead.size >= LINE_STATS_CACHE_SIZE) - lineStatsByHead.delete(lineStatsByHead.keys().next().value!); - lineStatsByHead.set(key, stats); - }), - ), - ); - }; - const merge = Effect.fn("GitCafePullRequestProvider.merge")(function* ( input: Parameters[0], ) { @@ -554,9 +548,7 @@ export const make = Effect.gen(function* () { const changes = pull.headOid === null ? null - : yield* lineStats(input, { version: pull.version, headOid: pull.headOid }).pipe( - Effect.orElseSucceed(() => null), - ); + : yield* readLineStats(input, pull.version).pipe(Effect.orElseSucceed(() => null)); // A count that could not be read is absent rather than zero. const { additions: _additions, @@ -588,9 +580,7 @@ export const make = Effect.gen(function* () { ), pull.headOid === null ? Effect.succeed(null) - : lineStats(input, { version: pull.version, headOid: pull.headOid }).pipe( - Effect.orElseSucceed(() => null), - ), + : readLineStats(input, pull.version).pipe(Effect.orElseSucceed(() => null)), ], { concurrency: 3 }, ); From a58e478cae54f1cffd73dfdb610a63fd6788fcd5 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 21 Oct 2026 05:23:11 +0000 Subject: [PATCH 6/6] fix(gitcafe): leave line counts unknown for pulls too large to count --- .../src/server/GitCafePullRequestProvider.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts index c3c344e2bd5cd..4347a2acec433 100644 --- a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts +++ b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.ts @@ -391,9 +391,10 @@ export const make = Effect.gen(function* () { const key = `${input.host} ${input.repository.toLowerCase()} ${input.number} ${revision.baseOid} ${revision.headOid}`; const held = lineTotals.get(key); if (held !== undefined) return { changedFiles: files.length, ...held }; - const details = - files.some((file) => file.additions !== undefined) || files.length > LINE_STATS_MAX_FILES - ? Option.some(files) + const details = files.some((file) => file.additions !== undefined) + ? Option.some(files) + : files.length > LINE_STATS_MAX_FILES + ? Option.none() : yield* Effect.forEach( chunk(detailablePaths(files), DIFF_FILES_BATCH), (paths) => readDiffFiles(input, revision, paths),