small fixes: staged sweep skip key carries the run ids; bench object model gains the column term

[espg]

Closes #417. Closes #418.

Two small-fix issues bundled per CLAUDE.md §5, both fallout from merged PR #416 (its questions 6 and 7). They share no code: phase 1 is the staged sweep's skip gate, phase 2 is the CI object-count model.

Phases

• Phase 1 — issue staged sweep skip gate: same-second rewrite blind spot — close it with the run_id already in the stamps #417 (4aa7107, folds 1ebd3e1 / 68bbc13 / 9d5a1f7): the staged sweep's skip key gains the run_id term, closing the same-second rewrite blind spot.
• Phase 2 — issue bench_objects.py object-count model needs a term for the leaf pyramid column (explicit .github change request) #418 (0691bcd, folds ff685f6 / a528a08 / b0b51c6): .github/scripts/bench_objects.py gains a leaf-pyramid-column term, its scope is documented in the script header, and test_hive_sharded_store_matches_model runs against the default (pyramid on) with the opt-out kept as a case.

Phase 1 — issue #417: the skip key carries the run ids

The blind spot. hive._utcnow() resolves to one second (datetime.now(timezone.utc).isoformat(timespec="seconds")), so the ruled skip key — summed child n_leaves + max child stamp timestamp — cannot see a child column rewritten inside its own recorded second at an unchanged leaf count. The gate reads it as current and the parent keeps the stale fold. The /1 content-hash backstop cannot apply here without doing the fold the skip exists to avoid.

The fix. The key becomes the triple (n_leaves, max_leaf_timestamp, run_ids), where run_ids is the sorted set of run_ids carried by the consumed children's stamps (PR #416 phase 2 already puts one in every stage stamp — no extra I/O, the stamp is read anyway). A same-second rewrite by a different run moves the set; a same-second rewrite by the same run is excluded by the single-writer law.

