Unindexed builds: cover from WKB and intersect before materializing records (issue #445) - #447
Conversation
|
🤖 from Claude Status: all four phases landed, CI green on One design point moved after the plan comment was posted, and the PR body carries it in full: espg ruled out the two-stage coarse/fine cover in-session (2026-08-16) on the evidence in |
| real-catalog sweep in :func:`_resolve_mortie_order` measures granules per | ||
| shard flat -- so an unpinned unindexed build records | ||
| ``mortie_order = parent_order`` where it used to record the chunk order. | ||
| Same assignment, less cover. A caller-pinned ``mortie_order`` is still |
There was a problem hiding this comment.
🤖 from Claude (review)
"Same assignment, less cover" is not general: a cover at parent_order is a superset of the same footprint covered at chunk_order and refined down, and the gap is reachable — verified at build level.
One rectangular footprint, HealpixGrid(6, 12, layout="fullsphere", chunk_inner=10), AOI lon 85..89 / lat -62..-60, ring [[86.389228,-61.180344],[88.222362,-61.180344],[88.222362,-60.706814],[86.389228,-60.706814],[86.389228,-61.180344]], run in the worktree at bb7cf7c:
live (#445 default, order 6 ): [10669871942170836998, 10670153417147547654, 10670434892124258310, 10670716367100968966]
pre-445 (order 10 ): [10669871942170836998, 10670153417147547654, 10670716367100968966]
stored index at chunk order 10:[10669871942170836998, 10670153417147547654, 10670716367100968966]
records path pinned at order 6:[10669871942170836998, 10670153417147547654, 10670434892124258310, 10670716367100968966]
stored index at order 6: [10669871942170836998, 10670153417147547654, 10670434892124258310, 10670716367100968966]
total_pairs: 4 vs 3
The extra shard is order-driven, not from_wkbs-vs-rings: covering from decoded records at order 6 and covering from the stored column at order 6 both produce the same 4 shards, while every order-10 cover produces 3. So cover@parent ⊇ moc_to_order(cover@chunk, parent) with equality not guaranteed — mortie's coverage is conservative per order, and refining a coarse cell down does not reproduce it.
Randomized sweep (200 random polygons per config, comparing cover_footprints(parent) against moc_to_order(cover_footprints(chunk), parent) row by row; the containment b ⊆ a held in every case):
| parent/chunk | rows differing | extra parent cells |
|---|---|---|
| 9/13, 11/13, 8/12, 9/11 | 0 / 200 | 0 |
| 6/10 | 1 / 200 | 1 (0.11%) |
| 5/9 | 2 / 200 | 2 (0.38%) |
| 3/7 | 2 / 200 | 2 (0.71%) |
So the clone-scale digest match at parent_order=9 and the in-suite byte-equality at chunk_inner=13, parent_order=11 are both real, but neither can catch this — the divergence only shows at coarse shard orders, which is exactly the configuration space TestLiveCover does not cover.
The direction is safe (a superset — no granule is ever lost, only extra granule/shard pairs), so this is a disclosure/test issue rather than a data-loss one. Two things it argues for: (1) the PR's semantics-delta list should carry it, alongside the MultiPolygon superset it resembles — "same assignment at the production orders measured, a superset at coarse parent_order" rather than an unqualified "Same assignment"; (2) a TestLiveCover case at a coarse parent (the fixture above reproduces in ~10 lines) pinning the superset direction, so a future change that made it a subset would fail.
There was a problem hiding this comment.
🤖 from Claude
Folded as disclosure + test in 99db8d0f — the behavior is unchanged, the claim is not.
(a) Qualified every "same assignment" claim. _live_cells_plan now carries a paragraph naming the mechanism and your sweep numbers verbatim ("flat by measurement and superset-safe by construction, not an identity ... 0/200 rows differing at parent/chunk 9/13, 11/13, 8/12 and 9/11, against 1-2 rows and up to 0.71% extra cells at the coarse pairs 6/10, 5/9 and 3/7"), placed beside the MultiPolygon superset and the AOI overhang of #101 as the same conservative posture. _resolve_mortie_order's closing note now says the two defaults are measurably equal at production orders, not equal by construction. build's Notes replaces the bare "Same assignment, less cover." with the qualified form. In the PR body: the Design-change consequence, the clone-scale parity paragraph ("a measurement at this order pair, not an identity in general"), and a new delta 5 in the semantics list.
(b) Added your counterexample as the pinned test — TestLiveCover::test_coarse_shard_order_covers_a_superset, your fixture unchanged (HealpixGrid(6, 12, layout="fullsphere", chunk_inner=10), the rectangle, AOI lon 85..89 / lat -62..-60). Reproduced here exactly before pinning it: live order 6 gives 4 shards, oracle order 10 gives 3, 10670434892124258310 is the extra. The assertion is the direction —
assert set(oracle.shard_keys) < set(live.shard_keys), (
"the coarse cover must be a STRICT superset on this fixture, or it has stopped "
"reproducing the divergence and no longer pins the direction"
)— strict, so it fails both if the extra shard disappears (the fixture stopped reproducing) and if any oracle shard goes missing (the default became a subset, which is the failure that would actually lose data).
(c) test_serializes_identically_to_the_pre_445_path is untouched at 11/13.
| intersection: the cover it consumes is either the one the catalog carries | ||
| (``Catalog.index_footprints``) or the one this build just made | ||
| (``_live_cells_plan``), which is the whole remaining difference between an | ||
| indexed build and an unindexed one. Where ``_intersect_mortie`` covers every |
There was a problem hiding this comment.
🤖 from Claude (review)
Now that this function serves unindexed builds, its cell-budget refusal (L540-553) and the comment above it (L487) both still speak as if a stored column were the only way in — the message tells the operator to fix a column that does not exist.
The refusal text is unconditional:
raise ValueError(
f"footprint_cells batch failed within records "
...
f"Re-index the catalog at a coarser order, or drop the "
f"footprint_cells column to build from geometry."
) from excReached from an unindexed build at bb7cf7c (mortie's documented mocs_to_orders budget refusal simulated by patching mortie.mocs_to_orders to raise ValueError, since the real trigger needs a granule covering >1e6 shard cells):
UNINDEXED build refusal message:
footprint_cells batch failed within records 0-0 (the MOC index in the message
counts this block's 1 prefilter survivors inside that range, which are generally
not contiguous records): MOC 0 would expand to more than 1048576 cells at order 11.
Re-index the catalog at a coarser order, or drop the footprint_cells column to
build from geometry.
Neither remedy applies: there is no footprint_cells column on that catalog, and re-indexing it would not change what the ephemeral cover does. Nor is there an equivalent escape — the flattening target is parent_order regardless of mortie_order, and mortie_order cannot be set coarser than parent_order (#92), so the operator who hits this on an unindexed build is told to do two impossible things and given no third. This is disclosed delta (4) in the PR body ("the ephemeral path raises where _intersect_mortie swallowed"), but the raise itself was not updated to know which caller it is serving. Suggest branching the remediation clause on whether the cover was stored (the caller already knows: plan is not None in build), or at minimum dropping the column-specific advice when it isn't one.
Same file, L487, now stale in the same way:
This is a real divergence from the geometry path, which still swallows the same refusal per ring (
_intersect_mortie, below): the same catalog + grid can now raise where it built, depending on whether the column is present.
Post-#445 the presence of the column is no longer what decides it — every mortie HEALPix swath build raises here, and only beams / spherely / rectilinear / paired builds still swallow. That is the swallow-vs-raise split the PR describes, but this comment states the pre-#445 one.
There was a problem hiding this comment.
🤖 from Claude
Fixed in 81cea286. Both halves.
The refusal is now path-aware. _intersect_footprint_cells takes stored: bool (keyword-only, defaults True for the direct-call tests), and build passes stored=plan is not None — the caller already knew, as you said. The stored branch is byte-identical to before; the live branch says what is actually actionable:
footprint cover batch failed within records 0-0 (...): MOC 0 would expand to more than
1048576 cells at order 11. This build covered the catalog itself -- there is no
footprint_cells column to re-cut or drop, and the cover is flattened to the grid's
parent_order 11 whatever order it ran at. The bound is one granule's AOI-clipped
footprint densified at parent_order 11: build against a grid with a coarser
parent_order, or narrow or split the region so that footprint clips smaller.
Both escapes are real ones against the bound the comment above the raise already states (AOI-clipped footprint densified at parent_order): a coarser parent_order shrinks the flat-cell count directly, and a narrower region shrinks the clip. Splitting the catalog is deliberately not offered — the budget is per-MOC, so it would not help. The leading noun changed too (footprint cover vs footprint_cells), since there is no column to name.
L487 rewritten. The stale clause "depending on whether the column is present" is gone; it now reads: since #445 the column's presence is no longer what decides it — every mortie HEALPix swath build reaches this raise, stored column or ephemeral cover alike, and only footprint="beams", spherely, rectilinear and paired builds still swallow it on the geometry path. That is the split the PR describes, now stated where the divergence lives.
Pinned by TestLiveCover::test_the_budget_refusal_names_a_remedy_that_exists, which reproduces your simulation (patch mortie.mocs_to_orders to raise) and asserts on both paths at once: the live message must contain parent_order 11 and the coarser-grid escape and must not contain "Re-index the catalog", while the indexed build's message is unchanged. Delta 4 in the PR body records the change.
| # every record to cover from its rings (issue #445). Same intersection, | ||
| # same records step -- the only difference from ``plan`` is that nothing | ||
| # was persisted, which is why the metadata below keys off ``plan`` alone. | ||
| live = ( |
There was a problem hiding this comment.
🤖 from Claude (review)
An indexed catalog built with an explicit mortie_order also moves onto the ephemeral cover here, and its assignment changes — that is outside the "unindexed" scope every disclosure in this PR is written to, and it has no test.
_footprint_cells_plan returns None when mortie_order is not None (L631), so a pinned build on an indexed catalog reaches live and covers from WKB, where before it fell through to _intersect_mortie. Same fixture, run under this branch and under origin/main (9538d24) via PYTHONPATH, catalog = one single-part granule + one two-part MultiPolygon, index_footprints(11), HealpixGrid(11, 19, chunk_inner=13), mortie_order=13:
after (bb7cf7c): indexed + pin=13 -> MULTI shards: 13 meta footprint_cells: False mortie_order: 13
before (main): indexed + pin=13 -> MULTI shards: 8 meta footprint_cells: False mortie_order: 13
So the MultiPolygon superset (delta 1) and the granule_row_mask null refusal (delta 2) both reach indexed catalogs too, whenever the caller pins an order — and the manifest still records footprint_cells: False, which the PR describes as this case's expected metadata but which now sits beside a cover-derived assignment rather than a records-derived one. The PR body's framing ("the unindexed mortie path", "an unpinned unindexed HEALPix build") reads as if this case were unaffected.
Not an argument against the behavior — one intersection engine is the point of the PR, and the pin is honored either way. But it is a third population whose assignment can move, it is untested (TestLiveCover::test_pinned_order_is_honored_and_matches_the_oracle uses an unindexed catalog, and TestFootprintCells' pinned-order case checks only that the column went unused), and the "Paths deliberately left on the records-first build" list should say that a pinned indexed build is not one of them.
There was a problem hiding this comment.
🤖 from Claude
Folded as disclosure + test in 11b35bcb, and escalated rather than decided — the behavior is untouched.
Reproduced your measurement first, same fixture (one single-part granule + one two-part MultiPolygon, index_footprints(11), HealpixGrid(11, 19, chunk_inner=13), mortie_order=13): 18 total shards indexed+pinned against 15 on the records path, which is the MULTI granule's 13 vs 8 you measured, and the indexed+pinned assignment is identical to the unindexed+pinned one.
(a) Disclosed. build's Notes gains a paragraph — "A pinned mortie_order covers live whether or not the catalog is indexed" — naming it as a third population, carrying the 8 -> 13 number, and noting footprint_cells: False now sits beside a cover-derived assignment. _footprint_cells_plan's pin bullet says declining no longer means falling back to geometry. _live_cells_plan's pin paragraph says a pinned indexed build lands here, not on the geometry path it took pre-#445. In the PR body: delta 6, plus a correction under "Paths deliberately left on the records-first build" saying explicitly that this is not one of them.
(b) Tested — TestLiveCover::test_a_pinned_indexed_build_covers_live_too, three assertions: the indexed+pinned manifest serializes identically to the unindexed pinned build (dropping footprint_cells_order, which is the catalog's own metadata and legitimately only on the indexed side), it is a strict superset of the pre-#445 records path with the extra shards carrying MULTI, and footprint_cells is still False.
(c) Raised as question 4 with the alternative named as you'd expect it to be: (b) gate it — add getattr(catalog, "footprint_cells", None) is None to _live_cells_plan's engagement test so a pinned indexed build keeps main's records-path assignment. Tradeoff stated both ways (uniform "one intersection engine" semantics vs. preserving an existing population's assignment, at the price of "indexed" meaning two things depending on whether the caller pinned, and a pinned indexed build still paying the full record decode). Shipped as (a), flagged in the code as awaiting a ruling on this PR.
small fixes 2026-08-17 (2): s2_neon_o9 toc flip, CHANGELOG entry for PR #447
1 participant
Closes #445. Implements the plan comment as amended by espg in-session on 2026-08-16 (see Design change below).
Issue #439/PR #440 inverted the indexed path — intersect first, decode records for the hits only. The unindexed path kept the old order:
granule_records()over every row, then cover each footprint from its decoded rings. On the 555,867-granule ATL03 clone against California that was 86.9 s to assign 2,357 granules, ~2.5x the cost ofindex_footprints(9)+ an indexed query put together.This makes an unindexed mortie
swathbuild build-an-ephemeral-index-and-query-it: cover the WKB column once (mortie.arrow.from_wkbs, no records, no column), intersect positionally with_intersect_footprint_cells, then decode records for the hit rows only via_hit_records. "Indexed" now means only that the cover was persisted — one intersection engine, two ways of getting its input.Design change vs the plan comment: one stage, not two
The plan comment specified a two-stage coarse/fine cover, because the live path resolved its MOC order through
_resolve_mortie_order(chunk order — o13 for the shipped ATL03 grids) and a whole-catalog o13 cover is not materializable (~9–13 GB against ~270–420 MB at o9).espg challenged why o13 appears at all, and
_resolve_mortie_order's own docstring settles it: a shard map recordsparent_ordershard membership and nothing finer, and the real-catalog order sweep (bench/neon_order_sweep.py) quoted there measures granules/shard flat for every order >=parent_order— "a finer MOC buys precision the order-parent_ordershard cells can't see". So the cover runs atparent_order, the assignment is already exact, and the coarse/fine machinery is unnecessary. Implemented single-stage; the two-stage form was stripped.Consequence worth flagging: an unpinned unindexed HEALPix build now records
metadata["mortie_order"] = parent_orderwhere it recorded the chunk order before. Same assignment at the orders production runs, and a superset elsewhere — never a subset (delta 5 below): flat by measurement at 9/13, 11/13, 8/12 and 9/11, up to 0.71% extra cells at coarse pairs like 3/7, because mortie's coverage is conservative per order and refining a fine cover down does not reproduce a coarse one. Fewer cover words either way. An explicitmortie_order=is still honored literally and still validated againstparent_order(#92).Phases
Catalog.cover_footprints(order)insources.py(the ephemeral cover:granule_row_maskscreen + onefrom_wkbs, row-aligned ragged output).index_footprintsrefactored onto it, so the two cannot drift on the cover._live_cells_plan+ thebuildbranch inshardmap.py, plus docstring updates (buildNotes,_intersect_footprint_cells,_resolve_mortie_order).total_granules, records-for-hits-only,build_wall_s, the MultiPolygon superset.docs/api/catalog.md: what indexing buys now that both paths share the cover.filter_bboxcan cut a catalog to nothing; the cover has to land where_flatten_ringsdid) + the clone-scale measurement below.Where it lives
The new cover is
Catalog.cover_footprintsinsrc/zagg/catalog/sources.py, deliberately not inshardmap.py(past the 1,200-line raise trigger; the split proposal flagged on PR #440 is still standing). It isindex_footprintsminus the column write —index_footprintsnow calls it, so there is one cover implementation, one screen, one row-aligned scatter.shardmap.pystill grew, from 1,647 to 1,759 lines (+112, of which ~95 is docstring/comment:_live_cells_plan's rationale, thebuildNotes, the order-default note on_resolve_mortie_order). The executable addition is 11 lines of_live_cells_planbody plus a 5-line branch inbuild. Raised under "Questions for review" below.Measured (555,867-granule ATL03 clone,
demo_aoi("california"),HealpixGrid(9, 19, chunk_inner=13))Same machine, same catalog, same AOI; "before" is
origin/main(9538d24) run against this venv viaPYTHONPATH, "after" is this branch.build_wall_sis the manifest's own number (cover/plan + intersection); "total" is the wholeShardMap.buildcall, so the difference is the record decode.build_wall_sbuild()dd43dd5f55a0f002mortie_order=9dd43dd5f55a0f002dd43dd5f55a0f002mortie_order=9dd43dd5f55a0f002dd43dd5f55a0f002Every run: 555,867 records considered, 2,357 granules assigned, 2,726 shards, 191,219 pairs.
Two readings, because the pre-#445 default was not the pre-#445 demo:
mortie_order=9, whatdemo/01_query.ipynbanddata/build_aoi_shardmap.pypass) this is 86.6 s -> ~37 s, and 86.6 s reproduces the issue's 86.9 s measurement to within noise.mortie_order, which resolved to the chunk order 13 — it is 1075 s -> ~39 s, ~27x. That 1075 s is the cost espg's ruling removes: covering 555,867 quarter-orbit footprints at o13 (~9.3k MOC words each) to answer an order-9 question. Anyone who built unindexed without pinning the order was paying it.The record decode is now ~0.3-0.4 s of the build (39.3 - 38.9) instead of ~19 s (86.6 - 67.9): 2,357 rows decoded instead of 555,867.
Parity is exact at clone scale. The digest above is a sha256 over
[[shard_key, [granule ids...]], ...]of the whole manifest — every shard, every granule, in order. All five runs share one digest, so the new path reproduces the old one byte for byte on the real catalog (single-part footprints throughout), including across the order change:before, defaultcovered at 13 andafter, defaultcovers at 9, and they agree. That is the order-sweep invariant (granules/shardflat for order >=parent_order) verified at 555,867 granules, not just on fixtures — a measurement at this order pair, not an identity in general; see delta 5.Semantics deltas (deliberate, disclosed)
from_wkbscovers the union of the parts inside each blob;granule_recordsreads only the largest part's exterior ring. The unindexed path now inherits the stored index's documented superset semantics — a multi-part footprint can assign to shards it did not before. Every CMR ATL03/06 granule is single-part (the same framing the stored path ships with), so no production build moves. Pinned byTestLiveCover::test_multipolygon_is_a_superset_of_the_records_path.granule_row_maskraisesValueErrornaming the count and the first offending row where the record loop raisedAttributeErroronNone.is_empty. Both refuse — neither drops the granule silently — but the exception type on the unindexed mortie path changes. Pinned inTestDeferredRecords::test_null_geometry_refuses_on_both_paths, which also keeps theAttributeErrorpin on a record-decoding backend.mortie_ordermetadata on unpinned unindexed HEALPix builds: chunk order ->parent_order(above)._intersect_footprint_cellsrefuses the whole call where_intersect_mortieswallowed the refusal per ring and dropped that granule silently. That divergence was disclosed for the stored path in batch mortie coverage in ShardMap.build (issue #396) #400 and now reaches the unindexed one. It takes a single granule covering >1e6 shard cells inside the AOI; loud-over-silent is the intended trade. The refusal's remedy is path-aware (fold81cea28): the stored path still says re-index or drop the column, the live path says neither exists here and names what does — a coarserparent_order, or a narrower/split region — since the flattening target isparent_orderwhatevermortie_orderwas. Pinned byTestLiveCover::test_the_budget_refusal_names_a_remedy_that_exists.parent_ordercovers a superset. The order default (delta 3) is flat by measurement, not identical by construction. mortie's coverage is conservative per order, socover(parent_order)⊇moc_to_order(cover(chunk_order), parent_order): refining a fine cover down does not reproduce a coarse one, and a boundary cell the fine cover misses can survive at the coarse one. The direction never inverts — a true granule/shard pair is never dropped, only extra ones admitted, the same conservative posture as the MultiPolygon superset (delta 1) and the AOI overhang (Optional strict-AOI polygon clip for shardmap output (sub-shard clipping) #101). A 200-polygon randomized sweep per config measured 0/200 rows differing at parent/chunk 9/13, 11/13, 8/12 and 9/11 (the production pairs, and the clone digest above agrees at 9/13), against 1–2 rows and up to 0.71% extra cells at 6/10, 5/9 and 3/7. Pinned byTestLiveCover::test_coarse_shard_order_covers_a_superset(parent 6 / chunk 10: 4 shards vs 3, strict superset).mortie_orderputs an indexed catalog on the live cover too._footprint_cells_plandeclines a pin (a persisted column cannot restate an arbitrary order) — and what catches that decline is now_live_cells_plan, not the geometry path it fell to pre-Unindexed builds: cover from WKB and intersect before materializing records (#439 for the live path) #445. So an indexed catalog built withmortie_order=is a third population whose assignment can move, inheriting deltas 1 and 2: measured on a two-part MultiPolygon atHealpixGrid(11, 19, chunk_inner=13)withmortie_order=13, 8 shards onmain-> 13 here for the multi-part granule. The pin is honored either way andmetadata["footprint_cells"]still recordsFalse. This is a semantic choice the PR made implicitly rather than by design — raised as question 4 below for an explicit ruling. Pinned byTestLiveCover::test_a_pinned_indexed_build_covers_live_too.Not changed:
total_granulesis still the records considered (screen popcount == the oldlen(records)),granules_assigned, per-shard granule order, and the manifest JSON otherwise.Paths deliberately left on the records-first build
sibling_catalogpaired builds (pairing filters the record list, which positional alignment cannot represent — PR #432's resolution, and now pinned against both cover-first paths),footprint="beams"(the cover is the CMR swath, not the per-beam corridors of #65), the spherely backend (an exact-S2 run must not be silently swapped for a MOC one, espg/mortie#32), and rectilinear grids (no shard order for the cover to key to)._intersect_mortietherefore stays — it still serves beams, non-HEALPix grids, andShardMap.reproject's refine.A pinned
mortie_orderon an indexed catalog is not one of them (delta 6): it declined the stored plan pre-#445 and fell to geometry, and it declines the stored plan now and covers live. Question 4 asks whether that should be gated back.How tested
tests/test_shardmap.py::TestLiveCover(new, 14 tests). The oracle throughout is the path this replaces:monkeypatch.setattr(shardmap, "_live_cells_plan", lambda *a, **k: None)restores the pre-Unindexed builds: cover from WKB and intersect before materializing records (#439 for the live path) #445_intersect_mortiebuild exactly, with no build kwarg that exists only for tests (thepre_445_pathfixture wraps it).test_serializes_identically_to_the_pre_445_path— same catalog/grid/AOI -> the same manifest JSON, key for key, over a fixture with a screened non-polygonal row and an AOI that discards most records. The grid ischunk_inner=13, parent_order=11, so the oracle covers at 13 and this path at 11: the byte-equality is the in-suite check of the order-sweep invariant.test_default_order_is_the_shard_order— one cover, atparent_order, and nothing persisted.test_pinned_order_is_honored_and_matches_the_oracle/test_pinned_order_coarser_than_the_shards_still_refuses.test_total_granules_counts_the_screen(screened-row fixture, live == oracle == 6).test_records_are_decoded_for_the_hits_only— spies ongranule_records; exactly the assigned rows get decoded.test_build_wall_spans_the_cover_but_not_the_records— sleeps on both sides, asserts the cover is insidebuild_wall_sand the record decode is outside (today's convention, Stored-index query path: intersect before materializing granule records #439).test_multipolygon_is_a_superset_of_the_records_path,test_gated_off_for_the_paths_that_need_records(spherely / beams / rectilinear each fall through).test_coarse_shard_order_covers_a_superset(fold99db8d0) —HealpixGrid(6, 12, chunk_inner=10), one rectangle straddling an order-6 seam: the coarse cover's 4 shards strictly contain the chunk-order cover's 3. Pins the direction of delta 5, so a future change that made the default a subset fails here. The production pairs are flat, which is why the byte-equality test above cannot catch it.test_a_pinned_indexed_build_covers_live_too(fold11b35bc) — delta 6: an indexed catalog withmortie_order=13serializes identically to the unindexed pinned build (minus the catalog's ownfootprint_cells_orderkey), is a strict superset of the pre-Unindexed builds: cover from WKB and intersect before materializing records (#439 for the live path) #445 records path on a MultiPolygon, and still recordsfootprint_cells: False.test_the_budget_refusal_names_a_remedy_that_exists(fold81cea28) —mortie.mocs_to_orderspatched to refuse; the live build's message must not say "re-index"/"drop the column", and must nameparent_order 11and the coarser-grid escape, while the indexed build's message is unchanged.TestPairedAssetBuild::test_paired_build_skips_the_ephemeral_cover_too— a paired build fails the test if it plans.TestDeferredRecordsandTestFootprintCellsrun untouched apart from the two intended-divergence oracles (the MultiPolygon pair now compares againstpre_445_path, since the live path shares the cover) and the null-geometry type above.TestMortieOrder::test_default_keys_to_chunk_ordernow pins_resolve_mortie_order's contract directly and the build's recorded order beside it, so the two defaults cannot drift unnoticed.tests/test_lambda_build.py::TestFunctionBuild::test_function_build_succeeds, environmental and unrelated to this diff (build_function.sh'spip installcannot resolvezarr>=3.1.5in this sandbox: "Could not find a version that satisfies the requirement zarr>=3.1.5", the index only offering up to3.0.0a5for the interpreter it picks). It touches none of the four files this PR changes. Ruff check + format clean on all four; the one repo-wideruff check src testsfinding isN818onsrc/zagg/registry.py:64(UnknownCapability), pre-existing onorigin/mainand outside the PR-lint bot's--select=E,F,W,I.pre-commitmypy reports the same errors on these files before and after (all pre-existing, import-stub andbuild-local).08e75d5is green (test 3.12 + 3.13, ruff, lambda build, docs). The firsttest (3.12)attempt failed ontests/test_index.py::TestWorkerSeam::test_finish_granule_called_once_per_granule— a pre-existing flake, unrelated to this diff:process_shardreads granules through a thread pool, so the recordedfinish_granuleorder is nondeterministic, and the test assertscalls == ["s3://a", "s3://b"](CI saw["s3://b", "s3://a"]). Nothing here toucheszagg/processing; it passed on the previous headbb7cf7cand passes locally. Re-run only, not "fixed" — flagging it rather than touching it (CLAUDE.md §4).Questions for review
shardmap.pyis at 1,846 lines (1,759 before the review fold, which was ~85% prose). It entered this PR at 1,647 — already past the 1,200 raise trigger, with a split proposal standing from PR Stored-index query path: intersect before materializing granule records #440. The new logic went tosources.pyas directed and the executable addition here is ~25 lines; the rest is prose. Prefer it trimmed, or fold it into the standing split proposal?The
mortie_ordermetadata change (chunk order ->parent_order) is visible in every newly built unindexed manifest. Worth a note in the release notes, or is the docstring/PR record enough?footprint_cellsmetadata verdict. Kept meaning "the stored column answered this build", so an unindexed cover-and-intersect build records no key at all (and an indexed catalog built with a pinned order still recordsFalse). The alternative reading — "this build was answered by a cover" — would now be true for both, but it would stop distinguishing the thing the operator can act on (did the index they paid for get used).Should a pinned build on an indexed catalog stay on the live cover? (delta 6, and the one place this PR changed a semantic without deciding to.)
_footprint_cells_plandeclines a pin; pre-Unindexed builds: cover from WKB and intersect before materializing records (#439 for the live path) #445 that decline landed on_intersect_mortie, and now it lands on_live_cells_plan, moving the assignment (8 -> 13 shards on the MultiPolygon fixture). Two readings, and the PR currently ships (a):getattr(catalog, "footprint_cells", None) is Noneto_live_cells_plan's engagement test, so an indexed catalog with a pin keepsmain's records-path assignment. Preserves behavior for an existing population, but makes "indexed" mean two different things depending on whether the caller pinned, and leaves a pinned indexed build paying the full record decode the PR exists to remove.Left as-is pending your call — the fold disclosed and tested the behavior rather than changing it.