Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions .github/workflows/file-size.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,19 @@ jobs:
name: file size ratchet
runs-on: ubuntu-latest
steps:
# Full history is not needed — the guard reads the working tree and the
# committed baseline, never a diff against the base.
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
with:
# Two, not one. The guard judges THE CHANGE, not the tree (#2004):
# on a `pull_request` event this checks out `refs/pull/N/merge`, and
# the ratchet reads that merge commit's first parent — the base
# branch tip — to ask whether a file went over its ceiling HERE or
# arrived that way. At the default depth of 1 the parents are not
# in the clone, the base cannot resolve, and the guard falls back to
# the strict whole-tree check: green on this PR, but blind to the
# difference this workflow exists to draw. It compares two TREES,
# not two histories, so one extra commit is the whole cost — the
# same trade ci.yml's guards job makes for check-deleted-tests.sh.
fetch-depth: 2

# The guard's own tests first: they are hermetic and they are what proves
# it can still FAIL. A ratchet that has quietly become incapable of
Expand Down
16 changes: 16 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,22 @@ inputs, not review afterthoughts:
an escape hatch for a genuinely irreducible line (a module declaration in
an already-oversized `lib.rs`), never something a plan may assume.

**The ratchet judges your change, not the tree** (#2004). A file over its
ceiling fails only when `current > max(ceiling, size in the base tree)` — so a
violation that was already there before your branch existed is reported as
drift and does not fail you, while anything your change grows past what it
inherited fails exactly as it always did. The base is the merge commit's first
parent on a pull request, the merge base locally, and nothing at all in a
shallow clone or a fresh repository — where the guard falls back to the strict
whole-tree check, because an unresolvable base must make it stricter, never
weaker. This exists because the baseline is one shared cell every growing PR
must write, and three times running two PRs that each wrote it correctly
composed into a red `main` that then failed everyone downstream. Two
consequences for planning: **do not "fix" a red ratchet you did not cause** by
folding a baseline regeneration into an unrelated PR — land it on its own from
a fresh `main`; and note that splitting a god file buys structure, not slack,
since `--update` retightens every ceiling to its file's current size.

The workspace's Rust god files, by crate (the bench harness's Python offenders
sit in the same baseline). Each file's ceiling lives in
`scripts/file-size-baseline.txt` and is deliberately not repeated here: that
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ automerge-nudge-test: ## Test which PR the auto-merge nudge picks (hermetic; not
./scripts/test-automerge-nudge.sh

.PHONY: file-size-test
file-size-test: ## Test which languages the file-size ratchet actually watches (hermetic; not part of `gate`)
file-size-test: ## Test the file-size ratchet's language coverage and its change-relative judgement (hermetic; not part of `gate`)
./scripts/test-file-size.sh

.PHONY: guard-sigpipe-test
Expand Down
191 changes: 187 additions & 4 deletions scripts/check-file-size.sh
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,47 @@
# Test files count. A 2,750-line test module is as hard to navigate as any other,
# and exempting tests would be an obvious loophole.
#
# ── What the ratchet judges: the CHANGE, not the tree (#2004) ─────────────────
#
# The baseline is a single shared cell that every growing PR must write, and for
# three occurrences running (#1761, #1782, #2003) two PRs that each wrote it
# CORRECTLY composed into a red `main`. Each regenerates the whole baseline
# against a snapshot of `main` that does not yet carry the other's growth, so
# each records a stale ceiling for a file it never touched. The merge is
# textually clean — the two sides edit different lines — and the result is a
# ceiling one line below an actual. `main` then stays red, and every subsequent
# PR inherits a failure it did not cause; #1992 sat blocked on exactly this with
# a tree byte-identical to `main`.
#
# Note what did NOT happen in that composition: the file never grew. Its ceiling
# moved DOWN underneath it. A guard asking "is this tree consistent with this
# baseline snapshot?" cannot tell those apart, because it has only one tree to
# look at. So it asks the per-change question instead, against the base:
#
# fail only when current > max(ceiling, size at base)
#
# The `max` is the whole rule, and each half earns its place:
#
# - `ceiling` alone is today's check, and fails the innocent PR above.
# - `size at base` alone would pass a file that is already over its ceiling
# and that THIS change grows further — drift would become a licence to
# bloat, which is the one outcome worse than the red main.
#
# Taking the larger fails a change that genuinely grows a god file past what it
# inherited, and is silent when the violation arrived from somewhere else. A
# ceiling raised deliberately via `--update` still passes exactly as before:
# the regenerated ceiling equals the current size.
#
# The base is the same pair `scripts/check-deleted-tests.sh` uses, for the same
# reason: on a `pull_request` event the checkout is `refs/pull/N/merge`, so HEAD
# is the merge commit and HEAD^1 is the base branch tip. That asks "does this
# MERGE grow a god file?", which is the question a required check should answer,
# and it needs `fetch-depth: 2` — which ci.yml already sets.
#
# When no base can be resolved the guard falls back to judging the tree, exactly
# as it did before. That direction is deliberate: an unresolvable base must
# never make the ratchet weaker, only stricter.
#
# Uses portable POSIX tools so it runs on a bare CI runner (macOS ships bash 3.2,
# so no associative arrays).
set -euo pipefail
Expand Down Expand Up @@ -122,9 +163,70 @@ if [ ! -f "$baseline" ]; then
exit 1
fi

