Skip to content

D24: admit build_tdigest_where to the digest family; honest composition declaration (issue #515) - #516

Merged
espg merged 30 commits into
mainfrom
claude/515-strata-composability
Aug 25, 2026
Merged

D24: admit build_tdigest_where to the digest family; honest composition declaration (issue #515)#516
espg merged 30 commits into
mainfrom
claude/515-strata-composability

Conversation

@espg

@espg espg commented Aug 24, 2026

Copy link
Copy Markdown
Member

Closes #515.

Status: all 4 phases complete, all 26 review findings folded or answered, CI fully green (37616292, after merging main to pick up PR #511's espg-ruled 30→32MB function-zip budget raise — the one test this branch had red). Ready for review.

Admits build_tdigest_where (the strata builder) to the D24 digest family, and gives the packed composition word an honest composability declaration — per espg's ruling (admit, 2026-08-24) recorded on the issue, and the plan comment (#515 plan).

The drift: both reducers are admitted by the spill fold gate (_TDIGEST_SPILL_FUNCTIONS, merge_composition_kway — issues #370/#321) and the published CA store's 2,726 shards were built by k-way merging per-block strata partials under exactly those laws — yet D24 classified every one of their fields {class: none}. Verified live: atl03_tdigest_o9.zarr/morton_hive.json declares h_tdigest_signal/h_tdigest_noise/composition all none (the manifest vendored in full as tests/data/ca_atl03_tdigest_o9_morton_hive.json).

Phases

  • Phase 1 — characterization (dd22f0ca). Pinned today's none classifications (strata + composition), the spill-gate admissions, and the LIVE CA-manifest declaration as an in-tree known-answer fixture (no network in tests). tests/test_strata_composability.py. Review findings folded in fd233463..d758a534.
  • Phase 2 — build_tdigest_where admission (2218b821). Added to _DIGEST_FAMILY_FUNCTIONS (streaming/spill gate tuples untouched — those re-run builders and keep their exact members); k-way parity tests for the strata payload and both companions (located → common-ancestor words, temporal → toc envelope) through fold_digests, plus order-independence; the shipped strata template + output.pyramid = {} now declares {class: approximate, method: tdigest_kway, delta: 8192, overview_delta: 512, location: leaf_id} for both strata fields.
  • Phase 3 — composition's honest declaration (28ddd8f1). New D24 class packed (decision below): classifier arm in semantics.field_composability, manifest entry in pyramid.declared_fields (method: composition_kway, dense dtype/fill, the §3.3 of linkage + threshold), and the fold law wired at every stored-payload fold site — sweep_overview._fold_node (leaves), _fold_child (cascade), column.fold_column (leaf column), sweep_stage._merge_slabs (stage merge; gathers assign gen-1 bytes unchanged) — each pairing a contributor's word with its of digest's weight at the same cell and collapsing per output cell in ONE merge_composition_kway call. Spec §3.4/§4.3/§4.4/§4.5 updated and the kitchen_sink conformance fixture regenerated in the same commit (Promote zagg-ragged/1 + digest + composition byte layouts to a normative spec (reader-migration gate) #340 rule).
  • Phase 4 — end to end (a73b64b6). Synthetic strata store (shard order 2 / cell order 4, one committed leaf): template → build → sweep_overviews /1 cascade regime — the finest level exact-from-leaves (_fold_node), the coarser a cascade (_cascade_node). At every level: both strata's weight_total folds exactly; the composition word equals the k-way merge of its contributors' (word, n) pairs, recomputed independently from the arrays one level finer; presence exact vs one direct pooled pack of every raw row; the overview composition array carries the writer-stamped §3.3 attrs block; zagg_overview.fields records {class: packed, method: composition_kway} at both levels. The CA-manifest before/after known-answer is the phase-1 fixture + the rebuild test (test_declaration_writer_rebuilds_the_published_block).

Composition-declaration decision

A third class token packed (method composition_kway) — not an extension of the exact arm.

  • Honesty. D24's exact normatively means "folds byte-equal to a direct aggregation at the coarser order" (spec §4.4). merge_composition_kway is deterministic and order-independent within one fold call (single quantization), but it is not byte-equal to a direct pack over the raw rows, and a cascade re-quantizes once per level — presence exact through the chain, counts within one lane quantization per fold (spec §3.4). Declaring that exact would recreate the exact dishonesty the issue exists to end; declaring it approximate binds readers to a digest-shaped entry (ragged decode, inner_shape, delta) that a dense uint64 word does not have.
  • Fail-safe degradation. Every deployed fold site gates on class in ("exact", "approximate"), so pre-D24: admit build_tdigest_where to the digest family; give composition an honest composability declaration #515 zagg (and any external reader) treats a packed entry as non-composable — native-resolution reads, exactly today's behavior. Extending the exact arm instead would send old sweeps into fold_dense with an unknown law: a fold-time crash rather than a graceful skip. Spec §4.5 now states the unknown-token posture explicitly.
  • Two-field law. The packed fold needs a divisor from a second field — the of digest's per-cell weights (N_signal, spec §3.3). Neither existing arm's machinery carries a cross-field law; a distinct class makes the pairing explicit at every fold site (word and divisor are read together, one missing half contributes nothing / counts the contributor unreadable) instead of special-casing "exact-but-not-really" everywhere.
  • Guard rail. declared_fields writes packed only when the of digest is itself declared approximate in the same map, and demotes to {class: none} otherwise — the law divides by that digest's weight at every level, so composition folds only where its divisor does.

Wire-format consequence, handled per the #340 rule in the same commit: spec §3.4 (k-way form + composability), §4.5 (the packed entry grammar), §4.3/§4.4 (cascade materiality, inclusion gate), and the kitchen_sink conformance fixture regenerated — its manifest now declares strata approximate + composition packed, and its all.pyramid.zarr leaf column carries the strata, locations and composition groups through the real fold path.

Fixture-regen note: kitchen_sink's two *_locations/zarr.json siblings were deliberately restored after regeneration (git checkout --) — they are the committed pre-§9 absent-located pin (TestLocatedDeclaration::test_absent_declaration_on_the_pre_section_9_fixture); a pristine-main regen also rewrites them, so that churn is staleness, not this PR. The generator docstring's staleness notes were updated to say exactly this.

Deployment note

Classification is template-time worker-side — the admission takes effect on the 0.50 fleet. The CA retrofit uses the leaf-reading path (declare_pyramid + cascade-from-leaves): the strata leaf columns were never written (class none at build time), so the retrofit re-reads leaves; no re-aggregation. semantic_hash is a frozen manifest key, so the retrofit's manifest write needs the declare-pyramid path (which updates the pyramid block only), never a manifest rebuild.

Adversarial review record

Three fresh-context review rounds (phase 1; phases 2–3; phase 4 + folds), 26 inline findings total (1 blocking — a fourth class-filter site sweep_stages.py missed by an earlier fold, fixed in 484fd36d), all folded or answered on-thread: 22 folded as fold review: commits, 4 answered leave-by-design/superseded with rationale. Notable folds beyond the findings' letter: half-pair poisoning now fires only in the true skew direction (word absent while the divisor folds) at both _fold_node and _merge_slabs, with mutation checks confirming each direction's test bites; the e2e store gained a second leaf so a cascade fold-window off-by-one is caught.

Testing

  • tests/test_strata_composability.py (phases 1–4: characterization → parity → end-to-end, 27 tests) + updated pins in tests/test_sweep_overview.py; conformance suite green against the regenerated fixture. Final full run: 4762 passed, 38 skipped (docker-only build test deselected locally).
  • test_lambda_build::TestFunctionBuild::test_function_build_succeeds is environmental on this machine (docker daemon not running; slow-marked build test), unrelated — deselected locally, runs on CI.
  • CI Tests is red on one known gate, not on this PR's logic: test_lambda_build::TestFunctionBuild::test_function_build_size — the function zip sits at exactly 30.0MB against the 30MB budget (phase 3's ~200 src lines nudged it over the line). The budget raise to 32MB is espg-ruled and lands on PR closest-observation Sentinel-2 ingest builder (issue #509) #511; per §1 this PR does not touch the gate. Resolves when closest-observation Sentinel-2 ingest builder (issue #509) #511 merges — will re-verify then. Every other job (Lint, Lambda Build, Docs) is green, and the pytest failure count on the Tests job is exactly this one test (4751 passed).
  • Pre-existing on main, not touched (per §4: don't fix unrelated): ruff N818 on src/zagg/registry.py::UnknownCapability; ruff format --check flag on tests/data/benchmark/README.md. The PR lint bot selects E,F,W,I only.

Questions for review

  • Module size (§4 note, not a raise-request for this PR): sweep_overview.py (2,403 lines) and sweep_stage.py (1,408) were already past the 1,200-line trigger on main before this PR; the packed fold adds ~130 lines across the two. Flagging per §4 rather than splitting mid-PR — if a split is wanted, it should be its own issue.

  • Artifact-visible poison record — when _fold_node poisons a half-paired leaf's composition cells (absence over a skewed word), the trace is a logger.warning only; an artifact-visible record would need a new §4.3 attrs key (a source_children analog for the /1 leaf fold) plus a fixture regen. Left standing as a scope decision rather than invented here. (_merge_slabs' poisoning IS recorded via source_children.unreadable.)

  • Config-side cross-check of a flux-weighted divisordeclared_fields demotes a packed field whose of digest declares flux weights, but _validate_composition_attrs does not refuse the pairing at config load (doing so would break configs legal today with no pyramid declared). Left standing.

  • Stage gathers and the packed pair — a /2 gather level assigns gen-1 bytes per field independently (_gather_slabs), so a child column carrying composition without its of digest (only possible via schema evolution or a hand-edited manifest) would gather the word against a fill divisor. The merge sites treat that half-pair as unreadable; the gather keeps its assign-only contract. Flagged rather than changed — tightening the gather to pair-check would break its "gen-1 content untouched" acceptance contract.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014eRcohZWarXGLrNMsLXD4b
@espg espg added the implement label Aug 24, 2026
manifest = json.loads(CA_MANIFEST.read_text())
pyramid = manifest["pyramid"]
assert pyramid["spec"] == "zagg-pyramid/2"
assert [e["node"] for e in pyramid["overviews"]] == list(range(9, -1, -1))

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.

@f1.md

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

(The review bodies on all six threads posted as the literal text @f1.md@f6.mdgh api -f does not expand @file, -F does. Restating each finding in one line so the threads stay readable; the folds below are against the findings as written.)

Finding 1 — the known-answer stops one assertion short: the ladder/fold_source/exact_levels are asserted against the static fixture, never against code. Folded in fd23346.

TestLiveCaManifestFixture.test_declaration_writer_rebuilds_the_published_block now runs the writer and diffs it against the published block:

config = default_config("atl03_tdigest_strata_healpix")
config.output["pyramid"] = {}            # the template ships pyramid: false
config.output["grid"]["parent_order"] = 9  # the CA store's shard order
block = build_pyramid_block(config, 9, chunk_order=13)

spec, overviews (the 9..0 ladder with its per-node cells) and the whole overview block minus fields compare equal to the fixture; the fields map is then asserted to differ ONLY in the two ruled entries — count byte-identical, composition still {"class": "none"} until phase 3, h_tdigest_signal/h_tdigest_noise the phase-2 admission (class: approximate). Phrased that way because phase 2 (2218b82) already flipped the declared classes, so a flat equality against the fixture is no longer the right pin.

assert (
manifest["semantic_hash"]
== "b9b15fdde78f147c15c929da8ca93de21930ad5c552ae082c5d8998fb83ada21"
)

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.

@f2.md

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

Finding 2 — the identity story overclaims: no in-tree config reproduces the live store's semantic core, and semantic_hash is a frozen manifest key. Folded as a comment correction in e596245 (no attempt to reproduce the hash).

Re-measured on the current tree, matching the finding exactly: fixture b9b15fdd…, default_config("atl03_tdigest_strata_healpix")0ac7d33b…, tests/data/benchmark/configs/atl03_tdigest_healpix_o9_hive_strata.yaml5ebf740f…. The comment above the semantic_hash assert now says the hash names the record, not a config; that no config in the tree reproduces it (both divergent hashes named by prefix); that the before/after is pinned against reality for the per-field declaration only; and that it is a frozen key (hive._FROZEN_MANIFEST_KEYS), so the retrofit write the PR body plans against that root needs overwrite=True or ensure_manifest refuses it.

The specific sentence the finding quoted ("the manifest fixture above is what this code writes…") no longer exists — phase 2 (2218b82) rewrote that comment when the test became test_declared_fields_no_longer_reproduce_the_live_ca_declaration, so the correction landed on the surviving identity claim instead.

On the provenance nit: the fixture's top-level keys are exactly build_manifest's /1 output (spec, dataset, semantic_hash, cell_order, shard_order, split_schedule, path_grouping, pyramid, generated_at) with nothing elided, so the module docstring's "byte-for-byte" is right and the PR body's "fragment" is the wrong word. Left standing for the PR body — out of scope for a test-file fold.

# admits the reducer by name in its scalar branch.
from zagg.processing.streaming import _COMPOSITION_FUNCTION

assert _COMPOSITION_FUNCTION == "zagg.stats.composition.pack_composition"

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.

@f3.md

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

Finding 3 — test_composition_is_spill_admitted pins a string constant, so the option (b) flip (removing the name from the validate_spill_fold scalar branch) leaves it green. Folded in 0d9cde1.

Kept the test rather than dropping it, and made it assert the branch as a pass/raise pair on one config:

config = default_config("atl03_tdigest_strata_healpix")
fields = get_agg_fields(config)
assert fields["composition"]["function"] == _COMPOSITION_FUNCTION
validate_spill_fold(config)
fields["composition"]["function"] = "zagg.stats.composition.pack_composition_v2"
with pytest.raises(ValueError, match="scalar function .* has no cross-block fold"):
    validate_spill_fold(config)

The positive call now fails under option (b) (the shipped composition field is scalar pack_composition), and the negative control shows the pass is the by-name admission rather than a branch that waves every scalar through. test_strata_template_passes_the_spill_gate keeps carrying the shipped shape, and tests/test_spill_crossblock.py::test_strata_and_composition_config_is_mergeable still carries the deep path — this file's job is only that its characterization cannot stay green through the flip.

Comment thread tests/test_strata_composability.py Outdated
classes = composability_classes(default_config("atl03_tdigest_strata_healpix"))
assert classes["count"] == "exact"
assert classes["h_tdigest_signal"] == "none"
assert classes["h_tdigest_noise"] == "none"

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.

@f4.md

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

Finding 4 — nothing records WHY the strata fields classify none, so phase 2's flip would not be provably the registry admission. Left standing as superseded by phase 2 (2218b82) — no residual gap I can find worth another assert.

Both halves the finding asked for landed with the admission itself:

  • the exact line phase 2 flipstests/test_sweep_overview.py::TestComposabilityClasses::test_digest_family_registry_members_pinned_by_value pins _DIGEST_FAMILY_FUNCTIONS by value, now (*_TDIGEST_FUNCTIONS, "zagg.stats.waveform.build_waveform_digest", "zagg.stats.tdigest.build_tdigest_where"), with the other two tuples asserted unchanged. Registry membership is the asserted mechanism, not a side effect.
  • the classifier preconditionstest_where_strata_template_classifies_approximate (renamed from …_classifies_none_today) now drives field_composability on a meta directly and asserts the guard still bites: inner_shape: [2]approximate, inner_shape: [3]none on the same function. And the template's own shape is pinned by value in tests/test_strata_composability.py::TestD24Classification::test_declared_fields_no_longer_reproduce_the_live_ca_declaration, which asserts the full declared entry for both strata fields — inner_shape: [2], delta: 8192, overview_delta: 512, location: "leaf_id". A template edit dropping location: leaf_id or mis-declaring inner_shape fails there, so the "green for the wrong reason" path the finding describes is closed.

Adding _TDIGEST_WHERE_FUNCTION in _DIGEST_FAMILY_FUNCTIONS here as well would be a third copy of the registry pin, so leaving it.

"h_tdigest_signal": {"class": "none"},
"h_tdigest_noise": {"class": "none"},
"composition": {"class": "none"},
}

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.

@f5.md

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

Finding 5 — the class map gates a write path (column.composable_fieldsleaf_column_plan/fold_column), and that half is unpinned even though the retrofit plan rests on it. Folded in c32fa62.

TestD24Classification.test_class_map_gates_the_leaf_column_write_path pins both sides of the flip:

assert composable_fields(CA_FIELDS_BEFORE) == {"count": CA_FIELDS_BEFORE["count"]}
fields, _ = declared_fields(default_config("atl03_tdigest_strata_healpix"))
assert sorted(composable_fields(fields)) == ["count", "h_tdigest_noise", "h_tdigest_signal"]

The first line is the published store's before — only count survives the filter, which is why the CA store has no strata leaf columns to fold and the retrofit must re-read leaves. The second is the behavior change phase 2 carries beyond the manifest text: the admission turns worker-side strata column writes on for new runs, and a later change that widens or narrows what the column path carries now shows up as a red test rather than silently on the fleet.

Comment thread tests/test_strata_composability.py Outdated
"dtype": "uint64",
"fill_value": 0,
"params": {"threshold": 2},
"attrs": {"composition": {"of": "h_tdigest_signal", "threshold": 2}},

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.

@f6.md

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

Finding 6 — the composition meta is hand-built and does not match the shipped field, and it re-pins what the template test already covers. Folded in d758a53 — driven off the template, as suggested.

test_composition_field_classifies_none_today (phase 2 renamed the class to TestD24Classification) now reads:

meta = get_agg_fields(default_config("atl03_tdigest_strata_healpix"))["composition"]
assert meta["function"] == "zagg.stats.composition.pack_composition"
assert field_composability(meta) == "none"

So the fabricated params: {"threshold": 2} / two-key attrs are gone and the verdict follows the shipped field: a declaration the template gains (temporal, location, a params change) moves this test rather than leaving a green pin over a shape no store writes. Kept rather than dropped because it names the reducer that drives the arm, which test_strata_template_classes does not.

espg and others added 8 commits August 24, 2026 14:17
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014eRcohZWarXGLrNMsLXD4b
… meta (issue #515)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014eRcohZWarXGLrNMsLXD4b
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014eRcohZWarXGLrNMsLXD4b
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014eRcohZWarXGLrNMsLXD4b
Comment thread src/zagg/pyramid.py Outdated
elif cls == "packed":
block = (meta.get("attrs") or {}).get("composition") or {}
of = block.get("of")
if classes.get(of) != "approximate":

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)

[non-blocking] The packed guard rail checks the of digest's class but not its weights declaration, and the law only holds for weights: counts.

merge_composition_kway divides by n, and payload_weight (sweep_overview.py:226) rounds the of digest's summed centroid weights to an int — its docstring justifies that with "strata weights are exact stratum photon counts (spec §2)". Nothing enforces it. A config whose attrs.composition.of names a §2.0 weights: flux digest passes every gate here: field_composability returns approximate for it (weights are not consulted), classes.get(of) == "approximate" here, and _validate_composition_attrs (config.py:894-908) only checks of != name and kind == "ragged". The result is a packed declaration whose divisor is a flux sum silently truncated to an integer at every fold site — wrong-but-plausible lanes, exactly the failure mode §2.0/issue #424 introduced the weights gate to stop (check_weights_match is not reachable on this pairing: the packed arms in _fold_node/_fold_child never call it).

Suggested fix: tighten the demotion predicate to the pairing the law actually needs, e.g.

of_meta = agg.get(of) or {}
if classes.get(of) != "approximate" or (of_meta.get("weights") or "counts") != "counts":

and/or add the same rule to _validate_composition_attrs so it fails at template time with a named error rather than demoting silently.

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 3367b7f — the demotion predicate now carries the weights half.

declared_fields' packed arm (src/zagg/pyramid.py):

of_weights = (agg.get(of) or {}).get("weights") if of else None
if classes.get(of) != "approximate" or of_weights not in (None, "counts"):

Took the weights off the SOURCE meta (agg, i.e. get_agg_fields) rather than off the built fields entry, because the manifest entry keys weights only when non-default — reading the source is the same test without depending on iteration order. Absent-or-"counts" is the pairing the law admits, matching how the fold gate itself reads the key.

Test beside the existing one: test_declaration_demotes_packed_over_a_flux_weighted_divisor drives one config twice — the counts control still declares packed/of: h_sig, and the flux variant (weights: flux + the §2.0 gain provenance the config gate requires) leaves h_sig folding as approximate while composition becomes {"class": "none"} and lands in excluded.

Left the _validate_composition_attrs half of the suggestion standing: refusing the pairing at TEMPLATE time would make a config that is legal today (a flux digest plus a composition field, no pyramid declared) fail to load, which is a scope change rather than a fix — the declaration is where this PR's honesty contract lives, and it now refuses. Flagging it here for @espg rather than acting on it.

Comment thread src/zagg/pyramid.py
of = block.get("of")
if classes.get(of) != "approximate":
fields[name] = {"class": "none"}
excluded.append(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 (review)

[non-blocking] The demotion has no diagnostic of its own — the operator gets a message that names the wrong cause.

When this arm fires, the only output is warn_excluded's generic line (pyramid.py:273-277): "fields ['composition'] are non-composable (D24 class 'none') and will exist ONLY at native resolution … declare a derived summary to opt in, issue #201". That is false for this path: the field is classified packed by field_composability; it was demoted because its divisor is not approximate. An operator reading that warning will go looking at the composition reducer, not at attrs.composition.of's target — and in the common case the of digest is itself in excluded right beside it, so the actual root cause is one line away but never stated.

This matters most for exactly the run the PR is aimed at: the CA retrofit's value is the strata pyramid plus its composition companion. If a future template change knocks h_tdigest_signal out of approximate (a non-(2,) inner_shape, a per-cell temporal companion, an expression), composition drops out silently with a misattributed warning.

Suggested fix: log at the demotion site before continue, e.g.

logger.warning(
    f"pyramid: field {name!r} classifies 'packed' but its attrs.composition.of "
    f"{of!r} is declared {classes.get(of)!r}, not 'approximate' — the fold divides "
    f"by that digest's weight at every level, so composition is demoted to "
    f"'none' (issue #515)"
)

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 89f7776 — the demotion now logs its own cause before continue:

pyramid: field 'composition' classifies 'packed' but its attrs.composition.of
'h_sig' is declared 'approximate'/weights='flux', not approximate/counts — the
fold divides by that digest's per-cell weight at every level (spec §3.3/§3.4),
so 'composition' is demoted to 'none' and exists ONLY at native resolution

Both demotion causes are in one line (the of class AND its §2.0 weights, per the sibling finding), so the operator reads the actual root cause rather than warn_excluded's "non-composable (D24 class 'none')" — which stays, since the field genuinely ends up excluded, but is no longer the only trace.

Pinned by the test in the same commit: test_declaration_demotes_packed_over_a_flux_weighted_divisor asserts the record names attrs.composition.of 'h_sig' and weights='flux' under caplog, so a future edit that drops the cause from the message fails rather than silently reverting to the misattributed warning.

#: through arbitrary chains and counts stay within one lane quantization per
#: fold — deterministic, but NOT byte-equal to a direct aggregation, which is
#: why ``packed`` is its own class rather than an ``exact`` method.
COMPOSITION_LAW = "composition_kway"

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)

[non-blocking] _field_provenance still defaults an absent method to TDIGEST_LAW, so a packed entry without method gets stamped into the artifact as tdigest_kway.

_field_provenance (sweep_overview.py:1408-ish, unchanged by this PR) is:

entry = {"class": meta["class"], "method": meta.get("method", TDIGEST_LAW)}

Verified on this head:

>>> _field_provenance({"class": "packed"})
{'class': 'packed', 'method': 'tdigest_kway'}

declared_fields always writes method today, so this is unreachable from zagg's own template path — but _field_provenance consumes a manifest entry (_write_overview's zagg_overview.fields, and _column_provenancezagg_column.fields), and manifests outlive the writer that produced them and can come from external writers (moczarr is meant to decode from the spec alone, per #340). The failure is the exact class of dishonesty this PR exists to remove: an overview whose own provenance attrs claim a t-digest law over a dense uint64 word.

Suggested fix: key the default off the class now that there is more than one non-exact law, e.g.

default = COMPOSITION_LAW if meta["class"] == "packed" else TDIGEST_LAW
entry = {"class": meta["class"], "method": meta.get("method", default)}

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 fa41e59 — the default is keyed off the class, exactly as suggested:

default = COMPOSITION_LAW if meta["class"] == "packed" else TDIGEST_LAW
entry = {"class": meta["class"], "method": meta.get("method", default)}

Docstring says why in the terms the finding uses: declared_fields always writes method, but _field_provenance consumes a MANIFEST, and manifests outlive their writer and may come from an external one (§4.5) — so a packed entry must not have the artifact's own provenance claim a t-digest law over a dense composition word.

Test: test_provenance_defaults_the_method_by_class pins {"class": "packed"}composition_kway, the approximate default unchanged, and an explicit method still winning over the class default.

f"{sib_element.get('shape')!r}; the channel is one flat uint64 "
f"word per centroid row (spec §8.3/§9, §1.1)"
)
elif meta["class"] == "packed":

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)

[non-blocking] The packed drift arm checks dtype and the of binding but not the stored spec/lanes — the two things that decide whether the words can be merged at all.

Compare the approximate arm directly above: it strict-checks the stored ragged element dtype and inner shape, the §2.0 weights declaration, every companion binding, and the sibling's element dtype/shape — precisely because declare_pyramid must not install a declaration the store contradicts. The packed arm stops at dtype + of.

merge_composition_kway unpacks byte lane i as LANES[i] unconditionally. A leaf whose attrs.composition carries a different spec (a future zagg-composition/2) or a permuted/truncated lanes list would be folded lane-wise under this writer's constants and produce a word that is well-formed and wrong — no error anywhere. Config validation guards this on the write side (config.py:869-883 rejects a disagreeing spec/lanes), which is exactly the argument for guarding it here on the read side too: the whole point of _field_drift is the store that a later declaration does not describe.

Secondary, same block: a leaf array with no composition attrs at all passes silently (stored_of is not None short-circuits), where the approximate arm returns drift when the ragged block is absent.

Suggested fix, on the array this probe already opened:

from zagg.stats.composition import COMPOSITION_ATTR, COMPOSITION_SPEC, LANES
stored = dict(arr.attrs.get(COMPOSITION_ATTR) or {})
if stored.get("spec") not in (None, COMPOSITION_SPEC):
    return f"field {name!r}: stored composition spec {stored['spec']!r} != {COMPOSITION_SPEC!r}"
if stored.get("lanes") is not None and list(stored["lanes"]) != list(LANES):
    return f"field {name!r}: stored composition lanes are not the {COMPOSITION_SPEC} order"

(and use COMPOSITION_ATTR rather than the literal "composition" on line 1041, for the same single-source reason the module states in its own docstring).

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 86c973d — the packed arm now checks the stored block, and both halves of the finding landed.

stored = dict(arr.attrs.get(COMPOSITION_ATTR) or {})
if not stored:
    return f"field {name!r}: declared packed but the stored array carries no {COMPOSITION_ATTR!r} attrs block (spec §3.3)"
if stored.get("spec") != COMPOSITION_SPEC:
    ...  # "this zagg folds 'zagg-composition/1' only (lanes are merged by position)"
if [str(lane) for lane in stored.get("lanes") or ()] != list(LANES):
    ...  # "are not the zagg-composition/1 order (spec §3.1)"

Notes on the two places I tightened past the suggestion:

  • spec is compared strictly (!= COMPOSITION_SPEC), not not in (None, COMPOSITION_SPEC) — the absent case is already refused one line above by the no-block check, so a present block with no spec is a malformed block rather than a pre-declaration store. This mirrors the approximate arm, which refuses a ragged block whose spec is not RAGGED_SPEC outright;
  • lanes likewise: the block is written by composition_attrs_block, which always stamps both keys, so a present-but-lane-less block is not a compatibility case.

COMPOSITION_ATTR replaces the literal "composition" on the of read, per the same single-source point. Test: test_packed_drift_checks_the_stored_composition_block builds the synthetic strata leaf, asserts the clean store is not drift, then mutates the block four ways (foreign spec, reversed lanes, rebound of, deleted block) and asserts each reports.

# writer's constants (``grids.base.apply_field_attrs``), while the
# per-product ``of``/``threshold`` halves ride the manifest entry,
# which is the only description the overview writer has.
block = {"of": meta.get("of")}

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)

