chore(dev): Add a script to analyze cross-team PR reviews #6919

lavoiesl · 2026-01-07T19:14:57Z

https://coveord.atlassian.net/browse/KIT-5371

See the README.md for detailed explanation.

Disclaimer: 99% is AI-generated.

I took a general look at the code to make sure it wasn't doing anything dangerous, especially with the API calls, but otherwise, I mainly tested it.

This is not intended to be long-lived or be related to production in any way, so I don't believe high scrutiny is necessary.

scripts/reports/pr-review-times/stats.mjs

+  // Priority: draft > changesRequested > asked/commented > approved?
+  // User example: "_combined status: changesRequested" when one team is approved, one changesRequested.
+
+  let status = 'requested';


scripts/reports/pr-review-times/stats.mjs

+      ownerHandle,
+      teamMembersCache
+    );
+    _isTeam = true;


scripts/reports/pr-review-times/holidays.mjs

+    start.setDate(start.getDate() + 1);
+    start.setHours(0, 0, 0, 0);
+    // Safety: don't pass end
+    if (start > end) return 0;


github-actions · 2026-01-07T19:26:27Z

🔗 Scratch Orgs ready to test this PR:

✅ LWS Enabled: Open Org
✅ LWS Disabled: Open Org

scripts/reports/pr-review-times/fetch-pr.mjs

Copilot

Pull request overview

This PR introduces a comprehensive suite of analysis scripts to measure and report on pull request review times and cross-team collaboration patterns. The tooling is designed to fetch PR data from GitHub, calculate business-day-adjusted review durations, and generate aggregated reports across teams and confidence levels.

Implements a modular architecture with separate concerns for data fetching, analysis, storage, and reporting
Adds business hours calculation that excludes weekends and holidays for more accurate metrics
Includes a GitHub Actions workflow to automatically label PRs requiring cross-team reviews

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
scripts/reports/pr-review-times/storage.mjs	Implements local filesystem caching for PR data with incremental update support
scripts/reports/pr-review-times/stats.mjs	Legacy script for fetching and analyzing PRs (appears to be superseded by newer modular approach)
scripts/reports/pr-review-times/run.sh	Shell script to orchestrate the fetch and report generation workflow
scripts/reports/pr-review-times/list-prs.mjs	GitHub API search wrapper for querying PRs
scripts/reports/pr-review-times/index.mjs	CLI entry point orchestrating fetch, report, and backfill commands
scripts/reports/pr-review-times/holidays.mjs	Business hours calculation logic excluding weekends and North American holidays
scripts/reports/pr-review-times/fetch-pr.mjs	Core PR analysis logic to calculate review durations and identify responsible teams
scripts/reports/pr-review-times/data/.keep	Placeholder for local data cache directory
scripts/reports/pr-review-times/constants.mjs	Shared configuration constants (labels, repo info, output paths)
scripts/reports/pr-review-times/codeowners.mjs	CODEOWNERS file parser with glob-to-regex conversion
scripts/reports/pr-review-times/backfill-label.mjs	Utility to retroactively add cross-team labels to existing PRs
scripts/reports/pr-review-times/aggregate.mjs	Aggregation engine that computes statistics and generates JSON/CSV reports
scripts/reports/pr-review-times/README.md	Comprehensive documentation of design decisions and usage
scripts/reports/pr-review-times/.gitignore	Excludes generated JSON and CSV files from version control
.github/workflows/label-cross-team.yml	Automated workflow to label PRs with multiple team reviewers
.cspell.json	Adds 'dxui' to the spell-check dictionary

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-07T19:48:51Z

scripts/reports/pr-review-times/codeowners.mjs

+    // Try searching up from this script location
+    // This script is at scripts/reports/pr-review-times/
+    // So root is ../../../
+    const __dirname = path.dirname(new URL(import.meta.url).pathname);


On Windows systems, new URL(import.meta.url).pathname will return a path starting with a slash followed by the drive letter (e.g., '/C:/Users/...'). This can cause issues with path.resolve. Consider using fileURLToPath from 'node:url' instead, as shown in storage.mjs lines 4-7.

Copilot · 2026-01-07T19:48:52Z

scripts/reports/pr-review-times/fetch-pr.mjs

+          ? readyEvents.sort(
+              (a, b) => new Date(b.created_at) - new Date(a.created_at)
+            )[0].created_at


The function sorts the 'readyEvents' array in place on lines 241-245, which mutates the original 'events' array passed as a parameter. While this might not cause issues in the current implementation, it's better practice to avoid mutating function parameters. Consider using '.slice()' or the spread operator before sorting.

