Skip to content

fix(photo-exif): shared HEIC-capable image decode for both AI passes#282

Merged
MSIH merged 5 commits into
mainfrom
fix/280-heic-decode
Jul 25, 2026
Merged

fix(photo-exif): shared HEIC-capable image decode for both AI passes#282
MSIH merged 5 commits into
mainfrom
fix/280-heic-decode

Conversation

@MSIH

@MSIH MSIH commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Closes #280

Summary

  • Adds connectors/photo-exif/lib/decode-image.js — a shared openImage(absPath) helper used by both AI enrichment passes. sharp stays the primary decoder for every format; heic-decode (pure-JS libde265/WASM, no native build) is the fallback for the ~47% of this library's HEICs that sharp's bundled libheif can't open (two distinct failure modes, both invisible to sharp.metadata(), which never actually decodes).
  • Migrates lib/face-detect.js and caption-worker.js to openImage, replacing their own raw sharp(absPath)/readFile calls.
  • Orientation is handled correctly per-path: the sharp path applies .rotate() (EXIF); the heic-decode path deliberately does not — libheif already applies the HEIC container's own rotation during decode, so a second rotate would double-rotate every portrait iPhone photo.
  • caption-worker.js's VLM failure handling changes from "abort the whole run on the first failure" to a consecutive-failure counter (VLM_MAX_CONSECUTIVE_FAILURES, default 5, resets on any success) — one bad photo at 2am shouldn't waste an overnight run. content_hash is still computed from the original file bytes, never the re-encoded JPEG sent to the VLM.
  • Resolves an unrelated pending allowScripts entry for onnxruntime-node that was sitting uncommitted on main (already committed to main directly ahead of this branch, per the issue's own note).

Changes

  • connectors/photo-exif/lib/decode-image.js (new) — openImage/createOpenImage with an injectable decoder seam for tests
  • connectors/photo-exif/lib/face-detect.js — uses openImage, drops the now-unused inline sharp import/pipeline builder
  • connectors/photo-exif/caption-worker.js — uses openImage + resize/re-encode for the VLM call; consecutive-failure counter
  • connectors/photo-exif/package.json / package-lock.json — adds heic-decode (no allowScripts needed, pure JS/WASM)
  • connectors/photo-exif/.env.example — documents VLM_MAX_CONSECUTIVE_FAILURES
  • connectors/photo-exif/README.md — new "HEIC decoding" section, updated caption-worker/setup/limitations/files sections
  • connectors/photo-exif/test.mjs — decode-image dispatch/fallback/no-rotation tests, caption-worker consecutive-failure tests

Verification

  • npm test (root): 186/186 pass
  • npm run check:boundary: green (no src/ import from the connector)
  • connectors/photo-exif: npm test — 37/39 pass in the full run; the 2 failures are a pre-existing Node v26 fetch+process.exit() native-crash race on this dev box (reproduced with plain fetch() calls and zero sharp/heic-decode involvement, and it already hits the same untouched face-worker test on unmodified main) — both new tests pass reliably in isolation (3/3). This connector's suite isn't part of CI (manual/on-device only, per CLAUDE.md).
  • Store→recall smoke test: booted the server on a throwaway port/db, POST /api/rememberPOST /api/recall returned the memory with a distance score, 0 server errors.
  • Pre-PR review found one edge-case bug (VLM_MAX_CONSECUTIVE_FAILURES=0 would stop the worker before its first VLM call) — fixed and covered by a new test in the same branch.

Test plan

  • openImage decodes the two known-failure-mode HEIC shapes (covered structurally via injected-decoder dispatch tests; real-file decode is a manual, on-device check per the issue — no HEIC fixture exists in this repo)
  • Sharp path still applies EXIF rotation; heic-decode path does not (unit-tested via injected fake pipelines)
  • Both decoders failing throws once, naming both causes
  • Caption worker survives an isolated VLM failure and keeps going
  • Caption worker stops only after VLM_MAX_CONSECUTIVE_FAILURES consecutive failures (and doesn't misbehave at the 0 edge case)
  • npm run check:boundary green
  • Root npm test green
  • Store→recall smoke test green

🤖 Generated with Claude Code

https://claude.ai/code/session_01LHTyidSMEdQyNZmHvHYdfP

Leftover from #273's tfjs-node -> onnxruntime-node swap; needed to
unblock branching for #280.
Add lib/decode-image.js (sharp-primary, heic-decode/libheif-WASM fallback,
orientation-safe) so the face and caption workers can read pixels from the
~47% of this library's photos sharp's bundled libheif can't decode. Caption
worker also survives an isolated VLM failure now, stopping only after
VLM_MAX_CONSECUTIVE_FAILURES in a row instead of on the first one.

Closes #280
Found in pre-PR review: a value of 0 (or negative) broke the caption
worker before its first VLM call, since the failure counter starts at
0 and 0 >= 0 is already true. Clamp to the default (5) below 1.

Refs #280
Copilot AI review requested due to automatic review settings July 25, 2026 13:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a shared, HEIC-capable image decode helper for the photo-exif connector so both AI enrichment passes (face detection + captioning) can reliably obtain pixel data, including for HEIC/HEIF files that sharp can’t decode on this environment.

Changes:

  • Adds lib/decode-image.js to centralize image opening with sharp-first / heic-decode fallback and orientation handling.
  • Updates lib/face-detect.js and caption-worker.js to use the shared decode helper (including caption-worker downscale/re-encode for VLM calls).
  • Changes caption-worker failure handling to stop only after VLM_MAX_CONSECUTIVE_FAILURES consecutive VLM failures (config + tests + docs).

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
connectors/photo-exif/lib/decode-image.js New shared openImage helper with sharp/heic-decode dispatch + orientation policy.
connectors/photo-exif/lib/face-detect.js Switches face detection to use openImage for a shared decode path.
connectors/photo-exif/caption-worker.js Uses openImage, downscales/re-encodes for VLM, and adds consecutive-failure stopping logic.
connectors/photo-exif/test.mjs Adds unit tests for decode-image dispatch/fallback/rotation and caption-worker failure behavior.
connectors/photo-exif/README.md Documents HEIC decoding behavior and updated caption-worker semantics.
connectors/photo-exif/.env.example Documents VLM_MAX_CONSECUTIVE_FAILURES.
connectors/photo-exif/package.json Adds heic-decode dependency and includes an allowScripts entry for onnxruntime-node.
connectors/photo-exif/package-lock.json Locks heic-decode and libheif-js transitive dependency details.
Files not reviewed (1)
  • connectors/photo-exif/package-lock.json: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread connectors/photo-exif/lib/decode-image.js
Comment thread connectors/photo-exif/lib/decode-image.js Outdated
Comment thread connectors/photo-exif/package.json
…ly fall back

Copilot review on PR #282: sharp defers real decode errors until an
output op runs, so sharp(absPath).rotate() never threw for an
undecodable file — the fallback to heic-decode for a mislabeled
extension (e.g. a real HEIC saved as .jpg) never actually triggered,
the same trap as sharp.metadata(). Force the decode on a clone before
returning. Also fixes a misleading "fallback" log line for the
.heic/.heif dispatch-first case.

Refs #280
@MSIH

MSIH commented Jul 25, 2026

Copy link
Copy Markdown
Owner Author

Thanks for the review — went through all three:

  1. decode-image.js:80 (lazy sharp decode breaks the fallback for mislabeled files) — confirmed real, and fixed in 3d97fda. sharp(absPath).rotate() never throws for an undecodable file (verified: only .toBuffer()/.stats() etc. force the actual libvips decode) — the same trap the issue itself calls out for sharp.metadata(). decodeWithSharp now forces a decode on a clone (pipeline.clone().stats()) before returning, so a real failure is caught in time to trigger the heic-decode fallback. Added a regression test using the real sharp module (not an injected fake) against a genuinely undecodable file.

  2. decode-image.js:63 (misleading "fallback" wording for the .heic-primary case) — agreed, reworded in the same commit.

  3. package.json:25 (allowScripts block) — this one's a false positive: git diff main...HEAD -- connectors/photo-exif/package.json shows this PR's only change to that file is the heic-decode dependency line. The allowScripts entry is already on main (committed there directly, ahead of this branch, exactly as the PR description says) — it isn't introduced here.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • connectors/photo-exif/package-lock.json: Generated file
Comments suppressed due to low confidence (1)

connectors/photo-exif/lib/decode-image.js:73

  • The log message says "fallback decoder" even in the common case where .heic/.heif dispatch selects heic-decode as the primary path. This can be misleading in logs; consider wording it neutrally (still noting the performance cost).
      console.error('photo-exif: using heic-decode (the slower ~1s/photo fallback decoder) for', absPath);

Comment thread connectors/photo-exif/caption-worker.js
Comment thread connectors/photo-exif/lib/decode-image.js Outdated
… heic-decode path

Second Copilot review round on PR #282:
- caption-worker.js's VLM-failure catch continue'd past the throttle
  sleep, so repeated failures hammered the VLM endpoint back-to-back
  instead of pacing like a success does. Throttle applies on failure
  too now, verified by a timing-gap test.
- decode-image.js's heic-decode path copied the whole decoded RGBA
  buffer via Buffer.from(typedArray); use a zero-copy Buffer view over
  the same ArrayBuffer instead (a 12MP RGBA decode is already ~48MB).

Refs #280
@MSIH

MSIH commented Jul 25, 2026

Copy link
Copy Markdown
Owner Author

Second round — both confirmed and fixed in 5a9213a:

  1. caption-worker.js (throttle skipped on VLM failure) — confirmed: the failure-path continue ran before reaching the sleep(VLM_THROTTLE_MS) at the loop bottom, so repeated failures would hammer the VLM endpoint with no pacing. Now sleeps on failure too. Added a test that measures the actual wall-clock gap between two failed VLM requests server-side (independent of subprocess spawn overhead) to guard against a regression here.

  2. decode-image.js (unnecessary copy in the heic-decode path) — confirmed: Buffer.from(typedArray) copies (verified with a quick repro — mutating the source after Buffer.from() does not propagate into the copy, but does into a Buffer.from(arrayBuffer, offset, length) view). Switched to the zero-copy view form.

The suppressed "using heic-decode fallback" wording comment was already addressed in the first round (3d97fda).

@MSIH
MSIH merged commit 9ae468d into main Jul 25, 2026
5 checks passed
@MSIH
MSIH deleted the fix/280-heic-decode branch July 25, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(photo-exif): shared HEIC-capable image decode for both AI passes

3 participants