Two pure helpers in zagg/column.py carry the grammar, next to COLUMN_ATTR (the generation block is part of that artifact's attrs, §4.6) — which also keeps sweep_stage.py under the §4 module cap, at 1,194 lines against 1,185 on main:

• generation_key(block) — the comparison key, (n_leaves, max_leaf_timestamp, sorted set of run_ids);
• stamped_generation_key(block, stamp) — one child's contribution: its recorded block (or, for a leaf column, the leaf identity) unioned with the run id on its own stamp. That second helper is the load-bearing half, and it exists because of the self-review (below): reading the relayed block alone leaves the term empty in every store the sweep actually builds.

Both comparison sites key off it — the ladder-entry gate in stage_node and _stage_column_current.

Two deliberate choices:

1. The run-id term is a SET over all contributing children, not the run id of the max-timestamp child. A scalar "run id at the max timestamp" is maskable: with children at (T, A) and (T, B), a rewrite of the first by a run sorting below B leaves the max pair at (T, B) and the blind spot open. The union over children moves under any child's foreign rewrite. It costs nothing in steady state — under the admission lease every stage column in a store is written by one run, so a re-sweep that rewrites nothing sees an unchanged set (no churn; pinned by test_second_pass_is_current).
2. Absent run_ids reads as the empty set, never as a wildcard. A pre-staged sweep skip gate: same-second rewrite blind spot — close it with the run_id already in the stamps #417 envelope entry folds once more on upgrade rather than inheriting the blind spot. On a store whose children are fleet-written leaf columns (no run_id in the stamp) both sides are empty and nothing re-folds at all — pinned by test_entry_without_run_ids_stays_current.

Spec (§4 obligation). §4.5 gains the normative triple (including the additive rule — absent run_ids MUST read as the empty set, never as a wildcard); §4.8 records how the key and the foreign-fresh abort divide the concurrency space (the abort covers a foreign stamp written since this run started; the key covers the foreign rewrite that landed before it, inside the second the timestamp cannot resolve); §4.4/§4.6 spell the block as {n_leaves, max_leaf_timestamp, run_ids}. docs/hive_layout.md's soft-barrier paragraph gets the narrative half.

Conformance fixtures: unaffected, and here is why. No fixture carries a generation block. tests/data/spec/column/ is a fleet-written leaf column (zagg_column with groups/cells_with_data_order, no generation — that block is written only by write_stage_column), and tests/data/spec/pyramid/ is a manifest whose stage content is the per-entry actuals, which this PR does not touch. tools/generate_spec_fixtures.py materializes no stage artifact (it calls run_finisher directly with synthetic per-level actuals). So the grammar change has no fixture surface to regenerate; tests/test_spec_fixtures.py passes untouched.

Phase 2 — issue #418: the bench object-count model

The gap. A default store now writes a leaf column per populated leaf ({node}/{window}.pyramid.zarr, §4.6) plus its D20 sidecar, and the model had no term for it. Measured against a real default store, the 28 column-zarr objects were silently absorbed into objects_overviews (excluded from the audited total) while the 29th — the all.pyramid.stats.json sidecar — inflated the exact per-shard count. Hence the pyramid: false escape PR #416 had to pin, and hence "the object-count audit currently does not cover the default store shape at all".

Scope, decided and documented (the issue's second acceptance criterion). LEAF-ONLY — the fleet write path, never a swept ladder. The module header now states it and says why, in three structural reasons rather than convenience: the sweep is fire-and-forget on Lambda so its artifacts may not have landed at measurement time; stage-column placement is dispatch cadence, which §4.6 declares "orchestration, never contract", so no fixed count could be right; and a partial or scoped sweep is a legal store state. So D9 rollups, ladder overview zarrs and ancestor-node stage columns stay in the unbounded buckets, while the leaf column — one deterministic artifact written inside the same worker the issue #215 tripwire guards — is audited exactly.

The term (_column_objects), derived through the writer's own functions (zagg.column.column_resolutions for the groups, composable_fields for the arrays) so it cannot drift from write_column:

    return 1 + len(resolutions) * (1 + 2 * arrays) + 1

one root zarr.json (template, attrs and stamp all rewrite it — three PUTs, one object), per declared group a group zarr.json plus zarr.json + chunk for morton and each composable field, and the sidecar. For the test geometry that is 1 + 3 * (1 + 2 * 4) + 1 = 29, matching a real store's listing exactly. Zero for a /1, declared-off, or leaf-level-less declaration.

No new argument at the harness call sites. measure_objects reads the pyramid block from the store's own manifest and threads it into the model. That is deliberate under §1: run_benchmark.py and run_full_aoi_benchmark.py are .github/ files issue #418 does not name, so they are untouched — and the manifest is the better source anyway, since hive.build_manifest writes it from the config before any leaf runs, making it the run's declaration rather than a product of the write path being audited. The audit therefore covers the default shape in CI now, not after a follow-up.

Measured side. _column_node classifies a column by the node it sits at: under a dispatched leaf's node it is that shard's write-path object (so it rides the exact per-shard #215 guard); anywhere else it is a stage column and joins the second-pass bucket, as does its sidecar. _overview_node now declines columns so the two buckets stay disjoint.

The pinning test is parametrized [default, pyramid-off]: the default arm has no escape hatch, and the opt-out arm asserts the term is exactly zero, so the column is pinned as a term rather than merely tolerated. Two unit tests join it — test_hive_columns_split_by_who_wrote_them (leaf column → per shard, stage column → second-pass bucket, both sidecars with them) and test_column_term_is_zero_without_a_v2_declaration.

Blast radius: .github/scripts/bench_objects.py and tests/test_benchmark_objects.py only. No other .github/ file, nothing under deployment/aws/.

Adversarial review (folded)

A fresh-context review ran after each phase and posted inline findings; all six are folded, with a reply and fix sha on every thread.

Phase 1 — the review caught a real defect this PR would otherwise have shipped: as first written, the run-id term was [] at every level of every store the sweep builds, because a stage column's contribution was read from its relayed generation block and never from its own stamp — the one place PR #416 put the run id. It passed only because the acceptance test injected a run_id into a leaf stamp, which no writer produces. Folded as stamped_generation_key (1ebd3e1), plus set-normalized comparison (68bbc13) and the missing stage-column test arm (9d5a1f7). That new arm fails against 4aa7107 (this PR's own phase-1 commit) and passes after — which is the evidence the fix is not inert.

Phase 2 — scope statement overstated the unit, (leaf, window) where the model counts per leaf, now fenced to the unwindowed leaf as _require_fullsphere fences the flat model (ff685f6); the objects_overviews key now describes itself as the second-pass sweep bucket holding two families, documented rather than renamed since its readers live in files this issue does not authorize (a528a08); and the column suffix resolves once at module scope instead of per key (b0b51c6).

Process deviation, disclosed: CLAUDE.md §2 asks for the review and fold to run as separate Opus-class subagents. No agent-spawning tool was available in this environment, so both passes were run by the same agent as separate, explicitly-scoped passes — review first (findings only, no edits), then fold (one commit per finding, staging only the files it touches, a reply on every thread). The independence the rule buys was approximated, not guaranteed; worth a skeptical eye on the phase-2 findings in particular, which are the weaker set.

Disclosed drive-by: test_ratchet_rewrites_on_child_change was failing on main

That test — the "stamp moved → re-fold" half of #417's acceptance ("pinned by the existing suite") — is red on a clean main checkout at most hours of the day. Its clock patch only moves the stamp when the current UTC hour string happens to start with 0:

        monkeypatch.setattr(
            hive_mod, "_utcnow", lambda: real().replace("T0", "T1", 1) if "T0" in real() else real()
        )

At any hour ≥ 10 UTC the else branch returns the real clock, the rewrite lands in the same second, and — exactly the bug this PR fixes — the ratchet does not move, so row["written"] > 0 fails. Verified on a stashed tree: 1 failed on main at this session's clock. It is now a deterministic +1 hour:

        later = (datetime.fromisoformat(_utcnow()) + timedelta(hours=1)).isoformat(
            timespec="seconds"
        )
        monkeypatch.setattr(hive_mod, "_utcnow", lambda: later)

Flagging rather than burying it, because CLAUDE.md §4 says to flag pre-existing failures rather than fix them: I judged this one in scope because it is the acceptance pin issue #417 names, in the file the issue's fix lands in, and it was failing for the very reason #417 exists. Say the word and it comes back out into its own issue.

How it was tested

• uv run pytest -q (full suite, after uv sync --extra test) → 3768 passed, 38 skipped in 7:46. Run again at the tip of the branch with the same result.
• uv run ruff check src tests .github/scripts → clean apart from the pre-existing N818 below. uv run ruff format --check → clean apart from the pre-existing markdown fence below. The PR's ruff CI job is green.
• Phase 1's bite, verified by stashing src/zagg/column.py and src/zagg/sweep_stage.py back to main: test_same_second_foreign_rewrite_is_refolded fails on assert row["written"] > 0 → assert 0 > 0, i.e. the gate skipped the node and served the stale fold. The stage-column arm fails the same way against the pre-fold commit.
• Phase 2's model checked against a real default store rather than only against itself: a listing of the end-to-end store shows exactly 28 all.pyramid.zarr/... objects plus one all.pyramid.stats.json, against the term's 29.

Questions for review

1. Fleet-written leaf columns still carry no run id, so the finest tuple's arm rests on the pair alone. column.write_column stamps without a run_id (hive.stamp_commit's is documented as stage-only: "Fleet-written leaves and columns never carry it"), so at the first dispatch tuple — whose children are leaf columns — the term is empty and a fleet rewrite of a leaf inside its recorded second is still invisible to the gate. Issue staged sweep skip gate: same-second rewrite blind spot — close it with the run_id already in the stamps #417's ruled fix is what landed; the leaf arm is a separate decision:
   • (1a) leave it (recommended for a small-fix — the sweep-side blind spot staged sweep skip gate: same-second rewrite blind spot — close it with the run_id already in the stamps #417 names is closed, and the leaf arm is a fleet-grammar change, not a sweep one);
   • (1b) thread the fleet run id into write_column → stamp_commit(run_id=...), making the term non-empty everywhere at the cost of changing what a fleet stamp means (§4.6 says a reader treats run_id's absence as "not a stage artifact");
   • (1c) add granule_count (already in the leaf column's stamp, already read — zero extra I/O) as a fourth key term, closing the leaf arm for the common append case without touching any writer.
2. Set-valued third term vs. the issue's scalar (timestamp, run_id) phrasing. Reasoning under phase 1's "deliberate choices" (1) — a scalar keyed to the max-timestamp child is maskable by a lexicographically smaller foreign run id at the same second. Confirm the set form, or say the word and it narrows to the scalar the issue text spells.
3. objects_overviews now holds two artifact families and its name says one. Documented, not renamed, because the key is read by bench_metrics.py, both run_*_benchmark.py and the rendered panel — .github/ files issue bench_objects.py object-count model needs a term for the leaf pyramid column (explicit .github change request) #418 does not authorize. A rename to objects_sweep across those files is a clean follow-up if you want it; it needs its own issue to carry the CI-change authorization.
4. Should the hive model grow a windowed arm? The model (column term included, and the per-shard attribution it inherits) assumes the unwindowed leaf — now stated explicitly rather than implied. A windowed store's leaves would surface as loud other findings long before the column term mattered, so nothing is silently wrong today; widening it is a real change with its own test surface and I did not smuggle it in.

Pre-existing findings (not fixed here)

• uv run ruff check src tests is red on main with N818 Exception name UnknownCapability should be named with an Error suffix (src/zagg/registry.py:64). The PR lint bot's ruleset (--select=E,F,W,I) does not see it.
• uv run ruff format --check src tests flags a python fence in tests/data/benchmark/README.md:176. Also pre-existing, also unseen by the bot's ruleset.

Both were flagged the same way on PR #379 (question (f)) and PR #416.

[espg]

🤖 from Claude (review)

Fresh-context adversarial review of phase 1 (issue #417) against CLAUDE.md and the issue's acceptance criteria.

Three findings, one of them load-bearing: the run-id term is empty in every store the sweep actually builds, because a stage column's contribution is read from its relayed generation block and never from its own commit stamp — the one place PR #416 phase 2 put the run id. The acceptance test passes only because it injects a run_id into a leaf-column stamp, which no writer here produces. Detail inline.

The other two: the key compares a recorded list where its meaning is a set, and the test battery has no stage-column arm (the arm the merge tiers depend on).

Not findings, checked and clean: the ratchet's no-churn property (a re-sweep that rewrites nothing sees an unchanged id set — test_second_pass_is_current pins it); the additive-read rule (absent run_ids → empty set, never a wildcard — the safe direction, and test_entry_without_run_ids_stays_current pins that a fleet-only store does not regenerate on upgrade); generation_key's () for a non-block, which can never equal a live 3-tuple; the §4 module cap (sweep_stage.py at 1,199); and the conformance-fixture argument in the PR body, which I verified independently — no fixture carries a generation block, and tools/generate_spec_fixtures.py materializes no stage artifact.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

The new term is empty everywhere in a real store — this fix is inert outside the injected test.

A child's contribution reads the run ids from its recorded generation block, and consults the child's own commit stamp only on the not isinstance(block, dict) branch. Trace the default width-3 sweep of a real store:

• tuple 1 (dispatch o6): children are fleet-written leaf columns → no block, and stamp["run_id"] is absent (column.write_column calls stamp_commit with no run_id; §4.6: "Fleet-written leaves and columns never carry it") → run_ids = [].
• The o6 stage column is then written with generation=fresh_gen, i.e. run_ids: [].
• tuple 2 (dispatch o3): children are those o6 stage columns → isinstance(block, dict) is true → the block's run_ids is [], and the branch that would read the stamp is never taken. The column's own stamp does carry run_id (write_stage_column(..., run_id=run_id)) and it is thrown away.

So run_ids is [] at every level of every store the sweep actually builds, and the skip key is byte-for-byte the old pair. The only thing that moves it today is a test that injects a run_id into a leaf column's stamp — which no writer in this repository does.

The run id issue #417 wants is the id of the run that wrote the child being examined, not the ids relayed up from that child's own children. _ColumnReader already holds it: self.stamp["run_id"], read at construction, no extra I/O. The contribution should be the block (or leaf identity) union the child's own stamp id.

Note the spec text this PR adds already describes the intended behavior rather than the implemented one — §4.5 says "the sorted set of run_ids those children's stamps carry", which is exactly the thing the code does not do for a stage column. Fixing the code makes the spec true; fixing the spec instead would document the blind spot as intentional.

Suggested shape, which also pulls lines out of this module (it is at 1,199 of the ~1,200 cap): a second helper next to generation_key in zagg/column.py taking (block, stamp), so the reader becomes a single return stamped_generation_key((self.attrs.get(COLUMN_ATTR) or {}).get("generation"), self.stamp).

---

Generated by Claude Code

[espg]

🤖 from Claude

Folded in 1ebd3e1 — the finding is correct and it was the whole fix: the term was empty at every level of every store the sweep builds.

A child's contribution is now its recorded block (or the leaf identity) unioned with the run id on its own stamp, in a new zagg.column.stamped_generation_key(block, stamp):

    runs = set(block.get("run_ids") or ())
    if stamp.get("run_id"):
        runs.add(stamp["run_id"])
    return generation_key({**block, "run_ids": sorted(runs)})

_ColumnReader.generation collapses to one call, which also took sweep_stage.py from 1,199 to 1,194 lines. The relayed ids stay in the union (they cost nothing and keep a deeper change's provenance flowing upward); the child's own id is what a same-second foreign rewrite moves.

Proof it was inert and is not any more: the new test_same_second_foreign_rewrite_of_a_stage_column_is_refolded (9d5a1f7) tampers with a real stage column's relay member and restamps it inside its recorded second under a foreign run_id. Against 4aa7107 (this PR's phase-1 commit, before the fold) it FAILS — the parent kept 136 + 272 at the '111' cell; after the fold it re-merges to 1000 + 272.

Spec followed the code rather than the reverse: §4.5 now reads "the sorted set of run_ids those children's stamps carry, unioned with the ids their own recorded blocks relay".

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

tuple(block.get("run_ids") or ()) keys on the recorded list order, but the key's meaning is a set (the PR body says so, and _summed_generation produces sorted(runs)). Every writer in this repo happens to sort, so this is latent rather than live — but the term is read back off disk from an artifact this process did not write, which is precisely where "happens to be sorted" is not a property the code may assume. An unsorted (or duplicated) recorded list yields a spurious mismatch and an unnecessary re-fold of a whole ladder.

tuple(sorted(set(block.get("run_ids") or ()))) costs nothing at this cardinality (1–2 elements) and makes the key an honest set comparison, matching what §4.5 now states normatively ("the sorted set of run_ids").

---

Generated by Claude Code

[espg]

🤖 from Claude

Folded in 68bbc13. The term is now normalized on read, so the comparison is a set comparison on both sides:

        tuple(sorted(set(block.get("run_ids") or ()))),

The docstring records why (the block is read back off an artifact this process did not write, so its list order is not a property to assume). No behavior change for anything this repo writes — _summed_generation already emits sorted(runs) — it removes a spurious-re-fold path for a store written by any other conformant writer.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

The acceptance test only exercises the leaf-column arm, and it exercises it by injecting a run_id into a stamp that no writer in this repository produces. That is why it passes over an implementation whose run-id term is empty in every real store (see the finding on sweep_stage.py): the test's injection is the only thing that ever populates the term.

A test that would have caught it: run the sweep at width=1 (_sweep(root, m, width=1) — test_width_1_writes_relay_columns shows the geometry, dispatch orders 2/1/0 with real stage columns at orders 2 and 1), then rewrite one stage column with foreign content and restamp it inside its recorded second under a different run_id. That child's stamp carries a real run_id written by write_stage_column, so it needs no unrealistic injection, and the parent must re-fold. Please add that arm alongside the leaf one — the stage-column case is the one the merge tiers actually depend on.

Two smaller notes on the same class:

• test_same_second_foreign_rewrite_is_refolded asserts row["written"] > 0. Worth pinning that the stale levels above also moved (the o0/o1 artifacts), not merely that something was written — the gate is per artifact node and a partial re-fold would satisfy the current assertion.
• _restamp returns the stamp dict but no caller uses the return value. Drop it or use it.

---

Generated by Claude Code

[espg]

🤖 from Claude

All three folded in 9d5a1f7.

The stage-column arm is now test_same_second_foreign_rewrite_of_a_stage_column_is_refolded, built exactly as suggested: sweep at width=1 so real stage columns exist at orders 2 and 1, tamper with the order-2 column's relay member (g["3"]["count"][0] = 1000 — the tier every coarser merge consumes), restamp it inside its recorded second under run_id: "C", re-sweep. The child's run_id there is written by write_stage_column, so nothing about the stamp is synthetic — only the same-second restamp is. The '111' output cell must move from 136 + 272 to 1000 + 272. Against 4aa7107 it fails; that is what proved the sweep_stage.py finding.

Worth recording why the tamper survives into the coarse tuple: the order-2 column's own current-check passes (its block is unchanged), so the pass leaves it alone and the order-1 dispatch reads the tampered bytes — which is the real-world shape of the race, not a contrivance.

The written > 0 assertion now also pins the two levels above:

        assert list(_artifact(root, "1/1/all.zarr")["2"]["count"][:])[0] == 136 * 10 + 272
        assert list(_artifact(root, "1/all.zarr")["1"]["count"][:])[0] == 136 * 10 + 272 + 408

_restamp's return is dropped.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Fresh-context adversarial review of phase 2 (issue #418) against CLAUDE.md, the issue's acceptance criteria, and the §1 rule the issue carves an exception to.

Three findings, all in the "state it correctly" class rather than the "it is wrong" class — the model term itself checks out arithmetically against a real store.

Checked and clean, for the record:

• §1 blast radius. The diff touches .github/scripts/bench_objects.py and tests/test_benchmark_objects.py and nothing else — no other .github/ file, nothing under deployment/aws/. git diff --stat on the phase-2 commit confirms it.
• The harness call sites did not need changing, and were not changed. Deriving the declaration from the store's own manifest inside measure_objects is what avoided touching run_benchmark.py / run_full_aoi_benchmark.py, which issue #418 does not name. It is also the more correct source: the pyramid block is written by hive.build_manifest before any leaf runs, so it is the run's declaration, not a product of the write path being audited. Worth stating on the PR that the audit is live in CI as a result rather than waiting on a follow-up.
• The term's arithmetic. 1 + len(resolutions) * (1 + 2 * arrays) + 1 = 29 for the test geometry (groups 8/7/6; morton + count/h_min/h_max), and a real default store lists exactly 28 column-zarr objects plus one all.pyramid.stats.json. Before this change those 28 were silently absorbed into objects_overviews and the 29th inflated the exact per-shard count — the audit gap the issue describes, now closed in both directions.
• The exactness claim. The column's groups are one chunk each, so no write_empty_chunks window is needed; counting the fail-open D20 sidecar as deterministic matches how the leaf's own stats.json is already counted, so the posture is consistent rather than newly optimistic.
• measure_objects can now raise on a hive store with an unreadable manifest. That is a real behavior change for run_benchmark.py, which does not catch (the full-AOI harness does), but it matches list_store_keys, which already raises on a missing store, and the alternative — silently modelling the pre-#384 shape — would hide the very gap this issue closes. Documented in the docstring; leaving it.
• The opt-out case is kept as a real case, not a comment. The parametrization asserts the term is exactly zero under pyramid: false, so the term is pinned as a term.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

The scope statement claims more than the model delivers: "per populated (leaf, window) unit" is not what is counted. expected_object_counts adds _column_objects(...) once per shard — lo = hi = 5 + len(members) + sidecar + _column_objects(...), multiplied by n_shards — so on a windowed store declaring W windows, the model expects one column where the fleet writes W.

Since issue #418's second acceptance criterion is precisely "the model's scope stated in the script header", a scope sentence that overstates the unit is the one defect that criterion cannot tolerate. Note the surrounding model has the same unwindowed assumption baked in and does not say so either — store_object_counts builds leaf_of from hive.shard_leaf_path("", int(k)) with no window=, so a windowed leaf ({id}_{window}.zarr) does not match any prefix and would fall out as a loud other long before the column term mattered.

So the honest fix is a documented assumption, not a windowed model: say that the hive model (column term included) is written for the unwindowed leaf, exactly as the flat model is fenced to fullsphere by _require_fullsphere. Widening it to windows is a separate change with its own test surface, and inventing it here would be untested speculation.

---

Generated by Claude Code

[espg]

🤖 from Claude

Folded in ff685f6, as a documented assumption rather than a windowed model — the finding's own recommendation, and the one the tests can back.

The scope paragraph now says "one deterministic artifact per populated leaf", and a new paragraph fences the whole hive model the way _require_fullsphere fences the flat one:

The hive model assumes the UNWINDOWED leaf (review finding), as the flat model assumes fullsphere HEALPix. One populated shard is one leaf and one column; a windowed store writes one of each per (leaf, window), which this model does not count — and would not reach in any case, since the per-shard attribution builds its leaf prefixes from hive.shard_leaf_path("", key) with no window=, so a windowed leaf matches no prefix and surfaces as a loud other first.

Verified rather than assumed: shard_leaf_path takes window=None by default and the windowed basename is {id}_{window}.zarr, so the existing prefix map genuinely cannot match one — the assumption predates the column term and is now stated instead of implied.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

overviews += 1 for a stage column makes objects_overviews hold two different artifact families: sweep overview zarrs (issue #201/#384) and, new here, stage columns at ancestor nodes. The key name is load-bearing outside this module — it is threaded into the record and read by the harnesses and the rendered panel — so renaming it is out of scope for an issue that authorizes exactly one file. But an operator reading objects_overviews: 28 in metrics.json and going looking for 28 overview zarrs will not find them.

Say so in store_object_counts's docstring, where the return contract is stated: the bucket is second-pass sweep artifacts, of which overviews are one family and stage columns another. One sentence, no behavior change, and it keeps the module honest about a key it cannot rename.

---

Generated by Claude Code

[espg]

🤖 from Claude

Folded in a528a08 — documented, not renamed, for the reason the finding gives (the key is read outside this module and issue #418 authorizes exactly one file). store_object_counts's return contract now reads:

objects_overviews is the SECOND-PASS SWEEP bucket, of which overview zarrs are one family and ancestor-node stage columns (issue #418) another — the name predates the second family and is load-bearing for the record's readers, so it is described here rather than renamed.

A rename (objects_sweep, with the harnesses and the panel following) would be a clean follow-up but needs bench_metrics.py and both run_*_benchmark.py in the same change; raising it under "Questions for review" rather than reaching for those files.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Two notes on _column_suffix, neither fatal but both cheap to settle now:

1. It runs on the hot path, twice per key. store_object_counts maps _column_node over every key in the pre-pass, calls _overview_node over every key again, and calls _column_node once more per key in the loop — each doing a from zagg.column import COLUMN_SUFFIX. sys.modules makes that a dict lookup rather than a real import, so this is tidiness rather than cost, but a module-level constant resolved once (still off COLUMN_SUFFIX, never a second literal) reads better than a function call standing in for a constant.
2. It puts a hard zagg dependency into _overview_node, which had none. That helper was pure string work; now importing it transitively requires an importable zagg. Every real caller already has one (the module imports zagg.store / zagg.hive elsewhere), so this is a coupling worth noticing rather than avoiding — but if it is going to exist, resolving it once at module scope makes the dependency visible at the top of the file instead of hiding it in a leaf helper.

Deliberately sourcing the suffix from zagg.column rather than re-spelling ".pyramid.zarr" is right, and matches that constant's own docstring ("the ONE definition of the name seam ... never off a second literal"). This is only about where it is resolved.

---

Generated by Claude Code

[espg]

🤖 from Claude

Both folded in b0b51c6. _column_suffix() is gone; the suffix is resolved once at module scope, still off the single definition:

from zagg.column import COLUMN_SUFFIX

and _column_node / _overview_node compare against the constant directly.

On note (2) — making the dependency hard rather than lazy — I checked the blast radius before doing it rather than assuming it was free: only run_benchmark.py and run_full_aoi_benchmark.py import bench_objects, and both already import zagg (from_config, get_store_layout) before reaching it. The zagg-free scripts (plot_series.py, plot_summary.py, update_series.py) import bench_metrics only, which is untouched and stays pure arithmetic. So no consumer loses the ability to import what it imports today.

---

Generated by Claude Code

[espg]

🤖 from Claude

Both phases are complete, all six self-review findings are folded with a reply on every thread, and CI is green at b0b51c6 — ruff, test (3.12), test (3.13), build, build / build-x86_64, build / build-arm64 all pass. Marked ready for review and labelled waiting: the ball is in the reviewer's court for the four questions in the description, none of which block the change as it stands (each names a recommended option).

Two things worth a reviewer's eye specifically, both written up in full in the description:

1. The self-review caught a defect this PR would otherwise have shipped. Phase 1's first implementation left the new run_id term empty in every store the sweep actually builds — it read the relayed generation block and never the child's own stamp — so it passed only because the acceptance test injected a run id a fleet writer never produces. The fold (1ebd3e1) fixes it, and the added stage-column test arm (9d5a1f7) fails against this PR's own phase-1 commit and passes after, which is the evidence the term is live rather than decorative.
2. A pre-existing test was repaired rather than only flagged. test_ratchet_rewrites_on_child_change is red on a clean main at any UTC hour ≥ 10 — its clock patch only moves the stamp when the hour string starts with 0, so at other hours the "rewrite" lands in the same second and the ratchet correctly does not move. It is the acceptance pin issue staged sweep skip gate: same-second rewrite blind spot — close it with the run_id already in the stamps #417 names, failing for exactly the reason staged sweep skip gate: same-second rewrite blind spot — close it with the run_id already in the stamps #417 exists, which is why I judged it in scope; happy to back it out into its own issue if you would rather keep the flag-don't-fix line strict.

Process deviation, disclosed rather than buried: CLAUDE.md §2 asks for the review and fold passes to run as separate Opus-class subagents. No agent-spawning tool existed in this environment, so both ran as separate explicitly-scoped passes by the same agent — review first (findings only, no edits), fold second (one commit per finding, one reply per thread). The independence the rule buys was approximated, not guaranteed.

---

Generated by Claude Code

[espg]

🤖 from Claude

Set back to draft — flagging a rule conflict rather than resolving it unilaterally.

This PR was flipped draft → ready-for-review under CLAUDE.md §2 (all phases done, findings folded, CI green, nothing awaiting a ruling). I have reverted it to draft. The flip is reversible in both directions, so nothing is lost either way, but the two governing documents disagree and the disagreement should be ruled on rather than guessed at:

• CLAUDE.md §2 directs the flip: "when a PR has all phases completed, all adversarial-review findings folded, CI green, and isn't waiting on [the owner], mark it ready for review yourself" — and §6 names it as the one explicit exception to the owner-only list, precisely because it is reversible.
• This routine's standing operating instruction says the opposite, without qualification: "Never mark a PR ready-for-review. Never merge. Drafts only."

Both documents give the same tie-break — on a genuine conflict, stop and leave a comment instead of guessing. Draft is the conservative side (it withholds a signal rather than emitting a false one), so that is where I have left it. Precedent is consistent: a prior run recorded the same override on PR #350.

No action needed to accept the work — the substance is unaffected, and flipping to ready is one click whenever you want the review signal on. What would help is a one-line ruling so later runs stop re-litigating it:

• (a) the routine instruction wins — routines never flip; the owner flips (this is the current behavior, and matches PR Narrative notebooks: query / write / read, each stage-timed (issue #328) #350);
• (b) CLAUDE.md §2 wins — routines flip once its four conditions hold, and the routine prompt gets amended;
• (c) flip only for small-fix bundles, stay draft for implement PRs.

The PR keeps implement + waiting, which is the correct state under either reading: all phases are complete and it awaits your review.

One process deviation on this PR, restated here so it is not buried in the body. CLAUDE.md §2 requires the adversarial self-review and the fold to run as two separate Opus-class subagents. No agent-spawning tool was reachable in that execution environment, so both passes were run by the same agent as separately-scoped passes — review first (findings only, no edits), then fold (one commit per finding, replies on every thread). The independence the rule buys was approximated, not guaranteed. The phase-1 finding is self-evidencing regardless — its added test arm fails against 4aa7107 and passes after 1ebd3e1 — but the phase-2 findings are the weaker set and deserve a skeptical read.

---

Generated by Claude Code