Skip to content

Consolidate PR reviewers into a single expert-reviewer agent#8157

Merged
Evangelink merged 7 commits into
mainfrom
dev/amauryleve/combined-expert-reviewer
May 12, 2026
Merged

Consolidate PR reviewers into a single expert-reviewer agent#8157
Evangelink merged 7 commits into
mainfrom
dev/amauryleve/combined-expert-reviewer

Conversation

@Evangelink

@Evangelink Evangelink commented May 12, 2026

Copy link
Copy Markdown
Member

Summary

Replaces the three separate PR reviewer workflows (pr-expert-reviewer, pr-nitpick-reviewer, pr-test-expert-reviewer) with a single combined expert-reviewer agent following the MSBuild pattern.

Motivation

  • Local usability: The agent at .github/agents/expert-reviewer.agent.md can be invoked directly in VS Code for local code review — no need to wait for GH agentic workflows.
  • Single reviewer: One unified review instead of 3 separate ones, with coordinated findings and a single summary.
  • Sub-agent architecture: Spawns per-dimension sub-agents in parallel batches for thorough coverage while keeping the orchestration clean.

What changed

New files

  • .github/agents/expert-reviewer.agent.md — Core agent with 21 review dimensions covering correctness, threading, security, API compat, performance, cross-TFM, test quality, style, and more. Multi-wave workflow (Load History → Find → Validate → Post → Summary).
  • .github/workflows/shared/review-shared.md — Shared permissions, tools, safe-outputs, and delegation logic.
  • .github/workflows/review.agent.md/review slash command trigger.
  • .github/workflows/review-on-open.agent.md — Auto-trigger on non-draft PR open.

Deleted files

  • .github/workflows/pr-expert-reviewer.md + .lock.yml
  • .github/workflows/pr-nitpick-reviewer.md + .lock.yml
  • .github/workflows/pr-test-expert-reviewer.md + .lock.yml

Modified files

  • .github/workflows/repo-historian.md — Updated consumer table to reference the unified reviewer.
  • .github/workflows/daily-malicious-code-scan.lock.yml — Updated CodeQL action to v4.35.4 to align with actions-lock.json.
  • .github/workflows/issue-arborist.lock.yml — Merged main's enhanced workflow with REST API fallback.

Review dimensions (21)

# Dimension Severity Source
1 Algorithmic Correctness MAJOR expert
2 Threading & Concurrency BLOCKING expert
3 Security & IPC Contract Safety BLOCKING expert
4 Public API & Binary Compat BLOCKING expert
5 Performance & Allocations MAJOR expert
6 Cross-TFM Compatibility MAJOR expert
7 Resource & IDisposable Mgmt MAJOR expert
8 Defensive Coding at Boundaries MAJOR expert
9 Localization & Resources MAJOR new
10 Test Isolation MAJOR test
11 Assertion Quality MAJOR test
12 Flakiness Patterns BLOCKING test
13 Test Completeness & Coverage MAJOR test
14 Data-Driven Test Coverage MODERATE test
15 Code Structure & Simplification MODERATE nitpick
16 Naming & Conventions NIT nitpick
17 Documentation Accuracy NIT nitpick
18 Analyzer & Code Fix Quality MAJOR new
19 IPC Wire Compatibility MAJOR new
20 Build Infrastructure & Dependencies MODERATE new
21 Scope & PR Discipline MODERATE new

Review feedback addressed

  • Added explicit noop safe-outputs with report-as-issue: false to both review workflows to prevent clean/no-op runs from being escalated as issues
  • Fixed assertion library guidance to respect per-project BannedSymbols.txt policies (some test projects use AwesomeAssertions, others use MSTest Assert)
  • Added checkout: false consistency, fixed batch size wording, and converted historical context note to blockquote callout

Replace the three separate PR reviewer workflows (pr-expert-reviewer,
pr-nitpick-reviewer, pr-test-expert-reviewer) with a single combined
expert-reviewer agent following the MSBuild pattern:

- .github/agents/expert-reviewer.agent.md: Core agent with 21 review
  dimensions covering correctness, threading, security, API compat,
  performance, cross-TFM, test quality, style, and more. Usable both
  locally in VS Code and by GH agentic workflows.
- .github/workflows/shared/review-shared.md: Shared permissions, tools,
  safe-outputs, and delegation logic.
