From 6b12f7a9f9663f5dcd744cba368f2505e6279546 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 30 Jul 2026 17:32:20 -0700 Subject: [PATCH 1/7] fix(tables): refuse scale suffixes and resolve the lone-separator ambiguity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../app/api/table/[tableId]/columns/route.ts | 3 +- .../api/v1/tables/[tableId]/columns/route.ts | 3 +- apps/sim/lib/table/__tests__/currency.test.ts | 23 +++++ apps/sim/lib/table/column-types/currency.ts | 4 +- apps/sim/lib/table/columns/service.ts | 9 +- apps/sim/lib/table/currency.ts | 90 +++++++++++++++---- 6 files changed, 107 insertions(+), 25 deletions(-) diff --git a/apps/sim/app/api/table/[tableId]/columns/route.ts b/apps/sim/app/api/table/[tableId]/columns/route.ts index 19289a0198c..5cad8c5e610 100644 --- a/apps/sim/app/api/table/[tableId]/columns/route.ts +++ b/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 }) } diff --git a/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts b/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts index 56caed8353e..d1a507fa195 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts +++ b/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 }) } diff --git a/apps/sim/lib/table/__tests__/currency.test.ts b/apps/sim/lib/table/__tests__/currency.test.ts index e7b7802552b..4c97a5409c4 100644 --- a/apps/sim/lib/table/__tests__/currency.test.ts +++ b/apps/sim/lib/table/__tests__/currency.test.ts @@ -160,6 +160,29 @@ describe('parseCurrencyInput', () => { expect(parseCurrencyInput('CHF 1’234.56')).toBe(1234.56) }) + it('resolves a lone separator using the marker and the currency', () => { + // `1.235 ¥` is 1235 yen; a typed `1.235` is one-and-a-bit. A marker means a + // formatter produced it, and formatters group — except for the currencies + // that genuinely carry three decimals. + expect(parseCurrencyInput('1.235 ¥', 'JPY')).toBe(1235) + expect(parseCurrencyInput('0,500 KWD', 'KWD')).toBe(0.5) + expect(parseCurrencyInput('12,000 TND', 'TND')).toBe(12) + // Bare input keeps the typed reading. + expect(parseCurrencyInput('1.234')).toBe(1.234) + expect(parseCurrencyInput('1,500')).toBe(1500) + }) + + 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('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 diff --git a/apps/sim/lib/table/column-types/currency.ts b/apps/sim/lib/table/column-types/currency.ts index 16ee515f830..86485a265d3 100644 --- a/apps/sim/lib/table/column-types/currency.ts +++ b/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 } }, diff --git a/apps/sim/lib/table/columns/service.ts b/apps/sim/lib/table/columns/service.ts index d8f627fc117..35f0a9ba46e 100644 --- a/apps/sim/lib/table/columns/service.ts +++ b/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) { diff --git a/apps/sim/lib/table/currency.ts b/apps/sim/lib/table/currency.ts index c4aeb97e09b..fd0f5833a7b 100644 --- a/apps/sim/lib/table/currency.ts +++ b/apps/sim/lib/table/currency.ts @@ -24,6 +24,12 @@ const BIDI_MARKS = /[\u200e\u200f\u061c\u202a-\u202e\u2066-\u2069]/g /** Minus-sign characters `Intl` emits in place of the ASCII hyphen. */ const UNICODE_MINUS = /[\u2212\u2012\u2013\uFE63\uFF0D]/g +/** + * A magnitude suffix (`1.2 M`, `5 K`, `3.4 bn`). Not a currency marker, and + * stripping it would silently shrink the value by orders of magnitude. + */ +const SCALE_SUFFIX = /\d\s*(?:k|m|b|t|bn|mn|tn|mm|mil|bil)\.?\s*$/i + /** A letter directly adjacent to a digit: an identifier, not an amount. */ const LETTER_TOUCHING_DIGIT = /\p{L}\d|\d\p{L}/u @@ -171,15 +177,23 @@ export function getCurrencyOptions(): readonly CurrencyOption[] { * therefore read as fifteen hundred — a known ambiguity that resolves in favor * of the far more common reading. * - * Reads ASCII digits only. Locales that format with their own numeral systems - * (Arabic-Indic `١٢٣`, for instance) are rejected rather than misread — - * supporting them is a wider decision than this type, since it would also - * touch `number`, display, and sorting. + * Two input families are deliberately refused rather than guessed at, because + * both would otherwise be misread as an amount rather than rejected: + * + * - Markers written flush against the digits (`Rp12,00`). A letter touching a + * digit is the only thing separating a currency marker from a part number, + * and reading `SKU400` as 400 invents a value where refusing merely + * inconveniences. Markers separated by a space or a symbol all work. + * - Locales formatting with their own numeral systems (Arabic-Indic `١٢٣`). + * Supporting them is a wider decision than this type, since it would also + * touch `number`, display, and sorting. + * + * Both fail closed — `null`, never a wrong number. * * Returns `null` when no amount can be read, so callers can distinguish * "unparseable" from a legitimate `0`. */ -export function parseCurrencyInput(raw: unknown): number | null { +export function parseCurrencyInput(raw: unknown, currencyCode?: string): number | null { if (typeof raw === 'number') return Number.isFinite(raw) ? raw : null if (typeof raw !== 'string') return null @@ -219,14 +233,23 @@ export function parseCurrencyInput(raw: unknown): number | null { // by a space or a symbol, so this distinguishes them without a symbol list. if (LETTER_TOUCHING_DIGIT.test(body)) return null + // A scale suffix is not a currency marker, and dropping it loses orders of + // magnitude: `1.2 M` would read as 1.2, so a column of `1.2 M` / `3.4 M` + // would convert cleanly and rewrite every cell a millionfold too small. + // Refuse rather than guess what the writer meant. + if (SCALE_SUFFIX.test(body)) return null + // Strip the currency marker: up to three letters (an ISO code, `kr`, `zł`) // optionally joined to a symbol (`R$`, `CHF`), at either end. Bounded at // three so prose does not qualify — `Revenue 5` keeps its letters and is // rejected below. - const cleaned = body - .replace(CURRENCY_MARKER_PREFIX, '$1') - .replace(CURRENCY_MARKER_SUFFIX, '') - .replace(/[\s\u00a0\u202f\u2019']/gu, '') + const withoutPrefix = body.replace(CURRENCY_MARKER_PREFIX, '$1') + const withoutMarkers = withoutPrefix.replace(CURRENCY_MARKER_SUFFIX, '') + // Whether a marker was present at all. A marked string came from a + // formatter; a bare one was typed by a person. That distinguishes the two + // readings of a lone separator followed by three digits, below. + const hadMarker = withoutMarkers !== body + const cleaned = withoutMarkers.replace(/[\s\u00a0\u202f\u2019']/gu, '') // What remains must be ONLY digits and separators. Anything else — a // US-format date, leftover prose — is not an amount. if (!AMOUNT_SHAPE.test(cleaned)) return null @@ -246,19 +269,25 @@ export function parseCurrencyInput(raw: unknown): number | null { if (decimalParts.length > 1 || !hasValidGrouping(integerPart, groupSeparator)) return null normalized = `${integerPart.split(groupSeparator).join('')}.${decimalParts[0]}` } else if (lastComma !== -1) { - // A single comma followed by exactly three digits is grouping (`1,500`); - // anything else is a decimal comma (`1,50`). - const grouping = digitsAndSeps.indexOf(',') !== lastComma || /,\d{3}$/.test(digitsAndSeps) + const repeated = digitsAndSeps.indexOf(',') !== lastComma + const grouping = + repeated || loneSeparatorIsGrouping(digitsAndSeps, ',', hadMarker, currencyCode) if (grouping) { if (!hasValidGrouping(digitsAndSeps, ',')) return null normalized = digitsAndSeps.split(',').join('') } else { normalized = digitsAndSeps.replace(',', '.') } - } else if (lastDot !== -1 && digitsAndSeps.indexOf('.') !== lastDot) { - // More than one dot can only be grouping: `1.234.567`. - if (!hasValidGrouping(digitsAndSeps, '.')) return null - normalized = digitsAndSeps.split('.').join('') + } else if (lastDot !== -1) { + const repeated = digitsAndSeps.indexOf('.') !== lastDot + const grouping = + repeated || loneSeparatorIsGrouping(digitsAndSeps, '.', hadMarker, currencyCode) + if (grouping) { + if (!hasValidGrouping(digitsAndSeps, '.')) return null + normalized = digitsAndSeps.split('.').join('') + } else { + normalized = digitsAndSeps + } } else { normalized = digitsAndSeps } @@ -268,6 +297,35 @@ export function parseCurrencyInput(raw: unknown): number | null { return negative ? -parsed : parsed } +/** + * Whether a lone separator followed by exactly three digits is grouping rather + * than a decimal point — `1.235 ¥` is one thousand two hundred thirty-five yen, + * while a typed `1.235` is one and a bit. + * + * Two signals resolve it. A marker means the string came from a formatter, and + * formatters group; a bare string was typed by a person, who meant decimals. + * And a currency with three decimal places (KWD, TND) legitimately ends in + * three digits after its separator, so for those the reading is always decimal. + */ +function loneSeparatorIsGrouping( + digitsAndSeps: string, + separator: string, + hadMarker: boolean, + currencyCode: string | undefined +): boolean { + if (!new RegExp(`\\${separator}\\d{3}$`).test(digitsAndSeps)) return false + if (separator === ',' && !hadMarker) return true + if (!hadMarker) return false + return currencyFractionDigits(currencyCode) !== 3 +} + +/** A currency's conventional decimal places, defaulting to 2 when unknown. */ +function currencyFractionDigits(currencyCode: string | undefined): number { + if (!currencyCode) return 2 + const formatter = currencyFormatter(resolveCurrencyCode(currencyCode), undefined) + return formatter?.resolvedOptions().maximumFractionDigits ?? 2 +} + /** * Formatters are cached by locale + code: a grid paints thousands of currency * cells per scroll, and constructing an `Intl.NumberFormat` per cell is orders From d90390058d65520106df2a46f50c43b8e64a5747 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 30 Jul 2026 17:41:53 -0700 Subject: [PATCH 2/7] fix(tables): fit the whole column-type list in the New column dropdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../column-config-sidebar/column-config-sidebar.tsx | 2 +- .../components/new-column-dropdown/new-column-dropdown.tsx | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-config-sidebar/column-config-sidebar.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-config-sidebar/column-config-sidebar.tsx index af2a0c797c6..116b43de286 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-config-sidebar/column-config-sidebar.tsx +++ b/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} /> diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/new-column-dropdown/new-column-dropdown.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/new-column-dropdown/new-column-dropdown.tsx index 031aa4cbe2b..161b4a32a51 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/new-column-dropdown/new-column-dropdown.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/new-column-dropdown/new-column-dropdown.tsx @@ -86,7 +86,11 @@ export function NewColumnDropdown({ const menu = ( {triggerButton} - + {/* 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. */} + <> From 5453662043fec6f8fb3162ec5d445f037a1bf894 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 30 Jul 2026 17:46:54 -0700 Subject: [PATCH 3/7] fix(tables): read a lone separator against the column's currency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/lib/table/__tests__/currency.test.ts | 13 +++++++++++++ apps/sim/lib/table/currency.ts | 12 +++++++++--- apps/sim/lib/table/import.test.ts | 9 +++++++++ apps/sim/lib/table/import.ts | 13 +++++++++---- 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/apps/sim/lib/table/__tests__/currency.test.ts b/apps/sim/lib/table/__tests__/currency.test.ts index 4c97a5409c4..1e44cd80c93 100644 --- a/apps/sim/lib/table/__tests__/currency.test.ts +++ b/apps/sim/lib/table/__tests__/currency.test.ts @@ -172,6 +172,19 @@ describe('parseCurrencyInput', () => { expect(parseCurrencyInput('1,500')).toBe(1500) }) + it('reads a lone separator against the currency, not just the marker', () => { + // A currency with decimals always formats with BOTH separators, so a lone + // one never comes from a formatter for those — only zero-decimal + // currencies produce `1.235 ¥`. And a three-decimal currency's trailing + // three digits are decimals however the value arrived, which matters for + // CSV import, where nothing carries a marker. + expect(parseCurrencyInput('1.235 ¥', 'JPY')).toBe(1235) + expect(parseCurrencyInput('0,500', 'KWD')).toBe(0.5) + expect(parseCurrencyInput('12,000', 'TND')).toBe(12) + expect(parseCurrencyInput('1,500', 'USD')).toBe(1500) + expect(parseCurrencyInput('1.234')).toBe(1.234) + }) + 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. diff --git a/apps/sim/lib/table/currency.ts b/apps/sim/lib/table/currency.ts index fd0f5833a7b..8cc2d48e229 100644 --- a/apps/sim/lib/table/currency.ts +++ b/apps/sim/lib/table/currency.ts @@ -314,9 +314,15 @@ function loneSeparatorIsGrouping( currencyCode: string | undefined ): boolean { if (!new RegExp(`\\${separator}\\d{3}$`).test(digitsAndSeps)) return false - if (separator === ',' && !hadMarker) return true - if (!hadMarker) return false - return currencyFractionDigits(currencyCode) !== 3 + // A currency with three decimal places (KWD, TND) legitimately ends in three + // digits after its separator, so for those the reading is always decimal — + // regardless of how the value arrived, since a CSV import carries no marker. + if (currencyFractionDigits(currencyCode) === 3) return false + // A comma tail of exactly three digits is grouping by convention (`1,500`). + if (separator === ',') return true + // A dot is the decimal point in the notation most users type, so it only + // reads as grouping when a marker shows a formatter produced the string. + return hadMarker } /** A currency's conventional decimal places, defaulting to 2 when unknown. */ diff --git a/apps/sim/lib/table/import.test.ts b/apps/sim/lib/table/import.test.ts index 2943870e899..4ef9a6ef375 100644 --- a/apps/sim/lib/table/import.test.ts +++ b/apps/sim/lib/table/import.test.ts @@ -145,6 +145,15 @@ describe('import', () => { expect(coerceValue('ask sales', 'currency')).toBeNull() }) + it('reads an imported amount against the column currency', () => { + // Nothing in a CSV carries a currency marker, so the column's own code is + // the only signal for a three-decimal currency: `0,500` KWD is a half, + // not five hundred. + expect(coerceValue('0,500', 'currency', { currencyCode: 'KWD' })).toBe(0.5) + expect(coerceValue('12,000', 'currency', { currencyCode: 'TND' })).toBe(12) + expect(coerceValue('1,500', 'currency', { currencyCode: 'USD' })).toBe(1500) + }) + it('coerces booleans strictly', () => { expect(coerceValue('true', 'boolean')).toBe(true) expect(coerceValue('FALSE', 'boolean')).toBe(false) diff --git a/apps/sim/lib/table/import.ts b/apps/sim/lib/table/import.ts index e01147c36f9..02c8a6e431a 100644 --- a/apps/sim/lib/table/import.ts +++ b/apps/sim/lib/table/import.ts @@ -385,7 +385,7 @@ export function inferSchemaFromCsv( export function coerceValue( value: unknown, colType: CsvColumnType, - options?: NormalizeDateCellOptions + options?: NormalizeDateCellOptions & { currencyCode?: string } ): string | number | boolean | null | Record | unknown[] { if (value === null || value === undefined || value === '') return null switch (colType) { @@ -394,9 +394,11 @@ export function coerceValue( return Number.isNaN(n) ? null : n } // Importing into an existing currency column: the file carries the - // formatted amount (`$1,234.56`) but the cell stores a bare number. + // formatted amount (`$1,234.56`) but the cell stores a bare number. The + // column's currency is forwarded because it decides how a lone separator + // reads — a three-decimal currency's `0,500` is a half, not five hundred. case 'currency': - return parseCurrencyInput(value) + return parseCurrencyInput(value, options?.currencyCode) case 'boolean': { const s = String(value).toLowerCase() if (s === 'true') return true @@ -590,7 +592,10 @@ export function coerceRowsForTable( const col = colByName.get(colName) if (!col) continue const colType = (col.type as CsvColumnType) ?? 'string' - coerced[getColumnId(col)] = coerceValue(value, colType, options) as RowData[string] + coerced[getColumnId(col)] = coerceValue(value, colType, { + ...options, + ...(col.currencyCode !== undefined ? { currencyCode: col.currencyCode } : {}), + }) as RowData[string] } return coerced }) From 6a4726733570ea7aacc5007f1ae20b31ca523fb0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 30 Jul 2026 17:52:42 -0700 Subject: [PATCH 4/7] fix(tables): let the separator decide a lone-separator amount, not the marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/lib/table/__tests__/currency.test.ts | 11 ++++- apps/sim/lib/table/currency.ts | 44 +++++++++---------- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/apps/sim/lib/table/__tests__/currency.test.ts b/apps/sim/lib/table/__tests__/currency.test.ts index 1e44cd80c93..48f3fa82605 100644 --- a/apps/sim/lib/table/__tests__/currency.test.ts +++ b/apps/sim/lib/table/__tests__/currency.test.ts @@ -178,11 +178,18 @@ describe('parseCurrencyInput', () => { // currencies produce `1.235 ¥`. And a three-decimal currency's trailing // three digits are decimals however the value arrived, which matters for // CSV import, where nothing carries a marker. + // A dot stays the decimal point: 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. + expect(parseCurrencyInput('$1.234', 'USD')).toBe(1.234) + expect(parseCurrencyInput('1.234')).toBe(1.234) + // Except where the currency has no decimals — the one lone-separator form + // a formatter actually emits. expect(parseCurrencyInput('1.235 ¥', 'JPY')).toBe(1235) + // A comma is grouping by convention, except for three-decimal currencies. + expect(parseCurrencyInput('1,500', 'USD')).toBe(1500) expect(parseCurrencyInput('0,500', 'KWD')).toBe(0.5) expect(parseCurrencyInput('12,000', 'TND')).toBe(12) - expect(parseCurrencyInput('1,500', 'USD')).toBe(1500) - expect(parseCurrencyInput('1.234')).toBe(1.234) }) it('refuses a scale suffix rather than shrinking the value', () => { diff --git a/apps/sim/lib/table/currency.ts b/apps/sim/lib/table/currency.ts index 8cc2d48e229..6e7afe11fe9 100644 --- a/apps/sim/lib/table/currency.ts +++ b/apps/sim/lib/table/currency.ts @@ -245,10 +245,6 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number // rejected below. const withoutPrefix = body.replace(CURRENCY_MARKER_PREFIX, '$1') const withoutMarkers = withoutPrefix.replace(CURRENCY_MARKER_SUFFIX, '') - // Whether a marker was present at all. A marked string came from a - // formatter; a bare one was typed by a person. That distinguishes the two - // readings of a lone separator followed by three digits, below. - const hadMarker = withoutMarkers !== body const cleaned = withoutMarkers.replace(/[\s\u00a0\u202f\u2019']/gu, '') // What remains must be ONLY digits and separators. Anything else — a // US-format date, leftover prose — is not an amount. @@ -270,8 +266,7 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number normalized = `${integerPart.split(groupSeparator).join('')}.${decimalParts[0]}` } else if (lastComma !== -1) { const repeated = digitsAndSeps.indexOf(',') !== lastComma - const grouping = - repeated || loneSeparatorIsGrouping(digitsAndSeps, ',', hadMarker, currencyCode) + const grouping = repeated || loneSeparatorIsGrouping(digitsAndSeps, ',', currencyCode) if (grouping) { if (!hasValidGrouping(digitsAndSeps, ',')) return null normalized = digitsAndSeps.split(',').join('') @@ -280,8 +275,7 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number } } else if (lastDot !== -1) { const repeated = digitsAndSeps.indexOf('.') !== lastDot - const grouping = - repeated || loneSeparatorIsGrouping(digitsAndSeps, '.', hadMarker, currencyCode) + const grouping = repeated || loneSeparatorIsGrouping(digitsAndSeps, '.', currencyCode) if (grouping) { if (!hasValidGrouping(digitsAndSeps, '.')) return null normalized = digitsAndSeps.split('.').join('') @@ -299,30 +293,32 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number /** * Whether a lone separator followed by exactly three digits is grouping rather - * than a decimal point — `1.235 ¥` is one thousand two hundred thirty-five yen, - * while a typed `1.235` is one and a bit. + * than a decimal point. * - * Two signals resolve it. A marker means the string came from a formatter, and - * formatters group; a bare string was typed by a person, who meant decimals. - * And a currency with three decimal places (KWD, TND) legitimately ends in - * three digits after its separator, so for those the reading is always decimal. + * The separator decides, tempered by what the currency can express: + * + * - A **dot** is the decimal point in the notation most people type, so it + * stays a decimal — typing `1.234` in a USD cell means 1.234, which the + * display then rounds to `$1.23`, exactly as a spreadsheet does. Reading it + * as 1,234 would be a thousandfold surprise. The exception is a currency with + * no decimal places at all: `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, not one + * and a half. The exception mirrors the first: a currency with three decimal + * places (KWD, TND) legitimately ends in three digits after its separator, + * so `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. */ function loneSeparatorIsGrouping( digitsAndSeps: string, separator: string, - hadMarker: boolean, currencyCode: string | undefined ): boolean { if (!new RegExp(`\\${separator}\\d{3}$`).test(digitsAndSeps)) return false - // A currency with three decimal places (KWD, TND) legitimately ends in three - // digits after its separator, so for those the reading is always decimal — - // regardless of how the value arrived, since a CSV import carries no marker. - if (currencyFractionDigits(currencyCode) === 3) return false - // A comma tail of exactly three digits is grouping by convention (`1,500`). - if (separator === ',') return true - // A dot is the decimal point in the notation most users type, so it only - // reads as grouping when a marker shows a formatter produced the string. - return hadMarker + const fractionDigits = currencyFractionDigits(currencyCode) + return separator === ',' ? fractionDigits !== 3 : fractionDigits === 0 } /** A currency's conventional decimal places, defaulting to 2 when unknown. */ From c64d42c0e725f24ea3aed044d8e5fe6e0832a518 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 30 Jul 2026 18:22:12 -0700 Subject: [PATCH 5/7] fix(tables): check conversions against the target column, not a rebuilt stub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../table/__tests__/column-conversion.test.ts | 25 ++++++++++- apps/sim/lib/table/columns/service.ts | 43 +++++++------------ 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/apps/sim/lib/table/__tests__/column-conversion.test.ts b/apps/sim/lib/table/__tests__/column-conversion.test.ts index 3e7c1bf8a55..83903b38761 100644 --- a/apps/sim/lib/table/__tests__/column-conversion.test.ts +++ b/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' }, diff --git a/apps/sim/lib/table/columns/service.ts b/apps/sim/lib/table/columns/service.ts index 35f0a9ba46e..2ed1dd3cbda 100644 --- a/apps/sim/lib/table/columns/service.ts +++ b/apps/sim/lib/table/columns/service.ts @@ -801,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 @@ -1330,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) } From d4148d1f020131b0e347a825f02c463158b2f365 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 30 Jul 2026 18:34:38 -0700 Subject: [PATCH 6/7] fix(tables): vet a currency marker against an allowlist, not a scale denylist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/lib/table/__tests__/currency.test.ts | 29 ++++++ apps/sim/lib/table/currency.ts | 89 +++++++++++++++---- 2 files changed, 101 insertions(+), 17 deletions(-) diff --git a/apps/sim/lib/table/__tests__/currency.test.ts b/apps/sim/lib/table/__tests__/currency.test.ts index 48f3fa82605..097030854c7 100644 --- a/apps/sim/lib/table/__tests__/currency.test.ts +++ b/apps/sim/lib/table/__tests__/currency.test.ts @@ -203,6 +203,35 @@ describe('parseCurrencyInput', () => { 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 diff --git a/apps/sim/lib/table/currency.ts b/apps/sim/lib/table/currency.ts index 6e7afe11fe9..03f6c6ecf78 100644 --- a/apps/sim/lib/table/currency.ts +++ b/apps/sim/lib/table/currency.ts @@ -25,24 +25,80 @@ const BIDI_MARKS = /[\u200e\u200f\u061c\u202a-\u202e\u2066-\u2069]/g const UNICODE_MINUS = /[\u2212\u2012\u2013\uFE63\uFF0D]/g /** - * A magnitude suffix (`1.2 M`, `5 K`, `3.4 bn`). Not a currency marker, and - * stripping it would silently shrink the value by orders of magnitude. + * A magnitude suffix (`1.2 M`, `5 K`, `3.4 bn`), used only on runtimes that + * cannot enumerate currency codes \u2014 see {@link isCurrencyMarkerLetters}. Every + * other runtime rejects these by not recognising them, so this list does not + * have to be complete. */ -const SCALE_SUFFIX = /\d\s*(?:k|m|b|t|bn|mn|tn|mm|mil|bil)\.?\s*$/i +const SCALE_SUFFIX = /^(?:k|m|b|t|bn|mn|tn|mm|mil|bil|mio|mrd|bio|tsd|mln|md)$/i /** A letter directly adjacent to a digit: an identifier, not an amount. */ const LETTER_TOUCHING_DIGIT = /\p{L}\d|\d\p{L}/u +/** + * Currency markers people type that are not ISO 4217 codes. Upper-cased for + * comparison; `\u0141` and `\u010c` upper-case as expected. + */ +const NON_ISO_CURRENCY_MARKERS = new Set([ + 'KR', + 'Z\u0141', + 'K\u010c', + 'LEI', + 'LV', + 'FT', + 'KN', + 'RM', + 'RP', + 'TL', + 'DIN', + 'SR', +]) + +/** + * Whether a bare letter token sitting beside a number is a currency marker. + * + * An allowlist, deliberately. The alternative is a denylist of everything that + * is *not* a currency, which has to guess every magnitude abbreviation in every + * language \u2014 `mio`, `mrd`, `tsd`, `mln`, `bio` \u2014 and silently divides the value + * by a million for each one it has not thought of. An unknown token now simply + * fails to strip, so the leftover letters fail {@link AMOUNT_SHAPE} and the + * input is rejected rather than quietly rescaled. + * + * No ISO code collides with a magnitude abbreviation, so nothing legitimate is + * lost \u2014 `lib/table/__tests__/currency.test.ts` pins that. + * + * A runtime that cannot enumerate codes keeps the old permissive behaviour, + * minus the magnitude words it knows by name; rejecting every letter marker + * there would break `USD 12.50` outright. + */ +function isCurrencyMarkerLetters(letters: string): boolean { + const upper = letters.toUpperCase() + if (NON_ISO_CURRENCY_MARKERS.has(upper)) return true + if (supportedCurrencyCodes === null) return !SCALE_SUFFIX.test(upper) + return supportedCurrencyCodes.has(upper) +} + /** * A leading currency marker: up to three letters and/or a symbol, optionally - * behind a sign. The sign is captured so `-$12.50` keeps it. + * behind a sign. The sign is captured so `-$12.50` keeps it, and the letters + * are captured so {@link isCurrencyMarkerLetters} can vet them. */ const CURRENCY_MARKER_PREFIX = - /^[\s\u00a0\u202f]*([+-]?)[\s\u00a0\u202f]*(?:\p{Sc}\p{L}{0,3}|\p{L}{1,3}\p{Sc}?)[\s\u00a0\u202f]*/u + /^[\s\u00a0\u202f]*([+-]?)[\s\u00a0\u202f]*(?:\p{Sc}\p{L}{0,3}|(\p{L}{1,3})(\p{Sc})?)[\s\u00a0\u202f]*/u /** The same, trailing. */ const CURRENCY_MARKER_SUFFIX = - /[\s\u00a0\u202f]*(?:\p{Sc}\p{L}{0,3}|\p{L}{1,3}\p{Sc}?)\.?[\s\u00a0\u202f]*$/u + /[\s\u00a0\u202f]*(?:\p{Sc}\p{L}{0,3}|(\p{L}{1,3})(\p{Sc})?)\.?[\s\u00a0\u202f]*$/u + +/** + * Whether a matched marker may be stripped. A marker carrying a currency + * SYMBOL needs no vetting — no magnitude abbreviation contains one — and + * `letters === undefined` means the symbol-led alternative matched. + */ +function isStrippableMarker(letters: string | undefined, symbol: string | undefined): boolean { + if (letters === undefined || symbol !== undefined) return true + return isCurrencyMarkerLetters(letters) +} /** Only digits and separators, with at least one digit. */ const AMOUNT_SHAPE = /^[+-]?[\d.,]*\d[\d.,]*$/ @@ -233,18 +289,17 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number // by a space or a symbol, so this distinguishes them without a symbol list. if (LETTER_TOUCHING_DIGIT.test(body)) return null - // A scale suffix is not a currency marker, and dropping it loses orders of - // magnitude: `1.2 M` would read as 1.2, so a column of `1.2 M` / `3.4 M` - // would convert cleanly and rewrite every cell a millionfold too small. - // Refuse rather than guess what the writer meant. - if (SCALE_SUFFIX.test(body)) return null - // Strip the currency marker: up to three letters (an ISO code, `kr`, `zł`) - // optionally joined to a symbol (`R$`, `CHF`), at either end. Bounded at - // three so prose does not qualify — `Revenue 5` keeps its letters and is - // rejected below. - const withoutPrefix = body.replace(CURRENCY_MARKER_PREFIX, '$1') - const withoutMarkers = withoutPrefix.replace(CURRENCY_MARKER_SUFFIX, '') + // optionally joined to a symbol (`R$`, `CHF`), at either end. Only a marker + // that actually NAMES a currency is stripped — a magnitude word like `1.2 M` + // or `3,4 mrd` is left in place, so it fails the amount-shape check below + // instead of silently reading as 1.2, orders of magnitude too small. + const withoutPrefix = body.replace(CURRENCY_MARKER_PREFIX, (match, sign, letters, symbol) => + isStrippableMarker(letters, symbol) ? sign : match + ) + const withoutMarkers = withoutPrefix.replace(CURRENCY_MARKER_SUFFIX, (match, letters, symbol) => + isStrippableMarker(letters, symbol) ? '' : match + ) const cleaned = withoutMarkers.replace(/[\s\u00a0\u202f\u2019']/gu, '') // What remains must be ONLY digits and separators. Anything else — a // US-format date, leftover prose — is not an amount. From 66d5479f8574dedeb2d16066b4ba94bbf536e788 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 30 Jul 2026 18:44:44 -0700 Subject: [PATCH 7/7] fix(tables): group a lone dot only when a zero-decimal amount came formatted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/lib/table/__tests__/currency.test.ts | 47 ++++++++--------- apps/sim/lib/table/currency.ts | 51 ++++++++++++------- 2 files changed, 57 insertions(+), 41 deletions(-) diff --git a/apps/sim/lib/table/__tests__/currency.test.ts b/apps/sim/lib/table/__tests__/currency.test.ts index 097030854c7..836200fe5c8 100644 --- a/apps/sim/lib/table/__tests__/currency.test.ts +++ b/apps/sim/lib/table/__tests__/currency.test.ts @@ -160,36 +160,37 @@ describe('parseCurrencyInput', () => { expect(parseCurrencyInput('CHF 1’234.56')).toBe(1234.56) }) - it('resolves a lone separator using the marker and the currency', () => { - // `1.235 ¥` is 1235 yen; a typed `1.235` is one-and-a-bit. A marker means a - // formatter produced it, and formatters group — except for the currencies - // that genuinely carry three decimals. - expect(parseCurrencyInput('1.235 ¥', 'JPY')).toBe(1235) - expect(parseCurrencyInput('0,500 KWD', 'KWD')).toBe(0.5) - expect(parseCurrencyInput('12,000 TND', 'TND')).toBe(12) - // Bare input keeps the typed reading. + 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,500')).toBe(1500) + expect(parseCurrencyInput('1.234', 'EUR')).toBe(1.234) }) - it('reads a lone separator against the currency, not just the marker', () => { - // A currency with decimals always formats with BOTH separators, so a lone - // one never comes from a formatter for those — only zero-decimal - // currencies produce `1.235 ¥`. And a three-decimal currency's trailing - // three digits are decimals however the value arrived, which matters for - // CSV import, where nothing carries a marker. - // A dot stays the decimal point: 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. - expect(parseCurrencyInput('$1.234', 'USD')).toBe(1.234) - expect(parseCurrencyInput('1.234')).toBe(1.234) - // Except where the currency has no decimals — the one lone-separator form - // a formatter actually emits. + 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) - // A comma is grouping by convention, except for three-decimal currencies. + 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', () => { diff --git a/apps/sim/lib/table/currency.ts b/apps/sim/lib/table/currency.ts index 03f6c6ecf78..16a508c4957 100644 --- a/apps/sim/lib/table/currency.ts +++ b/apps/sim/lib/table/currency.ts @@ -294,12 +294,19 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number // that actually NAMES a currency is stripped — a magnitude word like `1.2 M` // or `3,4 mrd` is left in place, so it fails the amount-shape check below // instead of silently reading as 1.2, orders of magnitude too small. - const withoutPrefix = body.replace(CURRENCY_MARKER_PREFIX, (match, sign, letters, symbol) => - isStrippableMarker(letters, symbol) ? sign : match - ) - const withoutMarkers = withoutPrefix.replace(CURRENCY_MARKER_SUFFIX, (match, letters, symbol) => - isStrippableMarker(letters, symbol) ? '' : match - ) + // Whether a marker was actually stripped decides one ambiguous case below — + // see {@link loneSeparatorIsGrouping}. + let hadMarker = false + const withoutPrefix = body.replace(CURRENCY_MARKER_PREFIX, (match, sign, letters, symbol) => { + if (!isStrippableMarker(letters, symbol)) return match + hadMarker = true + return sign + }) + const withoutMarkers = withoutPrefix.replace(CURRENCY_MARKER_SUFFIX, (match, letters, symbol) => { + if (!isStrippableMarker(letters, symbol)) return match + hadMarker = true + return '' + }) const cleaned = withoutMarkers.replace(/[\s\u00a0\u202f\u2019']/gu, '') // What remains must be ONLY digits and separators. Anything else — a // US-format date, leftover prose — is not an amount. @@ -321,7 +328,8 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number normalized = `${integerPart.split(groupSeparator).join('')}.${decimalParts[0]}` } else if (lastComma !== -1) { const repeated = digitsAndSeps.indexOf(',') !== lastComma - const grouping = repeated || loneSeparatorIsGrouping(digitsAndSeps, ',', currencyCode) + const grouping = + repeated || loneSeparatorIsGrouping(digitsAndSeps, ',', currencyCode, hadMarker) if (grouping) { if (!hasValidGrouping(digitsAndSeps, ',')) return null normalized = digitsAndSeps.split(',').join('') @@ -330,7 +338,8 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number } } else if (lastDot !== -1) { const repeated = digitsAndSeps.indexOf('.') !== lastDot - const grouping = repeated || loneSeparatorIsGrouping(digitsAndSeps, '.', currencyCode) + const grouping = + repeated || loneSeparatorIsGrouping(digitsAndSeps, '.', currencyCode, hadMarker) if (grouping) { if (!hasValidGrouping(digitsAndSeps, '.')) return null normalized = digitsAndSeps.split('.').join('') @@ -355,25 +364,31 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number * - A **dot** is the decimal point in the notation most people type, so it * stays a decimal — typing `1.234` in a USD cell means 1.234, which the * display then rounds to `$1.23`, exactly as a spreadsheet does. Reading it - * as 1,234 would be a thousandfold surprise. The exception is a currency with - * no decimal places at all: `1.235 ¥` cannot be a fraction of a yen, and is - * the one lone-separator form a formatter actually emits. + * as 1,234 would be a thousandfold surprise. * - A **comma** is grouping by convention — `1,500` is fifteen hundred, not one - * and a half. The exception mirrors the first: a currency with three decimal - * places (KWD, TND) legitimately ends in three digits after its separator, - * so `0,500` is a half. + * and a half. The exception: a currency with three decimal places (KWD, TND) + * legitimately ends in three digits after its separator, so `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. + * The one case where a dot groups is a currency with NO decimal places, and + * only when a currency marker came with it. `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. A bare `1.235` is someone typing, and inflating + * that to 1235 is a silent thousandfold error, where reading it literally stores + * 1.235 and displays `¥1` — wrong in a way the writer can see and correct. + * + * The marker carries no information for any other currency: one with one or two + * decimal places always formats with BOTH separators, so a lone separator there + * never came from a formatter in the first place. */ function loneSeparatorIsGrouping( digitsAndSeps: string, separator: string, - currencyCode: string | undefined + currencyCode: string | undefined, + hadMarker: boolean ): boolean { if (!new RegExp(`\\${separator}\\d{3}$`).test(digitsAndSeps)) return false const fractionDigits = currencyFractionDigits(currencyCode) - return separator === ',' ? fractionDigits !== 3 : fractionDigits === 0 + return separator === ',' ? fractionDigits !== 3 : fractionDigits === 0 && hadMarker } /** A currency's conventional decimal places, defaulting to 2 when unknown. */