docs(review): the wave-2 review gate — every gate check green
Read the wave-2 implementation against the pinned ADR text line by line (ADR-003/005/006/010/011/012/013 §2, store-api.md): the three numbered unknown-length rules each test-backed, the delete-window ordering invariants (arbitrate-at-delete-time, pin-before-publish on both put paths and both tiers, raced conversion ordering, delete-then-recover), abort-as-data with all three causes, dispatch purity + fall-through + constructor modes + kv-only's two put-time rejection points, stage-hygiene exact counts on both engines (incl. the Drop-cancellation story), both suite columns green/ADR-stamped/ traceable with the factory binding recorded, the dispatch-coverage shape exercised, all eight named design points recorded and consistent, and the staged-put key-at-commit amendment audited accepted. No wave-3 amendments arise. Inline fixes applied by the gate: - facade.rs: stale Batch::add dead-code allow + stale task-pointer comment removed (put_in_batch's call sites landed) - dispatch.rs: module-level allow(dead_code) removed (route/Tier now driven; clippy clean without it), scope note updated - backend.rs: the allow kept but its scope note corrected to what is actually dead (trait name, kv put until wave 3) - mem.rs: stale constructor doc line updated to the landed state - tasks/gc-protocol.md: stray placeholder lines removed Verification: cargo test (42 lib + 2 conformance + 3 git-interop), cargo test --features mem and --all-features (97 lib + 37 conformance + 4 git-interop), clippy --all-targets and --all-features --all-targets (-D warnings), fmt --check — all green post-fix. Wave 3 decomposition unblocked.
This commit is contained in:
1 parent
45c456989f
commit
79cf1f0220
6 files changed
+195
-25
No files matched your search
+4
-3
@@ -30,9 +30,10 @@
|
|||||||
//! (ADR-002/006). GC state never rides this trait either (ADR-010 §2) —
|
//! (ADR-002/006). GC state never rides this trait either (ADR-010 §2) —
|
||||||
//! the crate-internal engine-state companion seam lives in [`crate::gc`].
|
//! the crate-internal engine-state companion seam lives in [`crate::gc`].
|
||||||
//!
|
//!
|
||||||
//! The `#![allow(dead_code)]` below marks the seam's scope: these items are
|
//! The `#![allow(dead_code)]` below still has real scope: `Backend`
|
||||||
//! the wave-1 contract surface, defined ahead of their drivers (the store
|
//! trait items with no in-tree caller yet (reporting names, the kv
|
||||||
//! core and engines of waves 2–5). It shrinks as those waves land.
|
//! engines' whole-value `put` before wave 3) plus the wave-1 seam
|
||||||
|
//! tests' doubles. It shrinks as the waves land.
|
||||||
|
|
||||||
#![allow(dead_code)]
|
#![allow(dead_code)]
|
||||||
|
|
||||||
|
|||||||
+1
-6
@@ -20,15 +20,10 @@
|
|||||||
//! mode machinery, not here: this module routes lengths, it does not
|
//! mode machinery, not here: this module routes lengths, it does not
|
||||||
//! own modes.
|
//! own modes.
|
||||||
//!
|
//!
|
||||||
//! The `#![allow(dead_code)]` below marks the seam's scope: `route` is
|
//! Read-side ops do not consult `route` — the
|
||||||
//! driven by the facade's write-side tier selection (`Store::tier_for`),
|
|
||||||
//! whose own caller is the put seam (store-put-paths); the allow shrinks
|
|
||||||
//! as those land. Read-side ops do not consult `route` — the
|
|
||||||
//! fall-through consult order is the facade's (kv first, then large;
|
//! fall-through consult order is the facade's (kv first, then large;
|
||||||
//! ADR-012 §6.1).
|
//! ADR-012 §6.1).
|
||||||
|
|
||||||
#![allow(dead_code)]
|
|
||||||
|
|
||||||
/// The default dispatch threshold in bytes: 128 KiB — the conservative
|
/// The default dispatch threshold in bytes: 128 KiB — the conservative
|
||||||
/// edge of the measured ~128–256 KiB crossover zone (ADR-003; POC #3
|
/// edge of the measured ~128–256 KiB crossover zone (ADR-003; POC #3
|
||||||
/// finding A4's first-party benchmark). Constructor-tunable per
|
/// finding A4's first-party benchmark). Constructor-tunable per
|
||||||
|
|||||||
@@ -231,7 +231,6 @@ impl Batch {
|
|||||||
/// Contribute a put's pin to the group (store-core call). The
|
/// Contribute a put's pin to the group (store-core call). The
|
||||||
/// contributed guard is defused — its own drop releases nothing;
|
/// contributed guard is defused — its own drop releases nothing;
|
||||||
/// the batch's one-step release owns the protection from here.
|
/// the batch's one-step release owns the protection from here.
|
||||||
#[allow(dead_code)] // the batching put seam wires the call sites (store-put-paths); exercised by the tests
|
|
||||||
pub(crate) fn add(&mut self, pin: Pin) {
|
pub(crate) fn add(&mut self, pin: Pin) {
|
||||||
let mut pin = pin;
|
let mut pin = pin;
|
||||||
pin.armed = false;
|
pin.armed = false;
|
||||||
|
|||||||
+2
-3
@@ -35,9 +35,8 @@ use crate::Error;
|
|||||||
///
|
///
|
||||||
/// Clone shares the map, so a cloned [`Backend`] handle and every
|
/// Clone shares the map, so a cloned [`Backend`] handle and every
|
||||||
/// [`MemStaging`] issued from either see the same table — the shape
|
/// [`MemStaging`] issued from either see the same table — the shape
|
||||||
/// `put_staged`'s out-of-band commit requires. Clone is the constructor
|
/// `put_staged`'s out-of-band commit requires. The facade's `mem`
|
||||||
/// until dispatch names the engine (ADR-013 §4's feature graph; the
|
/// constructors construct it with `Default`.
|
||||||
/// store's `mem` construction lands with its wave's dispatch task).
|
|
||||||
#[derive(Clone, Default)]
|
#[derive(Clone, Default)]
|
||||||
pub(crate) struct MemBackend {
|
pub(crate) struct MemBackend {
|
||||||
rows: Arc<Mutex<BTreeMap<Vec<u8>, Vec<u8>>>>,
|
rows: Arc<Mutex<BTreeMap<Vec<u8>, Vec<u8>>>>,
|
||||||
|
|||||||
@@ -113,8 +113,6 @@ extends without changing its invariants.
|
|||||||
|
|
||||||
## Notes
|
## Notes
|
||||||
|
|
||||||
> To be filled by implementation agent
|
|
||||||
|
|
||||||
**Named design points decided here** (recorded in `src/gc.rs`'s module
|
**Named design points decided here** (recorded in `src/gc.rs`'s module
|
||||||
doc and on the traits in `src/facade.rs`):
|
doc and on the traits in `src/facade.rs`):
|
||||||
|
|
||||||
@@ -199,8 +197,6 @@ runtime flavor; the base crate's dependency set is unchanged.
|
|||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
> To be filled on completion
|
|
||||||
|
|
||||||
The in-process GC protocol (ADR-005; abort-as-data per ADR-012 §1;
|
The in-process GC protocol (ADR-005; abort-as-data per ADR-012 §1;
|
||||||
non-SQL inline window staging per ADR-012 §5) is wired on `Store`:
|
non-SQL inline window staging per ADR-012 §5) is wired on `Store`:
|
||||||
|
|
||||||
|
|||||||
+188
-8
@@ -1,7 +1,7 @@
|
|||||||
---
|
---
|
||||||
id: review-wave-2
|
id: review-wave-2
|
||||||
name: Wave 2 review gate — facade, dispatch, GC, and the first columns
|
name: Wave 2 review gate — facade, dispatch, GC, and the first columns
|
||||||
status: pending
|
status: completed
|
||||||
depends_on: [suite-column-local]
|
depends_on: [suite-column-local]
|
||||||
scope: narrow
|
scope: narrow
|
||||||
risk: low
|
risk: low
|
||||||
@@ -65,14 +65,14 @@ completion is what unblocks wave 3's decomposition.
|
|||||||
|
|
||||||
## Acceptance Criteria
|
## Acceptance Criteria
|
||||||
|
|
||||||
- [ ] Every gate check above read and verified against the cited ADR
|
- [x] Every gate check above read and verified against the cited ADR
|
||||||
text (not from memory of it)
|
text (not from memory of it)
|
||||||
- [ ] Named design points each decided, recorded, and consistent with
|
- [x] Named design points each decided, recorded, and consistent with
|
||||||
the specs
|
the specs
|
||||||
- [ ] Findings fixed inline or recorded as wave-3 amendments with
|
- [x] Findings fixed inline or recorded as wave-3 amendments with
|
||||||
owners
|
owners
|
||||||
- [ ] All gates re-run green after any fixes
|
- [x] All gates re-run green after any fixes
|
||||||
- [ ] Wave 3 decomposition unblocked (recorded in this task's Summary)
|
- [x] Wave 3 decomposition unblocked (recorded in this task's Summary)
|
||||||
|
|
||||||
## References
|
## References
|
||||||
|
|
||||||
@@ -86,8 +86,188 @@ completion is what unblocks wave 3's decomposition.
|
|||||||
|
|
||||||
## Notes
|
## Notes
|
||||||
|
|
||||||
> To be filled by implementation agent
|
> Filled by the review agent (2026-10-11). Every gate check was read
|
||||||
|
> against the cited ADR/spec text (the files, not memory): ADR-003,
|
||||||
|
> ADR-005, ADR-006, ADR-010, ADR-011, ADR-012, ADR-013 §2,
|
||||||
|
> store-api.md, and backends-and-dispatch citations. Findings below;
|
||||||
|
> everything else passed.
|
||||||
|
|
||||||
|
- **The three numbered unknown-length rules vs ADR-003 §Dispatch
|
||||||
|
policy** — each read against its tests, all faithful:
|
||||||
|
1. *Mid-stream flush, no restart requirement* —
|
||||||
|
`mid_stream_overflow_flushes_into_a_fresh_stage_and_the_stream_continues`
|
||||||
|
(the buffered prefix flushes into the fresh stage's first appends
|
||||||
|
and the stream continues appending into it; nothing migrates or
|
||||||
|
duplicates); strictly-past-the-threshold overflow with
|
||||||
|
`unknown_length_exactly_at_threshold_stays_kv`
|
||||||
|
pinning the boundary reading on the unknown path.
|
||||||
|
2. *Verification over the eventual stored artifact as a whole* —
|
||||||
|
`unknown_length_put_round_trips_on_both_tiers` pins rule 2 by key
|
||||||
|
equality (only a preamble + whole-artifact derivation yields the
|
||||||
|
canonical address), the staged form reading back over
|
||||||
|
preamble + staged bytes through the `StagingCursor: ReadCursor`
|
||||||
|
seam (the wave-1 design-led extension, now driven verbatim), the
|
||||||
|
buffer form over the complete buffer — the kv commit's whole
|
||||||
|
stored value. The known-length path's one streaming pass hashes
|
||||||
|
the same input.
|
||||||
|
3. *Stream error before commit* —
|
||||||
|
`stream_errors_before_commit_leave_no_entry_and_no_stage_file`
|
||||||
|
over all four failure forms (flushed staged, buffered kv,
|
||||||
|
known-length large, known-length kv): no entry, zero stage files.
|
||||||
|
- **The delete-window ordering invariants vs ADR-005 §Sweep** —
|
||||||
|
verified: mark → arbitrate-at-delete-time (per candidate, not once
|
||||||
|
upfront) → batched commit (~100/batch, `SWEEP_DELETE_BATCH`);
|
||||||
|
arbitration runs under the one arbitration lock (pins ∨ delete
|
||||||
|
markers ∞ sources ∨ — direct path only — protect callbacks), the
|
||||||
|
engine delete runs outside the lock behind the in-flight marker,
|
||||||
|
and the marker release notifies blocked pin mints (ADR-005's case
|
||||||
|
(a) delete-then-recover verbatim — raced in `sweep_racing_a_pinned_put_never_deletes_the_protected_entry`
|
||||||
|
and `conversion_raced_against_direct_delete_never_deletes`);
|
||||||
|
pin-before-publish holds on both put paths and both tiers
|
||||||
|
(`pin_before_publish_holds_on_both_put_paths_and_both_tiers`,
|
||||||
|
`put_publishing_after_a_sweep_started_is_never_deleted`); the
|
||||||
|
test-asserted property is covered from both sides plus the
|
||||||
|
no-torn-reads window observation. Sweeps consult the protect
|
||||||
|
callback pre-sweep only (mark), the direct path consults all three
|
||||||
|
sources at delete time — exactly the ADR-005/012 §6.4 split.
|
||||||
|
- **Abort-as-data vs ADR-012 §1** — `sweep()` returns
|
||||||
|
`Ok(SweepReport)` on every deliberate no-op; all three causes
|
||||||
|
(`NoLivenessSources`, `ProtectFailed`, `SweeperLock`) exercised as
|
||||||
|
report data with nothing deleted; `GcRefuse` occurs only on the
|
||||||
|
direct path, decided at the facade (`GcHome::arbitrate` answers as
|
||||||
|
data — the altitude discipline holds in code too), with the full
|
||||||
|
protection set (pinned / source-live / protect-named / failed
|
||||||
|
protect refused conservatively) per ADR-012 §6.4; `GcAbortCause`
|
||||||
|
is `#[non_exhaustive]` with exactly three variants (structurally
|
||||||
|
pinned, in-crate exhaustive match tested). Operational failures
|
||||||
|
alone carry `Err` (malformed pool list through the sweep's own
|
||||||
|
mark, tested) — the error half ADR-012 §1 reserves.
|
||||||
|
- **Dispatch, modes, and kv-only rejection vs ADR-003/011/012** —
|
||||||
|
`route` is pure (at-or-below → kv, pinned boundary values including
|
||||||
|
degenerate tunings and u64::MAX); `tier_for` per mode; read ops
|
||||||
|
never consult it (fall-through is consult order — test-asserted
|
||||||
|
with a planted cross-tier decoy); kv-only's two put-time rejection
|
||||||
|
points (known-length immediate via an Unreadable stream; unknown
|
||||||
|
length at mid-stream overflow) with zero residue, the at-threshold
|
||||||
|
legs landing; mem-only's threshold-free posture. The ADR-012 §6.3
|
||||||
|
dispatch-coverage shape is exercised both as a constructor test and
|
||||||
|
as the local column's dispatch rows.
|
||||||
|
- **Stage-hygiene exact-count tests vs store-api.md invariant 3 / POC
|
||||||
|
#3 A5** — present and exact on both engines: local's exact-count
|
||||||
|
rows over all three failure forms (discard, failed commit, Drop
|
||||||
|
cancellation — 3N left zero stage files and zero committed) plus
|
||||||
|
the success form (exactly N), mirrored on disk through the local
|
||||||
|
column's `stage_hygiene_exact_counts_on_disk_over_both_put_paths`
|
||||||
|
and through sweep-outcome counts (the INV-5 posture) everywhere
|
||||||
|
else; mem's degenerate stage form covered including injected
|
||||||
|
commit-failure self-discard. The wave-1 cancellation-residue
|
||||||
|
observation is owned: `drop_cancellation_leaves_zero_residue`
|
||||||
|
covers local's Drop story.
|
||||||
|
- **The suite's first columns** — green on mem + local; every row
|
||||||
|
ADR-stamped (`Contract stamp:` inventory, 13 + 7 stamps) and traced
|
||||||
|
to the backlog (the in-code traceability gate passes); the factory
|
||||||
|
binding recorded in factory.rs + README (the `Store` associated
|
||||||
|
type collapsed into the concrete facade — the ruling made upstream
|
||||||
|
in store-construction and verified here); the factory contract's
|
||||||
|
isolation and idempotent-teardown shapes structurally exercised,
|
||||||
|
including the panicking-row path on a medium-backed factory. The
|
||||||
|
mem-gating of the local column and the columns-beyond-the-reference-
|
||||||
|
set shape (concrete-factory rows observing the medium) are recorded
|
||||||
|
conventions, not ad-hoc deviations (README §Columns beyond the
|
||||||
|
reference set; suite-column-local's amended criterion).
|
||||||
|
- **The dispatch-coverage shape** — the `kv = mem` + `large = local`
|
||||||
|
column's dispatch rows exist and pull their weight: pure-function
|
||||||
|
routing with the on-disk placement oracle, threshold boundaries at
|
||||||
|
the tuned and shipped-default thresholds with the constructor-tuned
|
||||||
|
re-route leg, and fall-through per op (get/stat/has/read_range)
|
||||||
|
with the mirror-position leg and both-tier miss shapes.
|
||||||
|
- **Named design points** (each cross-checked from its recording to
|
||||||
|
the spec text): engine representation (concrete `Store` + enum
|
||||||
|
dispatch; the generic rejected — recorded with its reasons);
|
||||||
|
public stream type (`Blob` over the crate-internal `AnyCursor`);
|
||||||
|
`list` return shape (`Vec<Key>`, sorted, materialized — with the
|
||||||
|
sweep consuming crates-internally); verification-on-get (kv
|
||||||
|
re-derives and never falls through past a mismatch; large streams
|
||||||
|
metadata+handle, integrity on the put path's whole-blob pass +
|
||||||
|
slice digests; `read_range` carries no canonical check — read
|
||||||
|
against ADR-006's text, consistent with its Context's
|
||||||
|
caller-carried-digest posture); liveness-source shape (live-shared,
|
||||||
|
two sync verbs; mark-time outside the lock, delete-time probes
|
||||||
|
under it); Pin registry back-reference (`Arc<RegistryCore>` on the
|
||||||
|
guard, registry outlives the store when guards live) + lock
|
||||||
|
granularity (one short-held `parking_lot` arbitration lock;
|
||||||
|
one-directional lock order; the enable-before-check Notify recipe);
|
||||||
|
put stream parameter shape (generic `impl AsyncRead + Unpin +
|
||||||
|
Send`, no boxing); len-mismatch error (`Verification`, zero
|
||||||
|
residue, both tiers). The **staged-put seam's key-at-commit
|
||||||
|
amendment** was audited: no ADR text pins the key's arrival point
|
||||||
|
(ADR-003/010 pin handout/commit/discard mechanics), the amended
|
||||||
|
shape is consistent with all four documents' mechanics, and the
|
||||||
|
amendment carries operator sign-off — accepted.
|
||||||
|
- **Family conventions** — clean. No `unwrap`/`expect`/`panic!`
|
||||||
|
outside `#[cfg(test)]` modules (audited every occurrence); thiserror
|
||||||
|
error family (no `#[non_exhaustive]` on `Error`, correctly);
|
||||||
|
tokio-only async; all local file work in `spawn_blocking` (the
|
||||||
|
`blocking` helper audited end-to-end); parking_lot short-held
|
||||||
|
locks with the one-directional order; the few inline `//` comments
|
||||||
|
all carry non-obvious safety/accounting constraints (the rule-2
|
||||||
|
forms, the vanished-candidate accounting, the conservative-refuse
|
||||||
|
branch) — accepted under the wave-1 gate's exception; naming and
|
||||||
|
one-module-per-file re-export shape clean. The `#![allow(dead_code)]`
|
||||||
|
scope notes follow the house pattern and now more accurately match
|
||||||
|
their remaining scope (see the inline fixes).
|
||||||
|
- **Inline fixes applied (review-of-record)**:
|
||||||
|
1. `src/facade.rs` — the stale `#[allow(dead_code)]` + stale
|
||||||
|
store-put-paths task-point comment on `Batch::add` removed: the
|
||||||
|
call sites landed (`put_in_batch`), the attribute outlived its
|
||||||
|
scope.
|
||||||
|
2. `src/dispatch.rs` — the module-level `#![allow(dead_code)]`
|
||||||
|
removed: `route`/`Tier` have their in-tree drivers now; clippy
|
||||||
|
is clean without it in all three feature shapes. The module
|
||||||
|
doc's scope note updated to match.
|
||||||
|
3. `src/backend.rs` — attempted the same shrink and **reverted**:
|
||||||
|
the trait's `name` (and kv `put` in default builds) still have
|
||||||
|
no in-tree caller, so the allow is load-bearing; its scope note
|
||||||
|
now records what is actually dead (reporting names, the kv
|
||||||
|
whole-value `put` until wave 3).
|
||||||
|
4. `src/mem.rs` — the stale module-doc sentence ("Clone is the
|
||||||
|
constructor until dispatch names the engine") updated to the
|
||||||
|
landed state (the facade's mem constructors construct it).
|
||||||
|
5. `tasks/gc-protocol.md` — two stray "> To be filled" placeholder
|
||||||
|
lines removed above its filled Notes/Summary content.
|
||||||
|
- **No wave-3 amendments arise from this gate.** The one amendment
|
||||||
|
wave 2 itself carried (the staged-put key-at-commit handout shape,
|
||||||
|
store-put-paths Notes) was audited as consistent, not amended
|
||||||
|
further.
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
> To be filled on completion
|
Wave-2 review gate complete: every gate check was read against the
|
||||||
|
cited ADR text line by line and verified — the three numbered
|
||||||
|
unknown-length rules each test-backed (flush/no-restart, whole-artifact
|
||||||
|
verification over both forms, failure hygiene across all four paths),
|
||||||
|
the delete-window ordering invariants (arbitrate-at-delete-time under
|
||||||
|
the one arbitration lock, pin-before-publish on both put paths and
|
||||||
|
both tiers, dedup refcounts, the raced conversion ordering, the
|
||||||
|
blocked-mint delete-then-recover), abort-as-data with exactly three
|
||||||
|
causes and the `GcRefuse`/full-protection-set direct path, dispatch as
|
||||||
|
a pure length function with the fall-through consult order and the
|
||||||
|
three constructor modes and kv-only's two put-time rejection points,
|
||||||
|
stage-hygiene exact counts on both engines including the Drop
|
||||||
|
cancellation story, both suite columns green and ADR-stamped with the
|
||||||
|
factory binding recorded, and the dispatch-coverage shape exercised at
|
||||||
|
the constructor and in the local column's dispatch rows. All eight
|
||||||
|
named design points are recorded and consistent with the specs; the
|
||||||
|
staged-put key-at-commit amendment was audited and accepted (operator
|
||||||
|
sign-off in place). Findings were minor and fixed inline: a stale
|
||||||
|
`Batch::add` dead-code allow, an unnecessary module allow in
|
||||||
|
dispatch.rs (clippy-verified removable), a stale mem doc line, the
|
||||||
|
backend.rs allow's scope note corrected (it stays, load-bearing), and
|
||||||
|
two placeholder strays in tasks/gc-protocol.md. No wave-3 amendments
|
||||||
|
arise. All gates re-run green after the fixes: `cargo test` (42 lib +
|
||||||
|
4 git-interop), `cargo test --features mem` and `cargo test
|
||||||
|
--all-features` (97 lib + 37 conformance + 4 git-interop), `cargo
|
||||||
|
clippy --all-targets -- -D warnings` and `--all-features
|
||||||
|
--all-targets` green, `cargo fmt --check` green. **Wave 3
|
||||||
|
decomposition is unblocked** — sqlite kv engine decomposes against
|
||||||
|
the contract surface as reviewed.
|
||||||
Reference in new issue
Block a user