Auto-triage: flag area:UI PRs missing screenshots/demo from new contributors#64516
Auto-triage: flag area:UI PRs missing screenshots/demo from new contributors#64516potiuk merged 6 commits intoapache:mainfrom
Conversation
pierrejeambrun
left a comment
There was a problem hiding this comment.
That's interesting. Code looks good beside one comment.
Would love to hear what @potiuk thinks about this one. (personally I think it's a good idea)
d2fa110 to
b1b90fa
Compare
|
I just rebased the branch to rerun the CI |
|
Very interesting. Yes sometimes its not needed but I have found myself commenting a bunch to ask for a screenshot. |
potiuk
left a comment
There was a problem hiding this comment.
Looks good besides the one comment.
There was a problem hiding this comment.
Pull request overview
Adds a deterministic breeze pr auto-triage rule to warn on area:UI PRs from non-collaborators when the PR body lacks screenshots or demo evidence.
Changes:
- Add
_has_demo_evidence()+assess_pr_ui_demo()deterministic assessment inutils/github.py - Integrate the new assessment into deterministic triage and review-mode aggregation in
pr_commands.py - Add focused pytest coverage for demo detection, assessment behavior, and default-action handling
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| dev/breeze/tests/test_github_ui_demo.py | Adds tests for demo-evidence detection, UI demo assessment, and expected triage actions |
| dev/breeze/src/airflow_breeze/utils/github.py | Implements demo-evidence detection and a new deterministic UI-demo assessment producing a warning violation |
| dev/breeze/src/airflow_breeze/commands/pr_commands.py | Wires the new deterministic assessment into triage flows and merges violations/summaries |
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).
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.
014f1fd to
5f99ade
Compare
|
I just rebased the branch to see if it helps with the CI failures. |
|
CI failures are not related to this PR. Other PRs are also failing the same CI tests - #64669 |
Backport failed to create: v3-2-test. View the failure log Run detailsNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
You can attempt to backport this manually by running: cherry_picker ab918c9 v3-2-testThis should apply the commit to the v3-2-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continueIf you don't have cherry-picker installed, see the installation guide. |
Add a deterministic check to
breeze pr auto-triagethat flagsarea:UIPRs from non-collaborator contributors when their PR description lacks screenshots or demo videos. So owners, members, and committers / collaborators are excluded from this check. The rationale: having one PR merged doesn't mean they know the project's conventions around UI demos.There are some PR - #62885 that do not need a UI mock but it is not simple to exclude them.
Comment posted on the PR:
[WARNING] Missing UI demo: This PR changes UI code but the description does not include screenshots or a demo video.
For UI changes, please add screenshots or a short screen recording to the PR description so reviewers can verify the visual changes. You can paste images directly into the GitHub PR description or drag-and-drop a screen recording. See pull request guidelines.
What this does
When triaging PRs, the tool now checks if a PR:
area:UIlabelIf all three conditions are met, the PR is flagged with a
[WARNING] Missing UI demoviolation. The suggested action is comment (not draft conversion) — this is a soft violation handled by a new branch in_compute_default_action.How it works
assess_pr_ui_demo()inutils/github.py— new deterministic check function following the same pattern asassess_pr_checks,assess_pr_conflicts, andassess_pr_unresolved_comments<img>tags,user-attachments/assets/URLs (images & videos), markdownsyntax, and direct media file URLs (.png,.jpg,.gif,.mp4,.mov,.webm, etc.)_assess_pr_deterministic()so the check runs everywhere: main triage flow, TUI, pagination batches, and single-PR mode_compute_default_actionsoft-violation branch ensures UI-demo-only flags getCOMMENTinstead of falling through toDRAFTDry-run validation against all open
area:UIPRsRan
breeze pr auto-triage --label "area:UI" --llm-use api --dry-run --answer-triage s --authors all:area:UIPRs assessedFalse negative (marked as Area:UI, but dont need a UI mock)
True negatives (correctly NOT flagged)
user-attachments/assets/URL → passed all checks<img>tags in body → not flaggedTrue positives (correctly flagged)
Known limitations
Tests
Tests covering:
_has_demo_evidencehelper (empty/None body, text-only, all media patterns, false positive resistance)assess_pr_ui_demo(every exit path: no label, collaborator associations, demo present/absent)_compute_default_actionsoft-violation branch (comment for soft-only, draft when combined with hard issues)Was generative AI tooling used to co-author this PR?
Generated-by: Claude Opus 4.6 following the guidelines