Skip to content

fix(ci): list PR files when the compare API truncates - #9508

Open
vedithal-amd wants to merge 2 commits into
mainfrom
users/vedithal-amd/external-ci-pr-files-fallback
Open

vedithal-amd wants to merge 2 commits into
mainfrom
users/vedithal-amd/external-ci-pr-files-fallback

Conversation

@vedithal-amd

@vedithal-amd vedithal-amd commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Fixes the 300-file truncation in #9447.

configure_external_repo_ci.py gets a PR's changed files from the compare API, which returns at most 300 files. At 300 or more, it can't tell which projects changed and runs every test. Large PRs that touch one project, like rocprofiler-compute sprint drops in rocm-systems, build and test every project and fail on unrelated timeouts.

Technical Details

  • When the compare API result is truncated on a pull_request event, read the PR number from the event payload with gha_load_github_event() and list the files with the PR files endpoint, which returns up to 3000.
  • PRs under 300 files, push events, and runs outside GitHub Actions behave as before.
  • Callers need no change. rocm-systems and rocm-libraries already check out all of build_tools/github_actions, so the new import resolves.

Test Plan

  • Unit tests for configure_external_repo_ci.py, also run with GITHUB_EVENT_PATH set to a fake PR event to confirm no test calls the real API.
  • pre-commit run on the changed files.
  • Ran the script against sprint(rocprofiler-compute): Q4 Sprint 1 rocm-systems#12400 with a PR event payload, on this branch and on main.

Test Result

Unit tests and pre-commit pass locally. On this branch the script reports only projects/rocprofiler-compute as changed. On main it falls back to running all tests.

Submission Checklist

🤖 Generated with Claude Code

The compare API returns at most 300 files, so larger PRs to external
repos ran every test. When it truncates on a pull_request event, read
the PR number from the event payload and list the files with the PR
files endpoint, which returns up to 3000.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@therock-pr-bot

therock-pr-bot Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@vedithal-amd
vedithal-amd marked this pull request as ready for review October 26, 2026 23:09
Copilot AI balanced review requested due to automatic review settings October 26, 2026 23:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved revision matching, caller permissions, and package-import failures can skip required tests or prevent CI configuration.

4 open issues
What changed in this PR

This PR addresses #9507 by extending TheRock’s external-repository CI selection to handle large pull requests without immediately selecting every project.

Changes:

  • Falls back to the paginated PR-files endpoint when the compare API reaches its 300-file limit.
  • Reads the PR number from the event payload and retains the run-all fallback at 3,000 files.
  • Adds fallback-routing coverage and isolates the existing truncation test from event payloads.
File Description
build_tools/​tests/​configure_external_repo_ci_test.py Tests fallback routing and mocks event-derived PR detection.
build_tools/​github_actions/​configure_external_repo_ci.py Adds PR-number loading and paginated changed-file retrieval.

Review effort: Balanced

TypeVar,
)

from github_actions_api import gha_load_github_event
pr_number = get_event_pr_number() if modified_paths is None else None
if pr_number:
logger.info(f"Getting PR #{pr_number} files via API")
modified_paths = get_pr_modified_paths_api(github_repo, pr_number)
pr_number = get_event_pr_number() if modified_paths is None else None
if pr_number:
logger.info(f"Getting PR #{pr_number} files via API")
modified_paths = get_pr_modified_paths_api(github_repo, pr_number)
Comment on lines +288 to +290
@patch("configure_external_repo_ci.get_pr_modified_paths_api")
@patch("configure_external_repo_ci.get_event_pr_number", return_value=123)
@patch("configure_external_repo_ci.get_modified_paths_api", return_value=None)
@vedithal-amd

Copy link
Copy Markdown
Contributor Author

Hi @lamikr - do you think this PR will help with the issue assigned to you #9447 - if not i will close it - if yes could you please provide review - thanks!

I had originally created this PR to prevent rocprofiler-compute sprint prs from running unnecessary builds and tests due to workflow not able to figure out which component changed when more than 300 files changed

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: TODO

Development

Successfully merging this pull request may close these issues.

2 participants