feat(analytics): refactor bot event filtering to use is_bot_name utility and enhance tests

Author: Ayyanaruto

backend/analytics_server/mhq/service/code/sync/etl_code_analytics.py

@@ -10,7 +10,7 @@
     PullRequestState,
 )
 from mhq.utils.time import Interval
-import re
+from mhq.utils.string import is_bot_name
 
 
 class CodeETLAnalyticsService:
@@ -184,20 +184,4 @@ def filter_non_bot_events(
         self, pr_events: List[PullRequestEvent]
     ) -> List[PullRequestEvent]:
         """Filter out events created by bot users using regex patterns."""
-
-        bot_pattern = re.compile(
-            r"bot|[bB][oO][tT]|\[bot\]|automated|jenkins|ci-|github-actions",
-            re.IGNORECASE,
-        )
-
-        return [
-            event
-            for event in pr_events
-            if (
-                not bool(bot_pattern.search(event.actor_username))
-                and not (
-                    event.data.get("user")
-                    and event.data.get("user", {}).get("type") == "Bot"
-                )
-            )
-        ]
+        return [event for event in pr_events if (not is_bot_name(event.actor_username))]

backend/analytics_server/mhq/service/code/sync/etl_github_handler.py

@@ -181,9 +181,9 @@ def process_pr(
             pr_commits_model_list: List[PullRequestCommit] = self._to_pr_commits(
                 commits, pr_model
             )
-
+        non_bot_pr_events_model_list = self._github_bot_filter(pr_events_model_list)
         pr_model = self.code_etl_analytics_service.create_pr_metrics(
-            pr_model, pr_events_model_list, pr_commits_model_list
+            pr_model, non_bot_pr_events_model_list, pr_commits_model_list
         )
 
         return pr_model, pr_events_model_list, pr_commits_model_list
@@ -354,6 +354,17 @@ def _dt_from_github_dt_string(dt_string: str) -> datetime:
         dt_without_timezone = datetime.strptime(dt_string, ISO_8601_DATE_FORMAT)
         return dt_without_timezone.replace(tzinfo=pytz.UTC)
 
+    @staticmethod
+    def _github_bot_filter(pr_events: List[PullRequestEvent]) -> List[PullRequestEvent]:
+        filtered_events = []
+        for pr_event in pr_events:
+            pr_event_data = (
+                pr_event.data.get("user") or pr_event.data.get("actor") or {}
+            )
+            if not (pr_event_data.get("type") == "Bot"):
+                filtered_events.append(pr_event)
+        return filtered_events
+
 
 def get_github_etl_handler(org_id: str) -> GithubETLHandler:
     def _get_access_token():

backend/analytics_server/mhq/utils/string.py

@@ -1,5 +1,13 @@
 from uuid import uuid4
+import re
 
 
 def uuid4_str():
     return str(uuid4())
+
+
+def is_bot_name(name: str) -> bool:
+    pattern = re.compile(
+        r"(?i)(\b[\w@-]*[-_\[\]@ ]+bot[-_\d\[\]]*\b|\[bot\]|_bot_|_bot$|^bot_)"
+    )
+    return bool(pattern.search(name))

backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py

@@ -502,129 +502,106 @@ def test_rework_cycles_returs_1_for_multiple_approvals():
     )
 
 
-def test_create_pr_metrics_filters_bot_events():
+def test_filter_non_bot_events_common_patterns():
     pr_service = CodeETLAnalyticsService()
