From b7b153669fca72336c59d9c8da0fa4441329b566 Mon Sep 17 00:00:00 2001 From: Julius Simonelli Date: Sun, 9 Aug 2026 22:30:19 +0200 Subject: [PATCH 1/7] Implementation plan for PR 6b: stamping alignment and extraction Co-Authored-By: Claude Fable 5 --- .../2026-08-09-import-unify-pr6b-stamping.md | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-09-import-unify-pr6b-stamping.md diff --git a/docs/superpowers/plans/2026-08-09-import-unify-pr6b-stamping.md b/docs/superpowers/plans/2026-08-09-import-unify-pr6b-stamping.md new file mode 100644 index 000000000..820d6df81 --- /dev/null +++ b/docs/superpowers/plans/2026-08-09-import-unify-pr6b-stamping.md @@ -0,0 +1,67 @@ +# Import Unification PR 6b: Stamping Alignment + Extraction Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Resolve spec decision 11 (remote `file_hash` backfill), normalize the zero-byte `verified_hash` convention at `_LandedFile` construction, unify the two stamping loops' gating and messages, then extract the last un-shared loop as `_stamp_landed_and_validate_catalog` — in that order, so the two behavior changes are individually reviewable and the extraction is a provable move-only diff. + +**Architecture:** Four production commits per the definitive 2026-08-09 stamping comparison (embedded below): (1) D4 backfill red-green — the ONE commit a reviewer can veto without unwinding anything; (2) D2/D3 construction-normalization with its single externally-visible side effect tested; (3) D1 `attests_bytes` gating + message-table unification — provably zero-flip; (4) mechanical extraction. Plus spec bookkeeping and a small PR-1444 plan-doc erratum. + +**Tech Stack:** pytest; `_dest_photo_facts` (which compares persisted `file_hash` VALUES cross-path); the zero-byte revalidation quartet as guard rails. + +--- + +## Context for a zero-context engineer + +- Repo root: `/Users/julius/conductor/workspaces/vireo/nagoya`; branch `import-unify-pr6b-stamping` (tracks origin/main at `c21d55f5`). Baseline: `python -m pytest vireo/tests/test_import_job.py -q` → **225 passed, 1 skipped**. Run from repo root; commit per task; re-grep all line refs. +- **The two loops** (`vireo/import_job.py`): remote **3501-3739** (in `_run_remote_import_job`, def 2767); local **4260-4445** (in `run_import_job`, def 3782), including local-only `_rehash_dest_or_none` at 4267-4284 (re-raises `DestReadCancelled`, returns None on OSError). Each starts at `raw_companion_invalidations = set()` with the "deliberately NOT extracted" NOTE comment, ends before `_invalidate_changed_and_sweep(` (remote 3755, local 4461). +- **Branch structure** (same decision tree, remote tests `row is not None` first, local `row is None` first): direct-row hash-match stamp; row-hash-NULL → zero-byte revalidation (#1438, both paths) / non-empty re-hash; hash-mismatch reclassify; companion branch (lookup → re-hash → accept adds `raw_companion_invalidations` + `imported_photo_ids` / mismatch reclassify); no-row reclassify. Five `DestReadCancelled` sites (remote 2, local 3), all `state.cancelled = True; break`. +- **The four divergences:** + - **D4 (decision 11):** local's non-empty NULL-hash re-hash-agree branch backfills `update_photo_hash_check(row["id"], "ok", file_hash=verified_hash, commit=False)` (L4424-4427); remote's single stamp (R3626-3629, gated `if params.verify_by_hash:`) passes no `file_hash` — the row's NULL persists until the next full scan. `db.update_photo_hash_check` with `file_hash=` really writes `photos.file_hash` (db.py:3549-3554). + - **THE BACKFILL TRAP:** remote's single stamp site covers the zero-byte accept too. `file_hash=src_hash` there would write `EMPTY_FILE_SHA256` into `photos.file_hash` — the exact collision the zero-byte convention exists to prevent (scanner.py:2271-2277 nulls it; scanner.py:2099-2103 `empty_hash_needs_repair` would then churn repairs). The backfill must be `file_hash=(src_hash if src_hash != EMPTY_FILE_SHA256 else None)` and apply ONLY on the scan-NULL re-read-agree path (the scan-hash-agrees path needs no backfill — the row already holds the hash; local doesn't backfill there either). + - **D2/D3:** `verified_hash` is `None` for zero-byte ONLY at remote sites 3440-3447 (transfer landing; `src_hash` from `checker.content_hash` → None for size-0) and 3133-3140 (adopt — actually unreachable-None, adopt gate filters it). Local always produces `EMPTY_FILE_SHA256` (copy_and_hash_verify never returns None; adopt hardcodes EMPTY at 4071). Normalize AT THOSE TWO CONSTRUCTION SITES ONLY (`verified_hash=src_hash if src_hash is not None else EMPTY_FILE_SHA256`); tighten the field to `verified_hash: str` (~584) + docstring. **Do NOT touch the `src_hash` computation at 3023-3027** — it feeds `claimed_basenames`/`queued_src_hashes`/the adopt gate/`_record_checker`, all relying on the None convention; normalizing there changes intra-batch dedup for empty files (out of scope). + - **The ONE externally-visible normalization consequence:** the diff loop in `_invalidate_changed_and_sweep` (1398-1399) compares `pre_scan_hashes[dest_path]` (NULL for zero-byte rows) against `entry.verified_hash`. Today remote zero-byte re-import over an existing row: `None == None` → no invalidation; local: `None != EMPTY` → invalidation. Normalization flips remote to local's behavior (a harmless extra invalidation + sweep for empty files). Needs its own pin test. + - **D1:** stamp gating. `attests_bytes` ≡ the existing `verified_counted_for_copies` (local `True` at 3920, remote `params.verify_by_hash` at 2938) — gating local's three stamps on it is a NO-OP (it's True); remote's gate is unchanged. `imported_photo_ids.add` must stay on EVERY accept path regardless of the gate (remote already has it outside at 3633; a unified shape that puts it inside the gate would lose `result["photo_ids"]`/the chaining hook on no-verify remote runs). + - **D5/D6/D7 messages:** unified table below, chosen to keep ALL 8 pinned reason prefixes (`"scanned mount row hash"` 5052/5937/9763, `"scan wrote no mount row hash"` 6560/9858, `"scan wrote no archive row hash"` 10029, `"paired companion mount bytes"` 5741/6659/9963) via a `dest_noun` parameter ("archive"/"mount"). The unpinned local wordings (`"destination changed between copy verification..."`, `"archive file unhashable..."`, `"paired companion archive bytes no longer match the copy-time hash"`, bare `"not cataloged after scan"`) change to the unified forms — user-visible in `unsafe_files`, list in the PR body. +- **Unified message table** (implement exactly): + 1. Direct mismatch: `f"scanned {dest_noun} row hash does not match the hash this import verified ({dest_noun} base is likely stale or misconfigured)"` + 2. NULL-hash re-read disagrees/unreadable: `f"scan wrote no {dest_noun} row hash and a re-read of the {dest_noun} file disagrees with the hash this import verified ({dest_noun} file is likely stale, unreadable, or misconfigured)"` + 3. Companion unreadable: `f"paired companion {dest_noun} bytes could not be read"` + 4. Companion disagrees: `f"paired companion {dest_noun} bytes do not match the hash this import verified ({dest_noun} base is likely stale or misconfigured)"` + 5. No row: `"not cataloged after scan (no photo row)"` +- **Test landscape:** `_dest_photo_facts` (test ~9304) captures `(rel, filename, file_hash, hash_status)` — file_hash VALUES compared cross-path in the parity suite. The no-verify remote NULL pins (4474-4517 etc.) and local `verify_by_hash=False` 'ok' pin (3434) constrain the gating exactly as `verified_counted_for_copies` provides. NO existing test reaches the remote non-empty-NULL-re-hash-agree branch and inspects `file_hash` — D4 needs a new red-green pair + a zero-byte guard pair (`file_hash IS NULL` after zero-byte accept, both paths — protects against the backfill trap). +- Also in this PR: a docs-only erratum to `docs/superpowers/plans/2026-08-08-import-unify-pr6-phase-extraction.md` answering the two CodeRabbit comments on #1444 (goal says "six" commits, seven landed; Task-6 sketch signatures differ from as-built — add a short "As built" note rather than rewriting history). + +### Task 0: Sanity +- [ ] Branch check; baseline 225 passed, 1 skipped. + +### Task 1: Docs — spec decision 11 resolution + PR-1444 plan erratum +- [ ] Spec (`docs/superpowers/specs/2026-08-06-import-path-unification-design.md`) decision-table row 11: mark **RESOLVED — adopt local, with the zero-byte exclusion** (backfill only non-EMPTY hashes, only on the scan-NULL path; dated note). Update the PR-sequence 6b entry to this plan's four-commit shape. +- [ ] PR-6 plan erratum: append a short "## As built (erratum, 2026-08-09)" section to `2026-08-08-import-unify-pr6-phase-extraction.md`: seven commits (goal line said six); as-built signatures differ from the Task-6 sketch (`_rollback_on_mount_loss` takes no `rel` — reads `batch_st.rel`; `_batch_preflight` returns dest_folder and takes `missing_root_check`); pointer to the merged code as truth. (Answers CodeRabbit's two comments on #1444.) +- [ ] Commit: `"Spec: resolve decision 11 (backfill with zero-byte exclusion); PR-6 plan as-built erratum"` + +### Task 2: D4 — remote file_hash backfill (red-green; THE vetoable commit) +- [ ] **Tests first (4):** + 1. `test_remote_import_backfills_file_hash_when_scan_left_null` — RED: remote import of a non-empty file where `scan()` leaves the row's `file_hash` NULL (mirror the existing `test_remote_import_null_scan_hash_*` setup at ~6480 — read how it forces the NULL) with `verify_by_hash=True`; after import assert the row's `file_hash == compute_file_hash(mount_file)` (currently NULL) and `hash_status == "ok"`. + 2. `test_local_import_backfills_file_hash_when_scan_left_null` — expected GREEN (characterizes L4424-4427); same geometry, archive-side. + 3. + 4. `test_{remote,local}_zero_byte_accept_leaves_file_hash_null` — the guard pair: zero-byte accept (use the #1438 quartet's geometry WITHOUT the deletion step) → row `file_hash IS NULL`, `hash_status` per gating (remote verify on → "ok"; keep verify_by_hash=True both). Expected GREEN today; they hold the line against the backfill trap forever. +- [ ] **Implement:** remote stamp site (R3626-3629) becomes, on the scan-NULL-re-read-agree path only, `db.update_photo_hash_check(row["id"], "ok", file_hash=(src_hash if src_hash != EMPTY_FILE_SHA256 else None), commit=False)` — NOTE the remote loop currently has ONE stamp shared by scan-agrees AND scan-NULL paths; splitting the stamp per-path (mirroring local's three-site shape) is the cleanest implementation and pre-aligns the shape for Task 4. `db.update_photo_hash_check(..., file_hash=None)` hits the no-backfill arm (verify in db.py:3543-3560 — if `file_hash=None` is indistinguishable from omitting it, call without the kwarg on the zero-byte path instead). +- [ ] Full file: 225+4 → 229 passed, 1 skipped. Commit: `"Remote import backfills file_hash when scan leaves it NULL (spec decision 11)"` +- [ ] **Preflight-mirror check (spec checklist):** no dup-walk changes — n/a; say so in the PR body. + +### Task 3: D2/D3 — normalize verified_hash at construction +- [ ] Test first: `test_remote_zero_byte_reimport_invalidates_derived_caches` — RED: remote zero-byte file re-imported over an existing zero-byte row (row pre-exists → `pre_scan_hashes` holds NULL) with `vireo_dir` set and a spy on `_invalidate_derived_caches`; assert invalidation fires (today `None == None` skips it; post-normalization `None != EMPTY` fires, matching local). Add the local mirror (expected GREEN). +- [ ] Implement: the two remote construction sites (3440-3447, 3133-3140) get `verified_hash=src_hash if src_hash is not None else EMPTY_FILE_SHA256` (comment: checker.content_hash returns None for size-0; the ledger convention is EMPTY, scan's row convention is NULL — normalization here, once, instead of in every consumer; do NOT normalize src_hash itself, see the intra-batch dedup constraint). Tighten `_LandedFile.verified_hash: str` + docstring. Then simplify the remote loop's `src_h_norm` computations (3547-3550, 3696-3699) — they still normalize EMPTY→None for row comparison and stay, but the input is now never-None; adjust comments. +- [ ] Full file → 231 passed, 1 skipped; run the zero-byte quartet + Task 2's guard pair by name. Commit: `"Normalize zero-byte verified_hash at _LandedFile construction (spec D2/D3)"` + +### Task 4: D1 + messages — make the loops textually identical (zero-flip) +- [ ] Local: wrap all three stamps in `if verified_counted_for_copies:` (no-op — it's True; ensure `imported_photo_ids.add` stays OUTSIDE the gate on every accept path, matching remote's 3633). Remote: hoist `imported_photo_ids.add` shape to match. Apply the unified message table with a `dest_noun` local ("archive"/"mount") in each function. Restructure both loops to ONE shared shape (local's positive-test order; `_rehash_dest_or_none` used by both — hoist it to module level taking `(path, stop_requested)` now; remote's inline read_failed collapses to the helper's None-means-unreadable). At the end of this task the two loop bodies must be TEXTUALLY IDENTICAL modulo `dest_noun` value — verify with the stripped-diff method and paste the empty diff into the commit message. +- [ ] Full file → 231 passed, 1 skipped (ZERO flips — any red means the gating or a message pin broke; the 8 pinned prefixes must survive). Commit: `"Unify stamping gating and messages; loops now textually identical (spec D1/D5-D7)"` +- [ ] Reason-string changes (unpinned local wordings) listed for the PR body. + +### Task 5: Extraction (move-only) +- [ ] Add `_stamp_landed_and_validate_catalog(state, batch_st, db, params, rel, *, attests_bytes, dest_noun, stop_requested)` returning `raw_companion_invalidations`; move the (now-identical) loop body in; both functions call it (`dest_noun="mount"`/`"archive"`, `attests_bytes=verified_counted_for_copies`, `stop_requested=_stop_requested`) and pass the result to `_invalidate_changed_and_sweep` unchanged. Delete both loop bodies + the NOTE comments (their job is done). The docstring carries the gate/imported_photo_ids invariants from this plan's Context. +- [ ] Full file → 231 passed, 1 skipped; full parity net by name (`-k "behavior or agree_on_plain or adoption or renamed_twin or zero_byte or dest_read_cancel or mount_detach or pairs"`). Commit: `"Extract the unified stamping loop (last shared logic; transport cores remain)"` + +### Task 6: Verification + PR +- [ ] Full file + required CLAUDE.md suite (known env failure `test_api_exiftool_status_reports_missing` ignored). +- [ ] Push; PR: title `"Import unification PR 6b: unify and extract the stamping loop (spec decision 11)"`, base main. Body: spec/plan links; decision 11 with the zero-byte exclusion rationale + the backfill-trap explanation; the normalization's one visible flip (remote zero-byte re-import derived-cache invalidation, now matching local); the unified reason wordings (list old→new for the unpinned local strings); the zero-flip proof for Task 4 (stripped-diff empty); what this means: THE LAST shared logic is unified — both functions are now transport cores + shared calls, PR 7 is the merge; the CodeRabbit erratum; exact counts. End with the Claude Code attribution line. From d6e09a32ad59a21f8343e662d4199ee1594e6380 Mon Sep 17 00:00:00 2001 From: Julius Simonelli Date: Sun, 9 Aug 2026 22:37:41 +0200 Subject: [PATCH 2/7] Plan fixes from review: Task-3 geometry preconditions, 'recorded' wording, prefix count, anchors Co-Authored-By: Claude Fable 5 --- .../2026-08-09-import-unify-pr6b-stamping.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/superpowers/plans/2026-08-09-import-unify-pr6b-stamping.md b/docs/superpowers/plans/2026-08-09-import-unify-pr6b-stamping.md index 820d6df81..4e3b880d8 100644 --- a/docs/superpowers/plans/2026-08-09-import-unify-pr6b-stamping.md +++ b/docs/superpowers/plans/2026-08-09-import-unify-pr6b-stamping.md @@ -13,20 +13,21 @@ ## Context for a zero-context engineer - Repo root: `/Users/julius/conductor/workspaces/vireo/nagoya`; branch `import-unify-pr6b-stamping` (tracks origin/main at `c21d55f5`). Baseline: `python -m pytest vireo/tests/test_import_job.py -q` → **225 passed, 1 skipped**. Run from repo root; commit per task; re-grep all line refs. -- **The two loops** (`vireo/import_job.py`): remote **3501-3739** (in `_run_remote_import_job`, def 2767); local **4260-4445** (in `run_import_job`, def 3782), including local-only `_rehash_dest_or_none` at 4267-4284 (re-raises `DestReadCancelled`, returns None on OSError). Each starts at `raw_companion_invalidations = set()` with the "deliberately NOT extracted" NOTE comment, ends before `_invalidate_changed_and_sweep(` (remote 3755, local 4461). +- **The two loops** (`vireo/import_job.py`): remote **3501-3739** (in `_run_remote_import_job`, def 2767); local **4260-4445** (in `run_import_job`, def 3782), including local-only `_rehash_dest_or_none` at 4267-4284 (re-raises `DestReadCancelled`, returns None on OSError). Each starts at `raw_companion_invalidations = set()` with the "deliberately NOT extracted" NOTE comment, ends before `_invalidate_changed_and_sweep(` (remote 3756, local 4461). - **Branch structure** (same decision tree, remote tests `row is not None` first, local `row is None` first): direct-row hash-match stamp; row-hash-NULL → zero-byte revalidation (#1438, both paths) / non-empty re-hash; hash-mismatch reclassify; companion branch (lookup → re-hash → accept adds `raw_companion_invalidations` + `imported_photo_ids` / mismatch reclassify); no-row reclassify. Five `DestReadCancelled` sites (remote 2, local 3), all `state.cancelled = True; break`. - **The four divergences:** - - **D4 (decision 11):** local's non-empty NULL-hash re-hash-agree branch backfills `update_photo_hash_check(row["id"], "ok", file_hash=verified_hash, commit=False)` (L4424-4427); remote's single stamp (R3626-3629, gated `if params.verify_by_hash:`) passes no `file_hash` — the row's NULL persists until the next full scan. `db.update_photo_hash_check` with `file_hash=` really writes `photos.file_hash` (db.py:3549-3554). + - **D4 (decision 11):** local's non-empty NULL-hash re-hash-agree branch backfills `update_photo_hash_check(row["id"], "ok", file_hash=verified_hash, commit=False)` (L4423-4427); remote's single stamp (R3626-3629, gated `if params.verify_by_hash:`) passes no `file_hash` — the row's NULL persists until the next full scan. `db.update_photo_hash_check` with `file_hash=` really writes `photos.file_hash` (db.py:3549-3554). - **THE BACKFILL TRAP:** remote's single stamp site covers the zero-byte accept too. `file_hash=src_hash` there would write `EMPTY_FILE_SHA256` into `photos.file_hash` — the exact collision the zero-byte convention exists to prevent (scanner.py:2271-2277 nulls it; scanner.py:2099-2103 `empty_hash_needs_repair` would then churn repairs). The backfill must be `file_hash=(src_hash if src_hash != EMPTY_FILE_SHA256 else None)` and apply ONLY on the scan-NULL re-read-agree path (the scan-hash-agrees path needs no backfill — the row already holds the hash; local doesn't backfill there either). - **D2/D3:** `verified_hash` is `None` for zero-byte ONLY at remote sites 3440-3447 (transfer landing; `src_hash` from `checker.content_hash` → None for size-0) and 3133-3140 (adopt — actually unreachable-None, adopt gate filters it). Local always produces `EMPTY_FILE_SHA256` (copy_and_hash_verify never returns None; adopt hardcodes EMPTY at 4071). Normalize AT THOSE TWO CONSTRUCTION SITES ONLY (`verified_hash=src_hash if src_hash is not None else EMPTY_FILE_SHA256`); tighten the field to `verified_hash: str` (~584) + docstring. **Do NOT touch the `src_hash` computation at 3023-3027** — it feeds `claimed_basenames`/`queued_src_hashes`/the adopt gate/`_record_checker`, all relying on the None convention; normalizing there changes intra-batch dedup for empty files (out of scope). - **The ONE externally-visible normalization consequence:** the diff loop in `_invalidate_changed_and_sweep` (1398-1399) compares `pre_scan_hashes[dest_path]` (NULL for zero-byte rows) against `entry.verified_hash`. Today remote zero-byte re-import over an existing row: `None == None` → no invalidation; local: `None != EMPTY` → invalidation. Normalization flips remote to local's behavior (a harmless extra invalidation + sweep for empty files). Needs its own pin test. - **D1:** stamp gating. `attests_bytes` ≡ the existing `verified_counted_for_copies` (local `True` at 3920, remote `params.verify_by_hash` at 2938) — gating local's three stamps on it is a NO-OP (it's True); remote's gate is unchanged. `imported_photo_ids.add` must stay on EVERY accept path regardless of the gate (remote already has it outside at 3633; a unified shape that puts it inside the gate would lose `result["photo_ids"]`/the chaining hook on no-verify remote runs). - - **D5/D6/D7 messages:** unified table below, chosen to keep ALL 8 pinned reason prefixes (`"scanned mount row hash"` 5052/5937/9763, `"scan wrote no mount row hash"` 6560/9858, `"scan wrote no archive row hash"` 10029, `"paired companion mount bytes"` 5741/6659/9963) via a `dest_noun` parameter ("archive"/"mount"). The unpinned local wordings (`"destination changed between copy verification..."`, `"archive file unhashable..."`, `"paired companion archive bytes no longer match the copy-time hash"`, bare `"not cataloged after scan"`) change to the unified forms — user-visible in `unsafe_files`, list in the PR body. + - **D5/D6/D7 messages:** unified table below, chosen to keep all 4 pinned reason prefixes (asserted at 9 test sites) (`"scanned mount row hash"` 5052/5937/9763, `"scan wrote no mount row hash"` 6560/9858, `"scan wrote no archive row hash"` 10029, `"paired companion mount bytes"` 5741/6659/9963) via a `dest_noun` parameter ("archive"/"mount"); the PR body must list BOTH the unpinned local wording changes AND the remote verify/no-verify tail collapse. The unpinned local wordings (`"destination changed between copy verification..."`, `"archive file unhashable..."`, `"paired companion archive bytes no longer match the copy-time hash"`, bare `"not cataloged after scan"`) change to the unified forms — user-visible in `unsafe_files`, list in the PR body. - **Unified message table** (implement exactly): - 1. Direct mismatch: `f"scanned {dest_noun} row hash does not match the hash this import verified ({dest_noun} base is likely stale or misconfigured)"` - 2. NULL-hash re-read disagrees/unreadable: `f"scan wrote no {dest_noun} row hash and a re-read of the {dest_noun} file disagrees with the hash this import verified ({dest_noun} file is likely stale, unreadable, or misconfigured)"` + 1. Direct mismatch: `f"scanned {dest_noun} row hash does not match the hash this import recorded ({dest_noun} base is likely stale or misconfigured)"` + 2. NULL-hash re-read disagrees/unreadable: `f"scan wrote no {dest_noun} row hash and a re-read of the {dest_noun} file disagrees with the hash this import recorded ({dest_noun} file is likely stale, unreadable, or misconfigured)"` 3. Companion unreadable: `f"paired companion {dest_noun} bytes could not be read"` - 4. Companion disagrees: `f"paired companion {dest_noun} bytes do not match the hash this import verified ({dest_noun} base is likely stale or misconfigured)"` + 4. Companion disagrees: `f"paired companion {dest_noun} bytes do not match the hash this import recorded ({dest_noun} base is likely stale or misconfigured)"` + ("recorded", not "verified": a no-verify remote run computed the card-side hash but verified nothing destination-side — the wording must not overclaim, per the UI-transparency rule.) 5. No row: `"not cataloged after scan (no photo row)"` - **Test landscape:** `_dest_photo_facts` (test ~9304) captures `(rel, filename, file_hash, hash_status)` — file_hash VALUES compared cross-path in the parity suite. The no-verify remote NULL pins (4474-4517 etc.) and local `verify_by_hash=False` 'ok' pin (3434) constrain the gating exactly as `verified_counted_for_copies` provides. NO existing test reaches the remote non-empty-NULL-re-hash-agree branch and inspects `file_hash` — D4 needs a new red-green pair + a zero-byte guard pair (`file_hash IS NULL` after zero-byte accept, both paths — protects against the backfill trap). - Also in this PR: a docs-only erratum to `docs/superpowers/plans/2026-08-08-import-unify-pr6-phase-extraction.md` answering the two CodeRabbit comments on #1444 (goal says "six" commits, seven landed; Task-6 sketch signatures differ from as-built — add a short "As built" note rather than rewriting history). @@ -49,13 +50,13 @@ - [ ] **Preflight-mirror check (spec checklist):** no dup-walk changes — n/a; say so in the PR body. ### Task 3: D2/D3 — normalize verified_hash at construction -- [ ] Test first: `test_remote_zero_byte_reimport_invalidates_derived_caches` — RED: remote zero-byte file re-imported over an existing zero-byte row (row pre-exists → `pre_scan_hashes` holds NULL) with `vireo_dir` set and a spy on `_invalidate_derived_caches`; assert invalidation fires (today `None == None` skips it; post-normalization `None != EMPTY` fires, matching local). Add the local mirror (expected GREEN). +- [ ] Test first: `test_remote_zero_byte_reimport_invalidates_derived_caches` — RED. LOAD-BEARING GEOMETRY (all three preconditions): (a) a zero-byte photo ROW pre-exists at the dest path (so `pre_scan_hashes` holds its NULL); (b) the mount FILE at that path is DELETED — if present, the adopt gate can't match `on_disk == None`, the collision walk renames to `_1`, the landing is a fresh insert absent from `pre_scan_hashes`, and the test never goes green; (c) `skip_duplicates=True` — the checker path is what makes `verified_hash` None today; checker-less already produces EMPTY and the test would be green pre-change, breaking red-green. Spy `_invalidate_derived_caches` with `vireo_dir` set; assert invalidation fires (today `None == None` skips; post-normalization `None != EMPTY` fires, matching local). The LOCAL mirror (expected GREEN) necessarily uses DIFFERENT geometry — file present → the zero-byte adopt lands at the same path — state that in its docstring so nobody force-fits symmetry (the known mirror-geometry trap). - [ ] Implement: the two remote construction sites (3440-3447, 3133-3140) get `verified_hash=src_hash if src_hash is not None else EMPTY_FILE_SHA256` (comment: checker.content_hash returns None for size-0; the ledger convention is EMPTY, scan's row convention is NULL — normalization here, once, instead of in every consumer; do NOT normalize src_hash itself, see the intra-batch dedup constraint). Tighten `_LandedFile.verified_hash: str` + docstring. Then simplify the remote loop's `src_h_norm` computations (3547-3550, 3696-3699) — they still normalize EMPTY→None for row comparison and stay, but the input is now never-None; adjust comments. - [ ] Full file → 231 passed, 1 skipped; run the zero-byte quartet + Task 2's guard pair by name. Commit: `"Normalize zero-byte verified_hash at _LandedFile construction (spec D2/D3)"` ### Task 4: D1 + messages — make the loops textually identical (zero-flip) - [ ] Local: wrap all three stamps in `if verified_counted_for_copies:` (no-op — it's True; ensure `imported_photo_ids.add` stays OUTSIDE the gate on every accept path, matching remote's 3633). Remote: hoist `imported_photo_ids.add` shape to match. Apply the unified message table with a `dest_noun` local ("archive"/"mount") in each function. Restructure both loops to ONE shared shape (local's positive-test order; `_rehash_dest_or_none` used by both — hoist it to module level taking `(path, stop_requested)` now; remote's inline read_failed collapses to the helper's None-means-unreadable). At the end of this task the two loop bodies must be TEXTUALLY IDENTICAL modulo `dest_noun` value — verify with the stripped-diff method and paste the empty diff into the commit message. -- [ ] Full file → 231 passed, 1 skipped (ZERO flips — any red means the gating or a message pin broke; the 8 pinned prefixes must survive). Commit: `"Unify stamping gating and messages; loops now textually identical (spec D1/D5-D7)"` +- [ ] Full file → 231 passed, 1 skipped (ZERO flips — any red means the gating or a message pin broke; the 4 pinned prefixes (9 assertion sites) must survive). Commit: `"Unify stamping gating and messages; loops now textually identical (spec D1/D5-D7)"` - [ ] Reason-string changes (unpinned local wordings) listed for the PR body. ### Task 5: Extraction (move-only) From fd8bc98224f40d3a3727c4e1d9fab1460e06a3de Mon Sep 17 00:00:00 2001 From: Julius Simonelli Date: Sun, 9 Aug 2026 22:39:55 +0200 Subject: [PATCH 3/7] Spec: resolve decision 11 (backfill with zero-byte exclusion); PR-6 plan as-built erratum Co-Authored-By: Claude Fable 5 --- ...026-08-08-import-unify-pr6-phase-extraction.md | 15 +++++++++++++++ .../2026-08-06-import-path-unification-design.md | 13 ++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-08-08-import-unify-pr6-phase-extraction.md b/docs/superpowers/plans/2026-08-08-import-unify-pr6-phase-extraction.md index c924accae..0b51be392 100644 --- a/docs/superpowers/plans/2026-08-08-import-unify-pr6-phase-extraction.md +++ b/docs/superpowers/plans/2026-08-08-import-unify-pr6-phase-extraction.md @@ -68,3 +68,18 @@ - [ ] Symtable/grep audit for the batch-state names (same discipline as 5b; recursive variant). - [ ] Full file (225/1) + required CLAUDE.md suite (2077/14/1 known env failure). - [ ] Push; PR: title `"Import unification PR 6: extract the shared import phases"`, base main. Body: spec/plan links; the fresh-map methodology (stripped-diff verification); the extraction inventory with line counts; the proven-no-op transfer-keys note; the three narration alignments; what deliberately STAYS (stamping loops → PR 6b with divergence 11; collision/adopt walk + transfer → PR 7; local-only per-file guard recorded for PR 7); suite counts identical at every commit; the audit artifacts. End with the Claude Code attribution line. + +## As built (erratum, 2026-08-09) + +Two deltas between this plan and what merged in PR #1444; the merged code is +the source of truth. + +- **Seven commits, not six.** The goal line says "six suite-green commits"; + seven production commits landed — the Task 6 batch-state/preflight/rollback + work split into two commits (narration alignment landed separately from the + batch-state extraction) so each stayed individually reviewable. +- **As-built signatures differ from the Task 6 sketch:** + - `_rollback_on_mount_loss` takes no `rel` parameter — it reads + `batch_st.rel` instead. + - `_batch_preflight` returns `dest_folder` (or `None` → caller `continue`s) + and takes a `missing_root_check` parameter. diff --git a/docs/superpowers/specs/2026-08-06-import-path-unification-design.md b/docs/superpowers/specs/2026-08-06-import-path-unification-design.md index cf1ba6d1e..f5629168f 100644 --- a/docs/superpowers/specs/2026-08-06-import-path-unification-design.md +++ b/docs/superpowers/specs/2026-08-06-import-path-unification-design.md @@ -229,7 +229,7 @@ changes to match. | 8 | Local re-computes the source hash at 3996–3999 instead of reusing `_src_hash_cached()` | **No change — premise disproven (2026-08-08, PR 3).** `DuplicateChecker.content_hash` memoizes per source path (`import_dedup.py:319-327`), so with a checker the copy-site call is a cache hit whenever a hash was computed earlier in the run — and otherwise performs a read `copy_and_hash_verify` would do itself anyway (its `src_hash is None` branch runs a standalone `compute_file_hash(src)`). With no checker, reusing `_src_hash_cached()` is read-neutral: the standalone read merely moves, with a marginal saving only on the rare collision-walk path. No redundant I/O exists; the call-site duplication itself dissolves in PR 5's shared cached-hash closure. | | 9 | Remote rollback open-coded at 8 sites vs local `_reclassify_landed_failed` | **Structural — shared helper on `_ImportRunState`** (PR 5). | | 10 | Adopted (crash-recovery) files get `hash_status='ok'` stamped locally but stay `NULL` remotely — found empirically by PR 1's parity net (2026-08-07): local adoption folds into `landed` and hits the verify stamp; remote adoption lives in `adopted_paths`, whose validation cross-checks bytes but never stamps | **Adopt local, via the PR 5 structural change.** Folding remote adoptions into `landed` with their verified hash makes the stamp fall out of the unified catalog pass; no separate fix PR. Pinned per-path by `test_{local,remote}_adoption_uncataloged_dest_twin_current_behavior`, which flip when PR 5 lands. | -| 11 | Local stamping loop backfills `file_hash` on scan-NULL rows (`update_photo_hash_check(..., "ok", file_hash=verified_hash)`, ~L4511–4514 — NOT the non-backfilling stamp at L4480–4482 just above it, which handles the zero-byte case); remote stamps `"ok"` without backfilling — found by the 2026-08-08 extraction phase map (D4). Related, same loop: a zero-byte normalization-convention split (D2/D3) — remote normalizes `EMPTY_FILE_SHA256` → `None` at hash time with a `read_failed` flag; local compares raw hashes and its `verified_hash` is never `None`. | **Deferred to the stamping align-then-extract PR (6b): adopt local (backfill on both paths) unless review finds a reason the remote's NULL is load-bearing; until then the stamping loops stay per-function.** For D2/D3, unify by normalizing at `_LandedFile` construction in 6b so both loops see the same convention. | +| 11 | Local stamping loop backfills `file_hash` on scan-NULL rows (`update_photo_hash_check(..., "ok", file_hash=verified_hash)`, ~L4511–4514 — NOT the non-backfilling stamp at L4480–4482 just above it, which handles the zero-byte case); remote stamps `"ok"` without backfilling — found by the 2026-08-08 extraction phase map (D4). Related, same loop: a zero-byte normalization-convention split (D2/D3) — remote normalizes `EMPTY_FILE_SHA256` → `None` at hash time with a `read_failed` flag; local compares raw hashes and its `verified_hash` is never `None`. | **RESOLVED 2026-08-09 (PR 6b): adopt local, with the zero-byte exclusion.** Remote backfills `file_hash` only on the scan-NULL re-read-agree path, and only with non-`EMPTY_FILE_SHA256` hashes: `file_hash=(src_hash if src_hash != EMPTY_FILE_SHA256 else None)`. Backfilling `EMPTY_FILE_SHA256` would recreate the collision the scanner's zero-byte NULL convention exists to prevent (scanner nulls empty-file hashes; `empty_hash_needs_repair` would churn repairs). The scan-hash-agrees path needs no backfill — the row already holds the hash (local doesn't backfill there either). For D2/D3, unify by normalizing at `_LandedFile` construction in 6b so both loops see the same convention. | Kept as deliberate (transport-required) differences, expressed through the protocol rather than duplicated code: transfer sub-progress @@ -313,6 +313,17 @@ and goes through the normal PR-agent review cycle. **PR 6b — stamping alignment + extraction (behavior PR: D4, D2/D3)** between PR 6 and PR 7: align per decision 11 first, then extract the now-identical loop.* + *Update 2026-08-09: PR 6b lands as four production commits, each + individually reviewable: (1) D4 red-green — remote backfills `file_hash` + on the scan-NULL re-read-agree path with the zero-byte exclusion + (decision 11, now RESOLVED); (2) D2/D3 — normalize zero-byte + `verified_hash` to `EMPTY_FILE_SHA256` at `_LandedFile` construction, + with the one externally-visible flip (remote zero-byte re-import now + invalidates derived caches, matching local) pinned by its own test; + (3) D1 + messages — gate stamps on `attests_bytes` + (≡ `verified_counted_for_copies`, zero-flip) and unify the reason + strings via a `dest_noun` parameter; (4) mechanical extraction of the + now-textually-identical loop as `_stamp_landed_and_validate_catalog`.* 7. **PR 7 — the merge.** Introduce `_Transport`, `LocalTransport`, `RsyncTransport`; one orchestrator batch loop; delete `_run_remote_import_job`; repoint the remote tests' monkeypatch seam at From 84fbea8d2dac1e38347af7234f05e492e96384a3 Mon Sep 17 00:00:00 2001 From: Julius Simonelli Date: Sun, 9 Aug 2026 22:43:24 +0200 Subject: [PATCH 4/7] Remote import backfills file_hash when scan leaves it NULL (spec decision 11) Co-Authored-By: Claude Fable 5 --- vireo/import_job.py | 48 ++++++-- vireo/tests/test_import_job.py | 204 +++++++++++++++++++++++++++++++++ 2 files changed, 244 insertions(+), 8 deletions(-) diff --git a/vireo/import_job.py b/vireo/import_job.py index 0beef7323..d27684f20 100644 --- a/vireo/import_job.py +++ b/vireo/import_job.py @@ -3609,6 +3609,33 @@ def _rsync_cancelled(rc): ) batch_st.reclassified_landed_paths.add(entry.dest_path) continue + # Scan left the row's file_hash NULL and the + # mount re-read agrees with the hash this import + # recorded — backfill it so the row doesn't stay + # hashless until the next full scan (spec + # decision 11: adopt the local loop's backfill). + # Zero-byte exclusion: EMPTY_FILE_SHA256 never + # lands in photos.file_hash (it would collide as + # an exact duplicate of every other empty file + # and empty_hash_needs_repair would churn + # repairs), so empty files pass file_hash=None, + # which update_photo_hash_check treats as + # status-only — the row keeps scan's NULL. + if params.verify_by_hash: + db.update_photo_hash_check( + row["id"], "ok", + file_hash=( + src_hash + if src_hash != EMPTY_FILE_SHA256 + else None + ), + commit=False, + ) + # Fresh mount row this run stamped as valid — + # the after-import chaining hook builds its + # process job collection from these ids. Stays + # OUTSIDE the verify gate on every accept path. + state.imported_photo_ids.add(row["id"]) elif scan_h is not None and scan_h != src_h_norm: _reclassify_landed_failed( state, rel, entry, @@ -3623,14 +3650,19 @@ def _rsync_cancelled(rc): ) batch_st.reclassified_landed_paths.add(entry.dest_path) continue - if params.verify_by_hash: - db.update_photo_hash_check( - row["id"], "ok", commit=False, - ) - # Fresh mount row this run stamped as valid — the - # after-import chaining hook builds its process job - # collection from these ids. - state.imported_photo_ids.add(row["id"]) + else: + # Scan's row hash matches — no backfill needed, + # the row already holds the hash (local doesn't + # backfill on this path either). + if params.verify_by_hash: + db.update_photo_hash_check( + row["id"], "ok", commit=False, + ) + # Fresh mount row this run stamped as valid — + # the after-import chaining hook builds its + # process job collection from these ids. Stays + # OUTSIDE the verify gate on every accept path. + state.imported_photo_ids.add(row["id"]) else: # RAW+JPEG pairing merges the JPEG's photo row into # the RAW primary (companion_path) and deletes the diff --git a/vireo/tests/test_import_job.py b/vireo/tests/test_import_job.py index ce186687f..0aed03a3c 100644 --- a/vireo/tests/test_import_job.py +++ b/vireo/tests/test_import_job.py @@ -6568,6 +6568,117 @@ def scan_then_null_and_wipe(destination, db_arg, **kw): rows["DSC_0001.jpg"]) +def test_remote_import_backfills_file_hash_when_scan_left_null( + tmp_path, monkeypatch): + """Spec decision 11 (D4): when scan() leaves a non-empty landed + file's row with ``file_hash`` NULL and the mount re-read agrees with + the hash this import recorded, the remote path must backfill + ``photos.file_hash`` with that hash — mirroring the local stamping + loop's ``update_photo_hash_check(..., "ok", file_hash=...)``. + Without the backfill the row's NULL persists until the next full + scan, so exact-duplicate detection and hash-based features see a + hashless row for a file whose bytes were just verified.""" + import scanner as _scanner + from import_dedup import compute_file_hash + from import_job import ImportParams, run_import_job + + ra = _remote_archive_for(tmp_path) + calls = _remote_calls(ra) + _install_fake_remote_rsync(monkeypatch, calls, verify=None) + + card = _make_card(tmp_path, [ + ("DSC_0001.jpg", datetime(2026, 7, 3, 10, 0, 0), "red"), + ]) + + db_path = str(tmp_path / "test.db") + + # Wrap scan() to null out the file_hash the scanner wrote, forcing + # the row-exists-but-hash-unknown branch (same NULL-forcing shape as + # test_remote_import_null_scan_hash_but_mount_matches_still_ok). + orig_scan = _scanner.scan + + def scan_then_null_hash(destination, db_arg, **kw): + rv = orig_scan(destination, db_arg, **kw) + db_arg.conn.execute( + "UPDATE photos SET file_hash = NULL " + "WHERE filename = 'DSC_0001.jpg'" + ) + db_arg.conn.commit() + return rv + + monkeypatch.setattr(_scanner, "scan", scan_then_null_hash) + + db = Database(db_path) + ws_id = db._active_workspace_id + result = run_import_job( + _make_job(), FakeRunner(), db_path, ws_id, + ImportParams( + sources=[str(card)], destination=ra["mount_base"], + remote_target=ra, verify_by_hash=True, + ), + ) + + assert result["failed"] == 0, result + assert result["copied"] == 1, result + mount_file = os.path.join( + ra["mount_base"], "2026", "2026-07-03", "DSC_0001.jpg", + ) + rows = {r["filename"]: r for r in _photo_rows(db)} + row = rows["DSC_0001.jpg"] + assert row["hash_status"] == "ok", dict(row) + assert row["file_hash"] == compute_file_hash(mount_file), dict(row) + + +def test_local_import_backfills_file_hash_when_scan_left_null( + tmp_path, monkeypatch): + """Local mirror of + ``test_remote_import_backfills_file_hash_when_scan_left_null`` — + expected to pass WITHOUT a production change: it characterizes the + local stamping loop's existing non-empty-NULL-hash backfill + (``update_photo_hash_check(..., "ok", file_hash=verified_hash)``) + that spec decision 11 adopts for the remote path.""" + import scanner as _scanner + from import_dedup import compute_file_hash + from import_job import ImportParams, run_import_job + + card = _make_card(tmp_path, [ + ("DSC_0001.jpg", datetime(2026, 7, 3, 10, 0, 0), "red"), + ]) + archive = tmp_path / "archive" + + orig_scan = _scanner.scan + + def scan_then_null_hash(destination, db_arg, **kw): + rv = orig_scan(destination, db_arg, **kw) + db_arg.conn.execute( + "UPDATE photos SET file_hash = NULL " + "WHERE filename = 'DSC_0001.jpg'" + ) + db_arg.conn.commit() + return rv + + monkeypatch.setattr(_scanner, "scan", scan_then_null_hash) + + db_path = str(tmp_path / "test.db") + db = Database(db_path) + ws_id = db._active_workspace_id + result = run_import_job( + _make_job(), FakeRunner(), db_path, ws_id, + ImportParams( + sources=[str(card)], destination=str(archive), + verify_by_hash=True, + ), + ) + + assert result["failed"] == 0, result + assert result["copied"] == 1, result + dest_file = archive / "2026" / "2026-07-03" / "DSC_0001.jpg" + rows = {r["filename"]: r for r in _photo_rows(db)} + row = rows["DSC_0001.jpg"] + assert row["hash_status"] == "ok", dict(row) + assert row["file_hash"] == compute_file_hash(str(dest_file)), dict(row) + + def test_remote_import_paired_jpeg_no_verify_fails_on_mount_mismatch( tmp_path, monkeypatch): """Companion parity with the non-companion branch: without @@ -10105,6 +10216,99 @@ def deleting_scan(*args, **kwargs): assert obs["safe_to_format"] is False, obs +def test_remote_zero_byte_accept_leaves_file_hash_null( + tmp_path, monkeypatch): + """Guard for spec decision 11's zero-byte exclusion (the backfill + trap): a zero-byte accept must leave the row's ``file_hash`` NULL. + Backfilling ``EMPTY_FILE_SHA256`` into ``photos.file_hash`` would + recreate the collision the scanner's zero-byte NULL convention + exists to prevent (every empty file would become an exact duplicate + of every other, and ``empty_hash_needs_repair`` would churn + repairs). Same adoption geometry as + ``test_remote_zero_byte_adoption_revalidates_before_stamping`` but + WITHOUT the deletion step — the accept path, not the failure path.""" + from import_job import ImportParams, run_import_job + + card = tmp_path / "card" + card.mkdir() + card_file = card / "empty.jpg" + card_file.write_bytes(b"") + ts = datetime(2026, 7, 3, 10, 0, 0).timestamp() + os.utime(str(card_file), (ts, ts)) + + ra = _remote_archive_for(tmp_path) + calls = _remote_calls(ra) + _install_fake_remote_rsync(monkeypatch, calls, verify=None) + + mount_base = Path(ra["mount_base"]) + seed_folder = mount_base / "2026" / "2026-07-03" + seed_folder.mkdir(parents=True) + adopted_dest_path = seed_folder / "empty.jpg" + adopted_dest_path.write_bytes(b"") + os.utime(str(adopted_dest_path), (ts, ts)) + + db_path = str(tmp_path / "test.db") + db = Database(db_path) + result = run_import_job( + _make_job(), FakeRunner(), db_path, db._active_workspace_id, + ImportParams( + sources=[str(card)], destination=ra["mount_base"], + remote_target=ra, verify_by_hash=True, + skip_duplicates=False, + ), + ) + + assert result["skipped_duplicate"] == 1, result + assert result["failed"] == 0, result + rows = {r["filename"]: r for r in _photo_rows(db)} + row = rows["empty.jpg"] + assert row["hash_status"] == "ok", dict(row) + assert row["file_hash"] is None, dict(row) + + +def test_local_zero_byte_accept_leaves_file_hash_null( + tmp_path, monkeypatch): + """Local mirror of + ``test_remote_zero_byte_accept_leaves_file_hash_null``: the local + zero-byte stamp deliberately omits ``file_hash`` so the row keeps + scan's NULL (``EMPTY_FILE_SHA256`` never lands in the column). Same + adoption geometry as + ``test_local_zero_byte_adoption_revalidates_before_stamping`` + WITHOUT the deletion step.""" + from import_job import ImportParams, run_import_job + + card = tmp_path / "card" + card.mkdir() + card_file = card / "empty.jpg" + card_file.write_bytes(b"") + ts = datetime(2026, 7, 3, 10, 0, 0).timestamp() + os.utime(str(card_file), (ts, ts)) + + archive = tmp_path / "archive" + seed_folder = archive / "2026" / "2026-07-03" + seed_folder.mkdir(parents=True) + adopted_dest_path = seed_folder / "empty.jpg" + adopted_dest_path.write_bytes(b"") + os.utime(str(adopted_dest_path), (ts, ts)) + + db_path = str(tmp_path / "test.db") + db = Database(db_path) + result = run_import_job( + _make_job(), FakeRunner(), db_path, db._active_workspace_id, + ImportParams( + sources=[str(card)], destination=str(archive), + verify_by_hash=True, skip_duplicates=False, + ), + ) + + assert result["skipped_duplicate"] == 1, result + assert result["failed"] == 0, result + rows = {r["filename"]: r for r in _photo_rows(db)} + row = rows["empty.jpg"] + assert row["hash_status"] == "ok", dict(row) + assert row["file_hash"] is None, dict(row) + + def test_local_renamed_twin_of_accepted_duplicate_current_behavior( tmp_path, monkeypatch): """CHARACTERIZATION for spec decision 5 (local half). The local path From 6c0d4aa1a910f165ab37ab395fc2d5440c3828df Mon Sep 17 00:00:00 2001 From: Julius Simonelli Date: Sun, 9 Aug 2026 22:50:48 +0200 Subject: [PATCH 5/7] Normalize zero-byte verified_hash at _LandedFile construction (spec D2/D3) Co-Authored-By: Claude Fable 5 --- vireo/import_job.py | 54 ++++++--- vireo/tests/test_import_job.py | 196 +++++++++++++++++++++++++++++++++ 2 files changed, 237 insertions(+), 13 deletions(-) diff --git a/vireo/import_job.py b/vireo/import_job.py index d27684f20..6d23b7828 100644 --- a/vireo/import_job.py +++ b/vireo/import_job.py @@ -576,12 +576,17 @@ class _LandedFile: """One file this batch landed (fresh copy/transfer) or adopted. ``verified_hash`` is the hash the import attests is at ``dest_path`` - (copy-time hash locally; card-side hash remotely). ``origin`` is - "copied" or "skipped_duplicate" (adoption) and drives rollback - accounting in ``_reclassify_landed_failed``. + (copy-time hash locally; card-side hash remotely). Never None: the + ledger convention for zero-byte files is ``EMPTY_FILE_SHA256``, + normalized at the construction sites (``checker.content_hash`` + returns None for size-0; scan's photo-row convention is NULL — + consumers that compare against row hashes normalize EMPTY → None + themselves). ``origin`` is "copied" or "skipped_duplicate" + (adoption) and drives rollback accounting in + ``_reclassify_landed_failed``. """ dest_path: str - verified_hash: str | None + verified_hash: str source_path: str origin: str src_size: int | None @@ -3132,7 +3137,19 @@ def _emit_transfer(rel, transfer_current, transfer_total, current_file): # exactly 0). See PR #1113 review. batch_st.landed.append(_LandedFile( dest_path=cand_mount, - verified_hash=src_hash, + # checker.content_hash returns None for + # size-0; the ledger convention is EMPTY, + # scan's row convention is NULL — + # normalize here, once, instead of in + # every consumer. (The adopt gate above + # filters ``src_hash is None``, so this + # arm is defensive at this site.) Do NOT + # normalize ``src_hash`` itself: the + # intra-batch dedup maps rely on the None + # convention. + verified_hash=src_hash + if src_hash is not None + else EMPTY_FILE_SHA256, source_path=str(source_file), origin="skipped_duplicate", src_size=src_size, @@ -3439,7 +3456,18 @@ def _rsync_cancelled(rc): state.verified += 1 batch_st.landed.append(_LandedFile( dest_path=dest_path, - verified_hash=src_hash, + # checker.content_hash returns None for size-0; + # the ledger convention is EMPTY, scan's row + # convention is NULL — normalize here, once, + # instead of in every consumer. Do NOT + # normalize ``src_hash`` itself: the + # intra-batch dedup maps + # (claimed_basenames/queued_src_hashes), the + # adopt gate, and ``_record_checker`` rely on + # the None convention. + verified_hash=src_hash + if src_hash is not None + else EMPTY_FILE_SHA256, source_path=str(sf), origin="copied", src_size=sz, @@ -3534,13 +3562,13 @@ def _rsync_cancelled(rc): # cross-check against ``verified_hash``. See PR #1113 # review. # - # Normalize zero-byte convention on both sides: - # scan() writes NULL for zero-byte files; - # ``checker.content_hash`` returns None; a - # checker-less ``compute_file_hash`` returns - # ``EMPTY_FILE_SHA256``. Treat all three as - # equivalent so an empty card file matches its - # empty catalog row. + # Normalize zero-byte convention on both sides for + # the ROW comparison: scan() writes NULL for + # zero-byte files, while ``entry.verified_hash`` + # is never None (zero-byte is normalized to + # ``EMPTY_FILE_SHA256`` at ``_LandedFile`` + # construction). Map EMPTY → None on both sides so + # an empty card file matches its empty catalog row. scan_h = row["file_hash"] if scan_h == EMPTY_FILE_SHA256: scan_h = None diff --git a/vireo/tests/test_import_job.py b/vireo/tests/test_import_job.py index 0aed03a3c..ec4c3a7ac 100644 --- a/vireo/tests/test_import_job.py +++ b/vireo/tests/test_import_job.py @@ -3828,6 +3828,202 @@ def test_import_invalidates_derived_caches_when_pre_row_had_null_hash(tmp_path): ) +def test_remote_zero_byte_reimport_invalidates_derived_caches( + tmp_path, monkeypatch): + """Spec D2/D3 pin: a remote zero-byte re-import over a pre-existing + (NULL-hash) photo row must fire the import's own derived-cache + invalidation diff loop, matching local. + + Pre-normalization the checker path (``skip_duplicates=True``) hands + the remote loop ``verified_hash=None`` (``checker.content_hash`` + returns None for size-0), so the diff loop in + ``_invalidate_changed_and_sweep`` compares ``pre_scan_hashes``'s + NULL against None — equal, no invalidation. Local always carries + ``EMPTY_FILE_SHA256``, so ``None != EMPTY`` fires. Normalizing + ``verified_hash`` at ``_LandedFile`` construction flips remote to + local's behavior (a harmless extra invalidation + sweep). + + Geometry is load-bearing (all three): + (a) the zero-byte photo ROW pre-exists at the dest path, so + ``pre_scan_hashes`` holds its NULL; + (b) the mount FILE at that path is ABSENT — if present, the adopt + gate can't match ``on_disk == None`` (checker src hash is None), + the collision walk renames to ``_1``, and the landing is a + fresh insert absent from ``pre_scan_hashes``; + (c) ``skip_duplicates=True`` — the checker path is what makes + ``verified_hash`` None today; checker-less already produces + EMPTY and the test would be green pre-change. + + The spy only counts calls AFTER scan() returns: scanner's own + zero-byte content-change arm (``file_size == 0 and prev_file_hash + != EMPTY_FILE_SHA256``) also invalidates this row DURING the batch + scan, and counting it would make the test green pre-change. The + diff loop under test runs post-scan.""" + import scanner as _scanner + from import_job import ImportParams, run_import_job + + card = tmp_path / "card" + card.mkdir() + card_file = card / "empty.jpg" + card_file.write_bytes(b"") + ts = datetime(2026, 7, 3, 10, 0, 0).timestamp() + os.utime(str(card_file), (ts, ts)) + + ra = _remote_archive_for(tmp_path) + calls = _remote_calls(ra) + _install_fake_remote_rsync(monkeypatch, calls, verify=None) + + # Pre-existing zero-byte row at the template dest path; the mount + # file itself is deliberately ABSENT (precondition b). + dest_dir = Path(ra["mount_base"]) / "2026" / "2026-07-03" + dest_dir.mkdir(parents=True) + + vireo_dir = tmp_path / "vireo_data" + (vireo_dir / "working").mkdir(parents=True) + + db_path = str(tmp_path / "test.db") + db = Database(db_path) + ws_id = db._active_workspace_id + fid = db.conn.execute( + "INSERT INTO folders (path, name, status) VALUES (?, ?, 'ok')", + (str(dest_dir), dest_dir.name), + ).lastrowid + photo_id = db.conn.execute( + "INSERT INTO photos (folder_id, filename, extension, file_size," + " file_hash) VALUES (?, ?, '.jpg', 0, NULL)", + (fid, "empty.jpg"), + ).lastrowid + db.conn.commit() + + scan_done = {} + real_scan = _scanner.scan + + def tracking_scan(*args, **kwargs): + try: + return real_scan(*args, **kwargs) + finally: + scan_done["v"] = True + + real_inval = _scanner._invalidate_derived_caches + post_scan_invalidations = [] + + def spy_inval(db_arg, vdir, pid, **kw): + if scan_done.get("v"): + post_scan_invalidations.append(pid) + return real_inval(db_arg, vdir, pid, **kw) + + monkeypatch.setattr(_scanner, "scan", tracking_scan) + monkeypatch.setattr(_scanner, "_invalidate_derived_caches", spy_inval) + + result = run_import_job( + _make_job(), FakeRunner(), db_path, ws_id, + ImportParams( + sources=[str(card)], destination=ra["mount_base"], + remote_target=ra, verify_by_hash=True, + skip_duplicates=True, vireo_dir=str(vireo_dir), + ), + ) + + assert result["copied"] == 1, result + assert result["failed"] == 0, result + assert scan_done, "the batch scan never ran" + assert photo_id in post_scan_invalidations, ( + "remote zero-byte re-import over a pre-existing row must fire " + "the import's post-scan derived-cache invalidation (diff loop), " + "matching local", post_scan_invalidations, + ) + + +def test_local_zero_byte_reimport_invalidates_derived_caches( + tmp_path, monkeypatch): + """Local mirror of + ``test_remote_zero_byte_reimport_invalidates_derived_caches`` — + expected GREEN without a production change: local's zero-byte + landings always carry ``verified_hash=EMPTY_FILE_SHA256``, so the + diff loop's ``NULL != EMPTY`` comparison already fires. + + DELIBERATELY DIFFERENT geometry from the remote test — do not + force-fit symmetry: the dest FILE is PRESENT and + ``skip_duplicates=False``, so the checker-less zero-byte adopt + lands at the same path (local's idiomatic zero-byte landing, per + the #1438 quartet). The remote test needs the file ABSENT + + ``skip_duplicates=True`` because only the checker path produces the + ``verified_hash=None`` being normalized; local has no equivalent — + ``copy_and_hash_verify`` never returns None and the adopt hardcodes + EMPTY.""" + import scanner as _scanner + from import_job import ImportParams, run_import_job + + card = tmp_path / "card" + card.mkdir() + card_file = card / "empty.jpg" + card_file.write_bytes(b"") + ts = datetime(2026, 7, 3, 10, 0, 0).timestamp() + os.utime(str(card_file), (ts, ts)) + + archive = tmp_path / "archive" + dest_dir = archive / "2026" / "2026-07-03" + dest_dir.mkdir(parents=True) + dest_file = dest_dir / "empty.jpg" + dest_file.write_bytes(b"") + os.utime(str(dest_file), (ts, ts)) + + vireo_dir = tmp_path / "vireo_data" + (vireo_dir / "working").mkdir(parents=True) + + db_path = str(tmp_path / "test.db") + db = Database(db_path) + ws_id = db._active_workspace_id + fid = db.conn.execute( + "INSERT INTO folders (path, name, status) VALUES (?, ?, 'ok')", + (str(dest_dir), dest_dir.name), + ).lastrowid + photo_id = db.conn.execute( + "INSERT INTO photos (folder_id, filename, extension, file_size," + " file_hash) VALUES (?, ?, '.jpg', 0, NULL)", + (fid, "empty.jpg"), + ).lastrowid + db.conn.commit() + + scan_done = {} + real_scan = _scanner.scan + + def tracking_scan(*args, **kwargs): + try: + return real_scan(*args, **kwargs) + finally: + scan_done["v"] = True + + real_inval = _scanner._invalidate_derived_caches + post_scan_invalidations = [] + + def spy_inval(db_arg, vdir, pid, **kw): + if scan_done.get("v"): + post_scan_invalidations.append(pid) + return real_inval(db_arg, vdir, pid, **kw) + + monkeypatch.setattr(_scanner, "scan", tracking_scan) + monkeypatch.setattr(_scanner, "_invalidate_derived_caches", spy_inval) + + result = run_import_job( + _make_job(), FakeRunner(), db_path, ws_id, + ImportParams( + sources=[str(card)], destination=str(archive), + verify_by_hash=True, skip_duplicates=False, + vireo_dir=str(vireo_dir), + ), + ) + + assert result["skipped_duplicate"] == 1, result + assert result["failed"] == 0, result + assert scan_done, "the batch scan never ran" + assert photo_id in post_scan_invalidations, ( + "local zero-byte adopt over a pre-existing row must fire the " + "import's post-scan derived-cache invalidation (diff loop)", + post_scan_invalidations, + ) + + def test_import_invalidates_raw_caches_when_new_jpeg_pairs(tmp_path): """RAW+JPEG companion restore: when a freshly copied JPEG lands as companion to an existing RAW row (pair-merge deletes the JPEG's own From 9699ca117cad29feef2806029ffc51e5fa9a5a1b Mon Sep 17 00:00:00 2001 From: Julius Simonelli Date: Sun, 9 Aug 2026 22:58:12 +0200 Subject: [PATCH 6/7] Unify stamping gating and messages; loops now textually identical (spec D1/D5-D7) Both stamping loops restructured to one shared shape (local's branch order), stamps gated on verified_counted_for_copies in both (a no-op locally where it is always True; identical to the params.verify_by_hash gate remotely), imported_photo_ids.add outside the gate on every accept path, _rehash_dest_or_none hoisted to module level taking (path, stop_requested), and the unified five-message reason table applied via a per-function dest_noun local ("mount"/"archive"). Textual-identity verification (comments/blanks stripped, dest_noun literal normalized): extract both regions (raw_companion_invalidations decl through the end of the for-loop body), replace the dest_noun literal with "NOUN", drop blank/comment-only lines, then: diff /tmp/remote_stripped.txt /tmp/local_stripped.txt -> (empty) The full-text diff including comments is also empty modulo the noun literal (one trailing blank line after the local loop). One test tail updated as part of the remote verify/no-verify wording collapse: the no-verify paired-JPEG mismatch test now expects "hash this import recorded" instead of "source hash". All 4 pinned reason prefixes (9 assertion sites) unchanged. Full file: 231 passed, 1 skipped (zero flips). Co-Authored-By: Claude Fable 5 --- vireo/import_job.py | 648 ++++++++++++++------------------- vireo/tests/test_import_job.py | 2 +- 2 files changed, 281 insertions(+), 369 deletions(-) diff --git a/vireo/import_job.py b/vireo/import_job.py index 6d23b7828..d36a31406 100644 --- a/vireo/import_job.py +++ b/vireo/import_job.py @@ -2363,6 +2363,27 @@ def _worker(): return result["hash"] +def _rehash_dest_or_none(path, stop_requested): + """Re-hash a landed destination file, returning None on read failure. + + The stamping loop's last-line check that the bytes currently at the + destination path still match the hash this import recorded — + necessary any time the scan-side hash is missing (paired-JPEG row + deletion) or NULL (zero-byte convention aside, a NULL means the + scan-side read failed between landing and scan). ``None`` + unambiguously means unreadable: ``_LandedFile.verified_hash`` is + never None, so callers comparing against it cannot confuse a read + failure with a legitimately empty file. Re-raises + ``DestReadCancelled`` from a stop request. + """ + try: + return _hash_dest_file(path, stop_requested) + except DestReadCancelled: + raise + except OSError: + return None + + def _key_twin_rows(db, key): """Catalog rows whose stored identity equals a source metadata key. @@ -3527,14 +3548,31 @@ def _rsync_cancelled(rc): # already carried companion_path) and invalidate below. # Mirrors the local path — spec decision 6. raw_companion_invalidations = set() - # NOTE: this stamping loop is deliberately NOT extracted — - # it hides divergence 11 (local-only file_hash backfill) - # and the zero-byte normalization split; it is unified in - # the spec's PR 6b (align-then-extract). See the decision - # table in the import-path-unification spec. - for entry in list(batch_st.landed): + # NOTE: this stamping loop is deliberately not yet + # extracted — the divergences it hid (spec decisions 11, + # D2/D3, D1, D5-D7) were resolved in this PR's earlier + # commits; the two loop bodies are now textually identical + # modulo ``dest_noun`` and are extracted next. See the + # decision table in the import-path-unification spec. + # + # Post-scan stamping: cross-check what scan() cataloged for + # every landed file against ``entry.verified_hash`` — the + # hash this import recorded when the bytes landed (copy-time + # hash locally, card-side hash remotely). Catalog integrity + # is checked in BOTH verify modes; the ``hash_status='ok'`` + # stamp is the byte-attestation and stays gated on + # ``verified_counted_for_copies`` (True locally, where every + # copy is hash-verified; ``params.verify_by_hash`` remotely + # — the wording says "recorded", not "verified", because a + # no-verify remote run computed the card-side hash but + # verified nothing destination-side). ``imported_photo_ids + # .add`` stays OUTSIDE that gate on every accept path — the + # after-import chaining hook builds its process job + # collection from these ids even on no-verify runs. + dest_noun = "mount" + for entry in batch_st.landed: dest_path = entry.dest_path - src_hash = entry.verified_hash + verified_hash = entry.verified_hash row = db.conn.execute( """SELECT p.id, p.file_hash FROM photos p JOIN folders f ON f.id = p.folder_id @@ -3542,167 +3580,14 @@ def _rsync_cancelled(rc): (os.path.dirname(dest_path), os.path.basename(dest_path)), ).fetchone() - if row is not None: - # Cross-check the scanned MOUNT row's hash against the - # source hash (the bytes we intended to land). This - # runs even without ``verify_by_hash`` because catalog - # integrity is a separate concern from the format - # honesty gate: a stale/misconfigured mount that - # happens to already contain ``/`` - # for a name we ``--ignore-existing``-transferred, or - # a receiver-side race that left a different file at - # that path, would otherwise be cataloged against - # unrelated bytes while ``safe_to_format=False`` - # (correct on the format side, but the workspace - # catalog now points at the wrong photo). The - # ``hash_status='ok'`` stamp still runs ONLY behind - # ``verify_by_hash`` — that stamp is the independent - # card→NAS attestation, not just "scan and source - # agree on the mount view". Mirrors the local path's - # cross-check against ``verified_hash``. See PR #1113 - # review. - # - # Normalize zero-byte convention on both sides for - # the ROW comparison: scan() writes NULL for - # zero-byte files, while ``entry.verified_hash`` - # is never None (zero-byte is normalized to - # ``EMPTY_FILE_SHA256`` at ``_LandedFile`` - # construction). Map EMPTY → None on both sides so - # an empty card file matches its empty catalog row. - scan_h = row["file_hash"] - if scan_h == EMPTY_FILE_SHA256: - scan_h = None - src_h_norm = ( - None if src_hash == EMPTY_FILE_SHA256 - else src_hash - ) - # scan() can legitimately leave ``file_hash`` NULL - # (large files, prior partial scan, tests that stub - # the hash step, or a read/permission failure that - # scanner suppresses). A missing scan hash doesn't - # prove anything on its own, but silently accepting - # would let a stale/unreadable mount stamp ``ok`` - # under ``verify_by_hash`` — the NAS checksum only - # proves card bytes reached the SSH target, not that - # the cataloged mount path holds those bytes. Rehash - # the mount file directly as the last-line check; - # mirrors the local path's ``_rehash_dest_or_none`` - # fallback. See PR #1113 review. - # - # Zero-byte adopted files (``skip_duplicates=False`` - # only: the checker path returns ``None`` from - # ``content_hash`` so the collision walk never - # matches an empty source against an empty on-disk - # candidate) reach this point with ``src_h_norm == - # scan_h == None``. Pre-fold, the deleted - # ``adopted_paths`` validation pass always called - # ``_hash_dest_file`` when ``scan_h`` was ``None`` - # and treated ``OSError`` as failure — without that, - # an empty adopted file that vanished or became - # unreadable between the adopt-time hash and the - # stamping loop would keep its ``skipped_duplicate`` - # booking and get ``hash_status='ok'``, and - # ``safe_to_format`` could go green over an archive - # file that is no longer there. Distinguish OSError - # from a legitimate empty mount file so the empty- - # source, empty-mount case still passes. See PR - # #1437 review (Codex P1 r3741224300). - if scan_h is None: - read_failed = False - try: - mount_hash = _hash_dest_file( - dest_path, _stop_requested) - except DestReadCancelled: - state.cancelled = True - break - except OSError: - mount_hash = None - read_failed = True - mount_norm = ( - None if mount_hash == EMPTY_FILE_SHA256 - else mount_hash - ) - if read_failed or mount_norm != src_h_norm: - _reclassify_landed_failed( - state, rel, entry, - "scan wrote no mount row hash and a re-" - "read of the mount file " - + ("disagrees with the source hash" - if not params.verify_by_hash else - "disagrees with the hash verified " - "on the NAS") - + " (mount base is likely stale, " - "unreadable, or misconfigured)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(entry.dest_path) - continue - # Scan left the row's file_hash NULL and the - # mount re-read agrees with the hash this import - # recorded — backfill it so the row doesn't stay - # hashless until the next full scan (spec - # decision 11: adopt the local loop's backfill). - # Zero-byte exclusion: EMPTY_FILE_SHA256 never - # lands in photos.file_hash (it would collide as - # an exact duplicate of every other empty file - # and empty_hash_needs_repair would churn - # repairs), so empty files pass file_hash=None, - # which update_photo_hash_check treats as - # status-only — the row keeps scan's NULL. - if params.verify_by_hash: - db.update_photo_hash_check( - row["id"], "ok", - file_hash=( - src_hash - if src_hash != EMPTY_FILE_SHA256 - else None - ), - commit=False, - ) - # Fresh mount row this run stamped as valid — - # the after-import chaining hook builds its - # process job collection from these ids. Stays - # OUTSIDE the verify gate on every accept path. - state.imported_photo_ids.add(row["id"]) - elif scan_h is not None and scan_h != src_h_norm: - _reclassify_landed_failed( - state, rel, entry, - "scanned mount row hash does not match " - "the source hash (mount base is likely " - "stale or misconfigured)" - if not params.verify_by_hash else - "scanned mount row hash does not match " - "the hash verified on the NAS (mount " - "base is likely stale or misconfigured)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(entry.dest_path) - continue - else: - # Scan's row hash matches — no backfill needed, - # the row already holds the hash (local doesn't - # backfill on this path either). - if params.verify_by_hash: - db.update_photo_hash_check( - row["id"], "ok", commit=False, - ) - # Fresh mount row this run stamped as valid — - # the after-import chaining hook builds its - # process job collection from these ids. Stays - # OUTSIDE the verify gate on every accept path. - state.imported_photo_ids.add(row["id"]) - else: + if row is None: # RAW+JPEG pairing merges the JPEG's photo row into # the RAW primary (companion_path) and deletes the # JPEG's own row, so a landed JPEG whose sibling RAW # was scanned in the same batch legitimately has no # row of its own. Look it up as another row's # companion_path before deciding "not cataloged". - # When verifying, cross-check the mount JPEG bytes - # against the hash confirmed card->NAS (same - # stale-mount guard the non-companion branch runs - # above). Mirrors the local path's companion lookup. - # See PR #1113 review. + # See PR #1107/#1113 reviews. companion = db.conn.execute( """SELECT p.id FROM photos p JOIN folders f ON f.id = p.folder_id @@ -3711,92 +3596,131 @@ def _rsync_cancelled(rc): os.path.basename(dest_path)), ).fetchone() if companion is not None: - # The paired JPEG's own photo row is gone by - # design (pair-scan merges it into the RAW - # primary), so the non-companion branch above - # can't cross-check its bytes for us. Hash the - # mount JPEG here regardless of - # ``verify_by_hash`` — the non-companion branch - # compares ``scan_h`` vs ``src_h_norm`` even in - # no-verify mode as a stale-mount catalog- - # integrity guard, and paired JPEGs need the - # same protection or a stale/misconfigured - # mount with a same-named but different JPEG - # would enqueue after-import processing against - # the wrong companion. Only the - # ``verified``/``hash_status`` accounting is - # gated behind ``verify_by_hash``. See PR #1113 - # review. - # Same OSError-distinguishing shape as the - # non-companion branch above: a zero-byte - # adopted companion (``src_hash == - # EMPTY_FILE_SHA256`` → ``src_h_norm == - # None``) whose mount file vanished or became - # unreadable between the adopt-time hash and - # this loop would surface here as - # ``mount_hash = None`` and ``mount_norm == - # src_h_norm == None`` — comparison passes, - # the RAW row joins ``imported_photo_ids``, - # and ``safe_to_format`` could go green over - # a companion that is no longer there. The - # removed ``adopted_paths`` validator treated - # ``OSError`` as failure; preserve that - # explicit read-failure flag here. See PR - # #1437 review (Codex P1 r3741254301). - read_failed = False + # The paired JPEG's own row is gone by design, + # so the row branch below can't cross-check its + # bytes. Re-read the destination file here in + # BOTH verify modes — paired JPEGs need the same + # stale-destination catalog-integrity guard or + # after-import processing would enqueue against + # the wrong companion. ``None`` from the re-read + # unambiguously means unreadable: + # ``verified_hash`` is never None (zero-byte + # normalizes to ``EMPTY_FILE_SHA256`` at + # ``_LandedFile`` construction), so an empty + # companion whose file vanished cannot be + # confused with a legitimately empty one. See + # PR #1107/#1113/#1437 reviews. try: - mount_hash = _hash_dest_file( + actual = _rehash_dest_or_none( dest_path, _stop_requested) except DestReadCancelled: state.cancelled = True break - except OSError: - mount_hash = None - read_failed = True - src_h_norm = ( - None if src_hash == EMPTY_FILE_SHA256 - else src_hash - ) - mount_norm = ( - None if mount_hash == EMPTY_FILE_SHA256 - else mount_hash - ) - if read_failed or mount_norm != src_h_norm: - _reclassify_landed_failed( - state, rel, entry, - "paired companion mount bytes could " - "not be read" - if read_failed else - "paired companion mount bytes do " - "not match the source hash (mount " - "base is likely stale or " - "misconfigured)" - if not params.verify_by_hash else - "paired companion mount bytes do " - "not match the hash verified on " - "the NAS (mount base is likely " - "stale or misconfigured)", - verified_counted_for_copies, + if actual is not None and actual == verified_hash: + # Invalidate the RAW's derived caches + # regardless of origin: adoption only proves + # the JPEG bytes were already at the dest + # path, NOT that the RAW row already carried + # ``companion_path`` for this JPEG. A prior + # partial run or backfill may have left the + # RAW as RAW-only, and the deferred + # end-of-run ``_extract_working_copies`` + # skips rows whose ``working_copy_path IS + # NOT NULL`` — a stale RAW-only cache would + # persist past this import otherwise. See + # PR #1107 review. + raw_companion_invalidations.add( + companion["id"], ) - batch_st.reclassified_landed_paths.add(entry.dest_path) + # The landed JPEG's bytes are represented on + # the RAW primary — that row is what the + # chaining hook should process. + state.imported_photo_ids.add(companion["id"]) continue - # JPEG bytes are represented by the RAW row's - # companion_path — accept as landed; leave the - # copied/verified counters alone. The RAW row is - # what the chaining hook should process, so its - # id joins ``imported_photo_ids``. See PR #1113 - # review. - # Collected for the post-validation invalidation - # loop — see the raw_companion_invalidations decl. - raw_companion_invalidations.add(companion["id"]) - state.imported_photo_ids.add(companion["id"]) + _reclassify_landed_failed( + state, rel, entry, + f"paired companion {dest_noun} bytes could " + "not be read" + if actual is None else + f"paired companion {dest_noun} bytes do not " + "match the hash this import recorded " + f"({dest_noun} base is likely stale or " + "misconfigured)", + verified_counted_for_copies, + ) + batch_st.reclassified_landed_paths.add(dest_path) continue _reclassify_landed_failed( state, rel, entry, "not cataloged after scan (no photo row)", verified_counted_for_copies, ) - batch_st.reclassified_landed_paths.add(entry.dest_path) + batch_st.reclassified_landed_paths.add(dest_path) + continue + if row["file_hash"] == verified_hash: + if verified_counted_for_copies: + db.update_photo_hash_check( + row["id"], "ok", commit=False, + ) + state.imported_photo_ids.add(row["id"]) + elif row["file_hash"] is None: + # scan() legitimately writes NULL for zero-byte + # files (the convention keeps ``EMPTY_FILE_SHA256`` + # out of ``photos.file_hash``) and can leave NULL + # when its own read failed (large files, prior + # partial scan, or a suppressed read error). Don't + # stamp blind either way: re-read the destination + # file as the last-line check so a file that + # vanished, changed, or became unreadable between + # scan and stamping fails instead of keeping + # ``hash_status='ok'`` with ``safe_to_format`` + # green. See PR #1107/#1113/#1437 reviews. + try: + actual = _rehash_dest_or_none( + dest_path, _stop_requested) + except DestReadCancelled: + state.cancelled = True + break + if actual is not None and actual == verified_hash: + # Backfill the row hash scan couldn't write + # (spec decision 11) — except the zero-byte + # case, where ``file_hash=None`` hits + # ``update_photo_hash_check``'s status-only arm + # and the row keeps scan's NULL (backfilling + # EMPTY would recreate the every-empty-file + # collision the convention exists to prevent). + if verified_counted_for_copies: + db.update_photo_hash_check( + row["id"], "ok", + file_hash=( + verified_hash + if verified_hash != EMPTY_FILE_SHA256 + else None + ), + commit=False, + ) + state.imported_photo_ids.add(row["id"]) + else: + _reclassify_landed_failed( + state, rel, entry, + f"scan wrote no {dest_noun} row hash and a " + f"re-read of the {dest_noun} file disagrees " + "with the hash this import recorded " + f"({dest_noun} file is likely stale, " + "unreadable, or misconfigured)", + verified_counted_for_copies, + ) + batch_st.reclassified_landed_paths.add(dest_path) + else: + _reclassify_landed_failed( + state, rel, entry, + f"scanned {dest_noun} row hash does not match " + "the hash this import recorded " + f"({dest_noun} base is likely stale or " + "misconfigured)", + verified_counted_for_copies, + ) + batch_st.reclassified_landed_paths.add(dest_path) # Invalidate derived caches for any landed/adopted row whose # bytes differ from what was there pre-scan. The batch scan # passes ``vireo_dir`` through, so scanner's own @@ -4318,33 +4242,28 @@ def _src_hash_cached( # the RAW row already carried ``companion_path`` for this # JPEG (see the accept branch below). See PR #1107 review. raw_companion_invalidations = set() - # NOTE: this stamping loop is deliberately NOT extracted — - # it hides divergence 11 (local-only file_hash backfill) - # and the zero-byte normalization split; it is unified in - # the spec's PR 6b (align-then-extract). See the decision - # table in the import-path-unification spec. - - def _rehash_dest_or_none(path): - """Re-hash the archive file, returning None on read failure. - - Used as the last-line check that the bytes currently at the - archive path still match what ``copy_and_hash_verify()`` - landed — necessary any time the scan-side hash is missing - (paired-JPEG row deletion) or NULL (scanner hashed the empty - zero-byte convention aside, a NULL means the archive read - failed between promote and scan). Without it, mutation of - the archive file between promote and scan would still be - accepted as success. - """ - try: - return _hash_dest_file(path, _stop_requested) - except DestReadCancelled: - raise - except OSError: - return None - - # Stamp the verified hashes in the integrity-audit vocabulary, - # cross-checked against what scan() stored. + # NOTE: this stamping loop is deliberately not yet + # extracted — the divergences it hid (spec decisions 11, + # D2/D3, D1, D5-D7) were resolved in this PR's earlier + # commits; the two loop bodies are now textually identical + # modulo ``dest_noun`` and are extracted next. See the + # decision table in the import-path-unification spec. + # + # Post-scan stamping: cross-check what scan() cataloged for + # every landed file against ``entry.verified_hash`` — the + # hash this import recorded when the bytes landed (copy-time + # hash locally, card-side hash remotely). Catalog integrity + # is checked in BOTH verify modes; the ``hash_status='ok'`` + # stamp is the byte-attestation and stays gated on + # ``verified_counted_for_copies`` (True locally, where every + # copy is hash-verified; ``params.verify_by_hash`` remotely + # — the wording says "recorded", not "verified", because a + # no-verify remote run computed the card-side hash but + # verified nothing destination-side). ``imported_photo_ids + # .add`` stays OUTSIDE that gate on every accept path — the + # after-import chaining hook builds its process job + # collection from these ids even on no-verify runs. + dest_noun = "archive" for entry in batch_st.landed: dest_path = entry.dest_path verified_hash = entry.verified_hash @@ -4352,154 +4271,147 @@ def _rehash_dest_or_none(path): """SELECT p.id, p.file_hash FROM photos p JOIN folders f ON f.id = p.folder_id WHERE f.path = ? AND p.filename = ?""", - (os.path.dirname(dest_path), os.path.basename(dest_path)), + (os.path.dirname(dest_path), + os.path.basename(dest_path)), ).fetchone() if row is None: - # RAW+JPEG pairing merges the JPEG's photo row into the - # RAW primary (companion_path); the JPEG's own row is - # gone by design and the bytes are represented on the - # RAW. But the pair lookup can't tell us the JPEG's - # archive bytes are still the ones we verified — the - # archive file could have been rewritten or corrupted - # between promote and the restricted scan. Re-read the - # archive path and require its hash to still equal - # ``verified_hash`` before counting the JPEG landed; - # otherwise reclassify to failed. See PR #1107 review. + # RAW+JPEG pairing merges the JPEG's photo row into + # the RAW primary (companion_path) and deletes the + # JPEG's own row, so a landed JPEG whose sibling RAW + # was scanned in the same batch legitimately has no + # row of its own. Look it up as another row's + # companion_path before deciding "not cataloged". + # See PR #1107/#1113 reviews. companion = db.conn.execute( """SELECT p.id FROM photos p JOIN folders f ON f.id = p.folder_id WHERE f.path = ? AND p.companion_path = ?""", - ( - os.path.dirname(dest_path), - os.path.basename(dest_path), - ), + (os.path.dirname(dest_path), + os.path.basename(dest_path)), ).fetchone() if companion is not None: + # The paired JPEG's own row is gone by design, + # so the row branch below can't cross-check its + # bytes. Re-read the destination file here in + # BOTH verify modes — paired JPEGs need the same + # stale-destination catalog-integrity guard or + # after-import processing would enqueue against + # the wrong companion. ``None`` from the re-read + # unambiguously means unreadable: + # ``verified_hash`` is never None (zero-byte + # normalizes to ``EMPTY_FILE_SHA256`` at + # ``_LandedFile`` construction), so an empty + # companion whose file vanished cannot be + # confused with a legitimately empty one. See + # PR #1107/#1113/#1437 reviews. try: - actual = _rehash_dest_or_none(dest_path) + actual = _rehash_dest_or_none( + dest_path, _stop_requested) except DestReadCancelled: state.cancelled = True break if actual is not None and actual == verified_hash: - # Landed JPEG paired with an existing RAW - # row. Invalidate the RAW's derived caches - # regardless of origin: adoption - # (``skipped_duplicate``) only proves the - # JPEG bytes were already at the archive + # Invalidate the RAW's derived caches + # regardless of origin: adoption only proves + # the JPEG bytes were already at the dest # path, NOT that the RAW row already carried # ``companion_path`` for this JPEG. A prior # partial run or backfill may have left the - # RAW as RAW-only (with a - # ``working_copy_path`` or - # ``working_copy_failed_at`` built without - # knowing this companion existed); the - # deferred end-of-run - # ``_extract_working_copies`` skips RAWs - # whose ``working_copy_path IS NOT NULL``, - # so a stale RAW-only cache would persist - # past this import and the UI would keep - # serving derived files for the pre-pair - # state. Fresh-copy JPEGs need this too - # (RAW may have been standalone or paired - # with a since-deleted companion). See PR - # #1107 review. + # RAW as RAW-only, and the deferred + # end-of-run ``_extract_working_copies`` + # skips rows whose ``working_copy_path IS + # NOT NULL`` — a stale RAW-only cache would + # persist past this import otherwise. See + # PR #1107 review. raw_companion_invalidations.add( companion["id"], ) - # The landed JPEG's bytes are now represented - # on the RAW primary — that row is what the + # The landed JPEG's bytes are represented on + # the RAW primary — that row is what the # chaining hook should process. state.imported_photo_ids.add(companion["id"]) continue _reclassify_landed_failed( state, rel, entry, - "paired companion archive bytes no longer " - "match the copy-time hash", + f"paired companion {dest_noun} bytes could " + "not be read" + if actual is None else + f"paired companion {dest_noun} bytes do not " + "match the hash this import recorded " + f"({dest_noun} base is likely stale or " + "misconfigured)", verified_counted_for_copies, ) batch_st.reclassified_landed_paths.add(dest_path) continue _reclassify_landed_failed( - state, rel, entry, "not cataloged after scan", + state, rel, entry, + "not cataloged after scan (no photo row)", verified_counted_for_copies, ) batch_st.reclassified_landed_paths.add(dest_path) continue if row["file_hash"] == verified_hash: - db.update_photo_hash_check( - row["id"], "ok", commit=False, - ) + if verified_counted_for_copies: + db.update_photo_hash_check( + row["id"], "ok", commit=False, + ) state.imported_photo_ids.add(row["id"]) elif row["file_hash"] is None: - if verified_hash == EMPTY_FILE_SHA256: - # Zero-byte convention: EMPTY_FILE_SHA256 never - # lands in file_hash (it would collide with every - # other empty file), so a NULL row hash is - # expected here — but do not stamp blind. Re-read - # the archive path so an empty file that vanished - # or became unreadable between scan and stamping - # fails instead of keeping hash_status='ok' with - # safe_to_format green over a missing file. - # Mirrors the remote loop's zero-byte re-read - # (PR #1437 review, Codex P1 r3741224300 — the - # remote fold inherited this hole FROM this - # branch, and the fix landed remote-only). - try: - actual = _rehash_dest_or_none(dest_path) - except DestReadCancelled: - state.cancelled = True - break - if actual == EMPTY_FILE_SHA256: - db.update_photo_hash_check( - row["id"], "ok", commit=False, - ) - state.imported_photo_ids.add(row["id"]) - else: - _reclassify_landed_failed( - state, rel, entry, - "scan wrote no archive row hash and a " - "re-read of the archive file disagrees " - "with the copy-time hash (archive file " - "vanished, changed, or is unreadable)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(dest_path) - else: - # Non-empty file with NULL file_hash after scan - # means scanner._compute_file_features couldn't - # read the archive file (unreadable between - # promote and scan). Trusting the copy-time hash - # here would flip ``safe_to_format`` green for - # bytes we can't currently verify on disk. Re- - # hash the archive path from here as a last check - # — if that also fails or disagrees with our - # copy-time hash, reclassify to failed instead of - # stamping a stale value. See PR #1107 review. - try: - actual = _rehash_dest_or_none(dest_path) - except DestReadCancelled: - state.cancelled = True - break - if actual is not None and actual == verified_hash: + # scan() legitimately writes NULL for zero-byte + # files (the convention keeps ``EMPTY_FILE_SHA256`` + # out of ``photos.file_hash``) and can leave NULL + # when its own read failed (large files, prior + # partial scan, or a suppressed read error). Don't + # stamp blind either way: re-read the destination + # file as the last-line check so a file that + # vanished, changed, or became unreadable between + # scan and stamping fails instead of keeping + # ``hash_status='ok'`` with ``safe_to_format`` + # green. See PR #1107/#1113/#1437 reviews. + try: + actual = _rehash_dest_or_none( + dest_path, _stop_requested) + except DestReadCancelled: + state.cancelled = True + break + if actual is not None and actual == verified_hash: + # Backfill the row hash scan couldn't write + # (spec decision 11) — except the zero-byte + # case, where ``file_hash=None`` hits + # ``update_photo_hash_check``'s status-only arm + # and the row keeps scan's NULL (backfilling + # EMPTY would recreate the every-empty-file + # collision the convention exists to prevent). + if verified_counted_for_copies: db.update_photo_hash_check( - row["id"], "ok", file_hash=verified_hash, + row["id"], "ok", + file_hash=( + verified_hash + if verified_hash != EMPTY_FILE_SHA256 + else None + ), commit=False, ) - state.imported_photo_ids.add(row["id"]) - else: - _reclassify_landed_failed( - state, rel, entry, - "archive file unhashable after copy " - "verification (scan wrote no hash and " - "re-hash disagrees)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(dest_path) + state.imported_photo_ids.add(row["id"]) + else: + _reclassify_landed_failed( + state, rel, entry, + f"scan wrote no {dest_noun} row hash and a " + f"re-read of the {dest_noun} file disagrees " + "with the hash this import recorded " + f"({dest_noun} file is likely stale, " + "unreadable, or misconfigured)", + verified_counted_for_copies, + ) + batch_st.reclassified_landed_paths.add(dest_path) else: _reclassify_landed_failed( state, rel, entry, - "destination changed between copy verification and " - "catalog scan (hash mismatch)", + f"scanned {dest_noun} row hash does not match " + "the hash this import recorded " + f"({dest_noun} base is likely stale or " + "misconfigured)", verified_counted_for_copies, ) batch_st.reclassified_landed_paths.add(dest_path) diff --git a/vireo/tests/test_import_job.py b/vireo/tests/test_import_job.py index ec4c3a7ac..cf741f714 100644 --- a/vireo/tests/test_import_job.py +++ b/vireo/tests/test_import_job.py @@ -6964,7 +6964,7 @@ def fake_rsync_wrong_bytes( assert result["safe_to_format"] is False, result assert any( "paired companion mount bytes" in u["reason"] - and "source hash" in u["reason"] + and "hash this import recorded" in u["reason"] for u in result["unsafe_files"] ), result["unsafe_files"] From acdc22b759345bede722ed8c0111d5465b2778d7 Mon Sep 17 00:00:00 2001 From: Julius Simonelli Date: Sun, 9 Aug 2026 23:09:01 +0200 Subject: [PATCH 7/7] Extract the unified stamping loop (last shared logic; transport cores remain) Move-only: _stamp_landed_and_validate_catalog(state, batch_st, db, params, rel, *, attests_bytes, dest_noun, stop_requested) returns raw_companion_invalidations; both import functions call it (dest_noun="mount"/"archive", attests_bytes=verified_counted_for_copies, stop_requested=_stop_requested) and pass the result into _invalidate_changed_and_sweep unchanged. Both loop bodies and the "deliberately not yet extracted" NOTE comments are deleted. Move-only proof: the pre-extraction loop body equals the function body modulo an 8-space dedent and the parameter renames verified_counted_for_copies -> attests_bytes, _stop_requested -> stop_requested (verified by normalized diff: empty). The docstring carries the three invariants: stamps gated on attests_bytes; imported_photo_ids.add on every accept path OUTSIDE the gate; the decision-11 backfill excludes EMPTY_FILE_SHA256. Full file: 231 passed, 1 skipped. Parity net (-k "behavior or agree_on_plain or adoption or renamed_twin or zero_byte or dest_read_cancel or mount_detach or pairs"): 29 passed. Co-Authored-By: Claude Fable 5 --- vireo/import_job.py | 556 +++++++++++++++++--------------------------- 1 file changed, 207 insertions(+), 349 deletions(-) diff --git a/vireo/import_job.py b/vireo/import_job.py index d36a31406..3f9cc85e5 100644 --- a/vireo/import_job.py +++ b/vireo/import_job.py @@ -1369,6 +1369,198 @@ def _catalog_scan_and_prescan(state, batch_st, db, params, scan, destination, return pre_scan_hashes + +def _stamp_landed_and_validate_catalog(state, batch_st, db, params, rel, *, + attests_bytes, dest_noun, + stop_requested): + """Cross-check every landed file against what scan() cataloged and + stamp the integrity verdicts; returns the RAW photo ids whose + derived caches need invalidation because a landed JPEG became (or + already was) their companion (the caller passes them to + ``_invalidate_changed_and_sweep``). + + ``entry.verified_hash`` is the hash this import recorded when the + bytes landed (copy-time hash locally, card-side hash remotely). + Catalog integrity is checked in BOTH verify modes; the wording of + the failure reasons says "recorded", not "verified", because a + no-verify remote run computed the card-side hash but verified + nothing destination-side. + + Invariants (spec PR 6b — do not regress): + + - The ``hash_status='ok'`` stamps run ONLY behind ``attests_bytes`` + (== each caller's ``verified_counted_for_copies``: ``True`` + locally, where every copy is hash-verified; + ``params.verify_by_hash`` remotely) — the stamp is the + independent byte-attestation, not just "scan and import agree". + - ``state.imported_photo_ids.add`` runs on EVERY accept path and + stays OUTSIDE that gate: the after-import chaining hook builds + its process job collection from these ids even on no-verify runs. + - The scan-NULL backfill (spec decision 11) excludes + ``EMPTY_FILE_SHA256``: zero-byte accepts pass ``file_hash=None`` + (``update_photo_hash_check``'s status-only arm), keeping scan's + NULL — backfilling EMPTY would recreate the every-empty-file + collision the zero-byte convention exists to prevent. + + ``dest_noun`` ("archive" locally, "mount" remotely) parameterizes + the user-visible failure reasons; ``stop_requested`` is the + caller's cancellation poll, threaded into the destination re-reads. + """ + raw_companion_invalidations = set() + for entry in batch_st.landed: + dest_path = entry.dest_path + verified_hash = entry.verified_hash + row = db.conn.execute( + """SELECT p.id, p.file_hash FROM photos p + JOIN folders f ON f.id = p.folder_id + WHERE f.path = ? AND p.filename = ?""", + (os.path.dirname(dest_path), + os.path.basename(dest_path)), + ).fetchone() + if row is None: + # RAW+JPEG pairing merges the JPEG's photo row into + # the RAW primary (companion_path) and deletes the + # JPEG's own row, so a landed JPEG whose sibling RAW + # was scanned in the same batch legitimately has no + # row of its own. Look it up as another row's + # companion_path before deciding "not cataloged". + # See PR #1107/#1113 reviews. + companion = db.conn.execute( + """SELECT p.id FROM photos p + JOIN folders f ON f.id = p.folder_id + WHERE f.path = ? AND p.companion_path = ?""", + (os.path.dirname(dest_path), + os.path.basename(dest_path)), + ).fetchone() + if companion is not None: + # The paired JPEG's own row is gone by design, + # so the row branch below can't cross-check its + # bytes. Re-read the destination file here in + # BOTH verify modes — paired JPEGs need the same + # stale-destination catalog-integrity guard or + # after-import processing would enqueue against + # the wrong companion. ``None`` from the re-read + # unambiguously means unreadable: + # ``verified_hash`` is never None (zero-byte + # normalizes to ``EMPTY_FILE_SHA256`` at + # ``_LandedFile`` construction), so an empty + # companion whose file vanished cannot be + # confused with a legitimately empty one. See + # PR #1107/#1113/#1437 reviews. + try: + actual = _rehash_dest_or_none( + dest_path, stop_requested) + except DestReadCancelled: + state.cancelled = True + break + if actual is not None and actual == verified_hash: + # Invalidate the RAW's derived caches + # regardless of origin: adoption only proves + # the JPEG bytes were already at the dest + # path, NOT that the RAW row already carried + # ``companion_path`` for this JPEG. A prior + # partial run or backfill may have left the + # RAW as RAW-only, and the deferred + # end-of-run ``_extract_working_copies`` + # skips rows whose ``working_copy_path IS + # NOT NULL`` — a stale RAW-only cache would + # persist past this import otherwise. See + # PR #1107 review. + raw_companion_invalidations.add( + companion["id"], + ) + # The landed JPEG's bytes are represented on + # the RAW primary — that row is what the + # chaining hook should process. + state.imported_photo_ids.add(companion["id"]) + continue + _reclassify_landed_failed( + state, rel, entry, + f"paired companion {dest_noun} bytes could " + "not be read" + if actual is None else + f"paired companion {dest_noun} bytes do not " + "match the hash this import recorded " + f"({dest_noun} base is likely stale or " + "misconfigured)", + attests_bytes, + ) + batch_st.reclassified_landed_paths.add(dest_path) + continue + _reclassify_landed_failed( + state, rel, entry, + "not cataloged after scan (no photo row)", + attests_bytes, + ) + batch_st.reclassified_landed_paths.add(dest_path) + continue + if row["file_hash"] == verified_hash: + if attests_bytes: + db.update_photo_hash_check( + row["id"], "ok", commit=False, + ) + state.imported_photo_ids.add(row["id"]) + elif row["file_hash"] is None: + # scan() legitimately writes NULL for zero-byte + # files (the convention keeps ``EMPTY_FILE_SHA256`` + # out of ``photos.file_hash``) and can leave NULL + # when its own read failed (large files, prior + # partial scan, or a suppressed read error). Don't + # stamp blind either way: re-read the destination + # file as the last-line check so a file that + # vanished, changed, or became unreadable between + # scan and stamping fails instead of keeping + # ``hash_status='ok'`` with ``safe_to_format`` + # green. See PR #1107/#1113/#1437 reviews. + try: + actual = _rehash_dest_or_none( + dest_path, stop_requested) + except DestReadCancelled: + state.cancelled = True + break + if actual is not None and actual == verified_hash: + # Backfill the row hash scan couldn't write + # (spec decision 11) — except the zero-byte + # case, where ``file_hash=None`` hits + # ``update_photo_hash_check``'s status-only arm + # and the row keeps scan's NULL (backfilling + # EMPTY would recreate the every-empty-file + # collision the convention exists to prevent). + if attests_bytes: + db.update_photo_hash_check( + row["id"], "ok", + file_hash=( + verified_hash + if verified_hash != EMPTY_FILE_SHA256 + else None + ), + commit=False, + ) + state.imported_photo_ids.add(row["id"]) + else: + _reclassify_landed_failed( + state, rel, entry, + f"scan wrote no {dest_noun} row hash and a " + f"re-read of the {dest_noun} file disagrees " + "with the hash this import recorded " + f"({dest_noun} file is likely stale, " + "unreadable, or misconfigured)", + attests_bytes, + ) + batch_st.reclassified_landed_paths.add(dest_path) + else: + _reclassify_landed_failed( + state, rel, entry, + f"scanned {dest_noun} row hash does not match " + "the hash this import recorded " + f"({dest_noun} base is likely stale or " + "misconfigured)", + attests_bytes, + ) + batch_st.reclassified_landed_paths.add(dest_path) + return raw_companion_invalidations + + def _invalidate_changed_and_sweep(state, batch_st, db, params, pre_scan_hashes, raw_companion_invalidations): @@ -3547,180 +3739,12 @@ def _rsync_cancelled(rc): # the JPEG bytes pre-existed on the mount, not that the RAW # already carried companion_path) and invalidate below. # Mirrors the local path — spec decision 6. - raw_companion_invalidations = set() - # NOTE: this stamping loop is deliberately not yet - # extracted — the divergences it hid (spec decisions 11, - # D2/D3, D1, D5-D7) were resolved in this PR's earlier - # commits; the two loop bodies are now textually identical - # modulo ``dest_noun`` and are extracted next. See the - # decision table in the import-path-unification spec. - # - # Post-scan stamping: cross-check what scan() cataloged for - # every landed file against ``entry.verified_hash`` — the - # hash this import recorded when the bytes landed (copy-time - # hash locally, card-side hash remotely). Catalog integrity - # is checked in BOTH verify modes; the ``hash_status='ok'`` - # stamp is the byte-attestation and stays gated on - # ``verified_counted_for_copies`` (True locally, where every - # copy is hash-verified; ``params.verify_by_hash`` remotely - # — the wording says "recorded", not "verified", because a - # no-verify remote run computed the card-side hash but - # verified nothing destination-side). ``imported_photo_ids - # .add`` stays OUTSIDE that gate on every accept path — the - # after-import chaining hook builds its process job - # collection from these ids even on no-verify runs. - dest_noun = "mount" - for entry in batch_st.landed: - dest_path = entry.dest_path - verified_hash = entry.verified_hash - row = db.conn.execute( - """SELECT p.id, p.file_hash FROM photos p - JOIN folders f ON f.id = p.folder_id - WHERE f.path = ? AND p.filename = ?""", - (os.path.dirname(dest_path), - os.path.basename(dest_path)), - ).fetchone() - if row is None: - # RAW+JPEG pairing merges the JPEG's photo row into - # the RAW primary (companion_path) and deletes the - # JPEG's own row, so a landed JPEG whose sibling RAW - # was scanned in the same batch legitimately has no - # row of its own. Look it up as another row's - # companion_path before deciding "not cataloged". - # See PR #1107/#1113 reviews. - companion = db.conn.execute( - """SELECT p.id FROM photos p - JOIN folders f ON f.id = p.folder_id - WHERE f.path = ? AND p.companion_path = ?""", - (os.path.dirname(dest_path), - os.path.basename(dest_path)), - ).fetchone() - if companion is not None: - # The paired JPEG's own row is gone by design, - # so the row branch below can't cross-check its - # bytes. Re-read the destination file here in - # BOTH verify modes — paired JPEGs need the same - # stale-destination catalog-integrity guard or - # after-import processing would enqueue against - # the wrong companion. ``None`` from the re-read - # unambiguously means unreadable: - # ``verified_hash`` is never None (zero-byte - # normalizes to ``EMPTY_FILE_SHA256`` at - # ``_LandedFile`` construction), so an empty - # companion whose file vanished cannot be - # confused with a legitimately empty one. See - # PR #1107/#1113/#1437 reviews. - try: - actual = _rehash_dest_or_none( - dest_path, _stop_requested) - except DestReadCancelled: - state.cancelled = True - break - if actual is not None and actual == verified_hash: - # Invalidate the RAW's derived caches - # regardless of origin: adoption only proves - # the JPEG bytes were already at the dest - # path, NOT that the RAW row already carried - # ``companion_path`` for this JPEG. A prior - # partial run or backfill may have left the - # RAW as RAW-only, and the deferred - # end-of-run ``_extract_working_copies`` - # skips rows whose ``working_copy_path IS - # NOT NULL`` — a stale RAW-only cache would - # persist past this import otherwise. See - # PR #1107 review. - raw_companion_invalidations.add( - companion["id"], - ) - # The landed JPEG's bytes are represented on - # the RAW primary — that row is what the - # chaining hook should process. - state.imported_photo_ids.add(companion["id"]) - continue - _reclassify_landed_failed( - state, rel, entry, - f"paired companion {dest_noun} bytes could " - "not be read" - if actual is None else - f"paired companion {dest_noun} bytes do not " - "match the hash this import recorded " - f"({dest_noun} base is likely stale or " - "misconfigured)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(dest_path) - continue - _reclassify_landed_failed( - state, rel, entry, - "not cataloged after scan (no photo row)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(dest_path) - continue - if row["file_hash"] == verified_hash: - if verified_counted_for_copies: - db.update_photo_hash_check( - row["id"], "ok", commit=False, - ) - state.imported_photo_ids.add(row["id"]) - elif row["file_hash"] is None: - # scan() legitimately writes NULL for zero-byte - # files (the convention keeps ``EMPTY_FILE_SHA256`` - # out of ``photos.file_hash``) and can leave NULL - # when its own read failed (large files, prior - # partial scan, or a suppressed read error). Don't - # stamp blind either way: re-read the destination - # file as the last-line check so a file that - # vanished, changed, or became unreadable between - # scan and stamping fails instead of keeping - # ``hash_status='ok'`` with ``safe_to_format`` - # green. See PR #1107/#1113/#1437 reviews. - try: - actual = _rehash_dest_or_none( - dest_path, _stop_requested) - except DestReadCancelled: - state.cancelled = True - break - if actual is not None and actual == verified_hash: - # Backfill the row hash scan couldn't write - # (spec decision 11) — except the zero-byte - # case, where ``file_hash=None`` hits - # ``update_photo_hash_check``'s status-only arm - # and the row keeps scan's NULL (backfilling - # EMPTY would recreate the every-empty-file - # collision the convention exists to prevent). - if verified_counted_for_copies: - db.update_photo_hash_check( - row["id"], "ok", - file_hash=( - verified_hash - if verified_hash != EMPTY_FILE_SHA256 - else None - ), - commit=False, - ) - state.imported_photo_ids.add(row["id"]) - else: - _reclassify_landed_failed( - state, rel, entry, - f"scan wrote no {dest_noun} row hash and a " - f"re-read of the {dest_noun} file disagrees " - "with the hash this import recorded " - f"({dest_noun} file is likely stale, " - "unreadable, or misconfigured)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(dest_path) - else: - _reclassify_landed_failed( - state, rel, entry, - f"scanned {dest_noun} row hash does not match " - "the hash this import recorded " - f"({dest_noun} base is likely stale or " - "misconfigured)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(dest_path) + raw_companion_invalidations = _stamp_landed_and_validate_catalog( + state, batch_st, db, params, rel, + attests_bytes=verified_counted_for_copies, + dest_noun="mount", + stop_requested=_stop_requested, + ) # Invalidate derived caches for any landed/adopted row whose # bytes differ from what was there pre-scan. The batch scan # passes ``vireo_dir`` through, so scanner's own @@ -4240,181 +4264,15 @@ def _src_hash_cached( # adoption (``origin == "skipped_duplicate"``) only proves # the JPEG bytes were already at the archive path, NOT that # the RAW row already carried ``companion_path`` for this - # JPEG (see the accept branch below). See PR #1107 review. - raw_companion_invalidations = set() - # NOTE: this stamping loop is deliberately not yet - # extracted — the divergences it hid (spec decisions 11, - # D2/D3, D1, D5-D7) were resolved in this PR's earlier - # commits; the two loop bodies are now textually identical - # modulo ``dest_noun`` and are extracted next. See the - # decision table in the import-path-unification spec. - # - # Post-scan stamping: cross-check what scan() cataloged for - # every landed file against ``entry.verified_hash`` — the - # hash this import recorded when the bytes landed (copy-time - # hash locally, card-side hash remotely). Catalog integrity - # is checked in BOTH verify modes; the ``hash_status='ok'`` - # stamp is the byte-attestation and stays gated on - # ``verified_counted_for_copies`` (True locally, where every - # copy is hash-verified; ``params.verify_by_hash`` remotely - # — the wording says "recorded", not "verified", because a - # no-verify remote run computed the card-side hash but - # verified nothing destination-side). ``imported_photo_ids - # .add`` stays OUTSIDE that gate on every accept path — the - # after-import chaining hook builds its process job - # collection from these ids even on no-verify runs. - dest_noun = "archive" - for entry in batch_st.landed: - dest_path = entry.dest_path - verified_hash = entry.verified_hash - row = db.conn.execute( - """SELECT p.id, p.file_hash FROM photos p - JOIN folders f ON f.id = p.folder_id - WHERE f.path = ? AND p.filename = ?""", - (os.path.dirname(dest_path), - os.path.basename(dest_path)), - ).fetchone() - if row is None: - # RAW+JPEG pairing merges the JPEG's photo row into - # the RAW primary (companion_path) and deletes the - # JPEG's own row, so a landed JPEG whose sibling RAW - # was scanned in the same batch legitimately has no - # row of its own. Look it up as another row's - # companion_path before deciding "not cataloged". - # See PR #1107/#1113 reviews. - companion = db.conn.execute( - """SELECT p.id FROM photos p - JOIN folders f ON f.id = p.folder_id - WHERE f.path = ? AND p.companion_path = ?""", - (os.path.dirname(dest_path), - os.path.basename(dest_path)), - ).fetchone() - if companion is not None: - # The paired JPEG's own row is gone by design, - # so the row branch below can't cross-check its - # bytes. Re-read the destination file here in - # BOTH verify modes — paired JPEGs need the same - # stale-destination catalog-integrity guard or - # after-import processing would enqueue against - # the wrong companion. ``None`` from the re-read - # unambiguously means unreadable: - # ``verified_hash`` is never None (zero-byte - # normalizes to ``EMPTY_FILE_SHA256`` at - # ``_LandedFile`` construction), so an empty - # companion whose file vanished cannot be - # confused with a legitimately empty one. See - # PR #1107/#1113/#1437 reviews. - try: - actual = _rehash_dest_or_none( - dest_path, _stop_requested) - except DestReadCancelled: - state.cancelled = True - break - if actual is not None and actual == verified_hash: - # Invalidate the RAW's derived caches - # regardless of origin: adoption only proves - # the JPEG bytes were already at the dest - # path, NOT that the RAW row already carried - # ``companion_path`` for this JPEG. A prior - # partial run or backfill may have left the - # RAW as RAW-only, and the deferred - # end-of-run ``_extract_working_copies`` - # skips rows whose ``working_copy_path IS - # NOT NULL`` — a stale RAW-only cache would - # persist past this import otherwise. See - # PR #1107 review. - raw_companion_invalidations.add( - companion["id"], - ) - # The landed JPEG's bytes are represented on - # the RAW primary — that row is what the - # chaining hook should process. - state.imported_photo_ids.add(companion["id"]) - continue - _reclassify_landed_failed( - state, rel, entry, - f"paired companion {dest_noun} bytes could " - "not be read" - if actual is None else - f"paired companion {dest_noun} bytes do not " - "match the hash this import recorded " - f"({dest_noun} base is likely stale or " - "misconfigured)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(dest_path) - continue - _reclassify_landed_failed( - state, rel, entry, - "not cataloged after scan (no photo row)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(dest_path) - continue - if row["file_hash"] == verified_hash: - if verified_counted_for_copies: - db.update_photo_hash_check( - row["id"], "ok", commit=False, - ) - state.imported_photo_ids.add(row["id"]) - elif row["file_hash"] is None: - # scan() legitimately writes NULL for zero-byte - # files (the convention keeps ``EMPTY_FILE_SHA256`` - # out of ``photos.file_hash``) and can leave NULL - # when its own read failed (large files, prior - # partial scan, or a suppressed read error). Don't - # stamp blind either way: re-read the destination - # file as the last-line check so a file that - # vanished, changed, or became unreadable between - # scan and stamping fails instead of keeping - # ``hash_status='ok'`` with ``safe_to_format`` - # green. See PR #1107/#1113/#1437 reviews. - try: - actual = _rehash_dest_or_none( - dest_path, _stop_requested) - except DestReadCancelled: - state.cancelled = True - break - if actual is not None and actual == verified_hash: - # Backfill the row hash scan couldn't write - # (spec decision 11) — except the zero-byte - # case, where ``file_hash=None`` hits - # ``update_photo_hash_check``'s status-only arm - # and the row keeps scan's NULL (backfilling - # EMPTY would recreate the every-empty-file - # collision the convention exists to prevent). - if verified_counted_for_copies: - db.update_photo_hash_check( - row["id"], "ok", - file_hash=( - verified_hash - if verified_hash != EMPTY_FILE_SHA256 - else None - ), - commit=False, - ) - state.imported_photo_ids.add(row["id"]) - else: - _reclassify_landed_failed( - state, rel, entry, - f"scan wrote no {dest_noun} row hash and a " - f"re-read of the {dest_noun} file disagrees " - "with the hash this import recorded " - f"({dest_noun} file is likely stale, " - "unreadable, or misconfigured)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(dest_path) - else: - _reclassify_landed_failed( - state, rel, entry, - f"scanned {dest_noun} row hash does not match " - "the hash this import recorded " - f"({dest_noun} base is likely stale or " - "misconfigured)", - verified_counted_for_copies, - ) - batch_st.reclassified_landed_paths.add(dest_path) + # JPEG (see the companion accept branch in + # ``_stamp_landed_and_validate_catalog``). See PR #1107 + # review. + raw_companion_invalidations = _stamp_landed_and_validate_catalog( + state, batch_st, db, params, rel, + attests_bytes=verified_counted_for_copies, + dest_noun="archive", + stop_requested=_stop_requested, + ) # Invalidate derived caches for any landed row whose bytes # differ from what was there pre-scan. The batch scan passes