Remove the duplicated logic that let the code, the models and the tests drift - #24
Merged
Merged
Conversation
…ts drift Every decision the expiry repair rests on is now a kernel in protocol.rs that production and the Stateright models both call: listing_is_truth, plan_repair, cursorless_start_needs_repair, restore_key, restore_ahead. The models' hand copies are gone; their mutations forge kernel inputs instead of re-implementing decisions, and every one is still caught. watch_applied's body is split: a Fold struct (batch, applied cursor, store; ingest/correct/advance_to/flush/settle/export) replaces the flush!/ingest!/repair_fatal! macros, and both halves of the repair (the watch task's and the main loop's) live in repair.rs. Also: WatchCursor::rank replaces two cursor_rank copies; SnapshotStore::has_entries replaces the error-as-early-stop scan (one entry read on fjall/RocksDB; conformance-checked on every backend); NatsKvWatcher::create_resume is the one resume-window check for the three *_from paths; Snapshot::stale_keys takes the bucket's retention and returns None where the listing isn't the truth; tests/common splits so only crash injection needs `transport`, and integration.rs and the floor-guard unit tests share its nats-server instead of copying it. CI now runs dprint check, rustdoc with -D warnings (three broken links fixed) and a per-feature `cargo check --tests`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0152kQKDdP8XRhoeqisJYpWr
Merged
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.
Removes the places where the same logic lived twice and could drift: in the models and the code, in two
cursor_ranks, in three NATS resume paths, in three test harnesses. Also splits the 645-linewatch_appliedbody. Behavior is unchanged; the API changes are below.Models run production logic
Every decision the repair rests on is now a kernel in
protocol.rs, called by both the production code and the Stateright models:listing_is_truthplan_repair(RepairMode→RepairPlan)cursorless_start_needs_repairrestore_key(KeyState→KeyRestore)restore_aheadtests/model.rs,model_repair.rs,model_fleet.rsandmodel_live_watch.rsdrop their hand copies (listing_is_truth,restores,restore_op).A model now expresses a mutation by forging a kernel's input, never by re-implementing the decision:
RelistOnEvicting: the listing reads as the truth.UnanchoredRelist: the fold reads as empty.RestoreIgnoresVersions: both sides read as revisionless.RestoreIgnoresListing: the key reads as not listed.All mutations are still caught.
model_repairnow also checksAutoon a bucket that keeps current values.watch_appliedsplitFoldreplaces theflush!/ingest!/repair_fatal!macros.Foldholds the batch, the applied cursor and the store, and every change goes through it:ingest,correct,advance_to,flush,settle,export. The cursor-after-apply invariant is enforced in one place. The select loop is about 100 lines.repair.rs. The watch task side isrun_watch,plan, the listing and the artifact checks. The main loop side isfold_in,relistandrestore, which act on theFold.#[allow(unused_assignments)]is gone. It only existed for the macros' dead stores.Other duplication
WatchCursor::rank()replaces the twocursor_rankcopies (repair, transport).SnapshotStore::has_entries()replacesfold_has_entries, which used anErrto stop a scan early. fjall and RocksDB read one entry. New conformance check on all three backends.NatsKvWatcher::create_resumeis the one place the three*_frompaths check the resume window. It runs the pre-check, the creation with its expired-cursor error classification, and the post-check.tests/commonis split intonats.rs,minio.rsandcrash.rs. Onlycrash.rsneedstransport.tests/integration.rsandsrc/nats.rs's floor-guard tests use the shared server: the unit tests includenats.rsvia#[path].API changes (land in 0.8.0)
Snapshot::stale_keys(current_keys, retention)now returnsOption<Vec<&str>>. It returnsNonewhen retention says the listing isn't the truth, which is the same planner and rule asExpiryRepair::Relist. It was a raw-API path to the bug Repair expired cursors from artifacts on buckets that evict current values #21 fixed.SnapshotStore::has_entriesis a new provided method.protocolgains the kernels listed above.CI guards against drift
mise run format:check(dprint: rustfmt + yamlfmt). Unformatted code no longer waits for the nextcargo fmtto reformat someone else's diff. That was thetests/dst_invariants.rsdrift, now formatted.cargo docwith-D warnings. Three pre-existing broken intra-doc links are fixed.cargo check --testsper optional feature, so shared test infrastructure can't quietly depend on another feature.Verification
cargo clippy --all-targets --all-features -D warningsandcargo doc -D warnings: clean.Every feature combination compiles with its tests: none, fjall, rocksdb, transport, fjall+transport.
Full suite, both CI configurations: green locally. All features: 17 binaries (lib 124, integration 46, snapshot_store 73 with the new
has_entriescheck on all three backends, repair_dst on all three backends, live MinIO). Default features: green.Models (release): the state spaces are unchanged. The default tiers ran on
mainand on this branch:main, mutation runs included, have identical state and unique-state counts here. The only extra configuration is the one this PR adds (keep-current,Auto: 54,247 states).main.deep_evicting_more_revisionspasses.deep_more_revisionsanddeep_three_exportersexceed this 27 GB machine's memory; the runs were killed.tests/repair_dst.rscatches all nine code mutations of the restructured repair, each with a counterexample:on_appliedwent backwardAutotrusts the listingapplysaw a revision regress🤖 Generated with Claude Code
https://claude.ai/code/session_0152kQKDdP8XRhoeqisJYpWr