Repository navigation
OSAC-3065: fix design review triggering on PRD-only PRs - #153
openshift-merge-bot[bot] merged 2 commits into
Conversation
Remove get_incremental_files which used the GitHub compare API
(compare/{before}...{head}) for synchronize events. After a
force-push or rebase the two SHAs have different ancestry, causing
the compare to return every file in the repo — including design.md
files from other enhancement directories — which made detect_skills
schedule a spurious design review.
Always use get_changed_files (the PR files endpoint) which reliably
returns only the PR's actual diff regardless of rebase history.
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Eran Cohen <eranco@redhat.com>
|
@eranco74: This pull request references OSAC-3065 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe EP review script now always retrieves all changed pull-request files instead of using incremental synchronize-event diffs. The workflow passes updated review configuration and pull-request metadata to the script. ChangesEP review file selection
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 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 |
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 @.github/scripts/ep_review.py:
- Line 116: Update get_changed_files, which produces the files value used here,
to request paginated results with --slurp and extract filenames from the
combined JSON document in one pass. Remove the incompatible --jq usage from the
gh api invocation, and ensure the parsed result still returns the changed
filenames for both single-page and multi-page responses.
🪄 Autofix (Beta)
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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: abf74fc7-9aa7-4ecc-b39f-5459898c59c5
📒 Files selected for processing (2)
.github/scripts/ep_review.py.github/workflows/ep-review.yml
💤 Files with no reviewable changes (1)
- .github/workflows/ep-review.yml
Use .[].filename (one filename per line) instead of [.[].filename] (JSON array per page). With --paginate, each page runs --jq separately, so the array form produces concatenated JSON arrays that json.loads cannot parse on multi-page responses. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Eran Cohen <eranco@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eranco74, tchughesiv The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
get_incremental_fileswhich used the GitHub compare API (compare/{before}...{head}) forsynchronizeevents. After a force-push/rebase the two SHAs have different ancestry, causing the compare to return every.mdfile in the repo — includingdesign.mdfiles from other enhancement directories — which madedetect_skillsschedule a spurious design review.get_changed_files(the PR files endpoint) which reliably returns only the PR's actual diff regardless of rebase history.EVENT_NAME,EVENT_ACTION,EVENT_BEFORE_SHAenv vars from the workflow.Root cause
Confirmed via workflow logs:
synchronizeevent →get_incremental_filesreturned 46 files across the entire repo → bothprd-reviewanddesign-reviewtriggered on a PRD-only PRget_changed_filesreturned 1 file → onlyprd-reviewtriggeredTest plan
prd.mdanddesign.mdstill get both reviews🤖 Generated with Claude Code
Summary by CodeRabbit