[SashankBhamidi]

Closes #24, closes #27, closes #28.

Argument types are checked from the annotations now, so a wrong type fails the same way on both stores. Earlier delete_record("1") deleted record 1 on SQLite and did nothing in memory. Costs 1 to 8% on the big listing.

MemoryStore has _load() and _save() to override, for anyone who wants to pickle it or put it in an object database.

Reads don't wait for another thread's write anymore. Different from the issue though: reads share one SQLite connection instead of one per thread, because per-thread scans were 4 to 10x slower under the GIL. I also had to turn cache_spill off, otherwise SQLite locked reads out partway through a big write anyway. Catch is a large listing holds its changes in memory till it commits. Not in WAL mode, which doesn't have the problem.

Longest read during a 20,000 object listing went from ~2 s to 0.06 s. The listing itself is the same speed as main.

[read-the-docs-community]

Documentation build overview

📚 calendaring-sync | 🛠️ Build #35308332 | 📁 Comparing 866376a against latest (5c1567d)

  🔍 Preview build  

14 files changed · ± 14 modified

± Modified

• contribute.html
• explanation/design.html
• reference/constants.html
• reference/objects.html
• reference/replica.html
• reference/store.html
• _modules/calendaring_sync/replica.html
• _modules/calendaring_sync/objects/change.html
• _modules/calendaring_sync/objects/change_set.html
• _modules/calendaring_sync/objects/push.html
• and 4 more...

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[tobixen]

Disclaimer: AI-generated review below the line, generated by Claude Opus 5.5, from the prompt:

Do a review on PR #30 and post a comment

---

Nice work, and the benchmark numbers are convincing. All 680 unit tests pass locally on 1081bff. A few things worth a look before merging, roughly most important first:

1. An in-process read can now make a big listing fail. Before, a read in another thread waited for the write. Now a read that's open when the writer reaches COMMIT makes the writer wait up to timeout, and after that the whole listing is rolled back with StoreBusyError. test_a_read_held_open_past_the_timeout_makes_a_write_busy pins down exactly this. Reads in this process already take turns on _read_turn, so _commit (src/calendaring_sync/store/sqlite.py:447) could take that lock, with timeout, before COMMIT. That way only other processes can make a commit busy, as before.

2. cache_spill = OFF is also set in WAL mode (sqlite.py:381). In WAL, spilled pages go to the WAL file and don't lock readers out, so the extra memory buys nothing there. I checked a WAL file: the store still sets cache_spill to 0. Suggest checking PRAGMA journal_mode and turning spilling off only in rollback-journal mode. Even then the cost has no upper bound for scopes with large content such as attachments. That deserves a sentence in the news entry, not just the docstring.

3. Exact str/int refuses StrEnum/IntEnum (_checks.py:27). replica.session(Scope.WORK, ...) with a StrEnum now raises scope_id must be str, not Scope. #27 argues for this, but an enum is a common way to name scope ids. Either accept StrEnum and store str(value), or say in the news entry that subclasses are refused.

4. _read_turn.acquire() has no timeout (sqlite.py:443). A read queued behind another long read in this process waits forever, while the docs say a store waits timeout seconds for "another process or thread".

5. The MemoryStore doctest contradicts the paragraph above it (memory.py:86). The doctest's _load unpickles on every transaction. The text says a file-backed store reads once when it opens, leaves _load alone and calls super()._save(). _PickledStore in the tests follows the advice, so it would make the better example.

6. Minor: check_argument_types and test_every_entry_point_checks_its_types only look at inspect.isfunction. A public classmethod, staticmethod or property setter added later would go unchecked, and the test wouldn't notice. None exist today. The test could at least fail on public attributes it skips.

[SashankBhamidi]

Fixed 1, 2, 5 and 6.
Point 4 goes away with 1, since timeout now only covers other processes.
I'm keeping 3 as is: enums stay refused.