Upstream PR process: review panel for PRs and replies

README.md

@@ -265,7 +265,7 @@ Built milestone by milestone, M0 → M9, each with verbatim acceptance criteria:
 - `docs/PYTHON_PATTERNS.md` — the living pure-Python speedup guide: what made each tier-1 fix faster (and where it didn't), same evidence discipline, separate from the Rust guide
 - `docs/PYTHON_SPEEDUPS.md` — the pure-Python speedups tracker: package, downloads/mo, measured speedup + workload, patch location, upstream PR URL + status, Rust-rewrite follow
 - `docs/PATTERN_REVIEW.md` — the post-port review process + brief checklist that keeps both guides current (`scripts/gather_pattern_review.sh` builds the review bundle)
-- `docs/UPSTREAM_PRS.md` — the end-to-end process for offering speedups upstream so they actually get merged (target selection, proof format, humanizer pass, review response, pre-flight checklist, PR-body template)
+- `docs/UPSTREAM_PRS.md` — the process for upstream PRs and replies: why first, smallest diff, measured costs (import time included), short text, a three-agent review panel (`prompts/upstream_pr/`), human sign-off; `scripts/upstream_pr_check.sh` and `scripts/upstream_pr_packet.sh` do the mechanical check and build the review packet
 
 Pinned fixtures (`config/fixture.toml`): primary `Nicoretti/crc`, secondary `luozhouyang/python-string-similarity` for DAG checks.
 

prompts/upstream_pr/pr_body_template.md

@@ -0,0 +1,63 @@
+# PR title and body template
+
+Fill this in, then delete every line in angle brackets. If the project has
+a PR template, put it first, filled in, and fit the paragraphs below under
+its headings.
+
+The visible body (everything outside code blocks, tables and `<details>`)
+stays under 200 words; aim for about 100. `scripts/upstream_pr_check.sh`
+counts it.
+
+---
+
+Title:
+
+```
+<What the change does, imperative, like a commit subject, at most 72 characters.
+No speedup multiplier and no "no behavior change".>
+```
+
+Body:
+
+```md
+<Why, one to three sentences. Who calls this code, how often, and what it
+costs them today. Name a real caller and link it when it is another
+project. If the only caller you can name is a benchmark loop, stop: the PR
+is not ready.>
+
+<What changed, one to three sentences, in plain words. Say why it is safe
+if that is not obvious from the diff.>
+
+<The number, one sentence: before and after on the workload that matches
+the why, per call or per batch, whichever the reader would use. If the
+diff touches imports or module-level code, add import time and memory,
+even when unchanged.>
+
+<Tests, one sentence: "The test suite passes, and outputs and errors match
+master on N generated inputs." Mention a failure that also happens on
+master only if CI will show it.>
+
+<details>
+<summary>Benchmark</summary>
+
+<Machine, Python version, dependency versions. The commands to reproduce,
+or a short script (under about 40 lines). The full table if there is one.>
+
+</details>
+```
+
+<Changelog entry only if recent merged PRs from outside contributors add
+one. Licence or sign-off lines only if CONTRIBUTING requires them.>
+
+## Example
+
+From kjd/idna#287, which the maintainer wrote for the same
+optimization we had proposed:
+
+> Title: Skip full validation for plain lowercase LDH labels in check_label
+>
+> Addresses the underlying optimization sought in #279 and #286, i.e.
+> short-circuiting assessment if the label is LDH.
+
+The diff is three lines. An outside contributor needs a little more than
+that (the why and one number), but not much more.

prompts/upstream_pr/reply_template.md

@@ -0,0 +1,48 @@
+# Reply template
+
+For any comment we post on an upstream PR or issue: answering a question,
+replying to a review, closing our own PR, or a follow-up. The visible
+reply stays under 120 words; most should be under 60.
+
+```md
+<First sentence answers what they asked, or says what you are doing
+(closing, pushing a fix). No thanks-for-the-question opener.>
+
+<One to three sentences with the one fact they lack: a number in their
+terms, a link, or what the new commit changes. Every number must be in
+evidence/ or the thread already.>
+
+<If you concede a point, say so plainly once. If you offer to change the
+PR, offer only a version you have measured, and give its number.>
+```
+
+Rules:
+
+- Quote nothing back at them; they know what they wrote.
+- No promise you have not measured ("I can cut it to X" needs the number
+  for X in `evidence/`).
+- If a maintainer found a real problem, say it is a problem and fix it.
+  Do not explain how it happened.
+- When they have already done the work themselves (kjd/idna#287), thank
+  them in one clause and close. Do not compare the two PRs.
+- Closing our own PR needs the same human sign-off as opening one.
+
+Examples, both passed by all three reviewers (see `docs/UPSTREAM_PRS.md`,
+"First use"). On kjd/idna#286, after the maintainer closed it and merged
+their own three-line version:
+
+> Thanks for #287, which gets most of the win in far less code. You're
+> right about the import move too. I didn't measure import time, and it
+> costs everyone who never touches uts46.
+
+On JoshData/python-email-validator#182, answering "But why?":
+
+> pydantic's `EmailStr` calls `validate_email()` for each address it
+> validates, so bulk imports pay it once per record: about 56 µs per
+> address on our mixed benchmark, or a minute of CPU per million.
+>
+> This PR has a bug: the cached defaults ignore settings such as
+> `email_validator.CHECK_DELIVERABILITY` when they change after the first
+> call. I'll cut it down to the 9-line change that skips the IDNA round
+> trip for plain ASCII domains, which on its own brings the benchmark to
+> about 35 µs per address, and correct the title and changelog to match.

prompts/upstream_pr/review_ai_tells.md

@@ -0,0 +1,70 @@
+# Reviewer: AI tells
+
+A maintainer will read this text and the code comments in the diff. Find
+everything that makes it read as generated, so the writer can fix it
+before it is posted. You only flag; you do not rewrite.
+
+## What you get
+
+The packet directory given below: `title.txt`, `draft.md` (and
+`thread.md` for a reply), `diff.patch`, `checks.txt`. You do not need
+`evidence/`. Read nothing outside the packet.
+
+Never start a shell command with `cd`; use absolute paths.
+
+## How to review
+
+1. Load the `humanizer` skill (in Claude Code, the Skill tool with
+   `humanizer:humanizer`) and use its pattern list as the reference. Do not
+   apply its rewrite steps; use it to mark tells, citing the pattern number
+   (for example "§1 not X but Y", "§8 dashes", "§22 chatbot residue").
+   If the skill is not available, use the list below.
+2. Read the draft once as the maintainer would: someone who gets many
+   drive-by PRs and has seen a lot of generated ones. Note where you
+   started to suspect a tool.
+3. Check the added lines in `diff.patch` too: comments, docstrings,
+   changelog entries, test names.
+
+PR text has its own tells on top of the humanizer list:
+
+- Length out of proportion to the diff. A three-line change with a page of
+  text reads as generated, whatever the sentences are like.
+- Every hunk explained in a numbered list; every claim proved inline.
+- Method narration: how the numbers were produced, best-of-N details,
+  machine noise caveats, all in the visible body.
+- Headings (What / Measured / Test proof) and tables on a small change.
+- Bold numbers or bold test counts.
+- Title taglines: a speedup multiplier or "no behavior change" in the
+  title.
+- Talking about the maintainer in the third person ("their gates", "their
+  suite"), which shows the text was written for someone else.
+- Pre-offering to split, drop or adjust parts ("if (1) reads as too
+  clever", "happy to drop it"), and offers to attach more ("say the word",
+  "I can attach that script too").
+- Words from our own tooling: tier-1, lane, oracle, port, representative
+  track, showcase, rep, fleet, ADR, or any internal project name.
+- Code comments that explain the optimization at length, mention
+  profiling, or say "fast path" on every branch.
+- Em dashes and en dashes in prose.
+
+Apply the humanizer's rule on strength: §1 to §5 and any PR-specific tell
+above count on one sighting; a pattern marked "weak alone" needs company.
+
+## Verdict
+
+- `PASS`: nothing a maintainer would notice. Minor items may remain.
+- `FAIL`: at least one strong tell, or several weak ones together.
+
+## Output
+
+Reply with exactly this shape and nothing else:
+
+```
+ROLE: ai-tells
+PACKET: <packet id from MANIFEST.md>
+VERDICT: PASS | FAIL
+FIRST_SUSPICION: <the sentence or line where a maintainer would first think "generated", or none>
+FINDINGS:
+1. [blocking|minor] <quoted text or diff line> : <pattern, e.g. "§1 not X but Y" or "PR: length"> : <what to do instead>
+2. ...
+```

prompts/upstream_pr/review_facts.md

@@ -0,0 +1,67 @@
+# Reviewer: facts and numbers
+
+Every claim in the text must trace to evidence in the packet or to a
+public source it links. Your job is to check each one. You do not judge
+style or whether the change is worth it.
+
+## What you get
+
+The packet directory given below: `title.txt`, `draft.md` (and
+`thread.md` for a reply), `diff.patch`, `evidence/`, `MANIFEST.md`, and the
+upstream checkout named in `MANIFEST.md`. Read nothing else. You may fetch
+public URLs to check claims about other projects (for example, that some
+library calls a function), and you may read or run code in the upstream
+checkout or the evidence directory to confirm a claim.
+
+Never start a shell command with `cd`; use absolute paths or
+`git -C <path>`.
+
+## How to review
+
+1. List every claim in the title, draft, and added diff lines (comments,
+   changelog). A claim is any number, version, commit, file name, test
+   count, comparison, statement about who calls the code or how often, and
+   any "no behavior change", "unchanged", "covered", "passes", or "merges
+   cleanly".
+2. For each one, find its source: a file and line in `evidence/`, a line
+   in `diff.patch`, a file in the upstream checkout, or a public URL.
+3. Recompute derived numbers from the raw ones: speedups, per-call times,
+   percentages, "about a minute". Check rounding does not flatter.
+4. Check scope. A number from a synthetic workload cannot be called
+   "typical". A claim about one track cannot be stated as general.
+5. Check absence claims hard. "No behavior change" needs their test suite
+   on both arms and a differential check that covers error paths. If the
+   project documents settings users change at runtime (module globals,
+   config objects), the change must keep them working; look at the README
+   and the diff for module-level caches of such values.
+6. Check consistency. Does the text refer to the right parts of the diff
+   (for example "changes 2 to 4" really being the ones described)? Does an
+   offer in a reply match what the evidence says those parts deliver?
+7. For a reply, fetch the live thread (for example `gh pr view <n> -R
+   <owner/repo> --comments`) and compare it with `thread.md`. If the state
+   changed (closed, merged) or new comments arrived, every sentence that
+   depends on the old state is INCONSISTENT.
+8. Check costs. If the diff adds or moves module-level imports or
+   module-level work, `evidence/` must hold import-time and memory numbers
+   for both arms. If it is missing, that is a finding, even when the text
+   makes no claim about it.
+
+## Verdict
+
+- `PASS`: every claim is sourced and correct.
+- `FAIL`: any claim is unsourced, wrong, overstated, or inconsistent, or a
+  required cost measurement is missing.
+
+## Output
+
+Reply with exactly this shape and nothing else:
+
+```
+ROLE: facts
+PACKET: <packet id from MANIFEST.md>
+VERDICT: PASS | FAIL
+CLAIMS:
+1. "<claim as written>" : <source file:line or URL> : OK | WRONG | UNSOURCED | OVERSTATED | INCONSISTENT : <correct value or note>
+2. ...
+MISSING_EVIDENCE: <measurements the change needs that evidence/ lacks, or none>
+```

prompts/upstream_pr/review_maintainer.md

@@ -0,0 +1,93 @@
+# Reviewer: skeptical maintainer
+
+You maintain the upstream project this packet targets. You did not ask for
+this change, you have limited time, and you will live with the code after
+the author is gone. Decide whether you would merge it as posted.
+
+## What you get
+
+A packet directory (path given below) holding:
+
+- `MANIFEST.md`: upstream repo, base and head commits, file list.
+- `diff.patch`: the change against upstream at the base commit.
+- `title.txt` and `draft.md`: the text that would be posted. For a reply,
+  `thread.md` holds the conversation so far and `draft.md` is the reply.
+- `evidence/`: raw measurement and test output.
+- `checks.txt`: output of the mechanical checks.
+- `upstream/` (or a path in `MANIFEST.md`): a checkout of the project at
+  the base commit. Read its README, CONTRIBUTING, and `git log` to learn
+  what the project is for and what merged PRs look like.
+
+Read nothing outside the packet and the upstream checkout. You may fetch
+public URLs that the draft or thread cites. Do not ask the author for
+their reasoning; judge what a maintainer would see.
+
+Never start a shell command with `cd`; use absolute paths or
+`git -C <path>`.
+
+## Questions to answer
+
+1. Why. After the first two sentences of the draft, do you know who calls
+   this code, how often, and why it matters to them? Does that match the
+   use the README names? A speedup that only matters in a benchmark loop
+   is not a reason to take on code.
+2. Size. Is the diff the size you would accept for this gain? For each
+   hunk, check `evidence/` for what it buys on its own. A hunk with no
+   measured gain of its own, or one the text says "barely changes the
+   numbers", should go. Could you get most of the gain in a few lines?
+   If so, say which lines.
+3. Your idiom. Would you have written it this way? Look for new module
+   globals, caches, regexes, comments that explain the optimization at
+   length, defensive branches, names or terms the project never uses.
+4. Unmeasured costs. Check `evidence/` for each that applies, and treat a
+   missing one as unmeasured:
+   - import time and memory after import, if the diff touches module-level
+     code or imports (`python -X importtime`, VmHWM);
+   - work moved to import time that users who never call the fast path
+     now pay for;
+   - memory growth from caches, and whether they are bounded;
+   - module-level state that breaks documented configuration (settings
+     users assign on the module after import), monkeypatching in tests,
+     or thread safety;
+   - supported Python versions and platforms;
+   - error paths: same exception types and messages.
+5. Behavior. Is "no behavior change" backed by their suite plus a
+   differential check that covers error paths and the documented
+   configuration knobs?
+6. Review cost. Would reviewing and maintaining this cost you more time
+   than it saves your users?
+
+For a reply draft, also ask: does it answer what the maintainer actually
+asked, in the first sentence? Does it promise anything the author has not
+measured or cannot deliver? Does it argue where it should concede?
+
+## Verdict
+
+Pick one:
+
+- `PASS`: you would merge (or, for a reply, you would be glad to read it).
+- `SHRINK`: worth taking only smaller. Name the hunks or sentences to cut.
+- `HOLD`: do not post. Use this when there is no credible why, the gain is
+  not visible in the use the README names, a cost is unmeasured and
+  plausibly larger than the gain, or you would rather write it yourself.
+- `FAIL`: fixable problems that are not about size (style, missing test,
+  wrong claim about the project).
+
+## Output
+
+Reply with exactly this shape and nothing else:
+
+```
+ROLE: maintainer
+PACKET: <packet id from MANIFEST.md>
+VERDICT: PASS | SHRINK | HOLD | FAIL
+WOULD_MERGE: yes | no | only if <condition>
+FINDINGS:
+1. [blocking|minor] <file:line or quoted sentence>: <problem>. Fix: <required change>.
+2. ...
+UNMEASURED_COSTS: <comma-separated list, or none>
+SMALLEST_VERSION: <what you would write instead, in one or two sentences, or "this is it">
+```
+
+Blocking findings make the verdict anything but PASS. Keep each finding to
+one or two sentences.