Azure / Azure/azure-sdk-tools

Standardize PR-reviewer skills to enhance CCR output

Open
#16,035 0 comments 0 reactions 1 assignee Claimed by @JennyPng View on GitHub
needs-triage
Dominant language
C#
Stars
135
Forks
260
Avg merge
3d 1h
Merged PRs (30d)
143

Description

- A universal reviewer skill lives in and syncs from azure-sdk-tools at `.github/skills/`. It points towards language-specific skills.
- e.g. sync a `code-review/SKILL.md` (synced to all repos) which points to `detailed-code-review/SKILL.md` which is specific to the repo itself
- Enforce sub-agent usage for different focus areas: security, architecture design, bug/edge case catching, TODO

- Enforce review comments to _cite_ or _tag_ when they're referencing the skill for their review, for tracking

## Steps

- [ ] determine strengths of each repo's current approach
- [ ] draft a generic prompt and Python-specific prompt
- [ ] experiment with performance in Python on test PRs
- [ ] repeat for other repos

## Current state (Copilot report):

# Azure SDK Code-Review Prompt Organization — Cross-Repo Research

**Date:** 2026-06-18
**Scope:** Upstream `main` of `Azure/azure-sdk-for-{python,net,java,js,rust,go}`
**Goal:** Standardize how AI code-review guidance is organized — each repo keeps its own
details in its own file, but a single standard review skill is synced to all repos.

---

## 1. How each repo organizes AI code-review guidance

| Repo | Entry point | Reviewer guidance location | Scoping mechanism | Dedicated "do code review" unit | Shared-skill sync |
|------|-------------|----------------------------|-------------------|-------------------------------|-------------------|
| **azure-sdk-for-js** ⭐ most mature | `AGENTS.md` (router, single-source-of-truth rule) | `.github/instructions/reviewer/*.instructions.md` (6 files) + `.github/prompts/*-review-guidelines.md` (9 deep prompts) + `.github/agents/*.agent.md` (8 named reviewer personas) | `applyTo:` globs per domain (src, test, deps, docs, arm) | Yes — per-domain agent+prompt+instruction triads | `.github/skills/azsdk-common-*` `distribution: shared` |
| **azure-sdk-for-rust** | `AGENTS.md` | `.github/instructions/*.instructions.md` (changelog, pr, commit, pwsh, resourcemanager) + nested `sdk/cosmos/` mirror | `applyTo:` globs; nested per-service sub-tree | Partial — `cosmos-design-struct` + `cosmos-pre-commit-validation` skills | `.github/skills/` (invocable `/skill`), no `azsdk-common-*` yet |
| **azure-sdk-for-python** | `AGENTS.md` + `.github/copilot-instructions.md` (mgmt rules inline) | Inline mgmt rules in copilot-instructions + `ml/do-code-review/SKILL.md` (12 focus areas) | Hard-coded path scoping (`sdk/*/azure-mgmt-*`, `sdk/ml/azure-ai-ml`); no `applyTo` | Yes — `do-code-review` skill (one package only) | `azsdk-common-*` `distribution: shared` + per-package skills |
| **azure-sdk-for-java** | `AGENTS.md` + `.github/copilot-instructions.md` | copilot-instructions PR guidelines + `azsdk-common-apiview-feedback-resolution` | Skill `description` USE-FOR routing; per-package `find-package-skill` discovery | No general code-review skill (only APIView) | `azsdk-common-*` `distribution: shared` + `create-package-skill` wizard + `vally lint` |
| **azure-sdk-for-net** | `AGENTS.md` + `.github/copilot-instructions.md` | `azure-sdk-mgmt-pr-review` (3-phase) + `mpg-migration-pr-review` (+3 phases) + `Check-MgmtNamingRules.ps1` (deterministic scanner) + live `.github/workflows/mgmt-review.md` agentic workflow | Skill invocation + workflow path filters (`Azure.ResourceManager.*`) | Yes — strongest mgmt review skills | `azsdk-common-*` `distribution: shared` |
| **azure-sdk-for-go** | `AGENTS.md` + `.github/copilot-instructions.md` (router) | `.github/instructions/*.instructions.md` (go-code, tests, examples, go.mod) + live `.github/workflows/mgmt-review.md` | `applyTo:` globs per file type | No general skill; AGENTS.md review checklist + automated workflow | `.github/skills/` (one skill); `eng/common/instructions` |

