Azure / Azure/azure-rest-api-specs

ARM API Reviewer agent: Integrate TypeSpec Suppression Review

Open
#44,582 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 2h
Merged PRs (30d)
424

Description

### Background

The TypeSpec team is rolling out a new TypeSpec Suppression Review check that evaluates TypeSpec lint suppressions directly, instead of relying on Swagger LintDiff output from emitted OpenAPI. Rollout is non-blocking on July 27 and targets blocking around August 31. The check surfaces links to TypeSpec source, per-rule docs with correct/incorrect examples, reviewer guidance on when suppressions are acceptable, and correlation to existing LintDiff rules. Rules are enabled as a curated subset first and grown over time.

A key efficiency goal: where a TypeSpec suppression and its Swagger LintDiff equivalent are the same, an ARM reviewer should review the TypeSpec surface once and reuse that decision for LintDiff, rather than reviewing both. This saves review time even while the check is non-blocking.

This issue tracks making the ARM API Reviewer agent aware of that process.

### Design constraints (must hold)

Per the [Design Principles in azure-api-review/SKILL.md](.github/skills/azure-api-review/SKILL.md), the implementation must prioritize reuse and low maintenance:

- Define each rule or decision framework in exactly one place and cross-reference it. No parallel or mirrored files.
- The suppression decision framework (approve, push to fix, escalate, never-valid, GA vs preview) is format-agnostic. Extend the existing shared reference to cover TypeSpec rather than creating a TypeSpec-specific copy.
- The agent must reference shared files by path; it does not auto-discover docs. Keep the number of new anchors minimal.
- Reuse existing structural patterns (for example the "apply the same workflow, with these adjustments" pattern already used for suppressions.yaml, and the Step 5.5 reconciliation mechanism for de-duplicating findings).

### Findings from investigation

- The agent loads rules through a closed, path-based allowlist in the Authoritative Rule Sources and Step 2 sections of `.github/agents/arm-api-reviewer.agent.md`; the Critic fails findings whose source cannot be quoted. New docs must be wired in by path.
- The agent does not depend on the `azure-typespec-author` skill and has no parent skill hierarchy to inherit from. It uses the sibling `azure-api-review` skill.
- Current suppression review is Swagger LintDiff centric: Step 4 readme.md/suppressions.yaml handling in `.github/agents/arm-api-reviewer.agent.md`, `.github/skills/azure-api-review/references/suppression-review-criteria.md`, and `.github/skills/azure-api-review/references/linter-rule-coverage.md`. TypeSpec in-source `#suppress` handling is limited to the generic policy in section 4.1 of `.github/instructions/typespec-review.instructions.md`.

### Scope (reuse-first)

1. Extend, do not duplicate, the shared `.github/skills/azure-api-review/references/suppression-review-criteria.md` so it is format-agnostic. Add a short TypeSpec section covering only what genuinely differs (in-source `#suppress` syntax, source-versus-emitted verification note, SDK-impact class). The approve/push/escalate/never-valid/GA-vs-preview framework stays defined once and is reused.
2. Add a "For TypeSpec `#suppress` directives" subsection to Step 4 of `.github/agents/arm-api-reviewer.agent.md` using the existing "apply the same inventory-compare-classify workflow, with these adjustments" pattern already used for suppressions.yaml. It should point to the shared criteria reference rather than restating rules.
3. Trim section 4.1 of `.github/instructions/typespec-review.instructions.md` to TypeSpec-specific specifics plus a pointer to the shared criteria reference, so the policy is not maintained in two places.
4. Extend `.github/skills/azure-api-review/references/linter-rule-coverage.md` with a TypeSpec-rule column that correlates TypeSpec suppression rule IDs to their LintDiff equivalents, in the single existing map rather than a parallel file. Each mapping must be classified as one of: **equivalent** (decision reusable across both surfaces), **TypeSpec-only**, **LintDiff-only**, or **related-but-verify** (a mapping exists but the emitted shape can diverge, so each surface is reviewed separately). Only "equivalent" entries are eligible for the review-once reuse in item 8. This preserves existing "Also enforced by" annotations and ARM reviewer context.
5. Document SDK-impacting suppression rules and the SDK signoff escalation path once, in the shared criteria reference, and cross-reference it from the agent and the TypeSpec instructions. Do not restate escalation contacts in multiple files.
6. Register the new "TypeSpec Suppression Review" check by reusing the existing check-recognition pattern (for example the "Swagger LintDiff" precedent in `.github/workflows/src/summarize-checks/summarize-checks.js`) and reference it from the Downstream-CI Impact Check (`.github/skills/azure-api-review/references/downstream-ci-impact.md`) once blocking. Treat it as advisory during the non-blocking phase.
7. Coordinate rule additions with the `azure-typespec-author` maintainers per the note in `.github/skills/azure-api-review/SKILL.md`.
8. Cross-surface suppression de-duplication. When a TypeSpec `#suppress` and a readme.md or suppressions.yaml LintDiff suppression resolve to the same rule via the correlation map and are classified "equivalent", the agent treats them as one finding. It reviews the TypeSpec surface first (the surface reviewers look at first), records the decision once, and marks the LintDiff equivalent as already decided rather than re-reviewing it. Reuse the existing Step 5.5 reconciliation mechanism rather than adding a new one.

### Acceptance criteria

- No duplicated rule text or decision framework. New guidance lives in one shared reference and is cross-referenced.
- The agent cites the new TypeSpec suppression guidance via existing shared files, wired in by path.
- TypeSpec in-source `#suppress` directives are reviewed with the same rigor as readme.md and suppressions.yaml, reusing the shared workflow.
- Reviewers can map a TypeSpec suppression rule to its LintDiff equivalent from the single coverage map, including the equivalence classification.
- A suppression that is equivalent across both surfaces is reviewed once and the decision is reused, not reviewed twice.
- Non-equivalent or divergent cases (TypeSpec-only, LintDiff-only, related-but-verify) are reviewed independently, so reuse never causes a wrong approval.
- The efficiency gain holds while the check is non-blocking, since reuse is a review-workflow behavior, not a gating behavior.
- SDK-impacting rules and the escalation path are defined once and referenced.
- Agent behavior aligns with the rollout timeline (advisory before blocking, enforced after).
- After edits, docs pass the repo formatting check (`npm run format` from `.github`).

### Dependencies and notes

- Blocked on the TypeSpec team publishing the rule documentation and finalizing the curated rule subset.
- Add the correlation entries and criteria incrementally as rules migrate from LintDiff, so maintenance tracks the rollout rather than front-loading unused content.
- The review-once reuse depends on the correlation map's "equivalent" classification being accurate. When in doubt, classify as "related-but-verify" so both surfaces are reviewed.

Contributor guide

Open the contributing guide

Research direction

Read .github/agents/arm-api-reviewer.agent.md and the shared suppression-review-criteria.md and linter-rule-coverage.md references first, then inspect the existing check-recognition pattern in .github/workflows/src/summarize-checks/summarize-checks.js. Update the specified agent, skill, instruction, coverage, and downstream-CI references without duplicating guidance, reuse Step 5.5 for equivalent findings, and run npm run format from .github.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
documentation, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.