resolution nodes: the 8b order level in open_store (issue #15)

[espg]

Closes #15 — lands 8b, resolution/order nodes in the open_store DataTree, the last open phase of the issue (8a + 8c + the 8b design section landed in #24). The gate opened with englacial/zagg#344's merge: the overview writer (sweep_overviews) is on zagg main, so the fixture is real-writer end to end.

Implements the pinned 8b design (concepts page, quoting zagg#201's rulings) against the normative manifest grammar of the zagg store specification §4 (englacial/zagg#340 branch claude/340-store-spec, soon PR #346) — spec-first, per the ratified lean-4 softening.

What this does

Node discovery is manifest declaration (spec §4.5 — THE ruling that closed the 8b design section's open question): the reader binds pyramid.overview.orders and nothing else. [], a missing overview mapping, or no pyramid block at all (pre-pyramid manifests, and the committed fixtures' legacy {"orders": []} placeholder) all read as declared off — today's flat product node, unchanged, regression-pinned against the existing multi-product fixture. When orders is non-empty the block's spec is strict-checked (zagg-pyramid/1), spacing/all_time/fields MUST all be present (each absence has a test), and additional keys are tolerated (the fixture manifest itself carries the sweep's materialized actuals; per-field nan_policy rides in fields). No listing walk, no MOC extension — both explicitly not implemented.

Tree shape ({product}/{order}, nodes named by the integer cell order they store, §4.4 constant depth k_cell = c - (s - k)): the product node becomes an empty intermediate (attrs: semantic_hash + manifest summary), the source data becomes the source-order child — the same open_hive Dataset, assert_identical-pinned — and each declared overview order with ≥1 stamped object becomes a sibling node via the new moczarr.pyramid.open_overview_order. Candidates are the root-MOC source shards coarsened to the ancestor prefix (zagg#201 ruling (5): the MOC is the source domain — a different code path from the leaf arithmetic, sharing its tiers: D4 stamp admission, issue-#4 empty posture per node, zero chunk GETs on the moc default). Candidates are ordered by packed word, not decimal string — a leading - sorts before every digit as a string but after every northern base cell as a word, so a decimal sort mislabels rows on any store spanning hemispheres (found in review; pinned by a synthetic northern+southern graft). A bare single-product store grows the same order level under its degenerate child (pinned by opening the fixture's product subtree directly).

role is per object, never per node: every order node's dataset carries attrs["zagg_objects"] — one entry per admitted object with its own role (absence on disk = source, §4.3) and, for overviews, the full zagg_overview block (D11 companions: source_cell_order, per-field class/method/nan_policy). Enforcement is per §4.3 with two severities, split on §4.1's "regenerable caches, never load-bearing" (settled in review — thread): an uninterpretable cache object (role outside the closed vocabulary, missing or unknown-revision zagg_overview, no cell_order) is dropped with a UserWarning while the node — and the product's source nodes — stand, and an interpretable but wrong one (a cell_order other than the node's) raises, since those rows would be mis-ranked under the node's §4.4 coordinate. Every case has a test, including that the source node's count total survives a dropped object. Validation runs on every stamped object before any AOI scoping, so a store's integrity does not depend on the query that opened it. A doctored mixture node (one object's role attrs stripped → coarse source beside overviews) classifies per object.

Helpers range over source-order SETS (source_orders, overview_orders, finest_source_at, node_objects — exported at the package root): keyed on the per-object entries, never node names, and structural rather than query-scoped (unaffected by the aoi/window the tree was opened with, and defined on a flat product node, where source_orders is its own cell order and overview_orders is ()); the mixture test pins that a role flip moves an order into the source set while it stays in the overview set.

Per-node variable sets differ in both directions: option A falls out of the fixture (the default atl06 aggregation's h_mean/h_sigma/h_variance/quantiles are none-class — present at /8, absent at /6 and /4; count/h_min/h_max roll up), and option B (declared derived summaries / Phase F — zagg-side, not yet writable) is grammar-pinned by injecting an extra array into one overview object and asserting the node carries a variable no source order has.

Computed-compose never a node: children are exactly source + declared orders (pinned).

Kwargs forwarding per #24's rulings: window reaches windowed products only and scopes their order nodes to that window's {window}.zarr overviews (D23 naming; per-window folds differ, both windows golden-pinned); aoi scopes rows per node at each node's own order — never the tree shape (an out-of-coverage AOI empties every node schema-correct, issue #4); index_kind="pandas" materializes value-identical coordinates; fabricate_cell_ids fabricates at the node's own order; decode binds level to the node's morton_hive.cell_order.

Degradation (overviews are D9 regenerable caches, never load-bearing): unstamped overview = debris, skipped (doctored test); declared-but-unswept order → node omitted with a warning; no usable root MOC → order nodes omitted loudly while the source child opens via the walk — which now skips non-decimal overview basenames (all.zarr/{window}.zarr at ancestor nodes would previously have crashed _candidate_leaves/_schema_leaf on a MOC-less pyramid store; both walk consumers hardened + tested).

Supporting refactor: store.read_leaf_metas (raw root zarr.json payloads, batched) is now the substrate under read_commits, so the stamp and the role attrs come from the same GET — the tree layer adds zero extra object fetches per candidate, and zero chunk GETs (pinned). Its metadata tier is bounded per product, not per declared order: open_store builds one obstore handle and reads coverage.moc once for a product's whole order level, threading both into open_overview_order (issue #5's rule; measured 3 store constructions / 2 MOC GETs for a two-order pyramid, pinned by a test). open_hive gains a private _objects_out collection hook (the zagg occupied_out style) so the source child's per-object entries ride the stamps it already fetched.

Phases

• Phase 1 — zagg-written overview fixture + goldens (be208ec)
• Phase 2 — 8b implementation: pyramid.py, open_store order level, walk hardening, 38 tests (1c31bd7)
• Phase 4 — adversarial-review fold: 12 findings (2 major, 4 medium, 6 minor), one commit each (25f00dd…377fb1d)
• Phase 3 — docs: concepts section design→implemented (permalinks kept, discovery question resolved), api/pyramid.md, changelog (de55f2b)

Fixture provenance

tests/data/overview_hive — generated by tools/generate_overview_fixture.py run from a zagg checkout at main = 92a9f54 (the #344 merge; sha recorded in tests/data/overview_hive.golden.json), production write path end to end: build_manifest/ensure_manifest (pyramid declaration via build_pyramid_block), process_and_write_hive per source leaf, write_root_coverage, then the D22 second-pass sweep_overviews. Layout: a D19 multi-product root, SERC-area order-6 shards (7), parent 6 / child 8:

• atl06 (/1, pyramid: {orders: [4, 2]}): source /8 (7 leaves) + overviews /6 (4 objects at order-4 nodes) + /4 (1 object at 433);
• atl06_windows (/2, explicit 2019/2020 — 2020 covers one shard fewer — pyramid: {orders: [4], all_time: true}): per-window 2019.zarr/2020.zarr + all-time all.zarr folds at 4 nodes.

Goldens (node roster, per-order variables, per-object role/zagg_overview attrs, count totals, h_min minima) computed at generation time from the written bytes; generation asserts source↔overview count-total parity so a broken fold can never become a golden. 638 objects / ~241 KB.

Testing

tests/test_pyramid.py, 48 tests (518 passed / 1 skipped suite-wide, up from 470/1 at branch point; zero regressions). The review fold added: a synthetic northern+southern two-base-cell graft (row ordering vs the moc coordinate), an anonymous=True multi-product pin, window="all" refusal, grouped-store omission, AOI-independence of both the helpers and the §4.3 checks, a flat-node helper answer, malformed-cache-object drops (4 cases), the per-product store/MOC budget, and wall-clock-normalized golden comparison. Committed fixture bytes are unchanged (git diff -- tests/data empty). Gates: ruff check + ruff format --check clean; mypy clean on all new/touched modules (pyramid.py, open.py, store.py; the 6 pre-existing errors in products.py/moc_index.py/dggs.py are identical at the branch point in the same environment and don't surface in pre-commit's isolated env); codespell clean; mkdocs build --strict passes.

Questions for review

1. All-time folds on windowed products are not yet surfaced — and window="all" is now refused rather than half-working. The original claim here ("no reader call reaches them") was wrong: validate_label("all") passes, so it opened the all-time folds on the overview orders while the source order — which has no {shard}_all leaf — emptied, i.e. one tree whose source reported 0 cells beside overview orders summing every window. open_overview_order now raises a pointed error naming all as the reserved all-time token (§4.2, "excluded from the window grammar forever") and the folds as a deferred surface; the concepts page documents the gap for users. The opt-in surface (e.g. a kwarg on open_overview_order) is still deferred — say the word if it should land here. Related loose end, left as an 8a-scope fix: open_hive(product=…, window="all") still returns the issue-open_hive: AOI over empty coverage raises — decide vs schema-correct empty Dataset #4 empty dataset, and on a non-pyramid windowed product open_store(window="all") still empties quietly.
2. Grouped ancestor paths: the reader now names none rather than the wrong one. Review found the grouped branch was not just untested but not internally coherent — a grouped tree's directories exist only at multiples of path_grouping, so a non-boundary ancestor order has no node in it, and chunking the decimal produced a truncated group component (4/33 where the tree's component is 331). _node_rel is now the writer's convention verbatim (one component per digit, zagg.sweep._node_rel), correct at path_grouping: 1 — every store today — and open_overview_order refuses path_grouping > 1 with a warning naming the unsettled convention, omitting the order nodes while the source node still opens (§4.1: a reader MUST NOT require overviews). Pinned on the committed serc_hive_pg3 (path_grouping: 3) fixture. The zagg-side question stands: if mirroring the writer unconditionally is the wanted reader contract, dropping the guard is a one-line flip.
3. _objects_out as a private open_hive parameter (vs always recording the roster in every open_hive result): chosen to keep the 8a flat-node identity byte-stable (assert_identical pins exist against the old shape). Promoting it to a public kwarg or an always-on attr is a one-line flip if preferred.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FAKZ92TGiLCYKos2L6G1aw

[espg]

🤖 from Claude (review)

Fresh-context adversarial review of phase 8b (all three phases, 650 files). Environment: uv sync --extra test --extra xdggs, 508 passed / 1 skipped (38 in test_pyramid.py), ruff check + ruff format --check clean, mypy clean on pyramid.py / open.py / store.py. Fixture regenerated from englacial/zagg main = 92a9f54 with the committed generator.

Findings (12 inline)

Major

1. open_overview_order concatenates ancestor objects in decimal-string order while the index_kind="moc" coordinate comes from the packed-word MortonRanges union — the two disagree across base cells (sorted(["4331","-5111"]) vs word order), so rows are labeled with other cells' morton words on any north+south store. open_hive sorts packed words; this path lost that invariant. The single-base fixture can't catch it.
2. open_store mutates its own **store_kwargs (setdefault("anonymous", True)) inside the product loop → TypeError: got multiple values for keyword argument 'anonymous' on the next product. Reproduced on the committed fixture with open_store(R, window="2019", anonymous=True).

Medium
3. Deviation (a) is not a clean deferral: window="all" is reachable and yields a populated all-time overview node (count 2329 = 1347+982) beside an empty source node, with only the generic "intersects no coverage" warning.
4. Deviation (b): the grouped ancestor path is untested (no grouping test touches pyramid) and incoherent at path_grouping: 3 — _node_rel("433", 3) → 4/33, a truncated group component that is neither zagg's 4/3/3 nor a real grouped-tree node. Failure mode is silent overview loss under a "not yet swept, or deleted" warning.
5. source_orders / overview_orders return () and finest_source_at raises under an out-of-coverage AOI, because zagg_objects records only AOI-surviving objects — a store-structural question with a query-dependent answer. (Also () on a flat product node.)
6. §4.3 enforcement is query-dependent: a role: "composite" object raises with no AOI, is never validated when the AOI excludes it. Plus a severity question against §4.1's "regenerable caches, never load-bearing / a reader MUST NOT require them" — one malformed cache object currently fails the whole open_store, source nodes included.

Minor / nit
7. A genuine coarse source leaf at an overview order is structurally unreachable (candidates are {window}.zarr/all.zarr only; and ranges_words raises on a mixed-order root envelope) — so the documented mixture node cannot arise in a real store yet; the doctored test simulates it.
8. The golden embeds generated_at / max_leaf_timestamp verbatim, so those fields are pinned against themselves and fixture+golden can only be regenerated as a pair.
9. Per-order-node coverage.moc re-GET + fresh obstore handle (measured 5 / 6 for one two-product open); store= and the envelope are both already threadable.
10. int(block["cell_order"]) escapes as bare KeyError, unlike every sibling check's pointed ValueError.
11. Docs promise window= scoping without the all caveat; helper semantics under scoping / on flat nodes undocumented.
12. Generator docstring says "sum/min/min laws"; written provenance is sum/min/max.

Held up under scrutiny

• Fixture integrity. Regenerated at 92a9f54: 638 objects / 241 KB, identical file set (0 "Only in"), 50 differing files whose only differing JSON leaf keys are generated_at / max_leaf_timestamp / written_at; every zagg_overview.content_hash identical; golden logically equal after normalizing those keys. Real-writer end to end, as claimed.
• Goldens pin values, not echoes. Computed by direct zarr reads with an ancestor formula different from pyramid.py's (d[: len(d) - (s - k)] vs dec[: len(decimal_base(dec)) + k]) and paths from zagg's own sweep._node_rel. The source↔overview count-parity assertion is genuinely independent of the reader — a broken fold cannot become a golden.
• §4.5 binding. [], missing overview, no pyramid block, and the legacy {"orders": []} placeholder all read as declared-off → flat node, no error (regression-pinned on the multi-product fixture); non-empty orders strict-checks spec and requires spacing/all_time/fields with a test per absence; extra keys (materialized, per-field nan_policy) tolerated. Matches the §4.5 text clause for clause.
• Role per object, never per node. _object_entry classifies each object independently and never synthesizes a node-level role; the mixture test is discriminating (role flip moves order 6 into the source set while it stays in the overview set, and finest_source_at(node, 7) follows) and the helpers key on entries, not node names. Option A falls out of the real fixture (h_mean at /8, absent at /6 and /4), not from doctoring. Computed-compose-never-a-node pinned against the declaration.
• Walk hardening. _shard_leaf_name correctly rejects all.zarr/{window}.zarr in both consumers — verified _candidate_leaves (test_walk_open_hive_skips_overview_objects) and _schema_leaf by hand on a MOC-less copy (returns 4/3/3/1/2/4/4/4331244.zarr). overview.rollup.json never enters the walk at all.
• read_leaf_metas single-GET refactor. Stamp and role attrs come from one payload; read_commits is a thin map over it with unchanged semantics (None for absent, raise on unparsable, serial path preserved via read_json). Counting-store test confirmed: zero morton/cell_ids chunk GETs, and open_store's chunk set equals the wrapped per-node opens'.
• #27 (claude/19-ragged-hhdc) interaction — no semantic collision. That branch touches ragged.py / hhdc.py / stats.py only; it does not touch open.py or store.py, so there are no competing walk changes. Its hash_arrays debris tolerance operates inside a leaf prefix (list_prefix + node_type probe, skipping the in-leaf coverage.moc sidecar) while this PR's hardening operates on digit-tree basenames — orthogonal layers, and neither weakens the other. A trial merge produces exactly two textual conflicts, both mechanical: CHANGELOG.md (adjacent Unreleased entries) and src/moczarr/__init__.py (adjacent import + __all__ insertions). Flagging, not resolving.
• Repo conventions. CHANGELOG entry, mkdocs.yml nav, docs/api/pyramid.md, root exports + __all__ (sorted), concepts page design→implemented with permalinks kept. read_leaf_metas staying out of the root namespace is consistent with read_commits doing the same. 8a identity is pinned via assert_identical against open_hive with the golden roster injected, which also proves the default open_hive result carries no zagg_objects — the _objects_out privacy question (3) is answered by that test.

On the open questions: (3) _objects_out looks right as-is for the reason given — the pin above is exactly what would break if the roster were always-on. (1) and (2) are the two I'd push back on, per findings 3 and 4: the deferrals are defensible, the current behavior under each isn't quite inert.

[espg]

🤖 from Claude (review)

Major — rows are mislabeled on any store spanning northern and southern base cells: the concat order is decimal-string order, the moc coordinate is packed-word order.

ancestors is sorted({...}) over decimal strings, and that list drives both rels and the for dec, rel, meta in zip(...) loop — so xr.concat(opened, dim=dim) (line 359) lays rows down in string order. The coordinate, meanwhile, comes from the accumulated domain (line 307) via MortonMocIndex (line 364), which is packed-word ascending. Those two orders disagree across base cells, because a leading - sorts before every digit as a string but after every northern base cell as a word:

$ # two order-3 ancestors, one northern one southern
open_overview_order concat order (sorted decimals): ['-5111', '4331']
packed words -> word order:                        ['4331', '-5111']
fabricated coord first/last:                       433111 ... -511144
coordinate is ascending-by-word:                    True

So on index_kind="moc" (the default) node /6 would hand -5111's data rows the 4331… cell labels. open_hive does not have this bug — it sorts packed words (open.py:117, [leaf_path(int(w), …) for w in np.sort(words)]), which is exactly the invariant this path drops when it goes through decimals.

The fixture is single-base (4), so nothing catches it; the generator itself builds layout="fullsphere", and a global ATL06 product (or the 88S-plus-CONUS pairing in zagg's own benchmark set) crosses the boundary immediately. Suggested fix: sorted(candidates, key=lambda d: int(morton_word(d))), plus a test with one northern and one southern ancestor asserting pandas and moc agree on morton — test_pandas_index_kind_matches_moc is the right test, it just needs a two-base fixture to bite.

Per spec §4.4 the coordinate is the truth for row identity, and §4.3's off-order check exists precisely because "off-order objects would mis-rank rows" — this is the same failure mode arriving by a different door.

[espg]

🤖 from Claude

Fixed in 25f00dd. Confirmed the mechanism before touching anything — morton_word('-5111') is 12682136550675316739 vs morton_word('4331') = 5332261958806667267, so the decimal sort inverted the pair exactly as you described. open_overview_order now sorts the ancestor set by packed word, the invariant _candidate_leaves already keeps (open.py:117), with the reason in a comment naming §4.4's coordinate-is-row-identity rule.

The committed fixture is untouched (git diff -- tests/data is empty). The two-base case is synthetic instead, grafted in tests/test_pyramid.py::_two_base: one southern order-6 shard appended to atl06/coverage.moc, and a copy of 43312's overview object at -5/1/1/1/1 whose morton array is rewritten to the node's own 16 order-6 descendants — computed from the digit grammar (rank_tail), not from moczarr.ranges, so the stored rows are an independent check on the fabricated coordinate.

Two pins in TestMultiBaseOrdering:

• test_objects_follow_packed_word_order — ['43312', '43314', '43321', '43323', '-51111'], and the word list is ascending.
• test_moc_coordinate_labels_the_right_rows — pandas-stored morton vs moc-fabricated morton equal; the southern node's 16 descendants are the last rows; and count[-16:] == count[:16], since the grafted object is a copy of the first, so the data is pinned to ride with the right labels too.

Both fail on the pre-fix code (verified by removing key=morton_word): ACTUAL 12682136550675316742… / DESIRED 5336765558434037766… — the southern rows wearing northern labels.

[espg]

🤖 from Claude (review)

Major — store_kwargs.setdefault mutates the function's own **kwargs inside the product loop, so anonymous=True on a multi-product pyramid store raises TypeError on the next product.

Reproduced against the committed fixture:

>>> open_store("tests/data/overview_hive", window="2019", anonymous=True)
TypeError: moczarr.open.open_hive() got multiple values for keyword argument 'anonymous'

Mechanism: this line runs only for a product that has a pyramid, and it writes "anonymous" into the same dict that line 750's open_hive(..., anonymous=anonymous, **store_kwargs) splats on every subsequent iteration. atl06 (pyramid) precedes atl06_windows, so the second product blows up. A single-product pyramid store survives by luck.

This is the primary remote-access mode (public S3 buckets — anonymous is documented in open_hive's signature for exactly that), and no test in the suite passes anonymous= to open_store. Suggested fix: don't mutate — pass anonymous=anonymous explicitly to open_overview_order (it forwards **store_kwargs into _resolve_store → open_object_store) and leave store_kwargs alone; add an open_store(..., anonymous=True) pin on the two-product fixture.

[espg]

🤖 from Claude

Fixed in e8ffdab; reproduced your TypeError against the committed fixture first.

open_overview_order now takes anonymous: bool = False as a real keyword (mirroring open_hive's signature and its store_kwargs.setdefault on its own fresh dict), and open_store passes anonymous=anonymous instead of writing into the dict it splats every iteration. eca4beb then supersedes the mechanism entirely — the order level now gets store=product_store, a handle open_store builds once per product with anonymous=anonymous — so there is no path left where store_kwargs is mutated.

Pinned by TestTreeShape::test_anonymous_survives_the_product_loop on the two-product fixture: open_store(root, window="2019", anonymous=True) reaches both products' children, and the order-6 node is assert_identical to the signed open. Comment on the test names the mechanism (the next product's open_hive splat) so it doesn't get "simplified" back.

[espg]

🤖 from Claude (review)

Medium — the flagged deferral (question 1) is not clean: window="all" is reachable, and it produces a populated all-time overview node beside an empty source node.

The PR body says "no reader call reaches them". It does:

>>> open_store(R, products=["atl06_windows"], window="all")
children ['8', '6']
   8 cells= 0   count= 0
   6 cells= 64  count= 2329        # = 1347 (2019) + 982 (2020), the all-time fold
warnings: ["window 'all' intersects no coverage at …/atl06_windows; returning a…"]

validate_label("all") passes, so basename becomes all.zarr and this branch happily opens the all-time folds, while open_hive on the same window="all" finds no {shard}_all.zarr leaves and takes the issue-#4 empty path. The user gets one tree whose source order reports 0 and whose overview order reports the sum over every window — with only the generic "intersects no coverage" warning to explain it. That is the "silently mislead" case the deferral was supposed to avoid, not the "nothing reaches it" case.

Cheapest clean deferral, no new grammar invented: on a windowed product refuse window == ALL_TOKEN here with a pointed error naming it as the reserved all-time token and not yet a supported reader surface (spec §4.2 already declares all "excluded from the window grammar forever", so refusing it is spec-aligned, not an invention). That keeps question 1 open for a follow-up without leaving a live misleading path.

[espg]

🤖 from Claude

Agreed the deferral wasn't clean, and taken your cheapest-clean-deferral option verbatim in db1f63f: open_overview_order refuses window == ALL_TOKEN on a windowed product with a pointed error that names all as the reserved all-time token (§4.2, "excluded from the window grammar forever"), says the folds exist on disk (pyramid.overview.all_time, §4.5), and says why they aren't a surface yet — no all-time leaf on the source axis to pair them with.

TestDegradation::test_all_time_token_is_refused pins both entry points (direct open_overview_order(..., window="all") and open_store(..., window="all")).

Two things I left standing, both flagged rather than silently changed:

1. open_hive runs before the order level in open_store, so its pre-8b "intersects no coverage" warning still fires on the way to the raise (the test asserts both). Refusing all on the source axis too would be an 8a change outside this diff — open_hive(product=…, window="all") still returns the empty dataset, and on a non-pyramid windowed product open_store(window="all") still empties quietly. Worth its own small fix; say the word and I'll fold it here instead.
2. The PR-body claim is corrected (question 1 now reads "reachable and refused", not "no reader call reaches them"), and the gap is documented for users on the concepts page rather than only in the PR body — your comment on docs/concepts.md, folded in 377fb1d.

[espg]

🤖 from Claude (review)

Medium — the grouped ancestor path (question 2's seam) is both untested and, at path_grouping > 1, not a well-formed grouped-tree path.

No test in the suite exercises this function with path_grouping != 1 (grep path_grouping tests/ hits only test_path_grouping.py / test_convention.py / test_stats.py, none of which touch pyramid), so at grouping 1 the grouping argument is dead code. Worse, when it is exercised it produces a third convention that matches neither the writer nor the tree:

leaf at grouping 3:      4/331/422/41/433142241.zarr
ancestor order 2 (433):  moczarr g3 -> '4/33'      zagg writer -> '4/3/3'
ancestor order 4 (43314):moczarr g3 -> '4/331/4'   zagg writer -> '4/3/3/1/4'

4/33 is a truncated group component — the real component at that depth is 331, so 4/33 is not a node of the grouped tree at all, and 4/331/4 collides with a sibling name space (422). The PR body frames this as "the two would diverge"; the sharper statement is that this side is not internally coherent either, so mirroring zagg's per-digit _node_rel would actually be the more defensible reader contract, not less.

Behavioral consequence today: on a grouped pyramid store every read_leaf_metas lookup misses, schema_rel stays None, and the node is dropped with the line-323 warning that says "not yet swept, or deleted" — a misattributed diagnostic for what is really a path-convention mismatch. Whichever way the zagg-side question resolves, this deserves either a grouping test or a loud NotImplementedError for path_grouping > 1 so the failure isn't silent + misdescribed.

[espg]

🤖 from Claude

Fixed in 8c54842 — and you're right that this side wasn't internally coherent either, which decided it: I removed the invented convention rather than shipping a third one.

_node_rel lost its path_grouping argument and is now the writer's convention verbatim (zagg.sweep._node_rel, one component per digit), which at path_grouping: 1 — every store that exists — is the grouped tree's own node path. And open_overview_order now refuses path_grouping > 1 before any I/O, with a warning that states the actual obstruction: a grouped tree's directories exist only at multiples of the grouping, so a non-boundary ancestor order has no node in it (4/33 is a truncated component where the tree's is 331), while the sweep writes per digit regardless.

I chose warn + omit the order nodes over NotImplementedError, deliberately, on §4.1 grounds: overviews are "regenerable caches, never load-bearing… a reader MUST NOT require them", so a raise would make a grouped store's perfectly readable source data unopenable through open_store because of a cache-path question. Omission is also the posture the module already takes twice (no usable root MOC → warn + omit; declared-but-unswept → warn + omit), and it fixes the misattributed diagnostic you found: the "not yet swept, or deleted" warning no longer fires for what is a path-convention mismatch.

Tested on the committed grouped fixture: TestDegradation::test_grouped_store_omits_order_nodes doctors a zagg-pyramid/1 declaration onto tests/data/serc_hive_pg3 (path_grouping: 3), asserts the path_grouping 3 warning, and asserts the product keeps its source child with rows (["10"], non-empty) — the §4.1 point made executable.

The zagg-side question (PR-body question 2) stands unchanged for @espg: whichever way it resolves, the reader now names no path rather than the wrong one. If mirroring the writer unconditionally is the preferred contract, that's a one-line flip (drop the guard) plus a grouped fixture test.

[espg]

🤖 from Claude (review)

Medium — the selection helpers collapse to () under AOI/window scoping, because attrs["zagg_objects"] only records objects that survived the AOI.

>>> t = open_store(R, products=["atl06"], aoi=["-5111"])   # out-of-coverage AOI
children        ['8', '6', '4']
source_orders   ()
overview_orders ()
node_objects(t["atl06"]["6"])  ->  []

The tree shape is (correctly, per the issue-#4 posture) unchanged, but the helpers now answer "this product carries no source and no overviews", and finest_source_at raises ValueError("no source order …"). Since these are the documented selection API for the level (concepts page: "every stored order carrying at least one object of that role"), a query-dependent answer to a store-structural question is a footgun — the same store answers (8,)/(6, 4) or ()/() depending on the AOI passed at open.

Root cause is the continues at lines 289 and 313 firing before entries.append(...) at line 316. Two candidate fixes: record the entry for every stamped object and let the AOI govern only rows (which also makes the §4.3 checks AOI-independent — see the separate comment on _object_entry), or keep the roster query-scoped but say so in the helper docstrings and add a pin for the empty-AOI answer. Related, and worth one line of docs either way: on a flat (non-pyramid) product node these return () too, since the node has no children — source_orders(flat_node) == () reads as "no source", not "not a pyramid".

[espg]

🤖 from Claude

Took your first candidate fix in faf9821 — record the entry for every stamped object and let the AOI govern only rows — since it also makes the §4.3 checks AOI-independent (your _object_entry comment, folded in 78cd3f8). The continues stay where they are; the roster and its validation now run above them.

There was a second collapse the first fix didn't reach, which is why this took two changes:

• open_hive's _objects_out recording moved above the AOI rejects too (same reasoning).
• But the source node's roster can be legitimately empty however early it's recorded: _candidate_leaves prunes leaves at the root MOC ∩ AOI, so a non-intersecting AOI leaves no stamp to key on at all. So _orders_with_role now unions in the product's own morton_hive.cell_order when that order node exists — the leaf level carries source by definition, which is a manifest fact rather than a query result. No extra I/O.

Flat nodes now have a defined answer too, as you asked: no order children means the product node itself holds the data, so source_orders is its own cell order and overview_orders is (). ()/() reading as "no source data" is gone.

Pins: test_helpers_are_structural_not_query_scoped (out-of-coverage AOI → 0 cells but (8,)/(6, 4), finest_source_at(8) == 8, and node_objects byte-equal to the unscoped open) and test_helpers_on_a_flat_product_node (on multiproduct_hive). Docstrings on _orders_with_role/source_orders/overview_orders now say "structural, not query-scoped" explicitly, and the concepts page says it for users (377fb1d).

[espg]

🤖 from Claude (review)

Minor — the order level re-GETs coverage.moc and rebuilds an obstore handle per node, against issue #5's "ONE store construction pair for the whole open".

Measured on the committed two-product fixture (open_store(R, window="2019"), obstore get/get_async recorded):

store constructions:    6      # root + 2 open_hive + 3 order nodes
coverage.moc GETs:      5      # 2 from open_hive + 1 per order node
morton_hive.json GETs:  4
chunk GETs:             150    # data vars at concat, as documented

The PR's claim — "the tree layer adds zero extra object fetches per candidate" — holds, and test_tree_layer_adds_zero_chunk_gets pins the part that matters (verified: zero morton/cell_ids chunk GETs, and the store-vs-direct chunk sets match). But the summary reads as "zero extra fetches", and the sidecar tier grows linearly in declared orders. Both are already avoidable without new API: open_overview_order accepts store= (unused by this call site), and load_root_coverage's envelope is per product, so one read could be threaded through all of that product's order nodes. On a 5-order pyramid over S3 that's 5 round-trips traded for 1.

[espg]

🤖 from Claude

Both avoidable costs taken, in eca4beb. open_store now builds the product's obstore handle once and reads its root MOC once, threading store=product_store (the unused store= you spotted) plus a private _envelope= — the _objects_out precedent for a private threading hook in this same PR. open_overview_order's own load_root_coverage stays as the default for direct callers, and _envelope=None means "not supplied", so the degraded no-MOC case still re-reads and warns per order exactly as before.

Measured on the fixture, open_store(root, products=["atl06"]) with two declared orders: store constructions 4 → 3 (root + open_hive's + one for the whole order level), coverage.moc GETs 3 → 2. TestLaziness::test_order_level_costs_one_store_and_one_moc_read pins both counts and asserts len(overview_cell_orders(manifest)) == 2, so the point being pinned is that neither number grows with declared orders — your 5-order-over-S3 case.

Your correction to the summary is taken too: the PR body now says the order level adds no chunk GETs and a bounded metadata tier (one store + one MOC read per product), not "zero extra fetches".

[espg]

🤖 from Claude (review)

Minor — the golden roster embeds the non-reproducible fields of zagg_overview, so node_objects(...) == golden["objects"] is a self-echo for those fields and the fixture can only ever be regenerated wholesale.

I regenerated from zagg main = 92a9f54 (the recorded sha) with the committed generator. Result is reproducible to the byte except for timestamps — worth recording, since it's the strongest thing about this fixture:

638 objects, 241 KB   (identical count/size to the committed tree)
diff -rq: 50 differing files, 0 "Only in" either side
differing JSON leaf keys, across all 50: ['generated_at', 'max_leaf_timestamp', 'written_at']
zagg_overview.content_hash: identical on every overview
golden.json: raw equal False; equal after normalizing those keys: True

So the only obstacle to a byte-stability check is that objects[].zagg_overview is stored verbatim, generated_at / generation.max_leaf_timestamp included, and test_role_is_per_object compares it with ==. Two consequences: (1) those two fields are pinned against themselves, contributing no signal; (2) a fixture refresh always dirties the golden, so the pair can never be regenerated independently to check drift. Normalizing the timestamps out here (or comparing everything except them in the test) would make the golden all-signal and make "regenerate and diff" a usable drift check.

Separately, and this is the part that held up best: the parity assertion is computed independently of the code under test — direct zarr.open_group reads, ancestor prefixes derived by a different formula than pyramid.py's (d[: len(d) - (shard_order - k)] vs dec[: len(decimal_base(dec)) + k]), and paths from zagg's own sweep._node_rel. A broken fold genuinely cannot become a golden.

[espg]

🤖 from Claude

Folded in 6c8c973, test-side, so both the fixture and the golden keep their committed bytes (git diff -- tests/data empty).

tests/test_pyramid.py gains _without_wallclock and drops exactly the two paths your diff identified — zagg_overview.generated_at and zagg_overview.generation.max_leaf_timestamp — from both sides of the test_role_is_per_object comparison, then asserts they are still present and ISO-shaped. So the fields remain required (a sweep that stops stamping them fails) while contributing no self-echo, and "regenerate, diff the golden" becomes a usable drift check. written_at doesn't appear in these entries (it rides the leaf commit stamps), so it needed no handling here.

Your reproducibility measurement is now recorded in tools/generate_overview_fixture.py's docstring — byte-reproducible at a fixed zagg sha except those wall-clock fields, content_hash stable — since that's the strongest property of this fixture and it was nowhere in the tree.

I did not strip the timestamps in the generator: that would rewrite golden.json and dirty committed bytes for a change with no reader-visible effect. If you'd rather the golden not carry them at all, that's a regeneration and I'd want it to be a deliberate one.

Noted on the independence of the parity assertion — that's exactly why I left the generator's ancestor-prefix formula different from pyramid.py's rather than "de-duplicating" them.

[espg]

🤖 from Claude (review)

Nit — "(sum/min/min laws)" should be sum/min/max. The written provenance agrees with the latter (golden.json → atl06.overviews.6.all.objects[0].zagg_overview.fields):

"count":  {"class": "exact", "method": "sum", "nan_policy": "skip"},
"h_min":  {"class": "exact", "method": "min", "nan_policy": "skip"},
"h_max":  {"class": "exact", "method": "max", "nan_policy": "skip"}

Harmless in itself, but this docstring is the fixture's spec for a reader who doesn't have zagg installed, so a wrong fold law in it is the kind of thing that gets believed.

[espg]

🤖 from Claude

Fixed in a0264f4 — sum/min/max. You're right about why it matters: that docstring is the fixture's spec for anyone without zagg installed, and the written provenance you quoted (h_max → method: "max") is the truth.

[espg]

🤖 from Claude (review)

Nit — the option-B test is discriminating for the right rival, but what it directly pins is xarray's concat fill, not option-B tolerance.

It does rule out the plausible wrong implementation (filtering each node's variables against the manifest's pyramid.overview.fields, which would drop h_extra) — so it earns its place. But h_extra is injected into 1 of the 4 objects at node /6, so "h_extra" in ds.data_vars is satisfied by xr.concat broadcasting the variable across the other three with NaN. A reader can't tell from the assertion whether the node genuinely carries an option-B variable or whether 3/4 of its cells are fill.

Two cheap sharpenings: inject into all four objects (then the variable is real at every cell of the node), and/or assert the shape/finite-count so the concat semantics are stated rather than relied on. Same test, one more line, and it pins the declared-derived-summary case instead of the concat case.

[espg]

🤖 from Claude

Both sharpenings taken, in 15fae77. h_extra is now injected into all four objects of node /6, and the assertions state the semantics instead of relying on them:

extra = tree["atl06"]["6"].ds["h_extra"]
assert extra.sizes["cells"] == 64
assert int(np.isfinite(extra.values).sum()) == 64

So the variable is real at every cell of the node rather than 48/64 concat fill, and the test now pins the declared-derived-summary case. I kept the rival it already ruled out (h_extra absent at /8) and added one line making the manifest angle explicit — "h_extra" not in overview_declaration(...)["fields"] — so it's clear the node carries a variable the declaration never mentions, which is the option-B shape.

[espg]

🤖 from Claude (review)

Minor — user-facing docs promise more of window= than the reader delivers, and the all-time gap is recorded only in the PR body.

"window= scopes each order node to that window's {window}.zarr overviews" is true for 2019/2020 and actively misleading for all, which reaches the all-time folds on the overview nodes while emptying the source node (see my comment on pyramid.py's basename branch). The PR body's question 1 is the right place to ask whether to surface all-time folds; it isn't the place the gap gets documented for users, since the PR body isn't shipped.

Worth one sentence in this section either way the question resolves: that a windowed product's all.zarr all-time folds exist on disk (pyramid.overview.all_time, spec §4.5) and are not yet a reader surface. Same for the flat-node behavior of the helpers noted on _orders_with_role — the concepts page introduces source_orders/overview_orders as "every stored order carrying at least one object of that role", which reads as store structure, not as a query-scoped answer.

[espg]

🤖 from Claude

Folded in 377fb1d — the gap is documented where users read it, not only in the PR body.

docs/concepts.md, after the window= sentence you quoted:

window= takes a declared window label only. The reserved all-time token "all" is refused: a windowed product's all-time folds do exist on disk (pyramid.overview.all_time, spec §4.5 — all.zarr at each ancestor node) but they are not yet a reader surface […] all is excluded from the window grammar forever (§4.2), so the eventual surface will be its own opt-in rather than a window label.

Plus, in the same paragraph, that aoi=/window= scope rows and never the helper answers — the query-scoped reading you flagged on _orders_with_role. The source-order section now says the helpers answer about the store, are unaffected by aoi/window, and are defined on a flat product node (source_orders = its own cell order, overview_orders = ()), so an empty overview_orders always means "stores no overviews".

Two more docs changes rode along, since the code moved under them: the role section now describes the two §4.3 severities (uninterpretable cache object → warn + drop; off-order → raise, per §4.1) instead of "all raise", and the degradation section gains the path_grouping > 1 omission. CHANGELOG.md updated to match. mkdocs build --strict passes.

[espg]

🤖 from Claude

Coordination note: englacial/zagg#346 has merged (rebase merge — the spec now lives on zagg main, tip c0493ba; the branch-era shas this PR's provenance cites — spec vectors @ 166e908, composition fixture pin @ 555874b — are historical). Per the pin's own recorded process, re-confirm the vendored fixtures byte-equal against zagg main and update the pin text to the main sha. The merge also added a normative §1.5 "Subtree spans" clause (contiguous-span subtree reads; zagg-side reader is englacial/zagg#351, moczarr counterpart is #29) — text-only relative to what this PR vendors, no fixture bytes changed.

[espg]

🤖 from Claude

Re-pin done ahead of merge (espg-directed): 8e9d589 — the composition provenance (src/moczarr/composition.py §3 reference, tests/test_composition.py fixture pin) now points at englacial/zagg main @ c0493ba (the #346 rebase merge); the branch-era shas 166e908/555874b are noted as historical. The vendored composition array was cmp-verified byte-identical against zagg main before the text changed. tests/test_composition.py: 49 passed.

The edit is byte-identical to the same-file edit on PR #27, so whichever merges second rebases clean. This PR's own tests/data/overview_hive fixture provenance (zagg main @ 92a9f54) was already a main sha and is untouched.

[espg]

🤖 from Claude

PR #27's merge left this branch conflicting; resolved by merging main in: df7d560. Both conflicts were the both-PRs-append kind and resolved as unions — src/moczarr/__init__.py keeps the pyramid exports (this PR) and the ragged/hhdc exports (main), alphabetized; CHANGELOG.md keeps both Unreleased entries (this PR's first). The shared composition-pin edit merged clean automatically, as designed. Full suite on the merged branch: 587 passed, 29 skipped; ruff clean. Mergeable again from my side.

[espg]

🤖 from Claude

Follow-ups from the two review questions are filed, so the gaps survive the merge:

• Refuse the reserved window="all" token uniformly across open_hive / open_store / open_overview_order #30 — refuse the reserved window="all" token uniformly across all three entry points (this PR raises in open_overview_order; open_hive / non-pyramid open_store still empty quietly).
• All-time fold opt-in surface for windowed pyramid stores (gated on the source-axis ruling) #31 — the all-time fold opt-in surface, gated on the source-axis ruling (what a source node means under all-time: overviews-only vs computed fold vs concat). Per espg's review here, the opt-in deliberately does NOT land in this PR.
• Rule the sweep-artifact path convention on path_grouping > 1 trees (spec + fixtures) englacial/zagg#361 — the grouped-tree (path_grouping > 1) sweep-artifact path convention: the writer's one-component-per-digit behavior is accidental, the ruling has spec+fixture implications, and this PR's pg>1 guard stays until it lands (the recorded one-line flip follows the ruling, not precedes it).