---

## 2. Convergent conventions already shared across all repos

- **`AGENTS.md` at root** = model-agnostic hub (all 6). None use `CLAUDE.md`.
- **`.github/skills//SKILL.md`** with YAML frontmatter (`name`, `description`,
`metadata.distribution: shared`, `compatibility`) — the de-facto **cross-repo sync unit**,
prefixed `azsdk-common-*`, with `evals/`, `fixtures/`, `references/` subdirs.
- **`eng/common/instructions/azsdk-tools/*.instructions.md`** synced identically
(Python / Java / Go / .NET).
- **`.github/agents/agentic-workflows.agent.md`** dispatcher present in nearly all repos.
- **`applyTo:` glob frontmatter** for file-scoped instructions (JS, Rust, Go).
- **Generated-code protection** everywhere (never edit `generated/`, `src/Generated/`,
`_generated/`, `// Code generated by`).

---

## 3. Overlapping review rules that belong in ONE standard, synced skill

These appear (with language-specific wording) in 4–6 repos and form the natural body of a shared
`azsdk-common-code-review` skill:

1. **API design guideline compliance** — defer to each language's azure-sdk design guidelines.
2. **Naming conventions** — `Client` suffix, `Options`/options-bag suffix, standard verbs
(create/upsert/get/list/delete/begin), banned verbs (make/fetch/push), acronym casing.
3. **Breaking-change detection** — GA baseline comparison, version bump must match severity
(breaking→major), beta cycle exemptions, no breaking changes to GA without architect approval.
4. **Generated-code exclusion** — never review/flag generated files; never hand-edit, fix at
TypeSpec source.
5. **CHANGELOG discipline** — update for user-visible changes, correct section
(Features Added / Breaking Changes / Bugs Fixed / Other), unreleased-on-top, date format.
6. **Versioning consistency** — package version ↔ CHANGELOG ↔ constants/context all match;
stability flags ↔ preview/beta suffix.
7. **Tests required** — happy/error/edge/cancellation paths; recorded-test setup; no hardcoded
creds; proper assertion libraries.
8. **Error handling** — language `Result`/error types, no swallowed exceptions, validation
before calls.
9. **Async/cancellation** — AbortSignal/Context/cancellation token propagated; pagination via the
language's async-iterable type.
10. **Docs** — all public APIs documented (TSDoc/JavaDoc/`///`/docstring); snippet/sample consistency.
11. **Security** — no secrets in code/logs/recordings; credentials via Azure Identity/core-auth;
no dangerous patterns.
12. **Dependencies** — approved licenses (MIT/Apache-2.0/BSD/ISC); core-* before new deps;
version-range rules.
13. **Copyright headers** on every source file.
14. **Output discipline** — severity-tiered findings (🔴/🟡/🔵), one PR review, never APPROVE,
never flag style/whitespace/pre-existing/internal-only code.

---

## 4. Recommended standardization model

The **JS repo is the reference architecture**, and the existing `azsdk-common-* / distribution: shared`
skill system is the ready-made sync rail.

- **One synced skill:** `.github/skills/azsdk-common-code-review/SKILL.md` (`distribution: shared`)
containing the 14 language-agnostic rules above + the standard output format + severity scale +
exclusion list. Byte-identical across all 6 repos, owned centrally (azure-sdk-tools `eng/common`
sync, same as existing `azsdk-common-*` skills).
- **Per-repo override file** the shared skill references by convention, e.g.
`.github/instructions/reviewer/.instructions.md` (or `references/language-specifics.md`
inside a per-repo skill). Each repo fills in its own naming rules, type system, build/lint
commands, package layout, and mgmt-plane specifics — exactly what JS already splits into
`sdk-source` / `testing` / `dependencies` / `mgmt-sdk` and Go/Rust put in `*.instructions.md`.
- **Discovery glue:** keep `AGENTS.md` as the router that points the shared skill at the repo-local
details file, and keep `find-package-skill` for per-package tribal knowledge.
- **Eval discipline:** adopt JS's "no hard rule without an eval" policy (`evals/` directory) so the
synced skill is regression-tested.

