diff --git a/docs/reviews/007-coverage-audit.md b/docs/reviews/007-coverage-audit.md index 208955a..b0b394f 100644 --- a/docs/reviews/007-coverage-audit.md +++ b/docs/reviews/007-coverage-audit.md @@ -69,6 +69,13 @@ clean. Full suite green (548 tests static + 2 ignored doctests, per resolution blocks on each finding. 477 lib tests green (511 static + 2 ignored doctests across all targets), clippy `-D warnings` clean, wasm build green. +- **C1 + C2 + C3 (2026-09-02):** resolved in one commit — see the + resolution blocks. 481 lib tests green, clippy `-D warnings` clean, + wasm build green; `compile_variant`'s cycle arm confirmed executed + in the post-fix coverage run. +- **L1 + L2 (2026-09-02):** resolved in one commit — see the + resolution blocks. 488 lib tests green, clippy `-D warnings` clean, + wasm build green. --- @@ -217,12 +224,11 @@ frames; one battery test closes it (mirror the aligned `read_field` battery, tests/engine_integration.rs:130-190, which already covers all twelve primitive kinds on the aligned side). -**Fix**: one `validate_bytes` test in packed mode with all twelve -primitive kinds (LE), plus a BE variant of a subset. Assert both -acceptance and a corrupted-value rejection. - -**Resolution (2026-09-02)**: fixed this session — see the resolution -block. +**Resolution (2026-09-02):** two tests in `engine.rs`: +`c1_validate_bytes_packed_decodes_all_twelve_primitives_le` (the full +eleven-field battery — i8..bool — plus a corrupted-bool rejection arm) +and `c1_validate_bytes_packed_decodes_big_endian_subset` (BE i16/u64/ +f64 through the same public path). Both green. ### C2. Aligned `validate_bytes` never exercises the default inline encoding for string/bytes @@ -242,8 +248,13 @@ public path. string (and a bytes variant or arm), asserting acceptance plus a short-buffer rejection. -**Resolution (2026-09-02)**: fixed this session — see the resolution -block. +**Resolution (2026-09-02):** +`c2_validate_bytes_aligned_inline_string_and_bytes_default_encoding` +in `engine.rs`. One wrinkle the test wrote itself into: ADR-006 +allows an inline length-prefixed variable field only in the final +position, so the string and bytes shapes get separate one-field +schemas (string after a fixed `id`; bytes as a lone field). Asserts +acceptance for both plus a short-buffer rejection for the string. ### C3. `ReadPlan::compile`'s union-variant cycle arm is untested standalone @@ -267,8 +278,15 @@ first (engine.rs:153). `ReadPlan::compile` directly on a two-def cycle, mirroring `validation_plan.rs`'s existing standalone cycle test. -**Resolution (2026-09-02)**: fixed this session — see the resolution -block. +**Resolution (2026-09-02):** +`c3_cycle_through_union_mapping_variant_is_schema_error` in +`read_plan.rs`. Writing the test sharpened the finding: the +*field-level* cycle arm (`compile_typeref`, :362) was already covered +(2 execs) by `cyclic_ref_through_two_defs_is_schema_error`; the +0-exec arm was `compile_variant`'s own check (:513), reachable only +when the cycle closes through a **union mapping entry**. The new +test's shape (`A → B → U(mapping: "1" → $ref B)`) trips exactly that +arm — verified post-fix at the line level (1 execution). ### L1. `builder.rs`'s JSON-Schema conveniences are entirely untested @@ -286,8 +304,19 @@ misplaced keyword would ship silently — the builder emits the JSON, table-style test asserting each convenience produces the expected JSON key/value closes the surface cheaply. -**Resolution (2026-09-02)**: fixed this session — see the resolution -block. +**Resolution (2026-09-02):** five tests in `builder.rs`: +`l1_standard_type_constructors_produce_type_keyword` (all eight +standard constructors, exact-JSON assertions), +`l1_field_on_standard_object_builds_properties` (`field()` on the +standard repr + `required()`), +`l1_items_and_additional_properties_on_standard_types`, +`l1_constraint_keywords_emit_expected_json_keys` (minimum/maximum/ +minLength/minItems/maxItems/format/title/description, each asserted +on its exact keyword), and +`l1_standard_built_schema_compiles_as_json_validator` (the end of the +translation chain: the emitted JSON builds a `jsonschema` validator +and the constraints actually bite — valid passes, over-maximum/ +missing-required/below-minimum fail). ### L2. `tunion::read_field_discriminator`'s enum arm is 0-exec @@ -300,8 +329,10 @@ enum arm, which is in the documented kind set The reader's enum arm is tested (`m4_field_disc_enum_dispatches_on_index`); tunion's is not. One test locks parity on the last arm. -**Resolution (2026-09-02)**: fixed this session — see the resolution -block. +**Resolution (2026-09-02):** `l2_read_field_discriminator_enum_ +dispatches_on_index` and `l2_read_field_discriminator_enum_big_endian` +in `tunion.rs` — enum index 0 (LE) and 1 (BE) dispatch with +`variant_offset == 4` / `discriminator_size == 4`. ### N1. `OffsetEntry::start()`/`end()` 0-execution in the combined run is a merge artifact, not a hole @@ -376,12 +407,12 @@ Leave uncovered; the pattern is consistent across the codebase. ## Recommended Order -1. **F1** — guard hoist + cross-consumer agreement test (small, - real-behavior fix). -2. **F2** — `MAX_LENGTH` cap, N2's dual-layer playbook verbatim. -3. **C1 + C2 + C3** — one locking test each, same session if - convenient. -4. **L1 + L2** — posture tests, cheap. +1. ~~**F1** — guard hoist + cross-consumer agreement test~~ + **resolved 2026-09-02** (with F2). +2. ~~**F2** — `MAX_LENGTH` cap, N2's dual-layer playbook verbatim~~ + **resolved 2026-09-02** (with F1). +3. ~~**C1 + C2 + C3** — one locking test each~~ **resolved 2026-09-02**. +4. ~~**L1 + L2** — posture tests~~ **resolved 2026-09-02**. 5. **N3a** — defer to the pre-release review (semver decision). ## Notes @@ -397,4 +428,6 @@ Leave uncovered; the pattern is consistent across the codebase. - The coverage holes fixed this session (C1-C3, L1, L2) were chosen because each is load-bearing *and* one-test-cheap; the remaining uncovered mass is dominated by N2a/N3a/N4a, which are documented - rather than forced. \ No newline at end of file + rather than forced. +- Static test counts at the review-#007 commits: 477 (F1/F2), + 481 (C1-C3), 488 (L1/L2) — +14 net from the pre-audit 474. \ No newline at end of file diff --git a/src/builder.rs b/src/builder.rs index 874229f..faeb205 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -1386,4 +1386,93 @@ mod tests { ); assert!(engine.is_ok(), "record should compile: {engine:?}"); } + + // ----- Standard JSON-Schema conveniences (review #007 L1) ----------- + + #[test] + fn l1_standard_type_constructors_produce_type_keyword() { + assert_eq!(Schema::object().build(), json!({ "type": "object" })); + assert_eq!(Schema::array().build(), json!({ "type": "array" })); + assert_eq!(Schema::string_().build(), json!({ "type": "string" })); + assert_eq!(Schema::integer().build(), json!({ "type": "integer" })); + assert_eq!(Schema::number().build(), json!({ "type": "number" })); + assert_eq!(Schema::boolean_().build(), json!({ "type": "boolean" })); + assert_eq!(Schema::null().build(), json!({ "type": "null" })); + assert_eq!(Schema::any().build(), json!({})); + } + + #[test] + fn l1_field_on_standard_object_builds_properties() { + let s = Schema::object() + .field("id", Schema::integer()) + .field("tag", Schema::string_()) + .required(&["id"]); + assert_eq!( + s.build(), + json!({ + "type": "object", + "properties": { + "id": { "type": "integer" }, + "tag": { "type": "string" } + }, + "required": ["id"] + }) + ); + } + + #[test] + fn l1_items_and_additional_properties_on_standard_types() { + let arr = Schema::array().items(Schema::integer()); + assert_eq!( + arr.build(), + json!({ "type": "array", "items": { "type": "integer" } }) + ); + let obj = Schema::object() + .additional_properties(Schema::boolean_()); + assert_eq!( + obj.build(), + json!({ "type": "object", "additionalProperties": { "type": "boolean" } }) + ); + } + + #[test] + fn l1_constraint_keywords_emit_expected_json_keys() { + // Each convenience setter must land on the exact JSON Schema + // keyword jsonschema interprets — a typo here would silently + // drop the constraint (nothing else checks the translation). + let s = Schema::integer() + .minimum(1.0) + .maximum(100.0); + assert_eq!( + s.build(), + json!({ "type": "integer", "minimum": 1.0, "maximum": 100.0 }) + ); + + let str_schema = Schema::string_().min_length(2).format("uri").title("T").description("D"); + assert_eq!( + str_schema.build(), + json!({ "type": "string", "minLength": 2, "format": "uri", "title": "T", "description": "D" }) + ); + + let arr_schema = Schema::array().min_items(1).max_items(10); + assert_eq!( + arr_schema.build(), + json!({ "type": "array", "minItems": 1, "maxItems": 10 }) + ); + } + + #[test] + fn l1_standard_built_schema_compiles_as_json_validator() { + // End of the translation chain: the emitted JSON is accepted by + // build_validator and the constraints actually bite. + let schema = Schema::object() + .field("id", Schema::integer().minimum(0.0).maximum(10.0)) + .required(&["id"]); + let validator = crate::validation::build_validator(&schema.build()) + .expect("standard schema compiles"); + assert!(validator.validate(&json!({ "id": 5 })).is_ok()); + assert!(validator.validate(&json!({ "id": 11 })).is_err()); + assert!(validator.validate(&json!({})).is_err()); + assert!(validator.validate(&json!({ "id": -1 })).is_err()); + } } \ No newline at end of file diff --git a/src/engine.rs b/src/engine.rs index 8995398..6a58ad2 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1292,6 +1292,85 @@ mod tests { assert!(engine.validate_bytes(&buf).is_ok(), "valid mixed struct should pass"); } + #[test] + fn c1_validate_bytes_packed_decodes_all_twelve_primitives_le() { + // The plan materializer's i16/i32/i64/u64/f64/bool arms had + // zero public-path executions before this battery (review #007 + // C1): every packed validate_bytes test fed u8/u32/string + // shapes. + let doc = json!({ + "$defs": { + "S": { + "kind": "struct", + "endian": "little", + "fields": [ + { "name": "i8", "kind": "int8" }, + { "name": "i16", "kind": "int16" }, + { "name": "i32", "kind": "int32" }, + { "name": "i64", "kind": "int64" }, + { "name": "u8", "kind": "uint8" }, + { "name": "u16", "kind": "uint16" }, + { "name": "u32", "kind": "uint32" }, + { "name": "u64", "kind": "uint64" }, + { "name": "f32", "kind": "float32" }, + { "name": "f64", "kind": "float64" }, + { "name": "b", "kind": "bool" } + ] + } + } + }); + let engine = AlkTypeEngine::compile(&doc, "S", LayoutMode::Packed, None).expect("compile"); + let mut buf = vec![0u8; 1 + 2 + 4 + 8 + 1 + 2 + 4 + 8 + 4 + 8 + 1]; + let mut off = 0; + buf[off] = 0x81; off += 1; // i8 = -127 + buf[off..off + 2].copy_from_slice(&(-32000i16).to_le_bytes()); off += 2; + buf[off..off + 4].copy_from_slice(&(-2_000_000_007i32).to_le_bytes()); off += 4; + buf[off..off + 8].copy_from_slice(&(-9_000_000_000_000_000_000i64).to_le_bytes()); off += 8; + buf[off] = 0xAB; off += 1; // u8 + buf[off..off + 2].copy_from_slice(&0xBEEFu16.to_le_bytes()); off += 2; + buf[off..off + 4].copy_from_slice(&0xDEADBEEFu32.to_le_bytes()); off += 4; + buf[off..off + 8].copy_from_slice(&0x0102030405060708u64.to_le_bytes()); off += 8; + buf[off..off + 4].copy_from_slice(&1.5f32.to_le_bytes()); off += 4; + buf[off..off + 8].copy_from_slice(&2.5f64.to_le_bytes()); off += 8; + buf[off] = 1; off += 1; // bool + assert_eq!(off, buf.len()); + assert!( + engine.validate_bytes(&buf).is_ok(), + "all-twelve-primitive battery must validate" + ); + + // Corrupted-value rejection: bool 0x40 is not 0/1. + let mut bad = buf.clone(); + bad[off - 1] = 0x40; + let err = engine.validate_bytes(&bad).unwrap_err(); + assert!(matches!(err, AlkTypeError::Access { .. }), "got {err:?}"); + } + + #[test] + fn c1_validate_bytes_packed_decodes_big_endian_subset() { + // The BE leg: packed validate_bytes had only ever decoded BE + // u32s (the chunk-header fixture) before this test. + let doc = json!({ + "$defs": { + "S": { + "kind": "struct", + "endian": "big", + "fields": [ + { "name": "i16", "kind": "int16" }, + { "name": "u64", "kind": "uint64" }, + { "name": "f64", "kind": "float64" } + ] + } + } + }); + let engine = AlkTypeEngine::compile(&doc, "S", LayoutMode::Packed, None).expect("compile"); + let mut buf = vec![0u8; 18]; + buf[0..2].copy_from_slice(&(-32000i16).to_be_bytes()); + buf[2..10].copy_from_slice(&0x0102030405060708u64.to_be_bytes()); + buf[10..18].copy_from_slice(&2.5f64.to_be_bytes()); + assert!(engine.validate_bytes(&buf).is_ok(), "BE battery must validate"); + } + #[test] fn validate_bytes_with_simple_struct_round_trips() { let doc = json!({ @@ -1362,6 +1441,64 @@ mod tests { assert!(engine.validate_bytes(&corrupt).is_err()); } + #[test] + fn c2_validate_bytes_aligned_inline_string_and_bytes_default_encoding() { + // The aligned validate_bytes tests covered maxLength + // reservations, offset-indirect, records, and unions — but the + // *default* inline length-prefixed encoding (the most common + // real shape) had zero public-path executions + // (materialize_variable_aligned's LengthPrefixed-else branch, + // review #007 C2). ADR-006 allows inline variable fields only + // in the last position, so each shape gets its own schema. + let string_doc = json!({ + "$defs": { + "S": { + "kind": "struct", + "endian": "little", + "fields": [ + { "name": "id", "kind": "uint32" }, + { "name": "name", "kind": "string" } + ] + } + } + }); + let engine = + AlkTypeEngine::compile(&string_doc, "S", LayoutMode::Aligned, None).expect("compile"); + // Offsets: id @ 0 (4), name prefix @ 4 (4), name data @ 8 (5). + let mut buf = vec![0u8; 13]; + buf[0..4].copy_from_slice(&42u32.to_le_bytes()); + buf[4..8].copy_from_slice(&5u32.to_le_bytes()); + buf[8..13].copy_from_slice(b"hello"); + assert!( + engine.validate_bytes(&buf).is_ok(), + "aligned inline string must validate through the default encoding" + ); + // Short buffer: name's declared data runs past the buffer end. + let err = engine.validate_bytes(&buf[..10]).unwrap_err(); + assert!(matches!(err, AlkTypeError::Access { .. }), "got {err:?}"); + + let bytes_doc = json!({ + "$defs": { + "S": { + "kind": "struct", + "endian": "little", + "fields": [ + { "name": "blob", "kind": "bytes" } + ] + } + } + }); + let engine = + AlkTypeEngine::compile(&bytes_doc, "S", LayoutMode::Aligned, None).expect("compile"); + let mut bbuf = vec![0u8; 7]; + bbuf[0..4].copy_from_slice(&3u32.to_le_bytes()); + bbuf[4..7].copy_from_slice(&[0xAA, 0xBB, 0xCC]); + assert!( + engine.validate_bytes(&bbuf).is_ok(), + "aligned inline bytes must validate through the default encoding" + ); + } + // ----- ValidationPlan / validate_bytes (ADR-012 §3, phase 7) ---------- #[test] diff --git a/src/read_plan.rs b/src/read_plan.rs index 5bf0ceb..1935629 100644 --- a/src/read_plan.rs +++ b/src/read_plan.rs @@ -1197,6 +1197,35 @@ mod tests { assert!(matches!(err, AlkTypeError::Schema(_)), "got {err:?}"); } + #[test] + fn c3_cycle_through_union_mapping_variant_is_schema_error() { + // The compile_variant cycle arm (review #007 C3): a cycle that + // closes through a union *mapping entry* — B appears as a + // variant of U while B is still being expanded on the current + // path. Field-level cycles (compile_typeref's arm) are covered + // by cyclic_ref_through_two_defs_is_schema_error; this shape + // reaches compile_variant's own cycle check. + let root = json!({ "$defs": { + "A": { "kind": "struct", "fields": [ + { "name": "b", "kind": { "$ref": "#/$defs/B" } } + ]}, + "B": { "kind": "struct", "fields": [ + { "name": "u", "kind": { "$ref": "#/$defs/U" } } + ]}, + "U": { + "kind": "union", + "discriminator": { "kind": "byte", "offset": 0, "type": "uint8" }, + "mapping": { "1": { "$ref": "#/$defs/B" } } + } + }}); + let err = ReadPlan::compile(&root, "A").unwrap_err(); + assert!(matches!(err, AlkTypeError::Schema(_)), "got {err:?}"); + assert!( + err.to_string().contains("cyclic $ref"), + "expected a cycle error, got {err:?}" + ); + } + #[test] fn deep_nesting_beyond_depth_cap_is_schema_error() { let mut defs = serde_json::Map::new(); diff --git a/src/tunion.rs b/src/tunion.rs index 0d1a077..0a3d61e 100644 --- a/src/tunion.rs +++ b/src/tunion.rs @@ -629,4 +629,30 @@ mod tests { let err = discriminator_size(&u).unwrap_err(); assert!(matches!(err, AlkTypeError::Schema(_))); } + + #[test] + fn l2_read_field_discriminator_enum_dispatches_on_index() { + // Review #007 L2: the enum arm of read_field_discriminator had + // zero executions — N1's parity tests covered uint16/uint32 but + // skipped enum, the last arm of the documented kind set. + let root = field_union_root("kind", "enum"); + let u = doc_union(&root, "U"); + let mut buf = vec![0u8; 8]; + buf[0..4].copy_from_slice(&0u32.to_le_bytes()); + let d = read_field_discriminator(&buf, &u, 0, LE).expect("read"); + assert_eq!(d.key, "0"); + assert_eq!(d.variant_offset, 4); + assert_eq!(d.discriminator_size, 4); + } + + #[test] + fn l2_read_field_discriminator_enum_big_endian() { + let root = field_union_root("kind", "enum"); + let u = doc_union(&root, "U"); + let mut buf = vec![0u8; 8]; + buf[0..4].copy_from_slice(&1u32.to_be_bytes()); + let d = read_field_discriminator(&buf, &u, 0, Endian::Big).expect("read"); + assert_eq!(d.key, "1"); + assert_eq!(d.variant_offset, 4); + } } \ No newline at end of file