Feat/async backends

[PythonFZ]

• async backends
• mongo and redis backends
• restructure to

flowchart LR
    %% Define Styles (Colors)
    classDef ui fill:#dbeafe,stroke:#2563eb,stroke-width:2px,color:#000
    classDef protocol fill:#f3e8ff,stroke:#9333ea,stroke-width:2px,color:#000
    classDef adapter fill:#dcfce7,stroke:#16a34a,stroke-width:2px,color:#000
    classDef utility fill:#ffedd5,stroke:#ea580c,stroke-width:2px,color:#000

    BlobReadBackend --> BlobIO
    AsyncBlobReadBackend --> AsyncBlobIO

    BlobReadWriteBackend --> BlobIO
    AsyncBlobReadWriteBackend --> AsyncBlobIO

    ObjectReadBackend --> ObjectIO
    AsyncObjectReadBackend --> AsyncObjectIO

    ObjectReadWriteBackend --> ObjectIO
    AsyncObjectReadWriteBackend --> AsyncObjectIO

    %% If there is no AsyncBackend we use a wrapper
    BlobIO --> SyncToAsyncAdapter --> AsyncBlobIO
    ObjectIO --> SyncToAsyncAdapter --> AsyncObjectIO

    %% convienience ASE Interface
    ObjectIO --> ASEIO
    AsyncObjectIO --> AsyncASEIO

    %% Existing Backends
    LMDBReadWrite --> BlobReadWriteBackend
    HFRead --> ObjectReadBackend
    ZarrReadWrite --> BlobReadWriteBackend
    H5MDReadWrite --> ObjectReadWriteBackend
    %% special case, because ASE XYZ does result in ASE atoms directly
    XYZRead --> ASEIO

    %% New Backends
    RedisReadWrite --> BlobReadWriteBackend
    AsyncRedisReadWrite --> AsyncBlobReadWriteBackend
    MongoDBReadWrite --> ObjectReadWriteBackend
    AsyncMongoDBReadWrite --> AsyncObjectReadWriteBackend

%% Apply Styles to Nodes
    class BlobIO,AsyncBlobIO,AsyncObjectIO,ObjectIO,ASEIO,AsyncASEIO ui
    class BlobReadBackend,AsyncBlobReadBackend,BlobReadWriteBackend,AsyncBlobReadWriteBackend,ObjectReadBackend,AsyncObjectReadBackend,ObjectReadWriteBackend,AsyncObjectReadWriteBackend protocol
    class LMDBReadWrite,HFRead,ZarrReadWrite,H5MDReadWrite,RedisReadWrite,AsyncRedisReadWrite,MongoDBReadWrite,AsyncMongoDBReadWrite,XYZRead adapter
    class SyncToAsyncAdapter utility

Loading

Summary by CodeRabbit

• New Features

  • Async I/O facades, adapters, and new backends (MongoDB, Redis, LMDB variants, in-memory) with sync/async usage and column/row views.

• Refactor

  • Unified read/write API and consistent method names (e.g. get/get_many), richer typed views and ASE atom support.

• Documentation

  • Major README and design updates describing multi-layer I/O model, backends, URI schemes, and facade API.

• Tests

  • Large new sync/async unit and benchmark suites covering backends, adapters, views.

• Chores

  • CI now provides Redis and MongoDB services; version and dependency updates.

[coderabbitai]

Actionable comments posted: 11

♻️ Duplicate comments (5)

tests/test_async_blob_io.py (1)

96-96: ⚠️ Potential issue | 🟡 Minor

Drop unused fixture argument from test_await_single_row.

raw_backend is injected but never used in this test.

Suggested fix

-    async def test_await_single_row(self, io, raw_backend):
+    async def test_await_single_row(self, io):

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_async_blob_io.py` at line 96, The test function
test_await_single_row declares an unused fixture parameter raw_backend; remove
raw_backend from the async def test_await_single_row signature so the fixture is
not injected (leave the remaining parameters such as io intact), and run tests
to confirm no other references to raw_backend exist in that test.

src/asebytes/_views.py (1)

300-303: ⚠️ Potential issue | 🟠 Major

Handle placeholder rows in multi-key to_dict() path.

_read_rows() can yield None, but this branch unconditionally reads row[k], which will crash.

Suggested fix

         result: dict[str, list[Any]] = {k: [] for k in self._keys}
         for row in self._parent._read_rows(indices, keys=self._keys):
+            row = row or {}
             for k in self._keys:
-                result[k].append(row[k])
+                result[k].append(row.get(k))
         return result

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_views.py` around lines 300 - 303, The loop in the to_dict path
reads rows from self._parent._read_rows(indices, keys=self._keys) but assumes
row is never None; update the loop in the method that builds result (the code
using self._parent._read_rows and result[k].append(row[k])) to guard against
placeholder rows by checking "if row is None" and handling it consistently
(either skip the row or append None for each key in self._keys) so row[k] is
never accessed on None; ensure the fix references the same symbols (_read_rows,
self._keys, result) and preserves ordering/length semantics expected by to_dict.

src/asebytes/_async_views.py (1)

312-313: ⚠️ Potential issue | 🟠 Major

Guard None before row.get(...) in AsyncSingleColumnView._materialize().

If the backend returns a placeholder row, this path raises AttributeError.

Suggested fix

         if self._single:
             values = await self._parent._read_column(self._keys[0], [idx])
             return values[0]
         row = await self._parent._read_row(idx, keys=self._keys)
+        if row is None:
+            return [None] * len(self._keys)
         return [row.get(k) for k in self._keys]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_views.py` around lines 312 - 313, In
AsyncSingleColumnView._materialize(), guard against a None/placeholder row
returned by await self._parent._read_row(idx, keys=self._keys) before calling
row.get(...) by checking the row value and returning None (or the appropriate
default) for each key when row is falsy; e.g., replace the direct comprehension
[row.get(k) for k in self._keys] with a branch that yields [row.get(k) if row is
not None else None for k in self._keys] so calls to row.get are only made when
row is valid.

src/asebytes/_object_io.py (1)

224-230: ⚠️ Potential issue | 🟠 Major

Context-manager delegation is still incomplete.

ObjectIO now calls close(), but it still skips backend __enter__/__exit__ semantics. Backends with transactional/exception-aware context handling can still be mismanaged.

💡 Proposed fix

     def __enter__(self):
+        enter = getattr(self._backend, "__enter__", None)
+        if callable(enter):
+            enter()
         return self

     def __exit__(self, exc_type, exc_val, exc_tb):
-        if hasattr(self._backend, "close"):
-            self._backend.close()
+        exit_ = getattr(self._backend, "__exit__", None)
+        if callable(exit_):
+            return bool(exit_(exc_type, exc_val, exc_tb))
+        close = getattr(self._backend, "close", None)
+        if callable(close):
+            close()
         return False

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_object_io.py` around lines 224 - 230, ObjectIO's
context-manager methods must delegate to the underlying backend's context
semantics instead of only calling close(); update __enter__ to call and return
the backend's __enter__() when present (and still return self if backend doesn't
implement __enter__), and update __exit__ to call backend.__exit__(exc_type,
exc_val, exc_tb) when present and return its result; only fall back to calling
backend.close() and returning False when the backend lacks __exit__. Reference
the ObjectIO methods __enter__ and __exit__ and the backend's __enter__/__exit__
symbols when making the change.

src/asebytes/lmdb/_blob_backend.py (1)

236-239: ⚠️ Potential issue | 🟠 Major

keys=... reads are still strict, not projection-friendly.

Raising on missing projected keys makes sparse rows fail unexpectedly and breaks callers that rely on partial-row projection behavior.

💡 Proposed fix

-        if keys_set is not None and len(result) != len(keys_set):
-            retrieved_keys = set(result.keys())
-            invalid_keys = keys_set - retrieved_keys
-            raise KeyError(f"Invalid keys at index {index}: {sorted(invalid_keys)}")
+        # Projection semantics: return only keys that are present.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/lmdb/_blob_backend.py` around lines 236 - 239, The code
currently raises KeyError when some requested projection keys are missing
(keys_set, result, invalid_keys, KeyError at index), which makes reads strict;
instead, stop raising and make projection-friendly by populating any missing
keys from keys_set with a sentinel (e.g., None) or omitting them per the
function's projection contract so sparse rows succeed; locate the block using
keys_set/result/invalid_keys in _blob_backend.py and replace the raise with
logic that fills missing keys into result (or silently ignores them) so callers
receive a consistent partial-row projection.

🧹 Nitpick comments (7)

src/asebytes/mongodb/_async_backend.py (1)

206-238: Past review issue addressed: Race condition on sort-key allocation is now fixed via atomic find_one_and_update with $inc.

The atomic reservation pattern using ReturnDocument.BEFORE correctly prevents concurrent writers from allocating duplicate sort keys.

Minor: Consider adding strict=True to the zip() call for defensive programming.

Optional: Add strict=True to zip

-        docs = [self._row_to_doc(sk, v) for sk, v in zip(new_sks, values)]
+        docs = [self._row_to_doc(sk, v) for sk, v in zip(new_sks, values, strict=True)]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/mongodb/_async_backend.py` around lines 206 - 238, In extend(),
when pairing new_sks with values to build docs, use zip in strict mode to ensure
lengths match; update the docs creation that currently zips new_sks and values
(the list comprehension using self._row_to_doc over sk, v) to call zip(new_sks,
values, strict=True) so a mismatch raises immediately rather than silently
truncating.

tests/test_h5md_backend.py (1)

1099-1102: Assert the short-column expectation explicitly.

This test documents the key expectation but doesn’t assert it. Add explicit checks so regressions are caught.

Suggested assertions

         # The short column should only appear in rows where it exists
         # (indices 0 and 1)
         # Note: after the fix, rows[2] should NOT have calc.short_col
+        assert "calc.short_col" in rows[0]
+        assert "calc.short_col" in rows[1]
+        assert "calc.short_col" not in rows[2]
         io2.close()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_h5md_backend.py` around lines 1099 - 1102, Add explicit assertions
that verify the short column appears only for the first two rows: for rows[0]
and rows[1] assert that their calc objects have the attribute short_col (e.g.,
using hasattr(rows[i].calc, "short_col") or equivalent) and for rows[2] assert
that its calc does not have short_col (negate the same check); place these
assertions near the existing comment (before or immediately after io2.close())
so the test will fail if the short-column presence regresses.

tests/test_blob_native_bytes.py (2)

376-428: Consider consolidating duplicate test methods or differentiating them.

TestAsyncBlobIONativeBytesAlternate contains test methods (test_column_view_from_bytes_key, test_column_view_int_index, test_single_row_bytes_key, test_list_bytes_keys) that are identical to those in TestAsyncBlobIONativeBytes. If these are meant to test alternate scenarios, consider:

1. Adding distinguishing test data or assertions, or
2. Removing the duplicate class entirely if the original coverage is sufficient.

Currently, these run the same assertions twice without added value.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_blob_native_bytes.py` around lines 376 - 428, The tests in
TestAsyncBlobIONativeBytesAlternate (methods test_column_view_from_bytes_key,
test_column_view_int_index, test_single_row_bytes_key, test_list_bytes_keys) are
duplicates of TestAsyncBlobIONativeBytes; either remove the redundant
TestAsyncBlobIONativeBytesAlternate class or modify its methods to exercise
alternate scenarios (e.g., different backend data, edge cases, or additional
assertions) so they provide unique coverage; update or delete the class
accordingly to eliminate the duplicate assertions.

---

24-24: Unused tmp_path fixture parameters.

The tmp_path fixture is declared but not used in test_column_view_from_bytes_key and test_column_view_int_index. Consider removing these parameters since the tests use in-memory backends.

♻️ Proposed fix

-    def test_column_view_from_bytes_key(self, tmp_path):
+    def test_column_view_from_bytes_key(self):

-    def test_column_view_int_index(self, tmp_path):
+    def test_column_view_int_index(self):

Also applies to: 84-84

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_blob_native_bytes.py` at line 24, Remove the unused tmp_path
pytest fixture from the test functions to avoid unused-parameter warnings: edit
the function definitions for test_column_view_from_bytes_key and
test_column_view_int_index to drop the tmp_path parameter so they only accept
the fixtures they actually use (e.g., self or any other used params), ensuring
no other test logic relies on tmp_path.

src/asebytes/_async_object_io.py (1)

216-220: Consider batched iteration for large datasets.

The current __aiter__ fetches rows one at a time via await self._backend.get(i). For large datasets over network backends (MongoDB, Redis), this could be inefficient. Consider using get_many with batching similar to chunked() patterns elsewhere in the codebase.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_object_io.py` around lines 216 - 220, The async iterator
__aiter__ currently awaits _backend.get(i) per-row which is inefficient for
large/remote backends; change it to iterate in batches by calling _backend.len()
once, then use chunked ranges (e.g., batch_size) to call
_backend.get_many(start, end) (or equivalent) and yield _build_result for each
row in the returned batch; ensure you preserve ordering and handle the final
partial batch and any backends that lack get_many by falling back to per-item
_backend.get.

src/asebytes/_async_io.py (2)

36-68: Consider extracting shared initialization logic.

The __init__ implementation is nearly identical to AsyncObjectIO.__init__. Consider extracting a shared base class or mixin to reduce duplication, especially as more async IO classes may be added.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_io.py` around lines 36 - 68, The __init__ body duplicates
AsyncObjectIO.__init__; extract the shared backend-initialization into a small
helper (mixin or base) and call it from both classes: create e.g.
AsyncBackendInitializer with a method like _init_async_backend(backend,
readonly, **kwargs) that contains the
parse_uri/get_async_backend_cls/from_uri/cls instantiation logic and uses
AsyncReadBackend, sync_to_async, etc., then replace AsyncObjectIO.__init__ and
this class's __init__ to delegate to that helper (retaining type checks and
assignment to self._backend).

---

284-288: Same batching consideration applies here.

As noted for AsyncObjectIO, consider batched iteration for large datasets to reduce network round-trips.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_io.py` around lines 284 - 288, The async iterator
__aiter__ currently awaits _backend.len() then calls _backend.get(i) per item
causing many round-trips; change it to fetch items in batches (e.g., use a batch
API like _backend.get_many(ids) or _backend.get_range(start, end) if available)
by iterating over ranges of size batch_size and awaiting a single backend call
per batch, then yield _build_result(row) for each row from the batch; if the
backend has no batch API, implement a small internal batching loop that gathers
concurrent gets with asyncio.gather to reduce round-trips and keep using
_build_result for each returned row.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/asebytes/_adapters.py`:
- Around line 259-263: The ObjectToBlobReadWriteAdapter is missing a drop_keys
method implemented on BlobToObjectReadWriteAdapter; add a drop_keys(self, keys:
Iterable[str]) -> None method on ObjectToBlobReadWriteAdapter that delegates to
the underlying store (use self._store.drop_keys(keys) if the wrapped backend
provides drop_keys, otherwise iterate the keys and call self._store.remove(key)
or self.remove semantics) so callers can drop multiple keys through the adapter;
place the method alongside clear/remove in the ObjectToBlobReadWriteAdapter
class and match the signature/behavior used by BlobToObjectReadWriteAdapter.

In `@src/asebytes/_async_views.py`:
- Around line 239-244: Ensure AsyncRowView.set validates the input before
writing: in async def set(self, data: list[Any]) validate that data is actually
a list (raise TypeError if not) and that len(data) matches len(self._indices)
when self._indices is truthy (raise ValueError on mismatch) so updates aren’t
silently truncated; perform this validation before the contiguous check that
calls _is_contiguous and then proceed to call
self._parent._write_many(self._indices[0], data) or loop to
self._parent._write_row(idx, d) as existing.
- Around line 706-715: _in _DeferredSliceRowView.__getitem__, the current check
only returns a _DeferredSliceColumnView when cv_cls is exactly AsyncColumnView,
causing deferred slicing to be lost for AsyncASEColumnView; update the
conditional to treat AsyncASEColumnView (or any subclass meant to preserve
deferred evaluation) the same as AsyncColumnView so that when
self._column_view_cls is AsyncASEColumnView the method returns
_DeferredSliceColumnView(self._parent, key, self._slice) for both string/bytes
and list-of-strings keys; reference the symbols
_DeferredSliceRowView.__getitem__, self._column_view_cls, AsyncColumnView,
AsyncASEColumnView, and _DeferredSliceColumnView when making the change.

