Skip to content

Commit 8ba0a1c

Browse files
xirzecCopilot
andauthored
Reduce spurious Copilot Code Review comments on AutoPR PRs (#38768)
## Problem Copilot Code Reviewer (CCR) generates noisy, repetitive, or unactionable comments on auto-generated ARM SDK PRs. Observed in: - #38603 - #38664 **Examples:** - 12+ identical comments about subscriptionId placeholder format across every sample file - Comments on \pnpm-lock.yaml\ specifier mismatches (not actionable) - Comments on generated config files (\eslint.config.mjs\, \warp.config.yml\) deviating from other packages ## Changes - **\.github/copilot-instructions.md\** — Add repo-wide review behavior rules: deduplicate comments, skip lockfile, recognize \[AutoPR\ title prefix - **\.github/instructions/reviewer/mgmt-sdk.instructions.md\** — Add rules to skip placeholder values in samples, generated infrastructure config files, and duplicate comments - **\.github/instructions/reviewer/lockfile.instructions.md\** (new) — Path-scoped exclusion for \pnpm-lock.yaml\ ## Verification These changes will take effect on new PRs after merge. Effectiveness can be validated by observing comment patterns on subsequent AutoPR PRs. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c02185f commit 8ba0a1c

3 files changed

Lines changed: 29 additions & 1 deletion

File tree

.github/copilot-instructions.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,15 +113,24 @@ When possible, refer to the Azure SDK for JS Design Guidelines for specific exam
113113
## Local SDK Generation and Package Lifecycle (TypeSpec)
114114

115115
### AUTHORITATIVE REFERENCE
116+
116117
For all TypeSpec-based SDK workflows (generation, building, validation, testing, versioning, and release preparation), follow #file:../eng/common/instructions/azsdk-tools/local-sdk-workflow.instructions.md
117118

118119
### DEFAULT BEHAVIORS
120+
119121
- **Repository:** Use the current workspace as the local SDK repository unless the user specifies a different path.
120122
- **Configuration:** Identify `tsp-location.yaml` from files open in the editor. If unclear, ask the user.
121123

122124
### REQUIRED CONFIRMATIONS
125+
123126
Ask the user for clarification if repository path or configuration file is ambiguous.
124127

128+
## Code Review Behavior
129+
130+
- When the same issue appears identically across multiple files, comment ONCE on the first occurrence and note how many other files are affected (e.g., "This same issue appears in 12 other sample files"). Do not post duplicate comments.
131+
- Do NOT comment on `pnpm-lock.yaml` — it is auto-generated by pnpm and not human-editable. Version mismatches are caught by CI.
132+
- PRs with titles starting with `[AutoPR` are auto-generated by SDK generation tooling. Focus review on public API surface issues (types, exports, naming). Do not comment on infrastructure files (`eslint.config.mjs`, `warp.config.yml`, `tsconfig.json`, `vitest.config.ts`) — their patterns are set by the generation tool.
133+
125134
## SDK release
126135

127-
For detailed workflow instructions, see [SDK Release](https://github.com/Azure/azure-sdk-for-js/blob/main/eng/common/instructions/copilot/sdk-release.instructions.md).
136+
For detailed workflow instructions, invoke the `azsdk-common-sdk-release` skill.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
applyTo: "pnpm-lock.yaml"
3+
description: "Exclude pnpm lockfile from code review comments — it is auto-generated and not human-editable."
4+
---
5+
# Lockfile Review Rules
6+
7+
- Do NOT comment on pnpm-lock.yaml changes
8+
- The lockfile is auto-generated by pnpm and is not human-editable
9+
- Specifier mismatches, version differences, or formatting changes are expected and validated by CI checks
10+
- No code review comments on this file are actionable

.github/instructions/reviewer/mgmt-sdk.instructions.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,18 @@ Flag patterns indicating spec problems:
6363

6464
## Samples & Tests
6565
- Samples are auto-generated — only comment on syntax issues found while checking `src/` references
66+
- Do NOT comment on placeholder values in samples (e.g., `subscriptionId`, `resourceGroupName`) — these are intentionally non-real values
6667
- Do NOT comment on style, formatting, documentation, or whitespace
6768
- Do NOT comment on implementation internals (private methods, internal helpers)
6869

70+
## Generated Infrastructure Files
71+
- Do NOT comment on `eslint.config.mjs`, `warp.config.yml`, `tsconfig.json`, or `vitest.config.ts` patterns — these are set by the generation tool and intentionally differ from hand-written packages
72+
- Config deviations from other packages are expected when generation tooling evolves
73+
74+
## Comment Deduplication
75+
- If an issue repeats across multiple files (e.g., same placeholder format in every sample), comment ONCE on the first file and note the total count of affected files
76+
- Do NOT post the same comment on every file that has the same issue
77+
6978
## Issue Types
7079
**🔴 Tool Issue:** Generation bug → fix + report to [autorest.typescript](https://github.com/Azure/autorest.typescript/issues)
7180
**🔴 Design Issue:** API problem → `@clientName`/`@override` in spec repo

0 commit comments

Comments
 (0)