From 3ef62a07b4071037e6b4525746069ebd984e8540 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Sun, 2 Aug 2026 14:44:13 -0700 Subject: [PATCH 1/2] feat(groom): raise the pr_size_limit default from 400 to 600 lines (BE-6161) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured across both builder repos on 2026-08-02: 49 groom issues, 7 bails, exactly one patch-too-large — and that one was 403 lines changed (> 400). The other six bails are CI-privileged-file / security refusals no size change affects. So 400 was not systematically tight; it caught one boundary case, and 600 buys headroom for that shape without a large jump. 1000 (the org's human PR cap in pr-size.yml) is deliberately NOT copied: that cap counts non-generated lines with an oversized-ok bypass and is advisory on an author-defended diff, while this is a hard refusal on raw changed lines with no author to interrogate. The input description now records both reasons. pr_size_limit stays a _LOCKED_KEY — a repo must not raise its own ceiling via GROOM_CONFIG; raising the shared default under review is the sanctioned act. --- .github/workflows/groom.yml | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/.github/workflows/groom.yml b/.github/workflows/groom.yml index a956994..17750df 100644 --- a/.github/workflows/groom.yml +++ b/.github/workflows/groom.yml @@ -342,9 +342,33 @@ on: this many lines, it is NOT opened as a giant PR — it is filed as a `groom` issue for a human instead, so nothing balloons unreviewed. Ignored unless `builder` is true. + + + The default is deliberately BELOW the org's 1000-line human PR cap + (`pr-size.yml` `max_lines`), and copying that number here would loosen + this more than it looks. Two reasons. (1) Different counting basis: + `pr-size.yml` counts *non-generated* changed lines — it strips + lockfiles and generated output first, and offers an `oversized-ok` + bypass label — whereas this counts raw changed lines with no + exclusions. (2) Different risk: the human cap is advisory on a diff + whose author chose the scope and can defend it in review; this is a + hard refusal on a diff with no author to interrogate, where the + failure mode is a reviewer approving 900 plausible-looking + machine-written lines because nothing in it looks wrong. The machine + does not get the same benefit of the doubt, on purpose. + + + Raised 400 → 600 on 2026-08-02 (BE-6161) on measured evidence: across + both builder repos, 49 `groom` issues produced 7 bails and exactly ONE + patch-too-large (403 lines, three over) — 400 was not systematically + tight, it caught one boundary case, so 600 buys headroom for that + shape without a large jump. This default is the reviewed knob: it is a + `_LOCKED_KEY` in `.github/groom/config.py`, so a repo cannot raise its + own ceiling via `GROOM_CONFIG` — changing it here, under review, is + the only way it moves. type: number required: false - default: 400 + default: 600 secrets: ANTHROPIC_API_KEY: description: Anthropic API key the finder + verifier agents bill through. @@ -1811,7 +1835,7 @@ jobs: run: | set -euo pipefail mkdir -p /tmp/out - # `type: number` can arrive as a fractional string (e.g. 400.0) that the + # `type: number` can arrive as a fractional string (e.g. 600.0) that the # integer-only `-gt` test below would abort on under `set -euo pipefail`, # skipping the patch upload. Floor it to an int first (mirrors the file # job's MAX_FINDINGS handling). From 5755028e27628349001eec9044e2e0f20940a4ed Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Sun, 2 Aug 2026 14:55:17 -0700 Subject: [PATCH 2/2] docs(groom): correct two overclaims in the pr_size_limit rationale (BE-6161) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the new prose overstates two things about the size gate. - The `_LOCKED_KEY` note said review here is "the only way it moves". The lock covers the GROOM_CONFIG/config layers only and explicitly exempts caller defaults (config.py), and the builder job reads `inputs.pr_size_limit` directly — so a consumer caller can still raise its own ceiling via `with:`. Say what the lock actually buys (no un-reviewed variable-layer raise) and point an auditor at the caller. - "raw changed lines with no exclusions" oversells the bound: the count sums `git diff --cached --numstat`, which reports `-` for binary files (counted as 0), and counts lines regardless of length. Name both gaps so the basis is not read as a byte budget. --- .github/workflows/groom.yml | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/.github/workflows/groom.yml b/.github/workflows/groom.yml index 17750df..455146e 100644 --- a/.github/workflows/groom.yml +++ b/.github/workflows/groom.yml @@ -349,8 +349,14 @@ on: this more than it looks. Two reasons. (1) Different counting basis: `pr-size.yml` counts *non-generated* changed lines — it strips lockfiles and generated output first, and offers an `oversized-ok` - bypass label — whereas this counts raw changed lines with no - exclusions. (2) Different risk: the human cap is advisory on a diff + bypass label — whereas this counts changed lines straight off + `git diff --cached --numstat`, with no content-based exclusions. Note + that this basis is not a byte budget: `numstat` reports `-` for binary + files (counted as 0 here), and a line is a line however long, so one + minified line clears any ceiling. It bounds review effort in the + reviewable case, not patch size in the worst case — another reason not + to spend the headroom the human cap allows. (2) Different risk: the + human cap is advisory on a diff whose author chose the scope and can defend it in review; this is a hard refusal on a diff with no author to interrogate, where the failure mode is a reviewer approving 900 plausible-looking @@ -363,9 +369,13 @@ on: patch-too-large (403 lines, three over) — 400 was not systematically tight, it caught one boundary case, so 600 buys headroom for that shape without a large jump. This default is the reviewed knob: it is a - `_LOCKED_KEY` in `.github/groom/config.py`, so a repo cannot raise its - own ceiling via `GROOM_CONFIG` — changing it here, under review, is - the only way it moves. + `_LOCKED_KEY` in `.github/groom/config.py`, so the un-reviewed layer — + a repo's `GROOM_CONFIG` Actions variable, editable with repo write and + leaving no diff — cannot raise it. It can still be raised per-repo the + reviewed way, by a `with: pr_size_limit:` in that repo's caller (the + builder job reads `inputs.pr_size_limit` directly), which is a commit + on a branch someone has to approve. So auditing a repo's effective + ceiling means reading its caller, not assuming this default. type: number required: false default: 600