fix(tables): validate constraints after the migrations that rewrite cells

Self-caught while reviewing my own previous commit, which introduced both.

`updateColumnOptions` ran the shared `applyConstraints` BEFORE its cell
migrations. Those migrations rewrite stored values — a single<->multi toggle
changes the shape, removing an option clears cells — so a `unique` scan read
the pre-migration values, passed, and the rewrite could then produce the
duplicates the scan was meant to prevent. Moved to after the migrations, which
is where `updateColumnType` already had it.

The same commit also left the options path running `required`'s empty-cell
check twice: once in the shared helper and once in its original inline block,
whose comment still described a separate constraint write that no longer runs.
Removed the duplicate — one query, one rule, which is the whole point of the
shared helper.

Also routes the options path through `persistColumns` like the others.

Author: waleedlatif1

apps/sim/lib/table/__tests__/column-conversion.test.ts

@@ -5,6 +5,7 @@
  * coverage and every case below was a shipped defect caught in review.
  */
 import { describe, expect, it } from 'vitest'
+import { COLUMN_TYPE_REGISTRY } from '@/lib/table/column-types'
 import {
   applyPendingRename,
   isValueCompatibleWithType,
@@ -231,3 +232,27 @@ describe('select accepts scalar cells', () => {
     expect(resolveSelectOptionId({ a: 1 } as never, NUMERIC_OPTIONS)).toBeNull()
   })
 })
+
+describe('constraint validation reads post-migration values', () => {
+  // The ordering that matters: `updateColumnOptions` rewrites stored cells (a
+  // single<->multi toggle changes the shape; removing an option clears cells),
+  // and `updateColumnType` rewrites them through the coercion write-back. A
+  // `unique` scan run BEFORE those would read values that no longer exist by
+  // the time the constraint is persisted, pass, and let the rewrite produce the
+  // duplicates it was supposed to prevent.
+  //
+  // The coercion case is the concrete one: two distinct strings can collapse to
+  // one number.
+  it('shows how a conversion manufactures duplicates the pre-scan cannot see', () => {
+    const column: ColumnDefinition = { name: 'sku', type: 'number' }
+    const before = ['5', '5.0']
+    expect(new Set(before).size).toBe(2)
+
+    const after = before.map((value) => {
+      const coerced = COLUMN_TYPE_REGISTRY.number.coerce(value, column)
+      return coerced.ok ? coerced.value : value
+    })
+    expect(after).toEqual([5, 5])
+    expect(new Set(after).size).toBe(1)
+  })
+})

apps/sim/lib/table/columns/service.ts

@@ -1014,39 +1014,16 @@ export async function updateColumnOptions(
       throw new Error(`Invalid column: ${columnValidation.errors.join('; ')}`)
     }
 
-    const constrainedColumn = await applyConstraints(
-      trx,
-      data.tableId,
-      updatedColumn,
-      columnKey,
-      data
-    )
-    const withOptions = schema.columns.map((c, i) => (i === columnIndex ? constrainedColumn : c))
-    const updatedColumns = withOptions.map((c, i) =>
-      i === columnIndex ? applyPendingRename(withOptions, columnIndex, data.newName) : c
-    )
-    const updatedSchema: TableSchema = { ...schema, columns: updatedColumns }
-    const now = new Date()
-
     const nextMultiple = !!(data.multiple ?? column.multiple)
     const wasMultiple = !!column.multiple
     const keptIds = new Set(data.options.map((o) => o.id))
     const removedAny = (column.options ?? []).some((o) => !keptIds.has(o.id))
     const togglingCardinality = nextMultiple !== wasMultiple
+    // The constraint the column ENDS UP with, which may be arriving in this same
+    // request. `applyConstraints` validates and applies it below, after the cell
+    // migrations; the checks in between need to read the target value.
     const targetRequired = !!(data.required ?? column.required)
 
-    // Newly imposing `required` in the same request: rows that are ALREADY empty
-    // would fail the separate constraint write after this one commits, so they
-    // have to be caught here, through the same predicate that write will use.
-    if (targetRequired && !column.required) {
-      const emptyCount = await countEmptyCells(trx, data.tableId, columnKey)
-      if (emptyCount > 0) {
-        throw new Error(
-          `Cannot make column "${column.name}" required: ${emptyCount} row(s) have null, missing, or empty values. Fill them first.`
-        )
-      }
-    }
-
     if (togglingCardinality || removedAny) {
       const timeoutMs = scaledStatementTimeoutMs(table.rowCount ?? 0, {
         baseMs: 60_000,
@@ -1133,16 +1110,29 @@ export async function updateColumnOptions(
       })
     }
 
-    await trx
-      .update(userTableDefinitions)
-      .set({ schema: updatedSchema, updatedAt: now })
-      .where(eq(userTableDefinitions.id, data.tableId))
+    // Constraints are validated and applied AFTER the migrations above, because
+    // those migrations rewrite stored values — a `unique` scan run before them
+    // would read the pre-migration shape and pass, and the migration could then
+    // produce the duplicates it was meant to prevent.
+    const constrainedColumn = await applyConstraints(
+      trx,
+      data.tableId,
+      updatedColumn,
+      columnKey,
+      data
+    )
+    const withOptions = schema.columns.map((c, i) => (i === columnIndex ? constrainedColumn : c))
+    const updatedColumns = withOptions.map((c, i) =>
+      i === columnIndex ? applyPendingRename(withOptions, columnIndex, data.newName) : c
+    )
+
+    const updated = await persistColumns(trx, table, updatedColumns)
 
     logger.info(
       `[${requestId}] Updated options for column "${column.name}" in table ${data.tableId}`
     )
 
-    return { ...table, schema: updatedSchema, updatedAt: now }
+    return updated
   })
 }