Azure / Azure/azure-rest-api-specs
ARM API Reviewer agent: align automated and interactive review limits, severity, priority, reconciliation, and ARMChangesRequested
- Dominant language
- TypeSpec
- Stars
- 3.1k
- Forks
- 5.9k
- Avg merge
- 3d 37m
- Merged PRs (30d)
- 446
Description
## Problem
The automated GitHub Actions workflow (`arm-api-review.md`) and the interactive agent (`arm-api-reviewer.agent.md`) are two separately maintained prompt files that had drifted apart on core review behavior:
| Behavior | Automated run | Interactive run |
| --- | --- | --- |
| Findings posted | Per-category caps (security/breaking uncapped, ARM contract 15, property/naming 5, doc gaps 3); 50 inline overall; overflow collapsed into "key themes" | No caps, full findings report |
| Critic FAIL after 3 iterations | Drop the finding, no override path | Escalate to human, `critic: override` available |
| Critic unavailable | Downgrade Blocking to Warning | Keep original severity |
| `ARMChangesRequested` label | Applied whenever a Blocking finding was queued, with no carve-out | No equivalent gate |
Because `Azure/azure-rest-api-specs-pr` (the private repo) has no automated workflow of its own, its PRs are always reviewed interactively — so an identical API submitted to the public repo (automated) versus the private repo (interactive) could receive different severities, different finding counts, and a different queue-blocking label outcome purely due to review context, not review substance.
## Evidence
- RP Day feedback specifically raised: "identical APIs may receive different feedback depending on review context" between the private and public repos — the direct trigger for this work.
- Uncapped security/breaking-change findings in the automated path meant worst-case inline comment volume was unbounded, risking hitting GitHub review API limits on large PRs.
- The Critic-unavailable severity downgrade in the workflow (`arm-api-review.md` Step 5.6, pre-fix) silently weakened Blocking findings to Warning specifically in the one context (unattended CI) where no human was present to notice the downgrade — the opposite of the intended safety posture.
- The pre-fix `ARMChangesRequested` rule was asserted as exhaustive by a test (`arm-api-review-workflow.test.js:448`, "leaves no exception to the ARMChangesRequested label rule"), but had no accommodation for a Critic-unavailable run producing full-severity Blocking findings with no verification behind them.
- PR #45841.
## Proposed Solution (implemented in PR #45841)
1. Add the workflow's per-category posting caps and 50-inline budget to the interactive agent definition, so the **posted** finding set is capped identically in both contexts (the interactive chat report can still show every finding; only posting is capped).
2. Remove the Blocking-to-Warning downgrade on Critic-unavailable in the workflow; both paths now preserve original severity and disclose the Critic's unavailability prominently instead of silently softening findings.
3. When the Critic is unavailable, withhold `ARMChangesRequested` even though Blocking findings post at full severity — narrowing the label rule from one input (a Blocking finding was queued) to two (queued **and** Critic-verified), and updating the exhaustiveness test to match.
4. Make the default finding set identical: findings still failing after 3 Critic iterations are dropped in both paths; the interactive human override remains available but is now an explicit, recorded deviation (`critic: override` + `override-reason`), not a silently different baseline.
5. Document review-context parity explicitly in both prompts and in `documentation/api-reviewer-agent.md`, including the note that the private repo has no automated workflow and is reviewed interactively by design.
## Labels
`ARM API Reviewer agent`
Contributor guide
Research direction
Review PR #45841 first, then compare arm-api-review.md with arm-api-reviewer.agent.md and documentation/api-reviewer-agent.md. Run the relevant cases in arm-api-review-workflow.test.js, including the test near line 448. Done means both review paths document matching caps, severity, reconciliation, and ARMChangesRequested behavior, with the private-repository context covered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github-actions
- Domain
- ci-cd, documentation, tooling
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 25/100