fix(csv): do not read prose as a table - #697
Merged
Merged
Conversation
A comma between clauses splits every line of prose in two, which by field count alone is a consistent two column table. Any text whose lines share a separator was detected as csv and rendered as a spreadsheet. Measured against the probe, all of these came back `text/csv`: prose with one comma per line, German business prose, prose with semicolons or tabs, and a single paragraph with no newlines at all. Two rules, both requiring positive evidence of a table: - A separator that almost every following field opens with a space, in fields long enough to be sentences, is punctuation rather than a delimiter. Both halves are needed: a csv written `name, age, city` has the spacing but not the length, and stays a csv. - One record is a line, not a table, however many separators it holds. The doubt breaks towards text on purpose. Plain text is a readable rendering of a table; a table is not a readable rendering of text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TouAQNfktsp9THcceennEX
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c29864633c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Two ways the punctuation rule could reject a real csv. The prose threshold is stated in characters but `trimmed_length` counted bytes, so four cjk characters or three emoji read as a sentence. Count the bytes that open a character instead. And when the probe's sample ended mid-record, `score` dropped that record from the field counts but left its fields in the spacing and length statistics, so where the 64 KiB bound happened to fall could decide a file. The record now comes out of both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTfEEeZdHs1qEzXbtycGJ5
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.
A plain text file whose lines each contain a comma is, by field count alone, a consistent two column table. So prose was detected as csv and rendered as a spreadsheet.
This came out of a user report about the Android app: "does not work usefully any more with larger TXT files". The app hands the engine a cached copy named
cached-file.tmp, so detection is content-only and every text file goes through this probe.What the probe said before
Measured with
Odr.mimetypeon real samples:text/plaintext/csvtext/csvtext/csvtext/csvtext/csvtext/csvA comma before a subclause is ordinary punctuation in German, French and English alike, so this is most prose, not an edge case.
The rules added
Both ask for positive evidence of a table rather than the absence of evidence against one.
A separator used as punctuation is not a delimiter. If almost every field after a separator opens with a space and the fields average long enough to be sentences, the separator is punctuation. Both halves are required, which is what keeps a spaced-out csv working:
name, age, cityhas the spacing but its values are short.One record is a line, not a table, however many separators it holds. That is the single-paragraph case.
Direction of the doubt
Deliberate: plain text is a readable rendering of a table, and a table is not a readable rendering of text. A 2-column csv with long free-text values written
Ada Lovelace, mathematician and writerwill now fall back to text. Both thresholds are named constants (punctuation_spacing,prose_field_length) if that balance wants moving.Tests
Three new cases in
csv_file_test.cpp— prose with commas and with semicolons, spaced-out short-value csv, and the single record. Full suite: 883 passed, 8 skipped, 0 failed, including the reference-output comparisons.🤖 Generated with Claude Code