DESIGN.md's index chapter presents two module surfaces as C++ classes. Neither class exists. Documentation only — nothing in the code is wrong.
The two instances
### IndexWriter opens with
class IndexWriter {
static auto write(path, index) -> Result<Unit>;
};
### IndexReader does the same for a reader type.
$ grep -rn 'class IndexWriter\|struct IndexWriter\|class IndexReader\|struct IndexReader' src/ include/
$
Both surfaces are free functions:
| Documented |
Actual |
IndexWriter::write(path, index) -> Result<Unit> |
write_index(std::string_view, Index const&) -> Result<void> — src/index/writer.cpp:150 |
| — |
serialize_index(Index const&) -> Result<Vec<std::byte>> — src/index/writer.cpp:163 |
IndexReader::… |
read_index(IndexFile const&) -> Result<Index> — src/index/reader.cpp:126; read_index(std::string_view) -> Result<Index> — reader.cpp:186 |
| — |
index_verify_checksum(IndexFile const&) -> bool — reader.cpp:437, declared include/pup/index/reader.hpp:82 |
The return type is wrong too: Result<Unit>, where the codebase uses Result<void>.
The "Write process" list is wrong in the same way
Under ### IndexWriter:
- Serialize to temporary file
- Compute SHA-256 checksum
- Write footer with checksum
- Atomic rename to final path
Steps 1-3 describe operations on a file, and none of them touch one. serialize_index builds a Vec<std::byte> in memory; the checksum is computed over that buffer and the footer written into it at src/index/writer.cpp:363-366, before any file exists. write_index then hands the finished bytes to pup::platform::atomic_write, and the temporary file and the rename both live inside atomic_write — the platform layer's business, not the writer's. So step 4 names a real thing but attributes it to the wrong layer.
Why this is worth fixing rather than tolerating
The atomicity paragraph merged as 26ddb61 sits directly beneath that list and is load-bearing: it is what entitles the record reader to announce damage rather than suspect its own half-finished write. The stale list above it currently misattributes where the checksum lands, so a reader who catches the falsehood is invited to doubt the correct paragraph below it. Worse, a list claiming the writer stages its own file invites exactly the change that guarantee cannot survive — a writer that stages a file outside atomic_write.
The class is DESIGN.md presenting a module surface as a class that does not exist. Both instances should be fixed as a pair so the fix is checked as one.
Shape of the fix
Present both surfaces as the free functions they are; drop the class blocks. Re-cut the write list along the real seam:
serialize_index builds the whole record in memory, computing the SHA-256 over the buffer and writing the footer into it.
write_index hands that buffer to pup::platform::atomic_write, which owns the temporary file and the rename.
Constraint for whoever takes it: do not restate atomic_write's temp/fsync/rename steps in the list, and do not reword the atomicity paragraph below — the paragraph is that rationale's home, and restating it would put the same claim at two sites.
Found while promoting the record laws into DESIGN.md; deliberately left out of that doc-only change because it is an independent defect. The IndexReader sibling was found during the design review of the fix.
DESIGN.md's index chapter presents two module surfaces as C++ classes. Neither class exists. Documentation only — nothing in the code is wrong.
The two instances
### IndexWriteropens with### IndexReaderdoes the same for a reader type.Both surfaces are free functions:
IndexWriter::write(path, index) -> Result<Unit>write_index(std::string_view, Index const&) -> Result<void>—src/index/writer.cpp:150serialize_index(Index const&) -> Result<Vec<std::byte>>—src/index/writer.cpp:163IndexReader::…read_index(IndexFile const&) -> Result<Index>—src/index/reader.cpp:126;read_index(std::string_view) -> Result<Index>—reader.cpp:186index_verify_checksum(IndexFile const&) -> bool—reader.cpp:437, declaredinclude/pup/index/reader.hpp:82The return type is wrong too:
Result<Unit>, where the codebase usesResult<void>.The "Write process" list is wrong in the same way
Under
### IndexWriter:Steps 1-3 describe operations on a file, and none of them touch one.
serialize_indexbuilds aVec<std::byte>in memory; the checksum is computed over that buffer and the footer written into it atsrc/index/writer.cpp:363-366, before any file exists.write_indexthen hands the finished bytes topup::platform::atomic_write, and the temporary file and the rename both live insideatomic_write— the platform layer's business, not the writer's. So step 4 names a real thing but attributes it to the wrong layer.Why this is worth fixing rather than tolerating
The atomicity paragraph merged as 26ddb61 sits directly beneath that list and is load-bearing: it is what entitles the record reader to announce damage rather than suspect its own half-finished write. The stale list above it currently misattributes where the checksum lands, so a reader who catches the falsehood is invited to doubt the correct paragraph below it. Worse, a list claiming the writer stages its own file invites exactly the change that guarantee cannot survive — a writer that stages a file outside
atomic_write.The class is DESIGN.md presenting a module surface as a class that does not exist. Both instances should be fixed as a pair so the fix is checked as one.
Shape of the fix
Present both surfaces as the free functions they are; drop the class blocks. Re-cut the write list along the real seam:
serialize_indexbuilds the whole record in memory, computing the SHA-256 over the buffer and writing the footer into it.write_indexhands that buffer topup::platform::atomic_write, which owns the temporary file and the rename.Constraint for whoever takes it: do not restate
atomic_write's temp/fsync/rename steps in the list, and do not reword the atomicity paragraph below — the paragraph is that rationale's home, and restating it would put the same claim at two sites.Found while promoting the record laws into DESIGN.md; deliberately left out of that doc-only change because it is an independent defect. The
IndexReadersibling was found during the design review of the fix.