-    t1 = time_now()
-    t2 = t1 + timedelta(hours=1)
-    t3 = t2 + timedelta(hours=1)
-    pr = get_pull_request(state=PullRequestState.MERGED, created_at=t1, updated_at=t1)
-    bot_event = get_pull_request_event(
-        pull_request_id=pr.id,
-        reviewer="test-bot[bot]",
-        state=PullRequestEventState.COMMENTED.value,
-        created_at=t2,
-    )
-    human_event = get_pull_request_event(
-        pull_request_id=pr.id,
-        reviewer="human_user",
-        state=PullRequestEventState.APPROVED.value,
-        created_at=t3,
-    )
-    pr_metrics = pr_service.create_pr_metrics(pr, [bot_event, human_event], [])
-    assert "human_user" in pr_metrics.reviewers
-    assert "test-bot[bot]" not in pr_metrics.reviewers
-
 
-def test_create_pr_metrics_no_bot_first_response_time():
+    bot_events = [
+        get_pull_request_event(reviewer="github-actions[bot]"),
+        get_pull_request_event(reviewer="jenkins-bot"),
+        get_pull_request_event(reviewer="renovate[bot]"),
+        get_pull_request_event(reviewer="test_bot_service"),
+        get_pull_request_event(reviewer="my_bot"),
+        get_pull_request_event(reviewer="bot_user"),
+        get_pull_request_event(reviewer="SomeService-bot-123"),
+        get_pull_request_event(reviewer="CI-BOT"),
+        get_pull_request_event(reviewer="bot-123[bot]"),
+        get_pull_request_event(reviewer="helper_bot_v2"),
+    ]
+    human_events = [
+        get_pull_request_event(reviewer="john_doe"),
+        get_pull_request_event(reviewer="robotics_expert"),
+        get_pull_request_event(reviewer="sabotage"),
+        get_pull_request_event(reviewer="lobotomy"),
+        get_pull_request_event(reviewer="cubot"),
+        get_pull_request_event(reviewer="botany"),
+    ]
+
+    all_events = bot_events + human_events
+    filtered_events = pr_service.filter_non_bot_events(all_events)
+
+    assert len(filtered_events) == len(human_events)
+
+    filtered_usernames = [e.actor_username for e in filtered_events]
+    for event in bot_events:
+        assert event.actor_username not in filtered_usernames
+
+    for event in human_events:
+        assert event.actor_username in filtered_usernames
+
+
+def test_filter_non_bot_events_edge_cases():
     pr_service = CodeETLAnalyticsService()
-    t1 = time_now()
-    t2 = t1 + timedelta(hours=1)
-    pr = get_pull_request(state=PullRequestState.MERGED, created_at=t1, updated_at=t1)
-    first_review_event = get_pull_request_event(
-        pull_request_id=pr.id,
-        reviewer="reviewer",
-        state=PullRequestEventState.COMMENTED.value,
-        created_at=t2,
-    )
-    pr_metrics = pr_service.create_pr_metrics(pr, [first_review_event], [])
-    assert pr_metrics.first_response_time == 3600
-
-
-def test_create_pr_metrics_no_bot_rework_time():
-    pr_service = CodeETLAnalyticsService()
-    t1 = time_now()
-    t2 = t1 + timedelta(hours=1)
-    t3 = t2 + timedelta(hours=1)
-    pr = get_pull_request(state=PullRequestState.MERGED, created_at=t1, updated_at=t1)
-    changes_requested_event = get_pull_request_event(
-        pull_request_id=pr.id,
-        reviewer="reviewer",
-        state=PullRequestEventState.CHANGES_REQUESTED.value,
-        created_at=t2,
-    )
-    approval_event = get_pull_request_event(
-        pull_request_id=pr.id,
-        reviewer="reviewer",
-        state=PullRequestEventState.APPROVED.value,
-        created_at=t3,
-    )
-    pr_metrics = pr_service.create_pr_metrics(
-        pr, [changes_requested_event, approval_event], []
-    )
-    assert pr_metrics.rework_time == 3600
 
+    events = [
+        get_pull_request_event(reviewer="test-bot[123]"),
+        get_pull_request_event(reviewer="deploy bot"),
+        get_pull_request_event(reviewer="special@bot@chars"),
+        get_pull_request_event(reviewer="robo"),
+        get_pull_request_event(reviewer="botanic"),
+        get_pull_request_event(reviewer="robot"),
+        get_pull_request_event(reviewer="abot"),
+    ]
 
