feat(hashing): add PostgresHashCacher with psycopg3 backend (ITL-520) by kurodo3[bot] · Pull Request #223 · nauticalab/orcapod-python

kurodo3 · 2026-07-10T08:06:21Z

Summary

Add PostgresHashCacher — a PostgreSQL-backed CacherProtocol[FileHashKey, ContentHash] using thread-local psycopg3 connections and INSERT ... ON CONFLICT DO NOTHING for concurrency safety
Extend enable_file_hash_caching() with a conninfo keyword argument to activate the Postgres backend; mutually exclusive with db_path
Export PostgresHashCacher from orcapod.hashing under a try/except ImportError guard so users without psycopg pay nothing
Add full test suite in tests/test_hashing/test_postgres_hash_cacher.py using testcontainers[postgres] against a real Postgres 16 container
Document SQLite vs Postgres backend tradeoffs in docs/concepts/file-hash-caching.md

Closes ITL-520

Test plan

All non-Postgres tests pass (uv run pytest tests/ --ignore=tests/test_hashing/test_postgres_hash_cacher.py)
Postgres tests pass in CI where Docker is available (uv run pytest tests/test_hashing/test_postgres_hash_cacher.py -m postgres)
from orcapod.hashing import PostgresHashCacher returns None when psycopg not installed (ImportError guard)
enable_file_hash_caching(conninfo=..., db_path=...) raises ValueError (mutually exclusive)

🤖 Generated with Claude Code

…ections (ITL-520) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…Postgres backend (ITL-520) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… (ITL-520)

codecov · 2026-07-10T08:08:02Z

Codecov Report

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

Files with missing lines	Patch %	Lines
src/orcapod/hashing/migrate_hash_cache.py	97.05%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot

Pull request overview

This PR adds a PostgreSQL-backed file-hash caching backend (PostgresHashCacher) to complement the existing SQLite-based cacher, wiring it into enable_file_hash_caching(conninfo=...) and documenting backend tradeoffs. It also introduces a real-Postgres integration test suite using testcontainers.

Changes:

Add PostgresHashCacher (psycopg3-backed) and export it from orcapod.hashing.
Extend enable_file_hash_caching() with conninfo to activate the Postgres backend (and validate exclusivity with db_path).
Add Postgres integration tests + dev dependency and document SQLite vs Postgres usage.

Reviewed changes

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

Show a summary per file

File	Description
uv.lock	Adds `testcontainers[postgres]` to the dev lock set.
pyproject.toml	Adds `testcontainers[postgres]` to dev dependencies.
src/orcapod/hashing/postgres_hash_cacher.py	New Postgres-backed cacher implementation using psycopg3.
src/orcapod/hashing/init.py	Exports `PostgresHashCacher` under a guard.
src/orcapod/contexts/init.py	Adds `conninfo` path to `enable_file_hash_caching()` and mutual-exclusion check.
tests/test_hashing/test_postgres_hash_cacher.py	New testcontainers-based integration tests for the Postgres cacher and integration wiring.
tests/test_hashing/test_hash_cachers.py	Adds a regression test for mutually-exclusive `conninfo`/`db_path`.
docs/concepts/file-hash-caching.md	Documents how to choose/configure SQLite vs Postgres backend.
superpowers/specs/2026-07-10-itl-520-postgres-hash-cacher-design.md	Adds design/spec document for ITL-520.
superpowers/plans/2026-07-10-itl-520-postgres-hash-cacher.md	Adds implementation plan document for ITL-520.

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

kurodo3 · 2026-07-10T08:47:04Z

+try:
+    from orcapod.hashing.postgres_hash_cacher import PostgresHashCacher
+except ImportError:
+    PostgresHashCacher = None  # type: ignore[assignment,misc]


Fixed. The psycopg import guard has been moved to module level in postgres_hash_cacher.py:

try: import psycopg except ImportError as _exc: # pragma: no cover raise ImportError( "PostgresHashCacher requires psycopg. " "Install it with: pip install 'orcapod[postgresql]'" ) from _exc

This means importing the module itself now fails when psycopg is absent, so the try/except ImportError guard in hashing/__init__.py fires correctly and sets PostgresHashCacher = None. The redundant per-__init__ guard and all method-local import psycopg calls have been removed.

kurodo3 · 2026-07-10T08:47:14Z

 def enable_file_hash_caching(
    db_path: "Path | None" = None,
+    *,
+    conninfo: str | None = None,
    read_only: bool = False,
    min_cache_size_bytes: int | None = None,
 ) -> None:


