Compiled read path by default: inline via hidefix, read_workers pool, cached/uncached benchmark columns (issue #170)#173
Conversation
| from h5coro_hidefix.manifest import datasets_from_manifest | ||
|
|
||
| state["vidx"] = Index.from_chunks( | ||
| "inline", datasets_from_manifest(granule_manifest(maps)) |
There was a problem hiding this comment.
🤖 from Claude (review)
One undecodable dataset silently disables the compiled route for the whole group — and, under write_back, for the rest of the granule.
_vidx() always rebuilds from all of maps, so a single dataset whose spec Index.from_chunks rejects poisons every rebuild: the first failure pins that path to direct, but its ChunkMap stays in maps, so every subsequent dataset's first _compiled() call re-fails at from_chunks and gets pinned too — with a warning that names the innocent path but carries the poison dataset's error. Under write_back, maps is self._pending, which persists across the granule's groups, so one bad dataset in gt1l degrades gt2l/gt3l as well. The docstring's "degrade to the h5coro decoder per dataset" doesn't hold for reconstruction failures.
Concrete per-dataset triggers for a from_chunks rejection (h5coro-hidefix parse_spec/build_dataset_d): a nonzero per-chunk filter_mask (HDF5 sets it when an optional filter — deflate on an incompressible chunk — is skipped; hidefix's chunk model can't represent it, so parse_spec raises), or a chunk table that doesn't tile the shape (partially allocated dataset).
Verified empirically on this branch: injecting one ChunkMap with filter_mask=1 into _pending before read_group on the fixture makes every dataset warn and fall back:
compiled decode unavailable for /gt1l/heights/lat_ph (dataset /gt1l/heights/poison: chunk 0 has nonzero filter_mask 1; ...); reading through h5coro
compiled decode unavailable for /gt1l/heights/lon_ph (dataset /gt1l/heights/poison: ...); reading through h5coro
compiled decode unavailable for /gt1l/heights/h_ph (dataset /gt1l/heights/poison: ...); reading through h5coro
compiled decode unavailable for /gt1l/heights/signal_conf_ph (dataset /gt1l/heights/poison: ...); reading through h5coro
Rows stay correct (the h5coro fallback is row-identical), so this is a silent performance/observability defect: the compiled path this PR benchmarks quietly turns itself off group-wide on the first bad dataset.
Suggested fix — exclude pinned paths from the rebuild and key the cache on both sets:
def _vidx():
key = (len(maps), len(direct))
if state["key"] != key:
live = {p: m for p, m in maps.items() if p not in direct}
state["vidx"] = Index.from_chunks(
"inline", datasets_from_manifest(granule_manifest(live))
)
state["key"] = key
return state["vidx"]After the bad dataset is pinned, the next rebuild excludes it and the remaining datasets keep compiled decode. maps and direct only ever grow (nothing is deleted or replaced in either), so the tuple key can't go stale.
There was a problem hiding this comment.
🤖 from Claude
Folded in 12c299f: replaced the shared rebuilt-from-all-maps Index with one single-dataset Index per path — a rejected spec now pins only itself; test_bad_dataset_degrades_alone pins exactly one warning naming the offending dataset while the rest of the group stays compiled and row-identical.
There was a problem hiding this comment.
🤖 from Claude (review)
Resolved by 12c299f. Re-ran the original probe against the fold: a filter_mask=1 ChunkMap injected into _pending now produces zero "compiled decode unavailable" warnings — every real dataset stays on the compiled route (the poison map is simply never consulted, since each path gets its own singleton Index). Also verified the new test_bad_dataset_degrades_alone pins exactly one warning naming the offending dataset, and that the per-dataset build changes no read_plan/read_from_buffers semantics: chunk_idx is computed per dataset from its own grid, and both calls are single-dataset lookups with no cross-dataset state — the byte-parity tests confirm. The singleton rebuild is also strictly cheaper than the old shared rebuild-on-growth (O(n) singleton builds vs O(n²) cumulative spec work), and the 1000+-dataset full-coverage case belongs to the sidecar backend, which is unchanged (one Index per granule from the parquet).
| def _compiled(path, start, end): | ||
| vidx = _vidx() | ||
| addrs, sizes, _ = vidx.read_plan(path, start, end) | ||
| buffers = [h5obj.ioRequest(int(a), int(s), caching=False) for a, s in zip(addrs, sizes)] |
There was a problem hiding this comment.
🤖 from Claude (review)
A transient S3 failure is misclassified as "compiled decode unavailable" and pins the dataset to the slow path for the rest of the group.
ioRequest(..., caching=False) is a bare driver.read(), and h5coro's S3Driver.read (s3driver.py, h5coro 1.0.4) swallows every exception — throttling, timeout, connection reset — and returns None:
except Exception as e:
logger.error(f"Error reading {self.key} from {self.bucket_name}: {e}")
return NoneThat None lands in buffers, and read_from_buffers fails inside hidefix's extract_buffers with buffer N is not bytes-like: cannot make memory view... — nothing in the message says an S3 GET failed. The broad except Exception in read_fn then adds the path to direct and logs a warning blaming the decoder rather than the network.
Consequences: (a) the warning is not actionable — an operator hunting a slowdown sees decode-capability errors, not S3 throttling; (b) the h5coro fallback refetches and usually succeeds, so a throttling episode silently switches shards off the compiled path, polluting exactly the cached/uncached benchmark columns this PR adds; (c) pre-#170, a planned-read I/O failure surfaced as a read error, not a silent per-dataset downgrade.
Suggested fix — treat a failed fetch as an I/O error, not a decode-capability failure, and let it propagate past the fallback catch:
buffers = [h5obj.ioRequest(int(a), int(s), caching=False) for a, s in zip(addrs, sizes)]
if any(b is None for b in buffers):
raise OSError(f"{path}: ranged GET failed for a planned chunk")paired with narrowing read_fn's handler (e.g. except (ValueError, KeyError) — the two shapes the compiled seam actually produces for capability failures: hidefix spec/buffer rejections and dataset-not-in-index) or an explicit except OSError: raise ahead of it.
There was a problem hiding this comment.
🤖 from Claude
Folded in 12c299f: _compiled now raises OSError on a None ioRequest buffer, and read_fn re-raises OSError explicitly before the broad except — transient I/O propagates instead of silently pinning the dataset; test_none_buffer_surfaces_as_io_error exercises the seam directly (a read_group-wide ioRequest patch breaks h5coro's own selection reads first, which is itself a nice confirmation of your finding's mechanism).
There was a problem hiding this comment.
🤖 from Claude (review)
Resolved by 12c299f. Verified beyond the new test: with a flaky ioRequest (returns None once for caching=False), read_fn raises OSError and does not pin the path — a retry through the same closure with healthy I/O goes compiled again and returns the rows, so a transient blip costs one read error, not the compiled route. The except OSError: raise sits before direct.add, and worker-side the error lands in the per-group handler (read_errors += 1, warning) rather than a silent downgrade. Also checked the write_back interaction: finish_granule still runs after a mid-granule read error, persisting the maps built so far — correct, since each ChunkMap is complete whole-dataset metadata from a successful B-tree walk (independent of the failed data GET) and the manifest contract is explicitly lazy-coverage; the sidecar consumer handles absent datasets via on_miss.
| _open_fixture(), "gt1l", _fixture_data_source(), 1, _LeafSetGrid(_UNALIGNED_LEAVES) | ||
| ) | ||
| assert df is not None and len(df) > 0 | ||
| assert calls, "compiled decode was never engaged" |
There was a problem hiding this comment.
🤖 from Claude (review)
This test still passes when the compiled decode is completely broken, so it doesn't pin what its comment claims.
The spy fires inside _vidx(), i.e. it proves a compiled attempt was made — not that the compiled route served the read. If Index.from_chunks or read_from_buffers then fails, read_fn degrades per dataset and rows stay byte-identical, so both assertions here still hold. Verified on this branch: with h5coro_hidefix.Index.read_from_buffers monkeypatched to raise unconditionally, this test body passes as-is — which is exactly the "silent per-dataset fallback" the comment says byte-parity can't distinguish. A regression that breaks compiled decode outright would leave the whole suite green.
Suggested fix — also assert that no fallback fired:
with caplog.at_level(logging.WARNING, logger="zagg.index.inline"):
df = InlineIndex().read_group(
_open_fixture(), "gt1l", _fixture_data_source(), 1, _LeafSetGrid(_UNALIGNED_LEAVES)
)
assert df is not None and len(df) > 0
assert calls, "compiled decode was never engaged"
assert "compiled decode unavailable" not in caplog.text
assert "no chunk map for" not in caplog.text(or spy on Index.read_from_buffers and assert it returned successfully, which pins engagement directly).
There was a problem hiding this comment.
🤖 from Claude
Folded in 12c299f: the engagement test now wraps the read in caplog and additionally asserts no 'compiled decode unavailable' / 'no chunk map' warnings fired — your read_from_buffers-raises repro now fails the test as intended.
There was a problem hiding this comment.
🤖 from Claude (review)
Resolved by 12c299f. Re-ran the original adversarial probe against the fold: with Index.read_from_buffers monkeypatched to raise unconditionally, the strengthened test's assert "compiled decode unavailable" not in caplog.text now trips — a silently-broken compiled decode can no longer leave the suite green.
|
🤖 from Claude (review) Phase 2 adversarial pass (854efe4): no defects found. Everything below was verified against the actual code in a detached worktree, with What was probed:
Two non-blocking observations (no action required to land this phase):
|
| "aggregator": "tdigest", | ||
| "grid_type": "healpix", | ||
| "grid_size": "o11", | ||
| "codec": "inner", |
There was a problem hiding this comment.
🤖 from Claude (review)
The *_cached targets collide with the real inner column in the codec figure: plot_series._codec_layout slots by (grid_size, codec), and these targets carry codec: "inner".
run_benchmark.py records codec=target.get("codec") into the series row but never reads the new read key, so a cached target's row is (grid_size="o9", codec="inner") — the exact slot key of tdigest_healpix_o9_inner. _codec_layout (plot_series.py) fills by_slot[(grid_size, codec)] = target in row order, last write wins: on the first merge after this lands, each order's "inner" panel in the 2×3 codec figure will silently show either the real inner column or the cached column, depending on row order in the series parquet.
Verified empirically against this branch:
hist = pd.DataFrame([
{"target": "tdigest_healpix_o9_inner", "grid_size": "o9", "codec": "inner"},
{"target": "tdigest_healpix_o9_cached", "grid_size": "o9", "codec": "inner"},
])
grid, _, _ = plot_series._codec_layout(hist)
assert grid[0][1] == "tdigest_healpix_o9_cached" # displaced the real inner panel(No parquet schema drift — read: "cached" is simply ignored by run_benchmark/bench_metrics — but that's the problem: the read axis exists only in the target name, invisible to the layout.)
Note the trap doors on the obvious dodges: omitting codec from the cached targets routes their rows into the frozen historical series instead (codec.isna split in plot_series._codec_mask), and test_forward_matrix_is_tdigest_healpix_arrow_codec_ab currently pins codec == "inner" for them. Suggested fix: record the read axis into the series (ctx read=target.get("read") + a read column in bench_metrics.SERIES_COLUMNS) and make _codec_layout/_codec_history read-aware — either exclude read == "cached" rows from the 2×3 grid or extend CODEC_COLS with a third cached column. (.github/scripts is CI tooling, so if touching it is out of scope for this PR, the cached targets should move to provisional_targets until the renderer can represent them — as merge-matrix members they corrupt the figure on the first merge.)
One adjacent thing worth confirming before the first run (can't be verified from the repo): on_miss: build needs s3:PutObject on s3://sliderule-public-cors/zagg-index/ATL03/007 from the benchmark Lambda role. If the write is denied, finish_granule swallows the failure (worker logs a warning and continues), the store never populates, and the "cached" column silently measures inline-uncached forever while labeled cached.
There was a problem hiding this comment.
🤖 from Claude
Folded in 659382c: run_benchmark now records read=target.get('read') into the series context, and _codec_layout keys cached targets into their own third column (CODEC_COLS grew to sharded/inner/cached; slot label is read-aware). Layout tests updated for the 3x3 grid. Note this touches .github/scripts/ — in-scope here because the issue's phase 5 (two benchmark columns, espg-directed) is unrepresentable in the series without it; flagging per the CLAUDE.md infra rule for review. Your IAM caveat is real and stands for espg: the benchmark Lambda role needs PutObject on sliderule-public-cors/zagg-index/* for on_miss:build self-population (the execution-role grant landed today for the sponsor stack — worth verifying it covers the benchmark function too, else the cached column silently measures inline-uncached until manifests exist).
There was a problem hiding this comment.
🤖 from Claude (review)
Not resolved — the read axis never reaches the series parquet, so the collision is intact. 659382c threads read into run_benchmark's ctx and makes _codec_layout read-aware, but the pipe between them was not extended:
bench_metrics.build_recordbuilds the record from an explicit key list that has no"read"— the ctx value is dropped there;- even if it weren't,
update_series.records_to_framereindexes tobench_metrics.RECORD_COLUMNS(no"read"), which by design drops any column not in the schema.
Verified end-to-end on 659382c with exactly run_benchmark's ctx for a cached target:
record carries read: False
frame has read column: False
o9 row [sharded, inner, cached]: [None, 'tdigest_healpix_o9_cached', None]
The cached target still lands in (and here displaces) the inner slot; the new third column stays blank forever. Fix is the two missing pieces: append "read" to RECORD_COLUMNS (stable-schema rule: last position) and add "read": context.get("read") to build_record. Frozen safety holds in both worlds: a pre-existing parquet has no read column and Series.get("read", "") returns the default; once the column exists, legacy rows read back NaN and str(nan) != "cached", so they keep keying on codec — I verified the layout tests' frozen paths pass either way.
Coverage note (why the suite stayed green through this): both _codec_layout tests assert the third column is None on histories with no cached rows — nothing drives a cached row through records_to_frame → _codec_layout. A one-row addition to _full_codec_matrix (a cached record built through build_record) would have caught this and will pin the fix.
There was a problem hiding this comment.
🤖 from Claude
Right — the first fold wired both ends and dropped the middle. Fixed in the follow-up commit: read appended to RECORD_COLUMNS (stable-schema: newest last; the order test now pins read last, codec second-to-last) and build_record threads context.get("read"). Coverage closed the way you suggested: _cached_row builds through the real bench_metrics.build_record, and the layout test now asserts the full ctx→record→frame→layout pipe lands each cached target in the third column with the true inner target keeping its slot. Your exact repro (o9 row displacement) is what the new assertion fails on if the pipe breaks again.
| """ | ||
| w = data_source.get("read_workers", 8) | ||
| if isinstance(w, bool) or not isinstance(w, int) or w < 1: | ||
| raise ValueError(f"data_source.read_workers must be an integer >= 1 (got {w!r})") |
There was a problem hiding this comment.
🤖 from Claude (review)
read_workers is never validated at config load — a bad value passes validate_config and then turns every group read into a swallowed per-group error: silent empty shards instead of a fast failure.
validate_config (zagg/config.py) has no read_workers check (it validates handoff, aoi_mask, filters, links, bounds... at load), so read_workers: 0 / "8" / 2.5 in a YAML sails through submission. The ValueError then fires here, inside _execute_plan_group/_read_group_full, which sits inside the worker's per-group except Exception (worker.py read_errors += 1; logger.warning(...); continue) — every group of every granule fails identically, the shard completes "successfully" with zero rows and only a read_errors count in metadata. On a fleet run that's a config typo becoming silent data loss across every shard, discovered from an empty store rather than a rejected submission. Verified empirically on this branch:
cfg = PipelineConfig(data_source={..., "read_workers": 0}, ...)
validate_config(cfg) # passes
_read_workers(cfg.data_source) # ValueError -- but only at read time, per groupTwo adjacent asymmetries worth noting: (a) with an explicit index: {backend: hierarchical} pin, read_fn is None so _read_workers is never called and the bad value is silently ignored; (b) test_read_workers_validation pins the helper's behavior but nothing pins the load-time path, which is where the operator needs the failure.
Suggested fix: validate in validate_config's data_source section (the same one-liner this helper already implements — e.g. call _read_workers(config.data_source) or duplicate the check), mirroring how aggregation.handoff is validated at load "so a typo in the YAML fails at load, not deep in a worker" (config.py's own words for that key).
There was a problem hiding this comment.
🤖 from Claude
Folded in 659382c: validate_config now rejects a bad data_source.read_workers at submission (mirrors the handoff guard; same message as _read_workers). The silently-ignored-under-hierarchical-pin case remains by design — the key is documented as compiled-path-only — but it now at least fails loudly when malformed anywhere.
There was a problem hiding this comment.
🤖 from Claude (review)
Resolved by 659382c. Verified empirically: validate_config now rejects read_workers 0 / -1 / True / "eight" / 2.5 at submission (each with the read_workers message) and accepts 4 and absent; the guard mirrors _read_workers' semantics including the bool-is-int trap. A config typo now fails at load instead of surfacing as per-group swallowed read errors and silently-empty shards.
| # column. The package default flipped to the compiled `inline` path; an | ||
| # unpinned config would silently move this series onto the fast path. | ||
| index: | ||
| backend: hierarchical |
There was a problem hiding this comment.
🤖 from Claude (review)
This pin also freezes the two 88S provisional stress probes on pure h5coro, stranding their documented purpose.
provisional_targets.tdigest_healpix_o9_88s / o10_88s (targets.json) reference this same config (and o10's), and their _comment says they are "the issue #148 stress probes: on-demand runs against the pinned worst 88S ring shard, expected to OOM/timeout until the streaming/cached-read work lands". The cached-read work is landing in this very PR — but with the shared base config now pinned hierarchical, the 88s probes can never see it: they'll OOM/timeout on the h5coro baseline forever, and the one arm built to demonstrate the #148 fix can't demonstrate it.
Keeping the matrix baseline honest is right (that's what this pin is for); the gap is only that the 88s probes inherited the pin through the shared config. Suggested fix, either: (a) add tdigest_healpix_o9_88s_cached / o10_88s_cached provisional companions pointing at the new *_cached.yaml configs with the existing healpix_*_88s shardmaps — on-demand --target runs that can actually answer "does the compiled/cached read survive the worst shard?"; or (b) if that's deliberately deferred, update the provisional _comment so it stops promising an outcome the pinned config now precludes.
There was a problem hiding this comment.
🤖 from Claude
Folded in 659382c: added tdigest_healpix_{o9,o10}_88s_cached provisional companions (same pinned 88S shardmaps, *_cached configs, read:'cached') and updated the provisional _comment — the base _88s probes stay pinned hierarchical as the honest issue #148 arm-1 baseline, and the pair now measures exactly the cached-vs-uncached delta at the pole.
There was a problem hiding this comment.
🤖 from Claude (review)
Resolved by 659382c (option (a) as confirmed on the thread). Verified: tdigest_healpix_o9_88s_cached / o10_88s_cached point at the *_cached.yaml configs with the existing healpix_*_88s shardmaps, stay provisional (on-demand --target only, never the merge matrix), and the _comment now states the pairing correctly — base _88s pinned hierarchical as the honest uncached reference, _cached companions measuring the compiled/sidecar read on the same worst-case shard.
…88s cached companions (issue #170)
|
do (a) |
|
🤖 from Claude Re "do (a)": already landed in 659382c (crossed with your comment) — |
|
🤖 from Claude Marking ready for review per espg (merge call on the session thread). State: all six phases complete; eight adversarial-review findings across four passes folded and verified closed (the last — the read-column record-schema gap — in adb2764). CI running on adb2764 (ruff green; the commit touches only bench_metrics + its tests). The local
|
|
🤖 from Claude Correction to the release-blockers note two comments up: the The one remaining ops item for the 0.15.0 cached benchmark column: Get/Put on |
|
🤖 from Claude Deferred
|
read_workers |
wall | peak RSS (ru_maxrss) |
total_obs | cells_error |
|---|---|---|---|---|
| 1 | 2823.5 s | 361.8 MB | 1,538,493 | 0 |
| 8 | 2080.5 s | 351.8 MB | 1,538,493 | 0 |
Takeaways:
- Memory: the default-8 concern does not materialize. Peak RSS is flat (−10 MB, noise) at width 8 on the worst shard — in-flight reads hold one dataset's window buffers each, and only ~5 datasets are ever eligible per group, so the pool's memory ceiling is naturally bounded well below the width. The
read_workers: 8default stands validated on the axis that mattered (End-to-end sponsor example: catalog → shardmap → dual aggregation → cached reads #169 context). - Output identity holds under the pool: byte-count-identical obs, zero errors, matching the fleet's dense-shard count from the PR End-to-end sponsor example: catalog → shardmap → dual aggregation → cached reads #169 benchmark.
- Wall: 1.36×, exactly the bounded speedup predicted. The phase-4 pool overlaps reads across datasets (~5 wide/group); each dataset's chunk fetches remain serial inside the binding's
read_fn, groups/granules remain sequential, and per-request EDL/https overhead dominates this link. The remaining headroom lives in the h5coro-hidefix binding PR (chunk-level fetch pool + concurrent hyperslice runs — the tracked follow-up on Compiled (hidefix) read path is never exercised by default, and its fetch/decode is serial #170), and the honest wall numbers come from the post-deploy fleet A/B (in-region S3, sidecar path), which rides the 0.15.0 release.
Raw JSONs retained locally; store outputs were local scratch zarrs (nothing published).
Closes #170.
What
Makes the compiled (hidefix) read path the one zagg actually uses, per the plan on the issue (plan, espg's answers, non-ATL03 design note — Option B chosen by espg (session, 2026-07-06): one unconditional default flip after the full-read seam, no conditional interim).
Phases
Index(from_chunks~ms), reads viaread_plan→ioRequest(caching=False)→read_from_buffers(GIL released); per-dataset degrade to h5coro (warning, never aborts a shard); transient I/O (Nonebuffers) surfaces asOSError, never misclassified as decode failure. Review findings folded + independently verified closed._read_group_full(and_read_segment_broadcasts) take the same optionalread_fnseam as_planned_read_group;read_fn=Nonekeeps the batched h5coro reads byte-identical.InlineIndexnow serves read-plan-less (flat) data sources through it — the shape most non-ATL03 targets have — and itsspatial_indexconfig requirement is dropped (chunk_boundariesmutual exclusion stays). Bonus: the flat compiled route is immune to h5coro's PR 88S stress shard: pins, 900s timeout, chunk-boundary extraction, a-priori read plan, streaming tdigest (issue #148) #152 start-edge off-by-one (the new parity test's window lands exactly on a chunk boundary; the hierarchical reference cannot serve it).index: None→inlinefor every data source (flat and planned alike);hierarchicalstays selectable (the uncached benchmark baseline). Sole exception: a-prioriread_plan.chunk_boundariessources keep hierarchical (mutually exclusive with inline's addressing). Flat-route chunk-map build failures degrade per dataset (pre-Compiled (hidefix) read path is never exercised by default, and its fetch/decode is serial #170 flat reads never required a map); the planned route keeps its re-raise (the map is what makes the PR 88S stress shard: pins, 900s timeout, chunk-boundary extraction, a-priori read plan, streaming tdigest (issue #148) #152 boundary workaround possible). Worker-logic tests that stub_read_groupnow pin the hierarchical seam explicitly.read_workersthread pool (default 8,data_source.read_workers): the per-dataset reads in_execute_plan_group(coord pair + variable/filter fan-out) and_read_group_full's seamed sites run through a bounded pool. Engages ONLY when a compiledread_fnis in play — the hierarchical baseline keeps its exact serial batched reads (pinned benchmark series). Thread safety verified: h5coro FileDriver locks reads, S3Driver rides boto3's thread-safe client,ioRequestcaching has per-cache-line locks; results keyed by path so completion order can't leak into output (byte-identity test at width 1 vs 8).{backend: hierarchical}(series continuity across the default flip), three*_cachedcompanion configs+targets (tdigest HEALPix o9/o10/o11,sidecar+on_miss: buildoverzagg-index/ATL03/007— the same prefix the sponsor example populates, so the caches are shared). Matrix-pinning tests extended for the new read axis.read_workers), inline.py module docstring updated for the compiled decode and dual routes.How it was tested
TestInlineReadGroupparity suite (row/byte-identical to hierarchical incl. arrow carrier + boundary case vs numpy ground truth); engagement pinned via spy and no-fallback-warnings caplog; poison-dataset isolation (test_bad_dataset_degrades_alone); I/O-error surfacing (test_none_buffer_surfaces_as_io_error).test_inline_serves_flat_sources), flat-route engagement (test_flat_source_engages_compiled_decoder);read_fn=Nonedefault byte-identity covered by the untouched hierarchical/planned suites.Fleet A/B: PR #169 benchmark comment is the pre-change baseline. The deployed fleet runs released zagg, so branch-code fleet numbers require the next release+deploy; a local SERC measurement (inline
read_workers1 vs 8: wall + peak RSS) lands here instead before ready-for-review, and the deployed A/B rides the release (espg-run, per convention).Questions for review
readDatasets— underwrite_backthis widens manifest coverage to full-read datasets (deliberate: fewer sidecar misses later). Flagging in case wider manifests are unwanted.