From 93f9a9332cf4a8b710fa9f1641d0c5de27bb4096 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Mon, 12 Oct 2026 13:56:00 +0000 Subject: [PATCH] =?UTF-8?q?docs(review):=20the=20wave-5=20review=20gate=20?= =?UTF-8?q?=E2=80=94=20every=20gate=20check=20green?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engine-wave gate read the pg-lo engine, its window-family seam, the constructor wiring, and the suite column against the pinned ADR text and found them conformant. One finding fixed inline (PGLO-1, the wave-5 rustdoc private-link warnings — see the previous commit); no wave-6 amendments arise from this gate. All gates re-run green after the fix. Wave 6 (fleet GC) decomposition is unblocked. --- tasks/review-wave-5.md | 161 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 153 insertions(+), 8 deletions(-) diff --git a/tasks/review-wave-5.md b/tasks/review-wave-5.md index f463dc9..5ad4129 100644 --- a/tasks/review-wave-5.md +++ b/tasks/review-wave-5.md @@ -1,7 +1,7 @@ --- id: review-wave-5 name: Wave 5 review gate — the pg-lo engine vs the contract -status: pending +status: completed depends_on: [suite-column-pg-lo] scope: narrow risk: low @@ -92,14 +92,14 @@ what unblocks wave 6's decomposition. ## 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) -- [ ] Named design points each decided, recorded, and consistent with +- [x] Named design points each decided, recorded, and consistent with the specs -- [ ] Findings fixed inline or recorded as wave-6 amendments with +- [x] Findings fixed inline or recorded as wave-6 amendments with owners -- [ ] All gates re-run green after any fixes -- [ ] Wave 6 decomposition unblocked (recorded in this task's Summary) +- [x] All gates re-run green after any fixes +- [x] Wave 6 decomposition unblocked (recorded in this task's Summary) ## References @@ -116,8 +116,153 @@ what unblocks wave 6's decomposition. ## Notes -> To be filled by implementation agent +> The gate read the four wave-5 commits against the pinned ADR text +> (ADR-009 §1–§6, ADR-010 §1/§2, ADR-012 §3/§4/§5/§6.5/§6.7, ADR-013 +> §4, store-api.md, POC #7 findings C1–C7 — the documents, not memory) +> and re-ran every gate. The engine is conformant; **one finding, +> fixed inline**: +> +> - **PGLO-1 — `cargo doc --no-deps` was not clean (fixed).** Wave 5 +> introduced four rustdoc warnings: the public `store` module doc's +> new intra-doc links resolve to crate-internal items +> (`AnyCursor`, `KvCursor`, `LargeCursor`, and the wave-5 store-core +> helper `ensure_gc_schema_at_construction` — the last two new this +> wave; the first two were prose in wave 4 and became bracketed +> links here), which rustdoc warns as private-item links from +> public documentation. Fixed by de-linking those names in the +> module doc (`src/store.rs`, doc-only) — the wave-4 gate's own +> rustdoc-hygiene fix was the precedent. Verified post-fix: `cargo +> doc --no-deps` zero warnings, clippy/fmt unaffected. +> +> Gate checks verified, item by item: +> +> - **Engine-vs-contract conformance** — the pg-lo column green +> against the harness server (re-run live this gate: +> `--all-features` 188 lib + 106 conformance + 4 git-interop ok; +> zero LO/schema/database residue after the run); the window-family +> clause bodies contract-tested per ADR-012 §5's pg-lo row (schema +> creates `gc_window` only with `gc_pins`'s absence asserted, +> stage idempotence/restage, the atomic `DELETE … RETURNING` take +> with the `<=` boundary, both virgin shapes); virgin-store +> semantics unit-tested incl. the schema-less form; `size` +> metadata-only from the companion row (EXPLAIN PK-index gate). +> - **Engine shape vs ADR-009 §1–§4** — the companion table is the +> contract authority (list/size/has/CAS all table-level; `pg_largeobject` +> read by the engine only inside the orphan sweep's candidate +> enumeration); `get` closes over the companion row, rides +> descriptorless `lo_get(oid, off, len)` windows, and the +> held-descriptor fallback is named not-shipped in the module doc +> with no code; put = one tx with CAS existence first (a re-put +> commits an empty tx, never `lo_create`) and the 512 KiB knee as +> [`crate::pg_lo::LOWRITE_CHUNK`] carrying the C2 citation; both +> hygiene forms exact-count tested (discard → zero pages/rows, +> fail-commit → self-discard, Drop cancellation → spawned +> ROLLBACK + zero residue); the 64-bit-variants discipline holds +> (no `lo_lseek` anywhere, offsets bigint only, window length +> within the int4 parameter domain, the u64 clamping arithmetic +> shared); delete = one tx with the Missing discipline; LO +> creation transactional (C6 zero-orphan — structurally, from the +> held tx). +> - **Posture deltas carried as requirements** — the module doc +> states the catalog-space-never-returned property with the +> `VACUUM FULL`/`pg_repack` operator line and +> `pg_total_relation_size('pg_largeobject')` monitoring, the +> autovacuum-inherited keep-it-on note, and the 20–50× single-stream +> cached-get capacity fact with the aggregate serving picture; the +> orphan-recovery sweep is scope-noted (no store-core verb this +> wave) and contract-tested with a planted orphan + byte-exact +> committed-content survival. The cached-get capacity facts named +> honestly, not hedged. +> - **One-window-host rule vs ADR-012 §5** — the store core's +> `ensure_gc_schema_at_construction` dispatch keys off the engine +> representation, never a flag (kv host on dual-SQL — the pg-lo +> schema carries no `gc_window`, contract-tested genuine absence +> on two distinct schemas; pg-lo host on kv = mem); the sweep's +> mem arm runs the durable form on the pg-lo host (generic-over- +> `EngineWindowState` delegation), inline otherwise; **nothing of +> `EngineFleetState` exists in `src/pg_lo.rs`** (grep-clean — the +> one mention is a doc sentence recording the ruling); the fleet +> joint-put contribution's API shape is explicitly deferred to wave +> 6 (recorded in `src/gc.rs`'s `EngineFleetState` clause doc and +> the engine task's scope boundary, not silently absent). +> - **Consolidation constructor vs ADR-009/011/012** — the fleet-valid +> engine set is complete (`dual_tier_postgres_pglo` landed; the +> module doc records the completion), the implicitly-`fleet: false` +> posture is recorded verbatim in the constructor doc (wave 6's +> declaration slots in without reshaping), and the connection- +> settings design point (two DSN+opts pairs; the consolidation +> corollary of one minted schema — both engines' table names are +> distinct) is recorded with it. +> - **Async/blocking discipline** — natively async throughout, no +> `spawn_blocking` seam, no blocking I/O on the async path; the +> pool is the concurrency posture; no `unwrap`/`expect` outside +> tests. The two deliberate `std::thread`+`block_on` bodies are +> best-effort cancellation/test teardown paths with their +> rationale recorded (the staging cursor's `Drop`, the +> `#[cfg(test)]` janitors). +> - **Named design points audited as resolved** (each in its task's +> Notes, consistent with the specs): the schema name + +> consolidation schema sharing (`pg_common::DEFAULT_SCHEMA`, +> distinct table names), the shared-helper home (`src/pg_common.rs` +> gated `any(postgres, pg-lo)`) + the `PgOpts` gate widening (one +> shared type, re-export gate widened; the `synchronous_commit` +> knob carries with ship default `on`), the 512 KiB aggregation +> shape, the LO isolation + janitor shape (amendment A2 verified — +> per-test dedicated databases + `DatabaseJanitor`, zero schema +> AND zero `pg_largeobject` residue on the shared db re-run by this +> gate), the sub-enum slotting (`LargeCursor`/`LargeStaging` behind +> `AnyCursor`/`AnyStaging`'s one large arm), the +> connection-settings shape, the dormancy wiring, and the orphan +> sweep's scope note. +> - **Family conventions** — clean: doc comments on public API, no +> inline comments (the one cfg-shim comment documents a non-obvious +> cfg constraint, the allowed case), thiserror, tokio-only async, +> no unwrap/expect outside tests, naming and one-module-per-file +> re-export shape held, the `#![allow(dead_code)]` notes matching +> their remaining scope (`reap_orphans` awaiting its wave-7+ +> consumer — the postgres engine's twin posture). +> +> Verification, re-run by this gate (harness server live): `cargo +> test` 69/19/3 ok (default); `cargo test --features pg-lo` 92/19/3 +> ok; `cargo test --features "pg-lo,mem"` 151/71/4 ok; `cargo test +> --all-features` 188/106/4 ok against the server; `cargo check` + +> clippy `-D warnings` green on `--no-default-features --features +> "pg-lo,mem"` and `--features "postgres,pg-lo"` (the pure builds; +> the literal kv-less config is not a gate, d69d9a3's recorded +> finding) and on default/`pg-lo`/`postgres`/all-features +> effectively via the sweep above; `cargo fmt --check` green; `cargo +> doc --no-deps` clean post-fix. No wave-6 amendments arise from +> this gate; amendment A1 (the joint-tx drop discipline) stands from +> the wave-4 gate. ## Summary -> To be filled on completion \ No newline at end of file +> Completed 2026-10-12 (wave 5). The engine-wave gate read the pg-lo +> engine, its window-family seam, the constructor wiring, and the +> suite column against the pinned contract line by line and found +> them conformant — every gate check verified against the cited ADR +> text: ADR-009 §1–§4's engine shape (companion-table authority, +> one-tx puts with the 512 KiB knee, descriptorless `lo_get` windows +> with the held-descriptor fallback named not-shipped, 64-bit +> discipline, one-tx deletes, transactional LO creation), both +> hygiene forms + Drop cancellation exact-count tested, §5/§6's +> posture deltas stated as requirements (VACUUM FULL operator line, +> autovacuum, the 20–50× cached-get capacity fact), ADR-012 §5's +> one-window-host rule completed in the store core (kv host on +> dual-SQL with contract-tested genuine pg-lo dormancy; pg-lo host on +> kv = mem with the durable sweep form; the dispatch keyed off the +> engine representation, never a flag; nothing of +> `EngineFleetState`; the fleet joint-put API shape explicitly +> deferred to wave 6), consolidation + mem-pglo constructors with +> the implicitly-`fleet: false` posture recorded verbatim, the LO- +> isolation (A2) verified zero-residue, and all named design points +> audited as decided and recorded. One finding fixed inline (PGLO-1: +> four wave-5-introduced rustdoc private-link warnings — the gate's +> `cargo doc` claim had silently regressed; de-linked the private +> names). All gates re-run green after the fix (test variants, the +> two pure builds, clippy across the pg-lo feature matrix, fmt, doc). +> No wave-6 amendments arise from this gate. **Wave 6 (fleet GC) +> decomposition is unblocked** — the fleet-valid engine set is +> complete, the joint-tx driver carries amendment A1 from the wave-4 +> gate, and the constructors need only the declaration's predicate +> and parameter. \ No newline at end of file