fix: Add thread safety to session initialization in ModelRotationDetector

[duartebarbosadev]

This pull request improves the thread safety and error handling of the model rotation detector, particularly around loading the model session. The main changes ensure that model loading is protected by a lock to prevent race conditions, and that missing model errors are surfaced immediately and more clearly, allowing for better user feedback and recovery.

Thread safety and concurrency improvements:

• Added a threading.Lock (_session_init_lock) to the singleton instance to ensure that model session loading is thread-safe and prevents duplicate loads in concurrent scenarios. [1] [2] [3]

Error handling and user feedback:

• Changed the behavior when the model file is missing: instead of marking the detector as permanently failed, now raises a ModelNotFoundError with a best-effort path hint, allowing the UI to prompt the user to add the model and retry without restarting the app. [1] [2]
• Added a new _build_expected_model_path method to generate a helpful path hint for missing model errors, improving the feedback shown to users.

[Copilot]

Pull Request Overview

This PR enhances thread safety and error handling in the ModelRotationDetector's session initialization. It introduces a lock to prevent race conditions during concurrent model loading and changes the error handling strategy for missing models to allow runtime recovery without application restart.

Key Changes:

• Added thread-safe session initialization using a threading lock with double-checked locking pattern
• Changed missing model handling to raise an exception instead of permanently disabling the detector
• Added helper method to generate user-friendly model path hints for error messages

Comments suppressed due to low confidence (1)

src/core/image_features/model_rotation_detector.py:1

• Setting s.failure_logged = True before raising the exception (line 142-143) creates a race condition. If the exception is caught and handled elsewhere, and another thread enters this block before the first thread completes, the second thread will skip logging because failure_logged is already True, but will still raise the exception. The flag should be set after all logging is complete, or the logic should ensure atomic operation of both logging and flag setting.

"""Lazy-loading rotation detector for image orientation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The state flags s.tried_load and s.load_failed are checked at line 146 but are never set within the lock when the model path is missing. This creates a race condition where multiple threads could simultaneously raise ModelNotFoundError and log the same warning multiple times before s.failure_logged is set to True. Consider setting s.tried_load = True after raising the exception or restructuring the logic to prevent duplicate exception raising and logging across threads.