Suggested change

? readyEvents.sort(

(a, b) => new Date(b.created_at) - new Date(a.created_at)

)[0].created_at

? [...readyEvents]

.sort((a, b) => new Date(b.created_at) - new Date(a.created_at))[0]

.created_at

Copilot · 2026-01-07T19:48:52Z

scripts/reports/pr-review-times/stats.mjs

+async function analyzeTeamStat(
+  octokit,
+  ownerHandle,
+  pr,
+  events,
+  reviews,
+  teamMembersCache
+) {
+  // 1. Determine Reviewers
+  let potentialReviewers = [];
+  let _isTeam = false;
+  // Heuristic for teams vs users
+  if (ownerHandle.includes('/')) {
+    potentialReviewers = await getTeamMembers(
+      octokit,
+      ownerHandle,
+      teamMembersCache
+    );
+    _isTeam = true;
+  } else {
+    potentialReviewers = [ownerHandle.replace('@', '')];
+  }
+
+  // 2. Find Start Time: Earliest review_requested event for this team/user
+  // If PR was draft, we take the later of (Request Time, Ready For Review Time)
+  const requestEvents = events.filter((e) => e.event === 'review_requested');
+  const filteredRequests = idxRequestEvents(
+    requestEvents,
+    ownerHandle,
+    potentialReviewers
+  );
+
+  let startTime = null;
+  if (filteredRequests.length > 0) {
+    // Sort by earliest
+    filteredRequests.sort(
+      (a, b) => new Date(a.created_at) - new Date(b.created_at)
+    );
+    const firstRequest = filteredRequests[0].created_at;
+
+    // Check Draft Status
+    const readyEvents = events.filter((e) => e.event === 'ready_for_review');
+    if (pr.draft) {
+      // It is CURRENTLY draft.
+      // If filteredRequests exists, it means we requested review while in draft?
+      // Or before it went back to draft?
+      // User says: "draft: PR in draft, do not count review as started".
+      // If currently draft, result should be "not started"?
+      // But we need to return a structure.
+      return {
+        startTime: null,
+        endTime: null,
+        durationHours: null,
+        status: 'draft',
+      };
+    } else {
+      // It is NOT currently draft.
+      // Did the request happen while it WAS draft?
+      // Simple heuristic: If request < last ready_for_review, use valid ready_for_review time.
+      const latestReady =
+        readyEvents.length > 0
+          ? readyEvents.sort(
+              (a, b) => new Date(b.created_at) - new Date(a.created_at)
+            )[0].created_at
+          : null;
+
+      startTime = firstRequest;
+      if (latestReady && new Date(latestReady) > new Date(firstRequest)) {
+        startTime = latestReady;
+      }
+    }
+  } else {
+    // No explicit request found.
+    // If it's a code owner, maybe the 'labeled' event (legacy heuristic)?
+    // Or if PR is open and older than some date?
+    // Default: If no request event, not started.
+    // UNLESS we fallback to PR creation if map is empty?
+    // Let's return nulls if no request found.
+    return {
+      startTime: null,
+      endTime: null,
+      durationHours: null,
+      status: 'requested',
+    };
+  }
+
+  // 3. Status and End Time
+  // Calculate Status first
+  const relevantReviews = reviews.filter(
+    (r) =>
+      potentialReviewers.includes(r.user.login) &&
+      new Date(r.submitted_at) > new Date(startTime)
+  );
+  // Sort reviews chronologically
+  relevantReviews.sort(
+    (a, b) => new Date(a.submitted_at) - new Date(b.submitted_at)
+  );
+
+  // Latest review per user
+  const latestByUser = new Map();
+  for (const r of relevantReviews) {
+    latestByUser.set(r.user.login, r);
+  }
+  const finalReviews = Array.from(latestByUser.values());
+
+  let status = 'requested';
+  let endTime = null;
+
+  const hasChangesRequested = finalReviews.some(
+    (r) => r.state === 'CHANGES_REQUESTED'
+  );
+  const hasApproval = finalReviews.some((r) => r.state === 'APPROVED');
+  const hasComment = finalReviews.some((r) => r.state === 'COMMENTED');
+
+  // End Time Determination:
+  // If Approved: End Time is the timestamp of the *latest* necessary approval?
+  // For a Team, ONE approval is usually enough.
+  // So find the FIRST approval that happened?
+
+  if (hasChangesRequested) {
+    status = 'changesRequested';
+    endTime = null; // Blocked
+  } else if (hasApproval) {
+    status = 'approved';
+    // Find the first approval in the list
+    const approvals = relevantReviews.filter((r) => r.state === 'APPROVED');
+    if (approvals.length > 0) {
+      endTime = approvals[0].submitted_at;
+    }
+  } else if (hasComment) {
+    status = 'commented';
+    endTime = null;
+  }
+
+  // 4. Duration
+  let durationHours = null;
+  if (startTime) {
+    const start = new Date(startTime);
+    const end = endTime ? new Date(endTime) : new Date();
+    const diffMs = end - start;
+    durationHours = diffMs / (1000 * 60 * 60);
+  }
+
+  return {
+    startTime,
+    endTime,
+    durationHours,
+    status,
+  };
+}


