fix: auto mode by TheGreatAlgo · Pull Request #91 · dClimate/py-hamt

TheGreatAlgo · 2026-07-28T13:44:28Z

Summary by CodeRabbit

New Features
- Added an auto shard read mode, now the default.
- Automatically starts with efficient sparse reads and switches to full shard decoding when access patterns indicate scanning.
- Preserves read-mode detection across read-only and cloned store instances.
Bug Fixes
- Cache clearing and format migration now correctly reset automatic detection.
Tests
- Added coverage for scan detection, point reads, cache eviction, ranged reads, and cloned stores.

coderabbitai · 2026-07-28T13:44:45Z

Warning

Review limit reached

@TheGreatAlgo, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 38 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d7cd751c-bc74-4e9b-9276-caf76cc67647

📥 Commits

Reviewing files that changed from the base of the PR and between a49296e and f481bb8.

📒 Files selected for processing (2)

py_hamt/sharded_zarr_store.py
tests/test_sharded_zarr_store_v2.py

Walkthrough

ShardedZarrStore now defaults to an "auto" shard read mode that begins with sparse decoding and permanently switches to full decoding after repeated eligible reads. The state is shared across clones, reset during store changes, and covered by unit and benchmark tests.

Changes

Automatic shard reading

Layer / File(s)	Summary
Read-mode contract and state `py_hamt/sharded_zarr_store.py`	Adds `"auto"` to the public read-mode type, changes constructor and `open()` defaults, and introduces promotion counters and latch state.
Eligibility, latching, and lifecycle `py_hamt/sharded_zarr_store.py`	Updates shard read selection, shares auto-mode state across read-only clones, and resets it during clearing and migration.
Auto-mode behavior validation `tests/test_sharded_zarr_store_v2.py`, `tests/test_benchmark_stores.py`	Tests sparse and full transitions, threshold behavior, clone propagation, cache eviction, ineligible reads, full-mode isolation, benchmarks, and the new default.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

dClimate/py-hamt#68: Introduced the underlying ShardedZarrStore functionality extended by this change.
dClimate/py-hamt#87: Updated the sparse shard decoding path that this auto mode now controls.

Suggested reviewers: 0xswego

Poem

I’m a rabbit watching shards hop by,
Sparse at first beneath the sky.
When scans grow long, I flip the gate,
Full decoding takes the weight.
Clones share the trail I knew—
Tests confirm the path stays true.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title is related to the main change, but it is too vague to describe what "auto mode" actually does.	Use a more specific title such as "Add auto shard read mode to ShardedZarrStore".

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch auto-mode

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

da-code-reviewer

Codex Automated Review

Found 2 actionable performance issues.
Posted 2 inline comment(s).

da-code-reviewer · 2026-07-28T13:50:13Z

+        """
+        if self._full_mode_latched:
+            return False
+        count = self._sparse_read_counts.get(cache_key, 0) + 1


MEDIUM
The counter is scoped to one shard, so a full scan can never promote when chunks_per_shard < 256: every shard remains below the threshold even if the scan crosses thousands of shards, retaining the unbounded sparse-read penalty. The benchmark masks this by lowering the threshold to 8. Track scan-wide progress or cap the threshold based on the shard size.

thats intended. so sparse mode gets advantages across long time ranges

da-code-reviewer · 2026-07-28T13:50:13Z

+        # far but latches independently afterward — it has the opposite
+        # read/write posture, and a writable clone can never take the sparse
+        # path at all, so there is nothing for a shared latch to coordinate.
+        clone._full_mode_latched = self._full_mode_latched


MEDIUM
The counters and cache are shared across clones, but the latch is copied by value. After a read-only clone from a writable store latches and clears the shared counters, another with_read_only(True) clone from that writable parent inherits its stale False latch and resumes sparse reads for evicted shards. Keep the latch in shared mutable state so all clones observe promotion.

codecov-commenter · 2026-07-28T13:56:25Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (74ef73d) to head (f481bb8).

Additional details and impacted files

@@            Coverage Diff            @@
##              main       #91   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines         3168      3198   +30     
=========================================
+ Hits          3168      3198   +30

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

da-code-reviewer

Codex Automated Review

Found 2 actionable performance issues.
Posted 2 inline comment(s).

da-code-reviewer · 2026-07-28T14:14:44Z

+            return False
+        count = self._sparse_read_counts.get(cache_key, 0) + 1
+        if count >= self._SPARSE_PROMOTE_THRESHOLD:
+            self._full_mode_latched = True


MEDIUM
_sparse_read_counts grows once per distinct shard and is never bounded or pruned unless the entire store latches or is cleared. The intended long-range point-read workload may touch millions of shards without promotion, causing persistent memory growth outside the bounded LRU cache. Use a bounded/LRU counter map or discard inactive shard counters.

very unlikely. shards are limited to 1000.

da-code-reviewer · 2026-07-28T14:14:44Z

        self._pending_shard_loads.clear()
        await self._shard_data_cache.clear()
+        # A cleared store has a new access pattern to learn.
+        self._sparse_read_counts.clear()


MEDIUM
This reset only runs for v2 stores. clear() returns through Store.clear() for v1, although v1 also receives the new default auto mode, so a cleared v1 store retains its latch and stale counters. Subsequent reads can remain permanently in full mode or promote early; reset auto state in the common/v1 clear path as well.

don't worry about v1

fix: auto mode

a49296e

da-code-reviewer Bot reviewed Jul 28, 2026

View reviewed changes

fix: improve latching

f481bb8

da-code-reviewer Bot reviewed Jul 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: auto mode - #91

fix: auto mode#91
TheGreatAlgo wants to merge 2 commits into
mainfrom
auto-mode

TheGreatAlgo commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

Review limit reached

❌ Failed checks (1 inconclusive)

Uh oh!

da-code-reviewer Bot left a comment

Uh oh!

da-code-reviewer Bot Jul 28, 2026

Uh oh!

TheGreatAlgo Jul 28, 2026

Uh oh!

da-code-reviewer Bot Jul 28, 2026

Uh oh!

codecov-commenter commented Jul 28, 2026 •

edited

Loading

Uh oh!

da-code-reviewer Bot left a comment

Uh oh!

da-code-reviewer Bot Jul 28, 2026

Uh oh!

TheGreatAlgo Jul 28, 2026

Uh oh!

da-code-reviewer Bot Jul 28, 2026

Uh oh!

TheGreatAlgo Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

TheGreatAlgo commented Jul 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

❌ Failed checks (1 inconclusive)

Uh oh!

da-code-reviewer Bot left a comment

Choose a reason for hiding this comment

Codex Automated Review

Uh oh!

da-code-reviewer Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

TheGreatAlgo Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

da-code-reviewer Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

da-code-reviewer Bot left a comment

Choose a reason for hiding this comment

Codex Automated Review

Uh oh!

da-code-reviewer Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

TheGreatAlgo Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

da-code-reviewer Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

TheGreatAlgo Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TheGreatAlgo commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

codecov-commenter commented Jul 28, 2026 •

edited

Loading