fix: variant artwork was dropped as a near-duplicate even at threshold 0 (4223)
CI / lint (push) Failing after 3s
CI / extension-version (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
Build images / build-agent (push) Successful in 7s
CI / frontend-build (push) Successful in 30s
CI / backend-lint-and-test (push) Successful in 1m8s
Build images / build-web (push) Successful in 1m28s
Build images / smoke-web (push) Skipped
CI / integration (push) Successful in 2m51s
Build images / build-ml (push) Successful in 2m59s
Build images / promote (push) Skipped
CI / lint (push) Failing after 3s
CI / extension-version (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
Build images / build-agent (push) Successful in 7s
CI / frontend-build (push) Successful in 30s
CI / backend-lint-and-test (push) Successful in 1m8s
Build images / build-web (push) Successful in 1m28s
Build images / smoke-web (push) Skipped
CI / integration (push) Successful in 2m51s
Build images / build-ml (push) Successful in 2m59s
Build images / promote (push) Skipped
The operator reported a 15-image variant pack landing as 3 records, then reported variants STILL being dropped with phash_threshold at 0 — the floor of the dial. No setting could have fixed it: at hash_size=8 a pHash is 64 bits of coarse light/dark layout, so two variants sharing a composition produce the SAME bits. Distance 0 meant "identical hash", not "identical image", and the dial was simultaneously too coarse to keep variants and too tight to catch a re-encoded rescale. The hash no longer decides a merge on its own. find_similar now runs three gates, cheapest first: the threshold proposes candidates, aspect ratio (ASPECT_TOL, matching the tier-1 video path) rejects crops and re-canvases, and a pixel-level confirm on the two files accepts. Every gate fails closed — unknown dimensions, an unreadable candidate, a hash of the wrong width all mean "not a duplicate", because too strict keeps a redundant copy the operator can see while too loose deletes artwork only a source re-walk returns. - utils/phash.py: HASH_SIZE 8 -> 16 (256-bit, what ImageRepo always used); aspect_matches, fingerprint/fingerprint_path/fingerprints_match (PIL-only, mean drift + changed-pixel fraction), find_similar gains `confirm`. - importer: _pixel_confirmer supplies gate 3 on both dedup sites, lazily and cached, so a non-matching import costs no extra I/O. - 0098: widens image_record.phash to 64 chars and NULLs every value — a stored 64-bit hash cannot be compared to a 256-bit one, and backfill_phash is NULL-only, keyset-paginated and now on the daily beat, so the library re-hashes itself. Dedup degrades to sha256 until it finishes. - phash_threshold counts bits and the denominator went 64 -> 256, so the setting is reset to the new default of 24 (there is no honest carry-over) and the slider is rescaled to 0-64. - gallery_service dup_threshold 8 -> 32: the same fraction of the hash, so the Explore rail keeps the variance the operator tuned in on 2026-07-01. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user