---
name: type-design-review
description: Use when a backpressured code-review subagent is judging whether a diff uses the type system to make illegal states unrepresentable — i.e. the diff touches data models, function signatures, or domain types. TypeScript-centric, with mappings to Rust, Swift, Kotlin, and Haskell/OCaml.
---

# Type-Design Review

## Overview

**Push every invariant you can into the types, so the compiler — not a runtime check, not a test, not a human reviewer — is the thing that says "no."** This is the type-system instance of backpressure: a contradictory state that cannot be *constructed* needs no defensive check, no test, and no "this should never happen" comment, because it genuinely never can.

You are reviewing a diff for this property. Your job is not "do the types compile" — it is "could a wrong state have been represented at all, and if so, why wasn't it designed out?"

Three ideas, one throughline:
- **Make impossible states impossible** (Richard Feldman, elm-conf 2016): if the app forbids a combination of data, design the data model so that combination cannot be expressed.
- **Parse, don't validate** (Alexis King, 2019): don't check data and throw the knowledge away — parse it into a type whose existence *is the proof* of the invariant, then carry that type inward.
- **Make illegal states unrepresentable** (Yaron Minsky, *Effective ML*, c. 2010): making the wrong thing hard to express beats checking for the wrong thing at runtime.

## When to Use

- A `backpressured` code-review subagent is reviewing a diff (per-iteration or whole-changeset) and the diff introduces or changes **data models, function signatures, or domain types**.
- New state is modelled with **booleans / optional fields**, new `switch`/`match` over a union, new `as`/`!` assertions, or new functions taking raw `string`/`number`.

**Not for:** pure formatting, dependency bumps, or changes with no type-level surface. **And not as a general reviewer** — correctness/logic bugs, simplicity/reuse, and test quality belong to the general code-review reviewer. This skill is *only* the type-design lens: could a wrong state have been represented at all?

## The one question — apply it to every type in the diff

For each data model, signature, or domain type the diff adds or changes: **enumerate the states the type permits, then ask which of those the domain forbids.** Each permitted-but-forbidden state is a candidate finding, and the fix is to restructure so it can no longer be expressed. That single question *is* the review — the smells below are common instances of it, **not a closed checklist**. A diff can make an illegal state representable in a shape no row names (`end` before `start`, `min > max`, a list that should be a set, three different `null`s meaning three different things); that still counts, and you are expected to catch it from the principle. Run the question first; reach for the table for vocabulary and ready-made fixes, not as the boundary of what to look for.

## The smells — common instances (not a checklist)

Treat each row as a finding **only when it actually makes a domain-forbidden state representable here** — not on sight. The left column is a pattern to *notice*; it becomes a finding only once you can name the real illegal state it permits in this code (a matching pattern that forbids nothing real is not a finding — see *Proportionality*).

| Smell | Why it's a bug waiting to happen | Push toward |
|-------|----------------------------------|-------------|
| Boolean flags that can't all be true at once (`isLoading` + `isError` + `isSuccess`) | `2^n` combinations exist; most are nonsense the types permit | one discriminated union with a `status` discriminant |
| Optional fields that are really mutually exclusive (`error?` **and** `data?`) | "loaded *and* erroring" is representable but meaningless | a tagged union, one variant per real state |
| `as` / `as any` / `<T>x` casting into or out of a union | an assertion the compiler can't verify — reintroduces the runtime error types exist to prevent | narrow with a user-defined type guard, or parse |
| Non-null assertion `!` to silence "possibly undefined" | crashes at runtime if the value *is* null | narrow with a check, or restructure so it can't be null |
| `switch`/`match`/`when` with no exhaustiveness check | adding a variant later silently falls through instead of failing the build | `default: return assertNever(x)` (see below) |
| Primitive obsession / stringly-typed (`string` for ids, status, email, money, units) | `UserId` and `OrderId` are interchangeable; typos compile | branded/newtype/opaque types, literal unions |
| Validation returning `boolean` or `void` | the knowledge is discarded; every caller must re-trust | a parser returning a proof-carrying type |
| Two fields that must stay in sync (`list` + `selectedIndex`, cached `count` + array) | desync is representable; one can be updated without the other | a single structure that makes desync impossible; derive don't store |
| `enum` where a string-literal union would do | runtime footprint + surprising semantics (numeric enums accept any number) | `"a" \| "b"` union, often `as const`-derived |
| Mutable shared/value-object state | aliasing bugs, "who mutated this?" | `readonly`, `ReadonlyArray<T>`, `Readonly<T>` |

