Fix/ai tagging inconsistency #1060
Conversation
|
|
📝 WalkthroughWalkthroughBackend image processing now includes per-image error handling, conditional face detection based on classification results, granular status updates, and resource cleanup. Frontend MediaInfoPanel adds animation support with slide transitions and reorganizes layout with a close button. Changes
Sequence DiagramsequenceDiagram
participant App as Image Processing Loop
participant Classifier as Object Classifier
participant FaceDetector as Face Detector
participant DB as Database
participant Logger as Logger
App->>Classifier: classify_image()
alt Classification succeeds
Classifier-->>App: return classes
alt "person" class exists & count 1-6
App->>FaceDetector: detect_faces()
FaceDetector-->>App: return faces
Note over App: Face detection complete
else Skip face detection
Note over App: No person class or count out of range
end
App->>DB: insert image-class pairs
DB-->>App: success
App->>Logger: log image-class pairs
Logger-->>App: logged
App->>App: update image status = tagged
else Classification fails
Classifier-->>App: return None
App->>Logger: log classification error with path
Logger-->>App: logged
Note over App: Skip tagging, continue to next
end
Note over FaceDetector,Logger: finally: close classifier & face detector
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
|
I would add a draft PR so that it can act as a reference. |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/app/utils/images.py`:
- Around line 125-132: The db_insert_image_classes_batch call is not checked so
failures will still let the image be marked tagged; change the flow in the block
that creates image_class_pairs so you capture the return value of
db_insert_image_classes_batch(image_class_pairs), log an error/warning if it
returns False (including image_id and classes for context), and do not proceed
to the code that marks the image as tagged when the insert fails (either
return/raise or skip the status update); ensure db_insert_image_classes_batch,
image_class_pairs, classes and image_id are the anchors you use to locate and
change the logic.
In `@frontend/src/components/Media/MediaInfoPanel.tsx`:
- Around line 116-131: The conditional rendering currently uses a truthy check
on currentImage?.metadata?.latitude and longitude which will treat 0 as falsy
and hide valid coordinates; update the JSX conditional around currentImage to
explicitly check for null/undefined (e.g., currentImage?.metadata?.latitude !=
null && currentImage?.metadata?.longitude != null) so 0 values are allowed, and
keep using currentImage.metadata.latitude/toFixed and handleLocationClick only
inside that branch to avoid runtime errors.
| # Step 2: Insert class-image pairs if classes were detected | ||
| if len(classes) > 0: | ||
| # Create image-class pairs | ||
| image_class_pairs = [(image_id, class_id) for class_id in classes] | ||
| logger.debug(f"Image-class pairs: {image_class_pairs}") | ||
|
|
||
| # Insert the pairs into the database | ||
| db_insert_image_classes_batch(image_class_pairs) | ||
| # Insert the pairs into the database | ||
| db_insert_image_classes_batch(image_class_pairs) |
There was a problem hiding this comment.
Unchecked return value from db_insert_image_classes_batch may cause data inconsistency.
If the database insertion fails (returns False), processing continues and the image is still marked as tagged on line 139. This could leave the image in an inconsistent state—marked as tagged but with no class associations persisted.
Consider checking the return value and either skipping the status update or logging a warning on failure:
Suggested approach
# Insert the pairs into the database
- db_insert_image_classes_batch(image_class_pairs)
+ if not db_insert_image_classes_batch(image_class_pairs):
+ logger.warning(
+ f"Failed to insert class associations for image {image_path}"
+ )📝 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.
| # Step 2: Insert class-image pairs if classes were detected | |
| if len(classes) > 0: | |
| # Create image-class pairs | |
| image_class_pairs = [(image_id, class_id) for class_id in classes] | |
| logger.debug(f"Image-class pairs: {image_class_pairs}") | |
| # Insert the pairs into the database | |
| db_insert_image_classes_batch(image_class_pairs) | |
| # Insert the pairs into the database | |
| db_insert_image_classes_batch(image_class_pairs) | |
| # Step 2: Insert class-image pairs if classes were detected | |
| if len(classes) > 0: | |
| # Create image-class pairs | |
| image_class_pairs = [(image_id, class_id) for class_id in classes] | |
| logger.debug(f"Image-class pairs: {image_class_pairs}") | |
| # Insert the pairs into the database | |
| if not db_insert_image_classes_batch(image_class_pairs): | |
| logger.warning( | |
| f"Failed to insert class associations for image {image_path}" | |
| ) |
🤖 Prompt for AI Agents
In `@backend/app/utils/images.py` around lines 125 - 132, The
db_insert_image_classes_batch call is not checked so failures will still let the
image be marked tagged; change the flow in the block that creates
image_class_pairs so you capture the return value of
db_insert_image_classes_batch(image_class_pairs), log an error/warning if it
returns False (including image_id and classes for context), and do not proceed
to the code that marks the image as tagged when the insert fails (either
return/raise or skip the status update); ensure db_insert_image_classes_batch,
image_class_pairs, classes and image_id are the anchors you use to locate and
change the logic.
| {currentImage?.metadata?.latitude && | ||
| currentImage?.metadata?.longitude ? ( | ||
| <button | ||
| type="button" | ||
| onClick={handleLocationClick} | ||
| className="flex w-full cursor-pointer items-center truncate text-left font-medium text-white hover:underline" | ||
| title={`Lat: ${currentImage.metadata.latitude}, Lon: ${currentImage.metadata.longitude}`} | ||
| > | ||
| {tag} | ||
| </span> | ||
| ))} | ||
| {`Lat: ${currentImage.metadata.latitude.toFixed(4)}, Lon: ${currentImage.metadata.longitude.toFixed(4)}`} | ||
| <SquareArrowOutUpRight className="ml-1 h-[14px] w-[14px]" /> | ||
| </button> | ||
| ) : ( | ||
| <p className="font-medium text-white"> | ||
| Location not available | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
Truthy check for coordinates may incorrectly hide valid (0, 0) locations.
The condition currentImage?.metadata?.latitude && currentImage?.metadata?.longitude treats 0 as falsy. While rare, coordinates at or near the equator (latitude 0) or prime meridian (longitude 0) would incorrectly show "Location not available."
Suggested fix using explicit null/undefined checks
- {currentImage?.metadata?.latitude &&
- currentImage?.metadata?.longitude ? (
+ {currentImage?.metadata?.latitude != null &&
+ currentImage?.metadata?.longitude != null ? (📝 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.
| {currentImage?.metadata?.latitude && | |
| currentImage?.metadata?.longitude ? ( | |
| <button | |
| type="button" | |
| onClick={handleLocationClick} | |
| className="flex w-full cursor-pointer items-center truncate text-left font-medium text-white hover:underline" | |
| title={`Lat: ${currentImage.metadata.latitude}, Lon: ${currentImage.metadata.longitude}`} | |
| > | |
| {tag} | |
| </span> | |
| ))} | |
| {`Lat: ${currentImage.metadata.latitude.toFixed(4)}, Lon: ${currentImage.metadata.longitude.toFixed(4)}`} | |
| <SquareArrowOutUpRight className="ml-1 h-[14px] w-[14px]" /> | |
| </button> | |
| ) : ( | |
| <p className="font-medium text-white"> | |
| Location not available | |
| </p> | |
| )} | |
| {currentImage?.metadata?.latitude != null && | |
| currentImage?.metadata?.longitude != null ? ( | |
| <button | |
| type="button" | |
| onClick={handleLocationClick} | |
| className="flex w-full cursor-pointer items-center truncate text-left font-medium text-white hover:underline" | |
| title={`Lat: ${currentImage.metadata.latitude}, Lon: ${currentImage.metadata.longitude}`} | |
| > | |
| {`Lat: ${currentImage.metadata.latitude.toFixed(4)}, Lon: ${currentImage.metadata.longitude.toFixed(4)}`} | |
| <SquareArrowOutUpRight className="ml-1 h-[14px] w-[14px]" /> | |
| </button> | |
| ) : ( | |
| <p className="font-medium text-white"> | |
| Location not available | |
| </p> | |
| )} |
🤖 Prompt for AI Agents
In `@frontend/src/components/Media/MediaInfoPanel.tsx` around lines 116 - 131, The
conditional rendering currently uses a truthy check on
currentImage?.metadata?.latitude and longitude which will treat 0 as falsy and
hide valid coordinates; update the JSX conditional around currentImage to
explicitly check for null/undefined (e.g., currentImage?.metadata?.latitude !=
null && currentImage?.metadata?.longitude != null) so 0 values are allowed, and
keep using currentImage.metadata.latitude/toFixed and handleLocationClick only
inside that branch to avoid runtime errors.
1 participant
PR: Fix Inconsistent AI Tagging Behavior
Summary
This PR improves the robustness of the AI tagging batch process by isolating failures at the image level. A single corrupted or unclassifiable image no longer crashes the entire tagging run.
Problem
The AI tagging pipeline previously failed inconsistently during batch processing. When the object classifier returned None (or raised an unexpected exception), a TypeError was triggered, terminating the entire loop. As a result:
This behavior made the system unreliable and difficult to debug.
Solution
The image processing loop is now fault-tolerant. Errors are handled per image, logged explicitly, and do not affect the rest of the batch.
Implementation Details
File: backend/app/utils/images.py
Changes made:
Verification
Scope
Why this PR matters
Summary by CodeRabbit
Bug Fixes
New Features
✏️ Tip: You can customize this high-level summary in your review settings.