diff --git a/docs/reviews/006-implementation-review-030.md b/docs/reviews/006-implementation-review-030.md
new file mode 100644
index 0000000..24c8792
--- /dev/null
+++ b/docs/reviews/006-implementation-review-030.md
@@ -0,0 +1,683 @@
+---
+status: open
+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 |
+|----------|------:|
+| High | 3 (H1, H2, H3) |
+| Medium | 4 (M1, M2, M3, M4) |
+| Low | 6 (L1–L6) |
+| Nit | 1 (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 should
+block publishing 0.3.0 to crates.io until fixed. 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.
+
+---
+
+## 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`).
+
+### 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.
+
+### 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.
+
+### 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`.
+
+### 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.
+
+### 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.
+
+### 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`
+
+**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).
+
+### 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.
+
+### 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).
+
+### 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`
+
+**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.
+
+### 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)
+
+**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.
+
+### 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.
+
+---
+
+## 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. Smallest correct fix: drop the three
+   `Vec::with_capacity(count)`; add the compile-time cap + tests.
+   Isolate the reproducer (see the Methodology warning).
+2. **H3** — needs a convention *decision* before code: write the
+   addendum, enforce it, add L6's roundtrip test in the same session.
+3. **H2** — shared walk guard (or the minimum doc note if the full
+   guard is judged too invasive for 0.3.x), plus the cyclic-schema
+   tests for all three walkers.
+4. **M1 + M2** — same file, same test family; do together.
+5. **M3** — trivial deletion (or the feature decision, if kept).
+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.
+7. **L1–L6, N1** — opportunistic, folded into whichever session touches
+   the relevant file (L6 is the exception — it belongs with H3).
+
+## 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.
+- 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.
\ No newline at end of file