## Proportionality — generalize the principle, don't gold-plate

Applying the principle broadly is not licence to maximize type ceremony. Flag a representable illegal state when **all three** hold: it's realistically reachable (not merely theoretical), it would cause a real bug or force defensive checks / "this should never happen" comments downstream, and the restructuring is reasonably cheap and lives in domain/core code. Otherwise, leave it.

Do **not**:
- brand or wrap a value with no invariant to prove — free text a user types (a `note`, a `description`) is just `string`;
- force a union onto booleans/optionals that genuinely *can* coexist — a booking that is both `paid` and `cancelled` (refund pending) is a real state, and merging it would *lose* information, not add safety;
- restructure a throwaway local or one-shot internal shape with no downstream consumers.

When the domain genuinely permits every combination, the permissive type is the *correct* type — match the domain, not your appetite for tighter types. The test is always **"name the forbidden state this prevents"**; if you can't name one, it isn't a finding.

## Core pattern: impossible states, before/after

```ts
// BEFORE — a bag of optionals. isLoading && error && data is representable and meaningless.
interface State {
  isLoading: boolean;
  error?: Error;
  data?: User;
}

// AFTER — a discriminated union. Only the four real states exist; narrowing on
// `status` gives you exactly the fields valid in that state, nothing else.
type State =
  | { status: "idle" }
  | { status: "loading" }
  | { status: "success"; data: User }
  | { status: "error"; error: Error };
```

## Core pattern: exhaustiveness with `never`

```ts
function assertNever(x: never): never {
  throw new Error(`Unhandled case: ${String(x)}`);
}

function render(s: State): string {
  switch (s.status) {
    case "idle":    return "";
    case "loading": return "Loading…";
    case "success": return s.data.name;
    case "error":   return s.error.message;
    default:        return assertNever(s); // add a variant → this line stops compiling
  }
}
```

The `default: assertNever(s)` line is the point: a forgotten case becomes a **build failure**, not a silent runtime fallthrough. Flag every exhaustive-looking switch that lacks it.

## Core pattern: parse, don't validate

```ts
// BEFORE — validate returns a boolean; the result is lost, every caller must re-trust.
function isEmail(s: string): boolean { /* ... */ }
function sendInvite(email: string) { /* hope the caller checked */ }

// AFTER — parse returns a branded type; sendInvite cannot be called with an unparsed string.
type Email = string & { readonly __brand: "Email" };
function parseEmail(s: string): Email | undefined { /* check, then brand */ }
function sendInvite(email: Email) { /* the type IS the proof it was parsed */ }
```

Parse **at the boundary** (where untyped input enters), then pass the precise type inward. King's rule of thumb: treat functions that return `void` (Haskell's `()`) with deep suspicion — their effects are all you get; nothing is preserved in the types.

**Recommend the project's existing parser before a hand-rolled one.** Before suggesting a `x is T` guard, check what the repo already has — a schema/validation library in `package.json` or imported nearby (Zod, Valibot, ArkType, io-ts, runtypes, Yup, `@effect/schema`, …). If one exists, prefer pointing the fix at *it* (`Schema.safeParse(input)`): usually less code than a bespoke guard, a runtime error message for free, and consistent with the rest of the codebase. A hand-written guard is still perfectly fine for a trivial or local check — the rule isn't "always use the library," it's *don't reinvent a parser the project already has*, and **don't tell the author to add a new dependency just to satisfy this review** (suggesting they adopt one is fair at a real, repeated boundary; for a one-off, a hand guard or a flagged follow-up is the lighter call). Match the tool the project already uses, not your favorite.

## Branded / nominal types

