fix(tables): stop defaultMetadata overwriting caller-supplied column metadata

The previous commit's fix was defeated by its own last line. `defaultMetadata`
is spread AFTER the caller's metadata, and it was handed a bare
`{ name, type }` — so it re-answered from nothing and overwrote the values it
was meant to fall back to. An imported date column's `includeTime: true` became
`false`: the rows were coerced WITH their times while the column was saved
claiming to be date-only.

It now sees what the caller supplied, so `column.includeTime ?? false` falls
back only when the key is genuinely absent — the same shape `addTableColumn`
already had.

The builder is extracted as `buildAddedColumn` and exported, for the reason
this slipped through: the earlier test asserted what INFERENCE returns, not
what gets persisted, and the overwrite happens between the two. Testing the
persisted shape needed a transaction otherwise. Mirrors `buildConvertedColumn`.

Author: waleedlatif1

apps/sim/lib/table/__tests__/column-types-contact.test.ts

@@ -23,6 +23,7 @@ import { metadataMigrationFor } from '@/lib/table/column-types/registry.server'
 import { buildConvertedColumn } from '@/lib/table/columns/service'
 import { coerceValue, inferSchemaFromCsv } from '@/lib/table/import'
 import { filterRulesToFilter, prunePredicateForColumns } from '@/lib/table/query-builder/converters'
+import { buildAddedColumn } from '@/lib/table/service'
 import { buildFilterClause } from '@/lib/table/sql'
 import type { ColumnDefinition } from '@/lib/table/types'
 
@@ -604,6 +605,25 @@ describe('imported column metadata', () => {
     expect(String(coerced)).toContain('14:30')
   })
 
+  it('PERSISTS the inferred includeTime instead of re-defaulting over it', () => {
+    // The gap that let a bug through: the test above only checked what
+    // inference RETURNS. `defaultMetadata` runs last when the column is built
+    // for storage, so handed a bare `{ name, type }` it re-answered from
+    // nothing and overwrote the inferred `true` with `false` — rows coerced
+    // with times, column saved as date-only.
+    const { columns } = inferSchemaFromCsv(['when'], [{ when: '2024-01-15T14:30' }])
+    const stored = buildAddedColumn(columns[0], 'col_1')
+    expect(stored.type).toBe('date')
+    expect(stored.includeTime).toBe(true)
+  })
+
+  it('still applies a type default the caller did not supply', () => {
+    const stored = buildAddedColumn({ name: 'd', type: 'date' }, 'col_2')
+    expect(stored.includeTime).toBe(false)
+    const money = buildAddedColumn({ name: 'm', type: 'currency' }, 'col_3')
+    expect(money.currencyCode).toBeDefined()
+  })
+
   it('leaves a non-date inferred column without date metadata', () => {
     const { columns } = inferSchemaFromCsv(['n'], [{ n: '1' }, { n: '2' }])
     expect(columns[0].type).toBe('number')

apps/sim/lib/table/service.ts

@@ -469,6 +469,47 @@ export async function createTable(
  * Use this when composing a column addition with other writes (e.g., row
  * inserts) that must succeed or roll back together.
  */
+/**
+ * The column definition a bulk add persists.
+ *
+ * Extracted and exported so the metadata carry-through is testable without a
+ * transaction — the same reason `buildConvertedColumn` is exported from the
+ * column service.
+ */
+export function buildAddedColumn(
+  column: ColumnTypeMetadata & {
+    id?: string
+    name: string
+    type: string
+    required?: boolean
+    unique?: boolean
+  },
+  id: string
+): TableSchema['columns'][number] {
+  const type = column.type as TableSchema['columns'][number]['type']
+  // A create has nothing to clear, so resolve any nulls away first.
+  const supplied = metadataWithoutClears(pickMetadata(column, ownedKeysOf(type)))
+  return {
+    id,
+    name: column.name,
+    type,
+    required: column.required ?? false,
+    unique: column.unique ?? false,
+    // Carry the type's own metadata, mirroring `addTableColumn`. Building the
+    // column from the five fields above alone dropped everything else the
+    // caller supplied — an imported date column lost its `includeTime` and
+    // landed in the "predates the key" state, where the grid and coercion treat
+    // a column created moments ago as a legacy instant column.
+    ...supplied,
+    // `defaultMetadata` must SEE what the caller supplied. Handed a bare
+    // `{ name, type }` it re-answers from nothing and, spread last, overwrites
+    // the very values above — an imported date column's `includeTime: true`
+    // became `false`, so its rows were coerced WITH times while the column
+    // claimed to be date-only.
+    ...columnTypeById(type).defaultMetadata?.({ ...supplied, name: column.name, type }),
+  }
+}
+
 export async function addTableColumnsWithTx(
   trx: DbTransaction,
   table: TableDefinition,
@@ -514,22 +555,7 @@ export async function addTableColumnsWithTx(
     // Honor a caller-assigned id (the CSV append path pre-assigns so coercion
     // and persistence agree); otherwise mint one.
     const id = column.id ?? generateColumnId()
-    const type = column.type as TableSchema['columns'][number]['type']
-    additions.push({
-      id,
-      name: column.name,
-      type,
-      required: column.required ?? false,
-      unique: column.unique ?? false,
-      // Carry the type's own metadata, mirroring `addTableColumn`. Building the
-      // column from these five fields alone dropped everything else the caller
-      // supplied — an imported date column lost its `includeTime` and landed in
-      // the "predates the key" state, where the grid and coercion treat it as a
-      // legacy instant column rather than as what the import decided.
-      // A create has nothing to clear, so resolve any nulls away.
-      ...metadataWithoutClears(pickMetadata(column, ownedKeysOf(type))),
-      ...columnTypeById(type).defaultMetadata?.({ name: column.name, type }),
-    })
+    additions.push(buildAddedColumn(column, id))
   }
 
   if (table.schema.columns.length + additions.length > TABLE_LIMITS.MAX_COLUMNS_PER_TABLE) {