Pre-publish polish: metadata, threading contract, doc fixes#21
Merged
Conversation
antondalgren
force-pushed
the
prepublish-polish
branch
from
1bae2d9 to
2dd1121
Compare
Five items from the pre-public-release audit, bundled because they're
small, independent, and none carry regression risk:
1. shard.yml: add description, repository, documentation pointing at
ARCHITECTURE.md. Helps the shard appear properly on shards.info /
shardbox.org and gives new users a one-click route to the design doc.
2. Node#persist_snapshot_for_test moved under {% if flag?(:raft_debug) %},
matching the pause/partition/reset pattern. It was leftover scaffolding
from the snapshot implementation and shouldn't be public. The two
spec blocks that use it are also gated; non-raft_debug builds (incl.
CI) skip them.
3. ARCHITECTURE.md segment-flush drift: line 251 still claimed "one
write(2) syscall per appended entry" — post the segment-simplification
commit that's no longer true. Segment#append deliberately defers the
flush to Segment#sync (the durability boundary), batching N appends
between two boundaries into one write(2) + one fsync. Updated the
§4 description and the matching paragraph in the §6.5 class block.
4. ARCHITECTURE.md Config table: add rows for read_index_timeout_ticks
(ReadIndex callback timeout) and max_message_payload_bytes (wire-frame
cap). Both were added with the ReadIndex / payload-cap commits but
never made it into the table.
5. ARCHITECTURE.md L8: drop "embedding this in LavinMQ" as the canonical
integrator example. Generalised to "embedding this in a broker or
other consensus-backed service" since the library is about to go
public.
6. Node class: threading-contract paragraph at the top. Codifies the
single-fiber-per-Node invariant in the source itself, not just in
ARCHITECTURE.md. Also improved on_configuration_change /
on_configuration_applied docstrings — they now explain stored-vs-
committed semantics and the non-blocking constraint, matching the
already-good on_role_change docstring.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
antondalgren
force-pushed
the
prepublish-polish
branch
from
2dd1121 to
a7cb3ac
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
1 participant
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.
Summary
Bundles the remaining four "land before publishing" items from the pre-public-release audit. None carry regression risk; all are pre-publish hygiene.
Stacked on top of #20 (CI) — base set to `add-ci`. Merge #20 first, then this rebases cleanly onto `main`.
1. shard.yml metadata
Helps the shard appear correctly on shards.info / shardbox.org and gives new users a one-click route to the design doc. `raft-tui` target stays — it's only built when raft.cr is the root project (dependencies don't inherit targets), so consumers don't pay for it.
2. Hide `Node#persist_snapshot_for_test`
Moved under `{% if flag?(:raft_debug) %}` alongside the existing `pause` / `partition` / `reset` debug helpers. It was leftover scaffolding from the snapshot implementation that ended up public. The two spec blocks that use it are also gated; the production build (and CI, post-#20) skips them.
3. ARCHITECTURE.md segment-flush drift
`§4` and `§6.5` both claimed "one `write(2)` syscall per appended entry." Post the segment-simplification commit (#7) that's no longer true: `Segment#append` deliberately defers flush to `Segment#sync`, so a batch of N appends between two durability boundaries collapses into one `write(2)` + one `fsync`. Updated both paragraphs.
4. ARCHITECTURE.md Config table
Added rows for `read_index_timeout_ticks` and `max_message_payload_bytes` — both exist in `Config` but weren't documented in the table.
5. ARCHITECTURE.md L8 — drop LavinMQ-as-canonical-integrator
"embedding this in LavinMQ" → "embedding this in a broker or other consensus-backed service." A public release shouldn't lead with the maintainer's employer's product.
6. Threading contract in node.cr
Added a class-level docstring on `Raft::Node` codifying the single-fiber-per-Node invariant. Currently documented in ARCHITECTURE.md only; a contributor reading `node.cr` cold could miss it and call `propose` from an HTTP handler fiber. Also tightened `on_configuration_change` / `on_configuration_applied` docstrings — they now explain the stored-vs-committed semantics and the non-blocking constraint, matching the already-good `on_role_change` docstring.
Test plan
🤖 Generated with Claude Code