Library sync removes missing images - #70
Draft
gosku wants to merge 36 commits into
Draft
Conversation
Schema only: nothing writes these yet. A run will need somewhere to record how many images it removed, how many it found missing, which prune the caller asked for, and why a prune removed nothing. prune_mode is persisted rather than passed around because the caller that finalises a run may be a different process entirely, so command-line flags cannot reach it any other way. PRUNING joins ACTIVE_STATES so that, once pruning exists, the unique-active-run constraint keeps a second sync from starting while a prune walks the tree and re-importing files it is about to remove. The constraint is rebuilt because its condition embeds the state tuple. failure_reason lets a caller tell "folder is missing on disk" apart from any other failure, which today is guessed from the state alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mark_completed baked one specific transition, and its legality, into the model. Pruning needs a second guarded transition, and copying the pattern would put more policy there. transition_state does the guarded write and nothing else: it reports how many rows it touched and leaves the caller to decide which transitions are legal and whether the target state is terminal. The WHERE clause has to stay in SQL, since moving it into Python would turn the exactly-one-winner election into a read-then-write race. complete_sync_run moves onto it in the same commit, because the mutator it called no longer exists. It also now accepts a run that is pruning, which is the transition the next commits need. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bound how much an automatic prune may remove in one pass, so an unmounted drive or an unreadable directory is reported rather than applied. Both thresholds must be exceeded for the guard to engage, which keeps ordinary small cleanups from tripping it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The width lived as a constant inside the generate_thumbnails command, so nothing else could know which widths exist. Removing an image has to clear its cached thumbnail at every width, which needs a definitive list. Overridable from the env file as a comma-separated list (THUMBNAIL_WIDTHS=600,1200); the command now generates each configured width rather than a single hardcoded one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Thumbnail cache keys are derived from the image path, so removing an image without clearing its cache leaves files that a later image reusing that path would be served instead of its own. Fujifilm filenames wrap around from DSCF9999 to DSCF0001, which makes that a real possibility rather than a theoretical one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removing a folder from the Library can take its images with it, but library folders may nest: an image below /photos/2024 is also below /photos. These queries scope removal to images no other registered folder covers, so removing the inner folder never takes images the outer one still monitors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing calls it yet. A catalog record will need to follow its file when the file is renamed or moved, so that the record survives instead of being stranded on a path that no longer exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An import that matches an existing record by content hash is either a move or a copy, and the old file tells them apart: gone means the bytes were renamed or moved, still there means this is a second copy. Only a move repoints the record. Until now the filepath was never updated, so a renamed or moved photo left its record pointing at a path that no longer exists: the gallery kept showing it and every thumbnail request 404'd. Repointing also means a rename keeps the photo's rating, favourite mark and album membership, which a delete-and-re-import would throw away. Wired into process_image here, since a record that never followed its file is exactly the bug being fixed. One dedup test asserted the old behaviour and now asserts relocation. Never touches either file on disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing calls it yet. Removes a catalog entry and garbage-collects the FujifilmExif row it orphans, mirroring the tail of merge_image_into. The image file on disk is never touched; only the derived thumbnail cache is cleared, and that is for correctness rather than tidiness: cache keys come from the path, and Fujifilm filenames wrap around from DSCF9999 to DSCF0001, so a later file reusing a path would be served the previous image's thumbnail. The recipe is never deleted, since recipes are shared and outlive their images. Cover-image and recipe-card references are left to the schema's SET_NULL: a recipe with no explicit cover already falls back to its most-used image, and a card is a rendered JPEG that stands on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The scan skipped directories whose mtime predated the folder's last_checked_at. That cannot survive alongside removal: renaming a directory updates its parent's mtime and never its own, so a renamed subtree keeps an old timestamp, last_checked_at only moves forward, and a gated walk never looks at it again. Every image under a renamed folder would be treated as deleted and never found. The gate bought little anyway. os.walk has already listed each directory by the time the check runs, so it added a getmtime() per directory and saved only a filename suffix check; the costly work is already avoided by diffing against the known catalog paths. last_checked_at stays, since the Library page shows it and it is the only evidence a sync ran when nothing changed. Only its gating role goes, which also makes clearing it on a path update pointless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing calls it yet. Removal has to be scoped to one library folder, and Image has no foreign key to LibraryFolder, so membership can only be decided by path prefix. The separator is part of the prefix so that a folder named /photos does not also claim /photos-old. Scoping this way is what will keep images imported from outside the library, by the process_images command, out of any removal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing calls it yet. Removes catalog entries for images under one library folder whose files are gone, and nothing else: no image file is ever deleted from disk. Candidates come from the difference between the catalogued paths and a fresh walk, then each is confirmed with a stat. The walk alone would be wrong, because it does not follow symlinked directories, silently yields nothing for a directory it cannot read, and matches only JPEG extensions, so anything it cannot see would look deleted. Removal is permanent, so the filesystem gets the final say. lexists rather than exists, because a broken symlink still occupies the path and the conservative reading has to win. prune_guard_trips stops a pass that would take most of a folder at once, which is far more often an unmounted drive or an unreadable directory than a real deletion. Both thresholds must be exceeded, so ordinary cleanups are applied without a warning. Nothing is pruned when the folder itself is absent, since an unplugged drive must not empty the gallery. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing calls it yet. Moves a run from processing into a prune phase, electing a single winner under concurrent workers so the prune runs once however many of them arrive together. Electing here rather than after completion is deliberate. Pruning after a run is already complete would stop the progress poller before the counts are written, let a second sync start while the tree is still being walked, and leave a run claiming COMPLETED if the process dies mid-prune. A distinct active state avoids all three, and the existing startup recovery already handles it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sync_library inferred "this folder is missing from disk" from the run state alone, so any failure whatsoever was reported to the user as a missing folder. The free-text message that held the real reason was never surfaced anywhere. fail_sync_run now takes a code alongside the message, and its one caller passes it. The distinction matters more once removal exists: a missing folder is usually an unplugged drive, and it is the moment a user most needs telling that nothing was taken out of their gallery. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing calls it yet. Elects a single finaliser for a run, prunes the folder, then completes the run whatever the prune did. Pruning belongs at the end of a run rather than inside the scan, because imports have to land first: a file that moved has only repointed its record once it has been re-imported, and until then it looks deleted. Putting the orchestration in a use case keeps Celery out of the domain, and gives every possible last caller, the scan that found nothing and each per-image task, one function to reach. It also defers when another folder is still syncing, which is what stops a file moved between two library folders being removed by the source before the destination has picked it up. Delaying a removal is always safe; removing early is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing calls it yet. Removes catalog entries for one folder's missing files without scanning for new ones, and records the outcome on that folder's latest run so the Library page reflects it. Lite mode needs this: it scans folders one after another, so pruning inside each sync would remove a file moved into a folder that has not been looked at yet. Importing everything first and then pruning needs a way to prune on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removing a folder used to leave every image it had imported behind, with nothing recording where they came from. The choice is a required argument rather than a defaulted one, because which of the two happens is the whole point and no caller should get it by accident. That makes this one commit across three layers: the domain signature change forces the use case and the view with it. Only images no other registered folder covers are taken, so removing a folder nested inside another never touches images the outer one still monitors. The photo files themselves stay on disk. The view reads the choice from the form; the confirmation that offers it comes next. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This is where removal starts happening. The pieces existed but nothing called them. Both places that could be the last one standing now finalise through the same use case: the scan when it found nothing new, which is exactly the "the user deleted photos" case, and each per-image task as it finishes. Imports therefore always land before anything is removed, so a file that moved has already repointed its record and no longer looks missing. Lite mode scans folders one after another, so a prune inside each sync would run before later folders had been looked at, and a photo moved from the first folder to the last would be removed moments before being re-imported, losing its rating. It imports everything first, then prunes. Full mode starts every run back to back, so the deferral rule already covers the same case there. The scenario tests are the acceptance suite for all of this: one test per row of the matrix, building real trees of JPEGs, mutating them exactly as each scenario describes and syncing again. Moves assert the record kept its id, rating and favourite rather than merely still existing, and every scenario asserts the photo files are still on disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removing a folder can now take its images out of the gallery, so the user has to be able to choose, and to see what the choice costs before making it. A browser confirm() cannot show a count. The dialog names how many images come only from this folder, which is the number that would actually go, and says plainly that the photo files stay on disk. When nothing would leave the gallery it offers only the plain removal. It reuses the modal shell the folder browser already has rather than adding a second one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removal has been happening automatically with no way to look before leaping, no way to get past the safety guard, and no way to turn it off. --dry-run-prune lists what would go and removes nothing, which is the flag to reach for the first time the guard fires and you want to know what it caught. --force-prune overrides the guard once you have looked. --no-prune imports only. The guard warning now names the folder and the counts and points at the remedy, rather than leaving the user to guess why images they deleted are still in the gallery. A missing folder says outright that nothing was removed. PruneWarning carries the sampled paths the dry run prints. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Sync column knew nothing about removal: a prune the guard stopped was invisible, and a folder missing from disk rendered a bare "Sync failed" with no reason, which is the single most alarming thing the page can show and the least informative. It now reports how many images a run removed, shows the prune phase while it runs, and carries both warnings. The missing-folder one says "Folder not found on disk. Nothing was removed from the gallery", because an unplugged drive looks exactly like a mass deletion and that is the moment the reassurance is worth most. The view resolves the stored codes into booleans so templates never compare against database values. Warnings are styled as warnings rather than errors: nothing is broken and nothing was lost. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ADR 013 records the decisions and carries the scenario matrix as the specification of what "consistent with the folders" means, cross-referenced to the test module that covers each numbered row. ADRs 010 and 011 are left untouched: they record decisions that were taken at the time, and a later decision does not get to rewrite them. The supersession is stated in ADR 013 and in the docs index, following the same direction ADR 012 used for ADR 005. The user-facing docs lead with the thing that matters most: Filmcase never deletes a photo file, and "removing" an image only removes its catalog entry. The gating section is gone, since gating is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing writes it yet. A file the sync cannot import leaves no trace at all today: process_image raises before any write, the invalid-recipe path is rolled back by its own atomic block, and errors persist nothing. The outcome survives only as a counter on the run and a log line. That is why such a file is rediscovered on every single sync. On a 40k library with 14.5k non-Fujifilm JPEGs, each startup re-runs exiftool on every one of them to reach the same conclusion. file_size and file_modified_at are the change fingerprint. A file whose fingerprint still matches cannot have become importable, so a later sync can pass over it for the cost of one stat rather than one process. A file the user fixes in place changes its fingerprint and is examined again on its own. The foreign key scopes records to a folder for display, and takes them with the folder when it is removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing reads it yet. Sync currently publishes one broker message per new file, synchronously, before the command returns, which is what makes a large import block startup for tens of seconds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing calls them yet. Four reads the feature needs, each shaped by how many rows it has to cope with: a 40k library can carry tens of thousands of ignored files. get_ignored_fingerprints returns one mapping in one query, because the sync compares it against every candidate path it found on disk. get_ignored_images returns a queryset rather than a list so the caller can paginate rather than materialise the lot. get_ignored_counts_by_folder is a single aggregate, so showing a count on every Library row does not become a query per folder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing calls them yet. record_ignored_image captures the file's size and modification time at the moment it was examined. Re-recording an existing entry replaces that fingerprint, which is the part that is easy to get wrong: a file that changed, was examined again and failed again would otherwise keep its stale fingerprint and be re-examined on every sync from then on. forget_ignored_image and forget_ignored_images undo it, singly or in bulk by reason, so a batch that failed for an environmental reason can be reconsidered without touching the permanently unimportable ones. None of this touches a file or moves an image in or out of the gallery. An ignored file was never imported; forgetting its record only means the next sync looks at it again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing reads these records yet, so behaviour is unchanged. Every non-success branch now remembers the file alongside the counter it already bumped. The error branch keeps the exception type and message, truncated, which is what turns a run reporting "93 errors" into 93 files you can actually look at: until now that detail existed only in a log line. Success is the other half. A file that failed before and imports now must stop being ignored, or the record outlives the truth and shows a photo that is in the gallery as though it had been rejected. A file that disappeared between the scan and processing cannot be stat'ed. That is an ordinary race rather than a failure worth aborting the run for, so it is logged and skipped: the next sync will not find it either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This is the fix. A file the sync cannot import has no catalog entry, so
the path diff counted it as new every single time and handed it to the
worker to be read from scratch again. On a 40k library with 14.5k
non-Fujifilm JPEGs that is 14.5k exiftool processes per startup, forever,
every one of them reaching the conclusion already reached.
Measured before this commit, on a folder holding one such file:
run1: total=1 skipped=1
run2: total=1 skipped=1
run3: total=1 skipped=1
Candidates that already carry a record are now checked against the size
and modification time captured when they were last examined, and dropped
if neither has moved. The stat costs one syscall and is only paid for
files that are already ignored, so the extra work is bounded by how many
have failed rather than by the size of the tree.
Anything whose fingerprint has moved falls through and is examined again,
which is how a file the user fixes in place comes back on its own without
needing to be told.
This is a regression introduced earlier on this branch. Retiring mtime
gating was right, since it made renamed folders undetectable, but it
turned "retry a failed file when its directory changes" into "retry it
always". The tests here pin the inverse of the measurements above.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing calls it yet. Sync dispatches one message per file, synchronously, before the command can return, so the per-message bookkeeping sits directly on the critical path of startup: 14.5k files means resolving the same dotted path 14.5k times and running the structlog processor chain 14.5k times for a log record that is dropped at the handler anyway. Resolving once and publishing one event carrying the count removes both. Deliberately stopping there: sharing a broker producer and skipping the result-backend call would shave more off each publish, but the next commit cuts the message count by two orders of magnitude, after which the per-message cost stops being worth machinery. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dispatch is synchronous: the command that starts a sync cannot return until the last message is published, and make start runs it to completion before the server binds. One message per file therefore puts the whole publish loop on the critical path of startup, which on a large import is tens of seconds of an unreachable server. Batching cuts the message count by the batch size, so 14.5k files become ~146 messages. Each image is still handled individually inside the batch, so progress, ignore records and run completion are unchanged; only the number of broker round trips falls. The task is renamed rather than reused, because its arguments changed. That means restarting the worker once after deploying, the same consequence ADR 011 recorded for adding a task. Messages already queued under the old name would be rejected as unknown; their run is recovered as interrupted on the next start and re-imported. Lite mode still processes inline, unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing calls them yet. Forgetting a record is how a file gets another chance, so it needs to be reachable from both a button and a flag. Limiting a bulk retry to one reason is the case that matters: a batch that failed for an environmental reason deserves another look, while files that are simply not Fujifilm would only be re-read to reach the same verdict. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ignored files are now left alone until they change, which is right for the common case and wrong when the reason they failed was environmental and has since been put right. This is the escape hatch: forget everything and look again. It warns in its own help text that this is slow on a library carrying many ignored files, because it is: examining them again is exactly the cost the records exist to avoid. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records were being kept but there was nowhere to see them, so 14.5k skipped files and 93 errors existed only as counters that stopped being reported once the files stopped being re-examined. Paginated and filterable by reason, because the two groups want different attention: the non-Fujifilm ones are noise, and the handful of errors are the ones worth reading. Without the filter the errors are buried under thousands of rows and effectively unfindable. Each error shows the message that caused it, which until now lived only in a log line. The page leads by saying no file was deleted or changed, since a list of thousands of "ignored" photos invites exactly that fear. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The records were readable but permanent, so a file rejected for a reason that has since been fixed had no way back short of a command-line flag that forgets everything. Three ways in, matching how the two groups differ. Per-row for the one file you looked at and disagree with. "Retry all errors" for the common case, a batch that failed environmentally, without dragging thousands of non-Fujifilm files along. "Retry everything" behind a confirmation that says the next sync will be slow, because it will. A rejected non-Fujifilm file that has not changed will be rejected again the moment it is examined, so its Retry says as much rather than appearing broken when nothing happens. Error rows carry no such note, because there retrying is exactly the point. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Without this the page exists but nothing points at it, and the files it lists are invisible: once they stop being re-examined they also stop appearing in the sync summary, so a user would have no way of knowing the app had seen 14.5k files and decided against them. One aggregate for the whole table rather than a count per row, since a Library page should not cost a query per folder. Zero renders as plain text, because a link to an empty page is just a dead end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ADR 014 records why a file that cannot be imported is now remembered, why the record is keyed to the file's size and modification time rather than being a permanent verdict on a path, and why dispatch is batched. It also revises ADR 013's risk 7, which described the cost as "two full walks per sync" and understated it: the real cost was an exiftool process per previously-failed file, on every sync. ADR 013 itself is left untouched, as a record of what was decided then. The user-facing docs lead with the thing most likely to alarm someone looking at a list of 14,000 ignored photos: none of them has been deleted or changed, and a file they fix comes back on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gosku
force-pushed
the
library-sync-removes-missing-images
branch
from
August 9, 2026 11:21
b337b89 to
20e6263
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.