feat(github): gate review reminders on implementation lines added

github.py

@@ -13,6 +13,7 @@
 from tenacity import Retrying, before_sleep_log, stop_after_attempt, wait_exponential
 
 from config import get_github_orgs
+from implementation_lines import count_implementation_additions
 from time_window import TimeWindow
 
 load_dotenv()
@@ -33,6 +34,7 @@
 )
 GITHUB_GRAPHQL_EXECUTE_TIMEOUT_SECONDS = 30
 CURSOR_AGENT_LOGIN = "cursoragent"
+REVIEW_REMINDER_MAX_IMPLEMENTATION_ADDITIONS = 200
 _cursor_pr_cache_lock = threading.Lock()
 
 
@@ -153,6 +155,15 @@ def get_prs(repo_id, pr_states, repo_name=None):
                             closedAt
                             isDraft
                             additions
+                            files(first: 100) {
+                                pageInfo {
+                                    hasNextPage
+                                }
+                                nodes {
+                                    path
+                                    additions
+                                }
+                            }
                             reviews(
                                 first: 10,
                                 states: [APPROVED, CHANGES_REQUESTED]
@@ -667,16 +678,19 @@ def get_prs_waiting_for_review_by_reviewer():
 
     Includes pull requests with an open review request or active requested-changes
     reviewer that has been waiting more than 24 hours. Approved PRs are excluded
-    even if GitHub still has leftover review requests. Only includes PRs with
-    fewer than 200 lines added.
+    even if GitHub still has leftover review requests.
     """
     all_prs = _get_all_prs(["OPEN"])
     stuck_prs = {}
     threshold = datetime.now(timezone.utc) - timedelta(hours=24)
     for pr in all_prs:
-        additions = pr.get("additions")
-        if additions is None or additions >= 200:
+        implementation_additions = count_implementation_additions(pr)
+        if (
+            implementation_additions is None
+            or implementation_additions >= REVIEW_REMINDER_MAX_IMPLEMENTATION_ADDITIONS
+        ):
             continue
+        pr["implementation_additions"] = implementation_additions
         if has_known_merge_conflicts(pr):
             continue
         if has_required_approval(pr):

implementation_lines.py

@@ -0,0 +1,108 @@
+from __future__ import annotations
+
+import fnmatch
+import posixpath
+from typing import Any
+
+EXCLUDED_DIRECTORIES = frozenset(
+    {
+        "__tests__",
+        "__test__",
+        "test",
+        "tests",
+        "spec",
+        "specs",
+        "__mocks__",
+        "__fixtures__",
+        "fixtures",
+        "__snapshots__",
+        "e2e",
+        "cypress",
+        "docs",
+        "doc",
+        "__generated__",
+        "generated",
+        "locales",
+        "locale",
+        "i18n",
+        "translations",
+        ".storybook",
+        ".cursor",
+        ".claude",
+    }
+)
+
+EXCLUDED_FILENAME_PATTERNS = (
+    "*.test.*",
+    "*.tests.*",
+    "*.spec.*",
+    "*_test.py",
+    "test_*.py",
+    "conftest.py",
+    "*_test.go",
+    "*_spec.rb",
+    "*.snap",
+    "*.stories.*",
+    "*.md",
+    "*.mdx",
+    "*.rst",
+    "license*",
+    "changelog*",
+    "codeowners",
+    ".cursorrules",
+    "*.lock",
+    "package-lock.json",
+    "pnpm-lock.yaml",
+    "*.generated.*",
+    "*.min.js",
+    "*.min.css",
+    "*.pb.go",
+    "*_pb2.py",
+    "*_pb2_grpc.py",
+    "*.po",
+    "*.pot",
+    "*.png",
+    "*.jpg",
+    "*.jpeg",
+    "*.gif",
+    "*.svg",
+    "*.webp",
+    "*.ico",
+    "*.pdf",
+    "*.ttf",
+    "*.otf",
+    "*.woff",
+    "*.woff2",
+    "*.mp3",
+    "*.mp4",
+    "*.mov",
+)
+
+
+def is_implementation_path(path: str) -> bool:
+    normalized = path.lower()
+    if not normalized:
+        return False
+    directory, filename = posixpath.split(normalized)
+    if any(segment in EXCLUDED_DIRECTORIES for segment in directory.split("/")):
+        return False
+    return not any(fnmatch.fnmatchcase(filename, pattern) for pattern in EXCLUDED_FILENAME_PATTERNS)
+
+
+def count_implementation_additions(pr: dict[str, Any]) -> int | None:
+    """Sum additions across a PR's implementation files.
+
+    Falls back to the PR's total additions when GitHub did not return the file
+    list or truncated it, so an unclassifiable PR is never under-counted.
+    """
+    files = pr.get("files") or {}
+    nodes = files.get("nodes")
+    truncated = (files.get("pageInfo") or {}).get("hasNextPage", False)
+    if nodes is None or truncated:
+        total = pr.get("additions")
+        return int(total) if total is not None else None
+    return sum(
+        int(node.get("additions") or 0)
+        for node in nodes
+        if is_implementation_path(node.get("path") or "")
+    )

jobs.py

@@ -15,6 +15,7 @@
 from constants import ENGINEERING_TEAM_SLUG, PRIORITY_TO_SCORE
 from fleet_health_cache import refresh_fleet_health_cache, should_use_redis_cache
 from github import (
+    REVIEW_REMINDER_MAX_IMPLEMENTATION_ADDITIONS,
     GitHubDataError,
     get_merged_pr_activity,
     get_merged_pr_counts_for_user,
@@ -646,7 +647,12 @@ def post_stale():
             filtered[reviewer] = pr_list
     prs = filtered
     if prs:
-        markdown += "*PRs - Checks Passing, Waiting for Review (+24h, <200 lines added)*\n"
+        markdown += (
+            "*PRs - Checks Passing, Waiting for Review "
+            f"(+24h, <{REVIEW_REMINDER_MAX_IMPLEMENTATION_ADDITIONS} implementation lines added)*\n"
+            "_Implementation lines exclude tests, fixtures, snapshots, docs, lockfiles, "
+            "generated code, localization files and assets._\n"
+        )
         for reviewer, pr_list in prs.items():
             if not pr_list:
                 continue
@@ -676,7 +682,10 @@ def post_stale():
                 pr_days.append((days_waiting, pr))
 
             for days_waiting, pr in sorted(pr_days, key=lambda x: x[0], reverse=True):
-                markdown += f"- <{pr['url']}|{pr['title']}> (+{days_waiting}d)\n"
+                markdown += (
+                    f"- <{pr['url']}|{pr['title']}> "
+                    f"(+{days_waiting}d, {pr['implementation_additions']} impl lines)\n"
+                )
         markdown += "\n\n"
 
     filtered_stale_issues = {

tests/test_gql_client_requests.py

@@ -317,6 +317,7 @@ def test_get_prs_retries_only_failed_page_and_keeps_complete_results(self):
         sleep.assert_called_once_with(1)
         query = print_ast(execute.call_args.args[0].document)
         self.assertIn("first: 20", query)
+        self.assertIn("files(first: 100)", query)
 
     def test_get_prs_raises_when_repo_fetch_fails(self):
         with (
@@ -404,6 +405,50 @@ def now(cls, tz=None):
                 )
                 self.assertEqual(waiting, expected)
 
+    def test_waiting_for_review_gates_on_implementation_additions(self):
+        def stuck_pr(number, additions, files):
+            return {
+                "number": number,
+                "url": f"https://github.com/example/repo/pull/{number}",
+                "additions": additions,
+                "files": {"pageInfo": {"hasNextPage": False}, "nodes": files},
+                "mergeable": "MERGEABLE",
+                "reviewDecision": "REVIEW_REQUIRED",
+                "reviewRequests": {"nodes": [{"requestedReviewer": {"login": "darrylyip"}}]},
+                "reviews": {"nodes": []},
+                "timelineItems": {
+                    "nodes": [
+                        {
+                            "createdAt": "2020-01-01T00:00:00Z",
+                            "requestedReviewer": {"login": "darrylyip"},
+                        }
+                    ]
+                },
+                "statusCheckRollup": {"state": "SUCCESS"},
+            }
+
+        mostly_tests = stuck_pr(
+            1,
+            309,
+            [
+                {"path": "src/core/schema.js", "additions": 17},
+                {"path": "src/data/prayers/__tests__/resolver.tests.js", "additions": 191},
+                {"path": "src/data/prayers/dataSource.js", "additions": 93},
+                {"path": "src/data/prayers/resolver.js", "additions": 8},
+            ],
+        )
+        large_implementation = stuck_pr(
+            2, 250, [{"path": "src/data/giving/dataSource.js", "additions": 250}]
+        )
+
+        with patch.object(
+            github, "_get_all_prs", return_value=[mostly_tests, large_implementation]
+        ):
+            waiting = github.get_prs_waiting_for_review_by_reviewer()
+
+        self.assertEqual(waiting, {"darrylyip": [mostly_tests]})
+        self.assertEqual(mostly_tests["implementation_additions"], 118)
+
     def test_waiting_for_review_allows_unknown_mergeability(self):
         class FixedDateTime(datetime):
             @classmethod

tests/test_implementation_lines.py

@@ -0,0 +1,84 @@
+import unittest
+
+from implementation_lines import count_implementation_additions, is_implementation_path
+
+
+class IsImplementationPathTest(unittest.TestCase):
+    def test_classifies_paths(self):
+        cases = {
+            "src/data/prayers/dataSource.js": True,
+            ".github/workflows/ci.yml": True,
+            "src/data/prayers/__tests__/resolver.tests.js": False,
+            "src/data/prayers/__tests__/helpers.js": False,
+            "tests/helpers.py": False,
+            "e2e/setup.ts": False,
+            "docs/index.html": False,
+            "src/components/__snapshots__/notes.json": False,
+            "src/Foo.Test.js": False,
+            "src/utils/format.test.ts": False,
+            "e2e/giving.spec.ts": False,
+            "tests/test_regressions.py": False,
+            "shovel/conftest.py": False,
+            "pkg/sync/sync_test.go": False,
+            "src/components/__snapshots__/Card.test.tsx.snap": False,
+            "src/components/Card.stories.tsx": False,
+            "README.md": False,
+            "docs/setup.mdx": False,
+            ".cursor/rules/style.mdc": False,
+            "yarn.lock": False,
+            "package-lock.json": False,
+            "src/__generated__/graphql.ts": False,
+            "src/api/types.generated.ts": False,
+            "src/locales/en.json": False,
+            "assets/logo.svg": False,
+            "fonts/Inter.woff2": False,
+            "": False,
+        }
+        for path, expected in cases.items():
+            with self.subTest(path=path):
+                self.assertEqual(is_implementation_path(path), expected)
+
+
+class CountImplementationAdditionsTest(unittest.TestCase):
+    def test_sums_only_implementation_files(self):
+        pr = {
+            "additions": 309,
+            "files": {
+                "pageInfo": {"hasNextPage": False},
+                "nodes": [
+                    {"path": "src/core/schema.js", "additions": 17},
+                    {"path": "src/data/prayers/__tests__/resolver.tests.js", "additions": 191},
+                    {"path": "src/data/prayers/dataSource.js", "additions": 93},
+                    {"path": "src/data/prayers/resolver.js", "additions": 8},
+                ],
+            },
+        }
+        self.assertEqual(count_implementation_additions(pr), 118)
+
+    def test_falls_back_to_total_when_file_list_is_truncated(self):
+        pr = {
+            "additions": 49995,
+            "files": {
+                "pageInfo": {"hasNextPage": True},
+                "nodes": [{"path": "src/a.js", "additions": 1}],
+            },
+        }
+        self.assertEqual(count_implementation_additions(pr), 49995)
+
+    def test_falls_back_to_total_when_files_are_missing(self):
+        self.assertEqual(count_implementation_additions({"additions": 12}), 12)
+        self.assertEqual(count_implementation_additions({"additions": 12, "files": None}), 12)
+
+    def test_returns_none_without_any_size_data(self):
+        self.assertIsNone(count_implementation_additions({}))
+
+    def test_tolerates_missing_file_fields(self):
+        pr = {
+            "additions": 5,
+            "files": {"nodes": [{"path": None, "additions": 3}, {"path": "src/a.py"}]},
+        }
+        self.assertEqual(count_implementation_additions(pr), 0)
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_jobs.py

@@ -53,6 +53,7 @@ def _install_import_shims() -> None:
 
     github_module = cast(Any, types.ModuleType("github"))
     github_module.GitHubDataError = type("GitHubDataError", (RuntimeError,), {})
+    github_module.REVIEW_REMINDER_MAX_IMPLEMENTATION_ADDITIONS = 200
     github_module.get_prs_waiting_for_review_by_reviewer = lambda *args, **kwargs: {}
     github_module.get_merged_pr_activity = lambda *args, **kwargs: ({}, {})
     github_module.get_merged_pr_counts_for_user = lambda *args, **kwargs: (0, 0)
@@ -361,6 +362,36 @@ def fake_get_stale_issues(issues, days):
         self.assertIn("APO-7555", message)
         self.assertIn("(74d)", message)
 
+    def _post_stale_message(self, team_members, prs_by_reviewer):
+        with (
+            patch.object(jobs_module, "get_team_members", return_value=team_members),
+            patch.object(
+                jobs_module,
+                "get_prs_waiting_for_review_by_reviewer",
+                return_value=prs_by_reviewer,
+            ),
+            patch.object(jobs_module, "get_open_stale_issues", return_value=[]),
+            patch.object(jobs_module, "get_stale_issues_by_assignee", return_value={}),
+            patch.dict(jobs_module.os.environ, {"APP_URL": "https://bug-board.example"}),
+            patch.object(jobs_module, "post_to_slack") as post,
+        ):
+            jobs_module.post_stale()
+        return post.call_args.args[0]
+
+    def test_review_reminders_show_implementation_lines(self):
+        pr = {
+            "title": "Group prayer list via forwarded-community consent",
+            "url": "https://github.com/ApollosProject/apollos-cluster/pull/4678",
+            "implementation_additions": 118,
+            "timelineItems": {"nodes": []},
+        }
+        message = self._post_stale_message(
+            {"vincent": {"github_username": "vincentwilson", "slack_id": "U0VW"}},
+            {"vincentwilson": [pr]},
+        )
+        self.assertIn("<200 implementation lines added", message)
+        self.assertIn("(+0d, 118 impl lines)", message)
+
     def test_continues_with_linear_stale_issues_when_github_pr_fetch_fails(self):
         open_issues = [{"id": "APO-7555"}]