Skip to content

Commit 8e4a5bc

Browse files
authored
PR dashboard: Fix PR author when Renovate (#18458)
1 parent 53c7c3a commit 8e4a5bc

1 file changed

Lines changed: 53 additions & 31 deletions

File tree

.github/scripts/pull-request-dashboard.py

Lines changed: 53 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,37 @@
4949
the pull request dashboard.
5050
5151
Decide who needs to act next on this PR using ONLY the context below. \
52-
Merge-conflict and CI-failure status are shown in separate columns of the \
53-
dashboard, so do NOT use those alone to decide; focus on the conversation.
54-
55-
Guidelines:
56-
- If the latest substantive activity is from the AUTHOR and there is an \
57-
outstanding approver question, an approver should respond next.
58-
- If an approver's most recent comment asks for a specific change and the \
59-
author has not responded, the author should act next.
52+
Merge-conflict status is shown in a separate deterministic column of the \
53+
dashboard — do not infer it. CI is summarized as a single boolean \
54+
(failing yes/no); pending checks are treated as not-failing. CI failure \
55+
on its own is NOT a reason to assign the PR to the author: \
56+
PRs can still be reviewed and approved while CI is failing. Treat CI \
57+
status only as weak supporting evidence and focus on the conversation \
58+
(comments, reviews, commits).
59+
60+
The single most important signal is the latest substantive event in the \
61+
timeline. Apply these rules in order (first match wins):
62+
63+
1. EXTERNAL — Use "external" when the conversation explicitly indicates \
64+
the PR is blocked on something outside this repo (e.g., links to an \
65+
upstream PR/issue, "reported at <other-repo>", a spec change, or a \
66+
release in another project). Look especially at the latest comments. \
67+
A new PR with no reviews yet is NOT external. CI failing alone is \
68+
NOT external unless an upstream cause is named.
69+
2. AUTHOR — If the latest substantive event is an approver review or \
70+
review-comment with content (a question, suggestion, change \
71+
request, clarification ask, or [APPROVED/CHANGES_REQUESTED] state) \
72+
and the AUTHOR has not posted any comment, review, or commit AFTER \
73+
it, the AUTHOR should act next. This holds even when the comment \
74+
is just a question or a soft suggestion — the ball is in the \
75+
author's court until they respond. (Note: a *commit* by an approver \
76+
does not count here — that's an approver pushing a fix, not asking \
77+
the author for something.)
78+
3. APPROVER — Otherwise, an APPROVER should act next. This includes: \
79+
fresh PRs with no reviews yet; PRs where the author has posted the \
80+
latest substantive event (comment, review, or commit) addressing \
81+
prior approver feedback; and PRs where an approver pushed the \
82+
latest commit (waiting for someone to review/merge).
6083
6184
Respond with a single JSON object and nothing else (no prose, no fences):
6285
{{"side": "approver" | "author" | "external"}}
@@ -250,6 +273,12 @@ def role_for(login: str, author: str, reviewers: set[str]) -> str:
250273
# fix-up) pass through and are eligible to be picked as the delegator.
251274
_BOT_COMMITTER_LOGINS = {"copilot"}
252275

276+
# PR author logins that delegate work to a human (the Copilot SWE-agent
277+
# opens the PR but a human triggered it). Only for these authors do we look
278+
# up a human delegator from the first commit's committer. For other bots
279+
# (renovate, dependabot, etc.) we keep the bot as the author.
280+
_DELEGATING_BOT_AUTHORS = {"app/copilot-swe-agent", "copilot"}
281+
253282

