Skip to content

Remove unactionable info from reviewer agent#13578

Merged
JanKrivanek merged 3 commits intodotnet:mainfrom
JanKrivanek:dev/jankrivanek/actionable-reviewer
Apr 21, 2026
Merged

Remove unactionable info from reviewer agent#13578
JanKrivanek merged 3 commits intodotnet:mainfrom
JanKrivanek:dev/jankrivanek/actionable-reviewer

Conversation

@JanKrivanek
Copy link
Copy Markdown
Member

Context

@jankratochvilcz was pointing two friction points with pr-reviewer:

Changes

Guiding to reduce nonactionable outputs

cc @jankratochvilcz

Copilot AI review requested due to automatic review settings April 20, 2026 12:58
@JanKrivanek JanKrivanek requested a review from a team as a code owner April 20, 2026 12:58
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 20, 2026

🔍 Skill Validator Results

⚠️ Warnings or advisories found

Scope Checked
Skills 13
Agents 1
Total 14
Severity Count
--- ---:
❌ Errors 0
⚠️ Warnings 6
ℹ️ Advisories 0

Summary

Level Finding
ℹ️ Found 13 skill(s)
ℹ️ [assessing-breaking-changes] 📊 assessing-breaking-changes: 992 BPE tokens [chars/4: 1,154] (detailed ✓), 8 sections, 2 code blocks
ℹ️ [merge-dependency-updates] 📊 merge-dependency-updates: 2,030 BPE tokens [chars/4: 1,887] (detailed ✓), 16 sections, 7 code blocks
ℹ️ [reviewing-msbuild-code] 📊 reviewing-msbuild-code: 166 BPE tokens [chars/4: 216] (compact ✓), 1 sections, 0 code blocks
ℹ️ [reviewing-msbuild-code] ⚠ Skill is only 166 BPE tokens (chars/4 estimate: 216) — may be too sparse to provide actionable guidance.
ℹ️ [reviewing-msbuild-code] ⚠ No code blocks — agents perform better with concrete snippets and commands.
ℹ️ [reviewing-msbuild-code] ⚠ No numbered workflow steps — agents follow sequenced procedures more reliably.
ℹ️ [maintaining-binary-log-compatibility] 📊 maintaining-binary-log-compatibility: 1,271 BPE tokens [chars/4: 1,509] (detailed ✓), 12 sections, 2 code blocks
ℹ️ [release] 📊 release: 1,901 BPE tokens [chars/4: 1,889] (detailed ✓), 11 sections, 0 code blocks
ℹ️ [release] ⚠ No code blocks — agents perform better with concrete snippets and commands.
Full validator output ```text Found 13 skill(s) [assessing-breaking-changes] 📊 assessing-breaking-changes: 992 BPE tokens [chars/4: 1,154] (detailed ✓), 8 sections, 2 code blocks [merge-dependency-updates] 📊 merge-dependency-updates: 2,030 BPE tokens [chars/4: 1,887] (detailed ✓), 16 sections, 7 code blocks [reviewing-msbuild-code] 📊 reviewing-msbuild-code: 166 BPE tokens [chars/4: 216] (compact ✓), 1 sections, 0 code blocks [reviewing-msbuild-code] ⚠ Skill is only 166 BPE tokens (chars/4 estimate: 216) — may be too sparse to provide actionable guidance. [reviewing-msbuild-code] ⚠ No code blocks — agents perform better with concrete snippets and commands. [reviewing-msbuild-code] ⚠ No numbered workflow steps — agents follow sequenced procedures more reliably. [maintaining-binary-log-compatibility] 📊 maintaining-binary-log-compatibility: 1,271 BPE tokens [chars/4: 1,509] (detailed ✓), 12 sections, 2 code blocks [release] 📊 release: 1,901 BPE tokens [chars/4: 1,889] (detailed ✓), 11 sections, 0 code blocks [release] ⚠ No code blocks — agents perform better with concrete snippets and commands. [multithreaded-task-migration] 📊 multithreaded-task-migration: 3,319 BPE tokens [chars/4: 3,600] (standard ~), 29 sections, 9 code blocks [multithreaded-task-migration] ⚠ Skill is 3,319 BPE tokens (chars/4 estimate: 3,600) — approaching "comprehensive" range where gains diminish. [use-bootstrap-msbuild] 📊 use-bootstrap-msbuild: 709 BPE tokens [chars/4: 751] (detailed ✓), 14 sections, 5 code blocks [pipelines-health-check] 📊 pipelines-health-check: 5,025 BPE tokens [chars/4: 5,098] (comprehensive ✗), 40 sections, 10 code blocks [pipelines-health-check] ⚠ Skill is 5,025 BPE tokens (chars/4 estimate: 5,098) — "comprehensive" skills hurt performance by 2.9pp on average. Consider splitting into 2–3 focused skills. [authoring-errors-and-warnings] 📊 authoring-errors-and-warnings: 1,287 BPE tokens [chars/4: 1,393] (detailed ✓), 13 sections, 4 code blocks [changewaves] 📊 changewaves: 901 BPE tokens [chars/4: 1,007] (detailed ✓), 9 sections, 1 code blocks [optimizing-msbuild-performance] 📊 optimizing-msbuild-performance: 1,182 BPE tokens [chars/4: 1,342] (detailed ✓), 10 sections, 1 code blocks [integrating-sdk-and-msbuild] 📊 integrating-sdk-and-msbuild: 1,250 BPE tokens [chars/4: 1,440] (detailed ✓), 14 sections, 2 code blocks [deploy-msbuild-to-vs] 📊 deploy-msbuild-to-vs: 2,312 BPE tokens [chars/4: 2,363] (detailed ✓), 24 sections, 12 code blocks ✅ All checks passed (13 skill(s)) Found 1 agent(s) Validated 1 agent(s)

✅ All checks passed (1 agent(s))

</details>

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the expert-reviewer agent guidance to reduce non-actionable review output and improve signal-to-noise in the final summary.

Changes:

  • Adds explicit instruction that inline comments must be actionable (no praise/“looks good” inline notes).
  • Changes the summary guidance to omit LGTM dimensions from the summary table and instead report a single “X/Y dimensions clean” line.
Show a summary per file
File Description
.github/agents/expert-reviewer.agent.md Tightens reviewer-agent guidance to avoid non-actionable inline comments and reduce LGTM noise in summary output.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Comment thread .github/agents/expert-reviewer.agent.md Outdated
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Expert Review — PR #13578

# Dimension Verdict
22 Correctness & Edge Cases 🟡 1 MODERATE, 1 NIT

✅ 23/24 dimensions clean.

  • Correctness — contradictory wording between "record it as LGTM in the summary table" (line 649) and "Omit all LGTM dimensions from the table" (line 657); all-clear case not covered by example

Overall: Good change that meaningfully reduces review noise. The core idea — actionable-only inline comments and findings-only summary tables — is sound. The one real issue is the self-contradictory instruction at line 649 which references the old "record in table" behavior while the new line 657 says to omit from the table. A quick wording fix resolves it.

Note

🔒 Integrity filter blocked 1 item

The following item were blocked because they don't meet the GitHub integrity level.

  • #13578 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Expert Code Review (on open) for issue #13578 · ● 2.3M

Comment thread .github/agents/expert-reviewer.agent.md
Comment thread .github/agents/expert-reviewer.agent.md Outdated
Copy link
Copy Markdown
Contributor

@jankratochvilcz jankratochvilcz left a comment

Choose a reason for hiding this comment

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

Let one nit, otherwise looks good, thanks for quick action!

Comment thread .github/agents/expert-reviewer.agent.md Outdated
JanKrivanek and others added 2 commits April 21, 2026 11:50
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@JanKrivanek JanKrivanek enabled auto-merge April 21, 2026 09:55
@JanKrivanek JanKrivanek merged commit a76579a into dotnet:main Apr 21, 2026
11 checks passed
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