---

## 5. Per-repo source references

### azure-sdk-for-js
- `AGENTS.md` (master router; single-source-of-truth + "no hard rule without an eval")
- `.github/instructions/reviewer/{sdk-source,testing,dependencies,documentation,mgmt-sdk,lockfile}.instructions.md`
- `.github/prompts/{architecture,security,performance,dependency,documentation,test,mgmt}-review-guidelines.md`,
`mgmt-breaking-change-analysis-guidelines.md`, `mgmt-breaking-change-patterns.md`
- `.github/agents/{archie,dash,dexter,scribe,sentinel,tester,mgmt-review,mgmt-breaking-change-analysis}.agent.md`
- `.github/skills/azsdk-common-*` (`distribution: shared`)

### azure-sdk-for-rust
- `AGENTS.md` (v2.0) + nested `sdk/cosmos/AGENTS.md` (23 KB)
- `.github/instructions/{changelog,cosmos,git-commit,github-pullrequest,pwsh,resourcemanager}.instructions.md`
- `.github/skills/{check-spelling,create-crate,create-perf-test,create-recorded-test,lint-markdown,update-emitter,update-rust}/SKILL.md`
- `sdk/cosmos/.github/skills/{cosmos-pre-commit-validation,cosmos-design-struct}/SKILL.md`

### azure-sdk-for-python
- `AGENTS.md` + `.github/copilot-instructions.md` (inline MGMT SDK code-review rules)
- `.github/skills/ml/do-code-review/SKILL.md` (12 focus areas, primary review skill)
- `.github/skills/azsdk-common-*`, `fix-{pylint,mypy,sphinx,black}`, `create-package-skill`, `find-package-skill`
- `.github/prompts/*.prompt.md`

### azure-sdk-for-java
- `AGENTS.md` + `.github/copilot-instructions.md` (PR guidelines, behavior rules)
- `.github/skills/azsdk-common-*`, `create-package-skill` (+ phases), `find-package-skill`
- `sdk/search/azure-search-documents/.github/skills/azure-search-documents/SKILL.md` (per-package example)
- Validation via `vally lint`

### azure-sdk-for-net
- `AGENTS.md` + `.github/copilot-instructions.md`
- `.github/skills/{azure-sdk-mgmt-pr-review,mpg-migration-pr-review,mgmt-review-comment-resolution,analyze-ci-failures,mitigate-breaking-changes,pre-commit-checks}/SKILL.md`
- `azure-sdk-mgmt-pr-review/Check-MgmtNamingRules.ps1` (deterministic scanner: SUFFIX/RESNAME/ACRONYM/ARMCOMMON/BOOL/DATETIME/TTL rule families)
- `.github/workflows/mgmt-review.md` (live automated PR-review agentic workflow)

### azure-sdk-for-go
- `AGENTS.md` (PR review checklist) + `.github/copilot-instructions.md` (router)
- `.github/instructions/{go-code,go-examples,go-mod-standards,go-tests,tsp-location}.instructions.md` (`applyTo:` scoped)
- `.github/workflows/mgmt-review.md` (live automated PR-review agentic workflow)
- `.github/skills/upgrade-emitter-version/SKILL.md`

---

## 6. Notable gaps / inconsistencies to resolve when standardizing

- **No general code-review skill** in Java and Go (only APIView feedback / automated CI workflow).
- **Inconsistent placement** of code-review rules: inline in `copilot-instructions.md` (Python mgmt),
dedicated skill (Python ML, .NET, JS), or `*.instructions.md` (Go, Rust).
- **Scoping varies:** `applyTo:` globs (JS/Rust/Go) vs. hard-coded paths (Python) vs. skill
description routing (Java).
- **Naming target convention differs:** JS/Go data-plane vs. .NET ARM suffix rules vs. Rust struct-design.
- **`distribution: shared` + `azsdk-common-*`** already exists as the sync mechanism — Rust and Go
have not yet adopted the `azsdk-common-*` prefixed shared skills for review.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.