diff --git a/.claude/commands/pr-synthesis.md b/.claude/commands/pr-synthesis.md new file mode 100644 index 0000000000..f813adbc6b --- /dev/null +++ b/.claude/commands/pr-synthesis.md @@ -0,0 +1,76 @@ +--- +description: Produce a tight 1–3 paragraph what / why / how synthesis of a single PR. Fetches title, body, linked issue, and diff scope; outputs per `docs/skills/pr-context-synthesis.md`. Read-only. +--- + +Synthesise PR: $ARGUMENTS + +## Parse $ARGUMENTS + +Accept any of: + +- A PR number: `4267`/`#4267` +- A full URL: `https://github.com/cowprotocol/services/pull/4267` +- An `owner/repo#N` form: `cowprotocol/services#4267` + +Default `owner/repo` to `cowprotocol/services` when only a number is given. + +If `$ARGUMENTS` is empty or unparseable, print: + +``` +Usage: /pr-synthesis + /pr-synthesis https://github.com/owner/repo/pull/ + /pr-synthesis owner/repo# +``` + +and abort. + +## Procedure + +Fetch in this order — context first, then diff. Lets the synthesis read the diff with the author's intent already in mind, and lets `gh pr view` report `additions`/`deletions` so the diff fetch can be sized appropriately. + +**Step 1 — PR metadata + closing issues.** Cheap; bounded size. + +```bash +gh pr view -R / --json title,body,files,additions,deletions,labels,baseRefName,headRefName,closingIssuesReferences +``` + +**Step 2 — linked issues.** For each entry in `closingIssuesReferences` (GitHub's own parsing of `Fixes #N` / `Closes #N` / `Resolves #N`, more reliable than regexing the body, and follows cross-repo references correctly), fetch the issue: + +```bash +gh issue view -R / --json title,body,labels,state +``` + +If `closingIssuesReferences` is empty, proceed without a linked issue — never invent one. + +**Step 3 — diff fetch (size-gated).** Use `additions + deletions` from step 1 to decide: + +- **`additions + deletions <= 2000`** — fetch the full diff: + + ```bash + gh pr diff -R / + ``` + +- **`additions + deletions > 2000`** — *do not* fetch the full diff. It can blow the context window (e.g. PR #4217 = 376k lines). Get the complete per-file list via the paginated REST endpoint — `gh pr view --json files` silently caps at 100 files, which underreports scope on big PRs: + + ```bash + gh api --paginate "repos///pulls//files" \ + --jq '.[] | {filename, status, additions, deletions}' + ``` + + Build `` from these per-file records (`filename`, `additions`, `deletions`, `status` — added / modified / renamed / removed). Bucket lockfiles, generated bindings, vendored artifacts, and codegen output — call them out as a single line each, not file-by-file. State in the synthesis that the diff was summarised at file-scope only. + +Build: + +- `` = full hunks (small PR) or per-file scope buckets (large PR). The ground truth. +- `` = title + body +- `` = fetched issue(s), if any + +Then follow `docs/skills/pr-context-synthesis.md` Rules and Shape. Output the synthesis verbatim — no header, no metadata, no separator lines. The caller pastes this directly into a review thread, an incident report, or a Slack message. + +## Rules + +- **Read-only.** `gh pr view`, `gh pr diff`, `gh issue view`, `gh api` GET-only. No `gh pr review`, no `gh pr comment`, no mutating `gh api` verbs. +- **Don't fetch a diff you can't read.** The 2000-line gate above is a hard preflight, not a suggestion. Above the gate, the per-file scope is enough to write the synthesis honestly; trying to swallow a 100k-line diff just truncates the context and corrupts the output silently. +- **Synthesise, don't copy-paste.** If `` is sparse, say so plainly: *"description is minimal; intent inferred from diff"*. +- **No vague verbs.** *"This PR updates something"* is a failure. Name the component, the change, and the mechanism. The anti-vague-verb rule from the skill doc applies verbatim. +- **Watch for description-vs-diff drift.** `` must describe ``'s current state. If a claim is no longer true of the diff, note it in the synthesis as *"description claims X; diff shows Y"*. Fetching the issue + metadata before the diff is what lets you spot these — read the intent first, then check whether the diff matches. diff --git a/docs/skills/pr-context-synthesis.md b/docs/skills/pr-context-synthesis.md new file mode 100644 index 0000000000..941b5f22e5 --- /dev/null +++ b/docs/skills/pr-context-synthesis.md @@ -0,0 +1,49 @@ +# Skill — PR context synthesis + +Use to produce a tight 1–3 paragraph *what / why / how* block for a single PR (or PR-shaped change). Consumed by the PR review report's CONTEXT section and by other workflows that need a per-PR summary (e.g. ad-hoc *"summarise this PR for me"* or per-candidate context in an incident-investigation walk). + +## How to invoke + +Two ways: + +- **Procedurally** — follow the rules below when you need a tight 1–3 paragraph synthesis (CONTEXT block of `/review-pr`, ad-hoc *"summarise this PR for me"*, per-candidate context for an investigation). +- **Via slash command** — `/pr-synthesis ` fetches the PR, linked issue, and diff for you and prints the synthesis verbatim. + +## Inputs + +Listed ground-truth-first; downstream items are interpreted relative to the diff. + +- `` — file scope plus a codemap or per-file note of the actual change. The ground truth the synthesis must stay anchored to. For large PRs this may be file-scope only (paths + ±counts + change type), without hunks — `/pr-synthesis` falls back to that when the diff is too big to fetch in full. +- `` — PR title and body. If there is no PR yet (e.g. local-diff mode), use the current branch name and the relevant commit messages (i.e. diff against `main`). +- `` — title and body of any issue referenced via `Fixes #N` / `Closes #N` / `Resolves #N`. May be empty. + +## Rules + +1. **Synthesize, don't copy-paste.** If `` is five words, say so plainly: *"description is minimal; intent inferred from diff"*. Don't pad to look thorough. +2. **Watch for description-vs-diff drift.** `` must describe ``'s *current* state, not the author's iteration history. If a claim is no longer true of the diff, note it in the synthesis as *"description claims X; diff shows Y"*. Don't raise an `Action:` finding here — this skill reports facts; the consumer (e.g. `/review-pr`) decides whether to escalate. Do **not** flag the absence of a changelog of removed/superseded behaviour either — that belongs in commit history, not the description. +3. **No vague verbs.** *"This PR updates something"* is a failure. Name the component, the change, and the mechanism. + +## Shape + +- **Paragraph 1** — *what* changed. Component + concrete change, drawn from ``. +- **Paragraph 2** — *why*. Drawn from `` and ``. If both are thin, say so. +- **Paragraph 3** (only if warranted) — *how*. The approach, not a line-by-line walkthrough. + +## Example + +Inputs (real PR — cowprotocol/services#4371): + +- `` — *"Enforce EIP-7825 per-tx gas cap on settlement"*; body explains the Fusaka mempool cap and references the existing quote-side enforcement (#4261). +- `` — #4368, *"Driver doesn't enforce the EIP-7825 per-tx gas cap"* (labels: bug, good first issue). +- `` — +58 −1 in `settlement.rs`; new const `EIP_7825_TX_GAS_CAP`, `Gas::new` capped via `min(half_block, EIP_7825)`, three tests. + +Output: + +> Fusaka introduced EIP-7825, capping any single tx at 2^24 − 1 gas. The driver's `Gas::new` was applying only the older `block_limit/2` heuristic; a solution between the EIP-7825 cap and that heuristic could pass validation in `/solve` and never settle. Issue #4368 flagged this as theoretical (no production hits); the fix ports the same idea PR #4261 applied to the quote path. +> +> Mechanically: `max_gas = min(half_block, EIP_7825)`, preserving inclusion-economics on chains with low block gas limits via the `min`. + +## When to skip + +- Trivial changes (docs typo, single-line dep bump, lockfile-only). One sentence is enough; don't force the three-paragraph shape. +- `` and `` are both empty *and* `` already speaks for itself (e.g. a single-file rename). One sentence noting that the diff is self-explanatory is the correct output.