Skip to content

ARM API Reviewer agent: Integrate TypeSpec Suppression Review #44582

Description

@ravimeda

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, 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Fields

    No fields configured for Task.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions