Disable cluster-dedup pre-pass: net wall-clock loss at full-catalog scale

Its own sequential ~21.6h cost (unaffected by --workers) exceeded the
compute time it saved by absorbing ~20-28% of cards - HOLD #2's own
numbers showed 1.82 days raw vs 2.34 days with clustering. Also zero
progress visibility for its entire duration, indistinguishable from a
hung process. compute_own_image_clusters/ClusterResult left intact for
a future chunk-scoped redesign; run_pilot now passes a no-op
ClusterResult instead of calling it. 4 integration tests marked skip
with a clear reason, not deleted.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ

Author: WilfordGrimley

MPCAutofill/cardpicker/local_identify_printing_tags.py

@@ -722,11 +722,26 @@ def run_pilot(
         printing_pks_in_scope.update(c.pk for c in s.candidates)
     uncovered_printing_pks_in_scope = printing_pks_in_scope - covered_printing_pks_before
 
-    # addendum item 2a (2026-07-15): collapse distance-0 duplicate-image clusters to one
-    # representative BEFORE slicing/chunking - only representatives reach _compute_card; an
-    # absorbed member's vote comes from propagation in the write loop below instead.
-    cluster_result = compute_own_image_clusters(list(all_selected_by_card_id.values()), fetch_dpi)
-    all_selected_by_card_id = {s.card.pk: s for s in cluster_result.representatives}
+    # addendum item 2a (2026-07-15) - DISABLED (2026-07-16, live full-catalog run): the
+    # pre-pass itself is a genuine wall-clock net LOSS at full-catalog scale, not just an
+    # optimization with a cost - it's a fully SEQUENTIAL fetch over the entire selected pool
+    # (~172k cards), unaffected by --workers (see task #108's original finding), costing
+    # ~21.6h fixed regardless of core count - MORE than the compute time it saves by
+    # absorbing ~20-28% of cards into cheap propagated votes. Confirmed directly against this
+    # session's own HOLD #2 numbers: raw/no-clustering projected 1.82 days vs.
+    # cluster-dedup-adjusted 2.34 days - the "optimization" made the real run slower. It also
+    # has zero progress visibility for its entire duration (no print statements inside
+    # compute_own_image_clusters), which looks identical to a hung process from the outside -
+    # a job silently in this phase for 31 minutes was mistaken for possibly stuck before this
+    # was diagnosed. Left in place, not called: `compute_own_image_clusters`, `ClusterResult`,
+    # and all of the propagation/absorption logic below still work correctly against a
+    # no-op ClusterResult (every selected card is its own "representative", zero clusters) -
+    # this is the minimal, structurally-safe way to disable the feature without touching the
+    # write-loop code that depends on `cluster_result`'s shape. A future chunk-scoped redesign
+    # (compute the hash from the SAME image _compute_card already fetches for OCR/phash,
+    # cluster within a chunk instead of the whole pool) could recover the dedup benefit
+    # without the sequential-pre-pass cost or the observability gap - not built here.
+    cluster_result = ClusterResult(representatives=list(all_selected_by_card_id.values()), members_by_representative={})
     attributes.cluster_count = len(cluster_result.members_by_representative)
     attributes.cards_absorbed_into_clusters = sum(len(m) for m in cluster_result.members_by_representative.values())
 

MPCAutofill/cardpicker/tests/test_local_identify_printing_tags.py

@@ -1390,9 +1390,24 @@ def test_workers_one_does_not_set_omp_thread_limit(self, db, monkeypatch):
         assert "OMP_THREAD_LIMIT" not in os.environ
 
 
+_CLUSTERING_DISABLED_REASON = (
+    "addendum item 2a's clustering pre-pass is disabled in run_pilot (2026-07-16) - it's a "
+    "sequential fetch over the whole selected pool, unaffected by --workers, and its own fixed "
+    "cost measurably exceeded the compute time it saved at full-catalog scale (see "
+    "local_identify_printing_tags.py's run_pilot comment at the cluster_result assignment). "
+    "compute_own_image_clusters itself is untouched and still tested directly above; only the "
+    "run_pilot integration is skipped until a future chunk-scoped redesign re-enables it."
+)
+
+
 class TestClusterDedup:
     """Addendum item 2a (2026-07-15): distance-0 (byte-identical fetched image) clustering,
-    scoped to this run only - no schema/content_hash persistence (that's item 2b, deferred)."""
+    scoped to this run only - no schema/content_hash persistence (that's item 2b, deferred).
+
+    NOTE (2026-07-16): run_pilot no longer calls compute_own_image_clusters (see
+    _CLUSTERING_DISABLED_REASON) - the tests below that exercise clustering directly still
+    pass and still matter; the ones that expect run_pilot's own integration to cluster are
+    marked skip, not deleted, so they're ready to re-enable alongside a future redesign."""
 
     def test_two_cards_with_identical_images_cluster_with_lower_pk_as_representative(self, db):
         CanonicalCardFactory(name="Forest")
@@ -1458,6 +1473,7 @@ def test_unfetchable_image_stays_a_singleton_representative(self, db, monkeypatc
         assert cluster_result.members_by_representative == {}
         assert [s.card.pk for s in cluster_result.representatives] == [card.pk]
 
+    @pytest.mark.skip(reason=_CLUSTERING_DISABLED_REASON)
     def test_accepted_vote_on_representative_propagates_to_absorbed_member(self, db, monkeypatch):
         printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa"))
         card_a = CardFactory(name="Forest")
@@ -1491,6 +1507,11 @@ def test_accepted_vote_on_representative_propagates_to_absorbed_member(self, db,
         assert vote_a.source == vote_b.source == VoteSource.OCR
         assert vote_a.is_no_match == vote_b.is_no_match is False
 
+    @pytest.mark.skip(
+        reason=_CLUSTERING_DISABLED_REASON + " Passes vacuously with clustering off (no "
+        "propagation is ever attempted, so the guard it tests is never exercised) - skipped "
+        "rather than left green for the wrong reason."
+    )
     def test_member_with_an_existing_vote_from_a_prior_run_is_not_double_voted_or_overwritten(self, db, monkeypatch):
         printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="aaa"))
         other_printing = CanonicalCardFactory(name="Forest", expansion=CanonicalExpansionFactory(code="bbb"))
@@ -1533,6 +1554,7 @@ def test_member_with_an_existing_vote_from_a_prior_run_is_not_double_voted_or_ov
         assert untouched_vote.pk == existing_vote.pk
         assert untouched_vote.printing_id == other_printing.pk  # unchanged, not overwritten
 
+    @pytest.mark.skip(reason=_CLUSTERING_DISABLED_REASON)
     def test_absorbed_member_never_reaches_ocr_or_phash_processing(self, db, monkeypatch):
         # the whole point of dedup is not re-running the expensive engines on cluster members -
         # this is the test that actually proves the efficiency win, not just vote correctness.
@@ -1559,6 +1581,7 @@ def recording_run_ocr_for_card(selected, image, crop_box, bleed_class=None):
         assert ocr_called_for_card_ids == [card_a.pk]
         assert card_b.pk not in ocr_called_for_card_ids
 
+    @pytest.mark.skip(reason=_CLUSTERING_DISABLED_REASON)
     def test_absorbed_members_own_engine_eligibility_still_runs_via_the_representative(self, db, monkeypatch):
         # card_a (the lower-pk representative) is only phash-eligible; card_b (absorbed member)
         # is only ocr-eligible - the representative must still run OCR on card_a's behalf, or

docs/lessons.md

@@ -332,3 +332,17 @@ scan time (`transform_image_into_object`/`unpack_name` extract name/tags/languag
   it requires either a one-time raw-filename capture added to the import path going forward
   (useless retroactively for already-imported cards) or re-deriving candidate filenames from the
   Drive API directly per source (expensive, not a DB query).
+
+## A sequential single-item pre-pass over a large pool needs its own progress logging, not just the loop after it
+
+`local_identify_printing_tags.py`'s cluster-dedup pre-pass (`compute_own_image_clusters`)
+fetches every selected candidate's image ONE AT A TIME before the main chunked loop - which
+does have `progress_every` logging - even starts. A full-catalog run sat silent for 31 minutes
+before anyone could tell whether it was working or hung, because the pre-pass itself prints
+nothing for its entire (potentially many-hour) duration. Same shape as "verify claims before
+trusting aggregate numbers" (see this doc's other entries), applied to job observability
+specifically: a genuinely-working process with zero output is indistinguishable from a dead one
+from the outside, and "give it more time" is not a diagnosis. Any future sequential phase over a
+large pool - a pre-pass, a warm-up cache fill, a one-time backfill scan - needs a periodic print
+(even a bare `print(f"... {i}/{n}")` every few hundred items) BEFORE it ships for an unattended
+run, not added after the first time someone has to guess whether it's stuck.