There's duplicate logic between this function in stats.mjs and analyzeTeamStat in fetch-pr.mjs. The functions have nearly identical implementations but exist in different files. Consider extracting this shared logic into a common module to improve maintainability and reduce code duplication.

Copilot · 2026-01-07T19:48:52Z

scripts/reports/pr-review-times/README.md

+- `high-confidence`: The system is blocking from merging due to CODEOWNERS, but the author rather confident about the change and thinks they should be able to merge without an additional review.
+- `low-confidence`: The author is not that confident and actually wants another team to review the change before merging.


The description for 'high-confidence' on line 42 appears inverted. It states the author is "rather confident" and "should be able to merge without an additional review", but 'high-confidence' typically means high confidence in needing a review. Consider clarifying this terminology or swapping the descriptions for 'high-confidence' and 'low-confidence' to match conventional expectations.

Suggested change

- `high-confidence`: The system is blocking from merging due to CODEOWNERS, but the author rather confident about the change and thinks they should be able to merge without an additional review.

- `low-confidence`: The author is not that confident and actually wants another team to review the change before merging.

- `high-confidence`: The system is blocking from merging due to CODEOWNERS, and the author is not that confident and actually wants another team to review the change before merging.

- `low-confidence`: The author is rather confident about the change and thinks they should be able to merge without an additional review.

Copilot · 2026-01-07T19:48:53Z

scripts/reports/pr-review-times/aggregate.mjs

