---
status: in-progress (H1, L1, H3, L5, L6, H2, M1, M2, M3, L2, L3, N2 resolved 2026-09-02)
last_updated: 2026-09-02
reviewed_artifacts:
  - src/read_plan.rs
  - src/sequential_reader.rs
  - src/materialize.rs
  - src/offset_map.rs
  - src/layout_builder.rs
  - src/engine.rs
  - src/validation_plan.rs
  - src/bast.rs
  - src/bast_meta.rs
  - src/bast_validation.rs
  - src/tunion.rs
  - src/data_access.rs
  - src/lib.rs
  - tests/{poc_roundtrip,tunion_dispatch,error_paths,engine_integration}.rs
  - docs/plans/030-compiled-forms.md
  - docs/architecture/decisions/011-compiled-read-plan-for-packed-mode.md
  - docs/architecture/decisions/012-plan-fingerprinting-and-m1-closure.md
tool: manual source read + 0.2.0-vs-0.3.0 parity diff (git show cab4932) + cargo-llvm-cov 0.8.4 (release profile) + disposable probe tests (run in-session, then deleted)
reviewer: 0.3.0 post-implementation review (triggered after release commit 9949f91)
---

# Review #006 — 0.3.0 Implementation Review: Compiled Forms

## Purpose

The 0.3.0 release (eight phases, commits `ff85258`..`9949f91`) landed
the compiled-form rollup: `ReadPlan` (ADR-011), owned `BastDoc`,
`OffsetMap` `LeafMeta`, `ValidationPlan` (ADR-012 §3), and plan
fingerprinting (ADR-012 §1/§4). This review is the post-implementation
check the other reviews were not: it audits the *shipped code* —
correctness, untrusted-input discipline (AGENTS.md §3/§4), 0.2.0
behavioral parity, code smells, and coverage — rather than a plan or a
design. It runs after the release commit, before `cargo publish`.

The intent is not to relitigate the ADRs or the plan; both held up
well. The intent is to find the classic implementation-phase problems
the phase gates could not see: cross-consumer convention disagreements,
adversarial input that survives every gate, dead-but-live code paths,
and the test-coverage holes that hide them.

## Methodology

- Full read of every module in `src/` touched by 0.3.0, plus
  `docs/plans/030-compiled-forms.md` and ADR-011/012.
- Parity diff against 0.2.0 (`git show cab4932:src/...` for
  `sequential_reader.rs`, `engine.rs`, `offset_map.rs`, `materialize.rs`,
  `bast_validation.rs`) to separate pre-existing flaws from 0.3.0
  regressions. Where a finding is parity-preserved, it says so.
- `cargo llvm-cov --release` (cargo-llvm-cov 0.8.4, summary + per-line
  text) to find weak spots; every uncovered region cited in the
  findings was read to decide "untested but fine", "untested and
  load-bearing", or "unreachable".
- Disposable probe tests (`tests/zzz_review_probe.rs`, deleted after
  the session) to confirm/deny the four behaviors code reading flagged:
  the ADR-006 record gap, the field-disc union builder/reader split,
  the non-first discriminator field, and standalone cyclic-`$ref`
  handling. Probe results are quoted verbatim in the findings.

**Operational warning for future sessions:** two probes were *not*
safe to run in the default harness. The huge-array-count repro
(H1) OOM-aborted the machine it ran on (a `Vec::with_capacity(1e12)`
allocation failure takes the process down with SIGABRT — and in this
session it took down the agent server running it). The cyclic-`$ref`
repro (H2) reliably SIGABRTs the test runner via stack overflow. Any
session reproducing H1/H2 must do so in an isolated process (separate
`cargo test --test <probe>` invocation on a throwaway machine, `ulimit -v`
wrapper, or a CI job with a memory cap) — never inside the main suite.
This is recorded here so the reproduction hazard does not have to be
rediscovered.

## Verification Baseline

Reviewed at `main` HEAD `9949f91` ("Release v0.3.0", `Cargo.toml`
0.3.0). All gates re-run in this session and green: 474 tests pass
(397 + 17 + 34 + 14 + 12), `cargo clippy --all-targets -- -D warnings`
clean, `cargo doc --no-deps` zero warnings, `cargo build --target
wasm32-unknown-unknown --release` green, `cargo publish --dry-run
--allow-dirty` clean. The release summary's claims were cross-checked:
the four commit hashes exist with the described content; the bench
numbers are recorded in the plan and ADR-007; ADR-011/012 status
blocks, review #004/#005 status lines, and the ADR table in the
architecture README are flipped as claimed; the `lib.rs` re-export list
matches the plan's Semver Contract table row for row (net breaking =
`Bast*` lifetime removal, `OffsetMap::get` return type,
`SequentialReader::new`, `materialize_*` signatures; net additive =
`ReadPlan` + sub-types, `LeafMeta`/`OffsetEntry`, `ValidationPlan` +
sub-types, `fingerprint()` methods; `Hash` on `Endian`/
`VariableEncoding` present per phase 5's prerequisite step).

## Summary Statistics

| Severity | Count | Status |
|----------|------:|--------|
| High | 3 (H1, H2, H3) | all resolved 2026-09-02 |
| Medium | 4 (M1, M2, M3, M4) | M1, M2, M3 resolved 2026-09-02; M4 ongoing |
| Low | 6 (L1–L6) | L1, L2, L3, L5, L6 resolved 2026-09-02 |
| Nit | 2 (N1, N2) | N2 resolved 2026-09-02; N1 open |

All three Highs, three of four Mediums, five of six Lows, and one of
two Nits are resolved. Remaining: M4 (ongoing per-fix coverage
posture), L4, N1.

The three Highs are adversarial-input crashes (H1, H2) and a
cross-consumer wire-layout convention gap (H3) — all three are the
exact class of problem AGENTS.md §3 exists for, given that `alkcall`
feeds schemas from arbitrary internet peers into this engine. H1
blocked publishing 0.3.0 to crates.io until fixed (now fixed, see the
resolution note on the finding). None of the three is a 0.2.0
regression in the strict sense (details per finding), but two of
them became materially easier to hit with the 0.3.0 surface.

**Resolution log:**

- **H1 + L1 (2026-09-02):** resolved in one commit — see the
  "Resolution (2026-09-02)" block at the end of the H1 finding and the
  L1 finding. 482 tests green (405 crate + 67 integration + 2
  pre-existing ignored), clippy `-D warnings` clean, wasm build green.
  The fix session also surfaced a new Nit (N2, unbounded `align`
  annotations) — added below.
- **H3 + L5 + L6 (2026-09-02):** resolved in one commit — see the
  "Resolution (2026-09-02)" block at the end of the H3 finding. Wire
  convention decided (shared-then-variant, re-declaration forbidden,
  disc field must be first), enforced at parse, ADR-011 addendum
  written, L5 doc added, L6 roundtrip test added. 488 tests green,
  clippy `-D warnings` clean, wasm build green, `cargo doc --no-deps`
  zero warnings.