In `@src/asebytes/_blob_io.py`:
- Around line 230-234: BlobIO's context methods currently do nothing and thus
leak backend resources; update BlobIO.__enter__ to call and return
backend.__enter__() if present (or return self if backend has no __enter__), and
update BlobIO.__exit__ to delegate to backend.__exit__(exc_type, exc_val,
exc_tb) if available and always call backend.close() (or backend.close_async()
as appropriate) to release handles; ensure safe checks for attribute existence
on the backend so BlobIO.__enter__/__exit__ work for backends that lack these
methods and still close resources on both normal exit and exceptions.

In `@src/asebytes/_registry.py`:
- Around line 303-335: The fallback currently always returns a read-write
adapter when readonly is None which can wrap a read-only object backend; update
the logic in get_blob_backend_cls (the code that calls get_backend_cls and
constructs ObjectToBlobReadAdapter / ObjectToBlobReadWriteAdapter using obj_cls
or obj_cls.from_uri) so that when readonly is None you inspect the resolved
obj_cls for writable capability (e.g., a well-defined attribute or method
indicating write support, such as a `writable`/`readable` flag or presence of a
`write`/`open` method) and only return a factory that constructs
ObjectToBlobReadWriteAdapter if obj_cls is write-capable; otherwise return the
ObjectToBlobReadAdapter factory—do this for both URI-based (`obj_cls.from_uri`)
and direct-construct paths.

In `@src/asebytes/_views.py`:
- Around line 375-399: Calls to dict_to_atoms in ASEColumnView (e.g., in
__iter__ and the __getitem__ branch that calls self._parent._read_row or
_read_rows) currently pass through placeholder None rows; update those sites to
check for None after reading (from _read_row/_read_rows) and either raise a
clear, specific error (e.g., ValueError("placeholder row encountered") or a
custom exception) or skip/propagate placeholders consistently per library
semantics, so dict_to_atoms is never invoked with None; reference the methods
dict_to_atoms, ASEColumnView.__iter__, ASEColumnView.__getitem__,
_parent._read_row and _parent._read_rows and add the guard immediately after the
read operations before calling dict_to_atoms.

In `@src/asebytes/h5md/_backend.py`:
- Around line 469-478: The set() method currently assumes data is a dict and
does data.items(), causing an AttributeError if None is passed; update set()
(which calls _check_index, _find_dataset_path, _serialize_value, _pad_value and
writes to ds) to explicitly handle None by returning early (no-op) or by raising
a clear TypeError; add a guard at the top of set(): if data is None: return (or
raise TypeError("data must be a dict, got None")), and also validate
isinstance(data, dict) before iterating to provide a clear error message instead
of an AttributeError.

In `@src/asebytes/memory/_backend.py`:
- Around line 41-49: The delete() and insert() methods on the in-memory backend
currently rely on Python list semantics (allowing negative indices and oversized
inserts), causing inconsistent behavior vs get()/set(); update delete(self,
index) and insert(self, index, value) to validate index bounds exactly like
get()/set(): raise IndexError for negative indices or index >= len(self._data)
for delete, and for insert only allow 0 <= index <= len(self._data) (or raise
IndexError for negatives and index > len), ensuring no implicit
negative-indexing or silent appends; use the same validation helper or logic
used by get()/set() to keep parity with other backend operations.

In `@src/asebytes/mongodb/_async_backend.py`:
- Around line 377-388: The close() method currently creates an unreferenced task
and silently skips cleanup outside an event loop; modify close() to (1) when a
running loop exists, store the created task on self._close_task (or reuse if
existing and pending), attach a done-callback to surface exceptions, and avoid
creating multiple orphan tasks; (2) when no running loop (except RuntimeError),
call asyncio.run(self._client.aclose()) to perform synchronous cleanup;
reference the method close(), the async client method aclose()/close() on
self._client, and use a new instance attribute self._close_task to track the
background task.

In `@src/asebytes/redis/_backend.py`:
- Around line 145-166: The from_uri parser in src/asebytes/redis/_backend.py
(the block that splits uri by "://" into scheme/after_scheme, then
host_part/path_part and builds connection_url and prefix) is brittle and drops
query/auth/fragment parts; replace the manual splitting with
urllib.parse.urlsplit/urlunsplit: use urlsplit(uri) to get
scheme/netloc/path/query/fragment, extract path components to derive db (use a
sensible default like "0" or keep existing default behavior) and prefix (second
path segment or "default"), then rebuild connection_url with urlunsplit((scheme,
netloc, f"/{db}", query, fragment)) and return cls(url=connection_url,
prefix=prefix, **kwargs) so auth, ports, query params and fragments are
preserved.

In `@tests/test_lmdb_backend.py`:
- Line 179: The test function test_get_column_with_missing_key includes an
unused fixture parameter sample_row; remove sample_row from the function
signature so it becomes def test_get_column_with_missing_key(backend): and
update any references if present (there should be none), then run tests/lint to
confirm the unused-argument warning is resolved.

---

Duplicate comments:
In `@src/asebytes/_async_views.py`:
- Around line 312-313: In AsyncSingleColumnView._materialize(), guard against a
None/placeholder row returned by await self._parent._read_row(idx,
keys=self._keys) before calling row.get(...) by checking the row value and
returning None (or the appropriate default) for each key when row is falsy;
e.g., replace the direct comprehension [row.get(k) for k in self._keys] with a
branch that yields [row.get(k) if row is not None else None for k in self._keys]
so calls to row.get are only made when row is valid.

In `@src/asebytes/_object_io.py`:
- Around line 224-230: ObjectIO's context-manager methods must delegate to the
underlying backend's context semantics instead of only calling close(); update
__enter__ to call and return the backend's __enter__() when present (and still
return self if backend doesn't implement __enter__), and update __exit__ to call
backend.__exit__(exc_type, exc_val, exc_tb) when present and return its result;
only fall back to calling backend.close() and returning False when the backend
lacks __exit__. Reference the ObjectIO methods __enter__ and __exit__ and the
backend's __enter__/__exit__ symbols when making the change.

In `@src/asebytes/_views.py`:
- Around line 300-303: The loop in the to_dict path reads rows from
self._parent._read_rows(indices, keys=self._keys) but assumes row is never None;
update the loop in the method that builds result (the code using
self._parent._read_rows and result[k].append(row[k])) to guard against
placeholder rows by checking "if row is None" and handling it consistently
(either skip the row or append None for each key in self._keys) so row[k] is
never accessed on None; ensure the fix references the same symbols (_read_rows,
self._keys, result) and preserves ordering/length semantics expected by to_dict.

In `@src/asebytes/lmdb/_blob_backend.py`:
- Around line 236-239: The code currently raises KeyError when some requested
projection keys are missing (keys_set, result, invalid_keys, KeyError at index),
which makes reads strict; instead, stop raising and make projection-friendly by
populating any missing keys from keys_set with a sentinel (e.g., None) or
omitting them per the function's projection contract so sparse rows succeed;
locate the block using keys_set/result/invalid_keys in _blob_backend.py and
replace the raise with logic that fills missing keys into result (or silently
ignores them) so callers receive a consistent partial-row projection.

In `@tests/test_async_blob_io.py`:
- Line 96: The test function test_await_single_row declares an unused fixture
parameter raw_backend; remove raw_backend from the async def
test_await_single_row signature so the fixture is not injected (leave the
remaining parameters such as io intact), and run tests to confirm no other
references to raw_backend exist in that test.

---

Nitpick comments:
In `@src/asebytes/_async_io.py`:
- Around line 36-68: The __init__ body duplicates AsyncObjectIO.__init__;
extract the shared backend-initialization into a small helper (mixin or base)
and call it from both classes: create e.g. AsyncBackendInitializer with a method
like _init_async_backend(backend, readonly, **kwargs) that contains the
parse_uri/get_async_backend_cls/from_uri/cls instantiation logic and uses
AsyncReadBackend, sync_to_async, etc., then replace AsyncObjectIO.__init__ and
this class's __init__ to delegate to that helper (retaining type checks and
assignment to self._backend).
- Around line 284-288: The async iterator __aiter__ currently awaits
_backend.len() then calls _backend.get(i) per item causing many round-trips;
change it to fetch items in batches (e.g., use a batch API like
_backend.get_many(ids) or _backend.get_range(start, end) if available) by
iterating over ranges of size batch_size and awaiting a single backend call per
batch, then yield _build_result(row) for each row from the batch; if the backend
has no batch API, implement a small internal batching loop that gathers
concurrent gets with asyncio.gather to reduce round-trips and keep using
_build_result for each returned row.

In `@src/asebytes/_async_object_io.py`:
- Around line 216-220: The async iterator __aiter__ currently awaits
_backend.get(i) per-row which is inefficient for large/remote backends; change
it to iterate in batches by calling _backend.len() once, then use chunked ranges
(e.g., batch_size) to call _backend.get_many(start, end) (or equivalent) and
yield _build_result for each row in the returned batch; ensure you preserve
ordering and handle the final partial batch and any backends that lack get_many
by falling back to per-item _backend.get.

In `@src/asebytes/mongodb/_async_backend.py`:
- Around line 206-238: In extend(), when pairing new_sks with values to build
docs, use zip in strict mode to ensure lengths match; update the docs creation
that currently zips new_sks and values (the list comprehension using
self._row_to_doc over sk, v) to call zip(new_sks, values, strict=True) so a
mismatch raises immediately rather than silently truncating.

In `@tests/test_blob_native_bytes.py`:
- Around line 376-428: The tests in TestAsyncBlobIONativeBytesAlternate (methods
test_column_view_from_bytes_key, test_column_view_int_index,
test_single_row_bytes_key, test_list_bytes_keys) are duplicates of
TestAsyncBlobIONativeBytes; either remove the redundant
TestAsyncBlobIONativeBytesAlternate class or modify its methods to exercise
alternate scenarios (e.g., different backend data, edge cases, or additional
assertions) so they provide unique coverage; update or delete the class
accordingly to eliminate the duplicate assertions.
- Line 24: Remove the unused tmp_path pytest fixture from the test functions to
avoid unused-parameter warnings: edit the function definitions for
test_column_view_from_bytes_key and test_column_view_int_index to drop the
tmp_path parameter so they only accept the fixtures they actually use (e.g.,
self or any other used params), ensuring no other test logic relies on tmp_path.

In `@tests/test_h5md_backend.py`:
- Around line 1099-1102: Add explicit assertions that verify the short column
appears only for the first two rows: for rows[0] and rows[1] assert that their
calc objects have the attribute short_col (e.g., using hasattr(rows[i].calc,
"short_col") or equivalent) and for rows[2] assert that its calc does not have
short_col (negate the same check); place these assertions near the existing
comment (before or immediately after io2.close()) so the test will fail if the
short-column presence regresses.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 866d021 and 150cce0.

📒 Files selected for processing (29)

• .gitignore
• src/asebytes/_adapters.py
• src/asebytes/_async_io.py
• src/asebytes/_async_object_io.py
• src/asebytes/_async_views.py
• src/asebytes/_blob_io.py
• src/asebytes/_object_io.py
• src/asebytes/_registry.py
• src/asebytes/_views.py
• src/asebytes/h5md/_backend.py
• src/asebytes/lmdb/_backend.py
• src/asebytes/lmdb/_blob_backend.py
• src/asebytes/memory/_backend.py
• src/asebytes/mongodb/_async_backend.py
• src/asebytes/redis/_backend.py
• tests/test_ase_column_views.py
• tests/test_async_blob_io.py
• tests/test_async_protocols.py
• tests/test_blob_column_access.py
• tests/test_blob_native_bytes.py
• tests/test_blob_negative_index.py
• tests/test_cache_to.py
• tests/test_column_writes.py
• tests/test_h5md_backend.py
• tests/test_lmdb_backend.py
• tests/test_mongodb.py
• tests/test_protocols.py
• tests/test_protocols_new_methods.py
• tests/test_review_critical_fixes.py

🚧 Files skipped from review as they are similar to previous changes (3)

• tests/test_blob_column_access.py
• tests/test_async_protocols.py
• tests/test_blob_negative_index.py

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Missing drop_keys method in ObjectToBlobReadWriteAdapter.

BlobToObjectReadWriteAdapter implements drop_keys (lines 145-153), but ObjectToBlobReadWriteAdapter does not. This asymmetry means callers wrapping an object backend via this adapter cannot drop keys.

🔧 Proposed fix: Add drop_keys method

     def remove(self) -> None:
         self._store.remove()
+
+    def drop_keys(
+        self,
+        keys: list[bytes],
+        indices: list[int] | None = None,
+    ) -> None:
+        self._store.drop_keys(
+            [k.decode() for k in keys],
+            indices=indices,
+        )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_adapters.py` around lines 259 - 263, The
ObjectToBlobReadWriteAdapter is missing a drop_keys method implemented on
BlobToObjectReadWriteAdapter; add a drop_keys(self, keys: Iterable[str]) -> None
method on ObjectToBlobReadWriteAdapter that delegates to the underlying store
(use self._store.drop_keys(keys) if the wrapped backend provides drop_keys,
otherwise iterate the keys and call self._store.remove(key) or self.remove
semantics) so callers can drop multiple keys through the adapter; place the
method alongside clear/remove in the ObjectToBlobReadWriteAdapter class and
match the signature/behavior used by BlobToObjectReadWriteAdapter.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Preserve deferred slicing for ASE column views in _DeferredSliceRowView.__getitem__().

When column_view_cls is AsyncASEColumnView, this method falls through to super().__getitem__ with unresolved self._indices == [], producing empty results instead of deferred evaluation.

Suggested fix

     def __getitem__(self, key):
         if isinstance(key, (str, bytes)):
             cv_cls = self._column_view_cls or AsyncColumnView
+            if cv_cls is AsyncASEColumnView:
+                return _DeferredSliceASEColumnView(self._parent, key, self._slice)
             if cv_cls is AsyncColumnView:
                 return _DeferredSliceColumnView(self._parent, key, self._slice)
         if isinstance(key, list) and key and isinstance(key[0], (str, bytes)):
             cv_cls = self._column_view_cls or AsyncColumnView
+            if cv_cls is AsyncASEColumnView:
+                return _DeferredSliceASEColumnView(self._parent, key, self._slice)
             if cv_cls is AsyncColumnView:
                 return _DeferredSliceColumnView(self._parent, key, self._slice)
         return super().__getitem__(key)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_views.py` around lines 706 - 715, _in
_DeferredSliceRowView.__getitem__, the current check only returns a
_DeferredSliceColumnView when cv_cls is exactly AsyncColumnView, causing
deferred slicing to be lost for AsyncASEColumnView; update the conditional to
treat AsyncASEColumnView (or any subclass meant to preserve deferred evaluation)
the same as AsyncColumnView so that when self._column_view_cls is
AsyncASEColumnView the method returns _DeferredSliceColumnView(self._parent,
key, self._slice) for both string/bytes and list-of-strings keys; reference the
symbols _DeferredSliceRowView.__getitem__, self._column_view_cls,
AsyncColumnView, AsyncASEColumnView, and _DeferredSliceColumnView when making
the change.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

BlobIO context manager does not close/delegate backend lifecycle.

with BlobIO(...) currently skips backend __enter__/__exit__ and close(), which can leak LMDB/Redis handles.

💡 Proposed fix

     def __enter__(self):
+        enter = getattr(self._backend, "__enter__", None)
+        if callable(enter):
+            enter()
         return self

     def __exit__(self, exc_type, exc_val, exc_tb):
-        return False
+        exit_ = getattr(self._backend, "__exit__", None)
+        if callable(exit_):
+            return bool(exit_(exc_type, exc_val, exc_tb))
+        close = getattr(self._backend, "close", None)
+        if callable(close):
+            close()
+        return False

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_blob_io.py` around lines 230 - 234, BlobIO's context methods
currently do nothing and thus leak backend resources; update BlobIO.__enter__ to
call and return backend.__enter__() if present (or return self if backend has no
__enter__), and update BlobIO.__exit__ to delegate to backend.__exit__(exc_type,
exc_val, exc_tb) if available and always call backend.close() (or
backend.close_async() as appropriate) to release handles; ensure safe checks for
attribute existence on the backend so BlobIO.__enter__/__exit__ work for
backends that lack these methods and still close resources on both normal exit
and exceptions.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

close() implementation has resource cleanup issues.

Two problems:

1. The task created by loop.create_task() is not stored, risking garbage collection before completion.
2. The except RuntimeError: pass silently ignores cleanup when called outside an async context, potentially leaking the connection.

Since aclose() and the async context manager work correctly, this is lower priority, but users calling close() directly may experience unexpected behavior.

🔧 Suggested improvement

     def close(self) -> None:
         """Close the MongoDB client connection (sync wrapper for compatibility)."""
-        # Note: AsyncMongoClient.close() is async, but we provide a sync method
-        # for consistency with other backends. Use aclose() for proper async cleanup.
         import asyncio
 
         try:
             loop = asyncio.get_running_loop()
-            loop.create_task(self._client.close())
+            # Store task reference to prevent GC before completion
+            task = loop.create_task(self._client.close())
+            # Suppress "task was destroyed but pending" warning
+            task.add_done_callback(lambda t: t.exception() if not t.cancelled() else None)
         except RuntimeError:
-            # No running loop - just close synchronously (best effort)
-            pass
+            # No running loop - run close() in a new event loop
+            asyncio.run(self._client.close())

🧰 Tools

🪛 Ruff (0.15.2)

[warning] 385-385: Store a reference to the return value of loop.create_task

(RUF006)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/mongodb/_async_backend.py` around lines 377 - 388, The close()
method currently creates an unreferenced task and silently skips cleanup outside
an event loop; modify close() to (1) when a running loop exists, store the
created task on self._close_task (or reuse if existing and pending), attach a
done-callback to surface exceptions, and avoid creating multiple orphan tasks;
(2) when no running loop (except RuntimeError), call
asyncio.run(self._client.aclose()) to perform synchronous cleanup; reference the
method close(), the async client method aclose()/close() on self._client, and
use a new instance attribute self._close_task to track the background task.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/asebytes/_views.py (1)

