feat(pr-size): opt-in exclude_tests so the cap measures production code (BE-6791)

The size cap counts added + deleted lines across a PR's net diff and already
excludes generated files and dependency lockfiles. Test files were not
excluded, so a change that is mostly test coverage trips the cap even when the
production diff is small — and the only escape was the `oversized-ok` bypass
label, which signals "legitimately large" rather than "mostly tests".

Add an `exclude_tests` input (default false) that keeps test-file lines out of
the counted total. Test files are recognized by path convention: `*_test.go`;
`test_*.py` / `*_test.py` / `conftest.py`; `.test.` / `.spec.` before a JS/TS
source extension; and any file under a `test/`, `tests/`, `testing/`,
`testdata/`, `e2e/`, `__tests__/`, `__mocks__/` or `__snapshots__/` directory
segment.

Off by default deliberately. `bump-callers.sh` auto-opens SHA-bump PRs into
every calling repo, so a default-on change would propagate a weaker guardrail
fleet-wide with nobody opting in. Per-repo knobs are the established shape here
(`extra_lockfiles`, `extra_generated_globs`, `mode: warn`).

Detection is a naming convention, not a proof — unlike the Go generated marker,
which must precede the package clause, and `linguist-generated`, read from the
base ref so a PR cannot exempt itself. Nothing stops production code being
parked in `tests/`. Two mitigations: the opt-in itself, and the excluded total
is ALWAYS reported on its own line (`Excluded (tests): N`), so a large
test-only PR is visible rather than silently small. When the knob is off, test
lines are still broken out of the counted number so the option is discoverable.

Matching is on whole path segments and separator-anchored suffixes, so
`contest/`, `attestation/`, `latest.go`, `protest.go` and `manifest.ts` are
untouched. `spec/` is deliberately NOT a test directory — in this org it holds
OpenAPI schemas, which are production artifacts.

`Evaluate` now takes a `Policy` struct rather than a third positional bool;
adjacent bool parameters are silently swappable at a call site. The three
buckets never overlap — a generated file that is also a test counts once, as
generated — so counted + generated + test always sums to the diff's non-binary
changed lines regardless of policy.

Verified against Comfy-Org/cloud#6343 (the PR that prompted this): 1678 counted
today, 396 counted with the flag on, 1282 reported as test, with its
`openapi.yaml` correctly staying counted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: synap5e

.github/workflows/pr-size.yml

@@ -13,6 +13,18 @@ name: PR Size Cap (reusable)
 #     marker before the package clause (this rule simply never matches in
 #     non-Go repos, so it is unconditional)
 #   - files matching `extra_generated_globs`
+#   - test files, ONLY when `exclude_tests: true` (off by default) — see below
+#
+# `exclude_tests` (opt-in, per repo): keeps test-file lines out of the counted
+# total so a PR is capped on the production code a reviewer must actually reason
+# about, not on its test coverage. Test files are recognized by naming
+# convention only (`*_test.go`, `test_*.py`/`*_test.py`/`conftest.py`,
+# `*.test.ts`/`*.spec.ts` & friends, and paths under `test/`, `tests/`,
+# `testing/`, `testdata/`, `e2e/`, `__tests__/`, `__mocks__/`,
+# `__snapshots__/`). Unlike the generated-file rules this is a convention, not a
+# proof — nothing stops production code being parked in `tests/` — which is why
+# it is opt-in rather than the default. The excluded total is ALWAYS reported on
+# its own line, so a large test-only PR cannot pass unremarked.
 #
 # Bypass: add the `bypass_label` (default `oversized-ok`) to the PR for a
 # legitimately large change. The check still runs (so it always posts a
@@ -104,6 +116,20 @@ on:
         type: string
         required: false
         default: ''
+      exclude_tests:
+        description: >-
+          Keep test-file lines out of the counted total, so the cap measures
+          production code rather than test coverage. Test files are matched by
+          naming convention (`*_test.go`, `test_*.py`, `*.test.ts`,
+          `*.spec.ts`, and paths under test/, tests/, testing/, testdata/,
+          e2e/, __tests__/, __mocks__/, __snapshots__/). Off by default: it
+          loosens the cap, so each repo opts in deliberately. Excluded test
+          lines are always reported separately, never silently dropped. For a
+          layout these conventions miss, use `extra_generated_globs` (those
+          land in the generated bucket instead).
+        type: boolean
+        required: false
+        default: false
       comment:
         description: >-
           Post the sticky over-cap PR comment (needs bot_app_id +
@@ -144,6 +170,7 @@ env:
   PR_SIZE_BYPASS_LABEL: ${{ inputs.bypass_label }}
   PR_SIZE_EXTRA_LOCKFILES: ${{ inputs.extra_lockfiles }}
   PR_SIZE_EXTRA_GENERATED_GLOBS: ${{ inputs.extra_generated_globs }}
+  PR_SIZE_EXCLUDE_TESTS: ${{ inputs.exclude_tests }}
 
 jobs:
   pr-size:

README.md

@@ -19,7 +19,7 @@ complete, copy-pasteable caller.
 | [`cursor-review-auto-label.yml`](.github/workflows/cursor-review-auto-label.yml) | Companion to `cursor-review.yml`. On PR assignment, applies the review label for an opted-in reviewer (via the CLOUD_CODE_BOT app token, so the label actually triggers the review). The opt-in roster lives in the caller's `vars.CURSOR_REVIEW_OPTED_IN_LOGINS` — no roster is baked into the workflow. Requires `vars.APP_ID` + `CLOUD_CODE_BOT_PRIVATE_KEY`. | [cursor-review-auto-label.md](docs/callers/cursor-review-auto-label.md) |
 | [`assign-reviewers.yml`](.github/workflows/assign-reviewers.yml) | Auto-requests expertise-aware, load-balanced PR reviewers with new-folk randomization. Matches changed paths against a caller-repo `.github/reviewers.yml` (path-glob → reviewers, plus a `default_pool`), drops the author + `vars.REVIEWER_EXCLUDE`, ranks candidates by open review load (steering off anyone at/over `vars.REVIEWER_LOAD_CAP`), and may swap a slot for a `vars.REVIEWER_GROWTH_POOL` member. Requests go through the CLOUD_CODE_BOT app token so they work on fork PRs. Requires `vars.APP_ID` + `CLOUD_CODE_BOT_PRIVATE_KEY`. | [assign-reviewers.md](docs/callers/assign-reviewers.md) |
 | [`assign-prs-to-author.yml`](.github/workflows/assign-prs-to-author.yml) | Housekeeping — assigns every open PR with no assignees to its author (bot-authored PRs skipped by default). Run on a schedule from a thin caller; useful when a team tracks PR ownership via assignees. The calling job needs `pull-requests: write` and `issues: write`. | [assign-prs-to-author.md](docs/callers/assign-prs-to-author.md) |
-| [`pr-size.yml`](.github/workflows/pr-size.yml) | PR-size cap — fails (or, in `mode: warn`, only reports) when a PR's net diff exceeds `max_lines` non-generated changed lines, keeping diffs reviewable. Excludes dependency lockfiles, `linguist-generated` files (read from the base ref, so a PR can't exempt itself), Go generated-code markers, and per-repo `extra_lockfiles` / `extra_generated_globs`. A `bypass_label` (default `oversized-ok`) waves through a legitimately large change; a sticky bot comment explains overages when `bot_app_id` + `BOT_APP_PRIVATE_KEY` are supplied (degrades to status + step summary without them). Counting logic + tests live in [`scripts/check-pr-size/`](scripts/check-pr-size). | [pr-size.md](docs/callers/pr-size.md) |
+| [`pr-size.yml`](.github/workflows/pr-size.yml) | PR-size cap — fails (or, in `mode: warn`, only reports) when a PR's net diff exceeds `max_lines` non-generated changed lines, keeping diffs reviewable. Excludes dependency lockfiles, `linguist-generated` files (read from the base ref, so a PR can't exempt itself), Go generated-code markers, and per-repo `extra_lockfiles` / `extra_generated_globs`. Opt in to `exclude_tests` to cap production code rather than test coverage (excluded test lines are always reported, never silently dropped). A `bypass_label` (default `oversized-ok`) waves through a legitimately large change; a sticky bot comment explains overages when `bot_app_id` + `BOT_APP_PRIVATE_KEY` are supplied (degrades to status + step summary without them). Counting logic + tests live in [`scripts/check-pr-size/`](scripts/check-pr-size). | [pr-size.md](docs/callers/pr-size.md) |
 | [`pr-risk.yml`](.github/workflows/pr-risk.yml) | **Advisory PR risk grading (shadow check)** — **automatic grading off by default** (`enabled: false`; a manual `workflow_dispatch` grades regardless, so a repo can trial it before switching on); switch it on with `enabled: true` or by setting the caller repo's `RISK_CONFIG` variable to `{"enabled": true}`, which outranks the input in both directions so `{"enabled": false}` is a no-PR kill switch. Grades every PR into a tier `R0` (safest) .. `R3` (riskiest) and syncs one label (`risk:R0`..`risk:R3`, or `risk:ungraded` when an input was unreadable). The label is the entire product: nothing is gated, routed, commented, or merged. Deterministic (`gh` + `jq`, no LLM): `grade = worst(path_floor, provenance, reversibility)` — path-glob map, what-process-produced-the-diff (registered runbooks with identity + diff-shape assertions; forks are R3 with no exceptions), and revertability (persistent-state mutation, deletions under sensitive classes, did green checks cover the lines). Grader + generic defaults live in [`scripts/pr-risk/`](scripts/pr-risk); a consumer sharpens them with `.github/risk.json` / `.github/risk-runbooks.json`, read from the PR's **base ref** so a PR can't edit the rules that judge it. The job excludes its own run from the check rollup and waits (`wait_for_checks_minutes`) for the rest to settle before labeling. Labels ride the plain `GITHUB_TOKEN` (cannot fire `labeled` triggers — no cascade risk); disagreement is recorded with a human-owned `risk-dispute` label. **Two further publish surfaces are available and are OFF by default**, so an enrolled caller behaves byte-identically until it opts in: `sticky_comment: true` posts ONE comment (created once, updated in place — N pushes leave one comment) carrying the per-file path-axis breakdown, the risk CONCENTRATION sentence ("94% of this diff is R0/R1; the 6% that puts the path floor at R3 is these two files, 40 lines") and a "this grade is wrong" checkbox whose state round-trips into a `risk-grade-disputed` label (distinct from the human-owned `risk-dispute`, which the grader still never touches); `check_run: true` publishes the tier and reason as a Check Run on the head commit — the immutable, timestamped, commit-attached record a mutable label cannot be — from a SEPARATE job, so it is the only surface that needs an extra `checks: write` grant in the caller's block, and only when switched on. Both surfaces are advisory in the same sense as the label: the Check Run's conclusion is hardcoded `neutral`, and every publish failure is an annotation, never a red check. Label text is remappable via `label_map`. `workflows_ref` is **required**, and its **shape is enforced** — every job that checks it out fails the run *before* the tool checkout unless the value is a full 40-hex lowercase commit SHA, so a branch, a tag or a `refs/pull/N/head` is rejected and the grader cannot be loaded from a floating ref after the caller was reviewed. **That is the whole of what is machine-checked, and it is not provenance.** Shape says the ref is immutable, never *which* commit it is: a fork of this public repo shares its object store, so a fork-authored SHA — or a pin left behind when `uses:` moved — is just as well-shaped. The test that would close that is "equal to the commit `uses:` resolved to", and the runner does not expose it to the workflow (`github.workflow_sha` is the *caller's* top-level file; `job_workflow_sha` is an OIDC claim, not a `github` context property, so reading it would need `id-token: write` from every caller). **Reviewing the caller is what bounds it, and it is the only thing that can: require `uses:` at a full commit SHA of this repo and `with: workflows_ref:` set to that same SHA written out literally, character-for-character — never an expression, never a tag.** The guard also runs *before* enablement is resolved (the resolver is itself loaded from `workflows_ref`), so a floating pin fails red even with the `RISK_CONFIG` kill switch set — the switch stops the grading, not a broken enrollment. Call the workflow directly: a nested `workflow_call` chain through an org wrapper is unsupported. Enroll it as its own workflow rather than a job inside an existing CI workflow (the rollup exclusion is per-run). The calling job needs `contents: read` + `issues: write` + `pull-requests: write` + `checks: read` + `actions: read` + `statuses: read`; GitHub rejects a shorter grant at startup (a reusable workflow can only narrow the caller's token, never elevate it), so a caller enrolled from an older copy of this row fails before any step runs. Both writes are the ONE label: repo-side label creation on first use maps to `issues`, and labeling a PR maps to `pull-requests` (the labels endpoint is dual-mapped by what the "issue" is, so `issues: write` alone 403s on a PR). `actions: read` is for the rollup's `CheckRun -> checkSuite -> workflowRun` self-exclusion hop. No secrets. | [pr-risk.md](docs/callers/pr-risk.md) |
 | [`stale.yml`](.github/workflows/stale.yml) | Stale-PR sweeper (`actions/stale`) plus a Slack digest of what it touched. PRs inactive for N days are labeled `stale`; still-inactive PRs are closed. The digest header names the source repo so batches from different repos posted to the same channel are unambiguous. Thresholds, messages, exempt labels, and the Slack channel are inputs; the caller owns the schedule + dry-run toggle. The calling job needs `pull-requests: write` and `issues: write`. Optional `SLACK_BOT_TOKEN`. | [stale.md](docs/callers/stale.md) |
 | [`groom.yml`](.github/workflows/groom.yml) | Scheduled/dispatch org-wide **code-cleanup sweep** (finds only — no commits, no PRs, never merges). A read-only FINDER agent scans a clean default-branch checkout (whole-repo, not a diff) for high-value refactors; an INDEPENDENT VERIFIER agent (fresh session) re-checks each as CONFIRM/DOWNGRADE/REJECT with a stable dedup signature; survivors are deduped against a durable GitHub-issue-state ledger and filed as `groom`-labeled GitHub issues (security-adjacent ones get `groom-security` — investigate, don't auto-implement). Mirrors the cursor-review topology: briefs + ledger live in [`.github/groom/`](.github/groom) as the single source of truth. The finder/verifier/builder agent jobs invoke the Claude CLI directly and mint no GitHub token, so they need nothing beyond `contents: read`; filing runs in a separate job as the bot you configure via `bot_app_id` (Comfy: cloud-code-bot). `dry_run` reports what it would file without opening issues. Runs on a **daily base cron** with a runtime cadence gate: set repo Actions variable `GROOM_INTERVAL_DAYS` (default 7 = weekly) to retune how often a real run happens — weekly → every-3-days → daily — with no workflow-file edit; a tick within the interval no-ops before the finder (`workflow_dispatch` bypasses the interval gate, but the volume gate — when the caller leaves it on — still applies). The calling job must grant `contents: read` + `issues: write` + `pull-requests: read` + `actions: read` — the first three are declared by the `file` / `build_select` jobs (needed even with `bot_app_id` set), and the interval gate needs `actions: read` (reads run history for the last real run); GitHub rejects a shorter grant at startup. Requires `ANTHROPIC_API_KEY` (+ `BOT_APP_PRIVATE_KEY` when `bot_app_id` is set). **Opt-in auto-builder** (`builder: true`, BE-4003): the top `max_prs` (default 5) CONFIRMED, non-security findings become **review-gated PRs** (full CI + cursor-review, **never auto-merged**) instead of issues; a credential-free `build` job emits only a patch artifact and a separate `build_pr` job opens the PR as the bot, preserving the security boundary. The ledger's PR-state (open/merged/closed) stops a built finding being re-proposed. Requires `bot_app_id`. `max_prs` is typed **`string`**, not `number`, so a caller can forward its own `workflow_dispatch` input straight through (`max_prs: ${{ github.event.inputs.max_prs \|\| '1' }}`) and let an operator raise the ceiling for one manual run — no `fromJSON()` cast in the caller, and the parse/clamp (empty → default, non-numeric → 0 PRs + warning, never a failed run) happens once inside the reusable. A build that cannot become a PR (patch over `pr_size_limit`, patch touching CI-privileged paths) **bails** to a `groom` issue so the paid-for work isn't lost — that path lives in `build_pr`, so **`max_findings` does not cap it** and `max_findings: 0` alone does not silence it; set `bail_sink: none` (an operational knob, so `GROOM_CONFIG` can set it with no PR) to file nothing and get a run-log warning + summary line instead. | [groom.md](docs/callers/groom.md) |

docs/callers/pr-size.md

@@ -10,7 +10,8 @@ keep diffs reviewable. In `mode: warn` it reports without failing.
 Excluded from the count: dependency lockfiles, `linguist-generated` files (read
 from the **base ref**, so a PR cannot exempt itself by editing
 `.gitattributes`), Go generated-code markers, and anything you add via
-`extra_lockfiles` / `extra_generated_globs`.
+`extra_lockfiles` / `extra_generated_globs`. Optionally test files too — see
+`exclude_tests` below.
 
 The counting logic and its tests live in
 [`scripts/check-pr-size/`](../../scripts/check-pr-size) and are compiled from
@@ -65,6 +66,7 @@ contents: read
 | `bypass_label` | `oversized-ok` | Waves through a legitimately large change. |
 | `extra_lockfiles` | `''` | Additional lockfiles to exclude. |
 | `extra_generated_globs` | `''` | Additional generated-path globs to exclude. |
+| `exclude_tests` | `false` | Keep test-file lines out of the count — cap production code, not coverage. Always reported separately. |
 | `comment` | `true` | Sticky bot comment explaining an overage. |
 | `bot_app_id` | `''` | Without it, degrades to status + step summary. |
 | `workflows_ref` | `main` | **Set to your `uses:` SHA** — the tool is built from this ref. |
@@ -85,6 +87,24 @@ enforce.
 alone does not mention `oversized-ok`; the sticky comment is what tells an author
 the escape hatch exists. Supply the App or expect confused authors.
 
+**`exclude_tests` is a naming convention, not a proof.** Unlike the
+generated-file rules — which require Go's marker *before* the package clause,
+and read `.gitattributes` from the base ref precisely so a PR cannot exempt
+itself — test detection only looks at the path. Nothing stops production code
+being parked in `tests/` to duck the cap. That is why it is off by default, and
+why the excluded total is always printed on its own line: the report shows
+`Excluded (tests): N` next to the counted number, so a 5,000-line "test-only" PR
+is visible rather than silently small. Recognized: `*_test.go`; `test_*.py`,
+`*_test.py`, `conftest.py`; `*.test.*` / `*.spec.*` for `.js .jsx .mjs .cjs .ts
+.tsx .mts .cts`; and any file under a `test/`, `tests/`, `testing/`,
+`testdata/`, `e2e/`, `__tests__/`, `__mocks__/` or `__snapshots__/` **directory**
+segment. `spec/` is deliberately *not* a test directory — in this org it holds
+OpenAPI schemas, which are production artifacts. For a layout these miss, add
+`extra_generated_globs` (they land in the generated bucket instead).
+
+Leaving it off is a real choice, not just the safe one: a 5,000-line test diff
+is genuinely slow to review, and the cap is the only thing that says so.
+
 **Go workspaces:** a consumer with a root `go.work` needs `GOWORK=off` for the
 tool build, since `go build` otherwise discovers the consumer's workspace.