From eebe2932f35d6e37b928912a3b61255eeb8f3535 Mon Sep 17 00:00:00 2001 From: Gaurav Pooniwala Date: Tue, 30 Jun 2026 14:11:44 +0100 Subject: [PATCH] Normalize GitHub App actor names --- online/etl/pipeline/actors.py | 19 ++++ online/etl/pipeline/analyze.py | 43 ++++---- online/etl/pipeline/assemble.py | 11 +- online/etl/pipeline/quality.py | 14 +-- online/etl/tests/test_actor_normalization.py | 107 +++++++++++++++++++ 5 files changed, 161 insertions(+), 33 deletions(-) create mode 100644 online/etl/pipeline/actors.py create mode 100644 online/etl/tests/test_actor_normalization.py diff --git a/online/etl/pipeline/actors.py b/online/etl/pipeline/actors.py new file mode 100644 index 0000000..deedb20 --- /dev/null +++ b/online/etl/pipeline/actors.py @@ -0,0 +1,19 @@ +"""Helpers for comparing GitHub actor usernames across API sources.""" + +from __future__ import annotations + + +def normalize_github_actor(username: str | None) -> str: + """Canonicalize GitHub actor names for equality checks. + + GitHub App bot accounts can appear as either ``name[bot]`` or ``name`` + depending on the API/source object. Treat those forms as the same actor. + """ + return (username or "").strip().lower().removesuffix("[bot]") + + +def same_github_actor(left: str | None, right: str | None) -> bool: + """Return whether two GitHub actor names represent the same account/app.""" + left_norm = normalize_github_actor(left) + right_norm = normalize_github_actor(right) + return bool(left_norm and right_norm) and left_norm == right_norm diff --git a/online/etl/pipeline/analyze.py b/online/etl/pipeline/analyze.py index cc1b30d..953a6aa 100644 --- a/online/etl/pipeline/analyze.py +++ b/online/etl/pipeline/analyze.py @@ -18,6 +18,7 @@ from llm.schemas import BotSuggestionsResponse from llm.schemas import HumanActionsResponse from llm.schemas import MatchingResponse +from pipeline.actors import same_github_actor logger = logging.getLogger(__name__) @@ -35,30 +36,29 @@ def _find_bot_review_commit( 2. Fallback: original_commit_id on review_comment events in assembled timeline 3. Last resort: last commit before bot's first comment timestamp """ - bot_user_lower = chatbot_username.lower() - # Strategy 1: raw reviews for r in reviews: - author = (r.get("author") or r.get("user", {}).get("login", "")).lower() - if author == bot_user_lower and r.get("commit_id"): + author = r.get("author") or r.get("user", {}).get("login", "") + if same_github_actor(author, chatbot_username) and r.get("commit_id"): return r["commit_id"] # Strategy 2: review_comment events with original_commit_id for e in events: - if e.get("event_type") == "review_comment": - actor = (e.get("actor") or "").lower() - if actor == bot_user_lower: - data = e.get("data", {}) - if data.get("original_commit_id"): - return data["original_commit_id"] - if data.get("commit_id"): - return data["commit_id"] + if e.get("event_type") == "review_comment" and same_github_actor(e.get("actor"), chatbot_username): + data = e.get("data", {}) + if data.get("original_commit_id"): + return data["original_commit_id"] + if data.get("commit_id"): + return data["commit_id"] # Strategy 3: last commit before bot's first comment timestamp bot_first_ts = None for e in events: - actor = (e.get("actor") or "").lower() - if actor == bot_user_lower and e.get("event_type") in ("review", "review_comment", "issue_comment"): + if same_github_actor(e.get("actor"), chatbot_username) and e.get("event_type") in ( + "review", + "review_comment", + "issue_comment", + ): bot_first_ts = e.get("timestamp") break @@ -148,11 +148,9 @@ def _format_bot_comments(events: list[dict], chatbot_username: str) -> str: Skips review_comment replies (in_reply_to_id set) — these are responses to other commenters' threads, not original review suggestions. """ - bot_user_lower = chatbot_username.lower() lines = [] for e in events: - actor = (e.get("actor") or "").lower() - if actor != bot_user_lower: + if not same_github_actor(e.get("actor"), chatbot_username): continue etype = e.get("event_type", "") if etype not in ("review", "review_comment", "issue_comment"): @@ -194,7 +192,6 @@ def _format_post_review_activity( hash_x: str | None, # noqa: ARG001 ) -> str: """Format post-review commits with diffs + all human comments/replies after bot review.""" - bot_user_lower = chatbot_username.lower() sections = [] # Post-review commits with diffs @@ -206,16 +203,18 @@ def _format_post_review_activity( # We use the bot's first comment as the cutoff for "after bot review" bot_first_ts = None for e in events: - actor = (e.get("actor") or "").lower() - if actor == bot_user_lower and e.get("event_type") in ("review", "review_comment", "issue_comment"): + if same_github_actor(e.get("actor"), chatbot_username) and e.get("event_type") in ( + "review", + "review_comment", + "issue_comment", + ): bot_first_ts = e.get("timestamp") break # All human activity after bot review human_lines = [] for e in events: - actor = (e.get("actor") or "").lower() - if actor == bot_user_lower: + if same_github_actor(e.get("actor"), chatbot_username): continue ts = e.get("timestamp", "") etype = e.get("event_type", "") diff --git a/online/etl/pipeline/assemble.py b/online/etl/pipeline/assemble.py index a73193a..8fca3ed 100644 --- a/online/etl/pipeline/assemble.py +++ b/online/etl/pipeline/assemble.py @@ -16,6 +16,7 @@ from db.connection import DBAdapter from db.repository import PRRepository +from pipeline.actors import same_github_actor logger = logging.getLogger(__name__) @@ -400,12 +401,14 @@ def _compute_stats(target_user: str, timeline: list[TimelineEvent], threads: lis stats.total_events = len(timeline) stats.total_commits = sum(1 for e in timeline if e.event_type == "commit") stats.total_review_comments_by_target = sum( - 1 for e in timeline if e.event_type == "review_comment" and e.actor == target_user + 1 for e in timeline if e.event_type == "review_comment" and same_github_actor(e.actor, target_user) ) stats.total_review_threads = len(threads) stats.resolved_threads = sum(1 for t in threads if t.is_resolved) stats.target_user_comments_count = sum( - 1 for e in timeline if e.actor == target_user and e.event_type in ("review_comment", "issue_comment", "review") + 1 + for e in timeline + if same_github_actor(e.actor, target_user) and e.event_type in ("review_comment", "issue_comment", "review") ) return stats @@ -413,10 +416,10 @@ def _compute_stats(target_user: str, timeline: list[TimelineEvent], threads: lis def _determine_roles(target_user: str, timeline: list[TimelineEvent], pr_author: str | None) -> list[str]: """Determine what roles the target user played in this PR.""" roles: set[str] = set() - if pr_author == target_user: + if same_github_actor(pr_author, target_user): roles.add("author") for e in timeline: - if e.actor != target_user: + if not same_github_actor(e.actor, target_user): continue if e.event_type in ("review", "review_comment"): roles.add("reviewer") diff --git a/online/etl/pipeline/quality.py b/online/etl/pipeline/quality.py index baebb8c..ee35942 100644 --- a/online/etl/pipeline/quality.py +++ b/online/etl/pipeline/quality.py @@ -21,6 +21,8 @@ from typing import Any from config import DEFAULT_CHATBOT_USERNAMES +from pipeline.actors import normalize_github_actor +from pipeline.actors import same_github_actor logger = logging.getLogger(__name__) @@ -60,12 +62,11 @@ def is_bot_username(username: str) -> bool: def _find_bot_first_review_ts( events: list[dict[str, Any]], - bot_lower: str, + chatbot_username: str, ) -> str | None: """Find the timestamp of the bot's first review/comment event.""" for e in events: - actor = (e.get("actor") or "").lower() - if actor == bot_lower and e.get("event_type") in _REVIEW_EVENT_TYPES: + if same_github_actor(e.get("actor"), chatbot_username) and e.get("event_type") in _REVIEW_EVENT_TYPES: return e.get("timestamp") return None @@ -87,8 +88,7 @@ def compute_engagement_signals( "self-dealing" (only the PR author engaged with the review). """ events = assembled.get("events", []) - bot_lower = chatbot_username.lower() - bot_first_ts = _find_bot_first_review_ts(events, bot_lower) + bot_first_ts = _find_bot_first_review_ts(events, chatbot_username) if bot_first_ts is None: return { @@ -120,10 +120,10 @@ def compute_engagement_signals( if not actor: continue - actor_lower = actor.lower() + actor_lower = normalize_github_actor(actor) etype = e.get("event_type", "") - if actor_lower == bot_lower: + if same_github_actor(actor, chatbot_username): # Bot activity after human response = start of new round if etype in _REVIEW_EVENT_TYPES and last_phase == "human": back_and_forth_rounds += 1 diff --git a/online/etl/tests/test_actor_normalization.py b/online/etl/tests/test_actor_normalization.py new file mode 100644 index 0000000..6f34f17 --- /dev/null +++ b/online/etl/tests/test_actor_normalization.py @@ -0,0 +1,107 @@ +"""Tests for GitHub App actor normalization across benchmark pipeline stages.""" + +from __future__ import annotations + +from pipeline.actors import normalize_github_actor +from pipeline.actors import same_github_actor +from pipeline.analyze import _find_bot_review_commit +from pipeline.analyze import _format_bot_comments +from pipeline.analyze import _format_post_review_activity +from pipeline.assemble import TimelineEvent +from pipeline.assemble import _compute_stats +from pipeline.assemble import _determine_roles +from pipeline.quality import compute_engagement_signals + + +def test_normalizes_bot_suffix_for_github_app_actors() -> None: + assert normalize_github_actor("Cubic-Dev-AI[bot]") == "cubic-dev-ai" + assert same_github_actor("cubic-dev-ai", "cubic-dev-ai[bot]") + assert not same_github_actor("cubic-dev-ai", "coderabbitai[bot]") + + +def test_analyze_includes_graphql_thread_comments_from_bot_slug() -> None: + events = [ + { + "timestamp": "2026-06-18T10:00:00Z", + "event_type": "review_comment", + "actor": "cubic-dev-ai", + "data": {"body": "Fix the missing null check.", "path": "main.py", "line": 42}, + } + ] + + formatted = _format_bot_comments(events, "cubic-dev-ai[bot]") + + assert "Fix the missing null check." in formatted + assert "main.py:42" in formatted + + +def test_analyze_excludes_bot_slug_from_human_activity() -> None: + events = [ + { + "timestamp": "2026-06-18T10:00:00Z", + "event_type": "review_comment", + "actor": "cubic-dev-ai", + "data": {"body": "Initial bot review."}, + }, + { + "timestamp": "2026-06-18T10:05:00Z", + "event_type": "review_comment", + "actor": "alice", + "data": {"body": "I pushed the fix."}, + }, + { + "timestamp": "2026-06-18T10:06:00Z", + "event_type": "review_comment", + "actor": "cubic-dev-ai", + "data": {"body": "Bot follow-up should not be human activity."}, + }, + ] + + formatted = _format_post_review_activity([], {}, events, "cubic-dev-ai[bot]", None) + + assert "I pushed the fix." in formatted + assert "Bot follow-up should not be human activity." not in formatted + + +def test_find_bot_review_commit_matches_slug_without_bot_suffix() -> None: + reviews = [{"author": "cubic-dev-ai", "commit_id": "abc123"}] + + assert _find_bot_review_commit(reviews, [], [], "cubic-dev-ai[bot]") == "abc123" + + +def test_assemble_stats_and_roles_match_slug_without_bot_suffix() -> None: + timeline = [ + TimelineEvent("2026-06-18T10:00:00Z", "review_comment", "cubic-dev-ai"), + TimelineEvent("2026-06-18T10:05:00Z", "issue_comment", "alice"), + ] + + stats = _compute_stats("cubic-dev-ai[bot]", timeline, []) + roles = _determine_roles("cubic-dev-ai[bot]", timeline, "alice") + + assert stats.total_review_comments_by_target == 1 + assert stats.target_user_comments_count == 1 + assert roles == ["reviewer"] + + +def test_engagement_signals_start_after_bot_slug_review() -> None: + assembled = { + "events": [ + { + "timestamp": "2026-06-18T10:00:00Z", + "event_type": "review_comment", + "actor": "cubic-dev-ai", + "data": {"body": "Please fix this."}, + }, + { + "timestamp": "2026-06-18T10:05:00Z", + "event_type": "issue_comment", + "actor": "alice", + "data": {"body": "Fixed."}, + }, + ], + } + + signals = compute_engagement_signals(assembled, "cubic-dev-ai[bot]", pr_author="bob") + + assert signals["has_human_engagement"] is True + assert signals["human_comment_count"] == 1