Skimmable
Review the change as if a tired reviewer must understand it in one fast pass.
Prefer deleting code to explaining code.
Keep the pass narrow: improve readability, state modelling, and diff clarity without changing product behaviour unless the current behaviour is obviously dead, redundant, or inconsistent with types.
Checklist
Apply these rules aggressively:
- Make the code obvious on first read.
- Reduce the number of states the code can represent.
- Reduce parameter count.
- Remove optional parameters unless they are truly required by callers.
- Replace boolean flags and loose option bags with discriminated unions when behaviour differs by mode.
- Handle every known variant exhaustively.
- Fail on unknown variants immediately.
- Trust static types inside the typed core; do not add defensive branches for impossible states.
- Assert at boundaries when loading data, parsing input, or reading external systems.
- Prefer early returns over nested conditionals.
- Keep functions cohesive; do not split logic into many tiny wrappers.
- Avoid clever helpers, indirection, and generic abstractions.
- Remove overrides, escape hatches, and customisation points unless there is a proven need.
- Remove dead paths, temporary compatibility layers, and unrelated edits.
- Bias toward fewer lines and fewer moving parts.
Working method
Follow this order:
1. Trim the diff
Remove changes that do not help the main goal of the PR.
Delete:
- incidental renames
- opportunistic refactors
- styling churn with no behavioural value
- compatibility code for callers that no longer exist
- unused helpers, types, branches, and comments
If a simplification requires a separate behavioural decision, stop and call it out instead of smuggling it into the readability pass.
2. Simplify data shape first
Reduce complexity in types before touching control flow.
Prefer:
- required fields over optional fields
- one obvious input shape over many flexible ones
- discriminated unions over booleans or loosely related fields
- named domain types over generic maps and stringly typed objects
Ask of every argument:
- Is it required?
- Can it be derived?
- Can two arguments become one domain object?
- Is it just an override for an API that is too flexible?
3. Simplify control flow
Make happy paths short and visible.
Prefer:
- early returns
- straight-line code
- one level of indentation where possible
- local branching near use
Avoid:
- nested
ifpyramids - defensive
elsebranches for impossible states try/catchfor expected values- hidden branching spread across helper functions
4. Make invalid states loud
Assert at boundaries.
Use assertions when:
- decoding external data
- reading files, env vars, or request payloads
- converting untyped data into typed domain values
- checking invariants that must hold
After the boundary, write simple code that trusts the asserted shape.
5. Exhaust variants
When code depends on a variant, make the variant explicit and handle all cases.
Prefer a switch on a discriminant.
End with an exhaustive check or assertion if the language allows it.
Do not silently ignore unknown values.
6. Collapse unnecessary abstraction
Inline small one-use wrappers when they hide the real logic.
Keep helper functions only when they:
- remove real duplication
- encode a domain concept
- make the caller meaningfully clearer
Do not split a 20-line readable function into five 4-line helpers just to look tidy.
Preferred patterns
Keep arguments few and strict
Bad signs:
- more than 3 parameters
- multiple booleans
- optional callback plus optional overrides plus optional config
- passing values already available on an object
Prefer:
- one required object when the fields belong together
- one discriminated union when there are modes
- separate functions when the behaviours are actually different
Prefer discriminated unions over booleans
If a boolean changes behaviour, model the behaviour directly.
Use:
kind: "draft" | "published"source: "cache" | "network"status: "idle" | "loading" | "success" | "error"
Avoid:
isDraftuseCacheisLoadingpluserrorplusdataon the same object unless the state machine truly permits all combinations
Prefer assertions over defensive code
At boundaries, assert once. Inside, trust the assertion.
Avoid repetitive checks like:
if (!user) returnif (!user.id) returnif (typeof user.id !== "string") return
when types and boundary validation already guarantee the shape.
Prefer one obvious path
If there is a default behaviour, encode it directly.
Do not add overrides like sortOrder = "desc", strategy = "auto", separator = ",", shouldLog = false unless callers truly need them.
Every option multiplies states.
Review prompts
Use these prompts during the pass:
- Can this function lose a parameter?
- Can this object lose an optional field?
- Can this branch become an early return?
- Can these two booleans become one union?
- Can this helper be inlined?
- Can this error path become an assertion at the boundary?
- Can this code fail loudly instead of guessing?
- Can this diff lose files or lines?
- If a reviewer skims this in 30 seconds, will the intent be obvious?
Examples
Reduce arguments and remove overrides
Before:
function createReport(
userId: string,
includeDrafts = false,
sortBy: "date" | "name" = "date",
sortDirection: "asc" | "desc" = "desc",
limit?: number,
overrides?: { timezone?: string; now?: Date }
) {
const now = overrides?.now ?? new Date()
const timezone = overrides?.timezone ?? "UTC"
return buildReport({
userId,
includeDrafts,
sortBy,
sortDirection,
limit,
now,
timezone,
})
}
After:
type ReportScope =
| { kind: "published" }
| { kind: "all" }
function createReport(userId: string, scope: ReportScope) {
return buildReport({ userId, scope, now: new Date(), timezone: "UTC" })
}
Prefer a smaller API. Add configurability only when a real caller needs it.
Replace booleans with a discriminated union
Before:
type SaveState = {
isSaving: boolean
error?: string
receipt?: string
}
After:
type SaveState =
| { kind: "idle" }
| { kind: "saving" }
| { kind: "error"; message: string }
| { kind: "saved"; receipt: string }
The second version prevents impossible combinations.
Handle variants exhaustively
Before:
function badgeColor(status: Status) {
if (status === "success") return "green"
if (status === "error") return "red"
return "gray"
}
After:
function badgeColor(status: Status) {
switch (status) {
case "success":
return "green"
case "error":
return "red"
case "pending":
return "gray"
default:
return assertNever(status)
}
}
Unknown variants should fail loudly during development.
Assert at the boundary, trust inside
Before:
async function loadAccount(accountId: string) {
const raw = await db.accounts.find(accountId)
if (!raw) {
throw new Error("Account not found")
}
if (typeof raw.email !== "string") {
throw new Error("Invalid account email")
}
return raw
}
function sendWelcomeEmail(account: any) {
if (!account) return
if (!account.email) return
mailer.send(account.email)
}
After:
async function loadAccount(accountId: string): Promise<Account> {
const raw = await db.accounts.find(accountId)
assert(raw, "Account not found")
assert(typeof raw.email === "string", "Invalid account email")
return raw as Account
}
function sendWelcomeEmail(account: Account) {
mailer.send(account.email)
}
Validate once. Keep the typed core simple.
Prefer early returns over nesting
Before:
function visibleItems(items: Item[], user?: User) {
if (user) {
if (user.isAdmin) {
return items
} else {
return items.filter((item) => !item.hidden)
}
} else {
return []
}
}
After:
function visibleItems(items: Item[], user?: User) {
if (!user) return []
if (user.isAdmin) return items
return items.filter((item) => !item.hidden)
}
Flatten the shape of the code.
Do not split logic into too many tiny functions
Before:
function checkout(cart: Cart) {
validateCart(cart)
const subtotal = calculateSubtotal(cart)
const tax = calculateTax(subtotal)
const total = calculateTotal(subtotal, tax)
return finalizeCheckout(total)
}
function calculateTotal(subtotal: number, tax: number) {
return subtotal + tax
}
After:
function checkout(cart: Cart) {
assert(cart.items.length > 0, "Cart is empty")
const subtotal = sum(cart.items.map((item) => item.price))
const tax = subtotal * TAX_RATE
const total = subtotal + tax
return finalizeCheckout(total)
}
Keep trivial logic close to use.
PR gate
Before opening or updating the PR, verify:
- The diff contains only necessary changes.
- Public APIs are stricter or simpler, not more configurable.
- Modes are modelled explicitly.
- Variants are handled exhaustively.
- Assertions exist at boundaries.
- The core logic trusts types instead of re-checking everything.
- Functions read top-to-bottom with minimal nesting.
- Argument counts are low.
- Helpers exist only when they clarify.
- The final version is shorter, plainer, and easier to skim.
If a choice is between clever and obvious, choose obvious.
If a choice is between flexible and simple, choose simple.
If a choice is between adding code and deleting code, prefer deleting code.