From a424eff33be2ff21e49e132f89c1ae7f50036874 Mon Sep 17 00:00:00 2001 From: "agent-kurodo[bot]" <268466204+agent-kurodo[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 04:27:19 +0000 Subject: [PATCH 1/7] docs(specs): add ITL-474 op.File URL-form identity audit design spec Co-Authored-By: Claude Sonnet 4.6 --- ...itl-474-opfile-url-form-identity-design.md | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 superpowers/specs/2026-07-07-itl-474-opfile-url-form-identity-design.md diff --git a/superpowers/specs/2026-07-07-itl-474-opfile-url-form-identity-design.md b/superpowers/specs/2026-07-07-itl-474-opfile-url-form-identity-design.md new file mode 100644 index 00000000..ed00219d --- /dev/null +++ b/superpowers/specs/2026-07-07-itl-474-opfile-url-form-identity-design.md @@ -0,0 +1,117 @@ +# Design: `op.File` URL-form identity audit (ITL-474) + +**Date:** 2026-07-07 +**Issue:** ITL-474 +**Status:** Approved + +--- + +## Overview + +Companion audit to the ENIGMA File System Spec (engmfs) project. `op.File` is +`UPath`-based, so registering `engmfs` as an fsspec protocol makes +`op.File("engm://ephys/x.bin")` work transparently. This spec records the audit +findings and specifies the regression tests that lock in URL-form identity preservation. + +--- + +## Audit Findings + +The audit examined every layer where `op.File` could eagerly resolve a URL-form path +to a host-specific concrete backend path. **No issues were found.** URL-form identity is +already preserved throughout. + +### Construction (`File.__init__`) + +`File.__init__` calls only `.is_symlink()`, `.exists()`, `.is_dir()`, and `.is_file()` +on `self.__wrapped__` — all are read-only filesystem checks that go through the fsspec +backend without resolving the URL to a concrete local path. No `.resolve()` or +`.absolute()` is called. + +### `str()` + +`ProxyUPath.__str__` delegates to `UPath.__str__()`, which returns the URL form +verbatim. `str(File("engm://ns/x.bin"))` = `"engm://ns/x.bin"`. + +### Python `hash()` + +`ProxyUPath.__hash__` delegates to `UPath.__hash__()`: + +```python +def __hash__(self) -> int: + return hash((self.protocol, self.__vfspath__())) +``` + +For `File("engm://ns/x.bin")` this is `hash(("engm", "/ns/x.bin"))` — deterministic, +URL-based, and host-independent. + +### `LogicalFile` serialisation + +`python_to_storage` stores `json.dumps({"path": str(value)})`, which encodes the URL +form. `storage_to_python` reconstructs via `File(path)` — the URL string round-trips +intact. + +### Semantic hashing (`FileHandler`) + +`FileHandler.handle()` extracts `self.__wrapped__` (the raw `UPath`) and passes it to +`FileHasher.hash_file()`, which reads file bytes and returns a SHA-256 content hash. +The hash is based on file content, not the path string, making it inherently portable +across hosts. + +### `CachedFileHasher` + +`CachedFileHasher.hash_file()` calls `path.resolve()` before building the `FileHashKey` +cache key. `UPath.resolve()` only normalises `.` and `..` path components — it does not +call any fsspec backend resolution. For a URL like `engm://ns/x.bin` (no `.`/`..`), +`resolve()` returns the path unchanged. The `FileHashKey.path` therefore retains the +URL form. + +--- + +## What Changes + +**No production code changes.** The audit confirms the implementation is already correct. + +The deliverable is a suite of regression tests that: + +1. Verify URL-form identity preservation so it cannot silently regress. +2. Serve as executable documentation of the audit findings. + +--- + +## Regression Test Design + +### Stub filesystem + +Tests use the `memory://` fsspec protocol as a stand-in for `engm://`. It is always +available (bundled with fsspec), has identical non-local-protocol semantics, and requires +no mocking infrastructure beyond writing a file to the in-memory filesystem. + +### Test locations + +| Test class | File | What it covers | +|---|---|---| +| `TestURLFormIdentity` | `tests/test_extension_types/test_file_type.py` | `str()`, `hash()`, `LogicalFile` round-trip | +| `TestCachedFileHasherURLKey` (new method in existing class) | `tests/test_hashing/test_file_hashers.py` | `CachedFileHasher` cache-key URL preservation | + +### `TestURLFormIdentity` — five tests + +**Fixture:** Creates a file at `memory://ns/x.bin` with known content before the test +class runs; tears it down after. + +| Test | Assertion | +|---|---| +| `test_str_preserves_url_form` | `str(File("memory://ns/x.bin")) == "memory://ns/x.bin"` | +| `test_hash_is_stable` | `hash(File("memory://ns/x.bin")) == hash(File("memory://ns/x.bin"))` | +| `test_hash_equals_upath_protocol_tuple` | `hash(File(...))` equals `hash(("memory", "/ns/x.bin"))` — confirms it is the URL-tuple hash, not any resolved value | +| `test_logical_file_storage_encodes_url` | `json.loads(python_to_storage(file))["path"] == "memory://ns/x.bin"` | +| `test_logical_file_round_trip_preserves_url` | `str(storage_to_python(python_to_storage(file))) == "memory://ns/x.bin"` | + +### `CachedFileHasher` URL cache-key test + +Added to `tests/test_hashing/test_file_hashers.py` as a new method on the existing +`TestCachedFileHasher` class (or as a standalone test if no such class exists). + +| Test | Assertion | +|---|---| +| `test_cache_key_preserves_url_form` | After hashing `memory://ns/x.bin`, the `FileHashKey.path` in the cache has `str(key.path) == "memory://ns/x.bin"` | From 8ec081c156f1209660c6e244cd85d3c636353d7c Mon Sep 17 00:00:00 2001 From: "agent-kurodo[bot]" <268466204+agent-kurodo[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 04:34:00 +0000 Subject: [PATCH 2/7] docs(plans): add ITL-474 op.File URL-form identity regression tests plan Co-Authored-By: Claude Sonnet 4.6 --- ...-07-07-itl-474-opfile-url-form-identity.md | 223 ++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 docs/metamorphic/plans/2026-07-07-itl-474-opfile-url-form-identity.md diff --git a/docs/metamorphic/plans/2026-07-07-itl-474-opfile-url-form-identity.md b/docs/metamorphic/plans/2026-07-07-itl-474-opfile-url-form-identity.md new file mode 100644 index 00000000..c125074a --- /dev/null +++ b/docs/metamorphic/plans/2026-07-07-itl-474-opfile-url-form-identity.md @@ -0,0 +1,223 @@ +# op.File URL-form Identity — Regression Tests Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use sensei:subagent-driven-development (recommended) or sensei:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add regression tests that lock in URL-form identity preservation for `op.File` across construction, `str()`, `hash()`, `LogicalFile` serialisation, and `CachedFileHasher` cache-key behaviour. + +**Architecture:** Two test additions — a new `TestURLFormIdentity` class in the existing `test_file_type.py` (covering `File` and `LogicalFile`) and one new method in the existing `TestCachedFileHasher` class in `test_file_hashers.py`. No production code changes. The `memory://` fsspec protocol (always bundled with fsspec) stands in for `engm://`. + +**Tech Stack:** pytest, fsspec (memory filesystem), upath.UPath, orcapod.extension_types.file_type.File/LogicalFile, orcapod.hashing.file_hashers.CachedFileHasher/FileHasher/FileHashKey, orcapod.hashing.hash_cachers.InMemoryHashCacher + +--- + +### Task 1: Add `TestURLFormIdentity` to `test_file_type.py` + +**Files:** +- Modify: `tests/test_extension_types/test_file_type.py` (append new class at end of file) + +- [ ] **Step 1: Append the new test class** + +Open `tests/test_extension_types/test_file_type.py` and append the following block at the **end of the file**, after the last existing class: + +```python +class TestURLFormIdentity: + """Regression tests: op.File preserves URL-form identity for non-local protocols. + + Uses ``memory://`` as a stable, always-available stand-in for ``engm://``. + The same invariants hold for any non-local fsspec protocol. + """ + + @pytest.fixture(autouse=True) + def memory_file(self): + """Create ``memory://ns/x.bin`` with known content; clean up after.""" + import fsspec + fs = fsspec.filesystem("memory") + fs.mkdir("/ns", exist_ok=True) + with fs.open("/ns/x.bin", "wb") as fh: + fh.write(b"url-identity-test-content") + yield + fs.rm("/ns/x.bin") + + def test_str_preserves_url_form(self): + f = File("memory://ns/x.bin") + assert str(f) == "memory://ns/x.bin", ( + f"Expected URL form 'memory://ns/x.bin', got {str(f)!r}" + ) + + def test_hash_is_stable(self): + h1 = hash(File("memory://ns/x.bin")) + h2 = hash(File("memory://ns/x.bin")) + assert h1 == h2, "hash() must be identical across two constructions of the same URL" + + def test_hash_equals_upath_protocol_tuple(self): + # UPath.__hash__ = hash((protocol, vfspath)) + # For memory://ns/x.bin: protocol="memory", vfspath="/ns/x.bin" + # This test pins the exact hash contract so any regression is immediately visible. + expected = hash(("memory", "/ns/x.bin")) + actual = hash(File("memory://ns/x.bin")) + assert actual == expected, ( + f"hash(File('memory://ns/x.bin')) should equal hash(('memory', '/ns/x.bin')), " + f"got {actual} vs {expected}" + ) + + def test_logical_file_storage_encodes_url(self): + f = File("memory://ns/x.bin") + lt = LogicalFile() + storage = lt.python_to_storage(f) + data = json.loads(storage) + assert data["path"] == "memory://ns/x.bin", ( + f"python_to_storage must encode URL form; got path={data['path']!r}" + ) + + def test_logical_file_round_trip_preserves_url(self): + f = File("memory://ns/x.bin") + lt = LogicalFile() + recovered = lt.storage_to_python(lt.python_to_storage(f)) + assert str(recovered) == "memory://ns/x.bin", ( + f"storage_to_python(python_to_storage(f)) must preserve URL form; " + f"got {str(recovered)!r}" + ) +``` + +- [ ] **Step 2: Run the new tests to confirm they pass** + +```bash +uv run pytest tests/test_extension_types/test_file_type.py::TestURLFormIdentity -v +``` + +Expected output (all five pass): +``` +PASSED tests/test_extension_types/test_file_type.py::TestURLFormIdentity::test_str_preserves_url_form +PASSED tests/test_extension_types/test_file_type.py::TestURLFormIdentity::test_hash_is_stable +PASSED tests/test_extension_types/test_file_type.py::TestURLFormIdentity::test_hash_equals_upath_protocol_tuple +PASSED tests/test_extension_types/test_file_type.py::TestURLFormIdentity::test_logical_file_storage_encodes_url +PASSED tests/test_extension_types/test_file_type.py::TestURLFormIdentity::test_logical_file_round_trip_preserves_url +5 passed +``` + +- [ ] **Step 3: Run the full `test_file_type.py` to confirm nothing regressed** + +```bash +uv run pytest tests/test_extension_types/test_file_type.py -v +``` + +Expected: all pre-existing tests still pass. + +- [ ] **Step 4: Commit** + +```bash +git add tests/test_extension_types/test_file_type.py +git commit -m "test(file_type): add TestURLFormIdentity regression tests (ITL-474)" +``` + +--- + +### Task 2: Add URL cache-key test to `TestCachedFileHasher` + +**Files:** +- Modify: `tests/test_hashing/test_file_hashers.py` (append one method to `TestCachedFileHasher`) + +- [ ] **Step 1: Append the new test method to `TestCachedFileHasher`** + +Open `tests/test_hashing/test_file_hashers.py` and append the following method at the **end of the `TestCachedFileHasher` class** (after `test_clear_cache_forces_rehash`): + +```python + def test_cache_key_preserves_url_form(self): + """CachedFileHasher must not resolve URL-form paths to concrete backend paths. + + UPath.resolve() only normalises ``.``/``..`` components — it does not call + any fsspec backend resolution. The FileHashKey.path must therefore retain + the original URL string for non-local protocols. + """ + import fsspec + from upath import UPath + + fs = fsspec.filesystem("memory") + fs.mkdir("/ns", exist_ok=True) + with fs.open("/ns/cache_key_test.bin", "wb") as fh: + fh.write(b"cache-key-url-test") + + try: + inner = FileHasher(algorithm="sha256") + cacher = InMemoryHashCacher() + cached = CachedFileHasher(file_hasher=inner, cacher=cacher) + + cached.hash_file(UPath("memory://ns/cache_key_test.bin")) + + keys = list(cacher._cache.keys()) + assert len(keys) == 1, f"Expected 1 cache entry, got {len(keys)}" + key = keys[0] + assert str(key.path) == "memory://ns/cache_key_test.bin", ( + f"Cache key path must preserve URL form; got {str(key.path)!r}" + ) + finally: + fs.rm("/ns/cache_key_test.bin") +``` + +- [ ] **Step 2: Run the new test to confirm it passes** + +```bash +uv run pytest tests/test_hashing/test_file_hashers.py::TestCachedFileHasher::test_cache_key_preserves_url_form -v +``` + +Expected: +``` +PASSED tests/test_hashing/test_file_hashers.py::TestCachedFileHasher::test_cache_key_preserves_url_form +1 passed +``` + +- [ ] **Step 3: Run the full `test_file_hashers.py` to confirm nothing regressed** + +```bash +uv run pytest tests/test_hashing/test_file_hashers.py -v +``` + +Expected: all pre-existing tests still pass. + +- [ ] **Step 4: Commit** + +```bash +git add tests/test_hashing/test_file_hashers.py +git commit -m "test(file_hashers): add URL cache-key preservation test for CachedFileHasher (ITL-474)" +``` + +--- + +### Task 3: Full test suite verification and PR + +- [ ] **Step 1: Run the full test suite** + +```bash +uv run pytest tests/ -x -q +``` + +Expected: all tests pass, no new failures. + +- [ ] **Step 2: Push the branch** + +```bash +git push -u origin eywalker/itl-474-opfile-audit-url-form-identity-preservation-for-engmfs +``` + +- [ ] **Step 3: Open a PR against `main`** + +```bash +gh pr create \ + --base main \ + --title "test(file_type): add URL-form identity regression tests (ITL-474)" \ + --body "$(cat <<'EOF' +## Summary + +Audit result for ITL-474: `op.File` already preserves URL-form identity correctly at every layer — no production code changes needed. + +This PR adds regression tests that lock in that behaviour: + +- `TestURLFormIdentity` (5 tests) in `tests/test_extension_types/test_file_type.py`: covers `str()`, `hash()` stability, hash equals `hash((protocol, vfspath))`, `LogicalFile.python_to_storage()` encoding, and `LogicalFile` round-trip. +- `test_cache_key_preserves_url_form` in `tests/test_hashing/test_file_hashers.py`: confirms `CachedFileHasher` does not resolve URL-form paths to concrete backend paths when building the `FileHashKey`. + +Both test classes use `memory://` (always-available fsspec protocol) as a stand-in for `engm://`. + +Closes ITL-474 +EOF +)" +``` From 9ab558cef47e0f47ccc84e14068259a5dca2b5b4 Mon Sep 17 00:00:00 2001 From: "agent-kurodo[bot]" <268466204+agent-kurodo[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 04:36:27 +0000 Subject: [PATCH 3/7] test(file_type): add TestURLFormIdentity regression tests (ITL-474) --- tests/test_extension_types/test_file_type.py | 69 ++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tests/test_extension_types/test_file_type.py b/tests/test_extension_types/test_file_type.py index 1054c3dc..96bec63b 100644 --- a/tests/test_extension_types/test_file_type.py +++ b/tests/test_extension_types/test_file_type.py @@ -166,3 +166,72 @@ class _Stub(ProxyUPath): pass assert not issubclass(_Stub, os.PathLike) + + +class TestURLFormIdentity: + """Regression tests: op.File preserves URL-form identity for non-local protocols. + + Uses ``memory://`` as a stable, always-available stand-in for ``engm://``. + The same invariants hold for any non-local fsspec protocol. + """ + + @pytest.fixture(autouse=True) + def memory_file(self): + """Create ``memory://ns/x.bin`` with known content; clean up after.""" + import fsspec + fs = fsspec.filesystem("memory") + # Clean up any pre-existing state + if "/ns/x.bin" in fs.store: + fs.rm("/ns/x.bin") + if "/ns" in fs.pseudo_dirs: + fs.pseudo_dirs.remove("/ns") + # Create directory and file + fs.mkdir("/ns", create_parents=True) + with fs.open("/ns/x.bin", "wb") as fh: + fh.write(b"url-identity-test-content") + yield + # Clean up after test + if "/ns/x.bin" in fs.store: + fs.rm("/ns/x.bin") + if "/ns" in fs.pseudo_dirs: + fs.pseudo_dirs.remove("/ns") + + def test_str_preserves_url_form(self): + f = File("memory://ns/x.bin") + assert str(f) == "memory://ns/x.bin", ( + f"Expected URL form 'memory://ns/x.bin', got {str(f)!r}" + ) + + def test_hash_is_stable(self): + h1 = hash(File("memory://ns/x.bin")) + h2 = hash(File("memory://ns/x.bin")) + assert h1 == h2, "hash() must be identical across two constructions of the same URL" + + def test_hash_equals_upath_protocol_tuple(self): + # UPath.__hash__ = hash((protocol, vfspath)) + # For memory://ns/x.bin: protocol="memory", vfspath="/ns/x.bin" + # This test pins the exact hash contract so any regression is immediately visible. + expected = hash(("memory", "/ns/x.bin")) + actual = hash(File("memory://ns/x.bin")) + assert actual == expected, ( + f"hash(File('memory://ns/x.bin')) should equal hash(('memory', '/ns/x.bin')), " + f"got {actual} vs {expected}" + ) + + def test_logical_file_storage_encodes_url(self): + f = File("memory://ns/x.bin") + lt = LogicalFile() + storage = lt.python_to_storage(f) + data = json.loads(storage) + assert data["path"] == "memory://ns/x.bin", ( + f"python_to_storage must encode URL form; got path={data['path']!r}" + ) + + def test_logical_file_round_trip_preserves_url(self): + f = File("memory://ns/x.bin") + lt = LogicalFile() + recovered = lt.storage_to_python(lt.python_to_storage(f)) + assert str(recovered) == "memory://ns/x.bin", ( + f"storage_to_python(python_to_storage(f)) must preserve URL form; " + f"got {str(recovered)!r}" + ) From e0ba5d4c9de261c4b55b56c2582014def99c4bdd Mon Sep 17 00:00:00 2001 From: "agent-kurodo[bot]" <268466204+agent-kurodo[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 04:39:22 +0000 Subject: [PATCH 4/7] test(file_type): fix fixture teardown and import style in TestURLFormIdentity (ITL-474) --- tests/test_extension_types/test_file_type.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/test_extension_types/test_file_type.py b/tests/test_extension_types/test_file_type.py index 96bec63b..1c94e55e 100644 --- a/tests/test_extension_types/test_file_type.py +++ b/tests/test_extension_types/test_file_type.py @@ -6,6 +6,7 @@ import os import pathlib +import fsspec import pytest import pyarrow as pa from upath import UPath @@ -178,13 +179,14 @@ class TestURLFormIdentity: @pytest.fixture(autouse=True) def memory_file(self): """Create ``memory://ns/x.bin`` with known content; clean up after.""" - import fsspec fs = fsspec.filesystem("memory") # Clean up any pre-existing state if "/ns/x.bin" in fs.store: fs.rm("/ns/x.bin") - if "/ns" in fs.pseudo_dirs: - fs.pseudo_dirs.remove("/ns") + try: + fs.rmdir("/ns") + except (FileNotFoundError, OSError): + pass # Create directory and file fs.mkdir("/ns", create_parents=True) with fs.open("/ns/x.bin", "wb") as fh: @@ -193,8 +195,10 @@ def memory_file(self): # Clean up after test if "/ns/x.bin" in fs.store: fs.rm("/ns/x.bin") - if "/ns" in fs.pseudo_dirs: - fs.pseudo_dirs.remove("/ns") + try: + fs.rmdir("/ns") + except (FileNotFoundError, OSError): + pass def test_str_preserves_url_form(self): f = File("memory://ns/x.bin") From eae37babec1721a818224545dac862916bb4543f Mon Sep 17 00:00:00 2001 From: "agent-kurodo[bot]" <268466204+agent-kurodo[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 04:40:57 +0000 Subject: [PATCH 5/7] test(file_hashers): add URL cache-key preservation test for CachedFileHasher (ITL-474) --- tests/test_hashing/test_file_hashers.py | 31 +++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/test_hashing/test_file_hashers.py b/tests/test_hashing/test_file_hashers.py index 4a414b4a..1f0364a7 100644 --- a/tests/test_hashing/test_file_hashers.py +++ b/tests/test_hashing/test_file_hashers.py @@ -267,3 +267,34 @@ def test_clear_cache_forces_rehash(self, tmp_path): second = cached.hash_file(f) assert first.digest != second.digest + + def test_cache_key_preserves_url_form(self): + """CachedFileHasher must not resolve URL-form paths to concrete backend paths. + + UPath.resolve() only normalises ``.``/``..`` components — it does not call + any fsspec backend resolution. The FileHashKey.path must therefore retain + the original URL string for non-local protocols. + """ + import fsspec + from upath import UPath + + fs = fsspec.filesystem("memory") + fs.mkdir("/ns", exist_ok=True) + with fs.open("/ns/cache_key_test.bin", "wb") as fh: + fh.write(b"cache-key-url-test") + + try: + inner = FileHasher(algorithm="sha256") + cacher = InMemoryHashCacher() + cached = CachedFileHasher(file_hasher=inner, cacher=cacher) + + cached.hash_file(UPath("memory://ns/cache_key_test.bin")) + + keys = list(cacher._cache.keys()) + assert len(keys) == 1, f"Expected 1 cache entry, got {len(keys)}" + key = keys[0] + assert str(key.path) == "memory://ns/cache_key_test.bin", ( + f"Cache key path must preserve URL form; got {str(key.path)!r}" + ) + finally: + fs.rm("/ns/cache_key_test.bin") From 23e237b1d0fda15b28deb0c7927fb85b440e7c04 Mon Sep 17 00:00:00 2001 From: "agent-kurodo[bot]" <268466204+agent-kurodo[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 04:43:38 +0000 Subject: [PATCH 6/7] test(file_hashers): move fsspec and UPath imports to module level (ITL-474) --- tests/test_hashing/test_file_hashers.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/test_hashing/test_file_hashers.py b/tests/test_hashing/test_file_hashers.py index 1f0364a7..ff1e2881 100644 --- a/tests/test_hashing/test_file_hashers.py +++ b/tests/test_hashing/test_file_hashers.py @@ -6,7 +6,9 @@ correctly round-trips ContentHash through the cache. """ +import fsspec import pytest +from upath import UPath from orcapod.hashing.file_hashers import CachedFileHasher, FileHasher, FileHashKey from orcapod.hashing.hash_cachers import InMemoryHashCacher @@ -169,7 +171,6 @@ def test_cache_hit_returns_correct_content_hash(self, sample_file): def test_cache_stores_content_hash_directly(self, sample_file): """The cacher stores ContentHash objects keyed by FileHashKey.""" - from upath import UPath inner = FileHasher(algorithm="sha256") cacher = InMemoryHashCacher() cached = CachedFileHasher(file_hasher=inner, cacher=cacher) @@ -275,9 +276,6 @@ def test_cache_key_preserves_url_form(self): any fsspec backend resolution. The FileHashKey.path must therefore retain the original URL string for non-local protocols. """ - import fsspec - from upath import UPath - fs = fsspec.filesystem("memory") fs.mkdir("/ns", exist_ok=True) with fs.open("/ns/cache_key_test.bin", "wb") as fh: From ed3f1d338728a3dac109aaae7b07a885f6e06e1f Mon Sep 17 00:00:00 2001 From: "agent-kurodo[bot]" <268466204+agent-kurodo[bot]@users.noreply.github.com> Date: Tue, 7 Jul 2026 05:26:32 +0000 Subject: [PATCH 7/7] test: address PR review comments (ITL-474) - Replace fs.store membership checks with fs.exists() public API in TestURLFormIdentity fixture (avoids fsspec internals) - Derive expected hash from UPath directly in test_hash_equals_upath_protocol_tuple (no hard-coded vfspath string) - Replace cacher._cache inspection with FileHashKey reconstruction + cacher.get() in test_cache_key_preserves_url_form (uses public API) - Fix spec doc: correct TestCachedFileHasher test name and example path Co-Authored-By: Claude Sonnet 4.6 --- ...itl-474-opfile-url-form-identity-design.md | 4 ++-- tests/test_extension_types/test_file_type.py | 16 +++++++------- tests/test_hashing/test_file_hashers.py | 21 ++++++++++++------- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/superpowers/specs/2026-07-07-itl-474-opfile-url-form-identity-design.md b/superpowers/specs/2026-07-07-itl-474-opfile-url-form-identity-design.md index ed00219d..7c4f7bec 100644 --- a/superpowers/specs/2026-07-07-itl-474-opfile-url-form-identity-design.md +++ b/superpowers/specs/2026-07-07-itl-474-opfile-url-form-identity-design.md @@ -92,7 +92,7 @@ no mocking infrastructure beyond writing a file to the in-memory filesystem. | Test class | File | What it covers | |---|---|---| | `TestURLFormIdentity` | `tests/test_extension_types/test_file_type.py` | `str()`, `hash()`, `LogicalFile` round-trip | -| `TestCachedFileHasherURLKey` (new method in existing class) | `tests/test_hashing/test_file_hashers.py` | `CachedFileHasher` cache-key URL preservation | +| `TestCachedFileHasher.test_cache_key_preserves_url_form` | `tests/test_hashing/test_file_hashers.py` | `CachedFileHasher` cache-key URL preservation | ### `TestURLFormIdentity` — five tests @@ -114,4 +114,4 @@ Added to `tests/test_hashing/test_file_hashers.py` as a new method on the existi | Test | Assertion | |---|---| -| `test_cache_key_preserves_url_form` | After hashing `memory://ns/x.bin`, the `FileHashKey.path` in the cache has `str(key.path) == "memory://ns/x.bin"` | +| `test_cache_key_preserves_url_form` | After hashing `memory://ns/cache_key_test.bin`, reconstruct the `FileHashKey` via `resolve()` + `stat()` and verify a cache hit via `cacher.get()` and that `str(key.path) == "memory://ns/cache_key_test.bin"` | diff --git a/tests/test_extension_types/test_file_type.py b/tests/test_extension_types/test_file_type.py index 1c94e55e..c7c5598d 100644 --- a/tests/test_extension_types/test_file_type.py +++ b/tests/test_extension_types/test_file_type.py @@ -181,7 +181,7 @@ def memory_file(self): """Create ``memory://ns/x.bin`` with known content; clean up after.""" fs = fsspec.filesystem("memory") # Clean up any pre-existing state - if "/ns/x.bin" in fs.store: + if fs.exists("/ns/x.bin"): fs.rm("/ns/x.bin") try: fs.rmdir("/ns") @@ -193,7 +193,7 @@ def memory_file(self): fh.write(b"url-identity-test-content") yield # Clean up after test - if "/ns/x.bin" in fs.store: + if fs.exists("/ns/x.bin"): fs.rm("/ns/x.bin") try: fs.rmdir("/ns") @@ -212,13 +212,15 @@ def test_hash_is_stable(self): assert h1 == h2, "hash() must be identical across two constructions of the same URL" def test_hash_equals_upath_protocol_tuple(self): - # UPath.__hash__ = hash((protocol, vfspath)) - # For memory://ns/x.bin: protocol="memory", vfspath="/ns/x.bin" - # This test pins the exact hash contract so any regression is immediately visible. - expected = hash(("memory", "/ns/x.bin")) + # File.__hash__ delegates to ProxyUPath.__hash__ → UPath.__hash__. + # Derive the expected hash from UPath directly so the test stays focused + # on verifying that File follows the same hash contract as its wrapped UPath, + # rather than hard-coding the internal (protocol, vfspath) representation. + ref = UPath("memory://ns/x.bin") + expected = hash(ref) actual = hash(File("memory://ns/x.bin")) assert actual == expected, ( - f"hash(File('memory://ns/x.bin')) should equal hash(('memory', '/ns/x.bin')), " + f"hash(File('memory://ns/x.bin')) should equal hash(UPath('memory://ns/x.bin')), " f"got {actual} vs {expected}" ) diff --git a/tests/test_hashing/test_file_hashers.py b/tests/test_hashing/test_file_hashers.py index ff1e2881..3e5a8f7e 100644 --- a/tests/test_hashing/test_file_hashers.py +++ b/tests/test_hashing/test_file_hashers.py @@ -286,13 +286,20 @@ def test_cache_key_preserves_url_form(self): cacher = InMemoryHashCacher() cached = CachedFileHasher(file_hasher=inner, cacher=cacher) - cached.hash_file(UPath("memory://ns/cache_key_test.bin")) - - keys = list(cacher._cache.keys()) - assert len(keys) == 1, f"Expected 1 cache entry, got {len(keys)}" - key = keys[0] - assert str(key.path) == "memory://ns/cache_key_test.bin", ( - f"Cache key path must preserve URL form; got {str(key.path)!r}" + url_path = UPath("memory://ns/cache_key_test.bin") + cached.hash_file(url_path) + + # Reconstruct the expected FileHashKey the same way CachedFileHasher does, + # then verify a cache hit via the public get() API. + resolved = url_path.resolve() + stat = resolved.stat() + expected_key = FileHashKey(resolved, stat.st_mtime_ns, stat.st_size) + assert cacher.get(expected_key) is not None, ( + "Expected a cache hit for the URL-form path key" + ) + # The resolved path must still be in URL form — not a concrete backend path. + assert str(expected_key.path) == "memory://ns/cache_key_test.bin", ( + f"Cache key path must preserve URL form; got {str(expected_key.path)!r}" ) finally: fs.rm("/ns/cache_key_test.bin")