docs: make the README a doctest and correct the public-API docs (#99) - #134
Merged
Merged
Conversation
Two of this issue's four findings were already fixed earlier in the stack — PUBLIC-API-2 (Poisoned vs is_fatal) landed with #98, and PUBLIC-API-4's encrypted-database under-report landed with #94 as the stride fix. What remains: Every Rust snippet in the README was a compile error against the current public API: `use chisel::defrag::DefragOptions` (the module went pub(crate)), struct literals against `#[non_exhaustive]` `DefragOptions` and `Options`, a `max_pages` field that is called `max_values`, and bare integers where the post-I120/I126 `Tag` and `Handle` newtypes are required. The README is the entry point for every downstream user and nothing caught any of it, because the README was not wired into the build at all. Rather than only correcting the snippets — which would rot again on the next API change — the README is now included via `#![doc = include_str!("../README.md")]`, so all ten fences are doctests and `cargo test` fails the next time the docs fall behind. Fences that would touch the filesystem, or that need a live failure to illustrate, are `no_run`: compiled, not executed. The rest run against an in-memory database set up in hidden lines. Verified the wiring is load-bearing, not decorative: reverting one example to its old `allocate_tagged(b"row-a", 42)` form fails the suite with "expected `Tag`, found integer". The prose the snippets sit in is corrected to match — tags are non-zero and untagged means `tag()` returns None (the "tag 0 means untagged" model predates I126), and the API table now names `Handle`/`Tag` rather than `u64`/`u32`. Both `Stats` fields were also described wrongly, in opposite directions. `total_pages` claimed to match `Superblock.total_pages` while being filled from the PHYSICAL page count, which is the more useful of the two to name explicitly since they diverge exactly when a crash leaves orphan pages in the tail. `file_size_bytes` claimed it "may exceed `total_pages * PAGE_SIZE`" when it is computed from that same count and so can never exceed itself. Finally, `get_root_name` and `clear_root_name` both run the same name validation `set_root_name` does, so `""` or a 25-byte name is an `InvalidRootName` error, not a miss. Their `# Errors` sections said "Only on poisoning" and "NoActiveTransaction if no transaction is open", inviting a caller to treat any Err from a lookup as a fatal drop-and-reopen and tear down a healthy handle. `#[warn(missing_errors_doc)]` guarantees the section exists but not that it is complete, which is what makes an omission like this easy to trust. Both sections now list it, and a test pins the contract — including the half that is easy to over-correct: a valid-but-unbound name is still Ok. Closes #99.
This was referenced Jul 29, 2026
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.
Closes #99 (4 findings, DESIGN/api-design+docs).
Two of the four findings were already fixed earlier in this stack:
PUBLIC-API-2 (
Poisonedlisted as fatal whileis_fatal()returns falsefor it) landed with #133, and PUBLIC-API-4's encrypted-database
under-report landed with #129 as the stride fix. What follows is the remainder.
PUBLIC-API-3 — every README example was a compile error
Against the current public API:
use chisel::defrag::DefragOptions;— the module ispub(crate); the type is re-exported at the crate rootDefragOptions { sparse_threshold: 0.25, max_pages: 0 }— the field ismax_values, and the struct is#[non_exhaustive]so a downstream crate cannot use a struct literal at alllet options = Options { … };— same#[non_exhaustive]problem, which the very next paragraph then explainsdb.allocate_tagged(b"row-a", 42),assert_eq!(db.tag(a)?, 42),db.handles_with_tag(42)— these takeTagand returnOption<Tag>/Handlesince I120/I126Nothing caught any of it, because the README was not wired into the build.
The fix is the wiring, not just the snippets
Correcting the examples alone would leave them to rot on the next API change.
The README is now included via
#![doc = include_str!("../README.md")], so allten fences are doctests and
cargo testfails the next time the docs fallbehind the API. Fences that would touch the filesystem, or need a live failure
to illustrate, are
no_run— still compiled, just not executed; the rest runagainst an in-memory database set up in hidden (
#-prefixed) lines.Verified load-bearing rather than decorative — reverting one example to its old
form:
The surrounding prose is corrected to match: tags are non-zero and untagged
means
tag()returnsNone(the "tag0means untagged" model predatesI126), and the API table names
Handle/Tagrather thanu64/u32.PUBLIC-API-4 (remainder) — both
Statsfields were described wrongly, in opposite directionstotal_pagesclaimed to matchSuperblock.total_pagesbut is filled fromfile_page_count()— the physical count. Worth naming explicitly, becausethe two diverge exactly when a crash leaves orphan pages in the tail, and the
superblock figure is the authoritative one for what the database contains.
file_size_bytesclaimed it "may exceedtotal_pages * PAGE_SIZEwhen aprevious crash left orphan pages" — but it is computed from that same count, so
it can never exceed itself. The impossible clause is gone.
PUBLIC-API-5 —
# ErrorsomittedInvalidRootNameon two methodsget_root_namedocumented "Only on poisoning — an unboundnamereturnsOk(None)", andclear_root_namedocumented onlyNoActiveTransaction. Bothdelegate to inner functions that call
encode_root_name(name)?, so"", a25-byte name, or a name containing NUL is an
InvalidRootNameerror — not amiss.
A caller trusting "only on poisoning" treats any
Errfrom a lookup as afatal drop-and-reopen condition and tears down a healthy handle over a too-long
name.
#![warn(clippy::missing_errors_doc)]guarantees the section existsbut not that it is complete, which is what makes an omission like this easy
to trust.
New test —
root_name_validation_applies_to_the_read_and_clear_paths_toopins the corrected contract on both methods, including the half that is easy to
over-correct: a valid-but-unbound name is still
Ok(None)/Ok(()).Verification
cargo testall green (including the 10 new doctests),cargo clippy --workspace --all-targets -- -D warningsclean,cargo fmt --checkclean.