feat(launchpad): deterministic pre-review pass for batch PR review (#426)

[serina-mcfall]

Summary

Extracts the mechanical half of batch PR review into launchpad/scripts/pr_review_batch.py: which change-requests are stale or misfiled, which CI failures belong to this diff, whether the reviewer is independent, leaks, placement, and drift calibration. Emits no severity and posts nothing. Same extraction pr_body_check.py and adr_boundary_check.py already did for their own rules.

Related issue

Refs #426

Issue type

Task

---

Agent provenance

Field	Value
Harness / provider	Claude Code
Model	claude-opus-5
Session reference	N/A - the harness exposes no shareable run id
Initiating human	@serina-mcfall

Objective

Add launchpad/scripts/pr_review_batch.py and its controls — a deterministic pre-review pass over a batch of pull requests.

Impacted components

• launchpad/scripts/pr_review_batch.py
• launchpad/scripts/test_pr_review_batch.py
• launchpad/scripts/test_no_model.py — one ALLOWLIST entry

Approach and rejected alternatives

ADR-0019 rules that a deterministic script may gate a merge while a model verdict may only annotate. That line runs straight through batch review: the bookkeeping is a fixed rule set, the verdicts are not. So this script does the first and refuses the second.

Rejected: emitting severity. Across three review batches five proposed blockers were demoted to High and one upheld, each turning on whether a document's conclusion depended on the defect. A script guessing that is exactly the model-gating ADR-0019 forbids, wearing automation's clothes. test_the_briefing_states_no_severity_anywhere asserts the absence rather than trusting it.

Rejected: posting reviews or filing issues from the script. Keeping side effects out means it runs read-only, and it could eventually be a required check without being able to act.

Rejected: NOT_OURS in test_no_model.py. A script that prepares review material is precisely the thing that must be provably unable to call a model, so it belongs on ALLOWLIST with its imports named.

Rejected: registering in INTERFACE.md — see Escalations.

Verification

Command run:

cd launchpad/scripts && python3 -m unittest discover -s . -t .
python3 launchpad/scripts/pr_review_batch.py --pr 288 --session-since 2026-08-21T00:00:00Z
python3 launchpad/scripts/pr_review_batch.py --pr 374 --calibrate f8692fa9b --commits 67

Raw output:

................................................................................
Ran 256 tests in 0.353s

OK

PR #288  benmitchell11  3babc1716
  fix(desktop): hide Buzz shared compute from the provider picker on builds without mesh-llm (#269)
  DO NOT DISPATCH A REVIEWER YET
  review by serina-mcfall: CURRENT -- review postdates the head commit (2026-08-21T20:55:36+00:00)
  check Desktop: UNKNOWN -- no attributable path in Desktop's output
  check Desktop Smoke E2E (4): PRE_EXISTING -- failing path(s) absent from this diff: ['tests/e2e/profile-nsec-reveal.spec.ts', 'tests/e2e/project-commit-detail.spec.ts', 'tests/e2e/project-pr-review.spec.ts']
  check Desktop Core: REAL -- failing path(s) in this diff: ['desktop/src-tauri/src/lib.rs']
  independence: INDEPENDENT -- no commit by the reviewing identity in this window

Hold: [288] -- real CI failure or independence conflict.

PR #374  tucktuck101  11a639b62
  docs(research): 19 of the 796 upstream files in the current drop can reach the deployed relay (#355)
  review by benmitchell11: CURRENT -- review postdates the head commit (2026-08-21T21:09:53+00:00)
  independence: INDEPENDENT -- no commit by the reviewing identity in this window
  calibration: at 67 commits from f8692fa9b the tip is 9891e64f6 and the diff is 796 files

Both live runs reproduce conclusions previously reached by hand: #288's real blocker is the file-size guard on lib.rs while its E2E failures are inherited, and #374's disputed "796" is exact at the pinned point.

• Tests or checks were run and the raw output is pasted above
• The diff is confined to the scope of the linked issue
• No secrets, keys, tokens or hostnames were added to tracked files

Not verified

Three defects were found by running it against live PRs, not by reading it, and each is now a regression test. That is the honest headline: the suite passed 35/35 while all three were live.

1. A false MISFILED on docs(research): 19 of the 796 upstream files in the current drop can reach the deployed relay (#355) #374 — the most damaging answer this script can give, since it tells an author to dismiss a valid review. The only path token in a genuine review was launchpad/ARCHITECTURE.md, cited as corroborating evidence rather than as a defect site. MISFILED now requires two or more cited paths, none in the diff, and no mention of any changed file.
2. Log lines selected by position. The first draft took the last 80; GitHub appends checkout teardown, so on fix(desktop): hide Buzz shared compute from the provider picker on builds without mesh-llm (#269) #288 the size-guard line had scrolled past and a REAL failure classified as UNKNOWN. Selection is by content now.
3. The path regex required :\d+ with no space — it saw compiler output (lib.rs:276:15) but not the file-size guard (lib.rs: 1000 -> 1001).

Not verified beyond that: the flake-signature and toolchain-step lists are drawn from the failures actually seen in three batches, so they are certainly incomplete — an unrecognised infrastructure failure classifies as UNKNOWN, which is the safe direction but not the useful one. UNKNOWN is deliberately not clean, but it is also not information.

The --author and --review-required selection paths were exercised only against this repository; no other repo was tried. calibrate() assumes an upstream remote is fetched and returns None otherwise rather than explaining itself well.

Security implications

Read-only by construction: every gh and git invocation is a read, nothing is posted, no file is written. subprocess is permitted in test_no_model.py's allowlist for the same reason preflight_fetch.py holds it — to spawn gh and git — and that allowlist is what proves the script cannot reach a model, which matters because ADR-0019 forbids a model verdict gating anything.

One genuinely security-relevant behaviour: the leak scanner reads added diff lines for private-tooling references and prints the matching line. It prints from a public PR's own diff, so it discloses nothing not already public, and it skips removed lines so a cleanup commit is not punished for the string it deletes.

Escalations

1. Deviation from the issue's own done-when. task: deterministic pre-review pass for batch PR review #426 asked for registration in INTERFACE.md. Not done: INTERFACE.md is task: deterministic pre-flight for the PR review agent #116's pre-flight record contract, not a script index, and an unrelated entry would degrade a contract document. The module docstring is the interface. Flagging rather than quietly dropping it, since I wrote that criterion myself.
2. The flake and toolchain lists are heuristics that will rot. They encode the infrastructure failures of one week. Worth revisiting once there is more evidence, and worth not trusting a FLAKE verdict without a glance.
3. This is eligible to become a required check under ADR-0019 — it is deterministic and emits no verdict. Not proposed here; that belongs with the CI/CD pipeline programme ADR-0019 defers enforcement to.
4. A companion private skill exists and is not in this PR. It holds the fan-out, the per-reviewer budgets, the truncation-resume protocol and the severity rubric — the judgement half, deliberately not committed because it is not deterministic and must never gate.
5. Opened as a draft because no review-final pass has run on this branch, so there is no READY verdict to justify a non-draft PR.

🤖 Drafted by Claude Code (claude-opus-5) for @serina-mcfall.