Fix GUI screenshot gate to use gui/ diff paths - #1405
Conversation
Require UI screenshots only when changed paths include gui/, not when title or body merely mention GUI. Wire changedFilePaths through enforce-pr-target and align unit and integration tests.
|
Warning Review limit reached
Next review available in: 29 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR changes GUI screenshot enforcement from title or description detection to changed-file detection. The workflow passes changed paths and file-list truncation state to ChangesGUI Screenshot Enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant enforce-pr-target.yml
participant GitHub API
participant collectPrQualityFailures
participant guiPathsChanged
participant ScreenshotGate
enforce-pr-target.yml->>GitHub API: retrieve changed files and PR head
GitHub API-->>enforce-pr-target.yml: changed paths and file count
enforce-pr-target.yml->>collectPrQualityFailures: pass paths and truncation state
collectPrQualityFailures->>guiPathsChanged: inspect changed paths
guiPathsChanged-->>collectPrQualityFailures: return GUI path match
collectPrQualityFailures->>ScreenshotGate: validate screenshot evidence or waiver
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
UI screenshot waived by the Hygiene✅ Deterministic PR hygiene checks passed. |
Wibias
added
the
gui-screenshot-waived
|
@CodeRabbit review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/enforce-pr-target.yml:
- Around line 537-543: Update the changed-file handling around github.paginate
and collectPrQualityFailures to fetch pulls.get changed_files_count, detect when
the paginated list is incomplete, and pass an explicit filesTruncated flag. In
collectPrQualityFailures, add missing_ui_screenshot whenever filesTruncated is
true or changedFilePaths includes a gui/ file. Add a regression test covering
more than 3,000 changed files with a gui/ path.
In `@docs-site/src/content/docs/contributing/pr-quality.md`:
- Around line 40-43: Update the documentation around the maintainer waiver to
describe it as path-based: clarify that the label waives the screenshot
requirement for changes under gui/ or for a false-positive GUI-path
classification, while keeping the existing PR gate behavior description
unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4b935899-26c6-4323-870f-63f0917700d5
📒 Files selected for processing (7)
.github/scripts/enforce-pr-target.test.cjs.github/scripts/pr-quality-messages.cjs.github/scripts/pr-quality.cjs.github/scripts/pr-quality.test.cjs.github/workflows/enforce-pr-target.ymldocs-site/src/content/docs/contributing/pr-quality.mdtests/ci-workflows.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/enforce-pr-target.yml:
- Around line 544-546: Update the changed-file retrieval flow around the PR
metadata and changedFiles fetch so the file list and count are tied to the same
PR head SHA; if the snapshots differ, retry the fetch or fail closed. Treat
missing or non-integer changed-file counts as truncated, and only consider the
list complete when the same-head count is a valid integer no greater than
changedFiles.length.
In `@docs-site/src/content/docs/contributing/pr-quality.md`:
- Around line 40-46: Update the screenshot-gate requirements in the contributing
PR quality documentation to state that an incomplete GitHub changed-file list is
conservatively treated as a GUI change, even when no gui/ path is returned.
Clarify that the gui-screenshot-waived label and maintainer waiver also apply to
this case, keeping the documented behavior aligned with the workflow.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dd22559f-c5c2-4364-b52c-cdf430627f33
📒 Files selected for processing (7)
.github/scripts/enforce-pr-target.test.cjs.github/scripts/pr-quality-messages.cjs.github/scripts/pr-quality.cjs.github/scripts/pr-quality.test.cjs.github/workflows/enforce-pr-target.ymldocs-site/src/content/docs/contributing/pr-quality.mdtests/ci-workflows.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/helpers/enforce-pr-target-harness.ts`:
- Around line 656-662: Update the changed_files handling near prChangedFiles to
detect whether options.pr explicitly contains the changed_files property,
defaulting to listedFileCount only when it is absent; preserve supplied
undefined, negative, and fractional values unchanged. Add a workflow-level
regression case that verifies malformed pulls.get metadata produces the
missing_ui_screenshot failure.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a3f03258-d3c9-4a85-8909-044a4b641295
📒 Files selected for processing (7)
.github/scripts/enforce-pr-target.test.cjs.github/scripts/pr-quality.cjs.github/scripts/pr-quality.test.cjs.github/workflows/enforce-pr-target.ymldocs-site/src/content/docs/contributing/pr-quality.mdtests/ci-workflows.test.tstests/helpers/enforce-pr-target-harness.ts
1 participant
Summary
gui/, matching doctor/lint if-changed gating.changedFilePathsfrompulls.listFilesin enforce-pr-target instead of treating GUI mentions in title/body as cues.Test plan
node --test .github/scripts/pr-quality.test.cjs .github/scripts/enforce-pr-target.test.cjs(88 pass)bun test tests/ci-workflows.test.ts --test-name-pattern gui(26 pass)Summary by CodeRabbit
Bug Fixes
gui/or incomplete changed-file lists, rather than GUI-related pull request wording.Documentation
Tests