[bug] Fix range reads on whole-file caches and crossed ranges in cat_file

[nvbkdw]

filecache and simplecache serve range reads out of the local copy of a file rather than from the remote. Several defects in that path make range reads on a cached file fail, and chasing them down surfaced one more in the shared base-class implementation.

WholeFileCacheFileSystem / SimpleCacheFileSystem

1. Negative start/end raise OSError(EINVAL)

AbstractFileSystem.cat_file documents negative offsets as counting backwards from the end of the file ("like usual python slices"), and remote backends such as s3fs implement them as an HTTP suffix range. WholeFileCacheFileSystem._cat_file passed the value straight to f.seek(start), which asks for a negative absolute position:

fs = fsspec.filesystem("simplecache", target_protocol="s3")
fs.cat_file(path, start=-260)   # OSError: [Errno 22] Invalid argument

The failure happens after the whole object has been downloaded into the cache, so the local copy that could trivially serve the read is fetched and then discarded.

2. AttributeError: '_io.BufferedReader' object has no attribute 'size'

The sync path goes through AbstractFileSystem.cat_file, which needs f.size to resolve a negative offset. Both _open implementations here return a bare io.BufferedReader, so it raises. LocalFileOpener already attaches size to the raw file object (local.py), so this follows the existing idiom.

3. _check_file return type is not handled consistently

WholeFileCacheFileSystem._check_file returns a (detail, path) tuple, while the SimpleCacheFileSystem override returns the path alone. _cat_file and _cat_ranges assume the latter, so on filecache:

fs.cat_file(path)          # populate cache
asyncio.run(fs._cat_file(path))    # TypeError: expected str, bytes or os.PathLike object, not tuple
asyncio.run(fs._cat_ranges([path], [2], [5]))  # AttributeError: 'tuple' object has no attribute 'startswith'

_cat_ranges additionally never downloaded an uncached path, because it tests fn is None while the cache-miss sentinel here is False.

4. A range crossed by one byte reads the rest of the file

_cat_file computed the read length as end - f.tell(). read() treats a negative length as "to the end of the file", so a range crossed by exactly one byte hit that sentinel by accident. Resolving negative offsets to absolute ones (defect 1) makes this reachable from ordinary input:

# 10-byte cached file
asyncio.run(fs._cat_file(path, start=6, end=-5))   # b"6789", expected b""
asyncio.run(fs._cat_file(path, start=8, end=-5))   # ValueError: read length must be non-negative or -1

Note the two different failures: a one-byte crossing computes -1 and silently returns data, anything wider computes -2 or less and raises.

5. Repeated paths in _cat_ranges resolve to the cache-miss sentinel

_cat_ranges used rset purely as a membership set, so only the first occurrence of an uncached path was assigned a local path. Every later occurrence appended the falsy sentinel to lpaths, and LocalFileSystem.cat_ranges returned an exception object in that slot — silently, since on_error defaults to "return":

asyncio.run(fs._cat_ranges(["/d1", "/d1", "/d1"], [2, 0, 7], [5, 3, 9]))
# [b'234', AttributeError("'bool' object has no attribute 'startswith'"), AttributeError(...)]

Asking for several ranges of one object is the normal access pattern for sharded formats, which is exactly what this PR is about.

AbstractFileSystem.cat_file

Defect 4 is not specific to the caches — the base implementation computes the length the same way, so it is reachable on every backend that does not override cat_file:

fsspec.filesystem("file").cat_file(fn, start=6, end=-5)   # b"6789", expected b""

The offsets were also normalized inconsistently: a negative start clamped at 0, a negative end did not, so a suffix longer than the file raised instead of yielding empty (data[:-100] is b"", but cat_file(end=-100) raised ValueError). And None was handled by control flow rather than as a value, giving three different mechanisms for one idea.

slice.indices already performs exactly this normalization — clamping included — so both offsets now resolve through it, which is literally what the docstring promises. It needs the size up front and not every file object has one (TarFileSystem yields a tarfile.ExFileObject, which would then break even a plain full-file read), so the size is consulted only when an offset is actually negative — the same condition under which the old code read f.size. No backend loses a case that previously worked.

How this came up

zarr v3 sharded arrays store the inner-chunk index at the end of each shard object, so every read begins with a suffix-range request. Reading such an array over simplecache::s3:// downloads the full shard and use negative index to read files.

Changes

fsspec/implementations/cached.py

• _cat_file: resolve negative start/end against the file size before seeking, mirroring AbstractFileSystem.cat_file, and clamp the read length so a crossed range cannot collide with read()'s to-EOF sentinel.
• _open (both classes): expose size on the returned file object.
• _cat_file / _cat_ranges: accept either _check_file return shape, and treat any falsy value as a cache miss.
• _cat_ranges: track the resolved local path per unique path, so repeated paths all resolve.

fsspec/spec.py

• cat_file: resolve start/end through slice.indices against the file size, consulted only when an offset is negative.

Tests

• Negative offsets, crossed ranges, and cached cat_ranges on both filecache and simplecache, over sync cat_file and the awaited _cat_file that async consumers (e.g. zarr's FsspecStore) use.
• Repeated uncached paths through _cat_ranges, against an async target so the download branch is actually exercised.
• cat_file start/end combinations checked against data[start:end] directly, since python slice semantics are what the docstring promises.

[fallenmi]

filecache still does not record files downloaded through the async range paths, so the main async/Zarr use case repeatedly fetches the full remote object instead of serving later ranges from the cache.

On exact head 95df12a, I wrapped MemoryFileSystem.get / get_file with counters and awaited the same read twice. For WholeFileCacheFileSystem, both _cat_file and _cat_ranges made one backend download per invocation (counts 1 then 2), and _check_file("/afile") remained False after each successful download. The same probes on SimpleCacheFileSystem made one download total and then returned the cached local path.

The miss branches currently write directly to the hash path but never create/save the metadata that WholeFileCacheFileSystem._check_file() requires. This also means the new repeated-range test's comment “again now that the file is cached” is false for its filecache parameter: the bytes pass because the second invocation silently downloads the object again.

Please preserve the filecache metadata contract after a successful async download and assert the backend download count across a second _cat_file and _cat_ranges call. That is important here because sharded readers repeatedly request suffix/range data from the same full object.

For context, I independently confirmed the intended functional fixes: exact base returns errors/wrong bytes for the added cases, while this head returns the expected ranges. Both touched test files pass (151 passed, 17 skipped), as do pinned Ruff 0.14.3 check/format, compileall, and git diff --check.

Disclosure: OpenAI Codex assisted with this independent review and the deterministic probes above.