lucasfcosta/backpressured/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

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

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

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

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