Stage C: run-scope the resume filter so a bulk run redoes work from scratch - #645
Merged
Merged
Conversation
…cratch `run_image_evidence_cohort`'s resume filter asked an IDENTITY-scoped question - "has this card ever been extracted at these manifest versions" - so a run could never redo work, only ever see a shrinking subset of the previous run's pool. PR #604 established the owner's ruling for Stage D ("a bulk run redoes everything from scratch; flags tell it to narrow"; "prior runs cannot pollute, but the current run can be resumed") and never reached Stage C. This applies the same predicate one layer up, via `ImageEvidence.run_id` (already stamped unconditionally by `persist_evidence` and `transfer_evidence`, already indexed). The second consequence this closes: a field added WITHOUT a version bump was permanently unreachable. `bleed_diff_mm` is NULL on 215,921/220,579 rows (97.9%), 213,131 of them on rows whose `bleed_class` is a confident `bleed`, so the extractor's abstain path does not explain it - every row carries the one `geometry-bleed-v1` version, so a v1 manifest written before the field existed reads as current and is skipped forever. Within-run resume is preserved and tested: `--run-id <the killed run's id>` skips exactly what that run finished. The command prints its own resume line at startup, and `--max-rss-mb`'s help no longer claims a bare re-invocation resumes. `--only-never-extracted` narrows back to the old identity-scoped predicate as an opt-in flag, per "default the default things, disable them with flags". `--card-ids-file` is GENERALISED rather than duplicated: forcing re-extraction is now what the default does, so the flag's hardcoded resume-filter bypass is gone and it reduces to the pure scope narrowing its name promises. Strict gain - a killed targeted re-extraction now resumes, which under the bypass it could not. Its existing test claimed to prove that bypass while seeding `{key: "v1"}`, which fails the version-aware filter regardless; it now seeds the CURRENT version map and proves the run-scoped mechanism that replaced the bypass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
WilfordGrimley
force-pushed
the
fix/stage-c-run-scoped-resume
branch
from
July 30, 2026 11:11
d118584 to
18585d3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Item 1 of the "essential major blockers" brief, sourced from PR #616 /
docs/reports/2026-07-29-pipeline-coverage-composition-audit.md.The defect
run_image_evidence_cohort.handle()skipped a card whenMANIFEST_EXTRACTOR_CURRENT_VERSIONS.items() <= extractor_versions.items().That is identity-scoped — has this card ever been extracted at these versions — not run-scoped. Two consequences:
local_calculate_verdicts._eligible_cards_queryset) and never reached Stage C.ImageEvidence.bleed_diff_mmis NULL on 215,921 of 220,579 rows (97.9%), and 213,131 of those sit on rows whosebleed_classis a confidentbleed— socompute_bleed_diff_mm's abstain path does not explain it. All rows carry the single versiongeometry-bleed-v1. The field and the line writing it were added without bumping it, so every historical row reads as current and is skipped forever.The fix
already_extracted_card_ids(run_id, only_never_extracted=False)— one predicate, both selection paths. Default scopes toImageEvidence.run_id(already stamped unconditionally bypersist_evidenceandtransfer_evidence, alreadydb_index=True, so the default path filters in SQL instead of streaming all 220,579extractor_versionsblobs through the process).--run-iddefaults to a fresh timestamp ⇒ a plain invocation redoes the catalogue from scratch.--run-idand it skips exactly what that run finished. The command now prints its resume line at startup, and--max-rss-mb's help no longer claims a bare re-invocation resumes.--only-never-extractednarrows back to the old identity-scoped predicate, per "default the default things, disable them with flags".--card-ids-filegeneralised, not duplicatedThe brief noted this flag already forces re-extraction but is "the wrong shape". Forcing re-extraction is now what the default does, so the flag's hardcoded resume-filter bypass is deleted and it reduces to the pure scope narrowing its name promises. Strict gain: a killed targeted re-extraction now resumes, which under the bypass it could never do.
Its existing test (
test_card_ids_file_bypasses_the_resume_filter) claimed to prove that bypass while seeding{key: "v1"}— a value that fails the version-aware filter regardless, so the bypass was never actually exercised. Renamed, reseeded with the CURRENT version map, and it now proves the run-scoped mechanism that replaced it.Does
bleed_diff_mmneed a version bump?No — run-scoping alone makes it reachable, because the next from-scratch bulk run re-extracts every card. But that costs ~220k image fetches.
compute_bleed_diff_mmreads onlycard_image.size, andImageEvidence.width/heightare stored and 0% NULL, so a no-fetch arithmetic recompute is identical to re-extraction and is the cheaper close. Recommended as a separate backfill (a write — needs authorisation). A bump remains the correct tool for the--only-never-extractedpath, where the stale-v1 trap still applies; it is not needed to close this field.Verification
rows.filter(run_id=run_id)(restore identity scoping)only_never_extracted(always run-scope)pytest cardpicker/tests/test_run_image_evidence_cohort.py— 67 passedpytest cardpicker/tests/test_stage_e_dispatch.py— 74 passed (the other consumer of the manifest constants)docs_lint.py --strict— cleancheck_extractor_manifest_sync.py— cleanDocs
docs/features/stage-e-operations.md"Bulk run preconditions" item 3 rewritten (living wiki doc per.github/wiki-publish-map.json). No dated report.🤖 Generated with Claude Code
https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN