Azure / Azure/azure-rest-api-specs

ARM API Reviewer agent: align automated and interactive review limits, severity, priority, reconciliation, and ARMChangesRequested

Open
#45,946 0 comments 0 reactions 0 assignees View on GitHub
ARM API Reviewer agent
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.