-def test_create_pr_metrics_no_human_first_response_time():
-    pr_service = CodeETLAnalyticsService()
-    t1 = time_now()
-    t2 = t1 + timedelta(hours=1)
-    pr = get_pull_request(state=PullRequestState.MERGED, created_at=t1, updated_at=t1)
-    first_review_event = get_pull_request_event(
-        pull_request_id=pr.id,
-        reviewer="test-bot[bot]",
-        state=PullRequestEventState.COMMENTED.value,
-        created_at=t2,
-    )
-    pr_metrics = pr_service.create_pr_metrics(pr, [first_review_event], [])
-    assert pr_metrics.first_response_time is None
+    filtered_events = pr_service.filter_non_bot_events(events)
 
+    expected_remaining = ["robo", "botanic", "robot", "abot"]
+    filtered_usernames = [e.actor_username for e in filtered_events]
 
-def test_create_pr_metrics_no_human_rework_time():
-    pr_service = CodeETLAnalyticsService()
-    t1 = time_now()
-    t2 = t1 + timedelta(hours=1)
-    t3 = t2 + timedelta(hours=1)
-    pr = get_pull_request(state=PullRequestState.MERGED, created_at=t1, updated_at=t1)
-    changes_requested_event = get_pull_request_event(
-        pull_request_id=pr.id,
-        reviewer="test-bot[bot]",
-        state=PullRequestEventState.CHANGES_REQUESTED.value,
-        created_at=t2,
-    )
-    approval_event = get_pull_request_event(
-        pull_request_id=pr.id,
-        reviewer="test-bot[bot]",
-        state=PullRequestEventState.APPROVED.value,
-        created_at=t3,
-    )
-    pr_metrics = pr_service.create_pr_metrics(
-        pr, [changes_requested_event, approval_event], []
-    )
-    assert pr_metrics.rework_time is None
+    assert len(filtered_events) == 4
+    for username in expected_remaining:
+        assert username in filtered_usernames
 
 
-def test_create_pr_metrics_filters_bot_type_events():
+def test_create_pr_metrics_bot_detection_in_review_events():
     pr_service = CodeETLAnalyticsService()
     t1 = time_now()
-    t2 = t1 + timedelta(hours=1)
-    t3 = t2 + timedelta(hours=1)
     pr = get_pull_request(state=PullRequestState.MERGED, created_at=t1, updated_at=t1)
 
-    bot_event = get_pull_request_event(
-        pull_request_id=pr.id,
-        reviewer="github_app",
-        state=PullRequestEventState.COMMENTED.value,
-        created_at=t2,
-        data={"user": {"type": "Bot"}},
-    )
+    bot_reviewers = [
+        "github-actions[bot]",
+        "dependabot-preview[bot]",
+        "Jenkins_Bot",
+        "ci_bot_service",
+        "bot_reviewer",
+        "tool-bot-123",
+        "_bot_helper",
+        "helper_bot",
+    ]
+
+    bot_events = []
+    for i, reviewer in enumerate(bot_reviewers):
+        bot_events.append(
+            get_pull_request_event(
+                pull_request_id=pr.id,
+                reviewer=reviewer,
+                state=PullRequestEventState.COMMENTED.value,
+                created_at=t1 + timedelta(minutes=i + 1),
+            )
+        )
 
     human_event = get_pull_request_event(
         pull_request_id=pr.id,
-        reviewer="human_user",
+        reviewer="human_reviewer",
         state=PullRequestEventState.APPROVED.value,
-        created_at=t3,
+        created_at=t1 + timedelta(hours=1),
     )
 
