Skip to content

leaf skip-if-current: input-identity no-op with lifecycle touch (issue #388) - #397

Merged
espg merged 81 commits into
mainfrom
claude/388-leaf-skip
Aug 7, 2026
Merged

leaf skip-if-current: input-identity no-op with lifecycle touch (issue #388)#397
espg merged 81 commits into
mainfrom
claude/388-leaf-skip

Conversation

@espg

@espg espg commented Aug 5, 2026

Copy link
Copy Markdown
Member

Closes #388. Refs #381 (point (11)). Blocked by #391 Unblocked 2026-08-07: #391 merged into main (ad8aa30); base retargeted to main and origin/main merged into the branch (stacking order #389#391 → this PR held).

What this does

Leaf-level skip-if-current: a re-dispatched unit whose planned inputs and semantics match what its leaf already records no-ops the fold, reports current in the run stats, and touches every object under the leaf's prefix footprint (S3 self-copy, MetadataDirective=REPLACE) so bucket lifecycle purges see actively-used products. Identity is the ruled PAIR — the run's semantic_hash (D19, what/how) × the unit's planned granule-id set (over what) — compared per (shard, window) unit against the leaf's D20 sidecar; whole-set under all/ad-hoc. Both leaf families ship it (espg ruling on the issue): the vector seam (zagg.hive.process_and_write_hive) and the raster seam (zagg.processing.raster.process_and_write_raster_hive), shared by both backends.

  • Fast path: one granules_sha256 compare (+ semantic_hash equality). The id-set difference runs only on hash mismatch.
  • Contraction guard (RULED — option (2), refuse-without-flag; predicate recorded ∖ planned ≠ ∅, so the mixed add-and-drop signature trips it): the unit refuses, names the missing granule ids, and counts in its own run-stats counter — never folded into failures — unless allow_contraction rides the run (an agg() kwarg + a worker event field; deliberately NOT a config knob, which would pre-approve contraction store-wide against the ruling's intent).
  • Expansion / any other mismatch: wholesale D4 rewrite exactly as today (the pyramid v2: leaf-worker column writes (issue #383) #391 column artifact included).
  • Touch: best-effort, fail-open (a failed touch logs and degrades to today's behavior, never blocks); covers the unit's leaf .zarr tree, its stats sidecar and D22 sub-map, and the pyramid v2: leaf-worker column writes (issue #383) #391 column artifact + its sidecar. Touched-object count rides the run stats. Local stores touch via os.utime; S3 via a boto3 self-copy (already a dependency — obstore's copy has no metadata-directive control, and S3 rejects an identity self-copy without MetadataDirective=REPLACE).
  • A skipped unit writes nothing (no arrays, no stamp, no sidecar, no sub-map, no column) → zero sweep dirtiness by construction.

Phase 2 (what landed)

The gate lives in one shared helper, hive.leaf_identity_gate (sidecar read → dedup.classify_leaf_identity → skip/refuse/proceed verdict + the early-return unit metadata), called by both seams behind new opt-in kwargs (skip_if_current / allow_contraction / semantic_hash / sidecar_spec, all defaulting to today's behavior — the deployed Lambda handler is byte-identical until it opts in, per Questions (1)). The local backend arms the gate by default (skip_if_current=not overwrite on both the aggregation and raster paths — overwrite=True stays the operator's unconditional rewrite, per Questions (2)), passing the RUN config's D19 hash and the manifest spec in effect. A skipped/refused unit returns before any read or write and the dispatcher writes no record, no sidecar, no sub-map, no run-parquet row for it (the sidecar-clobber gate from Questions (1), now dispatcher-side on the local path). Both seams also now stamp metadata["semantic_hash"] (run-passed, or their own config's hash as fallback), which lights up phase 1's validated build_record fallback: fleet vector sidecars stop recording null for the identity half with zero handler changes (the pinned handler test now asserts the hash, not the null). Summaries gain three always-present counters — cells_current, cells_refused, cells_unrecorded (the inert-guard rewrites, counted apart) — on all four paths (spatial/raster × local/lambda; the lambda ones read worker envelopes and stay zero until the handler opts in). agg(allow_contraction=...) threads through every strategy; the lambda dispatchers add the allow_contraction event key only when true (both the _build_cell_event construction site and the raster event builder), so default event payloads stay byte-identical.

Phase 3 (what landed)

The touch lives in a new small module, src/zagg/lifecycle.py (hive.py was already past the §4 line aim, so the feature gets its own file, the column.py precedent): touch_current_unit assembles ONE unit's footprint — the leaf .zarr tree (commit stamp, arrays, and the in-leaf coverage.moc ride the prefix walk), the D20 stats-sidecar and D22 sub-map siblings via the spec-keyed naming seams (telemetry.sidecar_key / sweep.submap_key), and the #383 column tree plus its own stats sidecar when the run declares one — and touch_unit_footprint refreshes every object's LastModified. Local stores use os.utime; S3 uses the boto3 self-copy (CopyObject onto itself, MetadataDirective=REPLACE — one LIST per leaf/column tree, one copy per object). Both seams call it on the skip arm only (a refusal touches nothing — only a certified-current unit resets the purge clock), and only after the fold's artifact checks passed, so the touch cannot resurrect the column-drift ambiguity: the column is touched exactly when declared-and-committed. Fail-open is structural at three layers — per-object (count + log; an ABSENT sibling like a never-written sub-map is neither touched nor failed), per-footprint (assembly or LIST faults count one failure and abandon the remainder), and at the seam (try/except — a raising touch still returns current: True). Counts ride the unit metadata (touched_objects / touch_failed) and roll up into two new always-present summary keys, objects_touched / touch_failures, via the same _identity_counts seam as the phase-2 counters (never folded into cells_error). Documented caveats, not solved: versioned buckets mint a new version per touch; a ≥5 GB object would need multipart copy and simply counts failed; the self-copy REPLACEs user metadata with none (zagg's writers attach none).

The #391 handoff (deferred run-record key)

The 2026-08-07 scope-note ruling (issue comment) is discharged by phase 1metadata["column"] enters the run-record schema via the column key below (with both merge arms and the parquet round-trip pinned in the phase-1 fold), so no further phase-2/3 work item exists for it; only the rename question (5) stays open.

PR #391 deferred "should metadata["column"] enter the run-record schema" to this PR (the fold reply). Landed here in phase 1: the D20 record gains column (the artifact basename — a scalar string, in _EQ_OR_NONE_KEYS and _ROW_SCALARS, so D22 run-record discovery sees column-bearing leaves without a tree listing) and granule_ids (the recorded id list behind granules_sha256, in the hash's own canonical order — what a contraction refusal diffs against to name dropped ids; record + sidecar only, never a parquet column). The column's resolution set deliberately stays in the artifact's own zagg_column attrs — the skip gate verifies column currency by reading the artifact (the spec's "read the columns, not the manifest" posture), not by trusting a record that can lag it.

Phases

  • phase 1 — identity comparison core (zagg.dedup.classify_leaf_identity: equal / expansion / pure-contraction / mixed, every ambiguity → rewrite) + the D20 record keys above + the semantic_hash metadata fallback (the shared seams will stamp it, so Lambda-written sidecars finally record the identity half — today the handler passes none and fleet vector sidecars carry null) + tests
  • phase 1 adversarial-review fold — 7 findings: 5 folded (unknown-planned split from the empty planned set; the one-sided-conservatism correction on the unrecorded-ids rationale; D19-shape validation of the metadata semantic-hash fallback; the column-is-null meaning pinned; both merge arms + a write_run_parquet round-trip for column), 2 standing as design forks under Questions for review (5) and (6)
  • phase 2 — worker-side skip + contraction guard on BOTH leaf seams, run-stats counters (cells_current / cells_refused, plus unrecorded-ids counted apart from ordinary rewrites), allow_contraction plumbing (run kwarg + event field) + tests (windowed included)
  • phase 2 adversarial-review fold — 6 findings, 5 folded: the leaf's D4 commit stamp is now a precondition for the skip (a sidecar outliving its leaf can no longer certify an absent leaf current forever — bac97ec); the pyramid v2: leaf-worker column writes (fleet side) #383 leaf column is verified as an artifact rather than trusted from the record, so enabling/dropping output.pyramid over an existing store rewrites instead of skipping (5ab707d); a valid-JSON non-object sidecar degrades to rewrite instead of failing the unit (6967258); the CLI gained the three guard counters and --allow-contraction (f3ba09b); the raster gate matrix + the raster dispatcher's event-key pin now mirror the vector ones (546dac8). The raster Lambda handler's dropped semantic_hash is named at the seam and folded into question (1) (bebfe68) rather than fixed in deployment/aws/. Two halves stay standing as questions (8) and (9).
  • phase 3 — lifecycle touch (unit-footprint-wide, column included, fail-open) + tests (local store + mocked S3 client)
  • phase 3 adversarial-review fold — 6 findings, all 6 folded: the store-ROOT objects an all-skip run never re-PUTs now get a once-per-run touch on the local wrap-up seam (c71198f); the self-copy preserves StorageClass instead of silently demoting every touched object to STANDARD, with an exact ACL/KMS caveat list (b6caad1); the S3 client is cached process-wide per store kwargs and built under a lock through its own Session instead of once per unit off the default session (7cc08cb); touch_failures gets its own CLI line so a blanket AccessDenied can't render as unqualified success (3c7269f); the raster-handler seam note now names six dropped keys, not four (5ed8970); the column sidecar name comes from column._sidecar_name and the over-touch direction is pinned (58f06fb). The fleet half of finding (1) is question (10) below.
  • phase 4 — docs/hive_layout.md operator section (stating plainly that the contraction guard cannot protect leaves written before this release) + the spec assessment below

Spec assessment (flagged, not silently decided)

Phase-4 re-verification (final tree): the assessment above holds after phases 2–3 and both folds — docs/specification.md, tools/generate_spec_fixtures.py, and tests/data/spec/ are untouched across the whole branch; no spec marker string moved anywhere under src/zagg; SCHEMA_VERSION stays 1 (the two new D20 keys are additive and nullable, and the D20 design record never enumerates per-key additions — the n_obs_read/spill_blocks_closed precedent — so no design-doc edit is due either). The lifecycle touch is contract-invisible by construction: it changes only LastModified (plus the documented S3 system-metadata side effects — ACL/SSE-KMS; storage class is preserved), object content is byte-identical, the §5 content hashes are over decoded values, and no zagg reader keys on the LastModified of any store object (the poller reads it on .status/ result objects only, which are never touched). The gate itself only READS existing artifacts, and its column check implements §4.6's already-ratified "config decides, read the columns not the manifest" posture — no new normative language needed. The operator-facing semantics land in docs/hive_layout.md (narrative docs, phase 4), where §4.7's informative scope-out already places the stats/sub-map families.

No byte-contract change. The stats/sub-map families are scoped out of docs/specification.md as operational (§4.7 informative: "no new byte layouts — the stats rollup reuses the D20 sidecar schema"); the only spec-normative pieces of the sidecar are the §5.3 content_hashes shape and the sidecar naming grammar, both untouched. The two new D20 record keys (column, granule_ids) are nullable, telemetry-class additions per the D20 design record (docs/design/sparse_coverage.md), consistent with how n_obs_read (#374) and spill_blocks_closed (#370) landed; SCHEMA_VERSION stays 1 (additive nullable keys, no key changes). No spec marker moves, so no conformance-fixture regeneration.

Testing

  • After the merge of origin/main (post-pyramid v2: leaf-worker column writes (issue #383) #391) and phase 2: uv run pytest -q = 3,593 passed / 38 skipped / 1 failed — the failure is the pre-existing environment-dependent test_lambda_build.py::TestFunctionBuild::test_function_build_succeeds (flagged not fixed; same set pyramid v2: leaf-worker column writes (issue #383) #391 reports — the flaky test_client_transport.py::TestStatusPoller::test_invoke_fault_burns_an_attempt_and_retries passed on these runs). Phase 2 adds 20 tests: the seam gate matrix (skip / off-by-default / contraction / mixed / flagged-contraction / expansion / semantic-mismatch / unrecorded-ids / no-sidecar / hash stamp) on both leaf families, the per-window gate on a windowed store, three end-to-end local agg rerun tests (identical→cells_current with fold-not-invoked + sidecar-byte-equal, overwrite disables, contracted→refuse-then-flag-rewrites), the two allow_contraction event-key tests, and the summary-key byte-identity pins extended with the three counters. One existing test updated deliberately: test_hive_event_writes_stats_sidecar pinned semantic_hash is None on Lambda-written sidecars — the null phase 1 exists to eliminate; it now pins the recorded hash.
  • After the phase-2 review fold: uv run pytest -q = 3,621 passed / 38 skipped / 1 failed (same pre-existing lambda-zip-build failure). The fold adds 28 tests: destroyed-leaf and unstamped-debris reruns on both seams, a parametrized non-object-sidecar matrix, six column-drift cases (declaration on/off/unchanged, per-window, and the premise that the D19 hash does not move), the six raster gate cases that were vector-only (off-by-default / no-sidecar / unrecorded-ids / expansion / semantic-mismatch / mixed), three raster local agg rerun tests, the two raster allow_contraction event-key tests, and four CLI summary tests.
  • After phase 3: uv run pytest -q = 3,637 passed / 38 skipped / 1 failed (same pre-existing lambda-zip-build failure). Phase 3 adds 16 tests: tests/test_lifecycle.py (local footprint incl. column family, absent-column/absent-sub-map neither-touched-nor-failed, failing utime counted-not-raised, unresolvable footprint fail-open; mocked-S3 self-copy asserting MetadataDirective=REPLACE + identity CopySource over the enumerated footprint, NoSuchKey = absent not failed, AccessDenied counted, LIST-fault abort, store-kwargs→client keying, local-store-builds-no-client) plus seam/rollup coverage (both seams' skip tests now pin bytes-identical + every-mtime-advanced + exact touch counts, refusals pin NOT-touched, a raising touch never un-skips, the column family moves with the leaf in test_column.py, and the local rerun asserts the objects_touched/touch_failures summary rollup). The two phase-2 “wrote nothing” pins moved from mtime-equality to content-equality deliberately — the touch's entire purpose is to move mtimes on a skip.
  • After the phase-3 review fold: uv run pytest -q = 3,646 passed / 38 skipped / 1 failed (same pre-existing test_lambda_build.py::TestFunctionBuild::test_function_build_succeeds). The fold adds 9 tests: the touch_store_root family (local three-object touch, absent root MOC neither touched nor failed, missing root fail-open, S3 exact-keys-no-LIST), the storage-class preservation pin (LIST/HEAD echo + STANDARD default when S3 omits it), the client-cache pins (same kwargs → one client across units, different kwargs → a second; 16 concurrent touches across 8 threads build exactly one), the two CLI touch_failures pins, and the OVER-touch pins (a node neighbour / sibling window / string-prefix-extension key is not touched, in test_lifecycle.py and end-to-end in test_hive_windows.py::test_skip_if_current_is_per_window). Two existing reruns (test_hive.py vector, test_raster_runner.py raster) now also assert the store-root manifest and aggregation.yaml mtimes advanced on a skip.
  • After phase 4 (docs-only): uv run pytest -q = 3,645 passed / 38 skipped / 2 failed — the same pre-existing pair flagged at the top of this section (test_function_build_succeeds; the flaky test_invoke_fault_burns_an_attempt_and_retries, which fails on clean origin/main, resurfaced on this run). No code changed in phase 4.
  • uv run pytest -q at the merge commit (pre-phase-2): 3,573 passed / 38 skipped / 1 failed (same pre-existing failure) — the pyramid v2: leaf-worker column writes (issue #383) #391 merge broke nothing.
  • uv run ruff check src tests / uv run ruff format --check src tests: clean on the diff; pre-existing N818 at src/zagg/registry.py:64 and the format diff on tests/data/benchmark/README.md flagged, not fixed.

Questions for review

  1. The Lambda handler enablement is deliberately NOT in this diff (deployment/aws/ is excluded from agent edits per CLAUDE.md §1 unless an issue names it by name). The seams are opt-in kwargs (default off), so the deployed handler's behavior stays byte-identical until it opts in. To light the feature up fleet-side the handler needs, in both leaf branches: skip_if_current=True, allow_contraction=bool(event.get("allow_contraction")) on the seam call, plus its sidecar/sub-map writes gated on not metadata.get("current") — without that gate a Lambda skip would clobber the good sidecar with an n_obs=0 record, which is exactly why the seam defaults off. The dispatcher already sends the allow_contraction event field (only-when-true, so existing payloads stay byte-identical). Happy to draft that handler change as a separate espg-named follow-up.

    The raster branch needs more than the vector one. The aggregation branch returns the seam's own dict ("body": json.dumps(metadata), build_record(metadata=metadata, ...)), so phase 1's metadata fallback fires there and fleet vector sidecars stop recording null with no handler change. The raster branch does not: it rebuilds a fresh body from an explicit key list and passes THAT to build_record, so semantic_hash / identity / current / refused are dropped on the floor (grep -n semantic_hash deployment/aws/lambda_handler.py returns one hit, a docstring). Consequences: raster fleet sidecars keep recording null today, and once the gate is armed a null record can never match a resolved hash, so classify_leaf_identity falls through to semantic-mismatch and every raster leaf rewrites forever without self-healing; _identity_counts over the raster lambda envelopes also stays permanently zero. The follow-up must therefore add body["semantic_hash"] = meta.get("semantic_hash") plus the three gate keys (identity / current / refused) and the two phase-3 touch keys (touched_objects / touch_failed) alongside the opt-in kwargs — six keys, not four. Without the last two the fleet raster rollup would report cells_current: N, objects_touched: 0, touch_failures: 0, indistinguishable from a touch that never ran. The seam carries this note at the stamp site so the requirement is not only in a PR body (5ed8970).

  2. overwrite=True disables the skip — an operator forcing overwrite gets today's unconditional rewrite. Reasonable, or should skip still apply under overwrite?

  3. The recorded id space is driver-dependent on the vector path (resolved s3 vs https hrefs — the documented dedup.shard_status trap). A driver flip between runs reads as a full mixed contraction and refuses per leaf; allow_contraction is the escape hatch and the docs call it out. OK to leave as-is (consistent with granules_sha256 semantics) rather than normalizing ids?

  4. The contraction guard is inert on every leaf written before this release. granule_ids arrives with this PR, so a pre-leaf skip-if-current: input-identity no-op with lifecycle touch (semantic_hash x granule-id set) #388 sidecar has no recorded set to diff and classifies unrecorded-idsrewrite — i.e. a contracted rerun over an old leaf performs the wholesale rewrite the guard exists to prevent, silently. Phase 1 gives it its own classification so phase 2 can count it apart from ordinary rewrites and phase 4 can say so plainly, which is the plan of record. The alternative is to make unrecorded-ids refuse without allow_contraction, which is safe but means essentially every fleet leaf refuses once until it has been rewritten under this release. Ruling wanted: (a) count-and-document (current), or (b) refuse-until-rewritten?

    Raised by the phase-1 self-review at this comment; folded the docstring-accuracy half, left the behavior fork here.

  5. Rename the column record/parquet key? PR pyramid v2: leaf-worker column writes (issue #383) #391 deferred the key's existence into this PR; the name is the part still open, and it is cheap to change only while the schema is unreleased. column is SQL-reserved and reserved in both engines this parquet targets (DuckDB, Trino/Athena), so every filter must quote it — WHERE "column" IS NOT NULL; the unquoted form is a parse error. It is also ambiguous on the sidecar: the pyramid column's own stats record carries "column": null (only the leaf record carries the basename), so it reads as "the column basename this unit wrote alongside its leaf", not "the column this record is". Pick one and it is a two-line change: (a) keep column (comment already documents the quoting), (b) column_name, (c) leaf_column — which also resolves the sidecar ambiguity outright, (d) pyramid_column.

    Raised by the phase-1 self-review at this comment and this one; the inaccurate "plain column filter" comment is fixed, the rename is not taken on agent authority.

  6. Where should granule_ids ride? (supersedes the earlier sizing question, which measured against the wrong cap). At the pole figure, ~4,600 ids ≈ 550 KB per unit. That is comfortably under the 6 MB sync-response cap, but the costly consumers are the fan-out readers, and neither reads the field: dedup.shard_status GETs one sidecar per shard (has_run loops it over the whole shardmap) and reads only semantic_hash / granules_sha256 / content_hashes; telemetry.rows_from_status GETs every mirrored envelope through a thread pool inside a 900 s worker and flatten_record discards granule_ids (deliberately not in _ROW_SCALARS). So a few-thousand-leaf pre-run dedup check could go from a few MB of GETs to the better part of a GB. Options: (a) keep as-is on the record (envelope + sidecar) and measure both paths before phase 2 lands; (b) sidecar-only augmentation at the write sites — keeps it off the transport entirely, but needs the handler edit from (1) to cover fleet leaves; (c) split the id list into its own sibling object next to the sidecar, so identity-check readers keep their small GET and only a contraction refusal pays for the list — this adds an object family to the leaf footprint, which phase 3's touch and phase 4's docs would both have to cover.

    Raised by the phase-1 self-review at this comment; left standing as an architecture decision rather than folded.

  7. granule_workers is not in DATA_SOURCE_PACKAGING_KEYS, which makes the seam's fallback hash clamp-sensitive (found implementing phase 2). The seams' semantic_hash=None fallback hashes the config the WORKER received — but both dispatchers send a per-cell config whose data_source.granule_workers is clamped to min(K, n_granules) (runner._clamped_data_source, issue Follow-ons to #183: dispatcher-side shard-aware granule_workers, and function-size variants gated on utilization #184), and semantics.DATA_SOURCE_PACKAGING_KEYS = ("reader", "driver", "read_plan", "anonymous") does not exclude granule_workers even though the D19 docstring says "read machinery" is packaging. So a small shard's worker-side hash can differ from the run-level hash. The drift is one-sided by construction: a mismatch degrades to today's rewrite (or a stale in dedup.shard_status), and any collision implies output-identical semantics — never a false skip. The local runner is immune (it passes the run hash into the seam), and phase 2 documents the caveat on the seam docstring. The real fix — adding granule_workers to the packaging keys — changes every existing product's D19 hash (manifest refusals on rerun into existing named stores), so it is not taken on agent authority. Ruling wanted: (a) live with the documented one-sided drift (current), or (b) add granule_workers to DATA_SOURCE_PACKAGING_KEYS in a dedicated PR with a migration note.

    A second consequence weighs on the same fork: semantic_hash is in telemetry._EQ_OR_NONE_KEYS, so a cross-leaf rollup collapses it to None whenever a run mixes clamped and unclamped shards. That is not a regression (the key already collapses on any genuine divergence), but it means the clamp can erase the run's identity half from every rollup record, which (b) would also fix.

  8. How far should the recorded identity chase the leaf's ARTIFACT SET? The phase-2 fold closed the concrete regression — a config that declares output.pyramid.overviews will not skip a leaf that has no committed column beside it, and vice versa (hive.leaf_column_expectation, verified by reading the artifact per the spec's §4.6 "read the columns, not the manifest" posture). But the general hole stands: the whole output block is outside semantics.semantic_core and pyramid is deliberately not a frozen manifest key, so any output knob that changes what a leaf contains moves neither identity half. output.sharded is the checked example — flipping it over an existing store changes the leaf's object layout while the hash and the granule set both hold, so the gate reads equal. Options: (a) leave it — the column was the only knob that adds/removes an artifact, and a sharded flip over an existing store is already an operator-forced rewrite (--overwrite); (b) record a small output-layout digest in the D20 sidecar (not D19 — moving D19 breaks manifest compat) and add it as a third identity half; (c) widen semantic_core to include the leaf-shaping output keys, which is the same migration cost as (7)(b). This is deliberately not taken on agent authority: (b) and (c) both change what a sidecar means.

  9. Should an all-skipped / all-refused run still write a run-record parquet? Today it does not, and the CLI half of that gap is now closed (the summary prints cells_current / cells_refused / cells_unrecorded and names --allow-contraction when anything refused — f3ba09b). The durable half is left standing because it is a D20 schema decision, not an observability tweak: a skipped or refused unit writes nothing and produces no meta["stats"], so rows is empty and both _write_run_stats and _dispatch_run_stats return before the PUT. Synthesizing rows would mean emitting D20 records for units that wrote no leaf — records whose n_obs, content_hashes, and cells_with_data describe nothing — which changes what a run-record row is, and it interacts with (6): a refusal row's useful payload is exactly the granule_ids diff that question is trying to keep off the transport. Options: (a) leave it (the counters are in the summary and in the worker logs, and the leaf sidecars are untouched by construction); (b) add a distinct refusal row shape to the run parquet (identity, missing_granules count, no content fields), which is a SCHEMA_VERSION conversation; (c) write a separate small refusal manifest at the store root when cells_refused > 0.

  10. Where should the FLEET's store-root lifecycle touch live? The phase-3 review found that an all-skip run never refreshes the store-root objects: ensure_manifest accepts a frozen-key-matching manifest with no second PUT (and aggregation.yaml is written only inside that PUT branch), and every skip-capable run is overwrite=False by construction (skip_if_current=not overwrite). So {store}/morton_hive.json — REQUIRED reader-facing schema (D6) — keeps its original LastModified across arbitrarily many "keep this store alive" reruns and expires first under a bucket rule, bricking a store whose data objects are all pristine.

    Fixed for the local backend (c71198f): lifecycle.touch_store_root touches the manifest, aggregation.yaml and the root coverage.moc once per run, gated on cells_current > 0, riding the same post-units wrap-up seam that already GET-union-PUTs the root coverage.moc — the seam whose own comment records why it is not a D8 violation ("the LOCAL dispatcher runs it in-process — this process is also the worker and already PUTs every leaf with these credentials").

    Not fixed for the fleet, deliberately. D8 is a standing ruling (the dispatcher never writes; all store writes are worker-side), and a lifecycle self-copy IS a write — so the Lambda dispatchers cannot make these three requests themselves, and no existing worker-side seam runs exactly once per fleet run. Options, all needing a ruling: (a) ride the mode="sweep" worker invoke (the §7 sweep is already the D8-sanctioned "orchestrator needs a store write → send a worker" path) and have it touch the root when the run reports cells_current > 0; (b) a new one-shot mode="touch" worker invoke at finalize; (c) leave the fleet gap open and document it (the operator's bucket rule would need to exclude the root prefix). Until one lands, an all-skip fleet run refreshes every leaf and no root object.

    Same shape, second half: the §7 sweep's ancestor-node overviews and pyramid.json envelopes belong to no unit footprint and are not root objects either, and a skip produces zero sweep dirtiness by construction — so a skip-only store ages out its whole pyramid above the leaves while the leaves and the root survive. Touching them means either a store-wide walk (which this unit-scoped module deliberately does not own) or teaching the sweep to touch-not-rebuild the clean ancestors of a current leaf. Folded as a documented gap in the zagg.lifecycle module docstring rather than an invented walker.

    Raised by the phase-3 self-review at this comment.

Generated by Claude Code

espg added 30 commits August 4, 2026 16:49
@espg

espg commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

Phase 4 pushed (8304b04, docs-only): docs/hive_layout.md gains the operator section — the identity gate's three verdicts, local-on/--overwrite-off enablement (Lambda handler still opted out, question (1)), the plain statement that the contraction guard cannot protect leaves written before this release (unrecorded-ids → silent wholesale rewrite until a leaf is rewritten once under it, counted as cells_unrecorded), the driver-flip and output-knob caveats (questions (3)/(8)), the lifecycle-touch footprint + fail-open counters + S3 caveats, and the known gaps stated honestly (pyramid-above-the-leaves ages on skip-only runs; store-root touch is local-only — question (10)) with lifecycle-rule guidance around them; plus a Status bullet. Spec assessment re-verified on the final tree and recorded in the body: no byte-contract change, specification.md/fixtures untouched branch-wide, SCHEMA_VERSION stays 1, the touch is contract-invisible (LastModified only; no reader keys on it). Checklist complete: all four phases done. Gates: 3,645 passed / 38 skipped / 2 pre-existing flagged failures; ruff clean on the diff. Next: phase-4 review cycle; questions (1)/(7)/(8)/(9)/(10) standing.

Comment thread docs/hive_layout.md Outdated
`kms:Decrypt` + `kms:GenerateDataKey`.

**Known gaps** — plan lifecycle rules around them, they are not promised
away: the §7 sweep's **ancestor overviews and `pyramid.json` envelopes

@espg espg Aug 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

[high] pyramid.json names no object zagg writes — the gap list points an operator at a filename that does not exist, and names one of four aging families.

grep -rn "pyramid\.json" src tests docs deployment returns exactly two hits: this line and the zagg.lifecycle module docstring it was drafted from (src/zagg/lifecycle.py:67). The sweep's per-node envelopes are {family}.rollup.jsonsweep.py:105 (rollup_name), and the module docstring at sweep.py:39: "Rollup objects are JSON sidecars at digit nodes, named {family}.rollup.json". One per family in DEFAULT_FAMILIES = ("stats", "moc", "submap", "overview") (sweep.py:61), plus a root sweep_stats_{ts}.json per pass (sweep.py:689). Nothing anywhere is called pyramid.json.

Two knock-on inaccuracies in the same sentence:

  • it scopes the gap to "ancestor overviews and pyramid.json" — but stats.rollup.json, moc.rollup.json and submap.rollup.json at every digit node are equally outside the unit footprint and equally untouched by a skip. A skip-only store ages its whole ancestor rollup layer, not just the pyramid.
  • the workaround an operator will reach for — "just run the sweep periodically to keep them alive" — does not work, and the section does not say so: "A second sweep over an unchanged tree recomputes but PUTs nothing" (sweep.py:33). For a paragraph whose whole purpose is "plan lifecycle rules around them", that is the load-bearing half.

If pyramid.json was instead meant as the manifest's pyramid block: that block lives inside morton_hive.json, which is in the store-root trio lifecycle.touch_store_root refreshes — so under that reading the sentence names a non-gap. Either way the name needs to change here and at lifecycle.py:67.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude

Folded in c3ae8b4. (Restating the finding here — the comment above posted as a literal scratchpad path rather than its contents: pyramid.json names no object zagg writes, the gap was scoped to one of four aging families, and the "run the sweep to keep them alive" workaround silently no-ops.)

Confirmed all three: grep -rn "pyramid\.json" src tests docs deployment now returns zero hits — it previously returned exactly the two the finding names (docs/hive_layout.md:699 and the src/zagg/lifecycle.py:67 docstring it was drafted from), and both are fixed in the same commit. The lifecycle change is docstring-only, no behavior diff.

The gap paragraph now names the real artifacts and scopes the aging honestly:

That is the whole ancestor layer, not the overviews alone — all four rollup families age identically (stats.rollup.json, moc.rollup.json, submap.rollup.json, overview.rollup.json at every digit node, one per sweep.DEFAULT_FAMILIES entry), plus each pass's root sweep_stats_{ts}.json run record. And re-running the sweep does not refresh them: a second sweep over an unchanged tree recomputes but PUTs nothing, so the obvious "just sweep periodically to keep them alive" workaround silently no-ops.

The sweep.py:33 sentence was the load-bearing half you flagged and it is now stated in both places — the doc paragraph above and the zagg.lifecycle module docstring ("A repeat sweep does not refresh them either (an unchanged tree recomputes but PUTs nothing)").

Comment thread docs/hive_layout.md Outdated
**local backend only** (D8 keeps the Lambda dispatcher from writing to the
store, and the handler has no root-touch mode yet — PR #397 question (10)).
Until those close, do not scope a hard-expiry rule over the whole store
prefix and rely on skip re-runs alone to keep it alive: scope expiry to the

@espg espg Aug 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

[medium] The lifecycle-rule prescription is not implementable with an S3 lifecycle rule, and an operator following it verbatim expires the very objects the previous sentence flags.

"scope expiry to the leaf data planes" presumes leaves are separable from ancestor overviews by a lifecycle filter. They are not:

  • docs/specification.md §4.1: an overview zarr is written "at an ancestor digit node of the hive tree" — inside the same {sign+base}/{d1}/…/ prefix tree as the leaves it summarizes, just shallower.
  • §4.2: "Nothing about the name distinguishes an overview from a leaf" — both are {window}.zarr / all.zarr; classification is by root-group attrs (§4.3), which no lifecycle engine can read.
  • S3 lifecycle filters key on prefix, object tags, and size only — no suffix, no depth — and zagg's writers set no object tags. The only prefix that selects leaves while excluding ancestor overviews is a per-shard-node prefix, i.e. one rule per leaf node against a 1,000-rule-per-bucket cap.

So the rule an operator actually writes is over the digit tree, which sweeps up the ancestor overviews this same paragraph just said a skip never touches. The second alternative has the same defect: "give root and pyramid objects their own (longer) retention" is expressible for the root trio (three named objects at a known prefix) and not expressible for the pyramid, for the reason above.

Not data loss — §4.1 makes overviews "regenerable caches, never load-bearing", so the cost is a full pyramid rebuild via the sweep — but the paragraph presents this as the safe configuration, which is the opposite of its intent. Suggest replacing the prescription with the constraint: there is no prefix or tag that separates leaves from ancestor overviews today, so a hard-expiry rule over the digit tree costs the pyramid (rebuildable by a sweep) and the root trio must be excluded by name.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude

Folded in f9b6ffd. (Restating — the comment above posted as a literal scratchpad path: the lifecycle-rule prescription is not implementable as an S3 rule, and an operator following "scope expiry to the leaf data planes" verbatim expires the pyramid the previous sentence just flagged.)

The prescription is gone. It is replaced by the constraint plus three honest options:

There is no lifecycle rule that separates leaves from the ancestor artifacts above them. An S3 lifecycle filter keys on prefix, object tags, and size only; zagg's writers set no object tags; and per the specification §4.1–§4.2 an overview lives at an ancestor digit node inside the same prefix tree as the leaves it summarizes, where "nothing about the name distinguishes an overview from a leaf" (classification is by root-group attrs, which no lifecycle engine reads). So "scope expiry to the leaf data planes" is not writable as a rule: the only prefix that selects leaves while excluding ancestors is a per-shard-node prefix, against a 1,000-rule bucket cap.

Then (1) no expiry rule on a pyramid-declaring hive store; (2) expiry over the digit tree accepted with the ancestor artifacts named as regenerable caches per §4.1 — a sweep rebuild, not data loss — and the store-root trio excluded by name because morton_hive.json is REQUIRED reader-facing schema; (3) the levers named as not existing yet: write-time object tags (nothing in zagg writes any) and a scheduled ancestor-layer refresher, with issue #384's staged sweep called "the obvious candidate, but nothing about it promises a refresh today" and question (10) cross-referenced for the fleet-side root touch. No tagging invented, no #384 behavior promised.

Comment thread docs/hive_layout.md Outdated
it entirely (the operator's unconditional-rewrite hammer — it does not
acknowledge a contraction, it bypasses the guard). The **deployed Lambda
handler has not yet opted in**: the seams default off, so fleet re-runs
still rewrite unconditionally today, and fleet sidecars written by current

@espg espg Aug 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

[medium] "fleet sidecars … feed the gate only after the handler enablement lands" is wrong in both directions — vector sidecars start feeding it on redeploy with no handler change, and raster sidecars never will, including for LOCAL re-runs where the gate is armed.

Vector — the claim is too pessimistic. deployment/aws/lambda_handler.py:1842-1850 passes the seam's own metadata dict straight to build_record (with granule_ids=event.get("granule_urls") already riding), and the seam now stamps metadata["semantic_hash"] from the run value or its own config (hive.py:1396-1400, hive.py:1589-1590), which build_record's validated fallback picks up (telemetry.py:183-186). So the moment this release is deployed — gate still off fleet-side, zero handler changes — fleet vector sidecars record both identity halves and do feed the gate (a local re-run over a fleet-built vector store skips). The PR body says exactly this: "fleet vector sidecars stop recording null for the identity half with zero handler changes". The doc contradicts its own PR body.

Raster — the claim is too optimistic, and this is the operator-visible half. The raster branch rebuilds body from an explicit key list (lambda_handler.py:1355-1370) and never copies semantic_hash, so raster fleet sidecars record semantic_hash: null even after this release ships. In classify_leaf_identity that makes semantic_match False (dedup.py:242), the fast path fails, granule_ids is present (raster_granule_ids(event["granules"]), lambda_handler.py:1396), the id sets are equal, and it lands on semantic-mismatch → rewrite (dedup.py:265-270). Every raster leaf, every run, no self-heal until that leaf has been rewritten under this release.

The consequence this section owes the reader: an operator re-running a fleet-built raster store locally — where the gate is on by default per the previous sentence — gets cells_current: 0 forever, and nothing here explains why. That is question (1)'s six-key requirement leaking into local behavior, not just fleet behavior.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude

Folded in 68ec5f1. (Restating — the comment above posted as a literal scratchpad path: the "fleet sidecars feed the gate only after the handler enablement lands" sentence is wrong in both directions — vector needs no handler change, raster never self-heals, and the raster consequence is visible on local re-runs.)

Verified both halves against the tree before rewriting: the vector branch hands the seam's own metadata dict to build_record (lambda_handler.py:1842-1850), the seam stamps metadata.setdefault("semantic_hash", ...) (hive.py:1589-1590), and build_record's validated fallback picks it up (telemetry.py:183-186); the raster branch rebuilds body from a closed key list (lambda_handler.py:1355-1370) with no semantic_hash in it.

The single sentence is now a two-bullet split. Vector: "from this release's deploy onward — gate still off fleet-side, zero handler changes — fleet vector sidecars carry both identity halves, and a later local re-run over that store can skip", which is what the PR body already asserted. Raster: semantic_hash: null → fast path fails → semantic-mismatch rewrite every run, "with no self-heal until the leaf has been rewritten under a handler that carries the key", plus the operator consequence you asked for:

The consequence is local, not merely fleet-side: re-running a fleet-built raster store locally — where the gate is on by default — reports cells_current: 0 indefinitely until question (1)'s key-list fix lands.

Comment thread docs/hive_layout.md Outdated
| verdict | when | what happens |
|---|---|---|
| **current** | both halves match, leaf stamped, column agrees with the declaration | fold no-ops; the unit writes **nothing** (no arrays, no stamp, no sidecar, no sub-map, no column — zero sweep dirtiness); the lifecycle touch below runs; counted as `cells_current` |
| **refused** | the planned set drops recorded ids: `recorded ∖ planned ≠ ∅` — deliberately *not* strict-subset, so a shardmap that grew while silently dropping old granules (an upstream purge behind a fresh catalog query) still trips it | the unit refuses, names the missing ids in the log, writes nothing, and counts as `cells_refused` — never as an error. `--allow-contraction` (`agg(allow_contraction=True)`; on Lambda an `allow_contraction` event field) turns it into a normal rewrite |

@espg espg Aug 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

[low] "names the missing ids in the log" — the log names at most five of them.

hive.py:1294-1297 truncates before the warning:

shown = ", ".join(missing[:5]) + (f", +{len(missing) - 5} more" if len(missing) > 5 else "")
logger.warning(
    f"shard {shard_key}: REFUSED — planned inputs drop {len(missing)} recorded "
    f"granule id(s) ({shown}); pass allow_contraction to rewrite (issue #388)"
)

The full set rides only metadata["missing_granules"], and a refused unit writes no run-record row (question (9)), so on a real contraction the dropped ids are not recoverable from the log or from any durable artifact — the operator sees five names and a count. Worth saying "names the first few missing ids and their total count", since the table row is what an operator will plan a triage step around.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude

Folded in 7c62ae6. (Restating — the comment above posted as a literal scratchpad path: "names the missing ids in the log" overstates, since hive.py:1294 truncates to missing[:5] + a count.)

The refused table row now reads:

the unit refuses, logs the first five missing ids and their total count (the full list rides only the in-process missing_granules metadata, and a refused unit writes no run-record row — PR #397 question (9) — so plan triage around the count, not around recovering the set from durable artifacts), writes nothing, and counts as cells_refused — never as an error.

That states both halves you flagged: the truncation, and the recoverability limit (no durable artifact carries the full set, because the refusal writes no row — cross-referenced to question (9), where the refusal-row shape is the standing decision).

Comment thread docs/hive_layout.md Outdated
current too, verified by reading them, never trusted from the record: the
leaf must carry its commit stamp, and the [leaf
column](#leaf-columns-zagg-column1) must agree with the run's declaration
(the §4.6 config-decides gate: the declaration moves *neither* identity

@espg espg Aug 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

[low] Bare §4.6 here and bare §7 at line 699 resolve to two different documents, and specification.md's own §7 is an unrelated section.

§4.6 is docs/specification.md §4.6 (Leaf column artifacts, zagg-column/1). §7 at line 699 ("the §7 sweep") is docs/design/sparse_coverage.md §7 — docs/specification.md §7 is "Conformance fixtures". A reader who resolves the first reference to the spec and carries that document 67 lines forward lands on the wrong section.

This doc otherwise qualifies its spec references: "the specification §4.4–§4.5" (line 308), "Byte grammar: specification.md §4.6." (line 375), "design §4" (line 519). Matching that convention here is one link. (Bare "§7 sweep" is pre-existing at line 793 and fine on its own — it is the collision with an unqualified spec § inside the same new section that misleads.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude

Folded in d4a65bf. (Restating — the comment above posted as a literal scratchpad path: bare §4.6 and bare §7 67 lines apart resolve to two different documents, and specification.md §7 is the unrelated "Conformance fixtures".)

Both are qualified per the doc's own convention (lines 308/375/519):

The follow-on fold of finding (2) added two more qualified refs in the same section ("the specification §4.1–§4.2", "specification §4.1"), so every § in the new section now names its document. The pre-existing bare "§7 sweep" at line 793 is left alone as you noted — it carries no adjacent spec § to collide with.

@espg
espg marked this pull request as ready for review August 7, 2026 23:20
@espg espg added the waiting label Aug 7, 2026
@espg

espg commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

All four phases complete, all four adversarial-review cycles folded (26 findings total), CI green on f9b6ffd — flipped ready-for-review + waiting per §2. The review agenda is the Questions section: (5)/(6)/(9) shape schema/artifacts that become released at merge (leans on record in-session: (5)(c) leaf_column, (6)(c) sibling id object, (9)(c) refusal manifest + (a) for pure skips) — cheapest decided pre-merge; (1)/(2)/(3)/(4)/(7)/(8)/(10) are follow-up material and do not block (leans: (1) plumbing-now/arm-later, (2) keep, (3) leave now, (4)(a), (7)(b)+(8)(c) batched as one hash epoch timed with fleet arming, (10)(a) sweep-mode).

@espg

espg commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

espg rulings (in-session, 2026-08-07): questions (5)/(6)/(9) are decided per the recorded leans — (5)(c) the record/parquet key renames to leaf_column; (6)(c) granule_ids moves to a sibling object beside the sidecar (identity equality stays a granules_sha256 compare on the small sidecar; the id list is read only to name a diff on mismatch); (9)(c)+(a) a refusal manifest at the store root when cells_refused > 0 (identity classes + missing-granule diff, sourced from (6)'s sibling objects), and pure-skip runs stay row-less. Landing as pre-merge commits through the normal review pipeline; questions (1)/(2)/(3)/(4)/(7)/(8)/(10) remain follow-up material per the same comment.

@espg

espg commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

Follow-up bookkeeping now lives outside this thread: #402 tracks all ten review questions (rulings, landing sites, sequencing); #401 is the espg-named handler-plumbing issue for question (1a); the D19 hash-epoch companion ((7)(b)+(8)(c)) is recorded on #384 plan delta 2. The (5)/(6)/(9) pre-merge trio is in flight on this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

leaf skip-if-current: input-identity no-op with lifecycle touch (semantic_hash x granule-id set)

2 participants