Fix upstream-dev CI job: zarr test overrides, stateful testing, upstream GHA#2294
Merged
Conversation
li-em
force-pushed
the
li-em/2026-07-upstream
branch
from
July 24, 2026 14:18
8c62e31 to
1646c4f
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2294 +/- ##
=======================================
Coverage 84.70% 84.70%
=======================================
Files 88 88
Lines 38995 38995
=======================================
Hits 33030 33030
Misses 5965 5965
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
li-em
force-pushed
the
li-em/2026-07-upstream
branch
from
July 24, 2026 15:21
91c55ae to
f81fdae
Compare
cdata_shape counts inner chunks for sharded arrays, but store keys and Session.shift_array operate on the outer (shard) grid. zarr main's strategies now generate sharded arrays, so the mismatched granularity made the model shuffle keys over a grid that doesn't exist (nightly upstream-dev failure in #1921).
V1 expiration rewrites only ref-reachable chains, so an unreachable commit's on-disk ancestry can still pass through expired snapshots the model dropped. Creating a ref there resurrects history the model no longer tracks, failing check_ancestry. Apply the same full-ancestry assume() that reset_branch already uses.
The model re-parented kept snapshots whose parent was GC'd to the initial snapshot, but delete_snapshots_from_repo_info re-parents over the deleted run to the nearest surviving ancestor. After reset_branch onto such a snapshot the two disagreed on reachability, so the next garbage_collect predicted the wrong snapshots_deleted count (nightly upstream-dev failure in #1921).
Cover storage_chunk_sizes on sharded and plain arrays, and a deterministic model-vs-store shift of a sharded array mirroring the shift_array stateful rule.
li-em
force-pushed
the
li-em/2026-07-upstream
branch
from
July 24, 2026 15:53
7c98bee to
f5edf95
Compare
li-em
force-pushed
the
li-em/2026-07-upstream
branch
from
July 24, 2026 16:12
f5edf95 to
a249452
Compare
ianhi
approved these changes
Jul 24, 2026
ianhi
left a comment
ianhi
left a comment
Collaborator
There was a problem hiding this comment.
little headspinning to thnk about GC once more - but all seems correct to me!
| # covers resetting to a commit whose ancestry *reaches* an expired-but- | ||
| # on-disk snapshot: the real ancestry resurrects it while the model has | ||
| # popped it, so skip unless the whole chain is still modelled. | ||
| # but modelling that divergence isn't worthwhile — just skip. |
Collaborator
There was a problem hiding this comment.
is this comment no longer true?
Member
Author
There was a problem hiding this comment.
it got moved to the _v1_chain_fully_modelled docstring
Comment on lines
+48
to
+49
| if hasattr(arr, "write_chunk_sizes"): | ||
| return arr.write_chunk_sizes # type: ignore[no-any-return] |
Collaborator
There was a problem hiding this comment.
Can we leave a comment about why we are usign write (outer) instead of read (inner) chunk sizes. I got pretty tripped up by this at first
Member
Author
There was a problem hiding this comment.
Expanded the docstring, it is indeed confusing 😵
li-em
enabled auto-merge
July 24, 2026 20:08
This was referenced Jul 24, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Couple of different problems detected in #1921 fixed on this PR:
scientific-python/issue-from-pytest-log-action was not uploading the issue body anymore, fixed upstream, bumped version here.
chore: remove tests for private tenuous zarr-python store methods #2175 removed our zarr tests overrides, but that leaves us in an inconsistent state while zarr 3.3 (or the current
main) is not released, and we bump our minimum supported zarr. Opted to checking if the_get_*exist instead of a version range check, and we can actually drop the overrides once we move newer zarr releasesstateful testing fixes
test_zarr/test_stateful.py: use the storage-key chunk grid inshift_array. The rule usedcdata_shape, which counts inner chunks for sharded arrays, while store keys andSession.shift_arrayoperate on the outer (shard) grid. zarr main's hypothesis strategies now generate sharded arrays, exposing this: the model shuffled keys over a grid that doesn't exist (e.g. producing0/c/1for a single-shard array). A newstorage_chunk_sizeshelper uses zarr'swrite_chunk_sizeswhen available and derives the grid fromshards or chunkson older zarr. Includes deterministic regression tests, verified against both zarr 3.1.6 and zarr nightly.test_stateful_repo_ops.py: match the model's GC re-parenting to Rust. The model re-parented kept snapshots whose parent was GC'd to the initial snapshot, butdelete_snapshots_from_repo_infore-parents over the deleted run to the nearest surviving ancestor. After areset_branchback onto such a snapshot, model and repo disagreed on reachability, and the nextgarbage_collectpredicted the wrongsnapshots_deletedcount. Triggered only when a GC deletes a middle run while older (ref-pointed) and newer (flushed_at-protected) snapshots both survive — hence the intermittent nightly appearances.test_stateful_repo_ops.py: guardcreate_branch/create_tagagainst V1 expired-ancestor chains. V1 expiration rewrites only ref-reachable chains, so an unreachable commit's on-disk ancestry can still pass through expired snapshots the model dropped; creating a ref there resurrects history the model no longer tracks and failscheck_ancestry. Applies the same full-ancestryassume()thatreset_branchalready uses.