Functional TypeScript Code Review

A guide for reviewing server-side TypeScript code against the kamae-ts principles (see index). Each checklist item maps one-to-one to a chapter of the principles.

This guide is a human-readable version of the checklist used internally by the kamae-review skill in the kamae-ts plugin. Use it as a reference when reviewing manually without a coding agent.

Review Procedure

  1. Read the principle knowledge base first. Before looking at code, read the following so you can cite canonical principles in your findings:
  2. Read the files under review.
  3. Scan through the checklist items below in principle order (matching the chapter structure of index.md).
  4. When you find a violation, report it with the principle, the reason it matters, and a suggested fix.
  5. When something is not a violation but could be improved, present it as a suggestion.

Checklist

The checklist directly mirrors the structure of index.md. Each item links back to the canonical chapter.

1. Type-Driven Domain Modeling

1.1 Domain state modeled as a Discriminated Union

Reference: ./index.md §1 “Express domain state with Discriminated Unions”

Signal: a single type with many optional properties and a string status field (e.g. { state: string; driverId?: string; startTime?: Date }). Suggest splitting into per-state types combined into a union, making state-specific properties required.

1.2 Discriminant unified as kind

Reference: ./index.md §1 “Use kind as the unifying discriminant”

Signal: a discriminant named type, status, state, _tag, or anything other than kind. Suggest renaming to kind for codebase consistency.

1.3 No use of class for domain models

Reference: ./index.md §1 “Express domain state with Discriminated Unions” and the Companion Object pattern.

When class is used to define a domain entity or value object, suggest migrating to a Discriminated Union + Companion Object pattern. Class inheritance required by an external library is a justified deviation.

1.4 Companion Object pattern followed

Reference: ./index.md §1 “Companion Object pattern”

Verify:

  • Operations related to a type are consolidated in a const with the same name as the type.
  • A Branded Type’s validation schema is exposed as a .schema property on the companion object, not as a standalone XxxSchema export.
  • Domain logic that belongs on the companion object is not scattered as free-standing functions like xxxAssignDriver.

1.5 No interface for domain types

Reference: ./index.md §1 “Use type, not interface”

Declaration merging can silently alter a type’s shape. Define domain types with type. interface is only acceptable for library type augmentation.

1.6 No method notation in type definitions

Reference: ./index.md §1 “Use function-property notation, not method notation”

Method notation (save(task: Task): Promise<void>) makes parameters bivariant, allowing a narrower implementation (save(task: DoingTask): …) to pass type-checking at the injection site. Suggest switching to function-property notation (save: (task: Task) => Promise<void>).

1.7 Branded Types applied to semantically distinct primitives

Reference: ./index.md §1 “Use Branded Types to distinguish meaning”. Also see the project’s validation-library guide (./validation-libraries/).

Signal: IDs or semantically distinct values (UserId, OrderId, Email, monetary amounts, etc.) represented as plain string / number. If a validation library is present, verify that its branding feature is used (no as cast needed); otherwise confirm the unique symbol pattern is in place.

1.8 Domain objects wrapped in Readonly<>

Reference: ./index.md §1 “Use Readonly<> to guarantee immutability”

Signal: domain object type definitions not protected by Readonly<…> (or per-property readonly). State changes should be expressed by constructing new objects.

1.9 One-concept-per-file layout

Reference: ./index.md §1 “File layout: one concept per file”

Signal: catch-all files such as types.ts, models.ts, or domain.ts that aggregate many domain types, especially when companion objects live in separate files. Barrel files (index.ts) should contain re-exports only.

1.10 Domain-facing ports owned by the domain layer

Reference: “Place Ports in the Domain Layer”

Signal: repository, resolver, store, or other domain-facing dependency contracts defined outside the domain layer or collected in a dedicated port/ or ports/ directory, including domain/ports/. Recommend placing each contract beside the domain concept it serves (for example, src/domain/task/task-store.ts). Use cases and adapters should import that domain-owned contract; concrete I/O implementations remain in infrastructure.

Treat placement alone as Low. Raise to Medium when the domain contract imports an infrastructure implementation, database client, or external SDK type, and cite that dependency. Do not infer a dependency violation or runtime failure from a directory name alone. A concrete repository adapter correctly placed in infrastructure is not a misplaced port; inspect whether the file defines the contract or implements it. Respect explicit project overrides.

1.11 Separate resolvers and stores, preferably one operation each

Reference: Separate resolvers and stores by operation

Signal: an injected contract mixes reads and writes, or a resolver/store exposes multiple independent operations without a documented project reason. Report this as Low, citing the contract and its consumer. Suggest separate contracts with one method each, and give each consumer only what it needs. An event-only writer does not require a resolver.

Judge responsibilities rather than names: findById on a single-operation resolver is valid, as is a single-operation contract named Repository. Adapters may share a database client, and the composition root may assemble separate contracts. Keep an atomic state-and-events write in one store method. Respect explicit project overrides for broader contracts and explain the trade-off.

