Skip to content

Commit 66d5479

Browse files
committed
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.
1 parent d4148d1 commit 66d5479

2 files changed

Lines changed: 57 additions & 41 deletions

File tree

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

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -160,36 +160,37 @@ describe('parseCurrencyInput', () => {
160160
expect(parseCurrencyInput('CHF 1’234.56')).toBe(1234.56)
161161
})
162162

163-
it('resolves a lone separator using the marker and the currency', () => {
164-
// `1.235 ¥` is 1235 yen; a typed `1.235` is one-and-a-bit. A marker means a
165-
// formatter produced it, and formatters group — except for the currencies
166-
// that genuinely carry three decimals.
167-
expect(parseCurrencyInput('1.235 ¥', 'JPY')).toBe(1235)
168-
expect(parseCurrencyInput('0,500 KWD', 'KWD')).toBe(0.5)
169-
expect(parseCurrencyInput('12,000 TND', 'TND')).toBe(12)
170-
// Bare input keeps the typed reading.
163+
it('reads a lone dot as the decimal point people type', () => {
164+
// Typing `1.234` in a USD cell means 1.234, which the display rounds to
165+
// $1.23 exactly as a spreadsheet does. Reading it as 1,234 would be a
166+
// thousandfold surprise, marker or no marker.
167+
expect(parseCurrencyInput('$1.234', 'USD')).toBe(1.234)
171168
expect(parseCurrencyInput('1.234')).toBe(1.234)
172-
expect(parseCurrencyInput('1,500')).toBe(1500)
169+
expect(parseCurrencyInput('1.234', 'EUR')).toBe(1.234)
173170
})
174171

175-
it('reads a lone separator against the currency, not just the marker', () => {
176-
// A currency with decimals always formats with BOTH separators, so a lone
177-
// one never comes from a formatter for those — only zero-decimal
178-
// currencies produce `1.235 ¥`. And a three-decimal currency's trailing
179-
// three digits are decimals however the value arrived, which matters for
180-
// CSV import, where nothing carries a marker.
181-
// A dot stays the decimal point: typing `1.234` in a USD cell means 1.234,
182-
// which the display rounds to $1.23 exactly as a spreadsheet does. Reading
183-
// it as 1,234 would be a thousandfold surprise.
184-
expect(parseCurrencyInput('$1.234', 'USD')).toBe(1.234)
185-
expect(parseCurrencyInput('1.234')).toBe(1.234)
186-
// Except where the currency has no decimals — the one lone-separator form
187-
// a formatter actually emits.
172+
it('groups a lone dot only for a zero-decimal currency that came formatted', () => {
173+
// `1.235 ¥` cannot be a fraction of a yen, and is the single lone-separator
174+
// form a formatter emits — but a formatter always emits its marker too.
188175
expect(parseCurrencyInput('1.235 ¥', 'JPY')).toBe(1235)
189-
// A comma is grouping by convention, except for three-decimal currencies.
176+
expect(parseCurrencyInput('JPY 1.235', 'JPY')).toBe(1235)
177+
// Bare, it is someone typing. Inflating that to 1235 would be a silent
178+
// thousandfold error; read literally it stores 1.235 and displays ¥1 —
179+
// wrong in a way the writer can see and fix.
180+
expect(parseCurrencyInput('1.235', 'JPY')).toBe(1.235)
181+
expect(parseCurrencyInput('1.235', 'CLP')).toBe(1.235)
182+
})
183+
184+
it('reads a lone comma as grouping, except where the currency has three decimals', () => {
185+
// `1,500` is fifteen hundred by convention. A three-decimal currency's
186+
// trailing three digits are decimals however the value arrived — which
187+
// matters most for CSV import, where nothing carries a marker.
190188
expect(parseCurrencyInput('1,500', 'USD')).toBe(1500)
189+
expect(parseCurrencyInput('1,500')).toBe(1500)
191190
expect(parseCurrencyInput('0,500', 'KWD')).toBe(0.5)
191+
expect(parseCurrencyInput('0,500 KWD', 'KWD')).toBe(0.5)
192192
expect(parseCurrencyInput('12,000', 'TND')).toBe(12)
193+
expect(parseCurrencyInput('12,000 TND', 'TND')).toBe(12)
193194
})
194195

195196
it('refuses a scale suffix rather than shrinking the value', () => {

apps/sim/lib/table/currency.ts

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -294,12 +294,19 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number
294294
// that actually NAMES a currency is stripped — a magnitude word like `1.2 M`
295295
// or `3,4 mrd` is left in place, so it fails the amount-shape check below
296296
// instead of silently reading as 1.2, orders of magnitude too small.
297-
const withoutPrefix = body.replace(CURRENCY_MARKER_PREFIX, (match, sign, letters, symbol) =>
298-
isStrippableMarker(letters, symbol) ? sign : match
299-
)
300-
const withoutMarkers = withoutPrefix.replace(CURRENCY_MARKER_SUFFIX, (match, letters, symbol) =>
301-
isStrippableMarker(letters, symbol) ? '' : match
302-
)
297+
// Whether a marker was actually stripped decides one ambiguous case below —
298+
// see {@link loneSeparatorIsGrouping}.
299+
let hadMarker = false
300+
const withoutPrefix = body.replace(CURRENCY_MARKER_PREFIX, (match, sign, letters, symbol) => {
301+
if (!isStrippableMarker(letters, symbol)) return match
302+
hadMarker = true
303+
return sign
304+
})
305+
const withoutMarkers = withoutPrefix.replace(CURRENCY_MARKER_SUFFIX, (match, letters, symbol) => {
306+
if (!isStrippableMarker(letters, symbol)) return match
307+
hadMarker = true
308+
return ''
309+
})
303310
const cleaned = withoutMarkers.replace(/[\s\u00a0\u202f\u2019']/gu, '')
304311
// What remains must be ONLY digits and separators. Anything else — a
305312
// US-format date, leftover prose — is not an amount.
@@ -321,7 +328,8 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number
321328
normalized = `${integerPart.split(groupSeparator).join('')}.${decimalParts[0]}`
322329
} else if (lastComma !== -1) {
323330
const repeated = digitsAndSeps.indexOf(',') !== lastComma
324-
const grouping = repeated || loneSeparatorIsGrouping(digitsAndSeps, ',', currencyCode)
331+
const grouping =
332+
repeated || loneSeparatorIsGrouping(digitsAndSeps, ',', currencyCode, hadMarker)
325333
if (grouping) {
326334
if (!hasValidGrouping(digitsAndSeps, ',')) return null
327335
normalized = digitsAndSeps.split(',').join('')
@@ -330,7 +338,8 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number
330338
}
331339
} else if (lastDot !== -1) {
332340
const repeated = digitsAndSeps.indexOf('.') !== lastDot
333-
const grouping = repeated || loneSeparatorIsGrouping(digitsAndSeps, '.', currencyCode)
341+
const grouping =
342+
repeated || loneSeparatorIsGrouping(digitsAndSeps, '.', currencyCode, hadMarker)
334343
if (grouping) {
335344
if (!hasValidGrouping(digitsAndSeps, '.')) return null
336345
normalized = digitsAndSeps.split('.').join('')
@@ -355,25 +364,31 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number
355364
* - A **dot** is the decimal point in the notation most people type, so it
356365
* stays a decimal — typing `1.234` in a USD cell means 1.234, which the
357366
* display then rounds to `$1.23`, exactly as a spreadsheet does. Reading it
358-
* as 1,234 would be a thousandfold surprise. The exception is a currency with
359-
* no decimal places at all: `1.235 ¥` cannot be a fraction of a yen, and is
360-
* the one lone-separator form a formatter actually emits.
367+
* as 1,234 would be a thousandfold surprise.
361368
* - A **comma** is grouping by convention — `1,500` is fifteen hundred, not one
362-
* and a half. The exception mirrors the first: a currency with three decimal
363-
* places (KWD, TND) legitimately ends in three digits after its separator,
364-
* so `0,500` is a half.
369+
* and a half. The exception: a currency with three decimal places (KWD, TND)
370+
* legitimately ends in three digits after its separator, so `0,500` is a half.
365371
*
366-
* A currency with one or two decimal places always formats with BOTH
367-
* separators, so no formatter output is decided here.
372+
* The one case where a dot groups is a currency with NO decimal places, and
373+
* only when a currency marker came with it. `1.235 ¥` cannot be a fraction of a
374+
* yen, and is the single lone-separator form a formatter emits — but a formatter
375+
* always emits its marker too. A bare `1.235` is someone typing, and inflating
376+
* that to 1235 is a silent thousandfold error, where reading it literally stores
377+
* 1.235 and displays `¥1` — wrong in a way the writer can see and correct.
378+
*
379+
* The marker carries no information for any other currency: one with one or two
380+
* decimal places always formats with BOTH separators, so a lone separator there
381+
* never came from a formatter in the first place.
368382
*/
369383
function loneSeparatorIsGrouping(
370384
digitsAndSeps: string,
371385
separator: string,
372-
currencyCode: string | undefined
386+
currencyCode: string | undefined,
387+
hadMarker: boolean
373388
): boolean {
374389
if (!new RegExp(`\\${separator}\\d{3}$`).test(digitsAndSeps)) return false
375390
const fractionDigits = currencyFractionDigits(currencyCode)
376-
return separator === ',' ? fractionDigits !== 3 : fractionDigits === 0
391+
return separator === ',' ? fractionDigits !== 3 : fractionDigits === 0 && hadMarker
377392
}
378393

379394
/** A currency's conventional decimal places, defaulting to 2 when unknown. */

0 commit comments

Comments
 (0)