This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new c682ae0b5ab Add unresolved review comments check to auto-triage
(#63294)
c682ae0b5ab is described below
commit c682ae0b5abcd1d782286c4900ac78897d768ed9
Author: Jarek Potiuk <[email protected]>
AuthorDate: Tue Mar 10 21:52:38 2026 +0100
Add unresolved review comments check to auto-triage (#63294)
Add a new deterministic check that flags PRs with unresolved inline
review threads from maintainers. This helps ensure contributors
address reviewer feedback before PRs are re-reviewed.
Co-authored-by: Claude Opus 4.6 <[email protected]>
---
.../src/airflow_breeze/commands/pr_commands.py | 99 ++++++++++++++++++++--
dev/breeze/src/airflow_breeze/utils/github.py | 32 +++++++
2 files changed, 124 insertions(+), 7 deletions(-)
diff --git a/dev/breeze/src/airflow_breeze/commands/pr_commands.py
b/dev/breeze/src/airflow_breeze/commands/pr_commands.py
index 288dbb7e7c0..6ee27349bad 100644
--- a/dev/breeze/src/airflow_breeze/commands/pr_commands.py
+++ b/dev/breeze/src/airflow_breeze/commands/pr_commands.py
@@ -257,6 +257,7 @@ class PRData:
commits_behind: int # how many commits behind the base branch
mergeable: str # MERGEABLE, CONFLICTING, or UNKNOWN
labels: list[str] # label names attached to this PR
+ unresolved_review_comments: int # count of unresolved review threads from
maintainers
@click.group(cls=BreezeGroup, name="pr", help="Tools for managing GitHub pull
requests.")
@@ -485,6 +486,70 @@ def _fetch_check_details_batch(token: str,
github_repository: str, prs: list[PRD
pr.failed_checks = failed
+_REVIEW_THREADS_BATCH_SIZE = 10
+
+
+def _fetch_unresolved_comments_batch(token: str, github_repository: str, prs:
list[PRData]) -> None:
+ """Fetch unresolved review thread counts for PRs in chunked GraphQL
queries.
+
+ Counts only threads started by collaborators/members/owners (i.e.
maintainers).
+ Updates each PR's unresolved_review_comments in-place.
+ """
+ owner, repo = github_repository.split("/", 1)
+ if not prs:
+ return
+
+ for chunk_start in range(0, len(prs), _REVIEW_THREADS_BATCH_SIZE):
+ chunk = prs[chunk_start : chunk_start + _REVIEW_THREADS_BATCH_SIZE]
+
+ pr_fields = []
+ for pr in chunk:
+ alias = f"pr{pr.number}"
+ pr_fields.append(
+ f" {alias}: pullRequest(number: {pr.number}) {{\n"
+ f" reviewThreads(first: 100) {{\n"
+ f" nodes {{\n"
+ f" isResolved\n"
+ f" comments(first: 1) {{\n"
+ f" nodes {{\n"
+ f" author {{ login }}\n"
+ f" authorAssociation\n"
+ f" }}\n"
+ f" }}\n"
+ f" }}\n"
+ f" }}\n"
+ f" }}"
+ )
+
+ query = (
+ f'query {{\n repository(owner: "{owner}", name: "{repo}") {{\n'
+ + "\n".join(pr_fields)
+ + "\n }\n}"
+ )
+
+ try:
+ data = _graphql_request(token, query, {})
+ except SystemExit:
+ continue
+
+ repo_data = data.get("repository", {})
+ for pr in chunk:
+ alias = f"pr{pr.number}"
+ pr_data = repo_data.get(alias) or {}
+ threads = pr_data.get("reviewThreads", {}).get("nodes", [])
+ unresolved = 0
+ for thread in threads:
+ if thread.get("isResolved"):
+ continue
+ # Only count threads started by maintainers
(collaborators/members/owners)
+ comments = thread.get("comments", {}).get("nodes", [])
+ if comments:
+ assoc = comments[0].get("authorAssociation", "NONE")
+ if assoc in _COLLABORATOR_ASSOCIATIONS:
+ unresolved += 1
+ pr.unresolved_review_comments = unresolved
+
+
def _fetch_commits_behind_batch(token: str, github_repository: str, prs:
list[PRData]) -> dict[int, int]:
"""Fetch how many commits each PR is behind its base branch in chunked
GraphQL queries.
@@ -586,6 +651,7 @@ def _fetch_prs_graphql(
commits_behind=0,
mergeable=node.get("mergeable", "UNKNOWN"),
labels=[lbl["name"] for lbl in (node.get("labels") or
{}).get("nodes", []) if lbl],
+ unresolved_review_comments=0,
)
)
@@ -623,6 +689,7 @@ def _fetch_single_pr_graphql(token: str, github_repository:
str, pr_number: int)
commits_behind=0,
mergeable=node.get("mergeable", "UNKNOWN"),
labels=[lbl["name"] for lbl in (node.get("labels") or {}).get("nodes",
[]) if lbl],
+ unresolved_review_comments=0,
)
@@ -1248,7 +1315,12 @@ def auto_triage(
llm_model: str,
answer_triage: str | None,
):
- from airflow_breeze.utils.github import PRAssessment, assess_pr_checks,
assess_pr_conflicts
+ from airflow_breeze.utils.github import (
+ PRAssessment,
+ assess_pr_checks,
+ assess_pr_conflicts,
+ assess_pr_unresolved_comments,
+ )
from airflow_breeze.utils.llm_utils import (
_check_cli_available,
_resolve_cli_provider,
@@ -1400,7 +1472,16 @@ def auto_triage(
)
pr.failed_checks = _fetch_failed_checks(token,
github_repository, pr.head_sha)
- # Phase 3: Deterministic checks (CI failures + merge conflicts), then LLM
for the rest
+ # Phase 2c: Fetch unresolved review comment counts for candidate PRs
+ if candidate_prs and run_ci:
+ get_console().print(
+ f"[info]Fetching review thread details for {len(candidate_prs)} "
+ f"candidate {'PRs' if len(candidate_prs) != 1 else 'PR'}...[/]"
+ )
+ _fetch_unresolved_comments_batch(token, github_repository,
candidate_prs)
+
+ # Phase 3: Deterministic checks (CI failures + merge conflicts +
unresolved comments),
+ # then LLM for the rest
# PRs with NOT_RUN checks are separated for workflow approval instead of
LLM assessment.
assessments: dict[int, PRAssessment] = {}
llm_candidates: list[PRData] = []
@@ -1411,9 +1492,10 @@ def auto_triage(
for pr in candidate_prs:
ci_assessment = assess_pr_checks(pr.number, pr.checks_state,
pr.failed_checks)
conflict_assessment = assess_pr_conflicts(pr.number, pr.mergeable,
pr.base_ref, pr.commits_behind)
+ comments_assessment = assess_pr_unresolved_comments(pr.number,
pr.unresolved_review_comments)
- # Merge violations from both deterministic checks
- if ci_assessment or conflict_assessment:
+ # Merge violations from all deterministic checks
+ if ci_assessment or conflict_assessment or comments_assessment:
total_deterministic_flags += 1
violations = []
summaries = []
@@ -1423,6 +1505,9 @@ def auto_triage(
if ci_assessment:
violations.extend(ci_assessment.violations)
summaries.append(ci_assessment.summary)
+ if comments_assessment:
+ violations.extend(comments_assessment.violations)
+ summaries.append(comments_assessment.summary)
assessments[pr.number] = PRAssessment(
should_flag=True,
violations=violations,
@@ -1449,7 +1534,7 @@ def auto_triage(
f"{'PRs' if len(llm_candidates) != 1 else 'PR'}.[/]\n"
)
elif llm_candidates:
- skipped_detail = f"{total_deterministic_flags} CI/conflicts"
+ skipped_detail = f"{total_deterministic_flags} CI/conflicts/comments"
if pending_approval:
skipped_detail += f", {len(pending_approval)} awaiting workflow
approval"
get_console().print(
@@ -1481,7 +1566,7 @@ def auto_triage(
total_flagged = len(assessments)
summary_parts = [
- f"{total_deterministic_flags} CI/conflicts",
+ f"{total_deterministic_flags} CI/conflicts/comments",
f"{total_flagged - total_deterministic_flags} LLM-flagged",
]
if pending_approval:
@@ -1795,7 +1880,7 @@ def auto_triage(
summary_table.add_row("Ready-for-review skipped",
str(total_skipped_accepted))
summary_table.add_row("PRs skipped (filtered)", str(total_skipped))
summary_table.add_row("PRs assessed", str(len(candidate_prs)))
- summary_table.add_row("Flagged by CI/conflicts",
str(total_deterministic_flags))
+ summary_table.add_row("Flagged by CI/conflicts/comments",
str(total_deterministic_flags))
summary_table.add_row("Flagged by LLM", str(total_flagged -
total_deterministic_flags))
summary_table.add_row("LLM errors (skipped)", str(total_llm_errors))
summary_table.add_row("Total flagged", str(total_flagged))
diff --git a/dev/breeze/src/airflow_breeze/utils/github.py
b/dev/breeze/src/airflow_breeze/utils/github.py
index 60a77cedb70..a1a935048f3 100644
--- a/dev/breeze/src/airflow_breeze/utils/github.py
+++ b/dev/breeze/src/airflow_breeze/utils/github.py
@@ -587,3 +587,35 @@ def assess_pr_conflicts(
],
summary=f"PR #{pr_number} has merge conflicts.",
)
+
+
+def assess_pr_unresolved_comments(pr_number: int, unresolved_review_comments:
int) -> PRAssessment | None:
+ """Deterministically flag a PR if it has unresolved review comments from
maintainers.
+
+ Returns None if there are no unresolved comments.
+ """
+ if unresolved_review_comments <= 0:
+ return None
+
+ thread_word = "thread" if unresolved_review_comments == 1 else "threads"
+ return PRAssessment(
+ should_flag=True,
+ violations=[
+ Violation(
+ category="Unresolved review comments",
+ explanation=(
+ f"This PR has {unresolved_review_comments} unresolved
review "
+ f"{thread_word} from maintainers."
+ ),
+ severity="warning",
+ details=(
+ "Please review and resolve all inline review comments
before requesting "
+ "another review. You can resolve a conversation by
clicking 'Resolve conversation' "
+ "on each thread after addressing the feedback. "
+ f"See [pull request guidelines]"
+ f"({_CONTRIBUTING_DOCS_URL}/05_pull_requests.rst)."
+ ),
+ )
+ ],
+ summary=f"PR #{pr_number} has {unresolved_review_comments} unresolved
review {thread_word}.",
+ )
