Explain why -faces embedded no regions (issue #30)

[Majorfi]

Follow-up to #30. @Nurgak reports on v1.0.5 that a video shows date changes but no Face regions line, while an image works.

The actual problem

I reproduced that exact output — dates listed, no face line, 1 succeeded — from five different causes. Every guard in appendFaceRegionChange returned silently, so the output cannot distinguish:

1. Immich lists no named, visible person on the asset
2. the video container cannot hold regions
3. the video's rotation is 180°/non-cardinal (un-anchorable)
4. the file reports no pixel dimensions
5. Immich's face boxes carry no named person (or no dimensions)

So the report can't be diagnosed from the logs — by anyone, including the reporter. That ambiguity is the bug this PR fixes; it does not by itself change which assets get regions.

What changed

appendFaceRegionChange now returns a reason for each guard. It's surfaced two ways:

• as a diff entry, which is the one place a piped or -y run still prints:

  ~ Face regions   (skipped)   -> Immich lists no named, visible person on this asset
  

• appended to the skip message when there was nothing else to embed:

  metadata already matches; no face regions written: the file reports no pixel dimensions to anchor regions against
  

One ordering detail preserved deliberately: the GET /faces call stays ahead of the dimension guard so a missing face.read permission is still reported loudly rather than short-circuited. The existing permission tests caught this when I first had it the other way round.

Validation

Verified against a live Immich v3.0.1 — an image with no named people now renders:

[1/1] 4 EXIF mismatch found for L1001198.jpg:
    + TimeZoneOffset            (none)                    -> 2
    + XMP-exif:DateTimeOriginal (none)                    -> 2026-04-27T19:35:41.125+02:00
    ~ XMP-xmp:CreateDate        2026:04:27 19:35:41+02:00 -> 2026-04-27T19:35:41.125+02:00
    ~ Face regions              (skipped)                 -> Immich lists no named, visible person on this asset

go vet, gofmt, full suite green. New tests assert each of the five guards names itself, that a run without -faces stays silent, and that a successful embed reports no reason.

Next step for the report

With this, one -dry-run on the affected video tells us which guard fires, instead of guessing between five.

[Copilot]

Pull request overview

This PR improves observability for -faces runs by explaining why face-region embedding is skipped, so “no Face regions line” output can be diagnosed (especially for videos) without guessing which guard condition fired.

Changes:

• Extend appendFaceRegionChange to return a human-readable skip reason and surface it in both skip messages and diff output.
• Emit a (skipped) “Face regions” diff entry when other metadata changes exist but face regions were not written.
• Add tests asserting each skip guard produces an explanatory reason and that non--faces runs remain silent.

Reviewed changes

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

File	Description
src/process/pipeline.go	Plumbs faceSkip through processing, appending it to skip messages and diff output so -faces skips aren’t silent.
src/process/faces.go	Updates face-region embedding logic to return explicit skip reasons for each guard path.
src/process/faceSkipReason_test.go	Adds coverage to ensure every skip guard is explained and that successful embeds report no reason.

---

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

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

src/process/faces.go:42

• The skip reason for un-anchorable video rotation is grammatically incomplete ("only 0°, 90° and 270° are") and reads awkwardly in both the diff output and skipped message. Consider rephrasing to a complete sentence so users can understand it quickly.

	if !ok {
		return changes, nil, fmt.Sprintf("video rotation %d° cannot be anchored (only 0°, 90° and 270° are)", intTag(existing, "Rotation")), nil
	}

src/process/faces.go:59

• When GetAssetFaces returns 0 faces, the current reason string ends up as "none of Immich's 0 face box(es)…", which is confusing. It would be clearer to special-case 0 faces and report that Immich returned no face boxes for the asset.

	regions := exif.BuildFaceRegions(faces, orientation)
	if len(regions) == 0 {
		return changes, nil, fmt.Sprintf("none of Immich's %d face box(es) carry a named person with dimensions", len(faces)), nil
	}

src/process/faceSkipReason_test.go:120

• This test name says "RegionsMatch", but the setup has no existing XMP-mwg-rs:RegionInfo in tags, and the assertions expect a change to be generated (len(changes)==1). Renaming the test to reflect that it verifies "no skip reason when regions are written" would make the intent clearer.

func TestAppendFaceRegionChangeNoReasonWhenRegionsMatch(t *testing.T) {