Skip to content

Commit 33286cf

Browse files
committed
fix(tables): score CSV delimiter by width*consistency and dedupe headers
Addresses round-2 review findings: - Score each delimiter candidate by modalWidth * consistency instead of consistency alone. Consistency-only let a separator that appears uniformly inside values (e.g. a pipe once per row) beat a real delimiter whose rows are legitimately ragged; the product rewards a split that is both wide and uniform, with width breaking ties. Genuinely symmetric files (two valid columns under either separator) fall back to the global-default candidate order. - Dedupe the captured header row to match the parser's record keys. csv-parse collapses duplicate column names onto one key (last value wins), so reporting the raw duplicates made schema inference invent a phantom empty column and could fail mapping validation. dedupeHeaders keeps first-occurrence order, case-sensitively, matching the emitted keys.
1 parent a6cc15c commit 33286cf

2 files changed

Lines changed: 76 additions & 17 deletions

File tree

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
coerceValue,
1212
createCsvParser,
1313
csvParseOptions,
14+
dedupeHeaders,
1415
detectCsvDelimiter,
1516
inferColumnType,
1617
inferSchemaFromCsv,
@@ -348,6 +349,26 @@ describe('import', () => {
348349
const { columns } = inferSchemaFromCsv(headers, rows)
349350
expect(columns.map((c) => c.name)).toEqual(['a', 'b', 'c'])
350351
})
352+
353+
it('collapses duplicate header names to match the parser record keys', async () => {
354+
// csv-parse keys duplicate columns onto one object key (last value wins); the reported
355+
// headers must match, so schema inference does not invent a phantom empty column.
356+
const { headers, rows } = await parseCsvBuffer('a,a,b\n1,2,3\n4,5,6\n')
357+
expect(headers).toEqual(['a', 'b'])
358+
expect(Object.keys(rows[0])).toEqual(['a', 'b'])
359+
const { columns } = inferSchemaFromCsv(headers, rows)
360+
expect(columns.map((c) => c.name)).toEqual(['a', 'b'])
361+
})
362+
})
363+
364+
describe('dedupeHeaders', () => {
365+
it('drops later exact duplicates, preserving first-occurrence order', () => {
366+
expect(dedupeHeaders(['a', 'b', 'a', 'c', 'b'])).toEqual(['a', 'b', 'c'])
367+
})
368+
369+
it('keeps case-distinct names (matches csv-parse key sensitivity)', () => {
370+
expect(dedupeHeaders(['Name', 'name'])).toEqual(['Name', 'name'])
371+
})
351372
})
352373

353374
describe('detectCsvDelimiter', () => {
@@ -387,6 +408,18 @@ describe('import', () => {
387408
expect(await detectCsvDelimiter(csv)).toBe(';')
388409
})
389410

411+
it('prefers a wider ragged split over a narrow one that only looks uniform', async () => {
412+
// Semicolon is the real delimiter (2- and 3-column rows) while a pipe appears once per
413+
// row inside values. Scoring width*consistency keeps the wider semicolon split.
414+
expect(await detectCsvDelimiter('a|label;b;c\nx|y;1\nz|w;2;3\n')).toBe(';')
415+
})
416+
417+
it('falls back to comma order only when the split is genuinely ambiguous', async () => {
418+
// Both ; and , yield exactly two uniform columns — no content signal distinguishes them,
419+
// so the global-default candidate order (comma first) decides and either reading is valid.
420+
expect(await detectCsvDelimiter('name;value,unit\na;1,kg\nb;2,kg\n')).toBe(',')
421+
})
422+
390423
it('drops a partial trailing line so a mid-record byte cut cannot skew detection', async () => {
391424
// Simulates a fixed byte-window slice that ends mid-record; the last (partial) line
392425
// is ignored, so the complete rows above still drive the result.

apps/sim/lib/table/import.ts

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,12 @@ export function csvParseOptions(
4646
): CsvParseOptions {
4747
return {
4848
columns: (header: string[]) => {
49-
onHeaders?.(header)
49+
// Deliver headers deduped to match the record keys: csv-parse collapses duplicate
50+
// column names into a single object key (last value wins), so the consumer's schema
51+
// inference and mapping must see the same unique set — not the raw duplicates, which
52+
// would invent phantom columns that never receive a value. csv-parse still keys on the
53+
// raw array we return here, so returning it unchanged preserves that collapsing.
54+
onHeaders?.(dedupeHeaders(header))
5055
return header
5156
},
5257
skip_empty_lines: true,
@@ -72,6 +77,22 @@ export function createCsvParser(delimiter = ',', onHeaders?: (headers: string[])
7277
return parseCsvStream(csvParseOptions(delimiter, onHeaders))
7378
}
7479

80+
/**
81+
* Drops later exact-duplicate header names, preserving first-occurrence order. Mirrors how
82+
* `csv-parse` collapses duplicate column names into a single record key (last value wins), so
83+
* the header set stays in lockstep with the object keys the parser actually emits.
84+
*/
85+
export function dedupeHeaders(headers: string[]): string[] {
86+
const seen = new Set<string>()
87+
const unique: string[] = []
88+
for (const header of headers) {
89+
if (seen.has(header)) continue
90+
seen.add(header)
91+
unique.push(header)
92+
}
93+
return unique
94+
}
95+
7596
/** Decodes CSV bytes as UTF-8, passing strings through unchanged. */
7697
export function decodeCsvText(input: Buffer | Uint8Array | string): string {
7798
if (typeof input === 'string') return input
@@ -88,14 +109,22 @@ export function decodeCsvText(input: Buffer | Uint8Array | string): string {
88109
* semicolon-separated file whose `raw_text` cells are full of commas and
89110
* newlines, and a naive frequency count picks the comma.
90111
*
91-
* Ranking is **consistency first, then column count**. The true delimiter yields
92-
* a uniform column count across every row; a spurious one (e.g. a comma that
93-
* happens to appear in an unquoted header of a semicolon file) produces a wide
94-
* first row but ragged data rows. Ranking on column count alone would wrongly
95-
* prefer that wide-but-inconsistent split, so consistency dominates and column
96-
* count only breaks ties. The per-candidate column count is the *modal* (most
97-
* common) row width, so one stray ragged row doesn't distort it. A single-column
98-
* file (no candidate reaches two columns) falls back to `fallback`.
112+
* Each candidate is scored `modalWidth × consistency` — the modal (most common)
113+
* row width times the fraction of rows at that width. This balances the two
114+
* failure modes: ranking on width alone lets a delimiter that merely widens the
115+
* header win (a comma splitting a semicolon file's unquoted header), while
116+
* ranking on consistency alone lets a separator that appears uniformly *inside*
117+
* values win over a real delimiter whose rows are legitimately ragged (a pipe
118+
* embedded once per row beating a semicolon that yields 2- and 3-column rows).
119+
* The product rewards a split that is both wide and uniform; ties break toward
120+
* the wider split, then toward candidate order. Using the modal width (not the
121+
* first row's) keeps one stray ragged row from distorting the score, and a
122+
* single-column file (no candidate reaches two columns) falls back to `fallback`.
123+
*
124+
* Files that stay exactly tied on both score and width are genuinely ambiguous —
125+
* e.g. `name;value,unit` splits into two columns under either `;` or `,` — so the
126+
* global-default candidate order (comma first) decides, and both readings still
127+
* produce a valid table.
99128
*
100129
* All callers funnel through here, so the sample is prepared identically for
101130
* every import path: a possibly-partial trailing line (from slicing a fixed byte
@@ -114,7 +143,7 @@ export async function detectCsvDelimiter(
114143
const text = lastNewline > 0 ? decoded.slice(0, lastNewline + 1) : decoded
115144
if (text.trim() === '') return fallback
116145

117-
let best: { delimiter: CsvDelimiter; fields: number; consistency: number } | null = null
146+
let best: { delimiter: CsvDelimiter; fields: number; score: number } | null = null
118147

119148
for (const delimiter of CSV_DELIMITER_CANDIDATES) {
120149
let records: string[][]
@@ -149,14 +178,11 @@ export async function detectCsvDelimiter(
149178
}
150179
if (fields < 2) continue
151180

152-
const consistency = modalFreq / records.length
181+
// Reward a split that is both wide and uniform; ties break toward the wider split.
182+
const score = fields * (modalFreq / records.length)
153183

154-
if (
155-
!best ||
156-
consistency > best.consistency ||
157-
(consistency === best.consistency && fields > best.fields)
158-
) {
159-
best = { delimiter, fields, consistency }
184+
if (!best || score > best.score || (score === best.score && fields > best.fields)) {
185+
best = { delimiter, fields, score }
160186
}
161187
}
162188

0 commit comments

Comments
 (0)