diff --git a/alembic/versions/0098_phash_256_bit_rehash.py b/alembic/versions/0098_phash_256_bit_rehash.py new file mode 100644 index 0000000..569547b --- /dev/null +++ b/alembic/versions/0098_phash_256_bit_rehash.py @@ -0,0 +1,90 @@ +"""Widen image_record.phash to 256-bit and re-hash the library (issue #4223). + +The operator reported a 15-image variant pack landing as 3 records, and then +that variants were STILL being dropped with `phash_threshold` at 0. Zero was +already the floor of the dial, so no setting could have fixed it: at +`hash_size=8` a pHash is 64 bits of coarse light/dark layout, and variant +artwork sharing a composition produces the SAME 64 bits. Distance 0 meant +"identical hash", never "identical image". + +`utils/phash.py` moves to `hash_size=16` (256 bits, what ImageRepo always +used) and adds an aspect-ratio gate plus a pixel-level confirm, so a merge is +accepted on the files rather than on the hash. + +## Why this NULLs every phash + +Widening the column does not correct the values already in it. Every stored +hash is a 64-bit hash of an image the app will now hash at 256 bits, and the +two cannot be compared — `find_similar` skips a mismatched-length candidate +rather than guessing, so leaving them would silently mean "no dedup, forever, +for everything imported before today". NULL is the state `backfill_phash` +already knows how to repair: it is NULL-only, keyset-paginated and +restart-safe, and the beat schedule runs it daily. + +Until that backfill finishes, image dedup degrades to sha256 only — +duplicates may be kept. That is the safe direction, and the only one +available: the alternative is comparing hashes of different widths, which +would drop artwork. NOTHING here deletes or supersedes a file. + +## Why the threshold is reset rather than carried over + +`phash_threshold` counts bits, and the denominator went from 64 to 256. The +stored number would keep its value while meaning something four times +tighter. There is no honest carry-over, so every row goes to the new default +of 24 — including the operator's 0, which was a workaround for the bug this +revision fixes. + +Revision ID: 0098 +Revises: 0097 +Create Date: 2026-09-21 + +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "0098" +down_revision: Union[str, None] = "0097" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # varchar(32) -> varchar(64): widening a length limit is a catalog-only + # change in Postgres, so this does not rewrite the table or its index. + op.alter_column( + "image_record", "phash", + existing_type=sa.String(32), + type_=sa.String(64), + existing_nullable=True, + ) + op.execute("UPDATE image_record SET phash = NULL WHERE phash IS NOT NULL") + op.alter_column( + "import_settings", "phash_threshold", + existing_type=sa.Integer(), + server_default="24", + existing_nullable=False, + ) + op.execute("UPDATE import_settings SET phash_threshold = 24") + + +def downgrade() -> None: + # The 64-bit hashes this replaced are gone, and a 64-char value does not + # fit back into varchar(32) — so the column is cleared again on the way + # down and left for backfill_phash to refill at whatever HASH_SIZE the + # code is running. Rule #22: no legacy to preserve. + op.execute("UPDATE image_record SET phash = NULL WHERE phash IS NOT NULL") + op.alter_column( + "image_record", "phash", + existing_type=sa.String(64), + type_=sa.String(32), + existing_nullable=True, + ) + op.alter_column( + "import_settings", "phash_threshold", + existing_type=sa.Integer(), + server_default="10", + existing_nullable=False, + ) + op.execute("UPDATE import_settings SET phash_threshold = 10") diff --git a/backend/app/celery_app.py b/backend/app/celery_app.py index b702fc6..d49c8ef 100644 --- a/backend/app/celery_app.py +++ b/backend/app/celery_app.py @@ -120,6 +120,14 @@ def make_celery() -> Celery: "schedule": 86400.0, # daily — sweep .part/.partial left by a # download/import killed mid-write (graceful-shutdown fallout) }, + "backfill-phash-daily": { + "task": "backend.app.tasks.maintenance.backfill_phash", + "schedule": 86400.0, # daily — NULL-only, so a no-op once the + # library is hashed. This is what makes migration 0098's + # re-hash happen on its own: 0098 NULLs every phash, and + # without a scheduled refill the library would sit + # dedup-disabled until someone ran a deep scan (#4223). + }, "train-heads-nightly": { "task": "backend.app.tasks.ml.scheduled_train_heads", "schedule": 86400.0, # passive cadence; manual retrain stays available diff --git a/backend/app/models/image_record.py b/backend/app/models/image_record.py index f5f4050..223bb8d 100644 --- a/backend/app/models/image_record.py +++ b/backend/app/models/image_record.py @@ -64,7 +64,11 @@ class ImageRecord(Base): # that 0001 also built was an exact duplicate of it — dropped in 0089 # (#3301). Lookups by sha256 use the constraint's index. sha256: Mapped[str] = mapped_column(String(64), nullable=False) - phash: Mapped[str | None] = mapped_column(String(32), nullable=True, index=True) + # 64 hex chars = the 256-bit hash utils.phash emits at hash_size=16. Was + # String(32) (64-bit) until migration 0098; the narrow column was the + # reason for the undersized hash, and the undersized hash was collapsing + # variant artwork into one record (#4223). + phash: Mapped[str | None] = mapped_column(String(64), nullable=True, index=True) size_bytes: Mapped[int] = mapped_column(BigInteger, nullable=False) mime: Mapped[str] = mapped_column(String(64), nullable=False) width: Mapped[int | None] = mapped_column(Integer, nullable=True) diff --git a/backend/app/models/import_settings.py b/backend/app/models/import_settings.py index 0ed2e8b..3227724 100644 --- a/backend/app/models/import_settings.py +++ b/backend/app/models/import_settings.py @@ -42,7 +42,12 @@ class ImportSettings(Base): single_color_threshold: Mapped[float] = mapped_column(Float, nullable=False, default=0.95, server_default="0.95") single_color_tolerance: Mapped[int] = mapped_column(Integer, nullable=False, default=30, server_default="30") - phash_threshold: Mapped[int] = mapped_column(Integer, nullable=False, default=10, server_default="10") + # Hamming distance over a 256-bit pHash (utils.phash, hash_size=16). The + # unit CHANGED in migration 0098 — it used to be bits out of 64 — so the + # old default of 10 is not this scale's 10, and 0098 resets every row. + # This is now the cheap PRE-FILTER: the aspect + pixel gates decide, which + # is what lets it be generous enough to catch a re-encoded rescale. + phash_threshold: Mapped[int] = mapped_column(Integer, nullable=False, default=24, server_default="24") # FC-3c downloader knobs download_rate_limit_seconds: Mapped[float] = mapped_column( diff --git a/backend/app/services/gallery_service.py b/backend/app/services/gallery_service.py index 3f124db..87224ad 100644 --- a/backend/app/services/gallery_service.py +++ b/backend/app/services/gallery_service.py @@ -322,7 +322,7 @@ def _gallery_images(rows, artists: dict[int, dict]) -> list[GalleryImage]: ] -def _diversify_similar(src, rows, limit, *, dup_threshold=8, lam=0.40): +def _diversify_similar(src, rows, limit, *, dup_threshold=32, lam=0.40): """Trim a nearest-cosine candidate pool down to `limit` diverse picks. 1. pHash collapse: drop any candidate whose perceptual hash is within @@ -338,6 +338,11 @@ def _diversify_similar(src, rows, limit, *, dup_threshold=8, lam=0.40): 2026-07-01 — dropped 0.55→0.40, dup 6→8, paired with a wider pool in `similar()`). + `dup_threshold` counts Hamming bits, so it moved 8→32 when the pHash went + from 64 to 256 bits (#4223, migration 0098) — the same fraction of the + hash, i.e. the tuning the operator chose, unchanged. This collapse is + DISPLAY-only: it hides a near-dup from one rail, it never drops a record. + Falls back to nearest-order (`rows[:limit]`) on any failure or a small pool. """ if len(rows) <= 1: diff --git a/backend/app/services/importer.py b/backend/app/services/importer.py index db8cf0c..f4c0142 100644 --- a/backend/app/services/importer.py +++ b/backend/app/services/importer.py @@ -39,7 +39,7 @@ from ..utils.paths import ( hash_suffixed_name, safe_ext, ) -from ..utils.phash import compute_phash, find_similar +from ..utils.phash import compute_phash, find_similar, fingerprint_path, fingerprints_match from ..utils.sidecar import find_sidecar, parse_sidecar from ..utils.slug import slugify from .archive_extractor import extract_archive, is_archive @@ -234,6 +234,38 @@ class Importer: (phash, width or 0, height or 0, image_id) ) + def _pixel_confirmer(self, source: Path): + """Build `find_similar`'s gate-3 callback for an incoming file. + + pHash proposes; this accepts. A candidate is a duplicate only if its + file really is the same picture as `source` at a different size — + which is the only merge the operator asked for (#4223). Everything + else (a missing file, an unreadable one, a deleted row) returns + False: the destructive outcomes here are dropping a download and + overwriting a kept file, so an unanswerable question must not read + as "yes". + + Both sides' fingerprints are computed lazily and cached, so an + import that matches nothing costs no I/O at all and an archive + member that keeps hitting the same candidate pays for it once. + """ + new_fp: list = [] + cand_fps: dict[int, object] = {} + + def confirm(candidate_id: int) -> bool: + if not new_fp: + new_fp.append(fingerprint_path(source)) + if new_fp[0] is None: + return False + if candidate_id not in cand_fps: + rec = self.session.get(ImageRecord, candidate_id) + cand_fps[candidate_id] = ( + fingerprint_path(Path(rec.path)) if rec and rec.path else None + ) + return fingerprints_match(new_fp[0], cand_fps[candidate_id]) + + return confirm + def _get_or_create(self, stmt, factory): """Race-safe find-or-create. Run `stmt` (scalar_one_or_none); if a row exists, return it. Otherwise open a savepoint and INSERT @@ -862,6 +894,7 @@ class Importer: rel, match_id = find_similar( phash, width or 0, height or 0, candidates, self.settings.phash_threshold, + confirm=self._pixel_confirmer(source), ) if rel == "larger_exists": # Enrich-on-duplicate (parity with attach_in_place). @@ -1241,6 +1274,7 @@ class Importer: rel, match_id = find_similar( phash, width or 0, height or 0, candidates, self.settings.phash_threshold, + confirm=self._pixel_confirmer(path), ) if rel == "larger_exists": # Enrich-on-duplicate: link the near-dup's post to the diff --git a/backend/app/tasks/maintenance.py b/backend/app/tasks/maintenance.py index 9b8cd98..3a87924 100644 --- a/backend/app/tasks/maintenance.py +++ b/backend/app/tasks/maintenance.py @@ -502,10 +502,16 @@ def prune_task_runs() -> dict: soft_time_limit=1800, time_limit=2100, ) def backfill_phash() -> int: - """Recompute phash for stored images that have none (imported before - FC-2d-i+ii). Keyset-paginated by id (restart-safe), NULL-only fill, - idempotent. Videos legitimately keep phash NULL. A missing/unreadable - file is logged and left NULL — never fails the task.""" + """Recompute phash for stored images that have none. Keyset-paginated by + id (restart-safe), NULL-only fill, idempotent. Videos legitimately keep + phash NULL. A missing/unreadable file is logged and left NULL — never + fails the task. + + Two sources of NULLs: images imported before FC-2d-i+ii, and migration + 0098, which cleared every phash so the library could be re-hashed at + hash_size=16 (#4223). The daily beat entry exists for the second — until + a row is refilled it takes no part in dedup, which is why this runs on a + schedule rather than waiting for a deep scan.""" SessionLocal = _sync_session_factory() updated = 0 last_id = 0 diff --git a/backend/app/utils/phash.py b/backend/app/utils/phash.py index a610e96..f2d2ba0 100644 --- a/backend/app/utils/phash.py +++ b/backend/app/utils/phash.py @@ -1,57 +1,206 @@ """Perceptual-hash dedup helpers (ported from ImageRepo). -hash_size=8 -> 64-bit hash -> 16-hex-char string, which fits the existing -ImageRecord.phash String(32) column (no image_record migration). IR uses -hash_size=16; the deliberate FC deviation keeps the schema unchanged. The -Hamming threshold is the operator-exposed dial (ImportSettings). +hash_size=16 -> 256-bit hash -> 64-hex-char string, which is why +`ImageRecord.phash` is String(64) (widened in migration 0098). + +## Why 16, after running at 8 from FC-2d until 2026-09-21 + +`hash_size=8` keeps only the top-left 8x8 block of the DCT — 64 bits +describing an image's coarse light/dark layout and nothing else. Variant +artwork that shares a composition (same pose and framing, a different +outfit / expression / overlay) collided OUTRIGHT at that size, so the +operator's near-duplicate dial could not separate variants from rescales +even at its floor: `phash_threshold=0` means "the same 64 bits", not "the +same image", and packs of 15 variants were landing as 3 records +(issue #4223). IR always used 16; FC's deviation to 8 was made to fit the +old String(32) column without a migration — a schema convenience that cost +the operator artwork. + +## The hash no longer decides a merge on its own + +Dropping a file or superseding one is destructive, so `find_similar` now +runs three gates, cheapest first: + + 1. Hamming distance within the operator's `phash_threshold` — the cheap + indexed pre-filter that PROPOSES candidates. + 2. Aspect ratio within ASPECT_TOL. A crop or a re-canvas is not a + rescale, and this is the same identity test the tier-1 video near-dup + path already uses (`_VIDEO_DUP_ASPECT_TOL`). + 3. A pixel-level confirm on the candidate's actual file, supplied by the + caller (the importer opens the files; this module stays I/O-free apart + from `fingerprint_path`). + +Because the confirm is what ACCEPTS a match, the threshold can stay +generous enough to tolerate a re-encode without putting variants at risk. + +Every gate fails CLOSED: unknown dimensions, an unreadable candidate, a +hash that won't parse — all mean "not a duplicate". The two failure +directions are not symmetrical. Too strict keeps a redundant lower-res copy, +which the operator can see and delete; too loose deletes artwork that only +a re-walk of the source can bring back. """ import imagehash +from PIL import Image, ImageChops, ImageStat -HASH_SIZE = 8 +HASH_SIZE = 16 + +# Gate 2. Matching `importer._VIDEO_DUP_ASPECT_TOL` — a rescale preserves +# aspect ratio to within rounding, so this only has to absorb off-by-one +# pixel dimensions. +ASPECT_TOL = 0.02 + +# Gate 3. Both images are reduced to one FINGERPRINT_SIZE-square grayscale +# thumbnail and compared directly, which is the question actually being +# asked: "are these the same picture at different resolutions?" +# +# Two criteria, because they catch different things. MEAN absolute +# difference catches a global change (a recolour, a filter, a different +# shading pass) that leaves the composition intact. The CHANGED-PIXEL +# fraction catches a LOCAL one — an added overlay, a different expression, +# an alternate outfit on part of the figure — which a mean over 4096 pixels +# would otherwise dilute into noise. +# +# Tuned to fail closed (see the module docstring). A true rescale lands near +# zero on both; these ceilings sit well above that and well below a variant. +FINGERPRINT_SIZE = 64 +FINGERPRINT_MAX_MEAN_DIFF = 6.0 +FINGERPRINT_CHANGED_LEVEL = 64 +FINGERPRINT_MAX_CHANGED_FRACTION = 0.02 + + +def _seek_first_frame(pil_image) -> None: + """Animated images (multi-frame WebP/GIF/APNG) are hashed and + fingerprinted on frame 0 — the conventional choice for animated + content, and the one that keeps PIL from iterating every frame.""" + if getattr(pil_image, "is_animated", False): + try: + pil_image.seek(0) + except Exception: + pass def compute_phash(pil_image) -> str | None: """Perceptual hash of an opened PIL image, as a hex string. None on any failure (videos/unreadable/non-image). - For animated images (multi-frame WebP/GIF/APNG), explicitly seek to - frame 0 first. Without this, some PIL operations downstream of - imagehash.phash (convert("L"), resize) can iterate all frames and - blow past Celery's hard time limit on large animations - (operator-flagged 2026-05-26 against animated WebPs). The pHash of - frame 0 is the conventional choice for animated content. + Frame 0 for animated images: without the seek, PIL operations + downstream of imagehash.phash (convert("L"), resize) can iterate all + frames and blow past Celery's hard time limit on large animations + (operator-flagged 2026-05-26 against animated WebPs). """ try: - if getattr(pil_image, "is_animated", False): - try: - pil_image.seek(0) - except Exception: - pass + _seek_first_frame(pil_image) return str(imagehash.phash(pil_image, hash_size=HASH_SIZE)) except Exception: return None +def fingerprint(pil_image): + """A small grayscale thumbnail of an opened PIL image, for gate 3. + + Returns a detached PIL image (so the caller may close the original) or + None on any failure. PIL-only on purpose: numpy is an imagehash + transitive dependency, not a declared one for this path. + """ + try: + _seek_first_frame(pil_image) + return pil_image.convert("L").resize( + (FINGERPRINT_SIZE, FINGERPRINT_SIZE), Image.LANCZOS + ) + except Exception: + return None + + +def fingerprint_path(path) -> "Image.Image | None": + """`fingerprint` for a file on disk. None if it cannot be read — which + the gate treats as "not a duplicate".""" + try: + with Image.open(path) as im: + return fingerprint(im) + except Exception: + return None + + +def fingerprints_match( + a, b, + *, + max_mean_diff: float = FINGERPRINT_MAX_MEAN_DIFF, + changed_level: int = FINGERPRINT_CHANGED_LEVEL, + max_changed_fraction: float = FINGERPRINT_MAX_CHANGED_FRACTION, +) -> bool: + """True when two fingerprints are the same picture: no large global + drift AND no meaningful local region that differs. False on any + failure.""" + if a is None or b is None: + return False + try: + diff = ImageChops.difference(a, b) + if ImageStat.Stat(diff).mean[0] > max_mean_diff: + return False + hist = diff.histogram() + total = sum(hist) + if not total: + return False + changed = sum(hist[changed_level:]) + return (changed / total) <= max_changed_fraction + except Exception: + return False + + +def aspect_matches( + width: int | None, height: int | None, + cand_width: int | None, cand_height: int | None, + *, tol: float = ASPECT_TOL, +) -> bool: + """Gate 2. False when either side's dimensions are unknown — an + unmeasurable candidate is not a proven duplicate.""" + if not width or not height or not cand_width or not cand_height: + return False + a, b = width / height, cand_width / cand_height + if a <= 0 or b <= 0: + return False + return abs(a - b) / max(a, b) <= tol + + def find_similar( phash_hex: str, width: int, height: int, candidates: list[tuple[str, int, int, int]], threshold: int, + *, + confirm=None, ) -> tuple[str, int | None]: """candidates: (phash_hex, width, height, image_id). Returns one of ("none", None) / ("larger_exists", id) / ("smaller_exists", id). - First qualifying candidate wins (IR loop order).""" + First candidate to pass EVERY gate wins (IR loop order). + + `confirm` is gate 3: an optional callable(image_id) -> bool, called only + for a candidate that already passed the hash and aspect gates, and + expected to compare the two files' pixels. A rejected candidate does not + end the search — the loop moves on, so a false pre-filter hit cannot + mask a real duplicate further down the list. Omitting it leaves the + hash+aspect behaviour, which is what the unit tests exercise. + """ new_h = imagehash.hex_to_hash(phash_hex) for cand_hex, cw, ch, cid in candidates: try: dist = new_h - imagehash.hex_to_hash(cand_hex) except Exception: + # Includes the mismatched-length case while a library re-hash + # (migration 0098) is still in flight: an old 64-bit hash cannot + # be compared to a new 256-bit one, and skipping it degrades to + # "no dedup yet" rather than to a wrong merge. continue - if dist <= threshold: - if cw >= width and ch >= height: - return ("larger_exists", cid) - if width > cw or height > ch: - return ("smaller_exists", cid) + if dist > threshold: + continue + if not aspect_matches(width, height, cw, ch): + continue + if confirm is not None and not confirm(cid): + continue + if cw >= width and ch >= height: + return ("larger_exists", cid) + if width > cw or height > ch: + return ("smaller_exists", cid) return ("none", None) diff --git a/frontend/src/components/settings/ImportFiltersForm.vue b/frontend/src/components/settings/ImportFiltersForm.vue index 0ffb849..903342f 100644 --- a/frontend/src/components/settings/ImportFiltersForm.vue +++ b/frontend/src/components/settings/ImportFiltersForm.vue @@ -10,16 +10,20 @@
Near-duplicate sensitivity
- How aggressively imports merge look-alike images (perceptual-hash - distance). Lower it if edits/variants of the same image are - being dropped as duplicates; raise it to collapse more - look-alikes. Applies to new imports. + How wide a net imports cast when looking for the same image at a + different resolution (perceptual-hash distance, out of 256 bits). + A match is only merged if the two files also share an aspect ratio + and their pixels agree, so variant artwork — a different + outfit, expression or overlay on the same pose — is kept even at a + generous setting. Raise it if higher-resolution re-uploads + are landing as separate copies; lower it to merge less. + Applies to new imports.
{ if (!store.settings) store.loadSettings() }) // Labelled stops so the less-initiated get the gist without knowing what a -// Hamming distance is. 0 = byte-for-byte only; 10 = the shipped default. -const PHASH_TICKS = { 0: 'Exact', 4: 'Strict', 10: 'Default', 16: 'Loose' } +// Hamming distance is. 0 = an identical hash only; 24 = the shipped default. +// Bits out of 256 (utils/phash hash_size=16). The scale changed with +// migration 0098 — these are NOT the old 0-16 stops renamed (#4223). +const PHASH_TICKS = { 0: 'Exact', 12: 'Strict', 24: 'Default', 48: 'Loose' } // Downloader + schedule-defaults fields moved to // /subscriptions?tab=settings (operator decision 2026-05-27). This form // now only owns image-import filters. @@ -148,7 +154,7 @@ const local = reactive({ min_width: 0, min_height: 0, skip_transparent: false, transparency_threshold: 0.9, skip_single_color: false, single_color_threshold: 0.95, - phash_threshold: 10, + phash_threshold: 24, wip_title_tagging_enabled: true, wip_soft_title_tagging_enabled: false, }) diff --git a/tests/test_api_settings.py b/tests/test_api_settings.py index eef1dc6..24531fc 100644 --- a/tests/test_api_settings.py +++ b/tests/test_api_settings.py @@ -258,7 +258,7 @@ async def test_patch_rejects_non_object(client): async def test_import_settings_phash_threshold_default(client): resp = await client.get("/api/settings/import") assert resp.status_code == 200 - assert (await resp.get_json())["phash_threshold"] == 10 + assert (await resp.get_json())["phash_threshold"] == 24 @pytest.mark.asyncio diff --git a/tests/test_gallery_similar.py b/tests/test_gallery_similar.py index 2bd5e90..03b5402 100644 --- a/tests/test_gallery_similar.py +++ b/tests/test_gallery_similar.py @@ -146,12 +146,17 @@ async def test_similar_collapses_near_duplicate_phashes(db): dupes = [] for n in range(2, 7): # 5 near-identical reposts r = await _img(db, n, _vec(1, 0.01 * n)) - r.phash = "ffffffffffffffff" # identical perceptual hash + r.phash = "f" * 64 # identical perceptual hash dupes.append(r) + # 64 hex chars = the 256-bit hash utils.phash emits (#4223, migration + # 0098). The widths must match the real ones: _diversify_similar's + # dup_threshold moved 8 -> 32 with the hash, so 64-bit fixtures would + # now read as near-duplicates of each other and collapse the very rows + # this asserts come through. distinct_a = await _img(db, 7, _vec(1, 1)) - distinct_a.phash = "0000000000000000" + distinct_a.phash = "0" * 64 # 256 bits away from the dupes distinct_b = await _img(db, 8, _vec(0, 1)) - distinct_b.phash = "0f0f0f0f0f0f0f0f" + distinct_b.phash = "0f" * 32 # 128 bits away await db.flush() res = await GalleryService(db).similar(src.id, limit=10) diff --git a/tests/test_phash_dedup.py b/tests/test_phash_dedup.py index fe0bb26..ae88343 100644 --- a/tests/test_phash_dedup.py +++ b/tests/test_phash_dedup.py @@ -81,7 +81,7 @@ def test_non_similar_imports_with_phash(importer, import_layout): r = importer.import_one(src) assert r.status == "imported" row = importer.session.get(ImageRecord, r.image_id) - assert row.phash is not None and len(row.phash) == 16 + assert row.phash is not None and len(row.phash) == 64 # 256-bit def test_larger_existing_skips_new_phash_dup(importer, import_layout): @@ -223,6 +223,74 @@ def test_threshold_controls_match(importer, import_layout): assert r.status == "imported" # threshold 0 + far → independent import +# --- The three gates (#4223) ------------------------------------------------ +# +# Each of these opens the hash gate all the way (threshold 256 = every +# candidate passes) so the test is about the gate named in its title, and not +# about whether two fixtures happen to hash apart. That is the regression +# being guarded: the operator ran the dial down to 0 and STILL lost variants, +# because the hash was never the thing that could tell them apart. + + +def _wide_open(importer): + _set_threshold(importer, 256) + + +def test_variant_survives_a_wide_open_threshold(importer, import_layout): + """Same aspect, same size, different picture — only the pixel confirm can + save it, and it must.""" + import_root, _ = import_layout + a = import_root / "v.png" + _write_split(a, "v", (400, 400)) + _wide_open(importer) + assert importer.import_one(a).status == "imported" + + b = import_root / "h.png" + _write_split(b, "h", (400, 400)) + assert importer.import_one(b).status == "imported" + assert importer.session.execute( + select(func.count()).select_from(ImageRecord) + ).scalar_one() == 2 + + +def test_rescale_still_supersedes_at_a_wide_open_threshold(importer, import_layout): + """The other half of the deal: the merge the operator DOES want still + happens, and keeps the higher resolution.""" + import_root, _ = import_layout + small = import_root / "small.png" + _write_split(small, "v", (200, 200)) + _wide_open(importer) + r1 = importer.import_one(small) + assert r1.status == "imported" + + big = import_root / "big.png" + _write_split(big, "v", (900, 900)) + r2 = importer.import_one(big) + assert r2.status == "superseded" + assert r2.image_id == r1.image_id + + importer.session.expire_all() + row = importer.session.get(ImageRecord, r1.image_id) + assert row.width == 900 and row.height == 900 + + +def test_different_aspect_is_never_a_duplicate(importer, import_layout): + """Solid colours are pixel-identical once fingerprinted, so the aspect + gate is the only thing standing between a crop and a supersede.""" + import_root, _ = import_layout + square = import_root / "square.png" + _write(square, (90, 40, 180), (400, 400)) + _wide_open(importer) + assert importer.import_one(square).status == "imported" + + wide = import_root / "wide.png" + _write(wide, (90, 40, 180), (800, 400)) + assert importer.import_one(wide).status == "imported" + assert importer.session.execute( + select(func.count()).select_from(ImageRecord) + ).scalar_one() == 2 + + def test_import_task_maps_superseded_to_complete_and_requeues(): from backend.app.services.importer import ImportResult from backend.app.tasks.import_file import _map_result_to_status diff --git a/tests/test_phash_util.py b/tests/test_phash_util.py index 5d61553..50f29e1 100644 --- a/tests/test_phash_util.py +++ b/tests/test_phash_util.py @@ -3,7 +3,13 @@ import io import imagehash from PIL import Image -from backend.app.utils.phash import compute_phash, find_similar +from backend.app.utils.phash import ( + aspect_matches, + compute_phash, + find_similar, + fingerprint, + fingerprints_match, +) def _img(color, size=(64, 64)): @@ -25,7 +31,7 @@ def _split(orient, size=64): def test_compute_phash_stable_and_hex(): h1 = compute_phash(_img((10, 120, 200))) h2 = compute_phash(_img((10, 120, 200))) - assert isinstance(h1, str) and len(h1) == 16 # hash_size=8 -> 64-bit -> 16 hex + assert isinstance(h1, str) and len(h1) == 64 # hash_size=16 -> 256-bit -> 64 hex assert h1 == h2 @@ -62,3 +68,73 @@ def test_find_similar_threshold_boundary_inclusive_and_first_match(): assert rel == "larger_exists" rel2, _ = find_similar(h, 10, 10, [(far, 999, 999, 1)], threshold=d - 1) assert rel2 == "none" + + +# --- Gate 2: aspect ratio (#4223) ------------------------------------------- + +def test_aspect_matches_tolerates_rounding_but_not_a_crop(): + assert aspect_matches(1000, 1000, 250, 250) + assert aspect_matches(1999, 1000, 1000, 500) # off-by-one on a rescale + assert not aspect_matches(1000, 1000, 1000, 500) # 1:1 vs 2:1 + # Unmeasurable is not a proven duplicate — the gate fails closed. + assert not aspect_matches(1000, 1000, None, None) + assert not aspect_matches(0, 0, 100, 100) + + +def test_find_similar_skips_a_hash_twin_with_a_different_aspect(): + """A crop or re-canvas keeps the composition, so it can land inside the + threshold. Dimensions are what say it is not a rescale.""" + h = compute_phash(_img((123, 50, 7))) + rel, mid = find_similar(h, 100, 100, [(h, 400, 200, 9)], threshold=64) + assert rel == "none" and mid is None + + +# --- Gate 3: the pixel confirm (#4223) -------------------------------------- + +def test_fingerprints_match_across_a_rescale(): + assert fingerprints_match( + fingerprint(_split("v", 64)), fingerprint(_split("v", 512)) + ) + + +def test_fingerprints_reject_a_local_change(): + """The case the hash cannot see: same composition, one region redrawn.""" + base = _split("v", 128) + variant = base.copy() + for y in range(20, 60): + for x in range(80, 120): # a patch on the white half + variant.putpixel((x, y), (0, 0, 0)) + assert not fingerprints_match(fingerprint(base), fingerprint(variant)) + + +def test_fingerprints_match_is_false_when_either_side_is_missing(): + assert not fingerprints_match(fingerprint(_split("v")), None) + assert not fingerprints_match(None, None) + + +def test_find_similar_confirm_can_veto_a_hash_and_aspect_match(): + h = compute_phash(_img((123, 50, 7))) + cand = [(h, 400, 400, 9)] + rel, mid = find_similar(h, 100, 100, cand, threshold=64, confirm=lambda _: True) + assert rel == "larger_exists" and mid == 9 + rel2, mid2 = find_similar(h, 100, 100, cand, threshold=64, confirm=lambda _: False) + assert rel2 == "none" and mid2 is None + + +def test_find_similar_keeps_looking_after_a_veto(): + """A vetoed candidate must not end the search, or one false pre-filter hit + would hide the real duplicate sitting behind it.""" + h = compute_phash(_img((123, 50, 7))) + candidates = [(h, 400, 400, 1), (h, 400, 400, 2)] + rel, mid = find_similar( + h, 100, 100, candidates, threshold=64, confirm=lambda cid: cid == 2 + ) + assert rel == "larger_exists" and mid == 2 + + +def test_find_similar_skips_a_hash_of_the_wrong_width(): + """Mid-re-hash (migration 0098): a leftover 64-bit hash cannot be compared + to a 256-bit one. Skipping it degrades to no dedup, never to a merge.""" + h = compute_phash(_img((123, 50, 7))) + rel, mid = find_similar(h, 100, 100, [("ffffffffffffffff", 400, 400, 3)], threshold=64) + assert rel == "none" and mid is None