fix(tables): refuse scale suffixes and resolve the lone-separator ambiguity (#6111)

* fix(tables): refuse scale suffixes and resolve the lone-separator ambiguity

A verification pass over the previous commit found that my own fix opened a new
hole of the class it closed.

Widening the currency marker to 1-3 letters made scale suffixes parse: `1.2 M`
read as 1.2, so a column of `1.2 M` / `3.4 M` — an ordinary spreadsheet paste —
converted cleanly and rewrote every cell a millionfold too small. Before the
widening those were rejected and the data was safe. Now refused explicitly,
while `kr` and `zł` still parse despite starting with the same letters.

Stripping the marker also newly routed formatted zero-decimal amounts into the
lone-separator branch, where a single dot was always decimal: `1.235 ¥` read as
1.235 rather than 1235. That is a wrong number where there used to be a
refusal, which is the worse failure. A lone separator followed by three digits
is now resolved by two signals — a marker means a formatter produced it, and
formatters group; a currency carrying three decimals (KWD, TND) reads them as
decimals. `coerce` passes the column's code, so the parser can ask. Bare typed
input keeps the decimal reading.

Also: the unchanged-type throw fell through to a 500 for every type except
currency, whose message happened to contain the word; it now maps to 400. And
the last three comments describing the removed two-transaction architecture are
gone — the previous commit claimed they were and two survived.

Documented, and failing closed rather than guessing: markers written flush
against the digits (`Rp12,00`) stay rejected, because a letter touching a digit
is the only thing distinguishing a currency marker from a part number, and
reading `SKU400` as 400 invents a value where refusing merely inconveniences.

* fix(tables): fit the whole column-type list in the New column dropdown

Adding currency made the list nine items (295px with its separator and
padding), past the shared dropdown's 240px cap — so Select and Workflow sat
below a scrollbar.

Sized on this dropdown rather than on the shared `DropdownMenuContent`, which
every other menu in the app relies on. Also gave the sidebar's type combobox
headroom, since it gained the same entry and was within 4px of its own cap.

* fix(tables): read a lone separator against the column's currency

Review of the previous commit found the marker signal alone was too coarse.

The evidence: a currency with decimal places ALWAYS formats with both
separators — `1.234,56 €`, `$1,234.56`, `1.234,000 KWD`. Only a zero-decimal
currency emits a lone one (`1.235 ¥`). So a marker does not imply grouping; the
currency's own decimal places decide, and a three-decimal currency's trailing
three digits are decimals however the value arrived.

That matters most for CSV import, which passed no currency at all: a KWD column
importing `0,500` read as five hundred rather than a half. The column's code is
now forwarded, since nothing in a CSV carries a marker to fall back on.

A bare typed `1.234` still reads as decimals.

* fix(tables): let the separator decide a lone-separator amount, not the marker

Greptile blocked on `$1.234` reading as 1234, and offered the alternative of
stating the input contract explicitly. Looking for a precedent settled it
against me: a spreadsheet with a USD-formatted cell stores a typed `1.234` as
1.234 and displays $1.23. Reinterpreting it as one thousand two hundred
thirty-four is a thousandfold surprise, and my defense of it was wrong.

The rule now keys on the separator, tempered by what the currency can express:

- A dot is the decimal point in the notation most people type, so it stays a
  decimal — except for a currency with no decimal places, where `1.235 ¥`
  cannot be a fraction of a yen and is the one lone-separator form a formatter
  actually emits.
- A comma is grouping by convention (`1,500` is fifteen hundred), except for a
  three-decimal currency, where `0,500` is a half.

A currency with one or two decimal places always formats with both separators,
so no formatter output is decided here at all.

This also drops the `hadMarker` signal entirely — once the separator and the
currency answer the question, where the string came from stops mattering.

* fix(tables): check conversions against the target column, not a rebuilt stub

Bugbot flagged that the conversion gate calls `isValueCompatibleWithType`,
which hand-builds a column from loose arguments and never sets
`currencyCode` — while the coercion a few lines later reads the real one.

The reported impact does not occur. Whether an amount parses does not depend
on the currency, only which number it yields, so the gate and the coercion
always agree on accept/reject; and the value written back is already coerced
against the real column, so no cell is stored wrong. I checked this rather
than argued it: across nine currency codes and every separator shape in the
parser's repertoire, acceptance never diverged.

The stub is still worth removing. It is correct only because of an invariant
nobody wrote down, and `convertedColumn` — sitting right there, whose own
comment already says it exists so the scan reads the same option set and
currency — is what the gate should have used all along.

So the gate now takes the target column itself. The loose-argument form moves
into the test file as local marshalling, leaving production with only the
whole-column shape, which cannot silently drop a key a future type adds.

No behavior change, so no test changes: a currency-divergence regression test
could not fail today, and pinning an invariant this removes the need for would
only block a legitimate future change.

* fix(tables): vet a currency marker against an allowlist, not a scale denylist

Bugbot found that `SCALE_SUFFIX` misses `mio`, `mrd`, `bio`, `tsd`, `mln`
and `md`. Each is one to three letters, so each matched the currency-marker
pattern and was stripped: `1,2 mio` read as 1.2, a millionfold too small,
which is the same failure `1.2 M` had.

Adding the six words would leave the next six. The denylist is the defect —
it has to enumerate every magnitude abbreviation in every language, and each
one it misses silently rescales a value rather than refusing it.

So the check is inverted. A bare letter token is stripped only when it names
a currency: an ISO 4217 code the runtime can enumerate, or one of the common
non-ISO markers people type (`kr`, `zł`, `Kč`). Anything else is left in
place, fails the amount-shape check, and is refused. `12 units` and `12 pcs`
now fall out of the same rule the magnitude words do, without naming either.

A marker carrying a currency symbol skips the check — no magnitude
abbreviation contains one — so `R$`, `CHF`, `1 234,56 kr` are untouched.

This rests on ISO codes and magnitude words being disjoint, which is true for
all 162 codes the runtime knows, and is now pinned by a test so a future
collision fails loudly instead of silently.

`SCALE_SUFFIX` survives only for a runtime without `Intl.supportedValuesOf`,
which cannot enumerate codes and so keeps the old permissive behaviour;
rejecting every letter marker there would break `USD 12.50`. It no longer has
to be complete, since every modern runtime rejects by non-recognition.

Verified the new tests fail when the vetting is reverted.

* fix(tables): group a lone dot only when a zero-decimal amount came formatted

Greptile found that a bare `1.235` typed into a JPY column stored 1235. The
zero-decimal branch read the dot as grouping without asking where the value
came from, so typing, pasting, a tool write, or an import all inflated a
thousandfold in silence.

The rule was inverted relative to its own evidence. Grouping was justified as
"the one lone-separator form a formatter emits" — and a formatter always emits
its marker too. The marker is exactly what separates formatter output from
someone typing, and I had removed it as redundant two commits ago.

So a lone dot groups only for a zero-decimal currency AND only when a marker
came with it. `1.235 ¥` and `JPY 1.235` still read as 1235; a bare `1.235`
now stores 1.235 and displays `¥1` — wrong in a way the writer can see and
correct, rather than a silent thousandfold error.

This does not walk back the earlier fix. For a currency with one or two
decimal places a formatter always emits BOTH separators, so a lone dot there
never came from one and stays a decimal whether or not a marker is present —
`$1.234` is still 1.234. The marker only carries information in the
zero-decimal case, which is the only case that now consults it.

The two overlapping lone-separator tests are replaced by three that each state
one rule. Verified the new one fails when the marker requirement is reverted.

Author: waleedlatif1

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

@@ -323,7 +323,8 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
       msg.includes('incompatible') ||
       msg.includes('duplicate') ||
       msg.includes('option') ||
-      msg.includes('currency')
+      msg.includes('currency') ||
+      msg.includes('is already type')
     ) {
       return NextResponse.json({ error: msg }, { status: 400 })
     }

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

