Skip to content

Commit 875eef5

Browse files
j15zclaude
andcommitted
fix(tables): fix CsvColumnType type drift and a CSV number-import whitespace bug
CsvColumnType was a hand-duplicated, narrower copy of COLUMN_TYPES and had silently drifted to exclude 'select' — papered over at its one call site with an unsafe `as CsvColumnType` cast. Split it into two honestly-scoped types: ColumnType (= COLUMN_TYPES, what a CSV cell's target column can actually be) and InferableColumnType (the 4 types inferColumnType can actually guess from raw text, declared independently so it can't silently inherit types it was never taught to detect). Also fixes a real divergence found in the process: coerceValue's number case ran Number(value) without guarding whitespace-only input first, so a blank CSV cell imported as 0 instead of null. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 80642d5 commit 875eef5

2 files changed

Lines changed: 46 additions & 6 deletions

File tree

apps/sim/lib/table/import.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ describe('import', () => {
136136
expect(coerceValue('not a number', 'number')).toBeNull()
137137
})
138138

139+
it('treats a whitespace-only number cell as empty rather than 0', () => {
140+
expect(coerceValue(' ', 'number')).toBeNull()
141+
})
142+
139143
it('coerces booleans strictly', () => {
140144
expect(coerceValue('true', 'boolean')).toBe(true)
141145
expect(coerceValue('FALSE', 'boolean')).toBe(false)
@@ -150,6 +154,10 @@ describe('import', () => {
150154
)
151155
expect(coerceValue('not-a-date', 'date')).toBe('not-a-date')
152156
})
157+
158+
it('passes a select cell through as raw text — resolving to an option id happens later, at the schema level', () => {
159+
expect(coerceValue('Medium', 'select')).toBe('Medium')
160+
})
153161
})
154162

155163
describe('buildAutoMapping', () => {

apps/sim/lib/table/import.ts

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
import { type Options as CsvParseOptions, type Parser, parse as parseCsvStream } from 'csv-parse'
1515
import { getColumnId } from '@/lib/table/column-keys'
16+
import type { COLUMN_TYPES } from '@/lib/table/constants'
1617
import { type NormalizeDateCellOptions, normalizeDateCellValue } from '@/lib/table/dates'
1718
import type { ColumnDefinition, RowData, TableSchema } from '@/lib/table/types'
1819

@@ -193,8 +194,18 @@ export async function detectCsvDelimiter(
193194
return best?.delimiter ?? fallback
194195
}
195196

196-
/** Narrower type than `COLUMN_TYPES` used internally for coercion. */
197-
export type CsvColumnType = 'string' | 'number' | 'boolean' | 'date' | 'json'
197+
/**
198+
* The type of the column a CSV cell is being coerced into. Same union as
199+
* `COLUMN_TYPES` — a target column can be any type, including `select`, when
200+
* importing into an existing table. Derived from the constant rather than
201+
* hand-duplicated (as this alias previously was, which is how it silently
202+
* drifted out of sync and excluded `select`) so it can't drift again.
203+
*
204+
* Deliberately not reused for {@link inferColumnType}'s return type — "every
205+
* column type" and "every type we can guess from raw text" are different,
206+
* independently-sized questions. See `InferableColumnType`.
207+
*/
208+
export type ColumnType = (typeof COLUMN_TYPES)[number]
198209

199210
/** Number of CSV rows sampled when inferring column types for a new table. */
200211
export const CSV_SCHEMA_SAMPLE_SIZE = 100
@@ -272,12 +283,22 @@ export async function parseCsvBuffer(
272283
return { headers, rows: parsed }
273284
}
274285

286+
/**
287+
* Types `inferColumnType` can guess from raw CSV text. Independent of
288+
* `ColumnType`/`COLUMN_TYPES` on purpose: `select` can never be inferred (its
289+
* options have to be declared, not sniffed), and JSON is deliberately never
290+
* inferred either (see below) — neither exclusion should change just because
291+
* an unrelated column type is added elsewhere.
292+
*/
293+
const INFERABLE_COLUMN_TYPES = ['string', 'number', 'boolean', 'date'] as const
294+
export type InferableColumnType = (typeof INFERABLE_COLUMN_TYPES)[number]
295+
275296
/**
276297
* Infers a column type from a sample of non-empty values. Order matters: we
277298
* prefer narrower types (number > boolean > ISO date) and fall back to string.
278299
* JSON is never inferred automatically.
279300
*/
280-
export function inferColumnType(values: unknown[]): Exclude<CsvColumnType, 'json'> {
301+
export function inferColumnType(values: unknown[]): InferableColumnType {
281302
const nonEmpty = values.filter((v) => v !== null && v !== undefined && v !== '')
282303
if (nonEmpty.length === 0) return 'string'
283304

@@ -363,13 +384,18 @@ export function inferSchemaFromCsv(
363384
*/
364385
export function coerceValue(
365386
value: unknown,
366-
colType: CsvColumnType,
387+
colType: ColumnType,
367388
options?: NormalizeDateCellOptions
368389
): string | number | boolean | null | Record<string, unknown> | unknown[] {
369390
if (value === null || value === undefined || value === '') return null
370391
switch (colType) {
371392
case 'number': {
372-
const n = Number(value)
393+
// Guard the empty/whitespace case before Number() — Number(' ') is 0,
394+
// not NaN, so without this a blank CSV cell would silently import as 0
395+
// instead of null.
396+
const trimmed = String(value).trim()
397+
if (trimmed === '') return null
398+
const n = Number(trimmed)
373399
return Number.isNaN(n) ? null : n
374400
}
375401
case 'boolean': {
@@ -389,6 +415,12 @@ export function coerceValue(
389415
return String(value)
390416
}
391417
}
418+
case 'select':
419+
// A select cell's real coercion (raw text → option id) needs the
420+
// column's declared options, which this function doesn't have — it
421+
// passes the raw text through, and the schema-level coerceRowToSchema
422+
// pass (which does have the column, via the registry) resolves it.
423+
return String(value)
392424
default:
393425
return String(value)
394426
}
@@ -564,7 +596,7 @@ export function coerceRowsForTable(
564596
if (!colName) continue
565597
const col = colByName.get(colName)
566598
if (!col) continue
567-
const colType = (col.type as CsvColumnType) ?? 'string'
599+
const colType = col.type ?? 'string'
568600
coerced[getColumnId(col)] = coerceValue(value, colType, options) as RowData[string]
569601
}
570602
return coerced

0 commit comments

Comments
 (0)