diff --git a/services/github/pr-insights.test.ts b/services/github/pr-insights.test.ts new file mode 100644 index 000000000..a555e833f --- /dev/null +++ b/services/github/pr-insights.test.ts @@ -0,0 +1,133 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { fetchPRInsights, type PRInsightData } from './pr-insights'; +import { fetchWithRetry } from '@/lib/github'; + +vi.mock('@/lib/github', () => ({ + fetchWithRetry: vi.fn(), + getGitHubTokens: vi.fn((): string[] => ['mock-github-token']), +})); + +describe('fetchPRInsights - avgReviewTime math bug fix', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('correctly calculates repoPerformance avgReviewTime when self-reviews exist', async () => { + const mockResponse = new Response( + JSON.stringify({ + data: { + authored: { + nodes: [ + { + id: 'PR_1', + title: 'Feature PR', + url: 'https://github.com/owner/repo1/pull/1', + state: 'MERGED', + createdAt: '2026-06-01T10:00:00Z', + closedAt: '2026-06-01T14:00:00Z', + mergedAt: '2026-06-01T14:00:00Z', + additions: 150, + deletions: 20, + repository: { nameWithOwner: 'owner/repo1' }, + comments: { totalCount: 2 }, + reviews: { + nodes: [ + // Self review by author -> should be skipped in timing calculation + { + author: { login: 'test-user' }, + createdAt: '2026-06-01T10:30:00Z', + state: 'COMMENTED', + }, + // Reviewer review 4 hours after creation (2026-06-01T14:00:00Z) + { + author: { login: 'reviewer-user' }, + createdAt: '2026-06-01T14:00:00Z', + state: 'APPROVED', + }, + ], + totalCount: 2, + }, + }, + ], + pageInfo: { + hasNextPage: false, + endCursor: null, + }, + }, + reviewed: { + issueCount: 5, + }, + }, + }), + { status: 200 } + ); + + vi.mocked(fetchWithRetry).mockResolvedValue(mockResponse); + + const result: PRInsightData = await fetchPRInsights('test-user'); + + expect(result.repoPerformance).toHaveLength(1); + const repo1 = result.repoPerformance[0]; + expect(repo1.name).toBe('owner/repo1'); + expect(repo1.totalPRs).toBe(1); + expect(repo1.reviewCount).toBe(2); + + // Prior to fix: reviewTimeSum (4h) was divided by reviewCount (2) = 2h (incorrectly deflated by self-review) + // Post-fix: reviewTimeSum (4h) is divided by validReviewTimesCount (1) = 4h + expect(repo1.avgReviewTime).toBe(4); + }); + + it('returns avgReviewTime: 0 when a repo has no non-self reviews', async () => { + const mockResponse = new Response( + JSON.stringify({ + data: { + authored: { + nodes: [ + { + id: 'PR_2', + title: 'Solo PR', + url: 'https://github.com/owner/repo2/pull/2', + state: 'OPEN', + createdAt: '2026-06-01T10:00:00Z', + closedAt: null, + mergedAt: null, + additions: 50, + deletions: 10, + repository: { nameWithOwner: 'owner/repo2' }, + comments: { totalCount: 0 }, + reviews: { + nodes: [ + { + author: { login: 'solo-author' }, + createdAt: '2026-06-01T11:00:00Z', + state: 'COMMENTED', + }, + ], + totalCount: 1, + }, + }, + ], + pageInfo: { + hasNextPage: false, + endCursor: null, + }, + }, + reviewed: { + issueCount: 0, + }, + }, + }), + { status: 200 } + ); + + vi.mocked(fetchWithRetry).mockResolvedValue(mockResponse); + + const result: PRInsightData = await fetchPRInsights('solo-author'); + + expect(result.repoPerformance).toHaveLength(1); + const repo2 = result.repoPerformance[0]; + expect(repo2.name).toBe('owner/repo2'); + expect(repo2.reviewCount).toBe(1); + expect(repo2.avgReviewTime).toBe(0); + }); +}); diff --git a/services/github/pr-insights.ts b/services/github/pr-insights.ts index b5e59fef5..0d6ac9e08 100644 --- a/services/github/pr-insights.ts +++ b/services/github/pr-insights.ts @@ -219,7 +219,13 @@ async function fetchPRInsightsUncached( const repoMap = new Map< string, - { total: number; merged: number; reviewCount: number; reviewTimeSum: number } + { + total: number; + merged: number; + reviewCount: number; + reviewTimeSum: number; + validReviewTimesCount: number; + } >(); let mostDiscussed = { title: '', url: '', comments: -1 }; @@ -263,7 +269,13 @@ async function fetchPRInsightsUncached( // Repos const repoName = pr.repository?.nameWithOwner || 'Unknown'; if (!repoMap.has(repoName)) { - repoMap.set(repoName, { total: 0, merged: 0, reviewCount: 0, reviewTimeSum: 0 }); + repoMap.set(repoName, { + total: 0, + merged: 0, + reviewCount: 0, + reviewTimeSum: 0, + validReviewTimesCount: 0, + }); } const repoStats = repoMap.get(repoName)!; repoStats.total++; @@ -309,6 +321,7 @@ async function fetchPRInsightsUncached( prReviewTimes.push(diffHours); reviewTimes.push(diffHours); repoStats.reviewTimeSum += diffHours; + repoStats.validReviewTimesCount++; if (diffHours < fastestReview) fastestReview = diffHours; if (diffHours > slowestReview) slowestReview = diffHours; @@ -348,7 +361,8 @@ async function fetchPRInsightsUncached( totalPRs: stats.total, mergeRate: stats.total > 0 ? (stats.merged / stats.total) * 100 : 0, reviewCount: stats.reviewCount, - avgReviewTime: stats.reviewCount > 0 ? stats.reviewTimeSum / stats.reviewCount : 0, + avgReviewTime: + stats.validReviewTimesCount > 0 ? stats.reviewTimeSum / stats.validReviewTimesCount : 0, })) .sort((a, b) => b.totalPRs - a.totalPRs) .slice(0, 10);