feat(csv): tell a number from a code - #669
Merged
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dd4dc9792
ℹ️ 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".
andiwand
force-pushed
the
feat/csv-sheet
branch
from
August 9, 2026 08:17
cd054e3 to
3044143
Compare
andiwand
force-pushed
the
feat/csv-value-types
branch
from
August 9, 2026 08:17
8dd4dc9 to
01524bb
Compare
andiwand
force-pushed
the
feat/csv-sheet
branch
from
August 9, 2026 08:33
3044143 to
f094a81
Compare
andiwand
force-pushed
the
feat/csv-value-types
branch
from
August 9, 2026 08:33
01524bb to
4b3165d
Compare
andiwand
force-pushed
the
feat/csv-sheet
branch
from
August 9, 2026 08:54
f094a81 to
24bc739
Compare
andiwand
force-pushed
the
feat/csv-value-types
branch
from
August 9, 2026 08:54
4b3165d to
0acbaa5
Compare
andiwand
force-pushed
the
feat/csv-sheet
branch
from
August 9, 2026 09:09
24bc739 to
980d52b
Compare
andiwand
force-pushed
the
feat/csv-value-types
branch
from
August 9, 2026 09:09
0acbaa5 to
29b5d42
Compare
andiwand
force-pushed
the
feat/csv-sheet
branch
from
August 9, 2026 11:06
980d52b to
92976e1
Compare
andiwand
force-pushed
the
feat/csv-value-types
branch
from
August 9, 2026 11:09
29b5d42 to
b97900d
Compare
andiwand
force-pushed
the
feat/csv-sheet
branch
from
August 9, 2026 11:13
92976e1 to
7f58514
Compare
andiwand
force-pushed
the
feat/csv-value-types
branch
from
August 9, 2026 11:13
b97900d to
4522451
Compare
andiwand
force-pushed
the
feat/csv-sheet
branch
from
August 9, 2026 11:25
7f58514 to
e556a17
Compare
andiwand
force-pushed
the
feat/csv-value-types
branch
from
August 9, 2026 11:25
4522451 to
7492056
Compare
andiwand
force-pushed
the
feat/csv-sheet
branch
from
August 9, 2026 11:30
e556a17 to
5a22a93
Compare
andiwand
force-pushed
the
feat/csv-value-types
branch
from
August 9, 2026 11:30
7492056 to
1ae568b
Compare
`ValueType` is only `{unknown, string, float_number}` and drives one CSS class
— right alignment — so this stays small on purpose.
Decided per *column*, not per cell: a reader compares down a column, so one
number in a column of prose is not a quantity. The first row is left out of the
sample, because a header names its column rather than holding a value, and one
word would otherwise make every column prose. The header of a numeric column
stays text for the same reason.
The grammar is strict, and what it refuses is the point. A leading zero is what
tells `007` and `0123456789012` from quantities. A thousands separator does not
say which side of the Atlantic wrote it — `1,234` is a thousand or one point
two three four depending on where you live, and a csv does not carry that. Dates
are left alone entirely: `03/04/2026` is two different days and guessing gets it
wrong confidently.
Quoting is not consulted. RFC 4180 quoting is lexical — Excel quotes any field
holding a separator, a quote or a newline, and plenty of writers quote
everything — so it carries no type information at all.
The raw string stays what is displayed either way.
The inference walks the fields a file holds, not the rectangle they span. A
sheet is rectangular even where the file is not, so a single wide record widens
every row: sampling `rows * columns` cells would have a file of a hundred
thousand short records plus one wide one pay for ten billion lookups on open,
before the renderer's cap can matter.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QSgWdLTSLCWDeFvbVwZDVU
andiwand
force-pushed
the
feat/csv-value-types
branch
from
August 9, 2026 11:35
1ae568b to
d5d9449
Compare
andiwand
enabled auto-merge (squash)
August 9, 2026 11:41
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.
🤖 Generated with Claude Code
Last of five stacked PRs. Based on #668. Plan:
src/odr/internal/csv/PLAN.md.ValueTypeis only{unknown, string, float_number}(document_element.hpp:135) and drives exactly one CSS class —text-align:right(html/frontend.cpp:32). So this stays small on purpose.Per column, not per cell
A reader compares down a column, so one number in a column of prose is not a quantity:
The first row is left out of the sample: a header names its column rather than holding a value, and one word would otherwise make every column prose. The header of a numeric column stays text for the same reason.
Empty cells do not break a numeric column.
What the grammar refuses is the point
42,-3.14,1.5E-3007,01234567890121,234/1.234,562026-08-09,03/04/202603/04/2026is two different days; guessing gets it wrong confidentlyDates are left alone entirely rather than half-supported.
Quoting is not consulted. RFC 4180 quoting is lexical — Excel quotes any field holding a separator, a quote or a newline, and plenty of writers quote everything — so it carries no type information. This was the one item in the original brief I pushed back on, and it stays pushed back.
The raw string is always what gets displayed; the value type only changes alignment.
Tests
is_numberis a free function and tested directly, plus the column rules through the sheet. 577 unit tests and 234 reference-output tests green.