[non-blocking] A packed entry with no of stamps "of": null into every overview level's §3.3 attrs block, and the sweep then publishes an all-fill composition array — while fold_column raises on the same declaration.

Verified on this head:

>>> _overview_config({"composition": {"class": "packed", "dtype": "uint64", "fill_value": 0}})
... 'attrs': {'composition': {'of': None}}

declared_fields never writes that (the classifier requires a truthy of), so this is only reachable from a hand-edited or foreign manifest — but the postures then diverge three ways for one malformed entry:

  • sweep_overviews' filter (sweep_overview.py:1165) admits it on class alone → template stamps of: null_fold_node/_fold_child hit group[None]TypeErrorcontinue (debug only) → every level publishes a fill-0 composition array under an invalid §3.3 declaration. Per §3, fill 0 means "empty signal stratum", so the artifact positively asserts no flags anywhere rather than recording absence;
  • fold_column (column.py:284-289) raises by name;
  • _merge_slabs (sweep_stage.py:681-693) reads reader.read(res_src, None), whose behavior depends on the reader.

Suggested fix: make the entry filters carry the same requirement the classifier does — m.get("class") != "packed" or m.get("of") in sweep_overviews, composable_fields, and sweep_stage_pass — so a linkage-less packed entry is treated as non-composable (§4.5's stated unknown/unusable-declaration posture) instead of publishing a null-of array. At minimum, omit the attrs block here when of is None rather than writing null into the store.

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 9651e97 — took the first option (make the filters carry the classifier's requirement) rather than the "omit the attrs block" fallback: a level that publishes a composition array with no §3.3 declaration is a second malformed artifact, not a fix.

The requirement is now one predicate, shared:

def _is_composable(meta: dict) -> bool:
    cls = meta.get("class")
    if cls == "packed":
        return bool(meta.get("of"))
    return cls in ("exact", "approximate")

column.composable_fields and sweep_overview.sweep_overviews' own filter both go through it, which also closes the third site named in the finding: sweep_stage_pass filters through composable_fields (src/zagg/sweep_stage.py:955) before any of _gather_slabs/_merge_slabs/_write_stage_column sees fields, so the stage path inherits the same admission rather than needing its own copy. A linkage-less packed entry now degrades to native resolution exactly as an unknown class does, and fold_column's by-name raise stays as the last line for a caller that hands it an unfiltered map.

Test: test_a_packed_entry_without_its_of_linkage_is_not_composable strips of from the shipped strata declaration and asserts it drops out of composable_fields while the intact entry passes.

Comment thread src/zagg/sweep_stage.py
if (words is None) != (of_values is None):
logger.warning(
f"stage sweep: column {reader.path} carries only one of "
f"{name!r}/{of_name!r} at resolution {res_src}; counting the "

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)

[non-blocking] Skipping a half-pair contributor here does not keep the pair consistent: the of digest folds that contributor anyway, so the published word's fractions are over a strict subset of the N_signal a reader will divide by.

This is not the located-pair situation the comment cites. There, both halves are the same field's payload+sibling, so refusing one refuses both and the output stays self-consistent. Here the divisor is a different declared field, folded independently in the approximate loop above (lines 597-664), which has its own _companion_group gate and knows nothing about this one. Reproduced on this head (4 children, factor=4, child 1 carrying h_sig but not composition):

half-pair comp: [372]   counts folded=3 missing=0 unreadable=1
h_sig weight at the output cell: 18.0     # 3+4+5+6 — all four children

The word's lanes are Σ nᵢ·laneᵢ / 14; the level's N_signal is 18. A reader doing the §3.3 recovery counts_from_composition(word, N_signal) — which §3.4's new text explicitly licenses ("its N_signal at every level is the folded of digest's total weight at that level") — is off by 18/14, ~29% here. Presence stays exact, so nothing looks wrong. source_children.unreadable records that the artifact folded short, but it is per-child, not per-field, so it cannot tell a reader which denominator is the stale one.

The same skew arises in _fold_node (a leaf missing the composition array but carrying the digest: the packed arm never records it, the digest arm folds it) and in the /2 gather the PR body flags under "Questions for review" — but the gather already has this posture available: _gather_slabs pair-checks companions and does broken.add((i, 0)) + skip |= {...} for both halves (sweep_stage.py:509-516), so "tightening the gather would break its gen-1-untouched contract" understates what the gather already does.

Suggested fix (either is fine, but one of them should land): (a) make the skew normative in spec §3.4/§4.5 — a level's composition fractions are over the contributors that folded, which may be a subset of the of digest's contributors whenever source_children.unreadable/missing is nonzero, so N_signal recovery is exact only on a fully-covered level; or (b) record the folded weight beside the word (per output cell, Σ n of the parts actually merged) so the pair is self-describing. (a) is the cheap one and matches the PR's honesty framing.

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 69a8e75. Neither (a) nor (b) — a third option, which I think is stronger than making the skew normative: poison the affected output cells. Absence over wrongness. A cell whose word would be over a strict subset of the level's N_signal keeps the fill word 0, which makes no §3.2 presence/fraction claim, instead of publishing a plausible word that a §3.3 recovery divides wrongly. The of digest is untouched and stays correct on its own, and source_children.unreadable still records that the level folded short.

_merge_slabs (src/zagg/sweep_stage.py):

poisoned: set[int] = set()
...
        broken.add((i, w))
        poisoned.update(range(base // factor, (base + src_per_child + factor - 1) // factor))
...
for j, parts in parts_by_cell.items():
    if j not in poisoned:
        out[j] = merge_composition_kway(parts)

Poisoning drops cells AFTER accumulation, so every surviving cell still folds its parts in one k-way call — single quantization is preserved. The rule is keyed on the PAIR rather than on the skew direction, because a contributor's windows all land on the same output cells: one window's half-pair would otherwise mix into cells another window filled.

_fold_node gets the same treatment (packed_poison, per field, applied only once the leaf has read cleanly — an unreadable leaf is skipped whole, so the digest excludes it too and nothing needs poisoning). Its two half-pair paths are the word-absent one (poisoned only when the of array IS present — a leaf carrying neither half is ordinary schema evolution, nothing folds from it either way) and the of-absent one.

On _fold_child: verified, no change needed, and the invariant is now stated in the code. _cascade_node ASSIGNS each child's partials into a disjoint span (slabs[name][start : start + span] = partial, sweep_overview.py:1884-1886), so a child skipped for the field leaves its whole span at the fill word — no output cell can mix that child's absence with a sibling's contribution. That is the poisoned posture already.

Tests (both fail without the poisoning, checked by reverting the guard):

  • TestStageMergeHalfPair::test_a_half_paired_contributor_poisons_its_output_cells — two real written columns merged into ONE output cell (src_per_child=4, factor=8, n_out=1), control asserts the word is the k-way merge of all eight source cells; then one column loses its composition member and the assertions are (folded, missing, unreadable) == (1, 0, 1), composition[0] == 0, and h_sig's weight at that cell UNCHANGED — the divisor still folds both, which is precisely why the word may not be published;
  • TestLeafFoldHalfPair::test_a_half_paired_leaf_poisons_its_output_cells — the same shape at _fold_node, at the geometry where two leaves land in one output cell (shard order 3, cell order 4, k=1 ⇒ target_order < shard_order).

Comment thread src/zagg/sweep_stage.py Outdated
for w, reader in enumerate(row or ()):
if not _is_reader(reader):
continue
words = reader.read(res_src, 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 (review)

[non-blocking] words here shadows the digest loop's words dict, which is a free variable of the _close closure defined at line 618.

_close (line 618) reads pending, words and sibling_slabs from the enclosing function scope at call time, not from its default args. Today that is harmless — every _close call happens inside the approximate loop, which finishes before this loop starts — so this is latent, not a live bug. But the two bindings are 65 lines apart in one function and have incompatible types (dict[str, dict[int, list]] vs a numpy array of uint64), so any future change that defers a close, hoists the packed loop, or merges the two passes turns it into a TypeError at best and a wrong-cell write at worst. Nothing in the file's comments warns the next editor.

Suggested fix: rename this local (and its uses on lines 686/699) to something the digest loop does not own, e.g. word_values, matching of_values right beside it.

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 ac42a24 — the packed loop's local is word_slab now (the read, the is None test, and both uses in the accumulation), leaving words to the digest loop's _close closure. Went with word_slab over word_values because the value is a whole cell slab, not a per-cell vector like of_values.

Comment at the binding says why, so the next editor does not reintroduce it:

# Not ``words``: that name is a free variable of the digest loop's ``_close``
# closure 65 lines up, with an incompatible type — harmless today (every close
# happens before this loop starts) but a latent wrong-cell write.

Comment thread docs/specification.md
- **`fields`** — every aggregation field, keyed by name, with its
**composability class**: `exact` (folds byte-equal — count/sum/min/max),
`approximate` (t-digest merge — `np.isclose` equality class), or `none`
`approximate` (t-digest merge — `np.isclose` equality class), `packed`

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)

[non-blocking] The class list gained packed but the normative sentence that says what an entry carries was left two-class, so §4.5 now enumerates a class it never gives an entry grammar for in that paragraph.

Nine lines below (unchanged, docs/specification.md:947):

exact/approximate entries carry the fold method, any further fold provenance (an exact fold's nan_policy), and enough dtype/shape metadata to know the overview array's form up front.

A packed entry carries all of that (method, dtype, fill_value — verified against declared_fields on the shipped strata template), and the dedicated bullet added further down at line 1003 says so. But an external reader working the paragraph top-down — the moczarr posture #340 exists for — reads a MUST-shaped enumeration that excludes packed, then a bullet that contradicts it. The none sentence immediately above ("A none entry carries class only") is what makes this an exhaustive-looking partition rather than a list of examples.

Suggested fix: exact/approximate/packed entries carry the fold method, … — one token, and the paragraph stops disagreeing with line 1003.

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 e022105 — one token, as suggested:

exact/approximate/packed entries carry the fold method, any further fold provenance (an exact fold's nan_policy), and enough dtype/shape metadata to know the overview array's form up front.

The paragraph now agrees with the dedicated packed bullet further down and with what declared_fields writes (method, dtype, fill_value, plus of and the optional threshold).

Comment thread docs/specification.md Outdated
with each contributor's `n` taken from its `of` digest's weight at the same
cell. An overview's composition array carries the same §3.3 attrs block a
leaf does, and its `N_signal` at every level is the folded `of` digest's
total weight at that level (digest weights fold exactly, §2). One

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)

[question] "digest weights fold exactly, §2" is stated without the caveat §4.6 already carries, and packed is the one class where that caveat is load-bearing.

docs/specification.md:754-756 (unchanged, §4.6) says the opposite side of the same fact:

2^24 observations, and there sum(weights) is the nearest float32 to the true count rather than the count itself

payload_weight (sweep_overview.py:226) is int(round(float(decode_digest(...)[:, 1].sum()))) — a float32 accumulation, rounded to an int, used as the divisor at every fold site. So the new sentence promises exactness for the very quantity §4.6 documents as inexact past 2^24, and for the CA store (o9 leaves, ATL03 photon counts) coarse levels are squarely past it. The practical error is tiny (relative ~1e-7, far below one lane step of 1/255), which is why I am filing this as a question rather than a defect — but this PR's premise is that a declaration must not claim more than the code delivers, and this is the same shape of claim.

Suggested fix, if you agree: scope it the way §4.6 does — "(digest weights fold exactly up to the §4.6 float32 bound)" — or drop the parenthetical and let §4.6 be the single statement. If instead the intent is that N_signal recovery is only ever accurate to one lane quantization anyway (so float32 drift is provably immaterial), saying that here would be stronger than the current absolute.

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

Agreed, and folded in 05609a0 as wording only. Scoped it the way §4.6 does, and said which way the residual cuts rather than leaving the reader to weigh it:

… its N_signal at every level is the folded of digest's total weight at that level — exact up to §2.1's float32 bound: past 2^24 pooled observations that sum is the nearest float32 to the true count (§4.6), and the fold's divisor rounds that float to an integer. The residual is relative ~1e-7, orders below one lane step of 1/255, so it never moves a published lane; it is stated because a reader recovering counts from N_signal is entitled to the same bound §2.1 gives the digest itself.

Both halves of the suggestion, in effect: the claim is bounded rather than absolute, AND the "provably immaterial against one lane quantization" argument is on the page instead of left implicit. No code change — payload_weight is unchanged.

from the folded strata matches leaf truth exactly (digest weights fold
exactly, spec §2)."""

def _fold(self, per_cell, fields=_STRATA_FIELDS, cell_order=5, resolution=4):

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)

[question] Every packed assertion in this class goes through fold_column; the other three fold sites the commit wires are unexercised. Which phase covers them?

self._fold (line 431) calls zagg.column.fold_column only. So phase 3 ships four packed fold implementations and tests one:

site tested here covered by phase 4 as scoped in the PR body?
column.fold_column yes
sweep_overview._fold_node (leaf → overview, start + i // fold_factor) no yes ("template → build → overview fold")
sweep_overview._fold_child (cascade, j*factor window) no implied, not stated
sweep_stage._merge_slabs (stage merge, (base + pos) // factor, windows folding across) no not mentioned at all

The three untested sites are where the indexing is easiest to get subtly wrong, and a wrong-but-plausible word is the failure mode this class exists to catch — fold_column's loop is the one whose index arithmetic is trivially [j*factor, (j+1)*factor) over a single flat slab. I did drive _fold_child and _merge_slabs by hand against this head and both reproduce merge_composition_kway over the right contributors (including the windows-fold-across path), so I have no defect to report — but nothing in the suite pins that, and _merge_slabs in particular has behavior no other site has (the half-pair broken classification, lines 686-694).

Concretely: is _merge_slabs in phase 4's scope, or should the phase-4 checklist item be widened to name it? If it is out of scope for this PR, a one-line note on the phase-4 entry saying so would keep the gap deliberate rather than accidental.

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

Answering the question directly: _fold_node and _fold_child were phase 4's scope, _merge_slabs was not — and it is covered now, so all four sites are pinned.

  • _fold_node and _fold_child: phase 4 (a73b64b), pushed after the snapshot this review saw. tests/test_strata_composability.py::TestEndToEndStrataPyramid::test_strata_and_composition_fold_at_every_level builds a synthetic strata store and sweeps it under the /1 cascade regime, so level 1 is exact-from-leaves (_fold_node, start + i // fold_factor) and level 0 is a cascade (_cascade_node_fold_child, the j*factor window). At each level the word is recomputed independently from the arrays one level finer, and presence is checked against a direct pooled pack of the raw rows. test_overview_provenance_records_the_packed_law pins the per-level zagg_overview.fields entry for both regimes;
  • _merge_slabs: now TestStageMergeHalfPair::test_a_half_paired_contributor_poisons_its_output_cells (69a8e75), which drives it over two real written columns with an output cell spanning both children — including the half-pair broken classification this site alone has. That commit also changes the behavior there (see the sibling thread on line 690): the half-pair no longer just skips, it keeps the covered cells at the fill word;
  • _fold_node's multi-contributor geometry (target_order < shard_order, several leaves into one output cell) gets its own test in the same commit, TestLeafFoldHalfPair — the end-to-end fixture's geometry cannot reach it.

Comment thread src/zagg/column.py
declaration's ``fields`` map straight through can neither refuse a leaf
over a non-cell-extent vector slab nor materialize an all-empty ragged
group for a companion the column has no business carrying.
group for a companion the column has no business carrying. The ``packed``

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)

[non-blocking] composable_fields' own docstring was de-dualized here ("the two composable classes" → "the composable classes"), but its caller's docstring, which reasons from this filter, was not.

leaf_slabs at column.py:183:

fields is filtered to the composable classes first (:func:composable_fields), which is what makes the (n_cells,) extent check sound: those two classes admit nothing but cell-resolution scalars and ragged payloads …

Three classes now, and the sentence is the load-bearing justification for the raise at line 191 — a reader checking whether the extent check still holds is sent to count a set that has changed size. The argument itself survives (a packed word is a cell-resolution dense scalar, one row per cell), which is exactly why it should be restated rather than left to be re-derived.

Suggested fix: "those classes admit nothing but cell-resolution scalars (including the §3 composition word) and ragged payloads".

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 705591d, restated rather than left to be re-derived, as suggested:

fields is filtered to the composable classes first (composable_fields), which is what makes the (n_cells,) extent check sound: those classes admit nothing but cell-resolution scalars (including the §3 composition word) and ragged payloads, both of which are one row per cell …

Kept the argument's structure intact — it is still the justification for the raise below — with the packed word named as the case that would otherwise have to be checked against a class list.

Comment thread docs/specification.md
The distinction is only material for the `approximate` class: the exact
merge laws are associative, so a cascaded `sum`/`min`/`max` is the same
value either way, while a cascaded digest is a **merge of merges** — it
The distinction is material for the `approximate` and `packed` classes: the

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)

[non-blocking] §4.4 gained the packed cascade caveat but docs/hive_layout.md still states the two-class version of the same fact.

docs/hive_layout.md:266-268 (not touched by this PR):

The cost is precision in the approximate class — a cascaded t-digest is a merge of merges — which is why each level records the regime that made it …

That is now incomplete in exactly the way this line fixes: a cascaded composition word re-quantizes once per level too, so fold_source/exact_levels is a packed reader's concern as much as an approximate one. hive_layout.md is the narrative page a reader hits before the spec, and it also introduces pyramid as carrying "each field's D24 composability class (issue #201)" at line 253 — a sentence written when there were three tokens and now describing four.

Since this is the same wire-format change §4.4 is documenting, both should move together (§4 of CLAUDE.md pairs a spec change with its narrative docs). Suggested fix: "…precision in the approximate and packed classes — a cascaded t-digest is a merge of merges, and a cascaded composition word re-quantizes once per level (spec §3.4) —".

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 9c08b9a — both hive_layout.md sentences, moving with the §4.4 change as §4 of CLAUDE.md asks:

  • the cascade cost (line ~266): "The cost is precision in the approximate and packed classes — a cascaded t-digest is a merge of merges, and a cascaded composition word re-quantizes once per level (specification §3.4) — which is why each level records the regime that made it …";
  • the pyramid introduction (line ~253), which was the other three-token sentence: it now enumerates the class rather than gesturing at it — "each field's D24 composability class — exact, approximate, packed (the §3 composition word, issue D24: admit build_tdigest_where to the digest family; give composition an honest composability declaration #515) or none".

Rewrapped the touched paragraphs to the page's width; no other content moved.

Comment thread src/zagg/sweep_stages.py Outdated
n: dict(m)
for n, m in (decl.get("fields") or {}).items()
if isinstance(m, dict) and m.get("class") in ("exact", "approximate")
if isinstance(m, dict) and m.get("class") in ("exact", "approximate", "packed")

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)

[blocking] sweep_stage_pass is a FOURTH admission site and it still carries the literal filter — _is_composable (9651e97) never reached it.

The fold reply on the _is_composable thread says the stage path "inherits the same admission" via composable_fields at src/zagg/sweep_stage.py:955. That call is inside _write_stage_column (the column WRITER). sweep_stage_pass lives in this file and builds its own fields map right here, and that map goes to stage_node unfiltered (line 216) -> _stage_fold -> _merge_slabs/_gather_slabs. So a packed entry with no of is admitted to the fold at the stage path.

The consequence is worse than the "degrade to native resolution" _is_composable was added to guarantee. In _merge_slabs' packed arm of_name is then None, so reader.read(res_src, None) reaches group[None], which zarr 3.3.0 raises TypeError on (verified locally), and _ColumnReader.read catches only KeyError/FileNotFoundError/GroupNotFoundError (src/zagg/sweep_stage.py:334). Nothing between there and here wraps it — stage_node calls _stage_fold bare (sweep_stage.py:1186) and sweep_stage_pass calls stage_node bare (line 210) — so the whole staged pass aborts. _fold_node and _fold_child guard exactly this case on purpose (except (KeyError, TypeError)), which is what makes the gap here visible.

Fix, one line plus the import, mirroring sweep_overviews:

from zagg.column import _is_composable
fields = {
    n: dict(m)
    for n, m in (decl.get("fields") or {}).items()
    if isinstance(m, dict) and _is_composable(m)
}

and extend test_a_packed_entry_without_its_of_linkage_is_not_composable to cover this entry point too, so "all sites agree" is pinned rather than asserted.

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 484fd36. sweep_stage_pass's literal filter now routes through zagg.column._is_composable, exactly as sweep_overviews does — all four admission sites are one predicate. The comment at the site names why the literal was worse here than elsewhere: this map goes to stage_node unfiltered, so an of-less packed entry reaches _merge_slabs and reads group[None], a TypeError _ColumnReader.read does not catch.

Behavioral test rather than an import assertion: TestD24Classification::test_the_stage_pass_drops_a_packed_entry_without_its_of_linkage runs sweep_stage_pass twice on the same manifest shape, once with the of-less entry and once with the linkage restored. The fields gate sits before _candidate_decimals, so root_moc_stale in the summary is the "got past the gate" witness: absent for the broken entry, True for the whole one. Mutation-checked — the test fails with the literal filter restored.

Comment thread src/zagg/sweep_overview.py Outdated
f"sweep[overview]: leaf {leaf} lacks {of_name!r} for "
f"packed field {name!r}"
)
leaf_poison.add(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 (review)

[non-blocking] This half of the poison trigger fires in the direction where no skew is possible, and it deletes other leaves' correct words.

The sibling trigger at line 1681 is gated exactly right — if meta["class"] == "packed" and (meta.get("of") or "") in group — because that is the skew case: the word is gone, the divisor digest is present and the digest arm folds it, so the level's N_signal counts rows the surviving word never covered.

This one is the reverse: group[of_name] is absent. The digest arm hits the same KeyError for the of field at line 1673-1676 and skips it too, so the leaf contributes to NEITHER the word nor N_signal — the pair is already consistent. Poisoning here only discards what other leaves contributed to the same output cell. Reproduced on this branch, with TestLeafFoldHalfPair's own fixture but dropping leaf -3112's h_sig instead of its composition:

counts["failed"] = 0
weight(h_sig, 0)  = 80    # == leaf -3111 alone; the divisor already excludes -3112
composition[0]    = 0     # a consistent -3111-only word exists: 5999754895727340585

Both poison tests in 69a8e75 exercise only the word-missing direction, so nothing catches this. Suggested fix: drop leaf_poison.add(name) on this branch (keep the continue) and record in the comment why the two directions differ. If the pair-keying is deliberate, add the reverse direction to TestLeafFoldHalfPair so the loss is a pinned decision instead of a side effect.

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

Right, and folded in e5bcf12 — the trigger direction was backwards on this half. leaf_poison.add(name) is gone from the packed arm: when the of array is absent the digest arm reading that same array for the of field itself hits the same KeyError in the same leaf, so both halves drop together and the folded divisor already excludes the leaf. Nothing to skew, and poisoning destroyed sibling leaves' correct words. The comment now says so at the site, and the surviving trigger at the except KeyError branch is labelled the ONE skew direction (word absent, divisor PRESENT).

Both directions are now pinned by their own test, and each mutation-checked against its own half of the fix:

  • TestLeafFoldHalfPair::test_a_half_paired_leaf_poisons_its_output_cells (unchanged) fails when the skew-direction poison is disabled.
  • TestLeafFoldHalfPair::test_a_leaf_missing_the_divisor_drops_without_poisoning (new) drops leaf -3112's h_sig and asserts the cell keeps -3111's word and -3111-alone weights; it fails when leaf_poison.add(name) is put back on the divisor-absent branch.

# UNREADABLE leaf is skipped whole, so the digest excludes it too
# and the pair stays consistent without poisoning anything.
for name in leaf_poison:
packed_poison[name].update(range(start, start + span))

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)

[non-blocking] Poisoning at this site leaves no trace a reader can see — _fold_node returns no source_children.

The reply on the stage thread says "source_children.unreadable still records that the level folded short". That holds at _merge_slabs (broken.add((i, w)) + logger.warning). It does not hold here: _fold_node's return (lines 1807-1817) carries generation/content_hash/granule_count/time_range/fold_source and no source_children; a poisoned leaf still does n_leaves += 1; and both triggers log at logger.debug. TestLeafFoldHalfPair asserts counts["failed"] == 0 — so a poisoned cell is byte-indistinguishable from genuine emptiness, which is precisely the situation _cascade_node's own comment at line 1953 refuses to accept ("a log line in an exited process cannot tell a reader that a fill cell is under-coverage rather than emptiness").

Not a correctness bug — the fill word makes no §3.2 claim either way — but "absence over wrongness" only stands up if the absence is recoverable. Suggested fix: raise the two poison-path logger.debug calls to logger.warning (matching _merge_slabs), and/or carry a poisoned-cell count into the fold dict so _overview_provenance can record it; at minimum correct the thread's claim, which currently overstates what the leaf fold traces.

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

Correcting the earlier claim: you are right that source_children.unreadable is _merge_slabs' trace, not _fold_node's — _fold_node returns no source_children, a poisoned leaf still counts in n_leaves, and the log was debug. The stage-thread reply overstated what the leaf fold records.

Folded minimally in 62b3bb7. There is now only ONE poison site in _fold_node (e5bcf12 removed the other), and it logs at logger.warning naming the node, the field, the leaf, and the poisoned span:

sweep[overview]: node {node} field {name!r}: leaf {leaf} carries {of!r} but not the
word; output cells [{start}, {start + span}) keep the fill word (spec §3.3)

with a comment saying plainly that this line is the only trace, since the fill word 0 is byte-indistinguishable from genuine emptiness.

Left standing deliberately: the artifact-visible record. A source_children analog for the /1 leaf fold is a NEW attrs key on the overview group — spec §4.3 grammar plus a conformance-fixture regeneration under the issue #340 rule — which is a scope change, not a review fold. Leaving it for espg to rule on rather than inventing the key here.

Comment thread src/zagg/sweep_stage.py Outdated
f"contributor unreadable (spec §3.3, §1.1)"
)
broken.add((i, w))
poisoned.update(

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)

[non-blocking] The range arithmetic is exact; the trigger above it is not.

Span check first: the contributor covers source rows base .. base + src_per_child - 1, i.e. output cells base // factor .. (base + src_per_child - 1) // factor. (base + src_per_child + factor - 1) // factor == floor((base + src_per_child - 1) / factor) + 1, the exclusive end — no over- or under-poisoning of neighbours, at factor == 1 too.

The trigger is (word_slab is None) != (of_values is None), i.e. both directions, but only one skews. When the DIVISOR is the missing half, the digest loop above drops the same contributor as well (_companion_group / slab is None), so word and N_signal are already consistent and poisoning just deletes the other children's correct words. Reproduced with TestStageMergeHalfPair's fixture, removing child 1112's h_sig instead of its composition:

(folded, missing, unreadable) = (1, 0, 1)
composition[0] = 0     # a consistent 1111-only word exists: 6007324956125508149

The blast radius also runs wider than the comment's windows argument admits: factor = 4 ** (res_src - r) and src_per_child = 4 ** (res_src - child_order) (sweep_stage.py:787-788), so at every level with r < child_order — the ordinary coarse level, and the shape this test itself uses (factor=8, src_per_child=4) — one output cell is shared by factor / src_per_child different children, and one half-paired child blanks the cell for all of them.

Suggested fix: poison only when of_values is not None and word_slab is None, keeping broken.add((i, w)) for both directions. If pair-keying is deliberate, say in the comment that a poisoned cell drops SIBLING contributors and not merely other windows of the same one.

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 the direction half in f9d799d; the shared-cell blanking stays by design.

Direction: the poison is now gated if of_values is not None, so it fires only in the skew direction (word missing, divisor present). broken.add((i, w)) still fires for BOTH directions — the level did fold short either way. The comment now says why the reverse direction is safe: the digest loop above reads that same array for the of field itself and drops the contributor too (slab is None), so word and N_signal already exclude the same rows.

Blast radius: left by design, and now stated in the loop comment rather than left to the windows argument. At every level with r < child_order one output cell is shared by factor / src_per_child children, so poisoning it does drop sibling contributions — but a shared cell whose folded N_signal counts rows no surviving word describes cannot carry an honest word. Blanking beats skewing; the fill 0 makes no §3.2 claim, the skewed word would.

Tests, both mutation-checked against their own half:

  • TestStageMergeHalfPair::test_a_half_paired_contributor_poisons_its_output_cells (unchanged, still the skew direction) fails when the poison is disabled outright.
  • TestStageMergeHalfPair::test_a_contributor_missing_the_divisor_drops_without_poisoning (new) removes child 1112's h_sig, asserts (folded, missing, unreadable) == (1, 0, 1) and that the shared cell keeps 1111's word and 1111-alone weights; it fails when the poison is restored to both directions.

Comment thread tests/test_strata_composability.py Outdated
for i in range(4)
if weights(g1, "h_sig", i) > 0
]
assert int(g0["composition"][whole]) == merge_composition_kway(parts)

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)

[non-blocking] This assertion cannot catch a misaligned cascade window.

_fold_child's packed branch slices range(j * factor, min((j + 1) * factor, len(words_arr))) (src/zagg/sweep_overview.py:2036). The fixture has ONE leaf, so of level 1's 16 cells only 0..3 carry data and all four land in output cell 0; every other source cell has n == 0 and is dropped by the n > 0 guard. A window that over-reaches into the next child's cells is therefore invisible. Verified by mutation on this branch:

_fold_child:  min((j + 1) * factor, ...)  ->  min((j + 1) * factor + 1, ...)
pytest tests/test_strata_composability.py tests/test_sweep_overview.py tests/test_sweep_stage.py
  307 passed

The rest of the geometry IS well pinned — range(j * factor + 1, ...) (a shift) fails, a 1-for-n divisor fails at both _fold_node and _fold_child, and start + (i + 1) // fold_factor on the leaf fold fails — so this is the one hole, and it is the boundary the level-0 claim rests on.

Suggested fix: populate a second, non-adjacent output cell at level 1 (a second committed leaf under a different rank of -31 in _build_store), then assert level 0's word at BOTH cells and fill in between. That makes the j * factor boundary load-bearing rather than incidental.

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 0c105ad — the hole was real and the mutation now dies.

_build_store takes an optional second leaf and the fold test commits two: -311 (rank 0) and -312 (rank 1) under node -31, each with its own _strata_cells batch. Level 1 is populated at cells 0..7 (two four-cell spans), level 0 at cells 0 AND 1 — so the j * factor boundary now separates two POPULATED spans instead of a span and empty fill that the n > 0 guard swallows.

The level-1 and level-0 assertions loop over both leaves in the existing arithmetic style: level 1 at 4 * rank + j, level 0 at rank, each level-0 cell's word recomputed from its own four level-1 cells and its presence compared against a per-leaf pooled pack_composition_n of that leaf's raw rows. The composition[15] == 0 fill assertion and the §3.3 attrs-block check are unchanged.

Mutation verified: min((j + 1) * factor, ...) -> min((j + 1) * factor + 1, ...) in _fold_child now fails test_strata_and_composition_fold_at_every_level (it passed 309 tests before). Reverted after checking.

Comment thread tests/test_strata_composability.py Outdated
"""The synthetic strata leaf config: the CA shape at test scale."""
from zagg.config import PipelineConfig

where = "(conf_land >= 2)"

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)

[non-blocking] The fixture config declares a strata cut that does not match its own composition threshold, so what it pins is a config that would break the §3.3 divisor invariant if it were ever actually built.

where = "(conf_land >= 2)" makes h_sig a LAND-only digest, while composition packs threshold: 2 across all five §3.1 surfaces and _strata_cells (the bytes actually written) uses signal = (conf >= 2).any(axis=1). The identity the packed class rests on — weight(of) == N_signal, spec §3.3 — therefore holds here only because _build_store writes hand-built arrays and the declared reducers never run. A real build from this config would give weight(h_sig) < N_signal at every cell and silently wrong recovered counts at every level.

That is exactly the coupling zagg.config deliberately does not validate ("The third coupling — the digest fields' where predicates committing the same cut as threshold — is not validated here", src/zagg/config.py:856-862), documented there as a silent-wrong-answer class. A fixture whose docstring calls it "the CA shape at test scale" is what gets copied into the next product template.

Suggested fix: make where the cut the word packs, e.g. "(conf_land >= 2) | (conf_ocean >= 2) | (conf_sea_ice >= 2) | (conf_land_ice >= 2) | (conf_inland_water >= 2)". Separately worth noting for the PR body: phase 4 is template -> hand-written slabs -> sweep, not template -> build -> sweep — nothing here drives the reducers, so the config/data agreement is unpinned either way.

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 a61847c. _strata_leaf_cfg's where is now the five-column union the word actually packs:

((conf_land >= 2) | (conf_ocean >= 2) | (conf_sea_ice >= 2)
 | (conf_land_ice >= 2) | (conf_inland_water >= 2))

matching _strata_cells' signal = (conf >= 2).any(axis=1) and pack_composition_n's threshold: 2. Wrapped in outer parens so h_noise's f"~{where}" negates the whole union rather than binding ~ to the first term — _resolve_param evals it as a Python/numpy expression, where ~ binds tighter than |.

The comment above it says the predicate is never executed here (_write_strata_leaf writes the slabs by hand, so no reducer runs) but must still commit the SAME cut the words were packed at, points at zagg.config's documented non-validation of the third coupling, and names the reason: a land-only where gives weight(h_sig) < N_signal at every cell and breaks §3.3's recovery in anything copied from this fixture.

Your second point — phase 4 is template -> hand-written slabs -> sweep, not template -> build -> sweep, so config/data agreement is unpinned either way — is fair and left standing for espg rather than folded: driving the reducers end to end is a scope change, not a fixture fix.


SHARD_ORDER, CELL_ORDER, K = 3, 4, 1

def _leaf(self, root, dec, per_cell):

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)

[non-blocking] TestLeafFoldHalfPair._leaf and TestEndToEndStrataPyramid._build_store (line 913) are the same ~25 lines twice, added four commits apart in one file: HealpixGrid(..., config=_strata_leaf_cfg()), emit_shard_template, morton from generate_morton_children, count as n_signal + n_noise, the two ragged strata slabs, the composition word column, stamp_commit. They differ only in (SHARD_ORDER, CELL_ORDER) and the manifest write the e2e one additionally does.

Suggest one module-level helper — _write_strata_leaf(root, dec, per_cell, *, shard_order, cell_order) returning the leaf group path — with _build_store keeping only the manifest half. Beyond tidiness: both copies must track _strata_leaf_cfg, so a fix to one geometry (the where point on line 646, say) silently misses the other.

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 fbc2a5a — it fell out cleanly, and your "a fix to one geometry silently misses the other" argument decided it: the very next commit in this fold pass changed _strata_leaf_cfg's where, and the leaf after that added a second leaf to the e2e store.

Module-level _write_strata_leaf(root, dec, per_cell, *, shard_order, cell_order) now owns template / morton / count / the two ragged strata slabs / the packed word / stamp_commit, and returns the cell-order group path. TestLeafFoldHalfPair._leaf is a three-line call; _build_store keeps only its manifest half (and, since 0c105ad, the second-leaf loop). Both call sites and the whole file are green.

Comment thread docs/specification.md
load-bearing exactly as `location` is above: the overview writer
reconstructs a level's arrays from this entry alone, so without it the fold
has no divisor and the overview array's §3.3 attrs block could not be
written. A writer MUST declare a composition field `packed` only when the

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)

[non-blocking] This MUST is now weaker than what the writer enforces — 3367b7f added a second condition to the demotion and the spec did not move with it.

declared_fields demotes to {"class": "none"} when the of digest is not approximate or when it declares non-default §2.0 weights (src/zagg/pyramid.py:255: classes.get(of) != "approximate" or of_weights not in (None, "counts")), on the reasoning that N_signal is a photon COUNT so a flux-weighted divisor is the wrong n. §4.5 states only the first half, so an external writer following this page emits packed over a flux digest where zagg would not — and §3.4's "its N_signal at every level is the folded of digest's total weight at that level" quietly assumes counts as well.

Per the #340 rule (external readers decode from spec + fixtures alone), suggest extending the sentence, e.g. "…only when the digest of names is itself declared approximate in the same map and carries the default §2.0 weights: counts — the law divides by that digest's weight at every level and N_signal is a count, so a flux-weighted divisor is a different quantity, and such a composition is declared {"class": "none"} instead." No fixture change needed: kitchen_sink's divisor is a counts digest, so the regenerated fixture already conforms.

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 f3973aa. The §4.5 packed bullet now carries both halves of what declared_fields enforces:

A writer MUST declare a composition field packed only when the digest of names is itself declared approximate in the same map and carries the default §2.0 weights: counts — the law divides by that digest's weight at every level and N_signal is a photon COUNT, so a composition whose divisor is excluded from the pyramid, or whose divisor is flux-weighted (a different quantity), is declared {"class": "none"} instead.

Wording tracks the demotion warning's own reasoning at src/zagg/pyramid.py. No fixture change needed as you note — kitchen_sink's divisor is a counts digest, and tests/test_spec_conformance.py is green (178 passed).

espg and others added 9 commits August 24, 2026 15:42
@espg espg added the waiting label Aug 24, 2026
@espg

espg commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

All 4 phases are complete at 5b0b38fc and all 26 adversarial-review findings are folded or answered on-thread. CI state: Lint, Lambda Build, Docs and both build arches green; the Tests job is red on exactly one testtest_lambda_build::TestFunctionBuild::test_function_build_size, the function zip at 30.0MB against the 30MB budget (run 32787095167: 1 failed, 4762 passed). The 32MB raise is espg-ruled and lands on PR #511; per §1 this PR does not touch the gate, so it stays draft + waiting until #511 merges — at which point a re-run goes green and this PR flips ready-for-review.

Standing items for review are in the PR body's "Questions for review": the artifact-visible poison record (needs a new spec §4.3 attrs key — scope decision), the config-side flux-divisor cross-check, the stage-gather packed-pair posture, and the §4 module-size note.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014eRcohZWarXGLrNMsLXD4b
@espg

espg commented Aug 25, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

PR #511 merged, so the branch's single red is resolved: merged origin/main forward at 37616292 (clean merge — #511/#398 touch catalog/config territory, no overlap with this PR's semantics/pyramid/sweep files), and CI is now fully green on that sha — test_function_build_size passes under the 32MB budget, both test matrices green (Tests, Lint, Lambda Build, Docs, both build arches). Flipped ready-for-review per §2: all phases complete, all 26 findings folded or answered, CI green. waiting stays on — the ball is with review; the standing questions are in the PR body.

@espg

espg commented Aug 25, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

Recording espg's rulings (in-session 2026-08-24) on the four standing questions:

  1. Artifact-visible record for composition demotion → follow-up issue, now filed as record composition demotion in the store: artifact-visible attrs when the packed-class guard rail fires at an overview node #518 (spec §4.3 attrs key + fixture regen scope; the demotion guard already prevents wrong bytes — this is observability of the demotion, not correctness).
  2. Config-load refusal of a flux-weighted of divisor → NO. The demotion + warning stands; a load-time refusal would break currently-legal configs (espg: "please don't break valid configs").
  3. Stage-gather packed-pair posture — informational, acknowledged as recorded.
  4. Module-size raise (sweep_overview.py 2,403 / sweep_stage.py 1,408) — acknowledged; espg will circle back post-0.50 (the split is deliberately deferred past the release train).

No code changes follow from these; the PR merges as-is.

@espg
espg merged commit 419a17b into main Aug 25, 2026
8 checks passed
@espg
espg deleted the claude/515-strata-composability branch August 25, 2026 00:07
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.

D24: admit build_tdigest_where to the digest family; give composition an honest composability declaration

1 participant