Agreed that the signature needed rethinking. Rather than restoring backward compat, we went with the cleanest API: all four parameters are now keyword-only (including db_path):

def enable_file_hash_caching( *, db_path: "Path | None" = None, conninfo: str | None = None, read_only: bool = False, min_cache_size_bytes: int | None = None, ) -> None:

Since db_path and conninfo are mutually exclusive backends, having one positional and one keyword-only was asymmetric. All-keyword makes the interface unambiguous and self-documenting at call sites.

kurodo3 · 2026-07-10T08:47:21Z

+        db_path: Path to the SQLite cache database. Ignored when ``conninfo``
+            is provided. Defaults to ``~/.orcapod/file_hash_cache.db`` or
+            the ``ORCAPOD_HASH_CACHE_DB`` environment variable.


Fixed. The db_path doc now correctly describes the mutual-exclusion semantics:

db_path: Path to the SQLite cache database. Mutually exclusive with conninfo; providing both raises ValueError. Defaults to ~/.orcapod/file_hash_cache.db or the ORCAPOD_HASH_CACHE_DB environment variable.

The function-level docstring was also updated to lead with the backend-selection logic so the exclusivity is clear up front.

kurodo3 · 2026-07-10T08:47:29Z

+        return (
+            f"PostgresHashCacher("
+            f"conninfo={self._conninfo!r}, "
+            f"read_only={self._read_only!r}, "
+            f"min_cache_size_bytes={self._min_cache_size_bytes!r})"
+        )


Fixed. Added a _redact_conninfo() helper that strips the password from both conninfo forms before it reaches __repr__:

URL form: postgresql://alice:s3cr3t@host/db → postgresql://alice:***@host/db

Keyword DSN form: host=h dbname=d password=s3cr3t → host=h dbname=d password=***

The helper uses capture-group regex (Python's re doesn't support variable-width lookbehinds). Three unit tests cover both forms and the no-password case.

…cks (ITL-520) Both except-ImportError branches only execute when psycopg is not installed, a scenario that cannot be tested without sys.modules manipulation (which is prohibited by project conventions). Mark them with pragma: no cover so Codecov does not report them as missing lines.

…TL-520) Review fixes: - Move psycopg import guard to module level in postgres_hash_cacher.py so `from orcapod.hashing import PostgresHashCacher` correctly returns None when psycopg is not installed (Comment 2) - Redact password from conninfo in PostgresHashCacher.__repr__ via _redact_conninfo() helper using capture-group regex for both URL and keyword DSN forms (Comment 4) - Make all enable_file_hash_caching() params keyword-only (db_path, conninfo, read_only, min_cache_size_bytes) for a symmetric, unambiguous API (Comments 1, 3) - Fix db_path docstring: "Mutually exclusive with conninfo" not "Ignored" (Comment 1) Schema versioning: - SQLite: PRAGMA user_version tracks schema version (V0 = legacy, V1 = current with cached_at); SqliteHashCacher._ensure_schema() raises ValueError on old schema with migration command in the message - Postgres: file_hash_cache_meta table stores ('schema_version', '1'); _ensure_schema() checks information_schema.columns for cached_at and raises ValueError with migration DDL if the column is missing Migration script: - New src/orcapod/hashing/migrate_hash_cache.py: migrate_sqlite_hash_cache() adds cached_at column and stamps user_version=1; runnable as python -m orcapod.hashing.migrate_hash_cache <db_path> Tests: - TestSqliteSchemaVersion: fresh DB stamped V1, V0 raises with migration hint - TestMigrateHashCache: V0→V1 migration, idempotency, error cases, row preservation - TestPostgresSchemaVersion: meta table content, old-schema error (Postgres-marked) - TestPostgresReprRedaction: URL and keyword-form password redaction (no Docker)

kurodo3 · 2026-07-10T08:47:50Z

Review round 1 — changes summary

All four Copilot comments addressed plus schema versioning added. Single commit: 5637a661.

Copilot review fixes

