From d0b770ca140876d1507e58e83f64ae11c05dde4b Mon Sep 17 00:00:00 2001 From: Brian Hodges Date: Thu, 18 Jun 2026 10:40:55 -0700 Subject: [PATCH] fix(metricsai): query reactors, not the removed users field, for review reactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GraphQL reaction lookup for review summaries asked for ReactionGroup.users, a field GitHub removed in 2021, so the query errored and PyGithub raised — silently swallowed to (0,0). Every review summary 👍/👎 was therefore dropped (a 👎 went uncounted). Switch to the current 'reactors { totalCount }' (with a 'users' fallback for old GHE) and add a regression test asserting 👎 maps to thumbs_down. Co-Authored-By: Claude Opus 4.8 (1M context) --- metricsai/src/metricsai/sources/github.py | 11 +++++++---- metricsai/tests/test_github_source.py | 21 +++++++++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/metricsai/src/metricsai/sources/github.py b/metricsai/src/metricsai/sources/github.py index 55b46ce..089c678 100644 --- a/metricsai/src/metricsai/sources/github.py +++ b/metricsai/src/metricsai/sources/github.py @@ -479,8 +479,10 @@ def _review_reactions(requester: object, raw: dict) -> tuple[int, int]: REST omits reactions for pull-request reviews (there is no reactions endpoint for them), so the summary-level 👍/👎 is only reachable through GraphQL -- ``PullRequestReview`` - implements the ``Reactable`` interface. A missing node id, a GraphQL/permission error, - or a network failure degrades quietly to ``(0, 0)`` rather than aborting the scan. + implements the ``Reactable`` interface. The per-group total comes from ``reactors`` (the + ``users`` field GitHub once exposed here was removed in 2021); a missing node id, a + GraphQL/permission error, or a network failure degrades quietly to ``(0, 0)`` rather than + aborting the scan. :param requester: The PyGithub ``Requester`` (``Github.requester``). :param raw: The review's ``raw_data`` (its ``node_id`` keys the GraphQL node lookup). @@ -491,7 +493,7 @@ def _review_reactions(requester: object, raw: dict) -> tuple[int, int]: return 0, 0 try: _, resp = requester.graphql_node( - node_id, "reactionGroups { content users { totalCount } }", "PullRequestReview" + node_id, "reactionGroups { content reactors { totalCount } }", "PullRequestReview" ) except Exception as exc: # GraphQL / permission / network errors must not abort the scan. logger.debug("review reactions lookup failed for %s: %s", node_id, exc) @@ -499,7 +501,8 @@ def _review_reactions(requester: object, raw: dict) -> tuple[int, int]: node = ((resp or {}).get("data") or {}).get("node") or {} up = down = 0 for group in node.get("reactionGroups") or []: - total = int((group.get("users") or {}).get("totalCount") or 0) + # ``reactors`` is the current field; fall back to the legacy ``users`` for old GHE. + total = int((group.get("reactors") or group.get("users") or {}).get("totalCount") or 0) if group.get("content") == "THUMBS_UP": up = total elif group.get("content") == "THUMBS_DOWN": diff --git a/metricsai/tests/test_github_source.py b/metricsai/tests/test_github_source.py index 13e6d31..6606f65 100644 --- a/metricsai/tests/test_github_source.py +++ b/metricsai/tests/test_github_source.py @@ -174,12 +174,17 @@ def get_repo(self, name): class _FakeRequester: - """Stub GitHub requester returning fixed reaction totals for ``graphql_node``.""" + """Stub GitHub requester returning fixed reaction totals for ``graphql_node``. - def __init__(self, up: int = 0, down: int = 0): + Uses the current ``reactors`` field (the ``users`` field was removed from + ``ReactionGroup`` in 2021); ``legacy=True`` emulates an old GHE that still serves ``users``. + """ + + def __init__(self, up: int = 0, down: int = 0, *, legacy: bool = False): + key = "users" if legacy else "reactors" self._groups = [ - {"content": "THUMBS_UP", "users": {"totalCount": up}}, - {"content": "THUMBS_DOWN", "users": {"totalCount": down}}, + {"content": "THUMBS_UP", key: {"totalCount": up}}, + {"content": "THUMBS_DOWN", key: {"totalCount": down}}, ] def graphql_node(self, node_id, output_schema, node_type): @@ -252,6 +257,14 @@ def test_scan_review_offdiff_respects_author_and_window() -> None: assert _offdiff(wrong_author, _FakeRequester(0, 0), match_all_authors=True) +def test_review_reactions_reads_reactors_and_maps_down_correctly() -> None: + # The 👎 must land in thumbs_down, not thumbs_up (regression: wrong content mapping). + assert github._review_reactions(_FakeRequester(0, 3), {"node_id": "R_1"}) == (0, 3) + assert github._review_reactions(_FakeRequester(2, 0), {"node_id": "R_1"}) == (2, 0) + # Legacy GHE still serving the old ``users`` field is read via the fallback. + assert github._review_reactions(_FakeRequester(1, 4, legacy=True), {"node_id": "R_1"}) == (1, 4) + + def test_review_reactions_degrades_to_zero() -> None: assert github._review_reactions(_FakeRequester(1, 1), {}) == (0, 0) # no node_id assert github._review_reactions(None, {"node_id": "R_1"}) == (0, 0) # no requester