Hash-at-ingest + two-threshold clustering: dedup as a free DB read - #27
Merged
Conversation
Repurposes the dead Card.image_hash field into content_phash (nullable, indexed) rather than adding a confusing second hash column. Hashes new cards at ingest (update_database) and backfills existing ones via a new management command, both at small CDN size. local_clustering replaces the disabled fetch-based cluster-dedup pre-pass with a pure DB-column read: d=0 propagates votes exactly as before, d<=2 is a narrowing prior (computed, not yet wired into candidate matching - flagged as a fast-follow). Validated against 300 same-printing/300 different-printing pairs harvested from the live full-catalog run: 0 false splits among 79 confirmed true duplicates, 0 false merges. Does not touch the live run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ
…laim numpy was only a transitive dependency (via imagehash), so CI's mypy pre-commit environment resolved a different version/stub than local dev and failed on np.nonzero()'s return type. Pinned numpy explicitly in requirements.txt and the mypy hook; also switched to .tolist() before iterating so the code itself doesn't depend on stub version behavior. Also corrects the "~500-650x win" framing in the docs: that number compared a recurring network fetch against a one-time-amortized in-memory read (different operations), and used the never-threaded disabled pre-pass as the baseline instead of what a threaded fetch pre-pass could have achieved (~3h). The honest comparison is ~60-90x. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ
…gest # Conflicts: # docs/features/printing-tags.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Coherent follow-on to the local printing-ID pilot's research findings (docs/features/printing-tags.md's prior-art read and phash-at-small-CDN-size sections): hash once at ingest, store, never recompute corpus-wide. Absorbs deferred item 2b. Built while the fast-follow-enabled full-catalog run (PR #26, merged) continues unattended — this PR does not touch that run, it's the architecture for the next one. Held as draft per instruction.
Card.image_hashturned out to be a dead field (migration 0046 — always written0, never read anywhere). Renamed/repurposed tocontent_phash(nullable, indexed) instead of adding a confusing second hash column. Doubles as federation-v1's reservedcontent_hash.update_database.hash_newly_created_cards— a real, threaded, best-effort fetch+hash for newly-created cards at a small CDN size. Correction to the brief's premise:update_databasenever touched image bytes before this, so this is genuine new cost, not a free byproduct. Scoped to CREATED cards only (content_phashwas never inbulk_update's field list, so there's nothing to persist for updated cards anyway) — flagged as a deliberate narrowing of "new/changed cards."local_backfill_content_phash— idempotent via a NULL filter (no separate--resumestate), batched, threaded.cardpicker/local_clustering.py): replaces the disabled fetch-based pre-pass entirely. d=0 propagates votes (unchanged semantics, restores the representative-filtering the disabled no-op had dropped). d<=2 is a narrowing prior — computed and tested, not yet wired into_compute_card's live candidate matching (flagged explicitly as a scoped-out fast-follow, not silently half-built).update_database→local_phash→imagehash/pytesseract) wasn't in the mypy pre-commit hook's isolated dependency list — fixed in.pre-commit-config.yaml.Projected next-run wall-clock: ~0.52-0.54 days (12.6-12.9h), vs. this run's own ~0.73 days, once a one-time ~2.8h backfill completes.
Out of scope (logged, not built): art-region second hash, multi-hash ensembles, deep-embedding dedup,
hash_sizere-tuning, and wiring the d<=2 narrowing hint into live candidate matching.Test plan
pytest cardpicker/tests/— 759 passed (2 pre-existing, unrelated moxfield failures)pre-commit run(ruff, isort, black, mypy, prettier) — all cleanmakemigrations --check— migration matches model state exactly🤖 Generated with Claude Code
https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