refactor(tables): mint column ids via generateId (uuid), drop collision-check plumbing

Author: TheodoreSpeaks

apps/sim/app/api/table/[tableId]/import/route.test.ts

@@ -397,10 +397,10 @@ describe('POST /api/table/[tableId]/import', () => {
         expect.objectContaining({ name: 'email', type: 'string' }),
       ])
       // Existing columns have no id (legacy) → keyed by name; the new `email`
-      // column was assigned id `col_short-id` (mocked generateShortId).
+      // column was assigned id `col_deadbeefcafef00d` (mocked generateId).
       expect(appendRows()).toEqual([
-        { name: 'Alice', age: 30, 'col_short-id': 'a@x.io' },
-        { name: 'Bob', age: 40, 'col_short-id': 'b@x.io' },
+        { name: 'Alice', age: 30, col_deadbeefcafef00d: 'a@x.io' },
+        { name: 'Bob', age: 40, col_deadbeefcafef00d: 'b@x.io' },
       ])
     })
 

apps/sim/app/api/table/[tableId]/import/route.ts

@@ -22,7 +22,6 @@ import {
   type CsvHeaderMapping,
   CsvImportValidationError,
   coerceRowsForTable,
-  collectColumnIds,
   createCsvParser,
   dispatchAfterBatchInsert,
   generateColumnId,
@@ -193,7 +192,6 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
       }
 
       const usedNames = new Set(table.schema.columns.map((c) => c.name.toLowerCase()))
-      const takenIds = new Set(collectColumnIds(table.schema))
       const updatedMapping: CsvHeaderMapping = { ...effectiveMapping }
       const newColumns: TableSchema['columns'] = []
 
@@ -209,8 +207,7 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro
         const inferredType = inferColumnType(rows.map((r) => r[header]))
         // Pre-assign the id so the prospective schema (used to coerce rows) and
         // the persisted column (created in importAppendRows) share the same key.
