ktx/docs/code-design.md

ktx Code-Design Principles

Principles agents must apply when writing or changing behavior in this repository. These rules carry the same weight as the MUST / MUST NOT rules in AGENTS.md.

Overengineering rarely looks like over-engineering at the line level. It shows up as small, locally-reasonable choices that combine into a system where features fail silently and bug fixes have to be applied N times. The principles below are the lessons; if a piece of code violates one, that is enough reason to fix it even when the local code "works."

One way to say one thing

• MUST NOT: Accept two spellings of the same intent — e.g. a magic sentinel value AND absence-of-field both meaning "use the default". Pick one and reject the other.
• MUST NOT: Maintain two entry points that load/construct/resolve the same thing where one does strictly more work than the other. Callers will pick the wrong one. Unify them, or encode the difference as a required argument on a single entry point.
• MUST NOT: Let each consumer write its own private wrapper around a shared helper to make it usable. If three callers each prepend the same three lines, those three lines belong in the helper.

Behavior follows from inputs, not from which path the caller took

• MUST: A function's result must depend on its arguments, not on which sibling function the caller happened to invoke first. If "did setup step S run?" determines correctness, S belongs INSIDE the function that needs it, or its absence must be a hard error — not a silent degradation.
• SHOULD: When a value on disk requires runtime resolution (start a process, read state, hit a service), the resolution belongs in ONE place that every consumer goes through. If some consumers get the resolved form and some get the raw form, the abstraction is broken.

Failures must reach a decision-maker

• MUST NOT: Catch an error, log it through a logger that may be a no-op, and continue with a null/empty result. The error reaches no one. Either surface the failure to the caller (return type, status field, stderr line), or throw.
• MUST: A caller that receives "no result" must be able to distinguish "the input legitimately produced nothing" from "a dependency was unavailable" from "the operation was skipped." If those three look the same to the user, the system is hiding bugs — including this one.
• MUST: When a function returns T | null (or a "skipped" status), at least one caller in the codebase must branch on the negative case and surface it. If every caller treats absence as success, the function is laundering errors.

Don't build seams without a second piece on the other side

• MUST NOT: Introduce an interface, abstract type, or "port" boundary with exactly one implementation and no concrete plan for a second. Abstractions are paid for with indirection; pay only when you collect.
• MUST NOT: Add an optional dep-injection slot (deps.X ?? defaultX) unless at least one test exercises the production default. If every test injects a fake, the production codepath is type-checked and untested.
• MUST NOT: Add a wrapper "in case" callers later need to extend it. Add the wrapper when the second caller arrives.

Specification and behavior are one artifact

• MUST: When a schema, doc comment, or config description states a default or a meaning, the code MUST enforce it. Drift between "what the field claims" and "what the code does" is a contract bug even if both compile.
• MUST: When you change behavior, update the schema description, the doc, AND the example in the same change. Not later.

Verify the path you claim to have fixed

• MUST: Before claiming a feature works, run a command that actually exercises it end-to-end and observe the side effect — the file written, the process contacted, the row stored. Type-check passing is necessary, not sufficient. A test passing against a fake is not evidence the real path works.
• MUST: Before declaring a bug fixed, grep for the same shape elsewhere. Bugs of the kind described in this section repeat. Fix the class, not just the instance.

Naming asymmetries are bugs in waiting

• SHOULD: When two related identifiers have non-parallel names (loadX vs loadHigherX, createY vs createDefaultY, xClient vs xService), assume callers will pick the wrong one. Unify, or document inline why both must exist.

Dispatch and contract leaks across per-variant layers

Layers with multiple per-variant implementations (warehouse drivers, dialects, LLM providers, ingest adapters, historic-SQL probes) drift toward parallel switches and informal contracts. The patterns below look locally reasonable per file but multiply with the number of variants times the number of consumers — every fix has to be applied N times, and silent drift between variants is invisible until a user hits it.

• MUST NOT: Maintain two or more files that switch on the same enum or string union to dispatch to per-variant behavior. Promote the dispatch to a single registry table keyed by the union, exposed through one resolution function. If you find yourself writing the third such switch, the second one was already a bug.
• MUST: When every variant of an abstraction implements the same method, the method belongs on the shared interface. An informal contract that every implementation happens to satisfy is a leak waiting to happen — callers will reach for the concrete class instead of the contract, and the next variant added will silently forget to implement it.
• MUST: When a layer has both a thin shared interface and rich per-variant concrete classes, they must agree. Either widen the interface so callers never need the concrete class, or make the concrete class private (test-only /** @internal */ JSDoc plus a boundary check in scripts/check-boundaries.mjs). A class that is public AND has methods the interface does not expose is the exact configuration that produces leaks.

The warehouse driver / dialect layer in packages/cli/src/connectors/<driver>/ plus packages/cli/src/context/connections/{dialects,drivers}.ts is the canonical worked example: per-driver dialect classes carry /** @internal */, scripts/check-boundaries.mjs enforces the import boundary, and dispatch lives in the two registry files. Apply the same shape to any other per-variant layer that grows beyond two implementations.