300-302: ⚠️ Potential issue | 🟡 Minor

to_dict() may raise KeyError on None placeholder rows.

At line 302, row[k] is accessed without checking if row is None. If any row in the batch is a placeholder (None), this will raise TypeError: 'NoneType' object is not subscriptable.

💡 Proposed fix

         for row in self._parent._read_rows(indices, keys=self._keys):
+            if row is None:
+                for k in self._keys:
+                    result[k].append(None)
+                continue
             for k in self._keys:
-                result[k].append(row[k])
+                result[k].append(row.get(k))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_views.py` around lines 300 - 302, In to_dict(), when iterating
rows returned by _read_rows, row may be None; change the inner loop in the for
row in self._parent._read_rows(indices, keys=self._keys) block to check for None
and, for each key in self._keys, append a safe placeholder (e.g., None) to
result[k] instead of indexing row[k] directly so you don't raise
TypeError/KeyError; ensure you reference the same symbols (to_dict, _read_rows,
self._keys, result) and preserve alignment of entries by appending None for
every key when row is None.

♻️ Duplicate comments (1)

src/asebytes/_async_views.py (1)

319-320: ⚠️ Potential issue | 🟠 Major

None row handling missing in _materialize().

When self._single is False, the code calls row.get(k) on the result of _read_row(). If the row at the requested index is None (a placeholder), this will raise AttributeError: 'NoneType' object has no attribute 'get'.

This was flagged in a prior review and marked as addressed, but the fix does not appear in the current code.

🐛 Suggested fix

         row = await self._parent._read_row(idx, keys=self._keys)
+        if row is None:
+            return [None] * len(self._keys)
         return [row.get(k) for k in self._keys]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_views.py` around lines 319 - 320, In _materialize(), when
calling self._parent._read_row(idx, keys=self._keys) the code assumes the result
is a dict and does row.get(k) which raises if row is None; update _materialize()
to check if row is None (or falsy) before iterating keys and, if so, return a
list of None values matching len(self._keys) (or an appropriate placeholder)
when self._single is False; keep behavior for the self._single=True path
unchanged and reference the existing symbols: _materialize(),
self._parent._read_row, self._keys, and self._single.

🧹 Nitpick comments (12)

src/asebytes/h5md/_backend.py (2)

324-324: Unused loop variable h5_name.

The variable h5_name is unpacked but not used in this loop body. Consider renaming to _h5_name to indicate it's intentionally unused.

🔧 Suggested fix

-        for key, (ds, h5_name, tag) in self._col_cache.items():
+        for key, (ds, _h5_name, tag) in self._col_cache.items():

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/h5md/_backend.py` at line 324, The loop unpacks h5_name from
self._col_cache but never uses it; update the unpacking in the loop header (for
key, (ds, h5_name, tag) in self._col_cache.items()) to use a clearly unused name
such as _h5_name (for example: for key, (ds, _h5_name, tag) in
self._col_cache.items()) so linters and readers know the variable is
intentionally unused; leave all other identifiers (key, ds, tag, and
self._col_cache) unchanged.

---

1133-1133: Consider iterable unpacking for cleaner syntax.

The current concatenation can be simplified using tuple unpacking.

🔧 Suggested fix

-            return tuple([min(self._chunk_size, shape[0])] + list(shape[1:]))
+            return (min(self._chunk_size, shape[0]), *shape[1:])

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/h5md/_backend.py` at line 1133, Replace the list concatenation
and tuple() call with tuple unpacking to simplify the expression: instead of
tuple([min(self._chunk_size, shape[0])] + list(shape[1:])), return
(min(self._chunk_size, shape[0]), *shape[1:]) so it directly builds the tuple
using self._chunk_size and the rest of shape.

src/asebytes/memory/_backend.py (2)

55-59: Tiny dedup: route remove() through clear().

clear() and remove() currently do the same thing. Reusing one keeps behavior centralized.

Proposed fix

     def remove(self) -> None:
-        self._data.clear()
+        self.clear()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/memory/_backend.py` around lines 55 - 59, The remove() method
duplicates clear() behavior; change remove to delegate to clear to centralize
behavior—replace the body of remove() so it calls the clear() method (i.e.,
invoke self.clear() inside remove()) rather than duplicating self._data.clear();
this keeps behavior in one place for the methods in the same class (clear and
remove).

---

28-39: Prevent external mutations from leaking into backend state.

The get() method returns internal dict references directly, and set() stores caller references without copying. This allows external code to mutate backend state unintentionally (e.g., data = backend.get(0); data["key"] = 999; backend.set(0, data) or even just mutations on the returned reference with keys parameter). All other backends (zarr, mongodb, h5md, redis) guard against this through deserialization or reconstruction.

The update() method (default implementation in ReadWriteBackend) is safe to use even with deepcopy, as it reads a row, mutates it, and writes it back—deepcopy preserves this semantic.

Proposed fix

 from __future__ import annotations
 
+from copy import deepcopy
 from typing import Any
@@
     def get(self, index: int, keys: list[str] | None = None) -> dict[str, Any] | None:
         if index < 0 or index >= len(self._data):
             raise IndexError(index)
         row = self._data[index]
-        if row is None or keys is None:
-            return row
-        return {k: row[k] for k in keys if k in row}
+        if row is None:
+            return None
+        if keys is None:
+            return deepcopy(row)
+        return {k: deepcopy(row[k]) for k in keys if k in row}
@@
     def set(self, index: int, value: dict[str, Any] | None) -> None:
         if index < 0 or index >= len(self._data):
             raise IndexError(index)
-        self._data[index] = value
+        self._data[index] = None if value is None else deepcopy(value)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/memory/_backend.py` around lines 28 - 39, The get() method
currently returns internal dict references and set() stores caller references,
allowing external mutations to leak into the backend's _data; to fix, ensure
get() returns deep-copied data (and when keys is provided, construct and return
a new dict with deep-copied values) and ensure set() stores a deep copy of the
provided value before assigning into _data; import and use copy.deepcopy so
ReadWriteBackend.update() semantics remain unchanged when it reads, mutates, and
writes rows via get/set.

src/asebytes/_object_io.py (1)

136-141: Same bounds-check gap as BlobIO for positive indices.

Positive indices that equal or exceed len(self) are passed directly to self._backend.get(index) without validation. For consistency, consider adding the upper-bound check here as well.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_object_io.py` around lines 136 - 141, The positive-index path
in the indexing logic (the branch that currently does "if isinstance(index,
int): ... return self._backend.get(index)") lacks an upper-bound check and can
pass out-of-range indices to self._backend.get(index); modify that branch in
_object_io (the integer-index handling) to verify index < len(self) after
normalizing negative indices and raise IndexError(index) if it is >= len(self)
before calling self._backend.get(index), mirroring the BlobIO bounds behavior.

src/asebytes/_blob_io.py (1)

140-145: Potential IndexError not raised when index equals length.

When index >= 0 and equals len(self), the backend's get() is called without bounds checking. The backend may return None or raise inconsistently depending on implementation. Consider adding an explicit upper-bound check.

         if isinstance(index, int):
             if index < 0:
                 index += len(self)
             if index < 0:
                 raise IndexError(index)
+            if index >= len(self):
+                raise IndexError(index)
             return self._backend.get(index)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_blob_io.py` around lines 140 - 145, The positive-index branch
in the item access code calls self._backend.get(index) without ensuring index <
len(self), which can let an out-of-range index equal to len(self) slip through;
update the integer-index handling in the method (the block around
isinstance(index, int)) to explicitly check if index >= len(self) and raise
IndexError(index) before calling self._backend.get, so all upper-bound cases are
handled consistently.

src/asebytes/redis/_async_backend.py (1)

404-411: Consider adding a sync close() method for compatibility.

The class only provides aclose(). If a caller accidentally calls close() (common pattern), it will raise AttributeError. Adding a simple wrapper improves ergonomics.

💡 Suggested addition

     async def aclose(self) -> None:
         """Close the async Redis connection."""
         await self._r.aclose()