- .github/workflows/review.agent.md: /review slash command trigger.
- .github/workflows/review-on-open.agent.md: Auto-trigger on PR open.

Also updates repo-historian.md consumer table to reference the unified
reviewer.
Copilot AI review requested due to automatic review settings May 12, 2026 09:53

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

This PR consolidates three separate PR reviewer workflows into a single unified “expert-reviewer” agent and introduces two entrypoints for running it (on PR open and via /review). It also updates supporting workflow infrastructure/lockfiles to align with the newer gh-aw compiler output and references the new reviewer in repo-historian documentation.

Changes:

  • Add unified reviewer agent definition (.github/agents/expert-reviewer.agent.md) and shared workflow body (.github/workflows/shared/review-shared.md).
  • Add two new workflows to trigger the unified review (review.agent.md for /review, review-on-open.agent.md for non-draft PR opens).
  • Remove legacy reviewer workflows and update workflow/lockfile infrastructure to reflect the new compilation/versioning.
Show a summary per file
File Description
.github/workflows/shared/review-shared.md Shared config + instructions for invoking the unified expert reviewer.
.github/workflows/review.agent.md Slash-command (/review) entrypoint importing shared review logic.
.github/workflows/review-on-open.agent.md Auto-trigger entrypoint for non-draft PR opens importing shared review logic.
.github/agents/expert-reviewer.agent.md New consolidated reviewer agent spec with multi-dimension review workflow.
.github/workflows/repo-historian.md Updates consumer table to reference unified reviewer.
.github/workflows/repository-quality-improver.lock.yml Regenerated lockfile (gh-aw/compiler/runtime updates).
.github/workflows/repo-historian.lock.yml Regenerated lockfile (gh-aw/compiler/runtime updates).
.github/workflows/daily-malicious-code-scan.lock.yml Regenerated lockfile (gh-aw/compiler/runtime updates).
.github/workflows/agentics-maintenance.yml Updates generated maintenance workflow (new operations + version bumps).
.github/aw/actions-lock.json Updates pinned gh-aw action versions.
.github/workflows/pr-expert-reviewer.md Deleted legacy expert reviewer workflow.
.github/workflows/pr-nitpick-reviewer.md Deleted legacy nitpick reviewer workflow.
.github/workflows/pr-test-expert-reviewer.md Deleted legacy test-quality reviewer workflow.

Copilot's findings

  • Files reviewed: 21/26 changed files
  • Comments generated: 3

Comment thread .github/workflows/shared/review-shared.md
Comment thread .github/agents/expert-reviewer.agent.md Outdated
Comment thread .github/workflows/review-on-open.agent.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 12, 2026 10:07

@Evangelink Evangelink left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Summary

Workflow: PR Nitpick Reviewer
Date: 2026-05-12
Repository: microsoft/testfx

Key Findings

3 minor nitpicks across 2 files — no blocking issues.

# File Finding
1 review.agent.md:18 Missing checkout: false — inconsistent with review-on-open.agent.md
2 expert-reviewer.agent.md:520 Non-standard 1b. step numbering in ordered list
3 expert-reviewer.agent.md:522 "batches of 6" overstates last-batch size (21 = 3×6 + 3)

Positive Highlights

  • ✅ Excellent consolidation of three separate workflows into one well-structured agent with 21 clear review dimensions.
  • ✅ The sub-agent architecture (parallel per-dimension agents + validation wave) is a solid design pattern.
  • ✅ The review-on-open.agent.md correctly uses pull_request_target with checkout: false for fork safety.
  • review-shared.md keeps permissions, tools, and safe-outputs DRY across both entry points.
  • ✅ The Wave 2 validation step ("prove or disprove" before posting) is a great way to reduce false positives.

Recommendations

All findings are minor (NIT-level). The PR is high quality and well-structured.


Generated by PR Nitpick Reviewer 🔍 Meticulously inspected by Nitpick Reviewer

🔍 Meticulously inspected by PR Nitpick Reviewer 🔍

Comment thread .github/workflows/review.agent.md
Comment thread .github/agents/expert-reviewer.agent.md Outdated
Comment thread .github/agents/expert-reviewer.agent.md Outdated
@Evangelink

Copy link
Copy Markdown
Member Author