-    pr_metrics = pr_service.create_pr_metrics(pr, [bot_event, human_event], [])
-    assert "human_user" in pr_metrics.reviewers
-    assert "github_app" not in pr_metrics.reviewers
+    all_events = bot_events + [human_event]
+
+    pr_metrics = pr_service.create_pr_metrics(pr, all_events, [])
+
+    assert len(pr_metrics.reviewers) == 1
+    assert "human_reviewer" in pr_metrics.reviewers
+
+    for bot_reviewer in bot_reviewers:
+        assert bot_reviewer not in pr_metrics.reviewers

backend/analytics_server/tests/service/code/sync/test_etl_github_handler.py

@@ -351,3 +351,32 @@ def test__dt_from_github_dt_string_given_date_string_returns_correct_datetime():
     date_string = "2024-04-18T10:53:15Z"
     expected = datetime(2024, 4, 18, 10, 53, 15, tzinfo=pytz.UTC)
     assert GithubETLHandler._dt_from_github_dt_string(date_string) == expected
+
+
+def test__github_bot_filter_given_bot_events_returns_empty_list():
+    github_etl_handler = GithubETLHandler(ORG_ID, None, None, None, None)
+    bot_event = get_pull_request_event()
+    bot_event.data = {"user": {"type": "Bot", "login": "dependabot"}}
+    result = github_etl_handler._github_bot_filter([bot_event])
+    assert len(result) == 0
+
+
+def test__github_bot_filter_given_non_bot_events_returns_same_list():
+    # Arrange
+    github_etl_handler = GithubETLHandler(ORG_ID, None, None, None, None)
+    human_event = get_pull_request_event()
+    human_event.data = {"user": {"type": "User", "login": "john_doe"}}
+    result = github_etl_handler._github_bot_filter([human_event])
+    assert len(result) == 1
+    assert result[0] == human_event
+
+
+def test__github_bot_filter_given_mixed_events_returns_non_bot_events():
+    github_etl_handler = GithubETLHandler(ORG_ID, None, None, None, None)
+    bot_event = get_pull_request_event()
+    bot_event.data = {"actor": {"type": "Bot", "login": "dependabot"}}
+    human_event = get_pull_request_event()
+    human_event.data = {"user": {"type": "User", "login": "john_doe"}}
+    result = github_etl_handler._github_bot_filter([bot_event, human_event])
+    assert len(result) == 1
+    assert result[0] == human_event

backend/analytics_server/tests/utils/string/test_is_bot_name.py

@@ -0,0 +1,48 @@
+
+
+from mhq.utils.string import is_bot_name
+
+def test_simple_bot_names():
+    assert is_bot_name("test_bot")
+    assert is_bot_name("test-bot")
+
+
+def test_bot_with_prefixes_and_suffixes():
+    assert is_bot_name("my_bot")
+    assert is_bot_name("my-bot")
+    assert is_bot_name("my bot")
+    assert is_bot_name("test_bot_123")
+    assert is_bot_name("test-bot-123")
+    assert is_bot_name("test bot 123")
+
+
+def test_special_patterns():
+    assert is_bot_name("name_bot_suffix") 
+    assert is_bot_name("name_bot")   
+    assert is_bot_name("bot_name")        
+    assert is_bot_name("my_bot_is_cool")  
+
+
+def test_case_insensitivity():
+    assert is_bot_name("my_BOT")
+    assert is_bot_name("MY-bot")
+    assert is_bot_name("My Bot")
+
+
+def test_special_characters():
+    assert is_bot_name("test@bot")
+    assert is_bot_name("[bot]")
+
+def test_negative_cases():
+    assert not is_bot_name("robotics")
+    assert not is_bot_name("lobotomy")
+    assert not is_bot_name("botany")
+    assert not is_bot_name("about")
+    assert not is_bot_name("robotic")
+    assert not is_bot_name("bots") 
+
+
+def test_edge_cases():
+    assert not is_bot_name("")
+    assert not is_bot_name(" ")
+    assert not is_bot_name("12345")