Skip to content

Pre-scale program: source exclusion, yield reconciliation, future-work note - #24

Merged
WilfordGrimley merged 23 commits into
masterfrom
worktree-pilot-prescale
Jul 16, 2026
Merged

Pre-scale program: source exclusion, yield reconciliation, future-work note#24
WilfordGrimley merged 23 commits into
masterfrom
worktree-pilot-prescale

Conversation

@WilfordGrimley

@WilfordGrimley WilfordGrimley commented Jul 15, 2026

Copy link
Copy Markdown

Summary

Pre-scale program for the local OCR/phash printing-identification pilot (Stage 8), executed under the HOLD #1 go-ahead. HOLD #2 (full package report + full-catalog run authorization) is still pending — this PR accumulates everything gated on that hold.

Riders (HOLD #1 go-ahead):

  • Rider 2: flag-driven --exclude-sources-ocr/--exclude-sources-phash selection-time exclusion. OCR defaults to excluding source pk=1 (WilfordGrimley) — fully overridable, no source pk hardcoded in any query.
  • Rider 1: yield reconciliation paragraph in docs/features/printing-tags.md — separates the measured 62/250 (new logic) and 3/223 (old logic, measured) from the estimated ~7/27 old-logic tail (extrapolated, not measured).
  • Rider 3: logged the 1,097-card (Front)/(Back) name-matching fix as a future-work doc entry — not built, per instruction.

Item 2 — Batch-flush checkpointing: run_pilot() restructured to a single per-card loop with periodic flush (--batch-size, default 25) and a per-batch gate check, instead of one end-of-run write. Verified with a simulated-kill test: data already flushed survives a kill mid-run, and a plain re-invocation resumes cleanly via the existing idempotent selection query.

Item 3a/3b — Phase timing + CDN/quota: Measured per-phase wall-clock (fetch/OCR/phash breakdown). Found the image CDN Worker's full tier is a pure passthrough to lh4.googleusercontent.com — not R2-cached — and that the existing GOOGLE_DRIVE_RATE_LIMITER doesn't cover that path (different Google domain, different API). That gap is fixed by the standalone rate-limiter PR (#25), which must land before any full-catalog run. --fetch-budget is now documented as defense-in-depth alongside the Worker's own enforced limiter, not the primary safeguard.

Item 3c — Resolution floor, crop tightening, bleed-edge tagging:

  • Empirical 6-way dpi sweep (100/150/200/250/300/native) against a real 30-card sample: dpi≤150 degrades yield, dpi≥200 matches or exceeds native yield at 2-4x smaller payload. Adopted --fetch-dpi=250 as default (margin above the empirically-best 200).
  • Crop-box tightened using real Tesseract TSV bbox output to find dead space in the existing collector-line crop, validated with zero regression (identical match count and identical card-level match set, old vs. new box).
  • New: aspect-ratio-based bleed-edge classification (addendum item 7). Votes on the existing appropriate-bleed SENSITIVE tag (still requires a moderator co-sign regardless of machine votes). Uses chilli_axe's own reference trim size (CardWidthMM=63/CardHeightMM=88) plus the standard 1/8" bleed margin to compute two target aspect ratios, then classifies by which one a card's fetched image dimensions are closer to — purely geometric, no color/pixel heuristics, inherently robust to borderless/full-art cards (an earlier color-based design was discarded after finding it would false-positive on those). Verified independent of the new --fetch-dpi=250 resizing (0/15 mismatches, native vs. scaled classification) and validated against a 40-source diverse real sample (clean bimodal separation, no ambiguous middle cases).

Source pk=1 count (read-only, no bulk-tagging done): 20 cards in the OCR-eligible pool, 19 with a name-candidate.

Item 8 — DPI-tag audit (report only): Card.dpi is 99.97% at 300+ across the catalog — not a useful prioritization signal on its own. The low-res SENSITIVE tag has never been used in production (0 resolved, 0 pending, at any dpi bucket) — the report-flow exists in code but has never actually been exercised. Query mechanics sanity-checked against known-nonzero tags (NSFW, custom-art, AI-Generated, Borderless) before trusting the all-zero result. Logged a deferred future-work idea (art-crop-specific DPI check + Scryfall comparison, additive to Card.dpi not a replacement, would feed the moderation pipeline directly once built) rather than expanding this item's scope.

Item 3d — Pipeline concurrency + bleed-first crop normalization:

  • Live-contention test (not synthetic): 10 real cards, dry, against the live production DB, while a local probe hit the live API. Idle 79.8ms/94.7ms (mean/p95) → sequential 88.7ms/126.1ms → 2-worker concurrent 93.9ms/135.7ms. Only ~5ms extra mean latency for a near-ideal ~2.1x wall-clock speedup (13.42s → 6.34s for 10 cards) — tesseract's subprocess-based OCR genuinely parallelizes on this box's 2 cores. DEFAULT_WORKERS=2 adopted.
  • Also found (setting up the measurement): mpcautofill_django has no tesseract installed — confirms the host-venv execution path is the only one that currently works, relevant to Item 4's install-path decision.
  • Owner-directed mid-item: bleed classification now runs FIRST (before OCR/phash/border/frame/fallback) so its result can normalize the other four fixed-fraction crop boxes (OCR collector line, phash art crop, illus-anchor crop, symbol strip, border-sample bands) for the ~2.5% trimmed-image minority those boxes weren't tuned against. The border-sample-band case specifically got an empirical check (not just derivation) before inclusion — 15 real bleed-classified cards confirmed solid-color borders read identically regardless of exact sample position, so the existing bands are already correct for the majority.
  • Architecture: new _compute_card does the parallelizable pure-compute half (fetch, bleed-first, OCR, phash, border/frame, fallback) with no DB writes or shared state; run_pilot's DB-write/consensus loop stays single-threaded and unchanged, fed via ThreadPoolExecutor.map() (preserves order), chunked at batch_size (reuses Item 2's checkpointing boundary). OMP_THREAD_LIMIT=1 set when workers>1.
  • A real cross-thread DB-visibility bug was caught by the new tests (not just anticipated): a worker thread's own DB connection couldn't see pytest-django's uncommitted test transaction. Fixed with transactional_db, matching an existing precedent in test_sources.py for update_database()'s own worker threads.
  • --workers CLI flag added (default 2, --workers=1 disables concurrency).

Item 3e — Re-projected full-catalog wall-clock: Corrected an over-optimistic figure before it propagated: item 3d's narrow fetch+OCR+phash-only benchmark gave 2.1x concurrency speedup, but the full _compute_card pipeline (including detect_illus_anchor/pass-2 fallback, which don't parallelize as cleanly) measured 1.61x (2.520s/card sequential → 1.568s/card at 2 workers). Cross-validated bottom-up against a real top-down measurement: a real 300-card (392-candidate) --dry-run --workers 2 run timed end-to-end via the actual management command came in at 1.863s/card, consistent with the phase-delta projection. Full-catalog (171,853-card eligible pool) projection: native/single-threaded 6.42s/card (~12.8 days) → dpi=250+bleed+crop/single-threaded 4.48s/card (~8.9 days) → 2 workers 3.53s/card (~7.0 days), a real ~45% wall-clock reduction from items 3b/3c/3d combined.

