feat: optional photos.preserve_originals_as_bak — hide originals of edited photos via .bak

[epheterson]

Summary

New optional config: photos.preserve_originals_as_bak (default False).

When True AND photos.filters.file_sizes contains both original and
original_alt, edited photos land as TWO files on disk:

• IMG_1234.JPG — the edited "current view" (visible to
  Plex / Photos.app / Synology Photos)
• IMG_1234.HEIC.original.bak — the untouched original (no app
  recognises .bak as an image
  extension, so it's hidden from photo
  browsers but filesystem-recoverable
  for "revert to original" scenarios)

Unedited photos (no original_alt available for them on iCloud) are
unaffected — they get their normal single file per the chosen
filename_format.

Motivation

Users who download both versions of edited photos currently see both files
in their photo apps (duplicates). This option lets them treat the edited
version as the canonical "current view" while keeping the untouched
original as a hidden filesystem-level backup.

The toggle is opt-in and defaults False, so existing mandarons users see
no behaviour change.

Implementation

• New config_parser.get_photos_preserve_originals_as_bak(config) returns
  bool (default False).
• Module-level _PRESERVE_ORIGINALS_AS_BAK in photo_path_utils.py set
  once per sync run via the new set_preserve_originals_as_bak() setter
  (same pattern as set_default_filename_format).
• generate_photo_filename_with_metadata appends .original.bak to the
  filename when the toggle is on, file_size is original, AND the photo
  has an original_alt version on iCloud.
• Soft check: exceptions reading photo.versions are treated as "no alt"
  so a partial CloudKit record cannot break the filename pipeline.

Tests

10 new tests in tests/test_preserve_originals_as_bak.py:

• Config helper defaults / normalisation
• Edited photo's original gets the suffix
• Edited photo's original_alt does NOT get the suffix
• Unedited photo's original does NOT get the suffix
• Non-original sizes (medium, thumb, etc.) do not get the suffix
• Suffix not applied when toggle is off
• photo.versions exception is non-fatal (falls through to normal name)

Notes

• The convention chosen (.original.bak at the very end of the filename)
  is greppable (find . -name "*.original.bak") and self-documenting on
  casual filesystem browsing.
• Works the same way in both filename_format: metadata and
  filename_format: simple modes — the .original.bak qualifier is
  appended to whatever the base filename would have been.

[Copilot]

Pull request overview

Adds an opt-in Photos sync behavior to keep untouched originals of edited assets as a hidden-on-purpose sidecar (.original.bak) while letting the edited (“current view”) file remain visible to photo browsers.

Changes:

• Introduces photos.preserve_originals_as_bak config accessor in config_parser.
• Adds a module-level toggle + .original.bak suffixing logic to photo_path_utils.generate_photo_filename_with_metadata.
• Applies the toggle at the start of a photos sync run and adds a dedicated test suite covering the new behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

File	Description
src/config_parser.py	Adds get_photos_preserve_originals_as_bak() config helper (default False).
src/photo_path_utils.py	Adds global toggle + logic to suffix edited-photo original downloads with .original.bak.
src/sync_photos.py	Sets the new toggle once per sync run based on config.
tests/test_preserve_originals_as_bak.py	Adds unit tests for config defaulting and filename suffix behavior (including failure-safe alt detection).

[olfway]

@epheterson thanks for your pr!

I just thinking, maybe instead of new "preserve_originals_as_bak" option, we can use already existing file_sizes?

Something like this:

    file_sizes:
      - "original"
      - "original_alt"

What do you think?

[epheterson]

Hey @olfway, thanks though that currently downloads both still. Sticking with that structure, we could add original:hidden to define this behavior, or keep the flag or maybe this niche feature is only on the fork. For all of the PRs, very grateful for your foundation and happy to share and reshape. Some specific things for my setup won't always make sense to ship, no worries at all and for any let me know!

[mandarons]

/opencode please review this code.

[opencode-agent]

Here is my review of this PR:

---

Review: feat: optional photos.preserve_originals_as_bak

Overall: Well-implemented, safe, follows existing patterns. Minor concerns only.

Strengths

1. Follows established patterns: The module-level toggle + setter (set_preserve_originals_as_bak) mirrors the existing set_default_filename_format pattern in photo_path_utils.py. Consistent with codebase conventions.

2. Safety invariant is correctly enforced: sync_photos.py:401-404 gates the toggle on both preserve_originals_as_bak: true AND "original_alt" in file_sizes. This prevents the dangerous scenario where only the .bak file is downloaded (nothing visible to photo browsers). The Copilot review comment about this appears to predate the fix.

3. Soft exception handling is appropriate: _photo_has_alt_version() at photo_path_utils.py:83-92 catches all exceptions when reading photo.versions, treating them as "no alt". This prevents partial CloudKit records from breaking the filename pipeline.

4. File tracking is consistent: The .original.bak path flows through generate_photo_path() → collect_download_task() → files.add(photo_path), so the path is correctly tracked in the files set for both skip-if-exists checks and obsolete cleanup.

5. Docstring is accurate: The generate_photo_filename_with_metadata docstring (lines 106-111) correctly clarifies this only applies to the metadata-style filename, not the simple format.

Issues / Concerns

1. Module-level mutable state: _PRESERVE_ORIGINALS_AS_BAK as a module-level global is a code smell, but it's consistent with the existing _DEFAULT_FILENAME_FORMAT pattern. Not a blocker, but worth noting that this approach doesn't compose well with concurrent sync runs (though the codebase doesn't appear to support that currently).

2. Test patching path: test_preserve_originals_as_bak.py:186 patches "src.photo_path_utils.set_preserve_originals_as_bak". This works because the import in sync_photos.py:399 is a local from X import Y inside the function body (re-executed each call). If someone refactors that import to module-level, the patch would silently stop working. Consider patching "src.sync_photos.set_preserve_originals_as_bak" instead for robustness, or adding a comment explaining why the source-module path works.

3. No test for remove_obsolete cascade: The PR description mentions that .bak files are cleaned up when the photo is removed from iCloud, but there's no test verifying that remove_obsolete_files correctly handles .bak sidecars. The logic should work (both paths are in the files set, neither is orphaned), but a test would increase confidence.

4. Minor: test empty line at line 139: There's a blank line between TestPartialCloudkitRecordSafe and TestSyncGateRequiresOriginalAlt that's slightly inconsistent with the spacing elsewhere. Cosmetic only.

Summary

The implementation is solid and addresses a real user need (hiding duplicate originals of edited photos from Plex/Synology Photos). The safety gate on original_alt in file_sizes is correctly implemented and tested. The .bak convention is well-chosen (greppable, self-documenting, universally ignored by photo apps). The code follows existing patterns and maintains backward compatibility.

Recommendation: Approve — the concerns above are minor and don't block merging. The test patching robustness (point 2) could be addressed in a follow-up if desired.

opencode session  |  github run