fix(photo-exif): caption pass must preserve face enrichment

connectors/photo-exif/README.md

@@ -117,12 +117,23 @@ powershell -File prep-takeout.ps1 -NoScan   # unzip + recycle only (skip the ing
 
 **Recycle Bin, never permanent delete** — recoverable, and matching this project's append-only ethos and the box's delete-blocked posture (the `rm`/`Remove-Item` deny); it uses `Microsoft.VisualBasic.FileIO.FileSystem.DeleteFile(SendToRecycleBin)`, not `Remove-Item`. **Caveat:** the Recycle Bin still occupies disk until emptied — to actually reclaim the space after a verified run, empty it manually (the script can't, since permanent delete is blocked). Windows-only.
 
-### Wave order matters
+### Wave order — either order is safe, one at a time (#276)
 
-Run **scan → caption → face** for each photo. The ingest contract requires `text_repr` on every upsert and replaces it (and `extra`) wholesale — there is no deep-merge (doc 04 §3). The face worker therefore reconstructs `text_repr` as *base + caption + "Pictured: …"* by reading the caption cache, so it preserves a caption rather than dropping it. Two ordering caveats follow from the wholesale replace:
+Run `scan.js` first (it creates the artifacts). After that, **the caption and face passes may run in either order** — whichever runs second preserves the first one's work.
 
-- If the face worker runs on a photo whose caption isn't in its local cache yet (e.g. the caption was written on a different machine), it will write base-only text and overwrite the server-side caption. Keep the caption cache co-located with the face worker.
-- A caption run that lands *after* a face `label` on the same photo drops the "Pictured: …" sentence (the entity link itself is unaffected — that's the primary recall path). In practice each worker processes a photo once (file-state gated), so this only bites if a photo is labeled before it is ever captioned.
+**Run them one at a time, not concurrently.** Each worker snapshots the other's state once at startup, so a face `label` applied *while* a caption run is in flight won't be seen by that run, and the caption upsert will overwrite it with the older picture. Sequential runs in either order are safe; overlapping runs are not.
+
+The hazard this protects against: the ingest contract requires `text_repr` on every upsert and replaces it (and `extra`) **wholesale** — there is no deep-merge (doc 04 §3). So whichever enrichment pass runs second overwrites the first one's contribution unless both send the *union* of what's known.
+
+Both workers therefore build their payload through the one shared builder in **`lib/photo-payload.js`**:
+
+- The **face worker** reads the caption cache and rebuilds *base + caption + "Pictured: …"*.
+- The **caption worker** reads the face state + clusters files and carries `faces_detected` / `pictured` / the "Pictured: …" sentence through.
+
+Two properties worth knowing:
+
+- **Absent is not zero.** If the face pass hasn't touched a photo, the caption worker omits `faces_detected` and `pictured` entirely rather than sending `0`/`[]` — sending `0` would assert "detection ran and found nothing," which would be false and, thanks to the wholesale replace, permanent.
+- **Keep both state directories co-located with the workers.** They read each other's state from `~/.life-context/` (overridable via `PHOTO_EXIF_CAPTION_STATE_PATH`, `PHOTO_EXIF_FACE_STATE_PATH`, `PHOTO_EXIF_FACE_CLUSTERS_PATH`). A worker that can't see the other's state falls back to writing only what it knows — correct, but it will drop the other's contribution, which is the pre-#276 behavior. Running the two passes on *different machines* against the same server is therefore still unsupported.
 
 ### Nightly-window scheduling (config, not code)
 
@@ -168,6 +179,7 @@ On Windows, use Task Scheduler with a "Daily, 1:00 AM" trigger running `node cap
 - `lib/shared.js` — env loading, media walk (`walkImageFiles`/`walkMediaFiles`), the content-hash keying resolver (`keyForMedia`), `mediaType`, ingest client, contact-photos fetch (`suggest-labels`)
 - `lib/describe.js` — shared EXIF + Takeout-sidecar description logic (`describePhoto`/`readSidecar`/`sidecarPathFor`), used by every script so they can never drift
 - `lib/caption-cache.js` — caption state (relPath→text map) + `currentTextRepr`, shared by caption + face workers
+- `lib/photo-payload.js` — the single ingest-payload builder both enrichment workers use, so neither clobbers the other's `text_repr`/`extra` (#276); also reads the face state into a `relPath → {faces, pictured}` lookup for the caption worker
 - `lib/face-cluster.js` — pure, IO-free descriptor clustering (euclidean, nearest-centroid, (de)serialization)
 - `lib/face-align.js` — pure, IO-free SCRFD decode (distance2bbox/kps, anchor centers, NMS) + Umeyama similarity/bilinear warp used to align each 112x112 ArcFace input crop (#268)
 - `lib/face-detect.js` — lazy ML-model detector + the test fixture detector

connectors/photo-exif/caption-worker.js

@@ -14,8 +14,9 @@ import os from 'node:os';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { loadDotEnvIfPresent, walkImageFiles, keyForMedia, isTakeoutRoot, contentHashOfFile, ingestClient } from './lib/shared.js';
-import { describePhoto, buildTextRepr } from './lib/describe.js';
+import { describePhoto } from './lib/describe.js';
 import { readCaptionCache, writeCaptionCache } from './lib/caption-cache.js';
+import { buildPhotoPayload, readFaceEnrichment } from './lib/photo-payload.js';
 
 loadDotEnvIfPresent(path.dirname(fileURLToPath(import.meta.url)));
 
@@ -30,8 +31,15 @@ const VLM_PROMPT = process.env.VLM_PROMPT
 // "not set" (NaN) from "set to zero" (a real, useful value for tests and manual runs).
 const rawThrottle = Number(process.env.VLM_THROTTLE_MS);
 const VLM_THROTTLE_MS = Number.isFinite(rawThrottle) ? rawThrottle : 2000;
-const STATE_PATH = process.env.PHOTO_EXIF_CAPTION_STATE_PATH
-  || path.join(os.homedir(), '.life-context', 'photo-exif-captions.json');
+const HOME_STATE = (name) => path.join(os.homedir(), '.life-context', name);
+const STATE_PATH = process.env.PHOTO_EXIF_CAPTION_STATE_PATH || HOME_STATE('photo-exif-captions.json');
+// The face worker's state, read (never written) so a caption upsert carries face enrichment
+// through instead of wiping it — `extra` is replaced wholesale by ingest (#276). Same env vars and
+// defaults as face-worker.js; absent files just mean the face pass hasn't run yet.
+const FACE_STATE_PATH = process.env.PHOTO_EXIF_FACE_STATE_PATH || HOME_STATE('photo-exif-faces.json');
+const CLUSTERS_PATH = process.env.PHOTO_EXIF_FACE_CLUSTERS_PATH || HOME_STATE('photo-exif-face-clusters.json');
+const confRaw = Number(process.env.FACE_HINT_CONFIDENCE);
+const FACE_HINT_CONFIDENCE = Number.isFinite(confRaw) ? confRaw : 0.6;
 
 async function caption(base64Image) {
   const res = await fetch(`${VLM_BASE_URL}/api/generate`, {
@@ -72,6 +80,8 @@ async function main() {
   // "Pictured: ..." (lib/caption-cache.js). A present key means "already captioned" — same
   // skip semantics as the old Set, but the text is retained now.
   const captionCache = readCaptionCache(STATE_PATH);
+  // Read once, not per photo — the face pass is finished (or absent) by the time this runs.
+  const faceEnrichment = readFaceEnrichment(FACE_STATE_PATH, CLUSTERS_PATH);
   let done = 0;
   let vlmDown = false;
 
@@ -107,22 +117,29 @@ async function main() {
       break;
     }
 
-    const enrichedText = `${buildTextRepr(dateStr, path.basename(absPath))} ${captionText}`;
     // Same content-hash key scan.js computed (keyForMedia) so this enriches the SAME artifact —
     // a Takeout-export photo keys under source='google-photos', everything else under 'photo-exif'.
     const { source, source_id } = keyForMedia(contentHash, isTakeout);
+    // Carry through whatever the face pass already found for this photo. Absent (face pass never
+    // ran) leaves faces_detected/pictured off the payload entirely rather than sending 0/[] — see
+    // lib/photo-payload.js. This is what makes caption/face order-independent (#276).
+    const face = faceEnrichment.get(relPath);
     try {
-      // Present fields only: text_repr + extra. Per doc 04 §3 upsert merge semantics, everything
-      // scan.js already stored (occurred_at, GPS, raw_path, content_hash) — plus whatever core
-      // resolved into place_label from that GPS — is left untouched, since neither is present
-      // in this payload. This is exactly the "enrichment wave" the contract's upsert exists for.
-      await postIngest({
+      // Present fields only: text_repr + extra (+ hints when someone is pictured). Per doc 04 §3
+      // upsert merge semantics, everything scan.js already stored (occurred_at, GPS, raw_path,
+      // content_hash) — plus whatever core resolved into place_label from that GPS — is left
+      // untouched, since none of it is present in this payload. This is exactly the "enrichment
+      // wave" the contract's upsert exists for.
+      await postIngest(buildPhotoPayload({
         source,
         source_id,
-        type: 'photo',
-        text_repr: enrichedText,
-        extra: { captioned: true },
-      });
+        dateStr,
+        filename: path.basename(absPath),
+        caption: captionText,
+        faces: face?.faces,
+        pictured: face?.pictured,
+        hintConfidence: FACE_HINT_CONFIDENCE,
+      }));
       captionCache[relPath] = captionText;
       writeCaptionCache(STATE_PATH, captionCache); // after every success, not batched — kill-safe
       done++;

connectors/photo-exif/face-worker.js

@@ -23,8 +23,9 @@ import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { loadDotEnvIfPresent, walkImageFiles, keyForMedia, isTakeoutRoot, contentHashOfFile, ingestClient, fetchContactPhotos } from './lib/shared.js';
 import { describePhoto, readSidecar } from './lib/describe.js';
-import { readCaptionCache, currentTextRepr } from './lib/caption-cache.js';
+import { readCaptionCache } from './lib/caption-cache.js';
 import { assignCluster, euclideanDistance, parseClustersFile, serializeClustersFile } from './lib/face-cluster.js';
+import { buildPhotoPayload, picturedNames } from './lib/photo-payload.js';
 import { resolveDetector } from './lib/face-detect.js';
 
 const CONNECTOR_DIR = path.dirname(fileURLToPath(import.meta.url));
@@ -97,38 +98,22 @@ function saveClusters(state) {
   writeFileSync(CLUSTERS_PATH, serializeClustersFile(state.version, state.clusters));
 }
 
-// The pictured names on a photo = the labels of the (distinct) clusters its faces fall into.
-function picturedNames(clusterIds, clustersById) {
-  const names = new Set();
-  for (const id of clusterIds) {
-    const label = clustersById.get(id)?.label;
-    if (label) names.add(label);
-  }
-  return [...names].sort();
-}
-
 // Build the enrichment payload for one photo from its stored face entry + current cluster labels.
-// text_repr is REQUIRED by the contract on every ingest (IngestPayloadSchema.text_repr is not
-// optional), so we always send it — reconstructed as base + caption (from the caption cache) so a
-// caption is preserved rather than clobbered — and append "Pictured: …" only when a cluster the
-// photo belongs to has been named. Hints are added only for labeled clusters.
+// The payload shape itself lives in lib/photo-payload.js so the caption worker emits the identical
+// union of fields and the two passes can run in either order without clobbering each other (#276).
+// entry.{source,source_id} is the content-hash key scan() computed and persisted (keyForMedia),
+// so a labeled photo enriches the SAME artifact scan.js/caption-worker created.
 function buildPayload(relPath, entry, clustersById, captionCache) {
-  const pictured = picturedNames(entry.clusters, clustersById);
-  const caption = captionCache[relPath] ?? null;
-  const baseText = currentTextRepr(entry.dateStr, path.basename(relPath), caption);
-  // entry.{source,source_id} is the content-hash key scan() computed and persisted (keyForMedia),
-  // so a labeled photo enriches the SAME artifact scan.js/caption-worker created.
-  const payload = {
+  return buildPhotoPayload({
     source: entry.source,
     source_id: entry.source_id,
-    type: 'photo',
-    text_repr: pictured.length ? `${baseText} Pictured: ${pictured.join(', ')}.` : baseText,
-    extra: { faces_detected: entry.faces, pictured, captioned: caption != null },
-  };
-  if (pictured.length) {
-    payload.entity_hints = pictured.map((alias) => ({ alias, alias_type: 'name', role: 'pictured', confidence: FACE_HINT_CONFIDENCE }));
-  }
-  return payload;
+    dateStr: entry.dateStr,
+    filename: path.basename(relPath),
+    caption: captionCache[relPath] ?? null,
+    faces: entry.faces,
+    pictured: picturedNames(entry.clusters, clustersById),
+    hintConfidence: FACE_HINT_CONFIDENCE,
+  });
 }
 
 // Everything in the payload that can change between runs — used to skip an upsert that would be

connectors/photo-exif/lib/photo-payload.js

@@ -0,0 +1,81 @@
+// The one place a photo enrichment payload is built. Both enrichment workers (caption + face)
+// upsert the SAME (source, source_id) artifact, and the ingest contract replaces `text_repr` and
+// `extra` wholesale (doc 04 §3 — no deep-merge). So whichever worker runs second overwrites the
+// other's contribution unless BOTH send the union of what's known. Keeping that union in one
+// function is what makes the two passes order-independent — the same reason `lib/describe.js` is
+// shared: two scripts building the same thing separately is how they drift (#276).
+import { parseClustersFile } from './face-cluster.js';
+import { currentTextRepr } from './caption-cache.js';
+import { readFileSync, existsSync } from 'node:fs';
+
+// The pictured names on a photo = the labels of the (distinct) clusters its faces fall into.
+// Unlabeled clusters contribute nothing — an anonymous pile is not a name (see face-worker.js).
+export function picturedNames(clusterIds, clustersById) {
+  const names = new Set();
+  for (const id of clusterIds) {
+    const label = clustersById.get(id)?.label;
+    if (label) names.add(label);
+  }
+  return [...names].sort();
+}
+
+// Build the ingest payload for one photo from everything currently known about it. `faces` and
+// `pictured` are OPTIONAL and meaningfully so: a caller that has no face data must pass them as
+// undefined, NOT as 0/[] — see the extra-field note below.
+export function buildPhotoPayload({ source, source_id, dateStr, filename, caption, faces, pictured, hintConfidence }) {
+  const names = pictured ?? [];
+  const baseText = currentTextRepr(dateStr, filename, caption);
+  const payload = {
+    source,
+    source_id,
+    type: 'photo',
+    text_repr: names.length ? `${baseText} Pictured: ${names.join(', ')}.` : baseText,
+    // `faces_detected: 0` asserts "we ran detection and found no faces" — a claim a caller that
+    // never ran detection must not make, and one the wholesale `extra` replace would make
+    // permanent. So an absent count stays absent rather than defaulting to 0 (#276).
+    extra: { captioned: caption != null },
+  };
+  if (faces !== undefined && faces !== null) payload.extra.faces_detected = faces;
+  if (pictured !== undefined) payload.extra.pictured = names;
+  if (names.length) {
+    payload.entity_hints = names.map((alias) => ({ alias, alias_type: 'name', role: 'pictured', confidence: hintConfidence }));
+  }
+  return payload;
+}
+
+// Read the face worker's two state files into a relPath -> { faces, pictured } lookup, so the
+// caption worker can carry face enrichment through its own upsert without re-running detection.
+// Both files absent (face pass never ran) is the normal, non-exceptional case — returns an empty
+// map, and every caller then behaves exactly as it did before this existed.
+export function readFaceEnrichment(faceStatePath, clustersPath) {
+  const lookup = new Map();
+  if (!faceStatePath || !existsSync(faceStatePath)) return lookup;
+  let faceState;
+  try {
+    faceState = JSON.parse(readFileSync(faceStatePath, 'utf8'));
+  } catch (err) {
+    console.error(`photo-exif: unreadable face state at ${faceStatePath}; captioning without face data`, err);
+    return lookup;
+  }
+  // An ABSENT clusters file is legitimate (the face pass ran but nothing has been named yet) and
+  // yields `pictured: []`, matching what face-worker.js itself sends in that state. An
+  // UNREADABLE one is an anomaly (mid-write on Windows, a directory, bad permissions) — bail to
+  // the empty lookup rather than claim `pictured: []`, which would assert "nobody named is in
+  // this photo" and, via the wholesale `extra` replace, make that false claim permanent. Same
+  // absent-≠-empty reasoning as faces_detected above.
+  let clusters = [];
+  if (clustersPath && existsSync(clustersPath)) {
+    try {
+      clusters = parseClustersFile(readFileSync(clustersPath, 'utf8')).clusters;
+    } catch (err) {
+      console.error(`photo-exif: unreadable cluster state at ${clustersPath}; captioning without face data`, err);
+      return new Map();
+    }
+  }
+  const clustersById = new Map(clusters.map((c) => [c.id, c]));
+  for (const [relPath, entry] of Object.entries(faceState)) {
+    if (!entry || typeof entry.faces !== 'number') continue;
+    lookup.set(relPath, { faces: entry.faces, pictured: picturedNames(entry.clusters ?? [], clustersById) });
+  }
+  return lookup;
+}