fix(tables): address Bugbot round 2

Both findings are the same hand-listed-key pattern the round-1 fix removed
elsewhere, in code that round introduced.

`addTableColumn` built a new column from `options`/`multiple` plus whatever
`defaultMetadata` returned. `number` and `percent` deliberately declare no
`defaultMetadata` for `precision` (absent has to keep meaning "render as
stored"), so a precision the sidebar and the contract both accepted was dropped
before it reached the schema. It now carries every key the target type owns,
mirroring `buildConvertedColumn`.

Toggling `includeTime` off runs a migration that truncates every stored cell,
but the mutation treated the metadata path as schema-only and never invalidated
row data — the grid kept rendering pre-migration timestamps while filters and
exports saw the truncated values. Types now declare `metadataRewritesCells`,
the client-safe counterpart to the server registry's `migrateCellsForMetadata`,
and the hook reads it instead of naming keys.

Nothing but a test couples those two halves, so there is now one asserting a
type declares `metadataRewritesCells` exactly when it declares a migration.

Author: waleedlatif1

apps/sim/hooks/queries/tables.ts

@@ -102,6 +102,7 @@ import type {
   WorkflowGroupOutput,
 } from '@/lib/table'
 import { getColumnId } from '@/lib/table/column-keys'
+import { metadataKeysIn, metadataRewritesCells } from '@/lib/table/column-types'
 import { TABLE_LIMITS } from '@/lib/table/constants'
 import {
   areGroupDepsSatisfied,
@@ -1401,9 +1402,21 @@ export function useUpdateColumn({ workspaceId, tableId }: RowMutationContext) {
       // schema-only path would leave the cache holding pre-migration values,
       // which the grid hides but emptiness checks, filters, and dependent-group
       // eligibility still act on. Everything else really is metadata-only.
+      // `includeTime` going false truncates every stored cell in the migration
+      // the service runs, so the metadata path is NOT always schema-only. Which
+      // 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.
+      const updatedColumn = context?.previousDetail?.schema.columns.find(
+        (c) => getColumnId(c) === variables.columnName || c.name === variables.columnName
+      )
+      const { generic, dedicated } = metadataKeysIn(variables.updates)
       const rewritesRows =
         variables.updates.type !== undefined ||
         variables.updates.multiple !== undefined ||
+        (updatedColumn
+          ? metadataRewritesCells(updatedColumn, [...generic, ...dedicated])
+          : false) ||
         removesSelectOption(context?.previousDetail, variables)
       if (rewritesRows) invalidateTableSchema(queryClient, tableId)
       else invalidateTableSchemaOnly(queryClient, tableId)

apps/sim/lib/table/__tests__/column-type-registry.test.ts

@@ -21,6 +21,7 @@ import {
   ownersOfMetadataKey,
   TYPE_SPECIFIC_COLUMN_KEYS,
 } from '@/lib/table/column-types'
+import { metadataMigrationFor } from '@/lib/table/column-types/registry.server'
 import type { ColumnDefinition } from '@/lib/table/types'
 import { validateColumnDefinition } from '@/lib/table/validation'
 
@@ -113,6 +114,28 @@ describe('metadata key ownership', () => {
   })
 })
 
+describe('client/server registry coupling', () => {
+  it('declares metadataRewritesCells exactly when the server half migrates', () => {
+    // The two halves are coupled by nothing but this assertion. A type that
+    // migrates cells without declaring it leaves clients showing stale rows
+    // after a metadata edit; declaring it without a migration makes them
+    // refetch for nothing.
+    for (const definition of ALL_COLUMN_TYPES) {
+      const declares = (definition.metadataRewritesCells ?? []).length > 0
+      const migrates = metadataMigrationFor(definition.id) !== undefined
+      expect(declares, `${definition.id} declares=${declares} migrates=${migrates}`).toBe(migrates)
+    }
+  })
+
+  it('only names keys it actually owns as cell-rewriting', () => {
+    for (const definition of ALL_COLUMN_TYPES) {
+      for (const key of definition.metadataRewritesCells ?? []) {
+        expect(definition.ownedMetadata).toContain(key)
+      }
+    }
+  })
+})
+
 describe('conversion write-back', () => {
   // A retype is allowed exactly when the target's `coerce` accepts the value,
   // and `coerce` often TRANSFORMS it. The conversion must therefore write the

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

@@ -14,7 +14,11 @@ import {
   COLUMN_TYPE_REGISTRY,
   columnTypeById,
   isValueCompatible,
+  metadataRewritesCells,
+  metadataWithoutClears,
+  ownedKeysOf,
   ownersOfMetadataKey,
+  pickMetadata,
 } from '@/lib/table/column-types'
 import { coerceValue } from '@/lib/table/import'
 import { filterRulesToFilter } from '@/lib/table/query-builder/converters'
@@ -271,6 +275,27 @@ describe('review-round regressions', () => {
   })
 })
 
+describe('review-round-2 regressions', () => {
+  it("carries a create payload's precision onto the saved column", () => {
+    // `addTableColumn` named options/multiple explicitly and otherwise relied on
+    // `defaultMetadata`, which `number`/`percent` deliberately do not declare —
+    // so a precision accepted by the sidebar and the contract was dropped before
+    // it ever reached the schema.
+    const owned = ownedKeysOf('percent')
+    expect(owned).toContain('precision')
+    expect(metadataWithoutClears(pickMetadata({ precision: 2 }, owned))).toEqual({ precision: 2 })
+  })
+
+  it('marks a date-only toggle as rewriting cells so clients refetch rows', () => {
+    // The migration truncates every stored time server-side; a client that
+    // treated it as schema-only kept rendering pre-migration values.
+    expect(metadataRewritesCells(column('date'), ['includeTime'])).toBe(true)
+    // A purely presentational key must NOT force a row refetch.
+    expect(metadataRewritesCells(column('currency'), ['currencyCode'])).toBe(false)
+    expect(metadataRewritesCells(column('number'), ['precision'])).toBe(false)
+  })
+})
+
 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/date.ts

@@ -51,6 +51,8 @@ export const dateColumnType: ColumnTypeDefinition = {
   supportsUnique: true,
   sampleValue: '2024-01-31',
   ownedMetadata: ownedKeysOf('date'),
+  // Turning `includeTime` off truncates every stored cell.
+  metadataRewritesCells: ['includeTime'],
   workflowInputType: 'string',
   editor: 'date',
   expandable: false,

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

@@ -18,4 +18,9 @@ export type {
   ColumnTypeDefinition,
   TypeSpecificColumnKey,
 } from '@/lib/table/column-types/types'
-export { TYPE_SPECIFIC_COLUMN_KEYS } from '@/lib/table/column-types/types'
+export {
+  METADATA_KEY_OWNERS,
+  ownedKeysOf,
+  REQUIRED_METADATA_KEYS,
+  TYPE_SPECIFIC_COLUMN_KEYS,
+} from '@/lib/table/column-types/types'

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

@@ -223,6 +223,18 @@ export function metadataWithoutClears(patch: ColumnMetadataPatch): ColumnTypeMet
   return resolved
 }
 
+/**
+ * Whether writing `keys` on `column` rewrites its stored cells (rather than
+ * only how they render), so a client knows to invalidate cached row data.
+ */
+export function metadataRewritesCells(
+  column: ColumnDefinition,
+  keys: readonly TypeSpecificColumnKey[]
+): boolean {
+  const rewriting = columnTypeOf(column).metadataRewritesCells ?? []
+  return keys.some((key) => rewriting.includes(key))
+}
+
 /** Whether a column of `type` may carry `key`. */
 export function typeOwnsMetadataKey(type: string | undefined, key: TypeSpecificColumnKey): boolean {
   return columnTypeById(type).ownedMetadata.includes(key)

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

@@ -260,6 +260,21 @@ export interface ColumnTypeDefinition {
    */
   readonly genericMetadataUpdate?: readonly TypeSpecificColumnKey[]
 
+  /**
+   * Owned keys whose change REWRITES stored cells, not just how they render.
+   *
+   * The client-safe half of `migrateCellsForMetadata` (which lives in the
+   * server registry and cannot be imported here). Clients read it to decide
+   * whether a metadata edit invalidates cached ROW data as well as the schema:
+   * toggling a date column to date-only truncates every stored time
+   * server-side, and a grid still holding the pre-migration values disagrees
+   * with what filters and exports now see.
+   *
+   * `column-type-registry.test.ts` asserts this stays in step with the server
+   * registry, since nothing else couples the two halves.
+   */
+  readonly metadataRewritesCells?: readonly TypeSpecificColumnKey[]
+
   /** Workflow/block param type a column of this type maps onto. */
   readonly workflowInputType: 'string' | 'number' | 'boolean' | 'object'
 

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

@@ -20,6 +20,7 @@ import {
   columnTypeOf,
   isValueCompatible,
   metadataWithoutClears,
+  ownedKeysOf,
   pickMetadata,
   TYPE_SPECIFIC_COLUMN_KEYS,
   type TypeSpecificColumnKey,
@@ -110,6 +111,7 @@ export async function addTableColumn(
       )
     }
 
+    const definition = columnTypeById(column.type)
     const newColumn: TableSchema['columns'][number] = {
       // Honor a caller-provided id (undo of a delete reuses the original id);
       // otherwise mint a fresh one.
@@ -118,9 +120,13 @@ export async function addTableColumn(
       type: column.type as TableSchema['columns'][number]['type'],
       required: column.required ?? false,
       unique: column.unique ?? false,
-      ...(column.options ? { options: column.options } : {}),
-      ...(column.multiple ? { multiple: true } : {}),
-      ...columnTypeById(column.type).defaultMetadata?.(column as ColumnDefinition),
+      // Every key the TARGET type owns, then its defaults. Naming
+      // `options`/`multiple` here meant a key whose type declares no
+      // `defaultMetadata` was silently dropped on create — a `precision` was
+      // accepted by the sidebar and the contract and then never reached the
+      // saved schema. Mirrors `buildConvertedColumn`'s carry-back loop.
+      ...metadataWithoutClears(pickMetadata(column, ownedKeysOf(definition.id))),
+      ...definition.defaultMetadata?.(column as ColumnDefinition),
     }
 
     const columnValidation = validateColumnDefinition(newColumn)