Record BAST validator POC result; track OQ-BAST-001 error-payload decision
Adds the POC Result section (POC on branch bast-validator-poc, commit
f371fe4 — 20/20 tests, full 416-test suite green, clippy/wasm/doc clean).
Hypothesis confirmed: a recursive walker over the BAST type tree fully
replaces the 19 custom keyword validators on the validate_bytes path,
recovers OQ-008 union variant dispatch, and fixes the enum-membership
dead constraint. The POC code is reference scaffolding on the branch
and is not merged to main — it is superseded by Phase 1 step 5.
Marks Phase 1 step 2 and the POC Scope section as done with pointers to
the result section.
Elevates the deferred AlkTypeError::Validation payload-shape question
to OQ-BAST-001: keep jsonschema::ValidationError<'static> (POC choice,
simplest, dependency stays) vs introduce Validation(String) (drops
jsonschema from the error type; semver-relevant public-API change).
Decision belongs to the production refactor.
Verification: doc-only change, no code touched.
This commit is contained in:
glm-5.2 committed
1 parent
19f8162f1a
commit
e77268c951
1 file changed
+200
-4
+200
-4
@@ -935,9 +935,16 @@ through JSON Schema trees do not.
|
||||
|
||||
1. Define the BAST meta-schema (the JSON Schema that validates BAST
|
||||
documents)
|
||||
2. **Run the BAST-native validator POC** — implement the validator,
|
||||
2. ~~**Run the BAST-native validator POC** — implement the validator,
|
||||
wire it into `validate_bytes`, confirm the existing test suite passes.
|
||||
This de-risks the validation model before the full pivot.
|
||||
This de-risks the validation model before the full pivot.~~
|
||||
**Done.** See [POC Result — BAST-native validator](#poc-result--bast-native-validator).
|
||||
The hypothesis is confirmed; the production refactor (step 5 below)
|
||||
is now a straightforward port. POC code lives on branch
|
||||
`bast-validator-poc` (commit `f371fe4`) as a self-contained
|
||||
`src/bast_poc.rs` reference — deliberately not merged to main (it is
|
||||
throwaway scaffolding that gets replaced by the production module in
|
||||
step 5).
|
||||
3. Implement BAST document parsing in the engine (replace custom keyword
|
||||
detection with `kind` field parsing)
|
||||
4. Update `AlkTypeEngine::compile()` to accept a BAST document + root
|
||||
@@ -974,12 +981,19 @@ engine's byte-identity is already proven (alknet-typedef-poc,
|
||||
alktype-builder-poc) and the layout code is unchanged, so there is
|
||||
nothing empirical to de-risk there.
|
||||
|
||||
One targeted POC **is** needed to de-risk the validation model. The
|
||||
risk is specific and falsifiable: can a BAST-native validator — a
|
||||
One targeted POC **was** needed to de-risk the validation model. The
|
||||
risk was specific and falsifiable: can a BAST-native validator — a
|
||||
recursive walker over the BAST type tree — fully replace the 19 custom
|
||||
keyword validators on the `validate_bytes` path, including the OQ-008
|
||||
union variant dispatch, without regression?
|
||||
|
||||
**The POC has been run and succeeded.** The remainder of this section
|
||||
records the original scope and success criteria (kept for the record);
|
||||
the outcome is in [POC Result — BAST-native validator](#poc-result--bast-native-validator).
|
||||
The POC code is on branch `bast-validator-poc` (commit `f371fe4`), not
|
||||
merged to main — it is reference scaffolding superseded by Phase 1
|
||||
step 5.
|
||||
|
||||
### POC: BAST-native validator for `validate_bytes`
|
||||
|
||||
**Hypothesis:** A recursive walker over the BAST type tree can enforce
|
||||
@@ -1140,6 +1154,38 @@ Future crates (alktty, tunnels, sftp, git) will use BAST for their
|
||||
binary formats. The codegen feature (future) will generate
|
||||
readers/writers from BAST documents for these crates.
|
||||
|
||||
### OQ-BAST-001: `AlkTypeError::Validation` payload shape (open)
|
||||
|
||||
**Status: open — to be decided at pivot time, not in the POC.**
|
||||
|
||||
`AlkTypeError::Validation` currently wraps
|
||||
`jsonschema::ValidationError<'static>`. Under the BAST pivot the
|
||||
`validate_bytes` path no longer uses `jsonschema` at all (confirmed by
|
||||
the [POC](#poc-result--bast-native-validator) — observation 1), so the
|
||||
error payload on that path is constructed via
|
||||
`jsonschema::ValidationError::custom` purely to keep the variant's type
|
||||
unchanged. There are two options:
|
||||
|
||||
1. **Keep `Validation(jsonschema::ValidationError<'static>)`.** Simplest —
|
||||
`ValidationError::custom` is public and `'static`, so the bytes path
|
||||
can construct it without a real `jsonschema` validator. Cost: the
|
||||
error type retains its `jsonschema` dependency even though the bytes
|
||||
path no longer drives it. `validate_json` still uses `jsonschema`, so
|
||||
the dependency isn't removable either way — but the error type
|
||||
carries `jsonschema` only for one of its two callers.
|
||||
2. **Introduce `Validation(String)` (or a small structured payload).**
|
||||
Drops the `jsonschema` type from the public error enum. This is a
|
||||
**semver-relevant public-API change** (the `Validation` variant's
|
||||
payload type changes), so per AGENTS.md it requires an explicit
|
||||
decision, not a drive-by. Benefit: the error type is
|
||||
`jsonschema`-free, which matters if a future `no_std`/minimal build
|
||||
wants to drop `jsonschema` from the bytes-only path (relates to
|
||||
OQ-002).
|
||||
|
||||
The POC deferred this — it used option 1 to keep the error type
|
||||
unchanged. The decision belongs to the production refactor (Phase 1
|
||||
step 5) and should be made before the validator module lands.
|
||||
|
||||
## Risks and Mitigations
|
||||
|
||||
| Risk | Mitigation |
|
||||
@@ -1165,3 +1211,153 @@ readers/writers from BAST documents for these crates.
|
||||
- `/workspace/@alkimiadev/typebox-rs/src/codegen/` — Rust/TypeScript codegen from schemas (reference architecture)
|
||||
- `/workspace/alknet-typedef-poc/tests/sftp_roundtrip_test.rs` — SFTP POC proving byte-identical output (to be replicated with BAST)
|
||||
- `/workspace/@alkdev/alkcall/src/channels/wire.rs` — hand-rolled chunk header (target for BAST replacement)
|
||||
|
||||
## POC Result — BAST-native validator
|
||||
|
||||
**Status: succeeded.** The POC is on branch `bast-validator-poc` in
|
||||
`src/bast_poc.rs` (20 tests, all passing; full crate suite — 416 tests —
|
||||
green; `cargo clippy --all-targets -- -D warnings` clean;
|
||||
`cargo build --target wasm32-unknown-unknown --release` clean).
|
||||
|
||||
The POC implements the `validate_bytes` validation model from
|
||||
[D-BAST-006](#d-bast-006-validate_bytes-validation-model) as a
|
||||
self-contained module that does **not** touch the production schema /
|
||||
materializer / validator paths. It reuses only `data_access` (read
|
||||
primitives), `AlkTypeError` (error type), and `Endian`. The BAST
|
||||
document parser, a packed-mode materializer, and the BAST-native
|
||||
validator are all implemented from scratch — that is the point: prove
|
||||
the model works end-to-end before refactoring the production code.
|
||||
|
||||
### What the POC proves
|
||||
|
||||
The hypothesis from the [POC section](#poc-bast-native-validator-for-validate_bytes)
|
||||
is confirmed: a recursive walker over the BAST type tree fully
|
||||
replaces the 19 custom keyword validators on the `validate_bytes` path,
|
||||
including the OQ-008 union variant dispatch, and fixes the
|
||||
enum-membership dead constraint — all without `jsonschema` custom
|
||||
keywords and without an external JSON Schema.
|
||||
|
||||
The hard cases that were the actual de-risking targets all pass:
|
||||
|
||||
- **Union byte-offset discriminator + `maxLength` inside a variant
|
||||
(OQ-008).** `union_byte_disc_max_length_inside_variant_enforced`
|
||||
materializes a union with two `$ref` variants, dispatches on a
|
||||
byte-offset `uint8` discriminator, and enforces `maxLength` on a
|
||||
`bytes` field inside the selected variant. The validator reads
|
||||
`__discriminator`, looks up the variant's BAST definition, and
|
||||
recurses — same behavior as the current `UnionValidator`'s
|
||||
per-variant sub-validators, but with no `jsonschema` involvement.
|
||||
- **Union field-name discriminator + `maxLength` inside a variant.**
|
||||
`union_field_disc_max_length_inside_variant_enforced` covers the
|
||||
typedef.ts-style discriminator (a length-prefixed string field
|
||||
selects the variant). Same recursion model.
|
||||
- **Enum index-bounds fix.** `enum_index_out_of_bounds_rejected`
|
||||
exercises the constraint that is **broken in the current engine**
|
||||
(the built-in `enum` keyword checks string membership; the
|
||||
materializer emits `Value::Number(index)`, which never matches — a
|
||||
dead constraint). The BAST-native validator checks the materialized
|
||||
index against the `values` array bounds (0..len-1), which is the
|
||||
correct validation for a binary enum encoded as an index. Net
|
||||
improvement, not a regression.
|
||||
- **Nested struct wrapping a union wrapping a struct.**
|
||||
`nested_struct_with_union_variant` confirms the recursion composes
|
||||
through multiple type layers.
|
||||
- **Arrays of fixed-size structs with `count`.**
|
||||
`array_of_structs_with_count` covers the `Vector3`-style array
|
||||
(D-BAST-004).
|
||||
- **Records (count-prefixed string-keyed maps).**
|
||||
`record_of_uint32` covers the `TRecord` shape.
|
||||
- **Untrusted schema input.** `malformed_document_produces_schema_error_not_panic`
|
||||
confirms a malformed BAST document surfaces as `AlkTypeError::Schema`,
|
||||
not a panic (AGENTS.md §3).
|
||||
- **Basic cases** (chunk header, int8/uint32 ranges, string/bytes
|
||||
`maxLength`, timestamp, bool, short buffer) all pass — if a couple of
|
||||
basic examples work, all of them do, since the validator is a flat
|
||||
per-kind dispatch with no per-kind special-casing beyond the range
|
||||
bounds.
|
||||
|
||||
### How the validator works
|
||||
|
||||
The validator is a single recursive function
|
||||
(`validate_typeref`) that dispatches on the BAST `kind`. Each arm
|
||||
checks the value-domain constraint for that kind and, for composites,
|
||||
recurses into the child type definitions. The materializer (also
|
||||
implemented in the POC) guarantees structural correctness — bounds,
|
||||
UTF-8, bool byte, discriminator lookup, all fields present — so the
|
||||
validator only enforces what the materializer cannot:
|
||||
|
||||
| Constraint | Validator arm |
|
||||
|------------|---------------|
|
||||
| Integer range (Int8..Uint64) | `validate_int` / `validate_uint` with `as_i64`/`as_u64` + range check |
|
||||
| Int64/Uint64 (full range) | `validate_int64` / `validate_uint64` (JSON precision caveat per ADR-005) |
|
||||
| Float finiteness (Float32/64) | `validate_float` with `as_f64().is_finite()` |
|
||||
| String `maxLength` (byte length) | `check_string` reads the field-level `maxLength` annotation |
|
||||
| Bytes `maxLength` (array length) | `check_bytes` accepts both `Value::String` and `Value::Array` forms |
|
||||
| RFC 3339 timestamp shape | `validate_timestamp` reuses the same non-strict check as the current engine |
|
||||
| Enum index bounds | `validate_enum` checks `idx < values.len()` — the dead-constraint fix |
|
||||
| Union variant dispatch | `validate_union` reads `__discriminator`, looks up the variant, recurses via `validate_typeref` |
|
||||
| Struct fields | `validate_struct` walks `fields`, requires each declared field present, recurses |
|
||||
| Array count | `validate_array` checks `arr.len() == count` and recurses per element |
|
||||
| Record values | `validate_record` recurses into each value's `values` type |
|
||||
| Boolean | `validate_bool` (materializer already rejects non-0/1 bytes) |
|
||||
|
||||
### Observations for the production implementation
|
||||
|
||||
1. **No `jsonschema` dependency for `validate_bytes`.** The validator
|
||||
only needs `serde_json` (for `Value`) and the BAST document. The
|
||||
`jsonschema` crate is still a direct dependency for `validate_json`
|
||||
and for validating BAST documents against the BAST meta-schema, but
|
||||
the `validate_bytes` path no longer touches it. This is a small wasm
|
||||
binary-size win in addition to the architecture simplification.
|
||||
|
||||
2. **`$ref` resolution is a single hash lookup.** The POC's
|
||||
`resolve_ref_or_inline` handles only `#/$defs/Name` pointers — the
|
||||
only form BAST allows. The current engine's `normalize_refs` /
|
||||
`inline_union_variant_refs` / `resolve_ref_or_inline` machinery for
|
||||
bare-name refs and inlined union variants is no longer needed: BAST
|
||||
`$ref`s are always full JSON Pointers, and union variant refs are
|
||||
resolved lazily by the validator (the materializer already does this
|
||||
for the read path). The `inline_union_variant_refs` compile step can
|
||||
be removed entirely.
|
||||
|
||||
3. **The validator is ~250 lines.** The 19 custom keyword validators
|
||||
(`src/validation.rs`) plus the macro definitions are ~500 lines and
|
||||
require the `jsonschema::Keyword` trait plumbing (factory closures,
|
||||
`Box<dyn Keyword>`, sub-validator construction at factory time). The
|
||||
BAST-native validator is a flat match — no factories, no trait
|
||||
objects, no sub-validator pre-computation. The recursion is direct.
|
||||
|
||||
4. **The `AlkTypeError::Validation` variant still wraps
|
||||
`jsonschema::ValidationError<'static>`.** The POC uses
|
||||
`jsonschema::ValidationError::custom` to construct these so the
|
||||
error type is unchanged. The production version could either keep
|
||||
this (simplest — `ValidationError::custom` is public and
|
||||
`'static`) or introduce a small `Validation(String)` shape to drop
|
||||
the `jsonschema` dependency from the error type. The latter is a
|
||||
public-API change (the `Validation` variant's payload type changes),
|
||||
so it's a semver-relevant decision to make at pivot time, not in the
|
||||
POC.
|
||||
|
||||
5. **The materializer and validator share the BAST-walking code
|
||||
structure.** Both walk the same `kind`/`fields`/`mapping` tree. The
|
||||
production refactor could share a typed BAST tree (a small
|
||||
`BastNode` enum) between them so the walk is parsed once. The POC
|
||||
parses lazily from the raw JSON in both passes to keep the model
|
||||
honest; a typed tree is a straightforward follow-on optimization, not
|
||||
a risk.
|
||||
|
||||
### Verdict
|
||||
|
||||
The "how do we reproduce the same behavior?" question is answered:
|
||||
walk the BAST tree the same way the materializer does, checking the
|
||||
same value-domain constraints the custom keyword validators check
|
||||
today. The model is a strict simplification — fewer moving parts, no
|
||||
`jsonschema` integration on the bytes path, no compile-time
|
||||
`inline_union_variant_refs` step, no factory closures or trait
|
||||
objects, and the enum dead-constraint is fixed as a side effect.
|
||||
|
||||
The POC does not wire into `AlkTypeEngine::validate_bytes` — that is
|
||||
the production refactor (Phase 1 step 5 in the Migration Path), which
|
||||
replaces `validation::build_validator` usage on the bytes path with
|
||||
the BAST-native validator. The POC's job was to de-risk the model
|
||||
before that refactor; that job is done.
|
||||
Reference in new issue
Block a user