From fb485f6cf513a00f7463b84875a9be932e08292c Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 25 Oct 2026 18:12:10 -0700 Subject: [PATCH 1/3] Render pr_review's Digest From a Pure Leaf Module (#3493) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Implements #3457 (the closing keyword goes on the promotion pull request). - `scripts/pr_review_digest.py` is a new pure reader module. Its `render` takes the payload, the normalized checks, the run log's rate-limit sentence, and the seen set as arguments, and makes no call. It imports only the other readers and the reviewer adapter, never the gate. - `digest` stays in `pr_review.py` as the shell that does the reads (`full_read`, `check_nodes`, and `confirmed_quota` when a refusal is present), so the tests that patch it there keep working. - The `status` header is an ordered list of `(key, value)` pairs joined by one space, a `None` value leaving the field out, so a new field is one entry. - The window, threads-truncated, and reviews-truncated advisory lines print from an ordered `(value, template)` table, in the order they printed before. - `confirmed_quota` is now read once per digest rather than up to twice, so `refusal=QUOTA` and the `RATE LIMIT` line can no longer disagree across two run-log reads. - The new module joins the readers' import guard, the whole-source guards, and the size ceilings, and `scripts/README.md` names it. ## Verification - Every `digest` output the existing suite produces, 256 renderings across `tests/test_pr_review.py`, was recorded before and after the move and compared byte for byte, identical. - `TestDigestRenderIsPinned` pins a crowded and a quiet digest whole, with text captured from the pre-move code. - The full suite passes, `python -m unittest discover -s tests`, 2963 tests, 7 skipped. Ruff, mypy, and the pre-commit hook are clean. - A local strict review ran two rounds. Round 1 raised two `introduced` findings, the new helper's name colliding with `pr_review_rounds.refusal_of` and the pin test's docstring claiming more than it pins, both fixed in 023589f8. Round 2 raised none. ## Pre-existing findings, not filed - `scripts/README.md:9`, `scripts/README.md:190`, `scripts/pr_review.py:16`: `pre-existing`, each an older sentence over the opt-in sentence-length check's 25 words, flagged only because its line is in this diff. The `comments` label is applied because four comment lines moved verbatim from `pr_review.py` into the new module. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- scripts/README.md | 4 +- scripts/pr_review.py | 263 +---------------------------- scripts/pr_review_digest.py | 318 +++++++++++++++++++++++++++++++++++ tests/test_pr_review.py | 100 ++++++++++- tests/test_pr_review_size.py | 1 + 5 files changed, 422 insertions(+), 264 deletions(-) create mode 100644 scripts/pr_review_digest.py diff --git a/scripts/README.md b/scripts/README.md index 6b54bfeb..b6e3dc44 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -6,7 +6,7 @@ The fleet's checks and review tooling, run by hand, with the deterministic ones Python only, and standard library only, with markdown-it-py the one exception. `build_dist.py`, `canonical_review.py`, `carry.py`, `pr_review.py`, `pr_review_copilot.py`, and `pr_cleanup.py` declare it in their inline script metadata. `build_dist.py` and `carry.py` import it only on the path that reads a region, since `skills_install.py` and `handoff.py` import them under a plain `python3`. Every check script is read-only and exits non-zero on a finding. `build_dist.py`, `local_review.py`, `canonical_review.py`, `handoff.py`, and `skills_install.py` below are the exceptions, since a generator, two recorders, an installer, and the handoff chain all exist to write. `build_dist.py --check` and `skills_install.py --report` are read-only modes for CI and for asking without changing anything. `local_review.py status` and `check` answer without recording anything, but they are not read-only in that same sense: reading the working tree stages it, which writes inside `.git` and runs any `filter.*.clean` the repository configures, and only the receipt is left untouched. `canonical_review.py` reads files rather than staging them, so its `list`, `status`, `sweep`, and `report` are read-only outright, and `record` alone writes. -The directory separates its kinds by name and by tree. A gate checks and exits non-zero on a finding, and its name carries a `_lint` or `_gate` suffix saying what it gates. The `prose_lint.py` and `repo_gate.py` entry points delegate to the action-owned implementations that gate this tree in CI, and `host_gate.py` gates the machine it runs on. A utility does work rather than gating and carries no suffix: `backlog_metrics.py`, `build_dist.py`, `canonical_review.py`, `handoff.py`, `local_review.py`, `pr_cleanup.py`, `pr_review.py`, `pr_review_copilot.py`, `skills_install.py`. `gh_graphql.py` is a library with no command line, the GraphQL transport and cursor walker that `backlog_metrics.py`, `handoff.py`, `pr_cleanup.py`, and `pr_review.py` import from beside them. `pr_review_checks.py` and `pr_review_rounds.py` are libraries with no command line as well. They are the pure readers `pr_review.py` imports for the head's checks and its review rounds. The unit tests live apart at the repository root under [`tests/`][tests], one `test_