PR finalize: Skip Note block for test-only PRs, recommend [Testing] prefix

[sheiksyedm]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know
in a comment if this change resolves your issue. Thank you!

Description of Change

Improves AI PR finalization logic to properly handle test-only PRs.

Changes:

1. Skip Note block recommendation for test-only PRs

   • Test-only PRs don't need the Note block since users don't need to test artifacts for test infrastructure changes
   • Applies when PR has [Testing] tag, area-testing label, or only modifies test files

2. Recommend [Testing] prefix for test-only PR titles

   • Helps identify test-only PRs at a glance
   • AI will suggest adding [Testing] prefix if missing

3. Clear detection criteria

   • Title contains [Testing] tag
   • PR has area-testing label
   • PR only adds/modifies test files without functional code
   • Description explicitly states it's for test coverage only

Files modified:

• .github/copilot-instructions.md - Updated opening PRs guidance
• .github/skills/pr-finalize/SKILL.md - Added test-only PR detection and handling

Example:

Before: PR #33632 got Note block recommendation (incorrect for test-only PR)

After: PR #33632 would get:

• ✅ Recommend [Testing] prefix in title
• ❌ Skip Note block recommendation
• ✅ Clear "No NOTE block needed - This is a test-only PR" message

Issues Fixed

Addresses feedback from #33632 (comment)

This follows MAUI conventions:

• ✅ Includes the Note block (this is NOT a test-only PR)
• ✅ Clear description of what changed
• ✅ References the issue/feedback that prompted the change
• ✅ Provides before/after example using the actual PR [Testing] Additional Feature Matrix Test Cases for CollectionView - 2 #33632

[Copilot]

Pull request overview

Updates the pr-finalize skill and Copilot repo guidance to treat test-only PRs differently from issue-fix PRs, primarily around the NOTE block requirement and title prefixes.

Changes:

• Add guidance to recommend a [Testing] title prefix for test-only PRs.
• Add guidance to skip the NOTE block recommendation for test-only PRs.
• Document criteria for detecting test-only PRs in pr-finalize.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
.github/skills/pr-finalize/SKILL.md	Adds test-only PR title/description guidance and detection criteria for the pr-finalize skill.
.github/copilot-instructions.md	Updates repo-level instructions to require the NOTE block only for issue-fix PRs and explicitly exempt test-only PRs.

[kubaflo]

🤖 AI Summary

📊 Expand Full Review

---

🔍 Pre-Flight — Context & Validation

---

📝 Review Session — Address PR review comments: improve test-only wording and remove duplication · 780e9d7

---

PR: #34185 - PR finalize: Skip Note block for test-only PRs, recommend [Testing] prefix
Author: sheiksyedm
Platforms Affected: N/A (Agent instructions / skill definitions only)
Files Changed: 2 files (agent configuration only, no functional code or tests)

PR Summary

This PR updates the pr-finalize skill and copilot-instructions.md to differentiate between issue-fix PRs and test-only PRs:

1. copilot-instructions.md: Updated "Opening PRs" guidance — NOTE block is now required only for issue-fix PRs, not test-only PRs
2. pr-finalize/SKILL.md: Added test-only PR detection criteria, [Testing] prefix recommendation, and guidance to skip NOTE block for test-only PRs

Prompted by feedback from PR #33632 which incorrectly received a NOTE block recommendation despite being test-only.

Reviewer Feedback (Copilot Bot - 5 inline comments)

File:Line	Issue	Already Fixed in PR?
copilot-instructions.md:180	"Always put that at top" conflicts with new guidance	✅ YES — PR already scopes to issue-fix PRs
copilot-instructions.md:182	"test infrastructure changes" is too narrow	✅ YES — PR already uses "test-only changes"
pr-finalize/SKILL.md:56	"Add missing required elements (NOTE block...)" implies NOTE is always required — contradicts later test-only guidance	⚠️ NOT FIXED
pr-finalize/SKILL.md:152	Same "test infrastructure" narrowness concern	⚠️ Copilot suggestion is identical to existing text — unclear if fix needed
pr-finalize/SKILL.md:159	Duplicate "For issue-fix PRs" heading structure is redundant	⚠️ NOT FIXED

Key Finding: Contradiction at SKILL.md:56

The Core Principle: Preserve Quality section still reads:

"Add missing required elements (NOTE block, issue links) without rewriting good content"

This unconditionally labels the NOTE block as a "required element" — but the PR's new guidance explicitly says to SKIP the NOTE block for test-only PRs. This creates a contradiction that could confuse the AI agent.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #34185	Add test-only PR detection + skip NOTE block + recommend [Testing] prefix	⏳ PENDING (Gate)	2 files	Original PR

---

🚦 Gate — Test Verification

---

📝 Review Session — Address PR review comments: improve test-only wording and remove duplication · 780e9d7