2. State Transitions via Pure Functions

Reference: ./index.md §2 and ./state-modeling.md

2.1 Transition functions constrain the source state via argument types

Signal: a transition function’s argument type is the full union (TaxiRequest) rather than an individual state (Waiting). Accepting a wide type allows calls from invalid source states.

2.2 assertNever present in switch over a Discriminated Union

Reference: ./index.md §2 “Exhaustiveness check”

Signal: a switch branching on kind without default: return assertNever(x). Adding a new variant will not produce a compile error.

3. Error Handling — Railway Oriented Programming

Reference: ./index.md §3, ./error-handling.md, and the project’s Result-library guide (./result-libraries/).

3.1 Does each failure cross the boundary appropriately?

Classify an observed throw or caught error before reporting it. Apply the decision test: does a consumer have a specified domain decision for this failure?

Observed failure Review action
Expected validation or business-state failure is thrown Require a use-case-specific Result error
External failure has documented recovery but is thrown Model the named recoverable failure in Result
An arbitrary technical fault is caught, wrapped or renamed as a catch-all error, and added to a domain Result union Allow propagation to the application error boundary instead of using a catch-all domain error, regardless of the type or field names
Private sentinel is caught by its associated boundary after discriminating unknown, and every other value is rethrown No finding

Do not report throw inside assertNever, a failed internal assertion that propagates, or an unexpected fault that reaches the application error boundary. That boundary owns logging and the generic operational response. A private sentinel is acceptable only when it is clearer than equivalent Result composition, is tightly scoped, is recognized exclusively by its associated catch boundary, rethrows all other caught values, and does not represent an expected domain failure. Prefer Result when the two forms are equally clear. Converting an assertion or arbitrary technical fault into a catch-all domain Result error is a finding even when the wrapper is not named RepositoryError and its payload is not named cause.

Also flag: ResultAsync.fromSafePromise (or equivalent “safe” wrapper in other libraries) wrapping a Promise that can reject — database calls, network I/O, external API calls. fromSafePromise is a contract stating the Promise never rejects; violating it bypasses the Result error channel and produces an unhandled rejection at runtime. Suggest fromPromise with a named error only when the workflow specifies a recovery decision; otherwise let the rejection reach the application error boundary. Reference: ./error-handling.md §fromSafePromise Misuse

For fp-ts, do not flag an internal TaskEither that carries unexpected faults in a distinct execution channel when a native Promise boundary rethrows the original cause after execution. Distinguish this transport from adding faults to the domain error union or exposing them as business outcomes. Flag rejecting I/O represented as Task / lifted with TE.fromTask, or a TE.tryCatch error mapper that rethrows, as contract violations. See the fp-ts guide.

3.2 Error types defined as Discriminated Unions

For expected errors exposed in a Result or another public business contract, flag Error subclasses, free-form string error codes, or Result<T, string>. Suggest a Discriminated Union ({ kind: "DriverNotAvailable"; driverId } | { kind: "RequestAlreadyAssigned" }) so callers can handle expected outcomes exhaustively. Do not apply this rule to unexpected infrastructure faults, assertions, or contract-violation exceptions that propagate to the application error boundary.

Also flag: error DU variants where contextual data (IDs, codes, values that caused the error) exists only in a message: string field and is not exposed as typed fields. A message field itself is fine for logging or display, but when callers must parse it to extract values for branching or retry logic, the typed error has lost its purpose. Suggest adding the relevant context as named fields alongside message. Reference: ./error-handling.md §Designing Error Types

3.3 Result composition avoids unnecessary unwrap/re-wrap

Flag only an unnecessary unwrap followed by re-wrapping in the middle of composing expected decisions. In that case, cite the relevant guide under ./result-libraries/ and suggest the matching combinator. A clear if or match after the expected decision is complete is allowed, especially when crossing into native async persistence whose rejection must propagate. The official option-t recipe is intentionally explicit at that boundary.

Also flag: andThen / map callbacks exceeding ~5 lines or containing multi-branch if/else logic. This is procedural code wrapped in a Result combinator, not Railway Oriented Programming. Suggest extracting each logical step into a named function so the chain reads as a flat pipeline of operations. Reference: ./error-handling.md §Composing Operations

4. Boundary Defense

Reference: ./index.md §4, ./boundary-defense.md, and the project’s validation-library guide (./validation-libraries/).

4.1 Schema validation at every external boundary

Signal: API handlers, DB result mappings, queue/message handlers, file/config reads, or environment-variable reads where raw data is treated as a domain type without being parsed through a validation library (Zod / Valibot / ArkType).

4.2 No as type assertions

Reference: ./index.md §4 “Do not use type assertions (as)”

