Skip to content

RFC: split ColConfig into serializable descriptor + renderers (fixes 7 silent schema-pipeline defects) #86

Description

@mxkaske

Problem

ColConfig (packages/registry/src/lib/table-schema/types.ts:80-96) mixes two categorically different things: data (label, size, filter.min, filter.unit) and closures (display.cell, filter.component, sheet.component). Because they are mixed, serialization has to be a hand-written projection onto a parallel, hand-maintained type — ColumnDescriptor / FilterDescriptor / SheetDescriptor (types.ts:392-433) — and deserialization has to be a hand-written reconstruction (serialize.ts:143-270, a ~40-branch ladder).

Every defect below is a symptom of maintaining two types by hand.

Verified defects

  1. serializeSchema silently drops fields. FilterConfig.unit, FilterConfig.presets, and ColConfig.resizable have no counterpart in the descriptor types and are never written. .filterable("slider", { min: 0, max: 5000, unit: "ms" }) (apps/web/src/app/infinite/table-schema.tsx:158) loses unit on toJSON().

  2. The round-trip test is structurally incapable of detecting this. serialize.test.ts:228 defines roundTrip(def) = serializeSchema(deserializeSchema(serializeSchema(def))) — a fixed point on the lossy projection. All 23 round-trip assertions are blind to any field serializeSchema drops. You can delete unit from the serializer today and no test fails.

  3. array silently degrades to string. infer.ts:365 emits dataType: "array" with no arrayItemType for non-enum arrays; deserializeSchema's factory chain (serialize.ts:152-171) only handles arrays whose item is an enum, and otherwise falls through to col.string(). A string[] column with 11+ distinct values becomes a string column. This defect corrupts the input to server-side filtering — see the dependency note below.

  4. schemaToTypeScript emits code that does not compile. to-typescript.ts:145-147 emits col.${c.dataType}() in its else branch; for dataType: "array" that is col.array(), but col.ts:300-302 declares array<U>(itemBuilder: ColBuilder<U, any>) with a required parameter. No test parses, compiles, or evaluates the emitter's output — all 28 tests in to-typescript.test.ts build inputs by hand via a local makeCol() helper and assert with toContain().

  5. The emitter drops chain steps. buildChain (to-typescript.ts:120-255) never emits .hideHeader(), .resizable(), or enableHiding, though serialize/deserialize preserve two of them.

  6. Three divergent copies of "default display for a kind"infer.ts:32-51, generators/sheet-fields.ts:8-27, serialize.ts:46-50. The third disagrees with the other two: custom serializes to { type: "text" } unconditionally, so an enum column with a custom cell renderer serializes as text but generates a sheet field as badge.

  7. The sort hand-splice yields an internally inconsistent Schema. createSchema (lib/store/schema/index.ts:59-70) computes defaults eagerly. builder-table.tsx:65-74 and data-table-auto.tsx:61-70 both spread ...generated — carrying the pre-splice defaults and _type — then override only definition. So definition.sort exists while defaults.sort does not. Latent today (both call sites pass only .definition onward), live the moment anyone reads .defaults.

Test misallocation

~3,000 lines of tests against ~2,943 lines of source, distributed inversely to risk:

  • col.test.ts (355 lines) asserts on ._config, the field marked @internal … do not access directly at types.ts:117.
  • infer.test.ts (712 lines) characterizes private word-list Sets — ~14 cases on ID_WORDS/CODE_WORDS/EMAIL_WORDS membership alone. Adding a word breaks tests; changing what a user sees does not.
  • generators/columns.tsx342 LOC, zero tests. The largest generator, the one that actually produces the TanStack ColumnDef[]. Every sibling generator has a test file.
  • No test anywhere goes raw rows → usable table config.

Why this blocks other work

Defect 3 corrupts kind / itemKind on the deserialized schema. Any server-side filter-semantics work (see the companion RFC) needs exactly those fields to decide set-membership vs array-overlap. Building filter semantics on a lossy descriptor means the spec can be wrong before any engine sees it. This RFC should land first.

Proposed Interface

Split ColConfig into two disjoint types that share no property name.

/**
 * Everything about a column that survives JSON. TOTAL: no serializable
 * property of a column may live outside this type.
 */
export type ColumnDescriptor = ColumnDescriptorCommon &
  (
    | { kind: "array"; arrayItem: ColumnDescriptor }    // REQUIRED — kills defect 3
    | { kind: "enum"; enumValues: readonly string[] }   // REQUIRED
    | { kind: Exclude<ColKind, "array" | "enum"> }
  );