# The commit this change is measured against, or empty for the strict
# whole-tree check. Resolved once; see the header for why the pair matters.
#
# The order is by how precisely each candidate answers "what did this change
# inherit", and every rung is verified to exist before it is taken, so a
# shallow clone or a fresh repository falls through to strict rather than
# erroring.
#
# Every rung tolerates its own failure explicitly rather than leaning on the
# errexit suppression that `|| true` at the call site would grant: this function
# is the one place where "could not resolve" must stay an ordinary answer, and a
# reader should not have to know that rule to see it.
resolve_base_commit() {
local candidate mb
# An explicit override wins, for hand runs and for the hermetic tests in
# scripts/test-file-size.sh, which have no origin to infer one from.
if [ -n "${FILE_SIZE_BASE_REF:-}" ]; then
candidate="$(git rev-parse --verify --quiet "${FILE_SIZE_BASE_REF}^{commit}" 2>/dev/null || true)"
printf '%s' "$candidate"
return 0
fi
# A merge commit means a `refs/pull/N/merge` checkout: HEAD^1 is the base
# branch tip and HEAD^2 the PR head. This is the CI path on a pull request,
# and the same pair check-deleted-tests.sh is built on.
if git rev-parse --verify --quiet "HEAD^2" >/dev/null 2>&1; then
candidate="$(git rev-parse --verify --quiet "HEAD^1^{commit}" 2>/dev/null || true)"
printf '%s' "$candidate"
return 0
fi
# A local feature branch: judge every commit on it at once, not just the
# last. Skipped when the merge base IS HEAD — that means HEAD carries no
# change of its own relative to `main`, so any violation is inherited drift
# and the strict read ("main owes a regeneration") is the honest one.
mb="$(git merge-base HEAD origin/main 2>/dev/null || true)"
if [ -n "$mb" ] && [ "$mb" != "$(git rev-parse HEAD 2>/dev/null || true)" ]; then
printf '%s' "$mb"
return 0
fi
# A linear commit with no merge and no origin/main ahead of it — a push to
# `main` in CI, where the previous commit is exactly what it inherited.
candidate="$(git rev-parse --verify --quiet "HEAD^1^{commit}" 2>/dev/null || true)"
printf '%s' "$candidate"
return 0
}
base_commit="$(resolve_base_commit)"

# Line count of $1 in the base tree, or 0 when the path did not exist there.
# Zero is the fail-closed answer: it makes `max(ceiling, base)` collapse to the
# ceiling, i.e. the strict check.
size_at_base() {
local n
n="$(git show "$base_commit:$1" 2>/dev/null | wc -l | tr -d ' ')" || n=""
[ -n "$n" ] || n=0
printf '%s\n' "$n"
}

# Single awk pass: read the baseline into a map, then judge each current file.
# Baseline lines are tagged B, current sizes C, so one awk sees both streams.
report="$(
#
# A file over its ceiling is emitted as a CANDIDATE, not a verdict: awk sees one
# tree and so cannot tell growth from inherited drift. The classification needs
# the base tree and happens below. Candidates are emitted in GREW's position so
# the assembled report keeps its original section order.
raw_report="$(
{
grep -v '^#' "$baseline" | sed 's/^/B /'
current_sizes | sed 's/^/C /'
Expand All @@ -136,7 +238,7 @@ report="$(
if (n <= limit)
obsolete = obsolete sprintf(" %s is now %d lines (<= %d) — drop its baseline entry\n", path, n, limit)
else if (n > ceiling[path])
grew = grew sprintf(" %s grew to %d lines, over its baseline ceiling of %d (+%d)\n", path, n, ceiling[path], n - ceiling[path])
grew = grew sprintf("GREWCAND %s %d %d\n", path, n, ceiling[path])
} else if (n > limit) {
newover = newover sprintf(" %s is %d lines, over the %d-line limit\n", path, n, limit)
}
Expand All @@ -146,13 +248,77 @@ report="$(
if (!(p in seen))
stale = stale sprintf(" %s (baseline entry, file no longer tracked)\n", p)
if (newover) printf "NEWOVER\n%s", newover
if (grew) printf "GREW\n%s", grew
if (grew) printf "%s", grew
if (obsolete) printf "OBSOLETE\n%s", obsolete
if (stale) printf "STALE\n%s", stale
}
'
)"

# Classify each candidate against the base tree, preserving the stream order so
# the GREW section still lands between NEWOVER and OBSOLETE.
#
# `current > max(ceiling, size at base)` — see the header. With no base commit
# the second term is absent and this is the original whole-tree check.
report=""
drift=""
grew_header_emitted=0
while IFS= read -r line; do
case "$line" in
"GREWCAND "*)
# Deliberate word split: these are this script's own awk-formatted records,
# and the baseline format has never admitted a path containing a space.
# shellcheck disable=SC2086
set -- $line
cand_path="$2"
cand_now="$3"
cand_ceiling="$4"
cand_effective="$cand_ceiling"
if [ -n "$base_commit" ]; then
cand_base="$(size_at_base "$cand_path")"
if [ "$cand_base" -gt "$cand_effective" ]; then
cand_effective="$cand_base"
fi
fi
if [ "$cand_now" -gt "$cand_effective" ]; then
if [ "$grew_header_emitted" -eq 0 ]; then
report="${report}GREW
"
grew_header_emitted=1
fi
report="$report$(printf ' %s grew to %d lines, over its baseline ceiling of %d (+%d)' \
"$cand_path" "$cand_now" "$cand_ceiling" "$((cand_now - cand_ceiling))")
"
else
drift="$drift$(printf ' %s is %d lines against a ceiling of %d — already so at the base' \
"$cand_path" "$cand_now" "$cand_ceiling")
"
fi
;;
"") ;;
*)
report="${report}${line}
" ;;
esac
done <<EOF
$raw_report
EOF

# Drift is reported whichever way the verdict goes: it is real — the baseline
# owes a regeneration — but it is not THIS change's debt, and failing the next
# PR to walk past it is the bug this guard was rewritten to stop (#2004).
if [ -n "$drift" ]; then
{
echo "check-file-size: baseline drift (not caused by this change, not fatal)"
printf '%s' "$drift"
echo ""
echo "These files were already over their recorded ceiling in the base tree,"
echo "so another change put them there and this one only inherited it. Run"
echo "\"make file-size-update\" on a fresh main and land the baseline diff on"
echo "its own to clear it."
} >&2
fi

if [ -n "$report" ]; then
echo "check-file-size: FAILED" >&2
echo "$report" | awk '
Expand All @@ -177,5 +343,22 @@ tracked=$(git ls-files "${RATCHET_PATHSPECS[@]}" | wc -l | tr -d ' ')
# `scripts/test-file-size.sh` (which plants an empty baseline by design) was
# red (#1800).
grandfathered=$(grep -cv '^#' "$baseline" || true)
# A drifted baseline still passes, so the green line must say so rather than
# read as an unqualified clean bill — the summary is what most readers see.
drift_note=""
if [ -n "$drift" ]; then
drift_note=" $(printf '%s' "$drift" | grep -c '^') file(s) carry inherited drift (see above)."
fi
# Say which mode ran. The change-relative rule is silent by nature — it only
# shows itself when something drifts — so a checkout too shallow to resolve a
# base would fall back to strict and read as an ordinary green line forever.
# That is exactly how .github/workflows/file-size.yml shipped its ratchet as a
# whole-tree check while this script believed otherwise, and naming the mode is
# what makes the difference legible in a log rather than a thing to re-derive.
if [ -n "$base_commit" ]; then
mode_note=" Judged against $(git rev-parse --short "$base_commit" 2>/dev/null || echo "$base_commit")."
else
mode_note=" No base resolved — strict whole-tree check."
fi
trap '' PIPE
echo "check-file-size: OK — $tracked Rust/Python/shell files, none over $LIMIT lines except $grandfathered grandfathered (none grew)." || true
echo "check-file-size: OK — $tracked Rust/Python/shell files, none over $LIMIT lines except $grandfathered grandfathered (none grew by this change).${mode_note}${drift_note}" || true
Loading