Post-#463 fixture re-vendor sweep: whole-tree parity at zagg d52e3063, temporal companion fixture + spec-pin advance - #44
Conversation
| "pyramid": { | ||
| "orders": [], | ||
| "aggregation": {} | ||
| "spec": "zagg-pyramid/2", |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — the vendored zagg-pyramid/2 block reads as "pyramid declared off", and the mandated strict-check never fires.
Both re-vendored manifests now carry:
"spec": "zagg-pyramid/2",
"overviews": [ {"node": 4, "cells": [5]}, … ],
"overview": { "all_time": false, "fold_source": "cascade", "exact_levels": 1, "fields": {…} }moczarr.pyramid.overview_declaration branches on the /1 schedule key before the spec check (src/moczarr/pyramid.py:104):
overview = block.get("overview")
if not isinstance(overview, dict) or not overview.get("orders"):
return None
spec = block.get("spec")
if spec != PYRAMID_SPEC: # PYRAMID_SPEC = "zagg-pyramid/1"
raise ValueError(...)A /2 block has no overview.orders (the schedule moved to block-level overviews), so the early return wins and the spec strict-check is unreachable. Measured on this PR's fixture:
overview_declaration: None
overview_cell_orders: {}
pyramid keys: ["spec", "overviews", "overview"]
overview keys: ["all_time", "fold_source", "exact_levels", "fields"]
Two problems: (1) a store that does declare a pyramid is read as declared-off, so open_store silently drops the resolution axis; (2) the function's own docstring promises "spec is strict-checked (fail loudly on an unknown revision — the conformance rule)" and it now demonstrably does not — silent degrade is exactly what the strict-check exists to prevent. zagg spec §4.5 at d52e3063 is explicit about the order: "A reader MUST branch on spec first, then on the revision's schedule key: under /1 that is orders … and under /2 it is the block-level overviews."
Adopting the /2 view is legitimately out of scope here (#36/#37, per the PR body). Shipping the fixture that makes the gap reachable without recording it is the part worth fixing — suggest one of:
- minimal fix in this PR: raise when
blockcarries aspeckey that is notPYRAMID_SPEC(guarding it that way keeps the legacy{"orders": [], "aggregation": {}}placeholder — which has nospec— returningNone), plus a regression test overtests/data/spec/minimal/morton_hive.json; or - if the degrade is deliberate for now, pin it with a test asserting
overview_declaration(minimal_manifest) is None, correct the docstring's fail-loudly claim, and list it under "Questions for review" — nothing in the tree currently records that the vendored fixtures are/2while the reader implements/1.
There was a problem hiding this comment.
🤖 from Claude
Folded as remedy (b) — the degrade stays, now pinned and honestly documented — in c0f46cc.
Why not (a): raising on a non-zagg-pyramid/1 spec would make open_hive raise on every store zagg writes today. Since the englacial/zagg#384 default flip, /2 is the default declaration for any store whose resolved chunk order is interior — all three vendored §7 vectors carry it — and open.py reaches overview_declaration through overview_cell_orders on every open. Spec §4 is explicit that overviews are derived artifacts a reader MUST NOT require, so losing the ability to read a conforming store's SOURCE data is a strictly worse conformance outcome than declining to bind its overview view. There is also no mis-parse to prevent: a /2 block holds no /1 key this function would bind wrongly (overviews is block-level; the overview family dict carries no orders), so the answer None is a declared-off view, not a wrong reading of a /1 schedule.
What changed in c0f46cc:
src/moczarr/pyramid.py— the docstring now states that this function binds the/1schedule key only; that a/2block therefore reads asNonewith the/1strict-check unreachable by construction; why that is a view and not a mis-parse; and that binding (and strict-checking)/2is expose the zagg-pyramid/2 fixed ladder as a multiscale / datatree view #36/Design: the xarray data model for multiresolution /2 stores (DataTree vs resolution dimension vs custom multi-res index) #37, with a pointer to the new test.tests/test_pyramid.py::TestDeclarationBinding::test_vendored_v2_block_reads_as_no_family— a regression test over the vendored/2manifests (minimal,temporal,kitchen_sink) assertingspec == "zagg-pyramid/2", a non-empty block-leveloverviews, nooverview.orders, andoverview_declaration(...) is None/overview_cell_orders(...) == {}. The degrade is now a pinned decision rather than an accident: expose the zagg-pyramid/2 fixed ladder as a multiscale / datatree view #36/Design: the xarray data model for multiresolution /2 stores (DataTree vs resolution dimension vs custom multi-res index) #37 has to walk past a red test to change it.
The phase-2 assertion in test_spec_conformance.py::TestTemporalCompanion::test_manifest_reclassifies_the_companioned_digest stays as is — same expectation reached from the manifest-decode side, and the dedicated test now carries the rationale.
| "cells_with_data_order": 5, | ||
| "generated_at": "2026-08-10T11:57:30+00:00" | ||
| }, | ||
| "morton_hive_commit": { |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — store.walk_leaves now returns this column as a leaf, and its commit stamp means the callers' completeness check does not catch it.
Measured on the tree this PR vendors:
>>> list(store.walk_leaves("tests/data/spec/minimal"))
["1/1/2/1/3/11213.zarr", "1/1/2/1/3/all.pyramid.zarr"]
>>> store.read_commit(root, "1/1/2/1/3/all.pyramid.zarr")
{"spec": "morton-hive/1", "complete": True, "cells_with_data": 3, "granule_count": 1, ...}
_classify_children (src/moczarr/store.py:408) yields any *.zarr child as a leaf, and walk_leaves is public/exported with the documented contract "Yields stamped and debris leaves alike — completeness is the caller's check (read_commit)". That check does not settle this one: the column carries morton_hive_commit with "complete": true (the block on this line), so a caller following the documented contract gets a bona fide extra "leaf" reporting 3 cells of data.
zagg spec §4.6 at d52e3063 makes this normative, not stylistic:
The
.pyramid.zarrsuffix is the one name seam, and it is normative for name-grammar consumers (e.g. the root-MOC walker): a basename ending in.pyramid.zarrMUST NOT be read as a leaf or an overview.
What holds today is incidental rather than pinned: candidate_leaves and _schema_leaf filter via _shard_leaf_name, which rejects all.pyramid.zarr only because "all" is not a morton decimal (I checked 2019.pyramid.zarr, 1234.pyramid.zarr and 1121.pyramid.zarr too — all None, so the seam does hold for the windowed spellings). The equivalent hardening for §4.2 overview objects is pinned — tests/test_pyramid.py::test_walk_open_hive_skips_overview_objects and test_no_root_moc_omits_overview_nodes — but there is no test for the §4.6 column, and these fixtures are the first in-tree store that has one.
Concretely: add the suffix rule to _classify_children (skip basenames ending .pyramid.zarr, matching the §4.6 MUST NOT), and a regression test over tests/data/spec/minimal asserting walk_leaves and candidate_leaves agree on the single leaf. Without it the PR body's "no reader behavior changes" is true of src/ but not of what the discovery walk answers on the vendored trees.
Related, informational: this object also carries "role": "column" (line 3), a third value beside the source/overview vocabulary moczarr.pyramid classifies on (ROLE_ATTR, _orders_with_role). Not reachable today — nothing names this object — but worth knowing before #36/#37.
There was a problem hiding this comment.
🤖 from Claude
Folded in 7a49faa — the §4.6 name seam is now enforced at the classification step, not incidentally downstream.
What changed:
src/moczarr/convention.py— newPYRAMID_COLUMN_SUFFIX = ".pyramid.zarr", documented as spec §4.6's one normative name seam (unambiguous because the frozen D23 label charset admits no.), living besideALL_TOKENwhere the rest of the basename grammar lives.src/moczarr/store.py::_classify_children— a basename ending in that suffix is skipped before the*.zarr-is-a-leaf rule, with the §4.6 citation in the docstring and the reason the seam has to do the work alone: a column is commit-stamped like a leaf, soread_commitwould wave it through.walk_leaves' public contract paragraph now says so explicitly, since the old wording ("completeness is the caller's check") is what pointed callers at a check that cannot settle this case.tests/test_pyramid.py::TestDegradation::test_walk_open_hive_skip_the_leaf_column— pins it against the real vendored tree, mirroringtest_walk_open_hive_skips_overview_objects: asserts the column exists and stampscomplete: true, thenlist(walk_leaves("tests/data/spec/minimal")) == ["1/1/2/1/3/11213.zarr"]andopen_hive(...).sizes["cells"] == 16. Theminimalfixture carries no rootcoverage.moc, so thatopen_hivereally is the walk-driven path, not the arithmetic-candidate one.
Left alone deliberately: candidate_leaves / _schema_leaf keep filtering via _shard_leaf_name — that path is now belt-and-braces rather than the only thing holding, and tightening it further would change no answer.
Gates after the fix: 840 passed, 2 skipped; ruff check / ruff format --check clean on src tests tools.
| (zagg #420), the `zagg-pyramid/2` manifest declaration, and the §4.6 leaf | ||
| column surface (`all.pyramid.zarr` + `all.pyramid.stats.json`, | ||
| `granules.json`) vendored beside each leaf. Every array byte the conformance | ||
| suite decodes is unchanged; no reader behavior changes in this entry. |
There was a problem hiding this comment.
🤖 from Claude (review)
nit — two things in this entry.
-
no reader behavior changes in this entryis true ofsrc/, but not of what the reader answers on the vendored trees:store.walk_leaves("tests/data/spec/minimal")went from one path to two (the newall.pyramid.zarrcolumn, stampedcomplete: true). Suggest "nosrc/changes" or dropping the clause — see the separate finding on the column. -
The bullet immediately below (line 22) still tells downstreams "A downstream re-checking against its own copy of the zagg vectors must refresh from zagg
mainatb9347561or later." Both bullets sit in the same## Unreleasedsection, so the section now carries two different refresh instructions and only one of them reproduces what is vendored here. Worth a short "supersedes theb9347561refresh instruction below" clause on the new bullet (or striking the now-stale sentence from the authalic bullet), so a downstream reading the release notes lands ond52e3063.
(§4.6 for the leaf column and §4.5 for the pyramid block both check out against zagg docs/specification.md at d52e3063 — the section numbers in this entry are right.)
There was a problem hiding this comment.
🤖 from Claude
Both folded in 42b37e4.
- The "no reader behavior changes in this entry" clause is gone. The bullet now records the change the vendored columns forced:
store.walk_leavesapplies zagg spec §4.6's normative name seam — a basename ending.pyramid.zarris a leaf column and is never yielded as a leaf — with the reason spelled out (a column is commit-stamped like a leaf, so the walk'sread_commitcompleteness check would otherwise wave it through). That is the fix from the separate finding, landed in7a49faa. - Reconciled in both directions, so a downstream lands on one sha whichever bullet it reads first: the
d52e3063bullet now says it supersedes theb9347561refresh instruction below, and the authalic bullet's closing sentence is rewritten to point atd52e3063instead of asserting its own floor.
Thanks for the §4.5/§4.6 cross-check against zagg docs/specification.md at d52e3063 — section numbers unchanged.
| f"ragged field (spec §8.3)" | ||
| ) | ||
| times_path = _sibling_path(field, element.times) | ||
| times_arr, times_element = open_ragged(store, times_path, zarr_format=zarr_format) |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — the companion sibling's element declaration is never checked against §8.3's uint64 / empty-inner_shape MUST, so a mis-declared sibling silently mis-decodes.
read_ragged strict-checks the three metadata keys (spec/shape/grammar) and then opens the sibling with open_ragged, but never looks at the one declaration that actually governs the decode:
times_path = _sibling_path(field, element.times)
times_arr, times_element = open_ragged(store, times_path, zarr_format=zarr_format)times_element is used only for decode_cell(times_span[pos], times_element). The spec makes the element a MUST twice over — §1.1: "A temporal field's sibling {field}_times array (§8.3) is the same shape of thing" as the located sibling, which is "a zagg-ragged/1 vlen array (element dtype uint64, empty inner_shape)"; §8.3 restates it verbatim: "a zagg-ragged/1 vlen sibling array (element dtype uint64, empty inner_shape)".
Reproduced on this commit — build_store(..., timed=True), conforming §8.3 block left on t_words, only its element rewritten to {"dtype": "float32", "shape": [-1, 2]}:
cell 12711972898206646280 rows 2 times dtype float32 shape (2, 2) vals [1.5e-44 0. 3.1e-44 0.]
cell 12712025674764779528 rows 1 times dtype float32 shape (1, 2) vals [1.5e-44 0.]
No refusal. The per-cell row-alignment guard cannot catch it: float32 (n, 2) is 8 bytes/row too, so len(cell_times) == len(values) still holds and the words come back as float32 pairs presented as toc words. That is precisely the outcome §8 forbids — "a reader MUST refuse a shape it does not implement rather than mis-decode it" — and §2.2/§8.3 hang the word semantics on the uint64 element.
Concrete fix — one guard beside the declaration gate, applied to both companions:
def _require_word_element(element: RaggedElement, path: str, section: str) -> None:
if element.dtype != np.dtype("<u8") or element.inner_shape != ():
raise ValueError(
f"{path!r} declares element {element.dtype!s}{element.inner_shape} but a "
f"companion sibling MUST be uint64 with an empty inner_shape "
f"(spec §1.1/{section})"
)called after each open_ragged (§8.3 for times, §9 for locations). The locations path has carried the same hole since it landed, but phase 2 is where the companion gate is introduced and where the second, symmetric path is added — worth closing both here rather than shipping the new channel with a known silent-mis-decode mode. Note the conformance suite already asserts got.dtype == np.uint64 on the fixture, i.e. the tests rely on an invariant the library does not enforce.
There was a problem hiding this comment.
🤖 from Claude
Folded in 675a36a — both companion paths now check the element declaration before a word is decoded.
-
src/moczarr/ragged.py— new_require_word_element(element, path, section)beside_sibling_path, called immediately after both siblingopen_raggedcalls inread_ragged("§9"for the located sibling,"§8.3"for the temporal one). It requireselement.dtype == np.dtype("<u8")andelement.inner_shape == (), and raises naming the declared element and the clause:'g/t_words' declares element float32[2]; a companion sibling MUST hold one uint64 word per payload row (element dtype uint64, empty inner_shape — spec §1.1/§8.3), and a reader MUST refuse a declaration it does not implement rather than mis-decode itThe docstring reason is recorded on the helper (row alignment cannot catch it —
float32 (n, 2)is 8 bytes/row too), andread_ragged'sRaisessection now lists the element MUST beside the row-alignment MUST. -
tests/test_ragged.py—TestReadRagged::test_companion_element_must_be_uint64_words, parametrised over both channels (locations/geo_words/§9 andtimes/t_words/§8.3) and both §1.5 geometries: it builds the conforming store, rewrites only the sibling'szarr.jsonelement to your exact repro ({"dtype": "float32", "shape": [-1, 2]}) with_write, and asserts the refusal cites the right section. 4 new cases; the located half is the latent hole closed too, as you asked.
Gates on that commit: 844 passed, 2 skipped (from 840/2); ruff check + format clean.
| f"{want_spec!r} only — an unknown or future revision must be adopted " | ||
| f"deliberately, never half-parsed (spec §8)" | ||
| ) | ||
| if shape != _COMPANION_SHAPE: |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — the per-centroid-only refusal is hard-wired into the generic public parser, so parse_companion_attrs cannot parse a conformant §8.1/§8.2 declaration at all.
if shape != _COMPANION_SHAPE:
raise ValueError(
f"{field!r} declares {key} shape {shape!r}; the ragged sibling path "
f"implements {_COMPANION_SHAPE!r} only, ..."
)The constant's own comment says the restriction belongs to a call site, not to the parser:
#: The one §8 ``shape`` vocabulary value the ragged sibling path implements:
_COMPANION_SHAPE = "per-centroid"but the check lives in parse_companion_attrs, which is exported in __all__ and documented generically as "The §8/§9 declaration gate on a companion array's own attrs." The consequence is visible in this same commit: the fixture's own 6/observed array carries a fully conformant §8.2 block ({"spec": "zagg-toc/1", "shape": "per-cell", "grammar": "mortie-toc/1"}), and the test has to read it by hand and then assert the public gate raises on it:
attrs = dict(arr.attrs)
assert attrs["temporal"] == expected["declarations"]["observed"]
...
with pytest.raises(ValueError, match="per-cell"):
parse_companion_attrs(attrs, domain="temporal", field="6/observed")So the one public entry point for §8 declarations refuses the shape §8.2 defines, and the suite codifies that as the contract. That's fine as this reader's posture today (§8: refuse what you don't implement) but it makes the function unusable for the §8.2 per-cell channel a later phase or a downstream O3 consumer will want, and it puts the refusal in the layer that has no idea what the caller is about to decode.
Concrete fix: keep parse_companion_attrs strict on spec/grammar (the revision gates), return the parsed CompanionDeclaration for any shape in §8's vocabulary, and move the implementation gate to where the decode happens — either an explicit parameter with the current behaviour as the default:
def parse_companion_attrs(attrs, *, domain, field="<array>", shapes=(_COMPANION_SHAPE,)):
...
if shape not in shapes:
raise ValueError(...)or a bare _require_shape(decl, _COMPANION_SHAPE, path) at the two read_ragged call sites. The existing tests keep passing under the parameterised form (the default is unchanged), and test_observed_per_cell_words_match_expected can then pin the §8.2 block positively — parse_companion_attrs(attrs, domain="temporal", shapes=("per-cell",)).shape == "per-cell" — instead of pinning a refusal.
There was a problem hiding this comment.
🤖 from Claude
Folded in 60c912c — the shape refusal now belongs to the calling path, and the fixture's §8.2 block is pinned positively.
-
src/moczarr/ragged.py—parse_companion_attrstakesshapes: tuple[str, ...] = (_COMPANION_SHAPE,). Thespec/grammargates stay exactly as strict; the shape check becomesif shape not in shapes, and the message names what the caller declared it implements. The docstring says whose call it is — §8 hangs the refusal on the implementation ("a reader MUST refuse ashapeit does not implement"), which the parser cannot know for a caller it does not decode for — and_COMPANION_SHAPE's comment now reads as the default, not as the parser's own limit.read_ragged's two call sites take the default unchanged, so the sibling path's posture is byte-for-byte the same. -
tests/test_spec_conformance.py::test_observed_per_cell_words_match_expected— now pins the §8.2 declaration positively as you suggested, and keeps the sibling-path refusal beside it, so both halves of the contract are visible:decl = parse_companion_attrs(attrs, domain="temporal", field="6/observed", shapes=("per-cell",)) assert decl is not None and decl.shape == "per-cell" with pytest.raises(ValueError, match="per-cell"): parse_companion_attrs(attrs, domain="temporal", field="6/observed")
-
tests/test_ragged.py::TestParseCompanionAttrs::test_shapes_is_the_calling_path_naming_what_it_implements— the unit test for the parameter:per-cellparses undershapes=("per-cell",), azagg-toc/2spec still raises under the widenedshapes(the revision gate does not travel with the shape gate), and the default still refuses.
Gates on that commit: 845 passed, 2 skipped; ruff check + format clean.
| "decode_cell", | ||
| "iter_populated_chunks", | ||
| "open_ragged", | ||
| "parse_companion_attrs", |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — the new public names are in ragged.__all__ but not re-exported from the package, breaking the established pattern for this module's public API.
This commit adds "CompanionDeclaration" / "parse_companion_attrs" (and TIMES_ATTR, TEMPORAL_ATTR, LOCATED_ATTR, TOC_SPEC, LOCATED_SPEC, TOC_GRAMMAR, MORTON_GRAMMAR) to moczarr/ragged.py's __all__, but src/moczarr/__init__.py is untouched. Its ragged block still reads:
from moczarr.ragged import (
RAGGED_SPEC,
RaggedElement,
decode_cell,
parse_ragged_attrs,
read_cell,
read_ragged,
)So moczarr.parse_ragged_attrs resolves and moczarr.parse_companion_attrs does not — even though the two are exact peers (the §1.2 gate and the §8/§9 gate), and RaggedElement — whose new times field is the thing parse_companion_attrs completes — is re-exported. RAGGED_SPEC is re-exported too, so "constants stay submodule-private" isn't the existing rule either. The conformance suite has to reach past the package (from moczarr.ragged import ... parse_companion_attrs), which is the tell.
Fix: add CompanionDeclaration and parse_companion_attrs to the from moczarr.ragged import (...) block and to moczarr.__all__ (alphabetical, so "parse_companion_attrs" goes just before "parse_composition_attrs"), or — if the intent is that the companion gate stays submodule-level for now — drop the names from ragged.__all__ so the two lists agree. There is no test asserting moczarr.__all__ ⊇ submodule __all__s, so nothing catches the drift today.
There was a problem hiding this comment.
🤖 from Claude
Folded in 56be355 — the two names are re-exported, and the drift now has a test.
src/moczarr/__init__.py—CompanionDeclarationandparse_companion_attrsjoin thefrom moczarr.ragged import (...)block and__all__("CompanionDeclaration"between"COVERAGE_SPEC"and"ConservativeCoverageWarning","parse_companion_attrs"just before"parse_composition_attrs"— the file's ASCII ordering).- Scope call on the constants: left submodule-level. The package already re-exports a curated subset of
ragged.__all__—RAGGED_SPECis there butRAGGED_ATTR,RAGGED2_SPEC,RAGGED2_DATA_TYPE,open_ragged,iter_populated_chunksandstored_chunk_spansare not — so "every constant travels" was never the rule either. What your finding identified as the real break is the gate asymmetry (parse_ragged_attrspublic,parse_companion_attrsnot), and that is what is fixed. tests/test_ragged.py::TestPackageSurface— two assertions, written to the subset posture rather than against it: every name inmoczarr.__all__ ∩ moczarr.ragged.__all__must resolve to the same object as the submodule's (catches a shadowed or stale re-export), and the §1.2 and §8/§9 gates must both be public with their dataclasses ({RaggedElement, parse_ragged_attrs, CompanionDeclaration, parse_companion_attrs}⊆ both lists) — which is exactly the drift this finding caught, now failing loudly. A blanketmoczarr.__all__ ⊇ ragged.__all__would have to assertopen_raggedis public, which is a different (undiscussed) decision, so I did not make it here.
Gates on that commit: 847 passed, 2 skipped; ruff check + format clean.
| """The §8.3/§9 companion channels of the ``temporal/`` fixture, pinned on | ||
| committed bytes — leaf ingest words and the §4.6 column's MERGED words | ||
| (per-centroid at every level, the espg 2026-08-17 ruling amending zagg | ||
| issue #410's ruling 3). Words are asserted byte-exactly; their |
There was a problem hiding this comment.
🤖 from Claude (review)
nit — the class docstring overclaims: the §4.6 column's merged words are not asserted byte-exactly anywhere.
"...pinned on committed bytes — leaf ingest words and the §4.6 column's MERGED words (per-centroid at every level...). Words are asserted byte-exactly..."
That holds for the leaf (test_leaf_companion_words_match_expected compares against expected["cells"][*]["h_tdigest_times"] decimal goldens), but the only test that decodes the column's words asserts nothing about their values:
for word, values, locations, times in out:
assert values.dtype == np.float32 and values.shape[1:] == (2,)
assert locations.dtype == np.uint64 and times.dtype == np.uint64
assert len(locations) == len(values) == len(times)dtype + row alignment, no goldens. test_pyramid_column_hashes_match_the_stats_sidecar does pin the column's stored bytes, but it hashes the chunk objects — it never exercises the decode path, so a decode-side regression (a wrong slice offset, a misordered channel) would leave both tests green. And temporal.expected.json carries no per-level records for resolutions 4/5, only leaf cells, so there is no golden available to assert against.
Two ways to make the docstring true, either fine:
- Soften it — say the column's merged words are pinned at rest (O11 hashes) and that the decode is pinned for shape/alignment only, since the vendored
expected.jsonrecords no per-level goldens; note the mixed-level case §8.3 permits is what the level-4/5 test is really guarding. - Or pin something value-bearing that needs no zagg golden: e.g. that level 4's decoded
h_tdigest_timesfor a cell equals the words obtained by re-reading the same sibling throughread_cell(path parity), and that level 5's per-cell word multiset is not a copy of level 4's (i.e. the fold actually happened) — both derivable from committed bytes alone.
Same nit applies to the PR body / CHANGELOG line "the conformance suite pins ... the §4.6 column's folded per-centroid companions at resolutions 4 and 5", which reads as byte-pinning too.
There was a problem hiding this comment.
🤖 from Claude
Both halves folded in b10099b — the wording is corrected and the decode is now value-bearing, using your option (2)'s "needs no zagg golden" constraint.
(a) The overclaim is gone. TestTemporalCompanion's class docstring now separates the two strengths explicitly: the LEAF's ingest words are byte-exact against expected.json's decimal goldens; the §4.6 column's merged words have no such golden — the vendored expected.json records no per-level entries — so they are pinned at rest by the O11 hash gate against all.pyramid.stats.json and through the decode by dtype, row alignment, the reserved-0 exclusion and the digest's conserved total weight. The CHANGELOG bullet made the same overclaim ("the conformance suite pins … the §4.6 column's folded per-centroid companions at resolutions 4 and 5") and is reworded the same way in the same commit.
(b) test_overview_siblings_decode_via_the_leaf_path now asserts values. Two claims derivable from committed bytes alone:
total = sum(cell["count"] for cell in expected["cells"]) # the leaf's 346 obs
...
assert int(times.min()) > 0 and int(locations.min()) > 0
weight += float(np.sum(values[:, 1], dtype=np.float64))
assert weight == float(total)- Total weight is conserved by the fold — §2.1 makes total weight the exact observation count and the k-way merge preserves it, so each level's decoded centroid weights must sum to the leaf's total.
346is derived fromexpected.json's per-cellcounts (40 + 1 + 5 + 300), never hard-coded; it holds exactly at both levels. A wrong slice offset or a misordered channel moves that sum, so the decode-side regression you describe is now visible. - No companion word is the reserved
0(§8.2: not a value the grammar's encoders can produce), on both channels, pluslen(values)— a yielded cell is never a zero-row fold.
Gates on that commit: 850 passed, 2 skipped; ruff check + format clean.
| assert times.shape == (len(values),) # §8.3 row alignment | ||
| np.testing.assert_array_equal(times, np.arange(1, len(values) + 1, dtype="<u8") * 11) | ||
|
|
||
| def test_both_channels_yield_in_fixed_order(self, tmp_path, sharded): |
There was a problem hiding this comment.
🤖 from Claude (review)
nit — missing coverage: times=True is never exercised with subtree=, though locations=True is.
The located channel has an explicit subtree test (TestSubtreeSweep, tests/test_ragged.py):
got = list(read_ragged(store, "g/field", locations=True, subtree=SHARD + "1"))Nothing equivalent for times. The new times_span slice is taken from the same span_start:span_stop the payload's clipped spans produce:
times_span = (
cast(np.ndarray, times_arr[span_start:span_stop]) if times_arr is not None else None
)so the subtree path has a real interaction the plain sweep does not: stored_chunk_spans / _subtree_span are computed on the payload array only, and the clip span_start = max(span_start, span[0] - span[0] % cells_per_chunk) uses the payload's cells_per_chunk. §8.3 requires the sibling to share "the same shape and chunk geometry as the payload array", but that MUST is not checked anywhere (see the element-declaration finding), so the widened-chunk clip plus the per-cell membership guard is exactly where a geometry mismatch would surface as a silent off-by-chunk rather than a raise.
Cheap fix: add times=True (and locations=True, times=True) to the existing subtree parametrisation — e.g. mirror the located test with
got = list(read_ragged(store, "g/field", locations=True, times=True, subtree=SHARD + "1"))and assert the 4-tuples equal the whole-sweep entries filtered to that subtree, the same _filtered(...) golden contract the sibling test already uses.
There was a problem hiding this comment.
🤖 from Claude
Folded in 6c5f9c6 — tests/test_ragged.py::TestSubtreeReadRagged::test_times_ride_the_restricted_sweep, the mirror of the located test, on the same _filtered/_assert_same golden contract:
sweep = list(read_ragged(store, "g/field", times=True))
got = list(read_ragged(store, "g/field", times=True, subtree=SHARD + "1"))
self._assert_same(got, self._filtered(sweep, SHARD + "1"))
assert len(got) == 2 and all(len(entry) == 3 for entry in got)and the both-channels case you suggested as well (locations=True, times=True, subtree=… against the filtered 4-tuple sweep), since that is where the fixed channel order and the shared span_start:span_stop slice interact. The docstring records what the test is really guarding: the span and its widened-chunk clip are computed on the PAYLOAD array and then used to slice the siblings, so a geometry mismatch would land here as an off-by-chunk.
On the §8.3 "same shape and chunk geometry" MUST you note is unchecked: the element half of that gap is now closed by 675a36a (the sibling-element finding on this same review), and the row-alignment guard catches a per-cell count mismatch. A shape/chunk-grid check on the sibling array itself is still not there — it is a different guard from the one that finding asked for, so I left it standing for review rather than widening the fold's scope.
Gates on that commit: 850 passed, 2 skipped; ruff check + format clean.
|
|
||
| - Temporal companion channel (`zagg-toc/1`, spec §8.3; englacial/zagg#410 / | ||
| PR #463): `moczarr.ragged` now binds and decodes a digest field's | ||
| per-centroid `{field}_times` sibling — `read_ragged(..., times=True)`, |
There was a problem hiding this comment.
🤖 from Claude (review)
nit — the CHANGELOG names the sibling by the naming convention the module explicitly refuses to reconstruct.
"
moczarr.raggednow binds and decodes a digest field's per-centroid{field}_timessibling"
The whole point of the §8.3 binding — and the reason build_store deliberately names its fixture sibling t_words rather than field_times — is that the name is metadata, not convention. The module docstring says so in the same breath ("readers bind the sibling by metadata, never by reconstructing the {field}_locations naming convention"), and the parameter doc repeats it ("never by naming convention"). A reader skimming the changelog would reasonably conclude moczarr derives the path from the field name.
Fix: drop the {field}_times form — "binds and decodes a digest field's per-centroid temporal sibling, named by the payload's spec-owned top-level times attrs key" — the rest of the bullet already says exactly that in the next clause.
There was a problem hiding this comment.
🤖 from Claude
Folded in f7f2eb3. The bullet no longer names a basename pattern — it names the binding:
moczarr.raggednow binds and decodes a digest field's per-centroid temporal sibling —read_ragged(..., times=True)— named by the payload's spec-owned top-leveltimesattrs key, never by reconstructing a naming convention from the field name (§8.3; the spec-text-only fixture's sibling is deliberately namedt_words, nothing like{field}_times).
The one surviving {field}_times mention is the negative one, which is the point you are making. The same sentence now also records the sibling-element MUST enforced by 675a36a on this review.
Gates on that commit: 850 passed, 2 skipped; ruff check + format clean.
| ``grammar`` raises. Keys beyond ``{spec, shape, grammar}`` are ignored | ||
| rather than refused (§9: "informative extra keys ignored"). | ||
| """ | ||
| key, want_spec, want_grammar = _COMPANION_DOMAINS[domain] |
There was a problem hiding this comment.
🤖 from Claude (review)
nit — two small polish items on the new public gate.
(1) domain is an unvalidated public keyword that fails with a bare KeyError.
key, want_spec, want_grammar = _COMPANION_DOMAINS[domain]parse_companion_attrs is exported in __all__ and domain is a plain str, so parse_companion_attrs(attrs, domain="temporal ") or domain="located_" raises KeyError: 'located_' with no hint — while every other refusal in this module raises a ValueError naming the spec clause. Either type it Literal["temporal", "located"] (mypy then catches it statically, matching the zarr_format: Literal[2, 3] style already used here) or guard it:
if domain not in _COMPANION_DOMAINS:
raise ValueError(f"unknown companion domain {domain!r}; expected 'temporal' (§8) or 'located' (§9)")(2) every refusal cites (spec §8), including the located ones.
raise ValueError(
f"{field!r} declares {key} spec {spec!r}; this reader understands "
f"{want_spec!r} only — ... (spec §8)"
)For domain="located" that points a reader at the temporal section. §9 does say "Everything §8 says about the pattern ... applies here unchanged and is not restated", so the citation isn't wrong — but the reader hitting a zagg-located/2 refusal wants §9, and the domain is already in hand. Since _COMPANION_DOMAINS is a per-domain tuple, adding the section token to it is a one-line change: "temporal": (TEMPORAL_ATTR, TOC_SPEC, TOC_GRAMMAR, "§8"), "located": (LOCATED_ATTR, LOCATED_SPEC, MORTON_GRAMMAR, "§9"), and interpolate it. (The malformed-block message's spec §8/§9 is fine as-is.)
Also, while here: read_cell's docstring still scopes its companion note to the located channel — "Works on any conforming array — a located field's sibling included (its elements decode as (n,) uint64 words)" — which now reads as excluding the temporal sibling it equally supports. "a companion sibling (located or temporal) included" covers both.
There was a problem hiding this comment.
🤖 from Claude
All three folded in f0b9aa0.
domainis validated._COMPANION_DOMAINS[domain]is wrapped, so a typo raises a spec-citingValueErrornaming both domains instead of a bareKeyError:unknown companion domain 'temporal '; this layer implements the two the spec instantiates: 'temporal' (spec §8) and 'located' (spec §9). I took the runtime guard rather than theLiteraltyping:domainreaches this function from callers that pass a plainstr(the conformance suite parametrises it), and mypy would not see those.- Refusals cite the section that defines the domain.
_COMPANION_DOMAINSis now a 4-tuple carrying the section token —("temporal", …, "§8")/("located", …, "§9")— interpolated into all three refusals, exactly the one-line change you sketched. The malformed-block message keepsspec §8/§9. The constant's comment records why (§9 says §8's rules apply unchanged, but the reader meeting azagg-located/2refusal wants §9), and the docstring says every refusal cites the defining section. read_cell's companion note covers both channels — now "a companion sibling (located, §9, or temporal, §8.3) included: both decode as(n,)uint64 words."
New tests in TestParseCompanionAttrs: test_unknown_domain_raises_a_spec_citing_error, and test_refusals_cite_the_domain_that_defines_them (a zagg-located/2 spec refusal must match (spec §9), a temporal shape refusal (spec §8)).
Gates on that commit: 849 passed, 2 skipped; ruff check + format clean.
…fines it (issue #43)
| declaration (counts vs flux) is a reader-relevant MUST this layer does | ||
| NOT yet gate on — a known gap tracked as espg/moczarr#43, where the gate's | ||
| design is a standing question; digest payload bytes still decode | ||
| identically under either declaration, so nothing here mis-decodes, but a |
There was a problem hiding this comment.
🤖 from Claude (review)
"nothing here mis-decodes" is true for the two defined weights values, but the un-gated MUST has a third arm the docstring does not mention: an unknown declaration must be refused.
The claim:
digest payload bytes still decode identically under either declaration, so nothing here mis-decodes, but a consumer summing weights must consult #43's resolution before presenting the sum as an observation count
The first half checks out — §2.0 defines "counts" as integers ≥ 1 and "flux" as positive finite float32 reals, both inside the same {"dtype": "float32", "shape": [-1, 2]} element declaration (§2.1 unchanged on that point), so the byte decode is identical. But §2.0 at d52e3063 reads:
A reader MUST strict-check the value: an unknown declaration is a future revision of this section and MUST be refused, never read as either defined value.
Un-gated, this layer silently decodes a payload carrying "weights": "<anything else>" and hands it to a consumer, who — per the docstring's own guidance — would resolve the question as "counts vs flux". §2.0 says neither reading is legal there. The stated consequence ("consult #43 before presenting the sum as an observation count") therefore does not cover the case the MUST is actually about.
§2.0 also carries a second un-gated MUST worth a clause: "Merges are legal only between payloads carrying the same declaration … a merging reader or writer MUST refuse it" (an absent key counting as "counts" for that rule). That one is currently vacuous for this module — I grepped src/moczarr/ and found no payload-merging entry point — but it is part of what the pin is declining to gate, and #43 lists it explicitly ("Refuse (or loudly type) any moczarr-side operation that would combine payloads across mismatched declarations").
There was a problem hiding this comment.
🤖 from Claude
Folded in 46fb641. The third arm is now stated at the pin, and so is the merge rule.
The record now says that un-gating leaves §2.0's other two arms unenforced as well: an unknown weights value — which §2.0 says a reader MUST refuse, "never read as either defined value" — decodes here silently, and #43's gate covers that refusal too; and §2.0's same-declaration merge rule is meanwhile vacuous here, "since moczarr has no payload-merge entry point at all (moczarr.composition declines a read-side merge by design), which is why the gap is a documentation matter and not a live mis-merge risk."
I verified the vacuity claim independently rather than taking it from the finding: grepping src/moczarr/ for merge/combine/fold/concat/union entry points turns up only moc_index.concat (MOC interval concatenation), ranges.MortonRanges.union (interval set-union) and stats.combined_hash — none of which open a digest payload — and composition.py states the exclusion explicitly at line 57 ("Deliberately absent: a read-side merge"). No code path in this package combines payload bytes across arrays, so nothing can currently violate the mixed-merge MUST.
| englacial/zagg — normative on ``main``, re-checked at ``d52e3063``, the | ||
| englacial/zagg#463 merge; landed with the #346 rebase merge, and the interim | ||
| pin ``9e11e65`` is historical. The §1 delta between those two pins was | ||
| re-read line by line for this advance and is exactly the §8.3 temporal |
There was a problem hiding this comment.
🤖 from Claude (review)
Nit. "exactly the §8.3 temporal surface" is very slightly over-tight. Diffing §1 at both shas (old lines 74–232, new lines 77–246) gives three hunks; the third is the §1.2 rewrite, which carries one clause that is §2.0's, not §8.3's:
The §2.0
weightskey and §8.3'stimesbinding are likewise spec-owned on a ragged payload array — writer-stamped from the field's declaration, never author-transcribed — as are the §8/§9temporalandlocateddeclaration blocks on the companion arrays that carry them.
So §1's delta also extends §1.2's reserved-key discipline to weights. It is writer/config-validation-side ("enforced at config validation") and changes nothing for this reader, but the three-item inventory below this line does not cover it, and the sentence claims exhaustiveness.
Everything else in the §1 claim I verified sound: the sibling inventory does go three → four with {group}/{field}_times; §1.1 does gain the temporal-sibling bullet with the times attrs-key binding and "either sibling, both, or neither"; and §1.3, §1.4, §1.5 and §1.6 are byte-identical across the delta — the diff produces no hunk anywhere past §1.2, so the codec chain, wire framing, storage-geometry and subtree-span contracts this module depends on are unmoved, exactly as claimed.
There was a problem hiding this comment.
🤖 from Claude
Folded in 95e2d54. The "exactly the §8.3 temporal surface" exhaustiveness claim is gone and the §1 inventory now names both sub-sections.
The bullet reads: "§1.1 and §1.2 only. §1.1 carries the §8.3 temporal surface this module now decodes: the sibling inventory goes three to four with {group}/{field}_times, row-aligned and bound by the payload's top-level times attrs key (a field may carry either sibling, both, or neither). §1.2 extends its spec-owned-key discipline past the ragged block itself — the §2.0 weights key and §8.3's times binding are writer-stamped on the payload array, as are the §8/§9 temporal and located declaration blocks on the companion arrays that carry them — and restates the sibling rule as no user attrs, only the spec-owned declaration its own section defines. §1.3/§1.4 wire framing and the §1.5 storage-geometry/subtree-span contracts are byte-identical across the delta, as is §1.6."
I re-ran the §1 diff myself before writing that: three hunks, all inside §1.1/§1.2, nothing past line 90 of the section — so your byte-identity result for §1.3–§1.6 reproduces, and the record now states §1.6 explicitly rather than stopping at §1.5. CHANGELOG.md's pin bullet inherited the same correction (§1.3–§1.5 unchanged → §1.3–§1.6 unchanged, plus the §1.2 clause) in the same commit.
| ``times`` binding, and the rule that a sibling carries only the spec-owned | ||
| declaration its own section defines. §1.3/§1.4 wire framing and the §1.5 | ||
| storage-geometry/subtree-span contracts are byte-identical across the | ||
| delta. One §2 caveat rides this pin deliberately: the §2.0 ``weights`` |
There was a problem hiding this comment.
🤖 from Claude (review)
The "one §2 caveat" claim understates the §2 delta — §2.1 and §2.2 both moved too, and §2.2 is the section this module's own docstring leans on.
The pin record here says:
One §2 caveat rides this pin deliberately: the §2.0
weightsdeclaration (counts vs flux) is a reader-relevant MUST this layer does NOT yet gate on
I extracted §2 at both shas (git show 9e11e65:docs/specification.md / git show d52e3063:docs/specification.md, §2 = old lines 233–295, new lines 247–391) and diffed. §2.0 is not the only change:
-
§2.1 — the exact-count MUST was rescoped:
sum(weights)MUST equal the exact observation count became "under"counts",sum(weights)MUST equal …", plus a new bullet that under"flux""the exact-count recovery above (and §3.3's) is undefined … and no integrality holds". -
§2.2 — substantially rewritten, including a new reader MUST that did not exist at
9e11e65:Orders are heterogeneous, leaf arrays included. A reader MUST decode each word's order and kind from the word itself (mortie §1/§4) and MUST NOT assume a uniform order per array, per cell, or per store.
-
§2.2 also gained the sentence this module cites four times:
A located sibling MAY additionally carry the §9
locateddeclaration … its absence is this section verbatim, never a refusal (§9).That clause is +added in this delta.
ragged.pylines 33, 276, 737 and 802 all assert "an absentlocatedblock is §2.2 verbatim and never a refusal" — at the old pin that was an inference; atd52e3063it is spec text. The pin record is the place that should say so, and it says the opposite ("one §2 caveat").
The PR body itself itemizes §2.2 as "new-in-delta reader surface", so the docstring is also inconsistent with the PR's own re-check table. Since the whole point of the #33 → #32 pin convention is that the docstring, not the PR body, is the durable record a future reader diffs against, the §2 sentence should name §2.1/§2.2 (both benign for this layer — it decodes words as opaque row-aligned uint64 and assumes no order, so the new heterogeneous-orders MUST is satisfied by construction) rather than assert §2.0 is the only §2 item.
There was a problem hiding this comment.
🤖 from Claude
Folded in 92f4598. You are right on all three counts — I re-diffed §2 at both shas (old 233–295, new 247–391) and the "one §2 caveat" sentence was covering a three-item delta.
The pin record is now itemized rather than summarized. §2 reads as three items: §2.0 new (the un-gated weights declaration), §2.1 rescoped (exact-count MUST now under "counts", plus the flux bullet — "the exact-count recovery above (and §3.3's) is undefined for a flux payload, and no integrality holds"), and §2.2 substantially rewritten, with the two clauses this module actually leans on called out by name: the heterogeneous-orders reader MUST ("MUST NOT assume a uniform order per array, per cell, or per store" — satisfied by construction here, since companion words are yielded as opaque row-aligned uint64), and the "its absence is this section verbatim, never a refusal (§9)" sentence, noted as spec text at d52e3063 where it was an inference at 9e11e65.
Also folded into the same commit: the un-posted structural point. The phase-3 insertion had turned the opening sentence into a ~15-line parenthetical closing on ):. The pin record is now its own paragraph ("Pin record — re-checked at d52e3063, the englacial/zagg#463 merge …") with a per-section bullet list, and the opening sentence is back to one line: "The read side of zagg's ragged store spec (…§1 in englacial/zagg, normative on main): a kind: ragged field is ONE variable_length_bytes zarr v3 array on the cells axis." Every re-check claim survived the move. CHANGELOG.md's pin bullet inherited the same §2 correction in that commit.
Subsequent commits layered the remaining findings onto the same record: 46fb641 (§2.0's unknown-value arm), 6aaf8d3 (the #43 framing), 95e2d54 (§1.2), f74c042 (§6).
| delta. One §2 caveat rides this pin deliberately: the §2.0 ``weights`` | ||
| declaration (counts vs flux) is a reader-relevant MUST this layer does | ||
| NOT yet gate on — a known gap tracked as espg/moczarr#43, where the gate's | ||
| design is a standing question; digest payload bytes still decode |
There was a problem hiding this comment.
🤖 from Claude (review)
"where the gate's design is a standing question" does not match what issue #43 actually says — and #43 ties the pin advance to the gate landing in the same PR.
The docstring justifies the un-gated advance as:
a known gap tracked as #43, where the gate's design is a standing question
Issue #43's body is not an open design question. It prescribes the scope concretely, names the posture to mirror, and makes the pin advance a rider on the gate:
- Read the weights declaration off payload arrays … with the absent ⇒ counts default.
- Surface it on the read API …
- Refuse (or loudly type) any moczarr-side operation that would combine payloads across mismatched declarations — mirror zagg's
check_weights_matchposture.- Parity coverage: the
flux/conformance fixture zagg PR #431 added …- Advance the two spec-text pins (
src/moczarr/ragged.py,src/moczarr/composition.py, currently at zagg9e11e65) in the same PR — this issue is the re-check the pin advancement asserts (the PR Repin spec provenance to zagg main 9e11e65 #33 → Read (and verify) the D20 sidecars zagg now writes for overview leaves #32 precedent: never move a pin without performing the delta review; the §1/§2 delta IS this issue).
The issue carries zero comments (gh issue view 43 --comments → empty), so there is no on-thread question standing there either. The "standing question" framing appears to originate from PR #42's Q2 — which asked the opposite way round:
moczarr does not gate on
weightstoday, so advancingragged.py's pin would assert a re-check that has not happened. … confirm, and I'll leave both spec-text pins at9e11e65until it's worked?
espg's answer to that was to file #43 with "advance the pins in the same PR" as a bullet alongside the gate. Phase 3 takes that bullet without the four that precede it, and the docstring recasts #43 as a design question to carry the advance.
That may still be the right call — the delta review was performed and the gap is documented at the pin, which is a defensible reading of the #33 → #32 rule, and this sweep was espg-directed. But it is a scope call only espg can make, not one the docstring should settle by re-describing #43. Two concrete options:
- Leave
ragged.py's §1 pin at9e11e65(advance onlycomposition.py's §3 pin, which is a verified byte-identical pure advance) until the weights gate lands, per PR Re-vendor the kitchen_sink spec fixture under the authalic convention #42 Q2's original proposal. - Keep the advance, but reword to say what is true — that Gate reads on the §2.0 weights declaration (counts vs flux) + advance the spec-text pins #43's implementation scope is prescribed and deferred, not that its design is open — and get espg's explicit sign-off on decoupling the pin advance from the gate.
There was a problem hiding this comment.
🤖 from Claude
Wording folded in 6aaf8d3; the pin advance stays, on espg direction. Taking the two halves separately.
The wording was wrong and is fixed. You are right that #43's body is not an open design question — it prescribes the scope concretely. The docstring no longer says "where the gate's design is a standing question". It now reads: "a known gap tracked as #43. What stands open there is the gate's implementation, not its design: #43 prescribes the scope (read the declaration off payload arrays with the absent-⇒-counts default, surface it on the read API, and mirror zagg's check_weights_match posture across mismatched declarations). This pin advance asserts the narrower thing — the delta review #43's pin bullet calls for, performed and recorded right here — and leaves the gate as #43's scope." CHANGELOG.md inherited the same correction in that commit.
The scope call was already made, by espg, and is not mine to relitigate. This sweep is espg-directed — the post-#463 fixture re-vendor brief of 2026-08-17 — and that brief instructs three things explicitly: perform the delta re-check; advance the pins to the new zagg main sha, documenting the delta per pinned item; and do not decide the weights gate, leaving #43's gate question standing for espg and referenced as still-standing in the PR body. That is espg re-scoping #43's pin-advance bullet away from its gate bullets for this PR — the decoupling your option (2) asks for sign-off on has already been given. So option (1) (roll ragged.py's pin back to 9e11e65) would contradict the directive, and implementing any part of the gate here is out of scope by the same directive.
One caveat worth flagging rather than burying: the directive was conveyed in-session, not on this thread, so the on-thread record is exactly what you reconstructed from #43 and PR #42 Q2. If espg reads the decoupling differently at review, the fix is a one-line pin revert plus a CHANGELOG edit — the delta review recorded at the pin is the durable artifact either way, and it stands whether or not the pin moves. The PR body notes the sweep was espg-directed; I have left the gate itself entirely unimplemented.
|
🤖 from Claude (review) Nit, no natural line anchor in the phase-3 diff. §6 ( The PR body enumerates §1, §2.0, §2.2, §3, §5, §8, §9, including the negative results (§3 and §5 byte-identical — I independently confirm both: §3 §6.1 — the
§6.3 — a new parenthetical on the
For the record, the sections I diffed at |
|
🤖 from Claude Folded in I diffed §6 at both shas before writing it (old 757–839, new 1390–1481) and your read reproduces: three hunks, two normative. The new bullet reads: "§6 — not pinned by this module, but read through in the same pass because the The third hunk I left out deliberately: §6.3's informative tail gained the The PR body's re-check table is the orchestrator's to maintain, so I have not touched it — this reply is the standing note that §6 belongs in that row too. |
|
🤖 from Claude Closing the loop on the four review questions now that this is merged (espg rulings, in-session 2026-08-18):
|
1 participant
Refs #43 (the pin-advancement / re-check vehicle). Refs englacial/zagg#410 and zagg PR #463 (temporal + located companion channels; merged to zagg
mainasd52e3063), zagg PR #420 (D19 epoch fixture-metadata move), zagg PRs #456/#463 (spec text revisions). This sweep was directed by espg (post-#463 fixture re-vendor).What this does
Executes the post-#463 fixture re-vendor sweep against englacial/zagg
mainatd52e3063(the PR #463 merge commit): audits every vendored conformance fixture against current zagg bytes, re-vendors the drift, adopts the newtemporal/companion fixture with reader-side support for the §8.3timessibling, and performs the spec-text pin re-check #43 calls for, advancing the pins9e11e65→d52e3063.Phases
minimal/+kitchen_sink/to whole-tree byte parity with zaggmain. Drift found (measureddiff -rbefore editing):morton_hive.jsonin both fixtures (semantic_hashre-pinned by zagg #420; pyramid declaration{"orders": [], "aggregation": {}}→ thezagg-pyramid/2block;generated_at),11213.zarr/zarr.json(written_atonly), plus the pyramid-era surface newly present beside each leaf (all.pyramid.zarr,all.pyramid.stats.json, canonical-idgranules.json— zagg #420). Every array byte the conformance suite decodes is unchanged; bothexpected.jsonfiles are byte-identical atd52e3063. The standalonetests/data/compositioncopy re-verified byte-identical (cmp/diff) — pure pin advance intest_composition.py.temporal/+ reader support for the §8.3 temporal sibling. Vendortests/data/spec/temporal/+temporal.expected.json(whole tree, byte-identical atd52e3063); extendmoczarr.raggedwith the spec-ownedtimesbinding (payload-level attrs key, §8.3) and the §8/§9 companion-declaration gates (temporal/locatedblocks on the siblings, strict-checked when present, absent-located= §2.2 verbatim); parity tests decoding the leaf AND theall.pyramid.zarrresolution-4/5 per-centroid siblings from committed bytes (same decode path by design — espg's 2026-08-17 ruling amending zagg#410 ruling 3), the O11 hash gate over the pyramid column againstall.pyramid.stats.json(combinedb6859b89…), and the manifest composability reclassification (h_tdigestnone→approximate,observedstaysnone).9e11e65→d52e3063). Per-pinned-item re-check against the currentdocs/specification.md(delta itemized below), then advance the pins insrc/moczarr/ragged.pyandsrc/moczarr/composition.py. CHANGELOG.Spec-pin re-check (documented at the pins in phase 3
f2fb523, corrected by the phase-3 folds throughf74c042)Measured by extracting each section at both shas and diffing:
zagg-composition/1) — byte-identical between9e11e65andd52e3063.composition.py's pin advance is pure.stats.pyhash recipe needs no re-check.zagg-ragged/1) — three additions, no removals: (1) "up to four sibling arrays" — the temporal{field}_timessibling (§8.3); (2) thetimesbinding is a spec-owned attrs key beside theraggedblock ("Thezagg-ragged/1block grammar is therefore unchanged by this revision"); (3) siblings carry only spec-owned declaration blocks (located/temporal), no user attrs. Phase 2 implements (1)+(2) and adapts the sibling-attrs posture test for (3).weightsdeclaration: the reader-relevant MUST moczarr does not yet gate on — deliberately not implemented here, including its unknown-value refusal arm; Gate reads on the §2.0 weights declaration (counts vs flux) + advance the spec-text pins #43's gate scope stands, see below); §2.1 rescoped its exact-count MUST to "undercounts" with a flux bullet; §2.2 was rewritten (kind-keyed claims, the heterogeneous-orders reader MUST — satisfied here by construction, words are yielded as opaque row-aligneduint64— and the "absence is this section verbatim, never a refusal" sentence that was previously only an inference).zagg-ragged/2) — NOT byte-identical: §6.1 adds{field}_timesto the /2 element declaration and §6.3 notestimes/weightsride the /1→/2 migration untouched; both recorded at the pin (ragged.py cites §6.1/§6.3 for its /2 refusal carve-out).zagg-toc/1) / §9 (zagg-located/1) — new-in-delta reader surface; phase 2 adopts the declaration gates and the byte-level sibling decode. Full toc word semantics (mortie-toc/1 start/end-ns decode, window overlap predicates) are not adopted here — flagged under Questions.Not in scope
flux/fixture — Gate reads on the §2.0 weights declaration (counts vs flux) + advance the spec-text pins #43's main body. The design question there is still standing for espg; nothing in this PR decides it. Theragged.pypin advance documents §2.0 as a known, tracked gap (Gate reads on the §2.0 weights declaration (counts vs flux) + advance the spec-text pins #43) rather than asserting a gate that does not exist./2reader view overall.pyramid.zarr(open_hive / DataTree adoption) — expose the zagg-pyramid/2 fixed ladder as a multiscale / datatree view #36/Design: the xarray data model for multiresolution /2 stores (DataTree vs resolution dimension vs custom multi-res index) #37. This PR decodes the pyramid column's arrays store-scoped through the generic ragged layer only.column/,flux/,raster_toc/,pyramid/fixtures — not vendored before, not directed here (temporal is, per the sweep's brief).How it was tested
git archiveof zaggtests/data/specatd52e3063,diff -ragainst the vendored trees (per-fixture drift itemized in phase 1 above).origin/main(a0cce5b): 804 passed, 2 skipped (env:uv sync --extra test --extra xdggs --extra zagg— zagg 0.46.0, so the live-parity legs ran).ruff check/ruff format --checkclean.Questions for review
ragged.pypin advance is decoupled from Gate reads on the §2.0 weights declaration (counts vs flux) + advance the spec-text pins #43's weights-gate bullets — per the sweep directive, standing for your confirmation. Issue Gate reads on the §2.0 weights declaration (counts vs flux) + advance the spec-text pins #43's body couples "advance the two spec-text pins" to implementing the §2.0 gate in the same PR; the phase-3 reviewer challenged the decoupling (thread onsrc/moczarr/ragged.py). The sweep brief (2026-08-17, in-session) explicitly directed: perform the re-check, advance the pins documenting per pinned item, do NOT decide the weights gate. The delta review WAS performed (itemized above and at the pin); the gate — including thecheck_weights_match-mirroring scope Gate reads on the §2.0 weights declaration (counts vs flux) + advance the spec-text pins #43 prescribes — remains Gate reads on the §2.0 weights declaration (counts vs flux) + advance the spec-text pins #43's open implementation scope. If you read Gate reads on the §2.0 weights declaration (counts vs flux) + advance the spec-text pins #43's coupling as binding, the revert is one line on theragged.pypin.docs/specification.md§4.5 atd52e3063still says "A temporal companion (§8.3) is a different case and staysclass: "none"… those fields exist at native resolution only", while the §7 fixture catalog (same sha) documents the temporal column carrying folded per-centroid companions at every level (espg-ruled 2026-08-17, amending ruling 3), the §8.4 licensing text records the amendment, and the committedtemporal/morton_hive.jsondeclaresh_tdigestclass: "approximate"withtemporal: "per-centroid". The fixtures + §7 + §8.4 govern this PR's reader behavior; the §4.5 sentence looks like a leftover from before the amendment and probably wants a zagg-side edit. The §4.5 field-entrytemporal:key the manifest carries is also undocumented in §4.5's grammar bullet.675a36acloses the element half (uint64, empty inner shape), but "same shape and chunk geometry as the payload array" is not verified against the sibling's own zarr metadata. Folding it would have widened a review fold into new contract surface; if you want it gated, it is a small follow-up.temporal.expected.jsonrecords per-word containment data (centroid_spans_ns,obs_span_ns) that a full conforming decode would assert; phase 2 pins the words byte-exactly (decimal-string goldens, row alignment, O11 hashes) but does not decode start/end nanoseconds. If that adoption is wanted, it looks like its own issue (grammar implementation + window predicates), on the Repin spec provenance to zagg main 9e11e65 #33→Read (and verify) the D20 sidecars zagg now writes for overview leaves #32 precedent of not burying a contract adoption in a fixture PR.Review → fold log
c0f46cc(pin thezagg-pyramid/2declaration degrade — remedy (b): raising on the /2specwould have brokenopen_hiveon every store zagg writes today, while §4 makes overviews derived artifacts a reader MUST NOT require; degrade pinned by a parametrized regression test over all three vendored /2 manifests and documented inoverview_declaration's docstring, with /2 binding deferred to expose the zagg-pyramid/2 fixed ladder as a multiscale / datatree view #36/Design: the xarray data model for multiresolution /2 stores (DataTree vs resolution dimension vs custom multi-res index) #37),7a49faa(enforce §4.6's normative.pyramid.zarrname seam in_classify_children—walk_leaves/open_hiveno longer classify the vendored leaf columns as leaves; pinned on the realminimal/tree),42b37e4(CHANGELOG: drop the stale "no reader behavior changes" claim, reconcile the refresh shas both directions). Every thread has a reply with the fix sha.675a36a(enforce the §1.1/§8.3 companion-element MUST — a sibling whose ragged element is not bare little-endianuint64now refuses on both channels, closing a silent float-as-words mis-decode that row alignment could not catch; the located path had carried the same latent hole),60c912c(parse_companion_attrsgrewshapes=so the public §8 gate can validate the fixture's conformant §8.2 per-cellobserveddeclaration while the sibling path keeps its per-centroid-only default),56be355(package re-export of the new gate + dataclass, with a drift test over the shared__all__surface),f0b9aa0(spec-citing ValueError on a baddomain, §9 citations for located refusals,read_celldocstring covers the temporal sibling),6c5f9c6(times=True+subtree=restricted-sweep coverage mirroring the locations test),b10099b(corrected the overview-parity docstring overclaim and added goldens-free decode-side value assertions — per-level centroid weight sums equal the leaf total derived fromexpected.json),f7f2eb3(CHANGELOG: name thetimesbinding, not a{field}_timesnaming convention). Every thread has a reply with the fix sha.92f4598(itemize the FULL §2 delta in the pin record — §2.0 new, §2.1 rescoped undercounts+ flux bullet, §2.2 rewritten with the heterogeneous-orders reader MUST and the now-textual "never a refusal (§9)" sentence; also restructured the pin record into its own paragraph),46fb641(state §2.0's third arm: an UNKNOWNweightsvalue MUST be refused and currently decodes silently — the Gate reads on the §2.0 weights declaration (counts vs flux) + advance the spec-text pins #43 gate covers that refusal; the mixed-merge MUST is vacuous here, no payload-merge entry point exists),6aaf8d3(accurate Gate reads on the §2.0 weights declaration (counts vs flux) + advance the spec-text pins #43 framing — see the standing item below),95e2d54(§1.1 vs §1.2 split in the delta inventory; byte-identity widened to §1.3–§1.6),f74c042(§6 added to the delta record: §6.1 gains{field}_timesin the /2 element declaration, §6.3 the times/weights-ride-the-migration note). Every thread has a reply with the fix sha.Suite at the final head
f74c042: 850 passed, 2 skipped (base was 804, +46 tests);ruff check/ruff format --checkclean; CI green on every push (3.12/3.13 matrix + the zagg live-parity leg + ruff + notebook).