From 1753bd73d02b2c3bc7b0c6742d44b52af2d2cc98 Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Fri, 23 Oct 2026 22:51:36 -0600 Subject: [PATCH] fix(csvx): skip blank records in ParseCSV, including before the header Human-maintained sources (e.g. Google Sheets exports) routinely carry stray blank rows. A blank row above the header was taken as the header, so every data row decoded to zero values; blank data rows decoded as empty records. ParseCSV now wraps the stream in a streaming csvutil.Reader (skipBlanks) that drops records whose cells are all empty or whitespace-only. Records decode one at a time with a single record buffered; delimiter and ragged-row behavior are unchanged, and all-blank input yields nil, nil. --- database/csvx/blank_test.go | 84 ++++++++++++++++++++++++++++++ database/csvx/blankstream_test.go | 85 +++++++++++++++++++++++++++++++ database/csvx/encoding.go | 52 +++++++++++++++++++ 3 files changed, 221 insertions(+) create mode 100644 database/csvx/blank_test.go create mode 100644 database/csvx/blankstream_test.go diff --git a/database/csvx/blank_test.go b/database/csvx/blank_test.go new file mode 100644 index 0000000..8846921 --- /dev/null +++ b/database/csvx/blank_test.go @@ -0,0 +1,84 @@ +package csvx + +import ( + "encoding/csv" + "strings" + "testing" +) + +type blankRec struct { + A string `json:"a"` + B string `json:"b"` +} + +func parseBlank[T any](in string) ([]T, error) { + c := csv.NewReader(strings.NewReader(in)) + c.FieldsPerRecord = -1 // ragged-tolerant, as sheet readers configure + return ParseCSV[T](c) +} + +// Leading blank rows must not be taken as the header (Google Sheets +// exports with an empty row 1). +func TestParseCSVBlankHeaderRow(t *testing.T) { + items, err := parseBlank[blankRec](",,,,\r\n\r\na,b\r\n1,x\r\n2,y\r\n") + if err != nil { + t.Fatalf("parse: %v", err) + } + want := []blankRec{{A: "1", B: "x"}, {A: "2", B: "y"}} + if len(items) != len(want) { + t.Fatalf("rows: %d, want %d (%+v)", len(items), len(want), items) + } + for i := range want { + if items[i] != want[i] { + t.Errorf("row %d: %+v, want %+v", i, items[i], want[i]) + } + } +} + +// Blank and whitespace-only data rows are skipped. +func TestParseCSVBlankDataRows(t *testing.T) { + items, err := parseBlank[blankRec]("a,b\r\n1,x\r\n\r\n ,\r\n2,y\r\n") + if err != nil { + t.Fatalf("parse: %v", err) + } + if len(items) != 2 || items[0] != (blankRec{A: "1", B: "x"}) || items[1] != (blankRec{A: "2", B: "y"}) { + t.Fatalf("rows: %+v", items) + } +} + +// Normal input is unchanged. +func TestParseCSVNoBlanks(t *testing.T) { + items, err := parseBlank[blankRec]("a,b\r\n1,x\r\n2,y\r\n") + if err != nil { + t.Fatalf("parse: %v", err) + } + if len(items) != 2 || items[1] != (blankRec{A: "2", B: "y"}) { + t.Fatalf("rows: %+v", items) + } +} + +// TSV: blank header row with tab delimiter. +func TestParseCSVBlankHeaderTSV(t *testing.T) { + in := "\t\t\r\na\tb\r\n1\tx\r\n" + c := csv.NewReader(strings.NewReader(in)) + c.Comma = '\t' + c.FieldsPerRecord = -1 + items, err := ParseCSV[blankRec](c) + if err != nil { + t.Fatalf("parse: %v", err) + } + if len(items) != 1 || items[0] != (blankRec{A: "1", B: "x"}) { + t.Fatalf("rows: %+v", items) + } +} + +// All-blank input: no header, no rows. +func TestParseCSVAllBlank(t *testing.T) { + items, err := parseBlank[blankRec]("\r\n,\r\n") + if err != nil { + t.Fatalf("parse: %v", err) + } + if items != nil { + t.Fatalf("rows: %+v, want nil", items) + } +} diff --git a/database/csvx/blankstream_test.go b/database/csvx/blankstream_test.go new file mode 100644 index 0000000..193b83e --- /dev/null +++ b/database/csvx/blankstream_test.go @@ -0,0 +1,85 @@ +package csvx + +import ( + "errors" + "io" + "testing" + + "github.com/jszwec/csvutil" +) + +// scriptedReader is a csvutil.Reader that replays a fixed sequence of +// records one Read at a time, then io.EOF. +type scriptedReader struct { + records [][]string + i int +} + +func (s *scriptedReader) Read() ([]string, error) { + if s.i >= len(s.records) { + return nil, io.EOF + } + rec := s.records[s.i] + s.i++ + return rec, nil +} + +// skipBlanks must skip blank records and pass through non-blank ones in +// order, consuming source records lazily. +func TestSkipBlanksStreaming(t *testing.T) { + w := skipBlanks{in: &scriptedReader{records: [][]string{ + {"", ""}, // blank, skipped + {"a", "b"}, + {" ", ""}, // whitespace-only, skipped + {"1", "x"}, + {"2", "y"}, + }}} + + for _, want := range [][]string{{"a", "b"}, {"1", "x"}, {"2", "y"}} { + got, err := w.Read() + if err != nil { + t.Fatalf("Read: %v", err) + } + if len(got) != len(want) { + t.Fatalf("record = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("record = %v, want %v", got, want) + } + } + } + if _, err := w.Read(); !errors.Is(err, io.EOF) { + t.Fatalf("final Read err = %v, want io.EOF", err) + } +} + +// A non-EOF error from the wrapped reader must propagate untouched, even +// when reached while skipping blanks. +func TestSkipBlanksPropagatesError(t *testing.T) { + boom := errors.New("boom") + // First read: one blank record. Second read: boom. + in := &errAfter{in: &scriptedReader{records: [][]string{{""}}}, after: 1, boom: boom} + w := skipBlanks{in: in} + + if _, err := w.Read(); err != boom { + t.Fatalf("Read err = %v, want boom", err) + } +} + +// errAfter passes reads through until n of them have happened, then a +// fixed error. +type errAfter struct { + in csvutil.Reader + after int + n int + boom error +} + +func (e *errAfter) Read() ([]string, error) { + e.n++ + if e.n > e.after { + return nil, e.boom + } + return e.in.Read() +} diff --git a/database/csvx/encoding.go b/database/csvx/encoding.go index b7743c7..45d20f8 100644 --- a/database/csvx/encoding.go +++ b/database/csvx/encoding.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "io" + "strings" "github.com/jszwec/csvutil" ) @@ -34,8 +35,59 @@ func Parse[T any](r io.Reader, format string) ([]T, error) { // ParseCSV reads CSV/TSV from a csvutil Reader using csvutil streaming. // The reader must already have its delimiter configured. // Uses json struct tags and csvx marshalers for type handling. +// +// Blank records — rows where every cell is empty or whitespace-only — +// are skipped everywhere, including before the header. Human-maintained +// sources routinely carry stray blank lines (e.g. a Google Sheets export +// with an empty row above the header); without skipping, the first blank +// row would be taken as the header and every data row would decode to +// zero values. Skipping happens in a streaming reader wrapper: records +// are decoded one at a time and only one record is buffered at a time. +// +// A csv.Reader is a valid csvutil.Reader and can be passed directly. func ParseCSV[T any](r csvutil.Reader) ([]T, error) { + return parseDecoded[T](skipBlanks{in: r}) +} + +// skipBlanks is a streaming csvutil.Reader wrapper that drops blank +// records. It implements the csvutil.Reader interface (a single Read +// method) so *encoding/csv.Reader and other csvutil readers can be +// wrapped without concrete types. +type skipBlanks struct { + in csvutil.Reader +} + +func (s skipBlanks) Read() ([]string, error) { + for { + rec, err := s.in.Read() + if err != nil { + // io.EOF and other errors propagate; only blank + // records (err == nil) are skipped. + return rec, err + } + if !isBlankRecord(rec) { + return rec, nil + } + } +} + +// isBlankRecord reports whether every cell is empty or whitespace-only. +func isBlankRecord(rec []string) bool { + for _, f := range rec { + if strings.TrimSpace(f) != "" { + return false + } + } + return true +} + +// parseDecoded runs the csvutil decode loop over a csvutil.Reader. +func parseDecoded[T any](r csvutil.Reader) ([]T, error) { dec, err := csvutil.NewDecoder(r) + if err == io.EOF { + // Empty stream (e.g. all-blank input): no header, no rows. + return nil, nil + } if err != nil { return nil, err }