From 99996f9c70edd9b8eb60547d6c6fe9531fe7aa51 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Tue, 25 Aug 2026 14:44:17 -0400 Subject: [PATCH 1/2] fix(dashboard): preserve last-known PR enrichment when backfill fails Poll cycles fully replaced the PR list each cycle, so a transient enrichment-backfill failure (network blocker, blip, or GitHub hiccup) wiped size/check-status data and reclassified dependency PRs from Mergeable to Needs Action until the next successful poll. Add fallbackToPreviousEnrichment to carry forward a PR's prior enriched fields when the current cycle failed to re-enrich it, and guard the fine-grained merge path against the same regression. Add warn + Sentry telemetry to the backfill catch block to match its sibling fetchers. --- .../components/dashboard/DashboardPage.tsx | 35 ++++++++++-------- src/app/services/api.ts | 36 +++++++++++++++++++ 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index b070c1f8..7dc5f7a4 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -14,7 +14,7 @@ import DependenciesTab from "./DependenciesTab"; import { isDependencyPr, expandBotLogins, needsBodyFallback, parseRenovateBody, type VersionInfo } from "../../lib/dependency-detection"; import { isRepoExcludedFromDependencies } from "../../lib/dependency-exclusion"; import { findDashboardIssues, parseAbandonedSection, resetAbandonedPatternCache, type AbandonedDependency } from "../../lib/dependency-dashboard"; -import { fetchDashboardIssueBodies, fetchDepPRBodies } from "../../services/api"; +import { fetchDashboardIssueBodies, fetchDepPRBodies, fallbackToPreviousEnrichment } from "../../services/api"; import type { SortOption } from "../shared/SortDropdown"; import type { Issue, PullRequest, WorkflowRun } from "../../services/api"; import { fetchOrgs } from "../../services/api"; @@ -285,23 +285,29 @@ async function pollFetch(): Promise { for (let i = 0; i < state.pullRequests.length; i++) { const e = enrichedMap.get(state.pullRequests[i].id)!; const pr = state.pullRequests[i]; - pr.headSha = e.headSha; - pr.assigneeLogins = e.assigneeLogins; - pr.reviewerLogins = e.reviewerLogins; - pr.checkStatus = e.checkStatus; - pr.additions = e.additions; - pr.deletions = e.deletions; - pr.changedFiles = e.changedFiles; - pr.comments = e.comments; - pr.reviewThreads = e.reviewThreads; - pr.totalReviewCount = e.totalReviewCount; - pr.enriched = e.enriched; + // A failed backfill batch returns e.enriched === false for PRs it + // couldn't reach — don't let that regress a PR that was already + // enriched from a prior cycle. + const regressing = e.enriched === false && pr.enriched !== false; + if (!regressing) { + pr.headSha = e.headSha; + pr.assigneeLogins = e.assigneeLogins; + pr.reviewerLogins = e.reviewerLogins; + pr.checkStatus = e.checkStatus; + pr.additions = e.additions; + pr.deletions = e.deletions; + pr.changedFiles = e.changedFiles; + pr.comments = e.comments; + pr.reviewThreads = e.reviewThreads; + pr.totalReviewCount = e.totalReviewCount; + pr.enriched = e.enriched; + } pr.nodeId = e.nodeId; pr.surfacedBy = e.surfacedBy; pr.starCount = e.starCount; } } else { - state.pullRequests = data.pullRequests; + state.pullRequests = fallbackToPreviousEnrichment(state.pullRequests, data.pullRequests); } })); } else { @@ -310,10 +316,11 @@ async function pollFetch(): Promise { // changed since the last cycle. Preserve scroll position: SolidJS // DOM updates are synchronous within the setter, so save/restore // around it to prevent scroll reset from DOM rebuild. + const pullRequests = fallbackToPreviousEnrichment(dashboardData.pullRequests, data.pullRequests); withScrollLock(() => { setDashboardData({ issues: data.issues, - pullRequests: data.pullRequests, + pullRequests, workflowRuns: config.enableActions ? data.workflowRuns : [], loading: false, lastRefreshedAt: now, diff --git a/src/app/services/api.ts b/src/app/services/api.ts index 43d086b4..846931e4 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -1151,6 +1151,8 @@ export async function fetchPREnrichment( updateGraphqlRateLimit(partialErr.rateLimit); } const { statusCode, message } = extractRejectionError(err); + console.warn(`[api] PR enrichment batch ${batchIdx + 1}/${batches.length} failed:`, err); + Sentry.captureException(err, { tags: { source: "prEnrichment" } }); errors.push({ repo: `backfill-batch-${batchIdx + 1}/${batches.length}`, statusCode, message, @@ -1361,6 +1363,40 @@ function mergeEnrichment( }); } +/** + * Carries forward a PR's last-known enrichment when this cycle's backfill + * failed for it, instead of regressing an already-enriched PR to unenriched. + * Without this, a single transient backfill failure wipes size/check-status + * data and can flip dependency-status classification (e.g. Mergeable -> + * Needs Action) until the next successful poll re-enriches it. + */ +export function fallbackToPreviousEnrichment( + previous: PullRequest[], + next: PullRequest[] +): PullRequest[] { + if (previous.length === 0) return next; + const previousMap = new Map(previous.map((pr) => [pr.id, pr])); + return next.map((pr) => { + if (pr.enriched !== false) return pr; + const prev = previousMap.get(pr.id); + if (!prev || prev.enriched === false) return pr; + return { + ...pr, + headSha: prev.headSha, + assigneeLogins: prev.assigneeLogins, + reviewerLogins: prev.reviewerLogins, + checkStatus: prev.checkStatus, + additions: prev.additions, + deletions: prev.deletions, + changedFiles: prev.changedFiles, + comments: prev.comments, + reviewThreads: prev.reviewThreads, + totalReviewCount: prev.totalReviewCount, + enriched: true, + }; + }); +} + /** * Merges tracked user search results into the main issue/PR maps. * Items already present get the tracked user's login appended to surfacedBy. From f901a009eaaffe75539331a4c13ccef41fb1e8d9 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 26 Aug 2026 10:59:16 -0400 Subject: [PATCH 2/2] refactor(api): dedups PR enrichment field copy into a shared helper Extracts pickEnrichmentFields so mergeEnrichment, fallbackToPreviousEnrichment, and the DashboardPage fine-grained merge share one field list, removing the drift risk that a future enrichment field is added to some copy sites but not others. Adds unit tests covering fallbackToPreviousEnrichment (empty-previous, already-enriched passthrough, missing/unenriched prev, carry-forward, closed-PR drop) and pickEnrichmentFields field selection. --- .../components/dashboard/DashboardPage.tsx | 13 +- src/app/services/api.ts | 58 +++++--- tests/services/api-optimization.test.ts | 130 ++++++++++++++++++ 3 files changed, 170 insertions(+), 31 deletions(-) diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index 7dc5f7a4..6c35dad9 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -14,7 +14,7 @@ import DependenciesTab from "./DependenciesTab"; import { isDependencyPr, expandBotLogins, needsBodyFallback, parseRenovateBody, type VersionInfo } from "../../lib/dependency-detection"; import { isRepoExcludedFromDependencies } from "../../lib/dependency-exclusion"; import { findDashboardIssues, parseAbandonedSection, resetAbandonedPatternCache, type AbandonedDependency } from "../../lib/dependency-dashboard"; -import { fetchDashboardIssueBodies, fetchDepPRBodies, fallbackToPreviousEnrichment } from "../../services/api"; +import { fetchDashboardIssueBodies, fetchDepPRBodies, fallbackToPreviousEnrichment, pickEnrichmentFields } from "../../services/api"; import type { SortOption } from "../shared/SortDropdown"; import type { Issue, PullRequest, WorkflowRun } from "../../services/api"; import { fetchOrgs } from "../../services/api"; @@ -290,16 +290,7 @@ async function pollFetch(): Promise { // enriched from a prior cycle. const regressing = e.enriched === false && pr.enriched !== false; if (!regressing) { - pr.headSha = e.headSha; - pr.assigneeLogins = e.assigneeLogins; - pr.reviewerLogins = e.reviewerLogins; - pr.checkStatus = e.checkStatus; - pr.additions = e.additions; - pr.deletions = e.deletions; - pr.changedFiles = e.changedFiles; - pr.comments = e.comments; - pr.reviewThreads = e.reviewThreads; - pr.totalReviewCount = e.totalReviewCount; + Object.assign(pr, pickEnrichmentFields(e)); pr.enriched = e.enriched; } pr.nodeId = e.nodeId; diff --git a/src/app/services/api.ts b/src/app/services/api.ts index 846931e4..b6def37b 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -1329,6 +1329,42 @@ export async function fetchDepPRBodies( ); } +/** + * The heavy PR fields populated by phase-2 enrichment. Centralized so the sites + * that copy enrichment onto a PR — mergeEnrichment, fallbackToPreviousEnrichment, + * and the fine-grained store merge in DashboardPage — stay in sync when a field + * is added or removed. + */ +type EnrichmentFields = Pick< + PullRequest, + | "headSha" + | "assigneeLogins" + | "reviewerLogins" + | "checkStatus" + | "additions" + | "deletions" + | "changedFiles" + | "comments" + | "reviewThreads" + | "totalReviewCount" +>; + +/** Extracts just the heavy enrichment fields from any PR-shaped source. */ +export function pickEnrichmentFields(source: EnrichmentFields): EnrichmentFields { + return { + headSha: source.headSha, + assigneeLogins: source.assigneeLogins, + reviewerLogins: source.reviewerLogins, + checkStatus: source.checkStatus, + additions: source.additions, + deletions: source.deletions, + changedFiles: source.changedFiles, + comments: source.comments, + reviewThreads: source.reviewThreads, + totalReviewCount: source.totalReviewCount, + }; +} + /** * Merges phase 2 enrichment data into light PRs. Returns enriched PR array. * Also detects fork PRs for the statusCheckRollup fallback. @@ -1348,16 +1384,7 @@ function mergeEnrichment( return { ...pr, - headSha: e.headSha, - assigneeLogins: e.assigneeLogins, - reviewerLogins: e.reviewerLogins, - checkStatus: e.checkStatus, - additions: e.additions, - deletions: e.deletions, - changedFiles: e.changedFiles, - comments: e.comments, - reviewThreads: e.reviewThreads, - totalReviewCount: e.totalReviewCount, + ...pickEnrichmentFields(e), enriched: true, }; }); @@ -1382,16 +1409,7 @@ export function fallbackToPreviousEnrichment( if (!prev || prev.enriched === false) return pr; return { ...pr, - headSha: prev.headSha, - assigneeLogins: prev.assigneeLogins, - reviewerLogins: prev.reviewerLogins, - checkStatus: prev.checkStatus, - additions: prev.additions, - deletions: prev.deletions, - changedFiles: prev.changedFiles, - comments: prev.comments, - reviewThreads: prev.reviewThreads, - totalReviewCount: prev.totalReviewCount, + ...pickEnrichmentFields(prev), enriched: true, }; }); diff --git a/tests/services/api-optimization.test.ts b/tests/services/api-optimization.test.ts index 87cdf61a..674c39b2 100644 --- a/tests/services/api-optimization.test.ts +++ b/tests/services/api-optimization.test.ts @@ -4,9 +4,12 @@ import { fetchIssuesAndPullRequests, fetchWorkflowRuns, fetchPREnrichment, + fallbackToPreviousEnrichment, + pickEnrichmentFields, type RepoRef, } from "../../src/app/services/api"; import { clearCache } from "../../src/app/stores/cache"; +import { makePullRequest } from "../helpers/factories"; vi.mock("../../src/app/lib/errors", () => ({ pushNotification: vi.fn(), @@ -597,3 +600,130 @@ describe("fetchPREnrichment mergeStateStatus UNSTABLE override", () => { expect(enrichments.get(100)!.checkStatus).toBe("failure"); }); }); + +// ── Enrichment carry-forward on backfill failure ────────────────────────────── + +describe("fallbackToPreviousEnrichment", () => { + it("returns next unchanged when previous is empty", () => { + const next = [makePullRequest({ id: 1, enriched: false })]; + const result = fallbackToPreviousEnrichment([], next); + expect(result).toBe(next); + }); + + it("passes through a PR that is already enriched in next (no carry-forward)", () => { + const previous = [makePullRequest({ id: 1, enriched: true, checkStatus: "failure" })]; + const next = [makePullRequest({ id: 1, enriched: true, checkStatus: "success" })]; + const result = fallbackToPreviousEnrichment(previous, next); + // Fresh enriched data wins — prior "failure" must not overwrite fresh "success". + expect(result[0].checkStatus).toBe("success"); + expect(result[0]).toBe(next[0]); + }); + + it("passes through an unenriched next PR with no matching previous entry", () => { + const previous = [makePullRequest({ id: 99, enriched: true })]; + const next = [makePullRequest({ id: 1, enriched: false })]; + const result = fallbackToPreviousEnrichment(previous, next); + expect(result[0]).toBe(next[0]); + expect(result[0].enriched).toBe(false); + }); + + it("passes through an unenriched next PR whose previous entry was also unenriched", () => { + const previous = [makePullRequest({ id: 1, enriched: false })]; + const next = [makePullRequest({ id: 1, enriched: false })]; + const result = fallbackToPreviousEnrichment(previous, next); + expect(result[0]).toBe(next[0]); + expect(result[0].enriched).toBe(false); + }); + + it("carries forward prior enrichment for an unenriched next PR that was enriched before", () => { + const previous = [ + makePullRequest({ + id: 1, + enriched: true, + checkStatus: "success", + additions: 42, + deletions: 7, + changedFiles: 3, + comments: 5, + reviewThreads: 2, + totalReviewCount: 4, + reviewerLogins: ["reviewer1"], + assigneeLogins: ["assignee1"], + headSha: "prevsha", + }), + ]; + const next = [ + makePullRequest({ + id: 1, + enriched: false, + state: "OPEN", + title: "Fresh title", + checkStatus: null, + additions: 0, + deletions: 0, + changedFiles: 0, + comments: 0, + reviewThreads: 0, + totalReviewCount: 0, + reviewerLogins: [], + assigneeLogins: [], + headSha: "", + }), + ]; + const result = fallbackToPreviousEnrichment(previous, next); + // Heavy fields restored from the prior cycle... + expect(result[0].enriched).toBe(true); + expect(result[0].checkStatus).toBe("success"); + expect(result[0].additions).toBe(42); + expect(result[0].deletions).toBe(7); + expect(result[0].changedFiles).toBe(3); + expect(result[0].comments).toBe(5); + expect(result[0].reviewThreads).toBe(2); + expect(result[0].totalReviewCount).toBe(4); + expect(result[0].reviewerLogins).toEqual(["reviewer1"]); + expect(result[0].assigneeLogins).toEqual(["assignee1"]); + expect(result[0].headSha).toBe("prevsha"); + // ...but fresh light fields survive. + expect(result[0].title).toBe("Fresh title"); + }); + + it("drops a PR that closed (present in previous, absent from next)", () => { + const previous = [ + makePullRequest({ id: 1, enriched: true }), + makePullRequest({ id: 2, enriched: true }), + ]; + const next = [makePullRequest({ id: 1, enriched: false })]; + const result = fallbackToPreviousEnrichment(previous, next); + // Only the PR still in `next` remains — the closed PR (id 2) is not resurrected. + expect(result.map((pr) => pr.id)).toEqual([1]); + }); +}); + +describe("pickEnrichmentFields", () => { + it("returns only the heavy enrichment fields, not light fields", () => { + const pr = makePullRequest({ + id: 1, + title: "light title", + state: "OPEN", + checkStatus: "success", + additions: 10, + reviewerLogins: ["r1"], + }); + const fields = pickEnrichmentFields(pr); + expect(fields).toEqual({ + headSha: pr.headSha, + assigneeLogins: pr.assigneeLogins, + reviewerLogins: pr.reviewerLogins, + checkStatus: pr.checkStatus, + additions: pr.additions, + deletions: pr.deletions, + changedFiles: pr.changedFiles, + comments: pr.comments, + reviewThreads: pr.reviewThreads, + totalReviewCount: pr.totalReviewCount, + }); + expect(fields).not.toHaveProperty("title"); + expect(fields).not.toHaveProperty("state"); + expect(fields).not.toHaveProperty("enriched"); + }); +});