Skip to content

Close the gaps that kept getting missed: merge gate, instruments, tests - #71

Merged
GenericJam merged 1 commit into
masterfrom
docs/tighten-review-process
Sep 6, 2026
Merged

Close the gaps that kept getting missed: merge gate, instruments, tests#71
GenericJam merged 1 commit into
masterfrom
docs/tighten-review-process

Conversation

@GenericJam

Copy link
Copy Markdown
Owner

An audit of the three repos against the process we actually follow. Four things were undocumented, each of which cost time this week.

Guidance (judgement — can't be hooked)

Nothing described reviewing before a merge. The adversarial gate covers the commit; there were zero references to a second pass anywhere in six files. That pass has caught more: in one session it found a PR shipping its headline fix untested (deleting the clause left all 1545 tests green) and blocked another over per-widget state that navigation had silently stopped resetting. Neither was visible in the diff alone.

The section also lists the mechanical preconditions to check yourself: CI green and newer than the last commit (one PR carried a month-old green run from 40 commits ago), branch not behind master, cross-repo version claims true now rather than eventually, and stacked PRs merged base-first.

Nothing said to distrust a measurement. Every rung of the fidelity ladder assumes the instrument is honest; when it isn't, the failure looks like a result rather than an error. Two from one session: a benchmark reporting a 6.5x win that was an artifact of timing a frame still showing the old tree, and an on-device check printing PASS against a build that had failed to compile. Both were believed for a while. The rule that catches both: a number better than the theory allows is a bug in the measurement.

Linear was documented in mob only, though it's the board for all three — so an agent working in mob_dev or mob_new had no pointer to it.

The test mandate was an escape hatch ("add coverage or note it as a follow-up") and only in mob. It now states the bar that matters: would this test fail if the fix were reverted? Check by reverting.

Mechanical (hooks — facts, not opinions)

.githooks/pre-push gains an advisory tier that never blocks:

  • how far behind origin/master this branch is, with a louder note past 100 commits
  • whether code changed with no test changing, listing the files

Neither is safely blockable — a worktree legitimately lags, and plenty of real changes need no test. A check that blocks on a judgement call is one people learn to bypass, which costs more than it catches. These print, loudly, and you decide.

The staleness notice exists because a checkout ~370 commits behind produced a review finding that a function "did not exist" when it did, and cost roughly forty minutes debugging code that was never running.

Evidence

Both advisory functions were exercised against real history: silent on a range where tests moved with code, and firing correctly on a code-only commit (996e511, lib/mob/test.ex with no test change). bash -n clean in all three repos; the hook is byte-identical across them, as it was before.

Docs and hook only — no code paths touched.

🤖 Generated with Claude Code

An audit of the three repos against the process we actually follow found four
things undocumented, each of which cost time this week.
**Nothing described reviewing before a MERGE.** The adversarial gate covers the
commit; zero references existed to the second pass. That pass has caught more:
in one session it found a PR shipping its headline fix untested — deleting the
clause left all 1545 tests green — and blocked another over per-widget state
that navigation had silently stopped resetting. Neither was visible in the diff
alone.
**Nothing said to distrust a measurement.** Every rung of the fidelity ladder
assumes the instrument is honest, and when it is not the failure looks like a
result rather than an error. A benchmark reported a 6.5x win that was an
artifact of timing a frame which still showed the old tree; an on-device check
printed PASS against a build that had failed to compile. Both were believed for
a while. The rule that would have caught both: a number better than the theory
allows is a bug in the measurement.
**Linear was documented in mob only**, though it is the board for all three, so
an agent working in mob_dev or mob_new had no pointer to it.
**The test mandate was an escape hatch** — "add coverage or note it as a
follow-up" — and only in mob. It now states the bar that matters: would this
test fail if the fix were reverted? Check by reverting.
The mechanical half goes in .githooks/pre-push as an advisory tier that never
blocks: how far behind origin/master this branch is, and whether code changed
with no test changing. Neither is safely blockable — a worktree legitimately
lags and plenty of real changes need no test — and a check that blocks on a
judgement call is one people learn to bypass. They print, loudly, and you
decide. The staleness notice exists because a checkout ~370 commits behind
produced a review finding that a function "did not exist" when it did.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@GenericJam
GenericJam merged commit d107630 into masterSep 6, 2026
3 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@GenericJam