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

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,100 @@ 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, and lists the largest excluded
+files — the biggest contributors, not a complete accounting (it stops at 10), but
+enough to sanity-check that the exclusion is really tests. When the
+exclusion is the *only* reason a PR is under the cap, the sticky comment posts
+even though the check is green — otherwise the number would live solely in the
+Actions step summary in precisely the case that matters, and a 5,000-line
+"test-only" PR really would pass unremarked.
+
+**That green-check comment needs the bot App.** It requires `comment: true`
+(the default) *and* `bot_app_id` + `BOT_APP_PRIVATE_KEY` — all optional. Opt into
+`exclude_tests` without the App and you lose the sticky comment — the only
+surface that puts the number in front of a reviewer unprompted. The check
+annotation described below still fires (it needs no credentials), so the totals
+remain reachable via the Checks tab and the run's Details link, but reaching them
+is a deliberate click. **If you set `exclude_tests`, configure the App too** —
+otherwise you keep the loosening and keep only the weaker half of the
+visibility.
+
+**Fork and Dependabot PRs are a blind spot — weigh this before opting in.** Those
+runs never receive `BOT_APP_PRIVATE_KEY` (GitHub withholds secrets from them), so
+the green-check comment cannot post there *even for a caller that configured the
+App correctly*. The consequence is uncomfortable and worth stating bluntly: with
+`exclude_tests: true`, a fork PR gets the **weaker cap** and loses the mechanism
+that makes the weaker cap safe. The trust gradient inverts — the least-trusted
+contributions get the least-scrutinized guardrail — and it is silent, because a
+green check with no comment looks exactly like a PR that passed on its own
+merits.
+
+**This is why a decisive exclusion also emits a check annotation.** It comes from
+the size job itself and needs no credentials, so it reaches fork and Dependabot
+PRs, carrying the same numbers as the comment (`counted + test` against the
+applied cap).
+
+Be clear about what it does *not* buy, though. An annotation with no file/line
+renders on the run-summary page, behind the check's **Details** link, and in the
+**Checks** tab — but **not** in the PR conversation (a passing check collapses to
+"All checks have passed") and **not** inline on Files changed. So on a fork PR the
+number is recorded and reachable, but still a deliberate click away. Only the
+sticky comment puts it in front of a reviewer unprompted. Treat the annotation as
+a backstop that makes the number *findable*, not as parity with the comment.
+
+What you still lose without the App on a fork PR is the *sticky* comment: the
+in-conversation explanation that survives pushes and flips to ✅. If that matters
+for your outside contributions, leave `exclude_tests` off.
+
+Note also that `extra_generated_globs` (below) classifies matches as
+*generated*, not *test*: they never reach the excluded-test total and never
+trigger the green-check comment. A repo leaning on it for an unusual test layout
+opts out of this visibility guarantee.
+
+**Recognized as test files.** File names: `*_test.go`; `test_*.py`, `*_test.py`,
+`conftest.py`; and for `.js .jsx .mjs .cjs .ts .tsx .mts .cts`, a `test`
+component anywhere in the name after the first (`Button.test.tsx`, and the
+type-test form `api.test.d.ts`) or a `spec` component immediately before the
+extension (`api.spec.ts`) — `spec` is narrower on purpose, because
+`api.spec.types.ts` and `openapi.spec.client.ts` are OpenAPI *production* files
+here and excluding them would under-count.
+
+**Directories are matched in three cases, not one**, because the names are not
+equally trustworthy:
+
+| Case | Segments | Where they match |
+|---|---|---|
+| 1 | `__tests__/`, `__mocks__/`, `__snapshots__/`, `testdata/` | **any depth** — nothing else is ever called these, and Go nests `testdata` by design |
+| 2 | `test/`, `tests/`, `testing/`, `e2e/` | **repo root only** |
+| 3 | the same four, plus `it/` | **under a `src/` directory at any depth** — `module-a/src/test/java` works too. `it/` additionally requires a child segment, so `src/it/java/FooIT.java` is excluded but `src/it/messages.properties` **counts** (`it` is also the ISO-639-1 code for Italian) |
+
+The root restriction in case 2 is not fussiness, it is a bug fix. A consumer
+keeps production deployment manifests — cluster RBAC and ingress config — under
+`deploy/envs/testing/`, where `testing` names the deployment
+*environment*, not test code. Matching that name at any depth silently excluded
+**cluster RBAC changes** from the cap. Root-anchoring keeps that repo's whole
+root-level `testing/` tree excluded while counting the deployment files.
+
+The cost is deliberate and worth knowing: a **nested** ambiguous directory such as
+`services/checkout/e2e/` now counts. That is the safe direction — over-counting
+starts an argument, under-counting silently shrinks the number the cap protects —
+but if your tests live somewhere the three cases miss, they will be counted.
+
+Segment matching is case-insensitive over ASCII (so `Tests/` and `TestData/`
+work); the file-name rules stay case-sensitive, because their toolchains define
+them in lowercase. `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.