# Red Team Your Own Diff

> A revision is written, the tests are green, and the temptation is to push. Attack it first as the specific reviewers would — from their recorded rules, not from generic good practice — and resolve each objection with a measurement rather than an argument. Their standing asks are enumerable from what they have written on other PRs: never force-push, a test must hit the changed line, no unrelated changes, comment every test, minimise the diff for cherry-picking. Attack the packaging too: the title after a widened scope, comment symmetry, the shape of the helper, how the commits split. Use before pushing a revision, before replying to a review, and whenever a change grew past what the reviewer asked for. Trigger terms: ready to push, address the review, before I send this, self-review, is this mergeable, did I miss anything.

- Skill: `serhiy-bzhezytskyy/red-team-your-own-diff` (Agent Skill)
- Install (CLI): `npx skillmds@latest add serhiy-bzhezytskyy/red-team-your-own-diff`
- Raw SKILL.md: https://api.skillmd.com/api/skills/serhiy-bzhezytskyy/red-team-your-own-diff/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: serhiy-bzhezytskyy (https://skillmd.com/u/serhiy-bzhezytskyy)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/serhiy-bzhezytskyy/red-team-your-own-diff

---


# Red-team your own diff

## Purpose

A green suite says the change works. It says nothing about whether the change will survive the
people who have to read it, and those people are not generic: each has rules they have enforced
in writing, on other contributors' PRs, repeatedly. Attacking your own diff against *those* rules
finds a different class of defect from testing — the guard that hides a failure someone wanted
exposed, the title that stopped describing the change, the word the project does not use, the
helper twice as long as the invariant requires.

Doing it before pushing also converts what would have been review round-trips into a diff that
answers them in advance. In a house where an outside PR waits a month for attention, a round-trip
saved is worth more than a defect found.

## When to use

- A revision answering a review is written and about to be pushed.
- The change grew beyond what the reviewer asked for — the widened scope is what breaks titles,
  invalidates earlier measurements, and creates unrelated-change objections.
- You added a guard, a helper, or a default inside something with many callers.
- Before replying to a review at all: the reply's claims are part of the diff's surface.

## When NOT to use

- A one-line change nobody has reviewed yet, in a house with no recorded reviewer rules. Push and
  learn what they say.
- You want the *automated* reviewer's rulebook rather than a named human's — that is
  [run-the-bots-review-before-it-does](../run-the-bots-review-before-it-does/SKILL.md); its findings
  are public on the repo's other PRs and knowable in advance.
- The doubt is about inputs you did not imagine rather than rules you did not follow — then the
  cheaper adversary is
  [fuzz-before-you-claim-done](../fuzz-before-you-claim-done/SKILL.md).
- The reviewer has already approved and asked for nothing. Re-litigating spends the attention this
  is meant to protect.
- You have no profile material: gather the quotes first (see `../../PRINCIPLES.md` on receipts) —
  attacking from *generic* best practice produces generic objections and misses the house's own.

## The practice (checklist)

**Attack the code.**
- [ ] ⛔ **Does the change hide a failure someone wants exposed?** If you added a guard inside a shared
      helper, enumerate **every** caller and check none of them expects the guarded condition to fail.
      Sampling is not enough — check all of them, then say how many you checked.
- [ ] ⛔ **Re-measure any inertness claim after widening.** "This branch never executes in normal runs"
      is scoped to the code it was measured on. Moving the guard into a helper with N callers voids it.
      Re-probe and count the calls and the fires.
- [ ] **Diff what you deleted, not only what you added** — `git diff | grep -E '^-.*(assert|wait|&&)'`.
      A rewrite quietly relaxing a two-sided check into a one-sided one is invisible in the added lines.
- [ ] **Read the source behind your assumptions.** Any belief about a reply's shape, a coalescing rule,
      or an ordering guarantee is checkable. One such read here halved a helper: the loop was walking
      every element because I had not verified that the source already merges them.

**Attack the packaging — this is where the real defect usually is.**
- [ ] ⭐⭐ **Does the title still describe the change?** After a scope increase, almost never. And if the
      house squash-merges, the PR title becomes the permanent commit subject.
- [ ] ⭐ **Vocabulary**: grep the files you touched for every noun in your title and body. Words with
      **zero** occurrences in the project are imported and a reviewer will notice.
- [ ] ⭐ **Comment symmetry**: count comment lines against call sites. One site carrying four lines of
      explanation while five carry none is an inconsistency; and an explanation that narrates the bug
      you found is development process, not delivered state. The *why* belongs once, in the helper.
- [ ] **Style parity, measured not assumed**: timeout constants, multi-line message shapes, naming
      convention — count what the file already does and match the majority.
