From 5658797a5c5c55d476561a219cbde2f48ea5657c Mon Sep 17 00:00:00 2001 From: = <=> Date: Mon, 29 Jun 2026 11:28:55 -0700 Subject: [PATCH 1/8] fix(queue-trends): return null review velocity when observedDays is zero --- src/services/queue-trends.ts | 10 ++++++++-- test/unit/queue-trends.test.ts | 7 ++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/services/queue-trends.ts b/src/services/queue-trends.ts index de77f2cbc0..dd97895e9c 100644 --- a/src/services/queue-trends.ts +++ b/src/services/queue-trends.ts @@ -96,7 +96,7 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe const stalePullRequestRate = latestQueue ? staleRate(latestQueue) : null; const baselineStaleRate = baselineQueue ? staleRate(baselineQueue) : null; const duplicateTrend = latestQueue && baselineQueue ? latestQueue.collisionClusters - baselineQueue.collisionClusters : null; - const reviewVelocityPerDay = round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); + const reviewVelocityPerDay = computeReviewVelocityPerDay(mergedPullRequests, closedUnmergedPullRequests, observedDays); const pullRequestGrowth = latest.openPullRequestsTotal - baseline.openPullRequestsTotal; return { windowDays, @@ -112,10 +112,16 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe stalePullRequestRate, stalePullRequestRateDelta: stalePullRequestRate !== null && baselineStaleRate !== null ? round(stalePullRequestRate - baselineStaleRate) : null, duplicateTrend, - summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${reviewVelocityPerDay}/day.`, + summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${reviewVelocityPerDay === null ? "n/a" : `${reviewVelocityPerDay}/day`}.`, }; } +/** Returns null when the observation window spans zero days (avoids divide-by-zero / Infinity). */ +export function computeReviewVelocityPerDay(mergedPullRequests: number, closedUnmergedPullRequests: number, observedDays: number): number | null { + if (observedDays === 0) return null; + return round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); +} + function trendWarnings(windows: QueueTrendWindow[]): string[] { const warnings: string[] = []; for (const window of windows.filter((entry) => entry.status === "ready")) { diff --git a/test/unit/queue-trends.test.ts b/test/unit/queue-trends.test.ts index 22c42e68f3..23df385c1c 100644 --- a/test/unit/queue-trends.test.ts +++ b/test/unit/queue-trends.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { getRepoQueueTrendSnapshot, persistRepoGithubTotalsSnapshot, persistSignalSnapshot, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; import { generateSignalSnapshots } from "../../src/queue/processors"; -import { buildQueueTrendReport, buildUnavailableQueueTrendReport, type QueueTrendReport } from "../../src/services/queue-trends"; +import { buildQueueTrendReport, buildUnavailableQueueTrendReport, computeReviewVelocityPerDay, type QueueTrendReport } from "../../src/services/queue-trends"; import type { RepoGithubTotalsSnapshotRecord } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; @@ -55,6 +55,11 @@ describe("queue trend windows", () => { expect(report.warnings).toEqual(expect.arrayContaining([expect.stringContaining("stale PR rate"), expect.stringContaining("duplicate cluster")])); }); + it("returns null review velocity when observedDays is zero", () => { + expect(computeReviewVelocityPerDay(7, 3, 0)).toBeNull(); + expect(computeReviewVelocityPerDay(7, 3, 7)).toBe(1.43); + }); + it("returns clear unavailable windows when history is missing", () => { const report = buildQueueTrendReport({ repoFullName: "owner/repo", totalsSnapshots: [totals(0, { openIssues: 1, openPrs: 1, merged: 0, closed: 0 })] }); expect(report).toMatchObject({ From 45d9c0cdca6cbfd6a77549fd122e1be8e0fb02e5 Mon Sep 17 00:00:00 2001 From: = <=> Date: Mon, 29 Jun 2026 11:44:17 -0700 Subject: [PATCH 2/8] fix(queue-trends): guard review velocity division and test via buildQueueTrendReport --- src/services/queue-trends.ts | 10 ++-------- test/unit/queue-trends.test.ts | 29 +++++++++++++++++++++++++---- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/services/queue-trends.ts b/src/services/queue-trends.ts index dd97895e9c..7fb798242b 100644 --- a/src/services/queue-trends.ts +++ b/src/services/queue-trends.ts @@ -96,7 +96,7 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe const stalePullRequestRate = latestQueue ? staleRate(latestQueue) : null; const baselineStaleRate = baselineQueue ? staleRate(baselineQueue) : null; const duplicateTrend = latestQueue && baselineQueue ? latestQueue.collisionClusters - baselineQueue.collisionClusters : null; - const reviewVelocityPerDay = computeReviewVelocityPerDay(mergedPullRequests, closedUnmergedPullRequests, observedDays); + const reviewVelocityPerDay = round((mergedPullRequests + closedUnmergedPullRequests) / Math.max(observedDays, 1)); const pullRequestGrowth = latest.openPullRequestsTotal - baseline.openPullRequestsTotal; return { windowDays, @@ -112,16 +112,10 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe stalePullRequestRate, stalePullRequestRateDelta: stalePullRequestRate !== null && baselineStaleRate !== null ? round(stalePullRequestRate - baselineStaleRate) : null, duplicateTrend, - summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${reviewVelocityPerDay === null ? "n/a" : `${reviewVelocityPerDay}/day`}.`, + summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${reviewVelocityPerDay}/day.`, }; } -/** Returns null when the observation window spans zero days (avoids divide-by-zero / Infinity). */ -export function computeReviewVelocityPerDay(mergedPullRequests: number, closedUnmergedPullRequests: number, observedDays: number): number | null { - if (observedDays === 0) return null; - return round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); -} - function trendWarnings(windows: QueueTrendWindow[]): string[] { const warnings: string[] = []; for (const window of windows.filter((entry) => entry.status === "ready")) { diff --git a/test/unit/queue-trends.test.ts b/test/unit/queue-trends.test.ts index 23df385c1c..080b0d55fd 100644 --- a/test/unit/queue-trends.test.ts +++ b/test/unit/queue-trends.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { getRepoQueueTrendSnapshot, persistRepoGithubTotalsSnapshot, persistSignalSnapshot, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; import { generateSignalSnapshots } from "../../src/queue/processors"; -import { buildQueueTrendReport, buildUnavailableQueueTrendReport, computeReviewVelocityPerDay, type QueueTrendReport } from "../../src/services/queue-trends"; +import { buildQueueTrendReport, buildUnavailableQueueTrendReport, type QueueTrendReport } from "../../src/services/queue-trends"; import type { RepoGithubTotalsSnapshotRecord } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; @@ -55,9 +55,30 @@ describe("queue trend windows", () => { expect(report.warnings).toEqual(expect.arrayContaining([expect.stringContaining("stale PR rate"), expect.stringContaining("duplicate cluster")])); }); - it("returns null review velocity when observedDays is zero", () => { - expect(computeReviewVelocityPerDay(7, 3, 0)).toBeNull(); - expect(computeReviewVelocityPerDay(7, 3, 7)).toBe(1.43); + it("does not emit Infinity review velocity when latest totals snapshots share fetchedAt", () => { + const sharedAt = atDaysAgo(0); + const report = buildQueueTrendReport({ + repoFullName: "owner/repo", + totalsSnapshots: [ + totals(7, { openIssues: 10, openPrs: 5, merged: 10, closed: 4 }), + { ...totals(0, { openIssues: 8, openPrs: 3, merged: 11, closed: 4 }), id: "totals-dup-a", fetchedAt: sharedAt }, + { ...totals(0, { openIssues: 8, openPrs: 3, merged: 17, closed: 7 }), id: "totals-dup-b", fetchedAt: sharedAt }, + ], + }); + + expect(report.status).toBe("ready"); + for (const window of report.windows.filter((entry) => entry.status === "ready")) { + expect(window.reviewVelocityPerDay).not.toBe(Infinity); + expect(window.summary).not.toContain("Infinity"); + expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); + } + expect(report.windows[0]).toMatchObject({ + windowDays: 7, + mergedPullRequests: 7, + closedUnmergedPullRequests: 3, + reviewVelocityPerDay: 1.43, + summary: expect.stringContaining("review velocity 1.43/day"), + }); }); it("returns clear unavailable windows when history is missing", () => { From 8028431d864bf85029151d2a424992aa8c7c3c92 Mon Sep 17 00:00:00 2001 From: = <=> Date: Mon, 29 Jun 2026 15:08:37 -0700 Subject: [PATCH 3/8] retest --- src/services/queue-trends.ts | 10 ++++++++-- test/unit/queue-trends.test.ts | 11 +++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/services/queue-trends.ts b/src/services/queue-trends.ts index 7fb798242b..7d4b60affd 100644 --- a/src/services/queue-trends.ts +++ b/src/services/queue-trends.ts @@ -96,7 +96,7 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe const stalePullRequestRate = latestQueue ? staleRate(latestQueue) : null; const baselineStaleRate = baselineQueue ? staleRate(baselineQueue) : null; const duplicateTrend = latestQueue && baselineQueue ? latestQueue.collisionClusters - baselineQueue.collisionClusters : null; - const reviewVelocityPerDay = round((mergedPullRequests + closedUnmergedPullRequests) / Math.max(observedDays, 1)); + const reviewVelocityPerDay = computeReviewVelocityPerDay(mergedPullRequests, closedUnmergedPullRequests, observedDays); const pullRequestGrowth = latest.openPullRequestsTotal - baseline.openPullRequestsTotal; return { windowDays, @@ -112,10 +112,16 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe stalePullRequestRate, stalePullRequestRateDelta: stalePullRequestRate !== null && baselineStaleRate !== null ? round(stalePullRequestRate - baselineStaleRate) : null, duplicateTrend, - summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${reviewVelocityPerDay}/day.`, + summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${reviewVelocityPerDay === null ? "n/a" : `${reviewVelocityPerDay}/day`}.`, }; } +/** Returns null when the observation window spans zero days (avoids divide-by-zero / fabricated velocity). */ +export function computeReviewVelocityPerDay(mergedPullRequests: number, closedUnmergedPullRequests: number, observedDays: number): number | null { + if (observedDays === 0) return null; + return round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); +} + function trendWarnings(windows: QueueTrendWindow[]): string[] { const warnings: string[] = []; for (const window of windows.filter((entry) => entry.status === "ready")) { diff --git a/test/unit/queue-trends.test.ts b/test/unit/queue-trends.test.ts index 080b0d55fd..6f617b56fc 100644 --- a/test/unit/queue-trends.test.ts +++ b/test/unit/queue-trends.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { getRepoQueueTrendSnapshot, persistRepoGithubTotalsSnapshot, persistSignalSnapshot, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; import { generateSignalSnapshots } from "../../src/queue/processors"; -import { buildQueueTrendReport, buildUnavailableQueueTrendReport, type QueueTrendReport } from "../../src/services/queue-trends"; +import { buildQueueTrendReport, buildUnavailableQueueTrendReport, computeReviewVelocityPerDay, type QueueTrendReport } from "../../src/services/queue-trends"; import type { RepoGithubTotalsSnapshotRecord } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; @@ -70,7 +70,9 @@ describe("queue trend windows", () => { for (const window of report.windows.filter((entry) => entry.status === "ready")) { expect(window.reviewVelocityPerDay).not.toBe(Infinity); expect(window.summary).not.toContain("Infinity"); - expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); + if (window.reviewVelocityPerDay !== null) { + expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); + } } expect(report.windows[0]).toMatchObject({ windowDays: 7, @@ -81,6 +83,11 @@ describe("queue trend windows", () => { }); }); + it("returns null review velocity when observedDays is zero", () => { + expect(computeReviewVelocityPerDay(7, 3, 0)).toBeNull(); + expect(computeReviewVelocityPerDay(7, 3, 7)).toBe(1.43); + }); + it("returns clear unavailable windows when history is missing", () => { const report = buildQueueTrendReport({ repoFullName: "owner/repo", totalsSnapshots: [totals(0, { openIssues: 1, openPrs: 1, merged: 0, closed: 0 })] }); expect(report).toMatchObject({ From 70637dfbf39d4e47977df5f74cdc291bcce77101 Mon Sep 17 00:00:00 2001 From: = <=> Date: Mon, 29 Jun 2026 16:02:45 -0700 Subject: [PATCH 4/8] retest --- src/services/queue-trends.ts | 13 ++++--------- test/unit/queue-trends.test.ts | 11 ++--------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/src/services/queue-trends.ts b/src/services/queue-trends.ts index 7d4b60affd..3275277229 100644 --- a/src/services/queue-trends.ts +++ b/src/services/queue-trends.ts @@ -88,7 +88,8 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe const targetMs = latestMs - windowDays * 24 * 60 * 60 * 1000; const baseline = [...totals].reverse().find((snapshot) => Date.parse(snapshot.fetchedAt) <= targetMs); if (!baseline) return unavailableWindow(windowDays, `Need at least ${windowDays} days of totals history.`); - const observedDays = Math.max(0, round((latestMs - Date.parse(baseline.fetchedAt)) / (24 * 60 * 60 * 1000))); + // Baseline is the newest snapshot at or before (latest - windowDays), so observedDays is always >= windowDays. + const observedDays = round((latestMs - Date.parse(baseline.fetchedAt)) / (24 * 60 * 60 * 1000)); const mergedPullRequests = Math.max(0, latest.mergedPullRequestsTotal - baseline.mergedPullRequestsTotal); const closedUnmergedPullRequests = Math.max(0, latest.closedUnmergedPullRequestsTotal - baseline.closedUnmergedPullRequestsTotal); const latestQueue = latestQueuePoint(queuePoints); @@ -96,7 +97,7 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe const stalePullRequestRate = latestQueue ? staleRate(latestQueue) : null; const baselineStaleRate = baselineQueue ? staleRate(baselineQueue) : null; const duplicateTrend = latestQueue && baselineQueue ? latestQueue.collisionClusters - baselineQueue.collisionClusters : null; - const reviewVelocityPerDay = computeReviewVelocityPerDay(mergedPullRequests, closedUnmergedPullRequests, observedDays); + const reviewVelocityPerDay = round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); const pullRequestGrowth = latest.openPullRequestsTotal - baseline.openPullRequestsTotal; return { windowDays, @@ -112,16 +113,10 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe stalePullRequestRate, stalePullRequestRateDelta: stalePullRequestRate !== null && baselineStaleRate !== null ? round(stalePullRequestRate - baselineStaleRate) : null, duplicateTrend, - summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${reviewVelocityPerDay === null ? "n/a" : `${reviewVelocityPerDay}/day`}.`, + summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${reviewVelocityPerDay}/day.`, }; } -/** Returns null when the observation window spans zero days (avoids divide-by-zero / fabricated velocity). */ -export function computeReviewVelocityPerDay(mergedPullRequests: number, closedUnmergedPullRequests: number, observedDays: number): number | null { - if (observedDays === 0) return null; - return round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); -} - function trendWarnings(windows: QueueTrendWindow[]): string[] { const warnings: string[] = []; for (const window of windows.filter((entry) => entry.status === "ready")) { diff --git a/test/unit/queue-trends.test.ts b/test/unit/queue-trends.test.ts index 6f617b56fc..080b0d55fd 100644 --- a/test/unit/queue-trends.test.ts +++ b/test/unit/queue-trends.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { getRepoQueueTrendSnapshot, persistRepoGithubTotalsSnapshot, persistSignalSnapshot, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; import { generateSignalSnapshots } from "../../src/queue/processors"; -import { buildQueueTrendReport, buildUnavailableQueueTrendReport, computeReviewVelocityPerDay, type QueueTrendReport } from "../../src/services/queue-trends"; +import { buildQueueTrendReport, buildUnavailableQueueTrendReport, type QueueTrendReport } from "../../src/services/queue-trends"; import type { RepoGithubTotalsSnapshotRecord } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; @@ -70,9 +70,7 @@ describe("queue trend windows", () => { for (const window of report.windows.filter((entry) => entry.status === "ready")) { expect(window.reviewVelocityPerDay).not.toBe(Infinity); expect(window.summary).not.toContain("Infinity"); - if (window.reviewVelocityPerDay !== null) { - expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); - } + expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); } expect(report.windows[0]).toMatchObject({ windowDays: 7, @@ -83,11 +81,6 @@ describe("queue trend windows", () => { }); }); - it("returns null review velocity when observedDays is zero", () => { - expect(computeReviewVelocityPerDay(7, 3, 0)).toBeNull(); - expect(computeReviewVelocityPerDay(7, 3, 7)).toBe(1.43); - }); - it("returns clear unavailable windows when history is missing", () => { const report = buildQueueTrendReport({ repoFullName: "owner/repo", totalsSnapshots: [totals(0, { openIssues: 1, openPrs: 1, merged: 0, closed: 0 })] }); expect(report).toMatchObject({ From 3650c8e3e9911330764548c92bb4c3d6db5dbf34 Mon Sep 17 00:00:00 2001 From: andriypolandki <=> Date: Mon, 29 Jun 2026 16:58:25 -0700 Subject: [PATCH 5/8] retest --- src/services/queue-trends.ts | 12 ++++++++++-- test/unit/queue-trends.test.ts | 7 ++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/services/queue-trends.ts b/src/services/queue-trends.ts index 3275277229..cc2df8f187 100644 --- a/src/services/queue-trends.ts +++ b/src/services/queue-trends.ts @@ -88,16 +88,18 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe const targetMs = latestMs - windowDays * 24 * 60 * 60 * 1000; const baseline = [...totals].reverse().find((snapshot) => Date.parse(snapshot.fetchedAt) <= targetMs); if (!baseline) return unavailableWindow(windowDays, `Need at least ${windowDays} days of totals history.`); - // Baseline is the newest snapshot at or before (latest - windowDays), so observedDays is always >= windowDays. const observedDays = round((latestMs - Date.parse(baseline.fetchedAt)) / (24 * 60 * 60 * 1000)); const mergedPullRequests = Math.max(0, latest.mergedPullRequestsTotal - baseline.mergedPullRequestsTotal); const closedUnmergedPullRequests = Math.max(0, latest.closedUnmergedPullRequestsTotal - baseline.closedUnmergedPullRequestsTotal); + const reviewVelocityPerDay = computeReviewVelocityPerDay(mergedPullRequests, closedUnmergedPullRequests, observedDays); + if (reviewVelocityPerDay === null) { + return unavailableWindow(windowDays, "Totals snapshots lack spacing for a reliable trend window."); + } const latestQueue = latestQueuePoint(queuePoints); const baselineQueue = latestQueue ? baselineQueuePoint(queuePoints, latestQueue.generatedAt, windowDays) : null; const stalePullRequestRate = latestQueue ? staleRate(latestQueue) : null; const baselineStaleRate = baselineQueue ? staleRate(baselineQueue) : null; const duplicateTrend = latestQueue && baselineQueue ? latestQueue.collisionClusters - baselineQueue.collisionClusters : null; - const reviewVelocityPerDay = round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); const pullRequestGrowth = latest.openPullRequestsTotal - baseline.openPullRequestsTotal; return { windowDays, @@ -117,6 +119,12 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe }; } +/** Null when observedDays <= 0 — callers must not divide; ready windows only proceed with a finite rate. */ +export function computeReviewVelocityPerDay(mergedPullRequests: number, closedUnmergedPullRequests: number, observedDays: number): number | null { + if (observedDays <= 0) return null; + return round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); +} + function trendWarnings(windows: QueueTrendWindow[]): string[] { const warnings: string[] = []; for (const window of windows.filter((entry) => entry.status === "ready")) { diff --git a/test/unit/queue-trends.test.ts b/test/unit/queue-trends.test.ts index 080b0d55fd..b2ef620529 100644 --- a/test/unit/queue-trends.test.ts +++ b/test/unit/queue-trends.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { getRepoQueueTrendSnapshot, persistRepoGithubTotalsSnapshot, persistSignalSnapshot, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; import { generateSignalSnapshots } from "../../src/queue/processors"; -import { buildQueueTrendReport, buildUnavailableQueueTrendReport, type QueueTrendReport } from "../../src/services/queue-trends"; +import { buildQueueTrendReport, buildUnavailableQueueTrendReport, computeReviewVelocityPerDay, type QueueTrendReport } from "../../src/services/queue-trends"; import type { RepoGithubTotalsSnapshotRecord } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; @@ -81,6 +81,11 @@ describe("queue trend windows", () => { }); }); + it("returns null review velocity when observedDays is zero", () => { + expect(computeReviewVelocityPerDay(7, 3, 0)).toBeNull(); + expect(computeReviewVelocityPerDay(7, 3, 7)).toBe(1.43); + }); + it("returns clear unavailable windows when history is missing", () => { const report = buildQueueTrendReport({ repoFullName: "owner/repo", totalsSnapshots: [totals(0, { openIssues: 1, openPrs: 1, merged: 0, closed: 0 })] }); expect(report).toMatchObject({ From 41b9a53fba372f615f3b42cbdf4f8fe597a5aad0 Mon Sep 17 00:00:00 2001 From: andriypolandki <=> Date: Mon, 29 Jun 2026 17:16:06 -0700 Subject: [PATCH 6/8] retest --- src/services/queue-trends.ts | 12 ++---------- test/unit/queue-trends.test.ts | 18 ++++++++++++++---- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/services/queue-trends.ts b/src/services/queue-trends.ts index cc2df8f187..a0d7e55295 100644 --- a/src/services/queue-trends.ts +++ b/src/services/queue-trends.ts @@ -88,18 +88,16 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe const targetMs = latestMs - windowDays * 24 * 60 * 60 * 1000; const baseline = [...totals].reverse().find((snapshot) => Date.parse(snapshot.fetchedAt) <= targetMs); if (!baseline) return unavailableWindow(windowDays, `Need at least ${windowDays} days of totals history.`); + // Baseline is the newest snapshot at or before (latest - windowDays), so observedDays >= windowDays here. const observedDays = round((latestMs - Date.parse(baseline.fetchedAt)) / (24 * 60 * 60 * 1000)); const mergedPullRequests = Math.max(0, latest.mergedPullRequestsTotal - baseline.mergedPullRequestsTotal); const closedUnmergedPullRequests = Math.max(0, latest.closedUnmergedPullRequestsTotal - baseline.closedUnmergedPullRequestsTotal); - const reviewVelocityPerDay = computeReviewVelocityPerDay(mergedPullRequests, closedUnmergedPullRequests, observedDays); - if (reviewVelocityPerDay === null) { - return unavailableWindow(windowDays, "Totals snapshots lack spacing for a reliable trend window."); - } const latestQueue = latestQueuePoint(queuePoints); const baselineQueue = latestQueue ? baselineQueuePoint(queuePoints, latestQueue.generatedAt, windowDays) : null; const stalePullRequestRate = latestQueue ? staleRate(latestQueue) : null; const baselineStaleRate = baselineQueue ? staleRate(baselineQueue) : null; const duplicateTrend = latestQueue && baselineQueue ? latestQueue.collisionClusters - baselineQueue.collisionClusters : null; + const reviewVelocityPerDay = round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); const pullRequestGrowth = latest.openPullRequestsTotal - baseline.openPullRequestsTotal; return { windowDays, @@ -119,12 +117,6 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe }; } -/** Null when observedDays <= 0 — callers must not divide; ready windows only proceed with a finite rate. */ -export function computeReviewVelocityPerDay(mergedPullRequests: number, closedUnmergedPullRequests: number, observedDays: number): number | null { - if (observedDays <= 0) return null; - return round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); -} - function trendWarnings(windows: QueueTrendWindow[]): string[] { const warnings: string[] = []; for (const window of windows.filter((entry) => entry.status === "ready")) { diff --git a/test/unit/queue-trends.test.ts b/test/unit/queue-trends.test.ts index b2ef620529..b6e0c27b03 100644 --- a/test/unit/queue-trends.test.ts +++ b/test/unit/queue-trends.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { getRepoQueueTrendSnapshot, persistRepoGithubTotalsSnapshot, persistSignalSnapshot, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; import { generateSignalSnapshots } from "../../src/queue/processors"; -import { buildQueueTrendReport, buildUnavailableQueueTrendReport, computeReviewVelocityPerDay, type QueueTrendReport } from "../../src/services/queue-trends"; +import { buildQueueTrendReport, buildUnavailableQueueTrendReport, type QueueTrendReport } from "../../src/services/queue-trends"; import type { RepoGithubTotalsSnapshotRecord } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; @@ -81,9 +81,19 @@ describe("queue trend windows", () => { }); }); - it("returns null review velocity when observedDays is zero", () => { - expect(computeReviewVelocityPerDay(7, 3, 0)).toBeNull(); - expect(computeReviewVelocityPerDay(7, 3, 7)).toBe(1.43); + it("observedDays is at least the requested window span for ready windows", () => { + const report = buildQueueTrendReport({ + repoFullName: "owner/repo", + totalsSnapshots: [ + totals(30, { openIssues: 20, openPrs: 4, merged: 2, closed: 1 }), + totals(0, { openIssues: 21, openPrs: 4, merged: 4, closed: 2 }), + ], + }); + + for (const window of report.windows.filter((entry) => entry.status === "ready")) { + expect(window.observedDays).toBeGreaterThanOrEqual(window.windowDays); + expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); + } }); it("returns clear unavailable windows when history is missing", () => { From c5c3a5045dcbd124b8e166c297cd0615e540721a Mon Sep 17 00:00:00 2001 From: andriypolandki <=> Date: Mon, 29 Jun 2026 19:16:38 -0700 Subject: [PATCH 7/8] retest --- src/services/queue-trends.ts | 16 +++++++++++++--- test/unit/queue-trends.test.ts | 15 ++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/services/queue-trends.ts b/src/services/queue-trends.ts index a0d7e55295..e8c25146e1 100644 --- a/src/services/queue-trends.ts +++ b/src/services/queue-trends.ts @@ -88,7 +88,7 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe const targetMs = latestMs - windowDays * 24 * 60 * 60 * 1000; const baseline = [...totals].reverse().find((snapshot) => Date.parse(snapshot.fetchedAt) <= targetMs); if (!baseline) return unavailableWindow(windowDays, `Need at least ${windowDays} days of totals history.`); - // Baseline is the newest snapshot at or before (latest - windowDays), so observedDays >= windowDays here. + // With a valid baseline, observedDays is normally >= windowDays; zero falls back to null velocity. const observedDays = round((latestMs - Date.parse(baseline.fetchedAt)) / (24 * 60 * 60 * 1000)); const mergedPullRequests = Math.max(0, latest.mergedPullRequestsTotal - baseline.mergedPullRequestsTotal); const closedUnmergedPullRequests = Math.max(0, latest.closedUnmergedPullRequestsTotal - baseline.closedUnmergedPullRequestsTotal); @@ -97,7 +97,7 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe const stalePullRequestRate = latestQueue ? staleRate(latestQueue) : null; const baselineStaleRate = baselineQueue ? staleRate(baselineQueue) : null; const duplicateTrend = latestQueue && baselineQueue ? latestQueue.collisionClusters - baselineQueue.collisionClusters : null; - const reviewVelocityPerDay = round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); + const reviewVelocityPerDay = computeReviewVelocityPerDay(mergedPullRequests, closedUnmergedPullRequests, observedDays); const pullRequestGrowth = latest.openPullRequestsTotal - baseline.openPullRequestsTotal; return { windowDays, @@ -113,10 +113,20 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe stalePullRequestRate, stalePullRequestRateDelta: stalePullRequestRate !== null && baselineStaleRate !== null ? round(stalePullRequestRate - baselineStaleRate) : null, duplicateTrend, - summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${reviewVelocityPerDay}/day.`, + summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${formatReviewVelocityPerDay(reviewVelocityPerDay)}.`, }; } +/** Returns null when observedDays <= 0 (no divide-by-zero / Infinity); otherwise the per-day close+merge rate. */ +export function computeReviewVelocityPerDay(mergedPullRequests: number, closedUnmergedPullRequests: number, observedDays: number): number | null { + if (observedDays <= 0) return null; + return round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); +} + +function formatReviewVelocityPerDay(reviewVelocityPerDay: number | null): string { + return reviewVelocityPerDay === null ? "n/a" : `${reviewVelocityPerDay}/day`; +} + function trendWarnings(windows: QueueTrendWindow[]): string[] { const warnings: string[] = []; for (const window of windows.filter((entry) => entry.status === "ready")) { diff --git a/test/unit/queue-trends.test.ts b/test/unit/queue-trends.test.ts index b6e0c27b03..bb46f999bb 100644 --- a/test/unit/queue-trends.test.ts +++ b/test/unit/queue-trends.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { getRepoQueueTrendSnapshot, persistRepoGithubTotalsSnapshot, persistSignalSnapshot, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; import { generateSignalSnapshots } from "../../src/queue/processors"; -import { buildQueueTrendReport, buildUnavailableQueueTrendReport, type QueueTrendReport } from "../../src/services/queue-trends"; +import { buildQueueTrendReport, buildUnavailableQueueTrendReport, computeReviewVelocityPerDay, type QueueTrendReport } from "../../src/services/queue-trends"; import type { RepoGithubTotalsSnapshotRecord } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; @@ -70,7 +70,9 @@ describe("queue trend windows", () => { for (const window of report.windows.filter((entry) => entry.status === "ready")) { expect(window.reviewVelocityPerDay).not.toBe(Infinity); expect(window.summary).not.toContain("Infinity"); - expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); + if (window.reviewVelocityPerDay !== null) { + expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); + } } expect(report.windows[0]).toMatchObject({ windowDays: 7, @@ -92,10 +94,17 @@ describe("queue trend windows", () => { for (const window of report.windows.filter((entry) => entry.status === "ready")) { expect(window.observedDays).toBeGreaterThanOrEqual(window.windowDays); - expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); + if (window.reviewVelocityPerDay !== null) { + expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); + } } }); + it("returns null review velocity when observedDays is zero", () => { + expect(computeReviewVelocityPerDay(7, 3, 0)).toBeNull(); + expect(computeReviewVelocityPerDay(7, 3, 7)).toBe(1.43); + }); + it("returns clear unavailable windows when history is missing", () => { const report = buildQueueTrendReport({ repoFullName: "owner/repo", totalsSnapshots: [totals(0, { openIssues: 1, openPrs: 1, merged: 0, closed: 0 })] }); expect(report).toMatchObject({ From ba31b9efed3720acbdd1f0f0966e237f4dc4c625 Mon Sep 17 00:00:00 2001 From: andriypolandki <=> Date: Mon, 29 Jun 2026 22:39:16 -0700 Subject: [PATCH 8/8] test(queue-trends): lock in finite velocity for ready windows --- src/services/queue-trends.ts | 16 +++------------- test/unit/queue-trends.test.ts | 15 +++------------ 2 files changed, 6 insertions(+), 25 deletions(-) diff --git a/src/services/queue-trends.ts b/src/services/queue-trends.ts index e8c25146e1..41ffb1a7fe 100644 --- a/src/services/queue-trends.ts +++ b/src/services/queue-trends.ts @@ -88,7 +88,7 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe const targetMs = latestMs - windowDays * 24 * 60 * 60 * 1000; const baseline = [...totals].reverse().find((snapshot) => Date.parse(snapshot.fetchedAt) <= targetMs); if (!baseline) return unavailableWindow(windowDays, `Need at least ${windowDays} days of totals history.`); - // With a valid baseline, observedDays is normally >= windowDays; zero falls back to null velocity. + // Baseline is the newest snapshot at or before (latest - windowDays), so observedDays >= windowDays. const observedDays = round((latestMs - Date.parse(baseline.fetchedAt)) / (24 * 60 * 60 * 1000)); const mergedPullRequests = Math.max(0, latest.mergedPullRequestsTotal - baseline.mergedPullRequestsTotal); const closedUnmergedPullRequests = Math.max(0, latest.closedUnmergedPullRequestsTotal - baseline.closedUnmergedPullRequestsTotal); @@ -97,7 +97,7 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe const stalePullRequestRate = latestQueue ? staleRate(latestQueue) : null; const baselineStaleRate = baselineQueue ? staleRate(baselineQueue) : null; const duplicateTrend = latestQueue && baselineQueue ? latestQueue.collisionClusters - baselineQueue.collisionClusters : null; - const reviewVelocityPerDay = computeReviewVelocityPerDay(mergedPullRequests, closedUnmergedPullRequests, observedDays); + const reviewVelocityPerDay = round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); const pullRequestGrowth = latest.openPullRequestsTotal - baseline.openPullRequestsTotal; return { windowDays, @@ -113,20 +113,10 @@ function buildWindow(windowDays: 7 | 14 | 30, totals: RepoGithubTotalsSnapshotRe stalePullRequestRate, stalePullRequestRateDelta: stalePullRequestRate !== null && baselineStaleRate !== null ? round(stalePullRequestRate - baselineStaleRate) : null, duplicateTrend, - summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${formatReviewVelocityPerDay(reviewVelocityPerDay)}.`, + summary: `${windowDays}d trend: PR queue ${signed(pullRequestGrowth)}, review velocity ${reviewVelocityPerDay}/day.`, }; } -/** Returns null when observedDays <= 0 (no divide-by-zero / Infinity); otherwise the per-day close+merge rate. */ -export function computeReviewVelocityPerDay(mergedPullRequests: number, closedUnmergedPullRequests: number, observedDays: number): number | null { - if (observedDays <= 0) return null; - return round((mergedPullRequests + closedUnmergedPullRequests) / observedDays); -} - -function formatReviewVelocityPerDay(reviewVelocityPerDay: number | null): string { - return reviewVelocityPerDay === null ? "n/a" : `${reviewVelocityPerDay}/day`; -} - function trendWarnings(windows: QueueTrendWindow[]): string[] { const warnings: string[] = []; for (const window of windows.filter((entry) => entry.status === "ready")) { diff --git a/test/unit/queue-trends.test.ts b/test/unit/queue-trends.test.ts index bb46f999bb..b6e0c27b03 100644 --- a/test/unit/queue-trends.test.ts +++ b/test/unit/queue-trends.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { getRepoQueueTrendSnapshot, persistRepoGithubTotalsSnapshot, persistSignalSnapshot, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; import { generateSignalSnapshots } from "../../src/queue/processors"; -import { buildQueueTrendReport, buildUnavailableQueueTrendReport, computeReviewVelocityPerDay, type QueueTrendReport } from "../../src/services/queue-trends"; +import { buildQueueTrendReport, buildUnavailableQueueTrendReport, type QueueTrendReport } from "../../src/services/queue-trends"; import type { RepoGithubTotalsSnapshotRecord } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; @@ -70,9 +70,7 @@ describe("queue trend windows", () => { for (const window of report.windows.filter((entry) => entry.status === "ready")) { expect(window.reviewVelocityPerDay).not.toBe(Infinity); expect(window.summary).not.toContain("Infinity"); - if (window.reviewVelocityPerDay !== null) { - expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); - } + expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); } expect(report.windows[0]).toMatchObject({ windowDays: 7, @@ -94,17 +92,10 @@ describe("queue trend windows", () => { for (const window of report.windows.filter((entry) => entry.status === "ready")) { expect(window.observedDays).toBeGreaterThanOrEqual(window.windowDays); - if (window.reviewVelocityPerDay !== null) { - expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); - } + expect(Number.isFinite(window.reviewVelocityPerDay)).toBe(true); } }); - it("returns null review velocity when observedDays is zero", () => { - expect(computeReviewVelocityPerDay(7, 3, 0)).toBeNull(); - expect(computeReviewVelocityPerDay(7, 3, 7)).toBe(1.43); - }); - it("returns clear unavailable windows when history is missing", () => { const report = buildQueueTrendReport({ repoFullName: "owner/repo", totalsSnapshots: [totals(0, { openIssues: 1, openPrs: 1, merged: 0, closed: 0 })] }); expect(report).toMatchObject({