coverage sidecar: the versioned temporal section (issue #480)

[espg]

Closes #480.

The versioned temporal section on the root coverage sidecar ({store_root}/coverage.moc): the per-shard toc envelope word map plus the optional root weighted time-digest, both derived from the §8.3 per-centroid toc siblings a store already carries, both riding the ONE object a reader already GETs to bootstrap discovery. Sequenced ahead of the source-coop build — a store carries this metadata only if built by a zagg that writes it.

What lands

Grammar — docs/specification.md §10 zagg-coverage-toc/1: a section under the root envelope's temporal key, with its own spec marker so a reader gates on it exactly as it gates the enclosing morton-moc/1 carrier. docs/hive_layout.md's root-envelope block points at it.

• Tier 1 — the per-shard envelope word map (shards, §10.2): one toc word per populated shard, toc_reduce over that shard's sibling words, unioned across the store's temporal-carrying fields. Keys are D1 decimal shard ids, values are toc words as decimal strings (a uint64 exceeds 2^53 — the rule the ranges endpoints already follow). A shard listed by ranges but absent from shards is unknown, never empty.
• Tier 2 — the optional root time-digest (digest, §10.3): a t-digest over acquisition times, weights = observation counts, the §8.3 per-centroid toc envelopes as its companion, δ = 64, in the store's native ragged (k, 2) + word-sibling form — base64 of the §1.4 element bytes, so the only wrapper is the one a JSON carrier forces.
• Absence of the whole section = the store has no temporal channel = never a refusal (§10's standing posture). Absence of digest alone says the same one tier down: tier 1 stands without it. An unknown spec revision reads as absent rather than as a hard failure — the strict-gate-then-degrade rule the sibling coverage envelopes already use, spelled out as a deliberate carve-out from the page's general conformance rule — and is preserved, never downgraded, when a producer at this revision meets it.
• The issue's open question is flagged, not decided (§10.2): whether a multi-field store should publish per-field maps instead of the union. Recorded as a possible future shards_by_field key; shards keeps its meaning either way.

What the value axis actually is (§10.3)

Each contributing leaf centroid enters the fold at its own toc envelope's midpoint, weighted by that centroid's observation count; a merged root centroid's mean is the weight-weighted mean of those, so it lands inside its own merged word (to the float32 quantum) rather than at the word's centre. Two consequences, both now normative text rather than folklore:

• the digest is only as time-resolved as the leaf digest's own centroid partition — a heavy leaf centroid whose members straddle a campaign gap contributes its whole mass at one interior instant. Gap visibility is a property of the per-centroid envelope words, which is what issue coverage sidecar: versioned temporal section (per-shard toc envelopes + root time-digest) — land before the source-coop build #480 actually claims, not of the means;
• a weight-1 centroid is exact — its word is a §8.3 timestamp and its mean is that instant, to float32.

The mean column is float32 on §8's internal-ns scale, so ~2^-24 relative precision (≈10 min at present-day magnitudes). The companion word beside each centroid is the exact temporal claim; a reader needing exactness uses the words. Weight conservation is unaffected and exact.

Writer — src/zagg/coverage_toc.py (new) owns the per-leaf read, the fold, the section grammar and the reader accessors. It rides zagg.sweep.MocFamily, the walk that already visits every leaf to roll the spatial coverage up to the root: each stamped leaf of a temporal-declaring store also yields its §8.3 envelope word and a per-leaf time digest, accumulated on the family instance (one per run — get_family constructs a fresh one) and folded in finish with ONE flat merge_tdigests_kway(..., temporal=). No new fleet pass; no new node artifacts.

• It stays out of the per-node rollup payloads on purpose: those are the skip-if-current currency, compared byte for byte, and a per-node k-way fold would describe a different centroid partition at every node.
• Memory is bounded by the per-leaf compression: each leaf is folded to ~δ centroids before it is returned, so the root fold holds n_leaves × ~δ rows — ~2.8 MB for a 2,726-shard California store, not n_leaves × n_cells × k. Recorded in the docstring.
• Partial contributions are never published: a shard any of whose window leaves failed to read is dropped from the map entirely, because a half-derived word would break §10.2's never-under-reports claim for a shard the map lists (absence is safe; a wrong word is not). A field a leaf never carried is not a partial read — it holds no observations, and §10.2 says so.
• refresh_root_coverage rebuilds the section from its own whole-store walk, and composes with the standing section rather than replacing it when any leaf read failed, so a degraded walk can never delete coverage it merely failed to observe.
• write_root_coverage composes at the GET-union-PUT seam (§10.4): tier 1 unions elementwise under toc_merge (idempotent), tier 2 is never unioned — its weights are counts and a union would double-count — only replaced, and only by a producer whose own map covered every shard the merged map lists.
• The sweep's skip-if-current test now compares what would actually be written (the composed section, on content, ignoring the churning source/generated_at) rather than a one-sided coverage predicate that could never converge.
• Discovery is declaration-driven: pyramid.overview.fields entries carrying temporal: "per-centroid" — never a leaf member enumeration, never a reconstructed naming convention. A manifest that declares temporal fields but no cell_order is refused as broken rather than silently read at group "0".

Fixtures — tests/data/spec/temporal/ gains a root coverage.moc, written through the production writer (MocFamily.read_leaf + finish); temporal.expected.json gains a root_coverage block. §7's fixture text records what moved and why.

Fixture diff

fixture	change
temporal/coverage.moc	new — the §10 golden
temporal.expected.json	+193 lines, additive only (root_coverage); git diff shows zero removals
temporal/**/zarr.json, morton_hive.json, all.pyramid.stats.json	timestamps and zagg_version only — the informative provenance §7 already excludes from conformance. Every content_hash is unchanged.
minimal/, kitchen_sink/, column/, flux/, raster_toc/, pyramid/	untouched — not one byte

None of the six declares a temporal field, so a sweep of one produces no section at all; leaving them without a root coverage object is §10's absence rule, and the conformance suite pins both halves — the missing object and the precondition that makes it correct (no temporal declaration in the manifest).

Tests

tests/test_coverage_toc.py (new) + TestRootCoverageTemporalSection in tests/test_spec_conformance.py:

• Tier-1 conservatism on committed bytes — every observation instant in the shard falls inside that shard's one envelope word (toc2time bounds and toc_overlaps per instant), and the shard word equals the join of the committed leaf words, derived from the leaf arrays rather than from the sidecar.
• The multi-field union — a manifest declaring two temporal fields folds to the join across both.
• Kway order-independence — three leaf orderings produce byte-identical payload and times base64.
• Weight conservation — sum(weights) == weight_total == obs_total (346, the fixture's own cell plan).
• The value axis — every committed root mean lies inside its own companion word, and each weight-1 centroid's mean is its word's exact instant to float32.
• Non-temporal byte identity — a sweep of minimal/ writes bytes equal to json.dumps(build_root_coverage(...), indent=1) with the §10 parameter omitted entirely, i.e. the pre-coverage sidecar: versioned temporal section (per-shard toc envelopes + root time-digest) — land before the source-coop build #480 call.
• Absence composition — every reader helper returns None for a missing section, an unknown revision, a non-dict, or None itself.
• Grammar validation — required key set, decimal-string words, the element/encoding/weights/value declarations, the centroids MUST-check, an array-level truncated-companion refusal, and a spec-text-only decode of the digest (base64 → raw LE buffer at the declared dtype, no zagg decoder) which is what "zero new grammar" means.
• Composition rules — elementwise union + idempotence, partial producer drops the digest, whole-covering producer replaces it, no-section producer preserves, unknown revision preserved-not-downgraded, a partial refresh composes with the standing section, and a multi-shard re-sweep writes nothing.
• Invariant worth naming: toc_reduce(root digest's companion words) == the shard word — the two tiers are two views of one join.

Full suite: 4481 passed, 7 skipped (test_lambda_build deselected — known environmental). ruff check / ruff format --check clean apart from the pre-existing N818 in registry.py.

Phases

• 1 — zagg.coverage_toc: section grammar, per-leaf read, k-way fold, GET-union-PUT composition, reader helpers; wired into build_root_coverage / write_root_coverage / MocFamily / refresh_root_coverage
• 2 — docs/specification.md §10 + the docs/hive_layout.md root-envelope pointer
• 3 — conformance fixtures regenerated; §7 fixture-description text; the non-temporal byte-identity proof
• 4 — tests
• 5 — adversarial self-review folded (16 findings; 15 folded across 15 commits, 1 left by design with its numbers on the thread)

Open-PR interactions

Based on main. #475 (spill knobs), #478 (aggregate hoists) and #479 (temporal spill-fold) are open on the worker/sweep surfaces; this diff is confined to the coverage/sweep/spec seams — src/zagg/coverage_toc.py (new), hive.build_root_coverage/write_root_coverage, sweep.MocFamily, coverage.refresh_root_coverage, docs/, tools/generate_spec_fixtures.py, the fixtures. It touches no worker, aggregate, spill or streaming code, and no overview-fold kernel. #479 changes how the temporal companion survives a cross-block spill fold — that changes what the leaves contain, not how this reads them, so a merge should be textual-conflict-free; a sync may still be wanted when they land (the #457 lesson).

Questions for review

1. Digest resolution. The two limits are now spelled out in §10.3 (centroid-partition granularity, and ~10 min float32 relative precision), with the companion words as the exact instrument. Two alternatives were rejected: float64 means (needs new kernels — build_tdigest / merge_tdigests_kway cast to float32 — and puts a non-§2.1 dtype in a "native form" block) and an origin-relative axis (breaks the k-way law's precondition that contributors share an axis, without a whole-store pre-pass). Flagging in case that resolution is not enough for the source-coop store's density queries.
2. The staged sweep (pyramid v2: staged dense sweep on the partition machinery #384) does not write the section. sweep_stages.run_finisher refreshes the root MOC through the same write_root_coverage, so an existing section is preserved across a staged sweep, but not refreshed by one — the staged path does not do the MOC family's per-leaf walk. MocFamily and refresh_root_coverage are the two producers. Worth a follow-up issue, or is preserve-don't-refresh the right posture there?
3. Multi-field weight semantics. With more than one temporal-carrying field, an observation contributing to several is counted once per field in weight_total. §10.3 says so explicitly and ties it to the flagged per-field-map question; single-field stores (every store today) are unaffected.

[espg]

🤖 from Claude (review)

Adversarial review — PR #481 (issue #480)

Read the full diff, the mortie toc_* docstrings, the §10 text against the code, and decoded the committed golden tests/data/spec/temporal/coverage.moc. Reproduced two of the findings against this branch. pytest tests/test_coverage_toc.py tests/test_spec_conformance.py is green (199 passed) and no CI/deploy/infra file is touched; module sizes are fine (coverage_toc.py 399 lines, sweep.py 1310 — the latter pre-existing and under the §4 raise trigger); no new dependency.

16 findings, by severity.

High (4)

1. section_covers + the whole-coverage digest rule deadlock: on any multi-shard store swept incrementally — the normal fleet path — the composed section can never carry a digest while finish always builds one, so coverage.moc is re-PUT on every pass forever with byte-identical content. MocFamily.finish's "an unchanged tree re-sweep is a no-op here too" holds only for the single-shard fixture the tests use. Reproduced.
2. An unknown-revision section is deleted, not ignored: merge_temporal_sections(v2_section, None) -> None, and write_root_coverage then omits the key. The dispatcher's ordinary end-of-run write destroys a future revision it merely does not understand — against §10.4's own "a producer with no temporal contribution at all leaves an existing section untouched" and the page's "readers add revisions, they never drop them". Reproduced.
3. Tier 2 is not a digest over acquisition times: it is the value digest's centroid weights collapsed onto each centroid's toc-envelope midpoint. A heavy centroid spanning two campaigns puts all its mass at a date with no data — the opposite of §10.3's "gaps between campaign clusters stay visible". The committed golden shows 35 centroids at 4 distinct float32 means. PR question (1) is the smaller half of this.
4. A partially-derived word is still published as the shard's envelope (a leaf missing one declared field; a windowed shard where one window's read fails). §10.2 promises the word "conservatively contains every observation instant in that shard" and toc_overlaps "never under-reports"; on those paths it does.

Medium (5)
5. MocFamily.read_leaf goes from a stamp GET to a full ragged payload + companion read with an unbounded in-memory fold, on a family in DEFAULT_FAMILIES; contrast sweep_overview.OVERVIEW_DELTA_CAP's explicitly bounded chunk-batched fold. "Nearly free" is unmeasured off a 3-cell fixture.
6. refresh_root_coverage's per-leaf fail-open becomes fail-destructive in aggregate — all-leaves-failed deletes the whole standing section, in the escape hatch operators reach for when something is already wrong.
7. Row alignment is checked per row, never at array level: a truncated companion is silently accepted and under-claims the word.
8. §10.3 makes centroids a MUST-check; coverage_toc_digest never reads it. Spec/implementation drift in the page whose whole purpose is preventing that.
9. Fixture regeneration was not "timestamps only": zagg_version regressed to 0.0.0+unknown in a committed fixture.

Low (7) — merged fields misdescribing the digest's provenance (10); temporal_shards telemetry reporting the un-merged count (11); a # type: ignore against §4 (12); the docstring edit landing inside a pre-existing broken sentence (13); a near-tautological absence test with repeated filler (14); a missing-test list, headed by the untested multi-field union that is §10.2's headline rule (15); cell_order or 0 masking a manifest problem as a temporal-read failure (16).

Nothing found on: fixture additivity (temporal.expected.json has zero removed lines and the other six fixture trees are untouched), caller-dict mutation in the composition helpers, MocFamily state leaking across runs (get_family does construct a fresh instance per run_sweep), work-set duplicate double-counting (_rollup_shard_node's known is a set), the k-way order-independence claim, toc_reduce/toc_merge/toc2time/toc_overlaps usage against their docstrings, or the run_finisher preserve-don't-refresh posture (PR question (2)) — which behaves as described, though it is worth an issue.

Answering the PR's questions directly: (1) the float32 quantum is real but finding (3) is the larger problem behind it; (2) preserve-don't-refresh is defensible, but finding (2) means "preserve" is not what actually happens for an unrecognised revision; (3) the multi-field weight semantics are documented but wholly untested and unfixtured — see finding (15).

Findings this review declines to fold stay standing for espg.

[espg]

🤖 from Claude (review)

(1) HIGH — the digest rule and section_covers deadlock, so every incremental sweep re-PUTs the root object forever.

    return not (incoming.get("digest") is not None and a.get("digest") is None)

build_temporal_section always attaches a digest when any leaf contributed, but §10.4's whole-coverage rule guarantees a partial producer's digest is dropped at the seam. On any store with more than one shard swept incrementally — i.e. the normal fleet path, since run_sweep only reads the leaves in the run record and untouched siblings come in through their stored rollups — the composed section permanently has no digest while every subsequent finish builds one. Reproduced against this branch:

after sweep A: shards ['11211'] digest? True
after sweep B: shards ['11211', '11212'] digest? False
section_covers(root, B) = False   <- False means the sweep PUTs again
re-merged identical apart from generated_at: True
section_covers again: False

So MocFamily.finish's docstring claim ("an unchanged tree re-sweep is a no-op here too") holds only for the single-shard temporal/ fixture that test_a_sweep_writes_the_section_the_fixture_committed exercises. Every other store rewrites coverage.moc on every pass, forever, with byte-identical content except generated_at — and pays the full per-leaf ragged read plus a root k-way merge to produce a digest that is discarded by construction.

Two things worth separating: the skip test should compare on what can actually survive composition (e.g. only demand a digest when this producer is whole-covering), and build_temporal_section should probably not spend the root merge at all when the caller's map is known not to cover the store.

[espg]

🤖 from Claude

Fixed in 84f99c3b. Real bug, and the repro is right: the skip test compared the
built section against the standing one, and a partial producer always builds a
digest that §10.4 always drops — so it never converged on a multi-shard store.

section_covers is gone; the test is now section_unchanged(existing, incoming),
which composes the merge and compares what would actually be written:

merged = merge_temporal_sections(existing, incoming)
return _content(merged) == _content(existing if isinstance(existing, dict) else None)

_content is shards + the digest block + fields — source and
generated_at churn per pass by construction and are excluded. Regression test on
a genuinely multi-shard store (the committed fixture's leaf cloned to a second
shard, two incremental sweeps, then a third and fourth pass):
TestOnCommittedStores::test_a_second_pass_over_a_multi_shard_store_writes_nothing,
plus a unit-level
TestComposition::test_a_partial_producer_converges_instead_of_re_putting_forever
that pins the asymmetry you identified. Confirmed the old rule against the new
merge output first — it returns False forever, exactly as reported.

Left alone by design: the second half of the suggestion (skip the root merge when
the caller's map is known not to cover the store). It is a real cost, but the map
is not known not to cover until the standing section is read, which finish does
after the section is built — reordering that is a design change rather than a fold,
so it stays standing.

[espg]

🤖 from Claude (review)

(2) HIGH — an unknown-revision section is destroyed on write, not just ignored on read.

    a, b = _usable(existing), _usable(incoming)
    if a is None and b is None:
        return None

_usable correctly degrades an unrecognised spec to None for reading. But merge_temporal_sections is also the write composer, and write_root_coverage does if section is not None: merged[TEMPORAL_KEY] = section — so a None return means the key is simply absent from the PUT. Verified on this branch:

merge(future, None) -> None

Consequences: a producer carrying no temporal contribution at all (the dispatcher's ordinary end-of-run build_root_coverage(...) never passes temporal=) silently deletes a standing zagg-coverage-toc/2 section it merely does not understand; and a v1 producer with a section replaces a v2 one wholesale (test_an_unknown_revision_is_ignored_not_merged at tests/test_coverage_toc.py:180 pins exactly that: merge_temporal_sections(future, good) == good).

That contradicts two normative statements this PR adds or relies on:

• §10.4: "A producer with no temporal contribution at all leaves an existing section untouched — it is not evidence of absence, only of a walk that did not look."
• the page's standing conformance rule: "readers add revisions, they never drop them."

In a mixed-version fleet (which is exactly the situation a versioned key exists for) the older zagg is the one that wins. Preserving an unrecognised section verbatim when this revision has nothing to put there would cost one branch.

[espg]

🤖 from Claude

Fixed in a685a91f. Agreed on both counts — the merge is the write composer, so
a None return deletes the key, and §10.4's own text said the opposite of what the
code did.

Two arms now:

1. incoming is None (or unreadable) → the existing value is returned verbatim,
   whatever revision it carries;
2. incoming is a zagg-coverage-toc/1 section and the existing one is at an unknown
   revision → refuse to downgrade: keep the existing verbatim and log a warning
   naming the revision found.

An unmarked carrier (a temporal value with no spec string at all) is still
replaceable — otherwise one bad write wedges the key shut forever; that distinction
is _preserved().

§10.4 gained a bullet stating the preserve-and-refuse-downgrade rule explicitly, and
test_an_unknown_revision_is_ignored_not_merged — which pinned the clobber — is now
test_an_unknown_revision_on_the_standing_side_is_preserved.

[espg]

🤖 from Claude (review)

(3) HIGH — the tier-2 block is not a digest over acquisition times; it is the value digest's weights collapsed onto envelope midpoints, and §10.3 does not say so.

    start, end = toc2time(np.asarray(words, dtype=np.uint64))
    return (np.asarray(start, dtype=np.float64) + np.asarray(end, dtype=np.float64)) / 2.0

read_leaf_temporal takes each cell's elevation t-digest, keeps cell[order, 1] as the weight, and re-keys the value axis to _centroid_times(words). The centroid partition is therefore the one the value distribution produced: every observation in a value-centroid is placed at the midpoint of that centroid's toc envelope, whatever the real temporal spread of those observations was.

Worked case: a cell whose elevation digest has one heavy centroid of weight 10,000 whose members span two campaigns six months apart. Its word is the six-month range envelope, so all 10,000 observations land at the three-month midpoint — a date at which the store holds no data at all. §10.3 says the opposite: "Gaps between campaign clusters stay visible because k centroids carry k envelopes." The words do stay visible; the density answer the block exists to give does not.

The committed golden already shows the degeneracy from the other side — decoding tests/data/spec/temporal/coverage.moc:

35 centroids, 4 distinct float32 means
rows 0-17 all at 5.344625417305194e+18 (weights 1,1,1,1,1,2,2,2,2,3,3,3,3,3,3,3,3,5)
rows 27-34 are DESCENDING in time under one shared mean

So on the one store this PR ships as the §10 conformance golden, "a CDF difference over the payload" resolves the whole 29-minute record into four buckets. That is not a counter-argument to shipping the tier, but §10.3's contract text presently promises a t-digest over acquisition times and the reader will build against that sentence. Either the spec paragraph names the actual construction (value-partitioned, weights placed at envelope midpoints; the words are the only exact temporal claim) or the writer builds the time digest from the words themselves. PR question (1) is about float32 resolution, which is the smaller half of this.

[espg]

🤖 from Claude

Correction accepted — §10.3 rewritten in f79aaacd. The semantics you describe are
what the writer does, and the spec text was promising something else.

What §10.3 now says:

• column 0 is derived from the companion words, not measured from the
  observations: each contributing centroid enters the fold at the midpoint of
  its own §8.3 envelope, and a merged centroid's mean is the weight-weighted mean of
  those midpoints;
• the weight-1 case is the one exact arm (word is a timestamp, toc2time gives
  (t, t), midpoint is that instant) — §8.3's kind-keyed semantics;
• every other mean is a convex combination of midpoints, so it lies inside its own
  centroid's word but at no observation, and the partition is one of value, so a
  heavy centroid straddling a campaign gap puts its mass inside that gap;
• the "gaps stay visible" claim is scoped to the envelope words — which is what
  issue coverage sidecar: versioned temporal section (per-shard toc envelopes + root time-digest) — land before the source-coop build #480 actually claims (k centroids = k envelopes) — with an explicit "a
  reader answering is there data in this window at all MUST read the words";
• the float32 relative-precision paragraph is folded in as the smaller half of the
  same limitation.

Weight conservation is untouched and still stated as exact.

One refinement to your description, from measuring it on the committed golden: the
mass is not placed at the merged word's midpoint — the merge averages the member
midpoints by weight, so the mean is a convex combination and lands inside the merged
envelope (up to float32 rounding) rather than at its centre. New fixture-pinned test
TestRootCoverageTemporalSection::test_the_value_axis_is_the_envelope_midpoint
asserts both arms on committed bytes: the 5 weight-1 rows equal their word's exact
instant to float32, and all 35 means lie inside their own envelope to the float32
quantum.

[espg]

🤖 from Claude (review)

(4) HIGH — a partially-derived word is still published as the shard's envelope, which breaks §10.2's containment contract silently.

        except KeyError:
            # Schema evolution: the field postdates this leaf. It contributes
            # nothing, exactly as the pyramid fold treats the same gap.
            logger.debug(f"coverage[toc]: leaf {leaf_root} lacks field {name!r}")
            continue

§10.2 states the word is "the grammar's join (mortie toc_reduce) over every §8.3 companion word the shard's leaves hold, unioned across all of the store's temporal-carrying fields", and the reader-facing consequence is "The word therefore conservatively contains every observation instant in that shard". Two paths publish a word that does not:

1. this continue — a leaf missing one declared field's arrays yields a word joined over the remaining fields, and the shard is listed as if the union were complete;
2. the window case — _rollup_shard_node calls read_leaf once per window, and MocFamily._accumulate_temporal swallows a per-leaf exception internally (sweep.py:219) while read_leaf still returns its MOC payload. A windowed shard whose window 2019 read fails and whose window 2020 succeeded is published with 2020's word alone. toc_overlaps then under-reports for that shard — the one thing §10.2 promises it never does.

§10.2's escape hatch covers only unlisted shards ("unknown, never empty"); it says nothing about a listed shard whose word came from a subset of its inputs. Either the fail-open has to be shard-scoped (any failure inside a shard drops that shard from shards entirely) or §10.2 has to stop calling the word conservative. The first is closer to the absence posture the rest of the section takes.

[espg]

🤖 from Claude

Fixed in 32b3614f, taking your first option — the fail-open is now shard-scoped.

MocFamily tracks _temporal_failed; any leaf read that raises drops that shard
from the contributions map entirely and keeps it out for the rest of the run (a later
window that reads cannot resurrect it). refresh_root_coverage applies the same rule.
A shard absent from shards is "unknown" and stays a candidate; a shard listed with a
partial word is a promise the section cannot keep.

§10.2 gained the matching sentence: containment is a claim about a listed shard,
so a producer whose read of any input behind a shard's word failed MUST omit that
shard rather than publish the word it managed to join.

Path (1) — the KeyError → continue for a field the leaf does not carry — is left
as-is, and the spec now says why: a field added to the store after that leaf was
written holds no observations in it, so the word over the remaining fields is still
whole. That is absence, not a failed read.

Tests: TestPartialReadsDropTheShard::test_a_failed_window_leaf_drops_its_whole_shard
(two window leaves, one failing) and
TestOnCommittedStores::test_refresh_drops_only_the_shard_whose_leaf_failed.

[espg]

🤖 from Claude (review)

(5) MEDIUM — this turns MocFamily's leaf read from a small stamp GET into a full ragged read plus an unbounded in-memory fold, and moc is in DEFAULT_FAMILIES.

        self._accumulate_temporal(store_root, decimal, leaf, store_kwargs)

Before this line the family read one commit stamp per leaf ("this family reads only the stamp envelope, never the cell-order bitmap sidecar"). After it, every stamped leaf of a temporal store also opens the leaf group and reads payload[:] and sibling[:] in full, per declared field, then decodes every populated cell, lexsorts it, and hands the whole list to one merge_tdigests_kway. Nothing bounds that: read_leaf_temporal holds one (k, 2) float32 array per cell in digests before merging, and the merge promotes to float64.

For a leaf with ~10^5 populated cells at δ=512 that is a couple of GB of leaf bytes plus a multi-hundred-MB fold buffer, on the same worker that previously did a JSON GET, and _rollup_shard_node re-reads every recorded window of a dirty shard on every pass. The overview family already treats this as the thing to bound — see sweep_overview.OVERVIEW_DELTA_CAP's comment: "the cap also bounds the sweep's chunk-batched k-way fold buffers (4 children × δ × cells), which saturate toward ~1 GB per slab". This module ignores that discipline entirely.

The PR body calls the writer "nearly free" and "no new fleet pass"; the pass is free, the reads are not, and the claim should either be corrected or backed by a measurement on a real leaf (CA o9, say) rather than the 3-cell fixture. A chunk-batched fold, or reducing per cell before accumulating, would keep the memory story consistent with the neighbouring code.

[espg]

🤖 from Claude

Leaving the fold as-is by design; docstring added in e7d20608 so the bound does not
have to be re-derived.

The numbers: read_leaf_temporal ends in a k-way merge that compresses the whole
leaf to ~δ centroids before returning, so what MocFamily accumulates is
n_leaves × ~δ × 2 float32 plus the same count of uint64 companion words — for a
2,726-shard California store ~1.4 MB of centroids plus ~1.4 MB of words, ~2.8 MB
total. Not n_leaves × n_cells × k. That is why the root fold needs no chunk
batching of its own, and it is a different regime from
sweep_overview.OVERVIEW_DELTA_CAP's 4 children × δ × cells slab.

On the per-leaf read itself: it is the same shape as the overview family's existing
leaf read — _fold_node also does arr[:] per declared field and accumulates a
per-cell centroid list before merging (sweep_overview.py, the partials /
cell_digests loop). So this adds a read of the same class the sweep already
performs on the same leaves, not a new one.

The PR-body wording ("nearly free") is the writer's own framing from issue #480 and
does overstate the read side; that is a PR-body edit rather than a diff change, and a
measurement on a real CA o9 leaf is the right way to settle it — left standing for
review rather than folded.

[espg]

🤖 from Claude (review)

(12) LOW — # type: ignore in new code, against CLAUDE.md §4.

        return dict(b)  # type: ignore[arg-type]

§4: "Do not add broad lint-ignore / # noqa / # type: ignore blocks to silence ruff or mypy — fix the cause or flag it." This one is narrow rather than broad, but the cause is fixable: mypy cannot narrow b because the guard is the compound if a is None and b is None. Splitting it (if b is None: return dict(a) if a is not None else None) or an assert b is not None removes the suppression.

[espg]

🤖 from Claude

Fixed in bd568c54. Splitting the compound guard removes the cause, exactly as
suggested:

a, b = _usable(existing), _usable(incoming)
if b is None:
    return dict(a) if a is not None else None
if a is None:
    return dict(b)

a685a91f (finding (2)) then restructured the same two branches further for the
unknown-revision rule; no suppression came back.

[espg]

🤖 from Claude (review)

(13) LOW — the new sentence lands inside an already-broken one and re-adds the dangling "A".

    lives at the leaf's OWN node. A temporal-declaring store (spec §10, issue
    #480) also has its ``zagg-coverage-toc/1`` section rebuilt from this same
    walk — fail-open per leaf, so an unreadable companion costs the section a
    shard, never the refresh. A
    supersedes it). A successful refresh also re-arms the

main already carries the stray A\n supersedes it). fragment, so this is pre-existing damage — but the PR edits exactly this sentence and leaves a second dangling "A" behind it. Cheap to clean up while here (and CLAUDE.md §4 asks for docstring quality on the code you touch); leaving it means the next reader has to work out that two separate sentence fragments collided.

[espg]

🤖 from Claude

Fixed in 439fcef9. The inserted sentence now sits after the pre-existing
fragment rather than inside it, so A\n supersedes it). is left byte-identical to
main and the new text reads as one complete sentence following it.

The pre-existing breakage itself is left alone deliberately: it is main's, not this
PR's, and repairing it would need the missing clause reconstructed from the #200/#246
history — that is a small-fix of its own, not a fold. Flagging it here so it does not
disappear.

[espg]

🤖 from Claude (review)

(14) LOW — this test is close to a tautology, and its second assertion is filler repeated six times.

    def test_non_temporal_fixtures_carry_no_root_coverage_object(self, name):
        assert not (SPEC_DATA / name / "coverage.moc").exists()
        assert coverage_toc(None) is None

The first assertion is true of origin/main too — none of those six fixtures ever had a root coverage object — so it pins that the generator was not run on them, not that §10's absence rule holds. The claim the PR body makes ("a sweep of one produces no section at all") is only actually tested by tests/test_coverage_toc.py::test_a_non_temporal_store_writes_byte_identical_bytes, on minimal/ alone. The second assertion has nothing to do with the parameter and re-asserts the same fact six times; it is already covered by TestAbsence::test_readers_return_none_cleanly.

Making this test sweep each fixture (or at least assert temporal_fields(manifest) == {} per store, which is the actual precondition) would pin the rule rather than the tree.

[espg]

🤖 from Claude

Tightened in 2faa7674 — it was cheap, and your diagnosis is right: the assertion
pinned that the generator was never run on those trees, not that §10's rule holds.

The test now asserts the precondition per store and drops the parameter-independent
filler:

manifest = json.loads((SPEC_DATA / name / "morton_hive.json").read_text())
assert temporal_fields(manifest) == {}
assert not (SPEC_DATA / name / "coverage.moc").exists()

coverage_toc(None) is None was already covered by TestAbsence::test_readers_return_none_cleanly
and is gone from here.

[espg]

🤖 from Claude (review)

(15) LOW — the test set restates the implementation more than it pins the PR's claims. Missing cases, in rough order of value:

1. The multi-field union. §10.2's headline rule — "unioned across all of the store's temporal-carrying fields" — has no test at all. Every test here is single-field (["h_tdigest"]), and the only committed fixture declares one temporal field. The one place two fields could disagree is untested and unfixtured.
2. A leaf missing a declared field's arrays (coverage_toc.py:128 KeyError → continue) and a misaligned companion (coverage_toc.py:141 ValueError) — both are behaviours §10.2's containment claim depends on; neither is exercised.
3. The fail-open path itself — _accumulate_temporal's except Exception (sweep.py:219) has no test, so nothing pins that a failing leaf drops its contribution rather than the whole sweep, nor what the published shard word then means.
4. A windowed shard through the real reader. test_several_window_leaves_reduce_to_one_shard_word uses synthetic _leaf() tuples; no test drives read_leaf_temporal over a store with two window leaves, which is the path _rollup_shard_node actually takes.
5. The multi-shard incremental sweep — the case that surfaces finding (1) above; TestOnCommittedStores only ever sweeps the one-shard temporal/ fixture, which is why the idempotence break does not show up.
6. The empty-digest edge — a leaf whose companion rows are all empty (build_temporal_section skipping len(digest) == 0, then merge_tdigests_kway returning the empty (0, 2) array): _encode_digest_block would emit weight_total: 0.0 and empty buffers, and §10.3 says nothing about whether a zero-centroid block is legal.

test_section_covers (this line) is a good place to add the digest-asymmetry case from finding (1): section_covers(section_without_digest, section_with_digest) is False and stays False forever.

[espg]

🤖 from Claude

Most of this list is now covered; folded across several commits.

• (1) multi-field union — 851081e7. TestOnCommittedStores::test_the_shard_word_unions_across_every_temporal_field
  grafts a second temporal field onto a copy of the committed leaf (same payload rows,
  a companion whose words sit a whole campaign away) and asserts the shard word is the
  join across both, that neither field alone produces it, and — from the weight side —
  §10.3's once-per-field counting. No new committed fixture.
• (2) misaligned companion — b8bdbffa adds the array-level refusal and its test
  (finding (7)). The missing declared field stays a continue, and §10.2 now says
  why: a field a leaf does not carry holds no observations, so the word is still whole.
• (3) the fail-open path — 32b3614f. It is no longer just untested, it is
  different: a failed leaf now drops its whole shard, pinned by
  TestPartialReadsDropTheShard::test_a_failed_window_leaf_drops_its_whole_shard.
• (5) the multi-shard incremental sweep — 84f99c3b, which is where finding (1)'s
  regression test lives; it is exactly the case that surfaces the break.
• (6) the empty-digest edge — left standing. build_temporal_section skips
  zero-row digests, so a zero-centroid block is unreachable from this writer; whether
  §10.3 should legislate one is a spec question rather than a missing test.

(4) a windowed shard through the real reader is also left standing: it needs a
two-window committed store, and the §7 fixture set has none — building one is a
fixture addition, not a fold. The synthetic _leaf() coverage plus the real-store
multi-shard sweep are what stands in for it today.

[espg]

🤖 from Claude (review)

(16) LOW — or 0 turns a malformed manifest into a wrong group path instead of a skip.

            self._cell_order = int(manifest.get("cell_order") or 0)

cell_order is a required manifest key (hive.py:546), and read_leaf_temporal opens zarr.open_group(..., path=str(cell_order)). If it is missing or 0 this quietly asks for group "0", which either does not exist (caught by the fail-open two lines down, logged as "no temporal contribution" — a misleading message for a manifest problem) or, on a store whose shard order really is 0, silently reads the wrong group. refresh_root_coverage has the same line. Reading it strictly and skipping the section when absent would fail honestly.

[espg]

🤖 from Claude

Fixed in 00305ca5. cell_order is read strictly now, via a shared
temporal_cell_order(manifest) that returns None when the key is absent or
unparsable. Both call sites — MocFamily._accumulate_temporal and
refresh_root_coverage — treat a manifest that declares temporal fields but carries no
cell_order as "no temporal fields", with a warning that names the real problem:

sweep[moc]: {store_root} declares temporal fields but carries no cell_order —
publishing no §10 section rather than guessing a group

So it fails honestly instead of asking for group "0" and reporting it as a missing
contribution, and it can no longer read the wrong group on a store whose shard order
really is 0.

Test: TestOnCommittedStores::test_a_manifest_without_a_cell_order_publishes_no_section
— the fixture's cell_order is deleted, and both the sweep and the refresh publish no
section and say why.

[espg]

🤖 from Claude

Status: all five phases done and all 16 review findings answered; held off gh pr ready because the test matrix on the folded head is stalled, not failing.

• Phases 1–5 complete. 15 fold review: commits landed on top of the four phase commits; every inline thread has a reply (16 findings, 15 folded, 1 left by design with its numbers on the thread).
• Local, in-worktree, on the exact pushed head 044d7054: pytest -q --deselect tests/test_lambda_build.py → 4481 passed, 7 skipped; ruff check src tests tools clean apart from the pre-existing N818 in src/zagg/registry.py; ruff format --check clean.
• CI on 044d7054: ruff, build, build / build-x86_64, build / build-arm64 all pass. test (3.12) and test (3.13) are stuck — the run's updated_at has not moved off its run_started_at while the sibling runs from the same push finished, and the same suite completed in 8m02s / 8m35s on the previous head of this branch. That is the PR-event stall this repo has hit before, whose remedy is a close/reopen of the PR — a side-effecting action I am leaving for you rather than taking on my own.
• No label state applied, so the PR stays actionable: once those two checks land green it needs only the draft→ready flip. Nothing here is waiting on a code decision — the three items under Questions for review in the description are review questions, not blockers.

[espg]

🤖 from Claude

Correction to the status above: the stall cleared on its own — all six checks are green on 044d7054.

The 08:00Z comment reported test (3.12) / test (3.13) as stuck and asked for a close/reopen. Both completed shortly after it was written (run 32113604993, 08:11Z and 08:13Z), joining ruff, build, build / build-x86_64 and build / build-arm64. No close/reopen is needed and nothing is outstanding on the diff: five phases done, 16 review findings answered (15 folded, 1 left by design with its numbers on the thread).

So the only thing between this PR and review is the draft→ready flip, which is held on the standing conflict recorded on issue #50 — CLAUDE.md §2 has the author make that flip once phases are complete, findings are folded and CI is green (all true here), while the routine prompt forbids it outright. Per the routine's own conflict rule that stops rather than guesses, waiting stays on and the flip is left for a human. Everything else is finished.

---

Generated by Claude Code

[espg]

🤖 from Claude

Decision record (espg, in-session 2026-08-18) — two of this PR's three review questions are ruled; the PR merges as-is, nothing in it changes.

1. Staged sweep preserve-don't-refresh → ratified as the permanent posture. The composition analysis closed the correctness concern: every observation-adding path unions its words through write_root_coverage's merge seam and joins only grow envelopes, so claims stay true automatically; contraction leaves over-wide-but-true words; the walk is the tightener. Documentation of that contract is spec 10: document the walk-as-tightener refresh contract (staged sweep preserves, composition keeps claims true) #487 (small-fix). The mixed-fleet clobber belt (sweep warns when leaves carry companions but the root has no section) is sweep: warn when leaves carry temporal companions but the root has no coverage-toc section #488 (small-fix), with the reader-side sibling warn when a temporal-capable store has no readable root coverage-toc section (pruning degraded) espg/moczarr#47.
2. Multi-field weight semantics → deferred, with the surface pinned. The union stays this revision's only map ("any data here" is what candidate pruning asks; for today's stores every temporal-carrying field ingests the same observation set, so per-field maps would be near-identical copies). The per-field surface — shards_by_field tier-1 maps and the per-field tier-2 weight/digest semantics — lands additively in zagg-coverage-toc/2, bundled with the newly ruled mandatory space-time floor (require the temporal section on every store: granule-envelope passthrough floor + provenance grades (coverage-toc/2) #486: every store gets a section; granule-envelope passthrough + provenance grades), so the section's surface moves once. Related future refinement tracked: time-occupancy refinement: gap-preserving sub-shard temporal coverage (word-set cover / time-keyed digest) #489 (gap-preserving word-set cover / time-keyed structure, post-source-coop).
3. Digest δ for the source-coop store — still standing for espg; unchanged by the above.

Also ruled on the same thread of discussion: space and time coverage stay together in the one root object (one GET, one succession rule; a two-object split could tear a fresh-spatial/stale-temporal pair under the aoi= AND when= query).