D24 composability: admit build_waveform_digest via a shared digest-family registry (issue #508)

[espg]

Closes #508

D24's field_composability admits ragged fields as approximate only when the reducer is in zagg.processing.streaming._TDIGEST_FUNCTIONS, so zagg.stats.waveform.build_waveform_digest classifies none and a GEDI waveform store gets no overview ladder even with output.pyramid = {} — the SERC probe's observed manifest was pyramid.overview.fields.rx_flux = {"class": "none"} (issue evidence). Fix per the plan: one shared digest-family registry ("stored §2 payloads fold by the k-way t-digest law") consumed by the D24 ragged arm, with build_waveform_digest admitted. The licensing fact is that the k-way law is weight-agnostic — flux weights fold like counts (issue #431 §2 weights declaration) — and the pyramid fold operates on stored (k, 2) centroid payloads via merge_tdigests_kway, never re-running the builder, so no new merge law is introduced.

Phases

• Phase 1 — characterization tests pinning how each gate treats build_waveform_digest today: D24 → none (meta-level pin existed; added the template-level pin composability_classes(gedi01b)["rx_flux"] == "none"); validate_streaming (merge mode) refuses via the temporal arm for the template shape and via the no-merge-law arm otherwise; the spill path accepts the config non-mergeable — validate_spill_fold refuses the builder, SpillAggregator records the verdict, the single-block regime replays the pooled machinery byte-identically (payload AND per-centroid temporal channel — this is the mechanism the SERC 0.47–0.49 fleet runs used), and the first block close raises SpillOverflowError naming rx_flux. No gate was found inconsistent: every build-time gate treats the builder as outside the digest family; only the D24 stored-payload classification is at issue.
• Phase 2 — shared digest-family registry (c1fc01a0): _DIGEST_FAMILY_FUNCTIONS = (*_TDIGEST_FUNCTIONS, build_waveform_digest) in processing/streaming.py, consumed by the D24 ragged arm only; _TDIGEST_FUNCTIONS / _TDIGEST_SPILL_FUNCTIONS keep their exact members (pinned by value in a new test); build_tdigest_where deliberately excluded pending the gate-drift ruling raised on issue D24 composability: build_waveform_digest not admitted as approximate — GEDI pyramid declares class none with pyramid on #508; stale docstrings (field_composability, build_waveform_digest, gedi01b template comment) updated; phase-1 D24 pins flipped to approximate.
• Phase 3 — template-time classification end to end (218c9f35): gedi01b + pyramid = {} + rx_flux.overview_delta = 512 (+ uniform δ4096 test override) → exact manifest decl {class: approximate, method: tdigest_kway, delta: 4096, overview_delta: 512, temporal: per-centroid, inner_shape: [2]}, the seven per-record companions pinned at option-A {class: none}; the /2 default ladder covers node 9→0 with one fields map; the overview template (via composable_fields → _overview_config, the production writers' path) emits rx_flux_times with the §8.3 per-centroid declaration and binds it by name, with the class-none companions absent.
• Phase 4 — fold parity (d472c23b): synthetic waveform store (leaves built by build_waveform_digest itself, integer photoelectron counts so float32 parity is exact) swept through the production overview fold — per-level flux weight_total == leaf total at BOTH declared levels with the rx_flux_times sibling row-aligned at each; single-contributor round-trip pins the (2,) element byte-identical (decode → (k, 2) float32 → re-encode == stored bytes, temporal words exact).

Testing

• uv run pytest tests/test_spill_crossblock.py tests/test_streaming.py tests/test_sweep_overview.py — green (new: TestWaveformSpillBaseline, 2 merge-mode pins, 1 template-level D24 pin).
• Full pytest -q: 4610 passed; one pre-existing failure unrelated to this change (see below).

Deployment note (recorded, not in this PR)

Classification runs worker-side at template time, so the fix bites on the 0.50 fleet. The CA GEDI build proceeds pyramids-OFF on 0.49 per the runbook contingency; pyramids retrofit later via declare_pyramid + a sweep-only pass on the 0.50 fleet (derived artifacts, no re-aggregation). Validation: rerun the ~$0.07 SERC probe on the 0.50 fleet and check the runbook step-2 gates (ladder present, rx_flux_times at every level, weight-total parity, manifest declaration).

Questions for review

• Worker-side column fold at GEDI scale (phase-2 review finding, left standing): the class flip also switches ON the leaf-column fold — leaf_column_plan on a pyramid-ON gedi config now folds rx_flux in fold_column at the tail of hive.process_and_write_hive (pinned in tests/test_column.py::TestWaveformEntersTheColumn, docstrings corrected in 1123cd24). The 4-shard SERC probe fits; the runbook's pyramids-off + declare_pyramid + sweep-only retrofit avoids it for CA — but a full-AOI build declaring pyramid = {} puts a node-order k-way merge over 21–33M centroids on a worker whose measured HWM is already ~2.5 GB (column.py records ~2.0 GB at ~17.6M centroids). Options: (1) leave as-is and keep the runbook contingency normative (this PR's posture); (2) measure on the 0.50 fleet before declaring pyramid-ON aggregation green; (3) a knob gating the column fold (design change, not taken without a ruling).

• Flux ladder reads as counts until PR GEDI waveforms 1/3: §2 counts/flux weights declaration + δ=8,192 raise #431 lands: overview rx_flux arrays carry no weights/gain declaration while the template's weights: flux stays commented out pending the GEDI waveforms 1/3: §2 counts/flux weights declaration + δ=8,192 raise #431 rebase (recorded in the template comments, 69cf3c7c); post-GEDI waveforms 1/3: §2 counts/flux weights declaration + δ=8,192 raise #431 the carried gain is a bare scalar without the §2.0-required name/version — standing for the GEDI waveforms 1/3: §2 counts/flux weights declaration + δ=8,192 raise #431 rebase.

• build_tdigest_where gate drift raised on the issue (phase-1 review finding): D24 classifies strata none while _TDIGEST_SPILL_FUNCTIONS documents the k-way fold — status quo pinned; ruling standing at D24 composability: build_waveform_digest not admitted as approximate — GEDI pyramid declares class none with pyramid on #508 (comment).

• Pre-existing local failures (not touched): tests/test_client_transport.py::TestStatusPoller::test_invoke_fault_burns_an_attempt_and_retries flakes locally under machine load (injected-clock poller liveness; passes and fails on identical code) — green in CI. Also: ruff format --check tests fails on tests/data/benchmark/README.md under the dev env's newer ruff (formats markdown code blocks; pre-commit pins v0.14.10 where it passes), and tests/test_lambda_build.py::TestFunctionBuild::test_function_build_succeeds fails on clean origin/main on this machine (local deploy-script build environment). Both reproduce with the working tree stashed; flagged rather than fixed per CLAUDE.md §4.

• build_tdigest_where and D24: stored stratum payloads also fold k-way (_TDIGEST_SPILL_FUNCTIONS says so), yet where-fields classify none in D24 today. The plan scopes D24 composability: build_waveform_digest not admitted as approximate — GEDI pyramid declares class none with pyramid on #508 to admitting build_waveform_digest; the where-field question is left standing for a separate ruling rather than silently unified (mortie Parallelize the opt-in metadata-consolidation path (deferred from #191/PR #193) #194 lesson).

[espg]

🤖 from Claude (review)

Phase 1 missed the one gate pair that is already inconsistent — build_tdigest_where — and phase 2's shared registry will silently flip a second shipped template.

The plan comment sets the exit condition for this phase: "The existing tuples keep their exact current members and semantics unless phase 1 proves a gate is already inconsistent, in which case the finding is raised on the issue rather than silently unified." That case exists today and nothing in this commit pins it.

validate_spill_fold already declares the where-stratum reducer a member of the k-way t-digest family (src/zagg/processing/streaming.py):

#: Ragged reducers the SPILL fold additionally accepts (issue #370): ...
#: It folds k-way, like the ``build_tdigest`` it delegates to.
_TDIGEST_WHERE_FUNCTION = "zagg.stats.tdigest.build_tdigest_where"
_TDIGEST_SPILL_FUNCTIONS = (*_TDIGEST_FUNCTIONS, _TDIGEST_WHERE_FUNCTION)

D24 disagrees — it tests _TDIGEST_FUNCTIONS, so a strata field is none even though its stored payload is an ordinary (k, 2) centroid array indistinguishable from build_tdigest's, and the pyramid fold (TDIGEST_LAW) is the same k-way merge. Verified on the shipped strata template:

$ python -c "... composability_classes(default_config('atl03_tdigest_strata_healpix'))"
{'count': 'exact', 'h_tdigest_signal': 'none', 'h_tdigest_noise': 'none', 'composition': 'none'}

That is the same latent bug shape as #508, in a second template, and phase 2 will walk into it: a registry named "folds by the k-way t-digest law" built from the spill-side membership flips both strata fields without a decision being recorded. Simulated the phase-2 change to confirm it is not hypothetical:

# _TDIGEST_FUNCTIONS += (build_waveform_digest, build_tdigest_where)
gedi01b rx_flux            -> approximate        # the intended #508 fix
atl03 strata h_tdigest_*   -> approximate        # collateral, unrecorded

Concretely:

1. add a phase-1 pin beside test_a_temporal_waveform_field_is_still_none recording today's field_composability({... "function": "zagg.stats.tdigest.build_tdigest_where", "inner_shape": [2], "params": {"where": "h_ph > 0"}}) == "none" (and/or composability_classes(default_config("atl03_tdigest_strata_healpix"))), so phase 2 cannot flip it invisibly; and
2. raise the D24-vs-validate_spill_fold divergence on issue D24 composability: build_waveform_digest not admitted as approximate — GEDI pyramid declares class none with pyramid on #508 per the plan, so the phase-2 registry's membership for build_tdigest_where is a recorded decision rather than a side effect of the waveform admission.

[espg]

🤖 from Claude

Folded, both halves.

(a) Pinned — 76468a0 adds test_where_strata_template_classifies_none_today to TestComposabilityClasses, recording composability_classes(default_config("atl03_tdigest_strata_healpix")) with h_tdigest_signal and h_tdigest_noise == "none" today, plus the meta-level pin on a build_tdigest_where declaration (params: {where: ...}, matching the templates shape). Its comment states the gate drift is raised on issue #508 and that the phase-2 registry deliberately does NOT admit build_tdigest_where — so the collateral flip you simulated cannot happen silently.

(b) Raised — #508 (comment) states the _TDIGEST_SPILL_FUNCTIONS-vs-D24 divergence, cites the docstring that calls it a k-way member, shows the strata classes, and offers the two options: (1) keep strata none (recommended, this PRs posture) or (2) admit build_tdigest_where to the shared registry in a follow-up.

Phase 2 stays scoped to (*_TDIGEST_FUNCTIONS, "zagg.stats.waveform.build_waveform_digest"), with _TDIGEST_FUNCTIONS/_TDIGEST_SPILL_FUNCTIONS membership untouched. tests/test_sweep_overview.py::TestComposabilityClasses green (25 passed).

[espg]

🤖 from Claude

Correction to the count in my previous reply: tests/test_sweep_overview.py::TestComposabilityClasses is 24 passed, not 25 (the class had 23 tests before 76468a0). Green either way; the miscount was mine, the fold is unchanged.

[espg]

🤖 from Claude (review)

The recorded spill baseline is a 2-field synthetic, and the docstring's causal claim does not hold for the config the SERC runs actually used.

The docstring attributes the non-mergeable verdict to the builder alone:

the mechanism that carried them is the NON-mergeable single-block regime — validate_spill_fold refuses the builder (it is outside _TDIGEST_SPILL_FUNCTIONS)

For the shipped gedi01b_waveform_healpix_hive the refusal is over-determined — the builder is one of eight problems, and the other seven are the per-shot companions:

$ python -c "validate_spill_fold(default_config('gedi01b_waveform_healpix_hive'))"
spill blocks cannot cross-block fold this config:
  field 'rx_flux': ragged function 'zagg.stats.waveform.build_waveform_digest' has no fold law ...
  field 'shot_count': scalar function 'zagg.stats.waveform.shot_count' has no cross-block fold ...
  field 'shot_number': scalar function 'zagg.stats.waveform.shot_number' ...
  field 'noise_mean' / 'noise_stddev' / 'rx_energy' / 'elevation_bin0' / 'elevation_lastbin':
      scalar function 'zagg.stats.waveform.single_shot_value' ...

The template's own comment says the same thing (worker.extra_disk): "validate_spill_fold rejects it (build_waveform_digest and the single_shot_value companions have no cross-block fold law)".

Why it matters for a characterization phase: _waveform_variables() declares only count + rx_flux, so the pinned baseline is a config where the builder is the sole cause. A later reader — or phase 2 — could conclude from this class that admitting build_waveform_digest to a shared digest-family registry would change the GEDI store's spill posture. It would not: the seven scalar companions keep the shipped config non-mergeable regardless, so spill_blocks_closed: 0 stays a hard precondition of the deployed template. That is the load-bearing baseline fact and it is currently unrecorded.

Suggested fix — one extra pin on the real config, plus a docstring correction:

def test_the_shipped_template_is_refused_by_more_than_the_builder(self):
    # The deployed config's refusal is OVER-DETERMINED: even if the builder
    # were admitted, the seven per-shot companions keep it non-mergeable, so
    # issue #508 cannot move the GEDI store off single-block spill.
    from zagg.config import default_config

    with pytest.raises(ValueError) as exc:
        validate_spill_fold(default_config("gedi01b_waveform_healpix_hive"))
    assert "'rx_flux'" in str(exc.value)
    assert "'shot_count'" in str(exc.value) and "single_shot_value" in str(exc.value)

and reword the docstring's second sentence to "validate_spill_fold refuses the builder and the per-shot single_shot_value/shot_* companions".

[espg]

🤖 from Claude

Folded in 0de5dbf.

New pin — test_the_shipped_template_is_refused_by_more_than_the_builder runs validate_spill_fold(default_config("gedi01b_waveform_healpix_hive")) and asserts the refusal names rx_flux/build_waveform_digest and all seven per-shot companions (shot_count, shot_number, noise_mean, noise_stddev, rx_energy, elevation_bin0, elevation_lastbin) plus zagg.stats.waveform.single_shot_value. So the over-determination is recorded field-by-field: even if issue #508 admitted the builder, the deployed store stays non-mergeable and spill_blocks_closed: 0 remains a hard precondition.

Docstring corrected — it no longer attributes the shipped templates verdict to the builder alone; it now reads that validate_spill_fold refuses the builder "and on the SHIPPED template the refusal is OVER-determined, naming the per-shot single_shot_value/shot_* scalar companions alongside it", and says explicitly that the 2-field synthetic isolates the builder while the last test pins the deployed config where it is not the only cause. The new tests comment quotes the templates own worker.extra_disk line for provenance.

tests/test_spill_crossblock.py::TestWaveformSpillBaseline green (5 passed; 29 across both targeted classes, 302 across the three touched suites).

[espg]

🤖 from Claude (review)

The flip does not leave build time alone: it switches on the leaf column fold for rx_flux, and the repo's own measurement says a GEDI-scale digest load does not fit there.

Measured on this commit vs origin/main, gedi01b template with output.pyramid = {}:

# origin/main:  leaf_column_plan(cfg, HealpixGrid(9, 18, config=cfg, sharded=True, chunk_inner=12))
pyramid={} -> ([12, 11, 10, 9], ['count'])
# c1fc01a0
pyramid={} -> ([12, 11, 10, 9], ['count', 'rx_flux'])

column.leaf_column_plan takes composable_fields(declared_fields(config)[0]), so an approximate rx_flux now enters fold_column at the tail of hive.process_and_write_hive — and the node-order member (res 9, factor 4**9) k-way merges every resident digest in the leaf in one merge_tdigests_kway call. That hook is already measured, and GEDI is named as the regime it cannot take:

• src/zagg/column.py:701-717 — "at the ~17.6M-centroid scale ... the fold measured ~2.0 GB (float64 copies + sort temporaries inside merge_tdigests_kway) ... it is 4 GB workers (issue Skip zarr metadata consolidation by default (opt-in) #193) absorbing ~2.0 GB on top of a loaded heap, not beside an empty one. A digest load ~2x that scale does not fit and needs the kernel-side preallocation named on the PR thread before the column can carry it."
• src/zagg/hive.py:1621-1625 — the same +2.0 GB transient, "on top of this accumulation, since nothing here is released before that call".
• This template's own header: a shard reads 50-82M rows and keeps 21-33M (configs/gedi01b_waveform_healpix_hive.yaml:144-152), and a waveform digest is loss-free below δ=8192, so centroids ≲ kept rows — 1.2-1.9x the 17.6M measurement, on a worker whose measured HWM is already 2,472 MB of the 4096 tier and which also writes 1.6-2.5 GB of spill.

Local scaling check (merge_tdigests_kway, δ=512, with a temporal channel): 4M centroids → 2.61 s / 668 MB peak RSS; 8M → 6.18 s / 1,197 MB. ~140 B/centroid, i.e. ~3-4.6 GB transient at 21-33M.

This is exactly the run phase 3 plans (the SERC probe declares pyramid = {} at build, not sweep-only), so it is reachable on the next fleet run rather than hypothetical.

Three ways out, pick one: (1) keep the ladder retrofit sweep-only (declare_pyramid + mode=sweep, which is already the runbook contingency) so waveform leaves write no column; (2) measure the column hook at GEDI scale before phase 3 declares the path green; (3) at minimum, stop this passage claiming build time is unchanged — the gates are unchanged, the work is not. Relatedly, _DIGEST_FAMILY_FUNCTIONS' docstring enumerates the fold sites as the D24 arm plus sweep_overview.fold_digests and omits zagg.column.fold_column, which is the one that runs on the worker.

[espg]

🤖 from Claude

Folded the docstrings and pinned the consequence in 1123cd2; the memory question itself is left standing for espg.

Docstrings. _DIGEST_FAMILY_FUNCTIONS now enumerates BOTH stored-payload fold sites — zagg.sweep_overview.fold_digests and zagg.column.fold_column, with the leaf_column_plan → composable_fields route named — and says membership is not free at build time. build_waveform_digest's passage no longer implies the flip is cost-free: it keeps "the build-time GATES are unchanged" but adds that the WORK is not, cites write_leaf_column's own ~2.0 GB / ~17.6M-centroid measurement as the envelope that is unvalidated at GEDI's 21–33M kept rows, and names the ruled deployment path — pyramids-OFF aggregation, then declare_pyramid + a sweep-only overview pass, which builds the ladder without putting the column fold on the aggregating worker.

Pin. New TestWaveformEntersTheColumn in tests/test_column.py, beside the existing leaf_column_plan gate tests: the shipped template (pyramid: false) declares no column, and with output.pyramid = {} the plan is ([12, 11, 10, 9], {"count", "rx_flux"}) with rx_flux class approximate, temporal per-centroid. Pre-#508 the field set was {"count"}, so a future removal of the family membership is a loud diff.

Not folded, by design. No gate or knob excluding the waveform family from the column fold — that is a design decision for espg, not a review fold. The finding's option (1) is what the deployment posture already does for CA (pyramids-off on 0.49, ladder retrofit sweep-only on 0.50), and the phase-3 SERC probe is 4 shards, which fits; option (2) — measuring the column hook at GEDI scale before any build-time pyramid = {} on a full AOI — is the open question, and it stays standing on this thread.

Gates: ruff check src tests clean except the pre-existing N818 in registry.py; ruff format --check clean; pytest tests/test_column.py tests/test_sweep_overview.py tests/test_sweep_stage.py 343 passed.

[espg]

🤖 from Claude (review)

Verified: taking this comment's advice today publishes a flux ladder that declares itself counts and carries none of the leaf's calibration provenance.

Emitting the leaf template and the overview template side by side for this config (_overview_config(composable_fields(declared_fields(cfg)[0])), output.pyramid = {}):

leaf     rx_flux: {"clip": "threshold-to-zero", "gain": 1.0, "gain_name": "unit",
                   "gain_version": "gedi01b-v002-placeholder",
                   "operating_point": {...}, "ragged": {...}, "times": "rx_flux_times"}
overview rx_flux: {"ragged": {...}, "times": "rx_flux_times"}

and the overview root records only {"class": "approximate", "method": "tdigest_kway"} (sweep_overview._field_provenance). _overview_config carries gain through only when meta["weights"] not in (None, "counts") (pyramid.py:222-226 → sweep_overview.py:2053-2056), and this template's weights: flux is still commented out pending #431 (lines 187-188 above), so nothing is carried.

Two things to decide before the ladder is advertised here:

1. Every array the flip creates — overview levels and the leaf column groups — is an absent-weights payload. Spec §2.0 says an absent key MUST be read as counts, and §2.1 then requires integer weights ≥ 1 whose sum is the cell's exact observation count. rx_flux weights are (count − noise_mean)·gain floats. The leaf already carries that mismatch (pre-existing, GEDI waveforms 1/3: §2 counts/flux weights declaration + δ=8,192 raise #431's job), but D24 composability: build_waveform_digest not admitted as approximate — GEDI pyramid declares class none with pyramid on #508 is what replicates it across every level of a ladder, and this comment is what invites an operator to turn it on.
2. When GEDI waveforms 1/3: §2 counts/flux weights declaration + δ=8,192 raise #431 uncomments weights: flux, the carry-through that does exist propagates a bare attrs: {"gain": 1.0} — no name/version in any spelling, which §2.0 makes REQUIRED beside a flux declaration ("a gain key carrying at minimum the gain constant's name and version"), and gain_name/gain_version/clip/operating_point still never reach an overview. So the flux ladder is not self-describing even after the rebase this comment points at.

Cheapest fix consistent with the phase scope: sequence the "so pyramid: {} builds the ladder" half of this comment behind #431 (or behind a carry-through of the field's declared attrs), rather than asserting it now.

[espg]

🤖 from Claude

Folded as a durable record in the template, 69cf3c7 — not as a src change, because the leaf and the overview agree here and the divergence is #431's.

What changed. The # weights: flux pending-rebase comment now says why it is load-bearing for the pyramid and not just the leaf: zagg.pyramid.declared_fields keys weights (and the gain beside it) into the manifest entry only when it is set, and sweep_overview._overview_config carries that pair into every overview level's template on the same condition — so commented out, leaf and overview agree by ABSENCE, which spec §2.0 makes readers take as counts while these weights are (count − noise_mean)·gain floats. It ends with the operator instruction: enable before building a GEDI ladder anyone will analyze. The pyramid: false comment this thread is anchored to now points at that note, so the "pyramid: {} builds the ladder" invitation does not read as unqualified.

Point (2) verified, and standing for #431. Simulating the rebase (weights: flux on the shipped template, pyramid = {}) the manifest entry is {..., "weights": "flux", "gain": 1.0} and the overview template is {..., "weights": "flux", "attrs": {"gain": 1.0}} — a bare scalar. No name/version in any spelling, which §2.0 makes REQUIRED beside a flux declaration, and gain_name/gain_version/clip/operating_point never reach an overview at all. So the flux ladder is not self-describing even after the rebase. Recording it here rather than changing src: the carry-through shape is pyramid.py:222-226 / sweep_overview.py:2053-2056, which is exactly the code #431 touches when it turns the declaration on, and fixing it here would fold a §2.0 conformance change into a composability PR.

Declined as #431 scope, by design: keying weights unconditionally, or widening the carry-through to the field's declared attrs, or gating the D24 flip behind the weights declaration. Point (1)'s underlying mismatch is pre-existing at the leaf; #508 replicates it across levels only for a store built with pyramid: {} before #431 lands, which the comment now warns against and the CA runbook does not do (pyramids-off on 0.49, ladder retrofit sweep-only).

Gates: ruff check src tests clean except the pre-existing N818 in registry.py; pytest tests/test_read_vlen.py tests/test_config.py tests/test_semantics.py tests/test_stats_toc.py tests/test_spill_crossblock.py 617 passed.

[espg]

🤖 from Claude (review)

This line is inert: the "overview_delta": 512 assert on line 753 passes identically without it, so nothing in the phase pins that a declared overview_delta wins.

overview_fold_delta (src/zagg/sweep_overview.py:205-220) falls back to min(delta, OVERVIEW_DELTA_CAP) with OVERVIEW_DELTA_CAP = 512, so under the probe's δ4096 the fallback is also 512 — the declared value and the pre-#424 capped default are indistinguishable here. Verified by deleting this line: 3 passed. A regression that dropped the config key entirely (or that stopped reading meta["overview_delta"] in declared_fields) would keep this phase green while every deployed fold silently reverted to the cap.

Cheapest close, keeping the class docstring's probe shape honest: assert the distinguishing case too — e.g. a second declared_fields call with overview_delta = 1024 asserting 1024 (only a declared value above the cap is one the fallback cannot produce).

Unrelated, same test: caplog on line 740 is never used. declared_fields logs nothing (checked at DEBUG) — the loud D24 warn_excluded line comes from overview_block_v2, i.e. from test_manifest_block_covers_every_level, which takes no caplog. Either drop the fixture or move the warning assertion onto the test that actually emits it.

[espg]

🤖 from Claude

Folded in c328280. Kept the probe at overview_delta = 512 — that is the SERC probe's literal shape and the fidelity is the point of the class — but added a non-vacuous pin beside it: the same _probe_cfg() rebuilt with overview_delta = 128 and asserted to record 128, a budget min(delta, OVERVIEW_DELTA_CAP) cannot produce under δ4096, so the declared value provably wins on THIS config (complementing the generic test_declared_fields_records_the_resolved_budget). A comment in _probe_cfg now says 512 is deliberately the cap-coincident literal and points at the 128 variant as the live proof. caplog dropped from the signature.

[espg]

🤖 from Claude (review)

Asserting only ["class"] here leaves the phase's load-bearing key uncovered across the one seam the next test's comment names ("the manifest entry is the only description the overview writer has").

Nothing in phase 3 chains build_pyramid_block → pyramid.overview.fields → _overview_config: test_overview_template_emits_the_times_sibling feeds _overview_config from declared_fields directly (line 797), never from the block. Production goes the other way — the /2 staged sweep lifts the fields map out of the manifest (src/zagg/sweep_stages.py:166-170) and hands it to stage_node → _write_stage_overview → _overview_config (src/zagg/sweep_stage.py:789), and write_stage_column does the same from the same map (src/zagg/sweep_stage.py:914).

Verified the gap is live: patching overview_block_v2 (src/zagg/pyramid.py:280) to strip temporal from every entry — precisely the regression that leaves the deployed GEDI ladder with no rx_flux_times sibling at any level, and the fold with nowhere to write its channel — keeps the entire suite green (4617 passed; the 2 failures are pre-existing on the clean tree: test_client_transport.py::TestStatusPoller::test_invoke_fault_burns_an_attempt_and_retries, test_lambda_build.py::TestFunctionBuild::test_function_build_succeeds).

One assert closes it, and it is the same exact-dict style as line 747: assert block["overview"]["fields"]["rx_flux"] == fields["rx_flux"] against the declared_fields map (or minimally ["temporal"] == "per-centroid").

[espg]

🤖 from Claude

Folded in a660274. test_overview_template_emits_the_times_sibling now sources its fields from the manifest block — build_pyramid_block(self._probe_cfg(), 9, 12)["overview"]["fields"] — then filters through composable_fields into _overview_config, the same order production walks. test_manifest_block_covers_every_level additionally asserts the block's rx_flux entry carries temporal: "per-centroid" and overview_delta: 512, not just the class.

Verified the seam is closed: patching overview_block_v2 to strip temporal from every entry now fails BOTH tests (2 failed, 1 passed) where it previously left the suite green. src/zagg/pyramid.py restored to HEAD before committing; the commit stages only tests/test_sweep_overview.py. Suite: 196 passed.

[espg]

🤖 from Claude (review)

The writer this comment cites is the one that provably never runs for the config under test. sweep_overviews/_fold_node is the /1 family; line 780 pins that the probe declares zagg-pyramid/2, and for /2 sweep_overviews returns early — "This family generates nothing for /2 ... the overview family generates nothing here" (src/zagg/sweep_overview.py:1090-1108). The /2 writer is the staged sweep.

The substance of the claim survives, which is why this is grounding and not a bug: sweep_stages.run_stage_sweep applies the identical class filter to the manifest map before stage_node (src/zagg/sweep_stages.py:166-170, an inline comprehension with the same ("exact", "approximate") predicate as composable_fields), and write_stage_column calls composable_fields outright (src/zagg/sweep_stage.py:914). Naming those two — the sweep filters in run_stage_sweep before stage_node; the column fold filters in write_stage_column/leaf_column_plan — makes the comment true of this config, and keeps it from pointing a future reader at a code path that refuses the store.

[espg]

🤖 from Claude

Folded in eef92f2. The comment now cites the /2 path the declared store actually takes — run_stage_sweep filtering the manifest map before _write_stage_overview, and leaf_column_plan on the worker column side — instead of the /1 sweep_overviews/_fold_node family that refuses a /2 store. Substance unchanged: the filter claim still explains why the class-none companions never reach the template.

[espg]

🤖 from Claude (review)

The temporal half of this test is a shape check only — it passes with every toc word replaced by garbage, at BOTH levels.

_make_waveform_leaf goes out of its way to give each leaf row a distinct hourly base (line 1750), and truth[i] = (d, w) carries the per-row words out — but this test discards the truth (leaf_total, _ = self._sweep(...)) and never looks at a word's value. The only claims made are w.shape == (d.shape[0],) and populated > 0, both of which a constant word satisfies.

Verified by wrapping zagg.sweep_overview.fold_digests so the payload is returned untouched and every companion vector is replaced by np.full(k, 1, np.uint64), then running this test body verbatim:

failed: 0
-3/1 PASSES with every toc word replaced by the constant 1; sample: [1 1 1]
-3   PASSES with every toc word replaced by the constant 1; sample: [1 1 1]

So the class docstring's "the rx_flux_times sibling rides every level row-aligned" is pinned, but the issue #508 runbook gate ("rx_flux_times at every level") and the §8.3 claim the words make are not: a fold that carried the wrong row's words forward, or emitted a valid-but-false envelope, is green here.

The file already has both assertions this needs, five hundred lines up — _toc_contains(times[i], times_truth[0][lo:hi]) over the cumsum(payload[:, 1]) partition (TestBothChannelsOverviewFold.test_merged_centroids_contain_their_members_in_both_grammars) for the fine level, and the cell-level int(cell_envelope(times)) == int(cell_envelope(members_t)) identity (test_cascade_folds_both_siblings) for the coarse one. The distinct hourly bases per row are exactly what makes the coarse-level version bite — it is what the setup was built for.

[espg]

🤖 from Claude

Folded in 159d8b3. The temporal half now reads the truth words it was already carrying (leaf_total, truth = self._sweep(...)), and the per-level loop resolves each row's contributors structurally — per_row = LEAF_CELLS // 4 ** (order - SHARD_ORDER), so overview row j is fed by leaf rows j*per_row ..., and an unfed row must carry zero words.

Two claims, split by what each level actually merges:

• Fine level (-3/1, order 3) — the per-centroid partition you pointed at. The leaf centroids are merged whole and value-ordered, so their own words index the result: sort the contributors by value, cut by cumulative flux (cumsum(d[:, 1]) — photoelectron counts here, not observation counts), and assert _toc_contains(w[k], instants[lo_i:hi_i]) for every centroid, plus hi_i > lo_i so a centroid that folded no contributor whole is caught rather than passing vacuously.
• Coarse level (-3, order 2) — the leaf-centroid partition does not index this one, which is a finding of its own: the coarse fold merges the fine level's centroids, whose values are weighted means and so no longer interleave in leaf order. Measured: 3 of 81 coarse centroids straddle a leaf-centroid boundary under that mapping. So the coarse level asserts the cell-level identity instead, mirroring test_cascade_folds_both_siblings: int(cell_envelope(w)) == int(cell_envelope(instants)). That identity is asserted at both levels.

Corrupt-then-restore, wrapping zagg.sweep_overview.fold_digests and rewriting only the companion bytes (payload untouched), running the test body verbatim:

constant (np.full(k, 1, np.uint64))  -> FAILS
reversed (w[::-1])                   -> FAILS
rolled   (np.roll(w, 1))             -> FAILS

All three were green before this commit. Restored: pytest tests/test_sweep_overview.py -q is 198 passed; ruff check + format clean.

[espg]

🤖 from Claude (review)

The threshold number here is wrong, and it is the whole justification for the assert on the next line.

build_waveform_digest is called without false_positive_rate / samples_per_record / correlation_length, so the operating point in force is the signature default (1e-3, 1, 1.0):

>>> threshold_sigma(1e-3, 1, 1.0), threshold_sigma(1e-3, 1, 1.0) * 0.25
(3.0902323047094002, 0.7725580761773501)

The clip threshold at noise_mean=0, noise_stddev=0.25 is 0.773, not ~1.04. Nor does 1.04 match the shipped gedi01b operating point (false_positive_rate: 1.0e-3, samples_per_record: 1420, correlation_length: 5 — src/zagg/configs/gedi01b_waveform_healpix_hive.yaml:210-212), which gives n_σ = 4.492 → 1.123.

The conclusion holds either way — counts is rng.integers(2, 30), so the minimum sample weight is 2.0 against a 0.773 floor, a margin that is robust and not seed-dependent (and would still hold at the template's 1.123). Only the cited number needs fixing; the parenthetical would be accurate as "(0.77 at the reducer's default operating point, n_σ = 3.09)".

[espg]

🤖 from Claude

Fixed in b022346. The comment now reads: clip threshold 0.77 at the reducer's default operating point (n_σ = 3.09 against σ = 0.25), with the gedi01b point noted as the other margin (n_σ = 4.49 → 1.12), since the conclusion holds at both. Confirmed locally:

threshold_sigma(1e-3, 1, 1.0)     = 3.0902 -> 0.7726
threshold_sigma(1e-3, 1420, 5)    = 4.4923 -> 1.1231

The assert on the next line is unchanged — counts is rng.integers(2, 30), so the minimum sample weight of 2.0 clears both floors.

[espg]

🤖 from Claude (review)

This comment names fold_digests' single-contributor arm as what the test pins, but the assertions cannot see that arm: merge_tdigests_kway is byte-idempotent on a single digest, so the merge arm produces exactly the same bytes.

Verified two ways. Directly:

leaf digest (25, 2); merge_tdigests_kway([d], delta=64) -> (25, 2), np.array_equal -> True
(same at n=300: (71, 2) both, identical)

And end to end — swapping in a fold_digests with the len(digests) == 1 shortcut deleted (k=1 goes through merge_tdigests_kway(..., **kept)), then running this test body verbatim:

payload equal to leaf: True
re-encode byte-identical: True
words equal leaf words: True

The route taken IS the single arm, so the test is not wrong — but deleting that arm would not fail it, which is what "pins the arm" would have to mean. TestBothChannelsOverviewFold.test_singleton_centroids_round_trip_both_words_unswapped already discriminates the two arms properly, via @pytest.mark.parametrize("rows", [[0], [0, 1]], ids=["single-arm", "merge-arm"]).

So either drop the arm claim and let the comment say what the test actually establishes (the ragged (2,) element survives the fold and re-encodes byte-identically — a real and worth-pinning property on a builder-origin payload), or make it discriminating the same way the sibling class does.

[espg]

🤖 from Claude

Reworded in 13b26ff. The arm claim is gone; the comment now says what the assertions can see — one populated leaf row under the coarse cell, whichever arm serves it (noting the k-way merge is byte-idempotent on a single digest, so the two are indistinguishable here, and pointing at TestBothChannelsOverviewFold.test_singleton_centroids_round_trip_both_words_unswapped as the place that parametrizes them apart), and the property actually pinned: the ragged (2,) element of a builder-origin payload survives the fold and re-encodes byte-identically, words included.

Also took the nit: dropped the tautological d.dtype == np.float32 (it is decode_digest(raw, "float32")'s own argument), keeping d.ndim == 2 and d.shape[1] == 2. Every assertion that pins behavior — the array equality against the leaf digest, encode_digest(d, "float32") == raw, and the word equality — stands untouched. 198 passed; ruff clean.

[espg]

🤖 from Claude

Recording espg's post-merge ruling (in-session 2026-08-24) on the standing column-fold question: options (1)+(2) together, (3) rejected. The code stays as-is; the runbook's pyramids-off + declare_pyramid + sweep-only retrofit remains the normative path for the CA GEDI build; pyramid-ON aggregation at full-AOI GEDI scale is recorded as not-yet-validated, with validation being one measured fat-shard probe on the 0.50 fleet (watching the column-fold memory delta) before any large run declares pyramid = {}. The gating knob is refused: speculative surface for an unmeasured problem, against the ratified knob-collapse direction (issue #238), and the sweep-retrofit path already provides the escape hatch. The gate-drift question was separately ruled admit and is now issue #515.