Remove unactionable info from reviewer agent#13578
Conversation
🔍 Skill Validator Results
Summary
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)) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 | noneGenerated by Expert Code Review (on open) for issue #13578 · ● 2.3M
jankratochvilcz
left a comment
There was a problem hiding this comment.
Let one nit, otherwise looks good, thanks for quick action!
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Updated [Microsoft.Build](https://github.com/dotnet/msbuild) from 18.6.3 to 18.7.1. <details> <summary>Release notes</summary> _Sourced from [Microsoft.Build's releases](https://github.com/dotnet/msbuild/releases)._ ## 18.7.1 ## What's Changed * Fix TraceEngine file contention deadlock in multithreaded mode by @JanProvaznik in dotnet/msbuild#13446 * Remove duplicate test cases in MultithreadableTaskAnalyzer by @Youssef1313 in dotnet/msbuild#13483 * Ensure ThreadSafeTaskAnalyzer.Tests is considered as a unit test project by @Youssef1313 in dotnet/msbuild#13481 * Fix MSBuildTask0002 analyzer warnings in already-migrated tasks by @JanProvaznik in dotnet/msbuild#13466 * Fix race conditions in task host path resolution by @AR-May in dotnet/msbuild#13485 * Migrate ToolTask and Al task to TaskEnvironment API by @OvesN in dotnet/msbuild#13423 * Bump main to 18.7, add vs18.6 to merge flow by @MichalPavlik in dotnet/msbuild#13472 * Avoid allocations in GetHashCode implementations by @DustinCampbell in dotnet/msbuild#13475 * Add PATs rotation to agentic workflow(s) by @JanKrivanek in dotnet/msbuild#13496 * Fix ASP.NET WebSite projects to copy netstandard.dll facade when required by @JanProvaznik in dotnet/msbuild#13058 * Migrate AspNetCompiler to TaskEnvironment API by @OvesN in dotnet/msbuild#13424 * Add review workflow by @JanKrivanek in dotnet/msbuild#13503 * Strengthen reviewer skill: add step-back analysis dimensions by @JanProvaznik in dotnet/msbuild#13504 * Add 'Request Speedometer Perf Run' to VS experimental insertion build policies by @Copilot in dotnet/msbuild#13505 * Remove duplicate @ prefix from issueAuthor in GitOps by @akoeplinger in dotnet/msbuild#13492 * Improve review aw by @JanKrivanek in dotnet/msbuild#13510 * Migrates unit tests to use RoslynCodeTaskFactory to enable running tests under .NET Core by @jankratochvilcz in dotnet/msbuild#13500 * Fix cross-AppDomain TaskItem modifier cache regression by @DustinCampbell in dotnet/msbuild#13493 * Discourage review agent from approving PRs by @JanKrivanek in dotnet/msbuild#13512 * Stop trying to deploy ValueTuple by @rainersigwald in dotnet/msbuild#13507 * Ad-hoc re-sign bootstrap dotnet on macOS to prevent SIGKILL by @jankratochvilcz in dotnet/msbuild#13513 * RoslynCodeTaskFactory: Log MSB3753 when task class does not implement ITask by @jankratochvilcz in dotnet/msbuild#13517 * Update gh-aw (upon mcp policy changes) by @JanKrivanek in dotnet/msbuild#13526 * Eliminate XmlChildNodes allocations in GetXmlNodeInnerContents by @nareshjo in dotnet/msbuild#13509 * Fix telemetry allocation regression: per-engine collector ownership by @JanProvaznik in dotnet/msbuild#13516 * Migrate to xunit.v3 by @Youssef1313 in dotnet/msbuild#13482 * Fix stray brace in HandleBuildCancel trace string causing MSB1025 by @Copilot in dotnet/msbuild#13535 * Bumping to 10.0.4 runtime packages by @MichalPavlik in dotnet/msbuild#13533 * Remove early return in GetCanonicalForm, always call System.IO.Path by @OvesN in dotnet/msbuild#13532 * Do not overwrite GetCopyToOutputDirectoryItemsDependsOn, just add new… by @snechaev in dotnet/msbuild#13474 * Migrate GetReferenceAssemblyPaths task to TaskEnvironment API by @OvesN in dotnet/msbuild#13495 * Stabilize ToolTaskThatTimeoutAndRetry test by @rainersigwald in dotnet/msbuild#13489 * [automated] Merge branch 'vs18.6' => 'main' by @github-actions[bot] in dotnet/msbuild#13506 * Add extra test assertions around tests by @Youssef1313 in dotnet/msbuild#13536 * Add static eval for repo skills/agents via skill-validator by @JanKrivanek in dotnet/msbuild#13537 * Migrate SGen task to Task environment API by @OvesN in dotnet/msbuild#13457 * Fix TerminalLogger assert failure for metaproj files and cached project eval ID by @OvesN in dotnet/msbuild#13480 * Filter out approving review from pr-reviewer agent by @JanKrivanek in dotnet/msbuild#13553 * Use a unique task name per invocation to tabilize RoslynCodeTaskFactory_ReuseCompilation test by @huulinhnguyen-dev in dotnet/msbuild#13551 * Brief doc on feedback/logging/data systems by @rainersigwald in dotnet/msbuild#13554 * Localized file check-in by OneLocBuild Task: Build definition ID 9434: Build ID 13881982 by @dotnet-bot in dotnet/msbuild#13437 * Stage 3: Forward BuildProjectFile* callbacks from OOP TaskHost to worker node by @JanProvaznik in dotnet/msbuild#13350 * Enable TaskHost Callbacks by default by @JanProvaznik in dotnet/msbuild#13579 * Remove unactionable info from reviewer agent by @JanKrivanek in dotnet/msbuild#13578 * Enlighten RequiresFramework35SP1Assembly task for multithreaded mode by @jankratochvilcz in dotnet/msbuild#13575 * Make SdkResolver-provided environment variables take precedence over ambient environment by @Copilot in dotnet/msbuild#12655 * Add dotnet/skills marketplace and enable plugins by @Evangelink in dotnet/msbuild#13582 * The skills/agents check filters-in only touched files by @JanKrivanek in dotnet/msbuild#13586 * Fix skill-validation workflow failing when agents directory is deleted by @JeremyKuhne in dotnet/msbuild#13592 ... (truncated) Commits viewable in [compare view](dotnet/msbuild@v18.6.3...v18.7.1). </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Context
@jankratochvilcz was pointing two friction points with pr-reviewer:
Changes
Guiding to reduce nonactionable outputs
cc @jankratochvilcz