fix(tables): normalize contact filter values, keep clears through a retype

Greptile's P1: an equality filter on an Email or Phone column matched nothing.
`parseFilterScalar` gated type-aware parsing on `jsonbCast !== null`, but that
asks how the comparison is PERFORMED, not whether the stored form differs from
what the user typed. Email and Phone canonicalize on write and compare as text,
so they were skipped — a filter for `Ada@Example.com` was accepted and then
silently missed the stored `ada@example.com`, likewise `020 1234 5678` against
`+442071234567`. Types now declare `canonicalizesValues`, which is the question
the filter actually needs answered.

Second: a type change carrying `precision: null` dropped the clear. The routes
stripped nulls before `updateColumnType`, and `buildConvertedColumn` carries
un-supplied keys forward from the old column — so the stripped null read as
"not mentioned" and restored the very setting the user had cleared. Clears now
reach the retype, where absent and cleared are finally distinguishable.
`metadataWithoutClears` keeps serving the create and validate paths, which have
nothing to remove; its comment claiming the retype rebuilt from scratch was
simply wrong.

`buildConvertedColumn` is exported for the test, matching the three helpers this
module already exports for the same reason.

Author: waleedlatif1

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

@@ -20,12 +20,7 @@ import {
   updateColumnType,
 } from '@/lib/table'
 import { columnMatchesRef, getColumnId } from '@/lib/table/column-keys'
-import {
-  columnTypeById,
-  metadataKeysIn,
-  metadataWithoutClears,
-  pickMetadata,
-} from '@/lib/table/column-types'
+import { columnTypeById, metadataKeysIn, pickMetadata } from '@/lib/table/column-types'
 import { validateMetadataUpdate } from '@/lib/table/columns/metadata'
 import { signalTableSchemaChanged } from '@/lib/table/events'
 import {
@@ -220,10 +215,10 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
           newType: updates.type as NonNullable<typeof updates.type>,
           // Every type-specific key the payload carries, whichever writer would
           // own it standalone: a conversion applies its target's metadata in the
-          // same transaction rather than leaving it to a second write.
-          ...metadataWithoutClears(
-            pickMetadata(updates, [...genericMetadataKeys, ...dedicatedMetadataKeys])
-          ),
+          // same transaction rather than leaving it to a second write. Clears
+          // are forwarded as `null` — stripping them here made
+          // `buildConvertedColumn` fall back to the pre-conversion value.
+          ...pickMetadata(updates, [...genericMetadataKeys, ...dedicatedMetadataKeys]),
           // Forwarded so the conversion validates against the constraint this
           // same request is about to set, not the column's current one.
           ...(updates.required !== undefined ? { required: updates.required } : {}),

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

@@ -19,12 +19,7 @@ import {
   updateColumnType,
 } from '@/lib/table'
 import { columnMatchesRef, getColumnId } from '@/lib/table/column-keys'
-import {
-  columnTypeById,
-  metadataKeysIn,
-  metadataWithoutClears,
-  pickMetadata,
-} from '@/lib/table/column-types'
+import { columnTypeById, metadataKeysIn, pickMetadata } from '@/lib/table/column-types'
 import { validateMetadataUpdate } from '@/lib/table/columns/metadata'
 import { signalTableSchemaChanged } from '@/lib/table/events'
 import {
@@ -254,10 +249,10 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
           newType: updates.type as NonNullable<typeof updates.type>,
           // Every type-specific key the payload carries, whichever writer would
           // own it standalone: a conversion applies its target's metadata in the
-          // same transaction rather than leaving it to a second write.
-          ...metadataWithoutClears(
-            pickMetadata(updates, [...genericMetadataKeys, ...dedicatedMetadataKeys])
-          ),
+          // same transaction rather than leaving it to a second write. Clears
+          // are forwarded as `null` — stripping them here made
+          // `buildConvertedColumn` fall back to the pre-conversion value.
+          ...pickMetadata(updates, [...genericMetadataKeys, ...dedicatedMetadataKeys]),
           // Forwarded so the conversion validates against the constraint this
           // same request is about to set, not the column's current one.
           ...(updates.required !== undefined ? { required: updates.required } : {}),

apps/sim/lib/copilot/tools/server/table/user-table.ts

@@ -37,7 +37,6 @@ import { columnTypeForLeaf, deriveOutputColumnName } from '@/lib/table/column-na
 import {
   columnTypeById,
   metadataKeysIn,
-  metadataWithoutClears,
   pickMetadata,
   TYPE_SPECIFIC_COLUMN_KEYS,
   validateTypeMetadata,
@@ -1788,7 +1787,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
                 newType: newType as (typeof COLUMN_TYPES)[number],
                 options,
                 multiple,
-                ...metadataWithoutClears(pickMetadata(metadataUpdates, genericMetadataKeys)),
+                ...pickMetadata(metadataUpdates, genericMetadataKeys),
                 ...(uniqFlag !== undefined ? { unique: uniqFlag } : {}),
               },
               requestId

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

@@ -19,6 +19,7 @@ import {
   ownersOfMetadataKey,
   pickMetadata,
 } from '@/lib/table/column-types'
+import { buildConvertedColumn } from '@/lib/table/columns/service'
 import { coerceValue } from '@/lib/table/import'
 import { filterRulesToFilter, prunePredicateForColumns } from '@/lib/table/query-builder/converters'
 import type { ColumnDefinition } from '@/lib/table/types'
@@ -324,6 +325,90 @@ describe('review-round-4 regressions', () => {
   })
 })
 
+describe('review-round-7 regressions', () => {
+  it('normalizes a contact filter value so it can match the stored form', () => {
+    // Email/Phone canonicalize on write but compare as TEXT, so the earlier
+    // `jsonbCast !== null` gate skipped them: the filter was accepted and then
+    // matched nothing, because JSONB containment saw `Ada@Example.com` against
+    // a stored `ada@example.com`.
+    const eq = (value: string, col: ColumnDefinition) =>
+      filterRulesToFilter(
+        [{ id: 'r1', logicalOperator: 'and' as const, column: 'c', operator: 'eq', value }],
+        [{ ...col, id: 'c' }]
+      )
+    expect(eq('Ada@Example.COM', column('email'))).toEqual({ c: 'ada@example.com' })
+    expect(eq('+44 20 7123 4567', column('phone'))).toEqual({ c: '+442071234567' })
+    expect(eq('(555) 123-4567', column('phone'))).toEqual({ c: '5551234567' })
+  })
+
+  it('leaves a pass-through type on its existing filter coercion', () => {
+    const eq = (value: string, col: ColumnDefinition) =>
+      filterRulesToFilter(
+        [{ id: 'r1', logicalOperator: 'and' as const, column: 'c', operator: 'eq', value }],
+        [{ ...col, id: 'c' }]
+      )
+    expect(eq('123', column('string'))).toEqual({ c: 123 })
+    expect(eq('1', column('select', { options: [{ id: '1', name: 'One' }] }))).toEqual({ c: '1' })
+  })
+
+  it('declares canonicalization only where coerce actually transforms', () => {
+    // The property stated directly: a canonicalizing type turns a valid
+    // non-canonical input into something DIFFERENT, and a pass-through type
+    // returns it unchanged. Asserting the behaviour rather than the flag is
+    // what stops the two drifting.
+    const transforms: Array<[ColumnDefinition['type'], string, JsonValue]> = [
+      ['email', 'Ada@Example.COM', 'ada@example.com'],
+      ['phone', '(555) 123-4567', '5551234567'],
+      ['currency', '$1,234.56', 1234.56],
+      ['number', '42', 42],
+      ['percent', '50%', 50],
+      ['date', '01/15/2024', '2024-01-15'],
+    ]
+    for (const [type, input, expected] of transforms) {
+      expect(COLUMN_TYPE_REGISTRY[type].canonicalizesValues, type).toBe(true)
+      const result = COLUMN_TYPE_REGISTRY[type].coerce(input, column(type))
+      expect(result.ok && result.value, `${type} <- ${input}`).toEqual(expected)
+    }
+
+    for (const type of ['string', 'json'] as const) {
+      expect(COLUMN_TYPE_REGISTRY[type].canonicalizesValues, type).toBe(false)
+    }
+    // A pass-through type hands the value straight back.
+    const passed = COLUMN_TYPE_REGISTRY.string.coerce('Ada@Example.COM', column('string'))
+    expect(passed.ok && passed.value).toBe('Ada@Example.COM')
+  })
+
+  it('keeps an explicit metadata clear through a type conversion', () => {
+    // `buildConvertedColumn` carries UN-SUPPLIED keys forward from the old
+    // column, so an explicit clear must be distinguishable from silence. When
+    // the route stripped the null first, this read as "not mentioned" and
+    // restored the precision the user had just cleared.
+    const from: ColumnDefinition = { name: 'c', type: 'percent', precision: 2 }
+    const cleared = buildConvertedColumn(
+      from,
+      { tableId: 't', columnName: 'c', newType: 'number', precision: null },
+      { isSelectType: false, targetMultiple: false }
+    )
+    expect(cleared.precision).toBeUndefined()
+
+    // Silence still carries the old value across the conversion.
+    const carried = buildConvertedColumn(
+      from,
+      { tableId: 't', columnName: 'c', newType: 'number' },
+      { isSelectType: false, targetMultiple: false }
+    )
+    expect(carried.precision).toBe(2)
+
+    // And an explicit new value still wins.
+    const replaced = buildConvertedColumn(
+      from,
+      { tableId: 't', columnName: 'c', newType: 'number', precision: 4 },
+      { isSelectType: false, targetMultiple: false }
+    )
+    expect(replaced.precision).toBe(4)
+  })
+})
+
 describe('date includeTime', () => {
   it('truncates to a calendar day only when includeTime is explicitly false', () => {
     const dateOnly = column('date', { includeTime: false })

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

@@ -7,6 +7,7 @@ export const booleanColumnType: ColumnTypeDefinition = {
   label: 'Boolean',
   icon: TypeBoolean,
   jsonbCast: null,
+  canonicalizesValues: false,
   orderable: false,
   storesOpaqueIds: false,
   supportsUnique: true,

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

@@ -14,6 +14,7 @@ export const currencyColumnType: ColumnTypeDefinition = {
   label: 'Currency',
   icon: TypeCurrency,
   jsonbCast: 'numeric',
+  canonicalizesValues: true,
   orderable: true,
   storesOpaqueIds: false,
   supportsUnique: true,

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

@@ -46,6 +46,7 @@ export const dateColumnType: ColumnTypeDefinition = {
   label: 'Date',
   icon: CalendarIcon,
   jsonbCast: 'timestamptz',
+  canonicalizesValues: true,
   orderable: true,
   storesOpaqueIds: false,
   supportsUnique: true,

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

@@ -77,6 +77,7 @@ export const emailColumnType: ColumnTypeDefinition = {
   icon: TypeEmail,
   // Stored as plain text; comparison and sorting are lexical, like `string`.
   jsonbCast: null,
+  canonicalizesValues: true,
   orderable: true,
   storesOpaqueIds: false,
   supportsUnique: true,

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

@@ -7,6 +7,7 @@ export const jsonColumnType: ColumnTypeDefinition = {
   label: 'JSON',
   icon: TypeJson,
   jsonbCast: null,
+  canonicalizesValues: false,
   orderable: false,
   storesOpaqueIds: false,
   supportsUnique: true,

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

@@ -8,6 +8,7 @@ export const numberColumnType: ColumnTypeDefinition = {
   label: 'Number',
   icon: TypeNumber,
   jsonbCast: 'numeric',
+  canonicalizesValues: true,
   orderable: true,
   storesOpaqueIds: false,
   supportsUnique: true,