@@ -368,7 +368,8 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
         msg.includes('incompatible') ||
         msg.includes('duplicate') ||
         msg.includes('option') ||
-        msg.includes('currency')
+        msg.includes('currency') ||
+        msg.includes('is already type')
       ) {
         return NextResponse.json({ error: msg }, { status: 400 })
       }

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

@@ -282,7 +282,7 @@ function ColumnConfigBody({
                 value={typeInput}
                 onChange={(v) => setTypeInput(v as ColumnDefinition['type'])}
                 placeholder='Select type'
-                maxHeight={260}
+                maxHeight={300}
               />
             </div>
           </>

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/new-column-dropdown/new-column-dropdown.tsx

@@ -86,7 +86,11 @@ export function NewColumnDropdown({
   const menu = (
     <DropdownMenu>
       <DropdownMenuTrigger asChild>{triggerButton}</DropdownMenuTrigger>
-      <DropdownMenuContent align='start' side='bottom' sideOffset={4}>
+      {/* Taller than the 240px shared default: the full type list is 9 items
+          (295px with its separator and padding), so the default cut the last
+          two off behind a scrollbar. Sized here rather than in the shared
+          component, which every other dropdown in the app relies on. */}
+      <DropdownMenuContent align='start' side='bottom' sideOffset={4} className='max-h-[320px]'>
         <>
           <DropdownMenuItem onSelect={onPickEnrichment}>
             <Sparkles className='size-[14px] text-[var(--text-icon)]' />

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

@@ -8,11 +8,32 @@ import { describe, expect, it } from 'vitest'
 import { COLUMN_TYPE_REGISTRY } from '@/lib/table/column-types'
 import {
   applyPendingRename,
-  isValueCompatibleWithType,
+  isValueCompatibleWithColumn,
   selectValueForConversion,
 } from '@/lib/table/columns/service'
 import { resolveSelectOptionId } from '@/lib/table/select-options'
-import type { ColumnDefinition, SelectOption } from '@/lib/table/types'
+import type { ColumnDefinition, ColumnType, SelectOption } from '@/lib/table/types'
+
+/**
+ * Marshals the loose per-type arguments these cases are written in into the
+ * target column the gate takes. Lives here rather than in the service so
+ * production has only the whole-column form, which cannot drop a metadata key.
+ */
+function isValueCompatibleWithType(
+  value: unknown,
+  targetType: ColumnType,
+  targetOptions: SelectOption[] = [],
+  targetMultiple = false,
+  targetRequired = false
+): boolean {
+  return isValueCompatibleWithColumn(value, {
+    name: '',
+    type: targetType,
+    options: targetOptions,
+    multiple: targetMultiple,
+    required: targetRequired,
+  })
+}
 
 const OPTIONS: SelectOption[] = [
   { id: 'opt_a', name: 'Alpha' },

apps/sim/lib/table/__tests__/currency.test.ts

@@ -160,6 +160,79 @@ describe('parseCurrencyInput', () => {
     expect(parseCurrencyInput('CHF 1’234.56')).toBe(1234.56)
   })
 
+  it('reads a lone dot as the decimal point people type', () => {
+    // Typing `1.234` in a USD cell means 1.234, which the display rounds to
+    // $1.23 exactly as a spreadsheet does. Reading it as 1,234 would be a
+    // thousandfold surprise, marker or no marker.
+    expect(parseCurrencyInput('$1.234', 'USD')).toBe(1.234)
+    expect(parseCurrencyInput('1.234')).toBe(1.234)
+    expect(parseCurrencyInput('1.234', 'EUR')).toBe(1.234)
+  })
+
+  it('groups a lone dot only for a zero-decimal currency that came formatted', () => {
+    // `1.235 ¥` cannot be a fraction of a yen, and is the single lone-separator
+    // form a formatter emits — but a formatter always emits its marker too.
+    expect(parseCurrencyInput('1.235 ¥', 'JPY')).toBe(1235)
+    expect(parseCurrencyInput('JPY 1.235', 'JPY')).toBe(1235)
+    // Bare, it is someone typing. Inflating that to 1235 would be a silent
+    // thousandfold error; read literally it stores 1.235 and displays ¥1 —
+    // wrong in a way the writer can see and fix.
+    expect(parseCurrencyInput('1.235', 'JPY')).toBe(1.235)
+    expect(parseCurrencyInput('1.235', 'CLP')).toBe(1.235)
+  })
+
+  it('reads a lone comma as grouping, except where the currency has three decimals', () => {
+    // `1,500` is fifteen hundred by convention. A three-decimal currency's
+    // trailing three digits are decimals however the value arrived — which
+    // matters most for CSV import, where nothing carries a marker.
+    expect(parseCurrencyInput('1,500', 'USD')).toBe(1500)
+    expect(parseCurrencyInput('1,500')).toBe(1500)
+    expect(parseCurrencyInput('0,500', 'KWD')).toBe(0.5)
+    expect(parseCurrencyInput('0,500 KWD', 'KWD')).toBe(0.5)
+    expect(parseCurrencyInput('12,000', 'TND')).toBe(12)
+    expect(parseCurrencyInput('12,000 TND', 'TND')).toBe(12)
+  })
+
+  it('refuses a scale suffix rather than shrinking the value', () => {
+    // `1.2 M` read as 1.2 would rewrite a column of millions a millionfold too
+    // small — the same invented-value failure as an identifier, inverted.
+    expect(parseCurrencyInput('1.2 M')).toBeNull()
+    expect(parseCurrencyInput('5 K')).toBeNull()
+    expect(parseCurrencyInput('3.4 bn')).toBeNull()
+    expect(parseCurrencyInput('10 B')).toBeNull()
+    // `kr` is a currency marker, not a scale suffix, despite starting with k.
+    expect(parseCurrencyInput('1 234,56 kr')).toBe(1234.56)
+  })
+
+  it('refuses non-English magnitude words too', () => {
+    // These are why the marker check is an allowlist. As a denylist each one
+    // had to be named, and any that was missed read as a bare number — `1,2
+    // mio` as 1.2 rather than 1.2 million.
+    expect(parseCurrencyInput('1,2 mio')).toBeNull()
+    expect(parseCurrencyInput('3,4 mrd')).toBeNull()
+    expect(parseCurrencyInput('5 tsd')).toBeNull()
+    expect(parseCurrencyInput('2,5 mln')).toBeNull()
+    expect(parseCurrencyInput('1,5 bio')).toBeNull()
+    expect(parseCurrencyInput('7 md')).toBeNull()
+    // Anything else beside a number is refused by the same rule, so the parser
+    // no longer has to enumerate what a magnitude word looks like.
+    expect(parseCurrencyInput('12 units')).toBeNull()
+    expect(parseCurrencyInput('12 pcs')).toBeNull()
+  })
+
+  it('keeps every ISO code strippable — no code is a magnitude word', () => {
+    // The allowlist is only safe because these two sets do not overlap. If a
+    // future ISO code ever collided with a magnitude abbreviation, this fails
+    // and the marker rule needs a tiebreak.
+    const codes = new Set(
+      (Intl as unknown as { supportedValuesOf: (k: string) => string[] }).supportedValuesOf(
+        'currency'
+      )
+    )
+    const magnitudeWords = ['K', 'M', 'B', 'T', 'BN', 'MN', 'MIO', 'MRD', 'TSD', 'MLN', 'BIO', 'MD']
+    expect(magnitudeWords.filter((w) => codes.has(w))).toEqual([])
+  })
+
   it('rejects an identifier whose letters touch its digits', () => {
     // The distinguishing rule: a currency marker is always separated from the
     // number by a space or a symbol, so letters touching digits mean this is a

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

@@ -27,11 +27,11 @@ export const currencyColumnType: ColumnTypeDefinition = {
   typeaheadPattern: /[\d.,\-\p{Sc}]/u,
   parseErrorMessage: 'Invalid amount',
 
-  coerce(value) {
+  coerce(value, column) {
     // Stored as a bare number, but accepts the formatted shapes an amount
     // arrives in — `$1,234.56`, `1 234,56 €`, `(12.00)` — so a paste, CSV
     // import, or tool write lands as a number rather than being nulled.
-    const parsed = parseCurrencyInput(value)
+    const parsed = parseCurrencyInput(value, column.currencyCode)
     return parsed === null ? { ok: false } : { ok: true, value: parsed }
   },
 

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

@@ -749,15 +749,14 @@ export async function updateColumnType(
     // once the column is text/number/etc. Check compatibility against the option
     // NAME — that's what the cell will actually become (migrated below).
     const convertingAwayFromSelect = column.type === 'select' && !isSelectType
-    // The constraint the column ends up with, which may be arriving in this same
-    // request. `updateColumnConstraints` runs as its own transaction afterwards,
-    // so validating against the column's CURRENT flag would let the conversion
-    // commit and only then fail the constraint.
+    // The constraint the column ends up with, which may be arriving in this
+    // same request — this write applies it, so the scan below has to judge
+    // against the target value rather than the current one.
     const targetRequired = !!(data.required ?? column.required)
 
     // Rows missing the key (or holding null/`[]`) are filtered out of `rows`
     // entirely, so the loop below can never see them — they have to be counted
-    // separately, through the same predicate the constraint write will use.
+    // separately, through the same predicate `applyConstraints` uses.
     if (targetRequired) {
       const emptyCount = await countEmptyCells(trx, data.tableId, columnKey)
       if (emptyCount > 0) {
@@ -802,15 +801,7 @@ export async function updateColumnType(
 
       const effective = convertingAwayFromSelect ? selectValueForConversion(column, value) : value
 
-      if (
-        !isValueCompatibleWithType(
-          effective,
-          data.newType,
-          targetOptions,
-          !!targetMultiple,
-          targetRequired
-        )
-      ) {
+      if (!isValueCompatibleWithColumn(effective, convertedColumn)) {
         // A cell the target cannot read but that is merely EMPTY is not a
         // conversion failure — the write path already turns an unreadable value
         // into null on an optional column, so the conversion does the same. Only
@@ -1331,29 +1322,24 @@ export function selectValueForConversion(column: ColumnDefinition, value: unknow
 }
 
 /**
- * Checks if a value is compatible with a target column type. For `select`,
- * `targetOptions` is the option set the column will carry after the change: a
- * value is compatible only if every part of it resolves to one of those options
- * (by id or name). This blocks a conversion that would otherwise strand or
- * silently drop values on the next row write.
+ * Checks a value against the column definition the table will end up with.
+ *
+ * Takes the whole target column rather than loose per-type arguments: the gate
+ * has to read the SAME metadata the later coercion does, and a hand-built stub
+ * silently omits whatever key its author did not think of. Today that key would
+ * be `currencyCode` — a three-decimal column reads `0,500` as a half dinar
+ * while a stub without the code reads it as five hundred.
+ *
+ * That divergence is currently invisible, because whether an amount parses at
+ * all does not depend on the currency, only which number it yields — and the
+ * write-back already coerces against the real column. Passing the real column
+ * here means it cannot become visible when that stops being true.
  *
  * Callers converting *away* from `select` must pass the resolved option
  * name(s), not the stored ids — see {@link selectValueForConversion}.
  */
-export function isValueCompatibleWithType(
-  value: unknown,
-  targetType: (typeof COLUMN_TYPES)[number],
-  targetOptions: SelectOption[] = [],
-  targetMultiple = false,
-  targetRequired = false
-): boolean {
+export function isValueCompatibleWithColumn(value: unknown, target: ColumnDefinition): boolean {
   if (value === null || value === undefined) return true
   // Each type reads only the metadata it owns.
-  return isValueCompatible(value, {
-    name: '',
-    type: targetType,
-    options: targetOptions,
-    multiple: targetMultiple,
-    required: targetRequired,
-  })
+  return isValueCompatible(value, target)
 }