From 28f254753447e7a70ed0259909dcd46c24bf9d54 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Wed, 7 Oct 2026 00:23:23 -0700 Subject: [PATCH 1/3] perf(server): background GitHub reads pack into fewer queries Background branch lookups wait 500ms instead of 50ms for company, so a sweep's staggered lookups share one GraphQL document. PR summary batches grow from 25 to 50 aliases, which GitHub still prices at one point. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../PullRequestSyncReactor.ts | 2 +- .../src/pullRequest/GitHubPullRequestCli.ts | 10 +++++-- .../src/sourceControl/GitHubCli.test.ts | 27 ++++++++++--------- apps/server/src/sourceControl/GitHubCli.ts | 12 ++++++--- 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts b/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts index 5af4138bda9e..a631c1085453 100644 --- a/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts @@ -385,7 +385,7 @@ export const make = Effect.gen(function* () { }, // As wide as one batched summary read, so the sweep's reads on a host arrive together and // GitHub answers them in one request rather than one `gh pr view` apiece. - { concurrency: 25, discard: true }, + { concurrency: 50, discard: true }, ); // A host failure such as a signed-out CLI fails every due pull request the same way, so a // sweep reports one line per reason rather than one per pull request. diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index ce944b931aa8..2d9b49ab0d33 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -409,6 +409,12 @@ export interface GitHubPullRequestStat { */ const STAT_ALIASES_PER_REQUEST = 25; const STAT_REQUEST_CONCURRENCY = 4; +/** + * Summaries per aliased read. GitHub prices fifty at one point and seventy-five at two, and the + * background sync that asks for them in bulk waits on no one, so it takes the slower ~2.2s read + * of fifty over two reads of twenty-five. + */ +const SUMMARY_ALIASES_PER_REQUEST = 50; /** * How long a summary read waits for company. The background sync asks for every linked pull * request at once, and each read reaches the resolver after its own cache check, so a batch @@ -1754,7 +1760,7 @@ export const make = Effect.gen(function* () { /** * Summaries asked for together, on one host under one credential, share aliased GraphQL reads - * of twenty-five: the background sync reads every linked pull request each minute, and one + * of fifty: the background sync reads every linked pull request each minute, and one * `gh pr view` apiece is most of what it spends. Whatever the batch cannot answer — a selector * GraphQL cannot address, a pull request GitHub returned nothing for — is read on its own. */ @@ -1828,7 +1834,7 @@ export const make = Effect.gen(function* () { }, }).pipe( RequestResolver.setDelay(SUMMARY_BATCH_WINDOW), - RequestResolver.batchN(STAT_ALIASES_PER_REQUEST), + RequestResolver.batchN(SUMMARY_ALIASES_PER_REQUEST), ); const getPullRequestSummary: GitHubPullRequestCli["Service"]["getPullRequestSummary"] = (input) => Effect.request(new PullRequestSummaryRead(input), summaryResolver); diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index 2efab4674fc2..98608d3fb839 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -251,7 +251,7 @@ describe("GitHubCli repository resolution", () => { describe("GitHubCli.listPullRequestsByHead", () => { const remotes = remotesOutput(["origin", "git@github.com:acme/web.git"]); - it.effect("reads heads on one repository in one GraphQL document", () => { + it.effect("reads a background sweep's staggered heads in one GraphQL document", () => { const documents: Array = []; const { layer } = harness({ remotes, @@ -267,20 +267,23 @@ describe("GitHubCli.listPullRequestsByHead", () => { }); return Effect.gen(function* () { const gh = yield* GitHubCli.GitHubCli; - const lookups = yield* Effect.all( - ["feature/a", "feature/b"].map((headSelector) => - gh.listPullRequestsByHead({ + const lookup = (headSelector: string) => + gh + .listPullRequestsByHead({ cwd: "/repo", headSelector, state: "all", limit: 100, rateLimitHost: "github.com", - }), - ), - { concurrency: "unbounded" }, - ).pipe(Effect.forkChild); - yield* TestClock.adjust("50 millis"); - const [first, second] = yield* Fiber.join(lookups); + }) + .pipe(Effect.forkChild); + // Each branch's own git reads come first, so a sweep's lookups arrive spread out. + const firstLookup = yield* lookup("feature/a"); + yield* TestClock.adjust("200 millis"); + const secondLookup = yield* lookup("feature/b"); + yield* TestClock.adjust("300 millis"); + const first = yield* Fiber.join(firstLookup); + const second = yield* Fiber.join(secondLookup); assert.deepStrictEqual( first?.map((pr) => pr.number), [7], @@ -355,10 +358,10 @@ describe("GitHubCli.listPullRequestsByHead", () => { .listPullRequestsByHead({ cwd: "/repo", headSelector, state: "open", limit: 1 }) .pipe(Effect.flip, Effect.forkChild); const missing = yield* read("missing"); - yield* TestClock.adjust("50 millis"); + yield* TestClock.adjust("500 millis"); assert.strictEqual((yield* Fiber.join(missing))._tag, "GitHubCliUnavailableError"); const limited = yield* read("limited"); - yield* TestClock.adjust("50 millis"); + yield* TestClock.adjust("500 millis"); const error = yield* Fiber.join(limited); assert.strictEqual(error._tag, "GitHubCliRateLimitError"); assert.propertyVal(error, "retryAt", 123); diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index f4463be6de83..2973700add94 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -352,8 +352,11 @@ const HEAD_LOOKUPS_PER_DOCUMENT = 50; /** * How long a head lookup waits for company. Branch discovery reaches GitHub only after each * branch's own git reads, so lookups started together arrive tens of milliseconds apart. + * A background sweep's lookups spread over up to ~300ms, and every document costs a point no + * matter how few heads it holds, so lookups no user waits on (no reserve) wait longer. */ const HEAD_LOOKUP_BATCH_WINDOW = "50 millis"; +const BACKGROUND_HEAD_LOOKUP_BATCH_WINDOW = "500 millis"; /** A full document is 5,000 rows of well under 2 KB each. */ const HEAD_LOOKUP_MAX_RESPONSE_BYTES = 16_000_000; /** @@ -705,9 +708,12 @@ export const make = Effect.gen(function* () { ), ); }, - }).pipe( + }).pipe(RequestResolver.batchN(HEAD_LOOKUPS_PER_DOCUMENT)); + const interactiveHeadResolver = headResolver.pipe( RequestResolver.setDelay(HEAD_LOOKUP_BATCH_WINDOW), - RequestResolver.batchN(HEAD_LOOKUPS_PER_DOCUMENT), + ); + const backgroundHeadResolver = headResolver.pipe( + RequestResolver.setDelay(BACKGROUND_HEAD_LOOKUP_BATCH_WINDOW), ); const listByHead = Effect.fn("GitHubCli.listByHead")(function* (input: { @@ -733,7 +739,7 @@ export const make = Effect.gen(function* () { limit: ownerMatch ? OWNER_HEAD_SCAN_LIMIT : limit, allowReserve: input.allowReserve, }), - headResolver, + input.allowReserve ? interactiveHeadResolver : backgroundHeadResolver, ); if (!ownerMatch) return rows; const headOwner = ownerMatch[1]!.toLowerCase(); From 9f3b5ab29b94384509c2259b5912db5788df7b3a Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Wed, 7 Oct 2026 00:54:09 -0700 Subject: [PATCH 2/3] revert(server): keep summary batches at 25 One inaccessible pull request fails its whole batch into single reads, so a batch of fifty spreads that failure to twice as many healthy reads. Once settled threads stop syncing, few sweeps fill a batch of twenty-five anyway. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/orchestration-v2/PullRequestSyncReactor.ts | 2 +- apps/server/src/pullRequest/GitHubPullRequestCli.ts | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts b/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts index a631c1085453..5af4138bda9e 100644 --- a/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts @@ -385,7 +385,7 @@ export const make = Effect.gen(function* () { }, // As wide as one batched summary read, so the sweep's reads on a host arrive together and // GitHub answers them in one request rather than one `gh pr view` apiece. - { concurrency: 50, discard: true }, + { concurrency: 25, discard: true }, ); // A host failure such as a signed-out CLI fails every due pull request the same way, so a // sweep reports one line per reason rather than one per pull request. diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index 2d9b49ab0d33..ce944b931aa8 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -409,12 +409,6 @@ export interface GitHubPullRequestStat { */ const STAT_ALIASES_PER_REQUEST = 25; const STAT_REQUEST_CONCURRENCY = 4; -/** - * Summaries per aliased read. GitHub prices fifty at one point and seventy-five at two, and the - * background sync that asks for them in bulk waits on no one, so it takes the slower ~2.2s read - * of fifty over two reads of twenty-five. - */ -const SUMMARY_ALIASES_PER_REQUEST = 50; /** * How long a summary read waits for company. The background sync asks for every linked pull * request at once, and each read reaches the resolver after its own cache check, so a batch @@ -1760,7 +1754,7 @@ export const make = Effect.gen(function* () { /** * Summaries asked for together, on one host under one credential, share aliased GraphQL reads - * of fifty: the background sync reads every linked pull request each minute, and one + * of twenty-five: the background sync reads every linked pull request each minute, and one * `gh pr view` apiece is most of what it spends. Whatever the batch cannot answer — a selector * GraphQL cannot address, a pull request GitHub returned nothing for — is read on its own. */ @@ -1834,7 +1828,7 @@ export const make = Effect.gen(function* () { }, }).pipe( RequestResolver.setDelay(SUMMARY_BATCH_WINDOW), - RequestResolver.batchN(SUMMARY_ALIASES_PER_REQUEST), + RequestResolver.batchN(STAT_ALIASES_PER_REQUEST), ); const getPullRequestSummary: GitHubPullRequestCli["Service"]["getPullRequestSummary"] = (input) => Effect.request(new PullRequestSummaryRead(input), summaryResolver); From eccaeb96de43f7f8ed51bf81231768e4ba511cfe Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Wed, 7 Oct 2026 01:04:04 -0700 Subject: [PATCH 3/3] fix(server): cap background head lookups at 25 per document A fuller document can approach GitHub's 10s processing limit, and a failed document fails every head in it. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/sourceControl/GitHubCli.test.ts | 38 +++++++++++++++++++ apps/server/src/sourceControl/GitHubCli.ts | 9 ++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index 98608d3fb839..7e2b03fb1446 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -301,6 +301,44 @@ describe("GitHubCli.listPullRequestsByHead", () => { }).pipe(Effect.provide(layer)); }); + it.effect("caps a background document at twenty-five heads", () => { + const headCounts: Array = []; + const { layer } = harness({ + remotes, + api: { + graphql: (input) => + Effect.sync(() => { + const heads = Object.keys(input.variables ?? {}).filter((key) => /^h\d+$/.test(key)); + headCounts.push(heads.length); + return encodeJson({ + data: { repository: Object.fromEntries(heads.map((key) => [key, { nodes: [] }])) }, + }); + }), + }, + }); + return Effect.gen(function* () { + const gh = yield* GitHubCli.GitHubCli; + const lookups = yield* Effect.all( + Array.from({ length: 26 }, (_, index) => + gh.listPullRequestsByHead({ + cwd: "/repo", + headSelector: `feature/${index}`, + state: "all", + limit: 100, + rateLimitHost: "github.com", + }), + ), + { concurrency: "unbounded" }, + ).pipe(Effect.forkChild); + yield* TestClock.adjust("500 millis"); + yield* Fiber.join(lookups); + assert.deepStrictEqual( + headCounts.toSorted((a, b) => a - b), + [1, 25], + ); + }).pipe(Effect.provide(layer)); + }); + it.effect("matches an owner:branch selector on the head owner", () => { const { layer } = harness({ remotes, diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index 2973700add94..72cb8e261c69 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -353,10 +353,16 @@ const HEAD_LOOKUPS_PER_DOCUMENT = 50; * How long a head lookup waits for company. Branch discovery reaches GitHub only after each * branch's own git reads, so lookups started together arrive tens of milliseconds apart. * A background sweep's lookups spread over up to ~300ms, and every document costs a point no - * matter how few heads it holds, so lookups no user waits on (no reserve) wait longer. + * matter how few heads it holds, so reads without reserve wait longer. */ const HEAD_LOOKUP_BATCH_WINDOW = "50 millis"; const BACKGROUND_HEAD_LOOKUP_BATCH_WINDOW = "500 millis"; +/** + * Background documents fill up under the longer window, and a failed document fails every head + * in it. Fifty `main`-like heads of a hundred pull requests each took up to ~10s, GitHub's own + * processing limit; twenty-five took ~7s. + */ +const BACKGROUND_HEAD_LOOKUPS_PER_DOCUMENT = 25; /** A full document is 5,000 rows of well under 2 KB each. */ const HEAD_LOOKUP_MAX_RESPONSE_BYTES = 16_000_000; /** @@ -713,6 +719,7 @@ export const make = Effect.gen(function* () { RequestResolver.setDelay(HEAD_LOOKUP_BATCH_WINDOW), ); const backgroundHeadResolver = headResolver.pipe( + RequestResolver.batchN(BACKGROUND_HEAD_LOOKUPS_PER_DOCUMENT), RequestResolver.setDelay(BACKGROUND_HEAD_LOOKUP_BATCH_WINDOW), );