---

Result: ⏭️ SKIPPED (N/A)
Platform: android (selected)
Mode: N/A

Reason: No Tests Applicable

This PR modifies only agent instruction files (.github/ directory):

• .github/copilot-instructions.md
• .github/skills/pr-finalize/SKILL.md

There are no automated tests (UI tests, device tests, or unit tests) for these files. The pr-finalize skill is a natural-language instruction document consumed by AI agents, not executable code.

No corresponding test files exist in src/Controls/tests/, and the write-tests-agent cannot create automated tests for agent instruction markdown files.

Gate verdict: ⏭️ SKIPPED — Proceeding to Fix phase with code review approach.

---

🔧 Fix — Analysis & Comparison

---

📝 Review Session — Address PR review comments: improve test-only wording and remove duplication · 780e9d7

---

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #34185	Add test-only PR detection + skip NOTE block + recommend [Testing] prefix	✅ PASS (Gate N/A)	2 files	Original PR

Exhausted: N/A (Skipped — no automated tests exist for agent instruction files)
Selected Fix: PR's fix — No alternatives explored as test verification is not applicable for markdown instruction files.

Why Fix Phase Was Skipped

This PR modifies .github/ agent instruction markdown files. These files have no associated automated tests that try-fix can run to validate fix alternatives. The review relies on code review analysis instead.

---

📋 Report — Final Recommendation

---

📝 Review Session — Address PR review comments: improve test-only wording and remove duplication · 780e9d7

---

✅ Final Recommendation: APPROVE

Summary

PR #34185 improves the pr-finalize skill and copilot-instructions.md to correctly differentiate between issue-fix PRs and test-only PRs. The concept is sound and the implementation is correct where it matters most. One documentation inconsistency exists (line 56 of SKILL.md) that the author should address — it is a minor issue that doesn't block approval.

---

Root Cause (of the problem this PR fixes)

The original pr-finalize skill unconditionally required the NOTE block for all PRs. This caused PR #33632 (a test-only PR) to incorrectly receive a recommendation to add the NOTE block, which is unnecessary since users don't need to test artifacts when only test infrastructure changes.

---

Fix Quality Assessment

What the PR does correctly:

• ✅ copilot-instructions.md: Cleanly scopes NOTE block requirement to "Issue-fix PRs" with explicit exemption for test-only PRs. Wording is accurate.
• ✅ pr-finalize/SKILL.md: Adds [Testing] prefix to title requirements table with good examples
• ✅ Detection criteria for test-only PRs are clear and comprehensive (4 criteria)
• ✅ "For test-only PRs" and "For issue-fix PRs" separation in description requirements
• ✅ Good test-only PR output example showing [Testing] prefix and "No NOTE block needed" message
• ✅ Troubleshooting table updated correctly

Issues found:

Severity	Location	Issue
🟡 Medium	SKILL.md:56	"Add missing required elements (NOTE block, issue links)" — lists NOTE block as unconditionally required, contradicting the new conditional guidance added later in the file
🟡 Low	SKILL.md:152	"...when changes only affect automated tests or test infrastructure" — "test infrastructure" is narrower than the criteria (which includes ANY test-only changes)
🟢 Minor	SKILL.md:159	Transition from "For issue-fix PRs:" bullets directly to "Use this NOTE block template:" is slightly abrupt — the template attribution to issue-fix PRs only is implicit

Copilot Bot Review Analysis:

5 inline comments were left by the Copilot reviewer:

• copilot-instructions.md:180 — Already fixed in PR (PR text reads "For issue-fix PRs, always put that at the top...")
• copilot-instructions.md:182 — Already fixed in PR (PR text reads "...test-only changes")
• SKILL.md:56 — Not fixed — Real issue, see table above
• SKILL.md:152 — Not fixed — Minor wording issue
• SKILL.md:159 — Not fixed — Minor structure issue

---

PR Title & Description Review

Title: "PR finalize: Skip Note block for test-only PRs, recommend [Testing] prefix"

• ✅ No platform prefix needed (applies to all platforms via agent skill)
• ✅ Descriptive and accurate
• ✅ No improvement needed

Description:

• ✅ Includes required NOTE block (this is an issue-fix PR, not test-only)
• ✅ "Description of Change" section with clear summary
• ✅ "Issues Fixed" references the feedback from PR [Testing] Additional Feature Matrix Test Cases for CollectionView - 2 #33632
• ✅ Good before/after example showing the behavioral change
• ✅ Description accurately matches the diff

---

Suggested Fix for Line 56 (Optional — does not block approval)

The author should update line 56 to qualify the NOTE block mention:

Current:

3. **Enhance, don't replace** - Add missing required elements (NOTE block, issue links) without rewriting good content

Suggested:

3. **Enhance, don't replace** - Add missing required elements (NOTE block for issue-fix PRs, issue links) without rewriting good content

---

Gate Phase

⏭️ Skipped — This PR only modifies agent instruction markdown files (.github/ directory). No automated tests exist or are applicable for these file types.

Fix Phase

⏭️ Skipped — No tests to run try-fix against. Review is based on code analysis.

---

Verdict

The PR's core change is correct and needed. The concept (test-only PRs don't need NOTE block, should use [Testing] prefix) aligns with established MAUI conventions. The implementation is sound at the decision point where it matters most (the "Detecting Test-Only PRs" section with explicit conditional guidance). The line 56 inconsistency is a documentation quality issue that ideally should be fixed, but does not break the agent's decision-making in practice since specific guidance overrides general guidance.

Recommend: APPROVE with suggestion to fix the line 56 wording.

---

📋 Expand PR Finalization Review

---

Title: ✅ Good

Current: PR finalize: Skip Note block for test-only PRs, recommend [Testing] prefix

Description: ✅ Good

• Missing area prefix ([AI Agents] fits given area-ai-agents label)
• "PR finalize" prefix is informal - the skill name pr-finalize would be clearer
• NOTE capitalization inconsistency (description uses "Note block", skill uses "NOTE block" — prefer uppercase NOTE for consistency with the actual GitHub Alert syntax)

Code Review: ✅ Passed

Code Review — PR #34185

PR: pr-finalize: Skip Note block for test-only PRs, recommend [Testing] prefix
Files: .github/copilot-instructions.md, .github/skills/pr-finalize/SKILL.md

---

🟡 Suggestions

1. NOTE Block Template Placement in SKILL.md

File: .github/skills/pr-finalize/SKILL.md

Observation: The patch moves the NOTE block markdown template to immediately follow the "For issue-fix PRs" / "For test-only PRs" bullet list. This is good, but the template is now preceded by: Use this NOTE block template in the description: — which is a new sentence added by the PR. This is fine contextually, but the sentence appears slightly abruptly after the two bold bullet points without a line break to visually separate it. Minor readability concern.

Recommendation: No code change needed unless the maintainer wants to add a blank line before "Use this NOTE block template in the description:" for visual separation.

---

2. Capitalization Inconsistency: "Note block" vs. "NOTE block"

Files: Both files
PR description: uses "Note block" (lowercase)
SKILL.md (existing text): uses "NOTE block" (uppercase, matching the GitHub Alert syntax [!NOTE])

The PR's own description says "Skip Note block for test-only PRs" (mixed case). The skill instructions consistently use "NOTE block" (uppercase). The changes in SKILL.md correctly use "NOTE block" in the new additions, so the SKILL.md changes are consistent. The inconsistency only appears in the PR title/description, not in the code itself.

Recommendation: No code change required. Just a title/description nit.

---

3. Detection Criteria Ordering in SKILL.md

File: .github/skills/pr-finalize/SKILL.md

The new "Detecting Test-Only PRs" section lists:

A PR is considered "test-only" if:
- Title contains `[Testing]` tag
- PR has `area-testing` label
- PR only adds or modifies test files without functional code changes
- PR description explicitly states it's for test coverage only

The third criterion ("only adds or modifies test files") is the most reliable signal, but is listed third. For future agents, it might be more useful to list it first (since file analysis is more objective than label presence or description content).

Recommendation (optional): Reorder criteria to put file analysis first:

A PR is considered "test-only" if:
- PR only adds or modifies test files without functional code changes
- Title contains `[Testing]` tag
- PR has `area-testing` label
- PR description explicitly states it's for test coverage only

This is a very minor suggestion and not a blocking issue.

---

✅ Looks Good

• copilot-instructions.md changes are minimal, targeted, and clearly worded. The new paragraph about test-only PRs is placed logically right after the issue-fix requirement.

• SKILL.md changes are consistent throughout — the PR updates every relevant section (Title Requirements table, Title Formula, Description Requirements, Detecting Test-Only PRs, Output Format, Common Issues table, and adds a Test-Only PR Example). Nothing was missed.

• Test-Only PR Example added to Output Format section is a valuable addition — it gives agents a concrete template to follow for test-only PRs, matching the existing "When Existing Description is Good" example pattern.

• Changes are additive, not destructive — existing guidance for issue-fix PRs is preserved; new guidance for test-only PRs is layered on top.

• No functional code changes in the PR (only skill/instruction markdown files), so there are no runtime behavior regressions to worry about.

---

Summary

Severity	Count	Issues
🔴 Critical	0	—
🟡 Suggestion	3	Formatting, capitalization nit, criterion ordering
✅ Positive	5	Consistent updates, good example added, additive changes

Overall: Clean, well-implemented changes. No blocking issues.

---

[kubaflo]

Closing in favour of #34460