fix(store): run every LocalStore filesystem call off the event loop

[d-v-b]

AI-authored PR that does what it says. Keeps expensive LocalStore IO off the event loop with to_thread. I'm going to benchmark this to ensure that nothing insane happens.

🤖 AI text below 🤖

Summary

LocalStore.get and set already run their disk I/O in a worker thread via asyncio.to_thread, but nine other async methods did not: open/_open (and so the lazy open reached from get/set), clear, delete, delete_dir, list, list_prefix, list_dir, move and getsize all called pathlib/shutil/os directly from the event loop thread. Walking or modifying a directory tree therefore stalled every other task sharing the loop. Ruff's ASYNC rules cannot see these, since they cannot infer that self.root / key is a Path.

Each method now delegates to one small synchronous helper run under asyncio.to_thread. Exception types and ordering are unchanged. Two things fell out of that:

• Once _open suspends, concurrent lazy opens (for example from set_many) hit the ABC's "store is already open" error. LocalStore._ensure_open now verifies the root and sets the open flag directly, both of which are idempotent, so concurrent lazy opens cannot conflict; an explicit second _open() still raises as the conformance suite requires.
• list_prefix materializes its walk in the thread (as list already did) and uses str.removeprefix instead of str.replace, which also fixes a latent key-mangling case when the root path recurs inside a key.

Tests

tests/test_store/test_local.py gains an autouse detector fixture for the LocalStore conformance class: it patches the os and io entry points every pathlib/shutil helper bottoms out in and records a violation whenever one is reached on the event-loop thread from inside a _local.py coroutine. On unfixed main it flags all nine methods above. A companion test, parametrized over every async LocalStore method (open, lazy open, get, get_partial_values, set, set_if_not_exists, exists, getsize, delete, delete_dir, list, list_prefix, list_dir, clear, move), asserts the hooks really observe each method's I/O off-loop, so the detector cannot pass vacuously. Extending it beyond get/set showed that exists and getsize were invisible to the detector: they passed a bare Path method to to_thread, leaving no _local.py frame on the worker stack. Both now go through module helpers like the other methods. test_concurrent_lazy_open pins the race.

Full tests/ suite on the merged branch: 8787 passed, 1291 skipped, 4 xfailed. mypy, ruff and ruff-format clean.

Interaction with changes that landed on main since this was opened

Both touch the code this PR moves off the loop, so to spell out why neither is weakened:

• fix(store): retry the LocalStore rename when the destination is busy #4358 / refactor(store): polish the LocalStore rename retry from #4358 #4359 (retry the LocalStore rename when the destination is busy). _move_with_retry and _atomic_write are reached only from _put, which this branch already ran under asyncio.to_thread, so the retry's time.sleep blocks a worker thread rather than the event loop. This is checked rather than assumed: the detector patches os.rename, os.replace, os.link and os.unlink, which is exactly what those helpers bottom out in, so the new retry path is policed like everything else. The retry's own tests are module-level and therefore outside the class-scoped detector fixture, so the two do not interfere in either direction.
• fix: replace runtime assert statements with explicit checks #4363 (replace runtime asserts with explicit checks). The five redundant assert isinstance(key, str) it removed from _local.py stay removed; one of them sat on the line where this branch switches get to await self._ensure_open(), which is the only place the two needed reconciling. The S101 lint that PR enabled is satisfied, since the asserts this branch adds are all under tests/, which the rule excludes.

Benchmarks

CodSpeed on the pre-merge head: one benchmark improved 10.38% (test_sharded_morton_single_chunk[(30, 30, 30)-memory]), 112 untouched, no regressions — so the extra to_thread hops cost nothing measurable. The run against the merged head should repost on this push.

Split out of d-v-b#191, whose remaining content is lint-rule enablement.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.35%. Comparing base (5c37f97) to head (c4deeb2).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4353      +/-   ##
==========================================
+ Coverage   94.33%   94.35%   +0.01%     
==========================================
  Files          93       93              
  Lines       13127    13131       +4     
==========================================
+ Hits        12384    12390       +6     
+ Misses        743      741       -2     

Files with missing lines	Coverage Δ
src/zarr/storage/_local.py	98.59% <100.00%> (+0.97%)	⬆️

🚀 New features to boost your workflow:

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

[read-the-docs-community]

Documentation build overview

📚 zarr-indexing | 🛠️ Build #34546635 | 📁 Comparing 8d19392 against latest (1f13742)

  🔍 Preview build  

24 files changed · ± 24 modified

± Modified

• index.html
• api/index.html
• design-notes/index.html
• guide/index.html
• ndsel/index.html
• api/boundary/index.html
• api/chunk_resolution/index.html
• api/domain/index.html
• api/errors/index.html
• api/grid/index.html
• and 14 more...

[codspeed]

Merging this PR will improve performance by 10.38%

⚡ 1 improved benchmark
✅ 112 untouched benchmarks
⏩ 37 skipped benchmarks¹

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
⚡	test_sharded_morton_single_chunk[(30, 30, 30)-memory]	1.9 ms	1.8 ms	+10.38%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing d-v-b:fix/localstore-off-loop-io (2a89e10) with main (ba883a5)²}

Footnotes

1. 37 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

2. No successful run was found on main (1f13742) during the generation of this report, so ba883a5 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[ilan-gold]

Seems reasonable!