- [ ] **Commit split**: split along *what the reviewer asked for* versus *what you added on your own*,
      and only if the house's own revisions do that. Measure it —
      `gh pr view N --json commits --jq '[.commits[].messageHeadline]'` on merged PRs that had review
      rounds. ⛔ And never `--amend`/force-push once a reviewer is on the thread.

**Then resolve, not argue.** Each objection ends in one of three states, and say which: refuted by a
measurement; a live style objection you are choosing to keep, with the reason; or a real defect you
fixed. ⚠️ Never "considered and dismissed".

## Rationalizations

| Shortcut | Why it fails |
|---|---|
| "The tests pass, so it's ready." | The tests cover behaviour. They say nothing about a title that lags the diff, a word the project never uses, or a guard that suppresses a failure a test exists to catch. |
| "A reviewer will tell me if something's off." | That costs a round-trip. In a repo where 591 open PRs are older than thirty days, the round-trip is the expensive part, not the fix. |
| "I already reviewed it carefully." | Careful reading found none of the three defects here. Enumerating a *named person's recorded rules* found them, because the rules point at places care does not look. |
| "It's the same change, just applied more widely." | Wider is exactly what invalidates prior measurements and titles. The inertness claim and the PR title both broke on widening, and nothing else changed. |
| "The helper is fine, it works." | It worked at twice the length and O(range) instead of O(entries), because I had not read the invariant that made the short form correct. Working is not the bar for code a reviewer must hold in their head. |
| "Splitting commits is cleaner." | Measure the house. Ours squash-merges, and its own revision commits are often one, with subjects as short as `comment`. My instinct said two; the measurement said one. |

## RECEIPT

`redis/redis#15636`, 2026-08-13. A core reviewer asked for a helper; the revision that answered him
also extended the fix to eight more tests, so it touched a proc with **33 call sites**. Before
pushing, it was attacked against three reviewers' recorded rules — harvested from ~1,600 of their
inline comments — producing **eleven vectors**:

**Nine refuted by measurement, not by argument:**
- *"the guard masks a failure a caller wants"* (his own rule, stated on another PR: *"it feels like
  we are making the test pass without actually testing what we want"*) → all **9** call sites with
  error-expecting context assert **after** a successful setup; none expects the setup to fail.
- *"the guard is not inert in normal runs"* → probed: **47 calls, 0 fires**, assertions unchanged.
  ⚠️ This claim had been measured on the one-call-site version and carried over illegitimately; the
  attack is what forced the re-measure.
- *"it relaxes a two-sided wait to one-sided"* → the file uses both forms, 3 sites each, **and** a
  probe showed no task or trim in flight after the helper returns.
- timeout parity (`1000 10` is the file's idiom, **36** occurrences vs 7), reply-shape assumption
  (verified in the C source, not remembered), hard-coded range (all 5 sites migrate exactly that
  range), performance (349 s vs a ~350 s baseline), stale-view risk, and single-caller predicate.

**One kept as a style objection, with the reason stated** rather than silently resolved.

**One real defect, and it was in the packaging:** ⭐⭐ the title still said *"…before ASM
**error-handling tests** import them"* while the change now touched a shared helper and ten tests.
The house squash-merges, so that title would have become the permanent commit subject. Its top
reviewer asks for exactly this in 28 separate comments — *"pls update the PR title and top comment"*.

**Three changes the attack produced before anything was pushed:**
1. the title, rewritten to cover the real scope, using the house's own `Tests:` prefix;
2. a four-line comment cut — six call sites, only one carried an explanation, and that explanation
   retold the bug rather than describing the delivered state;
3. the ownership check reduced from **17 lines to 9** and from O(slots) to O(entries), after reading
   the C that guarantees contiguous entries are already merged. Total diff `+47/-22` → `+34/-21`.

⇒ And the attack's discipline caught one more thing on its way out: a leftover sentence reading
*"the 9th test fails with a different error I haven't looked into"*. Looking took ten minutes and
showed it was the **same** defect with the polarity reversed — the test's own comment described a
precondition it never created. Nine of nine, not eight of nine. Final sweep: **0 failures of 89,
down from 9**; ordered run unchanged at 104 assertions.

## Lifecycle

- **Run it on the revision, not the original.** The original was reviewed; the revision is what has
  outgrown its title and its earlier measurements.
- **Keep the vector list per house**, with each objection's verdict and the command that settled it.
  It becomes the pre-push gate for the next change and it takes minutes the second time.
- ⚠️ **A vector that ends in "probably fine" has not been resolved.** Push it to a measurement or
  record it as an accepted style objection with the reason, so the next person is not re-deciding it.
- ⭐ **Report the attack's yield honestly in your own notes** — nine refuted, one kept, one real. A
  red-team that finds nothing was aimed at the wrong rules.