From 76b0591df6e20d23b6b942c05388d090fd81acf7 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 9 Aug 2026 09:31:58 +0200 Subject: [PATCH] feat(csv): probe for a dialect instead of checking for one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check_csv_file` conflated two jobs: deciding whether a file is a csv, and deciding whether it can be parsed. It answered both by reading the *whole* file, and `open_strategy` called it speculatively for every unrecognised text file, so classifying a large one cost a full pass over it. It also hard-coded `,` and refused any file whose records disagreed on a field count. Detection and parsing split. `RecordReader` reads records out of decoded UTF-8 text for a given dialect and judges nothing: ragged records come out ragged, an unterminated quote still yields its field and sets a flag. `probe` scores a bounded sample and returns the dialect it resolved plus a verdict. Scoring tries `,`, `;`, tab and `|`, takes the field count most records carry, and picks the separator explaining the most records. Excel's `sep=` opening line wins outright. The verdict is a heuristic and says so: at least two columns, because one column is every line of prose ever written, and no dangling quote in a file we have all of. Neither is a statement about validity — a one-column csv is a legitimate csv, and once a caller declares a file to be one, it goes straight to `RecordReader`. A sample cut mid-record says nothing about the record it cut, so the last one is dropped unless the sample is the whole file. The sample is decoded first, which is what makes a UTF-16 csv detectable at all — Excel's "Unicode Text" export is UTF-16LE and tab-separated. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QSgWdLTSLCWDeFvbVwZDVU --- src/odr/internal/csv/csv_file.cpp | 13 +- src/odr/internal/csv/csv_file.hpp | 5 + src/odr/internal/csv/csv_util.cpp | 193 +++++++++++++++++++----- src/odr/internal/csv/csv_util.hpp | 71 +++++++-- test/src/internal/csv/csv_file_test.cpp | 137 +++++++++++++---- test/src/test_util.cpp | 7 +- 6 files changed, 340 insertions(+), 86 deletions(-) diff --git a/src/odr/internal/csv/csv_file.cpp b/src/odr/internal/csv/csv_file.cpp index 1c5751905..38e63af69 100644 --- a/src/odr/internal/csv/csv_file.cpp +++ b/src/odr/internal/csv/csv_file.cpp @@ -1,13 +1,20 @@ #include +#include + #include +#include + namespace odr::internal::csv { CsvFile::CsvFile(std::shared_ptr file) : m_file{std::move(file)} { - // TODO use text file? - check_csv_file(*m_file->file()->stream()); + const Probe probe = csv::probe(*m_file->file(), m_file->encoding()); + if (!probe.is_csv) { + throw NoCsvFile(); + } + m_dialect = probe.dialect; } std::shared_ptr CsvFile::file() const noexcept { @@ -31,4 +38,6 @@ bool CsvFile::is_decodable() const noexcept { return false; } TextEncoding CsvFile::encoding() const noexcept { return m_file->encoding(); } +Dialect CsvFile::dialect() const noexcept { return m_dialect; } + } // namespace odr::internal::csv diff --git a/src/odr/internal/csv/csv_file.hpp b/src/odr/internal/csv/csv_file.hpp index 2f4df0c7f..c72847497 100644 --- a/src/odr/internal/csv/csv_file.hpp +++ b/src/odr/internal/csv/csv_file.hpp @@ -2,6 +2,7 @@ #include +#include #include #include @@ -22,8 +23,12 @@ class CsvFile final : public abstract::TextFile { [[nodiscard]] TextEncoding encoding() const noexcept override; + /// The dialect detection resolved. + [[nodiscard]] Dialect dialect() const noexcept; + private: std::shared_ptr m_file; + Dialect m_dialect; }; } // namespace odr::internal::csv diff --git a/src/odr/internal/csv/csv_util.cpp b/src/odr/internal/csv/csv_util.cpp index c249b0fe2..47b24c8bc 100644 --- a/src/odr/internal/csv/csv_util.cpp +++ b/src/odr/internal/csv/csv_util.cpp @@ -1,23 +1,99 @@ #include -#include +#include + +#include +#include +#include + +#include +#include #include +#include +#include #include -#include -#include #include -#include namespace odr::internal { namespace { -constexpr char csv_delimiter = ','; -constexpr char csv_quote = '"'; +using csv::Dialect; +using csv::Probe; +using csv::RecordReader; + +/// The separators worth trying, in the order they win ties. +constexpr std::array candidate_separators{',', ';', '\t', '|'}; + +/// Share of records that must agree on a field count — not all of them, a +/// stray ragged row is normal and the parser tolerates it. +constexpr double required_agreement = 0.9; + +/// Excel opens a file it exported for a `;` locale with `sep=;`. +std::optional separator_directive(const std::string_view text) { + constexpr std::string_view prefix = "sep="; + if (!text.starts_with(prefix) || text.size() < prefix.size() + 2) { + return std::nullopt; + } + const char separator = text[prefix.size()]; + const char after = text[prefix.size() + 1]; + if (after != '\n' && after != '\r') { + return std::nullopt; + } + return separator; +} + +/// The field counts of every record @p dialect finds in @p text. +struct Scan final { + std::vector counts; + bool unterminated{false}; +}; + +Scan scan(const std::string_view text, const Dialect dialect) { + Scan result; + RecordReader reader(text, dialect); + std::vector fields; + while (reader.read(fields)) { + result.counts.push_back(static_cast(fields.size())); + } + result.unterminated = reader.unterminated(); + return result; +} + +/// How well @p dialect explains @p text: the field count most records carry, +/// and the share of records carrying it. +struct Score final { + std::uint32_t columns{0}; + double agreement{0.0}; +}; + +Score score(Scan scan, const bool complete) { + // a sample cut mid-record says nothing about the record it cut + if (!complete && !scan.counts.empty()) { + scan.counts.pop_back(); + } + if (scan.counts.empty()) { + return {}; + } + + std::map histogram; + for (const std::uint32_t count : scan.counts) { + ++histogram[count]; + } + const auto modal = std::ranges::max_element( + histogram, {}, [](const auto &entry) { return entry.second; }); + + return {modal->first, static_cast(modal->second) / + static_cast(scan.counts.size())}; +} } // namespace -bool csv::read_record(std::istream &in, std::vector &fields) { +csv::RecordReader::RecordReader(const std::string_view text, + const Dialect dialect) noexcept + : m_text{text}, m_dialect{dialect} {} + +bool csv::RecordReader::read(std::vector &fields) { fields.clear(); std::string field; bool quoted = false; @@ -28,51 +104,49 @@ bool csv::read_record(std::istream &in, std::vector &fields) { field.clear(); }; - char c = 0; - while (in.get(c)) { + for (; m_position < m_text.size(); ++m_position) { + const char c = m_text[m_position]; + if (quoted) { - if (c != csv_quote) { + if (c != m_dialect.quote) { field.push_back(c); continue; } // a doubled quote is an escaped one and stays inside the field - if (in.peek() == csv_quote) { - in.get(c); - field.push_back(csv_quote); + if (m_position + 1 < m_text.size() && + m_text[m_position + 1] == m_dialect.quote) { + ++m_position; + field.push_back(m_dialect.quote); continue; } quoted = false; continue; } - switch (c) { - case csv_quote: + if (c == m_dialect.quote) { quoted = true; started = true; - break; - case csv_delimiter: + } else if (c == m_dialect.separator) { end_field(); started = true; - break; - case '\r': - break; // CRLF, and a lone CR - case '\n': + } else if (c == '\r') { + // CRLF, and a lone CR + } else if (c == '\n') { if (!started) { - break; // empty line + continue; // empty line } end_field(); + ++m_position; return true; - default: + } else { field.push_back(c); started = true; - break; } } - // reaching EOF inside a quoted field means the quote was never closed; the - // partial field is not a record + // the caller decides what an unterminated field means if (quoted) { - throw std::runtime_error("csv quoted field is not terminated"); + m_unterminated = true; } if (!started) { @@ -82,26 +156,65 @@ bool csv::read_record(std::istream &in, std::vector &fields) { return true; } -void csv::check_csv_file(std::istream &in) { - std::optional columns; +bool csv::RecordReader::unterminated() const noexcept { return m_unterminated; } - std::vector fields; - while (read_record(in, fields)) { - if (!columns.has_value()) { - columns = fields.size(); +csv::Probe csv::probe(const std::string_view text, const bool complete, + const char quote) { + Probe result; + result.dialect.quote = quote; + + if (const std::optional declared = separator_directive(text); + declared.has_value()) { + result.dialect.separator = *declared; + result.separator_directive = true; + + const std::size_t line_end = text.find_first_of("\r\n"); + const std::size_t body = text.find_first_not_of("\r\n", line_end); + const Score scored = + score(scan(body == std::string_view::npos ? "" : text.substr(body), + result.dialect), + complete); + result.columns = scored.columns; + result.is_csv = scored.columns >= 1; + return result; + } + + Score best; + for (const char separator : candidate_separators) { + const Dialect dialect{.separator = separator, .quote = quote}; + const Scan scanned = scan(text, dialect); + + // a dangling quote in a file we have all of is good evidence of not-csv + if (complete && scanned.unterminated) { + continue; + } + + const Score scored = score(scanned, complete); + if (scored.columns < 2) { continue; } - if (fields.size() != *columns) { - throw std::runtime_error("csv row has " + std::to_string(fields.size()) + - " fields, expected " + std::to_string(*columns)); + if (scored.agreement > best.agreement || + (scored.agreement == best.agreement && scored.columns > best.columns)) { + best = scored; + result.dialect = dialect; } } - // one column is every line of prose ever written, so it is no evidence of a - // csv; the caller is probing an otherwise unrecognised text file - if (!columns.has_value() || *columns <= 1) { - throw std::runtime_error("no csv file"); + result.columns = best.columns; + result.is_csv = best.columns >= 2 && best.agreement >= required_agreement; + return result; +} + +csv::Probe csv::probe(const abstract::File &file, const TextEncoding encoding, + const char quote) { + if (!text_encoding_is_decodable(encoding)) { + return {}; } + + const std::unique_ptr in = file.stream(); + const std::string bytes = encoding::read_probe(*in); + const bool complete = bytes.size() < encoding::default_probe_size; + return probe(encoding::to_utf8(bytes, encoding), complete, quote); } } // namespace odr::internal diff --git a/src/odr/internal/csv/csv_util.hpp b/src/odr/internal/csv/csv_util.hpp index e72a390ad..1eae933a8 100644 --- a/src/odr/internal/csv/csv_util.hpp +++ b/src/odr/internal/csv/csv_util.hpp @@ -1,22 +1,73 @@ #pragma once -#include +#include + +#include +#include #include +#include #include +namespace odr::internal::abstract { +class File; +} + namespace odr::internal::csv { -/// Reads the next record into @p fields; `false` once the input is exhausted. +/// The lexical shape of a csv file. +struct Dialect final { + char separator{','}; + char quote{'"'}; +}; + +/// Reads RFC 4180 records out of already-decoded UTF-8 text. /// -/// RFC 4180: `,` separates, `"` quotes, `""` is a literal quote inside a quoted -/// field, and a quoted field may span lines. An empty line yields no record, -/// which is what keeps a trailing newline from reading as a one-field row. +/// Rejects nothing: text ending inside a quoted field still yields its record +/// and sets @ref unterminated. Refusing a file is detection's job. +class RecordReader final { +public: + RecordReader(std::string_view text, Dialect dialect) noexcept; + + /// Reads the next record into @p fields; `false` once the text is exhausted. + /// An empty line yields no record, so a trailing newline is not a row. + bool read(std::vector &fields); + + /// Whether the text ran out inside a quoted field. + [[nodiscard]] bool unterminated() const noexcept; + +private: + std::string_view m_text; + Dialect m_dialect; + std::size_t m_position{0}; + bool m_unterminated{false}; +}; + +/// What a probe made of a file's opening bytes. +struct Probe final { + Dialect dialect; + /// The field count most records carry. + std::uint32_t columns{0}; + /// Whether the opening line is Excel's `sep=` directive, which names the + /// separator and is not data. + bool separator_directive{false}; + /// Whether this looks like a csv at all — see @ref probe. + bool is_csv{false}; +}; + +/// Scores @p text — a file's opening bytes decoded to UTF-8 — as a csv and +/// resolves its dialect. @p complete says whether that is the whole file, since +/// a sample cut mid-record says nothing about the record it cut. /// -/// @throws std::runtime_error if the input ends inside a quoted field. -bool read_record(std::istream &in, std::vector &fields); +/// The verdict is a detection heuristic, not a validity rule: two columns +/// minimum, because one column is every line of prose ever written, and no +/// dangling quote in a complete file. A declared csv goes straight to +/// @ref RecordReader and is never asked to pass this. +[[nodiscard]] Probe probe(std::string_view text, bool complete, + char quote = '"'); -/// Throws unless @p in parses as a csv whose records all carry the same number -/// of fields, and more than one of them. -void check_csv_file(std::istream &in); +/// Reads @p file's opening bytes, decodes them and scores them. Not a csv when +/// @p encoding cannot be decoded. +[[nodiscard]] Probe probe(const abstract::File &file, TextEncoding encoding, + char quote = '"'); } // namespace odr::internal::csv diff --git a/test/src/internal/csv/csv_file_test.cpp b/test/src/internal/csv/csv_file_test.cpp index e8fe89024..04b4c5e5b 100644 --- a/test/src/internal/csv/csv_file_test.cpp +++ b/test/src/internal/csv/csv_file_test.cpp @@ -7,26 +7,49 @@ #include #include +#include #include -#include #include +#include using namespace odr; +using namespace odr::internal; using namespace odr::test; +namespace { + +/// Detection over a whole file, which is what the probe sees for anything +/// shorter than its bound. +csv::Probe probe(const std::string &content) { + return csv::probe(content, true); +} + +std::vector> records(const std::string &content, + const csv::Dialect dialect) { + std::vector> result; + csv::RecordReader reader(content, dialect); + std::vector fields; + while (reader.read(fields)) { + result.push_back(fields); + } + return result; +} + +} // namespace + TEST(CsvFile, odt) { const File file(TestData::test_file_path("odr-public/odt/about.odt")); EXPECT_THROW(internal::csv::CsvFile( std::make_shared(file.impl())), - std::runtime_error); + odr::Exception); } TEST(CsvFile, txt) { const File file(TestData::test_file_path("odr-public/txt/lorem ipsum.txt")); EXPECT_THROW(internal::csv::CsvFile( std::make_shared(file.impl())), - std::runtime_error); + NoCsvFile); } TEST(CsvFile, csv) { @@ -41,35 +64,85 @@ TEST(CsvFile, csv) { testing::Contains(FileType::comma_separated_values)); } -TEST(CsvFile, records_must_have_a_consistent_field_count) { - const auto check = [](const std::string &content) { - internal::csv::CsvFile(std::make_shared( - File::from_memory(content).impl())); - }; - - EXPECT_NO_THROW(check("a,b,c\n1,2,3\n")); - EXPECT_NO_THROW(check("a,b,c\n1,2,3")); // no trailing newline - EXPECT_NO_THROW(check("a,b\r\n1,2\r\n")); // crlf - EXPECT_NO_THROW(check("a,b\n\"x,y\",2\n")); // a quoted delimiter - EXPECT_NO_THROW(check("a,b\n\"x\ny\",2\n")); // a quoted newline - EXPECT_NO_THROW(check("a,b\n\"x\"\"y\",2\n")); // an escaped quote - - EXPECT_THROW(check("a,b,c\n1,2\n"), std::runtime_error); // short row - EXPECT_THROW(check("a,b\n1,2,3\n"), std::runtime_error); // long row - EXPECT_THROW(check("one column\nand another\n"), std::runtime_error); - EXPECT_THROW(check(""), std::runtime_error); +TEST(CsvProbe, a_consistent_field_count_is_what_makes_it_a_csv) { + EXPECT_TRUE(probe("a,b,c\n1,2,3\n").is_csv); + EXPECT_TRUE(probe("a,b,c\n1,2,3").is_csv); // no trailing newline + EXPECT_TRUE(probe("a,b\r\n1,2\r\n").is_csv); // crlf + EXPECT_TRUE(probe("a,b\n\"x,y\",2\n").is_csv); // a quoted separator + EXPECT_TRUE(probe("a,b\n\"x\ny\",2\n").is_csv); // a quoted newline + EXPECT_TRUE(probe("a,b\n\"x\"\"y\",2\n").is_csv); // an escaped quote + + EXPECT_FALSE(probe("one column\nand another\n").is_csv); + EXPECT_FALSE(probe("").is_csv); +} + +/// One column is every line of prose ever written, so it is no evidence of a +/// csv — which says nothing about whether a one-column csv is legitimate. +TEST(CsvProbe, one_column_is_not_evidence) { + EXPECT_FALSE(probe("a\nb\nc\n").is_csv); + EXPECT_EQ(records("a\nb\nc\n", {}).size(), 3u); +} + +/// A file that happens to hold an odd number of quotes and a consistent +/// separator count used to be classified as csv. +TEST(CsvProbe, a_dangling_quote_in_a_complete_file_is_evidence_against) { + EXPECT_FALSE(probe("a,b\n1,\"2,3").is_csv); + EXPECT_FALSE(probe("a,b\n\"1,2\n").is_csv); + EXPECT_FALSE(probe("a,b\n1,\"2").is_csv); +} + +/// The same text cut short says nothing — the sample ended mid-record, not the +/// file. +TEST(CsvProbe, a_dangling_quote_in_a_sample_is_not) { + EXPECT_TRUE(csv::probe("a,b\n1,2\n3,4\n5,\"6", false).is_csv); +} + +TEST(CsvProbe, the_separator_is_the_one_that_explains_the_file) { + EXPECT_EQ(probe("a;b;c\n1;2;3\n").dialect.separator, ';'); + EXPECT_EQ(probe("a\tb\tc\n1\t2\t3\n").dialect.separator, '\t'); + EXPECT_EQ(probe("a|b|c\n1|2|3\n").dialect.separator, '|'); + EXPECT_EQ(probe("a,b,c\n1,2,3\n").dialect.separator, ','); +} + +/// Commas inside fields of a semicolon-separated file must not win: they do +/// not produce a consistent count. +TEST(CsvProbe, a_separator_that_only_sometimes_appears_loses) { + const csv::Probe result = probe("name;note\nx;a, b, c\ny;d\n"); + EXPECT_TRUE(result.is_csv); + EXPECT_EQ(result.dialect.separator, ';'); + EXPECT_EQ(result.columns, 2u); +} + +TEST(CsvProbe, excel_declares_its_separator) { + const csv::Probe result = probe("sep=;\na;b\n1;2\n"); + EXPECT_TRUE(result.is_csv); + EXPECT_TRUE(result.separator_directive); + EXPECT_EQ(result.dialect.separator, ';'); + EXPECT_EQ(result.columns, 2u); +} + +TEST(RecordReader, parses_rfc4180) { + EXPECT_EQ(records("a,b\n1,2\n", {}), + (std::vector>{{"a", "b"}, {"1", "2"}})); + EXPECT_EQ(records("\"x,y\",2\n", {}), + (std::vector>{{"x,y", "2"}})); + EXPECT_EQ(records("\"x\ny\",2\n", {}), + (std::vector>{{"x\ny", "2"}})); + EXPECT_EQ(records("\"x\"\"y\",2\n", {}), + (std::vector>{{"x\"y", "2"}})); +} + +/// The parser judges nothing: a ragged file is a ragged file, and the sheet +/// pads it later. +TEST(RecordReader, ragged_records_come_out_as_they_are) { + EXPECT_EQ(records("a,b,c\n1,2\n", {}), (std::vector>{ + {"a", "b", "c"}, {"1", "2"}})); } -/// An unterminated quote used to reach EOF and emit the partial field as a -/// record, so text that happens to hold an odd number of quotes and a -/// consistent comma count was classified as csv. -TEST(CsvFile, a_quoted_field_must_be_terminated) { - const auto check = [](const std::string &content) { - internal::csv::CsvFile(std::make_shared( - File::from_memory(content).impl())); - }; - - EXPECT_THROW(check("a,b\n1,\"2,3"), std::runtime_error); - EXPECT_THROW(check("a,b\n\"1,2\n"), std::runtime_error); - EXPECT_THROW(check("a,b\n1,\"2"), std::runtime_error); +TEST(RecordReader, an_unterminated_quote_still_yields_its_field) { + csv::RecordReader reader("a,\"b", csv::Dialect{}); + std::vector fields; + EXPECT_TRUE(reader.read(fields)); + EXPECT_EQ(fields, (std::vector{"a", "b"})); + EXPECT_TRUE(reader.unterminated()); } diff --git a/test/src/test_util.cpp b/test/src/test_util.cpp index 81e373894..713cacd58 100644 --- a/test/src/test_util.cpp +++ b/test/src/test_util.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -28,15 +29,17 @@ namespace { std::vector> read_indexed_csv(const std::string &path) { std::ifstream in(path); + const std::string content = internal::util::stream::read(in); + csv::RecordReader reader(content, csv::Dialect{}); std::vector header; - if (!csv::read_record(in, header)) { + if (!reader.read(header)) { return {}; } std::vector> rows; std::vector fields; - while (csv::read_record(in, fields)) { + while (reader.read(fields)) { std::unordered_map row; for (std::size_t i = 0; i < header.size() && i < fields.size(); ++i) { row[header[i]] = fields[i];