From d20ddf33280464b1377aba8f755eb74df2f72724 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Tue, 15 Jul 2025 15:59:23 +0200 Subject: [PATCH] Fixed bug where PR author's own review was counted as first review. (#30815) Fixes #29140 Only impacts metrics gathering. ## Summary by CodeRabbit * **New Features** * Reviews made by the pull request creator are now filtered out in addition to bot reviews when viewing pull request review events. * **Tests** * Added and updated tests to verify correct filtering of both bot and pull request creator reviews, including improved logging checks. --- .../actions/eng-metrics/src/github-client.js | 42 ++++-- .../eng-metrics/src/metrics-collector.js | 5 +- ...ot-detection.test.js => pr-filter.test.js} | 120 +++++++++++++++++- 3 files changed, 152 insertions(+), 15 deletions(-) rename .github/actions/eng-metrics/test/{bot-detection.test.js => pr-filter.test.js} (60%) diff --git a/.github/actions/eng-metrics/src/github-client.js b/.github/actions/eng-metrics/src/github-client.js index 683ed34d22..6767b9280d 100644 --- a/.github/actions/eng-metrics/src/github-client.js +++ b/.github/actions/eng-metrics/src/github-client.js @@ -236,32 +236,50 @@ export class GitHubClient { } /** - * Filters out bot reviews from review events + * Filters out bot reviews and PR creator reviews from review events * @param {Array} reviewEvents - Array of review events * @param {boolean} excludeBots - Whether to exclude bot reviews (default: false) + * @param {Object} prCreator - PR creator user object (optional) * @returns {Array} Filtered review events */ - filterBotReviews(reviewEvents, excludeBots = false) { - if (!excludeBots) { + filterBotReviews(reviewEvents, excludeBots = false, prCreator = null) { + if (!excludeBots && !prCreator) { return reviewEvents; } const filteredReviews = reviewEvents.filter((review) => { - const botAnalysis = identifyBotUser(review.user); - if (botAnalysis.isBot) { - logger.debug(`Filtering out bot review from ${review.user.login}`, { - confidence: botAnalysis.confidence, - reasons: botAnalysis.reasons, - }); + // Filter out bot reviews + if (excludeBots) { + const botAnalysis = identifyBotUser(review.user); + if (botAnalysis.isBot) { + logger.debug(`Filtering out bot review from ${review.user.login}`, { + confidence: botAnalysis.confidence, + reasons: botAnalysis.reasons, + }); + return false; + } + } + + // Filter out PR creator reviews + if (prCreator && review.user.login === prCreator.login) { + logger.debug(`Filtering out PR creator review from ${review.user.login}`); return false; } + return true; }); - const botCount = reviewEvents.length - filteredReviews.length; - if (botCount > 0) { + const botCount = excludeBots ? reviewEvents.filter(review => identifyBotUser(review.user).isBot).length : 0; + const creatorCount = prCreator ? reviewEvents.filter(review => review.user.login === prCreator.login).length : 0; + const totalFiltered = reviewEvents.length - filteredReviews.length; + + if (totalFiltered > 0) { + const filterReasons = []; + if (botCount > 0) filterReasons.push(`${botCount} bot reviews`); + if (creatorCount > 0) filterReasons.push(`${creatorCount} PR creator reviews`); + logger.info( - `Filtered out ${botCount} bot reviews from ${reviewEvents.length} total reviews` + `Filtered out ${totalFiltered} reviews (${filterReasons.join(', ')}) from ${reviewEvents.length} total reviews` ); } diff --git a/.github/actions/eng-metrics/src/metrics-collector.js b/.github/actions/eng-metrics/src/metrics-collector.js index ea4fd7ced9..304f6f9198 100644 --- a/.github/actions/eng-metrics/src/metrics-collector.js +++ b/.github/actions/eng-metrics/src/metrics-collector.js @@ -125,10 +125,11 @@ export class MetricsCollector { pr.number ); - // Filter bot reviews if configured + // Filter bot reviews and PR creator reviews if configured const reviewEvents = this.githubClient.filterBotReviews( rawReviewEvents, - this.config.excludeBotReviews + this.config.excludeBotReviews, + pr.user ); // Collect enabled metrics for this PR diff --git a/.github/actions/eng-metrics/test/bot-detection.test.js b/.github/actions/eng-metrics/test/pr-filter.test.js similarity index 60% rename from .github/actions/eng-metrics/test/bot-detection.test.js rename to .github/actions/eng-metrics/test/pr-filter.test.js index ac7b1612af..e0aa97f565 100644 --- a/.github/actions/eng-metrics/test/bot-detection.test.js +++ b/.github/actions/eng-metrics/test/pr-filter.test.js @@ -177,7 +177,7 @@ describe('Bot Detection', () => { githubClient.filterBotReviews(reviews, true); expect(mockLogger.info).toHaveBeenCalledWith( - 'Filtered out 1 bot reviews from 2 total reviews' + 'Filtered out 1 reviews (1 bot reviews) from 2 total reviews' ); }); @@ -207,4 +207,122 @@ describe('Bot Detection', () => { ); }); }); + + describe('PR Creator Filtering', () => { + test('should filter out PR creator reviews', () => { + const prCreator = { login: 'prauthor', type: 'User' }; + const reviews = [ + { + user: { login: 'prauthor', type: 'User' }, + state: 'COMMENTED', + submitted_at: '2023-06-15T12:00:00Z' + }, + { + user: { login: 'reviewer1', type: 'User' }, + state: 'APPROVED', + submitted_at: '2023-06-15T13:00:00Z' + }, + { + user: { login: 'reviewer2', type: 'User' }, + state: 'CHANGES_REQUESTED', + submitted_at: '2023-06-15T14:00:00Z' + } + ]; + + const filtered = githubClient.filterBotReviews(reviews, false, prCreator); + expect(filtered).toHaveLength(2); + expect(filtered[0].user.login).toBe('reviewer1'); + expect(filtered[1].user.login).toBe('reviewer2'); + + expect(mockLogger.info).toHaveBeenCalledWith( + 'Filtered out 1 reviews (1 PR creator reviews) from 3 total reviews' + ); + }); + + test('should filter out multiple PR creator reviews', () => { + const prCreator = { login: 'prauthor', type: 'User' }; + const reviews = [ + { + user: { login: 'prauthor', type: 'User' }, + state: 'COMMENTED', + submitted_at: '2023-06-15T12:00:00Z' + }, + { + user: { login: 'reviewer1', type: 'User' }, + state: 'APPROVED', + submitted_at: '2023-06-15T13:00:00Z' + }, + { + user: { login: 'prauthor', type: 'User' }, + state: 'COMMENTED', + submitted_at: '2023-06-15T14:00:00Z' + } + ]; + + const filtered = githubClient.filterBotReviews(reviews, false, prCreator); + expect(filtered).toHaveLength(1); + expect(filtered[0].user.login).toBe('reviewer1'); + + expect(mockLogger.info).toHaveBeenCalledWith( + 'Filtered out 2 reviews (2 PR creator reviews) from 3 total reviews' + ); + }); + + test('should not filter when prCreator is null', () => { + const reviews = [ + { + user: { login: 'prauthor', type: 'User' }, + state: 'COMMENTED', + submitted_at: '2023-06-15T12:00:00Z' + }, + { + user: { login: 'reviewer1', type: 'User' }, + state: 'APPROVED', + submitted_at: '2023-06-15T13:00:00Z' + } + ]; + + const filtered = githubClient.filterBotReviews(reviews, false, null); + expect(filtered).toHaveLength(2); + + expect(mockLogger.info).not.toHaveBeenCalledWith( + expect.stringContaining('Filtered out') + ); + }); + + test('should filter both bots and PR creator', () => { + const prCreator = { login: 'prauthor', type: 'User' }; + const reviews = [ + { + user: { login: 'prauthor', type: 'User' }, + state: 'COMMENTED', + submitted_at: '2023-06-15T12:00:00Z' + }, + { + user: { login: 'coderabbitai[bot]', type: 'Bot' }, + state: 'COMMENTED', + submitted_at: '2023-06-15T13:00:00Z' + }, + { + user: { login: 'reviewer1', type: 'User' }, + state: 'APPROVED', + submitted_at: '2023-06-15T14:00:00Z' + }, + { + user: { login: 'dependabot[bot]', type: 'User' }, + state: 'COMMENTED', + submitted_at: '2023-06-15T15:00:00Z' + } + ]; + + const filtered = githubClient.filterBotReviews(reviews, true, prCreator); + expect(filtered).toHaveLength(1); + expect(filtered[0].user.login).toBe('reviewer1'); + + expect(mockLogger.info).toHaveBeenCalledWith( + 'Filtered out 3 reviews (2 bot reviews, 1 PR creator reviews) from 4 total reviews' + ); + }); + + }); }); \ No newline at end of file