From 10fe3561fbb94ca038959d6ff638b742d73a5ad1 Mon Sep 17 00:00:00 2001 From: AlanRockefeller Date: Sun, 5 Apr 2026 14:06:35 -0700 Subject: [PATCH 1/6] Improve editor reuse safety and harden crop/save session handling include _edits_rev in save session tokens and _on_save_finished() comparisons so editor state is preserved if the user keeps editing while a save is in flight, while normal post-save cleanup still runs when the revision is unchanged prevent load_image_for_editing() from reusing a matched preview-only session when float_image is missing, forcing a full reload for flows that need the master buffer harden toggle_crop_mode() entry by requiring a valid current image, reusing _block_if_saving() for consistent save-in-flight behavior, and ensuring the editor is loaded before enabling crop mode add targeted reopening tests covering: forced reload when float_image is missing crop mode blocked while saving crop mode blocked when no image is selected editor state preserved when _edits_rev advances during save normal editor clear when _edits_rev is unchanged --- faststack/app.py | 44 +++++++-- faststack/tests/test_editor_reopening.py | 121 +++++++++++++++++++++++ 2 files changed, 156 insertions(+), 9 deletions(-) diff --git a/faststack/app.py b/faststack/app.py index 83abf14..a6ff687 100644 --- a/faststack/app.py +++ b/faststack/app.py @@ -1630,6 +1630,9 @@ def save_edited_image(self): save_image_key, getattr(self, "view_override_kind", None), self.image_editor.session_id if self.image_editor else None, + # Include edit revision so _on_save_finished does not clear state + # if the user continued editing after save was submitted. + getattr(self.image_editor, "_edits_rev", None), ) if save_image_key and save_image_key in self._saving_keys: @@ -1778,6 +1781,7 @@ def _on_save_finished(self, save_result: dict): current_image_key, getattr(self, "view_override_kind", None), self.image_editor.session_id if self.image_editor else None, + getattr(self.image_editor, "_edits_rev", None), ) still_on_same_image = ( @@ -5956,6 +5960,13 @@ def load_image_for_editing(self): except (OSError, ValueError): pass + if match: + # Also require an intact float buffer — a preview_only load leaves + # current_filepath/mtime set but float_image=None, which breaks + # crop, darken, and full-editor flows that need the master buffer. + if getattr(self.image_editor, "float_image", None) is None: + match = False + if match: log.debug( "load_image_for_editing: Reusing existing session for %s", filepath @@ -6834,10 +6845,31 @@ def cancel_crop_mode(self): @Slot() def toggle_crop_mode(self): """Toggle crop mode on/off.""" - self.ui_state.isCropping = not self.ui_state.isCropping - if self.ui_state.isCropping: - # Entering crop mode: reset to full image defaults + # Exiting crop mode: cleanup + self.ui_state.isCropping = False + self.ui_state.currentCropBox = (0, 0, 1000, 1000) + # Ensure preview rotation is cleared when exiting + self.image_editor.set_edit_param("straighten_angle", 0.0) + self.update_status_message("Crop cancelled") + else: + # Entering crop mode requires a loaded image with a valid float buffer. + if not self.image_files or not ( + 0 <= self.current_index < len(self.image_files) + ): + self.update_status_message("No image to crop") + return + + # Block if a save is already in progress for this image. + current_path = self.image_files[self.current_index].path + if self._block_if_saving(current_path): + return + + if not self.load_image_for_editing(): + return + + self.ui_state.isCropping = True + # Reset to full image defaults self.ui_state.currentCropBox = (0, 0, 1000, 1000) self.ui_state.aspectRatioNames = [r["name"] for r in ASPECT_RATIOS] self.ui_state.currentAspectRatioIndex = 0 @@ -6845,12 +6877,6 @@ def toggle_crop_mode(self): # Reset rotation to 0 when starting fresh crop mode self.image_editor.set_edit_param("straighten_angle", 0.0) self.update_status_message("Crop mode: Drag to select area, Enter to crop") - else: - # Exiting crop mode: cleanup - self.ui_state.currentCropBox = (0, 0, 1000, 1000) - # Ensure preview rotation is cleared when exiting - self.image_editor.set_edit_param("straighten_angle", 0.0) - self.update_status_message("Crop cancelled") @Slot() def stack_source_raws(self): diff --git a/faststack/tests/test_editor_reopening.py b/faststack/tests/test_editor_reopening.py index 20db42a..6734238 100644 --- a/faststack/tests/test_editor_reopening.py +++ b/faststack/tests/test_editor_reopening.py @@ -220,5 +220,126 @@ def test_editor_close_clears_memory_if_no_save_active(self): self.controller.image_editor.clear.assert_called_once() + def test_reuse_blocked_when_float_image_is_none(self): + """Matching path/mtime with float_image=None must force a real reload, + not silently reuse a preview-only (float-less) session.""" + target = Path("test.jpg") + self.controller.image_editor.current_filepath = target + self.controller.image_editor.current_mtime = 123.4 + # Simulate a preview_only load: filepath/mtime set, but no float buffer. + self.controller.image_editor.float_image = None + self.controller.image_editor.load_image.return_value = True + + with patch("pathlib.Path.resolve", return_value=target.absolute()): + with patch("pathlib.Path.stat") as mock_stat: + mock_stat.return_value.st_mtime = 123.4 + res = self.controller.load_image_for_editing() + + # Must perform a real reload, not _REUSED + self.controller.image_editor.load_image.assert_called_once() + self.assertIsNot(res, AppController._REUSED) + + def test_crop_mode_blocked_while_saving(self): + """toggle_crop_mode must not enter crop mode when a save is in flight.""" + mock_file = MagicMock() + target = Path("test.jpg") + mock_file.path = target + self.controller.image_files = [mock_file] + self.controller.current_index = 0 + + # Put the image key in saving_keys + save_key = self.controller._key(target) + self.controller._saving_keys = {save_key} + self.controller.ui_state.isCropping = False + + with patch.object(self.controller, "load_image_for_editing") as mock_load: + self.controller.toggle_crop_mode() + + # isCropping must remain False + self.assertFalse(self.controller.ui_state.isCropping) + mock_load.assert_not_called() + + def test_crop_mode_blocked_when_load_fails(self): + """toggle_crop_mode must not set isCropping when load_image_for_editing fails.""" + self.controller._saving_keys = set() + self.controller.ui_state.isCropping = False + + with patch.object( + self.controller, "load_image_for_editing", return_value=False + ): + self.controller.toggle_crop_mode() + + self.assertFalse(self.controller.ui_state.isCropping) + + def test_crop_mode_blocked_no_image(self): + """toggle_crop_mode must not enter crop mode if no image is available.""" + self.controller.image_files = [] + self.controller.current_index = -1 + self.controller.ui_state.isCropping = False + + with patch.object(self.controller, "update_status_message") as mock_msg: + self.controller.toggle_crop_mode() + + self.assertFalse(self.controller.ui_state.isCropping) + mock_msg.assert_called_with("No image to crop") + + def test_save_finished_does_not_clear_editor_when_edits_rev_advanced(self): + """If _edits_rev changed after save started, _on_save_finished must not + call image_editor.clear() — the user has unsaved changes.""" + target = Path("test.jpg") + target_abs = self.controller._key(target) + self.controller.image_editor.current_filepath = target + self.controller.image_editor.session_id = "sess-1" + # _edits_rev at save-start was 5; user bumped it to 6 during the save + save_rev = 5 + self.controller.image_editor._edits_rev = 6 # newer than save token + + save_result = { + "success": True, + "result": (target, None), + "target": target_abs, + "save_image_key": target_abs, + "session_token": (target_abs, None, "sess-1", save_rev), + "editor_was_open": True, + "started_from_restore_override": False, + } + + # Patch list/refresh helpers the handler calls + with patch.object(self.controller, "refresh_image_list"): + with patch.object(self.controller, "sync_ui_state"): + self.controller._on_save_finished(save_result) + + # Token mismatch on _edits_rev → still_on_same_image is False → no clear + self.controller.image_editor.clear.assert_not_called() + + def test_save_finished_clears_editor_when_edits_rev_unchanged(self): + """Normal save completion (no edits made during save) must still clear + editor memory — the 4-part tokens are equal so still_on_same_image is True.""" + target = Path("test.jpg") + target_abs = self.controller._key(target) + self.controller.image_editor.current_filepath = target + self.controller.image_editor.session_id = "sess-1" + # _edits_rev same at save-start and now — user did not edit during save + rev = 5 + self.controller.image_editor._edits_rev = rev + + save_result = { + "success": True, + "result": (target, None), + "target": target_abs, + "save_image_key": target_abs, + "session_token": (target_abs, None, "sess-1", rev), + "editor_was_open": True, + "started_from_restore_override": False, + } + + with patch.object(self.controller, "refresh_image_list"): + with patch.object(self.controller, "sync_ui_state"): + self.controller._on_save_finished(save_result) + + # Tokens equal → still_on_same_image is True → editor_was_open → clear called + self.controller.image_editor.clear.assert_called_once() + + if __name__ == "__main__": unittest.main() From b37bd5c0083fa9924557087a0c4b8b3520e3e51e Mon Sep 17 00:00:00 2001 From: AlanRockefeller Date: Sun, 5 Apr 2026 14:06:50 -0700 Subject: [PATCH 2/6] format with black --- faststack/tests/test_editor_reopening.py | 1 - 1 file changed, 1 deletion(-) diff --git a/faststack/tests/test_editor_reopening.py b/faststack/tests/test_editor_reopening.py index 6734238..fa8e457 100644 --- a/faststack/tests/test_editor_reopening.py +++ b/faststack/tests/test_editor_reopening.py @@ -219,7 +219,6 @@ def test_editor_close_clears_memory_if_no_save_active(self): # VERIFY: Clear IS called because no save active for this file self.controller.image_editor.clear.assert_called_once() - def test_reuse_blocked_when_float_image_is_none(self): """Matching path/mtime with float_image=None must force a real reload, not silently reuse a preview-only (float-less) session.""" From 7784b02bbcd82883051e0ded582267dc42340972 Mon Sep 17 00:00:00 2001 From: AlanRockefeller Date: Sun, 5 Apr 2026 23:02:31 -0700 Subject: [PATCH 3/6] fix more crop box bugs --- faststack/app.py | 6 ++++-- faststack/tests/test_editor_reopening.py | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/faststack/app.py b/faststack/app.py index a6ff687..4275be8 100644 --- a/faststack/app.py +++ b/faststack/app.py @@ -6849,7 +6849,8 @@ def toggle_crop_mode(self): # Exiting crop mode: cleanup self.ui_state.isCropping = False self.ui_state.currentCropBox = (0, 0, 1000, 1000) - # Ensure preview rotation is cleared when exiting + # Ensure backend crop state and preview rotation are cleared when exiting + self.image_editor.set_crop_box(None) self.image_editor.set_edit_param("straighten_angle", 0.0) self.update_status_message("Crop cancelled") else: @@ -6869,8 +6870,9 @@ def toggle_crop_mode(self): return self.ui_state.isCropping = True - # Reset to full image defaults + # Reset to full image defaults (UI and Backend) self.ui_state.currentCropBox = (0, 0, 1000, 1000) + self.image_editor.set_crop_box(None) self.ui_state.aspectRatioNames = [r["name"] for r in ASPECT_RATIOS] self.ui_state.currentAspectRatioIndex = 0 diff --git a/faststack/tests/test_editor_reopening.py b/faststack/tests/test_editor_reopening.py index fa8e457..628020b 100644 --- a/faststack/tests/test_editor_reopening.py +++ b/faststack/tests/test_editor_reopening.py @@ -234,9 +234,10 @@ def test_reuse_blocked_when_float_image_is_none(self): mock_stat.return_value.st_mtime = 123.4 res = self.controller.load_image_for_editing() - # Must perform a real reload, not _REUSED + # Must perform a real reload, not _REUSED, and return truthy. self.controller.image_editor.load_image.assert_called_once() self.assertIsNot(res, AppController._REUSED) + self.assertTrue(res) def test_crop_mode_blocked_while_saving(self): """toggle_crop_mode must not enter crop mode when a save is in flight.""" From 3adaeeb342479547867b7955311c3e866cbf3c3a Mon Sep 17 00:00:00 2001 From: AlanRockefeller Date: Sun, 5 Apr 2026 23:30:04 -0700 Subject: [PATCH 4/6] updated toggle_crop_mode() to explicitly call self._kick_preview_worker() after resetting the backend crop and straighten angle parameters --- faststack/app.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/faststack/app.py b/faststack/app.py index 4275be8..227cdf5 100644 --- a/faststack/app.py +++ b/faststack/app.py @@ -6852,6 +6852,7 @@ def toggle_crop_mode(self): # Ensure backend crop state and preview rotation are cleared when exiting self.image_editor.set_crop_box(None) self.image_editor.set_edit_param("straighten_angle", 0.0) + self._kick_preview_worker() self.update_status_message("Crop cancelled") else: # Entering crop mode requires a loaded image with a valid float buffer. @@ -6878,6 +6879,7 @@ def toggle_crop_mode(self): # Reset rotation to 0 when starting fresh crop mode self.image_editor.set_edit_param("straighten_angle", 0.0) + self._kick_preview_worker() self.update_status_message("Crop mode: Drag to select area, Enter to crop") @Slot() From 0ca96991b947f5a23afb1aaa4a08be67176bfc07 Mon Sep 17 00:00:00 2001 From: AlanRockefeller Date: Wed, 8 Apr 2026 15:20:08 -0700 Subject: [PATCH 5/6] minor bug fixes --- faststack/app.py | 60 ++++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/faststack/app.py b/faststack/app.py index 227cdf5..dbb3b8e 100644 --- a/faststack/app.py +++ b/faststack/app.py @@ -1784,26 +1784,37 @@ def _on_save_finished(self, save_result: dict): getattr(self.image_editor, "_edits_rev", None), ) - still_on_same_image = ( + # Check whether the user is still viewing the same image/session + # (image key, variant kind, and session_id matching) + still_on_same_session = ( save_session_token is not None and current_session_token is not None - and current_session_token == save_session_token + and len(save_session_token) >= 3 + and current_session_token[:3] == save_session_token[:3] ) - if still_on_same_image: - # Clear Editor State (release memory) — only when the - # editor dialog was actually open for this save. - if editor_was_open: - if self.ui_state.isEditorOpen: - self.ui_state.isEditorOpen = False - # Closing triggers _on_editor_open_changed -> image_editor.clear() - # but we call it explicitly here just in case they closed it manually. - self.image_editor.clear() - - # Call this regardless of editor_was_open IF it was a restore-override - if save_result.get("started_from_restore_override"): - self._clear_variant_override() + # Check if it is the EXACT identical revision (no user edits since save started) + still_on_identical_revision = ( + still_on_same_session + and len(save_session_token) >= 4 + and current_session_token[3] == save_session_token[3] + ) + if still_on_same_session: + # 1. Editor Cleanup (only if revision is unchanged) + if still_on_identical_revision: + if editor_was_open: + if self.ui_state.isEditorOpen: + self.ui_state.isEditorOpen = False + # Closing triggers _on_editor_open_changed -> image_editor.clear() + # but we call it explicitly here just in case they closed it manually. + self.image_editor.clear() + + # Also clear variant override if we started from one + if save_result.get("started_from_restore_override"): + self._clear_variant_override() + + # 2. Update variants and re-select index # Refresh list to pick up new backup files and update variant map self.refresh_image_list() @@ -6834,26 +6845,21 @@ def cancel_crop_mode(self): """Cancel crop mode without applying changes.""" if self.ui_state.isCropping: self.ui_state.isCropping = False - self.ui_state.currentCropBox = [0, 0, 1000, 1000] - # Ensure preview rotation is cleared + self.ui_state.currentCropBox = (0, 0, 1000, 1000) + # Ensure backend crop state and preview rotation are cleared + self.image_editor.set_crop_box(None) self.image_editor.set_edit_param("straighten_angle", 0.0) - # Force QML to refresh if it's showing provider preview frames + # Notify UI and kick fresh render self.ui_refresh_generation += 1 - self.ui_state.currentImageSourceChanged.emit() + self._kick_preview_worker() self.update_status_message("Crop cancelled") @Slot() def toggle_crop_mode(self): """Toggle crop mode on/off.""" if self.ui_state.isCropping: - # Exiting crop mode: cleanup - self.ui_state.isCropping = False - self.ui_state.currentCropBox = (0, 0, 1000, 1000) - # Ensure backend crop state and preview rotation are cleared when exiting - self.image_editor.set_crop_box(None) - self.image_editor.set_edit_param("straighten_angle", 0.0) - self._kick_preview_worker() - self.update_status_message("Crop cancelled") + # Exiting crop mode: reuse the specialized cleanup + self.cancel_crop_mode() else: # Entering crop mode requires a loaded image with a valid float buffer. if not self.image_files or not ( From 8bcff9942ac85d4483bad0b1d7c2a7c6b7873bec Mon Sep 17 00:00:00 2001 From: AlanRockefeller Date: Wed, 8 Apr 2026 15:20:27 -0700 Subject: [PATCH 6/6] format with black --- faststack/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/faststack/app.py b/faststack/app.py index dbb3b8e..13910fa 100644 --- a/faststack/app.py +++ b/faststack/app.py @@ -1809,7 +1809,7 @@ def _on_save_finished(self, save_result: dict): # Closing triggers _on_editor_open_changed -> image_editor.clear() # but we call it explicitly here just in case they closed it manually. self.image_editor.clear() - + # Also clear variant override if we started from one if save_result.get("started_from_restore_override"): self._clear_variant_override()