254283
def _is_bot_login(login: str) -> bool:
255284
if not login:
@@ -312,16 +341,18 @@ def fetch_pr_context(
312341

313342
# For Copilot SWE-agent PRs the API author is the bot; surface the human
314343
# who delegated the task so reviews/comments by that person are classified
315-
# as "author" activity instead of "approver".
344+
# as "author" activity instead of "approver". Other bot authors
345+
# (renovate, dependabot, ...) have no human delegator, so we keep the bot.
316346
delegator = ""
317-
if _is_bot_login(author):
347+
if author.lower() in _DELEGATING_BOT_AUTHORS:
318348
delegator = detect_human_delegator(commits)
319349
if delegator:
320350
author = delegator
321351

322352
# Fetch per-commit diffs for the most recent commits, in parallel.
323353
recent_commits = commits[-MAX_COMMITS:]
324354
patches: dict[str, str] = {}
355+
merge_shas: set[str] = set()
325356
if recent_commits:
326357
with ThreadPoolExecutor(max_workers=4) as pool:
327358
futs = {
@@ -335,6 +366,8 @@ def fetch_pr_context(
335366
except Exception:
336367
detail = {}
337368
patches[sha] = format_commit_patch(detail, MAX_COMMIT_DIFF_CHARS)
369+
if len(detail.get("parents") or []) >= 2:
370+
merge_shas.add(sha)
338371

339372
# Build unified activity timeline.
340373
events: list[dict[str, Any]] = []
@@ -356,6 +389,7 @@ def fetch_pr_context(
356389
"login": login,
357390
"body": body,
358391
"sha": sha_full[:7],
392+
"is_merge": sha_full in merge_shas,
359393
})
360394
for c in issue_comments:
361395
events.append({
@@ -383,10 +417,14 @@ def fetch_pr_context(
383417
events.sort(key=lambda e: e["ts"])
384418

385419
# Last substantive event = last event whose body is non-empty OR whose
386-
# kind is not "review:COMMENTED" (state changes always count).
420+
# kind is not "review:COMMENTED" (state changes always count). Merge
421+
# commits (≥2 parents — e.g. "Update branch" merging base into the PR)
422+
# don't count as substantive: they don't move the conversation forward.
387423
def is_substantive(e: dict[str, Any]) -> bool:
388424
if e["kind"].startswith("review:") and e["kind"] != "review:COMMENTED":
389425
return True
426+
if e.get("is_merge"):
427+
return False
390428
return bool((e.get("body") or "").strip())
391429

392430
substantive = [e for e in events if is_substantive(e)]
@@ -481,9 +519,11 @@ def render_context(ctx: dict[str, Any]) -> str:
481519
f"@{author} is treated as the effective author for triage.)"
482520
)
483521
lines.append(
484-
f"State: open | draft={pr.get('isDraft')} | mergeable={pr.get('mergeable')} "
485-
f"| mergeStateStatus={pr.get('mergeStateStatus')} | reviewDecision={pr.get('reviewDecision')}"
522+
f"State: open | draft={pr.get('isDraft')} "
523+
f"| reviewDecision={pr.get('reviewDecision')}"
486524
)
525+
ci_failing = bool(ctx["checks_failing"])
526+
lines.append(f"CI failing: {'yes' if ci_failing else 'no'}")
487527
lines.append(f"Created: {pr.get('createdAt')} ({pr_age}d ago)")
488528
lines.append(f"Updated: {pr.get('updatedAt')} ({updated_age}d ago)")
489529
lines.append(f"Size: +{pr.get('additions', 0)}/-{pr.get('deletions', 0)} across {pr.get('changedFiles', 0)} files")
@@ -500,18 +540,6 @@ def render_context(ctx: dict[str, Any]) -> str:
500540
lines.append(truncate(pr.get("body") or "", 800))
501541
lines.append("")
502542

503-
# Checks
504-
lines.append("CI checks summary:")
505-
lines.append(
506-
f" successful={len(ctx['checks_successful'])} failing={len(ctx['checks_failing'])} "
507-
f"pending={len(ctx['checks_pending'])} skipped={len(ctx['checks_skipped'])}"
508-
)
509-
for c in ctx["checks_failing"][:10]:
510-
lines.append(f" FAIL: {c.get('name')} ({c.get('workflow') or ''})")
511-
for c in ctx["checks_pending"][:5]:
512-
lines.append(f" PENDING: {c.get('name')}")
513-
lines.append("")
514-
515543
# Commits
516544
lines.append(f"Last {len(ctx['commits'])} commits (oldest first):")
517545
for c in ctx["commits"]:
@@ -561,12 +589,6 @@ def render_context(ctx: dict[str, Any]) -> str:
561589
signals: list[str] = []
562590
if pr.get("isDraft"):
563591
signals.append("PR is a draft")
564-
if pr.get("mergeable") == "CONFLICTING" or pr.get("mergeStateStatus") == "DIRTY":
565-
signals.append("merge conflict with base")
566-
if ctx["checks_failing"]:
567-
signals.append(f"{len(ctx['checks_failing'])} CI checks failing")
568-
if pr.get("reviewDecision") == "APPROVED" and not ctx["checks_failing"] and pr.get("mergeStateStatus") == "CLEAN":
569-
signals.append("approved + clean + green = mergeable")
570592
if last_role == "author" and ctx["approvers"]:
571593
signals.append("latest substantive activity is from author after approvals")
572594
if last_role == "approver" and last_sub:

0 commit comments

Comments
 (0)