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).
This commit is contained in:
1 parent
1037e68091
commit
2310f6cbd8
4 files changed
+919
-13
No files matched your search
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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<String, usize>`. `HashMap` iteration order is non-deterministic
|
||||
and `HashMap` does not implement `Hash`, which blocks `#[derive(Hash)]`
|
||||
on `ReadPlan`. Switch `by_name` to `BTreeMap<String, usize>`. 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<str>`, `&'a Value` →
|
||||
`Value`/`Arc<Value>`) 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.
|
||||
@@ -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<Self, AlkTypeError>`. 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<SequentialReader>` (unchanged type), but the reader is now constructed from `Arc<ReadPlan>`, 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<FieldValue, AlkTypeError>`. 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<str>` — decision in phase 3). Every `&'a Value` → `Value` (or `Arc<Value>`). 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<ReadPlan>)`. 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<str>` vs `String` for owned `BastDoc` names** (phase 3): `Arc<str>`
|
||||
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<str>` 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<Self, AlkTypeError>`. 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<String, usize>` (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<ReadPlan>` +
|
||||
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<ReadPlan>` 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<ReadPlan>)`. The reader stores
|
||||
`plan: Arc<ReadPlan>`, `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<ReadPlan>` 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<str>`/`Arc<Value>`
|
||||
— 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<usize>`,
|
||||
`fields: Vec<BastField>`, `source: Value`.
|
||||
- `BastField<'a>` → `BastField`: `name: String`, `ty: BastType`,
|
||||
`endian: Option<Endian>`, `align: Option<usize>`,
|
||||
`encoding: VariableEncoding`, `max_length: Option<usize>`,
|
||||
`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<BastField>`,
|
||||
`mapping: Vec<(String, BastType)>` (was `Vec<(&'a str, BastType)>`),
|
||||
`source: Value`.
|
||||
- `BastDiscriminator::Field { name: String }` (was `name: &'a str`).
|
||||
- `BastEnum<'a>` → `BastEnum`: `values: Vec<String>` (was
|
||||
`Vec<&'a str>`), `source: Value`.
|
||||
- `BastArray<'a>` → `BastArray`: `element: Box<BastType>`, `count: usize`,
|
||||
`source: Value`.
|
||||
- `BastRecord<'a>` → `BastRecord`: `values: Box<BastType>`,
|
||||
`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<str>` vs `String` (deferred decision 1):** default to
|
||||
`String`. The typed tree is built once; name sharing via `Arc<str>`
|
||||
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<ReadPlan>` 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.
|
||||
Reference in new issue
Block a user