fix: read thumbnail dimensions locally so photos cannot go silently invisible - #1961
Open
dotanm wants to merge 1 commit into
Open
fix: read thumbnail dimensions locally so photos cannot go silently invisible#1961dotanm wants to merge 1 commit into
dotanm wants to merge 1 commit into
Conversation
Every grid view filters on thumbnail__aspect_ratio__isnull=False, so a photo whose aspect ratio is NULL is invisible everywhere in the UI with no error and no warning — the library just looks short. The aspect ratio was derived by asking the exif service for the dimensions of the big thumbnail, a file we had just generated ourselves. When that service was degraded during a scan get_metadata returned [None, None], the ratio was skipped, and the affected photos silently vanished. The existing repair pass called the same service, so rescanning while exif was still unhealthy no-oped and the library stayed broken. Read the dimensions from the thumbnail with PIL instead, which removes the network dependency from the path entirely rather than making its failure louder. PIL and exiftool were verified to agree exactly on all 665 generated thumbnails of a real library, and on every aspect ratio already stored for them, so this is not a behaviour change for healthy setups. Also guard the empty-thumbnail case, which previously raised ValueError from inside the exception handler while logging, and extract the repair pass out of scan_photos into backfill_missing_aspect_ratios so it can be tested and so it reports how many photos are still invisible after it runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Every grid view filters on
thumbnail__aspect_ratio__isnull=False— the timeline (api/views/albums.py:447and:580), the photo list (api/views/photos.py:76), and the shared filter inapi/views/photo_filters.py:39. A photo whose aspect ratio is NULL is therefore invisible everywhere in the UI, with no error, no warning, and nothing in the job result to say so. The library just looks short, and there is no way for a user to tell "healthy but empty" from "broken".The aspect ratio was derived by asking the exif service for the dimensions of the big thumbnail — a file we had just generated ourselves a few lines earlier. When that service was degraded while a scan ran,
get_metadatareturned[None, None],_calculate_aspect_ratiologged a warning and returned without setting the field, and the affected photos silently disappeared. What makes it sticky rather than transient is that the existing repair pass inscan_photoscalls the same service, so rescanning while exif was still unhealthy silently no-oped and the library stayed broken. That is exactly what I hit on my own install: photos missing, rescan, no change, no clue why.I should say that the skip-and-continue behaviour is my own doing — it came from #1873, which made
get_metadatadegrade to None instead of aborting. That was the right call, since one flaky exif call must not kill a 155k-photo scan, but the downstream consequence was never surfaced.Rather than making the failure louder, this removes the network dependency from the path entirely: the dimensions now come from the thumbnail via PIL, which is already a dependency and is already used on this exact file elsewhere (
api/models/photo.py:425). Asking a sidecar over HTTP about a local file we generated ourselves was never buying anything. Note thatget_metadatareturned[height, width]whileImage.sizeis(width, height); the ratio is stillround(width / height, 2).To confirm this is not a behaviour change for healthy setups, I compared both methods across all 665 generated big thumbnails of a real library: zero PIL failures, zero dimension disagreements with exiftool, and zero differences against the 1320 aspect ratios already stored in the database for them. On a live install I then blanked 20 photos' aspect ratios to reproduce the bug, confirmed they dropped out of the grid filter (665 → 645), and ran the repair pass: all 20 came back with exactly their original values.
Two smaller things ride along.
_calculate_aspect_rationow checks for an emptythumbnail_bigbefore touching.path; previously that case raisedValueErrorfrom inside the exception handler that was logging it, because the log message itself interpolated.path. And the repair block is extracted out ofscan_photosintobackfill_missing_aspect_ratios, which makes it testable, and which now counts how many photos are still without an aspect ratio afterwards and logs a warning naming the consequence, so photos that genuinely cannot be repaired (a missing thumbnail file, say) become a visible number instead of silence. That recount deliberately re-queries: iterating the queryset caches its rows, so a naive second.count()would report the pre-repair number and claim failures that never happened.Nine tests cover this. Four of them fail against the current
dev— the two ratio calculations and the no-network assertion fail because in a test environment there is no exif service on port 8010, which is the bug in miniature, and the empty-thumbnail case errors on theValueErrordescribed above.For scale, this is neutral-to-positive in both directions. Per photo it replaces an HTTP round trip (plus up to three retries with backoff on a degraded service) with a local header read, so it is strictly faster and removes 1–2 requests per photo from the sidecar during a scan; on a small library the difference is a few milliseconds either way, and on a large one it removes a per-photo network dependency from the scan path. No new queries: the extracted helper runs the same single filtered query as before, plus one
COUNTfor the report.