Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions .github/scripts/pull-request-dashboard/RATIONALE.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,13 @@ the implementation understandable and operationally cheap.
- Coalescing is safe because each refresh loads current PR state from GitHub.
Intermediate states can go unobserved, but the surviving run reflects the
state that exists when it executes.
- Submitted reviews use the review id as a distinct concurrency identity so a
pending review-guidance event cannot be replaced by a generic PR refresh.
Manual runs are also separate because they can refresh large repositories
that webhook-driven runs intentionally skip.
- Submitted reviews can coalesce with generic PR refreshes because the live PR
status comment is rendered from current accepted dashboard state rather than
a review-specific event. Manual runs remain separate because they can refresh
large repositories that webhook-driven runs intentionally skip.
Comment thread
trask marked this conversation as resolved.
- Concurrency bounds pending jobs per target; it does not debounce webhook
delivery or workflow dispatch. Different repositories, PRs, and submitted
review ids can still run independently, and every accepted webhook still
creates a workflow run.
delivery or workflow dispatch. Different repositories and PRs can still run
independently, and every accepted webhook still creates a workflow run.

## GitHub Actions Instead Of Netlify For Scheduled Backfills

Expand All @@ -65,8 +64,11 @@ the implementation understandable and operationally cheap.
do not contend on the same git ref during scheduled and webhook-driven runs.
- Updates use `git push --force-with-lease`, so git refs provide the durable
compare-and-swap boundary for concurrent same-repository runs.
- A missing repository state branch is bootstrapped by the next non-PR backfill;
targeted PR runs skip until initial dashboard state exists.
- A missing repository state branch is bootstrapped by non-PR backfills. The
dashboard state records when every open non-draft PR has been populated at
least once. Targeted PR runs, dashboard publishing, status comments, and
Slack notifications skip until that initial backfill is complete, so no
partial dashboard is exposed.
- Targeted PR runs compute the triggered PR and merge that one PR slot with the
latest accepted state on each state-branch compare-and-swap retry.

Expand All @@ -88,6 +90,11 @@ the implementation understandable and operationally cheap.
`backfill-state.json`. The cursor is the last successfully refreshed PR
number, and the next run continues after it in sorted PR-number order,
wrapping when needed.
- Initial-backfill completion is stored in dashboard state and becomes true in
the same accepted state commit that populates the final missing open
non-draft PR. Once set, it remains true. New PRs do not reset bootstrap; they
appear after their first targeted refresh or backfill, while existing
dashboard entries and Slack reminders continue normally.
- A selected PR failure stops the run without advancing the cursor. This can
temporarily block later PRs, but it keeps the scheduled workflow failure issue
open and pointing at the blocked backfill until the failure is fixed.
Expand Down
46 changes: 34 additions & 12 deletions .github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Repository permissions:
- Contents: read-only
- Issues: read and write
- Metadata: read-only
- Pull requests: read and write
- Pull requests: read-only

Organization permissions:

Expand All @@ -63,14 +63,14 @@ Permission rationale:
| ---------- | ------ | ---------------- |
| Checks | Read | Required to subscribe to check-suite events and to read check data for dashboard rows. |
| Contents | Read | Reads PR commits and repository metadata needed by pull/commit APIs. |
| Issues | Read and write | Finds/creates/updates the dashboard issue and reads existing PR conversation comments before posting review guidance. |
| Issues | Read and write | Finds/creates/updates the dashboard issue and creates or updates the dashboard-managed PR status comment. |
| Metadata | Read | Required by GitHub for GitHub App repository access. |
| Pull requests | Read and write | Required to subscribe to PR review/comment/thread events, read PR details, reviews, review comments, commits, and GraphQL review threads, and post review-guidance comments on PRs. |
| Pull requests | Read | Required to subscribe to PR review/comment/thread events and read PR details, reviews, review comments, commits, and GraphQL review threads. |
| Members | Read | Reads approver-team membership configured in `repositories.json`. |

