Skip zarr metadata consolidation by default (opt-in)

[espg]

Closes #191

What / approach

Metadata consolidation (zarr.consolidate_metadata, run as a finalize step after every AOI dispatch) costs ~70 s/run and produces a consolidated-metadata blob that nothing in zagg's read path consults — readers navigate to specific paths in a few GETs, and the write path already passes consolidated=False everywhere. This makes consolidation opt-in via a new output.consolidate_metadata flag (default false), gated dispatcher-side so the finalize invoke is skipped entirely when off.

• Config (src/zagg/config.py): added output.consolidate_metadata (default false) with a boolean validator (mirrors output.aoi_mask) and a get_consolidate_metadata() accessor (mirrors get_aoi_mask()).
• Local backend (src/zagg/runner.py _run_local): the consolidate_metadata(zarr_store, zarr_format=3) call now runs only when the flag is true.
• Lambda backend (_run_lambda): the mode: "finalize" invoke is gated dispatcher-side — when the flag is off the executor gets a no-op finalize_fn (mirroring the temporal path _run_lambda_events, which already skips finalize because tabular output has no metadata to consolidate), so no finalize Lambda is dispatched. When on, _invoke_lambda_finalize runs byte-identically to before. The mode: "finalize" handler in deployment/aws/lambda_handler.py is unchanged — it still consolidates when invoked (which now only happens on the opt-in path), so it needed no edit.

Behavior when the flag is true is identical to the pre-change path (still consolidates, both backends).

Notebooks

