From e191648b1c3a636f55b5134792f6ad2bb88d3852 Mon Sep 17 00:00:00 2001
From: Guillaume Verdier <guillaumev@google.com>
Date: Fri, 7 Aug 2026 12:09:24 +0000
Subject: [PATCH] fix: count reviewers who commented in assigned reviewers
 count

Include reviewers who submitted non-approving reviews (such as comments) in the assigned reviewers set so that the governance report accurately reflects active reviewers.
---
 org-tools/governance/scripts/pr_validator.py  |   6 ++
 .../governance/tests/test_pr_validator.py     | 101 ++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/org-tools/governance/scripts/pr_validator.py b/org-tools/governance/scripts/pr_validator.py
index 84d7498..297b887 100755
--- a/org-tools/governance/scripts/pr_validator.py
+++ b/org-tools/governance/scripts/pr_validator.py
@@ -291,7 +291,13 @@ def _get_all_approvers_and_assigned_usernames(
             else:
                 assigned.add(user)
 
+        # Count reviewers who have submitted reviews (including comments) as actively reviewing
+        for review in pr.reviews:
+            if review.user != pr.author:
+                assigned.add(review.user.lower())
+
         assigned.difference_update(approvers)
+        assigned.discard(pr.author)
         return approvers, assigned
 
     def _evaluate_file_statuses(
diff --git a/org-tools/governance/tests/test_pr_validator.py b/org-tools/governance/tests/test_pr_validator.py
index cd5ef1e..9fc1217 100644
--- a/org-tools/governance/tests/test_pr_validator.py
+++ b/org-tools/governance/tests/test_pr_validator.py
@@ -712,6 +712,107 @@ def test_one_approved_one_changes_requested_assigned_count(self):
         self.assertFalse(status.is_satisfied)
         self.assertEqual(status.approvers, ["maint1"])
 
+    def test_commented_reviewer_counts_towards_assigned_count(self):
+        """Test that a reviewer who added comments is counted in assigned_count."""
+        # Setup hierarchy and config requiring 2 approvals from maintainers
+        hierarchy = {
+            "maintainers": Team(name="maintainers", level=2),
+        }
+        rules = [
+            GovernanceRule(
+                name="Test Rule",
+                patterns=["*"],
+                requires_all=[
+                    RuleRequirement(min_approvals=2, min_team=hierarchy["maintainers"])
+                ],
+            )
+        ]
+        config = GovernanceConfig(
+            teams=hierarchy,
+            rules=rules,
+            fallback=[],
+            proxy_reviewers=set(),
+        )
+        memberships = TeamMemberships.create(
+            members_by_team={
+                "maintainers": {"maint1", "maint2", "maint3"},
+            },
+            teams=hierarchy,
+        )
+        validator = PullRequestValidator(config, memberships)
+
+        # maint1 approved, maint2 commented (no longer in review_requests on GitHub)
+        pr = PullRequest(
+            number=1,
+            author="author1",
+            is_draft=False,
+            changed_files=["file.txt"],
+            reviews=[
+                Review(user="maint1", state=ReviewState.APPROVED),
+                Review(user="maint2", state=ReviewState.COMMENTED),
+            ],
+            assigned_user_names=[],
+        )
+
+        res = validator.validate(pr)
+        self.assertFalse(res.is_mergeable)
+        self.assertEqual(res.error, ValidationErrorReason.INSUFFICIENT_APPROVALS)
+
+        self.assertEqual(len(res.requirement_statuses), 1)
+        status = res.requirement_statuses[0]
+        self.assertEqual(status.approved_count, 1)
+        self.assertEqual(status.assigned_count, 1)  # maint2 who commented is counted
+        self.assertFalse(status.is_satisfied)
+        self.assertEqual(status.approvers, ["maint1"])
+
+    def test_author_comments_not_counted_in_assigned(self):
+        """Test that PR author comments do not count towards assigned_count."""
+        hierarchy = {
+            "maintainers": Team(name="maintainers", level=2),
+        }
+        rules = [
+            GovernanceRule(
+                name="Test Rule",
+                patterns=["*"],
+                requires_all=[
+                    RuleRequirement(min_approvals=2, min_team=hierarchy["maintainers"])
+                ],
+            )
+        ]
+        config = GovernanceConfig(
+            teams=hierarchy,
+            rules=rules,
+            fallback=[],
+            proxy_reviewers=set(),
+        )
+        # Author is also a maintainer
+        memberships = TeamMemberships.create(
+            members_by_team={
+                "maintainers": {"author1", "maint1", "maint2"},
+            },
+            teams=hierarchy,
+        )
+        validator = PullRequestValidator(config, memberships)
+
+        pr = PullRequest(
+            number=1,
+            author="author1",
+            is_draft=False,
+            changed_files=["file.txt"],
+            reviews=[
+                Review(user="author1", state=ReviewState.COMMENTED),
+                Review(user="maint1", state=ReviewState.APPROVED),
+            ],
+            assigned_user_names=[],
+        )
+
+        res = validator.validate(pr)
+        self.assertFalse(res.is_mergeable)
+        status = res.requirement_statuses[0]
+        self.assertEqual(status.approved_count, 1)
+        self.assertEqual(status.assigned_count, 0)  # Author comments do not count
+        self.assertEqual(status.approvers, ["maint1"])
+
 
 class TestFetchTeamMemberships(unittest.TestCase):
     """Tests for GitHubClient.fetch_team_memberships method."""