Skip to content

finding — a PR against a ratcheted file can be textually clean and still blow the budget: PR CI, mergeable_state and git merge-tree are all textual, and none of them reads the merged result #16287

Description

@hotlong

Recording only — unassigned, no claim. Found the hard way on PR #15427 today, and generalised by that PR's own seat.

What happened, with the readings

PR #15427 added one line to AGENTS.md. At the moment it was written the file was 1161 lines against a ceiling of 1162 — headroom 1, and the line fitted exactly.

While it waited for human review, the #15379 rules-only programme compressed the same file to 1058 and, by its own shrink-only discipline, re-locked the ceiling to 1058. That is correct behaviour for a shrink-only ratchet. It also silently took back the single line of headroom the open PR had been paid from.

The PR was then approved and enqueued. It would have been dropped by the merge queue. Measured before that happened:

git merge-tree --write-tree origin/main <head> | head -1   →  TREE
git cat-file -p "$TREE:AGENTS.md" | wc -l                  →  1059
ceiling (check-skill-line-ratchet.mjs)                     →  1058

The PR's seat reproduced the red verbatim on the merged tree: check-skill-line-ratchet: AGENTS.md is 1059 lines; the ratchet ceiling is 1058.

⚠️ Why nothing caught it — all three signals are textual by construction

signal said why it could not see this
the PR's own CI green ran days earlier, against the old file and the old ceiling
mergeable_state clean answers "do the texts conflict", nothing else
git merge-tree no conflict same question, same blind spot

All three read clean, and the merged tree violated a gate. There is no textual conflict here at all: one side deletes lines elsewhere in the file, the other adds one, and git composes them perfectly. What broke is a budget, and no textual check has a concept of one.

⛔ Note this is not the merge queue failing — the queue is the thing that would have caught it, which is exactly why it exists. The cost of finding out there is a full CI cycle plus a drop whose reason is only visible in the queue's own logs.

The class, stated so it is recognisable

Any PR that touches a ratcheted file can be textually clean and still over-budget, whenever a sibling PR compresses that file and re-anchors its ceiling in between. The longer a PR waits for review, the likelier it is: #15427 sat two days, and AGENTS.md moved 103 lines under it.

The same shape applies to every other budget in this repo that is pinned rather than derived — the second map in the same script (max table-row bytes, which also moved, 1081 → 768), check:skills-token-ratchet, check:type-check-debt, and the source-token ratchets in the app repos.

The mechanical answer

A merged-result probe: for each open PR touching a ratcheted path, materialise the merge with main and run the ratchet against that tree rather than against either side.

TREE=$(git merge-tree --write-tree origin/main "$HEAD" | head -1)
git cat-file -p "$TREE:$FILE" | wc -l          # against the ceiling on main
git cat-file -p "$TREE:$FILE" | LC_ALL=C awk '{print length}' | sort -rn | head -1

⚠️ LC_ALL=C matters — the second pin is in bytes, and this corpus is partly Chinese, so a character count silently under-reports by ~3x on those lines.

Two placements worth weighing (⛔ not adjudicated here):

  1. In the ratchet gate itself, as a mode that takes a base ref — so the PR-time run measures the merged result rather than the branch. Catches it on the PR, before enqueue.
  2. In the compressing PR, as an outbound check: when a PR lowers a ceiling, it enumerates the open PRs touching that file and reports which would now exceed it. Catches it at the moment the headroom is taken, which is earlier and names the right owner — the PR that removed the budget, not the one that had already paid from it.

Direction 2 is the one that fits this repo's usual instinct (the change that breaks something should be the change that reports it), but it needs a live GitHub read at gate time, which several gates here deliberately avoid. Worth costing before choosing.

Related, same day, same underlying shape

All four are the same sentence: a signal that reads clean because it was never looking at the thing that broke.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions