fix(selfhost): cap historical PR file hydration and cache by head SHA

migrations/0090_pull_request_detail_sync_head_sha.sql

@@ -0,0 +1,2 @@
+ALTER TABLE pull_request_detail_sync_state
+  ADD COLUMN head_sha TEXT;

src/db/repositories.ts

@@ -1098,6 +1098,12 @@ export async function getRepoQueueTrendSnapshot(env: Env, repoFullName: string):
   return row ? toRepoQueueTrendSnapshotRecord(row) : null;
 }
 
+// PARTIAL-UPDATE CONTRACT: an omitted (`undefined`) field on `state` leaves that column UNCHANGED on conflict —
+// drizzle's `onConflictDoUpdate` strips `undefined` entries from the generated SQL `SET` clause rather than
+// writing NULL. Every "running" pre-fetch stamp (backfill.ts) relies on this to touch only `status` without
+// clearing the PREVIOUS `headSha`/`*SyncedAt` row — including the repo+PR+headSha file cache
+// (#audit-rate-headroom), which would silently stop hitting if a future edit here coalesced an omitted field to
+// `null` (e.g. `headSha: state.headSha ?? null`). Pass `null` explicitly to actually clear a column.
 export async function upsertPullRequestDetailSyncState(env: Env, state: PullRequestDetailSyncStateRecord): Promise<void> {
   const db = getDb(env.DB);
   await db
@@ -1107,6 +1113,7 @@ export async function upsertPullRequestDetailSyncState(env: Env, state: PullRequ
       repoFullName: state.repoFullName,
       pullNumber: state.pullNumber,
       status: state.status,
+      headSha: state.headSha,
       filesSyncedAt: state.filesSyncedAt,
       reviewsSyncedAt: state.reviewsSyncedAt,
       checksSyncedAt: state.checksSyncedAt,
@@ -1118,6 +1125,7 @@ export async function upsertPullRequestDetailSyncState(env: Env, state: PullRequ
       target: [pullRequestDetailSyncState.repoFullName, pullRequestDetailSyncState.pullNumber],
       set: {
         status: state.status,
+        headSha: state.headSha,
         filesSyncedAt: state.filesSyncedAt,
         reviewsSyncedAt: state.reviewsSyncedAt,
         checksSyncedAt: state.checksSyncedAt,
@@ -1128,6 +1136,16 @@ export async function upsertPullRequestDetailSyncState(env: Env, state: PullRequ
     });
 }
 
+export async function getPullRequestDetailSyncState(env: Env, fullName: string, pullNumber: number): Promise<PullRequestDetailSyncStateRecord | null> {
+  const db = getDb(env.DB);
+  const [row] = await db
+    .select()
+    .from(pullRequestDetailSyncState)
+    .where(and(eq(pullRequestDetailSyncState.repoFullName, fullName), eq(pullRequestDetailSyncState.pullNumber, pullNumber)))
+    .limit(1);
+  return row ? toPullRequestDetailSyncStateRecord(row) : null;
+}
+
 export async function listPullRequestDetailSyncStates(env: Env, fullName: string): Promise<PullRequestDetailSyncStateRecord[]> {
   const db = getDb(env.DB);
   const rows = await db.select().from(pullRequestDetailSyncState).where(eq(pullRequestDetailSyncState.repoFullName, fullName)).limit(2000);
@@ -4147,6 +4165,7 @@ function toPullRequestDetailSyncStateRecord(row: typeof pullRequestDetailSyncSta
     repoFullName: row.repoFullName,
     pullNumber: row.pullNumber,
     status: parsePullRequestDetailSyncStatus(row.status),
+    headSha: row.headSha,
     filesSyncedAt: row.filesSyncedAt,
     reviewsSyncedAt: row.reviewsSyncedAt,
     checksSyncedAt: row.checksSyncedAt,

src/db/schema.ts

@@ -215,6 +215,9 @@ export const pullRequestDetailSyncState = sqliteTable(
     repoFullName: text("repo_full_name").notNull(),
     pullNumber: integer("pull_number").notNull(),
     status: text("status").notNull().default("never_synced"),
+    // The head SHA the FILES were last synced for (not the review/checks SHA) — lets a caller skip a
+    // `/pulls/{n}/files` refetch when the PR's current head still matches what is already stored (#audit-rate-headroom).
+    headSha: text("head_sha"),
     filesSyncedAt: text("files_synced_at"),
     reviewsSyncedAt: text("reviews_synced_at"),
     checksSyncedAt: text("checks_synced_at"),

src/github/backfill.ts

@@ -8,6 +8,7 @@ import {
   deletePullRequestFiles,
   countRepoLabels,
   getInstallation,
+  getPullRequestDetailSyncState,
   listRepoGithubTotalsSnapshotHistory,
   getRepoSyncSegment,
   getRepoSyncState,
@@ -67,7 +68,7 @@ import {
   GITTENSORY_LEGACY_GATE_CHECK_NAME,
 } from "../review/check-names";
 import { buildReviewThreadBlocker, type ReviewThreadBlocker } from "../review/review-thread-findings";
-import { delayUntil, shouldWaitForGitHubRateLimit } from "./rate-limit";
+import { delayUntil, HISTORICAL_BACKFILL_RESERVED_HEADROOM, shouldWaitForGitHubRateLimit } from "./rate-limit";
 import {
   githubRateLimitAdmissionKeyForPublicToken,
   githubRateLimitAdmissionKeyForToken,
@@ -76,6 +77,7 @@ import {
   type GitHubRateLimitAdmissionKey,
 } from "./client";
 import { fetchCachedGitHubGraphQl } from "./graphql-cache";
+import { incr } from "../selfhost/metrics";
 type GitHubLabelPayload = {
   name: string;
   color?: string;
@@ -316,6 +318,14 @@ const FRESH_SYNC_MS = 6 * 60 * 60 * 1000;
 const ERROR_BACKOFF_MS = 60 * 60 * 1000;
 const SEGMENT_PAGE_BUDGET: Record<BackfillMode, number> = { light: 2, full: 10, resume: 10 };
 const PR_DETAIL_BATCH_SIZE: Record<BackfillMode, number> = { light: 12, full: 40, resume: 40 };
+// Caps how many NOT-yet-hydrated merged PRs get a `/pulls/{n}/files` fetch per `recent_merged_pull_requests`
+// page (independent of SEGMENT_PAGE_BUDGET, which only caps LIST pages). Without this a repo with a large
+// un-hydrated merged-PR backlog can fan out one files fetch per PR across up to SEGMENT_PAGE_BUDGET * 100 PRs
+// in a single job execution, draining the shared installation bucket before the once-per-segment rate check
+// runs again (#audit-rate-headroom). Any PR left un-hydrated this run stays a candidate on the next page/run.
+const MERGED_PR_FILE_HYDRATION_BATCH_SIZE: Record<BackfillMode, number> = { light: 10, full: 20, resume: 20 };
+const PULL_REQUEST_FILES_FETCH_METRIC = "gittensory_github_pull_request_files_fetch_total";
+type PullRequestFilesFetchCaller = "backfill_open_pr_details" | "backfill_merged_history" | "live_review";
 const CURRENT_OPEN_SCAN_MARKER = "gittensory-current-open-scan-v1";
 const FRESH_TOTALS_SNAPSHOT_MS = 10 * 60 * 1000;
 const TOTALS_SNAPSHOT_LOOKBACK = 8;
@@ -603,13 +613,14 @@ export async function backfillOpenPullRequestDetails(
   await mapWithConcurrency(batch, 2, async (pr) => {
     await upsertPullRequestDetailSyncState(env, { repoFullName: repo.fullName, pullNumber: pr.number, status: "running" });
     const before = warnings.length;
-    await fetchAndStorePullRequestDetails(env, repo.fullName, pr, token, warnings, admissionKey);
+    await fetchAndStorePullRequestDetails(env, repo.fullName, pr, token, warnings, admissionKey, "backfill_open_pr_details");
     const syncedAt = nowIso();
     const newWarnings = warnings.slice(before);
     await upsertPullRequestDetailSyncState(env, {
       repoFullName: repo.fullName,
       pullNumber: pr.number,
       status: newWarnings.length > 0 ? "partial" : "complete",
+      headSha: pr.headSha,
       filesSyncedAt: syncedAt,
       reviewsSyncedAt: syncedAt,
       checksSyncedAt: syncedAt,
@@ -659,22 +670,33 @@ export async function refreshPullRequestDetails(
   env: Env,
   repoFullName: string,
   pullNumber: number,
+  options: { force?: boolean } = {},
 ): Promise<{ ok: true; repoFullName: string; pullNumber: number; status: PullRequestDetailSyncStateRecord["status"]; warnings: string[] }> {
   const [repo, pr] = await Promise.all([getRepository(env, repoFullName), getPullRequest(env, repoFullName, pullNumber)]);
   if (!repo || !pr) {
     return { ok: true, repoFullName, pullNumber, status: "partial", warnings: ["Repository or pull request was not found."] };
   }
+  // Closed/missing PR guard (#audit-rate-headroom): a CLOSED PR that already has a complete detail sync has all
+  // the outcome/telemetry it will ever need — GitHub's data for it is final. Skip the files/reviews/checks
+  // refetch unless the caller explicitly forces one (e.g. the manual "review-now" repair command).
+  if (!options.force && pr.state !== "open") {
+    const existingState = await getPullRequestDetailSyncState(env, repoFullName, pullNumber);
+    if (existingState?.status === "complete") {
+      return { ok: true, repoFullName, pullNumber, status: existingState.status, warnings: [] };
+    }
+  }
   const token = await tokenForRepo(env, repo);
   const admissionKey = repoAdmissionKeyForToken(env, repo, token);
   const warnings: string[] = [];
   await upsertPullRequestDetailSyncState(env, { repoFullName, pullNumber, status: "running" });
-  await fetchAndStorePullRequestDetails(env, repoFullName, pr, token, warnings, admissionKey);
+  await fetchAndStorePullRequestDetails(env, repoFullName, pr, token, warnings, admissionKey, "live_review", { forceFiles: options.force });
   const syncedAt = nowIso();
   const status: PullRequestDetailSyncStateRecord["status"] = warnings.length > 0 ? "partial" : "complete";
   await upsertPullRequestDetailSyncState(env, {
     repoFullName,
     pullNumber,
     status,
+    headSha: pr.headSha,
     filesSyncedAt: syncedAt,
     reviewsSyncedAt: syncedAt,
     checksSyncedAt: syncedAt,
@@ -1276,7 +1298,7 @@ async function backfillRecentMergedSegment(
       // recent_merged_pull_requests.changedFiles is populated instead of always empty.
       const warnings: string[] = [];
       const admissionKey = repoAdmissionKeyForToken(env, repo, token);
-      await hydrateMergedPullRequestFiles(env, repo.fullName, merged, token, warnings, 8, admissionKey);
+      await hydrateMergedPullRequestFiles(env, repo.fullName, merged, token, warnings, 8, mode, admissionKey);
       return merged.length;
     },
     { progressiveHistory: true, countPersisted: () => countRecentMergedPullRequests(env, repo.fullName) },
@@ -1287,25 +1309,37 @@ async function backfillRecentMergedSegment(
 // fetch — the N+1 REST fan-out that dominated this segment's GitHub cost — for any merged PR ALREADY hydrated,
 // re-upserting only the cheap metadata (the upsert preserves the stored files when passed an empty list). One
 // `listRecentMergedPullRequests` read per batch replaces up to one `/files` fetch per merged PR. (#1941)
+//
+// This is scheduled, historical work: none of it is needed for a CURRENT review, so it is both hard-capped
+// (MERGED_PR_FILE_HYDRATION_BATCH_SIZE, independent of the page's own size) and budget-gated at the earliest,
+// most conservative floor (HISTORICAL_BACKFILL_RESERVED_HEADROOM) — re-checked on every page, not just once at
+// segment entry, so a large un-hydrated backlog can never flood the shared bucket in one job execution
+// (#audit-rate-headroom). A PR skipped for either reason is upserted with cheap metadata only (empty
+// changedFiles, preserved by the upsert if already hydrated) and stays a candidate on the next run.
 async function hydrateMergedPullRequestFiles(
   env: Env,
   repoFullName: string,
   merged: GitHubPullRequestPayload[],
   token: string | undefined,
   warnings: string[],
   concurrency: number,
+  mode: BackfillMode,
   admissionKey?: GitHubRateLimitAdmissionKey,
 ): Promise<void> {
   const alreadyHydrated = new Set(
     (await listRecentMergedPullRequests(env, repoFullName))
       .filter((record) => record.changedFiles.length > 0)
       .map((record) => record.number),
   );
+  const pending = merged.filter((pr) => !alreadyHydrated.has(pr.number));
+  const resetAt = pending.length > 0 ? await shouldWaitForGitHubRateLimit(env, HISTORICAL_BACKFILL_RESERVED_HEADROOM) : undefined;
+  if (resetAt) warnings.push(`Historical merged PR file hydration deferred for ${pending.length} pull request(s): GitHub REST budget below the historical-backfill headroom floor (retry after ${resetAt}).`);
+  const budgeted = resetAt ? new Set<number>() : new Set(pending.slice(0, MERGED_PR_FILE_HYDRATION_BATCH_SIZE[mode]).map((pr) => pr.number));
   await mapWithConcurrency(merged, concurrency, async (pr) => {
     // fetchPullRequestFiles never throws — it returns [] (and records a warning) on any fetch failure.
-    const changedFiles = alreadyHydrated.has(pr.number)
-      ? []
-      : await fetchPullRequestFiles(env, repoFullName, pr.number, token, warnings, admissionKey);
+    const changedFiles = budgeted.has(pr.number)
+      ? await fetchPullRequestFiles(env, repoFullName, pr.number, token, warnings, admissionKey, "backfill_merged_history")
+      : [];
     await upsertRecentMergedPullRequest(env, toRecentMergedPullRequest(repoFullName, pr, changedFiles));
   });
 }
@@ -1767,12 +1801,30 @@ async function backfillRepository(env: Env, repo: RepositoryRecord, limits: Back
     const normalizedPullRequests = await mapWithConcurrency(pullRequests, 16, async (pr) => upsertPullRequestFromGitHub(env, repo.fullName, pr, { seenOpenAt: startedAt }));
 
     const mergedFileWarningStart = warnings.length;
-    await hydrateMergedPullRequestFiles(env, repo.fullName, recentMerged, token, warnings, limits.detailConcurrency, admissionKey);
+    await hydrateMergedPullRequestFiles(env, repo.fullName, recentMerged, token, warnings, limits.detailConcurrency, mode, admissionKey);
 
     const detailTargets = normalizedPullRequests.slice(0, limits.pullRequestDetails);
     const detailWarningStart = warnings.length;
     await mapWithConcurrency(detailTargets, limits.detailConcurrency, async (pr) => {
-      await fetchAndStorePullRequestDetails(env, repo.fullName, pr, token, warnings, admissionKey);
+      const before = warnings.length;
+      await fetchAndStorePullRequestDetails(env, repo.fullName, pr, token, warnings, admissionKey, "backfill_open_pr_details");
+      // Persist the repo+PR+headSha snapshot marker (#audit-rate-headroom) so a later call through ANY
+      // cache-aware path (open-PR convergence, live review) can skip refetching this PR's files while its
+      // head is unchanged — without this write, fetchAndStorePullRequestDetails's cache check always misses
+      // for PRs only ever touched by this monolithic backfill path.
+      const syncedAt = nowIso();
+      const newWarnings = warnings.slice(before);
+      await upsertPullRequestDetailSyncState(env, {
+        repoFullName: repo.fullName,
+        pullNumber: pr.number,
+        status: newWarnings.length > 0 ? "partial" : "complete",
+        headSha: pr.headSha,
+        filesSyncedAt: syncedAt,
+        reviewsSyncedAt: syncedAt,
+        checksSyncedAt: syncedAt,
+        lastSyncedAt: syncedAt,
+        errorSummary: newWarnings.at(-1),
+      });
     });
     const fileWarnings = warnings.slice(mergedFileWarningStart).filter((warning) => /File sync failed/i.test(warning));
     const reviewWarnings = warnings.slice(detailWarningStart).filter((warning) => /Review sync failed/i.test(warning));
@@ -1923,17 +1975,25 @@ async function fetchAndStorePullRequestDetails(
   pr: PullRequestRecord,
   token: string | undefined,
   warnings: string[],
-  admissionKey?: GitHubRateLimitAdmissionKey,
+  admissionKey: GitHubRateLimitAdmissionKey | undefined,
+  caller: PullRequestFilesFetchCaller,
+  options: { forceFiles?: boolean | undefined } = {},
 ): Promise<void> {
+  // Durable repo+PR+headSha file snapshot (#audit-rate-headroom): a bare URL cache is insufficient because
+  // `/pulls/{n}/files` has the SAME url across different heads. Reuse the stored `pull_request_files` rows
+  // instead of refetching when the last successful files sync already covered the PR's CURRENT head SHA —
+  // only files are cached here; reviews/checks are more volatile at a fixed head and still refresh every call.
+  const existingState = !options.forceFiles && pr.headSha ? await getPullRequestDetailSyncState(env, repoFullName, pr.number) : null;
+  const filesUpToDate = Boolean(existingState?.headSha) && existingState?.headSha === pr.headSha && Boolean(existingState?.filesSyncedAt);
   const warningStart = warnings.length;
   const [files, reviews, checks] = await Promise.all([
-    fetchPullRequestFiles(env, repoFullName, pr.number, token, warnings, admissionKey),
+    filesUpToDate ? Promise.resolve<GitHubFilePayload[]>([]) : fetchPullRequestFiles(env, repoFullName, pr.number, token, warnings, admissionKey, caller),
     fetchPullRequestReviews(env, repoFullName, pr.number, token, warnings, admissionKey),
     fetchPullRequestChecks(env, repoFullName, pr, token, warnings, admissionKey),
   ]);
   const fileSyncFailed = warnings.slice(warningStart).some((warning) => warning.startsWith(`File sync failed for #${pr.number}:`));
 
-  if (!fileSyncFailed) {
+  if (!filesUpToDate && !fileSyncFailed) {
     await deletePullRequestFiles(env, repoFullName, pr.number);
     for (const file of files) {
       await upsertPullRequestFile(env, {
@@ -2009,8 +2069,10 @@ async function fetchPullRequestFiles(
   pullNumber: number,
   token: string | undefined,
   warnings: string[],
-  admissionKey?: GitHubRateLimitAdmissionKey,
+  admissionKey: GitHubRateLimitAdmissionKey | undefined,
+  caller: PullRequestFilesFetchCaller,
 ): Promise<GitHubFilePayload[]> {
+  incr(PULL_REQUEST_FILES_FETCH_METRIC, { caller });
   const files = await githubPaginatedList<GitHubFilePayload>(env, repoFullName, `/pulls/${pullNumber}/files`, token, admissionKey);
   if (files) return files;
   const fallback = token ? await fetchPullRequestDetailsFromGraphQl(env, repoFullName, pullNumber, token, admissionKey).catch(() => undefined) : undefined;
@@ -2055,7 +2117,7 @@ export async function fetchAndStorePullRequestFilesForReview(
   admissionKey?: GitHubRateLimitAdmissionKey,
 ): Promise<PullRequestFileRecord[]> {
   const warnings: string[] = [];
-  const files = await fetchPullRequestFiles(env, repoFullName, pullNumber, token, warnings, admissionKey).catch(() => [] as GitHubFilePayload[]);
+  const files = await fetchPullRequestFiles(env, repoFullName, pullNumber, token, warnings, admissionKey, "live_review").catch(() => [] as GitHubFilePayload[]);
   if (files.length === 0) return [];
   const records = files.map((file) => toPullRequestFileRecordFromGitHub(repoFullName, pullNumber, file));
   // Persist so the AI review, grounding, gate, check-run, and unified-comment reads in THIS run (and any later

src/github/rate-limit.ts

@@ -4,12 +4,16 @@ import { listLatestGitHubRateLimitObservations } from "../db/repositories";
 // from draining the budget real webhook traffic needs, maintenance yields while there is still headroom:
 //   - backfill yields at LOW_REST_RATE_LIMIT_REMAINING;
 //   - the re-gate sweep + its per-PR jobs yield EARLIER, at MAINTENANCE_RESERVED_HEADROOM, reserving the budget
-//     between the two floors for webhooks.
+//     between the two floors for webhooks;
+//   - historical/scheduled hydration that isn't needed for any CURRENT PR (e.g. backfilling file lists for old
+//     merged pull requests) yields EARLIEST, at HISTORICAL_BACKFILL_RESERVED_HEADROOM — it is the least urgent
+//     GitHub REST consumer, so it must never be the reason a live review or an open-PR convergence pass stalls.
 // Self-host queues also use the latest persisted observation for admission control, so a known-exhausted bucket
 // delays webhook jobs before they start and avoids burning the first live delivery just to discover the limit.
 // (#audit-rate-headroom)
 export const LOW_REST_RATE_LIMIT_REMAINING = 75;
 export const MAINTENANCE_RESERVED_HEADROOM = 150;
+export const HISTORICAL_BACKFILL_RESERVED_HEADROOM = 300;
 
 /** The REST rate-limit reset time to wait until when the latest recorded REST budget is at/below `floor`, or
  *  undefined when there is headroom, no usable observation, or the reset is already in the past. Reads the latest

src/github/webhook-coalesce.ts

@@ -1,7 +1,11 @@
 import type { GitHubWebhookPayload } from "../types";
 
+// Kept in sync with PR_PUBLIC_SURFACE_ACTIONS (src/queue/processors.ts) minus "closed" (merge/close has its own
+// non-coalesced handling): every action that can trigger a file refresh for the SAME PR+head should collapse a
+// burst into one job, not one job per delivery (#audit-rate-headroom).
 const COALESCABLE_PULL_REQUEST_ACTIONS = new Set([
   "opened",
+  "reopened",
   "synchronize",
   "edited",
   "ready_for_review",