fix(tables): sniff CSV delimiter and capture the real header row on import#5888
fix(tables): sniff CSV delimiter and capture the real header row on import#5888waleedlatif1 wants to merge 7 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Header/schema inference no longer uses Reviewed by Cursor Bugbot for commit 35156cc. Configure here. |
Greptile SummaryImproves CSV imports by detecting delimiters from bounded file prefixes and preserving the parser’s complete header row.
Confidence Score: 5/5The PR appears safe to merge. No blocking failures remain. Important Files Changed
Reviews (6): Last reviewed commit: "fix(tables): observe EOF at the exact sn..." | Re-trigger Greptile |
|
@cursor review |
…mport Table CSV import derived the delimiter purely from the file extension, so semicolon-delimited exports (European-locale Excel) parsed as a single column. Header derivation also read Object.keys(rows[0]), so with relax_column_count a ragged/sparse first data row dropped its trailing columns from schema inference. - Add detectCsvDelimiter: trial-parse the head against ,/;/tab/| and pick the candidate with the most columns (tie-break on row-width consistency), quote-aware so separators inside quoted cells don't skew the guess; extension is now only the fallback - Add sniffCsvDelimiterFromStream: memory-bounded (64KB) peek-then-replay for the streaming import paths so multi-GB files sniff without buffering - Capture the true header row via the csv-parse columns callback on every path, fixing dropped columns when the first row is short - Wire into the create route, append/replace route, background runner, client dialog preview, and parseFileRows (copilot)
… buffer Addresses review findings on the delimiter sniffer: - Rank candidates by row-width consistency first (modal column count), then column count. Ranking on column count alone let a comma that appears in a semicolon file's unquoted header win over the real separator; the wide-but-ragged split now loses to the uniform one. - Move the partial-trailing-line trim into detectCsvDelimiter so every path (stream, client preview, parseFileRows) prepares the sniff sample identically and can't disagree on the delimiter. - Cap the stream peeker's detection copy at the sniff window via Buffer.concat's length arg and replay the buffered chunks by reference, so an oversized source chunk can't break the bounded-memory contract.
a5d29bc to
a6cc15c
Compare
|
@cursor review |
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.
|
@cursor review |
detectCsvDelimiter always trimmed to the last newline, so a complete file with no trailing newline lost its final data row from scoring — leaving the header alone and letting a header-widening separator beat the real one. It now takes a `complete` flag: the trim runs only for truncated prefixes, where a mid-record cut must be dropped. The stream sniffer passes it from `exhausted`, and the buffered callers (client preview, parseFileRows) pass it when the sample covers the whole file.
|
@cursor review |
Hoist the csv-parse options out of the multi-line parse() call so the `// double-cast-allowed` annotation sits directly above the `as unknown as` token — the strict boundary audit only matches an annotation within three lines of the cast. Also simplify the stream sniffer's completeness check to `exhausted` (the loop only ends early on end-of-stream, so it already means the whole file fit the sniff window).
The stream sniffer's read loop stopped as soon as the buffered size reached the sniff window, so a file whose size is exactly the window never triggered the extra read that observes end-of-stream — it was judged a truncated prefix and dropped its final newline-less row, disagreeing with the buffered callers (which mark that size complete). Read while size <= window so the boundary case sees EOF and `exhausted` (hence `complete`) is set correctly. Regression test added.
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 35156cc. Configure here.
Summary
,;\t|), quote-aware, with the extension only as a fallback.Object.keys(rows[0]); withrelax_column_counta ragged/sparse first data row dropped its trailing columns from schema inference and mapping. The true header row is now captured via thecsv-parsecolumnscallback on every path — fixes the "sparse csv didn't work" report.detectCsvDelimiter(trial-parse each candidate, pick most columns, tie-break on row-width consistency) mirrors PapaParse/csv.Sniffer, andsniffCsvDelimiterFromStream(64KB peek-then-replay) keeps the streaming paths memory-bounded on multi-GB files.parseFileRows(copilot).Type of Change
Testing
alarms-exportfile: comma → 1 column, semicolon → 34 columns / 1249 records; sniffer detects;check:api-validationpassesChecklist