[Engineering task] Copilot PR Review: excessive noise from `review_on_push` and false positives

[gladjohn]

Task type

Development

Copilot PR Review: excessive noise from review_on_push and false positives

Summary

The Copilot PR review ruleset (review_on_push: true) is generating excessive review noise — 328 comments across 40 of 52 PRs (77%) since March 6. Reviews trigger on every push, producing duplicate comments and an escalating volume that has grown 5x (18/week to 90/week). Verified false-positive rate is 15–20%.

Repro / Evidence

Configuration

The ruleset (ID 13579084) is at https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/rules/13579084

Setting	Current Value	Problem
review_on_push	true	Triggers a full review on every commit pushed, not just PR creation
review_draft_pull_requests	true	Reviews draft PRs that are still work-in-progress
current_user_can_bypass	never	No one can skip or dismiss the review

Data: volume and re-review pattern

Analyzed the last 52 PRs on AzureAD/microsoft-authentication-library-for-dotnet.

Metric	Value
Total Copilot review comments	328
PRs receiving comments	40 out of 52 (77%)
Unique commits reviewed	105
PRs re-reviewed across multiple pushes	17 out of 39 (44%)
Near-duplicate comments (same text re-posted)	24 out of 312 (8%)

Data: escalating weekly volume

Week	Comments	PRs Hit	Week-over-Week
Mar 22	18	1	baseline
Mar 29	37	8	+106%
Apr 05	40	11	+8%
Apr 12	63	11	+58%
Apr 19	64	12	+2%
Apr 26	90	11	+41%

Data: worst-hit PRs

PR	Title	Comments	Commits Reviewed	Avg per Commit
#5888	WithAttributeTokens	57	12	4.8
#5947	Expose refresh token	52	22	2.4
#5902	Remove region req	21	2	10.5
#5949	HttpClient leak fix	16	7	2.3
#5919	STS error code	13	3	4.3

Data: same file commented repeatedly in one PR

PR	File	Comments	Across Commits
#5888	AbstractConfidentialClient...Extension.cs	15	5
#5888	WithAttributeTokensTests.cs	14	4
#5949	SimpleHttpClientFactory.cs	13	6
#5947	SilentAuthTests.cs	11	6
#5947	TokenCache.ITokenCacheInternal.cs	10	5

Data: files commented across unrelated PRs

These files received Copilot comments in 3+ unrelated PRs, indicating the reviewer comments on existing code, not just changes.

File	Total Comments	PRs
OtelInstrumentation.cs	15	#5961, #5928, #5920, #5919
OTelInstrumentationTests.cs	10	#5961, #5947, #5928, #5919
RequestBase.cs	4	#5947, #5928, #5920, #5919

Verified False Positives

These were confirmed by building and running tests against PRs #5849 and #5888.

1. Namespace resolution

• Claim: Client.AppConfig.CertificateOptions does not resolve (no Client namespace alias in scope)
• Reality: Compiles successfully. C# resolves Client via the parent namespace Microsoft.Identity
• Occurrences: 4 comments across 2 PRs

2. Missing test attribute

• Claim: Test method is missing [TestMethod] and will not be discovered
• Reality: [RunOn] attribute inherits from TestMethodAttribute (see TargetFramework.cs line 15). Tests are discovered and run
• Occurrences: 4+ comments across multiple PRs

3. Nullable bool

• Claim: Assert.IsTrue expects non-nullable bool, so CertificateOptions?.SendCertificateOverMtls will not compile
• Reality: MSTest provides an Assert.IsTrue(bool?) overload. Code compiles and runs
• Occurrences: 2 comments

4. Style comments on pre-existing code

• Claim: Indentation and bracing are inconsistent
• Reality: The flagged style is pre-existing code not modified by the PR
• Occurrences: 8+ comments on PR Expose refresh token via extension and add CacheOptions.DisableInternalCache #5947

Expected Behavior

