Type Design Review

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 nulls 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).

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

tsCopy code
// 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

tsCopy code
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

tsCopy code
// 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:

tsCopy code
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.

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

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."