Skip to content

Add Stage C evidence transfer, decoupled fetch-ahead, and echo suppression - #484

Merged
WilfordGrimley merged 2 commits into
masterfrom
md5-stage-c-overhaul
Jul 25, 2026
Merged

Add Stage C evidence transfer, decoupled fetch-ahead, and echo suppression#484
WilfordGrimley merged 2 commits into
masterfrom
md5-stage-c-overhaul

Conversation

@WilfordGrimley

@WilfordGrimley WilfordGrimley commented Jul 25, 2026

Copy link
Copy Markdown

Description

Issue #473 PR-2 (evidence transfer + staleness + interim Stage D guard), folded with issue #472 (decoupled fetch-ahead + echo suppression) per owner-approved "same function, one coherent change" 2026-07-25.

Stacking: originally cut from md5-checksum-substrate (PR #477, issue #473 PR-1). PR #477 has since merged to master (which now carries 0084_card_checksums with both Card.md5_checksum AND Card.sha256_checksum in one migration) — this branch has been rebased directly onto origin/master (GitHub auto-retargeted the base once #477 merged). Card.sha256_checksum is a real column on this base; no tolerant-read indirection is needed or used anymore.

Tron §8 gate — NO-GO → fixes (2026-07-25 round)

The first version of this PR failed Tron's gate with three blockers + four conditions. Fixed on the same branch, mapped below:

  1. Rebase onto origin/master — done (see Stacking above). Dropped the stacked-base-only 0084_card_md5_checksum migration entirely (superseded by master's own 0084_card_checksums); the transfer-fields migration is renumbered 0085 depending on 0084_card_checksums. Full backend suite run against the rebased tree (below).
  2. Deleted checksum_pairing's getattr tolerance — the module is removed entirely; every reader now reads card.sha256_checksum directly (the column exists on master). Added a test asserting a sha256 mismatch skips transfer with the real field populated (TestFindTransferSourcePairingRule::test_sha256_mismatch_is_a_loud_anomaly_and_skips_transfer, no monkeypatching).
  3. Compute-side deadlock (HIGH), fixed_run_stage_c's finally block now calls stop_event.set() before fetch_thread.join(), and drains fetch_queue so a put() already blocked on a full queue can complete and let the thread observe stop_event on its own next loop-top check. Regression test raises from the compute step (not fetch) mid-batch with more cards than the queue depth, so the fetch-ahead thread is genuinely racing ahead and blocked by the time compute crashes (TestDecoupledFetchAhead::test_a_compute_crash_does_not_wedge_the_fetch_ahead_thread, wall-clock-timeout-guarded on a background thread — proves "does not hang", not just "eventually finishes").
  4. Transfer-source integrity, tightened — eligibility now requires the SOURCE evidence row's own stamped md5_checksum to be NOT NULL and equal to the target's live md5 (evidence_transfer._current_sibling_evidence_queryset's own .filter(md5_checksum=card.md5_checksum), a strict lookup, never the null-tolerant md5_currency_q() rule — that rule stays scoped to CURRENCY checks only). A null-stamped legacy source is never eligible, so a fresh stamp is never minted on the copy from an unverified source. Two new tests (TestFindTransferSourceIntegrity).
  5. Durable anomaly flag — both anomaly paths in find_transfer_source (sha256 mismatch, content-hash mismatch) now write a CardScanLog(anonymous_id="evidence-transfer-v1", skip_reason=<specific anomaly>) row in addition to the existing logger.error call, so a whole-catalog run's anomalies are countable per card afterward. Two new tests asserting the row lands.
  6. Kill-switch addedsettings.STAGE_C_EVIDENCE_TRANSFER_ENABLED (default True — transfer is ON for this first pass), wired into find_transfer_source's own top (single choke point covers both call sites). docker-compose.prod.yml passes it through the same way STAGE_E_STREAMING_ENABLED already is. Exists for first-pass reversibility — a settings flip isolates whether a live anomaly originates in transfer, no redeploy needed. Two new tests.
  7. Three "all three Stage D calculators" comments fixed (local_calculate_verdicts.py, evidence_transfer.py, models.py) to state the two-calculator (join-key/fallback) guard reality, with the slow-path exclusion reasoning (it casts no machine vote, only a human-review routing marker) cited in place rather than just asserted. The fetch-fallback scope item named in issue md5 identity groups: checksum substrate, evidence transfer, and group-level vote pooling #473's own PR-2 text ("on 404/lockout, try an md5 sibling's source URL") is explicitly DEFERRED, not built — documented in docs/features/stage-e-operations.md's own "Evidence transfer and decoupled fetch-ahead" section rather than silently dropped.

What shipped (original scope, unchanged)

  1. Evidence transfer (cardpicker/evidence_transfer.py) — in both Stage C seams (stage_e_dispatch._run_stage_c, checked before handing a card to the fetch-ahead thread; run_image_evidence_cohort._fetch_one_card, checked before the network fetch), a card with a known Card.md5_checksum and an md5-sibling's own CURRENT full-manifest ImageEvidence row (now under the tightened integrity rule above) gets that row's field values copied onto its own (card, content_hash) row instead of paying for a real fetch+extraction pass. ImageEvidence gains md5_checksum/sha256_checksum (stamped at both transfer and real-extraction time) and transferred/transferred_from_card_id (migration 0085_imageevidence_transfer_fields.py).
  2. Staleness fiximage_evidence.current_evidence_queryset is the ONE shared "is this evidence row CURRENT" query (replacing N independent inline copies) — additionally requires the row's own stamped md5_checksum to agree with Card.md5_checksum whenever BOTH are non-null. Null-tolerant by design for CURRENCY (a legacy row or a card with no md5 stays current) — this null-tolerance is explicitly NOT reused for transfer-source eligibility (see condition 4 above).
  3. Interim Stage D guard — a card whose CURRENT evidence is transferred=True is excluded from the two machine-voting Stage D calculators (join-key, fallback — TRANSFERRED_INTERIM_GUARD_SKIP_REASON, rescannable). The slow-path calculator is deliberately NOT guarded. Removal is PR-3's own business — do not remove before that PR merges.
  4. Decoupled fetch-ahead (stage_e_dispatch._run_stage_c) — one fetch-ahead thread (never pooled), bounded (maxsize=2) queue, overlapping fetch with the sequential compute loop. GoogleFetchLockoutError still halts new fetches immediately; already-queued work drains first (FIFO). A non-lockout fetch-side exception is caught and re-raised in the main thread (separate regression test from condition 3's compute-side one).
  5. Echo suppression (cardpicker/stage_e_signals.py) — suppress_evidence_change_echo() wraps every ImageEvidence write _run_stage_c performs; the evidence-change receiver skips queuing a fresh dispatch for a write made from inside the dispatch loop itself. BULK-mode writes (run_image_evidence_cohort.py) are unaffected.

Checklist

  • I have installed pre-commit and installed the hooks with pre-commit install before creating any commits.
  • I have updated any related tests for code I modified or added new tests where appropriate — see the per-condition breakdown above plus the original scope's own tests (transfer happy-path/no-sibling/partial-manifest, staleness null-tolerant/stamped-mismatch, guard transferred-vs-real-extraction/slow-path-unguarded, decoupled-fetch overlap/lockout-drain/window-ordering).
  • I have manually tested my changes as follows:
    • Full backend suite on the rebased tree (venv /tmp/mpcvenv, real testcontainer Postgres+ES): pytest cardpicker -q2154 passed, 4 skipped, 0 failed in 206s. This is the tree CI has never seen (rebase + all seven fix conditions applied).
    • Every touched/new test file also run individually beforehand to isolate signal from this shared box's own testcontainer port contention (documented, known issue): test_stage_e_dispatch.py (51), test_run_image_evidence_cohort.py (47), test_evidence_transfer.py + test_stage_e_signals.py (25 → 107 combined with test_image_evidence.py), test_local_calculate_verdicts.py + test_image_evidence.py (209/266 combined), and the six other touched calculator/command test files (224).
    • pre-commit run (ruff/isort/black/mypy/prettier) clean on the full changed-file set, both commits.
    • manage.py makemigrations --check clean (no missing migrations) against the rebased tree.
  • I have updated any relevant documentation or created new documentation where appropriate — docs/features/stage-e-operations.md's Phase 2 ordering step + "Evidence transfer and decoupled fetch-ahead" section (now covering the kill-switch, strict transfer-source integrity, durable anomaly logging, and the explicitly-deferred fetch-fallback), and its "Evidence-change echo" section (marked RESOLVED); stage_e_shakedown.py's own matching module-docstring correction.

Open items

  1. Mandatory Tron gate: a fresh Tron §8-style review pass is required before this ships to a live prod run (standing project convention) — this round's fixes address the prior NO-GO's three blockers + four conditions but have not themselves been re-reviewed yet.
  2. PR-3 (group-level vote pooling) removes the interim Stage D guard added here — do not remove it independently.
  3. _run_cohort's own return tuple / final ledger counters were NOT extended with a transferred count (only the progress-line log was) — a minor observability gap, left as a follow-up rather than widening this PR's own blast radius on the BULK command's public return shape.
  4. Fetch fallback (issue md5 identity groups: checksum substrate, evidence transfer, and group-level vote pooling #473's own PR-2 scope text: "on 404/lockout, try an md5 sibling's source URL") is explicitly DEFERRED, not built in this PR — evidence transfer already covers the dominant win.

@WilfordGrimley
WilfordGrimley changed the base branch from md5-checksum-substrate to master July 25, 2026 20:01
…ssion

Issue #473 PR-2: md5-sibling evidence transfer (with a binding sha256
pairing rule) before a card is fetched, an md5-staleness fix to the
shared evidence-currency check, and a temporary interim guard excluding
transferred evidence from Stage D machine voting until PR-3 lands.

Issue #472 (folded per owner approval): a decoupled fetch-ahead thread
overlapping Stage C fetch with compute in the streaming dispatcher, and
evidence-change echo suppression for writes made from inside the
dispatch path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rebases onto origin/master (which now carries 0084_card_checksums with
both md5+sha256) and re-numbers the transfer-fields migration on top of
it. Removes checksum_pairing's getattr tolerance now that
Card.sha256_checksum is a real column. Tightens transfer-source
eligibility to a strict non-null md5 match (currency keeps its
null-tolerant rule; transfer sources never inherit it). Fixes a
compute-side deadlock in _run_stage_c's finally block (stop_event.set()
before fetch_thread.join(), plus draining the queue). Adds a durable
CardScanLog anomaly marker alongside the existing ERROR log for both
transfer anomaly paths. Adds settings.STAGE_C_EVIDENCE_TRANSFER_ENABLED
as a first-pass kill-switch (default True). Corrects three "all three
Stage D calculators" comments to the two-calculator (join-key/fallback)
guard reality, with the slow-path exclusion reasoning cited in place.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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