docs(review): the wave-3 review gate — every gate check green
Reviewed the wave-3 commits (engine-sqlite, engine-sqlite-window, store-sqlite-construction, store-sweep-durable-window, suite-column-sqlite) against the cited ADR/spec text line by line: ADR-003/007/008 §2, ADR-010 §2, ADR-012 §1/§5/§6.7, store-api.md, implementation.md §Review gates — the engine-vs-contract conformance (suite column green server-less in the default build; the window family contract-tested per its applicability row; virgin-store semantics incl. the no-tables-yet form; the size metadata-only discipline proven index-riding), the durability posture (WAL + synchronous=NORMAL verified at open; the single-writer ceiling posture, not hedge), the durable sweep form vs ADR-008 §2/ADR-012 §6.7 (stage at mark_end + window; next-sweep execution with raced execution-time re-arbitration; schedule-relative exact counts; aborts stage and execute nothing), the seam reduction (gc_window only; no pin rows, no sweep-lock body, no EngineFleetState), and the blocking discipline (all rusqlite in spawn_blocking). Named design points audited as resolved; family conventions clean; no wave-4 amendments arise. Inline fixes (review-of-record): - src/gc.rs: the seam's window_stage doc retired the wave-1 lazy-create hedge for the shipped constructor-at-open posture (a tableless database surfaces the engine error on stage; per-engine tolerances are each engine's recorded posture) - tasks/engine-sqlite-window, store-sweep-durable-window, suite-column-sqlite: stray placeholder lines above the filled Notes/Summary removed (the wave-2 gate's same cleanup) - tests/conformance/README.md: trailing newline added Verification (all re-run green after the fixes): - cargo test (default): 69 lib + 19 conformance + 3 git-interop - cargo test --features mem / --all-features: 124 lib + 54 conformance + 4 git-interop each - cargo clippy --all-targets -- -D warnings (default + all-features) - cargo fmt --check - cargo doc --no-deps at the pre-existing warning baseline (26) Wave 4 (postgres) decomposition is unblocked.
This commit is contained in:
1 parent
3f8e8158a7
commit
97cdf682c4
6 files changed
+180
-24
No files matched your search
@@ -561,9 +561,14 @@ pub(crate) trait EngineWindowState: Backend {
|
|||||||
|
|
||||||
/// Stage candidates into this engine's durable delete window (idempotent
|
/// Stage candidates into this engine's durable delete window (idempotent
|
||||||
/// staging per `(key, deadline)` is the body's discipline; the store
|
/// staging per `(key, deadline)` is the body's discipline; the store
|
||||||
/// stages each candidate once per sweep). Virgin store: creates the
|
/// stages each candidate once per sweep). The window's schema runs at
|
||||||
/// window lazily or errors nothing — reads and no-op writes stay
|
/// store construction (the store-core [`Self::ensure_gc_schema`] call,
|
||||||
/// no-ops (the Backend trait's engine posture carries here).
|
/// the constructor-vs-lazy timing design point), so staging always sees
|
||||||
|
/// its table in real service — no lazy create. A genuinely tableless
|
||||||
|
/// database surfaces the engine error on stage rather than silently
|
||||||
|
/// creating schema; staging tolerance beyond that is each engine's
|
||||||
|
/// recorded posture (the Backend trait's engine posture carries;
|
||||||
|
/// sqlite's is recorded in src/sqlite.rs's module doc).
|
||||||
async fn window_stage(&self, candidates: &[WindowCandidate]) -> Result<(), Error>;
|
async fn window_stage(&self, candidates: &[WindowCandidate]) -> Result<(), Error>;
|
||||||
|
|
||||||
/// Take overdue candidates (`visible_after <= now`): the rows leave
|
/// Take overdue candidates (`visible_after <= now`): the rows leave
|
||||||
|
|||||||
@@ -81,8 +81,6 @@ this task ships the clauses and their engine-level contract tests.
|
|||||||
|
|
||||||
## Notes
|
## Notes
|
||||||
|
|
||||||
> To be filled by implementation agent
|
|
||||||
|
|
||||||
- The `EngineWindowState` impl lives in `src/sqlite.rs` after the
|
- The `EngineWindowState` impl lives in `src/sqlite.rs` after the
|
||||||
`Backend` impl, as `impl EngineWindowState for SqliteBackend`. The
|
`Backend` impl, as `impl EngineWindowState for SqliteBackend`. The
|
||||||
pin clauses and the sweep-lock body are absent by construction —
|
pin clauses and the sweep-lock body are absent by construction —
|
||||||
@@ -128,8 +126,6 @@ this task ships the clauses and their engine-level contract tests.
|
|||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
> To be filled on completion
|
|
||||||
|
|
||||||
The sqlite engine-state seam's window family landed: `impl
|
The sqlite engine-state seam's window family landed: `impl
|
||||||
EngineWindowState for SqliteBackend` in `src/sqlite.rs` —
|
EngineWindowState for SqliteBackend` in `src/sqlite.rs` —
|
||||||
`ensure_gc_schema` (creates `gc_window` only, never `gc_pins`,
|
`ensure_gc_schema` (creates `gc_window` only, never `gc_pins`,
|
||||||
|
|||||||
+171
-8
@@ -1,7 +1,7 @@
|
|||||||
---
|
---
|
||||||
id: review-wave-3
|
id: review-wave-3
|
||||||
name: Wave 3 review gate — the sqlite engine vs the contract
|
name: Wave 3 review gate — the sqlite engine vs the contract
|
||||||
status: pending
|
status: completed
|
||||||
depends_on: [suite-column-sqlite]
|
depends_on: [suite-column-sqlite]
|
||||||
scope: narrow
|
scope: narrow
|
||||||
risk: low
|
risk: low
|
||||||
@@ -67,14 +67,14 @@ completion is what unblocks wave 4'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-4 amendments with
|
- [x] Findings fixed inline or recorded as wave-4 amendments with
|
||||||
owners
|
owners
|
||||||
- [ ] All gates re-run green after any fixes
|
- [x] All gates re-run green after any fixes
|
||||||
- [ ] Wave 4 decomposition unblocked (recorded in this task's Summary)
|
- [x] Wave 4 decomposition unblocked (recorded in this task's Summary)
|
||||||
|
|
||||||
## References
|
## References
|
||||||
|
|
||||||
@@ -89,8 +89,171 @@ completion is what unblocks wave 4's decomposition.
|
|||||||
|
|
||||||
## Notes
|
## Notes
|
||||||
|
|
||||||
> To be filled by the review agent
|
> Filled by the review agent (2026-10-12). Every gate check was read
|
||||||
|
> against the cited ADR/spec text (the files, not memory): ADR-003,
|
||||||
|
> ADR-007, ADR-008 §2, ADR-010 §2, ADR-012 §1/§5/§6.7 (plus §2–§4/§6.1–
|
||||||
|
> §6.5 where the gate's checks cite them), docs/plans/implementation.md
|
||||||
|
> §Review gates, store-api.md, and the gate method from review-wave-2.
|
||||||
|
> Findings below; everything else passed.
|
||||||
|
|
||||||
|
- **Engine-vs-contract conformance** — verified. The suite column is
|
||||||
|
green server-less and runs in the default build (feature `sqlite`
|
||||||
|
default-on, ADR-007/013 §4; 19 conformance tests, never skips —
|
||||||
|
README §SKIP posture honored). The window family is contract-tested
|
||||||
|
per its ADR-012 §5 applicability row: schema creates `gc_window`
|
||||||
|
only (`gc_pins` asserted never created, through `sqlite_master`, on
|
||||||
|
constructed and tableless databases alike; the twice-at-open call
|
||||||
|
idempotent), staging idempotence per `(key, deadline)` incl. the
|
||||||
|
re-stage-at-new-deadline upsert form, take returns exactly the
|
||||||
|
overdue set and removes it atomically, future rows left in place —
|
||||||
|
and nothing of `EngineFleetState` is implementable or implemented on
|
||||||
|
the engine. Virgin-store semantics: all read paths are no-ops
|
||||||
|
against a genuinely tableless database file (has/get/size/list →
|
||||||
|
absent/empty, delete → `Missing`; `window_take` → empty), tested at
|
||||||
|
both engine and facade form; the write-read-only split (stage
|
||||||
|
surfaces the engine error, never silently creates schema) is the
|
||||||
|
recorded constructor-vs-lazy design point. The `size` metadata-only
|
||||||
|
discipline: the dedicated size column probed via `SELECT size WHERE
|
||||||
|
key = ?`, exact for large/absent keys, and `EXPLAIN QUERY PLAN`
|
||||||
|
asserts the probe rides the key's index (`USING INDEX`) — never a
|
||||||
|
scan or value load.
|
||||||
|
- **The sqlite posture vs ADR-003/007** — verified. WAL +
|
||||||
|
`synchronous=NORMAL` set at open `PRAGMA`-verified (the constructor
|
||||||
|
probes the mode actually engaged; a `:memory:` database that cannot
|
||||||
|
hold WAL is a construction error, tested; the pragma posture asserted
|
||||||
|
on the backend's own connection — `synchronous` is per-connection).
|
||||||
|
The single-writer ceiling is recorded as posture, not hedged (the
|
||||||
|
module doc carries the ~1.2k objects/s POC #5 B3 number beside
|
||||||
|
REQ-1's economics, matching ADR-007's text). `prepare_cached`
|
||||||
|
discipline throughout (every engine statement); `has` is the
|
||||||
|
`SELECT EXISTS` probe; the B5 row shape
|
||||||
|
`blobs (key BLOB PRIMARY KEY, value BLOB, size INTEGER NOT NULL)`;
|
||||||
|
reads bounded (the materializing body holds only kv-tier content;
|
||||||
|
the 64 KiB put-chunk; the staged commit's full publication through
|
||||||
|
the same `INSERT OR IGNORE` clause).
|
||||||
|
- **The durable-window sweep vs ADR-008 §2 / ADR-012 §5/§6.7** —
|
||||||
|
verified. Candidates staged at `visible_after = mark_end + window`
|
||||||
|
with the constructor-tunable delay (`DEFAULT_DELETE_WINDOW_DELAY`,
|
||||||
|
5 min pinned and constructor-passable; `Duration::ZERO` the legal
|
||||||
|
degenerate test tuning). The executor is the next sweep: the sweep
|
||||||
|
body runs `execute_overdue_window` (take-then-re-arbitrate-at-
|
||||||
|
execution-time, batched ~100) before its own mark, exactly §6.7's
|
||||||
|
"before/alongside its own marking". Execution-time re-arbitration:
|
||||||
|
the race-across-the-delay test proves a put landing during the
|
||||||
|
window survives via its pin (pin-before-publish) cancelling the
|
||||||
|
candidate. Report counts exact relative to the schedule:
|
||||||
|
`candidates_staged` = staged this sweep, `deleted`/`cancelled` = the
|
||||||
|
overdue set executed this sweep; the durable abort shapes
|
||||||
|
(`NoLivenessSources`/`ProtectFailed`/`SweeperLock`) delete, execute,
|
||||||
|
and stage nothing (the ProtectFailed row proves the overdue set
|
||||||
|
stays in the window — abort precedes `window_take`). The inline form
|
||||||
|
is preserved verbatim (extracted body; the mem/local wave-2 suites
|
||||||
|
green unchanged). The single-node posture is unchanged (pins/
|
||||||
|
liveness/arbitration lock in-process; only the window's persistence
|
||||||
|
is engine rows) and the one-window-host posture is recorded in the
|
||||||
|
store module doc + constructor docs (the form branch keys off the
|
||||||
|
kv-engine enum, never a flag; wave-5 pg-lo dormancy slots in).
|
||||||
|
- **The seam reduction honored** — verified. No pin rows and no
|
||||||
|
sweep-lock body anywhere in the sqlite engine (dead machinery per
|
||||||
|
ADR-012 §5 dropped by construction — not stubbed); `ensure_gc_schema`
|
||||||
|
creates `gc_window` only; the embedder's liveness table is never
|
||||||
|
created by the engine (ADR-008's schema-ownership rule, doc-pinned at
|
||||||
|
the seam and asserted in tests).
|
||||||
|
- **Blocking discipline** — verified. Every rusqlite operation
|
||||||
|
(constructor, all seven methods, the staged commit, the window
|
||||||
|
clauses, the test instruments) routes through the `blocking`
|
||||||
|
`spawn_blocking` helper; the cursor/staging read bodies are
|
||||||
|
in-memory after materialization and need none. No blocking I/O
|
||||||
|
under internal async locks: the GcHome arbitration lock stays
|
||||||
|
short-held and I/O-free (the marker mechanism), the sqlite
|
||||||
|
connection mutex is short-held and lives inside `spawn_blocking`.
|
||||||
|
The connection posture (one connection behind a short-held
|
||||||
|
`parking_lot::Mutex`, shared with staging handouts via `Arc`,
|
||||||
|
parking_lot non-poisoning) is recorded.
|
||||||
|
- **Named design points audited as resolved** — each recorded in its
|
||||||
|
task's Notes and consistent with the specs: the connection posture
|
||||||
|
(engine-sqlite Notes + module doc); the timestamp encoding (signed
|
||||||
|
i64 nanos since epoch, sub-second precision for test tunings,
|
||||||
|
pre-epoch negatives — boundary-tested); schema-creation timing vs
|
||||||
|
the no-tables-yet tolerance (constructor `CREATE TABLE IF NOT
|
||||||
|
EXISTS`, read-only tolerance, write surfaces the error — engine-
|
||||||
|
tested against a tableless file); the kv cursor dispatch shape
|
||||||
|
(`KvCursor` sub-enum behind `AnyCursor`'s one kv arm — per-engine
|
||||||
|
variants rejected for copying the materializing arm's consumers;
|
||||||
|
postgres slots one arm in at wave 4); the protect-consult posture
|
||||||
|
on overdue execution (the consult runs once per sweep above the
|
||||||
|
form branch and extends to the executor phase — overdue
|
||||||
|
re-arbitrations carry `consult_protect = false`, the inline form's
|
||||||
|
same posture; the ProtectFailed-before-execution test pins it); the
|
||||||
|
delay-0 test tuning's legality (`Duration::ZERO` the legal
|
||||||
|
degenerate tuning, execute-on-next-sweep, ADR-008 §2
|
||||||
|
constructor-tunable reading).
|
||||||
|
- **Family conventions** — clean. No unwrap/expect/panic outside
|
||||||
|
`#[cfg(test)]` (audited every occurrence); thiserror family (engine
|
||||||
|
errors surface as `Error::Io` via `db_err`, `Missing` via the
|
||||||
|
rows-affected/virgin-store mapping); tokio-only async; naming and
|
||||||
|
one-module-per-file re-export shape hold (`DEFAULT_DELETE_WINDOW_DELAY`
|
||||||
|
re-exported from lib.rs); the `#![allow(dead_code)]` scope notes
|
||||||
|
updated to the landed state (backend.rs's kv-put line; gc.rs's
|
||||||
|
window-family-has-callers line; sqlite.rs's cfg-gated scope).
|
||||||
|
- **Boundary-wording note (no fix, recorded)**: ADR-012 §6.7 says
|
||||||
|
overdue rows are taken "past `visible_after`" while the seam's
|
||||||
|
operative contract (src/gc.rs's clause doc, wave-1-pinned and
|
||||||
|
contract-tested) is `visible_after <= now` — a row visible exactly
|
||||||
|
at `now` is taken. The nanosecond-scale divergence is immaterial
|
||||||
|
(the window delay is minutes; execution-time re-arbitration
|
||||||
|
protects regardless), the seam doc is the crate-internal contract
|
||||||
|
the engine implements, and the ADR's prose is loose where the seam
|
||||||
|
is precise. No change.
|
||||||
|
- **Inline fixes applied (review-of-record)**:
|
||||||
|
1. `src/gc.rs` — `EngineWindowState::window_stage`'s doc still
|
||||||
|
carried the wave-1 hedge ("Virgin store: creates the window
|
||||||
|
lazily or errors nothing"), contradicting the shipped posture's
|
||||||
|
recorded design point (staging surfaces the engine error on a
|
||||||
|
tableless database; the lazy-create option was rejected in
|
||||||
|
engine-sqlite-window's Notes). The clause doc now states the
|
||||||
|
settled posture: the schema runs at store construction (the
|
||||||
|
store-core `ensure_gc_schema` call), a tableless database
|
||||||
|
surfaces the error rather than silently creating schema, and
|
||||||
|
per-engine tolerances are each engine's recorded posture.
|
||||||
|
2. `tasks/engine-sqlite-window.md`, `tasks/store-sweep-durable-
|
||||||
|
window.md`, `tasks/suite-column-sqlite.md` — the stray "> To be
|
||||||
|
filled…" placeholder lines above their already-filled
|
||||||
|
Notes/Summary bodies removed (the wave-2 gate's same cleanup).
|
||||||
|
3. `tests/conformance/README.md` — trailing newline added (the
|
||||||
|
wave-3 amendment hunk ended the file mid-line).
|
||||||
|
- **No wave-4 amendments arise from this gate.**
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
> To be filled on completion
|
Wave-3 review gate complete: every gate check was read against the
|
||||||
|
cited ADR/spec text line by line and verified — the sqlite engine's
|
||||||
|
contract conformance (WAL+NORMAL pragma posture verified at open,
|
||||||
|
`prepare_cached` discipline, the EXISTS probe, the B5 row shape, the
|
||||||
|
indexed `size` probe proven index-riding via `EXPLAIN QUERY PLAN`,
|
||||||
|
CAS first-writer-wins on both put forms, both stage-hygiene forms,
|
||||||
|
virgin-store reads incl. the no-tables-yet form against a genuinely
|
||||||
|
tableless file), the window family contract-tested per its ADR-012 §5
|
||||||
|
applicability row (`gc_window` only, no pin rows, no sweep-lock body,
|
||||||
|
no `EngineFleetState` anywhere), the durable sweep form against
|
||||||
|
ADR-008 §2/ADR-012 §6.7 (stage at `mark_end + window`, the next sweep
|
||||||
|
as executor with execution-time re-arbitration raced and proven,
|
||||||
|
schedule-relative exact counts, the three abort shapes executing and
|
||||||
|
staging nothing, the protect-consult posture recorded), the seam
|
||||||
|
reduction honored, the blocking discipline (all rusqlite in
|
||||||
|
`spawn_blocking`, no I/O under internal async locks), the conformance
|
||||||
|
column green server-less in the default build with the medium oracle
|
||||||
|
and durable-schedule rows (README amendment recorded), and all named
|
||||||
|
design points decided/recorded/consistent. Findings were minor and
|
||||||
|
fixed inline: the stale lazy-create hedge on the seam's
|
||||||
|
`window_stage` doc amended to the settled posture, three task-file
|
||||||
|
placeholder strays removed, README's missing trailing newline. No
|
||||||
|
wave-4 amendments arise. All gates re-run green after the fixes:
|
||||||
|
`cargo test` (69 lib + 19 conformance + 3 git-interop, default),
|
||||||
|
`cargo test --features mem` and `cargo test --all-features` (124 lib +
|
||||||
|
54 conformance + 4 git-interop each), `cargo clippy --all-targets --
|
||||||
|
-D warnings` and `--all-features --all-targets` green, `cargo fmt
|
||||||
|
--check` green, `cargo doc --no-deps` at the pre-existing warning
|
||||||
|
baseline (26, unchanged). **Wave 4 decomposition is unblocked** — the
|
||||||
|
postgres kv engine decomposes against the contract surface as
|
||||||
|
reviewed.
|
||||||
@@ -92,8 +92,6 @@ window, single-node:
|
|||||||
|
|
||||||
## Notes
|
## Notes
|
||||||
|
|
||||||
> To be filled by implementation agent
|
|
||||||
|
|
||||||
The durable form's shape as it landed:
|
The durable form's shape as it landed:
|
||||||
|
|
||||||
- **The branch is the kv-engine enum match itself** (`sweep_window`'s
|
- **The branch is the kv-engine enum match itself** (`sweep_window`'s
|
||||||
@@ -126,8 +124,6 @@ The durable form's shape as it landed:
|
|||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
> To be filled on completion
|
|
||||||
|
|
||||||
The sweep's durable SQL-backed form landed beside the inline form in
|
The sweep's durable SQL-backed form landed beside the inline form in
|
||||||
`src/store.rs` (the first durable delete window, single-node; ADR-008
|
`src/store.rs` (the first durable delete window, single-node; ADR-008
|
||||||
§2 staged re-arbitrated delete, ADR-012 §5/§6.7 the-executor-is-the-
|
§2 staged re-arbitrated delete, ADR-012 §5/§6.7 the-executor-is-the-
|
||||||
|
|||||||
@@ -101,8 +101,6 @@ the default build (feature `sqlite` is default-on).
|
|||||||
|
|
||||||
## Notes
|
## Notes
|
||||||
|
|
||||||
> To be filled by implementation agent
|
|
||||||
|
|
||||||
The column as it landed (`tests/conformance/sqlite_column.rs`, 17 tests):
|
The column as it landed (`tests/conformance/sqlite_column.rs`, 17 tests):
|
||||||
|
|
||||||
- **Factories**: `dual-tier sqlite+local` (the default-build
|
- **Factories**: `dual-tier sqlite+local` (the default-build
|
||||||
@@ -158,8 +156,6 @@ The column as it landed (`tests/conformance/sqlite_column.rs`, 17 tests):
|
|||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
> To be filled on completion
|
|
||||||
|
|
||||||
The sqlite column — the suite's first SQL-backed column — is green:
|
The sqlite column — the suite's first SQL-backed column — is green:
|
||||||
the default-build production shape (`kv = sqlite` + `large = local`,
|
the default-build production shape (`kv = sqlite` + `large = local`,
|
||||||
feature `sqlite` default-on) runs the form-independent reference rows
|
feature `sqlite` default-on) runs the form-independent reference rows
|
||||||
|
|||||||
@@ -360,4 +360,4 @@ feature-gated on `sqlite` — **default-on**, so the column runs in the
|
|||||||
default build: every `cargo test` run exercises it. The sqlite
|
default build: every `cargo test` run exercises it. The sqlite
|
||||||
engine's no-tables-yet read tolerance is engine-instrumented
|
engine's no-tables-yet read tolerance is engine-instrumented
|
||||||
(src/sqlite.rs's tests; the constructor-vs-lazy timing design point
|
(src/sqlite.rs's tests; the constructor-vs-lazy timing design point
|
||||||
there), the column's virgin-store row being its facade form.
|
there), the column's virgin-store row being its facade form.
|
||||||
Reference in new issue
Block a user