-        const id = generateColumnId(takenIds)
-        takenIds.add(id)
+        const id = generateColumnId()
         additions.push({ id, name: columnName, type: inferredType })
         newColumns.push({
           id,

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

@@ -3,18 +3,17 @@
  */
 import { describe, expect, it, vi } from 'vitest'
 
-const { mockGenerateShortId } = vi.hoisted(() => ({
-  mockGenerateShortId: vi.fn(),
+const { mockGenerateId } = vi.hoisted(() => ({
+  mockGenerateId: vi.fn(),
 }))
 
 vi.mock('@sim/utils/id', () => ({
-  generateShortId: mockGenerateShortId,
+  generateId: mockGenerateId,
 }))
 
 import {
   buildIdByName,
   buildNameById,
-  collectColumnIds,
   filterNamesToIds,
   generateColumnId,
   getColumnId,
@@ -36,22 +35,15 @@ describe('getColumnId', () => {
 })
 
 describe('generateColumnId', () => {
-  it('mints a col_-prefixed id not already taken', () => {
-    let n = 0
-    mockGenerateShortId.mockImplementation(() => `gen${n++}`)
-    expect(generateColumnId([])).toBe('col_gen0')
+  it('mints a col_-prefixed id with the uuid dashes stripped', () => {
+    mockGenerateId.mockReturnValue('11111111-2222-4333-8444-555566667777')
+    expect(generateColumnId()).toBe('col_11111111222243338444555566667777')
   })
 
-  it('skips collisions against existing ids', () => {
-    let n = 0
-    mockGenerateShortId.mockImplementation(() => ['dup', 'dup', 'fresh'][n++] ?? 'x')
-    expect(generateColumnId(['col_dup'])).toBe('col_fresh')
-  })
-
-  it('falls back to a counter suffix when the RNG is fully deterministic', () => {
-    mockGenerateShortId.mockReturnValue('fixed')
-    // 'col_fixed' is taken and the RNG never changes → counter fallback.
-    expect(generateColumnId(['col_fixed'])).toBe('col_fixed_1')
+  it('produces an id that satisfies NAME_PATTERN (valid JSONB key / filter field)', () => {
+    mockGenerateId.mockReturnValue('0a1b2c3d-4e5f-4607-8809-0a1b2c3d4e5f')
+    // Must start with a letter/underscore and contain only [a-z0-9_].
+    expect(generateColumnId()).toMatch(/^[a-z_][a-z0-9_]*$/i)
   })
 })
 
@@ -69,9 +61,6 @@ describe('name ↔ id maps', () => {
   it('buildNameById maps storage id → display name', () => {
     expect(Object.fromEntries(buildNameById(schema))).toEqual({ col_1: 'email', age: 'age' })
   })
-  it('collectColumnIds resolves every column', () => {
-    expect(collectColumnIds(schema)).toEqual(['col_1', 'age'])
-  })
 })
 
 describe('row data translation', () => {
@@ -124,7 +113,7 @@ describe('filter / sort translation', () => {
 
 describe('withGeneratedColumnIds', () => {
   it('stamps ids on id-less columns and remaps group refs name → id', () => {
-    mockGenerateShortId.mockReturnValueOnce('a').mockReturnValueOnce('b')
+    mockGenerateId.mockReturnValueOnce('a').mockReturnValueOnce('b')
     const schema: TableSchema = {
       columns: [
         { name: 'email', type: 'string', workflowGroupId: 'g1' },

apps/sim/lib/table/column-keys.ts

@@ -8,16 +8,9 @@
  * map builders here.
  */
 
-import { generateShortId } from '@sim/utils/id'
+import { generateId } from '@sim/utils/id'
 import type { ColumnDefinition, Filter, RowData, Sort, TableSchema, WorkflowGroup } from './types'
 
-/**
- * Alphanumeric alphabet (no `-`) so a generated `col_…` id satisfies
- * `NAME_PATTERN` — filter/sort field validation runs on the id, and the id is
- * embedded as a JSONB key, so it must contain only safe characters.
- */
-const COLUMN_ID_ALPHABET = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'
-
 /**
  * Resolves a column's stable storage key. Falls back to `name` for legacy
  * columns that predate the id backfill — those rows were written keyed by name,
@@ -28,32 +21,16 @@ export function getColumnId(col: Pick<ColumnDefinition, 'id' | 'name'>): string
 }
 
 /**
- * Mints a fresh, collision-free column id. Generated ids are opaque (`col_…`)
- * and deliberately distinct from display names so renames never disturb them.
- * Checked against `existingIds` (which includes legacy name-as-id values) so a
- * generated id can never alias an existing column.
+ * Mints a fresh column id. Generated ids are opaque (`col_<uuid>`) and
+ * deliberately distinct from display names so renames never disturb them. The
+ * `col_` prefix is required: the id is validated against `NAME_PATTERN` (it's a
+ * JSONB key and a filter/sort field) which must start with a letter/underscore,
+ * and a bare UUID can start with a digit. Dashes are stripped for the same
+ * reason. A v4 UUID's 122 random bits make a collision within a table's columns
+ * effectively impossible, so no uniqueness check is needed.
  */
-export function generateColumnId(existingIds: Iterable<string>): string {
-  const taken = new Set(existingIds)
-  const mint = () => `col_${generateShortId(16, COLUMN_ID_ALPHABET)}`
-  for (let i = 0; i < 100; i++) {
-    const id = mint()
-    if (!taken.has(id)) return id
-  }
-  // Deterministic-RNG fallback (e.g. a fixed-value `generateShortId` mock in
-  // tests): append a counter so a batch of column adds can't collide/loop.
-  let n = 1
-  let id = `${mint()}_${n}`
-  while (taken.has(id)) {
-    n++
-    id = `${mint()}_${n}`
-  }
-  return id
-}
-
-/** All current column ids in a schema (resolved via `getColumnId`). */
-export function collectColumnIds(schema: TableSchema): string[] {
-  return schema.columns.map(getColumnId)
+export function generateColumnId(): string {
+  return `col_${generateId().replace(/-/g, '')}`
 }
 
 /**
@@ -75,15 +52,13 @@ export function columnMatchesRef(col: ColumnDefinition, ref: string): boolean {
  * already carry an id.
  */
 export function withGeneratedColumnIds(schema: TableSchema): TableSchema {
-  const taken = new Set(schema.columns.map(getColumnId))
   const idByName = new Map<string, string>()
   const columns = schema.columns.map((col) => {
     if (col.id) {
       idByName.set(col.name, col.id)
       return col
     }
-    const id = generateColumnId(taken)
-    taken.add(id)
+    const id = generateColumnId()
     idByName.set(col.name, id)
     return { ...col, id }
   })

apps/sim/lib/table/service.ts

@@ -38,7 +38,6 @@ import { generateRestoreName } from '@/lib/core/utils/restore-name'
 import type { DbOrTx } from '@/lib/db/types'
 import { materializeExecutionData } from '@/lib/logs/execution/trace-store'
 import {
-  collectColumnIds,
   columnMatchesRef,
   generateColumnId,
   getColumnId,
@@ -585,7 +584,7 @@ export async function addTableColumn(
     const newColumn: TableSchema['columns'][number] = {
       // Honor a caller-provided id (undo of a delete reuses the original id);
       // otherwise mint a fresh one.
-      id: column.id ?? generateColumnId(collectColumnIds(schema)),
+      id: column.id ?? generateColumnId(),
       name: column.name,
       type: column.type as TableSchema['columns'][number]['type'],
       required: column.required ?? false,
@@ -662,7 +661,6 @@ export async function addTableColumnsWithTx(
   if (columns.length === 0) return table
 
   const usedNames = new Set(table.schema.columns.map((c) => c.name.toLowerCase()))
-  const takenIds = new Set(collectColumnIds(table.schema))
   const additions: TableSchema['columns'] = []
 
   for (const column of columns) {
@@ -688,8 +686,7 @@ export async function addTableColumnsWithTx(
     usedNames.add(lower)
     // Honor a caller-assigned id (the CSV append path pre-assigns so coercion
     // and persistence agree); otherwise mint one.
-    const id = column.id ?? generateColumnId(takenIds)
-    takenIds.add(id)
+    const id = column.id ?? generateColumnId()
     additions.push({
       id,
       name: column.name,
@@ -3932,13 +3929,9 @@ export async function addWorkflowGroup(
     // Assign stable ids to the new output columns, then rewrite the group's
     // column refs from name → id so outputs/deps/inputMappings key on ids —
     // matching the row-data storage key and surviving future renames.
-    const takenIds = new Set(collectColumnIds(schema))
-    const outputColumns = data.outputColumns.map((col) => {
-      if (col.id) return col
-      const id = generateColumnId(takenIds)
-      takenIds.add(id)
-      return { ...col, id }
-    })
+    const outputColumns = data.outputColumns.map((col) =>
+      col.id ? col : { ...col, id: generateColumnId() }
+    )
     const updatedColumns = [...schema.columns, ...outputColumns]
     const idByName = new Map(updatedColumns.map((c) => [c.name, getColumnId(c)]))
     const group = remapGroupColumnRefs(data.group, idByName)
@@ -4083,13 +4076,9 @@ export async function updateWorkflowGroup(
       // row-data storage key). New output columns get ids first; then output
       // `columnName`, deps, input mappings, and mapping-update targets are
       // remapped name → id. Callers that already pass ids are unaffected.
-      const takenIds = new Set(collectColumnIds(schema))
-      const newColDefs = (data.newOutputColumns ?? []).map((col) => {
-        if (col.id) return col
-        const id = generateColumnId(takenIds)
-        takenIds.add(id)
-        return { ...col, id }
-      })
+      const newColDefs = (data.newOutputColumns ?? []).map((col) =>
+        col.id ? col : { ...col, id: generateColumnId() }
+      )
       const idByName = new Map(
         [...schema.columns, ...newColDefs].map((c) => [c.name, getColumnId(c)])
       )
@@ -4474,7 +4463,7 @@ export async function addWorkflowGroupOutput(
     }
 
     const newColDef: ColumnDefinition = {
-      id: generateColumnId(collectColumnIds(schema)),
+      id: generateColumnId(),
       name: columnName,
       type: newColumnType,
       required: false,