Files
alkgit/docs/reviews/003-substrate-rebaseline-review.md
T
glm-5.3-flash 38757baa98 docs(architecture): review-003 substrate re-baseline review + fixes; specs flip reviewed
Gate review for the re-baseline cycle (ADR-019..025): contradiction +
gap + staleness pass, all findings dispositioned 2026-10-20.

- C-1: ADR-013's pre-substrate ingestion + CAS composition annotated
  (superseded composition kept as the decision record; §7 CAS home is
  the substrate row transaction per ADR-019 §2)
- C-2: ADR-018's storage_root rationale superseded (record carries no
  path; impls key on repo_id), Invalid doc de-staled
- W-1: ADR-017 §2 construction shape annotated per ADR-023 §2
- W-2 pinned: pin-custody across the frozen trait seam (composition-
  internal pending-pin registry, content-addressed correlation,
  post-CAS guard commits, GC-decreed recovery) — substrate.md
- W-3 decided: put_repo seeds HEAD -> refs/heads/main in the record
  transaction (git init parity; v1 replication limitation stated in
  ADR-023 §4; the unborn-HEAD OQ-19 fixture falls out of it)
- W-4 fixed: substrate freeze-inventory member list completed (the
  five trait impls public; receive-pack composition internal)
- W-5/S-1..S-5: disk-odb staleness annotations (ADR-004/014), naming
  fix (session substrate types), deferred(scope) taxonomy covers
  booked work items, ADR-020 typo, ADR-009 peek-field note, OQ-20
  tracker task added
- OQ-19 reclassified open -> deferred(scope) (blocked on the booked
  wave item); OQ-21 blocked-on wording aligned
- Specs (6) + README + open-questions flipped draft -> reviewed;
  AGENTS.md lifecycle updated

Verification: taskgraph validate (10 tasks), cargo test, clippy
-D warnings, fmt --check, cargo doc --no-deps
2026-10-20 16:42:43 +00:00

9.2 KiB
Raw Permalink Blame History


status: resolved last_updated: 2026-10-20 reviewed_artifacts:

  • docs/architecture/README.md
  • docs/architecture/overview.md
  • docs/architecture/transport.md
  • docs/architecture/backend.md
  • docs/architecture/substrate.md
  • docs/architecture/lfs.md
  • docs/architecture/doors.md
  • docs/architecture/open-questions.md
  • docs/architecture/decisions/ (ADR-019..025 — the re-baseline package; the pre-substrate ADRs they amend: 004, 009, 013, 014, 017, 018)
  • docs/research/substrate-recomposition.md + the four re-baseline evidence docs (pocl/poco/pocr/pocx)
  • docs/sdd_process.md tool: general subagent review (fresh-context full-corpus read) per the architecture-reviewer standard + architect verification of findings and dispositions reviewer: substrate re-baseline architecture review (contradiction + gap
  • staleness pass over ADR-019..025 and the re-baselined specs) scope_note: gate review for the re-baseline cycle — reviews 001/002 ran pre-substrate; this review re-runs their standard over the re-baselined corpus. Same bar: could a decomposition agent and an implementation agent proceed without inventing divergent answers?

Review 003 — Substrate Re-Baseline Architecture Review

Purpose

The substrate re-baseline (ADR-019..024, 2026-10-19) and the crate-shape round (ADR-025, 2026-10-20) rewrote the backend story after reviews 001/002 passed decomposition-readiness. This review re-runs the same contradiction/gap/staleness standard over the re-baselined six specs and the ADR package, before the specs flip draft → reviewed and the wave-based decomposition (docs/plans/implementation.md) begins.

Summary

  • Critical issues: 2 (both fixed 2026-10-20)
  • Warnings: 5 (4 fixed 2026-10-20; W-3 resolved by a pinned decision)
  • Suggestions: 5 (all fixed 2026-10-20)
  • Overall verdict at review time: needs revision — revision landed same day; all findings dispositioned below.
  • ADR-025 integration is thorough: no surviving single-crate feature-graph language, storage_root, or registry-file default in any decomposer-facing spec prose. The re-baseline staleness was concentrated in the pre-substrate ADR files themselves.

Critical issues (fixed)

C-1 — ADR-013's unamended pre-substrate ingestion + CAS composition

The receive-pack decomposer's first ADR read pinned, in normative voice, the exact composition the re-baseline killed (Bundle:: write_to_directory_eagerly with .keep-guarded pack landing, gix-fsck over the odb handle, gix-ref file-transaction CAS) while ADR-020's status already claimed those sections amended — the file itself carried no annotation. Fix: status annotation naming §3/§6 as amended (kept as the decision record of the retired disk-odb composition, with the surviving parts called out verbatim) and §7's CAS sentence rewritten to name the substrate ref-table row CAS as the operative home (ADR-019 §2) with the one-transaction-per-push and observed ordering standing.

C-2 — ADR-018's storage_root record-schema rationale

The freeze-inventory ADR's §2 still asserted the record carries a filesystem path the impls open — directly contradicting ADR-019 §8 and backend.md's schema section — and StorageError::Invalid's doc carried the storage-root example. Fix: status annotation on the whole rationale supersession; §2 rewritten to the resolve-once/repo_id-keyed rationale; Invalid's doc reworded; the References gix-ref line marked as the retired engine's realization.

Warnings

W-1 — ADR-017 §2's construction shape contradicted ADR-023 §2 (fixed)

Status annotation added (no new Transport trait impl — GitSession wraps gix-transport's git-daemon Connection over the BiStream compat halves) plus an in-§2 pointer. Workstream sizing for the consumer half now reads correctly from ADR-017 alone.

W-2 — Pin-custody ownership across the frozen trait seam (fixed by decision)

The pin bridge said "held by the ingest flow" across a seam (prepare returns PreparedPush; the wire layer then calls apply_updates) that carries no guards — no pinned answer for who holds them or how the prepare-returned-apply-never-arrives path cleans up. Decision pinned in substrate.md §object storage: a composition-internal pending-pin registry shared by the constructed-together GitPackIngest/GitRefs impls — content-addressed correlation (each apply drains the pending entry whose RefUpdate list matches; identical lists guard identical CAS requirements), guards commit only after the CAS transaction commits, stale entries pruned on the repo's next apply, and leaked guards recover the same GC-decreed way the crash case already does (process-lifetime custody; bounded cost, never partial repo state).

W-3 — v1 HEAD posture (resolved by decision)

Protocol-native v1 replication cannot propagate HEAD (the push grammar carries no symref; the diff is ref rows), git/repo/create seeds nothing, and the mirror plane's head verb is optimization-bucket — so replicas cloned checkout-less, silently. Decision pinned (in substrate.md §refs + ADR-023 §4's consequences): the substrate's put_repo seeds HEAD → refs/heads/main in the record's row-write transaction (git parity — git init --bare creates the dangling HEAD at init, not at first push; the record-write is the one moment the composition owns both surfaces); v1 pins the target, a constructor knob is additive later; the v1 replication limitation (a replica clones from its own seeded default until the mirror plane lands) is recorded rather than silent. Bonus: the seeded dangling symref is exactly the unborn-HEAD shape OQ-19's rider (a) needs to validate.

W-4 — The substrate freeze-inventory member list omitted the object side (fixed)

ADR-025 declares substrate.md §public API to be exactly the freeze inventory, but the list omitted the five trait implementations and the substrate Find front — constructible public types an assembler needs. Fix: the list extended (registry pair + engine constructors + in-memory adapter; cached provider + reactor; the five trait impls public-by-necessity including the Find/odb front; the receive-pack composition explicitly internal; GC liveness + LFS surface), mirrored in backend.md §public API.

W-5 — Residual disk-odb staleness in ADR-004/ADR-014 (fixed)

One-line status annotations on both (pipeline shape unchanged on the substrate baseline / common_haves resolves over the substrate Find); the retired-engine clauses are now marked as such where both ADRs are cited normatively.

Suggestions (all fixed)

  • S-1: "substrate types" naming collision (ADR-005 session substrates vs the alkgit-substrate crate) — transport.md (×2), backend.md §public API now read "session substrate types (ADR-005)".
  • S-2: the deferred(scope) taxonomy wording ("waiting on external information") conflicted with two correctly-shaped deferrals whose blockers are booked internal work items (OQ-19, OQ-21) — the definition now covers "a not-yet-existing internal artifact (a booked work item)" and both blocked-on clauses name their booking.
  • S-3: ADR-020 §1's "a takes the packless" typo → "a timeout takes the packless".
  • S-4: ADR-009 gains a status note for the Limits inventory's new bounded-peek field (ADR-020 §1).
  • S-5: OQ-20 lacked a tracker task — tasks/architecture/ oq-20-lfs-row-pruning.md added (demand-triggered; "do not act before this fires" posture recorded) and linked from the OQ entry.

Decision Quality (category F) — findings

  • OQ-19: open was the wrong state by the register's own letter (needs-resolution-now) for an explicitly implementation-phase validation — reclassified deferred(scope) blocked on the booked wave item; endorsed as non-circular (the validation outcome folds back into the same wave via ADR-003's honesty rule, not into the precondition of running it). Impacts field specific; door type and priority correct.
  • OQ-21: the two-layer deferral is honest — the build waits on a demand trigger (external information arriving), the validation runs against alkstore machinery without waiting on what it validates; no circularity or inversion. Only fix was the S-2 blocked-on wording.
  • The remaining resolved/rescoped OQs (02/04/06/08/09/17, OQ-03's partial) read cleanly — no hedges found; OQ-03's deferred-on-release-timing escape is correctly classified.

Strengths

  • ADR-025's two-crate partition is genuinely diffused across every spec's crate/feature/public-API/assembly sections.
  • The trait-seam discipline (ADR-018's zero-drift record) is used consistently; frozen seam vs. free composition story is honestly separated everywhere.
  • Claim-to-evidence fidelity: every measured datum cited in the specs checks out against the POC findings; no spec claim contradicts its evidence doc.
  • ADR-020's amendments are well-diffused and coherent across transport/backend/substrate.
  • OQ register hygiene: impacts fields specific everywhere; priorities match.

Disposition

All findings fixed/resolved 2026-10-20. The specs flip draft → reviewed; decomposition into implementation waves begins per docs/plans/implementation.md (wave-based, the alkblobs/ADR-025 rhythm — waves decompose one at a time, each ending in a review gate).