Comment	Fix
Docstring: `db_path` said "Ignored when conninfo provided" but raises `ValueError`	Updated to "Mutually exclusive with `conninfo`; providing both raises `ValueError`". Function docstring also leads with the backend-selection logic now.
ImportError guard ineffective: psycopg not imported at module level so the `__init__.py` guard never fired	Moved `import psycopg` to module level in `postgres_hash_cacher.py` (under `try/except`). Importing the module now fails when psycopg is absent, so `PostgresHashCacher = None` in `hashing/__init__.py` works correctly. Removed the redundant per-method imports.
Breaking API change: `*` before `conninfo` made `read_only`/`min_cache_size_bytes` keyword-only	Made all four parameters keyword-only (`*, db_path, conninfo, read_only, min_cache_size_bytes`). Since `db_path` and `conninfo` are mutually exclusive backends, symmetry and clarity win over positional convention.
`__repr__` leaks credentials	Added `_redact_conninfo()` helper using capture-group regex. URL-form passwords (`://user:pass@`) and keyword-form (`password=val`) are both replaced with `***`. Three unit tests cover URL, keyword, and no-password cases.

Schema versioning (new)

SQLite — PRAGMA user_version (built-in SQLite integer, no extra tables):

Fresh databases are stamped user_version = 1 on first open.
Databases at user_version = 0 that are missing the cached_at column raise ValueError with the migration command.

PostgreSQL — companion file_hash_cache_meta(key TEXT PK, value TEXT) table:

Row ('schema_version', '1') inserted ON CONFLICT DO NOTHING on first open.
If file_hash_cache exists but lacks cached_at, raises ValueError with the migration DDL.

Migration script — src/orcapod/hashing/migrate_hash_cache.py:

migrate_sqlite_hash_cache(db_path) — adds cached_at INTEGER DEFAULT 0 and stamps user_version = 1.
Idempotent; prints status messages.
Runnable as python -m orcapod.hashing.migrate_hash_cache <db_path>.

9 new tests — schema version detection (fresh DB stamped, V0 raises with hint, V1 reopens cleanly), migration (V0→V1, idempotency, error cases, row preservation), Postgres schema version in meta table, old-schema detection, repr redaction.

All 4328 non-Postgres tests pass (4319 before → +9 new).

…onents, and function_info_extractors

Copilot

Pull request overview

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

kurodo3 · 2026-07-10T23:50:21Z

+        print(f"{db_path}: migrating from schema V0 → V1 …")
+        conn.execute(
+            "ALTER TABLE file_hash_cache "
+            "ADD COLUMN cached_at INTEGER NOT NULL DEFAULT 0"
+        )
+        conn.execute("PRAGMA user_version = 1")
+        conn.commit()


Fixed in commit 6391ef43. SQLite's ALTER TABLE ADD COLUMN only accepts literal constants as DEFAULT — function calls like strftime('%s','now') are disallowed — so the migration had to use DEFAULT 0. The real fix is in SqliteHashCacher.put(), which now explicitly supplies cached_at on every INSERT:

INSERT OR REPLACE INTO file_hash_cache (path, mtime_ns, size, hash, cached_at) VALUES (?, ?, ?, ?, strftime('%s', 'now'))

Both freshly created and migrated databases now produce real timestamps on new rows. Pre-migration rows keep cached_at = 0 (meaning 'timestamp unknown'). A new regression test (test_put_after_migration_sets_nonzero_cached_at) verifies this end-to-end.

- postgres_hash_cacher: change schema_version insert to ON CONFLICT DO UPDATE so the meta table always reflects the running code's _SCHEMA_VERSION on every open, making future migration logic reliable - hash_cachers: explicitly set cached_at in SqliteHashCacher.put() using strftime('%s','now') so migrated databases get real timestamps on new rows (ALTER TABLE ADD COLUMN only supports literal defaults, not the strftime expression used by CREATE TABLE) - test: add test_put_after_migration_sets_nonzero_cached_at to verify the fix Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

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

kurodo3 · 2026-07-10T23:50:32Z

+            conn.execute("DROP TABLE IF EXISTS file_hash_cache CASCADE")
+
+
+class TestPostgresReprRedaction:


Keeping the current setup intentionally. TestPostgresReprRedaction tests only _redact_conninfo() — a pure utility with zero Postgres dependency. Without the marker it runs in every default pytest invocation, giving broader coverage than if it were gated behind --postgres. Adding @pytest.mark.postgres would restrict these tests to the Postgres-only CI run and exclude them from all other runs, which would be a coverage regression. The module docstring has been expanded (commit b5b25d04) to explain this distinction explicitly.

kurodo3 · 2026-07-10T23:50:38Z

+            row = conn.execute(
+                """
+                SELECT column_name
+                FROM information_schema.columns
+                WHERE table_name = 'file_hash_cache'
+                  AND column_name = 'cached_at'
+                """
+            ).fetchone()


