FIX: Strip UTF-8 BOM in csv converter to prevent parse errors - #81
Merged
Conversation
Dnyaneshwar Mane (dnyaneshwarmane7)
requested a review
from a team
as a code owner
July 28, 2026 14:29
Copilot started reviewing on behalf of
Dnyaneshwar Mane (dnyaneshwarmane7)
July 28, 2026 14:30
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets CSV ingestion reliability by stripping a UTF-8 BOM from incoming CSV data to avoid parse issues (notably on the header row) in the pipeline task converter.
Changes:
- Added a
prepareCSVLinehelper to strip a UTF-8 BOM (and currently whitespace) from CSV input. - Applied BOM/line preparation when initializing columns from the first CSV row.
- Reworked header sanitization logic to use an in-file regex-based approach.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Yash Shrivastava (alephys26)
left a comment
Contributor
There was a problem hiding this comment.
Lets remove the regexp work here and stick to textutil.
The BOM was only stripped in initializeColumns, but convert() builds its record-parsing reader over the raw data. With skip_first: false the first row is parsed as data, so the BOM still reached that reader and either failed with `bare " in non-quoted-field` or embedded the BOM in the first value. Add fixtures for the two paths a BOM can reach: header-derived column names and first-row-as-data. Document the CSV header normalization contract, which was previously unstated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
stripUTF8BOMAndWhitespace's TrimPrefix could never match: convert() strips the BOM before initializeColumns is reached. Only the whitespace trim was still doing work, so inline it and note why it is there — it rescues a header row whose quoted first field is preceded by stray whitespace, which csv.Reader otherwise rejects the same way it rejects a BOM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Divyanshu Tiwari (divyanshu-tiwari)
requested a review
from Yash Shrivastava (alephys26)
July 28, 2026 16:05
Divyanshu Tiwari (divyanshu-tiwari)
approved these changes
Jul 28, 2026
Yash Shrivastava (alephys26)
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem
The CSV converter task failed when processing files that start with a UTF-8 byte-order mark (BOM), which is common in S3/spreadsheet exports. The BOM bytes get attached to the first field of the header row, causing Go's
csv.Readerto choke:Fix
prepareCSVLine, a small helper that strips a leading UTF-8 BOM (0xEF 0xBB 0xBF) and trims surrounding whitespace before the data is handed tocsv.Reader. This is applied when initializing columns from the header row (and when parsing records), so BOM-prefixed headers no longer break parsing.textutil.Slugifydependency for column-name sanitization with an inline, pre-compiled regex ([^a-zA-Z0-9]+→_, lowercased). This removes the external dependency and keeps sanitization behavior self-contained within the converter.Impact
Types of changes
Checklist