type ColumnDescriptorCommon = {
  label: string;
  description?: string;
  optional: boolean;
  display: DisplayDescriptor;        // = Exclude<DisplayConfig, { type: "custom" }>
  size?: number;
  hidden: boolean;
  enableHiding: boolean;
  hideHeader: boolean;
  resizable: boolean;                // ← was absent
  sortable: boolean;
  filter: FilterDescriptor | null;
  sheet: SheetDescriptor | null;
  /** Recorded at construction. Deletes `detectPreset`'s reverse-engineering. */
  provenance: Provenance;
};

export type FilterDescriptor = {
  type: FilterType;
  defaultOpen: boolean;
  commandDisabled: boolean;
  options?: Option[];
  min?: number;
  max?: number;
  unit?: string;                       // ← was dropped
  presets?: DatePresetDescriptor[];    // ← was dropped
};

export type Provenance =
  | { source: "manual" }
  | { source: "preset"; preset: string; args: readonly JsonValue[] }
  | { source: "inferred"; rule: string };

/** Everything that CANNOT survive JSON. Disjoint from ColumnDescriptor. */
export type ColRenderers = {
  cell?: (value: unknown, row: unknown) => JSX.Element | null;
  filterComponent?: (props: Option) => JSX.Element | null;
  sheetComponent?: (row: unknown) => JSX.Element | null | string;
  sheetCondition?: (row: unknown) => boolean;
};

export interface ColBuilder<T, F extends FilterType = FilterType> {
  /** @internal serializable half */   readonly _descriptor: ColumnDescriptor;
  /** @internal non-serializable half */ readonly _renderers: ColRenderers;
  // all existing fluent methods unchanged in signature
}

Serialization stops being hand-written:

export function serializeSchema(def: TableSchemaDefinition): SchemaJSON {
  return { version: 1, columns: Object.entries(def).map(([key, b]) => ({ key, ...b._descriptor })) };
}

export function deserializeSchema(json: SchemaJSON): TableSchemaDefinition {
  return Object.fromEntries(
    json.columns.map(({ key, ...descriptor }) => [key, createColBuilder(descriptor, {})]),
  );
}

serialize.ts drops from 270 lines to roughly 25.

Authoring API is unchanged

col.* and createTableSchema keep their exact current signatures — they are documented in skills/, AGENTS.md, and the docs site. .display("custom", { cell }) now writes the closure to _renderers.cell and leaves _descriptor.display at the kind's default, so the descriptor is already the correct serializable fallback.

Optional second stage: one traversal

The four generators take the same argument and are always called together at all four call sites (infinite/client.tsx:51-58, drizzle/client.tsx:41-48, builder-table.tsx:55-79, data-table-auto.tsx:41-69), each re-walking Object.entries(schema).

interface TableConfig<TRow, TState extends SchemaDefinition> {
  readonly definition: TableSchemaDefinition;
  readonly columns: ColumnDef<TRow>[];
  readonly filterFields: DataTableFilterField<TRow>[];
  readonly sheetFields: SheetField<TRow>[];
  readonly defaultColumnVisibility: VisibilityState;
  /** `state` merged BEFORE createSchema — defaults can never disagree with definition. */
  readonly state: Schema<TState>;
  readonly json: SchemaJSON;
}

tableSchema.build({ state?, extraColumns?, columnOverrides?, renderers? }): TableConfig<...>

This kills defect 7 by construction. Stage 1 and stage 2 should ship separately — stage 1 delivers defects 1-6, stage 2 delivers caller ergonomics. Do not fuse them.

What it hides

Descriptor→builder reconstruction including the enableHiding === false && hidden ⇒ .sheetOnly() heuristic (serialize.ts:245) — inference archaeology that vanishes when descriptors are stored verbatim. detectPreset's pattern-matching (to-typescript.ts:14-115), replaced by reading provenance. Three copies of defaultDisplayForKind. The getFilterFn mapping and the undocumented "you must register inDateRange/arrSome" contract (columns.tsx:26-27).

Dependency Strategy

In-process. This subsystem has no I/O — no DB, no network, no clock, no React in the serialization half. No new runtime dependencies; typescript is already a devDependency if an emitter-compile test is preferred over the new Function approach below.

Packaging note: types.ts currently ships in the data-table block (type-only) while the other 11 files ship in data-table-schema (registry.json). components/data-table/types.ts:1 imports SerializableDisplayConfig from it. Keep types.ts type-only so this split survives.

