Skip to content

Commit 2d305fc

Browse files
Wire the attribute-chip casters into an engine; strip the vote from the uncalled wrapper (#654)
The 2026-07-29 composition audit measured `Old Border`, `Modern Border` and `appropriate-bleed` at ZERO machine rows with no substitute. Cause: the only code that ever cast them was `local_identify_printing_tags.run_pilot` (a live-FETCH pilot, one completed run in its history) and `image_evidence.extract_card_evidence`, which has ZERO production callers - both engines call `compute_card_evidence` + `persist_evidence` directly. Border colour survived the same purge only because `local_layout_class_cast` independently re-derives it, and even that was reachable only from its own standalone command. NEW `local_attribute_chip_cast` (`frame-style-cast-v1`/`bleed-edge-cast-v1`): reads stored `ImageEvidence`, casts frame-style and bleed-edge chips, fetches NOTHING. Modelled on `local_layout_class_cast`, not on the pilot. Reuses `classify_frame_style`/`FRAME_STYLE_TO_TAG`/`FRAME_VOTE_CONFIDENCE`/ `BLEED_EDGE_TAG_NAME`/`BLEED_EDGE_VOTE_CONFIDENCE` verbatim so it cannot drift from the pilot on what a frame class is or what it is worth. Derivable populations, measured read-only 2026-07-29: 133,627 + 9,006 + 2,786 = 145,419 votes, none of which needs an image fetch. It does NOT cast border - that would be a THIRD border channel, and the duplication is the audit's own cull recommendation, not a pattern to extend. TWO IDENTITIES, not one. The bleed chip is negative-only, so under a shared identity a card's frame vote would read as "handled" and permanently strand its bleed chip. REQUIRED_EXTRACTOR_KEYS PER CHIP FAMILY. The frame chip gates on `artist_ocr` as well as `collector_line_ocr`: `illus_anchor_fired` is nullable and `bool(None)` is False, which without the gate classifies every card with no anchor evidence as `modern`. This is item 4's bug class, applied correctly from the start. WIRED INTO THE CONVEYOR. `stage_e_dispatch._run_stage_d` now runs both this caster and `local_layout_class_cast`, so all three chip families are reachable from an engine for the first time. Zero fetches means no envelope/fetch-budget cost. A missing tag seed is caught and logged at ERROR rather than marking a dispatch FAILED over an advisory chip after the printing votes already landed - the counters staying at zero is the observable, not a silent swallow. `extract_card_evidence` -> `fetch_and_compute_card_evidence_for_tests`, and its `cast_border_attribute_vote(...).save()` is deleted. Its test class inverted with it: `test_classified_border_casts_one_vote_per_card` was a green test over a channel that had not existed since the 2026-07-20 fetch/compute decoupling, which is precisely how the hole stayed invisible. Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent cacb86e commit 2d305fc

14 files changed

Lines changed: 1268 additions & 134 deletions

MPCAutofill/cardpicker/image_evidence.py

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@
99
1010
Persistence (`persist_evidence`) is a separate, thin step so callers control their own
1111
transaction boundaries (the bulk runner's future atomic-batch-seam, task #147 item 3; a
12-
lazy-mode task's own single-card transaction) - `extract_card_evidence` itself never touches
12+
lazy-mode task's own single-card transaction) - `fetch_and_compute_card_evidence_for_tests` itself never touches
1313
the DB, and image bytes never persist anywhere (CLAUDE.md's "Governing premise": we index, we
1414
do not store images) - they go out of scope the moment this function returns.
1515
1616
Extend this module (not ImageEvidence's callers) when adding a new extractor: fetch once at
17-
the top of `extract_card_evidence`, call each new pure extractor function against the same
17+
the top of `fetch_and_compute_card_evidence_for_tests`, call each new pure extractor function against the same
1818
in-memory image, and add its fields/version/skip-reason to the result. `fetch_health`,
1919
`geometry_bleed` (task #147), `layout_class`/`crop_coordinates` (issue #148, the geometry-group),
2020
and `collector_line_ocr`/`artist_ocr`/`collector_line_tsv` (issue #149, the OCR-group) exist
@@ -252,7 +252,6 @@
252252
from cardpicker.local_fallback import (
253253
ARTIST_CROP_BOX,
254254
SYMBOL_STRIP_BOX,
255-
cast_border_attribute_vote,
256255
classify_bleed_edge,
257256
classify_border_color,
258257
classify_frame_style,
@@ -643,7 +642,7 @@ def _confidently_digit_free(tier1_raw_texts: list[str]) -> bool:
643642
return all(text.strip() for text in tier1_raw_texts) and not any(_contains_digit(text) for text in tier1_raw_texts)
644643

645644

646-
def extract_card_evidence(
645+
def fetch_and_compute_card_evidence_for_tests(
647646
card: Card,
648647
dpi: Optional[int] = DEFAULT_FETCH_DPI,
649648
profile: Optional[dict[str, float]] = None,
@@ -654,18 +653,40 @@ def extract_card_evidence(
654653
name_artist_lookup: Optional[Callable[[str], tuple[str, ...]]] = None,
655654
) -> ExtractionResult:
656655
"""
657-
The per-card callable work unit - fetch, then compute. `card.content_phash` (not recomputed
658-
here) is the content hash this evidence is keyed against - hash-at-ingest (Part 2) already
659-
populates it for essentially every card by the time Stage C runs. If it's still null, the
660-
result's `content_hash` is None and `persist_evidence` will refuse to write a row, since
661-
ImageEvidence's "computed-once-forever" premise depends on a stable hash to key on.
656+
A TEST-ONLY CONVENIENCE WRAPPER. It has no production caller and has not had one since the
657+
2026-07-20 fetch/compute decoupling (#228): both engines - `run_image_evidence_cohort` (which
658+
splits fetch and compute across two pools) and `stage_e_dispatch._run_stage_c` - call
659+
`fetch_card_image` and `compute_card_evidence` + `persist_evidence` themselves. The name says
660+
so since 2026-07-30; it was `extract_card_evidence` for the four months this was not true, and
661+
that gap is what hid the defect below.
662+
663+
WHAT WAS DELETED HERE, and why the rename matters. Until 2026-07-30 this function's last act
664+
before returning was `cast_border_attribute_vote(...).save()` - a real machine vote, cast from a
665+
function nothing calls. The 2026-07-29 composition audit found it: the border chip survived the
666+
2026-07-29 purge only because `local_layout_class_cast` independently re-derives it, while the
667+
two chips whose only other caster was the live-fetch pilot (frame style, bleed edge) went to
668+
zero rows with nothing able to produce another. A vote cast in an uncalled function is
669+
indistinguishable from a wired channel by any grep, which is exactly how that stayed invisible.
670+
The chips are now cast by `local_attribute_chip_cast` and `local_layout_class_cast`, both of
671+
which read stored `ImageEvidence` and are wired into `stage_e_dispatch._run_stage_d`.
672+
673+
DO NOT ADD A WRITE HERE. Not a vote, not a `persist_evidence` call, not a `CardScanLog` row.
674+
Anything this function writes is unreachable in production by construction, and will be read as
675+
a live channel by anyone auditing the codebase later. Compute and return; the caller persists.
676+
677+
Behaviourally: fetch, then compute. `card.content_phash` (not recomputed here) is the content
678+
hash this evidence is keyed against - hash-at-ingest (Part 2) already populates it for
679+
essentially every card by the time Stage C runs. If it's still null, the result's
680+
`content_hash` is None and `persist_evidence` will refuse to write a row, since ImageEvidence's
681+
"computed-once-forever" premise depends on a stable hash to key on.
662682
663683
Split into a fetch step (here) + `compute_card_evidence` (2026-07-20, Stage C fetch/compute
664684
decoupling design, docs/features/catalog-completion-plan.md's Stage C section, #228) so a
665685
concurrent driver can run the fetch on an I/O-bound thread and the compute on a CPU-bound
666-
process, without this function's own single-caller behavior changing at all - every existing
667-
caller of `extract_card_evidence` (this pilot's tests, any future direct caller) still gets
668-
the exact same fetch-then-compute behavior in one call.
686+
process. That split is the reason this wrapper has no production caller: the drivers took the
687+
two halves and left the bundle behind. It is kept because ~95 tests exercise the extractors
688+
end-to-end through it, and a compute-only equivalent would have to re-stub the fetch at every
689+
one of those call sites for no gain.
669690
670691
`profile`, if given, is forwarded straight through to `compute_card_evidence` below and
671692
populated (in place) there with a `time.monotonic()`-delta timing breakdown - `fetch_ms`,
@@ -725,9 +746,9 @@ def extract_card_evidence(
725746
md5_checksum=card.md5_checksum,
726747
sha256_checksum=card.sha256_checksum,
727748
)
728-
vote = cast_border_attribute_vote(card, result.fields.get("layout_class") or None, confidence=0.5)
729-
if vote is not None:
730-
vote.save()
749+
# NO VOTE CAST HERE - see this function's own docstring. A `cast_border_attribute_vote(...)
750+
# .save()` used to sit on this line, unreachable in production because nothing calls this
751+
# function; removed 2026-07-30.
731752
return result
732753

733754

@@ -746,7 +767,7 @@ def compute_card_evidence(
746767
sha256_checksum: Optional[str] = None,
747768
) -> ExtractionResult:
748769
"""
749-
Compute-only continuation of `extract_card_evidence` above - everything that function does
770+
Compute-only continuation of `fetch_and_compute_card_evidence_for_tests` above - everything that function does
750771
AFTER its own fetch step, against an already-fetched `image` (a `PIL.Image.Image`, or `None`
751772
for a failed/skipped fetch) and a `fetch_latency_ms` the caller already measured. Takes a
752773
plain `card_id`/`content_hash` pair rather than a `Card` instance deliberately: this is the
@@ -762,10 +783,10 @@ def compute_card_evidence(
762783
the hardware's network-vs-compute core allocation).
763784
764785
`profile` (2026-07-20, docs/reports/2026-07-20-fetch-compute-timing-diagnostic.md): see
765-
`extract_card_evidence`'s own docstring for the full field breakdown. `fetch_ms` is set here
786+
`fetch_and_compute_card_evidence_for_tests`'s own docstring for the full field breakdown. `fetch_ms` is set here
766787
directly from the caller-supplied `fetch_latency_ms` (this function never measures its own
767788
fetch) - so the resulting profile shape is identical regardless of whether the caller is
768-
`extract_card_evidence` (bundled fetch+compute, one process/call) or the decoupled compute
789+
`fetch_and_compute_card_evidence_for_tests` (bundled fetch+compute, one process/call) or the decoupled compute
769790
stage calling this directly with a `fetch_latency_ms` its own separate fetch stage already
770791
measured.
771792
@@ -895,7 +916,7 @@ def compute_card_evidence(
895916
(79.0% of canonical names have exactly one, 93.1% at most two), used to narrow an
896917
otherwise-ambiguous reading. Passed in as a resolved tuple rather than a callable so this
897918
function keeps issuing no DB query of its own and stays picklable for the
898-
`ProcessPoolExecutor` compute stage; `extract_card_evidence`/`stage_e_dispatch` resolve it
919+
`ProcessPoolExecutor` compute stage; `fetch_and_compute_card_evidence_for_tests`/`stage_e_dispatch` resolve it
899920
from `collector_line_artist.build_name_artist_lookup()`, whose name resolution is
900921
`local_identify_printing_tags.CandidateNameIndex.candidates_for` - the codebase's existing
901922
normaliser, not a new one. Empty (the default) means no narrowing.
@@ -1413,7 +1434,7 @@ def current_evidence_queryset(card: Card) -> "QuerySet[ImageEvidence]":
14131434
def persist_evidence(result: ExtractionResult, run_id: Optional[str] = None) -> Optional[ImageEvidence]:
14141435
"""
14151436
The thin, separate DB-write step (see module docstring for why this is split from
1416-
`extract_card_evidence`). Refuses to write if `content_hash` is None. Uses
1437+
`fetch_and_compute_card_evidence_for_tests`). Refuses to write if `content_hash` is None. Uses
14171438
`get_or_create` + field merge (not a blind create) so a re-run against the SAME (card,
14181439
content_hash) pair updates in place rather than erroring on the unique constraint - this is
14191440
what makes independently-landing extractor PRs additive: each one's own pass only ever
@@ -1525,7 +1546,7 @@ def build_reconciliation_report(
15251546
"EXTRACTOR_AMBIGUOUS_SKIP_REASON",
15261547
"EXTRACTOR_NO_TEXT_SKIP_REASON",
15271548
"ExtractionResult",
1528-
"extract_card_evidence",
1549+
"fetch_and_compute_card_evidence_for_tests",
15291550
"compute_card_evidence",
15301551
"current_evidence_queryset",
15311552
"persist_evidence",

0 commit comments

Comments
 (0)