- **H2 (2026-09-02):** resolved — see the "Resolution (2026-09-02)"
  block at the end of the H2 finding. Shared one-shot reference-graph
  guard (`walk_guard::check_ref_graph`) added and run at the entry of
  all three standalone walkers; full cyclic/deep-nesting/diamond test
  family added. 501 tests green, clippy `-D warnings` clean, wasm build
  green, `cargo doc --no-deps` zero warnings.
- **M1 + M2 + M3 (2026-09-02):** resolved in one commit (same file /
  adjacent surfaces, per the recommended order) — see the resolution
  blocks on each finding. 508 tests green, clippy `-D warnings` clean,
  wasm build green, `cargo doc --no-deps` zero warnings.
- **L2 + L3 (2026-09-02):** resolved in one commit (both touch the
  0.3.0 compiled-form files already in play) — see the resolution
  blocks on each finding. 508 tests green, clippy `-D warnings` clean,
  wasm build green, `cargo doc --no-deps` zero warnings.
- **N2 (2026-09-02):** resolved — see the "Resolution (2026-09-02)"
  block on the finding. `MAX_ALIGN = 4096` enforced in `parse_align`
  (clean `Schema` error, standalone-safe) and in the meta-schema
  (`"maximum": 4096` on both align properties); H1's byte-cap test
  retuned to the new legal maximum. 511 tests green, clippy
  `-D warnings` clean, wasm build green, `cargo doc --no-deps` zero
  warnings.

---

## Findings

### H1. A huge declared array `count` OOM-aborts the process (DoS on untrusted schemas)

**Files**: `src/read_plan.rs:533-553`, `src/materialize.rs:248`,
`src/materialize.rs:637`, `src/materialize.rs:924`,
`src/bast_meta.rs:113`

**Problem**: The BAST meta-schema allows any array `count >= 0`
(`bast_meta.rs:113`, `"minimum": 0`, no upper bound), and nothing
downstream bounds the product `count × element size`:

- `ReadPlan::compile_array` (`read_plan.rs:533`) accepts any `count`
  verbatim. `compile` validates depth and cycles but never total size.
- `materialize_plan_array` (`materialize.rs:248`) — the packed
  `validate_bytes` path — does `Vec::with_capacity(count)` before
  reading a single byte. With `count: 1000000000000` (1e12) and u8
  elements, that is a 1 TB capacity request: the allocator fails and
  the process aborts (SIGABRT), not an `Err` return.
- The same `Vec::with_capacity(count)` pattern is on the aligned path
  twice: `materialize_array_packed` (`materialize.rs:637`, reached via
  aligned record values) and `materialize_array_aligned`
  (`materialize.rs:924`).

The sequential reader gets away with it only incidentally:
`plan_read_array` (`sequential_reader.rs:726-733`) computes
`count.checked_mul(element_stride)` and errors cleanly when the stride
is nonzero — but a fixed-stride array's *materialize* (the
`validate_bytes` half) still hits the `with_capacity`, so
`validate_bytes` on an accepted schema aborts regardless of mode. For
stride-0 (variable-element) arrays, `plan_walk_variable_array_size`
(:749-771) errors per element on a short buffer — safe, but after
walking up to `count` iterations of per-element error returns.

Probe (run in-session; OOM-aborted before printing): an engine compiled
from `{"kind": "array", "element": "uint8", "count": 1000000000000}`
accepted `AlkTypeEngine::compile` in both modes; the
`validate_bytes(&[0,0,0,0])` call did not return — the process died in
the allocator. Note the meta-schema itself is *not* at fault (count is
a legitimate schema knob); the engine's failure to bound its own
allocation is.

**This violates AGENTS.md §3** ("a malicious or unsupported schema
definition must produce a handleable error, not a crash") on the
engine's flagship untrusted-input path (`validate_bytes` on an incoming
stream). **Fix before publishing 0.3.0.**

**Fix**: two layers, both cheap:

1. Replace all three `Vec::with_capacity(count)` with a plain
   `Vec::new()` + push loop (the loops already push `count` elements;
   the pre-allocation is pure downside for adversarial input and no
   measurable win for honest input at the sizes that matter — the
   materializer's per-element work dominates).
2. Optionally, bound total computed array bytes at compile time
   (`count.checked_mul(elem_size)` against a documented cap, returning
   `AlkTypeError::Schema("array size exceeds compile-time limit")`), so
   adversarial schemas die at compile with a clean error instead of
   per-buffer. The reader's `checked_mul` in `plan_read_array`
   (`sequential_reader.rs:729`) already refuses oversized fixed-stride
   arrays at read time; the materializer should not be weaker than the
   reader.

Add tests: compile+validate with `count: usize::MAX` (expect clean
`Err`), and a moderate count against a short buffer (expect clean
`Err`).

**Resolution (2026-09-02) — both layers, plus two gaps the original fix
text missed:**

1. **Abort removal (fix layer 1):** all three `Vec::with_capacity(count)`
   sites are now `Vec::new()` + push loop (`materialize_plan_array`,
   `materialize_array_packed`, `materialize_array_aligned`). The
   `validate_bytes` OOM-abort is gone: with an oversized count the
   per-element walk now errors on buffer bounds instead of allocating.

2. **Compile-time caps (fix layer 2), at two choke points** — the
   review's "documented cap" suggestion landed as two constants in
   `schema.rs`:
   - `MAX_ARRAY_ELEMENTS = 2^16`, enforced in `BastArray::parse`
     (`bast.rs`) — the single point every consumer inherits
     (`BastDoc::new` is the only construction path, and all five
     consumers — engine, ReadPlan, LayoutBuilder, OffsetMap, and
     standalone `BastDoc` users — parse through it). This cap is what
     bounds the walkers' *per-element push loops*: with a product cap
     alone, 256 MiB of u8 elements would still mean 268M loop
     iterations accumulating ~20 GB of offset-map/layout entries at
     compile time. The original fix text ("bound `count × element
     size`") missed this; a product cap bounds wire size, not walker
     memory.
   - `MAX_ARRAY_BYTES = 2^26`, enforced per walker against the
     mode-appropriate stride: `compile_array` (ReadPlan, schema error),
     `walk_array` (LayoutBuilder, offset error — defense-in-depth
     there: packed mode has no alignment, so max fixed element 8 ×
     count 2^16 = 2^19 << 2^26 can't reach the cap), `compute_array_field`
     (OffsetMap, offset error — *reachable* there, via an
     align-driven stride, see below).
   - L1 folded in: `fixed_composite_size`/`fixed_plan_size` now return
     `Result<Option<usize>>`; `unwrap_or_default()` is gone, and the
     checked-mul overflow arm is a clean `Schema` error instead of a
     silent stride-0.

3. **Zero-progress runtime guards** — a gap neither the finding nor the
   fix text covered: a stride-0 array whose elements consume 0 bytes
   (empty-struct elements are legal; `stride 0` means per-element
   sequential walking) loops `count` times with no buffer bound even
   after both caps. `plan_walk_variable_array_size` (reader) and
   `materialize_array_packed` (materializer) now error with "array
   element consumed 0 bytes" when an element makes no wire progress.
   The aligned materializer needs no guard: aligned array elements are
   always fixed-size kinds (`type_size() >= 1`).

Tests (8 new + 1 rewritten): parse-cap rejection (bast), byte-cap
rejection via ReadPlan/LayoutBuilder/OffsetMap, count-at-cap
acceptance, packed 2^16-array build acceptance (layout_builder,
documents why the byte cap is unreachable in packed mode), huge-count
short-buffer clean access error, and the old
`array_count_large_u64_parses_on_64bit` (which asserted the *old*
unbounded-parse behavior as a feature) rewritten as
`array_count_large_u64_rejected_by_compile_cap`. No OOM reproducers
in-tree (per the Methodology warning); the in-tree tests assert only
the compile-time rejections — the safe half.

Verified: 482 tests green, clippy `-D warnings` clean,
`cargo build --target wasm32-unknown-unknown --release` green.

### H2. Cyclic `$ref` stack-overflows `OffsetMap::compute` / `LayoutBuilder::new` / `materialize_aligned` when driven standalone

**Files**: `src/offset_map.rs:123` (`compute`),
`src/layout_builder.rs:150` (`new`), `src/materialize.rs:462`
(`materialize_aligned`), contrast `src/engine.rs:146-152`,
`src/read_plan.rs:54,283`, `src/validation_plan.rs:81`

**Problem**: The engine is safe: `AlkTypeEngine::compile` runs
`ValidationPlan::compile` *before* the layout builders, and the plan's
compile walk rejects cyclic `$ref` graphs with a clean `Schema` error
(engine.rs:146-152 documents this gate explicitly — good defensive
design). `ReadPlan::compile` and `ValidationPlan::compile` each carry
their own depth cap (128) + definition-level cycle set
(`read_plan.rs:54,283`, `validation_plan.rs:81`), so both are
untrusted-input-safe standalone.

But the *other three public schema walkers* have neither guard, and
`BastDoc::new` (`bast.rs:74`) parses the root eagerly while resolving
`$ref`s lazily (`bast.rs:110-130`) — so a cyclic document parses
successfully and the walkers recurse unboundedly:

- `OffsetMap::compute` → `ComputeCtx::compute_struct` →
  `resolve_typeref` → nested `compute_struct` → …
- `LayoutBuilder::new` (parse succeeds) → `build` → `walk_struct` →
  same shape.
- `materialize_aligned` → `materialize_struct_aligned` → same shape.

Probe result (verbatim):

```
PROBE6 … process didn't exit successfully: … (signal: 6, SIGABRT:
process abort signal)
```

i.e. standalone `OffsetMap::compute` on
`S = { next: { "$ref": "#/$defs/S" } }` stack-overflows. All three
types are public re-exports (`lib.rs:66,72,73`), so a consumer using
them without the engine — exactly the "consumer holds a `BastDoc`"
use case `bast_validation::validate_value` was retained for — hits an
abort, violating AGENTS.md §3 for the public API surface, not just the
engine.

This is **parity-preserved** (0.2.0 had the same gap; it predates
0.3.0), but 0.3.0 is the release that made the *pattern* explicit:
`ReadPlan`/`ValidationPlan` got guards and the plan text says the
engine's ValidationPlan gate "serves as the engine's cyclic-`$ref`
gate (the layout walkers have no cycle guard)". The residual exposure
— public walkers without guards — was left silent. AGENTS.md §3 makes
no engine-vs-standalone distinction.

**Fix**: extract the depth-cap + cycle-set guard into a shared helper
(the two existing implementations in `read_plan.rs` and
`validation_plan.rs` are already near-identical — `depth_err`/
`cycle_err`/`seen` threading) and thread it through `ComputeCtx`,
`BuildCtx`, and the aligned materializer's struct/array/record walk.
Alternatively (smaller): document on `OffsetMap::compute`,
`LayoutBuilder::new`, and `materialize_aligned` that cyclic input is
rejected *only* via `AlkTypeEngine::compile`'s gate and that standalone
calls require pre-validated documents — but that re-introduces the
"trust boundary depends on call path" trap the plan's phase-1 notes
explicitly retired for `ReadPlan`. The shared guard is the right fix;
the doc note is the minimum acceptable one.

**Resolution (2026-09-02) — shared one-shot graph guard, all three
walkers gated:**

1. **New module `src/walk_guard.rs`** (private, `pub(crate)`) with
   `check_ref_graph(&BastDoc)` — a single bounded walk over the
   reachable reference graph that rejects depth > 128
   (`MAX_GRAPH_DEPTH`, matching the plan compilers'
   `MAX_COMPILE_DEPTH`) and any `$ref` cycle, using the same
   path-scoped cycle-set semantics the plan compilers use (diamond
   refs compile; only genuine cycles trip). The walk covers every
   carrier shape: inline structs recurse into fields, named defs are
   entered via `resolve_ref`, union mappings and shared `fields` are
   both checked, and arrays/records are seen through to their
   element/value types (so a cycle behind an array-of-`$ref` hop is
   caught — the probe-verified gap shape). Error text mirrors the plan
   compilers' wording ("cyclic $ref through…", "compile depth
   exceeded…") so downstream matching sees one shape.

2. **All three standalone walkers run the guard at entry**, before any
   recursion: `OffsetMap::compute`, `LayoutBuilder::new`, and
   `materialize_aligned` (the last as defense-in-depth — a cyclic doc
   can no longer produce an `OffsetMap`, but a mismatched
   doc/map pairing must still fail with a clean `Schema` error, not
   overflow). `AlkTypeEngine::compile`'s ValidationPlan gate is
   unchanged and now redundant-but-harmless; its doc comment is
   updated to say so. The full shared-guard recommendation was taken
   (not the minimum doc note).

3. **Behavioral side effect, net-positive:** `check_ref_graph`
   resolves every reachable def eagerly (via `resolve_ref`), which
   parses each def's full shape — so an invalid *non-root* def (e.g. a
   `$defs` union that re-declares a shared field, or whose discriminator
   field is missing/non-first) now surfaces at `LayoutBuilder::new`
   instead of at `build()`. Four pre-existing H3 tests asserted the old
   lazy-parse timing ("root parses, fails at build"); they were updated
   to expect the same `Schema` error at `new()`. Earlier rejection of
   the same malformed schemas — strictly better for untrusted input,
   no accepted schema's behavior changed.

4. **Test family added (12 tests):** in `walk_guard.rs` (self-cycle,
   two-def cycle, diamond allowed, 201-def deep chain → clean depth
   error), in `offset_map.rs` (cycle rejected at compute, two-def
   cycle, cycle behind inline-struct + array hop, diamond still
   computes), in `layout_builder.rs` (cycle rejected at `new`, two-def
   cycle, diamond still builds), and in `materialize.rs` (cyclic doc
   + unrelated map → guard fires before offset lookup, diamond still
   materializes). No stack-overflow reproducers in the default suite —
   the tests assert the clean-error half only (the overflow itself was
   probe-verified SIGABRT in the review session; see the Methodology
   warning about running it).

Verified: 501 tests green (423 crate + 17 + 34 + 15 + 12 + 2
pre-existing ignored), `cargo clippy --all-targets -- -D warnings`
clean, `cargo build --target wasm32-unknown-unknown --release` green,
`cargo doc --no-deps` zero warnings.

### H3. Field-name-discriminator unions: builder, reader, and materializer disagree on layout and on discriminator position

**Files**: `src/layout_builder.rs:485-527` (write side),
`src/sequential_reader.rs:551-598,607-646` (read side),
`src/materialize.rs:356-404` (materialize side), builder doc
`src/layout_builder.rs:35-40`

**Problem**: Three packed-mode consumers each implement the field-disc
union slightly differently, and their agreements/disagreements are
undocumented and untested end-to-end:

1. **Layout convention (builder vs reader/materializer).** The builder
   walks *only the selected variant struct* starting at the union's
   offset (`walk_field_discriminator_union`, layout_builder.rs:485-527
   — `self.walk_struct(variant_struct, field_path, offset)`). The
   reader and materializer walk the union's declared `fields` (the
   `shared` sub-plan: discriminator field + any shared fields) *first*,
   then the variant (`sequential_reader.rs:573-588`,
   `materialize.rs:371-381`). The two conventions never agree on the
   wire: reader span = `shared_size + variant_size`, builder span =
   `variant_size`. On the builder tests' own fixture (union
   `fields: [type: uint8]`, variant `Read { type, handle }` — the
   variant *re-declares* the discriminator), the builder produces
   `type@0, handle@1, total 5` (probe-verified:
   `PROBE1 builder: total=5 event.type=Some((0, 1)) event.handle=Some((1, 4))`)
   while the reader's walk places the variant's `type` at 1 and
   `handle` at 2 (span 6) — the discriminator field would appear twice
   on the wire under the reader's convention. Conversely, if the
   variant does *not* re-declare the disc field, the reader's
   convention is coherent (disc from `shared`, variant fields follow)
   and the *builder's* is the one that drops the disc's wire position.
   `LayoutBuilder`'s tests (:1221-1334) and the reader/materializer
   tests (`sequential_reader.rs:1260-1305`,
   `materialize.rs:1183-1258`) each encode their own convention with
   different fixture shapes, so nothing cross-checks them. No doc
   comment on either side states the constraint; no roundtrip test
   drives a field-disc union from `LayoutBuilder` output through
   `SequentialReader`/`materialize_packed`.

2. **Discriminator position (reader vs materializer).** The reader's
   `plan_read_union` Field arm reads the discriminator value at the
   union's *start* offset
   (`plan_discriminator_string_value(disc_field, buffer, offset, …)`,
   :563-564 — `offset` is the union start, not the disc field's
   position). The materializer's Field arm walks all shared fields in
   order and captures the disc value at its *real* position
   (`materialize.rs:371-381`). For a non-first discriminator field the
   two consumers read different bytes. Probe-verified (verbatim):

   ```
   PROBE4 reader error: access error at event: unknown union
          discriminator value: 1     ← read seq's first byte at union start
   PROBE4 validate_bytes ok          ← materializer dispatched correctly on
                                       the same buffer
   ```

   the reader stringified `1` (seq's first byte at the union start,
   offset 0) and failed the mapping lookup; `validate_bytes` on the
   identical buffer passed. Note this probe ran after probe 1's reader
   section had already panicked on an assertion; the error itself is
   the evidence — the reader dispatched on the wrong field's bytes
   while the materializer dispatched correctly on the same buffer.

3. **0.2.0 parity, precisely scoped.** The reader-side flaw (disc read
   at union start) is parity-preserved: 0.2.0's `read_union_value`
   Field arm also read only the disc field at the union start
   (`cab4932:src/sequential_reader.rs:460-472`), and 0.2.0's
   materializer did the same — so in 0.2.0 reader and materializer
   *agreed* (both position-blind). 0.3.0's materializer became
   position-correct (it must, to walk shared fields in order), which
   silently made the two packed read-side consumers disagree for
   non-first disc fields. The builder's variant-only layout is also
   parity-preserved. So H3 is not a regression — it is a
   long-standing convention gap that 0.3.0's `shared`-plan shape made
   visible and partially divergent.

**Fix**: make the decision explicit and enforce it:

- Decide the wire convention. The reader/materializer shape (shared
  fields, then variant) is the one the materializer already implements
  position-correctly, and it matches ADR-011's `shared` plan design —
  recommend: builder lays out `shared` then the variant, and the
  "variant re-declares the discriminator field" pattern becomes either
  required (validated at compile) or forbidden (rejected at compile).
  The other consistent option — variant-only layout, no shared walk —
  would mean reverting the reader/materializer to 0.2.0's
  position-blind disc read, which the `shared` plan shape was
  specifically built to move past.
- Enforce at compile/parse time whatever constraint the convention
  needs (e.g. discriminator field must be first in `fields`; variant
  must not re-declare shared fields — or must, if that's the
  convention). `BastUnion::parse` (`bast.rs:494-547`) or
  `compile_union` is the natural enforcement point; today nothing
  constrains field order in a field-disc union.
- Record the decision in ADR-011's union section (a short addendum) or
  ADR-003 §4, and add the write→read→validate roundtrip test through a
  field-disc union (packed mode) — see L6.

**Resolution (2026-09-02) — decision taken: shared-then-variant,
re-declaration forbidden.** Both recommendations accepted as
recommended:

1. **Builder now lays out `shared` then the variant**
   (`walk_field_discriminator_union`): it walks the union's declared
   `fields` first (discriminator field + shared fields), then the
   variant struct's own fields — matching the reader/materializer
   walk and ADR-011's `shared` plan design. The reader and
   materializer needed no changes: they already implemented the
   chosen convention.

2. **Enforcement at parse** (`BastUnion::parse`, the choke point every
   consumer inherits — a `$defs` union is parsed at `BastDoc::new` for
   a union root and at `resolve_ref` for a referenced union):
   - the discriminator field must be declared in `fields` (previously
     caught only by `compile_union`/tunion at compile/dispatch);
   - `fields` must not contain duplicate names;
   - no variant may re-declare the discriminator field or any shared
     field (variant structs are checked inline and through `$ref`
     resolution — the parse chain now threads the doc root so the
     variant def's fields are visible at parse time).
   Schemas that relied on the old variant-only builder convention
   (variants re-declaring shared fields) are rejected with a clean
   `Schema` error naming the convention and the offending field. This
   is a breaking constraint for 0.2.0-era re-declaring schemas,
   announced with the 0.3.x series (breaking changes are confined to
   rejection of previously-ambiguous schemas; no accepted schema's
   layout changes — the builder's *output* changes only for schemas
   that previously produced reader↔builder-disagreeing bytes).

3. **Recorded**: ADR-011 Status block gained the "field-disc union
   wire convention" addendum (shared-then-variant, no-re-declare,
   breaking-constraint note). `FieldValue::Union.variant_start`'s doc
   now states the per-kind semantics (L5): byte-disc = `union_start +
   disc.offset + disc.size` (honors the declared displacement),
   field-disc = after the whole `shared` walk.

4. **L6 roundtrip test added**
   (`tests/poc_roundtrip.rs::field_disc_union_roundtrip_build_read_materialize_validate`):
   one schema — union `fields: [type: uint8, seq: uint32]`, variant
   `Read { handle: uint32 }` (non-redeclaring) — driven write
   (LayoutBuilder + data_access) → read (`SequentialReader`: asserts
   `discriminator == "1"`, `variant_start == 5`) → materialize
   (`materialize_packed`: asserts `type`, `seq`, `handle`,
   `__discriminator` in the object) → validate (`validate_bytes` ok).
   The builder-side assertions pin `event.type@0, event.seq@1,
   event.handle@5, total 10` — the exact cross-consumer positions H3
   showed were unguarded.

5. **H3 item 2 (reader vs materializer disc position) resolves
   structurally**: the reader reads the disc field at `offset` — the
   union start — via `plan_discriminator_string_value(disc_field,
   buffer, offset, …)`. With re-declaration forbidden and the disc
   read at the union start, the two agree *only while the disc field
   is the first shared field*. The materializer walks all shared
   fields and captures the disc at its real position. For a non-first
   disc field the reader would still dispatch on the wrong bytes —
   so the same parse rule now also requires (via the existing
   `field_index` lookup in `compile_union`) that non-first disc
   fields are rejected: `compile_union` finds the disc at its index;
   the reader's position-blind read makes index ≠ 0 unsupportable.
   Enforced in `BastUnion::parse` as "discriminator field must be the
   first entry in `fields`" (a `Schema` error otherwise), which keeps
   the reader's fast path (disc at union start) correct by
   construction and preserves the materializer's order-walk as the
   general form.

Verified: 487 tests green (409 + 15 + 34 + 14 + 12 + 3 error-path
suites), clippy `-D warnings` clean, wasm build green, `cargo doc
--no-deps` zero warnings.

### M1. The ADR-006 check misses non-final inline `Record` fields in aligned mode

**Files**: `src/offset_map.rs:235-255` (the check),
`src/offset_map.rs:515-520` (`field_variable_kind`)

**Problem**: `compute_struct`'s non-final inline length-prefixed
variable-field rejection (ADR-006) dispatches on
`field_variable_kind(field)`, which matches only *primitive*
variable-length kinds (`BastType::Primitive(k) if
k.is_variable_length()`). A `Record` field — whose inline
length-prefixed form has the same 4-byte-prefix-then-variable-data
shape and the same clobbering hazard the ADR-006 error text describes —
slips past the check. Probe-verified (verbatim):

```
PROBE2 ADR-006 MISSED for record: counts=Some((0, 4)) id=Some((4, 8)) total=8
```

`{ counts: record<uint32>, id: uint32 }` computes `counts` as a 4-byte
prefix at 0 and `id` at 4 — the record's variable data lands exactly
where `id` reads from. `validate_bytes` then succeeds with silently
corrupt data; `read_field("counts")` fails only incidentally (the
composite rejection, M2). **Parity-preserved** — the identical code
shipped in 0.2.0 (`cab4932:src/offset_map.rs:177,433-438`) — but it is
a hole in the exact check ADR-006 created, and it is the kind of thing
the check exists to catch.

**Fix**: include `BastType::Record(_) => Some(AlkTypeKind::Record)` in
`field_variable_kind` (offset_map.rs:515-520). The ADR-006 error
message's fix list ("use `maxLength` … `offset-indirect` … move to last
position") already covers records correctly. Add the
`non_final_record_rejected_in_aligned_mode` test mirroring
`non_final_inline_string_rejected_in_aligned_mode`.

**Resolution (2026-09-02):** exactly the recommended fix —
`field_variable_kind` now matches `BastType::Record(_) =>
Some(AlkTypeKind::Record)` (with a doc comment noting the
prefix-then-variable shape and the M1 hole it closes). Tests added
mirroring the string family: `non_final_record_rejected_in_aligned_mode`
(Offset error, path `counts`, message contains ADR-006 + "record") and
`final_inline_record_allowed_in_aligned_mode` (record-as-last-field
computes `counts` prefix @ 4, total 8 — the M2 position). Done with M2
in the same commit.

### M2. Aligned-mode record fields are untested end-to-end and sit right next to the M1 hole

**Files**: `src/materialize.rs:880-893` (aligned record dispatch),
`src/engine.rs:453-460` (`read_field` composite rejection)

**Problem**: `materialize_struct_aligned` dispatches `Record` through
`materialize_typeref_packed` at the entry's prefix offset — the
count-prefixed walk, which works. But nothing else in the aligned stack
coheres around records: `OffsetMap` records the entry as
`encoding: LengthPrefixed` with the 4-byte prefix range;
`engine::read_field` refuses `Record` as a composite ("use the
layout-specific APIs" — there is no aligned record API); and only the
M1 hole makes the "record as last field" position the safe one, with no
error distinguishing it from the clobbering position. Probe 5 showed
the happy path validates (`PROBE5 map: counts=Some((0, 4)) total=4`,
`PROBE5 validate ok`), so the *mechanism* works — but zero tests
exercise an aligned record field through any public path (coverage:
`materialize_struct_aligned`'s Record arm and `materialize_leaf_at`'s
record dispatch are hit only by in-module unit tests, never through
`validate_bytes`/`read_field`). Whatever M1's fix decides about
records, this path needs a locking test: record-as-last-field
round-trips, record-mid-struct is rejected (post-M1), and
`total_size` accounting for the prefix is asserted.

**Resolution (2026-09-02):** the requested locking tests, post-M1:

- `materialize.rs::materialize_aligned_record_last_field_roundtrips` —
  `record<uint16>` as last field driven end-to-end through
  `OffsetMap::compute` + `materialize_aligned`: both entries
  materialize (`{ "b": 10, "a": 20 }`), `id` intact at its fixed
  offset, wire arithmetic asserted in the test (22 bytes: 4 id + 4
  record count + 2 × (4 key-len + 1 key + 2 value)).
- `engine.rs::validate_bytes_aligned_record_last_field_round_trips` —
  the same shape through the full public `validate_bytes` path
  (`record<uint32>`, 26 bytes), plus a corrupted-buffer arm asserting
  garbage in entry bytes is rejected (not silently accepted).
- Post-M1 rejection covered by `non_final_record_rejected_in_aligned_mode`
  (see M1's resolution); `total_size` prefix accounting asserted by
  `final_inline_record_allowed_in_aligned_mode`.
Done with M1 + M3 in the same commit.

### M3. `read_field`'s aligned `Struct` arm is unreachable dead code

**File**: `src/engine.rs:449-452`

**Problem**: `read_field`'s kind dispatch has a
`AlkTypeKind::Struct => Ok(FieldValue::Struct { start, end })` arm,
but no struct entry ever exists in an `OffsetMap`: `compute_struct_field`
(offset_map.rs:346-387) pushes only the nested struct's *inner leaf*
entries, never an entry for the struct path itself, and the root is not
an entry either. `read_field("header")` on a nested-struct schema
returns the `Offset` "field not found in offset map" error — the
`Struct` arm cannot be reached through any schema the engine accepts.
Coverage confirms it: the arm shows 0 executions, and the composite
rejection arm right below it (:453-460) is likewise 0-execution
(because composites also never get entries — the reachable failure for
composite paths is the `Offset` miss, which the tests accept in
either variant). Two options: delete the `Struct` arm (and tighten the
doc comment, which currently implies composites error at the dispatch
stage rather than at lookup), or keep it and make struct entries exist
(deliberate feature — then document and test it). Deleting is the
smaller, honest change for 0.3.0's design.

**Resolution (2026-09-02):** deletion taken. The `Struct` arm now
returns a documented defensive `Offset` error ("read_field does not
support composite types (no offset-map entry exists for a struct path —
only its leaf fields are recorded)") instead of a fabricated
`FieldValue::Struct` — the reachable failure for a struct path remains
the offset-map lookup miss, as the finding established. The doc comment
now states that struct paths have no `OffsetMap` entry (read the leaf
fields by dotted paths) and that the Offset miss is the reachable
failure for any composite path. Locking test
`read_field_on_nested_struct_path_is_offset_error_not_struct_value`:
`read_field("header")` is an `Offset` error while
`read_field("header.magic")` succeeds. `FieldValue::Struct` itself is
untouched (public API; still constructed by the packed reader).

### M4. Coverage weak spots map onto the findings above — the aligned materializer half is the least-tested code in the engine

**Tool**: `cargo llvm-cov --release` (0.8.4). Overall: **89.59% lines,
84.80% functions**. Per-file (worst first):

| file | line% | fn% |
|---|---:|---:|
| `materialize.rs` | 64.48 | 59.30 |
| `data_access.rs` | 79.74 | 74.65 |
| `sequential_reader.rs` | 81.42 | 70.77 |
| `bast.rs` | 87.00 | 88.06 |
| `offset_map.rs` | 91.13 | 82.46 |
| `layout_builder.rs` | 91.18 | 81.54 |
| `builder.rs` | 91.28 | 87.50 |
| `tunion.rs` | 92.73 | 91.67 |
| `validation_plan.rs` | 93.97 | 83.87 |
| `read_plan.rs` | 94.53 | 95.89 |
| `engine.rs` | 96.57 | 100.00 |

The specific holes worth closing (each was read; all are
load-bearing-or-error-path, none is trivially dead except where noted):

1. **`materialize.rs` aligned half** — `materialize_struct_aligned`,
   `materialize_array_aligned`, `materialize_variable_aligned`, and
   `materialize_typeref_packed` carry ~47 uncovered lines between
   them. Most striking: the nested-struct recursion
   (`materialize_struct_aligned(doc, s, &path, …)`, :866) has **0
   executions** — the function is only ever entered at the root
   (10 calls, all `path_prefix: ""`). Aligned nested-struct
   materialization — the heart of aligned `validate_bytes` — has never
   been exercised through any test. The maxLength-trim arm
   (:971-998), the offset-indirect arms (:956-969), the UTF-8 error
   (:984-987), and both "resolved kind X but type was Y" internal
   errors (:868-878) are likewise 0-execution. The engine-facing
   aligned `validate_bytes` tests cover exactly one flat 2-field
   struct. This is where H1's aligned variant, M1, and M2 all live.
2. **`sequential_reader.rs`** — the public `schema()` (:235-237) and
   `plan()` (:240-242) accessors are never called by any test (public
   API, phase-2 deliverables). The field-disc discriminator-kind arms
   for uint16/uint32/enum (:621-641) and `plan_read_byte_discriminator`'s
   uint16/uint32 arms (:663-668) are 0-execution — wire-visible
   dispatch paths. `plan_walk_variant_size`'s union arm (:688-691) —
   the nested-union variant size walk, the capability phase 1
   specifically restored — is 0-execution (the compile side is tested;
   the read-side size walk is not).
3. **`data_access.rs`** — nearly all uncovered lines are error paths
   (offset overflow, mutable-slice-missing, and notably the
   `write_bytes` u32-truncation guard :266-270 and
   `write_bytes_indirect`'s two u32 guards :383-391 — review #002 M2's
   "canonical overflow-guard pattern" is itself untested). One test
   each would lock the canonical pattern.
4. **`offset_map.rs`** — `field_endian_for_element`'s
   `BastType::Struct(s) => s.endian()` / `Union(u) => u.endian()`
   arms (:529-530) are 0-execution. The union arm is unreachable
   (unions rejected in aligned mode); the struct arm says an array of
   inline struct elements inherits the *element struct's own* endian
   annotation — which contradicts the phase-5 parity rule everywhere
   else in 0.3.0 ("the referring field's effective endian propagates;
   the nested container's own annotation is not consulted"). Either
   this is a genuine divergence in array-element endian propagation
   (then it needs a parity test and probably a fix), or it's
   unreachable for inline elements too (then it's dead). Worth a
   focused look — it touches the exact parity story phase 5 closed.

Recommended posture (matches how this review was run): fold a coverage
check into each fix session — after fixing a finding, extend
`cargo llvm-cov` over the touched file and close the immediately
adjacent uncovered branches while the context is fresh, rather than
scheduling a standalone coverage sweep.

### L1. `unwrap_or_default()` conflates "variable-length" with "checked-mul overflow" for array strides

**File**: `src/read_plan.rs:547`

**Status**: resolved 2026-09-02 (with H1).

**Problem**: `let element_stride = fixed_composite_size(&element)
.unwrap_or_default();` — `fixed_composite_size` returns `None` both
for genuine variable-length elements (correct → stride 0) and for
`checked_mul` overflow of an absurd `count × stride` (→ silently
stride 0, i.e. "walk every element sequentially" — the reader then
does up to `count` per-element error returns instead of one clean
arithmetic error). With H1 fixed at compile time this becomes
unreachable, but as written the two cases are indistinguishable at the
call site. Prefer making `compile_array` return `Err` on the overflow
arm (a small refactor of `fixed_composite_size` to return a
`Result<Option<usize>, …>` or to take the cap from H1's fix).

**Resolution (2026-09-02):** `fixed_composite_size` and
`fixed_plan_size` now return `Result<Option<usize>, AlkTypeError>`;
`None` means variable-length only, and overflow arms return clean
`Schema` errors. `compile_array` uses `fixed_composite_size(&element)?
.unwrap_or(0)` — the conflation is gone. See the H1 resolution block
for the full change description.

### L2. `ReadPlan` construction carries a temporary `Value::Null` schema placeholder

**Files**: `src/read_plan.rs:295,460-461,601` (three
`schema: Arc::new(Value::Null)` sites), `src/read_plan.rs:176-179`
(the caller's overwrite)

**Problem**: every internal `compile_struct`/`compile_union`/
`wrap_leaf` builds a `ReadPlan` with a placeholder `Arc<Value::Null>`
schema that the top-level `compile` then overwrites via a `map` on the
result. Harmless today (the placeholder never escapes; the public
`schema()` accessor always returns the real doc), but it is a lie in
the type for the duration of the walk and a trap if a future refactor
adds a consumer mid-walk. Passing `schema: &Arc<Value>` (or the
already-`Arc`'d clone) down through `compile_struct` removes the
placeholder, the `map`, and the smell in one small change.

**Resolution (2026-09-02):** the recommended change, exactly.
`compile` builds the `Arc<Value>` once and threads `schema:
&Arc<Value>` down through `compile_struct`/`compile_field_list`/
`compile_field`/`compile_typeref`/`compile_resolved`/`compile_def_kind`/
`compile_union`/`compile_variant`/`compile_array`/`compile_record`;
every real `ReadPlan` carries `Arc::clone(schema)` at construction —
the placeholder-then-`map`-overwrite dance is gone. One
`Arc::new(Value::Null)` remains in `wrap_leaf`, now documented: the
wrapper is an anonymous synthetic node over a TypeRef (not a schema
struct), it never escapes (the materializer unwraps it;
`schema()` returns the root plan's doc), so `Null` is correct by
construction rather than a lie. No public-surface change.

### L3. Dead locals left from the phase-2 rewrite

**Files**: `src/materialize.rs:68` (`let _ = plan;` in
`materialize_plan_field`), `src/materialize.rs:360-365`
(`disc_field` bound then discarded via `let _ = disc_field;`)

**Problem**: `materialize_plan_field` takes `plan: &ReadPlan` solely
to discard it (`let _ = plan;`), and the field-disc union arm binds
`disc_field` from `shared_plan.fields().get(*field_index)` then
discards it — the materializer walks all shared fields and captures
the disc by name instead (the correct design, per the parity note at
:366-371). Both are clippy-invisible (`let _ =` suppresses the unused
warnings). Drop the `plan` parameter (and fix the recursive call
sites) and delete the `disc_field`/`let _` pair — or keep `disc_field`
and use it to assert the walk actually encountered the disc field
(the current `disc_value.ok_or_else(...)` at :382-384 already does
that job, so deletion is cleaner).

**Resolution (2026-09-02):** deletion taken on both counts. The `plan`
parameter is gone from `materialize_plan_field` (all four call sites
updated); the `disc_field`/`let _` pair is deleted, with
`DiscriminatorPlan::Field`'s `field_index` now matched as
`field_index: _` (the index is the *reader's* fast-path handle —
the materializer's order-walk + by-name capture is the correct
design here, exactly as the finding's parity note described). No
behavioral change; 508 tests green.

### L4. `OffsetMap::get` / `PackedLayout::get` are linear scans positioned as random access

**Files**: `src/offset_map.rs:154-159`, `src/layout_builder.rs:83-88`

**Problem**: both `get` implementations are
`self.fields.iter().find(…)` over a `Vec` — O(n) per lookup, on the
types whose docs pitch random access ("the consumer can read field N
without reading fields 0..N-1 first", offset_map.rs:92-94). Fine for
small schemas; the claim gets less honest with schema size, and
`read_field`/`write_field` (the aligned hot path for per-field
access) pays it per call. `ReadPlan` already moved to `BTreeMap` for
exactly this class of reason (phase 1, and to enable `Hash`). An
internal `BTreeMap<String, usize>` path→index alongside the
insertion-ordered `Vec` (keeping `iter()` order and the `Hash` derive
over the `Vec`) makes the claim true with no public-surface change.
Not a 0.3.0 blocker; a good 0.3.x cleanup with an existing in-repo
pattern to copy.

### L5. `FieldValue::Union`'s `variant_start` semantics are undocumented and inconsistent between discriminator kinds

**File**: `src/sequential_reader.rs:84-91`

**Status**: resolved 2026-09-02 (with H3).

**Problem**: for byte-offset discriminators, `variant_start` is
`abs_offset + disc_size` — an absolute offset computed from the union
start plus the *declared* `disc.offset` (which may be nonzero, so the
variant can start before or after the shared-field walk would place
it). For field-name discriminators it is the position after the whole
`shared` walk. The `FieldValue` enum doc (the "consumer recurses"
contract) says only "Byte offset where the variant struct begins" —
nothing about which base the offset is relative to, that byte-disc
`offset` is honored as an arbitrary displacement, or (per H3) that the
field-disc variant starts after shared fields while the builder lays
out only the variant. A consumer writing generic union-handling code
against `FieldValue::Union` alone can get this wrong silently. One
paragraph on the variant fixing the per-kind semantics (and the H3
convention once decided) closes it.

**Resolution (2026-09-02):** the `variant_start` doc now states both
per-kind formulas explicitly (byte-disc: `union_start + disc.offset +
disc.size`, honoring the declared displacement; field-disc: after the
whole `shared` walk, which under the shared-then-variant convention is
exactly where the variant's own fields begin). See the H3 resolution
block for the convention decision.

### L6. No write→read→validate roundtrip test for field-disc unions

**Files**: `tests/poc_roundtrip.rs` (no field-disc case at all),
`tests/tunion_dispatch.rs` (tunion-level only), the builder tests
(write-side only), the reader tests (read-side only)

**Status**: resolved 2026-09-02 (with H3).

**Problem**: every field-disc union test lives on one side of the
wire. The builder tests assume the variant-only layout
(`layout_builder.rs:1221-1334`); the reader/materializer tests assume
the shared-then-variant walk (`sequential_reader.rs:1260-1305`,
`materialize.rs:1183-1258`) — and the two fixtures don't even use the
same schema shape, so nothing would catch H3's divergence. A single
roundtrip test (build with `LayoutBuilder` → feed the buffer to
`SequentialReader::read_next` and `materialize_packed` →
`validate_bytes` on the engine) over a field-disc union with a
*non-redeclaring* variant and a second shared field would have caught
H3 immediately. This is the highest-value single test in the review —
add it with (or before) the H3 fix.

**Resolution (2026-09-02):**
`tests/poc_roundtrip.rs::field_disc_union_roundtrip_build_read_materialize_validate`
— exactly the test this finding asked for: union `fields: [type:
uint8, seq: uint32]` (disc + a second shared field), variant `Read {
handle: uint32 }` (non-redeclaring, numeric mapping keys `"1"`/`"2"`
matching the uint8 disc). Write with `LayoutBuilder` +
`data_access` (asserting `event.type@0, event.seq@1, event.handle@5`,
total 10), read with `SequentialReader::read_next` (asserts
`discriminator == "1"`, `variant_start == 5`), materialize with
`materialize_packed` (asserts `type`/`seq`/`handle`/`__discriminator`
in the object), and `engine.validate_bytes` on the same buffer. This
is the test that would have caught every part of H3.

### N1. Field-discriminator kind support differs between `tunion` and the reader

**Files**: `src/tunion.rs:136-164` (supports `String`/`Uint8`/`Enum`),
`src/sequential_reader.rs:613-645` (supports `String`/
`Uint8`/`Uint16`/`Uint32`/`Enum`)

**Problem**: `tunion::read_field_discriminator` rejects uint16/uint32
discriminator fields with a `Schema` error, while the plan reader
handles them. Parity-preserved (tunion is unchanged from 0.2.0), but
the public dispatch surface now has two answers to "which field kinds
can discriminate a union". Either extend `tunion` to match the reader
or document the divergence; the meta-schema does not constrain the
discriminator field's kind, so both code paths are reachable from the
same schema.

### N2. `align` annotations are unbounded — no crash, but absurd layouts and the reachable path to H1's byte cap

**Files**: `src/bast_meta.rs:64,79` (`"align": { "type": "integer",
"minimum": 1 }`, no maximum), `src/offset_map.rs` (`align_up`/
`round_up`), `src/bast.rs:926` (`parse_align`)

**Problem**: found during the H1 fix session (probe: a struct with
`align: 2^62` compiles in aligned mode and reports `total_size =
2^63`). The meta-schema accepts any `align >= 1`, and `align_up`
willingly rounds the running offset up by the full annotation — so a
one-field schema can declare a layout of exabyte scale. Unlike H1 this
does not crash: the allocation is offset *arithmetic*, not
`with_capacity`, and `validate_bytes` on the absurd layout fails
cleanly with a buffer-bounds `Access` error (probe-verified). Severity
is Nit because there is no abort and no unbounded memory *at read
time*; it is recorded because (a) `total_size` in that range is
meaningless output the consumer may act on, (b) the align-driven
stride is the one reachable path to `MAX_ARRAY_BYTES` (the H1 byte-cap
test in `offset_map` uses exactly this), and (c) the same unbounded
knob exists on `FieldDef.align`. A maximum align (e.g. 64 or 4096) in
the meta-schema and/or a clamp-with-error in `parse_align` closes it;
the honest layouts in the wild never need >64.

**Fix**: add `"maximum"` to both `align` properties in
`bast_meta.rs`, or reject/clamp oversized values in `parse_align`
(with a clean `Schema` error). A locking test (align above the cap →
clean `Err`) mirrors the H1 test family.

**Resolution (2026-09-02):** both layers, cap = 4096 (page granularity;
the finding's suggestion):

1. **`MAX_ALIGN = 4096`** added to `schema.rs` (documented with the
   N2 probe arithmetic: `align: 2^62` → `total_size = 2^63`).
2. **`parse_align` now returns `Result<Option<usize>>`** and rejects
   over-cap values with a clean `Schema` error naming the path, the
   value, and the maximum — a clamp-with-silent-drop was considered
   and rejected: it would change layout semantics without telling the
   consumer (AGENTS.md §3 wants a handleable error, not a surprise).
   Both call sites (`BastStruct::parse`, `BastField::parse`) thread the
   path. This closes the standalone `BastDoc::new` path, which never
   runs the meta-schema.
3. **Meta-schema `"maximum": 4096`** added to both `align` properties
   (`StructDef.align`, `FieldDef.align`) — the published
   `https://alk.dev/bast/v1/schema` contract now matches the parser.
4. **H1 byte-cap test retuned**: the old fixture used `align: 2^20` to
   reach `MAX_ARRAY_BYTES`; it now uses `align: 4096` × count 2^16 =
   2^28 > 2^26 — the byte cap stays reachable under the new align cap
   (the finding's point (b) remains testable).

Tests: `n2_struct_align_above_cap_rejected_at_parse` (message names
align + maximum), `n2_field_align_above_cap_rejected_at_parse`,
`n2_align_at_cap_accepted` (boundary: align 4096 accepted, total_size
4096). Verified: 511 tests green, clippy `-D warnings` clean, wasm
build green, `cargo doc --no-deps` zero warnings.

---

## What's Good

Worth recording, because the findings shouldn't eclipse it:

- **The compiled forms are well-built.** Both `ReadPlan::compile` and
  `ValidationPlan::compile` carry their own depth cap + definition-level
  cycle set (the correct untrusted-input shape), and the
  cycle-set-is-path-scoped semantics are tested for diamond refs on
  both (`repeated_ref_to_shared_def_is_allowed`,
  `shared_refs_compile_without_false_cycle`).
- **The engine's ValidationPlan-as-cycle-gate layering**
  (`engine.rs:146-152`) is a genuinely clever piece of defensive design,
  and it is documented at the place a future maintainer will read it.
- **Parity discipline is real, not ritual.** The effective-endian
  propagation divergence (phase 5) was found, fixed, and locked with
  tests on both the plan and the offset map; the stride fix (deferred
  decision 4) is documented in the `FieldValue::Array` doc comment with
  the behavioral-change note; the union paths were compared arm-by-arm
  against the 0.2.0 `read_union_value` in this review and match on
  every dispatch shape.
- **The fingerprint derives are sound.** Verified against the pinned
  `serde_json` 1.0.150 source: under `preserve_order`,
  `Map::hash` sorts keys (`map.rs:418-429`), so `Value: Hash` is
  consistent with `Value: Eq` and the phase-6 plan's analysis holds.
  Contract tests on all three plans (ReadPlan/OffsetMap/ValidationPlan)
  plus the root-name sensitivity test.
- **`Send + Sync` static assertions** on the plan types (phase 1 +
  phase 7), as ADR-011 required.
- **Docs/status hygiene is accurate.** Every status flip, bench number,
  and "closed" claim in the release commit checked out against the
  actual files.
- **Semver contract honored**: `lib.rs` re-exports match the plan's
  contract table exactly; the breaking surface is exactly the declared
  one.

## Recommended Order

1. ~~**H1** — release-blocking~~ **resolved 2026-09-02** (with L1;
   see the resolution block on the finding).
2. ~~**H3**~~ **resolved 2026-09-02** (with L5 + L6; see the
   resolution block on the finding).
3. ~~**H2** — shared walk guard~~ **resolved 2026-09-02** (see the
   resolution block on the finding).
4. ~~**M1 + M2**~~ **resolved 2026-09-02** (with M3; see the
   resolution blocks on the findings).
5. ~~**M3**~~ **resolved 2026-09-02** (see the resolution block on the
   finding).
6. **M4** — ongoing: per-fix coverage extension as recommended above;
   the aligned-materializer test gap (1) is the single biggest chunk
   and deserves its own session. (M1/M2/M3's fixes each extended
   coverage over their touched paths — the aligned record dispatch and
   the aligned record `validate_bytes` path are now publicly exercised.)
7. **L1–L6, N1, N2** — opportunistic, folded into whichever session
   touches the relevant file (L6 is the exception — it belongs with
   H3; N2 pairs naturally with any bast/bast_meta session; L1 done
   with H1; L5/L6 done with H3; L2/L3 done with the M-fix session;
   N2 done in the M-fix session's tail).

## Notes

- The probe file used in this review was deleted before commit; no
  test changes ship with this review.
- The H1 OOM incident (test run aborted the session's host) is the
  reason the two crash reproducers are described rather than committed.
  If a reproducer test is wanted in-tree for H1/H2, it should be
  `#[ignore]`-gated with a comment pointing at the isolation
  requirements, or assert only the compile-time rejection (the safe
  half of the fix) in the default suite. The H1 resolution followed
  the second option: in-tree tests assert only compile-time
  rejections.
- Coverage was measured with the default harness (`cargo llvm-cov
  --release`, summary + text). Numbers quoted are stable across two
  runs this session.
- Severity here keys off AGENTS.md §3 (untrusted schema ⇒ handleable
  error) and the semver contract, not off effort: two of the three
  Highs are single-file, small-diff fixes; H3 is the only one that
  needs a decision first.
- The H1 fix session (2026-09-02) also confirmed the zero-progress
  gap (stride-0 array + zero-byte elements = unbounded loop) and the
  unbounded-`align` observation (new N2) while re-deriving the cap
  arithmetic — both recorded on their own findings rather than
  silently absorbed.