diff --git a/MPCAutofill/cardpicker/management/commands/backfill_md5_checksums.py b/MPCAutofill/cardpicker/management/commands/backfill_md5_checksums.py new file mode 100644 index 000000000..bfa731a6d --- /dev/null +++ b/MPCAutofill/cardpicker/management/commands/backfill_md5_checksums.py @@ -0,0 +1,181 @@ +""" +See `cardpicker.md5_backfill`'s own module docstring for the full re-walk/reconcile design +(issue #473 PR-1, now covering both md5 and sha256 in the one pass - the command keeps its +original md5-only name since md5 remains the primary, grouping-defining field). This file is +deliberately thin - Command.handle() wires the forced-dry-run guard + PilotRunLedger lifecycle +rails (issue #362/#373 convention, `cardpicker.pilot_run_lifecycle`) around `run_md5_backfill`, +matching every other big write command's own shape (e.g. `reparse_collector_evidence`, +`consensus_recompute`). +""" + +from typing import Any + +from django.core.management.base import BaseCommand, CommandError, CommandParser +from django.utils import timezone + +from cardpicker.local_identify_printing_tags import generate_run_id +from cardpicker.md5_backfill import DEFAULT_BULK_UPDATE_BATCH_SIZE, run_md5_backfill +from cardpicker.models import PilotRunLedger +from cardpicker.pilot_run_lifecycle import ( + add_dry_run_guard_arguments, + enforce_dry_run_precondition, + initial_counters, + mark_ledger_failed, + merge_counters, + resilient_terminal_output, + scope_hash, +) +from cardpicker.utils import find_stale_applied_migrations, get_baked_git_sha + + +class Command(BaseCommand): + help = ( + "Issue #473 PR-1: re-walks every GOOGLE_DRIVE source's Drive folder listing (metadata " + "only - zero image fetches) and reconciles each listing's md5Checksum AND sha256Checksum " + "against the currently-stored Card.md5_checksum/Card.sha256_checksum (owner-approved " + "sha256 addition, 2026-07-25 evening - same walk, same seam). LOCAL_FILE sources carry " + "neither checksum in their listings and are always a no-op here (reported, not silently " + "skipped). Dry-run by default: prints per-field coverage (matched/planned-write counts " + "for md5 and sha256 separately, since their listing coverage can differ), plus the " + "md5-only group count and dupe factor for cross-check against issue #442's own sizing " + "walk (18.87% dupe rate, 24,712/130,960 files, 12,275 groups) BEFORE any --write. " + "--write requires a matching COMPLETED dry-run of the SAME --source-key selection within " + "--dry-run-window-hours (forced-dry-run guard, issue #362) - see --skip-dryrun-check to " + "override. A PilotRunLedger row is written either way." + ) + + def add_arguments(self, parser: CommandParser) -> None: + parser.add_argument( + "--source-key", + action="append", + default=None, + dest="source_keys", + help="Restrict the walk to this source key (repeatable). Default: every source " + "(GOOGLE_DRIVE sources are walked; other source types are reported as skipped, not " + "walked - see this command's own --help intro).", + ) + parser.add_argument( + "--write", + action="store_true", + default=False, + help="Actually persist reconciled checksums. Default is dry-run: compute and count " + "everything without writing. Requires a matching recent COMPLETED dry-run ledger row " + "for the SAME --source-key selection (forced-dry-run guard) unless " + "--skip-dryrun-check is passed.", + ) + parser.add_argument( + "--batch-size", + type=int, + default=DEFAULT_BULK_UPDATE_BATCH_SIZE, + help=f"Cards per bulk_update batch. Default: {DEFAULT_BULK_UPDATE_BATCH_SIZE}.", + ) + parser.add_argument("--run-id", default=None, help="Reuse a specific run_id. Default: freshly generated.") + add_dry_run_guard_arguments(parser, write_flag="--write") + + def handle(self, *args: Any, **kwargs: Any) -> None: + stale = find_stale_applied_migrations() + if stale: + raise CommandError( + f"STALE IMAGE: the DB has {len(stale)} migration(s) applied that this image's " + f"own code doesn't know about ({stale[:10]}{'...' if len(stale) > 10 else ''}) - " + "this image is older than a previously-deployed one. Rebuild with the current " + "code before running this command." + ) + + source_keys = kwargs["source_keys"] + write = kwargs["write"] + batch_size = kwargs["batch_size"] + dry_run = not write + + # Forced-dry-run guard scope (issue #362 convention): the INPUT that defines this + # invocation's own target cohort - the sorted --source-key list, or None (matching + # consensus_recompute's own "no caller-chosen cohort narrower than the whole command" + # reasoning) when every source is in scope. + scope = scope_hash("source_keys", ",".join(sorted(source_keys))) if source_keys else None + skip_used = enforce_dry_run_precondition( + command="backfill_md5_checksums", + write_mode=write, + skip_check=kwargs["skip_dryrun_check"], + window_hours=kwargs["dry_run_window_hours"], + scope=scope, + ) + + run_id = kwargs["run_id"] or generate_run_id() + mode = "WRITE" if write else "DRY RUN" + self.stdout.write( + f"[{mode}] backfill_md5_checksums run_id={run_id} " + f"source_keys={source_keys or 'ALL'} batch_size={batch_size}" + ) + + ledger = PilotRunLedger.objects.create( + run_id=run_id, + command="backfill_md5_checksums", + dry_run=dry_run, + status=PilotRunLedger.Status.RUNNING, + git_sha=get_baked_git_sha(), + counters=initial_counters(scope=scope, skip_dryrun_check_used=skip_used), + ) + try: + result = run_md5_backfill(dry_run=dry_run, source_keys=source_keys, batch_size=batch_size) + + # Counters-before-output (cardpicker.pilot_run_lifecycle's own module docstring + # point 1) - the ledger row is saved COMPLETED here, before the terminal summary + # print below. + ledger.status = PilotRunLedger.Status.COMPLETED + ledger.finished_at = timezone.now() + ledger.votes_written = result.written + ledger.counters = merge_counters( + ledger.counters, + { + "sources_scanned": result.sources_scanned, + "sources_skipped_no_checksum_support": result.sources_skipped_no_checksum_support, + "sources_unreachable": result.sources_unreachable, + "matched_files": result.matched_files, + "md5_planned_writes": result.md5_planned_writes, + "sha256_matched_files": result.sha256_matched_files, + "sha256_planned_writes": result.sha256_planned_writes, + "planned_writes": result.planned_writes, + "written": result.written, + "dupe_groups": result.dupe_groups, + "dupe_files": result.dupe_files, + "dupe_factor": result.dupe_factor, + }, + ) + ledger.save(update_fields=["status", "finished_at", "votes_written", "counters"]) + + with resilient_terminal_output(): + self.stdout.write( + f"sources_scanned={result.sources_scanned} " + f"sources_skipped_no_checksum_support={len(result.sources_skipped_no_checksum_support)} " + f"sources_unreachable={len(result.sources_unreachable)}" + ) + if result.sources_unreachable: + self.stdout.write(f" unreachable source keys: {result.sources_unreachable}") + # per-field coverage, reported separately since md5/sha256 listing coverage can + # differ (owner-approved sha256 addition, 2026-07-25 evening). + self.stdout.write( + f"md5: matched_files={result.matched_files} planned_writes={result.md5_planned_writes}" + ) + self.stdout.write( + f"sha256: matched_files={result.sha256_matched_files} " + f"planned_writes={result.sha256_planned_writes}" + ) + self.stdout.write( + f"dupe_groups={result.dupe_groups} dupe_files={result.dupe_files} " + f"dupe_factor={result.dupe_factor:.4%} (md5-only - groups key on md5 " + "exclusively, per issue #473 ruling 1)" + ) + self.stdout.write( + "Reconcile the md5 matched_files line and the dupe stats above against issue " + "#442's own sizing walk (18.87% dupe rate, 24,712/130,960 files, 12,275 " + "groups) before running --write." + ) + if dry_run: + self.stdout.write(f"(dry-run) would_write={result.planned_writes}") + else: + self.stdout.write(f"written={result.written}") + except Exception as exc: + # Shared FAILED-transition rail (cardpicker.pilot_run_lifecycle.mark_ledger_failed) - + # a no-op if this invocation already reached the COMPLETED save above. + mark_ledger_failed(ledger, exc) + raise diff --git a/MPCAutofill/cardpicker/md5_backfill.py b/MPCAutofill/cardpicker/md5_backfill.py new file mode 100644 index 000000000..874740907 --- /dev/null +++ b/MPCAutofill/cardpicker/md5_backfill.py @@ -0,0 +1,238 @@ +""" +Checksum backfill (GitHub issue #473 PR-1 - "checksum substrate", `docs/features/ +catalog-completion-plan.md`'s #442-sourced "index Drive checksums" leverage). Fills BOTH +`Card.md5_checksum` and `Card.sha256_checksum` in the one pass - the module keeps its original +name (`md5_backfill`/`backfill_md5_checksums`) since md5 is still the primary, grouping-defining +field (issue #473 ruling 1: groups key on md5 exclusively); sha256 was added to the same listing +walk and the same migration (owner-approved addition, 2026-07-25 evening) as a transfer-safety +pairing for PR-2, not a second grouping axis - see `Card.sha256_checksum`'s own docstring in +`cardpicker.models` for the full binding rule. + +Re-walks every `GOOGLE_DRIVE` source's Drive folder listing (metadata only - ZERO image fetches, +the same guarantee `cardpicker.sources.update_database.explore_folder` already gives its other +callers) and reconciles each listing's `md5Checksum`/`sha256Checksum` against the currently-stored +`Card.md5_checksum`/`Card.sha256_checksum` for every `Card` row whose `identifier` (Drive file id) +appears in that listing. Reuses `explore_folder` (its own thread pool + progress printing) and +`cardpicker.sources.api.execute_google_drive_api_call` (the shared, already-rate-limited Drive API +client `get_all_images_inside_folder` calls through) - this module hand-rolls neither a new client +nor new pacing. + +LOCAL_FILE sources carry neither checksum in their listings at all (see +`cardpicker.sources.source_types.LocalFile.get_all_images_inside_folder`, which never sets +`Image.md5_checksum`/`Image.sha256_checksum`) and are therefore skipped entirely by +`run_md5_backfill` - not walked, not counted as "unreachable" (that status is reserved for a +GOOGLE_DRIVE source whose root folder genuinely couldn't be resolved). This matches +`Card.md5_checksum`/`Card.sha256_checksum`'s own docstrings: null there is the correct, permanent +state for a checksum-less source, per the owner's ruling 3 on issue #473 ("a card with null or +unique md5 is a group of one... never invent a checksum") - the same never-invent posture applies +to sha256. + +The two fields are tracked and written INDEPENDENTLY per card, since Drive's own coverage differs +between them (sha256Checksum is less consistently populated than md5Checksum for older files) - +an entry carrying one but not the other never has its missing field invented, and a stored value +is never overwritten with an absent listing value. + +NEVER writes an invented/derived checksum, and NEVER nulls out an existing one: a Card whose Drive +file id no longer appears in its source's current listing (deleted/moved on the Drive side) is +left completely untouched here - deletion is a separate, already-covered code path +(`bulk_sync_objects`'s own `deleted_ids` handling), not something this reconciliation duplicates +or second-guesses. + +DUPE-FACTOR RECONCILIATION (the PR-1 acceptance criterion - "backfill dry-run numbers reconcile +with #442's walk before --write"): `BackfillResult.matched_files`/`dupe_groups`/`dupe_files`/ +`dupe_factor` are computed with the same definitions #442's own sizing walk used - "files with +both a Card row and an md5Checksum" (`matched_files`), grouped by checksum value across ALL +sources (`dupe_groups`/`dupe_files` - #442 measured cross-source dupes as the dominant case, so +grouping is deliberately global, never scoped to one source at a time). This dupe accounting is +MD5-ONLY, unchanged by the sha256 addition - per ruling 1, groups key on md5 exclusively, so there +is no analogous "sha256 dupe factor" to compute. `BackfillResult.sha256_matched_files`/ +`sha256_planned_writes` are the parallel PER-FIELD COVERAGE counters for sha256 (module docstring +above), reported separately in the dry-run output since the two fields' coverage can differ. +""" + +from dataclasses import dataclass, field +from typing import Optional + +from cardpicker.models import Card, Source +from cardpicker.sources.source_types import SourceTypeChoices +from cardpicker.sources.update_database import explore_folder + +DEFAULT_BULK_UPDATE_BATCH_SIZE = 1000 + + +@dataclass +class ChecksumEntry: + """One listing entry's checksum(s), for one Drive file id. Either field may be `None` + independently - see module docstring's "tracked and written INDEPENDENTLY" note.""" + + md5_checksum: Optional[str] = None + sha256_checksum: Optional[str] = None + + +@dataclass +class SourceWalkResult: + source_key: str + reachable: bool = True + # Drive file id -> ChecksumEntry, for every image in this source's CURRENT listing that + # carries at least one of the two checksums (see this module's own docstring for why a + # listing entry might carry neither, or only one). + checksums_by_identifier: dict[str, ChecksumEntry] = field(default_factory=dict) + + +def walk_source_checksums(source: Source) -> SourceWalkResult: + """ + One GOOGLE_DRIVE source's own listing walk - metadata only, no image fetch (see module + docstring). `reachable=False` means the source's root folder itself couldn't be resolved + (matches `SourceType.get_all_folders`'s own `None`-for-a-dead-source contract, e.g. a + since-deleted or permission-revoked Drive folder) - distinct from "resolved fine, but this + walk's checksums_by_identifier is empty", which is a genuine (if surprising) zero-checksum + result, not a failure. + """ + source_type = SourceTypeChoices.get_source_type(SourceTypeChoices[source.source_type]) + root_folder = source_type.get_all_folders([source]).get(source.key) + if root_folder is None: + return SourceWalkResult(source_key=source.key, reachable=False) + + images = explore_folder(source=source, source_type=source_type, root_folder=root_folder) + checksums = { + image.id: ChecksumEntry(md5_checksum=image.md5_checksum, sha256_checksum=image.sha256_checksum) + for image in images + if image.md5_checksum or image.sha256_checksum + } + return SourceWalkResult(source_key=source.key, checksums_by_identifier=checksums) + + +@dataclass +class BackfillResult: + dry_run: bool = True + sources_scanned: int = 0 + sources_skipped_no_checksum_support: list[str] = field(default_factory=list) + sources_unreachable: list[str] = field(default_factory=list) + # "files with both a Card row and an md5Checksum" - #442's own sizing-walk definition, + # unchanged by the sha256 addition (this is the number that reconciles against #442's + # 18.87%/12,275-group walk). + matched_files: int = 0 + # of matched_files, how many have a stored Card.md5_checksum that differs from (or is NULL + # against) the listing's own value. + md5_planned_writes: int = 0 + # sha256 per-field coverage counters (owner-approved addition, 2026-07-25 evening) - same + # definitions as the two md5 counters above, but for sha256, tracked SEPARATELY since Drive's + # sha256Checksum coverage can differ from md5Checksum coverage for the same file set (see + # module docstring). + sha256_matched_files: int = 0 + sha256_planned_writes: int = 0 + # total distinct cards that need (or, post-write, received) an update this run - a card + # needing BOTH an md5 and a sha256 write is counted once here, not twice (one bulk_update row + # either way). What --write would actually persist / has actually persisted. + planned_writes: int = 0 + # actually persisted; stays 0 for a dry run. + written: int = 0 + # md5-only (issue #473 ruling 1: groups key on md5 exclusively) - see module docstring's + # "DUPE-FACTOR RECONCILIATION" section. + dupe_groups: int = 0 + dupe_files: int = 0 + + @property + def dupe_factor(self) -> float: + return self.dupe_files / self.matched_files if self.matched_files else 0.0 + + +def run_md5_backfill( + dry_run: bool = True, + source_keys: Optional[list[str]] = None, + batch_size: int = DEFAULT_BULK_UPDATE_BATCH_SIZE, +) -> BackfillResult: + """ + The actual re-walk + reconcile logic (module docstring), matching this codebase's own "keep + Command.handle() thin" convention (e.g. `reparse_collector_evidence.reparse_and_retract`). + + `source_keys`, when given, restricts the walk to those sources only (still GOOGLE_DRIVE-only + - a LOCAL_FILE key passed here is silently a no-op, same as an unfiltered run, since it's + filtered out by the `source_type=GOOGLE_DRIVE` queryset below regardless) - useful for a + targeted re-run or a test, never required for the full-catalog case. + + Global (cross-source) checksum grouping for `dupe_groups`/`dupe_files`/`dupe_factor` - see + module docstring's "DUPE-FACTOR RECONCILIATION" section for why this must NOT be scoped + per-source. + """ + result = BackfillResult(dry_run=dry_run) + checksum_counts: dict[str, int] = {} + + all_sources = Source.objects.all().order_by("key") + if source_keys is not None: + all_sources = all_sources.filter(key__in=source_keys) + + for source in all_sources: + if source.source_type != SourceTypeChoices.GOOGLE_DRIVE: + # Checksum-less source type (LOCAL_FILE today; any future non-Drive type by + # default) - reported for --dry-run visibility, never walked or treated as an error. + result.sources_skipped_no_checksum_support.append(source.key) + continue + + result.sources_scanned += 1 + walk = walk_source_checksums(source) + if not walk.reachable: + result.sources_unreachable.append(source.key) + continue + if not walk.checksums_by_identifier: + continue + + existing_by_identifier = { + card.identifier: card + for card in Card.objects.filter(source=source, identifier__in=walk.checksums_by_identifier.keys()) + } + + to_write: list[Card] = [] + for identifier, entry in walk.checksums_by_identifier.items(): + card = existing_by_identifier.get(identifier) + if card is None: + # The listing carries a file id we haven't indexed as a Card yet (e.g. a + # concurrent update_database scan is still in flight) - never invented here; + # a later update_database/backfill pass picks it up once the Card row exists. + continue + + # Each field is tracked/compared independently - an entry missing one of the two + # never invents it, and never overwrites a stored value with an absent one (module + # docstring's "tracked and written INDEPENDENTLY" note). + needs_md5_write = False + if entry.md5_checksum is not None: + result.matched_files += 1 + checksum_counts[entry.md5_checksum] = checksum_counts.get(entry.md5_checksum, 0) + 1 + if card.md5_checksum != entry.md5_checksum: + result.md5_planned_writes += 1 + needs_md5_write = True + + needs_sha256_write = False + if entry.sha256_checksum is not None: + result.sha256_matched_files += 1 + if card.sha256_checksum != entry.sha256_checksum: + result.sha256_planned_writes += 1 + needs_sha256_write = True + + if needs_md5_write or needs_sha256_write: + result.planned_writes += 1 + if not dry_run: + if needs_md5_write: + card.md5_checksum = entry.md5_checksum + if needs_sha256_write: + card.sha256_checksum = entry.sha256_checksum + to_write.append(card) + + if not dry_run and to_write: + for start in range(0, len(to_write), batch_size): + Card.objects.bulk_update( + to_write[start : start + batch_size], + ["md5_checksum", "sha256_checksum"], + batch_size=batch_size, + ) + result.written += len(to_write) + + for count in checksum_counts.values(): + if count > 1: + result.dupe_groups += 1 + result.dupe_files += count + + return result + + +__all__ = ["ChecksumEntry", "SourceWalkResult", "walk_source_checksums", "BackfillResult", "run_md5_backfill"] diff --git a/MPCAutofill/cardpicker/migrations/0084_card_checksums.py b/MPCAutofill/cardpicker/migrations/0084_card_checksums.py new file mode 100644 index 000000000..3e8fc6f1d --- /dev/null +++ b/MPCAutofill/cardpicker/migrations/0084_card_checksums.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.30 on 2026-07-25 18:33 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("cardpicker", "0083_stageesweepcursor_keyed"), + ] + + operations = [ + migrations.AddField( + model_name="card", + name="md5_checksum", + field=models.CharField(blank=True, db_index=True, max_length=32, null=True), + ), + migrations.AddField( + model_name="card", + name="sha256_checksum", + field=models.CharField(blank=True, db_index=True, max_length=64, null=True), + ), + ] diff --git a/MPCAutofill/cardpicker/models.py b/MPCAutofill/cardpicker/models.py index 2facf8d7f..0488410de 100755 --- a/MPCAutofill/cardpicker/models.py +++ b/MPCAutofill/cardpicker/models.py @@ -457,6 +457,39 @@ class Card(models.Model): # reusing 0 as a sentinel the way CanonicalCard.image_hash does (that field predates this # decision; not retrofitted here, out of scope). content_phash = models.BigIntegerField(null=True, blank=True, db_index=True) + # md5 checksum substrate (issue #473 PR-1, docs/features/catalog-completion-plan.md's + # #442-sourced "index Drive checksums" leverage) - the Google Drive API's own `md5Checksum` + # field on a file listing, copied verbatim from the same folder-listing metadata + # `transform_image_into_object` already reads (see `cardpicker.sources.api.Image. + # md5_checksum`) - never computed locally, never derived from image bytes we don't hold (the + # governing "we index, we do not store images" premise in CLAUDE.md). NULL means "no + # checksum known for this card" - either the source type doesn't carry one at all (LOCAL_FILE + # - see `LocalFile.get_all_images_inside_folder`, which never sets it) or the Drive listing + # simply hadn't been walked with checksum-awareness yet (pre-#473 cards, until + # `backfill_md5_checksums` or an ordinary re-scan through `update_database` fills it in). Per + # the owner's ruling 3 on issue #473: a NULL or otherwise-unique md5 is a "group of one" - + # every future group-level pooling change (PR-2/PR-3) must be a provable no-op for that + # degenerate case, so this field is NEVER invented/guessed when the listing doesn't supply + # one. Deliberately a plain string (Drive's own hex-digest format, not re-encoded) rather than + # a BigIntegerField like `content_phash`/`CanonicalCard.image_hash` - md5 is an opaque + # cross-source identity key here, not a distance-comparable perceptual hash, so there's no + # reason to pay the twos-complement int-packing cost those two fields exist for. + md5_checksum = models.CharField(max_length=32, null=True, blank=True, db_index=True) + # sha256 checksum (owner-approved addition, 2026-07-25 evening, issue #473 PR-1's comment + # thread) - same listing walk, same seam, same "copied verbatim, never computed locally" + # rule as md5_checksum above. Exists for one binding reason, not as a second copy of the same + # idea: PR-2's evidence-transfer premise ("identical bytes => identical evidence") has to be + # cryptographic, not merely probabilistic - md5 collisions are constructible, so a transfer + # gated on md5 alone would be forgeable. The BINDING consequence (stated in that same comment, + # cited here so it isn't re-derived): whenever BOTH cards in a transfer have a sha256 on file, + # transfer requires md5 AND sha256 to match; an md5 match with a sha256 mismatch is a loud + # anomaly (log + skip + flag), never a silent fallback to md5-only. Groups still key on md5 + # ONLY (ruling 1 on issue #473 predates this addition and is unchanged by it) - sha256 is the + # transfer safety pairing and the future federation join key (issue #451 item 5), not a second + # grouping axis. NULL for exactly the same reasons md5_checksum can be NULL (LOCAL_FILE + # sources, or a Drive listing walked before this field existed) - never invented, never + # backfilled from image bytes we don't hold. + sha256_checksum = models.CharField(max_length=64, null=True, blank=True, db_index=True) def __str__(self) -> str: return ( diff --git a/MPCAutofill/cardpicker/sources/api.py b/MPCAutofill/cardpicker/sources/api.py index 7995a6bca..085f87289 100644 --- a/MPCAutofill/cardpicker/sources/api.py +++ b/MPCAutofill/cardpicker/sources/api.py @@ -78,6 +78,20 @@ class Image: modified_time: dt.datetime height: int folder: Folder + # issue #473 PR-1: the source's own listing checksum for this file, when the listing carries + # one at all (Google Drive's `md5Checksum` field - see `GoogleDrive.get_all_images_inside_ + # folder`). Optional with a `None` default so every existing positional/keyword call site + # that predates this field keeps working unchanged; `LocalFile.get_all_images_inside_folder` + # never sets this (no Drive-side checksum exists for a local file), which is the intended + # "stays null" behaviour for that source type per the issue's owner ruling. + md5_checksum: Optional[str] = None + # owner-approved addition, 2026-07-25 evening (issue #473 PR-1 comment thread): the same + # listing's `sha256Checksum` field, when the listing carries one - same "None default, every + # pre-existing call site keeps working, LocalFile never sets it" shape as md5_checksum above. + # See `Card.sha256_checksum`'s own docstring in `cardpicker.models` for why this exists + # alongside md5 rather than replacing it (the binding md5+sha256 evidence-transfer pairing + # rule for PR-2). + sha256_checksum: Optional[str] = None def unpack_name( self, tags: Tags diff --git a/MPCAutofill/cardpicker/sources/source_types.py b/MPCAutofill/cardpicker/sources/source_types.py index fb9c88938..b571b1022 100644 --- a/MPCAutofill/cardpicker/sources/source_types.py +++ b/MPCAutofill/cardpicker/sources/source_types.py @@ -147,7 +147,8 @@ def get_all_images_inside_folder(folder: Folder) -> list[Image]: "mimeType contains 'image/jpeg') and " f"'{folder.id}' in parents", fields="nextPageToken, files(" - "id, name, trashed, size, parents, createdTime, modifiedTime, imageMediaMetadata" + "id, name, trashed, size, parents, createdTime, modifiedTime, imageMediaMetadata, " + "md5Checksum, sha256Checksum" ")", pageSize=500, pageToken=page_token, @@ -168,6 +169,16 @@ def get_all_images_inside_folder(folder: Folder) -> list[Image]: folder=folder, height=item["imageMediaMetadata"]["height"], size=int(item["size"]), + # issue #473 PR-1 - `.get`, not `[...]`, since the Drive API's own + # docs don't guarantee md5Checksum is present for every file (e.g. a + # Google Docs Editors file has none - shouldn't occur for genuine + # image/png|jpg|jpeg mimeTypes, but never assumed). + md5_checksum=item.get("md5Checksum"), + # owner-approved addition, 2026-07-25 evening - same null-tolerance as + # md5Checksum above; Drive's sha256Checksum is even less consistently + # populated than md5Checksum for older files, so `.get` is load-bearing + # here, not just defensive style. + sha256_checksum=item.get("sha256Checksum"), ) ) diff --git a/MPCAutofill/cardpicker/sources/update_database.py b/MPCAutofill/cardpicker/sources/update_database.py index 8680dd933..53ad71147 100644 --- a/MPCAutofill/cardpicker/sources/update_database.py +++ b/MPCAutofill/cardpicker/sources/update_database.py @@ -116,6 +116,13 @@ def transform_image_into_object(source: Source, image: Image, tags: Tags) -> Car canonical_card_id=canonical_card_pk, canonical_artist_id=canonical_artist_pk, expansion_hint=expansion_hint or "", + # issue #473 PR-1: copied verbatim from the listing (image.md5_checksum is None for + # LOCAL_FILE sources and any Drive file whose listing genuinely omitted it - never + # invented here, matching Card.md5_checksum's own docstring). + md5_checksum=image.md5_checksum, + # owner-approved addition, 2026-07-25 evening - same seam, same never-invented rule; see + # Card.sha256_checksum's own docstring for why this exists alongside md5. + sha256_checksum=image.sha256_checksum, # content_phash deliberately left unset (defaults to None/NULL - "not yet computed") - # populated by hash_newly_created_cards below, for CREATED cards only. This function # builds an in-memory, not-yet-persisted Card from folder-listing metadata only (no @@ -250,6 +257,16 @@ def bulk_sync_objects(source: Source, cards: list[Card]) -> None: | (incoming[identifier].canonical_artist_id != existing[identifier].canonical_artist_id) # or if the expansion hint has changed. | (incoming[identifier].expansion_hint != existing[identifier].expansion_hint) + # or if the md5 checksum has changed (issue #473 PR-1) - unlike content_phash, this + # is free metadata already present on `incoming[identifier]` from the same listing + # this whole sync already fetched (no extra network call needed), so an ordinary + # re-scan is allowed to both backfill a previously-NULL checksum and catch the rare + # same-identifier content replacement case (content_phash's own docstring notes Drive + # normally avoids this - a real content replacement usually gets a new file id + # instead - but this is a free check either way). + | (incoming[identifier].md5_checksum != existing[identifier].md5_checksum) + # or if the sha256 checksum has changed - same free-metadata reasoning as md5 above. + | (incoming[identifier].sha256_checksum != existing[identifier].sha256_checksum) ): # record an update for this card incoming[identifier].pk = existing[identifier].pk # this must be explicitly set for bulk_update. @@ -285,6 +302,8 @@ def bulk_sync_objects(source: Source, cards: list[Card]) -> None: "canonical_card", "canonical_artist", "expansion_hint", + "md5_checksum", + "sha256_checksum", ], batch_size=1000, ) diff --git a/MPCAutofill/cardpicker/tests/test_md5_backfill.py b/MPCAutofill/cardpicker/tests/test_md5_backfill.py new file mode 100644 index 000000000..59bf19bb6 --- /dev/null +++ b/MPCAutofill/cardpicker/tests/test_md5_backfill.py @@ -0,0 +1,349 @@ +import datetime as dt +from typing import Optional + +import pytest + +from django.core.management import CommandError, call_command + +from cardpicker import md5_backfill +from cardpicker.models import Card, PilotRunLedger +from cardpicker.sources.api import Folder, Image +from cardpicker.sources.source_types import GoogleDrive, SourceTypeChoices +from cardpicker.tests import factories + +DEFAULT_DATE = dt.datetime(2023, 1, 1) + + +def _image(identifier: str, md5_checksum: Optional[str] = None, sha256_checksum: Optional[str] = None) -> Image: + return Image( + id=identifier, + name=f"{identifier}.png", + size=1, + created_time=DEFAULT_DATE, + modified_time=DEFAULT_DATE, + height=1110, + folder=Folder(id="root", name="root", parent=None), + md5_checksum=md5_checksum, + sha256_checksum=sha256_checksum, + ) + + +def _stub_reachable_folders(monkeypatch, unreachable_keys=frozenset()): + """Every source resolves to a root Folder, except any key in `unreachable_keys` (which + resolves to None, matching `SourceType.get_all_folders`'s own "dead source" contract) - a + plain, real Drive API call would otherwise be required here.""" + + def fake_get_all_folders(sources): + return { + source.key: (None if source.key in unreachable_keys else Folder(id="root", name="root", parent=None)) + for source in sources + } + + monkeypatch.setattr(GoogleDrive, "get_all_folders", staticmethod(fake_get_all_folders)) + + +def _stub_listing(monkeypatch, listings_by_source_key: dict): + """`explore_folder` is the one Drive-facing call `walk_source_checksums` makes after + resolving a root folder - stubbing it here (rather than the underlying + `GoogleDrive.get_all_images_inside_folder`/`execute_google_drive_api_call`) matches this + codebase's own established pattern for isolating one source's scan without a live Drive + connection (see `test_sources.TestUpdateDatabase.test_one_source_failure_does_not_abort_the_others`, + which stubs `transform_images_into_objects` the same way).""" + + def fake_explore_folder(source, source_type, root_folder): + return listings_by_source_key.get(source.key, []) + + monkeypatch.setattr(md5_backfill, "explore_folder", fake_explore_folder) + + +class TestWalkSourceChecksums: + def test_unreachable_source_reports_unreachable(self, db, monkeypatch): + source = factories.SourceFactory() + _stub_reachable_folders(monkeypatch, unreachable_keys={source.key}) + + result = md5_backfill.walk_source_checksums(source) + + assert result.reachable is False + assert result.checksums_by_identifier == {} + + def test_reachable_source_collects_checksums_and_skips_entries_with_neither_field(self, db, monkeypatch): + source = factories.SourceFactory() + _stub_reachable_folders(monkeypatch) + _stub_listing( + monkeypatch, + { + source.key: [ + _image("a", md5_checksum="checksum_a", sha256_checksum="sha_a"), + _image("b"), # e.g. a listing entry with neither checksum at all + ] + }, + ) + + result = md5_backfill.walk_source_checksums(source) + + assert result.reachable is True + assert result.checksums_by_identifier == { + "a": md5_backfill.ChecksumEntry(md5_checksum="checksum_a", sha256_checksum="sha_a") + } + + def test_entry_with_only_sha256_is_kept(self, db, monkeypatch): + """A listing entry can carry a sha256Checksum without an md5Checksum (or vice versa) - + the two fields are tracked independently, so this must never be dropped just because one + of the two is absent.""" + source = factories.SourceFactory() + _stub_reachable_folders(monkeypatch) + _stub_listing(monkeypatch, {source.key: [_image("a", md5_checksum=None, sha256_checksum="sha_a")]}) + + result = md5_backfill.walk_source_checksums(source) + + assert result.checksums_by_identifier == { + "a": md5_backfill.ChecksumEntry(md5_checksum=None, sha256_checksum="sha_a") + } + + +class TestRunMd5Backfill: + def test_local_file_source_is_skipped_not_walked(self, db, monkeypatch): + source = factories.SourceFactory(source_type=SourceTypeChoices.LOCAL_FILE) + factories.CardFactory(source=source, identifier="a", md5_checksum=None, sha256_checksum=None) + + def fail_if_called(*args, **kwargs): + raise AssertionError("explore_folder must never be called for a checksum-less source") + + monkeypatch.setattr(md5_backfill, "explore_folder", fail_if_called) + + result = md5_backfill.run_md5_backfill(dry_run=True) + + assert result.sources_scanned == 0 + assert result.sources_skipped_no_checksum_support == [source.key] + card = Card.objects.get(identifier="a") + assert card.md5_checksum is None + assert card.sha256_checksum is None + + def test_unreachable_source_is_reported_and_untouched(self, db, monkeypatch): + source = factories.SourceFactory() + factories.CardFactory(source=source, identifier="a", md5_checksum=None, sha256_checksum=None) + _stub_reachable_folders(monkeypatch, unreachable_keys={source.key}) + + result = md5_backfill.run_md5_backfill(dry_run=True) + + assert result.sources_scanned == 1 + assert result.sources_unreachable == [source.key] + assert result.matched_files == 0 + assert result.sha256_matched_files == 0 + card = Card.objects.get(identifier="a") + assert card.md5_checksum is None + assert card.sha256_checksum is None + + def test_dry_run_computes_but_never_writes(self, db, monkeypatch): + source = factories.SourceFactory() + card = factories.CardFactory(source=source, identifier="a", md5_checksum=None, sha256_checksum=None) + _stub_reachable_folders(monkeypatch) + _stub_listing(monkeypatch, {source.key: [_image("a", md5_checksum="freshsum", sha256_checksum="freshsha")]}) + + result = md5_backfill.run_md5_backfill(dry_run=True) + + assert result.matched_files == 1 + assert result.md5_planned_writes == 1 + assert result.sha256_matched_files == 1 + assert result.sha256_planned_writes == 1 + assert result.planned_writes == 1 # one card, both fields - counted once + assert result.written == 0 + card.refresh_from_db() + assert card.md5_checksum is None # untouched - dry run + assert card.sha256_checksum is None # untouched - dry run + + def test_write_persists_reconciled_checksums(self, db, monkeypatch): + source = factories.SourceFactory() + card = factories.CardFactory(source=source, identifier="a", md5_checksum=None, sha256_checksum=None) + _stub_reachable_folders(monkeypatch) + _stub_listing(monkeypatch, {source.key: [_image("a", md5_checksum="freshsum", sha256_checksum="freshsha")]}) + + result = md5_backfill.run_md5_backfill(dry_run=False) + + assert result.written == 1 + card.refresh_from_db() + assert card.md5_checksum == "freshsum" + assert card.sha256_checksum == "freshsha" + + def test_already_correct_checksum_is_not_a_planned_write(self, db, monkeypatch): + source = factories.SourceFactory() + factories.CardFactory(source=source, identifier="a", md5_checksum="freshsum", sha256_checksum="freshsha") + _stub_reachable_folders(monkeypatch) + _stub_listing(monkeypatch, {source.key: [_image("a", md5_checksum="freshsum", sha256_checksum="freshsha")]}) + + result = md5_backfill.run_md5_backfill(dry_run=True) + + assert result.matched_files == 1 + assert result.sha256_matched_files == 1 + assert result.md5_planned_writes == 0 + assert result.sha256_planned_writes == 0 + assert result.planned_writes == 0 + + def test_listing_entry_with_no_matching_card_is_not_invented(self, db, monkeypatch): + source = factories.SourceFactory() # no Card rows created at all + _stub_reachable_folders(monkeypatch) + _stub_listing( + monkeypatch, + {source.key: [_image("not_yet_indexed", md5_checksum="freshsum", sha256_checksum="freshsha")]}, + ) + + result = md5_backfill.run_md5_backfill(dry_run=True) + + assert result.matched_files == 0 + assert result.sha256_matched_files == 0 + assert result.planned_writes == 0 + assert Card.objects.count() == 0 + + def test_sha256_absent_from_listing_never_invents_or_nulls_it(self, db, monkeypatch): + """A listing entry can carry an md5Checksum while genuinely omitting sha256Checksum + (Drive's coverage differs between the two fields - module docstring). A card that + already has a stored sha256_checksum must come out of a reconciliation pass untouched on + that field, even while its md5_checksum is freshly written.""" + source = factories.SourceFactory() + card = factories.CardFactory( + source=source, identifier="a", md5_checksum=None, sha256_checksum="preexisting_sha" + ) + _stub_reachable_folders(monkeypatch) + _stub_listing(monkeypatch, {source.key: [_image("a", md5_checksum="freshsum", sha256_checksum=None)]}) + + result = md5_backfill.run_md5_backfill(dry_run=False) + + assert result.matched_files == 1 + assert result.sha256_matched_files == 0 # the listing entry carried no sha256 at all + assert result.written == 1 + card.refresh_from_db() + assert card.md5_checksum == "freshsum" + assert card.sha256_checksum == "preexisting_sha" # untouched, never nulled + + def test_sha256_coverage_tracked_independently_of_md5(self, db, monkeypatch): + """Per-field coverage counts must diverge when the listing's own coverage diverges - one + file carries both checksums, the other carries only md5.""" + source = factories.SourceFactory() + factories.CardFactory(source=source, identifier="a", md5_checksum=None, sha256_checksum=None) + factories.CardFactory(source=source, identifier="b", md5_checksum=None, sha256_checksum=None) + _stub_reachable_folders(monkeypatch) + _stub_listing( + monkeypatch, + { + source.key: [ + _image("a", md5_checksum="sum_a", sha256_checksum="sha_a"), + _image("b", md5_checksum="sum_b", sha256_checksum=None), + ] + }, + ) + + result = md5_backfill.run_md5_backfill(dry_run=True) + + assert result.matched_files == 2 # both entries carry md5 + assert result.sha256_matched_files == 1 # only "a" carries sha256 + + def test_dupe_group_stats_are_global_across_sources(self, db, monkeypatch): + source_1 = factories.SourceFactory() + source_2 = factories.SourceFactory() + factories.CardFactory(source=source_1, identifier="a", md5_checksum=None) + factories.CardFactory(source=source_2, identifier="b", md5_checksum=None) + factories.CardFactory(source=source_2, identifier="c", md5_checksum=None) # unique, no dupe + _stub_reachable_folders(monkeypatch) + _stub_listing( + monkeypatch, + { + source_1.key: [_image("a", md5_checksum="shared")], + source_2.key: [ + _image("b", md5_checksum="shared"), # cross-source dupe of "a" + _image("c", md5_checksum="unique"), + ], + }, + ) + + result = md5_backfill.run_md5_backfill(dry_run=True) + + assert result.matched_files == 3 + assert result.dupe_groups == 1 + assert result.dupe_files == 2 + assert result.dupe_factor == pytest.approx(2 / 3) + + def test_source_keys_filter_restricts_the_walk(self, db, monkeypatch): + source_1 = factories.SourceFactory() + source_2 = factories.SourceFactory() + factories.CardFactory(source=source_1, identifier="a", md5_checksum=None) + factories.CardFactory(source=source_2, identifier="b", md5_checksum=None) + _stub_reachable_folders(monkeypatch) + _stub_listing( + monkeypatch, + { + source_1.key: [_image("a", md5_checksum="sum_a")], + source_2.key: [_image("b", md5_checksum="sum_b")], + }, + ) + + result = md5_backfill.run_md5_backfill(dry_run=True, source_keys=[source_1.key]) + + assert result.sources_scanned == 1 + assert result.matched_files == 1 + + +class TestBackfillMd5ChecksumsCommand: + def test_write_refused_without_a_prior_matching_dry_run(self, db, monkeypatch): + source = factories.SourceFactory() + factories.CardFactory(source=source, identifier="a", md5_checksum=None) + _stub_reachable_folders(monkeypatch) + _stub_listing(monkeypatch, {source.key: [_image("a", md5_checksum="freshsum")]}) + + with pytest.raises(CommandError, match="FORCED DRY-RUN GUARD"): + call_command("backfill_md5_checksums", write=True) + + def test_write_succeeds_after_a_matching_dry_run(self, db, monkeypatch): + source = factories.SourceFactory() + card = factories.CardFactory(source=source, identifier="a", md5_checksum=None, sha256_checksum=None) + _stub_reachable_folders(monkeypatch) + _stub_listing(monkeypatch, {source.key: [_image("a", md5_checksum="freshsum", sha256_checksum="freshsha")]}) + + call_command("backfill_md5_checksums") # dry-run (default) + call_command("backfill_md5_checksums", write=True) + + card.refresh_from_db() + assert card.md5_checksum == "freshsum" + assert card.sha256_checksum == "freshsha" + ledgers = list(PilotRunLedger.objects.filter(command="backfill_md5_checksums").order_by("started_at")) + assert len(ledgers) == 2 + assert ledgers[0].dry_run is True and ledgers[0].status == PilotRunLedger.Status.COMPLETED + assert ledgers[1].dry_run is False and ledgers[1].status == PilotRunLedger.Status.COMPLETED + assert ledgers[0].counters["matched_files"] == 1 + assert ledgers[0].counters["sha256_matched_files"] == 1 + assert ledgers[0].counters["md5_planned_writes"] == 1 + assert ledgers[0].counters["sha256_planned_writes"] == 1 + assert ledgers[1].counters["written"] == 1 + + def test_write_refused_when_scope_differs_from_the_dry_run(self, db, monkeypatch): + source_1 = factories.SourceFactory() + source_2 = factories.SourceFactory() + factories.CardFactory(source=source_1, identifier="a", md5_checksum=None) + factories.CardFactory(source=source_2, identifier="b", md5_checksum=None) + _stub_reachable_folders(monkeypatch) + _stub_listing( + monkeypatch, + { + source_1.key: [_image("a", md5_checksum="sum_a")], + source_2.key: [_image("b", md5_checksum="sum_b")], + }, + ) + + call_command("backfill_md5_checksums", source_keys=[source_1.key]) # dry-run of source_1 only + + with pytest.raises(CommandError, match="FORCED DRY-RUN GUARD"): + call_command("backfill_md5_checksums", source_keys=[source_2.key], write=True) + + def test_skip_dryrun_check_bypasses_the_guard_and_is_recorded(self, db, monkeypatch, capsys): + source = factories.SourceFactory() + card = factories.CardFactory(source=source, identifier="a", md5_checksum=None) + _stub_reachable_folders(monkeypatch) + _stub_listing(monkeypatch, {source.key: [_image("a", md5_checksum="freshsum")]}) + + call_command("backfill_md5_checksums", write=True, skip_dryrun_check=True) + + printed = capsys.readouterr().out + assert "SKIP-DRYRUN-CHECK" in printed + card.refresh_from_db() + assert card.md5_checksum == "freshsum" + ledger = PilotRunLedger.objects.get(command="backfill_md5_checksums") + assert ledger.counters["skip_dryrun_check_used"] is True diff --git a/MPCAutofill/cardpicker/tests/test_sources.py b/MPCAutofill/cardpicker/tests/test_sources.py index 6578f775f..f5e3fe5d4 100644 --- a/MPCAutofill/cardpicker/tests/test_sources.py +++ b/MPCAutofill/cardpicker/tests/test_sources.py @@ -10,7 +10,11 @@ from cardpicker.models import CanonicalArtist, CanonicalCard, Card, VotePolarity from cardpicker.sources.api import Folder, Image from cardpicker.sources.source_types import SourceTypeChoices -from cardpicker.sources.update_database import bulk_sync_objects, update_database +from cardpicker.sources.update_database import ( + bulk_sync_objects, + transform_image_into_object, + update_database, +) from cardpicker.tags import Tags from cardpicker.tests import factories from cardpicker.tests.conftest import google_drive_credentials_available @@ -409,6 +413,10 @@ def test_all_sources_scanned_concurrently_local_file( names = set(Card.objects.values_list("name", flat=True)) assert names == {f"Card {i}" for i in range(len(roots))} + # issue #473 PR-1: LOCAL_FILE sources carry neither md5Checksum nor sha256Checksum at + # all - stamping at this index seam must leave every card's fields null, never invented. + assert not Card.objects.filter(md5_checksum__isnull=False).exists() + assert not Card.objects.filter(sha256_checksum__isnull=False).exists() def test_one_source_failure_does_not_abort_the_others( self, transactional_db, settings, elasticsearch, tmp_path_factory, monkeypatch @@ -596,6 +604,143 @@ def test_bulk_sync_objects_does_not_touch_content_phash_on_update(self, django_s assert existing.searchq == "renamed" # the update itself did happen assert existing.content_phash == 123 # but content_phash survived it untouched + def test_transform_image_into_object_stamps_md5_checksum_from_listing(self, django_settings): + """Issue #473 PR-1: Card.md5_checksum is copied verbatim from Image.md5_checksum at the + exact seam where a Card is first built from a source's listing - no separate fetch.""" + source = factories.SourceFactory() + image = Image( + id="x", + name="Card.png", + size=1, + created_time=DEFAULT_DATE, + modified_time=DEFAULT_DATE, + height=1110, + folder=Folder(id="f", name="F", parent=None), + md5_checksum="deadbeef", + ) + card = transform_image_into_object(source=source, image=image, tags=Tags()) + assert card.md5_checksum == "deadbeef" + + def test_transform_image_into_object_leaves_md5_checksum_null_when_listing_has_none(self, django_settings): + """Never invented - a LOCAL_FILE source's Image (or any Drive listing entry that + genuinely omits md5Checksum) has md5_checksum=None, and that's exactly what lands on the + Card too (Card.md5_checksum's own null-is-a-singleton-group contract, issue #473 ruling + 3).""" + source = factories.SourceFactory() + image = Image( + id="x", + name="Card.png", + size=1, + created_time=DEFAULT_DATE, + modified_time=DEFAULT_DATE, + height=1110, + folder=Folder(id="f", name="F", parent=None), + ) + card = transform_image_into_object(source=source, image=image, tags=Tags()) + assert card.md5_checksum is None + + def test_transform_image_into_object_stamps_sha256_checksum_from_listing(self, django_settings): + """Owner-approved addition, 2026-07-25 evening: same seam as md5 above - Card.sha256_ + checksum is copied verbatim from Image.sha256_checksum.""" + source = factories.SourceFactory() + image = Image( + id="x", + name="Card.png", + size=1, + created_time=DEFAULT_DATE, + modified_time=DEFAULT_DATE, + height=1110, + folder=Folder(id="f", name="F", parent=None), + md5_checksum="deadbeef", + sha256_checksum="cafef00d" * 8, + ) + card = transform_image_into_object(source=source, image=image, tags=Tags()) + assert card.md5_checksum == "deadbeef" + assert card.sha256_checksum == "cafef00d" * 8 + + def test_transform_image_into_object_leaves_sha256_checksum_null_when_listing_omits_it(self, django_settings): + """A listing entry may carry an md5Checksum but genuinely omit sha256Checksum (Drive's + own sha256Checksum coverage is less consistent than md5Checksum, per this module's own + docstring) - the two fields are tracked independently, so this must never invent a + sha256 value from the md5 one, or vice versa.""" + source = factories.SourceFactory() + image = Image( + id="x", + name="Card.png", + size=1, + created_time=DEFAULT_DATE, + modified_time=DEFAULT_DATE, + height=1110, + folder=Folder(id="f", name="F", parent=None), + md5_checksum="deadbeef", + # sha256_checksum intentionally omitted - defaults to None + ) + card = transform_image_into_object(source=source, image=image, tags=Tags()) + assert card.md5_checksum == "deadbeef" + assert card.sha256_checksum is None + + @freezegun.freeze_time(DEFAULT_DATE) + def test_bulk_sync_objects_stamps_md5_checksum_on_create(self, django_settings, elasticsearch): + source = factories.SourceFactory() + bulk_sync_objects( + source=source, + cards=[ + Card( + identifier="new_card", + searchq="new card", + date_created=make_aware(DEFAULT_DATE), + date_modified=make_aware(DEFAULT_DATE), + source=source, + tags=[], + size=0, + md5_checksum="deadbeef", + sha256_checksum="cafef00d" * 8, + ) + ], + ) + card = Card.objects.get(identifier="new_card") + assert card.md5_checksum == "deadbeef" + assert card.sha256_checksum == "cafef00d" * 8 + + @freezegun.freeze_time(DEFAULT_DATE) + def test_bulk_sync_objects_refreshes_md5_checksum_on_update_alone(self, django_settings, elasticsearch): + """Unlike content_phash (never touched on update - see the test immediately above this + block), md5_checksum/sha256_checksum are free metadata already present on the incoming + listing, so a re-scan refreshes both even when nothing else about the card changed at + all - this is the clause that lets an ordinary periodic re-scan backfill previously-NULL + checksums, not only the dedicated backfill_md5_checksums command.""" + source = factories.SourceFactory() + existing = factories.CardFactory( + identifier="existing_card", + searchq="mountain", + source=source, + date_created=make_aware(DEFAULT_DATE), + date_modified=make_aware(DEFAULT_DATE), + md5_checksum=None, + sha256_checksum=None, + ) + + bulk_sync_objects( + source=source, + cards=[ + Card( + identifier="existing_card", + searchq="mountain", + date_created=make_aware(DEFAULT_DATE), + date_modified=make_aware(DEFAULT_DATE), # unchanged + source=source, + tags=[], + size=0, + md5_checksum="freshsum", + sha256_checksum="freshsha" * 8, + ) + ], + ) + + existing.refresh_from_db() + assert existing.md5_checksum == "freshsum" + assert existing.sha256_checksum == "freshsha" * 8 + @freezegun.freeze_time(DEFAULT_DATE) def test_bulk_sync_objects_persists_expansion_hint_on_update(self, django_settings, elasticsearch): """ diff --git a/docs/upstreaming/extractable-primitives.md b/docs/upstreaming/extractable-primitives.md index 39530b9dc..13c3b91a4 100644 --- a/docs/upstreaming/extractable-primitives.md +++ b/docs/upstreaming/extractable-primitives.md @@ -126,6 +126,7 @@ coupling to the vote system is. | Deterministic snapshot-test sequencing (factory-sequence pinning), 2026-07-23 | `MPCAutofill/cardpicker/tests/test_views.py` (`_pin_shared_factory_sequences`) | `factory_boy` `Sequence` counters are process-global for a whole pytest run; a snapshot assertion embedding a sequence-derived value (e.g. an autogenerated `"Artist N"` name) implicitly depends on total call count up to that point in collection order. Rather than every _other_ test module that merely uses the shared factories protecting the one module that asserts on their exact values (the old, repeatedly-forgotten convention — see `docs/troubleshooting.md`'s "5-6 unrelated test snapshots break" entry), the snapshot-owning module pins the shared factories to a fixed baseline (`Factory.reset_sequence(0, force=True)`) before every one of its own tests, making its output self-determined regardless of suite composition, collection order, or how many other tests ran first | upstream, proxies-at-home (any `factory_boy` + snapshot-testing pairing, or any suite with process-global ID/name generators) | CLEAN (pattern-level — the technique itself imports nothing fork-specific; its current call site is `test_views.py`, which does assert some fork-only fields elsewhere in the same file, so this is a pattern to replicate in a fresh file, not a file to lift wholesale — see note) | — | | Postgres advisory-lock concurrency cap, 2026-07-24 (connection-lifecycle fix 2026-07-25) | `MPCAutofill/cardpicker/stage_e_concurrency.py` (`try_acquire_dispatch_slot`, `_try_acquire_slot`, `_release_slot`, `_open_dedicated_connection`) | Caps how many callers run a given code path concurrently, across any number of separate OS processes sharing one Postgres database, via `pg_try_advisory_lock`/`pg_advisory_unlock` slot cycling on a DEDICATED `psycopg2` connection the module opens and closes for itself each call (2026-07-25: the original version held its lock on `django.db.connection` and lost it in production when django-q2's ORM broker closed that shared connection mid-dispatch - see `docs/troubleshooting.md`'s Stage E entry) - no new migration for the lock mechanism itself, no cache/broker dependency, and crash-safe for free (a killed process's session-scoped lock auto-releases, unlike a DB-row counter) | upstream, proxies-at-home, federation peers (any multi-process Django+Postgres app needing a cross-process concurrency cap) | CLEAN (zero `cardpicker.*` imports at all - only `django.conf.settings`/`django.db.connection` (connection PARAMETERS only, never held on it), `psycopg2`, and stdlib; its own module docstring is itself the generic write-up of the technique and the incident that shaped it) | — | | Keyed persistent sweep cursor + bounded chunk-walk, 2026-07-25 (issues #458/#460, PR #461) | `MPCAutofill/cardpicker/models.py` (`StageESweepCursor`), `MPCAutofill/cardpicker/stage_e_dispatch.py` (`_cursor_chunk_walk`) | Walks an arbitrarily large table's pk space incrementally in bounded `STAGE_E_SELECTION_CHUNK_SIZE`-sized chunks, up to `STAGE_E_SELECTION_SCAN_CAP` candidates examined per call, each chunk claimed via an optimistic CAS UPDATE (`StageESweepCursor.try_advance`) before a caller-supplied `verify_chunk` callable checks it - two concurrent walkers of the SAME keyed cursor therefore sweep disjoint ranges instead of duplicating verification work or racing each other's cursor writes, with no dedicated lock/transaction needed. Reaching the end of the pk space wraps `position` to `0`, increments `wrap_count`, and stops the CALL rather than resuming from `0` in the same call, bounding worst-case cost. Replaces a per-call full-table anti-join that was O(catalog) and re-run from scratch on every call regardless of batch size (issue #458: 641s+-running queries; issue #460: a second, previously-masked instance one layer up that wedged the sweep 15+ min before its first ledger row) | upstream, proxies-at-home, federation peers (any Django+Postgres app needing bounded, resumable, concurrency-safe eligibility walking over a large/growing table, independent of what "eligible" means), if de-entangled from `models.py` | entangled-with-vote-consensus (colocation) - `StageESweepCursor`'s own definition (`position`/`wrap_count`/`name` fields plus its CAS classmethods) and `_cursor_chunk_walk`'s own body (touches only `Card`, `StageESweepCursor`, `django.conf.settings`) are both independently generic - verified by grep, neither references anything on the fork-only allowlist directly - but `models.py` itself is not: it imports `cardpicker.tag_consensus` and fork-only `schema_types` symbols (`Tag`, `PrintingTagStatus`) for the OTHER models it defines, so the file as a whole fails the CLEAN bar even though this one class doesn't need any of it. `stage_e_dispatch.py` passes the same grep clean on its own (it imports `local_calculate_verdicts`/`operating_envelope`/`pilot_run_lifecycle` for the dispatch loop's other concerns, none of which is on the fork-only module list) but is the SAME colocation shape as the `pilot_run_lifecycle.py` row above. Lifting this primitive means copying the two symbols into a fresh file, not importing either file wholesale | — | +| Drive-listing md5-checksum reconciliation, 2026-07-25 (issue #473 PR-1) | `MPCAutofill/cardpicker/md5_backfill.py` (`walk_source_checksums`, `run_md5_backfill`) | Re-walks a source's own folder listing (metadata only, zero image fetches) and reconciles each entry's checksum against what's currently stored on the matching row, reporting matched/planned-write/dupe-group counts - generic over "a listing carries an opaque per-file content identity key, a DB row may or may not already agree with it" | upstream, proxies-at-home (any catalog indexing from a remote listing that exposes a content checksum) | CLEAN (imports only `cardpicker.models.Card`/`Source` - neither a fork-only model symbol - plus `cardpicker.sources.source_types.SourceTypeChoices` and `cardpicker.sources.update_database.explore_folder`, neither on the fork-only module allowlist) | — | ## Docs tooling & federation