The dashboard does not create inline review comments, submit reviews, or resolve
review threads. It does create PR conversation comments for review guidance,
which requires `Pull requests: read and write`.
review threads. It creates and updates one PR conversation comment through the
issues API, which requires `Issues: read and write`.

Subscribe to events:

Expand All @@ -87,8 +87,8 @@ Event rationale:
| ----- | ---------------- |
| Check suite | Refreshes CI status when checks are requested, rerequested, or completed. |
| Pull request | Refreshes dashboard rows when PR state, draft status, labels, assignees, branches, or metadata change. |
| Issue comment | Refreshes PR conversation state when PR issue comments are created, edited, or deleted. |
| Pull request review | Refreshes approval/change-request state and posts guidance for submitted reviews with review comments. |
| Issue comment | Refreshes PR conversation state when PR issue comments are created, edited, or deleted. Events generated by the dashboard App changing its own comments are ignored. |
| Pull request review | Refreshes approval/change-request state and the live PR status comment. |
| Pull request review comment | Refreshes inline review-comment discussion state. |
| Pull request review thread | Refreshes when inline review threads are resolved or unresolved. |

Expand Down Expand Up @@ -154,16 +154,38 @@ The webhook bridge should dispatch `pull-request-dashboard.yml` in
{
"repository": "opentelemetry-java-instrumentation",
"pr_number": "12345",
"trigger_event": "pull_request_review_comment",
"trigger_action": "created",
"trigger_review_id": "67890"
"trigger_event": "pull_request_review_comment"
}
```

Notes:

- `repository` is the short repository name under `open-telemetry`.
- Omit `pr_number` or set it to an empty string for a backfill.
- `trigger_review_id` is only required for `pull_request_review` `submitted`
events when review guidance may need to be posted.
- The central workflow validates these inputs before using them.

## 6. Post-rollout compatibility cleanup

The central workflow temporarily accepts the deprecated `trigger_action` and
`trigger_review_id` inputs so it remains compatible with the previously
deployed webhook bridge. The status-comment publisher also recognizes the
legacy `review-guidance` and `copilot-review-guidance` markers so it can upgrade
existing comments in place.

Remove those compatibility paths after completing this checklist:

1. Confirm the **Deploy pull request dashboard webhook** workflow completed
successfully on `main` for the commit containing the current dispatch
contract.
2. Remove the deprecated `trigger_action` and `trigger_review_id` inputs from
`pull-request-dashboard.yml`.
3. Dispatch one targeted dashboard refresh for each open PR with a
dashboard-App-authored legacy guidance comment. Hourly backfills do not
update PR status comments, so they do not perform this migration.
4. Verify that no open PR in a configured repository has a
dashboard-App-authored comment containing either legacy marker.
5. Remove `LEGACY_MARKERS` from `pr_status_comment.py`, leaving only
`STATUS_MARKER` as the managed-comment marker.

Closed PR comments do not need migration because they will not be updated
again.
2 changes: 1 addition & 1 deletion .github/scripts/pull-request-dashboard/classification.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def discussion_prompt_input(discussion: dict[str, Any]) -> dict[str, Any]:
prompt_thread = {
key: value
for key, value in discussion.items()
if key != "comments"
if key not in ("comments", "discussion_url")
}
prompt_thread["comments"] = [
{
Expand Down
75 changes: 72 additions & 3 deletions .github/scripts/pull-request-dashboard/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
--approver-team TEAM
[--approver-team TEAM]
[--pr-number N]
[--github-output PATH]
[--model NAME]

Architecture overview
Expand Down Expand Up @@ -114,6 +115,13 @@
or PR creation time.
waiting_age_basis str Which heuristic chose
waiting_since.
author_action_review_thread_urls
list[str] Canonical links to unresolved
inline review threads routed
to the author.
author_action_top_level_feedback_urls
list[str] Canonical links to top-level
feedback routed to the author.
reviewers list[dict] Reviewers to display (added by
add_reviewers). Each entry is
{"login": str, "approved": bool,
Expand Down Expand Up @@ -174,7 +182,9 @@
prune_classification_cache,
)
from state import (
INITIAL_BACKFILL_COMPLETE_KEY,
empty_state,
initial_backfill_complete,
load_dashboard_state_cache,
load_backfill_state,
results_from_dashboard_state,
Expand Down Expand Up @@ -338,6 +348,7 @@ def normalize_events(raw: dict[str, Any], author: str, reviewers: set[str]) -> l
timestamp = c.get("updated_at") or c.get("created_at") or ""
events.append({
"source_id": c.get("id"),
"discussion_url": c.get("html_url") or "",
"kind": "issue-comment",
"timestamp": timestamp,
"created_timestamp": c.get("created_at") or timestamp,
Expand Down Expand Up @@ -369,6 +380,7 @@ def normalize_events(raw: dict[str, Any], author: str, reviewers: set[str]) -> l
state = r.get("state") or ""
events.append({
"source_id": r.get("id"),
"discussion_url": r.get("url") or "",
"kind": "review-state",
"timestamp": r.get("submitted_at") or "",
"updated_timestamp": r.get("updated_at") or r.get("submitted_at") or "",
Expand Down Expand Up @@ -557,8 +569,9 @@ def group_review_threads(
# as live.
if discussion.get("isResolved") or discussion.get("isOutdated"):
continue
raw_comments = (discussion.get("comments") or {}).get("nodes") or []
comments = []
for c in ((discussion.get("comments") or {}).get("nodes") or []):
for c in raw_comments:
actor = reviewer_actor_login(c.get("author") or {})
comments.append(discussion_comment(
c.get("updatedAt") or c.get("createdAt") or "",
Expand All @@ -578,6 +591,7 @@ def group_review_threads(
"path": discussion.get("path"),
"line": discussion.get("line"),
"resolved": False,
"discussion_url": raw_comments[0].get("url") if raw_comments else "",
"comments": comments,
}, comments, facts))
discussions.sort(key=lambda t: t["comments"][-1]["timestamp"])
Expand Down Expand Up @@ -628,6 +642,7 @@ def derive_top_level_items(
"discussion_kind": "top-level-feedback",
"source_kind": source_kind,
"source_id": event["source_id"],
"discussion_url": event.get("discussion_url") or "",
"requester": comment["actor"],
"review_state": state or None,
"root_timestamp": root_timestamp,
Expand Down Expand Up @@ -900,6 +915,23 @@ def add_wait_age_facts(
facts["waiting_age_basis"] = basis


def author_action_discussion_urls(
discussions: list[dict[str, Any]],
pending_actions: dict[str, dict[str, Any]],
) -> list[str]:
by_id = discussions_by_id(discussions)
urls: list[str] = []
for discussion_id, entry in pending_actions.items():
action = normalize_discussion_action(entry.get("action") or "")
if action != "author":
continue
thread = by_id.get(discussion_id)
url = (thread or {}).get("discussion_url") or ""
if url and url not in urls:
urls.append(url)
return urls


# Discussion actions that count as open and unresolved. A reviewer who commented
# in such a discussion is not yet satisfied, even if they have approved.
# "none" means no follow-up is needed, so it does not block a clear check.
Expand Down Expand Up @@ -1052,6 +1084,12 @@ def build_pr_result(
}
route = route_pr(facts, pending_actions, required_approvals)
add_wait_age_facts(facts, route, pending_actions)
facts["author_action_review_thread_urls"] = author_action_discussion_urls(
review_threads, pending_actions
)
facts["author_action_top_level_feedback_urls"] = author_action_discussion_urls(
top_level_items, pending_actions
)
add_reviewers(
facts, events, review_threads, top_level_items, pending_actions
)
Expand Down Expand Up @@ -1210,6 +1248,18 @@ def dashboard_state_pr_numbers(state: dict[str, Any]) -> set[int]:
return numbers


def complete_initial_backfill_if_ready(
state: dict[str, Any],
open_pr_numbers: set[int],
) -> bool:
if initial_backfill_complete(state):
return False
if not open_pr_numbers.issubset(dashboard_state_pr_numbers(state)):
return False
state[INITIAL_BACKFILL_COMPLETE_KEY] = True
return True


def backfill_cursor_pr_number(backfill_state: dict[str, Any]) -> int | None:
cursor = backfill_state.get("cursor") or {}
if not isinstance(cursor, dict):
Expand Down Expand Up @@ -1507,6 +1557,7 @@ def update_dashboard_for_backfill(args: argparse.Namespace, state_dir: Path) ->
if not selection.selected_prs:
def save_current_dashboard_state() -> int:
dashboard_state = load_dashboard_state_cache() or empty_state()
complete_initial_backfill_if_ready(dashboard_state, open_non_draft_pr_numbers)
return save_dashboard_update_state(args, dashboard_state, False)

return state_branch.push_state_changes(
Expand Down Expand Up @@ -1538,10 +1589,14 @@ def update_selected_pr(pr_summary: dict[str, Any] = pr_summary) -> int:
)
if not dashboard_state_unchanged and reject_failed_dashboard_result(calculation.trigger_pr_result):
return 1
initial_backfill_completed = complete_initial_backfill_if_ready(
calculation.dashboard_state,
open_non_draft_pr_numbers,
)
status = save_dashboard_update_state(
args,
calculation.dashboard_state,
dashboard_state_unchanged,
dashboard_state_unchanged and not initial_backfill_completed,
)
if status != 0:
return status
Expand All @@ -1564,6 +1619,12 @@ def update_dashboard_via_state_branch(args: argparse.Namespace, state_dir: Path)
return update_dashboard_for_pr_number(args, state_dir)


def write_initial_backfill_output(github_output: Path) -> None:
complete = initial_backfill_complete(load_dashboard_state_cache())
with github_output.open("a", encoding="utf-8") as output:
output.write(f"initial_backfill_complete={'true' if complete else 'false'}\n")


def main() -> int:
parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
parser.add_argument(
Expand All @@ -1586,13 +1647,21 @@ def main() -> int:
help="minimum non-bot approvals needed before a PR can route to maintainers",
)
parser.add_argument("--model", default=DEFAULT_MODEL, help=f"copilot model (default: {DEFAULT_MODEL})")
parser.add_argument(
"--github-output",
type=Path,
help="append initial_backfill_complete to this GitHub Actions output file",
)
args = parser.parse_args()
if args.required_approvals < 1:
parser.error("--required-approvals must be at least 1")
with state_branch.temporary_state_dir() as state_dir:
repo_key = repo_state_key(args.repo) if args.repo else repo_state_key(detect_repo())
set_state_dir(state_dir / repo_key)
return update_dashboard_via_state_branch(args, state_dir)
status = update_dashboard_via_state_branch(args, state_dir)
if status == 0 and args.github_output:
write_initial_backfill_output(args.github_output)
return status


if __name__ == "__main__":
Expand Down
4 changes: 4 additions & 0 deletions .github/scripts/pull-request-dashboard/github_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ def gh_pr_view(repo: str, number: int) -> dict[str, Any]:
}
nodes {
fullDatabaseId
url
body
state
submittedAt
Expand Down Expand Up @@ -231,6 +232,7 @@ def fetch_pr_review_data(owner: str, repo_name: str, number: int) -> dict[str, A
continue
reviews.append({
"id": database_id,
"url": review.get("url") or "",
"user": review.get("author") or {},
"state": review.get("state") or "",
"body": review.get("body") or "",
Expand Down Expand Up @@ -341,6 +343,7 @@ def load_reviewer_set(org: str, approver_team_slugs: list[str]) -> set[str]:
}
nodes {
id
url
body
createdAt
updatedAt
Expand Down Expand Up @@ -377,6 +380,7 @@ def load_reviewer_set(org: str, approver_team_slugs: list[str]) -> set[str]:
}
nodes {
id
url
body
createdAt
updatedAt
Expand Down
Loading