Testing Strategy

New boundary tests

  • Law A — round trip against the original, not against another projection:
    expect(deserializeSchema(serializeSchema(def))).toEqual(def) compared on descriptors directly. Meaningful only because the descriptor is now the complete serializable state.
  • Law B — compile-time totality. A type assertion that ColRenderers and ColumnDescriptorCommon share no keys, and that SchemaJSON's column type covers every ColumnDescriptor key. A field added to one and not the other fails tsc, not a test.
  • Law C — codegen round trip. The emitted chain is valid JavaScript (no type annotations), so this needs no new dependency:
    const body = schema.toTypeScript().replace(/^import .*\n/, "");
    const rebuilt = new Function("col", "createTableSchema", `${body}; return schema;`)(col, createTableSchema);
    expect(rebuilt.toJSON()).toEqual(schema.toJSON());
    Driven by inferSchemaFromJSON(realFixtureRows), not by hand-built descriptors. This is the test that catches defect 4.
  • Exhaustive emitter. Drive buildChain from a { [K in keyof ColumnDescriptorCommon]-?: Emitter<K> } map. -? plus full key coverage means adding a descriptor field without an emitter is a compile error. Catches defect 5.
  • The first generators/columns.tsx test: raw rows → createTableSchema.fromRows(rows).build() → assert a usable table config. This does not exist anywhere today and is the single highest-value item here.
  • Adversarial corpus: string[] with 11+ distinct values (defect 3), custom display on an enum column (defect 6), unit + presets + resizable in combination (defect 1).

Old tests to delete or rebase

  • serialize.test.ts:228's roundTrip helper and the 23 assertions built on it — replaced by Laws A/B.
  • col.test.ts (355 lines) — asserts on ._config, which this RFC splits in two. Migrate onto the public descriptor.
  • infer.test.ts (712 lines) — the ~14 word-list Set-membership cases characterize private heuristics. Keep a representative handful; the corpus-driven laws cover the rest.
  • to-typescript.test.ts (515 lines) — the 28 makeCol()-built toContain() assertions are superseded by Law C.

Test environment

None beyond vitest, already configured at packages/registry/vitest.config.ts.

Implementation Recommendations

What the module should own: the single definition of a column's serializable shape; the single definition of per-kind defaults; construction from three sources (builders, JSON, inferred rows); and the guarantee that JSON → builder → JSON is identity.

What it should hide: how descriptors are reconstructed; how presets are re-emitted (read provenance, never pattern-match); per-kind default resolution; the generator traversal.

What it should expose: col, createTableSchema (+ .fromJSON, .fromRows), definition, toJSON(), and a public read model to replace the seven external ._config reads (lib/drizzle/handler.ts:21, lib/ai/{context,detect,parse-response,diff-partial,output-schema}.ts, api/builder/data/helpers.ts:36). ._config being read across block boundaries while marked @internal is the honest limit of the encapsulation here — a public ResolvedColumn accessor is what makes it legitimate.

Migration:

  • SchemaJSON changes shape (arrayItemType → recursive arrayItem; new resizable, unit, presets, provenance, version). It is persisted by the builder (apps/web/src/app/api/builder/cache.ts) and pasted by users into the schema editor, so a migrateSchemaJSON(unknown): SchemaJSON v0→v1 path is mandatory. This reintroduces exactly one hand-written conversion, permanently — accept it knowingly.
  • createTableSchema.fromJSON should take unknown, not SchemaJSON. builder-client.tsx currently does JSON.parse(text) as SchemaJSON on user-typed text — an unchecked cast on untrusted input, and the largest hole in the current surface.
  • Land the public read model and migrate tests off ._config first, as a separate change, before touching serialization. Otherwise a large mechanical diff lands on top of a behavioral one.
  • Keep the four generate* functions exported as thin wrappers over the shared traversal so skills/data-table-filters/references/schema-api.md stays accurate and build() is purely additive.

Known residual loss: DatePreset is { from: Date, to: Date }, but presets are typically relative ("last hour") and computed at construction. Serializing them as ISO instants round-trips byte-exactly and satisfies defect 1, but a schema saved Monday and loaded Friday yields a stale range. The correct model is a relative descriptor ({ label, shortcut, duration: "PT1H" }), which is a larger change to DataTableFilterTimerange. Ship the ISO round-trip now; track the relative descriptor as follow-up. The important property is that the loss stops being silent.


🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions