feat(tables): add email and phone column types by waleedlatif1 · Pull Request #6172 · simstudioai/sim

waleedlatif1 · 2026-08-01T19:47:14Z

Summary

Adds five column types — Email, Phone, URL, Percent, Duration — plus decimal places on Number/Percent, a date-only toggle, and colors on select options
Percent and Duration store bare numbers, so > 50% and >= 1h stay ordinary numeric comparisons rather than string ones
Email/Phone/URL normalize on write (lower-cased addresses, E.164-stripped numbers, absolute http(s) links) so the same value entered two ways matches
Collapses the per-metadata-key update plumbing into one registry-driven writer: adding a key was ~9 near-identical edits across contracts, service, both routes, the copilot tool, the sidebar, undo and import — none compiler-enforced. The add-column-type skill listed this as its known gap; the skill is updated since the gap is now closed

Fixes found while auditing this change

Range filters on any text-shaped type 500'd the rows query. buildComparisonClause fell through to ?? 'numeric' for every type whose jsonbCast is null but whose id isn't literally string, emitting (data->>'email')::numeric — a Postgres error on the first non-numeric row. Types now declare orderable and a null cast means text comparison for all of them
Date truncation assumed a four-digit year. 0001-01-01T00:00:00Z (.NET's DateTime.MinValue, routine in exported CSVs) normalized to an unpadded 1-01-01T00:00:00Z; years are padded at the source and truncation is anchored on a YYYY-MM-DD match. The set-based migration got the same guard — legacy columns can hold unparseable strings like March 5, 2024, which left(…, 10) would have rewritten irreversibly
A default precision rounded new Percent columns in the grid and in CSV export (12.5 exporting as 13%), and rode back onto Number through a number → percent → number round-trip
Also: URL read example.com:8080's host as a scheme and nulled the cell; Duration rewrote 90.7 to 91 when an editor merely opened and closed; the color picker put raw buttons inside a role="menu", so it never closed on select and had no keyboard navigation

Consolidation

The v2 predicate grammar had a private copy of the select operator allowlist, so a restricted type was gated on one wire shape and not the other — it now asks the registry
Sort-by-label, filter value coercion, filter pruning, the filter UI and the grid's dirty check read storesOpaqueIds / storesMultipleValues instead of naming select
The expanded popover formats every type through formatForDisplay rather than only date and currency (a Duration cell read 5400 there while the grid read 1:30:00)
json's three separate type === 'json' branches became one editor variant
formatValueForInput takes the column instead of synthesizing one and dropping its metadata

Type of Change

New feature

Testing

Typecheck clean; 2311 tests passing across lib/table, lib/api, the table routes, the copilot table tool, the tables UI and hooks. New regression tests were each verified to fail with their fix reverted. All repo gates pass (lint:check, check:api-validation + strict, check:client-boundary, check:boundaries, check:react-query, check:utils, check:realtime-prune, icon checks).

Not yet exercised in the running app — worth a pass over create/edit/paste/filter/sort/convert/export/undo on a table with one column of each type before merging, since includeTime and the select-color change touch stored data.

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

vercel · 2026-08-01T19:47:20Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Aug 2, 2026 2:58am

cursor · 2026-08-01T19:47:24Z

PR Summary

High Risk
Changes core column mutation, validation, and import/write coercion paths used by every table API and the grid; metadata edits like includeTime: false can rewrite stored cell data, and error-mapping changes affect all table HTTP surfaces.

Overview
This PR centralizes column metadata on the type registry: ownership lives in METADATA_KEY_OWNERS, updates go through updateColumnMetadata (replacing per-key writers like updateColumnCurrency), and HTTP/v1 column routes route patches via metadataKeysIn + validateMetadataUpdate instead of hard-coded currency checks.

API behavior shifts from substring-matched error messages to TableRequestError with explicit status codes, plus tableRequestErrorResponse on table create/import routes so append failures (column cap, invalid type) return 400/404 instead of generic 500s.

Grid and sidebar UI gate controls with typeOwnsMetadataKey / registry helpers (storesMultipleValues, describeColumnType, columnTypeOf(column).display) rather than column.type === 'select'. Select options gain per-option colors (picker + pill rendering). Column config adds decimal places and include time for date columns, with a warning when turning off time rewrites cells.

Client write path uses full ColumnDefinition in formatValueForInput and editor-keyed cleanCellValue; optimistic updates clear metadata keys when the API sends null, and row refetch follows metadataRewritesCells when metadata edits migrate stored values.

Import aligns inferred date columns via inferredColumnDefinition across append/import paths; coerceValue default arm falls back to registry coerce with safer nulling on typed casts.

Docs, add-column-type skills, and backward-compatibility tests lock legacy bare columns (no new metadata keys) to prior filter/sort/coerce/export behavior.

^{Reviewed by Cursor Bugbot for commit 9dc14c1. Configure here.}

waleedlatif1 · 2026-08-01T20:27:20Z

@greptile

waleedlatif1 · 2026-08-01T20:27:21Z

@cursor review

waleedlatif1 · 2026-08-01T20:49:54Z

@greptile

waleedlatif1 · 2026-08-01T20:49:55Z

@cursor review

waleedlatif1 · 2026-08-01T20:53:12Z

@greptile

waleedlatif1 · 2026-08-01T20:53:13Z

@cursor review

waleedlatif1 · 2026-08-01T20:57:29Z

@greptile

waleedlatif1 · 2026-08-01T20:57:30Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit c624e50. Configure here.}

waleedlatif1 · 2026-08-01T21:00:47Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 9edd338. Configure here.}

waleedlatif1 · 2026-08-01T23:41:41Z

@cursor review

waleedlatif1 · 2026-08-01T23:43:20Z

@greptile review

greptile-apps · 2026-08-01T23:46:20Z

Greptile Summary

The PR adds Email, Phone, URL, Percent, and Duration table columns while consolidating type behavior and metadata updates behind the column registry.

Normalizes contact and URL values consistently across writes and filters.
Adds precision, date-only, and select-color metadata support across APIs, imports, undo, and UI.
Centralizes filtering, rendering, conversion, and metadata behavior in column-type definitions.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported Email and Phone equality mismatch is corrected through type-aware coercion, and the formatted Phone text-filter mismatch is corrected in both client conversion and server-side SQL compilation.

Important Files Changed

Filename	Overview
apps/sim/lib/table/query-builder/converters.ts	Filter operands are now normalized according to column storage semantics, including canonical Email and Phone equality values and formatted Phone fragments.
apps/sim/lib/table/sql.ts	Server-side predicate compilation provides the authoritative operand canonicalization path when callers do not have column definitions.
apps/sim/lib/table/column-types/phone.ts	Defines validated phone storage normalization and a separate permissive fragment normalizer for text searches.
apps/sim/lib/table/tests/column-types-contact.test.ts	Covers canonical contact equality filters, formatted Phone text filters, and server-side normalization when client schema context is absent.
apps/sim/lib/table/column-types/types.ts	Expands the registry contract with storage, filtering, rendering, and metadata capabilities used throughout the table stack.

_{Reviews (11): Last reviewed commit: "fix(tables): map typed service errors in..." | Re-trigger Greptile}

Retype's stale-work guard hardcoded options/multiple/currencyCode, so a request carrying `precision` or `includeTime` against an already-correct type hit the rename-only path and was silently discarded while reporting success — the exact hand-listed-key bug this PR exists to remove. It reads TYPE_SPECIFIC_COLUMN_KEYS now. Filter values are read through the column that owns them. `parseScalar` returns NaN for `50%`, `1h` and `$1,234.56`, so the value stayed a string, met the numeric cast, and the range filter was rejected outright. Only cast columns take this path — a text column keeps parseScalar's existing coercion and a select keeps its option id verbatim. CSV import was the one write path that bypassed `applyIncludeTime`, so an import could put an instant into a column whose schema says it holds calendar days. Decimal places is now clearable: the update sent nothing when the field was emptied, so a precision could be set but never unset. `null` on the update contract removes a key, applied in `updateColumnMetadata` and mirrored in the optimistic cache. Non-finite input no longer clamps to 0 and saves as "zero decimal places". Filter pruning also moved off `type === 'select'` onto storesOpaqueIds / storesMultipleValues, which the earlier sweep had missed.

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.

CI's tool-registry boundary check caught this. Importing SELECT_OPTION_COLORS as a VALUE from the `@/lib/table` barrel turned what had been a type-only import in a client component into a real runtime edge, and that barrel reaches the executor — pulling the executable tool registry (~4,700 modules) into the chat, home and tables route graphs. Deep-imported from `@/lib/table/types`, which is a leaf: its only other imports are type-only. The barrel stays fine for `import type`.

`prunePredicateForColumns` guards a saved predicate against operators a select column no longer accepts, but it checked the legacy UI set. The two grammars are deliberately not 1:1 — `isNull`/`isNotNull` are meaningful on a select column and have no `$` equivalent — so a saved v2 predicate using either was stripped client-side before the rows query ran, silently widening the filter rather than narrowing it. It now asks `predicateOperatorsFor`, the same registry answer the SQL leaf uses, so the pruner and the query can no longer disagree about what is allowed. Its legacy sibling stays on the UI set, which is correctly aligned with the `$` grammar's own allowlist.

Converting a column to Date silently opted it into times while creating one gave date-only. The seed read `existingColumn?.includeTime !== false`, and on a column that is not yet a date that key is simply absent — so the expression answered `true`, and the save sent `includeTime: true` alongside the type change. Only an existing date column now answers from its own value (absent there means a column predating the key, which does hold instants). Every other starting point takes the date-only default a new date column gets. The seed and the dirty-check read one `baselineIncludeTime` so they cannot answer differently.

Removes the `url` and `duration` types entirely — type files, icons, registry entries, tests and docs. `percent` stays, and with it the shared `precision` key that also gives Number columns decimal places. The remaining two now validate properly rather than permissively. Email was `/^[^\s@]+@[^\s@.]+(\.[^\s@.]+)+$/`, which accepted `a..b@x.com`, `.a@x.com`, `a.@x.com`, `a,b@x.com` (a LIST of addresses, not one), `a@-x.com`, `a@x-.com`, `a@x.123`, and an address of unbounded length. It now checks an RFC 5322 dot-atom local part, real DNS labels, an alphabetic TLD, and the RFC 5321 length caps (254 total, 64 local, 253 domain, 63 per label). Quoted local parts stay unsupported, deliberately: admitting them would mean carrying quoting rules through case-folding and every downstream comparison. Phone accepted `+0123456789` — an E.164 number whose country code starts with zero, which no network can route. A leading `+` now requires a country code starting 1-9, while a number WITHOUT one keeps its leading zero, since that is a real national trunk prefix (UK 020, DE 030). Whether the country code exists is deliberately not checked: that needs a table that goes stale and then starts rejecting valid numbers. Every new rule is covered, and each was verified to fail with the old loose rule restored — 12 email cases and the phone one.

…etype 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.

Three follow-ups from the residual audit. Routes decided 400-vs-500 by matching substrings of the error message, a list that had to grow with every new message and silently mis-classified anything it missed. The metadata-ownership error ("Cannot set precision on column …") matched nothing and returned a 500, telling the caller the server broke when their request was invalid. The service now raises `TableColumnError` carrying its own status; all 38 throws in the column service and the shared "Table not found" are migrated, and both routes replace the lists with one typed check. An untyped throw now correctly means a genuine internal error. CSV import never inferred Email or Phone, so a contacts file arrived as text. Email is inferred through the type's OWN coerce, so inference and the write path cannot disagree. Phone is deliberately inferred only on strong evidence — every value carrying a `+` or a parenthesised area code — because ISBNs, SKUs and part numbers are dash-separated digit runs too, and a phone column keeps only the digits, so a wrong guess drops their punctuation silently. A column of bare digits still infers Number, which is right for an ID column. `Number()` accepts far more than a decimal number: `0x10` read as 16, `0b11` as 3, `Infinity` as infinity. Someone typing `0x10` means the text, so Number and Percent now parse through a shared decimal parser. Exponent notation stays accepted — spreadsheets export it — while the alternate bases are refused.

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.

…known Greptile was right that the earlier fix was incomplete. Canonicalization lived in the UI-facing converters, which only run it when the caller supplies column definitions — and the workflow Table blocks build a filter from user input long before any schema exists, so they call those converters with none. A workflow filtering an Email column for `Ada@Example.com`, or a Phone column for `020 1234 5678`, compiled an operand that never met the stored value and silently returned no rows. Moved to `fieldPredicate`, which is the one place every filter arrives with its column in hand: both wire grammars compile through it, so raw API callers and the copilot tool are covered too. The converter pass stays as a convenience for the filter UI rather than as the guarantee. Only TEXT operands are canonicalized. `date.coerce` accepts an epoch number, so running a number through it would silently reinterpret `{ birthDate: { $gte: 1704067200000 } }` as a date instead of letting the range validator reject a likely mistake. Opaque ids are skipped — they are resolved from option names upstream, where the option set is known — and an operand the type rejects is left alone rather than nulled, so the error still names what the user typed. Also routes CSV number coercion AND number inference through the same decimal parser inline edits use. A CSV could import `0x10` as 16 where typing it was rejected; had only coercion been fixed, a hex column would have inferred `number` and then nulled every cell.

A backward-compatibility check on the whole change turned this up. Making the range-comparison cast registry-driven fixed the real bug — a text-shaped type emitting `(data->>'email')::numeric` and 500ing the rows query — but it also swept in fields with NO schema entry, which resolve to `string` through `columnTypeById`'s fallback and so began comparing lexicographically. Ad-hoc numeric fields have always compared numerically there, and `'10' > '5'` is false as text, so a saved filter on a field that is not in the schema (a typo, or a column since removed) would quietly return a different row set with no error at all. An absent column type is now distinguished from a known text type: unknown keeps `::numeric` and its operand validation exactly as before, while a known type still answers from the registry. Both cases are pinned by tests.

`addTableColumnsWithTx` built each column from five fields and dropped everything else the caller supplied, so an imported date column landed with `includeTime` absent — the "predates the key" state — and coercion and the grid then treated a column created moments ago as a legacy instant column. It now carries the type's owned keys and defaults, the same way `addTableColumn` does. An inferred date column is stamped `includeTime: true` rather than taking the sidebar's date-only default. The pattern that infers `date` accepts `2024-01-15T14:30`, so the file may genuinely contain times, and defaulting to date-only would truncate every one of them on write. Creating a Date column by hand still defaults to date-only: there the user sees the toggle and there is no data to lose yet.

…metadata The previous commit's fix was defeated by its own last line. `defaultMetadata` is spread AFTER the caller's metadata, and it was handed a bare `{ name, type }` — so it re-answered from nothing and overwrote the values it was meant to fall back to. An imported date column's `includeTime: true` became `false`: the rows were coerced WITH their times while the column was saved claiming to be date-only. It now sees what the caller supplied, so `column.includeTime ?? false` falls back only when the key is genuinely absent — the same shape `addTableColumn` already had. The builder is extracted as `buildAddedColumn` and exported, for the reason this slipped through: the earlier test asserted what INFERENCE returns, not what gets persisted, and the overwrite happens between the two. Testing the persisted shape needed a transaction otherwise. Mirrors `buildConvertedColumn`.

The copilot tool's retype passed `options`/`multiple` from separately normalized variables while the HTTP routes spread both the generic AND dedicated key sets. `normalizeSelectOptionsInput` answers `undefined` for a non-array, so an agent sending `options: null` reached the conversion as "not mentioned" and it carried the stale option set forward, where the route would have cleared it. It now spreads both sets from the same patch, which already holds the normalized options. Adds `backward-compatibility.test.ts`, which pins what an EXISTING table does rather than testing the new code. The consolidation rewrote how every consumer answers per-type questions — casts, coercion, filter operands, sort, display, import — and each of those fails SILENTLY when wrong: a wrong cast errors a filter, a wrong operand returns the wrong rows, a wrong coercion nulls a cell on the next write. 51 cases across the seven pre-existing types: bare definitions still validate without the new metadata keys, legacy cell values coerce identically, a legacy date column still stores full instants, a number column with no `precision` still renders as stored, filter and sort SQL emit the same casts, an unknown field keeps `::numeric`, a select option id is never coerced, and CSV inference still refuses to guess email or phone. Verified the harness actually bites by reverting three fixes in turn — the unknown-column fallback, the legacy-date reading, and email inference — each of which fails it.

The bulk column-add path kept throwing plain `Error` after the single-column path was migrated, so an invalid column type or the column cap fell past the import route's substring list and surfaced as a 500 with the message swallowed ("Failed to import CSV") — the caller never learned their header was invalid. All 13 remaining throws in the table service are now typed: a missing table is 404, everything else 400. The import route and both table-create routes check the type before their substring lists, which stay only for messages raised by the CSV parser, which is not table-aware. Extracts `buildAddedColumns` so the batch validation is testable without a transaction, mirroring `buildAddedColumn`, and pins the typing rather than the message — this is the third error-status gap found on this PR, and each came from a list that had to be remembered. Also pins that message text and `instanceof Error` are unchanged, since the runners, the copilot tool and sibling routes still match on text.

waleedlatif1 · 2026-08-02T02:23:26Z

@greptile

waleedlatif1 · 2026-08-02T02:23:27Z

@cursor review

The append route and the import runner each had their own inline copy of "infer a type, build a column", so the `includeTime: true` added to the create-from-CSV path never reached them. An appended date column was persisted DATE-ONLY while its rows were coerced WITH their times — the schema and the data disagreeing about the same column, which then shows up as wrong display, wrong filters, and truncation on the next write. Fixing the third copy in place would have left a fourth: the import runner had the same inline call. The decision now lives in `inferredColumnDefinition`, and all four sites call it — because the inferred TYPE is not the whole answer, and splitting the two halves across call sites is what let them drift. Pinned by asserting the entry points agree with each other rather than restating the expected value in each, plus that the inferred column survives persistence so the stored shape matches how its rows were coerced.

waleedlatif1 · 2026-08-02T02:35:12Z

@greptile

waleedlatif1 · 2026-08-02T02:35:13Z

@cursor review

The append branch returns instead of rethrowing, so nothing the outer catch does applies to it. addTableColumnsWithTx runs inside importAppendRows, so the column cap and an invalid column name surfaced there as a generic 500 with the real reason replaced by 'Failed to import CSV'. Extract tableRequestErrorResponse alongside the existing rowWriteErrorResponse funnel and call it from all three catches (both import routes), so a new service validation message is classified the day it is added rather than when someone remembers to extend a substring list.

waleedlatif1 · 2026-08-02T02:58:46Z

@greptile

waleedlatif1 · 2026-08-02T02:58:47Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 9dc14c1. Configure here.}