fix(tables): final-audit findings

Backs out CSV email/phone inference. Inference reads a 100-row sample but the
write path coerces every row and NULLS what the column's type rejects, so a
contact export whose first 100 rows are clean and whose row 250 holds `n/a` had
that cell silently destroyed — where inferring `string` imported it verbatim.
The same hazard already exists for number/boolean/date and is accepted there;
it is materially likelier on contact data, which is exactly where these types
get used. Importing into a column the user typed themselves is unaffected, and
the reasoning is recorded so this is not re-added as an "improvement".

Clearing `includeTime` ran the irreversible truncation. The migration fired
whenever `target.includeTime` was falsy, and a cleared key is absent — the
tri-state's "legacy column, holds instants", the one state that must never be
truncated. It now requires an explicit transition to `false`. Reachable through
the copilot tool, which builds its patch from a loose arg bag.

A fractional "Decimal places" silently saved 0: `2.5` is finite, so the
`Number.isFinite` guard passed it to `clampPrecision`, which falls back to 0 for
a non-integer — the exact outcome the comment above it promised to prevent.

Phone substring filters compared a formatted fragment against punctuation-
stripped storage and matched nothing. Fragments are not whole values, so
`coerce` cannot validate them; types now declare `normalizeFilterFragment`.
A fragment with no digits is left alone — reducing it to `''` would turn
`ILIKE '%%'` into a match on every row.

Also: the retype's select branch was still two-state, so an option clear fell
back to the pre-conversion value; `onSettled`'s column lookup was case-sensitive
where `onMutate`'s is not, skipping row invalidation after a truncation;
`percent` rendered a non-numeric cell as blank, hiding its contents; `boolean`
and `select` declared `canonicalizesValues: false` despite both transforming,
and `select` declared itself orderable.

Renames `TableColumnError` to `TableRequestError` — `withLockedTable` and
`restoreTable` raise it too — and finishes the migration in the DELETE handlers
and v1 POST, which still carried substring lists. Those covered today's messages
but would have silently 500'd the next typed error added.

The `includeTime` toggle now warns before a save that would drop stored times;
the comment claiming it already did was false. Corrects two other stale docs:
the unknown-column range fallback is text, not numeric, and `precision` IS
lossy in CSV export even though storage keeps the full value.

Author: waleedlatif1

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

@@ -12,7 +12,7 @@ import { hybridAuthMockFns } from '@sim/testing'
 import { getErrorMessage } from '@sim/utils/errors'
 import { NextRequest } from 'next/server'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
-import { TableColumnError } from '@/lib/table/errors'
+import { TableRequestError } from '@/lib/table/errors'
 
 const {
   mockCheckAccess,
@@ -162,11 +162,11 @@ describe('PATCH /api/table/[tableId]/columns — pre-flight guards', () => {
     })
     // Stands in for the race the guards cannot close: the column stopped being
     // a currency between the snapshot the guards read and this write. The
-    // service raises `TableColumnError` for a caller-fixable failure, which is
+    // service raises `TableRequestError` for a caller-fixable failure, which is
     // what earns the 400 — a plain `Error` now means the server genuinely broke
     // and correctly returns 500.
     mockUpdateColumnMetadata.mockRejectedValue(
-      new TableColumnError('Cannot set currency on column "amount" of type "string"')
+      new TableRequestError('Cannot set currency on column "amount" of type "string"')
     )
 
     const response = await patch({ name: 'renamed', currencyCode: 'USD' })

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

@@ -22,13 +22,12 @@ import {
 import { columnMatchesRef, getColumnId } from '@/lib/table/column-keys'
 import { columnTypeById, metadataKeysIn, pickMetadata } from '@/lib/table/column-types'
 import { validateMetadataUpdate } from '@/lib/table/columns/metadata'
-import { TableColumnError } from '@/lib/table/errors'
+import { TableRequestError } from '@/lib/table/errors'
 import { signalTableSchemaChanged } from '@/lib/table/events'
 import {
   accessError,
   checkAccess,
   normalizeColumn,
-  rootErrorMessage,
   tableLockErrorResponse,
 } from '@/app/api/table/utils'
 
@@ -81,7 +80,7 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Colum
 
     // One typed check instead of a per-message substring list: the service
     // says whether a failure is the caller's and what status it deserves.
-    if (error instanceof TableColumnError) {
+    if (error instanceof TableRequestError) {
       return NextResponse.json({ error: error.message }, { status: error.status })
     }
 
@@ -300,7 +299,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
 
     // One typed check instead of a per-message substring list: the service
     // says whether a failure is the caller's and what status it deserves.
-    if (error instanceof TableColumnError) {
+    if (error instanceof TableRequestError) {
       return NextResponse.json({ error: error.message }, { status: error.status })
     }
 
@@ -354,12 +353,10 @@ export const DELETE = withRouteHandler(
         return validationErrorResponse(error, 'Invalid request data')
       }
 
-      const msg = rootErrorMessage(error)
-      if (msg.includes('not found') || msg === 'Table not found') {
-        return NextResponse.json({ error: msg }, { status: 404 })
-      }
-      if (msg.includes('Cannot delete') || msg.includes('last column')) {
-        return NextResponse.json({ error: msg }, { status: 400 })
+      // One typed check instead of a per-message substring list: the service
+      // says whether a failure is the caller's and what status it deserves.
+      if (error instanceof TableRequestError) {
+        return NextResponse.json({ error: error.message }, { status: error.status })
       }
 
       logger.error(`[${requestId}] Error deleting column from table ${tableId}:`, error)

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

@@ -430,7 +430,9 @@ describe('POST /api/table/[tableId]/import', () => {
       expect(mockImportAppendRows).toHaveBeenCalledTimes(1)
       // Inferred as `email`, not `string`: the fixture values are real
       // addresses, and CSV inference now recognises them.
-      expect(appendAdditions()).toEqual([expect.objectContaining({ name: 'email', type: 'email' })])
+      expect(appendAdditions()).toEqual([
+        expect.objectContaining({ name: 'email', type: 'string' }),
+      ])
       // Existing columns have no id (legacy) → keyed by name; the new `email`
       // column was assigned id `col_deadbeefcafef00d` (mocked generateId).
       expect(appendRows()).toEqual([
@@ -473,7 +475,7 @@ describe('POST /api/table/[tableId]/import', () => {
       )
       expect(response.status).toBe(200)
       expect(appendAdditions()).toEqual([
-        expect.objectContaining({ name: 'Email_2', type: 'email' }),
+        expect.objectContaining({ name: 'Email_2', type: 'string' }),
       ])
     })
 
@@ -539,7 +541,9 @@ describe('POST /api/table/[tableId]/import', () => {
       // Route forwarded the column addition into the (now atomic) import op.
       // Inferred as `email`, not `string`: the fixture values are real
       // addresses, and CSV inference now recognises them.
-      expect(appendAdditions()).toEqual([expect.objectContaining({ name: 'email', type: 'email' })])
+      expect(appendAdditions()).toEqual([
+        expect.objectContaining({ name: 'email', type: 'string' }),
+      ])
       expect(response.status).toBe(400)
       const data = await response.json()
       expect(data.success).toBeUndefined()

apps/sim/app/api/v1/tables/[tableId]/columns/route.ts

@@ -21,7 +21,7 @@ import {
 import { columnMatchesRef, getColumnId } from '@/lib/table/column-keys'
 import { columnTypeById, metadataKeysIn, pickMetadata } from '@/lib/table/column-types'
 import { validateMetadataUpdate } from '@/lib/table/columns/metadata'
-import { TableColumnError } from '@/lib/table/errors'
+import { TableRequestError } from '@/lib/table/errors'
 import { signalTableSchemaChanged } from '@/lib/table/events'
 import {
   accessError,
@@ -104,21 +104,10 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Colum
     const validationResponse = v1ValidationErrorResponseFromError(error)
     if (validationResponse) return validationResponse
 
-    if (error instanceof Error) {
-      // Same caller-error set the internal columns route maps — an invalid
-      // select option set is a bad request, not a server fault.
-      if (
-        error.message.includes('already exists') ||
-        error.message.includes('maximum column') ||
-        error.message.includes('Invalid column') ||
-        error.message.includes('exceeds maximum') ||
-        error.message.includes('option')
-      ) {
-        return NextResponse.json({ error: error.message }, { status: 400 })
-      }
-      if (error.message === 'Table not found') {
-        return NextResponse.json({ error: error.message }, { status: 404 })
-      }
+    // One typed check instead of a per-message substring list: the service
+    // says whether a failure is the caller's and what status it deserves.
+    if (error instanceof TableRequestError) {
+      return NextResponse.json({ error: error.message }, { status: error.status })
     }
 
     logger.error(`[${requestId}] Error adding column to table:`, error)
@@ -353,7 +342,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
 
     // One typed check instead of a per-message substring list: the service
     // says whether a failure is the caller's and what status it deserves.
-    if (error instanceof TableColumnError) {
+    if (error instanceof TableRequestError) {
       return NextResponse.json({ error: error.message }, { status: error.status })
     }
 
@@ -424,13 +413,10 @@ export const DELETE = withRouteHandler(
       const validationResponse = v1ValidationErrorResponseFromError(error)
       if (validationResponse) return validationResponse
 
-      if (error instanceof Error) {
-        if (error.message.includes('not found') || error.message === 'Table not found') {
-          return NextResponse.json({ error: error.message }, { status: 404 })
-        }
-        if (error.message.includes('Cannot delete') || error.message.includes('last column')) {
-          return NextResponse.json({ error: error.message }, { status: 400 })
-        }
+      // One typed check instead of a per-message substring list: the service
+      // says whether a failure is the caller's and what status it deserves.
+      if (error instanceof TableRequestError) {
+        return NextResponse.json({ error: error.message }, { status: error.status })
       }
 
       logger.error(`[${requestId}] Error deleting column from table:`, error)

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-config-sidebar/column-config-sidebar.tsx

@@ -172,12 +172,13 @@ function ColumnConfigBody({
   const wantsCurrency = typeOwnsMetadataKey(typeInput, 'currencyCode')
   const wantsPrecision = typeOwnsMetadataKey(typeInput, 'precision')
   // `undefined` means "no precision declared" — the field is legitimately
-  // clearable back to rendering values as stored. A non-empty value that is not
-  // a finite number is treated the same rather than clamping to 0, so garbage
-  // never silently saves as "zero decimal places".
+  // clearable back to rendering values as stored. Anything that is not a whole
+  // number is treated the same rather than clamped, because `clampPrecision`
+  // falls back to 0 for a non-integer: `2.5` is finite, so a `Number.isFinite`
+  // guard let it through and silently saved "zero decimal places".
   const precisionNumber = Number(precisionInput)
   const parsedPrecision =
-    precisionInput.trim() === '' || !Number.isFinite(precisionNumber)
+    precisionInput.trim() === '' || !Number.isInteger(precisionNumber)
       ? undefined
       : clampPrecision(precisionNumber)
   const wantsIncludeTime = typeOwnsMetadataKey(typeInput, 'includeTime')
@@ -392,13 +393,23 @@ function ColumnConfigBody({
         {wantsIncludeTime && (
           <>
             <FieldDivider />
-            <div className='flex items-center justify-between pl-0.5'>
-              <Label htmlFor='column-sidebar-include-time'>Include time</Label>
-              <Switch
-                id='column-sidebar-include-time'
-                checked={includeTimeInput}
-                onCheckedChange={(v) => setIncludeTimeInput(!!v)}
-              />
+            <div className='flex flex-col gap-[9px]'>
+              <div className='flex items-center justify-between pl-0.5'>
+                <Label htmlFor='column-sidebar-include-time'>Include time</Label>
+                <Switch
+                  id='column-sidebar-include-time'
+                  checked={includeTimeInput}
+                  onCheckedChange={(v) => setIncludeTimeInput(!!v)}
+                />
+              </div>
+              {/* Turning this off rewrites every cell and cannot be undone by
+                  turning it back on — the times are gone. Shown only when the
+                  save would actually perform that rewrite. */}
+              {baselineIncludeTime && !includeTimeInput && (
+                <p className='pl-0.5 text-[var(--text-error)] text-caption'>
+                  Saving will remove the time from every cell in this column. This can’t be undone.
+                </p>
+              )}
             </div>
           </>
         )}

apps/sim/hooks/queries/tables.ts

@@ -1407,8 +1407,12 @@ export function useUpdateColumn({ workspaceId, tableId }: RowMutationContext) {
       // keys do that is declared by the type (`metadataRewritesCells`) rather
       // than listed here, so a future key that rewrites cells invalidates rows
       // without an edit at this call site.
+      // Case-insensitive on the NAME, matching `onMutate`'s lookup. A
+      // mismatched lookup here silently skips the row invalidation an
+      // `includeTime` truncation needs, leaving the grid on pre-migration values.
+      const settledLower = variables.columnName.toLowerCase()
       const updatedColumn = context?.previousDetail?.schema.columns.find(
-        (c) => getColumnId(c) === variables.columnName || c.name === variables.columnName
+        (c) => getColumnId(c) === variables.columnName || c.name.toLowerCase() === settledLower
       )
       const { generic, dedicated } = metadataKeysIn(variables.updates)
       const rewritesRows =

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

@@ -19,8 +19,9 @@ import {
   ownersOfMetadataKey,
   pickMetadata,
 } from '@/lib/table/column-types'
+import { metadataMigrationFor } from '@/lib/table/column-types/registry.server'
 import { buildConvertedColumn } from '@/lib/table/columns/service'
-import { coerceValue, inferColumnType } from '@/lib/table/import'
+import { coerceValue } from '@/lib/table/import'
 import { filterRulesToFilter, prunePredicateForColumns } from '@/lib/table/query-builder/converters'
 import type { ColumnDefinition } from '@/lib/table/types'
 
@@ -426,20 +427,86 @@ describe('follow-up fixes', () => {
       expect(neg.ok && neg.value).toBe(-2.5)
     }
   })
+})
 
-  it('infers an email column from a CSV, using the type’s own validation', () => {
-    expect(inferColumnType(['ada@example.com', 'bob@example.co.uk'])).toBe('email')
-    expect(inferColumnType(['ada@example.com', 'not an address'])).toBe('string')
+describe('final-audit regressions', () => {
+  it('never truncates a legacy date column when includeTime is CLEARED', async () => {
+    // Absent is the tri-state's "legacy column, holds instants". Gating the
+    // migration on `target.includeTime` being falsy also fired for a cleared
+    // key, destroying every stored time of day.
+    const migrate = metadataMigrationFor('date')
+    expect(migrate).toBeDefined()
+    if (!migrate) return
+    const ran: string[] = []
+    const trx = {
+      execute: async () => {
+        ran.push('rewrote')
+      },
+    } as never
+
+    const call = (previous: ColumnDefinition, target: ColumnDefinition) =>
+      migrate({ trx, tableId: 't', columnKey: 'c', previous, target, resolved: new Map() })
+
+    // Cleared (absent) on a legacy column: must NOT rewrite.
+    await call({ name: 'c', type: 'date' }, { name: 'c', type: 'date' })
+    expect(ran).toHaveLength(0)
+    // Cleared from an explicit true: must NOT rewrite either.
+    await call({ name: 'c', type: 'date', includeTime: true }, { name: 'c', type: 'date' })
+    expect(ran).toHaveLength(0)
+    // The real transition to date-only DOES rewrite.
+    await call(
+      { name: 'c', type: 'date', includeTime: true },
+      { name: 'c', type: 'date', includeTime: false }
+    )
+    expect(ran.length).toBeGreaterThan(0)
   })
 
-  it('infers phone only on strong evidence, never from bare digits or dashes', () => {
-    expect(inferColumnType(['+1 555 123 4567', '+44 20 7123 4567'])).toBe('phone')
-    expect(inferColumnType(['(555) 123-4567', '(555) 987-6543'])).toBe('phone')
-    // An ID column of bare digits is a Number, which is the right answer.
-    expect(inferColumnType(['5551234567', '5559876543'])).toBe('number')
-    // Dash-separated digit runs are ISBNs and SKUs as often as phone numbers,
-    // and a phone column keeps only the digits — so these stay text.
-    expect(inferColumnType(['978-0-13-235088-4', '978-0-32-135668-0'])).toBe('string')
+  it('keeps a select option clear through a retype, like every other key', () => {
+    const from: ColumnDefinition = {
+      name: 'c',
+      type: 'select',
+      options: [{ id: 'o1', name: 'One' }],
+    }
+    // Silence carries the old options across.
+    const carried = buildConvertedColumn(
+      from,
+      { tableId: 't', columnName: 'c', newType: 'select' },
+      { isSelectType: true, targetMultiple: false }
+    )
+    expect(carried.options).toEqual([{ id: 'o1', name: 'One' }])
+    // An explicit clear does not silently restore them.
+    const cleared = buildConvertedColumn(
+      from,
+      { tableId: 't', columnName: 'c', newType: 'select', options: null },
+      { isSelectType: true, targetMultiple: false }
+    )
+    expect(cleared.options).toBeUndefined()
+  })
+
+  it('shows a non-numeric percent cell rather than hiding it', () => {
+    // Returning '' made a drifted cell look empty, which reads as data loss.
+    expect(COLUMN_TYPE_REGISTRY.percent.formatForDisplay('n/a', column('percent'))).toBe('n/a')
+  })
+
+  it('declares canonicalization for boolean and select, which both transform', () => {
+    expect(COLUMN_TYPE_REGISTRY.boolean.canonicalizesValues).toBe(true)
+    expect(COLUMN_TYPE_REGISTRY.select.canonicalizesValues).toBe(true)
+    // Range comparison on opaque option ids is meaningless.
+    expect(COLUMN_TYPE_REGISTRY.select.orderable).toBe(false)
+  })
+
+  it('normalizes a phone SEARCH FRAGMENT so a substring filter can meet the stored value', () => {
+    const contains = (value: string) =>
+      filterRulesToFilter(
+        [{ id: 'r1', logicalOperator: 'and' as const, column: 'c', operator: 'contains', value }],
+        [{ id: 'c', name: 'c', type: 'phone' }]
+      )
+    // Stored as +442071234567; a formatted fragment must be stripped to meet it.
+    expect(contains('+44 20 7123')).toEqual({ c: { $contains: '+44207123' } })
+    expect(contains('(555) 123')).toEqual({ c: { $contains: '555123' } })
+    // A fragment with NO digits is left alone — reducing it to '' would make
+    // ILIKE '%%' match every row.
+    expect(contains('abc')).toEqual({ c: { $contains: 'abc' } })
   })
 })
 

apps/sim/lib/table/column-types/boolean.ts

@@ -7,7 +7,9 @@ export const booleanColumnType: ColumnTypeDefinition = {
   label: 'Boolean',
   icon: TypeBoolean,
   jsonbCast: null,
-  canonicalizesValues: false,
+  // `coerce` folds case and trims (' True ' -> true), so a filter operand has
+  // to go through it too.
+  canonicalizesValues: true,
   orderable: false,
   storesOpaqueIds: false,
   supportsUnique: true,

apps/sim/lib/table/column-types/percent.ts

@@ -60,7 +60,9 @@ export const percentColumnType: ColumnTypeDefinition = {
   },
 
   formatForDisplay(value, column) {
-    if (typeof value !== 'number') return ''
+    // A non-number renders as itself, matching `number`. Returning '' hid the
+    // cell's actual contents, which reads as data loss rather than as drift.
+    if (typeof value !== 'number') return String(value ?? '')
     return `${formatWithPrecision(value, column.precision)}%`
   },
 

apps/sim/lib/table/column-types/phone.ts

@@ -55,6 +55,21 @@ function normalizePhone(raw: string): string | null {
   return `${hasPlus ? '+' : ''}${digits}`
 }
 
+/**
+ * Strips a search fragment the same way a stored number was stripped, so a
+ * substring filter can meet it.
+ *
+ * A fragment with NO digits is returned untouched. Reducing it to `''` would
+ * make the resulting `ILIKE '%%'` match every row in the table — turning a
+ * filter that should find nothing into one that finds everything.
+ */
+function normalizePhoneFragment(fragment: string): string {
+  const trimmed = fragment.trim()
+  const digits = trimmed.replace(/\D/g, '')
+  if (digits === '') return fragment
+  return `${trimmed.startsWith('+') ? '+' : ''}${digits}`
+}
+
 export const phoneColumnType: ColumnTypeDefinition = {
   id: 'phone',
   label: 'Phone',
@@ -74,6 +89,8 @@ export const phoneColumnType: ColumnTypeDefinition = {
   typeaheadPattern: /[\d+\s\-().]/,
   parseErrorMessage: 'Invalid phone number',
 
+  normalizeFilterFragment: normalizePhoneFragment,
+
   coerce(value) {
     // A number reaches here from a CSV whose phone column was read as numeric.
     if (typeof value === 'number') {