+
+  const p95Index = Math.floor(values.length * 0.95);
+  const p95 = sorted[p95Index];
+
+  return {
+    count: values.length,
+    avg: parseFloat(avg.toFixed(2)),


The 'avg' calculation on line 193 uses a simple arithmetic mean which can be heavily skewed by outliers. For duration metrics, consider also calculating or primarily using the median, which provides a more robust measure of central tendency for distributions that may have extreme values.

Suggested change

const p95Index = Math.floor(values.length * 0.95);

const p95 = sorted[p95Index];

return {

count: values.length,

avg: parseFloat(avg.toFixed(2)),

const midIndex = Math.floor(sorted.length / 2);

const median =

sorted.length % 2 === 0

? (sorted[midIndex - 1] + sorted[midIndex]) / 2

: sorted[midIndex];

const p95Index = Math.floor(values.length * 0.95);

const p95 = sorted[p95Index];

return {

count: values.length,

avg: parseFloat(avg.toFixed(2)),

median: parseFloat(median.toFixed(2)),

Copilot · 2026-01-07T19:48:55Z

.github/workflows/label-cross-team.yml

+              }
+            } catch (error) {
+              console.error('Error in label-cross-team action:', error);
+              core.setFailed(error.message);


The 'core' object is referenced on line 53 but is never imported or defined in the script. This will cause a ReferenceError at runtime if an error occurs. The correct usage would be to throw the error or simply log it, since 'core.setFailed' is not available in this context.

Suggested change

core.setFailed(error.message);

throw error;

Copilot · 2026-01-07T19:48:55Z

scripts/reports/pr-review-times/fetch-pr.mjs

+
+  // Calculate Status
+  const isMerged = events.some((e) => e.event === 'merged');
+  let status = 'unknown';


The initial value of status is unused, since it is always overwritten.

Suggested change

let status = 'unknown';

let status;

Copilot · 2026-01-07T19:48:56Z

scripts/reports/pr-review-times/stats.mjs

+  // Priority: draft > changesRequested > asked/commented > approved?
+  // User example: "_combined status: changesRequested" when one team is approved, one changesRequested.
+
+  let status = 'requested';


The initial value of status is unused, since it is always overwritten.

Suggested change

let status = 'requested';

let status;

Copilot · 2026-01-07T19:48:56Z

scripts/reports/pr-review-times/stats.mjs

+      ownerHandle,
+      teamMembersCache
+    );
+    _isTeam = true;


The value assigned to _isTeam here is unused.

Copilot · 2026-01-07T19:48:56Z

scripts/reports/pr-review-times/holidays.mjs

+    start.setDate(start.getDate() + 1);
+    start.setHours(0, 0, 0, 0);
+    // Safety: don't pass end
+    if (start > end) return 0;


The condition 'start > end' is always false.

scripts/reports/pr-review-times/fetch-pr.mjs

+
+  // Calculate Status
+  const isMerged = events.some((e) => e.event === 'merged');
+  let status = 'unknown';


lavoiesl temporarily deployed to PR Artifacts January 7, 2026 19:15 — with GitHub Actions Inactive

lavoiesl had a problem deploying to PR Artifacts January 7, 2026 19:15 — with GitHub Actions Failure

lavoiesl temporarily deployed to PR Artifacts January 7, 2026 19:15 — with GitHub Actions Inactive

lavoiesl force-pushed the seb-pr-review-times branch from 3d8b42b to af55e4a Compare January 7, 2026 19:16

lavoiesl temporarily deployed to PR Artifacts January 7, 2026 19:16 — with GitHub Actions Inactive

github-code-quality bot found potential problems Jan 7, 2026

View reviewed changes

lavoiesl temporarily deployed to Prerelease (CDN) January 7, 2026 19:22 — with GitHub Actions Inactive

lavoiesl temporarily deployed to PR Artifacts January 7, 2026 19:28 — with GitHub Actions Inactive

lavoiesl temporarily deployed to PR Artifacts January 7, 2026 19:33 — with GitHub Actions Inactive

lavoiesl force-pushed the seb-pr-review-times branch from af55e4a to 8fd4f46 Compare January 7, 2026 19:42

lavoiesl temporarily deployed to PR Artifacts January 7, 2026 19:42 — with GitHub Actions Inactive

lavoiesl marked this pull request as ready for review January 7, 2026 19:43

lavoiesl requested a review from a team as a code owner January 7, 2026 19:43

lavoiesl requested review from alexprudhomme, Copilot, fbeaudoincoveo and y-lakhdar January 7, 2026 19:43

Copilot started reviewing on behalf of lavoiesl January 7, 2026 19:43 View session

github-code-quality bot found potential problems Jan 7, 2026

View reviewed changes

scripts/reports/pr-review-times/fetch-pr.mjs Fixed Show fixed Hide fixed

lavoiesl temporarily deployed to Prerelease (CDN) January 7, 2026 19:48 — with GitHub Actions Inactive

Copilot AI reviewed Jan 7, 2026

View reviewed changes

lavoiesl temporarily deployed to PR Artifacts January 7, 2026 19:54 — with GitHub Actions Inactive

lavoiesl temporarily deployed to PR Artifacts January 7, 2026 19:59 — with GitHub Actions Inactive

chore(dev): Add a script to analyze cross-team PR reviews

878ccee

lavoiesl force-pushed the seb-pr-review-times branch from 8fd4f46 to 878ccee Compare January 8, 2026 14:21

lavoiesl temporarily deployed to PR Artifacts January 8, 2026 14:21 — with GitHub Actions Inactive

github-code-quality bot found potential problems Jan 8, 2026

View reviewed changes

scripts/reports/pr-review-times/fetch-pr.mjs

// Calculate Status

const isMerged = events.some((e) => e.event === 'merged');

let status = 'unknown';

lavoiesl temporarily deployed to Prerelease (CDN) January 8, 2026 14:27 — with GitHub Actions Inactive

lavoiesl temporarily deployed to PR Artifacts January 8, 2026 14:34 — with GitHub Actions Inactive

lavoiesl temporarily deployed to PR Artifacts January 8, 2026 14:37 — with GitHub Actions Inactive

		- `high-confidence`: The system is blocking from merging due to CODEOWNERS, but the author rather confident about the change and thinks they should be able to merge without an additional review.
		- `low-confidence`: The author is not that confident and actually wants another team to review the change before merging.

chore(dev): Add a script to analyze cross-team PR reviews #6919

Are you sure you want to change the base?

chore(dev): Add a script to analyze cross-team PR reviews #6919

Uh oh!

Conversation

lavoiesl commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lavoiesl commented Jan 7, 2026 •

edited

Loading

github-actions bot commented Jan 7, 2026 •

edited

Loading