Fixed in commit 2ef095e5. Added AND table_schema = ANY(current_schemas(false)) to scope the query to tables visible through the connection's search_path, preventing a same-named table in a different schema from falsely clearing the old-schema error.

kurodo3 · 2026-07-10T23:50:46Z

+            version = conn.execute("PRAGMA user_version").fetchone()[0]
+            if version < _SQLITE_SCHEMA_VERSION:
+                # Inspect actual columns in case the table pre-dates versioning.
+                columns = {
+                    row[1]
+                    for row in conn.execute("PRAGMA table_info(file_hash_cache)")
+                }
+                if "cached_at" not in columns:
+                    raise ValueError(
+                        f"SQLite hash cache at '{self.db_path}' uses an outdated "
+                        f"schema (version {version}, missing 'cached_at' column). "
+                        f"Run the migration script to upgrade:\n\n"
+                        f"    python -m orcapod.hashing.migrate_hash_cache "
+                        f"{self.db_path}\n"
+                    )
+                # Table already has cached_at but version was never stamped
+                # (created by code before versioning was introduced).  Stamp now.
+                conn.execute(f"PRAGMA user_version = {_SQLITE_SCHEMA_VERSION}")
+


Fixed in commit 2ef095e5. The PRAGMA table_info column check is now unconditional — it always runs regardless of user_version. A DB with user_version = 1 but no cached_at column (e.g. manually corrupted) now raises a clear ValueError with the migration hint at open time instead of failing cryptically in put(). New test test_manually_bumped_version_with_missing_column_raises covers this path.

- postgres_hash_cacher: add table_schema = ANY(current_schemas(false)) constraint to information_schema.columns query so the cached_at check is scoped to the connection's search_path and not confused by a same-named table in a different schema - hash_cachers: always validate cached_at column presence in _ensure_schema() regardless of user_version, catching manually- corrupted databases (bumped user_version without adding the column) before a cryptic INSERT failure in put() - test: add test_manually_bumped_version_with_missing_column_raises to cover the new unconditional column check - docs: update enable_file_hash_caching() signature in design spec and implementation plan to reflect all-keyword-only API (*, db_path=...) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

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

kurodo3 · 2026-07-10T23:51:08Z

+        """When eval_str=True fails, falls back to unresolved signature."""
+        # Create a function with an annotation that cannot be resolved at eval time
+        # by using exec with a forward reference that doesn't exist
+        import types, sys


Fixed in commit b5b25d04. Removed the import types, sys line entirely — neither was referenced in the function body.

kurodo3 · 2026-07-10T23:51:13Z

@@ -0,0 +1,452 @@
+"""Tests for PostgresHashCacher using a real Postgres via testcontainers."""


Fixed in commit b5b25d04. Expanded the module docstring to describe the --postgres flag, the Docker/testcontainers auto-setup, and to explicitly note that TestPostgresReprRedaction intentionally carries no postgres marker (it tests a pure utility and should run in all CI contexts).

…round 4) - test_function_info_extractors: remove unused top-level `inspect` import and unused inline `types, sys` import in test_eval_str_fallback - test_hash_utils: drop unused `types` from `import types, textwrap`; rename `has_comment` to `_` (assigned but never referenced) - test_hash_cachers: remove string quotes from `path: "Path"` so the import is used directly in the annotation - test_postgres_hash_cacher: expand module docstring to describe the --postgres flag and Docker/testcontainers requirement, and note that TestPostgresReprRedaction runs unconditionally (no postgres marker) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

kurodo3 · 2026-07-10T23:52:18Z

Review response — rounds 2–4 summary

All outstanding review comments addressed across three commits. Thread-level replies have been posted to each inline comment.

Round 2 — commit `6391ef43`

Comment	Action	Detail
Postgres `ON CONFLICT DO NOTHING` for schema_version	Fixed	Changed to `DO UPDATE SET value = EXCLUDED.value` so the meta table always reflects the running code's `_SCHEMA_VERSION` on every open
`DEFAULT 0` in migration vs `DEFAULT (strftime)`	Fixed	`SqliteHashCacher.put()` now explicitly supplies `cached_at = strftime('%s','now')` in the INSERT; migrated and fresh DBs both get real timestamps. New regression test added.

Round 3 — commit `2ef095e5`

