Azure / Azure/azure-rest-api-specs
ARM API Reviewer agent: reports a misleading review-scope denominator when the PR files API is truncated
- Dominant language
- TypeSpec
- Stars
- 3.1k
- Forks
- 5.9k
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 444
Description
## Summary
The ARM API Reviewer workflow assumes that paginating `pull_request_read(method: "get_files")` yields a complete and reliable changed-file list. GitHub hard-caps that endpoint at 3,000 files. On very large pull requests the agent silently reviews a tiny alphabetical slice of the diff while reporting a scope denominator that is smaller than the real one by orders of magnitude.
Observed on PR #41047 at head SHA `befd1687368facdf2e5f0878725c66107c2cd8d5` ([review comment](https://github.com/Azure/azure-rest-api-specs/pull/41047#issuecomment-5415939251), [workflow run](https://github.com/Azure/azure-rest-api-specs/actions/runs/32890880411)).
## Describe the bug
Trigger Validation step 3 in `.github/workflows/arm-api-review.md` (line 318) instructs the agent to "Paginate the file list so busy PRs are counted reliably." Pagination does not make the count reliable. `GET /repos/{owner}/{repo}/pulls/{number}/files` stops returning results after 3,000 entries, and it does so without any error or explicit truncation flag.
Two consequences follow:
1. The scoped-review subset in step 4 is selected from a truncated, path-sorted prefix of the diff rather than from the whole diff, so the documented priority order ("files in API version directories added by this PR", then `.tsp` and `resource-manager/**/*.json`) is applied only to whatever sorts earliest by path.
2. The Step 8 summary reports `M of N changed specification/ files reviewed` where `N` is the truncated count, which materially understates how much of the pull request went unreviewed.
## Expected behavior
The agent should detect that the file list was truncated and disclose it. Concretely:
- Compare the number of entries returned by `get_files` against `changed_files` from the `pull_request_read(method: "get")` response the agent already makes in Trigger Validation step 1, and treat a returned count of exactly 3,000 as truncated.
- Use the authoritative total in the `**Scoped review:**` disclosure, or state plainly that the total could not be determined, rather than presenting the truncated count as `N`.
- Name the services or path range actually covered, so the assigned human reviewer can see that coverage stopped at a particular point in path order instead of being spread across the pull request.
## Actual behavior
On PR #41047 the agent reported:
> **Scoped review:** ~40 of 2000+ changed `specification/` files reviewed (PR exceeds the automated-review size cap).
The real figures:
| Measure | Value |
|---|---|
| Files reported by `get_files` (30 pages of 100, page 31 empty) | 3,000 |
| Of those, under `specification/` | 1,337 |
| Actual changed files in the pull request | 236,886 |
| Actual changed files under `specification/` | 235,223 |
| Files actually reviewed | approximately 40 |
| Reported coverage | approximately 2 percent |
| True coverage | approximately 0.017 percent |
The 3,000-file window covered only six services in path order: `advisor`, `agricultureplatform`, `ai`, `ai-foundry`, `alertsmanagement`, `apicenter`. The window ends inside `specification/apicenter/`, so every service from `apimanagement` onward was never returned by the API and the agent had no way to know it existed. The subset selector then spent its entire budget on `advisor`, which sorts first in the window and has two newly added API version directories (`2026-02-01-preview` and `2026-03-01-preview`).
A contributing factor is that this pull request also reports `changed_files: 0`, `additions: 0`, and `deletions: 0` on the PR object, because the diff far exceeds what GitHub will compute for the Files tab. That makes the discrepancy easy to miss from the UI, since the Files tab appears empty while the agent reports a substantive review. It also means a guard based solely on `changed_files` needs to handle the zero case, for example by treating `changed_files == 0` alongside a non-empty `get_files` response as a strong truncation signal in its own right.
## Reproduction Steps
1. Fetch the PR object and note the counters:
```
gh api repos/Azure/azure-rest-api-specs/pulls/41047 \
--jq '{changed_files, commits, mergeable_state}'
```
Returns `changed_files: 0`, `commits: 3104`, `mergeable_state: "dirty"`.
2. Page the files endpoint and observe the hard cap:
```
gh api "repos/Azure/azure-rest-api-specs/pulls/41047/files?per_page=100&page=30" --jq 'length' # 100
gh api "repos/Azure/azure-rest-api-specs/pulls/41047/files?per_page=100&page=31" --jq 'length' # 0
```
3. Confirm the true diff size against the merge base reported by the compare API (`fd457ad2d981b67dd76f6cea7dc378f8088579ac`):
```
git fetch --depth=1 origin fd457ad2d981b67dd76f6cea7dc378f8088579ac
git diff --name-only fd457ad2d981b67dd76f6cea7dc378f8088579ac befd1687368facdf2e5f0878725c66107c2cd8d5 | wc -l
```
Returns 236,886.
## Suggested fix
In `.github/workflows/arm-api-review.md`, extend Trigger Validation steps 3 and 4 with a truncation check, and extend the Step 8 `**Scoped review:**` disclosure format to carry the authoritative total plus an explicit note when the file list was capped. The data needed is already available in the `pull_request_read(method: "get")` call the agent makes in step 1, so no additional API surface or a repository checkout is required, which keeps the existing `checkout: false` guardrail intact.
The current behavior is safe in the sense that the agent produced no incorrect findings. The Advisor review itself was accurate. The improvement is purely in the honesty of the coverage disclosure, so a human ARM reviewer is not led to believe a large pull request received broader automated coverage than it did.
Contributor guide
Research direction
Start in .github/workflows/arm-api-review.md, reading Trigger Validation steps 1, 3, and 4 plus the Step 8 scoped-review disclosure. Reproduce the 3,000-file cap with the documented gh api commands, then verify that the workflow detects truncation, uses the authoritative total or states it is unknown, and names the covered path range without adding checkout or API requirements.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github-actions
- Domain
- ci-cd
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 84/100