Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,48 @@ describe("GitCafePullRequestProvider", () => {
}).pipe(Effect.provide(server.layer));
});

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 compared commits.
"GET /repos/owner/repo/pulls/7": () => ({ ...pull, version: 4 + pullReads++ }),
"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) => ({
version: 4,
headOid,
comparisonBaseOid,
items: (request.body as { paths: ReadonlyArray<string> }).paths.map((path) => ({
path,
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.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));
});

it.effect("fences a native merge on the target branch's current commit", () => {
const branchOid = "fedcba9876543210fedcba9876543210fedcba98";
const server = fakeGitCafe({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <A>(items: ReadonlyArray<A>, size: number): Array<ReadonlyArray<A>> =>
Array.from({ length: Math.ceil(items.length / size) }, (_, index) =>
Expand Down Expand Up @@ -354,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<string, { readonly additions: number; readonly deletions: number }>();

const readLineStats = Effect.fn("GitCafePullRequestProvider.readLineStats")(function* (
input: PullRef,
version: number,
Expand Down Expand Up @@ -381,21 +387,33 @@ 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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
? 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),
{ concurrency: 4 },
).pipe(Effect.orElseSucceed(() => [[]]))).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 };
});

const merge = Effect.fn("GitCafePullRequestProvider.merge")(function* (
Expand Down Expand Up @@ -525,14 +543,27 @@ export const make = Effect.gen(function* () {
};
},
),
getChangeRequestSummary: (input) =>
readPull("getChangeRequestSummary", input).pipe(
Effect.map((pull) => ({
...Json.toChangeRequest(pull, input),
getChangeRequestSummary: Effect.fn("GitCafePullRequestProvider.getChangeRequestSummary")(
function* (input) {
const pull = yield* readPull("getChangeRequestSummary", input);
const changes =
pull.headOid === null
? 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,
deletions: _deletions,
...summary
} = Json.toChangeRequest(pull, input);
return {
...summary,
...(changes === null ? {} : changes),
closedAt: pull.closedAt,
mergedAt: pull.mergedAt,
})),
),
};
},
),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
getChangeRequest: Effect.fn("GitCafePullRequestProvider.getChangeRequest")(function* (input) {
const pull = yield* readPull("getChangeRequest", input);
const path = yield* pullPath("getChangeRequest", input);
Expand Down
Loading