/defect-hunting

name: defect-hunting
description: Use when reviewing code, auditing an app, writing a guard rule or test, or certifying any threshold/constant/formula. Finds the defects a diff review structurally cannot see — the second write path, the mis-calibrated constant, the rule that cannot fail. Triggers from code-reviewer, /app-audit, qa-engineer, verification-engineer, and any task that adds a lint rule, architecture test, or CI grep.

Defect hunting

Mined from a real remediation programme where twelve screen-by-screen review rounds found nothing new, and one round organised differently found dozens of live defects. Every one of these rules is paid for.

**The sentence that generates all four:**

> Verify the thing that has to be true, not the thing you changed.

Filtering the list is not filtering the parser. Capping the severity is not capping its renderer. Matching the token is not matching the value. Mutating the file is not confirming the mutation landed.

---

1. Audit the data's entry points, not the screens

A review organised by screen structurally cannot find these, because in every case **the audited surface was correct and the bug was in the second path to the same data** — usually another file, often another module:

| What was reviewed | Where the defect actually was | |---|---| | Add-form validation | the **edit** path, which validated nothing | | the dashboard alert banner | the **detail screen** the banner opens — they disagreed | | the growth-target reader | the **writer**, which destroyed data | | the photo picker's success branch | its **cancel** branch, which wiped the existing photo | | the purchase flow | the **still-loading** entitlement state, which paywalled a paying customer | | sync's happy path | the `RECONCILE_FAILED` branch |

The question that does the work

> **"What is the second way this value gets written?"**

And its siblings: what is the second way it gets *read*? What happens on cancel? On failure? On restore? On import? On sync? On each remaining enum case?

Procedure

Before approving any change that touches persisted or user-visible state:

1. Name the data the change touches — the field, the row, the preference, the entitlement. 2. **Enumerate every writer.** `grep` the field name across the whole repo, not the module — prefer `ast-grep` where the language has one available, same reasoning as §1b: text search false-negatives on a renamed reference or a match sitting inside a comment or string. Create, edit, import, sync, restore, migration, reset-to-default, and every failure branch. 3. **Enumerate every reader**, same way. 4. Show the invariant holding **on each one**. A validation that one producer applies and another walks around is not a validation. 5. If you cannot enumerate them, say so — do not approve on the strength of the path you read.

A one-line fix at the shared choke point beats a guard in every caller, and it is also the *lazier* fix. Patching only the path the ticket names leaves every sibling caller broken.

---

1b. Prove the new code is reachable — and its required companions exist

Found in this studio's own work, by a review that finally checked mechanically instead of by eye: a component (`VerdictBar`) was written, exported, and never imported anywhere. It typechecked. It built. It shipped in a merged PR. Nothing in §1–§4 would have caught it, because §1 asks whether a value's *other* writers and readers agree with each other — this is the narrower, cheaper question of whether the diff's *own* new code is ever reached by anything at all.

The question that does the work

> **"Who calls this, outside its own definition?"**

Applied to every function, component, type, constant, or route the diff introduces.

Procedure

1. For each new symbol the diff adds, search for a real call/reference site **outside the file that defines it** (or outside its own declaration, for a symbol used only within one file). `export`ed with zero importers is the specific, mechanical shape to catch — `grep -rn '<Name>' src/ | grep -v '<file that defines it>'` at minimum; prefer an AST-aware search (`ast-grep`) over plain `grep`/`rg` where the language has one available, since text search false-negatives on a renamed reference or a match sitting inside a comment or string. 2. **Missing co-changes** — the same "enumerate" discipline as §1, aimed at what a change like this one *always* requires alongside it, not at what it says it changed: an implementation with no test file for it, a new database column with no migration, a new API handler with no route registration, a changed struct/type with no update to its (de)serialization. Name the class of companion this diff's *kind* of change requires, then confirm each one is actually present. 3. Dead code found this way is not a style nit — it is unreviewed, untested surface area sitting in the codebase looking finished. Either wire it in or delete it. "Leave it for later" is how it becomes permanent.

---

2. Never certify a number by reading it — execute it

An age-aware plausibility envelope read perfectly sensibly, survived 35 sprints and every review. Executed against the app's own bundled reference table, it rejected the **median** child at 26 of 61 ages: an average 12-month-old was told their measurement looked wrong.

Mis-calibration is invisible to inspection **because the code is correct**. The arithmetic does exactly what it says. Only the numbers are wrong, and numbers do not read as wrong.

Same failure shape as a fabricated reference standard and a wrong lookup table forced on the wrong population: plausible-looking numbers that nobody ever ran.

The rule

Any threshold, bound, formula, coefficient, table, or rate that makes a **clinical, financial, or safety claim** must be checked by *executing it across its whole input range against reference data*, and the comparison recorded.

for every input across the re