• Reviews should run once per PR (or on-demand), not on every push
• Reviews should not comment on unchanged code
• Reviews should not produce false positives for patterns that compile and run successfully
• Draft PRs should not be reviewed until marked ready
• Review instructions should include repo-specific rules (test attributes, namespace patterns, C# coding standards)

Root Cause: Custom Instructions Are Not Reaching the Reviewer

Copilot code review reads only the first 4,000 characters of .github/copilot-instructions.md.

From the GitHub documentation on customizing Copilot code reviews:

"Copilot code review only reads the first 4,000 characters of any custom instruction file. Any instructions beyond this limit will not affect the reviews generated by Copilot code review. This limit does not apply to Copilot Chat or Copilot cloud agent."

"When reviewing a pull request, Copilot uses the custom instructions in the base branch of the pull request."

References:

• Custom instructions for code review: https://docs.github.com/en/copilot/using-github-copilot/code-review/using-copilot-code-review#customizing-copilots-reviews-with-custom-instructions
• Adding repository custom instructions: https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot
• Configuring automatic code review: https://docs.github.com/en/copilot/how-tos/agents/copilot-code-review/automatic-code-review
• Path-specific instructions (.github/instructions/**/*.instructions.md): https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot#creating-path-specific-custom-instructions

The current .github/copilot-instructions.md is 19,582 characters — nearly 5x over the review limit. Here is what the reviewer actually sees vs what it misses:

What Copilot review sees (first 4,000 chars)

Content	Chars	Useful for Review?
"Read .clinerules folder"	90	No — review cannot read other folders
ConcurrentDictionary.GetOrAdd rule	650	Yes
GitHub Agent Skills documentation	1,100	No — only relevant to Copilot Chat
mTLS PoP skill discovery prompts	2,160	No — only relevant to Copilot Chat

What Copilot review never sees (chars 4,001–19,582)

All remaining content is beyond the 4,000-char cutoff and is ignored by code review.

What Copilot review cannot access at all

The repo has good coding guidelines in .clinerules/ but Copilot code review cannot read files outside .github/copilot-instructions.md. Only Copilot Chat and Copilot Agent can read .clinerules/.

File	Chars	Content
.clinerules/csharp-guidelines.md	5,950	C# coding standards, naming, null checks, async
.clinerules/msal-guidelines.md	4,115	MSAL architecture, testing, public API rules
.clinerules/cline-instructions.md	2,581	Cline-specific tool usage

The good rules exist but the reviewer never sees them. The 4,000-char budget is consumed by skill documentation that has no relevance to code review.

Repo-specific patterns causing false positives

These patterns are unique to this repo and need explicit instructions:

Pattern	Why Copilot gets it wrong
[RunOn] test attribute	Inherits from TestMethodAttribute but Copilot flags as missing [TestMethod]
Client.AppConfig.X namespace	C# resolves via parent namespace Microsoft.Identity, but Copilot flags as unresolved
MSTest Assert.IsTrue(bool?)	MSTest has a bool? overload, but Copilot flags as type mismatch

Proposed Fix

1. Restructure .github/copilot-instructions.md

Move all code review rules to the first 4,000 characters. Push skill documentation (which is only used by Copilot Chat, not code review) below the cutoff.

Proposed structure for the first 4,000 chars:

# Code Review Rules (read by Copilot code review)

## Repo-specific patterns — do not flag these
- [RunOn] inherits TestMethodAttribute — do not flag as missing [TestMethod]
- Client.AppConfig.X resolves via parent namespace Microsoft.Identity — do not flag
- Assert.IsTrue(bool?) is valid in MSTest — do not flag nullable bool

## Review scope
- Only comment on code modified in the PR diff
- Do not comment on style, formatting, or indentation
- Do not re-post comments already made on earlier commits
- Focus on: bugs, security, logic errors, API contract violations

## C# coding standards (from .clinerules/csharp-guidelines.md)
- Use pattern matching for null checks (x is null, not x == null)
- Use GetOrAdd with factory delegate, not eager evaluation
- Mark new public API in PublicAPI.Unshipped.txt
- (condensed key rules from csharp-guidelines.md)

## MSAL-specific rules (from .clinerules/msal-guidelines.md)
- (condensed key rules from msal-guidelines.md)

# --- Below this line is for Copilot Chat/Agent only ---
# (skills documentation, discovery prompts, etc.)

2. Update ruleset settings

From the GitHub docs on configuring automatic code review:

"Optionally, if you want Copilot to review all new pushes to the pull request, select Review new pushes. If this option is not selected, Copilot will only review the pull request once."

Setting	Current	Proposed	Reason
review_on_push	true	false	Stops per-commit re-review (44% of PRs affected)
review_draft_pull_requests	true	false	Stops reviewing work-in-progress
current_user_can_bypass	never	always	Allows maintainers to skip for trivial changes

Ruleset settings page: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/rules/13579084

3. Consider .github/instructions/*.instructions.md for path-specific rules

From the GitHub docs on path-specific instructions:

"Path-specific custom instructions apply to requests made in the context of files that match a specified path. These are specified in one or more NAME.instructions.md files within or below the .github/instructions directory."

These files are not subject to the 4,000-char limit per file. This can be used for test-specific review rules (e.g., do not flag [RunOn] as missing [TestMethod] in test files).