The example notebooks read the published atl06_cycle22_fullsphere.zarr store with xr.open_dataset(..., consolidated=True). Since that store will be regenerated without consolidation (opt-out, no backward-compat needed), those reads are flipped to consolidated=False. consolidated=False is a safe v3 read regardless (readers do lazy metadata reads natively). Edited via a surgical JSON round-trip (indent=1, ensure_ascii=False — verified identical to the repo's on-disk formatting), so each notebook has a one-line diff and cell ids/outputs are untouched.

Phases

• Phase 1 — Default-skip gate + output.consolidate_metadata config flag (config accessor/validator; local + Lambda dispatcher-side gate).
• Phase 2 — Notebooks flipped to consolidated=False + read-parity test.
• Phase 3 — Parallelize the opt-in consolidation path. Out of scope here; tracked in Parallelize the opt-in metadata-consolidation path (deferred from #191/PR #193) #194 (see Questions for review 2).
• Fold adversarial-review finding (review): document in the parity fixture that the leaf ShardingCodec is deliberately omitted (orthogonal to the metadata-tree consolidation path).

How it was tested

• uv run pytest -q (excluding two pre-existing pyarrow-gated modules — see below): 1443 passed, 20 skipped in 401 s.
• Touched suites (test_readers test_config test_runner test_output): 419 passed.
• uv run ruff check and uv run ruff format --check on every touched file: clean.

New tests:

• tests/test_config.py::TestConsolidateMetadataFlag — accessor default off, accessor true, validator rejects non-bool.
• tests/test_runner.py::TestConsolidationGate — local runner skips consolidate_metadata by default and calls it when enabled; Lambda dispatcher skips _invoke_lambda_finalize by default and calls it when enabled.
• tests/test_readers.py::TestReadParityWithoutConsolidation — builds a two-shard located t-digest CSR store with no consolidated metadata (the default now), asserts read_tensors / read_raw_values / read_locations reach the data, then consolidates the same store and asserts every reader returns byte-identical results. Also pins that a freshly written store carries no consolidated-metadata blob (root.metadata.consolidated_metadata is None) yet is fully navigable.

Questions for review

1. Third notebook beyond the two named in the issue. notebooks/rasterized_zarr.ipynb reads the same regenerated store with consolidated=True, so it's flipped too (a one-line kwarg change), otherwise it would break against the non-consolidated store. Call out if you'd rather scope this PR to only the two the issue enumerated.
2. Phase 3 deferred → tracked in Parallelize the opt-in metadata-consolidation path (deferred from #191/PR #193) #194. zarr==3.2.1's consolidate_metadata(store, path, zarr_format) exposes no concurrency parameter; the only lever is the global zarr.config['async.concurrency'] (default 10), whose effect on the ~4,102-object serial-GET path can't be verified without the production store the issue measured against. Per the issue's guidance ("if it doesn't expose concurrency cleanly, don't fight it — note the finding and defer; do not add a dependency"), it's deferred to Parallelize the opt-in metadata-consolidation path (deferred from #191/PR #193) #194. The opt-in path is off by default, so it's not a default foot-gun.
3. Pre-existing lint drift (not fixed, per conventions). ruff check src tests reports 1 pre-existing N818 in src/zagg/registry.py (UnknownCapability), and ruff format --check src tests would reformat 7 files I did not touch (src/zagg/__init__.py, src/zagg/__main__.py, src/zagg/catalog/sources.py, src/zagg/viz/crs.py, tests/test_catalog.py, tests/test_extract.py, tests/test_integration.py). All of my touched files are clean.
4. Pre-existing test-collection gap. tests/test_shardmap.py and tests/test_viz.py fail to collect under uv sync --extra test because they import pyarrow, which the test extra doesn't install (present on origin/main too). Excluded from the run above; unrelated to this change.

Note on the earlier #188 merge-order caveat: PR #188 has since merged to main, and this branch is based on 5004df4 (post-merge), so the notebook cells already carry both the open_store rewire and consolidated=False — no rebase needed.

[espg]

🤖 from Claude (review)

Fresh-context adversarial review of the opt-in-consolidation change. Verdict: clean — no must-fix / should-fix. One low-severity inline note on the read-parity test; everything else checked out.

What I verified (independently, not on faith):

1. Lambda gate correctness — clean. Traced _finalize_fn → LambdaExecutor.finalize() (dispatch.py:390): finalize() calls self._finalize_fn(), discards its return, and returns a fresh RunReport() that the caller also discards. So the no-op path dispatches no mode:"finalize" invoke, and nothing downstream keys off a real finalize — the finalize_s bracket (runner.py:1524) still runs (~0 s), and cost/summary accounting reads report.cost / per-cell results, never the finalize result. This exactly mirrors the temporal path's finalize_fn=lambda: None (runner.py:722). Local path: workers already write with consolidated=False, so gating the standalone consolidate_metadata call still yields a valid store. Default false confirmed end-to-end (config accessor → both backends).

2. Store validity + reader parity — clean. git grep across src/ + notebooks: no reader opens with consolidated=True / use_consolidated; readers navigate to paths directly. The parity test asserts consolidated_metadata is None on a fresh store, then byte-identical reads after consolidating the same store. See the one inline note: the store is a CSR-subgroup store rather than a literal ShardingCodec store — faithful for the consolidation claim (codec-independent), worth a clarifying comment.

3. Third notebook — correct. rasterized_zarr.ipynb reads the same regenerated published store; one-kwarg diff, valid JSON, nbformat_minor 4 (no cell-id requirement — pre-existing, untouched). I also checked a fourth consolidated=True reader, bench/layout_access.ipynb: correctly left alone — its stores come from bench/layout_materialize.py:141, which always calls consolidate_metadata itself, so that pair is self-consistent and unaffected by the config gate.

4. Config validator — clean. Exact mirror of output.aoi_mask (validator placed adjacent, bool accessor with default=False). Non-bool rejected and tested. output has no allowed-keys whitelist to register the new key in, and no other output.* reader needs to honor it.

5. Test quality — solid. TestConsolidationGate drives the real _run_local / _run_lambda and counts real consolidate_metadata / _invoke_lambda_finalize calls (0 off, 1 on) across both backends × both flag states — a dropped gate or re-enabled default would fail them; not tautological (both branches genuinely reach finalize()).

6. Conventions — clean. Title-only commits (phase 1/2 of issue #191); Phase 3 deferred honestly with rationale; pre-existing N818/format drift + pyarrow collection gap correctly flagged-not-fixed. runner.py is well over ~1000 lines but that's pre-existing (+23 here), not this PR's remit.

What I ran: uv run pytest tests/test_config.py::TestConsolidateMetadataFlag tests/test_runner.py::TestConsolidationGate tests/test_readers.py::TestReadParityWithoutConsolidation -q → 8 passed. Notebook JSON validated for all three (nbformat 4, one-line diffs, cell structure intact).

No side-effecting directives raised — the PR body's #188 merge-order note is already moot since the open_store rewire is present on main (base 5004df4); whichever the author lands, keep both the open_store call and consolidated=False.

---

Generated by Claude Code