fix: repair dead discussion mentor-counting branch

discussions.py

@@ -31,9 +31,21 @@ def get_discussions(token: str, search_query: str, ghe: str):
                         title
                         url
                         createdAt
-                        comments(first: 1) {
+                        author {
+                            login
+                            __typename
+                        }
+                        # Only the first 100 comments are fetched (no
+                        # pagination). MAX_COMMENTS_EVAL defaults to 20, so
+                        # this ceiling is not hit in practice; setting it above
+                        # 100 would silently cap discussion mentor counts.
+                        comments(first: 100) {
                             nodes {
                                 createdAt
+                                author {
+                                    login
+                                    __typename
+                                }
                             }
                         }
                         answerChosenAt

most_active_mentors.py

@@ -56,8 +56,13 @@ def count_comments_per_user(
 
     Args:
         issue (Union[Issue, None]): A GitHub issue.
+        discussion (Union[dict, None]): A GitHub discussion as returned by
+        discussions.get_discussions (a plain GraphQL dict, not a PyGithub
+        object).
         pull_request (Union[PullRequest, None]): A GitHub pull
         request.
+        ready_for_review_at (Union[datetime, None]): When the item became
+        ready for review; comments before this are ignored.
         ignore_users (List[str]): A list of GitHub usernames to ignore.
         max_comments_to_eval: Maximum number of comments per item to look at.
         heavily_involved: Maximum number of comments to count for one
@@ -118,27 +123,41 @@ def count_comments_per_user(
                 else:
                     mentor_count[review_comment.user.login] = 1
 
-        # The discussion branch below is dead in production (tracked in #774):
-        # the GraphQL query in discussions.get_discussions fetches no comment
-        # author data, attribute access on dict nodes would AttributeError,
-        # and the ignore_comment call below passes comment.user as both
-        # issue_user and comment_user (self-reference always True).
-        if discussion and len(discussion["comments"]["nodes"]) > 0:  # pragma: no cover
-            for comment in discussion["comments"]["nodes"]:
-                if ignore_comment(
-                    comment.user,
-                    comment.user,
-                    ignore_users,
-                    comment.submitted_at,
-                    comment.ready_for_review_at,
-                ):
-                    continue
+    # The discussion branch: use dict access because GraphQL returns plain
+    # dicts (not PyGithub objects). Filtering is inlined here (rather than
+    # reusing ignore_comment, which expects PyGithub objects): the discussion
+    # author's login is compared against each comment author to drop
+    # self-comments.
+    if discussion and len(discussion["comments"]["nodes"]) > 0:
+        discussion_author_login = (discussion.get("author") or {}).get("login", "")
+        comment_count = 0
+        for comment in discussion["comments"]["nodes"]:
+            if comment_count >= max_comments_to_eval:
+                break
+            comment_count += 1
+            comment_author = comment.get("author") or {}
+            comment_login = comment_author.get("login", "")
+            comment_type = comment_author.get("__typename", "")
+            comment_created_at = comment.get("createdAt")
+
+            if (
+                not comment_login
+                # ignore bots
+                or comment_type == "Bot"
+                # ignore comments by the discussion author
+                or comment_login == discussion_author_login
+                # ignore users in the ignore list
+                or comment_login in ignore_users
+                # ignore comments without a timestamp
+                or not comment_created_at
+            ):
+                continue
 
-                # increase the number of comments left by current user by 1
-                if comment.user.login in mentor_count:
-                    mentor_count[comment.user.login] += 1
-                else:
-                    mentor_count[comment.user.login] = 1
+            if comment_login in mentor_count:
+                if mentor_count[comment_login] < heavily_involved:
+                    mentor_count[comment_login] += 1
+            else:
+                mentor_count[comment_login] = 1
 
     return mentor_count
 

test_most_active_mentors.py

@@ -201,3 +201,185 @@ def test_count_comments_per_user_review_limit(self):
             mock_issue, pull_request=mock_pr, max_comments_to_eval=5
         )
         self.assertEqual(result, {"reviewer": 5})
+
+
+class TestCountCommentsDiscussions(unittest.TestCase):
+    """Covers the discussion branch of count_comments_per_user.
+
+    Before the fix for #774 this branch was dead code because:
+      1. The GraphQL query fetched no comment author data.
+      2. Attribute access on dict nodes would raise AttributeError.
+      3. The ignore_comment call passed comment.user as both issue_user
+         and comment_user, so every comment was silently skipped.
+    """
+
+    def _make_discussion(self, author_login, comments):
+        """Return a minimal discussion dict as returned by get_discussions."""
+        return {
+            "title": "Test discussion",
+            "url": "https://github.com/org/repo/discussions/1",
+            "createdAt": "2024-01-01T00:00:00Z",
+            "author": {"login": author_login, "__typename": "User"},
+            "comments": {"nodes": comments},
+            "answerChosenAt": None,
+            "closedAt": None,
+        }
+
+    def test_counts_commenter(self):
+        """A comment by a user other than the author is counted."""
+        discussion = self._make_discussion(
+            "op",
+            [
+                {
+                    "createdAt": "2024-01-02T00:00:00Z",
+                    "author": {"login": "mentor", "__typename": "User"},
+                },
+            ],
+        )
+        result = count_comments_per_user(None, discussion=discussion)
+        self.assertEqual(result, {"mentor": 1})
+
+    def test_ignores_discussion_author_self_comment(self):
+        """Comments by the discussion author are not counted."""
+        discussion = self._make_discussion(
+            "op",
+            [
+                {
+                    "createdAt": "2024-01-02T00:00:00Z",
+                    "author": {"login": "op", "__typename": "User"},
+                },
+            ],
+        )
+        result = count_comments_per_user(None, discussion=discussion)
+        self.assertEqual(result, {})
+
+    def test_ignores_bot_commenter(self):
+        """Comments from Bot actors are not counted."""
+        discussion = self._make_discussion(
+            "op",
+            [
+                {
+                    "createdAt": "2024-01-02T00:00:00Z",
+                    "author": {"login": "github-actions", "__typename": "Bot"},
+                },
+            ],
+        )
+        result = count_comments_per_user(None, discussion=discussion)
+        self.assertEqual(result, {})
+
+    def test_ignores_user_in_ignore_list(self):
+        """Comments by explicitly ignored users are not counted."""
+        discussion = self._make_discussion(
+            "op",
+            [
+                {
+                    "createdAt": "2024-01-02T00:00:00Z",
+                    "author": {"login": "spammer", "__typename": "User"},
+                },
+            ],
+        )
+        result = count_comments_per_user(
+            None, discussion=discussion, ignore_users=["spammer"]
+        )
+        self.assertEqual(result, {})
+
+    def test_multiple_commenters(self):
+        """Multiple distinct commenters each get their own count."""
+        discussion = self._make_discussion(
+            "op",
+            [
+                {
+                    "createdAt": "2024-01-02T00:00:00Z",
+                    "author": {"login": "alice", "__typename": "User"},
+                },
+                {
+                    "createdAt": "2024-01-03T00:00:00Z",
+                    "author": {"login": "bob", "__typename": "User"},
+                },
+                {
+                    "createdAt": "2024-01-04T00:00:00Z",
+                    "author": {"login": "alice", "__typename": "User"},
+                },
+            ],
+        )
+        result = count_comments_per_user(None, discussion=discussion)
+        self.assertEqual(result, {"alice": 2, "bob": 1})
+
+    def test_no_comments(self):
+        """A discussion with an empty comments list returns an empty dict."""
+        discussion = self._make_discussion("op", [])
+        result = count_comments_per_user(None, discussion=discussion)
+        self.assertEqual(result, {})
+
+    def test_respects_max_comments_to_eval(self):
+        """Comments beyond max_comments_to_eval are not evaluated."""
+        comments = [
+            {
+                "createdAt": f"2024-01-{i + 2:02d}T00:00:00Z",
+                "author": {"login": f"mentor{i}", "__typename": "User"},
+            }
+            for i in range(5)
+        ]
+        discussion = self._make_discussion("op", comments)
+        result = count_comments_per_user(
+            None, discussion=discussion, max_comments_to_eval=2
+        )
+        self.assertEqual(result, {"mentor0": 1, "mentor1": 1})
+
+    def test_caps_count_at_heavily_involved(self):
+        """A single commenter is capped at heavily_involved comments."""
+        comments = [
+            {
+                "createdAt": f"2024-01-{i + 2:02d}T00:00:00Z",
+                "author": {"login": "mentor", "__typename": "User"},
+            }
+            for i in range(5)
+        ]
+        discussion = self._make_discussion("op", comments)
+        result = count_comments_per_user(
+            None, discussion=discussion, heavily_involved=3
+        )
+        self.assertEqual(result, {"mentor": 3})
+
+    def test_null_author_skipped(self):
+        """A comment with a null author (deleted account) is skipped gracefully."""
+        discussion = self._make_discussion(
+            "op",
+            [
+                {
+                    "createdAt": "2024-01-02T00:00:00Z",
+                    "author": None,
+                },
+            ],
+        )
+        result = count_comments_per_user(None, discussion=discussion)
+        self.assertEqual(result, {})
+
+    def test_null_created_at_skipped(self):
+        """A comment with a null createdAt (pending) is skipped."""
+        discussion = self._make_discussion(
+            "op",
+            [
+                {
+                    "createdAt": None,
+                    "author": {"login": "mentor", "__typename": "User"},
+                },
+            ],
+        )
+        result = count_comments_per_user(None, discussion=discussion)
+        self.assertEqual(result, {})
+
+    def test_null_discussion_author(self):
+        """A discussion whose author is null (deleted OP) is handled."""
+        discussion = self._make_discussion(
+            "op",
+            [
+                {
+                    "createdAt": "2024-01-02T00:00:00Z",
+                    "author": {"login": "mentor", "__typename": "User"},
+                },
+            ],
+        )
+        discussion["author"] = None
+        result = count_comments_per_user(None, discussion=discussion)
+        self.assertEqual(result, {"mentor": 1})