Comment	Action	Detail
`TestPostgresReprRedaction` not marked `@pytest.mark.postgres`	Declined	These tests cover a pure utility (`_redact_conninfo`). Without the marker they run in all CI contexts — adding it would restrict them to the postgres-only run and reduce coverage. Module docstring updated to explain this.
`information_schema.columns` missing `table_schema` constraint	Fixed	Added `AND table_schema = ANY(current_schemas(false))` to scope the check to the connection's search_path
Column check gated behind `user_version`	Fixed	`cached_at` presence is now always validated unconditionally; a manually-bumped `user_version` without the column now fails fast with a clear error. New test added.
Implementation plan shows old positional signature	Fixed	Updated `superpowers/plans/` snippet to `*, db_path=None, ...`
Design spec shows old positional signature	Fixed	Updated `superpowers/specs/` snippet to `*, db_path=None, ...`

Round 4 — commit `b5b25d04`

Comment	Action	Detail
`inspect` unused in `test_function_info_extractors.py`	Fixed	Removed
`types`, `sys` unused inline imports in same file	Fixed	Removed both
`types` unused in `test_hash_utils.py`	Fixed	Trimmed to `import textwrap`
`has_comment` assigned but never used	Fixed	Changed to `_ = ns["has_comment"]`
`Path` only in string annotation in `test_hash_cachers.py`	Fixed	Removed quotes: `path: "Path"` → `path: Path`
Module docstring missing run instructions	Fixed	Expanded to document `--postgres` flag, Docker requirement, and intent of un-marked redaction tests

kurodo3 Bot and others added 7 commits July 10, 2026 05:12

docs(specs): add ITL-520 PostgresHashCacher design spec

d4c0e16

docs(plans): add ITL-520 PostgresHashCacher implementation plan

0050aa0

chore(deps): add testcontainers[postgres] to dev deps (ITL-520)

fd8cb13

feat(hashing): add PostgresHashCacher with thread-local psycopg3 conn…

7fd5112

…ections (ITL-520) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(hashing): export PostgresHashCacher from orcapod.hashing (ITL-520)

b7fecb8

feat(contexts): add conninfo param to enable_file_hash_caching() for …

cae234a

…Postgres backend (ITL-520) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(hashing): add Postgres backend section to file-hash-caching docs…

25bc874

… (ITL-520)

eywalker requested a review from Copilot July 10, 2026 08:20

Copilot started reviewing on behalf of eywalker July 10, 2026 08:20 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

kurodo3 Bot added 2 commits July 10, 2026 08:26

test(hashing): add coverage for migrate_hash_cache, get_function_comp…

43f986e

…onents, and function_info_extractors

eywalker requested a review from Copilot July 10, 2026 16:50

Copilot started reviewing on behalf of eywalker July 10, 2026 16:51 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

eywalker requested a review from Copilot July 10, 2026 22:05

Copilot started reviewing on behalf of eywalker July 10, 2026 22:06 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

eywalker requested a review from Copilot July 10, 2026 23:11

Copilot started reviewing on behalf of eywalker July 10, 2026 23:12 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

eywalker approved these changes Jul 11, 2026

View reviewed changes

eywalker merged commit b02e060 into main Jul 11, 2026
11 checks passed

eywalker deleted the eywalker/itl-520-filehasher-add-postgresql-backend-for-the-hash-cacher branch July 11, 2026 01:13

		conn.execute("DROP TABLE IF EXISTS file_hash_cache CASCADE")


		class TestPostgresReprRedaction:

		@@ -0,0 +1,452 @@
		"""Tests for PostgresHashCacher using a real Postgres via testcontainers."""

Uh oh!

Conversation

kurodo3 Bot commented Jul 10, 2026

Summary

Test plan

Uh oh!

codecov Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

kurodo3 Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

kurodo3 Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

kurodo3 Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

kurodo3 Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

kurodo3 Bot commented Jul 10, 2026

Review round 1 — changes summary

Copilot review fixes

Schema versioning (new)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

kurodo3 Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

kurodo3 Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

kurodo3 Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

kurodo3 Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

kurodo3 Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kurodo3 Bot Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

kurodo3 Bot commented Jul 10, 2026

Review response — rounds 2–4 summary

Round 2 — commit 6391ef43

Round 3 — commit 2ef095e5

Round 4 — commit b5b25d04

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

codecov Bot commented Jul 10, 2026 •

edited

Loading

Round 2 — commit `6391ef43`

Round 3 — commit `2ef095e5`

Round 4 — commit `b5b25d04`