Item 4 — Phash investigation + scaling proposal: Investigated a 13-vote phash yield drop between runs; formed a testable hypothesis (dpi=250 resolution regression), tested it directly against 12 real cards (native vs. dpi=250 phash outcome), found zero difference, and rejected the hypothesis rather than asserting it — more likely explanation is candidate-pool drift from the baseline run's own prior votes now excluded via idempotence (flagged honestly as not fully proven). Compared two scaling shapes against what already exists in this codebase: a screen'd host process (--workers=2, gets the full ~7.0-day projection, zero new infra) vs. chunked django-q nightly slices via the existing cluster (reuses established retry/schedule infra, but that cluster's Q_CLUSTER.cpu_affinity=1 — a deliberate live-traffic reservation — would cap it to the slower ~8.9-day single-threaded rate). Recommended the host-process path; flagged two residual gaps (the workers=2 safety check was only a ~20s burst test, not a sustained soak test; and the host venv used for every measurement was job-scoped/ephemeral) rather than presenting the proposal as fully resolved.

Dockerized execution, host-venv retired: Closed the host-venv gap from Item 4 directly: added tesseract-ocr tesseract-ocr-eng to docker/django/Dockerfile's shared builder stage (inherited by both webserver and worker targets). Verified end-to-end, not just "image builds" — rebuilt the worker image and ran local_identify_printing_tags --dry-run --limit 3 --skip-checks inside a one-off docker compose run --rm worker container against the real live DB; tesseract resolved (v5.5.0) and OCR/phash/fallback/attribute voting all executed correctly. The job-scoped host venv is now deleted — no job dependency for this recurring task lives outside the image.

Still to come on this branch before HOLD #2: the remaining addendum items — coverage-gap prioritization (item 1), demand order via edhrec_rank (item 3), skip-before-fetch resolution floor (item 4), and cluster dedup scoped to a single run (item 2a) — shipping together as the next commit(s) on this PR. Cluster-dedup's content_hash persistence (item 2b) and the questionFeed ordering mirror (item 5) are logged as separate follow-up work, not part of this PR.

Test plan

  • pytest cardpicker/tests/test_local_identify_printing_tags.py cardpicker/tests/test_local_fallback.py — 115 passed, including a new TestConcurrency class
  • mypy clean on all modified library/command files (pre-existing unrelated models.py errors only)
  • black/prettier/pre-commit hooks passed on all commits
  • Simulated-kill checkpointing test (flush survives a kill mid-batch, clean resume)
  • Bleed-edge classifier validated against real 40-source sample and against the --fetch-dpi=250 change specifically
  • Live-contention measurement against the real production API and DB (not simulated) — 2-worker default validated safe

🤖 Generated with Claude Code

https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ

…liation, future-work note

- --exclude-sources-ocr / --exclude-sources-phash flags, OCR defaults to
  excluding source pk=1 (WilfordGrimley), fully overridable, never hardcoded
- reconcile old vs new logic yield on the same fresh 250-card window
- log the 1,097-card (Front)/(Back) name-matching fix as future work
WilfordGrimley and others added 21 commits July 15, 2026 18:58
Split per-card compute (fetch/OCR/phash/border/frame/fallback) from the
sequential DB-write loop, run via ThreadPoolExecutor validated against
real live-API contention (2 workers, ~2.1x speedup, ~5ms extra latency
on this box's 2 cores). Bleed classification now runs first and
normalizes every other fixed-fraction crop box for the trimmed-image
minority. Fixed a real cross-thread DB-visibility bug caught by the new
concurrency tests (transactional_db, matching test_sources.py's
existing precedent for update_database()'s own worker threads).
Verified end-to-end with a real dry-run inside the rebuilt worker
container; closes the host-venv-disposition gap from the scaling
proposal.
select_candidates now sorts by coverage-gap tier, descending uncovered
count, edhrec_rank demand, candidate count, pk - replacing the old
multi-candidate-first split. Cards below the empirical dpi=200
resolution floor are excluded from selection entirely, never fetched.
New uncovered_printings_closed progress metric.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ
compute_own_image_clusters phashes our own eligible images and
collapses distance-0 clusters to one representative before slicing;
absorbed members skip OCR/phash/fallback entirely and get their vote
via propagation instead. Guards against double-voting a member that
already has its own vote from a prior run. No schema change (item 2b
deferred).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ
cast_bleed_edge_vote now writes a vote only for a 'trimmed' reading;
'bleed' (the ~97.5% common case) casts nothing at all, so absence of
a vote becomes the documented convention for normal bleed - avoids
flooding moderation with routine APPLY confirmations on a SENSITIVE
tag meant to flag the rare exception. Updated sensitive_tags.py's
comment, which previously documented the opposite convention.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ
Fresh instrumented 50-card run against current code shows fetch is
only ~13% of per-card cost; border/frame classification + pass-2
fallback dominate at ~65-72%. No separate fetch-thread pool exists
currently - _compute_card's single worker pool handles fetch+OCR+
phash+fallback together. This favors a core-count resize or manifest
mode over decoupling fetch threads.

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

Real 250-card dry run (not a burst) confirms ~1.94s/effective-card,
consistent with the prior top-down measurement, and independently
corroborates the ~26-28% cluster-absorption rate seen in the
bottleneck-split sample.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ
Exact OCPU/RAM/region values shouldn't sit in a public-facing doc -
keeping the substantive finding (core count matches DEFAULT_WORKERS)
without the specific numbers. History still has the prior values;
owner declined a history rewrite for this pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ
Diagnosed live: a token's printed collector line reads its parent
set's code, while its CanonicalCard candidates use token-specific set
codes that never match - structural, not a parsing bug. Item 1's
descending-uncovered-count ordering was front-loading generic
multi-set token names (huge candidate counts, near-zero coverage) to
the very front of every real selection, yielding 0/250 OCR votes in
today's soak tests where the original pre-items-1/3/4 baseline (94/300)
had none of this population dominating.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ
Real 2.24x speedup and healthy OCR yield (56/198, matching the
original baseline) once tokens are correctly excluded from selection.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ
Synthesizes infra prerequisites, corrected throughput, cluster/coverage
census, Track 4 status, git/branch audit, and the updated scaling
recommendation (single continuous run, not chunked slices, given the
now-real ~1.8-2.3 day runtime). Awaiting owner go-ahead for the
full-catalog run.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ
@WilfordGrimley
WilfordGrimley marked this pull request as ready for review July 16, 2026 02:32
@WilfordGrimley
WilfordGrimley merged commit e6b09d1 into master Jul 16, 2026
1 of 2 checks passed
@WilfordGrimley
WilfordGrimley deleted the worktree-pilot-prescale branch July 16, 2026 02:32
WilfordGrimley added a commit that referenced this pull request Jul 16, 2026
get_baked_git_sha reads a GIT_SHA file baked into the image at build
time (Dockerfile ARG + docker-compose build.args, both now require the
documented GIT_SHA=$(git rev-parse --short HEAD) prefix on the rebuild
command) - best-effort visibility only, logged at startup, never the
gate itself.

find_stale_applied_migrations is the actual hard gate: compares what
this image's own migrations/ directory knows about against what the DB
reports as applied (MigrationLoader vs MigrationRecorder) - pure DB+code
introspection, automates the PR #24/#26 stale-image lesson instead of
relying on someone remembering to check docker images timestamps.

Caught a real bug via the new test: MigrationRecorder.applied_migrations()
returns a dict, not a set - the first draft's `applied - disk` would have
raised TypeError at the first real invocation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016i9S7LQsCL3FGaih3ZTRBJ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant