9ca9922fd46c64da21ff4c09b724edac6988b3d2
81
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
9ca9922fd4 |
fuzz: W3-1 — pin the indirect-reservation invariant; harness over-assertion fixed, reproducers committed
The running validate_pair campaign found the first wave-3 crash
(artifact crash-e40d...): the harness invariant 'validate_bytes Ok ⇒
every offset-map leaf's range.end ≤ buffer.len()' is WRONG for
offset-indirect entries. In aligned mode a maxLength reservation
contributes its full window to the layout (menu 2's total is 68), while
the {data_offset, data_length} pair is absolute — the data may live
anywhere in the buffer and the all-zero pair {0,0} over a 64-byte
buffer validates and reads fine. The wave-1 data_access bounds
partition is the real contract; the new invariant exempted the read
side but over-asserted the window. Harness-bug, not engine-bug.
- invariant now splits: non-indirect leaves keep the full window
assertion; offset-indirect leaves assert only the read_ok ⇒ pair
agreement (the pointed-to window sits inside the buffer)
- the exact artifact bytes pinned as a regression test
(validated_buffer_may_be_shorter_than_the_indirect_reservation_window)
and as corpus seed-044; the generator emits the same shape
deterministically (44→45 seeds)
- PairInput fields made pub for out-of-crate triage probes
Verification: corpus replay 30/30 green; clippy -D warnings clean.
|
||
|
|
aef8d9f6ab |
fuzz: wave 3 — validate_pair two-input harness, 44 seeds, release-budget campaigns next
Target 5 (§3): compile an attacker schema (10-lane menu incl. raw JSON bytes lane) in both modes, then hammer the hostile buffer through validate_bytes, an independent materialize_packed/materialize_aligned, read_field over every offset-map leaf, junk field paths, and the packed sequential walk under the spin bound. Invariants coded (per §3 target 5): mode agreement (aligned Ok ⇒ packed Ok; packed-Ok/aligned-Err only for the documented ADR-006/ADR-008/ offset-indirect rejections), the materialize⇄validate_bytes verdict lattice with verbatim error propagation, unknown-path echo, serde round-trip of materialized output, non-finite-float Access pinning, out-of-range enum Validation pinning, and the record-count spin bound. 44 committed seeds (menu/raw lanes × valid/valid, hostile-schema/ valid-bytes, valid-schema/hostile-bytes incl. a per-prefix truncation sweep, NaN/Inf, enum 99, spin fixtures, mode-agreement pins), hand- encoded against the pinned arbitrary 1.4.2 derive layout and pinned by decode tests. The aligned maxLength-reservation offset pin (s@8..72, tail@72, total 76) caught a fixture assumption error pre-commit. Verification: corpus replay 29/29 green (44 new seeds decode+replay), main crate 570 tests pass, clippy -D warnings clean (crate + shared), wasm clean, cargo fuzz build clean. Hand-run drives (indirect pair escape, enum-Validation, unknown discriminator, trailing garbage) all held. |
||
|
|
16b9023f60 |
fuzz: wave 2 — stateful read_opseq + layout_build targets, seeds, one engine fix
Targets 3-4 of docs/plans/fuzzing.md, per the sibling layout:
- fuzz/shared/src/read_opseq.rs — SequentialReader op sequences
(Next/NextBorrowed/Field/Reset/End, Arbitrary-derived) over hostile
buffers under the fixed packed schema menu. Invariants: cursor
discipline (failed read leaves position untouched, state replay
deterministic), None sticky at plan end, plan-order full walks with
a spin bound, read_field leaves a usable reader, ADR-007 reader
independence (shared Arc, isolated cursors), and the plan §6-1
record-count ≥4-verified-bytes bound encoded as an explicit End-op
assertion.
- fuzz/shared/src/layout_build.rs — LayoutBuilder::build with
adversarial var_sizes over a five-schema menu (string/bytes, nested
struct, byte-disc union, record+array, fixed control). Invariants:
Offset-class failures only, position disjointness + total-size
bounds, variable fields record their 4-byte prefix, failed writes
leave the buffer byte-identical, write→read pair round trip.
- derive_var_sizes discovers the synthetic keys ('p.__discriminator')
the builder actually wants by parsing the quoted key from the
Offset reason.
- 73 committed seeds (58 read_opseq + 15 layout_build) hand-encoded
against the pinned arbitrary 1.4.2 derive layout (4-byte LE
multiply-shift variant selectors, keep-going vec elements,
take-rest last field) and pinned by decode_lands_on_the_intended_variants
replay tests; gen_fuzz_seeds.py mirrors the encoders.
- Engine fix (finding W2-1): plan_read_array returned Ok for a
fixed-stride array whose count*stride window extended past the
buffer — the struct/union arms bounds-check, the array arm did not;
a truncated array deferred the failure to the next field (wrong
path) or masked it entirely as an Ok walk. Now an Access error
naming the array, regression test in sequential_reader.rs.
- Packed-mode 'encoding: offset-indirect' pinned as the documented
inline-length-prefix no-op (finding W2-2, bast-format.md Default
strategy selection); open design question recorded as plan §6-7.
Verification: fuzz corpus replay 19/19; main crate 570 tests incl.
the new regression; clippy -D warnings clean (crate + shared); wasm
build clean; cargo fuzz build clean (nightly confined to fuzz/).
Smoke campaigns (10 min detached each): read_opseq 52.1k execs exit 0
empty artifacts, layout_build 42.4k execs exit 0 empty artifacts; no
crash/oom/timeout on any fork job.
|
||
|
|
752e36b526 |
docs: fuzzing wave-1 status — campaigns clean, seeds count, dict fix
- bast_compile smoke: 543k execs, 10830 edges, coverage still growing at budget end; data_access smoke: 3.5M execs, saturated at 379 edges - both exited 0, empty artifact dirs, oom/timeout/crash 0/0/0 - json.dict: libFuzzer's parser rejects \u escapes and unquoted tails (caught at campaign launch, not by the fuzzer) - plan doc §3/§5/§6/§7 updated with wave-1 results |
||
|
|
ed41d77e72 |
fuzz: wave 1 — infra + bast_compile/data_access targets, seeds, corpus-replay gate
- fuzz/ workspace (nightly-pinned subtree, own [workspace]), copied
from the alkhttp/alkcall pattern: thin fuzz_target wrappers,
stable-toolchain shared crate holding the invariant logic, detached
runner, seed generator, json.dict, README
- bast_compile: AlkTypeEngine::compile both modes over attacker BAST
JSON; meta-schema gate ordering, always-Result, fixed-size leaf
metadata partition, json_schema lane
- data_access: the hand-rolled decode core over raw bytes at
attacker-chosen offsets; bool strictness, UTF-8 discipline, bounds
partitions, indirect {offset,length} pair contract, write-side
no-touch-on-failure + write/read round trips
- 136 committed seeds (38 + 98) via fuzz/gen_fuzz_seeds.py
- root Cargo.toml: explicit [workspace] exclude=[fuzz]; publish
exclude gains fuzz/
- AGENTS.md verification checklist gains the corpus-replay gate
- .gitignore: fuzz artifacts + grown-corpus pattern
Verification: cargo test 569 pass; clippy -D warnings clean; corpus
replay 4/4 green (136 seeds); cargo fuzz build clean (nightly
confined to fuzz/)
|
||
|
|
8d779e7672 | docs: fuzzing plan for alktype (5 targets, 3 waves, sibling pattern) | ||
|
|
9803d3b768 |
Pre-publish review #008: gate int_keys on canonical keys, restore no-prealloc array rule
Review #008 (docs/reviews/008-pre-publish-review.md) audits the two post-#007 unreviewed commits (v0.3.0 |
||
|
|
d4635d28f0 |
perf: fixed-size struct fast path, integer union dispatch, zero-alloc read_next_borrowed
Targets the bench gaps from the 0.3.0 port review (commit
|
||
|
|
dea96f0195 |
bench: port wire_vs_bast from alktty, add union + validate_bytes groups
The wire_vs_bast bench originated in alktty as the uncommitted curiosity probe that surfaced review #004's 400x read gap (the driver for the 0.3.0 compiled-forms release). It now lives here so alktype owns its perf story; the alktty-only async roundtrip group (tokio ChunkReader/ ChunkWriter over a duplex pipe) was dropped — that measures alktty's I/O stack, not this engine. The alktty copy is deleted. Groups: - read_chunk_stream / write_chunk_stream — the original ChunkHeader shape, byte-identical methodology, so numbers stay comparable with the historical series (400x → ~18x on read p64). - read_packet_stream (new) — SFTP-shaped byte-discriminator union (Read/Write variants, Write carries a length-prefixed bytes field): exercises CompositePlan::Union dispatch + variant walks + variable reads, the case ADR-011's framing argument was about. The alktype consumer pattern follows the documented FieldValue::Union contract; a pre-measurement parity check locks the pattern (variant walk size + disc size == packet size) so the stream loop can't drift silently. - validate_stream (new) — engine.validate_bytes per buffer (materialize + ValidationPlan walk), the read+validate-on-untrusted-stream shape alkcall cares about; closes the phase-7/8 bench deferral. - one-shots — engine_compile, sequential_reader_new, layout_build. criterion 0.7 dev-dep (default-features off). Benches don't affect the wasm gate (bench targets never compile under wasm32-unknown-unknown). Numbers (1024 chunks/iter, Xeon D-1521, shared box — ±10% noise): - read p64: hand 5.7 µs / alktype 104.8 µs (~18x; parity with the phase-2/8 record of 98-99 ns/chunk) - read p4k: hand 12.1 µs / alktype 107.5 µs - write p64: hand 14.0 µs / alktype 37.5 µs; p4k: 324/362 µs - packet read p64: hand 3.3 µs / alktype 622.6 µs (~189x — dominated by per-field String allocs + variant reader construction; the read_next (String, FieldValue) signature is pinned by the semver contract) - validate: header 458 ns/chunk, packet p64 2.23 µs, packet p4k 47 µs - one-shots: compile 615 µs (meta-schema dominated), reader_new 15.7 ns, layout_build 343 ns Verification: cargo test --release (566 tests green), clippy --all-targets -D warnings, wasm32-unknown-unknown build clean. |
||
|
|
557a0d791e | Review #007: fix YAML frontmatter — unquoted colon in reviewer value broke parsing | ||
|
|
120c05cd60 | Review #007: expand brace shorthand in reviewed_artifacts frontmatter | ||
|
|
cb952c9bf3 | Review #007: record post-fix coverage (91.66% lines, +0.99) | ||
|
|
7e5e58aa1b |
Close review #007 coverage holes C1-C3, L1, L2 (locking tests)
- C1: packed validate_bytes now exercised over all eleven primitive kinds (LE battery + BE subset + corrupted-bool rejection) — the plan materializer's i16..bool arms had zero public-path executions. - C2: aligned validate_bytes over the default inline length-prefixed encoding (string + bytes; ADR-006 last-position rule honored). - C3: ReadPlan::compile cycle rejection through a union mapping entry (compile_variant's own cycle arm — field-level cycles were already covered; this shape reaches the variant path). Arm confirmed executed in the post-fix coverage run. - L1: builder.rs standard JSON-Schema conveniences locked with exact- JSON table tests, plus an end-to-end build_validator compile test. - L2: tunion::read_field_discriminator's enum arm (both endians) — the last untested arm of the documented kind set (N1 parity). docs/reviews/007-coverage-audit.md updated with per-finding resolution blocks. Verification: 488 lib + 78 integration tests green, clippy -D warnings clean, wasm32-unknown-unknown build green. |
||
|
|
844c199fb8 |
Fix F1/F2 from review #007: zero-progress guard parity + maxLength cap
- F1: materialize_plan_array (validate_bytes' packed path) now carries the zero-progress array guard the reader and legacy walker already had; validate_bytes no longer accepts an empty buffer against a stride-0 empty-struct-element array that SequentialReader rejects. Cross-consumer agreement test added (review #007 probe transcript). - F2: MAX_LENGTH = 2^26 cap on the maxLength annotation — the N2 dual-layer pattern (clean Schema parse error naming value+maximum, meta-schema "maximum": 67108864 so the published contract matches). Also closes the silent usize-overflow drop in parse_max_length. - docs/reviews/007-coverage-audit.md records the full audit: per-file numbers, all classifications, and the N3a dead-surface list deferred to the pre-release review. Verification: 477 lib + 78 integration tests green, clippy -D warnings clean, wasm32-unknown-unknown build green. |
||
|
|
bb28ba3006 |
Resolve N3: maxLength is string/bytes-only (review #006)
- Parse gate in BastField::parse: maxLength on any kind other than
string/bytes is a clean Schema error (records, arrays, inline
structs, refs, union shared fields all covered; the choke point
needs no ref-following since $defs entries are struct/union/enum)
- Meta-schema FieldDef: if kind in {string, bytes} else maxLength
forbidden — the published alk.dev/bast/v1 contract matches the
parser (N2 dual-layer pattern)
- M5's compute-side record maxLength arm became unreachable and was
deleted (offset-indirect arm stays); the two superseded M5
maxLength tests rewritten as the n3_* parse-rejection family
- ADR-006 remedy message tailored per kind: for records both
annotated remedies are dead ends, so the error text points at the
last-position fix only
- Docs aligned: bast-format.md (FieldDef meta-schema + FieldDef/
Variable-Length Encoding prose), layout-engine.md (Strategy 2 +
ADR-006 paragraph), schema-layer.md, ADR-003 §2/§3a amended,
builder .max_length() doc
- Review #006: N3 resolved (all findings now closed), M5 update
note, test-count bookkeeping note (in-session probes vs static
counts), status lines flipped to fully resolved
Verified: 547 tests green + 2 ignored doctests in BOTH release and
default profiles (a stale debug artifact from an earlier session
masked one H3 roundtrip test in debug; clean rebuild passes both),
clippy -D warnings clean, cargo doc --no-deps zero warnings, wasm
build green.
|
||
|
|
0857ea1c23 |
Review #006: record L4/N1/M4 closure; add M5/M6 (fixed) and N3 (open)
- Resolution blocks on L4 (BTreeMap index + first-occurrence-wins pinned), N1 (tunion extended), M4 items 1-4 (aligned family, reader arms, data-access guards, dead-arm verdict with the structural note that the legacy packed walker serves only aligned fallbacks now). - New findings: M5 (aligned record maxLength/offset-indirect silent corruption, resolved same-day), M6 (legacy walker field-disc union skipped shared fields, resolved same-day), N3 (packed record maxLength unenforced in validate_bytes, open - posture decision). - Stats, resolution log, recommended order, and notes updated. |
||
|
|
2eb086f400 |
Fix M6, extend M4 coverage over aligned/legacy-walk/reader paths (review #006)
- M6 (new finding, fixed): the legacy packed BAST-walker's field-disc
union arm materialized only the discriminator field and started the
variant immediately after it — silently reading the remaining shared
fields' bytes as variant data whenever the union had any (probe:
record<union> values produced {"handle": 5} where 5 was seq's value).
The arm now walks all shared fields in order and starts the variant
after the whole shared walk, matching H3's convention and the plan
materializer's object shape (__discriminator + typed disc value +
shared + variant). Reachable via aligned record/leaf paths only.
- M4 item 1: aligned-materializer test family — nested-struct
recursion (3-level, previously 0 executions), maxLength trim in
nested structs, invalid-UTF-8 Access error, offset-indirect
out-of-bounds + data-after-sibling roundtrip, and records with
struct/array/byte-disc-union/field-disc-union/wide-primitive values
driving the legacy walker's previously-dead arms.
- M4 item 2: reader coverage — field-disc uint16/uint32/enum arms,
byte-disc uint16/uint32 arms, nested-union variant size walk, and
the public schema()/plan() accessors (all previously 0-execution).
- M4 item 3: data_access indirect-write tests at nonzero pair offset +
data-region bounds refusal (the u32-truncation guards themselves
need >4GiB slices and stay documented as defensively unreachable
on 64-bit).
- M4 item 4 follow-through: OQ-001 rejection for struct elements and
endian propagation for fixed elements locked with offset-map tests;
the dead composite arms were removed in the previous commit.
Coverage after: materialize.rs 64.48→85.72% lines, TOTAL 89.59→90.60%.
Verified: 567 tests green (463 crate + 17 + 34 + 15 + 12 + 2 ignored),
clippy -D warnings clean, wasm build green, cargo doc zero warnings.
|
||
|
|
5f9793f9c0 |
Resolve L4/N1, fix M5, close M4 item 4 (review #006)
- L4: BTreeMap path->index for OffsetMap::get and PackedLayout::get; the linear scans behind the "random access" doc claim are gone. First-occurrence-wins preserved (BastStruct::parse doesn't reject duplicate names); locking tests in both modules. - N1: tunion::read_field_discriminator now accepts uint16/uint32 disc fields (matching the reader's plan_discriminator_string_value set); one answer to "which field kinds can discriminate a union". - M5 (new finding, fixed): aligned Record fields accepted maxLength / offset-indirect annotations, but the materializer always walks the inline count-prefixed form from the entry start — probe-verified silent corruption (record data crossed the reservation into the next field's bytes; validate_bytes accepted the corrupt buffer). Both annotations now rejected at compute with clean Offset errors. Parity-preserved from 0.2.0. - M4 item 4: field_endian_for_element's Struct/Union arms and element_alignment were dead — the OQ-001 gate rejects every non-fixed-size element kind before either runs, so the phase-5 "element's own endian is consulted" divergence never existed on any reachable path. Dead arms deleted; OQ-001 rejection for struct elements and endian propagation for fixed elements locked with tests. Verified: 546 tests green (446 crate + 17 + 34 + 15 + 12 + 2 ignored), clippy -D warnings clean, wasm build green. |
||
|
|
0bc5a541ac |
Resolve N2: bound align annotations at 4096 (review #006)
align: 2^62 compiled and reported total_size = 2^63 — meaningless layout output the consumer may act on, and the reachable path to the MAX_ARRAY_BYTES cap used exactly this knob. - MAX_ALIGN = 4096 (page granularity) in schema.rs, documented with the probe arithmetic - parse_align returns Result and rejects over-cap values with a clean Schema error naming path/value/maximum — a silent clamp was rejected (it would change layout semantics without telling the consumer); both call sites thread the path, so standalone BastDoc::new (which never runs the meta-schema) is covered - Meta-schema: "maximum": 4096 on StructDef.align and FieldDef.align — the published alk.dev/bast/v1/schema contract now matches the parser - H1's byte-cap test retuned to align 4096 x count 2^16 = 2^28 > 2^26 (the byte cap stays reachable under the new align cap) Tests: 3 new (struct align above cap, field align above cap, align at cap accepted). 511 tests green, clippy -D warnings clean, wasm32 build green, cargo doc zero warnings. |
||
|
|
8739d29550 |
Resolve L2/L3: real schema threaded through ReadPlan compile; dead locals dropped (review #006)
L2: compile() builds Arc<Value> once and threads &Arc<Value> down the compile walk; every real ReadPlan carries the document at construction — the Value::Null-placeholder-then-map-overwrite dance is gone. The one remaining Null in wrap_leaf is documented as correct-by-construction (anonymous synthetic wrapper, never escapes). L3: materialize_plan_field drops its plan parameter (taken solely to discard) and the field-disc union arm's disc_field/let _ pair is deleted — the order-walk + by-name capture is the materializer's correct design, as the finding's parity note described. 508 tests green, clippy -D warnings clean, wasm32 build green, cargo doc zero warnings. |
||
|
|
dcfe9d16ff |
Fix M1/M2/M3: ADR-006 record gap + aligned record coverage + dead Struct arm (review #006)
M1: field_variable_kind (offset_map) now matches Record — a non-final inline length-prefixed record field in aligned mode hits the ADR-006 rejection instead of computing silently corrupt offsets (probe-verified clobber in the review: counts prefix at 0, id at 4). M2: aligned record path locked with public-path tests — materialize_aligned roundtrip (record<uint16>, wire arithmetic asserted) and engine validate_bytes roundtrip + corrupted-buffer rejection (record<uint32>). Record-as-last-field is the only safe inline position post-M1. M3: read_field's unreachable Struct arm (no struct-path entry ever exists in an OffsetMap) replaced with a documented defensive Offset error; doc comment states struct paths have no entry and the Offset miss is the reachable composite failure. FieldValue::Struct (public API, constructed by the packed reader) untouched. Tests: 7 new (2 offset_map, 2 materialize, 1 engine M3 lock, plus the roundtrip pair). 508 tests green, clippy -D warnings clean, wasm32 build green, cargo doc zero warnings. |
||
|
|
5e74b991ac |
Fix H2: shared reference-graph guard for standalone walkers (review #006)
Cyclic or over-deep $ref graphs stack-overflowed the three standalone schema walkers (OffsetMap::compute, LayoutBuilder::new, materialize_aligned) — SIGABRT on probe, parity-preserved from 0.2.0. - New src/walk_guard.rs: check_ref_graph() — one bounded walk over the reachable reference graph (depth cap 128 matching the plan compilers, path-scoped cycle set; diamonds allowed, cycles and 201-def chains rejected with the plan compilers' error wording) - All three walkers run the guard at entry, before any recursion; materialize_aligned's is defense-in-depth (a cyclic doc can no longer produce an OffsetMap, but mismatched doc/map inputs must still fail cleanly) - Behavioral side effect, net-positive: the guard eagerly parses every reachable def, so an invalid non-root def now surfaces at LayoutBuilder::new instead of build() — four H3 tests updated to expect the same Schema error earlier - Test family: 12 new tests (walk_guard, offset_map, layout_builder, materialize) covering self/two-def/composite-carrier cycles, deep chains, and diamond non-rejection; no stack-overflow reproducers in-tree per the review's Methodology warning - Stale "walkers have no cycle guard" statements updated in validation.md, 030 plan, ADR-012, and the engine gate comment Verified: 501 tests green (423 + 17 + 34 + 15 + 12 + 2 ignored), clippy -D warnings clean, wasm32 build green, cargo doc zero warnings. |
||
|
|
05a2a42983 |
Fix H3: field-disc union wire convention — shared-then-variant (review #006)
Decision (recorded as an ADR-011 addendum): the packed-mode wire layout for a field-name-discriminator TUnion is shared-then-variant — the union's declared `fields` (disc + shared fields) first, then the variant's own fields. Reader and materializer already implemented this; LayoutBuilder was corrected from variant-only layout. Enforcement in BastUnion::parse (the choke point every consumer inherits — union roots at BastDoc::new, referenced unions at resolve_ref): - discriminator field must be declared in `fields` - `fields` must not contain duplicate names - variants must not re-declare shared fields (checked inline and through $ref resolution — parse chain now threads the doc root) - the discriminator field must be the FIRST entry in `fields` (the reader reads the disc at the union start; a later position made it dispatch on the wrong bytes — H3 item 2, probe-verified) Schemas relying on the old variant-only builder convention (variants re-declaring shared fields) are rejected with a clean Schema error naming the convention. Breaking for 0.2.0-era re-declaring schemas; announced with 0.3.x. - L5: FieldValue::Union::variant_start doc now states per-kind semantics (byte-disc: union_start + disc.offset + disc.size; field-disc: after the shared walk). - L6: roundtrip test added (poc_roundtrip.rs) — LayoutBuilder write → SequentialReader read → materialize_packed → validate_bytes over a field-disc union with a second shared field and non-redeclaring variant; pins event.type@0/seq@1/handle@5, total 10. - ADR-011: Status-block addendum recording the convention decision, the no-re-declare rule, and the breaking-constraint note. - Review #006 updated: H3/L5/L6 resolution blocks, resolution log, recommended order. Verified: 488 tests green (410+17+34+15+12, 2 pre-existing ignored), clippy -D warnings clean, wasm32 build green, cargo doc zero warnings. |
||
|
|
2d166f567b |
Fix H1: bound untrusted array counts; resolve L1 (review #006)
- Replace the three Vec::with_capacity(count) sites in materialize.rs with Vec::new() — validate_bytes on an adversarial count no longer OOM-aborts the process (AGENTS.md §3). - New compile-time caps in schema.rs: MAX_ARRAY_ELEMENTS (2^16, enforced at BastArray::parse — the choke point every consumer inherits, bounds the walkers' per-element entry loops) and MAX_ARRAY_BYTES (2^26, enforced per walker against the mode-specific stride: compile_array, walk_array, compute_array_field). - L1: fixed_composite_size/fixed_plan_size now return Result<Option<usize>>; unwrap_or_default() gone, overflow is a clean Schema error instead of silent stride-0. - Zero-progress guard: stride-0 arrays whose elements consume 0 bytes (legal empty-struct elements) now error in plan_walk_variable_array_ size and materialize_array_packed instead of looping count times. - Tests: 8 new (parse/build/compile rejections, cap boundary, short-buffer clean error) + array_count_large_u64_parses_on_64bit rewritten to assert the new cap rejection. In-tree tests assert only the safe (compile-time) half per review #006's Methodology warning. - Review #006 updated: H1/L1 resolution blocks, new finding N2 (unbounded align annotations, found while re-deriving the cap arithmetic), resolution log, recommended order. Verified: 482 tests green (405+17+34+14+12, 2 pre-existing ignored), clippy -D warnings clean, wasm32-unknown-unknown build green. |
||
|
|
27be01af93 |
Add review #006: 0.3.0 post-implementation review
Audits the shipped 0.3.0 code (commits ff85258..9949f91) for correctness, untrusted-input discipline, 0.2.0 parity, code smells, and coverage. Gates re-run green in-session (474 tests, clippy -D warnings, doc, wasm32, publish --dry-run); coverage measured with cargo-llvm-cov (89.59% lines / 84.80% fn). Findings: - H1: huge declared array count OOM-aborts validate_bytes (three Vec::with_capacity(count) sites; release-blocking) - H2: cyclic $ref stack-overflows OffsetMap::compute / LayoutBuilder::new / materialize_aligned when driven standalone (engine gated, public walkers not; parity-preserved) - H3: field-disc unions — builder (variant-only layout), reader (disc at union start), and materializer (position-correct shared walk) disagree on layout and discriminator position; needs a convention decision - M1: ADR-006 check misses non-final inline Record fields (probe- confirmed) - M2: aligned-mode record fields untested end-to-end - M3: read_field's aligned Struct arm is unreachable dead code - M4: coverage weak spots (materialize.rs 64.5% lines; aligned nested-struct recursion 0 executions through any test) - L1-L6, N1: stride unwrap_or_default conflation, Null-schema placeholder, dead locals, linear-scan get, variant_start doc gap, missing field-disc roundtrip test, tunion/reader disc-kind split Includes an operational warning: the H1/H2 reproducers OOM/stack- abort the test harness — reproduce in an isolated process only. Verification: file-only change, no code touched; suite green before commit. |
||
|
|
9949f914df |
Release v0.3.0: compiled forms — ReadPlan, owned BastDoc, LeafMeta, ValidationPlan, fingerprinting
Public API bump 0.2.0 -> 0.3.0 (the 030-compiled-forms plan is now fully implemented; all eight phases landed). - Cargo.toml: version 0.3.0. lib.rs re-exports complete (ReadPlan + sub-types, LeafMeta, OffsetEntry, ValidationPlan + sub-types). - ADR-007 "Cost" rewritten to the Arc<ReadPlan> cost (15.7 ns) with the 0.2.0 "re-parse on demand" framing as a historical note (review #004 L2, the last loose end from that review). - ADR-011/012 status blocks flipped to implemented; architecture README ADR table rows updated; layout-engine.md rewritten for the 0.3.0 surface (engine-factory reader construction, OffsetMap OffsetEntry/LeafMeta/fingerprint section, owned BastDoc compute signature); SequentialReader module doc points at the engine factory. Reviews #004 and #005 flipped to closed. - Bench re-run (alktty wire_vs_bast, 0.3.0 tree): read p64 98 ns/chunk (parity with phase 2; hand-rolled 5.7 us/stream), layout_build 180 ns (was ~1.2 us — the phase-4 owned-doc cache removed the per-build re-parse, ~7x), sequential_reader_new 15.7 ns, write p64 -3%, engine_compile unchanged (meta-schema validation dominates). No dedicated validate_bytes-stream bench: the phase-7 spot check (~0.2 us plan-validate vs ~0.6 us compile-per-call) stands; a dedicated bench is a follow-up if alkcall profiling motivates it. - Downstream: alktty compiles against the path dep unchanged; alkcall has no dependency yet. Verification (full block, all green): 474 tests; clippy -D warnings clean; cargo doc zero warnings; wasm32 release build green; cargo publish --dry-run clean at 0.3.0. |
||
|
|
537a2170fb |
Fingerprint ReadPlan/OffsetMap: Hash + Eq + fingerprint() (ADR-012 §1/§4, plan phase 6)
- #[derive(Hash, Eq)] on ReadPlan, FieldPlan, CompositePlan, ReadKind, DiscriminatorPlan (schema: Arc<Value> hashes via serde_json Value Hash + Eq under preserve_order), and on OffsetMap (+ Clone; LeafMeta/OffsetEntry/ByteRange payload already Hash from phase 5 / this phase). - fingerprint() -> u64 on both via std DefaultHasher (deferred decision 3 resolved: no new dep, not hot, cross-version stability a non-goal per ADR-012). - Contract tests both sides: equal schemas -> equal PartialEq + fingerprint; field-kind / field-order / endianness changes each break equality and fingerprint; different root names over the same document fingerprint differently (ReadPlan). - ValidationPlan already carries its own Hash/Eq/fingerprint + contract test (phase 7 landed early). Verification: 474 tests pass (9 new fingerprint contract tests); clippy -D warnings clean; cargo doc zero warnings; wasm32 release build green. |
||
|
|
255c8c493e |
OffsetMap carries LeafMeta; read/write_field dispatch on it (ADR-012 §2b, plan phase 5)
Prerequisite (review #005 M2): Hash added to Endian/VariableEncoding derives (additive; fieldless Eq enums), and to ByteRange. - New public types LeafMeta { kind, encoding, endian } (Copy + Eq + Hash) and OffsetEntry { range, meta } (start()/end() accessors), re-exported from lib.rs. Deferred decision 2 resolved: struct — get(path) -> Option<&OffsetEntry>, iter() -> (&str, &OffsetEntry). Storage: Vec<(String, OffsetEntry)>. - LeafMeta computed at compute time with effective endian threaded through the aligned walk (container default -> field override, propagated into nested-struct probes and array elements via the referring field, matching the aligned materializer). - engine read_field/write_field dispatch on the entry's LeafMeta: the per-access BastDoc re-parse + lookup_leaf_field walk + LeafFieldInfo are gone — the last two review #004 M1 sites. - Parity note: lookup_leaf_field computed nested-struct defaults from the nested struct's own endian annotation; the map now agrees with the aligned materializer and packed ReadPlan (referring-field propagation). The old divergence (nested struct declaring endian under a field that also declares one) is closed; no test pinned it. - Behavior change: read_field on a map-absent path (whole-struct field) errors Offset ("field not found") instead of Access ("composite types"); the composite-path test accepted either. - materialize_aligned's four offset_map.get call sites updated to .range.start. alktty/alkcall untouched (bench never uses OffsetMap::get; alkcall has no dependency yet). Verification: 465 tests pass (offset_map tests updated to the OffsetEntry shape with per-kind LeafMeta expectations; engine test for the old lookup walk rewritten to assert map entries carry the LeafMeta); clippy -D warnings clean; cargo doc zero warnings; wasm32 release build green. |
||
|
|
b7c7dbe2a1 |
LayoutBuilder caches the owned BastDoc (ADR-012 §2a, plan phase 4)
The builder stores doc: BastDoc + endian (the doc_value: Value + root_name: String cache is gone); new parses the typed tree once and build walks &self.doc — the per-build BastDoc::new re-parse (layout_builder.rs M1) is retired. - build's root-is-struct re-check replaces its unreachable!() with a clean Schema error (AGENTS.md §3 never-panic; invariant unchanged — new already rejects non-struct roots). - Boxing fallout: the builder now holds the full owned tree, so Layout::Packed boxes it (Box<LayoutBuilder>) to keep the engine's Layout enum variant sizes balanced (clippy large_enum_variant). layout_builder() still returns Option<&LayoutBuilder> via auto-deref; public API unchanged. Verification: 465 tests pass unchanged (layout_builder.rs suites drive new/build through the public API); clippy -D warnings clean; wasm32 release build green. |
||
|
|
c583762352 |
Make BastDoc owned: drop Bast* lifetimes (ADR-012 §2a, plan phase 3)
Every Bast* type drops <'a>: &'a str -> String, &'a Value -> Value (deferred decision 1: plain String/Value — the tree is built once; Arc<str> name-sharing needs a bench justification that doesn't exist). BastDoc::new(&Value, &str) still takes references in and clones into owned storage; the doc gains Clone. resolve_ref/resolve_typeref/ resolve_typeref_as_def return owned types. - Engine ownership flip: AlkTypeEngine holds the owned BastDoc (replacing bast_doc: Value + root_name: String; root_name() delegates to the doc), killing its three per-call BastDoc::new re-parses (aligned validate_bytes, read_field, write_field — the review #004 M1 pattern removed by construction; phase 5 retires the lookup_leaf_field walk itself). New public accessor root_name() (additive). Engine Send + Sync with the owned doc, asserted in the existing thread-share test. - Bonus cleanup: materialize_typeref_packed's dead _field param dropped (phase 2 left it dangling). Under ownership, keeping it would force a deep Value clone per array element / record value / union variant via dummy_field_for. The param, dummy_field_for, and ty_source are gone; no behavior change (the arg was already ignored). BastField::synthetic keeps an owned-signature #[allow(dead_code)] definition (no remaining callers today). - Consumers adapted: OffsetMap::compute(&BastDoc), materialize_aligned(&BastDoc, ...) (no lifetime), BuildCtx/ ComputeCtx hold &'d BastDoc, tunion/discriminator name borrows, lib.rs module doc. LayoutBuilder's doc_value re-parse cache is unchanged pending phase 4. Verification: 465 tests pass with zero test-logic changes (bast.rs suites exercise every parser path through the public API); clippy -D warnings clean; cargo doc zero warnings; wasm32 release build green. |
||
|
|
1641dab505 |
Document session-continuity rules in AGENTS.md
opencode ends the turn when an assistant message contains no tool call, including analysis-only messages. Document the working fix (end bursts with a tool call or a final report, land work incrementally, resume without re-deriving) so every session inherits it. |
||
|
|
e5f1b9d825 |
Wire packed read path through ReadPlan (ADR-011 steps 2-4, plan phase 2)
SequentialReader now walks Arc<ReadPlan> instead of re-parsing the BAST typed tree per field (the 400x read-path gap, review #004 H1); materialize_packed walks the same plan, unifying the two packed read-side consumers on one compiled form. - SequentialReader::new(Arc<ReadPlan>) -> Self, infallible: the fallible BastDoc parse moved to ReadPlan::compile (phase 1). The reader holds the plan Arc + cursor state only; schema() returns the Arc<Value> retained on the plan (review #005 H2 — no self-referential struct); new plan() accessor exposes the shared plan. - ReadPlan carries schema: Arc<Value> (set at compile; sub-plans hold a Null placeholder — only the root plan is handed out). - materialize_packed(&ReadPlan, &[u8]): plan-walking packed materializer. The aligned path keeps walking BastDoc with the retained dummy_field_for/ty_source/materialize_typeref_packed helpers (phase 5 Scope Boundary: aligned structure walk is the permanent 0.3.0 design). - Engine: Layout::Packed stores Arc<ReadPlan> alongside the builder; sequential_reader() is an Arc::clone (was a full-document Value clone); packed validate_bytes calls materialize_packed(&self.plan). - Stride (deferred decision 4): FieldValue::Array now reports the true stride for fixed-size struct/nested-array elements (0.2.0 returned 0); doc comment documents the behavioral change; no existing test asserted the 0, so none needed changing. - Two parity subtleties found and preserved: (a) materialize_plan_composite unwraps the plan's anonymous single-field wrapper for primitive array elements/record values — without it, materialized records nest each leaf under a synthetic object (caught by the record parity test); (b) field-disc unions keep 0.2.0's materialized key order (__discriminator first), observable under preserve_order. Both are now covered by plan-phase tests or construction. Bench (alktty wire_vs_bast, 1024 chunks/stream): packed read 2.27 us/chunk (review #004) -> 98 ns/chunk p64 / 100 ns/chunk p4k (~23x; the 400x gap closes to ~17x vs hand-rolled 5.6 ns/chunk). Residual gap is the per-field String allocation mandated by the unchanged (String, FieldValue) read_next signature (2 allocs/chunk) plus data_access bounds checks. sequential_reader() construction: 15.7 ns (was a whole-document clone). Verification: 465 tests pass unchanged (the existing reader/ materialize/engine suites drive the rewrite through the public API — only constructor call sites moved to ReadPlan::compile); clippy -D warnings clean; cargo doc zero warnings; wasm32 release build green. |
||
|
|
ff85258d03 |
Implement ReadPlan type + compile (ADR-011 step 1, plan phase 1)
Pure addition: the packed read-side compiled form (src/read_plan.rs) and lib.rs wiring (module + re-exports of ReadPlan, FieldPlan, CompositePlan, ReadKind, DiscriminatorPlan). No existing engine code touched — phases 2-5 wire the plan into the reader/materializer/engine. - Refined union shape (ADR-011 as refined by review #005): CompositePlan::Union { disc, shared, variants } with shared: Option<Box<ReadPlan>> for field-disc unions and variants: Vec<(String, CompositePlan)> — no VariantPlan/VariantKind, nested-union variants work by ordinary CompositePlan recursion (restores the 0.2.0 capability the POC rejected). - by_name is BTreeMap (ADR-012 §1 Hash-derive prerequisite). - True array strides (deferred decision 4): fixed struct/nested-array elements compute their real stride via fixed_composite_size; variable-length elements stay 0. 0.2.0 returned 0 for fixed struct arrays; that behavioral change rides the 0.3.0 bump (phase 2 will surface it through SequentialReader). - Endianness: effective endian baked at every node. Parity lock: the plan propagates the referring field's effective endian into nested structs/unions — what the 0.2.0 packed reader/materializer actually do — and ignores nested containers' own endian annotations (the POC baked s.endian() there; latent divergence, never exercised by its equivalence tests). Nested-annotation tests lock this in. - Untrusted input: compile carries its own depth cap (128) + definition-level cycle set (mirrors ValidationPlan::compile), so standalone compile is safe on adversarial docs: cyclic refs, deep chains, dangling refs, non-struct roots, and non-struct/union variants all surface as AlkTypeError::Schema, never a panic. Overflow-safe stride arithmetic (checked_mul). Verification: 388 tests pass (355 existing + 33 new: every BastType arm coverage, field-disc shared/nested-union compile shape, stride computation, endian parity, cycle/depth/malformed rejection, Send + Sync static-bound assertion); clippy -D warnings clean; cargo doc zero warnings; wasm32-unknown-unknown release build green. Next: phase 2 (SequentialReader + materialize_packed consume the plan). |
||
|
|
e4636e6a44 |
Implement ValidationPlan (ADR-012 §3, plan phase 7)
The compiled value-domain validation form: replaces the interpretive BastDoc walk in validate_bytes with a compile-once-walk-many constraint tree built at engine-compile time. This was the design session + implementation ADR-012 §3 delegated; the shape decisions are recorded in new ADR-012 §3a. - New src/validation_plan.rs: ValidationPlan + ValidNode/ValidField/ ValidVariant (Debug+Clone+PartialEq+Eq+Hash+Send+Sync), compile(&BastDoc) with eager $ref resolution, and a per-buffer walk with deferred error-path rendering (zero happy-path allocation, byte-identical error messages vs the 0.2.0 walker). fingerprint() via DefaultHasher, same as the phase-6 pattern. - Compile-time graph safety: definition-level cycle set + depth cap (128) reject cyclic $ref graphs with AlkTypeError::Schema. The interpretive walker resolved refs lazily with no guard (stack- overflow hazard); diamond (shared) refs still compile. - bast_validation.rs: interpretive walker retired (deleted); validate_value survives as a one-shot wrapper (compile + validate) for callers holding a doc without an engine. - engine: Arc<ValidationPlan> built at compile in BOTH modes; the plan compile runs before the layout build and doubles as the engine's cyclic-ref gate (LayoutBuilder/OffsetMap struct recursion has no cycle guard; a cyclic doc previously overflowed there). validate_bytes walks the plan; new accessor validation_plan(). validate_bytes signature unchanged. - lib.rs: pub mod validation_plan + re-exports (ValidationPlan, ValidNode, ValidField, ValidVariant). Verification: cargo test --release (355 pass, incl. parity suite, fingerprint contract, cycle/depth rejection, Send+Sync + thread-share assertions); clippy --all-targets -D warnings clean; cargo doc zero warnings; wasm32-unknown-unknown release build green. Co-authored-by: opencode <noreply@alk.dev> |
||
|
|
e461f01c97 |
Resolve review #005: refine 0.3.0 plan + ADR-011/012
Resolve all 11 findings from the 0.3.0 plan review (#005) in one docs-only pass. No source changes; the crate still builds/tests at v0.2.0. The one substantive decision change is M3 (per user direction: ship ValidationPlan in 0.3.0, no more hedging); the rest are spec corrections or pre-implementation refinements to types that do not yet exist on main. - H1: refine ADR-011 CompositePlan::Union to carry shared: Option<Box<ReadPlan>> (field-disc shared fields) and variants: Vec<(String, CompositePlan)> (drop VariantPlan/ VariantKind). Plan phase 1 implements the refined shape. - H2: plan phase 2 specifies ReadPlan stores schema: Arc<Value> (not &Value), avoiding the self-referential struct ADR-011 rejects. Verified serde_json::Value: Hash + Eq holds with preserve_order, so phase 6 derives are not blocked. - M1: nested-union support falls out of the H1 shape refinement (a variant can be CompositePlan::Union) — option (a) from the review, no behavioral drop vs 0.2.0, no Semver regression row. - M2: plan phase 5 adds an explicit first sub-step to derive Hash on Endian and VariableEncoding in src/schema.rs (additive, semver-safe prerequisite the original plan omitted). - M3: reverse the ValidationPlan deferral. ADR-012's "Deferring ValidationPlan" becomes "ValidationPlan — in scope for 0.3.0"; new ADR-012 §3 commits the decision (compiled form, no per-buffer BastDoc walk, Hash + Eq + fingerprint()) and defers only the concrete shape to a follow-on design session + the plan's new phase 7. Plan gains phase 7 (ValidationPlan); old phase 7 (bump) renumbered to phase 8. ADR-011's Out-of-scope and Scope Boundaries bullets updated to point at ADR-012 §3. The deferral black hole this review's methodology flagged is closed: the work is committed with a concrete reactivation trigger, not hedged into an unplanned future. - L1: plan phase 2 corrects the dummy_field_for/ty_source removal claim — only packed-side call sites go away; the helpers stay for the aligned materialize_leaf_at path. - L2: plan phase 2 states the packed-vs-aligned materialize_typeref_packed split (packed gets a new plan-walking function; the existing function stays for aligned). - L3: plan phase 5 adds a Scope Boundary note — aligned materialize's BastDoc structure walk is the permanent 0.3.0 design; an AlignedPlan is out of scope, tracked as an OQ. - N1: fix "back-comat" -> "back-compat" typo. - N2: plan phase 1 verification adds the read_plan_is_send_sync static-bound assertion test ADR-011 requires. - N3: Semver Contract table notes the Result drop on SequentialReader::new (Result<Self, AlkTypeError> -> Self) alongside the argument-type change. Also: ADR-012 title -> "Plan Fingerprinting, ValidationPlan, and Closing the Deferred M1 Sites in 0.3.0"; §3 (Fingerprinting OffsetMap) renumbered to §4; README ADR table updated; review #005 gets a Resolution section recording how each finding was closed. Verification (docs-only change, v0.2.0 unchanged): cargo test --release ok (310 crate + 86 integration + 2 doctests) cargo clippy --all-targets -- -D warnings ok cargo doc --no-deps ok |
||
|
|
0e7921a02a |
Add review #005: 0.3.0 plan review
Cross-checks docs/plans/030-compiled-forms.md against the codebase, ADRs 011/012, the POC on readplan-poc, and review #004. Findings: - H1: field-disc union shape is in neither ADR-011 nor the POC - H2: schema() &Value on Arc<ReadPlan> is the self-referential pattern ADR-011 rejects - M1: nested-union silent behavioral drop (POC rejects what 0.2.0 accepts); deferral-black-hole pattern - M2: Endian/VariableEncoding missing Hash derive (phases 5/6 break) - M3: ValidationPlan deferral flagged for re-evaluation — the read+validate-on-untrusted-input case may be hotter than ADR-012's 'not a hot loop' dismissal accounts for - L1/L2/L3: dummy_field_for wording, materialize_packed split, materialize_aligned BastDoc walk silence - N1/N2/N3: typo, Send+Sync assertion test, Result drop on new Includes a deferral-pattern scan methodology section surfacing M1/L3/H2 as black-hole instances and confirming the plan's four explicit deferred decisions are the healthy pattern. Verification: file-only change, no code touched. |
||
|
|
2310f6cbd8 |
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).
|
||
|
|
1037e68091 |
Accept ADR-011: compiled read plan for packed mode
Tighten framing per pre-acceptance review (no decision changes): - Be precise about M1 coverage: closes the packed-side site (engine.rs:284 validate_bytes); the aligned-side M1 sites (engine.rs:334,467, layout_builder.rs:190) are a deliberate reversible bet, not a non-issue. - Replace the 'two type walkers is symmetric with OffsetMap/ PackedLayout' spin with an honest 'parallel typed tree, permanent maintenance tax, justified by ~20-50x composite-dispatch win on SFTP-shaped union-with-$ref-variants packets.' - Move the 'determinism enables fingerprinting/cache/handshake' future work out of the positives list into a dedicated 'Future capabilities' section — it is a forward reference, not a current win. - Clarify bast_doc: Value is retained unconditionally (unused in packed mode, still needed in aligned mode); add a Send+Sync test note for Arc<ReadPlan> shared from the Send+Sync engine. - Tighten the 'no format! allocations' claim to 'no resolve_typeref, no BastDef::parse, no JSON node access on the happy path' (error- path format! remains, and is not the cost being removed). - Add a 'POC coverage' section recording that the readplan-poc branch walked every BastType arm and confirmed ReadPlan covers all cases, including the union Byte/Field split and the array variable-stride (element_stride = 0) case. Status flipped Proposed -> Accepted. README ADR table updated. Verification: docs-only change; cargo test --release, cargo clippy --all-targets -- -D warnings, cargo doc --no-deps unchanged (no source touched). |
||
|
|
3184818c08 |
Propose ADR-011: compiled read plan for packed mode
Review #004 measured the packed read path at ~400x slower per chunk than a hand-rolled codec, root-caused to SequentialReader re-parsing BastDoc::new on every field read (the 're-parse on demand' framing from ADR-007). The write path is competitive because it has a compiled form (PackedLayout); the read path is the only mode/side pair without one. ADR-011 proposes ReadPlan — the packed read-side compiled form, symmetric to OffsetMap (aligned R/W) and PackedLayout (packed W). Packed positions are data-dependent (variable-length fields shift subsequent fields), so the compiled form is necessarily a read program (a pre-resolved tree of read instructions), not a flat lookup table like OffsetMap. ReadPlan::compile walks BastDoc once at engine construction, resolves all $ref\s eagerly, computes effective endianness at every node, and inlines union variants; the read loop then indexes into a Vec, matches on ReadKind, and calls data_access with a precomputed Endian — no BastDoc, no resolve_typeref, no JSON node access at read time. Scope: SequentialReader and materialize_packed consume the plan (one walker, not two); bast_validation, LayoutBuilder, and aligned one-shot paths stay on BastDoc (different concern, not hot loops). Includes a 7-step Recommended Order matching review #004's structure. Breaking public-API change (0.2.0 -> 0.3.0): SequentialReader::new and materialize_packed take ReadPlan; BastDoc and the Bast* types are unchanged (smaller breakage than review #004's Option A). Cross-links: ADR-007's 'Cost' section gets a Note pointing at review #004 and ADR-011, marking the 're-parse on demand' framing as the root cause slated for retirement (the factory decision itself is retained); the actual Cost/doc-comment rewrite happens in the implementation commit per ADR-011's recommended order. README ADR table updated. Verification: docs-only change, no source touched. Closes review #004 H1, M1 (packed side), L1, L2 (on implementation). |
||
|
|
51cb552715 |
Add review #004: read-path performance review
Traces the ~400x read-path gap (alktty wire_vs_bast bench) to
SequentialReader::read_field_at re-parsing BastDoc::new on every field
read, with the self-referential lifetime constraint as the root cause.
Findings:
- H1: per-field BastDoc::new re-parse in sequential_reader.rs:262
- M1: same re-parse in four one-shot paths (LayoutBuilder::build,
validate_bytes, engine read_field/write_field)
- L1: dead _field_schema param + Value clones in SequentialReader
- L2: ADR-007 Cost section + engine doc comment understate the re-parse
- N1: carry-forward of review #003 N2 (no new action)
Lays out fix options: Option A (owned typed tree, recommended, closes
H1+M1, breaking), Option B (read-plan precompute, fallback, H1 only,
non-breaking), Option C (borrow-from-engine, rejected, contradicts
ADR-007).
Verification: docs-only review; alktype source unchanged.
|
||
|
|
cab493206c |
Release v0.2.0: BAST pivot
Bump version to 0.2.0, exclude AGENTS.md from the published crate, and add a CHANGELOG.md covering the breaking BAST pivot (schema format, compile signature, validation split) plus bug fixes vs v0.1.0. Verification: - cargo test --release: all tests pass - cargo clippy --all-targets -- -D warnings: clean - cargo publish --dry-run --allow-dirty: packages as v0.2.0, no collision - AGENTS.md no longer in cargo package --list; CHANGELOG.md includedv0.2.0 |
||
|
|
82fec45bc6 |
Sync docs to 18 BAST kinds (Timestamp removal fallout)
- README: 19 -> 18 kinds, drop the timestamp row from the kinds table, drop 'timestamp shape' from the validate_bytes constraint list, remove the residual 'upcoming alkcall crate' sentence from the crate independence section (alkcall exists now), fix two '19 kinds' refs in the documentation pointer list and schema-layer link. - src/data_access.rs: module doc comment still said 'all 19 AlkType kinds' -> 18. - bast-format.md (normative): 19 -> 18 AlkTypeKind enum variants. - layout-engine.md: cross-reference to 'the 19 AlkType kinds' -> 18. - ADR-BAST (bast-bast-format.md): the Decision section claimed the post- pivot enum has '19 unchanged' variants; now 18, with the wording adjusted so it no longer says 'unchanged' across the pivot. - ADR-VAL-SPLIT: drop 'timestamp shape' from the value-domain constraint list and the validator-arm table row (validate_timestamp no longer exists). Left as historically accurate (describe the v0.1.0 pre-pivot state): ADR-003/004/006 Context mentions of AlkType:Timestamp, ADR-005 'engine now has 19', and the '19 jsonschema::Keyword factories' references in the What-is-removed sections of ADR-BAST and ADR-VAL-SPLIT. Verification: cargo test --release (407 pass), cargo clippy --all-targets -- -D warnings (clean), cargo doc --no-deps (clean), cargo build --target wasm32-unknown-unknown --release (clean). |
||
|
|
510553d800 |
Remove the Timestamp kind
The Timestamp kind was a residual from an early research reference. It was byte-identical to String everywhere (length-prefixed UTF-8) and its only distinguishing behavior was a hand-rolled non-strict RFC 3339 check that the docs admitted was incomplete (Feb 31 passes, seconds range unchecked, no leap seconds). JSON-level timestamp validation is jsonschema's job (format: date-time on the validate_json path), not alktype's. Removes the AlkTypeKind::Timestamp variant, its to_bast_str/from_bast_str mapping, the builder's Schema::timestamp() constructor, the validate_timestamp/is_rfc3339_timestamp validator arms, and the materializer/reader/engine timestamp arms. Updates the meta-schema primitive enum (14 -> 13), the spec docs (bast-format.md, schema-layer.md, builder.md, validation.md, overview.md, data-access.md, README.md), and the kind-count references (19 -> 18). Verification: cargo test --release (407 pass), cargo clippy --all-targets -- -D warnings (clean), cargo doc --no-deps (clean), cargo build --target wasm32-unknown-unknown --release (clean). |
||
|
|
62ed009281 |
Resolve review #003 nits N1, N3, N4
- N1: number_from_f64 now returns AlkTypeError::Access for non-finite floats instead of silently materializing Value::Null. A NaN/Inf in the buffer surfaces as a clear access error rather than a misleading 'expected a number' validation error. - N3: drop the dead Value::String arm from check_bytes; the materializer only ever emits bytes as an array of u8. Update the validation-model docs to match. - N4: fix stale ADR references in doc comments (ADR-096 -> ADR-002, ADR-097 -> ADR-003, ADR-098 -> ADR-004, ADR-101 -> ADR-007). N2 (BastType::alk_kind returning Struct for ) left as documented; every current caller resolves the ref first, so forcing Option through the call sites is churn without benefit. Verification: cargo test --release (411 pass), cargo clippy --all-targets -- -D warnings (clean), cargo doc --no-deps (clean). |
||
|
|
230345a867 |
Validate BAST documents against the meta-schema at compile time
Resolves review #003 finding M3. - Add bast_meta::validate_bast_doc and call it from AlkTypeEngine::compile before parsing. Malformed annotations (unknown endian/encoding strings, non-integer align/maxLength, missing required properties) now surface as AlkTypeError::Schema instead of being silently tolerated by the parser. - Align the meta-schema TypeRef with the parser: allow inline struct/union/ enum as TypeRefs (the parser and builder already accepted them; the meta-schema and spec did not). Update bast-format.md TypeRef table and the BastType doc comment to the seven-form vocabulary. Verification: cargo test --release (410 pass), cargo clippy --all-targets -- -D warnings (clean), cargo doc --no-deps (clean), cargo build --target wasm32-unknown-unknown --release (clean). |
||
|
|
e5c7cc1ca2 |
Fix offset-indirect, field-level endian, and aligned materialization
Resolves review #003 findings M1, M2, L1, and L2. - offset-indirect (L1 + M2 arm): rework read_*_indirect to same-buffer absolute offsets (safetensors-style, per the metatensor model) and add write_*_indirect. Wire the encoding dispatch into engine.read_field/ write_field and the aligned materializer. Previously the encoding was laid out but never read back. - field-level endian override (M1): thread field.effective_endian through sequential_reader, engine.read_field/write_field, and materialize_struct_aligned. Previously a per-field endian override was silently ignored, misreading multi-byte values. - aligned-mode materialization (M2): materialize fixed-size arrays via their vals[i] offset-map entries and maxLength reservations as zero-padded fixed-size slices (trailing NULs trimmed). Previously both read garbage or errored. - dead endian param (L2): drop the ignored endian argument from materialize_packed/materialize_aligned; endianness is read from the root struct. Verification: cargo test --release (404 pass), cargo clippy --all-targets -- -D warnings (clean), cargo build --target wasm32-unknown-unknown --release (clean), cargo llvm-cov --release (89.60% lines). |
||
|
|
ec73440c19 |
Add post-BAST-pivot code review (#003)
Covers the whole crate after the BAST pivot: correctness, code smell, panic safety, and coverage (cargo-llvm-cov). 3 Medium findings (field- level endian override ignored, aligned-mode validate_bytes broken for arrays/maxLength/offset-indirect, meta-schema never applied at compile time), 2 Low (offset-indirect dead code, dead endian param), 4 Nits. Timestamp removal recorded as a publisher decision, tracked separately. Verification: cargo test --release (389 pass), cargo clippy --all-targets -- -D warnings (clean), cargo llvm-cov --release (90.14% lines / 86.68% functions). |
||
|
|
562284faf4 |
Rewrite README for BAST pivot
The README still described the v0.1.0 custom-keyword JSON Schema format (`AlkType:*` kinds, `compile(&mut schema, mode)`, single jsonschema validator). Rewrite it for the BAST-era shipped API: - What-it-is table: two validation specs (bytes via BAST-native, JSON via consumer-provided JSON Schema) instead of one. - Usage example: `Definitions::new().build_doc(ChunkHeader, ...)` + `AlkTypeEngine::compile(&doc, ChunkHeader, LayoutMode::Packed, None)`; added a json!-literal variant showing the BAST document shape. - The 19 kinds table: lowercase BAST `kind` strings instead of `AlkType:*` keywords; note the enum index bounds fix. - Two layout modes: updated `compile` signature. - Variable-length handling: BAST field-level `encoding` annotation. - Union discriminators: `kind: union`, lazy variant $ref resolution, D-BAST-005 fields requirement. - Endianness: struct-level (was 'top-level schema'). - Validation: two validators (ADR-VAL-SPLIT), BAST-native for bytes, standard jsonschema for JSON; uniform AlkTypeError::Validation payload (D-BAST-009); enum bounds fix noted. - New 'BAST document shape' section: $defs required, root_name parameter, $ref restricted to #/$defs/<name>, BAST_META_SCHEMA re-export. - Untrusted input: BAST document (was 'schema'); BAST parser preserves the no-panic invariant. - Documentation index: updated for the new/rewritten docs and the ADR-BAST / ADR-VAL-SPLIT additions. Verified both code examples compile and pass against the shipped API (via a throwaway integration test, since removed). |
||
|
|
62270b03ca |
Sync architecture docs and ADRs to BAST pivot (steps 9-10)
Step 9 (convert tests to BAST format) was a no-op: steps 4-8 converted
the tests as they went. The only remaining reference in
src/tests was the intentional rejection test at
src/schema.rs:462 (asserting the old keyword form is rejected). Full
suite passes: 389 tests (312 lib + 77 integration).
Step 10 (sync architecture docs and ADRs):
Descriptive docs rewritten/updated for BAST:
- schema-layer.md: rewritten for the BAST parser (BastDoc/BastDef/
BastType typed tree, AlkTypeKind enum with to_bast_str/from_bast_str,
what was removed). Points at bast-format.md for the normative format.
- validation.md: rewritten for the two-validator model
(bast_validation for validate_bytes, standard jsonschema for
validate_json). Documents the repurposed build_validator, the
AlkTypeError::Validation uniform payload (D-BAST-009), and what is
removed.
- builder.md: updated all output examples to BAST JSON
(struct_() -> { kind: struct, fields: [...] }; object() -> standard
JSON Schema). Documents build_doc, count(), and the field-name union
fields requirement (D-BAST-005).
- overview.md: updated for BAST (what/why, schema-is-the-format table,
dependencies, architecture pointers, design decisions table).
- README.md (architecture index): updated document table, ADR table
(new ADR-BAST + ADR-VAL-SPLIT, superseded ADR-001), OQ table
(OQ-007/OQ-008 resolutions updated for BAST-native validator), and
key design principles (#1, #2, #7, #10 reworded for BAST).
- data-access.md: updated tunion function signatures to BastUnion and
the variant resolution to return BastType (resolve_typeref for refs).
- layout-engine.md: updated construct signatures
(LayoutBuilder::new(bast_doc, root_name), OffsetMap::compute(&doc),
SequentialReader::new(bast_doc, root_name)), the recursive-walk
description (BAST typed tree), and composite-kind headings
(TStruct/TUnion/TArray -> struct/union/array). Added D-BAST-004
note on array count requirement.
New ADRs:
- ADR-BAST (bast-bast-format.md): the BAST format, meta-schema,
//kind vocabulary, design principles, what is removed, the
enum index bounds bug fix. Supersedes ADR-001's format-specific
content; records D-BAST-001..009.
- ADR-VAL-SPLIT (val-split-two-validator-model.md): the two-validator
model (BAST-native for validate_bytes, standard jsonschema for
validate_json), the repurposed build_validator, the uniform
AlkTypeError::Validation payload. Refines ADR-004's validation
strategy and ADR-010's validation step; records D-BAST-006/007/009.
Amended ADRs (supersession/amendment notes added; original decision
text preserved as historical record):
- ADR-001: format-specific content superseded by ADR-BAST;
purpose/scope and schema-is-the-format principle retained.
- ADR-002: unchanged under the pivot; one-line note that the input
format changed but the modes didn't.
- ADR-003: annotation semantics retained; annotation location moved
to BAST type-level properties (amended by ADR-BAST).
- ADR-004: AlkTypeError enum retained (D-BAST-009); validation
strategy section refined by ADR-VAL-SPLIT.
- ADR-009: builder API surface retained; build() output format
amended to BAST / standard JSON Schema by ADR-BAST (D-BAST-008).
- ADR-010: validate_bytes two-step concept retained; validation step
amended to the BAST-native validator by ADR-VAL-SPLIT.
Other:
- Cargo.toml description: JSON Schema with AlkType:* custom keywords
-> BAST document.
- bast-pivot.md research record: status draft -> implemented, with a
pointer to the ADRs that superseded its decisions.
- bast-implementation.md plan: status draft -> complete, with a note
that step 9 was a no-op and step 10 is this commit.
- open-questions.md: OQ-006/OQ-007/OQ-008 resolutions updated for the
BAST-native validator.
- questions/008-unionvalidator-variant-dispatch.md: added a
post-BAST-pivot note pointing to the current bast_validation
implementation; v0.1.0 resolution text preserved as historical
record.
Verification:
- cargo test --release: 389 pass (312 lib + 77 integration)
- cargo clippy --all-targets -- -D warnings: clean
- cargo doc --no-deps: clean
- cross-reference check: every relative link in the new/updated docs
resolves (verified by script).
|
||
|
|
54fd112fde |
Remove v0.1.0 custom-keyword machinery (step 8)
The BAST parser (step 3) and BAST-native validator (step 5) replaced the v0.1.0 custom-keyword accessor layer; step 7 moved the builder to BAST output. This step removes the now-dead code: Removed from src/schema.rs: - get_alktype_kind / get_alktype_kind_enum / get_alktype_kind_loose / get_alktype_kind_loose_enum (replaced by the BAST parser's kind dispatch) - normalize_refs / inline_union_variant_refs + helpers (BAST refs are always #/$defs/<name>; resolution is a single hash lookup, variant refs resolve lazily) - parse_encoding / parse_align / parse_max_length / parse_endian (bast.rs has its own BAST-property-form copies) - parse_discriminator + DiscriminatorKind (replaced by bast::BastDiscriminator; builder has its own Discriminator enum) - resolve_ref / resolve_ref_or_inline (replaced by BastDoc::lookup_def / resolve_typeref) - FromStr impl, as_str, Endian::from_schema, ALKTYPE_PREFIX, BYTE_DISCRIMINATOR_TYPES, and the associated unit tests Kept: AlkTypeKind enum + methods (type_size, natural_alignment, is_fixed_size, needs_endian, is_composite, is_variable_length, to_bast_str, from_bast_str), Display (now backed by to_bast_str), Endian, VariableEncoding, U32_SIZE, DISCRIMINATOR_PATH. src/lib.rs: dropped the 13 schema::* helper re-exports and DiscriminatorKind from the public surface; kept Endian, AlkTypeKind, VariableEncoding. Doc/comment updates: bast.rs, builder.rs, engine.rs, error.rs — removed references to the deleted functions and the AlkType:* keyword form. The jsonschema crate remains a dependency (validate_json path + BAST meta-schema validation); build_validator was already repurposed in step 6 (no custom keywords). Verification: - cargo test --release: 389 pass (312 lib + 77 integration) - cargo clippy --all-targets -- -D warnings: clean - cargo doc --no-deps: clean - cargo build --target wasm32-unknown-unknown --release: clean |