+
+    def close(self) -> None:
+        """Sync close -- schedules aclose() in event loop."""
+        import asyncio
+        try:
+            loop = asyncio.get_running_loop()
+            loop.create_task(self.aclose())
+        except RuntimeError:
+            asyncio.run(self.aclose())

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/redis/_async_backend.py` around lines 404 - 411, Add a
synchronous close() wrapper to the AsyncRedisBlobBackend class so callers that
call close() won’t get AttributeError; implement close() to synchronously run
the existing aclose() (e.g. use
asyncio.get_event_loop().run_until_complete(self.aclose()) and handle the case
where the loop is already running by raising a clear error or documenting that
async callers should use aclose()), keeping aclose() unchanged and ensuring
__aenter__/__aexit__ semantics remain intact.

src/asebytes/_registry.py (1)

110-206: Consider extracting common registry lookup logic.

The four get_*_backend_cls functions share significant structural similarity (URI lookup, pattern matching, import handling, readonly dispatch). A helper function could reduce duplication and make future changes easier.

Also applies to: 209-240, 243-345, 348-378

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_registry.py` around lines 110 - 206, Extract the duplicated
registry/lookup logic used by get_backend_cls, get_blob_backend_cls,
get_object_backend_cls, and get_remote_backend_cls into a single helper (e.g.,
_lookup_backend_cls(path, *, readonly, uri_registry, backend_registry,
extras_hint, blob_registry=None, blob_getter=None)) that: performs URI parsing
and checks uri_registry, does fnmatch over backend_registry, centralizes
importlib.import_module + ImportError hint handling using extras_hint,
implements readonly dispatch (readonly True/False/None) returning the
appropriate writable/read_only attribute names, and optionally handles the blob
fallback by calling blob_getter when blob_registry is provided; then replace the
bodies of get_backend_cls and the other get_* functions to call this helper with
the appropriate registries/attribute names (and for blob wrapping use the
helper's blob fallback path to construct BlobToObject adapters). Ensure you keep
existing exception messages and preserve returned callable/class behavior
(including _make_read_adapter/_make_readwrite_adapter semantics).

src/asebytes/redis/_backend.py (1)

329-332: Document the intentional index clamping behavior in insert() method.

The implementation unconditionally clamps negative indices to 0, which diverges from Python's list.insert() semantics (e.g., list.insert(-1, x) inserts before the last element). While this behavior is intentional and consistent across Redis, MongoDB, and LMDB backends, it should be documented via a docstring on the insert() method since callers familiar with Python semantics may not expect this behavior.

For reference: Python's list.insert(-10, x) on a 3-element list results in insertion at index 0, matching this implementation's behavior for out-of-range negatives. However, Python still respects negative indices within range (e.g., -1 inserts before the last element), whereas this implementation treats all negatives the same.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/redis/_backend.py` around lines 329 - 332, Add a docstring to
the insert() method explaining the intentional index-clamping behavior: negative
indices are treated as 0 (not as Python's in-range negative indexing), indices
greater than n are clamped to n, and this behavior is consistent with the
Redis/MongoDB/LMDB backends; mention the divergence from Python's list.insert
semantics (e.g., list.insert(-1, x) inserts before the last element, whereas
this insert() treats all negatives as 0) so callers are aware. Include the
documentation directly on the insert() method (the function named insert) and
keep the wording concise so callers understand the clamping rules and
cross-backend consistency.

src/asebytes/_async_views.py (2)

349-355: Use raise ... from None to suppress exception chaining.

When converting a caught TypeError to a new TypeError with a clearer message, use from None to avoid confusing users with the original exception context.

♻️ Suggested fix

         try:
             return list(range(len(self._parent)))
         except TypeError:
             raise TypeError(
                 "Cannot resolve indices synchronously on async column views. "
                 "Use _aresolved_indices() instead."
-            )
+            ) from None

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_views.py` around lines 349 - 355, The except block in the
method that attempts to return list(range(len(self._parent))) catches a
TypeError and re-raises a new TypeError with a clearer message (in
_async_views.py); change the raise to suppress exception chaining by using
"raise TypeError(... ) from None" so the original traceback isn't shown—update
the raise in that except TypeError block to use "from None".

---

370-376: Use raise ... from None to suppress exception chaining.

Same pattern as _resolved_indices()—when raising a clearer error message, suppress the original exception context.

♻️ Suggested fix

         except TypeError:
             raise TypeError(
                 "len() not available on async column views without explicit "
                 "indices. Use 'await col.to_list()' to materialize first."
-            )
+            ) from None

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_views.py` around lines 370 - 376, The except block in the
length computation should suppress exception chaining like _resolved_indices()
does: replace the current raise in the except TypeError handler inside the
len(self._parent) try/except with a raise TypeError(... ) from None so the
original TypeError context is not attached; update the same raise pattern
wherever similar explicit-index errors are surfaced in _async_views.py for
consistency.

src/asebytes/mongodb/_async_backend.py (1)

226-226: Consider adding strict=True to zip for defensive coding.

While new_sks and values are guaranteed to have the same length here, adding strict=True makes the invariant explicit and will catch any future regressions.

♻️ Suggested change

-        docs = [self._row_to_doc(sk, v) for sk, v in zip(new_sks, values)]
+        docs = [self._row_to_doc(sk, v) for sk, v in zip(new_sks, values, strict=True)]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/mongodb/_async_backend.py` at line 226, The list comprehension
building docs should enforce the invariant that new_sks and values have equal
length; update the zip call used when creating docs (where docs =
[self._row_to_doc(sk, v) for sk, v in zip(new_sks, values)]) to use zip(new_sks,
values, strict=True) so mismatched lengths raise immediately and help catch
regressions, keeping the use of self._row_to_doc(sk, v) unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/asebytes/_adapters.py`:
- Around line 80-83: In get_column, guard against None values returned by
self._store.get_column so msgpack.unpackb isn't called on None: retrieve raw_col
via self._store.get_column(byte_key, indices), if raw_col is None return an
empty list (or list of Nones if that's required by callers), otherwise iterate
raw_col and for each element v return None when v is None and msgpack.unpackb(v,
object_hook=m.decode) when v is not None; update get_column to perform this
conditional mapping so unpackb is only invoked on non-None values.

In `@src/asebytes/memory/_backend.py`:
- Around line 21-23: The from_uri classmethod on MemoryObjectBackend accepts any
uri and forwards **kwargs into cls(...) even though __init__ accepts no kwargs,
causing a confusing TypeError; update from_uri to validate the URI scheme (e.g.,
parse uri and ensure scheme == "memory") and raise a clear ValueError if it does
not match, and make kwargs handling explicit by either rejecting unexpected
kwargs with a clear TypeError or by extracting and validating only documented
parameters before calling cls(); reference the from_uri method and the class
initializer (__init__) when making these changes so callers get a helpful error
message instead of an opaque exception.

In `@src/asebytes/mongodb/_async_backend.py`:
- Around line 386-389: The done-callback on the task created in the async
backend currently calls t.result() which will re-raise any exception from
self._client.close(); change the callback to use t.exception() instead (e.g.,
call t.exception() and handle/log the returned Exception if not None) so the
exception is observed without being re-raised; update the callback attached to
the Task created by loop.create_task(self._client.close()) in the async backend
to use t.exception() and handle the result safely.

---

Outside diff comments:
In `@src/asebytes/_views.py`:
- Around line 300-302: In to_dict(), when iterating rows returned by _read_rows,
row may be None; change the inner loop in the for row in
self._parent._read_rows(indices, keys=self._keys) block to check for None and,
for each key in self._keys, append a safe placeholder (e.g., None) to result[k]
instead of indexing row[k] directly so you don't raise TypeError/KeyError;
ensure you reference the same symbols (to_dict, _read_rows, self._keys, result)
and preserve alignment of entries by appending None for every key when row is
None.

---

Duplicate comments:
In `@src/asebytes/_async_views.py`:
- Around line 319-320: In _materialize(), when calling
self._parent._read_row(idx, keys=self._keys) the code assumes the result is a
dict and does row.get(k) which raises if row is None; update _materialize() to
check if row is None (or falsy) before iterating keys and, if so, return a list
of None values matching len(self._keys) (or an appropriate placeholder) when
self._single is False; keep behavior for the self._single=True path unchanged
and reference the existing symbols: _materialize(), self._parent._read_row,
self._keys, and self._single.

---

Nitpick comments:
In `@src/asebytes/_async_views.py`:
- Around line 349-355: The except block in the method that attempts to return
list(range(len(self._parent))) catches a TypeError and re-raises a new TypeError
with a clearer message (in _async_views.py); change the raise to suppress
exception chaining by using "raise TypeError(... ) from None" so the original
traceback isn't shown—update the raise in that except TypeError block to use
"from None".
- Around line 370-376: The except block in the length computation should
suppress exception chaining like _resolved_indices() does: replace the current
raise in the except TypeError handler inside the len(self._parent) try/except
with a raise TypeError(... ) from None so the original TypeError context is not
attached; update the same raise pattern wherever similar explicit-index errors
are surfaced in _async_views.py for consistency.

In `@src/asebytes/_blob_io.py`:
- Around line 140-145: The positive-index branch in the item access code calls
self._backend.get(index) without ensuring index < len(self), which can let an
out-of-range index equal to len(self) slip through; update the integer-index
handling in the method (the block around isinstance(index, int)) to explicitly
check if index >= len(self) and raise IndexError(index) before calling
self._backend.get, so all upper-bound cases are handled consistently.

In `@src/asebytes/_object_io.py`:
- Around line 136-141: The positive-index path in the indexing logic (the branch
that currently does "if isinstance(index, int): ... return
self._backend.get(index)") lacks an upper-bound check and can pass out-of-range
indices to self._backend.get(index); modify that branch in _object_io (the
integer-index handling) to verify index < len(self) after normalizing negative
indices and raise IndexError(index) if it is >= len(self) before calling
self._backend.get(index), mirroring the BlobIO bounds behavior.

In `@src/asebytes/_registry.py`:
- Around line 110-206: Extract the duplicated registry/lookup logic used by
get_backend_cls, get_blob_backend_cls, get_object_backend_cls, and
get_remote_backend_cls into a single helper (e.g., _lookup_backend_cls(path, *,
readonly, uri_registry, backend_registry, extras_hint, blob_registry=None,
blob_getter=None)) that: performs URI parsing and checks uri_registry, does
fnmatch over backend_registry, centralizes importlib.import_module + ImportError
hint handling using extras_hint, implements readonly dispatch (readonly
True/False/None) returning the appropriate writable/read_only attribute names,
and optionally handles the blob fallback by calling blob_getter when
blob_registry is provided; then replace the bodies of get_backend_cls and the
other get_* functions to call this helper with the appropriate
registries/attribute names (and for blob wrapping use the helper's blob fallback
path to construct BlobToObject adapters). Ensure you keep existing exception
messages and preserve returned callable/class behavior (including
_make_read_adapter/_make_readwrite_adapter semantics).

In `@src/asebytes/h5md/_backend.py`:
- Line 324: The loop unpacks h5_name from self._col_cache but never uses it;
update the unpacking in the loop header (for key, (ds, h5_name, tag) in
self._col_cache.items()) to use a clearly unused name such as _h5_name (for
example: for key, (ds, _h5_name, tag) in self._col_cache.items()) so linters and
readers know the variable is intentionally unused; leave all other identifiers
(key, ds, tag, and self._col_cache) unchanged.
- Line 1133: Replace the list concatenation and tuple() call with tuple
unpacking to simplify the expression: instead of tuple([min(self._chunk_size,
shape[0])] + list(shape[1:])), return (min(self._chunk_size, shape[0]),
*shape[1:]) so it directly builds the tuple using self._chunk_size and the rest
of shape.

In `@src/asebytes/memory/_backend.py`:
- Around line 55-59: The remove() method duplicates clear() behavior; change
remove to delegate to clear to centralize behavior—replace the body of remove()
so it calls the clear() method (i.e., invoke self.clear() inside remove())
rather than duplicating self._data.clear(); this keeps behavior in one place for
the methods in the same class (clear and remove).
- Around line 28-39: The get() method currently returns internal dict references
and set() stores caller references, allowing external mutations to leak into the
backend's _data; to fix, ensure get() returns deep-copied data (and when keys is
provided, construct and return a new dict with deep-copied values) and ensure
set() stores a deep copy of the provided value before assigning into _data;
import and use copy.deepcopy so ReadWriteBackend.update() semantics remain
unchanged when it reads, mutates, and writes rows via get/set.

In `@src/asebytes/mongodb/_async_backend.py`:
- Line 226: The list comprehension building docs should enforce the invariant
that new_sks and values have equal length; update the zip call used when
creating docs (where docs = [self._row_to_doc(sk, v) for sk, v in zip(new_sks,
values)]) to use zip(new_sks, values, strict=True) so mismatched lengths raise
immediately and help catch regressions, keeping the use of self._row_to_doc(sk,
v) unchanged.

In `@src/asebytes/redis/_async_backend.py`:
- Around line 404-411: Add a synchronous close() wrapper to the
AsyncRedisBlobBackend class so callers that call close() won’t get
AttributeError; implement close() to synchronously run the existing aclose()
(e.g. use asyncio.get_event_loop().run_until_complete(self.aclose()) and handle
the case where the loop is already running by raising a clear error or
documenting that async callers should use aclose()), keeping aclose() unchanged
and ensuring __aenter__/__aexit__ semantics remain intact.

In `@src/asebytes/redis/_backend.py`:
- Around line 329-332: Add a docstring to the insert() method explaining the
intentional index-clamping behavior: negative indices are treated as 0 (not as
Python's in-range negative indexing), indices greater than n are clamped to n,
and this behavior is consistent with the Redis/MongoDB/LMDB backends; mention
the divergence from Python's list.insert semantics (e.g., list.insert(-1, x)
inserts before the last element, whereas this insert() treats all negatives as
0) so callers are aware. Include the documentation directly on the insert()
method (the function named insert) and keep the wording concise so callers
understand the clamping rules and cross-backend consistency.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 150cce0 and 69e9a50.

📒 Files selected for processing (12)

• src/asebytes/_adapters.py
• src/asebytes/_async_views.py
• src/asebytes/_blob_io.py
• src/asebytes/_object_io.py
• src/asebytes/_registry.py
• src/asebytes/_views.py
• src/asebytes/h5md/_backend.py
• src/asebytes/memory/_backend.py
• src/asebytes/mongodb/_async_backend.py
• src/asebytes/redis/_async_backend.py
• src/asebytes/redis/_backend.py
• tests/test_lmdb_backend.py

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Validate URI scheme and make kwargs handling explicit in from_uri.

Line 23 forwards **kwargs into cls(...), but __init__ currently accepts no kwargs, so callers get an opaque TypeError. Also, uri is not validated, so non-memory:// input is silently accepted.

Proposed fix

 from __future__ import annotations
 
 from typing import Any
+from urllib.parse import urlsplit
 
 from .._backends import ReadWriteBackend
@@
     `@classmethod`
     def from_uri(cls, uri: str, **kwargs: Any) -> MemoryObjectBackend:
         """Create from a ``memory://`` URI. The path is ignored."""
-        return cls(**kwargs)
+        parts = urlsplit(uri)
+        if parts.scheme != "memory":
+            raise ValueError(f"Unsupported URI scheme for memory backend: {parts.scheme!r}")
+        if kwargs:
+            unexpected = ", ".join(sorted(kwargs))
+            raise TypeError(f"Unsupported kwargs for MemoryObjectBackend: {unexpected}")
+        return cls()

🧰 Tools

🪛 Ruff (0.15.2)

[warning] 21-21: Unused class method argument: uri

(ARG003)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/memory/_backend.py` around lines 21 - 23, The from_uri
classmethod on MemoryObjectBackend accepts any uri and forwards **kwargs into
cls(...) even though __init__ accepts no kwargs, causing a confusing TypeError;
update from_uri to validate the URI scheme (e.g., parse uri and ensure scheme ==
"memory") and raise a clear ValueError if it does not match, and make kwargs
handling explicit by either rejecting unexpected kwargs with a clear TypeError
or by extracting and validating only documented parameters before calling cls();
reference the from_uri method and the class initializer (__init__) when making
these changes so callers get a helpful error message instead of an opaque
exception.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Task callback may raise unhandled exception.

Using t.result() in the done callback will re-raise any exception that occurred during close(). Since callbacks run in the event loop context, this can cause an unhandled exception warning. Use t.exception() instead to retrieve the exception without raising it.

🔧 Suggested fix

             task = loop.create_task(self._client.close())
-            task.add_done_callback(lambda t: t.result() if not t.cancelled() else None)
+            task.add_done_callback(lambda t: t.exception() if not t.cancelled() else None)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/mongodb/_async_backend.py` around lines 386 - 389, The
done-callback on the task created in the async backend currently calls
t.result() which will re-raise any exception from self._client.close(); change
the callback to use t.exception() instead (e.g., call t.exception() and
handle/log the returned Exception if not None) so the exception is observed
without being re-raised; update the callback attached to the Task created by
loop.create_task(self._client.close()) in the async backend to use t.exception()
and handle the result safely.

[coderabbitai]

Actionable comments posted: 14

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/asebytes/zarr/_backend.py (2)

243-256: ⚠️ Potential issue | 🟠 Major

Normalize requested indices in get_column() before reading arrays.

get_column() currently uses raw indices, so negative indices can map to unintended array positions. This diverges from get()/get_many() and can return incorrect values.

🔧 Proposed fix

-        order = np.argsort(indices)
-        sorted_idx = [indices[j] for j in order]
+        checked = [self._check_index(i) for i in indices]
+        order = np.argsort(checked)
+        sorted_idx = [checked[j] for j in order]
         # Filter to valid indices
         valid_idx = [idx for idx in sorted_idx if idx < arr_len]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/zarr/_backend.py` around lines 243 - 256, In get_column(),
normalize requested indices to Python-style positions before sorting and
slicing: for each idx in indices compute norm = idx if idx >= 0 else arr_len +
idx, then build sorted_idx from these normalized values, filter valid_idx using
0 <= norm < arr_len, read raw = arr[valid_idx] using the normalized indices, and
map results back into result at positions determined by order (using order[j] as
before); ensure negative indices and out-of-range values are handled
consistently with get()/get_many() by using the normalized indices and
preserving the original indices' output ordering.

---

303-317: ⚠️ Potential issue | 🔴 Critical

set() crashes on None and does not behave as an overwrite.

set() should support None placeholders and overwrite semantics, but current logic dereferences data.items() unconditionally and only patches provided keys, leaving stale values behind.

🛠️ Proposed fix

     def set(self, index: int, data: dict[str, Any] | None) -> None:
         index = self._check_index(index)
-        for key, val in data.items():
+        # Clear existing row first to preserve overwrite semantics.
+        for _key, arr in self._col_cache.items():
+            if arr.dtype.kind in ("S", "U", "O"):
+                arr[index] = ""
+            elif arr.dtype.kind == "b":
+                arr[index] = False
+            else:
+                arr[index] = np.nan
+
+        if data is None:
+            return
+
+        for key, val in data.items():
             if key not in self._col_cache:
                 continue
             arr = self._col_cache[key]
             val = self._serialize_value(val)
             if isinstance(val, np.ndarray) and self._variable_shape:
                 if arr.ndim > 1 and val.ndim >= 1 and val.shape[0] < arr.shape[1]:
                     padded = np.full(arr.shape[1:], np.nan, dtype=np.float64)
                     slices = tuple(slice(0, s) for s in val.shape)
                     padded[slices] = val
                     val = padded
             arr[index] = val

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/zarr/_backend.py` around lines 303 - 317, The set method
currently dereferences data.items() unconditionally and only updates keys
present in data, leaving other columns unchanged and crashing when data is None;
change set(self, index, data) so it first normalizes data to an empty dict when
None (treat as all-missing/overwrite), then iterate over all keys in
self._col_cache (not data.items()) and for each key determine val =
data.get(key, None); if val is None assign the appropriate missing/fill value
into arr[index] (respecting dtype: e.g., None for object, np.nan or a sentinel
for numeric) instead of skipping, otherwise call self._serialize_value(val) and
apply existing variable-shape padding logic before arr[index] = val; keep use of
self._check_index, self._col_cache, self._serialize_value, self._variable_shape
and arr variables to locate the code to change.

src/asebytes/io.py (1)

211-217: ⚠️ Potential issue | 🟠 Major

__getitem__(int) crashes on reserved rows.

At Line 217, dict_to_atoms(row) is called unconditionally. After reserve(), row may be None, causing a conversion failure.

💡 Proposed fix

         if isinstance(index, int):
@@
             row = self._read_row(index)
-            return dict_to_atoms(row)
+            if row is None:
+                return None
+            return dict_to_atoms(row)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/io.py` around lines 211 - 217, In __getitem__ when handling
integer indices, dict_to_atoms(row) is called without checking if _read_row
returned None for reserved rows; update the __getitem__ implementation to check
the result of self._read_row(index) (in the __getitem__ method) and return an
appropriate value (e.g., None or raise IndexError) when row is None instead of
passing it to dict_to_atoms, ensuring reserved rows don't trigger a conversion
failure; reference the __getitem__ method, _read_row, and dict_to_atoms to
locate and implement the guard.

♻️ Duplicate comments (4)

tests/test_async_blob_io.py (1)

100-100: ⚠️ Potential issue | 🟡 Minor

Remove the unused raw_backend fixture from this test signature.

Line 100 includes raw_backend but the test body does not use it.

Suggested change

-    async def test_await_single_row(self, io, raw_backend):
+    async def test_await_single_row(self, io):
         """await io[0] → dict[bytes, bytes]."""
         result = await io[0]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_async_blob_io.py` at line 100, The test function
test_await_single_row currently declares an unused fixture parameter
raw_backend; remove raw_backend from the async def test_await_single_row(...)
signature so the test only accepts the fixtures it actually uses (e.g., io),
leaving the test body unchanged and avoiding unused-parameter warnings.

src/asebytes/mongodb/_async_backend.py (1)

401-412: ⚠️ Potential issue | 🟡 Minor

close() creates unreferenced task that may be garbage collected.

The task created by loop.create_task() on Line 410 is not stored, which can cause:

1. The task to be garbage collected before completion
2. "Task was destroyed but it is pending" warnings

🔧 Suggested fix

     def close(self) -> None:
         """Close the MongoDB client connection (best-effort from sync context).

         Prefer :meth:`aclose` when inside an async context.
         """
         import asyncio

         try:
             loop = asyncio.get_running_loop()
-            loop.create_task(self._client.close())
+            task = loop.create_task(self._client.close())
+            # Suppress "task was destroyed but pending" warning
+            task.add_done_callback(lambda t: t.exception() if not t.cancelled() else None)
         except RuntimeError:
             asyncio.run(self._client.close())

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/mongodb/_async_backend.py` around lines 401 - 412, The current
close() creates an anonymous task with loop.create_task(self._client.close())
which can be garbage-collected and produce "Task was destroyed but it is
pending" warnings; fix by saving the returned Task (e.g. assign task =
loop.create_task(self._client.close())) and keep a short-lived reference (store
on self, e.g. self._close_task) and attach a done callback to consume/handle
exceptions (task.add_done_callback(...)) so the task isn’t unreferenced and
exceptions are handled; update the close() implementation to use the saved task
and callback while still calling self._client.close().

src/asebytes/memory/_backend.py (1)

45-48: ⚠️ Potential issue | 🟡 Minor

from_uri does not validate URI scheme.

The method accepts any URI string without validating that it's a memory:// scheme. It also forwards **kwargs to the constructor, but the constructor only accepts group.

🔧 Suggested fix

+    from urllib.parse import urlsplit
+
     `@classmethod`
     def from_uri(cls, uri: str, **kwargs: Any) -> MemoryObjectBackend:
         """Create from a ``memory://`` URI. The path is ignored."""
