From beb6420274fea44f342d29cdac5e9753c482d36f Mon Sep 17 00:00:00 2001 From: Debasis Mandal Date: Fri, 19 Jun 2026 17:15:05 +0000 Subject: [PATCH 1/2] docs: add post-PR Copilot review loop; fix split inline-code span Document the after-create workflow in the pr-workflow skill: wait for all Copilot comments, evaluate each on merits, fix the worthwhile ones, then resolve every thread with either a fix+SHA reply or a won't-fix rationale (GraphQL recipes included, since REST 404s on review-thread comments). Also fix a CLAUDE.md inline-code span that was split across a newline (markdown can't render a multi-line backtick span) by using two separate spans. Co-Authored-By: Claude Opus 4.7 --- .claude/skills/pr-workflow/SKILL.md | 49 +++++++++++++++++++++++++++++ CLAUDE.md | 9 +++--- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/.claude/skills/pr-workflow/SKILL.md b/.claude/skills/pr-workflow/SKILL.md index 841efd6a8d..ec5f3601f3 100644 --- a/.claude/skills/pr-workflow/SKILL.md +++ b/.claude/skills/pr-workflow/SKILL.md @@ -104,6 +104,55 @@ publish to a shared repo and shouldn't be triggered without explicit consent. Before any `gh pr create`, also complete the fail-closed PR target safeguard above. +## After creating a PR: handle the Copilot review + +Every PR on this repo gets an automated Copilot review. After `gh pr create`, +always run this loop before considering the PR done: + +1. **Wait for all Copilot comments to land.** The review is not instant — Copilot + posts a top-level review plus inline comments a short while after the PR (and + after each later push). Poll until the review has arrived and the comment set + is stable; don't evaluate a half-posted review. Copilot may also *auto-push* + "Potential fix for pull request finding" commits to the branch — if so, + `git fetch` and integrate them before adding your own (rebase; resolve + conflicts keeping the more complete version). + +2. **Evaluate each comment on its merits.** Decide per comment whether to fix it + — Copilot is often right but not always. Use judgement; do not blanket-apply. + +3. **Address the ones worth fixing**, commit, and push to the PR branch. + +4. **Resolve every thread**, with the right closure for each: + - *Fixed* → reply citing the commit SHA, then resolve the thread. + - *Won't fix* → reply with the reason you decided not to address it, then + resolve the thread. + Either way the thread ends resolved with a written rationale. + +List threads with their resolved status, and resolve them, via GraphQL (the REST +`/comments/` endpoint 404s on these review-thread comments): + +```bash +# List threads (id + resolved + comment bodies) +gh api graphql -f query=' +{ repository(owner:"ROCm", name:"flashinfer") { + pullRequest(number: ) { + reviewThreads(first: 50) { nodes { + id isResolved + comments(first: 10) { nodes { databaseId author { login } path body } } } } } } }' + +# Reply to a comment (use the databaseId from above) +gh api repos/ROCm/flashinfer/pulls//comments//replies \ + --method POST --field body="" + +# Resolve a thread (use the thread node id, e.g. PRRT_...) +gh api graphql -f query=' +mutation { resolveReviewThread(input:{threadId:""}) { + thread { isResolved } } }' +``` + +Done = no unresolved Copilot threads remain, each carrying either a fix+SHA reply +or a won't-fix rationale. + ## PR Description **Body** — include sections that apply, skip the rest: diff --git a/CLAUDE.md b/CLAUDE.md index ac216a26a1..de89f5083b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,10 +17,11 @@ Failing to raise a PR is always preferable to raising one against upstream. Also NEVER push to or raise a PR from the `amd-integration` branch itself — it is the base, never the head. If you are on `amd-integration` with commits to ship, create a fresh topic branch at the current HEAD -(`git branch `), then `git fetch origin amd-integration && -git reset --hard origin/amd-integration` to restore `amd-integration` (the -commits stay safe on ``), and PR from ``. Treat a -detached HEAD (empty `git branch --show-current`) as an abort condition too. +(`git branch `), then run `git fetch origin amd-integration` +followed by `git reset --hard origin/amd-integration` to restore +`amd-integration` (the commits stay safe on ``), and PR from +``. Treat a detached HEAD (empty `git branch --show-current`) as +an abort condition too. Full procedure: `pr-workflow` skill. ## Essential Commands From 499544fedd02898b8761ebf0e566cafa15cbe6a3 Mon Sep 17 00:00:00 2001 From: Debasis Mandal Date: Fri, 19 Jun 2026 17:20:29 +0000 Subject: [PATCH 2/2] docs: add pre-PR quality gate; fix Copilot review on #263 Add a "Before creating a PR: quality gate" section to the pr-workflow skill (simplify/production-ready, /code-review and apply, run relevant pytests, then commit). Reword the GraphQL aside to correctly state that thread resolution and isResolved are unavailable via REST (replies are available over REST), and note the reviewThreads(first:50) query is not exhaustive on large PRs. Co-Authored-By: Claude Opus 4.7 --- .claude/skills/pr-workflow/SKILL.md | 31 ++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/.claude/skills/pr-workflow/SKILL.md b/.claude/skills/pr-workflow/SKILL.md index ec5f3601f3..a4feb4b11a 100644 --- a/.claude/skills/pr-workflow/SKILL.md +++ b/.claude/skills/pr-workflow/SKILL.md @@ -104,6 +104,28 @@ publish to a shared repo and shouldn't be triggered without explicit consent. Before any `gh pr create`, also complete the fail-closed PR target safeguard above. +## Before creating a PR: quality gate + +Run this gate on the branch's full diff before `gh pr create`, in order: + +1. **Simplify / make production-ready.** Review all changes on the branch and run + `/simplify`: remove dead code, debug/scratch code, debug-only comments, and + unused imports. Keep comments that carry real value (the *why*, hidden + constraints, non-obvious invariants) — do not strip those. + +2. **Code review.** Run `/code-review` on the diff, then apply the suggestions and + recommendations you judge worthwhile. + +3. **Run the relevant tests.** Run the pytests covering the changed code (see + CLAUDE.md for commands, e.g. `pytest -n auto --reruns 2 -m "not slow"`) and + make sure there are no failures after the changes. Docs/skill-only branches + that touch no Python have no relevant tests — say so explicitly rather than + claiming a run. + +4. **Commit** the resulting changes. + +Only after this gate passes do the pre-flight safeguards and `gh pr create`. + ## After creating a PR: handle the Copilot review Every PR on this repo gets an automated Copilot review. After `gh pr create`, @@ -128,11 +150,14 @@ always run this loop before considering the PR done: resolve the thread. Either way the thread ends resolved with a written rationale. -List threads with their resolved status, and resolve them, via GraphQL (the REST -`/comments/` endpoint 404s on these review-thread comments): +List threads with their resolved status, and resolve them, via GraphQL. GraphQL +is required because thread resolution and the thread-level `isResolved` flag are +not exposed via REST (replying to a comment is available over REST — see below): ```bash -# List threads (id + resolved + comment bodies) +# List threads (id + resolved + comment bodies). +# Bump `first:` if a PR may have more than 50 threads / 10 comments per thread — +# this query is not exhaustive beyond those limits. gh api graphql -f query=' { repository(owner:"ROCm", name:"flashinfer") { pullRequest(number: ) {