Import unification PR 6b: unify and extract the stamping loop (spec decision 11)

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.

docs/superpowers/plans/2026-08-09-import-unify-pr6b-stamping.md

@@ -0,0 +1,68 @@
+# 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 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)` (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 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 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 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).
+
+### 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. 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 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)
+- [ ] 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.