alktype/docs/reviews/001-coverage-analysis.md

status, last_updated, reviewed_artifacts, tool, invocation, reviewer

status	last_updated	reviewed_artifacts	tool	invocation	reviewer
partially-resolved	2026-08-02	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	cargo-llvm-cov	cargo llvm-cov --html --output-dir target/coverage	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::<AlkTypeKind>().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.