Full-coverage sidecar write-back (issue #190)

[espg]

Closes #190.

What this does

The inline write-back path persisted only the datasets the running config touched (~48 for ATL03: 8 datasets × 6 beams), not the full granule (~1,000+). This contradicts the #163 store-design decision (full ~1,055-dataset coverage) and gives up the "build once, serve any user group" property for zero read savings (the metadata was already fetched).

This makes write-back persist chunk maps for every decodable dataset in the granule, enumerated from the front-of-file metadata h5coro already cached. The read path is unchanged — it still reads only what the config needs; only what write-back persists grows.

Approach

• New _iter_datasets(h5obj) recursively walks the group tree via h5coro (metaOnly, attributes off), classifying each node (group vs dataset) and yielding (path, H5Dataset) — the parsed dataset handed back so its chunk map is built without a second object-header parse.
• build_chunk_map is split into build_chunk_map (opens the dataset) + _chunk_map_from_dataset (the metadata-only assembly), so the walk reuses the same single-source primitive.
• New full_granule_maps(h5obj, existing) widens the read-set maps to full coverage, reusing already-built maps verbatim (config datasets stay byte-identical to the read path) and mapping the rest.
• finish_granule calls full_granule_maps before persisting when write_back_coverage == "full" (the default).

Design decisions

1. Default = full; write_back_coverage: full|lazy knob added. full is the Virtual chunk-index backends for the read path (issue #160) #163 intent. I added the lazy escape hatch (reproduces the pre-Sidecar write-back persists only the config's read set (lazy), not full ~1,055-dataset coverage #190 read-set-only manifest) because the measured build delta is materially larger than 88S offsets extraction: per-chunk byte offsets for the pinned benchmark shard (issue #158) #159 predicted (see below), so a latency-sensitive private run has a way out. Validation rejects bad values and rejects the key without write_back: true (mirrors the existing store coupling).
2. Undecodable dtypes (strings/compounds): include-and-skip-at-read. The chunk map is still built (valid file offsets) with a blank dtype; datasets_from_manifest already skips blank-dtype rows at read. This matches the consumer and keeps the manifest a complete community chunk manifest. Verified on a fixture string dataset and 13 undecodable-dtype datasets on a real granule.
3. Contiguous vs chunked: build_chunk_map already handles both (pseudo-chunk for contiguous). Confirmed on real data — both routes covered. COMPACT-layout datasets (data in the object header, no file offset) raise ValueError and are dropped at write: a ranged read could never serve them.

Measurements (real ATL03 granule, local ~/ignore)

• Dataset count: 48 → 1068 persisted (1075 enumerated; 7 empty/zero-storage dropped, 1 compact dropped, strings kept with blank dtype).
• Build-time delta: lazy read-set build ~22 ms; full-coverage walk adds ~150 ms (each node's object header is parsed once). Still ~5–6× the 88S offsets extraction: per-chunk byte offsets for the pinned benchmark shard (issue #158) #159 estimate of +20–30 ms — the cost is CPU parsing ~1,000 dataset object headers, not I/O.
• No extra reads: the entire full walk stays within one 4 MiB h5coro cache line — the same front-of-file GET the config's read already paid for (file is ~1 GiB; only 4 MiB touched). Confirms 88S offsets extraction: per-chunk byte offsets for the pinned benchmark shard (issue #158) #159's core premise.
• No regression: the previously-covered (read-set) datasets are byte-identical in the full manifest (byte_offset/nbytes/dtype), because full coverage reuses the read-set maps verbatim.

Tests

• Hermetic (extended atl03_mini.h5): the fixture gained datasets the config never reads — non-read chunked (delta_time) + contiguous (segment_id), a top-level ancillary_data group, an undecodable string, and a compact dataset. Regenerated via make_fixture.py (h5py, offline). New/updated tests assert full coverage (22 fixture datasets vs 16 read set), spot-check non-read datasets present, string recorded with blank dtype, compact dropped; the byte-identical-manifest and interleaved-granule tests stay green (updated to full coverage).
• Real-granule (slow-marked, skip-if-absent): TestFullCoverageRealGranule validates >1000 datasets, read-set byte-identity, the one-cache-line bound, and prints the build-time delta. Gated on a local ~/ignore granule so CI skips it (lands in phase 2).
• Read-path parity: existing inline read tests unchanged and green — decode of the config's datasets is untouched.

Phases

• Phase 1 — full-coverage write-back + write_back_coverage knob, extended fixture, hermetic tests.
• Phase 2 — real-granule slow validation test (TestFullCoverageRealGranule).

Adversarial self-review (folded)

A fresh-context review verified the walk against h5coro internals (confirmed pathAddresses keying, and that earlyExit=False fully enumerates dense-link groups, not just the followed target). Findings folded:

• should-fix — write-back was all-or-nothing across ~1068 datasets. full_granule_maps caught only ValueError (compact); any other per-dataset build failure would propagate and sink the entire granule manifest, making full coverage strictly more fragile than the read path (which degrades one dataset to the h5coro decoder). Now the walk catches any per-dataset build failure (log + skip), matching read-path tolerance. New test: test_full_granule_maps_tolerates_one_bad_dataset.
• nit — group object header parsed twice. Restructured _iter_datasets so each node is parsed exactly once (the classification parse doubles as the group's enumeration parse) — cut the walk delta ~189 → ~150 ms.
• nit — recursion cycle guard. Added a seen set of object-header addresses to bound the descent against a (non-ICESat-2) hard-link cycle.
• nit — validation message order. write_back_coverage without write_back now reports the actionable "only meaningful" message before the value range-check.

Questions for review

• Build delta ~189 ms, not ~20–30 ms (88S offsets extraction: per-chunk byte offsets for the pinned benchmark shard (issue #158) #159). The extra cost is pure metadata parsing of ~1,000 object headers (zero extra network — one 4 MiB cache line). Acceptable per-granule for a once-built community manifest? The lazy knob is the escape hatch if not. Options: (a) keep full default as-is; (b) keep default but document the delta prominently; (c) investigate reusing h5coro's already-parsed metadataTable to shave the classification parse.
• Re-extracting the canonical NEON cache to full coverage is a deploy/ops step (per the issue) — left as a follow-up for @espg, not done here.
• Pre-existing, unrelated: tests/test_lambda_build.py::TestFunctionBuild::test_function_build_succeeds fails locally because pip can't resolve zarr>=3.1.5 for the Lambda target platform on this machine (environmental); and the repo carries pre-existing mypy errors across ~10 modules (identical on main, including the write_manifest(self.store, ...) str | None call I did not introduce). Flagged, not touched.

🤖 Generated with Claude Code

https://claude.ai/code/session_017w2BCjStojyfaTkw77BfT2

[espg]

Build delta ~189 ms, not ~20–30 ms (#159). The extra cost is pure metadata parsing of ~1,000 object headers (zero extra network — one 4 MiB cache line). Acceptable per-granule for a once-built community manifest?

yes. this is a fifth of a second total for doing the complete cache, and is noise for workers that run in 1 to 2 minutes.

[espg]

Re-extracting the canonical NEON cache to full coverage is a deploy/ops step (per the issue) — left as a follow-up for @espg, not done here.

fine