From 2310f6cbd827cd312f72d7555f3d29dc3ac51259 Mon Sep 17 00:00:00 2001 From: "glm-5.2" Date: Wed, 19 Aug 2026 08:06:55 +0000 Subject: [PATCH] Propose ADR-012 + 0.3.0 implementation plan ADR-012 bundles two pieces of work into the 0.3.0 release so the crate ships one round of breaking changes, not two: - Fingerprinting: #[derive(Hash, Eq)] + fingerprint() -> u64 on ReadPlan and OffsetMap. BTreeMap for ReadPlan.by_name (HashMap blocks Hash derive). Fingerprint contract: equal hashes => identical reads over identical bytes. Enables cross-run plan caching, alkcall hub/spoke schema handshake, schema-version diagnostics. - Closing the deferred M1 sites via owned BastDoc (lifetime removal, scoped to LayoutBuilder/bast_validation/materialize_aligned/ OffsetMap::compute) + extending OffsetMap with LeafMeta {kind, encoding, endian} for the aligned read_field/write_field paths. Reframes the 'WritePlan' candidate from ADR-011's Future capabilities section: the packed write-side compiled form is PackedLayout; the aligned R/W compiled form is OffsetMap; the M1 fixes are 'cache the parse' and 'extend the compiled form with leaf metadata', not 'add a third compiled form.' Serves minimal-public-API-changes better than a literal WritePlan type. ValidationPlan deferred (different shape, not a hot loop). The plan (docs/plans/030-compiled-forms.md) is the execution entry point: seven phases ordered by dependency, each phase a session boundary. Phase 1-2: ReadPlan (ADR-011). Phase 3: owned BastDoc. Phase 4: LayoutBuilder M1 fix. Phase 5: OffsetMap LeafMeta. Phase 6: fingerprinting. Phase 7: version bump + docs + verification. Includes a semver contract table, deferred decisions, cross-phase invariants, and the verification block. ADR-011's Future capabilities section updated to point at ADR-012 for the items moving into 0.3.0 and record the WritePlan reframe. README ADR table gets ADR-012 as Proposed. Verification: docs-only change; cargo test --release, cargo clippy --all-targets -- -D warnings, cargo doc --no-deps unchanged (no source touched). --- docs/architecture/README.md | 1 + .../011-compiled-read-plan-for-packed-mode.md | 34 +- .../012-plan-fingerprinting-and-m1-closure.md | 360 ++++++++++++ docs/plans/030-compiled-forms.md | 537 ++++++++++++++++++ 4 files changed, 919 insertions(+), 13 deletions(-) create mode 100644 docs/architecture/decisions/012-plan-fingerprinting-and-m1-closure.md create mode 100644 docs/plans/030-compiled-forms.md diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 3d5dc0e..e15e251 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -46,6 +46,7 @@ format definition; the engine is generic. | [009](decisions/009-builder-api.md) | Builder API for Schema Construction | Fluent Rust API producing `serde_json::Value`; covers BAST kinds + standard JSON Schema; resolves OQ-003. *Output format amended to BAST / standard JSON Schema by ADR-BAST.* | | [010](decisions/010-generalized-validation-validate-bytes.md) | Generalized Validation — `validate_bytes` on `AlkTypeEngine` | Single-call binary-buffer validation; materialize `Value` from bytes, then validate. *Validation step amended to the BAST-native validator by ADR-VAL-SPLIT.* | | [011](decisions/011-compiled-read-plan-for-packed-mode.md) | Compiled Read Plan for Packed Mode | `ReadPlan` — the packed read-side compiled form, symmetric to `OffsetMap` (aligned) and `PackedLayout` (packed write). Closes review #004's 400x read-path gap; retires ADR-007's "re-parse on demand" framing. *Accepted.* | +| [012](decisions/012-plan-fingerprinting-and-m1-closure.md) | Plan Fingerprinting and Closing the Deferred M1 Sites in 0.3.0 | `ReadPlan`/`OffsetMap` `Hash + Eq` + `fingerprint()`; owned `BastDoc` (lifetime removal); `OffsetMap` carries `LeafMeta` to close the aligned-side M1 sites. Bundles with ADR-011 into one 0.3.0 breaking release. *Proposed.* | ## Relevant Open Questions diff --git a/docs/architecture/decisions/011-compiled-read-plan-for-packed-mode.md b/docs/architecture/decisions/011-compiled-read-plan-for-packed-mode.md index 30d912b..64e74c1 100644 --- a/docs/architecture/decisions/011-compiled-read-plan-for-packed-mode.md +++ b/docs/architecture/decisions/011-compiled-read-plan-for-packed-mode.md @@ -448,23 +448,31 @@ closes L2. L1 falls out at step 2. The aligned-side M1 paths `validate_bytes` (packed) consumes the `ReadPlan` via `materialize_packed` -## Future capabilities (not part of this ADR) +## Future capabilities (partly in 0.3.0 via ADR-012) The deterministic-compile property of `ReadPlan` is a prerequisite for -several capabilities that are explicitly out of scope here but worth -naming so a future ADR doesn't re-derive the prerequisite: +several capabilities. ADR-012 ("Plan Fingerprinting and Closing the +Deferred M1 Sites in 0.3.0") picks up the first two items below into +the 0.3.0 release so they ship with this ADR's breaking changes in one +round of downstream churn, not two: -- Fingerprinting the plan (e.g. `#[derive(Hash)]`) for cross-run - caching of compiled plans. -- Disk-cached compiled plans (skip `compile` on warm start). -- Schema-version handshakes for `alkcall`'s hub/spoke topology — - peers exchange plan fingerprints instead of full BAST documents. -- A `WritePlan` and/or `ValidationPlan` that follow the same - compile-once-walk-many pattern for the deferred M1 sites - (`engine.rs:334,467`, `layout_builder.rs:190`, `bast_validation`). +- **Fingerprinting the plan** (`#[derive(Hash)]` + a `fingerprint()` + method) for cross-run caching of compiled plans, disk-cached plans, + and `alkcall` schema-version handshakes. → **In 0.3.0 (ADR-012 §1).** +- **Closing the deferred M1 sites** via an owned `BastDoc` (lifetime + removal) for `LayoutBuilder` + extending `OffsetMap` with leaf + metadata for the aligned `read_field`/`write_field` paths. → **In + 0.3.0 (ADR-012 §2).** Note: ADR-012 reframes the earlier "WritePlan" + candidate listed here as "not a new type — extend the existing + compiled forms (`PackedLayout`/`OffsetMap`) and cache the parse." +- A `ValidationPlan` that follows the same compile-once-walk-many + pattern for `bast_validation`. → **Deferred (ADR-012).** Different + shape (value-domain, not byte-position); not a hot loop; separate + ADR if a bench motivates it. -None of these justify this ADR; the 400x read-path gap does. They are -listed only as forward references. +None of the in-0.3.0 items justify this ADR; the 400x read-path gap +does. They are listed here as forward references and to record that +the "WritePlan" candidate has been reframed out by ADR-012. ## POC coverage diff --git a/docs/architecture/decisions/012-plan-fingerprinting-and-m1-closure.md b/docs/architecture/decisions/012-plan-fingerprinting-and-m1-closure.md new file mode 100644 index 0000000..548942f --- /dev/null +++ b/docs/architecture/decisions/012-plan-fingerprinting-and-m1-closure.md @@ -0,0 +1,360 @@ +# ADR-012: Plan Fingerprinting and Closing the Deferred M1 Sites in 0.3.0 + +## Status + +Proposed. Bundles two pieces of work into the 0.3.0 release so the +crate ships one round of breaking changes, not two. Companion to +[ADR-011](011-compiled-read-plan-for-packed-mode.md) (the `ReadPlan`) +and the [0.3.0 implementation plan](../../plans/030-compiled-forms.md). + +## Context + +ADR-011 accepted the `ReadPlan` as the packed read-side compiled form +and deferred two things to "future capabilities": + +1. **Fingerprinting the plan** for cross-run caching, disk-cached + compiled plans, and `alkcall` hub/spoke schema-version handshakes. +2. **A `WritePlan` and/or `ValidationPlan`** following the same + compile-once-walk-many pattern for the deferred M1 sites. + +ADR-011 also explicitly deferred the aligned-side M1 sites +(`engine.rs:334,467` `read_field`/`write_field`; +`layout_builder.rs:190` `LayoutBuilder::build`) as "a deliberate +reversible bet that an aligned-mode hot loop won't emerge." + +This ADR retires both deferrals in one release. The reasoning is +timing: 0.3.0 is already a breaking bump (ADR-011 changes +`SequentialReader::new` and `materialize_packed` signatures), and the +crate has no real downstream consumers yet (only `alktty`/`alkcall`, +both in-house). Doing both pieces now costs one round of downstream +churn instead of two, and the fingerprinting work cuts across both +`ReadPlan` and `OffsetMap` — splitting would create a cross-release +dependency that's cleaner in one release. + +### Reframing "WritePlan" + +ADR-011's "Future capabilities" section listed a `WritePlan` as a +candidate. On inspection, a new public `WritePlan` type is the wrong +shape for the deferred M1 sites, for two reasons: + +1. **The packed write-side already has a compiled form: `PackedLayout`.** + `LayoutBuilder::build`'s M1 re-parse is the *builder* re-parsing + `BastDoc::new` on each `build()` call to get the typed tree it + walks. The fix is to cache the parsed tree on the builder at `new()` + time — internal, non-breaking, no new public type. The compiled + form (`PackedLayout`) is unchanged; only its construction stops + re-parsing. + +2. **The aligned R/W side already has a compiled form: `OffsetMap`.** + `read_field`/`write_field`'s M1 re-parse is `lookup_leaf_field` + walking `BastDoc` to get leaf metadata (`kind`, `encoding`, + `endian`) that `OffsetMap` doesn't carry. The fix is to extend + `OffsetMap`'s entries with that metadata at `compute` time — + additive fields on an existing public type (breaking, but we're + bumping anyway). No new public type. + +A new `WritePlan` type would overlap with `PackedLayout` (packed +write) and `OffsetMap` (aligned R/W) without a clean distinguishing +shape. The honest picture: the packed write-side compiled form is +`PackedLayout`; the aligned R/W compiled form is `OffsetMap`; the M1 +fixes are "cache the parse" and "extend the compiled form with leaf +metadata," not "add a third compiled form." This serves the +minimal-public-API-changes goal better than a literal `WritePlan`. + +### Deferring `ValidationPlan` + +The BAST-native validator (`bast_validation`) walks `BastDoc` to +check value-domain constraints (enum value sets, integer ranges, +`maxLength` caps, union variant keys). This is a different shape from +`ReadPlan`/`OffsetMap` (value-domain, not byte-position) and is not a +hot loop — validation is opt-in per operation per AGENTS.md. A future +`ValidationPlan` is a two-way door with its own ADR if a bench +motivates it; for 0.3.0 it is scope creep that would delay the +read-path fix. Explicitly deferred. + +## Decision + +### 1. Fingerprinting — `ReadPlan: Hash + Eq`, `OffsetMap: Hash + Eq` + +Add `#[derive(Hash, Eq)]` (alongside the existing `Debug, Clone, PartialEq`) +to `ReadPlan` and `OffsetMap`, plus their public sub-types +(`FieldPlan`, `CompositePlan`, `ReadKind`, `DiscriminatorPlan`, +`VariantPlan`, `ByteRange`, and the new `LeafMeta` — see §2). + +**`by_name` representation change.** `ReadPlan.by_name` is currently +`HashMap`. `HashMap` iteration order is non-deterministic +and `HashMap` does not implement `Hash`, which blocks `#[derive(Hash)]` +on `ReadPlan`. Switch `by_name` to `BTreeMap`. Lookup +cost at protocol-header N (~5 fields) is negligible (the `BTreeMap` is +only used by `read_field`'s name→index lookup, not by the sequential +`read_next` hot path). This makes the derived `Hash` cover the full +structural state of the plan. + +**Fingerprint contract.** Two plans with equal `Hash` (or equal under +`PartialEq`) produce identical reads over identical bytes. Formally: +`plan1 == plan2 ⟹ ∀ buffer. read(plan1, buffer) == read(plan2, buffer)`. +This is the contract the downstream uses rely on: + +- **Cross-run disk cache.** A consumer can hash a `ReadPlan`/ + `OffsetMap` and cache the compiled plan keyed by the hash, skipping + `compile` on warm starts. Safe because the contract guarantees a + cache hit produces identical read behavior. +- **`alkcall` hub/spoke schema handshake.** Peers exchange plan + fingerprints instead of full BAST documents. A peer that receives a + fingerprint it has already compiled can skip re-transmitting the + schema. The contract guarantees fingerprint equality implies + behavioral equivalence, so the handshake is sound. +- **Schema-version diagnostics.** A consumer can log a plan + fingerprint alongside read results for reproducibility — two runs + over "the same schema" that produce different fingerprints reveal a + silent schema drift. + +The contract is a *behavioral* equivalence, not a structural identity: +two plans with different `by_name` insertion order but the same +`fields` Vec produce the same reads, and after the `BTreeMap` change +they also produce the same `Hash`. The contract is documented on the +`Hash` impl and tested by a property-style test (compile the same +schema twice, assert `plan1 == plan2` and `plan1.hash() == +plan2.hash()`). + +**Fingerprint API.** No new public method is strictly needed — +consumers call `std::hash::Hash` directly. For ergonomics and to make +the contract visible, add a convenience method: + +```rust +impl ReadPlan { + /// A stable 64-bit fingerprint of this plan's read behavior. + /// + /// Two plans with the same fingerprint produce identical reads + /// over identical bytes (the fingerprint contract). + pub fn fingerprint(&self) -> u64; +} +impl OffsetMap { + /// A stable 64-bit fingerprint of this offset map's read/write + /// behavior. Same contract as `ReadPlan::fingerprint`. + pub fn fingerprint(&self) -> u64; +} +``` + +Implemented via `std::hash::DefaultHasher` (or a stable hasher like +`FxHasher` if we want cross-version stability — decision belongs to +the implementation step, called out in the plan). The fingerprint is +additive API, not breaking. + +### 2. Closing the deferred M1 sites + +#### 2a. `LayoutBuilder` — cache the parsed `BastDoc` at `new()` + +`LayoutBuilder` currently stores `doc_value: Value` + `root_name: String` +and re-parses `BastDoc::new(&self.doc_value, &self.root_name)` on every +`build()` call (`layout_builder.rs:190`). The fix: store the parsed +typed tree at `new()` time and reuse it in `build()`. + +This requires `BastDoc` to be owned (no lifetime borrowing from +`doc_value`). Two options: + +- **Option α (smaller):** keep `BastDoc<'a>` borrowing, store + `doc_value: Value` + a *pre-resolved, owned* representation of just + what `build` needs (the field tree with `$ref`s resolved). This is + essentially a `WritePlan` by another name — rejected per the + reframing above. +- **Option β (cleaner):** make `BastDoc` own its data. This is + review #004's Option A, scoped to `LayoutBuilder` only. It's a + larger refactor but eliminates the lifetime entanglement for the + builder and is the prerequisite for any future owning consumer that + wants to cache the parsed tree. + +**Decision: Option β, scoped to `LayoutBuilder`.** The `BastDoc<'a>` → +`BastDoc` (owned) refactor is the principled fix and is already +breaking (the `Bast*` types are re-exported from `lib.rs`), so it +rides the 0.3.0 bump. This does *not* change `SequentialReader` or +`materialize_packed` (those consume `ReadPlan` per ADR-011, not +`BastDoc`). It changes `LayoutBuilder::new` to parse once and `build` +to reuse. The `doc_value: Value` field is removed; the builder holds +the owned `BastDoc` directly. + +**Note on `BastDoc` ownership scope:** ADR-011 left `BastDoc` borrowed +and unchanged ("the validation-side typed tree"). This ADR changes +that: `BastDoc` becomes owned. The validation-side (`bast_validation`) +and aligned-side (`OffsetMap::compute`, `materialize_aligned`) +consumers adapt to the owned `BastDoc` — they no longer need a +borrowed `&Value` kept alive alongside. This is a net simplification: +one typed-tree type, owned, used by all non-`ReadPlan` consumers. The +POC on `readplan-poc` confirmed `ReadPlan` doesn't need `BastDoc` to +be borrowed (it compiles from `&Value` once and discards the +`BastDoc`), so making `BastDoc` owned doesn't regress the read path. + +#### 2b. `OffsetMap` — carry leaf metadata + +`OffsetMap` currently stores `Vec<(String, ByteRange)>`. The +`read_field`/`write_field` M1 re-parse is `lookup_leaf_field` walking +`BastDoc` to get `LeafFieldInfo { kind, encoding, endian }` +(`engine.rs:556-600`). The fix: extend `OffsetMap`'s entries to carry +that metadata at `compute` time. + +```rust +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct LeafMeta { + pub kind: AlkTypeKind, + pub encoding: VariableEncoding, + pub endian: Endian, +} + +pub struct OffsetMap { + fields: Vec<(String, ByteRange, LeafMeta)>, // was Vec<(String, ByteRange)> + total_size: usize, +} +``` + +`OffsetMap::compute` resolves each leaf field's `LeafMeta` during the +walk (it already walks the tree; it just doesn't currently record the +metadata). `read_field`/`write_field` drop the `BastDoc::new` + +`lookup_leaf_field` calls and read `LeafMeta` from the map. The +`LeafFieldInfo` struct in `engine.rs` is removed (replaced by +`OffsetMap`'s `LeafMeta`). + +**Breaking changes:** +- `OffsetMap::get` return type: `Option<&ByteRange>` → + `Option<(&ByteRange, &LeafMeta)>` (or a small accessor struct). + Call sites in `alktty`/`alkcall` update with the bump. +- `ByteRange` is unchanged (still `Copy + Hash`). +- `LeafMeta` is a new public type, re-exported from `lib.rs`. + +This is additive on the *capability* (the map now answers questions it +previously couldn't) but breaking on the *signature* (`get`'s return +type changes). Rides the 0.3.0 bump. + +### 3. Fingerprinting `OffsetMap` (bundled with §2b) + +Since `OffsetMap` is getting new fields (`LeafMeta`) in §2b, its +`#[derive(Hash, Eq)]` (from §1) covers the new fields automatically. +The fingerprint contract for `OffsetMap` is the aligned-side analog +of `ReadPlan`'s: two offset maps with equal hashes produce identical +aligned reads/writes over identical bytes. + +## Scope + +### In scope + +- `ReadPlan: Hash + Eq` + `fingerprint()` method (§1). +- `OffsetMap: Hash + Eq` + `fingerprint()` method (§1, §3). +- `BastDoc<'a>` → `BastDoc` (owned) refactor, scoped to the consumers + that currently hold `doc_value: Value` and re-parse: `LayoutBuilder`, + `bast_validation`, `materialize_aligned`, `OffsetMap::compute` + (§2a). `ReadPlan::compile` and the packed read path are unaffected + (they consume `&Value` once and discard `BastDoc`). +- `LayoutBuilder` caches the owned `BastDoc` at `new()`, `build()` + reuses it — no re-parse (§2a). +- `OffsetMap` carries `LeafMeta`; `read_field`/`write_field` drop + `BastDoc::new` + `lookup_leaf_field` (§2b). +- `LeafMeta` new public type (§2b). +- `BTreeMap` for `ReadPlan.by_name` (§1). + +### Out of scope + +- `ValidationPlan` — different shape, not a hot loop, deferred (see + "Deferring `ValidationPlan`" above). +- Disk-cache or handshake *implementations* — the fingerprint + *contract* and method are in scope; the downstream uses (cache + format, wire protocol) are the consumers' problem, not this ADR's. +- Aligned-mode `validate_bytes` — already uses `OffsetMap` + the + BAST-native validator; the `BastDoc` ownership change touches it + but no new compiled form is needed. +- Cross-version fingerprint stability — the fingerprint is stable + within a crate version but may change across versions (a new + `AlkTypeKind` variant, for example, changes the hash). Cross-version + stability is a non-goal; consumers cache within a version. The + implementation step chooses a hasher and documents the stability + contract. + +## Consequences + +### Positive + +- **One breaking release, not two.** ADR-011's `ReadPlan` + this + ADR's `BastDoc`-owned + `OffsetMap` extension ship together. The + two in-house downstream consumers (`alktty`, `alkcall`) update once. +- **Closes all deferred M1 sites.** `LayoutBuilder::build` + (`layout_builder.rs:190`), `read_field` (`engine.rs:334`), + `write_field` (`engine.rs:467`) all stop re-parsing. The packed-side + `validate_bytes` (`engine.rs:284`) was already closed by ADR-011; + this ADR closes the aligned-side equivalent. +- **Fingerprinting enables downstream uses.** Cross-run plan caching, + `alkcall` schema handshake, and schema-version diagnostics all + become possible without further API work. +- **`BastDoc` owned is a net simplification.** One typed-tree type, + owned, used by all non-`ReadPlan` consumers. No more + lifetime-entanglement workarounds. The "re-parse on demand" framing + from ADR-007 is fully retired across both read and write paths. +- **`OffsetMap` extension is additive capability.** The map now + answers `kind`/`encoding`/`endian` questions it previously couldn't, + enabling future aligned-side tools without re-walking `BastDoc`. + +### Negative + +- **Breaking public-API changes (0.2.0 → 0.3.0).** `BastDoc<'a>` → + `BastDoc` (owned) changes every `Bast*` signature that took `&'a`. + `OffsetMap::get` return type changes. `LeafMeta` is new public. + `ReadPlan` is new public (from ADR-011). All ride the bump. +- **`BastDoc` ownership refactor is broad.** Touches `bast.rs` (every + typed node: `&'a str` → `String`/`Arc`, `&'a Value` → + `Value`/`Arc`) and every consumer (`layout_builder`, + `offset_map`, `materialize`, `bast_validation`, `engine`). This is + review #004's Option A, which ADR-011 deferred — this ADR picks it + up because the `LayoutBuilder` M1 fix requires it and we're bumping + anyway. The refactor is mechanical (lifetime removal, not logic + rewrites); the POC on `readplan-poc` confirmed the read path is + unaffected. +- **Fingerprint cross-version stability is not guaranteed.** A future + `AlkTypeKind` variant changes the hash. Documented as a within- + version contract. Consumers that need cross-version stability + serialize the BAST document and re-compile. +- **`BTreeMap` for `by_name` is a tiny lookup cost.** Negligible at + protocol-header N; irrelevant to the 400x fix. + +## Scope Boundaries (What This Is Not) + +- **Not a `WritePlan` type.** The packed write-side compiled form is + `PackedLayout`; the aligned R/W compiled form is `OffsetMap`. The + M1 fixes are "cache the parse" (§2a) and "extend the compiled form + with leaf metadata" (§2b), not "add a third compiled form." +- **Not a `ValidationPlan`.** Deferred — different shape, not hot. +- **Not cross-version fingerprint stability.** Within-version only. +- **Not a disk-cache or wire-protocol spec.** The fingerprint contract + and method are in scope; the downstream uses are the consumers' + concern. + +## Recommended Order + +See [the 0.3.0 implementation plan](../../plans/030-compiled-forms.md) +for the step-by-step execution order. The high-level grouping: + +1. **`ReadPlan` (ADR-011 steps 1–5)** — the packed read-path fix. Closes + H1 + packed-side M1 + L1 + L2. +2. **`BastDoc` owned (§2a)** — the typed-tree ownership refactor. Prerequisite + for the `LayoutBuilder` M1 fix. +3. **`LayoutBuilder` M1 fix (§2a)** — cache the owned `BastDoc` at `new()`. +4. **`OffsetMap` extension (§2b)** — carry `LeafMeta`; close the + aligned-side `read_field`/`write_field` M1. +5. **Fingerprinting (§1, §3)** — `Hash + Eq` + `fingerprint()` on + `ReadPlan` and `OffsetMap`. Rides on top of the above. +6. **Public API bump (0.2.0 → 0.3.0)** — `lib.rs` re-exports, version + bump, update `alktty`/`alkcall`. +7. **Verification block** — full suite + wasm + bench. + +## References + +- [ADR-011](011-compiled-read-plan-for-packed-mode.md) — the + `ReadPlan` (packed read-side compiled form). This ADR extends the + 0.3.0 release with fingerprinting and the deferred M1 fixes. +- [Review #004](../../reviews/004-performance-review.md) — the + performance finding (H1, M1, L1, L2). ADR-011 closed H1 + packed + M1 + L1 + L2; this ADR closes the aligned-side M1. +- [ADR-007](007-packed-mode-read-factory.md) — the "re-parse on + demand" framing, retired across both read and write paths by + ADR-011 + this ADR. +- [ADR-002](002-two-layout-modes-packed-vs-aligned.md) — the two + layout modes; `OffsetMap` is the aligned R/W compiled form extended + here with `LeafMeta`. +- [0.3.0 implementation plan](../../plans/030-compiled-forms.md) — + the step-by-step execution plan. \ No newline at end of file diff --git a/docs/plans/030-compiled-forms.md b/docs/plans/030-compiled-forms.md new file mode 100644 index 0000000..04d6600 --- /dev/null +++ b/docs/plans/030-compiled-forms.md @@ -0,0 +1,537 @@ +--- +status: in-progress +created: 2026-08-19 +last_updated: 2026-08-19 +adr: ADR-011, ADR-012 +--- + +# 0.3.0 — Compiled Forms: ReadPlan, Owned BastDoc, OffsetMap LeafMeta, Fingerprinting + +This is the execution plan for the 0.3.0 release: the compiled-form +rollup that closes review #004's 400x read-path gap (ADR-011) *and* +the deferred M1 sites (ADR-012) *and* adds plan fingerprinting +(ADR-012) in one breaking bump. It is the **entry point** an +implementing agent reads first. + +Companion documents: + +- [ADR-011](../architecture/decisions/011-compiled-read-plan-for-packed-mode.md) + — the `ReadPlan` decision (packed read-side compiled form). +- [ADR-012](../architecture/decisions/012-plan-fingerprinting-and-m1-closure.md) + — fingerprinting + owned `BastDoc` + `OffsetMap` `LeafMeta` (this + release's two other pieces). +- [Review #004](../reviews/004-performance-review.md) — the + performance finding being closed. +- [POC findings](../../poc/readplan/FINDINGS.md) (branch `readplan-poc`) + — the derisking POC that confirmed the `ReadPlan` shape and surfaced + two findings (field-disc union read shape; struct-array stride). + +**Working order:** read this plan top-to-bottom. The Semver Contract +section is the scope-creep guardrail — consult it before each step. +Each step links to its ADR and lists its verification gate. Implement +phases in order; within a phase, steps are ordered by dependency. + +## Phases vs sessions + +This plan is deliberately larger than one session's work. The seven +phases are the session boundaries — each phase is a coherent unit +that leaves the tree building and tests green, so any one session +can pick up a phase without needing context from the previous one. +Phase boundaries are also commit boundaries (and push boundaries per +AGENTS.md). If a phase is large enough to span sessions, the steps +within it are the sub-session boundaries. + +## Semver Contract + +The crate is on crates.io at 0.2.0 with zero real consumers (only +`alktty`/`alkcall`, both in-house path dev-deps). A breaking bump to +0.3.0 is free but the contract is explicit so the implementation +doesn't drift. Per AGENTS.md, the public surface is the `lib.rs` +re-exports. + +| Public item (from `lib.rs` re-exports) | Class | Change | +|---|---|---| +| `AlkTypeEngine::compile` | **Breaking (internal)** | Signature unchanged `(bast_doc: &Value, root_name: &str, mode, json_schema) -> Result`. Internally builds a `ReadPlan` (packed) or extended `OffsetMap` (aligned) and stores it. The `bast_doc: Value` clone is retained (ADR-011 §Engine integration). | +| `AlkTypeEngine::sequential_reader` | **Breaking (return type)** | Returns `Option` (unchanged type), but the reader is now constructed from `Arc`, not from `&bast_doc`. The reader's public methods (`read_next`/`read_field`/`reset`/`position`/`endian`/`schema`) keep their signatures. `schema()` returns the `&Value` the plan was compiled from (retained on the engine). | +| `AlkTypeEngine::read_field` / `write_field` | **Unchanged (signature)** | Still `(buffer, field_path) -> Result`. Internally reads `LeafMeta` from the extended `OffsetMap` instead of re-parsing `BastDoc`. | +| `AlkTypeEngine::validate_bytes` | **Unchanged (signature)** | Packed mode calls `materialize_packed(&self.plan, buffer)` (ADR-011); aligned mode calls `materialize_aligned(&doc, buffer, &self.offset_map)` with the owned `BastDoc`. | +| `LayoutMode`, `AlkTypeEngine` | **Unchanged** | — | +| `BastDoc`, `BastDef`, `BastDefKind`, `BastStruct`, `BastField`, `BastType`, `BastUnion`, `BastDiscriminator`, `BastEnum`, `BastArray`, `BastRecord`, `BastRef` | **Breaking (lifetime removal)** | `BastDoc<'a>` → `BastDoc` (owned). Every `&'a str` → `String` (or `Arc` — decision in phase 3). Every `&'a Value` → `Value` (or `Arc`). Every method signature that took/returned `&'a` changes. The `Bast*` types are re-exported from `lib.rs` so this is a public break. | +| `OffsetMap` | **Breaking (`get` return type)** | `get(field_path) -> Option<&ByteRange>` → `get(field_path) -> Option<&OffsetEntry>` where `OffsetEntry { range: ByteRange, meta: LeafMeta }` (or two accessors). Additive capability. | +| `ByteRange` | **Unchanged** | Still `Copy + PartialEq + Eq + Hash`. | +| `LeafMeta` | **New public type** | `{ kind: AlkTypeKind, encoding: VariableEncoding, endian: Endian }`, re-exported from `lib.rs`. `Copy + PartialEq + Eq + Hash`. | +| `ReadPlan` | **New public type** | From ADR-011. Re-exported from `lib.rs`. `Debug + Clone + PartialEq + Eq + Hash`. | +| `SequentialReader` | **Breaking (constructor)** | `SequentialReader::new(&Value, &str)` → `SequentialReader::new(Arc)`. Public methods (`read_next`/`read_field`/`reset`/`position`/`endian`/`schema`) unchanged. `schema()` returns `&Value` retained on the plan. | +| `FieldValue` | **Unchanged** | — | +| `materialize_packed` | **Breaking (signature)** | `materialize_packed(&BastDoc<'_>, &[u8])` → `materialize_packed(&ReadPlan, &[u8])`. | +| `materialize_aligned` | **Breaking (signature)** | `materialize_aligned(&BastDoc<'_>, &[u8], &OffsetMap)` → `materialize_aligned(&BastDoc, &[u8], &OffsetMap)` (owned `BastDoc`, no lifetime). | +| `LayoutBuilder`, `PackedLayout`, `FieldPosition` | **Unchanged (signature)** | `LayoutBuilder::new`/`build` signatures unchanged. Internally caches the owned `BastDoc` instead of re-parsing. | +| `AlkTypeKind`, `Endian`, `VariableEncoding` | **Unchanged** | — | +| `AlkTypeError` | **Unchanged** | — | +| `Schema`, `Definitions`, `Discriminator` builders | **Unchanged** | — | +| `UnionDispatch`, `build_validator`, `BAST_META_SCHEMA` | **Unchanged** | — | +| `data_access::*` functions | **Unchanged** | — | + +**Net breaking surface:** `BastDoc` and all `Bast*` types (lifetime +removal), `OffsetMap::get` (return type), `SequentialReader::new` +(constructor), `materialize_packed`/`materialize_aligned` (signatures). +**Net additive:** `ReadPlan`, `LeafMeta`, `fingerprint()` methods, +`Hash + Eq` derives on `ReadPlan`/`OffsetMap`. **Net unchanged:** the +builder, `AlkTypeEngine` accessors (signatures), `FieldValue`, +`AlkTypeKind`, `AlkTypeError`, `data_access`, the `Schema`/`Definitions` +builders. + +### Decisions deferred to their implementation phases + +1. **`Arc` vs `String` for owned `BastDoc` names** (phase 3): `Arc` + shares allocation for repeated names (e.g. union variant keys appearing + in multiple places); `String` is simpler. The POC used `String`. Lean: + `String` unless a bench shows `Arc` matters — the typed tree is + built once, not hot. Decided in phase 3. +2. **`OffsetMap::get` return shape** (phase 5): `Option<&OffsetEntry>` (a + new accessor struct) vs two methods `range(path) -> Option<&ByteRange>` + + `meta(path) -> Option<&LeafMeta>`. The struct is fewer calls; the two + methods preserve back-compat shape for callers that only want the range. + Lean: struct — it's a breaking bump anyway and the struct is cleaner. + Decided in phase 5. +3. **Fingerprint hasher** (phase 6): `DefaultHasher` (std, stable within a + version) vs `FxHasher` (faster, also stable). Cross-version stability + is a non-goal (ADR-012). Lean: `DefaultHasher` — no new dep, the + fingerprint isn't hot. Decided in phase 6. +4. **Struct-array stride** (phase 2): the POC found the existing reader + returns `element_stride: 0` for fixed-size struct arrays + (`sequential_reader.rs:567`). The `ReadPlan` correctly computes the + stride. Decision: preserve the existing `0` behavior in `ReadPlan` for + back-comat with `SequentialReader`'s consumer contract, *or* fix it + and document the behavioral change. Lean: fix it — the `0` is a latent + bug, the stride is behaviorally observable, and we're bumping. The + plan step calls this out explicitly. Decided in phase 2. + +## Phase 1 — `ReadPlan` type + `compile` (ADR-011 step 1) + +**Goal:** Add the `ReadPlan`/`FieldPlan`/`CompositePlan`/`ReadKind`/ +`DiscriminatorPlan`/`VariantPlan` types and `ReadPlan::compile(&Value, +&str) -> Result`. Pure addition; no existing code +touched. This is the foundation — later phases wire it into the engine +and reader. + +**ADR reference:** [ADR-011 §The `ReadPlan` shape](../architecture/decisions/011-compiled-read-plan-for-packed-mode.md#the-readplan-shape), +[ADR-011 §Construction](../architecture/decisions/011-compiled-read-plan-for-packed-mode.md#construction). + +**POC reference:** `poc/readplan/src/lib.rs` (branch `readplan-poc`) is +the reference scaffold. The production version lives in `src/` and +adds doc comments, clippy cleanliness, and the field-name-discriminator +union read shape the POC stubbed (see POC Finding 1). + +**Files:** New `src/read_plan.rs`. Update `src/lib.rs` to add +`pub mod read_plan;` and re-export `ReadPlan` (and the plan sub-types +that are part of the public surface — `ReadKind`, `CompositePlan`, etc. +if the ADR's public-API section calls for them; the ADR lists +`ReadPlan` as the public type, sub-types can stay `pub` in-module if +consumers don't need to name them). + +**Implementation notes:** +- `compile` walks `BastDoc` once (via the existing borrowed `BastDoc`, + which still exists at this phase — the owned-`BastDoc` refactor is + phase 3). Resolves all `$ref`s eagerly, computes effective endianness + at every node, inlines union variants. Malformed schemas surface as + `AlkTypeError::Schema` (AGENTS.md §3); overflow-safe arithmetic + (AGENTS.md §4). +- `by_name: BTreeMap` (not `HashMap` — ADR-012 §1 + requires `Hash` on `ReadPlan`, and `HashMap` blocks derive). The + POC used `HashMap`; swap to `BTreeMap`. +- **Field-name-discriminator union read shape (POC Finding 1):** the + POC stubbed `plan_read_union`'s `Field` arm. The production + `compile_union` must produce a plan that the read loop can walk for + the field-disc case. The shape: `CompositePlan::Union` carries the + union's declared `fields` as a sub-`ReadPlan` (the discriminator field + + any shared fields), and the variant plans are laid out *after* the + shared fields. The read loop reads the discriminator field from the + sub-plan, looks up the variant, and walks the variant plan. This is + the one piece the POC deliberately left as a TODO — the production + version must implement it. See `poc/readplan/FINDINGS.md` Finding 1. +- Do *not* wire `ReadPlan` into `SequentialReader` or `materialize` yet + — that's phase 2. Phase 1 is the type + `compile` only, unit-tested + against the same BAST fixtures `bast.rs` uses (the existing `bast.rs` + tests are a ready source of fixtures). + +**Verification:** `cargo test --release` (new unit tests for `compile` +covering every `BastType` arm — port the `cov_*` tests from +`poc/readplan/tests/coverage.rs`). `cargo clippy --all-targets -- -D +warnings`. `cargo doc --no-deps` (new public type). `cargo build +--target wasm32-unknown-unknown --release` (`read_plan.rs` is +wasm-relevant). + +--- + +## Phase 2 — `SequentialReader` + `materialize_packed` consume `ReadPlan` (ADR-011 steps 2–4) + +**Goal:** Rewrite the packed read loop to walk `&ReadPlan` instead of +reconstructing `BastDoc`. `SequentialReader` stores `Arc` + +cursor state; `materialize_packed` takes `&ReadPlan`. Closes H1 +(the 400x gap) + the packed-side M1 + L1. + +**ADR reference:** [ADR-011 §Scope](../architecture/decisions/011-compiled-read-plan-for-packed-mode.md#scope), +[ADR-011 §Recommended Order](../architecture/decisions/011-compiled-read-plan-for-packed-mode.md#recommended-order) +steps 2–4. + +**Files:** `src/sequential_reader.rs` (rewrite the read loop, change +`new`'s signature), `src/materialize.rs` (`materialize_packed` takes +`&ReadPlan`), `src/engine.rs` (`compile` builds `Arc` in +packed mode, `sequential_reader()` hands out `Arc::clone`, packed +`validate_bytes` calls `materialize_packed(&self.plan, ...)`). + +**Implementation notes:** +- `SequentialReader::new(&Value, &str)` → + `SequentialReader::new(Arc)`. The reader stores + `plan: Arc`, `field_index: usize`, `position: usize`. + `endian()` reads `self.plan.endian()`. `schema()` returns a `&Value` + retained on the plan (the plan stores the `&Value` it was compiled + from — see ADR-011 §Engine integration; the `&Value` outlives the + plan because the engine owns both). +- `read_field_at`/`read_field_value`/`read_typeref_value`/ + `walk_struct_size`/`read_union_value`/`read_array_value`/ + `read_record_value` are rewritten to take plan nodes + (`&FieldPlan`/`&CompositePlan`/`&ReadKind`) instead of + `&BastField`/`&BastType`/`&BastDoc`. Port `plan_read_field_at`/ + `plan_walk_struct_size`/etc. from `poc/readplan/src/lib.rs` — they're + the reference implementations. +- **Struct-array stride (deferred decision 4):** the POC computes the + true fixed-struct stride; the existing reader returns `0`. The + production `ReadPlan::compile_array` should compute the true stride + (the POC's `fixed_struct_size` helper). Document the behavioral + change in the `FieldValue::Array` doc comment: `element_stride` is + now the true stride for fixed-size struct elements, not `0`. This is + a breaking behavioral change; rides the bump. Update the + `eq_array_ref_element`-style test to assert the new stride. +- `materialize_packed(&BastDoc<'_>, &[u8])` → + `materialize_packed(&ReadPlan, &[u8])`. The materializer walks the + plan instead of `BastDoc`. The `dummy_field_for`/`ty_source` helpers + in `materialize.rs` are removed (the plan carries everything). +- `AlkTypeEngine::compile` (packed branch): build `Arc` via + `ReadPlan::compile(bast_doc, root_name)`, store it in `Layout::Packed`. + `sequential_reader()` returns + `Some(SequentialReader::new(Arc::clone(&self.plan)))`. + `validate_bytes` (packed) calls + `materialize_packed(&self.plan, buffer)` then `bast_validation` on + the result (validation still uses `BastDoc` until/unless a future + `ValidationPlan` ADR; for 0.3.0 `validate_bytes` reconstructs a + `BastDoc` for the validator only — the *read* path uses the plan, + the *validation* path uses `BastDoc`. This is acceptable: validation + is not the hot loop H1 traces). + +**Verification:** `cargo test --release` — the existing +`sequential_reader.rs` and `materialize.rs` tests drive `read_next`/ +`read_field`/`reset`/`validate_bytes` through the public API, so they +validate the rewrite without modification. If any test breaks, the +rewrite diverged from the existing behavior — investigate before +patching the test. `cargo clippy --all-targets -- -D warnings`. +`cargo build --target wasm32-unknown-unknown --release`. **Re-run the +alktty `wire_vs_bast` bench** to confirm the 400x gap closes (this is +the headline result; record the before/after numbers in the commit +message). + +--- + +## Phase 3 — Owned `BastDoc` (ADR-012 §2a) + +**Goal:** Make `BastDoc` own its data (drop the `<'a>` lifetime). +`&'a str` → `String`, `&'a Value` → `Value` (or `Arc`/`Arc` +— deferred decision 1). This is the prerequisite for the `LayoutBuilder` +M1 fix (phase 4) and simplifies all owning consumers. Broad but +mechanical refactor. + +**ADR reference:** [ADR-012 §2a](../architecture/decisions/012-plan-fingerprinting-and-m1-closure.md#2a-layoutbuilder--cache-the-parsed-bastdoc-at-new). + +**Files:** `src/bast.rs` (every `Bast*` type), every consumer: +`src/layout_builder.rs`, `src/offset_map.rs`, `src/materialize.rs`, +`src/bast_validation.rs`, `src/engine.rs`, `src/tunion.rs`, +`src/bast_meta.rs` (if it walks `BastDoc`), `src/builder.rs` (if it +consumes `Bast*`). `src/lib.rs` re-exports (signatures change but +names stay). + +**Implementation notes:** +- `BastDoc<'a>` → `BastDoc`. Fields: `root: Value` (was `&'a Value`), + `root_name: String` (was `&'a str`), `root_def: BastDef` (was + `BastDef<'a>`). `new(root: &Value, root_name: &str)` takes references + *in* (the caller still owns the input `Value`) but clones into owned + storage. The `&Value` → `Value` clone is the cost of ownership; it + happens once at `compile`/`new`, not per-field. +- `BastDef<'a>` → `BastDef`: `name: String`, `kind: BastDefKind`, + `source: Value`. +- `BastStruct<'a>` → `BastStruct`: `endian: Endian`, `align: Option`, + `fields: Vec`, `source: Value`. +- `BastField<'a>` → `BastField`: `name: String`, `ty: BastType`, + `endian: Option`, `align: Option`, + `encoding: VariableEncoding`, `max_length: Option`, + `source: Value`. `synthetic` constructor takes owned `BastType` + + `Value`. +- `BastType<'a>` → `BastType`: `Primitive(AlkTypeKind)`, + `Ref(BastRef)`, `Array(BastArray)`, `Record(BastRecord)`, + `Struct(BastStruct)`, `Union(BastUnion)`, `Enum(BastEnum)`. +- `BastUnion<'a>` → `BastUnion`: `endian: Endian`, + `discriminator: BastDiscriminator`, `fields: Vec`, + `mapping: Vec<(String, BastType)>` (was `Vec<(&'a str, BastType)>`), + `source: Value`. +- `BastDiscriminator::Field { name: String }` (was `name: &'a str`). +- `BastEnum<'a>` → `BastEnum`: `values: Vec` (was + `Vec<&'a str>`), `source: Value`. +- `BastArray<'a>` → `BastArray`: `element: Box`, `count: usize`, + `source: Value`. +- `BastRecord<'a>` → `BastRecord`: `values: Box`, + `source: Value`. +- `BastRef<'a>` → `BastRef`: `name: String`. +- **`resolve_typeref` / `resolve_ref` / `lookup_def`** now return owned + `BastType`/`BastDef`/`Value` instead of borrowed. The `clone()` in + the current `resolve_typeref` passthrough (`other => Ok(other.clone())`) + is no longer needed for the borrow case (everything is owned) but + the logic is unchanged — `BastType` is `Clone` either way. +- **Consumers adapt:** any code that held `&'a Value` alongside a + `BastDoc<'a>` (e.g. `LayoutBuilder.doc_value`, `AlkTypeEngine.bast_doc`, + `SequentialReader.doc_value` — though the reader is already on + `ReadPlan` after phase 2) drops the separate `Value` and holds the + owned `BastDoc` directly. `materialize_packed` is already on + `ReadPlan` (phase 2) and doesn't need `BastDoc` — unaffected. + `materialize_aligned` takes `&BastDoc` (owned, no lifetime). +- **`Arc` vs `String` (deferred decision 1):** default to + `String`. The typed tree is built once; name sharing via `Arc` + is a micro-optimization not justified without a bench. If phase 4's + `LayoutBuilder` work shows name allocation is measurable, revisit. + +**Verification:** `cargo test --release` — the existing `bast.rs` tests +are the primary validation (they exercise every parser path). All +`Bast*`-consuming tests must pass unchanged (they go through public +APIs that still take `&Value`/`&str` in, just return owned types out). +`cargo clippy --all-targets -- -D warnings`. `cargo doc --no-deps`. +`cargo build --target wasm32-unknown-unknown --release` (`bast.rs` is +wasm-relevant). + +--- + +## Phase 4 — `LayoutBuilder` caches the owned `BastDoc` (ADR-012 §2a) + +**Goal:** `LayoutBuilder::new` parses the owned `BastDoc` once and +stores it; `build` reuses it. Removes the `layout_builder.rs:190` +re-parse (M1). Non-breaking from the public API perspective +(`new`/`build` signatures unchanged); the change is internal. + +**ADR reference:** [ADR-012 §2a](../architecture/decisions/012-plan-fingerprinting-and-m1-closure.md#2a-layoutbuilder--cache-the-parsed-bastdoc-at-new). + +**Files:** `src/layout_builder.rs`. + +**Implementation notes:** +- `LayoutBuilder` currently stores `doc_value: Value` + `root_name: + String` + `endian: Endian`. After phase 3, it stores `doc: BastDoc` + (owned) + `endian: Endian`. `new` calls `BastDoc::new` once; + `build(&self, var_sizes)` uses `&self.doc` directly — no + `BastDoc::new` call inside `build`. +- The `BuildCtx<'d>` struct (currently `doc: &'d BastDoc<'d>`) becomes + `doc: &BastDoc` (no lifetime, or a single lifetime for the borrow + from `&self`). The walk logic is unchanged. +- The `doc_value: Value` clone is removed; the builder holds the owned + `BastDoc` directly. `endian` is read from the doc at `new` time + (already is). + +**Verification:** `cargo test --release` — the existing +`layout_builder.rs` tests pass unchanged (they go through +`LayoutBuilder::new` + `build`). `cargo clippy --all-targets -- -D +warnings`. `cargo build --target wasm32-unknown-unknown --release`. + +--- + +## Phase 5 — `OffsetMap` carries `LeafMeta` (ADR-012 §2b) + +**Goal:** Extend `OffsetMap`'s entries with `LeafMeta { kind, encoding, +endian }` computed at `compute` time. `read_field`/`write_field` drop +the `BastDoc::new` + `lookup_leaf_field` calls (M1 aligned-side). +Closes the last two M1 sites (`engine.rs:334,467`). + +**ADR reference:** [ADR-012 §2b](../architecture/decisions/012-plan-fingerprinting-and-m1-closure.md#2b-offsetmap--carry-leaf-metadata). + +**Files:** `src/offset_map.rs` (extend entries, compute `LeafMeta`), +`src/engine.rs` (rewrite `read_field`/`write_field` to use the map's +`LeafMeta`, remove `lookup_leaf_field` + `LeafFieldInfo`), `src/lib.rs` +(re-export `LeafMeta`). + +**Implementation notes:** +- New public type `LeafMeta { kind: AlkTypeKind, encoding: + VariableEncoding, endian: Endian }`. `Copy + PartialEq + Eq + Hash` + (all fields are `Copy + Hash`). +- `OffsetMap` storage: `fields: Vec<(String, ByteRange, LeafMeta)>` + (was `Vec<(String, ByteRange)>`). The `compute` walk already resolves + each leaf's type; add the `LeafMeta` extraction at the point where + the leaf `ByteRange` is recorded. +- **`OffsetMap::get` return type (deferred decision 2):** change to + `get(field_path) -> Option<&OffsetEntry>` where `pub struct + OffsetEntry { range: ByteRange, meta: LeafMeta }`. Add + `OffsetEntry` to `lib.rs` re-exports. Callers that used + `map.get(path).unwrap().start` become + `map.get(path).unwrap().range.start`. Update `alktty`/`alkcall` call + sites (in-house). +- `engine.rs::read_field`/`write_field`: drop the + `BastDoc::new(&self.bast_doc, &self.root_name)?` + + `lookup_leaf_field(&doc, field_path)?` calls. Read `LeafMeta` + from `offset_map.get(field_path)?.meta`. The `kind`/`encoding`/ + `endian` match arms in `read_field`/`write_field` are unchanged + (they already dispatch on `AlkTypeKind`/`VariableEncoding`/`Endian`). +- Remove `LeafFieldInfo` and `lookup_leaf_field` from `engine.rs` + (subsumed by `LeafMeta` on the map). +- `materialize_aligned` also uses `OffsetMap` — it currently calls + `offset_map.get(&path)?.start` for leaf reads. Update those call + sites to `.range.start`. The materializer's `resolve_typeref` calls + for composite walks stay (composites aren't in the offset map as + leaves; they're walked recursively). The `BastDoc` argument to + `materialize_aligned` is now owned (phase 3) — no signature change + beyond the lifetime drop. + +**Verification:** `cargo test --release` — existing `offset_map.rs` +and `engine.rs` `read_field`/`write_field` tests pass (they go through +public APIs). `cargo clippy --all-targets -- -D warnings`. `cargo doc +--no-deps` (new public `LeafMeta`/`OffsetEntry`). `cargo build --target +wasm32-unknown-unknown --release`. + +--- + +## Phase 6 — Fingerprinting (ADR-012 §1, §3) + +**Goal:** Add `Hash + Eq` derives + `fingerprint() -> u64` to `ReadPlan` +and `OffsetMap`. Enables cross-run caching, `alkcall` schema handshake, +schema-version diagnostics. + +**ADR reference:** [ADR-012 §1](../architecture/decisions/012-plan-fingerprinting-and-m1-closure.md#1-fingerprinting--readplan-hash--eq-offsetmap-hash--eq), +[ADR-012 §3](../architecture/decisions/012-plan-fingerprinting-and-m1-closure.md#3-fingerprinting-offsetmap-bundled-with-2b). + +**Files:** `src/read_plan.rs` (derives + `fingerprint`), `src/offset_map.rs` +(derives + `fingerprint`), `src/lib.rs` (no new re-exports — `Hash`/`Eq` +are trait derives, `fingerprint` is an inherent method). + +**Implementation notes:** +- `ReadPlan` already uses `BTreeMap` for `by_name` (phase 1), so + `#[derive(Hash, Eq)]` works. Add it alongside the existing + `Debug, Clone, PartialEq`. Same for `FieldPlan`, `CompositePlan`, + `ReadKind`, `DiscriminatorPlan`, `VariantPlan`, `VariantKind`. +- `OffsetMap` already carries `LeafMeta` (phase 5), and `LeafMeta` is + `Copy + Hash + Eq`. Add `#[derive(Hash, Eq)]` to `OffsetMap`, + `OffsetEntry`, `ByteRange` (already `Eq + Hash`), `LeafMeta`. +- **Fingerprint hasher (deferred decision 3):** `DefaultHasher` (std, + no new dep). The fingerprint isn't hot; cross-version stability is a + non-goal. `fingerprint()`: + ```rust + pub fn fingerprint(&self) -> u64 { + use std::hash::{Hash, Hasher}; + let mut h = std::hash::DefaultHasher::new(); + self.hash(&mut h); + h.finish() + } + ``` +- **Fingerprint contract test:** compile the same schema twice, assert + `plan1 == plan2` and `plan1.fingerprint() == plan2.fingerprint()`. + Compile a schema with one field changed, assert fingerprints differ. + This is the contract test for ADR-012 §1's "two plans with equal + hashes produce identical reads over identical bytes." + +**Verification:** `cargo test --release` (new contract tests). +`cargo clippy --all-targets -- -D warnings`. `cargo doc --no-deps`. + +--- + +## Phase 7 — Public API bump, docs, verification (ADR-011 step 6, ADR-012) + +**Goal:** Flip the version to 0.3.0, update `lib.rs` re-exports, update +the architecture docs (ADR-007 "Cost" rewrite, ADR-011/012 status flip +if not already, README ADR table), update in-house downstream +consumers, run the full verification block. + +**ADR reference:** [ADR-011 §Public API change](../architecture/decisions/011-compiled-read-plan-for-packed-mode.md#public-api-change-breaking--version-bump-to-030), +[ADR-012](../architecture/decisions/012-plan-fingerprinting-and-m1-closure.md). + +**Files:** `Cargo.toml` (version 0.2.0 → 0.3.0), `src/lib.rs` +(re-export `ReadPlan`, `LeafMeta`, `OffsetEntry`), `docs/architecture/` +(README ADR table, ADR-007 "Cost" section, ADR-011/012 status), +`docs/architecture/validation.md` / `layout-engine.md` (mention +`ReadPlan`/`LeafMeta` where relevant), in-house downstream repos +(`alktty`, `alkcall` — update call sites for `OffsetMap::get`, +`SequentialReader::new`, `materialize_packed`, `BastDoc` owned). + +**Implementation notes:** +- **ADR-007 "Cost" section (L2 from review #004):** rewrite the + "re-parse on demand" paragraph to describe the `Arc` cost + and the owned-`BastDoc` cache. The factory decision itself stays + "Accepted." This is the last loose end from review #004. +- **`src/engine.rs:112-115` doc comment (L2):** rewrite the "re-parse + the typed tree on demand" comment to describe the compiled-form + architecture (ReadPlan for packed reads, OffsetMap+LeafMeta for + aligned, owned BastDoc for the builder/validator). +- **`lib.rs` re-exports:** add `ReadPlan`, `LeafMeta`, `OffsetEntry`. + `BastDoc` and `Bast*` stay re-exported (signatures changed in phase 3, + names unchanged). `materialize_packed`/`materialize_aligned` stay + re-exported (signatures changed). `SequentialReader` stays + re-exported (`new` signature changed). +- **Downstream updates:** `alktty`'s bench (`benches/wire_vs_bast.rs`) + updates `SequentialReader::new` call + any `OffsetMap::get` usage. + `alkcall` updates similarly. Both are in-house path dev-deps; the + updates ride this release's commits (or follow-on commits in those + repos — they're separate repos, but the path dev-dep means a local + update is immediate). +- **`Cargo.toml` version bump:** `0.2.0` → `0.3.0`. The workspace + section added for the POC (`[workspace] members = ["poc/readplan"]`) + stays on the `readplan-poc` branch and is *not* merged to main — the + POC branch is derisking-only, like `bast-validator-poc`. If the POC + files ever merge to main, drop the workspace section (the POC is + disposable). + +**Verification block (run all, all must pass):** +```bash +cargo test --release # full suite +cargo clippy --all-targets -- -D warnings +cargo doc --no-deps # new public types +cargo build --target wasm32-unknown-unknown --release # wasm-clean +cargo publish --dry-run --allow-dirty # before publish +``` +Plus: **re-run the alktty `wire_vs_bast` bench** and record the +before/after numbers in the release commit message. The 400x gap +should close to within ~2–5x of hand-rolled (the `data_access` calls +are the same; the remaining gap is the `match` dispatch + `Arc` refcount +vs hand-rolled's direct calls). The SFTP-shaped union case (the one +ADR-011's framing argument cared about) should close further because +eager `$ref` resolution removes the `resolve_typeref_as_def` per- +variant dispatch cost. + +--- + +## Cross-phase invariants + +- **The tree builds and tests pass at every phase boundary.** No phase + leaves the crate in a non-compiling state. Phase 1 (add `ReadPlan`) + and phase 6 (add derives) are pure additions. Phases 2–5 are rewrites + that must leave tests green. +- **The POC on `readplan-poc` is the reference scaffold for phases 1–2.** + It is *not* merged to main; it stays on the branch as the derisking + record, like `bast-validator-poc`. If a phase 1–2 implementation + question arises about the plan shape, consult the POC. +- **Review #004 is the closure target.** H1 → phase 2; packed M1 → + phase 2; aligned M1 → phases 4–5; L1 → phase 2 (falls out); L2 → + phase 7 (doc rewrite). The review's status flips to "closed" in the + phase 7 commit. +- **No `unsafe`, no `async`, no new deps, no feature flags** (AGENTS.md + §5–§11). The owned-`BastDoc` refactor uses `String`/`Value`, not + `unsafe` self-referential tricks. `DefaultHasher` is std. Wasm-clean + throughout. +- **`preserve_order` stays load-bearing** (AGENTS.md §8). The owned- + `BastDoc` refactor must not sort schema object keys anywhere; field + order in the `Value` still determines byte order in packed mode and + iteration order in both modes. + +## What this plan is *not* + +- **Not a `ValidationPlan`.** Deferred per ADR-012 — different shape, + not a hot loop. +- **Not a disk-cache or wire-protocol spec.** The fingerprint contract + and method are in scope (phase 6); downstream uses are the consumers' + concern. +- **Not cross-version fingerprint stability.** Within-version only + (ADR-012). The fingerprint may change across versions if a new + `AlkTypeKind` variant is added; consumers cache within a version. +- **Not a perf bench.** The bench lives in alktty; this plan re-runs it + at phase 2 and phase 7 to confirm the gap closes. The plan itself + only asserts correctness/coverage. \ No newline at end of file