TypeScript is *structural*, so `UserId` and `OrderId` are both just `string` and freely interchangeable. A phantom brand makes them distinct:

```ts
type Brand<T, B> = T & { readonly __brand: B };
type UserId = Brand<string, "UserId">;
type OrderId = Brand<string, "OrderId">;

declare function getUser(id: UserId): User;
declare const oid: OrderId;
getUser(oid); // ERROR — OrderId is not assignable to UserId
```

Brands should only be minted by a parser/constructor — that is what ties them to "parse, don't validate."

## When an escape hatch is acceptable

`as` and `!` are not *banned* — they are *unchecked*, so each one must be justified in the diff:
- ✅ `as const` (narrows + freezes a literal — not an unsafe assertion).
- ✅ `x satisfies T` instead of `const x: T = …` when you want the check **and** the narrow inferred type.
- ✅ An `as`/`!` immediately after a real runtime check you can see, against a truly external invariant.
- ❌ `JSON.parse(body) as User`, `resp as SuccessResponse`, `maybeUser!.name` — assertions of facts nothing verified. Push toward a type guard (`x is T`) backed by a real check, or a parser built with the project's existing schema library (see "Recommend the project's existing parser" above).

If the diff adds an `as`/`!` with no nearby justifying check, flag it and name the narrowing or parse that would replace it.

## Same principle, other languages

The TypeScript mechanics above are one dialect of a language-agnostic idea: a **closed sum type the compiler checks for exhaustiveness**, **no implicit null**, and a **newtype to defeat primitive interchangeability**.

| Language | Sum type / illegal-state mechanism | Exhaustiveness & null |
|----------|------------------------------------|-----------------------|
| Rust | `enum` with data per variant; newtype `struct UserId(u64)` | `match` exhaustive by default; `Option<T>`/`Result<T,E>`, no null |
| Haskell / OCaml | ADTs + smart constructors (the origin lineage) | incomplete-pattern warnings; `Maybe`/`option`, no null |
| Swift | `enum` with associated values; `struct` for products | `switch` must be exhaustive; `Optional<T>` (`T?`) |
| Kotlin | `sealed` class/interface hierarchies | `when` expression must be exhaustive; nullable `T?` tracked |

When reviewing non-TypeScript code, apply the same questions: is this a closed set of states the compiler can check? Is absence modelled explicitly rather than with null? Are domain ids distinct types or bare primitives?

## How to report findings

For each finding give: the **smell** (from the table), the **state that is wrongly representable** ("`error` and `data` can both be set"), and the **concrete restructuring** (the union/brand/parser to use). A finding without a representable-wrong-state is not a type-design finding — hand it to the general code-review reviewer instead. Push back is expected: if the author justifies an escape hatch with a real boundary check, that is a resolved finding, not a violation.

## Common rationalizations

| Rationalization | Reality |
|-----------------|---------|
| "The booleans are never set inconsistently in practice" | "In practice" is exactly the invariant the types should enforce. If it can't happen, make it unrepresentable. |
| "`as User` is fine, the API always returns that shape" | The compiler didn't verify it and the API can change. Parse at the boundary; carry the type in. |
| "Adding `assertNever` is boilerplate" | It is the difference between a forgotten case being a compile error vs. a production bug. |
| "It's just an id, a string is simpler" | A string lets `OrderId` flow into `getUser(userId)` silently. A brand costs one type and removes the whole class. |
| "Validation already happens at the controller" | If the type doesn't say so, the next caller doesn't know. Encode the proof, don't rely on convention. |

## Sources

- Richard Feldman, *Making Impossible States Impossible*, elm-conf 2016 — the SelectList example (a list whose selection is part of the structure, so you cannot select an item not in the list).
- Alexis King, *Parse, don't validate* (2019-11-05): parse into a proof-carrying type at the boundary. Two of her closing maxims (distinct bullet points in the essay): "Use a data structure that makes illegal states unrepresentable." and "Push the burden of proof upward as far as possible, but no further."
- Yaron Minsky, *Effective ML* (c. 2010, Harvard CS51 / Jane Street), popularized via *Real World OCaml*: "make illegal states unrepresentable."