alkblobs/tasks/review-wave-5.md

id, name, status, depends_on, scope, risk, impact, level, tags

id	name	status	depends_on	scope	risk	impact	level	tags
review-wave-5	Wave 5 review gate — the pg-lo engine vs the contract	completed	suite-column-pg-lo	narrow	low	phase	review	wave-5 review

Description

The wave-5 review gate — the engine-wave review shape (docs/plans/implementation.md §Review gates: "engine-vs-contract conformance"), before wave 6 decomposes. The gate reads the implementation against the pinned ADR text line by line (the wave-1/2/3/4 gates' method).

Gate checks:

• Engine-vs-contract conformance: the suite column green against dockerized postgres (server-less runs skip cleanly — a skip never substitutes for acceptance); the engine-state clause families contract-tested per the applicability row (ADR-012 §5's pg-lo row — the window family only, conditional per the one-window-host rule); virgin-store semantics; the size metadata-only discipline (the companion-row probe never loads LO content).
• The engine shape vs ADR-009 §1–§4: the companion table is the contract authority (list/size/has/CAS all table-level); pg_largeobject never read for addressing; put = one tx with the 512 KiB knee as a named constant (the C2 citation); the staged put's held-tx body with both hygiene forms exact-count tested; gets ride descriptorless lo_get(oid, off, len) windows (the C4 posture — the held-descriptor fallback named as not-shipped, never coded); the 64-bit-variants discipline (the C7 int4 trap not inherited); delete = one tx under table-level arbitration; transactional LO creation (zero crash orphans structurally — the C6 reading).
• The posture deltas vs ADR-009 §5/§6: carried as requirements, not hedges — catalog space never returned (the VACUUM FULL operator line + rel-size monitoring named in the module doc); autovacuum inherited, keep it on; the orphan-recovery sweep present for the legacy/bypass class (scope-noted, contract-tested with a planted orphan); the cached-get capacity facts named honestly.
• The one-window-host rule vs ADR-012 §5: the store-core completion — ensure_gc_schema follows the host rule at construction (dormancy on dual-SQL: the consolidation shape's pg-lo schema carries no gc_window; the pg-lo host on kv=mem); the sweep form branch runs the durable form on the pg-lo host for kv=mem + large=pg-lo and keys off the engine representation, never a flag; no fleet state anywhere in pg-lo (nothing of EngineFleetState); the fleet joint-put contribution's API shape explicitly deferred to wave 6 (recorded, not silently absent).
• The consolidation constructor vs ADR-009/011/012: the fleet-valid engine set completed; the implicitly-fleet: false posture recorded verbatim (wave 6's declaration slots in without reshaping); the connection-settings design point (two DSN+opts pairs; cross-instance valid non-fleet) recorded.
• Async/blocking discipline: tokio-postgres natively async — no spawn_blocking seam; no blocking I/O on the async path; the pool is the concurrency posture; no unwrap/expect outside tests.
• Named design points audited as resolved (each recorded in its task's Notes): the schema name + the consolidation shape's schema sharing; the shared-helper home (quote_identifier, 42P01 discriminant, pg_err, timestamp helpers) + the PgOpts gate question; the 512 KiB aggregation shape; the LO isolation + janitor shape (amendment A2 verified — zero schema AND zero pg_largeobject residue on the shared db); the sub-enum slotting for the large cursor/staging bodies; the connection-settings shape; the dormancy wiring; the orphan sweep's scope note.
• Family conventions: no comments in code (doc comments on public API fine), thiserror, tokio-only async, no unwrap/expect outside tests, naming, one-module-per-file re-export shape, the #![allow(dead_code)] scope notes matching their remaining scope.
• Gates green: cargo test (default), cargo test --features pg-lo (server-less: skipping), cargo test --features "pg-lo,mem", cargo test --all-features (against the server), the pg-lo pure builds (cargo check --no-default-features --features "pg-lo,mem" and --features "postgres,pg-lo" — the literal kv-less --features pg-lo config is not a gate: with no kv engine the facade's KvEngine/KvCursor go empty, E0004; see store-pg-lo-construction), cargo clippy --all-targets -- -D warnings (default + the pg-lo variants + --all-features), cargo fmt --check, cargo doc --no-deps.

Findings are fixed inline where cheap, or recorded as wave-6 amendments where they shape the next decomposition (the fleet mechanism's joint-tx driver already carries amendment A1; any pg-lo contribution-shape finding lands beside it). The gate's completion is what unblocks wave 6's decomposition.

Acceptance Criteria

• 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 the specs
• 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)

References

• docs/plans/implementation.md (§Wave 5, §Review gates — engine wave reviews)
• docs/architecture/decisions/009-pg-lo-large-tier-engine.md
• docs/architecture/decisions/010-backend-io-seams-and-fleet-state-home.md §1/§2
• docs/architecture/decisions/012-pre-decomposition-consistency-rulings.md §3/§4/§5/§6.7
• docs/architecture/decisions/013-family-composition-reactive-boundary-and-feature-graph.md §4
• docs/architecture/store-api.md
• docs/research/poc-pglo-findings.md (the evidence base the engine claims)
• tasks/review-wave-4.md (the gate method; the structural twin's gate)

Notes

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

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.