D19 hash epoch: granule_workers into DATA_SOURCE_PACKAGING_KEYS; semantic_core widened to the leaf-shaping output knobs (issue #415) - #420
Conversation
espg
left a comment
espg
left a comment
There was a problem hiding this comment.
🤖 from Claude (review)
Adversarial self-review of phase 1 (granule_workers / shard_workers into DATA_SOURCE_PACKAGING_KEYS), against CLAUDE.md and issue #415's acceptance.
Three findings, no blockers: one scope question (the unruled second spelling), one doc-accuracy nit at the constant, and one test-depth finding — the two new tests pin the semantics layer but not the seam where the clamp-sensitivity actually bit. The epoch framing itself is sound: phase 1 alone moves a hash only for configs that spell a fan-out width, so the "every pre-epoch hash is invalidated" claim in the body rests on phase 2, not this commit — worth keeping straight when the migration note is written.
Generated by Claude Code
| DATA_SOURCE_PACKAGING_KEYS = ( | ||
| "reader", | ||
| "driver", | ||
| "read_plan", | ||
| "anonymous", | ||
| "shard_workers", | ||
| "granule_workers", |
There was a problem hiding this comment.
🤖 from Claude (review)
[medium] shard_workers is not in the ruled scope, and it does not fix the defect question (7) actually reported.
Issue #415 scope item (1) is one key, spelled once: "add granule_workers to DATA_SOURCE_PACKAGING_KEYS". CLAUDE.md §6 puts "change the PR's agreed scope" on the @espg-only list, so the burden is on the diff to show the second key is implied rather than added.
It is not implied by the reported mechanism. The clamp writes exactly one key:
# runner._clamped_data_source
return {**data_source, "granule_workers": clamped}so granule_workers alone closes the clamp-sensitivity, the _EQ_OR_NONE_KEYS rollup collapse, and every consequence question (7) enumerates — including for configs that spell shard_workers, because the clamp appends granule_workers regardless of which name the config used. shard_workers in the tuple therefore buys a different property (an operator retuning fan-out width does not mint a new product identity), which is defensible on D19's "worker size is packaging" line but is a second decision, not the ruled one.
The counter-argument in the PR body (a second epoch would be needed later) is real, but it cuts both ways: an unruled key silently riding a hash epoch is exactly the kind of thing that is invisible in review and unrecoverable after release. Either drop it to the ruled key and leave both spellings as question (2), or — if it stays — the constant's comment should say at the constant that the second spelling is unruled and awaiting a ruling, so a reader of the code (not the PR body) knows.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Split: the code-comment half is folded in 2d6921b; the removal is declined and stands as question (2) for @espg.
You are right that granule_workers alone closes the reported drift — the clamp appends that key whatever the config spelled, so the clamp-sensitivity, the _EQ_OR_NONE_KEYS collapse, and the dedup.shard_status stale all resolve without the second key. The constant now says exactly that instead of implying the pair was one decision:
#: The clamp writes ``granule_workers`` whatever the config spelled, so that
#: key alone closes the reported drift; ``shard_workers`` rides along because
#: the two ARE one knob (``zagg.processing.worker._granule_workers`` resolves
#: the canonical name first) and hashing the canonical spelling of an excluded
#: knob would be incoherent. Issue #415 named only the legacy alias, so the
#: canonical one is flagged for ruling on the epoch PR rather than assumed.
Why it is not removed on agent authority either way. Removing it is as much a call as keeping it, and the asymmetry decides: if shard_workers stays hashed and @espg later wants it excluded, that needs a second epoch — issue #415's own framing for the granule-id passenger ("it cannot ride a later train without a second epoch"). Keeping it costs a one-line revert before merge, which is the cheaper error. It is not silent: it is in the commit, in the constant, in the PR body's "what this does", and it is question (2) with an explicit "(b) drop back to granule_workers only" option.
Generated by Claude Code
| @@ -54,7 +55,27 @@ | |||
|
|
|||
| #: ``data_source`` keys that are read machinery, not output semantics (D19). | |||
There was a problem hiding this comment.
🤖 from Claude (review)
[low] The constant's one-line summary is now false, and it is the line a reader greps.
The module docstring was updated to "read machinery and fan-out sizing", but this line — the #: summary that lands in the API docs next to the tuple and is what anyone scanning the file reads first — still says "read machinery, not output semantics". shard_workers/granule_workers are not read machinery; they are worker sizing, which D19's exclusion list names as a separate category from read knobs:
Excluded as packaging: cell order (a resolution axis — D24), parent/shard order,
chunk_inner/sharded, worker size, streaming mode ... and read knobs
Two categories, one summary line claiming only one of them.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Folded in 14b670d. The summary line now names both categories, matching D19's own two-category exclusion list rather than only the read-knob half:
#: ``data_source`` keys that are read machinery or worker sizing, not output
#: semantics (D19 names the two as separate packaging categories).
#: Changing any of these must never change the ``semantic_hash``.
Generated by Claude Code
| def test_dispatch_clamp_never_moves_the_hash(self): | ||
| # The exact mechanism question (7) reported: the dispatcher hands each | ||
| # cell a data_source clamped to min(K, n_granules) (issue #184), and | ||
| # the worker-side fallback hashes THAT config. A 2-granule cell must | ||
| # not hash differently from the run. | ||
| from dataclasses import replace | ||
|
|
||
| from zagg.runner import _clamped_data_source | ||
|
|
||
| cfg = _cfg() | ||
| clamped = _clamped_data_source(cfg.data_source, 2) | ||
| assert clamped is not None and clamped["granule_workers"] == 2 | ||
| assert semantic_hash(replace(cfg, data_source=clamped)) == semantic_hash(cfg) | ||
|
|
There was a problem hiding this comment.
🤖 from Claude (review)
[medium] Both new tests stop one seam short of the thing that was broken.
The reported defect is not "a config carrying granule_workers hashes differently" — it is that the worker-side fallback hash disagrees with the run hash, at these two call sites:
# processing/raster.py
if semantic_hash is None:
from zagg.semantics import semantic_hash as _semantic_hash
semantic_hash = _semantic_hash(config)with the same shape in hive.process_and_write_hive. test_dispatch_clamp_never_moves_the_hash reconstructs the clamp by hand (_clamped_data_source(cfg.data_source, 2)) and hashes the result, which pins the composition of two functions but not the seam that composes them. If a later change made a seam hash something other than the config it was handed — the raster handler dropping keys is the live precedent, PR #397 question (1) — these tests stay green while the drift is back.
A gate-level pin is cheap here and exists already in a form you can copy: tests/test_hive.py's skip-gate tests drive process_and_write_hive with skip_if_current=True and no semantic_hash. One test that folds a leaf under the run config, then reruns the same unit with the clamped per-cell config and asserts current: True (rather than identity: "semantic-mismatch") would pin the actual claim: a small shard's worker no longer reads its own leaf as stale. Without it, phase 1 is pinned only where it cannot regress.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Folded in b882d94 — tests/test_hive.py::TestLeafSkipIfCurrent::test_a_clamped_per_cell_config_still_reads_current, exactly the shape you named: write + stamp + sidecar the leaf under the RUN config, then rerun the same unit with the dispatcher's clamped per-cell config and no semantic_hash, so the seam's fallback is the thing under test.
clamped = _clamped_data_source(cfg.data_source, len(self.URLS))
assert clamped is not None and clamped["granule_workers"] == len(self.URLS)
self._arm_boom(monkeypatch)
meta = hive.process_and_write_hive(
shard, list(self.URLS), grid, {}, root,
replace(cfg, data_source=clamped), store_kwargs={}, skip_if_current=True,
)
assert meta["current"] is True and meta["identity"] == "equal"_arm_boom makes it a real skip pin rather than an identity-string pin: the fold raises AssertionError("fold ran on a gated unit") if the unit does not skip, so a reintroduced drift fails on the fold, not only on the classification. Pre-epoch this asserts identity == "semantic-mismatch", which is the regression it exists to catch.
Not extended to the raster seam here: the raster branch of the fleet handler drops semantic_hash on the floor today (PR #397 question (1), still unarmed), so a raster gate pin would be testing a path no fleet run reaches. Named rather than silently skipped.
Generated by Claude Code
espg
left a comment
espg
left a comment
There was a problem hiding this comment.
🤖 from Claude (review)
Adversarial self-review of phase 2 (semantic_core widened to the leaf-shaping output knobs).
Five findings. The pyramid exclusion is the right call and well-grounded — the declare_pyramid retrofit and hive.leaf_column_expectation both say so, and the branch proves it by leaving test_the_declaration_moves_neither_identity_half green. The substantive worry is the opposite one: sharded is the knob the epoch is being spent on, and hashing the declaration rather than the effective layout leaves the object-layout hole half-open (chunk_inner) while opening a new false split (K == 1). Second, two of the four keys in the new constants (emit_cell_ids, and arguably sharded itself) are inferred rather than named by the filing, and one of them had a ratified exclusion and a dedicated test that this diff deletes — that should be on the record for @espg, not folded into a docstring. Third, the class pins hashes but not the gate behavior the hashes exist to drive.
Generated by Claude Code
| # grid-family-shaped exactly as `grids.from_config` resolves it — HEALPix | ||
| # output defaults to sharded (issue #215), rect does not — so an explicit | ||
| # `sharded: true` on a HEALPix config hashes identically to omitting it. | ||
| grid["sharded"] = get_sharded(config, default=grid_type == "healpix") |
There was a problem hiding this comment.
🤖 from Claude (review)
[high] The core now hashes the sharding declaration, but the leaf's object layout is decided by the declaration ∧ the orders — so this both over- and under-discriminates, in opposite directions, on the exact knob the epoch exists for.
Over. The grid no-ops sharding at K == 1 — get_sharded's own docstring: "a K==1 grid has nothing to bundle, so the grid silently no-ops it (issue #215)". So on a K == 1 config, sharded: true and sharded: false produce byte-identical leaves and now hash apart. That is a false product split: two runs that would write the same bytes refuse each other's stores.
Under. chunk_inner is what sets K, and it stays packaging by the D24 line this diff deliberately keeps:
if chunk_inner is None and sharded and layout == "fullsphere" and not is_raster:
derived = child_order - 6
if derived > resolved_parent:
chunk_inner = derivedSo chunk_inner: 11 → 13 changes the leaf's object set completely — different K, different shard object extents — with sharded unchanged, and the hash holds. The gate still reads equal for the very failure mode question (8) described ("changes the leaf's object layout while the hash and the granule set both hold"), just entered through a different key.
Net: the epoch pays a full compatibility break for sharded and closes maybe half of the object-layout hole, while adding a new false-split at K == 1. Three coherent positions, none of them "what landed":
- resolve the effective sharding (
sharded ∧ K > 1) rather than the declared flag — kills the false split, but needs the orders to compute K, which is precisely what D24 keeps out of the core; - take the whole packing group (
sharded+chunk_inner) and accept that chunk geometry is product identity — coherent, but a much bigger bite out of D24 than the filing describes; - drop
shardedand put the object-layout half in the D20 sidecar instead — question (8) option (b), which espg did not pick.
At minimum the K == 1 false split needs to be stated at the constant, because nothing else in the tree says it.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Split: the documentation half is folded in a8cdb00; the three-way design fork is declined and stands as question (3) for @espg.
Both mechanisms confirmed as described. The K == 1 no-op is get_sharded's own documented behavior, and chunk_inner is what decides K, including via the issue #259 derivation quoted. So the constant now states both, rather than leaving the reader to find them:
#: * at ``K == 1`` the grid silently no-ops sharding (issue #215), so
#: ``sharded: true`` and ``sharded: false`` write identical leaves and
#: still hash apart — an over-discrimination, the safe direction (F1's
#: recorded posture: "over-discriminating is safe, a semantic collision
#: is not");
#: * ``chunk_inner`` changes K, hence the leaf's object set, and still
#: hashes equal — the object-layout hole is narrowed here, not closed.
Why the fork is not taken here. Your three options are three different answers to "how much of D24 does this epoch eat", and each one is a bigger decision than the filing authorizes:
- (1) resolving
sharded ∧ K > 1needschild_order/chunk_inner/parent_orderinsidesemantic_coreto compute K. Even if the orders are only read and never hashed, the core's value would then depend on them, which is the composability property D24 protects — o8 and o9 runs of the same product could land on different sides of theK > 1boundary and split. - (2) hashing the packing group outright contradicts the ratified exclusion line verbatim (
chunk_inner/shardedare named together there). - (3) is question (8) option (b), which espg explicitly did not pick.
The direction that landed is at least the safe one at both ends: the K == 1 case over-discriminates (a spurious rewrite, never a false skip), and the chunk_inner case is no worse than pre-epoch. It is now question (3) on the body with those same three options, since only @espg can size the bite out of D24.
Generated by Claude Code
| #: - ``emit_cell_ids`` (issue #304) — the D16 transition hatch writes an | ||
| #: ADDITIONAL ``cell_ids`` array into every leaf, which is a leaf-content | ||
| #: difference by the same test, not a display preference. | ||
| GRID_LEAF_SHAPING_KEYS = ("sharded", "emit_cell_ids") |
There was a problem hiding this comment.
🤖 from Claude (review)
[medium] emit_cell_ids is a second unruled key, and unlike shard_workers it was excluded by name in this very docstring.
The pre-epoch text you edited read:
Excluded as packaging: all orders (
parent_order/child_order/chunk_inner),sharded, store layout/path,emit_cell_ids(the issue #304 transition hatch), worker sizing, ...
and tests/test_semantics.py carried a dedicated assertion for it with a stated rationale ("emit_cell_ids is the issue #304 transition hatch, not identity (F4)"), which this diff deletes. Issue #415 names sharded nowhere either, but sharded is at least the knob PR #397 question (8) checked and reported. emit_cell_ids appears in neither the filing nor the question — it is inferred from the criterion.
The inference is sound (an extra cell_ids array in every leaf is a leaf-content difference by exactly the test the constant states), but it has a consequence the diff does not mention: the hatch is scheduled for removal (#304, gated on the gridlook morton decode). Once it is removed, a store built with emit_cell_ids: true has a hash no supported config can reproduce — its config is illegal, so the store's frozen key is unverifiable rather than merely stale. Default-off stores are unaffected because the resolved False is what they hash today, but the hatch's users are exactly the population that would notice.
Not a defect — a decision that should be espg's, on the record, alongside question (2). Deleting a test that carried a ratified rationale is the part that should not pass silently.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Folded in 6be4b1a, and raised as question (4) on the body. You are right that the deleted assertion carried a stated rationale and should not have gone quietly. The constant now says both halves out loud:
#: difference by the same test, not a display preference. **Inferred from
#: the criterion, not named by issue #415** (and previously excluded here
#: by name), so it is flagged for ruling on the epoch PR alongside
#: ``shard_workers``. Note the hatch is scheduled for removal: after that,
#: a store built with it ON has a hash no legal config reproduces.
The deleted test is not restored, because restoring it would assert the opposite of what the code now does — the replacement is TestLeafShapingOutputKnobs::test_emit_cell_ids_is_identity, which carries the new rationale in the same place. What the old test's rationale ("the core reads only the grid type/scheme, so it drops out") described was a mechanism, not a ruling: the key dropped out because nothing in output was read at all, so the F4 note recorded a consequence of the old scope rather than a decision to exclude this knob specifically. That is the argument for the change, and it is exactly the argument @espg should be able to reject in one line — hence question (4), with "(b) drop emit_cell_ids back out" as an explicit option.
On the removal consequence: it is real but bounded to hatch-on stores, and it is now stated at the constant rather than only here.
Generated by Claude Code
| def _windowing_leaf_shape(config: PipelineConfig): | ||
| """The normalized ``output.windowing`` declaration, or ``None`` (D13). | ||
|
|
||
| Normalized rather than as-spelled so ``epoch`` spellings and the issue | ||
| #355 point-window sugar canonicalize (§8.3). Total by contract: an | ||
| out-of-grammar block is ``_validate_windowing``'s refusal to raise, not | ||
| the hash's — :func:`semantic_core` must not start raising on a config that | ||
| hashed before the epoch (the Lambda worker builds its config without | ||
| ``validate_config``), so such a block simply hashes as spelled. | ||
| """ | ||
| from zagg.config import get_windowing |
There was a problem hiding this comment.
🤖 from Claude (review)
[medium] The totality guard is a four-exception catch that was grown by test failure, and it now swallows any bug inside get_windowing as "hash it as spelled".
AttributeError was added only after test_the_core_stays_total_on_an_out_of_grammar_block failed on a string block — which is the guard doing its job, but it is also evidence that the exception set is empirical rather than derived. get_windowing calls into zagg.windows.parse_utc and _explicit_window_bounds; a genuine regression there (a bad ISO round-trip, a mis-parsed explicit window) raises one of these four and is then silently downgraded to hashing the raw block. The store gets a hash, the run proceeds, and the only symptom is that two configs which should collide do not.
Cheaper and tighter: normalize only what the guard is actually for. The failures you are defending against are all shape failures on an out-of-grammar block, which is one isinstance check:
block = (config.output or {}).get("windowing")
if not isinstance(block, dict):
return block # out of grammar; validate_config refuses it by name
return get_windowing(config)That keeps totality for the case that motivated it (non-dict blocks) and lets a real fault inside the normalizer raise, where it is findable. If a dict-shaped-but-invalid block must also stay total, catch on that path explicitly rather than around the whole call.
Also worth pinning either way: the guard means a windowed config whose block is slightly wrong hashes differently from the same block spelled correctly, which is safe (over-discrimination) but currently undocumented and untested — there is no test that a raising block still yields a stable, repeatable digest.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Folded in 3747cb9 — the guard is now split by fault class rather than wrapped around the call, and your point that the exception set was empirical is exactly right (AttributeError was added reactively).
block = (config.output or {}).get("windowing")
if not isinstance(block, dict):
return block
try:
return get_windowing(config)
except (KeyError, ValueError):
return blockShape faults are refused by isinstance; a mapping missing a required key or carrying an unparseable epoch/window bound raises KeyError/ValueError, which is a GRAMMAR fault _validate_windowing also refuses by name. TypeError/AttributeError no longer catch — those would be a fault inside the normalizer and now propagate to where they are findable, which is the property you asked for. The docstring states that split, so the next person extending the catch has to argue with it.
The dict-shaped-but-invalid arm is kept total deliberately (your "if it must, catch on that path explicitly"): the Lambda worker builds its config without validate_config, so the hash is reachable ahead of any refusal.
Your untested-stability point is folded too — test_the_core_stays_total_on_an_out_of_grammar_block is now parametrized over both fault classes and asserts three things per block, not one: 64 hex chars, the same digest twice, and no collision with the unwindowed store.
Generated by Claude Code
| "96e5b12005fcf09a9508e643abb7eecd7893220adef2ae75cf2b34f40c04d4c1" | ||
| ) | ||
|
|
||
| def test_pipeline_type_is_semantic(self): |
There was a problem hiding this comment.
🤖 from Claude (review)
[medium] Phase 2 pins that the hash moves; it never pins the thing the hash moving is for.
Question (8)'s claim is behavioral and testable in one place: a sharded flip over an existing store must now rewrite instead of reading equal. Every test in TestLeafShapingOutputKnobs stops at semantic_hash(a) != semantic_hash(b), which is a statement about this module, not about the gate. The gate could stop consulting the hash tomorrow and this class stays green.
Phase 1's fold already established the pattern and the fixture — tests/test_hive.py::TestLeafSkipIfCurrent, _write_leaf + skip_if_current=True + _arm_boom. The mirror is four lines of setup:
grid, shard, root, _ = self._write_leaf(monkeypatch, cfg, tmp_path)
flipped = copy.deepcopy(cfg)
flipped.output["grid"]["sharded"] = False
# ...rerun the unit under `flipped`, assert identity == "semantic-mismatch"Note the grid must be rebuilt from flipped for the rerun to be honest — reusing the leaf's grid would pass the old sharding into the write path and test nothing. Without this, the PR's central claim ("the gate can now see a leaf-shape change") is asserted only in prose.
test_column.py::TestColumnDefeatsTheSkipGate is the precedent for exactly this shape of test, and it is worth noting that its premise test — test_the_declaration_moves_neither_identity_half — still passes on this branch, which is the check that the pyramid exclusion decision actually holds end to end.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Folded in 2165be9 — tests/test_hive.py::TestLeafSkipIfCurrent::test_a_sharded_flip_defeats_the_skip, and your note about rebuilding the grid was the load-bearing part:
flipped.output["grid"]["sharded"] = False
flipped_grid = self._grid(flipped)
calls = self._counting_fake(monkeypatch, flipped_grid)
meta = hive.process_and_write_hive(
shard, list(self.URLS), flipped_grid, {}, root, flipped,
store_kwargs={}, skip_if_current=True,
)
assert calls == [int(shard)] and meta["identity"] == "semantic-mismatch"It uses _counting_fake rather than _arm_boom because this arm must assert the fold DID run (calls == [int(shard)]), which is the behavioral claim — pre-epoch it is [] with identity == "equal".
Your closing observation is the other half and it holds on this branch: test_column.py::TestColumnDefeatsTheSkipGate::test_the_declaration_moves_neither_identity_half still passes, which is the end-to-end evidence that excluding pyramid was right — the column's currency is still carried by the artifact read (hive.leaf_column_expectation), not by the hash, so declare_pyramid's retrofit keeps working. That test failing was in fact how the pyramid inclusion was caught before this commit.
Generated by Claude Code
| #: precisely to add a pyramid declaration to the config that built a store, | ||
| #: which its own semantic guard would then refuse. Hashing it would break a | ||
| #: supported workflow to re-cover ground already covered. | ||
| OUTPUT_LEAF_SHAPING_KEYS = ("aoi_mask", "windowing") |
There was a problem hiding this comment.
🤖 from Claude (review)
[low] windowing is already frozen at two other layers, so it is the one key in this constant that closes no reported hole — and the docstring should say which layer it is actually for.
hive._FROZEN_MANIFEST_KEYS already includes temporal:
_FROZEN_MANIFEST_KEYS = (
"spec", "dataset", "semantic_hash", "cell_order",
"shard_order", "split_schedule", "path_grouping", "temporal",
)so a windowing change already refuses pre-dispatch at ensure_manifest, and separately, windowed leaves are named {window}.zarr (D23), so a schedule change lands the unit on a leaf path with no sidecar → no-sidecar → rewrite. Both guards predate the epoch.
That does not make it wrong to include — the D20 sidecar identity a fleet leaf compares is not the manifest, and product identity in a multi-product root is not either — but the constant currently justifies it with "a windowed and an unwindowed store over the same granules shared a hash before the epoch", which reads as though a hole were open. Say instead which layer gains: product identity and the D20 identity half, not the manifest guard. Otherwise the next reader trying to work out what the epoch bought will re-derive the two existing guards and conclude the key was unnecessary.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Folded in fbcccaf. Both existing guards verified as described — temporal is in _FROZEN_MANIFEST_KEYS, and a schedule change lands the unit on a {window}.zarr path with no sidecar (D23) → no-sidecar → rewrite. The constant now names the layer that actually gains instead of implying an open hole:
#: granules shared a hash before the epoch. What this buys is **product
#: identity and the D20 sidecar's identity half**, not the manifest guard:
#: ``temporal`` is already a frozen manifest key ... so pre-dispatch refusal
#: and the leaf gate were already covered — the hash is what a multi-product
#: root and a fleet leaf's recorded identity compare.
Kept rather than dropped for the reason you allow: the two existing guards are store-scoped and path-scoped, and neither reaches the recorded semantic_hash that a fleet leaf's D20 sidecar compares or that distinguishes two products under one root. Over-discrimination on a knob that genuinely changes leaf content is the safe side of F1's recorded posture, and the epoch is the only free moment to add it.
Generated by Claude Code
espg
left a comment
espg
left a comment
There was a problem hiding this comment.
🤖 from Claude (review)
Adversarial self-review of phase 3 (conformance fixtures + the spec cross-reference).
The regeneration itself is clean and verifiable: the whole tests/data/spec/ diff is 26 lines, all of it semantic_hash and generated_at — no compressed-byte churn, no content_hashes movement, which is the evidence that the epoch is contract-invisible at the byte level (§5 hashes are over decoded values). Two findings, both about enforcement rather than the diff: the fixture obligation is unguarded by any test, and the spec page now documents the output.* rule only in a §6 parenthetical while docs/hive_layout.md still carries the pre-epoch claim.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude (review)
[medium] The fixtures were regenerated, but nothing in the tree would have noticed if they had not been — so the epoch's own fixture obligation is unenforced.
grep -rn semantic tests/test_spec_conformance.py returns nothing: the conformance suite asserts decoded values and §5 content hashes, and no test recomputes a fixture's semantic_hash from the config that built it. The full suite passed on this branch before this commit with the stale hashes still in place. So the regeneration is correct and invisible, which is the bad combination — the next semantic-core edit that forgets it produces fixtures whose semantic_hash is a value no zagg reproduces, moczarr vendors them (espg/moczarr#19/#20), and the parity gate stays green because it does not look at that field either.
The generator already carries every input needed to close this — it builds each fixture from a config in the same process. A single conformance assertion (fixture manifest's semantic_hash == semantic_hash(that fixture's config)) would make the obligation self-enforcing rather than a CLAUDE.md §4 habit.
Related, and the reason this is worth more than a nit: the generator's own docstring already warns about the opposite failure mode ("STALE BY DESIGN … real churn in files no test asserts"), which is precisely a statement that this part of the fixture tree is unguarded.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Folded in fc608e8 — tests/test_spec_conformance.py::TestFixtureSemanticHash, four assertions, and your diagnosis was exact: the suite passed on this branch with the stale hashes still committed, so the obligation was a habit rather than a gate.
The generator loads as a module through the test_content_hash precedent, so each fixture's own config is available:
gen = self._generator()
assert self._recorded(name) == semantic_hash(gen._config(**kwargs))parametrized over minimal / kitchen_sink / column, plus one for pyramid/ that is worth more than reproducibility: its manifest is built from the /1 config and then retrofitted by declare_pyramid under the /2 one, so
assert semantic_hash(cfg_v1) == semantic_hash(cfg_v2)
assert self._recorded(PYRAMID) == semantic_hash(cfg_v1)pins the digest and the property that keeps the retrofit legal — the pyramid block staying outside the core. That is the phase-2 exclusion decision, now enforced by a conformance fixture rather than only by a unit test.
Generated by Claude Code
| *(Informative.)* Writing `/2` will be a per-product opt-in | ||
| (`output.ragged_encoding: typed`), which shifts the product's | ||
| `semantic_hash` — a new product identity, by design. The default stays `/1`; | ||
| flipping it is a schema epoch deferred to its own ruling (public/interop | ||
| stores may deliberately stay `/1` for vanilla-zarr openability). | ||
| `semantic_hash` — a new product identity, by design. That shift is not | ||
| automatic: `output.*` keys reach the semantic core only by being listed as | ||
| leaf-shaping (`zagg.semantics.OUTPUT_LEAF_SHAPING_KEYS`, issue #415), so the | ||
| `/2` implementation PR must add the knob there in the same change. The |
There was a problem hiding this comment.
🤖 from Claude (review)
[low] This edit is correct and useful, but it makes the spec page's silence elsewhere louder.
Pointing the future /2 PR at OUTPUT_LEAF_SHAPING_KEYS is right — pre-epoch the sentence was aspirational (no output.* key could shift the hash at all), and post-epoch it is conditionally true, which is worth saying.
But this is now the only place in docs/specification.md that mentions how output.* reaches the semantic core, and it is a parenthetical inside §6's ragged-encoding note — a reader looking for the rule will not find it there. §4.7's informative scope-out and the O11 note at §5 both touch product identity without it. Either the epoch deserves one informative sentence where identity is actually discussed, or this cross-reference belongs in docs/hive_layout.md (where phase 4 is putting the migration note) with §6 keeping a pointer to it.
Also worth checking before this lands: docs/hive_layout.md:405 still reads "output.* is not in the semantic core, so adding output.pyramid to the original config hashes identically". The second clause survives the epoch (that is exactly why pyramid was excluded); the first clause is now false, and it is the sentence that documents declare_pyramid's semantic guard — the one workflow whose correctness depends on getting this distinction right.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Both halves folded. The docs/hive_layout.md:405 claim is fixed in 7a3a705 — you were right that it is the highest-stakes sentence in the tree on this point, since it documents the one guard whose correctness turns on the distinction:
Adding
output.pyramidto the original config hashes identically, so the retrofit never false-refuses — the wholepyramidblock is deliberately outside the semantic core, and keeping this workflow working is one of the reasons why (the D19 hash epoch of issue #415 put the leaf-shapingoutputknobs into the core, listed inzagg.semantics.OUTPUT_LEAF_SHAPING_KEYS;pyramidis not one of them).
The surviving clause is kept and the false one is replaced by the reason it survives, rather than deleted — a reader who wonders "why does the retrofit still work after the epoch" now gets the answer at the guard.
On placement: taking your option (b). docs/specification.md stays as-is with the §6 parenthetical as a pointer, and the operator-facing statement of the epoch lands in docs/hive_layout.md in phase 4, where §4.7's informative scope-out already places this family and where the two guards it interacts with (the frozen-key manifest check and the leaf identity gate) are already described. Putting normative-adjacent identity language into the spec page would be a bigger move than the filing calls for — the derivation of semantic_hash has never been spec-normative, and this PR is not the place to make it so.
Generated by Claude Code
espg
left a comment
espg
left a comment
There was a problem hiding this comment.
🤖 from Claude (review)
Adversarial self-review of phase 4 (the migration note + the D19 design-record amendment).
The note does the thing the filing asked for — it states what changed and argues why re-hashing is correct rather than a defect, and the "wrong in both directions" framing is the right argument. Three findings, one of which I would not let ship as written: migration path (3) understates what a manifest-only restamp does (it is not one field, and it manually defeats the guard that exists to catch a wrong config). The other two are coverage: the fleet consequence and the arming unblock are absent, and the design-record amendment sits after the sentence it contradicts with no marker at the contradiction.
Generated by Claude Code
| 3. **Restamp in place.** The data really was produced by this config, so | ||
| rewriting the manifest's `semantic_hash` to the new digest | ||
| (`python -c "from zagg.config import load_config; from zagg.semantics import | ||
| semantic_hash; print(semantic_hash(load_config('cfg.yaml')))"`) is | ||
| *semantically* correct. Be aware of the cost: every leaf's D20 stats | ||
| sidecar records the **old** digest too, so a rerun then classifies each | ||
| unit `semantic-mismatch` and rewrites it wholesale — the store self-heals, | ||
| leaf by leaf, at the price of one full re-aggregation spread over reruns. | ||
| There is no zagg tool for this; it is a deliberate operator action. | ||
|
|
||
| Whichever path you take, the first post-epoch run over a store is a **full |
There was a problem hiding this comment.
🤖 from Claude (review)
[high] Path (3) hands the operator a one-liner and understates what it does to a store — and one of its two stated consequences is wrong in the operator's favour, which is the dangerous direction.
The claim is that restamping the manifest leaves every leaf sidecar carrying the old digest, so units classify semantic-mismatch and the store "self-heals, leaf by leaf". Two problems:
- A
semantic-mismatchrewrite is not a heal, it is a full re-aggregation of that unit — the same cost as path (2), except paid unpredictably over future runs instead of once, deliberately. The text says "at the price of one full re-aggregation spread over reruns" only at the end of a sentence that opens with "semantically correct", and the recommended-path ordering puts it last but does not mark it as the one with a footgun. - The restamp is not one field.
semantic_hashalso rides leaf attrs and every D20 sidecar (the design record: "is recorded in leaf attrs and D20 sidecars"), anddedup.shard_statusconsults the sidecar's copy independently of the manifest. So a manifest-only restamp produces a store whose root says one thing and whose leaves say another — a mixed-identity store, which is a state nothing else in this document describes and no zagg code expects to persist. That is worth more than a cost note.
Concretely, the snippet is also load-bearing in a way it should not be: it prints a digest for whatever config file the operator names, with no check that the config is the one that built the store. Get that wrong and the restamp installs a digest for a different product, and then the frozen-key guard — the thing that exists to catch exactly this — has been manually defeated and will never fire again for that store. Path (1) has no such failure mode.
Either drop path (3), or gate it: state that the config must be verified as the store's own first (its pre-epoch digest recomputed under the previous zagg and matched against the manifest), and say plainly that the result is a mixed-identity store until every leaf has been rewritten.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Folded in e0711d8 — path (3) is gated rather than dropped, with all three of your objections stated at the path instead of implied. The one-liner is gone.
It now leads with "expert path, read the whole entry" and enumerates:
- not one field —
semantic_hashrides leaf attrs and every D20 sidecar, anddedupreads the sidecar's copy, so a manifest-only restamp leaves a mixed-identity store until every leaf is rewritten. Your wording, because it is the accurate one; the design record's "recorded in leaf attrs and D20 sidecars" is the citation; - not a heal — the rewrite is the same re-aggregation as path (2), paid unpredictably instead of once deliberately;
- it defeats the guard — nothing checks that the config being hashed is the store's own, and after a foreign digest is installed the frozen-key check never fires for that store again. The mitigation is stated as a precondition: recompute the config's pre-epoch digest under the previous zagg and match it against the recorded one first.
Kept rather than dropped because it is the only path that preserves an existing large store's data without a new prefix, and an operator who needs it will do it whether or not it is documented — better documented with its footguns named. Paths (1) and (2) are now explicitly called out as having none of these failure modes.
Generated by Claude Code
| construction; the root object simply doesn't appear until the sweep or a | ||
| refresh builds it. | ||
|
|
||
| ## Migration: the D19 hash epoch |
There was a problem hiding this comment.
🤖 from Claude (review)
[medium] The note documents the manifest refusal thoroughly and the fleet consequence not at all — and the fleet is where the epoch actually costs money.
Everything under "What an operator sees" is the pre-dispatch guard: a ValueError from validate_manifest, three ways forward, done. But on the Lambda path the manifest write is an async init-time Event invoke with a finalize backstop (issue #252 hybrid, described 500 lines above in this same document), and the fleet's leaf gate is not armed at all yet — PR #397 question (1), the raster branch of which drops semantic_hash from its record entirely. Those two facts change what an operator experiences:
- the
validate_manifestprecheck does run before fan-out on the ping, so the refusal still fires first — worth saying, because a reader who knows the manifest write is asynchronous will reasonably wonder whether an epoch mismatch is caught before or after a few thousand workers have run; - and once the gate is armed post-epoch, the first fleet run over a migrated store rewrites every leaf, which for the pole figure is a full re-aggregation — the sentence at the end ("the first post-epoch run over a store is a full rewrite of everything it touches") is correct but reads as a footnote rather than as the headline cost.
Also missing: the epoch is the thing that unblocks fleet-gate arming (issue #415's own "sequencing consequences" — "Fleet gate ARMING waits for this epoch; this release satisfies it automatically"). An operator reading this section learns what breaks and nothing about what it enables, which is the half that justifies the break.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Folded in 93bb630, all three points.
- The async-manifest question is answered explicitly, because you are right that a reader who knows the write is deferred will ask it: the read-only precheck (
zagg.hive.validate_manifest) runs on themode: "ping"preflight before fan-out, so "an epoch mismatch costs one preflight, not a few thousand worker invocations." - The full-rewrite cost is promoted from footnote to headline — "on a fleet-scale store that is a full re-aggregation, and it is the headline cost of the epoch, not a footnote."
- A new "What the epoch buys" subsection carries the half that justifies the break: fleet arming of the leaf identity gate was gated on this epoch (issue D19 hash epoch: granule_workers into DATA_SOURCE_PACKAGING_KEYS; semantic_core widened to the leaf-shaping output knobs #415's sequencing), for the phase (7) reason — pre-epoch a small shard's worker compared its leaf against a digest the run never wrote, so an armed fleet gate would have rewritten forever without self-healing. It closes on the payoff: the second post-epoch run over an unchanged store is the no-op the gate exists for.
Deliberately not stated there: that the raster fleet branch still drops semantic_hash from its record. That is PR #397 question (1), it is unresolved, and its fix lives in deployment/aws/ — writing an operator-facing promise about raster fleet behavior before that lands would be documenting an intention rather than the code.
Generated by Claude Code
| knobs — hashing the whole template would have made o8 and o9 runs | ||
| different products and blocked mixed-order processing. The hash is a | ||
| different products and blocked mixed-order processing. | ||
| **Amended by the D19 hash epoch** (espg-ruled 2026-08-07 as PR #397 |
There was a problem hiding this comment.
🤖 from Claude (review)
[low] The amendment is appended after the paragraph it contradicts, so the D19 entry now states both positions and the reader has to reach the end to learn which one holds.
The paragraph above still reads chunk_inner/sharded as an exclusion, verbatim and unqualified, and the amendment three sentences later moves sharded out of it. That is the right content — this file is a design record, and records amend rather than rewrite (the D19 entry's own "revision history, all espg-ratified in-session" is the precedent) — but nothing at the exclusion sentence signals that it has been superseded in part. A reader grepping for sharded lands on the stale clause first.
The cheapest fix that keeps the record's amend-don't-rewrite discipline is a marker at the point of contradiction — chunk_inner/sharded (sharded amended below) — rather than editing the original list.
Two smaller things in the same block: the epoch is dated "espg-ruled 2026-08-07" but the ruling comment is the one linked from #384, and this entry elsewhere is careful to date-and-attribute in-session rulings distinctly from thread rulings; and the amendment says operator consequences "are in docs/hive_layout.md" without naming the section, which is the kind of cross-reference that rots the next time that file is reorganized.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Folded in bc910da, all three, keeping the amend-don't-rewrite discipline you named:
- the exclusion list gains a marker at the point of contradiction rather than an edit —
`chunk_inner`/`sharded` (`sharded` amended below — the D19 hash epoch)— so a reader greppingshardedlands on the pointer, not on the stale clause alone; - the attribution is now "espg-ruled on-thread 2026-08-07", matching how this entry distinguishes thread rulings from the in-session ones ("all espg-ratified in-session 2026-07-20");
- the cross-reference names its target section:
docs/hive_layout.md, "Migration: the D19 hash epoch".
Generated by Claude Code
|
🤖 from Claude Status: all four phases landed, all 13 adversarial-review findings resolved (10 folded, 3 declined and standing as questions), CI green except one pre-existing failure. Labeled Two things worth calling out beyond the body: The Two keys in this diff are inferred rather than named by the filing — The one CI failure is Generated by Claude Code |
|
🤖 from Claude Merge-order note: this PR's red The single CI failure on # before — only moves the stamp when the UTC hour string starts with "0"
lambda: real().replace("T0", "T1", 1) if "T0" in real() else real()So: merge PR #421 before this one, or rebase this branch onto it, and the 3.13 job goes green. Merged in the other order, this PR lands with a red check that has nothing to do with its diff. The two PRs touch disjoint code — the only coupling is that shared test file — so there is no rebase conflict risk either way. I have not applied the Two disclosures about how this PR was produced, both process rather than content:
Generated by Claude Code |
| "driver", | ||
| "read_plan", | ||
| "anonymous", | ||
| "credentials_provider", |
There was a problem hiding this comment.
🤖 from Claude (review)
[medium] The epoch excludes the auth half of the fetch-mechanism class but leaves read_workers, source_region and write_buffer in the core — and this epoch is the last cheap moment to rule on them.
The criterion the ruling turns on — "it selects HOW source bytes are fetched … never WHAT is computed from them" — is satisfied verbatim by three data_source keys that are not on this tuple, so they still move semantic_hash (everything not listed falls through _without at src/zagg/semantics.py:422):
read_workers— the third fan-out width in the family phase 1 excluded two of.src/zagg/processing/read.py:849-863: "per-worker read concurrency (issue Compiled (hidefix) read path is never exercised by default, and its fetch/decode is serial #170) … Default 8. Peak RSS grows with width …, so dense-shard configs can dial it down;1is serial", andsrc/zagg/processing/worker.py:98-101sizes the budget asgranule_workers x read_workers x fetch width, telling operators to "dialread_workersdown as K rises". That is worker sizing in D19's own words, and it is a tuned knob (the pinned read-path baseline isread_workers: 8), so two runs of one product at different widths hash apart and every leaf readssemantic-mismatch— the exact defect (7)(b) closed forgranule_workers.source_region— sits in the same dict literal as the already-excludedanonymous,src/zagg/runner.py:907-911:One of those two keys is packaging after this commit and the other is identity.source = config.data_source or {} src_kwargs = { "region": source.get("source_region"), "anonymous": source.get("anonymous", True), }
write_buffer— max slabs alive under the streamed raster sink (src/zagg/processing/raster.py:590-599), a memory/throughput knob only.
What makes this worth raising on this commit rather than later is that the design record it amends now asserts the class is already handled: docs/design/sparse_coverage.md:756 justifies the new exclusion as "the same class as the read knobs and as anonymous, already excluded", and the pre-existing D19 list names "worker size" and "read knobs" as packaging. That is precisely the shape of defect (a) two paragraphs above it (sparse_coverage.md:740-744): "Worker size was already named as packaging but its keys were never excluded". After this commit the sentence is still true for three more keys.
Not proposing a unilateral change — adding keys to the exclusion list is a ruling, not a fold (CLAUDE.md §6). But the docstring's own argument cuts both ways: "Ruled at the epoch deliberately … this costs nothing now and cannot be taken back cheaply later." Either they ride this epoch, or the omission deserves a line in the "Deliberately not changed" paragraph of docs/hive_layout.md (which today names only the orders and pyramid) so a reader cannot mistake it for an oversight.
There was a problem hiding this comment.
🤖 from Claude
Documentation half folded in 0594d400; the exclusion itself is declined on agent authority — adding a key to DATA_SOURCE_PACKAGING_KEYS is a ruling, not a fold (CLAUDE.md §6), so it stays standing as question (2) on the PR body for espg. Option (c) there already names read_workers and write_buffer; source_region is new here and rides the same question now.
docs/hive_layout.md, after the "Deliberately not changed" paragraph:
Also not changed, and not an oversight: three
data_sourcekeys that read as the same fetch-mechanism class as item 1 and item 3 are still in the core, so they still move the digest —read_workers(the third fan-out width, beside the two spellings item 1 excluded),source_region(the raster source-store kwarg that sits in the same dict literal as the already-excludedanonymous), andwrite_buffer(the live-slab bound on the streamed raster sink). Excluding a key is a ruling, not housekeeping, so this epoch carries only what was ruled; the three stand unruled as an open question on the epoch PR (#420), and excluding them later would be its own epoch.
src/zagg/semantics.py, a new #: paragraph directly above the constant, in the voice the block already uses for shard_workers' unruled status:
Unruled, and therefore still hashed:
read_workers(the third fan-out width beside the two spellings above),source_region(the raster source-store kwarg that sits in the same dict literal asanonymous) andwrite_buffer(the live-slab bound on the streamed raster sink) all read as the same fetch-mechanism class as the entries here. Excluding a key is a ruling, not an assumption — the same footingshard_workerswas flagged on above — so they are raised on the epoch PR rather than added, and every one of them still movessemantic_hashtoday.
docs/design/sparse_coverage.md is untouched: its amendment (c) already uses the accurate class wording, and nothing in it became inaccurate.
| joined `DATA_SOURCE_PACKAGING_KEYS` (espg-ruled 2026-08-17): the provider | ||
| name selects *how* source bytes are fetched, never *what* is computed from | ||
| them, so the same granules read with `lpdaac` credentials, `gesdisc` | ||
| credentials, or an anonymous open are one product — `anonymous`, the other |
There was a problem hiding this comment.
🤖 from Claude (review)
[low] "anonymous, the other spelling of the same choice" overstates the relationship — the two keys are read by disjoint pipelines, and the phrasing is repeated in three places.
data_source.anonymous has exactly one consumer, the raster source-store kwargs (src/zagg/runner.py:910, declared only by src/zagg/configs/sentinel2_l2a.yaml:32), and the raster path never resolves a credential provider. credentials_provider has two consumers, both on the other side of that split: runner._resolve_source_credentials for the point path's S3 driver (src/zagg/runner.py:2649-2665) and the temporal fan-out (src/zagg/runner.py:1817). No run reads both — a raster run ignores the provider, a point/temporal run ignores anonymous — so they are not two spellings of one knob; they are two knobs of the same class on two paths.
The conclusion is untouched (both are fetch mechanism, both belong out of the core), but the phrase is doing argumentative work in three places — here, src/zagg/semantics.py:111-112, and the test comment at tests/test_semantics.py (test_credentials_provider_is_packaging) — while the design record already uses the accurate form: "the same class as the read knobs and as anonymous, already excluded" (docs/design/sparse_coverage.md:756). Worth making the three agree on the design record's wording.
There was a problem hiding this comment.
🤖 from Claude
Folded in e8377b3c. Verified the disjointness against src/zagg/runner.py before rewording: anonymous has exactly one consumer, RasterStrategy.run's source-store kwargs (runner.py:913, declared only by configs/sentinel2_l2a.yaml:32); credentials_provider is read by _resolve_source_credentials (runner.py:2662, called from the point path at runner.py:2955/3486 and client.py:791) and by the temporal fan-out (runner.py:1820). No path consults both.
All three occurrences now carry the design record's wording. src/zagg/semantics.py, first bullet:
a hash that moves with the provider is a false product split. It is the same class as the read knobs and as
anonymous, already excluded (a class, not one knob under two names:anonymousis read only by the raster source-store kwargs,credentials_provideronly by the point and temporal paths);
docs/hive_layout.md, item 3:
It is the same class as the read knobs and as
anonymous, already excluded — the same class, not one knob under two names:anonymousis read only by the raster source-store kwargs,credentials_provideronly by the point and temporal paths, so no single run consults both.
tests/test_semantics.py::TestCanonicalization::test_credentials_provider_is_packaging gets the same sentence in comment form.
On test_a_credential_migration_never_rehashes: the comment did overstate, so it was rewritten (assertions unchanged — the both-keys config is still a valid canonicalization input even though no run produces it):
# ...and a config carrying BOTH auth keys drops both. They are two
# knobs of one class on disjoint paths --anonymousis read only by
# the raster source-store kwargs, the provider only by the point and
# temporal paths -- so this pins canonicalization, not a run shape.
|
🤖 from Claude Status: synced with Synced with The old red Phase 5 — the ruled addition. PR #450 interaction. That PR's Phase 6 — a real defect the merge surfaced. Unchanged, and still yours. The five questions in the body stand exactly as filed, as do the three findings the earlier folds declined. Phase 5's own review found two things and split the same way: the The PR stays draft and CI confirms it, on |
|
🤖 from Claude Status: synced with What the merge brought in. The #450 pin-test flip, exactly as forecast in the previous sync comment. #450 landed The Tests. Full suite on the merged tree: effectively 2 failed, ~4185 passed, 38 skipped — both failures pre-existing and previously flagged ( CI. Check-runs fired on Unchanged. The five questions in the body stand exactly as filed, as do the declined findings. Nothing in this sync touches them. The PR stays draft and |
espg
left a comment
espg
left a comment
There was a problem hiding this comment.
🤖 from Claude (review)
Fresh-context adversarial review of phases 7 and 8 only (2cf022e7..623bded0; everything at or below 38785e45 treated as already folded). Six findings, ranked in the inline comments: two blocking (a false back-compat claim in the operator docs, and a contraction that the canonicalization can now hide), two worth fixing (an unsupported docstring claim with a vacuous test behind it, and a dated measurement cited for two knobs it cannot cover), two nits.
Verified locally on the worktree: tests/test_telemetry.py tests/test_dedup.py tests/test_semantics.py tests/test_spec_conformance.py → 314 passed; tests/test_hive.py → 127 passed; ruff check / ruff format --check clean on all eight touched Python files. The hand-edited fixture digest checks out — sha256("a.h5") == d02afc90c38fa5066e787040b91eaeb33b840928aaae85561eb54869fd6a668d, matching all four granules.json edits — and nothing in tests/test_spec_conformance.py or docs/specification.md reads granules_sha256/granules.json, so leaving docs/specification.md untouched is right (but see the note in finding (1) about what that means for coverage). Key names for the three question (2)(c) additions all check out as data_source.* (config.py:495, config.py:1006, runner.py:912) and validate_config still range-checks all three, so nothing became silently unvalidated. emit_cell_ids is cleanly out of semantics.py with no dangling reference and no now-unused import (get_emit_cell_ids is still live in grids/healpix.py:132, and healpix.signature() still carries it — which is the artifact-level cover the ruling leans on).
| The **driver-dependence** caveat is gone. The recorded id space used to be the | ||
| resolved href, so flipping `data_source.driver` between runs read as a full | ||
| mixed contraction and refused per leaf. Since the epoch both sides are reduced | ||
| to the canonical driver-stripped bare granule id (item 5 of the migration | ||
| note), so a driver switch over unchanged granules reads `equal` — including | ||
| against leaves whose recorded list predates the epoch, since the *recorded* | ||
| side is canonicalized on read too. |
There was a problem hiding this comment.
🤖 from Claude (review)
(1) BLOCKING — this paragraph's back-compat claim is false, and it is the one an operator will act on.
"a driver switch over unchanged granules reads equal — including against leaves whose recorded list predates the epoch, since the recorded side is canonicalized on read too."
A pre-epoch leaf cannot reach equal, because the recorded side of the fast path is not a list — it is the stored granules_sha256, which was taken over resolved hrefs. classify_leaf_identity computes hashes_match = rec_hash == granules_sha256(planned) with planned now canonical, so on every pre-epoch leaf that compare is False and the equal arm is unreachable no matter what the sibling holds. Reproduced on this tree:
S3 = ['s3://b/g1.h5', 's3://b/g2.h5']
pre = hashlib.sha256('\n'.join(sorted(S3)).encode()).hexdigest() # href-space, as written pre-epoch
rec = {'granules_sha256': pre, 'semantic_hash': 'a'*64}
classify_leaf_identity(rec, semantic_hash='a'*64, planned_ids=S3, load_recorded_ids=lambda: S3)
# -> {'action': 'rewrite', 'classification': 'id-multiset-drift', 'missing': []}Same granules, same semantic hash, and it still rewrites — plus it pays the sibling GET the fast path exists to avoid. So the operator consequence is the opposite of what is promised here: over a pre-epoch store a driver switch rewrites every leaf once (which is fine and expected at an epoch), it does not skip.
Migration-note item 5 already states the accurate, weaker version — "the classifier canonicalizes the recorded side as well, so a pre-epoch leaf diffs cleanly instead of reading as a full contraction" — and the dedup.py:317 comment says the same thing correctly. Only this paragraph overclaims.
Suggested fix: scope the claim to leaves written at or after the epoch, and say plainly that pre-epoch leaves rewrite once because their recorded catalog digest is in the old space (which the note's own "every pre-epoch hash invalidated" headline already implies). Two related gaps worth closing in the same pass, since nothing currently guards the claim either way: there is no test anywhere in this diff for a pre-epoch (href-space) sidecar hash — every new pin writes both sides post-epoch — and docs/specification.md does not describe granules_sha256/granules.json at all, so the surgical granules.json fixture edits are covered by no assertion in tests/test_spec_conformance.py (TestFixtureSemanticHash only recomputes semantic_hash). A single pin in tests/test_dedup.py with an href-space granules_sha256 on the recorded record would pin the real behaviour and keep the doc honest.
There was a problem hiding this comment.
🤖 from Claude
Folded in 31af5392 (doc + comment + pin) and ab7cd89d (the fixture guard you flagged in the same finding).
You are right and the repro is right — hashes_match compares the stored digest, which on a pre-epoch leaf is href-space, so the equal arm is unreachable there. The paragraph now splits the claim by which side of the epoch the leaf was written on: at-or-after → equal + skip on the hash fast path; pre-epoch → rewrites once, and what canonicalizing the recorded side buys is the direction of that rewrite (id-multiset-drift/expansion instead of refusing as a spurious contraction, so a driver switch over a pre-epoch store no longer needs --allow-contraction).
Verified your other two calls before touching them: migration-note item 5 and the dedup.py comment above planned = canonical_granule_ids(...) are both already accurate, so neither was rewritten — the comment only gained the explicit "such a leaf still REWRITES once, because hashes_match is False by construction" clause so no reader can re-derive the claim the doc used to make.
New pin, tests/test_dedup.py::TestClassifyLeafIdentity::test_a_pre_epoch_leaf_rewrites_rather_than_refusing: a genuinely href-space granules_sha256 (hashlib.sha256 over the sorted hrefs, asserted != granules_sha256(IDS)) with an href-space sibling, parametrized over a canonical plan and the pre-epoch spelling. Both read {"action": "rewrite", "classification": "id-multiset-drift", "missing": []} with exactly one sibling GET — not skip, and specifically not a refusal.
Fixture gap closed too, in ab7cd89d: tests/test_spec_conformance.py::TestFixtureGranuleIdentity now asserts every committed granules.json self-pairs (granules_sha256(granule_ids) == granules_sha256 field) and is canonical-and-sorted (canonical_granule_ids(ids) == ids == sorted(ids)), plus test_every_leaf_fixture_carries_the_sibling, enumerated off TestFixtureSemanticHash.COVERED × each fixture's expected["leaf"] the way test_every_fixture_is_covered does, so a new leaf fixture cannot ship its sibling unguarded (pyramid/ is manifest-only, hence keyed on the leaf). Six siblings, all green; no digest moved.
| # docstring's one-sided-conservatism note). | ||
| return {"action": "rewrite", "classification": "unrecorded-ids", "missing": []} | ||
| recorded_set = {str(g) for g in rec_ids} | ||
| recorded_set = set(canonical_granule_ids(rec_ids) or []) |
There was a problem hiding this comment.
🤖 from Claude (review)
(2) BLOCKING — basename collapse converts a genuine contraction into a silent rewrite, which inverts the guard's one-sided conservatism.
canonical_granule_ids preserves list length, so granules_sha256 still distinguishes a colliding pair. But the diff below is over sets, so once two distinct granules collapse to one canonical id, dropping one of them is invisible:
REC = ['s3://v006/ATL06_X.h5', 's3://v007/ATL06_X.h5', 's3://v007/ATL06_Y.h5']
planned = ['s3://v007/ATL06_X.h5', 's3://v007/ATL06_Y.h5'] # one distinct granule genuinely dropped
classify_leaf_identity(rec, semantic_hash=SEM, planned_ids=planned, load_recorded_ids=lambda: REC)
# -> {'action': 'rewrite', 'classification': 'id-multiset-drift', 'missing': []}Pre-epoch that same input refused (contraction, missing naming the dropped href). This is exactly the direction the class docstring twelve lines up says must never happen: "For the contraction guard it is not [safe]: a rewrite over a contracted input set destroys aggregated data the operator was never told about, which is the loss issue #388 exists to prevent." The refusal manifest records nothing, and the CLI reports an ordinary rewrite.
The collapse is documented as an accepted cost, and the ruling is espg's — but the docs price it as an identity collision only (telemetry.py:157, sparse_coverage.md: "two granules whose hrefs differ only in prefix collapse to one identity"), not as the contraction guard going quiet. That second consequence is the one that costs data, and it is not stated anywhere in this diff.
Suggested fix — cheap and it restores the pre-epoch verdict without touching the ruling: make the diff a multiset. missing = sorted((Counter(recorded_set_ids) - Counter(planned)).elements()) and added = Counter(planned) - Counter(recorded) recovers the refusal under collisions, and is a no-op on every non-colliding input (so no existing pin moves; id-multiset-drift still catches the true duplicate-id case, which has equal counts on both sides). If a multiset diff is out of scope for this PR, then at minimum say in telemetry.canonical_granule_id's "accepted cost" paragraph and in the migration note that a collision degrades a contraction to a rewrite, so the cost is recorded on the side where it actually lands, and add a pin for the behaviour you intend.
There was a problem hiding this comment.
🤖 from Claude
Partly folded in 46936c56 — the honest parts landed; the predicate change is declined and stands for espg.
Folded:
zagg.dedup._warn_on_collapsed_recorded_idsnow runs before the diff and emits alogger.warningwhenever canonicalization collapses distinct recorded ids (grouped by canonical id, so it fires only on genuine collisions, never on a recorded duplicate). It names the colliding canonical id(s) and says the guard cannot resolve a drop within a collapsed group on that leaf. Silent was the complaint; loud is what is available without a ruling.- Docs re-priced, in all three places that treated the collapse as an identity collision only:
canonical_granule_id's "accepted cost" paragraph,docs/hive_layout.mdmigration item 5, and the catalog-identity clause indocs/design/sparse_coverage.md. Each now also states the direction: a dropped member of a collided pair reads asid-multiset-driftand rewrites where the pre-epoch href-space diff refused and named the dropped href. - New pin,
tests/test_dedup.py::TestClassifyLeafIdentity::test_collapsed_recorded_ids_degrade_the_guard_loudly, on your exactv006/v007input: asserts therewrite/id-multiset-driftverdict and that the warning fires namingATL06_X.h5. The state is now pinned rather than incidental.
Declined — the Counter-based multiset diff. It is not a no-op on every existing input: tests/test_dedup.py::TestClassifyLeafIdentity::test_duplicate_drift_with_equal_sets_rewrites feeds recorded = IDS + [IDS[0]] against planned = IDS, whose counts are not equal on both sides (2 vs 1 for g1.h5), so Counter(recorded) - Counter(planned) is non-empty and that pinned, espg-ruled rewrite becomes a refusal. And after collapse the two cases are byte-identical data — a recorded duplicate and two distinct granules sharing a basename are indistinguishable — so no diff can separate them. Choosing which way that ambiguity resolves is a ruling on the ruled predicate recorded − planned ≠ ∅, not a fold.
So it stands for espg as one question: should a collapsed-id ambiguity resolve toward refuse (data-loss-safe, at the cost of a false refusal on genuine duplicate drift, i.e. re-ruling the pin above), or stay as shipped (rewrite, now warned loudly per leaf and documented in both directions)?
| mappings; the record's granule identity is the **primary**, exactly as | ||
| before, so a caller passing resolved strings (the local backend) and one | ||
| passing the worker payload verbatim (the Lambda handler) still agree. Each | ||
| sibling asset href normalizes by this same rule wherever it is taken, so a | ||
| paired entry is href-form-independent in every component, not just its | ||
| primary. |
There was a problem hiding this comment.
🤖 from Claude (review)
(3) The "every component" claim is not what the code does, and the test named after it cannot fail.
"Each sibling asset href normalizes by this same rule wherever it is taken, so a paired entry is href-form-independent in every component, not just its primary."
canonical_granule_id reduces a paired entry to entry.get("url") and drops assets entirely — the sibling hrefs never enter the recorded identity, so a paired entry is href-form-independent in its assets not because they normalize but because they are absent. That is pre-existing behaviour from issue #425 and probably fine, but the sentence asserts a property of a code path that does not exist, and a reader taking it at face value would conclude the recorded catalog identity covers the L2B sibling a paired GEDI read actually consumed. It does not: swap the sibling asset for a genuinely different granule and granules_sha256 is unchanged.
Which is also why tests/test_telemetry.py::TestCanonicalGranuleIdentity::test_paired_asset_entries_normalize_in_every_component is vacuous — both sides use the same sibling basename (SIB_A.h5), so the assertion passes identically whether assets are normalized, ignored, or replaced. It pins the primary-URL rule (which is worth pinning) and nothing about "every component."
Suggested fix: reword the paragraph to say the sibling assets sit outside the recorded identity (the primary is the identity, unchanged by this epoch), and rename the test to what it checks — something like test_paired_asset_entries_identify_by_their_primary. If sibling-asset identity should be in the catalog hash, that is a separate ruling and belongs as its own question rather than as an implied property here.
There was a problem hiding this comment.
🤖 from Claude
Folded in 34f1d40b.
The docstring no longer asserts a normalization that does not happen. It now says a paired entry's identity is its primary alone — issue #425's invariant, which is what keeps the local backend (resolved strings) and the Lambda handler (payload verbatim) on one hash — that assets are dropped so a sibling href never reaches a digest at all, and that whether the recorded catalog identity should cover the sibling a paired read consumes is a separate ruling, not implied here.
The test is renamed to test_paired_asset_entries_identify_by_their_primary and made discriminating: the two sides now carry entirely different sibling granules (s3://b/prefix/SIB_A.h5 vs https://h/other/GEDI02_B_WHOLLY_OTHER.h5, under different asset keys) and the digests must still agree. That is the property, and it fails if assets ever start reaching the digest — where the old same-basename form passed whether assets were normalized, ignored, or replaced.
| #: open the source" decision across both sides of the D19 line. | ||
| #: * *The failure asymmetry runs the safe way.* Each fails LOUDLY in its own | ||
| #: direction — a too-small pool is slower, a wrong region is a connection | ||
| #: error, an over-large buffer is an OOM — so nothing depended on the digest | ||
| #: to catch them, while hashing them silently splits one product in two. | ||
| #: The live demonstration is dated: two GEDI flux builds of the same shard on | ||
| #: 2026-08-17 produced identical ``total_obs`` and ``cells_with_data`` in the | ||
| #: exact single-block spill regime and still hashed apart, purely on | ||
| #: worker/streaming machinery. | ||
| #: * *A machinery migration must never rehash unchanged data* — retuning a pool |
There was a problem hiding this comment.
🤖 from Claude (review)
(4) The dated measurement cannot demonstrate two of the three knobs it is cited for, and the three copies of it disagree on scope.
write_buffer and source_region are both raster-only: write_buffer is read by zagg.processing.raster._write_buffer (raster.py:632, guarded in config.py:1006) and reaches nothing on the point/temporal path; source_region appears exactly once, in the raster branch's source-store src_kwargs (runner.py:911-914, right after emit_raster_template). A GEDI flux build is the point path, so it cannot have hashed apart on either of them — the demonstration covers the fan-out widths (read_workers and the two *_workers spellings phase 1 excluded) and no more. As written the anecdote reads as evidence for all three.
The same paragraph is pasted in three places with three different scopes, which makes the overreach easy to spot: here it is "purely on worker/streaming machinery"; docs/hive_layout.md item 4 says "purely on worker/streaming/index machinery" (index machinery is not one of the three keys at all); docs/design/sparse_coverage.md clause (d) says "on worker/streaming machinery alone."
The arguments for excluding the two raster keys stand on their own without the anecdote — source_region sharing a dict literal with the already-excluded anonymous is the strongest single line in this section, and it needs no measurement. Suggested fix: attach the dated measurement to read_workers where it applies, drop the "index machinery" variant, and make the three copies say the same thing. (Separately, for the record and not a change request: the claim that write_buffer is "a peak-memory knob over an identical written result" is true of the array bytes, but the streamed content-hash recipe can be invalidated by a parking overflow bounded by STREAM_PENDING_MAX_BYTES under in-flight slabs — content_hash.py:164-172 — so write_buffer can change whether content_hashes is recorded at all. That is O11 telemetry, not leaf identity, so it does not touch the ruling; it just means "identical written result" is doing narrower work than it sounds.)
There was a problem hiding this comment.
🤖 from Claude
Folded in 344f53ad.
All four copies of the anecdote are now scoped to what it can actually demonstrate — the fan-out widths (read_workers plus the two *_workers spellings) — and each says so in the same words, with the reason the other two are out of its reach stated inline: write_buffer is read only by zagg.processing.raster._write_buffer and source_region only in the raster branch's src_kwargs, while a GEDI flux build is the point path. Their exclusion now rests explicitly on the arguments (mechanism, loud failure, and source_region sharing a dict literal with the already-excluded anonymous), which stand without a measurement.
Sites touched: the DATA_SOURCE_PACKAGING_KEYS docstring in src/zagg/semantics.py, docs/hive_layout.md migration item 4 (the "index machinery" variant is gone — you are right that index machinery is not one of the three keys), docs/design/sparse_coverage.md clause (d), and the comment on tests/test_semantics.py::test_a_machinery_migration_never_rehashes, which now says why it pins all three keys even though the measurement covers one.
Your STREAM_PENDING_MAX_BYTES note is deliberately not in the diff, as you framed it: a parking overflow can change whether content_hashes is recorded, which is O11 telemetry rather than leaf identity, so it touches no ruling here. Acknowledged on the record — "identical written result" is doing narrower work than it sounds — and left for a separate thread if it should be written down.
| ruling excludes. | ||
| """ | ||
| if isinstance(entry, dict): | ||
| entry = entry.get("url") |
There was a problem hiding this comment.
🤖 from Claude (review)
(5) Nit, but a loud failure became a silent one: a paired entry missing url now mints the literal id "None".
canonical_granule_id({"assets": {}}) # -> 'None'
canonical_granule_id(None) # -> 'None'Pre-epoch, build_record's g["url"] if isinstance(g, dict) else g raised KeyError on a url-less mapping — a malformed worker payload could not become a recorded identity. .get("url") swallows it, and because every such entry canonicalizes to the same string, N malformed entries collapse to one recorded id, which shrinks the recorded set and can hide a contraction the same way finding (2) describes. Nothing in the current dispatchers produces such an entry (_resolve_granule_entries always sets url), so this is a robustness nit rather than a live bug — but the whole point of the D19/D20 identity pair is that an identity is never silently wrong.
Suggested fix: keep the pre-epoch loudness — entry = entry["url"] in the dict branch, and reject None/"" rather than stringifying it. Related, same function: granules_sha256's annotation is still Iterable[str] | None, but the documented contract now accepts paired-asset mappings and any href form (see the shard_status warning this diff rewrote), so Iterable[Any] | None would match canonical_granule_ids and the callers.
There was a problem hiding this comment.
🤖 from Claude
Folded in 88d234f2.
canonical_granule_id no longer mints "None". A mapping with no usable url primary raises ValueError naming the offending entry, and a None/"" id raises too — restoring the pre-epoch loudness of build_record's g["url"] without reintroducing the KeyError. The docstring records why it must be loud rather than lenient: every such entry canonicalizes to the same string, so N of them collapse onto one recorded id and shrink the recorded set — the same shape as finding (2)'s collision. Confirmed your read that nothing live produces one: zagg.runner._resolve_granule_entries skips a record without a href (if not url: continue) and always sets a non-empty url.
Annotation widened: granules_sha256(granule_ids: Iterable[Any] | None), matching canonical_granule_ids and the documented contract, with the reason noted in its docstring. build_record's granule_ids parameter had the same stale Iterable[str] and takes paired mappings through the same path, so it moved with it.
Pin: tests/test_telemetry.py::TestCanonicalGranuleIdentity::test_a_malformed_entry_raises_rather_than_minting_an_identity, parametrized over {"assets": {}}, {"url": None}, {"url": ""}, None and "", asserting the raise through canonical_granule_id, canonical_granule_ids and granules_sha256 — so it cannot be swallowed on the way to a sidecar. Full suite green apart from the two known pre-existing failures; no digest moved.
| CANONICAL granule-id space — the driver-stripped bare id | ||
| (:func:`canonical_granule_id`, espg-ruled at the D19 hash epoch) — so the | ||
| identity a leaf carries names the granules it read and not the driver that | ||
| fetched them. It is deliberately not a record key — identity equality is the hash compare every fan-out reader |
There was a problem hiding this comment.
🤖 from Claude (review)
(6) Nit — reflow miss. This line is 112 chars and is the only over-100 line this diff adds (ruff does not catch it: E501 is in the ignore list per pyproject.toml, so nothing but review will). The rest of the module docstring wraps at ~79, and the sentence it interrupts ("It is deliberately not a record key — ...") reads as if the insertion above was spliced in without rewrapping. Reflow the paragraph.
There was a problem hiding this comment.
🤖 from Claude
Folded in d0e9fbdb. The paragraph is rewrapped so the "It is deliberately not a record key — ..." sentence flows at the module docstring's ~79-column width instead of running to 112. Checked the rest of the diff for the same miss with awk 'length > 100' over the touched modules — this was the only one, and it stays clean after the later folds.
|
🤖 from Claude Status: all five standing questions are ruled and landed. Phases 7 and 8 are in, the review findings are folded, and the PR is out of draft and espg ruled all five in-session on 2026-08-17. What landed for each: (1) Canonical granule identity → (b), the driver-stripped bare granule id (phase 8, (2) (3) The sharding declaration → (a), left as landed (phase 7). No code change; the constant's text reads as espg-ruled rather than open, with both limits ( (4) (5) The draft→ready flip → resolved. With the rulings landed, the findings folded and CI green, CLAUDE.md §2 applies without conflict: flipped ready-for-review (reversible), The fold, and the one thing it left open. Phases 7 and 8 drew 6 review findings; 5 are folded and 1 is split. The folded five, briefly: Question (6), new and standing for espg (now in the body): basename collapse can make a genuine contraction invisible, because Fixtures — a deliberate deviation worth flagging. The epoch's fixture obligation was applied field-surgically this time (only Tests. Full suite on the final tree: Both golden pins moved deliberately: Pre-existing failures, flagged not fixed: No digest moved in the fold: CI. Fully green on the phase-8 head |
|
🤖 from Claude Question (6) ruled: (a), leave as shipped (espg, in-session 2026-08-17). Rationale on record: the collapse precondition (two granules in one shard sharing a basename across prefixes) is impossible by construction in every catalog zagg reads — Optional hardening noted for a future small-fix train (not this PR): enforce the invariant where it is owned — refuse at shardmap build when two granules in one shard share a basename. A build-time, human-present error provably keeps the ambiguous state from ever reaching the leaf gate, making the leaf-side variants permanently unnecessary. With this, questions (1)–(6) are all ruled and landed or closed; nothing further stands on this PR. |
2 participants
Closes #415. Refs #384 (the release train and the recorded ruling — plan delta 2), PR #397 (questions (3), (7), (8) and their review threads).
What this does
The D19 hash epoch: every ruled identity fix landed in one release so the compatibility break happens exactly once. Two came from PR #397's ruled questions (7) and (8); three more were ruled by espg in-session on 2026-08-17 —
credentials_provider(issue #449, phase 5), the byte-movement knobsread_workers/write_buffer/source_region(phase 7), and canonical granule identity (phase 8) — together with two rulings that changed nothing but the record: keep the sharding declaration as landed, and dropemit_cell_idsback out of the core (phase 7).Four of them change what
zagg.semantics.semantic_hashdigests, so every pre-epoch D19 hash is invalidated by design — the point of the release, not a defect. The fifth changes the other half of the identity pair, the sidecars'granules_sha256, so pre-epoch catalog identities taken over resolved hrefs are invalidated too. Nothing in any store changes on disk; only the derivation of the digests that label it.(7)(b) — the granule fan-out width is packaging. D19's ratified exclusion list already named "worker size" as packaging, but the keys were never in
DATA_SOURCE_PACKAGING_KEYS, which made the leaf seams'semantic_hash=Nonefallback clamp-sensitive: both dispatchers hand each cell adata_sourceclamped tomin(K, n_granules)(runner._clamped_data_source, issue #184), so a small shard's worker-side hash differed from the run-level hash, andsemantic_hashbeing an_EQ_OR_NONE_KEYSmember then collapsed the identity half tonullin any rollup mixing clamped and unclamped shards.(8)(c) —
semantic_coreis widened to the leaf-shapingoutputknobs. Until now the wholeoutputblock was outside the core, so any knob that changes what a leaf contains moved neither half of the skip gate's identity pair and the gate readequal.output.aoi_maskoutput.windowingepochspellings and the issue #355 point-window sugar canonicalizeoutput.grid.shardedoutput.grid.emit_cell_idscell_idsarray into every leaf, so it met the criterion — and espg ruled it back out, 2026-08-17 (question (4)(b)): the hatch is scheduled for removal (issue #304), after which a store built with it ON would carry a digest no legal config reproduces. A leaf's array inventory is verified by reading the leaf, the same closing argument asoutput.pyramidbelow.output.pyramidhive.leaf_column_expectation, spec §4.6) — that is what closed question (8)'s concrete regression; D11 keeps the block out of the frozen manifest keys; anddeclare_pyramid's retrofit exists precisely to add the declaration to the config that built a store, which its own semantic guard would then refuse. Hashing it breaks a supported workflow to re-cover covered ground — caught bytest_pyramid_only_edit_hashes_identicallygoing red on an earlier draft of this branch.parent_order/child_order/chunk_inner)store/store_layout/product_name,coverage_moc,sweep,consolidate_metadataEvery included knob is resolved through its accessor, never as spelled, so an explicit default hashes identically to an absent key (the
pipeline.typediscipline, §8.3) —sharded: trueon a HEALPix config is a no-op, as iswindowing: {schedule: none}.credentials_provideris packaging — espg-ruled in-session 2026-08-17 (issue #449).data_source.credentials_providerjoinsDATA_SOURCE_PACKAGING_KEYSalongsidereader/driver/read_plan: it selects how source bytes are fetched — which registry name mints the DAAC credentials the dispatcher attaches to every event (issue #213 Phase 4) — and never what is computed from them. Three things make it the same D19 class rather than a judgment call:lpdaaccredentials,gesdisccredentials, or an anonymous open produce byte-identical leaves.anonymous— the other spelling of the same choice — was already excluded, so hashing the provider was hashing half of one decision.semantic-mismatchand rewrite the whole store to produce the same bytes.Ruled now, at the epoch, deliberately: no store carries a provider-bearing hash yet, so the first GEDI store's identity is born without an auth knob in it. The
credentials_provider: lpdaacline lands with the GEDI template on PR #450; ruling after that lands would mean paying a second epoch for a knob that never should have been in the core.The byte-movement knobs are packaging too — espg-ruled in-session 2026-08-17 (question (2)(c)).
read_workers,write_bufferandsource_regionjoinDATA_SOURCE_PACKAGING_KEYSin phase 7, oncredentials_provider's three-part test:read_workers(issue Compiled (hidefix) read path is never exercised by default, and its fetch/decode is serial #170) is the third fan-out width beside the two spellings phase 1 excluded (processing/worker.pysizes the budget asgranule_workers × read_workers × fetch width);write_bufferbounds how many slabs are alive under the streamed raster sink (processing/raster._write_buffer) — peak memory over an identical written result;source_regionis the raster source store's AWS region kwarg, sitting in the same dict literal as the already-excludedanonymous(runner.py'ssrc_kwargs), so the D19 line ran through the middle of one "how do we open the source" decision.semantic-mismatchand rewrite it to produce the same bytes.That last point is not hypothetical. espg ran the same GEDI SERC flux build twice on the deployed 0.46.0 fleet on 2026-08-17 with identical outputs, and got two different identities:
semantic_hash490044e3e5e380de…shard_workers 1,buffer_granules 4, 8192 MB5d081b68f0487b4d…shard_workers 4,buffer_granules 8, 4096 MB, hierarchical index pinSame shard (
5347294481781620745), identicaltotal_obs(23,353,274), identicalcells_with_data(27,727), both in the exact single-block spill regime (byte-identical fold by design) — and the hashes split purely on worker/streaming/index machinery. Both run-stats parquets are unders3://sliderule-public/zagg-demo/serc_gedi_flux.zarr/(stats_20260817T084340Z_8344cde5…and the laterstats_*_5d081b68…). That is a live, dated instance of exactly the false product split these exclusions close.Canonical granule identity — espg-ruled in-session 2026-08-17 (question (1)(b)). This is the epoch's other digest:
granules_sha256, the catalog identity half recorded in every D20 sidecar, plus the id list in itsgranules.jsonsibling. espg's words: "we want the granule to trigger the hash, not how that granule is fetched."One physical granule reaches the recording seams under three names — a resolved
s3://bucket/key/FILEhref, anhttps://host/path/FILEhref (runner._resolve_urlspicks one bydata_source.driver), or the bare catalog id, which for every CMR/STAC catalog zagg reads is the basename of both hrefs (verified againstcatalog_cycle22_*.parquet:id == "ATL06_20240108040423_03102212_007_01.h5", and both asset hrefs end in exactly that).driverhas been packaging in the semantic core since D19 was written, so pre-epoch the two digests disagreed about the same run: the semantic half said "same product", the catalog half said "different inputs".The canonical form is therefore the basename — the scheme, host, bucket and key prefix are all where the bytes live, and the s3 and https spellings of one granule agree on nothing else. Landed at three seams so no path can record a non-canonical id:
telemetry.canonical_granule_id/canonical_granule_ids— the rule, with paired-asset{url, assets}entries (issue GEDI waveforms 2/3: generic vlen reader primitives, paired-asset shardmap, flux transform + gedi01b template #425) identifying by their primary exactly as before, so the local backend (resolved strings) and the Lambda handler (payload entries verbatim) still agree on one hash;telemetry.granules_sha256andtelemetry.write_granule_ids— the digest and the recorded list, so themissingids a contraction names are driver-independent too;dedup.classify_leaf_identity— both sides of the diff, which is what lets a pre-epoch leaf (full hrefs on its sibling) compare cleanly against a post-epoch plan instead of reading as a full contraction.Accepted cost, recorded at the function and in the migration note: two granules whose hrefs differ only in prefix collapse to one identity. Every catalog zagg reads names granules globally uniquely — which is why the catalog's own id equals the basename — and the alternative, keeping any part of the fetch path, is precisely what the ruling excludes. The raster id space (
raster_granule_ids: STAC item ids or ISO datetimes) carries no path separator, so it is canonical already and no raster digest moves.What this closes for operators:
docs/hive_layout.md's skip-if-current section carried a caveat that "the recorded id space is driver-dependent … flipping the driver between runs reads as a full mixed contraction and refuses per leaf —--allow-contractionis the escape hatch." That caveat is now deleted rather than documented, because the behavior is fixed.Phases
phase 1 —
granule_workers(and the canonicalshard_workers) intoDATA_SOURCE_PACKAGING_KEYS+ testsphase 1 adversarial-review fold — 3 findings, 2 folded + 1 split: the constant's summary line now names worker sizing as its own D19 category (
14b670d); a seam-level pin replaced the module-level-only coverage (b882d94,test_a_clamped_per_cell_config_still_reads_current); theshard_workersscope objection is declined and stands as question (2), with its unruled status now stated at the constant (2d6921b)phase 2 —
semantic_corewidened to the leaf-shapingoutputknobs + testsphase 2 adversarial-review fold — 5 findings, 4 folded + 1 split: the K == 1 /
chunk_innerlimits of hashing the sharding declaration are now stated at the constant (a8cdb00) while the three-way design fork is declined and stands as question (3);emit_cell_idsmarked as inferred-not-ruled (6be4b1a, question (4)); the windowing totality guard split by fault class so a fault inside the normalizer propagates instead of laundering into a digest (3747cb9); a gate-level pin for theshardedflip (2165be9); thewindowingrationale corrected to name the layer it actually buys (fbcccaf)phase 3 — conformance fixtures regenerated through
tools/generate_spec_fixtures.py;docs/specification.md§6 cross-referencephase 3 adversarial-review fold — 2 findings, both folded:
TestFixtureSemanticHashmakes the fixture obligation self-enforcing instead of a habit (fc608e8);docs/hive_layout.md's pre-epochoutput.*claim at thedeclare_pyramidguard corrected (7a3a705)phase 4 — the migration note in
docs/hive_layout.md+ the D19 design-record amendment indocs/design/sparse_coverage.mdphase 4 adversarial-review fold — 3 findings, all folded: the in-place restamp path gated with its three real hazards instead of a one-liner (
e0711d8); the fleet consequence, the async-manifest question, and what the epoch unblocks added (93bb630); the design-record amendment marked at the point of contradiction (bc910da)merge
origin/main(e0ae75b) — the branch predated weeks of merges. Conflicts were narrow:tests/test_semantics.py(both sides appended a new class at EOF — kept both,TestLeafShapingOutputKnobsthen main's issue GEDI waveforms 1/3: §2 counts/flux weights declaration + δ=8,192 raise #424TestWeightsAndOverviewDeltaHashing) and seventests/data/spec/fixtures where only the regeneration timestamps collided (generated_at/written_at/ the stats blob'stimestamp+zagg_version) — main's side taken, since main regenerated later. Nosemantic_hashline conflicted: main's issue GEDI waveforms 1/3: §2 counts/flux weights declaration + δ=8,192 raise #424 normalization (VARIABLE_PACKAGING_KEYS,weights: counts) touches per-variable aggregation keys no fixture config declares, so the epoch's digests merged through untouched andTestFixtureSemanticHashis green without regenerating anything.src/zagg/semantics.pyauto-merged (main's_normalize_variables+ this branch's widened core, disjoint).phase 5 —
credentials_providerintoDATA_SOURCE_PACKAGING_KEYS(espg-ruled 2026-08-17, issue GEDI template ships without an LPDAAC credentials provider — reads 403 as NSIDC #449) + tests; the migration note and the D19 design-record amendment extended with the third exclusionsecond merge of
origin/main(7f7452c) —mainmoved again mid-run (issues Raster time axis encodes as toc words (instant|range) — S2 first #443/Unindexed builds: cover from WKB and intersect before materializing records (#439 for the live path) #445 landed). Conflicts: thesemantics.pymodule docstring (both sides appended a bullet after the pipeline-type one — kept both, main'soutput.time_encoding§8 entry now follows the epoch's leaf-shaping entry) andtests/test_spec_conformance.pyat EOF (both appended a class — kept both). No behavioral overlap: main'stime_encodingenters the core as a top-levelcore["time_encoding"]key, not throughcore["output"], soOUTPUT_LEAF_SHAPING_KEYSandtest_the_documented_constants_match_the_coreare untouched.phase 5 adversarial-review fold — 2 findings, 1 folded + 1 split:
anonymousandcredentials_providerare the same class, not one knob under two names (disjoint consumers —anonymousonly in the raster source-store kwargs, the provider only on the point/temporal paths), reworded to the design record's accurate form in all three places (e8377b3c); the proposal to excluderead_workers/source_region/write_bufferin the same breath is declined on agent authority — excluding a key is a ruling, not a fold — and rides question (2), with the three now named as still-hashed-and-unruled at the constant and in the migration note (0594d400)phase 7 — the three semantics rulings (
2cf022e7):read_workers/write_buffer/source_regionintoDATA_SOURCE_PACKAGING_KEYSwith per-key equality pins and per-key machinery-migration pins (question (2)(c));emit_cell_idsback OUT of the core with its by-name exclusion and dedicated rationale test restored (question (4)(b)); the sharding declaration left exactly as landed with its two limits restated as espg-ruled rather than open (question (3)(a)).test_golden_hash_pinre-pinned tofb15224f7265…and the seven fixture manifests'semantic_hashupdated.phase 8 — canonical granule identity (
623bded0, question (1)(b)):telemetry.canonical_granule_id/canonical_granule_ids, threaded throughgranules_sha256,write_granule_ids,build_record,dedup.classify_leaf_identity(both sides) and the twohive.pycall sites; the fourgranules.jsonfixtures updated; the migration note's item 5 and the design record's catalog-identity clause amended; the operator-facing driver-dependence caveat indocs/hive_layout.mddeleted rather than documented, because the behavior is fixed.phases 7–8 adversarial-review fold — 6 findings, 5 folded + 1 split. Folded:
docs/hive_layout.md's "the driver-dependence caveat is gone" paragraph overclaimed and is now split by epoch side (31af5392) — a pre-epoch leaf's storedgranules_sha256is href-space, sohashes_matchis False by construction and theequalarm is unreachable; what canonicalizing the recorded side actually buys there is the direction (id-multiset-drift/expansioninstead of a spuriouscontractionrefusal), pinned bytest_a_pre_epoch_leaf_rewrites_rather_than_refusing. Thegranules.jsonsiblings were unguarded — nothing in the suite read them — closed withTestFixtureGranuleIdentityplustest_every_leaf_fixture_carries_the_sibling, enumerated so a new fixture cannot ship one unguarded (ab7cd89d); it also found there are six committed siblings, not four.canonical_granule_id's claim that sibling asset hrefs are "href-form-independent in every component" was unsupported — a paired entry's identity is its primary alone — reworded, and the vacuous test replaced with a discriminating one over two entirely different sibling granules (34f1d40b). The dated GEDI measurement was cited for two raster-only knobs a point-path build cannot exercise: scoped to the fan-out widths at all four sites, with the three divergent paste variants reconciled (344f53ad). Aurl-less mapping used to mint the literal id"None"and collapse N such entries into one — now a pointedValueError, withgranules_sha256/build_record's staleIterable[str]annotations widened (88d234f2). One 112-char docstring line reflowed (d0e9fbdb;E501is ignored, so ruff would not have caught it).Split, and standing as question (6): basename collapse can make a genuine contraction invisible, because the diff is over SETS. The loud half is folded (
46936c56) — a warning that names the colliding canonical id and says the guard's per-granule resolution is degraded on that leaf, plus the doc/docstring re-pricing andtest_collapsed_recorded_ids_degrade_the_guard_loudly. TheCounter-based predicate change itself is declined on agent authority: it would fliptest_duplicate_drift_with_equal_sets_rewrites, an existing pin of the ruled predicaterecorded ∖ planned ≠ ∅, fromrewritetorefuse. After collapse the two inputs are byte-identical data, so no diff can separate them — which way that ambiguity resolves is a ruling.phase 6 — the epoch's fixture obligation applied to the two fixtures that landed on
mainafter phase 3.flux/(issue GEDI waveforms 1/3: §2 counts/flux weights declaration + δ=8,192 raise #424) andraster_toc/(issue Raster time axis encodes as toc words (instant|range) — S2 first #443) were generated onmainand merged in carrying pre-epochsemantic_hashvalues that no post-epoch zagg reproduces — the exact failureTestFixtureSemanticHashexists to catch, which it missed only because its parametrize list was a hardcoded three. Regenerated both throughtools/generate_spec_fixtures.py --only flux raster_toc(diff issemantic_hash+ timestamps + onezagg_version, nocontent_hashesmovement), pinned both, and closed the hole withtest_every_fixture_is_covered: it walkstests/data/spec/**/morton_hive.jsonand fails if any fixture carrying asemantic_hashis not in the covered set, so the next fixture cannot ship unpinned.raster_toc/'s config literal was factored out of its builder into_raster_toc_config()so the test can rebuild it.The fixtures
tests/data/spec/regenerated through the production path. The whole diff is 26 lines —semantic_hashandgenerated_at, nothing else. No compressed-byte churn and nocontent_hashesmovement, which is the evidence that the epoch is contract-invisible at the byte level: the §5 hashes are over decoded values, not over identity.docs/specification.mdis otherwise untouched — the derivation ofsemantic_hashhas never been spec-normative, and the operator-facing statement lands indocs/hive_layout.mdwhere §4.7's informative scope-out already places this family.Update after the
mainmerges (phase 6): two fixtures generated onmainafter phase 3 —flux/(issue #424) andraster_toc/(issue #443) — merged in carrying pre-epoch digests. Both regenerated through the same production path; the diff is again onlysemantic_hash, the timestamps, and onezagg_versionstring, withcontent_hashesuntouched.TestFixtureSemanticHashnow covers all six fixtures and carriestest_every_fixture_is_covered, which enumeratestests/data/spec/**/morton_hive.jsonand fails on any fixture whose digest is not pinned — the fold's obligation is now closed under new fixtures, not just under new semantic-core edits.Update after the 2026-08-17 rulings (phases 7 and 8). All seven
morton_hive.jsondigests moved (question (4)'s removal) and fourgranules.jsonsiblings moved (question (1)'s canonicalization). Both sets of values came from the production path —tools/generate_spec_fixtures.py --out <scratch>— but were applied field-surgically rather than by committing a wholesale regeneration, and that deviation is deliberate: a full regeneration breaksTestLocatedDeclaration::test_absent_declaration_on_the_pre_section_9_fixture, becausetests/data/spec/kitchen_sink/is frozen on purpose as the pre-§9 fixture and today's writers add alocateddeclaration to two of its arrays (issue #410) and anoverview_deltato its pyramid attrs (issue #424). Those two fields are stale onmainin every fixture generated before that work merged — pre-existing, separable from this epoch, flagged not fixed per CLAUDE.md §4. Nocontent_hashesmoved in either step.New in the fold:
tests/test_spec_conformance.py::TestFixtureSemanticHashrecomputes each fixture's digest from the config that built it. Before it, nothing in the suite read that field — the whole suite passed on this branch with the stale pre-epoch hashes still committed, and moczarr would have vendored them (espg/moczarr#19/#20) with every parity gate green. Itspyramid/case pins the exclusion decision as well as the digest: that fixture's manifest is built from the/1config and retrofitted bydeclare_pyramidunder the/2one, sosemantic_hash(cfg_v1) == semantic_hash(cfg_v2)is asserted on a committed conformance artifact.How it was tested
After phases 7 and 8 + the fold (head
88d234f2):uv run pytest -q→ 2 failed, 4203 passed, 38 skipped in 253 s — both failures the known pre-existing pair listed below, unchanged.tests/test_semantics.py tests/test_telemetry.py tests/test_dedup.py tests/test_hive.py tests/test_spec_conformance.py→ 441 passed before the fold; see the fold entry for the post-fold count.test_byte_movement_knobs_are_packagingandtest_a_machinery_migration_never_rehashes, both parametrized per key, so each of the three carries its own equality assertion (key in the tuple, absent fromsemantic_core, digest equal to the bare config) and its own migration assertion (a retune over unchanged data never rehashes) — the treatmentcredentials_providergot in phase 5.test_emit_cell_ids_is_packaging, carrying the issue Morton-only writer flip: stop writing cell_ids (implements #262 / D16) #304 rationale in the test itself, restoring the dedicated coverage this PR had replaced.tests/test_telemetry.py::TestCanonicalGranuleIdentity— real ICESat-2 href trios (s3:///https:/// bare id / the s3 driver's strippedbucket/keyform) hashing identically and recording identical ids; thegranules.jsonsibling round trip byte-identical across driver forms; paired-asset entries; the raster id space provably untouched; and the trailing-slash edge. Plus two gate-level pins where the cost would land:tests/test_dedup.py::TestClassifyLeafIdentity::test_a_driver_switch_still_reads_current(parametrized over four href forms, asserting the hash fast path decides and the sibling is never read) andtests/test_hive.py::TestLeafSkipIfCurrent::test_a_driver_switch_still_reads_current— a leaf written from s3 hrefs, rerun with the https hrefs of the same granules, must readequalwith the fold armed to raise if it runs.test_golden_hash_pin→fb15224f7265…(question (4)'s removal in one line), the seven fixturesemantic_hashvalues, and — in the canonical id space — the recorded-id andmissing-id assertions intests/test_dedup.py,tests/test_telemetry.pyandtests/test_hive.py. Every one of those is the ruling's behavior, not a weakened assertion: each now asserts the bare granule id where it previously asserted a resolved href.docs/specification.mdstays untouched, verified rather than assumed: neithergranules_sha256norgranules.jsonappears anywhere in it, so question (1) moves no spec-normative text. The operator-facing statement lands indocs/hive_layout.md, where §4.7's informative scope-out already places this family.623bded0:ruff,build,build-x86_64,build-arm64,test (3.12)green. CI on the fold head88d234f2fired 8 check-runs (no PR-event stall);ruffandbuildgreen, the test and arch-build jobs settling.After the second
origin/mainmerge + phase 6 (fb93108):uv run pytest -q→ 2 failed, 4049 passed, 38 skipped in 245 s — the same two failures as below, unchanged.tests/test_spec_conformance.py tests/test_semantics.py tests/test_raster_pipeline.py tests/test_time_axis.py→ 273 passed (the merged §8 time-axis surface and the epoch's fixture pins together).ruff check/ruff format --checkclean ontools/generate_spec_fixtures.pyandtests/test_spec_conformance.py.After the first
origin/mainmerge + phase 5 (4297585):uv run pytest -q→ 2 failed, 3980 passed, 38 skipped in 285 s. Neither failure is this diff's:tests/test_lambda_build.py::TestFunctionBuild::test_function_build_succeeds— the known environmental docker/build failure on this machine.tests/test_client_transport.py::TestStatusPoller::test_invoke_fault_burns_an_attempt_and_retries— a wall-clock-sensitive poller test (assert calls["n"] == 2after a 5 s spin). Verified red on a cleanorigin/maincheckout at this session's clock, so it is pre-existing; flagged, not fixed, per CLAUDE.md §4.test_sweep_stage.py::TestStagePass::test_ratchet_rewrites_on_child_changefailure that madetest (3.13)red onbc910dais gone — it was fixed onmainby PR small fixes: staged sweep skip key carries the run ids; bench object model gains the column term #421, which this merge brings in.tests/test_semantics.py tests/test_spec_conformance.py tests/test_hive.py tests/test_config.py→ 557 passed.test_golden_hash_pinis unchanged by phase 5 (96e5b120…stands) — the pinned config declares nocredentials_provider, and neither does any conformance fixture, so no fixture regeneration was needed and no committed digest moves. What does move is the digest of a config that spells the key: it now equals the digest of the same config without it, which is the ruling.tests/test_semantics.py::TestCanonicalization::test_credentials_provider_is_packaging(the equality assertion — key in the tuple, absent fromsemantic_core, hash equal to the bare config) and::test_a_credential_migration_never_rehashes(add / drop / swaplpdaac↔gesdisc, and composition withanonymous), plustests/test_hive.py::TestLeafSkipIfCurrent::test_a_credential_migration_still_reads_current— the gate-level mirror of phase 1'stest_a_clamped_per_cell_config_still_reads_current: a leaf written without a provider, rerun under a config that names one, must still readequalwith the fold armed to raise if it runs.tests/test_semantics.py::test_credentials_provider_is_not_in_the_packaging_list— a deliberate pin of the pre-ruling behavior whose own comment says "if it is ruled packaging, this test flips to the equality assertion." It is not onmainyet, so this PR writes the equality test fresh; small fixes 2026-08-17: NaN-safe dispatch payload, LP DAAC credentials, GEDI filter set #450's pin will conflict at its nextmainsync and should adopt this flip (itscredentials_provider: lpdaacline on the GEDI template is unaffected and correct either way).uv run ruff checkandruff format --checkon the touched files (src/zagg/semantics.py,tests/test_semantics.py,tests/test_hive.py) → clean.On
bc910da, before the merge:uv run pytest -q→ 1 failed, 3781 passed, 38 skipped in 464 s. The single failure istests/test_sweep_stage.py::TestStagePass::test_ratchet_rewrites_on_child_change, which is pre-existing and unrelated — verified red on a cleanorigin/maincheckout at this session's clock (itsmonkeypatchonly moves the stamp when the UTC hour string starts with0, so it fails at any hour ≥ 10 UTC). Flagged, not fixed, per CLAUDE.md §4.bc910da:ruffgreen,build/build-x86_64/build-arm64green,test (3.12)green,test (3.13)red on exactly that one pre-existing test —1 failed, 3781 passed, 38 skipped, the same counts as the local run,assert 0 > 0attests/test_sweep_stage.py:376. Nothing in this diff is implicated; the 3.12 job running the same tree green is the evidence.tests/test_runner.py::TestConsolidationGatewrites to a relative./out.zarr(gitignored) and does not clean up, so a leftover directory from an earlier run made it fail on the second full run of this session with the epoch's own manifest refusal. It passes on a clean tree (4 passed). Pre-existing, and arguably the epoch working as designed.tests/test_semantics.py(theTestLeafShapingOutputKnobsclass plus the two phase-1 packaging tests), 2 gate-level pins intests/test_hive.py(test_a_clamped_per_cell_config_still_reads_current,test_a_sharded_flip_defeats_the_skip), 4 intests/test_spec_conformance.py. Both golden pins re-pinned deliberately:test_golden_hash_pin→96e5b120…(the epoch in one line) and the fixture manifests.uv run ruff check src tests→ 1 error, the pre-existingN818atsrc/zagg/registry.py:64.uv run ruff format --check src tests→ 1 file would be reformatted, the pre-existingtests/data/benchmark/README.md. Both are the same pair PR leaf skip-if-current: input-identity no-op with lifecycle touch (issue #388) #397 flagged; neither is touched by this diff, and neither is in the CI lint job's scope (it is green).uv run pre-commit run --all-files: ruff-format passes;codespellandcheck-yamlclean on every file this PR touches (the repo-wide failures aretests/test_hive.py:2074'sstaticsand the CloudFormation!Ref/!GetAtttags underdeployment/aws/, both pre-existing onmain). mypy: 140 error lines, identical to theorigin/mainbaseline — the one error insrc/zagg/semantics.pyis the pre-existingDataSourceDictarg-type, moved by line number only.Questions for review — the original five are ruled and landed; one new one is open
espg ruled questions (1)–(5) in-session on 2026-08-17, and all five are landed. They are kept here as the decision trail rather than deleted, because each ruling is a compatibility fact a future reader will need. Question (6) is new, opened by the phases 7–8 fold, and is the only thing on this PR still needing a decision.
Canonical granule-id normalization → (b): the driver-stripped bare granule id. Ruled, and landed in phase 8 across both halves —
granules_sha256and the recorded id list. espg's words: "we want the granule to trigger the hash, not how that granule is fetched." One granule reaches the recording seams as ans3://bucket/key/FILEhref, anhttps://host/path/FILEhref (runner._resolve_urlspicks bydata_source.driver) or the bare catalog id — which for every catalog zagg reads is the basename of both hrefs.driverhas always been packaging in the semantic core, so hashing the href form made a fetch-mechanism edit look like a catalog change.shard_workersscope → (a) + (c). Both spellings stay excluded, andread_workers/write_buffer/source_regionjoined them in phase 7. The class test iscredentials_provider's: selects how bytes are fetched or moved, never what is computed; fails loudly rather than silently; and a machinery migration over unchanged data must not rehash. The "still-hashed-and-unruled" language phase 5's fold added for the three is gone — they are ruled.The sharding declaration → (a): left exactly as landed. No code change; the constant's text now reads as espg-ruled rather than open. Both limits stand documented and bounded:
K == 1over-discriminates (the safe direction), andchunk_innerstill moves K without moving the digest. Closing the second costs part of D24, which is why it stays open by ruling rather than by omission.emit_cell_ids→ (b): dropped back OUT of the core. The by-name exclusion and a dedicated rationale test are restored (test_emit_cell_ids_is_packaging), now carrying the issue Morton-only writer flip: stop writing cell_ids (implements #262 / D16) #304 argument the criterion alone does not price: the hatch is scheduled for removal, after which a store built with it ON would carry a digest no legal config can reproduce. A leaf's array inventory is verified by reading the leaf — the same precedent that keepsoutput.pyramidout.The draft→ready flip → resolved by the above. With every ruling landed, the findings folded and CI green, CLAUDE.md §2 applies unambiguously: the PR is flipped ready-for-review by the agent (reversible; merging is not). It carries
waiting, which now covers both the merge decision and question (6).NEW — should a collapsed granule id resolve toward refuse? Opened by the phases 7–8 self-review, and the one open item.
classify_leaf_identitydiffs the recorded and planned id sets, so if two distinct granules in one shard canonicalize to the same bare id — different key prefixes, same basename — dropping one becomes invisible: the unit readsid-multiset-driftand rewrites where pre-epoch it refused and named the dropped href. That is the direction the classifier's own docstring says must never happen ("a rewrite over a contracted input set destroys aggregated data the operator was never told about").The loud half is folded: the collapse now emits a warning naming the colliding id and saying the guard's per-granule resolution is degraded on that leaf, the docs price it as a guard consequence rather than only an identity collision, and
test_collapsed_recorded_ids_degrade_the_guard_loudlypins it. What is not taken on agent authority is the predicate: after collapse, "a recorded duplicate of one granule" and "two distinct granules, one dropped" are byte-identical data, so no diff can separate them, and aCounter-based diff would fliptest_duplicate_drift_with_equal_sets_rewrites— an existing pin of the ruled predicaterecorded ∖ planned ≠ ∅— fromrewritetorefuse. Options: (a) leave as shipped (current — loud, but a collapsed drop rewrites); (b) multiset diff, so the ambiguity resolves towardrefuse(data-loss-safe, at the cost of a false refusal on genuine duplicate drift, escapable with--allow-contraction); (c) refuse only when a collapse was actually detected, keeping the plain-duplicate path as it is.Worth pricing before choosing: the collapse needs two granules in ONE shard sharing a basename across different prefixes, which every catalog zagg reads rules out by construction — that is why
rec["id"]equals the basename. Nothing observed has ever hit it.Generated by Claude Code