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 ab918c9260a Auto-triage: flag area:UI PRs missing screenshots/demo
from new contributors (#64516)
ab918c9260a is described below
commit ab918c9260a73f7fc7c39cdd5b86b90ab7ad4a76
Author: Shivam Rastogi <[email protected]>
AuthorDate: Mon Apr 6 01:46:37 2026 -0700
Auto-triage: flag area:UI PRs missing screenshots/demo from new
contributors (#64516)
* Add UI demo/screenshot check to auto-triage for area:UI PRs
Add a deterministic check to `breeze pr auto-triage` that flags
`area:UI` PRs from non-collaborator contributors when their PR
description lacks screenshots or demo videos.
The check runs as part of the existing deterministic checks in
`_assess_pr_deterministic()` alongside CI failure, merge conflict,
and unresolved comment checks. PRs flagged only for missing UI demo
receive a comment (not draft conversion), using a new soft-violation
branch in `_compute_default_action`.
Detection patterns cover GitHub drag-and-drop uploads (`<img>` tags,
`user-attachments/assets/` URLs), markdown image syntax, and direct
media file URLs (png, jpg, gif, mp4, etc).
* Clarify that demo should be in PR description, not comments
* Remove soft-violation special-case in _compute_default_action
Per reviewer feedback from @pierrejeambrun and @potiuk: the
separate elif branch for soft-only violations (e.g. missing UI
demo) defaulting to COMMENT is unnecessary — maintainers can
override DRAFT to COMMENT interactively. A future enhancement
will let each check define its own suggested action severity.
---------
Co-authored-by: Jarek Potiuk <[email protected]>
---
.../src/airflow_breeze/commands/pr_commands.py | 15 +-
dev/breeze/src/airflow_breeze/utils/github.py | 67 ++++++
dev/breeze/tests/test_github_ui_demo.py | 224 +++++++++++++++++++++
3 files changed, 303 insertions(+), 3 deletions(-)
diff --git a/dev/breeze/src/airflow_breeze/commands/pr_commands.py
b/dev/breeze/src/airflow_breeze/commands/pr_commands.py
index bd701d469c8..c2f92873642 100644
--- a/dev/breeze/src/airflow_breeze/commands/pr_commands.py
+++ b/dev/breeze/src/airflow_breeze/commands/pr_commands.py
@@ -7249,6 +7249,7 @@ def _review_ready_prs_review_mode(
PRAssessment,
assess_pr_checks,
assess_pr_conflicts,
+ assess_pr_ui_demo,
assess_pr_unresolved_comments,
)
@@ -7350,8 +7351,9 @@ def _review_ready_prs_review_mode(
comments_assessment = assess_pr_unresolved_comments(
pr.number, pr.unresolved_review_comments, pr.unresolved_threads
)
+ ui_demo_assessment = assess_pr_ui_demo(pr.number, pr.labels,
pr.body, pr.author_association)
- if ci_assessment or conflict_assessment or comments_assessment:
+ if ci_assessment or conflict_assessment or comments_assessment or
ui_demo_assessment:
violations = []
summaries = []
if conflict_assessment:
@@ -7363,6 +7365,9 @@ def _review_ready_prs_review_mode(
if comments_assessment:
violations.extend(comments_assessment.violations)
summaries.append(comments_assessment.summary)
+ if ui_demo_assessment:
+ violations.extend(ui_demo_assessment.violations)
+ summaries.append(ui_demo_assessment.summary)
assessment = PRAssessment(
should_flag=True,
@@ -8922,6 +8927,7 @@ def _assess_pr_deterministic(
from airflow_breeze.utils.github import (
assess_pr_checks,
assess_pr_conflicts,
+ assess_pr_ui_demo,
assess_pr_unresolved_comments,
)
@@ -8950,11 +8956,14 @@ def _assess_pr_deterministic(
comments_assessment = assess_pr_unresolved_comments(
pr.number, pr.unresolved_review_comments, pr.unresolved_threads
)
+ ui_demo_assessment = assess_pr_ui_demo(pr.number, pr.labels, pr.body,
pr.author_association)
- if ci_assessment or conflict_assessment or comments_assessment:
+ if ci_assessment or conflict_assessment or comments_assessment or
ui_demo_assessment:
if pr.is_draft:
return DeterministicResult(category="draft_skipped")
- merged = _merge_pr_assessments(conflict_assessment, ci_assessment,
comments_assessment)
+ merged = _merge_pr_assessments(
+ conflict_assessment, ci_assessment, comments_assessment,
ui_demo_assessment
+ )
return DeterministicResult(category="flagged", assessment=merged)
# No deterministic issues found — needs further categorization
diff --git a/dev/breeze/src/airflow_breeze/utils/github.py
b/dev/breeze/src/airflow_breeze/utils/github.py
index c1a6641105c..1a6885c57c0 100644
--- a/dev/breeze/src/airflow_breeze/utils/github.py
+++ b/dev/breeze/src/airflow_breeze/utils/github.py
@@ -646,3 +646,70 @@ def assess_pr_unresolved_comments(
],
summary=f"PR #{pr_number} has {unresolved_review_comments} unresolved
review {thread_word}.",
)
+
+
+# Patterns that indicate screenshots or video demos in a PR body.
+# Based on analysis of real Airflow UI PRs — GitHub drag-and-drop uploads
+# produce HTML <img> tags or bare URLs with user-attachments/assets/ paths.
+_DEMO_EVIDENCE_PATTERNS = [
+ r"<img\s", # HTML <img> tag (GitHub drag-and-drop screenshots)
+ r"https://github\.com/user-attachments/assets/";, # GitHub-uploaded assets
(images & videos)
+ r"!\[", # Markdown image syntax (rare but valid)
+ r"https?://\S+\.(?:png|jpg|jpeg|gif|webp|mp4|mov|webm)", # Direct media
file URLs
+]
+
+# authorAssociation values that indicate the author has write access
+_COLLABORATOR_ASSOCIATIONS_FOR_UI = {"COLLABORATOR", "MEMBER", "OWNER"}
+
+
+def _has_demo_evidence(body: str | None) -> bool:
+ """Check if PR body contains screenshots, images, or video links."""
+ if not body:
+ return False
+ for pattern in _DEMO_EVIDENCE_PATTERNS:
+ if re.search(pattern, body, re.IGNORECASE):
+ return True
+ return False
+
+
+def assess_pr_ui_demo(
+ pr_number: int,
+ labels: list[str],
+ body: str | None,
+ author_association: str,
+) -> PRAssessment | None:
+ """Flag UI PRs from new contributors that lack screenshots or demo videos.
+
+ Returns None if the PR does not have the ``area:UI`` label, the author is a
+ collaborator/member/owner, or the PR body already contains demo evidence.
+ """
+ if "area:UI" not in labels:
+ return None
+
+ if author_association in _COLLABORATOR_ASSOCIATIONS_FOR_UI:
+ return None
+
+ if _has_demo_evidence(body):
+ return None
+
+ return PRAssessment(
+ should_flag=True,
+ violations=[
+ Violation(
+ category="Missing UI demo",
+ explanation=(
+ "This PR changes UI code but the description does not
include "
+ "screenshots or a demo video."
+ ),
+ severity="warning",
+ details=(
+ "For UI changes, please add screenshots or a short screen
recording "
+ "directly to the PR description (not in comments) so
reviewers can "
+ "verify the visual changes. You can paste images directly
into the "
+ "GitHub PR description or drag-and-drop a screen
recording. "
+ f"See [pull request
guidelines]({_CONTRIBUTING_DOCS_URL}/05_pull_requests.rst)."
+ ),
+ )
+ ],
+ summary=f"PR #{pr_number} changes UI code but has no screenshots or
demo.",
+ )
diff --git a/dev/breeze/tests/test_github_ui_demo.py
b/dev/breeze/tests/test_github_ui_demo.py
new file mode 100644
index 00000000000..e38a7d8a98c
--- /dev/null
+++ b/dev/breeze/tests/test_github_ui_demo.py
@@ -0,0 +1,224 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import pytest
+
+from airflow_breeze.commands.pr_commands import _compute_default_action
+from airflow_breeze.utils.confirm import TriageAction
+from airflow_breeze.utils.github import PRAssessment, Violation,
_has_demo_evidence, assess_pr_ui_demo
+from airflow_breeze.utils.pr_models import PRData
+
+
+class TestHasDemoEvidence:
+ """Tests for _has_demo_evidence helper function."""
+
+ def test_empty_body(self):
+ assert _has_demo_evidence("") is False
+
+ def test_none_body(self):
+ assert _has_demo_evidence(None) is False
+
+ def test_text_only_body(self):
+ assert _has_demo_evidence("This PR fixes a bug in the UI sidebar
component.") is False
+
+ def test_html_img_tag_github_assets(self):
+ body = (
+ "## Changes\n"
+ '<img width="1159" alt="image" '
+ 'src="https://github.com/user-attachments/assets/abc123-def456";>'
+ )
+ assert _has_demo_evidence(body) is True
+
+ def test_bare_github_assets_url(self):
+ body = "##
Demo\n\nhttps://github.com/user-attachments/assets/abc123-def456-video\n";
+ assert _has_demo_evidence(body) is True
+
+ def test_markdown_image_syntax(self):
+ body = ""
+ assert _has_demo_evidence(body) is True
+
+ def test_direct_image_url(self):
+ body = "See the result at https://example.com/demo.png";
+ assert _has_demo_evidence(body) is True
+
+ def test_direct_video_url(self):
+ body = "Recording: https://example.com/screen.mp4";
+ assert _has_demo_evidence(body) is True
+
+ @pytest.mark.parametrize(
+ "extension",
+ ["png", "jpg", "jpeg", "gif", "webp", "mp4", "mov", "webm"],
+ )
+ def test_various_media_extensions(self, extension):
+ body = f"https://example.com/file.{extension}";
+ assert _has_demo_evidence(body) is True
+
+ def test_no_false_positive_on_code_mention(self):
+ body = "Updated the image component to handle png files better."
+ assert _has_demo_evidence(body) is False
+
+ def test_no_false_positive_on_url_without_media_extension(self):
+ body = "See https://github.com/apache/airflow/pull/12345 for context."
+ assert _has_demo_evidence(body) is False
+
+
+class TestAssessPrUiDemo:
+ """Tests for assess_pr_ui_demo deterministic check."""
+
+ def test_no_ui_label_returns_none(self):
+ result = assess_pr_ui_demo(
+ pr_number=123,
+ labels=["area:API", "kind:bug"],
+ body="No screenshots here.",
+ author_association="CONTRIBUTOR",
+ )
+ assert result is None
+
+ @pytest.mark.parametrize("association", ["COLLABORATOR", "MEMBER",
"OWNER"])
+ def test_collaborator_association_returns_none(self, association):
+ result = assess_pr_ui_demo(
+ pr_number=123,
+ labels=["area:UI"],
+ body="No screenshots here.",
+ author_association=association,
+ )
+ assert result is None
+
+ def test_contributor_with_demo_returns_none(self):
+ body = '<img width="500"
src="https://github.com/user-attachments/assets/abc123";>'
+ result = assess_pr_ui_demo(
+ pr_number=123,
+ labels=["area:UI"],
+ body=body,
+ author_association="CONTRIBUTOR",
+ )
+ assert result is None
+
+ def test_contributor_without_demo_returns_assessment(self):
+ result = assess_pr_ui_demo(
+ pr_number=123,
+ labels=["area:UI"],
+ body="This fixes a UI bug.",
+ author_association="CONTRIBUTOR",
+ )
+ assert result is not None
+ assert result.should_flag is True
+ assert len(result.violations) == 1
+ assert result.violations[0].category == "Missing UI demo"
+ assert result.violations[0].severity == "warning"
+
+ def test_none_association_without_demo_returns_assessment(self):
+ result = assess_pr_ui_demo(
+ pr_number=456,
+ labels=["area:UI", "kind:feature"],
+ body="Added new sidebar component.",
+ author_association="NONE",
+ )
+ assert result is not None
+ assert result.should_flag is True
+ assert "screenshots" in result.violations[0].explanation.lower()
+
+ def test_empty_body_returns_assessment(self):
+ result = assess_pr_ui_demo(
+ pr_number=789,
+ labels=["area:UI"],
+ body="",
+ author_association="CONTRIBUTOR",
+ )
+ assert result is not None
+ assert result.should_flag is True
+
+ def test_summary_includes_pr_number(self):
+ result = assess_pr_ui_demo(
+ pr_number=42,
+ labels=["area:UI"],
+ body="Some text",
+ author_association="CONTRIBUTOR",
+ )
+ assert result is not None
+ assert "42" in result.summary
+
+
+class TestComputeDefaultActionSoftViolations:
+ """Test default action for PRs with UI demo violations."""
+
+ @staticmethod
+ def _make_pr(**overrides) -> PRData:
+ defaults = {
+ "number": 123,
+ "title": "UI change",
+ "body": "Some text",
+ "url": "https://github.com/apache/airflow/pull/123";,
+ "created_at": "2026-01-01T00:00:00Z",
+ "updated_at": "2026-01-01T00:00:00Z",
+ "node_id": "PR_123",
+ "author_login": "contributor",
+ "author_association": "NONE",
+ "head_sha": "abc123",
+ "base_ref": "main",
+ "check_summary": "",
+ "checks_state": "SUCCESS",
+ "failed_checks": [],
+ "commits_behind": 10,
+ "is_draft": False,
+ "mergeable": "MERGEABLE",
+ "labels": ["area:UI"],
+ "unresolved_threads": [],
+ }
+ defaults.update(overrides)
+ return PRData(**defaults)
+
+ @staticmethod
+ def _make_assessment(summary: str) -> PRAssessment:
+ return PRAssessment(
+ should_flag=True,
+ violations=[
+ Violation(
+ category="Missing UI demo",
+ explanation="No screenshots",
+ severity="warning",
+ details="Add screenshots",
+ )
+ ],
+ summary=summary,
+ )
+
+ def test_soft_violation_only_suggests_draft(self):
+ """A PR with only a soft violation (no CI failures, conflicts, or
comments) gets DRAFT."""
+ pr = self._make_pr()
+ assessment = self._make_assessment("PR #123 changes UI code but has no
screenshots or demo.")
+ action, reason = _compute_default_action(pr, assessment,
author_flagged_count={})
+ assert action == TriageAction.DRAFT
+ assert "draft" in reason
+
+ def test_soft_violation_with_ci_failures_suggests_draft(self):
+ """A PR with a soft violation AND multiple CI failures gets DRAFT
(hard issue takes priority)."""
+ pr = self._make_pr(
+ failed_checks=["Tests / Core", "Tests / API", "Tests / CLI"],
+ checks_state="FAILURE",
+ )
+ assessment = self._make_assessment("PR has CI failures and no demo.")
+ action, _reason = _compute_default_action(pr, assessment,
author_flagged_count={})
+ assert action == TriageAction.DRAFT
+
+ def test_soft_violation_with_conflicts_suggests_draft(self):
+ """A PR with a soft violation AND merge conflicts gets DRAFT."""
+ pr = self._make_pr(mergeable="CONFLICTING")
+ assessment = self._make_assessment("PR has conflicts and no demo.")
+ action, _reason = _compute_default_action(pr, assessment,
author_flagged_count={})
+ assert action == TriageAction.DRAFT
![https://example.com/demo.png"](https://example.com/demo.png"){kind=link}