Fix: TypeError in image processing when file is missing or corrupt #1046
Conversation
|
|
📝 WalkthroughWalkthroughEnhanced image processing robustness in the utility module by adding file existence validation, conditional classification handling with None checks, optimized face detection (only for valid person classes), and per-image error handling with granular logging and conditional status updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/app/utils/images.py`:
- Around line 115-123: When detecting a missing source file in the block that
checks os.path.exists(image_path), also remove the associated thumbnail file
referenced on the image record (thumbnailPath) before deleting the DB record;
locate the check using image_path and image_id and call
os.path.exists(thumbnailPath) then os.remove(thumbnailPath) (wrap in try/except
to log any unlink errors) and only then call db_delete_images_by_ids([image_id])
so orphaned thumbnails are cleaned up.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/utils/images.py
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/utils/images.py (3)
backend/app/database/images.py (3)
db_delete_images_by_ids(364-396)db_update_image_tagged_status(264-290)db_insert_image_classes_batch(293-324)backend/app/models/ObjectClassifier.py (1)
get_classes(15-25)backend/app/models/FaceDetector.py (1)
detect_faces(26-67)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| try: | ||
| # Step 0: Check if image file still exists on disk | ||
| if not os.path.exists(image_path): | ||
| logger.warning( | ||
| f"Image file no longer exists, removing from database: {image_path}" | ||
| ) | ||
| db_delete_images_by_ids([image_id]) | ||
| continue | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "def db_get_untagged_images" -A 15Repository: AOSSIE-Org/PictoPy
Length of output: 1071
🏁 Script executed:
rg -n "def db_delete_images_by_ids" -A 5Repository: AOSSIE-Org/PictoPy
Length of output: 490
🏁 Script executed:
fd -t f "images.py" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 179
🏁 Script executed:
rg -n "UntaggedImageRecord" -A 5 -B 2 | head -40Repository: AOSSIE-Org/PictoPy
Length of output: 1010
🏁 Script executed:
sed -n '100,140p' backend/app/utils/images.pyRepository: AOSSIE-Org/PictoPy
Length of output: 1748
🏁 Script executed:
sed -n '364,390p' backend/app/database/images.pyRepository: AOSSIE-Org/PictoPy
Length of output: 857
🏁 Script executed:
sed -n '32,45p' backend/app/database/images.pyRepository: AOSSIE-Org/PictoPy
Length of output: 357
🏁 Script executed:
rg -n "thumbnailPath\|thumbnail_path" backend/app/utils/images.py | head -10Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
rg -n "os.remove.*thumbnail\|thumbnail.*os.remove" backend/app/Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
sed -n '217,250p' backend/app/database/images.pyRepository: AOSSIE-Org/PictoPy
Length of output: 1120
🏁 Script executed:
sed -n '95,115p' backend/app/utils/images.pyRepository: AOSSIE-Org/PictoPy
Length of output: 695
🏁 Script executed:
rg -n "image_util_classify_and_face_detect_images" backend/app/ -B 2 -A 2Repository: AOSSIE-Org/PictoPy
Length of output: 619
Clean up thumbnails when source file is missing.
When the source image is deleted from disk, the code removes the DB record but leaves any thumbnail file orphaned. The image record includes thumbnailPath, so consider deleting it to prevent disk growth.
♻️ Suggested fix
if not os.path.exists(image_path):
logger.warning(
f"Image file no longer exists, removing from database: {image_path}"
)
+ thumbnail_path = image.get("thumbnailPath")
+ if thumbnail_path and os.path.exists(thumbnail_path):
+ try:
+ os.remove(thumbnail_path)
+ logger.info(f"Removed orphaned thumbnail: {thumbnail_path}")
+ except OSError as e:
+ logger.warning(f"Error removing thumbnail {thumbnail_path}: {e}")
db_delete_images_by_ids([image_id])
continue📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| # Step 0: Check if image file still exists on disk | |
| if not os.path.exists(image_path): | |
| logger.warning( | |
| f"Image file no longer exists, removing from database: {image_path}" | |
| ) | |
| db_delete_images_by_ids([image_id]) | |
| continue | |
| try: | |
| # Step 0: Check if image file still exists on disk | |
| if not os.path.exists(image_path): | |
| logger.warning( | |
| f"Image file no longer exists, removing from database: {image_path}" | |
| ) | |
| thumbnail_path = image.get("thumbnailPath") | |
| if thumbnail_path and os.path.exists(thumbnail_path): | |
| try: | |
| os.remove(thumbnail_path) | |
| logger.info(f"Removed orphaned thumbnail: {thumbnail_path}") | |
| except OSError as e: | |
| logger.warning(f"Error removing thumbnail {thumbnail_path}: {e}") | |
| db_delete_images_by_ids([image_id]) | |
| continue |
🤖 Prompt for AI Agents
In `@backend/app/utils/images.py` around lines 115 - 123, When detecting a missing
source file in the block that checks os.path.exists(image_path), also remove the
associated thumbnail file referenced on the image record (thumbnailPath) before
deleting the DB record; locate the check using image_path and image_id and call
os.path.exists(thumbnailPath) then os.remove(thumbnailPath) (wrap in try/except
to log any unlink errors) and only then call db_delete_images_by_ids([image_id])
so orphaned thumbnails are cleaned up.
|
hi @rahulharpal1603, |
1 participant
Summary
This PR fixes a critical failure mode where a single missing or corrupt image could halt all AI image processing in PictoPy.
The root issue was an unchecked
Nonereturn from the object classification pipeline, combined with batch-level failure behavior and missing recovery for orphaned database records.With this change, AI tagging becomes resilient and self-healing, allowing processing to complete even when individual images are unreadable or removed from disk.
Impact
Before this fix:
After this fix:
This significantly improves reliability for users with large libraries, external storage, or frequent filesystem changes.
Steps to Reproduce (Before Fix)
Add a folder containing many images (e.g. 100+).
Enable AI tagging and let background processing start.
While processing is running:
.jpg).Observe:
TypeErrorrelated tolen(None).Root Cause
ObjectClassifier.get_classes()explicitly returnsNonewhen an image cannot be read.len(classes)unconditionally.Fix Overview
This PR makes the AI pipeline defensive at image boundaries instead of failing the entire batch:
Nonereturns from the classifier are handled explicitly.Key Example
Handling unreadable images safely:
Ensuring filesystem consistency:
Result
This change preserves existing behavior for valid images while preventing a single bad file from breaking the entire system.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.