@copilot address review ocmments

@Evangelink

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

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.

Copilot's findings

  • Files reviewed: 21/26 changed files
  • Comments generated: 4

Comment thread .github/workflows/shared/review-shared.md
Comment thread .github/agents/expert-reviewer.agent.md Outdated
Comment thread .github/agents/expert-reviewer.agent.md Outdated
Comment thread .github/workflows/daily-malicious-code-scan.lock.yml
…batch size wording

Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Resolved conflicts in .github/workflows/issue-arborist.lock.yml by accepting main's enhanced Issue Arborist workflow with REST API fallback and GH_AW_ORIGINAL_GITHUB_API_URL support.

Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 12, 2026 10:31
@Evangelink Evangelink removed the request for review from Copilot May 12, 2026 10:31

Copilot AI commented May 12, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in .github/workflows/issue-arborist.lock.yml. Accepted main's enhanced Issue Arborist workflow with REST API fallback logic and GH_AW_ORIGINAL_GITHUB_API_URL environment variable support.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 12, 2026 10:51
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>

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.

Copilot's findings

Comments suppressed due to low confidence (1)

.github/agents/expert-reviewer.agent.md:475

  • This table states “FluentAssertions style for all”, but the repo uses different assertion libraries depending on the test project (e.g., many use AwesomeAssertions, and some ban it). Update this entry to reflect “follow the test project’s assertion convention / BannedSymbols.txt” to prevent incorrect review recommendations.
| 1 | **Public API Shipping** | Declare in `PublicAPI.Unshipped.txt`. Run `eng/mark-shipped.ps1` to promote. Multi-TFM: `net8.0/`, `net9.0/`, `netstandard2.0/` subfolders. | `eng/mark-shipped.ps1` |
| 2 | **No `init` on Public API** | New public API MUST NOT use `init` accessors. Existing MTP `init` accessors are grandfathered. | `.github/copilot-instructions.md` |
| 3 | **Localization** | `.resx` for user-facing strings. Never edit `.xlf` — build generates them. | `src/*/Strings.resx` |
| 4 | **Test Architecture** | MSTest unit tests use `TestFramework.ForTestingMSTest`. MTP/analyzer tests use MSTest. FluentAssertions style for all. | `test/Utilities/TestFramework.ForTestingMSTest` |
| 5 | **IPC Protocol** | Named pipes, JSON-RPC between test platform and runners. Wire format backward-compatible. | `src/Platform/` |
| 6 | **Analyzer IDs** | `MSTEST0001`+ for MSTest analyzers. Unique across codebase. | `src/Analyzers/` |
  • Files reviewed: 22/25 changed files
  • Comments generated: 4

Comment thread .github/workflows/review.agent.md
Comment thread .github/workflows/review-on-open.agent.md
Comment thread .github/workflows/daily-malicious-code-scan.lock.yml
Comment thread .github/agents/expert-reviewer.agent.md
@Evangelink

Copy link
Copy Markdown
Member Author

@copilot address review comments

- Add explicit noop safe-outputs with report-as-issue: false to review workflows
- Update CodeQL action metadata to v4.35.4 in lock file headers
- Change assertion guidance to follow test project's BannedSymbols.txt policy instead of hardcoding FluentAssertions

Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>

Copilot AI commented May 12, 2026

Copy link
Copy Markdown
Contributor

@copilot address review comments

All review comments addressed:

  1. Added explicit noop safe-outputs with report-as-issue: false to both review.agent.md and review-on-open.agent.md to prevent clean/no-op runs from being escalated as issues.

  2. Completed CodeQL action update to v4.35.4 — updated both the action reference and the metadata headers in daily-malicious-code-scan.lock.yml.

  3. Fixed assertion library guidance in expert-reviewer.agent.md to respect per-project BannedSymbols.txt policies instead of hardcoding FluentAssertions (some projects ban AwesomeAssertions in favor of MSTest Assert, others ban MSTest Assert in favor of AwesomeAssertions).

@Evangelink Evangelink enabled auto-merge (squash) May 12, 2026 11:50
@Evangelink Evangelink merged commit 6e759a7 into main May 12, 2026
10 checks passed
@Evangelink Evangelink deleted the dev/amauryleve/combined-expert-reviewer branch May 12, 2026 12:14
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