Skip to content

🤖 Add PR-review agent skills (code-review + review-pr) sharing one rubric#533

Merged
bwateratmsft merged 4 commits into
mainfrom
NCarlsonMSFT/pr-review-skills
Jul 23, 2026
Merged

🤖 Add PR-review agent skills (code-review + review-pr) sharing one rubric#533
bwateratmsft merged 4 commits into
mainfrom
NCarlsonMSFT/pr-review-skills

Conversation

@NCarlsonMSFT

Copy link
Copy Markdown
Member

🤖 This PR adds a semi-autonomous PR-review capability distilled from this repo's (and its predecessor vscode-docker's) PR-review history.

What's here

Two agent skills under .github/skills/, sharing a single grading rubric so they never drift:

  • code-review - auto-invoked by GitHub Copilot code review on PRs; applies the shared rubric to the diff it's given (never fetches, never approves).
  • review-pr - CLI/on-demand skill to review a specific PR by number/URL/branch and, only when explicitly asked, post comments.
  • code-review/rubric.md - the single source of truth: what to look for, severity/confidence grading, and an explicit "what NOT to flag" list.

The rubric encodes the recurring, high-signal patterns that maintainers actually raise here: localization via l10n.t, reuse of the shared platform packages (vscode-processutils, vscode-container-client, azext helpers), disposables/cancellation, cross-platform correctness, falsy-coercion bugs, command-line construction with the arg helpers, telemetry/command-registration wrappers, and settings/package.json contribution rules. The imported-extensibility section is hard-gated to packages/** so it stays silent until the import PRs land.

How it was built and validated

  • Mined SME comments from historical PRs, distilled the most common durable patterns into the rubric.
  • Backtested against real maintainer comments; A/B-tested rubric variants and ran antagonistic (prosecutor/defender) debates to strip low-signal rules.
  • Final "did we cut too much?" A/B (6 held-out PRs) plus a live demo on current open PRs: SME-validated recall held within noise while ~20% of finding volume was shed; ~78% of what it emits is a genuine issue. On live PRs it caught real bugs (a compose regex mis-match, a tree-state atomicity bug) and correctly stayed silent on a clean 4-line fix.

Precision-first by design - it's meant to clear the mechanical layer (localization, reuse, cancellation, cross-platform, etc.) before a human reviews, not to replace design review.

Feedback very welcome on the rubric's rules - it's meant to be tuned by the maintainers over time.

NCarlsonMSFT and others added 3 commits July 17, 2026 09:37
…bric

Adds two Copilot agent skills under .github/skills, both driven by a single
shared grading rubric distilled from this repo's maintainer review history:

- code-review/: rubric-only skill the cloud Copilot code reviewer auto-invokes
  (review-focused name); works from the diff it is given, never approves.
- review-pr/: on-demand CLI skill to review a PR by number/URL, fetching via
  gh/GitHub MCP, report by default, optional posting with a reviewer marker.

rubric.md is the single source of truth (localization, async/resource
correctness, TypeScript conventions, command-line construction, telemetry,
settings, cross-platform), graded by severity/confidence for high signal,
low noise.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 14b94f62-70f3-4e6d-8d2d-cd5a318a26ca
Backtested the rubric against real maintainer (SME) comments on historical
vscode-docker PRs (blind reviewer + judge agents), then A/B-tested two variants
on a held-out set. Adopted the higher-value variant:

- Reframe "high signal, low noise" as *no speculation*, not *few findings*, and
  ask for thoroughness on changed lines. A/B (6 held-out PRs): +40% relative
  recall and ~65% more real issues surfaced, at ~4% noise cost (96% vs 100%
  precision).
- Add "Reuse before adding (check first)" -- reuse was the single most common
  SME request across the corpus -- plus a minimal-API rule.

Mined the two repos being imported into the monorepo (vscode-docker-extensibility,
compose-language-service) for recurring, generalizable maintainer patterns and
added coverage for where the repo is going:
- Contracts / client / registry packages: self-contained public contracts,
  CancellationToken ownership, zod schema parsing, command-runner stream
  lifecycle, image/tag modeling, registry connection-state, export loadability.
- Dependencies / manifests: keep dependency/lockfile churn scoped and Node-engine
  compatible.
- Tests/docs: avoid flaky wall-clock upper bounds; preserve line endings.

Also migrated rubric path references to the monorepo layout
(extensions/vscode-containers/src) and corrected the package manager to pnpm.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 14b94f62-70f3-4e6d-8d2d-cd5a318a26ca
Noise-reduction pass validated by A/B backtest (v3 pre-cut vs this leaner
version) over 6 held-out PRs plus a live demo on current open PRs:
SME-validated recall held within noise while ~20% of finding volume was
shed and false-positive rate stayed flat (~78% of emitted findings are
genuine issues).

- Remove the Declarations/style section (const/quotes/named-export/=== true)
  which contradicted the repo's mixed-style source and earned no catches.
- Narrow async/await, TypeScript, localization, UI/UX, cross-platform and
  regex rules to concrete, demonstrable cases; drop copy-editing nits.
- Hard-gate the imported-extensibility section to packages/** so it cannot
  fire noise on today's extensions/** PRs but is ready for the import PRs.
- Scope dependencies/manifests review; stop reviewing lockfile lines.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 14b94f62-70f3-4e6d-8d2d-cd5a318a26ca
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: fe20b6f7-b994-413a-9ff1-3daae41f1225
@bwateratmsft
bwateratmsft marked this pull request as ready for review July 23, 2026 18:11
Copilot AI review requested due to automatic review settings July 23, 2026 18:11
@bwateratmsft
bwateratmsft requested a review from a team as a code owner July 23, 2026 18:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds two GitHub Copilot agent skills under .github/skills/ to provide consistent PR review behavior for microsoft/vscode-containers, backed by a single shared rubric to avoid drift between automated and on-demand review modes.

Changes:

  • Introduces the code-review skill for automated, diff-only review using a shared rubric.
  • Introduces the review-pr skill for on-demand PR review (by number/URL/branch) with optional GitHub posting guidance.
  • Adds code-review/rubric.md as the single shared source of truth for review grading and repo-specific review patterns.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
.github/skills/review-pr/SKILL.md Defines the on-demand PR review skill, including fetch + optional posting workflow guidance.
.github/skills/code-review/SKILL.md Defines the automated review skill that applies the shared rubric to provided diffs.
.github/skills/code-review/rubric.md Adds the shared grading rubric and repo-specific review patterns used by both skills.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/skills/code-review/rubric.md
Comment thread .github/skills/review-pr/SKILL.md
@bwateratmsft
bwateratmsft merged commit 49351b7 into main Jul 23, 2026
3 checks passed
@bwateratmsft
bwateratmsft deleted the NCarlsonMSFT/pr-review-skills branch July 23, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants