feat(csv): load a csv as a spreadsheet - #668
Merged
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd054e3f2f
ℹ️ 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-options
branch
from
August 9, 2026 08:17
1c21594 to
9250683
Compare
andiwand
force-pushed
the
feat/csv-sheet
branch
from
August 9, 2026 08:17
cd054e3 to
3044143
Compare
andiwand
force-pushed
the
feat/csv-options
branch
from
August 9, 2026 08:33
9250683 to
08fad77
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-sheet
branch
2 times, most recently
from
August 9, 2026 09:09
24bc739 to
980d52b
Compare
andiwand
force-pushed
the
feat/csv-options
branch
2 times, most recently
from
August 9, 2026 10:50
8026aa2 to
90dfbac
Compare
andiwand
force-pushed
the
feat/csv-sheet
branch
3 times, most recently
from
August 9, 2026 11:25
7f58514 to
e556a17
Compare
A csv rendered as a line list, because it was a text file with no decoder. It is a table, and the library already knows how to render tables — the whole job is to expose one. A csv stays a text file. It gains a second view of the same bytes: `CsvFile::document()` yields a one-sheet spreadsheet, and `translate` renders that rather than a line list. Nothing about `is_text_file()` or `as_text_file()` changes, so reading a csv as text still needs no reopening. `CsvDocument` is that sheet. The generic renderer walks it, and every binding gets table rendering without a line of format-specific code. Cells are not elements. The registry pattern the root `AGENTS.md` prescribes costs an entry per element, and a sheet has one per cell, so a large file would pay for millions before any were looked at. `ElementIdentifier` is 64 bits, so an id *is* the coordinate — kind, row, column packed in — and the adapter decodes rather than looks up. The consequence to respect is that a sheet's cells are not reachable by walking; they come from `sheet_cell(column, row)`, which is how the renderer asks for them anyway. Everything reaches the data through `cell` and `dimensions`, so an index and a window can move in behind them later without the adapter noticing. Neither the handle nor the file meta names a document type. A csv always yields a spreadsheet, so asking a particular file is asking a constant: the table answers it for the type (`document_type_by_file_type`), and the document answers it for itself. The sheet is rectangular even where the file is not: a short row pads, a long one widens. That is the counterpart to detection having stopped rejecting ragged files. An encoding `internal/encoding` cannot decode has no document: cell text has to be UTF-8 by the time a binding sees it. Such a file still reads as text. 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-sheet
branch
from
August 9, 2026 11:30
e556a17 to
5a22a93
Compare
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
Fourth of five stacked PRs. #667 is merged, so this now sits on
main. Module rules:src/odr/internal/csv/AGENTS.md.This is the visible one. A csv rendered as a line list because it was a text file with no decoder. It is a table, the library already knows how to render tables, and the whole job is to expose one.
A csv stays a text file
It gains a second view of the same bytes:
CsvFile::document()yields a one-sheet spreadsheet, andhtml::translaterenders that rather than a line list.is_text_file(),as_text_file()andTextFile::text()are unchanged, so reading a csv as text still needs no reopening.The only table change is one line — csv keeps
FileCategory::textand gainsDocumentType::spreadsheet, i.e. what it is stays text, what it yields is a spreadsheet:translate(DecodedFile)gets a csv branch before the text branch, since a csv would otherwise be claimed by it and rendered as a line list.Nothing asks a csv what document type it is
A csv always yields a spreadsheet, so a per-file accessor for it would be a constant dressed as a query — unlike
DocumentFile::document_type(), where odf, ooxml and oldms each inspect the package to answer. So there is noCsvFile::document_type(), no virtual behind it, and the file meta does not carry one either. The two places that already say it are enough:document_type_by_file_type(FileType::comma_separated_values)for the type, andDocument::document_type()for the object.That also means no reference-output change: csv
meta.jsonis byte-identical to what is on the pin today, and the pin stays where it is.Cells are not elements
The registry pattern the root
AGENTS.mdprescribes costs an entry per element, and a sheet has one per cell — a large file would pay for millions before any were looked at, whilespreadsheet_limitmeans the renderer asks for 10 000 rows at most.ElementIdentifieris astd::uint64_t, which is room to spare:An id is the coordinate; the adapter decodes rather than looks up.
null_element_idis zero, so no kind may be.The consequence to respect, and the reason it is in
AGENTS.md: a sheet's cells are not reachable by walking.element_first_childof a sheet isnull_element_id. Cells come fromsheet_cell(column, row), which is how the renderer asks for them anyway (html/document_element.cpp:163).Everything reaches the data through
cellanddimensions, so an index and a window can move in behind those two later without the adapter noticing.The sheet is rectangular even where the file is not
A short row pads, a long one widens — the counterpart to detection having stopped rejecting ragged files in #666.
An encoding
internal/encodingcannot decode has no document: cell text has to be UTF-8 by the time a binding sees it (Text::content()→NewStringUTF/embind/str/NSString). Such a file still reads as text.Tests
Sheet shape, ragged rows, the
sep=directive not being data, undecodable encodings, a cellDocumentPathround-trip (the one place virtual ids could silently break, sinceextract_pathspecial-casessheet_cell), and that a csv actually renders<table>/<td>.572 unit tests and 234 reference-output tests green.