-        return cls(**kwargs)
+        parts = urlsplit(uri)
+        if parts.scheme != "memory":
+            raise ValueError(f"Unsupported URI scheme: {parts.scheme!r}")
+        group = kwargs.pop("group", None)
+        if kwargs:
+            unexpected = ", ".join(sorted(kwargs))
+            raise TypeError(f"Unexpected kwargs: {unexpected}")
+        return cls(group=group)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/memory/_backend.py` around lines 45 - 48, from_uri currently
accepts any URI and blindly forwards **kwargs to the constructor; update
from_uri to parse and validate the URI scheme (use urllib.parse.urlparse(uri)
and ensure scheme == "memory"), and raise a ValueError if it doesn't match; also
validate/filter kwargs so only the allowed constructor parameter (group) is
passed to cls (either extract group from kwargs or raise a TypeError for
unexpected keys) before calling cls(...), referencing the from_uri classmethod
and the MemoryObjectBackend constructor/group parameter.

src/asebytes/lmdb/_blob_backend.py (1)

281-285: ⚠️ Potential issue | 🟠 Major

keys= should stay projection-only, not strict validation.

At Lines 281-285, missing projected keys raise KeyError. This breaks sparse-row reads where callers expect partial rows.

💡 Proposed fix

-        if keys_set is not None and len(result) != len(keys_set):
-            retrieved_keys = set(result.keys())
-            invalid_keys = keys_set - retrieved_keys
-            raise KeyError(f"Invalid keys at index {index}: {sorted(invalid_keys)}")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/lmdb/_blob_backend.py` around lines 281 - 285, The code
currently treats keys= as strict validation by computing
retrieved_keys/invalid_keys and raising KeyError when projected keys are
missing; instead make projection non-strict: do not raise for missing
keys—simply return the available entries in result for the given index (leave
keys_set, result, and index logic intact but remove the raise and any related
invalid_keys computation), so callers requesting sparse projections get back
partial rows rather than an exception.

🧹 Nitpick comments (12)

tests/test_ase_column_views.py (1)

67-70: Guard get_column() against None rows.

At Line 70, self.get(i, [key]).get(key) will fail if self.get(...) returns None. Making this null-safe prevents brittle behavior when placeholder rows appear.

Suggested change

     def get_column(self, key, indices=None):
         if indices is None:
             indices = list(range(len(self)))
-        return [self.get(i, [key]).get(key) for i in indices]
+        out = []
+        for i in indices:
+            row = self.get(i, [key])
+            out.append(None if row is None else row.get(key))
+        return out

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_ase_column_views.py` around lines 67 - 70, The get_column method
can raise when self.get(i, [key]) returns None; update get_column to guard each
row for None before calling .get on it—inside get_column iterate indices and for
each i call row = self.get(i, [key]) and append row.get(key) if row is not None
else None (or another appropriate placeholder), ensuring get_column returns a
list with null-safe values; reference get_column and the call to
self.get(...).get(key) when making the change.

tests/test_async_protocols.py (1)

3-8: Remove the duplicated bullet in the module docstring.

Line 6 and Line 7 repeat the same statement; trimming one keeps the file header clean.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_async_protocols.py` around lines 3 - 8, Remove the duplicated
bullet from the module docstring in tests/test_async_protocols.py: locate the
header docstring that lists the test items and delete one of the two identical
lines that read "SyncToAsyncAdapter wraps a sync ReadWriteBackend and works
correctly" so the bullet list contains each test description only once; ensure
the docstring formatting (triple quotes and newline spacing) remains unchanged.

tests/test_async_aseio.py (2)

325-328: Use raw string for regex pattern.

The pattern contains . which is a regex metacharacter. While it works as intended here, using a raw string makes the intent clearer and avoids warnings.

🔧 Proposed fix

     `@pytest.mark.anyio`
     async def test_column_to_dict_raises(self, db):
         """await db["calc.energy"].to_dict() raises TypeError."""
-        with pytest.raises(TypeError, match="to_dict.*not available"):
+        with pytest.raises(TypeError, match=r"to_dict.*not available"):
             await db["calc.energy"].to_dict()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_async_aseio.py` around lines 325 - 328, In
test_column_to_dict_raises, the regex passed to pytest.raises contains
metacharacters (the dot) and should be written as a raw string; update the
pytest.raises call (in the test_column_to_dict_raises function that awaits
db["calc.energy"].to_dict()) to use a raw-string pattern (prefix the string with
r) for the match argument so the dot is treated literally and avoids regex
interpretation/warnings.

---

25-92: Consider extracting shared test fixtures to reduce duplication.

MemoryBackend, _make_row, and _make_atoms are duplicated across test_async_aseio.py and test_async_aseio_atoms.py (as shown in the relevant code snippets). Extracting these to a shared conftest.py or test utilities module would reduce maintenance burden.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_async_aseio.py` around lines 25 - 92, Tests duplicate the
MemoryBackend class and helper functions _make_row and _make_atoms across
test_async_aseio.py and test_async_aseio_atoms.py; extract these into a shared
test module (e.g., conftest.py or tests/utils.py) and expose them as fixtures or
imports so both test files reuse the single implementation. Move the
MemoryBackend class, _make_row, and _make_atoms into the shared file, create
pytest fixtures (or plain helper imports) that return the backend or generated
rows/Atoms, and update the tests to import or depend on those fixtures instead
of redefining MemoryBackend, _make_row, and _make_atoms locally.

tests/test_edge_cases.py (1)

254-296: Consider renaming test functions to match the tested class.

The section header was updated to "BlobIO" but test function names still use the bytesio prefix (e.g., test_bytesio_with_empty_group, test_bytesio_with_custom_group). This creates a naming inconsistency that could confuse future maintainers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_edge_cases.py` around lines 254 - 296, Rename the misnamed test
functions to reflect the class under test (BlobIO) to avoid confusion: change
test_bytesio_with_empty_group -> test_blobio_with_empty_group,
test_bytesio_with_custom_group -> test_blobio_with_custom_group,
test_bytesio_multiple_groups_isolated -> test_blobio_multiple_groups_isolated,
and test_bytesio_empty_data_dict -> test_blobio_empty_data_dict so the names
match the section header and the tested class (BlobIO/LMDBBlobBackend) and leave
test bodies unchanged.

src/asebytes/mongodb/_backend.py (1)

31-38: Consider narrowing the exception type or adding a comment.

The bare except Exception at line 36 catches all exceptions when msgpack.unpackb fails. While this is intentional (binary data may not be msgpack), it could mask unexpected errors. Consider catching specific msgpack exceptions or adding a comment explaining the intent.

💡 Proposed improvement

 def _bson_restore(value: Any) -> Any:
     """Restore numpy arrays from BSON Binary msgpack blobs."""
     if isinstance(value, (bytes, Binary)):
         try:
             return msgpack.unpackb(value, object_hook=m.decode)
-        except Exception:
+        except (msgpack.UnpackException, msgpack.ExtraData, ValueError):
+            # Not a valid msgpack blob — return raw bytes
             return value
     return value

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/mongodb/_backend.py` around lines 31 - 38, In _bson_restore,
narrow the bare except by catching msgpack-specific unpack errors (e.g.,
msgpack.exceptions.UnpackException and msgpack.exceptions.ExtraData) around the
msgpack.unpackb call and add a short comment explaining that non-msgpack binary
data is expected and should be returned unchanged; if your msgpack variant
exposes different exception names, import and use those specific exception
classes (or fall back to a very narrow set) instead of catching Exception.

src/asebytes/mongodb/_async_backend.py (1)

273-273: zip() without explicit strict= parameter.

While new_sks and values are guaranteed to have equal length (both derived from len(values)), adding strict=True makes this invariant explicit.

💡 Suggested fix

-        docs = [self._row_to_doc(sk, v) for sk, v in zip(new_sks, values)]
+        docs = [self._row_to_doc(sk, v) for sk, v in zip(new_sks, values, strict=True)]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/mongodb/_async_backend.py` at line 273, The list comprehension
building docs uses zip(new_sks, values) in _async_backend.py; update it to
zip(new_sks, values, strict=True) so the length invariant is enforced at runtime
(i.e., change the comprehension that calls self._row_to_doc(sk, v) to iterate
over zip(new_sks, values, strict=True)).

tests/test_blob_native_bytes.py (1)

24-24: Unused tmp_path fixture arguments.

The tmp_path fixture is declared but not used in test_column_view_from_bytes_key and test_column_view_int_index.

💡 Remove unused fixture arguments

-    def test_column_view_from_bytes_key(self, tmp_path):
+    def test_column_view_from_bytes_key(self):
         """blobdb[b"key"] returns ColumnView (no decode)."""
...
-    def test_column_view_int_index(self, tmp_path):
+    def test_column_view_int_index(self):
         """blobdb[b"name"][0] returns single bytes value."""

Also applies to: 88-88

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_blob_native_bytes.py` at line 24, The tests declare an unused
pytest fixture parameter tmp_path in test_column_view_from_bytes_key and
test_column_view_int_index; remove tmp_path from the function signatures (i.e.,
update the def lines for test_column_view_from_bytes_key and
test_column_view_int_index to not accept tmp_path) so the tests no longer
include an unused fixture argument.

src/asebytes/_object_io.py (1)

199-202: Return type of extend differs from MutableSequence base.

MutableSequence.extend returns None, but this implementation returns int (new length) to match the backend contract. This is intentional but worth noting for type checkers.

Consider adding a return type annotation override to silence type checkers:

def extend(self, values) -> int:  # type: ignore[override]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_object_io.py` around lines 199 - 202, The extend method in the
class implements MutableSequence.extend but deliberately returns an int (new
length) from self._backend.extend, which conflicts with the base signature; to
silence type checkers add a type ignore override on the method signature (e.g.,
annotate extend with a type: ignore[override]) while keeping the current
behavior that checks ReadWriteBackend and returns
self._backend.extend(list(values)); reference the extend method, the _backend
attribute and ReadWriteBackend to locate the code to modify.

src/asebytes/h5md/_backend.py (1)

335-353: Unused loop variable h5_name.

The loop variable h5_name is extracted but never used within the loop body.

💡 Suggested fix

-        for key, (ds, h5_name, tag) in self._col_cache.items():
+        for key, (ds, _h5_name, tag) in self._col_cache.items():

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/h5md/_backend.py` around lines 335 - 353, The for-loop unpacks
(ds, h5_name, tag) from self._col_cache but never uses h5_name; remove the
unused variable by changing the unpack to (ds, _, tag) or otherwise ignore
h5_name where the tuple is unpacked in the loop that iterates over
self._col_cache, and ensure references to ds, tag, _postprocess_typed,
unique_sorted and unique_rows remain unchanged so behavior is preserved.

src/asebytes/lmdb/_backend.py (1)

92-104: get_column fetches full rows then filters.

The current implementation fetches all keys via get_with_txn(txn, i, None) then extracts the requested key. This works correctly but is less efficient than fetching only the needed key.

Consider passing [byte_key] to get_with_txn for key-filtered reads:

-                raw = self._store.get_with_txn(txn, i, None)  # Get all keys
+                raw = self._store.get_with_txn(txn, i, [byte_key])

This optimization depends on whether LMDBBlobBackend.get_with_txn supports efficient key filtering.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/lmdb/_backend.py` around lines 92 - 104, The get_column method
currently calls self._store.get_with_txn(txn, i, None) and then filters the
returned row, which fetches all keys; change that call to request only the
needed key by passing [byte_key] (i.e. use self._store.get_with_txn(txn, i,
[byte_key])) so LMDBBlobBackend.get_with_txn can perform a key-filtered read;
keep the existing logic that checks if raw is None or byte_key not in raw and
unpacks raw[byte_key] with msgpack.unpackb/object_hook=m.decode to preserve
behavior; if get_with_txn does not support a keys list, add a small
feature-detect branch that falls back to the current call.

src/asebytes/io.py (1)

121-124: Avoid fully silent cache write failures.

Catching broad Exception and pass makes persistent cache errors invisible in production debugging.

💡 Proposed fix

-            except Exception:
-                pass  # cache write is best-effort
+            except Exception as exc:
+                warnings.warn(
+                    f"cache write failed at index {index}: {exc}",
+                    RuntimeWarning,
+                    stacklevel=2,
+                )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/io.py` around lines 121 - 124, The current try/except around
self._cache.set(index, full_row) silently swallows all exceptions; update it to
catch Exception as e and emit a logged warning including the exception traceback
(prefer self._logger.exception(...) if the class has self._logger, otherwise use
logging.getLogger(__name__).exception(...)) so cache write failures are visible
while still treating cache writes as best-effort and not re-raising the error.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@README.md`:
- Around line 5-12: The fenced code block containing the pip install commands
should include a language identifier to satisfy markdownlint MD040; update the
opening triple-backtick for the block that wraps the pip install lines (the
block with "pip install asebytes[lmdb]" through the in-memory backend comment)
to use "```bash" so the block is language-labeled and renders/satisfies linting.

In `@src/asebytes/_adapters.py`:
- Around line 279-284: ObjectToBlobReadWriteAdapter.set_column currently unpacks
every item with msgpack.unpackb which breaks when
ObjectToBlobReadAdapter.get_column returns None placeholders; modify set_column
to tolerate None by only calling msgpack.unpackb(..., object_hook=m.decode) for
non-None values and passing None through unchanged, and update the values type
annotation if needed to allow Optional entries so the _store.set_column call
receives the same None placeholders instead of raising.

In `@src/asebytes/_async_adapters.py`:
- Around line 81-85: The async adapters call msgpack.unpackb/packb
unconditionally and will raise on None; update
StrToMsgPackAsyncAdapter.get_column, BytesToMsgPackAsyncAdapter.get_column, and
MsgPackAsyncObjectAdapter.set_column to guard against None exactly like their
sync counterparts: iterate over the returned/raw lists (raw_col, values, etc.),
if an element is None append None to the output list, otherwise call
msgpack.unpackb() or msgpack.packb() with the existing object_hook/encoding
logic and append the result; ensure return types and ordering match the sync
implementations so missing rows remain None.

In `@src/asebytes/_async_backends.py`:
- Around line 64-67: The default column-read loop in the async backend currently
uses row[key] which can raise KeyError when a row exists but the requested key
is missing; update the logic in the method (the loop that calls self.get and
appends results) to safely extract the value using a non-raising access (e.g.
use row.get(key) or check key in row) so that when row is not None but the key
is absent you append None instead of raising.

In `@src/asebytes/_async_io.py`:
- Around line 108-112: The backend write calls in _write_row (and the
corresponding multi-write method that calls backend.set_many) currently forward
ase.Atoms objects directly; detect when data (or items in data list) is an
instance of ase.Atoms and convert it to a plain dict (e.g., data = data.todict()
or equivalent atom→dict serialization) before calling self._backend.set or
self._backend.set_many, preserving None values and existing behavior for
non-Atoms inputs; update both _write_row and the method that invokes
backend.set_many to perform this conversion so view-based writes work correctly.

