--- status: partially-resolved last_updated: 2026-08-02 reviewed_artifacts: - src/lib.rs - src/error.rs - src/macros.rs - src/data_access.rs - src/validation.rs - src/schema.rs - src/offset_map.rs - src/layout_builder.rs - src/sequential_reader.rs - src/tunion.rs - src/engine.rs - tests/{engine_integration,error_paths,poc_roundtrip,tunion_dispatch}.rs tool: cargo-llvm-cov invocation: cargo llvm-cov --html --output-dir target/coverage reviewer: coverage pass (post-rebrand sanity check) --- # Coverage Analysis #001 ## Purpose First dedicated code-coverage pass for the alktype crate, run immediately after the rebranding cleanup (commit `6d61429`). This pass has two goals: 1. Establish a coverage baseline before any new development begins, so future passes can measure deltas. 2. Identify the weakly-covered areas that should be tightened up before publishing — while the surface is still small and the cost of adding tests is low. Headline result: **coverage is in reasonable shape for a crate that was built task-by-task with per-task tests.** Line coverage is **88.9%** (4557/5125), function coverage is **82.3%** (433/526), all 285 tests pass, and the build is clean. The gaps concentrate in three categories: (a) defensive overflow guards that are structurally hard to reach, (b) an asymmetry between `is_valid()` (heavily tested) and `validate()` (rarely called) in the validation layer, and (c) the `error::Display` impl which is never exercised. None of the gaps indicate a logic defect — the code under test was reviewed during the rebranding pass and is consistent. ## Methodology - `cargo llvm-cov --json --output-path /tmp/coverage.json` then per-file line + function breakdown from the JSON export. - `cargo llvm-cov --text` to extract the exact uncovered source lines per file, mapped back to the source for attribution. - Each gap classified by testability: trivial (pure function / Display impl), easy (existing test patterns extend naturally), medium (new error-path scaffolding), or hard (overflow guards needing `usize::MAX`-adjacent inputs). - No `--all-features` flag — the crate has no feature flags currently (`[features] default = []` in `Cargo.toml`). ## Summary Statistics | Severity | Count | |----------|-------| | Critical | 0 | | Warning | 0 | | Suggestion | 8 (S1–S8) | No correctness findings — this is a coverage pass, not a logic review. The logic review was performed as part of the rebranding sanity check (commit `6d61429` message documents the full sweep). Everything below is a suggestion ordered by leverage, not severity. --- ## Per-File Coverage | File | Lines | Covered | % | Fns | Covered | % | |------|------:|--------:|--:|----:|--------:|--:| | src/error.rs | 9 | 0 | 0.0% | 1 | 0 | 0.0% | | src/macros.rs | 144 | 109 | 75.7% | 19 | 17 | 89.5% | | src/data_access.rs | 459 | 388 | 84.5% | 62 | 48 | 77.4% | | src/validation.rs | 433 | 368 | 85.0% | 51 | 45 | 88.2% | | src/sequential_reader.rs | 1115 | 965 | 86.5% | 82 | 58 | 70.7% | | src/layout_builder.rs | 1036 | 926 | 89.4% | 95 | 72 | 75.8% | | src/schema.rs | 464 | 423 | 91.2% | 64 | 56 | 87.5% | | src/offset_map.rs | 605 | 560 | 92.6% | 63 | 54 | 85.7% | | src/tunion.rs | 423 | 395 | 93.4% | 49 | 45 | 91.8% | | src/engine.rs | 437 | 423 | 96.8% | 40 | 38 | 95.0% | | **TOTAL** | **5125** | **4557** | **88.9%** | **526** | **433** | **82.3%** | `engine.rs` and `tunion.rs` are in good shape. The gaps concentrate in `error.rs` (the `Display` impl), the macro-generated validation code (`macros.rs`), and the error-path arms across the layout and reader modules. --- ## Suggestions ### S1. `AlkTypeError::Display` impl is completely unexercised (error.rs, 0%) **File**: `src/error.rs:30–43` **Problem**: The `Display` impl for `AlkTypeError` is never called by any test. All four arms — `Schema`, `Offset`, `Access`, `Validation` — are uncovered. This is the only file at 0% and the only place where a public trait implementation has zero coverage. The `Display` impl is public API surface: consumers format errors for logging, error responses, and debugging. A typo or a `field_path`/`reason` swap in the format strings would not be caught by any test today. **Fix**: ~10 lines of unit tests. Construct one error of each variant and assert `format!("{e}")` contains the expected substrings: - `AlkTypeError::Schema("bad")` → `"schema error: bad"` - `AlkTypeError::Offset { field_path: "f", reason: "r" }` → `"offset error at f: r"` - `AlkTypeError::Access { field_path: "f", reason: "r" }` → `"access error at f: r"` - `AlkTypeError::Validation(...)` → `"validation error: ..."` (use a synthetic `ValidationError` or skip the substring check on the inner message) Add a `std::error::Error::source()` call to confirm it returns `None` (the impl is the blanket `impl std::error::Error for AlkTypeError {}`). **Lift**: 9 uncovered lines → 100% for `error.rs`. Trivial effort, no new infrastructure. --- ### S2. `validate()` vs `is_valid()` asymmetry in the validation layer (validation.rs, macros.rs) **Files**: `src/validation.rs` (the `Int64Validator`, `Uint64Validator`, `StringValidator`, `BytesValidator`, `EnumValidator`, `TimestampValidator` `validate()` methods); `src/macros.rs` (the macro-generated `validate()` methods for `Int8/16/32`, `Uint8/16/32`, `Float32/64`, `Struct/Union/Array/Record/Boolean`) **Problem**: The tests heavily exercise `is_valid()` (the bool check) but rarely call `validate()` (the `Result`-returning method). The `AlkTypeEngine::validate_json` public API calls `validator.validate()`, so the `Result` paths are the ones consumers actually hit when they want error messages — but they're uncovered. Specifically uncovered: - `Int64Validator::validate` / `Uint64Validator::validate` — the `Ok` and `Err` arms for both. `is_valid()` is tested; `validate()` is not. - `StringValidator::validate` — the `maxLength` exceeded arm and the non-string arm. The `Ok` arm is hit via `is_valid()` but not `validate()`. - `BytesValidator::validate` — same shape as StringValidator. - `EnumValidator::validate` — the whole method (it is a no-op that returns `Ok(())`, but it should be called at least once). - `TimestampValidator::validate` — the `Ok` arm (valid RFC 3339) and the `Err` arm (non-timestamp string). - Macro-generated `validate()` for all numeric / type-check validators — the `Ok` arm and the `Err` arm of each. The factory error arms are also uncovered: `string_factory`, `bytes_factory`, `enum_factory`, `timestamp_factory` each have a branch that rejects a keyword value that isn't `true` (or `true`/object for string/bytes). Only the `Ok` path of each factory is exercised. **Fix**: ~40 lines of unit tests. For each validator, call `validator.validate(&instance)` on a valid instance (assert `Ok(())`) and an invalid instance (assert `Err`, check the error message substring). For the factories, build a validator with a malformed keyword value (e.g. `{"AlkType:String": 42}`, `{"AlkType:Enum": false}`) and assert `build_validator` returns `Err(AlkTypeError::Schema(_))`. **Lift**: ~65 uncovered lines in `validation.rs`, ~35 in `macros.rs`. Brings both files above 95% line coverage. Easy — the test patterns already exist for `is_valid()`, just call the `Result`-returning twin. --- ### S3. Engine `read_field` for String and Struct (engine.rs, ~12 lines) **File**: `src/engine.rs:267–282` **Problem**: `AlkTypeEngine::read_field` in aligned mode is tested for `Uint8`, `Uint32`, and `String` (the last only via a string-specific test). The `AlkTypeKind::String` arm at line 267 and the `AlkTypeKind::Struct` arm at line 279 are uncovered — the Struct arm returns `FieldValue::Struct { start, end }` and is the path consumers use to get a byte range for nested-struct recursion. The two `AlkTypeError::Offset` error arms at lines 210–216 (field schema not found / no `AlkType:*` kind) are also uncovered. These fire when the `OffsetMap` and the schema tree disagree about a field's existence — a defensive path, but it's public API surface that should produce a sensible error. **Fix**: ~20 lines of tests, extending the existing `read_field_aligned_*` tests in `src/engine.rs`: - A test with a nested struct field, asserting `read_field` returns `FieldValue::Struct { start, end }` matching the `OffsetMap` range. - A test calling `read_field` on a field path that doesn't exist in the schema tree (but does exist in the offset map by construction — or skip this if the two are always in sync; the error arm is defensive). - A `String` read via `read_field` (the existing `read_field_aligned_reads_string_length_prefixed` test covers this — verify it is actually hitting line 267 and not just the data_access call). **Lift**: 12 uncovered lines, lifts `engine.rs` from 96.8% to ~100%. Easy. --- ### S4. `SequentialReader` error paths and uncommon reads (sequential_reader.rs, ~117 lines) **File**: `src/sequential_reader.rs` (many locations — see below) **Problem**: This is the largest gap area by line count. The uncovered lines fall into four groups: 1. **Union error paths** (~30 lines): the `read_union_value` overflow guards (`variant_start + disc_size overflows`, `after_disc + variant_size overflows`, `union end ... overflows`), the missing- properties and missing-discriminator-field schema errors, and the unknown-discriminator-value access error for field-name unions. These are defensive guards, but the "unknown discriminator value" path is a real consumer-facing error that should produce a clear message. 2. **Array error paths** (~15 lines): the `array schema is not an object`, `declared fixed count but minItems is absent`, `items schema has no AlkType:* kind`, `array size × stride overflows`, and `array end overflows` arms. The `items schema has no AlkType:* kind` arm is a real validation error a consumer would hit with a malformed schema. 3. **Discriminator helper arms** (~10 lines): `read_byte_discriminator` only exercises the `Uint8` arm; the `Uint16` and `Uint32` arms are uncovered. `discriminator_string_value` only exercises the `String` arm; `U8`/`U16`/`U32`/`Enum` are uncovered. 4. **Miscellaneous** (~60 lines): the `schema()` accessor, the "walked backwards" guards (defensive), the `resolve_and_walk_variant` `Union`-as-variant arm (nested unions), the `resolve_variant_schema` `None`-return arms, and several `panic!("expected ...")` arms in test helpers (these are unreachable by construction — they're match exhaustiveness guards). **Fix**: ~50 lines of tests, split by group: - **Group 1**: feed a `read_next` call a buffer with a byte-discriminator union whose discriminator value isn't in the mapping (the existing `union_unknown_discriminator_returns_access_error` covers the byte case; add the field-name case). Add a field-name union whose `properties` is missing entirely (schema error). - **Group 2**: an array with `items` set to a schema with no `AlkType:*` keyword (schema error). An array with `minItems` set but `maxItems` absent and a count prefix that's too short (access error — already partly covered by `buffer_too_short_returns_access_error`). - **Group 3**: a `read_next` with a `Uint16` byte discriminator and a `Uint32` byte discriminator (extend the existing `byte_discriminator_union_reads_value` with a parametrized disc type). A `discriminator_string_value` test with `U8`/`Enum` field discriminators (extend `field_discriminator_union_reads_value`). - **Group 4**: a `schema()` accessor one-liner. A nested-union-as- variant test (a union whose mapping points at another union). The "walked backwards" guards and `panic!("expected ...")` arms are unreachable by construction — leave them. **Lift**: ~60 uncovered lines (excluding the unreachable guards), lifts `sequential_reader.rs` from 86.5% to ~92%. Medium effort — the test patterns exist, but the error-path scaffolding needs a few crafted schemas and buffers. --- ### S5. `LayoutBuilder` error paths (layout_builder.rs, ~85 lines) **File**: `src/layout_builder.rs` (many locations) **Problem**: Similar shape to S4 but on the write-side. The uncovered lines are almost all `AlkTypeError::Offset` construction arms for defensive guards: `type_size returned None for fixed kind`, `offset + size overflows`, `prefix + data size overflows`, array/union error constructions, and the `unreachable!` exhaustiveness guard. Three `panic!("expected Offset, ...")` arms in test helpers are unreachable by construction. The one error path that's a real consumer-facing validation error is the "TUnion variant must be AlkType:Struct" arm (lines 528, 591) — a consumer who points a union mapping at a non-struct variant should get a clear error. This is uncovered for both byte-offset and field-name discriminators. **Fix**: ~15 lines of tests: - A union mapping whose variant is `AlkType:Uint32` (not a struct) — assert `build` returns `Err(AlkTypeError::Offset { .. })` with a reason containing "variant must be AlkType:Struct". Test both byte-offset and field-name discriminator variants. - The remaining ~70 lines are overflow guards and `unreachable!` — leave them (same reasoning as S8 below). **Lift**: ~6 uncovered lines (the two "variant must be Struct" arms). The rest is defensive. Low effort for the consumer-facing error; the guards are S8 territory. --- ### S6. `schema.rs` unused public accessors (schema.rs, ~34 lines) **File**: `src/schema.rs` **Problem**: Several public API methods are never called by any internal code or test: - `AlkTypeKind::as_str()` — ~15 of the 19 match arms are uncovered (only `Uint32`, `Float32`, `Float64`, `Timestamp` are hit by existing tests). The method is public; consumers use it for error messages and debugging. - `AlkTypeKind::needs_endian()` — completely uncovered. The method exists but no internal caller uses it (the engine and data_access layer infer endianness need from the kind directly). - `AlkTypeKind::is_composite()` — completely uncovered. Same situation. - `AlkTypeKind::get_alktype_kind_enum()` — the strict (boolean-form-only) variant. Tests use `get_alktype_kind_loose_enum` instead. **Fix**: ~15 lines of tests: - A `as_str()` round-trip test: `for kind in [all 19 variants] { assert kind.as_str().parse::().unwrap() == kind }`. - A `needs_endian()` test asserting `true` for the multi-byte kinds and `false` for the rest. - An `is_composite()` test asserting `true` for `Struct/Union/Array/ Record` and `false` for the rest. - A `get_alktype_kind_enum()` test asserting it returns `None` for the object-annotation form (mirroring the existing `get_alktype_kind_ignores_object_keyword` test for the loose variant). **Lift**: ~34 uncovered lines, lifts `schema.rs` from 91.2% to ~100%. Trivial — these are pure functions over the enum. **Question**: `needs_endian()` and `is_composite()` are currently unused internally. If they have no external consumer either (this is a fresh crate, no consumers yet), consider whether they're dead code that should be removed, or kept as part of the intended public API. If kept, test them; if removed, the coverage gap disappears. This is a design decision, not a coverage decision. --- ### S7. `offset_map.rs` defensive guards and one missing-properties path (offset_map.rs, ~35 lines) **File**: `src/offset_map.rs` **Problem**: The uncovered lines are almost all `AlkTypeError::Offset` construction arms for defensive guards: `type_size returned None`, `element kind has no fixed size`, `array schema is not an object`, `TArray is missing 'items'`, `could not resolve TArray items schema`, `TArray element schema has no AlkType:* kind`. One `unreachable!` exhaustiveness guard and one `round_up`/`align_up` early-return arm. The one path that's a real consumer-facing error is the "struct schema has no 'properties' object" arm — a nested struct missing `properties` should produce a schema error. This is uncovered. **Fix**: ~5 lines: a test with a nested struct whose `properties` is absent, asserting `OffsetMap::compute` returns `Err(AlkTypeError::Schema(_))`. **Lift**: ~3 uncovered lines (the missing-properties arm). The rest is defensive (S8). Low effort. --- ### S8. Defensive overflow guards — the long tail (all modules, ~150 lines total) **Files**: `src/data_access.rs`, `src/layout_builder.rs`, `src/sequential_reader.rs`, `src/tunion.rs`, `src/offset_map.rs` **Problem**: The single largest category of uncovered lines across the crate is `checked_add(...).ok_or_else(|| ... overflows usize)` guards. These appear in `data_access.rs` (the read/write array and string functions), `layout_builder.rs` (every offset computation), `sequential_reader.rs` (every offset + size computation in the union/ array/record readers), `tunion.rs` (the discriminator offset + size guards), and `offset_map.rs` (the array offset computations). They are structurally hard to test because they require `usize::MAX`-adjacent inputs to trigger the overflow. The code is simple — a `format!` constructing an error message — and the guards are correct-by-construction (they wrap a `checked_add` that returns `None` on overflow). **Fix**: Two options, in increasing fidelity: 1. **Skip them** — these are defensive guards over trivial format- string construction. They're low-risk gaps. Document in this review that they're intentionally left, and move on. This is the recommended path for v1. 2. **Targeted overflow tests** — for each `checked_add` site, construct an input where `offset + size` would overflow `usize` (e.g. `offset = usize::MAX - 1`, `size = 2`). This requires crafted buffers or `var_sizes` entries near `usize::MAX`. ~20 tests, ~150 lines, covers the full long tail. High effort, low value — the code is a format string. **Lift**: up to ~150 lines if option 2 is taken. Option 1 is recommended — the effort/value ratio is poor and the guards are correct-by-construction. --- ## Recommended Order 1. **S1** (error.rs Display) — trivial, 9 lines, the only 0% file. Do this first; it's a 5-minute fix. 2. **S2** (validate() paths) — easy, ~100 lines across validation.rs and macros.rs. Largest single leverage: lifts two files above 95% and exercises the public `validate_json` error-return path. 3. **S6** (schema.rs accessors) — trivial, ~15 lines, lifts schema.rs to ~100%. Also resolve the "keep or remove `needs_endian`/ `is_composite`" design question. 4. **S3** (engine String/Struct read) — easy, ~20 lines, lifts engine.rs to ~100%. 5. **S4** (sequential reader error paths) — medium, ~50 lines for ~60 lifted lines. The discriminator-helper arms and the unknown-value error paths are the high-value subset; the overflow guards are S8. 6. **S5** (layout_builder "variant must be Struct") — low effort, ~6 lines for the consumer-facing error. The rest is S8. 7. **S7** (offset_map missing-properties) — low effort, ~3 lines. The rest is S8. 8. **S8** (overflow guards) — recommended to skip for v1. Document as intentionally deferred; revisit if the crate ever handles untrusted offsets from untrusted input. After S1–S7 (skipping S8), estimated coverage: **~95% lines, ~92% functions**. The remaining ~5% is the overflow-guard long tail. --- ## Notes - All line numbers refer to the tree at commit `6d61429` (the rebranding cleanup commit). The coverage was measured on that tree. - No critical or warning findings: this pass confirms the gaps are testing-only. The underlying logic was reviewed during the rebranding sweep (the full source was read; all `AlkType:*` keyword strings, the `AlkTypeKind` enum, the parsers, the validators, and the public API are consistent). - The crate has no feature flags (`[features] default = []`), so `--all-features` is not needed. If feature flags are added later (e.g. `no_std` per OQ-002), this pass should be re-run with `--all-features` to catch the cfg-gated paths. - The `panic!("expected ...")` arms in test helpers across `sequential_reader.rs`, `tunion.rs`, `layout_builder.rs`, and `data_access.rs` are unreachable by construction (they're match exhaustiveness guards on `AlkTypeError` variants in `assert!` matchers). They show as uncovered but are not test gaps — they're defensive code in test helpers. --- ## Resolution (2026-08-02) The high-leverage, low-effort suggestions (S1, S2, S3, S5, S6, S7) were implemented in the same pass. 61 new tests added (285 → 346 passing). Workspace coverage rose from **88.9% → 91.9%** lines (4557/5125 → 5118/5568) and **82.3% → 86.3%** functions (433/526 → 498/577). `cargo build`, `cargo test`, and `cargo clippy -- -D warnings` are all green (0 warnings). Per-file deltas on the targeted files: | File | Before | After | |------|-------:|------:| | src/error.rs | 0.0% | 100.0% | | src/validation.rs | 85.0% | 97.5% | | src/macros.rs | 75.7% | 93.1% | | src/schema.rs | 91.2% | 97.5% | | src/layout_builder.rs | 89.4% | 90.3% | | src/offset_map.rs | 92.6% | 93.3% | What landed: - **S1 (error.rs Display)**: 5 tests covering `Display` for all four `AlkTypeError` variants (`Schema`, `Offset`, `Access`, `Validation`) and `Error::source()` returning `None`. error.rs is now at 100%. - **S2 (validate() paths)**: 28 tests covering the `validate()` (Result- returning) method of every validator — `Int8/16/32`, `Uint8/16/32`, `Int64`, `Uint64`, `Float32/64`, `Boolean`, `String` (with maxLength exceeded), `Bytes` (with maxLength exceeded), `Enum`, `Timestamp`, `Struct`, `Union`, `Array`, `Record` — plus the factory rejection arms for all 12 keyword factories (rejecting non-true / non-bool-or-object values). validation.rs is now at 97.5% lines / 100% functions; macros.rs at 93.1% lines / 100% functions. - **S3 (engine reads)**: 3 tests covering `read_field` for nested struct leaf fields (`header.magic`, `header.version`), `Bytes` fields, and `Timestamp` fields (read as length-prefixed string). The `AlkTypeKind::Struct` arm in `read_field` remains uncovered — it requires an offset-map entry for a struct path, but the `OffsetMap` only records leaf fields. This is a design characteristic, not a test gap (see Notes below). - **S5 (layout_builder "variant must be Struct")**: 2 tests covering the byte-offset and field-name discriminator union variant-must-be- struct error paths. - **S6 (schema.rs accessors)**: 6 tests covering `AlkTypeKind::as_str()` round-trip for all 19 variants, `Display`, `needs_endian()`, `is_composite()`, and `get_alktype_kind_enum()` (strict bool-form variant). schema.rs is now at 97.5% lines / 94.3% functions. The `needs_endian()` and `is_composite()` methods are kept as public API (they're useful for consumers even though no internal caller uses them currently). - **S7 (offset_map missing-properties)**: 1 test covering the nested- struct-without-properties schema error path. Remaining (deferred to follow-up sessions): - **S4 (sequential_reader error paths)** — the largest remaining gap (~117 uncovered lines). Medium effort: union error paths, array error paths, discriminator helper Uint16/Uint32 arms, `discriminator_string_value` for U8/U16/U32/Enum, nested-union-as-variant, and the `schema()` accessor. The test patterns exist but need crafted schemas and buffers. Good candidate for a dedicated session. - **S8 (overflow guards)** — ~150 lines of `checked_add` overflow guards across all modules. Recommended to skip for v1 (correct-by-construction format strings, poor effort/value ratio). Revisit if the crate ever handles untrusted offsets from untrusted input. ### Notes on the `AlkTypeKind::Struct` arm in `engine.rs::read_field` The `AlkTypeKind::Struct` arm (engine.rs:279) returns `FieldValue::Struct { start, end }` for a struct field. It's uncovered because the `OffsetMap` records only leaf fields (`header.magic`, `header.version`), not intermediate struct paths (`header`). A `read_field(&buf, "header")` call fails at the offset-map lookup, never reaching the `AlkTypeKind::Struct` arm. This is a design characteristic: the aligned-mode API is designed for reading individual leaf fields at known offsets, not for extracting nested-struct byte ranges (the consumer knows the range from the schema and the offset map's leaf positions). The arm exists for completeness but is unreachable through the current offset-map + schema-tree lookup path. Leaving it uncovered is correct; removing it would be a design decision for a separate pass.