Skip to content

feat(hashing): add file hash caching with SQLite backend (ITL-472)#204

Merged
eywalker merged 12 commits into
mainfrom
eywalker/itl-472-add-file-hash-caching-to-filehasher-sqlite3-v1-turso
Jul 2, 2026
Merged

feat(hashing): add file hash caching with SQLite backend (ITL-472)#204
eywalker merged 12 commits into
mainfrom
eywalker/itl-472-add-file-hash-caching-to-filehasher-sqlite3-v1-turso

Conversation

@kurodo3

@kurodo3 kurodo3 Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds CacherProtocol[K, V] — a generic two-method (get/put) protocol for typed caches
  • Renames BasicFileHasherFileHasher; adds FileHashKey (frozen dataclass with path: UPath, mtime_ns, size) as the typed cache lookup key
  • Adds CachedFileHasher — a decorator that wraps any FileContentHasherProtocol with a CacherProtocol[FileHashKey, ContentHash], reducing repeat hashing of the same file to a sub-millisecond DB lookup
  • Adds InMemoryHashCacher (dict-backed, for tests) and SqliteHashCacher (WAL mode, WITHOUT ROWID, thread-local connections, BLOB storage via ContentHash.to_prefixed_digest())
  • Adds enable_file_hash_caching(db_path=None) to orcapod.contexts — one call at startup wires CachedFileHasher + SqliteHashCacher into the default context; warns and unwraps on double-call
  • Benchmark shows ~2600× speedup on cache hits (0.1 ms vs 254 ms for a 100 MB file)

Closes ITL-472

Test Plan

  • uv run pytest tests/test_hashing/test_hash_cachers.py — all InMemoryHashCacher, SqliteHashCacher, and enable_file_hash_caching() tests pass
  • uv run pytest tests/test_hashing/test_file_hashers.py tests/test_hashing/test_file_handler.py — all file hasher tests pass with renamed FileHasher
  • uv run pytest tests/ -q — full suite: 3992 passed, 56 skipped, 6 xfailed
  • uv run python bench/bench_file_hasher_cache.py — sub-millisecond cache hit confirmed

🤖 Generated with Claude Code

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.01961% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/orcapod/contexts/__init__.py 95.23% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an opt-in, persistent file-hash cache layer to Orcapod’s file hashing pipeline to dramatically reduce repeated hashing costs by memoizing ContentHash results keyed by (resolved path, mtime_ns, size).

Changes:

  • Introduces a generic CacherProtocol[K, V] and file-specific cache key FileHashKey.
  • Renames BasicFileHasherFileHasher, and adds CachedFileHasher that composes a FileContentHasherProtocol with a typed cache backend.
  • Implements InMemoryHashCacher (tests) and SqliteHashCacher (persistent WAL/WITHOUT ROWID), plus enable_file_hash_caching() to wire caching into the default context; adds tests and a benchmark script.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/test_hashing/test_hash_cachers.py Adds coverage for in-memory and SQLite cache backends plus enable_file_hash_caching() wiring behavior.