In `@src/asebytes/_backends.py`:
- Around line 60-68: The get_column method may raise KeyError when a fetched row
exists but lacks the requested key; update get_column (and its use of self.get)
to safely extract the value (e.g., use row.get(key) or check "if row is not None
and key in row" before indexing) and append None when the key is absent so
missing keys do not raise exceptions.

In `@src/asebytes/io.py`:
- Around line 119-127: The cache read path assumes rows are dicts but can
receive None placeholders; update the code paths around the backend/cache get
logic (where self._backend.get(index) is assigned to full_row and where
cached_row is read later) to guard for None before doing key filtering or column
access: if full_row (or cached_row) is None, skip key comprehension and return
None (or an empty dict if keys is not None per existing API), and avoid calling
self._cache.set when full_row is None; apply the same None checks to the
analogous block at lines 144-147 so neither key filtering nor item access raises
TypeError.

In `@tests/test_ase_column_views.py`:
- Around line 214-229: Replace the normal string regex literals passed to
pytest.raises(..., match=...) with raw string literals for the four tests
(test_sync_single_key_to_dict_raises, test_sync_multi_key_to_dict_raises,
test_async_single_key_to_dict_raises, test_async_multi_key_to_dict_raises) so
the pattern "to_dict.*not available" becomes a raw regex r"to_dict.*not
available" in each raises call; update the match argument in
db["calc.energy"].to_dict(), db[["calc.energy", "info.tag"]].to_dict(), await
async_db["calc.energy"].to_dict(), and await async_db[["calc.energy",
"info.tag"]].to_dict() accordingly.

In `@tests/test_async_aseio_atoms.py`:
- Around line 49-51: The test double MemoryBackend.extend currently returns None
but must follow the ReadWriteBackend.extend contract by returning the new
length; update MemoryBackend.extend (the method that calls
self._rows.extend(data)) to compute and return the length after extending (e.g.,
call len(self._rows) after extend) so tests mirror the backend contract.

In `@tests/test_async_aseio.py`:
- Around line 56-59: get_column currently assumes every entry in self._rows is a
dict and will crash when encountering a placeholder None; update the logic in
get_column (and the called get method) to guard against None rows by skipping
them or treating them as missing values rather than subscripting them.
Specifically, when iterating over self._rows (or when calling self.get(i,
[key])), add a check like "if row is None: yield or append None/continue" so
accessing [key] never happens on a None; reference the methods get_column, get,
extend and the _rows list when making the change.

In `@tests/test_async_facade_update.py`:
- Around line 56-58: The test backends' extend() implementations
(MemoryBlobBackend.extend and MemoryObjectBackend.extend in
tests/test_async_facade_update.py) currently mutate the internal container but
return None, violating ReadWriteBackend.extend()'s contract; update both methods
to return the new total length after extending by returning len(self._rows) or
the appropriate container (e.g., len(self._data)) immediately after calling
.extend(values) so they match the production backends and the abstract method's
docstring.

In `@tests/test_async_protocols.py`:
- Around line 199-204: The test test_set_many_default is calling backend.set in
a loop instead of exercising the AsyncReadWriteBackend.set_many default
behavior; change the loop that does await backend.set(1 + offset, row) to a
single call to await backend.set_many(1, [{b"a": b"99"}, {b"a": b"98"}]) so the
test invokes backend.set_many (and thereby validates
AsyncReadWriteBackend.set_many/MemoryAsyncRawWritable.set_many) and then keep
the two asserts for await backend.get(1) and await backend.get(2).

In `@tests/test_blob_column_access.py`:
- Around line 56-59: The get_column method can raise TypeError when self.get(i,
[key]) returns None; update get_column (the get_column function) to avoid
indexing into None by replacing the list comprehension with an explicit loop:
for each index call row = self.get(i, [key]), then if row is None append None
(or skip depending on desired behavior) else append row[key]; ensure the default
indices logic remains the same and preserve return type (list of values with
None for missing rows) so callers don’t get a TypeError.
- Around line 149-166: The test class TestAsyncBlobIOColumnAccessMigrated
duplicates three tests already present in TestAsyncBlobIOColumnAccess; remove
the redundant class definition (TestAsyncBlobIOColumnAccessMigrated) including
its methods test_bytes_key_returns_async_column_view,
test_list_bytes_returns_async_column_view, and test_bytes_key_to_list so only
the original tests remain; ensure no other references to
TestAsyncBlobIOColumnAccessMigrated exist and that the remaining tests still
create AsyncBlobIO(SyncToAsyncAdapter(blob_backend)) and assert isinstance(...,
AsyncColumnView) as before.

---

Outside diff comments:
In `@src/asebytes/io.py`:
- Around line 211-217: In __getitem__ when handling integer indices,
dict_to_atoms(row) is called without checking if _read_row returned None for
reserved rows; update the __getitem__ implementation to check the result of
self._read_row(index) (in the __getitem__ method) and return an appropriate
value (e.g., None or raise IndexError) when row is None instead of passing it to
dict_to_atoms, ensuring reserved rows don't trigger a conversion failure;
reference the __getitem__ method, _read_row, and dict_to_atoms to locate and
implement the guard.

In `@src/asebytes/zarr/_backend.py`:
- Around line 243-256: In get_column(), normalize requested indices to
Python-style positions before sorting and slicing: for each idx in indices
compute norm = idx if idx >= 0 else arr_len + idx, then build sorted_idx from
these normalized values, filter valid_idx using 0 <= norm < arr_len, read raw =
arr[valid_idx] using the normalized indices, and map results back into result at
positions determined by order (using order[j] as before); ensure negative
indices and out-of-range values are handled consistently with get()/get_many()
by using the normalized indices and preserving the original indices' output
ordering.
- Around line 303-317: The set method currently dereferences data.items()
unconditionally and only updates keys present in data, leaving other columns
unchanged and crashing when data is None; change set(self, index, data) so it
first normalizes data to an empty dict when None (treat as
all-missing/overwrite), then iterate over all keys in self._col_cache (not
data.items()) and for each key determine val = data.get(key, None); if val is
None assign the appropriate missing/fill value into arr[index] (respecting
dtype: e.g., None for object, np.nan or a sentinel for numeric) instead of
skipping, otherwise call self._serialize_value(val) and apply existing
variable-shape padding logic before arr[index] = val; keep use of
self._check_index, self._col_cache, self._serialize_value, self._variable_shape
and arr variables to locate the code to change.

---

Duplicate comments:
In `@src/asebytes/lmdb/_blob_backend.py`:
- Around line 281-285: The code currently treats keys= as strict validation by
computing retrieved_keys/invalid_keys and raising KeyError when projected keys
are missing; instead make projection non-strict: do not raise for missing
keys—simply return the available entries in result for the given index (leave
keys_set, result, and index logic intact but remove the raise and any related
invalid_keys computation), so callers requesting sparse projections get back
partial rows rather than an exception.

In `@src/asebytes/memory/_backend.py`:
- Around line 45-48: from_uri currently accepts any URI and blindly forwards
**kwargs to the constructor; update from_uri to parse and validate the URI
scheme (use urllib.parse.urlparse(uri) and ensure scheme == "memory"), and raise
a ValueError if it doesn't match; also validate/filter kwargs so only the
allowed constructor parameter (group) is passed to cls (either extract group
from kwargs or raise a TypeError for unexpected keys) before calling cls(...),
referencing the from_uri classmethod and the MemoryObjectBackend
constructor/group parameter.

In `@src/asebytes/mongodb/_async_backend.py`:
- Around line 401-412: The current close() creates an anonymous task with
loop.create_task(self._client.close()) which can be garbage-collected and
produce "Task was destroyed but it is pending" warnings; fix by saving the
returned Task (e.g. assign task = loop.create_task(self._client.close())) and
keep a short-lived reference (store on self, e.g. self._close_task) and attach a
done callback to consume/handle exceptions (task.add_done_callback(...)) so the
task isn’t unreferenced and exceptions are handled; update the close()
implementation to use the saved task and callback while still calling
self._client.close().

In `@tests/test_async_blob_io.py`:
- Line 100: The test function test_await_single_row currently declares an unused
fixture parameter raw_backend; remove raw_backend from the async def
test_await_single_row(...) signature so the test only accepts the fixtures it
actually uses (e.g., io), leaving the test body unchanged and avoiding
unused-parameter warnings.

---

Nitpick comments:
In `@src/asebytes/_object_io.py`:
- Around line 199-202: The extend method in the class implements
MutableSequence.extend but deliberately returns an int (new length) from
self._backend.extend, which conflicts with the base signature; to silence type
checkers add a type ignore override on the method signature (e.g., annotate
extend with a type: ignore[override]) while keeping the current behavior that
checks ReadWriteBackend and returns self._backend.extend(list(values));
reference the extend method, the _backend attribute and ReadWriteBackend to
locate the code to modify.

In `@src/asebytes/h5md/_backend.py`:
- Around line 335-353: The for-loop unpacks (ds, h5_name, tag) from
self._col_cache but never uses h5_name; remove the unused variable by changing
the unpack to (ds, _, tag) or otherwise ignore h5_name where the tuple is
unpacked in the loop that iterates over self._col_cache, and ensure references
to ds, tag, _postprocess_typed, unique_sorted and unique_rows remain unchanged
so behavior is preserved.

In `@src/asebytes/io.py`:
- Around line 121-124: The current try/except around self._cache.set(index,
full_row) silently swallows all exceptions; update it to catch Exception as e
and emit a logged warning including the exception traceback (prefer
self._logger.exception(...) if the class has self._logger, otherwise use
logging.getLogger(__name__).exception(...)) so cache write failures are visible
while still treating cache writes as best-effort and not re-raising the error.

In `@src/asebytes/lmdb/_backend.py`:
- Around line 92-104: The get_column method currently calls
self._store.get_with_txn(txn, i, None) and then filters the returned row, which
fetches all keys; change that call to request only the needed key by passing
[byte_key] (i.e. use self._store.get_with_txn(txn, i, [byte_key])) so
LMDBBlobBackend.get_with_txn can perform a key-filtered read; keep the existing
logic that checks if raw is None or byte_key not in raw and unpacks
raw[byte_key] with msgpack.unpackb/object_hook=m.decode to preserve behavior; if
get_with_txn does not support a keys list, add a small feature-detect branch
that falls back to the current call.

In `@src/asebytes/mongodb/_async_backend.py`:
- Line 273: The list comprehension building docs uses zip(new_sks, values) in
_async_backend.py; update it to zip(new_sks, values, strict=True) so the length
invariant is enforced at runtime (i.e., change the comprehension that calls
self._row_to_doc(sk, v) to iterate over zip(new_sks, values, strict=True)).

In `@src/asebytes/mongodb/_backend.py`:
- Around line 31-38: In _bson_restore, narrow the bare except by catching
msgpack-specific unpack errors (e.g., msgpack.exceptions.UnpackException and
msgpack.exceptions.ExtraData) around the msgpack.unpackb call and add a short
comment explaining that non-msgpack binary data is expected and should be
returned unchanged; if your msgpack variant exposes different exception names,
import and use those specific exception classes (or fall back to a very narrow
set) instead of catching Exception.

In `@tests/test_ase_column_views.py`:
- Around line 67-70: The get_column method can raise when self.get(i, [key])
returns None; update get_column to guard each row for None before calling .get
on it—inside get_column iterate indices and for each i call row = self.get(i,
[key]) and append row.get(key) if row is not None else None (or another
appropriate placeholder), ensuring get_column returns a list with null-safe
values; reference get_column and the call to self.get(...).get(key) when making
the change.

In `@tests/test_async_aseio.py`:
- Around line 325-328: In test_column_to_dict_raises, the regex passed to
pytest.raises contains metacharacters (the dot) and should be written as a raw
string; update the pytest.raises call (in the test_column_to_dict_raises
function that awaits db["calc.energy"].to_dict()) to use a raw-string pattern
(prefix the string with r) for the match argument so the dot is treated
literally and avoids regex interpretation/warnings.
- Around line 25-92: Tests duplicate the MemoryBackend class and helper
functions _make_row and _make_atoms across test_async_aseio.py and
test_async_aseio_atoms.py; extract these into a shared test module (e.g.,
conftest.py or tests/utils.py) and expose them as fixtures or imports so both
test files reuse the single implementation. Move the MemoryBackend class,
_make_row, and _make_atoms into the shared file, create pytest fixtures (or
plain helper imports) that return the backend or generated rows/Atoms, and
update the tests to import or depend on those fixtures instead of redefining
MemoryBackend, _make_row, and _make_atoms locally.

In `@tests/test_async_protocols.py`:
- Around line 3-8: Remove the duplicated bullet from the module docstring in
tests/test_async_protocols.py: locate the header docstring that lists the test
items and delete one of the two identical lines that read "SyncToAsyncAdapter
wraps a sync ReadWriteBackend and works correctly" so the bullet list contains
each test description only once; ensure the docstring formatting (triple quotes
and newline spacing) remains unchanged.

In `@tests/test_blob_native_bytes.py`:
- Line 24: The tests declare an unused pytest fixture parameter tmp_path in
test_column_view_from_bytes_key and test_column_view_int_index; remove tmp_path
from the function signatures (i.e., update the def lines for
test_column_view_from_bytes_key and test_column_view_int_index to not accept
tmp_path) so the tests no longer include an unused fixture argument.

In `@tests/test_edge_cases.py`:
- Around line 254-296: Rename the misnamed test functions to reflect the class
under test (BlobIO) to avoid confusion: change test_bytesio_with_empty_group ->
test_blobio_with_empty_group, test_bytesio_with_custom_group ->
test_blobio_with_custom_group, test_bytesio_multiple_groups_isolated ->
test_blobio_multiple_groups_isolated, and test_bytesio_empty_data_dict ->
test_blobio_empty_data_dict so the names match the section header and the tested
class (BlobIO/LMDBBlobBackend) and leave test bodies unchanged.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 69e9a50 and da1e0b3.

📒 Files selected for processing (53)

• README.md
• src/asebytes/_adapters.py
• src/asebytes/_async_adapters.py
• src/asebytes/_async_backends.py
• src/asebytes/_async_blob_io.py
• src/asebytes/_async_io.py
• src/asebytes/_async_object_io.py
• src/asebytes/_backends.py
• src/asebytes/_blob_io.py
• src/asebytes/_object_io.py
• src/asebytes/ase/_backend.py
• src/asebytes/decode.py
• src/asebytes/h5md/_backend.py
• src/asebytes/hf/_backend.py
• src/asebytes/io.py
• src/asebytes/lmdb/_backend.py
• src/asebytes/lmdb/_blob_backend.py
• src/asebytes/memory/_backend.py
• src/asebytes/mongodb/_async_backend.py
• src/asebytes/mongodb/_backend.py
• src/asebytes/redis/_async_backend.py
• src/asebytes/redis/_backend.py
• src/asebytes/zarr/_backend.py
• tests/test_ase_column_views.py
• tests/test_aseio.py
• tests/test_async_aseio.py
• tests/test_async_aseio_atoms.py
• tests/test_async_blob_io.py
• tests/test_async_facade_update.py
• tests/test_async_protocols.py
• tests/test_blob_column_access.py
• tests/test_blob_native_bytes.py
• tests/test_blob_negative_index.py
• tests/test_bulk_write_abc.py
• tests/test_bulk_write_async_abc.py
• tests/test_cache_to.py
• tests/test_edge_cases.py
• tests/test_error_handling.py
• tests/test_h5md_backend.py
• tests/test_hf_aseio.py
• tests/test_memory_backend.py
• tests/test_memory_uri.py
• tests/test_mongodb.py
• tests/test_optional_parameters.py
• tests/test_protocols.py
• tests/test_protocols_new_methods.py
• tests/test_raw_protocols.py
• tests/test_redis.py
• tests/test_review_critical_fixes.py
• tests/test_sync_facade_clear_remove.py
• tests/test_sync_rowview_drop.py
• tests/test_sync_to_async_adapter.py
• tests/test_sync_write_infra.py

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/test_aseio.py

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add a language identifier to the install command code fence.

Line 5 starts an unlabeled fenced block; this triggers markdownlint (MD040).

Suggested change

-```
+```bash
 pip install asebytes[lmdb]      # LMDB backend (recommended)
 pip install asebytes[zarr]      # Zarr backend (fast compression)
 pip install asebytes[h5md]      # HDF5/H5MD backend
 pip install asebytes[hf]        # HuggingFace Datasets backend
 pip install asebytes[mongodb]   # MongoDB backend (shared remote storage)
 # In-memory backend (MemoryObjectBackend) is built-in — no extras needed

</details>

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.21.0)</summary>

[warning] 5-5: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @README.md around lines 5 - 12, The fenced code block containing the pip
install commands should include a language identifier to satisfy markdownlint
MD040; update the opening triple-backtick for the block that wraps the pip
install lines (the block with "pip install asebytes[lmdb]" through the in-memory
backend comment) to use "```bash" so the block is language-labeled and
renders/satisfies linting.

</details>

<!-- fingerprinting:phantom:poseidon:hawk -->

<!-- This is an auto-generated comment by CodeRabbit -->

[coderabbitai]

⚠️ Potential issue | 🟠 Major

set_column should tolerate placeholder values from column reads.

ObjectToBlobReadAdapter.get_column can emit None placeholders, but ObjectToBlobReadWriteAdapter.set_column unpacks every value directly, which can fail on None.

💡 Proposed fix

-    def set_column(self, key: bytes, start: int, values: list[bytes]) -> None:
+    def set_column(
+        self, key: bytes, start: int, values: list[bytes | None]
+    ) -> None:
         self._store.set_column(
             key.decode(),
             start,
-            [msgpack.unpackb(v, object_hook=m.decode) for v in values],
+            [
+                msgpack.unpackb(v, object_hook=m.decode) if v is not None else None
+                for v in values
+            ],
         )

#!/bin/bash
# Verify sync adapter column round-trip None handling.
rg -n -C2 'class ObjectToBlobReadAdapter|class ObjectToBlobReadWriteAdapter|def get_column|def set_column|msgpack\.unpackb' src/asebytes/_adapters.py

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_adapters.py` around lines 279 - 284,
ObjectToBlobReadWriteAdapter.set_column currently unpacks every item with
msgpack.unpackb which breaks when ObjectToBlobReadAdapter.get_column returns
None placeholders; modify set_column to tolerate None by only calling
msgpack.unpackb(..., object_hook=m.decode) for non-None values and passing None
through unchanged, and update the values type annotation if needed to allow
Optional entries so the _store.set_column call receives the same None
placeholders instead of raising.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's look at the file structure
cd /repo
ls -la src/asebytes/

# Get the actual code at the mentioned lines in async_adapters.py
echo "=== Lines 81-85 in async_adapters.py ==="
sed -n '75,90p' src/asebytes/_async_adapters.py

echo ""
echo "=== Lines 212-217 in async_adapters.py ==="
sed -n '206,223p' src/asebytes/_async_adapters.py

echo ""
echo "=== Lines 268-273 in async_adapters.py ==="
sed -n '262,280p' src/asebytes/_async_adapters.py

Repository: zincware/asebytes

Length of output: 3949

---

🏁 Script executed:

#!/bin/bash
# Now let's check the sync adapters for comparison
echo "=== Sync adapters column methods ==="
rg -n -A10 'def get_column|def set_column' src/asebytes/_adapters.py | head -80

Repository: zincware/asebytes

Length of output: 1788

---

🏁 Script executed:

#!/bin/bash
# Test msgpack behavior with None values to understand the core issue
python3 << 'EOF'
import msgpack

# Test 1: Can msgpack pack None?
try:
    packed_none = msgpack.packb(None)
    print(f"✓ msgpack.packb(None) works: {repr(packed_none)}")
    unpacked = msgpack.unpackb(packed_none)
    print(f"✓ msgpack.unpackb(packed_none) works: {repr(unpacked)}")
except Exception as e:
    print(f"✗ msgpack error with None: {e}")

# Test 2: Can msgpack unpack None directly?
try:
    result = msgpack.unpackb(None)
    print(f"msgpack.unpackb(None) returned: {repr(result)}")
except Exception as e:
    print(f"✓ msgpack.unpackb(None) raises error (expected): {type(e).__name__}")

# Test 3: What does a list with None look like?
print("\nTest with list containing None:")
test_list = [1, None, 3]
try:
    packed = msgpack.packb(test_list)
    unpacked = msgpack.unpackb(packed)
    print(f"✓ Packed and unpacked list: {unpacked}")
except Exception as e:
    print(f"✗ Error: {e}")
EOF

Repository: zincware/asebytes

Length of output: 180

---

🏁 Script executed:

pip install msgpack -q && python3 << 'EOF'
import msgpack

# Test 1: Can msgpack pack None?
try:
    packed_none = msgpack.packb(None)
    print(f"✓ msgpack.packb(None) works: {repr(packed_none)}")
    unpacked = msgpack.unpackb(packed_none)
    print(f"✓ msgpack.unpackb(packed_none) works: {repr(unpacked)}")
except Exception as e:
    print(f"✗ msgpack.packb(None) error: {type(e).__name__}: {e}")

# Test 2: Can msgpack unpack None directly?
try:
    result = msgpack.unpackb(None)
    print(f"msgpack.unpackb(None) returned: {repr(result)}")
except Exception as e:
    print(f"✓ msgpack.unpackb(None) raises error (expected): {type(e).__name__}")

# Test 3: What happens in a list comprehension with None?
print("\nTest with list containing None (current async code pattern):")
test_col = [b'\x00', None, b'\x02']
try:
    result = [msgpack.unpackb(v) for v in test_col]
    print(f"Result: {result}")
except Exception as e:
    print(f"✗ Error with None in list: {type(e).__name__}: {e}")

print("\nTest with None check (proposed fix):")
result = [msgpack.unpackb(v) if v is not None else None for v in test_col]
print(f"Result: {result}")
EOF

Repository: zincware/asebytes

Length of output: 406

---

🏁 Script executed:

# Check if there are any tests that might show intended behavior
rg -n -B2 -A5 'get_column|set_column' tests/ --type py 2>/dev/null | head -100

Repository: zincware/asebytes

Length of output: 5940

---

Async column adapters don't safely preserve None values in three locations.

The async implementations fail where sync implementations have safeguards:

• Lines 81-85 (get_column in StrToMsgPackAsyncAdapter): Calls msgpack.unpackb() unconditionally; sync version (lines 91-97) checks if v is not None.
• Lines 212-217 (get_column in BytesToMsgPackAsyncAdapter): Calls msgpack.packb() unconditionally; sync version (lines 223-228) checks if v is not None.
• Lines 268-273 (set_column in MsgPackAsyncObjectAdapter): Calls msgpack.unpackb() unconditionally on all values; sync version (lines 279-284) has the same issue.

When a column contains None (representing a missing row), msgpack.unpackb(None) raises TypeError: a bytes-like object is required, not 'NoneType'. Tests at test_redis.py:307-310 and test_redis.py:732-735 confirm the expected behavior: get_column() should preserve None in the result list.

Fix

    async def get_column(self, key: str, indices: list[int] | None = None) -> list[Any]:
         byte_key = key.encode()
         raw_col = await self._store.get_column(byte_key, indices)
-        return [msgpack.unpackb(v, object_hook=m.decode) for v in raw_col]
+        return [
+            msgpack.unpackb(v, object_hook=m.decode) if v is not None else None
+            for v in raw_col
+        ]

    async def get_column(
         self, key: bytes, indices: list[int] | None = None
-    ) -> list[bytes]:
+    ) -> list[bytes | None]:
         str_key = key.decode()
         col = await self._store.get_column(str_key, indices)
-        return [msgpack.packb(v, default=m.encode) for v in col]
+        return [
+            msgpack.packb(v, default=m.encode) if v is not None else None
+            for v in col
+        ]

    async def set_column(self, key: bytes, start: int, values: list[bytes]) -> None:
         await self._store.set_column(
             key.decode(),
             start,
-            [msgpack.unpackb(v, object_hook=m.decode) for v in values],
+            [
+                msgpack.unpackb(v, object_hook=m.decode) if v is not None else None
+                for v in values
+            ],
         )

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	async def get_column(self, key: str, indices: list[int] | None = None) -> list[Any]:
	byte_key = key.encode()
	raw_col = await self._store.get_column(byte_key, indices)
	return [msgpack.unpackb(v, object_hook=m.decode) for v in raw_col]
	async def get_column(self, key: str, indices: list[int] | None = None) -> list[Any]:
	byte_key = key.encode()
	raw_col = await self._store.get_column(byte_key, indices)
	return [
	msgpack.unpackb(v, object_hook=m.decode) if v is not None else None
	for v in raw_col
	]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_adapters.py` around lines 81 - 85, The async adapters
call msgpack.unpackb/packb unconditionally and will raise on None; update
StrToMsgPackAsyncAdapter.get_column, BytesToMsgPackAsyncAdapter.get_column, and
MsgPackAsyncObjectAdapter.set_column to guard against None exactly like their
sync counterparts: iterate over the returned/raw lists (raw_col, values, etc.),
if an element is None append None to the output list, otherwise call
msgpack.unpackb() or msgpack.packb() with the existing object_hook/encoding
logic and append the result; ensure return types and ordering match the sync
implementations so missing rows remain None.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Default get_column() should not raise on missing keys.

Using row[key] can raise KeyError when a row exists but that key is absent. Default behavior should return None for missing values.

✅ Proposed fix

         results = []
         for i in indices:
             row = await self.get(i, [key])
-            results.append(row[key] if row is not None else None)
+            results.append(row.get(key) if row is not None else None)
         return results

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_backends.py` around lines 64 - 67, The default
column-read loop in the async backend currently uses row[key] which can raise
KeyError when a row exists but the requested key is missing; update the logic in
the method (the loop that calls self.get and appends results) to safely extract
the value using a non-raising access (e.g. use row.get(key) or check key in row)
so that when row is not None but the key is absent you append None instead of
raising.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Convert ase.Atoms to dict before calling backend set/set_many.

These methods currently forward ase.Atoms objects directly to backend write APIs that expect dict[str, Any] | None, which breaks view-based writes.

🛠️ Proposed fix

     async def _write_row(self, index: int, data: Any) -> None:
         if not isinstance(self._backend, AsyncReadWriteBackend):
             raise TypeError("Backend is read-only")
-        await self._backend.set(index, data)
+        if data is None:
+            await self._backend.set(index, None)
+            return
+        from ._convert import atoms_to_dict
+        await self._backend.set(index, atoms_to_dict(data))
@@
     async def _write_many(self, start: int, data: list) -> None:
         if not isinstance(self._backend, AsyncReadWriteBackend):
             raise TypeError("Backend is read-only")
-        await self._backend.set_many(start, data)
+        from ._convert import atoms_to_dict
+        payload = [None if atoms is None else atoms_to_dict(atoms) for atoms in data]
+        await self._backend.set_many(start, payload)

Also applies to: 138-142

🧰 Tools

🪛 Ruff (0.15.2)

[warning] 110-110: Avoid specifying long messages outside the exception class

(TRY003)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_io.py` around lines 108 - 112, The backend write calls in
_write_row (and the corresponding multi-write method that calls
backend.set_many) currently forward ase.Atoms objects directly; detect when data
(or items in data list) is an instance of ase.Atoms and convert it to a plain
dict (e.g., data = data.todict() or equivalent atom→dict serialization) before
calling self._backend.set or self._backend.set_many, preserving None values and
existing behavior for non-Atoms inputs; update both _write_row and the method
that invokes backend.set_many to perform this conversion so view-based writes
work correctly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

get_column will raise TypeError if any row is None.

When self.get(i, [key]) returns None (for a placeholder row), accessing [key] on it will raise TypeError: 'NoneType' object is not subscriptable.

🐛 Proposed fix

     def get_column(self, key, indices=None):
         if indices is None:
             indices = list(range(len(self)))
-        return [self.get(i, [key])[key] for i in indices]
+        results = []
+        for i in indices:
+            row = self.get(i, [key])
+            results.append(row[key] if row is not None else None)
+        return results

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_async_aseio.py` around lines 56 - 59, get_column currently assumes
every entry in self._rows is a dict and will crash when encountering a
placeholder None; update the logic in get_column (and the called get method) to
guard against None rows by skipping them or treating them as missing values
rather than subscripting them. Specifically, when iterating over self._rows (or
when calling self.get(i, [key])), add a check like "if row is None: yield or
append None/continue" so accessing [key] never happens on a None; reference the
methods get_column, get, extend and the _rows list when making the change.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n tests/test_async_facade_update.py | sed -n '50,65p'

Repository: zincware/asebytes

Length of output: 557

---

🏁 Script executed:

cat -n tests/test_async_facade_update.py | sed -n '90,105p'

Repository: zincware/asebytes

Length of output: 508

---

🏁 Script executed:

rg "class ReadWriteBackend" -A 20

Repository: zincware/asebytes

Length of output: 1243

---

🏁 Script executed:

rg "def extend" -A 5 src/asebytes/_backends.py

Repository: zincware/asebytes

Length of output: 264

---

🏁 Script executed:

git ls-files tests/ | head -20

Repository: zincware/asebytes

Length of output: 670

---

🏁 Script executed:

rg "class MemoryBlobBackend|class MemoryObjectBackend" -B 2 -A 10 tests/test_async_facade_update.py

Repository: zincware/asebytes

Length of output: 713

---

🏁 Script executed:

rg "ReadWriteBackend" tests/test_async_facade_update.py

Repository: zincware/asebytes

Length of output: 209

---

🏁 Script executed:

# Check if there are any other extend() implementations in real backends
rg "def extend" src/ -A 3

Repository: zincware/asebytes

Length of output: 6581

---

Both test backends violate ReadWriteBackend.extend() contract by returning None instead of int.

The abstract base class ReadWriteBackend.extend() (src/asebytes/_backends.py) specifies return type int with docstring "Returns the new total length after the extend." However, MemoryBlobBackend.extend() at lines 56-58 and MemoryObjectBackend.extend() at lines 97-99 omit the return statement. All production backends (memory, zarr, redis, mongodb, lmdb, h5md) correctly return the new length; test doubles must do the same to preserve contract fidelity and prevent silent regressions through the IO/Adapter facades that propagate the return value.

Suggested change

 def extend(self, values):
     self._rows.extend(values)
+    return len(self._rows)

Also applies to: 97-99

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_async_facade_update.py` around lines 56 - 58, The test backends'
extend() implementations (MemoryBlobBackend.extend and
MemoryObjectBackend.extend in tests/test_async_facade_update.py) currently
mutate the internal container but return None, violating
ReadWriteBackend.extend()'s contract; update both methods to return the new
total length after extending by returning len(self._rows) or the appropriate
container (e.g., len(self._data)) immediately after calling .extend(values) so
they match the production backends and the abstract method's docstring.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate and examine the test file
find . -name "test_async_protocols.py" -type f

Repository: zincware/asebytes

Length of output: 92

---

🏁 Script executed:

# Also search for the AsyncReadWriteBackend implementation
find . -name "*.py" -type f | head -20

Repository: zincware/asebytes

Length of output: 657

---

🏁 Script executed:

# Search for AsyncReadWriteBackend class definition
rg "class AsyncReadWriteBackend" -A 5

Repository: zincware/asebytes

Length of output: 483

---

🏁 Script executed:

# Search for set_many method
rg "def set_many" -B 2 -A 10

Repository: zincware/asebytes

Length of output: 2854

---

🏁 Script executed:

# Examine the actual test file at lines around 199-204
sed -n '190,210p' tests/test_async_protocols.py

Repository: zincware/asebytes

Length of output: 839

---

🏁 Script executed:

# Search for MemoryAsyncRawWritable class
rg "class MemoryAsyncRawWritable" -B 2 -A 20

Repository: zincware/asebytes

Length of output: 1520

---

🏁 Script executed:

# Check if MemoryAsyncRawWritable overrides set_many
rg "class MemoryAsyncRawWritable" -A 50 | grep -E "(def set_many|def set\()"

Repository: zincware/asebytes

Length of output: 168

---

test_set_many_default does not actually test set_many().

Lines 201-202 call set() in a loop, so regressions in AsyncReadWriteBackend.set_many() default logic would pass undetected. The test should invoke set_many() directly to verify its behavior.

Suggested change

     async def test_set_many_default(self):
         backend = MemoryAsyncRawWritable([{b"a": b"0"}, {b"a": b"1"}, {b"a": b"2"}])
-        for offset, row in enumerate([{b"a": b"99"}, {b"a": b"98"}]):
-            await backend.set(1 + offset, row)
+        await backend.set_many(1, [{b"a": b"99"}, {b"a": b"98"}])
         assert await backend.get(1) == {b"a": b"99"}
         assert await backend.get(2) == {b"a": b"98"}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_async_protocols.py` around lines 199 - 204, The test
test_set_many_default is calling backend.set in a loop instead of exercising the
AsyncReadWriteBackend.set_many default behavior; change the loop that does await
backend.set(1 + offset, row) to a single call to await backend.set_many(1,
[{b"a": b"99"}, {b"a": b"98"}]) so the test invokes backend.set_many (and
thereby validates
AsyncReadWriteBackend.set_many/MemoryAsyncRawWritable.set_many) and then keep
the two asserts for await backend.get(1) and await backend.get(2).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

get_column will raise TypeError if any row is None.

Same issue as in test_async_aseio.py: when self.get(i, [key]) returns None, accessing [key] raises TypeError.

🐛 Proposed fix

     def get_column(self, key, indices=None):
         if indices is None:
             indices = list(range(len(self)))
-        return [self.get(i, [key])[key] for i in indices]
+        results = []
+        for i in indices:
+            row = self.get(i, [key])
+            results.append(row[key] if row is not None else None)
+        return results

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def get_column(self, key, indices=None):
	if indices is None:
	indices = list(range(len(self)))
	return [self.get(i, [key])[key] for i in indices]
	def get_column(self, key, indices=None):
	if indices is None:
	indices = list(range(len(self)))
	results = []
	for i in indices:
	row = self.get(i, [key])
	results.append(row[key] if row is not None else None)
	return results

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_blob_column_access.py` around lines 56 - 59, The get_column method
can raise TypeError when self.get(i, [key]) returns None; update get_column (the
get_column function) to avoid indexing into None by replacing the list
comprehension with an explicit loop: for each index call row = self.get(i,
[key]), then if row is None append None (or skip depending on desired behavior)
else append row[key]; ensure the default indices logic remains the same and
preserve return type (list of values with None for missing rows) so callers
don’t get a TypeError.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Remove duplicate test class.

TestAsyncBlobIOColumnAccessMigrated (lines 149-166) duplicates the first three tests from TestAsyncBlobIOColumnAccess (lines 120-143). This appears to be leftover from a migration and should be removed.

🧹 Proposed fix

-# ── AsyncBlobIO column access ────────────────────────────────────────────
-
-
-class TestAsyncBlobIOColumnAccessMigrated:
-    `@pytest.mark.anyio`
-    async def test_bytes_key_returns_async_column_view(self, blob_backend):
-        io = AsyncBlobIO(SyncToAsyncAdapter(blob_backend))
-        view = io[b"name"]
-        assert isinstance(view, AsyncColumnView)
-
-    `@pytest.mark.anyio`
-    async def test_list_bytes_returns_async_column_view(self, blob_backend):
-        io = AsyncBlobIO(SyncToAsyncAdapter(blob_backend))
-        view = io[[b"name", b"age"]]
-        assert isinstance(view, AsyncColumnView)
-
-    `@pytest.mark.anyio`
-    async def test_bytes_key_to_list(self, blob_backend):
-        io = AsyncBlobIO(SyncToAsyncAdapter(blob_backend))
-        result = await io[b"name"].to_list()
-        assert result == [b"alice", b"bob", b"carol"]

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	class TestAsyncBlobIOColumnAccessMigrated:
	@pytest.mark.anyio
	async def test_bytes_key_returns_async_column_view(self, blob_backend):
	io = AsyncBlobIO(SyncToAsyncAdapter(blob_backend))
	view = io[b"name"]
	assert isinstance(view, AsyncColumnView)
	@pytest.mark.anyio
	async def test_list_bytes_returns_async_column_view(self, blob_backend):
	io = AsyncBlobIO(SyncToAsyncAdapter(blob_backend))
	view = io[[b"name", b"age"]]
	assert isinstance(view, AsyncColumnView)
	@pytest.mark.anyio
	async def test_bytes_key_to_list(self, blob_backend):
	io = AsyncBlobIO(SyncToAsyncAdapter(blob_backend))
	result = await io[b"name"].to_list()
	assert result == [b"alice", b"bob", b"carol"]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_blob_column_access.py` around lines 149 - 166, The test class
TestAsyncBlobIOColumnAccessMigrated duplicates three tests already present in
TestAsyncBlobIOColumnAccess; remove the redundant class definition
(TestAsyncBlobIOColumnAccessMigrated) including its methods
test_bytes_key_returns_async_column_view,
test_list_bytes_returns_async_column_view, and test_bytes_key_to_list so only
the original tests remain; ensure no other references to
TestAsyncBlobIOColumnAccessMigrated exist and that the remaining tests still
create AsyncBlobIO(SyncToAsyncAdapter(blob_backend)) and assert isinstance(...,
AsyncColumnView) as before.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/asebytes/lmdb/_blob_backend.py (1)

149-158: ⚠️ Potential issue | 🟡 Minor

Negative indices are not supported in _resolve_sort_key, causing inconsistent behavior.

delete (Line 402-403) normalizes negative indices before resolution, but get/update call _resolve_sort_key directly, which raises KeyError for any negative index. Consider normalizing negative indices consistently across all public methods, or documenting that negative indices are only supported for delete.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/lmdb/_blob_backend.py` around lines 149 - 158, _current
behavior_: _resolve_sort_key raises KeyError for negative indices while delete
normalizes negatives before calling it; to fix, make negative-index handling
consistent. Update _resolve_sort_key to accept negative indices by normalizing
index = index + total_count (compute total_count as sum(self._block_sizes) or
use existing length property if present), then validate range and proceed to
walk _block_sizes/_blocks as before; alternatively, update callers (get/update)
to normalize negatives exactly like delete does. Touchpoints: _resolve_sort_key,
_block_sizes, _blocks, and callers get/update/delete — ensure the same
normalization logic is applied and out-of-range indices still raise KeyError.

♻️ Duplicate comments (1)

src/asebytes/_async_views.py (1)

311-323: ⚠️ Potential issue | 🟠 Major

Handle negative-index underflow and placeholder rows in _materialize().

This path still allows underflowed negative indices and can call .get() on None rows in the multi-key branch.

🐛 Proposed fix

     async def _materialize(self):
         idx = self._index
         if idx < 0:
             try:
                 n = len(self._parent)
             except TypeError:
                 n = await self._parent.len()
             idx += n
+            if idx < 0:
+                raise IndexError(self._index)
         if self._single:
             values = await self._parent._read_column(self._keys[0], [idx])
             return values[0]
         row = await self._parent._read_row(idx, keys=self._keys)
+        if row is None:
+            return None
         return [row.get(k) for k in self._keys]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_views.py` around lines 311 - 323, In _materialize(),
after converting a negative self._index by adding n (from self._parent or await
self._parent.len()), clamp/validate the resulting idx and raise IndexError if it
is still < 0 or >= n to prevent underflow/overflow; then in the multi-key branch
after awaiting self._parent._read_row(idx, keys=self._keys) verify that row is
not None (and raise IndexError if it is) before calling row.get(k) for each key
so you don't call .get() on a placeholder None row; reference symbols:
_materialize, self._index, self._parent.len(), _read_column, _read_row,
self._keys, self._single.

🧹 Nitpick comments (3)

src/asebytes/lmdb/_blob_backend.py (1)

516-518: Consider adding strict=True to zip() for defensive coding.

While sort_keys is derived from len(values) ensuring equal lengths, adding strict=True explicitly documents this invariant and catches any future bugs where lengths might diverge.

♻️ Proposed fix

-            for sort_key, item in zip(sort_keys, values):
+            for sort_key, item in zip(sort_keys, values, strict=True):

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/lmdb/_blob_backend.py` around lines 516 - 518, In the loop that
pairs sort_keys and values (the for loop iterating "for sort_key, item in
zip(sort_keys, values):" in _blob_backend.py), make the zip defensive by using
zip(sort_keys, values, strict=True) so mismatched lengths raise immediately;
update any tests or callers if they depended on silent truncation and keep the
existing None-to-{} fallback for item.

src/asebytes/lmdb/_backend.py (2)

206-210: Consider checking index before serialization.

The index validation happens after the data is serialized (Line 208 serializes, Line 209 validates). Moving _check_index earlier avoids unnecessary work when the index is invalid.

♻️ Proposed fix

 def update(self, index: int, data: dict[str, Any]) -> None:
     """Optimized partial update -- only serializes and writes changed keys."""
+    self._check_index(index)
     raw = {k.encode(): msgpack.packb(v, default=m.encode) for k, v in data.items()}
-    self._check_index(index)
     self._store.update(index, raw)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/lmdb/_backend.py` around lines 206 - 210, The update method
currently serializes the data before validating the index, doing unnecessary
work when the index is invalid; move the call to _check_index(index) to the top
of update (before building raw via msgpack.packb) so the index is validated
first, then serialize the data and call self._store.update(index, raw) as before
(refer to the update method, _check_index, msgpack.packb and
self._store.update).

---

70-108: Significant code duplication between LMDBObjectReadBackend and LMDBObjectBackend.

The iter_rows, get_many, and get_column implementations are nearly identical in both classes. Consider extracting shared logic into a mixin or helper to reduce maintenance burden.

♻️ Suggested approach: Extract a mixin

class _LMDBReadMixin:
    """Shared single-transaction read overrides for LMDB backends."""
    
    _store: LMDBBlobBackend  # type hint for IDE support

    def iter_rows(
        self, indices: list[int], keys: list[str] | None = None
    ) -> Iterator[dict[str, Any] | None]:
        byte_keys = [k.encode() for k in keys] if keys is not None else None
        with self._store.env.begin() as txn:
            for i in indices:
                raw = self._store.get_with_txn(txn, i, byte_keys)
                yield None if raw is None else _deserialize_row(raw)

    def get_many(
        self, indices: list[int], keys: list[str] | None = None
    ) -> list[dict[str, Any] | None]:
        byte_keys = [k.encode() for k in keys] if keys is not None else None
        with self._store.env.begin() as txn:
            return [
                None if (raw := self._store.get_with_txn(txn, i, byte_keys)) is None
                else _deserialize_row(raw)
                for i in indices
            ]

    def get_column(self, key: str, indices: list[int] | None = None) -> list[Any]:
        if indices is None:
            indices = list(range(len(self)))
        byte_key = key.encode()
        result = []
        with self._store.env.begin() as txn:
            for i in indices:
                try:
                    raw = self._store.get_with_txn(txn, i, [byte_key])
                except KeyError:
                    result.append(None)
                    continue
                if raw is None or byte_key not in raw:
                    result.append(None)
                else:
                    result.append(msgpack.unpackb(raw[byte_key], object_hook=m.decode))
        return result


class LMDBObjectReadBackend(_LMDBReadMixin, BlobToObjectReadAdapter):
    ...

class LMDBObjectBackend(_LMDBReadMixin, BlobToObjectReadWriteAdapter):
    ...

Also applies to: 164-202

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/lmdb/_backend.py` around lines 70 - 108, There is duplicated
read-only LMDB logic in LMDBObjectReadBackend and LMDBObjectBackend (iter_rows,
get_many, get_column); extract that shared logic into a single mixin (e.g.
_LMDBReadMixin) that provides iter_rows, get_many, and get_column and declares
_store: LMDBBlobBackend for type hints, then have LMDBObjectReadBackend and
LMDBObjectBackend inherit the mixin (e.g. class
LMDBObjectReadBackend(_LMDBReadMixin, BlobToObjectReadAdapter) and class
LMDBObjectBackend(_LMDBReadMixin, BlobToObjectReadWriteAdapter)) and remove the
duplicated methods from those classes; preserve existing behavior (byte key
encoding, txn begin, use of _deserialize_row, msgpack.unpackb and m.decode, and
KeyError handling) when moving the implementations.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/asebytes/_async_views.py`:
- Around line 451-456: When handling deferred list selectors in the __getitem__
path, validate that every element of the incoming list `key` is an int when
`self._indices` is None; if any element is not an int raise a TypeError with a
clear message. Update the branch that constructs and returns
`AsyncColumnView(self._parent, ...)` to check the list contents before
returning, and apply the same validation in the parallel branch (the other
deferred list-handling block around the secondary occurrence) so both places
reject non-int lists early instead of accepting arbitrary lists.
- Around line 401-403: In to_dict(), when iterating over rows and self._keys,
replace direct indexing row[k] with safe access (e.g., row.get(k)) to avoid
KeyError for sparse/partial rows and to match the behavior of to_list(); keep
the existing None check for a missing row (i.e., if row is None append None) and
otherwise append the value returned by get so missing keys yield None instead of
raising.

In `@src/asebytes/lmdb/_blob_backend.py`:
- Around line 578-601: The update method currently relies on _resolve_sort_key
to check the index and that can raise KeyError for out-of-range indexes, causing
inconsistency with delete which raises IndexError; change update to perform an
explicit bounds check and raise IndexError for out-of-range indexes (either by
calling a count accessor used elsewhere or by wrapping the _resolve_sort_key
call in try/except and re-raising an IndexError), e.g., check the index before
opening the txn or catch KeyError from _resolve_sort_key inside update and raise
IndexError to match delete's behavior; reference methods: update,
_resolve_sort_key, delete, and _save_schema.

---

Outside diff comments:
In `@src/asebytes/lmdb/_blob_backend.py`:
- Around line 149-158: _current behavior_: _resolve_sort_key raises KeyError for
negative indices while delete normalizes negatives before calling it; to fix,
make negative-index handling consistent. Update _resolve_sort_key to accept
negative indices by normalizing index = index + total_count (compute total_count
as sum(self._block_sizes) or use existing length property if present), then
validate range and proceed to walk _block_sizes/_blocks as before;
alternatively, update callers (get/update) to normalize negatives exactly like
delete does. Touchpoints: _resolve_sort_key, _block_sizes, _blocks, and callers
get/update/delete — ensure the same normalization logic is applied and
out-of-range indices still raise KeyError.

---

Duplicate comments:
In `@src/asebytes/_async_views.py`:
- Around line 311-323: In _materialize(), after converting a negative
self._index by adding n (from self._parent or await self._parent.len()),
clamp/validate the resulting idx and raise IndexError if it is still < 0 or >= n
to prevent underflow/overflow; then in the multi-key branch after awaiting
self._parent._read_row(idx, keys=self._keys) verify that row is not None (and
raise IndexError if it is) before calling row.get(k) for each key so you don't
call .get() on a placeholder None row; reference symbols: _materialize,
self._index, self._parent.len(), _read_column, _read_row, self._keys,
self._single.

---

Nitpick comments:
In `@src/asebytes/lmdb/_backend.py`:
- Around line 206-210: The update method currently serializes the data before
validating the index, doing unnecessary work when the index is invalid; move the
call to _check_index(index) to the top of update (before building raw via
msgpack.packb) so the index is validated first, then serialize the data and call
self._store.update(index, raw) as before (refer to the update method,
_check_index, msgpack.packb and self._store.update).
- Around line 70-108: There is duplicated read-only LMDB logic in
LMDBObjectReadBackend and LMDBObjectBackend (iter_rows, get_many, get_column);
extract that shared logic into a single mixin (e.g. _LMDBReadMixin) that
provides iter_rows, get_many, and get_column and declares _store:
LMDBBlobBackend for type hints, then have LMDBObjectReadBackend and
LMDBObjectBackend inherit the mixin (e.g. class
LMDBObjectReadBackend(_LMDBReadMixin, BlobToObjectReadAdapter) and class
LMDBObjectBackend(_LMDBReadMixin, BlobToObjectReadWriteAdapter)) and remove the
duplicated methods from those classes; preserve existing behavior (byte key
encoding, txn begin, use of _deserialize_row, msgpack.unpackb and m.decode, and
KeyError handling) when moving the implementations.

In `@src/asebytes/lmdb/_blob_backend.py`:
- Around line 516-518: In the loop that pairs sort_keys and values (the for loop
iterating "for sort_key, item in zip(sort_keys, values):" in _blob_backend.py),
make the zip defensive by using zip(sort_keys, values, strict=True) so
mismatched lengths raise immediately; update any tests or callers if they
depended on silent truncation and keep the existing None-to-{} fallback for
item.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between da1e0b3 and ae4e1a7.

📒 Files selected for processing (4)

• src/asebytes/_async_views.py
• src/asebytes/lmdb/_backend.py
• src/asebytes/lmdb/_blob_backend.py
• tests/test_object_io_none_indexing.py

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Validate list selector element types in deferred __getitem__ paths.

Both branches accept arbitrary list as row selectors when _indices is None. Non-int lists should fail early with TypeError.

🛠️ Proposed fix

             if isinstance(key, list):
+                if any(not isinstance(i, int) for i in key):
+                    raise TypeError("List selectors must contain integer row indices.")
                 return AsyncColumnView(
                     self._parent,
                     self._keys[0] if self._single else self._keys,
                     key,
                 )

             if isinstance(key, list):
+                if any(not isinstance(i, int) for i in key):
+                    raise TypeError("List selectors must contain integer row indices.")
                 return AsyncASEColumnView(
                     self._parent,
                     self._keys[0] if self._single else self._keys,
                     key,
                 )

Also applies to: 624-629

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/_async_views.py` around lines 451 - 456, When handling deferred
list selectors in the __getitem__ path, validate that every element of the
incoming list `key` is an int when `self._indices` is None; if any element is
not an int raise a TypeError with a clear message. Update the branch that
constructs and returns `AsyncColumnView(self._parent, ...)` to check the list
contents before returning, and apply the same validation in the parallel branch
(the other deferred list-handling block around the secondary occurrence) so both
places reject non-int lists early instead of accepting arbitrary lists.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

update raises KeyError for out-of-bounds index; consider IndexError for consistency.

When index is out of range, _resolve_sort_key raises KeyError. However, delete (Line 405) raises IndexError for the same condition. Consider adding explicit bounds checking with IndexError for API consistency.

💡 Proposed fix

 def update(self, index: int, data: dict[bytes, bytes]) -> None:
     """Optimized partial update -- only writes changed keys."""
     if not data:
         return

     with self.env.begin(write=True) as txn:
         self._ensure_cache(txn)
+        current_count = self._get_count(txn)
+        if index < 0 or index >= current_count:
+            raise IndexError(f"Index {index} out of range [0, {current_count})")
         sort_key = self._resolve_sort_key(index)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/asebytes/lmdb/_blob_backend.py` around lines 578 - 601, The update method
currently relies on _resolve_sort_key to check the index and that can raise
KeyError for out-of-range indexes, causing inconsistency with delete which
raises IndexError; change update to perform an explicit bounds check and raise
IndexError for out-of-range indexes (either by calling a count accessor used
elsewhere or by wrapping the _resolve_sort_key call in try/except and re-raising
an IndexError), e.g., check the index before opening the txn or catch KeyError
from _resolve_sort_key inside update and raise IndexError to match delete's
behavior; reference methods: update, _resolve_sort_key, delete, and
_save_schema.