The only permitted as forms are as const and as const satisfies Type. Enumerate every other as and verify it falls into one of:

  • External or unknown-typed data: should be replaced with schema parsing. as does not give the guarantee its type claims.
  • as inside a Branded Type constructor: tolerated only as a last-resort fallback when no validation library is present (unique symbol pattern). When flagged, recommend introducing a validation library and rewriting the brand with z.brand() / v.brand() / .brand() so the as can be removed.
  • Internal data: should be resolvable via type inference. If not, the type design is likely wrong.

4.3 PII fields wrapped in Sensitive<T>

Reference: ./index.md §4 “PII defense”, ./boundary-defense.md

Signal: fields that may contain personal information (name, email address, phone number, address, various IDs, payment information, health/diagnostic information, IP address, etc.) represented as plain string / number. Pay particular attention to objects that may appear in logs or error messages. Also check that the validation schema automatically wraps PII fields with Sensitive.of.

5. Declarative Style

Reference: ./index.md §5, ./state-modeling.md

5.1 Array operations written declaratively

Signal: transformations expressible with filter / map / reduce being built imperatively with for / for…of loops. Suggest defining predicate functions on the companion object and writing tasks.filter(Task.isActive).

5.2 Domain events published as immutable records

Signal: state-mutation code directly mutating a shared event log, or domain events not being published where the state-modeling guide requires them. Events should be recorded as Readonly<{ eventId; eventAt; eventName; payload; aggregateId }>, separated from the repository.

5.3 Companion-object predicates free of redundant x is Y annotations

Signal: predicate functions over a discriminated union that carry an explicit : x is Y return-type annotation when the body is just kind === "..." comparisons (or their !== negation). TypeScript 5.5+ infers the type predicate from such bodies and Array.prototype.filter consumes the inferred predicate, so the annotation adds nothing — and falsely implies that discriminated union narrowing alone is insufficient. Suggest dropping the annotation.

6. Test Data

Reference: ./index.md §6

6.1 Fixtures defined with as const satisfies Type

Signal: test fixtures typed with : Type = or as Type, causing discriminant literal types to widen to string. Suggest as const satisfies Type to preserve the kind literal type.

How to Write Findings

Each finding should include:

  1. What is wrong: the specific code location (path:line).
  2. Why it matters: the principle (with a reference link to ./...) and the risk introduced by the violation.
  3. How to fix it: a code example showing the suggested fix.
### Method notation used

`src/domain/task/task-store.ts:15`

`save(task: Task): Promise<void>` uses method notation.
Per [`./index.md` §1 "Use function-property notation"](/kamae-ts/en/),
method notation makes parameters bivariant, so a narrower implementation
`save(task: DoingTask): Promise<void>` would pass type-checking at the injection site.

Suggested fix:
\`\`\`typescript
type TaskStore = {
  save: (task: Task) => Promise<void>;
};
\`\`\`

Severity

Severity Item Reason
[High] as type assertions (4.2) Direct cause of runtime errors
[High] Unprotected PII (4.3) Compliance violation risk
[High] Missing schema validation at external boundaries (4.1) Direct cause of runtime errors
[High] Missing Branded Types for semantically distinct primitives (1.7) Mixed-up IDs surface at runtime
[Medium] Use of class (1.3) Degrades type safety during extension
[Medium] State modeled with optional properties (1.1) Invalid states become representable
[Medium] Expected domain, business, or validation failure is thrown (3.1) Callers cannot handle an expected outcome explicitly
[Medium] Expected error in a Result or public business contract is not a Discriminated Union (3.2) Callers cannot handle expected errors exhaustively
[Medium] Missing assertNever (2.2) New variants go undetected at compile time
[Medium] Transition function accepts the full union type (2.1) Invalid transitions compile without error
[Medium] Catch-all type files (1.9) Circular dependencies; type/behavior separation
[Medium] Companion Object pattern violated; schema exported standalone (1.4) Leaks implementation details
[Medium] Domain contract imports infrastructure types (1.10) Couples the domain to a concrete implementation
[Low] Misplaced domain-facing port without an outward dependency (1.10) Separates the contract from its owning domain concept
[Low] Unnecessarily broad resolver/store contracts (1.11) Consumers depend on unrelated operations
[Low] Method notation (1.6) Only problematic under specific conditions
[Low] interface for domain types (1.5) Declaration-merging accidents are rare
[Low] Domain types missing Readonly<> (1.8) Mutations are often caught in review
[Low] Discriminant is not kind (1.2) Style inconsistency rather than a bug
[Low] Imperative array loops (5.1) Readability, not correctness
[Low] Domain events not published (5.2) Depends on whether event sourcing is adopted
[Low] Redundant x is Y predicate annotation (5.3) Wastes characters; misleads about discriminated union narrowing
[Low] Fixtures missing as const satisfies (6.1) Typically caught by tests in practice
This site uses Just the Docs, a documentation theme for Jekyll.