tests/test_hashing/test_file_hashers.py Updates tests for FileHasher rename and typed-cache-based CachedFileHasher.
tests/test_hashing/test_file_handler.py Updates handler tests to use FileHasher instead of BasicFileHasher.
superpowers/specs/2026-07-02-itl-472-file-hash-caching-design.md Design spec documenting the caching architecture, schema, and constraints.
superpowers/plans/2026-07-02-itl-472-file-hash-caching.md Implementation plan and checklist for delivering ITL-472.
src/orcapod/protocols/hashing_protocols.py Adds generic CacherProtocol[K, V] to formalize typed cache backends.
src/orcapod/hashing/semantic_hashing/builtin_handlers.py Updates fallback hasher to FileHasher(sha256) and related docstrings.
src/orcapod/hashing/hash_cachers.py Adds InMemoryHashCacher and SqliteHashCacher implementations.
src/orcapod/hashing/file_hashers.py Adds FileHashKey, renames BasicFileHasherFileHasher, and refactors CachedFileHasher to use typed caching.
src/orcapod/hashing/init.py Updates public hashing exports to include new caching-related symbols and removes BasicFileHasher.
src/orcapod/contexts/data/v0.1.json Updates default context config to reference FileHasher.
src/orcapod/contexts/init.py Adds enable_file_hash_caching() to wrap the default FileHandler with a SQLite-backed cache layer.
bench/bench_file_hasher_cache.py Adds a benchmark script to measure cached vs uncached file hashing performance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/orcapod/contexts/__init__.py Outdated
``~/.orcapod/file_hash_cache.db`` or the
``ORCAPOD_HASH_CACHE_DB`` environment variable.
"""
from pathlib import Path # noqa: F811

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — removed the from pathlib import Path import and the # noqa: F811 suppression from inside enable_file_hash_caching(). The annotation "Path | None" is already a quoted string so no runtime import is needed.

Comment on lines +283 to +284
while isinstance(base_hasher, CachedFileHasher):
base_hasher = base_hasher.file_hasher

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — the unwrap loop now calls cacher.close() on each discarded CachedFileHasher layer if the cacher exposes a close() method:

while isinstance(base_hasher, CachedFileHasher):
    cacher = base_hasher.cacher
    if hasattr(cacher, "close"):
        cacher.close()
    base_hasher = base_hasher.file_hasher

This closes any thread-local SQLite connections held by SqliteHashCacher instances before they are discarded, preventing file descriptor leaks.

Comment thread src/orcapod/hashing/hash_cachers.py Outdated
Comment thread tests/test_hashing/test_hash_cachers.py
@kurodo3

kurodo3 Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Review round 1 — all comments addressed

All 4 Copilot review comments resolved in commit 98f3554.

Comment Fix
leaks cacher file descriptors on double-call Unwrap loop now calls on each discarded layer if the cacher exposes a method. Prevents SQLite connection leaks.
Unused in Removed.
Unused / in Removed both and .
Unused (+ ) in Removed. The annotation is a quoted string — no runtime import needed.

432 hashing tests still pass after these changes.

@kurodo3

kurodo3 Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Review round 1 — all 4 comments addressed

All changes in commit 98f3554b.

Comment Fix
enable_file_hash_caching() leaks cacher connections on double-call Unwrap loop now calls cacher.close() on each discarded layer when the cacher exposes a close() method, preventing SQLite file descriptor leaks.
Unused import threading in test_hash_cachers.py Removed.
Unused logging/logger in hash_cachers.py Removed both import logging and logger = logging.getLogger(__name__).
Unused from pathlib import Path (+ # noqa: F811) in enable_file_hash_caching() Removed. The `"Path

All 432 hashing tests pass after these changes.

Comment thread src/orcapod/contexts/__init__.py Outdated
from orcapod.protocols import hashing_protocols as hp
from orcapod.protocols import semantic_types_protocols as sp

_logger = _logging.getLogger(__name__)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why set it as _logger?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — no reason for the underscore prefix. Renamed to plain logging / logger = logging.getLogger(__name__) to match the convention used everywhere else in the codebase (e.g. builtin_handlers.py).

@kurodo3

kurodo3 Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Review round 2 — addressed

Single commit 35e08a01.

Comment Fix
Why _logger / import logging as _logging? Renamed to the standard import logging / logger = logging.getLogger(__name__), consistent with how every other module in the codebase declares its logger (e.g. builtin_handlers.py). The underscore aliases served no purpose here.

kurodo3 Bot and others added 11 commits July 2, 2026 05:08
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… and CachedFileHasher

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Key; update CachedFileHasher to CacherProtocol
- Close existing cachers (if closeable) when unwrapping in enable_file_hash_caching()
- Remove unused 'threading' import from test_hash_cachers.py
- Remove unused 'logging'/'logger' from hash_cachers.py
- Remove unused 'from pathlib import Path' and noqa comment from enable_file_hash_caching()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kurodo3 kurodo3 Bot force-pushed the eywalker/itl-472-add-file-hash-caching-to-filehasher-sqlite3-v1-turso branch from 35e08a0 to 31b98de Compare July 2, 2026 05:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comment thread src/orcapod/hashing/hash_cachers.py Outdated
@kurodo3

kurodo3 Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Review round 3 — addressed

Single commit 8a11b7f4.

Comment Fix
unixepoch() not available on older SQLite builds Replaced with strftime('%s', 'now') in the cached_at column default — consistent with the same portable pattern already used in string_cachers.py:315.

All 18 hash cacher tests pass after this change.

@eywalker eywalker merged commit 868262f into main Jul 2, 2026
11 checks passed
@eywalker eywalker deleted the eywalker/itl-472-add-file-hash-caching-to-filehasher-sqlite3-v1-turso branch July 2, 2026 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants