Skip to content

The md5 group as one unit; name-frequency's missing conjunct and its census leak; the monolith re-samples the envelope - #665

Merged
WilfordGrimley merged 1 commit into
masterfrom
feat/md5-group-unit-namefreq-envelope
Jul 30, 2026
Merged

The md5 group as one unit; name-frequency's missing conjunct and its census leak; the monolith re-samples the envelope#665
WilfordGrimley merged 1 commit into
masterfrom
feat/md5-group-unit-namefreq-envelope

Conversation

@WilfordGrimley

Copy link
Copy Markdown

Three owner-ruled fixes from one design conversation, in one PR. They are independent in mechanism but land in the same three files and the same three living docs; splitting them would have meant three PRs racing each other through local_identify_printing_tags.py.


Fix 1 — the md5 group behaves as one unit through the monolith

"the md5 dedupe should only fetch each identical image once across sources and then apply votes to the entire group as the fetched card passes through the monolith." — owner

The fetch half already existed (evidence_transfer, keyed on Card.md5_checksum). The vote half existed only on the phash distance-0 key. So the set that got a fetch saved and the set that got a vote propagated were different sets: byte-identical files always share a phash, but files sharing a phash are not necessarily byte-identical.

Checked before building, as instructed

The brief asked whether propagation might be redundant — evidence_transfer already gives every md5 sibling its own ImageEvidence row with byte-identical extractor values, so each member could plausibly reach the same conclusion on its own. It does not, and the reason is structural rather than incidental:

A Stage D printing deduction is not a function of the evidence row alone. local_calculate_verdicts._resolve_candidates_for_card keys the candidate list on Card.name, and md5-identical uploads from different sources routinely carry different names — that is the ordinary state of a cross-source catalogue. Members also differ on per-card eligibility.

test_the_unfetched_twin_has_no_verdict_of_its_own_without_propagation falsifies the "N independent deductions already agree" hypothesis on the fixture rather than arguing about it.

Shape

Stage C+ now runs two tiers through one propagation engine (_propagate_over_groups), which takes the grouping as a parameter and knows nothing about how it was keyed:

tier claim shares order
md5 exact identity — same bytes a printing vote first
phash d0 near identity a printing vote (unchanged from #660) second, fills only what md5 did not

That parameter is the seam #661's illustration-grain phash tier plugs into later — "a single grouping abstraction with the key as a parameter", arrived at here rather than deferred to it.

Two #660 defects fixed on the way

  • Source votes were read from representatives only. Stage D has no reason to reach a group's lowest pk first, so whenever it reached any other member, nothing propagated at all. Source votes are now read across every member, with one source per (group, identity) chosen deterministically — otherwise two vote-holders in one group generate duplicate (card, printing, anonymous_id) rows inside a single write batch, which the already-voted guard cannot catch because it is computed from the DB before the batch is written.
  • Representatives were never propagation targets. Same root cause, opposite direction.

A group whose members hold disagreeing verdicts is counted (groups_with_conflicting_verdicts), not silently averaged — byte-identical images cannot depict two different printings, so a disagreement is a real signal about the upstream deduction.

Propagation never overrides a member's own ineligibility

Per the owner constraint. A member already resolved, already confirmed to a canonical_card, not a CARD, or carrying a resolved custom-art / non-english tag is skipped. custom-art is the sharp one: it is the catalogue declaring that an image is not a faithful depiction of a printing, and a checksum must not overturn a human-visible declaration.

⚠️ The phash tier is left in place — flagged, not accreted

This is the open question the coordinator put to the owner and it did not come back before this was ready, so per instruction: it stays, and here is the conflict, plainly.

Removing it is itself a behaviour change, and it is currently the only propagation reaching cards with no md5 at allmd5_checksum is NULL for every LOCAL_FILE source by design and is never invented. But it is also the thing #661 says is arguably mis-grained: the owner's direction is that phash should eventually share an illustration (same artwork, possibly a different printing — a near-identity claim at a grain where a weaker claim is appropriate), not a printing verdict.

Decision needed: does the existing phash printing-verdict propagation come out now, or stay dormant until phash is rewired to illustration grain? Nothing here depends on the answer; the md5 tier is correct either way.


Fix 2a — run_name_frequency_elimination never looked at the image

"just because a card was printed exactly once doesn't mean that the image in our catalogue is an accurate depiction of that card, it may have a different border or another issue." — owner

Everything the 1:1 gate checked was a count. Counting establishes that if this card depicts one of the name's printings, then it is the uncovered one. Nothing established the antecedent, and the only filters that spoke to it were the declared custom-art / non-English tags — so an untagged altered border sailed straight through.

"It is only a vote" is a weaker defence than it sounds: #593 established that a machine vote is what the question feed renders as the suggestion to confirm, and the human's click returns as a full-weight USER vote. The human-backed consensus gate is the mechanism that launders it.

The owner leaned toward the conjunct; the conjunct is what shipped. The tier was not dropped even though it could have been cleanly (zero PilotRunLedger invocations, ever — nothing contaminated, no retraction). It is kept because the deduction is genuinely sound once the antecedent holds, and elimination reaches a population the image-based channels structurally cannot: a name whose single uncovered printing has no distinguishing collector line to read.

Shared, not duplicated

_apply_agreement_checks' border/frame check moved to local_identify_printing_tags.printing_attribute_disagreement; both callers now use it. The direction is forcedlocal_calculate_verdicts imports local_identify_printing_tags and never the reverse, so that is the only side of the pair both callers can reach without a cycle.

Sharing it also inherits PR #656's artist_ocr gate for free, which is precisely the half a second implementation would most likely have got wrong: illus_anchor_fired is nullable and bool(None) is indistinguishable from "ran and found no anchor", so an ungated frame check confidently answers "modern" for a card it has no anchor evidence about — and the resulting frame-mismatch is not rescannable, so the wrong answer is permanent.

No stored evidence means abstain

This module's usual "missing data is not evidence" rule points the other way here, deliberately: that rule protects a match from being vetoed by silence, whereas here silence is being asked to establish something. Abstentions are counted separately (abstained_no_evidence vs abstained_attribute_mismatch) so the cost of the gate is legible on the first run rather than inferred from a smaller total.


Fix 2b — the census leak (a fresh wrong positive, not a stale vote)

_eligible_base_queryset(NAME_FREQUENCY_ANONYMOUS_ID) was called with no run_id, making its "exclude cards already carrying this calculator's vote" lifetime. The gate is a count over exactly that population — so the calculator was taking a census over a pool it permanently shrinks itself:

  • run 1 — one unresolved eligible card, one uncovered printing. Sound; it votes.
  • later — a second upload of that name arrives from another source.
  • run 2 — the run-1 card is excluded forever by its own vote, so the name presents as having exactly one unresolved card and votes for the new one. Correctly the pool is two and the gate should abstain.

Nothing about the second card changed; only the size of the population the gate counts.

What changed and what was left, stated deliberately

  • Changed: run_name_frequency_elimination now passes run_id. Within-run resume still works — re-invoking with the same run_id still skips cards this run already voted on.
  • Left: compute_covered_printing_pks() stays catalogue-wide and unscoped — "covered" is a fact about the world, not about this calculator's progress, and run-scoping it would make every run treat every printing as uncovered. The two halves of the gate are scoped differently, on purpose.
  • Left: run_pilot's select_candidates and count_below_resolution_floor. Neither gates on a count over the returned population, so neither has this defect. Recorded in _eligible_base_queryset's own docstring so the asymmetry is visible from the function rather than only from its callers.

One existing test genuinely changed meaning: test_idempotent_on_a_second_invocation asserted second.votes_written == 0, which was a side effect of the lifetime exclusion and directly contradicted the 2026-07-29 run-scoping directive. It is now test_a_fresh_run_recasts_the_same_verdict_without_duplicating_the_row, asserting the property that actually matters: one row, same printing, superseded rather than accumulated.


Fix 3 — the monolith re-samples the envelope mid-pass

"host resampling is likely required (for steps that aren't fetch) as the same monolith will run for small datasets and large ones so needs to fit the available compute appropriately." — owner

PR #660 checked the envelope once, before Stage C. _EnvelopeSentry now re-samples at every stage seam: after Stage C, between each of Stage D's calculators and chip casters, and before each Stage C+ tier. Sample counts land on the ledger, so a run reporting one sample is visibly a run that never re-sampled.

The Stage D seam is a new optional envelope_check callback on stage_e_dispatch._run_stage_d, defaulting to None — the conveyor path is byte-identical, and it already samples per micro-batch above that function.

The three protections are not conflated

protection protects where
7/sec ceiling Google harvest_rate_coordinator, per request
the envelope this host operating_envelope, per seam — this fix
compute gating everything else batch sizing / concurrency slots

Halt semantics preserved. A breach still persists an EnvelopeTrip, still exits 3, still requires resolve_envelope_trip — no self-resume, and not converted to a throttle. Converting a host-load breach into a throttle would undo PR #644's 429/503 distinction from the opposite direction: going slower on Google does not reduce this box's own load.

A mid-pass halt tells the truth about what is on disk. The preflight can honestly say "nothing was written"; by the time a re-sample fires, real rows exist. The message says they stay, and names the --run-id to resume with.

Cadence: 60s minimum, derived not tuned. The host-load bar compares against the one-minute load average, so sampling faster re-reads a number that has not finished moving — it cannot detect a breach earlier, it only multiplies DB round trips.

Residual, reported rather than hidden

The seams are between calculators, not inside them. At catalogue scale a single calculator can run a long time between checks. Closing that gap means threading a progress callback into each of seven calculators' own batch loops — a real refactor of shared code, and the coordinator accepted shipping the reachable seams instead.


One deliberate duplication, with a tripwire

_members_eligible_for_a_propagated_vote expresses four catalogue-level facts that _eligible_base_queryset also expresses. It does not call that function — which bundles them with workload rules wrong for a propagation target (a scan-log exclusion keyed to the pilot's rescannable vocabulary; a deductive-backfill exclusion that is a "don't spend a scan" choice, not an ineligibility) — and that function could not be refactored to expose the four, because its own docstring records that several tests and stream_backstop_sweep assert against its compiled SQL.

Rather than duplicate silently, TestPropagationEligibilityMatchesTheBaseQueryset fails if the two ever disagree. This is the one place in the change where a predicate is expressed rather than reused, and it is flagged here because the project has three documented cases of duplicated logic drifting.


Verification — mutation red, restore green

Every mutation was applied to the shipped code, the targeted suite run, then restored.

# mutation result
M1 md5 tier returns no groups 3 failed
M2 source votes read from representatives only (the #660 shape) 2 failed
M3 propagation ignores member ineligibility 1 failed
M4 envelope re-sampling reverted to preflight-only 2 failed
M5 visual conjunct never disagrees 1 failed
M6 no-evidence no longer abstains 1 failed
M7 propagation eligibility drops the tag excludes 1 failed (the tripwire)
restored 263 passed

None of these is in the class the brief warned about — an assertion satisfied by a value both paths reach. Each mutant flips a row that only the fixed path can produce: M1/M2 turn a propagated vote into no row at all; M3 turns a skipped ineligible member into a written vote; M4 turns samples > 1 into samples == 1; M5/M6 turn an abstention into a vote. test_within_run_resume_still_skips_cards_this_run_already_voted_on deliberately survives M6's sibling mutation — a lifetime exclude also skips within-run — which is why the discriminating census test is a separate one.

Suites: test_run_pipeline, test_local_identify_printing_tags, test_local_calculate_verdicts, test_stage_e_dispatch — 520+ tests green.
Lint: ruff, isort, black, mypy, prettier all pass (pre-commit).
Migrations: no model changes, so none. Leaf is 0101_delete_printingtagvote, unmoved.
Write polarity: unchanged. --dry-run still withholds every write, including both new propagation tiers; no organ's dry-run default was inherited.

Docs

Living pages only, no dated report, per the standing rule: docs/identification-pipeline.md (Stage C+ md5 section), docs/features/printing-tags.md (both name-frequency fixes), docs/features/stage-e-operations.md (mid-pass re-sampling).

🤖 Generated with Claude Code

https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN

…ess + census leak, mid-pass envelope re-sampling

FIX 1 - THE MD5 GROUP BEHAVES AS ONE UNIT THROUGH THE MONOLITH

Owner: "the md5 dedupe should only fetch each identical image once across sources and then
apply votes to the entire group as the fetched card passes through the monolith."

The fetch half already existed (`evidence_transfer`, keyed on `Card.md5_checksum`). The vote
half existed only on the phash distance-0 key, so the set that got a fetch saved and the set
that got a vote propagated were DIFFERENT SETS - byte-identical files always share a phash,
but files sharing a phash are not necessarily byte-identical.

CHECKED BEFORE BUILDING, as instructed: propagation is NOT redundant. `evidence_transfer`
gives every md5 sibling its own ImageEvidence row with byte-identical extractor values, so it
is reasonable to ask whether each member already reaches the same conclusion independently.
It does not, structurally: a Stage D printing deduction is not a function of the evidence row
alone. `_resolve_candidates_for_card` keys the candidate list on `Card.name`, and md5-identical
uploads from different sources routinely carry different names. Members also differ on per-card
eligibility. `test_the_unfetched_twin_has_no_verdict_of_its_own_without_propagation` falsifies
the "N independent deductions already agree" hypothesis on the fixture rather than arguing it.

Stage C+ now runs TWO tiers through ONE propagation engine (`_propagate_over_groups`), which
takes the grouping as a parameter and knows nothing about how it was keyed:

  md5       exact identity -> shares a PRINTING vote. New. Runs first.
  phash d0  unchanged from PR #660. Runs second, filling only what md5 did not.

Two defects in the PR #660 propagation are fixed on the way:

  - SOURCE VOTES WERE READ FROM REPRESENTATIVES ONLY. Stage D has no reason to reach a group's
    lowest pk first, so whenever it reached any other member, nothing propagated. Source votes
    are now read across every member, and one source per (group, identity) is chosen
    deterministically so two vote-holders in one group cannot generate duplicate rows inside a
    single write batch.
  - REPRESENTATIVES WERE NEVER PROPAGATION TARGETS. Same root cause, opposite direction.

PROPAGATION NEVER OVERRIDES A MEMBER'S OWN INELIGIBILITY (owner constraint). A member already
resolved, already confirmed to a `canonical_card`, not a CARD, or carrying a resolved
custom-art/non-english tag is skipped. custom-art is the catalogue DECLARING the image is not a
faithful depiction of a printing; a checksum must not overturn that.

THE PHASH TIER IS LEFT IN PLACE, flagged rather than accreted. Issue #661 holds what phash
grouping is FOR - the owner's direction is that it should eventually share an ILLUSTRATION
(same artwork, possibly a different printing), not a printing verdict. Removing it now would
itself be a behaviour change, and it is currently the only propagation reaching cards with no
md5 at all (md5 is NULL for every LOCAL_FILE source by design). The `groups` parameter is the
seam that tier plugs into later.

FIX 2a - `run_name_frequency_elimination` NEVER LOOKED AT THE IMAGE

Owner: "just because a card was printed exactly once doesn't mean that the image in our
catalogue is an accurate depiction of that card, it may have a different border or another
issue." Owner leaned toward adding the conjunct rather than dropping the tier; the conjunct is
what shipped, and the reasoning for keeping the tier is in its own docstring.

Everything the 1:1 gate checked was a COUNT. Counting establishes that IF the card depicts one
of the name's printings THEN it is the uncovered one; nothing established the antecedent, and
the only filters that spoke to it were the DECLARED custom-art/non-english tags - so an
untagged altered border sailed through. "It is only a vote" is weaker than it sounds: #593
established a machine vote is what the question feed renders as the suggestion to confirm, and
the human's click returns as a full-weight USER vote.

The missing conjunct now requires the card's ALREADY-STORED evidence to be consistent with the
candidate printing. NOT a new implementation: `_apply_agreement_checks`' border/frame check was
lifted to `local_identify_printing_tags.printing_attribute_disagreement` and both callers now
share it. That direction is forced - `local_calculate_verdicts` imports
`local_identify_printing_tags`, never the reverse. Sharing also inherits PR #656's `artist_ocr`
gate for free, which is the half a second copy would most likely have got wrong.

NO STORED EVIDENCE MEANS ABSTAIN. This module's "missing data is not evidence" rule protects a
match from being VETOED by silence; here silence is being asked to ESTABLISH something, so it
points the other way. Counted separately from mismatches so the cost is legible.

FIX 2b - THE CENSUS LEAK (a fresh wrong positive, not a stale vote)

`_eligible_base_queryset(NAME_FREQUENCY_ANONYMOUS_ID)` was called with no `run_id`, making its
"exclude cards already carrying this calculator's vote" LIFETIME. The gate is a COUNT over
exactly that population, so the calculator was taking a census over a pool it permanently
shrinks itself: run 1 votes on a card, a second upload of that name arrives, and run 2 sees one
unresolved card where there are really two - and votes. Nothing about the second card changed;
only the size of the population the gate counts.

`compute_covered_printing_pks()` stays catalogue-wide and unscoped, deliberately: "covered" is
a fact about the world, not about this calculator's progress. `run_pilot`'s `select_candidates`
and `count_below_resolution_floor` are LEFT UNSCOPED - neither gates on a count over the
returned population, so neither has this defect. Stated in `_eligible_base_queryset`'s docstring
so the asymmetry is visible from the function rather than only from its callers.

FIX 3 - THE MONOLITH RE-SAMPLES THE ENVELOPE MID-PASS

Owner: "host resampling is likely required (for steps that aren't fetch) as the same monolith
will run for small datasets and large ones so needs to fit the available compute appropriately."

PR #660 checked the envelope ONCE, before Stage C. `_EnvelopeSentry` now re-samples at every
stage seam: after Stage C, between each of Stage D's calculators/casters (via a new OPTIONAL
`envelope_check` callback on `stage_e_dispatch._run_stage_d`, defaulting to None so the
conveyor is byte-identical), and before each Stage C+ tier. Sample counts land on the ledger.

HALT SEMANTICS PRESERVED. A breach still persists an EnvelopeTrip, still exits 3, still needs
`resolve_envelope_trip` - no self-resume, and NOT converted to a throttle (that is rate
pressure's channel, beneath Stage C, PR #644). A mid-pass halt message differs from the
preflight's: rows already written STAY written, and it names the `--run-id` to resume with.

Interval-gated at 60s so the check cannot become its own load. The number is derived, not
tuned: the host-load bar reads the ONE-MINUTE load average, so sampling faster re-reads a
number that has not finished moving.

RESIDUAL, reported not hidden: the seams are BETWEEN calculators, not inside them. Closing that
gap means threading a callback into each of seven calculators' own batch loops - a real
refactor of shared code, deliberately not done here.

DELIBERATE DUPLICATION, WITH A TRIPWIRE. `_members_eligible_for_a_propagated_vote` expresses
four catalogue-level facts `_eligible_base_queryset` also expresses. It does not call that
function (which bundles workload rules wrong for a propagation target) and that function could
not be refactored to expose them (its own docstring records that tests and
`stream_backstop_sweep` assert against its COMPILED SQL).
`TestPropagationEligibilityMatchesTheBaseQueryset` fails if the two ever disagree.

VERIFICATION - mutation red, restore green (7 mutants, all red; 263 tests green restored):

  M1  md5 tier returns no groups                     3 failed
  M2  source votes read from representatives only    2 failed
  M3  propagation ignores member ineligibility       1 failed
  M4  envelope re-sampling reverted to preflight     2 failed
  M5  visual conjunct never disagrees                1 failed
  M6  no-evidence no longer abstains                 1 failed
  M7  propagation eligibility drops the tag excludes 1 failed (the tripwire)

Suites: test_run_pipeline, test_local_identify_printing_tags,
test_local_calculate_verdicts, test_stage_e_dispatch - 520+ tests, all green.
No model changes, so no migration.

Docs: living pages only, no dated report - docs/identification-pipeline.md (Stage C+ md5
section), docs/features/printing-tags.md (both name-frequency fixes),
docs/features/stage-e-operations.md (mid-pass re-sampling).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
@WilfordGrimley
WilfordGrimley merged commit d55684f into master Jul 30, 2026
14 checks passed
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