Require an adversarial review by a subagent before committing - #61
Open
GenericJam wants to merge 2 commits into
Open
Require an adversarial review by a subagent before committing#61GenericJam wants to merge 2 commits into
GenericJam wants to merge 2 commits into
Conversation
Codifies the process gap this session exposed. Reviews run before commit caught, each of which would otherwise have shipped: a helper defined inside `#if !MOB_RELEASE` but called unconditionally from Swift, which linked in debug and would have failed every iOS release build; a cache whose tests asserted the write path and nothing about the read, so deleting the lookup passed the whole suite; a fix covering 3 of 7 call sites while claiming cross-platform parity; a comment and a decision record asserting a race was closed when the code only narrowed it; and generated source telling every user a feature does nothing, in the release that made it work. The one substantial change that skipped review was the largest in the batch, which is the argument for not letting size be the reason to skip. The reviewer must be a separate agent. What is usually wrong is the author's mental model of the change, and a self-review carries that model into the second pass. The rule also says what to hand the reviewer, including the explicit diff base — a diverged local branch otherwise sweeps the whole tree into the diff, which happened three separate times this session. Scoped: skipped for mechanical changes (formatting, typos, version bumps, changelog edits, file moves), reached for when a change has behaviour, touches native code, or spans a platform boundary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The checklist covered tests, formatting and linting but never prompted the question "does this decision belong in decisions/". More importantly it never prompted the reverse question, which is the one that actually bit. Both halves failed in one session, on the same change: - A decision record claimed "the frame-registry generation is untouched because the parked slot stops re-registering once it stops laying out". It reasoned about the outgoing direction only. The returning direction was broken — silently, for exactly the screens the change optimised for — and the record asserted it was fine. A record making a claim the code does not support is worse than no record, because a maintainer will act on it. - Source comments elsewhere stated invariants the same change inverted: MobLazyList's latch reasoned that "only navigation changes the container's identity", which had just stopped being true. So the step asks both: does this need a new record, and does it invalidate one that already exists. Grep decisions/ for the mechanism you are changing. Also says to correct a wrong record in place with a note rather than deleting the claim. The wrong version is the part a future reader needs in order to recognise the mistake; append-only applies to superseding whole decisions, not to silently editing away an error inside one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Codifies the process gap this session exposed.
Pre-commit reviews caught, each of which would otherwise have shipped:
#if !MOB_RELEASEbut called unconditionally from Swift — linked in debug, would have failed every iOS release buildThe one substantial change that skipped review was the largest in the batch. That is the argument for not letting size be the reason to skip.
What the rule says
The reviewer must be a separate agent. What is usually wrong is the author's mental model of the change, and a self-review carries that model into the second pass.
It also specifies what to hand the reviewer — including the explicit diff base, because a diverged local branch otherwise sweeps the whole tree into the diff. That happened three separate times this session, to three different people.
Scoped deliberately: skipped for mechanical changes (formatting, typos, version bumps, changelog edits, file moves); reached for when a change has behaviour, touches native code, or spans a platform boundary.
RELEASE.mdgains one paragraph distinguishing this from the existing release-review gate — that one asks whether the accumulated diff is coherent and safe to ship, this one asks whether a change is correct. Neither substitutes for the other.Docs only. No
mix.exschange, so no release is triggered.