[waldekmastykarz]

Improves the pr-review agent skill (.github/skills/pr-review/) based on running it blind on the original commits of 10 recently merged PRs (#7632, #7619, #7616, #7613, #7611, #7599, #7596, #7595, #7586, #7585) and comparing its output with actual human and Copilot review feedback. The skill caught ~19/27 real issues (2 partial, 6 missed), was too strict with its verdict, and flagged pre-existing issues as blocking.

No linked issue: this is a maintainer tooling change that only touches the skill files.

Changes

• Verdict: only Errors trigger REQUEST_CHANGES; Warnings/Suggestions only → COMMENT; zero findings → APPROVE.
• Scope policy: read as widely as needed, but only issues the PR introduces or exposes (incl. unchanged code whose behavior changes because of the PR, and copied bad patterns) can block. Pre-existing issues go under a non-blocking "Noticed, out of scope" heading. Heuristic: "Would this still be a problem if the PR were reverted?"
• Severity calibration: security issues and regressions in behavior users rely on are always Errors.
• Consistency fixes: "four categories", Documentation category added to findings and inline comment format, file-reference link format defined (blob URL at the reviewed SHA with #L anchors).
• Phase 1: read linked issues, review a specific commit, build/test/lint in a separate worktree (and say so if not run).
• Breaking changes/removals: release-notes wording, upgrade guidance, leftover references, unknown-options behavior.
• Refactors/migrations: behavior parity (aliases, option sets, customCode: 'optionSet'/'required', zod.coercedEnum, unknown options, docs).
• Test conventions: behavior-describing test names, exact URL/body assertions, settingsNames constants, asserting changed arguments.
• Output docs: only text output applies defaultProperties; verify each format's sample.
• New references/area-checklists.md: SPFx upgrade/doctor rules, docs site, telemetry/infrastructure.

Validation

Documentation-only change to agent skill files; no build or tests apply. The skill was not re-run against PRs as part of this change.

[Copilot]

🟡 Changes recommended

The review guidance contains inaccuracies in PR diff selection, SPFx support checks, and CSV/Markdown property handling that can cause missed changes or false findings.

3 open issues

• Resolve PR base at reviewed revision and honor target branch · New
• Preserve nested object and array properties in CSV and Markdown · New
• Avoid false positives from outdated externalize support matrix · New

What changed in this PR

Refines the pr-review skill using feedback from 10 merged PRs, aiming to reduce false positives and better distinguish blocking issues from advisory feedback.

Changes:

• Calibrates severity, verdicts, and scope rules for introduced versus pre-existing issues.
• Expands guidance for commit-specific reviews, isolated validation, migrations, tests, and output documentation.
• Adds focused checklists for SPFx, the docs site, and telemetry/infrastructure.

File	Description
.github/​skills/​pr-review/​SKILL.md	Updates review workflow, scope, severity, validation, and reporting guidance.
.github/​skills/​pr-review/​references/​area-checklists.md	Adds area-specific checks for SPFx, docs, and shared infrastructure.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Unresolved inaccuracies in diff selection and review checks can hide historical PR changes, run unintended tests, and flag valid behavior as defects.

5 open issues

• Mocha config causes targeted tests to run the entire suite · New
• Allow historical and rejection-test references in docs and tests · New
• Avoid false positives from outdated externalize support matrix
• Preserve nested object and array properties in CSV and Markdown
• Resolve PR base at reviewed revision and honor target branch

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The latest-head review workflow does not fetch the PR head before creating a worktree, so it can fail with an invalid-reference error when the commit is absent locally, particularly for fork PRs.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Fetch PR head before creating worktree for latest-head reviews

.github/​skills/​pr-review/​SKILL.md:48

For an ordinary latest-head review, the preceding steps never fetch the PR head (gh pr view and gh pr diff only read API data), so this fails with invalid reference when that commit is absent locally, notably for fork PRs. Consider fetching the PR head from the base repository before creating the worktree; the existing fetch only covers the specific-commit workflow.

5 resolved since last review

• Allow historical and rejection-test references in docs and tests
• Mocha config causes targeted tests to run the entire suite
• Avoid false positives from outdated externalize support matrix
• Preserve nested object and array properties in CSV and Markdown
• Resolve PR base at reviewed revision and honor target branch

_{Review effort: Balanced}

[waldekmastykarz]

Addressed the latest Copilot review in d898939. The validation workflow now fetches pull/<number>/head from the base repository before creating the worktree, so latest-head reviews work when the commit is not already available locally, including fork PRs. Verified with exact aliases nrb && nt: 16,288 tests passed with 100% statements, branches, functions, and lines coverage.

[Copilot]

🟡 Changes recommended

Validation commands check out the latest PR head instead of the reviewed SHA, allowing later fixes to mask defects in the revision under review.

1 open issue

• Validate the reviewed commit SHA instead of the latest PR head · New

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Human assessment is needed to distinguish actual PR changes from merged target-branch changes that the diff may incorrectly flag as blocking findings.

No open issues 🎉

1 resolved since last review

• Validate the reviewed commit SHA instead of the latest PR head

_{Review effort: Balanced}